Skip to content

Commit 1076ed0

Browse files
committed
fix(api): address skiller review follow-ups
1 parent b652ffb commit 1076ed0

3 files changed

Lines changed: 15 additions & 10 deletions

File tree

packages/api/Dockerfile

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,10 @@ RUN if [ "$DOCKER_GIT_CONTROLLER_BUILD_SKILLER" = "1" ]; then \
122122
sleep $((attempt * 2)); \
123123
done \
124124
&& electron_zip="$(find "${electron_config_cache:-/root/.cache/electron}" -name 'electron-v*-linux-*.zip' -print -quit)" \
125-
&& test -n "$electron_zip" \
125+
&& if [ -z "$electron_zip" ]; then echo "Electron zip not found in cache: ${electron_config_cache:-/root/.cache/electron}" >&2; exit 1; fi \
126+
&& unzip -Z1 "$electron_zip" > /tmp/electron-zip-entries \
127+
&& if grep -Eq '(^/|(^|/)\.\.($|/))' /tmp/electron-zip-entries; then echo "Unsafe paths in Electron zip: $electron_zip" >&2; exit 1; fi \
128+
&& rm -f /tmp/electron-zip-entries \
126129
&& rm -rf node_modules/electron/dist node_modules/electron/path.txt \
127130
&& mkdir -p node_modules/electron/dist \
128131
&& unzip -q "$electron_zip" -d node_modules/electron/dist \

packages/api/tests/skiller-routes.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,13 @@ const scope = (projectKey: string): SkillerContainerScope => ({
3232
})
3333

3434
describe("skiller routes", () => {
35-
it("launches Electron as the controller process user", () => {
35+
it("launches Electron through the Skiller launch script", () => {
3636
const [command, args] = skillerLaunchCommand()
37+
const launchCommand = args.join("\n")
3738

3839
expect(command).toBe("bash")
39-
expect(args.join(" ")).not.toContain("setpriv")
40-
expect(args).toContainEqual(expect.stringContaining("node_modules/electron/dist/electron"))
40+
expect(launchCommand).toContain("xvfb-run -a ./node_modules/electron/dist/electron")
41+
expect(launchCommand).toContain("exec ./node_modules/electron/dist/electron")
4142
})
4243

4344
it("keeps the terminal session id on session-scoped app routes", () => {

packages/app/tests/docker-git/controller-resource-limits.test.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,16 @@ describe("controller compose resource limits", () => {
9595
}
9696
})
9797

98-
describe("controller Skiller Dockerfile", () => {
98+
describe("API Dockerfile Electron materialization", () => {
9999
it.effect("materializes Electron binary before bundling Skiller", () =>
100100
Effect.gen(function*(_) {
101101
const contents = yield* _(readComposeFile("packages/api/Dockerfile"))
102-
expect(contents).toContain(
103-
`electron_zip="$(find "\${electron_config_cache:-/root/.cache/electron}" -name 'electron-v*-linux-*.zip' -print -quit)"`
104-
)
105-
expect(contents).toContain("unzip -q \"$electron_zip\" -d node_modules/electron/dist")
106-
expect(contents).toContain("test -x node_modules/electron/dist/electron")
102+
expect(contents).toMatch(/electron_zip="\$\(find "\$\{electron_config_cache:-\/root\/\.cache\/electron\}"/u)
103+
expect(contents).toMatch(/Electron zip not found in cache/u)
104+
expect(contents).toMatch(/unzip -Z1 "\$electron_zip"/u)
105+
expect(contents).toMatch(/Unsafe paths in Electron zip/u)
106+
expect(contents).toMatch(/unzip -q "\$electron_zip" -d node_modules\/electron\/dist/u)
107+
expect(contents).toMatch(/test -x node_modules\/electron\/dist\/electron/u)
107108
}))
108109
})
109110

0 commit comments

Comments
 (0)