From 298c60d4e37b6c3ce98ef8ee2aa6e5e2a6bf4f58 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 00:34:45 +0000 Subject: [PATCH] fix(spec): scope the `modifyAllRecords` bypass declaration to what sharing enforces (#6698) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_018ffcE95NaMJcL9XJ9VDYgk --- ...odifyall-records-owner-less-declaration.md | 25 +++++++++ .../docs/references/security/permission.mdx | 4 +- packages/spec/src/security/permission.test.ts | 52 +++++++++++++++++++ packages/spec/src/security/permission.zod.ts | 24 +++++++-- 4 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 .changeset/modifyall-records-owner-less-declaration.md diff --git a/.changeset/modifyall-records-owner-less-declaration.md b/.changeset/modifyall-records-owner-less-declaration.md new file mode 100644 index 0000000000..c9b95982a4 --- /dev/null +++ b/.changeset/modifyall-records-owner-less-declaration.md @@ -0,0 +1,25 @@ +--- +"@objectstack/spec": patch +--- + +fix(spec): `modifyAllRecords` 的声明不再承诺它在无 owner 字段对象上做不到的 bypass(#6698) + +`ObjectPermissionSchema.modifyAllRecords` 的 `.describe()` 此前只写 +`Modify All Data (Bypass Sharing)`,旁边的文档注释更进一步宣称它 +"Bypasses Sharing Rules and Ownership checks"。在**没有 owner 字段**的对象上, +两样都不成立:记录共享在这类对象上根本不参与判定 —— `checkEdit` / `checkDelete` +在探测 bypass 之前就返回 `abstain`(#6428 的三态),于是平台自己的行级写入底线 +`created_by == current_user.id`(#1985 的 `owner_only_writes` / +`owner_only_deletes` 通配策略)继续生效,按 id 写别人创建的行仍然被 403 拒绝。 +这一格是 #6684 明确测量并钉住的既定行为(plugin-security 的 +`row-write-widener-composition.test.ts`),不是缺陷 —— 缺陷在于声明比实现讲得多, +正是 ADR-0049 `declared ≠ enforced` 那一类残留。 + +这次改的只有**声明**:describe 把 bypass 限定在"记录共享真正参与判定的对象"上, +并披露 owner-less 对象上仍然生效的平台 `created_by` 写入底线。带 owner 字段的 +对象 —— 也就是授予这个位的常见场景 —— bypass 依旧是真的,措辞刻意保留了这一半, +以免矫枉过正成相反的谎;新增的 pin 对两半都会变红。 + +**没有任何运行时行为变化**,合法元数据集合逐字节不变(只有描述字符串变了), +`plugin-sharing` / `plugin-security` 一个文件都没有碰。生成的 +`content/docs/references/security/permission.mdx` 随之重算。 diff --git a/content/docs/references/security/permission.mdx b/content/docs/references/security/permission.mdx index 6c52130a20..af1fdf9e9e 100644 --- a/content/docs/references/security/permission.mdx +++ b/content/docs/references/security/permission.mdx @@ -60,7 +60,7 @@ const result = AdminScopeSchema.parse(data); | **allowRestore** | `boolean` | ✅ | [RBAC-gated; operation pending M2] Restore from trash (Undelete) | | **allowPurge** | `boolean` | ✅ | [RBAC-gated; operation pending M2] Permanently delete (Hard Delete/GDPR) | | **viewAllRecords** | `boolean` | ✅ | View All Data (Bypass Sharing) | -| **modifyAllRecords** | `boolean` | ✅ | Modify All Data (Bypass Sharing) | +| **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). | | **readScope** | `Enum<'own' \| 'own_and_reports' \| 'unit' \| 'unit_and_below' \| 'org'>` | optional | [ADR-0057 D1] Read depth: own\|unit\|unit_and_below\|org | | **writeScope** | `Enum<'own' \| 'own_and_reports' \| 'unit' \| 'unit_and_below' \| 'org'>` | optional | [ADR-0057 D1] Write depth: own\|unit\|unit_and_below\|org | | **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); | **allowRestore** | `boolean` | ✅ | [RBAC-gated; operation pending M2] Restore from trash (Undelete) | | **allowPurge** | `boolean` | ✅ | [RBAC-gated; operation pending M2] Permanently delete (Hard Delete/GDPR) | | **viewAllRecords** | `boolean` | ✅ | View All Data (Bypass Sharing) | -| **modifyAllRecords** | `boolean` | ✅ | Modify All Data (Bypass Sharing) | +| **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). | | **readScope** | `Enum<'own' \| 'own_and_reports' \| 'unit' \| 'unit_and_below' \| 'org'>` | optional | [ADR-0057 D1] Read depth: own\|unit\|unit_and_below\|org | | **writeScope** | `Enum<'own' \| 'own_and_reports' \| 'unit' \| 'unit_and_below' \| 'org'>` | optional | [ADR-0057 D1] Write depth: own\|unit\|unit_and_below\|org | diff --git a/packages/spec/src/security/permission.test.ts b/packages/spec/src/security/permission.test.ts index 0d6db78338..dd8dd24c06 100644 --- a/packages/spec/src/security/permission.test.ts +++ b/packages/spec/src/security/permission.test.ts @@ -658,3 +658,55 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { }); }); }); + +/** + * [#6698] The `modifyAllRecords` `.describe()` is a published contract, not a + * code comment: it is the field help Studio renders inline, and it is the cell + * the generated `content/docs/references/security/permission.mdx` table carries + * — so an author (very often an AI maintainer, ADR-0033) deciding whether this + * bit covers their object reads THIS text and nothing else. + * + * It used to read `Modify All Data (Bypass Sharing)` beside a JSDoc block + * promising 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 ever + * probed, #6428), so the platform's own row-level write floor + * `created_by == current_user.id` (#1985) survives and the by-id write is still + * refused — measured and pinned in plugin-security's + * `row-write-widener-composition.test.ts`. + * + * The assertions read the text back OUT of the schema and match the FACTS it + * must carry rather than its wording: re-spell the qualification however reads + * best and this stays green; drop it — or empty the `.describe()`, which is why + * every assertion here is POSITIVE — and it goes red. Both halves are pinned on + * purpose: a declaration that forgot to say the bit is a genuine super-user + * bypass would be the opposite lie, since on the common owner-bearing object it + * does exactly what it says. + */ +describe('[#6698] modifyAllRecords declares its bypass AND the limit of that bypass', () => { + const description = ObjectPermissionSchema.shape.modifyAllRecords.description ?? ''; + + /** Idioms that SCOPE the bypass to the objects record sharing enforces on. */ + const OWNERLESS_LIMIT = + /owner-?less|no owner field|without an owner|objects (that )?(record )?sharing enforces on|as (record )?sharing computes/i; + /** Idioms naming the gate that SURVIVES the bypass on such an object. */ + const SURVIVING_FLOOR = /created_by|ownership floor|write floor|abstain/i; + + it('still tells the author this is a super-user bypass, not an inert bit', () => { + expect(description, 'modifyAllRecords must carry a description — it is the form field help') + .not.toBe(''); + expect(description, 'the capability must stay nameable in the text').toMatch(/modify all data/i); + expect(description, 'on an owner-bearing object the bypass is real and must stay legible') + .toMatch(/bypass/i); + }); + + it('scopes the bypass to the objects record sharing actually enforces on', () => { + expect(description, `no owner-less qualification found in: ${description}`) + .toMatch(OWNERLESS_LIMIT); + }); + + it('discloses the platform write floor that survives on an owner-less object', () => { + expect(description, `no surviving-floor disclosure found in: ${description}`) + .toMatch(SURVIVING_FLOOR); + }); +}); diff --git a/packages/spec/src/security/permission.zod.ts b/packages/spec/src/security/permission.zod.ts index dd2a1e4d45..a718a1c282 100644 --- a/packages/spec/src/security/permission.zod.ts +++ b/packages/spec/src/security/permission.zod.ts @@ -164,12 +164,28 @@ export const ObjectPermissionSchema = lazySchema(() => strictObject( */ viewAllRecords: z.boolean().default(false).describe('View All Data (Bypass Sharing)'), - /** - * Modify All Records: Super-user write access. - * Bypasses Sharing Rules and Ownership checks. + /** + * Modify All Records: Super-user write access. + * Bypasses Sharing Rules and Ownership checks — as RECORD SHARING computes + * them, i.e. on every object `ISharingService` enforces on, which is any + * object carrying an owner field (the common case, and the one this bit is + * granted for). * Equivalent to Microsoft Dataverse "Organization" level write access. + * + * [#6698] It is NOT a bypass of every ownership check the platform runs. On + * an object with NO owner field record sharing does not enforce at all — + * `checkEdit` / `checkDelete` answer `abstain` before the bypass is ever + * probed (#6428's tri-state) — so the platform's own row-level WRITE floor + * (`created_by == current_user.id`, shipped as the wildcard + * `owner_only_writes` / `owner_only_deletes` policies that answer #1985) + * stays in force, and a by-id write to another user's row is still refused. + * Measured and pinned in plugin-security's + * `row-write-widener-composition.test.ts`. Widening that cell would be a + * RUNTIME change in `plugin-sharing` (option B on #6698) and is deliberately + * not taken — what moved here is only the declaration, so that it stops + * over-claiming (ADR-0049 `declared ≠ enforced`). */ - modifyAllRecords: z.boolean().default(false).describe('Modify All Data (Bypass Sharing)'), + 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).'), /** * [ADR-0057 D1] Read access DEPTH (Dataverse-style access level), layered on