Skip to content

fix(plugin-security): resolve the org-admin permission set per organization, and keep the revoke reach wide - #13818

Merged
os-steve merged 2 commits into
mainfrom
claude/issue-11670-scope-org-admin-permission-set-resolver
Aug 31, 2026
Merged

fix(plugin-security): resolve the org-admin permission set per organization, and keep the revoke reach wide#13818
os-steve merged 2 commits into
mainfrom
claude/issue-11670-scope-org-admin-permission-set-resolver

Conversation

@claude

@claude claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Closes #11670

auto-org-admin-grant.ts resolved the sys_permission_set row that every auto-provisioned
org-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/main
at eb717a12a; the issue's :212-229 had rotted to :227-237, and the symbols are what
was located.

Before / after of the resolver

Before — name-only, limit: 1, cached per ObjectQL instance on the NAME alone:

const permissionSetIdCache = new WeakMap(); // ql  ->  Map keyed on NAME  ->  set id

async function resolvePermissionSetId(ql, name, logger) {
  const cached = perQl.get(name);                                            // keyed on NAME
  if (cached) return cached;
  const rows = await tryFind(ql, 'sys_permission_set', { name }, 1, logger); // unscoped, limit 1
  const id = rows[0]?.id;                                                    // first row wins
  ...
  return null;
}

After — threaded with the granting organization, resolved through the catalog's own
spelling, cached on the pair:

async function resolvePermissionSetId(ql, name, organizationId, logger) {
  const key = permissionSetCacheKey(name, organizationId);       // JSON pair, injective
  const cached = perQl.ids.get(key);
  if (cached) return cached;
  const rows = await tryFind(
    ql, 'sys_permission_set', { name },
    organizationId ? 5 : 1,        // a scoped page holds own rows AND organization-less ones
    logger,
    seedCtx(organizationId),       // routes through SqlDriver.applyTenantScope
  );
  const { own, organizationLessResidue } = resolveOwnOrganizationRow(rows, organizationId);
  ...
}

resolveOwnOrganizationRow, seedCtx and catalogIsPerOrganization all come from
per-organization-catalog.ts in the same package, so no dependency edge is added and
there 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, :469 at 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's
existing skipped / permission_set_missing no-op — and warn loudly. No fallback to the
organization-less row.
Rationale, in order:

  1. A fallback keeps the producer producing. 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's reap of the organization-less
    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.
  2. A fallback row is never repaired. Once the organization's own row appears, the
    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 manufactures
    permanent duplicate grants across two set ids.
  3. The state it declines to act in is already broken and already warned about. No own
    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 the sys_organization insert, so the normal path has the
    own row before the first sys_member write 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_member write and by the kernel:ready backfill. A warn names
the organization, the set name, the visible organization-less row id, and the remedy.

⚠️ The availability half that makes this not a loosening. The refusal is one-directional.
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:

  • a demoted admin whose grant predates this repair names the organization-less row; a
    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;
  • the ADR-0105 D4 F2 close-out — a deployment that drops its wall must not leave the
    unbounded organization_admin grant standing — converges across copies written under the
    OTHER posture, which the wall-less resolution cannot see;
  • the backfill's orphan sweep asks the installation-wide question by construction, so a
    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 single carve-out, measured

  • Answer unchanged. Under single nothing is threaded, resolveOwnOrganizationRow
    returns the first row, and the grant still points at the organization-less row even in a
    fixture where an organization copy exists. Pinned.
  • The grant-target read is byte-identical: sys_permission_set, where { name },
    limit: 1, context { isSystem: true }. Pinned as a whole-object equality.
  • No read on any single path carries a tenantId — 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.
  • Read order and objects read are unchanged: 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 single is NOT
byte-identical. The two revoke reads widened — limit: 1 to ORG_ADMIN_SET_COPY_SCAN_LIMIT
and a scalar permission_set_id to { $in: [every copy] } — and the single revoke path
takes one extra unscoped resolve. Keeping them narrow was measured to re-open F2: with the
narrow form, the walled-to-single flip left the walled organization_admin grant standing
beside 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. The
reads stay unscoped — the tenantId pin above covers them.

Ablation transcript

The test imports the module by relative source path, so the pins read src/, not dist/;
the mutation reddening them is itself the proof of that resolution. The dependency closure
was built first regardless (pnpm --filter '@objectstack/plugin-security^...' build, exit
0), 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 TERM
with an absolute REPO_ROOT-anchored path, refused to proceed on an empty HEAD blob, and
proved 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: 1 with seedCtx(undefined), answer back to rows[0]).
Predicted before the run: redden the per-organization pins, leave the single block green.

HEAD blob: 62447b9fcc504f5acc457e6fbe3a142afa32eed4
injected marker count : 4      removed-text count : 0
blob now              : b18c4cb4911cdfafe9f8dfd682cd0cba22a68c73
 auto-org-admin-grant.ts | 8 ++++----   (4 insertions, 4 deletions)
Tests  15 failed | 31 passed (46)

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.

injected marker count      : 8      removed-text count (wide) : 0
blob now                   : 0ff2a7b2efab114015c54f1992f6230e9ee4f8c5
 auto-org-admin-grant.ts | 20 ++++++++------------   (8 insertions, 12 deletions)
Tests  12 failed | 34 passed (46)

The 12 include the F2 flip (revokes the superseded variant when the posture changes), the
one-directional-refusal pin (still REVOKES in that same state), both demotion/removal
cases, the two #4640 revoke-channel pins, the backfill sweep pins, and the DECLARED DEVIATION pin. One red was NOT the property under test and is reported as observed rather
than claimed: warns once per (organization, name) reddened because mutation B routes the
superseded 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 HEAD empty · git hash-object back to 62447b9fcc504f5acc457e6fbe3a142afa32eed4
(the HEAD blob) · zero ABLATION_ markers left · git status clean · and the restored tree
re-measured at Tests 46 passed (46).

Declared controls (green in BOTH directions, ⛔ not ablation evidence): the whole
[#11670] single posture is carved out block under mutation A, and the grant-target pins
(own row, tenant routing, cache key, refusal) under mutation B.

⛔ Boundaries this PR does not cross

  • No repair of EXISTING mis-targeted rows is claimed or performed. This makes NEW
    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.
  • Nothing is deleted or reaped, and the organization-less platform-bucket rows are
    untouched. Revocation removes sys_user_permission_set grant rows only, and only for a
    pair the platform has decided should not hold the capability — which the module already
    did.
  • resolve-authz-context.ts is untouched. Its by-id permission-set read is deliberately
    not 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/objectstack
reports Model tier — no path-derived mandate: the surface hits none of the 3 declared glob(s).
Clause ② is not reachable: no published surface changes (reconcileOrgAdminGrant and
backfillOrgAdminGrants keep their signatures, return shapes and reason values), and
accept/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.tsTests 46 passed (46)
  • pnpm --filter @objectstack/plugin-security testTest Files 92 passed (92), Tests 1708 passed (1708)
  • pnpm --filter @objectstack/plugin-security typecheck — exit 0 (tsc --noEmit plus tsconfig.scripts.json and tsconfig.test.json; --listFiles confirms 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-wide eslint . --no-inline-config) — exit 0, whole repo, no narrowing claimed
  • The 34 path- and kind-derived gate families, re-derived from the actual diff — all exit 0,
    except three that first reported PREREQUISITE NOT MET and were re-run after building the
    workspace package closure: check:i18n (now OK (9 packages — all bundles in sync)),
    check:dual-build-cjs-loads (now exit 0), check:type-check-debt (now
    check-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 a
    red: it needs a saved turbo run test log, which only CI produces. Its own text prescribes
    recording it as NOT MEASURED when the family is run locally.

Generated by Claude Code


Generated by Claude Code

…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
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 31, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security, touching 13 documentable anchor(s).

6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-catalog.mdx (via sys_permission_set (literal, a string literal in resolvePermissionSetId; a string literal in resolvePermissionSetIdsForName))
  • content/docs/data-modeling/objects.mdx (via sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))
  • content/docs/deployment/environment-variables.mdx (via sys_permission_set (literal, a string literal in resolvePermissionSetId; a string literal in resolvePermissionSetIdsForName), sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))
  • content/docs/permissions/authorization.mdx (via sys_permission_set (literal, a string literal in resolvePermissionSetId; a string literal in resolvePermissionSetIdsForName), sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))
  • content/docs/permissions/delegated-administration.mdx (via sys_permission_set (literal, a string literal in resolvePermissionSetId; a string literal in resolvePermissionSetIdsForName), sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))
  • content/docs/permissions/permission-sets.mdx (via sys_permission_set (literal, a string literal in resolvePermissionSetId; a string literal in resolvePermissionSetIdsForName), sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))

7 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))
  • content/docs/releases/v12.mdx (via sys_permission_set (literal, a string literal in resolvePermissionSetId; a string literal in resolvePermissionSetIdsForName))
  • content/docs/releases/v13.mdx (via sys_permission_set (literal, a string literal in resolvePermissionSetId; a string literal in resolvePermissionSetIdsForName), sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))
  • content/docs/releases/v14.mdx (via sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))
  • content/docs/releases/v15.mdx (via sys_permission_set (literal, a string literal in resolvePermissionSetId; a string literal in resolvePermissionSetIdsForName))
  • content/docs/releases/v16.mdx (via sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))
  • content/docs/releases/v17.mdx (via sys_permission_set (literal, a string literal in resolvePermissionSetId; a string literal in resolvePermissionSetIdsForName), sys_user_permission_set (literal, a string literal in reconcileOrgAdminGrant))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 14 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 8c6a7fc0b4559fb8bd303729c3988ef9ebe49c45packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6a85493ea2222d7b8fd0600b8bdd45d7b125a1c8 — the merge of head c2f1944c49aa292d8fc99f7e81a4745400462d6c into base 8c6a7fc0b4559fb8bd303729c3988ef9ebe49c45, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 8c6a7fc0b4559fb8bd303729c3988ef9ebe49c45 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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
@claude

claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: check-system-context-census red, fixed in c2f1944c4. Documentation-counter only — the repair, its pins and the changeset are untouched. Recorded here rather than by editing the body above, so the accepted text stays as reviewed.

Which site was the 22nd, by identity. Enumerating the census's own declaration classifier over the corpus lists 22; the one origin/main (b9be867b3, several merges past my base eb717a12a) does not have is packages/plugins/plugin-security/src/auto-org-admin-grant.ts:148context: { isSystem: true; tenantId?: string } = SYSTEM_CTX. git show origin/main of that file greps zero hits for it, and this branch's diff against current main touches three files, only one of which holds any declaration site.

One correction to the mechanism, since it changes what a reader should look for: seedCtx is a call, and its own return-type declaration in per-organization-catalog.ts:193 predates this branch. The census counts a declaration as the identifier in a PropertySignature / PropertyDeclaration / GetAccessor / EnumMember slot, so what actually arrived is the type literal on the tryFind context parameter that exists to carry seedCtx(organizationId). Same conclusion, different node.

Class of failure. [declared-count] — one number. Not the [anchor-is-not-a-read-site] population class whose text prescribes hand-writing a row, and correctly so: this diff adds zero elevation reads, so the 109 sites, their anchors, and the packages and files totals are unmoved, and that page's per-site rows enumerate reads rather than declarations. --fix was not used.

Written by hand: the enforced row | — parsed as a declaration | moved 21 to 22 (the label is the gate's regex anchor, so only the digit moved), plus a paragraph saying what that number counts — the four distinct fields plus the structural type literals that restate ExecutionContext.isSystem's shape inline — and naming the arrival, so the next one is placeable without re-deriving the census.

⚠️ The citation carries no line number on purpose. Written first as a path:line span, the gate went red a second way ([anchor-is-not-a-read-site], 146 anchors instead of 145): a path:line span is an anchor, this page anchors elevation reads, and a declaration citation would need a NON_READ_ANCHORS ledger row with a needle. Editing a gate script is not a counter fix, and the anchor would have rotted on the next edit to that file for no benefit. Anchor count is back to 145.

Re-derived family count, before vs after. Adding one .mdx moved the derivation from 35 runnable commands (29 path-derived + 8 KIND) to 58 (52 path-derived + 8 KIND) — 23 new families, none dropped, exactly the #13724 trap. All 58 run: 57 exit 0, and scripts/check-test-completeness.mjs is exit 3 = NOT MEASURED (it needs a saved turbo run test log only CI produces). Seven first reported PREREQUISITE NOT MET on the reopened worktree and were re-run green after clearing the closure they name — check:doc-formula-expressions, check:doc-security-posture, spec check:docs, spec check:skill-examples, check:dual-build-cjs-loads, check:i18n, check:type-check-debt. That build dirtied no tracked file.

Also green at c2f1944c4: check-system-context-census (OK — 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read), its --self-test, pnpm --filter @objectstack/plugin-security test (Tests 1708 passed (1708)), and repo-wide pnpm lint.

Generated by Claude Code


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review August 31, 2026 16:02
@os-steve
os-steve enabled auto-merge August 31, 2026 16:03
@os-steve
os-steve added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit 73c8466 Aug 31, 2026
35 checks passed
@os-steve
os-steve deleted the claude/issue-11670-scope-org-admin-permission-set-resolver branch August 31, 2026 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

1 participant