From b093dab8f87109be26a7288bbbb9d0cc83ab5dc9 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Fri, 10 Jul 2026 13:03:32 +0800 Subject: [PATCH] =?UTF-8?q?feat(data):=20enable.files=20goes=20live=20?= =?UTF-8?q?=E2=80=94=20opt-in=20gate=20for=20the=20generic=20Attachments?= =?UTF-8?q?=20surface=20(#2727)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last dead ObjectCapabilities flag gets its enforcement contract. enable.files stays opt-IN (default false) — the Attachments panel is a new surface, not an existing behavior: - plugin-audit sys_attachment beforeInsert hook: attachment join rows may only target objects explicitly declaring enable.files true; absent block/flag, explicit false, and unknown parent objects all reject fail-closed with 403 FILES_DISABLED (CLONE_DISABLED/FEEDS_DISABLED pattern). - mapDataError maps FILES_DISABLED → 403 with the gated target object (generic data routes bypass sendError's .status passthrough — the #2707 lesson applied at introduction time). - Field.file / Field.image stay independent: they store the file URL in the record's own column and never create sys_attachment rows. - Liveness ledger: enable.files dead→live, authorWarn dropped — ObjectCapabilities is now 100% live; describe() and reference docs state the real contract; cli lint contract tests re-anchored on `versioning` as the canonical misleading-dead example. Verified: unit (4 new gate tests, FILES_DISABLED mapping, lint contract, 32/139/464 suites green), check:liveness + check:api-surface green, and live E2E on a fresh showcase — presigned 3-step upload → attach to a files:true probe object 201; attach targeting a no-flag object → 403 FILES_DISABLED; stable download 200. Browser-verified via the console dev server: panel renders with upload/list/download/delete on the files:true object and is absent on a no-flag object. Companion objectui PR ships RecordAttachmentsPanel. Closes #2727. Co-Authored-By: Claude Fable 5 --- .../files-capability-attachments-2727.md | 33 ++++++++++++ content/docs/references/data/object.mdx | 2 +- .../utils/lint-liveness-properties.test.ts | 42 ++++++--------- .../plugin-audit/src/audit-writers.test.ts | 54 +++++++++++++++++++ .../plugins/plugin-audit/src/audit-writers.ts | 30 +++++++++++ packages/rest/src/rest-server.ts | 19 +++---- packages/rest/src/rest.test.ts | 15 ++++++ packages/spec/liveness/README.md | 4 +- packages/spec/liveness/object.json | 6 +-- packages/spec/src/data/object.zod.ts | 17 +++--- 10 files changed, 172 insertions(+), 50 deletions(-) create mode 100644 .changeset/files-capability-attachments-2727.md diff --git a/.changeset/files-capability-attachments-2727.md b/.changeset/files-capability-attachments-2727.md new file mode 100644 index 0000000000..0cf783c389 --- /dev/null +++ b/.changeset/files-capability-attachments-2727.md @@ -0,0 +1,33 @@ +--- +'@objectstack/spec': minor +'@objectstack/plugin-audit': minor +'@objectstack/rest': patch +'@objectstack/cli': patch +--- + +feat(data): `enable.files` goes live — opt-in gate for the generic Attachments surface (#2727) + +The last dead ObjectCapabilities flag gets its enforcement contract. +`enable.files` is opt-IN (spec default stays `false`): the generic record +Attachments panel is a new surface, not an existing behavior. + +- plugin-audit registers a `sys_attachment` beforeInsert hook: attachment + join rows may only target objects that explicitly declare + `enable: { files: true }` — anything else (absent block, absent flag, + explicit false, unknown object) rejects fail-closed with + 403 `FILES_DISABLED` (CLONE_DISABLED / FEEDS_DISABLED pattern). +- `mapDataError` maps `FILES_DISABLED` → 403 with the gated target object + (generic data routes bypass `sendError`'s `.status` passthrough — the + #2707 lesson, applied at introduction time). +- `Field.file` / `Field.image` are deliberately independent: they store + the file URL in the record's own column and never create + `sys_attachment` rows, so field-level attachments work regardless of + this flag. +- Liveness ledger: `enable.files` dead→live, authorWarn dropped — + ObjectCapabilities is now 100% live. The compile-time + liveness-dead-property warning no longer fires for it; `describe()` and + the reference docs state the real contract. + +Companion objectui PR ships `RecordAttachmentsPanel` (upload/list/ +download/delete over the presigned three-step storage flow), rendered on +record pages when the flag is true. diff --git a/content/docs/references/data/object.mdx b/content/docs/references/data/object.mdx index c62e9b8045..fe02ea184b 100644 --- a/content/docs/references/data/object.mdx +++ b/content/docs/references/data/object.mdx @@ -81,7 +81,7 @@ const result = ApiMethod.parse(data); | **searchable** | `boolean` | ✅ | Index records for global search | | **apiEnabled** | `boolean` | ✅ | Expose object via automatic APIs | | **apiMethods** | `Enum<'get' \| 'list' \| 'create' \| 'update' \| 'delete' \| 'upsert' \| 'bulk' \| 'aggregate' \| 'history' \| 'search' \| 'restore' \| 'purge' \| 'import' \| 'export'>[]` | optional | Whitelist of allowed API operations | -| **files** | `boolean` | ✅ | RESERVED (no runtime effect yet) — use Field.file/Field.image for attachments; this flag will gate the future generic Attachments panel | +| **files** | `boolean` | ✅ | Generic record Attachments panel (sys_attachment). Opt-in: true surfaces the panel and permits attachments targeting this object; otherwise creation is rejected. Field.file/Field.image are independent | | **feeds** | `boolean` | ✅ | Record comments/collaboration feed. Default on; explicit false hides the feed UI and rejects new comments for this object | | **activities** | `boolean` | ✅ | Record activity timeline (sys_activity mirror of CRUD). Default on; explicit false stops mirroring and hides the timeline | | **trash** | `boolean` | ✅ | Enable soft-delete with restore capability | diff --git a/packages/cli/src/utils/lint-liveness-properties.test.ts b/packages/cli/src/utils/lint-liveness-properties.test.ts index 495ca66fa1..f9884703fa 100644 --- a/packages/cli/src/utils/lint-liveness-properties.test.ts +++ b/packages/cli/src/utils/lint-liveness-properties.test.ts @@ -9,7 +9,7 @@ import { /** * These run against the REAL ledgers shipped by `@objectstack/spec` (the same * files the gate enforces), so they double as a contract test: if an - * `authorWarn` annotation is removed from `enable.files` / `columnName` / etc., + * `authorWarn` annotation is removed from `versioning` / `columnName` / etc., * the matching assertion fails. */ @@ -18,14 +18,13 @@ const rules = (findings: { rule: string }[]) => findings.map((f) => f.rule); const paths = (findings: { message: string }[]) => findings.map((f) => f.message); describe('lintLivenessProperties', () => { - it('warns on an authored dead capability flag (enable.files: true)', () => { - const findings = lintLivenessProperties(objStack({ enable: { files: true } })); - expect(findings.length).toBeGreaterThan(0); - const files = findings.find((f) => f.message.includes('enable.files')); - expect(files).toBeDefined(); - expect(files!.rule).toBe(LIVENESS_DEAD_PROPERTY); - expect(files!.where).toBe("object 'widget'"); - expect(files!.hint.length).toBeGreaterThan(0); + it('warns on a present dead object block (versioning)', () => { + const findings = lintLivenessProperties(objStack({ versioning: { enabled: true } })); + const v = findings.find((f) => f.message.includes('versioning')); + expect(v).toBeDefined(); + expect(v!.rule).toBe(LIVENESS_DEAD_PROPERTY); + expect(v!.where).toBe("object 'widget'"); + expect(v!.hint.length).toBeGreaterThan(0); }); it('does NOT warn on a default-on flag the author left alone (enable.trash: true)', () => { @@ -33,25 +32,14 @@ describe('lintLivenessProperties', () => { expect(paths(findings).some((m) => m.includes('enable.trash'))).toBe(false); }); - // #2707: feeds/activities/trackHistory went LIVE (opt-out writer/UI gates + - // History-tab master switch) — authoring them must no longer warn. - it('does NOT warn on the now-live capability flags (feeds/activities/trackHistory)', () => { + // #2707/#2727: every ObjectCapabilities flag is now LIVE (opt-out + // writer/UI gates, the History-tab master switch, the opt-in Attachments + // gate) — authoring them must no longer warn. + it('does NOT warn on the now-live capability flags (feeds/activities/trackHistory/files)', () => { const findings = lintLivenessProperties( - objStack({ enable: { feeds: true, activities: true, trackHistory: true } }), + objStack({ enable: { feeds: true, activities: true, trackHistory: true, files: true } }), ); - expect(paths(findings).some((m) => m.includes('enable.feeds'))).toBe(false); - expect(paths(findings).some((m) => m.includes('enable.activities'))).toBe(false); - expect(paths(findings).some((m) => m.includes('enable.trackHistory'))).toBe(false); - }); - - it('does NOT warn when a dead boolean flag is explicitly false (enable.files: false)', () => { - const findings = lintLivenessProperties(objStack({ enable: { files: false } })); - expect(paths(findings).some((m) => m.includes('enable.files'))).toBe(false); - }); - - it('warns on a present dead object block (versioning)', () => { - const findings = lintLivenessProperties(objStack({ versioning: { enabled: true } })); - expect(paths(findings).some((m) => m.includes('versioning'))).toBe(true); + expect(paths(findings).some((m) => m.includes('enable.'))).toBe(false); }); it('warns on a misleading dead field prop (columnName)', () => { @@ -83,7 +71,7 @@ describe('lintLivenessProperties', () => { it('handles objects as a keyed record (not just arrays)', () => { const findings = lintLivenessProperties({ - objects: { widget: { name: 'widget', enable: { files: true } } }, + objects: { widget: { name: 'widget', versioning: { enabled: true } } }, }); expect(rules(findings)).toContain(LIVENESS_DEAD_PROPERTY); }); diff --git a/packages/plugins/plugin-audit/src/audit-writers.test.ts b/packages/plugins/plugin-audit/src/audit-writers.test.ts index 898c0fe877..7aa750de91 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.test.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.test.ts @@ -385,3 +385,57 @@ describe('audit writers — enable.feeds server-side enforcement (#2707)', () => await expect(fire('beforeInsert', commentInsert('ghost_object:rec-9'))).resolves.toBeUndefined(); }); }); + +describe('audit writers — enable.files server-side enforcement (#2727)', () => { + const SCHEMA = { + sys_audit_log: SINGLE_TENANT.sys_audit_log, + sys_activity: SINGLE_TENANT.sys_activity, + crm_lead: ['id', 'name'], + }; + + const attachmentInsert = (parentObject?: unknown) => ({ + object: 'sys_attachment', + input: { data: { parent_object: parentObject, parent_id: 'rec-1', file_id: 'file-1' } }, + session: {}, + }); + + it('allows sys_attachment creation when the parent object declares files: true', async () => { + const { engine, fire } = makeEngine(SCHEMA, { + crm_lead: { enable: { files: true } }, + }); + installAuditWriters(engine as any, 'test.audit'); + + await expect(fire('beforeInsert', attachmentInsert('crm_lead'))).resolves.toBeUndefined(); + }); + + it('rejects when the flag is absent — opt-in means explicit (403 FILES_DISABLED)', async () => { + const noBlock = makeEngine(SCHEMA); + installAuditWriters(noBlock.engine as any, 'test.audit'); + await expect(noBlock.fire('beforeInsert', attachmentInsert('crm_lead'))).rejects.toMatchObject({ + code: 'FILES_DISABLED', + status: 403, + object: 'crm_lead', + }); + + const explicitFalse = makeEngine(SCHEMA, { crm_lead: { enable: { files: false } } }); + installAuditWriters(explicitFalse.engine as any, 'test.audit'); + await expect(explicitFalse.fire('beforeInsert', attachmentInsert('crm_lead'))).rejects.toMatchObject({ + code: 'FILES_DISABLED', + }); + }); + + it('rejects an unknown parent object (fail-closed, unlike the opt-out feeds gate)', async () => { + const { engine, fire } = makeEngine(SCHEMA); + installAuditWriters(engine as any, 'test.audit'); + await expect(fire('beforeInsert', attachmentInsert('ghost_object'))).rejects.toMatchObject({ + code: 'FILES_DISABLED', + object: 'ghost_object', + }); + }); + + it('leaves a missing parent_object to schema validation (no gate error)', async () => { + const { engine, fire } = makeEngine(SCHEMA, { crm_lead: { enable: { files: false } } }); + installAuditWriters(engine as any, 'test.audit'); + await expect(fire('beforeInsert', attachmentInsert(undefined))).resolves.toBeUndefined(); + }); +}); diff --git a/packages/plugins/plugin-audit/src/audit-writers.ts b/packages/plugins/plugin-audit/src/audit-writers.ts index d7158fc284..cb6f6168bf 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.ts @@ -562,6 +562,36 @@ export function installAuditWriters( }; engine.registerHook('beforeInsert', enforceFeedsCapability, { object: 'sys_comment', packageId }); + /** + * `enable.files` server-side enforcement (#2727). The generic Attachments + * panel persists `sys_attachment` join rows through the generic data path, + * so — like the feeds gate above — the engine hook seam is the one gate + * every caller crosses. Unlike feeds, `files` is opt-IN (spec default + * `false`): the panel is a new surface, not an existing behavior, so a + * parent object must declare `enable: { files: true }` before attachments + * may target it. Fail-closed: an absent enable block, an absent flag, and + * an unknown parent object all reject — opt-in means *explicit*. + * + * Deliberately NOT gated: `Field.file` / `Field.image` uploads. Those + * store the file URL in the record's own column via service-storage and + * never create a sys_attachment row, so field-level attachments keep + * working regardless of this flag. + */ + const enforceFilesCapability = async (ctx: HookContext) => { + const data: any = (ctx.input as any)?.data; + const parentObject = data?.parent_object; + if (typeof parentObject !== 'string' || parentObject.length === 0) return; // schema requires it; let validation report the miss + const def = getObjectDef(parentObject); + if (def?.enable?.files !== true) { + const err: any = new Error(`File attachments are not enabled for object '${parentObject}' (requires enable.files: true)`); + err.code = 'FILES_DISABLED'; + err.status = 403; + err.object = parentObject; + throw err; + } + }; + engine.registerHook('beforeInsert', enforceFilesCapability, { object: 'sys_attachment', packageId }); + /** * M10.8: Dedicated hook on `sys_comment` afterInsert that parses the * `mentions` JSON field and writes one sys_notification per mentioned diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index eef869f5fc..2037f3038d 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -104,18 +104,19 @@ export function mapDataError(error: any, object?: string): { status: number; bod }, }; } - // Capability gate (#2707): the target object opted out of comments - // (`enable.feeds: false`) — plugin-audit's engine hook rejects the - // sys_comment insert fail-closed. 403 like CLONE_DISABLED; surfaced by - // `code` because the generic data routes map through here (they never - // reach sendError's `.status` passthrough). `error.object` names the - // gated TARGET object (not sys_comment), so prefer it. - if (error?.code === 'FEEDS_DISABLED') { + // Capability gates (#2707 feeds / #2727 files): plugin-audit's engine + // hooks reject sys_comment / sys_attachment inserts fail-closed when the + // TARGET object's capability flag disallows them. 403 like + // CLONE_DISABLED; surfaced by `code` because the generic data routes map + // through here (they never reach sendError's `.status` passthrough). + // `error.object` names the gated TARGET object (not the join table), so + // prefer it. + if (error?.code === 'FEEDS_DISABLED' || error?.code === 'FILES_DISABLED') { return { status: 403, body: { - error: error?.message ?? 'Comments are disabled for this object', - code: 'FEEDS_DISABLED', + error: error?.message ?? 'This capability is disabled for the target object', + code: error.code, ...(error?.object || object ? { object: error?.object ?? object } : {}), }, }; diff --git a/packages/rest/src/rest.test.ts b/packages/rest/src/rest.test.ts index 689e09e95a..6716c703bc 100644 --- a/packages/rest/src/rest.test.ts +++ b/packages/rest/src/rest.test.ts @@ -1947,6 +1947,21 @@ describe('mapDataError — schema/constraint envelopes', () => { expect(r.body.object).toBe('gate_probe'); }); + // #2727: same shape for the opt-in attachments gate on sys_attachment. + it('maps FILES_DISABLED → 403 with the gated target object', () => { + const r = mapDataError( + Object.assign(new Error("File attachments are not enabled for object 'crm_lead' (requires enable.files: true)"), { + code: 'FILES_DISABLED', + status: 403, + object: 'crm_lead', + }), + 'sys_attachment', + ); + expect(r.status).toBe(403); + expect(r.body.code).toBe('FILES_DISABLED'); + expect(r.body.object).toBe('crm_lead'); + }); + it('maps SQLite "has no column named" → 400 INVALID_FIELD with the field', () => { const r = mapDataError( sqliteError( diff --git a/packages/spec/liveness/README.md b/packages/spec/liveness/README.md index fae1c14423..7b6960783a 100644 --- a/packages/spec/liveness/README.md +++ b/packages/spec/liveness/README.md @@ -105,7 +105,7 @@ Signal over noise is the whole point, so warnings are **opt-in per entry**: Two rules keep it false-positive-free, **both of which the marker author must respect**: 1. **Only mark genuinely *misleading* dead props** — ones that imply a capability/behavior - that doesn't exist (`enable.files`, `field.columnName`, `versioning`). Benign display/doc + that doesn't exist (`versioning`, `field.columnName`, `softDelete`). Benign display/doc metadata that's "dead" (no runtime reader) — `description`, `tags`, `icon` — must NOT be marked; an author isn't misled by them. 2. **Booleans: only mark `default(false)` flags.** The lint warns on a boolean only when set @@ -163,7 +163,7 @@ The governed set is `GOVERNED` at the top of `check-liveness.mts`. To add a type | Type | live | exp | dead | Notes | |---|---|---|---|---| -| object | 34 | – | 14 | versioning/partitioning/cdc tier dead; ObjectCapabilities mostly live post-#2707 (`apiEnabled`/`apiMethods` enforced #1937; `feeds`/`activities` opt-out gates + `trackHistory` UI master switch); `enable.files` reserved/dead | +| object | 35 | – | 13 | versioning/partitioning/cdc tier dead; ObjectCapabilities fully live post-#2707/#2727 (`apiEnabled`/`apiMethods` enforced #1937; `feeds`/`activities` opt-out gates; `trackHistory` UI master switch; `files` opt-in Attachments gate) | | field | 34 | – | 39 | ~half dead — aspirational enhanced-type + governance config; naming-drift props server-live/client-snake | | flow | 29 | 1 | 7 | `runAs` experimental (unenforced identity switch); status/active gate nothing; FlowNodeAction enum out of sync | | action | 26 | – | 5 | `disabled` CEL ignored (renderers read non-spec `enabled`); type:'form'/shortcut/bulkEnabled dead | diff --git a/packages/spec/liveness/object.json b/packages/spec/liveness/object.json index 6f52c9c57b..e2c2a3952b 100644 --- a/packages/spec/liveness/object.json +++ b/packages/spec/liveness/object.json @@ -165,10 +165,8 @@ "_authorWarnSkipped": "defaults to true in the schema — the lint can't distinguish author-set-true from the default, so warning here would fire on every object with an enable block. Only default-FALSE booleans are safe to authorWarn." }, "files": { - "status": "dead", - "evidence": "no behavior-changing reader in framework or objectui — reserved for the generic Attachments related-list (Salesforce Notes & Attachments parity), tracked in #2727", - "authorWarn": true, - "authorHint": "File attachments are modeled with a `Field.file`/`Field.image` (or the sys_attachment object), not this object flag — it enables nothing on its own yet (reserved for the future generic Attachments panel)." + "status": "live", + "evidence": "packages/plugins/plugin-audit/src/audit-writers.ts (enforceFilesCapability beforeInsert hook: sys_attachment rows may only target objects declaring enable.files true — 403 FILES_DISABLED otherwise); objectui RecordAttachmentsPanel renders the record Attachments surface (upload/list/download/delete) when the flag is true. Opt-in — spec default false (#2727)." }, "feeds": { "status": "live", diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 07eda22d17..9f2b8a9347 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -74,15 +74,18 @@ export const ObjectCapabilities = z.object({ apiMethods: z.array(ApiMethod).optional().describe('Whitelist of allowed API operations'), /** - * Standard attachments/files engine — opt-in, NOT YET CONSUMED at runtime. + * Generic Attachments panel (Salesforce "Notes & Attachments" parity) — + * opt-in. * - * Reserved for the generic Attachments related-list (Salesforce - * "Notes & Attachments" parity). Until that ships, model attachments with - * `Field.file` / `Field.image` (or relate to `sys_attachment`) — setting - * this flag alone enables nothing, and the compile-time liveness lint - * warns on it. + * Contract (#2727): `true` surfaces the record Attachments panel in the + * console (upload/list/download/delete over `sys_attachment` join rows) + * and permits `sys_attachment` rows to target this object; anything else + * rejects new attachments server-side (403 FILES_DISABLED, enforced at + * the engine hook seam by plugin-audit — opt-in means explicit). + * `Field.file` / `Field.image` column attachments are independent of + * this flag. */ - files: z.boolean().default(false).describe('RESERVED (no runtime effect yet) — use Field.file/Field.image for attachments; this flag will gate the future generic Attachments panel'), + files: z.boolean().default(false).describe('Generic record Attachments panel (sys_attachment). Opt-in: true surfaces the panel and permits attachments targeting this object; otherwise creation is rejected. Field.file/Field.image are independent'), /** * Social collaboration (Comments, Mentions, Feeds) — opt-out.