Skip to content

fix: complete the v17 permission, approval and media migrations - #580

Merged
os-zhuang merged 1 commit into
codex/upgrade-objectstack-v17-rc1from
claude/upgrade-development-review-pt1aqt
Aug 1, 2026
Merged

fix: complete the v17 permission, approval and media migrations#580
os-zhuang merged 1 commit into
codex/upgrade-objectstack-v17-rc1from
claude/upgrade-development-review-pt1aqt

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Description

Follow-up to #579, stacked on its branch. The version bump there is sound, but three 17.0 contracts are left half-migrated. The first is a silent outage: it passes os validate, os build and the entire test suite, and only fails at runtime, for every user.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update

Related Issues

Related to #579

Changes Made

allowExport — the opt-in export axis (upstream #3544). This is the blocker.

Before 17.0 an unset allowExport inherited read, so "can list ⇒ can export" and the bit only ever hid a button. 17.0 inverted the default:

// plugin-security — resolveUserExportAllowed
if (objPerm?.allowExport === true) return true;   // only an explicit true
return false;                                      // unset = DENY

Neither viewAllRecords nor modifyAllRecords substitutes. No profile on the branch carried the bit, so every CSV/XLSX list export and every report export would have 403'd for every user, system_admin included — at both bulk-egress doors, the list views' built-in exportOptions and ReportService.assertExportAllowed (EXPORT_NOT_PERMITTED). Nothing in CI catches it, because the grant is well-formed; it just isn't there.

The grant is now authored on the five objects with a real export surface — crm_account, crm_case, crm_contact, crm_lead, crm_opportunity — for each profile that already reads them. guest_portal deliberately carries none: ADR-0090 D9 classes allowExport as high-privilege, so a set holding it stops binding to the guest anchor altogether. Canonical rationale lives in src/profiles/index.ts.

Export is read-derived, so the grant opens the door without widening the rows: a sales_rep exporting crm_opportunity still gets their own book.

Approvalsopportunity_approval's two nodes drop the hand-rolled org_membership_level: 'owner' approver for the native onEmptyApprovers: 'admin_rescue'. 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 rather than a rescue when the bench is empty. This is a deliberate behaviour change — flagging it explicitly.

Media fieldscrm_product.image, crm_product.datasheet, crm_account.logo, crm_contact.avatar declare accept / maxSize, which 17.0 enforces server-side against the stored sys_file (ADR-0104 D3 wave 2). Previously the upload widget read them while FieldSchema dropped them at parse, so the constraint never reached a caller talking to the API directly.

rowActions guard — dropped the edit/delete/view builtin whitelist from test/metadata-references.test.ts. 17.0's action-name-undefined rule refuses a rowActions string naming no defined action, so the whitelist was letting metadata past CI that the platform then rejects. (This confirms #579's removal of rowActions was correct and required — an earlier read of mine, based on the Zod schema alone, had it backwards; os validate settled it.)

Docsdocs/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.

Testing

  • Unit tests pass — 636 passed | 1 skipped (33 files), up from 631 with 5 new guards
  • Linting passes — 2 warnings, both pre-existing (field groups on crm_campaign_member / crm_task)
  • Build succeeds
  • New tests added
  • Manual testing completed — not done; see below

Full pnpm verify (validate → typecheck → lint → hygiene → build → test) is green against @objectstack/* 17.0.0-rc.1 installed from the branch lockfile.

New guards in test/authorization-coverage.test.ts derive the export surfaces from real metadata (views declaring exportOptions, plus every report's dataset object, walking blocks[]) and fail in both directions — an export surface no profile can reach, and a grant with no surface behind it. Teeth verified by a negative run: adding allowExport to crm_quote fails with undeclared bulk egress.

Checklist

  • I have added a changeset — extended the existing .changeset/upgrade-objectstack-v17-rc.md
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes

Additional Notes

Not verified in a browser. The export fix is the one change whose payoff is a runtime permission decision, and the guards prove the metadata is authored, not that the Export button works end-to-end. Worth one console pass on a list view and a report export before this leaves draft.

The allowExport grant set is a judgement call, not a mechanical migration — the rule chosen ("a profile grants it where it already reads AND the app ships an export surface") is documented in src/profiles/index.ts and enforced by the guards, but the per-persona spread is worth a review, particularly service_agent on crm_account / crm_contact.


Generated by Claude Code

The version bump leaves three 17.0 contracts half-migrated. The first is a
silent outage.

allowExport became an opt-in bit. Before 17.0 an unset value inherited read,
so "can list => can export" and the bit only ever hid a button; 17.0 inverted
the default, and `resolveUserExportAllowed` now demands an explicit
`allowExport: true` with neither `viewAllRecords` nor `modifyAllRecords`
standing in for it. No profile carried the bit, so every CSV/XLSX list export
and every report export would have 403'd for every user, system_admin
included — while `os validate`, `os build` and the whole suite stayed green,
because the grant is well-formed, it just isn't there.

Grant it on the five objects with a real export surface (a list view declaring
`exportOptions`, or a report whose dataset is built on them), for each profile
that already reads them. `guest_portal` gets none: ADR-0090 D9 classes
`allowExport` as high-privilege, so a set carrying it stops binding to the
`guest` anchor entirely. New guards in authorization-coverage fail in both
directions — an export surface nobody can reach, and a grant with no surface
behind it — so the next `exportOptions` cannot ship without its grant.

Approval nodes drop the hand-rolled `org_membership_level: 'owner'` approver
for the native `onEmptyApprovers: 'admin_rescue'`. 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
rather than a rescue when the bench is empty.

Media fields declare `accept` / `maxSize`, which 17.0 enforces server-side
against the stored sys_file (ADR-0104 D3 wave 2). Previously the upload widget
read them while FieldSchema dropped them at parse, so the constraint never
reached a caller talking to the API directly.

Also drop the `edit`/`delete`/`view` builtin whitelist from the rowActions
guard. 17.0's `action-name-undefined` rule refuses a rowActions string naming
no defined action, so the whitelist was letting metadata past CI that the
platform then rejects.

MAINTENANCE.md §3.2 documents the `os migrate` data gates and the
`OS_ALLOW_LAX_*` escape hatches: `pnpm verify` and `demo:reset` start from an
empty database, so neither can catch what an in-place upgrade needs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YHzpTPCxNfskro88FRuBwH
@vercel

vercel Bot commented Aug 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hotcrm Ignored Ignored Aug 1, 2026 12:38am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation ci/cd CI plumbing and the verification pipeline metadata Declarative metadata — schema, security posture, UI surfaces backend Server-side behaviour — hooks, flows, actions labels Aug 1, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 1, 2026 00:57
@os-zhuang
os-zhuang merged commit cc433cb into codex/upgrade-objectstack-v17-rc1 Aug 1, 2026
4 checks passed
os-zhuang added a commit that referenced this pull request Aug 1, 2026
* feat: upgrade ObjectStack to 17.0.0-rc.1

* chore: sync StackBlitz npm lockfile

* fix: complete the v17 permission, approval and media migrations (#580)

The version bump leaves three 17.0 contracts half-migrated. The first is a
silent outage.

allowExport became an opt-in bit. Before 17.0 an unset value inherited read,
so "can list => can export" and the bit only ever hid a button; 17.0 inverted
the default, and `resolveUserExportAllowed` now demands an explicit
`allowExport: true` with neither `viewAllRecords` nor `modifyAllRecords`
standing in for it. No profile carried the bit, so every CSV/XLSX list export
and every report export would have 403'd for every user, system_admin
included — while `os validate`, `os build` and the whole suite stayed green,
because the grant is well-formed, it just isn't there.

Grant it on the five objects with a real export surface (a list view declaring
`exportOptions`, or a report whose dataset is built on them), for each profile
that already reads them. `guest_portal` gets none: ADR-0090 D9 classes
`allowExport` as high-privilege, so a set carrying it stops binding to the
`guest` anchor entirely. New guards in authorization-coverage fail in both
directions — an export surface nobody can reach, and a grant with no surface
behind it — so the next `exportOptions` cannot ship without its grant.

Approval nodes drop the hand-rolled `org_membership_level: 'owner'` approver
for the native `onEmptyApprovers: 'admin_rescue'`. 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
rather than a rescue when the bench is empty.

Media fields declare `accept` / `maxSize`, which 17.0 enforces server-side
against the stored sys_file (ADR-0104 D3 wave 2). Previously the upload widget
read them while FieldSchema dropped them at parse, so the constraint never
reached a caller talking to the API directly.

Also drop the `edit`/`delete`/`view` builtin whitelist from the rowActions
guard. 17.0's `action-name-undefined` rule refuses a rowActions string naming
no defined action, so the whitelist was letting metadata past CI that the
platform then rejects.

MAINTENANCE.md §3.2 documents the `os migrate` data gates and the
`OS_ALLOW_LAX_*` escape hatches: `pnpm verify` and `demo:reset` start from an
empty database, so neither can catch what an in-place upgrade needs.


Claude-Session: https://claude.ai/code/session_01YHzpTPCxNfskro88FRuBwH

Co-authored-by: Claude <noreply@anthropic.com>

---------

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Server-side behaviour — hooks, flows, actions ci/cd CI plumbing and the verification pipeline documentation Improvements or additions to documentation metadata Declarative metadata — schema, security posture, UI surfaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants