diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index ff0176c946..65908d43b8 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -242,9 +242,10 @@ struct PermissionEntry { /// any non-accepted outcome (entry is denied instead). early_decision: Option, /// Human-readable description of the requested operation, extracted from - /// `params.subject` in the ACP `session/request_permission` message. - /// Truncated to `SENTINEL_STRING_MAX_BYTES`. `None` when the adapter did - /// not provide a subject or provided an empty string. + /// the ACP `session/request_permission` message via `description_from_request_permission`. + /// Tries `params.title`, `params.subject.toolCall.title`, `params.toolCall.title`, + /// `params.toolCall.rawInput.command`, and `params._meta.codex.params.reason` in order. + /// Truncated to `SENTINEL_STRING_MAX_BYTES`. `None` when no path yields a non-empty string. description: Option, } @@ -3868,9 +3869,16 @@ fn sentinel_option_fields(actions: &CardActions) -> (Vec, ser /// in `crates/buzz-agent/src/wire.rs`, version >= 2). /// 2. `params.subject.toolCall.title` — v2 nested fallback (same value, different /// path). -/// 3. `params.toolCall.title` — buzz-agent v1 and codex-acp permissions-request. +/// 3. `params.toolCall.title` — buzz-agent v1 and codex-acp permissions-request +/// (`buildPermissionsRequest`, kind "other", in codex-acp `CodexApprovalHandler.ts`). +/// 4. `params.toolCall.rawInput.command` — codex-acp v1.1.7 command execution +/// requests (`buildCommandPermissionRequest`): no title, command string in rawInput. +/// Verbatim wire shape from codex-acp tag v1.1.7. +/// 5. `params._meta.codex.params.reason` — codex-acp v1.1.7 file-change requests +/// (`buildFileChangePermissionRequest`): no title or rawInput command, reason is +/// in codex-supplied metadata. Verbatim wire shape from codex-acp tag v1.1.7. /// -/// Returns `None` when no non-empty title is found in any of these paths, or when +/// Returns `None` when no non-empty string is found in any of these paths, or when /// the `msg` argument does not have a `params` object. /// /// Extracted as a pure function (rather than inline in the event handler) so @@ -3883,6 +3891,12 @@ pub(crate) fn description_from_request_permission(msg: &serde_json::Value) -> Op .and_then(|v| v.as_str()), msg.pointer("/params/toolCall/title") .and_then(|v| v.as_str()), + // codex-acp v1.1.7 command execution: toolCall.rawInput.command + msg.pointer("/params/toolCall/rawInput/command") + .and_then(|v| v.as_str()), + // codex-acp v1.1.7 file-change: reason lives in _meta.codex.params + msg.pointer("/params/_meta/codex/params/reason") + .and_then(|v| v.as_str()), ] .into_iter() .flatten() @@ -7773,6 +7787,111 @@ mod tests { // ── Pinned §3: duplicate option IDs ────────────────────────────────────── + // ── F2 codex-acp v1.1.7 wire shapes ───────────────────────────────────── + // + // Verbatim wire shapes from `buildCommandPermissionRequest` and + // `buildFileChangePermissionRequest` in codex-acp tag v1.1.7 + // (`src/CodexApprovalHandler.ts`). These tests go red if the extraction + // pointers are changed back to anything that misses the v1.1.7 shapes. + + #[test] + fn description_from_codex_v1_1_7_command_request() { + // codex-acp v1.1.7 `buildCommandPermissionRequest`: + // { sessionId, toolCall: { toolCallId, kind:"execute", status:"pending", + // rawInput: { command, cwd } | null }, options, _meta: { codex: { params } } } + // No `title` at params or toolCall level — command is in rawInput.command. + // Verbatim from codex-acp tag v1.1.7 `CodexApprovalHandler.ts:119-135`. + let msg = serde_json::json!({ + "jsonrpc": "2.0", + "id": 3, + "method": "session/request_permission", + "params": { + "sessionId": "ses-codex-1", + "toolCall": { + "toolCallId": "tc-cmd-001", + "kind": "execute", + "status": "pending", + "rawInput": { + "command": "ls -la /tmp", + "cwd": "/home/user" + } + }, + "options": [], + "_meta": { "codex": { "params": { "command": "ls -la /tmp", "itemId": "tc-cmd-001" } } } + } + }); + let desc = description_from_request_permission(&msg); + assert_eq!( + desc.as_deref(), + Some("ls -la /tmp"), + "codex v1.1.7 command request must extract rawInput.command as description" + ); + } + + #[test] + fn description_from_codex_v1_1_7_file_change_request() { + // codex-acp v1.1.7 `buildFileChangePermissionRequest`: + // { sessionId, toolCall: { toolCallId, kind:"edit", status:"pending" }, + // options, _meta: { codex: { params: { ..., reason? } } } } + // No title or rawInput — useful context is in _meta.codex.params.reason. + // Verbatim from codex-acp tag v1.1.7 `CodexApprovalHandler.ts:137-151`. + let msg = serde_json::json!({ + "jsonrpc": "2.0", + "id": 4, + "method": "session/request_permission", + "params": { + "sessionId": "ses-codex-2", + "toolCall": { + "toolCallId": "tc-edit-002", + "kind": "edit", + "status": "pending" + }, + "options": [], + "_meta": { + "codex": { + "params": { + "itemId": "tc-edit-002", + "reason": "Write updated config to /etc/app.conf" + } + } + } + } + }); + let desc = description_from_request_permission(&msg); + assert_eq!( + desc.as_deref(), + Some("Write updated config to /etc/app.conf"), + "codex v1.1.7 file-change request must extract _meta.codex.params.reason as description" + ); + } + + #[test] + fn description_returns_none_for_file_change_without_reason() { + // codex-acp v1.1.7 file-change with no reason field — no fallback available. + let msg = serde_json::json!({ + "jsonrpc": "2.0", + "id": 5, + "method": "session/request_permission", + "params": { + "sessionId": "ses-codex-3", + "toolCall": { + "toolCallId": "tc-edit-003", + "kind": "edit", + "status": "pending" + }, + "options": [], + "_meta": { "codex": { "params": { "itemId": "tc-edit-003" } } } + } + }); + let desc = description_from_request_permission(&msg); + assert_eq!( + desc, None, + "file-change request with no reason must yield None" + ); + } + + // ── Pinned §3: duplicate option IDs ────────────────────────────────────── + #[test] fn admission_preflight_rejects_duplicate_option_ids() { let id = serde_json::json!(1); diff --git a/desktop/src/features/messages/ui/MessageRow.tsx b/desktop/src/features/messages/ui/MessageRow.tsx index d010761607..0bff36f505 100644 --- a/desktop/src/features/messages/ui/MessageRow.tsx +++ b/desktop/src/features/messages/ui/MessageRow.tsx @@ -35,7 +35,7 @@ import { getConfigNudgeAuthorPubkey } from "@/features/messages/ui/configNudgeAu import { PermissionRequestCardBlock } from "@/features/messages/ui/PermissionRequestCardBlock"; import { cn } from "@/shared/lib/cn"; import { normalizePubkey } from "@/shared/lib/pubkey"; -import { hasPermissionRequestCard as hasPermCard } from "@/features/messages/ui/permissionRequestAuthPubkey"; +import { selectPermissionRequest as selectPermReq } from "@/features/messages/ui/permissionRequestAuthPubkey"; import { UserAvatar } from "@/shared/ui/UserAvatar"; import { useChannelNavigation } from "@/shared/context/ChannelNavigationContext"; import { parseImetaTags } from "@/shared/ui/markdown/parseImeta"; @@ -378,6 +378,9 @@ export const MessageRow = React.memo( const getTag = (name: string) => message.tags?.find((tag) => tag[0] === name)?.[1]; + // Computed once — prose suppressed iff card renders, by construction. + const permReq = selectPermReq(message, isKnownAgentPubkey, channelId); + const renderBody = () => { switch (message.kind) { case KIND_STREAM_MESSAGE_DIFF: @@ -425,9 +428,7 @@ export const MessageRow = React.memo( ); } - if (message.isAgent && hasPermCard(message, isKnownAgentPubkey)) { - return null; - } + if (permReq !== null) return null; return ( {continuationMetadataNode} diff --git a/desktop/src/features/messages/ui/MessageRowProseGate.test.mjs b/desktop/src/features/messages/ui/MessageRowProseGate.test.mjs new file mode 100644 index 0000000000..9717137f82 --- /dev/null +++ b/desktop/src/features/messages/ui/MessageRowProseGate.test.mjs @@ -0,0 +1,292 @@ +/** + * MessageRow prose-suppression gate: forged-signer sentinel renders prose + * (not a blank row). + * + * This test exercises the REAL MessageRow call site for the prose gate: + * + * line 431: `if (permReq !== null) return null;` + * + * Thufir's B2 mutation replaces that line with + * `if (isPermissionRequestSentinel(message.body)) return null;` + * — which causes this test to fail: the sentinel shape matches → prose + * suppressed → the distinctive nonce string disappears from textContent → + * assertion fires. + * + * Sub-components that use `useAppNavigation` (which requires a live + * RouterProvider) are stubbed via inline `registerHooks` so the test runs + * in the standard Node.js + JSDOM harness without router overhead. + * The stub pattern follows inboxReopenNavigation.test.mjs. + * + * What is stubbed (and why): + * - `useAppNavigation` — called by `markdown.tsx` MarkdownInner and + * `SentFromThreadLine` for navigation callbacks; requires a live + * TanStack RouterProvider (async init, hangs the test in JSDOM). All + * stubbed callbacks are noops — navigation is not exercised here. + * - `MessageActionBar` — brings in `useRemindLater` and other Tauri-backed + * contexts; only needed for the action bar overlay which is irrelevant + * to the prose-gate contract. + * + * What is NOT stubbed: + * - `VideoReviewCommentMarkdown` / `MarkdownInner` — the actual prose + * renderer that displays the body text. Its output is exactly what + * the test observes. + */ +import assert from "node:assert/strict"; +import { registerHooks } from "node:module"; +import { test, before, afterEach, after, mock } from "node:test"; +import { JSDOM } from "jsdom"; + +// ── Stubs ────────────────────────────────────────────────────────────────── + +const NAV_STUB_SOURCE = + "export function useAppNavigation() {\n" + + " return {\n" + + " goChannel: async () => {},\n" + + " goAgents: async () => {},\n" + + " goHome: async () => {},\n" + + " goWorkflows: async () => {},\n" + + " goBack: async () => {},\n" + + " commitNavigation: async () => {},\n" + + " navigate: async () => {},\n" + + " handleSearchHit: async () => {},\n" + + " };\n" + + "}\n"; + +registerHooks({ + resolve(specifier, context, nextResolve) { + // useAppNavigation: called by markdown.tsx + SentFromThreadLine. + // Requires a live RouterProvider — stub so tests run without one. + if ( + specifier === "@/app/navigation/useAppNavigation" || + specifier.endsWith("/useAppNavigation.ts") || + specifier.endsWith("/useAppNavigation") + ) { + return { + shortCircuit: true, + url: "buzz-prose-gate-stub:useAppNavigation", + }; + } + // MessageActionBar: uses useRemindLater + other contexts irrelevant to + // the prose-gate contract. + if ( + specifier === "./MessageActionBar" || + specifier === "@/features/messages/ui/MessageActionBar" + ) { + return { + shortCircuit: true, + url: "buzz-prose-gate-stub:MessageActionBar", + }; + } + return nextResolve(specifier, context); + }, + load(url, context, nextLoad) { + if (url === "buzz-prose-gate-stub:useAppNavigation") { + return { format: "module", shortCircuit: true, source: NAV_STUB_SOURCE }; + } + if (url === "buzz-prose-gate-stub:MessageActionBar") { + return { + format: "module", + shortCircuit: true, + source: "export const MessageActionBar = () => null;\n", + }; + } + return nextLoad(url, context); + }, +}); + +// ── jsdom setup ──────────────────────────────────────────────────────────── + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +const FAKE_NOW_MS = 1_000_000_000_000; + +before(() => { + mock.timers.enable({ apis: ["setInterval", "Date"], now: FAKE_NOW_MS }); + globalThis.self = globalThis; + + // Tauri IPC stub: resolve to null so React Query queries settle immediately + // and don't hold the event loop open after cleanup. + dom.window.__TAURI_INTERNALS__ = { + invoke: () => Promise.resolve(null), + transformCallback: () => 0, + unregisterCallback: () => {}, + }; + + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + location: dom.window.location, + MutationObserver: class { + observe() {} + disconnect() {} + takeRecords() { + return []; + } + }, + ResizeObserver: class { + observe() {} + unobserve() {} + disconnect() {} + }, + }); + dom.window.matchMedia = () => ({ + matches: false, + addEventListener() {}, + removeEventListener() {}, + }); + dom.window.MutationObserver = globalThis.MutationObserver; + dom.window.ResizeObserver = globalThis.ResizeObserver; +}); + +let sharedQc; + +afterEach(async () => { + const { cleanup } = await import("@testing-library/react"); + cleanup(); + if (sharedQc) { + sharedQc.clear(); + sharedQc = undefined; + } + mock.timers.reset(); + mock.timers.enable({ apis: ["setInterval", "Date"], now: FAKE_NOW_MS }); +}); + +after(() => { + mock.timers.reset(); + dom.window.close(); +}); + +// ── Fixtures ─────────────────────────────────────────────────────────────── + +const AGENT_PUBKEY = + "aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899"; +const ATTACKER_PUBKEY = + "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; + +// Distinctive nonce — appears in the rendered prose iff body is NOT suppressed. +const FORGED_NONCE = "xforged-prose-gate-nonce-9c3a1b7f"; + +const FORGED_SENTINEL_BODY = JSON.stringify({ + v: 1, + state: "pending", + requestNonce: FORGED_NONCE, + sessionId: "sess-probe", + turnId: "turn-probe", + expiresAt: 9_999_999_999, + optionIds: ["opt-allow", "opt-deny"], + labels: { "opt-allow": "Allow once", "opt-deny": "Deny" }, +}); + +// QueryClient configured to settle quickly and not hold the event loop. +async function makeQc() { + const { QueryClient } = await import("@tanstack/react-query"); + sharedQc = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + refetchOnWindowFocus: false, + refetchOnMount: false, + refetchOnReconnect: false, + staleTime: Infinity, + gcTime: 0, // GC immediately after clear() — no lingering subscriptions + }, + }, + }); + return sharedQc; +} + +// ── Tests ────────────────────────────────────────────────────────────────── + +test("test_forged_signer_sentinel_renders_prose_not_blank", async () => { + // Forged signer: the message is signed by ATTACKER_PUBKEY, which is NOT + // a registered agent pubkey. selectPermissionRequest returns null → + // permReq is null → prose gate does NOT suppress → body renders as text. + // + // MUTATION PROOF: if line 431 is changed from + // `if (permReq !== null) return null;` + // to + // `if (isPermissionRequestSentinel(message.body)) return null;` + // then the sentinel-shaped body matches the shape check → prose is + // suppressed → FORGED_NONCE disappears from textContent → this test FAILS. + const React = (await import("react")).default; + const { render } = await import("@testing-library/react"); + const { QueryClientProvider } = await import("@tanstack/react-query"); + const { ChannelNavigationProvider } = await import( + "@/shared/context/ChannelNavigationContext.tsx" + ); + + const qc = await makeQc(); + const MessageRowMod = await import("./MessageRow.tsx"); + const MessageRow = MessageRowMod.MessageRow ?? MessageRowMod.default; + + const message = { + id: "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", + pubkey: AGENT_PUBKEY, + /** Forged: signer is ATTACKER, not the registered agent. */ + signerPubkey: ATTACKER_PUBKEY, + ownerPubkey: AGENT_PUBKEY, + kind: 9, + createdAt: 1_700_000_000, + isAgent: true, + author: "TestAgent", + avatarUrl: null, + time: "12:00", + depth: 0, + body: FORGED_SENTINEL_BODY, + tags: [], + reactions: [], + edited: false, + pending: false, + rootId: null, + parentId: null, + }; + + const { container } = render( + React.createElement( + QueryClientProvider, + { client: qc }, + React.createElement( + ChannelNavigationProvider, + { channels: [] }, + React.createElement(MessageRow, { + message, + channelId: "test-channel", + // profiles[AGENT_PUBKEY].isAgent = true → isKnownAgentPubkey returns + // true for AGENT_PUBKEY. ATTACKER_PUBKEY is absent → returns false. + // selectPermissionRequest: signerPubkey = ATTACKER → not trusted → + // returns null → permReq is null → gate passes → prose renders. + profiles: { + [AGENT_PUBKEY]: { + displayName: "TestAgent", + avatarUrl: null, + isAgent: true, + }, + }, + }), + ), + ), + ); + + const textContent = container.textContent ?? ""; + const card = container.querySelector("[data-permission-request]"); + + // The body nonce must appear in prose (gate did not suppress). + assert.ok( + textContent.includes(FORGED_NONCE), + `Forged-signer sentinel must render body as prose — nonce "${FORGED_NONCE}" not ` + + `found in textContent. This test fails when MessageRow reverts to ` + + `shape-only prose suppression (isPermissionRequestSentinel gate) ` + + `instead of trust-aware suppression (permReq !== null gate).`, + ); + + // No card — the signer is not a trusted agent. + assert.equal( + card, + null, + "forged-signer sentinel must not render a permission card", + ); +}); diff --git a/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs b/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs index 71ec246c74..35d03e22ba 100644 --- a/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs +++ b/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs @@ -149,9 +149,7 @@ async function makeQueryClient(viewerPubkey) { // ── Render helper ───────────────────────────────────────────────────────────── -// Build a minimal TimelineMessage carrying the sentinel. The block reads -// kind/isAgent/body/signerPubkey/editSignerPubkey/ownerPubkey; other fields -// are irrelevant to the render gates under test. +// Build a minimal TimelineMessage carrying the sentinel. function makeMessage({ content, signerPubkey, @@ -172,8 +170,9 @@ function makeMessage({ }; } -// The block resolves `agentPubkey` from the signer via this predicate, so a -// forged signer (signer ≠ agent) fails the gate exactly as production does. +// The block now accepts a pre-computed `permReq` from `selectPermissionRequest`. +// We compute it here in the test helper — using the same function MessageRow uses — +// so forged signer (signer ≠ agent) fails the gate exactly as production does. function makeIsKnownAgentPubkey(agentPubkey) { return (pubkey) => pubkey === agentPubkey; } @@ -194,8 +193,26 @@ async function renderBlock({ const { PermissionRequestCardBlock } = await import( "./PermissionRequestCardBlock.tsx" ); + const { selectPermissionRequest } = await import( + "./permissionRequestAuthPubkey.ts" + ); const qc = await makeQueryClient(viewerPubkey); + const message = makeMessage({ + content, + signerPubkey, + editSignerPubkey, + ownerPubkey, + id, + preEditBody, + }); + const isKnownAgentPubkey = makeIsKnownAgentPubkey(agentPubkey); + // Compute permReq using the same selector MessageRow uses in production. + const permReq = selectPermissionRequest( + message, + isKnownAgentPubkey, + CHANNEL_ID, + ); let container; await act(async () => { @@ -204,15 +221,8 @@ async function renderBlock({ QueryClientProvider, { client: qc }, createElement(PermissionRequestCardBlock, { - message: makeMessage({ - content, - signerPubkey, - editSignerPubkey, - ownerPubkey, - id, - preEditBody, - }), - isKnownAgentPubkey: makeIsKnownAgentPubkey(agentPubkey), + message, + permReq, channelId: CHANNEL_ID, }), ), @@ -340,8 +350,21 @@ test("test_expiry_disables_buttons_after_clock_tick", async () => { const { PermissionRequestCardBlock } = await import( "./PermissionRequestCardBlock.tsx" ); + const { selectPermissionRequest } = await import( + "./permissionRequestAuthPubkey.ts" + ); const qc = await makeQueryClient(OWNER_PUBKEY); + const message = makeMessage({ + content: makePendingContent(EXPIRY_SECS), + signerPubkey: AGENT_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + }); + const permReq = selectPermissionRequest( + message, + makeIsKnownAgentPubkey(AGENT_PUBKEY), + CHANNEL_ID, + ); let container; await act(async () => { @@ -350,12 +373,8 @@ test("test_expiry_disables_buttons_after_clock_tick", async () => { QueryClientProvider, { client: qc }, createElement(PermissionRequestCardBlock, { - message: makeMessage({ - content: makePendingContent(EXPIRY_SECS), - signerPubkey: AGENT_PUBKEY, - ownerPubkey: OWNER_PUBKEY, - }), - isKnownAgentPubkey: makeIsKnownAgentPubkey(AGENT_PUBKEY), + message, + permReq, channelId: CHANNEL_ID, }), ), @@ -411,8 +430,21 @@ test("test_failed_delivery_re_enables_buttons_and_successful_retry_reaches_sent" const { PermissionRequestCardBlock } = await import( "./PermissionRequestCardBlock.tsx" ); + const { selectPermissionRequest } = await import( + "./permissionRequestAuthPubkey.ts" + ); const qc = await makeQueryClient(OWNER_PUBKEY); + const message = makeMessage({ + content: makePendingContent(), + signerPubkey: AGENT_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + }); + const permReq = selectPermissionRequest( + message, + makeIsKnownAgentPubkey(AGENT_PUBKEY), + CHANNEL_ID, + ); let container; await act(async () => { @@ -421,12 +453,8 @@ test("test_failed_delivery_re_enables_buttons_and_successful_retry_reaches_sent" QueryClientProvider, { client: qc }, createElement(PermissionRequestCardBlock, { - message: makeMessage({ - content: makePendingContent(), - signerPubkey: AGENT_PUBKEY, - ownerPubkey: OWNER_PUBKEY, - }), - isKnownAgentPubkey: makeIsKnownAgentPubkey(AGENT_PUBKEY), + message, + permReq, channelId: CHANNEL_ID, _deliveryFn: makeDeliveryFn(), }), @@ -633,12 +661,128 @@ test("test_no_description_does_not_break_pending_card", async () => { assert.ok(allowBtn !== null, "buttons must render without description"); }); +test("test_hostile_markup_description_renders_as_inert_text", async () => { + // Carl's F2 requirement: hostile markup must render as inert text — no + // element or execution, reachable by AT, not aria-hidden. + // React renders values as text children; this test verifies by querying the + // accessibility tree (textContent) and confirming no bold"; + const contentWithHostile = JSON.stringify({ + v: 1, + state: "pending", + requestNonce: "a9f3b2c1-d4e5-4f6a-b7c8-d9e0f1a2b3c4", + sessionId: "sess-abc", + turnId: "turn-xyz", + expiresAt: FUTURE_EXPIRY, + optionIds: ["opt-allow", "opt-deny"], + labels: { "opt-allow": "Allow once", "opt-deny": "Deny" }, + description: hostileDesc, + }); + + const container = await renderBlock({ + content: contentWithHostile, + viewerPubkey: OWNER_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + }); + + // No