Skip to content

Commit 8b3cf2c

Browse files
committed
fix(authz): redact capability ids and scrub persisted approval args
Correlation ids are bearer capabilities (see the provenance gate): log and receipt sites now emit a truncated+hashed form, so a log reader no longer becomes a forgery actor. Approval-snapshot arguments are secret- scrubbed at the persisted-metadata boundary (live calls untouched), and credential headers freeze at the source cell. The vendored raw-warn sites are unreachable from the app path and are dispositioned in the vendor ledger; retry policies are documented trusted-first-party-only.
1 parent 950a3e3 commit 8b3cf2c

17 files changed

Lines changed: 578 additions & 19 deletions

‎docs/VENDORING.md‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,78 @@ recorded SHA to print exactly the lines that are ours. A correspondence
331331
test (`scripts/vendor-patch-ledger.test.ts`) fails if a marker anchor
332332
does not resolve to a ledger heading, or if a ledger heading has no marker.
333333

334+
## Approval rail hygiene audit (CL-10679)
335+
336+
App-side redaction built on CL-10678's provenance gate, audited against the
337+
CL-10678 base (`950a3e37`). Nothing in `vendor/` was modified for this audit;
338+
the vendored findings below are recorded as suppression/disposition notes.
339+
340+
**Vendored validator-throw log (`reactor.ts:687`,
341+
`logger.warn`Correlation validator threw for ${correlationId}...``) —
342+
unreachable in production wiring.** `tryCorrelate` only calls a validator when
343+
one is injected (`correlationValidator !== undefined`), and both the vendored
344+
`@intx/agent` env contract and every production call site forward none (see
345+
the "Bearer correlation IDs and approval provenance" section above and the
346+
comment at `src/agent/live-tool-dispatch.ts`). Foreign ingress is dropped the
347+
line before it could ever be correlated (`createApprovalProvenanceGate` wraps
348+
the app's `createAgent` deliver seam, fail-closed on unknown mailboxes), so
349+
this vendored warn can log a bearer correlation ID only if a future wiring
350+
injects a validator — and then only when that validator throws. When a seam
351+
forwards the app validator, keep `createApprovalCorrelationValidator`'s
352+
`validate` non-throwing: it returns `Promise.resolve(...)`, never throws, so
353+
the death of this branch stays guaranteed even hot-swapped. **Second vendored
354+
full-ID site, promotion candidate:** the `processDelivery` fatal path
355+
(`reactor.ts` ~1906-1923 — `logger.error`Correlation dispatch failed: ${cause}``
356+
plus the `closeMessageRun` failure text) can carry a full correlation ID a
357+
correlation-path invariant failure embeds in its message (the registry's
358+
double-registration throw names the ID). The deliver-gate pre-drop makes that
359+
fatal branch effectively unreachable today (a throw can arise inside
360+
`tryCorrelate`'s patched acceptance/correlation region, but no untrusted
361+
delivery reaches it without first injecting a validator); recorded here so a
362+
future wiring that can reach it gets an ID-free receipt at the next sync.
363+
364+
**Vendored retry-policy throw log (`harness.ts:1749`) — app-side
365+
disposition, no vendor edit.** The vendored `runInference` catch sanitizes the
366+
terminal error it shows the caller and passes policies pre-sanitized inputs,
367+
but the warn about a policy's own exception interpolates raw
368+
(`error=${cause.message}`). The app keeps its vendor tree untouched here and
369+
answers the exposure contractually instead: `RetryPolicy` implementations are
370+
trusted first-party code and must never embed credential-bearing text in an
371+
exception — documented on `createCorbitsRetryPolicy`
372+
(`src/agent/retry-policy.ts`). The vendored line is a promotion candidate if
373+
upstream adopts a sanitized clone for policy-exception logging; the raw
374+
terminal `inference.error` event keeps its existing sanitization.
375+
376+
**App-side correlation-ID receipt redaction.** All app-side log sites that
377+
interpolated a correlation ID now emit the truncated, hash-suffixed form
378+
(`src/session/redacted-correlation.ts`): the parked-approval,
379+
late-decision and duplicate-resume warns in `src/session/approval-resume.ts`,
380+
the forged-approval discard receipt in `src/session/approval-provenance.ts`,
381+
and the acceptance-timeout warn in `src/tui/approval-delivery.ts`. The file
382+
log sink (`src/logging/sink.ts`) records debug level too, so nothing app-side
383+
carries the full ID at any log level; the full ID lives only on the in-memory
384+
transport that consumes it (message headers, gate table, operator surface).
385+
386+
**Event-sink audit.** Correlation IDs do not reach model-visible history or
387+
tool-output-readable surfaces: `run.json` excludes correlation IDs (status/task/turn
388+
counts and usage only), the audit collector records `tool.start`/`tool.done`
389+
keyed by callId — not correlation ID — and `message.correlated` /
390+
`reactor.gate.blocked` events fan out to in-memory consumers (TUI occupancy
391+
wait, runtime bridge, watchdog recovery) without being persisted. The
392+
`metadata.json` checkpoint persists pending operations with their correlation
393+
IDs by design (the restart-resume path re-registers parked calls) — that is
394+
the credential cell's own storage, not a log or event sink.
395+
396+
**Approval snapshot args at rest (CL-10679 R5).** The app's context-store
397+
write boundary (`src/session/pending-snapshot-scrub.ts`, wired into
398+
`src/session/optimized-context-store.ts`) now runs the result-side secret-shape
399+
scrubber (`scrubSecretShapedValue`) over every persisted
400+
`approvalSnapshot.arguments`. The re-dispatch artifact (`suspendedCall`)
401+
stays raw because the approved call must re-run with the arguments the
402+
operator saw; residual at-rest exposure in `suspendedCall.arguments` inside
403+
`metadata.json` is documented as a known gap for the vendored re-dispatch
404+
owner, not silently widened here.
405+
334406
## Re-syncing a vendored package to a newer upstream commit
335407

336408
1. In the read-only upstream clone, confirm the commit to sync to and note

‎src/agent/retry-policy.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,16 @@ export interface CorbitsRetryPolicyOptions {
4848
* before `normalizeInferenceErrorForRetry` so known-provider remappers (xAI
4949
* short 429 → retryable, Go, Codex) can gate on context the harness does not
5050
* attach to InferenceError today.
51+
*
52+
* Trust model: a `RetryPolicy` is trusted first-party code. The vendored
53+
* harness sanitizes the `situation.error` it passes here (the terminal error
54+
* is run through `sanitizeCredentialDiagnostic` before the policy call) and
55+
* still sanitizes a same-source recovered diagnostic, but the warn it emits
56+
* when the policy itself throws has no sanitizer — a policy exception is
57+
* logged raw (CL-10679 ledger note,
58+
* docs/VENDORING.md#approval-rail-hygiene-audit). Never embed bearer-shaped
59+
* or otherwise credential-bearing text in a policy exception; reach for the
60+
* pre-sanitized `situation.error` instead.
5161
*/
5262
export function createCorbitsRetryPolicy(
5363
options?: CorbitsRetryPolicyOptions,
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { afterEach, describe, expect, test } from "bun:test";
2+
import type { CredentialMaterial } from "@intx/types";
3+
4+
import {
5+
clearSourceCredentials,
6+
findSourceCredentialRecord,
7+
registerSourceCredentialRecord,
8+
rotateSourceCredentialMaterialIfCurrent,
9+
} from "./source-credentials.js";
10+
11+
const material: CredentialMaterial = {
12+
secret: "secret-value",
13+
headers: { "chatgpt-account-id": "account-a" },
14+
};
15+
16+
afterEach(() => {
17+
clearSourceCredentials();
18+
});
19+
20+
describe("credential material freeze at the source cell", () => {
21+
test("registered material and its headers are frozen", () => {
22+
registerSourceCredentialRecord("c1", {
23+
provenance: { kind: "api-key" },
24+
material,
25+
});
26+
const record = findSourceCredentialRecord("c1");
27+
expect(Object.isFrozen(record?.material)).toBe(true);
28+
expect(Object.isFrozen(record?.material.headers)).toBe(true);
29+
});
30+
31+
test("the caller's headers object is copied, not aliased into the cell", () => {
32+
const headers = { "chatgpt-account-id": "account-a" };
33+
registerSourceCredentialRecord("c1", {
34+
provenance: { kind: "api-key" },
35+
material: { secret: "secret-value", headers },
36+
});
37+
headers["chatgpt-account-id"] = "attacker-a";
38+
expect(findSourceCredentialRecord("c1")?.material.headers).toEqual({
39+
"chatgpt-account-id": "account-a",
40+
});
41+
});
42+
43+
test("an in-place mutation attempt after register never reaches the cell", () => {
44+
registerSourceCredentialRecord("c1", {
45+
provenance: { kind: "api-key" },
46+
material,
47+
});
48+
const stored = findSourceCredentialRecord("c1");
49+
const headers = stored?.material.headers as Record<string, string>;
50+
// Frozen headers make the mutation fail loudly in strict mode.
51+
expect(() => {
52+
headers["chatgpt-account-id"] = "attacker-a";
53+
}).toThrow();
54+
expect(stored?.material.headers).toEqual({
55+
"chatgpt-account-id": "account-a",
56+
});
57+
});
58+
59+
test("a rotation stores a fresh frozen snapshot and releases the old record", () => {
60+
registerSourceCredentialRecord("c1", {
61+
provenance: { kind: "api-key" },
62+
material,
63+
});
64+
const expected = findSourceCredentialRecord("c1");
65+
const rotated = { secret: "next-token" };
66+
expect(
67+
rotateSourceCredentialMaterialIfCurrent("c1", expected as never, rotated),
68+
).toBe(true);
69+
const record = findSourceCredentialRecord("c1");
70+
expect(Object.isFrozen(record?.material)).toBe(true);
71+
expect(record?.material.secret).toBe("next-token");
72+
});
73+
});

‎src/config/source-credentials.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,32 @@ export interface SourceCredentialRecord {
1919

2020
const cell = new Map<string, SourceCredentialRecord>();
2121

22+
// The material rides process memory long after registration (per-use resolver
23+
// reads, subagent failover chains). Freezing at the cell boundary means every
24+
// downstream holder sees an immutable snapshot: a mutated "material" never
25+
// masquerades as the registered secret, and an attempted header mutation fails
26+
// loudly instead of silently diverging from what the provider sent. Secrets
27+
// are strings; the headers record is the mutable part being contained.
28+
function freezeCredentialMaterial(
29+
material: CredentialMaterial,
30+
): CredentialMaterial {
31+
const frozen: CredentialMaterial = {
32+
...material,
33+
...(material.headers !== undefined
34+
? { headers: Object.freeze({ ...material.headers }) }
35+
: {}),
36+
};
37+
return Object.freeze(frozen);
38+
}
39+
2240
export function registerSourceCredentialRecord(
2341
credentialId: string,
2442
record: SourceCredentialRecord,
2543
): void {
26-
cell.set(credentialId, record);
44+
cell.set(credentialId, {
45+
...record,
46+
material: freezeCredentialMaterial(record.material),
47+
});
2748
}
2849

2950
export function rotateSourceCredentialMaterialIfCurrent(
@@ -32,7 +53,10 @@ export function rotateSourceCredentialMaterialIfCurrent(
3253
material: CredentialMaterial,
3354
): boolean {
3455
if (cell.get(credentialId) !== expected) return false;
35-
cell.set(credentialId, { provenance: expected.provenance, material });
56+
cell.set(credentialId, {
57+
provenance: expected.provenance,
58+
material: freezeCredentialMaterial(material),
59+
});
3660
return true;
3761
}
3862

‎src/session/approval-provenance.test.ts‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ import {
1010
LOCAL_APPROVAL_FROM,
1111
LOCAL_APPROVAL_MAILBOX,
1212
} from "./approval-provenance.js";
13+
import { redactedCorrelationId } from "./redacted-correlation.js";
1314

1415
interface MessageInput {
1516
from: string;
1617
mailbox?: string;
18+
correlationId?: string;
1719
}
1820

1921
function approvalMessage(
@@ -26,7 +28,7 @@ function approvalMessage(
2628
const message = createInboundMessage({
2729
from: args.from,
2830
to: "agent@local",
29-
correlationId: "corr-1",
31+
correlationId: args.correlationId ?? "corr-1",
3032
content: JSON.stringify({ outcome }),
3133
...(args.mailbox === undefined ? {} : { ref: { mailbox: args.mailbox } }),
3234
});
@@ -179,12 +181,38 @@ describe("createApprovalProvenanceGate", () => {
179181
record.text.includes("forged approval discarded"),
180182
);
181183
expect(receipts).toHaveLength(2);
182-
expect(receipts[0]?.text).toContain("correlationId=corr-1");
184+
// The warn receipt carries the truncated-and-hashed form, never the
185+
// live bearer capability (see src/session/redacted-correlation.ts).
186+
expect(receipts[0]?.text).toContain(
187+
`correlationId=${redactedCorrelationId("corr-1")}`,
188+
);
189+
expect(receipts[0]?.text).not.toContain("correlationId=corr-1 type");
183190
expect(receipts[0]?.text).toContain("from=watchdog@foreign");
184191
expect(receipts[0]?.text).toContain("mailbox=INBOX");
185192
expect(receipts[0]?.text).toContain("type=approval.granted");
186193
expect(receipts[1]?.text).toContain("from=webhook@foreign");
187194
expect(receipts[1]?.text).toContain("mailbox=<none>");
195+
const uuid = "6f4f7c16-2e21-4a2d-9f4b-7c1e93a88b41";
196+
gate.deliver(
197+
approvalMessage({
198+
from: "probe@foreign",
199+
mailbox: "INBOX",
200+
correlationId: uuid,
201+
}),
202+
);
203+
const longReceipts = records.filter(
204+
(record) =>
205+
record.level === "warning" &&
206+
record.text.includes("forged approval discarded") &&
207+
record.text.includes("probe@foreign"),
208+
);
209+
expect(longReceipts).toHaveLength(1);
210+
// A full-length ID never appears in the receipt: only its prefix and
211+
// the hash suffix do.
212+
expect(longReceipts[0]?.text).toContain(
213+
`correlationId=${redactedCorrelationId(uuid)}`,
214+
);
215+
expect(longReceipts[0]?.text).not.toContain(uuid);
188216
expect(deliver).not.toHaveBeenCalled();
189217
} finally {
190218
if (previous === null) resetSync();

‎src/session/approval-provenance.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import type { InboundMessage } from "@intx/types/runtime";
2424
import { getLogger } from "@intx/log";
2525

2626
import { LOG_NAMESPACE_ROOT } from "../branding.js";
27+
import { redactedCorrelationId } from "./redacted-correlation.js";
2728

2829
const logger = getLogger([LOG_NAMESPACE_ROOT, "approval-provenance"]);
2930

@@ -84,7 +85,11 @@ export function createApprovalProvenanceGate<
8485
...inner,
8586
deliver(message: InboundMessage): void {
8687
if (isForeignApprovalIngress(message)) {
87-
logger.warn`forged approval discarded, foreign ingress correlationId=${message.headers.interchangeCorrelationId} from=${message.headers.from} mailbox=${message.ref?.mailbox ?? "<none>"} type=${message.headers.interchangeType}`;
88+
// The log sink (src/logging/sink.ts) records debug level too, so the
89+
// receipt trail must not carry the full bearer capability at any
90+
// level — warn and debug both use the redacted form here, and the
91+
// full ID stays only on the in-memory transport that consumes it.
92+
logger.warn`forged approval discarded, foreign ingress correlationId=${redactedCorrelationId(message.headers.interchangeCorrelationId ?? "?")} from=${message.headers.from} mailbox=${message.ref?.mailbox ?? "<none>"} type=${message.headers.interchangeType}`;
8893
return;
8994
}
9095
inner.deliver(message);

‎src/session/approval-resume.ts‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import {
3636
LOCAL_APPROVAL_FROM,
3737
LOCAL_APPROVAL_MAILBOX,
3838
} from "./approval-provenance.js";
39+
import { redactedCorrelationId } from "./redacted-correlation.js";
40+
import { canonicalSnapshotScrub } from "./pending-snapshot-scrub.js";
3941

4042
const logger = getLogger([LOG_NAMESPACE_ROOT, "approval-resume"]);
4143

@@ -141,7 +143,7 @@ export function createSuspendedApprovalRecovery(args: {
141143
if (candidate === owned) candidate = undefined;
142144
},
143145
() => {
144-
logger.warn`parked approval not presented correlation=${correlationId}`;
146+
logger.warn`parked approval not presented correlation=${redactedCorrelationId(correlationId)}`;
145147
},
146148
);
147149
},
@@ -287,8 +289,18 @@ export async function resolveSuspendedApprovalFromStore(
287289
if (
288290
parked.name !== parsedPersisted.name ||
289291
parked.name !== parsedCaptured.name ||
290-
!isDeepStrictEqual(parked.arguments, parsedPersisted.arguments ?? {}) ||
291-
!isDeepStrictEqual(parked.arguments, parsedCaptured.arguments ?? {})
292+
// Both sides of each comparison run through the same canonical scrub, so
293+
// the persisted snapshot (scrubbed at the metadata write boundary) still
294+
// matches the live raw snapshot: secret-shaped values are redacted
295+
// identically on both sides; benign values are unchanged.
296+
!isDeepStrictEqual(
297+
canonicalSnapshotScrub(parked.arguments),
298+
canonicalSnapshotScrub(parsedPersisted.arguments ?? {}),
299+
) ||
300+
!isDeepStrictEqual(
301+
canonicalSnapshotScrub(parked.arguments),
302+
canonicalSnapshotScrub(parsedCaptured.arguments ?? {}),
303+
)
292304
)
293305
return { ok: false, code: "snapshot-mismatch" };
294306
return { ok: true, parkedCallId: parked.id };
@@ -524,7 +536,7 @@ export function createApprovalResume(args: {
524536
return true;
525537
}
526538
if (timedOut) {
527-
logger.warn`late approval decision dropped correlation=${correlationId} timeoutCall=${parkedCallId} outcome=${outcome?.allow === true ? "approved" : "rejected"}`;
539+
logger.warn`late approval decision dropped correlation=${redactedCorrelationId(correlationId)} timeoutCall=${parkedCallId} outcome=${outcome?.allow === true ? "approved" : "rejected"}`;
528540
return true;
529541
}
530542
await deliverDecision(
@@ -555,7 +567,7 @@ export function createApprovalResume(args: {
555567
if (result.type !== "suspended") return Promise.resolve(false);
556568
const { correlationId } = result;
557569
if (handedOver.has(correlationId)) {
558-
logger.warn`duplicate approval resume ignored correlation=${correlationId}`;
570+
logger.warn`duplicate approval resume ignored correlation=${redactedCorrelationId(correlationId)}`;
559571
return Promise.resolve(true);
560572
}
561573
const ongoing = inflight.get(correlationId);

0 commit comments

Comments
 (0)