Fix cross-channel Mercure whisper routing - #1
Merged
dunglas merged 3 commits intoSep 8, 2026
Merged
Conversation
J3m5
marked this pull request as ready for review
September 8, 2026 07:05
J3m5
marked this pull request as draft
September 8, 2026 07:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Whisper topics are currently multiplexed over a single EventSource, while
the destination channel is read from the publisher-controlled JSON envelope.
A member authorized to publish whispers on channel A can publish an envelope
naming channel B. A recipient subscribed to both channels can then dispatch
that client event to B's listeners, even though the sender has no publishing
permission on B's whisper topic. A joined public channel can also be targeted.
The existing client-* filter prevents this from becoming arbitrary
server-event injection, but does not enforce isolation between channels.
Change
Use one exact-topic whisper EventSource per authorized guarded channel.
The receiving stream supplies the destination channel, and the envelope
must name that same channel before dispatch or decryption.
The change also keeps independent replay cursors, closes streams when their
channels are left, and ignores callbacks from replaced streams. Terminal
failures continue to use the shared reauthentication and reconnect path.
The main broadcast stream remains multiplexed. No backend changes,
additional dependencies, or changes to the whisper payload format or
encryption format are required.
Tradeoff
Native EventSource does not expose the Mercure publishing topic, so an
untrusted envelope cannot safely route multiplexed whispers by itself.
This approach requires one main SSE stream plus one whisper stream per
guarded channel when client events are enabled. HTTP/2 or HTTP/3 is
recommended, but connection/stream limits and server resource usage still
need to be considered.
The PR is initially a draft so this transport tradeoff can be reviewed
alongside the isolation fix.
Tests
Regression coverage includes cross-channel routing to joined private,
presence and public channels; mismatched encrypted envelopes; independent
replay cursors; leaving and rejoining channels; stale stream callbacks;
shared reconnection; and stream cleanup when client events are disabled
or the connector disconnects.
Existing tests continue to cover normal whispers, sender exclusion,
rejection of non-client events, and encrypted whispers.
Validation
Validation was run after formatting, on the tree recorded in formatting-only commit
eb8a0e662ea98f38bdf3a8d2026f182c9e5c98a3, using Node 24.20.0, pnpm 12.3.4 and Vitest 5.0.0. Dependencies, lockfile and lint configuration were unchanged. Commands below were run from the Echo root throughmise exec node@24.20.0 pnpm@12.3.4 --.pnpm --filter laravel-echo exec prettier --check src/connector/mercure-connector.ts tests/connector/mercure-connector.test.ts: passed on both files (exit 0).pnpm --filter laravel-echo exec vitest run tests/connector/mercure-connector.test.ts: 58 tests passed (exit 0).pnpm --filter laravel-echo exec vitest run: 98 tests passed across 8 files (exit 0).pnpm --filter laravel-echo run typecheck: passed (exit 0).git diff --check: passed (exit 0).pnpm --filter laravel-echo run lint: failed (exit 1). ESLint still reports 4 errors and 37 warnings, also reproduced on base commit09d8f8fa2755bea028690f292b355b33bc000a7dwith the same toolchain. No new diagnostics were introduced. The existing lint issues are outside the scope of this change.The ESLint JSON reports were compared by rule, severity, message, relative file, columns and source context, mapping shifted line ranges back to unchanged base lines. All 41 diagnostics matched exactly. The four errors are
@typescript-eslint/no-unnecessary-type-assertionin the connector (base lines 404, 422, 440 and 1659; branch lines 406, 424, 442 and 1668). Warning rules are@typescript-eslint/no-unsafe-call(18),no-unsafe-return(8),no-unsafe-member-access(7),no-unsafe-argument(3) andno-unsafe-assignment(1), all under the@typescript-eslintnamespace. The global lint check is not green.Before the fix, a previous native Vitest run used a detached worktree at the base commit with only the final regression test file applied and the original connector retained.
pnpm --filter laravel-echo exec vitest run tests/connector/mercure-connector.test.ts -t 'a whisper cannot target another joined'produced 3 assertion failures and 55 skipped tests. Each failure showed the wrong channel's callback being invoked once, for private, presence and public targets. These were not installation, import or compilation failures. After the fix, all three cases pass within the connector and full-package suites rerun above.The additional commit changes only formatting of two expressions in the test file; assertions and scenarios are unchanged.
These tests use MockEventSource. They do not establish real-browser or
real-hub interoperability unless separately tested and reported here.
This contribution targets the Mercure connector branch used by
laravel#549, the companion to laravel/framework#61474.