fix: read env vars by name instead of Deno.env.toObject() (#64)

.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

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
 

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<string, string | undefined> {
+    const env: Record<string, string | undefined> = {};
+    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

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": {

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<void> {
+    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<string, string>,
+): 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<Uint8Array>) => {
+        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<void>((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,
+});