fix(plugin-grid): re-apply FLS on generateColumns()'s authored columns path - #6897
Conversation
…ns` path
`generateColumns()` has three default paths. Two re-applied field-level
security and the authored `columns` path did not — the last one left after
objectui#6723 closed the inline-data path, and the worst of the three to leave:
it is the MOST reachable, running whether the grid fetches its own rows or not,
while the inline-data path needs a host to hand rows down.
Both arms (`ListColumn[]` and `string[]`) now filter through
`perms.checkField(objectName, fieldName, 'read')` under the same
`perms.isLoaded && schema.objectName` deferral the other two paths use.
Only keys the OBJECT DECLARES are judged. Host-joined and derived columns pass
through untouched — a `ListColumn` carries `label`/`link`/`action`/`prefix`/
`width`, so a column whose `field` the object does not declare is a legitimate
authored derived column, and an over-eager filter would delete authoring work.
`hasOwnProperty` keeps an inherited name (`constructor`) from being mistaken
for a declared field.
The judged key is read through `columnIdentity`, never off a bare string: it
folds `'salary'`, `{ field: 'salary' }` and the legacy `{ name: 'salary' }`, so
one predicate serves both arms. A gate reading `col.field` directly would find
no identity on the legacy spelling and pass a denied declared field through.
`resolvesToDataColumn` still runs first and keeps owning its own decisions.
objectui#6598's `hasAuthoredColumns` predicate is unchanged and its rationale
is rewritten here, in the same commit: it rested on "the grid would not
re-check", which this change makes false. It now rests on the half that never
depended on the grid — an empty projection is the author's projection after
filtering, and the object's default columns are not what was authored whether
or not they are FLS-checked on the way out.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
✅ 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 — ACCEPTLanding gated on CI. ⛔ No clause-② review, and its absence is deliberate: the ruling states this ⭐ The census changed what this card isThe card's own axis ① said "zero measured victims" and both triage and the maintainer reasoned on
The control hits, so the zeros are a reading. Three shipped components reach the Re-verifying the card's reading was handled exactly rightMy order said: re-run it on today's "The path table is correct as written; only the raw grep count moved" is the correct conclusion, The two things most likely to have been got wrong, both got right1. Derived columns survive. This was the constraint I flagged as most likely to break under an 2. #6598's rationale: the ruling said one site, the sweep found threeThe stale premise lived in the And the rewrite is a real re-derivation, not a patch over the words: the predicate stays correct Confirming the pin was undisturbed by measurement (18/18 green with the fix, and again after the VerificationRed-first is a security assertion, not a shape one — 5 failed / 8 passed on untouched TS2307 on every The out-of-scope note is right to be a note: the query projection at L1402 building Generated by Claude Code |
2 participants
Fixes #6799
Executes the maintainer ruling of 2026-08-30 12:21Z (verbatim 「第一批其余同意」): 补检查, same
direction as #6723. Option B (host owns it) was refused and is not re-raised here.
generateColumns()has three default paths. Two re-applied field-level security and the authoredcolumnspath did not — the last one left after #6723 closed the inline-data path, and the worstof the three to leave: it is the most reachable, running whether the grid fetches its own rows
or not, while the inline-data path needs a host to hand rows down.
Both arms (
ListColumn[]andstring[]) now filter throughperms.checkField(objectName, fieldName, 'read')under the sameperms.isLoaded && schema.objectNamedeferral the other two paths already use.Measurements owed before implementing
1. The card's own reading, re-verified on today's merged
main(b392674c5). The card'sevidence was measured on
d06059f24, before #6723 merged. It no longer reproduces literally, andthe difference is fully explained:
d06059f24)b392674c5)git grep -n checkField -- packages/plugin-grid/src/ObjectGrid.tsxcolumnsblock re-applies FLSThe load-bearing claim holds: the
if (cols)block (lines 1961–2289 onb392674c5, both arms)contained 0 occurrences of
checkFieldorperms. Both armsreturnbefore reaching theobject-schema path's gate. The path table is correct as written; only the raw grep count moved.
2. The composition census — the impact denominator nobody had produced. Searched
packages/,apps/,examples/for sites reaching the authored-columnspath without passing throughcheckFieldfirst. It is not zero:packages/plugin-view/src/ObjectView.tsx(L1470 / L1822)objectName+columnsfrom the named view /table.columnscheckFieldin filepackages/plugin-designer/src/ObjectManager.tsx(L113 / L252)objectName: 'object_definition', 4 authoredListColumnspackages/plugin-designer/src/FieldDesigner.tsx(L166 / L414)objectName: 'field_definition', authoredListColumnsapps/console/src/dev/DevRowActions.tsx(L45)columns+ inline datapackages/plugin-grid/demo/bulk-actions.tsx(L294)3 shipped + 2 dev/demo. The control that must hit in the same query is non-zero as required:
ListView(L2166) emitstype: 'object-grid'withcolumns: effectiveFields, andeffectiveFieldsis gated (L2060–2067) — 32 in-repoListViewreferences. So a zero abovewould have been a reading; it was not zero anyway. One count serves this card and #6723.
3. Whether rewriting #6598's rationale disturbs its own pin. It does not. Its three files plus
#6723's
ListView.inlineFlsNoopran 18/18 green with this fix in place and again after therationale rewrite. The grid-side gate is a measured no-op through
ListView, which is theredundancy the ruling wants.
The limit, and which key is judged
matters more here than on the inline-data path: a
ListColumncarrieslabel/link/action/prefix/width, so a column whosefieldthe object does not declare is not a mistake but alegitimate authored derived column, and deleting it would destroy authoring work.
checkFieldanswers
falsefor a field the policy has never heard of, so judging derived keys is a different,wrong rule — not a stricter reading of the same one.
hasOwnPropertykeeps an inherited name(
constructor) from being mistaken for a declared field.⛔ The judged key is read through
columnIdentity, never off a bare string, as the rulingrequires by name. It folds the three authored identity spellings —
'salary',{ field: 'salary' }and the legacy{ name: 'salary' }— which is why one predicate servesboth arms. A gate reading
col.fielddirectly would find no identity on the legacy spelling andwave a denied declared field straight through; PIN 5 pins exactly that.
resolvesToDataColumnstill runs first and keeps owning its own decisions, so the gate narrowswhat survives and never resurrects a hidden or unresolvable column.
#6598's rationale, rewritten in this same commit (ruling requirement)
The stale premise was in three places, not one — a repo-wide sweep found them all:
packages/plugin-list/src/ListView.tsx— thehasAuthoredColumnsdocblock..../__tests__/ListView.unauthoredColumnProjection-6598.test.tsx— the file docblock.All three said the predicate is correct because the grid would not re-check. That is now
false. The predicate is unchanged and stays correct for the half that never depended on the grid:
an empty projection is the author's projection after filtering, and handing the grid
"unauthored" would replace it with the object's default columns — fields the author never
declared. FLS-checked now, but still not what was authored. Authoring intent, not the field
gate, is what #6598 protects. Each site names the retired reason explicitly so the next reader
sees a correction rather than a silent edit.
Verification
Red first, and it is a security assertion. On untouched
main, the new pin file ran5 failed / 8 passed — every red the denied column rendering:
After the fix: 13/13 green.
Reverse-verification (prediction stated before the run). Predicted: reverting only the two
passesFieldGatefilter lines turns exactly 5 tests red — PIN 2, PIN 2b, PIN 2c, PIN 5, WIRING —each with the message above; the 8 controls do not move; #6598's pins stay green. Observed exactly
that:
Tests 5 failed | 14 passed (19), with the two #6598 files green in the same run. The 8unmoved controls are the half that says the guard is narrow.
passesFieldGate(call sites2 → 0, andgit hash-object(9c9c06ba…) differing fromgit rev-parse HEAD:path(
0356ce42…).git hash-object==git rev-parse HEAD:<path>(both0356ce42d41e4aa1a0fc3b251ad7097eb2029799),git diff HEADempty,git statusclean, 2 callsites back.
trap restore EXIT INT TERMwith an absoluteREPO_ROOT-anchored path; restore namesHEADexplicitly rather than a bare
git checkout --.vitest.config.mtsaliases every@object-ui/*specifier to that package's
src(verified in this PR, not taken on faith), and the test imports../ObjectGridrelatively — so the ablation reads source directly.Union at the final commit
74ef4bd25(working tree clean, run after the last commit):pnpm exec vitest run packages/plugin-grid/ packages/plugin-list/Test Files 153 passed (153)·Tests 1632 passed (1632)pnpm --filter @object-ui/plugin-grid --filter @object-ui/plugin-list run type-checktype-check, the hyphenated name)pnpm --filter … run lint✖ 699 problems (0 errors, 699 warnings)/✖ 442 problems (0 errors, 442 warnings)— exit 0An earlier
type-checkreported TS2307 on every@object-ui/*import; that is the unbuilt-closuresignature and was read as NOT MEASURED, not as red. After
pnpm --workspace-concurrency=2 --filter '<pkg>^...' buildit is clean. Coverage was confirmedrather than assumed:
tsc -p tsconfig.test.json --listFilesshows all four edited source/testfiles in the checked population.
Lint is reported on errors, which is what this repo's own gate measures —
lint.ymlstates--max-warningsis deliberately unset because repo-wide warnings "run into the thousands …known historical debt, not a signal". The two affected packages' full lint populations were
run (not a narrowing), and separately all 5 changed files were confirmed present in eslint's own
--format jsonpopulation (5 of 5, none ignored, 0 errors). Type-aware linting is not enabled(no
project/projectServiceinlanguageOptions) and no eslint config is touched, so thisdiff cannot move the verdict on any untouched file.
Gates derived from the actual diff, each quoting its own verdict line:
check:control-bytes—✅ OK (scanned 5745 tracked text file(s); skipped 85 binary)check:vi-mock-specifiers—✅ OK (…499 carry a mock…)(the new test usesvi.mock)check:element-data-source-declaration—OK — 13 gate-consuming file(s) checkedcheck:phantom-deps—✅ Every in-scope import is declared by the package that publishes it.check:self-import—✅ No package names itself inside its own src/.check-changeset-presence.mjs—✅ 4 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)No gate or tooling script is edited by this diff, so no gate self-test suite is owed.
Notes
⇒ human floor, and the control is the ruling itself. Its absence here is deliberate, not an
oversight.
patchon@object-ui/plugin-grid, matching plugin-grid: ObjectGrid re-applies field-level security only on the object-schema column path, so a host-fed grid with an authored fields projection skips it #6723's precedent, and states what aconsumer will feel: hosts that already filter (
ListView) see no change; hosts that do not willsee denied columns disappear.
@object-ui/plugin-list's change is comment-only, so it takes nobump — same split plugin-grid: ObjectGrid re-applies field-level security only on the object-schema column path, so a host-fed grid with an authored fields projection skips it #6723 made.
$selectfrom the authored projection with no FLS gate #6898 rather than fixed here: the grid's query projection(
ObjectGrid.tsx,getSelectFields()around L1402) still builds$selectfrom the authoredprojection with no field gate, so a denied field can be requested and returned into the browser
even though this PR now hides its column. The ruling scopes this card to
generateColumns(),and filtering a projection raises its own questions (
ensureId, the predicate-only fields), soit is deliberately untouched here and left for triage.
https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB— recorded in theprose because a body edit rewrites the footer below to its bare form.
Generated by Claude Code