fix(plugin-security): refuse ADR-0068 built-in identity names at both position write doors - #17436
Conversation
… position write doors `sys_position.name` and `sys_user_position.position` were unconstrained, so a tenant could mint a row spelling any framework-reserved built-in identity name (`platform_admin`, `org_owner`, `org_admin`, `org_member`). PR #15948 closed every in-repo reader that turned such a name into authority; it could not stop the row existing, and an out-of-repo reader that reads the name instead of the capability rung reopens the hole with nothing mechanical to catch it. Both declarations now carry an object-level `validations[]` rule whose CEL list literal is GENERATED from `BUILTIN_IDENTITY_NAMES` — the spec constant that declares the identities — so the closed enumeration is imported, never retyped and never widened to an `org_*` pattern. Object-level validations are evaluated by the engine on insert, by-id update and multi-row update, so the data API, the seeders and metadata import are all covered by ONE refusal carrying ONE code (`VALIDATION_FAILED`). `sys_position` exempts the platform's own catalog provenance (`managed_by` of `platform`, or its legacy `system` spelling) because `bootstrapBuiltinRoles` seeds exactly these names; `sys_user_position` takes no exemption at all, since no writer in any package creates an assignment row spelling one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW
…y-name rows `scripts/measure-reserved-identity-name-census.mjs` reports rows that already stand on an ADR-0068 built-in identity name and rewrites none of them, per the maintainer ruling («refuse new writes only. No migration. A read-only census reports existing colliding rows to the maintainer»). Two modes: the default censuses DECLARATIONS in this repository; `--rows FILE` censuses a deployment from a read-only export, separating the platform's own seeded catalog rows from real collisions and refusing an input that never exported a table rather than reading it as zero. The reserved set is parsed out of the spec constant that declares it, with a control that throws instead of reporting a comfortable zero when the parse finds nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW
…s-position-reserved-names
…voked-as, and make the gate double refuse combinators
Two gate findings on this branch's own diff:
- `check:entry-guard` — the census script carried a hand-typed
`import.meta.url === file://${process.argv[1]}` guard, which answers false
through a symlink and silently does nothing. Routed through
`scripts/invoked-as.mjs`'s `isEntrypoint`, like every other `scripts/` entry.
- `check:where-matcher` — the DelegatedAdminGate test double read a `$and` /
`$or` key as a field name instead of refusing it, the silently-wrong shape:
every row would fail the lookup and the assertion would pass for a reason
unrelated to what it measures. It now throws on any combinator it does not
implement, matching the sibling double in `delegated-admin-gate.test.ts`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW
📓 Docs Drift CheckThis PR changes 1 package(s): 14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 10 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e2df5f6bcf5cf9b081497948f862b91907968222 && git checkout e2df5f6bcf5cf9b081497948f862b91907968222
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fd62a66b7c48be167deb74467ca93e6ad3223f38 03b02318646e89e2447a1ba7f94ff608f5a78f0a && git checkout -B drift-repro fd62a66b7c48be167deb74467ca93e6ad3223f38 && git merge --no-ff 03b02318646e89e2447a1ba7f94ff608f5a78f0a
node scripts/docs-audit/affected-docs.mjs --json fd62a66b7c48be167deb74467ca93e6ad3223f38
|
…ation messages `node scripts/check-i18n-bundles.mjs --write`, nothing else in this commit. The two `validations[]` entries added on this branch carry an authored `message`, which the rule validator resolves through i18n at refusal time (`objects.<object>._validations.<rule>.message`), so the package's bundles were behind the schema — `check:i18n` reported `plugins/plugin-security: 7 bundle(s) drifted` on CI, which is the measurement this branch could not take locally until the gate's build prerequisite was cleared. Exactly the gate's designed output: `en` is rewritten from source (it is a copy, not a translation), and merge mode adds the new keys to the translated locales filled with the source text, which still needs translating. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW
Docs-drift advisory (re-run on
|
| count | |
|---|---|
| changed files | 13 |
content/docs/releases/** |
0 ✅ |
any content/docs/** |
0 ✅ |
governed surface (docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md) |
0 ✅ |
⇒ The guardrail is listed, ⛔ not crossed. The 7 added files are exactly the regenerated bundles and nothing else.
⇒ Advisory answered, no action, and ⛔ this PR is not widened on account of it.
Generated by Claude Code
|
| HEAD | declaredObjects |
--self-test |
|---|---|---|
origin/main (ab56ea3a1) |
298 | exit 0 |
this branch (638d2b544) |
300 | exit 1 |
Mechanism. declaredObjects() in scripts/tenant-audit-census.mjs walks every *.object.ts file and counts each object literal carrying a snake_case name: string literal — it does not distinguish an object declaration from a nested one. The two validations[] rules this PR adds are counted as declared objects:
reserved_identity_name -> packages/plugins/plugin-security/src/objects/sys-position.object.ts
reserved_identity_position -> packages/plugins/plugin-security/src/objects/sys-user-position.object.ts
298 + 2 = 300. ⛔ Not the i18n bundles: the count was already 300 before those were regenerated, and the walk reads *.object.ts only.
The over-match itself predates this PR — the same rule already counts the four actions[] names on sys_position (activate_position, clone_position, …), so 298 was never a count of objects. What this PR did is move the number, and the gate's own design is why that breaks only the self-test: the corpus-scale figures are dated and deliberately not compared (the bare gate is green at 300 — check-tenant-audit-census: OK … 23 prose figures held to the census), while the self-test's mutation builds the string to replace from the LIVE count, so it silently becomes a no-op once the tolerated drift becomes real.
The remedy, measured green and then withdrawn from this branch. node scripts/tenant-audit-census.mjs --write (the script's own documented mode — no checker, fixture or expectation is touched) plus the one prose figure that sits outside the GENERATED block:
content/docs/permissions/tenant-audit-census.mdx— line 87 proseAcross 298 declared objects→300; and the generated block:declared objects in the registry298 → 300,tracked non-test sources scanned557 → 562,engine-shaped types recognised59 → 58,Measured on2026-09-079cefca9a3→ 2026-09-10638d2b544.docs/audits/2026-08-tenant-audit-write-call-sites.counts.md— the same generated block.
With that applied locally, --self-test exits 0 and the bare gate stays 0.
⛔ Not pushed. content/docs/** is domain:devx, outside this card's declared file surface, so it is handed to the PM to route rather than carried here. This comment is the standing-down record for that hand-off.
The rest of the job is clear. Lint & Repo Gates aborts at the first failure, so a red there is a lower bound. With the docs remedy applied locally I re-ran every one of the 182 gate invocations in that job's step list, exit code captured before any pipe: 182 of 182 exit 0. ⇒ this docs count is the only thing between this PR and a green Lint & Repo Gates.
#17437 stays open and is unaffected: it is the card for the checker fragility itself — a self-test whose mutation depends on a figure its own gate declares unenforced will re-break for the next author who adds a snake_case name: to any *.object.ts. Fixing the count clears this PR; it does not close that.
Generated by Claude Code
`node scripts/tenant-audit-census.mjs --write` — the script's own documented mode — plus the one prose figure that mirrors the generated count and sits outside the GENERATED block. ROOT CAUSE, and it is this branch's. `declaredObjects()` in `scripts/tenant-audit-census.mjs` walks every `*.object.ts` and counts EACH object literal carrying a snake_case `name:` string literal; it does not distinguish an object declaration from a nested one. The four `actions[]` names on `sys_position` (`activate_position`, `clone_position`, …) were already in the tally before this branch, so 298 was never a count of objects. The two `validations[]` rules added here are counted the same way, moving it to 300 — and a rule name cannot dodge it, since `packages/spec` requires it to be snake_case. Measured in ONE worktree with ONE `node_modules`, switching only HEAD: at `origin/main` (`ab56ea3a1`) the census reports 298 and `check-tenant-audit-census --self-test` exits 0; at this branch's head it reports 300 and the self-test exits 1. The gate itself is green either way — the corpus-scale figures are dated and explicitly NOT compared. What breaks is the self-test case that rewords the prose claim off the page: it builds the string to replace from the LIVE count, so it silently becomes a no-op once the page's tolerated drift becomes real, and the case then fails for the page rather than for the classifier it pins. That latent fragility is #17437 and is NOT closed by this commit. DISCLOSED: the block regenerates whole, so it also refreshes two figures this diff did not cause — `tracked non-test sources scanned` 557 -> 562 and `engine-shaped types recognised` 59 -> 58 — drift accumulated since the block was last measured at `9cefca9a3`. ⛔ The checker, its fixtures and its expectations are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW
Correction to the comment above — ⛔ this is NOT waiting on another laneThe PM routed the remedy back into this PR, and it is pushed ( Why the surface extension is a distinction, not a convenience — the PM's reasoning, recorded here so the next reader does not re-derive it: What landed, exactly and nothing else:
After the edit: #17437 stays open and gains this mechanism: a self-test whose mutation string is built from a live figure its own gate declares unenforced will re-break for the next author who adds a snake_case Generated by Claude Code |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 34501527873 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
|
Merge-queue build 34501527873 failed — triaged, and it is ⛔ NOT this PR's. Standing down without spending a re-queue, because the root has already left the queue and GitHub has re-queued this PR on a clean base.
What failed, and the two controls that place it
Control 1 — the file surface. This PR's diff, counted mechanically: 0 changed files matching Control 2 — the base branch. On ⇒ Not this PR's, and ⛔ not main-red either. By elimination it is a semantic conflict inside the queue stack — the third case on the triage workflow's own checklist. The root, and why the inherited rows were bystandersThe queue-flake anchor's #17444 is fix(objectql)!: refuse a field whose Full evidence and reasoning posted to the anchors: #17479 (with the complete reading) and #17481 (same build, same shape, pointer). ⛔ Stated there as the place to look, not as a proven cause — confirming it is the root's owner's read. Why no fix was ported and no re-queue was spent
⇒ Watching this build. If it fails again on a base without #17444, that result is this PR's to root-cause and this seat will treat it as such. ⛔ No test will be skipped, disabled or quarantined to reach green, and no empty commit will be pushed to kick CI.
Generated by Claude Code |
Fixes #15972
Clause-②: no
(declaration line written by the
domain:servicesreview seat, not by the implementer. It is added NOW, after the contract review passed andneeds:contract-reviewwas cleared from both carriers: while that label was on the PR it WAS the carrier (scripts/check-changeset-no-major.mjsreads the label asyesand it outranks the body), so a body line would have been redundant then and anowould have contradicted it. Measured on THIS diff: 4 addedexports, all in the newsrc/objects/reserved-identity-names.ts, which the package barrelsrc/index.tsdoes not re-export (explicit named re-exports only, noobjects/line) and the package'sexportsmap does not reach ("."only) ⇒ 0 net public surface; 0 new error codes (the refusal answers the already-registeredVALIDATION_FAILED); and the accept set narrows — the whole change is a refusal.no+ aminorchangeset is a legal pair. ⛔ Do not delete this line.)What this closes
sys_position.nameandsys_user_position.positionwere unconstrained, so a tenant could mint a row spelling any framework-reserved built-in identity name —platform_admin,org_owner,org_admin,org_member. PR #15948 closed every in-repo READER that turned such a name into authority; it could not stop the row existing, and a reader is not an invariant.Both declarations already SAID so, in prose.
sys-position.object.ts: "Framework-reserved built-in identities (platform_admin / org_) … MUST NOT be repurposed by a tenant"*.resolve-authz-context.ts, from the other side: "Read the RUNG — neverpositions.includes(...); an ADR-0057 D4sys_user_positionrow may spell that very name." Both were comments. This PR is the enforcement of sentences that were already in the tree.Ruling implemented
Maintainer ruling via director seat, summon #20, decision batch #105 item 4, 2026-09-09 (comment 5595726426); maintainer reply verbatim: 「16934 关闭 ;其他同意」 = item 4 = A.
validations[]on both objects —objectql's rule validator runs them on insert, by-id update and multi-row updateorg_*by patternRESERVED_IDENTITY_NAMESisBUILTIN_IDENTITY_NAMES; the CEL list literal is GENERATED from itscripts/measure-reserved-identity-name-census.mjs; nothing here rewrites a rowThe second door, and why it grew no refusal of its own
The card's exposure is the ASSIGNMENT row, not the definition:
sys_user_position.positionis free text (it referencessys_position.nameby convention, not by lookup), the platform seeds aplatform_admincatalog row in every organization, and a delegated administrator reaches the write —assertAssignmentWritejudges a position by the permission sets it DISTRIBUTES, and that seeded position distributes none, soboundSets.every(…)approves it vacuously. Re-measured on this branch, and still true (the gate APPROVES the assignmentin the suite).So the refusal has to cover that door. It does — at the object layer, on
sys_user_positiontoo. The delegated-admin gate is a hook on the same engine write, so an assignment that clears the gate still meets the refusal, and both doors answer with one error code.⛔ A second refusal inside the gate was deliberately NOT added. It would carry that gate's own code (
PERMISSION_DENIED) for a condition the object layer already names (VALIDATION_FAILED), and which of the two a caller saw would depend on hook order — two vocabularies for one condition, i.e. the "second copy" the ruling refuses. The predicate is shared; the refusal is single. This is the one place the implementation reads the ruling's wording rather than following it literally, so it is flagged for review.Error code — no new code, and no
packages/speceditAcceptance (a) contemplates a new
ERROR_CODE_LEDGERrow. None is taken: the refusal is authored as METADATA, so it carriesVALIDATION_FAILED— objectql'sValidationError.code, already ledger-registered under the door that serves it, and already what every other object-level validation on this platform answers. There is no new stamp site inplugin-security, socheck:error-code-provenanceneeds nothing. A dedicated code remains available as a follow-up if the maintainer wants the condition named on the wire; it would be apackages/specchange and route to that seat.Two doors, two shapes
sys_positionexempts the platform's own catalog provenance (managed_byofplatform, or its legacysystemspelling — the pairSYSTEM_ROW_PROVENANCEalso maps to "the platform").bootstrapBuiltinRolesseeds exactly these four names per organization on purpose. Apackage- or tenant-authored row is refused; a tenant cannot reach the exemption by claiming it, sincemanaged_byisreadonlyand the admin-door provenance gate refuses a payload spellingplatform/packageoutright.sys_user_positiontakes no exemption. No writer in any package creates an assignment row spelling a built-in identity name —platform_adminstanding comes from the unscopedadmin_full_accessgrant, theorg_*trio fromsys_member.role.验收备注
Acceptance, verbatim from the ruling, and where each half is met:
(a) a
sys_positionwrite spelling any ADR-0068 built-in identity name is refused at the object layer and at the service door with one error code, registered inERROR_CODE_LEDGERif new.Met. Both doors refuse with
VALIDATION_FAILED; no new code, so no ledger row (see above). Pinned per name, on a real engine over a real SQL driver, for insert AND update, at both doors.(b) negative control — every other name still writes.
Met, and it is asserted with the shapes a pattern-based guard would have swallowed:
sales_manager,org_manager,platform_admin_deputy,hr_specialistall still write. In both ablation legs below the negative controls stayed GREEN while the refusals went red.(c) the census lists existing colliding rows and modifies none.
Met.
scripts/measure-reserved-identity-name-census.mjsopens no connection and takes no credentials: the default mode censuses DECLARATIONS in this repository,--rows FILEcensuses a deployment from a read-only export. It separates the platform's own seeded catalog rows from real collisions, and refuses an input that never exported a table rather than reading it as zero. Its reserved set is parsed out of the declaring spec constant, with a control that throws instead of reporting a comfortable zero.(d) zero reader changes (readers are #15948's, done).
Met — the diff touches no reader. Two object declarations, one new predicate module, one test, one census script, one changeset.
Fleet census of name-as-authority readers (the card's first deliverable), objectstack half, with a firing control:
node scripts/measure-reserved-identity-name-census.mjsover 2683 non-test source files returns exactly one candidate —packages/core/src/security/__tests__/resolve-authz-context.batch-equivalence.testkit.ts:262 name: 'org_admin', a test-kit fixture, not a shipped declaration. The FIRING CONTROL is the script's own--self-test: 5 controls, including a synthetic corpus where the scanner must hitplatform_admin/org_adminand must NOT hitorg_manager,platform_admin_x, or alabel:key. ⛔ A zero (or a one) here is a reading over DECLARATIONS in this repository, never over a deployment — the script prints that sentence beside every count. Thecloudhalf is out of reach from this session and remains the seam card the ruling files.docs-drift-checkadvisory — read and answered, no page falsified. The bot lists 6 hand-written pages because they name a symbol this diff touched; the anchors arecross_fieldandmanaged_by, both string literals inside the newvalidations[]entries, so the pages match on the MECHANISM this change uses rather than on anything it changes about that mechanism. Checked against the falsifying shapes: no page enumerates which system objects carry validations, and none states thatsys_position.nameis unconstrained.validation.mdx/objectql/schema.mdx/seed-data.mdxdocument the rule TYPES;authorization.mdx/capabilities.mdx/permission-sets.mdxdocumentmanaged_byprovenance, and the exemption here reuses that vocabulary unchanged. ⛔ No docs edited, and nocontent/docs/releases/**page touched. The bot can only match shared identifiers, so the page that restates this invariant WITHOUT naming these symbols was checked by hand:content/docs/permissions/positions.mdxlists the four names as "Framework-seeded" and documents'org_admin' in current_user.positionsas the RLS/CEL membership test. Neither sentence is falsified — the first gains the enforcement it already implied, and the second is strictly safer now that the array can no longer carry a forged built-in name.Ablation — C3, both negative pins
Mutation:
severity: 'error'→'warning'on one rule at a time. That is the defect this card closes, spelled minimally: the rule stays declared, the metadata still lists it, the predicate still evaluates, and it refuses nothing — onlyerrorblocks the write. Predicted direction: RED. No dist is involved (both objects are imported by RELATIVE path inside their own package, so vitest resolves them tosrc/*.ts).sys-position.object.tsseverity: 'error'1→0; injectedseverity: 'warning'0→1sys_positionrefusal red, negative controls greensys-user-position.object.tsRestoration proved BY STATE, not by exit code, on each leg:
git checkout HEAD -- PATH(never a baregit checkout --), thengit diff HEADempty ANDgit hash-objectequal to the HEAD blob. Final state:git diff HEADEMPTY. The script carriedtrap … EXIT INT TERMwith absolute paths throughout.A census figure this PR moves, and what it does NOT mean
scripts/tenant-audit-census.mjs'sdeclaredObjects()counts every object literal in a*.object.tscarrying a snake_casename:string literal — it does not distinguish an object declaration from a nested one. It already counts the fouractions[]names onsys_position(activate_position,deactivate_position,set_default_position,clone_position), so 298 was never a count of objects. ⛔ This PR does not add two objects; it adds twovalidations[]rules whosenameis snake_case by contract (packages/specrequires/^[a-z_][a-z0-9_]*$/), and the counter tallies them the same way — 298 → 300.Measured in ONE worktree with ONE
node_modules, switching only HEAD: atorigin/main(ab56ea3a1) the census reports 298 andcheck-tenant-audit-census --self-testexits 0; at this branch's head it reports 300 and the self-test exits 1. So the artefacts are regenerated here with the script's own documented--write, per 「碰生成物的 PR 入队前先同步 + 整体重生成」.tracked non-test sources scanned557 → 562 andengine-shaped types recognised59 → 58 — drift accumulated since the block was last measured at9cefca9a3. Onlydeclared objects in the registry298 → 300 is this PR's.The gate itself is green either way: the corpus-scale figures are dated and deliberately not compared. What broke is the self-test case that rewords the prose claim off the page — it builds the string to replace from the LIVE count, so it silently no-ops once the page's tolerated drift becomes real, and then fails for the page rather than for the classifier it pins. ⛔ That latent fragility is #17437 and is not closed here; the checker, its fixtures and its expectations are untouched.
Verification
pnpm --filter @objectstack/plugin-security test— 110 files, 2127 tests, all passing (VERDICT command-exit 0).pnpm --filter @objectstack/plugin-security typecheck—VERDICT command-exit 0.pnpm --filter '@objectstack/plugin-security^...' build—VERDICT command-exit 0.pnpm lint— the WHOLE repo,eslint . --no-inline-config, exit 0. Not narrowed, so no narrowing evidence is owed. Run atce329d7c.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, re-derived after the last commit (unchanged), and reconciled — 79 derived, 79 run, 0 UNRUN. Exit codes captured before any pipe.check:entry-guard(a hand-typedprocess.argv[1]entry guard in the census script) andcheck:where-matcher(the gate test double read a$and/$orkey as a field name instead of refusing it).check:dual-build-cjs-loads,check:i18n,check:type-check-debteach exited 3 = PREREQUISITE NOT MET (they read a fully built workspace). Declared to CI's Build Core job; nothing was measured, and that is not a finding in either direction.638d2b544, before the census artefacts were regenerated):node scripts/check-tenant-audit-census.mjs --self-testexited 1, while its PR-verdict siblingnode scripts/check-tenant-audit-census.mjswas green. Cause: the live census counted 300 declared objects whilecontent/docs/permissions/tenant-audit-census.mdxstill said 298, and the gate deliberately does not compare that number ("value free, sentence required"), so only the self-test's exact-string.replace()noticed.git diff origin/main...HEAD | grep -c '^+.*ObjectSchema.create'= 0), and the underlying counter fragility is ⛔ not fixed here — it is check-tenant-audit-census --self-test exits 1 on main: the page states 298 declared objects, the live census counts 300, and the self-test needs the exact string the main gate deliberately ignores #17437.Scope
No reader touched, no
packages/specedit, none of the five files fenced to PR #17332, and no edit todelegated-admin-gate.ts(the reasoning is under "the second door" above).Generated by Claude Code