Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docker-compose.api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ services:
- 1.1.1.1
volumes:
- docker_git_projects:${DOCKER_GIT_PROJECTS_ROOT:-/home/dev/.docker-git}
- docker_git_docker_data:/var/lib/docker
- /var/lib/docker:/var/lib/docker
- /var/run/docker.sock:/var/run/docker.sock
privileged: ${DOCKER_GIT_CONTROLLER_PRIVILEGED:-false}
cgroup: host
Expand Down
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ services:
- 1.1.1.1
volumes:
- docker_git_projects:${DOCKER_GIT_PROJECTS_ROOT:-/home/dev/.docker-git}
- docker_git_docker_data:/var/lib/docker
- /var/lib/docker:/var/lib/docker
- /var/run/docker.sock:/var/run/docker.sock
privileged: ${DOCKER_GIT_CONTROLLER_PRIVILEGED:-false}
cgroup: host
Expand Down
7 changes: 7 additions & 0 deletions packages/api/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ RUN if [ "$DOCKER_GIT_CONTROLLER_BUILD_SKILLER" = "1" ]; then \
rm -rf /root/.bun/install/cache node_modules; \
sleep $((attempt * 2)); \
done \
&& electron_zip="$(find "${electron_config_cache:-/root/.cache/electron}" -name 'electron-v*-linux-*.zip' -print -quit)" \
&& test -n "$electron_zip" \
&& rm -rf node_modules/electron/dist node_modules/electron/path.txt \
&& mkdir -p node_modules/electron/dist \
&& unzip -q "$electron_zip" -d node_modules/electron/dist \
&& printf '%s' electron > node_modules/electron/path.txt \
&& test -x node_modules/electron/dist/electron \
Comment thread
coderabbitai[bot] marked this conversation as resolved.
&& bun run build \
&& touch out/.docker-git-browser-folder-picker.patch \
&& mkdir -p out/preload \
Expand Down
18 changes: 10 additions & 8 deletions packages/api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@ This is now the intended controller plane:
## Runtime contract: host-Docker-backed

`docker-git` is host-Docker-backed by default. The primary controller
container created from this package binds the host socket
(`/var/run/docker.sock:/var/run/docker.sock`, see `docker-compose.yml`) and
uses it to spawn per-project containers. `DOCKER_GIT_DOCKER_RUNTIME=isolated`
is an opt-in fallback for environments that explicitly require an embedded
controller daemon. In isolated mode, start the controller through the host CLI
or include `docker-compose.isolated.yml`; that overlay removes the host socket
bind and defaults project containers to the embedded daemon endpoint
`tcp://host.docker.internal:2375`.
container created from this package binds the host socket and Docker data root
(`/var/run/docker.sock:/var/run/docker.sock` and
`/var/lib/docker:/var/lib/docker`, see `docker-compose.yml`) and uses them to
spawn per-project containers and access the Docker volume paths reported by
`docker inspect`. `DOCKER_GIT_DOCKER_RUNTIME=isolated` is an opt-in fallback for
environments that explicitly require an embedded controller daemon. In isolated
mode, start the controller through the host CLI or include
`docker-compose.isolated.yml`; that overlay removes the host socket bind, keeps
Docker data inside the controller volume, and defaults project containers to the
embedded daemon endpoint `tcp://host.docker.internal:2375`.

Security note: binding `/var/run/docker.sock` gives the controller container
root-equivalent control over the host Docker daemon, including the ability to
Expand Down
24 changes: 6 additions & 18 deletions packages/api/src/services/skiller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,22 +363,10 @@ const prepareSkillerScopeHome = (scope: SkillerContainerScope | null): SkillerPr
return processUser
}

const skillerLaunchCommand = (
user: SkillerProcessUser | null
): readonly [string, ReadonlyArray<string>] =>
user === null
? ["bash", ["-lc", launchScript]]
: [
"setpriv",
[
`--reuid=${user.uid}`,
`--regid=${user.gid}`,
"--clear-groups",
"bash",
"-lc",
launchScript
]
]
// Electron aborts under setpriv in the controller image even with --no-sandbox.
// Project scope still comes from explicit host paths and the browser bootstrap.
export const skillerLaunchCommand = (): readonly [string, ReadonlyArray<string>] =>
["bash", ["-lc", launchScript]]
Comment thread
skulidropek marked this conversation as resolved.
Outdated

const stopSkillerProcess = (process: SkillerProcess): void => {
const pid = process.process.pid
Expand Down Expand Up @@ -422,10 +410,10 @@ const launchSkillerProcess = (
scope: SkillerContainerScope | null
): SkillerLaunch => {
mkdirSync(dirname(launchLogPath), { recursive: true })
const processUser = prepareSkillerScopeHome(scope)
prepareSkillerScopeHome(scope)
const logFd = openSync(launchLogPath, "a")
try {
const [command, args] = skillerLaunchCommand(processUser)
const [command, args] = skillerLaunchCommand()
const child = spawn(command, args, {
cwd: skillerDir,
detached: true,
Expand Down
9 changes: 9 additions & 0 deletions packages/api/tests/skiller-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
parseSkillerRoute,
resolveSkillerBrowserScopeSelection,
resolveSkillerRouteScopeSelection,
skillerLaunchCommand,
type SkillerRoute
} from "../src/services/skiller.js"
import type { SkillerContainerScope } from "../src/services/skiller-core.js"
Expand Down Expand Up @@ -31,6 +32,14 @@ const scope = (projectKey: string): SkillerContainerScope => ({
})

describe("skiller routes", () => {
it("launches Electron as the controller process user", () => {
const [command, args] = skillerLaunchCommand()

expect(command).toBe("bash")
expect(args.join(" ")).not.toContain("setpriv")
expect(args).toContainEqual(expect.stringContaining("node_modules/electron/dist/electron"))
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

it("keeps the terminal session id on session-scoped app routes", () => {
expect(parseSkillerRoute("/api/ssh/session/terminal-proof/skiller/app/")).toEqual({
_tag: "App",
Expand Down
28 changes: 28 additions & 0 deletions packages/app/tests/docker-git/controller-resource-limits.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {

const composeFiles: ReadonlyArray<string> = ["docker-compose.yml", "docker-compose.api.yml"]
const isolatedComposeFiles: ReadonlyArray<string> = ["docker-compose.isolated.yml", "docker-compose.api.isolated.yml"]
const hostDockerDataBind = "/var/lib/docker:/var/lib/docker"
const isolatedDockerDataVolume = "docker_git_docker_data:/var/lib/docker"

const readComposeFile = (relativePath: string): Effect.Effect<string> =>
Effect.gen(function*(_) {
Expand Down Expand Up @@ -50,6 +52,13 @@ describe("controller compose resource limits", () => {
const contents = yield* _(readComposeFile(composeFile))
expect(contents).toMatch(/pids_limit: \$\{DOCKER_GIT_CONTROLLER_PIDS:-\d+\}/u)
}))

it.effect("binds host Docker data root for host runtime volume path access", () =>
Effect.gen(function*(_) {
const contents = yield* _(readComposeFile(composeFile))
expect(contents).toContain(`- ${hostDockerDataBind}`)
expect(contents).not.toContain(`- ${isolatedDockerDataVolume}`)
}))
})
}

Expand All @@ -75,10 +84,29 @@ describe("controller compose resource limits", () => {
const contents = yield* _(readComposeFile(composeFile))
expect(contents).toContain("privileged: ${DOCKER_GIT_CONTROLLER_PRIVILEGED:-true}")
}))

it.effect("keeps Docker data inside the embedded controller daemon volume", () =>
Effect.gen(function*(_) {
const contents = yield* _(readComposeFile(composeFile))
expect(contents).toContain(`- ${isolatedDockerDataVolume}`)
expect(contents).not.toContain(`- ${hostDockerDataBind}`)
}))
})
}
})

describe("controller Skiller Dockerfile", () => {
it.effect("materializes Electron binary before bundling Skiller", () =>
Effect.gen(function*(_) {
const contents = yield* _(readComposeFile("packages/api/Dockerfile"))
expect(contents).toContain(
`electron_zip="$(find "\${electron_config_cache:-/root/.cache/electron}" -name 'electron-v*-linux-*.zip' -print -quit)"`
)
expect(contents).toContain("unzip -q \"$electron_zip\" -d node_modules/electron/dist")
expect(contents).toContain("test -x node_modules/electron/dist/electron")
}))
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

describe("controller resource limit resolution", () => {
it.effect("resolves CPU and RAM defaults to 90% of host resources", () =>
Effect.sync(() => {
Expand Down
Loading