fix(agent): close two ways a local caller could subvert the session daemon

Both were found by the 2026-08-17 self-review and confirmed by speaking to
the socket directly. The daemon is the enforcement point for the whole token
model and treats every caller as untrusted, so neither fix leans on the CLI's
own validation.

1. A `put` reported as FAILED could leave the seed resident and unexpiring.
   State was committed to `held` before the audit append was awaited. A
   non-numeric ttl made `expiresAt` NaN; the audit write then threw on the
   non-finite value, so the caller saw ok:false while the seed stayed in
   memory forever (`NaN <= now` is always false) with no record it was ever
   unlocked. Now ttl is validated as a positive integer at the boundary, and
   an audit failure rolls the seed back out — ok:false means genuinely locked.

2. A `use`-scoped token could crash the whole agent. `stdinSink` wrote to the
   child's stdin with no error listener, so a command that exits before
   draining raised an unhandled EPIPE; the agent runs sinks in-process and
   installs no uncaughtException handler, so it exited and dropped every held
   seed. The least-privileged principal could take down the enforcement point
   by naming a command that exits early. stdin errors are now swallowed and
   the child's exit code carries the result, as feedClipboardTool already did.

Also hardens token-issue: scopes/kinds are shape-checked before the array
methods run against them (a bare-string kinds turned an exact match into a
substring match), and ttl/uses go through the same positive-integer boundary
so a token can no longer be minted with a NaN, never-expiring lifetime.

Three regression tests speak the wire protocol directly. Verified as negative
controls: with the fixes stashed, the stdin test reproduces the EPIPE uncaught
exception at sinks.ts:143 and two of the three fail; with them restored all
131 CLI tests pass.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Tkj4rzKBA3uhfgk7kqjsb8

TODO.md

@@ -143,8 +143,8 @@ everything still open, in the order it should be tackled.
 Found by reading and probing, not by the suite — all 128 CLI tests passed
 before and after each was confirmed. Ordered by severity.
 
-- [ ] **The agent can be left holding a parent seed forever, after reporting
-      failure.** `AgentDaemon.handle`'s `put` writes to `this.held` *before*
+- [x] **(fixed 2026-08-17)** **The agent can be left holding a parent seed
+      forever, after reporting failure.** `AgentDaemon.handle`'s `put` writes to `this.held` *before*
       awaiting the audit append. A non-numeric `ttl` makes `expiresAt` NaN; the
       audit write then throws on the non-finite number, so the caller is told
       the unlock failed — but the seed is resident, `expire()`'s
@@ -157,8 +157,8 @@ before and after each was confirmed. Ordered by severity.
       which is exactly why the daemon must not rely on it — `agent.ts:1-22`
       states the CLI is untrusted, and the browser extension will be the next
       thing speaking this protocol.
-- [ ] **A `use`-scoped token holder can kill the agent and drop every held
-      seed.** `stdinSink` writes to `child.stdin` with no `error` listener, so a
+- [x] **(fixed 2026-08-17)** **A `use`-scoped token holder can kill the agent
+      and drop every held seed.** `stdinSink` writes to `child.stdin` with no `error` listener, so a
       command that exits before draining stdin raises an unhandled EPIPE.
       Confirmed as an uncaught exception inside the agent process via a
       `use-sink` request naming `/bin/true`; `main.ts` installs no
@@ -201,7 +201,7 @@ before and after each was confirmed. Ordered by severity.
       alter one can remove the other. The doc comment claims the head "pins the
       expected length" against exactly this attack. Either require the head once
       the log exists, or keep it somewhere the log's writer cannot reach.
-- [ ] **Unvalidated token constraint shapes.** `token-issue` casts `kinds` and
+- [x] **(fixed 2026-08-17)** **Unvalidated token constraint shapes.** `token-issue` casts `kinds` and
       `scopes` without checking they are arrays. `kinds: "totp"` makes
       `Array.includes` become `String.includes`, turning an exact kind match into
       a substring match — inert today only because no SeedPass kind is a

js/packages/cli/src/agent.ts

@@ -52,6 +52,22 @@ export const TIMESTAMP_SKEW_SECONDS = 120;
 export const MAX_LINE_BYTES = 512 * 1024;
 export const MAX_CONNECTIONS = 64;
 
+/**
+ * A positive integer taken from a wire message, or null if the value is not
+ * one. The CLI validates these too, but the daemon cannot rely on that: it
+ * treats every caller as untrusted (see the module header), and the next
+ * thing to speak this protocol will be a browser extension.
+ *
+ * Coercions that must be rejected here rather than stored: a non-numeric ttl
+ * makes `expiresAt` NaN, and `NaN <= now` is always false — the seed would
+ * then never expire. The same NaN in a token's `expires_at` makes a token
+ * that never expires.
+ */
+function positiveIntField(value: unknown, fallback: number): number | null {
+  const n = value === undefined || value === null ? fallback : Number(value);
+  return Number.isInteger(n) && n >= 1 ? n : null;
+}
+
 export type TokenScope = "read" | "use" | "reveal";
 
 export interface TokenRecord {
@@ -460,20 +476,35 @@ export class AgentDaemon {
         // profile must currently be unlocked in this agent.
         const fingerprint = String(msg["fingerprint"] ?? "");
         if (!this.held.has(fingerprint)) return { ok: false, error: "profile not unlocked" };
-        const scopes = (msg["scopes"] as TokenScope[] | undefined) ?? ["read"];
-        const bad = scopes.filter((s) => !["read", "use", "reveal"].includes(s));
+        const scopes = msg["scopes"] ?? ["read"];
+        // Shape-check before .filter/.includes run against them: a bare string
+        // scope would throw a raw TypeError back to the caller, and a string
+        // `kinds` turns the exact kind match in tokenMaySee into a substring
+        // match (String.includes, not Array.includes).
+        if (!Array.isArray(scopes)) return { ok: false, error: "scopes must be an array" };
+        const bad = (scopes as unknown[]).filter(
+          (s) => !["read", "use", "reveal"].includes(s as string),
+        );
         if (bad.length) return { ok: false, error: `unknown scopes: ${bad.join(",")}` };
+        const kinds = msg["kinds"];
+        if (kinds !== undefined && kinds !== null && !Array.isArray(kinds)) {
+          return { ok: false, error: "kinds must be an array" };
+        }
+        const ttl = positiveIntField(msg["ttl"], 300);
+        if (ttl === null) return { ok: false, error: "ttl must be a positive integer" };
+        const uses = positiveIntField(msg["uses"], 1);
+        if (uses === null) return { ok: false, error: "uses must be a positive integer" };
         const secret = randomBytes(24).toString("base64url");
         const record: TokenRecord = {
           id: `tok-${randomBytes(6).toString("hex")}`,
           name: String(msg["name"] ?? "agent"),
           fingerprint,
           secret_hash: sha256Hex(utf8(secret)),
-          scopes,
-          kinds: (msg["kinds"] as string[] | undefined) ?? null,
+          scopes: scopes as TokenScope[],
+          kinds: (kinds as string[] | undefined) ?? null,
           label_regex: String(msg["label_regex"] ?? ".*"),
-          expires_at: Math.floor(Date.now() / 1000 + Number(msg["ttl"] ?? 300)),
-          uses_remaining: Number(msg["uses"] ?? 1),
+          expires_at: Math.floor(Date.now() / 1000 + ttl),
+          uses_remaining: uses,
           revoked: false,
           ...(Array.isArray(msg["exec_allowlist"]) && (msg["exec_allowlist"] as string[]).length
             ? { exec_allowlist: msg["exec_allowlist"] as string[] }
@@ -605,10 +636,21 @@ export class AgentDaemon {
         const fingerprint = String(msg["fingerprint"] ?? "");
         const mnemonic = String(msg["mnemonic"] ?? "");
         if (!fingerprint || !mnemonic) return { ok: false, error: "missing fields" };
-        const ttl = Number(msg["ttl"] ?? this.defaultTtl);
+        const ttl = positiveIntField(msg["ttl"], this.defaultTtl);
+        if (ttl === null) return { ok: false, error: "ttl must be a positive integer" };
         const expiresAt = Math.floor(Date.now() / 1000 + ttl);
+        // The audit key is derived from the held seed, so the seed must go in
+        // before the record is written — but an audit failure must then undo
+        // it. Otherwise a `put` that reports ok:false can leave the seed
+        // resident and unexpiring, with nothing on record that it was ever
+        // unlocked. `ok:false` has to mean the vault is genuinely still locked.
         this.held.set(fingerprint, { mnemonic, expiresAt });
-        await this.auditLog(fingerprint, "vault_unlocked", { ttl });
+        try {
+          await this.auditLog(fingerprint, "vault_unlocked", { ttl });
+        } catch (e) {
+          this.forget(fingerprint);
+          throw e;
+        }
         return { ok: true, expires_at: expiresAt };
       }
       case "owner-mnemonic": {

js/packages/cli/src/sinks.ts

@@ -139,10 +139,21 @@ export async function stdinSink(
       stdio: ["pipe", "inherit", "inherit"],
       env: sinkEnv(),
     });
-    child.on("error", reject);
-    child.stdin.write(secret);
-    child.stdin.end();
+    let spawnFailed = false;
+    child.on("error", (e) => {
+      spawnFailed = true;
+      reject(e);
+    });
+    // A command that exits before reading stdin makes this write fail with
+    // EPIPE. With no listener that becomes an *unhandled* error event, which
+    // crashes the whole process — and the session agent runs sinks in-process,
+    // so a use-scoped token holder could take the agent down (dropping every
+    // held seed) just by naming a command that exits early. Swallow the stdin
+    // error: the child's exit code, delivered on `close`, is the real result.
+    child.stdin.on("error", () => {});
+    child.stdin.end(secret);
     child.on("close", (code) => {
+      if (spawnFailed) return;
       resolve({ sink: "stdin", detail: `piped to ${command}`, exitCode: code ?? -1 });
     });
   });

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

@@ -385,3 +385,79 @@ describe("locking clears derived state", () => {
     expect(String(after["error"])).toContain("unknown token");
   });
 });
+
+describe("malformed daemon input cannot corrupt held state", () => {
+  // Reproduces the 2026-08-17 self-review findings, spoken to the socket
+  // directly. The CLI would reject these, which is exactly why the daemon
+  // must too: it treats every caller as untrusted.
+  const SCRATCH_FP = "BBBBBBBBBBBBBBBB";
+  let cap: string;
+  beforeAll(async () => {
+    cap = (await readFile(daemon.capabilityPath, "utf8")).trim();
+  });
+
+  it("a put with a non-numeric ttl is refused, and holds nothing", async () => {
+    const put = await rawRequest({
+      op: "put", cap, fingerprint: SCRATCH_FP, mnemonic: MNEMONIC, ttl: "abc",
+    });
+    expect(put["ok"]).toBe(false);
+    expect(String(put["error"])).toContain("ttl");
+    // A reported failure must be a real one: NaN once left the seed resident
+    // and unexpiring with no audit record. The seed must not be held at all.
+    const owner = await rawRequest({ op: "owner-mnemonic", cap, fingerprint: SCRATCH_FP });
+    expect(owner["ok"]).toBe(false);
+    expect(JSON.stringify(owner)).not.toContain("abandon");
+  });
+
+  it("token-issue refuses a bare-string scope and a non-numeric ttl without crashing", async () => {
+    const strScope = await rawRequest({
+      op: "token-issue", cap, fingerprint: FINGERPRINT, scopes: "reveal", ttl: 600, uses: 1,
+    });
+    expect(strScope["ok"]).toBe(false);
+    expect(strScope["token"]).toBeUndefined();
+
+    const badTtl = await rawRequest({
+      op: "token-issue", cap, fingerprint: FINGERPRINT, scopes: ["read"], ttl: "soon", uses: 1,
+    });
+    expect(badTtl["ok"]).toBe(false);
+    expect(String(badTtl["error"])).toContain("ttl");
+
+    // The daemon is still answering — a raw TypeError must not have escaped.
+    expect((await rawRequest({ op: "ping" }))["pong"]).toBe(true);
+  });
+});
+
+describe("a use-scoped token cannot crash the agent", () => {
+  it("a stdin sink to a command that exits before reading does not kill the daemon", async () => {
+    // A large secret so the payload cannot fit the pipe buffer: the child
+    // exits before draining it, which raised an unhandled EPIPE and took the
+    // whole agent process down.
+    const bigValue = "y".repeat(200_000);
+    const added = JSON.parse(
+      (await run(
+        { SEEDPASS_MNEMONIC: MNEMONIC },
+        "entry", "add", "key-value", "bulk", "k", bigValue,
+      )).stdout,
+    );
+    const id = String(added.id);
+
+    // A fresh token, not the shared one: earlier tests may have spent its uses.
+    const useToken = JSON.parse(
+      (await run(
+        {}, "agent", "token-issue", "--scope", "read", "use", "--uses", "5", "--ttl", "600",
+      )).stdout,
+    ).token;
+
+    const r = await rawRequest({
+      op: "use-sink", fingerprint: FINGERPRINT, id, token: useToken,
+      sink: "stdin", command: ["/bin/true"],
+    });
+    expect(r["ok"]).toBe(true);
+
+    // The load-bearing assertions: the agent is still alive and still holding
+    // the seed after a delivery that used to crash it.
+    expect((await rawRequest({ op: "ping" }))["pong"]).toBe(true);
+    const status = JSON.parse((await run({}, "agent", "status")).stdout);
+    expect(status.map((p: { fingerprint: string }) => p.fingerprint)).toContain(FINGERPRINT);
+  });
+});