Repository navigation
fix(plugin-security): every delegable-scope branch leaves out the built-in identity positions - #24826
Conversation
…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>
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
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): 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 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
|
席位核对 · ACCEPT ·
|
Fixes #24737
Clause-②: no
What changed
GET /security/my-delegable-scopeanswers fromDelegatedAdminGate.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) inassignablePositions.sys_user_position.positionrefuses those names from every writer with400 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 sharedisReservedIdentityNamepredicate.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 sameisReservedIdentityNamepredicate 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:
objects/reserved-identity-names.ts);placeableBusinessUnitIds.packages/spec/**andpackages/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.InviteMemberDialog.tsxrendersassignablePositionsverbatim. A and B serve it equally. C reaches surfaces with no such need.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 realObjectQLengine over a realSqlDriver, with the realSecurityPluginregistered (its own declared built-in positions included), in the posturessingle,groupandisolated. There is one enumerated case per branch: tenant-level (admin_full_access), organization-level (organization_admin) and delegated (aneast_adminplacingadminScope). Nine cases in total. Each case asserts:[400, VALIDATION_FAILED]and stores nothing;isTenantAdminand thescopesset names show which branch answered;assignablePositionscontains none of the four names;field_repis still listed;sys_user_positioninsert ({ 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)scripts/ablation-replace.mjs. Anchorx1 -> x0, blob08da6da24c92 -> 616808d66e5d.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'.08da6da24c92matches HEAD, andgit diff HEADis empty.6b222464ce(git restore --source=BASE, blob2a202d1153). This is the old listing plus the organization-level branch's own copy.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.git checkout HEAD -- PATH: blob08da6da24c92matches HEAD,git diff HEADis empty, andgit status --porcelainis empty.Verification (head
5e6b336871)pnpm --filter '@objectstack/plugin-security^...' build:VERDICT command-exit 0.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 --listFileslists 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.--ranresult:68 derived famil(ies) accounted for — 66 run, 2 NOT-MEASURED.check:dual-build-cjs-loadsandcheck:i18n, each exit 3PREREQUISITE NOT MET. They read whole-repo build output, and this run built only the plugin-security closure. CI owns both families.test:repowas not run locally. CI'sTest Coreruns it.Acceptance notes
Two prose sentences outside this card's surface still say a tenant-level admin's report enumerates "everything":
DelegableScope.isTenantAdminTSDoc inpackages/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
DelegableScopeReportcomment 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 touchesplugin-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