Skip to content

fix(plugin-security): every delegable-scope branch leaves out the built-in identity positions - #24826

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-24737-delegable-scope-no-identity-positions
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-24737-delegable-scope-no-identity-positions

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24737

Clause-②: no

What changed

GET /security/my-delegable-scope answers from DelegatedAdminGate.describeDelegableScope (packages/plugins/plugin-security/src/delegated-admin-gate.ts). Its tenant-level branch and its delegated branch listed the four ADR-0068 D2 identity names (platform_admin, org_owner, org_admin, org_member) in assignablePositions. sys_user_position.position refuses those names from every writer with 400 VALIDATION_FAILED, so the invite dialog and Setup's position picker offered choices that could only fail. The organization-level branch already skipped them through the shared isReservedIdentityName predicate.

All three branches read one local listing, allPositions(). That listing already left out the two audience anchors. It now also leaves out the identity names, through the same isReservedIdentityName predicate that the write's validation is generated from. The organization-level branch's own copy of the skip is gone, because the listing it reads now does the skip once. Its behaviour is unchanged.

Unchanged, as the card bounds it:

  • the write path's refusal, and the reserved list (objects/reserved-identity-names.ts);
  • every other position in every branch;
  • placeableBusinessUnitIds.

packages/spec/** and packages/rest/** are not touched.

Why the shared listing, not a second copy (decision frame)

Option A applies the predicate once, in the listing all three branches read. Option B adds a skip to each of the other two branch loops, which makes three copies of one rule. Option C filters in the catalog reader (@objectstack/core), which would also change the metadata door, and that door is meant to list the built-ins.

  • Real business need. The consumer is real and named on the card: objectui InviteMemberDialog.tsx renders assignablePositions verbatim. A and B serve it equally. C reaches surfaces with no such need.
  • Long-term soundness. A keeps one rule in one place inside the report. A future fourth branch reads the same listing and inherits the skip. B is the copy-per-branch shape that produced this defect: one branch had the skip and two did not.
  • Making AI mistakes harder. Under A, a new branch cannot forget the skip, because there is nothing to remember. Under B, the next branch has to know to add it.
  • No scope spread. A is a one-line filter plus the deletion of a copy. It adds no new gate, no new surface and no spec change.

Recommendation: A, which is what this PR implements.

Pin

New: packages/plugins/plugin-security/src/delegable-scope-no-identity-positions.posture.test.ts. It runs on a real ObjectQL engine over a real SqlDriver, with the real SecurityPlugin registered (its own declared built-in positions included), in the postures single, group and isolated. There is one enumerated case per branch: tenant-level (admin_full_access), organization-level (organization_admin) and delegated (an east_admin placing adminScope). Nine cases in total. Each case asserts:

  • ground truth:
    • the catalog this stack serves lists all four identity names and the control position (otherwise the absence check would pass over a catalog that never held them);
    • the real write refuses each identity name from the same caller with the ADR-0112 envelope [400, VALIDATION_FAILED] and stores nothing;
  • branch identity: isTenantAdmin and the scopes set names show which branch answered;
  • the rule:
    • assignablePositions contains none of the four names;
    • the control position field_rep is still listed;
    • every listed position is admitted by the real sys_user_position insert ({ status: 200 }), and the stored row is read back.

This case is modelled on the PR #24793 suite (card #24736), delegated-admin-own-name-set.posture.test.ts.

Ablation (both legs at head 5e6b336871, run under the shared verify lock)

  • Leg 1: the listing's new predicate is removed with scripts/ablation-replace.mjs. Anchor x1 -> x0, blob 08da6da24c92 -> 616808d66e5d.
    • Result: Tests 9 failed (9). All three branches go red, which shows the one predicate now guards the organization-level branch too. First failure: platform_admin offered: expected [ 'platform_admin', 'org_owner', …(3) ] to not include 'platform_admin'.
    • Restored: blob 08da6da24c92 matches HEAD, and git diff HEAD is empty.
  • Leg 2: the whole gate file is put back at the base 6b222464ce (git restore --source=BASE, blob 2a202d1153). This is the old listing plus the organization-level branch's own copy.
    • Result: Tests 6 failed | 3 passed (9). The tenant-level and delegated cases are red in all three postures, and the organization-level cases are green. That is exactly the defect the card reports.
    • Restored with git checkout HEAD -- PATH: blob 08da6da24c92 matches HEAD, git diff HEAD is empty, and git status --porcelain is empty.

Verification (head 5e6b336871)

  • Closure build pnpm --filter '@objectstack/plugin-security^...' build: VERDICT command-exit 0.
  • New file alone: Tests 9 passed (9), test time 634 ms (measured on a shared box).
  • pnpm --filter @objectstack/plugin-security test: Test Files 194 passed (194), Tests 4040 passed | 45 skipped (4085), VERDICT command-exit 0.
  • pnpm --filter @objectstack/plugin-security typecheck: VERDICT command-exit 0. tsc -p tsconfig.test.json --listFiles lists the new test file (1 hit).
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack: 68 families. All were run locally, each exit captured before any pipe.
    • --ran result: 68 derived famil(ies) accounted for — 66 run, 2 NOT-MEASURED.
    • NOT MEASURED locally: check:dual-build-cjs-loads and check:i18n, each exit 3 PREREQUISITE NOT MET. They read whole-repo build output, and this run built only the plugin-security closure. CI owns both families.
  • test:repo was not run locally. CI's Test Core runs it.

Acceptance notes

  • Two prose sentences outside this card's surface still say a tenant-level admin's report enumerates "everything":

    • the DelegableScope.isTenantAdmin TSDoc in packages/spec/src/contracts/security-service.ts;
    • content/docs/permissions/delegated-administration.mdx ("with everything enumerated").

    Both already excluded the audience anchors before this PR. This is wording drift only, not a defect: no reader parses those sentences. This PR's own DelegableScopeReport comment now says "everything the write path admits". No carrier has been named for the two sentences.

  • The branch is 3 commits behind origin/main. None of those commits touches plugin-security, and CI tests the merge ref.

Related, for reference only: card #24136 and its PR #24732 (the organization-level branch's skip), and card #24736 and its PR #24793 (the delegated walk's own-name set).


Generated by Claude Code

…lt-in identity positions

describeDelegableScope's tenant-level and delegated branches offered the
ADR-0068 D2 identity names (platform_admin, org_owner, org_admin,
org_member), which sys_user_position.position refuses from every writer.
The shared position listing every branch reads now applies the
isReservedIdentityName predicate once, and the organization-level
branch's own copy of the skip is folded into it.

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/permissions/delegated-administration.mdx (via DelegatedAdminGate (symbol, a top-level class), /api/v1/security/my-delegable-scope (route, the route ledger binds it to client method security.describeDelegableScope))

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

  • content/docs/releases/v13.mdx (via DelegatedAdminGate (symbol, a top-level class))

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 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 — 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 ddb3360a4954c1ccb6878deb3ca0b303c79d3289 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json ddb3360a4954c1ccb6878deb3ca0b303c79d3289

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:services seat 1 · 2026-10-20T08:57Z

os-justin · session_011TdZ8GjixNZFLz1xJD9zzK (seat post #6021). Reviewed against GitHub and origin/main c3a8e07b6a, not against the report (os-dev-report on #24737). Head under review:

5e6b3368715cbc689c837ddd5324a0167ff761df
  1. Shape. Draft, base main, first line Fixes #24737, line-start Clause-②: no in the body and the changeset. The body names plugin-security: organisation admins assign positions and permission sets to their own members inside the D12 boundary — gate branch, overrides, explain and my-delegable-scope, three-posture acceptance (ruling B on #23960) #24136, feat(plugin-security): organization administrators assign positions and permission sets inside the D12 organization boundary #24732, plugin-security: the delegated-administration gate's position walk does not count the permission set a position grants by its own name, so a delegate's allowlist check judges fewer sets than the assignment grants #24736 and fix(plugin-security): the delegated walk counts the permission set of the position's own name #24793 with no keyword beside any of them. 3 files, +251/−8, none on a governed surface (governedPathsIn answers []). The trial merge onto origin/main is clean. No contract review is owed: no published schema, no governed text, no declared widening or narrowing. The report offers fewer options, and every option it drops is one the write already refuses.
  2. One listing, read in the code.
  3. Pins and ablations.
    • delegable-scope-no-identity-positions.posture.test.ts runs on a real ObjectQL engine over SqlDriver, with the real SecurityPlugin. It covers 3 branches × 3 postures (single, group, isolated).
    • Ground truth: the served catalog holds all four identity names and the control, and the real write refuses each name with [400, VALIDATION_FAILED] and stores nothing.
    • The rule: none of the four is offered, the control is, and every offered position is admitted by the real sys_user_position insert, read back. So the listing equals the write.
    • Leg 1 (the new predicate removed) turns 9 of 9 red.
    • Leg 2 (the gate file at the base) turns 6 red and leaves 3 green: tenant-level and delegated red, organization-level green, which is exactly the card's defect.
    • Both restores were proven by blob equality.
  4. Changeset, @objectstack/plugin-security patch, read sentence by sentence: what was listed and where the write refuses it, the one listing, and "Unchanged" (the write path's refusal, the reserved list, placeable units, no spec key, endpoint or configuration). Each matches 2.
  5. Gates. At ACCEPT, every check-run on this head is success or an expected skip, among them TypeScript Type Check, Lint & Repo Gates, Test Core 1–6/6, Build Core and Governed Surface Queue Guard. Locally, 66 of 68 derived families exited 0. 2 were NOT MEASURED (check:dual-build-cjs-loads, check:i18n: exit 3, prerequisite), and CI's lint lane measures them. The full package passes: 4040 passed, 45 skipped.
  6. Accepted deviations.
    • The organization-level branch's copy of the skip is deleted, with no behaviour change, as leg 2 shows.
    • The full package ran at vitest's default worker count, because a bare -- --maxWorkers=2 was dropped. That is resource discipline only.
    • The branch is 3 commits behind main, none of them in plugin-security. The merge group builds the combination, and the trial merge is clean.
    • Commit trailers are model-free, per AGENTS.md.
  7. Out-of-scope dispositions. Two prose sentences still say a tenant-level admin's report enumerates "everything": the DelegableScope.isTenantAdmin TSDoc in packages/spec/src/contracts/security-service.ts, and content/docs/permissions/delegated-administration.mdx. Both already excluded the anchors before this PR, so this is wording drift with no reader that parses it. Disposition: the seat's list of stale comments.
  8. Landing clause. Tier S: not governed, under the human-merge threshold. Next step: ready + auto-merge through the relay once every check on the current head is green or an expected skip. A push resets this reading.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 08:58
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 08:58
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 1366740 Oct 20, 2026
42 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24737-delegable-scope-no-identity-positions branch October 20, 2026 09:25
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

Development

Successfully merging this pull request may close these issues.

plugin-security: my-delegable-scope offers a platform administrator the built-in identity positions that sys_user_position refuses from every writer

2 participants