Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-list): FLS-gate the speculative $select fields in ListView - #7261
Merged
Conversation
`ListView`'s projection builder asked two different questions of two different populations. `schema.columns` went through `perms.checkField(...)` (objectui#6898); everything `addSpeculative` adds on top — the kanban / gantt / timeline / calendar / gallery bindings, the timeline's auto-added `status` / `priority` badges, and the predicate operands harvested by objectui#3501 — was intersected against the object's declared fields and then added unconditionally. The two gates answer unrelated questions. The known-field gate keeps an UNKNOWN key out (some backends answer an unknown `$select` key with an empty result set). The FLS gate keeps a KNOWN BUT DENIED key out (sending it leaks the value at the server boundary even though the UI hides it). A field can be well-declared and still denied. The gate goes INSIDE `addSpeculative`, after its known-field intersection — objectui#7179's ordering, because `checkField` answers false for an undeclared key. Platform record columns are carved out for the reason they already are in `addPredicateField`: every object carries them, none declares them, so an FLS answer about them is always false. `addGroupingField` is removed; its predicate was identical to the one now inside the helper, and that path gains the FLS pin plugin-list never had. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRRumy89BYdW9ogbcdHTho
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
|
os-litant
marked this pull request as ready for review
September 2, 2026 02:11
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
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#7216
What was wrong
ListView's$selectbuilder assembles its projection from two populations and, untilnow, asked them different questions.
schema.columnsaddSpeculativeThe two gates answer unrelated questions and neither substitutes for the other:
unknown
$selectkey with an empty result set rather than ignoring it (the cloudmulti-tenant runtime does exactly that), which silently zeroes the whole list.
at the server boundary even though the UI hides it.
A field can be perfectly well-declared and still denied. That is the case this path did
not handle: a kanban grouped by a denied field, a gantt bound to a denied date, a gallery
bound to a denied cover — each named the denied field in the request.
Premise: verified, not assumed
The card was filed as the implementing lane's reading, never independently reproduced.
It reproduces. On the unmodified tree at
67dadd602, 8 of the 14 new pins fail(
Tests 8 failed | 6 passed (14)), for example:Every failing pin reaches the denied field only through a view binding, with the
columnslist holding permitted fields. That is deliberate: a denied column has beendropped since objectui#6898, so a pin naming one would pass on the unmodified tree and
prove nothing. PIN 12 asserts that discriminator instead of leaving it a convention.
The change
One gate, placed inside
addSpeculative, after the known-field intersection italready performs — so all five call sites are covered at once rather than one at a time,
which is how the asymmetry arose in the first place. Per-caller state after the change:
addPredicateField(non-platform arm)addPredicateField(platform arm)owner_id,organization_id, the audit FKs — added directly, bypassing the helpercollectViewFieldsoptions.spellingstatus/prioritybadge fieldsgrouping.fields[]entriesOrdering is load-bearing and follows objectui#7179's shape: intersect against the
declared fields first, ask
checkFieldonly about the survivors.checkFieldanswersfalse for an undeclared key, so asking it first would drop derived and computed bindings
— and would be the reason they were dropped, a worse failure than the known-field gate
declining them. PIN 10 pins that.
The platform carve-out is load-bearing too, and is objectui#7179's, not new. The
platform record columns are provisioned on every object and published in none, so no
field policy mentions them and
checkFieldanswers false for every one.created_atisin
knownObjectFields(the builder adds it), so without the carve-out a calendar boundto
created_atwould go blank for everybody. PIN 11 pins that.In-place cleanup, declared
addGroupingFieldis removed, and its loop callsaddSpeculativedirectly. Itspredicate was character-identical to the one now inside the helper, so the collapse is
behaviour-preserving by inspection — and the ablation below measures it: with the moved
gate deleted, PIN 9 (
grouping.fields[]) turns red along with the eight, which it didnot do before the fix. Two spellings of one gate is the shape that lets them drift.
That path also gains a pin it never had:
plugin-listhad no FLS test for thegrouping projection before this PR (
ListView.groupingProjection-7179.test.tsxpins theunion, not the gate).
Ablation
Run after committing, so the restore leg has a real reference. The test imports
../ListView— a relative source import, not a packageexportsboundary — so nodist/rebuild is in the resolution path and none was needed.Mutation confirmed on disk before the run, not inferred from an editor exit code:
Ablated result:
Tests 9 failed | 5 passed (14)— the eight defect pins plus PIN 9, withall five controls still green (permitted binding, undeclared binding, platform column,
denied column via the old gate, deferral). Restore leg proven by observation, not by an
exit code:
git diff HEADempty, worktree blob0f575c8e…equal to the HEAD blob, gateline count back to 1.
Verification
All at
b081602ee, the head this PR opens on.pnpm exec vitest run packages/plugin-list/Test Files 62 passed (62)·Tests 787 passed (787)pnpm --filter '@object-ui/plugin-list' run type-checktsc --noEmit && tsc -p tsconfig.test.json— the test tsconfig covers the new file)pnpm --filter '@object-ui/plugin-list' run lint466 problems (0 errors, 466 warnings)check-control-bytesOK (scanned 5998 tracked text file(s); skipped 85 binary)check-vi-mock-specifiersOK (4128 tracked source file(s), 2398 test-named; 533 carry a mock; …)check-vi-mock-inheritOK (… 118 call site(s) on @object-ui/react judged (118 inherit, 0 auto-mocked))check-package-self-importNo package names itself inside its own src/.check-phantom-dependenciesEvery in-scope import is declared by the package that publishes it.check-changeset-presence2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-fixedAll workspace packages are in the changeset fixed group.check-changeset-no-majorNo changeset declares a major bump.Lint narrowing, declared as a measurement. The repo-wide farm is CI's run; locally
lint was narrowed to the affected package, and the narrowing excluded nothing that this
diff could move: (1) the population came from eslint's own resolution —
eslint .insidepackages/plugin-list, which has no config of its own and resolves the rooteslint.config.js; (2)--format jsonreports 73 files linted, 0 errors, 466warnings, and both edited files are in that list; (3) the root config declares no
project/projectServiceinlanguageOptionsand no cross-file rules (noimport-plugin resolution, no
settingsblock), so type-aware linting is off and thisdiff cannot move a verdict on any file it does not touch.
Scope
Client-side request projection only. No
@objectstack/specsurface moves, noaccept/reject behaviour changes, no public API widens. Three files:
ListView.tsx, onenew test, one changeset.
Related, and none of them addressed here: objectui#6898 (the original
$selectFLS card,closed) · objectui#7179 (the reference shape) · objectui#7215 / PR #7229 (the
$expandgate, untouched — this PR does not go near
expandFields) · objectui#7218 (thefieldOrder/rowColorrelay, out of scope) · #7230 remains open.🤖 Generated with Claude Code
https://claude.ai/code/session_01NRRumy89BYdW9ogbcdHTho
Generated by Claude Code
Generated by Claude Code