Self-audit: agent could overwrite its own guardrails; missing-env was retryable

Two bugs found by inspecting the built image before the review agents reported.

BLOCKER -- the agent owned its own constraints. The runner runs AS the `pi` user, and the
Dockerfile COPY --chown=pi:pi'd the guardrails, the runner, and (via chown -R /app) pi's own
node_modules. Verified against the real image: as pi, all three were writable.

  echo x > /opt/pi-dispatch/HARD_RULES.md   # succeeded
  echo x > /app/image/runner/run-job.mjs    # succeeded
  echo x > /app/node_modules/.poison        # succeeded

/job:ro exists precisely so the agent cannot rewrite its instructions; leaving its code and
its safety floor agent-writable is the identical hole through a different door. The container
is ephemeral so it is not trivially exploitable within one run, but that argument is too
fragile to rest a security property on -- if anything re-reads the guardrails mid-session
(compaction, which is OQ-003) the floor is poisonable, and the fix costs nothing.

Only ~/.pi/agent (pi writes auth.json) and /ms-playwright (pi reads browsers) are chowned to
pi now. The guardrails and /app are root-owned and `chmod -R a-w`; pi reads and executes them,
which is all it needs. Verified: all three now Permission denied to pi, pi still imports the
package, agent dir still writable.

Correctness -- a missing required env var exited 1 (retryable). requireEnv threw
"missing required env: PI_PROVIDER", which classifyThrow's config regex did not match, so it
fell to EXIT_INFRA. But the worker passes the same absent value on every retry, so the queue
would pay to rediscover a deterministic misconfiguration. Now matched -> exit 2. Verified in
the image: the runner exits 2. Added classify-throw.test.mjs covering the config/infra split.

Author: edgehero

image/Dockerfile

@@ -91,28 +91,35 @@ ENV PLAYWRIGHT_MCP_BROWSER=chromium \
 # Non-root. CONST-ISOLATION-CONTAINER-PER-JOB.
 RUN useradd --create-home --shell /bin/bash --uid 1001 pi
 
-# pi lazily creates ~/.pi/agent (0700) and writes auth.json (0600) on the FIRST credential
-# operation -- ensureParentDir/ensureFileExists run inside withLock. Root-owned means the
-# job dies EACCES at runtime, inside the container, on a path nothing here hints at.
+# ONLY these two are chowned to pi, because they are the only things the runtime user must
+# WRITE. Everything else it merely reads or executes, and code the agent can rewrite is not a
+# boundary -- see the guardrails/runner COPYs below.
 #
-# `COPY --chown` alone does NOT fix this: it does not apply to parent directories COPY
-# auto-creates, so /home/pi/.pi would still be born root:root. The trap survives the
-# obvious fix. Create and chown explicitly.
+# ~/.pi/agent: pi lazily creates it (0700) and writes auth.json (0600) on the FIRST credential
+# operation (ensureParentDir/ensureFileExists inside withLock). Root-owned means the job dies
+# EACCES at runtime, on a path nothing here hints at. `COPY --chown` would not fix this anyway:
+# it does not apply to parent dirs COPY auto-creates, so /home/pi/.pi would still be root:root.
+# /ms-playwright: the runtime user must be able to read the root-installed browser binaries.
 RUN mkdir -p /home/pi/.pi/agent \
     && chown -R pi:pi /home/pi/.pi \
-    && chown -R pi:pi /ms-playwright \
-    && chown -R pi:pi /app
+    && chown -R pi:pi /ms-playwright
 
-# The safety floor. Deliberately NOT at ~/.pi/agent/APPEND_SYSTEM.md: a trusted project's
-# .pi/APPEND_SYSTEM.md shadows that path via an early return in discoverAppendSystemPromptFile,
-# which would delete these rules from the prompt with no error and a job that succeeds. The
-# runner reads this path explicitly instead, so discovery cannot shadow it.
-COPY --chown=pi:pi guardrails/HARD_RULES.md /opt/pi-dispatch/HARD_RULES.md
+# The safety floor and the runner are ROOT-OWNED and NOT writable by pi. The agent runs AS pi;
+# if it could overwrite /opt/pi-dispatch/HARD_RULES.md it would own its own constraints, and if
+# it could overwrite /app it would own the runner and pi itself. `/job:ro` exists precisely so
+# the agent cannot rewrite its instructions -- leaving its code agent-writable is the identical
+# hole through a different door. Root owns it; pi reads it (npm's 0644/0755 defaults suffice).
+#
+# The guardrails path is deliberately NOT ~/.pi/agent/APPEND_SYSTEM.md: a trusted project's
+# .pi/APPEND_SYSTEM.md shadows that via an early return in discoverAppendSystemPromptFile, which
+# would delete the floor from the prompt with no error. The runner reads this path explicitly.
+COPY guardrails/HARD_RULES.md /opt/pi-dispatch/HARD_RULES.md
 
 # Source last: everything above is cacheable and this layer changes on every commit.
-COPY --chown=pi:pi image/runner /app/image/runner
-COPY --chown=pi:pi image/entrypoint.sh /entrypoint.sh
-RUN chmod +x /entrypoint.sh
+COPY image/runner /app/image/runner
+COPY image/entrypoint.sh /entrypoint.sh
+RUN chmod +x /entrypoint.sh \
+    && chmod -R a-w /opt/pi-dispatch /app/image/runner /app/node_modules
 
 USER pi
 WORKDIR /workspace

image/runner/src/outcome.mjs

@@ -26,8 +26,10 @@ export const STOP_REASONS = ["stop", "length", "toolUse", "error", "aborted"];
 export function classifyThrow(error) {
 	const message = error instanceof Error ? error.message : String(error);
 
-	// Config errors. Retrying cannot fix these, so do not let the queue try.
-	if (/no model|model not|no api key|no.*credential|not authenticated/i.test(message)) {
+	// Config errors. Retrying cannot fix these, so do not let the queue try. A missing required
+	// env var is deterministic misconfiguration -- the worker will pass the same (absent) value
+	// on every retry, so exit 2 (not retried), not 1 (retryable).
+	if (/no model|model not|no api key|no.*credential|not authenticated|missing required env/i.test(message)) {
 		return { code: EXIT_POLICY, reason: "config", message };
 	}
 

image/runner/test/classify-throw.test.mjs

@@ -0,0 +1,28 @@
+import assert from "node:assert/strict";
+import { test } from "node:test";
+import { classifyThrow, EXIT_INFRA, EXIT_POLICY } from "../src/outcome.mjs";
+
+// Retrying a deterministic config error pays to rediscover it. Every string a real
+// deployment throws before the agent loop must land on exit 2, not the retryable 1.
+test("config errors are exit 2 (not retried), not exit 1", () => {
+	for (const message of [
+		"missing required env: PI_PROVIDER",
+		"missing required env: PI_MODEL",
+		"No model selected",
+		"No API key found for provider anthropic",
+		"provider anthropic is not authenticated",
+	]) {
+		assert.equal(classifyThrow(new Error(message)).code, EXIT_POLICY, message);
+	}
+});
+
+test("genuine infra/our-bug errors stay retryable (exit 1)", () => {
+	for (const message of ["Agent is already processing.", "ECONNRESET", "socket hang up"]) {
+		assert.equal(classifyThrow(new Error(message)).code, EXIT_INFRA, message);
+	}
+});
+
+test("a non-Error throw is classified, not crashed on", () => {
+	assert.equal(classifyThrow("bare string").code, EXIT_INFRA);
+	assert.equal(classifyThrow(undefined).code, EXIT_INFRA);
+});