diff --git a/supervisor/src/docker.ts b/supervisor/src/docker.ts index 6938bb954..daf2ecde4 100644 --- a/supervisor/src/docker.ts +++ b/supervisor/src/docker.ts @@ -308,7 +308,19 @@ export type EnsureOptions = { * a test that wants an answer in seconds. */ readyTimeoutMs?: number; - pidsLimit?: number; + /** + * A ceiling on the container's own process count. Defaults to 512 when left unset. + * + * Explicit `null` omits the field entirely rather than falling back to the default — for a + * host whose Docker daemon cannot register a PID cgroup limit at all (a rootless daemon whose + * systemd cgroup driver cannot complete a scope registration is a known case: creation fails + * with an OCI runtime error before the computer ever starts, not a soft degradation). Losing + * this control on such a host is a real reduction in defense-in-depth, not a free choice — a + * deployment disabling it is expected to compensate elsewhere (run timeouts, per-run + * concurrency ceilings, a watchdog that kills leaked containers, host-level process-count + * alerting), not merely to make computer creation succeed. + */ + pidsLimit?: number | null; /** * The volume holding the SPIRE agent's Workload API socket, mounted read-only into each computer * so it can ask what it is. Unset means no identity, which is a deployment choice rather than a @@ -361,7 +373,9 @@ function hostConfig(names: ComputerNames, options: EnsureOptions) { CapDrop: ["ALL"], // A runaway Bot is a resource problem for itself, not for every other Bot on the host. ...(options.memoryBytes ? { Memory: options.memoryBytes } : {}), - PidsLimit: options.pidsLimit ?? 512, + ...(options.pidsLimit === null + ? {} + : { PidsLimit: options.pidsLimit ?? 512 }), // Chromium's sandbox wants shared memory and will crash on the 64MB default. ShmSize: 1_073_741_824, }; diff --git a/supervisor/src/index.ts b/supervisor/src/index.ts index 41fc2e08c..5953031f1 100644 --- a/supervisor/src/index.ts +++ b/supervisor/src/index.ts @@ -57,6 +57,19 @@ const runtime = process.env.COMPUTER_RUNTIME; const memoryBytes = process.env.COMPUTER_MEMORY_BYTES ? Number.parseInt(process.env.COMPUTER_MEMORY_BYTES, 10) : undefined; +/** + * Unset (the variable absent) leaves the 512 default in `docker.ts`. Set to the literal empty + * string, it means "this daemon cannot honour a PID limit" and is passed through as `null`, + * which omits the field from the container's HostConfig entirely rather than failing creation — + * see the `pidsLimit` doc comment in `docker.ts` for what a deployment must compensate with + * before disabling this. + */ +const pidsLimit = + process.env.COMPUTER_PIDS_LIMIT === "" + ? null + : process.env.COMPUTER_PIDS_LIMIT + ? Number.parseInt(process.env.COMPUTER_PIDS_LIMIT, 10) + : undefined; const spireSocketVolume = process.env.SPIRE_AGENT_SOCKET_VOLUME; /** @@ -127,6 +140,7 @@ app.post("/computers/:botId/ensure", async (context) => { ...(network ? { network } : {}), ...(runtime ? { runtime } : {}), ...(memoryBytes ? { memoryBytes } : {}), + ...(pidsLimit === undefined ? {} : { pidsLimit }), ...(spireSocketVolume ? { spireSocketVolume } : {}), }); return context.json({ diff --git a/supervisor/tests/docker.integration.test.ts b/supervisor/tests/docker.integration.test.ts index 57653c1ee..67ea46d4d 100644 --- a/supervisor/tests/docker.integration.test.ts +++ b/supervisor/tests/docker.integration.test.ts @@ -157,6 +157,46 @@ describe.skipIf(runtime === null)("a computer that never answers", () => { }, 90_000); }); +describe.skipIf(runtime === null)( + "the PID limit a computer is created with", + () => { + /* + * A rootless Docker daemon whose systemd cgroup driver cannot register a scope for a PID + * limit fails container creation outright — an OCI runtime error, not a soft degradation + * (found running this supervisor against a rootless enclave, #20). `pidsLimit: null` is the + * escape hatch: omit the field so creation does not depend on that cgroup path at all, rather + * than silently keep asking for a limit the daemon cannot grant. + */ + test("defaults to 512", async () => { + await withDocker().supervisor.ensure(names, { + image: IMAGE, + environment: [], + }); + + const info = await withDocker() + .docker.getContainer(names.container) + .inspect(); + expect(info.HostConfig?.PidsLimit).toBe(512); + }, 90_000); + + test("explicit null omits it rather than falling back to the default", async () => { + await withDocker().supervisor.ensure(names, { + image: IMAGE, + environment: [], + pidsLimit: null, + }); + + const info = await withDocker() + .docker.getContainer(names.container) + .inspect(); + // "No limit configured", not the 512 default — this daemon reports that as `null` rather + // than `0`, which is itself daemon-version-dependent, so the assertion is on the absence of + // a limit rather than pinning a specific sentinel value. + expect(info.HostConfig?.PidsLimit).toBeFalsy(); + }, 90_000); + }, +); + describe.skipIf(runtime === null)( "a computer built from an older image", () => {