From f6bc4fea0bdfe2586eb711584c497049ee5f3afc Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:18:55 +0800 Subject: [PATCH 1/2] fix(plugin-auth): remove the 200-row ceiling from the invitation carve-out probe hasPendingInvitationFor read at most 200 pending sys_invitation rows and scanned them in memory, so past 200 concurrently-pending invitations an invitee outside the first page was refused SELF_REGISTRATION_CLOSED under the invite_only default posture. The address now goes into the query (sys_invitation.email is indexed) and the page chain is exhausted. Measured on better-auth 1.7.1: the invitation route lowercases the address it stores and internalAdapter.createUser lowercases the registrant's before validateUserInfo, so both sides of the comparison are already normalized; the vendor's own findPendingInvitation / listUserInvitations / findMemberByEmail read with email.toLowerCase(). The row-side fold is kept so a case- or accent-folding collation cannot widen what counts as an invitation, expiry stays in JS so an unreadable expires_at keeps reading as live, and the catch still fails closed. Co-Authored-By: Claude Fable 5 --- .changeset/invitation-probe-page-ceiling.md | 42 +++++++ .../src/audience-gate-test-support.ts | 9 +- .../plugin-auth/src/audience-posture.test.ts | 99 ++++++++++++++- .../plugins/plugin-auth/src/auth-manager.ts | 118 +++++++++++++++--- packages/verify/src/harness.ts | 5 +- 5 files changed, 253 insertions(+), 20 deletions(-) create mode 100644 .changeset/invitation-probe-page-ceiling.md diff --git a/.changeset/invitation-probe-page-ceiling.md b/.changeset/invitation-probe-page-ceiling.md new file mode 100644 index 0000000000..ec445aaafc --- /dev/null +++ b/.changeset/invitation-probe-page-ceiling.md @@ -0,0 +1,42 @@ +--- +"@objectstack/plugin-auth": patch +--- + +fix(plugin-auth): the invitation carve-out stopped admitting past 200 pending invitations (#11770) + +Under the `invite_only` audience posture that #11739 made the default, an +administrator could send an invitation and the invitee's account creation would +still be refused with `SELF_REGISTRATION_CLOSED` — silently, with no signal to +either party — as soon as the environment held more than 200 concurrently +pending invitations. A 500-employee onboarding is an ordinary way to reach that. + +`AuthManager.hasPendingInvitationFor` answered "does this address hold a pending +invitation?" by reading at most 200 rows filtered only on `status = 'pending'` +and scanning them in memory for a case-insensitive email match. Past the first +page the invitee simply was not there, so the fail-closed `catch`-alike branch +applied the posture and refused a legitimate invitee. + +The address now goes into the query — `sys_invitation.email` carries a declared +index — and the page chain is exhausted, so no row count can hide a live +invitation. A page is "pending invitations addressed to this one person", which +better-auth bounds by refusing a second pending invitation per organization, so +this is not a read of the environment's pending population on the sign-up path; +in practice it is a single indexed lookup where the old code always read 200 +rows. + +The in-memory scan existed on the stated belief that invitation addresses are +stored as the inviter typed them. Measured against the installed better-auth +1.7.1, that is wrong on both halves — `organization/invite-member` lowercases +the address before storing it, and `internalAdapter.createUser` lowercases the +registrant's before calling `validateUserInfo` — and the vendor's own +`findPendingInvitation` / `listUserInvitations` / `findMemberByEmail` all query +with `email.toLowerCase()`, so a mixed-case row was never redeemable through +`accept-invitation` anyway. + +The row-side comparison is kept rather than deleted: `=` folds case on some +collations and folds accents with it, so every returned row is re-checked +against the normalized address — a case-only difference still matches, an +accent-only difference does not. Expiry stays in JS so a row with no readable +`expires_at` keeps reading as live. The security properties are unchanged: +`status = 'pending'` only, expiry still enforced, and an unanswerable probe +still means no carve-out. diff --git a/packages/plugins/plugin-auth/src/audience-gate-test-support.ts b/packages/plugins/plugin-auth/src/audience-gate-test-support.ts index 5c05a66eba..ad02411cad 100644 --- a/packages/plugins/plugin-auth/src/audience-gate-test-support.ts +++ b/packages/plugins/plugin-auth/src/audience-gate-test-support.ts @@ -44,7 +44,14 @@ interface TablesEngine { export function inviteForAudienceGate(engineOrManager: unknown, email: string): Promise { const row = { id: `inv_audience_${Math.random().toString(36).slice(2, 10)}`, - email, + // [#11770] Stored NORMALIZED, because that is the only form the product + // writes: `organization/invite-member` lowercases the address before it + // reaches `createInvitation`, and the vendor's own reads + // (`findPendingInvitation`, `listUserInvitations`) look it up with + // `email.toLowerCase()`. A fixture holding the inviter's raw casing would + // pin a row shape no invitation route can produce and no accept route + // could redeem. + email: email.trim().toLowerCase(), status: 'pending', // A dedicated org id so suites that count THEIR invitations per // organization never see these rows. diff --git a/packages/plugins/plugin-auth/src/audience-posture.test.ts b/packages/plugins/plugin-auth/src/audience-posture.test.ts index 0e4030f0b3..868997d1b4 100644 --- a/packages/plugins/plugin-auth/src/audience-posture.test.ts +++ b/packages/plugins/plugin-auth/src/audience-posture.test.ts @@ -493,8 +493,14 @@ describe('end of the chain: better-auth pipeline over the memory engine (#11739) const engine = createMemoryEngine(); seedExistingUser(engine); const manager = makeManager(engine); - seedPendingInvitation(engine, 'Bob@Acme.com'); - const admitted = await signUp(manager, 'bob@acme.com'); + // better-auth 1.7.1 normalizes BOTH sides before this gate ever runs: + // `organization/invite-member` lowercases the address it stores, and + // `internalAdapter.createUser` lowercases the registrant's before calling + // `validateUserInfo`. So the row holds the normalized form, and the + // case-insensitivity the invitee experiences is that the address THEY type + // may carry any case at all. + seedPendingInvitation(engine, 'bob@acme.com'); + const admitted = await signUp(manager, 'Bob@Acme.com'); expect(admitted.status).toBeLessThan(300); seedPendingInvitation(engine, 'late@acme.com', { expires_at: new Date(Date.now() - 60_000) }); @@ -503,6 +509,95 @@ describe('end of the chain: better-auth pipeline over the memory engine (#11739) expect((await refused.json()).code).toBe(SELF_REGISTRATION_CLOSED); }); + // ── [#11770] The page boundary of the pending-invitation probe ──────────── + // + // The carve-out's first spelling read `{ status: 'pending' }` with + // `limit: 200` and matched the address in JS, so an invitee outside that + // first page was refused `SELF_REGISTRATION_CLOSED` under the `invite_only` + // default — the invitation lane silently failing for the TAIL of a large + // rollout. The suite covered expiry and case-insensitivity but never crossed + // a page, which is precisely why the ceiling was invisible. + + it('invite_only: an invitation past the page boundary still admits — a 500-person rollout has no silent tail (#11770)', async () => { + const engine = createMemoryEngine(); + seedExistingUser(engine); + const manager = makeManager(engine); + + // A 500-employee onboarding, with the target seeded LAST so it sits far + // outside the first page of any read that pages the pending POPULATION. + for (let i = 0; i < 500; i++) seedPendingInvitation(engine, `colleague${i}@acme.com`); + seedPendingInvitation(engine, 'last.hire@acme.com'); + + const pending = engine.tables.get('sys_invitation') ?? []; + expect(pending.length).toBe(501); + // The fixture really crosses the old boundary — assert the placement + // rather than trusting the loop above to have produced it. + expect(pending.findIndex((r: any) => r.email === 'last.hire@acme.com')).toBeGreaterThan(200); + + const admitted = await signUp(manager, 'last.hire@acme.com'); + expect(admitted.status).toBeLessThan(300); + expect((engine.tables.get('sys_user') ?? []).some((u: any) => u.email === 'last.hire@acme.com')).toBe(true); + }); + + it('the probe reads sys_invitation NARROWED by email — the answer never depends on how many invitations exist (#11770)', async () => { + const engine = createMemoryEngine(); + seedExistingUser(engine); + const manager = makeManager(engine); + for (let i = 0; i < 300; i++) seedPendingInvitation(engine, `colleague${i}@acme.com`); + seedPendingInvitation(engine, 'targeted@acme.com'); + + const reads: any[] = []; + const find = engine.find.bind(engine); + engine.find = async (name: string, q: any = {}) => { + if (name === 'sys_invitation') reads.push(q); + return find(name, q); + }; + + const admitted = await signUp(manager, 'targeted@acme.com'); + expect(admitted.status).toBeLessThan(300); + + // Every read carries BOTH predicates. Without the email predicate the only + // way to stay correct is to walk the whole pending population, which is a + // read of unbounded size on the self-serve sign-up path. + expect(reads.length).toBeGreaterThan(0); + for (const q of reads) { + expect(q.where?.status).toBe('pending'); + expect(q.where?.email).toBe('targeted@acme.com'); + } + // And one page settled it: 301 pending rows, a single read. + expect(reads.length).toBe(1); + }); + + it('a case-folding collation may answer with a differently-cased row (matched) but never a different address (#11770)', async () => { + // Some collations answer `email = 'x'` case-insensitively — and MySQL's + // default folds accents with it. The JS re-check is what keeps the probe's + // answer identical across drivers: a case-only difference still matches, + // an accent-only difference must not. + const foldingEngine = (stored: string) => { + const engine = createMemoryEngine(); + seedExistingUser(engine); + seedPendingInvitation(engine, stored); + const find = engine.find.bind(engine); + engine.find = async (name: string, q: any = {}) => { + if (name !== 'sys_invitation') return find(name, q); + // The store ignores the email predicate entirely — the widest a + // folding collation could plausibly be. + const where: Record = { ...(q.where ?? {}) }; + delete where.email; + return find(name, { ...q, where }); + }; + return engine; + }; + + const cased = foldingEngine('Bob@Acme.com'); + expect((await signUp(makeManager(cased), 'bob@acme.com')).status).toBeLessThan(300); + + const accented = foldingEngine('bób@acme.com'); + const refused = await signUp(makeManager(accented), 'bob@acme.com'); + expect(refused.status).toBe(403); + expect((await refused.json()).code).toBe(SELF_REGISTRATION_CLOSED); + }); + it('email_domain: on-list admitted AND the declared permission set really lands; off-list refused with EMAIL_DOMAIN_NOT_ALLOWED', async () => { const engine = createMemoryEngine(); seedExistingUser(engine); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index aaaf7ba9ee..b40f45d8e8 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -3467,6 +3467,21 @@ export class AuthManager { */ private static readonly BOOTSTRAP_USER_PROBE_LIMIT = 50; + /** + * Page size of the pending-invitation probe ({@link hasPendingInvitationFor}). + * Unlike the bootstrap probe's limit this is NOT a bound on the answer — the + * probe's read is narrowed to one address and its page chain is exhausted — + * so the value only trades round trips against page size. + */ + private static readonly PENDING_INVITATION_PROBE_PAGE = 50; + + /** + * Termination guarantee for that page chain, not a ceiling on the answer: + * reaching it would mean 10k pending invitations addressed to a single + * person, and it is reported at error level rather than answered silently. + */ + private static readonly PENDING_INVITATION_PROBE_MAX_PAGES = 200; + /** `error` when the host logger carries it, else the guaranteed `warn` channel (#9754). */ private audienceLogError(message: string, meta?: Record): void { const logger = this.config.logger as @@ -3632,12 +3647,55 @@ export class AuthManager { } /** - * A pending, unexpired `sys_invitation` row exists for this email. Rows are - * fetched by status and compared lowercased in JS because invitation - * addresses are stored as the inviter typed them while better-auth - * lowercases the registrant's — a case-sensitive store-side equality would - * dead-end `Bob@Acme.com`'s invitee. Bounded read (an environment's pending - * invitations are few); unanswerable ⇒ no carve-out (fail closed). + * A pending, unexpired `sys_invitation` row exists for this email. + * + * ## Why the address goes into the QUERY rather than into a memory scan + * + * The first spelling read `{ status: 'pending' }` with `limit: 200` and + * matched the address in JS. That is a CEILING, not a bound: past 200 + * concurrently-pending invitations in one environment, an invitee outside + * the first page was not found, so under the `invite_only` default their + * registration was refused with `SELF_REGISTRATION_CLOSED` — the invitation + * lane silently failing for the TAIL of a large rollout, with no signal to + * the administrator or the invitee. A 500-employee onboarding reaches it. + * + * The scan existed to make the match case-insensitive, on the stated belief + * that invitation addresses are stored as the inviter typed them while + * better-auth lowercases the registrant's. Measured on the installed + * better-auth 1.7.1, that belief is wrong on BOTH halves — the vendor + * normalizes each side before this gate ever sees it: + * + * - `organization/invite-member` lowercases `ctx.body.email` and carries + * that value into `createInvitation` (and into the resend path), so the + * stored `sys_invitation.email` is already the normalized form; + * - `internalAdapter.createUser` lowercases `user.email` *before* it calls + * `validateUserInfo`, so the address this gate is asked about is already + * normalized too. + * + * The vendor's own reads agree: `findPendingInvitation`, + * `listUserInvitations` and `findMemberByEmail` all query with + * `email.toLowerCase()`. A mixed-case row would therefore be unredeemable + * by `accept-invitation` and invisible in the invitee's own inbox — the old + * tolerance admitted a registrant to an invitation they could never accept. + * + * So the address is pushed into the query (`sys_invitation.email` carries a + * declared index) and the page chain is EXHAUSTED. That removes the ceiling + * without putting a read of the environment's whole pending population on + * the self-serve sign-up path: a page here is "pending invitations + * addressed to this one person", which the vendor bounds by refusing a + * second pending invitation per organization. + * + * The JS comparison is KEPT and is not dead. `=` folds case on some + * collations (MySQL's default) and folds accents with it, so every returned + * row is re-checked against the normalized target: a case-only difference + * still matches — identical to the old behaviour on such a store — while an + * accent-only difference does not, so a folding collation cannot WIDEN what + * counts as an invitation. Expiry stays in JS for the reason it was there: + * a row with no readable `expires_at` keeps reading as live, which an + * `expires_at: { $gt: … }` predicate would silently narrow away. + * + * Unanswerable ⇒ no carve-out (fail CLOSED): the `catch` returns false, so + * the declared posture applies rather than an unverified admission. */ private async hasPendingInvitationFor(email: string): Promise { const engine = this.config.dataEngine; @@ -3646,19 +3704,47 @@ export class AuthManager { if (!target) return false; try { const reader = withSystemReadContext(engine) as any; - const raw = await reader.find('sys_invitation', { where: { status: 'pending' }, limit: 200 }); - const rows: any[] = Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : []; const nowMs = Date.now(); - for (const row of rows) { - const rowEmail = typeof row?.email === 'string' ? row.email.trim().toLowerCase() : ''; - if (rowEmail !== target) continue; - const expires = row?.expires_at ?? row?.expiresAt; - if (expires != null) { - const expMs = new Date(expires as any).getTime(); - if (Number.isFinite(expMs) && expMs <= nowMs) continue; + const page = AuthManager.PENDING_INVITATION_PROBE_PAGE; + const seenIds = new Set(); + for (let pageIndex = 0; pageIndex < AuthManager.PENDING_INVITATION_PROBE_MAX_PAGES; pageIndex++) { + const offset = pageIndex * page; + const raw = await reader.find('sys_invitation', { + where: { status: 'pending', email: target }, + limit: page, + // Omitted on the first page so the ordinary single-page read sends + // exactly the option shape every driver already answers. + ...(offset > 0 ? { offset } : {}), + }); + const rows: any[] = Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : []; + const idsBefore = seenIds.size; + for (const row of rows) { + if (row?.id != null) seenIds.add(row.id); + const rowEmail = typeof row?.email === 'string' ? row.email.trim().toLowerCase() : ''; + if (rowEmail !== target) continue; + const expires = row?.expires_at ?? row?.expiresAt; + if (expires != null) { + const expMs = new Date(expires as any).getTime(); + if (Number.isFinite(expMs) && expMs <= nowMs) continue; + } + return true; } - return true; + // Exhausted: a short page is the end of the chain. The second arm is + // the termination guarantee against a driver that accepts `offset` + // and ignores it — a page that carried no row id this loop had not + // already read cannot carry a new answer either, and looping forever + // on the sign-up path would be worse than the ceiling being removed. + if (rows.length < page) return false; + if (seenIds.size === idsBefore && rows.some((r) => r?.id != null)) return false; } + // Unreachable on any real population (that is 10k pending invitations + // addressed to ONE person). Loud rather than silent: the defect this + // method closes was a boundary nobody could see from either side. + this.audienceLogError( + '[audience] pending-invitation probe hit its page ceiling — the carve-out is being refused ' + + 'without having read every pending invitation for this address.', + { pages: AuthManager.PENDING_INVITATION_PROBE_MAX_PAGES, pageSize: page }, + ); return false; } catch { return false; diff --git a/packages/verify/src/harness.ts b/packages/verify/src/harness.ts index 73ee81230b..c2c448f782 100644 --- a/packages/verify/src/harness.ts +++ b/packages/verify/src/harness.ts @@ -700,7 +700,10 @@ export async function bootStack( 'sys_invitation', { id: `inv_verify_${Math.random().toString(36).slice(2, 10)}`, - email, + // [#11770] Normalized, as the invitation route stores it — the + // audience probe reads `sys_invitation` by address, and better-auth + // lowercases both the stored address and the registrant's. + email: email.trim().toLowerCase(), status: 'pending', // A dedicated org id so fixtures counting THEIR invitations never // see these rows. From d29ebd2bb20e74c11e70e087db972d161155aa25 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:25:22 +0800 Subject: [PATCH 2/2] test(plugin-auth): pin the probe's read as invariant in the pending population MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read count is >1 on the email sign-up route by design (the route pre-checks the same decision the validateUserInfo gate then makes), so the pin is invariance — 2 pending rows and 401 produce identical reads, each narrowed by email and each a single page — rather than a raw count. Co-Authored-By: Claude Fable 5 --- .../plugin-auth/src/audience-posture.test.ts | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/packages/plugins/plugin-auth/src/audience-posture.test.ts b/packages/plugins/plugin-auth/src/audience-posture.test.ts index 868997d1b4..963e9b725e 100644 --- a/packages/plugins/plugin-auth/src/audience-posture.test.ts +++ b/packages/plugins/plugin-auth/src/audience-posture.test.ts @@ -539,33 +539,42 @@ describe('end of the chain: better-auth pipeline over the memory engine (#11739) expect((engine.tables.get('sys_user') ?? []).some((u: any) => u.email === 'last.hire@acme.com')).toBe(true); }); - it('the probe reads sys_invitation NARROWED by email — the answer never depends on how many invitations exist (#11770)', async () => { - const engine = createMemoryEngine(); - seedExistingUser(engine); - const manager = makeManager(engine); - for (let i = 0; i < 300; i++) seedPendingInvitation(engine, `colleague${i}@acme.com`); - seedPendingInvitation(engine, 'targeted@acme.com'); - - const reads: any[] = []; - const find = engine.find.bind(engine); - engine.find = async (name: string, q: any = {}) => { - if (name === 'sys_invitation') reads.push(q); - return find(name, q); + it('the probe reads sys_invitation NARROWED by email — the work never depends on how many invitations exist (#11770)', async () => { + const readsForPopulation = async (otherPending: number) => { + const engine = createMemoryEngine(); + seedExistingUser(engine); + const manager = makeManager(engine); + for (let i = 0; i < otherPending; i++) seedPendingInvitation(engine, `colleague${i}@acme.com`); + seedPendingInvitation(engine, 'targeted@acme.com'); + + const reads: any[] = []; + const find = engine.find.bind(engine); + engine.find = async (name: string, q: any = {}) => { + if (name === 'sys_invitation') reads.push(q); + return find(name, q); + }; + + const admitted = await signUp(manager, 'targeted@acme.com'); + expect(admitted.status).toBeLessThan(300); + return reads; }; - const admitted = await signUp(manager, 'targeted@acme.com'); - expect(admitted.status).toBeLessThan(300); + const small = await readsForPopulation(1); + const large = await readsForPopulation(400); // Every read carries BOTH predicates. Without the email predicate the only - // way to stay correct is to walk the whole pending population, which is a - // read of unbounded size on the self-serve sign-up path. - expect(reads.length).toBeGreaterThan(0); - for (const q of reads) { + // way to stay correct is to walk the whole pending population, which is an + // unbounded read on the self-serve sign-up path. + expect(small.length).toBeGreaterThan(0); + for (const q of [...small, ...large]) { expect(q.where?.status).toBe('pending'); expect(q.where?.email).toBe('targeted@acme.com'); + expect(q.offset ?? 0).toBe(0); // one page settled it, either way } - // And one page settled it: 301 pending rows, a single read. - expect(reads.length).toBe(1); + // 2 pending rows or 401 — identical work. (The read count is >1 because + // the email sign-up route pre-checks the very decision the + // `validateUserInfo` gate then makes; see `validateAudienceAdmission`.) + expect(large.length).toBe(small.length); }); it('a case-folding collation may answer with a differently-cased row (matched) but never a different address (#11770)', async () => {