fix(plugin-security): resolve the org-admin permission set per organization, and keep the revoke reach wide - #13818
Conversation
…zation (#11670) The `sys_permission_set` row every auto-provisioned org-admin grant points at was resolved by name alone — no `organization_id` predicate, `limit: 1`, and cached per ObjectQL instance on the name alone. Post-#10103 one name carries a row per organization plus the organization-less platform-bucket row, which is the oldest of them, so a walled deployment could point grants at a row belonging to no organization. The read is now threaded with the granting organization and resolved through `resolveOwnOrganizationRow`, with the cache keyed on `(organization, name)`. `single` is carved out and unchanged. With no own row the resolver refuses loudly rather than falling back to the organization-less row. Revocation is widened in the same change so the narrowing is not a loosening: the superseded, demotion and orphan-sweep legs match every copy of the set name in every posture. The grant target is posture-scoped; the revoke reach is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
📓 Docs Drift CheckThis PR changes 1 package(s): 6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 14 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 6a85493ea2222d7b8fd0600b8bdd45d7b125a1c8 && git checkout 6a85493ea2222d7b8fd0600b8bdd45d7b125a1c8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8c6a7fc0b4559fb8bd303729c3988ef9ebe49c45 c2f1944c49aa292d8fc99f7e81a4745400462d6c && git checkout -B drift-repro 8c6a7fc0b4559fb8bd303729c3988ef9ebe49c45 && git merge --no-ff c2f1944c49aa292d8fc99f7e81a4745400462d6c
node scripts/docs-audit/affected-docs.mjs --json 8c6a7fc0b4559fb8bd303729c3988ef9ebe49c45
|
The repair on this branch adds one DECLARING position for the identifier — the
`context: { isSystem: true; tenantId?: string }` parameter type on the org-admin
reconciler's read wrapper, which exists to carry `seedCtx(organizationId)`. That
moves the enforced `table-declarations` count and nothing else: no elevation
read arrives, so the 109 sites, their anchors, and the package and file totals
are unmoved.
`--fix` does not repair a population change. The count is hand-written, and the
paragraph beside it now says what that enforced row counts — the four distinct
fields plus the structural type literals that restate the shape inline — so the
next arrival is placeable without re-deriving the census. The new site is cited
without a line number on purpose: this page anchors elevation reads, and the
gate refuses an anchor that is not one.
Part of #11670
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
|
Follow-up: Which site was the 22nd, by identity. Enumerating the census's own declaration classifier over the corpus lists 22; the one One correction to the mechanism, since it changes what a reader should look for: Class of failure. Written by hand: the enforced row
Re-derived family count, before vs after. Adding one Also green at Generated by Claude Code Generated by Claude Code |
Closes #11670
auto-org-admin-grant.tsresolved thesys_permission_setrow that every auto-provisionedorg-admin grant points at by NAME alone. Three properties, each defensible on its own,
combined into an answer nobody chose. All measurements below were re-taken on
origin/mainat
eb717a12a; the issue's:212-229had rotted to:227-237, and the symbols are whatwas located.
Before / after of the resolver
Before — name-only,
limit: 1, cached per ObjectQL instance on the NAME alone:After — threaded with the granting organization, resolved through the catalog's own
spelling, cached on the pair:
resolveOwnOrganizationRow,seedCtxandcatalogIsPerOrganizationall come fromper-organization-catalog.tsin the same package, so no dependency edge is added andthere is no second local spelling of "which row is this organization's" — which is the
shape that produced the defect.
The four call sites (
:307,:333,:457,:469at dispatch time) are all rewired.The no-own-row decision: a refusal in the GRANT direction only
Routing through the governed read forces an answer to "what does a walled rig do when the
granting organization has no own row of that name". Chosen: return
null— the module'sexisting
skipped/permission_set_missingno-op — and warn loudly. No fallback to theorganization-less row. Rationale, in order:
platform bucket is gated on this card precisely because a live producer makes its census
unclosable. A resolver that falls back would keep minting new grants at the bucket after
the census counted them.
reconciler looks for a grant carrying THAT id, finds none, and inserts a second one — the
dedup only collapses duplicates sharing a
permission_set_id. So a fallback manufacturespermanent duplicate grants across two set ids.
row means the organization has no catalog at all — no positions, no permission sets, no
sharing rules — which the per-organization seeding warns about and retries on
organization creation and on every boot sweep. The organization-creation middleware
awaits that seeding inside thesys_organizationinsert, so the normal path has theown row before the first
sys_memberwrite arrives.What the caller then does:
{ action: 'skipped', reason: 'permission_set_missing' }—the value it already returned for the boot-ordering case, so no consumer learns a new one —
retried by the next
sys_memberwrite and by thekernel:readybackfill. Awarnnamesthe organization, the set name, the visible organization-less row id, and the remedy.
Revocation does not consult the scoped resolver at all: it matches every copy of the set
name, in every posture. Narrowing the grant target without that would have loosened a
permission boundary in three measured places:
demotion matched only against this organization's own id would not find it, and the
capability the platform just decided to remove would stay in force;
unbounded
organization_admingrant standing — converges across copies written under theOTHER posture, which the wall-less resolution cannot see;
single id would match no per-organization grant at all.
So: the grant target is posture-scoped; the revoke reach never is. Both directions fail
closed.
The
singlecarve-out, measuredsinglenothing is threaded,resolveOwnOrganizationRowreturns the first row, and the grant still points at the organization-less row even in a
fixture where an organization copy exists. Pinned.
sys_permission_set,where { name },limit: 1, context{ isSystem: true }. Pinned as a whole-object equality.singlepath carries atenantId— reconcile and backfill together.This is the leak detector: threading an organization is what routes a read through the
wall, so its absence is the whole property.
sys_permission_set,sys_member,sys_permission_set,sys_user_permission_set,sys_user_permission_set.DECLARED DEVIATION, pinned as such: the query multiset under
singleis NOTbyte-identical. The two revoke reads widened —
limit: 1toORG_ADMIN_SET_COPY_SCAN_LIMITand a scalar
permission_set_idto{ $in: [every copy] }— and thesinglerevoke pathtakes one extra unscoped resolve. Keeping them narrow was measured to re-open F2: with the
narrow form, the walled-to-
singleflip left the walledorganization_admingrant standingbeside the new one (2 rows where the D4 pin requires 1). The deviation is recorded in a test
named
DECLARED DEVIATION, with the pre-diff predicate written beside the new one. Thereads stay unscoped — the
tenantIdpin above covers them.Ablation transcript
The test imports the module by relative source path, so the pins read
src/, notdist/;the mutation reddening them is itself the proof of that resolution. The dependency closure
was built first regardless (
pnpm --filter '@objectstack/plugin-security^...' build, exit0), and the full workspace package closure was built later for the prerequisite gates.
Both ablations ran from the committed repair, carried
trap 'git checkout HEAD -- REL' EXIT INT TERMwith an absolute
REPO_ROOT-anchored path, refused to proceed on an empty HEAD blob, andproved the mutation on disk by marker count AND blob-hash change before measuring.
Ablation A — revert the scoping (cache key back to name-only, read back to unscoped
limit: 1withseedCtx(undefined), answer back torows[0]).Predicted before the run: redden the per-organization pins, leave the
singleblock green.The 15:
grants when membership role is "owner"·grants the full set under isolated·grants the full set under group·revokes the superseded variant when the posture changes·suppression ON: isolated·suppression ON: group·suppression OFF (explicit false)·turning suppression on REVOKES a standing unbounded grant·backfill threads the suppression to every pair AND the orphan sweep·grants against THIS organization's own row, never the organization-less one·routes the catalog read through the tenant scope rather than a local predicate·two organizations in ONE process resolve to DIFFERENT ids (the cache key)·REFUSES to grant, loudly·warns once per (organization, name)·leaves an EXISTING mis-targeted grant exactly as it is.Ablation B — narrow the revoke reach back to the pre-diff scalar, grant-target scoping
left intact. Predicted before the run: redden the revoke pins, leave the grant-target pins
green.
The 12 include the F2 flip (
revokes the superseded variant when the posture changes), theone-directional-refusal pin (
still REVOKES in that same state), both demotion/removalcases, the two #4640 revoke-channel pins, the backfill sweep pins, and the
DECLARED DEVIATIONpin. One red was NOT the property under test and is reported as observed ratherthan claimed:
warns once per (organization, name)reddened because mutation B routes thesuperseded resolution back through the scoped resolver, which emits a SECOND refusal warning
— a diagnostics-count change, not a convergence failure.
Restore, proven by state after each run (not by exit code):
git diff HEADempty ·git hash-objectback to62447b9fcc504f5acc457e6fbe3a142afa32eed4(the HEAD blob) · zero
ABLATION_markers left ·git statusclean · and the restored treere-measured at
Tests 46 passed (46).Declared controls (green in BOTH directions, ⛔ not ablation evidence): the whole
[#11670] single posture is carved outblock under mutation A, and the grant-target pins(own row, tenant routing, cache key, refusal) under mutation B.
⛔ Boundaries this PR does not cross
resolutions correct. A grant already pointing at the organization-less row, held by
someone who still qualifies, is neither re-pointed nor deleted — counting and repairing
those is platform-admin re-anchor L6 (reap): reader census on a walled rig; organization-scope auto-org-admin-grant's resolver; stop minting org-less rows; only then reap #11978 step 1's census. The visible consequence is pinned honestly: a second row
appears beside the old one, both conferring the same capability.
untouched. Revocation removes
sys_user_permission_setgrant rows only, and only for apair the platform has decided should not hold the capability — which the module already
did.
resolve-authz-context.tsis untouched. Its by-id permission-set read is deliberatelynot tenant-scoped, which is exactly why this defect is invisible today; re-deciding that
asymmetry is not this card.
content/docs/releases/untouched; the release-notes input is the changeset.Tier
Judged against the ACTUAL diff, not the dispatch's prediction.
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackreports
Model tier — no path-derived mandate: the surface hits none of the 3 declared glob(s).Clause ② is not reachable: no published surface changes (
reconcileOrgAdminGrantandbackfillOrgAdminGrantskeep their signatures, return shapes andreasonvalues), andaccept/reject is unchanged today. The maintainer security-boundary floor's 2026-08-28
negative boundary fires on a loosening; every behaviour change here narrows a read or
widens a revoke, and the one place a narrowing could have loosened a boundary is closed by
the wide revoke reach, ablated above.
Verification
Union run at
f51e4bc02(the final commit).pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 src/auto-org-admin-grant.test.ts—Tests 46 passed (46)pnpm --filter @objectstack/plugin-security test—Test Files 92 passed (92),Tests 1708 passed (1708)pnpm --filter @objectstack/plugin-security typecheck— exit 0 (tsc --noEmitplustsconfig.scripts.jsonandtsconfig.test.json;--listFilesconfirms both edited files are in the test program, so this is a measurement of them and not a green over unread source)pnpm lint(repo-wideeslint . --no-inline-config) — exit 0, whole repo, no narrowing claimedexcept three that first reported PREREQUISITE NOT MET and were re-run after building the
workspace package closure:
check:i18n(nowOK (9 packages — all bundles in sync)),check:dual-build-cjs-loads(now exit 0),check:type-check-debt(nowcheck-type-check-coverage --re-measure: OK — 29 ledger entries re-measured in 207.5s, 1531 raw tsc errors total, none above its recorded number).node scripts/check-test-completeness.mjs— exit 3, NOT MEASURED, not a pass and not ared: it needs a saved
turbo run testlog, which only CI produces. Its own text prescribesrecording it as NOT MEASURED when the family is run locally.
Generated by Claude Code
Generated by Claude Code