fix(realtime): scope sse streams by authorized route#786
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdc2ae48d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| define([]ast.Expr{id("audiences")}, &ast.IndexExpr{X: id("realtimeSubscriptionAudiences"), Index: selExpr(id("event"), "Type")}), | ||
| &ast.IfStmt{ | ||
| Cond: &ast.BinaryExpr{X: call(id("len"), id("audiences")), Op: token.EQL, Y: intLit(0)}, | ||
| Body: block(&ast.ReturnStmt{Results: []ast.Expr{eventEnvelopeSliceExpr(id("event"))}}), |
There was a problem hiding this comment.
Preserve broadcast delivery for mixed subscription owners
When an event type has any page-owned subscription, realtimeSubscriptionAudiences[event.Type] becomes non-empty, so this branch emits only route-labeled copies. If the same presentation event type is also subscribed from a component/non-page owner, that unscoped subscriber has no route audience and no broadcast copy is sent, so apps mixing a shared component subscription with a page-specific subscription stop receiving those realtime updates outside the labeled routes. Track whether the event type also has unscoped owners and append the original event for them.
Useful? React with 👍 / 👎.
| Body: block(&ast.RangeStmt{ | ||
| Key: id("_"), | ||
| Value: id("audience"), | ||
| Tok: token.DEFINE, | ||
| X: &ast.IndexExpr{X: id("realtimeQueryAudiences"), Index: id("query")}, |
There was a problem hiding this comment.
Preserve unscoped queries in mixed invalidation notices
For a generated gowdk.query.invalidate notice containing both route-owned and component/non-page query types, this loop only carries forward queries that have entries in realtimeQueryAudiences. Once at least one route audience is found, the function returns scoped notices instead of the original broadcast, so any unscoped query in the same notice is omitted from every event and those regions never receive the realtime refresh. Keep a separate broadcast notice for queries without route audiences, or otherwise include them when splitting scoped notices.
Useful? React with 👍 / 👎.
| Cond: &ast.BinaryExpr{X: selExpr(id("event"), "Type"), Op: token.EQL, Y: sel("gowdkcontracts", "QueryInvalidationPresentationEventType")}, | ||
| Body: block(&ast.ReturnStmt{Results: []ast.Expr{call(id("realtimeQueryInvalidationAudienceEvents"), id("event"))}}), |
There was a problem hiding this comment.
Route-scope generated invalidation notices
For page-owned query invalidations, the generated notice is emitted by currentContractEventSink via QueryInvalidationCommandEventSink(fanout, realtimeQueryInvalidations) before fanoutSink, so it is sent straight to SSE with an empty Audience and never reaches this scoping branch. A client connected to another authorized route therefore still receives the invalidation notice (query type/event IDs) for this route. Route-scope the invalidation sink itself or pass it through the realtimeSubscriptionFanout wrapper.
Useful? React with 👍 / 👎.
| define([]ast.Expr{id("requestPath")}, call( | ||
| selExpr(call(selExpr(selExpr(id("request"), "URL"), "Query")), "Get"), | ||
| stringLit("path"), |
There was a problem hiding this comment.
Do not trust the client-supplied stream route
The route audience is derived from the path query parameter, so any client can open /_gowdk/realtime/events?path=/some/other/page and be assigned that page's audience (subject only to that selected route's guards). For public or broadly authorized routes this bypasses the route isolation this change is trying to enforce and lets a page receive another page's scoped presentation events. Derive the audience from server-owned state, or treat this parameter only as a hint rather than authorization.
Useful? React with 👍 / 👎.
Summary
WithSSEAudienceFromRequest(realtimeStreamAudience), using the same generated route matcher that selects stream guards./patientssubscribes toPatientNoticeand/dashboardsubscribes toOtherNotice; the/patientsstream fails if the dashboard event arrives.Issue Closure
Fixes #778
Verification
scripts/test-go-modules.shwhen Go code or compiler behavior changed.go build ./cmd/gowdkwhen CLI, compiler, runtime, addon, or release behavior changed.node --check editors/vscode/extension.jswhen editor files changed.Commands run:
go test ./internal/appgen -run 'TestGenerateWritesRealtime|TestGenerateRealtime|TestGeneratedBinaryRealtime' -count=1go test ./internal/appgen ./runtime/contracts/sse ./runtime/realtime ./addons/realtime -count=1go build ./cmd/gowdkgo test ./...git diff --checkLLM Assistance