Teach the mutation harness two blind spots it had, and close what they found

The harness only mutated block-form guards (`if (cond) {`), so every guard
written as a one-liner was invisible to it — which turned out to be most of
the load-bearing ones. src/highRisk.ts generated ZERO mutants and printed as
a clean sweep. Three changes to the tool:

- Mutate single-statement guards too (`if (cond) return ...;`). This alone
  surfaced 27 previously unreachable mutants, including the entire token
  validation chain: revoked, expired, exhausted, scope-not-granted.
- Report a target that generates no mutants as NOT MEASURED. Zero killed and
  zero survived prints exactly like a clean sweep, and it is the one result
  that must never be read as a pass.
- Add `kind`, `mnemonic` and `seed` to the security hints. A token's `kinds`
  list is half of what it may reach, and leaving the word out hid the shape
  check that stops a string `kinds` becoming a substring match.

Also adds `--kind` so a new mutation rule can be swept across every target
without re-judging the mutants already settled.

What the new mutants found, all of it previously unasserted:

- A token's `kinds` restriction was never tested. This file's own header
  claimed kinds and labels were both covered; only labels were. Removing the
  check let a token scoped to one kind read every other kind's secrets.
- `token-issue` would mint a token for a profile the agent is not holding.
  The seed being resident is the only proof the caller had authority over
  that vault.
- `token-issue`'s array shape checks for `scopes` and `kinds`. A string
  `kinds` turns tokenMaySee's exact match into String.includes.
- An empty high-risk factor. Refused in both implementations, tested in
  neither; it would wrap the partition key under nothing while
  `factorConfigured` still answered true.
- POST /api/v1/fingerprint/select accepting a profile that does not exist —
  the caller's string becomes ctx.fingerprint, which later requests join onto
  a path. AppDir repeats the check, so the guard's real value is answering
  404 instead of an unhandled 500.
- The API's requiresPassword flag is redundant for every route today, so
  disabling it changed nothing. Its purpose is the route that does not exist
  yet, whose author sets the flag and forgets the call — so the test now
  registers exactly that route and confirms the server layer stops it before
  the handler runs.
- sinks.ts drops SEEDPASS_-prefixed variables from a child's environment.
  Nothing on the allowlist starts with SEEDPASS_, so the backstop cannot fire
  against the real list. sinkEnv now takes the allowlist as a defaulted
  parameter purely so that future-edit case can be exercised.

All six targets now sweep to zero survivors.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN

js/packages/cli/src/sinks.ts

@@ -28,9 +28,16 @@ const PASSTHROUGH_ENV = ["PATH", "HOME", "LANG", "LC_ALL", "TERM", "TZ", "TMPDIR
  * Build the environment for a sink child: the allowlist above, never the
  * caller's full environment.
  */
-export function sinkEnv(extra: Record<string, string> = {}): Record<string, string> {
+export function sinkEnv(
+  extra: Record<string, string> = {},
+  // The allowlist is a parameter purely so the SEEDPASS_ backstop below can
+  // be tested. No entry in PASSTHROUGH_ENV starts with SEEDPASS_ today, so
+  // the check cannot fire against the real list -- which is what makes it a
+  // backstop, and also what made removing it invisible to the suite.
+  passthrough: readonly string[] = PASSTHROUGH_ENV,
+): Record<string, string> {
   const env: Record<string, string> = {};
-  for (const key of PASSTHROUGH_ENV) {
+  for (const key of passthrough) {
     // Backstop, not the control: nothing SEEDPASS-prefixed rides along even
     // if a future edit adds one to the allowlist. `extra` is exempt — the
     // exec sink's whole job is injecting SEEDPASS_SECRET.

js/packages/cli/test/agentSecurity.test.ts

@@ -594,6 +594,74 @@ async function daemonAt(now: () => number): Promise<{ d: AgentDaemon; sock: stri
   return { d, sock };
 }
 
+describe("token-issue validates before it mints", () => {
+  // Everything here is reachable over the wire by a process that already
+  // holds the owner socket, and none of it was asserted: the unlocked check
+  // and both shape checks could be removed with the suite still green.
+  let ownerCap: string;
+  beforeAll(async () => {
+    ownerCap = (await readFile(daemon.capabilityPath, "utf8")).trim();
+  });
+
+  it("refuses to mint a token for a profile it is not holding", async () => {
+    // A token is an authorization to read a specific vault. Minting one for a
+    // profile the agent never unlocked hands out authority nobody proved they
+    // had — the seed being resident IS the proof.
+    const other = generateFingerprint(mnemonics["zoo24"]!);
+    const r = await rawRequest({
+      op: "token-issue",
+      cap: ownerCap,
+      fingerprint: other,
+      name: "for-a-vault-i-do-not-hold",
+      scopes: ["read"],
+      ttl: 600,
+      uses: 1,
+    });
+    expect(r["ok"]).toBe(false);
+    expect(String(r["error"])).toContain("not unlocked");
+    expect(r["token"]).toBeUndefined();
+  });
+
+  it("refuses a bare string where an array is required", async () => {
+    // `"read"` and `["read"]` are easy to confuse when hand-writing the
+    // protocol, and the difference is not cosmetic: a string `kinds` turns
+    // the exact kind match in tokenMaySee into String.includes, so a token
+    // declared for one kind starts matching any kind whose name is a
+    // substring of it. Refuse the shape rather than discovering it later.
+    const base = {
+      op: "token-issue",
+      cap: ownerCap,
+      fingerprint: FINGERPRINT,
+      name: "wrong-shape",
+      ttl: 600,
+      uses: 1,
+    };
+    const badScopes = await rawRequest({ ...base, scopes: "read" });
+    expect(badScopes["ok"]).toBe(false);
+    expect(String(badScopes["error"])).toContain("scopes must be an array");
+    expect(badScopes["token"]).toBeUndefined();
+
+    const badKinds = await rawRequest({
+      ...base,
+      scopes: ["read"],
+      kinds: "password",
+    });
+    expect(badKinds["ok"]).toBe(false);
+    expect(String(badKinds["error"])).toContain("kinds must be an array");
+    expect(badKinds["token"]).toBeUndefined();
+
+    // The same request with both shapes correct does mint, so these tests
+    // cannot pass by token-issue being broken outright.
+    const good = await rawRequest({
+      ...base,
+      scopes: ["read"],
+      kinds: ["password"],
+    });
+    expect(good["ok"]).toBe(true);
+    expect(String(good["token"]).length).toBeGreaterThan(0);
+  });
+});
+
 describe("put refuses a half-supplied identity", () => {
   // `put` is the only door a seed comes through, and everything after it
   // trusts that both halves arrived. Mutation testing turned the `||` in its

js/packages/cli/test/api.test.ts

@@ -452,6 +452,60 @@ describe("profiles and vault state", () => {
     expect(existsSync(app.profileDir(OTHER))).toBe(false);
   });
 
+  it("refuses to select a profile that does not exist", async () => {
+    // Without this the caller's string becomes ctx.fingerprint, which every
+    // later request joins onto a path — so the refusal is what keeps
+    // "../../etc" out of the profile directory. Asserted on the shared
+    // server precisely because a refusal must leave its state untouched.
+    const before = ctx.fingerprint;
+    for (const bogus of [generateFingerprint(mnemonics["zoo24"]!), "../../etc"]) {
+      const res = await call("POST", "/api/v1/fingerprint/select", {
+        body: { fingerprint: bogus },
+      });
+      // 404 specifically. AppDir.switchProfile repeats both checks, so
+      // dropping this one still refuses — as a raw 500 from an unhandled
+      // error, telling a caller the server broke when it simply named a
+      // profile that is not there.
+      expect(res.status).toBe(404);
+      expect(ctx.fingerprint).toBe(before);
+    }
+  });
+
+  it("selects a real profile, dropping the seed on the way", async () => {
+    // The positive half, on its own context so the shared server keeps
+    // serving the profile the rest of this file expects.
+    const second = generateFingerprint(mnemonics["zoo24"]!);
+    await app.mutateFingerprints((data) => {
+      if (!data.fingerprints.includes(second)) data.fingerprints.push(second);
+    });
+    await mkdir(app.profileDir(second), { recursive: true });
+
+    const srv = new ApiServer({ host: "127.0.0.1", port: 0, token: TOKEN });
+    const ctx2 = buildContext({ app, fingerprint: FINGERPRINT, mnemonic: MNEMONIC });
+    registerRoutes(srv, ctx2);
+    const bound = await srv.listen();
+    try {
+      const res = await fetch(
+        `http://${bound.host}:${bound.port}/api/v1/fingerprint/select`,
+        {
+          method: "POST",
+          headers: {
+            authorization: `Bearer ${TOKEN}`,
+            "content-type": "application/json",
+          },
+          body: JSON.stringify({ fingerprint: second }),
+        },
+      );
+      expect(res.status).toBe(200);
+      expect(ctx2.fingerprint).toBe(second);
+      // The held seed belongs to the profile being left; carrying it across
+      // would read one vault while the caller believed they selected another.
+      expect(ctx2.mnemonic).toBeNull();
+    } finally {
+      await srv.close();
+    }
+  });
+
   it("reports lock state that follows the actual seed", async () => {
     expect((await call("GET", "/api/v1/vault/status")).json.locked).toBe(false);
 
@@ -935,6 +989,48 @@ describe("structural invariants of the route table", () => {
     expect(mismatched).toEqual([]);
   });
 
+  it("refuses a flagged route whose handler forgot to check", async () => {
+    // The behavioural half of the pairing test above. The flag is redundant
+    // for every route that exists today, which is why disabling the server's
+    // check changed nothing — but its entire purpose is the route that does
+    // NOT exist yet, written by someone who set the flag and skipped the
+    // call. So register exactly that route and confirm the server layer
+    // stops it before the handler ever runs.
+    const srv = new ApiServer({ host: "127.0.0.1", port: 0, token: TOKEN });
+    let handlerRan = false;
+    srv.route(
+      "GET",
+      "/api/v1/test-forgot-to-check",
+      async () => {
+        handlerRan = true;
+        return { json: { secret: "leaked-plaintext" } };
+      },
+      { requiresPassword: true },
+    );
+    const bound = await srv.listen();
+    try {
+      const url = `http://${bound.host}:${bound.port}/api/v1/test-forgot-to-check`;
+      const res = await fetch(url, { headers: { authorization: `Bearer ${TOKEN}` } });
+      expect(res.status).toBe(401);
+      expect(await res.text()).not.toContain("leaked-plaintext");
+      // Not merely refused after the fact — never invoked.
+      expect(handlerRan).toBe(false);
+
+      // With the password it goes through, so the test cannot pass by the
+      // route being broken.
+      const ok = await fetch(url, {
+        headers: {
+          authorization: `Bearer ${TOKEN}`,
+          "x-seedpass-password": PASSWORD,
+        },
+      });
+      expect(ok.status).toBe(200);
+      expect(handlerRan).toBe(true);
+    } finally {
+      await srv.close();
+    }
+  });
+
   it("password-gates every route that can return plaintext", async () => {
     // The failure this guards against is a NEW secret-returning route with
     // neither the flag nor the call — which the pairing test above cannot

js/packages/cli/test/highRisk.test.ts

@@ -131,6 +131,21 @@ describe("the factor", () => {
     expect(String((second.error as Error).message)).toContain("already configured");
   });
 
+  it("cannot be empty, and leaves nothing behind when refused", async () => {
+    // An empty factor would wrap the partition key under nothing: the
+    // envelope unwraps for anyone who supplies "", while `configured` still
+    // reports true. So the vault would show a second factor that is not one.
+    // Python raises on the same falsy check, so this is parity as well.
+    const fresh = await mkdtemp(join(tmpdir(), "seedpass-empty-factor-"));
+    await expect(setFactor(fresh, "")).rejects.toThrow(/cannot be empty/);
+    // Refused BEFORE the envelope is written — a half-configured factor would
+    // be worse than none, because factorConfigured() would start saying yes.
+    expect(factorConfigured(fresh)).toBe(false);
+    // And a real factor still works in the same directory, so the guard is
+    // rejecting the empty string rather than the directory.
+    await expect(setFactor(fresh, "a-real-factor")).resolves.toMatch(/^[0-9a-f]{64}$/);
+  });
+
   it("is never read from the command line", async () => {
     const r = await run({ ...asOwner, SEEDPASS_HIGH_RISK_FACTOR: undefined },
       "agent", "high-risk", "unlock");

js/packages/cli/test/sinks.test.ts

@@ -167,3 +167,43 @@ describe("command spec parsing keeps every argument", () => {
     expect(() => parseCommandSpec([""])).toThrow(/empty command/);
   });
 });
+
+import { sinkEnv } from "../src/sinks.js";
+
+describe("the environment a sink child receives", () => {
+  it("passes only the allowlist through, and never the caller's own env", async () => {
+    process.env["SEEDPASS_TEST_LEAK"] = "must-not-appear";
+    try {
+      const env = sinkEnv();
+      expect(env["SEEDPASS_TEST_LEAK"]).toBeUndefined();
+      // PATH is on the allowlist and the child genuinely needs it, so its
+      // presence is what proves the allowlist is being applied rather than
+      // the whole environment simply being empty.
+      expect(env["PATH"]).toBe(process.env["PATH"]);
+    } finally {
+      delete process.env["SEEDPASS_TEST_LEAK"];
+    }
+  });
+
+  it("drops a SEEDPASS_ variable even if the allowlist grows one", async () => {
+    // The backstop. It cannot fire against today's allowlist — which is why
+    // deleting it changed nothing anywhere — so the allowlist is injected
+    // here to stand in for the future edit that adds one by mistake.
+    process.env["SEEDPASS_SNEAKY"] = "must-not-appear";
+    try {
+      const env = sinkEnv({}, ["PATH", "SEEDPASS_SNEAKY"]);
+      expect(env["SEEDPASS_SNEAKY"]).toBeUndefined();
+      expect(env["PATH"]).toBeDefined();
+    } finally {
+      delete process.env["SEEDPASS_SNEAKY"];
+    }
+  });
+
+  it("still injects the exec sink's own secret variable", async () => {
+    // `extra` is deliberately exempt from the SEEDPASS_ rule: handing the
+    // secret to the child through the environment is the exec sink's entire
+    // job. A backstop that swallowed it would break the feature.
+    const env = sinkEnv({ SEEDPASS_SECRET: "the-secret" });
+    expect(env["SEEDPASS_SECRET"]).toBe("the-secret");
+  });
+});

js/packages/cli/test/tokens.test.ts

@@ -111,6 +111,52 @@ describe("token issuance", () => {
  * issued. These tests pin the semantics so the surprise stays documented
  * rather than becoming an accidental behavior change later.
  */
+describe("the kind restriction on a token", () => {
+  /**
+   * `--kind` is half of a token's reach; `--label-regex` is the other half.
+   * The header of this file claimed both were covered and only the label side
+   * was — removing the kind check entirely left every test here green, which
+   * means a token scoped to one kind could read every other kind's secrets
+   * and nothing would have said so.
+   */
+  it("hides other kinds from listings and refuses their secrets", async () => {
+    await run(asOwner, "entry", "add", "key-value", "kv-only", "k", "kv-secret");
+    await run(asOwner, "entry", "add", "password", "pw-only");
+
+    const issued = JSON.parse(
+      (await run(
+        {},
+        "agent", "token-issue",
+        "--name", "kv-scoped",
+        "--scope", "read",
+        "--kind", "key_value",
+        "--label-regex", "only",
+        "--ttl", "600",
+      )).stdout,
+    );
+
+    // The label pattern matches BOTH entries, so anything visible here is the
+    // kind check's doing and not the label's.
+    const rows = JSON.parse(
+      (await run(asTokenHolder(issued.token), "entry", "list")).stdout,
+    ) as { label: string }[];
+    expect(rows.map((x) => x.label)).toEqual(["kv-only"]);
+
+    // And listing is not the only door: the secret itself must be refused,
+    // not merely omitted from the index the token can see.
+    const pwRow = JSON.parse(
+      (await run(asOwner, "entry", "list")).stdout,
+    ) as { id: string | number; label: string }[];
+    const pwId = String(pwRow.find((r) => r.label === "pw-only")!.id);
+    const denied = await run(
+      asTokenHolder(issued.token),
+      "entry", "get", pwId, "--secret",
+    );
+    expect(denied.error).toBeDefined();
+    expect(denied.stdout).not.toContain("pw-only");
+  });
+});
+
 describe("label_regex matches anywhere, matching Python's re.search", () => {
   it("an unanchored pattern reaches labels that merely contain it", async () => {
     await run(asOwner, "entry", "add", "key-value", "not-prod-db", "k", "adjacent-secret");

scripts/mutation_test.py

@@ -114,7 +114,12 @@ SECURITY_HINTS = re.compile(
     r"\b("
     r"auth|token|cap|capab|scope|permit|allow|deny|refus|valid|verif|"
     r"expire|expires|ttl|limit|rate|uses|remaining|owner|secret|password|"
-    r"factor|unlock|lock|held|fingerprint|hash|equal|match|forbidden"
+    r"factor|unlock|lock|held|fingerprint|hash|equal|match|forbidden|"
+    # `kind` is an authorization dimension in its own right -- a token's
+    # `kinds` list is half of what it may reach -- and leaving it out hid the
+    # shape check that stops a string `kinds` becoming a substring match.
+    # `mnemonic`/`seed` guard the material everything else protects.
+    r"kind|mnemonic|seed"
     r")",
     re.IGNORECASE,
 )
@@ -173,6 +178,7 @@ def generate(source: str) -> list[Mutant]:
         # Force a guard to always allow. This is the single most informative
         # mutation for security code: if no test notices a check being removed
         # entirely, that check is unverified.
+        # Block form: `if (cond) {`
         guard = re.match(r"^(\s*)if \((.+)\) \{\s*$", line)
         if guard and SECURITY_HINTS.search(guard.group(2)):
             mutants.append(
@@ -183,6 +189,28 @@ def generate(source: str) -> list[Mutant]:
                     "guard disabled (always allow)",
                 )
             )
+        # Single-statement form: `if (cond) throw ...;` / `if (cond) return ...;`
+        # Missed for a long time, and it is the shape a short refusal takes --
+        # src/highRisk.ts generated NO mutants at all because its only guard
+        # is written this way.
+        # Greedy, so the condition runs to the LAST `) ` on the line -- a
+        # non-greedy match splits `if (a(b)) return x;` in the wrong place and
+        # produces garbage that only shows up as "invalid". Lines ending in
+        # `{` are the block form, already handled above.
+        inline = re.match(r"^(\s*)if \((.+)\) (\S.*)$", line)
+        if (
+            inline
+            and not line.rstrip().endswith("{")
+            and SECURITY_HINTS.search(inline.group(2))
+        ):
+            mutants.append(
+                Mutant(
+                    i,
+                    line,
+                    f"{inline.group(1)}if (false) {inline.group(3)}",
+                    "inline guard disabled (always allow)",
+                )
+            )
     return mutants
 
 
@@ -217,11 +245,24 @@ def main() -> int:
         ),
     )
     parser.add_argument("--timeout", type=int, default=420)
+    parser.add_argument(
+        "--kind",
+        default="",
+        help=(
+            "only run mutants whose description contains this substring, e.g. "
+            "--kind inline. Lets a newly added mutation rule be swept across "
+            "every target without re-running the ones already judged."
+        ),
+    )
     args = parser.parse_args()
 
     targets = sorted(TARGETS) if args.target == "all" else [args.target]
     overall_survivors: list[tuple[str, Mutant]] = []
     overall_counts = {"killed": 0, "survived": 0, "invalid": 0}
+    # A target the generator cannot reach scores 0 killed / 0 survived, which
+    # prints identically to a clean sweep. That is the one result that must
+    # never be mistaken for a pass: it means the file was NOT MEASURED.
+    unmeasured: list[str] = []
 
     acquire_lock()
     print(
@@ -234,11 +275,20 @@ def main() -> int:
         path = CLI / rel
         original = path.read_text()
         all_mutants = generate(original)
+        if args.kind:
+            all_mutants = [m for m in all_mutants if args.kind in m.description]
         mutants = all_mutants[args.offset :]
         if args.limit:
             mutants = mutants[: args.limit]
         span = f"{args.offset + 1}-{args.offset + len(mutants)} of {len(all_mutants)}"
         print(f"\n=== {name} ({rel}): mutants {span} ===")
+        if not all_mutants:
+            unmeasured.append(f"{name} ({rel})")
+            print(
+                "  NOT MEASURED: this file produced no mutants. Its security\n"
+                "  logic is not in comparisons the generator can reach (or the\n"
+                "  generator has a blind spot). Do not read this as a pass."
+            )
 
         # Restore the file whatever happens, including Ctrl-C. Leaving a
         # deliberate security hole in the tree would be a spectacular own goal.
@@ -295,6 +345,14 @@ def main() -> int:
     if viable:
         print(f"mutation score: {overall_counts['killed'] / viable:.0%} of viable mutants caught")
 
+    if unmeasured:
+        print(
+            "\nNOT MEASURED — no mutants generated, so these files were not\n"
+            "tested at all by this run:\n"
+        )
+        for where in unmeasured:
+            print(f"  {where}")
+
     if overall_survivors:
         print(f"\nSURVIVORS — each is either a gap or an equivalent mutant, and needs judging:\n")
         for where, mutant in overall_survivors: