From b96345ee00b8c253dcc465e23874600e38372b43 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 16:13:22 +0000 Subject: [PATCH 1/2] feat(security)!: export axis goes opt-in, gains explain, covers reports (#3544, #3710) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups to #3709, in one change because they share one semantic. BREAKING: `allowExport` unset no longer inherits read. Export is now an opt-in grant like every other `allow*` bit — reading a record and taking a bulk machine-readable copy of the whole table are different privileges. Migration: add `allowExport: true` to the object entry (or `'*'`) of any permission set whose holders should keep exporting. Package-shipped sets are re-seeded, so the built-ins are handled; environment-authored sets are not. - spec/plugin-security: `resolveUserExportAllowed` becomes a grant fold (any set `true` → granted; unset and `false` are both "no grant", since permission sets are additive and nothing in them is a deny). The super-user bits deliberately do NOT imply export — separating "may see all data" from "may take a bulk copy" is the segregation-of-duties case the axis exists for. `admin_full_access` / `organization_admin` carry the grant explicitly; `member_default` deliberately does not. - spec: a set carrying `allowExport` is high-privilege, so it cannot be bound to the `everyone` / `guest` anchors — otherwise the opt-in was defeatable by binding an export-granting set to everyone. One shared predicate, so the runtime gate, the lint rule and the suggestion surface all pick it up. - spec/plugin-security: `ExplainOperationSchema` gains `export`, so an admin can ask WHY a caller got 403 EXPORT_NOT_PERMITTED — `explain(read)` would answer `allowed` and tell them nothing. object_crud reports `read ∧ grant` and attributes the granting set; every data-shaped layer is computed as the `find` the export performs, so RLS is resolved against a real select policy instead of an `export` the compiler has none for. - plugin-reports: closes the side door (#3710). A csv/json report is the same bulk copy of the same object, gated by the same canExport in `executeReport` — which the interactive run, ad-hoc run and scheduled dispatch all funnel through. `scheduleReport` also refuses at create time. A schedule created while granted stops delivering once the grant is revoked. `html_table` stays a read. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PDbwCy9Jrc1chhR2vnAUos --- .changeset/export-axis-opt-in.md | 75 ++++++ content/docs/permissions/explain.mdx | 13 +- content/docs/permissions/permission-sets.mdx | 60 +++-- content/docs/references/security/explain.mdx | 5 +- .../docs/references/security/permission.mdx | 4 +- .../src/effective-api-operations.test.ts | 86 ++++--- .../plugin-hono-server/src/hono-plugin.ts | 22 +- .../src/report-export-axis.test.ts | 220 ++++++++++++++++++ .../plugin-reports/src/report-service.ts | 79 +++++++ .../plugin-reports/src/reports-plugin.ts | 17 ++ .../src/audience-anchors.test.ts | 27 +++ .../src/explain-engine.test.ts | 86 +++++++ .../plugin-security/src/explain-engine.ts | 37 ++- .../src/export-permission-axis.test.ts | 143 ++++++------ .../src/objects/default-permission-sets.ts | 17 ++ .../src/permission-evaluator.ts | 52 ++--- packages/spec/src/security/explain.zod.ts | 17 +- packages/spec/src/security/high-privilege.ts | 16 +- packages/spec/src/security/permission.zod.ts | 38 ++- 19 files changed, 832 insertions(+), 182 deletions(-) create mode 100644 .changeset/export-axis-opt-in.md create mode 100644 packages/plugins/plugin-reports/src/report-export-axis.test.ts diff --git a/.changeset/export-axis-opt-in.md b/.changeset/export-axis-opt-in.md new file mode 100644 index 0000000000..f6a47715b3 --- /dev/null +++ b/.changeset/export-axis-opt-in.md @@ -0,0 +1,75 @@ +--- +"@objectstack/spec": major +"@objectstack/plugin-security": major +"@objectstack/plugin-reports": major +"@objectstack/plugin-hono-server": minor +--- + +feat(security)!: the export axis is now OPT-IN, explainable, and covers reports (#3544, #3710) + +**BREAKING — `allowExport` unset no longer means "inherit read".** Reading a +record and taking a bulk machine-readable copy of the whole table are different +privileges (Salesforce "Export Reports", Dynamics "Export to Excel", NetSuite +"Export Lists", SAP `S_GUI` 61 all separate them). The axis now says so. + +### Migration — FROM → TO + +| | before | after | +|---|---|---| +| `allowExport` unset | export **allowed** (inherited read) | export **denied** | +| `allowExport: false` | export denied | export denied (unchanged) | +| `allowExport: true` | export allowed | export allowed (unchanged) | + +**The one-line fix:** add `allowExport: true` to the object entry (or the `'*'` +wildcard) of every permission set whose holders should keep exporting. + +```ts +objects: { + deal: { allowRead: true, allowExport: true }, // ← add the grant +} +``` + +Nothing else changes: read, CRUD, RLS, FLS and sharing are untouched, and a set +that never exported is unaffected. + +**Who is affected.** Package-shipped sets are re-seeded on upgrade, so the +built-ins are handled for you — `admin_full_access` and `organization_admin` now +carry `allowExport: true` explicitly. **Environment-authored sets are not**: any +custom set whose users export must be edited. `member_default` deliberately does +NOT carry the grant, so ordinary authenticated users lose export until an admin +grants it — that is the point of the flip, not an oversight. + +**Merge semantics.** Most-permissive, exactly like the CRUD bits: any set +granting `true` grants export. `false` and unset are the same outcome; `false` +is authoring intent, not a veto, because permission sets are additive capability +containers (ADR-0090). + +**Not implied by super-user bits.** `viewAllRecords` / `modifyAllRecords` no +longer confer export. Separating "may see all data" from "may take a bulk copy" +is the segregation-of-duties case the axis exists for. + +### Also in this change + +- **spec** — a set carrying `allowExport` is now **high-privilege** + (`describeHighPrivilegeBits`), so it cannot be bound to the `everyone` / + `guest` audience anchors. Without this the opt-in was defeatable by binding an + export-granting set to `everyone`. One predicate, so the runtime anchor gate, + the `@objectstack/lint` security-posture rule and the install-time suggestion + surface all pick it up together. +- **spec / plugin-security** — `ExplainOperationSchema` gains `export`, so + `explain` can answer *why* a caller got `403 EXPORT_NOT_PERMITTED`. It + explains as `read ∧ the export grant`: `object_crud` reports the conjunction + and attributes the granting set, while every data-shaped layer + (requiredPermissions, OWD/depth/sharing, RLS, record attribution) is computed + as the `find` the export actually performs — asking the RLS compiler about an + `export` operation would match no policy and wrongly report "no RLS applies". + `readFilter` is surfaced for `export` as it is for `read`. +- **plugin-reports** — closes the reports side door (#3710). A report rendered + as `csv`/`json` is the same bulk copy of the same object, so it is gated by + the same `ISecurityService.canExport`. Enforced in `executeReport`, which the + interactive run, the ad-hoc run and the scheduled dispatch all funnel through; + `scheduleReport` additionally refuses at create time so an author is not told + at 3am. A schedule created while granted stops delivering once the grant is + revoked. `html_table` stays a read — it is a rendered view, not a bulk copy. + Deployments without `plugin-security` are unaffected (no permission sets + exist, so the axis does not apply). diff --git a/content/docs/permissions/explain.mdx b/content/docs/permissions/explain.mdx index 5fadeb751d..b240fcdd7b 100644 --- a/content/docs/permissions/explain.mdx +++ b/content/docs/permissions/explain.mdx @@ -71,7 +71,18 @@ curl -X POST -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/jso ``` `operation` is one of `read | create | update | delete | transfer | restore | -purge` (defaults to `read`); omit `userId` to explain **yourself**. +purge | export` (defaults to `read`); omit `userId` to explain **yourself**. + +`export` is the one operation whose answer you cannot infer from `read`. Since +the [export axis](/docs/permissions/permission-sets#allowexport--the-export-axis) +is opt-in, a user can be fully able to read an object and still be refused a +bulk copy of it — so when someone hits `403 EXPORT_NOT_PERMITTED`, ask with +`operation: 'export'`. Asking with `read` will answer `allowed` and tell you +nothing, because reading is precisely what they may still do. The `object_crud` +layer reports the conjunction (`read ∧ the export grant`) and names the set that +supplies it; every other layer is computed as the `find` the export performs, so +the `readFilter` you get back is the real row filter the export would stream +through. ## Who may ask diff --git a/content/docs/permissions/permission-sets.mdx b/content/docs/permissions/permission-sets.mdx index a43c58c735..47a5cadb3f 100644 --- a/content/docs/permissions/permission-sets.mdx +++ b/content/docs/permissions/permission-sets.mdx @@ -50,7 +50,7 @@ export const SalesUser = definePermissionSet({ | Permission | Description | |------------|-------------| | `allowCreate` / `allowRead` / `allowEdit` / `allowDelete` | CRUD on records the user can see | -| `allowExport` | Bulk data egress — narrows read, see below | +| `allowExport` | Bulk data egress — an opt-in grant on top of read, see below | | `allowTransfer` / `allowRestore` / `allowPurge` | Lifecycle class (RBAC-gated ahead of the M2 operations) | | `viewAllRecords` | Read ALL records regardless of ownership (super-user read) | | `modifyAllRecords` | Edit ALL records regardless of ownership (super-user write) | @@ -63,27 +63,49 @@ Salesforce ("Export Reports"), Dynamics ("Export to Excel"), NetSuite ("Export Lists") and SAP (`S_GUI` 61) all carry a separate export permission. `allowExport` is that axis here: `export = list ∧ allowExport`. -It is a **tri-state**, and unset is not the same as `false`: +It is an **opt-in grant**, like every other `allow*` bit: | Value | Meaning | |------------|-------------| -| unset | Inherit read — anyone who can list can export. The default, so adding the key changes nothing for existing permission sets. | -| `false` | Deny export while **keeping** read. The reason the axis exists. | -| `true` | Explicitly granted. | - -Across several permission sets the merge is most-permissive with an explicit -deny in the middle: any set saying `true` wins, otherwise any set saying `false` -wins, otherwise the bit stays unset and inherits read. - -It narrows read — it never widens it. A set granting `allowExport: true` without -a read grant exports nothing, and a `modifyAllRecords` super-user wildcard does -**not** override an explicit per-object `allowExport: false`. - -Enforcement is server-side: `GET /api/v1/data/:object/export` answers -`403 EXPORT_NOT_PERMITTED` before it reads the first row. The same decision is -published on `/me/permissions` as the object's effective `apiOperations`, which -is what makes the client hide its Export button — the button and the refusal are -one decision, not two. +| `true` | Export granted — still bounded by read. | +| unset | **No export.** Read alone never confers it. | +| `false` | No export. Same outcome as unset; write it when you want the intent on the record. | + +Across several permission sets the merge is most-permissive, exactly like the +CRUD bits: any set granting `true` grants export. `false` is therefore +documentation rather than a veto — permission sets are additive capability +containers, and nothing in them is a deny. + +Two consequences worth stating plainly: + +- **It narrows read; it never widens it.** A set granting `allowExport: true` + without a read grant exports nothing — `export = list ∧ allowExport`. +- **`viewAllRecords` / `modifyAllRecords` do not imply it.** Separating "may see + all data" from "may take a bulk copy of it" is the segregation-of-duties case + the axis exists for. The built-in `admin_full_access` and `organization_admin` + sets carry `allowExport: true` explicitly; remove that line to get the + separation. `member_default` deliberately does not carry it, so ordinary + authenticated users have no export by default. + +A set carrying `allowExport` is treated as **high-privilege** and cannot be +bound to the `everyone` or `guest` audience anchors — binding it there would +hand bulk egress to every authenticated user (or every visitor) and undo the +opt-in. Grant it through an ordinary position-distributed set instead. + +Enforcement is server-side and covers both egress doors: + +- `GET /api/v1/data/:object/export` answers `403 EXPORT_NOT_PERMITTED` before it + reads the first row. +- A **report** rendered as `csv` or `json` is the same bulk copy and is gated the + same way, whether run interactively or delivered by a schedule. `html_table` + is a rendered view and stays a read. + +The same decision is published on `/me/permissions` as the object's effective +`apiOperations`, which is what makes the client hide its Export button — the +button and the refusal are one decision, not two. To ask why a particular user +was refused, use [`explain`](/docs/permissions/explain) with +`operation: 'export'`; `operation: 'read'` will answer `allowed` and tell you +nothing, because reading is exactly what they are still permitted to do. ## Access depth — `readScope` / `writeScope` (ADR-0057 D1) diff --git a/content/docs/references/security/explain.mdx b/content/docs/references/security/explain.mdx index 98fb45f473..9f492fd7ec 100644 --- a/content/docs/references/security/explain.mdx +++ b/content/docs/references/security/explain.mdx @@ -120,7 +120,7 @@ ADR-0095 D2 posture rung — PLATFORM_ADMIN crosses the tenant wall where object | :--- | :--- | :--- | :--- | | **allowed** | `boolean` | ✅ | | | **object** | `string` | ✅ | | -| **operation** | `Enum<'read' \| 'create' \| 'update' \| 'delete' \| 'transfer' \| 'restore' \| 'purge'>` | ✅ | | +| **operation** | `Enum<'read' \| 'create' \| 'update' \| 'delete' \| 'transfer' \| 'restore' \| 'purge' \| 'export'>` | ✅ | | | **principal** | `{ userId: string \| null; positions: string[]; permissionSets: string[]; principalKind?: Enum<'human' \| 'agent' \| 'service' \| 'guest' \| 'system'>; … }` | ✅ | | | **layers** | `{ layer: Enum<'tenant_isolation' \| 'principal' \| 'required_permissions' \| 'object_crud' \| 'fls' \| 'owd_baseline' \| 'depth' \| 'sharing' \| 'vama_bypass' \| 'rls'>; kernelTier?: Enum<'layer_0_tenant' \| 'layer_1_business'>; verdict: Enum<'grants' \| 'denies' \| 'narrows' \| 'widens' \| 'neutral' \| 'not_applicable'>; detail: string; … }[]` | ✅ | | | **readFilter** | `any` | optional | | @@ -172,6 +172,7 @@ ADR-0095 D2 posture rung — PLATFORM_ADMIN crosses the tenant wall where object * `transfer` * `restore` * `purge` +* `export` --- @@ -198,7 +199,7 @@ ADR-0095 D2 posture rung — PLATFORM_ADMIN crosses the tenant wall where object | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **object** | `string` | ✅ | | -| **operation** | `Enum<'read' \| 'create' \| 'update' \| 'delete' \| 'transfer' \| 'restore' \| 'purge'>` | ✅ | | +| **operation** | `Enum<'read' \| 'create' \| 'update' \| 'delete' \| 'transfer' \| 'restore' \| 'purge' \| 'export'>` | ✅ | | | **recordId** | `string` | optional | Optional id of one concrete record to explain at row granularity; omitted = object-level (pre-C2) request. | | **userId** | `string` | optional | | diff --git a/content/docs/references/security/permission.mdx b/content/docs/references/security/permission.mdx index 7ea130d284..3b63556a60 100644 --- a/content/docs/references/security/permission.mdx +++ b/content/docs/references/security/permission.mdx @@ -59,7 +59,7 @@ const result = AdminScope.parse(data); | **allowRead** | `boolean` | ✅ | Read permission | | **allowEdit** | `boolean` | ✅ | Edit permission | | **allowDelete** | `boolean` | ✅ | Delete permission | -| **allowExport** | `boolean` | optional | [#3544] User-level export axis over read. Unset = inherit read (can-list ⇒ can-export, backward-compatible); false = deny export while keeping read; true = granted. | +| **allowExport** | `boolean` | optional | [#3544] User-level export axis over read (opt-in grant). true = export granted (still bounded by read); unset/false = no export. Merged most-permissively like the CRUD bits; NOT implied by viewAllRecords/modifyAllRecords. | | **allowTransfer** | `boolean` | ✅ | [RBAC-gated; ENFORCED now via insert/update owner_id guard, #3004] Change record ownership (assign/reassign/disown owner_id) | | **allowRestore** | `boolean` | ✅ | [RBAC-gated; operation pending M2] Restore from trash (Undelete) | | **allowPurge** | `boolean` | ✅ | [RBAC-gated; operation pending M2] Permanently delete (Hard Delete/GDPR) | @@ -107,7 +107,7 @@ const result = AdminScope.parse(data); | **allowRead** | `boolean` | ✅ | Read permission | | **allowEdit** | `boolean` | ✅ | Edit permission | | **allowDelete** | `boolean` | ✅ | Delete permission | -| **allowExport** | `boolean` | optional | [#3544] User-level export axis over read. Unset = inherit read (can-list ⇒ can-export, backward-compatible); false = deny export while keeping read; true = granted. | +| **allowExport** | `boolean` | optional | [#3544] User-level export axis over read (opt-in grant). true = export granted (still bounded by read); unset/false = no export. Merged most-permissively like the CRUD bits; NOT implied by viewAllRecords/modifyAllRecords. | | **allowTransfer** | `boolean` | ✅ | [RBAC-gated; ENFORCED now via insert/update owner_id guard, #3004] Change record ownership (assign/reassign/disown owner_id) | | **allowRestore** | `boolean` | ✅ | [RBAC-gated; operation pending M2] Restore from trash (Undelete) | | **allowPurge** | `boolean` | ✅ | [RBAC-gated; operation pending M2] Permanently delete (Hard Delete/GDPR) | diff --git a/packages/plugins/plugin-hono-server/src/effective-api-operations.test.ts b/packages/plugins/plugin-hono-server/src/effective-api-operations.test.ts index b2aaa2239f..55ace31b4e 100644 --- a/packages/plugins/plugin-hono-server/src/effective-api-operations.test.ts +++ b/packages/plugins/plugin-hono-server/src/effective-api-operations.test.ts @@ -15,8 +15,12 @@ import { describe('annotateEffectiveApiOperations (#3391)', () => { const schemaOf = (map: Record) => (name: string) => map[name]; + // [#3544] These pin the `apiMethods` DERIVATION, so they grant `allowExport` + // explicitly — since the export axis went opt-in, a perm entry without it + // strips `export` from the effective set and would mask what is under test. + // The axis itself is covered in its own describe block below. it('annotates a restricting object with its effective operation set', () => { - const objects: Record = { widget: { allowRead: true } }; + const objects: Record = { widget: { allowRead: true, allowExport: true } }; annotateEffectiveApiOperations(objects, schemaOf({ widget: { name: 'widget', enable: { apiMethods: ['get', 'list'] } }, })); @@ -25,7 +29,7 @@ describe('annotateEffectiveApiOperations (#3391)', () => { }); it('does NOT annotate an unrestricted object (client keeps default-allow)', () => { - const objects: Record = { widget: { allowRead: true } }; + const objects: Record = { widget: { allowRead: true, allowExport: true } }; annotateEffectiveApiOperations(objects, schemaOf({ widget: { name: 'widget', enable: {} }, // no apiMethods → unrestricted })); @@ -57,7 +61,7 @@ describe('annotateEffectiveApiOperations (#3391)', () => { }); it('reverse-derived import/export appear in the effective set for a CRUD whitelist', () => { - const objects: Record = { deal: { allowRead: true } }; + const objects: Record = { deal: { allowRead: true, allowExport: true } }; annotateEffectiveApiOperations(objects, schemaOf({ deal: { name: 'deal', enable: { apiMethods: ['get', 'list', 'create', 'update', 'delete'] } }, })); @@ -68,10 +72,10 @@ describe('annotateEffectiveApiOperations (#3391)', () => { }); // [#3544] user-level export axis: allowExport on the per-object perm entry - // drives userExportAllowed → export derives from list ∧ that bit. + // drives userExportAllowed → export derives from list ∧ that GRANT (opt-in). describe('user-level export axis (#3544)', () => { - it('allowExport:false removes export from a restricting object', () => { - const objects: Record = { deal: { allowRead: true, allowExport: false } }; + it('no export grant strips export from a restricting object', () => { + const objects: Record = { deal: { allowRead: true } }; annotateEffectiveApiOperations(objects, schemaOf({ deal: { name: 'deal', enable: { apiMethods: ['get', 'list'] } }, })); @@ -79,26 +83,26 @@ describe('annotateEffectiveApiOperations (#3391)', () => { expect(objects.deal.apiOperations).not.toContain('export'); }); - it('allowExport:false removes export even from an otherwise-open object (annotation forced)', () => { - const objects: Record = { deal: { allowRead: true, allowExport: false } }; + it('no export grant forces an annotation even on an otherwise-open object', () => { + const objects: Record = { deal: { allowRead: true } }; annotateEffectiveApiOperations(objects, schemaOf({ deal: { name: 'deal', enable: {} }, // no apiMethods → unrestricted })); - // Even though unrestricted, the export axis forces an annotation that - // excludes export while keeping the rest. + // Unrestricted, but the axis still forces an annotation that excludes + // export while keeping the rest — otherwise the client's default-allow + // path would show an Export button the server refuses. expect(objects.deal.apiOperations).toBeDefined(); expect(objects.deal.apiOperations).not.toContain('export'); expect(objects.deal.apiOperations).toContain('create'); expect(objects.deal.apiOperations).toContain('import'); }); - it('unset allowExport inherits read — export kept; unrestricted object still unannotated', () => { - const objects: Record = { deal: { allowRead: true } }; // allowExport unset + it('allowExport:false is annotated the same as silence', () => { + const objects: Record = { deal: { allowRead: true, allowExport: false } }; annotateEffectiveApiOperations(objects, schemaOf({ - deal: { name: 'deal', enable: {} }, + deal: { name: 'deal', enable: { apiMethods: ['get', 'list'] } }, })); - // Unrestricted + export allowed → no annotation (client default-allow). - expect('apiOperations' in objects.deal).toBe(false); + expect(objects.deal.apiOperations).not.toContain('export'); }); it('allowExport:true keeps export on a list-derived restricting object', () => { @@ -109,37 +113,45 @@ describe('annotateEffectiveApiOperations (#3391)', () => { expect(objects.deal.apiOperations).toContain('export'); }); + it('allowExport:true on an unrestricted object needs no annotation', () => { + // Nothing is being taken away, so the client keeps its default-allow path. + const objects: Record = { deal: { allowRead: true, allowExport: true } }; + annotateEffectiveApiOperations(objects, schemaOf({ + deal: { name: 'deal', enable: {} }, + })); + expect('apiOperations' in objects.deal).toBe(false); + }); + // The merge keeps `'*'` and named objects as independent keys, but the // SERVER evaluator does not — `resolveObjectPermission` falls back to the // wildcard for any object a set has no explicit entry for. Reading it here - // too is what keeps the hidden button and the refused request the same - // decision; without it a `'*': {allowExport:false}` set would still be - // offered an Export button that then 403s. - it("inherits the '*' export bit when the object entry declares none", () => { + // too is what keeps the shown button and the accepted request the same + // decision; without it an admin's `'*': {allowExport:true}` would have its + // Export button hidden on every object it never names explicitly. + it("inherits the '*' export grant when the object entry declares none", () => { const objects: Record = { - '*': { allowRead: true, allowExport: false }, + '*': { allowRead: true, allowExport: true }, deal: { allowRead: true }, // no allowExport of its own }; annotateEffectiveApiOperations(objects, schemaOf({ - deal: { name: 'deal', enable: {} }, // unrestricted + deal: { name: 'deal', enable: { apiMethods: ['get', 'list'] } }, })); - expect(objects.deal.apiOperations).toBeDefined(); - expect(objects.deal.apiOperations).not.toContain('export'); - expect(objects.deal.apiOperations).toContain('list'); + expect(objects.deal.apiOperations).toContain('export'); }); - it("an explicit per-object allowExport:true overrides a '*' deny", () => { + it("an explicit per-object allowExport:false overrides a '*' grant", () => { const objects: Record = { - '*': { allowRead: true, allowExport: false }, - deal: { allowRead: true, allowExport: true }, + '*': { allowRead: true, allowExport: true }, + deal: { allowRead: true, allowExport: false }, }; annotateEffectiveApiOperations(objects, schemaOf({ deal: { name: 'deal', enable: { apiMethods: ['get', 'list'] } }, })); - expect(objects.deal.apiOperations).toContain('export'); + expect(objects.deal.apiOperations).not.toContain('export'); }); - it("a '*' carrying no export bit changes nothing", () => { + it("a '*' carrying no export grant does not confer export", () => { + // The super-user bits specifically do NOT imply export. const objects: Record = { '*': { modifyAllRecords: true }, deal: { allowRead: true }, @@ -147,7 +159,8 @@ describe('annotateEffectiveApiOperations (#3391)', () => { annotateEffectiveApiOperations(objects, schemaOf({ deal: { name: 'deal', enable: {} }, })); - expect('apiOperations' in objects.deal).toBe(false); + expect(objects.deal.apiOperations).toBeDefined(); + expect(objects.deal.apiOperations).not.toContain('export'); }); }); }); @@ -188,7 +201,18 @@ describe('seedSuperUserRestrictedObjects (#3391)', () => { const objects: Record = { '*': { modifyAllRecords: true } }; seedSuperUserRestrictedObjects(objects, schemas); annotateEffectiveApiOperations(objects, (name) => schemas.find((s) => s.name === name)); - expect(objects.widget.apiOperations).toEqual(['get', 'list', 'aggregate', 'search', 'export']); + // [#3544] NO `export`: modifyAllRecords is a record-visibility bypass, not + // an export grant, and the axis is opt-in. A super-user who should also be + // able to take bulk copies carries `allowExport: true` explicitly — which + // is exactly what the built-in admin sets now do. + expect(objects.widget.apiOperations).toEqual(['get', 'list', 'aggregate', 'search']); expect(objects.locked.apiOperations).toEqual([]); }); + + it('end-to-end: a super-user wildcard CARRYING the export grant keeps export', () => { + const objects: Record = { '*': { modifyAllRecords: true, allowExport: true } }; + seedSuperUserRestrictedObjects(objects, schemas); + annotateEffectiveApiOperations(objects, (name) => schemas.find((s) => s.name === name)); + expect(objects.widget.apiOperations).toEqual(['get', 'list', 'aggregate', 'search', 'export']); + }); }); diff --git a/packages/plugins/plugin-hono-server/src/hono-plugin.ts b/packages/plugins/plugin-hono-server/src/hono-plugin.ts index 47b9bc9544..a6f08d7b46 100644 --- a/packages/plugins/plugin-hono-server/src/hono-plugin.ts +++ b/packages/plugins/plugin-hono-server/src/hono-plugin.ts @@ -340,26 +340,26 @@ export function annotateEffectiveApiOperations( objects: Record, schemaOf: (objectName: string) => ApiExposureSchemaLike | undefined, ): void { - // [#3544] The `'*'` entry's export bit is the FALLBACK for objects that do - // not declare one of their own. The merge keeps `'*'` and named objects as + // [#3544] The `'*'` entry's export grant is the FALLBACK for objects that do + // not carry one of their own. The merge keeps `'*'` and named objects as // independent keys, but the server evaluator does not: its // `resolveObjectPermission` falls back to the wildcard whenever a set has no - // explicit entry for the object, so a set that denies export wholesale via - // `'*': { allowExport: false }` really does deny it per-object. Reading the - // wildcard here keeps the button the client hides and the request the server - // refuses in agreement — the same class of client/server divergence + // explicit entry for the object, so an admin set granting export wholesale + // via `'*': { allowExport: true }` really does grant it per-object. Reading + // the wildcard here keeps the button the client shows and the request the + // server accepts in agreement — the same class of client/server divergence // `foldWildcardSuperUser` exists to close, on the export axis. const wildExport = objects?.['*']?.allowExport; for (const [obj, acc] of Object.entries(objects) as Array<[string, any]>) { if (obj === '*' || !acc) continue; const schema = schemaOf(obj); if (!schema) continue; // schema missing → no annotation (client falls back) - // [#3544] User-level export axis: `export` derives from `list ∧ this bit`. - // Unset → inherit the wildcard, then read (backward-compatible: - // can-list ⇒ can-export); explicit `false` → export removed from the - // effective set. + // [#3544] User-level export axis: `export` derives from `list ∧ this + // grant`. OPT-IN — only an explicit `true` (on the object entry, else + // inherited from `'*'`) allows export; unset and `false` both withhold + // it, and the super-user bits do NOT imply it. const exportBit = acc.allowExport ?? wildExport; - const userExportAllowed = exportBit !== false; + const userExportAllowed = exportBit === true; const eff = resolveEffectiveApiMethods(schema.enable ?? undefined, { userExportAllowed }); // Annotate when the object tightens via `apiMethods`, OR when the export // axis removes `export` from an otherwise-open object (so the client diff --git a/packages/plugins/plugin-reports/src/report-export-axis.test.ts b/packages/plugins/plugin-reports/src/report-export-axis.test.ts new file mode 100644 index 0000000000..0d801603d5 --- /dev/null +++ b/packages/plugins/plugin-reports/src/report-export-axis.test.ts @@ -0,0 +1,220 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3710 — the reports side door around the user-level export axis (#3544). + * + * `GET /data/:object/export` answers 403 for a caller without the export grant. + * A report over the SAME object rendered as CSV is the same bulk copy of the + * same rows, so before this gate a refused caller could simply save a report, + * run it as CSV — or schedule one to their own inbox — and get the data anyway. + * + * The gate lives in `executeReport`, which every path funnels through (run, + * ad-hoc, and the scheduled dispatch), so these exercise all three rather than + * trusting one call site. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { ReportService, type ReportEmail } from './report-service.js'; + +function makeFakeEngine() { + const tables: Record = {}; + const ensure = (n: string) => (tables[n] ??= []); + const matches = (row: any, filter: any): boolean => { + if (!filter || typeof filter !== 'object') return true; + for (const [k, v] of Object.entries(filter)) if (row[k] !== v) return false; + return true; + }; + return { + _tables: tables, + async find(object: string, options?: any) { + return ensure(object) + .filter((r) => matches(r, options?.filter ?? options?.where)) + .slice(0, options?.limit ?? 1000); + }, + async insert(object: string, data: any) { ensure(object).push({ ...data }); return { ...data }; }, + async update(object: string, idOrData: any, _opts?: any) { + const data = typeof idOrData === 'object' ? idOrData : _opts; + const id = typeof idOrData === 'object' ? idOrData.id : idOrData; + const table = ensure(object); + const i = table.findIndex((r) => r.id === id); + if (i >= 0) table[i] = { ...table[i], ...data }; + return table[i]; + }, + async delete(object: string, options?: any) { + const table = ensure(object); + const i = table.findIndex((r) => r.id === (options?.where?.id ?? options?.id)); + if (i >= 0) table.splice(i, 1); + return {}; + }, + }; +} + +function makeFakeEmail() { + const sent: any[] = []; + const email: ReportEmail & { _sent: any[] } = { + _sent: sent, + async send(input) { sent.push(input); return { status: 'sent' }; }, + }; + return email; +} + +const CTX = { userId: 'u1', tenantId: 't1', positions: [], permissions: [] }; +const now = new Date('2026-01-15T10:00:00Z'); + +describe('reports × the user-level export axis (#3710)', () => { + let engine: ReturnType; + let email: ReturnType; + let canExport: ReturnType; + + const build = (opts: { canExport?: any } = {}) => + new ReportService({ + engine: engine as any, + email, + clock: { now: () => now }, + maxRows: 5000, + resolveOwnerContext: async (ownerId: string) => + ownerId ? { userId: ownerId, tenantId: 't1', positions: [], permissions: [] } : null, + ...('canExport' in opts ? { canExport: opts.canExport } : { canExport }), + }); + + beforeEach(() => { + engine = makeFakeEngine(); + email = makeFakeEmail(); + canExport = vi.fn().mockResolvedValue(true); + engine._tables['lead'] = [ + { id: 'l1', name: 'Acme', status: 'open' }, + { id: 'l2', name: 'Globex', status: 'won' }, + ]; + }); + + const saveReport = async (svc: ReportService, format: 'csv' | 'json' | 'html_table') => + svc.saveReport( + { name: 'leads', object: 'lead', query: { fields: ['id', 'name'] }, format }, + CTX, + ); + + describe('interactive run', () => { + it('denies a CSV report run when the caller may not export the object', async () => { + const svc = build({ canExport: vi.fn().mockResolvedValue(false) }); + const report = await saveReport(svc, 'csv'); + await expect(svc.run(report.id, CTX)).rejects.toThrow(/EXPORT_NOT_PERMITTED/); + }); + + it('denies a JSON report run too — json is a bulk machine-readable copy', async () => { + const svc = build({ canExport: vi.fn().mockResolvedValue(false) }); + const report = await saveReport(svc, 'json'); + await expect(svc.run(report.id, CTX)).rejects.toThrow(/EXPORT_NOT_PERMITTED/); + }); + + it('ALLOWS html_table — a rendered view is reading, not exporting', async () => { + // The axis must not become a second read permission: a caller holding + // allowRead may already see these rows on screen. + const svc = build({ canExport: vi.fn().mockResolvedValue(false) }); + const report = await saveReport(svc, 'html_table'); + const result = await svc.run(report.id, CTX); + expect(result.rowCount).toBe(2); + }); + + it('allows CSV when the grant is held, and asks about the REPORT object', async () => { + const svc = build(); + const report = await saveReport(svc, 'csv'); + const result = await svc.run(report.id, CTX); + expect(result.rowCount).toBe(2); + expect(canExport).toHaveBeenCalledWith('lead', expect.objectContaining({ userId: 'u1' })); + }); + + it('refuses BEFORE reading any row', async () => { + const svc = build({ canExport: vi.fn().mockResolvedValue(false) }); + const report = await saveReport(svc, 'csv'); + const findSpy = vi.spyOn(engine, 'find'); + await expect(svc.run(report.id, CTX)).rejects.toThrow(/EXPORT_NOT_PERMITTED/); + // `sys_saved_report` lookups are fine; the OBJECT must never be queried. + expect(findSpy.mock.calls.some(([obj]) => obj === 'lead')).toBe(false); + }); + + it('a throwing canExport denies (fail closed, ADR-0049)', async () => { + const svc = build({ canExport: vi.fn().mockRejectedValue(new Error('resolution failed')) }); + const report = await saveReport(svc, 'csv'); + await expect(svc.run(report.id, CTX)).rejects.toThrow(/EXPORT_NOT_PERMITTED/); + }); + + it('no canExport wired → axis does not apply (no plugin-security deployment)', async () => { + const svc = build({ canExport: undefined }); + const report = await saveReport(svc, 'csv'); + const result = await svc.run(report.id, CTX); + expect(result.rowCount).toBe(2); + }); + }); + + describe('scheduling', () => { + it('refuses to CREATE a csv schedule the author could not run', async () => { + const svc = build({ canExport: vi.fn().mockResolvedValue(false) }); + const report = await saveReport(svc, 'html_table'); + await expect( + svc.scheduleReport({ reportId: report.id, recipients: ['a@b.c'], format: 'csv' }, CTX), + ).rejects.toThrow(/EXPORT_NOT_PERMITTED/); + }); + + it('allows an html_table schedule for the same caller', async () => { + const svc = build({ canExport: vi.fn().mockResolvedValue(false) }); + const report = await saveReport(svc, 'html_table'); + const sched = await svc.scheduleReport( + { reportId: report.id, recipients: ['a@b.c'], format: 'html_table' }, + CTX, + ); + expect(sched.id).toBeTruthy(); + }); + }); + + describe('scheduled dispatch — the original side door', () => { + it('a csv schedule created while granted STOPS delivering once the grant is revoked', async () => { + // The reason the dispatch re-checks instead of trusting the create-time + // check: permissions change after a schedule exists. + const gate = vi.fn().mockResolvedValue(true); + const svc = build({ canExport: gate }); + const report = await saveReport(svc, 'html_table'); + await svc.scheduleReport( + { reportId: report.id, recipients: ['a@b.c'], format: 'csv', intervalMinutes: 1 }, + CTX, + ); + + gate.mockResolvedValue(false); // grant revoked + // Force the schedule due (mirrors the existing dispatch suite). + engine._tables['sys_report_schedule'][0].next_run_at = new Date(now.getTime() - 1000).toISOString(); + const out = await svc.dispatchDue(); + + expect(out.failed).toBe(1); + expect(email._sent).toHaveLength(0); + const sched = engine._tables['sys_report_schedule'][0]; + expect(String(sched.last_error)).toMatch(/EXPORT_NOT_PERMITTED/); + }); + + it('an html_table schedule still delivers for a caller without the grant', async () => { + const svc = build({ canExport: vi.fn().mockResolvedValue(false) }); + const report = await saveReport(svc, 'html_table'); + await svc.scheduleReport( + { reportId: report.id, recipients: ['a@b.c'], format: 'html_table', intervalMinutes: 1 }, + CTX, + ); + // Force the schedule due (mirrors the existing dispatch suite). + engine._tables['sys_report_schedule'][0].next_run_at = new Date(now.getTime() - 1000).toISOString(); + const out = await svc.dispatchDue(); + expect(out.fired).toBe(1); + expect(email._sent).toHaveLength(1); + }); + + it('a csv schedule delivers when the owner holds the grant', async () => { + const svc = build(); + const report = await saveReport(svc, 'html_table'); + await svc.scheduleReport( + { reportId: report.id, recipients: ['a@b.c'], format: 'csv', intervalMinutes: 1 }, + CTX, + ); + // Force the schedule due (mirrors the existing dispatch suite). + engine._tables['sys_report_schedule'][0].next_run_at = new Date(now.getTime() - 1000).toISOString(); + const out = await svc.dispatchDue(); + expect(out.fired).toBe(1); + expect(email._sent[0].attachments?.[0]?.contentType).toBe('text/csv'); + }); + }); +}); diff --git a/packages/plugins/plugin-reports/src/report-service.ts b/packages/plugins/plugin-reports/src/report-service.ts index 4f1f8ccc10..587ac44a9b 100644 --- a/packages/plugins/plugin-reports/src/report-service.ts +++ b/packages/plugins/plugin-reports/src/report-service.ts @@ -197,8 +197,35 @@ export interface ReportServiceOptions { * closed instead of running with RLS bypassed (#2980). */ resolveOwnerContext?: OwnerContextResolver; + /** + * [#3544 / #3710] The user-level export axis — + * `ISecurityService.canExport(object, context)`, wired by the reports plugin + * from `getService('security')`. + * + * A report rendered as `csv`/`json` IS a bulk machine-readable copy of the + * object, so it is the same privilege `GET /data/:object/export` gates. + * Without this the axis had a side door: a caller refused at that route could + * save a report on the same object, run it as CSV — or schedule one to their + * own inbox — and receive the identical rows. + * + * Omitted (no `plugin-security`, so no permission sets exist anywhere) → the + * axis does not apply, matching the REST export route's own fail-open. + */ + canExport?: (object: string, context: unknown) => Promise; } +/** + * [#3544 / #3710] Report formats that constitute a BULK EXPORT rather than a + * rendering. + * + * `csv`/`json` are machine-readable copies — re-importable elsewhere, and + * exactly what `GET /data/:object/export` serves. `html_table` is a PRESENTED + * view: the report equivalent of reading rows on screen, which any caller + * holding `allowRead` may already do. Gating it would restrict reading rather + * than exporting and would take the axis past what it is for. + */ +const BULK_EXPORT_FORMATS: ReadonlySet = new Set(['csv', 'json']); + export class ReportService implements IReportService { private readonly engine: ReportEngine; private readonly email?: ReportEmail; @@ -206,6 +233,7 @@ export class ReportService implements IReportService { private readonly logger: NonNullable; private readonly maxRows: number; private readonly resolveOwnerContext?: OwnerContextResolver; + private readonly canExportFn?: (object: string, context: unknown) => Promise; constructor(opts: ReportServiceOptions) { this.engine = opts.engine; @@ -214,6 +242,48 @@ export class ReportService implements IReportService { this.logger = opts.logger ?? {}; this.maxRows = Math.max(1, opts.maxRows ?? 5000); this.resolveOwnerContext = opts.resolveOwnerContext; + this.canExportFn = opts.canExport; + } + + /** + * [#3544 / #3710] Gate a report rendering on the user-level export axis. + * + * Throws `EXPORT_NOT_PERMITTED` when the principal behind `context` may not + * take a bulk copy of `object`. A no-op for non-bulk formats (`html_table`), + * for a system context, and when no `canExport` is wired. + * + * Deliberately checked HERE — one place — rather than at each of the three + * callers (`runReport`, the ad-hoc run, and the scheduled dispatch): a gate + * per call site is how a fourth call site later ships ungated. `dispatchDue` + * routes through `executeReport` too, so the scheduled CSV is covered by the + * same line. (`scheduleReport` additionally pre-checks, so an author is + * refused when they create the schedule rather than silently at 3am — but + * that is UX, and THIS is the enforcement: a grant revoked after the schedule + * was created must still stop the delivery.) + * + * Fails CLOSED on a throw — it resolves permission sets to decide, and a + * resolution failure must never read as a grant (ADR-0049). + */ + private async assertExportAllowed( + object: string, + format: string, + context: SharingExecutionContext | undefined, + ): Promise { + if (!BULK_EXPORT_FORMATS.has(format)) return; + if (context?.isSystem) return; + if (!this.canExportFn) return; + let allowed: boolean; + try { + allowed = await this.canExportFn(object, context); + } catch (err) { + this.logger.warn?.('ReportService: canExport check failed — denying export', err); + allowed = false; + } + if (!allowed) { + throw new Error( + `EXPORT_NOT_PERMITTED: exporting '${object}' as ${format} is not permitted for this user`, + ); + } } // ── Access control ───────────────────────────────────────────── @@ -361,6 +431,9 @@ export class ReportService implements IReportService { context: SharingExecutionContext, stamp = true, ): Promise { + // [#3544 / #3710] The export axis, BEFORE any row is read — a refusal must + // not be reachable after the data has already been pulled. + await this.assertExportAllowed(report.object_name, report.format, context); const q = report.query ?? {}; const limit = Math.min(q.limit ?? DEFAULT_LIMIT, this.maxRows); const rows = await this.engine.find(report.object_name, { @@ -416,6 +489,12 @@ export class ReportService implements IReportService { const report = await this.getReport(input.reportId, context); if (!report) throw new Error(`REPORT_NOT_FOUND: ${input.reportId}`); + // [#3544 / #3710] Refuse a bulk-format schedule the author could not run + // themselves, at CREATE time — otherwise the refusal only surfaces on the + // first silent 3am sweep. Advisory only: `executeReport` re-checks on every + // dispatch, which is what catches a grant revoked after this point. + await this.assertExportAllowed(report.object_name, input.format ?? 'html_table', context); + const now = this.clock.now(); const interval = input.intervalMinutes ?? DEFAULT_INTERVAL_MIN; const cron = input.cronExpression?.trim() || null; diff --git a/packages/plugins/plugin-reports/src/reports-plugin.ts b/packages/plugins/plugin-reports/src/reports-plugin.ts index bcc63c769d..e58e8f75be 100644 --- a/packages/plugins/plugin-reports/src/reports-plugin.ts +++ b/packages/plugins/plugin-reports/src/reports-plugin.ts @@ -82,11 +82,28 @@ export class ReportsServicePlugin implements Plugin { ctx.logger.warn('ReportsServicePlugin: no email service — schedules will fire without delivery'); } + // [#3544 / #3710] The user-level export axis. A `csv`/`json` report is a + // bulk machine-readable copy of its object — the same privilege + // `GET /data/:object/export` gates — so the reports surface must ask the + // SAME question of the SAME authority, or it is a side door around the + // axis. Resolved lazily per call rather than captured here: `security` is + // registered by plugin-security's own `kernel:ready` hook and may not + // exist yet at construction time. Absent service (no plugin-security ⇒ no + // permission sets anywhere) → the axis does not apply, matching the REST + // export route's fail-open. + const canExport = async (object: string, context: unknown): Promise => { + let security: any; + try { security = ctx.getService('security'); } catch { return true; } + if (!security || typeof security.canExport !== 'function') return true; + return await security.canExport(object, context); + }; + this.service = new ReportService({ engine: engine as ReportEngine, email, logger: ctx.logger, maxRows: this.options.maxRows, + canExport, // Scheduled reports run under the owner's resolved RLS context, not a // system bypass (#2980). No owner-context resolver is wired yet — that // is the reports-surface consumer of ADR-0073's user-less identity diff --git a/packages/plugins/plugin-security/src/audience-anchors.test.ts b/packages/plugins/plugin-security/src/audience-anchors.test.ts index bffc1dbe1a..03cca52ae3 100644 --- a/packages/plugins/plugin-security/src/audience-anchors.test.ts +++ b/packages/plugins/plugin-security/src/audience-anchors.test.ts @@ -84,6 +84,33 @@ describe('describeHighPrivilegeBits (anchor-binding predicate)', () => { expect(describeHighPrivilegeBits({ system_permissions: ['setup.access'] })).toMatch(/system permissions/); expect(describeHighPrivilegeBits({ objects: JSON.stringify({ a: { allowRead: true } }) })).toBeNull(); }); + + // [#3544] Bulk export joined the list when the axis went opt-in. Making + // export a deliberate grant accomplishes nothing if a set carrying it can + // still be bound to `everyone` — that would hand bulk egress back to every + // authenticated user and restore the "can-list ⇒ can-export" posture the + // axis exists to end. + it('flags bulk export (#3544)', () => { + expect(describeHighPrivilegeBits({ objects: { a: { allowRead: true, allowExport: true } } })) + .toMatch(/bulk export/); + expect(describeHighPrivilegeBits({ objects: { '*': { allowRead: true, allowExport: true } } })) + .toMatch(/bulk export/); + // Row shape too — the gate reads seeded `sys_permission_set` rows. + expect(describeHighPrivilegeBits({ objects: JSON.stringify({ a: { allowExport: true } }) })) + .toMatch(/bulk export/); + }); + + it('allowExport:false / unset stays anchor-safe (member_default keeps binding)', () => { + // The platform baseline deliberately carries no export grant, so the + // everyone anchor must still accept it. + expect(describeHighPrivilegeBits({ objects: { a: { allowRead: true, allowExport: false } } })).toBeNull(); + expect(describeHighPrivilegeBits({ objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true } } })).toBeNull(); + }); + + it('a guest anchor refuses an export grant too', () => { + expect(describeAnchorForbiddenBits({ objects: { a: { allowRead: true, allowExport: true } } }, 'guest')) + .toMatch(/bulk export/); + }); }); describe('describeAnchorForbiddenBits (ADR-0090 D9 anchor tiers)', () => { diff --git a/packages/plugins/plugin-security/src/explain-engine.test.ts b/packages/plugins/plugin-security/src/explain-engine.test.ts index ead2de3b5d..c28701db15 100644 --- a/packages/plugins/plugin-security/src/explain-engine.test.ts +++ b/packages/plugins/plugin-security/src/explain-engine.test.ts @@ -469,3 +469,89 @@ describe('buildContextForUser', () => { ]); }); }); + +// ─── [#3544] the export axis is explainable ──────────────────────────────── +// +// Without this the axis is undiagnosable: a caller hits 403 +// EXPORT_NOT_PERMITTED, an admin runs explain(read), gets `allowed: true`, and +// has nowhere left to look. The answer must be a first-class operation. +describe('explainAccess — export axis (#3544)', () => { + const READER = PermissionSetSchema.parse({ + name: 'reader', + objects: { leave_request: { allowRead: true } }, + }); + const EXPORTER = PermissionSetSchema.parse({ + name: 'exporter', + objects: { leave_request: { allowRead: true, allowExport: true } }, + }); + + it('denies export for a reader without the grant, while read is allowed', async () => { + const readDecision = await explainAccess(makeDeps({ sets: [READER] }), { + object: 'leave_request', operation: 'read', context: CTX, + }); + expect(readDecision.allowed).toBe(true); + + const exportDecision = await explainAccess(makeDeps({ sets: [READER] }), { + object: 'leave_request', operation: 'export', context: CTX, + }); + expect(exportDecision.allowed).toBe(false); + const crud = exportDecision.layers.find((l) => l.layer === 'object_crud'); + expect(crud?.verdict).toBe('denies'); + expect(crud?.detail).toMatch(/export/); + }); + + it('allows export when a set carries the grant, and attributes WHICH set', async () => { + const d = await explainAccess(makeDeps({ sets: [READER, EXPORTER] }), { + object: 'leave_request', operation: 'export', context: CTX, + }); + expect(d.allowed).toBe(true); + const crud = d.layers.find((l) => l.layer === 'object_crud'); + // The attribution is the point: it names the granting set, so an admin can + // see which grant to remove (or which one is missing). + expect(crud?.contributors.map((c) => c.name)).toContain('exporter'); + expect(crud?.contributors.map((c) => c.name)).not.toContain('reader'); + }); + + it('surfaces the readFilter — an export streams the same filtered rows a read does', async () => { + const d = await explainAccess(makeDeps({ sets: [EXPORTER], rls: { owner_id: 'u1' } }), { + object: 'leave_request', operation: 'export', context: CTX, + }); + expect(d.readFilter).toEqual({ owner_id: 'u1' }); + }); + + it('computes RLS as a find, not as an "export" the compiler has no policy for', async () => { + // The bug this pins: asking the RLS compiler about an `export` operation + // matches no select policy, so the report would claim "No RLS policy + // applies" for a principal whose rows ARE filtered. + const seen: string[] = []; + const d = await explainAccess( + makeDeps({ + sets: [EXPORTER], + computeRlsFilter: async (_s: any, _o: string, op: string) => { seen.push(op); return { owner_id: 'u1' }; }, + }), + { object: 'leave_request', operation: 'export', context: CTX }, + ); + expect(seen).toContain('find'); + expect(seen).not.toContain('export'); + expect(d.layers.find((l) => l.layer === 'rls')?.verdict).toBe('narrows'); + }); + + it('resolves requiredPermissions against the READ bucket', async () => { + const seen: string[] = []; + await explainAccess( + makeDeps({ + sets: [EXPORTER], + requiredCaps: (_m: any, op: string) => { seen.push(op); return []; }, + }), + { object: 'leave_request', operation: 'export', context: CTX }, + ); + expect(seen).toEqual(['find']); + }); + + it('a super-user wildcard does not confer export', async () => { + const d = await explainAccess(makeDeps({ sets: [ADMIN] }), { + object: 'leave_request', operation: 'export', context: CTX, + }); + expect(d.allowed).toBe(false); + }); +}); diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index 9ec2511fa2..4ea2efb086 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -116,8 +116,27 @@ const EXPLAIN_TO_ENGINE_OP: Record = { transfer: 'transfer', restore: 'restore', purge: 'purge', + // [#3544] Not a copy-paste slip: `export` IS its own evaluator operation + // (`read ∧ the export grant`). It is also the only entry whose data path + // differs from its gate — see {@link dataOpOf}. + export: 'export', }; +/** + * [#3544] The operation every DATA-shaped layer must be computed as. + * + * `export` is gated as its own operation but performs an ordinary `find` to + * fetch its rows, so requiredPermissions, OWD/depth/sharing, RLS and record + * attribution all have to resolve as a READ. Asking the RLS compiler about an + * `export` operation would match no policy and report "no RLS applies" for a + * principal whose rows are in fact filtered — an explanation that contradicts + * the export it is explaining. Only `object_crud` uses the gate operation, + * because that is the only layer the axis changes. + */ +function dataOpOf(operation: ExplainOperation, engineOp: string): string { + return operation === 'export' ? 'find' : engineOp; +} + export interface ExplainEngineDeps { ql: any; /** The middleware's own set resolution (baseline + everyone semantics included). */ @@ -691,6 +710,9 @@ async function applyRecordAttribution( export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput): Promise { const { object, operation, context } = input; const engineOp = EXPLAIN_TO_ENGINE_OP[operation]; + // [#3544] The gate operation (`engineOp`) and the data operation may differ — + // today only for `export`, which is gated as itself but reads as a `find`. + const dataOp = dataOpOf(operation, engineOp); const layers: ExplainLayer[] = []; // ── 1. principal ────────────────────────────────────────────────────── @@ -779,7 +801,7 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput try { schema = deps.ql?.getSchema?.(object) ?? null; } catch { schema = null; } // ── 2. required_permissions AND-gate ────────────────────────────────── - const required = deps.requiredCaps(secMeta.requiredPermissions, engineOp); + const required = deps.requiredCaps(secMeta.requiredPermissions, dataOp); let capsDeny = false; if (required.length > 0) { const held = deps.evaluator.getSystemPermissions(sets); @@ -871,7 +893,7 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput }); // ── 6. depth ─────────────────────────────────────────────────────────── - const opClass = engineOp === 'find' ? 'read' : 'write'; + const opClass = dataOp === 'find' ? 'read' : 'write'; const agentScope = deps.evaluator.getEffectiveScope(opClass as 'read' | 'write', object, sets, { isPrivate: secMeta.isPrivate }); // [ADR-0090 D10] The delegated principal sees the NARROWER of the two depths. const scope = delegatorSets @@ -932,7 +954,7 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput // ── 9. rls — the composed machine artifact ───────────────────────────── let agentFilter: Record | null | undefined; try { - agentFilter = await deps.computeRlsFilter(sets, object, engineOp, context); + agentFilter = await deps.computeRlsFilter(sets, object, dataOp, context); } catch { agentFilter = { id: '__deny_all__' }; } @@ -941,7 +963,7 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput let delegatorFilter: Record | null | undefined; if (delegatorSets && delegatorContextForRls) { try { - delegatorFilter = await deps.computeRlsFilter(delegatorSets, object, engineOp, delegatorContextForRls); + delegatorFilter = await deps.computeRlsFilter(delegatorSets, object, dataOp, delegatorContextForRls); } catch { delegatorFilter = { id: '__deny_all__' }; } @@ -972,7 +994,7 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput let posture: AuthzPosture | undefined; if (input.recordId) { const out = await applyRecordAttribution({ - deps, object, recordId: input.recordId, engineOp, context, sets, layers, owd, capsDeny, crudAllowed, + deps, object, recordId: input.recordId, engineOp: dataOp, context, sets, layers, owd, capsDeny, crudAllowed, }); recordVerdict = out.record; posture = out.posture; @@ -991,7 +1013,10 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput ...(posture ? { posture } : {}), }, layers, - ...(operation === 'read' ? { readFilter: readFilter ?? null } : {}), + // [#3544] `export` surfaces the read filter too — it streams the same + // rows through the same filter, so omitting it would hide the very + // narrowing that explains a short export. + ...(operation === 'read' || operation === 'export' ? { readFilter: readFilter ?? null } : {}), ...(recordVerdict ? { record: recordVerdict } : {}), }; return decision; diff --git a/packages/plugins/plugin-security/src/export-permission-axis.test.ts b/packages/plugins/plugin-security/src/export-permission-axis.test.ts index 658424fce4..9d267dd7e5 100644 --- a/packages/plugins/plugin-security/src/export-permission-axis.test.ts +++ b/packages/plugins/plugin-security/src/export-permission-axis.test.ts @@ -3,17 +3,18 @@ /** * #3544 — the user-level EXPORT axis, at the evaluator. * - * `allowExport` shipped as a spec bit and a `/me/permissions` annotation, which - * hid the client's Export button but enforced nothing: `export ⊆ list`, so a - * bulk export reaches the engine middleware as an ordinary `find` gated by - * `allowRead`, and no code path ever read the bit. These pin the enforcement - * half — the evaluator's `export` branch and the tri-state fold it rests on. + * `allowExport` first shipped as a spec bit and a `/me/permissions` annotation, + * which hid the client's Export button but enforced nothing: `export ⊆ list`, + * so a bulk export reaches the engine middleware as an ordinary `find` gated by + * `allowRead`, and no code path ever read the bit. #3709 added the enforcement; + * these pin it, now under the OPT-IN posture: export is a grant, and read alone + * never confers it. * * The fold must agree, case for case, with the `/me/permissions` per-object - * merge in `hono-plugin.ts` (`if (v === true) acc[k] = true; else if - * (acc[k] === undefined) acc[k] = v`, read back as `acc.allowExport !== false`). - * A divergence there is the same declared ≠ enforced bug, just inverted: the - * client would offer a button the server refuses. + * merge in `hono-plugin.ts` (`if (v === true) acc[k] = true; …`, read back as + * `acc.allowExport === true`). A divergence there is the same + * declared ≠ enforced bug, just inverted: the client would offer a button the + * server refuses. */ import { describe, it, expect } from 'vitest'; @@ -29,72 +30,75 @@ const set = (name: string, objects: Record): PermissionSet => /** The read-granting baseline every export case builds on. */ const READER = { allowRead: true }; -describe('resolveUserExportAllowed — tri-state fold (#3544)', () => { - it('all sets unset → undefined (inherit read, backward-compatible)', () => { - expect(resolveUserExportAllowed('deal', [set('a', { deal: READER })])).toBeUndefined(); +describe('resolveUserExportAllowed — opt-in grant fold (#3544)', () => { + it('no set mentions export → NOT granted (read alone never confers it)', () => { + expect(resolveUserExportAllowed('deal', [set('a', { deal: READER })])).toBe(false); }); - it('an explicit false → false', () => { - expect( - resolveUserExportAllowed('deal', [set('a', { deal: { ...READER, allowExport: false } })]), - ).toBe(false); - }); - - it('an explicit true → true', () => { + it('an explicit true → granted', () => { expect( resolveUserExportAllowed('deal', [set('a', { deal: { ...READER, allowExport: true } })]), ).toBe(true); }); - it('true outranks false regardless of set order (most-permissive merge)', () => { - const deny = set('deny', { deal: { ...READER, allowExport: false } }); - const grant = set('grant', { deal: { ...READER, allowExport: true } }); - expect(resolveUserExportAllowed('deal', [deny, grant])).toBe(true); - expect(resolveUserExportAllowed('deal', [grant, deny])).toBe(true); - }); - - it('false outranks silence — one opt-out set denies even alongside plain readers', () => { + it('an explicit false → not granted (same outcome as silence)', () => { + // Permission sets are additive capability containers (ADR-0090) — nothing + // in them is a deny, so `false` documents intent rather than vetoing. expect( - resolveUserExportAllowed('deal', [ - set('reader', { deal: READER }), - set('no_export', { deal: { ...READER, allowExport: false } }), - ]), + resolveUserExportAllowed('deal', [set('a', { deal: { ...READER, allowExport: false } })]), ).toBe(false); }); - it('is per-object — a deny on one object leaves another untouched', () => { - const sets = [set('a', { deal: { ...READER, allowExport: false }, lead: READER })]; - expect(resolveUserExportAllowed('deal', sets)).toBe(false); - expect(resolveUserExportAllowed('lead', sets)).toBeUndefined(); + it('one granting set is enough, regardless of order (most-permissive merge)', () => { + const plain = set('plain', { deal: { ...READER, allowExport: false } }); + const grant = set('grant', { deal: { ...READER, allowExport: true } }); + expect(resolveUserExportAllowed('deal', [plain, grant])).toBe(true); + expect(resolveUserExportAllowed('deal', [grant, plain])).toBe(true); }); - it("reads the '*' wildcard when the set has no explicit entry for the object", () => { - const sets = [set('a', { '*': { ...READER, allowExport: false } })]; - expect(resolveUserExportAllowed('deal', sets)).toBe(false); + it('is per-object — a grant on one object does not leak to another', () => { + const sets = [set('a', { deal: { ...READER, allowExport: true }, lead: READER })]; + expect(resolveUserExportAllowed('deal', sets)).toBe(true); + expect(resolveUserExportAllowed('lead', sets)).toBe(false); }); - it("an explicit per-object entry overrides the '*' wildcard", () => { - const sets = [set('a', { '*': { ...READER, allowExport: false }, deal: { ...READER, allowExport: true } })]; + it("reads the '*' wildcard when the set has no explicit entry for the object", () => { + const sets = [set('a', { '*': { ...READER, allowExport: true } })]; expect(resolveUserExportAllowed('deal', sets)).toBe(true); }); + it("an explicit per-object entry overrides the '*' wildcard (lookup, not merge)", () => { + // resolveObjectPermission returns the explicit entry INSTEAD of the + // wildcard, so an entry that says nothing about export withholds it even + // though the wildcard grants it. + const sets = [set('a', { '*': { ...READER, allowExport: true }, deal: READER })]; + expect(resolveUserExportAllowed('deal', sets)).toBe(false); + }); + it("a non-super-user '*' does not reach a PRIVATE object (ADR-0066 D2)", () => { - const sets = [set('a', { '*': { ...READER, allowExport: false } })]; - expect(resolveUserExportAllowed('deal', sets, { isPrivate: true })).toBeUndefined(); - // …but a super-user wildcard does. - const superSets = [set('a', { '*': { viewAllRecords: true, allowExport: false } as any })]; - expect(resolveUserExportAllowed('deal', superSets, { isPrivate: true })).toBe(false); + const sets = [set('a', { '*': { ...READER, allowExport: true } })]; + expect(resolveUserExportAllowed('deal', sets, { isPrivate: true })).toBe(false); + // …but a super-user wildcard carrying the grant does. + const superSets = [set('a', { '*': { viewAllRecords: true, allowExport: true } as any })]; + expect(resolveUserExportAllowed('deal', superSets, { isPrivate: true })).toBe(true); }); }); -describe("checkObjectPermission('export') — export ⊆ list ∧ allowExport (#3544)", () => { +describe("checkObjectPermission('export') — export ⊆ list ∧ grant (#3544)", () => { const canExport = (sets: PermissionSet[], opts?: { isPrivate?: boolean }) => evaluator.checkObjectPermission('export', 'deal', sets, opts ?? {}); - it('unset allowExport inherits read — a plain reader may still export', () => { - // The whole point of the opt-out default: every permission set authored - // before this axis existed keeps working unchanged. - expect(canExport([set('a', { deal: READER })])).toBe(true); + it('a plain reader may NOT export — this is the opt-in flip', () => { + // Pre-flip this returned true (unset inherited read). The whole point of + // the axis is that reading a record and taking a bulk copy of the table + // are different privileges. + const sets = [set('a', { deal: READER })]; + expect(canExport(sets)).toBe(false); + expect(evaluator.checkObjectPermission('find', 'deal', sets)).toBe(true); + }); + + it('allowExport:true + read → granted', () => { + expect(canExport([set('a', { deal: { ...READER, allowExport: true } })])).toBe(true); }); it('allowExport:false denies export while READ stays granted', () => { @@ -104,33 +108,36 @@ describe("checkObjectPermission('export') — export ⊆ list ∧ allowExport (# expect(evaluator.checkObjectPermission('find', 'deal', sets)).toBe(true); }); - it('allowExport:true grants export', () => { - expect(canExport([set('a', { deal: { ...READER, allowExport: true } })])).toBe(true); - }); - it('no read grant → no export, even with allowExport:true (export ⊆ list)', () => { - // The axis NARROWS read; it is not an independent grant. A set that says - // "may export" but not "may read" exports nothing. + // The grant is one half of a conjunction, not an independent capability. + // A set that says "may export" but not "may read" exports nothing. expect(canExport([set('a', { deal: { allowRead: false, allowExport: true } })])).toBe(false); }); - it('no matching grant at all → denied', () => { - expect(canExport([set('a', { other: READER })])).toBe(false); + it('viewAllRecords satisfies the read half, but does NOT supply the grant', () => { + // Separating "may see all data" from "may take a bulk copy" is the + // segregation-of-duties case the axis exists for. + expect(canExport([set('a', { deal: { viewAllRecords: true } })])).toBe(false); + expect(canExport([set('a', { deal: { viewAllRecords: true, allowExport: true } })])).toBe(true); }); - it('viewAllRecords satisfies the read half', () => { - expect(canExport([set('a', { deal: { viewAllRecords: true } })])).toBe(true); + it('a modifyAllRecords super-user wildcard does not imply export either', () => { + const sets = [set('admin', { '*': { modifyAllRecords: true } })]; + expect(canExport(sets)).toBe(false); + expect(evaluator.checkObjectPermission('find', 'deal', sets)).toBe(true); }); - it('a super-user wildcard does NOT bypass an explicit per-object export deny', () => { - // modifyAllRecords is a bypass for the CRUD bits, not a licence to exfiltrate - // an object whose set explicitly opted out of export. + it('the grant may come from a DIFFERENT set than the read grant', () => { + // Additive containers: the union is what matters, not any single set. const sets = [ - set('admin', { '*': { modifyAllRecords: true } }), - set('no_export', { deal: { ...READER, allowExport: false } }), + set('reader', { deal: READER }), + set('exporter', { deal: { allowExport: true } }), ]; - expect(canExport(sets)).toBe(false); - expect(evaluator.checkObjectPermission('find', 'deal', sets)).toBe(true); + expect(canExport(sets)).toBe(true); + }); + + it('no matching grant at all → denied', () => { + expect(canExport([set('a', { other: { ...READER, allowExport: true } })])).toBe(false); }); it('an empty set list denies (the middleware skips its CRUD gate in that case)', () => { @@ -141,7 +148,7 @@ describe("checkObjectPermission('export') — export ⊆ list ∧ allowExport (# }); it('other operations are untouched by the axis', () => { - const sets = [set('a', { deal: { allowRead: true, allowCreate: true, allowExport: false } })]; + const sets = [set('a', { deal: { allowRead: true, allowCreate: true } })]; expect(evaluator.checkObjectPermission('find', 'deal', sets)).toBe(true); expect(evaluator.checkObjectPermission('insert', 'deal', sets)).toBe(true); expect(evaluator.checkObjectPermission('export', 'deal', sets)).toBe(false); diff --git a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts index 8eefd79090..d04b7a8daa 100644 --- a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts +++ b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts @@ -109,6 +109,12 @@ const baseDefaultPermissionSets: PermissionSet[] = [ allowDelete: true, viewAllRecords: true, modifyAllRecords: true, + // [#3544] Export is an OPT-IN grant and is deliberately NOT implied by + // the super-user bits — "may see all data" and "may take a bulk copy of + // it" are separable on purpose (SAP S_GUI 61 / segregation of duties). + // Stated explicitly so the platform administrator keeps export, and so + // a deployment wanting that separation has one obvious line to remove. + allowExport: true, }, }, systemPermissions: [ @@ -163,6 +169,10 @@ const baseDefaultPermissionSets: PermissionSet[] = [ allowDelete: true, viewAllRecords: true, modifyAllRecords: true, + // [#3544] Explicit — the super-user bits do not imply export. Bounded + // by this set's organization wall (the `tenant_isolation` RLS below), + // so it is an org-scoped export, never a cross-tenant one. + allowExport: true, }, // Identity tables — go through better-auth endpoints (invite, // accept, remove-member, transfer, …) rather than raw CRUD. @@ -308,6 +318,13 @@ const baseDefaultPermissionSets: PermissionSet[] = [ // ordinary (position-distributed) set where the domain calls for it. // The owner-scoped delete RLS below is KEPT as a narrowing defense for // members who receive a delete bit from such a set. + // + // [#3544] NO `allowExport` either, for the same reason and deliberately: + // this set is the `everyone` baseline, so granting export here would hand + // bulk egress to every authenticated user and make the opt-in axis a + // no-op. Bulk export is not a baseline right — grant it per object via an + // ordinary position-distributed set where the domain calls for it. Do not + // "fix" its absence. '*': { allowRead: true, allowCreate: true, diff --git a/packages/plugins/plugin-security/src/permission-evaluator.ts b/packages/plugins/plugin-security/src/permission-evaluator.ts index 1ea55a1e31..0773d9e563 100644 --- a/packages/plugins/plugin-security/src/permission-evaluator.ts +++ b/packages/plugins/plugin-security/src/permission-evaluator.ts @@ -104,36 +104,33 @@ function resolveObjectPermission( /** * [#3544] Fold the user-level EXPORT axis across the resolved permission sets. * - * `allowExport` is deliberately absent from {@link OPERATION_TO_PERMISSION}: - * that map's semantics are "the bit must be truthy", which would deny export to - * every permission set authored before the axis existed. The bit is a TRI-state - * instead, and this is its merge rule: + * `allowExport` is an OPT-IN grant: export is allowed only where a set says + * `true`. Unset and `false` both mean "no grant" — permission sets are additive + * capability containers (ADR-0090), so nothing in them is a deny, and `false` + * is authoring intent rather than a veto another set must respect. * - * - any set `true` → `true` (an explicit grant outranks another set's deny) - * - else any `false` → `false` (an explicit opt-out outranks silence) - * - else all unset → `undefined` (inherit read — the pre-#3544 - * "can-list ⇒ can-export" behaviour, so existing sets are unaffected) + * The bit stays out of {@link OPERATION_TO_PERMISSION} even so, because export + * is not a plain bit lookup: it is `read ∧ grant` (`export ⊆ list` in the + * spec's `API_METHOD_DERIVATION`), and that conjunction lives in + * {@link PermissionEvaluator.checkObjectPermission}'s `export` branch. * - * This is the same answer the `/me/permissions` per-object merge produces - * (`if (v === true) acc[k] = true; else if (acc[k] === undefined) acc[k] = v`, - * read back as `acc.allowExport !== false`). That equality is load-bearing, not - * incidental: the client hides its Export button on the merged map while this - * decides the server's 403, and the two disagreeing is exactly the - * `declared ≠ enforced` gap the axis exists to close. + * This must stay identical to what the `/me/permissions` per-object merge + * yields (`if (v === true) acc[k] = true; …`, read back as + * `acc.allowExport === true`). That equality is load-bearing, not incidental: + * the client hides its Export button on the merged map while this decides the + * server's 403, and the two disagreeing is exactly the `declared ≠ enforced` + * gap the axis exists to close. */ export function resolveUserExportAllowed( objectName: string, permissionSets: PermissionSet[], opts: { isPrivate?: boolean } = {}, -): boolean | undefined { - let denied = false; +): boolean { for (const ps of permissionSets) { const objPerm = resolveObjectPermission(ps, objectName, opts.isPrivate ?? false); - const bit = objPerm?.allowExport; - if (bit === true) return true; - if (bit === false) denied = true; + if (objPerm?.allowExport === true) return true; } - return denied ? false : undefined; + return false; } /** @@ -154,14 +151,15 @@ export class PermissionEvaluator { /** [ADR-0066 D2] When the object is `private`, the `'*'` wildcard only covers it if it is a super-user grant. */ opts: { isPrivate?: boolean } = {}, ): boolean { - // [#3544] User-level export axis. `export` is NOT a bit lookup: per the - // spec's derivation table (`API_METHOD_DERIVATION`) it is `list ∧ - // userExportAllowed`, so it requires READ and is then vetoed by an explicit - // `allowExport: false`. Handled here rather than in OPERATION_TO_PERMISSION - // so an UNSET bit keeps inheriting read — otherwise every permission set - // written before the axis existed would silently lose export. + // [#3544] User-level export axis — `read ∧ grant`, not a bit lookup. Per + // the spec's derivation table (`API_METHOD_DERIVATION`) export is + // `list ∧ userExportAllowed`, so BOTH halves must hold: the caller must be + // able to read the object at all, and some set must opt them into bulk + // egress. Kept out of OPERATION_TO_PERMISSION because that map checks one + // bit and would miss the read half — granting export to a caller who cannot + // even list the object. if (operation === 'export') { - if (resolveUserExportAllowed(objectName, permissionSets, opts) === false) return false; + if (!resolveUserExportAllowed(objectName, permissionSets, opts)) return false; return this.checkObjectPermission('find', objectName, permissionSets, opts); } diff --git a/packages/spec/src/security/explain.zod.ts b/packages/spec/src/security/explain.zod.ts index 51f839edea..58804a329e 100644 --- a/packages/spec/src/security/explain.zod.ts +++ b/packages/spec/src/security/explain.zod.ts @@ -32,9 +32,22 @@ import { lazySchema } from '../shared/lazy-schema'; * (PLATFORM_ADMIN > TENANT_ADMIN > MEMBER > EXTERNAL) on the resolved principal. */ -/** Operations the explain API accepts (the CRUD + lifecycle classes the evaluator maps). */ +/** + * Operations the explain API accepts (the CRUD + lifecycle classes the + * evaluator maps, plus the bulk-egress axis). + * + * [#3544] `export` is here because it is the one operation whose answer cannot + * be inferred from `read`: since the axis went opt-in, a principal can be fully + * able to read an object and still be refused a bulk copy of it. Without an + * explainable `export`, an admin facing a 403 `EXPORT_NOT_PERMITTED` would have + * no way to ask WHY — `explain(read)` would come back `allowed`, which is true + * and useless. It explains as `read ∧ the export grant`: the object_crud layer + * reports the conjunction, while every data-shaped layer (requiredPermissions, + * OWD/depth/sharing, RLS, record attribution) is computed as the `find` the + * export actually performs. + */ export const ExplainOperationSchema = z.enum([ - 'read', 'create', 'update', 'delete', 'transfer', 'restore', 'purge', + 'read', 'create', 'update', 'delete', 'transfer', 'restore', 'purge', 'export', ]); export type ExplainOperation = z.infer; diff --git a/packages/spec/src/security/high-privilege.ts b/packages/spec/src/security/high-privilege.ts index fa56fc885e..46fda84b99 100644 --- a/packages/spec/src/security/high-privilege.ts +++ b/packages/spec/src/security/high-privilege.ts @@ -27,8 +27,9 @@ function coerceRecord(v: unknown): Record | undefined { * anchor (`everyone` / `guest`)? Returns a human-readable description of the * first offending bit, or `null` when the set is anchor-safe. * - * Offending bits — exactly the ADR-0090 D5 list: any `systemPermissions`, - * View/Modify All Data (VAMA), or delete/purge/transfer on any object. + * Offending bits — the ADR-0090 D5 list: any `systemPermissions`, + * View/Modify All Data (VAMA), or delete/purge/transfer on any object, plus + * bulk `export` (#3544). * A plain `'*'` wildcard grant is NOT high-privilege by itself (D5 permits * a read/create/edit-own baseline to cover all objects — the platform's own * `member_default` is exactly that shape); the wildcard ban is the GUEST @@ -36,6 +37,16 @@ function coerceRecord(v: unknown): Record | undefined { * {@link describeAnchorForbiddenBits}. Fixes #2753: the former blanket * wildcard rejection made the default baseline unbindable to `everyone`, * forcing it through the separate fallback channel D5 explicitly rejected. + * + * [#3544] `allowExport` joins the list because the export axis is an OPT-IN + * grant: making export deliberate at the permission-set layer accomplishes + * nothing if a set carrying it can still be bound to `everyone` (or `guest`), + * which would hand bulk table egress back to every authenticated user — or to + * anonymous visitors — and quietly restore the "can-list ⇒ can-export" posture + * the axis exists to end. It sits in the same class as delete/purge/transfer: + * not a baseline right, and never something an anchor should confer wholesale. + * (`member_default` deliberately carries no `allowExport`, so the platform's + * own baseline stays anchor-bindable.) */ export function describeHighPrivilegeBits(def: any): string | null { if (!def || typeof def !== 'object') return null; @@ -50,6 +61,7 @@ export function describeHighPrivilegeBits(def: any): string | null { const p: any = rawPerm ?? {}; if (p.viewAllRecords || p.modifyAllRecords) return `View/Modify All Data on '${objName}'`; if (p.allowDelete || p.allowPurge || p.allowTransfer) return `delete/purge/transfer on '${objName}'`; + if (p.allowExport) return `bulk export on '${objName}'`; } } return null; diff --git a/packages/spec/src/security/permission.zod.ts b/packages/spec/src/security/permission.zod.ts index dcb31760c3..a9dd4d9402 100644 --- a/packages/spec/src/security/permission.zod.ts +++ b/packages/spec/src/security/permission.zod.ts @@ -37,20 +37,36 @@ export const ObjectPermissionSchema = lazySchema(() => z.object({ * * `export` derives from `list` AND this bit * (`@objectstack/spec/data` `resolveEffectiveApiMethods` → - * `ResolveApiOptions.userExportAllowed`). Deliberately OPTIONAL with no - * default so it is a backward-compatible **opt-out**, not an opt-in: - * - UNSET (undefined) → inherits read — the current "anyone who can list can - * one-click export" behavior — so adding this key changes nothing for - * existing permission sets. - * - `false` → deny export while KEEPING read (Salesforce "Export Reports", - * Dynamics "Export to Excel", NetSuite "Export Lists", SAP S_GUI 61 parity). - * - `true` → explicitly granted. + * `ResolveApiOptions.userExportAllowed`). It is an **OPT-IN GRANT**, like + * every other `allow*` bit: + * - `true` → export granted (still bounded by read: `export ⊆ list`). + * - UNSET or `false` → NO export. Reading a record on screen and pulling + * the whole table down as a CSV are different privileges (Salesforce + * "Export Reports", Dynamics "Export to Excel", NetSuite "Export Lists", + * SAP S_GUI 61 all separate them), so read alone never implies export. + * + * Merged most-permissively across a caller's permission sets, exactly like + * the CRUD bits: ANY set granting `true` grants export. `false` is therefore + * documentation of intent rather than a veto — permission sets are additive + * capability containers (ADR-0090), and no bit in them is a deny. + * + * NOT implied by the `viewAllRecords` / `modifyAllRecords` super-user bits. + * That is deliberate: separating "may see all data" from "may take a bulk + * copy of it" is the segregation-of-duties case the axis exists for. The + * built-in admin sets carry the grant explicitly instead. + * + * Deliberately OPTIONAL (no Zod default) so the resolved value stays + * three-valued at the AUTHORING layer — a set that says nothing about export + * is distinguishable from one that says `false`, which is what lets Setup + * render "inherited / explicitly off" instead of collapsing both to a blank + * checkbox. Enforcement collapses them: only `true` grants. * * The server resolves this into the per-object effective operation set - * (`apiOperations` on `/me/permissions`); the frontend renders that set and - * never reads this bit directly. + * (`apiOperations` on `/me/permissions`) and enforces it at the export door + * (`GET /data/:object/export` → 403 `EXPORT_NOT_PERMITTED`); the frontend + * renders that set and never reads this bit directly. */ - allowExport: z.boolean().optional().describe('[#3544] User-level export axis over read. Unset = inherit read (can-list ⇒ can-export, backward-compatible); false = deny export while keeping read; true = granted.'), + allowExport: z.boolean().optional().describe('[#3544] User-level export axis over read (opt-in grant). true = export granted (still bounded by read); unset/false = no export. Merged most-permissively like the CRUD bits; NOT implied by viewAllRecords/modifyAllRecords.'), /** * Lifecycle Operations. From 191fbd2cfd68d33170e9f2acf76a069377b2b42e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 16:17:11 +0000 Subject: [PATCH 2/2] docs(permissions): add allowExport to the bit matrix and carve the super-user exception The permissions matrix is the canonical hand-written table of object permission bits, and it had no `allowExport` row. Worse, its super-user bypass callout is exactly the sentence this axis carves an exception into: neither VAMA bit confers export, so an admin can read every record and still be refused a bulk copy. Left as-is it would read as "admins automatically have export", which is now false. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PDbwCy9Jrc1chhR2vnAUos --- content/docs/permissions/permissions-matrix.mdx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/content/docs/permissions/permissions-matrix.mdx b/content/docs/permissions/permissions-matrix.mdx index e2b9fa01ac..c541035350 100644 --- a/content/docs/permissions/permissions-matrix.mdx +++ b/content/docs/permissions/permissions-matrix.mdx @@ -23,6 +23,7 @@ ObjectStack's `ObjectPermission` schema defines these boolean flags for object a | **Create** | `allowCreate` | Create new records | Insert records | | **Edit** | `allowEdit` | Modify records owned by the user or shared with them | Edit own records | | **Delete** | `allowDelete` | Remove records owned by the user or shared with them | Delete own records | +| **Export** | `allowExport` | Take a bulk machine-readable copy of the records the user can read | Export / bulk egress ([details](/docs/permissions/permission-sets#allowexport--the-export-axis)) | | **Transfer** | `allowTransfer` | Change record ownership | Reassign owner | | **Restore** | `allowRestore` | Undelete from trash | Recover soft-deleted records | | **Purge** | `allowPurge` | Permanently (hard) delete | GDPR / compliance erase | @@ -32,6 +33,14 @@ ObjectStack's `ObjectPermission` schema defines these boolean flags for object a **Super-user bypass:** When `modifyAllRecords` is set it satisfies write checks (`allowEdit`/`allowDelete`, and the lifecycle class `allowTransfer`/`allowRestore`/`allowPurge`) on any record; `viewAllRecords` (or `modifyAllRecords`) satisfies `allowRead` on any record — both bypass ownership and sharing. See `packages/plugins/plugin-security/src/permission-evaluator.ts`. +**The one exception is `allowExport`.** Neither super-user bit confers it: a +principal with View/Modify All Data may read every record and still be refused a +bulk copy of them. Export is an opt-in grant that must be stated explicitly — +separating "may see all data" from "may take a bulk copy of it" is the +segregation-of-duties case the axis exists for. The built-in +`admin_full_access` / `organization_admin` sets therefore carry +`allowExport: true` alongside their super-user bits. + The bypass is **posture-gated for row-level security** (ADR-0066 D2 ①): RLS policies are short-circuited only on objects whose posture permits it — `access: { default: 'private' }`, `tenancy.enabled: false`