fix(desktop): canonicalize mesh connect-request #p target before signing#835
Closed
tlongwell-block wants to merge 1 commit into
Closed
fix(desktop): canonicalize mesh connect-request #p target before signing#835tlongwell-block wants to merge 1 commit into
tlongwell-block wants to merge 1 commit into
Conversation
The relay rejects a kind:24621 connect request whose target is unusable with "mesh connect request missing #p target". Today the desktop signs `["p", input.targetPubkey]` verbatim from discovery; a non-canonical value (uppercase, whitespace, or an npub) would be signed as-is. Harden the single signing boundary in publishMeshConnectRequest with a new canonicalPubkeyOrThrow() that accepts hex (any case, padded) or npub1…, returns canonical 64-char lowercase hex, and throws a clear local error otherwise — instead of emitting an event the relay rejects opaquely. Also make the e2e mock relay reject a tagless 24621 rather than blindly ACK it, so the missing-#p shape is covered in tests. Defense-in-depth + test-harness hardening. NOT a confirmed fix for a specific reported "missing #p" — that error requires a genuinely tagless 24621, which no current source emits; root cause there is most likely a stale desktop build (pending build-SHA confirmation). Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co>
Collaborator
Author
|
Closing in favor of #834 (Max's) — we both built this in parallel (my fault for not locking the lane). His is the better diff: a real e2e Playwright test that captures the signed 24621 and asserts the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Canonicalize the mesh connect-request (
kind:24621)#ptarget at the single signing boundary, and stop the e2e mock relay from masking a missing-#pregression.Why
Starting a mesh-powered agent can fail with the relay error:
Today
publishMeshConnectRequestsigns["p", input.targetPubkey]verbatim from discovery. A non-canonical value (uppercase, surrounding whitespace, or annpub1…) would be signed as-is and then mishandled downstream. We harden the boundary so the desktop always emits a canonical["p", <64-hex>]tag — or fails locally with a clearinvalid pubkeyerror — instead of producing an event the relay rejects opaquely.Changes
pubkey.ts— newcanonicalPubkeyOrThrow(value): accepts hex (any case, padded) ornpub1…, returns canonical 64-char lowercase hex, throws a clear error otherwise. Reuses the already-presentnostr-tools/nip19decoder.relayMeshSignaling.ts—publishMeshConnectRequestruns the target throughcanonicalPubkeyOrThrowbefore signing.e2eBridge.ts— the mock relay now rejects a24621with no#ptag (mirroring the real relay) instead of blindly ACKing it, so the missing-#pshape is actually exercised.pubkey.test.mjs— 9 cases: hex passthrough, uppercase, whitespace, npub→hex, and the throw paths (empty / short / garbage / malformed npub).Verification
pnpm test(full desktop suite): 479/479 passtsc --noEmit: cleanbiome check: cleanScope / honesty
This is defense-in-depth + test-harness hardening, not a confirmed fix for a specific reported "missing #p" error. That exact error requires a genuinely tagless
24621, which no current source emits — investigation (Eva + Max) points to a stale desktop build as the likely cause of the live report, pending build-SHA confirmation. This PR makes the class of failure impossible-by-construction and adds the missing regression coverage regardless of that root cause.