Skip to content

feat(swarm): window.swarm messaging extension (PSS + GSOC) - #165

Merged
meinharrd merged 13 commits into
mainfrom
feature/window-swarm-enhancements
Aug 12, 2026
Merged

feat(swarm): window.swarm messaging extension (PSS + GSOC)#165
meinharrd merged 13 commits into
mainfrom
feature/window-swarm-enhancements

Conversation

@flotob

@flotob flotob commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements the messaging companion SWIP against Ant v0.5.42's new light-node PSS/GSOC support: five new provider methods — swarm_getMessagingIdentity, swarm_subscribe, swarm_unsubscribe, swarm_sendPss, swarm_sendGsoc — plus the provider's first streaming surface, an EIP-1193-style message event fed by main-process subscription WebSockets relayed to the subscribing webview.

Built to spec revision 50ec686 (flotob/SWIPs master): required PSS targets, truncated pssTarget (the node-global overlay never crosses the page boundary), retryable 4900 on node-capacity exhaustion, empty payloads accepted for PSS / rejected as invalid_payload for GSOC, and no raw-address GSOC send.

Architecture

Follows the existing four-layer provider pipeline (page → renderer consent → main authority → services):

  • messaging-service.js (new) — bee-js-backed identity/sends and deterministic GSOC topic derivation (freedom-gsoc-v1 profile: keccak identifier + namespaced target overlay + mined owner key, cached and bounded). Subscribe sockets are hand-rolled on the Node-native WebSocket rather than bee-js pssSubscribe/gsocSubscribe: bee-js discards WS close codes, and the node signals lurker-pool exhaustion via close 1013 with a reason, which subscribers must distinguish from transient closes. Reconnects with exponential backoff; per-socket payload dedup.
  • subscription-registry.js (new) — Electron-free lifecycle state with injected collaborators: multiplexes one socket per (kind, key) against the node's 8-neighborhood lurker pool, enforces the per-origin cap, tears down on unsubscribe, main-frame navigation, webContents destruction, and permission revocation (via a revoke hook the provider layer registers with the permission store).
  • swarm-provider-ipc.js — full re-validation with the spec's error vocabulary, the new messaging permission tier, capabilities features: ['messaging'] + limits (maxMessageBytes 4000 / maxTargetDepth 3 / maxSubscriptions 32), and message delivery targeted at the subscribing webview's webContents.
  • Renderer/UI — messaging consent tier (grant prompt + per-send approval with a messaging auto-approve), mirroring the publish/feed prompt patterns.

Verification

  • 2,200+ unit tests green (60+ new), lint clean.
  • Live against a stamped antd 0.5.42 on Gnosis mainnet: GSOC broadcast round-trip (send → pushsync → lurker pull → message delivery, sent.address === subscribe.key), directed PSS delivery including a zero-byte ping, slot-refusal mapped to retryable 4900, subscription establishment and clean teardown.

Also included

  • chore(build): bundled Ant bumped to v0.5.42 (pinned tag + SHA256SUMS trust digest).
  • fix(build): sample config/ant.yaml booleans unquoted — antd ≥ 0.5.42 parses its config strictly and refused ant:init-provisioned nodes.

Notes for reviewers

  • The GSOC derivation constants in messaging-service.js define address stability for every room created through this provider — treat that block as frozen once shipped.
  • Messaging sends are deliberately not recorded in publish history (volume; ephemeral). Follow-ups tracked separately: messaging-grant listing/revocation in the permission-manage UI, and the PSS ?neighborhood= rendezvous override our WS client could pass for many-to-many PSS rooms.

🤖 Generated with Claude Code

flotob and others added 5 commits July 15, 2026 17:56
Pin the new release together with the SHA256SUMS trust digest. This picks up the upstream PSS + GSOC messaging support (send and receive on a light node, bee-interoperable) that the upcoming window.swarm messaging API will build against.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
antd 0.5.42 parses its YAML config strictly and rejects string values
for boolean keys, so the quoted booleans in the sample config broke
ant:init-provisioned nodes. ant-manager's runtime template has always
written them unquoted and is unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements the messaging companion SWIP against Ant v0.5.42's new
PSS/GSOC light-node support: swarm_getMessagingIdentity, swarm_subscribe,
swarm_unsubscribe, swarm_sendPss, swarm_sendGsoc, plus the provider's
first streaming surface — a 'message' event fed by main-process
subscription WebSockets relayed to the subscribing webview.

Main additions:
- messaging-service: bee-js-backed identity/sends and deterministic
  GSOC topic derivation (freedom-gsoc-v1 profile, mined owner cached);
  subscribe sockets are hand-rolled on the Node-native WebSocket because
  bee-js hides close codes and the node signals lurker-pool exhaustion
  via close 1013, which subscribers must be able to distinguish.
- subscription-registry: Electron-free lifecycle state; multiplexes one
  socket per (kind, key) against the node's 8-neighborhood lurker pool,
  enforces the per-origin cap, and tears down on unsubscribe, navigation,
  webContents destruction, and permission revocation (via a revoke hook
  the provider layer registers with the permission store).
- provider-ipc: full re-validation (spec error reasons incl. invalid_target,
  payload_too_large, too_many_subscriptions, node_subscription_limit),
  messaging permission tier, capabilities features/limits
  (maxMessageBytes 4000, maxTargetDepth 3, maxSubscriptions 32), and
  pssTarget truncation so the node-global overlay never reaches a page.
- renderer/UI: messaging consent tier with grant + per-send prompts and
  a messaging auto-approve, mirroring the publish/feed prompt patterns.

Raw-address GSOC sends are rejected (the signing key derives from the
topic; an address alone carries nothing to sign with) — flagged for a
spec amendment.

Verified against a live antd 0.5.42 node: identity, derivation, gsoc/pss
subscribe establishment, slot-refusal mapping, clean teardown. Send
round-trip requires a stamped node and was exercised at the unit level.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…9cfb9d

Node-wide subscription-capacity exhaustion now surfaces as a retryable
4900 (it can be caused by other origins and frees as subscriptions
close) instead of -32603. PSS sends accept zero-byte payloads (the
trojan framing carries an explicit length; useful for pings), while
GSOC sends reject them with invalid_payload since an empty SOC payload
is inexpressible on the chunk layer. Both verified against a live,
stamped antd 0.5.42 node, including a zero-byte PSS round-trip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bee-js reports remainingSize 0 once a mutable batch's buckets are full,
so selectBestBatch skipped it even though the node keeps accepting
writes by overwriting the oldest stamp per bucket. For content
publishes that conservatism is correct (overwriting can evict stamps
protecting durable content), but for messaging it bricked sending
entirely once the only batch filled up — messages are ephemeral, so a
rolling stamp window is the right trade-off. The fallback is opt-in
per call site and only engages when no batch has remaining capacity;
content paths keep the existing behavior. Surfaced by live compliance
testing against a depth-17 batch (two buckets) that filled after a
handful of sends.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@flotob
flotob marked this pull request as ready for review July 16, 2026 10:30
Aligns to the messaging SWIP's adopted L=16 mining-prefix convention
(revision 765f8e6), which landed after this branch was built to 50ec686:

- swarm_getMessagingIdentity now emits a 2-byte pssTarget (was 3). A
  3-byte target makes remote senders mine 24 bits to reach this node —
  ~256x the hashes of a 16-bit mine, i.e. seconds on a mobile sender and
  enough to trip Ant's send timeout — for no reception benefit at
  light-node residency (Ant's lurker assumes L=16). 2 bytes is the
  network default: cheap to mine, and still above the storage depth so
  the trojan is retained.
- swarm_sendPss now rejects targets shorter than 2 bytes with
  invalid_target (the storability floor: a 1-byte target is too shallow
  for any storer to keep). Range is 2-3 bytes; 3 stays allowed for
  callers that explicitly want a deeper (private-agreement) prefix.

Introduces DEFAULT_TARGET_DEPTH (2) / minTargetDepth alongside the
existing MAX_TARGET_DEPTH (3); capabilities still advertise only
maxTargetDepth. Tests updated (2-byte pssTarget, 2-3 byte acceptance,
1-byte rejection); 1079 swarm+renderer tests green, lint clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@flotob

flotob commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 5809b54 to catch this up with the messaging SWIP, which moved from 50ec686 (what this branch was built to) to 765f8e6 after landing the PSS mining-depth convention.

What changed: PSS targets now default to 2 bytes (the L=16 network convention), not 3.

  • swarm_getMessagingIdentity emits a 2-byte pssTarget (was 3).
  • swarm_sendPss rejects targets outside 2–3 bytes with invalid_target (2 is the storability floor — below the network storage depth a trojan isn't retained by any storer).

Why it matters for us specifically: a 3-byte pssTarget makes every remote sender mine 24 bits to reach a Freedom node — ~256× the hashes of a 16-bit mine, i.e. seconds on a mobile sender, enough to trip Ant's /pss/send timeout — and it buys nothing on the receive side, because Ant's lurker assumes L=16 and a light node's covering peers pull the same bins regardless of prefix depth (measured identical at 16 and 24 on mainnet). This was exactly the reasoning behind reverting the Ant/SWIP convention to L=16; this change aligns the Freedom provider to it so directed PSS sends stay cheap on mobile.

maxTargetDepth stays 3 (a caller who privately agrees a deeper prefix with a deeply-resident receiver may still use it); only the default and the floor moved. Capabilities are unchanged (still advertise maxTargetDepth: 3). 1079 swarm+renderer tests green, lint clean.

Not touched here (still deferred, as noted in the PR): the history mailbox option and the ?neighborhood= rendezvous override — both are new optional capabilities from the same SWIP revision, safe to add in the follow-up.

🤖 Generated with Claude Code

# Conflicts:
#	scripts/fetch-ant.js
@meinharrd meinharrd added the alan:reviewing alan loop currently running on this PR label Aug 10, 2026
@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R1] Blocking — swarm_subscribe has no establishment timeout in the main process; a never-establishing subscription leaks forever and permanently consumes the per-origin cap

src/main/swarm/swarm-provider-ipc.js:1747 (await subscriptionRegistry.subscribe(...)) has no timeout. The comment in messaging-service.js:270-271 says a pre-establishment unreachable node leaves the establish promise "pending until provider-ipc's own timeout" — that timeout does not exist. The only timeout is the in-page 300s one in webview-preload.js, which merely rejects the page's promise; the main process never finds out.

Failure scenario (verified against subscription-registry.js with a scratch jest run — pending subs count toward the cap and nothing ever cleans them):

  1. Node stops (or its WS upgrade keeps failing, e.g. subscribing to a key the node refuses with an HTTP error) after checkBeeReachable() passes.
  2. openSubscriptionSocket treats every close except pre-establish 1013 as transient → reconnects with backoff forever; established never settles.
  3. The registry entry was added before the await (subscription-registry.js:91), so it counts toward maxSubscriptions (32) indefinitely.
  4. attachWebContentsTeardown (swarm-provider-ipc.js:1753) runs only after a successful await — so for a stuck subscription no did-navigate/destroyed listener is ever attached: navigating away or even closing the tab does not cancel it. The socket keeps redialing every 30s until app quit.
  5. A dApp that retries subscribe (natural response to its 5-min page timeout) leaks one entry per attempt; after 32 attempts the origin gets too_many_subscriptions forever — messaging is bricked for that origin until app restart, even after the node recovers.

Fix sketch: race established against a real timeout in handleSubscribe (removing the registry entry on timeout), and attach the webContents teardown before awaiting establishment so navigation/close always cleans up pending subscriptions.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R1] Blocking — subscription messages can be delivered cross-origin to whatever page the webview navigates to next

Two ordering gaps combine into a cross-origin leak:

  1. attachWebContentsTeardown is called only after await subscriptionRegistry.subscribe(...) resolves (src/main/swarm/swarm-provider-ipc.js:1747-1753). Establishment takes ≥500ms (ESTABLISH_GRACE_MS) and can take arbitrarily long (reconnect loop during a node hiccup). Any main-frame navigation that commits inside that window fires did-navigate before the listener exists, so the subscription survives into the new page.
  2. deliverSubscriptionMessage (swarm-provider-ipc.js:1655) sends to webContentsId without checking that the webview still hosts the subscribing origin — and fan-out runs for registry entries even before establishment (verified with a scratch jest run: onMessage fired pre-establishment is delivered).

Failure scenario: page A (bzz://chat-app) calls swarm_subscribe for a PSS topic; the user clicks a link to site B while establishment is pending (or the node blips, making the window seconds-to-minutes long — if the node recovers after navigation, the stale await resolves and binds the live subscription to the webview now showing B). Every PSS/GSOC message for A's subscription is then contents.send(...)'d into B, whose page script can simply register window.swarm.on('message', ...) (the provider is injected into every page). B receives A's private message traffic until the next navigation or tab close.

Fix sketch: attach the teardown listeners (or at least record a navigation epoch) before opening the subscription, and/or have the renderer re-verify the webview's display origin at delivery time / cancel on did-start-navigation.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R1] Blocking — payload-hash dedup silently drops legitimate identical messages; repeated zero-byte PSS pings are inexpressible

src/main/swarm/messaging-service.js:239-241: the per-socket dedup ring keys on sha256(payload) alone. Any two messages with byte-identical payloads within the 128-entry window are collapsed — there is no sequence number, timestamp, or chunk address in the key.

Failure scenarios:

  • Zero-byte pings (a use case this PR explicitly supports — allowEmpty for PSS, and the PR description says the empty payload is "spec: pings/signals" and was live-verified): every zero-byte payload hashes identically, so a presence/heartbeat protocol sending periodic empty pings delivers exactly one ping to the subscriber; all subsequent ones are silently dropped until 128 distinct other payloads happen to evict the hash. A liveness protocol built on the documented ping primitive reads the peer as dead.
  • Repeated chat text: a GSOC room where a user sends "ok" twice (or any app-level retry with identical bytes) — the second message never reaches subscribers.

The existing unit test (delivers payloads and dedups byte-identical redeliveries) asserts this behavior — 'hello' sent twice on a live socket is delivered once — so the drop is baked in, not incidental.

If the dedup exists for GSOC replay-on-reconnect, it should key on something that distinguishes deliveries (e.g. only dedup across a reconnect boundary, clear the ring once messages flow, or exempt PSS entirely — PSS trojans aren't redelivered by the node). At minimum PSS zero-byte pings must not be swallowed, since the PR advertises them as a supported signal primitive.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R1] Minor findings (combined):

  1. src/main/swarm/messaging-service.js:270-271 — comment states the establish promise "stays pending until provider-ipc's own timeout", but no such timeout exists in provider-ipc (see the blocking finding). If the timeout is added, keep this comment; otherwise it documents fictional behavior.
  2. src/main/swarm/swarm-provider-ipc.js:1698 — for kind: 'gsoc' with neither topic nor address, the error message reads "Provide either topic or address, not both", which is misleading for the missing-both case. Split the hasTopic === hasAddress check into two messages.
  3. src/main/swarm/messaging-service.js:189-190url is computed once per openSubscriptionSocket call; the reconnect loop redials the original URL forever. If the Ant node restarts on a different port (getAntApiUrl() changes), established subscriptions reconnect to a dead address indefinitely instead of following the node. Low likelihood with the fixed 1633 port, but cheap to recompute inside connect().
  4. src/main/swarm/messaging-service.js:226-234, 260 — refusal classification is time-based, not event-based: a 1013 refusal arriving after the 500ms grace window (slow node under load) is treated as a transient close, so subscribe reports success for a subscription that was actually refused, and the socket then retries a refusing node on backoff. Consider treating 1013 as "not established" whenever no message has been received yet, rather than keying on the grace timer alone.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-verify R1] All three findings reproduce against the code as written. I wrote throwaway jest repros driving the real subscription-registry + swarm-provider-ipc + messaging-service (only electron/permissions/fetch/WS faked); they are deleted again, tree is clean, and the three existing swarm suites still pass (190 tests).

  • R1-F1 — CONFIRMED. subscribe() inserts into the subscriptions map before await socket.handle.established, and nothing in handleSubscribe bounds that await — messaging-service.js:271 even claims "the establish promise stays pending until provider-ipc's own timeout", but no such timeout exists. Repro: with a socket whose established never settles, 32 attempts leave countByOrigin() === 32, the 33rd returns too_many_subscriptions, and because attachWebContentsTeardown() runs only after the await, no did-navigate/destroyed listener is ever registered — firing both events leaves all 32 in place and every socket un-cancelled (reconnect loop keeps running at the 30s ceiling). Only a grant revoke (cancelByOrigin) can recover; app restart otherwise.

  • R1-F2 — CONFIRMED, and slightly worse than reported: fanOut() delivers to subscriptions that are still pending, so the leak starts before establishment. Repro: origin A subscribes, did-navigate fires while attachWebContentsTeardown has not run yet (listener map is provably empty at that point), the webview's URL is now https://evil.example, and both the pre-establishment and post-establishment PSS payloads are contents.send()'d into it. deliverSubscriptionMessage never re-checks contents.getURL(), and webview-preload.js forwards every swarm:provider-event to the page via postMessage(..., window.location.origin) with no subscription-id or grant filter — so page B reads A's messages through window.swarm.on('message') without holding any grant of its own.

  • R1-F3 — CONFIRMED. seenSet/seenHashes live outside connect(), so the sha256 dedup is payload-only, per-socket, and survives reconnects. Repro: 5 consecutive empty PSS frames → onMessage called once; 'gm' delivered, socket closed 1006, reconnect, 'gm' sent again as a genuinely new message → still only one delivery. Note handleSendPss passes allowEmpty: true deliberately (unlike sendGsoc, which rejects empty), so this PR explicitly supports sending zero-byte pings that the receive path then collapses to one.

No visual/e2e evidence attached — all three live in main-process protocol paths with no user-visible UI surface to steer to; unit-level repro against the real modules is the stronger evidence here.

@meinharrd meinharrd added alan:reviewing alan loop currently running on this PR and removed alan:reviewing alan loop currently running on this PR labels Aug 10, 2026
@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R1] Blocking — concurrent messaging prompts clobber each other: the second request is auto-rejected as "User dismissed prompt" and the first is orphaned (Allow/Cancel settle nothing)

src/renderer/lib/wallet/swarm-connect.js:550 sets swarmMessagingPending = { permissionKey, resolve, reject } before hideAllSubscreens() runs at line 593, while the messaging screen-hider (lines 158–165) rejects swarmMessagingPending whenever the messaging screen was already visible. So when a second messaging prompt is requested while the first is open:

  1. swarmMessagingPending is overwritten with request 2's {resolve, reject} (request 1's are lost).
  2. hideAllSubscreens() fires the messaging hider → screen is visible (from prompt 1) → it rejects request 2 with {code: 4001, message: 'User dismissed prompt'} and nulls the pending.
  3. The prompt is then re-shown with swarmMessagingPending === null — the user clicks Allow (or Cancel) and approveSwarmMessaging/rejectSwarmMessaging early-return on the null pending. Request 1's promise in handleMessagingRequest never settles; the page's call hangs until the 300s webview timeout.

Failure scenario (fully realistic — nothing serializes requests: handleSwarmRequest in src/renderer/lib/swarm-provider.js:41-46 fires per ipc-message with no queue): a chat dApp calls swarm.subscribe() for two rooms in parallel at page load (first messaging use → both take the grant-mode prompt path since neither has the grant yet), or sends two PSS messages back-to-back before messaging auto-approve is enabled. Result: one request instantly rejected "User dismissed prompt" with no user action, the other stuck 5 minutes, and an approval screen whose buttons do nothing but hide it.

Verified empirically with a scratch jest repro driving the real swarm-connect.js (real registerScreenHider/hideAllSubscreens semantics, fake DOM): r2.reject called with 4001 immediately, then clicking the confirm button settles neither request. Scratch file deleted; tree clean.

Note: the connect/publish/feed prompts (showSwarmPublishApproval at line 417 etc.) have the identical latent ordering bug, but those flows rarely see concurrent same-type prompts (one-off connect, rare publishes). Messaging makes it a hot path (parallel subscribes at startup are the natural dApp idiom). Fix sketch: call hideAllSubscreens() before assigning swarmMessagingPending (the hider then correctly rejects the old pending, exactly like the cross-type case already does) — and per the sibling-sites lesson, apply the same reorder to the connect/publish/feed screens in the same commit.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R1] Minor findings (combined, round-2 pass — not repeating the five already on the PR):

  1. src/main/swarm/swarm-provider-ipc.js:1756-1777handleSubscribe's catch maps too_many_subscriptions and node_subscription_limit, but a subscription cancelled while establishment is pending rejects with err.reason === 'cancelled' (messaging-service.js:295, e.g. the origin's grant is revoked mid-establish via cancelByOrigin) and falls through to INTERNAL_ERROR (-32603, message "Subscription cancelled"). The dApp did nothing wrong and the node isn't broken — this should map to a 4001-style rejection (or at least a semantic reason like subscription_cancelled) rather than presenting a revocation as a provider internal error.

  2. src/renderer/lib/wallet/swarm-connect.js:583-586 — in grant mode triggered by a send (swarm_sendPss before any grant exists), the warning copy is the tier-wide grant text and the auto-approve checkbox is hidden, but the approval also implicitly authorizes that first send (no per-send prompt follows in handleMessagingRequest). The prompt does show topic/size rows, so this is only a copy nit: the grant-mode warning could mention that approving also sends this message.

No further blocking issues found beyond the one posted above and the three already confirmed by the earlier review pass (establishment timeout / teardown ordering, cross-origin delivery, payload dedup) — those remain unaddressed in the code as of cc51991.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-verify R1] R1-F4 (swarm-connect.js:550, concurrent messaging prompts clobber swarmMessagingPending) — CONFIRMED.

Reproduced against the real module (throwaway jest test, fake DOM mirroring index.html's initially-hidden subscreens, real registerScreenHider/hideAllSubscreens semantics from wallet-state.js): two showSwarmMessagingApproval() calls with no settle in between →

after 2nd show:   {"a":null,"b":"rejected:4001:User dismissed prompt"}  screenHidden=false  topicShown=b
after Allow click:{"a":null,"b":"rejected:4001:User dismissed prompt"}
race: TIMEOUT-something-hangs

i.e. exactly the reported behaviour: showSwarmMessagingApproval assigns swarmMessagingPending at line 550 before hideAllSubscreens() at line 593, so the messaging screen-hider (line 158-165) sees wasVisible === true (prompt #1 still up) and rejects the new pending with 4001 "User dismissed prompt", then nulls it. The screen is immediately re-shown with request #2's details, but swarmMessagingPending is now null, so approveSwarmMessaging()/rejectSwarmMessaging() both early-return — request #1 is orphaned and never settles.

Refutation attempts that failed:

  • Serialization upstream? No. setupSwarmProvider dispatches each swarm:provider-request into a fire-and-forget handleSwarmRequest(), and handleMessagingRequest awaits requirePermission + hasMessagingGrant/getAutoApprove IPC before prompting — so two swarm.subscribe() calls at page load (both !hasGrant) or two back-to-back sendPss without messaging auto-approve interleave and both reach the prompt. No queue anywhere in swarm-provider.js or wallet-state.js.
  • Does the orphan really hang? Yes, until the page-side timeout: src/main/webview-preload.js:656 gives swarm_send*/swarm_subscribe a 300000 ms budget, so the dApp's promise sits for 5 minutes before rejecting with a misleading -32603 Request timed out.
  • Wrong reading of visibility? No — verified against src/renderer/index.html:2230 (sidebar-swarm-messaging-approve starts with class="... hidden"), so the first prompt is safe and only the second-and-later clobber. (My first repro run wrongly showed both rejecting because the fake screen didn't start hidden; fixing that produced the output above, which matches the report exactly.)

Note for the fix (not a new finding, pre-existing outside this PR's diff): the connect/publish/feed prompts have the identical set-pending-then-hideAllSubscreens() ordering (swarmConnectPending L471, swarmPublishPending, swarmFeedPending), so whatever shape the fix takes — capture-and-settle the old pending before overwriting, or queue prompts — please apply it to all four sites in the same commit rather than only the messaging one.

Evidence caveat: verified at the module level, not visually — the harness e2e project has no Swarm-messaging fixture (no spec drives window.swarm), and the renderer exposes no hook to drive showSwarmMessagingApproval from Playwright, so there was no way to steer the real app into a double-prompt state without building new fixtures.

meinharrd added a commit to solardev-xyz/alan-artifacts that referenced this pull request Aug 11, 2026
meinharrd added a commit to solardev-xyz/alan-artifacts that referenced this pull request Aug 11, 2026
Each Swarm approval prompt kept a single `*Pending` slot, so a second
request arriving while a prompt was on screen overwrote the first. The
newer request was then auto-rejected by the screen hider that
hideAllSubscreens() fires during the transition (it saw the screen still
visible), and the re-shown prompt had a null pending — so Allow/Cancel
settled nothing and both page calls hung until the request timeout.

Replace the single slot with a small FIFO prompt queue per screen: a
request that arrives while a prompt is up waits its turn and is shown when
that prompt settles; dismissing the screen rejects the whole queue (4001)
so nothing is left hanging. Applied to all four structurally identical
prompts (connect, publish, messaging, feed), not just messaging.

Also snapshot the publisher identity state in approveSwarmFeed() so the
in-flight approval isn't disturbed by a queued feed prompt resetting it.

Adds swarm-connect.test.js covering the queue behavior for the messaging,
publish, and connect prompts; all five cases fail against the pre-fix
module.
@meinharrd

Copy link
Copy Markdown
Contributor

[alan-fix R1] — fixed in ca4164e.

R1-F4 (blocking) — concurrent messaging prompts clobber swarmMessagingPending — fixed.

Root cause was the single *Pending slot, not the messaging screen specifically: a second request arriving while a prompt is on screen overwrote the first, then got auto-rejected by its own screen hider (fired by hideAllSubscreens() during the transition, which still saw the screen visible), leaving the re-shown prompt with a null pending so Allow/Cancel settled nothing.

Fix: a small FIFO prompt queue per screen (createPromptQueue) in src/renderer/lib/wallet/swarm-connect.js.

  • A request arriving while a prompt is up is queued, not shown — no clobbering, nothing auto-rejected.
  • The queue's current is assigned after present() runs hideAllSubscreens(), so the hider can never mistake an incoming request for a dismissed one.
  • Approve/Cancel settles only the request on screen, then the next queued one is presented.
  • Dismissing the screen (another subscreen taking over, sidebar teardown) rejects the current and every queued request with 4001, so nothing hangs to the 300s page timeout.

Per the repo lesson about identical siblings, the queue is applied to all four structurally identical prompts — showSwarmConnect, showSwarmPublishApproval, showSwarmMessagingApproval, showSwarmFeedApproval — which all had the same single-slot bug. One related tidy-up: approveSwarmFeed() now snapshots swarmFeedIdentityState locally, so an in-flight approval isn't disturbed by a queued feed prompt resetting the shared state.

Tests — new src/renderer/lib/wallet/swarm-connect.test.js (5 cases: messaging queueing, cancel-then-next, dismiss-rejects-queue, publish sibling, connect sibling). All 5 fail against the pre-fix module and pass after. Full unit suite: 2241 passed, 1 failed — only the known pre-existing vault auto-locks after timeout flake. npx eslint . clean.

Visual verification — drove the real app under the Playwright harness: two messaging requests fired concurrently, then Allow clicked twice. Prompt #1 shows and prompt #2 waits (both promises still pending); Allow resolves #1 only and the queued #2 is then presented with its own params; Allow resolves #2 and the screen closes. Acceptance evidence:

prompt 1 — grant prompt for alpha-topic, request 2 queued
prompt 2 — queued send prompt for beta-topic shown after Allow on #1

The temporary e2e spec used for those shots was deleted before committing.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R2] Blocking — ca4164e regression: feed "always allow" auto-approve is silently dropped whenever another feed prompt is queued

src/renderer/lib/wallet/swarm-connect.js:1010approveSwarmFeed() reads swarmFeedAutoApproveCheckbox?.checked only after await persistFeedPromptIdentity(...) (line 1007). The click handler (lines 710–715) calls approveSwarmFeed(); closeSwarmFeedApproval(); — close runs synchronously while the approval is suspended on that await, and settle() immediately presents the next queued feed prompt, whose presentSwarmFeedApproval() resets the checkbox to false (line 751). When persist resolves, the checkbox reads unchecked and setAutoApprove is never called.

The commit's own tidy-up snapshots swarmFeedIdentityState for exactly this in-flight-vs-queued race, but missed the sibling shared-DOM state (the checkbox). The messaging and publish approvals are immune only by accident — they read their checkbox before the first await.

Failure scenario (verified with a throwaway jest repro against the real module — deleted, tree clean; a control run without a queued prompt passes and does call setAutoApprove):

  1. dApp fires two feed operations back-to-back (the exact concurrency this fix targets); prompt Swarm encrypted references are not supported #1 shows, feat: swarm encrypted reference support #2 queues.
  2. User checks "Always allow this site to manage feeds" and clicks Allow on Swarm encrypted references are not supported #1.
  3. setAutoApprove mock: 0 calls. The approval itself resolves, so nothing looks wrong — but the user's persisted choice is silently discarded and every subsequent feed op re-prompts.

Fix sketch: read the checkbox synchronously at the top of approveSwarmFeed() (alongside the identity-state snapshot), before any await.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R2] Blocking — ca4164e: double-click on connect Approve double-settles the queue and silently drops a queued request (never resolved, never rejected)

src/renderer/lib/wallet/swarm-connect.js:352approveSwarmConnect() awaits grantPermission + the swarm_requestAccess round-trip before calling closeSwarmConnect(), and the Approve button is never disabled while that is in flight. settle() (line 127) is not guarded per-entry: every call unconditionally nulls current and advances the queue.

Failure scenario (verified with a throwaway jest repro against the real module — two click dispatches, second not awaited behind the first handler's async work; scratch file deleted, tree clean):

  1. Requests C1 (on screen) and C2 (queued). User double-clicks Approve — both handler invocations read current === C1 and start the grant round-trip.
  2. Invocation 1 resolves C1 → closeSwarmConnect()settle() → C2 is presented (current = C2).
  3. Invocation 2 resumes → closeSwarmConnect()settle() again → current = null: C2 is consumed without being resolved or rejected, and the screen closes.
  4. Observed end state: first: {resolved: true}, second: {resolved: false, rejected: null}, connect screen hidden. C2's page call hangs until the 60s webview timeout, then gets a misleading -32603 Request timed out; the prompt is never re-shown.

Fix sketch: make settlement idempotent per entry (e.g. settle(entry) that no-ops unless entry === current), and/or disable the approve/reject buttons while an approval is in flight. Note the same unguarded double-settle() is reachable on the other three screens via their close paths.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R2] Blocking — ca4164e: the queued consent prompt is presented synchronously inside the settling click, so the second click of a double-click approves a prompt the user never saw

src/renderer/lib/wallet/swarm-connect.js:580 (messaging confirm handler; publish and feed are identical) — the handler runs approveSwarmMessaging(); closeSwarmMessagingApproval();, and close → settle() → showNext() → present() all execute synchronously within the same click dispatch. The Allow button is re-armed for the next queued request before the click gesture is even over, at the same screen position, with no cooldown or disable window.

Failure scenario: a chat dApp's first messaging use fires a grant request plus a send (or two sends) back-to-back — the queue-on-purpose case this commit introduces. Prompt #1 (grant) shows, #2 (send, or a second grant/send) queues. The user double-clicks Allow on #1 — an ordinary accidental gesture. Click 2 lands ~50–300ms later on the Allow button now bound to prompt #2, approving a message send (stamp consumption, network-visible payload) or the tier grant with no opportunity to read what was authorized. Same applies to Cancel (queued request rejected unseen) and to the publish/feed screens (a queued publish approved unseen).

This is the standard permission-prompt clickjacking/prompt-farm concern that browser prompts guard against with input protection. Before ca4164e it was unreachable (there was never a queued next prompt); the queue makes it the hot path.

Fix sketch: after presenting a queued entry, ignore clicks on the action buttons for a short window (e.g. 500ms, Chromium-style), or disable the buttons and re-enable on a timer/next animation frame + delay. A per-entry settle(entry) guard (see the double-settle finding) composes with this but does not replace it.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R2] Blocking (carry-over) — R1's three confirmed main-process findings remain unaddressed at ca4164e

Re-checked the code at ca4164e (which fixes only R1-F4, the prompt clobbering): the three blocking findings confirmed by the R1 verify pass are all still present, unchanged —

  1. No establishment timeout / teardown-attached-too-latesrc/main/swarm/swarm-provider-ipc.js:1747 still awaits subscriptionRegistry.subscribe(...) unbounded, and attachWebContentsTeardown (line 1753) still runs only after a successful await; messaging-service.js:271 still documents a provider-ipc timeout that does not exist. Stuck subscriptions still leak, count toward the 32-cap forever, and survive navigation/tab close.
  2. Cross-origin message deliverydeliverSubscriptionMessage (line 1655) still sends to webContentsId with no check that the webview still hosts the subscribing origin, and fanOut still delivers pre-establishment; a navigation during the establishment window still routes origin A's messages into whatever page the webview shows next.
  3. Payload-hash dedup drops legitimate messagesmessaging-service.js:239-241 still keys the dedup ring on sha256(payload) alone, per-socket, surviving reconnects; repeated zero-byte PSS pings and byte-identical resends are still collapsed to one delivery.

Not new findings — restating so this round's ledger reflects that they are still open and blocking.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R2] Minor findings (combined):

  1. src/renderer/lib/wallet/swarm-connect.js:122 / src/renderer/lib/swarm-provider.js:139-151 — queued prompt entries capture their mode at enqueue time and are never re-evaluated when finally presented, so state changed by an earlier queued approval is ignored. Two concrete shapes: (a) two swarm.subscribe() calls at page load both take the grant-mode path (both saw !hasGrant); approving Swarm encrypted references are not supported #1 grants the messaging tier, yet feat: swarm encrypted reference support #2 still presents the full tier-grant prompt for a grant the origin already holds. (b) User checks "always allow" on send prompt Swarm encrypted references are not supported #1 (messaging or publish); queued send prompt feat: swarm encrypted reference support #2 is still shown even though auto-approve is now enabled. Nothing breaks — the redundant approval is harmless — but the double-ask is confusing and trains users to click through consent prompts. Cheap fix: before presenting a queued messaging/publish entry, re-check hasMessagingGrant/getAutoApprove and auto-resolve if the prompt is no longer needed.

  2. src/renderer/lib/wallet/swarm-connect.js:118 (showNext) — if present(next) throws (a DOM ref unexpectedly null, a render bug), current is never assigned: the entry is lost un-rejected (page hangs to timeout) and the queue stalls (current null but the entry consumed). Wrapping present in try/catch and rejecting the entry on failure would make the queue robust to render errors.

Evidence note: findings verified at module level with throwaway jest repros against the real swarm-connect.js (deleted; tree clean; the three existing swarm suites still pass, 31 tests). No visual pass — as established in R1, the harness has no fixture that drives window.swarm prompts and the renderer exposes no hook to steer these screens from Playwright.

@meinharrd meinharrd removed the alan:reviewing alan loop currently running on this PR label Aug 11, 2026
@meinharrd meinharrd added the alan:reviewing alan loop currently running on this PR label Aug 11, 2026
@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R1] Fresh review pass at ca4164e (no new commits since the last fix round).

Carry-over: six confirmed blocking findings remain unaddressed

Independently re-verified each against the code at ca4164e — all still present, all already documented in earlier comments, so not re-posting details:

  1. No establishment timeout + teardown attached only after a successful await (swarm-provider-ipc.js:1747/1753; messaging-service.js:271 still documents a provider-ipc timeout that does not exist).
  2. Cross-origin subscription message delivery after navigation (deliverSubscriptionMessage, swarm-provider-ipc.js:1655 — no origin re-check at delivery; fan-out runs pre-establishment).
  3. Payload-hash dedup drops legitimate identical messages / repeated zero-byte PSS pings (messaging-service.js:239-241).
  4. ca4164e regression: feed "always allow" checkbox read after await persistFeedPromptIdentity (swarm-connect.js:1010) — dropped whenever a feed prompt is queued.
  5. ca4164e: unguarded settle() + un-disabled buttons → double-click on connect Approve consumes a queued request without settling it (swarm-connect.js:127/352).
  6. ca4164e: queued prompt presented synchronously inside the settling click — second click of a double-click approves a prompt the user never saw (swarm-connect.js:580 and siblings).

New minor findings (combined)

  1. src/renderer/lib/wallet/swarm-connect.js:389 — the connection-banner auto-approve badge checks publish || feeds || signing but not autoApprove.messaging. A user who ticks "Always allow this site to send messages" gets no badge, even though messaging auto-approve is arguably the most consequential tier (silent stamp-consuming, network-visible sends). Classic missed-sibling site: every other autoApprove consumer was extended for messaging, this one wasn't. (Related, acknowledged in the PR as follow-up: permission-manage.js has no messaging toggle either, so once enabled the only off-switch is full disconnect.)

  2. Approval handlers can leave a request permanently unsettled when their own IPC failsapproveSwarmMessaging (swarm-connect.js:675) and approveSwarmPublish (:544) await setAutoApprove(...) before resolve() with no try/catch: if that IPC rejects, resolve() is never reached, the click handler's promise rejection is unhandled, and the page call hangs to its 60/300s timeout (the queue itself advances via close → settle(), so the loss is silent). approveSwarmConnect (:348-350) has the mirror-image bug: it catches grant/round-trip failures but only logs — neither resolve nor reject is called, so a failed grant hangs the page's requestAccess for 60s. Only the feed sibling handles this correctly (rejects with -32603 in its catch). Fix in all three: wrap in try/catch and reject the pending request on failure.

  3. Cross-type prompt concurrency still auto-rejects rather than queues — the ca4164e queues are per-screen, so a dApp firing e.g. swarm.subscribe() + swarm.createFeed() concurrently has the feed prompt's hideAllSubscreens() reject the on-screen messaging prompt (and its whole queue) with 4001 "User dismissed prompt", no user action involved. This matches the documented design of the fix (one sidebar prompt surface; dismissal rejects), so flagging as a design note rather than a defect: the spurious 4001 is deterministic and doesn't hang, but dApps mixing messaging with feed/publish calls at load will see phantom dismissals.

Evidence note: verified at code level; per the earlier rounds, the e2e harness has no fixture driving window.swarm prompts, and these minors don't warrant building one. Main-process messaging paths (bee-js API usage — gsocMine/gsocSend/pssSend signatures, unprefixed toHex for the pssTarget slice) checked against bee-js 12.2.1 typings and runtime — no issues. Changed suites: 212 tests green, eslint clean.

meinharrd added a commit to solardev-xyz/alan-artifacts that referenced this pull request Aug 11, 2026
meinharrd added a commit to solardev-xyz/alan-artifacts that referenced this pull request Aug 11, 2026
@meinharrd

Copy link
Copy Markdown
Contributor

[alan-verify R1] All six findings verified against ca4164e. I tried to refute each one; none died. Evidence below (throwaway specs deleted, tree left clean).

R1-F1 — CONFIRMED (swarm-provider-ipc.js:1747). There is no establishment timeout anywhere: grep -n "timeout|setTimeout" in swarm-provider-ipc.js returns nothing relevant, and the only timer in messaging-service.js is ESTABLISH_GRACE_MS (a grace window, not a cap) — while the socket's own comment says the promise "stays pending until provider-ipc's own timeout", which does not exist. subscription-registry.subscribe() inserts the entry (and its cap slot) before await socket.handle.established and only removes it on rejection; any non-1013 close reconnects forever, so the promise never settles. Scratch test against the real registry with a never-settling established: countByOrigin stays 1 indefinitely, handle.cancel is never called, and a third subscribe from the same origin throws too_many_subscriptions. attachWebContentsTeardown(webContentsId) is only reached after the await, so no did-navigate/destroyed listener ever exists for a stuck subscription.

R1-F2 — CONFIRMED (swarm-provider-ipc.js:1655). deliverSubscriptionMessage resolves the webContents by id and never re-checks its current origin. Fan-out is live pre-establishment (same scratch test: deliver fires for a subscription whose established is still pending). So a main-frame navigation during the pending window happens before the teardown listener is attached, the did-navigate is missed, and the subscription keeps delivering afterwards. Delivery into the new document is real, not theoretical: webview-preload.js:764 re-posts the event with window.location.origin (now B), and the injected page provider (webview-preload.js:720) emits it to every window.swarm.on('message') listener with no subscription-id or origin filtering.

R1-F3 — CONFIRMED (messaging-service.js:239). seenHashes/seenSet are closure state outside connect(), so the ring survives reconnects, and the key is sha256(payload) with nothing distinguishing "node redelivered" from "app sent the same bytes again". The PR's own test asserts the data loss: "delivers payloads and dedups byte-identical redeliveries" — two hello sends produce one delivery. Zero-byte PSS pings (a payload shape this PR explicitly supports) all hash identically, so exactly one ever reaches the page.

R1-F4 — CONFIRMED (swarm-connect.js:1010). Jest against the real module, both directions: with a lone feed prompt the Always-allow checkbox works (setAutoApprove(origin,'feeds',true) is called); with a second feed prompt queued, setAutoApprove is never called — the click handler's synchronous closeSwarmFeedApproval()settle()presentSwarmFeedApproval() resets swarmFeedAutoApproveCheckbox.checked = false (line 751) before the await persistFeedPromptIdentity resumes. The code already snapshots swarmFeedIdentityState for exactly this reason (line 1000) but not the checkbox.

R1-F5 — CONFIRMED, with a timing note (swarm-connect.js:352). Real app (Playwright harness, real renderer module): two Approve activations inside the handler's async window leave C2 pending forever — never resolved, never rejected — with the connect screen hidden while swarm-connect-site still reads https://b.example, i.e. the prompt was presented and silently consumed. Honest caveat on reachability: I measured the async window on an idle app at well under 15ms (two IPC round-trips), and a 15ms-apart pair instead approved C2 unread. So the ordinary mouse double-click usually lands in the F6-style variant, and the orphaning needs a busier main process (or Enter key-repeat). Both outcomes are wrong and both come from the same root cause — settle() is unguarded and the buttons are never disabled — so the finding stands.

R1-F6 — CONFIRMED (swarm-connect.js:580). Real app, two back-to-back PSS/GSOC send prompts: click 1 approves prompt #1 and closeSwarmMessagingApproval() presents prompt #2 synchronously inside the same click, with #swarm-messaging-confirm still enabled and its bounding box pixel-identical ({"x":1047,"y":543.97,"width":138,"height":38} before and after). A second click 120ms later — a normal double-click interval — approved the stamp-consuming GSOC broadcast the user never saw.

after click 1 approves prompt #1 …prompt #2 is instantly under the cursor
f6-prompt1 f6-prompt2

Verdict: 6 confirmed, 0 refuted.

…prompts

Round-1 review fixes for the window.swarm messaging extension.

Main process:
- subscribe() now gives up after an establishment timeout (30s) and
  releases the slot: the socket layer reconnects forever, so a node that
  stopped answering left the subscription pending — and its share of the
  32-per-origin cap held — for the life of the app.
- did-navigate/destroyed teardown is armed before establishment is
  awaited, not after it succeeds, so a page that navigates or closes
  mid-establishment still tears its pending subscription down. A
  subscription cancelled while establishing rejects instead of handing
  the page a dead id.
- Messages are no longer fanned out to subscriptions that have not
  established, and the deliverer re-checks the subscribing document's
  committed URL before sending: a webview keeps its webContents across
  navigations, so without this a message could land in whatever origin
  navigated in afterwards (which has no messaging grant).
- Dropped the per-socket sha256 payload dedup ring. At the wire level a
  redelivery is indistinguishable from a genuine repeat, so it silently
  swallowed legitimate traffic — byte-identical chat messages and the
  zero-byte PSS pings this PR supports. Delivery is at-least-once and
  payload-transparent; apps that need exactly-once carry a message id.

Renderer approval prompts:
- Settling goes through queue.claim(), which hands the on-screen request
  to exactly one caller: a double-click can no longer settle the same
  request twice, which silently consumed the request queued behind it
  (its page call then hung to the 60s timeout, prompt never shown).
- A freshly presented prompt ignores input for 500ms and shows its
  buttons disabled, so the second click of a double-click cannot approve
  or cancel a stamp-spending prompt the user never saw.
- The feed "always allow" checkbox is read before the first await, like
  the publish and messaging prompts already did — presenting a queued
  prompt resets the shared checkbox, so the choice was being dropped.
- Screen hiders no longer drain their own queue while presenting the
  next request, which spuriously rejected the third and later prompts.
meinharrd added a commit to solardev-xyz/alan-artifacts that referenced this pull request Aug 11, 2026
meinharrd added a commit to solardev-xyz/alan-artifacts that referenced this pull request Aug 11, 2026
@meinharrd

Copy link
Copy Markdown
Contributor

[alan-fix R1] All six confirmed findings fixed in 85d2378.

id fix
R1-F1 subscriptionRegistry.subscribe() now races establishment against a timeout (establishTimeoutMs, default 30s): on expiry it removes the subscription, releasing the per-origin slot and closing the socket, and rejects with establish_timeout. handleSubscribe arms the did-navigate/destroyed teardown before awaiting establishment (it used to attach only after success), so a page that navigates or closes mid-establishment still tears its pending subscription down. If the subscription is cancelled while we wait, subscribe rejects with subscription_cancelled instead of handing the page a dead id; both map to a retryable 4900. (subscription-registry.js, swarm-provider-ipc.js)
R1-F2 Two parts. Fan-out now skips subscriptions still pending (pre-establishment), and deliverSubscriptionMessage re-checks the target: the subscription carries the committed pageUrl of the document that subscribed, and if contents.getURL() no longer matches, the message is dropped and cancelByWebContents tears the whole page's subscriptions down. Combined with F1's early teardown, a message can no longer land in a page that navigated in afterwards.
R1-F3 Removed the per-socket sha256 dedup ring in messaging-service.js. On the wire a redelivery is indistinguishable from a genuine repeat, so it silently swallowed real traffic — byte-identical GSOC chat lines and the zero-byte PSS pings this PR supports. Delivery is now at-least-once and payload-transparent (documented in the module header); apps needing exactly-once carry their own message id.
R1-F4 approveSwarmFeed snapshots swarmFeedAutoApproveCheckbox.checked synchronously, before the first await — matching what the publish and messaging prompts already did. Presenting a queued prompt resets the shared checkbox, so the Always-allow choice was previously dropped whenever another feed op was queued.
R1-F5 Settling now goes through queue.claim(), which hands the on-screen request to exactly one caller (it nulls current and sets a settling flag). Every approve/reject/back handler claims first and does nothing when the claim comes back null, so a double-click can no longer settle twice and consume the queued request. show() also honours settling, so a request arriving during an in-flight approval waits rather than jumping the screen.
R1-F6 A freshly presented prompt is unarmed for PROMPT_ARM_DELAY_MS (500ms): claim() refuses during the window and the screen's buttons are visibly disabled, so the second click of a double-click cannot approve or cancel a prompt the user never saw. The feed Allow button stays gated on the vault check as well (syncFeedApproveButton).

Also fixed while in the same machinery (not in the findings list, same root cause as F5/F6): the screen hiders drained their own queue during present()'s hideAllSubscreens(), so with three or more queued prompts the third and later ones were spuriously rejected as "User dismissed prompt". Hiders now no-op while their own queue is presenting.

Verification

  • npx jest — 2253 passed, 1 failure: the known pre-existing vault auto-locks after timeout flake, unrelated to these files.
  • npx eslint clean on all touched files.
  • New/updated assertions in the same commit: registry pending fan-out, establish timeout releasing the slot, cancel-during-establish; provider-ipc teardown-armed-before-establishment, 4900 mapping, cross-origin delivery drop; messaging-service delivering repeats and empty pings; and four renderer tests (double-click keeps the queued request, input-protection window, feed Always-allow survives a queued prompt, feed Allow stays disabled while locked). I mutation-checked the three renderer tests — reverting each fix individually fails exactly one of them.
  • xvfb-run npx playwright test --project=harness — 21/21 pass, including a throwaway spec that drove the real app through the double-click scenario (removed before commit).

The queued send prompt presented by the approving click, buttons dead during the input-protection window — the second click of the double-click hit this and was ignored:

queued prompt during input protection

…and the same prompt 500ms later, live and settling normally on a real click:

queued prompt armed

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R2] Blocking — 85d2378 regression: an in-page navigation (hash change / pushState) silently kills every subscription on the webview and drops the triggering message

src/main/swarm/swarm-provider-ipc.js:1664 — the new delivery-time guard compares contents.getURL() !== subscription.pageUrl by exact string. But the two teardown signals this PR uses are not symmetric:

  • did-navigate (the teardown listener, line 1646) fires only for cross-document main-frame navigations — Electron explicitly does not emit it for in-page navigations (anchor clicks, location.hash, history.pushState/replaceState; those emit did-navigate-in-page, which this app already handles separately in src/renderer/lib/tabs.js:453).
  • webContents.getURL() does change on in-page navigations.

So a page that stays on the same document but updates its URL fails the equality check on the next message: deliverSubscriptionMessage drops the payload and calls subscriptionRegistry.cancelByWebContents(...), cancelling all of the webview's subscriptions — with no event, error, or unsubscribe notification to the page, whose JS context (and swarm.on('message') listeners) is still alive and believes it is subscribed.

Failure scenario (verified with a throwaway jest repro driving the real subscription-registry + swarm-provider-ipc, only electron/permissions/fetch mocked — deleted, tree clean): a chat dApp at bzz://chat.eth/#/lobby subscribes, receives one message fine; user clicks into a room (location.hash = '#/room/42' — hash routing is the natural idiom for statically-hosted dweb SPAs, since bzz sites have no server-side routing); the next incoming message is dropped, countByOrigin === 0, and every later message for any of its subscriptions is gone. Repro output: send count after hash change: 1, subscriptions left for origin: 0. Messaging for the origin is dead until it re-subscribes — which it has no way to know it must do.

Fix sketch: make the delivery check match the teardown semantics — compare same-document identity, not full URL. E.g. strip the fragment and compare, or better: don't compare URLs at all; record a per-webContents navigation epoch bumped in the did-navigate listener and stamp it into the subscription, so delivery only requires "no cross-document navigation since subscribe". (Note fragment-stripping alone still breaks pushState, which changes the path — the epoch approach covers both.)

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R2] Blocking — cross-origin delivery not fully closed: pageUrl is captured after the grant prompt and main-process awaits, so a navigation in that window binds the subscription to the new page — and the new delivery check then validates the wrong page

src/main/swarm/swarm-provider-ipc.js:177485d2378 scopes delivery to pageUrl: contents.getURL(), but that snapshot is taken at registry-subscribe time, which is after:

  1. the renderer's messaging grant prompt (handleMessagingRequest in src/renderer/lib/swarm-provider.js awaits showSwarmMessagingApproval before calling executeWithPermission — a user-paced, unbounded window; nothing rejects a pending prompt or re-checks the webview's display origin when the webview navigates: the only navigation listener in swarm-connect.js just updates the banner), and
  2. main's own await checkBeeReachable() fetch (line 1739, no timeout) and GSOC derivation.

If a main-frame navigation commits anywhere in that window, handleSubscribe resumes and snapshots the new page's URL as pageUrl while origin is still the old page's. The registry entry didn't exist when did-navigate fired, so the (now correctly pre-armed) teardown had nothing to cancel; the subscription establishes bound to whatever the webview shows now, and every message for origin A's topic is contents.send(...)'d into page B — which holds no messaging grant — and the line-1664 URL check passes, because pageUrl is B's URL.

Verified with a throwaway jest repro against the real subscription-registry + swarm-provider-ipc (deleted, tree clean): navigation flipped getURL() to bzz/evil.eth/ inside the checkBeeReachable fetch and fired did-navigate; subscribe still returned a live subscriptionId for chat.eth, and the next PSS payload was delivered into the evil.eth page (send called once, event type: 'swarm_subscription').

Concrete scenario needing no tight timing: page A (bzz://chat.eth) calls swarm.subscribe() for the first time → grant prompt appears in the sidebar; the user instead navigates the tab to site B, then (prompt is still on screen, showing chat.eth's request) clicks Allow. The subscription is created against B's document; B's page script reads A's message traffic via swarm.on('message').

Fix sketch: capture the page identity at request time, not post-await — e.g. have the renderer pass the webview's committed URL in meta when the request arrives (alongside webContentsId) and use that as pageUrl, so any later navigation fails the delivery check / establishes-then-cancels; and/or reject queued+on-screen prompts for a webview on did-navigate in the renderer. The navigation-epoch approach from the in-page-navigation finding also closes this: snapshot the epoch when the request enters handleSubscribe, before any await.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R2] Minor findings (combined), reviewed at 85d2378:

  1. src/renderer/lib/wallet/swarm-connect.js:230 + back buttons — the input-protection wiring disables the approve/reject buttons during the 500ms arm window, but none of the four Back buttons (swarm-connect-back, swarm-publish-back, swarm-messaging-back, swarm-feed-back) are in any onArmedChange list, even though their handlers are claim()-gated. During the arm window the Back button looks enabled but silently no-ops — exactly the "mysterious dead click" the commit message says the disabled styling exists to avoid. Add the back buttons to their queues' disabled sets. (The feed queue also omits its confirm button from the plain-disable list, relying on syncFeedApproveButton — correct, just worth keeping in mind for future edits.)

  2. src/main/swarm/messaging-service.js:263 — comment still says the pre-establishment pending promise lasts "until provider-ipc's own timeout"; the timeout added in 85d2378 lives in subscription-registry (withEstablishTimeout), not provider-ipc. Cheap pointer fix so the next reader greps the right module.

  3. Carry-over ledger — previously-raised minors still present at 85d2378 (listed for tracking, not re-derived): connection-banner auto-approve badge ignores autoApprove.messaging (swarm-connect.js:497); approveSwarmConnect catch only logs — neither resolve nor reject on grant/round-trip failure (swarm-connect.js:459), and approveSwarmMessaging/approveSwarmPublish have no try/catch around setAutoApprove before resolve(); GSOC subscribe missing-both error text says "not both" (swarm-provider-ipc.js:1709); subscription socket URL computed once, never recomputed across reconnects (messaging-service.js:195); cross-type prompt concurrency still auto-rejects the other screen's queue (documented design); queued prompt entries never re-evaluated against state changed by an earlier approval (redundant double-ask).

Evidence note: both R2 blocking findings were verified with throwaway jest repros driving the real subscription-registry + swarm-provider-ipc modules (electron/permissions/fetch mocked); scratch files deleted, tree clean, and the five changed swarm/renderer suites pass unmodified (224 tests, npx jest green). No visual pass — per the R1 precedent these are main-process protocol paths with no harness fixture driving window.swarm.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-verify R2] Both findings CONFIRMED — reproduced independently against the real modules, plus a real-Electron check of the navigation semantics they rest on.

R2-F1 (in-page navigation kills every subscription) — CONFIRMED.
deliverSubscriptionMessage compares contents.getURL() to the snapshotted pageUrl with exact string equality, and a hash/pushState navigation changes that string without firing did-navigate. Verified empirically on a real Electron webContents loading an http://127.0.0.1:PORT/bzz/chat.eth/index.html page (the same internal URL shape dweb pages get):

did-navigate            http://127.0.0.1:PORT/bzz/chat.eth/index.html
AFTER LOAD    getURL =  http://127.0.0.1:PORT/bzz/chat.eth/index.html
location.hash = "#/room/42"
did-navigate-in-page    http://127.0.0.1:PORT/bzz/chat.eth/index.html#/room/42   <- did-navigate does NOT fire
AFTER HASH    getURL =  http://127.0.0.1:PORT/bzz/chat.eth/index.html#/room/42

(history.pushState() likewise: getURL() became file:///pushed/path, only did-navigate-in-page.)

Fed into a jest harness using the real swarm-provider-ipc + real subscription-registry (only electron/messaging-service/permissions mocked): message #1 delivers, the hash flips, message #2 is dropped and countByOrigin goes 1 -> 0 — the whole tab’s subscriptions are torn down, and the only IPC ever sent to the page is that first message, so a hash-routed dApp (the normal case for a chat room) has no way to learn it must resubscribe.

R2-F2 (pre-subscribe navigation binds the subscription to the new page) — CONFIRMED.
The renderer awaits a user-paced prompt (swarm-provider.js handleMessagingRequest -> showSwarmMessagingApproval) with permissionKey captured before the prompt, and nothing cancels a pending messaging prompt on navigation (swarm-connect.js only refreshes a banner on navigation-completed). Main then adds its own await (checkBeeReachable) before attachWebContentsTeardown and before pageUrl: contents.getURL(). So a navigation inside that window: (a) fires did-navigate when no listener is attached and no registry entry exists — teardown cancels nothing; (b) is then snapshotted as the subscription’s pageUrl. Reproduced: flipping getURL() to .../bzz/evil.eth/index.html during the reachability fetch still returned a live subscriptionId for chat.eth, and the next PSS payload was contents.send()-ed into the evil.eth document with the line-1664 check passing. The preload bridge does not filter (webview-preload.js swarm:provider-event -> FREEDOM_SWARM_EVENT -> emitEvent(...) to any window.swarm.on("message") listener), so the destination page needs no messaging grant to receive it. Note subscription.origin is never re-checked against pageUrl at delivery time — an origin/pageUrl consistency check would close this.

Repro harness was temporary and has been deleted; no repo state changed.

…r awaits

R2-F1: the delivery guard compared the webContents' live URL against an
exact snapshot of the subscribing document's URL. An in-page navigation
(hash route, pushState) changes getURL() without firing did-navigate, so
the first route change of a hash-routed dApp dropped the message and
cancelled every subscription on the webview — with no event to the page,
which stayed alive with dead messaging. Compare normalized permission
keys instead (path/query/hash-insensitive, same key the grant is stored
under), and cancel only the subscriptions whose origin is actually gone
via the new registry.cancelStaleByWebContents().

R2-F2: pageUrl was snapshotted after the user-paced messaging grant
prompt and the reachability probe, so a navigation in that window bound
origin A's subscription to whatever page had landed in the webview — and
the delivery-time check then validated the leak. did-navigate can't help:
it fires before any registry entry exists. Re-check the live page's
origin after the awaits and refuse with a retryable 4900 if it no longer
matches the caller.

Verified against the real modules: pre-fix, a hash change dropped
delivery (send count 1, countByOrigin 0) and a navigation during the
await returned a live subscription id delivering into evil.eth; post-fix,
delivery survives the hash change (count 2, countByOrigin 1) and the
racing subscribe is refused with no socket opened.
@meinharrd

Copy link
Copy Markdown
Contributor

[alan-fix R2] Both confirmed findings fixed in a436c09.

R2-F1 — in-page navigation killed messaging (swarm-provider-ipc.js, delivery guard)
Root cause was the exact-URL comparison: a hash route / pushState changes getURL() without firing did-navigate, so the same live document failed the check. Now the guard compares normalized permission keys (normalizeOrigin(contents.getURL()) !== subscription.origin) — the same key the messaging grant is stored under, and path/query/hash-insensitive by construction, so an in-page navigation is a no-op while a real cross-origin swap still fails. The mismatch path no longer calls cancelByWebContents (which nuked every subscription on the webview, including any the new page had legitimately opened); a new subscriptionRegistry.cancelStaleByWebContents(webContentsId, currentOrigin) cancels only the subscriptions whose origin is actually gone. pageUrl is dropped from the registry entry — origin was already there and is the correct key.

R2-F2 — grant-prompt race bound a subscription to the page that navigated in (swarm-provider-ipc.js, handleSubscribe)
Root cause was snapshotting the page identity after the user-paced awaits. Removing the snapshot alone isn't enough — the registry entry would still be aimed at a webContents now hosting someone else, and did-navigate can't save it because it fires before the entry exists. Added an explicit re-check of the live page right after the grant/reachability awaits and before anything is registered: if normalizeOrigin(contents.getURL()) !== origin, subscribe returns a retryable 4900 / subscription_cancelled (same shape as the existing "torn down while we waited" path) and never opens a socket.

Evidence — repro'd both against the real subscription-registry + swarm-provider-ipc (only electron/services stubbed), then re-ran on the fix:

scenario pre-fix post-fix
R2-F1: subscribe at bzz://chat.eth/#/lobby, then #/room/42, then a message send count 1, countByOrigin 0 send count 2, countByOrigin 1
R2-F2: tab navigates to bzz://evil.eth/ during the await {result:{subscriptionId:"2426…"}}, 1 socket opened, delivery into evil.eth {error:{code:4900, reason:"subscription_cancelled"}}, 0 sockets opened, no delivery

Both scenarios are now permanent regression tests (delivery survives an in-page navigation (hash route / pushState), refuses to bind the subscription when the page navigated while we waited), plus a registry test that cancelStaleByWebContents spares the origin now loaded. Mutation-checked: reverting either guard fails exactly those two tests.

Suites: npx jest src/main/swarm/ 471 passed; full unit suite 2256 passed / 1 failed — the known pre-existing vault auto-locks after timeout flake (fails identically in isolation on a clean tree). eslint src/main/swarm clean. xvfb-run npx playwright test --project=harness test-e2e/tabs.spec.js 2 passed (app still boots/navigates).

No screenshot: this change has no UI surface — the delivery path is main-process IPC into a webview, and exercising window.swarm messaging needs a live Bee node that the fixture harness project (offline by design) can't provide. The table above is the acceptance evidence.

@meinharrd

Copy link
Copy Markdown
Contributor

[alan-review R3] Fresh pass at a436c09. No new blocking findings — I re-traced both R2 fixes against the real modules (origin-keyed delivery guard, post-await page re-check, cancelStaleByWebContents) plus the full prompt-queue machinery, ran all 14 swarm/renderer suites (480 green) and eslint (clean), and verified that bzz://name.eth URLs survive into webContents.getURL() (navigation.js loads ENS sites as bzz://<name>/, so normalizeOrigin(getURL()) and the renderer's permission key agree — the guards are sound for every dweb shape I could construct).

Minor findings (combined). #1 verified with a throwaway jest repro driving the real subscription-registry + swarm-provider-ipc (only electron/permissions/fetch/socket faked; deleted, tree clean). No visual pass: all four live in main-process protocol/policy paths with no steerable UI surface (same rationale as R1/R2).

  1. src/main/swarm/swarm-provider-ipc.js:1783 — same-origin ghost subscription survives the post-await re-check. The R2-F2 guard compares permission keys, so a same-origin cross-document navigation during the grant-prompt/reachability awaits passes it: page1 (bzz://chat.eth/page1.html) calls subscribe, user navigates to bzz://chat.eth/page2.html mid-await (did-navigate fires with no entry to cancel), subscribe resumes, re-check passes (chat.eth === chat.eth), and the subscription binds live. Repro output: subscriptionId returned, and the next PSS payload is contents.send()-ed into page2 — a document that never subscribed and holds an unknown subscription id; the response itself lands in page2's preload where the request id is unknown, so nobody owns the subscription. Confined to the same origin (no grant/privacy boundary crossed) and self-healing (next cross-document navigation cancels it; the event carries the sub id so the page could unsubscribe), hence minor — but it does silently consume one of the origin's 32 slots per occurrence. A navigation-epoch snapshot taken at request entry (as sketched in R2) would close this residue too.

  2. src/main/swarm/swarm-provider-ipc.js:17-21 — trust-model header now contradicts the code. Rationale (c) says webContents.getURL() "cannot be used because dweb pages resolve through the request-rewriter — the internal URL doesn't carry the dweb protocol identity". That was true pre-protocol-handlers, but both new security guards (delivery check line 1674, subscribe re-check line 1783) now depend on getURL() carrying dweb identity — and it does, because bzz:/ipfs:/ipns: are served by custom protocol handlers that preserve the URL. If a future reader believes the comment, the guards look broken; if a future transport reintroduces gateway-URL loading for some scheme, the guards silently fail closed (messaging bricked for that scheme) with nothing pointing at why. Update the comment to state the new invariant the guards rely on.

  3. PSS subscription scope: any granted origin can read any topic's node-decrypted traffic. handleSubscribe puts no origin constraint on PSS topics, and the registry multiplexes per (kind, key) with fan-out to every subscribed origin — so origin E with a messaging grant subscribing to another dApp's well-known topic receives that dApp's directed PSS messages, decrypted by the node key. This is inherent to bee-wallet-mode PSS (single node inbox) and matches the SWIP, so not a code defect — but the grant-prompt copy (swarm-connect.js:765) says only "discloses a stable identity key / uses stamps / bandwidth" and never that the site gains read access to the node's whole PSS inbox by topic. Worth a line of consent copy, and a note in the module docs.

  4. src/main/swarm/swarm-service.js selectBestBatch({allowFullMutable}) — messaging sends can silently evict stamp protection from previously published content. When no batch has remaining capacity, messaging falls back to a full mutable batch, where each new stamp overwrites the batch's oldest — and the eviction victims are whatever those old stamps protected, e.g. the user's published site uploaded on that same batch. The only signal is a main-process log.warn; with messaging auto-approve enabled, sustained chat traffic can degrade older published content with zero user-visible indication. The in-code comment frames the trade-off as "acceptable for ephemeral traffic", but the ephemerality of the messages doesn't make the evicted content ephemeral. Consider refusing (as content publishes do) or surfacing the fallback in the send prompt/UI.

Carry-over note: the minor findings from earlier rounds (gsoc missing-both error copy, reconnect redialing a stale WS URL, post-grace 1013 classification, back buttons outside the arm-window disable set, banner badge without messaging, approve-handler IPC failures leaving requests unsettled, queued prompts not re-evaluated, showNext present-throw robustness) remain unaddressed at a436c09 — none escalate to blocking.

@meinharrd meinharrd added alan:clean alan loop finished: no confirmed blocking findings and removed alan:reviewing alan loop currently running on this PR labels Aug 11, 2026
…y, trust-model doc

- Close the same-origin cross-document subscription residue: a navigation
  during the grant-prompt/reachability awaits fires did-navigate with no
  entry yet to cancel, and the origin re-check passes (chat.eth → chat.eth),
  so the new document was bound a subscription it never asked for. Snapshot
  a per-webContents navigation epoch at request entry and re-check it after
  the awaits (in-page hash/pushState fires no did-navigate, so a live page's
  own routing is unaffected). Regression test added.
- Messaging grant consent copy now states that a subscription can read any
  PSS traffic the node decrypts for the joined topic, not only the site's
  own messages (inherent to bee-wallet-mode single-inbox PSS).
- Trust-model header rewritten: it claimed getURL() is unusable, but both
  security guards now depend on it carrying dweb identity (preserved by the
  bzz/ipfs/ipns protocol handlers). Documented the invariant and the
  fail-closed behavior if a future transport breaks it.

Deferred (design decision, noted on the PR): selectBestBatch full-mutable
fallback can evict older content's stamp protection under sustained
messaging — refuse-vs-surface needs a maintainer/UX call.
@meinharrd

Copy link
Copy Markdown
Contributor

Addressed the R3 minors in 01aee1c (branch also merged with main — preload.js swarm-exposures conflict resolved keep-both):

  1. Same-origin subscription residue — added a per-webContents navigation-epoch snapshot taken at request entry and re-checked after the awaits. A cross-document navigation (fires did-navigate) now cancels the subscribe even when it stays on the same origin; in-page hash/pushState fires no did-navigate so a live page's own routing is unaffected. Regression test added (472/472 swarm tests green).
  2. Trust-model header — rewritten to state the invariant both guards now depend on (getURL() carries dweb identity via the bzz/ipfs/ipns protocol handlers), including the fail-closed behavior if a future transport reintroduces gateway-URL loading.
  3. PSS scope consent copy — the messaging grant warning now says a subscription can read any PSS traffic the node decrypts for the joined topic, not only the site's own messages.
  4. selectBestBatch full-mutable eviction — left as a documented follow-up: refusing breaks messaging when all batches are full, and surfacing needs UX for the auto-approve path, so the refuse-vs-surface behavior is a maintainer/UX call rather than something to change unilaterally here.

Full suite green except the known vault auto-lock load flake (fails on main too).

@meinharrd
meinharrd merged commit 340c3a0 into main Aug 12, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

alan:clean alan loop finished: no confirmed blocking findings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants