fix(security,sharing): materialize the RBAC catalog per organization - #11121
Conversation
…10103) On a walled posture every principal listed ZERO positions, permission sets and sharing rules while the tables held rows: Layer 0's strict `organization_id = :tenant` AND-composes over the driver's `(organization_id = :tenant OR organization_id IS NULL)`, and the conjunction is the strict equality alone. The rows were all organization-less. The wall is untouched at both layers. The rows get an owner: - the four declared/built-in seeders upsert by `(name, organization_id)` and run one pass per organization under a walled posture, built-ins included; - seeding also fires on organization creation, not only at `kernel:ready`; - `single` posture keeps exactly one organization-less pass. Nothing is reaped — grants point at these rows by id. A per-organization pass that meets pre-fix organization-less rows warns loudly, naming the rows and the remedy, and still creates the organization's own copies. That closes the silent no-op where a tenant-threaded pass saw the old row through the driver's compatibility arm and created nothing. The enforcement-plane reads that only become exposures once copies exist are scoped in the same landing: `resolve-authz-context`'s section 6a name-sweep (packages/core) and plugin-security's permission-set dbLoader, whose `limit` was also a truncation once several organizations hold a row per name. Boot reconciliation is O(changed declarations); steady state rides the organization-creation hook. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
…ducer's update dispatch `check:engine-double-contract` reads the suite's registry-facing seam as an engine double: it declares update() (and previously delete()) alongside engine siblings, and a delegating pass-through wrapping a real engine is exactly the shape that reads as "not a double" and then admits a call the real engine would reject. - update() now opens with `assertEngineUpdateDispatch(data, options)` from @objectstack/metadata-core, the producer's own predicate; - delete() is dropped entirely — none of the three seeders deletes, so the seam no longer makes a contract nothing exercises. The ledger movement is the TIGHTENING one: one row added to `engine-double-contract.pinned.json` recording new pinned coverage, written by the gate's own `--write`. The shrink-only `engine-double-contract.baseline.json` is untouched — no measured exemption was added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
…r-org-catalog-materialization-v2 # Conflicts: # packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts # packages/plugins/plugin-security/src/bootstrap-declared-positions.ts
…s the merge The batched seed existence read (#10946) and the per-organization catalog (#10103) landed independently and each grew a resolution for the same question. Two implementations of one question is the shape that produced the defect this scoping repairs, so `seed-name-lookup.ts` now delegates to `resolveOwnOrganizationRow` and only translates its answer into the module's present/absent tri-state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
📓 Docs Drift CheckThis PR changes 3 package(s): 23 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 8 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 33 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 bbe964e97e6e2962323efaffb1e8b2ee92a63b7c && git checkout bbe964e97e6e2962323efaffb1e8b2ee92a63b7c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d806081ddecc18631c29121b8d196d3a9a9f702d 1452fe3209bb30e18f7e880295199fa9576630f7 && git checkout -B drift-repro d806081ddecc18631c29121b8d196d3a9a9f702d && git merge --no-ff 1452fe3209bb30e18f7e880295199fa9576630f7
node scripts/docs-audit/affected-docs.mjs --json d806081ddecc18631c29121b8d196d3a9a9f702d
|
|
CI is green on ⛔ This PR stays draft on purpose and is NOT waiting on me. The card carries Four things for the reviewer, all in the PR body but easy to lose in its length:
This is a Generated by Claude Code |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32596637081 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Fixes #10103
Implements the maintainer ruling of 2026-08-20 04:12Z (Option C — per-organization materialization, four ruled points) and the 09:17Z ruling resolving Q1/Q2/Q3. Neither is re-litigated here.
target:v17release blocker, routed from cloud#1406. Independently reproduced from an app project (titanwind EHR) in the 08:46Z comment.The defect, restated from the measurement
On a walled posture (
group/isolated) every principal — an organization owner and a platform admin alike — listed zero positions, permission sets and sharing rules while the tables held rows. Nothing could be bound through Setup, and a declaredhierarchy-securitycould never be armed.Every row in those three tables was organization-less. plugin-security's Layer 0 composes a strict
organization_id = :tenant, and the middleware ANDs it into the read AST over the driver's(organization_id = :tenant OR organization_id IS NULL). The conjunction of the two is the strict equality alone, so the driver's null arm was annihilated on every authenticated read.The wall is not changed, at either layer (ruled point 3).
tenant-layer.tsanddriver-sql/src/sql-driver.tsare untouched — they do not appear in the diff. The rows get an owner instead.What landed
The four seeders —
bootstrapDeclaredPositions,bootstrapBuiltinRoles,bootstrapDeclaredPermissions(plugin-security) andbootstrapDeclaredSharingRules(plugin-sharing) — upsert by(name, organization_id)and run one pass per organization under a walled posture, following #8617's landed pattern in this same plugin. The framework built-ins (platform_admin,org_*,everyone,guest) get a per-organization copy, as the ruling explicitly includes:sys_user_positionis already per-organization, and both objects spell their own name indexunique: 'organization'.singleposture keeps exactly one organization-less pass, byte for byte the pre-existing behaviour — the ruled carve-out.Seeding also fires on organization creation (ruled point 2), not only at
kernel:ready. Re-derived: the ruling citedsecurity-plugin.ts:2937-2962for this, and that citation is now stale — on currentmainthose lines are the everyone-anchor baseline binding, thekernel:readyhook sits at 3056 and the first-user-insert re-run at 3074-3084, and the note at 3160 records that the per-organization seed data replay moved to@objectstack/organizations. There was no organization-creation trigger for the RBAC catalog at all. Both plugins now register one, walled postures only, refusing to fall back to an organization-less pass when the new id cannot be read.(a) How the loud guard replaces the reap, and the M3 pin
Q1+Q2 collapsed the reap and the remap on the maintainer's supplied premise: 「新项目还没上线,不需要清理旧数据,也没有老客户升级」and 「titanwind EHR 不是 saas 模式运行的,是本地单库的企业版」. So no reap machinery and no remap/migration machinery ships here.
A reap would also be the wrong instrument on these tables independently of that premise, and the source says so: the rows a reap would delete are grant targets —
sys_user_position,sys_position_permission_set,sys_user_permission_setandsys_record_shareall point at them by row id. #8617's reap could promise "NO grant changes" precisely because it never touched a junction table; here the junctions are the grants.What ships instead is
warnPreFixOrganizationLessRows: under a walled posture, a per-organization pass that meets pre-fix organization-less rows for names it would seed emits one warning naming the rows and the remedy (re-initialize, or adopt by hand by stamping the owning organization), and states that nothing was deleted and why.The load-bearing half is that the pass still creates that organization's own copies. The measured M3 failure was the silent no-op: a tenant-threaded pass sees the pre-fix row through the driver's compatibility arm, reads the name as already represented, takes the update branch, and creates nothing — leaving the deployment as broken as before while reporting success.
resolveOwnOrganizationRowis the single read that tells "this organization has its row" apart from "somebody's organization-less row is visible here", and every seeder routes through it.Pinned by case 3 of
per-organization-catalog.test.ts, which reproduces a pre-fix deployment through the real seeders and then asserts, separately: the per-organization copies exist (everyonenow has anorg_jiarow, anorg_yirow and the untouched organization-less one); the guard fired per organization, namingeveryoneandsales_manager; the message carriesre-initialize,adoptandNOT deleted; and nothing was reaped.Q1's platform-defaults residue (
bootstrapPlatformAdmin's three sets, the ADR-0094 env-door projection) stays outside this card, unreaped, as ruled. The shared projection helpers gained an optionalorganizationIdso the environment door's behaviour is byte-unchanged.(b) The
deal_p1assertion — kept verbatim, retitledRe-derived from the current file rather than the snippet on the card.
packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts:263readsexpect(granted).toEqual(['deal_a1', 'deal_p1']), wheredeal_p1is anos10119_dealbusiness record withorganization_id = NULL.The assertion is unchanged. What changed is its justification, in three places (file header, fixture table, inline comment).
The reasoning: ruled point 4 makes D-invalid the single general rule and demotes D-global to a driver-level compatibility note. Ruled point 3 leaves the wall untouched, so
applyTenantScopestill emits itsOR organization_id IS NULLarm — that arm is the compatibility behaviour the ruling preserves. An org-stamped rule routed through it therefore still matches an organization-less record, and the expected value stays correct.The axis this case actually pins is the third direction its own header names: chokepoint-routing versus a bare-equality reimplementation. That axis is unaffected by which doctrine justifies the arm — the case would be equally load-bearing if the arm existed for no reason at all. So deleting it would cost the suite its most valuable part, and weakening it to a bare-equality expectation would pin the defect. What is now wrong is only reading it as an endorsement of NULL-means-global, and the comments now say so explicitly, in the ruling's own "historical behaviour superseded by this ruling" language.
Verified green as its own run: all 4 cases pass, including this one.
⛔ The ADR-0120 D3 and
sql-driver.tscomment retitles are not in this PR — they are a separate maintainer-merged docs pass, as Q3 ruled.docs/adris untouched.Q3 — the file surface widened, deliberately
Both halves land here, because the cross-organization bleed only exists once per-organization copies exist and no intermediate version may ship with copies present and the sweep global:
packages/core/src/security/resolve-authz-context.ts— the section 6a name-sweep. Re-derived: it is at line 501 now (the prior seat's:497cited theADMIN_FULL_ACCESSderivation, which is at 497 and is untouched — see the fork note below). It sweptsys_positionby name across every organization, and the junction read behind it then collected another organization'severyonebinding. Scoped by threading the organization into the context, not by adding anorganization_idpredicate at the call site: the driver'sapplyTenantScopeis the one governed spelling of this wall, and a bare equality written here would be a second ungoverned implementation of it — the exact shape that produced this card's defect. The limit rises with it so the cap admits this organization's rows alongside any organization-less ones, which stay reachable on purpose since grants point at them.dbLoader— resolved sets by name under a bare{ isSystem: true }withlimit: names.length. Correct while one row existed per name; a truncation the moment copies exist. Now a factory parameterised by the caller's organization, with a bounded but wider limit, preferring the organization's own row per name.(c) Boot reconciliation stays O(changed declarations)
Each pass issues a write only where a declaration actually differs. After the merge this rides #11116's landed mechanism: one batched
$inexistence read per organization for the whole declaration, thenpositionRecordDiffers/recordDiffersFromBodydeciding per row.bootstrapBuiltinRoles, which #11116 did not convert, usesrowMatchesDeclarationfor the same purpose. On the common boot, where nothing declared changed, every organization costs one batched read and zero writes. Steady state does not ride this sweep at all; it rides the organization-creation hook, which seeds exactly the one new organization. Per-request authz resolution stays O(the caller's own organization's catalog) — the 6a sweep was O(organizations) before this change and is not now.Cross-link: #10119 / PR #10422 — the criteria-sweep org scoping is the third leg, and makes per-organization rules cheaper than the unscoped sweep they replace.
Merged with #11116, which landed in the same seeders mid-run
mainadvanced during this work and5337ef162(#11116 / #10946, batch the identity boot seeds' existence read and skip no-op writes) landed in two of the same files, solving the same round-trip concern I had solved per-name. The merge conflicted, and it was resolved by adopting their design and layering the organization scope onto it rather than keeping mine:seed-name-lookup.tsbatches the whole declaration into one$inread and — the part that matters — distinguishes three outcomes,present/absent/unknown, so a read that could not answer is never read as "none of these exist". That is strictly better than the per-name read I had written, and myrowMatchesDeclarationno-op skip is superseded by theirpositionRecordDiffers/recordDiffersFromBody.absentresult asorganizationLessResidueso the caller both creates the copy and reports the leftover. Theirlimit: names.lengthis widened when scoped, for the same truncation reason as thedbLoader.seed-name-lookup.tsdelegates toresolveOwnOrganizationRowand only translates its answer. One question, one implementation — two spellings of it is precisely the shape that produced this card's defect.Their new
bootstrap-seed-round-trips.test.ts(420 lines) passes unchanged alongside the scoping, which is the compatibility signal worth having.Proof
Behavioural, on a real ObjectQL + better-sqlite3
SqlDriver, through the real shipped seeders — the standard the prior seat set. New suitepackages/plugins/plugin-security/src/per-organization-catalog.test.ts, 6 cases: rows stamped per organization; the read that measured ZERO now returns the organization's own catalog and only its own; the M3 guard (above); thesinglecarve-out with the guard silent; a second pass writing nothing; and the cross-organization bleed closed against the realresolveUserAuthzGrantsimported from@objectstack/core, with a positive control in the same case so the negative assertion cannot pass by the resolver having stopped resolving.Ablation, signature predicted in writing before either leg ran, and both legs matched exactly:
resolveOwnOrganizationRowreturns the first row regardless of organization (reintroducing M3)expected [ null ] to deeply equal [ null, 'org_jia', 'org_yi' ], the silent no-op verbatimexpected [ 'jia_wide_grant' ] to not include 'jia_wide_grant'Leg A was re-run after the merge, on the merged tree, and reproduced the same signature — case 3 alone — with the collapse above meaning that one mutation point now covers all four seeders rather than only the built-in one. Restore byte-identical (
b0d3272c…), re-run to a real verdict, 6 passed.Leg B's suite consumes
@objectstack/corethrough the packageexports, i.e. itsdist/, so both legs rebuilt core and proved the artifact:ablation-dist-preflightreported the marker present in 2 built files on the mutated leg and absent from all 12 on the restore. Both restores are byte-identical bygit hash-object(b0d3272c…,abf4f76b…) and were re-run to a real verdict (6 passed).Verification
All through the shared verification lock; exit codes captured before any pipe.
VERDICT command-exit 0.pnpm teston the three affected packages, at the final commit1452fe320: 3 of 3 script names echoed (no silent zero-match), core 887 / sharing 624 / security 1377 — 2888 tests, all passing.@objectstack/corehas notypecheckscript, so a--filterover it exits 0 having run nothing; and both plugintsconfigfiles exclude test files. Explicit programs were run for all three: 0 errors in any file this PR touches (core's 98, plugin-security's 11 and plugin-sharing's 3 are pre-existing, all in test files this PR does not touch, which is why those packages carry ledger entries).check:i18nfirst returnedPREREQUISITE NOT MET— not a pass. The workspace closure was built and it was re-run to a real verdict: OK, exit 0, plugin-security and plugin-sharing both in sync.check:type-check-debt --re-measureon the merged final commit:OK — 33 ledger entries re-measured, 1895 raw errors total, none above its recorded number.(d) Derived gate union, re-derived on the final commit
node scripts/pm/dispatch-gates.mjswith no paths (the script derives its own change set from the merge base). The union was derived three times — after the fix commit, after the pin commit, and again on the merged head — and re-deriving after the second commit ADDED two families:check:entry-guardandcheck:parse-guard, pulled in byscripts/engine-double-contract.pinned.json. Both were run. All 23 at1452fe320, eachEXIT=0:check:entry-guard·check:parse-guard·check:engine-double-contract·check:query-options-erasure·check:where-matcher·check:type-check-coverage·check:authz-resolver·check:changeset-gate-self-tests·check:cross-package-test-inputs·check:kernel-hook-pairs·check:objectui-changeset·check:slot-lookup·check:test-source-alias·check:type-source-resolution·check:i18n·check:nul-bytes·check-adr-0087-registration·check-changeset-no-major·check-ci-filter-parity·check-cross-package-test-inputs·check-empty-changeset·check-plugin-teardown-shape·docs-audit/check-affected-docs— pluscheck:type-check-debt --re-measure,OK.The script reports
check:driver-memory-censusandcheck-test-completeness.mjsas unreachable by construction (dead package literals), i.e. quiet green for every card in the tree — recorded, not claimed as coverage.Ledger movement, stated in the required direction:
check:engine-double-contractlegitimately reddened on my new test's registry-facing seam. Repaired in the code:update()now opens withassertEngineUpdateDispatch(data, options)(the producer's own predicate), anddelete()was dropped since no seeder deletes. The gate then asked for the new pinned coverage to be recorded — one row added toengine-double-contract.pinned.jsonvia its own--write. That is the tightening ledger. The shrink-onlyengine-double-contract.baseline.jsonis untouched; no measured exemption was added.(e) Forks
None taken — and the two that could have fired were checked rather than assumed:
packages/corebe org-scoped without changing thePLATFORM_ADMINderivation, which hangs off an unscoped grant pointing at a specific row id? Re-derived on currentmain: the derivation isif (ps.name === ADMIN_FULL_ACCESS && unscopedUserPsIds.has(ps.id))atresolve-authz-context.ts:497, keyed on thesys_user_permission_setgrant being unscoped. Since nothing is reaped andbootstrapPlatformAdminstays outside this card,admin_full_accesskeeps its organization-less row and its row id. The section 6a scoping is a separate read and does not touch line 497. Fork does not fire.packages/specis untouched. So are the wall files.Clause-② — this PR stays draft
Contract accept/reject change: an organization-less write becomes invalid state under a walled posture.
needs:contract-reviewstays hung on the card. This seat did not clear it, did not mark ready, did not arm auto-merge and did not merge. The ruled tier isCONTRACT_REVIEW_TIER, read live atscripts/pm/dispatch-gates.mjs:2722— note the1932circulating in the 04:17Z claim comment is stale; the constant's value is unchanged.Generated by Claude Code