Repository navigation
fix(security): the curated capability seeder reconciles the row the platform owns, not whichever row shares the name (#8470) - #8537
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 12 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
…ance instead of asserting ownership
Q1 — measured. The worst case is not engine-reachable; the diagnostic was still wrong and is fixed.Reachability: NO for the state you described, on every path I can find.
Limit of that evidence, stated rather than papered over: this clone is shallow (157 commits, But your reading of the message was right, and that part was a real defectThe warning asserted "a row this pass does not own". The seeder cannot observe that. All it knows is: no row matched Fixed in with ⛔ I did not act on the distinction, only report it. Tolerate / backfill / adopt remain your and the maintainer's call, and the diagnostic now hands whoever makes it the fact it needs. A pin asserts the message does not contain The negative row you asked for, plus the whole matrix
Re-ablated against the new set: 7 failed / 16 passed (23) in that file — the four from before plus these three, Q2 — reachable, and more ordinary than the PR claimedNot a hypothetical deployment shape. From
So a deployment without that runtime is
Verification after the change
Generated by Claude Code Generated by Claude Code |
…ing row visible' from 'row with no managed_by'
…gine-double baseline is not raised
…Object.assign is not an invisible third option (#8553) PR #8537's CI history is an unusually clean natural experiment: the same test file went red -> green on this gate by changing only the SPELLING of a double. A fresh literal declaring the engine verbs was counted; `Object.assign(makeQl(), { … })` over the file's existing double was not, and the count returned to its baseline with the double still there. The mechanism is the two-engine-sibling test. This gate counts DECLARATION SITES, so an override literal — which normally carries only the members it varies — has too few siblings of its own, `consider()` returns, and the construct is accounted for by whatever the base was pinned or baselined as. That is CORRECT while the override leaves the scanned verb alone, and #8537's use was legitimate and documented in the test. What made it a card is that the gate's own message offers exactly two ways out ("pin the new one" / "raise it, do not") while a third, cheaper, invisible route exists. A ratchet does not erode by someone defeating it; it erodes when the cheapest route out is the unseen one. Settled by measurement rather than by preference. `--census` (#9943, previous commit) now reports both `Object.assign` shapes, and on 2026-08-20 the scanned corpus holds: 0 override literals that RESTATE `delete`/`update` <- the hole proper 6 that vary other engine members and inherit the verb <- inheritance, correct So counting by VALUE instead of by site would move zero constructs today, at the price of a resolution step over an empty population — and every construct it might later admit would arrive UNPINNED against a shrink-only, maintainer-only baseline. The defensible act is the other one the card offers: say the rule. Both ratchet remedies now carry it, because an author meets whichever one their file's baseline state produces and a clause on only one of them is a coin flip. Guarded the same way #8435 guarded its authority marker: a detector rather than a string compare, self-tested on both remedies plus a negative fixture that REJECTS the two-option framing this card filed against — so a rewording that dropped the clause fails here instead of passing vacuously. And the day a VERB-shaped override does land, `--census` names it and this becomes a priced act rather than a hypothesis. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
… rule, and derive check-engine-double-contract's own corpus figures (objectstack-ai#10176) * fix(tooling): derive check-engine-double-contract's own corpus figures instead of writing them down (objectstack-ai#9943) Every figure this gate's prose carried about its own corpus was a constant that nothing regenerated: `310` CallExpression members with a callee table summing to it, `93` admitted / `217` rejected, `250 doubles this gate discovers`, `82 PINNED`, and two spellings "measured at ZERO". By 2026-08-19 the DISCOVERED and PINNED figures had drifted roughly 2x and 4x, and the comment carrying them ends `re-run that count before adding a name here` — it is an instruction to a future editor calibrating against numbers that are wrong. Refreshing the constants would reproduce the defect with newer values (objectstack-ai#9803 / PR objectstack-ai#9909), so this removes them and adds `--census`: the gate's own `implOf` / `unwrapCallImpl` / `isEngineVerbShape` walk, printed. objectstack-ai#9943 had to hand-copy the criterion to re-scan and got 341 against the recorded 310 with no way to say which was wrong; that question no longer exists. Two results worth recording, both re-derived rather than assumed: - The REPOSITORY_ONLY_MEMBERS claim SURVIVED the corpus growth. `--census` runs the veto's own ablation (discovery with the veto off, diffed) and on 2026-08-20 the pair still moves EXACTLY 2 constructs, still both of objectstack-ai#5945's `IScopedContext` witnesses under packages/spec, and still 0 pinned doubles. The conclusion held even though every number describing the corpus did not. - The two "measured at ZERO" spellings are STILL zero. That claim held up too. The census population itself is the finding. It was scoped to "every member whose initializer is a CallExpression" — the shape the recogniser already reads — so it could only ever find more of what it already understood, and both of its ZERO readings are CallExpression spellings for that reason. `--census` buckets by initializer KIND first, so a spelling this gate cannot read shows up as its own row rather than as silence. On this commit that row reads `defaulted ?? — read 0/2`, which is objectstack-ai#9877's blind spot, visible for the first time. `--census` reaches no verdict and changes no exit code; `report()` does not call it. Self-tested on both sides of every bucket it splits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt * fix(tooling): read a DEFAULTED mock initializer, which was hiding a live unguarded engine delete double (objectstack-ai#9877) `delete: overrides.delete ?? vi.fn(async (o, arg) => …)` is a BinaryExpression. `fnInitializer` knew a function literal and a mock constructor and nothing else, so `implOf` answered null, `consider()` returned before `isEngineVerbShape` was ever asked, and the construct left the population with no verdict recorded — not pinned, not baselined, not exempt. Absent, which reads to every consumer of this script's output as clean. `packages/rest/src/rest-batch-endpoint.test.ts` was that case, and it is not cosmetic: its fake ObjectQL declares an engine `delete` this suite never drives, which is exactly the shape objectstack-ai#4434 shipped a dead REST route with. Measured on this branch, both directions: widening only, no pin PINNED [delete] and PINNED [update] on that file, exit 1 widening + the pin OK, exit 0; UNRECOGNISED 23 -> 21 ⛔ The shrink-only baseline is NOT touched: it stays at 66 delete / 100 update entries across all three commits. Both newly-visible doubles are PINNED at the source — the fake now opens with `assertEngineUpdateDispatch(data, options)` and `assertEngineDeleteDispatch(options)` — so the ratchet tightens (329 -> 331 pinned, ledger 318 -> 320 rows) rather than being raised. `@objectstack/rest` passes 2134/2134 with the predicates in place. The widening stops at `??` / `||`, and that is a narrowing, not an oversight. `a ?? b` reaches the subject as one arm or the other and BOTH are read; a conditional's arms are selected by a test this gate cannot evaluate, so it is deliberately left out (`--census` reports it, ZERO on 2026-08-20). The remaining 21 UNRECOGNISED constructs are the local-binding and shorthand spellings, which need `localFunctions`-style resolution and are reported, not silently widened. The census self-test that pinned this spelling as UNRECOGNISED is retargeted rather than deleted: the same source is now asserted absent from the census AND present in the population, so the two walks are proved to agree. Its old limb keeps a witness (`traced('DELETE', fn)`) or the branch would rot unreachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt * fix(tooling): name the inheritance rule in both ratchet remedies, so Object.assign is not an invisible third option (objectstack-ai#8553) PR objectstack-ai#8537's CI history is an unusually clean natural experiment: the same test file went red -> green on this gate by changing only the SPELLING of a double. A fresh literal declaring the engine verbs was counted; `Object.assign(makeQl(), { … })` over the file's existing double was not, and the count returned to its baseline with the double still there. The mechanism is the two-engine-sibling test. This gate counts DECLARATION SITES, so an override literal — which normally carries only the members it varies — has too few siblings of its own, `consider()` returns, and the construct is accounted for by whatever the base was pinned or baselined as. That is CORRECT while the override leaves the scanned verb alone, and objectstack-ai#8537's use was legitimate and documented in the test. What made it a card is that the gate's own message offers exactly two ways out ("pin the new one" / "raise it, do not") while a third, cheaper, invisible route exists. A ratchet does not erode by someone defeating it; it erodes when the cheapest route out is the unseen one. Settled by measurement rather than by preference. `--census` (objectstack-ai#9943, previous commit) now reports both `Object.assign` shapes, and on 2026-08-20 the scanned corpus holds: 0 override literals that RESTATE `delete`/`update` <- the hole proper 6 that vary other engine members and inherit the verb <- inheritance, correct So counting by VALUE instead of by site would move zero constructs today, at the price of a resolution step over an empty population — and every construct it might later admit would arrive UNPINNED against a shrink-only, maintainer-only baseline. The defensible act is the other one the card offers: say the rule. Both ratchet remedies now carry it, because an author meets whichever one their file's baseline state produces and a clause on only one of them is a coin flip. Guarded the same way objectstack-ai#8435 guarded its authority marker: a detector rather than a string compare, self-tested on both remedies plus a negative fixture that REJECTS the two-option framing this card filed against — so a rewording that dropped the clause fails here instead of passing vacuously. And the day a VERB-shaped override does land, `--census` names it and this becomes a priced act rather than a hypothesis. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #8470
The curated half of
bootstrapSystemCapabilitiesnow looks up the row the platform owns instead of the first row that happens to share the name.The DERIVED half is untouched (its own #5876 guard already refuses to write to a row it does not own).
The card's mechanism was right about the defect and wrong about the cause — and the difference selects the route
The card offered two directions: make the lookup deterministic, or scope it. I took scoping, and the measurement that selected it also falsifies the other option outright rather than merely ranking it second.
The lookup was already deterministic. #4363 shipped a pagination tie-breaker:
orderKeysForappendsORDER BY idto any paged read of a driver-managed table, andlimit: 1counts as paged on bothSqlDriverandMongoDBDriver— onlyfindOneopts out, viasingleRowLookup. Sofind('sys_capability', { where: { name }, limit: 1 })has carried anORDER BY id ASCthis whole time.Measured on
SqlDriver+ better-sqlite3, against a fixture copyingsys-capability.object.tsincluding itsunique: 'organization'index, with a platform row (NULL organization) and an org row sharingmanage_users:cap_orgcap_orgcap_orgcap_orgaaa_orgaaa_orgzzz_orgaaa_platformThe selection tracks
idascending and ignores insertion order entirely. I predicted rows 1-2 would differ (insertion order) and was wrong; chasing that miss is what surfaced #4363.Two consequences:
orderBywould change which wrong row is chosen, not that a wrong row can be chosen. Determinism was never the missing property — ownership was.The card's headline harm —
platformRowExists: false— is real and unaffected by any of this. With no platform row there is only one candidate, so ordering never mattered for that case.Why both predicates, and why
limit: 1is now safe with no ORDER BYmanaged_by: 'platform'ANDorganization_id: nullare the two facts that jointly define the platform's own row, and together they make the result set a provable singleton: the post-#8461 unique key is(COALESCE(organization_id, …), name), so the NULL-organization bucket admits at most one row per name.limit: 1over a set of size 0 or 1 cannot be arbitrary. That is what retires the ordering question instead of answering it.Neither predicate alone is enough.
managed_byalone loses the singleton guarantee if a platform-marked row ever sits inside an organization (seed data, legacy import).organization_idalone cannot tell the platform's row from an admin's on a single-organization deployment, where both live in the same bucket.Measured, same harness:
{ name, managed_by: 'platform', organization_id: null }returns the platform row under both physical orders; returns zero rows when only an org row exists;{ organization_id: null }does not match an org-scoped row (it really compiles toIS NULL); and the platform's global insert is not blocked by an org row of the same name.⛔ The installation-wide unique index is not restored and must not be — it is #8323's cross-tenant existence oracle, closed by #8461.
The platform-bucket provenance matrix
The predicate is a conjunction, so it can also match zero rows where it should match one. One row in the platform's own bucket, varying only by
managed_by. Predicted before running; all four matched.managed_byplatformblockedCurated: 0, no warnadminmanaged_by='admin'packagemanaged_by='package'adminis not a hypothetical:organization_idauto-stamping lives in the enterprise@objectstack/organizationsruntime, which is also what activates every walled posture. A deployment without it issingleposture with no stamper, so every Setup-authored capability row lands in the NULL-organization bucket. That is the default community shape.packagearises by promotion:bootstrapDeclaredCapabilitiesrefuses a name already inPLATFORM_CAPABILITY_NAMES, but a package that declared a name before the platform curated it left amanaged_by:'package'row there — andsetup.writeandmanage_sharingwere both added to the curated set after the fact.managed_byisrequired: truewithdefaultValue: 'admin', andapplyFieldDefaultsresolves defaults on insert before the beforeInsert hooks, so an insert omitting it stores'admin'. Pinned anyway, because the diagnostic must not assert what it cannot observe.The open question the card asked: measured, and the filer's expectation holds
No authorization decision is affected. Method, on
origin/mainat30f1b7488d:requiredPermissionsAND-gate compares string sets:PermissionEvaluator.getSystemPermissions()unionspermissionSets[].systemPermissions, andnormalizeRequiredPermissionsreads the resource's declared strings. No row is loaded.validateCapabilityReferences— the candidate the card named — resolves againstPLATFORM_CAPABILITY_NAMES(the spec constant), stack declarations and seed-data records. It never reads the table, so a curated name with no row does not warn.find/findOne/count/aggregateagainst'sys_capability', plus every consumer of itsplatform-object-namesentry) finds exactly two production call sites — the two seeders. Neither is on an authorization path.So the harm is a registry/Setup-listing defect, as the filer expected. The severity and release targeting the card was given still stand; nothing here needed to widen.
That sweep turned up something the card did not ask about, filed separately as #8535:
sys_capability.activeis read by nothing, while the Deactivate action's confirmation dialog tells the admin that grants and requirements stop resolving. Not addressed here; that issue remains open.Anti-vacuity: ablation, prediction first
Prediction written down before each run (fix committed first, so the restore came out of a real commit). Ablation = revert only the lookup to
{ name }, keep the pins and the upgraded double.managed_by=platformmanaged_by=admin/package/ absent7 failed | 16 passed (23)— the predicted seven, no others. Prediction and measurement did not diverge, including the deliberately-green rows, which are regression controls rather than gates.One sub-prediction inside a red pin is worth naming because it is where the information is: in the collision cases the
seeded === KNOWN - 1assertion passes coincidentally under the ablation (the old code also fails to insert that row — it overwrites the other author's instead). The assertion carrying the information there isblockedCurated.The three-way diagnostic split was ablated separately (two-way → three-way): 1 failed | 23 passed, exactly the new pin.
How the adverse ordering was forced
Per the card's warning, the old double returned rows in insertion order, which is a property of the double and of no driver in the system. The double now models what was measured instead:
limitsorts byidascending (the 分页读取在没有 orderBy 时同样不确定:tie-breaker 只覆盖了「排了序的翻页」 #4363 tie-breaker, BINARY collation as SQLite compares);nullcomparand matches null-or-absent (IS NULLin SQL,value == conditionindriver-memory, null-or-missing in Mongo) — strict===matched none of them and would have madeorganization_id: nullunsatisfiable in the double while working in production;insertenforces(COALESCE(organization_id, '__global__'), name).The adverse case is then forced by choosing ids, exactly as a real driver would order them: the org row gets
aaa_org_authored, which sorts before every seeder-mintedcap_…id.zzz_org_authoredis the same fixture benign. The pair is the anti-vacuity argument: the old behaviour differs between the two ids, the fixed behaviour is identical.Each of the three double changes was measured against a real driver first; none was made to get a test to pass.
The case the fix newly declines, and does not do silently
A curated name can be held in the platform bucket by a row the scoped lookup does not match. Previously the seeder "resolved" that by overwriting the other author. Now it declines — but
tryInsertswallows the engine's unique-constraint refusal, so declining would look exactly like a clean boot with a curated capability missing installation-wide. The result gainsblockedCurated, counted and warned.The warning states what it observed rather than asserting authorship, and splits three ways, because "no row came back" and "a row came back carrying no
managed_by" are different observations and the first one would otherwise be described as an ordinary unstamped row:managed_by→ says that;⛔ It reports the distinction and deliberately does not act on it. Adopting a differently-stamped row into the platform's identity would reverse #5876's "not provably ours resolves to leave-it-alone", and backfilling a stamp is a data migration. Both are maintainer calls; neither is made here.
Scope
bootstrap-declared-capabilities.test.ts) needed the same null-comparand fix, since several of its cases callbootstrapSystemCapabilities.sys_capabilityunique index asserted "bootstrapSystemCapabilities' upsert-by-name is unaffected". That claim is what this card falsifies, so it is corrected next to the index that unmasked it.packages/metadata-protocol/src/protocol.tsis not touched (seed-loaderinterpolates driver text intoerrors[].message, which rides onseedAppliedof a 200 publish response #8442 holds a region there);sys-position.object.tsanddriver-sql/src/schema-drift.tsare not touched (sys_position.nameis the third instance of the #8323 class: an admin-authored name on a tenant-scoped RBAC object carries an installation-wide unique index #8468 holds those).bootstrapSystemCapabilitiesalso leaves the platform bucket unseeded when an organization holds the name — the #5876 guard skips instead of inserting #8536 and remains open.check:engine-double-contracthas nothing new to pin or ledger — its counts are byte-identical tomain's.Verification — re-run at the final head,
1abef9fd27An earlier round of this PR reported a green union that was run before the last two commits existed, and CI went red on
5a877ffefor a gate that had genuinely passed earlier. Everything below is from a single run at the final commit. That failure mode is filed as #8550 and remains open.pnpm --filter @objectstack/plugin-security test— 54 files, 1072 passed;typecheckEXIT=0pnpm check:type-check-debt(the package script —--self-testand--re-measure, perdispatch-gates.mjsnamescheck:type-check-coveragefor a rationale onlycheck:type-check-debtsatisfies — the ratchet half never runs locally #8545) — EXIT=0:33 ledger entries re-measured in 312.7s, 1969 raw tsc errors, none above its recorded number; surplus: nonecheck:nul-bytes,check:cross-package-test-inputs,check:docs-audit-scope,check:test-source-alias,check:type-source-resolution,check:query-options-erasure,check:type-check-coverage,check:engine-double-contract,check:empty-changeset,check:i18n,check-changeset-no-major.mjs201 pinned, 133 DEBT, 2 exempt; query-options-erasure67 unswept, none new; type-check coverage64/77, 13 in DEBT, 436 frozenturbo run build --filter='./packages/*' --filter='./packages/*/*') — the [finding] dogfood 消融跑在构建产物 dist 上:不先重建的消融会「绿着通过」,从而认证一个空洞的测试(三次独立实测) #8246 dist-resolution trap bit twice (once on driver-sql, once as--re-measurerefusing to run at all), and numbers taken before each rebuild were discarded.Changeset:
minor(widened return type; breaking-for-constructors recorded in the body, per the launch-window convention).Generated by Claude Code