Fail startup when NAD_POLICY names a file that is not there

README.md

@@ -58,7 +58,9 @@ the wallet enforces it before every confirmation prompt:
   rather than silently skipping a rule the wallet cannot evaluate.
 
 Every field is optional and **no file means no policy**, so behavior is unchanged unless you opt
-in. A violation is refused before the prompt, naming the rule
+in. The one exception is a file you asked for: if `NAD_POLICY` names a path that does not exist,
+startup stops and says so, because a typo there would otherwise run the agent with none of the
+limits you configured. A violation is refused before the prompt, naming the rule
 (`policy maxPerSend: amount 0.9 MON is above the policy limit of 0.5 MON per send.`), and the
 confirmation block gains a `Policy:` line showing what applied. A malformed policy file stops the
 agent at startup instead of being ignored.

src/policy.mjs

@@ -22,33 +22,55 @@ const DEFAULT_PATH = resolve(HERE, "..", "policy.json");
  * Load and validate `policy.json` (override with NAD_POLICY). Returns null when
  * no file exists. Throws on a malformed policy: a policy that cannot be parsed
  * must stop the agent, never silently permit everything.
+ *
+ * A missing file is only optional when nobody asked for that file. `NAD_POLICY` is an operator
+ * saying "use this one", so a typo in it used to start the agent with no limits and no allowlist
+ * at all — the same silent everything-permitted state a malformed file already refuses to cause.
+ * An explicit argument keeps its older meaning: the caller named the path and owns the result.
  */
-export function loadPolicy(path = process.env.NAD_POLICY || DEFAULT_PATH) {
+export function loadPolicy(path) {
+  // The value is used exactly as configured, never trimmed: a filesystem path may legitimately
+  // carry leading or trailing whitespace, and trimming it opens a DIFFERENT file. On Linux
+  // `policy.json ` and `policy.json` are two paths, so a trim could quietly swap a policy with
+  // rules for one without any. Only a completely empty value means "no override"; a blank one is
+  // a path the operator typed, and if nothing is there the startup has to say so.
+  const override = process.env.NAD_POLICY ?? "";
+  const selected = path ?? (override !== "" ? override : DEFAULT_PATH);
+  const isOverride = path === undefined && override !== "";
   let raw;
   try {
-    raw = readFileSync(path, "utf8");
+    raw = readFileSync(selected, "utf8");
   } catch (err) {
-    if (err.code === "ENOENT") return null;
-    throw new Error(`policy file ${path} could not be read: ${err.message}`);
+    if (err.code === "ENOENT") {
+      // A dangling symlink lands here too: the open follows the link and misses its target.
+      if (isOverride) {
+        throw new Error(
+          `policy file ${selected} does not exist, and NAD_POLICY points at it: ` +
+            `create the file or unset NAD_POLICY to run without a policy`,
+        );
+      }
+      return null;
+    }
+    throw new Error(`policy file ${selected} could not be read: ${err.message}`);
   }
 
   let parsed;
   try {
     parsed = JSON.parse(raw);
   } catch (err) {
-    throw new Error(`policy file ${path} is not valid JSON: ${err.message}`);
+    throw new Error(`policy file ${selected} is not valid JSON: ${err.message}`);
   }
   if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
-    throw new Error(`policy file ${path} must contain a JSON object`);
+    throw new Error(`policy file ${selected} must contain a JSON object`);
   }
 
   const KNOWN_KEYS = new Set(["maxPerSend", "maxPerSession", "allowlist"]);
   const unknown = Object.keys(parsed).filter((k) => !KNOWN_KEYS.has(k));
   if (unknown.length) {
-    throw new Error(`policy file ${path} contains unknown key${unknown.length > 1 ? "s" : ""}: ${unknown.join(", ")}`);
+    throw new Error(`policy file ${selected} contains unknown key${unknown.length > 1 ? "s" : ""}: ${unknown.join(", ")}`);
   }
 
-  const policy = { path, maxPerSend: null, maxPerSession: null, allowlist: null };
+  const policy = { path: selected, maxPerSend: null, maxPerSession: null, allowlist: null };
 
   for (const key of ["maxPerSend", "maxPerSession"]) {
     if (parsed[key] == null) continue;

test/native-tools-cli-exit.test.mjs

@@ -16,7 +16,7 @@
 import { describe, test, beforeEach } from "node:test";
 import assert from "node:assert/strict";
 import { execFileSync } from "node:child_process";
-import { existsSync, mkdtempSync } from "node:fs";
+import { existsSync, mkdtempSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { fileURLToPath, pathToFileURL } from "node:url";
@@ -291,6 +291,8 @@ const SHIM_REGISTER_URL = pathToFileURL(SHIM_REGISTER).href;
 // derives locally from it (dry-run); the startup balance read is best-effort.
 const TEST_SEED = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about";
 const shimStateDir = mkdtempSync(join(tmpdir(), "nad-shim-"));
+const EMPTY_POLICY = join(shimStateDir, "empty-policy.json");
+writeFileSync(EMPTY_POLICY, "{}");
 
 function runRealCli(scenario) {
   assert.ok(existsSync(DIST_CLI), "dist/cli.mjs missing — run npm run build first (CI builds before testing)");
@@ -301,7 +303,10 @@ function runRealCli(scenario) {
     USE_NATIVE_TOOLS: "true",
     MONAD_NETWORK: "testnet",
     NAD_STATE_PATH: join(shimStateDir, `${scenario}.json`),
-    NAD_POLICY: join(shimStateDir, "no-policy.json"), // absent → no policy
+    // An empty policy object rather than an absent file: a nonempty NAD_POLICY now means the
+    // operator chose that file, and a missing one stops startup. `{}` keeps this harness's
+    // intent, which is a run with no rules rather than a run with no policy file.
+    NAD_POLICY: EMPTY_POLICY,
     NAD_MCP_CONFIG: join(shimStateDir, "no-mcp.json"), // absent → no servers
     NAD_SHIM_SCENARIO: scenario,
     NO_COLOR: "1",

test/policy-override.test.mjs

@@ -0,0 +1,143 @@
+/**
+ * Issue #108: an explicit NAD_POLICY that names a missing file used to start the agent with no
+ * limits and no allowlist at all.
+ *
+ * The distinction these tests pin is not "does the file exist" but "did anybody ask for it".
+ * A default that is absent stays optional, because that is the documented way to run without a
+ * policy; a nonempty NAD_POLICY is an operator choosing a file, and a typo in it must stop the
+ * agent rather than quietly remove every rule. Offline throughout: no wallet, no network.
+ */
+
+import { describe, it, afterEach } from "node:test";
+import assert from "node:assert/strict";
+import { execFileSync } from "node:child_process";
+import { mkdtempSync, symlinkSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { fileURLToPath } from "node:url";
+
+import { loadPolicy } from "../src/policy.mjs";
+
+const DIR = mkdtempSync(join(tmpdir(), "nad-policy-override-"));
+const MISSING = join(DIR, "not-here.json");
+const VALID = join(DIR, "valid.json");
+const BROKEN = join(DIR, "broken.json");
+writeFileSync(VALID, JSON.stringify({ maxPerSend: "0.5" }));
+writeFileSync(BROKEN, "{ not json");
+
+const previous = process.env.NAD_POLICY;
+const restore = () => {
+  if (previous === undefined) delete process.env.NAD_POLICY;
+  else process.env.NAD_POLICY = previous;
+};
+
+describe("loadPolicy — an explicit override that is not there", () => {
+  afterEach(restore);
+
+  it("refuses a missing file the operator named, and names it", () => {
+    process.env.NAD_POLICY = MISSING;
+    assert.throws(() => loadPolicy(), (err) => {
+      assert.match(err.message, /does not exist/);
+      assert.ok(err.message.includes(MISSING), `the path has to be in the message: ${err.message}`);
+      return true;
+    });
+  });
+
+  it("still returns null when nobody named a file", () => {
+    // The documented way to run without limits: no override, and the default is absent. This is
+    // the behaviour the refusal above must not swallow.
+    delete process.env.NAD_POLICY;
+    assert.equal(loadPolicy(join(DIR, "no-default-here", "policy.json")), null);
+  });
+
+  it("treats only a completely empty override as no override", () => {
+    // Driven through the environment on purpose. An earlier version of this test passed an
+    // explicit path argument, which takes the other branch entirely and proved nothing about
+    // the environment selection it claimed to cover.
+    process.env.NAD_POLICY = "";
+    assert.equal(loadPolicy(), null, "an empty override falls back to the optional default");
+  });
+
+  it("refuses a whitespace-only override, because it is a path that is not there", () => {
+    // A blank string is not "no override": it is a path the operator typed. #108 asks for a
+    // nonempty override naming a missing file to stop startup, and "   " is nonempty.
+    process.env.NAD_POLICY = "   ";
+    assert.throws(() => loadPolicy(), /does not exist/);
+  });
+
+  it("opens the configured path literally, whitespace and all", (t) => {
+    // The review case: two files one trailing space apart. Trimming the configured value opens
+    // the wrong one and silently drops every rule the operator meant to apply.
+    if (process.platform === "win32") {
+      t.skip("Win32 strips trailing spaces from path components");
+      return;
+    }
+    const plain = join(DIR, "pair.json");
+    const spaced = `${plain} `;
+    writeFileSync(plain, JSON.stringify({}));
+    writeFileSync(spaced, JSON.stringify({ maxPerSend: "0.5" }));
+
+    process.env.NAD_POLICY = spaced;
+    const policy = loadPolicy();
+
+    assert.equal(policy.path, spaced, "the file named is the file opened");
+    assert.ok(policy.maxPerSend > 0n, "the rules from the configured file have to survive");
+  });
+
+  it("still loads an override that exists", () => {
+    process.env.NAD_POLICY = VALID;
+    const policy = loadPolicy();
+    assert.equal(policy.path, VALID);
+    assert.ok(policy.maxPerSend > 0n);
+  });
+
+  it("still reports a malformed override as malformed, not as missing", () => {
+    process.env.NAD_POLICY = BROKEN;
+    assert.throws(() => loadPolicy(), /not valid JSON/);
+  });
+
+  it("treats a dangling symlink as missing", (t) => {
+    const link = join(DIR, "dangling.json");
+    try {
+      symlinkSync(join(DIR, "target-that-never-existed.json"), link);
+    } catch {
+      // Windows refuses symlinks without privileges; the branch is the same ENOENT either way.
+      t.skip("symlinks unavailable on this platform");
+      return;
+    }
+    process.env.NAD_POLICY = link;
+    assert.throws(() => loadPolicy(), /does not exist/);
+  });
+
+  it("keeps the older contract for an explicit argument", () => {
+    // Callers that pass a path have always owned the consequences, and the existing suite
+    // depends on it. Only the environment override changed meaning.
+    delete process.env.NAD_POLICY;
+    assert.equal(loadPolicy(MISSING), null);
+  });
+});
+
+describe("startup — the refusal happens before the wallet", () => {
+  it("exits nonzero and never initialises the wallet", () => {
+    const cli = fileURLToPath(new URL("../dist/cli.mjs", import.meta.url));
+    const env = { ...process.env, NAD_POLICY: MISSING, NO_COLOR: "1" };
+    delete env.NAD_CLI_NO_RUN;
+
+    let status = 0;
+    let output = "";
+    try {
+      output = execFileSync(process.execPath, [cli], {
+        env, input: "", encoding: "utf8", timeout: 120000, stdio: ["pipe", "pipe", "pipe"],
+      });
+    } catch (err) {
+      status = err.status;
+      output = `${err.stdout ?? ""}${err.stderr ?? ""}`;
+    }
+
+    assert.equal(status, 1, `expected exit 1, transcript:\n${output}`);
+    assert.match(output, /does not exist/);
+    // The ordering is the point: a policy the operator meant to apply must stop the run before
+    // anything can be signed, so the wallet line must never appear.
+    assert.doesNotMatch(output, /initializing wallet/i);
+  });
+});