fix(auth): ship plugin-derived credential fields read-denied - #1013
Conversation
Session.token, Verification.value, and Account.password/accessToken/ refreshToken/idToken now carry a field-level read deny in deriveAuthLists, so granting operation-level access to a derived auth list no longer also exposes these live credentials through context.db or the admin UI. A denied field is stripped, not an error; sudo() still reads all six. The deny is keyed to better-auth's own model/field, so it survives a modelName or column remap. Closes #981 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3qvRgej7jAvdbvNC3c8C3
|
Deployment failed for project stack-docs with the following error: Learn More: https://vercel.com/open-saas?upgradeToPro=build-rate-limit |
🦋 Changeset detectedLatest commit: 39d8cbc The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cff354b66c
ℹ️ 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".
Review (self-review, high effort)One finding surfaced, worth recording even though I'm treating it as out of scope for this PR: Plugin-table credential fields (e.g. the This is real, but both the issue and ADR-0036 explicitly enumerate exactly six fields across the four base models ( Generated by Claude Code |
Coverage Report for Core Package Coverage (./packages/core)
File CoverageNo changed files found. |
Coverage Report for UI Package Coverage (./packages/ui)
File CoverageNo changed files found. |
Coverage Report for CLI Package Coverage (./packages/cli)
File CoverageNo changed files found. |
Coverage Report for Auth Package Coverage (./packages/auth)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Coverage Report for Storage Package Coverage (./packages/storage)
File CoverageNo changed files found. |
Coverage Report for RAG Package Coverage (./packages/rag)
File CoverageNo changed files found. |
Coverage Report for Storage S3 Package Coverage (./packages/storage-s3)
File CoverageNo changed files found. |
Coverage Report for Storage Vercel Package Coverage (./packages/storage-vercel)
File CoverageNo changed files found. |
A Codex review comment on the PR flagged the changeset's blanket "not an error" claim. Verified the actual behavior: findMany's/count's where/orderBy naming a denied credential field throws ValidationError up front (validateQueryFieldReadAccess); findUnique's where only unique-selects the row and is not walked by that check, so a lookup by the denied field still succeeds with the field stripped from the result. Corrected the changeset, docs/content/reference/auth.md, and packages/auth/CLAUDE.md to describe both paths accurately, and added e2e coverage for the findMany-throws case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L3qvRgej7jAvdbvNC3c8C3
…ck653y # Conflicts: # packages/auth/src/config/derive-auth-lists.ts
Summary
Implements ADR-0036: the derived auth lists' credential-bearing fields —
Session.token,Verification.value, andAccount.password/accessToken/refreshToken/idToken— now carry a field-levelreaddeny thatderiveAuthListssets unconditionally when it derives the list. Previously, granting operation-levelqueryaccess to a derived auth list (e.g. to build a "your active sessions" screen) had no way to withhold just the credential column — the safe configuration was unexpressible (see the issue for the full analysis).CREDENTIAL_FIELDS+withCredentialAccessinderive-auth-lists.ts, applied in the scalar-field derivation loop. Keyed by better-auth's own model/field key (not the app's list key or columndb.map), so the deny survives amodelNameremap and a columnfieldsoverride.sudo()still reads all six fields — the supported path for a genuine need.ipAddress,userAgent,providerId,accountId,expiresAt,identifier, everyUserfield) stay open.examples/starter's ownpasswordfield (not the auth-plugin-derived one — this example doesn't useauthPlugin) also gets areaddeny, with a comment pointing atcontext.sudo()for an app's own auth implementation.packages/auth/CLAUDE.mdanddocs/content/reference/auth.mdto document the deny.@opensaas/stack-auth(pre-1.0 breaking-behavior policy) naming each affected field.Test plan
derive-auth-lists.test.ts: each of the six fields deniesread; identifying fields and everyUserfield stay open; the deny survives amodelName+ columnfieldsremap.credential-field-read-deny-e2e.test.ts(gated behindRUN_CREDENTIAL_DENY_E2E=1, same offline-toolchain pattern asrate-limit-e2e.test.ts/mcp-oauth-cascade-e2e.test.ts): generates a real schema, pushes to real SQLite, and proves — against a realbetterAuth()instance — that an openedcontext.dbread stripstoken/password/accessToken/refreshToken/idToken/valuewhilesudo()still reads them, and that sign-up, sign-in, session refresh, and password reset all still work end-to-end with the denies in place. Wired into.github/workflows/test.yml'se2ejob.examples/starterregenerates cleanly (opensaas generate) with the new field-level access.pnpm --filter @opensaas/stack-auth test(275 passed),pnpm --filter @opensaas/stack-core test(1164 passed)pnpm lint,pnpm manypkg fix,pnpm formatCloses #981
Generated by Claude Code