Skip to content

fix(sdk): verify React Native wallet response delivery - #485

Open
sirdeggen wants to merge 5 commits into
mainfrom
claude/issue-483-resolution-pdrr5e
Open

fix(sdk): verify React Native wallet response delivery#485
sirdeggen wants to merge 5 commits into
mainfrom
claude/issue-483-resolution-pdrr5e

Conversation

@sirdeggen

@sirdeggen sirdeggen commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Program and scope

  • Tracker or issue: ReactNativeWebview origin validation #483 (ReactNativeWebView origin validation)
  • Program gate(s) advanced: SDK transport trust boundary; repository quality gate (zero new Sonar findings)
  • Why this change is needed: the ReactNativeWebView substrate resolved a BRC-100
    invocation from any message event whose payload matched the request identifier.
    It never established that the message came from the React Native bridge, and it
    compared origins only when a caller configured an exact domain — so the default
    wildcard target that WalletClient discovery uses (new ReactNativeWebView('*', …))
    accepted a wallet response from any browsing context sharing the page. The state
    SonarCloud had analysed when ReactNativeWebview origin validation #483 was filed (55f9eb2, the commit before fix: normalize BRC-100 bytes across wallet transports #482
    merged) had no origin check at all; fix: normalize BRC-100 bytes across wallet transports #482 added the configured-domain comparison
    minutes later, and this change completes the hardening by verifying delivery
    before the payload is read.
  • Explicitly out of scope: the XDM substrate (already verifies isTrusted,
    e.source === window.parent, and origin); BRC-100 envelope/wire format; the
    ts2md-generated packages/sdk/docs/reference/*.md files, which are stale
    repo-wide and would add ~2,500 unrelated lines if regenerated here.
  • Exact head SHA reviewed: 7cebd63cf2b437c60e48a28670a8db14ef72af0b

Impact

  • No public package source or manifest changed
  • Public package source or manifest changed; affected packages are listed below
  • Infrastructure source, dependency, image, or deployment configuration changed
  • Public API, exports, types, runtime targets, or browser/mobile behavior changed
  • Security-sensitive boundary changed
  • Documentation or examples changed

Affected packages/services and intended patch versions (publication occurs only
through the release workflow after approval):

  • @bsv/sdk — source 2.4.1, published 2.4.0, release type patch (already the
    pending patch; no further bump required). No exported signature changes: the
    constructor, invoke, error identities, and the CWI envelope are unchanged.
  • @bsv/wallet-toolbox-client — no source change; its browser platform budget is
    ratcheted for the SDK growth (evidence under Verification).

What the listener now requires

A React Native host answers by injecting the response into the document that made
the call, so a response must arrive that way. isBridgeDelivered checks this
before JSON.parse(e.data) — an untrusted context's payload is never read:

  1. Delivery — the event carries no source (a host-synthesized MessageEvent),
    comes from this window (window.postMessage injected by the host), or comes from
    the frame bridging for it (window.parent, which is this window in a top-level
    document). Framed documents, openers, and sandboxed frames reporting the opaque
    "null" origin are ignored.
  2. Origin, where the browser attests it — a relaying host frame is a separate
    browsing context, so its origin is browser-attested and has to be this document's
    origin (window.origin) or the configured wallet origin. An event synthesized
    inside this document is same-origin by construction, and the origin a host stamps
    on it is the host's own choice rather than an attestation, so it is accepted as
    before.
  3. Pinning — an exact configured domain still rejects a mismatched non-empty
    origin with the same error string, unchanged from main.

Compatibility (BRC-100 / React Native)

Unchanged on the wire: one stringified CWI envelope over
window.ReactNativeWebView.postMessage, responses matched by type, id, and
isInvocation, the same WalletError(description, code) and byte normalisation.
Host behaviours that keep working, each covered by a test:

  • bridge injections with no origin and no source (injectJavaScript +
    dispatchEvent);
  • window.postMessage(response, '*') injected into this document;
  • a synthesized event that sets source to this window with an empty origin or a
    host-stamped token such as react-native;
  • host-synthesized events that stamp a vendor origin the browser never attested
    (the existing WalletClient expectation that a BRC-100 originator is not a
    MessageEvent origin);
  • responses relayed by a same-origin host frame, or by a host frame on the
    configured wallet domain;
  • an exact configured domain, including the schemeless and port forms.

Verification

  • Local commands and results (Node 24.19.0, pnpm 10.33.2):
    • pnpm health:check — pass (38 projects, 0 contract findings, 0 control errors),
      re-run after merging main into the branch
    • pnpm lint — pass (oxlint, --deny-warnings)
    • pnpm format:check — pass
    • pnpm build && pnpm typecheck — pass across all workspace packages
    • pnpm --filter @bsv/sdk test — 156 suites, 5,924 tests pass
    • node ../../scripts/check-browser-package.mjs . (SDK) — pass
    • pnpm --filter @bsv/wallet-toolbox-client run test:browser — pass
    • pnpm --filter @bsv/wallet-toolbox-mobile run test:mobile — pass
    • The three new negative tests were confirmed to fail against the pre-change
      implementation (3 failed, 20 passed) and pass after it.
  • Hosted CI: all 41 checks on 7cebd63 are terminal and successful (run
    31925984019),
    including merge-gate, Quality gate — zero new Sonar findings, CodeQL,
    Platform / wallet browser, Platform / wallet mobile, Platform / browser packages, every coverage shard, mutation, and affected-dependent tests. Skips are
    scope-based and validated by the merge gate. 7cebd63 merges current main into
    the branch to clear a behind state; main's only new commit was the published
    version-sync automation, which touches no file this PR changes. The same 41 checks
    were already green on ec3b250 (run
    31899287547).
    Earlier heads each failed once and were diagnosed rather than waived:
    • 20eecdc — quality gate: 1 new Sonar finding. The inline delivery guards raised
      the listener's cyclomatic complexity from 9 to 15, past the profile threshold.
      5258384 factored them into isBridgeDelivered (listener back to 10, predicate
      5) and hoisted repeated test literals into constants so no duplicated-literal
      finding replaced it; Sonar then reported 0 new issues.
    • 5258384Platform / wallet browser: esbuild browser bundle raw size 1252526 exceeds budget 1252500. Reproduced locally byte-for-byte and fixed in
      ec3b250.
  • Conformance evidence: no portable wire behaviour changed, so no vector applies.
    The BRC-100 byte-boundary contract test for this file still passes.
  • Coverage delta: Codecov reports all modified lines covered on this head.
    ReactNativeWebView.ts locally: 98.5 % statements, 91.66 % branches, 100 %
    functions, 100 % lines. packages/sdk global 94.76 / 87.08 / 95.81 / 95.51
    against 85/80/85/85 thresholds. 9 tests added.
  • Lint/typecheck delta: 0 findings, 0 errors.
  • Browser/mobile/packed-consumer evidence, measured from the exact packed tarballs:
    • @bsv/sdk esbuild raw 559,731 → 559,887 (limit 560,000); Vite raw
      741,120 → 741,345 (limit 742,000); UMD raw 554,475 → 554,625 (limit
      555,000). No SDK budget was raised.
    • @bsv/wallet-toolbox-client esbuild raw 1,252,506 and gzip 345,035 against
      previous ceilings of 1,252,500 and 345,000 — 6 and 35 bytes over. Ceilings
      ratcheted to 1,253,000 and 345,500; brotli 277,222 and all three Vite
      figures (1,606,919 / 377,313 / 296,637) stay inside their unchanged
      ceilings. Composition is unchanged: 101 Vite modules across @bsv/sdk,
      @bsv/wallet-toolbox-client, hash-wasm, idb; 167 esbuild modules.
    • @bsv/wallet-toolbox-mobile metro 1,660,280 and hermes 3,366,013 raw stay
      inside unchanged ceilings.
    • The alternatives to the ratchet were measured, not assumed: reading
      window.origin instead of window.location?.origin reclaimed most of the
      growth, reformulating the predicate as a single expression saved zero bytes, and
      the only larger saving available was dropping the host-frame relay allowance,
      which iframe consumers with a shimmed bridge depend on. Reviewers who prefer the
      tighter budget can ask for that allowance to be dropped instead.
  • I self-reviewed the complete diff for correctness, security,
    compatibility, public API, artifacts, dependencies, docs, and operations
  • All applicable checks are terminal and successful on the exact head; any
    scope-based skip is expected and validated by the merge gate

Security and dependencies

  • No dependency or lockfile change
  • Changelog, runtime relevance, peer compatibility, transitive graph, and
    audit results were reviewed — not applicable, no dependency change
  • CodeQL/negative tests cover any changed trust boundary
  • The exact-head CodeQL analysis has no new alert
  • The exact-head repository quality gate reports zero new Sonar findings
    (including accepted or false-positive issue states) and zero unreviewed hotspots;
    Sonar's aggregate Quality Gate passed verdict alone is not merge evidence —
    the repository's own Quality gate — zero new Sonar findings job passed on
    7cebd63, with 0 new issues, 0 accepted issues, and 0 security hotspots
  • No new override, advisory dismissal, quality suppression, or skipped test —
    the browser budget ratchet is a measured baseline change under
    governance/browser-artifact-policy.json, not a suppression
  • Any temporary exception is registered with owner, evidence, review date,
    and removal condition — none was added
  • Workflow permissions and lifecycle-script behavior remain least privilege

Note on the Sonar evidence: sonarcloud.io is blocked by the egress policy of the
environment this change was developed in, so individual issues could not be opened
from here; findings were reproduced locally with eslint-plugin-sonarjs and
confirmed against the hosted gate. The finding class behind #483 was reconstructed
from the analysed commit: only XDM.ts and ReactNativeWebView.ts register
message listeners, and at 55f9eb2 the latter read e.data with no origin check
whatsoever — the S2819 "verify the origin of the received message" shape.

Dependency evidence

Not applicable; no dependency, lockfile, or manifest change.

Release and operations

  • No npm publication was performed from a workstation or from this PR
  • Required npm patch bumps are included or intentionally deferred by the
    controlling program — @bsv/sdk already carries the pending 2.4.1 patch;
    @bsv/wallet-toolbox-client ships no source change
  • Image/SBOM/provenance/deployment/rollback impact is documented — none; no
    container, service, or deployment surface is touched
  • Documentation, changelog, migration, and operational guidance are current —
    the @bsv/sdk changelog, governance/package-release-notes.json, and the
    regenerated docs/reference/package-api-migrations.md are current, and the
    class TSDoc documents the new policy. packages/sdk/docs/reference/wallet.md
    is ts2md output that is already stale against main for unrelated reasons;
    regenerating it belongs in a dedicated change.

Completion evidence

  • The linked tracker is updated only for work fully proved by merged code,
    passing checks, resolved alerts, measurements, or an approved exception —
    ReactNativeWebview origin validation #483 will be updated after merge

The ReactNativeWebView substrate resolved a BRC-100 invocation from any
message event whose payload matched the request identifier. It never
established that the message came from the React Native bridge, and it
compared origins only when a caller configured an exact domain, so the
default wildcard target that WalletClient discovery uses accepted a
response from any browsing context sharing the page.

A React Native host answers by injecting the response into the document
that made the call, so the listener now requires that delivery shape
before it reads the payload: the event carries no source (a
host-synthesized event), comes from this window, or comes from the frame
bridging for it, and a browser-attested origin has to belong to this
document or to the configured wallet origin. Framed documents, openers,
and sandboxed frames reporting an opaque origin can no longer answer an
invocation.

BRC-100 framing is untouched: the invocation is still one stringified
CWI envelope over window.ReactNativeWebView.postMessage, responses are
still matched by type, identifier, and isInvocation, and an exact
configured domain keeps pinning responses with the same error. Bridge
injections without an origin, window.postMessage responses, and
host-stamped vendor origins continue to resolve, so hosts need no
change.

Refs #483

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aQnVKCDEecajUHTV2RSNG
@cursor

cursor Bot commented Aug 15, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@codecov

codecov Bot commented Aug 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

claude added 4 commits August 15, 2026 05:35
The delivery check compared a browser-attested origin whenever the event
carried a source. A host that synthesizes its response inside the WebView
document may set that source to this window while stamping an origin the
browser never attested, or leaving it empty, so strict comparison could
drop a conforming BRC-100 response and fail wallet discovery on mobile.

Origin comparison now applies only where the browser actually attests it:
a message relayed by the host frame, which is a separate browsing
context. A response injected into this document is same-origin by
construction, so the origin the host stamps on it is accepted, exactly as
before this branch. An exact configured domain still pins every response.

Inlining the delivery predicates back into the listener also keeps the
esbuild browser bundle inside its budget: 559,877 raw against a 560,000
limit, where the helper functions had put it 7 bytes over.

Refs #483

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aQnVKCDEecajUHTV2RSNG
The exact-head Sonar analysis reported one new finding on this branch: the
delivery guards raised the response listener's cyclomatic complexity from
9 to 15, past the profile's threshold.

The guards move into one isBridgeDelivered predicate, which reads better
than the inline conjunction chain and returns the listener to 10 with the
predicate itself at 5. The tests hoist the origins and the host-frame stub
they now repeat into constants so no new duplicated-literal finding takes
its place. Behaviour, wire format, and error identities are unchanged, and
the esbuild browser bundle stays inside its budget at 559,907 raw bytes
against a 560,000 limit.

Refs #483

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aQnVKCDEecajUHTV2RSNG
The wallet client browser contract failed on the exact head: the SDK's
delivery check grew the packed @bsv/wallet-toolbox-client bundle past two
esbuild ceilings that sat within a few dozen bytes of the measurement.

Reading window.origin instead of window.location?.origin is the same
document origin per the HTML standard, in fewer bytes, and reclaims most
of the growth. What remains is the irreducible cost of verifying that a
response reached this document the way the React Native bridge delivers
one, so the client budget is ratcheted with its measurement rather than
dropping the check or the host-frame relay that iframe consumers depend
on. Reformulating the predicate as one expression was measured too and
saved nothing, so the clearer early-return form stands.

Measured for @bsv/wallet-toolbox-client, esbuild: raw 1252506 against a
new 1253000 ceiling, gzip 345035 against a new 345500, brotli 277222
inside its unchanged 277300. Vite raw 1606919, gzip 377313, and brotli
296637 stay inside their unchanged ceilings, as do both wallet mobile
profiles. Composition is unchanged at 101 vite modules across @bsv/sdk,
@bsv/wallet-toolbox-client, hash-wasm, and idb.

Refs #483

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aQnVKCDEecajUHTV2RSNG
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants