Skip to content

Commit b652ffb

Browse files
committed
fix(api): make bundled skiller launch reproducible
1 parent 6eabb7e commit b652ffb

4 files changed

Lines changed: 34 additions & 18 deletions

File tree

packages/api/Dockerfile

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,13 @@ RUN if [ "$DOCKER_GIT_CONTROLLER_BUILD_SKILLER" = "1" ]; then \
121121
rm -rf /root/.bun/install/cache node_modules; \
122122
sleep $((attempt * 2)); \
123123
done \
124+
&& electron_zip="$(find "${electron_config_cache:-/root/.cache/electron}" -name 'electron-v*-linux-*.zip' -print -quit)" \
125+
&& test -n "$electron_zip" \
126+
&& rm -rf node_modules/electron/dist node_modules/electron/path.txt \
127+
&& mkdir -p node_modules/electron/dist \
128+
&& unzip -q "$electron_zip" -d node_modules/electron/dist \
129+
&& printf '%s' electron > node_modules/electron/path.txt \
130+
&& test -x node_modules/electron/dist/electron \
124131
&& bun run build \
125132
&& touch out/.docker-git-browser-folder-picker.patch \
126133
&& mkdir -p out/preload \

packages/api/src/services/skiller.ts

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -363,22 +363,10 @@ const prepareSkillerScopeHome = (scope: SkillerContainerScope | null): SkillerPr
363363
return processUser
364364
}
365365

366-
const skillerLaunchCommand = (
367-
user: SkillerProcessUser | null
368-
): readonly [string, ReadonlyArray<string>] =>
369-
user === null
370-
? ["bash", ["-lc", launchScript]]
371-
: [
372-
"setpriv",
373-
[
374-
`--reuid=${user.uid}`,
375-
`--regid=${user.gid}`,
376-
"--clear-groups",
377-
"bash",
378-
"-lc",
379-
launchScript
380-
]
381-
]
366+
// Electron aborts under setpriv in the controller image even with --no-sandbox.
367+
// Project scope still comes from explicit host paths and the browser bootstrap.
368+
export const skillerLaunchCommand = (): readonly [string, ReadonlyArray<string>] =>
369+
["bash", ["-lc", launchScript]]
382370

383371
const stopSkillerProcess = (process: SkillerProcess): void => {
384372
const pid = process.process.pid
@@ -422,10 +410,10 @@ const launchSkillerProcess = (
422410
scope: SkillerContainerScope | null
423411
): SkillerLaunch => {
424412
mkdirSync(dirname(launchLogPath), { recursive: true })
425-
const processUser = prepareSkillerScopeHome(scope)
413+
prepareSkillerScopeHome(scope)
426414
const logFd = openSync(launchLogPath, "a")
427415
try {
428-
const [command, args] = skillerLaunchCommand(processUser)
416+
const [command, args] = skillerLaunchCommand()
429417
const child = spawn(command, args, {
430418
cwd: skillerDir,
431419
detached: true,

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
parseSkillerRoute,
55
resolveSkillerBrowserScopeSelection,
66
resolveSkillerRouteScopeSelection,
7+
skillerLaunchCommand,
78
type SkillerRoute
89
} from "../src/services/skiller.js"
910
import type { SkillerContainerScope } from "../src/services/skiller-core.js"
@@ -31,6 +32,14 @@ const scope = (projectKey: string): SkillerContainerScope => ({
3132
})
3233

3334
describe("skiller routes", () => {
35+
it("launches Electron as the controller process user", () => {
36+
const [command, args] = skillerLaunchCommand()
37+
38+
expect(command).toBe("bash")
39+
expect(args.join(" ")).not.toContain("setpriv")
40+
expect(args).toContainEqual(expect.stringContaining("node_modules/electron/dist/electron"))
41+
})
42+
3443
it("keeps the terminal session id on session-scoped app routes", () => {
3544
expect(parseSkillerRoute("/api/ssh/session/terminal-proof/skiller/app/")).toEqual({
3645
_tag: "App",

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

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

98+
describe("controller Skiller Dockerfile", () => {
99+
it.effect("materializes Electron binary before bundling Skiller", () =>
100+
Effect.gen(function*(_) {
101+
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")
107+
}))
108+
})
109+
98110
describe("controller resource limit resolution", () => {
99111
it.effect("resolves CPU and RAM defaults to 90% of host resources", () =>
100112
Effect.sync(() => {

0 commit comments

Comments
 (0)