Skip to content

Commit 298c60d

Browse files
committed
fix(spec): scope the modifyAllRecords bypass declaration to what sharing enforces (#6698)
`ObjectPermissionSchema.modifyAllRecords` promised a bypass of "Sharing Rules and Ownership checks". On an object with NO owner field it bypasses neither: record sharing does not enforce there at all (`checkEdit`/`checkDelete` answer `abstain` before the bypass is probed, #6428), so the platform's own row-level write floor `created_by == current_user.id` (#1985) survives and the by-id write is refused — measured and pinned in plugin-security's `row-write-widener-composition.test.ts` by #6684. Option A on #6698: only the DECLARATION moves. The describe now scopes the bypass to the objects record sharing enforces on and discloses the surviving platform floor, while keeping legible that on an owner-bearing object — the common case this bit is granted for — the bypass is real. No runtime change; the accepted/rejected metadata set is byte-for-byte unchanged; no `plugin-sharing` or `plugin-security` file is touched. `content/docs/references/security/permission.mdx` is regenerated by `pnpm --filter @objectstack/spec gen:docs` (two table cells). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ffcE95NaMJcL9XJ9VDYgk
1 parent c32944d commit 298c60d

4 files changed

Lines changed: 99 additions & 6 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): `modifyAllRecords` 的声明不再承诺它在无 owner 字段对象上做不到的 bypass(#6698)
6+
7+
`ObjectPermissionSchema.modifyAllRecords``.describe()` 此前只写
8+
`Modify All Data (Bypass Sharing)`,旁边的文档注释更进一步宣称它
9+
"Bypasses Sharing Rules and Ownership checks"。在**没有 owner 字段**的对象上,
10+
两样都不成立:记录共享在这类对象上根本不参与判定 —— `checkEdit` / `checkDelete`
11+
在探测 bypass 之前就返回 `abstain`(#6428 的三态),于是平台自己的行级写入底线
12+
`created_by == current_user.id`(#1985`owner_only_writes` /
13+
`owner_only_deletes` 通配策略)继续生效,按 id 写别人创建的行仍然被 403 拒绝。
14+
这一格是 #6684 明确测量并钉住的既定行为(plugin-security 的
15+
`row-write-widener-composition.test.ts`),不是缺陷 —— 缺陷在于声明比实现讲得多,
16+
正是 ADR-0049 `declared ≠ enforced` 那一类残留。
17+
18+
这次改的只有**声明**:describe 把 bypass 限定在"记录共享真正参与判定的对象"上,
19+
并披露 owner-less 对象上仍然生效的平台 `created_by` 写入底线。带 owner 字段的
20+
对象 —— 也就是授予这个位的常见场景 —— bypass 依旧是真的,措辞刻意保留了这一半,
21+
以免矫枉过正成相反的谎;新增的 pin 对两半都会变红。
22+
23+
**没有任何运行时行为变化**,合法元数据集合逐字节不变(只有描述字符串变了),
24+
`plugin-sharing` / `plugin-security` 一个文件都没有碰。生成的
25+
`content/docs/references/security/permission.mdx` 随之重算。

content/docs/references/security/permission.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ const result = AdminScopeSchema.parse(data);
6060
| **allowRestore** | `boolean` || [RBAC-gated; operation pending M2] Restore from trash (Undelete) |
6161
| **allowPurge** | `boolean` || [RBAC-gated; operation pending M2] Permanently delete (Hard Delete/GDPR) |
6262
| **viewAllRecords** | `boolean` || View All Data (Bypass Sharing) |
63-
| **modifyAllRecords** | `boolean` || Modify All Data (Bypass Sharing) |
63+
| **modifyAllRecords** | `boolean` || Modify All Data (Bypass Sharing) — bypasses sharing rules and ownership on the objects record sharing enforces on; on an object with NO owner field sharing abstains, so the platform created_by write floor still applies (#6698). |
6464
| **readScope** | `Enum<'own' \| 'own_and_reports' \| 'unit' \| 'unit_and_below' \| 'org'>` | optional | [ADR-0057 D1] Read depth: own\|unit\|unit_and_below\|org |
6565
| **writeScope** | `Enum<'own' \| 'own_and_reports' \| 'unit' \| 'unit_and_below' \| 'org'>` | optional | [ADR-0057 D1] Write depth: own\|unit\|unit_and_below\|org |
6666
| **apiOperations** | `Enum<'get' \| 'list' \| 'create' \| 'update' \| 'delete' \| 'upsert' \| 'bulk' \| 'aggregate' \| 'history' \| 'search' \| 'restore' \| 'purge' \| 'import' \| 'export'>[]` | optional | Server-resolved effective API operations for this object (#3391). Present only when the object tightens exposure via apiMethods; absent = default-allow. The frontend renders this effective set, never the raw whitelist. Vocabulary is the EFFECTIVE ApiOperation set (six primitives + eight derived verbs, #3543), not the authored six-value ApiMethod enum. |
@@ -108,7 +108,7 @@ const result = AdminScopeSchema.parse(data);
108108
| **allowRestore** | `boolean` || [RBAC-gated; operation pending M2] Restore from trash (Undelete) |
109109
| **allowPurge** | `boolean` || [RBAC-gated; operation pending M2] Permanently delete (Hard Delete/GDPR) |
110110
| **viewAllRecords** | `boolean` || View All Data (Bypass Sharing) |
111-
| **modifyAllRecords** | `boolean` || Modify All Data (Bypass Sharing) |
111+
| **modifyAllRecords** | `boolean` || Modify All Data (Bypass Sharing) — bypasses sharing rules and ownership on the objects record sharing enforces on; on an object with NO owner field sharing abstains, so the platform created_by write floor still applies (#6698). |
112112
| **readScope** | `Enum<'own' \| 'own_and_reports' \| 'unit' \| 'unit_and_below' \| 'org'>` | optional | [ADR-0057 D1] Read depth: own\|unit\|unit_and_below\|org |
113113
| **writeScope** | `Enum<'own' \| 'own_and_reports' \| 'unit' \| 'unit_and_below' \| 'org'>` | optional | [ADR-0057 D1] Write depth: own\|unit\|unit_and_below\|org |
114114

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,3 +658,55 @@ describe('unknown keys are rejected, not stripped (#4001)', () => {
658658
});
659659
});
660660
});
661+
662+
/**
663+
* [#6698] The `modifyAllRecords` `.describe()` is a published contract, not a
664+
* code comment: it is the field help Studio renders inline, and it is the cell
665+
* the generated `content/docs/references/security/permission.mdx` table carries
666+
* — so an author (very often an AI maintainer, ADR-0033) deciding whether this
667+
* bit covers their object reads THIS text and nothing else.
668+
*
669+
* It used to read `Modify All Data (Bypass Sharing)` beside a JSDoc block
670+
* promising a bypass of "Sharing Rules and Ownership checks". On an object with
671+
* NO owner field it bypasses neither: record sharing does not enforce there at
672+
* all (`checkEdit` / `checkDelete` answer `abstain` before the bypass is ever
673+
* probed, #6428), so the platform's own row-level write floor
674+
* `created_by == current_user.id` (#1985) survives and the by-id write is still
675+
* refused — measured and pinned in plugin-security's
676+
* `row-write-widener-composition.test.ts`.
677+
*
678+
* The assertions read the text back OUT of the schema and match the FACTS it
679+
* must carry rather than its wording: re-spell the qualification however reads
680+
* best and this stays green; drop it — or empty the `.describe()`, which is why
681+
* every assertion here is POSITIVE — and it goes red. Both halves are pinned on
682+
* purpose: a declaration that forgot to say the bit is a genuine super-user
683+
* bypass would be the opposite lie, since on the common owner-bearing object it
684+
* does exactly what it says.
685+
*/
686+
describe('[#6698] modifyAllRecords declares its bypass AND the limit of that bypass', () => {
687+
const description = ObjectPermissionSchema.shape.modifyAllRecords.description ?? '';
688+
689+
/** Idioms that SCOPE the bypass to the objects record sharing enforces on. */
690+
const OWNERLESS_LIMIT =
691+
/owner-?less|no owner field|without an owner|objects (that )?(record )?sharing enforces on|as (record )?sharing computes/i;
692+
/** Idioms naming the gate that SURVIVES the bypass on such an object. */
693+
const SURVIVING_FLOOR = /created_by|ownership floor|write floor|abstain/i;
694+
695+
it('still tells the author this is a super-user bypass, not an inert bit', () => {
696+
expect(description, 'modifyAllRecords must carry a description — it is the form field help')
697+
.not.toBe('');
698+
expect(description, 'the capability must stay nameable in the text').toMatch(/modify all data/i);
699+
expect(description, 'on an owner-bearing object the bypass is real and must stay legible')
700+
.toMatch(/bypass/i);
701+
});
702+
703+
it('scopes the bypass to the objects record sharing actually enforces on', () => {
704+
expect(description, `no owner-less qualification found in: ${description}`)
705+
.toMatch(OWNERLESS_LIMIT);
706+
});
707+
708+
it('discloses the platform write floor that survives on an owner-less object', () => {
709+
expect(description, `no surviving-floor disclosure found in: ${description}`)
710+
.toMatch(SURVIVING_FLOOR);
711+
});
712+
});

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,28 @@ export const ObjectPermissionSchema = lazySchema(() => strictObject(
164164
*/
165165
viewAllRecords: z.boolean().default(false).describe('View All Data (Bypass Sharing)'),
166166

167-
/**
168-
* Modify All Records: Super-user write access.
169-
* Bypasses Sharing Rules and Ownership checks.
167+
/**
168+
* Modify All Records: Super-user write access.
169+
* Bypasses Sharing Rules and Ownership checks — as RECORD SHARING computes
170+
* them, i.e. on every object `ISharingService` enforces on, which is any
171+
* object carrying an owner field (the common case, and the one this bit is
172+
* granted for).
170173
* Equivalent to Microsoft Dataverse "Organization" level write access.
174+
*
175+
* [#6698] It is NOT a bypass of every ownership check the platform runs. On
176+
* an object with NO owner field record sharing does not enforce at all —
177+
* `checkEdit` / `checkDelete` answer `abstain` before the bypass is ever
178+
* probed (#6428's tri-state) — so the platform's own row-level WRITE floor
179+
* (`created_by == current_user.id`, shipped as the wildcard
180+
* `owner_only_writes` / `owner_only_deletes` policies that answer #1985)
181+
* stays in force, and a by-id write to another user's row is still refused.
182+
* Measured and pinned in plugin-security's
183+
* `row-write-widener-composition.test.ts`. Widening that cell would be a
184+
* RUNTIME change in `plugin-sharing` (option B on #6698) and is deliberately
185+
* not taken — what moved here is only the declaration, so that it stops
186+
* over-claiming (ADR-0049 `declared ≠ enforced`).
171187
*/
172-
modifyAllRecords: z.boolean().default(false).describe('Modify All Data (Bypass Sharing)'),
188+
modifyAllRecords: z.boolean().default(false).describe('Modify All Data (Bypass Sharing) — bypasses sharing rules and ownership on the objects record sharing enforces on; on an object with NO owner field sharing abstains, so the platform created_by write floor still applies (#6698).'),
173189

174190
/**
175191
* [ADR-0057 D1] Read access DEPTH (Dataverse-style access level), layered on

0 commit comments

Comments
 (0)