Repository navigation
fix(plugin-auth): SSO domain verification accepts the TXT record request-domain-verification names - #25161
Conversation
…h so SSO verify-domain accepts the instructed TXT record @better-auth/sso's /sso/verify-domain builds the record it accepts as IDENTIFIER=VALUE from the verification row the storage adapter returns. sys_verification.identifier is internal, so the row reached the vendor without it and the instructed record _better-auth-token-PROVIDER=TOKEN answered 502 DOMAIN_VERIFICATION_FAILED while undefined=TOKEN verified. The read-back census now re-attaches identifier beside value; a read keyed on identifier answers it from the lookup. Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 18 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 a764d438a2feef563749640bf478ad7ce53ee48d && git checkout a764d438a2feef563749640bf478ad7ce53ee48d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6b44efa7b4a429994dc6d92c3de96392d4e03f3f 7316682282de9ecdd7b57175b107d64edb48641d && git checkout -B drift-repro 6b44efa7b4a429994dc6d92c3de96392d4e03f3f && git merge --no-ff 7316682282de9ecdd7b57175b107d64edb48641d
node scripts/docs-audit/affected-docs.mjs --json 6b44efa7b4a429994dc6d92c3de96392d4e03f3f |
Contract reviewServed-tier: Inputs: card #25111 (body and both comments: the dispatch claim and the os-dev-report), PR #25161 (body, file list, and the net diff against Check-runs on the head, read at 2026-10-22T03:16Z: 40 runs; 28 completed ① Derived judgmentsThe diff changes one thing at runtime: (a) What the seam feeds, and whether any caller outside the vendor now reads wider. Right: it widens only what the vendor's storage adapter sees. Inside the vendor, the hunk's "only reader" claim was re-traced on the four 1.7.12 dists rather than taken from the comment. Every (b) Is the newly accepted record exactly the one the platform instructs? Right. (c) Is the newly refused (d) Does the verification still prove control of the DNS name for that provider? Right. The vendor never reads the DNS name from the row: it queries Full accept-set delta of the verify door, vendor route and bridge alike: Consistency of the new seam row. ② Semver level
Changeset level: ③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
席位核对
Landing: ordinary queue landing. The seat sets ready and auto-merge through the relay in this act. After the merge, #25111 closes by Changeset sentences checked against the diff:
All hold. Generated by Claude Code |
Fixes #25111
Clause-②: no — the verify door now accepts the record the platform already tells the operator to publish, as
@better-auth/ssodesigned it. The one record it stops accepting isundefined=TOKEN, which only the defect produced and nothing instructs (table below).Metadata-protocol: none
What was wrong
POST /api/v1/auth/admin/sso/request-domain-verificationtells the operator to publish a TXT record named_better-auth-token-PROVIDER.DOMAINwith the value_better-auth-token-PROVIDER=TOKEN(register-sso-provider.ts,dnsRecordValue). The vendor route behind the bridge,POST /api/v1/auth/sso/verify-domain(@better-auth/sso1.7.12,verifyDomain), accepts a TXT record at that name whose trimmed value equals eitheractiveVerification.identifier + '=' + activeVerification.valueor the bareactiveVerification.value.activeVerificationis the rowinternalAdapter.findVerificationValue(identifier)reads back: afindManyonverificationkeyed onidentifier, newest first, limit 1.sys_verification.identifieris declaredinternal, so the engine strips it from that read, andinternal-field-readback.tsre-attached onlyvalue. The vendor therefore compared the published record againstundefined=TOKEN.Measured (the dispatch's four mechanism assumptions)
Base
9cfc57533, better-auth /@better-auth/sso1.7.12. RealObjectQLover@objectstack/driver-sql(better-sqlite3) with this plugin's manifest, so the strip is live. RealAuthManagerwith the realsso({ domainVerification: { enabled: true } }), and the plugin's real route registration on a Hono app for the bridge. DNS is a double onnode:dns/promises.resolveTxt. This is the new suite run against the unfixed source:_better-auth-token-PROVIDER.DOMAIN/sso/verify-domain/admin/sso/verify-domain_better-auth-token-PROVIDER=TOKEN(the instructed record)DOMAIN_VERIFICATION_FAILEDDOMAIN_VERIFICATION_FAILEDundefined=TOKENverified: trueTOKENidentifier. Theundefined=TOKENrecord verified, and only an undefined identifier can produce that string.identifierback was false. I re-traced it on 1.7.12: across better-auth,@better-auth/sso,/oauth-providerand/scim,verifyDomainis the only code that readsverification.identifieroff a row. The other.identifierhits are the oauth-provider resource model, a doc comment and test-utils.VALUEbesideIDENTIFIER=VALUEby design (normalizedRecord === verificationRecord || normalizedRecord === verificationValue), andvaluewas always re-attached.undefined=TOKEN.The remedy, under the four axes
Two candidates were measured against the vendor code:
sys_verification.identifierto the rows the adapter hands better-auth.dnsRecordValueto the bare token.What the verification proves under each. The vendor queries DNS only at
identifier + '.' + domain. Thatidentifieris computed from the provider id (getVerificationIdentifier); it is never read from the row. The token is a fresh random value stored under that provider's identifier. So under both A and B the provider binding is the queried name plus the per-provider token, and the vendor already accepts the bare token. The dispatch's premise that the bare-token instruction "would make the published record prove less" does not hold against the vendor code: the label inside the value repeats what the queried name already binds. Neither remedy weakens the proof, so this is not theneeds_decisioncase.Chosen: A.
content/docs/permissions/sso.mdxrepeats ("publishdnsRecordName/dnsRecordValue"). A makes that record verify. B would change the shown record and leave the vendor'sIDENTIFIER=VALUEbranch dead on this store, withundefined=TOKENstill verifying.dnsRecordValue) that operators and the docs read.What A hands to whom. Every verification read better-auth issues is keyed on
identifier(findVerificationValue,consumeVerificationValue), so A hands the vendor a value it passed in itself. AfindOnekeyed on it is answered from the lookup with no privileged read. No vendor route returns a verification row to its caller: I checked everyfindVerificationValue/consumeVerificationValueresult variable in those four packages againstctx.json(and found none passed whole or spread. The generic data API still strips the column, and the suite's precondition case pins that.What the verify door accepts, before and after
_better-auth-token-PROVIDER=TOKEN(instructed)TOKENundefined=TOKENThe
Clause-②: noline above follows the claim. The narrowing is onlyundefined=TOKEN, a string that exists only because of the defect.Change
packages/plugins/plugin-auth/src/internal-field-readback.ts: thesys_verificationreadback row gainsidentifierwithabsenceProvesStrip: true, because the column isrequired: true. The census comment now names the one reader, traced on 1.7.12, and says why handing the column back discloses nothing. The fail-closed consequence text also names domain verification.packages/plugins/plugin-auth/src/sso-domain-verification-instructed-record.test.ts(new): the pins on the vendor's real handler, both doors. Refusals assertcodeandstatus, the DNS name the vendor queried, and that the provider stays unverified at rest.packages/plugins/plugin-auth/src/internal-field-readback.test.ts: three seam pins. Both columns come back on a stripped findMany row, one batched read per column. A read keyed onidentifieranswers it from the lookup. The seam fails closed with no accessor..changeset/25111-sso-domain-verification-instructed-record.md: patch for@objectstack/plugin-auth.register-sso-provider.tsis unchanged, and so is the instruction.Tests
Tests 4 failed | 8 passed (12). The four failures are the instructed record (502 on both doors) andundefined=TOKEN(204 / 200).7f0292dde), the new suite plusinternal-field-readback.test.tsandinternal-credential-readback.test.ts:Test Files 3 passed (3),Tests 46 passed (46).@objectstack/plugin-authsuite and typecheck on the final head731668228(the fix merged withorigin/main23540582f):Test Files 183 passed (183),Tests 3466 passed | 10 skipped (3476).typecheck:tsc --noEmit, the examples program, andcheck:test-typecheck: OK(10 files / 88 errors / 23 pinned signatures held, unchanged). Lock verdictcommand-exit 0. Both new test files are in thetsconfig.test.jsonprogram (--listFilesOnly).Ablation (run once, then restored)
Run on the fix commit
7f0292dde. The ablation usednode scripts/ablation-replace.mjs --deleteon the anchor for the newidentifierreadback row, wrappingvitest runover the new suite andinternal-field-readback.test.ts. The subject is imported fromsrc/by relative path, so no build sits between the mutation and the run.e8e98f4e53e8to4d7810f74bf0.Tests 7 failed | 37 passed (44). The instructed record went back to 502 on both doors,undefined=TOKENwent back to 204 / 200, and the three seam pins failed.e8e98f4e53e8equals HEAD,git diff HEADis empty, andgit status --porcelainis empty.Gates
Local subset, on the final head
731668228.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderives 67 commands for these 4 paths (merge base23540582f). All 67 were run, each exit code captured before any pipe, and all 67 exited 0.--ranreconciliation:67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.pnpm check:dual-build-cjs-loadsdeserves a note. On the pre-merge head7f0292ddeit exited 3 (PREREQUISITE NOT MET) because not every package had adist/. The workspace build thatcheck:type-check-debtruns supplied them, and on the final head it measured green (112 require entry points across 69 packages load).The five artifact-roster gates whose roster sits under a changed directory also exit 0 on the final head:
check-changeset-fixed,check:auth-mount-ledger,check:authz-resolver,check:error-code-casing,check:filter-alias-parity.Every family CI runs is cited from this PR's check-runs, not re-run here.
Acceptance notes
packages/platform-objects/src/identity/sys-verification.object.tsstill saysidentifier"has none [readback row], because no better-auth flow reads it back off a row". That sentence was already false (this card) and is now plainly so. It is the engine lane's file, outside this card's file surface, so it is not edited here. Carrier: none.objectql-adapter.ts'sfindManypasses no lookup to the seam, soidentifiercosts one id-batched privileged read on each verificationfindMany/consumeOne. AfindOnekeyed on it is answered from the lookup. These are one-time flows (OTP, magic link, reset, OAuth state, domain verification), not per-request reads. PassingfindMany'swhereas the lookup would make that read free. It is not done here becauseobjectql-adapter.tsis outside the file surface.internal-credential-readback.test.ts).Generated by Claude Code