fix(plugin-auth): refuse a sign-up for an address that already exists, instead of a 200 for a row never written - #15738
Conversation
… synthetic 200 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…gnup-already-exists-200
📓 Docs Drift CheckThis PR changes 2 package(s): 8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 133 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1b56c70128d5ae561bc438bf210a509ae0a36fd6 && git checkout 1b56c70128d5ae561bc438bf210a509ae0a36fd6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 66e68adc667acaca9035c135f57d9f13c8cde56a 64aba4af5dbe7b490e8d0a1e48e4165cb2c6a95e && git checkout -B drift-repro 66e68adc667acaca9035c135f57d9f13c8cde56a && git merge --no-ff 64aba4af5dbe7b490e8d0a1e48e4165cb2c6a95e
node scripts/docs-audit/affected-docs.mjs --json 66e68adc667acaca9035c135f57d9f13c8cde56a
|
Clause-② contract review — head
|
…make an unanswerable uniqueness probe loud Review round 1 on #15738: - content/docs/deployment/self-hosting.mdx published as MEASURED that a seeded person's registration under a widened posture answers 200 and persists nothing. That sentence is false after this branch: the request is now refused 422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL. Only the mechanism sentence moves; the bullet's conclusion (opening the posture is not enough on its own) is still true and stays. - hasExistingUserFor's catch fell through SILENTLY. The direction is correct and unchanged, but a failure specific to this query's shape re-opened #15587 with no signal, because the vendor's duplicate shield answers a synthetic 200 rather than raising. It now reports through audienceLogError, the facility the sibling probe already uses at its page ceiling. Pinned as case 7. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…gnup-already-exists-200
Review round 1 — both items addressedNew head Required — the false published statement
The bullet's bolded conclusion is unchanged, as instructed — only the mechanism sentence moved. The rewrite also reconciles it with the paragraph ~10 lines above, which already stated the Nothing else on the page was touched, and Also-do — the silent probe
with Pinned, as case ⑦ — because a log line nothing asserts is the next thing to go silent. It reproduces your harness: a throw scoped to this probe's own signature ( Mutated, like the others. Removing only the Case ⑦ alone reds, and the seven that do not depend on the log stay green — so it discriminates on exactly the line it exists to protect. Correction to my own reportYou are right that I understated it: I inferred Re-derived gate family at
|
newly applicable via content/docs/deployment/self-hosting.mdx |
exit |
|---|---|
check:corpus-claim-drift |
0 |
check:docs-transcript-drift |
0 |
check:doc-anchors |
0 |
check:docs-audit-scope |
0 |
check:docs-image-tag |
0 |
check:docs-redirects |
0 |
check:docs-single-h1 |
0 |
check:published-readme-links |
0 |
check:doc-security-posture (@objectstack/lint) |
0 |
Re-run at the same head:
| check | exit |
|---|---|
@objectstack/plugin-auth vitest (whole package) |
0 — 95 files / 1994 tests (case ⑦ is the new one) |
typecheck (plugin-auth + spec) |
0 — test-typecheck debt unchanged at 10 files / 94 errors |
pnpm lint (eslint . --no-inline-config) |
0 — 5981 files, 0 errors, 0 warnings |
check:generated (spec) |
0 — all 15 artifacts current after the second merge |
check-adr-0087-registration --base origin/main --head 64aba4af5 |
0 (--self-test control: 0) |
check:type-check-debt (--re-measure), check:dual-build-cjs-loads |
0 — after a full workspace build; on an unbuilt tree the latter exits 3 PREREQUISITE NOT MET, which is not a pass |
check:nul-bytes, check:error-code-casing, check:dispatcher-error-vocabulary, check:cross-package-test-inputs, check:engine-double-contract, check:where-matcher, check:objectql-double-limit, check:query-options-erasure, check:type-check-coverage, check:auth-mount-ledger, check:error-status-conformance, check:test-source-alias, check:merge-driver, check:logger-receiver-detach, check:partof-closing-keyword, check:objectui-changeset, check:pm-half-states, check:doc-authoring |
0 |
Exit codes captured by redirect, never through a pipe.
Not acted on, per your instruction
The two open questions are the maintainer's — no changeset severity change, and the enumeration follow-up is yours to file, so I have not filed it and have not widened this PR for it.
Generated by Claude Code
Clause-② contract review, round 2 — head
|
| clause | status |
|---|---|
seeded person's registration on email_domain → 422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL, nothing written |
driven at this head: sign-up 422 with that code; case ① green (no insert attempted) |
"the same answer the invite_only default already gives for an address the directory holds" |
true as the page reads it — the recovery flow is the invited case (the table above hand-inserts a sys_invitation), and an invited existing address on invite_only is 422 (case ⓪ shield-off leg, ⑥). See the note below on the paragraph above it. |
"so … their sign-in stays 401" |
driven: sign-in after the 422 → 401 INVALID_EMAIL_OR_PASSWORD; case ② green |
| "A fresh address does get an account" | driven: dave@corp.example → 200 (token:null, verification pending), sys_user row, 1 sys_account row; case ③ green |
"every posture other than invite_only forces email verification on" |
exact: AUDIENCE_POSTURES = ['invite_only','email_domain','open'], audiencePermitsSelfRegistration = email_domain || open (auth-config.zod.ts:370-372), mapped to requireEmailVerification: true at auth-manager.ts:5912 |
"its first sign-in is refused 403 EMAIL_NOT_VERIFIED until a mail transport delivers the link" |
driven: fresh address's first sign-in → 403 EMAIL_NOT_VERIFIED |
What would falsify it: any admitted sign-up for an existing address answering other than 422. The one known way is an unanswerable probe — case ⑦ shows that lane falls back to the vendor's synthetic 200 — which is exactly why the log line and its pin exist; the paragraph is true whenever the probe can be answered. (A folding-collation accent-adjacent address is the other candidate — NOT MEASURED, no MySQL here.) Adding a fourth posture without touching audiencePermitsSelfRegistration would falsify the "every posture" clause; that is the predicate's job, not this page's.
Against the paragraph ten lines above: they now agree on the 422 for an address the directory holds — the contradiction is gone. One pre-existing imprecision, noted only, not this PR's sentence: that paragraph says "invitation or not", but on invite_only an uninvited existing address answers 403 SELF_REGISTRATION_CLOSED, not 422 (round-1 E6 / case ④); "invitation or not" is literally true only on the widened postures. Its point — an invitation does not get an existing address in — stands. Two-word docs nit for whenever that page is next edited.
3. Case ⑦ and the mutations — reproduced; one stale number in the body
- Baseline at this head: 8 passed (8).
- Log-only mutation: removed only the
audienceLogError(...)call (7 lines) — log-string occurrences 1→0, call site ofhasExistingUserForunchanged at 1, blobc3b78c6→2354364. Suite: 1 failed | 7 passed (8) — ⑦ alone red, at the "exactly one line named the probe" assertion (:373). Restore:git diff HEAD0 lines, hash equal to the HEAD blob. So the pin discriminates on precisely the line it protects, and the direction assertion (< 300, nothing written) is what keeps a future "make it refuse" change from passing silently. - Whole-block ablation at this head: call site 1→0, blob
c3b78c6→81a3d3f. Suite: 6 failed | 2 passed (8) — ⓪ ① ② ⑤ ⑥ red as before, plus ⑦ red because the ablated block is the probe's only caller, so nothing logs; the two controls ③ ④ stay green. Restore: 0 diff lines, hash equal. ⚠️ PR body, non-blocking: the verification section still prints the round-1 figure5 failed | 2 passed (7). At the shipped 8-case head the whole-block ablation is6 failed | 2 passed (8)with ⑦ among the red for the reason above. The1 failed | 7 passed (8)line for the log-only mutation is correct. One-line body fix, no code change.
4. PR body — says what was measured
"Two zeros are reported as NOT MEASURED … check:dispatcher-error-vocabulary and check:error-code-provenance both pass, and a control proves neither can see this stamp: with the ledger entry deleted from disk both still exit 0" — that is the round-1 control I ran, now stated as measured for both. The docs-correction section is present and accurate. check:dual-build-cjs-loads is listed as 0 only after a full build, with the unbuilt exit 3 explicitly "not a pass" — not counted.
5. Docs gate family — spot-checked
Driven at this head: check:docs-transcript-drift 0, check:corpus-claim-drift 0, check:doc-anchors 0, check:docs-single-h1 0. The other five in the family (audit-scope, image-tag, redirects, published-readme-links, doc-security-posture) NOT MEASURED — the dev's table.
Not measured
- Any store other than better-sqlite3
:memory:. check:generatedas a whole, lint, typecheck, the full plugin-auth suite, ADR-0087 (PM verified).- The five docs gates named above.
Generated by Claude Code
PM note on landing — one stale figure in this PR's verification recordPosted by the The whole-block ablation figure in the body is round-1's. It still prints ⭐ The extra red is not a control going red — it is case ⑦, the new pin on the probe's log line, and it goes red for a structural reason: the ablated block is the only caller of The other figure in that section is correct as printed: the log-only mutation (removing just the Also recorded — a pre-existing docs imprecision, ⛔ NOT this PR's sentence and not fixed here
That paragraph's point stands — an invitation does not get an existing address in — so this is a wording nit for whoever next edits that page, ⛔ not a defect and ⛔ not in scope here. Noted so the next editor inherits the measurement rather than rediscovering it. Both items come from the round-2 review (comment 5550058129); the verdict there is PASS. Generated by Claude Code |
Fixes #15587
Under a self-registration-permitting audience posture,
POST /sign-up/emailfor an address that already carried asys_userrow answered 200 with a freshly minted user id and persisted nothing — no new row, nosys_account, and the next sign-in a 401 with nothing anywhere explaining it. It now answers 422USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL, the same refusal theinvite_onlydefault already shipped.Deliverable 1: which mechanism it actually is
The card deliberately left this open — "whether the response is synthesized on the forced-email-verification lane before the uniqueness refusal, or whether an insert is attempted and swallowed" — and the two have different fixes, so it was established before anything was written.
It is the first: synthesized ahead of the uniqueness refusal. No insert is attempted, and nothing is swallowed.
Three independent pieces of evidence, none of them the symptom:
dist/api/routes/sign-up.mjscomputesshouldReturnGenericDuplicateResponse = requireEmailVerification || autoSignIn === false(line 163). On a duplicate,findUserByEmailhits at line 199 and returnsbuildGenericDuplicateResponse()— a user object built in memory fromgenerateId()— instead of throwing at line 212.createUseris never reached. (The shield's other arm, the 403 catch at line 235, is the oneauth-manager.tsalready steps around for the audience refusal.)insertreaching the engine across the request was counted: the list is empty. A swallowed insert would showsys_userthere. This is pinned as case ①.invite_onlydefault with a pending invitation, so the audience gate admits identically and the only moving part isrequireEmailVerification. Shield off: 422. Shield on: the synthetic 200. So the posture is not the cause — it is only what arms the shield, because a posture permitting self-registration forces verification on increateAuthInstance. Pinned as case ⓪.A consequence worth having in the card's record: the defect was never confined to the widened postures.
emailAndPassword.autoSignIn: falsearms the same shield under any posture, includinginvite_only. Pinned as case ⑥.The fix, and why it is minimal
The uniqueness refusal is raised on the
/sign-up/emailbefore-hook — the same seam, for the same reason, as the audience-posture refusal already raised there, with the block sitting directly beneath it. It is built from better-auth's ownBASE_ERROR_CODESentry, so both lanes are byte-identical by construction rather than by copying a string (case ⑤ is the drift detector: a vendor re-wording parts the lanes and reds).Two decisions that carry the design:
invite_onlydefault (422 for a real address versus 403 for an unknown one) — inventing on the closed door exactly what the vendor's shield exists to prevent. Case ④ pins that the default posture is untouched.sys_userrow is refused" is one fact under every posture and every verification setting, rather than a contract that is a function of a vendor internal and that a widened shield would silently reopen. Nothing is lost where the shield is off: the vendor'sonExistingUserSignUphook is not wired anywhere in this repo, and its timing-equalizing hash equalizes against an oracle this 422 states outright.The probe (
hasExistingUserFor) fails toward the vendor: an unanswerable read returns false and the request falls through to better-auth's ownfindUserByEmail. It can only ever narrow a synthetic 200 into the honest 422; it can never admit a creation the vendor would have refused. That is the opposite of its neighbourhasPendingInvitationFor, which must fail closed because it grants a carve-out — the two are annotated as deliberate opposites. It is not silent about it either: an unanswerable probe reports throughaudienceLogErrorbefore falling through, because a failure specific to this query's shape would otherwise re-open #15587 with no signal anywhere (review round 1; pinned as case ⑦, which drives a throw scoped to the probe's own signature).What a reviewer should weigh
This is a published wire-behaviour change, and it discloses something the synthetic 200 hid. On
openandemail_domain, a caller the audience gate admits can now distinguish an address that has an account from one that does not. That is the disclosure theinvite_onlylane has always made to an invitation holder, and the card's acceptance chose it deliberately — a false receipt on the recovery path was judged worse.invite_onlyitself gains no oracle (case ④). Flagging it because it is the one thing here that is a trade rather than a repair.It does not make #15588's remedy (2) work — it makes its failure honest. That remedy tells a locked-out operator to open the posture "so an existing person can register their own login". Before: a silent false 200. After: an explicit 422. The person still cannot register a login, because per the 2026-09-02 ruling recorded on closed #14349 that door stays shut by design and recovery is out of band. Case ② pins the whole sequence (sign-up 422, then sign-in 401). Raised here so the two cards can be reconciled; no line of
boot-sign-in-reachability.tsis touched by this PR — that text is #15588's surface.A published statement this PR had to correct
content/docs/deployment/self-hosting.mdxpublished as a measured fact that the widened-posture registration "answers200and persists nothing". This branch makes that false, so the mechanism sentence is rewritten to the422— the bullet's conclusion ("opening the posture is not enough on its own") is unchanged, because it is still true. That also reconciles it with the paragraph ten lines above, which already stated the422for an address the directory holds.content/docs/releases/is untouched.Verification
Head
64aba4af5(merged withorigin/maintwice viascripts/pm/os-regen-merge.sh; the gate union below was run on that commit).Every new pin was mutated, including case ⑦. The fix was ablated on disk (mutation confirmed by counting the call site off the file and by a distinct
git hash-object; restore confirmed by an emptygit diff HEADand a hash equal to the HEAD blob). Result — exactly the predicted split:The two survivors are the controls that assert unchanged behaviour, so they must stay green under ablation, and do. Removing only the new log call reds case ⑦ alone (
1 failed | 7 passed). The pin runs on a realObjectQLover@objectstack/driver-sqland better-sqlite3:memory:with plugin-auth's ownauthIdentityObjects, driven throughAuthManager.handleRequest— the card's harness, because the population predicate and the uniqueness check both live below the fake doubles.@objectstack/plugin-authvitest (whole package)typecheck(plugin-auth + spec)pnpm lint(eslint . --no-inline-config)check:generated(spec, all 15 artifacts)check-adr-0087-registration --base origin/main --head 64aba4af5--self-testcontrol: 0, 304 assertions)content/docs/deployment/—check:corpus-claim-drift,check:docs-transcript-drift,check:doc-anchors,check:docs-audit-scope,check:docs-image-tag,check:docs-redirects,check:docs-single-h1,check:published-readme-links,check:doc-security-posturecheck:dispatcher-error-vocabulary,check:error-code-casing,check:error-code-provenancecheck:type-check-debt(--re-measure),check:type-check-coveragecheck:dual-build-cjs-loadsPREREQUISITE NOT METon an unbuilt tree, which is not a pass)check:nul-bytes,check:cross-package-test-inputs,check:engine-double-contract,check:where-matcher,check:objectql-double-limit,check:query-options-erasure,check:auth-mount-ledger,check:error-status-conformance,check:test-source-alias,check:merge-driver,check:logger-receiver-detach,check:doc-authoringExit codes were captured by redirect, never through a pipe. The gate family was re-derived from the actual changed files with
scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackat the new head, which is what surfaced the docs family above.Two zeros are reported as NOT MEASURED rather than as coverage.
check:dispatcher-error-vocabularyandcheck:error-code-provenanceboth pass, and a control proves neither can see this stamp: with the ledger entry deleted from disk both still exit 0. The code is stamped throughAPIError.from(status, BASE_ERROR_CODES.MEMBER)— an argument position holding a member expression on an imported vendor object — outside the first gate's population and outside the second's printed bounds. The registration in this PR is correct under ADR-0112; nothing enforces it. Filed as #15723.Also in this diff
USER_ALREADY_EXISTS_USE_ANOTHER_EMAILis registered in the ADR-0112 ledger under@objectstack/plugin-auth, because the platform now emits it rather than only passing it through, and an emitted-but-unregistered code is the silent fourth state that ledger exists to prevent. The two regeneratedcontent/docs/references/api/files are that entry's generated consequence — one code added, nothing else moved.🤖 Generated with Claude Code
https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y