Skip to content

fix(plugin-security): an organization-less permission-set read resolves organization-less rows only - #20584

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-20555-orgless-position-set-fold
Sep 29, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-20555-orgless-position-set-fold

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #20555

Clause-②: no

The measurement: reached

The card's question was measured before any fix, on main at 1c761c0d, which already contains PR #20540 (merged as f6ceddc3). The probe is committed on this branch as bf72f620 and later became the pin. It runs a real ObjectQL over a real SqlDriver (better-sqlite3 :memory:), the real platform object definitions and the real SecurityPlugin, resolved through the security service it registers. Principals are built by buildContextForUser, which runs the same resolveUserAuthzGrants the request path uses.

What it read, stated abstractly:

  • Reached. The principal is a member of one organization only and has no organization active. It resolved permission sets that a different organization had authored under the names of built-in roles it holds as positions. The organization-less by-name read returned the other organization's rows. Their systemPermissions reached the resolved sets, and their object map (view-all and modify-all included) reached getEffectiveObjectPermissions. The capabilities did not reach the core envelope's systemPermissions, because §6b of resolveUserAuthzGrants resolves by id. They arrived only through plugin-security's by-name fold.
  • Control. With the principal's own organization active, the read was scoped and returned none of those rows.
  • A second face of the same read. An organization-less principal holding a global grant resolved another organization's same-named copy of the granted set, not the global row the grant names.

Mechanism assumptions from the dispatch, each measured:

  • A1 holds. resolvePermissionSetsForContextUnmemoized folds positions and permissions into one request. The loader's by-name read carries no tenant when none is active, and the driver's applyTenantScope reads "no tenant" as an unscoped path. resolveOwnOrganizationRow(rows, undefined) then returns whichever row came first.
  • A2 holds, at runtime. After f6ceddc3, a resolution with no organization still lists every current membership's role in positions, plus the audience anchor.
  • A3 holds. reserved-identity-names.ts guards sys_position.name and sys_user_position.position only. The package-owned collision refusal (ADR-0086 D4) is about package ownership. Neither one touches an organization-authored sys_permission_set.name.
  • A4: nothing legitimate is dropped. The pin also holds the global grants an organization-less principal really holds. A global position assignment folded onto a global same-named set still resolves, and so does a global user grant to a global set. Both resolve identically with the principal's own organization active.

The fix: one mechanism

The change lands in packages/plugins/plugin-security/src/security-plugin.ts, in the permission-set loader built in start(). With no active organization, the by-name read now asks for organization-less rows only (organization_id: null). A row scoped to an organization applies only while that organization is active, and an organization-less row applies everywhere. This is the rule resolveUserAuthzGrants already applies to grant rows, and it matches ADR-0123 D2 ("tenant-scoped reads resolve to nothing"). A read with an active organization is unchanged.

  • Why a predicate in the read, not a filter after it. The read keeps its limit. Filtered afterwards, other organizations' copies of a name could fill the page and push out the global row the caller does hold. organization_id: null is the spec's has-no-value predicate. The organizations runtime already reads by it.
  • No reserved-identity guard. The scoped fold alone closes the cross-organization reach, so no second mechanism is added. Inside one organization, a set named after a position still folds for that organization's own members while it is active. That is the governed-fold question, which is already warned about at runtime. It is not this card.
  • Blast radius. Every consumer of resolvePermissionSetsForContext reads through this loader: the data-plane middleware, getEffectiveObjectPermissions, the delegated-admin gate, explain and /me/apps. So the fix reaches all of them from this one place. packages/core/src/security/** is not touched.
  • Public surface. Nothing reachable from the package's exports entry changes: no new export and no new option.

Pins and their ablation

packages/plugins/plugin-security/src/orgless-position-name-fold.test.ts holds 10 cases:

  • a precondition: the principal really holds the colliding names;
  • six negative pins, one fact each;
  • three keep-pins: the global grants, the same grants with the principal's own organization active, and a control that the authored set does resolve for its own organization's member.

Ablation, from the committed state at c3237a7b. The loader's read was put back unscoped through scripts/ablation-replace.mjs (anchor hit 1 time, blob 026ca66a to 69cc688b). A trap restored the file, and the restore was proven against the HEAD blob.

     × the by-name read returns no other organization's row
     × the authored sets are not among the resolved sets
     × their systemPermissions do not reach the principal
     × their object map does not reach the effective map
     × a GLOBAL grant resolves the global row it names, not another organization's same-named copy
     × …and the same-named copy's systemPermissions do not reach that principal
      Tests  6 failed | 4 passed (10)
ABLATION RESTORED: packages/plugins/plugin-security/src/security-plugin.ts blob 026ca66aea1712cfac9a8d9bfa6ddc9b3189b941 == HEAD

The four that stay green are the precondition and the three keep-pins, which is the expected direction. On the first ablation run, the global-grant pin stayed green: the SQL driver returns the page ordered by id, and the global row's id sorted first. The fixture ids were reordered so that the other organization's copy sorts first (c60be715). The pin was re-ablated red after that.

Local verification (all at c3237a7b)

  • pnpm --filter @objectstack/plugin-security exec vitest run: 144 files, 3062 passed and 16 skipped.
  • pnpm --filter @objectstack/plugin-security run typecheck: green. The new test file is in the tsconfig.test.json program, confirmed with --listFiles.
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 63 commands. All 63 were run, and --ran reports 63 run, 0 NOT-MEASURED, 0 UNRUN.
    • check:type-check-debt first answered exit 3 at this head: the ablation restore left the source newer than dist/*.d.ts. It answered 0 after pnpm --filter @objectstack/plugin-security build.
    • Earlier in the round, check:engine-double-contract flagged by-id and write verbs on the pin's observed engine. The loader under test calls find only, so the double now exposes find alone, and the ledger is unchanged.
  • A targeted dogfood subset ran against the rebuilt dist/ (the fix marker was confirmed in dist/index.mjs and dist/index.js): sharing-rule-org-less-caller, me-apps-and-everyone-baseline, two-doors-permission, showcase-permission-zoo and showcase-crud-persona-matrix. 5 files, 87 tests passed.
  • Lint, narrowed and stated as such. eslint --no-inline-config --format json over the two touched TypeScript files reports 2 files, 0 errors and 0 warnings. eslint.config.mjs enables no type-aware linting: every parserOptions carries only ecmaVersion and sourceType, and there is no parserOptions.project. So this diff cannot move a verdict on any untouched file. The full pnpm lint run is CI's.

Acceptance notes

  • Boundary, not a regression. A principal with no organization active no longer resolves a permission-set row that is stamped with an organization. Could a real principal have relied on that? It would need a global grant pointing at an organization-stamped set, or a global position named like one. No producer in this repository writes either. bootstrapPlatformAdmin points its global grant at an organization-less row, and Setup stamps both the grant and the set with the active organization. The remedy, stated in the changeset, is to make the organization active or to grant the set globally.
  • Seeders are untouched. resolveOwnOrganizationRow(rows, undefined) still returns the first row for the seeders' own single-posture pass. The fix changes the enforcement read only.
  • A docblock to recheck. The docblock on callerOrganizationId says a single-posture caller carries no organization. A single-posture session that has an active organization does carry one into ctx.tenantId, so that sentence is worth rechecking when the file is next touched. This PR does not change that behaviour.

Generated by Claude Code

…sition-name fold

Records, over a real ObjectQL + SqlDriver, which sys_permission_set rows the
permission-set loader returns for a principal with no active organization,
and whether their capabilities reach the resolved sets and the effective
object map. Readings only; converted into the pin once the fix lands.

Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H
Co-authored-by: Claude <noreply@anthropic.com>
…es organization-less rows only

With no active organization the permission-set loader read sys_permission_set
by name with no tenant, which the driver treats as an unscoped path: every
organization's row of each requested name came back and the first one won.
The requested names include the caller's positions, which with no active
organization still carry every membership's role and the everyone anchor.

The read now asks for organization-less rows only when no organization is
active, the rule the grants resolver already applies to grant rows. Scoped
reads are unchanged. The measurement probe becomes the pin.

Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H
Co-authored-by: Claude <noreply@anthropic.com>
…grant pin can fail

The SQL driver returns the by-name read ordered by id, and the global row's
id sorted first, so the global-grant pin stayed green against the unscoped
read. The other organization's copy now sorts first.

Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H
Co-authored-by: Claude <noreply@anthropic.com>
The loader under test calls find alone, and the plugin's kernel:ready
bootstraps are never fired, so no by-id or write verb is needed.

Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

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): node scripts/docs-audit/affected-docs.mjs --json c876a7426d930e9e8d310fdffb1f64ae836cdced → packageMentionDocs.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: c3237a7bfe42a1a9bfcb91eee5f2df1da297a070
Local-runs: none

Inputs read: card #20555 (body and all three comments — triage 5882737881, claim 5883430106, os-dev-report 5884201563), PR #20584 (body, file list, net diff against main; the branch base is 1c761c0d, main has since moved to c876a742), and the check-runs on the head. Nothing was built, run or re-run locally.

Check-runs as read: 26 success (Check Changeset, Governed Surface Queue Guard, Build Core, Dogfood Regression Gate 1/3–3/3, Dogfood Verify CLI, Temporal Conformance, Type Check source / debt ledger / workspace / consumer gates, Test Core 2/6, 3/6 and 6/6, and the PR-shape checks), 3 skipped (Build Docs, Console Pin Gate, Packed-tarball smoke), 4 still in progress (Lint & Repo Gates, Test Core 1/6, 4/6, 5/6), 0 failure. The seat checks convergence before landing; this record judges ①②③.

① Derived judgments

The diff is three files: packages/plugins/plugin-security/src/security-plugin.ts (+32/−2 — one predicate in the permission-set loader built in start(), plus its comment), a new pin packages/plugins/plugin-security/src/orgless-position-name-fold.test.ts (+314), and .changeset/20555-orgless-permission-set-read.md (+12). No governed surface, no packages/spec, no packages/core/src/security: the lane boundary the claim drew holds.

  1. Organization-less by-name read narrowed to organization-less rows — RIGHT. With callerOrganizationId(context) undefined the where becomes { name: { $in: names }, organization_id: null }. This is the rule core already applies to every grant row it reads (grantAppliesInTenant in packages/core/src/security/resolve-authz-context.ts: no organization ⇒ global, applies everywhere; organization-scoped ⇒ applies only while that organization is the active tenant; no active tenant ⇒ does not apply), and ADR-0123 D2 ("tenant-scoped reads resolve to nothing"). The plugin's by-name fold was the one read left on the resolution path that still answered "no organization" as "every organization". Read at source, the defect needed no grant row at all: the requested names include §3's sys_member role projection and the everyone anchor, which every organization's catalog copies by name, so any organization's copy could answer for an organization-less caller.
  2. Predicate spelling and placement — RIGHT. { field: null } is the spec's has-no-value predicate: driver-sql compiles it to IS NULL (its own docblock calls it already total), driver-memory matches null-valued and key-absent rows, and the repo already reads by it (metadata-protocol/src/protocol.ts, organizations/src/claim-orphan-org-rows.ts). Putting the predicate in the read rather than filtering after it keeps the limit honest: filtered afterwards, other organizations' copies of a name could fill the page and crowd out the global row the caller does hold. The dev's own ablation exposed that ordering sensitivity (the global-grant pin stayed green until the fixture ids were reordered), which is the measurement that justifies the placement.
  3. Active-organization read unchanged — RIGHT. With a tenant the where is byte-identical; applyTenantScope's (= tenant OR IS NULL) arm still returns the organization's rows and the organization-less ones, and resolveOwnOrganizationRow still prefers the organization's own row over a leftover. The fix(security,sharing): materialize the RBAC catalog per organization #11121 preference order is preserved.
  4. No reserved-identity guard on sys_permission_set.name — RIGHT. Triage set the direction: scoped fold first, a guard only if the fold alone cannot close it, no two mechanisms. The scoped fold closes the cross-organization reach alone — with the read scoped, no other organization's row is on the page for any name. The within-organization fold of a position name onto a same-named set remains the Permission-set resolution binds by POSITION NAME while sys_position_permission_set sits near-empty — confirm name-based resolution is the intended mechanism, not an accident the empty junction hides #13419 governed-fold warning, which is not this card.
  5. Blast radius as claimed — RIGHT. dbLoaderFor has one consumer path, dbLoaderForContext(context), called from resolvePermissionSetsForContextUnmemoized (primary and fallback resolution) and resolveFallbackPermissionSets; the delegated-admin gate, getEffectiveObjectPermissions, explain and /me/apps all resolve through resolvePermissionSetsForContext. Seeders are untouched: resolveOwnOrganizationRow(rows, undefined) still returns the first row for the single-posture pass, and the seeder reads in security-plugin.ts still go through seedCtx(organizationId).
  6. Nothing legitimate dropped (A4) — RIGHT, as far as this repository's producers go. The grant producers write global grants only at organization-less rows: bootstrapPlatformAdmin grants admin_full_access with organization_id: null at the organization-less row; auth-manager's self-registration grant picks the organization's row when an organization is present (and stamps the grant), else the organization-less row; the position-to-set junction seeders stamp through seedCtx. The keep-pins hold a global position folded onto a global same-named set and a global user grant to a global set, both organization-less and with the home organization active.
  7. Public surface — unchanged, RIGHT. No export, option, signature or spec key changes; the edit is inside SecurityPlugin.start(). Nothing reachable from the package exports moves.
  8. Pin and ablation — RIGHT. Ten cases: one precondition (the principal really holds the colliding names), six single-fact negative pins, three keep-pins including a control that the authoring organization's own member does resolve the set. The reported ablation of the predicate at this head (restore proven against the HEAD blob) reds exactly the six negative pins and leaves the precondition and keep-pins green — the expected direction, each pin standing on its own fact.

② Semver level

@objectstack/plugin-security: patch, Clause-②: no, no direction arm — in the changeset body and in the PR body, bare at line start. RIGHT. A bug fix in a released package takes patch, never skip-changeset; the Check Changeset check-run on this head is green. The accept set — what an author can write and store (sys_permission_set rows, grants, positions) — is unchanged, and the public surface is unchanged. The changeset carries the remedy sentence an upgrading consumer greps ("make the organization active, or grant the permission set globally"), so a migration prescription is present even though this level does not require one.

③ Boundary flags

  1. open_questions — should the Clause-② line carry (narrowing)? Answer: A. Clause-②: no, patch; the arm is not owed. The arm (scripts/pm/clause2-line.mjs) declares a narrowing of a published accept set — the feat(platform-objects): sys_job.timezone and sys_report_schedule.timezone are validated against the IANA domain #16296 class, where a stored value domain shrank under consumers. Here no accept set or value domain moves. The rule that decides which stored rows apply to an organization-less resolution was already published, as BREAKING, by [finding] resolveUserAuthzGrants keeps EVERY organization-scoped grant when no organization is active, so a member removed from an organization keeps that organization's capabilities (measured: manage_metadata passes on DELETE /packages/:id) #20515's changeset (.changeset/20515-orgless-grants-global-only.md: "Such a principal now holds its global grants and nothing scoped to an organization" — yes (narrowing), minor on @objectstack/core and @objectstack/plugin-security) and by ADR-0123 D2. This diff makes the plugin's by-name read honor that already-published rule; it declares no new rule. The only reach it removes is (a) another organization's set the principal was never granted, and (b) another organization's same-named copy of a globally granted set — while the global row the grant names keeps resolving. Neither is a grant under the published rule, and no in-repository producer writes the one shape that would lose a capability (a global grant or global position resolved by name against an organization-stamped set with no organization-less copy). A BREAKING banner here would record the cross-organization reach as a removed feature. The dev's reading of fix(core,plugin-security)!: a grants resolution with no active organization applies only global grants (#20515) #20540 is verified: its yes came from the new buildContextForUser(..., tenantId?) parameter, and its (narrowing) declared the grant-row rule this PR completes. Should the seat nevertheless read it as B, the gate path is no (narrowing), minor, a **BREAKING** marker and one ADR-0087 disposition marker in the changeset body (the not-required (no-migration-prescription) category [finding] resolveUserAuthzGrants keeps EVERY organization-scoped grant when no organization is active, so a member removed from an organization keeps that organization's capabilities (measured: manage_metadata passes on DELETE /packages/:id) #20515 used fits); the code does not change.
  2. deviations — none on the file surface; agreed. objects/reserved-identity-names.ts is untouched because the scoped fold closes it alone, which was triage's own condition. packages/core/src/security is untouched. origin/main was not merged: main moved six commits past the branch base, none in plugin-security, core security or driver-sql, and the queue leg verifies the merge ref. The report fields beyond the os-dev template are what the dispatch asked for.
  3. deviations — shared state; noted, not a review defect. Fetching pinned fixture 621a4876 at depth 1 into the shared object store (adding a .git/shallow root) and creating then deleting refs/os-issue-20555/main are disclosed; the ref is gone, and the fetch is an additive object-store fact no worktree reads as a ref.
  4. out_of_scope_findings — two, both noted with no carrier; neither blocks. (a) The callerOrganizationId docblock says a single-posture caller carries no organization, while resolveAuthzContext copies a session's activeOrganizationId into ctx.tenantId under every posture — a doc-accuracy item, no behaviour change in this PR. (b) resolveOwnOrganizationRow(rows, undefined) still returns the first row for the seeders' single-posture pass; enforcement no longer depends on it. Escalated to the seat: (a) deserves a docs card or the next touch of the file; (b) is unmeasured and may stay noted.
  5. Regrade to p0 — escalated to the seat. Triage's rule was "reached ⇒ regrade to p0", and the measurement answered REACHED before any fix was written. The dev correctly left the relay to the seat.
  6. Reviewer boundary note — the empty-string-stamped edge; not blocking. driver-sql stores organization_id: '', and { organization_id: null } does not return such a row (measured and recorded in bootstrap-platform-admin.ts's two-reads docblock). For this loader that removes nothing a published rule grants: an ''-stamped sys_permission_set row was already invisible to every active-organization read (the = tenant OR IS NULL arm never matched it) and rowOrganizationId reads '' as an organization id, so it was reachable only through the unscoped organization-less path this PR closes. No in-repository producer writes '' on this object. Consistent with the wall; recorded so the edge is on file.
  7. Labels. documentation, tests, tooling and size/m were added by the labeler, not the dev; they carry no weight in this verdict.
  8. Security-family disclosure. The defect is stated abstractly here and in the PR; no request recipe is written in this record.

Implemented-by: claude/issue-20555-orgless-position-set-fold
Reviewed-by: session_01XY5uCwTjZj7884yYtyur4H

VERDICT: PASS


Generated by Claude Code

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/m tests tooling

Projects

None yet

2 participants