Skip to content

Commit af1636c

Browse files
os-warrenclaude
andauthored
fix(plugin-auth): answer a registered ADR-0112 code from the SSO domain-verification routes (#10858)
`request-domain-verification` and `verify-domain` shape their failure as `code: parsed?.code || <our default>`. The `parsed?.code` half is @better-auth/sso's own code passing through; the default half is ours, and both defaults were lowercase (`request_domain_verification_failed`, `verify_domain_failed`) — an ADR-0112 D1 violation that the casing gate could not see, because a literal reached through an `||` chain matched none of its recognizers until #10658 widened them. Both defaults now answer `DOMAIN_VERIFICATION_FAILED`, already registered for `@objectstack/plugin-auth` in the error-code ledger. Reused rather than invented: a new spelling would need a `packages/spec` registration to be a legal `error.code`, and — measured here — an unregistered code in a fallback slot is invisible to both error-code gates, so it would have shipped as the silent fourth state ADR-0112 D3 exists to prevent. The vendor pass-through arm is untouched, and that is pinned in both directions: a handler that stamped our code unconditionally would pass a suite that only pinned "our code appears" while destroying the vendor's diagnosis. Also removes both `KNOWN_LOWERCASE_CODES` entries from `scripts/check-error-code-casing.mjs` — the coordination #10658 asked of whichever side landed second, so the gate ends green with zero exceptions. Its registry self-test now drives a fixture registry (the shrink-only semantics outlive the live list reaching zero) and pins the live list AT zero. Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 81845c6 commit af1636c

5 files changed

Lines changed: 213 additions & 18 deletions

File tree

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
"@objectstack/plugin-auth": minor
3+
---
4+
5+
The two SSO domain-verification admin routes now answer a registered ADR-0112
6+
error code. `POST /admin/sso/request-domain-verification` and
7+
`POST /admin/sso/verify-domain` shape their failure as
8+
`code: parsed?.code || <our default>`, and the default half — the code
9+
ObjectStack itself authors when @better-auth/sso returns none — was lowercase
10+
(#10716, found by #10658):
11+
12+
| route | wrote | writes instead |
13+
| --- | --- | --- |
14+
| `POST /admin/sso/request-domain-verification` | `request_domain_verification_failed` | `DOMAIN_VERIFICATION_FAILED` |
15+
| `POST /admin/sso/verify-domain` | `verify_domain_failed` | `DOMAIN_VERIFICATION_FAILED` |
16+
17+
If you match on either lowercase spelling, match on `DOMAIN_VERIFICATION_FAILED`
18+
instead — the two routes are distinguished by their path, as they already were
19+
for every other failure they can answer.
20+
21+
`DOMAIN_VERIFICATION_FAILED` is reused, not invented: it is already registered
22+
for `@objectstack/plugin-auth` in the error-code ledger, so this PR adds nothing
23+
to `packages/spec` and the emitted vocabulary gets no new member. A new spelling
24+
(`VERIFY_DOMAIN_FAILED`) would have needed a ledger registration to be a legal
25+
`error.code` at all, and — measured while fixing this — an unregistered code in
26+
an `||` fallback slot is currently invisible to BOTH error-code gates, so it
27+
would have shipped as exactly the silent fourth state ADR-0112 D3 exists to
28+
prevent.
29+
30+
**The vendor pass-through arm is unchanged.** `parsed?.code` still reaches the
31+
caller verbatim, so @better-auth/sso's own diagnosis (`NO_PENDING_VERIFICATION`,
32+
`DOMAIN_VERIFICATION_FAILED`) is never overwritten by ours — the half that would
33+
be silently lost by a handler that stamped our code unconditionally, and it is
34+
pinned in both directions by
35+
`packages/plugins/plugin-auth/src/sso-domain-verification-error-codes.test.ts`.
36+
Statuses and messages are untouched on every path.
37+
38+
ADR-0087 disposition, in prose because the marker vocabulary has no slot for
39+
this shape: nothing is registered and nothing needs to be. The declared wire
40+
contract is `error.code ∈ StandardErrorCode ∪ ERROR_CODE_LEDGER`, and neither
41+
lowercase spelling was ever a member of it — they were undeclared values a
42+
blind gate let through, so this brings the implementation onto the published
43+
contract rather than changing that contract. There is no metadata surface for
44+
`objectstack migrate meta` to rewrite: error codes live in responses, not in
45+
stored metadata. The table above is here for anyone who matched the undeclared
46+
spelling anyway, which is why this ships as `minor` rather than `patch`.

‎packages/plugins/plugin-auth/src/register-sso-provider.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,11 @@ export async function runRequestDomainVerification(
408408
if (resp.status === 404 && !parsed?.code) {
409409
return { status: 400, body: { success: false, error: { code: 'DOMAIN_VERIFICATION_DISABLED', message: 'Domain verification is not enabled for this environment (set OS_SSO_DOMAIN_VERIFICATION).' } } };
410410
}
411-
return { status: resp.status, body: { success: false, error: { code: parsed?.code || 'request_domain_verification_failed', message: parsed?.message || 'Failed to request domain verification' } } };
411+
// ADR-0112: `parsed?.code` is better-auth's own code passing through — never
412+
// overwritten. The DEFAULT is ours, so it is the registered SCREAMING ledger
413+
// entry for this feature (`DOMAIN_VERIFICATION_FAILED`, @objectstack/plugin-auth)
414+
// rather than a bespoke lowercase spelling (#10716).
415+
return { status: resp.status, body: { success: false, error: { code: parsed?.code || 'DOMAIN_VERIFICATION_FAILED', message: parsed?.message || 'Failed to request domain verification' } } };
412416
}
413417

414418
const token = str(parsed?.domainVerificationToken);
@@ -462,5 +466,8 @@ export async function runVerifyDomain(
462466
} else if (parsed?.code === 'DOMAIN_VERIFICATION_FAILED') {
463467
message = 'DNS TXT record not found yet. Add the record shown when you requested verification, allow time for DNS to propagate, then retry.';
464468
}
465-
return { status: resp.status, body: { success: false, error: { code: parsed?.code || 'verify_domain_failed', message } } };
469+
// ADR-0112, as above: the vendor's code passes through untouched; our default
470+
// is the registered ledger entry (#10716). The `message` above still carries
471+
// the specific diagnosis for each expected failure mode.
472+
return { status: resp.status, body: { success: false, error: { code: parsed?.code || 'DOMAIN_VERIFICATION_FAILED', message } } };
466473
}
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #10716 — the two SSO domain-verification routes answer an ADR-0112 error code.
5+
*
6+
* Both handlers shape their failure as `code: parsed?.code || <our default>`,
7+
* and the two halves of that expression have DIFFERENT owners:
8+
*
9+
* - `parsed?.code` is @better-auth/sso's own code passing through. It is the
10+
* vendor's diagnosis and must reach the caller unchanged.
11+
* - the default is OURS, so ADR-0112 applies to it: SCREAMING_SNAKE, drawn
12+
* from the registered vocabulary rather than invented at the call site.
13+
*
14+
* Both directions are pinned below, and that pairing is the point. Pinning only
15+
* "our code appears" would go green against a handler that overwrites the
16+
* vendor's code unconditionally — which would swallow exactly the diagnosis the
17+
* caller needs (`NO_PENDING_VERIFICATION` tells them to request the DNS record
18+
* first; a blanket "verification failed" does not). Statuses are pinned in both
19+
* directions too: the inner status passes through untouched.
20+
*
21+
* Rejection cases assert `code` AND `status` per ADR-0112 — a bare "it threw"
22+
* would pass against a handler that refuses everyone.
23+
*/
24+
25+
import { describe, it, expect } from 'vitest';
26+
// The `/api` subpath is where the built package exposes the ledger VALUE — the
27+
// root entry re-exports the schemas only. Reading it through the published
28+
// exports map (i.e. `dist`) is deliberate: this asserts against the surface a
29+
// consumer actually gets, not against a source file this test could reach.
30+
import { ERROR_CODE_LEDGER } from '@objectstack/spec/api';
31+
import { runRequestDomainVerification, runVerifyDomain, type AuthRequestHandler } from './register-sso-provider.js';
32+
33+
/** The default this package authors for both routes when the vendor gives no code. */
34+
const OUR_DEFAULT = 'DOMAIN_VERIFICATION_FAILED';
35+
/** A vendor code with its own meaning — the one an overwrite would destroy. */
36+
const VENDOR_CODE = 'NO_PENDING_VERIFICATION';
37+
38+
const REQUEST_URL = 'https://app.example/api/v1/auth/admin/sso/request-domain-verification';
39+
const VERIFY_URL = 'https://app.example/api/v1/auth/admin/sso/verify-domain';
40+
41+
/** A fake inner @better-auth/sso endpoint that answers one canned response. */
42+
function fakeHandle(status: number, body: unknown): AuthRequestHandler {
43+
return async () =>
44+
new Response(body === undefined ? '' : JSON.stringify(body), {
45+
status,
46+
headers: { 'content-type': 'application/json' },
47+
});
48+
}
49+
50+
const post = (url: string) =>
51+
new Request(url, {
52+
method: 'POST',
53+
headers: { 'content-type': 'application/json' },
54+
body: JSON.stringify({ providerId: 'p1', domain: 'acme.example' }),
55+
});
56+
57+
describe('#10716 SSO domain verification — our default is a registered ADR-0112 code', () => {
58+
it('request-domain-verification: an uncoded vendor failure answers our SCREAMING default, status passed through', async () => {
59+
const res = await runRequestDomainVerification(fakeHandle(502, { message: 'upstream exploded' }), post(REQUEST_URL));
60+
61+
expect(res.body.success).toBe(false);
62+
expect(res.body.error?.code).toBe(OUR_DEFAULT);
63+
expect(res.status).toBe(502);
64+
});
65+
66+
it('verify-domain: an uncoded vendor failure answers our SCREAMING default, status passed through', async () => {
67+
// The 404-without-a-code shape: SSO domain verification is off for this env.
68+
// This is the response the dogfood admin-route probe observes.
69+
const res = await runVerifyDomain(fakeHandle(404, undefined), post(VERIFY_URL));
70+
71+
expect(res.body.success).toBe(false);
72+
expect(res.body.error?.code).toBe(OUR_DEFAULT);
73+
expect(res.status).toBe(404);
74+
});
75+
76+
it('both defaults are SCREAMING_SNAKE and registered for this package — reused, not invented', () => {
77+
expect(OUR_DEFAULT).toMatch(/^[A-Z][A-Z0-9_]*$/);
78+
// The mechanical half of "we reused a ledger entry": if this code were a new
79+
// spelling it would need a `packages/spec` registration, and an unregistered
80+
// code in a fallback slot is invisible to BOTH error-code gates — a silent
81+
// fourth state, which is precisely what ADR-0112 D3 exists to prevent.
82+
expect(ERROR_CODE_LEDGER['@objectstack/plugin-auth']).toContain(OUR_DEFAULT);
83+
});
84+
});
85+
86+
describe('#10716 the vendor pass-through arm is untouched', () => {
87+
it('request-domain-verification: the vendor code and status reach the caller unchanged', async () => {
88+
const res = await runRequestDomainVerification(
89+
fakeHandle(400, { code: VENDOR_CODE, message: 'vendor copy' }),
90+
post(REQUEST_URL),
91+
);
92+
93+
expect(res.body.error?.code).toBe(VENDOR_CODE);
94+
expect(res.body.error?.code).not.toBe(OUR_DEFAULT);
95+
expect(res.status).toBe(400);
96+
expect(res.body.error?.message).toBe('vendor copy');
97+
});
98+
99+
it('verify-domain: the vendor code and status reach the caller unchanged', async () => {
100+
const res = await runVerifyDomain(fakeHandle(400, { code: VENDOR_CODE, message: 'vendor copy' }), post(VERIFY_URL));
101+
102+
expect(res.body.error?.code).toBe(VENDOR_CODE);
103+
expect(res.body.error?.code).not.toBe(OUR_DEFAULT);
104+
expect(res.status).toBe(400);
105+
// This route substitutes friendlier COPY for known vendor codes — the code
106+
// itself still passes through, which is the half that matters on the wire.
107+
expect(res.body.error?.message).toContain('Request Domain Verification');
108+
});
109+
110+
it('request-domain-verification: the disabled branch keeps its own dedicated code', async () => {
111+
// Unchanged by #10716 and pinned so the rename cannot have blurred the two:
112+
// "the feature is off" is a different answer from "verification failed".
113+
const res = await runRequestDomainVerification(fakeHandle(404, undefined), post(REQUEST_URL));
114+
115+
expect(res.body.error?.code).toBe('DOMAIN_VERIFICATION_DISABLED');
116+
expect(res.status).toBe(400);
117+
});
118+
});

‎packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ function expectationsFor(targetUserId: string): Record<string, RouteExpectation>
258258
'POST /api/v1/auth/admin/sso/verify-domain': {
259259
bucket: 'objectstack-gate',
260260
body: { providerId: 'refusal-probe-oidc' },
261-
note: 'admin passes the gate and lands on 404 verify_domain_failed while SSO is off',
261+
note: 'admin passes the gate and lands on 404 DOMAIN_VERIFICATION_FAILED while SSO is off',
262262
},
263263

264264
// ── #9652: ban / unban moved from the vendor to an ObjectStack mount ────

‎scripts/check-error-code-casing.mjs‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -162,27 +162,28 @@ const CODE_POSITION_PATTERNS = [
162162
* an allowlist nobody re-reads. A NEW lowercase code never joins it: the gate
163163
* refuses that, and the remedy for a fresh finding is a registered SCREAMING
164164
* code, never a line here.
165+
*
166+
* [#10716] It is now EMPTY, and that is this list reaching its designed end
167+
* rather than a list that never had a use: both entries it was created for
168+
* (`request_domain_verification_failed`, `verify_domain_failed`) were renamed
169+
* onto the registered ledger code `DOMAIN_VERIFICATION_FAILED` by the services
170+
* lane, and the owning PR deleted them here — the coordination #10658 asked
171+
* for, so the gate ends up green with zero exceptions. Emptiness costs no
172+
* coverage: --self-test drives the shrink-only semantics against a FIXTURE
173+
* registry, and pins the live one AT zero so a future exception cannot be
174+
* added quietly.
165175
*/
166-
const KNOWN_LOWERCASE_CODES = new Map([
167-
[
168-
'packages/plugins/plugin-auth/src/register-sso-provider.ts::request_domain_verification_failed',
169-
'wire-visible; rename owned by #10716 (services lane)',
170-
],
171-
[
172-
'packages/plugins/plugin-auth/src/register-sso-provider.ts::verify_domain_failed',
173-
'wire-visible; rename owned by #10716 (services lane); pinned by name in the dogfood suite',
174-
],
175-
]);
176+
const KNOWN_LOWERCASE_CODES = new Map([]);
176177

177178
/** Split findings into the two the wire already carries and everything else. */
178-
export function partitionKnown(violations) {
179+
export function partitionKnown(violations, registry = KNOWN_LOWERCASE_CODES) {
179180
const known = [];
180181
const fresh = [];
181182
for (const v of violations) {
182-
(KNOWN_LOWERCASE_CODES.has(`${v.file}::${v.literal}`) ? known : fresh).push(v);
183+
(registry.has(`${v.file}::${v.literal}`) ? known : fresh).push(v);
183184
}
184185
const reached = new Set(known.map((v) => `${v.file}::${v.literal}`));
185-
const stale = [...KNOWN_LOWERCASE_CODES.keys()].filter((k) => !reached.has(k)).sort();
186+
const stale = [...registry.keys()].filter((k) => !reached.has(k)).sort();
186187
return { known, fresh, stale };
187188
}
188189

@@ -302,8 +303,19 @@ function selfTest() {
302303
// [#10658] The shrink-only registry, in both directions. The second one is
303304
// the load-bearing half: when the owning card's rename lands, a stale line
304305
// must FAIL rather than sit there as a quiet allowlist entry.
306+
//
307+
// [#10716] These drive a FIXTURE registry rather than the live one, which is
308+
// now empty. The semantics being pinned belong to the MECHANISM (an entry
309+
// that stops matching is stale and fails; a new code is never absorbed), and
310+
// they have to survive the live list reaching zero — otherwise emptying it
311+
// would have silently taken the coverage with it. The live list gets its own
312+
// assertion below.
305313
const SSO = 'packages/plugins/plugin-auth/src/register-sso-provider.ts';
306314
const row = (literal) => ({ file: SSO, line: 1, literal, form: 'fallback' });
315+
const fixtureRegistry = new Map([
316+
[`${SSO}::request_domain_verification_failed`, 'fixture — the shape the live list had before #10716'],
317+
[`${SSO}::verify_domain_failed`, 'fixture — the shape the live list had before #10716'],
318+
]);
307319
const partitionCases = [
308320
[[row('request_domain_verification_failed'), row('verify_domain_failed')], { known: 2, fresh: 0, stale: 0 }, 'both known rows still present'],
309321
[
@@ -315,20 +327,32 @@ function selfTest() {
315327
[[], { known: 0, fresh: 0, stale: 2 }, 'an empty tree makes every entry stale'],
316328
];
317329
for (const [input, want, label] of partitionCases) {
318-
const got = partitionKnown(input);
330+
const got = partitionKnown(input, fixtureRegistry);
319331
const shape = { known: got.known.length, fresh: got.fresh.length, stale: got.stale.length };
320332
if (shape.known !== want.known || shape.fresh !== want.fresh || shape.stale !== want.stale) {
321333
console.error(` ✗ self-test "${label}": expected ${JSON.stringify(want)}, got ${JSON.stringify(shape)}`);
322334
failed++;
323335
}
324336
}
325337

338+
// [#10716] The live registry, at zero. It is closed to new entries by the rule
339+
// above, so "closed" is checked rather than merely written down: a wire-visible
340+
// code that genuinely needs deferring is a call for the ADR-0112 owner to make
341+
// in the open, not a line someone adds back here on the way past.
342+
if (KNOWN_LOWERCASE_CODES.size !== 0) {
343+
console.error(
344+
` ✗ self-test "the live registry stays empty": KNOWN_LOWERCASE_CODES holds ${KNOWN_LOWERCASE_CODES.size} entry/entries — ` +
345+
`this list is closed (#10658/#10716); a new deferral is an ADR-0112 decision, not a line here.`,
346+
);
347+
failed++;
348+
}
349+
326350
if (failed) {
327351
console.error(`\n✗ check-error-code-casing self-test failed (${failed} case(s)).`);
328352
process.exit(1);
329353
}
330354
console.log(
331-
`✓ check-error-code-casing self-test: ${cases.length} recognizer case(s) + ${partitionCases.length} registry case(s) pass.`,
355+
`✓ check-error-code-casing self-test: ${cases.length} recognizer case(s) + ${partitionCases.length + 1} registry case(s) pass.`,
332356
);
333357
}
334358

0 commit comments

Comments
 (0)