Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-grid): row kebab ANDs the record-level write verdict, batched per page (#4296) - #4689
Merged
Merged
Conversation
The list row kebab ANDed only the OBJECT-level grant, so a user with a broad object grant saw Edit/Delete on every row they could read - including rows the server answers 403 for - while the record detail header, which folds the record-grained verdict, correctly hid both on the same record. Layer (e) of rowCrudAffordances' intersection chain now folds the explain engine's record-grained verdict, batched once per (object, operation) for the rows on screen via the recordIds form landed in objectstack#8326. Unanswered rows keep the object-level verdict - degrade to today, never over-hide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQ
…e, add the changeset Three plugin-grid suites render a grid with no host apiFetch, so the new batched probe fell back to the global fetch and escaped to the real network (0 such lines before this change, 180 after). Answer it from a recorded double instead - the shape objectui#3339 / PR #4105 settled - never a global error sink. The double answers visible:true for every row, so those files measure what they measured before. Consumer packages hit the same escape and are filed as objectui#4688. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQ
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This was referenced Aug 15, 2026
yinlianghui
marked this pull request as ready for review
August 15, 2026 09:32
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 15, 2026
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
2 participants
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#4296
The list row kebab ANDed only the OBJECT-level grant, so one screen gave two opposite answers for one record and one user: the kebab offered Edit and Delete, the record detail header hid both, and the server agreed with the detail header (
403 "You do not have access to this record").writeScope, the sharing model and RLS narrow the object grant per row, sorowCrudAffordances' layer (d) fails open for every record the principal does not own — the same argument that layer's own doc header makes about layer (c), one level down.What this does
Adds layer (e) to the intersection chain: the explain engine's RECORD-grained verdict, the same authority
plugin-detail'suseRecordEditablehas asked since objectstack#3821, so the two surfaces cannot disagree.POST /api/v1/security/explainper (object, operation) for the rows on screen, using therecordIdsform that landed in objectstack#8326 / PR #8452. A 50-row page issues 2 calls, not 100 — the cost that kept this card blocked. Pages above the server's 200-id cap paginate under it (the spec's own documented consumer contract) rather than sending a request the server refuses with400 VALIDATION_FAILED.planRowActionMenuconjoins the object verdict with the Row CRUD actions (Edit/Delete) should support per-record visible/disabled CEL predicates #2614visibleWhenpredicates in one expression, so the items and the "⋮" guard read ONE decision and a fully denied row grows no empty trigger (console: 平台内置「审批请求」(sys_approval_request) 行「更多操作」菜单展开后 0 个菜单项——渲染成 128×10 的空白方块 #3562's invariant).useRecordEditabletakes.objectCanDelete(the bulk-delete bar) is untouched, so 列表行内建【编辑】【删除】没接权限门:只与 apiOperations 求交,而它与用户无关 ⇒ 无写权账号恒可见 #4096's object-level behaviour is unchanged on every surface.Two things the reviewer should look at
1. Which per-row channel the verdict rides. The ruling named
RowActionMenu'seditPredicates/deletePredicates. The verdict is fed through thecanEdit/canDeleteprops of the same per-row call instead, because those are the OTHER half of the very same conjunction —planRowActionMenucomputesBoolean(canEdit && onEdit) && isBuiltinRowActionVisible(editPredicates, …), andRowActionMenuis instantiated per row by the_actionscell. Same decision, same guard, same #3562 consistency. The literal spelling was not available:RowCrudPredicates['visibleWhen']is spec-typedExpression | ExpressionInput, so a boolean verdict is not assignable to it, and synthesising a CEL string would have fabricated an authoring artifact to carry a server answer. The authored predicates are passed through untouched.2. A local constant for the batch cap. The pinned
@objectstack/spec@17.0.0-rc.6predates the batch form and exports neitherEXPLAIN_BATCH_MAX_RECORD_IDSnor the batch request/response types (verified against the installed package). The hook declares a narrow local constant and reads the wire payload asunknownbefore narrowing; the #4636 pin bump supersedes that declaration. Nothing else was loosened.Tests
Verified at
e3c3dc331(the gate union was re-run at this commit, after the final one).rowRecordCrudVerdict.test.tsx(new, 15 tests) — the card's exact repro shape (object grant true +writeScope: 'own'+ a non-owned row loses both entries); an owner's own row keeps both; the fully denied row grows no trigger while the permitted row on the same screen keeps its own; update and delete gated independently; the truth-table cell as a literal agreement test, driving BOTH surfaces against ONE fake explain service out of ONE verdict table, with the detail side consuming the realuseRecordEditableunchanged and unmocked; five degradation cases (non-OK response, network failure, a pre-batch backend answering norecords, a row with no id, and the pre-answer window with the response held open); the batch-shape pins (50 rows to 2 calls carrying the ids once, never the singularrecordId; 250 rows to 2 calls per operation each under the cap; no call at all when the object verdict already hides both); and both intersection directions.rowCrudAffordances.test.ts— 5 unit tests on the new pure layer, including the unknown-verdict degradation and the never-unions direction.pnpm --filter @object-ui/plugin-grid test— 72 files / 645 tests pass.pnpm --filter @object-ui/plugin-grid run type-check— clean (closure built first).check:control-bytes,check:phantom-deps,check:spec-symbols,check-changeset-presence.mjs,check-changeset-no-major.mjs— all pass.Reverse verification (direction predicted first, then observed): reverting only
ObjectGrid.tsxtoorigin/mainturned 12 of the 15 new tests red and left 3 green. The 3 survivors are the ones asserting that NO explain call happens and the object-level answer stands (no row id; object verdict already closed; closed API exposure surface) — correct, since those hold pre-fix too. One prediction was wrong and is worth recording: the degradation cases went red as well, not green. They wait for the batched request to be issued before asserting that its failure changes nothing, so with the fold absent they fail at that wait — a degradation pin over a mechanism that does not exist is vacuous, so this is the honest coupling, but it is not what the template presumed.Test-hygiene fallout, handled in-package and filed for the rest
The probe rides
apiFetch ?? fetch, matchinguseRecordEditableexactly — the global fallback is what keeps a standalone same-origin embed's kebab agreeing with its detail header. Under happy-dom that fallback is a real request to the default origin, so suites rendering a grid without a host fetch escape to the network: the pattern objectui#3339 recorded and PR #4105 settled (answer it from a recorded double, never a global error sink).Measured: 0 such lines before this change, 180 after, in three plugin-grid suites. All three now install a double (
src/__tests__/explainDouble.ts), which answersvisible: truefor every row — so those files' assertions measure exactly what they measured before — and records every URL it is handed, so an escape elsewhere stays visible instead of vanishing into a swallowed rejection. plugin-grid is back to 0.The same escape lands in consumer packages, which resolve
@object-ui/plugin-gridto source through the root vitest alias: plugin-view (11 files / 111 tests pass, 96 lines) and app-shell + plugin-designer (409 files / 3843 tests pass, 180 lines). Those files are outside this card's declared surface, so they are filed rather than edited here: objectui#4688, unassigned, with the measurements and the ready-made double. Dropping the global-fetch fallback would remove the escape at its root but would reintroduce this card's contradiction for every same-origin host that does not wireapiFetch, so it is not the remedy.Generated by Claude Code