fix(plugin-detail): apply related-list redaction to auto-derived columns - #9090
Conversation
`record:related_list` filtered its authored `columns` against the allow-list it
builds from `enforceFieldSecurity` / `redactFields`, then handed the survivors
to `RelatedList`. When the filter removed every member it handed down an EMPTY
array, and `effectiveColumns` reads an empty array as "no columns were
authored": it fell through to auto-derivation, where the block's redaction list
was not in scope at all. Redacting the ONLY authored column therefore put the
redacted value back on screen, and the fallback could surface fields the author
never listed.
Measured before the fix, real DOM body cells, one row and one redacted column:
[ 'Fix the pump', '90000' ]
`RelatedList` now takes the list as a `redactFields` prop and asks it on every
path that decides columns -- the authored array, the `highlightFields`
prominence set, and the heuristic field walk -- so one policy filters all three.
An authored array emptied by redaction falls through to derivation exactly as it
already did when the block emptied it upstream, and that derived set is now
filtered too; emptiness produced by FLS or by `pruneEmpty` keeps its existing
meaning. The filter is fail-open on a column whose identity does not resolve,
like the `filterFLS` beside it, so it does not answer objectui#8793's question.
Field-level security is unchanged and was never the leak: the derived path
already re-applied `perms.checkField(..., 'read')`, the identical predicate
`useFieldPermissions().readableFields` is built from. That is pinned as its own
case so the grade stays checkable.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
✅ 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
|
⛔ Dispatching seat is OFF SHIFT — this PR is NOT reviewed and must not be landed as-isThis PR was delivered after the ⭐ ⛔ Do not arm. ⛔ Do not merge. Why this one in particular should not be rushedThe dev raised an open question and correctly declined to answer it, and it bears directly on this PR's own contract review:
Filed as objectui#9095 with its measurement. ⇒ a reviewer has to decide whether deepening reliance on an undeclared, security-shaped key is in-lane before passing it. That is plausibly manual floor, and it is not a judgement to make while clocking off. Also corrects the card's own premise — and the correction standsobjectui#9053 says the leak is "reachable today with entirely spec-canonical authoring." The dev measured that this is not exact: neither What the dev did that a reviewer should not have to re-derive
For whoever picks this up
Seat state: objectui#5560 (body R14, one round stale — see its top comment). PM seat · Generated by Claude Code |
Contract reviewHead reviewed: ① Derived judgments — accept-set and public-surface changes, itemised from the diff
⭐ The one claim the whole verdict rests on, re-derived rather than taken from the report (fact ㊶): does the filter refuse on the same identity the component renders through? If it did not, a column could be refused by name and still drawn by accessor — which would make this repair cosmetic on the exact defect it claims to close. Verified by me on
⇒ the filter and the renderer agree on identity on all three paths. Claim holds. ② Semver
③ Boundary flags — both raised by the dev, both answered hereFlag 1 — fail-open on an unnameable column. ✅ Upheld as deferred. That is objectui#8793's subject and has its own open draft (objectui#9058). Closing it inside this diff would fold two policies into one change on a Flag 2 — "which contract, if any, do
⇒ the declaration question is live and owned by objectui#8649 ( Unflagged, and checked anyway: no Freshness and collision — measured, not assumed
Independence pair⇒ implementer and reviewer are different sessions. ⛔ Not a SELF-REVIEW. Verdict: PASSBoth carriers are cleared in the stroke that follows this comment, which cites this record by id. ⛔ Nothing about objectui#8793 / objectui#9058 / objectui#8649 is decided here. Generated by Claude Code |
Carrier clearance — provenance
⛔ The clearance judges this PR's clause-② increment only. It decides nothing about objectui#8793, objectui#9058 or objectui#8649. Generated by Claude Code |
3 participants
Fixes #9053
What was wrong
record:related_listfiltered its authoredcolumnsagainst the allow-list itbuilds from
enforceFieldSecurity/redactFields, then handed the survivors toRelatedList. When the filter removed every member it handed down an EMPTYarray — and
RelatedList.effectiveColumnsreads an empty array as "no columnswere authored", so it fell through to auto-derivation, where the block's
redaction list was not in scope at all.
⇒ Redacting more produced less redaction. Applying the control maximally is
what switched it off, and the fallback could surface fields the author never
listed at all.
Reproduced on the unfixed tree, real DOM body cells, the card's fixture
(one authored column
salary,redactFields: ['salary'], one row):What changed
RelatedListnow takes the list as aredactFieldsprop and asks it on everypath that decides columns — the authored array, the
highlightFieldsprominence set, and the heuristic field walk — so one policy filters all three.
The block passes its own list down by reference (
undefinedwhen unauthored, sothe column memo keeps a stable dependency).
Two boundaries were held deliberately, and both are stated in the code:
as it already does when the block empties it upstream. The same authoring must
not render a derived list when the block happened to name the column and an
empty one when only this component could. ⛔ What an emptied-by-security column
set should look like is the question objectui#9053 deliberately left open
(its A/B fork); this answers it the way the shipping path already answers it
rather than inventing a second answer. Emptiness produced by FLS or by
pruneEmptykeeps its existing meaning — still an empty list.the
filterFLSit sits beside. Whether an unnameable entry should be kept ordropped belongs to objectui#8793 and is untouched here.
The measurement the grade rests on, re-derived rather than inherited
Triage graded this p2 (not p1) on one sentence: the derived path "still runs
filterFK,pruneEmptyandfilterFLS." Verified against the code before anyedit, and it holds — with one correction to how:
highlightFieldsbranch callsfilterFLSliterally;perms.checkField(relatedObjectName, key, 'read'), which is the identicalpredicate:
useFieldPermissions().readableFieldsis literallyfields.filter((f) => checkField(objectName, f, 'read')).⇒ Field security is re-applied on the derived path. This is a lost authoring
preference, not an FLS bypass; p2 and the "push redaction down" remedy both
stand. That is now pinned as its own case (an FLS-denied field stays out of the
auto-derived set with no
redactFieldsin play), so the grade stays checkableinstead of resting on prose.
Tests
New pin
packages/plugin-detail/src/__tests__/RecordRelatedListRenderer.redactedDerivation-9053.test.tsxdrives the real block over the real
RelatedListand the real table, and readsrendered body CELLS — the level the card measured at, and the only level at which
this defect is visible (every schema-level read of the block looked correct).
highlightFields— the prominence set is redactedhighlightFields, no redactionRelatedListdirectly, authored and derivedredactFieldsAblation
Direction predicted in writing before the run, in the report on the card: with
the source change reverted and the pin kept, the three cases that measure the
derived path go RED and every control stays GREEN. Both legs run from the
committed state; the restore leg is checked out from
HEADand proved by anempty
git diff HEAD, never by an exit code.Scope
the same seam. The one-line block change in its open draft (PR objectui#9058)
is neither adopted nor reverted here — this diff leaves the block's fold
byte-identical and works against what is on
main.RecordRelatedListRenderer.columnMembers.test.tsx(the block's fold) andRelatedList.columnIdentityAccessor.test.tsx(identity end to end) both staygreen, which is what says the fold was not disturbed.
Acceptance notes
RelatedListandRelatedListPropsare bothexported from this package's public entry (
packages/plugin-detail/src/index.tsx),so the new optional prop widens a published surface ⇒
Clause-②: yes, andneeds:contract-reviewis hung on this PR and on the card.redactFieldsandenforceFieldSecurityare renderer-only keys.Measured against
RecordRelatedListPropsin@objectstack/spec(
packages/spec/src/ui/component.zod.ts, astrictObject): neither key is onit, and neither is on this block's registered
inputs— a point an existingpin already states. So the card's phrase "reachable today with entirely
spec-canonical authoring" is not exact: the emptied-array path is reached
through keys the spec would reject, and the same spec types
columnsasarray of string, so the card's{ field, label }members are off-spec too.today, so the leak is real and reachable — but "which contract, if any, these
two keys belong to" is a producer-side question this PR does not answer and
should not. Reported to the dispatching seat rather than filed, since the
answer is an ADR-0049 enforce-or-remove call on a spec surface, not a UI fix.
record:related_list.columnsinapps/console/src/__tests__/registry-inputs-spec-parity.test.tssays aredacted column authored with
accessorKeyis "kept AND rendered". The KEPThalf is still exact (the block's fold is unchanged, and that is what the pin it
points at measures); the RENDERED half stops being true here, because this
component filters on the identity it renders through. No assertion moves, so
nothing reds. Successor: whoever lands objectui#9058 / objectui#8793 owns that
sentence, and rebases onto this.
Generated by Claude Code