fix(auth): unwedge PGlite sign-up by routing single-user guard through the transaction adapter#952
Merged
Merged
Conversation
…h the transaction adapter The `user.create.before` and `account.create.before` hooks called `getDb()` for a fresh pool connection while Better Auth's `/api/auth/sign-up/email` (and OAuth registration) endpoints already held the only one via `runWithTransaction`. In PGlite mode the pool is sized 1 (LOBU_DISABLE_PREPARE → poolMax=1), so the hook deadlocked the whole request — no log line, no response, curl headers-timeout. Routes the count + lookup through `ctx.context.internalAdapter` (`countTotalUsers` / `findUserById`), which reuses the in-flight transaction connection. Declares `principalKind` as a Better Auth additional field (input/returned: false, fieldName 'principal_kind') so the where-clause field name resolves without BA throwing "Field principal_kind not found in model user". Fail-closed if BA ever invokes the hook without an auth context. Closes #947.
📝 WalkthroughWalkthroughAdds a non-input/non-returned ChangesPrincipal kind schema and auth hook refactoring
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthServer
participant InternalAdapter
participant Database
Client->>AuthServer: POST /api/auth/sign-up/email (signup payload)
AuthServer->>InternalAdapter: countTotalUsers({ excludeKinds: ["install_operator","bootstrap-user"] })
InternalAdapter->>Database: SELECT COUNT(...) filtering principal_kind
Database-->>InternalAdapter: count
InternalAdapter-->>AuthServer: count result
AuthServer->>AuthServer: allow or reject signup based on count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/server/src/auth/index.tsx`:
- Around line 766-773: The principalKind guard for OAuth account linking uses
optional chaining on ctx which silently allows linking when ctx is missing;
update the account-linking flow (the block using
ctx?.context.internalAdapter.findUserById, linkedUser, and the principalKind ===
"install_operator" check) to fail closed like the user.create.before hook:
explicitly require ctx (throw the same APIError when ctx is undefined or missing
context), then call ctx.context.internalAdapter.findUserById and perform the
principalKind check, preventing account linking if ctx is absent or the user is
an install_operator.
- Around line 649-661: The filter passed to
ctx.context.internalAdapter.countTotalUsers uses the additionalField key
"principalKind" but the API expects the actual DB column name; update the first
filter object in the countTotalUsers call inside
packages/server/src/auth/index.tsx to use field: "principal_kind" (leave
operator and value unchanged) so the call to countTotalUsers([{ field:
"principal_kind", operator: "ne", value: "install_operator" }, { field: "id",
operator: "ne", value: "bootstrap-user" }]) matches the expected format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c8d25a6-f7c8-4f8e-a2df-616583769ce3
📒 Files selected for processing (1)
packages/server/src/auth/index.tsx
Comment on lines
+766
to
+773
| const linkedUser = | ||
| await ctx?.context.internalAdapter.findUserById( | ||
| account.userId, | ||
| ); | ||
| const principalKind = ( | ||
| linkedUser as { principalKind?: string } | null | ||
| )?.principalKind; | ||
| if (principalKind === "install_operator") { |
There was a problem hiding this comment.
Inconsistent fail-closed behavior compared to signup hook.
The user.create.before hook (line 635) throws an APIError when ctx is missing, enforcing fail-closed semantics. Here, optional chaining silently proceeds if ctx is undefined, allowing account linking to succeed even when the principalKind check couldn't be performed.
If ctx is unexpectedly missing during an OAuth account link onto an install_operator user, the guard is bypassed. Consider aligning with the signup hook's fail-closed approach:
Proposed fix for fail-closed consistency
if (account.providerId !== "credential") {
+ if (!ctx) {
+ throw new APIError("INTERNAL_SERVER_ERROR", {
+ code: "ACCOUNT_LINK_NO_AUTH_CONTEXT",
+ message:
+ "Account linking rejected: missing auth context for install_operator guard.",
+ });
+ }
const linkedUser =
- await ctx?.context.internalAdapter.findUserById(
+ await ctx.context.internalAdapter.findUserById(
account.userId,
);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/server/src/auth/index.tsx` around lines 766 - 773, The principalKind
guard for OAuth account linking uses optional chaining on ctx which silently
allows linking when ctx is missing; update the account-linking flow (the block
using ctx?.context.internalAdapter.findUserById, linkedUser, and the
principalKind === "install_operator" check) to fail closed like the
user.create.before hook: explicitly require ctx (throw the same APIError when
ctx is undefined or missing context), then call
ctx.context.internalAdapter.findUserById and perform the principalKind check,
preventing account linking if ctx is absent or the user is an install_operator.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…dant fail-closed branch Trims the diff added by the previous commit: - Folds the two adjacent comment blocks in user.create.before into one paragraph, keeps the "why ctx.internalAdapter" explanation. - Drops the explicit `if (!ctx) throw …` — uses ctx! instead. A null ctx would throw a TypeError at the property access, which BA catches and surfaces as FAILED_TO_CREATE_USER (422); fail-closed for free, fewer lines. - Matches the same pattern in account.create.before. - Compacts the principalKind additionalField comment. Reproducer still green (sign-up #1 200, sign-up #2 403 with SIGN_UP_DISABLED_IN_SINGLE_USER_MODE).
deadlock Backend-agnostic integration test that runs unchanged against external Postgres (default) and PGlite (LOBU_TEST_BACKEND=pglite). Asserts: - first human signup is admitted and the row is sign-in-ready (principal_kind defaults to 'human' via the DB default, credential hash verifies against the submitted password); - the second signup is refused with SIGN_UP_DISABLED_IN_SINGLE_USER_MODE; - seeded install_operator and bootstrap-user rows don't count as the existing human. Under the PGlite backend this reproduces #947: reverting the hook to a fresh getDb() query hangs the request and the test fails on timeout (verified — all three time out at 15s with the old code).
…ministic in the full suite createAuth() memoizes betterAuth instances in a per-org TtlCache. Under the integration suite's shared module graph (isolate:false), an earlier test file builds the "__system__" instance while LOBU_SINGLE_USER is unset, so its user.create.before closure has the guard disabled. My test then reused that stale instance and the second signup was admitted — order-dependent flake (passed alone, failed late in the suite). - Add clearAuthCacheForTests() to auth/index.tsx (mirrors the existing clearXForTests helpers); production never needs it since env is stable per-process. - Clear the cache in beforeEach (protect against upstream pollution) and afterEach (don't leak our LOBU_SINGLE_USER=1 instance to later files — this was the likely cause of the member-privacy flake in CI too). - Make the "refuses" case seed a committed human via SQL instead of chaining two signups, removing the cross-request visibility dependency. Verified against the full integration suite locally on real Postgres (CI's singleFork/isolate:false config): the 3 single-user tests pass; the only remaining local failures are the isolated-vm sandbox tests, which are environment-specific and pass in CI.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/server/src/auth/index.tsx (1)
646-653:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the correct database column name in the
countTotalUsersfilter.The
fieldparameter inwhereclauses must use the actual database column name ("principal_kind"), not the additionalFields key ("principalKind"). Better Auth does not auto-map additionalFields keys in where clauses. Line 648 must be updated:Fix required at line 648
field: "principal_kind" // not "principalKind"Without this change, the filter fails silently and does not exclude
install_operatorrows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server/src/auth/index.tsx` around lines 646 - 653, The where filter passed to ctx!.context.internalAdapter.countTotalUsers uses the wrong field name; update the filter object in the countTotalUsers call so the field key for the principal kind uses the database column name "principal_kind" instead of "principalKind" (leave the other condition for id/"bootstrap-user" intact) so the install_operator rows are correctly excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/server/src/auth/index.tsx`:
- Around line 646-653: The where filter passed to
ctx!.context.internalAdapter.countTotalUsers uses the wrong field name; update
the filter object in the countTotalUsers call so the field key for the principal
kind uses the database column name "principal_kind" instead of "principalKind"
(leave the other condition for id/"bootstrap-user" intact) so the
install_operator rows are correctly excluded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c4e5635-b173-4f4a-860e-961b61c6d1f8
📒 Files selected for processing (2)
packages/server/src/__tests__/integration/auth/single-user-signup.test.tspackages/server/src/auth/index.tsx
This was referenced May 20, 2026
4 tasks
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
2 participants
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
user.create.beforeandaccount.create.beforecalledgetDb()for a fresh pool connection while Better Auth's sign-up handler (and OAuth-user registration) already held the only one viarunWithTransaction. In PGlite mode the pool is sized 1 (LOBU_DISABLE_PREPARE=1→poolMax=1), so the hook deadlocked the request — no log line, no response, curl headers-timeout.ctx.context.internalAdapter(countTotalUsers/findUserById), which reuses the in-flight transaction connection.principalKindas a Better Auth additional field (input/returned: false,fieldName: 'principal_kind') so the where-clause field name resolves without BA throwingField principal_kind not found in model user. The column already hasNOT NULL DEFAULT 'human'in the migration, soinput: falselets the DB default fill in on signup.Closes #947.
Codex consult
Initial investigation + recommendation from
codex exectraced the deadlock to the postgres.js + Kyselytransaction: truepath holding the reserved connection while the hook asked for a second. Diff-review pass flagged two follow-ups, both applied (fail-closed viactx!; corrected theaccount.create.beforecomment — only new OAuth user registration is transaction-wrapped).Reproducer (red → green)
Manual run against the pre-built
start-local.bundle.mjs(PGlite, ephemeral data dir, defaultLOBU_SINGLE_USER=1):RED (before fix):
POST /api/auth/sign-up/emailtimed out after 12s with 0 bytes; server log had zero entries for the request.GREEN (with fix):
SIGN_UP_DISABLED_IN_SINGLE_USER_MODEINVALID_EMAIL_OR_PASSWORDIntegration test (both backends, same code path)
packages/server/src/__tests__/integration/auth/single-user-signup.test.ts— backend-agnostic, runs unchanged against external Postgres and PGlite. Asserts first-signup admitted + sign-in-ready, second-signup refused, and install_operator/bootstrap-user rows excluded from the human count.LOBU_TEST_BACKEND=pglite): 3/3 pass.auth/integration folder 17/17 pass.getDb()query makes all three time out at 15s under PGlite — the test catches PGlite mode: /api/auth/sign-up/email hangs (headers timeout) #947, it doesn't just pass vacuously.Test plan
bun run typecheckcleanmake build-packagescleanaccount.create.beforenot directly E2E'd — samectx!.context.internalAdapterpattern as the verified count path; needs a configured OAuth provider to exercise the new-OAuth-user transactional pathSummary by CodeRabbit
Bug Fixes
Tests