Skip to content

Commit fe8ec9e

Browse files
os-helpclaude
andauthored
test(plugin-auth): fail the D7 collision guard on unmapped better-auth-managed objects (#7822)
* test(plugin-auth): fail the D7 collision guard on unmapped better-auth-managed objects `managed-extension-fields.test.ts` derived better-auth's field surface through `MODEL_TO_OBJECT` and skipped any object that map did not produce (`if (!owned) continue`). An object declaring `managedBy: 'better-auth'` but absent from the map therefore got ZERO collision coverage while reading as covered — `sys_api_key` has been in exactly that position since #7727 gave it its first `MANAGED_EXTENSION_FIELDS` entry. Close the class rather than the instance, on the `SINGLE_RECORD_WRITE_ONLY` pattern from `api-methods-batch-conformance.test.ts`: - scan every `*.object.ts` under `packages/` for `managedBy: 'better-auth'` (22 objects today) and fail unless each is mapped in `MODEL_TO_OBJECT` or registered in a new `UNMAPPED_MANAGED_OBJECTS` registry with a reason; - fail when an object carrying `MANAGED_EXTENSION_FIELDS` entries is skipped by the collision loop, unless its registry entry claims `noBetterAuthColumns`; - fail on a stale registry entry (no longer a declared managed object, or now mapped); - pin the nine mapped models' derived surface so coverage cannot shrink; - pin the premise behind `sys_api_key`'s exemption: better-auth 1.7.0-rc.2 ships no `apiKey` plugin, so the exemption expires at the dependency bump rather than waiting for someone to enable it. The failure messages deliberately warn against the obvious fix: completing `MANAGED_EXTENSION_FIELDS` to make the comparison meaningful would widen the ADR-0092 D2 write whitelist on a credential table, which is a security decision and not a test fix. Refs #7770 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVc1ekPpi6yaWywAUhfzfd * test(plugin-auth): pin the covered-object list independently of MODEL_TO_OBJECT Reverse verification caught this in the assertion added one commit ago. The "coverage cannot shrink" pin compared `Object.keys(byObject)` against `Object.values(MODEL_TO_OBJECT)` — both sides derived from the same map, so deleting a mapping shrank the expected list in step with the actual one and the pin stayed green while the object dropped out of the collision loop. Ablation: removing `teamMember: 'sys_team_member'` produced one red (the accounting assertion), not the two predicted. Write the nine covered objects out as a literal and assert the map against it, so a deleted mapping and a model the library stops emitting each fail out loud, against a list that does not move with either. Refs #7770 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVc1ekPpi6yaWywAUhfzfd * test(plugin-auth): point at #7820, the second axis of the same blindness The unloaded-plugin axis: this file's getAuthTables() call loads `organization` only, so a column owned by a plugin the auth manager loads behind a flag is invisible even for a MAPPED object. `sys_user.phone_number` is a live instance. Not fixed here — widening the plugin set turns the collision assertion red on an ownership question nobody has decided. Refs #7770 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVc1ekPpi6yaWywAUhfzfd --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 225ab04 commit fe8ec9e

1 file changed

Lines changed: 325 additions & 0 deletions

File tree

packages/plugins/plugin-auth/src/managed-extension-fields.test.ts

Lines changed: 325 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,44 @@
1515
* at the pinned version, rather than trusting a hand-maintained list, and fails
1616
* on any overlap. The upgrade that would cause the collision is the moment
1717
* someone finds out.
18+
*
19+
* ## The absence that used to read as coverage (#7770)
20+
*
21+
* The derivation is keyed by `MODEL_TO_OBJECT`, and the collision loop skipped
22+
* any object that map does not produce — `if (!owned) continue`. So an object
23+
* declaring `managedBy: 'better-auth'` but missing from the map got ZERO
24+
* collision coverage while looking covered from the outside, and adding an
25+
* extension field to it would never have been checked by anything.
26+
* `sys_api_key` was in exactly that position from the moment #7727 gave it its
27+
* first `MANAGED_EXTENSION_FIELDS` entry.
28+
*
29+
* Absence is now accounted for in both directions, on the
30+
* `SINGLE_RECORD_WRITE_ONLY` pattern (`packages/spec/src/data/
31+
* api-methods-batch-conformance.test.ts`):
32+
*
33+
* - every object declaring `managedBy: 'better-auth'` anywhere under
34+
* `packages/` is either mapped in `MODEL_TO_OBJECT` or registered in
35+
* {@link UNMAPPED_MANAGED_OBJECTS} with the reason it is not;
36+
* - an unmapped object may carry declared extension fields only when
37+
* better-auth owns no column on that table at all (`noBetterAuthColumns`) —
38+
* otherwise the fields are unchecked and the gate says so;
39+
* - a registry entry that stopped describing a real, still-unmapped
40+
* better-auth object fails as stale, so the exemption list cannot rot into
41+
* documentation of nothing.
42+
*
43+
* There is a SECOND axis of the same blindness that this file does not close:
44+
* the `getAuthTables()` call below loads `organization` only, while the auth
45+
* manager loads many more plugins behind feature flags, so a column an unloaded
46+
* plugin owns is invisible to the comparison even for a MAPPED object.
47+
* `sys_user.phone_number` is a live instance — filed as #7820, deliberately not
48+
* fixed here, because widening the plugin set turns this guard red on an
49+
* ownership question nobody has decided.
1850
*/
1951

2052
import { describe, it, expect } from 'vitest';
53+
import { readFileSync, readdirSync, statSync } from 'node:fs';
54+
import { dirname, join, resolve } from 'node:path';
55+
import { fileURLToPath } from 'node:url';
2156
import { getAuthTables } from 'better-auth/db';
2257
import { organization } from 'better-auth/plugins';
2358

@@ -41,6 +76,178 @@ const MODEL_TO_OBJECT: Record<string, string> = {
4176
teamMember: 'sys_team_member',
4277
};
4378

79+
interface UnmappedManagedObject {
80+
/** Why this object is deliberately absent from `MODEL_TO_OBJECT`. */
81+
reason: string;
82+
/**
83+
* Set ONLY when better-auth owns no column on this table at the pinned
84+
* version — i.e. the table is ours end to end and an extension field on it
85+
* cannot collide with anything. This is what makes it safe for an unmapped
86+
* object to carry `MANAGED_EXTENSION_FIELDS` entries; without it, declared
87+
* fields on an unmapped object fail the "nothing is silently skipped"
88+
* assertion below.
89+
*
90+
* A claim this strong needs its own tripwire, not just a sentence — see the
91+
* `sys_api_key` premise test at the bottom of this file.
92+
*/
93+
noBetterAuthColumns?: true;
94+
}
95+
96+
/**
97+
* Objects that declare `managedBy: 'better-auth'` and are deliberately NOT in
98+
* `MODEL_TO_OBJECT`, keyed by object name with the reason.
99+
*
100+
* Adding an entry is a real decision: an unmapped object gets no D7 collision
101+
* coverage at all, so the reason has to say why that is the right answer for
102+
* this table rather than an oversight. `managedBy` alone does not mean
103+
* better-auth's core `getAuthTables()` surface owns the columns — a plugin may
104+
* be opt-in, ship in its own package, or (for sso/scim) expose no `schema`
105+
* option this call can read.
106+
*/
107+
const UNMAPPED_MANAGED_OBJECTS: Record<string, UnmappedManagedObject> = {
108+
sys_api_key: {
109+
// The one entry that carries declared extension fields, so the one whose
110+
// reason has to be a statement about better-auth rather than about scope.
111+
reason:
112+
'Hand-rolled ObjectStack table, not a better-auth model at all. '
113+
+ '`packages/core/src/security/api-key.ts` mints and verifies the key and POST /api/v1/keys '
114+
+ 'writes the row, and better-auth 1.7.0-rc.2 ships NO apiKey plugin: there is no '
115+
+ '"better-auth/plugins/api-key" export subpath and "better-auth/plugins" exports no apiKey. '
116+
+ 'So no model exists to derive and no column on this table can change hands. '
117+
+ 'Pinned by the premise test at the bottom of this file (#7770).',
118+
noBetterAuthColumns: true,
119+
},
120+
121+
// ── Opt-in core plugins this file's getAuthTables() call does not load ────
122+
// It loads `organization` only, which is the surface D7 has covered since
123+
// #3624. Column-level parity for these three IS covered — by
124+
// `better-auth-schema-parity.test.ts`, which loads their plugins. What is
125+
// missing here is only the D7 collision direction, and it costs nothing
126+
// while no extension field is declared on them. The moment one is, the
127+
// "nothing is silently skipped" assertion below turns red and the mapping
128+
// has to be done properly.
129+
sys_two_factor: {
130+
reason:
131+
"Owned by better-auth's opt-in `twoFactor` plugin (model `twoFactor`), which this file's "
132+
+ 'getAuthTables() call does not load. No extension field is declared on it.',
133+
},
134+
sys_device_code: {
135+
reason:
136+
"Owned by better-auth's opt-in `deviceAuthorization` plugin (model `deviceCode`), which this "
137+
+ "file's getAuthTables() call does not load. No extension field is declared on it.",
138+
},
139+
sys_jwks: {
140+
reason:
141+
"Owned by better-auth's opt-in `jwt` plugin (model `jwks`), which this file's getAuthTables() "
142+
+ 'call does not load. No extension field is declared on it.',
143+
},
144+
145+
// ── Plugins getAuthTables() structurally cannot see (#3653) ───────────────
146+
sys_sso_provider: {
147+
reason:
148+
'@better-auth/sso accepts no `schema` option, so getAuthTables() cannot see its models at all '
149+
+ '(#3653). Its columns are bridged mechanically by objectql-adapter.ts and gated by the '
150+
+ 'dedicated sso/scim block in better-auth-schema-parity.test.ts.',
151+
},
152+
sys_scim_provider: {
153+
reason:
154+
'@better-auth/scim accepts no `schema` option, so getAuthTables() cannot see its models at all '
155+
+ '(#3653). Same bridge and same dedicated gate as sys_sso_provider.',
156+
},
157+
158+
// ── @better-auth/oauth-provider — separate package, dedicated gate ────────
159+
// Excluded here for exactly the reason better-auth-schema-parity.test.ts
160+
// excludes it: it ships as its own package on its own pinned version and has
161+
// oauth-provider-schema-parity.test.ts covering its column surface.
162+
sys_oauth_application: { reason: '@better-auth/oauth-provider — separate package, covered by oauth-provider-schema-parity.test.ts.' },
163+
sys_oauth_access_token: { reason: '@better-auth/oauth-provider — separate package, covered by oauth-provider-schema-parity.test.ts.' },
164+
sys_oauth_refresh_token: { reason: '@better-auth/oauth-provider — separate package, covered by oauth-provider-schema-parity.test.ts.' },
165+
sys_oauth_consent: { reason: '@better-auth/oauth-provider — separate package, covered by oauth-provider-schema-parity.test.ts.' },
166+
sys_oauth_resource: { reason: '@better-auth/oauth-provider — separate package, covered by oauth-provider-schema-parity.test.ts.' },
167+
sys_oauth_client_resource: { reason: '@better-auth/oauth-provider — separate package, covered by oauth-provider-schema-parity.test.ts.' },
168+
sys_oauth_client_assertion: { reason: '@better-auth/oauth-provider — separate package, covered by oauth-provider-schema-parity.test.ts.' },
169+
};
170+
171+
/** The objects `MODEL_TO_OBJECT` maps right now — the live skip criterion. */
172+
const MAPPED_OBJECTS: readonly string[] = Object.values(MODEL_TO_OBJECT);
173+
174+
/**
175+
* The nine objects this guard has real collision coverage for, written out as
176+
* a LITERAL rather than derived from `MODEL_TO_OBJECT`.
177+
*
178+
* Deriving it would have made the pin below unable to see the regression it
179+
* exists for: deleting a mapping shrinks the derived surface and the expected
180+
* list together, so the assertion stays green while an object drops out of the
181+
* collision loop. Measured — an ablation that removed `teamMember` left a
182+
* derived version of this pin green and was caught only by the accounting
183+
* assertion. Two independent lists, so one of them has to be wrong out loud.
184+
*/
185+
const COVERED_OBJECTS: readonly string[] = [
186+
'sys_user',
187+
'sys_session',
188+
'sys_account',
189+
'sys_verification',
190+
'sys_organization',
191+
'sys_member',
192+
'sys_invitation',
193+
'sys_team',
194+
'sys_team_member',
195+
];
196+
197+
const HERE = dirname(fileURLToPath(import.meta.url));
198+
/** …/packages/plugins/plugin-auth/src → repo root */
199+
const REPO_ROOT = resolve(HERE, '../../../..');
200+
const PACKAGES_DIR = join(REPO_ROOT, 'packages');
201+
202+
/** Every `*.object.ts` under `packages/`, skipping build output and deps. */
203+
function walkObjectFiles(dir: string, out: string[] = []): string[] {
204+
let entries: string[];
205+
try {
206+
entries = readdirSync(dir);
207+
} catch {
208+
return out;
209+
}
210+
for (const entry of entries) {
211+
if (entry === 'node_modules' || entry === 'dist' || entry.startsWith('.')) continue;
212+
const full = join(dir, entry);
213+
let isDir: boolean;
214+
try {
215+
isDir = statSync(full).isDirectory();
216+
} catch {
217+
continue;
218+
}
219+
if (isDir) walkObjectFiles(full, out);
220+
else if (entry.endsWith('.object.ts')) out.push(full);
221+
}
222+
return out;
223+
}
224+
225+
/**
226+
* Every object declaring `managedBy: 'better-auth'`, scanned from source under
227+
* `packages/`.
228+
*
229+
* Scanning rather than importing is deliberate, and is the lesson
230+
* `api-methods-batch-conformance.test.ts` was written from: an audit whose
231+
* scope is a package name misses the declaration that lands in a package
232+
* nobody thought to open. Every such object is in `platform-objects/identity`
233+
* today; nothing makes that permanent.
234+
*
235+
* The `managedBy` match is anchored to the start of a line so a prose mention
236+
* in a comment (`sys_team.object.ts` has one) cannot be read as a declaration.
237+
*/
238+
function declaredBetterAuthObjects(): Array<{ object: string; file: string }> {
239+
const out: Array<{ object: string; file: string }> = [];
240+
for (const file of walkObjectFiles(PACKAGES_DIR)) {
241+
const source = readFileSync(file, 'utf8');
242+
if (!/^[ \t]*managedBy:\s*'better-auth'\s*,?\s*$/m.test(source)) continue;
243+
const object = source.match(
244+
/ObjectSchema\.create\(\{[\s\S]{0,600}?name:\s*'([a-z0-9_]+)'/,
245+
)?.[1];
246+
if (object) out.push({ object, file: file.slice(REPO_ROOT.length + 1) });
247+
}
248+
return out;
249+
}
250+
44251
/** better-auth authors fields in camelCase; ObjectStack columns are snake_case. */
45252
function toSnakeCase(name: string): string {
46253
return name.replace(/([a-z0-9])([A-Z])/g, '$1_$2').toLowerCase();
@@ -77,17 +284,108 @@ function betterAuthFieldsByObject(): Record<string, Set<string>> {
77284

78285
describe('managed extension fields (ADR-0105 D7)', () => {
79286
const byObject = betterAuthFieldsByObject();
287+
const managedObjects = declaredBetterAuthObjects();
80288

81289
it('derives a non-empty better-auth surface (the guard must not pass vacuously)', () => {
82290
expect(Object.keys(byObject).length).toBeGreaterThan(0);
83291
expect(byObject.sys_organization?.size ?? 0).toBeGreaterThan(0);
84292
expect(byObject.sys_user?.size ?? 0).toBeGreaterThan(0);
85293
});
86294

295+
it('keeps the nine mapped models covered, exactly (coverage cannot shrink)', () => {
296+
// Two ways an object can silently withdraw from the collision loop, and
297+
// this pins both against the same literal: the LIBRARY stops emitting the
298+
// model, or the MAP stops naming it. Either is the #7770 shape arriving
299+
// as a regression rather than as a pre-existing absence.
300+
expect(
301+
[...MAPPED_OBJECTS].sort(),
302+
'MODEL_TO_OBJECT no longer maps exactly the nine covered objects — a deleted mapping '
303+
+ 'withdraws an object from the collision loop as surely as a missing one ever did.',
304+
).toEqual([...COVERED_OBJECTS].sort());
305+
expect(
306+
Object.keys(byObject).sort(),
307+
'getAuthTables() no longer emits a model for every mapped object — the derived surface '
308+
+ 'shrank underneath the map, so the objects it dropped are no longer being compared.',
309+
).toEqual([...COVERED_OBJECTS].sort());
310+
for (const object of COVERED_OBJECTS) {
311+
expect(byObject[object]?.size ?? 0, `${object} derived an empty field set`).toBeGreaterThan(0);
312+
}
313+
});
314+
315+
it('scans a plausible number of better-auth-managed objects (guards a silently empty sweep)', () => {
316+
// A scan that matches nothing passes the two assertions below vacuously —
317+
// the exact failure mode this file is being hardened against. Pin a floor.
318+
expect(managedObjects.length).toBeGreaterThan(15);
319+
expect(managedObjects.map((m) => m.object)).toContain('sys_api_key');
320+
});
321+
322+
it("every better-auth-managed object is mapped or registered as deliberately unmapped (#7770)", () => {
323+
const unaccounted = managedObjects
324+
.filter(
325+
({ object }) =>
326+
!MAPPED_OBJECTS.includes(object) && UNMAPPED_MANAGED_OBJECTS[object] === undefined,
327+
)
328+
.map(({ object, file }) => `${object} (${file})`);
329+
expect(
330+
unaccounted,
331+
`these objects declare managedBy: 'better-auth' but are absent from MODEL_TO_OBJECT, so this `
332+
+ `guard skips them entirely and any extension field declared on them gets ZERO collision `
333+
+ `coverage while reading as covered: ${unaccounted.join(', ')}. Pick one deliberately:\n`
334+
+ ` (a) MAP IT — and it takes both halves, in this order: 1. pass the plugin that owns the `
335+
+ `model into the getAuthTables() call in betterAuthFieldsByObject(), because a map entry `
336+
+ `alone derives no table and the object stays silently skipped; 2. add the `
337+
+ `model: 'object_name' entry to MODEL_TO_OBJECT.\n`
338+
+ ` (b) REGISTER IT in UNMAPPED_MANAGED_OBJECTS with the reason better-auth owns no `
339+
+ `derivable surface for it here.\n`
340+
+ ` Do NOT complete MANAGED_EXTENSION_FIELDS just to make the comparison look meaningful: `
341+
+ `that map is also the ADR-0092 D2 write whitelist, so adding a column there widens what a `
342+
+ `generic write surface may touch. On a credential table that is a security change, not a `
343+
+ `test fix. Coverage of the columns better-auth itself owns is the parity gate's job `
344+
+ `(better-auth-schema-parity.test.ts), not D7's.`,
345+
).toEqual([]);
346+
});
347+
348+
it('keeps the unmapped registry free of stale entries', () => {
349+
// An exemption that no longer describes a real, still-unmapped
350+
// better-auth object reads as a documented decision while documenting
351+
// nothing — the failure mode the exemption itself was meant to prevent.
352+
const declared = new Set(managedObjects.map((m) => m.object));
353+
const stale = Object.entries(UNMAPPED_MANAGED_OBJECTS)
354+
.filter(([object]) => !declared.has(object) || MAPPED_OBJECTS.includes(object))
355+
.map(([object]) =>
356+
declared.has(object)
357+
? `${object} is now mapped in MODEL_TO_OBJECT — drop its exemption`
358+
: `${object} is no longer a declared managedBy: 'better-auth' object — drop its exemption`,
359+
);
360+
expect(stale, `stale UNMAPPED_MANAGED_OBJECTS entries: ${stale.join('; ')}`).toEqual([]);
361+
});
362+
363+
it('no declared extension field is silently skipped by the collision check', () => {
364+
// The `if (!owned) continue` below is the whole #7770 defect when it is
365+
// unaccounted for: the loop passes, having compared nothing.
366+
const skipped = Object.keys(MANAGED_EXTENSION_FIELDS).filter((object) => !byObject[object]);
367+
const unaccounted = skipped.filter(
368+
(object) => UNMAPPED_MANAGED_OBJECTS[object]?.noBetterAuthColumns !== true,
369+
);
370+
expect(
371+
unaccounted,
372+
`these objects declare extension fields in MANAGED_EXTENSION_FIELDS but no better-auth `
373+
+ `surface was derived for them, so the collision assertion skipped them and proved nothing `
374+
+ `about their fields: ${unaccounted.join(', ')}. Either map the object (see the `
375+
+ `two-step recipe in the previous assertion), or — only if better-auth genuinely owns no `
376+
+ `column on that table at the pinned version — register it in UNMAPPED_MANAGED_OBJECTS `
377+
+ `with noBetterAuthColumns: true and a tripwire that fails when that stops being true.`,
378+
).toEqual([]);
379+
// Positive half: the objects that ARE compared are the ones we think.
380+
expect(Object.keys(MANAGED_EXTENSION_FIELDS).filter((object) => byObject[object]).sort())
381+
.toEqual(['sys_invitation', 'sys_organization', 'sys_user']);
382+
});
383+
87384
it('no declared extension field collides with better-auth\'s own schema', () => {
88385
const collisions: string[] = [];
89386
for (const [object, fields] of Object.entries(MANAGED_EXTENSION_FIELDS)) {
90387
const owned = byObject[object];
388+
// Accounted for by the assertion directly above — never a silent pass.
91389
if (!owned) continue;
92390
for (const field of fields) {
93391
if (owned.has(field)) collisions.push(`${object}.${field}`);
@@ -133,3 +431,30 @@ describe('managed extension fields (ADR-0105 D7)', () => {
133431
expect(managedExtensionEditableFields('sys_session').size).toBe(0);
134432
});
135433
});
434+
435+
describe('sys_api_key exemption premise (#7770)', () => {
436+
it('better-auth still ships no apiKey plugin, so sys_api_key has no model to collide with', async () => {
437+
// This is the whole warrant for `noBetterAuthColumns: true` on
438+
// sys_api_key: the columns the issue worried about (`name`, `prefix`,
439+
// `key`, `userId`, `expiresAt`, `permissions`, `metadata`) belong to a
440+
// plugin that does not exist at the pinned version — better-auth 1.7.0-rc.2
441+
// publishes no `./plugins/api-key` subpath and `better-auth/plugins`
442+
// exports no `apiKey`.
443+
//
444+
// Going red here is CORRECT and is the point: a bump that (re)introduces
445+
// the plugin makes the exemption's premise expire BEFORE anyone can enable
446+
// it, which is one step earlier than the enablement this card anticipated.
447+
const plugins = (await import('better-auth/plugins')) as Record<string, unknown>;
448+
expect(
449+
plugins.apiKey,
450+
'better-auth now exports an apiKey plugin, so sys_api_key CAN acquire better-auth-owned '
451+
+ 'columns and its UNMAPPED_MANAGED_OBJECTS exemption no longer holds. Re-decide: keep the '
452+
+ 'hand-rolled table and do not load the plugin (restate the reason and repoint this '
453+
+ "tripwire at the auth manager's plugin list), or adopt the plugin — in which case map "
454+
+ "apikey: 'sys_api_key' AND pass apiKey() to getAuthTables() here, add sys_api_key to "
455+
+ 'better-auth-schema-parity.test.ts, and reconcile the overlapping columns (name, prefix, '
456+
+ 'key, user_id, expires_at) as an ownership decision, NOT by widening '
457+
+ 'MANAGED_EXTENSION_FIELDS, which is also the ADR-0092 D2 write whitelist.',
458+
).toBeUndefined();
459+
});
460+
});

0 commit comments

Comments
 (0)