Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .changeset/seed-name-lookup-unscoped-page-budget.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-security": patch
---

fix(security): stop reading a truncated existence page as "absent" — the unscoped page cap is now measured, not trusted (#11518)

`buildExistingByName` (`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`, which is 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, and an unscoped page of N names can match far more than N rows.

The rows that fall off a full page are the highest `id`s 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; the unscoped arm never got the repair, and two seeders on `main`
read unscoped (`bootstrapDeclaredCapabilities`, `permission-set-projection`'s
env-overlay pass).

⛔ `names.length * 2` would have been the same defect with a larger constant:
rows-per-name is bounded only by the number of organizations, so no constant
multiplier is correct. Instead the cap stopped being a promise and became a
**measurement** — the read asks for one row MORE than it is willing to hold, and
a page that comes back carrying that extra row is a PREFIX of the answer rather
than the answer. It then 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`. Both directions are exact: no complete page is ever mistaken for
a truncated one, and no truncated page for a complete one.

**Behaviour change, stated rather than slipped in.** In the truncating case the
two unscoped seeders go from a **silent wrong answer to a loud slow one**: names
that used to be reported `absent` (and re-inserted, or refused by the unique key
as a collision naming a row nobody ever saw) are now answered correctly, at the
cost of one read per name plus a warning naming the object and the budget it
could not fit inside. 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.

The SCOPED arm keeps #10103's cap exactly (`names.length * 2`), because there the
number is a proven bound rather than a budget: `applyTenantScope` returns this
organization's rows plus organization-less ones, and the declared name index is
unique per organization. It gains the same probe, which turns a scoped page that
overflows that bound — reachable only where the unique index is absent or not yet
created — into the same loud degradation instead of a silent truncation.
Original file line numberDiff line numberDiff line change
Expand Up@@ -36,7 +36,7 @@ function makeQl(declared: any[] = []) {
// of these names exist" — so every provenance case below would silently
// become a first-boot insert while the suite reported green. That is the
// double's limits masquerading as the seeder's behaviour.
return rows.filter((r) =>
const matched = rows.filter((r) =>
Object.entries(where).every(([k, v]) => {
if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`);
if (v && typeof v === 'object' && !Array.isArray(v)) {
Expand All@@ -47,6 +47,16 @@ function makeQl(declared: any[] = []) {
return (v === null ? r[k] == null : r[k] === v);
}),
);
// [#11518] `limit` is HONOURED, and a paged read is ordered by `id`
// ascending (#4363's pagination tie-breaker). Both are properties of the
// shipped drivers, measured for the sibling double in
// `bootstrap-system-capabilities.test.ts`; this one ignored `limit`
// entirely, which made the whole class of page-cap defect INEXPRESSIBLE
// here — including #11518's, whose consequence lands on THIS seeder.
if (q?.limit === undefined) return matched;
return [...matched]
.sort((a, b) => (String(a.id) < String(b.id) ? -1 : String(a.id) > String(b.id) ? 1 : 0))
.slice(0, q.limit);
},
async insert(object: string, data: any) {
if (object !== 'sys_capability') return null;
Expand DownExpand Up@@ -409,3 +419,86 @@ describe('unowned-declaration diagnostic (#4967 Part 3)', () => {
expect(w!.meta?.grantedBy).toEqual(['(unnamed permission set)']);
});
});

/**
* [#11518] THE CONSEQUENCE THIS SEEDER PAYS FOR A TRUNCATED EXISTENCE PAGE.
*
* This is one of the two callers on `main` that read UNSCOPED (the other is
* `permission-set-projection`'s overlay pass), and `seed-name-lookup.ts` capped
* an unscoped page at `names.length` — exact only while one row can exist per
* name. Since #8461 / ADR-0120 D1 `sys_capability.name` is unique PER
* ORGANIZATION and ADR-0066 D1 encourages admins to EXTEND the registry inside
* their own organization, so a healthy install carries a row per organization
* plus the package's. The rows that fall off a full page are the highest `id`s
* (#4363's `ORDER BY id ASC`), so whole names vanish — and a vanished name reads
* as `absent`, which sends THIS loop to its INSERT branch.
*
* That is the whole severity of the card: not a read that under-reports, but a
* read that under-reports and then WRITES. Measured here on the seeder rather
* than argued, which needs the double above to honour `limit` — it did not, and
* that is why nothing in this file could see the defect.
*/
describe('#11518 — a truncated existence page must never route this seeder to INSERT', () => {
const NAMES = Array.from({ length: 8 }, (_, i) => `acme.cap_${i}`);
const PACKAGE_ID = 'com.acme.suite';
const declared = NAMES.map((name, i) => ({
name, label: `Cap ${i}`, description: `Capability ${i}.`, scope: 'org', _packageId: PACKAGE_ID,
}));

/**
* A REBUILD on a healthy install: every declared name already has this
* package's row, and two organizations have extended the first one. Their ids
* sort first, so they take the head of the page and the package's own rows are
* what falls off the end.
*/
function fixture() {
const ql = makeQl(declared);
ql.rows.push(
{ id: 'aaa_org_jia', organization_id: 'org_jia', name: NAMES[0], label: 'Jia copy', description: 'jia', scope: 'org', managed_by: 'admin', active: true },
{ id: 'aab_org_yi', organization_id: 'org_yi', name: NAMES[0], label: 'Yi copy', description: 'yi', scope: 'org', managed_by: 'admin', active: true },
);
NAMES.forEach((name, i) => ql.rows.push({
id: `cap_${i}`, name, label: `Cap ${i}`, description: `Capability ${i}.`, scope: 'org',
managed_by: 'package', package_id: PACKAGE_ID, organization_id: null, active: true,
}));
return ql;
}

it('POSITIVE CONTROL: at the cap that was live before this fix, two declared names have no row on the page', async () => {
// Pins the DOUBLE, so it holds before and after the repair — without it,
// "the seeder wrote nothing" could be green because the trap was never set.
const ql = fixture();
const page: any[] = await ql.find('sys_capability', {
where: { name: { $in: NAMES } },
limit: NAMES.length, // ← the UNSCOPED cap this card repairs
});
const onThePage = new Set(page.map((r) => r.name));
expect(page).toHaveLength(NAMES.length);
expect([...onThePage].sort()).toEqual(NAMES.slice(0, 6).sort());
for (const lost of [NAMES[6], NAMES[7]]) {
expect(onThePage.has(lost), `${lost} fell off a full page`).toBe(false);
expect(ql.rows.some((r: any) => r.name === lost && r.managed_by === 'package')).toBe(true);
}
});

it('re-seeds a healthy multi-organization install WITHOUT writing anything', async () => {
const ql = fixture();
const rowsBefore = ql.rows.length;
const out = await bootstrapDeclaredCapabilities(ql, null);

// ⛔ The load-bearing assertion. Before the repair this was `seeded: 2` and
// two DUPLICATE rows for names whose package rows were sitting in the table
// — every boot, on an install that is simply using ADR-0066 D1.
expect(out.seeded).toBe(0);
expect(ql.rows).toHaveLength(rowsBefore);
// Nothing else was written either: seven of the names resolve to this
// package's own unchanged row…
expect(out.updated).toBe(0);
expect(out.unchanged).toBe(NAMES.length - 1);
// …and the eighth resolves to an organization's authored copy, which this
// seeder never clobbers. Unscoped, the first row by id is the row — the
// pre-#10946 per-item answer, unchanged by this repair.
expect(out.skippedAdmin).toBe(1);
expect(out.unreadable).toBe(0);
});
});
Loading
Loading