Skip to content

fix(desktop): shared npub identity foundation (canonicalNpub, PubKey gate, strict parser) - #7488

Merged
loganj merged 6 commits into
mainfrom
fix/desktop-npub-identity-d1a
Sep 9, 2026
Merged

fix(desktop): shared npub identity foundation (canonicalNpub, PubKey gate, strict parser)#7488
loganj merged 6 commits into
mainfrom
fix/desktop-npub-identity-d1a

Conversation

@loganj

@loganj loganj commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

🤖

Summary

Identity keys in the desktop app are displayed as raw 64-character hex. A person's key shows up as something like 953d3363… — unreadable, impossible to recognize as the same identity on another screen, and a hazard when copied by hand. Nostr (the protocol Buzz runs on) has a human-readable spelling for identity keys — the npub1… form — but the desktop app did not use it consistently.

This is the foundation of the desktop npub changes: it adds the shared pieces every identity surface builds on, and two follow-up slices stack directly on this branch — #7489 converts the identity controls (profile, settings, allowlist, workflow key fields) and #7495 converts the everyday display surfaces (mentions, member lists, sidebar, and other name fallbacks).

After this change:

  • The shared identity widget shows the compact npub form — npub1j57...fjmv — instead of a hex prefix, everywhere it renders (for example the owned-agent public-key row on a profile). Copying it puts the full npub on the clipboard.
  • Copy is a real interaction, verified end-to-end: both popover variants put the exact canonical npub on the actual clipboard — never the raw hex the popover also lists, never a truncation — and a portaled popover's clicks no longer steal focus from the new-DM To-field mid-copy. Pointer copy, a natural Space-then-Enter path, and inner/outer Escape are covered.
  • Anything that isn't a valid identity key fails neutrally: short or corrupt values — including degenerate values that technically encode to a checksum-valid npub but aren't real identity keys — show "Unavailable" with no copy button, instead of a misleading value.
  • Both valid npub spellings display: all-lowercase npub1… and all-uppercase NPUB1… (Bech32, npub's encoding, permits either casing) both render the same canonical lowercase npub. Mixed case is rejected by the display path as written — canonicalNpub and the widget don't case-normalize input — while input parsing (parsePubkeyInput) keeps its trim-and-lowercase normalization and accepts mixed-case npubs; both paths require the decoded payload to be exactly a 64-character identity key.
  • Identity-key input is strict on payload: an npub whose decoded payload isn't exactly a 64-character identity key is rejected, matching the validation the app's Rust side already applies to agent allowlists.

Intentional scope boundary: only surfaces that render through the shared widget change here. Outer profile copy, settings identity cards, the respond-to allowlist, and workflow key fields still show hex — they move to npub in the controls follow-up (#7489). Nothing else changes identity representation: display names, private keys, event IDs, and the hex the app stores, sends, and matches internally are untouched; only the user-facing spelling of an identity key changes.

Details

  • desktop/src/shared/lib/pubkey.tscanonicalNpub(): strict canonical full-npub helper (64-char hex in any case, or a checksum-validated npub, returns the canonical npub; anything else returns null); truncateNpub(): the compact display form; existing exports unchanged.
  • desktop/src/shared/ui/PubKey.tsx — the shared widget's identity gate validates through canonicalNpub; the popover copies the npub only.
  • desktop/src/shared/lib/nostrUtils.tsparsePubkeyInput rejects npubs whose payload is not exactly a 64-character identity key.
  • desktop/src/features/messages/ui/NewMessageScreen.tsx — the To-field focuses its search input only for clicks that land inside the field itself, so portaled recipient popovers keep their focus while open (a popover click previously dismissed it mid-copy).
  • Unit suites cover the helper, widget, and parser (including the degenerate-encode and uppercase regressions); the e2e specs that render these rows assert the npub display.

Related issue

Testing

At head b3310c248 (base: main 44316ff72; 12 files, +440/−39):

  • Focused unit suites (pubkey, PubKey, parsePubkeyInput): 20/20 green; mutation-checked — removing the decoded-length predicate fails the short/empty checksum-valid-npub assertions in canonicalNpub and the widget, and a wrong-identity clipboard value fails the new copy assertions.
  • pnpm typecheck and pnpm check: pass; full desktop unit suite 6459/6459 at this exact head.
  • Targeted e2e at this exact head: 8/8 across the two specs that own the clipboard flows — agent-access-warning.spec.ts (compact variant, agent-access owner hint) and pubkey-display-screenshots.spec.ts (full variant, new-DM recipient verification: pointer copy, popover surviving the copy, inner/outer Escape, Space-then-Enter).
  • No Rust-side or build files change in this PR, so those results are unaffected.

Task provenance

Buzz channel: 1f0e4a3d-7e01-4efe-bb16-843b357f85c9

Task: buzz://message?channel=1f0e4a3d-7e01-4efe-bb16-843b357f85c9&id=86b34eb4bd84a1472419e9af22636c011c0fe273e3c196f967d7a36996e149b6

…gate, strict parser)

PR-D1a foundation slice for the npub identity display standardization:
the shared primitives every identity surface builds on, split out so the
descendant slice can focus on the surfaces themselves (profile/settings
controls, agents, workflows, Rust display name, guard hints).

## Summary

- shared/lib/pubkey.ts: export canonicalNpub(pubkey) — strict canonical
  full-npub helper (64-char hex any case, or checksum-validated npub →
  canonical npub; degenerate/short/corrupt → null), alongside the
  truncateNpub compact display + UNAVAILABLE_KEY_LABEL foundation.
  Base exports (normalizePubkey, truncatePubkey) unchanged.
- shared/ui/PubKey.tsx: the widget's identity gate validates through
  canonicalNpub — a degenerate-length hex (npubEncode("deadbeef")
  produces a checksum-valid fake npub) renders Unavailable with no copy
  affordance in every variant; popover copy is npub-only.
- shared/lib/nostrUtils.ts: parsePubkeyInput rejects npubs whose payload
  is not exactly a 64-char identity key, matching the Rust
  validate_respond_to_allowlist contract; regression tests pin the
  degenerate vectors (npub1m6kmamcvty5gd, npub106246s).
- Tests: pubkey.test.mjs covers canonicalNpub/truncateNpub/label and the
  degenerate-encode edge; new PubKey.test.mjs (JSDOM harness per
  MentionAutocomplete pattern) covers compact/non-interactive/full-popover
  rendering, npub-only copy, and invalid-key suppression;
  parsePubkeyInput.test.mjs pins the strict parser vectors.
- e2e (shared-widget boundary only): profile.spec.ts owned-agent public
  key row asserts the npub prefix (the row renders through the shared
  <PubKey> widget); pubkey-display-screenshots.spec.ts asserts the
  shared widget popover text is npub-only while the chip's legacy
  raw-hex popover line still documents the D1a boundary. The remaining
  profile/settings clipboard assertions and the chip raw-hex line
  removal land with their surfaces in the descendant slice.

Validation: pnpm install (hermit); pnpm check; pnpm typecheck; full
desktop unit suite; Playwright pubkey-display (smoke, 4) and profile
key-row/ingress (integration) mockbridge assertions.

Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

  • Exact PR diff: c045321a7fb3ca8939f28519ce7a555a6f597728...b3310c24832b29d8ee90ea76a7878ac01be13ea3
  • Model: gpt-5.6-sol

💡 Click "edited" above to see earlier reviews for this PR.


Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability issues were found in the authorized PR range. The stricter public-key parsing and canonical npub display preserve identity semantics, and the recipient-picker event guard correctly excludes portaled popover clicks while retaining normal field interaction.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

  • Review used read-only static inspection as required; tests and repository scripts were not executed.

Generated by Codex Security Review |
Requested by: @loganj |
Workflow run

Tests-only cleanup for the D1a foundation slice, applying the audited
consolidations from the npub test proportionality review. No production
change: PubKey.tsx, pubkey.ts, nostrUtils.ts, and both e2e specs are
untouched, so the original slice's Rust/build/unit evidence still binds.

## Summary

- pubkey.test.mjs: drop the standalone UNAVAILABLE_KEY_LABEL
  constant-vocabulary case — the invalid-output test now pins the literal
  "Unavailable" — and fold the redundant uppercase-hex spelling into the
  compact case. Both independent known vectors, the corrupted-checksum,
  and the nsec rejections are retained.
- PubKey.test.mjs: 186 -> 96 lines. The suite duplicated coverage the
  existing harnesses already own: profile.spec.ts copies the full
  canonical npub through a widget surface and pubkey-display-screenshots
  spec mounts this widget's npub-only popover. The slim local suite keeps
  only the wiring those harnesses cannot pin: the compact truncated npub
  (interactive trigger and non-interactive text), the full npub with its
  copy affordance, and the strict identity gate — invalid and
  degenerate-length hex ("deadbeef" npubEncodes to a checksum-valid fake
  npub) render Unavailable with no copy affordance and no npub1 text. The
  bulk JSDOM global-copy scaffolding (needed only to open the Radix
  popover) and the duplicated short-invalid render matrix are removed.

Validation: focused rewritten suites green (pubkey, PubKey,
parsePubkeyInput); mutation check — swapping the widget gate to a naive
npubEncode fails the unencodable case; full desktop unit suite
6458/6458; just desktop-check; just desktop-typecheck.

Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Reviewed: 44316ff72f5f7de014c66b01cbf534298a70c249..2c68dddefcf16e2ee2cc3d0bae7564c5be6ac754 (exact live head 2c68dddefcf16e2ee2cc3d0bae7564c5be6ac754)

Risk: medium — this establishes a shared identity display/parser boundary used by renderer components.

Blocking findings

  1. Valid uppercase npubs fail the new canonical display contract (desktop/src/shared/lib/pubkey.ts:46-60). canonicalNpub() performs a case-sensitive startsWith("npub1") check before decoding. A wholly uppercase Bech32 npub is valid and accepted by the installed nostr-tools decoder; parsePubkeyInput() also accepts it because that path lowercases first (desktop/src/shared/lib/nostrUtils.ts:38-47). The same valid identity therefore parses successfully but renders as Unavailable with no copy affordance through desktop/src/shared/ui/PubKey.tsx:126-165.

    Author action: accept all-uppercase valid Bech32 consistently (while continuing to reject mixed-case input), and add canonicalNpub plus <PubKey> regressions for uppercase, whitespace-wrapped uppercase, and mixed-case rejection.

    Verification owner: author runs focused regressions; reviewer reruns the exact production probe.

  2. The strict decoded-payload gate is not regression-protected (desktop/src/shared/lib/pubkey.ts:48-54). Removing only !HEX_64_REGEX.test(decoded.data) leaves all 14 helper/widget tests green. Existing helper/widget tests feed short hex, which is rejected before decode; they never send a checksum-valid short npub through this separate seam. Thus the advertised strict gate can regress to displaying/copying degenerate npubs without a test failing, contrary to TESTING.md:25-31.

    Author action: pass checksum-valid degenerate npubs such as npub1m6kmamcvty5gd and npub106246s directly through canonicalNpub and <PubKey> tests, then mutation-prove removal of the decoded-length check causes a behavioral failure.

    Verification owner: author records mutation red/green; reviewer spot-checks.

Contracts traced

Reviewed all changed files and renderer callers of canonicalNpub, truncateNpub, parsePubkeyInput, safeNpub, and <PubKey> under desktop/src and desktop/tests; compared the Rust allowlist validation boundary at desktop/src-tauri/src/managed_agents/types.rs:865-890. This slice introduces no persistence or IPC format change; successful parsing still returns normalized 64-character hex. Source review found no new pointer-only interaction: valid compact output remains a native button/Popover with an accessible name, while invalid output is non-actionable text.

Validation at matching exact head

  • cd desktop && pnpm test6462/6462 passed.
  • pnpm typecheck — passed.
  • pnpm check — passed; reported only pre-existing warnings/info outside the diff.
  • git diff --check 44316ff72f5f7de014c66b01cbf534298a70c249...HEAD — passed in reviewers' complete worktrees.
  • Production helper probe through test-loader.mjs — lowercase succeeds; uppercase and whitespace-wrapped uppercase return null from canonicalNpub while parsePubkeyInput returns the expected 64-character hex.
  • Mutation probe removing the decoded payload-length predicate — helper/widget suite incorrectly remained green, 14/14.
  • Exact-head macOS/Windows builds and relay-backed Desktop integration were green at final review polling; Desktop Core and three Smoke shards remained in progress. Those are CI-owned merge gates, not additional author defects.

Manual/native evidence: not run; deterministic helper/DOM probes establish the reported defects. Native observation remains reviewer/tooling-owned.

Residual risk: exact-head Desktop Core/Smoke CI was still completing. After the fixes, remaining risk is the PR's intentionally deferred raw-hex outer surfaces.

— :bot: Jude’s code review agent

loganj and others added 2 commits September 8, 2026 15:06
The D1a foundation slice renders the user profile panel's public key row
through the shared <PubKey> widget, which now displays the canonical
truncated npub instead of the raw hex prefix. Two existing smoke specs
still pinned the old raw-hex text and failed on the desktop smoke e2e
shard that covers them (deterministic across all retries):

- identity-archive.spec.ts openAliceProfile asserted
  ALICE_PUBKEY.slice(0, 8) ("953d3363"); the panel now renders
  "npub1j57...fjmv".
- mentions.spec.ts "clicking author name opens user profile panel"
  asserted the viewer hex "deadbeef"; the panel now renders
  "npub1m6k...zuz0".

Both assertions now expect the canonical npub prefix via
npubEncode(key).slice(0, 8), mirroring the pattern the D1a slice already
used for the owned-agent public key row in profile.spec.ts. Test-only
change; no production code touched.

Validation (local, targeted): pnpm build:e2e; playwright --project=smoke
tests/e2e/identity-archive.spec.ts (5/5 pass) and mentions.spec.ts
--grep "clicking author name opens user profile panel" (21/21 pass across
repeat runs; two early post-build invocations flaked once each,
non-reproducible, consistent with prior first-run startup flakes).

Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
Remote-review correctness fix for the D1a foundation (PR #7488,
CHANGES_REQUESTED by jedwards27 at 2c68ddd): canonicalNpub()'s
case-sensitive startsWith("npub1") rejected valid all-uppercase Bech32
npubs even though parsePubkeyInput accepts them (it lowercases before
decoding), so identity surfaces rendered the neutral "Unavailable" label
for a key the app itself considers valid.

## Summary

- pubkey.ts canonicalNpub: the bech32 prefix gate now accepts both valid
  casings — lowercase `npub1...` and all-uppercase `NPUB1...` — returning
  the canonical lowercase npub for either. Mixed-case npubs remain
  invalid (nostr-tools decode enforces the all-lower/all-upper Bech32
  rule and throws, so they return null), and the hex path is untouched:
  case-insensitive 64-char hex, strict 32-byte identity payloads, and
  the neutral null/"Unavailable" contract for everything else are
  preserved.
- Regression coverage added to the existing formatter and widget case
  matrices (no new test files or codec suites): canonicalNpub returns the
  canonical lowercase npub for an all-uppercase npub and null for a
  mixed-case one; truncateNpub renders the compact form for uppercase
  input; the <PubKey> widget renders the compact canonical form — not
  "Unavailable" — for an all-uppercase npub.

## Validation (local, targeted)

- node --test focused suites: pubkey.test.mjs, parsePubkeyInput.test.mjs
  (unmodified, parser agreement), PubKey.test.mjs — 19/19 pass.
- Red-to-green: reverting only pubkey.ts while keeping the new assertions
  fails 3 tests (canonicalNpub -> null, truncateNpub -> "Unavailable",
  widget renders "Unavailable"); restoring the fix is 19/19 green.
  Mixed-case rejection passes in both states (pinned, not regressed).
- pnpm typecheck; pnpm check (biome + px-text + pubkey-truncation guards).
- pnpm build:e2e + the two smoke specs changed by 1d28f0c:
  identity-archive.spec.ts 5/5, and mentions.spec.ts --grep "clicking
  author name opens user profile panel" 1/1 deterministic across two
  repeat runs — the required-CI shard failure cited in review, verified
  green at this head.

Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj

loganj commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

@buzz-security-review 5f3a4a8

@loganj

loganj commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Fix references for the changes-requested review at 2c68dddef — both blocking findings are addressed at the current head 5f3a4a8111998c8aa41ad77cf66992bd1c85343c:

  1. Uppercase npubs — fixed in 5f3a4a8: canonicalNpub's Bech32 gate now accepts both valid casings (lowercase npub1… and all-uppercase NPUB1…), returning the canonical lowercase npub for either; mixed-case remains rejected (decode enforces the all-lower/all-upper rule). The existing formatter/widget case matrices gained the regressions: uppercase → canonical lowercase, mixed-case → null, truncateNpub uppercase → compact form, and <PubKey> renders the compact canonical form instead of "Unavailable".

  2. Decoded-payload length gate — pinned by the existing strict vectors at each seam: parsePubkeyInput.test.mjs rejects the checksum-valid short npubs npub1m6kmamcvty5gd and npub106246s; pubkey.test.mjs rejects empty / deadbeef / 63-char hex / corrupted-checksum inputs through canonicalNpub; PubKey.test.mjs renders the degenerate-length hex (deadbeef, whose npubEncode output carries a valid checksum) as "Unavailable" with no copy affordance.

Required CI at this exact head is green — run 34270205747: Desktop Core, all four Smoke E2E shards, and the integration aggregates.

@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Sep 8, 2026

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Reviewed: 44316ff72f5f7de014c66b01cbf534298a70c249..5f3a4a8111998c8aa41ad77cf66992bd1c85343c (delta from previously reviewed 2c68dddefcf16e2ee2cc3d0bae7564c5be6ac754; exact live head 5f3a4a8111998c8aa41ad77cf66992bd1c85343c)

Risk: medium — shared identity canonicalization and copy behavior on trust-decision surfaces.

The uppercase production fix is correct and causal: lowercase, uppercase, and whitespace-wrapped uppercase npubs now canonicalize, and removing the new uppercase prefix arm makes three focused tests fail. Two required regression seams remain non-falsifiable, and the stated parser casing contract is inconsistent.

Blocking findings

  1. The decoded npub identity-length gate remains unprotected (desktop/src/shared/lib/pubkey.ts:52-66). The helper/widget tests at pubkey.test.mjs:77-82 and PubKey.test.mjs:94-105 pass short hex or corruption through the pre-decode gate. Checksum-valid short npubs are tested only against the separate parser (parsePubkeyInput.test.mjs:45-50). Removing only !HEX_64_REGEX.test(decoded.data) leaves all three focused suites green (19/19), so <PubKey> can regress to displaying and copying a checksum-valid fake identity without a regression failing. That does not meet TESTING.md:25-31.

    Author action: pass npub1m6kmamcvty5gd and/or npub106246s directly through canonicalNpub and <PubKey> compact/full cases; assert neutral text and no copy affordance; mutation-prove removal of the decoded-length predicate fails behaviorally.

    Verification owner: author records mutation red/green; reviewer reruns the exact mutation.

  2. The new canonical-npub clipboard behavior is not tested (desktop/src/shared/ui/PubKey.tsx:41-68). PubKey.test.mjs:84-91 asserts only that a button exists; it never opens the compact popover or clicks the full copy control. Scoped search under desktop/src and desktop/tests found no interaction with Copy npub. The cited profile E2E at profile.spec.ts:1477-1486 clicks an outer profile field and expects raw agentPubkey, not this widget's npub-only CopyRow. Replacing the production clipboard value with literal wrong-identity leaves the entire widget suite green (3/3).

    Author action: test compact and full flows through open/click → exact canonical full-npub clipboard assertion → copied feedback; mutation-prove a wrong clipboard value fails; correct the misleading suite comment.

    Verification owner: author records mutation red/green; reviewer reruns it.

Additional contract defect

parsePubkeyInput lowercases before decode (desktop/src/shared/lib/nostrUtils.ts:38-46), so it accepts mixed-case npubs, while canonicalNpub rejects them. Exact production probes reproduced the split. This contradicts the PR body and pubkey.ts:46-50, which say mixed case remains invalid and the helper agrees with the parser.

Author action: either decode the original trimmed casing and add parser rejection coverage, or explicitly choose normalization and correct the advertised/helper contract, including the false agreement claim.

Verification owner: author unit test; reviewer reruns the production probe.

Contracts traced

Reviewed the changed-head delta and full changed-file/caller surface. parsePubkeyInput callers under desktop/src are member/channel invites, workflow author input, and Git-author profile hints; valid values still exit as normalized 64-character hex. <PubKey> canonicalizes only for rendering/copy and does not write state. Scoped review found no PR change to IPC commands, Tauri validation, relay payloads, persistence schemas, cache keys, or stored identity representation. The Rust comparison boundary remains hex-only validation at desktop/src-tauri/src/managed_agents/types.rs:865-890. Invalid widget values fail neutrally and remove interaction.

Exact-head validation

  • Focused three suites: 19/19 passed before mutation; predicate-removal mutation incorrectly remained 19/19 green.
  • Clipboard wrong-value mutation incorrectly remained 3/3 green.
  • Uppercase-prefix mutation produced 3 failures, establishing the current uppercase fix is causal.
  • cd desktop && pnpm test: 6458/6458 passed.
  • pnpm typecheck: passed.
  • pnpm check: passed with 4 warnings/5 infos outside the PR diff.
  • git diff --check 44316ff72f5f7de014c66b01cbf534298a70c249...HEAD: passed.
  • Exact-head GitHub Desktop Core, four Smoke shards, Windows/macOS builds, relay-backed integration, Semgrep, zizmor, and DCO: SUCCESS.

Manual/native evidence: not run; no GUI launch was authorized. This is reviewer/tooling confidence debt, not an additional defect.

Residual risk: after fixes, real OS clipboard contents and keyboard/focus return for both popover variants remain to be observed; intentionally deferred outer raw-hex surfaces remain outside this PR.

— :bot: Jude’s code review agent

loganj and others added 2 commits September 8, 2026 18:27
Add direct canonicalNpub null assertions for checksum-valid short npubs (npub1m6kmamcvty5gd, npub106246s) and route the same vectors through the existing PubKey widget invalid loop, replacing the false header claim that profile.spec.ts covered widget copy. Pin the parser pre-existing mixed-case npub normalization to hex, and document in pubkey.ts and nostrUtils.ts that canonicalNpub enforces strict Bech32 display casing while parsePubkeyInput case-normalizes input — both requiring a 64-hex identity payload. Production changes are comments only.

Co-authored-by: 627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz
Signed-off-by: Logan Johnson <loganj@squareup.com>
Recipient-inspection popovers portal their content to the document body
but still bubble click events through React's tree to the new-DM To-field
div, whose onClick focused the search input. Focusing that input dismisses
the popover via focus-outside, so ordinary clicks on the nested Copy npub
button detached the popover mid-click and never copied.

The field's onClick now only acts on clicks whose event target is a DOM
descendant of the field itself; portaled popover clicks keep their own
focus. Clicks physically within the field — including the To label —
still focus the input and open the recipient picker.

The clipboard regressions this unblocks pin both PubKey variants through
the mock bridge to the real browser clipboard: the new-DM recipient
verification flow (full variant: pointer copy, inspection survives the
copy, inner/outer Escape with the recipient retained, and a natural
keyboard Space-then-Enter path) and the agent-access owner hint (compact
variant), each expecting the exact canonical npub of the identity shown
— never the raw hex the popover also lists, and never a truncation. The
static PubKey suite header now names those E2E owners of the clipboard
interaction.

Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj

loganj commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Fix references for the changes-requested review at 5f3a4a811 — all three findings are addressed at the new head b3310c248:

  1. Decoded-length gate5c20712c: direct canonicalNpub null assertions for the checksum-valid short npubs (npub1m6kmamcvty5gd, npub106246s) and the empty payload, plus the same vectors routed through the <PubKey> widget invalid loop (compact and full: neutral label, no copy affordance). Removing only the !HEX_64_REGEX.test(decoded.data) predicate now fails these suites (red/green recorded).

  2. CopyRow clipboardb3310c24: both variants are now tested through the real browser clipboard (mock bridge) — the new-DM recipient verification flow (full variant) and the agent-access owner hint (compact variant) — each asserting the exact canonical npub of the identity shown, never the raw hex the popover also lists and never a truncation. Substituting a wrong-identity value fails them (red/green recorded). The suite comment now names these E2E owners of the clipboard interaction.

  3. Casing contract — normalization chosen and made explicit in 5c20712c (pubkey.ts, nostrUtils.ts): canonicalNpub and the display path enforce Bech32 casing as written (mixed case → null), while parsePubkeyInput retains trim + lowercase normalization and accepts mixed-case npubs; both require the decoded payload to be exactly a 64-char identity key. The parser's mixed-case-to-hex normalization is pinned by test, and the PR body's agreement claim is corrected to match.

  4. Portal focus steal (found while landing Initial release — Sprout Nostr relay with enterprise extensions #2) — b3310c24: recipient popovers portal to the document body, and their clicks bubbled to the To-field's onClick, which focused the search input and dismissed the popover mid-click — Copy npub never fired. The field's onClick now only acts on clicks whose target lies inside the field itself; the E2E covers normal pointer copy, the popover surviving the copy, inner/outer Escape with the recipient retained, and a natural Space-then-Enter path.

Re-review deferred to the exact-head green gate.

@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Sep 8, 2026
@loganj

loganj commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

@buzz-security-review b3310c2

@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Sep 8, 2026
@loganj
loganj requested a review from jedwards27 September 8, 2026 23:44

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — APPROVE at exact head b3310c24832b29d8ee90ea76a7878ac01be13ea3 against base 44316ff72f5f7de014c66b01cbf534298a70c249.

No concrete defects remain after changed-head review across systems/integration and product/UI/accessibility boundaries.

Verified contracts and behavior:

  • canonicalNpub and parsePubkeyInput fail closed on decoded payloads that are not exactly 32 bytes while preserving canonical lowercase npub output.
  • <PubKey> gates copy interaction on valid identity data, keeps keyboard/accessibility behavior, and clears owned timer state on unmount.
  • The new-DM focus guard distinguishes physical descendants from React-bubbled portal targets without breaking ordinary To-field focus, nested popover interaction, or Escape layering.
  • No persistence, IPC, relay payload, subscription, tenancy, or migration contract is changed.

Exact-head evidence:

  • Full Desktop tests: 6,459/6,459 passed.
  • Desktop typecheck and check passed; reported warnings/infos are outside this diff.
  • E2E build passed; focused clipboard/focus journeys passed 8/8, including pointer and keyboard copy, nested/outer Escape, popover survival, and ordinary label focus.
  • Payload-length and clipboard-value mutations caused the relevant tests to fail, confirming the regressions are detected.
  • Applicable required CI gates are green, including Desktop, Desktop E2E Integration, relay E2E, security, macOS, Windows Rust, and Desktop Release Candidate.

Confidence gap, not an author defect: native Tauri/OS clipboard and focus presentation was not directly exercised. The browser bridge journeys and screenshot evidence passed; residual risk is limited to native OS fidelity. Author action: none. Verification owner: reviewer/native tooling.

This approval applies only to the exact head above; a new head requires re-review.

@loganj
loganj merged commit bfc3848 into main Sep 9, 2026
73 checks passed
@loganj
loganj deleted the fix/desktop-npub-identity-d1a branch September 9, 2026 14:32
loganj added a commit that referenced this pull request Sep 9, 2026
…w surfaces (#7495)

🤖
## Summary

Every Buzz account is identified by a long public key. Before this
change, when someone had no display name, surfaces fell back to
inconsistent labels — mostly raw hex fragments like `abcd1234…wxyz`,
sometimes a generic role label with no key — so the same person looked
different from surface to surface, and nothing looked like an npub
address. This PR applies the npub identity foundation from #7488 to the
everyday surfaces: a person without a display name now falls back to the
same compact npub everywhere — `npub1xxxx…yyyy`, the human-readable
spelling of their public key (first 8 + last 4 characters of the full
npub) — across messages and mentions, reactions, huddles, member and
participant lists, the sidebar and channel activity, search, projects,
tray, notifications, and workflow surfaces.

- **Mentions and messages**: key-only mention chips render the compact
npub. Pasting a copied mention back still re-binds it byte-exactly to
the identity it declares, for both the new npub chips and legacy
hex-truncated chips copied by older clients — wrong, missing, or
tampered key qualification is rejected instead of silently degrading to
plain text.
- **Reactions and huddles**: huddle reaction events and the huddle
roster/participants render the compact npub for unnamed participants;
workflow reaction triggers describe authors with the same form.
- **Members and sidebar**: channel and community member lists,
add-member results and invites, the members sidebar, the
channel-activity popover, search, projects (assignees/reviewers/PR
panels), the tray menu, and desktop notifications all fall back to the
compact npub; titles and aria labels keep the machine-readable full
labels.
- **Profile labels**: panel/popover display names and owner handles fall
back to the compact npub (never raw hex) when there is no name;
linked-event (nevent) message metadata shows the npub-shaped author
fallback while the event lookup and event IDs are unchanged.
- **Workflows**: author-picker secondary labels, step destination keys,
and trigger-author references render compact npubs; event and blob IDs
keep their existing hex compacts (they are not identities).
- **Avatars stay distinct**: fallback avatars for key-only identities
derive initials from the key's tail, so prefixed role labels like
"Participant npub1…" no longer collapse every unnamed participant onto
the same initials; people with names keep their name initials.

Preserved exactly: display names and distinct avatars, internal hex keys
(storage/API forms unchanged), clipboard identity roundtrips, event/blob
ID compaction, private keys (no nsec path is touched), and nevent link
handling.

Scope: this PR changes what identity labels **display**, not identity
controls — profile/settings copy controls, the respond-to allowlist,
workflow key fields, and agent dialogs are the sibling slice #7489, and
the shared primitives (`canonicalNpub`, `truncateNpub`, the `<PubKey>`
gate, strict input parsing) come from the foundation #7488.

### Related issue

- Fixes: N/A. Searched existing issues/PRs for duplicates — none found;
the related work is the npub identity stack this slice belongs to.
- Base/dependency: stacks on #7488 (foundation) — this PR does not stand
alone on main.
- #7489 is a sibling slice on the same #7488 base
(profile/agent/workflow controls), not a dependency: this PR does not
require #7489, and #7489 does not require this PR — both only require
#7488.

### Testing

At exact head `4763cbeae1dd521309755e6d61f657324cb98667` (base:
`fix/desktop-npub-identity-d1a` @
`5f3a4a8111998c8aa41ad77cf66992bd1c85343c`; 71 files, +656/−189 —
production +277/−136, test support +379/−53):

- At this head: targeted `mentions.spec.ts` (1/1), the e2e build,
typecheck, and biome — green.
- 9 changed/related unit files: 100/100 green; typecheck, e2e build,
biome, and px text/truncation checks clean; huddle-roster focused run
green; channel-activity e2e 11/11; mutation checks confirm the fallback
wiring (removing it collapses shared initials and drops fallback rows).
- Known pre-existing local e2e failures, unchanged by this PR and
reproduced identically at the upstream merge-base: huddle-transcription
voice-menu attribution (25 pass / 1 fail) and the
`workflow-local-controls` 438px caret drift. Not claimed green locally.
- Update at head `236af9e6137386737e84d3a474d6bc808a704c50` (test-only
follow-ups `1143af345` + `236af9e6`): the `workflow-local-controls`
races were fixed in the test drivers, and the 438px diff was shown to be
a stale Darwin snapshot baseline (name-row enable switch already absent
and `message_posted` already MessageSquare at recording commit
`9390e11c9`) and refreshed — the focused screenshot test, including
keyboard/caret assertions, now passes locally (twice). The full spec was
not rerun after the snapshot refresh; the huddle-transcription item
above is unchanged.

Label/copy text changes are asserted by the e2e specs (`mentions`,
`mention-recipients`, `pubkey-display-screenshots`,
`huddle-transcription`, `channel-activity-popover`,
`workflow-local-controls`) rather than new screenshots; the screenshot
spec pins the compact npub text forms.

### Task provenance

Buzz channel: `1f0e4a3d-7e01-4efe-bb16-843b357f85c9`

Task:
buzz://message?channel=1f0e4a3d-7e01-4efe-bb16-843b357f85c9&id=86b34eb4bd84a1472419e9af22636c011c0fe273e3c196f967d7a36996e149b6

---------

Signed-off-by: Logan Johnson <loganj@squareup.com>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
loganj added a commit that referenced this pull request Sep 9, 2026
…flows (#7489)

🤖
## Summary

Building on #7488's npub foundation, this PR finishes the identity
display change for the controls where you actually manage people and
keys: profile, settings, agent access, and workflows. Everywhere in
these surfaces, an identity key shows — and copies — as its canonical
npub (npub is the human-readable encoding of a Nostr public key: the
compact `npub1j57...fjmv` form where space is tight, the full npub where
the whole key matters), and accepts npub as input.

After this change:

- Profile panel: the public-key row and the managed-by / declared-owner
copies show the full npub. If a key can't be encoded, you see
"Unavailable" with no copy button — never a raw or partial key.
- Settings: the identity card shows and copies the npub. The
hosted-communities account identity derives from the bound key
(`pubkey_hex`) — the same authority as the mismatch gate and hosted
operations — so the display can never disagree with what the app acts
on; an unusable hex falls back to a neutral label instead of rendering
the unverified server npub. The connected claim and a community's
Connect action require that same usable bound key to match the local one
— with no usable binding the card cannot claim connected or start
Connect, while the community list, linking, and delete/rebind recovery
stay available.
- Hosted create/onboarding: the account and device identity rows in the
create flow and owner onboarding derive from the same authoritative
fields (bound key / local key), with the same neutral fallback;
readiness requires a usable bound key that matches the local one.
- Respond-to allowlist (controls who may respond to an agent): entries
can be typed or pasted as hex or npub; both spellings of the same key
are recognized as one entry and dedupe. Search results, chips, and
remove buttons use the compact npub.
- Workflow key fields: to/from keys display as npubs in the form and
save back as canonical hex. Templates like `{{trigger.author}}`, roles,
and free text pass through untouched; placeholders accept both
spellings.
- Recipient and agent dialogs: the verify popover is npub-only (the
raw-hex line is gone); denied-membership screens never show a raw key.
- The Rust-side truncated display name (used for native surfaces) shows
the same compact npub, so those surfaces match the web UI.

Internal representation is unchanged: keys are still stored, sent, and
matched as canonical 64-character hex — npub is a display and input
spelling, normalized to hex at the boundary, so existing data and
integrations keep working. Bound-key usability and comparison use one
normalized form (trimmed, lowercased, 64 hex characters; npub rejected),
so padded or mixed-case spellings of the same key match. Display names,
private keys, and event IDs are untouched.

## Details

- `respondToAllowlist` / `RespondToField`: npub entries normalize to
canonical hex; cross-form dedupe; compact npub in rows and chips;
direct-add accepts npub and stores canonical hex.
- `workflowFormTypes` / `WorkflowStepCard`: hex → npub for display, npub
→ canonical hex on save; templates, roles, and free text pass through in
both directions (roundtrip-tested).
- `UserProfilePanelFields`, `ProfileSettingsCard`,
`HostedCommunitiesSettingsCard`, `MembershipDenied`,
`SelectedRecipientChip`, `AddAgentToChannelDialog`: npub display and
copy; invalid keys → "Unavailable" with no copy; hosted identity rows
derive from the bound `pubkey_hex` (create/onboarding rows from the
bound and local keys), never the unverified server npub;
connected/readiness/Connect gates use the same usable-bound-key
predicate, and the settings Connect invocation callback re-checks it
before starting.
- `src-tauri/src/commands/identity.rs`: `truncated_display_name`
compacts to the first 8 + last 4 characters of the npub (above a 12-char
threshold), mirroring `truncateNpub`.
- e2e: profile key rows and clipboard polls assert npub forms and
raw-hex suppression; the display-screenshots spec pins the npub-only
popover; hosted specs drive the real settings card, create flow, and
onboarding rows through their real providers, and the unlinked/npub-only
identity cases assert no connected claim and no Connect action.

### Related issue

- Fixes: N/A. No separate issue; the related work is the stack below.
- Stack: builds on #7488 (shared npub foundation), now merged; this PR
is rebased onto main and stands on its own.

### Testing

At head `303c90ffa` (base: main `bfc38485`; 24 files, +1125/−146):

- Focused unit suites (respondToAllowlist, workflowFormTypes,
hostedCommunityApi bound-key helpers) green; mutation-checked — dropping
allowlist canonicalization fails the dedupe case, and dropping bound-key
normalization fails the npub-in-hex and padded same-key cases.
- Full desktop unit suite 6,477/6,477, `desktop-typecheck`,
`desktop-check` (formatting fixed narrowly with `biome check --write` on
the touched files only), and a fresh E2E build at the current head; the
add-community + hosted-communities-settings specs 18/18 and onboarding
integration 69/69 on a fresh dedicated port, with focused new-case runs
4+4 covering padded same-key (ready, Connect kept — no false rebind) and
npub-in-hex (neutral label, recovery, no Connect) across the settings
card, create flow, and first-community onboarding, plus the
unlinked-account settings regression asserting Connect cannot occur.
- `cargo fmt`/clippy (both feature sets) and `cargo test identity` (71
pass) passed at the earlier full-change head; since then, the only
production changes in this PR's delta are the hosted identity display
authority and its fail-closed bound-key gating/normalization above
(base-side fixes carry #7488's receipts) — every other change is
test-only.

### Task provenance

Buzz channel: `1f0e4a3d-7e01-4efe-bb16-843b357f85c9`

Task:
buzz://message?channel=1f0e4a3d-7e01-4efe-bb16-843b357f85c9&id=86b34eb4bd84a1472419e9af22636c011c0fe273e3c196f967d7a36996e149b6

---------

Signed-off-by: Logan Johnson <loganj@squareup.com>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
loganj added a commit that referenced this pull request Sep 9, 2026
🤖

## Summary

In the mobile app, anyone who hasn't set a display name shows up as a
raw 64-character hex key (e.g. `3a5d4f9c…`) — unreadable, and
unrecognizable as the same identity across screens. Profile and Settings
also let you copy that raw hex. Nostr public keys have a standard
readable form — `npub1…`, the same encoding other Nostr apps and our
desktop app already display. This PR makes every mobile identity surface
render npub instead:

- **Unnamed people everywhere** — message and thread authors, reactions,
typing indicators, member lists, channel details, DM headers and tiles,
inbox, search, forum cards, Pulse notes and reply context, mention
suggestions, and invite rows — now show a compact npub label: first 8 +
last 4 characters of the full npub joined by an ellipsis
(`npub1abcd…wxyz`), the same truncation desktop uses. Previously these
showed truncated raw hex.
- **DM fallback avatars and blank names** — 1:1 DM tiles and headers key
their fallback avatar to the same non-self counterpart the label names,
including self-first participant order; a self-DM keeps its
hex-key-derived initial. Blank or whitespace-only display names fall
back to the compact npub instead of rendering empty, while nonblank
authored names render verbatim (padding included).
- **Profile sheet → "Copy public key"** now copies the full canonical
npub — never raw hex. When the identity string isn't a valid public key,
the copy tile is disabled, so a malformed key never reaches the
clipboard.
- **Settings → Identity (pubkey)** displays and copies the full npub; an
invalid identity reads "Identity unavailable" with copy disabled.
- **Invalid identities never leak truncated raw hex** into the UI
anywhere — they render a neutral "Unknown identity" label.
- **Unchanged on purpose:** display names and verified handles (NIP-05 —
the `name@domain` badge) still render as before. Unnamed avatars keep
distinct per-key initials, derived from the underlying hex key rather
than the npub — otherwise every unnamed key would render the same "N"
initial. Event IDs are not public keys, so they keep their hex
truncation (in Pulse's "Replying to", the parent author shows npub while
an event-id fallback still shows hex). The nevent share link, private
keys, and internal hex storage are untouched. Inputs that accept a key
(invite/member entry) accept both hex and npub and keep working in hex
internally.

### Related issue

N/A. Searched open issues/PRs for npub identity display on mobile —
closest related: none found. Desktop's parallel npub standardization
lives in the stacked desktop PRs (#7488 foundation, #7489 controls,
#7495 display surfaces); this is the independent mobile slice (based
directly on `main`, not on those branches).

### Testing

At exact head `5a620e420a1fd57d9d8011ac26434eed32fcf765` (base: `main`
`44316ff72`; 40 files, +1,345/−154):

- Full mobile suite: 2,098 tests passing (`cd mobile && flutter test`);
`flutter analyze` clean; `dart format --set-exit-if-changed .` clean —
the same checks CI runs.
- Widget/unit coverage at production seams: compact labels and hex-keyed
avatar initials for DM headers/tiles, member rows, mention suggestions,
and Pulse reply context; DM fallback avatars keyed to the labeled
counterpart (self-first order and self-DMs); blank/whitespace
display-name npub fallback with nonblank authored labels verbatim,
including the Activity inbox sender and profile-sheet heading (each with
its own empty/whitespace production-seam regression); full-npub copy and
disabled-copy semantics in profile and settings; invalid-key
suppression; and hex↔npub input round-trips.

Verified via unit and widget tests — no device/simulator validation is
claimed.

### Task provenance

Buzz channel: `1f0e4a3d-7e01-4efe-bb16-843b357f85c9`

Task:
buzz://message?channel=1f0e4a3d-7e01-4efe-bb16-843b357f85c9&id=86b34eb4bd84a1472419e9af22636c011c0fe273e3c196f967d7a36996e149b6

---------

Signed-off-by: Logan Johnson <loganj@squareup.com>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
mfethe1 added a commit to mfethe1/buzz that referenced this pull request Sep 9, 2026
Upstream block#7488/block#7493 made the compact npub the canonical identity label
and made shortPubkey return 'Unknown identity' for non-key strings. Three
fork tests still expected raw-hex truncation or accepted invalid keys:
update fixtures/expectations to the npub contract. Behavior under test
(plain-text actor labels, digest author naming) is unchanged.

Signed-off-by: Michael Feth <mfethe1@gmail.com>
jrobotham-square added a commit to jrobotham-square/buzz that referenced this pull request Sep 9, 2026
…stody

* origin/main:
  fix(desktop): order unnamed roster members by full canonical npub (block#7503)
  fix(mobile): standardize public-key identity display on npub (block#7493)
  fix(desktop): npub identity controls across profile, agents, and workflows (block#7489)
  fix(desktop): npub identity displays for mention, member, and workflow surfaces (block#7495)
  fix(desktop): shared npub identity foundation (canonicalNpub, PubKey gate, strict parser) (block#7488)
  fix(mobile): render push notification sender identity as npub (block#7494)

Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex-security-review-current The posted Codex security review matches its recorded range.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants