Skip to content

fix(plugin-security)!: retire the position-name fold; a namesake set is reported, never granted (#24757) - #26064

Merged
objectstack-fleet[bot] merged 16 commits into
mainfrom
claude/project-thread-6wuxt3
Oct 26, 2026
Merged

objectstack-fleet[bot] merged 16 commits into
mainfrom
claude/project-thread-6wuxt3

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #24757
Fixes #25799

Clause-②: yes (narrowing: a holder of position P is no longer granted the permission set named P unless P's definition names it in permissionSets; widened: @objectstack/core's UserAuthzGrants and ResolvedAuthzContext gain the additive field directPermissions)
Metadata-protocol: none
Ruling-ref: #13419 ruling B (5472608110); 「甲」 as amended (6303865025); order and unlock on #24757 (6290781571, 6392096931); #25799 seat ruling A and S1 (claims 6414302166, 6414315157)

Summary

ADR-0131 D3/D4 says a position grants only the permission sets its definition's permissionSets names. The security plugin still folded every held position's name into the set request, so a set that shared a position's name was granted without anyone writing the binding. #13419 ruling B retires that channel. HotCRM declared its bindings first (objectstack-ai/hotcrm#2200, released in v6.0.0), and #24757's unlock (6392096931) clears this half.

  • Resolver (security-plugin-permission-sets.ts): requests only context.permissions plus the baseline. dropDeactivatedFoldedSets, the second activation read only the fold needed, is deleted.
  • Warning: position_name_fold_grant keeps its event name and fields, adds granted: false, and now says the same-named set is not granted and how to declare it. reportNameFoldCollisions became reportUngrantedNamesakeSets. It probes the held position names that the request did not already name, through the same evaluator and loaders, once per name per process. The probe result is never granted.
  • describePositionGrants: lists only the sets a position's definition names. It no longer serves via: 'name'.
  • Delegated-admin gate (delegated-admin-gate.ts): setsCarriedByPosition returns the definition's sets only. A set that only shares a position's name is no longer walked for the allowlist, containment or the organization boundary, and the ownNameNote refusal suffix is gone.
  • Explain (explain-engine.ts): position:P when P's definition names the set (read from the security catalog), otherwise direct grant. When the definitions are unreadable it says direct grant or position definition (the position definitions could not be read) instead of guessing. This also fixes the decision's "另" note, where a definition-bound set showed as a direct grant, and that is security/explain: sets a position grants through its definition's permissionSets are attributed "via direct grant", not to the position #25799's case.
  • scripts/pm/dispatch-gates.mjs (docblock only): it cited the deleted census's #selfTest as a live anchor, which check-scripts-symbol-anchors flagged as unresolved. It now names the two files that remain and records the third as deleted. This is a bounded in-place fix forced by the deletion.
  • Gate retirement: deleted scripts/check-position-name-fold-loaders.mjs, its .github/workflows/lint.yml step, and scripts/measure-position-name-fold-census.mjs (its self-test was already failing on main). Removed the gate's block from scripts/pm/dispatch-gates.self-test.mjs. scripts/pm/dispatch-gates.data.mjs held no entry for it (measured: no match), so it is unchanged.

#25799: a set held through both channels names both

  • Core (resolve-authz-context.ts): UserAuthzGrants and ResolvedAuthzContext gain directPermissions: string[]. These are the names the user's own sys_user_permission_set rows confer in this resolution, by §6's own rules: the row applies in the tenant (or anchors platform standing), and its set is held by the catalog and not switched off. A set a position definition also names is listed; a set only a definition names is not. The field is additive and grants nothing. Hand-built literals must now set it (directPermissions: []).
  • Explain: a set in both channels reads direct grant + position:P, so revoking the position is not read as removing the access. On the userId path, buildContextForUser carries the split.
  • Self path (security-plugin-explain-access.ts): the request envelope's field set is closed (ExecutionContextSchema), so it cannot carry the split without a spec change. A human caller is therefore resolved once more through resolveUserAuthzGrants (bypassGrantsCache, as buildContextForUser does), and only the split is attached. Agent, guest and system callers keep the context as it arrived. A failed re-resolve reads position:P (whether a direct grant also names it could not be read).
  • Goldens: the core batch-equivalence golden JSON gains each fixture's directPermissions, generated from the current resolver; the captured envelope is otherwise unchanged. Two exact envelopes add it (request-rounds, explain-engine buildContextForUser). In platform-admin-config's posture-invariance arm, the posture moves only the channel (a single legacy grant anchors; walled, the config confers the set), so it compares without the split. Three plugin-security grant goldens, recorded before the split existed, compare the grants without it.
  • scripts/engine-double-contract.pinned.json records the new self-explain double (--write, as the gate asks).

Carried by #26022 (spec and UI half, Blocked-by: #24757)

PositionGrantVia's 'name' member and its docblock (packages/spec/src/contracts/security-service.ts), the "(by name)" badge (PositionGrantCells.tsx) and the managedEnvironmentUsers.tsx reader are outside this lane's fence (no packages/spec/** or ui/** change here). Until #26022 lands, 'name' is a published value that nothing emits. Explain's via is z.string(), so the attribution fix is runtime-only.

Observations carried from the 18.x half (6303645764)

  1. Multi-org dedupe of the warning: the security catalog is environment-level (dbLoaderForContext returns the same catalogSetLoader for every caller), so per-name dedupe is correct. Nothing to change.
  2. Lazy warning: kept. The warning fires on the first resolution that holds the position, not at boot. This is documented on reportUngrantedNamesakeSets.

BREAKING

  • Before: a holder of position sales_manager was granted the set sales_manager whether or not the position named it.
  • Now: only the sets the definition lists. The first such resolution logs position_name_fold_grant with granted: false.
  • Fix: permissionSets: ['sales_manager'] on the position, or nothing if the shared name is a coincidence.
  • Cloud ordering: an environment running HotCRM must be on HotCRM v6.0.0 or later before it takes this release.

Changeset: .changeset/24757-retire-position-name-fold.md, @objectstack/plugin-security patch, with the ADR-0087 marker not-required (no-migration-prescription).

Tests

Every test that granted through a shared name now pins the opposite, and each fixture that used the fold to hand out a set now declares the grant:

  • position-name-fold-warning.test.ts: MUST FIRE asserts granted: false, the set is not granted, the exact payload keys and the new text (no "next major", "deprecated" or "in force"). The MUST NOT FIRE cases are kept.
  • orgless-position-name-fold.test.ts, permission-set-active.test.ts, guest-grants-channel.test.ts, describe-position-grants.test.ts: a position name reaches no set. Each has a control showing the set is real and resolves when named.
  • delegated-admin-gate.test.ts, delegated-admin-own-name-set.posture.test.ts (three postures): the own-name set is not judged, with a field_audit teeth control that a listed set still is.
  • New explain-position-set-attribution.test.ts: definition-bound, direct, a direct namesake, and an unreadable catalog.
  • Fixtures moved from name-granting to declared grants: security-plugin.test.ts (D10), controlled-by-parent-chain-middle-master.test.ts, permission-set-resolution-memo.test.ts.

Ablations (negative pins; scripts/ablation-replace.mjs, each restored to blob == HEAD)

Leg Mutation (the fold put back) Result
A1 resolver: const requested = [...positions, ...explicitPermissionSets]; 5 files / 11 tests red (warning, orgless, permission-set-active, guest, posture)
A2 setsCarriedByPosition walks the own-name set again 2 files / 17 tests red
A3 describePositionGrants requests the position's own name 2 tests red (names each set its definition grants … no set that only shares its name, agreement with the resolver)
A4 explain: positions.includes(name) ? name : … 2 tests red (expected 'position:sales_manager' to be 'direct grant')
A5 silence reportUngrantedNamesakeSets 5 MUST FIRE tests red
B1 explain: the dual case reads position:P again 1 red (expected 'position:sales_rep' to be 'direct grant + position:sales_rep')
B2 core: no push into directPermissions 3 of 4 core pins red
B3 core: applyingGrants tenant-blind 4 of 4 core pins red
C1 self path: the human guard is false 2 red (dual case, failed-read wording)
C2 self path: re-resolve for every non-system caller 1 red (the agent pin: expected 'direct grant + position:sales_rep' to be 'position:sales_rep')

The dual pin was red before the change: expected 'position:sales_rep' not to be 'position:sales_rep'. The userId pin was red against a core built without the field: expected undefined to deeply equal ['crm_sales_user','forecasting'].

Readings (head 32fe4f1b)

  • At 32fe4f1b (main merged a second time): @objectstack/plugin-security 226 files, 4747 passed, 45 skipped, 0 failed. @objectstack/core 128 files, 2865 passed, 0 failed. Typecheck of both (src, scripts, test layer) exit 0. check:engine-double-contract, check:objectql-double-limit, check:where-matcher, check-scripts-symbol-anchors and check:authz-resolver all exit 0.
  • At 621e6afe + the ledger commit: full build exit 0. plugin-security 4705 passed, core 2855 passed. eslint over plugin-security and core/src/security exit 0.
  • dispatch-gates --commands: 113 derived, each run with its exit code captured. 112 were green; check:engine-double-contract asked for the pinned-ledger --write, which was committed and re-run at 0.
  • dispatch-gates --ran: 113 derived, 113 run, 0 NOT-MEASURED, 0 UNRUN.
  • The grant-resolving tests of the other packages that call the resolver or buildContextForUser (cli, client, objectql, plugin-approvals, plugin-audit, plugin-auth, plugin-hono-server, plugin-sharing, http-conformance, rest, runtime, service-automation, service-datasource, service-storage, trigger-record-change) all passed.
  • Dogfood (full): 296 files passed, 3 skipped; 2523 tests passed, 18 skipped, 0 failed.
  • The first round's readings (aa5c5b22, 110/110 gates) stand in the history. The gate repairs made in that round, dispatch-gates.mjs docblock among them, are listed in the Summary.
  • Changed lines: 3524 (+985 / −2539), over the 3000 human-merge threshold. Most of that is the two deleted scripts (−1907).
  • Open PRs on plugin-security/**: only feat(spec,plugin-security,lint): sharing.controlledByParent.ownerMayEdit — a detail row's owner updates it with read on the master #26005 touches security-plugin.ts (CBP ownerMayEdit), in other hunks. A local git merge-tree of the two merges that file cleanly.

Acceptance notes

Stale prose left in place because it is outside this claim's file surface. Each describes the fold as live:

维护者速读(草稿)

改了什么
以前,只要一个权限集和某个职位同名,持有这个职位的人就会自动拿到这个权限集,哪怕职位定义里根本没写它。现在取消了这条"同名自动授权"的暗道:职位只授予它定义里 permissionSets 明确列出的权限集。委派管理员的检查、权限解释面板("这个权限从哪来")和职位列表,也都改成只认定义里写明的绑定。解释面板还能分辨"直接授予"和"通过职位授予":两条渠道都有时会同时写明(#25799),这样撤掉职位也不会被误读成撤掉了权限。为此 core 的授权结果增加了一个只读字段 directPermissions。原来专门守护这条暗道的一个 CI 检查脚本和一个统计脚本一并删除。

为什么改
按 ADR-0131 和 #13419 裁决 B,"绑定要写出来,不能靠猜"。同名授权是隐式的:改个名字就可能无声地多给或少给权限,审计时也看不出来。HotCRM 已在 v6.0.0 把所有绑定写明,这一步是计划好的收尾。

风险与代价(含回滚)

  • 这是收窄(BREAKING):如果某个环境靠同名拿权限、但职位定义里没写,升级后这些用户会失去该权限集。系统会在第一次遇到时打出一条明确警告(position_name_fold_grant,granted: false),写明该怎么补。
  • 云上顺序:跑 HotCRM 的环境要先升到 HotCRM v6.0.0 或更高,再升这个版本。
  • 有一个过渡缺口:规范里的 PositionGrantVia 仍然列着 'name' 这个取值,界面上也还有"(同名)"标记,但运行时已不再产生它。这部分由 spec: retire PositionGrantVia 'name' (the position-name fold's grant mark) once #24757's runtime retirement lands #26022 收尾,在它合入前,这个已发布的取值不会再出现。
  • @objectstack/core 公开类型新增 directPermissions 字段,属于纯新增。手写这两个类型对象的代码需要补上 directPermissions: []。
  • 自查自己的权限时,会为真人用户多做一次授权解析,只用于诊断,不影响实际授权。
  • 回滚:直接 revert 这个 PR 即可恢复同名授权,不涉及数据迁移。

席位意见

你要做的
审阅后决定是否批准合并(Tier H,改动了 .github/workflows/lint.yml,改动行数约 3500 行,超过 3000 行人工合并阈值,其中约 1900 行是删除的两个脚本)。合并后请让 #26022 跟进规范和界面的那一半。

🤖 Generated with Claude Code

https://claude.ai/code/session_01J8LYP8UCJURxbFWZqnywyD

…is reported, never granted

WIP: resolver, describePositionGrants, delegated walk, explain attribution,
the fold-loader gate and census instrument.

Claude-Session: https://claude.ai/code/session_01J8LYP8UCJURxbFWZqnywyD
Co-authored-by: Claude <noreply@anthropic.com>
…es to declared grants

Claude-Session: https://claude.ai/code/session_01J8LYP8UCJURxbFWZqnywyD
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/core, @objectstack/plugin-security, touching 31 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/core/src/security/resolve-authz-context.batch-equivalence.golden.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/deployment/environment-variables.mdx (via resolveUserAuthzGrants (symbol, a top-level function))
  • 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))
  • content/docs/permissions/field-level-security.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/permissions/index.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/permissions/system-context.mdx (via explainAccessForCaller (symbol, a top-level function))
  • content/docs/plugins/packages.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/ui/forms.mdx (via SecurityPlugin (symbol, a top-level class))

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

  • content/docs/releases/implementation-status.mdx (via SecurityPlugin (symbol, a top-level class))
  • content/docs/releases/v13.mdx (via DelegatedAdminGate (symbol, a top-level class))
  • content/docs/releases/v16.mdx (via resolveUserAuthzGrants (symbol, a top-level function))
  • content/docs/releases/v17/17-1.mdx (via resolveUserAuthzGrants (symbol, a top-level function))
  • content/docs/releases/v17/17-4.mdx (via ResolvedAuthzContext (symbol, a top-level interface))
  • content/docs/releases/v17/17-5.mdx (via buildContextForUser (symbol, a top-level function), resolveUserAuthzGrants (symbol, a top-level function))
  • content/docs/releases/v18/18-0.mdx (via SecurityPlugin (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
  • 1 changed file(s) yielded no anchor (packages/core/src/security/resolve-authz-context.batch-equivalence.golden.json) — pages documenting those are invisible to this run
  • 1 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 — 36 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 6862ef62abd9062b54e92a8d8f82ec1abf1320c1 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 6862ef62abd9062b54e92a8d8f82ec1abf1320c1

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

…d directly and through a position (#25799)

resolveUserAuthzGrants reports its own split of the two channels as the
additive directPermissions on UserAuthzGrants and ResolvedAuthzContext: the
names the user's own sys_user_permission_set rows confer in the active
organization. The explainer reads it on the self and userId paths, so a set
held both directly and through a held position's definition reads
'direct grant + position:P' instead of hiding the direct row.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8LYP8UCJURxbFWZqnywyD
…channel split (#25799)

The request envelope a self-explanation runs on carries no
directPermissions, so explainAccessForCaller resolves a human caller once
more through resolveUserAuthzGrants and attaches only that split. Agent,
guest and system callers keep the context as it arrived; a failed
resolution is said, never guessed.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8LYP8UCJURxbFWZqnywyD
…; conforming engine doubles (#25799)

The batch-equivalence golden and two exact envelopes gain the resolver's
channel split; the plugin-security grant goldens, recorded before the split
existed, compare the grants without it. The new explain doubles honour
where (refusing combinators), the caller's limit and the findOne predicate.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8LYP8UCJURxbFWZqnywyD
…lares its positions' sets (#24757, #25799)

plugin-audit's hand-built authz envelopes add the now-required
directPermissions (workspace typecheck). The parent-write-reach fixture
granted pwr_manager / pwr_rep through the retired position-name fold; its
positions now name their sets. Stale fold-as-live prose in
resolve-authz-context.ts is corrected.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8LYP8UCJURxbFWZqnywyD
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: df4eb65c483127bc070e49cd30b0ea8685f39eae
Local-runs: none
Scope: .github/workflows/lint.yml, packages/cli/test/verify-parent-write-reach.test.ts, packages/core/src/security/assemble-execution-context.test.ts, packages/core/src/security/resolve-authz-context.batch-equivalence.golden.json, packages/core/src/security/resolve-authz-context.batch-equivalence.test.ts, packages/core/src/security/resolve-authz-context.direct-permissions.test.ts, packages/core/src/security/resolve-authz-context.platform-admin-config.test.ts, packages/core/src/security/resolve-authz-context.request-rounds.test.ts, packages/core/src/security/resolve-authz-context.ts, packages/plugins/plugin-audit/src/agent-dual-attribution.integration.test.ts, packages/plugins/plugin-audit/src/comment-access-hooks.test.ts, packages/plugins/plugin-security/src/builtin-positions.boot.test.ts, packages/plugins/plugin-security/src/controlled-by-parent-chain-middle-master.test.ts, packages/plugins/plugin-security/src/delegated-admin-gate.test.ts, packages/plugins/plugin-security/src/delegated-admin-gate.ts, packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts, packages/plugins/plugin-security/src/describe-position-grants.test.ts, packages/plugins/plugin-security/src/explain-engine.test.ts, packages/plugins/plugin-security/src/explain-engine.ts, packages/plugins/plugin-security/src/explain-position-set-attribution.test.ts, packages/plugins/plugin-security/src/explain-self-direct-grant.test.ts, packages/plugins/plugin-security/src/grant-permission-set-name.equivalence.test.ts, packages/plugins/plugin-security/src/guest-grants-channel.test.ts, packages/plugins/plugin-security/src/orgless-position-name-fold.test.ts, packages/plugins/plugin-security/src/permission-set-active.test.ts, packages/plugins/plugin-security/src/permission-set-resolution-memo.test.ts, packages/plugins/plugin-security/src/position-name-fold-warning.test.ts, packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.golden.test.ts, packages/plugins/plugin-security/src/security-plugin-explain-access.ts, packages/plugins/plugin-security/src/security-plugin-permission-sets.ts, packages/plugins/plugin-security/src/security-plugin.test.ts, packages/plugins/plugin-security/src/security-plugin.ts, packages/verify/src/parent-write-reach.test.ts, scripts/check-position-name-fold-loaders.mjs, scripts/engine-double-contract.pinned.json, scripts/measure-position-name-fold-census.mjs, scripts/pm/dispatch-gates.mjs, scripts/pm/dispatch-gates.self-test.mjs + packages/plugins/plugin-security/src/permission-evaluator.ts (resolvePermissionSets: empty request, loader failure swallowed), packages/spec/src/contracts/security-service.ts (PositionGrantVia), packages/spec/src/security/explain.zod.ts (via: z.string().optional()), packages/spec/src/kernel/execution-context.zod.ts (ExecutionContextSchema is a strip-mode z.object, principalKind optional), packages/core/src/security/assemble-execution-context.ts (principalKind set at the door), packages/core/src/security/index.ts (both types exported), scripts/pm/dispatch-gates.data.mjs (no fold entry, as claimed), objectstack-ai/hotcrm src/sales/sharing/positions.ts and test/demo-staffing.test.ts (the declared bindings and the no-fold-event boot pin)

Check-runs on the head: 47 check-runs, 39 success, 8 skipped (opt-in harnesses and matrix placeholders: 18.x upgrade harness (opt-in), Packed-tarball smoke (opt-in), Temporal Conformance (live PG + MySQL), Build Docs, Lint UI, Dogfood Verify CLI, the Test UI and Dogfood Regression Gate shard placeholders), 0 failed, 0 in progress. Type Check · workspace and TypeScript Type Check, red on the previous head 32fe4f1b, are green here, as are all eight Test Core shards, Type Check UI, Check Changeset and Governed Surface Queue Guard.

Reviewed against card #24757 (body and all 18 comments, rulings 6290781571, 「甲」 as amended, unlock 6392096931, claims 6414302166, dev reports 6414056485 and the addendum 6417094296) and card #25799 (body and all 5 comments, seat ruling A and S1 in claims 6414107849 / 6414315157), the PR body (declarations unchanged between the two heads), its 39-file list, the net diff of df4eb65c against merge base 1bbb7bf4 (+1005 / −2552 over 39 files; −1907 of it the two deleted scripts), and the two-commit repair diff 32fe4f1b..df4eb65c (5 files, +20 / −13) that followed the first round of this review.

① Derived judgments

  1. Resolver request narrows to the context's set names (security-plugin-permission-sets.ts: const requested = [...explicitPermissionSets], baseline pushed after; security-plugin.ts drops the dropDeactivatedFoldedSets method and its GUEST_POSITION import). A held position's NAME no longer resolves a same-named set. Right. This is 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 ruling B as ordered on Setup: a position's same-named permission set is in force but invisible — the set's Assigned Users panel is empty and the position reads as granting nothing #24757 (6290781571: HotCRM declares first, released in hotcrm v6.0.0 per the unlock 6392096931), shipped in 18.x under 「甲」 as amended. The guest arm (positions = isGuest ? [] : …) is kept, so ADR-0138 D3 item 3 holds for the guest by construction rather than by exclusion. The deleted second activation read is correct to delete: every name the plugin now requests arrived already judged by core's readDisabledCatalogNames (§6a/§6b), and permission-set-active.test.ts pins that a held position issues no ledger read. The four deleted pins on the ledger's 0/1 storage shape and type-scoped rows pinned that second reader, which no longer exists; core's own resolver tests parameterise active and metadata_type for the one reader left.

  2. The loud half (reportUngrantedNamesakeSets, event position_name_fold_grant, payload event/position/permissionSet unchanged, removedIn: 'next_major' replaced by granted: false, new text with no tracker id). Right. 要点 5 is honoured: the difference is a warning, never a silent change. The probe resolves the held position names the request did not already name through the same evaluator and the same three sources, and what comes back is never granted (namesakes is only iterated, never merged into permissionSets). The predicate reads requested after the baseline push, so a baseline-named position stays silent, and once-per-name dedupe keys on a name that actually had a namesake. Read in the cited evaluator: an empty candidate list returns [] before any source is read; a catalog-loader failure is swallowed and warned once as a lookup failure, so the probe cannot fail the authorization resolution. removedIn was a log-payload key that shipped on 2026-10-20 (f90ea677c) and nothing in the tree consumes it; dropping it is a log change, not a published surface. Observation, not a finding: a held position with no namesake (everyone, org_member) is re-probed on every unmemoized resolution, which is a second metadata.list('permission') call per such resolution. The fold made equivalent catalog reads for the same names in the one evaluator call; the docblock states the cadence.

  3. describePositionGrants serves via: 'definition' only (security-plugin.ts: resolvePermissionSets(declared, …), via: 'definition' unconditionally). Right, and the test pins that no summary carries any other via. PositionGrantVia in packages/spec/src/contracts/security-service.ts:130 still publishes 'definition' | 'name'; the 'name' member is published and unemitted until spec: retire PositionGrantVia 'name' (the position-name fold's grant mark) once #24757's runtime retirement lands #26022 (open, Blocked-by: #24757) retires it. Nothing reads 'name' to grant anything (the objectui badge renders it; HotCRM and objectui construct neither core type and read no 'name' member at their pins), so the transitional state is sound.

  4. Delegated-admin gate (delegated-admin-gate.ts: setsCarriedByPosition returns [...new Set(listed)]; ownNameNote deleted). Right. A set that only shares the position's name is granted by nothing after item 1, so judging it would refuse assignments that hand out nothing. Both consumers (the organization-level boundary and the delegated walk through setsBoundToPosition) read the one function, and the gate and the resolver read the same security catalog, so there is no window in which the gate admits what the resolver grants. This is a widening of what a delegate may assign relative to main, and it is the correct one; the unit and the three-posture suites each keep a teeth control (a LISTED set outside the allowlist is still refused, a LISTED admin set still cannot be self-delegated).

  5. Explain attribution (explain-engine.ts: positionBindingsOf reads each held position's definition from the security catalog; viaOf says position:P when P's definition names the set, direct grant + position:P when context.directPermissions also names it, position:P (whether a direct grant also names it could not be read) under directPermissionsUnread, direct grant otherwise, and direct grant or position definition (the position definitions could not be read) when the catalog is unreadable). Right. Ruling A is honoured: the dual case names both channels. via is z.string().optional() in explain.zod.ts, so the wording moves no published accept set, and the dev's claim to that effect holds. Two bounded readings: a set named by two held definitions is attributed to the first in held order (the one channel a position grants through; sufficient for the card's distinction), and a context the resolver did not build and that carries no split (an agent acting for a user, a hand-built context without principalKind) reads position:P alone, which is today's reading and what S1 asks for.

  6. Core: directPermissions: string[] on UserAuthzGrants and ResolvedAuthzContext (resolve-authz-context.ts: pushed in the §6 loop over applyingGrants, after the tenant/anchor filter and only when active, never from the §6a boundSetNames loop; initialised [] in all three literals; copied into ctx beside permissions). Right, and it is the split the resolver already computed: applyingGrants is the row set §6 confers from, so the field cannot disagree with permissions about which direct rows applied. Both types are exported from @objectstack/core (security/index.ts:223,225). The field is REQUIRED, not optional: additive for every reader, a compile-time change for code that builds either object as a literal. That is what the previous head's Type Check · workspace red measured: packages/plugins/plugin-audit test literals built envelopes without it, and the repair adds directPermissions: [] to the three literals (comment-access-hooks.test.ts twice, agent-dual-attribution.integration.test.ts once), changing no assertion. Out of the repo, objectstack-ai/hotcrm and objectui at their checkouts build neither type. Required is the right choice (an optional field would let a hand-built context silently read as "no direct grants"), and the changeset states the one-line fix. See ②.

  7. Self path (security-plugin-explain-access.ts: else if (principalKind === 'human' && isSystem !== true && !onBehalfOf?.userId && userId) re-resolves through resolveUserAuthzGrants(this.ql, userId, { tenantId, nowMs, bypassGrantsCache: true }) and attaches only directPermissions, else directPermissionsUnread: true; the context's grants stay the request's). Right. Ruling S1 is honoured: agent, guest, service and system callers take neither branch and keep the context as it arrived; the dev's C2 ablation (re-resolve for every non-system caller) reds the agent pin. The premise holds in the cited spec: ExecutionContextSchema is a strip-mode z.object, so a transport parse drops an unknown directPermissions key, and the split cannot ride the envelope without a spec change. Cost: one extra resolver run per human self-explanation on a diagnostic endpoint, and the spread context is a new object so the per-context set memo misses once; both acceptable for explain. directPermissionsUnread is a private context key read only by viaOf.

  8. Goldens and ledgers. Each exclusion or extension was judged on whether it hides a behaviour change:

    • resolve-authz-context.batch-equivalence.golden.json and its test docblock: the field is ADDED to every fixture and compared (not excluded). The values are consistent with the fixtures' own grant rows (the u_ta fixture lists low and not the position-bound organization_admin; the admin fixture lists both its direct rows). Legitimate; the pin is extended.
    • request-rounds.test.ts and explain-engine.test.ts (buildContextForUser): exact envelopes extended with the field. Legitimate.
    • platform-admin-config.test.ts posture-invariance arm: compares single against isolated WITHOUT the field. Legitimate, and the one place the new field truly differs between arms: under single the legacy organization-less admin_full_access row anchors and is in applyingGrants (so it is direct); walled, the row applies nowhere and the 6b-config anchor confers the set, which §6 does not push. The standing (permissions, systemPermissions, positions) is identical across arms, which is what the test pins; the channel is not the standing. Not a hidden change.
    • builtin-positions.boot.test.ts, grant-permission-set-name.equivalence.test.ts, resolve-authz-grant-set-by-name.golden.test.ts: strip the field before comparing to goldens recorded before it existed. Legitimate: these pin the grant envelope, and the stripped field grants nothing. Note, not a finding: extending those goldens would have pinned the split on a real engine; the split is pinned on the recording double in core's new direct-permissions.test.ts and on a real resolver run in explain-position-set-attribution.test.ts and explain-self-direct-grant.test.ts.
    • scripts/engine-double-contract.pinned.json: records the new self-explain double's findOne (which asserts the engine's predicate). A ledger entry the gate itself writes; legitimate.
  9. Gate retirement and gate-roster repairs. .github/workflows/lint.yml loses the one step (both invocations); at the head no other line of the workflow names the step or its id. scripts/check-position-name-fold-loaders.mjs and scripts/measure-position-name-fold-census.mjs deleted; dispatch-gates.self-test.mjs loses the gate's walked-roots block; dispatch-gates.mjs docblock repointed to the two remaining #selfTest anchors; dispatch-gates.data.mjs holds no entry (verified at the head). Right: the gate pinned that the fold's only in-repo route was test-only, and the route is gone. Remaining mentions (scripts/pm/bare-root-worklist.mjs, scripts/measure-reserved-identity-name-census.mjs, scripts/check-workflow-step-name-quoting.mjs fixture strings, docs/audits/*) are comments, fixtures and history.

  10. Test fixtures moved from name-granting to declared or direct grants. First round: security-plugin.test.ts D10 harness now seeds sys_user_permission_set and a fake registry so direct grants resolve through the catalog; controlled-by-parent-chain-middle-master.test.ts; permission-set-resolution-memo.test.ts; guest-grants-channel.test.ts and orgless-position-name-fold.test.ts keep a control that the same set resolves when named. Repair round: packages/verify/src/parent-write-reach.test.ts (pwr_manager, pwr_rep) and packages/cli/test/verify-parent-write-reach.test.ts (vpw_manager) declared positions with a same-named permission set and no permissionSets, so their holders were reaching the set through the fold alone; each position now names its set (permissionSets: ['pwr_manager'] etc.). Right, and a pin move rather than a weakening: the repair diff touches only the fixture declarations, no expect or assertion line moves in either file, so every verdict those suites pin (the parent-write reach of a manager against a rep, the control hook's refusal) is now reached through the declared binding ADR-0131 D3/D4 names, which is exactly the migration the changeset prescribes. These are also the two suites that would have fallen in the then-in-progress Test Core shards on the previous head; on this head all eight shards are green.

  11. Prose-only corrections in resolve-authz-context.ts (repair round: the readDisabledCatalogNames docblock no longer lists the fold as a second reader; readPositionGrantsInForce says the definition is the whole answer; the guest-anchor docblock and the §6a "dropped from positions" comment no longer explain the drop by a fold that requests no set any more). Right: no code line moves in that commit's hunks of this file; the §6a drop itself is kept, for the reason the comment now states (no downstream reader sees a switched-off position as held). This closes the stale-prose flag the first round carried for this file.

  12. HotCRM at the unlock. src/sales/sharing/positions.ts declares permissionSets on its positions (tagged v6.0.0), so its namesake sets arrive in context.permissions and are not probe candidates; its boot pin test/demo-staffing.test.ts:485 (no fold event names a HotCRM position) keeps holding under the new predicate.

② Semver level

  • Changeset .changeset/24757-retire-position-name-fold.md: @objectstack/plugin-security patch, @objectstack/core patch. Matches what the diff publishes: both packages move, no @objectstack/spec file changes, Metadata-protocol: none is true (no key, value, type or default an author writes moves), so patch is the level AGENTS.md assigns, with the BREAKING banner and the ADR-0087 marker (not-required (no-migration-prescription)) present in the body. The banner carries Before / Now / Fix and the cloud ordering (HotCRM v6.0.0 or later first). The repair round changes no published file and needs no changeset change.
  • Clause-②: yes (narrowing: … ; widened: …) on both the PR body and the changeset, read through clause2-line.mjs's arm reader as value yes, arm narrowing (the arm word is the first token inside the parenthetical; the trailing colon is not an identifier character). Right: the diff narrows one surface (the plugin's effective grant set) and widens another (the two core types gain a field), which is exactly the yes (narrowing) case. The record's own face is the narrowing face at CONTRACT_REVIEW_TIER, and the PR's paths also put it there through .github/workflows/lint.yml (governed). One precision for the maintainer: the core field is required, so "additive" is exact for readers of the types and a one-line migration (directPermissions: []) for out-of-repo code that constructs them; the changeset says so, and the previous head's own type-check red is the in-repo instance of that migration.
  • The core change has no stored-artifact or authored-metadata source, so the ADR-0087 not-required disposition covers it as written.

③ Boundary flags

  • Ruling A (additive directPermissions on both core types; explain reads the dual case as direct grant + position:P; PR declares Clause-②: yes (narrowing)): honoured, see ① items 5, 6 and ②.
  • Ruling S1 (self path re-resolves a human principal through resolveUserAuthzGrants and attaches only directPermissions; agent, guest and system unchanged): honoured, see ① item 7.
  • Previous head's Type Check · workspace red: this PR's own (① item 6), repaired on this head by the migration the changeset prescribes; the dev's first-round typecheck reading covered core and plugin-security only and its "other packages passed" line read vitest, not tsc — the workspace lane is what caught it, and it is green here.
  • Dev flag: PositionGrantVia 'name', its docblock, the UI badge and managedEnvironmentUsers.tsx are spec: retire PositionGrantVia 'name' (the position-name fold's grant mark) once #24757's runtime retirement lands #26022's — answered: a published value nothing emits is an acceptable transitional state because no consumer grants on it; spec: retire PositionGrantVia 'name' (the position-name fold's grant mark) once #24757's runtime retirement lands #26022 is open and blocked on this PR and should follow it.
  • Dev flag: stale prose naming the fold as live — the core docblocks are corrected on this head (① item 11). What remains (packages/rest/src/meta-item-read-gate.ts:3376, dogfood comments, fixture strings) is prose only, outside the claim's surface, with no behaviour on it; the rest docblock goes with spec: retire PositionGrantVia 'name' (the position-name fold's grant mark) once #24757's runtime retirement lands #26022. Not a landing blocker.
  • Dev flag: open PR feat(spec,plugin-security,lint): sharing.controlledByParent.ownerMayEdit — a detail row's owner updates it with read on the master #26005 edits security-plugin.ts in other hunks — answered: disjoint regions, the dev's merge-tree read is clean; whichever lands second merges main and re-runs.
  • Dev flag: 3557 changed lines, above the 3000 human-merge line — answered: this PR is Tier H, so the landing is the maintainer's regardless; the size line is the same reader.
  • Dev open_questions: none in either report (6414056485, 6417094296).
  • For the maintainer (not re-decided here): the retirement's cloud ordering. The hold comment 6318977882 measured the cloud on a released HotCRM behind the declaration; the unlock 6392096931 reads hotcrm v6.0.0 as declaring. Before the framework release that carries this PR reaches a cloud environment running HotCRM, that environment must be on HotCRM v6.0.0 or later, or its five same-named positions lose their sets (with the granted: false warning on first resolution, so loudly). The banner names it; the cloud lane's pin is the maintainer's to confirm.
  • Observations carried, no action: the probe's per-resolution cost for held positions with no namesake (① item 2); a set named by two held definitions attributes to the first (① item 5).

Implemented-by: claude/project-thread-6wuxt3
Reviewed-by: session_0186vfjHHMsRXC9EUT9hTFMo

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · domain:server seat 2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-10-26T16:03Z · head df4eb65c483127bc070e49cd30b0ea8685f39eae · #24757, #25799

The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」. This PR is Tier H twice over (.github/workflows/lint.yml is on the governed register, and 3,557 changed lines are over the human-merge line). It carries an APPROVED review from os-zhuang (a governed approver; the rule counts an approval on any commit), and the maintainer said, verbatim, 「24757红了 我批准了」 and 「修正之后可以合并」 (2026-10-26T14:32Z): the reds are now fixed.

  1. Shape: Fixes #24757 and Fixes #25799. Clause-②: yes (narrowing …; widened …) and Metadata-protocol: none are line-initial. One changeset (patch on both packages, BREAKING banner, ADR-0087 marker, the HotCRM v6.0.0 ordering note). check-governed-merges --pr 26064: GOVERNED (Tier H), 3,557 changed lines.
  2. Contract review: 6420762600, CONTRACT_REVIEW_TIER, PASS on this head. The repair since 32fe4f1b (5 files, +20/−13) is fixture-only plus prose: three hand-built envelopes add the new required directPermissions: [], and three test positions now name their permission sets in permissionSets instead of relying on the retired name fold. No assertion moved; those suites now reach the same verdicts through the declared binding.
  3. Gates: at this stamp the head's 47 check-runs read 39 success and 8 skipped (opt-in harnesses and placeholders), with none failing or in progress; Type Check · workspace and the Test Core shards that were red on 32fe4f1b are green; mergeable, clean.
  4. Accepted deviations: none beyond the review's carried notes. PositionGrantVia 'name' stays published but unemitted until spec: retire PositionGrantVia 'name' (the position-name fold's grant mark) once #24757's runtime retirement lands #26022 lands.
  5. Out of scope: the hosted ordering (a hosted environment on HotCRM v6.0.0 or later before its framework pin crosses the release carrying this PR) is the cloud-lane card cloud: move every hosted HotCRM to v6.0.0 or later before the framework pin crosses the release carrying #26064, or its five same-named positions lose their permission sets #26141.
  6. Landing: the approval is on record and the maintainer's words cover the fixed head, so this seat arms auto-merge through the relay, confirms MERGED on both readings, and records the landing on Setup: a position's same-named permission set is in force but invisible — the set's Assigned Users panel is empty and the position reads as granting nothing #24757 and security/explain: sets a position grants through its definition's permissionSets are attributed "via direct grant", not to the position #25799, which the merge closes.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 26, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 40355861805 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

分类:failure —— 按下面的日志分诊。

失败的 job(日志抽取,best effort):

  • Test Core (5/8) — 失败步骤: Run this shard's tests

    @objectstack/plugin-security:test:  FAIL  src/controlled-by-parent-owner-may-edit.test.ts > [#25760] what the opt-in does NOT widen — each shape keeps the master-edit rule > a DELEGATED update by the 
      ↳ 失败原因: @objectstack/plugin-security:test: AssertionError: expected '[Security] Access denied: on-behalf-o…' to contain '[Security] Access denied: update on \…'
    

↳ 失败原因 是判读的关键:超时(Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言(AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError。 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

  • src/controlled-by-parent-owner-may-edit.test.ts — 24h 窗口内只有本 PR 撞到过,暂不汇总(再有一个不同 PR 撞到就会自动开汇总 issue)。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 26 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Queue red, triaged · domain:server seat 2 · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-10-26T16:34Z

Run 40355861805 (Test Core 5/8) failed packages/plugins/plugin-security/src/controlled-by-parent-owner-may-edit.test.ts with an assertion, not a timeout. Signature: new, first sighting. It is this PR's: the file landed on main through #26005 after this branch's base, and its delegator is granted the requester's set by a stored position whose name equals the set's name, the fold this PR retires. So the delegated-update pin now refuses at the on-behalf-of check instead of the master gate. ⛔ Not re-queued. Disposition: a patch round merges main and binds that fixture through a declared permissionSets, with every assertion unchanged; the contract review re-runs on the new head before re-arming.


Generated by Claude Code

…a direct grant (#24757)

controlled-by-parent-owner-may-edit.test.ts landed on main (#26005) after this
branch's base. Its delegator reached omd_requester through a sys_user_position
row named after the set, the fold this PR retires, so the delegated-update pin
refused at the on-behalf-of check instead of the master gate. The row is now a
sys_user_permission_set grant, the same rebinding the chain-middle-master suite
received; every expectation is unchanged.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8LYP8UCJURxbFWZqnywyD
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 0239ff1635ef4879e49ebaea37190839f4f402f6
Local-runs: none
Scope: .github/workflows/lint.yml, packages/cli/test/verify-parent-write-reach.test.ts, packages/core/src/security/assemble-execution-context.test.ts, packages/core/src/security/resolve-authz-context.batch-equivalence.golden.json, packages/core/src/security/resolve-authz-context.batch-equivalence.test.ts, packages/core/src/security/resolve-authz-context.direct-permissions.test.ts, packages/core/src/security/resolve-authz-context.platform-admin-config.test.ts, packages/core/src/security/resolve-authz-context.request-rounds.test.ts, packages/core/src/security/resolve-authz-context.ts, packages/plugins/plugin-audit/src/agent-dual-attribution.integration.test.ts, packages/plugins/plugin-audit/src/comment-access-hooks.test.ts, packages/plugins/plugin-security/src/builtin-positions.boot.test.ts, packages/plugins/plugin-security/src/controlled-by-parent-chain-middle-master.test.ts, packages/plugins/plugin-security/src/controlled-by-parent-owner-may-edit.test.ts, packages/plugins/plugin-security/src/delegated-admin-gate.test.ts, packages/plugins/plugin-security/src/delegated-admin-gate.ts, packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts, packages/plugins/plugin-security/src/describe-position-grants.test.ts, packages/plugins/plugin-security/src/explain-engine.test.ts, packages/plugins/plugin-security/src/explain-engine.ts, packages/plugins/plugin-security/src/explain-position-set-attribution.test.ts, packages/plugins/plugin-security/src/explain-self-direct-grant.test.ts, packages/plugins/plugin-security/src/grant-permission-set-name.equivalence.test.ts, packages/plugins/plugin-security/src/guest-grants-channel.test.ts, packages/plugins/plugin-security/src/orgless-position-name-fold.test.ts, packages/plugins/plugin-security/src/permission-set-active.test.ts, packages/plugins/plugin-security/src/permission-set-resolution-memo.test.ts, packages/plugins/plugin-security/src/position-name-fold-warning.test.ts, packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.golden.test.ts, packages/plugins/plugin-security/src/security-plugin-explain-access.ts, packages/plugins/plugin-security/src/security-plugin-permission-sets.ts, packages/plugins/plugin-security/src/security-plugin.test.ts, packages/plugins/plugin-security/src/security-plugin.ts, packages/verify/src/parent-write-reach.test.ts, scripts/check-position-name-fold-loaders.mjs, scripts/engine-double-contract.pinned.json, scripts/measure-position-name-fold-census.mjs, scripts/pm/dispatch-gates.mjs, scripts/pm/dispatch-gates.self-test.mjs + packages/plugins/plugin-security/src/permission-evaluator.ts (resolvePermissionSets: empty request, loader failure swallowed), packages/spec/src/contracts/security-service.ts (PositionGrantVia), packages/spec/src/security/explain.zod.ts (via: z.string().optional()), packages/spec/src/kernel/execution-context.zod.ts (ExecutionContextSchema is a strip-mode z.object, principalKind optional), packages/core/src/security/assemble-execution-context.ts (principalKind set at the door), packages/core/src/security/index.ts (both types exported), scripts/pm/dispatch-gates.data.mjs (no fold entry, as claimed), objectstack-ai/hotcrm src/sales/sharing/positions.ts and test/demo-staffing.test.ts (the declared bindings and the no-fold-event boot pin), packages/lint/src/validate-security-posture.test.ts and packages/plugins/plugin-auth/src/last-admin-guard.anchor-parity.test.ts (main-side tests since the base that seed position rows: neither grants through a position's name)

Check-runs on the head (read 2026-10-26T17:23Z, all concluded): 47 check-runs, 39 success, 8 skipped (opt-in harnesses and matrix placeholders: 18.x upgrade harness (opt-in), Packed-tarball smoke (opt-in), Temporal Conformance (live PG + MySQL), Build Docs, Lint UI, Dogfood Verify CLI, the Test UI and Dogfood Regression Gate shard placeholders), 0 failed, 0 in progress. Green among them: Build Core, Build UI Deps, Build & E2E UI, all seven Lint · lanes and Lint & Repo Gates, Test Core and its eight shards, Type Check · consumer gates / debt ledger / source gates / workspace, TypeScript Type Check, Type Check UI, Check Changeset, Governed Surface Queue Guard, and the claim and card gates. Type Check · workspace, red on the first head 32fe4f1b, is green here as on df4eb65c.

Reviewed against card #24757 (body and all 18 comments, rulings 6290781571, 「甲」 as amended, unlock 6392096931, claims 6414302166, dev reports 6414056485 and the addendum 6417094296) and card #25799 (body and all 5 comments, seat ruling A and S1 in claims 6414107849 / 6414315157), the PR body (declarations unchanged across the three heads), its 40-file list, the net diff of 0239ff16 against its merge base df8aac41 (+1008 / −2555 over 40 files; −1907 of it the two deleted scripts), the two-commit repair diff 32fe4f1b..df4eb65c (5 files, +20 / −13) and the patch round df4eb65c..0239ff16: one merge of origin/main at df8aac41 (6d0d586a) and one test commit (0239ff16, 1 file, +3 / −3). The merge was checked two ways: every file it changed on the branch side is among main's changes since the old base 1bbb7bf4 (no file outside that set, no conflict marker in the one file both sides touched, security-plugin.ts, whose main-side hunks from #26005 are the imports, CbpRelation and the class body's ownerMayEdit, disjoint from the fold regions); and the PR's own added and removed lines against the new base are the same set as against the old base plus exactly the six lines of the test commit.

① Derived judgments

  1. Resolver request narrows to the context's set names (security-plugin-permission-sets.ts: const requested = [...explicitPermissionSets], baseline pushed after; security-plugin.ts drops the dropDeactivatedFoldedSets method and its GUEST_POSITION import). A held position's NAME no longer resolves a same-named set. Right. This is 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 ruling B as ordered on Setup: a position's same-named permission set is in force but invisible — the set's Assigned Users panel is empty and the position reads as granting nothing #24757 (6290781571: HotCRM declares first, released in hotcrm v6.0.0 per the unlock 6392096931), shipped in 18.x under 「甲」 as amended. The guest arm (positions = isGuest ? [] : …) is kept, so ADR-0138 D3 item 3 holds for the guest by construction rather than by exclusion. The deleted second activation read is correct to delete: every name the plugin now requests arrived already judged by core's readDisabledCatalogNames (§6a/§6b), and permission-set-active.test.ts pins that a held position issues no ledger read. The four deleted pins on the ledger's 0/1 storage shape and type-scoped rows pinned that second reader, which no longer exists; core's own resolver tests parameterise active and metadata_type for the one reader left.

  2. The loud half (reportUngrantedNamesakeSets, event position_name_fold_grant, payload event/position/permissionSet unchanged, removedIn: 'next_major' replaced by granted: false, new text with no tracker id). Right. 要点 5 is honoured: the difference is a warning, never a silent change. The probe resolves the held position names the request did not already name through the same evaluator and the same three sources, and what comes back is never granted (namesakes is only iterated, never merged into permissionSets). The predicate reads requested after the baseline push, so a baseline-named position stays silent, and once-per-name dedupe keys on a name that actually had a namesake. Read in the cited evaluator: an empty candidate list returns [] before any source is read; a catalog-loader failure is swallowed and warned once as a lookup failure, so the probe cannot fail the authorization resolution. removedIn was a log-payload key that shipped on 2026-10-20 (f90ea677c) and nothing in the tree consumes it; dropping it is a log change, not a published surface. Observation, not a finding: a held position with no namesake (everyone, org_member) is re-probed on every unmemoized resolution, which is a second metadata.list('permission') call per such resolution. The fold made equivalent catalog reads for the same names in the one evaluator call; the docblock states the cadence.

  3. describePositionGrants serves via: 'definition' only (security-plugin.ts: resolvePermissionSets(declared, …), via: 'definition' unconditionally). Right, and the test pins that no summary carries any other via. PositionGrantVia in packages/spec/src/contracts/security-service.ts:130 still publishes 'definition' | 'name'; the 'name' member is published and unemitted until spec: retire PositionGrantVia 'name' (the position-name fold's grant mark) once #24757's runtime retirement lands #26022 (open, Blocked-by: #24757) retires it. Nothing reads 'name' to grant anything (the objectui badge renders it; HotCRM and objectui construct neither core type and read no 'name' member at their pins), so the transitional state is sound.

  4. Delegated-admin gate (delegated-admin-gate.ts: setsCarriedByPosition returns [...new Set(listed)]; ownNameNote deleted). Right. A set that only shares the position's name is granted by nothing after item 1, so judging it would refuse assignments that hand out nothing. Both consumers (the organization-level boundary and the delegated walk through setsBoundToPosition) read the one function, and the gate and the resolver read the same security catalog, so there is no window in which the gate admits what the resolver grants. This is a widening of what a delegate may assign relative to main, and it is the correct one; the unit and the three-posture suites each keep a teeth control (a LISTED set outside the allowlist is still refused, a LISTED admin set still cannot be self-delegated).

  5. Explain attribution (explain-engine.ts: positionBindingsOf reads each held position's definition from the security catalog; viaOf says position:P when P's definition names the set, direct grant + position:P when context.directPermissions also names it, position:P (whether a direct grant also names it could not be read) under directPermissionsUnread, direct grant otherwise, and direct grant or position definition (the position definitions could not be read) when the catalog is unreadable). Right. Ruling A is honoured: the dual case names both channels. via is z.string().optional() in explain.zod.ts, so the wording moves no published accept set, and the dev's claim to that effect holds. Two bounded readings: a set named by two held definitions is attributed to the first in held order (the one channel a position grants through; sufficient for the card's distinction), and a context the resolver did not build and that carries no split (an agent acting for a user, a hand-built context without principalKind) reads position:P alone, which is today's reading and what S1 asks for.

  6. Core: directPermissions: string[] on UserAuthzGrants and ResolvedAuthzContext (resolve-authz-context.ts: pushed in the §6 loop over applyingGrants, after the tenant/anchor filter and only when active, never from the §6a boundSetNames loop; initialised [] in all three literals; copied into ctx beside permissions). Right, and it is the split the resolver already computed: applyingGrants is the row set §6 confers from, so the field cannot disagree with permissions about which direct rows applied. Both types are exported from @objectstack/core (security/index.ts:223,225). The field is REQUIRED, not optional: additive for every reader, a compile-time change for code that builds either object as a literal. That is what the previous head's Type Check · workspace red measured: packages/plugins/plugin-audit test literals built envelopes without it, and the repair adds directPermissions: [] to the three literals (comment-access-hooks.test.ts twice, agent-dual-attribution.integration.test.ts once), changing no assertion. Out of the repo, objectstack-ai/hotcrm and objectui at their checkouts build neither type. Required is the right choice (an optional field would let a hand-built context silently read as "no direct grants"), and the changeset states the one-line fix. See ②.

  7. Self path (security-plugin-explain-access.ts: else if (principalKind === 'human' && isSystem !== true && !onBehalfOf?.userId && userId) re-resolves through resolveUserAuthzGrants(this.ql, userId, { tenantId, nowMs, bypassGrantsCache: true }) and attaches only directPermissions, else directPermissionsUnread: true; the context's grants stay the request's). Right. Ruling S1 is honoured: agent, guest, service and system callers take neither branch and keep the context as it arrived; the dev's C2 ablation (re-resolve for every non-system caller) reds the agent pin. The premise holds in the cited spec: ExecutionContextSchema is a strip-mode z.object, so a transport parse drops an unknown directPermissions key, and the split cannot ride the envelope without a spec change. Cost: one extra resolver run per human self-explanation on a diagnostic endpoint, and the spread context is a new object so the per-context set memo misses once; both acceptable for explain. directPermissionsUnread is a private context key read only by viaOf.

  8. Goldens and ledgers. Each exclusion or extension was judged on whether it hides a behaviour change:

    • resolve-authz-context.batch-equivalence.golden.json and its test docblock: the field is ADDED to every fixture and compared (not excluded). The values are consistent with the fixtures' own grant rows (the u_ta fixture lists low and not the position-bound organization_admin; the admin fixture lists both its direct rows). Legitimate; the pin is extended.
    • request-rounds.test.ts and explain-engine.test.ts (buildContextForUser): exact envelopes extended with the field. Legitimate.
    • platform-admin-config.test.ts posture-invariance arm: compares single against isolated WITHOUT the field. Legitimate, and the one place the new field truly differs between arms: under single the legacy organization-less admin_full_access row anchors and is in applyingGrants (so it is direct); walled, the row applies nowhere and the 6b-config anchor confers the set, which §6 does not push. The standing (permissions, systemPermissions, positions) is identical across arms, which is what the test pins; the channel is not the standing. Not a hidden change.
    • builtin-positions.boot.test.ts, grant-permission-set-name.equivalence.test.ts, resolve-authz-grant-set-by-name.golden.test.ts: strip the field before comparing to goldens recorded before it existed. Legitimate: these pin the grant envelope, and the stripped field grants nothing. Note, not a finding: extending those goldens would have pinned the split on a real engine; the split is pinned on the recording double in core's new direct-permissions.test.ts and on a real resolver run in explain-position-set-attribution.test.ts and explain-self-direct-grant.test.ts.
    • scripts/engine-double-contract.pinned.json: records the new self-explain double's findOne (which asserts the engine's predicate). A ledger entry the gate itself writes; legitimate.
  9. Gate retirement and gate-roster repairs. .github/workflows/lint.yml loses the one step (both invocations); at the head no other line of the workflow names the step or its id. scripts/check-position-name-fold-loaders.mjs and scripts/measure-position-name-fold-census.mjs deleted; dispatch-gates.self-test.mjs loses the gate's walked-roots block; dispatch-gates.mjs docblock repointed to the two remaining #selfTest anchors; dispatch-gates.data.mjs holds no entry (verified at the head). Right: the gate pinned that the fold's only in-repo route was test-only, and the route is gone. Remaining mentions (scripts/pm/bare-root-worklist.mjs, scripts/measure-reserved-identity-name-census.mjs, scripts/check-workflow-step-name-quoting.mjs fixture strings, docs/audits/*) are comments, fixtures and history.

  10. Test fixtures moved from name-granting to declared or direct grants. First round: security-plugin.test.ts D10 harness now seeds sys_user_permission_set and a fake registry so direct grants resolve through the catalog; controlled-by-parent-chain-middle-master.test.ts; permission-set-resolution-memo.test.ts; guest-grants-channel.test.ts and orgless-position-name-fold.test.ts keep a control that the same set resolves when named. Repair round: packages/verify/src/parent-write-reach.test.ts (pwr_manager, pwr_rep) and packages/cli/test/verify-parent-write-reach.test.ts (vpw_manager) declared positions with a same-named permission set and no permissionSets, so their holders were reaching the set through the fold alone; each position now names its set (permissionSets: ['pwr_manager'] etc.). Right, and a pin move rather than a weakening: the repair diff touches only the fixture declarations, no expect or assertion line moves in either file, so every verdict those suites pin (the parent-write reach of a manager against a rep, the control hook's refusal) is now reached through the declared binding ADR-0131 D3/D4 names, which is exactly the migration the changeset prescribes. These are also the two suites that would have fallen in the then-in-progress Test Core shards on the previous head; on this head all eight shards are green.

  11. Prose-only corrections in resolve-authz-context.ts (repair round: the readDisabledCatalogNames docblock no longer lists the fold as a second reader; readPositionGrantsInForce says the definition is the whole answer; the guest-anchor docblock and the §6a "dropped from positions" comment no longer explain the drop by a fold that requests no set any more). Right: no code line moves in that commit's hunks of this file; the §6a drop itself is kept, for the reason the comment now states (no downstream reader sees a switched-off position as held). This closes the stale-prose flag the first round carried for this file.

  12. The patch-round rebinding (controlled-by-parent-owner-may-edit.test.ts, landed on main by feat(spec,plugin-security,lint): sharing.controlledByParent.ownerMayEdit — a detail row's owner updates it with read on the master #26005 after this branch's base): the delegator held the omd_requester set through a sys_user_position row naming a position omd_requester that no definition in the suite's catalog declares, so the on-behalf-of link reached the set through the fold alone and, with the fold retired, the delegated-update pin refused at the on-behalf-of check instead of the master gate. The row is now a sys_user_permission_set grant in ORG (ups_delegator), which §6 confers without a position and without membership, exactly the rebinding the chain-middle-master suite received in the first round; the comment is updated and no expect line moves. Right, and a pin move rather than a weakening: the suite's intent (the delegator resolves to the same grants the owner holds, no sys_member row so no ownership floor, the master gate is the one refusal left) is reached through the binding ADR-0131 D3/D4 names, and it no longer depends on the retired fold. The merge brought in nothing else that relied on the fold: main's commits since the base add no line naming position_name_fold, dropDeactivatedFoldedSets, reportNameFoldCollisions, via: 'name' or a positions-into-request spelling, and the two other main-side tests that seed position rows (validate-security-posture.test.ts, a static lint fixture whose position and set names differ; last-admin-guard.anchor-parity.test.ts, which grants through sys_user_permission_set rows) grant nothing through a position's name.

  13. HotCRM at the unlock. src/sales/sharing/positions.ts declares permissionSets on its positions (tagged v6.0.0), so its namesake sets arrive in context.permissions and are not probe candidates; its boot pin test/demo-staffing.test.ts:485 (no fold event names a HotCRM position) keeps holding under the new predicate.

② Semver level

  • Changeset .changeset/24757-retire-position-name-fold.md: @objectstack/plugin-security patch, @objectstack/core patch. Matches what the diff publishes: both packages move, no @objectstack/spec file changes, Metadata-protocol: none is true (no key, value, type or default an author writes moves), so patch is the level AGENTS.md assigns, with the BREAKING banner and the ADR-0087 marker (not-required (no-migration-prescription)) present in the body. The banner carries Before / Now / Fix and the cloud ordering (HotCRM v6.0.0 or later first). The repair round changes no published file and needs no changeset change.
  • Clause-②: yes (narrowing: … ; widened: …) on both the PR body and the changeset, read through clause2-line.mjs's arm reader as value yes, arm narrowing (the arm word is the first token inside the parenthetical; the trailing colon is not an identifier character). Right: the diff narrows one surface (the plugin's effective grant set) and widens another (the two core types gain a field), which is exactly the yes (narrowing) case. The record's own face is the narrowing face at CONTRACT_REVIEW_TIER, and the PR's paths also put it there through .github/workflows/lint.yml (governed). One precision for the maintainer: the core field is required, so "additive" is exact for readers of the types and a one-line migration (directPermissions: []) for out-of-repo code that constructs them; the changeset says so, and the previous head's own type-check red is the in-repo instance of that migration.
  • The core change has no stored-artifact or authored-metadata source, so the ADR-0087 not-required disposition covers it as written.

③ Boundary flags

Implemented-by: claude/project-thread-6wuxt3
Reviewed-by: session_0186vfjHHMsRXC9EUT9hTFMo

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 (re-ACCEPT after the dequeue)

ACCEPT · domain:server seat 2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-10-26T17:25Z · head 0239ff1635ef4879e49ebaea37190839f4f402f6 · #24757, #25799

This supersedes the ACCEPT on df4eb65c for the new head; items 1, 4 and 5 there stand unchanged. The maintainer's standing authorisation, verbatim: 「按照 pm-dispatch 规则复核过的都授权」; on this PR, verbatim, 「24757红了 我批准了」 and 「修正之后可以合并」 (2026-10-26T14:32Z), with os-zhuang's APPROVED review on record (Tier H: governed workflow path and 3,563 changed lines).

  1. Contract review: 6422799890, CONTRACT_REVIEW_TIER, PASS on this head. The queue ejection came from controlled-by-parent-owner-may-edit.test.ts (added by feat(spec,plugin-security,lint): sharing.controlledByParent.ownerMayEdit — a detail row's owner updates it with read on the master #26005), whose delegator reached its set through the name fold this PR retires. The patch round merges main at df8aac41 (clean, no rebase) and rebinds that one fixture row to a direct permission-set grant, the same move the chain-middle-master suite got in round one. No expect line moves. Nothing else on main since the base relies on the fold.
  2. Gates: at this stamp the head's 47 check-runs read 39 success and 8 skipped (opt-in harnesses, UI and docs families, matrix placeholders), with none failing or in progress; Test Core 1–8, every Type Check · lane and the Governed Surface Queue Guard are green.
  3. Landing: the approval is on record and the maintainer's words cover the fixed head, so this seat arms auto-merge through the relay, confirms MERGED on both readings, and records the landing on Setup: a position's same-named permission set is in force but invisible — the set's Assigned Users panel is empty and the position reads as granting nothing #24757 and security/explain: sets a position grants through its definition's permissionSets are attributed "via direct grant", not to the position #25799. The queue-flake anchor Queue-flake anchor: src/controlled-by-parent-owner-may-edit.test.ts #26247 names this file; its card thread measures at this head and stays serial behind this merge.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit 733b3c8 Oct 26, 2026
47 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-6wuxt3 branch October 26, 2026 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

3 participants