diff --git a/.changeset/system-data-import-opt-in.md b/.changeset/system-data-import-opt-in.md new file mode 100644 index 0000000000..5166c46ca8 --- /dev/null +++ b/.changeset/system-data-import-opt-in.md @@ -0,0 +1,66 @@ +--- +"@objectstack/spec": major +--- + +feat(spec)!: `system-data` 的桶默认不再包含 CSV `import`,改为按对象显式 opt-in (#4671) + +**FROM → TO:`managedBy: 'system-data'` 的默认 affordance 从 +`create/import/edit/delete/exportCsv: true` 收窄为 +`create/edit/delete/exportCsv: true`,`import: false`。** 需要 CSV 导入向导的对象 +显式写一行: + +```ts +export const SysHolidayCalendar = ObjectSchema.create({ + name: 'sys_holiday_calendar', + managedBy: 'system-data', + userActions: { import: true }, // 明确要这个入口 +}); +``` + +`platform` 现在是唯一默认授予 `import` 的桶。其余五个桶(`config`、`system-data`、 +`engine-owned`、`append-only`、`better-auth`)一致地把它留给对象自己声明。 + +## 具体消失的是哪几个 UI 入口 + +仓内 8 个 `system-data` 对象都不再从桶默认继承导入向导,其中要紧的是三张 RBAC 关联表 —— +它们是整个权限模型的**授予面**: + +| 对象 | v17-rc.3 之前的管理台入口 | 本次之后 | +| :--- | :--- | :--- | +| `sys_user_position` | 「CSV 批量绑定用户 ↔ 岗位」 | 不再出现(需显式 opt-in) | +| `sys_user_permission_set` | 「CSV 批量绑定用户 ↔ 权限集」 | 不再出现(需显式 opt-in) | +| `sys_position_permission_set` | 「CSV 批量绑定岗位 ↔ 权限集」 | 不再出现(需显式 opt-in) | + +另外 5 个成员(`sys_user_preference`、`sys_approval_delegation`、 +`sys_notification_template`、`sys_notification_subscription`、 +`sys_notification_preference`)同样从「有导入入口」回到「无导入入口」。 + +**要恢复其中任意一个,在该对象上加 `userActions: { import: true }` 即可** —— 只动 +`import` 这一个动词,create/edit/delete/exportCsv 仍走桶默认,不需要像 v16 那样把整块 +`userActions` 抄回来。 + +## 为什么 + +授权边界一点没动。`import` 是 **affordance**,只决定 UI 入口是否渲染;CSV 导入写下的每一行 +仍然逐条经过 `DelegatedAdminGate`、RLS 与权限集裁决 —— 一个无权手工授予某权限集的 admin, +通过 CSV 同样授不出去(ADR-0103 D5 关于 enforcement 的结论完全不变)。 + +变的是**杠杆**:逐行点选时一次误操作影响一个人;一份错误 CSV 就是一次批量授权,且没有天然的 +复核节奏 —— 而这三张表恰好决定「谁能做什么」。所以批量授予入口应当是一次显式声明,而不是 +「被归进了正确的桶」就自动继承的东西。对成批继承桶默认的 AI 生成对象元数据尤其如此: +「没想过 import」的默认结果落在安全侧,打开它则是 reviewer 能看见的一行。 + +原先「默认含 import」出自 #3355 上更早的 agent 会话(评论带 Claude Code 脚注),不是维护者 +拍板;当时的实现 agent 自己标注了这条 security-adjacent 并指出裁决可能未考虑批量绑定权限集 +这一具体场景。维护者 2026-08-03 正式裁决收窄,2026-08-06 最终确认。记录见 ADR-0103 的 +#4671 addendum。 + +## 升级影响 + +**从 v16 升上来的用户:零影响。** v16 的 `managedBy: 'system'` 默认 LOCKED,8 个成员各自用 +`userActions: { create, edit, delete }` 重开写入,没有一个重开 `import` —— 所以 CSV 导入在 +v16 就解析为 `false`,改名后仍是 `false`。#3355 的 4 个包逐对象 before/after 等价 pin 因此 +从「四动词等价 + 一条 import 差异」变成**五动词全等价**,并新增一条 opt-in 可达性 pin。 + +**已在 v17 rc.1–rc.3 上依赖 `system-data` 默认导入入口的用户:** 加 +`userActions: { import: true }`。 diff --git a/content/docs/data-modeling/objects.mdx b/content/docs/data-modeling/objects.mdx index 232b5eaf59..f3725f4712 100644 --- a/content/docs/data-modeling/objects.mdx +++ b/content/docs/data-modeling/objects.mdx @@ -243,7 +243,7 @@ indexes: [ | :--- | :--- | :--- | | `isSystem` | `boolean` | System object, protected from deletion (default: `false`) | | `managedBy` | `enum` | Lifecycle bucket that sets the default CRUD affordances and write policy — `'platform'` (default), `'config'`, `'system-data'`, `'engine-owned'`, `'append-only'`, `'better-auth'`. See [Lifecycle bucket](#lifecycle-bucket-managedby) below. | -| `userActions` | `object` | Per-object override of the CRUD affordances the `managedBy` default implies — `{ create?, edit?, delete?, import?, exportCsv? }`. This is what NARROWS a `system-data` object, or opens a verb on an `engine-owned`/`append-only` one. See [Lifecycle bucket](#lifecycle-bucket-managedby). | +| `userActions` | `object` | Per-object override of the CRUD affordances the `managedBy` default implies — `{ create?, edit?, delete?, import?, exportCsv? }`. This is what NARROWS a `system-data` object, opens CSV `import` on one, or opens a verb on an `engine-owned`/`append-only` one. See [Lifecycle bucket](#lifecycle-bucket-managedby). | | `sharingModel` | `enum` | Org-Wide Default record visibility (ADR-0055/0056/0090). Canonical four only: `'private'`, `'public_read'`, `'public_read_write'`, `'controlled_by_parent'` (detail visibility derived from its master). The legacy aliases (`'read'`, `'read_write'`, `'full'`) were removed from the enum (ADR-0090 D4) — authoring rejects them. Unset on a custom object resolves to `'private'` (ADR-0090 D1) | | `ownership` | `enum` | Record-ownership model: `'user'` (default — injects the reassignable `owner_id` lookup, engaging owner-scoped RLS, "My" views and owner reports), `'org'`, or `'none'` (no per-record owner — Dataverse-style catalog / junction tables, skips `owner_id`). Distinct from the package `own`/`extend` contribution kind. | | `validations` | `ValidationRule[]` | Object-level validation rules (see [Validation](/docs/data-modeling/validation)) | @@ -260,7 +260,7 @@ bare bucket string. | :--- | :--- | | `platform` | **Default.** User-owned business data — full New / Import / Edit / Delete. | | `config` | Admin-authored configuration — New / Edit / Delete, no CSV import. | -| `system-data` | Platform-defined schema holding **admin/user-writable data** (RBAC link tables, preferences, messaging config). Full CRUD by default; narrow it with `userActions`. | +| `system-data` | Platform-defined schema holding **admin/user-writable data** (RBAC link tables, preferences, messaging config). New / Edit / Delete / Export by default — **no CSV import**, which is opt-in per object; narrow the rest with `userActions`. | | `engine-owned` | Runtime rows a platform service owns end to end — generic CRUD hidden, exposed `['get', 'list']` only, **no user writes ever**. | | `append-only` | Immutable audit trail — View + Export only. | | `better-auth` | Identity tables owned by the better-auth driver — generic user-context CRUD is suppressed; mutations flow through the auth API (sign-in, invite, reset). | @@ -275,15 +275,15 @@ platform-defined schema no tenant may model; they differ on who owns the *rows*: guard (`assertEngineOwnedWriteAllowed`) rejects user-context generic writes. - **`system-data`** — the schema is the platform's, the *data* is the admin's or the user's: the RBAC link tables, `sys_user_preference`, - `sys_approval_delegation`, the messaging config grids. Full CRUD by default, - and no write guard covers the bucket — a writable default has nothing to fail - closed on: + `sys_approval_delegation`, the messaging config grids. New / Edit / Delete / + Export by default, and no write guard covers the bucket — a writable default + has nothing to fail closed on: ```typescript export const SysUserPreference = ObjectSchema.create({ name: 'sys_user_preference', - // Full CRUD by default — no `userActions` needed. RLS / delegated - // administration is the actual authz. + // New / Edit / Delete / Export by default — no `userActions` needed. RLS / + // delegated administration is the actual authz. managedBy: 'system-data', // … }); @@ -299,11 +299,45 @@ delete: false }`) and OPENS a verb on `append-only`. Either way it is an *affordance* declaration; the real authorization for these rows is still enforced by RLS, delegated administration, and permission sets. +#### CSV import on `system-data` is opt-in + +`system-data` is the one writable bucket that does **not** hand out the CSV +bulk-import wizard. `platform` is now the only bucket whose default grants +`import`: + +```typescript +export const SysUserPermissionSet = ObjectSchema.create({ + name: 'sys_user_permission_set', + managedBy: 'system-data', + // No `userActions` → New / Edit / Delete / Export, but no Import wizard. +}); + +export const SysHolidayCalendar = ObjectSchema.create({ + name: 'sys_holiday_calendar', + managedBy: 'system-data', + // Bulk loading a year of dates from a spreadsheet is the whole point here, + // so this object asks for the wizard explicitly. + userActions: { import: true }, +}); +``` + +The reason is leverage, not authorization. The bucket's charter members are the +RBAC link tables — `sys_user_position`, `sys_user_permission_set`, +`sys_position_permission_set` — which are the grant surface of the entire +permission model. Every row a CSV import writes still passes the delegated-admin +gate, RLS and permission-set adjudication one at a time, so an admin who cannot +grant a permission set by hand cannot grant it by file either. What differs is +blast radius: row by row, one misclick affects one person; one wrong CSV is a +bulk grant with no natural review rhythm. Making the wizard a per-object +declaration keeps "nobody thought about import" resolving to the safe answer. + **Upgrading from v16.** `managedBy: 'system'` was retired in protocol 17 — rename it to `'system-data'`, or run `os migrate meta --from 16`. Because the -new bucket defaults to full CRUD, a `userActions` block that existed only to -re-open create/edit/delete is now redundant and can be deleted. +new bucket defaults to New / Edit / Delete / Export, a `userActions` block that +existed only to re-open create/edit/delete is now redundant and can be deleted. +CSV import needs no attention either way: a v16 `system` object resolved +`import: false`, and so does its renamed `system-data` self. diff --git a/docs/adr/0103-managedby-write-policy-and-engine-write-guard.md b/docs/adr/0103-managedby-write-policy-and-engine-write-guard.md index 086903396e..489c72df78 100644 --- a/docs/adr/0103-managedby-write-policy-and-engine-write-guard.md +++ b/docs/adr/0103-managedby-write-policy-and-engine-write-guard.md @@ -233,7 +233,9 @@ are therefore deleted, and `userActions` on this bucket now only NARROWS. The affordance side-effect is that CSV `import` resolves `true` where it resolved `false` under the locked default — an affordance change only; every row a CSV import writes is still adjudicated by the `DelegatedAdminGate` / RLS / permission -sets. +sets. *(This last sentence is **revised** by the #4671 addendum below: `import` +was taken back out of the bucket default before v17 shipped, so it resolves +`false` on both sides and the flip described here never reached a release.)* **Enforcement is unchanged, as in D5.** `system-data` joins `platform` / `config` as a bucket neither `ENGINE_OWNED_BUCKETS` (guard) nor `GUARDED_WRITE_BUCKETS` @@ -250,3 +252,62 @@ covers the bucket to catch it. `ObjectSchema.create()` therefore **refuses** `system-data` on an object whose resolved affordances grant no create, edit or delete — a contradiction with no honest reading, computable from the declaration alone. Partial narrowing stays legal; only the all-writes-false shape is refused. + +--- + +## Addendum (v17, #4671) — CSV `import` is opt-in on `system-data` + +This addendum **revises one line** of the #3355 addendum above: the affordance +side-effect that let CSV `import` resolve `true` on the writable default. Nothing +else about the rename, the enum retirement, the conversion, the enforcement +boundary or the mis-assignment refusal changes. + +**Why it was reopened.** The two comments on #3355 that settled "the `system-data` +default is `create/edit/delete/import/exportCsv: true`" carry a Claude Code +footer — they are an earlier **agent** session's adjudication, not the +maintainer's. The implementing agent said so itself, flagged the consequence as +security-adjacent, and noted the adjudication may not have had the specific +scenario in view. It did not. + +**The scenario.** Three of the bucket's eight charter members are the RBAC link +tables — `sys_user_position`, `sys_user_permission_set`, +`sys_position_permission_set` — i.e. the grant surface of the whole permission +model. Under v16 all eight resolved `import: false` (locked `system` plus +`userActions` blocks that only ever re-opened create/edit/delete), so the flip +would have put a "bulk-bind permission sets from a spreadsheet" entry point in the +admin console for the first time. + +**What is NOT at stake.** No authorization boundary. `import` is an affordance — +it decides whether a UI entry point renders. Every row a CSV import writes still +goes through `DelegatedAdminGate`, RLS and permission-set adjudication one at a +time, and an admin who cannot grant a permission set by hand cannot grant it by +file. D5's enforcement conclusions stand unchanged. + +**What is.** Leverage. Row by row, one misclick affects one person; one wrong CSV +is a bulk grant with no natural review rhythm, on precisely the tables that +decide who can do what. + +**Decision (maintainer, 2026-08-03; reconfirmed 2026-08-06).** +`CRUD_AFFORDANCE_DEFAULTS['system-data']` grants +`create / edit / delete / exportCsv` and **not** `import`. An object that wants +the wizard declares `userActions: { import: true }`. `platform` is now the only +bucket whose default grants `import`. + +Two axes carried it: + +- **Long-term soundness.** A batch entry point onto authorization data should be + an explicit declaration, not something eight objects inherit by being filed in + the right bucket. The bucket still describes its members completely — none of + the eight needs a `userActions` block to claw a verb back, so the v16 shape this + rename existed to end does not return. +- **Making AI-written metadata hard to get wrong.** Model-authored object metadata + inherits bucket defaults in bulk. Minimum leverage by default means the result + of "forgot to think about import" lands on the safe side, and turning it on is a + line a reviewer can see. + +**Consequence for the #3355 equivalence pins.** The one deliberate +non-equivalence is gone: the rename now moves **no** affordance on **any** verb, +so the per-object pins in `plugin-security`, `service-messaging`, +`plugin-approvals` and `platform-objects` assert full five-verb v16/v17 equality, +plus that the wizard is still reachable via the opt-in — a pin of the surviving +mechanism rather than of an absence. diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index aa6b2a1b30..2bc5afcee8 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -166,7 +166,7 @@ The `script` flow node converges on its one real path (#4343). It had four ways The same audit reaches the driver contract itself: `IDataDriver.findStream` is removed (#4484). It was REQUIRED — every driver and every test double had to implement it — and documented as the read "optimized for large datasets to avoid memory overflow", while two of its three implementations awaited `find()` for the whole result set and then yielded it row by row, reaching exactly the peak it promised to avoid; the third streamed for real but was the one read in that driver that skipped `buildFindOptions`, so it dropped `query.fields`. Nothing anywhere called it, which is why a contract method could carry an inverted guarantee for this long and why ~20 test doubles could satisfy it by throwing `not implemented`. Paged `find()` is the read that exists and is enforced (its total-order guarantee is checked by the shared pagination-conformance cases); a cursor-based read is worth building when a caller asks for one, which is the honest order. A TS/API surface, never stored — one semantic TODO for driver authors, no source rewrite, and no tombstone: `DriverInterfaceSchema` describes a contract that code IMPLEMENTS and nothing ever `.parse()`d a driver, so tsc is the only channel that could carry the prescription, and it carries it where it matters — at a call site. -Separately, `object.managedBy: 'system'` is retired in favour of `'system-data'` (#3355), finishing the split ADR-0103 began in v16. That split was deliberately ADDITIVE: the 20 engine-owned objects moved to the new explicit `engine-owned`, and the 8 admin/user-writable ones — the RBAC link tables, `sys_user_preference`, the three messaging config grids — stayed behind on `system`. What was left is a value whose name describes the half that had already moved out: "system" sitting on precisely the objects a user writes. That is not a cosmetic complaint. An author choosing between `system` and `engine-owned` had nothing in the vocabulary to choose on, so the bucket was re-overloadable by anyone reading the name in good faith — a model author most of all. `system-data` states both boundaries: the SCHEMA is the platform's (versus `platform`, which is tenant-modelled), the DATA is the admin's or the user's (versus `engine-owned`, where the engine owns both). Reusing `config` was considered and rejected — `sys_user_preference` is user-owned rather than admin-authored, and `config` suppresses CSV import — as was `platform-data`, which sits one word away from the unrelated `platform` in the same closed enum and would reintroduce the confusion at the point of choosing. Because v16 already drained the engine side, the conversion is a ONE-TO-ONE mechanical value rename with no judgement call. One deliberate consequence: `system` defaulted LOCKED and each object re-opened its writes through `userActions`, while `system-data` defaults WRITABLE, so those blocks become redundant and are deleted (keep `userActions` only to NARROW). No enforcement moves — the engine write guard, the DelegatedAdminGate, RLS and permission sets all adjudicate off resolved affordances and the principal, never off the bucket name; `system-data` simply joins `platform`/`config` as a bucket the guard does not cover, because a writable default has nothing to fail closed on. Retired from the load path: the enum rejection is what teaches the new spelling, and absorbing `'system'` silently at load would leave every author writing the name this rename exists to retire. +Separately, `object.managedBy: 'system'` is retired in favour of `'system-data'` (#3355), finishing the split ADR-0103 began in v16. That split was deliberately ADDITIVE: the 20 engine-owned objects moved to the new explicit `engine-owned`, and the 8 admin/user-writable ones — the RBAC link tables, `sys_user_preference`, the three messaging config grids — stayed behind on `system`. What was left is a value whose name describes the half that had already moved out: "system" sitting on precisely the objects a user writes. That is not a cosmetic complaint. An author choosing between `system` and `engine-owned` had nothing in the vocabulary to choose on, so the bucket was re-overloadable by anyone reading the name in good faith — a model author most of all. `system-data` states both boundaries: the SCHEMA is the platform's (versus `platform`, which is tenant-modelled), the DATA is the admin's or the user's (versus `engine-owned`, where the engine owns both). Reusing `config` was considered and rejected — `sys_user_preference` is user-owned rather than admin-authored, and `config` suppresses CSV import — as was `platform-data`, which sits one word away from the unrelated `platform` in the same closed enum and would reintroduce the confusion at the point of choosing. Because v16 already drained the engine side, the conversion is a ONE-TO-ONE mechanical value rename with no judgement call. One deliberate consequence: `system` defaulted LOCKED and each object re-opened its writes through `userActions`, while `system-data` defaults WRITABLE on create, edit, delete and exportCsv, so those blocks become redundant and are deleted (keep `userActions` only to NARROW). CSV `import` is the one verb that default deliberately withholds (#4671): it stays opt-in per object via `userActions: { import: true }`, so a v16 `system` object — which resolved `import: false`, because the re-open blocks only ever named create/edit/delete — keeps resolving `import: false` after the rename. The reason is leverage, not authorization: three of the eight members are the RBAC link tables, and a bulk-grant entry point on the permission model's grant surface should be a per-object declaration rather than something inherited by being filed in the right bucket. No enforcement moves — the engine write guard, the DelegatedAdminGate, RLS and permission sets all adjudicate off resolved affordances and the principal, never off the bucket name; `system-data` simply joins `platform`/`config` as a bucket the guard does not cover, because a writable default has nothing to fail closed on. Retired from the load path: the enum rejection is what teaches the new spelling, and absorbing `'system'` silently at load would leave every author writing the name this rename exists to retire. Finally, five keys retire because the advisory lint could never have warned about them (#4509): mapping `extractQuery` / `errorPolicy` / `batchSize`, and app `contextSelectors[].includeAll` / `.placement`. Four of the five carry schema DEFAULTS, and a default materialises at parse time — so the liveness lint cannot tell a value the author wrote from one the schema supplied, and marking them would have warned on every mapping and every selector in existence. For a key in that state removal is not the escalation after a warning; it is the only channel that ever reaches the author, which is why they ship inside the 17.0.0 window rather than after a deprecation cycle. What they claimed: `extractQuery` promised an export path no exporter implements (exports go through the ordinary query API); `errorPolicy` offered skip/abort/retry where error handling belongs to the import REQUEST; `batchSize` sized batches the write path sizes itself; `placement` offered a topbar that places nothing. `includeAll` is the one worth reading twice — it was not unread but deliberately DISOBEYED, because context selectors are mandatory-scope and an "All" row would clear the scope: on Studio's package selector that means listing the platform's own system/cloud kernel packages to a developer who scoped to their package. `STUDIO_APP` authored `includeAll: true` against a renderer that ignored it. The mapping prescription for `batchSize` deliberately offers no rename: bulk-action, connector, sync, offline, seed-loader and NoSQL-cursor `batchSize` are all live, but each is a different key sizing its own path — the same trap `datasource.retryPolicy` vs `hook`/`job` `retryPolicy` had to defuse one issue earlier. diff --git a/packages/objectql/src/registry.test.ts b/packages/objectql/src/registry.test.ts index a62bf04b9c..a07f1d14d8 100644 --- a/packages/objectql/src/registry.test.ts +++ b/packages/objectql/src/registry.test.ts @@ -970,9 +970,11 @@ describe('reconcileManagedApiMethods', () => { const writable: any = { name: 'sys_user_position', // #3355: was `managedBy: 'system'` + a `userActions` re-open block. - // The rename made full CRUD the bucket default, so the reconciliation - // must reach the same "strip nothing" answer with no `userActions` at - // all — that equivalence is the whole claim of the rename. + // The rename made create/edit/delete/exportCsv the bucket default, so + // the reconciliation must reach the same "strip nothing" answer with no + // `userActions` at all — that equivalence is the whole claim of the + // rename. (CSV `import` is opt-in on this bucket since #4671, and is + // not an `apiMethods` verb, so it does not enter this reconciliation.) managedBy: 'system-data', enable: { apiEnabled: true, apiMethods: ['get', 'list', 'create', 'update', 'delete'] }, }; diff --git a/packages/platform-objects/src/identity/sys-user-preference.managed-by.test.ts b/packages/platform-objects/src/identity/sys-user-preference.managed-by.test.ts index f4a8c0b38e..f4d1437a61 100644 --- a/packages/platform-objects/src/identity/sys-user-preference.managed-by.test.ts +++ b/packages/platform-objects/src/identity/sys-user-preference.managed-by.test.ts @@ -19,7 +19,12 @@ import { resolveCrudAffordances } from '@objectstack/spec/data'; import { SysUserPreference } from './sys-user-preference.object.js'; const V16_EXPECTED = { create: true, import: false, edit: true, delete: true, exportCsv: true }; -const V17_EXPECTED = { create: true, import: true, edit: true, delete: true, exportCsv: true }; +/** + * Byte-identical to {@link V16_EXPECTED} since #4671 narrowed the bucket default's + * `import` to opt-in — kept as its own constant so a future move of EITHER side + * shows up as a diff rather than being absorbed by a shared literal. + */ +const V17_EXPECTED = { create: true, import: false, edit: true, delete: true, exportCsv: true }; /** * The v16 shape, reconstructed via `engine-owned` — which ADR-0103 D5 gave the @@ -36,21 +41,24 @@ describe('#3355 — sys_user_preference moves to `system-data` with its affordan expect(SysUserPreference.userActions).toBeUndefined(); }); - it('resolves the full-CRUD matrix from the bucket default alone', () => { + it('resolves create / edit / delete / exportCsv — but NOT import — from the bucket default alone', () => { expect(resolveCrudAffordances(SysUserPreference as never)).toEqual(V17_EXPECTED); }); - it('is write-equivalent to its v16 self on create / edit / delete / exportCsv', () => { + it('is affordance-equivalent to its v16 self on EVERY verb, import included (#4671)', () => { const v16 = resolveCrudAffordances(asV16 as never); const v17 = resolveCrudAffordances(SysUserPreference as never); expect(v16).toEqual(V16_EXPECTED); - for (const verb of ['create', 'edit', 'delete', 'exportCsv'] as const) { + for (const verb of ['create', 'import', 'edit', 'delete', 'exportCsv'] as const) { expect(v17[verb], `sys_user_preference.${verb} must not move`).toBe(v16[verb]); } + expect(v17).toEqual(v16); }); - it('gains CSV import — the one adjudicated delta, pinned so it cannot move silently', () => { - expect(resolveCrudAffordances(asV16 as never).import).toBe(false); - expect(resolveCrudAffordances(SysUserPreference as never).import).toBe(true); + it('keeps CSV import opt-IN — off by bucket default, reachable only by declaring it (#4671)', () => { + expect(resolveCrudAffordances(SysUserPreference as never).import).toBe(false); + const optedIn = resolveCrudAffordances({ ...SysUserPreference, userActions: { import: true } } as never); + expect(optedIn.import).toBe(true); + expect({ ...optedIn, import: false }).toEqual(V17_EXPECTED); }); }); diff --git a/packages/plugins/plugin-approvals/src/sys-approval-delegation.object.test.ts b/packages/plugins/plugin-approvals/src/sys-approval-delegation.object.test.ts index f63f565d38..5050f7f1fd 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-delegation.object.test.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-delegation.object.test.ts @@ -6,7 +6,8 @@ * * The object is `managedBy: 'system-data'` (#3355 — it was `'system'` plus a * `userActions: { create, edit, delete }` re-open block until v17 renamed the - * bucket and made full CRUD the default) and grants generic writes deliberately: + * bucket and made create/edit/delete/exportCsv the default; CSV import is opt-in + * per object since #4671) and grants generic writes deliberately: * an out-of-office rule is authored by its own user through the plain data * endpoint. So the ADR-0103 D3 reconciliation strips nothing and its boilerplate * CRUD-five whitelist reaches the REST gate as authored. Since the #3391 P1 @@ -26,7 +27,12 @@ import { SysApprovalDelegation } from './sys-approval-delegation.object'; */ describe('#3355 — sys_approval_delegation moves to `system-data` with its affordances intact', () => { const V16_EXPECTED = { create: true, import: false, edit: true, delete: true, exportCsv: true }; - const V17_EXPECTED = { create: true, import: true, edit: true, delete: true, exportCsv: true }; + /** + * Byte-identical to {@link V16_EXPECTED} since #4671 narrowed the bucket + * default's `import` to opt-in — kept as its own constant so a future move of + * EITHER side shows up as a diff rather than being absorbed by a shared literal. + */ + const V17_EXPECTED = { create: true, import: false, edit: true, delete: true, exportCsv: true }; /** * The v16 shape, reconstructed via `engine-owned` — which ADR-0103 D5 gave the * byte-identical locked default row `system` carried in v16. The retired @@ -41,22 +47,25 @@ describe('#3355 — sys_approval_delegation moves to `system-data` with its affo expect(SysApprovalDelegation.userActions).toBeUndefined(); }); - it('resolves the full-CRUD matrix from the bucket default alone', () => { + it('resolves create / edit / delete / exportCsv — but NOT import — from the bucket default alone', () => { expect(resolveCrudAffordances(SysApprovalDelegation as never)).toEqual(V17_EXPECTED); }); - it('is write-equivalent to its v16 self on create / edit / delete / exportCsv', () => { + it('is affordance-equivalent to its v16 self on EVERY verb, import included (#4671)', () => { const v16 = resolveCrudAffordances(asV16 as never); const v17 = resolveCrudAffordances(SysApprovalDelegation as never); expect(v16).toEqual(V16_EXPECTED); - for (const verb of ['create', 'edit', 'delete', 'exportCsv'] as const) { + for (const verb of ['create', 'import', 'edit', 'delete', 'exportCsv'] as const) { expect(v17[verb], `sys_approval_delegation.${verb} must not move`).toBe(v16[verb]); } + expect(v17).toEqual(v16); }); - it('gains CSV import — the one adjudicated delta, pinned so it cannot move silently', () => { - expect(resolveCrudAffordances(asV16 as never).import).toBe(false); - expect(resolveCrudAffordances(SysApprovalDelegation as never).import).toBe(true); + it('keeps CSV import opt-IN — off by bucket default, reachable only by declaring it (#4671)', () => { + expect(resolveCrudAffordances(SysApprovalDelegation as never).import).toBe(false); + const optedIn = resolveCrudAffordances({ ...SysApprovalDelegation, userActions: { import: true } } as never); + expect(optedIn.import).toBe(true); + expect({ ...optedIn, import: false }).toEqual(V17_EXPECTED); }); }); diff --git a/packages/plugins/plugin-security/src/objects/managed-by-system-data.test.ts b/packages/plugins/plugin-security/src/objects/managed-by-system-data.test.ts index 56613665fc..9e112a1d3a 100644 --- a/packages/plugins/plugin-security/src/objects/managed-by-system-data.test.ts +++ b/packages/plugins/plugin-security/src/objects/managed-by-system-data.test.ts @@ -14,14 +14,20 @@ * less than {@link V17_EXPECTED} and the per-object assert goes red; * - a bucket left on the old value (or moved to `engine-owned`) → same. * - * The one DELIBERATE non-equivalence is `import`, and it is pinned as such rather - * than waved past: `system` was locked-with-no-import and the `userActions` blocks - * only ever re-opened create/edit/delete, so CSV import resolved FALSE; the - * `system-data` default grants it. That flip is the maintainer's explicit - * adjudication on #3355 ("默认 affordance 可写 create/edit/delete/import/exportCsv: - * true"). It is an affordance only — the DelegatedAdminGate still adjudicates every - * row a CSV import would write — but it IS new UI surface on the RBAC link tables, - * so it gets its own named assertion that a future edit cannot flip back silently. + * `import` USED to be the one deliberate non-equivalence: `system` was + * locked-with-no-import and the `userActions` blocks only ever re-opened + * create/edit/delete, so CSV import resolved FALSE, while the v17 `system-data` + * default granted it. #4671 retired that delta — the bucket default no longer + * carries `import`, so the rename now moves NO affordance on ANY verb and these + * three tables are affordance-identical to their v16 selves. + * + * These are precisely the objects that decided #4671. Authorization was never the + * question — the DelegatedAdminGate, RLS and permission sets adjudicate every row + * a CSV import would write, so no boundary was ever bypassed. The question was + * LEVERAGE: these three tables ARE the grant surface of the whole RBAC model, and + * one wrong CSV is one bulk grant with no natural review rhythm. So the wizard + * became opt-IN — the last test below pins that it is still REACHABLE, which is + * what keeps this file from passing for the empty reason that nothing is produced. */ import { describe, expect, it } from 'vitest'; @@ -33,8 +39,14 @@ import { SysPositionPermissionSet } from './sys-position-permission-set.object.j /** What each object resolved to in v16: LOCKED `system` + `userActions: {c,e,d}`. */ const V16_EXPECTED = { create: true, import: false, edit: true, delete: true, exportCsv: true }; -/** What each object resolves to in v17: the `system-data` default, no `userActions`. */ -const V17_EXPECTED = { create: true, import: true, edit: true, delete: true, exportCsv: true }; +/** + * What each object resolves to in v17: the `system-data` default, no `userActions`. + * + * Byte-identical to {@link V16_EXPECTED} since #4671 — kept as a separate constant + * on purpose, so that a future move of EITHER side shows up as a diff here rather + * than being absorbed by a shared literal. + */ +const V17_EXPECTED = { create: true, import: false, edit: true, delete: true, exportCsv: true }; /** * The v16 declaration shape, reconstructed so the equivalence is COMPUTED rather @@ -69,22 +81,30 @@ describe('#3355 — RBAC link tables move to `system-data` with their affordance expect(obj.userActions).toBeUndefined(); }); - it('resolves the full-CRUD matrix from the bucket default alone', () => { + it('resolves create / edit / delete / exportCsv — but NOT import — from the bucket default alone', () => { expect(resolveCrudAffordances(obj as never)).toEqual(V17_EXPECTED); }); - it('is write-equivalent to its v16 self on create / edit / delete / exportCsv', () => { + it('is affordance-equivalent to its v16 self on EVERY verb, import included (#4671)', () => { const v16 = resolveCrudAffordances(asV16(obj) as never); const v17 = resolveCrudAffordances(obj as never); expect(v16).toEqual(V16_EXPECTED); // the reconstruction is honest - for (const verb of ['create', 'edit', 'delete', 'exportCsv'] as const) { + for (const verb of ['create', 'import', 'edit', 'delete', 'exportCsv'] as const) { expect(v17[verb], `${name}.${verb} must not move`).toBe(v16[verb]); } + expect(v17).toEqual(v16); }); - it('gains CSV import — the one adjudicated delta, pinned so it cannot move silently', () => { - expect(resolveCrudAffordances(asV16(obj) as never).import).toBe(false); - expect(resolveCrudAffordances(obj as never).import).toBe(true); + it('keeps CSV import opt-IN — off by bucket default, reachable only by declaring it (#4671)', () => { + // Off by inheritance: a member that never thought about import gets the + // safe side. This is the half a deleted `system-data` row would NOT + // satisfy — `platform` is the fallback and it grants import. + expect(resolveCrudAffordances(obj as never).import).toBe(false); + // Still reachable: the verb was made opt-in, not removed. Without this + // half the test above would pass for the empty reason. + const optedIn = resolveCrudAffordances({ ...obj, userActions: { import: true } } as never); + expect(optedIn.import).toBe(true); + expect({ ...optedIn, import: false }).toEqual(V17_EXPECTED); // opt-in moves import and nothing else }); }); } diff --git a/packages/services/service-messaging/src/objects/managed-by-system-data.test.ts b/packages/services/service-messaging/src/objects/managed-by-system-data.test.ts index 356fa39e7d..f891a0fbe3 100644 --- a/packages/services/service-messaging/src/objects/managed-by-system-data.test.ts +++ b/packages/services/service-messaging/src/objects/managed-by-system-data.test.ts @@ -20,7 +20,12 @@ import { NotificationSubscription } from './notification-subscription.object.js' import { NotificationPreference } from './notification-preference.object.js'; const V16_EXPECTED = { create: true, import: false, edit: true, delete: true, exportCsv: true }; -const V17_EXPECTED = { create: true, import: true, edit: true, delete: true, exportCsv: true }; +/** + * Byte-identical to {@link V16_EXPECTED} since #4671 narrowed the bucket default's + * `import` to opt-in — kept as its own constant so a future move of EITHER side + * shows up as a diff rather than being absorbed by a shared literal. + */ +const V17_EXPECTED = { create: true, import: false, edit: true, delete: true, exportCsv: true }; /** * The v16 declaration shape, reconstructed so the equivalence is COMPUTED rather @@ -54,22 +59,25 @@ describe('#3355 — messaging config grids move to `system-data` with their affo expect(obj.userActions).toBeUndefined(); }); - it('resolves the full-CRUD matrix from the bucket default alone', () => { + it('resolves create / edit / delete / exportCsv — but NOT import — from the bucket default alone', () => { expect(resolveCrudAffordances(obj as never)).toEqual(V17_EXPECTED); }); - it('is write-equivalent to its v16 self on create / edit / delete / exportCsv', () => { + it('is affordance-equivalent to its v16 self on EVERY verb, import included (#4671)', () => { const v16 = resolveCrudAffordances(asV16(obj) as never); const v17 = resolveCrudAffordances(obj as never); expect(v16).toEqual(V16_EXPECTED); - for (const verb of ['create', 'edit', 'delete', 'exportCsv'] as const) { + for (const verb of ['create', 'import', 'edit', 'delete', 'exportCsv'] as const) { expect(v17[verb], `${name}.${verb} must not move`).toBe(v16[verb]); } + expect(v17).toEqual(v16); }); - it('gains CSV import — the one adjudicated delta, pinned so it cannot move silently', () => { - expect(resolveCrudAffordances(asV16(obj) as never).import).toBe(false); - expect(resolveCrudAffordances(obj as never).import).toBe(true); + it('keeps CSV import opt-IN — off by bucket default, reachable only by declaring it (#4671)', () => { + expect(resolveCrudAffordances(obj as never).import).toBe(false); + const optedIn = resolveCrudAffordances({ ...obj, userActions: { import: true } } as never); + expect(optedIn.import).toBe(true); + expect({ ...optedIn, import: false }).toEqual(V17_EXPECTED); }); }); } diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index e24bebcb62..401f016162 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -1808,9 +1808,51 @@ describe('managedBy: retiring the overloaded `system` bucket (#3355)', () => { expect(msg).not.toMatch(/removed in @objectstack\/spec 17/s); }); - it('`system-data` defaults to full CRUD — the bucket says the data is yours', () => { + it('`system-data` defaults to writable CRUD — the bucket says the data is yours', () => { expect(resolveCrudAffordances({ managedBy: 'system-data' } as never)).toEqual({ - create: true, import: true, edit: true, delete: true, exportCsv: true, + create: true, import: false, edit: true, delete: true, exportCsv: true, + }); + }); + + /** + * #4671 — CSV import is the ONE verb the writable default does not hand out. + * + * The bucket's charter members are the three RBAC link tables + * (`sys_user_position`, `sys_user_permission_set`, + * `sys_position_permission_set`), i.e. the grant surface of the whole + * permission model. Authorization is untouched — the DelegatedAdminGate, RLS + * and permission sets adjudicate every row a CSV import would write, and an + * admin who cannot grant a permission set by hand cannot grant it by file + * either. What moves is LEVERAGE: row-by-row, one misclick is one person; one + * wrong CSV is one bulk grant with no natural review rhythm. So the wizard is + * a per-object declaration rather than something eight objects inherit by + * being filed in the right bucket — and the default result of "nobody thought + * about import" is the safe one, which is the shape that matters most for + * model-authored object metadata. + */ + describe('`system-data` makes CSV import opt-IN (#4671)', () => { + it('does not grant import by bucket default', () => { + expect(resolveCrudAffordances({ managedBy: 'system-data' } as never).import).toBe(false); + }); + + it('grants it when the object declares `userActions: { import: true }`, and moves nothing else', () => { + expect(resolveCrudAffordances({ + managedBy: 'system-data', + userActions: { import: true }, + } as never)).toEqual({ + create: true, import: true, edit: true, delete: true, exportCsv: true, + }); + }); + + it('leaves `platform` — the one bucket that still grants import by default — alone', () => { + expect(resolveCrudAffordances({} as never).import).toBe(true); + expect(resolveCrudAffordances({ managedBy: 'platform' } as never).import).toBe(true); + }); + + it('is now the same answer every non-`platform` bucket gives', () => { + for (const bucket of ['config', 'system-data', 'engine-owned', 'append-only', 'better-auth'] as const) { + expect(resolveCrudAffordances({ managedBy: bucket } as never).import, bucket).toBe(false); + } }); }); diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 7ac54be5b6..fdf1c9aa42 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -1097,10 +1097,13 @@ const MANAGED_BY_SYSTEM_RETIRED = + 'labelling admin/user-writable platform DATA under a name that says the opposite. ' + "Use `managedBy: 'system-data'` (platform-defined schema, admin/user-writable data; " + 'authz stays the DelegatedAdminGate / RLS / permission sets). Rename the value; nothing ' - + 'else about the object changes. Note `system-data` defaults to FULL CRUD affordances ' - + '(the old `system` default was locked), so a `userActions` block that existed only to ' - + 're-open create/edit/delete is now redundant and can be deleted — keep it only to ' - + 'NARROW. Run `os migrate meta --from 16` to rewrite it automatically.'; + + 'else about the object changes. Note `system-data` defaults to WRITABLE affordances — ' + + 'create, edit, delete and exportCsv (the old `system` default was locked) — so a ' + + '`userActions` block that existed only to re-open create/edit/delete is now redundant ' + + 'and can be deleted; keep it only to NARROW. CSV `import` is deliberately NOT in that ' + + 'default (#4671): it stays opt-in per object via `userActions: { import: true }`, which ' + + 'is what a v16 `system` object already resolved to. Run `os migrate meta --from 16` to ' + + 'rewrite it automatically.'; const ObjectSchemaBase = z.object({ /** @@ -2028,8 +2031,9 @@ function assertSystemDataIsWritable( + 'false. Use `managedBy: \'engine-owned\'` for rows a platform service owns end to end ' + '(written via `isSystem` / a service SYSTEM_CTX), or `append-only` for an immutable ' + 'audit log. If the object IS user-writable, drop the `userActions` entries closing ' - + 'create/edit/delete — the `system-data` default is full CRUD, and `userActions` is for ' - + 'NARROWING only (#3355).', + + 'create/edit/delete — the `system-data` default already grants create, edit, delete and ' + + 'exportCsv, so `userActions` is for NARROWING those (#3355). The one verb it does not ' + + 'grant is CSV `import`, which is opt-in per object (#4671).', ); } @@ -2191,7 +2195,12 @@ export type Lifecycle = z.infer; export interface CrudAffordances { /** Generic "New" button (single record creation form). */ create: boolean; - /** CSV bulk-import wizard. Disabled for config / system / append-only / better-auth by default. */ + /** + * CSV bulk-import wizard. `platform` is the only bucket that grants it by + * default; every other bucket — `config`, `system-data`, `engine-owned`, + * `append-only`, `better-auth` — makes it opt-in via + * `userActions: { import: true }`. + */ import: boolean; /** Inline + form editing of existing rows. */ edit: boolean; @@ -2232,6 +2241,12 @@ export interface RowCrudPredicates { * (RBAC link tables, prefs, messaging config). DEFAULT is * WRITABLE — the bucket exists to say "the data is yours" — * and an object that takes less NARROWS via `userActions`. + * The ONE exception is CSV import, which is opt-IN here: + * the bucket's charter members are the RBAC link tables, and + * a bulk-import entry point on a grant table is a lever a + * bucket default should not hand out by inheritance (#4671). + * An object that genuinely wants the wizard writes + * `userActions: { import: true }`. * Affordance declaration only; authz stays the delegated-admin * gate / RLS / permission sets (ADR-0103, renamed from the * locked-default `system` in v17 — #3355) @@ -2246,7 +2261,7 @@ export interface RowCrudPredicates { const CRUD_AFFORDANCE_DEFAULTS: Record | 'platform', CrudAffordances> = { platform: { create: true, import: true, edit: true, delete: true, exportCsv: true }, config: { create: true, import: false, edit: true, delete: true, exportCsv: true }, - 'system-data': { create: true, import: true, edit: true, delete: true, exportCsv: true }, + 'system-data': { create: true, import: false, edit: true, delete: true, exportCsv: true }, 'engine-owned': { create: false, import: false, edit: false, delete: false, exportCsv: true }, 'append-only': { create: false, import: false, edit: false, delete: false, exportCsv: true }, 'better-auth': { create: false, import: false, edit: false, delete: false, exportCsv: true }, diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 91ea4cb54d..6a08d25441 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -623,8 +623,16 @@ const step17: MigrationStep = { + 'reintroduce the confusion at the point of choosing. Because v16 already drained the ' + 'engine side, the conversion is a ONE-TO-ONE mechanical value rename with no judgement ' + 'call. One deliberate consequence: `system` defaulted LOCKED and each object re-opened its ' - + 'writes through `userActions`, while `system-data` defaults WRITABLE, so those blocks ' - + 'become redundant and are deleted (keep `userActions` only to NARROW). No enforcement ' + + 'writes through `userActions`, while `system-data` defaults WRITABLE on create, edit, ' + + 'delete and exportCsv, so those blocks become redundant and are deleted (keep ' + + '`userActions` only to NARROW). CSV `import` is the one verb that default deliberately ' + + 'withholds (#4671): it stays opt-in per object via `userActions: { import: true }`, so a ' + + 'v16 `system` object — which resolved `import: false`, because the re-open blocks only ' + + 'ever named create/edit/delete — keeps resolving `import: false` after the rename. The ' + + 'reason is leverage, not authorization: three of the eight members are the RBAC link ' + + 'tables, and a bulk-grant entry point on the permission model\'s grant surface should be ' + + 'a per-object declaration rather than something inherited by being filed in the right ' + + 'bucket. No enforcement ' + 'moves — the engine write guard, the DelegatedAdminGate, RLS and permission sets all ' + 'adjudicate off resolved affordances and the principal, never off the bucket name; ' + '`system-data` simply joins `platform`/`config` as a bucket the guard does not cover, '