Skip to content

fix(plugin-security)!: a position assignment or permission-set grant scoped to an organization must name a member of it - #22275

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-22226-position-holder-membership
Oct 8, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-22226-position-holder-membership

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #22226

Clause-②: no (narrowing)

Executes the maintainer's ruling A on objectstack-ai/cloud#1765 (6053780796) as #22226 states it: a non-system insert or update of sys_user_position whose user_id has no sys_member row in the row's organization_id is refused, for every caller, platform administrators included, with the row's existing 400 VALIDATION_FAILED envelope (fields[].code: reference_not_found), no new error code; system writes stand down. The sibling junction sys_user_permission_set measured as the same class and takes the same predicate. Not taken: B (narrowing the picker) and C (accepting the row).

What changes

  • packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts (new, beside position-catalog-refusal.ts): registerGrantHolderMembershipRefusal binds a beforeInsert and a beforeUpdate engine hook on sys_user_position and sys_user_permission_set.
    • Insert (one row or a batch): judges the organization the row is STORED with. That is the row's own non-empty organization_id, or else the caller's active organization, which the driver writes into the empty slot after the hooks run. organizationTheInsertStores mirrors that stamp: buildDriverOptions in @objectstack/objectql and injectTenantOnInsert in @objectstack/driver-sql.
    • Update (by id and by predicate): the engine hands each matched row's stored image to the hook as previous. The post-image pair (user_id, organization_id) is judged only when it differs from the stored pair. An edit that leaves both alone, such as end-dating the row of a holder who has since left, is not judged.
    • Membership read: sys_member by (user_id, organization_id) under { isSystem: true, tenantId }, with tenantId set to the row's own organization. It joins the writer's transaction. A placeholder-shaped identifier is compared literally.
    • Stand-downs: every isSystem write; a row stored with no organization (a global grant names no organization to be a member of); a user_id that is not an id (the engine answers required itself); and a composition that registers no sys_member object.
    • Failure: a membership read that throws propagates and the write is refused. A security refusal that cannot read its input does not admit the write.
    • Order: priority 40. That is after the authority/standing guards (10, 20) and the grant-name derivation (30). sys_user_position.position is judged by the catalog refusal middleware, ahead of every hook. So on both tables the value a row names is judged before its holder, and a caller the CRUD check or the delegated-admin gate refuses never sees a membership verdict.
  • security-plugin.ts: one import plus one registration line, placed beside the catalog refusal's wiring. No other region is touched.
  • .changeset/22226-grant-holder-must-be-organization-member.md: minor, declared breaking, ADR-0087 not-required (no-migration-prescription).
  • content/docs/permissions/system-context.mdx: row 23d plus regenerated counts. The hooks read isSystem to stand down, and check:system-context-census requires a row for each such read site.
  • Sibling suites (position-catalog-refusal.test.ts, grant-permission-set-name.test.ts): their harnesses now provision sys_member, and the holders of their organization-scoped grants become members.

Why hooks rather than a branch inside createPositionCatalogRefusal (H1)

H1 is confirmed on 73a0a6bf: createPositionCatalogRefusal is the middleware on sys_user_position, wired once at security-plugin.ts:4386. It stands down on isSystem and answers the 400 envelope. A membership branch there would see only the payload. A predicate update needs every matched row's stored organization, and the engine hands exactly that to a beforeUpdate hook. The sibling table needs the same rule too, so the refusal is a module of its own with one wiring line.

Before / after, per write case

Measured on a real ObjectQL engine over SQLite with the real SecurityPlugin. The posture is isolated unless marked single. The holder belongs to another organization only.

Write Before (73a0a6bf, pins red) After
platform administrator inserts sys_user_position (stamped organization) stored 400 VALIDATION_FAILED, reference_not_found at user_id; row absent
same insert, holder is a member (positive control) stored stored, organization_id = the writer's organization
insert naming the writer's organization explicitly stored refused, same envelope
batch insert, one row a non-member stored refused whole; nothing stored
update by id moving user_id to a non-member landed refused; stored row unchanged
predicate update moving user_id to a non-member landed refused; no matched row changes
update leaving holder and organization alone, on a non-member's stored row landed lands (not judged)
system write of a non-member stored stored (stands down)
caller the CRUD check refuses 403 403, member or not
platform administrator grants sys_user_permission_set (stamped organization) stored refused, same envelope; row absent
update by id of that grant to a non-member landed refused
single, writer with no active organization (row stored with no organization) stored stored (not judged)
single, writer's active organization holds the user stored stored
single, organization-scoped row naming a non-member stored refused

single posture (H2). Under the default auto membership policy, plugin-auth's reconciler binds every created user to the default organization (reconcile-membership.ts, ADR-0093 D7), and the one-time backfill binds users who predate it. The platform administrator is bound as owner by ensureDefaultOrganization. So on a stock single-organization deployment, every organization-scoped row names a member. Users created under invite-only and not yet invited are refused, which is the predicate as ruled.

Tests

At 74f8180344 (merge of origin/main 3513ac7781). Code is byte-identical at head cfaac2bf9c, whose last commit touches only the census page.

  • pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2: 180 files passed, 3796 tests passed, 45 skipped.
  • src/grant-holder-membership-refusal.test.ts: 21 tests. 8 were red on the base before the fix (every negative pin: the write landed). All 21 are green after it.
  • pnpm --filter @objectstack/plugin-security typecheck at cfaac2bf9c: exit 0, including check:test-typecheck: OK.
  • Downstream consumers, measured at 2fbc0ba585:
    • @objectstack/plugin-auth: 128 files, 2637 passed.
    • @objectstack/organizations: 11 files, 147 passed. Both suites alias plugin-security to source.
    • @objectstack/dogfood: the 39 test files that touch these two tables, 369 passed against a rebuilt plugin-security dist.

Ablation

Each leg is a mutation of the committed module through scripts/ablation-replace.mjs. The anchor hits 1 and then 0, the marker shows 1 on disk, and the blob changes. Each leg is followed by a restore to the HEAD blob 37dea505, with git diff HEAD empty. The suite imports the module from source, so no build sits in the path.

  • Leg 1: the refusal's throw disabled. Exactly the 8 negative pins turn red; 13 stay green.
  • Leg 2: the insert-stamp mirror disabled (only a row's own organization is judged). The 5 pins that rely on the stamped organization turn red: platform-admin insert, batch, permission-set insert, single predicate, and fail-closed read. The explicit-organization pin stays green, as predicted.

Gates

node scripts/pm/dispatch-gates.mjs derived 96 families at cfaac2bf9c. All 96 were run, and the reconciliation via --ran reports 96 run, 0 NOT-MEASURED (a DERIVED zero).

  • check-changeset-no-major and check-adr-0087-registration are both green.
  • check:dual-build-cjs-loads is green after building the packages whose dist/ was missing.
  • check:changeset-gate-self-tests failed once on the container's commit-signing service (a 503 inside its throwaway repositories), then passed on rerun.

Lint was a proven narrowing, not a repo-wide run. eslint --no-inline-config --format json over the 5 touched .ts files reports 5 files, 0 errors, 0 warnings. All 5 are in the population of eslint.config.mjs (files: **/*.{ts,…}, and --print-config resolves). The config enables no type-aware linting (no parserOptions.project), so this diff cannot move the verdict on any untouched file.

Acceptance notes

  • content/docs/permissions/system-context.mdx is outside the claim's enumerated file surface. check:system-context-census requires it, because the stand-down for system writes is two new isSystem read sites.
  • The hooks are not unbound in SecurityPlugin.destroy(). Doing so would be a second edit region in security-plugin.ts. Instead, registration first unbinds its own packageId, so a re-run start() replaces the binding rather than doubling it. That registration lifecycle matches the engine middlewares this plugin registers.
  • Membership is read fail-closed. The sibling catalog refusal fails open; the sibling grant-name hook fails closed. This refusal follows the grant-name hook because it is a security refusal.
  • A row stored with no organization is outside this ruling's predicate. An adjacent class, re-scoping an existing organization-scoped grant, is reported to the seat for triage and is not changed here.

Generated by Claude Code

claude added 5 commits October 8, 2026 09:35
…mes a member of it

A non-system insert or update of sys_user_position or sys_user_permission_set
whose user_id holds no sys_member row in the row's organization_id is refused
with the existing 400 VALIDATION_FAILED envelope, reference_not_found at
user_id, for every caller. System writes stand down; a row stored with no
organization is a global grant and stays outside the predicate.

Engine hooks (beforeInsert / beforeUpdate) so an update is judged on the
post-image of every matched row; wired once beside the position catalog
refusal.

Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ
Co-authored-by: Claude <noreply@anthropic.com>
…the membership hook runs after the value refusals

The position-catalog and grant-name suites write organization-scoped grants
under an organization-bound writer, so their harnesses now provision
sys_member and make those holders members. The membership hook moves after
the grant-name derivation so each table judges the value a row names before
its holder. Pins added for the fail-closed membership read and for the
idempotent registration.

Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ
Co-authored-by: Claude <noreply@anthropic.com>
…embership stand-down

The two hooks read isSystem to stand down for system writes, so the census
page carries their row and its counts move by the two reads.

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

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

42 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 8cbe255ef6cadf6a38bcf68a49afa43591f88543.

⛔ 11 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: organization_id (literal, 33 pages)
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 16 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 8cbe255ef6cadf6a38bcf68a49afa43591f88543 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from bf65fec9c3148b69a627c09fb4b9aff132315c52 — the merge of head cfaac2bf9cc8af64078d4973b7f6f6258ac558cf into base 8cbe255ef6cadf6a38bcf68a49afa43591f88543, 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 bf65fec9c3148b69a627c09fb4b9aff132315c52 && git checkout bf65fec9c3148b69a627c09fb4b9aff132315c52
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8cbe255ef6cadf6a38bcf68a49afa43591f88543 cfaac2bf9cc8af64078d4973b7f6f6258ac558cf && git checkout -B drift-repro 8cbe255ef6cadf6a38bcf68a49afa43591f88543 && git merge --no-ff cfaac2bf9cc8af64078d4973b7f6f6258ac558cf

node scripts/docs-audit/affected-docs.mjs --json 8cbe255ef6cadf6a38bcf68a49afa43591f88543

⚠️ 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 8cbe255ef6cadf6a38bcf68a49afa43591f88543 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 8, 2026 11:49
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 8, 2026 11:49
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit 81bd9fa Oct 8, 2026
37 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-22226-position-holder-membership branch October 8, 2026 12:23
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

2 participants