Repository navigation
Invitation carve-out silently stops admitting past 200 pending invitations (in-memory scan of a capped read) #11770
Description
Activity
Triage (first-touch grading): graduated
finding→pm:blocked,domain:services, type Bug.Blocked-by: #11739
Rationale: under the ruled default posture
invite_only, an invited user past the 200-pending-row page boundary is refused registration withSELF_REGISTRATION_CLOSED— a silent functional ceiling on exactly the flow the audience-posture epic exists to make work, and a 500-employee onboarding is an ordinary way to hit it. Graded Bug, not observation: the declared behaviour ("an invitee with a pending invitation may register") is not enforced past the first page. Blocked rather than queued because the fix site (AuthManager.hasPendingInvitationFor, added by PR #11767) is not onorigin/mainuntil #11739 lands — re-verify on the merged ref at unlock, and pin with a >1-page pending-invitations test per the card's own note (the current suite covers expiry and case-insensitivity but not the page boundary).Session:
session_012od9QE3Wzmgb3aXQ7U156P(triage seat, hourly round)
Generated by Claude Code
Claim: PM loop (maintainer direct-dispatch channel)
Session:session_71836b57-5db6-459d-ae4d-c6d9d7dd2cc6
Branch:claude/issue-11770-invitation-probe-ceiling
Worktree:objectstack-issue-11770
Domain:domain:services
File surface:packages/plugins/plugin-auth/src/auth-manager.ts(thehasPendingInvitationForprobe) + its test (stop on breach; explain in the report)
Container & model: S–M,mode:subagent,model: opus
Clause-②: no
Serial constraints cleared: the Blocked-by (#11739) is MERGED (4f24e9d2e). Sibling #11768 also lands in plugin-auth and is deliberately held OUT of this batch (same-package rule) — it stayspm:queue, unassigned. #11959 touchespackages/specand is held for a contract-tier dispatch. No open PR claimsauth-manager.ts.{ "issue": 11770, "status": "done", "branch": "claude/issue-11770-invitation-probe-ceiling", "pr": "https://github.com/objectstack-ai/objectstack/pull/12070", "premise_still_valid": true, "summary": "Premise re-verified on origin/main after #11739 merged (4f24e9d2e): hasPendingInvitationFor still read `{ status: 'pending' }` with `limit: 200` and matched the address in JS. 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 and the read is not a population-sized scan on the self-serve sign-up path. The card's 'verify before designing around it' instruction changed the design: measured against the installed better-auth 1.7.1 dist, the docblock's premise is FALSE on both halves — organization/invite-member lowercases ctx.body.email before createInvitation (create and resend), and internalAdapter.createUser lowercases user.email before calling validateUserInfo — and the vendor's own findPendingInvitation / listUserInvitations / findMemberByEmail all read with email.toLowerCase(), so a mixed-case row was never redeemable through accept-invitation anyway. Consequence a reviewer should see: on a case-SENSITIVE store a mixed-case stored address is no longer matched. That is a deliberate narrowing of a row shape the product cannot write and cannot redeem, argued in the PR body; the row-side fold is KEPT so a case- or accent-folding collation (MySQL default) still matches case-only differences and still refuses accent-only ones, and expiry stays in JS so an unreadable expires_at keeps reading as live. Security properties unchanged: status='pending' only, expiry enforced, catch fails CLOSED. A case-insensitive query predicate was rejected on evidence, not taste: $ilike is declared but STAGED AHEAD OF ITS BACKENDS (packages/spec filter-operator-vocabulary.test.ts pins STAGED_AHEAD_OF_BACKENDS = ['$ilike','$like']; driver-memory and driver-mongodb refuse it loudly), which under this method's fail-closed catch would become 'no invitation' on those backends — the same defect, everywhere; $icontains is enforced but is a substring match and would WIDEN. Also re-pointed the existing case-insensitivity fixture (row seeded lowercase, invitee signs up mixed-case) and normalized the address in both invitation test-support helpers, so no fixture pins a row shape the product cannot produce.", "tests": "All at d29ebd2bb, every heavy command routed through scripts/pm/os-verify-lock.sh; exit codes captured before any pipe. DECLARED NARROWING: this macOS host has no usable flock (bash 3.2; flock is util-linux), so the script's own --status reports DECLARED UNLOCKED MODE and nothing was serialized against sibling agents — declared in the PR body, quoting the script's wording. (1) Build closure: `pnpm --filter '@objectstack/plugin-auth^...' build` -> VERDICT command-exit 0 (1128s); then `--filter '@objectstack/plugin-auth...' --filter '@objectstack/verify...' build` -> VERDICT command-exit 0 (101s). (2) Targeted: audience-posture.test.ts -> 'Tests 45 passed (45)', VERDICT command-exit 0. (3) Package: `pnpm --filter @objectstack/plugin-auth test` -> 'Test Files 76 passed (76)' / 'Tests 1570 passed (1570)', VERDICT command-exit 0. (4) typecheck plugin-auth + verify -> both 'Done', VERDICT command-exit 0 (the first attempt failed only because plugin-auth's own dist was absent for tsconfig.examples.json; building the package cleared it). (5) Repo-wide `pnpm lint` (eslint . --no-inline-config) -> VERDICT command-exit 0 in 32s — the whole farm, NOT a narrowed run, so no narrowing evidence is owed for lint. (6) Gate families derived with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (it confirmed the repo assertion and derived the change set itself from merge-base be21955ba): 18 of 19 exit 0 — check:nul-bytes, check:changeset-gate-self-tests, check:cross-package-test-inputs (script + pnpm forms), check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:type-check-coverage, check-adr-0087-registration, check-changeset-no-major, check-ci-filter-parity, check-empty-changeset, check-plugin-teardown-shape, release-rehearsal-clone --self-test. The one red is check:objectui-changeset, exit 1, host-only: its --self-test shells scripts/bump-objectui.sh which uses `mapfile` (a bash 4 builtin) and this host is GNU bash 3.2.57 — every failing case prints 'bump-objectui.sh: line 324: mapfile: command not found', and neither that script nor .objectui-sha is in the five-file diff. Filed as #12071. ABLATION: only auth-manager.ts reverted to the branch base be21955ba, tests kept, restore trapped on EXIT/INT/TERM. Mutation proven on disk BEFORE the result was read, anchored on both texts rather than a bare --stat: marker PENDING_INVITATION_PROBE_PAGE 2 -> 0 and \"where: { status: 'pending' }, limit: 200\" 0 -> 1, porcelain ' M packages/plugins/plugin-auth/src/auth-manager.ts'. Direction observed: RED, with the card's own symptom — 'FAIL … invite_only: an invitation past the page boundary still admits' / 'AssertionError: expected 403 to be less than 300' (403 = SELF_REGISTRATION_CLOSED) / 'Tests 2 failed | 43 passed (45)'. Note the case-insensitivity and expiry test stayed GREEN under the mutation, which is the point: the boundary test is what carries the new signal. Restore leg proven the same way (markers back to 2 / 0, porcelain empty) and the suite returns to 45 passed (45). NO REBUILD LEG APPLIES and none is claimed: the subject is imported as './auth-manager', a relative import inside the package, so vitest resolves it from SOURCE — no dist sits between the mutation and the assertion.", "open_questions": [], "out_of_scope_findings": [ "filed as #12071: check:objectui-changeset --self-test cannot run on macOS because scripts/bump-objectui.sh uses the bash-4 `mapfile` builtin (lines 324/327) and macOS ships bash 3.2 — CI is Linux so it is green there, but the pin-bump procedure is run by hand locally and the #10495 branch-reachability warnings are exactly the code that cannot execute" ] }ACCEPT — PR #12070. Verified against the branch and the vendor dist, not against the report.
- The dispatch's "verify before designing around it" earned its keep. The docblock this card was filed against claimed invitation addresses are stored as the inviter typed them — that premise is false, and the fix design changed because of it. Independently confirmed by this seat in the installed better-auth 1.7.1 dist:
organization/routes/crud-invites.mjs:87isconst email = ctx.body.email.toLowerCase();, and the vendor's own recipient checks compareinvitation.email.toLowerCase(). A mixed-case stored row was never redeemable through accept-invitation, so narrowing to a query-side match on the normalized address gives up a row shape the product can neither write nor honour. - The ceiling is gone at the right layer: the address is now IN the query (
where: { status: 'pending', email: target }) rather than scanned for in memory, and the page chain is exhausted with aseenIdsguard so a driver that ignoresoffsetcannot spin. That also stops this being a population-sized read on the self-serve sign-up path, which matters now that every gated signup pays for it. - Security properties preserved, and one deliberately kept in JS:
status='pending'only, expiry still enforced client-side so an unreadableexpires_atreads as LIVE (fail-safe direction),catchstill fails CLOSED, and the row-side case fold is retained so a folding collation still matches case-only differences while accent-only differences stay refused. $ilikerejected on evidence, not taste — it is declared but staged ahead of its backends (driver-memory/driver-mongodbrefuse it loudly), which under this method's fail-closed catch would silently become "no invitation" on those backends: the same defect, spread wider.$icontainswould widen to substring. Correct call.- Ablation is the strong form: mutation proven on disk by anchoring on BOTH the injected marker and the deleted text, direction predicted before reading, and the telling detail — the case-insensitivity and expiry tests stayed GREEN under the mutation while only the boundary test went red. That is the new signal carrying its own weight rather than riding existing coverage.
- Not a governed face, no
packages/specin the diff (checked: 0 files), so no contract-review chain applies. Host-only gate red (check:objectui-changesetneeds bash-4mapfile, this host is 3.2) filed as [finding] check:objectui-changeset --self-test cannot run on macOS: bump-objectui.sh uses the bash-4mapfilebuiltin #12071 — environmental, CI owns it on Linux.
Landing: enqueue once the remaining checks report green.
- The dispatch's "verify before designing around it" earned its keep. The docblock this card was filed against claimed invitation addresses are stored as the inviter typed them — that premise is false, and the fix design changed because of it. Independently confirmed by this seat in the installed better-auth 1.7.1 dist:
Found while reviewing PR #11767 (audience posture, #11739). Fail-closed, so not a security hole — a functional ceiling in the invitation carve-out.
AuthManager.hasPendingInvitationFor(email)(plugin-authauth-manager.ts) resolves "does this address hold a pending invitation?" by reading at most 200 pending rows and scanning them in memory for a case-insensitive email match:Above 200 concurrently-pending invitations in one environment, an invitee outside the first page is not found. Under the new default posture
invite_onlythat means: an administrator sends an invitation, the invitee tries to create their account, and registration is refused withSELF_REGISTRATION_CLOSED— the invitation lane silently stops working for the tail of a large rollout, with no signal to either party. A 500-employee onboarding is an ordinary way to reach that ceiling, and it is the exact flow the audience-posture epic exists to make work.The in-memory scan appears to exist so the match can be case-insensitive. Likely fixes (implementer judges): filter on the email in the query (better-auth lowercases invitation emails on write — verify) and drop the scan; or keep the scan but page until exhausted; or add a driver-supported case-insensitive predicate. Whatever lands should be pinned by a test with more than one page of pending rows — the current suite covers expiry and case-insensitivity but not the page boundary, which is why the ceiling is invisible.
Not fixed in #11767: the carve-out is the dev's own scoped addition there and the behaviour is fail-closed, so this was filed rather than sent back for a rework round.