fix(plugin-grid,plugin-list): FLS-gate the $expand projection at both build sites - #7229
Conversation
…th sites objectui#6898 closed field-level security on `$select`; `$expand` was ungated at both projection build sites, so a lookup / master_detail / user / tree field the principal cannot read was still handed to the server for expansion — and an expansion returns the RESOLVED related record where `$select` returns only a bare foreign key. Reproduced first as failing tests at both sites. On the `ListView` path the same gap also reopened `$select`: that builder gates its columns and then adds the expand roots back unconditionally, so the denied field re-entered the projection through the union rather than through its own filter. The gate judges the OUTPUT of `buildExpandFields` rather than its input, which is what keeps the required ordering structural: the helper already returns a subset of the object's declared reference-bearing fields, so no undeclared key is ever judged. Gating the input would instead WIDEN the expansion, because the helper reads an empty column list as "no column restriction" and falls back to every relation — and it cannot reach the no-columns case at all. `buildExpandFields` itself is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
✅ 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
|
Review — ACCEPT. The ZONE 1.1 deviation is CONFIRMED: option A, as shippedReviewer of record: My ruling was wrong; yours is right, and it is pinned rather than arguedI ruled the gate onto the input column list. You put it on
The output gate also satisfies ZONE 1.2's mandatory ordering structurally rather than by convention: the helper already returns a subset of the object's declared reference-bearing fields, so every name judged is declared and the " Both failure modes of the input-side form are pinned as tests, so B and C are falsifiable claims rather than my preference against yours. That is what made this a five-minute confirmation instead of an argument. Second ruling of mine overturned today, both by measurement. ⭐ The finding that matters most is the one beyond the card
So a denied lookup column walked straight back into ⇒ #6898 was believed closed and had a live bypass. That is not a second defect, it is the measured reach of this one, and gating at the single output point closes both halves. Worth stating plainly in the landing note, because anyone auditing FLS would have read #6898 as settled. The ZONE 2 falsification was a crash, not a nuance
I flagged "is The severity statement is exactly rightI asked for a precise, narrow claim rather than an alarming one, and got a mechanism read rather than assumed:
⇒ Against ObjectStack's own server this is defence-in-depth, not a live disclosure; it is load-bearing for a non-enforcing backend. Same p2 grading as #6898, for a stated reason. And the client-request side is reachable in a real configuration, because the fetch effect reads the authored Also correctly disposed of: the "denied leaf under a permitted root" question is not expressible here — MethodRed first at Ablation legs A and B: removing either site's gate leaves the other site's tests fully green. The two-site property demonstrated, not asserted — the same property that made #7179's one-site fix a trap. And the whole ablation was run twice, before and after merging The no-rebuild leg is declared load-bearing rather than skipped: the root vitest config aliases Armed. PM follow-upYour out-of-scope finding could not be filed because the dedupe search hit the rate limit and you correctly did not file blind. I am filing it — the same ungated Generated by Claude Code |
…hell): FLS-gate `$expand` at the five remaining build sites objectui#7215 / PR #7229 FLS-gated the `$expand` projection at the two sites in its scope. `buildExpandFields` (and `computeLookupExpand`, the dashboard's own whitelist) are reached from more places; this closes the five the card names. Three of them — calendar, gantt and the record page — pass NO column list, so the helper falls back to every declared relation on the object, denied ones included: the maximal ask, by default rather than by configuration. DetailView was INPUT-gated, which is the route #7229 measured as unsound: an emptied column list reads as "no restriction" and WIDENS the request. The gate is on the helper's OUTPUT at every site, copied from #7229 rather than re-derived, so the "checkField answers false for an undeclared key" trap stays structurally unreachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
2 participants
Fixes #7215
What an unauthorised principal could observe before, and cannot after
Before. A view could name a
lookup/master_detail/user/treecolumn that thecurrent principal is denied
readon, and that field name went out in$expand. The clientwas therefore asking the server to resolve the relation and return the related record,
not merely to return a bare foreign key. objectui#6898 had already stopped the client asking
for the denied field in
$select; the same field could still be asked for in$expand, whichis the larger of the two requests.
On the
ListViewpath it was also asking for the field in$selectagain. Not a seconddefect — the measured reach of this one. That builder gates its columns
(
rawCols.filter(c => perms.checkField(...))) and then adds the expand roots backunconditionally (
for (const e of expandFields) required.add(e)), on the recorded ground thatthose roots are "known-valid because
buildExpandFields()derived them from the objectschema". Valid, yes; readable, never asked. So the denied column walked back into
$selectthrough the union. Measured, pre-fix, in the harness:
$selectcame out as['id', 'subject', 'account', 'secret_account']withsecret_accountdenied.After. Neither projection names a denied relation.
$expandcarries only relations theprincipal can read; the
$selectre-entry on theListViewpath is closed with it.Reachability — stated narrowly
needed: the fetch effect reads the authored
schema.columnsdirectly, not the FLS-filteredrender columns, so an ordinary authored view with a denied lookup column is enough. The
sharpest case needs no column list at all — with no
columns,buildExpandFieldsexpands every declared relation of the object, denied ones included. That is the default
shape, not an edge case.
a mechanism I read rather than assumed:
plugin-security'sFieldMasker.maskRecorddoesdelete result[field]for every unreadable key, and objectql's expand path writes theresolved record back under that same key (
record[fieldName] = recordMap.get(...)inengine.ts), so one statement removes the expanded object and the bare id alike. Theexpansion sub-read is itself gated —
__expandReadtakes the referenced object's fullCRUD + RLS + FLS treatment (objectstack#7626). Same grading objectui#6898 was given, for the
same reason, and it becomes load-bearing for any backend that does not strip.
change is that the invariant stops resting on every future backend having enforced it.
The fix, and one deviation from the card's suggested route
Both sites, because objectui#7179 measured that they are independent paths and a one-site fix
ships with a green suite and a still-open surface:
packages/plugin-grid/src/ObjectGrid.tsx— the grid's own fetchpackages/plugin-list/src/ListView.tsx— theexpandFieldsmemoThe card asked for the gate on the column list feeding
buildExpandFields. It is on thathelper's OUTPUT instead, and the reason is measured, not stylistic — both failure modes are
pinned as tests:
buildExpandFieldsreads an empty column list as "no column restriction"(
columns.length > 0guards its intersection) and falls back to expanding every declaredrelation. Gating the input would therefore widen a view whose only relational column is
denied, from that one expansion to all of them.
undefined— there is no input to gate, and it is the case thatexpands the most.
Gating the output also satisfies the ordering the card called mandatory — intersect against the
object's declared fields first, ask
checkFieldonly about survivors — structurallyrather than by convention, because
buildExpandFieldsreturns a subset of the declaredreference-bearing fields. Every name the gate judges is declared by construction, so the
"
checkFieldanswers false for an undeclared key" trap cannot be reached and a derived /host-joined column is never judged.
buildExpandFieldsitself is unchanged, as ruled.One structural note in
ListView:const perms = usePermissions()moved above theexpandFieldsmemo.useMemoruns its callback during the render that declares it, so a memoreading
permsfrom below would hit the temporal dead zone and throwCannot access 'perms' before initialization— a crash, not a stale value.Relation roots, not leaves
buildExpandFieldsreturns top-level field names of the object being fetched, never dottedpaths, so the gate asks
checkField(object, root, 'read')— exactly the right question. Adenied leaf on the related object is a different object's FLS and is not answerable at
this call site; it is not in scope here and is not silently claimed to be handled.
Tests — reproduced first, as the card required
New, both mirroring
projectionFls-6898.test.tsx:packages/plugin-grid/src/__tests__/expandFls-7215.test.tsx— 8 pinspackages/plugin-list/src/__tests__/ListView.expandFls-7215.test.tsx— 10 pinsPre-fix, on
5015fcf52: grid 5 failed | 3 passed (8), list 7 failed | 3 passed (10).The leak is real at both sites.
Each file carries the live controls, not just the reds: a permitted lookup still expands; an
undeclared derived column is untouched and does not take the expansion down with it; an
unanswered permission policy filters nothing; the input-gating widening trap is pinned;
master_detailis pinned besidelookup; and the grouping / kanban-binding routes into theexpand list take the same gate.
Ablation — the two-site property demonstrated, not asserted
Predicted before running; every count landed as predicted. Each leg mutates one site, proves
the mutation reached disk by replacement-marker count and blob hash, runs both files,
then restores via
git checkout HEAD -- ABSOLUTE_PATHand proves the restore by state(
git diff HEAD,git diff --cached,git status --shortall empty, blobs back to the HEADhashes), with the restore trapped on
EXIT INT TERM. The root vitest config aliases@object-ui/*to each package'ssrcand both files import their subject relatively, so nobuild stands between the edit and the run.
ObjectGridgate removedListViewgate removedRemoving either site's gate leaves the other site's tests fully green. That is the property
objectui#7179 warned about, shown rather than claimed.
Suites, at the commit being shipped
All runs below are at
74f9521ac(this branch's head, after mergingorigin/main— bothin-flight PRs touching these files landed mid-task and are merged in, not rebased over).
pnpm exec vitest run packages/plugin-grid/ packages/plugin-list/from the repo root —170 files passed, 1766 tests passed, including
projectionFls-6898.test.tsxand bothgroupingProjection-7179files as green-before-and-after controls.pnpm type-check— 81/81 tasks successful. Measured rather than assumed that this coversthe new tests: each package's
tsconfig.jsonexcludes__tests__, buttype-checkchainstsc -p tsconfig.test.json, and--listFilesshows both new files in that program (1 hiteach).
npx eslint packages/plugin-grid packages/plugin-list --format json— 219 files, 0 errors(1236 pre-existing warnings). Per-file before/after on the two edited sources is identical:
ObjectGrid.tsx0 errors / 211 warnings,ListView.tsx0 errors / 182 warnings, both onorigin/mainand here — this change adds no lint finding.node scripts/check-changeset-presence.mjs,pnpm check:control-bytes,pnpm check:vi-mock-specifiers,pnpm check:vi-mock-inherit— all green.Declared narrowing. The repo-wide
pnpm testandturbo run lintare left to CI. eslint wasrun over both affected packages in full rather than the whole repo; the config
(
eslint.config.js) sets noparserOptions.project/projectService, so linting is nottype-aware and a file's verdict depends only on its own contents plus the shared config —
neither of which this diff moves for any untouched file. Vitest was narrowed by path to the two
packages the diff touches, which is this repo's documented way to narrow (root cwd, path
filter, no
--).A changeset is included; both packages are published and their behaviour changes.
Generated by Claude Code
Generated by Claude Code