fix(authz): close stale-membership access in chat, align REST session freshness with tRPC - #438
Merged
Merged
Conversation
…g chat session The chat route trusted chatSessions.organizationId — a stored value written when the session was created — to reach the organization's file search store, RAG settings and document proxy URLs. A user removed from the organization kept that access for as long as the chat session row existed, while the documents and uploads routes revoked it correctly. Resolve the chat session, then run requireOrgAccess against its organizationId before any org-scoped read. The guard reuses the per-request session memo, so this costs one membership lookup and no extra session round trip. Also pin every guard entry point, not just requireUser, to an uncached session read, so a guard that grows its own getSession call cannot silently reopen the gap between the REST edge and createTRPCContext.
filopedraz
pushed a commit
that referenced
this pull request
Sep 7, 2026
Resolves bun.lock by regeneration; package.json auto-merged cleanly and keeps the AI SDK v7 pins alongside husky 9, exceljs and Node 22 from #436. Fixes a semantic conflict git could not see. #438 added __tests__/api/chat/route.test.ts with mocks written against the v5 AI SDK surface, while this branch migrated the route to v7: - @ai-sdk/google: createGoogleGenerativeAI -> createGoogle - ai: result.toUIMessageStreamResponse() -> createUIMessageStreamResponse({ stream: toUIMessageStream(...) }) Both mocks now match the surface the route actually calls. The merge was textually clean and the suite failed at collection, which is the only reason this was caught. Gate verified: typecheck, lint, 744 tests across 55 files, format:check, knip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GyBfVKhYQGLZhkcWiHaruY
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
1 participant
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.
What's Changed
Two authorization inconsistencies found while extracting the REST guards in #434, both deliberately left out of that PR because they change behavior.
1. Stale-membership access in the chat route.
app/api/chat/route.tsverified that a chat session belonged to the caller, then readorganizationIdoff that session row and used it to reach the org's file search store and RAG settings, without checking that the user was still a member. A user removed from an organization kept working access to that org's document store through any session created while they were a member. Now goes throughrequireOrgAccess.2. REST accepted cached sessions while tRPC did not.
createTRPCContextpassesdisableCookieCache: true; the REST guards used the default. A revoked session stayed valid at the REST edge for up tocookieCache.maxAge(300s) while tRPC rejected it in the same instant. Two doors, two answers, same credential. Fixed in one place, which is the point of having extracted the guards.Type of Change
Testing
49 files, 582 tests passing. New tests cover a deleted membership row returning 403 on an existing chat session, and a guard-level assertion that the uncached session path is requested so this cannot silently regress.
The latency tradeoff
Measured in-sandbox, 2000 iterations after 200 warmup, against running Redis and Postgres.
storeSessionInDatabaseis false andsecondaryStorageis Redis, so the marginal cost is one RedisGET, not a Postgres query.Loopback Redis is a floor; against a same-region managed instance the honest estimate is +0.5 to 1.5 ms per REST request. The REST surface is four routes: two serve files, one takes a Stripe webhook, and the fourth streams an LLM response measured in seconds. This buys back a 300-second window where the two doors disagreed about the same credential.
The chat route's new membership lookup measured p50 0.136 ms, p95 0.194 ms.
Follow-up found, not fixed here
orgOwnerProcedureinlib/trpc/init.tsauthorizes offctx.activeOrganizationIdandctx.orgRole, both read from the session row, with noorgMembershipslookup.activeOrganizationRoleis written only by the session-create hook and the active-org-change hook; nothing inmember-servicerewrites existing sessions. So demoting an owner to member, or removing them, leaves existing sessions carryingactiveOrganizationRole: 'owner'until they switch orgs or re-authenticate, and that procedure gates delete-organization and billing.disableCookieCachedoes not help, because the stale value is in the session row itself. The fix is oneorgMembershipslookup in that middleware, same shape asorgProcedure.🤖 Generated with Claude Code
https://claude.ai/code/session_01GyBfVKhYQGLZhkcWiHaruY