Repository navigation
fix(plugin-security)!: a position assignment or permission-set grant scoped to an organization must name a member of it - #22275
Conversation
…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>
…rrowing) Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
…sition-holder-membership
…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>
📓 Docs Drift CheckThis PR changes 1 package(s): 42 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 11 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 16 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 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
|
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 ofsys_user_positionwhoseuser_idhas nosys_memberrow in the row'sorganization_idis refused, for every caller, platform administrators included, with the row's existing400 VALIDATION_FAILEDenvelope (fields[].code: reference_not_found), no new error code; system writes stand down. The sibling junctionsys_user_permission_setmeasured 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, besideposition-catalog-refusal.ts):registerGrantHolderMembershipRefusalbinds abeforeInsertand abeforeUpdateengine hook onsys_user_positionandsys_user_permission_set.organization_id, or else the caller's active organization, which the driver writes into the empty slot after the hooks run.organizationTheInsertStoresmirrors that stamp:buildDriverOptionsin@objectstack/objectqlandinjectTenantOnInsertin@objectstack/driver-sql.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.sys_memberby (user_id,organization_id) under{ isSystem: true, tenantId }, withtenantIdset to the row's own organization. It joins the writer's transaction. A placeholder-shaped identifier is compared literally.isSystemwrite; a row stored with no organization (a global grant names no organization to be a member of); auser_idthat is not an id (the engine answersrequireditself); and a composition that registers nosys_memberobject.sys_user_position.positionis 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-0087not-required (no-migration-prescription).content/docs/permissions/system-context.mdx: row 23d plus regenerated counts. The hooks readisSystemto stand down, andcheck:system-context-censusrequires a row for each such read site.position-catalog-refusal.test.ts,grant-permission-set-name.test.ts): their harnesses now provisionsys_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:createPositionCatalogRefusalis the middleware onsys_user_position, wired once atsecurity-plugin.ts:4386. It stands down onisSystemand answers the400envelope. 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 abeforeUpdatehook. 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
ObjectQLengine over SQLite with the realSecurityPlugin. The posture isisolatedunless markedsingle. The holder belongs to another organization only.73a0a6bf, pins red)sys_user_position(stamped organization)VALIDATION_FAILED,reference_not_foundatuser_id; row absentorganization_id= the writer's organizationuser_idto a non-memberuser_idto a non-membersys_user_permission_set(stamped organization)single, writer with no active organization (row stored with no organization)single, writer's active organization holds the usersingle, organization-scoped row naming a non-membersingleposture (H2). Under the defaultautomembership 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 byensureDefaultOrganization. So on a stock single-organization deployment, every organization-scoped row names a member. Users created underinvite-onlyand not yet invited are refused, which is the predicate as ruled.Tests
At
74f8180344(merge oforigin/main3513ac7781). Code is byte-identical at headcfaac2bf9c, 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 typecheckatcfaac2bf9c: exit 0, includingcheck:test-typecheck: OK.2fbc0ba585:@objectstack/plugin-auth: 128 files, 2637 passed.@objectstack/organizations: 11 files, 147 passed. Both suites aliasplugin-securityto source.@objectstack/dogfood: the 39 test files that touch these two tables, 369 passed against a rebuiltplugin-securitydist.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 blob37dea505, withgit diff HEADempty. The suite imports the module from source, so no build sits in the path.singlepredicate, and fail-closed read. The explicit-organization pin stays green, as predicted.Gates
node scripts/pm/dispatch-gates.mjsderived 96 families atcfaac2bf9c. All 96 were run, and the reconciliation via--ranreports96 run, 0 NOT-MEASURED (a DERIVED zero).check-changeset-no-majorandcheck-adr-0087-registrationare both green.check:dual-build-cjs-loadsis green after building the packages whosedist/was missing.check:changeset-gate-self-testsfailed 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 jsonover the 5 touched.tsfiles reports 5 files, 0 errors, 0 warnings. All 5 are in the population ofeslint.config.mjs(files: **/*.{ts,…}, and--print-configresolves). The config enables no type-aware linting (noparserOptions.project), so this diff cannot move the verdict on any untouched file.Acceptance notes
content/docs/permissions/system-context.mdxis outside the claim's enumerated file surface.check:system-context-censusrequires it, because the stand-down for system writes is two newisSystemread sites.SecurityPlugin.destroy(). Doing so would be a second edit region insecurity-plugin.ts. Instead, registration first unbinds its ownpackageId, so a re-runstart()replaces the binding rather than doubling it. That registration lifecycle matches the engine middlewares this plugin registers.Generated by Claude Code