feat(desktop): preserve mentions across copy and paste - #7228
Conversation
🔐 Codex Security Review
|
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — request changes on d89dbf3d5fcb95f5282fd0c927f2a62888badebf.
Blocking defect
P2 — visible foreign clipboard HTML can forge an exact mention identity.
The composer claims any HTML containing data-mention (desktop/src/features/messages/ui/useComposerPasteHandler.ts:65-76; desktop/src/features/messages/lib/normalizeMentionClipboard.ts:11-13). parseMentionClipboardRecords then accepts an arbitrary bounded label plus any syntactically valid 64-hex pubkey, while selectVisibleMentionIdentities checks only that the inserted text visibly contains @<label> (desktop/src/features/messages/lib/mentionClipboard.ts:276-332). That pair is registered before insertion (desktop/src/features/messages/lib/mentionClipboardPaste.ts:59-79) and becomes authoritative for send-time mention extraction.
A foreign page can therefore place this shape on the clipboard:
<span data-mention
data-mention-pubkey="<attacker 64-hex pubkey>"
data-mention-label="John Smith">@John Smith</span>The user sees the expected @John Smith label/chip, but Buzz can emit the attacker's pubkey in the outbound p tag. Requiring the claimed label to be visible prevents hidden binding, but does not authenticate the visible label-to-pubkey binding. Existing adversarial coverage exercises the hidden-record case, not this visible forgery.
Author action: fail closed on identity recovery unless Buzz independently validates/authenticates the label↔pubkey binding or verifies trustworthy provenance. Forged/foreign visible HTML must paste readable text without registering or emitting the claimed identity. Add a production-seam regression using the visible hostile shape above, asserting no impostor binding/outbound p tag while preserving genuine Buzz round trips.
Reconciled evidence
The product/UI lane found no separate defect: readable external plain text, timeline/thread/forum/action-bar and composer wiring, multi-word labels, partial selections, and hidden-clone accessibility semantics are covered. The systems/integration lane found the trust-boundary defect above and ran the full desktop unit package at this exact head: 5,927 passed, 0 failed; git diff --check also passed. I independently traced the clipboard handler → record parser → registration path and agree the visible forgery crosses the outbound identity boundary.
Confidence gaps (not additional author blockers): no reviewer-owned native OS clipboard round trip was completed. CI had macOS/Windows builds and desktop integration lanes passing when checked; one smoke shard was red and remaining jobs were still settling, so required gates must independently reach their required terminal state.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Reviewed: 1c8321cd08feb597f8bcff5195c21148fb3e98ed..d89dbf3d5fcb95f5282fd0c927f2a62888badebf (exact head d89dbf3d5fcb95f5282fd0c927f2a62888badebf)
Risk: high — untrusted clipboard HTML is being promoted into cryptographic mention identity and ultimately outbound Nostr p tags.
Behavior/contracts traced: timeline/thread/forum/action-bar/composer copy and cut; external text/plain; rich composer paste; clipboard parsing/normalization; mention-map registration; chip decoration; send-time mention extraction. The product/UI lane found the requested surfaces wired, readable external output, bounded records, whole/partial-chip handling, and no new accessibility-semantic owner. The integrated trust-boundary trace found one blocking identity-forgery path.
Blocking — visible foreign HTML can forge the identity behind a mention
useComposerPasteHandler.ts:65-76 routes any HTML containing mention-like attributes into the rich path. mentionClipboard.ts:276-305 accepts a bounded label paired with any syntactically valid 64-hex pubkey, while selectVisibleMentionIdentities at :326-332 proves only that the inserted text contains @<label>. mentionClipboardPaste.ts:59-79 registers that pair before insertion; useMentions.ts:521-532 stores it as selected mention identity; and extractMentionPubkeys.ts:52-53,69-88 prefers selected-map identities when producing outbound tags.
Therefore foreign clipboard HTML such as:
<span data-mention data-mention-pubkey="<attacker 64hex>"
data-mention-label="John Smith">@John Smith</span>pastes a visually plausible @John Smith while authoritatively binding it to the attacker key. Visibility prevents hidden smuggling, but it does not authenticate label ↔ pubkey ownership or Buzz provenance. The existing adversarial E2E at desktop/tests/e2e/mention-clipboard.spec.ts:359-430 covers empty/ignored hidden records, not this visible forged pair.
Consequence: a user can paste what visibly looks like a known person and send a message whose p tag targets a different key. In an identity-native messenger, readable labels are presentation; they cannot vouch for cryptographic identity.
Author action: fail closed on identity recovery unless the label/pubkey pair is independently validated against trusted Buzz state (or another provenance mechanism whose authenticity is actually verified). Foreign/forged visible HTML must remain readable text without registering the claimed pair or emitting its p tag. Add a production-seam regression using the visible hostile shape above, asserting no impostor chip/binding/outbound p tag while retaining genuine Buzz round trips.
Verification owner: author for the patch and biting regression; reviewer for exact-head re-review.
Validation at matching HEAD:
- PASS —
git diff --check 1c8321cd08feb597f8bcff5195c21148fb3e98ed..HEAD. - PASS — full Desktop unit package run reported 5927 passed, 0 failed; post-run HEAD matched and the worktree was clean.
- PASS — independent source trace at exact clean head confirmed the untrusted HTML → registration → selected-map → outbound-tag path above.
- CI snapshot: required checks reported by
gh pr checks --requiredwere passing/skipped as configured. One non-requiredDesktop Smoke E2E (3)shard failed whileDesktop Coreremained in progress; this is additional unresolved CI evidence, not the basis for this request-changes verdict.
Manual/native evidence: no real OS clipboard Cmd/Ctrl+C/X/V round trip was independently exercised in this review.
Residual risk: native clipboard flavor behavior remains reviewer/tooling-owned verification after the code defect is corrected. Any new head invalidates this verdict until its delta and exact-head gates are reviewed.
— :bot: Jude’s code review agent
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Reviewed: 1c8321cd08feb597f8bcff5195c21148fb3e98ed..f8a8f55570762a559f939687abf40518cc0ec0bb (exact head f8a8f55570762a559f939687abf40518cc0ec0bb)
Risk: high — clipboard identity verification now crosses an asynchronous relay boundary, but message send and subsequent mention choices are synchronous user actions.
Behavior/contracts traced: foreign-visible HTML verification; local/profile alias trust; timeline/thread/forum/inbox/action-bar/composer copy; paste insertion; asynchronous verification settlement; mention-map ownership; immediate send; same-label replacement; outbound Nostr p extraction. The prior visible-forgery defect is addressed for unvouched/failed lookups, and the product/UI lane found the inbox copy surfaces wired without a separate accessibility-semantic defect. One lifecycle defect remains.
Blocking P2 — detached verification can miss the send or overwrite newer same-label intent
mentionClipboardPaste.ts:63-77,129-140 inserts the visible mention and launches verification as a detached void ...then(...). Send does not await or otherwise own that promise: it synchronously reads mention identities (useMentions.ts:701-714; selected-map extraction in extractMentionPubkeys.ts:52-88). A genuine Buzz mention pasted and immediately sent can therefore leave before verification registers its identity, silently omitting the intended p tag.
Settlement is fenced only by whether the whole composer still contains @<label> (mentionClipboardPaste.ts:30-34,68-75), not by the pasted occurrence, a generation, or the binding that was current when verification began. Reordering two same-label operations demonstrates the worse failure:
- Paste trusted
@Alex→ key A; verification is slow. - Remove/replace it, then explicitly choose or paste trusted
@Alex→ key B; B resolves first. - A resolves later, sees some
@Alexstill visible, andregisterMentionPubkeyunconditionally overwrites the name entry (useMentions.ts:522-532). - The chip remains visually identical while send can emit key A rather than the user's newer B choice.
Deleting the original paste and later hand-typing the same label has the same stale-settlement problem. Existing verifier tests exercise trust answers/cache freshness, while the genuine and hostile clipboard E2E cases wait for chip/lookup settlement before send; no deferred/reordered completion test protects this boundary (useVerifyMentionIdentities.test.mjs:109-276; mention-clipboard.spec.ts:562-599,676-686,706-723,778-794).
Consequence: fast send can silently fail to notify the copied recipient; stale verification can silently retarget a visibly unchanged mention to an older identity.
Author action: make recovery transactionally owned by the pasted occurrence and send state. Fence completion by paste generation/range (not label presence), never let an older same-label completion overwrite newer explicit/pasted intent, and either await pending verification in send or truthfully block/defer send until its result is incorporated. Add deterministic production-seam regressions for: (1) delayed lookup → immediate send, (2) A-slow/B-fast same-label replacement, and (3) delete A then hand-type the same label before A resolves. Assert exact outbound p tags and visible state while preserving fail-closed degraded behavior.
Verification owner: author for patch and causal regressions; reviewer for exact-head lifecycle re-review.
Validation at matching HEAD:
- PASS — both independent review lanes traced and reproduced the ownership gap from source at exact clean head.
- PASS —
git diff --check 1c8321cd08feb597f8bcff5195c21148fb3e98ed..HEAD. - PASS — current CI rollup is green for Desktop Core, all four Desktop Smoke E2E shards, Desktop E2E integration, macOS/Windows builds, and required checks.
- PASS — prior full Desktop package evidence at the earlier head was 5927/5927; the new head's package gates are represented by green exact-head CI, but reviewers did not independently rerun the full package locally.
Manual/native evidence: real OS clipboard Cmd/Ctrl+C/X/V was not independently exercised for this head.
Residual risk: native clipboard flavor behavior remains reviewer/tooling-owned after the author fixes the lifecycle defect. Any new head invalidates this verdict until its delta is reviewed.
— :bot: Jude’s code review agent
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Blocking finding
P2 — asynchronous mention verification is not owned by either the pasted occurrence or the send transaction.
mentionClipboardPaste.ts inserts the visible label immediately and launches verification through a detached promise (desktop/src/features/messages/lib/mentionClipboardPaste.ts:63-77,129-140). A send can therefore extract mention identities before a genuine copied mention has been verified, silently publishing the readable @Label without its intended p tag (desktop/src/features/messages/lib/useMentions.ts:701-714; desktop/src/features/messages/lib/extractMentionPubkeys.ts:52-88).
The completion fence checks only whether the same label remains anywhere in the composer (mentionClipboardPaste.ts:68-75), not whether the originating paste occurrence or identity choice is still current. With duplicate display names, paste A (slow), replace it with same-label B (fast), then let A resolve: registerMentionPubkey unconditionally overwrites the display-name entry (desktop/src/features/messages/lib/useMentions.ts:522-532), so the UI can still show the expected label while send emits A. Removing the original paste and hand-typing the same label has the same stale-settlement problem.
The added verifier tests establish trust answers/cache behavior, while the clipboard E2E waits for lookup completion before sending; neither covers reordered completion or immediate send (desktop/src/features/messages/lib/useVerifyMentionIdentities.test.mjs:109-276; desktop/tests/e2e/mention-clipboard.spec.ts:562-599).
Author action: make recovered identity state generation/range-owned by the pasted occurrence; prevent older same-label verification from overwriting newer explicit or pasted intent; and include pending verification in send readiness (await it or truthfully defer/block send). Add deterministic production-seam regressions for delayed verification plus immediate send, slow A/fast B with the same label, and delete-then-hand-type before A resolves, asserting exact outbound p tags.
The prior forged-visible-HTML issue is otherwise addressed: unverified/failed pairs bind nothing, and identity is checked against trusted community/profile aliases. Inbox copy coverage is also wired. All current GitHub checks are green or intentionally skipped. Native OS clipboard exercise remains a reviewer/tooling-owned confidence gap, not an additional author defect.
Reviewed exact head f8a8f55570762a559f939687abf40518cc0ec0bb against base 1c8321cd08feb597f8bcff5195c21148fb3e98ed. Both independent review lanes reached the same blocking conclusion; git diff --check passed at the exact clean head.
Copying a message out of the timeline lost the mention: the rendered chip drops the `@` for display, so the clipboard carried "John Smith" — two ordinary words that no composer could bind back to a pubkey. Pasting into another channel produced dead text, and sending it tagged nobody. Every Buzz copy now writes two clipboard flavors in one transaction: - `text/plain` — readable anywhere, sigils restored, no pubkeys. This is what TextEdit, Slack, and every other external app receive. - `text/html` — the same content with each mention wrapped in a span carrying `data-mention-pubkey` / `-label` / `-kind`. On paste, the composer harvests those records, registers each `name → pubkey` with the existing mention machinery, and inserts the content: the chip re-lights and the send path recovers the identity the author tagged. A marker attribute records what the plain flavor holds, so a Markdown copy pastes through the text pipeline and a rendered copy through the HTML one. Covered surfaces: timeline selection copy, thread-panel selection copy, "Copy message", and composer copy/cut — plus paste in both the channel and forum composers. Notes: - Clipboard HTML is untrusted. Records are capped (50), labels bounded (200 chars), and a pubkey must be 64 hex before it can become a `p` tag. - A partially selected chip drops its identity attributes and gains no sigil, so paste can never register a truncated name against a real key. - Mention matching reuses `getMentionOffsets`, so code spans and fences are excluded and the longest display name wins — the same rules the send-time extractor applies. - The plain flavor inlines chip boxes before reading `innerText`; a chip is a flex container, so the browser's own copy split "@john Smith" onto its own line. `MarkdownMention` and `MacEmacsTextShortcuts` are extracted verbatim from `markdown.tsx` and `useRichTextEditor.ts` to keep both files under the size gate. Tests: 20 unit tests over the flavor builder/parser, and a Playwright spec driving the real copy/cut/paste DOM events — timeline copy and "Copy message" of a multi-word non-member mention, pasted into another channel, sent with the original pubkey in its `p` tag; composer copy/cut round trip; plain flavor asserted to contain no 64-hex string; half-selected chip asserted to carry no identity. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Clipboard HTML is untrusted, and paste registered every identity record it carried. An empty `<span data-mention-pubkey=… data-mention-label="Jane Doe">` — invisible in the pasted content, and easy to hide on any copied page — rebound that display name for the rest of the composer session. Registered names take precedence over channel members, so a later hand-written @jane Doe would chip-light convincingly and carry the attacker's pubkey in its `p` tag. The record cap, label bound, and hex check bounded volume, not this. Each branch now registers only the records whose label is mentioned in the content *it* inserts: the plain flavor for a Markdown copy, the normalized HTML's own text for a rich one. Matching reuses `getMentionOffsets`, the same matcher the send-time extractor applies, so a dropped record is one that could not have tagged anyone from this paste anyway — and every binding that survives is visible to the user who accepted it. `normalizeMentionClipboardHtml` becomes `normalizeMentionClipboardContent` and returns the HTML to insert alongside its rendered text, so the visibility check and the insertion cannot read different markup. Block-tag boundaries become newlines there: a `DOMParser` document is never laid out, so `textContent` would run "…the bug" straight into "@john Smith" and drop a legitimate identity at the head of a paragraph. Tests: five unit tests over the visibility filter and the gated registration, and a Playwright case that pastes a hidden record claiming "John Smith", then writes that name in the composer and sends. With the gate removed the composer lights a chip and the outgoing event carries the impostor's `p` tag — both assertions fail; with it, the full clipboard spec passes. Signed-off-by: Matt Toohey <contact@matttoohey.com>
The paste normalizer restored a chip's sigil unconditionally. The copy-side partial-chip guard only runs when Buzz's handler wins the copy; a selection that crosses a chip boundary covers no chip fully, so the handler declines and the browser's default copy serializes the partially covered chip element with its full identity attributes around only the covered slice of its text. Pasting that invented a mention the user never copied — "@smith" out of "John Smith". normalizeMentionClipboardContent now mirrors restoreChipSigils: when the element declares a full label (data-mention-label / data-channel-label) that its text no longer matches, the fragment stays plain text. The comparison tolerates what a legitimate full chip picks up in transit — the sigil Buzz's own copy handlers write back, the author's casing buildMentionSpanHtml preserves, and spaces swapped for U+00A0 on a pasteboard round trip. Label-less legacy chips keep the old restore. With no sigiled label in the inserted content, the visibility gate goes on to discard the identity record the attributes still carry, so the fragment binds nothing. Tests: four unit tests over the match predicate, and a Playwright case driving the real boundary-crossing drag — it dispatches a copy on a selection running from mid-chip into the sentence beyond, proves the production handler declines it, serializes the range exactly as the default copy does (attributes intact), and pastes. With the guard removed the composer reads "@smith fixed the" and the test fails; with it, the fragment stays the plain words the user copied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The forum composer understood the identity sidecar on paste, but the forum's read surfaces never produced it: ForumPostCard and ForumThreadPanel render mention chips through the same Markdown as the timeline with no copy handler, so a selection copy from a forum post or reply fell through to the browser's default — sigil-less text with no identity attached. The original dead-text bug, persisting on this surface. Both forum containers now wire handleTimelineMentionCopy the way MessageTimeline and MessageThreadPanel do: the posts-list scroller in ForumView covers every post card, and the thread scroller in ForumThreadPanel covers the root post and every reply. The handler is surface-agnostic — it works off the selection and the rendered message-markdown DOM — so the forum inherits the sigil restore, the partial-chip guard, and the pubkey-free plain flavor unchanged. Test: a Playwright case seeds a forum post and a reply carrying a multi-word non-member mention, selection-copies each surface through real copy events, asserts both flavors, then pastes into the forum's own reply composer and sends — the outgoing reply recovers the original pubkey in its p tag. With the wiring removed the copy falls through to the default path and the test fails; with it, the full spec passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The paste-visibility gate reads the text a paste inserts and registers only the identity records that text mentions. It derived that text by recursing into every element, but prosemirror-model's `DOMParser` hard drops the content of `script`, `style`, `title`, `noscript`, `object`, and `head` — so `view.pasteHTML` never inserts it. Crafted clipboard HTML exploited the gap: `visible<style>@jane Doe </style>` beside an empty `<span data-mention-pubkey=… -label="Jane Doe">` passes `hasMentionClipboardHtml`, takes the rich path, and the gate reads "@jane Doe" out of the style element. The composer shows only "visible", so the binding is invisible to the user who accepted it — and it outlives the paste, so a later hand-written @jane Doe chip-lights against the attacker's pubkey. That is the hidden-binding vector the gate exists to close, reopened through markup the gate itself couldn't see. `normalizeMentionClipboardContent` now removes those elements before deriving either output, so the inserted HTML and the text the gate reads stay one view of what the composer ends up holding. The sweep spans the whole document rather than `<body>`: the parser hoists a leading `<style>` or `<title>` into `<head>`. Running it ahead of the chip flattening also drops a `data-mention` span nested in an ignored tag outright, instead of flattening it into sigiled text the gate would then read as visible. Tests: unit coverage over each ignored tag, the `<head>` hoist, a chip nested in `<object>`, and an ordinary chip still flattening to a registrable mention; plus a Playwright case pasting the `<style>` vector and sending. With the sweep removed the gate registers the impostor and both fail; with it, the full clipboard spec passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
`restoreChipSigils` decided whether a copied chip was whole by comparing its DOM text to the label its attributes declare, with `===`. That bound the copy handler to InlineChip rendering nothing but the bare label. Any chip whose text differs classifies as a partial selection: the handler strips its identity, `restored` stays false, and — if that was the selection's only chip — the whole copy silently falls through to the browser's default. The sigil-less dead text this branch set out to fix, back with nothing to signal it. That is already reachable. `BuzzLinkChip` runs a wrapping chip's label through `truncateInlineChipLabel`, so a channel name past the 48-grapheme cap renders ellipsized while `data-channel-label` still declares it in full — and relay-side validation caps no channel name. Copying a reference to one loses its `#` today. The comparison moves to `matchChipTextToLabel` in the module that owns the label attributes, which both clipboard sides now call, so neither can drift on what counts as a whole chip. It returns `full`, `truncated`, or `fragment` rather than a boolean: `truncated` is the cap's own output for that label, derived by calling the helper the chips render with, so the tolerated form cannot fall out of step with what a fully selected capped chip carries. Both sides write the declared label back, so the copy carries the real reference rather than the visual ellipsis, and the paste side's visibility gate — which matches records against inserted text — still sees a mention copied whole. Text a chip adds beyond its label still reads as `fragment`. That is the safe direction: it costs a copy its identity rather than inventing one. InlineChip carries a note saying so, since that is where such text would be introduced. Tests: unit coverage of the three verdicts, including that only the cap's own output is tolerated; and truncated-chip cases bound to both production seams — `buildTimelineClipboardFlavors` under jsdom for the copy, `normalizeMentionClipboardContent` for the paste. Both fail against the exact-equality comparison and pass with the fix, and a genuine fragment of the same chip still gains no sigil. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
handleTimelineMentionCopy's doc comment promised a no-op for copies whose clipboard data the browser withheld, but the handler called event.clipboardData.setData unconditionally after preventDefault(). React's types claim clipboardData is always present; if the runtime ever hands over null, the default copy is already suppressed and the setData throws — an empty clipboard. handleComposerMentionCopy already guarded this; the timeline handler now does the same, declining before it touches the event so the browser's default copy proceeds. Tests: the handler seam itself was previously untested — two unit tests now drive handleTimelineMentionCopy directly: a withheld-clipboard copy stays a no-op (fails against the unguarded handler, which throws after suppressing the default), and an ordinary chip copy still claims the event and writes both flavors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
matchChipTextToLabel judges a pasted chip whole while tolerating what a legitimate one picks up in transit: a pasteboard's U+00A0 for a space, padding, the author's casing, the ellipsis the inline-chip cap renders. The paste normalizer then wrote that verbatim text into the composer, so every tolerance leaked through to layers that tolerate nothing — they all match through getMentionOffsets, which escapes the label and requires its literal characters. A chip that came back from the pasteboard with U+00A0 inside its name inserted a mention no record matched: the visibility gate silently dropped the identity, and neither the decorations nor the send-time extractor found it either. Sigiled dead text, for the multi-word-name case this branch exists to fix. Every non-fragment match now writes the declared label back, trimmed, the way restoreChipSigils already does on the copy side; fragments and label-less legacy chips keep their verbatim text. Deriving the inserted text from the same attribute parseMentionClipboardRecords reads, trimmed the same way, makes the two provably consistent for any label — the trim because an untrimmed padded label produces "@ John Smith" against a record reading "John Smith", the same silent drop through another door, and the label rather than a normalized copy of the text because a display name that genuinely contains U+00A0 would not survive the latter. It also closes the class rather than the instance: a later addition to the classifier's tolerance set cannot reopen the gap, because the write-back no longer mirrors that list at all. On the happy path it is a no-op, since a Buzz rich copy already carries the label as its chip text. Tests: four unit cases bound to the production gate via selectVisibleMentionIdentities — a U+00A0 round trip and a padded chip both deliver their pubkey (both fail against the verbatim write-back), a padded label attribute pins the trim (fails without it), and a label-less chip still pastes verbatim; plus a Playwright case pasting a rich flavor whose chip text carries U+00A0, asserting the composer chip lights and the sent event carries the original pubkey in its p tag. That one fails against the old write-back with no chip in the composer, and the full clipboard spec passes with the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The home inbox was the one message-reading surface this branch left behind, in both directions. Its "Copy message" resolved zero identities: MessageActionBar builds the HTML sidecar from useMessageMentionIdentities, resolveMentionProps returns undefined without profiles, and InboxMessageRow was the one call site that never passed them — so the copy wrote a plain-only flavor. And the detail scroller had no onCopy, so dragging across a rendered chip fell through to the browser's default: sigil-less text, no identity. The original dead-text bug, on the surface a mention is most likely to be read from. Everything the shared machinery needs was already here. The pane receives `profiles` and already spends it on edit-permission checks; it just never reached the rows. Inbox bodies render through the same Markdown as the timeline, so the chips already carry the full attribute set the copy handler reads. The pane now forwards `profiles` to each row's action bar and wires handleTimelineMentionCopy on the message scroller, the way MessageTimeline, MessageThreadPanel, and the two forum containers do. The scroller wraps only the messages — the composer is a sibling overlay — so this cannot shadow the composer's own copy handler. Left alone deliberately: inbox list previews truncate upstream and render chips with no pubkeys, so there is no identity there to carry; project inbox detail renders no mention chips at all, so the wiring would be dead code. A wider audit of the remaining Markdown surfaces — project conversation panels, agent transcripts, pulse — is its own change. Tests: two Playwright cases seed a mention message into the home feed, open it, and close the loop through a DM composer — "Copy message" and a selection copy over the detail scroller, each pasted, sent, and asserted to carry the original pubkey in its p tag. Each binds only its own wiring: with `profiles` dropped the copy-message case fails with no HTML flavor written, with onCopy dropped the selection case fails on a copy the handler never claimed, and each still passes when the other's wiring is removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Visibility was the paste's whole trust boundary: a record whose label appeared in the inserted content was registered, and registration outlives the paste. Clipboard HTML is attacker-authored, so any copied page could carry `<span data-mention-pubkey="<their key>" data-mention-label="John Smith">@john Smith</span>` — plausible, plainly visible, and accepted. The chip lit, the send carried their key in its `p` tag, and the name stayed bound for the rest of the session. Seeing a label only proves the user saw a name, never that the name belongs to the key beside it; a Buzz provenance marker proves less still, since an attacker writes the same marker. The pair itself is now checked against state the community supplied. Local first — the mention candidates the composer would offer and the profile lookup the surface renders from, both already in hand — then the relay's own profile for the pubkey. Labels are compared against `collectProfileAliases`, the same alias set that turned a `p` tag into the label the copy carries, so a chip rendered off a kind-0 `name` or a NIP-05 handle verifies on the alias it was rendered from. Anything neither source names is dropped: the words paste as readable text and tag nobody. That relay lookup is what keeps the headline case working. A mention of someone who is not a member of the destination channel is exactly what no local directory can speak to, so it is the case the fetch exists for — and it is bounded by the record cap the parser already applies, reads the profile entries `useUsersBatchQuery` maintains before spending a request, and treats an entry those hooks call stale as no answer. Verification can need that round trip, so binding lands after the insertion. The visibility gate therefore runs a second time against what the composer holds when the answer arrives: an in-flight check whose paste the user has since deleted or replaced must not bind a name nothing on screen shows. A composer with no verifier binds nothing, and a failed lookup binds nothing — the pasted words are already in place and simply stay plain, so there is no partial state to retry. `selectBindableMentionIdentities` filters the verifier's answer back down to what it asked about, so the seam cannot widen into "the verifier decides what gets bound". `canonicalMentionLabel` moves out of `matchChipTextToLabel` and is shared, so the copy-side chip classifier and this check agree on what one name spelled two ways means. Tests: eleven cases over the hook the composers actually pass, driving the real `get_users_batch` seam — the forged pair dropped whether the key is unknown or known under another name, each local source and the relay vouching on their own, a mixed paste separated, and the cache freshness rule; four over the bindable selector; and a Playwright case pasting the visible forged shape into a DM, asserting the lookup ran, the chip stayed dark, and the sent event carried no impostor `p` tag. Removing the check lights the chip and fails it; ignoring only its verdict fails it the same way. The full clipboard spec passes. Signed-off-by: Matt Toohey <contact@matttoohey.com>
f8a8f55 to
7d347ec
Compare
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Reviewed: 40220d5618fc7dbe06421f358053ca77ed0f67d7..7d347ec410e6c374e2d45c8da7ae10a97f8d2fea (exact head 7d347ec410e6c374e2d45c8da7ae10a97f8d2fea)
Risk: high — this changes message identity binding and outbound Nostr p tags across clipboard/composer/send boundaries.
Blocking P2 — pasted-mention verification is neither occurrence-owned nor joined to send
handleMentionClipboardPaste inserts text and starts detached verification (desktop/src/features/messages/lib/mentionClipboardPaste.ts:39-45,63-83,129-141). Settlement accepts the result if that label appears anywhere in the current composer (mentionClipboardPaste.ts:69-76; desktop/src/features/messages/lib/mentionClipboard.ts:339-345), then the label→pubkey map is overwritten (desktop/src/features/messages/ui/useMentions.ts:522-540). Meanwhile chat and forum send paths synchronously extract the current mappings without awaiting applicable verification (desktop/src/features/messages/ui/useMentionSendFlow.ts:718-725; desktop/src/features/forum/ui/ForumComposer.tsx:244-246). There is no pending-operation registry, composer generation/range ownership, cancellation, or stale-settlement fence.
This permits four user-visible integrity failures:
- Paste a valid mention and send immediately: readable
@Labelis published without the intendedptag. - Paste slow identity A, replace it with same-label fast identity B, then let A settle: stale A can overwrite B.
- Delete pasted A and hand-type the same label elsewhere before A settles: A can bind to text the user did not select.
- Clear/send and begin a new draft before old verification settles: the late completion can repopulate the next draft's mapping.
The first case was reproduced against the production path with the existing usersBatchDelayMs: 2_000 seam: immediate Send omitted John Smith's expected 7c1f…931b tag and emitted only the DM participant tag. The checked-in clipboard E2E waits for resolution and therefore does not guard this schedule. Both independent systems and product/adversarial passes reached the same defect; the load-bearing implementation and test files are blob-identical to the previously rejected head.
Author action: own each pasted mention verification by composer generation plus exact inserted occurrence/range; invalidate or ignore it after deletion, replacement, clear, unmount, or a newer same-label choice; prevent older settlement from overwriting newer intent; and transactionally join applicable pending verification to Send with truthful pending/failure behavior. Add deterministic production-seam regressions for immediate paste→send, A-slow/B-fast same-label ordering, delete→hand-type, clear/new-draft late settlement, and verification failure, asserting exact outbound p tags and visible state.
Verification owner: author owns the fix and causal regressions; reviewer owns exact-head source/test re-review and adversarial schedule reruns. Native-harness ownership remains with the reviewer for real macOS/Windows Cmd/Ctrl+C/X/V and pasteboard-flavor exercise.
Validation at exact head
pnpm --dir desktop test— 6,111/6,111 passed; post-run HEAD unchanged and tree clean.- Checked-in
mention-clipboard.spec.ts— 12/12 passed; it lacks delayed-verification/immediate-send coverage. - Temporary production-path mutation/reproduction:
pnpm --dir desktop build:e2e && pnpm --dir desktop exec playwright test tests/e2e/mention-clipboard.spec.ts --project=smoke --grep 'MUTATION REVIEW'— failed causally because the expected pasted identity tag was absent; temporary test removed and tree restored clean. git diff --checkand repository file-size/policy preflight passed.- Community query-provider tenancy is keyed/isolated (
desktop/src/app/App.tsx:402-407,630-640); the blocker is ownership of asynchronous work within and across composer lifecycles.
Confidence gap, not a separate blocker: real OS keyboard clipboard and native pasteboard-flavor behavior were not exercised in this review. Current CI also includes an E2E shard failure/pending jobs; this verdict rests on the reproduced product defect, not those gate states.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
@.scratch/review7228.md
Superseded immediately: CLI passed the review body file path literally instead of its contents.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Reviewed: 40220d5618fc7dbe06421f358053ca77ed0f67d7..7d347ec410e6c374e2d45c8da7ae10a97f8d2fea (exact head 7d347ec410e6c374e2d45c8da7ae10a97f8d2fea)
Risk: high — clipboard identity recovery crosses an asynchronous relay boundary and controls outbound Nostr p tags.
Behavior/contracts traced: rich clipboard parsing and identity verification; paste insertion; mention-map ownership; chat and forum send; immediate send; same-label replacement; deletion/clear/new-draft lifecycle; community cache tenancy; exact outbound tags. Both independent review lanes confirmed the same remaining lifecycle defect, and one reproduced the immediate-send failure through the production Playwright seam.
Blocking P2 — pending verification is neither joined to send nor owned by its pasted occurrence
handleMentionClipboardPaste inserts immediately, then starts detached verification through void ...then(...) (desktop/src/features/messages/lib/mentionClipboardPaste.ts:39-45,63-83,129-141). Settlement proves only that the same label appears somewhere in the current composer (mentionClipboardPaste.ts:69-76; mentionClipboard.ts:339-345), then registration overwrites the global label mapping (useMentions.ts:522-540). Neither chat send (desktop/src/features/messages/ui/useMentionSendFlow.ts:718-725) nor forum send (desktop/src/features/forum/ui/ForumComposer.tsx:244-246) awaits or otherwise owns pending verification.
This produces concrete identity failures:
- A valid copied mention pasted and immediately sent can publish before verification settles, omitting the intended recipient
ptag. - Slow identity A followed by a newer same-label identity B can settle B then A; A overwrites the newer choice while the visible label remains unchanged.
- Deleting pasted A and later typing the same label can let A bind to text from a different user action. A completion after clear/send can likewise repopulate a later draft's mapping.
The five load-bearing implementation/test files for this path remain blob-identical to the prior rejected head. The new head does not add immediate-send, reordered A/B, delete-and-retype, or clear/new-draft lifecycle coverage.
Reproduction: an independent lane added a temporary test at the existing production Playwright seam with usersBatchDelayMs: 2_000, pasted a valid non-member @John Smith, and clicked Send immediately. The assertion failed: the outbound tags contained only the DM participant, not John's expected key. The temporary mutation test was removed afterward and the exact-head worktree was clean.
Author action: make verification an operation owned by the exact composer generation and pasted occurrence/range; invalidate or ignore it after replacement, deletion, clear, send, unmount, or a newer same-label choice. Transactionally join applicable pending verification to Send with truthful pending/failure behavior, and prevent stale completion from overwriting newer intent. Add deterministic production-seam regressions for immediate paste→send, slow-A/fast-B with the same label, delete→hand-type, clear/new-draft stale completion, and verification failure, asserting exact outbound p tags.
Verification owner: author for the fix and causal regressions; reviewer for exact-head mutation/adversarial re-run. Native-harness ownership remains reviewer/tooling.
Validation at matching HEAD:
- PASS — both lanes confirmed local clean HEAD and remote PR head
7d347ec410e6c374e2d45c8da7ae10a97f8d2fea;git diff --checkpassed. - PASS — full Desktop unit package: 6,111 passed, 0 failed.
- PASS — checked-in clipboard Playwright spec: 12 passed, 0 failed; it lacks the delayed-verification/immediate-send schedule.
- FAIL as expected — temporary production-seam delayed-verification mutation test reproduced the missing recipient tag; temporary change removed and tree returned clean.
- CI snapshot: macOS/Windows builds and Desktop integration passed; Desktop Core and four smoke shards were still pending. Pending CI is not the basis for this verdict.
Manual/native evidence: no real OS Cmd/Ctrl+C/X/V pasteboard round trip was completed at this head.
Residual risk: native clipboard flavor behavior remains reviewer/tooling-owned after the author fixes the concrete lifecycle defect. Any new head invalidates this verdict until its delta is reviewed.
— :bot: Jude’s code review agent
A pasted pair binds only once this community vouches for it, and for a non-member that check crosses the network — so the answer lands after the words are on screen. Its only completion fence was "does this label appear anywhere in the composer right now", which is a question about the composer rather than about this paste, and three things went wrong behind it. Sending inside the window dropped the identity outright. Extraction reads the mention map synchronously, so a genuine copied mention whose lookup was still in flight published a readable `@Label` with no `p` tag — the non-member case the feature exists for is exactly the slow one. Every send seam now awaits `settlePendingMentionBindings()` before extracting recipients: the channel flow, the edit path, and the forum composer. It is bounded at ten seconds, so a lookup that never answers delays the send rather than blocking it, and on expiry the send proceeds with what the composer truthfully shows — the chip never lit, so plain text sends as plain text, with the unsettled count logged rather than silently capped. A stale settlement overwrote newer intent. Paste a label, replace it with a same-label paste of somebody else, and the first answer arrived to find its label still visible, so it took the name back. Every claim on a label now carries a generation, and a settlement writes only if its own claim is still the newest. Explicit acts claim too — a picker selection, a resolved insert, a persona registered at send time — so choosing a name from the list retires a paste still verifying it, persona routing included, since a settled paste would delete that entry. And deleting a paste bound a ghost. Remove the pasted text, hand-type the same label before the answer lands, and the label-anywhere check passed: the clipboard's pubkey bound to characters the user typed, whose own candidates should have resolved them. A new ProseMirror plugin now holds the range each in-flight paste inserted and maps it through every transaction, mapping `from` forward and `to` backward so text typed at either edge lands outside it. Settlement asks the narrower question — is the label still in the text *this* paste owns — and a range whose content was deleted or replaced dies rather than drifting, which is the fail-closed direction: no live range binds nothing. The extension is registered in the shared composer extension list, so every composer that accepts a paste is fenced by construction. The binder moves out of the paste handler into `mentionPasteBinding`, owned by `useMentions`, so the fences sit next to the map they guard: handlers now pass one `bindPastedMentionIdentities` callback instead of a register/verify pair, and `registerMentionPubkey` splits into the claiming write every explicit caller wants and the non-bumping write a settlement is allowed. Tests: eleven cases over the occurrence plugin driving a real `EditorState` — reading its own text across edits elsewhere, dying on deletion, on replacement by identical text, and on an edit into either end; seven over the binder through the production hook, with verifiers deferred by hand so each pins an ordering rather than a race; and three Playwright cases against a new deterministic hold on the relay profile lookup: send-during-verification recovers the tag, a namesake paste that settles last does not take the newer paste's name, and a deleted paste binds nothing to the same sentence retyped. Each fence was removed in turn and fails exactly its own cases. Signed-off-by: Matt Toohey <contact@matttoohey.com>
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
REQUEST CHANGES at exact head 7b1736931d2074238e08d149f93b7d4593830739 (base 40220d5618fc7dbe06421f358053ca77ed0f67d7).
P2: Replacing a pasted mention token inside a larger range preserves stale clipboard identity
The new occurrence fence in desktop/src/features/messages/lib/pastedMentionOccurrences.ts:67-78 drops a tracked paste only when an outer endpoint is deleted. An edit wholly inside the tracked range leaves the occurrence alive. Settlement in mentionPasteBinding.ts:147-166 then accepts the same visible label in the mutated range and registers the clipboard pubkey.
Both A Team lanes independently confirmed the mechanism. A deterministic production-bound probe pasted Hello @John Smith fixed the bug, held identity verification, replaced only the interior @John Smith with identical text, then released verification and extracted outbound recipients. Expected no recovered recipient; actual output contained the clipboard key aaaa…aaaa. The focused test result was 7 passed, 1 failed at this exact head. The checked-in occurrence tests cover whole-range replacement and edits consuming the head/tail, but not this interior schedule.
User impact: a user can explicitly replace the identity-bearing token, yet identical visible text silently retains the former person's identity and outbound Nostr p tag. This applies to shared editor paths across stream/DM/thread/edit and forum composers.
Required author action: invalidate or split a tracked occurrence whenever a transaction deletes or replaces text overlapping a carried mention token; tracking exact mention spans is safer than relying on the enclosing paste's endpoints. Add a production-bound regression for interior identical-label replacement that asserts no recovered chip/binding and exact outbound p tags exclude the clipboard key. Mutation-check the overlap fence and cover partial edits through the mention token.
Verification owner: author supplies the fix, causal regression, and mutation proof; code review reruns exact-tag lifecycle coverage on the next immutable head.
Repairs accepted on this head
Immediate paste→send is now joined before extraction in chat/thread/DM, edit, and forum paths. Slow-A/fast-B whole-paste ordering has a generation fence. Whole-paste deletion/retype and clear paths are fenced. No second author-actionable defect was found in the reviewed trust, send, tenancy, or product/UI paths.
Exact-head validation
- Full Desktop package: 6,129/6,129 passed in the product/UI lane; Mongo's earlier exact-head package run and focused causal probe support the same source state.
- File-size preflight/policy: 10/10 passed, with all production surface checks passing.
git diff --check: passed; local lane heads and live PR head were confirmed as7b173693….- Required relay/Desktop integration, macOS, and Windows checks observed were passing; Desktop Core and four smoke shards remained in progress at final refresh. This finding does not depend on CI.
Confidence gap, not additional author action: native macOS/Windows keyboard and real pasteboard-flavor behavior were not exercised. Native/release verification owns that observation after the repair.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Reviewed: 40220d5618fc7dbe06421f358053ca77ed0f67d7..7b1736931d2074238e08d149f93b7d4593830739 (exact head 7b1736931d2074238e08d149f93b7d4593830739)
Risk: high — clipboard identity recovery determines outbound Nostr p tags across every shared-editor message surface.
Blocking P2 — interior replacement does not revoke clipboard identity
The new lifecycle work fixes the earlier immediate-send and broad stale-generation failures, but occurrence ownership remains too coarse. desktop/src/features/messages/lib/pastedMentionOccurrences.ts:67-78 retires a tracked paste when an outer endpoint is deleted; an edit wholly inside the tracked range preserves both endpoints. Settlement then reads the mutated range and accepts a still-visible same label (desktop/src/features/messages/lib/mentionPasteBinding.ts:147-166).
Consequently, after pasting Hello @John Smith fixed the bug, a user can select only @John Smith and replace it with identical hand-entered/plain text. The old clipboard pubkey is still recovered onto the replacement and can enter outbound p tags, even though the user explicitly replaced the identity-bearing token. This affects stream, DM, thread, edit, and forum composers that share this editor path.
A deterministic probe using the shipped hook, real ProseMirror occurrence plugin, and production extractMentionPubkeys reproduced the failure on this clean exact head: hold verification, replace only the interior mention with identical text, then release verification. The focused suite reported 7 pass, 1 fail; expected no recipients, but extracted clipboard key aaaa…aaaa. The temporary probe was removed and the worktree restored clean. Existing tests cover whole-range replacement and edits consuming the head/tail (desktop/src/features/messages/lib/pastedMentionOccurrences.test.mjs:112-143) but omit this interior schedule.
Author action: invalidate the occurrence—or, preferably, the exact carried mention span—when a transaction deletes/replaces text overlapping that mention token. Add a production-bound regression for late settlement after interior identical-label replacement, asserting no recovered chip and exact outbound p tags exclude the clipboard key. Cover partial token edits as well as whole-token replacement, and mutation-prove the overlap fence.
Verification owner: author owns the fix and causal unit/Playwright regressions; reviewer owns exact-head lifecycle, cross-surface UI, and exact-tag re-verification. Native OS clipboard verification remains reviewer-owned confidence work.
Improvements verified on this head
- Immediate paste→send now awaits settlement before extraction in chat/thread/DM (
desktop/src/features/messages/ui/useMentionSendFlow.ts:669-677), edit (desktop/src/features/messages/ui/MessageComposer.tsx:550-576), and forum (desktop/src/features/forum/ui/ForumComposer.tsx:221-249); the existing disabled “Sending” state exposes the wait. - Slow-A/fast-B whole-paste same-label ordering has a generation fence (
mentionPasteBinding.ts:96-113,138-165). - Whole-paste deletion/retype and clear paths are fenced by occurrence death/intent clearing.
Exact-head validation
- Full Desktop package: 6,129 passed, 0 failed; post-run HEAD unchanged and tree clean.
git diff --checkpassed.- Repository file-size preflight: 10/10 passed, with production-surface policy checks clean.
- Focused temporary interior-replacement probe failed causally as described; probe removed afterward.
- Final local/remote head confirmed as
7b1736931d2074238e08d149f93b7d4593830739; base merge-base40220d5618fc7dbe06421f358053ca77ed0f67d7. - CI was still running at review freeze; this verdict does not depend on pending CI.
Confidence gap, not a separate blocker: real macOS/Windows Cmd/Ctrl+C/X/V and native pasteboard-flavor behavior were not exercised in this review.
A pasted pair binds only once this community vouches for it, and the occurrence fence is what decides whether that late answer still describes something the paste put on screen. It tracked the whole insertion and retired it only when an *endpoint* landed in deleted text, so an edit wholly inside the range was invisible to it. Paste "Hello @john Smith fixed the bug", select exactly the mention while the lookup is in flight, and retype it character for character: both endpoints survive, the label is still there, and the clipboard's pubkey binds to words the user typed — whose own candidates should have resolved them. The pair is vouched for, so this is the real John Smith rather than an impostor; the cost is intent, not forgery. Someone who surgically demoted a mention to plain text still publishes a p tag and pings him. The fence moves to the thing it protects. `mentionTokenSpans` locates each `@Label` run inside the pasted range in document coordinates — read out of the document rather than off the clipboard's text, so the Markdown branch, whose `**@john Smith**` TipTap has already parsed, lands on the same characters — and the binder tracks one range per token, binding a record only if one of its own tokens is still alive. Editing the sentence around a mention now costs nothing, which matters precisely because the slow-verifying non-member case is the one this feature exists for: killing the whole paste on any interior edit would have closed the hole by reintroducing the dead-text bug it was opened to fix. Two rules do that, and neither is sufficient alone. Remapping now walks a transaction step by step and drops a range on any strict overlap with a replaced region, because retyping *part* of a token — "John" inside "@john Smith" — moves no endpoint either; a boundary touch is not an overlap, so an edit that butts the token spares it. And settlement re-asks `findMentionTokenSpans` for the live range plus a character either side, which catches what a range survives by design: an insertion inside the token replaces nothing, and a character typed against its outer edge lands outside the range while destroying the word boundary a mention needs. Tests: four plugin cases over a real `EditorState` — interior replacement, a later overlapping step, a boundary-butting replacement spared, an interior insertion left alive for the text check to refuse; five binder cases through the production hook with the verifier deferred by hand; ten over the new span helper, each reading its span back out of the document; and a Playwright pair against the deterministic relay-lookup hold, where retyping the mention mid-flight sends no p tag and deleting a word beside it still sends one. Each fence was removed in turn: without overlap-kill the interior cases fail, with the whole insertion tracked and overlap-kill kept the "edit elsewhere still binds" cases fail, and with both reverted the reviewer's schedule fails. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: 4b0744d7f3161d9f045dd67a73e014e4440c2b47..95c95153f29f51a3400b46c3ecb10cca6dfd22e4 (exact head 95c95153f29f51a3400b46c3ecb10cca6dfd22e4)
Risk: high — untrusted clipboard identity crosses asynchronous profile verification, ProseMirror transaction mapping, composer ownership, and outbound Nostr recipient tags.
Behavior/contracts traced: timeline/action/composer/forum/inbox copy flavors; rich/plain paste normalization; label→pubkey trust verification; token-scoped occurrence mapping; stale-generation and explicit-selection ownership; message/edit/forum send settlement; exact outbound p extraction; accessibility semantics.
Findings: no unresolved blocking code/product/test defect. The prior interior-replacement blocker is fixed: identity ownership now tracks the exact carried mention token, retires on strict overlapping replacement (including interior/partial and later mapping steps), revalidates token boundaries at settlement, and preserves valid edits outside the token. Both independent lanes found the clipboard trust and async lifecycle fail closed: forged/unvouched pairs stay plain and untagged; stale verification cannot reclaim newer user intent.
Author action: none.
Verification owner: reviewer/tooling for native pasteboard/menu observation; CI for the five still-running Desktop Core/smoke jobs (author only if a required gate later fails causally).
Validation at matching clean head:
- Full Desktop unit suite: 6,236/6,236 passed.
- Exact feature E2E: 17/17 passed.
- Focused occurrence/binding suites: 38/38 passed.
pnpm typecheck,pnpm check,pnpm check:file-sizes, andgit diff --check: passed.- Causal mutation removing strict-overlap retirement produced three relevant failures; source restored, focused suites passed, and the tree returned clean at
95c95153…. - Live browser regressions passed for both “retype pasted mention ⇒ no clipboard pubkey” and “edit beside mention ⇒ preserve identity.”
- Exact-head CI observed green for macOS/Windows Desktop builds, both relay-backed Desktop integration shards, security checks, DCO, and release-candidate check; no failed check observed. Desktop Core and four smoke shards remained in progress at final poll.
Manual/native evidence: browser-level copy/cut/paste DOM journeys were exercised across timeline, Copy message, composer, forum, inbox, partial-chip, cross-channel, NBSP, forged/hidden/unvouched, and send-tag cases. A packaged Tauri journey using native macOS/Windows/Linux pasteboards and menu/context-menu dispatch was not run.
Residual risk: native OS pasteboard flavor/dispatch behavior remains independently unwitnessed. This is a reviewer/release confidence gap, not an author-actionable defect; checked-in browser evidence and existing Tauri HTML-write boundary support the approval.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed: 4b0744d7f3161d9f045dd67a73e014e4440c2b47..95c95153f29f51a3400b46c3ecb10cca6dfd22e4 (exact head 95c95153f29f51a3400b46c3ecb10cca6dfd22e4)
Risk: high — clipboard metadata is attacker-controlled input, and recovered identity determines outbound Nostr p tags across message, edit, and forum composers.
Findings: no unresolved author-actionable defect. The new token-scoped overlap fence resolves the prior interior-replacement blocker.
Behavior and contracts verified
- Clipboard label→pubkey metadata binds only when trusted local aliases or relay profile state vouch for it; forged/unvouched input remains readable plain text and emits no corresponding
ptag. - ProseMirror transaction mapping retires carried identity on strict overlap with the exact mention token, including whole- and partial-token interior replacement. It preserves valid adjacent edits and revalidates the surviving token plus both word boundaries before settlement (
mentionPasteBinding.ts,mentionTokenSpans.ts,pastedMentionOccurrences.ts). - Boundary-destroying insertion cannot bind, while replacement merely abutting the token preserves valid ownership.
- Async generation fencing prevents stale completion from reclaiming newer same-label intent or explicit selection.
- Message, edit, and forum send paths settle applicable pending verification before extracting recipients. Immediate Send therefore waits rather than publishing readable
@Labelwithout its identity tag; clear/draft/composer lifecycle retires old ownership. - Product E2E proves that replacing a pasted mention leaves plain text and excludes the clipboard key, while an untouched pasted chip sends the exact key. Copy/cut/paste coverage spans timeline selection, message actions, composer, forum post/reply, Home inbox, partial-chip copy, cross-channel paste, NBSP normalization, and forged/hidden/unvouched identities.
- Added mention metadata are inert
data-*attributes; no new interactive control or accessible stop was introduced.
Exact-head validation
- Full Desktop unit suite: 6,236/6,236 passed.
pnpm typecheck: passed.pnpm check: passed with pre-existing/non-PR warnings only.pnpm check:file-sizes: passed.- Exact feature E2E under
CI=1with one worker: 17/17 passed. - Focused occurrence/binding suites: 38/38 passed.
- Causal mutation removing strict-overlap retirement produced 3 relevant failures for partial/interior replacement and later-step overlap; source restored, focused restored run 28/28 passed, exact HEAD unchanged, tree clean.
- Live browser regressions passed for both retyping a pasted mention (no binding to replacement text) and editing beside a pasted mention (identity preserved).
git diff --check, repository policy/file-size checks, DCO, Semgrep, zizmor, Windows/macOS builds, and relay-backed integration shards passed at the final poll.
Confidence gaps / external gates
- Packaged-Tauri native pasteboard and menu/context-menu dispatch were not independently observed across macOS/Windows/Linux. Synthetic browser E2E proves flavor construction/consumption, and the Tauri HTML-write API is pre-existing, but this is not native artifact proof.
- Author action: none.
- Verification owner: native reviewer / release validation.
- Desktop Core and four Desktop Smoke E2E jobs remained in progress at the final review poll; no failed exact-head check was observed.
- Author action: none unless a required gate fails causally.
- Verification owner: CI / merge gate.
Residual risk: native clipboard flavor/menu behavior remains unwitnessed in a packaged app. The source, causal mutation, browser behavior, unit coverage, and completed platform/integration gates provide sufficient code-review evidence; the remaining native/CI observations retain their named owners.
* origin/main: refactor(relay): extract NIP-29 membership authorization (#7285) chore(release): release Buzz Desktop version 0.5.22 (#7308) feat(desktop): preserve mentions across copy and paste (#7228) test(desktop): await Bestie drag and profile hover endpoints (#7294) Collapse contiguous join messages (#7262) Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
2 participants
Copying a message out of the timeline lost the mention. The rendered chip drops
the
@for display, so the clipboard carried "John Smith" — two ordinary wordsno composer could bind back to a pubkey. Pasting into another channel produced
dead text, and sending it tagged nobody.
What changed
Every Buzz copy now writes two clipboard flavors in one transaction:
text/plain— readable anywhere, sigils restored, no pubkeys. This is whatTextEdit, Slack, and every other external app receive.
text/html— the same content with each mention wrapped in a span carryingdata-mention-pubkey/-label/-kind.On paste, the composer harvests those records, registers each
name → pubkeywith the existing mention machinery, and inserts the content: the chip re-lights
and the send path recovers the identity the author tagged. A marker attribute
records what the plain flavor holds, so a Markdown copy pastes through the text
pipeline and a rendered copy through the HTML one.
Covered surfaces: timeline selection copy, thread-panel selection copy,
forum post/reply selection copy, "Copy message", and composer copy/cut — plus
paste in both the channel and forum composers.
Trust boundary
Clipboard HTML is untrusted, and the branch treats it that way:
64 hex before it can become a
ptag.<span data-mention-pubkey=… -label="Jane Doe">would otherwise rebind thatdisplay name for the rest of the composer session, so a later hand-written
@jane Doe would chip-light convincingly against the attacker's key. Each
branch registers only the records whose label appears in the text it
inserts, matched with
getMentionOffsets— the same matcher the send-timeextractor uses.
DOMParserhard-dropsscript,style,title,noscript,object, andheadcontent, sovisible<style>@Jane Doe</style>beside an empty chip spanused to smuggle a binding past the gate. Those elements are stripped before
either output is derived.
falls through to the browser's default copy, which serializes the full identity
attributes around a slice of the text — pasting that invented "@smith" out of
"John Smith". Paste now leaves a fragment as plain text, tolerating only what a
whole chip picks up in transit (restored sigil, author casing, U+00A0 swaps,
and the label cap's own ellipsis).
Both clipboard sides share one
matchChipTextToLabelhelper, in the module thatowns the label attributes, so copy and paste cannot drift on what counts as a
whole chip.
Notes
getMentionOffsets, so code spans and fences areexcluded and the longest display name wins.
innerText; a chip is aflex container, so the browser's own copy split "@john Smith" onto its own line.
MarkdownMentionandMacEmacsTextShortcutsare extracted verbatim frommarkdown.tsxanduseRichTextEditor.tsto keep both files under the size gate.Testing
ignored-tag sweep, and the chip-match verdicts.
desktop/tests/e2e/mention-clipboard.spec.ts) driving realcopy/cut/paste DOM events: timeline and "Copy message" of a multi-word
non-member mention pasted into another channel and sent with the original
pubkey in its
ptag; the forum copy → forum reply round trip; composercopy/cut; plain flavor asserted to contain no 64-hex string; the
boundary-crossing drag; and the hidden-record and
<style>smuggling vectors.removed.
just ci/ pre-push lanes green (5927 desktop unit tests, typecheck, lint,file-size gate).