Skip to content

Commit 88a072e

Browse files
os-billclaude
andauthored
fix(spec): refuse a depth axis declared beside the super-user bit that short-circuits it (#16870) (#17889)
Fixes #16870 Clause-②: no — SET BY THE SEAT AFTER MEASUREMENT, and the earlier `yes` is withdrawn with its reason. The rule is `references/lanes/spec.md` — "放宽接受集或扩大公开面的卡,不论多小,即条款②;收窄仍是语义面,⛔ 不触条款②" — and `references/contract-review.md` line 13 — "新导出符号或已发布载荷上的新键恒 `yes`". This diff is a pure accept-set NARROWING with zero new exported symbols and zero new keys, both legs measured by the round with lit controls (a dummy `export const` in the same file DOES move `api-surface/security.json` and `export-origins/security.json`; one extra optional key on the same shape DOES turn `check:authorable-surface` red naming it). The seat verified the export leg itself: the only added line in the whole diff containing the token `export` is prose inside a comment. ⚠️ The seat's original `yes` rested on "narrowing is also moving a declared surface", which is NOT a rule in this repo. ⭐ An at-tier contract review is still being run on this PR — as a SEAT ELECTION for a BREAKING refusal on a permission surface, ⛔ not because clause ② requires one. ## The defect, reproduced on this branch point An object permission could carry `readScope` beside `viewAllRecords: true`. `PermissionEvaluator.getEffectiveScope` answers `org` on the super-user bit **before** it consults the depth key (`permission-evaluator.ts:264` ahead of the read at `:266`), and `getDeclaredScope` — the ADR-0090 D10 delegated-path input — carries the identical short-circuit at `:305` ahead of the identical read at `:307`. So the declared narrowing is unread on the direct read path **and** dropped from the delegation fold. The reading preserved by triage is confirmed, unchanged, on `272c04b460`; nothing in `plugin-security` is touched by this PR. Yet the declaration was accepted with zero diagnostics, materialised into `sys_permission_set.object_permissions`, and counted by a capability census reading the deployed shape as coverage. Measured on the branch point before any edit: ``` ObjectPermissionSchema.parse({ allowRead: true, viewAllRecords: true, readScope: 'own_and_reports' }) -> ACCEPTED { …, viewAllRecords: true, readScope: 'own_and_reports' } PermissionSetSchema.parse({ name: 'sales_manager', objects: { crm_opportunity: { …the pair… } } }) -> ACCEPTED ``` ## Escalation condition — re-measured on today's tree, still **No** Triage's rule: if any shipped or seeded permission set carries the pair today, this is p1 on the maintainer floor. Re-taken on `272c04b460` (the earlier reading was on `ecdfc9411`), **0**, with a lit control on every probe: | probe | result | control that could have come back the other way | |:--|--:|:--| | structural `jq` walk, every tracked `.json` (536 files) | **0** pair nodes | 130 nodes declare `viewAllRecords`, **53 of them `true`**; 18 declare `readScope` | | `examples/app-showcase/access-matrix.json` | **0** | 47 / 5 nodes, 4 with `viewAllRecords: true` | | `examples/app-crm/access-matrix.json` | **0** | 7 / 0 nodes — ⚠️ the left leg is empty here, so this file's zero is weak and is carried by the others | | `packages/metadata/src/__fixtures__/hotcrm-17.1-built-permissions.artifact.json` | **0** | 75 / 12 nodes, 49 with `viewAllRecords: true` | | shipped defaults, `packages/plugins/plugin-security/src/objects/default-permission-sets.ts` | **0** | 8 `viewAllRecords` tokens; the single `readScope` token is prose inside a comment | | seeded example, `examples/app-showcase/src/security/permission-sets.ts` | **0** | 4 `viewAllRecords: true` grants and 5 `readScope` grants, disjoint | | same-literal brace-local sweep, every tracked `.ts` / `.json` | **2 hits, one file** | **133** brace-local `viewAllRecords: true` literals matched — the regex is not dead | | nesting-tolerant pass (both tokens within 400 chars) | 6 candidate files | each read by hand: in all six the two tokens are in **separate permission sets or separate objects**, never one object permission | | FORMAT control — `*.yml` / `*.yaml` | 0 files carry either token | 38 yaml files carry `permission`, so the sweep can see yaml | The only same-literal pair in the tree is `packages/spec/src/security/explain.test.ts`, which is the guard below, not an instance of the defect. ⇒ nobody is running on a grant they believe is scoped. `priority:p2` stands. No STOP. ## The site, picked by measurement Two candidates were open to this lane: a refinement on `ObjectPermissionSchema`, an author-time rule beside `SECURITY_PRIVATE_NO_READSCOPE`, or both. **Chosen: the schema refinement, alone.** The measurement that decided it is ORDER — which site runs before the declaration is stored: - `packages/cli/src/commands/compile.ts:374` parses through `ObjectStackDefinitionSchema` and exits at `:376` on failure. The author-time rule registry does not run until `:415`, and `buildAccessMatrix` — the census substrate — not until `:678`. Both read `result.data`, i.e. post-parse. - `validateSecurityPosture` is registered in `packages/lint/src/authoring-rules.ts` with `input: 'parsed'`. There is no call site that feeds it pre-parse data. - The metadata write path is the same order: `saveMetaItem` resolves `PermissionSetSchema` from the `permission` row of `packages/spec/src/kernel/metadata-type-schemas.ts` and runs that Zod parse first; `packages/metadata-protocol/src/runtime-authoring-gate.ts`'s own header states the gate was added *because* that path "ran a per-type Zod `safeParse` and stopped". - And the runtime rule gate has an escape hatch the schema does not: `OS_ALLOW_UNLINTED_METADATA_WRITES=1` degrades a rule refusal to a log. A lint-only fix would leave the declaration materialising through it, which is the headline harm. ⇒ the schema is the only site that stops the declaration from being STORED. A lint rule for the same pair, added beside it, could then never fire — a check no code path can reach, which is the very shape this card is about. **"Both" is measured NOT owed**, and the cost it would have carried (a finding with a `hint`) is paid instead by the refusal message, which names both remedies at the located path. What this does **not** do: ⛔ it does not change what `viewAllRecords: true` grants. Triage closed "honour the scope" as a permission-semantics change and the maintainer's; only which declarations are accepted moves. ## What is refused, and the one shape that deliberately is not The refusal is the two short-circuits transcribed, not a guess — which makes it asymmetric: | declaration | resolver line | verdict | |:--|:--|:--| | `readScope` + `viewAllRecords: true` | `opClass === 'read' && (viewAllRecords \|\| modifyAllRecords)` | **refused** | | `readScope` + `modifyAllRecords: true` | same disjunct | **refused** | | `writeScope` + `modifyAllRecords: true` | `opClass === 'write' && modifyAllRecords` | **refused** | | `writeScope` + `viewAllRecords: true` | the write short-circuit does not name `viewAllRecords` | **accepted — honoured; refusing it would delete a real grant** | The two sibling refusals are the same defect class read off the same two lines; refusing only the headline third of it would have left two silent variants in the same accept set. Scope is ONE object-permission entry, which is exactly the resolver's input: `resolveObjectPermission` returns a single entry (explicit, else the `'*'` wildcard) and never merges two. A super-user bit in one permission set widening past another set's `readScope` is ADR-0090's documented additive "widest wins" semantics, not a contradictory declaration, and is not judged. ## Placement, and why the wire surface does not move The refinement rides on `ObjectPermissionBaseSchema` **inside** the `acceptRetiredDefaultResidue` stage. Both halves were measured on zod 4.4.3, not assumed: - `.superRefine()` on a `ZodObject` returns a `ZodObject` that keeps `.shape`; the same call on the residue **pipe** returns a schema with **no** `.shape` — and that read-through `shape` is what shape-reading consumers and the schema walkers duck-test. - `EffectiveObjectPermissionSchema` extends the **unrefined** base, so the effective-permissions response surface stays tolerant (#4001's authorable/wire split). A server still running an older toolchain can return a stored pair there without crashing a client. Pinned. ⚠️ **Correction, from at-tier review — the earlier flat claim "the WIRE surface stays tolerant" was broader than what was measured.** It covered `GetEffectivePermissionsResponseSchema` only. One further response payload embeds the **authoring** schema directly: `GetObjectPermissionsResponseSchema` at `packages/spec/src/api/protocol.zod.ts:2516-2519` declares `permissions: ObjectPermissionSchema`, so that response now carries the refinement too. **Disposition: stated, not changed, and here is why.** Review measured no in-tree break — the only parse site is one spec test (`packages/spec/src/api/protocol.test.ts:216`), there is no production or client runtime parse (`packages/client` re-exports the TYPE, and a refinement does not move `z.input`), and this is the pre-existing posture rather than something this PR introduces: that response was already tied to the strict authoring shape, refinement or no. Splitting it onto a tolerant clone would be a response-contract change with no measured consumer asking for it, and it belongs to whoever next revisits that endpoint, not to a refusal PR. ⇒ recorded as a residual a reviewer should see, with the scope of the tolerance claim narrowed to match the measurement. The check function is deliberately module-private: the `security` barrel re-exports this module with `export *`, and an exported refinement helper would land on the published API surface for no caller. `check:api-surface` is green with no artifact regeneration. **A `.shape` consumer outside both spec vitest projects, added to the placement evidence.** `packages/lint/src/validate-security-posture.test.ts:1220` and `:1226` read `shapeKeysOf(ObjectPermissionSchema)` — a CROSS-PACKAGE consumer of exactly the read-through the placement preserves, in a package neither `--project local` nor `--project repo` covers. It resolves `@objectstack/spec` through the package `exports`, i.e. `dist`, so it is only a reading once spec is built. Both legs are reported below. ## The nail — `explain.test.ts`, rewritten with its reason, not deleted The assertion triage flagged is `AccessMatrixEntrySchema.parse({ … viewAllRecords: true, … readScope: 'unit_and_below' … })` (verified by symbol on this tree: the parse-side pair sits at lines 374–375 and the assert-side at 380–381, one line off the cited `:375` / `:381`). ⚠️ **One dispatch premise is falsified here, and reported rather than carried**: it is *not* a guard this refusal flips. `AccessMatrixEntry` is a derived SNAPSHOT shape, not an accept set, and it should keep tolerating the pair for two measured reasons: 1. `buildAccessMatrix` constructs entries from already-parsed metadata, and `os build` reads a committed `access-matrix.json` back with a **bare `JSON.parse`** (`compile.ts:692`) to diff it. A snapshot written by an older toolchain may carry the pair and the drift diff must keep describing it. 2. **Nothing in the tree parses through that schema outside that one test file.** Whole-tree grep for `AccessMatrixEntrySchema` / `AccessMatrixSchema`: 8 files — two docs pages, three generated spec surface manifests, `explain.zod.ts`, `explain.test.ts`, and a type-alias pin. Zero production parse sites. Control: `PermissionSetSchema` appears in 120 files. ⇒ a refusal added there would be a check no code path can run — the declared-but-unenforced shape of this very card, reproduced one level up. So the verdict stays and the **reason** is rewritten, which is what the guard was actually missing: the test now says in its own text that the pair is refused by the AUTHORING accept set and tolerated here on purpose, and a **new sibling test asserts that boundary** rather than describing it — if the authoring schema ever starts accepting the pair again, `explain.test.ts` goes red too. ⛔ Nothing was deleted. ## Pins, both directions `packages/spec/src/security/permission.test.ts` — refusal direction: each of the three refused shapes, the located path (`readScope` / `writeScope`), the message naming both remedies, and the refusal reaching through a whole permission set at `objects.crm_opportunity.readScope`. Cost direction, because a pin set proving only the new refusal measures nothing about its price: `viewAllRecords: false` beside a `readScope` (the ordinary, honoured shape), a bare `readScope`, `viewAllRecords: true` alone, `writeScope` beside `viewAllRecords: true`, the wire surface staying tolerant, and the `shape` read-through surviving. ## Ablation — two legs, one of them the cost direction Each leg: mutate on disk, prove it landed by occurrence count AND `git hash-object`, run, restore, prove the restore by hash against the HEAD blob. Restores are `git checkout HEAD -- path` (never bare), the script carries `trap … EXIT INT TERM` with an absolute repo root, and an empty hash is treated as FAILURE. The fix was committed first, so the restore leg has a real target. HEAD blob `7c073285`. | leg | mutation | on-disk proof | result | |:--|:--|:--|:--| | A — refusal | disable the read-axis refusal | anchor lines 1→0, injected 0→1, hash `7c073285` → `9ae136b3` | **RED**: 4 failed / 99 passed. Restored hash `7c073285` (match), `git diff HEAD` empty → **GREEN** 103 passed | | B — **cost** | over-refuse: also reject `writeScope` beside `viewAllRecords` | anchor 1→0, injected 0→1, hash → `247ae59e` | **RED on exactly one test**, `writeScope beside viewAllRecords: true stays ACCEPTED`. Restored hash `7c073285` (match) → **GREEN** 103 passed | Leg B is what makes the cost pins a measurement: widening the refusal one shape too far is caught, and caught by the pin that exists to catch it. ## Tests Spec readings were taken at `dd71bf31bc`; the revise commit `3740c1d9a0f` moves only the changeset's category token and one documentation section, neither of which is spec source, and the gate families those two paths schedule were re-derived and re-run on the new head (below). | command | exit | result | |:--|--:|:--| | `pnpm --filter @objectstack/spec test` (`--project local`) | **0** | 472 files, 13445 tests passed | | `pnpm --filter @objectstack/spec test:repo` (`--project repo`) | **0** | 30 files, 520 tests passed | | `pnpm --filter @objectstack/spec typecheck` | **0** | clean | Both projects are reported separately on purpose: `pnpm --filter @objectstack/spec test` is `--project local` ONLY, and CI runs both. **Cross-package `.shape` consumer, run on the revise head** — `packages/lint/src/validate-security-posture.test.ts`, which reads `shapeKeysOf(ObjectPermissionSchema)` at `:1220` and `:1226` and is in neither spec vitest project: **EXIT 0, 1 file / 122 tests passed**. ⚠️ Its first run in a fresh worktree was `PREREQUISITE NOT MET` — `Failed to resolve entry for package "@objectstack/spec"`, because it resolves spec through the package `exports` i.e. `dist`, which did not exist yet. That is NOT MEASURED and is not read as a result in either direction; the 122 above is from the run AFTER `pnpm --filter '@objectstack/lint^...' build`. **Gate families re-derived on the revise head** (`scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, change set now 5 paths — the documentation edit schedules a docs family the first round did not owe). Newly scheduled and green: `check-doc-frontmatter`, `check-docs-section-name`, `check-section-landing-index`, `check-doc-route-spelling --advisory`, `docs-audit/check-affected-docs`, `docs-audit/check-drift-comment`, `@objectstack/lint check:doc-security-posture`, `@objectstack/lint check:doc-formula-expressions`, `check-reference-carrier-shape`. Re-run and green: `check-adr-0087-registration` (**EXIT 0**, and `--self-test` 0 over 353 assertions), `check-empty-changeset`, `check-changeset-no-major`, `check-changeset-fixed`, `check:doc-authoring`, `check:nul-bytes`, `check:llms-txt`, `check:docs`. ⚠️ `check:doc-security-posture` and `check:doc-formula-expressions` first answered **exit 3**, which that gate's own text defines as NOTHING MEASURED (its `packages/lint/dist` was unbuilt) — not read as a result until `pnpm --filter @objectstack/lint build` made them measurable, after which both exit 0. Gates run locally beyond the two test projects: the derived family list from `scripts/pm/dispatch-gates.mjs` for this diff, including `check:authorable-surface`, `check:strictness-ledger`, `check:api-surface`, `check:dual-source-exports`, `check:export-origins`, `check:exported-any`, `check:entry-nameability`, `check:browser-reachable-entries`, `check:dts-closure`, `check:published-files`, `check:nul-bytes`, `check:doc-authoring`, `check:test-source-alias`, `check:type-check-coverage`, `check:adr-0087-registration`, `check:empty-changeset`, `check:changeset-no-major`. The five dist-reading gates were re-run after a rebuild — their first run reported PREREQUISITE NOT MET, which is NOT MEASURED and was not read as a result in either direction. Repo-wide scans (`pnpm lint` and the rest) are CI's. ## Documentation `content/docs/permissions/permission-sets.mdx` — the "Access depth" section documented `readScope` / `writeScope` and even noted that `org` is approximately `viewAllRecords` / `modifyAllRecords`, without saying that declaring the two together is now refused. It now says so, and states the **asymmetry** explicitly: `viewAllRecords` bypasses READ only, so `writeScope` beside it stays accepted and honoured, while `modifyAllRecords` bypasses both. ⓘ No documentation example was falsified — a nesting-tolerant scan of `content/docs` for a `readScope` within 400 characters of a `viewAllRecords: true` returns **0** candidates with the control lit (7 `viewAllRecords: true` literals present, so the scan can see the token). This sentence is owed by judgment, not by any gate. ## Changeset One changeset, `@objectstack/spec: minor`, marked **BREAKING** in prose — matching the precedent this repo set for an accept-set narrowing. No `skip-changeset`: `packages/spec` publishes this surface, the parse behaviour of a shipped schema moves, and `packages/spec` additionally ships `src/**/*.zod.ts` as source with comments intact, so even the reasoning is published text. The changeset carries an ADR-0087 disposition marker, `not-required (no-migration-prescription)` — no authorable key is added, renamed or retired, and a COMBINATION narrowing has no mechanical conversion because the two remedies express opposite author intents and only the author can choose. ⚠️ **Correction — an earlier revision of this body asserted two things about `check:adr-0087-registration` that are both FALSE, and it reported the gate green when it was RED.** It said the gate "keys on `major`" and "reports no declared-breaking changeset". Measured: the gate keys on the `**BREAKING**` marker in the changeset body (`scripts/check-adr-0087-registration.mjs`, `breakingDeclaration` — `if (/\*\*BREAKING/i.test(parsed.body) …) signals.push('BREAKING')`), so it *does* select this changeset; and it REFUSES an unknown `not-required` category rather than ignoring it. The vocabulary is closed: `unpublished` · `already-registered` · `no-migration-prescription` · `runtime-interface-only` · `type-surface-only`. The category first used here, `no-key-moves`, is not in it, and the gate exited **1** on `dd71bf31bc`. **Why the earlier reading was a dead probe, stated rather than glossed:** this gate scans the **committed** base..head diff, not the working tree. When it was first run the changeset file was still **untracked** (`??` in `git status`), so the scan saw zero changesets and printed "0 non-breaking changeset(s) seen" — an empty population reported as a clean one. That is a probe with no lit control, and it is exactly the failure class this PR is about: a green that measured nothing. It is corrected here rather than quietly dropped. **Fixed:** only the category token moves, to `no-migration-prescription`; the reason prose is unchanged byte for byte, because it already argued precisely that case. `node scripts/check-adr-0087-registration.mjs --base origin/main` now exits **0** on the committed blob and names the disposition back; its `--self-test` exits 0 over 353 assertions. ## Publish-surface reading — new exported symbols, new payload keys Answered from the diff and from the REGENERATED surface artifacts, each leg with a control that could have come back the other way. **(a) New exported symbols: ZERO.** The only added line in the whole diff containing the token `export` is prose inside a comment. The refinement helper is deliberately module-private — the `security` barrel re-exports this module with `export *`, so exporting it would have put a refinement helper on the published API surface for no caller. - *Negative leg*: regenerating `gen:api-surface`, `gen:export-origins` and `gen:declaration-map` from this diff's dist moves **no artifact** — `git status` over `packages/spec/api-surface`, `packages/spec/export-origins` and `packages/spec/declaration-map` is empty afterwards. - *Lit control*: appending ONE dummy `export const` to the very same file and regenerating moves `api-surface/security.json` AND `export-origins/security.json`, and the dummy symbol appears in the regenerated `api-surface/security.json`. ⇒ the generators do see a new export in this file; the zero above is a reading, not a dead probe. Reverted, source hash back to the HEAD blob, tree clean. **(b) New keys on a published payload: ZERO.** No key is added, renamed or retired on any schema; what narrows is a key COMBINATION. - *Negative leg*: `check:authorable-surface` exits 0 and `packages/spec/authorable-surface.base.json` is untouched by the diff. - *Lit control*: adding ONE optional key to the same `ObjectPermission` shape turns that gate RED and names it — `+ security/ObjectPermission:os16870ControlKey` and `+ security/EffectiveObjectPermission:os16870ControlKey`. Reverted, hash match, tree clean. ⇒ **zero new exports, zero new keys.** The only published-surface movement is the accept set itself. Per the seat's correction, the final value of the clause declaration is the seat's to set before landing; this round leaves the line and both carriers exactly as dispatched. ## Landing ⛔ Left as a draft. Not flipped ready, not enqueued, no auto-merge — the `domain:spec` seat lands it after an at-tier verdict. `needs:contract-review` is attached from the moment this opens. Session attribution for this work, in prose so it survives a body PATCH: generated by Claude Code, session `session_01MkQhmuuJAVDjmeWNixwDDH`. ⚠️ Attribution, in prose because a body `PATCH` makes the platform append a second bare footer: authored by Claude Code in session `session_01MkQhmuuJAVDjmeWNixwDDH` — https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ef47459 commit 88a072e

5 files changed

Lines changed: 339 additions & 2 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
fix(spec): an object permission that declares a depth axis beside the super-user bit which short-circuits it is now REFUSED, instead of being stored and counted as coverage (#16870)
6+
7+
**BREAKING**`ObjectPermissionSchema` no longer accepts a `readScope` beside
8+
`viewAllRecords: true`. Two sibling shapes are refused with it, read off the
9+
same resolver lines rather than guessed at.
10+
11+
The pair was accepted with **zero diagnostics**, materialised into
12+
`sys_permission_set.object_permissions`, and counted by a capability census
13+
reading the deployed shape as coverage — while the read stayed org-wide.
14+
`PermissionEvaluator.getEffectiveScope` answers `org` on the super-user bit
15+
**before** it consults the depth key, and `getDeclaredScope` (the ADR-0090 D10
16+
delegated-path input) carries the identical short-circuit ahead of the identical
17+
read, so the declared narrowing was dropped from the delegation fold as well.
18+
19+
⇒ the author declared a narrowing, the platform stored it, an audit of the
20+
deployed shape reported the capability as exercised, and the read was still
21+
org-wide. That is ADR-0049 `declared ≠ enforced` at the capability container
22+
itself, and the accept set is the only door that stops the declaration from
23+
being STORED: a diagnostic raised later fires after the shape is already there.
24+
25+
```
26+
FROM ObjectPermissionSchema.parse({ allowRead: true, viewAllRecords: true,
27+
readScope: 'own_and_reports' })
28+
-> { …, viewAllRecords: true, readScope: 'own_and_reports' } // stored, unread
29+
30+
TO -> ZodError, located at ['readScope']:
31+
"readScope: 'own_and_reports' is declared beside viewAllRecords: true,
32+
which already grants org-wide read. … Delete readScope if the org-wide
33+
read is intended, or set viewAllRecords: false if the narrowing is."
34+
```
35+
36+
**Which pairs move, and the one that deliberately does not.** The refusal is the
37+
two short-circuits, transcribed:
38+
39+
| declaration | resolver | verdict |
40+
|:--|:--|:--|
41+
| `readScope` + `viewAllRecords: true` | `opClass === 'read' && (viewAllRecords \|\| modifyAllRecords)` | **refused** |
42+
| `readScope` + `modifyAllRecords: true` | same disjunct | **refused** |
43+
| `writeScope` + `modifyAllRecords: true` | `opClass === 'write' && modifyAllRecords` | **refused** |
44+
| `writeScope` + `viewAllRecords: true` | the write short-circuit does not name `viewAllRecords` | **accepted — honoured, and refusing it would delete a real grant** |
45+
46+
**What `viewAllRecords: true` GRANTS is untouched.** This changes which
47+
declarations are accepted, never what an accepted one does — a permission-
48+
semantics change is not in this change's remit. `viewAllRecords: true` alone,
49+
`viewAllRecords: false` beside a `readScope` (the ordinary, honoured shape), and
50+
a bare `readScope` all parse exactly as before; each is pinned as a
51+
cost-direction guard in `permission.test.ts`, and an ablation that widens the
52+
refusal one shape too far turns the `writeScope`-beside-`viewAllRecords` pin red.
53+
54+
**The wire surface stays tolerant.** The refinement rides on the AUTHORING
55+
wrapper only; `EffectiveObjectPermissionSchema` extends the unrefined base, so a
56+
server still running an older toolchain can return a stored pair in an
57+
effective-permission response without crashing a client (#4001's authorable/wire
58+
split). `AccessMatrixEntry` likewise keeps describing the pair: it is a derived
59+
SNAPSHOT shape whose committed `access-matrix.json` may predate this refusal, and
60+
its tolerance is now stated with that reason in `explain.test.ts` rather than
61+
reading as evidence that the platform accepts the declaration.
62+
63+
**Scope is one object-permission entry**, which is exactly the resolver's input —
64+
`resolveObjectPermission` returns a single entry (explicit, else the `'*'`
65+
wildcard) and never merges two. A super-user bit in one permission set widening
66+
past another set's `readScope` is ADR-0090's documented additive "widest wins"
67+
semantics, not a contradictory declaration, and is not judged here.
68+
69+
**Nothing in the fleet moves.** Measured across shipped defaults, both seeded
70+
examples, two built access matrices, the built artifact fixture and every tracked
71+
`.ts` / `.json`: **0** object permissions carry any refused pair, with lit
72+
controls on every probe (130 nodes declaring `viewAllRecords`, 53 of them `true`,
73+
18 declaring `readScope`; 133 brace-local `viewAllRecords: true` literals).
74+
75+
<!-- adr-0087: not-required (no-migration-prescription) No authorable key is added, renamed or
76+
retired — `readScope`, `writeScope`, `viewAllRecords` and `modifyAllRecords` all
77+
keep their spelling, position and meaning. What narrows is a COMBINATION, and it
78+
has no mechanical conversion: the two remedies (delete the depth key, or clear
79+
the super-user bit) express opposite author intents and only the author can
80+
choose. The refusal message names both at the located path, which is the whole
81+
notification channel a migration entry would have provided. -->

content/docs/permissions/permission-sets.mdx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,17 @@ the anchor decides *which* unit `unit*` means.
131131
> require the enterprise hierarchy resolver; without it they **fail closed to
132132
> `own`** (never fail-open).
133133
134+
> **A depth axis beside the super-user bit that bypasses it is REFUSED.** The
135+
> read-scope resolver answers `org` on the super-user bit *before* it consults
136+
> the depth key, so such a declaration would be stored and never read — and a
137+
> capability census over the deployed shape would count it as coverage. The
138+
> accept set now rejects it at the offending key, naming both remedies. Note the
139+
> **asymmetry**, which follows the resolver exactly: `viewAllRecords` bypasses
140+
> READ only, so `readScope` beside `viewAllRecords: true` is refused while
141+
> `writeScope` beside it is **accepted and honoured**. `modifyAllRecords`
142+
> bypasses both, so both `readScope` and `writeScope` beside it are refused.
143+
> Declare the depth, or declare the bypass — not both on the same axis.
144+
134145
## Capabilities & required permissions (ADR-0066)
135146

136147
- **Capabilities** are first-class `sys_capability` records — named privileges

packages/spec/src/security/explain.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ import {
2828
ExplainRecordAttributionSchema,
2929
EXPLAIN_BATCH_MAX_RECORD_IDS,
3030
} from './explain.zod';
31+
// [#16870] Imported for ONE assertion: the boundary between the snapshot shape
32+
// this file locks and the AUTHORING accept set that now refuses the same pair.
33+
import { ObjectPermissionSchema } from './permission.zod';
3134

3235
describe('ExplainOperationSchema — the operation vocabulary is fixed', () => {
3336
it('accepts exactly the seven CRUD + lifecycle operations', () => {
@@ -367,7 +370,32 @@ describe('ExplainDecisionSchema — the full decision report L3 consumes', () =>
367370
});
368371

369372
describe('AccessMatrix schemas — the authoring-time companion', () => {
370-
it('AccessMatrixEntry locks the crud bits + super-user bypass + scopes + sharingModel', () => {
373+
// [#16870] ⚠️ The sample row below carries `viewAllRecords: true` BESIDE a
374+
// `readScope`, and that is deliberate — but its reason changed, so read this
375+
// before reading the assertion.
376+
//
377+
// That pair is a contradiction: the read-scope resolver answers `org` on the
378+
// super-user bit before it ever consults `readScope`, so the depth is unread
379+
// on every runtime path. The AUTHORING accept set (`ObjectPermissionSchema`)
380+
// therefore REFUSES it now — pinned in `permission.test.ts`, and asserted
381+
// from here in the next test so the two surfaces cannot drift apart silently.
382+
//
383+
// `AccessMatrixEntry` deliberately does NOT refuse it, for two measured
384+
// reasons:
385+
//
386+
// 1. It is a DERIVED SNAPSHOT shape, not an accept set. `buildAccessMatrix`
387+
// constructs entries from ALREADY-PARSED metadata, and `os build` reads a
388+
// committed `access-matrix.json` back with a bare `JSON.parse` to diff it.
389+
// A snapshot written by an older toolchain may well carry the pair, and
390+
// the drift diff has to keep describing it.
391+
// 2. Nothing in the tree parses through this schema outside this file —
392+
// it is a TYPE contract that cloud's L3 product reads. A refusal added
393+
// here would be a check no code path can ever run: exactly the
394+
// declared-but-unenforced shape #16870 is about, reproduced one level up.
395+
//
396+
// ⇒ What this test locks is unchanged — the FIELD SHAPE, every key present
397+
// and typed. It is no longer evidence that the platform accepts the pair.
398+
it('AccessMatrixEntry locks the crud bits + super-user bypass + scopes + sharingModel (a SNAPSHOT shape: tolerant by design)', () => {
371399
const entry = AccessMatrixEntrySchema.parse({
372400
permissionSet: 'crm_admin', object: 'crm_lead',
373401
create: true, read: true, edit: true, delete: false,
@@ -382,6 +410,23 @@ describe('AccessMatrix schemas — the authoring-time companion', () => {
382410
});
383411
});
384412

413+
it('[#16870] the AUTHORING accept set refuses the very pair this snapshot shape tolerates', () => {
414+
// The boundary, asserted rather than described. If a later change makes
415+
// the authoring schema accept the pair again, this fails here too — the
416+
// snapshot tolerance above is only defensible while the door upstream of
417+
// it is shut.
418+
const authored = ObjectPermissionSchema.safeParse({
419+
allowRead: true, viewAllRecords: true, readScope: 'unit_and_below',
420+
});
421+
expect(authored.success, 'the accept set refuses a depth the resolver never reads').toBe(false);
422+
const snapshot = AccessMatrixEntrySchema.safeParse({
423+
permissionSet: 'crm_admin', object: 'crm_lead',
424+
create: true, read: true, edit: true, delete: false,
425+
viewAllRecords: true, modifyAllRecords: false, readScope: 'unit_and_below',
426+
});
427+
expect(snapshot.success, 'the snapshot shape still describes artifacts written before the refusal').toBe(true);
428+
});
429+
385430
it('the crud + bypass bits are REQUIRED (a missing bit is a contract break)', () => {
386431
expect(() => AccessMatrixEntrySchema.parse({
387432
permissionSet: 'x', object: 'y', create: true, read: true, edit: true, delete: true,

packages/spec/src/security/permission.test.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,111 @@ describe('ObjectPermissionSchema', () => {
109109
});
110110
});
111111

112+
describe('[#16870] a depth axis beside the super-user bit that short-circuits it is REFUSED', () => {
113+
// The defect: `PermissionEvaluator.getEffectiveScope` answers `org` on the
114+
// super-user bit BEFORE it consults the depth key, and `getDeclaredScope`
115+
// (the ADR-0090 D10 delegated-path input) carries the identical
116+
// short-circuit ahead of the identical read. So the pair was accepted with
117+
// zero diagnostics, materialised into `sys_permission_set.object_permissions`
118+
// and counted by a capability census as coverage, while the read stayed
119+
// org-wide. Refused at the accept set, which is the only site that stops the
120+
// declaration from being STORED: `saveMetaItem` runs this parse before the
121+
// runtime authoring gate, and `os build` runs it (compile.ts) before both the
122+
// author-time rule registry and `buildAccessMatrix`.
123+
124+
// ── REFUSAL direction ────────────────────────────────────────────────
125+
it('readScope beside viewAllRecords: true is refused, at the readScope path', () => {
126+
const r = ObjectPermissionSchema.safeParse({
127+
allowRead: true, viewAllRecords: true, readScope: 'own_and_reports',
128+
});
129+
expect(r.success).toBe(false);
130+
const issue = r.error!.issues.find((i) => i.path[i.path.length - 1] === 'readScope')!;
131+
expect(issue, 'the issue is LOCATED on the key the author must remove').toBeDefined();
132+
expect(issue.message).toContain('viewAllRecords: true');
133+
expect(issue.message).toContain('never enforced on any read path');
134+
expect(issue.message, 'both remedies are named, not just one').toContain('set viewAllRecords: false');
135+
});
136+
137+
it('readScope beside modifyAllRecords: true is refused too — the same disjunct short-circuits READ', () => {
138+
const r = ObjectPermissionSchema.safeParse({
139+
allowRead: true, modifyAllRecords: true, readScope: 'unit',
140+
});
141+
expect(r.success).toBe(false);
142+
expect(r.error!.issues.some((i) => i.path[i.path.length - 1] === 'readScope')).toBe(true);
143+
});
144+
145+
it('writeScope beside modifyAllRecords: true is refused, at the writeScope path', () => {
146+
const r = ObjectPermissionSchema.safeParse({
147+
allowEdit: true, modifyAllRecords: true, writeScope: 'unit',
148+
});
149+
expect(r.success).toBe(false);
150+
const issue = r.error!.issues.find((i) => i.path[i.path.length - 1] === 'writeScope')!;
151+
expect(issue).toBeDefined();
152+
expect(issue.message).toContain('modifyAllRecords: true');
153+
});
154+
155+
it('the refusal reaches through a whole permission set, located at objects.<name>.<key>', () => {
156+
const r = PermissionSetSchema.safeParse({
157+
name: 'sales_manager',
158+
objects: { crm_opportunity: { allowRead: true, viewAllRecords: true, readScope: 'own_and_reports' } },
159+
});
160+
expect(r.success).toBe(false);
161+
expect(r.error!.issues.some((i) => i.path.join('.') === 'objects.crm_opportunity.readScope')).toBe(true);
162+
});
163+
164+
// ── COST direction: what the refusal must NOT cost ───────────────────
165+
// A pin set that only proves the new refusal measures nothing about its
166+
// price. These four are the shapes the resolver DOES read, and every one of
167+
// them has to keep parsing.
168+
it('viewAllRecords: false beside a readScope stays accepted — the ordinary, honoured shape', () => {
169+
const parsed = ObjectPermissionSchema.parse({
170+
allowRead: true, viewAllRecords: false, readScope: 'own_and_reports',
171+
});
172+
expect(parsed.readScope).toBe('own_and_reports');
173+
});
174+
175+
it('a readScope with the super-user bits merely ABSENT stays accepted (the default is false)', () => {
176+
const parsed = ObjectPermissionSchema.parse({ allowRead: true, readScope: 'unit' });
177+
expect(parsed.readScope).toBe('unit');
178+
expect(parsed.viewAllRecords).toBe(false);
179+
});
180+
181+
it('viewAllRecords: true ALONE stays accepted — the refusal is about the pair, not the bit', () => {
182+
const parsed = ObjectPermissionSchema.parse({ allowRead: true, viewAllRecords: true });
183+
expect(parsed.viewAllRecords).toBe(true);
184+
});
185+
186+
it('writeScope beside viewAllRecords: true stays ACCEPTED — viewAllRecords does not bypass write', () => {
187+
// The asymmetry is read off the resolver, not assumed: the write
188+
// short-circuit is `opClass === 'write' && op.modifyAllRecords`, which does
189+
// not name `viewAllRecords`. Refusing this pair would delete a grant the
190+
// platform honours.
191+
const parsed = ObjectPermissionSchema.parse({
192+
allowRead: true, allowEdit: true, viewAllRecords: true, writeScope: 'unit',
193+
});
194+
expect(parsed.writeScope).toBe('unit');
195+
});
196+
197+
it('the WIRE surface stays tolerant — an older server may still emit a stored pair', () => {
198+
// #4001's authorable/wire split: the refusal rides on the AUTHORING
199+
// wrapper only. `EffectiveObjectPermissionSchema` extends the unrefined
200+
// base, so a response carrying a pair stored before this refusal landed
201+
// does not crash an older or newer client.
202+
const parsed = EffectiveObjectPermissionSchema.parse({
203+
allowRead: true, viewAllRecords: true, readScope: 'own_and_reports',
204+
});
205+
expect(parsed.readScope).toBe('own_and_reports');
206+
});
207+
208+
it('the refusal does not disturb the shape read-through the residue stage publishes', () => {
209+
// `acceptRetiredDefaultResidue` re-attaches a read-through `shape`; the
210+
// refinement rides INSIDE it precisely so that stays true (a
211+
// `.superRefine()` on the pipe would have dropped it).
212+
expect(Object.keys(ObjectPermissionSchema.shape)).toContain('readScope');
213+
expect(Object.keys(ObjectPermissionSchema.shape)).toContain('viewAllRecords');
214+
});
215+
});
216+
112217
describe('allowRestore / allowPurge are RETIRED (#12497, ADR-0049)', () => {
113218
// Removed by the 2026-08-26 maintainer ruling accepting #1883's
114219
// recommendation B: the `restore`/`purge` ObjectQL operations the bits

0 commit comments

Comments
 (0)