Skip to content

Commit f90ea67

Browse files
fix(plugin-security): the position-name fold warning says the same-name grant is removed in the next major (#24988)
Part of #24757 Clause-②: no (a runtime warning's wording plus one added field in its log payload; no accept set, served shape, resolution result or error code moves) This is the card's **18.x half** under the standing rule 「甲」: the existing `position_name_fold_grant` runtime warning in `plugin-security`, re-pointed as the loud half of the position-name fold's retirement. The fold itself stays. Its removal is the card's 19.0 half, and #24757 remains open for it. ## What changed - **Text.** The warning now says three things. ① Permission set `P` is in force for holders of position `P` only because the two share a name; `P`'s definition does not name it in `permissionSets`. ② This name-based grant is deprecated and is removed in the next major version. From then on, holders of `P` do not receive set `P` unless the position's definition names it. ③ The way out: `permissionSets: ['P']` on position `P` to keep the grant, or a rename if the shared name is a coincidence. The string carries no tracker id and no version number (`check:doc-authoring`; the docblock says why). - **Payload.** `{ event, position, permissionSet }` gains `removedIn: 'next_major'`. The three existing fields are unchanged, because a log consumer may key on them. - **Docblocks.** `POSITION_NAME_FOLD_EVENT` and `reportNameFoldCollisions` now say this is the 18.x loud half under 「甲」 and cite the ruling records. The stale "junction table is the governed channel" wording is replaced with ADR-0131's definition `permissionSets`. The "HotCRM NOT MEASURED" boundary note is replaced with the card's measurement (5 of 12 positions at `f22dfa52`, declared by objectstack-ai/hotcrm#2200). A "When it fires" section records the cadence measured below. - **Unchanged.** The trigger predicate, the once-per-position cadence, and every resolution result. A holder of `P` still receives set `P` in 18.x, as 要点 5 of ruling B on #13419 requires: 「任何行为差异只能表现为拒绝/告警,⛔ 永不静默改变解析结果」. ## 「甲」 reading - **The rule** (#6015, ruling record `6292005854`), verbatim: "No conversion can carry it: 18.x ships only the loud half (a warning, an explain line, a reported event). The refusal is filed for 19.0." - **Triage's derivation for this card** (`6292072572`), verbatim: "**18.x:** objectstack-ai/hotcrm#2200 (HotCRM declares `permissionSets` on its five positions), and the existing runtime warning re-pointed as the loud half. **19.0:** the retirement itself: the fold is removed, and the delegation walk, explain and the collision check follow it." - **Ships in 18.x (this PR):** the warning text and its reported event (the structured payload with `removedIn`). The rule lists three loud-half forms; triage's derivation for this card names the warning. `explain-engine.ts` is fenced out of this claim, so no explain line changes here. - **Ships in 18.x, in HotCRM:** objectstack-ai/hotcrm#2200, HotCRM's half. It declares `permissionSets` on its five same-name positions. That silences this warning for them (pinned below as the declared-pair case) and makes Setup's two panels show the binding. - **Waits for 19.0, on this card:** deleting the position-name request in `resolvePermissionSetsForContextUnmemoized` and `dropDeactivatedFoldedSets`, and the follow-ups in the delegated walk, explain and the collision gate. That half deletes this warning together with the fold. ## Measured (PM mechanism assumptions) Every reading below is from `origin/main` `b811c7369`, this branch's base, by source reading. - **The warning exists as assumed.** `reportNameFoldCollisions` emits `position_name_fold_grant` through `logger.warn` with `{ event, position, permissionSet }`. It reports a pair `(position P, set P)` only when set `P` resolved, `P` is not among the explicitly requested sets (the definition's `permissionSets` and direct grants, §6b), and `P` is not a baseline name. It shipped in 17.3.0 (changeset `936aa2d` in the package CHANGELOG). - **Assumed gap confirmed.** The old text was "This grant is in force and ungoverned: no position declares it. Declare it explicitly … or rename one of the two." It said nothing about the retirement or what holders lose. - **Cadence.** Lazy: it fires on the first resolution that meets an undeclared pair, never at boot. It fires once per position NAME per plugin instance (`nameFoldWarned`). That means not per holder and not per organization, because the key is the name alone. A restart reports again. - **Consumers of the payload.** None beyond the log line. `git grep position_name_fold_grant` finds only the pin and docblock/CHANGELOG prose. The core logger (`packages/core/src/logger.ts` `write`) spreads `meta` into the JSON line, so the new field arrives in the log as a top-level key beside `event`. No new channel, endpoint or gate is added. ## Tests `packages/plugins/plugin-security/src/position-name-fold-warning.test.ts`, 29 cases (28 before): - **MUST FIRE, for the census's two cross_scope folds** (`sales_rep`, `sales_manager`, each bound to `crm_sales_user`): the payload is exactly `{ event, position, permissionSet, removedIn: 'next_major' }`, and ⛔ the holder still resolves the same-name set. - **New: the payload as data.** The exact key set is `event`, `permissionSet`, `position` and `removedIn`, so a renamed legacy field fails as loudly as a missing retirement field. - **The text** pins clauses ① ② ③ above, no `sys_position_permission_set`, and no `#` followed by a digit. - **Once per position:** three distinct contexts give one report, and each request still resolves `sales_rep`. - **MUST NOT FIRE (unchanged cases):** the 7 inert positions, the `org_admin` near-miss, the 13 declared bindings, a declared pair (`permissions: ['sales_rep', …]`, the shape hotcrm#2200 produces), a baseline-named position, and no positions. **Ablations**, each through `scripts/ablation-replace.mjs` in WRAP mode, run against the warning file. For each one the tool's own evidence reads `ok mutation landed` and `ok restored: blob == HEAD (5db0a1f) and git diff HEAD is empty`. The plugin's tests import `./security-plugin.js` from `src`, so no `dist/` leg applies. | leg | mutation (forbidden behaviour put back) | red | |:--|:--|:--| | A1 | drop `removedIn` from the payload | 4 (both MUST FIRE, payload keys, once) | | A2 | put back the 17.x "in force and ungoverned" wording instead of the retirement sentence | 1 (text) | | A3 | a tracker id in the string | 1 (text) | | A4 | stop treating explicitly requested sets as declared | 1 (declared pair) | | A5 | stop treating baseline names as declared | 1 (baseline) | | A6 | drop the once-per-position dedupe | 1 (once) | | A7 | predicate "is this position bound to anything?" | 2 (both bound-to-another-set folds) | | A8 | remove the position-name request (the 19.0 removal early) | 5 (every MUST FIRE case) | | A9 | keep the warning but drop the undeclared same-name set after it (the refusal shipped early) | 4, each on the grant assertion: `expected [ 'crm_sales_user', 'member_default' ] to deeply equal [ …(3) ]` and `expected [ 'member_default' ] to include 'sales_rep'` | A9's first attempt was a no-op: its replacement contained the anchor, and the tool refused with "the anchor count moved 1 -> 1" and restored. The leg was re-run with a replacement that does not contain the anchor. The table reports the second run. **Suite and typecheck**, at `63246cedf` (the merge of `origin/main` that followed touched only `plugin-approvals`, outside this package's dependency and devDependency closure): - `pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2`: 197 files passed, 4078 tests passed, 45 skipped. - `pnpm --filter @objectstack/plugin-security run typecheck` (tsc, the scripts project and `check:test-typecheck`): exit 0, with `0 file(s) / 0 error(s) / 0 pinned signature(s)` in `test-typecheck-debt.json`. The test file is in the `tsconfig.test.json` program (`--listFilesOnly`: 1). ## Gates These were run locally at `e41097d23`, the head this PR opens on, from the `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derivation at that head: 69 commands, the same 69 the PM's reading at `6440cf163` listed. Each exit code was captured before any pipe. - **67 exit 0.** Among them: `check:doc-authoring` (sibling-package prose ids hold the empty baseline), `check:nul-bytes`, `check:position-name-fold-loaders` and its `--self-test`, `check:issue-citations`, `check:changeset-no-major` and `check:adr-0087-registration` (both `--base origin/main`), `check:empty-changeset`, `check:published-files`, `check:dts-closure`, `check:test-source-alias`, `check:type-check-coverage` and `check:type-check-debt`. - **2 exit 3, PREREQUISITE NOT MET: NOT MEASURED locally.** `pnpm check:dual-build-cjs-loads` and `pnpm check:i18n` read every package's `dist/`. This worktree built only `plugin-security` and its dependency closure. CI's fresh build runs both. - **Reconciliation.** `dispatch-gates --ran` with the exit-coded record: "69 derived famil(ies) accounted for — 67 run, 2 NOT-MEASURED (2 DERIVED from a recorded exit 3)". - **Roster gates with a roster beside this diff,** read rather than trusted silent: `check-changeset-fixed.mjs`, `check:authz-resolver`, `check:error-code-casing` and `check:filter-alias-parity`, all exit 0. - **Left to CI:** the rest of the farm (`pnpm lint`, the type-check lanes, Test Core, Build Core). ## Acceptance notes - **Per-name cadence across organizations (observation, not filed).** `nameFoldWarned` keys on the position name alone and the payload names no organization. On a multi-organization deployment, an undeclared pair `P` in a second organization is not reported until restart once the first organization's `P` has warned. This is not reproduced through a public door; it is a source reading. Carrier: #24757's 19.0 half, which removes the warning. - **Lazy firing (observation).** The warning speaks only when a holder of `P` makes a request. A deployment where no such request has happened since boot has not been told. The readings report on the card noted that a boot-time pass over the registry would be the cheaper place for this. That would be a new trigger, outside this dispatch's "keep the trigger conditions". Carrier: #24757's 19.0 half. - **Stale cross-reference outside the claimed regions.** The `nameFoldWarned` docblock links `reportNameFoldCollision` (singular), which names no member. It is outside this claim's file surface, so it is left as it is. - **Explain line.** 「甲」 lists "an explain line" among the loud-half forms. Triage's derivation for this card names only the warning, and `explain-engine.ts` is fenced out of this claim, so this PR ships the warning and its reported event. --- _Generated by [Claude Code](https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent e4d604e commit f90ea67

3 files changed

Lines changed: 170 additions & 53 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security): the `position_name_fold_grant` warning says the same-name grant is removed in the next major version, and what holders lose then
6+
7+
Clause-②: no (a runtime warning's wording plus one added field in its log payload; no accept set, served shape, resolution result or error code moves)
8+
9+
- **What it was.** A position resolves the permission set that has its own name even when the position's definition does not list that set in `permissionSets`. `plugin-security` has warned about this since 17.3 (`position_name_fold_grant`), once per position. The warning said the grant was "in force and ungoverned" and asked for `permissionSets: ['P']` or a rename. It did not say that this name-based grant is being retired, or what happens to holders when it is.
10+
- **What changed.** The warning now says three things. The set is in force for holders of position `P` only because the two share a name. This name-based grant is deprecated and is removed in the next major version: from then on, holders of `P` do not receive set `P` unless `P`'s definition names it. To keep the grant, declare `permissionSets: ['P']` on position `P`; if the shared name is a coincidence, rename one of the two. The structured payload adds `removedIn: 'next_major'`. `event`, `position` and `permissionSet` are unchanged.
11+
- **Why only a warning in 18.x.** No ADR-0087 conversion can carry this retirement: the platform cannot write `permissionSets` into a sealed managed position definition (ADR-0131 D6). So 18.x ships the warning, and the removal of the name-based grant waits for 19.0.
12+
- **Unchanged.** Every resolution result. A holder of `P` still receives set `P` in 18.x. The warning still fires under the same conditions, including for a position bound to a different set that also shares its name with one. It stays silent for a declared pair, a baseline set and a position whose name matches no set, and it still fires once per position name until the next restart.
13+
- **What to do.** For each position named in a `position_name_fold_grant` warning, add the same-named set to that position's `permissionSets` before upgrading to the next major version. Setup's Permission Sets field and the set's Assigned Users panel then show the binding as well.

‎packages/plugins/plugin-security/src/position-name-fold-warning.test.ts‎

Lines changed: 81 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,32 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
/**
4-
* [#13419 执行要点 3, warning half] The position-name fold, said out loud.
4+
* [#13419 执行要点 3; #24757 under 「甲」] The position-name fold, said out loud —
5+
* and, since 18.x, said as the loud half of its retirement.
56
*
67
* ## What is pinned
78
*
89
* `resolvePermissionSetsForContextUnmemoized` requests
910
* `[...positions, ...explicitPermissionSets]`, so a POSITION name resolves a
10-
* same-named PERMISSION SET with no `sys_position_permission_set` row behind
11-
* it. The maintainer ruling (2026-08-31, 「同意」) makes the junction table the
12-
* one governed channel; 要点 5 permits a warning and nothing else until the fold
13-
* itself is deleted — 「任何行为差异只能表现为拒绝/告警,永不静默改变解析结果」.
11+
* same-named PERMISSION SET that the position's definition does not name in
12+
* `permissionSets` (ADR-0131 D3/D4: the definition is the binding). The
13+
* maintainer ruling on #13419 (2026-08-31, 「同意」 to B) retires the fold; 要点 5
14+
* permits a warning and nothing else until the fold itself is deleted —
15+
* 「任何行为差异只能表现为拒绝/告警,永不静默改变解析结果」.
16+
*
17+
* The standing ruling 「甲」 (ruling record 6292005854 on #6015) places that
18+
* deletion in 19.0, because no ADR-0087 conversion can carry it, and ships its
19+
* loud half in 18.x. This file pins the loud half in four directions:
20+
*
21+
* - the TEXT says the grant is in force only because the names match, that
22+
* the name-based grant is removed in the next major version and what
23+
* holders lose then, and the fix (`permissionSets: ['P']`, or a rename);
24+
* - the PAYLOAD carries the consequence as `removedIn: 'next_major'` beside
25+
* its unchanged `event` / `position` / `permissionSet`, and no other key;
26+
* - the TRIGGER and CADENCE are unchanged: an undeclared pair warns, also
27+
* for a position bound to some OTHER set; a declared pair, a baseline name
28+
* and an inert position stay silent; once per position, not per request;
29+
* - ⛔ the GRANT is unchanged: the holder still resolves set `P`.
1430
*
1531
* ## Where the tuples come from
1632
*
@@ -19,9 +35,12 @@
1935
* below are that classification transcribed. This file pins the runtime
2036
* PREDICATE against that classification in BOTH directions; the census pins the
2137
* classification against the repository. Neither substitutes for the other.
38+
* The census called the declared bindings "junction" rows; since ADR-0131 the
39+
* same pairs are the definitions' `permissionSets`, which reach this resolver
40+
* as set names in `context.permissions` — the shape every case below feeds.
2241
*
2342
* ⚠️ `sales_rep` and `sales_manager` appear in BOTH the fold list and the
24-
* junction list, and that is the finding slice 1 exists for: each is bound to
43+
* binding list, and that is the finding slice 1 exists for: each is bound to
2544
* `crm_sales_user`, and is folded onto its own same-name set anyway. A
2645
* predicate that asked "is this position bound to anything?" would report
2746
* neither of the repository's two real folds while looking complete.
@@ -46,15 +65,15 @@ import type { ISecurityService } from '@objectstack/spec/contracts';
4665
const EVENT = 'position_name_fold_grant';
4766

4867
/**
49-
* The census's `NAME-FOLD DEPENDENCIES` block — grants in force with no
50-
* junction row. Position half declared by `examples/app-crm/src/security/
68+
* The census's `NAME-FOLD DEPENDENCIES` block — grants in force that no
69+
* position definition declares. Position half declared by `examples/app-crm/src/security/
5170
* sales-positions.ts`; permission-set half by the vendored HotCRM artifact.
5271
*/
5372
const CENSUS_NAME_FOLDS = ['sales_rep', 'sales_manager'] as const;
5473

5574
/**
56-
* The census's `JUNCTION BINDINGS` block — 13 rows, the governed channel. Every
57-
* one binds a position to a DIFFERENTLY named set, which is why none of them is
75+
* The census's `JUNCTION BINDINGS` block — 13 rows, the governed channel
76+
* (since ADR-0131, the definitions' `permissionSets`). Every one binds a position to a DIFFERENTLY named set, which is why none of them is
5877
* a fold: the fold is about a position's own name.
5978
*/
6079
const CENSUS_JUNCTION_BINDINGS: ReadonlyArray<readonly [position: string, set: string]> = [
@@ -139,38 +158,64 @@ async function resolveWith(context: Record<string, unknown>, universe: Permissio
139158
return { sets: (sets ?? []).map((s) => s.name), events, warn, svc };
140159
}
141160

142-
describe('[#13419] MUST FIRE — a position folded onto its own same-name set with no junction row', () => {
161+
/** Every key the structured report carries — the three it has always carried,
162+
* which a log consumer may key on, plus the retirement (「甲」, 18.x). */
163+
const PAYLOAD_KEYS = ['event', 'permissionSet', 'position', 'removedIn'];
164+
165+
describe('[#13419] MUST FIRE — a position folded onto its own same-name set that its definition does not name', () => {
143166
for (const position of CENSUS_NAME_FOLDS) {
144-
it(`warns for '${position}', the census's own cross_scope fold`, async () => {
145-
// The measured shape: the position IS junction-bound — to `crm_sales_user`,
146-
// not to itself — so `permissions` carries that other set. The grant on
147-
// the same-name set has no junction row behind it at all.
167+
it(`warns for '${position}', the census's own cross_scope fold, and still grants the set`, async () => {
168+
// The measured shape: the position IS bound — to `crm_sales_user`, not
169+
// to itself — so `permissions` carries that other set. The grant on the
170+
// same-name set has no declaration behind it at all.
148171
const { sets, events } = await resolveWith({
149172
userId: 'u1',
150173
positions: [position],
151174
permissions: ['crm_sales_user'],
152175
});
153176

154-
// Reported once, naming both halves of the pair.
177+
// Reported once, naming both halves of the pair and the retirement.
155178
expect(events).toHaveLength(1);
156-
expect(events[0]).toMatchObject({ event: EVENT, position, permissionSet: position });
179+
expect(events[0]).toEqual({ event: EVENT, position, permissionSet: position, removedIn: 'next_major' });
157180

158181
// ⛔ Purely additive: the fold still grants exactly what it granted before.
159-
// A warning that also changed the answer would be the silent behaviour
160-
// change 要点 5 forbids.
182+
// 「甲」 moves the removal to 19.0, so in 18.x the holder still resolves
183+
// set `P`. A warning that also changed the answer would be the silent
184+
// behaviour change 要点 5 forbids.
161185
expect(sets.sort()).toEqual(['crm_sales_user', 'member_default', position].sort());
162186
});
163187
}
164188

165-
it('names the ungoverned grant and the two ways out, not just the collision', async () => {
189+
it('carries the consequence as data: the unchanged fields plus removedIn, and no other key', async () => {
190+
const { events, sets } = await resolveWith({ userId: 'u1', positions: ['sales_rep'], permissions: [] });
191+
expect(events).toHaveLength(1);
192+
// Exact keys, so a renamed or dropped legacy field fails here as loudly as
193+
// a missing retirement field does.
194+
expect(Object.keys(events[0]).sort()).toEqual(PAYLOAD_KEYS);
195+
expect(events[0].removedIn).toBe('next_major');
196+
expect(sets.sort()).toEqual(['member_default', 'sales_rep'].sort());
197+
});
198+
199+
it('says why the grant is in force, that it is removed in the next major and what holders lose, and the fix', async () => {
166200
const { warn } = await resolveWith({ userId: 'u1', positions: ['sales_rep'], permissions: [] });
167201
const message = warn.mock.calls.map((c) => String(c[0])).find((m) => m.includes(EVENT))!;
168-
// [ADR-0131 D3/D4] The governed way out is the position definition's
169-
// `permissionSets` — a junction row no longer grants or governs anything.
202+
// ① In force today only because the names match.
203+
expect(message).toContain(
204+
"permission set 'sales_rep' is in force for holders of position 'sales_rep' only because the two share a name",
205+
);
206+
// ② Deprecated, removed in the next major, and what holders lose then.
207+
expect(message).toContain('This name-based grant is deprecated and is removed in the next major version');
208+
expect(message).toContain(
209+
"holders of position 'sales_rep' do not receive permission set 'sales_rep' unless the position's definition names it",
210+
);
211+
// ③ The fix. [ADR-0131 D3/D4] The governed way out is the position
212+
// definition's `permissionSets` — a junction row no longer grants or
213+
// governs anything.
170214
expect(message).toContain("permissionSets: ['sales_rep']");
215+
expect(message).toMatch(/rename one of the two/);
171216
expect(message).not.toContain('sys_position_permission_set');
172-
expect(message).toContain('ungoverned');
173-
expect(message).toMatch(/rename/i);
217+
// An operator reading the log has no tracker to resolve an issue id against.
218+
expect(message).not.toMatch(/#\d/);
174219
});
175220

176221
it('is LOUD ONCE per position, not once per request', async () => {
@@ -180,11 +225,14 @@ describe('[#13419] MUST FIRE — a position folded onto its own same-name set wi
180225
const svc = ctx.registerService.mock.calls.find((c: any[]) => c[0] === 'security')?.[1] as Partial<ISecurityService>;
181226
// Distinct context OBJECTS, so the per-context memo cannot be what silences
182227
// the second call — the deduplication under test has to be the one in the
183-
// reporter.
228+
// reporter. Each request still resolves the same-name set.
184229
for (let i = 0; i < 3; i++) {
185-
await svc.resolvePermissionSetsForContext?.({ userId: `u${i}`, positions: ['sales_rep'] } as any);
230+
const sets = await svc.resolvePermissionSetsForContext?.({ userId: `u${i}`, positions: ['sales_rep'] } as any);
231+
expect((sets ?? []).map((s) => s.name)).toContain('sales_rep');
186232
}
187-
expect(warn.mock.calls.filter((c) => String(c[0]).includes(EVENT))).toHaveLength(1);
233+
const reports = warn.mock.calls.filter((c) => String(c[0]).includes(EVENT));
234+
expect(reports).toHaveLength(1);
235+
expect(reports[0][1]).toMatchObject({ position: 'sales_rep', removedIn: 'next_major' });
188236
});
189237
});
190238

@@ -219,10 +267,12 @@ describe('[#13419] ⛔ MUST NOT FIRE — the census groups the ruling protects',
219267
},
220268
);
221269

222-
it('stays silent once 要点 2 materialises the pair (position N, set N)', async () => {
223-
// The exact row the ruling's 要点 2 would create. This is the forward pin:
224-
// when materialisation lands, the warning must retire itself for the pairs
225-
// it covers rather than needing a second edit.
270+
it('stays silent once the definition declares the pair (position N: permissionSets: [N])', async () => {
271+
// The fix the warning prescribes, and what objectstack-ai/hotcrm#2200 does
272+
// for HotCRM's five: the definition's `permissionSets` names the same-name
273+
// set, so it arrives in `permissions` (§6b) and the grant no longer depends
274+
// on the fold. This is the forward pin: a declared pair is silent without a
275+
// second edit here.
226276
const { events, sets } = await resolveWith({
227277
userId: 'u1',
228278
positions: ['sales_rep'],

0 commit comments

Comments
 (0)