Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-grid): re-apply field-level security on ObjectGrid's inline-data column path - #6800
Merged
Conversation
…lumn path `ObjectGrid.generateColumns()` re-applied FLS at exactly one place, the object-schema path. The inline-data path — taken when a host hands rows down as `data` AND the author declared a `fields` projection — had no equivalent check, so whether an object-bound grid re-checked FLS depended on who fetched the rows. Same object, same authored projection, two answers. The inline-data path now filters each column through `perms.checkField(objectName, fieldName, 'read')` when `perms.isLoaded && schema.objectName`, the same gate and the same deferral condition the object-schema path already used. Only keys the OBJECT DECLARES are judged; host-joined and derived keys pass through untouched. That limit is load-bearing rather than an optimisation: a policy that enumerates readable fields answers "no" for a key it never heard of, so judging derived keys would silently drop them. Declaration is read with `hasOwnProperty` so an inherited name is not mistaken for a declared field. Deliberately unchanged (refused by name in the maintainer ruling of 2026-08-29): the two paths' label handling, the schema path's drop of undeclared names, and everything on the `ListView` host side. Part of #6723 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
…ader The header predicted one red. The measured ablation (this file restored to d06059f, guard removed) produced two: PIN 2 and WIRING, the same assertion asked once of the stub and once of the real `PermissionProvider`. The six that did not move are named as such, since "did not move" is the half that says the guard is narrow. Also records why no rebuild stands between the edit and the run here: `vitest.config.mts` aliases every `@object-ui/*` specifier to that package's `src`, and the pin imports `../ObjectGrid` relatively. Part of #6723 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Aug 29, 2026
os-sales
marked this pull request as ready for review
August 29, 2026 22:15
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 30, 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#6723
Narrow defence-in-depth fix, implementing the maintainer ruling of 2026-08-29
(quoted verbatim on the card).
ObjectGrid.generateColumns()re-appliedfield-level security at exactly one place — the object-schema path. The
inline-data path, taken when a host hands rows down as
dataand the authordeclared a
fieldsprojection, had no equivalent check, so whether anobject-bound grid re-checked FLS depended on who fetched the rows.
What changed
One predicate, in
packages/plugin-grid/src/ObjectGrid.tsx:Same gate and same deferral condition (
perms.isLoaded && schema.objectName)the object-schema path has always used.
Only keys the object DECLARES are judged. That limit is the ruling's
load-bearing clause, not an optimisation: host-joined and derived keys pass
through untouched, because keeping them is this path's whole reason to exist
(the object-schema path drops them outright with
if (!field) return;). A fieldpolicy that enumerates readable fields answers "no" for a key it has never heard
of, so judging derived keys would silently drop them — the exact failure the
card's own analysis warned about. Declaration is read with
hasOwnProperty, soan inherited name (
constructor,toString) is not mistaken for a declaredfield.
Refused by name in the ruling, and not done here: merging the two paths'
label handling (
resolveFieldLabeli18n vs the inline path's localhumanisation), the undeclared-key drop, and anything on the
ListViewhostside.
Premise verification
The card's premise holds on my base,
d06059f24(same ref the dispatch named).git grep -n checkField -- packages/plugin-grid/src/ObjectGrid.tsxreturnsexactly two lines — one comment and one call — and the call is inside the
object-schema path's
fieldsToShow.forEach.generateColumnsopens at line1827; the call was at 2400. The inline-data path did not filter.
Acceptance pins, and the ablation in both directions
packages/plugin-grid/src/__tests__/inlineDataFls-6723.test.tsx(8 cases) andpackages/plugin-list/src/__tests__/ListView.inlineFlsNoop-6723.test.tsx(3cases). Ablation method: this branch's only edit to
ObjectGrid.tsxis theguard, so the mutation is
git checkout d06059f24 -- ObjectGrid.tsx, confirmedon disk by blob hash before the run (
0988ddca0= base blob, not2b55805e4=head blob) and restored under a
trap ... EXIT INT TERM; the restore was provedby an empty
git diff HEADplus a disk hash equal to the HEAD blob, not by anexit code. No rebuild stands between the edit and the run:
vitest.config.mtsaliases every
@object-ui/*specifier to that package'ssrc, and the pinimports
../ObjectGridrelatively.objectName: nothing filteredPermissionProviderdrops the denied columnListView, all three casesAblated run:
Tests 2 failed | 9 passed (11). Both reds are the same assertionasked twice — once of the stub, once of the real provider — and both read
AssertionError: expected [ 'Opportunity Name', 'Salary' ] to deeply equal [ 'Opportunity Name' ]. Restored:Test Files 2 passed (2) / Tests 11 passed (11).So the guard is not decorative: pin 2 moves, and it moves through the real
PermissionProviderand not only against a double.Why the stub's
checkFieldis an allowlistPermissionProvideranswerstruefor a field no policy mentions, so under ita derived key survives whether or not the guard judges it — pin 3 would be green
in both worlds for the wrong reason and the limit would be untestable. The stub
models the shape a server that ENUMERATES readable fields produces: deny
anything not listed. That is the only policy shape under which the limit is
load-bearing, so it is the one the limit is pinned against.
Pin 4 is measured, not assumed
ListView.inlineFlsNoop-6723.test.tsxmounts the REALListViewover the REALObjectGrid(registered inheavyDomTests, the same routeListView.crossPageSelectAll.test.tsxand the two #6598 files take) with a realPermissionProviderdenying a declared field, and asserts the rendered headersin both handoff shapes. It is green identically with and without the guard —
that identity is the no-op.
The mechanism behind the no-op is pinned too, not just the outcome.
ListViewreaches the grid two ways and neither arrives at the inline-data branch carrying
a declared-but-denied key: with authored columns it forwards the already
FLS-filtered
columnsand thenormalizeColumnsbranch returns first (pinned byexact header equality); unauthored it forwards
fields: undefined, columns: undefined(#6598), sorowKeysWouldOutrankSchemaPolicysends the grid to theobject-schema path (pinned by the ABSENCE of
Id, which ishidden: true—the object-schema policy drops it and the row-key derivation would keep it). A
CONTROL with no policy mounted keeps the two denials from being vacuous.
Verification
All of the below on
7159718f0, the branch head, re-run after the final commit.pnpm exec vitest run packages/plugin-grid/ packages/plugin-list/→Test Files 151 passed (151)/Tests 1613 passed (1613)pnpm exec vitest run scripts/(this diff editsvitest.config.mts, whichseveral
scripts/__tests__helpers read) →Test Files 88 passed (88)/Tests 2447 passed (2447)pnpm --filter @object-ui/plugin-grid type-checkand the same forplugin-list→ both clean. Each package'stype-checkistsc --noEmit && tsc -p tsconfig.test.json, and--listFilesconfirms bothnew test files are in that program (1 hit each), so this is a measurement of
the tests and not merely of
src.check-control-bytes: OK (scanned 5647 tracked text file(s))·check-vi-mock-specifiers: OK (491 carry a mock)·check-changeset-presence: 3 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)·check-changeset-fixed: OK·check-changeset-no-major: No changeset declares a major bump·check-phantom-dependencies: Every in-scope import is declared·check-side-effects-array: OKpnpm --filter @object-ui/plugin-grid lint,pnpm --filter @object-ui/plugin-list lint,pnpm lint:root→ 0 errorsin all three (683 / 442 / 28 pre-existing warnings; eslint exits 0). Declared
narrowing: this covers every file this PR touches AND every file in both
packages it touches — 134 and 65 files judged respectively, counted from
eslint --format json.eslint.config.jssets noproject/projectService, so type-aware linting is off and nothing in this diff canmove the verdict on a file it does not contain.
Out of scope, filed separately
After this lands, the authored
columnspath (normalizeColumns(schemaColumns),both arms) is the one remaining
generateColumns()path with no FLS re-check —the same defect class, reachable without the host supplying rows at all. The
ruling scoped this card to the inline-data path, so it is not touched here.
Filed as #6799 with the evidence and the two reasons it is a decision rather
than a mechanical follow-up.
Generated by Claude Code