From a16b059836c219834055c8d6842d7a62f3f0d30c Mon Sep 17 00:00:00 2001 From: Hayt <41ea58f1e64c243627e8acde7c89be667052ee6e17d8f021c1195be4324ebf04@buzz.block.builderlab.xyz> Date: Sat, 8 Aug 2026 15:55:30 -0400 Subject: [PATCH] fix(desktop): parse bare-JSON sentinel, unify expiry clock, tighten guards, add component tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - extractPermissionRequest: replace fence parser with JSON.parse; sentinel is bare JSON object with v:1 (no code-fence wrapper) - isPermissionRequestSentinel replaces stripPermissionRequestSentinel; MessageRow suppresses markdown body for agent sentinel messages - PendingPermissionRequestCard owns single nowSecs tick; ExpiryCountdown and PermissionButtons share same clock — split-state expiry bug eliminated; click handler rechecks expiry at call time - Type guards: outcome narrowed to union literal; expiresAt must be non-negative integer; originalEventId validated as 64-char lowercase hex; chosenOptionId non-null iff outcome===applied; every advertised optionId must have a label entry - PermissionRequestCardBlock.test.mjs: 7 component tests (owner/non-owner/forged/ edit-auth/expiry) with jsdom + @testing-library/react + fake timers - permissionRequest.test.mjs: raw JSON fixtures (no wrap helper), 22+ rejection cases per frozen schema invariants, isPermissionRequestSentinel test suite - computePermissionRequest.test.mjs: raw JSON fixtures, non-owner viewer path, replay/archive resolved state, past expiresAt accepted at parse time - All 4631 desktop tests pass; all 6 gates green Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../src/features/messages/ui/MessageRow.tsx | 9 + .../ui/PermissionRequestCardBlock.test.mjs | 349 ++++++++++++++++++ .../lib/computePermissionRequest.test.mjs | 38 +- .../src/shared/lib/permissionRequest.test.mjs | 182 ++++++--- desktop/src/shared/lib/permissionRequest.ts | 121 +++--- .../src/shared/ui/permission-request-card.tsx | 91 +++-- 6 files changed, 640 insertions(+), 150 deletions(-) create mode 100644 desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs diff --git a/desktop/src/features/messages/ui/MessageRow.tsx b/desktop/src/features/messages/ui/MessageRow.tsx index 710fa245e8..f43a3550c6 100644 --- a/desktop/src/features/messages/ui/MessageRow.tsx +++ b/desktop/src/features/messages/ui/MessageRow.tsx @@ -33,6 +33,7 @@ import { getPermissionRequestAgentPubkey } from "@/features/messages/ui/permissi import { PermissionRequestCardBlock } from "@/features/messages/ui/PermissionRequestCardBlock"; import { cn } from "@/shared/lib/cn"; import { normalizePubkey } from "@/shared/lib/pubkey"; +import { isPermissionRequestSentinel } from "@/shared/lib/permissionRequest"; import { UserAvatar } from "@/shared/ui/UserAvatar"; import { useChannelNavigation } from "@/shared/context/ChannelNavigationContext"; import { parseImetaTags } from "@/shared/ui/markdown/parseImeta"; @@ -387,6 +388,14 @@ export const MessageRow = React.memo( ); } + // Suppress prose for permission-request sentinels. The harness + // encodes the sentinel as bare JSON in the event content — the + // PermissionRequestCardBlock below renders the card; there is no + // separate prose to preserve. + if (message.isAgent && isPermissionRequestSentinel(message.body)) { + return null; + } + const reviewRootEventId = videoReviewCommentRootId; const reviewTimecode = reviewRootEventId ? parseVideoReviewTimecode(message.body) diff --git a/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs b/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs new file mode 100644 index 0000000000..429813d424 --- /dev/null +++ b/desktop/src/features/messages/ui/PermissionRequestCardBlock.test.mjs @@ -0,0 +1,349 @@ +/** + * Component-level render tests for `PermissionRequestCardBlock`. + * + * These tests verify the render-time security and behaviour gates: + * - non-owner viewer sees read-only card (no buttons) + * - forged signer (signerPubkey ≠ agentPubkey) renders nothing + * - agent-signed edit resolves the card to non-actionable state + * - owner/attacker-signed edits do NOT resolve the card + * - expiry: buttons disabled after the ticking clock crosses expiresAt + * + * Wire contract: harness signs bare JSON as the kind:9 event content. + */ +import assert from "node:assert/strict"; +import { after, afterEach, before, mock, test } from "node:test"; + +import { JSDOM } from "jsdom"; + +// ── jsdom setup ─────────────────────────────────────────────────────────────── + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +// Use fake timers for all tests: prevents real setIntervals in +// PendingPermissionRequestCard from keeping the event loop alive after unmount. +// All tests use a fixed epoch so `Date.now()` returns a deterministic value. +const FAKE_NOW_MS = 1_000_000_000_000; // far from real time — avoids expiry surprises + +before(() => { + // Enable fake timers before any components load so Date.now() is stable. + mock.timers.enable({ apis: ["setInterval", "Date"], now: FAKE_NOW_MS }); + + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + // smoothCorners.ts requires MutationObserver; ResizeObserver used by + // various attachment components. Provide no-op stubs. + MutationObserver: class { + observe() {} + disconnect() {} + takeRecords() { + return []; + } + }, + ResizeObserver: class { + observe() {} + unobserve() {} + disconnect() {} + }, + }); + dom.window.matchMedia = () => ({ + matches: false, + addEventListener() {}, + removeEventListener() {}, + }); + // smoothCorners.ts attaches a MutationObserver to the document; stub on window too + dom.window.MutationObserver = globalThis.MutationObserver; + dom.window.ResizeObserver = globalThis.ResizeObserver; +}); + +afterEach(async () => { + const { cleanup } = await import("@testing-library/react"); + cleanup(); + if (sharedQc) { + sharedQc.clear(); + sharedQc = undefined; + } + // Drain any pending fake timers from this test before the next one starts. + mock.timers.reset(); + mock.timers.enable({ apis: ["setInterval", "Date"], now: FAKE_NOW_MS }); +}); + +after(async () => { + mock.timers.reset(); + dom.window.close(); +}); + +// ── Fixtures ────────────────────────────────────────────────────────────────── + +const AGENT_PUBKEY = + "aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899"; +const OWNER_PUBKEY = + "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"; +const ATTACKER_PUBKEY = + "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; +const CHANNEL_ID = "test-channel-id"; + +// Unix epoch far in the future — buttons are live under the fake clock +const FUTURE_EXPIRY = Math.floor(FAKE_NOW_MS / 1000) + 9_999_999; +// Unix epoch in the past — buttons expired immediately (prefixed _ = intentionally unused) +const _PAST_EXPIRY = 1; + +function makePendingContent(expiresAt = FUTURE_EXPIRY) { + return JSON.stringify({ + v: 1, + state: "pending", + requestNonce: "a9f3b2c1-d4e5-4f6a-b7c8-d9e0f1a2b3c4", + sessionId: "sess-abc", + turnId: "turn-xyz", + expiresAt, + optionIds: ["opt-allow", "opt-deny"], + labels: { "opt-allow": "Allow once", "opt-deny": "Deny" }, + hasDurableRule: false, + durableRuleNote: null, + }); +} + +function makeResolvedContent() { + return JSON.stringify({ + v: 1, + state: "resolved", + requestNonce: "a9f3b2c1-d4e5-4f6a-b7c8-d9e0f1a2b3c4", + originalEventId: + "deadbeef0001deadbeef0002deadbeef0003deadbeef0004deadbeef0005dead", + sessionId: "sess-abc", + turnId: "turn-xyz", + expiresAt: FUTURE_EXPIRY, + optionIds: ["opt-allow", "opt-deny"], + labels: { "opt-allow": "Allow once", "opt-deny": "Deny" }, + hasDurableRule: false, + durableRuleNote: null, + outcome: "applied", + chosenOptionId: "opt-allow", + }); +} + +// Shared QueryClient — created once, cleared between tests. +// `gcTime: 0` prevents React Query's garbage-collection timer from keeping +// the event loop alive after the test completes. +let sharedQc; + +async function getQueryClient(viewerPubkey) { + const { QueryClient } = await import("@tanstack/react-query"); + if (sharedQc) sharedQc.clear(); + sharedQc = new QueryClient({ + defaultOptions: { + queries: { retry: false, gcTime: 0, staleTime: Infinity }, + }, + }); + sharedQc.setQueryData(["identity"], { pubkey: viewerPubkey }); + return sharedQc; +} + +async function makeQueryClient(viewerPubkey) { + return getQueryClient(viewerPubkey); +} + +// ── Render helper ───────────────────────────────────────────────────────────── + +async function renderBlock({ + content, + signerPubkey = AGENT_PUBKEY, + agentPubkey = AGENT_PUBKEY, + editSignerPubkey = undefined, + ownerPubkey = OWNER_PUBKEY, + viewerPubkey = OWNER_PUBKEY, +}) { + const { createElement, act } = await import("react"); + const { render } = await import("@testing-library/react"); + const { QueryClientProvider } = await import("@tanstack/react-query"); + const { PermissionRequestCardBlock } = await import( + "./PermissionRequestCardBlock.tsx" + ); + + const qc = await makeQueryClient(viewerPubkey); + + let container; + await act(async () => { + ({ container } = render( + createElement( + QueryClientProvider, + { client: qc }, + createElement(PermissionRequestCardBlock, { + content, + interactive: true, + agentPubkey, + signerPubkey, + editSignerPubkey, + ownerPubkey, + channelId: CHANNEL_ID, + }), + ), + )); + }); + + return container; +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +test("test_owner_viewer_sees_action_buttons_on_pending_card", async () => { + const container = await renderBlock({ + content: makePendingContent(), + viewerPubkey: OWNER_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + }); + + const allowBtn = container.querySelector( + '[data-testid="permission-decision-opt-allow"]', + ); + const denyBtn = container.querySelector( + '[data-testid="permission-decision-opt-deny"]', + ); + assert.ok(allowBtn !== null, "owner should see allow button"); + assert.ok(denyBtn !== null, "owner should see deny button"); +}); + +test("test_non_owner_viewer_sees_read_only_card_no_buttons", async () => { + const container = await renderBlock({ + content: makePendingContent(), + viewerPubkey: ATTACKER_PUBKEY, // not the owner + ownerPubkey: OWNER_PUBKEY, + }); + + // Card should render (sentinel parsed and agent matches signer) + const card = container.querySelector("[data-permission-request]"); + assert.ok(card !== null, "card renders for non-owner"); + + // But no action buttons + const btn = container.querySelector('[data-testid^="permission-decision-"]'); + assert.equal(btn, null, "non-owner must not see action buttons"); + + // Read-only indicator text present + assert.ok( + container.textContent?.includes("Waiting for owner approval"), + "non-owner sees waiting message", + ); +}); + +test("test_forged_signer_renders_nothing", async () => { + const container = await renderBlock({ + content: makePendingContent(), + agentPubkey: AGENT_PUBKEY, + signerPubkey: ATTACKER_PUBKEY, // signer ≠ agent → rejected + viewerPubkey: OWNER_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + }); + + const card = container.querySelector("[data-permission-request]"); + assert.equal(card, null, "forged signer must not render any card"); +}); + +test("test_agent_signed_edit_resolves_card_to_non_actionable", async () => { + // kind-40003 edit signed by the original agent → resolved card, no buttons + const container = await renderBlock({ + content: makeResolvedContent(), + agentPubkey: AGENT_PUBKEY, + signerPubkey: AGENT_PUBKEY, + editSignerPubkey: AGENT_PUBKEY, // edit signed by agent ✓ + viewerPubkey: OWNER_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + }); + + const card = container.querySelector("[data-permission-request]"); + assert.ok(card !== null, "resolved card renders"); + + const btn = container.querySelector('[data-testid^="permission-decision-"]'); + assert.equal(btn, null, "resolved card has no action buttons"); + + assert.ok( + container.textContent?.includes("Permission request resolved"), + "resolved label present", + ); +}); + +test("test_owner_signed_edit_does_not_resolve_card", async () => { + // kind-40003 signed by owner, not agent → edit-authenticity gate rejects + const container = await renderBlock({ + content: makeResolvedContent(), + agentPubkey: AGENT_PUBKEY, + signerPubkey: AGENT_PUBKEY, + editSignerPubkey: OWNER_PUBKEY, // edit signed by owner ✗ + viewerPubkey: OWNER_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + }); + + // computePermissionRequest returns null → block renders nothing + const card = container.querySelector("[data-permission-request]"); + assert.equal(card, null, "owner-signed edit must not resolve card"); +}); + +test("test_attacker_signed_edit_does_not_resolve_card", async () => { + const container = await renderBlock({ + content: makeResolvedContent(), + agentPubkey: AGENT_PUBKEY, + signerPubkey: AGENT_PUBKEY, + editSignerPubkey: ATTACKER_PUBKEY, // attacker edit ✗ + viewerPubkey: OWNER_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + }); + + const card = container.querySelector("[data-permission-request]"); + assert.equal(card, null, "attacker-signed edit must not resolve card"); +}); + +test("test_expiry_disables_buttons_after_clock_tick", async () => { + // FAKE_NOW_MS is the current epoch. Set expiry to 1s in the future. + const EXPIRY_SECS = Math.floor(FAKE_NOW_MS / 1000) + 1; + + const { createElement, act } = await import("react"); + const { render } = await import("@testing-library/react"); + const { QueryClientProvider } = await import("@tanstack/react-query"); + const { PermissionRequestCardBlock } = await import( + "./PermissionRequestCardBlock.tsx" + ); + + const qc = await makeQueryClient(OWNER_PUBKEY); + + let container; + await act(async () => { + ({ container } = render( + createElement( + QueryClientProvider, + { client: qc }, + createElement(PermissionRequestCardBlock, { + content: makePendingContent(EXPIRY_SECS), + interactive: true, + agentPubkey: AGENT_PUBKEY, + signerPubkey: AGENT_PUBKEY, + ownerPubkey: OWNER_PUBKEY, + channelId: CHANNEL_ID, + }), + ), + )); + }); + + // Before expiry: buttons must be present + const btnBefore = container.querySelector( + '[data-testid="permission-decision-opt-allow"]', + ); + assert.ok(btnBefore !== null, "buttons present before expiry"); + + // Advance clock by 2 seconds — past the 1s expiry + await act(async () => { + mock.timers.tick(2_000); + }); + + // After expiry: buttons must be gone, timed-out message shown + const btnAfter = container.querySelector( + '[data-testid="permission-decision-opt-allow"]', + ); + assert.equal(btnAfter, null, "buttons absent after expiry tick"); + assert.ok( + container.textContent?.includes("Timed out"), + "timed-out message shown after expiry", + ); +}); diff --git a/desktop/src/shared/lib/computePermissionRequest.test.mjs b/desktop/src/shared/lib/computePermissionRequest.test.mjs index 810582cf7c..8c38fc6e28 100644 --- a/desktop/src/shared/lib/computePermissionRequest.test.mjs +++ b/desktop/src/shared/lib/computePermissionRequest.test.mjs @@ -50,12 +50,10 @@ const RESOLVED_PAYLOAD = { chosenOptionId: "opt-allow", }; -function fence(payload) { - return `\`\`\`buzz:permission-request\n${JSON.stringify(payload)}\n\`\`\``; -} - -function body(payload) { - return `May I?\n\n${fence(payload)}`; +// Wire contract: the harness signs bare JSON as the kind:9 event content. +// computePermissionRequest receives the raw event content string — no fence. +function raw(payload) { + return JSON.stringify(payload); } // ── computePermissionRequest ────────────────────────────────────────────────── @@ -63,7 +61,7 @@ function body(payload) { test("test_not_interactive_returns_null", () => { assert.equal( computePermissionRequest( - body(PENDING_PAYLOAD), + raw(PENDING_PAYLOAD), false, AGENT_PUBKEY, AGENT_PUBKEY, @@ -75,7 +73,7 @@ test("test_not_interactive_returns_null", () => { test("test_missing_agentPubkey_returns_null", () => { assert.equal( computePermissionRequest( - body(PENDING_PAYLOAD), + raw(PENDING_PAYLOAD), true, undefined, AGENT_PUBKEY, @@ -87,7 +85,7 @@ test("test_missing_agentPubkey_returns_null", () => { test("test_missing_signerPubkey_returns_null", () => { assert.equal( computePermissionRequest( - body(PENDING_PAYLOAD), + raw(PENDING_PAYLOAD), true, AGENT_PUBKEY, undefined, @@ -100,7 +98,7 @@ test("test_forged_card_wrong_signer_returns_null", () => { // agentPubkey (channel's known agent) ≠ signerPubkey (event signer) assert.equal( computePermissionRequest( - body(PENDING_PAYLOAD), + raw(PENDING_PAYLOAD), true, AGENT_PUBKEY, ATTACKER_PUBKEY, @@ -111,7 +109,7 @@ test("test_forged_card_wrong_signer_returns_null", () => { test("test_valid_signer_returns_payload", () => { const result = computePermissionRequest( - body(PENDING_PAYLOAD), + raw(PENDING_PAYLOAD), true, AGENT_PUBKEY, AGENT_PUBKEY, @@ -121,7 +119,7 @@ test("test_valid_signer_returns_payload", () => { test("test_signer_check_is_case_insensitive", () => { const result = computePermissionRequest( - body(PENDING_PAYLOAD), + raw(PENDING_PAYLOAD), true, AGENT_PUBKEY.toUpperCase(), AGENT_PUBKEY.toLowerCase(), @@ -143,7 +141,7 @@ test("test_no_sentinel_returns_null", () => { test("test_agent_signed_edit_resolves_card", () => { const result = computePermissionRequest( - body(RESOLVED_PAYLOAD), + raw(RESOLVED_PAYLOAD), true, AGENT_PUBKEY, AGENT_PUBKEY, // original event signer @@ -155,7 +153,7 @@ test("test_agent_signed_edit_resolves_card", () => { test("test_owner_signed_edit_does_not_resolve", () => { assert.equal( computePermissionRequest( - body(RESOLVED_PAYLOAD), + raw(RESOLVED_PAYLOAD), true, AGENT_PUBKEY, AGENT_PUBKEY, @@ -168,7 +166,7 @@ test("test_owner_signed_edit_does_not_resolve", () => { test("test_attacker_signed_edit_does_not_resolve", () => { assert.equal( computePermissionRequest( - body(RESOLVED_PAYLOAD), + raw(RESOLVED_PAYLOAD), true, AGENT_PUBKEY, AGENT_PUBKEY, @@ -184,7 +182,7 @@ test("test_resolved_body_with_no_edit_arrived_parses_body_directly", () => { // return it. This handles the edge case where the edit arrives before we // query the original event. const result = computePermissionRequest( - body(RESOLVED_PAYLOAD), + raw(RESOLVED_PAYLOAD), true, AGENT_PUBKEY, AGENT_PUBKEY, @@ -216,7 +214,7 @@ test("test_non_owner_viewer_gets_payload_but_is_owner_false", () => { // viewerPubkey to ownerPubkey. Verify the payload is returned so the card // renders, then the test documents that a non-owner sees it as read-only. const result = computePermissionRequest( - body(PENDING_PAYLOAD), + raw(PENDING_PAYLOAD), true, AGENT_PUBKEY, AGENT_PUBKEY, @@ -232,7 +230,7 @@ test("test_replay_archive_resolved_state_returns_resolved_payload", () => { // computePermissionRequest must return the resolved payload — the card // renders in non-actionable archived state. const result = computePermissionRequest( - body(RESOLVED_PAYLOAD), + raw(RESOLVED_PAYLOAD), true, AGENT_PUBKEY, AGENT_PUBKEY, @@ -247,7 +245,7 @@ test("test_expiry_field_is_preserved_for_local_disable", () => { // PermissionButtons component can compare it to Date.now() / 1000 and // disable buttons locally when the harness deadline has passed. const result = computePermissionRequest( - body(PENDING_PAYLOAD), + raw(PENDING_PAYLOAD), true, AGENT_PUBKEY, AGENT_PUBKEY, @@ -267,7 +265,7 @@ test("test_past_expiresAt_parsed_without_rejection", () => { // enforced by the component at render time, not at parse time. const expired = { ...PENDING_PAYLOAD, expiresAt: 1 }; // Unix epoch + 1s (past) const result = computePermissionRequest( - body(expired), + raw(expired), true, AGENT_PUBKEY, AGENT_PUBKEY, diff --git a/desktop/src/shared/lib/permissionRequest.test.mjs b/desktop/src/shared/lib/permissionRequest.test.mjs index f6ab54f25c..2697604e11 100644 --- a/desktop/src/shared/lib/permissionRequest.test.mjs +++ b/desktop/src/shared/lib/permissionRequest.test.mjs @@ -2,21 +2,20 @@ * Named test matrix for the `permissionRequest` sentinel parser. * * All fixtures are verbatim from Duncan's frozen schema (event b31c716e). - * Tests cover: parse, reject, and sentinel extraction/stripping. + * Tests cover: parse, reject, and sentinel identification. + * + * Wire contract: the harness signs BARE JSON as the kind:9 event content — + * no fence wrapper. Tests feed raw JSON strings matching that shape exactly. */ import assert from "node:assert/strict"; import { describe, it } from "node:test"; -// ── Import via dynamic import to work with the ESM test runner ──────────────── -// The tests run against the compiled JS (tsc outputs CJS); for the mjs runner -// we use a relative path that resolves after build or through tsx. - const mod = await import("./permissionRequest.js").catch( () => import("./permissionRequest.ts"), ); -const { extractPermissionRequest, stripPermissionRequestSentinel } = mod; +const { extractPermissionRequest, isPermissionRequestSentinel } = mod; -// ── Fixtures (verbatim from event b31c716e) ─────────────────────────────────── +// ── Fixtures (verbatim from event b31c716e — bare JSON as harness emits) ───── const PENDING_NORMAL = { v: 1, @@ -117,17 +116,17 @@ const RESOLVED_REJECTED = { chosenOptionId: null, }; -// ── Helpers ─────────────────────────────────────────────────────────────────── - -function wrap(payload) { - return `Some prose above.\n\n\`\`\`buzz:permission-request\n${JSON.stringify(payload)}\n\`\`\`\n`; +// ── Helper: bare JSON string as the harness emits ───────────────────────────── +// No fence, no prose — this is the exact kind:9 event content string. +function raw(payload) { + return JSON.stringify(payload); } -// ── Parse: happy-path fixtures ──────────────────────────────────────────────── +// ── Parse: happy-path fixtures — raw JSON strings ───────────────────────────── describe("extractPermissionRequest — pending fixtures", () => { it("test_pending_normal_parses_correctly", () => { - const result = extractPermissionRequest(wrap(PENDING_NORMAL)); + const result = extractPermissionRequest(raw(PENDING_NORMAL)); assert.ok(result !== null, "should parse"); assert.equal(result.state, "pending"); assert.equal(result.requestNonce, "a9f3b2c1-d4e5-4f6a-b7c8-d9e0f1a2b3c4"); @@ -148,7 +147,7 @@ describe("extractPermissionRequest — pending fixtures", () => { }); it("test_pending_durable_rule_parses_correctly", () => { - const result = extractPermissionRequest(wrap(PENDING_DURABLE)); + const result = extractPermissionRequest(raw(PENDING_DURABLE)); assert.ok(result !== null, "should parse"); assert.equal(result.state, "pending"); assert.equal(result.hasDurableRule, true); @@ -163,11 +162,18 @@ describe("extractPermissionRequest — pending fixtures", () => { ]); assert.equal(result.labels["opt-allow-always"], "Always allow"); }); + + it("test_pending_with_leading_whitespace_parses_correctly", () => { + // trim() before parse — consistent with how relay may deliver content + const result = extractPermissionRequest(` ${raw(PENDING_NORMAL)}\n`); + assert.ok(result !== null, "should parse with surrounding whitespace"); + assert.equal(result.state, "pending"); + }); }); describe("extractPermissionRequest — resolved fixtures", () => { it("test_resolved_applied_parses_correctly", () => { - const result = extractPermissionRequest(wrap(RESOLVED_APPLIED)); + const result = extractPermissionRequest(raw(RESOLVED_APPLIED)); assert.ok(result !== null, "should parse"); assert.equal(result.state, "resolved"); assert.equal(result.outcome, "applied"); @@ -179,7 +185,7 @@ describe("extractPermissionRequest — resolved fixtures", () => { }); it("test_resolved_timed_out_parses_correctly", () => { - const result = extractPermissionRequest(wrap(RESOLVED_TIMED_OUT)); + const result = extractPermissionRequest(raw(RESOLVED_TIMED_OUT)); assert.ok(result !== null, "should parse"); assert.equal(result.state, "resolved"); assert.equal(result.outcome, "timed_out"); @@ -187,7 +193,7 @@ describe("extractPermissionRequest — resolved fixtures", () => { }); it("test_resolved_cancelled_parses_correctly", () => { - const result = extractPermissionRequest(wrap(RESOLVED_CANCELLED)); + const result = extractPermissionRequest(raw(RESOLVED_CANCELLED)); assert.ok(result !== null, "should parse"); assert.equal(result.state, "resolved"); assert.equal(result.outcome, "cancelled"); @@ -195,7 +201,7 @@ describe("extractPermissionRequest — resolved fixtures", () => { }); it("test_resolved_rejected_parses_correctly", () => { - const result = extractPermissionRequest(wrap(RESOLVED_REJECTED)); + const result = extractPermissionRequest(raw(RESOLVED_REJECTED)); assert.ok(result !== null, "should parse"); assert.equal(result.state, "resolved"); assert.equal(result.outcome, "rejected"); @@ -206,92 +212,160 @@ describe("extractPermissionRequest — resolved fixtures", () => { // ── Parse: rejection cases ──────────────────────────────────────────────────── describe("extractPermissionRequest — rejection cases", () => { - it("test_no_sentinel_returns_null", () => { - assert.equal(extractPermissionRequest("just prose, no fence"), null); + it("test_prose_only_returns_null", () => { + // Ordinary kind:9 message (no sentinel) — must not parse + assert.equal(extractPermissionRequest("just prose, no JSON"), null); }); it("test_wrong_version_returns_null", () => { const bad = { ...PENDING_NORMAL, v: 2 }; - assert.equal(extractPermissionRequest(wrap(bad)), null); + assert.equal(extractPermissionRequest(raw(bad)), null); }); it("test_unknown_state_returns_null", () => { const bad = { ...PENDING_NORMAL, state: "unknown" }; - assert.equal(extractPermissionRequest(wrap(bad)), null); + assert.equal(extractPermissionRequest(raw(bad)), null); }); it("test_empty_optionIds_returns_null", () => { const bad = { ...PENDING_NORMAL, optionIds: [] }; - assert.equal(extractPermissionRequest(wrap(bad)), null); + assert.equal(extractPermissionRequest(raw(bad)), null); }); it("test_too_many_optionIds_returns_null", () => { + const ids = Array.from({ length: 11 }, (_, i) => `opt-${i}`); const bad = { ...PENDING_NORMAL, - optionIds: Array.from({ length: 11 }, (_, i) => `opt-${i}`), - labels: Object.fromEntries( - Array.from({ length: 11 }, (_, i) => [`opt-${i}`, `Option ${i}`]), - ), + optionIds: ids, + labels: Object.fromEntries(ids.map((id) => [id, `Option ${id}`])), }; - assert.equal(extractPermissionRequest(wrap(bad)), null); + assert.equal(extractPermissionRequest(raw(bad)), null); }); it("test_label_exceeding_200_chars_returns_null", () => { - const longLabel = "x".repeat(201); const bad = { ...PENDING_NORMAL, - labels: { "opt-allow": longLabel, "opt-deny": "Deny" }, + labels: { "opt-allow": "x".repeat(201), "opt-deny": "Deny" }, }; - assert.equal(extractPermissionRequest(wrap(bad)), null); + assert.equal(extractPermissionRequest(raw(bad)), null); }); it("test_missing_requestNonce_returns_null", () => { const { requestNonce: _, ...bad } = PENDING_NORMAL; - assert.equal(extractPermissionRequest(wrap(bad)), null); + assert.equal(extractPermissionRequest(raw(bad)), null); + }); + + it("test_fractional_expiresAt_returns_null", () => { + // Frozen schema requires integer seconds + const bad = { ...PENDING_NORMAL, expiresAt: 1786206732.5 }; + assert.equal(extractPermissionRequest(raw(bad)), null); + }); + + it("test_negative_expiresAt_returns_null", () => { + const bad = { ...PENDING_NORMAL, expiresAt: -1 }; + assert.equal(extractPermissionRequest(raw(bad)), null); + }); + + it("test_non_finite_expiresAt_returns_null", () => { + // JSON.stringify converts Infinity to null, so this tests null expiresAt + const bad = { ...PENDING_NORMAL, expiresAt: null }; + assert.equal(extractPermissionRequest(raw(bad)), null); + }); + + it("test_optionId_without_label_returns_null", () => { + // Every advertised optionId must have a label entry + const bad = { + ...PENDING_NORMAL, + optionIds: ["opt-allow", "opt-deny", "opt-extra"], + labels: { "opt-allow": "Allow once", "opt-deny": "Deny" }, + // "opt-extra" has no label + }; + assert.equal(extractPermissionRequest(raw(bad)), null); }); it("test_resolved_missing_originalEventId_returns_null", () => { const { originalEventId: _, ...bad } = RESOLVED_APPLIED; - assert.equal(extractPermissionRequest(wrap(bad)), null); + assert.equal(extractPermissionRequest(raw(bad)), null); }); it("test_resolved_originalEventId_wrong_length_returns_null", () => { const bad = { ...RESOLVED_APPLIED, originalEventId: "tooshort" }; - assert.equal(extractPermissionRequest(wrap(bad)), null); + assert.equal(extractPermissionRequest(raw(bad)), null); + }); + + it("test_resolved_originalEventId_uppercase_returns_null", () => { + // Must be lowercase hex per HEX64_RE + const bad = { + ...RESOLVED_APPLIED, + originalEventId: + "DEADBEEF0001DEADBEEF0002DEADBEEF0003DEADBEEF0004DEADBEEF0005DEAD", + }; + assert.equal(extractPermissionRequest(raw(bad)), null); + }); + + it("test_resolved_unknown_outcome_returns_null", () => { + const bad = { ...RESOLVED_TIMED_OUT, outcome: "expired" }; + assert.equal(extractPermissionRequest(raw(bad)), null); + }); + + it("test_resolved_applied_with_null_chosenOptionId_returns_null", () => { + // outcome === "applied" requires a non-null chosenOptionId + const bad = { ...RESOLVED_APPLIED, chosenOptionId: null }; + assert.equal(extractPermissionRequest(raw(bad)), null); + }); + + it("test_resolved_timed_out_with_nonnull_chosenOptionId_returns_null", () => { + // outcome !== "applied" requires null chosenOptionId + const bad = { ...RESOLVED_TIMED_OUT, chosenOptionId: "opt-allow" }; + assert.equal(extractPermissionRequest(raw(bad)), null); }); it("test_invalid_json_returns_null", () => { - const content = "```buzz:permission-request\n{not valid json}\n```\n"; - assert.equal(extractPermissionRequest(content), null); + assert.equal(extractPermissionRequest("{not valid json}"), null); }); - it("test_empty_fence_body_returns_null", () => { - const content = "```buzz:permission-request\n\n```\n"; - assert.equal(extractPermissionRequest(content), null); + it("test_empty_string_returns_null", () => { + assert.equal(extractPermissionRequest(""), null); }); - it("test_non_finite_expiresAt_returns_null", () => { - const bad = { ...PENDING_NORMAL, expiresAt: Infinity }; - assert.equal(extractPermissionRequest(wrap(bad)), null); + it("test_json_array_returns_null", () => { + // Arrays are not sentinel objects + assert.equal(extractPermissionRequest("[1,2,3]"), null); + }); + + it("test_json_null_returns_null", () => { + assert.equal(extractPermissionRequest("null"), null); + }); + + it("test_json_number_returns_null", () => { + assert.equal(extractPermissionRequest("42"), null); }); }); -// ── stripPermissionRequestSentinel ─────────────────────────────────────────── +// ── isPermissionRequestSentinel ─────────────────────────────────────────────── -describe("stripPermissionRequestSentinel", () => { - it("test_strip_removes_fence_and_preserves_prose", () => { - const content = `Some prose.\n\n\`\`\`buzz:permission-request\n${JSON.stringify(PENDING_NORMAL)}\n\`\`\`\n`; - const stripped = stripPermissionRequestSentinel(content); - assert.ok(!stripped.includes("buzz:permission-request")); - assert.ok(stripped.includes("Some prose.")); +describe("isPermissionRequestSentinel", () => { + it("test_sentinel_pending_returns_true", () => { + assert.equal(isPermissionRequestSentinel(raw(PENDING_NORMAL)), true); }); - it("test_strip_no_sentinel_returns_original", () => { - const content = "just prose here"; - assert.equal(stripPermissionRequestSentinel(content), content); + it("test_sentinel_resolved_returns_true", () => { + assert.equal(isPermissionRequestSentinel(raw(RESOLVED_APPLIED)), true); }); - it("test_strip_empty_string_returns_empty", () => { - assert.equal(stripPermissionRequestSentinel(""), ""); + it("test_prose_message_returns_false", () => { + assert.equal(isPermissionRequestSentinel("Hello world"), false); + }); + + it("test_invalid_json_returns_false", () => { + assert.equal(isPermissionRequestSentinel("{bad json"), false); + }); + + it("test_json_without_v1_returns_false", () => { + // A valid JSON object that is not a sentinel + assert.equal( + isPermissionRequestSentinel('{"type":"normal_message"}'), + false, + ); }); }); diff --git a/desktop/src/shared/lib/permissionRequest.ts b/desktop/src/shared/lib/permissionRequest.ts index e3c3c2ac75..c72bacd513 100644 --- a/desktop/src/shared/lib/permissionRequest.ts +++ b/desktop/src/shared/lib/permissionRequest.ts @@ -1,17 +1,18 @@ /** - * Utilities for extracting and parsing the `buzz:permission-request` sentinel - * that `buzz-acp` publishes as a kind:9 reply into the triggering thread when - * an `ask`-policy permission request is admitted. + * Utilities for extracting and parsing the permission-request sentinel that + * `buzz-acp` publishes as a kind:9 reply into the triggering thread when an + * `ask`-policy permission request is admitted. * * Wire format (versioned discriminated union, schema v1 — frozen at event * b31c716e): * - * ```buzz:permission-request - * {"v":1,"state":"pending", … } - * ``` + * The harness serialises a bare JSON object as the kind:9 event content: * - * The prose above the fence is the plaintext fallback for non-card clients. - * Desktop strips the sentinel and renders a `PermissionRequestCard` instead. + * {"v":1,"state":"pending","requestNonce":"…", …} + * + * Desktop identifies a sentinel by `"v":1` in the top-level JSON object. + * Non-JSON content and JSON objects without `"v":1` are left untouched. + * There is no fenced wire format — non-sentinel kind:9s must NOT be modified. * * Security invariants: * - `agentPubkey` and `channelId` are derived from the SIGNED EVENT ENVELOPE, @@ -71,8 +72,8 @@ export type PermissionRequestResolved = { labels: Record; hasDurableRule: boolean; durableRuleNote: string | null; - /** One of "applied" | "timed_out" | "cancelled" | "rejected". */ - outcome: string; + /** Outcome of the permission request. */ + outcome: "applied" | "timed_out" | "cancelled" | "rejected"; /** Non-null only when outcome === "applied". */ chosenOptionId: string | null; }; @@ -83,67 +84,63 @@ export type PermissionRequestPayload = // ── Constants ───────────────────────────────────────────────────────────────── -const FENCE_OPEN = "```buzz:permission-request"; -const FENCE_CLOSE = "```"; - /** Maximum character length for any untrusted display string in the sentinel. */ const MAX_LABEL_CHARS = 200; /** Maximum number of option IDs in a sentinel (PERMISSION_OPTIONS_MAX). */ const MAX_OPTION_IDS = 10; +/** Regex for a valid 64-character lowercase hex Nostr event ID. */ +const HEX64_RE = /^[0-9a-f]{64}$/; + +/** The four valid outcome strings. */ +const VALID_OUTCOMES = new Set([ + "applied", + "timed_out", + "cancelled", + "rejected", +]); + // ── Extractor ───────────────────────────────────────────────────────────────── /** - * Extract the `PermissionRequestPayload` from a message body, if present. + * Extract the `PermissionRequestPayload` from a kind:9 event content string, + * if present. + * + * The harness signs bare JSON as the event content — no fence wrapper. Desktop + * identifies sentinels by `"v":1` at the top level. Non-JSON content and JSON + * objects that do not carry `"v":1` are returned as `null`; `MessageRow` renders + * them as ordinary markdown. * * Returns `null` when: - * - the sentinel fence is absent - * - the JSON inside is malformed - * - the parsed value does not match the expected shape + * - the content is not valid JSON + * - the parsed value is not a sentinel object (missing `v:1`) + * - the parsed value does not match the expected shape or invariants * * Never throws — all errors are swallowed so this is safe in the render path. */ export function extractPermissionRequest( content: string, ): PermissionRequestPayload | null { - const openIdx = content.indexOf(FENCE_OPEN); - if (openIdx === -1) return null; - - const jsonStart = content.indexOf("\n", openIdx); - if (jsonStart === -1) return null; - - const closeIdx = content.indexOf(`\n${FENCE_CLOSE}`, jsonStart); - if (closeIdx === -1) return null; - - const json = content.slice(jsonStart + 1, closeIdx).trim(); - if (!json) return null; - + let parsed: unknown; try { - const parsed: unknown = JSON.parse(json); - return isPermissionRequestPayload(parsed) ? parsed : null; + parsed = JSON.parse(content.trim()); } catch { return null; } + return isPermissionRequestPayload(parsed) ? parsed : null; } /** - * Strip the `buzz:permission-request` sentinel block (and any preceding blank - * line) from a message body. Returns the original string unchanged when no - * sentinel is present. + * Returns `true` when the kind:9 content is a permission-request sentinel. + * Used by `MessageRow` to decide whether to suppress markdown rendering. * - * Used so the prose fallback renders without the raw code block. + * When `extractPermissionRequest` returns a non-null value the content IS the + * sentinel; the entire string is consumed by the card. Non-sentinel kind:9s are + * rendered as ordinary markdown, unchanged. */ -export function stripPermissionRequestSentinel(content: string): string { - const openIdx = content.indexOf(FENCE_OPEN); - if (openIdx === -1) return content; - - const closeIdx = content.indexOf(`\n${FENCE_CLOSE}`, openIdx); - if (closeIdx === -1) return content; - - const afterFence = closeIdx + `\n${FENCE_CLOSE}`.length; - const prose = content.slice(0, openIdx).replace(/\n{2,}$/, "\n"); - return prose + content.slice(afterFence); +export function isPermissionRequestSentinel(content: string): boolean { + return extractPermissionRequest(content) !== null; } // ── Type guards ──────────────────────────────────────────────────────────────── @@ -162,7 +159,7 @@ function isLabelsRecord(v: unknown): v is Record { } function isPermissionRequestPayload(v: unknown): v is PermissionRequestPayload { - if (typeof v !== "object" || v === null) return false; + if (typeof v !== "object" || v === null || Array.isArray(v)) return false; const p = v as Record; if (p.v !== 1) return false; @@ -172,7 +169,13 @@ function isPermissionRequestPayload(v: unknown): v is PermissionRequestPayload { } if (!isNullableString(p.sessionId)) return false; if (!isNullableString(p.turnId)) return false; - if (typeof p.expiresAt !== "number" || !Number.isFinite(p.expiresAt)) { + // expiresAt must be an integer (no fractional seconds, no negative values) + if ( + typeof p.expiresAt !== "number" || + !Number.isFinite(p.expiresAt) || + !Number.isInteger(p.expiresAt) || + p.expiresAt < 0 + ) { return false; } if ( @@ -184,6 +187,14 @@ function isPermissionRequestPayload(v: unknown): v is PermissionRequestPayload { return false; } if (!isLabelsRecord(p.labels)) return false; + // Every advertised optionId must have a label entry + if ( + !(p.optionIds as string[]).every( + (id) => typeof (p.labels as Record)[id] === "string", + ) + ) { + return false; + } if (typeof p.hasDurableRule !== "boolean") return false; if (!isNullableString(p.durableRuleNote)) return false; @@ -192,17 +203,21 @@ function isPermissionRequestPayload(v: unknown): v is PermissionRequestPayload { } if (p.state === "resolved") { - // originalEventId: 64-char hex string + // originalEventId: 64-char lowercase hex string if ( typeof p.originalEventId !== "string" || - p.originalEventId.length !== 64 + !HEX64_RE.test(p.originalEventId) ) { return false; } - if (typeof p.outcome !== "string" || p.outcome.length === 0) return false; - // chosenOptionId: string or null (non-null only on "applied") - if (p.chosenOptionId !== null && typeof p.chosenOptionId !== "string") { - return false; + // outcome: exactly one of the four literals + if (!VALID_OUTCOMES.has(p.outcome as string)) return false; + // chosenOptionId: non-null ⟺ outcome === "applied" + if (p.outcome === "applied") { + if (typeof p.chosenOptionId !== "string" || p.chosenOptionId.length === 0) + return false; + } else { + if (p.chosenOptionId !== null) return false; } return true; } diff --git a/desktop/src/shared/ui/permission-request-card.tsx b/desktop/src/shared/ui/permission-request-card.tsx index 0564928698..1cb94792de 100644 --- a/desktop/src/shared/ui/permission-request-card.tsx +++ b/desktop/src/shared/ui/permission-request-card.tsx @@ -91,15 +91,17 @@ function PermissionButtons({ agentPubkey, channelId, request, + nowSecs, }: { agentPubkey: string; channelId: string; request: PermissionRequestPending; + /** Current time in seconds (driven by a parent ticking state). */ + nowSecs: number; }) { const [submitted, setSubmitted] = React.useState(null); - const now = Date.now() / 1000; - const expired = request.expiresAt <= now; + const expired = request.expiresAt <= nowSecs; if (expired) { return ( @@ -125,6 +127,9 @@ function PermissionButtons({ className={buttonClass(isDenyLabel(label))} data-testid={`permission-decision-${optionId}`} onClick={() => { + // Recheck expiry at click time — prevents submitting a + // decision on a card that expired between renders. + if (request.expiresAt <= Date.now() / 1000) return; setSubmitted(optionId); void sendPermissionDecision( agentPubkey, @@ -152,23 +157,18 @@ function PermissionButtons({ } /** - * Countdown display for a pending card. Updates every second until expiry. - * Returns null when already expired (buttons handle that state). + * Countdown display for a pending card. Updates the shared `now` state + * every second until expiry so that both the countdown and the button + * actionability are driven by the same tick. */ -function ExpiryCountdown({ expiresAt }: { expiresAt: number }) { - const [secsLeft, setSecsLeft] = React.useState(() => - Math.max(0, Math.round(expiresAt - Date.now() / 1000)), - ); - - React.useEffect(() => { - if (secsLeft <= 0) return; - const id = setInterval(() => { - const remaining = Math.max(0, Math.round(expiresAt - Date.now() / 1000)); - setSecsLeft(remaining); - if (remaining <= 0) clearInterval(id); - }, 1000); - return () => clearInterval(id); - }, [expiresAt, secsLeft]); +function ExpiryCountdown({ + expiresAt, + nowSecs, +}: { + expiresAt: number; + nowSecs: number; +}) { + const secsLeft = Math.max(0, Math.round(expiresAt - nowSecs)); if (secsLeft <= 0) return null; const mins = Math.floor(secsLeft / 60); @@ -219,9 +219,51 @@ export function PermissionRequestCard({ ); } - // Pending state - const pending = request as PermissionRequestPending; - const expired = pending.expiresAt <= Date.now() / 1000; + // Pending state — one ticking `nowSecs` drives both the countdown display + // and button actionability so expiry is observed atomically. + return ( + + ); +} + +/** + * Pending-state card. Owns the ticking `nowSecs` state so that + * `ExpiryCountdown` and `PermissionButtons` always see the same clock value. + */ +function PendingPermissionRequestCard({ + className, + request, + agentPubkey, + channelId, + isOwner, +}: { + className?: string; + request: PermissionRequestPending; + agentPubkey: string; + channelId: string; + isOwner?: boolean; +}) { + const [nowSecs, setNowSecs] = React.useState(() => Date.now() / 1000); + + React.useEffect(() => { + const id = setInterval(() => { + const now = Date.now() / 1000; + setNowSecs(now); + if (now >= request.expiresAt) clearInterval(id); + }, 1000); + // In Node test environments (not browsers), intervals can keep the process + // alive. Call unref() when available to allow clean test exits. + (id as unknown as { unref?: () => void }).unref?.(); + return () => clearInterval(id); + }, [request.expiresAt]); + + const expired = request.expiresAt <= nowSecs; return ( Permission request - {!expired ? : null} + {!expired ? ( + + ) : null} {isOwner ? ( ) : (