From a82cfda3f763046d4383ba0ed94c23b604ff7470 Mon Sep 17 00:00:00 2001 From: Bit Cloud Date: Thu, 3 Sep 2026 20:57:34 +0000 Subject: [PATCH] feat: make a computer's PID limit optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found running supervisor against a rootless-Docker enclave (bit-mind #20): container creation for every computer fails outright on a daemon whose systemd cgroup driver cannot register a scope for a PID limit — an OCI runtime error ("systemd error: Interactive authentication required"), not a soft degradation. Reproduced with both runc and runsc (gVisor), and with COMPUTER_MEMORY_BYTES unset, isolating it to the unconditional `PidsLimit: options.pidsLimit ?? 512` in hostConfig(). `pidsLimit` on EnsureOptions is now `number | null`. Unset keeps today's behavior (512 default). Explicit `null` omits the field from the container's HostConfig entirely, so creation no longer depends on that cgroup path at all. Wired to a new COMPUTER_PIDS_LIMIT env var: unset leaves the default, set to the empty string disables the limit. This is a real reduction in defense-in-depth on a host that needs it, not a free choice — documented on the EnsureOptions field and the env var comment in index.ts that a deployment disabling it should compensate elsewhere (run timeouts, per-run concurrency ceilings, a watchdog killing leaked containers, host-level process-count alerting). Root cause (systemd cgroup driver + rootless Docker + D-Bus scope registration) is a separate, host-level infrastructure question, not fixed here — this unblocks the product path while that gets investigated. Two new integration tests in docker.integration.test.ts (against a real Docker daemon, following the file's existing pattern) cover both the unchanged default and the new omission. Co-Authored-By: Claude Sonnet 5 --- supervisor/src/docker.ts | 18 ++++++++-- supervisor/src/index.ts | 14 ++++++++ supervisor/tests/docker.integration.test.ts | 40 +++++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) 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", () => {