fix(appkit): bind SSE streams to creator and reject cross-user reconnects#312
Draft
MarioCadenas wants to merge 1 commit intomainfrom
Draft
fix(appkit): bind SSE streams to creator and reject cross-user reconnects#312MarioCadenas wants to merge 1 commit intomainfrom
MarioCadenas wants to merge 1 commit intomainfrom
Conversation
…ects The StreamManager registry was a global lookup keyed only by streamId. Any request that supplied a known or guessable streamId could attach to an existing stream via _attachToExistingStream and receive its events, because there was no authorization step on reconnection. In the Genie plugin this was directly exposed: requestId comes from the client query string and is passed through as streamId, so guessing or replaying another user's requestId leaked their conversation events. Fix: thread the calling principal's user key (resolved by Plugin.executeStream the same way it's used for cache scoping) through to the stream manager and store it on the stream entry as ownerKey. On reconnection, only attach when the requesting caller's owner key matches the stream's owner key; otherwise emit a STREAM_FORBIDDEN error and end the connection. This applies to every plugin that uses executeStream, including Genie and Analytics, with no per-plugin changes required. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
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.
Summary
The
StreamManagerSSE registry was a global lookup keyed only bystreamId. Any request that supplied a known or guessablestreamIdcould attach to an existing stream via_attachToExistingStreamand receive its events — there was no authorization step on reconnection. In the Genie plugin this was directly reachable:requestIdis read from the client query string and passed through asstreamId, so guessing or replaying another user'srequestIdleaked their conversation events.The fix binds every stream to the principal that created it and refuses reconnections from any other principal.
Changes
Plugin.executeStreamfor cache scoping —userKey ?? getCurrentUserId()) intoStreamManager.stream(...)as a newownerKeyargument.ownerKeyonStreamEntrywhen a stream is created.ownerKeydoes not strictly equal the requesting caller'sownerKey. The response gets aSTREAM_FORBIDDENSSE error event and is ended; no events from the original stream are leaked, and the requested generator is not started.STREAM_FORBIDDENtoSSEErrorCode.The fix is centralized — every plugin that streams via
executeStream(Genie, Analytics, and any future plugin) now gets per-creator binding without per-plugin changes. Analytics already passed an explicitexecutorKey(resolveUserId(req)for OBO,"global"for service-principal queries) which now also serves as the stream owner key, preserving its existing scoping.Backward compatibility for direct
streamManager.stream(...)callers without anownerKeyis preserved (undefined === undefinedmatches).Test plan
packages/appkit/src/stream/tests/stream.test.ts:STREAM_FORBIDDEN, no buffered events leak, and the new generator does not run.pnpm build,pnpm check:fix,pnpm -r typecheck,pnpm testall green.requestIdquery param and verify the response is aSTREAM_FORBIDDENSSE error rather than the other user's events.