Repository navigation
Commit f6ba134
feat(spec,lint)!: refuse allowDelete: false beside modifyAllRecords: true (#25707)
Fixes #25575
Clause-②: no (narrowing)
Metadata-protocol: narrowed
Ruling-ref: 6377890121 (letter A, under 6377842908); retriage 6382861731
## What changes
`modifyAllRecords` stays the write bypass for edit, delete and transfer
(the runtime and `MODIFY_ALL_WRITE_KEYS` are untouched). What an author
can write narrows:
- **Schema refusal.** `ObjectPermissionSchema` refuses an explicit
`allowDelete: false` in an entry that sets `modifyAllRecords: true`: one
`custom` issue at `objects.OBJECT.allowDelete` (wildcard `*` included).
The message names the route for "edit every record, delete none". It
rides the existing `checkScopeAgainstSuperUserBits` refinement (the
#16870 precedent), so the response-side
`EffectiveObjectPermissionSchema` stays tolerant.
- **Omitted key materialises as `true`.** A raw-input stage in front of
the schema writes `allowDelete: true` into an entry that sets
`modifyAllRecords: true` and omits `allowDelete`. See "Mechanism
assumption" below for why the refinement alone could not work.
- **Lint arm.** `security-modify-all-delete-withheld` (error) in the
security-posture family
(`packages/lint/src/validate-security-posture.ts`), with an `os explain`
entry. On parsed doors the schema refuses first, so the arm reaches the
unparsed doors only (`os lint` on a raw object-literal config, a direct
call), the same posture `security-owd-alias` documents.
- **Data at rest.** ADR-0087 D2 conversion
`permission-modify-all-allow-delete-false-to-true` (retired from the
load path from day one, `retiredAfter: '18.1.0'`) plus the D3 entry
`permission-modify-all-allow-delete-false-refused`. Stored rows and
built artifacts inside their declared-floor window get the inert `false`
rewritten to the `true` the evaluator always acted on. A source authored
at the current surface is refused, not rewritten.
- **Docs.** A measured recipe in
`content/docs/permissions/sharing-rules.mdx` ("Recipe — edit every
record, delete none"); one paragraph in `permissions-matrix.mdx`; two
table rows in `permission-sets.mdx` / `permission-metadata.mdx` that
said Modify All is "edit" now say "edit and delete". Generated
references regenerated.
## Mechanism assumption (re-measured, and why the shape differs from "a
parse-time refinement")
`allowDelete` carries `.default(false)` and a refinement sees the parsed
value, so `{ modifyAllRecords: true }` parsed to `{ allowDelete: false,
modifyAllRecords: true }` on `origin/main` 6021b5e (measured on the
built dist). A refinement alone would refuse that output the next time
it is parsed: `definePermissionSet` output handed to `defineStack`, a
build re-read at boot, a stored row saved again. So the explicitness is
read off the raw input. The stage writes `true` into an omitted key
(what `objectPermissionGrants` already answers), and the refinement then
only ever sees an authored `false`. `parse(parse(x))` equals `parse(x)`
is pinned. Data at rest carries the pair two ways. Authored: HotCRM's
`src/sales/profiles/sales-manager.profile.ts:57` declares `crm_forecast`
with `allowDelete: false` beside `modifyAllRecords: true`, and its built
17.1 artifact (the fixture
`packages/metadata/src/__fixtures__/hotcrm-17.1-built-permissions.artifact.json`)
carries that entry. Filled in: every earlier parse wrote `false` into an
omitted `allowDelete`, so an artifact or stored row built from a source
that omits it (the showcase's `showcase_ops`) carries the pair too.
Hence the D2 conversion. The artifact door converts the 17.1 artifact
(floor below `retiredAfter`) and refuses the same body when it claims a
surface past `retiredAfter`; both pinned in
`plugin-artifact-forward-conversion.test.ts`.
## Reading: can a criteria rule select every row, and does it grant edit
without delete?
Read: `checkDelete` in `plugin-sharing/src/sharing-service.ts` has no
share branch (ADR-0111 D3), so an `edit` share opens update and never
delete. `rule-criteria.ts` `isMatchAllCriteria` refuses or skips a
criteria that lowers to an empty filter, and accepts any concrete field
predicate.
Measured on a booted in-process stack (`@objectstack/verify`
`bootStack`, at 9f2f3fa, whose runtime code the later commits do not
touch; a temporary test, not committed), the card's own `srm_material`
shape: `public_read`, admin-owned rows, permission set `{ allowRead,
allowCreate: false, allowEdit, allowDelete: false }` distributed by
position `srm_buyer`, rule `{ type: 'criteria', condition: "record.id !=
''", accessLevel: 'edit', sharedWith: { type: 'position', value:
'srm_buyer' } }`:
| step | result |
|---|---|
| buyer updates an admin-owned row, before holding the position | 403
PERMISSION_DENIED |
| `sys_user_position` row inserted | one `sys_record_share` row, `edit`,
`source: rule`, to the buyer |
| buyer updates the admin-owned row | 200, `category` written, owner
unchanged |
| buyer deletes it | 403 PERMISSION_DENIED; row still present |
| admin inserts a new row; buyer updates / deletes it | 200 / 403 |
| buyer creates a row | 403 |
Control, same stack with `condition: 'true'`: boot logs `declared rule
not served … reason: match-all-criteria`, no share row, and the buyer
holding the position is refused the same update at the record ("You do
not have access to this record"). So the update above came from the
share. A literal `true` is not refused at parse; it is never served. The
docs and every prescription say so.
## Producers
Measured on 6021b5e before the change: no permission set in this
repository (`examples/**`, `packages/platform-objects`, plugins,
services) authors the pair. Two set `modifyAllRecords: true` and omit
`allowDelete`, and now parse to `allowDelete: true` with no edit:
`examples/app-showcase/src/security/permission-sets.ts`
`showcase_ops.showcase_announcement` and
`packages/verify/src/parent-write-reach.ts` `modifyAll`. The platform
sets (`admin_full_access`, `organization_admin`) write `allowDelete:
true` explicitly. Raw evaluator fixtures that declare the pair
(`plugin-security` `security-plugin.test.ts`, "should allow edit/delete
via modifyAllRecords") are never parsed and keep pinning the runtime
semantics the ruling keeps. **Downstream producer that authors the pair:
HotCRM.** `src/sales/profiles/sales-manager.profile.ts:57`
(`sales_manager.crm_forecast`). Its deployed 17.1 artifact is converted
at load (the evaluator's behaviour unchanged); its source build is
refused at `objects.crm_forecast.allowDelete` once it upgrades, and the
fix is the ruling's choice (drop `allowDelete: false` to keep Modify
All, or replace `modifyAllRecords` with the edit-share route). The seat
files the HotCRM card at merge.
## Tests and gates
**Patch round 1, at head `0a8846cd6`** (strings and comments only: the
D3 `reason` without a tracker id, the D2 docblock and fixture comment, a
test comment). The CI red was reproduced first on `c5b702ced` (cli
integration `migrate-meta-engine-guidance.test.ts`, 1 failed). At
`0a8846cd6`, each under the lock with VERDICT exit 0: cli `test` (unit +
integration) 499 files / 6286 tests; spec 719 / 21788 and `test:repo` 79
/ 1269; lint 161 / 7274; metadata 64 / 906. `dispatch-gates --commands`:
116 run, 0 NOT-MEASURED. Ablations, each restored blob-identical: (a)
disabling the refinement arm turns 2 of 101 `permission.test.ts` pins
red, as predicted; (b) making the materialise stage return its input
unchanged turns 2 of 101 red, as predicted. The dogfood battery was not
re-run: the push changes strings and comments only. `retiredAfter:
'18.1.0'` still names the latest published `@objectstack/spec`.
Round 1, at head `c5b702ced`:
- **Derived gates** (`node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack`): 116 derived, 116 run, every exit
captured before any pipe; `--ran` with exit codes answers "116 run, 0
NOT-MEASURED (a DERIVED zero)". One gate (`check-comment-mask-corpus`)
first went red on ENOENT for a temporary bundle the concurrent dogfood
run wrote into `examples/app-showcase`; re-run alone it is green (10121
files, 0 disagree). `check-plugin-teardown-shape --self-test` needed its
pinned fixture commit fetched in this shallow clone; green after.
- **Package suites** (lock VERDICT exit 0 each): spec `test` 719 files /
21788 tests, spec `test:repo` 79 / 1269, spec `typecheck`; lint `test`
161 / 7274 and `typecheck`; metadata `test` 64 / 906 and `typecheck`.
- **Consumers of the contract** (each green): metadata-core 19 / 800;
runtime 441 / 7080 and `test:repo` 3 / 854; core 118 / 2757 and
`test:repo` 6 / 82; verify 46 / 393; metadata-protocol 263 / 21475; rest
320 / 6406 and `test:repo` 5 / 280; plugin-security 222 / 4628; cli
`--project unit` 328 / 4816 (integration layer declared to CI: the diff
touches no CLI spawn entry).
- **Dogfood battery** (`(narrowing)` arm): 295 passed, 3 skipped files;
2520 passed, 18 skipped tests; VERDICT command-exit 0.
- `check:dev-prereqs`: green (local-only).
## Acceptance notes
- `GetObjectPermissionsResponseSchema.permissions` uses the authoring
`ObjectPermissionSchema` (not the effective one), so it inherits the
refusal, as it already inherits the #16870 scope refusals. Noted, not
changed.
- `os migrate meta --from 17` replays the retired D2 entry, so a source
carrying the pair would be rewritten to `allowDelete: true` (the runtime
behaviour) with the D3 entry listed beside it as the judgment owed.
Recorded in the D3 entry's `conversionIds`.
- The no-share control above also shows the platform's write floor on a
`public_read` object: without the share, the update is refused at the
record, not at the object CRUD check.
_Body edited by the `domain:spec` seat 2 after patch round 1, with the
dev's proposed text from `6390006109`, plus the round's readings._
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_017Eg8KpUtdKh6r8YTPjbhod
---
_Generated by [Claude
Code](https://claude.ai/code/session_017Eg8KpUtdKh6r8YTPjbhod)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 04273b9 commit f6ba134
18 files changed
Lines changed: 593 additions & 20 deletions
File tree
- .changeset
- content/docs
- permissions
- references
- api
- security
- packages
- lint/src
- metadata/src
- spec
- src
- conversions
- migrations/entries/semantic
- security
| 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 | |
|---|---|---|---|
| |||
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
97 | | - | |
| 97 | + | |
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
53 | 53 | | |
54 | 54 | | |
55 | 55 | | |
56 | | - | |
| 56 | + | |
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
33 | 39 | | |
34 | 40 | | |
35 | 41 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
347 | 347 | | |
348 | 348 | | |
349 | 349 | | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
350 | 399 | | |
351 | 400 | | |
352 | 401 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1196 | 1196 | | |
1197 | 1197 | | |
1198 | 1198 | | |
1199 | | - | |
| 1199 | + | |
1200 | 1200 | | |
1201 | 1201 | | |
1202 | 1202 | | |
| |||
1648 | 1648 | | |
1649 | 1649 | | |
1650 | 1650 | | |
1651 | | - | |
| 1651 | + | |
1652 | 1652 | | |
1653 | 1653 | | |
1654 | 1654 | | |
| |||
0 commit comments