Repository navigation
Commit 733b3c8
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XAT2rj3G1muSjHR3CQ5LksT)_
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 #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 #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:
- `packages/core/src/security/resolve-authz-context.ts:1124`, `:1307`,
`:1544` (docblocks naming the position-name fold).
- `packages/rest/src/meta-item-read-gate.ts:3376` (`via: 'definition' |
'name'` docblock; the `'name'` half goes with #26022).
- `packages/qa/dogfood/test/guest-grants-channel.dogfood.test.ts:15`,
`:109` (comments on "the position-name fold's only route").
- `scripts/check-workflow-step-name-quoting.mjs:388`, `:409`: inline
fixtures that spell the deleted gate's command. These are fixture
strings and are not executed.
- `scripts/pm/bare-root-worklist.mjs`,
`scripts/measure-reserved-identity-name-census.mjs`, `docs/audits/*`:
historical mentions of the deleted scripts.
- `plugin-security/src/permission-set-name-collision.ts` and
`cli/src/utils/permission-set-name-collisions.ts`: measured. These are
the cross-package set-name collision (#17516), not the fold, so they are
unchanged.
## 维护者速读(草稿)
**改了什么**
以前,只要一个权限集和某个职位同名,持有这个职位的人就会自动拿到这个权限集,哪怕职位定义里根本没写它。现在取消了这条"同名自动授权"的暗道:职位只授予它定义里
`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'`
这个取值,界面上也还有"(同名)"标记,但运行时已不再产生它。这部分由 #26022 收尾,在它合入前,这个已发布的取值不会再出现。
- `@objectstack/core` 公开类型新增 `directPermissions`
字段,属于纯新增。手写这两个类型对象的代码需要补上 `directPermissions: []`。
- 自查自己的权限时,会为真人用户多做一次授权解析,只用于诊断,不影响实际授权。
- 回滚:直接 revert 这个 PR 即可恢复同名授权,不涉及数据迁移。
**席位意见**
**你要做的**
审阅后决定是否批准合并(Tier H,改动了 `.github/workflows/lint.yml`,改动行数约 3500 行,超过 3000
行人工合并阈值,其中约 1900 行是删除的两个脚本)。合并后请让 #26022 跟进规范和界面的那一半。
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01J8LYP8UCJURxbFWZqnywyD
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9598405 commit 733b3c8
40 files changed
Lines changed: 1008 additions & 2555 deletions
File tree
- .changeset
- .github/workflows
- packages
- cli/test
- core/src/security
- plugins
- plugin-audit/src
- plugin-security/src
- verify/src
- scripts
- pm
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
947 | 947 | | |
948 | 948 | | |
949 | 949 | | |
950 | | - | |
951 | | - | |
952 | | - | |
953 | | - | |
954 | | - | |
955 | | - | |
956 | | - | |
957 | | - | |
958 | | - | |
959 | | - | |
960 | | - | |
961 | | - | |
962 | | - | |
963 | | - | |
964 | | - | |
965 | | - | |
966 | | - | |
967 | | - | |
968 | | - | |
969 | | - | |
970 | | - | |
971 | | - | |
972 | | - | |
973 | 950 | | |
974 | 951 | | |
975 | 952 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
74 | | - | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
75 | 77 | | |
76 | 78 | | |
77 | 79 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
141 | 141 | | |
142 | 142 | | |
143 | 143 | | |
| 144 | + | |
144 | 145 | | |
145 | 146 | | |
146 | 147 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| |||
75 | 76 | | |
76 | 77 | | |
77 | 78 | | |
| 79 | + | |
78 | 80 | | |
79 | 81 | | |
80 | 82 | | |
| |||
156 | 158 | | |
157 | 159 | | |
158 | 160 | | |
| 161 | + | |
159 | 162 | | |
160 | 163 | | |
161 | 164 | | |
| |||
241 | 244 | | |
242 | 245 | | |
243 | 246 | | |
| 247 | + | |
244 | 248 | | |
245 | 249 | | |
246 | 250 | | |
| |||
337 | 341 | | |
338 | 342 | | |
339 | 343 | | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
340 | 348 | | |
341 | 349 | | |
342 | 350 | | |
| |||
436 | 444 | | |
437 | 445 | | |
438 | 446 | | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
439 | 450 | | |
440 | 451 | | |
441 | 452 | | |
| |||
523 | 534 | | |
524 | 535 | | |
525 | 536 | | |
| 537 | + | |
526 | 538 | | |
527 | 539 | | |
528 | 540 | | |
| |||
602 | 614 | | |
603 | 615 | | |
604 | 616 | | |
| 617 | + | |
605 | 618 | | |
606 | 619 | | |
607 | 620 | | |
| |||
676 | 689 | | |
677 | 690 | | |
678 | 691 | | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
679 | 695 | | |
680 | 696 | | |
681 | 697 | | |
| |||
749 | 765 | | |
750 | 766 | | |
751 | 767 | | |
| 768 | + | |
752 | 769 | | |
753 | 770 | | |
754 | 771 | | |
| |||
2029 | 2046 | | |
2030 | 2047 | | |
2031 | 2048 | | |
| 2049 | + | |
2032 | 2050 | | |
2033 | 2051 | | |
2034 | 2052 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
114 | 114 | | |
115 | 115 | | |
116 | 116 | | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
117 | 121 | | |
118 | 122 | | |
119 | 123 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
636 | 636 | | |
637 | 637 | | |
638 | 638 | | |
639 | | - | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
640 | 646 | | |
641 | 647 | | |
642 | 648 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
| 97 | + | |
97 | 98 | | |
98 | 99 | | |
99 | 100 | | |
| |||
0 commit comments