Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-auth): bind the dev-admin seed's operator-provisioning ticket to more than the seed address (#14373) - #14730
Conversation
…ket to more than the seed address (#14373) `stageOperatorProvisioning`/`isOperatorProvisioning` peeked the ticket by email alone. The address is not a secret (documented default, printed on the boot banner), so a stranger's own concurrent sign-up for the same address, arriving while the ticket was staged, could satisfy an email-only peek at both admission seams and be admitted as the operator class — and since a unique-email constraint lets only one concurrent signUpEmail land, the stranger's row could become the account at that address. `stageOperatorProvisioning` now also generates a random ticket value and returns it; the caller threads it into the same signUpEmail call's body under the new `AuthManager.OPERATOR_PROVISIONING_TICKET_FIELD` key. `isOperatorProvisioning` now requires an exact match on both email and ticket — a missing or mismatched ticket reads as "not provisioning", no email-only fallback. Disposition 3 only, per triage's ruling (14373#issuecomment-5504337158): disposition 1 (document the trust assumption) is already landed and untouched; disposition 2 (move the method off the barrel-public surface) is a published-surface removal, the human floor, not touched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…erator-ticket-binding
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 11 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 2dc527df332cfdee110aa7ce0d5505506e2ea0ca && git checkout 2dc527df332cfdee110aa7ce0d5505506e2ea0ca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4d0d9445a8ed0240e7ca6a393bbe7f4c637e6bd6 4a07e99c455a23037cb81900bc7fbeacebd214d8 && git checkout -B drift-repro 4d0d9445a8ed0240e7ca6a393bbe7f4c637e6bd6 && git merge --no-ff 4a07e99c455a23037cb81900bc7fbeacebd214d8
node scripts/docs-audit/affected-docs.mjs --json 4d0d9445a8ed0240e7ca6a393bbe7f4c637e6bd6
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#14373
What changed
Disposition 3 only, per triage's ruling (14373#issuecomment-5504337158) and the claim comment's carry-forward: bind the dev-admin seed's operator-provisioning ticket to more than the seed address.
AuthManager.stageOperatorProvisioning(email)staged its ticket keyed onemail.trim().toLowerCase()alone. The address is not a secret —admin@objectos.aiis the documented default and the boot banner prints it (with the password) once the seed completes — so "the address is the operator's own" did not narrow the attacker set the way it would for an unguessable value. A stranger's own concurrentPOST /sign-up/emailfor that same address, arriving while the ticket was staged, would satisfy an email-only peek at BOTH admission seams (thedisableSignUpbefore-hook atauth-manager.ts:~2069andvalidateAudienceAdmission'screationClasscomputation at~3940) and be admitted as theoperatorclass too — and since a unique-email constraint lets only one of the two concurrentsignUpEmailcalls actually land, a stranger who won that race would not merely read as the operator, their row would become the account at that address. The safety this rested on — "milliseconds, and dev-only" — was true today, but both are properties of the caller, not of what the ticket asserted.stageOperatorProvisioning(email)now also generates a random, unguessable ticket value (128 bits from WebCrypto'sgetRandomValues, matching the existingresolvePasswordHasherWebContainer-salt convention) and returns it.AuthPlugin.maybeSeedDevAdminthreads that value into the SAMEsignUpEmailcall's body, under a newAuthManager.OPERATOR_PROVISIONING_TICKET_FIELDkey — an unrecognized key that better-auth'ssignUpEmailBodySchema(z.object({...}).and(z.record(z.string(), z.any()))) lets ride through untouched, so it never becomes a declaredsys_userfield or a DB column.isOperatorProvisioning(email, ticket)now requires an exact match on both; a missing, wrong-typed, or mismatched ticket reads as "not provisioning" — same as no ticket at all, no email-only fallback.This converts a timing argument into a structural one: admission now asks "did THIS process's own boot command make THIS exact call", not "does the address match". A stranger's request carries no value that was ever transmitted anywhere for them to replay, however precisely they time the window.
Why the nonce, not
nameTriage offered two implementation routes: bind to the seed's own
name, or thread a nonce through the sign-up body. I measured both against the actual threat (a concurrent stranger, not merely "harder to guess"):namedefaults to'Dev Admin'(OS_SEED_ADMIN_NAMEenv override), and — unlikeemail/password— is not printed by the boot banner (AuthManager.devSeedResultonly carries{email, password}; confirmed by readingserve.ts's banner code andformat.ts). So it is not observable from a live deployment's own output. But this is an open-source codebase:'Dev Admin'is exactly as discoverable by readingauth-plugin.tsasadmin@objectos.aiis by readingwalled-owner-verification-path.ts. Binding tonamealone would narrow the window against an attacker who knows the printed email but hasn't read the source — a real but narrow-obscurity improvement, not a structural one. Per the card's own instruction, I did not write "harder to guess" as "impossible."Is the binding value a secret? (asked directly)
Yes, and that is the load-bearing difference from the seed address. Traced end to end, what it travels with and where it can surface:
AuthManager.generateOperatorProvisioningTicket()— 16 bytes fromcrypto.getRandomValues, hex-encoded, held only in the localticketvariable and in thependingOperatorProvisioningin-memoryMap(keyed by email, TTL-pruned, cleared in afinally). Nothing durable — no table, no file, no env var.AuthManager.OPERATOR_PROVISIONING_TICKET_FIELD(__osOperatorProvisioningTicket), riding inside the samePOST /sign-up/emailrequest bodyAuthPlugin.maybeSeedDevAdminwas always going to send — alongsideemail,password, andname. It does not get its own call or its own channel.signUpEmailBodySchemaisz.object({...}).and(z.record(z.string(), z.any()))— the declared fields get validated and become thesys_user/sys_accountrows; the catch-all lets the ticket field ride through the parse untouched, but it is read ONLY byisOperatorProvisioning's comparison and is never assigned to any column, never passed tointernalAdapter.createUser.AuthManager.devSeedResult— the only thing the boot banner reads (seeserve.ts's banner code,format.ts) — carries exactly{email, password}. The ticket is never part of that struct, so it cannot reach stdout, a log line, or any other deployment-observable output by any path this diff touches.So the fix does not make the window narrower (a better guess still fails against the seed address) — it removes the window's public observable entirely: there is nothing about this deployment's own output that a stranger could read to learn the value, at any point in its lifetime. Contrast with binding to
name(above), which would only have been "harder to guess," sincenamedefaults to a fixed, source-readable string.Region split held
Per the claim comment's declared region (origin/main line numbers):
auth-manager.ts:2070/:3810/:3912-3914/:4233/:4238/:4251;auth-plugin.ts:1874.Corrected measurement (
git diff -U0 origin/main...HEAD, exact changed old-side lines, zero context padding — the first PR-body draft measured this with-U3-style context bleed and understated the gap at "22 lines"; this is the precise figure and it supersedes that one):auth-manager.tstouches old-side lines2068,2070/3811,3813,3821/3890-3893,3900,3914/4231,4233,4235,4242,4249,4251,4254;auth-plugin.tstouches1874,1876. Against PR #14600's declared hunks (auth-manager.ts14/990/1336/1368/1399/1425/2105/2921/3067/3089/4327-4337/4340/4354/4554-4591;auth-plugin.ts784-788 only): closest pairs are 2070 vs 2105 = 35 lines and 4254 vs 4327 = 73 lines inauth-manager.ts, and ~1090 lines apart inauth-plugin.ts(784-788 vs 1874-1876). This matches the PM seat's independent re-measurement exactly. Disjoint throughout; split holds.Not in this PR (per triage's ruling)
main; the JSDoc abovestageOperatorProvisioningstates it in full. No-op, not touched.删除已发布能力) with ADR-0049 enforce-or-remove discipline owed. Not a dev-agent edit.#14349's question, in the decision inbox.Public surface (Clause ②: yes) — exact signature deltas, and why
grep exportcannot see themAuthManageris barrel-public (packages/plugins/plugin-auth/src/index.ts:export * from './auth-manager.js'), so every public member of the class is on the package's public surface even though nothing in this diff adds a new top-levelexportstatement —git diff -U0 origin/main... | grep -E '^\+\s*export 'returns zero output, and reading only that grep would wrongly conclude Clause ② isno. The actual surface change is on the SIGNATURES of two already-exported class members, which a line-prefix grep for theexportkeyword structurally cannot see (the keyword lives once, on theclass AuthManagerdeclaration itself, not repeated per member):origin/main)stageOperatorProvisioningstageOperatorProvisioning(email: string): voidstageOperatorProvisioning(email: string): stringisOperatorProvisioningisOperatorProvisioning(email: unknown): booleanisOperatorProvisioning(email: unknown, ticket?: unknown): booleanstatic readonly OPERATOR_PROVISIONING_TICKET_FIELD: stringstageOperatorProvisioning's return type widensvoid→string— additive; any existing caller that ignores the return value is unaffected, but a caller that TYPE-CHECKS againstvoid(unusual, but possible in a strict wrapper) would need to update.isOperatorProvisioninggains an optional second parameter — additive at the call site (omitting it now always reads "not provisioning," the safe default), but it is a signature widening on a public method, which is exactly what Clause ② asks about regardless of backward-compatibility.Graded
minorin the changeset (confirmed:.changeset/operator-provisioning-ticket-binding.mdfrontmatter is"@objectstack/plugin-auth": minor— this repo's convention for a barrel-public signature change, notpatch).needs:contract-reviewis on both carriers: issue #14373 (pre-hung by the claim comment) and this PR (confirmed present via a read-back after adding it).Docs drift — three pages checked, none touched
Per PM's pointer, checked whether this diff falsifies any sentence on the three pages that reach
AuthManageras a symbol:content/docs/kernel/contracts/auth-service.mdx— grepped forAuthManager/stageOperatorProvisioning/isOperatorProvisioning/operator provisioning/dev-admin/seed. One hit: a line contrastingapivsgetApi()namingAuthManagergenerically ("the shippedplugin-authregisters anAuthManager, which has noapimember at all"). Still true — this diff adds noapimember and does not touch that contrast. No change needed.content/docs/kernel/services-checklist.mdx— one hit, a table row: "Implementation:AuthPlugin→AuthManager(@objectstack/plugin-auth, built on better-auth)". Still true —AuthManageris still the implementation, still built on better-auth; this diff changes neither fact. No change needed.content/docs/permissions/authentication.mdx— the page that documents the public auth surface in the most detail (bootstrap-status,sign-up/email, etc.). Read the fullsign-up/emailentry and the "First-run bootstrap status" section (#first-run-bootstrap-status): both describe the PUBLIC bootstrap probe (isBootstrapCreation, "does this environment have no human user yet") and the genericsign-up/emailendpoint description ("Register new user"). Neither mentions the dev-admin seed's internal operator-provisioning ticket mechanism (stageOperatorProvisioning/isOperatorProvisioning) at all — that mechanism is deliberately a SEPARATE, undocumented-by-design internal seam (see the[#14157]docblock: "moves no public door"), distinct from the bootstrap probe this page does document. No sentence on this page asserts anything about ticket keying that this diff changes. No change needed.Verdict for all three: the change is internal to the dev-admin seed's ticket-keying implementation detail: none of the three pages' contracts (service implementation identity, public bootstrap probe semantics, public endpoint list) assert anything this diff makes false.
Tests
Real end-to-end suite (
dev-admin-seed-credential-gate.test.ts, realObjectQLengine + real better-auth over:memory:SQLite). Existing cases ⓪–⑧ stay green unmodified; two new cases pin the fix:maybeSeedDevAdminopens), fires a stranger'sPOST /sign-up/emailfor the SAME address with NO ticket field while the window is open — asserts403 SELF_REGISTRATION_CLOSEDand zero accounts created — then, in the SAME still-open window, fires the correctly-bound call as a positive control and asserts it IS admitted, its credential actually authenticates viasign-in/email.pnpm --filter @objectstack/plugin-auth test: 91 test files passed, 1864 tests passed, on the merged tree at4a07e99c4.pnpm --filter @objectstack/plugin-auth typecheck: clean.Ablation (proves the pin actually depends on the binding)
Reverted
isOperatorProvisioningto email-only matching. Mutation proof, redone with a non-comment marker (aglobalThiswrite, since a//comment is stripped by esbuild and cannot prove the mutation reached the built artifact):66ba6e5c…→6b477b29…; injected marker token present at 2 occurrences (write + implicit read via grep), 0 remaining occurrences of the originalticket === entry.ticketcomparison line.dist/index.mjs; the marker token is present in the BUILT output (DIST_MARKER_COUNT=1) — confirms the mutation reached the code the tests actually load, not just the source.Tests 2 failed | 13 passed (15), with ⑨ and ⑨(b) specifically red (the stranger is admitted once the ticket check is gone).trap ... EXIT INT TERM:git diff HEAD -- auth-manager.tsbyte count0after restore. Rebuilt again afterward: marker count0in the rebuiltdist/index.mjs, and the file's suite is back to15 passed (15)—dist/is confirmed returned to the real implementation, not left mutated.Gate family (re-derived from the actual diff at HEAD
4a07e99c4,scripts/pm/dispatch-gates.mjs)38 commands derived. 35 exit 0. The remaining 3 are explicitly NOT MEASURED (their own output says so, exit code 3, distinct from a finding's exit 1), each requiring a full-monorepo
dist/closure this scoped local run does not build — CI'slint.ymlbuilds that closure and runs these for real:check-test-completeness— no localturbo run testlog to parse.check:dual-build-cjs-loads— PREREQUISITE NOT MET, 38 unrelated packages have no localdist/.check:type-check-debt --re-measure— PREREQUISITE NOT MET, 28 workspace deps have no built type entry point locally.None of the 3 touch
plugin-auth,auth-manager.ts, orauth-plugin.tsin their own diagnostics.🤖 Generated with Claude Code
https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8