Skip to content

fix(security): measure the unscoped existence page cap instead of trusting it - #11962

Merged
os-sam merged 1 commit into
mainfrom
claude/issue-11518-unscoped-page-cap
Aug 25, 2026
Merged

fix(security): measure the unscoped existence page cap instead of trusting it#11962
os-sam merged 1 commit into
mainfrom
claude/issue-11518-unscoped-page-cap

Conversation

@claude

@claudeclaudeBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes#11518

The defect

buildExistingByName (packages/plugins/plugin-security/src/seed-name-lookup.ts) is the batched
existence oracle the identity seeders consult in place of a per-item read. Its UNSCOPED page was
capped at limit: names.length — exact only while one row can exist per name. Since #8461 /
ADR-0120 D1 sys_capability.name and sys_permission_set.name are unique per organization, and
ADR-0066 D1 explicitly encourages admins to EXTEND the registry inside their own organization, so one
name legitimately carries a row per organization plus the platform's.

The rows that fall off a full page are the highest ids under #4363's ORDER BY id ASC tie-breaker,
so whole names vanish from the page — and a name missing from the page reads as absent, which
routes its caller to the INSERT branch. #10103 had already found and repaired exactly this on the
SCOPED arm (its comment sat directly above the line); the unscoped arm never got the repair, and two
seeders on main read unscoped — bootstrapDeclaredCapabilities and permission-set-projection's
env-overlay pass.

names.length * 2 is not the fix — it is the same defect with a larger constant. Rows-per-name
is bounded only by the number of organizations, so no constant multiplier is correct.

The shape implemented

The cap stopped being a promise and became a measurement. readNamePage asks the driver for one
row more than the budget it is willing to hold:

  • page.length <= budget — the driver had no budget + 1-th row to give, so the page is provably the
    complete set for these names;
  • page.length > budget — there is at least one more row than this read will hold, so the page is a
    prefix of the answer and cannot be read as one.

Both directions are exact, which is what makes the budget a free choice: no complete page is ever
mistaken for a truncated one, and no truncated page for a complete one. An over-budget page joins the
module's existing "could not answer" causes and reuses the degradation path that was already there
for a driver that cannot do $inperItemIndex. No second fallback was invented.

Budgets:

armbudgetwhy
unscopedmax(names.length * 4, 20)a budget, not a bound — nothing bounds rows-per-name here. Shape matches the sibling generous cap on the same table in security-plugin.ts's permission-set loader
scopednames.length * 2 (unchanged, #10103)here the number is a proven bound: applyTenantScope returns this organization's rows plus organization-less ones, and the declared name index is unique per organization

The scoped arm keeps #10103's exact cap and gains the same probe, so a scoped page that overflows the
proven bound — reachable only where the unique index is absent or not yet created, the deployment the
module header already calls out — becomes a loud degradation instead of a silent truncation.

⭐ Declared behaviour change

This changes existing callers' behaviour in the truncating case: from a silent wrong answer to a
loud slow one.
That is the intended direction — a read that under-reports and then inserts is the
defect — but it lands on two shipped seeders and is stated here rather than slipped in:

  • names that used to be reported absent (and re-inserted as duplicates, or refused by the unique key
    as a "collision" naming a row nobody ever saw) are now answered correctly;
  • the cost is one read per name plus one warning naming the object and the budget it could not fit
    inside. [security] batched seed existence read TRUNCATED — … is a separate diagnostic from the
    existing failed-read one, because the remedies differ: an outage versus an install whose catalog is
    simply wider than one page.
  • An install that does not overflow the budget — every stock one, where a name carries a single row —
    issues exactly the same single read it issued before and says nothing.

Scope

Verification

Every exit code captured before any pipe; each gate quoted by its own verdict line. Union run at
cb9d109c25, the head commit of this branch.

The failing case the card handed over transplants.bootstrap-system-capabilities.test.ts's
makeQl double is the one that HONOURS limit and orders by id ascending (the two shipped-driver
behaviours the defect is made of), and its 8-curated-platform-rows + 2-organization-rows fixture is
reused directly. The test that pinned the defect — "WITHOUT the predicate the page also TRUNCATES, and
a truncated page reads as "absent""
— asserted the wrong outcome and is rewritten in place, with
the change flagged in its body as the repair rather than a weakened assertion. #11451's own argument is
preserved and sharpened: what the predicate buys, and what no page budget can reach, is which row
answers
.

Reverse-verification. Prediction written before the run; seed-name-lookup.ts restored to its
origin/main blob by plain file write (never git checkout <ref> -- <path>, which stages the ablated
blob), under a trap … EXIT INT TERM restore. Mutation confirmed on disk by marker counts, not by
an editor exit code: limit: budget + 1 1 → 0, names.length * 2 : names.length 0 → 1.
No rebuild is involved and none is owed: the ablated subject is imported relatively
(./seed-name-lookup.js) from inside the same package, so vitest reads src/ — there is no
exports/dist hop for the mutation to miss.

Predicted RED: 5 named tests. Observed: exactly those 5, at exactly the predicted assertions.

testpredictedobserved
#11451 › WITHOUT the predicate the page hauls every organization's copynames[6..7] read absentstudio.access: expected 'absent' to be 'present'
#11518 › reads every name whose row exists, in ONE round tripabsent === [names[6], names[7]]expected [ 'studio.access', 'manage_sharing' ] to deeply equal []
#11518 › an overflowing page degrades to the per-item read — LOUDLYsilent truncation, names read absentmanage_org_users: expected 'absent' to be 'present'
#11518 › a SCOPED page that overflows that bound degrades toostatus assertions still hold; the warn assertion failsexpected "vi.fn()" to be called 1 times, but got 0 times
#11518 › re-seeds a healthy multi-organization install WITHOUT writing anythingout.seeded === 2, two duplicate INSERTsexpected 2 to be +0

Tests 5 failed | 71 passed (76). Every control stayed green as predicted — both POSITIVE CONTROLs
(they pin the DOUBLE, so they hold on both sides), the unreadable-page diagnostic, and the scoped
exact-bound test. Restored and verified byte-identical: disk 0775e020d9… == index == HEAD, git status --porcelain empty.

The consequence measured on the seeder, not argued.bootstrap-declared-capabilities.test.ts's
double ignored limit entirely, which made this whole class of page-cap defect inexpressible in the
file where its consequence actually lands. It now honours limit with the #4363 id-ascending
tie-breaker, matching the sibling double. On a REBUILD over a healthy multi-organization install the
old code inserted two duplicate rows (out.seeded === 2); it now writes nothing.

Gates, each by its own verdict line:

  • pnpm --filter @objectstack/plugin-security testTest Files 80 passed (80) / Tests 1523 passed (1523), os-verify-lock: VERDICT command-exit 0
  • pnpm --filter @objectstack/plugin-security typecheckVERDICT command-exit 0
  • pnpm check:type-check-debt (the ratchet half, --re-measure, run on a built closure) —
    check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 243.5s, 1898 raw tsc error(s) total, none above its recorded number.@objectstack/plugin-security is not among the
    drifted; the one reported surplus is @objectstack/plugin-approvals (−1, an entry that can be
    lowered
    ), a package this PR does not touch. The ledger was not raised.
  • pnpm check:type-check-coverageOK — 65/78 workspace packages type-checked … 19 package(s) still hide their own tests
  • pnpm check:engine-double-contractOK — 405 pinned, 133 in the DEBT ledger, 2 exempt.
  • pnpm check:where-matcher297 matcher(s) discovered, 297 answer the combinator battery correctly or refuse it loudly … none new.
  • pnpm check:query-options-erasureratchet holds: 67 unswept non-test site(s) … none new · test surface: 240 site(s) … at the ceiling
  • pnpm check:cross-package-test-inputsOK: 16 package(s) read outside themselves, all declared
  • pnpm check:test-source-aliasOK — 72 packages with tests scanned
  • pnpm check:nul-bytesOK (scanned 6655 text file(s) … no raw ASCII control bytes)
  • pnpm check:i18nOK (9 package(s) — all bundles in sync, no undeclared authoring keys).⚠️ its
    first run returned PREREQUISITE NOT MET — the workspace CLI is not built, which is not measured,
    never a pass; re-run green after the closure build.
  • pnpm lint (the full repo scan, eslint . --no-inline-config) — VERDICT command-exit 0, so no
    narrowing is claimed here
  • also green: check:published-files, check:slot-lookup, check:type-source-resolution,
    check:changeset-gate-self-tests, check:objectui-changeset, check-plugin-teardown-shape,
    check-ci-filter-parity, check-adr-0087-registration, check-changeset-no-major,
    check-empty-changeset, release-rehearsal-clone --self-test

Gate family derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
(gate list derived from the tree of 'objectstack-ai/objectstack' at commit cb9d109c25), not from a
recalled list.

Changeset: .changeset/seed-name-lookup-unscoped-page-budget.md (@objectstack/plugin-security: patch)
— the behaviour change is stated there too, since it is what a reader of the release notes needs.


Generated by Claude Code

…sting it (#11518)
`buildExistingByName`'s UNSCOPED page was capped at `limit: names.length`,
exact only while one row can exist per name. Since #8461 / ADR-0120 D1 the
identity tables are unique PER ORGANIZATION and ADR-0066 D1 encourages admins
to EXTEND the registry inside their own organization, so one name legitimately
carries a row per organization plus the platform's. The rows that fall off a
full page are the highest ids under #4363's `ORDER BY id ASC`, so whole names
vanish — and a vanished name reads as `absent`, which INSERTS.
No constant multiplier is correct (the bound is the organization count), so the
cap is now a measurement: the read asks for one row MORE than it will hold, and
a page carrying that extra row is a PREFIX of the answer. It joins the module's
existing "could not answer" causes and degrades to the per-item read — the
fallback already there for a driver without `$in` — with its own diagnostic.
⚠️ Behaviour change on two shipped seeders in the truncating case: from a
silent wrong answer to a loud slow one. Non-truncating installs are unchanged,
one read and no warning.
The SCOPED arm keeps #10103's `names.length * 2` — there the number is a proven
bound, not a budget — and gains the same probe, so a scoped page that overflows
it (an absent unique index) degrades loudly instead of truncating silently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

7 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ⚠️1 changed file(s) yielded no anchor (packages/plugins/plugin-security/src/packaged-permission-set-lock.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-security/src/packaged-permission-set-lock.ts) — pages documenting those are invisible to this run
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 14 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 0140e678fa4771ea8330a9da075af96e1f8798bdpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 5bfa01440689d67350dea7af31d1adc675e5eb0d — the merge of head cb9d109c258aa6039df7482c9d983965245bb544 into base 0140e678fa4771ea8330a9da075af96e1f8798bd, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5bfa01440689d67350dea7af31d1adc675e5eb0d && git checkout 5bfa01440689d67350dea7af31d1adc675e5eb0d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0140e678fa4771ea8330a9da075af96e1f8798bd cb9d109c258aa6039df7482c9d983965245bb544 && git checkout -B drift-repro 0140e678fa4771ea8330a9da075af96e1f8798bd && git merge --no-ff cb9d109c258aa6039df7482c9d983965245bb544
node scripts/docs-audit/affected-docs.mjs --json 0140e678fa4771ea8330a9da075af96e1f8798bd

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 25, 2026
@os-samClaude

Copy link
Copy Markdown
Collaborator

Docs hand-read done — no page is falsified, and one page is un-falsified

PM note (domain:services seat). The drift check on this PR reported 7 anchors with no page naming them, plus ⚠️ one changed file yielding no anchor (packaged-permission-set-lock.ts) — and it says in terms that this is "not a clean bill of health." Its self-documented blind spot is that a page stating a rule by its inputs shares no identifier with the emitter, so an emitter-only diff can never list it. That blind spot is only closed by hand, so I closed it.

Method — pinned to a ref rather than a working tree (git grep … origin/main -- 'content/docs/**'), with a positive control (5 pages mention sys_capability on that ref, so the channel reaches the docs tree).

Result — two pages name this seeder's implementation directly, which the anchor set did not surface: content/docs/permissions/authorization.mdx:179 and content/docs/permissions/capabilities.mdx (lines 52, 67, 159).

The load-bearing one is capabilities.mdx:159:

bootstrapDeclaredCapabilities runs on kernel:ready and is idempotent: it re-seeds on every boot, so the row always reflects the shipped declaration.

That sentence was false on main, and this PR is what makes it true. On any install using ADR-0066 D1 — an admin extending the registry inside their own organization — the truncated page reported the package's own names absent and the seeder re-inserted them. This PR's own fixture says so in the assertion comment: "Before the repair this was seeded: 2 and two DUPLICATE rows for names whose package rows were sitting in the table — every boot." A seeder that writes two duplicate rows on every boot is not idempotent.

No docs edit is owed by this PR, and none should be added to it. The page already documents the intended contract correctly; the code was the half that disagreed. Worth recording because it is independent corroboration of severity from a source neither the card nor the implementation cited: the documentation asserted an invariant the read path had quietly stopped honouring, and nothing connected the two.

⚠️ On the file the bot could not anchor (packaged-permission-set-lock.ts): its diff here is docblock prose only — no behaviour — so no page can have gone false against it. The "not covered" warning is real in general and inert on this particular diff.

⛔ No action requested from the implementer. This is a review-side measurement, recorded so it is not re-derived.


Generated by Claude Code

@os-sam
os-sam marked this pull request as ready for review August 25, 2026 03:35
@os-sam
os-sam added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit e1d773eAug 25, 2026
32 checks passed
@os-sam
os-sam deleted the claude/issue-11518-unscoped-page-cap branch August 25, 2026 03:52
os-steve pushed a commit that referenced this pull request Aug 25, 2026
The initial measurement was taken at merge base 311e207. Merging 42 commits
of main moved the corpus underneath it: three new test files arrived carrying
find doubles, and main's #11962 independently taught the
bootstrap-declared-capabilities double to honour the bound.
Re-measured: 251 -> 253 files, 167 -> 168 blind, 54 -> 55 unjudged, wrong
unchanged at 32. Ratchet DOWN applied, three arrivals seated as pre-existing
debt.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

buildExistingByName's UNSCOPED page cap truncates once a name can have more than one row — and a truncated page reads as "absent", which inserts

2 participants

@os-sam@claude