Uh oh!
There was an error while loading. Please reload this page.
fix(grid,list,app-shell): gate row Edit/Delete on the caller's permission, not just apiOperations - #4108
Merged
Conversation
…sion (#4096) The list row kebab's built-in Edit/Delete intersected only object-scoped layers — the ADR-0103 bucket, `userActions`, and the server's effective API operation set (`/me/permissions` `apiOperations`). `apiOperations` is the object's API EXPOSURE surface and is principal-independent (measured 30/30 identical between an account with `allowEdit` and one without), so the gate failed open for every account with no write grant: Delete sat one click away from users the server answers 403 to, and Edit opened a prefilled dialog that could only fail on save. The toolbar's New and the record header's Edit/Delete on the same screen were already gated correctly. Ands the principal's own verdict — `can(obj, 'update' | 'delete')`, i.e. `allowEdit` / `allowDelete`, the toolbar's source — on top of the existing layers, on four surfaces: - the grid row kebab (`resolveRowCrudAffordances` gained `permissionUpdate` / `permissionDelete`, filled at the `ObjectGrid` call site); - the grid bulk-delete bar, which rides the same object-level delete verdict; - the non-grid bulk bar `ListView` renders itself; - the related-list Create/Edit/Delete in `RelatedRecordActionsBridge`. An intersection tightening, not a swap: `apiOperations` and every other layer stay, and no layer can re-open what another closed. Fail-open is preserved where it is the contract — `usePermissions()` with no `PermissionProvider` answers `can: () => true`, so standalone embeds are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FQbuY8A4jabkVwkP3GU9oe
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
baozhoutao
marked this pull request as ready for review
August 10, 2026 10:23
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#4096
What was wrong
The list row kebab's built-in Edit / Delete intersected only object-scoped layers — the ADR-0103 lifecycle bucket,
userActions.edit/delete, and the server's effective API operation set (/me/permissionsapiOperations, #3720).apiOperationsis the object's API exposure surface ("which verbs does this object publish"), and the report measured it byte-identical across two accounts with oppositeallowEdit— 30 shared objects, 30/30 identical. A gate built only from object-scoped layers therefore fails open for every unprivileged caller.Result: one screen, three different answers to "may this user write this object". The toolbar's New (
affordances.create && can(obj, 'create')) and the record header's Edit/Delete (per-record write probe) were correct; the row kebab was not.The fix — an intersection tightening, not a swap
apiOperationsstays as the exposure layer. A principal-scoped verdict is ANDed on top:can(obj, 'update' | 'delete'), whichMePermissionsProvidermaps to/me/permissionsallowEdit/allowDelete— the toolbar's own source. No layer can re-open what another closed; a permission grant cannot resurrect an entry the bucket,userActionsorapiOperationsclosed, and none of them survives a permission denial.resolveRowCrudAffordances()gained two optional inputs,permissionUpdate?/permissionDelete?, with the sameundefined ⇒ no narrowingsemantics as the existingeffectiveApiOperations(backward compatible).ObjectGridfills them from theusePermissions()it already holds — no new data channel.Per-face verdicts
packages/plugin-grid/src/ObjectGrid.tsxrow kebab built-in Edit/DeletepermissionUpdate/permissionDeleteANDed inresolveRowCrudAffordances; filled at the call site (ObjectGrid.tsx).packages/app-shell/src/views/RelatedRecordActionsBridge.tsxrelated-list rowsresolveEffectiveCrudAffordances(childDef, getObjectApiOperations(objectName))and nothing else, soonCreate/onEdit/onDeletewere all offered to a principal with no write grant on the child. Now ANDscan(child, 'create'|'update'|'delete').onViewdeliberately untouched — viewing is not a write.packages/plugin-list/src/ListView.tsxnon-grid bulkdeletepermittedBulkActionsgated the built-indeleteonresolveEffectiveCrudAffordances(objectDef, effectiveApiOps).deletealone. All three layers describe the OBJECT, so a kanban/gallery board's most destructive control stayed visible for an account with noallowDelete. Now ANDscan(obj, 'delete'). Custom action ids still pass through untouched (own gates via the action runner).objectCanDelete/onBulkDelete)explicitBulkActions,bulkActionDefsfilter, implicit['delete']) all rideobjectCanDelete, which is returned by the same resolver, so the principal layer reaches it by construction. Pinned explicitly rather than assumed. On #3492: itsrequiredPermissionswork is a different, orthogonal gate — the ADR-0066 D4 capability gate for declared actions (mayInvoke/useCapabilityGate), which never covered the built-indelete. That is why this one was still open.⛔ Untouched, as declared out of scope:
packages/core/src/evaluator/**andpackages/plugin-grid/src/components/RowActionMenu.tsx(another seat's in-flightevalRowPredicatesurface, #3796 / #3792). The fix did not need them — the verdict arrives atRowActionMenuthrough the existingcanEdit/canDeleteprops.fail-open / fail-closed — the named regression risk
All three cases are pinned, in the shared consistency files (no new test file):
rowCrudEffectiveOps.test.tsx,rowCrudAffordances.test.ts,RelatedRecordActionsBridge.effectiveOps.test.tsx,ListView.permissions.test.tsxPermissionProviderThe no-provider case in
rowCrudEffectiveOps.test.tsxruns the realusePermissions(the mock keeps the actual module reachable viaimportOriginaland returns it for that branch), so it exercises the genuinecan: () => truefallback rather than an imitation of it.ListView.permissions.test.tsxrenders with no provider mounted at all.MePermissionsProviderfail-closed semantics (objPerm ? objPerm[k] !== false : data.authenticated !== true, #2926 ④) are inherited deliberately — "consistent with the toolbar" is the standard this card was given, and the toolbar'scan(obj, 'create')has run under exactly these semantics on the same screen all along. Checked for the console's own grids specifically:packages/app-shell/src/views/metadata-admin/ResourceListPage.tsxrenders its own "ObjectGrid-like table" — it does not go throughObjectGrid, so this change cannot touch it. (grep -rln 'ObjectGrid' packages/app-shell/src/views/metadata-admin/returns only preview/config/i18n files.)sys_*object lists reached throughObjectView→ObjectGridare the only console surface in range, and they already runaffordances.create && can(objectDef.name, 'create')for the New button. If/me/permissions.objectsomittedsys_*for a console admin, New would already be missing today — so the map does carry them (or a'*'entry, whichcheckhonors). Per-key absence is permissive anyway (allowEdit !== false).MePermissionsProviderrendersloadingFallbackand does not mount children until the set resolves (MePermissionsProvider.tsx:303), so thepermissions-loading→allowed: falsewindow is never observed by a rendered grid./me/permissions, so the evidence above is code + test level. Flagging rather than implying otherwise.PM mechanism assumptions — verified against
origin/main@c29ceffAll three confirmed, none falsified:
ObjectGrid.tsx:1647passedeffectiveApiOperations: effectiveApiOpsand ANDed no principal-scoped verdict;rowCrudAffordances.ts's docblock folded exactly three layers (bucket /userActions/apiOperations).ObjectView.tsx:1798/1809/1863→affordances.create && can(objectDef.name, 'create'); detail headerRecordDetailView.tsx:1899-1902→objectAffordances.edit && recordWriteAllowed/.delete && recordDeleteAllowed.MePermissionsProvider.checkmapsupdate → allowEdit,delete → allowDelete(MePermissionsProvider.tsx:230-236), andObjectGrid.tsx:435already heldusePermissions()— no new data channel was needed.The suggested route (explicit params on the pure function + call-site fill) was taken.
useRecordEditablewas not used on list rows — it is a per-record HTTP probe, N rows ⇒ 2N requests; the object-level verdict is what the toolbar uses and what the issue asked for. Sinking the layer into@object-ui/core'sresolveEffectiveCrudAffordanceswas not done: minimum-change was the default instruction, and the four existing call sites there already ANDcan()themselves, so sinking would be idempotent but far broader. Noted as an open question.Verification
Run in a dedicated worktree off
origin/main@c29ceff. Dependency closure built first (pnpm --filter '@object-ui/plugin-grid...' --filter '@object-ui/plugin-list...' --filter '@object-ui/app-shell...' build) so the cross-package type surface is not read from staledist/*.d.ts.(Package-level
pnpm --filter <pkg> testis fine forplugin-grid/plugin-listbut the repo's vitest guard refuses it forapp-shell— everything above was re-run from the repo root, which is what CI does.)Negative check — the new cases are not vacuous. Each face's gate was temporarily reverted and the suite re-run:
The "WITH permission" and "no provider" cases stayed green throughout — they assert unchanged behavior, which is the point.
Cross-package sweep direction: dependents (downstream consumers),
pnpm --filter '...@object-ui/plugin-grid'— the prefix form. It resolves to 9 packages:app-shell,console,example-byo-backend-console,example-console-starter,plugin-designer,plugin-grid,plugin-report,plugin-view,site.resolveRowCrudAffordanceshas no consumer outsideplugin-griditself (grep overpackages/apps/examples), and the signature change is additive-optional, so nothing downstream needed adapting;pnpm type-checkcovers all 9 and is green.No new
t()keys ⇒check:i18n-keysnot applicable. Changeset:.changeset/row-crud-permission-gate-4096.md(patch × 3).Out of scope, per the issue author's own closing line
Extending the CEL predicate scope so
os.usercarries permission/role information is not in this PR.Generated by Claude Code