From d3e76dda4dee038733b696096206049aa5cfe5f5 Mon Sep 17 00:00:00 2001 From: Jonathan Baldie Date: Mon, 31 Aug 2026 12:38:28 +0100 Subject: [PATCH 1/2] fix: read env vars by name instead of Deno.env.toObject() (#64) Deno.env.toObject() enumerates the entire process environment, which requires unrestricted --allow-env. A compiled binary built with a scoped --allow-env allowlist (as the Dockerfile/CI already declare) crashes on startup with a NotCapable error the moment main.ts calls toObject(), because the allowlist only grants access to specific variable names, not enumeration. Root cause: main.ts called Deno.env.toObject() while the Dockerfile and CI compile step already used a scoped --allow-env allowlist, so the binary they produced could never actually start. The build step alone couldn't have caught this because no CI job runs the compiled artifact. Fix reads each of the 7 known config vars individually via Deno.env.get(name), which respects the allowlist. Widened the Dockerfile/CI --allow-env list to include the two vars that were previously missing from it (QUEUE_DEPTH_LIMIT, QUEUE_COUNT_LIMIT, RATE_LIMIT_REQUESTS were already runtime-configurable per config.ts but not present in the compile-time allowlist). Added tests/compiled_binary_test.ts as a permanent regression seam: it compiles the actual binary with the scoped allowlist and asserts it reaches "Listening on", closing the gap that let this ship green previously. Fixes #64 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Q25veeccRvgA5zjp6Fhijq --- .github/workflows/ci.yml | 2 +- Dockerfile | 2 +- main.ts | 24 +++++++- tests/compiled_binary_test.ts | 102 ++++++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 3 deletions(-) create mode 100644 tests/compiled_binary_test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 81f21cf..060aea4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,7 +37,7 @@ jobs: --allow-read \ --allow-write=./persist.dat \ --allow-net=0.0.0.0:3000 \ - --allow-env=HOST,PORT,PERSIST,QUEUE_API_TOKEN \ + --allow-env=HOST,PORT,PERSIST,QUEUE_API_TOKEN,QUEUE_DEPTH_LIMIT,QUEUE_COUNT_LIMIT,RATE_LIMIT_REQUESTS \ main.ts - name: Run tests run: deno test --allow-read --allow-write --allow-net --allow-env --allow-run diff --git a/Dockerfile b/Dockerfile index 7576ed2..0f540b1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,7 +7,7 @@ COPY . /queue WORKDIR /queue -RUN deno compile --allow-read --allow-write=./persist.dat --allow-net=${DENO_HOST}:${DENO_PORT} --allow-env=HOST,PORT,PERSIST,QUEUE_API_TOKEN --allow-sys main.ts && cp ./queue /usr/bin/ +RUN deno compile --allow-read --allow-write=./persist.dat --allow-net=${DENO_HOST}:${DENO_PORT} --allow-env=HOST,PORT,PERSIST,QUEUE_API_TOKEN,QUEUE_DEPTH_LIMIT,QUEUE_COUNT_LIMIT,RATE_LIMIT_REQUESTS --allow-sys main.ts && cp ./queue /usr/bin/ RUN chown -R deno:deno /queue /usr/bin/queue diff --git a/main.ts b/main.ts index b7d6a72..4088e5b 100644 --- a/main.ts +++ b/main.ts @@ -9,7 +9,29 @@ function writeLog(message: string): void { Deno.stdout.writeSync(LOG_ENCODER.encode(`${message}\n`)); } -const CONFIG = parseConfig(Deno.env.toObject(), Deno.args); +// Read each config var by name rather than Deno.env.toObject(), which +// enumerates the entire process environment and therefore requires +// unrestricted env access. A compiled binary with a scoped --allow-env +// allowlist (see Dockerfile/CI) can only grant per-name access. +const ENV_VAR_NAMES = [ + "HOST", + "PORT", + "PERSIST", + "QUEUE_API_TOKEN", + "QUEUE_DEPTH_LIMIT", + "QUEUE_COUNT_LIMIT", + "RATE_LIMIT_REQUESTS", +] as const; + +function readEnv(): Record { + const env: Record = {}; + for (const name of ENV_VAR_NAMES) { + env[name] = Deno.env.get(name); + } + return env; +} + +const CONFIG = parseConfig(readEnv(), Deno.args); // Set up our persistency manager const PERSIST_ENGINE = CONFIG.persistEnabled diff --git a/tests/compiled_binary_test.ts b/tests/compiled_binary_test.ts new file mode 100644 index 0000000..b023c58 --- /dev/null +++ b/tests/compiled_binary_test.ts @@ -0,0 +1,102 @@ +// Regression coverage for the class of bug where `deno compile` permission +// flags don't match what main.ts actually needs at runtime. No other CI job +// executes the compiled artifact (the "test" job compiles it but then runs +// `deno test`; mutation/quality jobs use `deno run`), so a broken binary can +// ship green without these tests. +// +// Each test isolates one permission dimension by leaving the others +// unrestricted, mirroring how each bug was originally diagnosed. + +import { assertEquals } from "jsr:@std/assert@1.0"; + +async function compile(permFlags: string[], outPath: string): Promise { + const cmd = new Deno.Command(Deno.execPath(), { + args: ["compile", ...permFlags, "-o", outPath, "main.ts"], + cwd: ".", + stdout: "piped", + stderr: "piped", + }); + const { code, stderr } = await cmd.output(); + if (code !== 0) { + throw new Error(`compile failed: ${new TextDecoder().decode(stderr)}`); + } +} + +// Spawns a compiled binary and waits for it to either announce it's +// listening, or exit/crash. Returns the outcome so callers can assert on it. +async function probeStartup( + binPath: string, + args: string[], + env: Record, +): Promise<{ started: boolean; output: string }> { + const decoder = new TextDecoder(); + const child = new Deno.Command(binPath, { + args, + env, + stdout: "piped", + stderr: "piped", + }).spawn(); + + let buf = ""; + let started = false; + + const stdoutReader = child.stdout.getReader(); + const stderrReader = child.stderr.getReader(); + + const readAll = async (reader: ReadableStreamDefaultReader) => { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + buf += decoder.decode(value, { stream: true }); + if (buf.includes("Listening on")) { + started = true; + return; + } + } + }; + + const timeout = new Promise((resolve) => setTimeout(resolve, 3000)); + + await Promise.race([ + Promise.all([readAll(stdoutReader), readAll(stderrReader)]), + timeout, + ]); + + try { stdoutReader.releaseLock(); } catch { /* ignore */ } + try { stderrReader.releaseLock(); } catch { /* ignore */ } + try { child.kill("SIGKILL"); } catch { /* ignore */ } + try { await child.status; } catch { /* ignore */ } + + return { started, output: buf }; +} + +Deno.test({ + name: "compiled binary: starts with a scoped --allow-env allowlist (#64)", + fn: async () => { + const tempDir = await Deno.makeTempDir({ prefix: "queue-compile-test-" }); + const outPath = `${tempDir}/queue`; + try { + await compile( + [ + "--allow-read", + "--allow-write", + "--allow-net", + "--allow-env=HOST,PORT,PERSIST,QUEUE_API_TOKEN,QUEUE_DEPTH_LIMIT,QUEUE_COUNT_LIMIT,RATE_LIMIT_REQUESTS", + "--allow-sys", + ], + outPath, + ); + const { started, output } = await probeStartup(outPath, [], { + QUEUE_API_TOKEN: "compile-test-token", + HOST: "127.0.0.1", + PORT: "0", + }); + assertEquals(started, true, `binary did not report listening. Output:\n${output}`); + } finally { + await Deno.remove(tempDir, { recursive: true }).catch(() => {}); + } + }, + // Compiling a 100MB+ binary is slow; this is an integration test, not a unit test. + sanitizeResources: false, + sanitizeOps: false, +}); From 2c3ac2f69db2567fd93c0d34c3aa4b8e8b073293 Mon Sep 17 00:00:00 2001 From: Jonathan Baldie Date: Mon, 31 Aug 2026 12:49:55 +0100 Subject: [PATCH 2/2] fix: exclude compiled_binary_test.ts from Stryker's test run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Stryker commandRunner runs the full deno test suite (coverageAnalysis is off, so every mutant re-runs it in full). Two problems surfaced when tests/compiled_binary_test.ts (added for #64) was included: 1. Stryker instruments src/persist.ts with mutant-switching code that reads process.env.__STRYKER_ACTIVE_MUTANT__. compiled_binary_test.ts compiles main.ts with a fixed, scoped --allow-env allowlist that doesn't grant that name, so the inner compiled binary crashed with NotCapable during Stryker's dry run — failing CI on PR #71. 2. Even without that crash, spawning `deno compile` (a ~95MB binary) inside every mutant's test run would multiply the Stryker job's runtime by however many mutants exist in src/**/*.ts, which the 30s per-mutant timeout can't absorb. compiled_binary_test.ts is a build/integration test, not a unit test covering src/ mutations, so it has nothing to contribute to Stryker's mutation score. Excluded it via --ignore on the commandRunner, mirroring how mutation/mutasaurus_ci.ts already scopes to an explicit test file list that doesn't include it. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Q25veeccRvgA5zjp6Fhijq --- mutation/stryker.config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mutation/stryker.config.json b/mutation/stryker.config.json index 995113e..647c6c8 100644 --- a/mutation/stryker.config.json +++ b/mutation/stryker.config.json @@ -3,7 +3,7 @@ "mutate": ["src/**/*.ts"], "testRunner": "command", "commandRunner": { - "command": "deno test --allow-read --allow-write --allow-net --allow-env --allow-run --no-check" + "command": "deno test --allow-read --allow-write --allow-net --allow-env --allow-run --no-check --ignore=tests/compiled_binary_test.ts" }, "reporters": ["json", "clear-text"], "jsonReporter": {