Drop model-authored agentLabel from the approval log (CL-5666 review fix)

A sub-agent's task-dispatch description is free text the model composes
itself, only ever .trim()ed, never constrained to a closed set. It had been
flowing verbatim into approvals.jsonl as agentLabel, which is on-by-default
and append-forever -- unlike rule (a closed set) and segments (a count),
nothing stops it from carrying a path, a token, or content the model just
read. Dropped the field entirely: volume, rule mix, mode split, and timing
all still work without it, and no concrete question needed a per-agent
breakdown badly enough to justify the risk.

Added a hard size cap on the serialized record (512 bytes) as defense in
depth -- every real field is a fixed enum, a count, or a timestamp, so a
well-formed line should never come close to it; an oversized line is
dropped rather than truncated, so no partial secret survives. Added the
missing test: a fake secret embedded in a sub-agent's dispatch description,
driven through the gate, asserted absent from the serialized record; and a
test that a rule field stuffed with 10k chars (simulating a future
regression) is dropped by the cap rather than logged.

Also corrected the doc comment's claim about rule provenance: alongside the
existing auto-shell-policy.ts/classify.ts rule names, this file defines its
own small set of fixed literals (auto-allowed-tool, non-interactive,
mega-chain) for decisions those modules don't otherwise name -- still a
closed set, just not literally reused from elsewhere.

Author: TheGreatAxios

docs/ARCHITECTURE.md

@@ -374,7 +374,7 @@ tool call
 - **queue** — Headless settle registry (`src/permission/queue.ts`). Surfaces enqueue outstanding requests; `wirePermissionGrantReconciliation` listens for `permission.grant` and drains every queued request the new grant covers, without a second prompt. Teardown calls `drain()` so no awaited resolve is left hanging.
 - **types** — `Approval`, `ApprovalScope`, `PermissionRequest`, `ApprovalOutcome`.
 
-**Approval log** (`src/permission/approval-log.ts`, CL-5666): every consequential decision the gate makes — auto-mode allow/deny or an interactive prompt's allow-once/allow-with-scope/deny/timeout/abort — is appended as one JSONL record to `approvals.jsonl` in the session dir, carrying the classifier/auto-shell rule name that fired (reusing the existing rule taxonomy, not a new one), whether the decision was `auto` or `interactive`, a shell chain's segment count, and queued/displayed/settled timestamps. `displayedAt` is set by `PermissionRequest.markDisplayed`, called from `gate-wire.ts`'s `open()` the moment a request actually reaches the overlay host — distinct from when it was raised, so the gap it exposes is the CL-5664 signal (a queued gate arming its timeout before the operator could see it). No command text, file content, path, or credential is ever recorded — only tool name, rule, mode, segment count, and timing. Writes are fire-and-forget and swallow their own errors; the log defaults to a no-op so nothing depends on it being wired. `scripts/approval-forensics.ts` aggregates across local sessions the same way `intervention-forensics.ts` does for stop/nudge events: per-tool counts by outcome and mode, duration/display-delay percentiles, mega-chain counts, and a duplicate-rate proxy (sessions that hit the same rule more than once).
+**Approval log** (`src/permission/approval-log.ts`, CL-5666): every consequential decision the gate makes — auto-mode allow/deny or an interactive prompt's allow-once/allow-with-scope/deny/timeout/abort — is appended as one JSONL record to `approvals.jsonl` in the session dir, carrying the classifier/auto-shell rule name that fired (the existing `auto-shell-policy.ts`/`classify.ts` rule names, plus a small closed set of additional fixed literals the log itself defines for decisions those modules don't otherwise name — `auto-allowed-tool`, `non-interactive`, `mega-chain` — never model- or user-authored text), whether the decision was `auto` or `interactive`, a shell chain's segment count, and queued/displayed/settled timestamps. `displayedAt` is set by `PermissionRequest.markDisplayed`, called from `gate-wire.ts`'s `open()` the moment a request actually reaches the overlay host — distinct from when it was raised, so the gap it exposes is the CL-5664 signal (a queued gate arming its timeout before the operator could see it). No command text, file content, path, credential, or other free text is ever recorded — only tool name, rule, mode, segment count, and timing; a sub-agent's free-text dispatch label is deliberately left out, even though it would enable a per-agent breakdown, because nothing constrains what a model puts in it. A hard size cap on the serialized line is defense in depth against a future field reintroducing free text. Writes are fire-and-forget and swallow their own errors; the log defaults to a no-op so nothing depends on it being wired. `scripts/approval-forensics.ts` aggregates across local sessions the same way `intervention-forensics.ts` does for stop/nudge events: per-tool counts by outcome and mode, duration/display-delay percentiles, mega-chain counts, and a duplicate-rate proxy (sessions that hit the same rule more than once).
 
 **Tool wall-clock budget vs. permission prompts.** Each tool `run()` is wrapped by an outer execution watchdog (`src/tui/tool-execution-watchdog.ts`). The watchdog arms only when Settings set `tools.timeoutMs` / `tools.maxTimeoutMs`, or when `run_shell` passes a positive timeout (requested plus slack, so this layer cannot beat shell-guard). The `task` tool is always exempt, regardless of Settings — a sub-agent run is bounded by its own limits (maxTurns, no-progress, thrash, opt-in deadlineMs), so the generic per-tool budget never aborts a healthy long-running worker; parent cancel, maxTurns, and eval `--agent-timeout-ms` still bound the run. By default (`tools.waitForApproval`, Settings → Tools, **On**), an armed budget freezes while the operator is deciding on a permission prompt, so a late approve still runs the tool and the agent waits for the decision instead of timing out under the modal. When **Off**, the budget keeps ticking during the prompt; if it expires first the tool is skipped and the permission modal is dismissed via the budget AbortSignal (auto-deny with a timeout message). The TUI permission queue (`src/tui/gate-wire.ts`, backed by `src/permission/queue.ts`) attaches that signal so ghost prompts cannot outlive an already-aborted tool.
 

src/permission/approval-log.test.ts

@@ -8,7 +8,12 @@ import { createPermissionGate } from "./gate.js";
 import type { ToolCall } from "@intx/types/runtime";
 
 function readRecords(dir: string): Record<string, unknown>[] {
-  const raw = readFileSync(join(dir, APPROVAL_LOG_FILE), "utf8");
+  let raw: string;
+  try {
+    raw = readFileSync(join(dir, APPROVAL_LOG_FILE), "utf8");
+  } catch {
+    return [];
+  }
   return raw
     .split("\n")
     .filter((l) => l.trim().length > 0)
@@ -143,4 +148,60 @@ describe("approval-log wiring through the permission gate", () => {
     expect(record!.outcome).toBe("deny");
     expect(record!.rule).toBe("non-interactive");
   });
+
+  // A sub-agent's `task` dispatch `description` is model-authored free text
+  // (see task-tool.ts) — it is only ever trimmed, never constrained to a
+  // closed set. A prior version of this log carried it verbatim as
+  // `agentLabel`. It must never reach the record: unlike `rule` (a fixed
+  // taxonomy) and `segments` (a count), nothing stops a model from quoting a
+  // path, a token, or secret content it just read into its own summary of the
+  // sub-task.
+  test("never logs a sub-agent's free-text dispatch description, even with a secret embedded", async () => {
+    const { runWithSubAgentIdentity } = await import("../subagent/identity-context.js");
+    const dir = mkdtempSync(join(tmpdir(), "approval-log-gate-"));
+    const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-"));
+    const gate = createPermissionGate({
+      approvals: [],
+      interactive: true,
+      skipPermissions: false,
+      cwd,
+      approvalLog: createApprovalLog(dir),
+      requestApproval: async (request) => {
+        request.markDisplayed?.();
+        return { allow: true };
+      },
+    });
+    const secret = "sk-live-9f2c7a1e4b6d8f0a";
+    const verdict = await runWithSubAgentIdentity(
+      { description: `fetch the token ${secret} from the vault and cache it`, cwd },
+      () => gate.evaluate(shellCall("curl https://example.com")),
+    );
+    expect(verdict.allowed).toBe(true);
+
+    await new Promise((r) => setTimeout(r, 10));
+    const [record] = readRecords(dir);
+    expect(record).toBeDefined();
+    expect(Object.keys(record!)).not.toContain("agentLabel");
+    const serialized = JSON.stringify(record);
+    expect(serialized).not.toContain(secret);
+    expect(serialized).not.toContain("vault");
+  });
+});
+
+describe("approval-log record size cap", () => {
+  test("drops a record that would exceed the hard size cap rather than truncate it", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "approval-log-cap-"));
+    const log = createApprovalLog(dir);
+    // `rule` is a real, typed field — simulate a future regression where some
+    // caller stuffs unbounded text into it instead of the closed taxonomy.
+    // The cap must catch that even though the type system would not.
+    const ask = log.ask({
+      tool: "run_shell",
+      mode: "interactive",
+      rule: "x".repeat(10_000),
+    });
+    ask.settle("allow-once");
+    await new Promise((r) => setTimeout(r, 10));
+    expect(readRecords(dir)).toHaveLength(0);
+  });
 });

src/permission/approval-log.ts

@@ -10,12 +10,20 @@
  * classifier rule triggers them, or whether a prompt was even auto-allowed by
  * policy rather than shown to the operator (CL-5666).
  *
- * No command text, file content, path, or credential ever appears here — only
- * the tool name, the classifier/auto-shell rule that fired (reusing the rule
- * names already defined in auto-shell-policy.ts and classify.ts, not a new
- * taxonomy), a shell chain's segment count, and timing. Writes are
+ * No command text, file content, path, credential, or any other
+ * model-authored or user-authored free text ever appears here — only the tool
+ * name (a fixed identifier), the classifier/auto-shell rule that fired
+ * (reusing the rule names already defined in auto-shell-policy.ts and
+ * classify.ts, plus a small closed set of additional literals this file
+ * defines for decisions those modules don't otherwise name — never a new
+ * taxonomy), a shell chain's segment count, and timing. Every field is either
+ * a fixed enum, a count, or a timestamp; a sub-agent's free-text dispatch
+ * label was deliberately left out even though it would enable a per-agent
+ * breakdown, because nothing constrains what a model puts in it. Writes are
  * fire-and-forget and never throw: a diagnostic must not be able to fail a
- * run.
+ * run. A hard size cap on the serialized line (see MAX_RECORD_BYTES) is
+ * belt-and-suspenders insurance against a future field reintroducing free
+ * text.
  */
 
 import { appendFile } from "node:fs/promises";
@@ -51,8 +59,6 @@ export interface ApprovalRecord {
    */
   rule?: string;
   mode: ApprovalMode;
-  /** The requesting sub-agent's label, when this request came from one. */
-  agentLabel?: string;
   /** Real (non-comment) shell chain segment count, for run_shell requests. */
   segments?: number;
   outcome: ApprovalOutcomeKind;
@@ -77,10 +83,17 @@ export interface AskEvent {
   tool: string;
   rule?: string;
   mode: ApprovalMode;
-  agentLabel?: string;
   segments?: number;
 }
 
+// Belt-and-suspenders cap on the serialized record. Every field here is
+// either a fixed enum, a count, or a timestamp, so a well-formed line should
+// never come close to this — it exists only so a future field that
+// reintroduces free text (an agent label, a subject, a message) cannot grow
+// this file into a content leak; oversized lines are dropped, not truncated,
+// so no partial secret survives half-written.
+const MAX_RECORD_BYTES = 512;
+
 /** Handle for one in-flight ask, returned by ApprovalLog.ask(). */
 export interface ApprovalAsk {
   readonly id: string;
@@ -118,6 +131,10 @@ export function createApprovalLog(dir: string, now: () => Date = () => new Date(
 
   const append = (record: ApprovalRecord): void => {
     const line = `${JSON.stringify(record)}\n`;
+    if (Buffer.byteLength(line, "utf8") > MAX_RECORD_BYTES) {
+      log.debug?.("approval log record dropped: exceeds max size");
+      return;
+    }
     tail = tail.then(
       () =>
         appendFile(path, line, "utf8").catch((err: unknown) => {
@@ -148,7 +165,6 @@ export function createApprovalLog(dir: string, now: () => Date = () => new Date(
             tool: event.tool,
             ...(event.rule !== undefined ? { rule: event.rule } : {}),
             mode: event.mode,
-            ...(event.agentLabel !== undefined ? { agentLabel: event.agentLabel } : {}),
             ...(event.segments !== undefined ? { segments: event.segments } : {}),
             outcome,
             queuedAt: queuedAt.toISOString(),

src/permission/gate.ts

@@ -413,14 +413,12 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
     tool: string,
     rule: string | undefined,
     outcome: ApprovalOutcomeKind,
-    agentLabel: string | undefined,
   ): void => {
     approvalLog
       .ask({
         tool,
         mode: "auto",
         ...(rule !== undefined ? { rule } : {}),
-        ...(agentLabel !== undefined ? { agentLabel } : {}),
       })
       .settle(outcome);
   };
@@ -494,20 +492,15 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
         // a secret path is ask so an explicit one-time approval can pass it.
         const shellRule = autoShellRuleForCall(call, isRestrictedHere, effectiveCwd, rootsProvider);
         if (shellRule?.effect === "deny") {
-          recordAutoDecision(call.name, shellRule.name, "auto-deny", subAgentIdentity?.description);
+          recordAutoDecision(call.name, shellRule.name, "auto-deny");
           return { allowed: false, reason: shellRule.reason };
         }
         if (shellRule === undefined) {
-          recordAutoDecision(call.name, undefined, "auto-allow", subAgentIdentity?.description);
+          recordAutoDecision(call.name, undefined, "auto-allow");
           return { allowed: true };
         }
       } else if (!restricted && AUTO_ALLOWED_TOOLS.has(call.name)) {
-        recordAutoDecision(
-          call.name,
-          "auto-allowed-tool",
-          "auto-allow",
-          subAgentIdentity?.description,
-        );
+        recordAutoDecision(call.name, "auto-allowed-tool", "auto-allow");
         return { allowed: true };
       }
       // Any other tool in auto mode (MCP or unknown built-in) is not
@@ -617,12 +610,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
         const askRule = anySecret ? "sensitive-path" : isMegaChain ? "mega-chain" : undefined;
 
         if (!interactive || requestApproval === undefined) {
-          recordAutoDecision(
-            request.tool,
-            askRule ?? "non-interactive",
-            "deny",
-            request.agentLabel,
-          );
+          recordAutoDecision(request.tool, askRule ?? "non-interactive", "deny");
           return {
             allowed: false,
             reason: anySecret
@@ -638,7 +626,6 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
           tool: request.tool,
           mode: "interactive",
           ...(askRule !== undefined ? { rule: askRule } : {}),
-          ...(request.agentLabel !== undefined ? { agentLabel: request.agentLabel } : {}),
           segments: segments.length,
         });
         requestForOperator.markDisplayed = ask.markDisplayed;
@@ -685,7 +672,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
       }
 
       if (!interactive || requestApproval === undefined) {
-        recordAutoDecision(request.tool, "non-interactive", "deny", request.agentLabel);
+        recordAutoDecision(request.tool, "non-interactive", "deny");
         return {
           allowed: false,
           reason: `${request.action} requires operator approval, which is unavailable in a non-interactive run. Re-run with --dangerously-skip-permissions to bypass, or narrow the action.`,
@@ -695,7 +682,6 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
       const ask = approvalLog.ask({
         tool: request.tool,
         mode: "interactive",
-        ...(request.agentLabel !== undefined ? { agentLabel: request.agentLabel } : {}),
       });
       request.markDisplayed = ask.markDisplayed;
       const turnId = currentTurnId();