Skip to content

Commit 4aac8f8

Browse files
committed
fix(session): reject swapped credentials in approval snapshot compare
Integrity compare no longer collapses every secret to one redaction token. Redacted-at-rest snapshots still match the live call; raw re-dispatch arguments must match exactly.
1 parent 8b3cf2c commit 4aac8f8

3 files changed

Lines changed: 118 additions & 16 deletions

File tree

‎src/session/approval-resume.ts‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import type {
1616
ContextStore,
1717
InboundMessage,
1818
} from "@intx/types/runtime";
19-
import { isDeepStrictEqual } from "node:util";
2019
import { type } from "arktype";
2120

2221
import { getLogger } from "@intx/log";
@@ -37,7 +36,7 @@ import {
3736
LOCAL_APPROVAL_MAILBOX,
3837
} from "./approval-provenance.js";
3938
import { redactedCorrelationId } from "./redacted-correlation.js";
40-
import { canonicalSnapshotScrub } from "./pending-snapshot-scrub.js";
39+
import { snapshotArgumentsMatch } from "./pending-snapshot-scrub.js";
4140

4241
const logger = getLogger([LOG_NAMESPACE_ROOT, "approval-resume"]);
4342

@@ -289,17 +288,19 @@ export async function resolveSuspendedApprovalFromStore(
289288
if (
290289
parked.name !== parsedPersisted.name ||
291290
parked.name !== parsedCaptured.name ||
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 ?? {}),
291+
// The persisted copy is redacted at rest, so its compare still accepts the
292+
// canonical scrub when that copy is fully redacted. The re-dispatch
293+
// snapshot is compared raw: different credential values are a mismatch
294+
// even though both would scrub to the same token.
295+
!snapshotArgumentsMatch(
296+
parked.arguments,
297+
parsedPersisted.arguments ?? {},
298+
true,
299299
) ||
300-
!isDeepStrictEqual(
301-
canonicalSnapshotScrub(parked.arguments),
302-
canonicalSnapshotScrub(parsedCaptured.arguments ?? {}),
300+
!snapshotArgumentsMatch(
301+
parked.arguments,
302+
parsedCaptured.arguments ?? {},
303+
false,
303304
)
304305
)
305306
return { ok: false, code: "snapshot-mismatch" };

‎src/session/pending-snapshot-scrub.test.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,47 @@ describe("scrubPersistedPendingOperations", () => {
143143
);
144144
expect(drifted).toEqual({ ok: false, code: "snapshot-mismatch" });
145145
});
146+
147+
test("a swapped credential in the re-dispatch payload is a snapshot mismatch", async () => {
148+
const keyA = "sk-proj-aaaaaaaaaaaaaaaaaaaa";
149+
const keyB = "sk-proj-bbbbbbbbbbbbbbbbbbbb";
150+
const parkedArgs = { api_key: keyA, mode: "fast" };
151+
const swappedArgs = { api_key: keyB, mode: "fast" };
152+
const op: PendingOperation = {
153+
correlationId: "corr-9",
154+
kind: "approval",
155+
registeredAt: 1_700_000_000_000,
156+
gateId: "gate-9",
157+
timeoutAt: Date.now() + 60_000,
158+
suspendedCall: { id: "call-9", name: "run_shell", arguments: parkedArgs },
159+
approvalSnapshot: {
160+
name: "run_shell",
161+
description: "runs a shell command",
162+
inputSchema: {},
163+
arguments: swappedArgs,
164+
},
165+
};
166+
const swapped = await resolveSuspendedApprovalFromStore(
167+
{
168+
load: async () => ({
169+
turns: [],
170+
pendingOperations: [op],
171+
tokenUsage: {
172+
input: 0,
173+
output: 0,
174+
cacheRead: 0,
175+
cacheWrite: 0,
176+
thinking: 0,
177+
},
178+
connectorState: null,
179+
}),
180+
},
181+
{
182+
type: "suspended",
183+
correlationId: "corr-9",
184+
approvalSnapshot: op.approvalSnapshot,
185+
} as Extract<SendResult, { type: "suspended" }>,
186+
);
187+
expect(swapped).toEqual({ ok: false, code: "snapshot-mismatch" });
188+
});
146189
});

‎src/session/pending-snapshot-scrub.ts‎

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,15 @@
77
// metadata write boundary; `suspendedCall` stays untouched because the
88
// re-dispatch of an approved call must see the raw arguments.
99
//
10-
// Snapshot equality checks (`resolveSuspendedApprovalFromStore`) run through
11-
// the same canonical transform on BOTH sides, so a scrubbed persisted snapshot
12-
// still matches the live raw snapshot exactly the way the unscrubbed copies
13-
// used to — one transform, symmetrically applied.
10+
// Snapshot equality checks (`resolveSuspendedApprovalFromStore`) must not
11+
// collapse every secret to one redaction token before comparing: that made
12+
// two different credentials compare equal. Persisted snapshots are redacted
13+
// at the metadata write, so a fully redacted copy still matches the live raw
14+
// snapshot through the canonical scrub. A copy that still carries raw secret
15+
// material — and the in-memory re-dispatch snapshot — compare without that
16+
// lossy collapse.
17+
18+
import { isDeepStrictEqual } from "node:util";
1419

1520
import type { PendingOperation } from "@intx/types/runtime";
1621

@@ -38,6 +43,59 @@ export function canonicalSnapshotScrub(value: unknown): unknown {
3843
}
3944
}
4045

46+
function jsonPlain(value: unknown): unknown {
47+
return JSON.parse(JSON.stringify(value)) as unknown;
48+
}
49+
50+
// True when `value` is already the redacted canonical form (the scrub is a
51+
// no-op). A snapshot that still holds a raw credential fails this check.
52+
function isRedactedCanonical(value: unknown): boolean {
53+
try {
54+
return isDeepStrictEqual(jsonPlain(value), canonicalSnapshotScrub(value));
55+
} catch {
56+
return false;
57+
}
58+
}
59+
60+
function rawArgumentsEqual(left: unknown, right: unknown): boolean {
61+
try {
62+
return isDeepStrictEqual(jsonPlain(left), jsonPlain(right));
63+
} catch {
64+
// Non-JSON-safe args cannot be compared raw. Fall back to the shared
65+
// scrub stub so a parked call still lines up with its own snapshot;
66+
// JSON-safe credentials never take this path.
67+
return isDeepStrictEqual(
68+
canonicalSnapshotScrub(left),
69+
canonicalSnapshotScrub(right),
70+
);
71+
}
72+
}
73+
74+
/**
75+
* Integrity compare for approval arguments.
76+
*
77+
* `persisted` is the on-disk snapshot. When that copy was redacted at rest,
78+
* secret fields are the stable redaction token and the canonical scrub still
79+
* lines up with the live raw args. When it still contains a raw credential,
80+
* compare the values themselves so a different secret is a mismatch.
81+
*
82+
* The in-memory re-dispatch snapshot (`persisted: false`) is never collapsed:
83+
* two different credential values must not compare equal.
84+
*/
85+
export function snapshotArgumentsMatch(
86+
parked: unknown,
87+
other: unknown,
88+
persisted: boolean,
89+
): boolean {
90+
if (persisted && isRedactedCanonical(other)) {
91+
return isDeepStrictEqual(
92+
canonicalSnapshotScrub(parked),
93+
canonicalSnapshotScrub(other),
94+
);
95+
}
96+
return rawArgumentsEqual(parked, other);
97+
}
98+
4199
export function scrubPersistedPendingOperations(
42100
pendingOperations: readonly PendingOperation[],
43101
): PendingOperation[] {

0 commit comments

Comments
 (0)