Repository navigation
fix(2738): identity helpers honor pinned attr.dbUserId before tiered/sapId path - #2739
Merged
Merged
Conversation
…sapId path
resolveDbUserScoped and getOrProvisionDbUser (packages/core/resolve-db-user.js)
did not consult the before('*')-pinned req.user.attr.dbUserId, resolving via the
tiered resolveUser / sapId path instead. A PAT req.user (srv/lib/mcp-pat-middleware.js)
pins the exact attr.dbUserId but has NO iss/sub and, for a migrated user, a NULL
sapId. Routed into these non-*For handlers, a write would mint a spurious
sapId=<email-shaped user.id> Users row and a read would return empty for the
pinned user.
Both helpers now resolve Users by the pinned PK first (mirroring resolveDbUser's
existing attr.dbUserId fast-path), falling through to the tiered/sapId path only
when the pin is absent or its row is gone. getOrProvisionDbUser threads req.tx
when present and falls back to ambient otherwise, matching resolveDbUserScoped
and avoiding the single-connection SQLite deadlock cds.tx(req) causes on a
tx-less synthetic req.
Adds PAT-shaped regression tests (pinned dbUserId, null sapId, no iss/sub)
asserting both helpers resolve the pinned row and mint no spurious row.
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.
Closes #2738.
Problem
Neither
resolveDbUserScopednorgetOrProvisionDbUser(packages/core/resolve-db-user.js) consulted the pinnedreq.user.attr.dbUserIdset by the globalbefore('*')hook (pinResolvedUser). They resolved via the tieredresolveUser/ sapId path instead.A PAT
req.user(srv/lib/mcp-pat-middleware.js) pins the exactattr.dbUserIdbut has noiss/suband, for a migrated user, a NULL sapId. Routed into a non-*Foruser-scoped handler:getOrProvisionDbUser):hasTokenfalse → sapId branch →resolveUserSapIdreturns the email-shapeduser.id→ mints a spurioussapId=<email>Users row.resolveDbUserScoped+ the twobefore('READ')filters): resolve by the same path → return empty for the pinned user.Not a current live bug (no MCP tool routes a PAT into these non-
*Forhandlers yet — PAT writes go through srv-mcp*Foractions), but it is a latent trap for the planned PAT write tools (complete_step/reset, "Task 12"). Behavior was byte-identical to pre-#2730 at these sites.Fix
Both helpers now honor the pin first: when
req.user.attr.dbUserIdis set, resolveUsersby thatIDbefore falling to the tiered/sapId path — mirroringresolveDbUser's existing fast-path. If the pin is set but the row is gone (deleted mid-request), both fall through to the tiered path.getOrProvisionDbUser's pinned path threadsreq.txwhen present and falls back to ambient otherwise (matchingresolveDbUserScoped), avoiding the single-connection SQLite deadlockcds.tx(req)causes on a tx-less syntheticreq.Tests
Added PAT-shaped regression tests to
test/unit/resolve-db-user-scoped.test.js(pinneddbUserId, null sapId, no iss/sub): both helpers resolve the pinned row and mint no spurioussapId=<email>row.Verified green locally (file-scoped vitest):
resolve-db-user-scoped(4),developer-service-identity-reads/-writes(14),resolve-db-user-{provision,resolve}+resolve-db-user+session-favorites-identity(46).Affected sites covered
The ~16 user-scoped handlers converted in #2736, plus
getMyFavorites/toggleSessionFavoritefrom #2734 — all route through these two helpers.