fix: complete the v17 permission, approval and media migrations

.changeset/upgrade-objectstack-v17-rc.md

@@ -6,3 +6,11 @@ Upgrade HotCRM to ObjectStack 17.0.0-rc.1. The app now declares protocol 17
 compatibility, preserves existing required-field database constraints explicitly,
 uses the live metadata contracts for skills, flows, and APIs, and enables the
 date buckets that make CRM analytics aggregate by month, day, and quarter.
+
+Also completes the 17.0 permission migration that the version bump alone leaves
+half-done: `allowExport` became an opt-in bit whose absence DENIES, so every
+CSV/XLSX list export and every report export is authored explicitly on the
+profiles that need it. Approval nodes drop their hand-rolled org-owner backstop
+for the native `onEmptyApprovers` policy, media fields declare the `accept` and
+`maxSize` constraints 17.0 enforces server-side, and the platform-upgrade
+checklist now covers the `os migrate` data gates.

CHANGELOG.md

@@ -9,6 +9,10 @@ Everything merged after the 2.2.2 tag. Not yet versioned or published.
 ### Changed
 
 - **ObjectStack platform → 17.0.0-rc.1** across all `@objectstack/*` packages (from 16.1.0). The manifest protocol handshake and marketplace `specVersion` now declare the 17.0.0-rc.1 compatibility line; API methods, skills and flow notifications use only live metadata keys; pre-17 required fields preserve their database `NOT NULL` constraints explicitly; and the analytics semantic layer now declares the month/day/quarter buckets v17 executes correctly.
+- **`allowExport` is authored on every profile that needs it (17.0 opt-in export axis, upstream #3544).** Before 17.0 an unset `allowExport` inherited read, so "can list ⇒ can export". 17.0 inverted the default: `resolveUserExportAllowed` demands an explicit `allowExport: true` and neither `viewAllRecords` nor `modifyAllRecords` substitutes, so an unset bit now DENIES — at both bulk-egress doors, the list views' built-in `exportOptions` and `ReportService.assertExportAllowed`. No profile carried the bit, which would have 403'd every CSV/XLSX and report export for every user, `system_admin` included, while `os validate`, `os build` and the whole test suite stayed green. The five objects with a real export surface (`crm_account`, `crm_case`, `crm_contact`, `crm_lead`, `crm_opportunity`) now carry the grant on each profile that already reads them; `guest_portal` deliberately carries none (ADR-0090 D9 forbids binding a high-privilege set to the `guest` anchor). Pinned by `test/authorization-coverage.test.ts`, which fails both on an export surface nobody can reach and on a grant with no surface behind it.
+- Approval nodes in `opportunity_approval` use the native `onEmptyApprovers: 'admin_rescue'` policy instead of the hand-rolled `org_membership_level: 'owner'` approver. That entry existed only to avoid the empty-position dead-end, and with `behavior: 'first_response'` it overshot — it made an org owner a routine approver on every deal over $100K, not a rescue when the bench is empty.
+- Media fields (`crm_product.image`, `crm_product.datasheet`, `crm_account.logo`, `crm_contact.avatar`) declare `accept` and `maxSize`. 17.0 enforces both server-side against the stored `sys_file` (ADR-0104 D3 wave 2); before, the upload widget read them but `FieldSchema` dropped them at parse, so the constraint never reached a direct API caller.
+- Docs: [`docs/MAINTENANCE.md`](docs/MAINTENANCE.md) §3.2 covers the `os migrate` data gates (`files-to-references`, `value-shapes`) and the `OS_ALLOW_LAX_*` escape hatches. `pnpm verify` and `demo:reset` cannot catch these — they start from an empty database, so a green local run says nothing about an in-place upgrade.
 - **ObjectStack platform → 16.1.0 stable** across all `@objectstack/*` packages (from the 16.0.0-rc.1 line pinned in 2.2.0). [#465](https://github.com/objectstack-ai/hotcrm/pull/465)
 - CI: bump `actions/checkout` 4 → 7 ([#422](https://github.com/objectstack-ai/hotcrm/pull/422)) and `actions/setup-node` 4 → 6 ([#421](https://github.com/objectstack-ai/hotcrm/pull/421)).
 - Docs: README hero states HotCRM's size in tokens ([#483](https://github.com/objectstack-ai/hotcrm/pull/483)); drifted README counts fixed ([#466](https://github.com/objectstack-ai/hotcrm/pull/466)).

docs/MAINTENANCE.md

@@ -68,6 +68,47 @@ can silently invalidate existing metadata or **seed data** (see §4). Treat ever
    native binary was built for a different Node ABI — `pnpm rebuild better-sqlite3`
    and restart. This is an environment issue, not an app change.
 7. Note the new platform version in `CHANGELOG.md`.
+8. **Check the release notes for `os migrate` steps that run against DATA, not
+   metadata** — see §3.2. `pnpm verify` cannot catch these: they gate runtime
+   behaviour on a deployment flag, so a fresh install is clean and an in-place
+   upgrade is not.
+
+### 3.2 Data migrations and enforcement gates (`os migrate`)
+
+Steps 1–7 cover the app's own metadata. A major can additionally ship
+migrations that rewrite or re-validate **stored rows**, gated behind a
+deployment flag so the new enforcement turns on only once the data is known
+clean. `demo:reset` hides these entirely — it starts from an empty database, so
+a green local run says nothing about an existing deployment.
+
+Run the metadata replay first, then each data gate as a dry run before
+`--apply`:
+
+```bash
+os migrate meta --from <previous major>   # replays renames and key conversions
+os migrate files-to-references            # dry run: media fields → sys_file records
+os migrate files-to-references --apply    # convert, verify, record the flag
+os migrate value-shapes                   # dry run: scan reference & JSON validity
+os migrate value-shapes --apply           # record the gate if the scan is clean
+```
+
+For **17.0** specifically: `files-to-references` backfills the four media
+fields this app declares (`crm_product.image`, `crm_product.datasheet`,
+`crm_account.logo`, `crm_contact.avatar`) into `sys_file` records, and
+`value-shapes` scans reference and JSON columns. Neither is needed for a fresh
+install — no seed data populates a media field — but an in-place upgrade needs
+both before strict validation is safe to enable.
+
+If a scan reports rows it cannot convert, the escape hatches downgrade the new
+enforcement to warnings while you fix the data. They are temporary, not a
+destination:
+
+| Variable | Effect |
+| --- | --- |
+| `OS_ALLOW_LAX_MEDIA_VALUES=1` | File-value verification warns instead of failing |
+| `OS_ALLOW_LAX_VALUE_SHAPES=1` | Reference/JSON validation warns instead of failing |
+| `OS_ALLOW_LAX_ACTION_PARAMS=1` | Action-param shape enforcement warns instead of failing |
+| `OS_DATA_VALUE_SHAPE_STRICT_ENABLED=1` | Opt into strict value shapes immediately, without the gate |
 
 ### 3.1 Destructive schema drift — database-only columns after an upgrade
 

src/flows/opportunity-approval.flow.ts

@@ -75,15 +75,18 @@ export const OpportunityApprovalFlow: Flow = {
       type: 'approval',
       label: 'Sales Manager Review',
       config: {
-        // Org-owner backstop: approvers snapshot at request creation, and a
-        // position with no holders leaves the request undecidable while
-        // lockRecord holds the record hostage (no admin override exists).
-        // behavior:'first_response' means whoever responds first wins, so the
-        // backstop changes nothing when the sales-manager bench is staffed.
-        approvers: [
-          { type: 'position', value: 'sales_manager' },
-          { type: 'org_membership_level', value: 'owner' },
-        ],
+        // The empty-position dead-end (approvers snapshot at request creation,
+        // so a position with no holders left the request undecidable while
+        // lockRecord held the record hostage) is a NODE POLICY on @objectstack
+        // 17, not something the approver list has to work around. The
+        // `{ type: 'org_membership_level', value: 'owner' }` entry that used to
+        // sit here was that workaround, and it overshot: with
+        // `behavior: 'first_response'` it made an org owner a routine approver
+        // for every deal, not just a rescue when the bench is empty.
+        approvers: [{ type: 'position', value: 'sales_manager' }],
+        // Explicit even though `admin_rescue` is the schema default — this is
+        // the mechanism the node depends on, so it is authored, not inherited.
+        onEmptyApprovers: 'admin_rescue',
         behavior: 'first_response',
         lockRecord: true,
         approvalStatusField: 'approval_status',
@@ -104,11 +107,9 @@ export const OpportunityApprovalFlow: Flow = {
       type: 'approval',
       label: 'Sales Director Sign-off',
       config: {
-        // Same org-owner backstop as manager_review (empty-position dead-end).
-        approvers: [
-          { type: 'position', value: 'sales_director' },
-          { type: 'org_membership_level', value: 'owner' },
-        ],
+        // Same node policy as manager_review — see the note there.
+        approvers: [{ type: 'position', value: 'sales_director' }],
+        onEmptyApprovers: 'admin_rescue',
         behavior: 'first_response',
         lockRecord: true,
         approvalStatusField: 'approval_status',

src/objects/account.object.ts

@@ -153,10 +153,13 @@ export const Account = ObjectSchema.create({
       group: 'branding',
     }),
 
-    // Company logo (uploaded image)
+    // Company logo (uploaded image). `accept` / `maxSize` are server-enforced
+    // from @objectstack 17 — see the note on `crm_product.image`.
     logo: Field.image({
       label: 'Company Logo',
       group: 'branding',
+      accept: ['image/png', 'image/jpeg', 'image/svg+xml', 'image/webp'],
+      maxSize: 2 * 1024 * 1024,
     }),
 
     // Date field

src/objects/contact.object.ts

@@ -58,10 +58,13 @@ export const Contact = ObjectSchema.create({
       group: 'identity',
     }),
 
-    // Avatar field
+    // Avatar field. `accept` / `maxSize` are server-enforced from
+    // @objectstack 17 — see the note on `crm_product.image`.
     avatar: Field.avatar({
       label: 'Profile Picture',
       group: 'identity',
+      accept: ['image/png', 'image/jpeg', 'image/webp'],
+      maxSize: 2 * 1024 * 1024,
     }),
 
     // Relationship: Link to Account (Master-Detail)

src/objects/product.object.ts

@@ -146,14 +146,22 @@ export const Product = ObjectSchema.create({
     }),
 
     // Images and Assets
+    // `accept` / `maxSize` are declarable AND server-enforced from
+    // @objectstack 17 (ADR-0104 D3 wave 2). Before that the upload widget read
+    // them but `FieldSchema` dropped them at parse, so the constraint existed
+    // only in the browser and any direct API caller walked past it.
     image: Field.image({
       label: 'Product Image',
       group: 'metadata',
+      accept: ['image/png', 'image/jpeg', 'image/webp'],
+      maxSize: 5 * 1024 * 1024,
     }),
 
     datasheet: Field.file({
       label: 'Datasheet',
       group: 'metadata',
+      accept: ['application/pdf'],
+      maxSize: 20 * 1024 * 1024,
     }),
 
     // Tax & billing

src/profiles/guest-portal.profile.ts

@@ -25,6 +25,10 @@ export const GuestPortalProfile = {
   description:
     'Anonymous visitors submitting public Web-to-Lead / Web-to-Case forms. ' +
     'INSERT-only on lead and case; no read/edit/delete on any object.',
+  // No `allowExport` anywhere below, deliberately: this set is bound to the
+  // `guest` anchor, and ADR-0090 D9 classes `allowExport` as a high-privilege
+  // bit no anchor may confer — the set would stop binding at all, on top of
+  // handing anonymous visitors bulk table egress. See `src/profiles/index.ts`.
   objects: {
     crm_lead: {
       allowCreate: true,

src/profiles/index.ts

@@ -2,6 +2,42 @@
 
 /**
  * Profile Definitions Barrel
+ *
+ * ─── `allowExport` — the opt-in bulk-egress axis (@objectstack 17, #3544) ───
+ *
+ * This is the canonical note; the profiles point here.
+ *
+ * Before 17.0 the export axis was advisory: an unset `allowExport` inherited
+ * read, so "can list ⇒ can export" and the bit only ever hid a button. 17.0
+ * makes it a real gate and inverts the default — `resolveUserExportAllowed`
+ * (plugin-security) returns true only for an explicit `allowExport: true`, and
+ * neither `viewAllRecords` nor `modifyAllRecords` substitutes for it. An unset
+ * bit now DENIES. Both bulk-egress doors ask before they read: the list views'
+ * built-in `exportOptions`, and `ReportService.assertExportAllowed`, which
+ * fails a report export closed with `EXPORT_NOT_PERMITTED`.
+ *
+ * So the grants below are authored, not inherited. The rule, pinned by
+ * `test/authorization-coverage.test.ts`:
+ *
+ *   a profile grants `allowExport` on an object IFF it already holds
+ *   `allowRead` there AND the app ships an export surface for that object —
+ *   a list view declaring `exportOptions`, or a report whose dataset is
+ *   built on it.
+ *
+ * That union is exactly `crm_account`, `crm_case`, `crm_contact`, `crm_lead`,
+ * `crm_opportunity` today. Adding `exportOptions` to a view (or a report over
+ * a new dataset) means adding the matching grant here in the same change — the
+ * guard fails otherwise, which is the point: a surface nobody can use is the
+ * failure this axis exists to make loud instead of silent.
+ *
+ * Export is READ-DERIVED (`export ⊆ list`), so the grant opens the door but
+ * does not widen the rows: record scope, RLS and sharing still apply on top. A
+ * `sales_rep` exporting `crm_opportunity` gets their own book, not the org's.
+ *
+ * `guest_portal` deliberately carries none. Per ADR-0090 D9 a set holding
+ * `allowExport` is high-privilege and cannot be bound to the `everyone` or
+ * `guest` anchors at all — granting it there would both hand anonymous
+ * visitors bulk table egress and make the set unbindable.
  */
 export { GuestPortalProfile } from './guest-portal.profile';
 export { MarketingUserProfile } from './marketing-user.profile';

src/profiles/marketing-user.profile.ts

@@ -4,15 +4,19 @@ export const MarketingUserProfile = {
   name: 'marketing_user',
   label: 'Marketing User',
   objects: {
-    crm_lead:        { allowCreate: true,  allowRead: true,  allowEdit: true,  allowDelete: false, viewAllRecords: true,  modifyAllRecords: false },
-    crm_account:     { allowCreate: false, allowRead: true,  allowEdit: false, allowDelete: false, viewAllRecords: true,  modifyAllRecords: false },
-    crm_contact:     { allowCreate: true,  allowRead: true,  allowEdit: true,  allowDelete: false, viewAllRecords: true,  modifyAllRecords: false },
+    // `allowExport` where an export surface exists — canonical note in
+    // `src/profiles/index.ts`. Lead and contact list exports are marketing's
+    // core targeting workflow; account/opportunity back the campaign-ROI
+    // reports. No `crm_case` grant here, so no export bit for it either.
+    crm_lead:        { allowCreate: true,  allowRead: true,  allowEdit: true,  allowDelete: false, viewAllRecords: true,  modifyAllRecords: false, allowExport: true },
+    crm_account:     { allowCreate: false, allowRead: true,  allowEdit: false, allowDelete: false, viewAllRecords: true,  modifyAllRecords: false, allowExport: true },
+    crm_contact:     { allowCreate: true,  allowRead: true,  allowEdit: true,  allowDelete: false, viewAllRecords: true,  modifyAllRecords: false, allowExport: true },
     crm_campaign:    { allowCreate: true,  allowRead: true,  allowEdit: true,  allowDelete: false, viewAllRecords: true,  modifyAllRecords: false },
     // Org-wide read: marketing attributes campaign → pipeline ROI, which needs
     // every opportunity, not just self-owned. viewAllRecords was false here
     // while every other object on this set is true — an oversight that hid all
     // pipeline from marketing (and tripped security-private-no-readscope).
-    crm_opportunity: { allowCreate: false, allowRead: true,  allowEdit: false, allowDelete: false, viewAllRecords: true,  modifyAllRecords: false },
+    crm_opportunity: { allowCreate: false, allowRead: true,  allowEdit: false, allowDelete: false, viewAllRecords: true,  modifyAllRecords: false, allowExport: true },
     // Campaign membership is THIS profile's core write surface: the
     // "Add to Campaign" action (`src/actions/lead.actions.ts`) inserts
     // `crm_campaign_member` rows, and before #488 no permission set granted the

src/profiles/sales-manager.profile.ts

@@ -10,15 +10,18 @@ export const SalesManagerProfile = {
   name: 'sales_manager',
   label: 'Sales Manager',
   objects: {
-    crm_lead:        { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true },
-    crm_account:     { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true },
-    crm_contact:     { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true },
-    crm_opportunity: { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true },
+    // `allowExport` where an export surface exists — canonical note in
+    // `src/profiles/index.ts`. A manager owns the number, so the pipeline and
+    // book exports that feed offline forecasting are part of the job.
+    crm_lead:        { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true, allowExport: true },
+    crm_account:     { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true, allowExport: true },
+    crm_contact:     { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true, allowExport: true },
+    crm_opportunity: { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true, allowExport: true },
     crm_quote:       { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true },
     crm_contract:    { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: false, viewAllRecords: true,  modifyAllRecords: false },
     crm_product:     { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: false, viewAllRecords: true,  modifyAllRecords: false },
     crm_campaign:    { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: false, viewAllRecords: true,  modifyAllRecords: false },
-    crm_case:        { allowCreate: false, allowRead: true, allowEdit: false, allowDelete: false, viewAllRecords: true,  modifyAllRecords: false },
+    crm_case:        { allowCreate: false, allowRead: true, allowEdit: false, allowDelete: false, viewAllRecords: true,  modifyAllRecords: false, allowExport: true },
     crm_task:        { allowCreate: true,  allowRead: true, allowEdit: true,  allowDelete: true,  viewAllRecords: true,  modifyAllRecords: true },
     // The forecast IS the manager's job: they read every rep's snapshot and
     // adjust the committed number, so this is org-wide read AND write on a