From 571fba8c7bd7bb63dd6f207a755e6c7448de071b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 00:16:03 +0000 Subject: [PATCH] fix(plugin-detail): apply related-list redaction to auto-derived columns `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 Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB --- .changeset/olive-pugs-repeat.md | 21 ++ packages/plugin-detail/src/RelatedList.tsx | 84 +++++- ...tRenderer.redactedDerivation-9053.test.tsx | 244 ++++++++++++++++++ .../src/renderers/record-related-list.tsx | 9 + 4 files changed, 355 insertions(+), 3 deletions(-) create mode 100644 .changeset/olive-pugs-repeat.md create mode 100644 packages/plugin-detail/src/__tests__/RecordRelatedListRenderer.redactedDerivation-9053.test.tsx diff --git a/.changeset/olive-pugs-repeat.md b/.changeset/olive-pugs-repeat.md new file mode 100644 index 0000000000..20386484aa --- /dev/null +++ b/.changeset/olive-pugs-repeat.md @@ -0,0 +1,21 @@ +--- +'@object-ui/plugin-detail': minor +--- + +fix(plugin-detail): `record:related_list` redaction now reaches the auto-derived columns + +Redacting **every** authored column used to switch redaction off. The block +filtered its authored `columns` against `redactFields`, handed `RelatedList` the +empty result, and `RelatedList` read an empty array as "no columns were +authored" — falling through to auto-derivation, which the block's redaction list +never reached. The redacted field came back, and the fallback could surface +fields the author never listed at all. + +`RelatedList` now takes the list as a `redactFields` prop and applies 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 the derived set exactly as it already +did, and that set is now filtered too. + +Field-level security is unchanged: it was, and remains, enforced independently on +every path. diff --git a/packages/plugin-detail/src/RelatedList.tsx b/packages/plugin-detail/src/RelatedList.tsx index 42d6b30664..9c985aa192 100644 --- a/packages/plugin-detail/src/RelatedList.tsx +++ b/packages/plugin-detail/src/RelatedList.tsx @@ -134,6 +134,26 @@ export interface RelatedListProps { toolbarActions?: RelatedRowActionDef[]; /** Execute one of {@link toolbarActions} (no row context). */ onToolbarAction?: (action: RelatedRowActionDef) => void | Promise; + /** + * Field names this list must never show, whatever decided its columns + * (objectui#9053). + * + * The block-level authoring preference `record:related_list` reads as + * `redactFields`, pushed down to the component that actually decides + * columns. It used to be applied only where the block could apply it — over + * the AUTHORED `columns` array — and this component has two more paths that + * decide columns on their own (`highlightFields` prominence and the + * heuristic field walk), which that list never reached. Redacting EVERY + * authored column therefore emptied the array, the empty array read as "no + * columns were authored", and the derived set brought the redacted field + * straight back: applying the control maximally switched it off. + * + * ⚠️ This is an AUTHORING preference, not the permission boundary. Field + * security is enforced independently and unconditionally through + * `perms.checkField(..., 'read')` on every path below; a field that must be + * unreachable belongs in FLS, not here. + */ + redactFields?: string[]; /** Maximum number of columns to auto-generate. Default 6. */ maxColumns?: number; /** Page size for pagination (enables pagination when set) */ @@ -391,6 +411,7 @@ export const RelatedList: React.FC = ({ toolbarActions, onToolbarAction, add, + redactFields, maxColumns = 6, pageSize, defaultSort, @@ -1009,6 +1030,22 @@ export const RelatedList: React.FC = ({ // - Cap at `maxColumns` to keep the related card readable; users can // click "View All" to see the full list. const perms = usePermissions(); + /** + * [objectui#9053] The redaction list as a lookup, memoised on the PROP's + * identity so `effectiveColumns` keeps the reference-stable dependency the + * rest of this file is built around: a caller that passes no list passes + * `undefined`, which never changes, and one that passes its authored array + * passes it by reference. + */ + const redactedFields = React.useMemo( + () => + new Set( + (Array.isArray(redactFields) ? redactFields : []).filter( + (f): f is string => typeof f === 'string' && f.length > 0, + ), + ), + [redactFields], + ); const effectiveColumns = React.useMemo(() => { const relatedObjectName = objectName || api || ''; // FLS: drop columns the current user cannot read on the related object. @@ -1028,6 +1065,27 @@ export const RelatedList: React.FC = ({ }) : cols; + /** + * [objectui#9053] Redaction — the block-level authoring preference, asked + * on EVERY path below rather than only over the authored array. + * + * Identity is resolved the way this component resolves it everywhere else + * (`accessorKey || columnIdentity`), because that is the key it RENDERS + * through: filtering on any other reading would leave a column refused by + * name and drawn by accessor. + * + * ⛔ Fail-OPEN on a column it cannot name, exactly like `filterFLS` beside + * it. Whether an entry whose identity does not resolve should be kept or + * dropped is objectui#8793's question, not this one, and answering it here + * would fold two policies into one diff. + */ + const isRedacted = (key: unknown): boolean => + redactedFields.size > 0 && !!key && redactedFields.has(String(key)); + const filterRedacted = (cols: any[]): any[] => + redactedFields.size > 0 + ? cols.filter((c) => !isRedacted(c?.accessorKey || columnIdentity(c))) + : cols; + /** * Does this cell have nothing to show? **THE** definition of emptiness on * this surface (objectui#8459), read by BOTH places that decide what the @@ -1268,7 +1326,21 @@ export const RelatedList: React.FC = ({ }; if (columns && columns.length > 0) { const normalized = columns.map(normalizeColumn); - return pruneEmpty(filterFLS(filterFK(normalized))); + // [objectui#9053] Redaction is applied to the authored candidates FIRST + // and their emptiness judged HERE, so an array emptied by redaction + // behaves exactly as it already does when the BLOCK empties it upstream + // — it falls through to the derivation below, which is redaction-filtered + // too. That keeps one outcome for one input: 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 objectui#9053's deferred question; this + // deliberately answers it the way the shipping path already answers it + // rather than inventing a second answer. Emptiness produced by FLS or by + // `pruneEmpty` keeps its existing meaning untouched: still an empty list. + const candidates = filterRedacted(normalized); + if (candidates.length > 0) { + return pruneEmpty(filterFLS(filterFK(candidates))); + } } if (!objectSchema?.fields) return []; @@ -1285,7 +1357,9 @@ export const RelatedList: React.FC = ({ ) : []; if (declaredHighlights.length > 0) { - const hf = pruneEmpty(filterFLS(filterFK(declaredHighlights.map(normalizeColumn)))); + const hf = pruneEmpty( + filterFLS(filterFK(filterRedacted(declaredHighlights.map(normalizeColumn)))), + ); if (hf.length > 0) return hf.slice(0, Math.max(1, maxColumns)); } @@ -1331,6 +1405,10 @@ export const RelatedList: React.FC = ({ if (key === 'id' || key === referenceField) return false; if (def?.hidden) return false; if (def?.type && SKIP_TYPES.has(def.type)) return false; + // [objectui#9053] Redaction: drop redacted fields from the walk too — + // asked here rather than over `generated` so the priority sort and the + // `maxColumns` slice below both see the set the reader will get. + if (isRedacted(key)) return false; // FLS: drop unreadable fields from auto-derived columns too. if (perms?.isLoaded && resolvedObjectName && !perms.checkField(resolvedObjectName, key, 'read')) { @@ -1378,7 +1456,7 @@ export const RelatedList: React.FC = ({ const pruned = pruneEmpty(generated); return pruned.slice(0, Math.max(1, maxColumns)); - }, [columns, objectSchema, objectName, api, resolveFieldLabel, referenceField, relatedData, maxColumns, lookupLabels, perms]); + }, [columns, objectSchema, objectName, api, resolveFieldLabel, referenceField, relatedData, maxColumns, lookupLabels, perms, redactedFields]); /** * [#6108] The SERVED per-column sortability projection for this object — diff --git a/packages/plugin-detail/src/__tests__/RecordRelatedListRenderer.redactedDerivation-9053.test.tsx b/packages/plugin-detail/src/__tests__/RecordRelatedListRenderer.redactedDerivation-9053.test.tsx new file mode 100644 index 0000000000..3bb52dd5bb --- /dev/null +++ b/packages/plugin-detail/src/__tests__/RecordRelatedListRenderer.redactedDerivation-9053.test.tsx @@ -0,0 +1,244 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * objectui#9053 — redaction must reach EVERY path that decides columns. + * + * `record:related_list` filters its authored `columns` against an allow-list + * built from `enforceFieldSecurity` / `redactFields`, then hands the survivors + * to `RelatedList`. `RelatedList.effectiveColumns` takes the authored array + * only `if (columns && columns.length > 0)`; otherwise it AUTO-DERIVES a set + * from the child object's `highlightFields` or from a heuristic field walk. + * + * So redacting EVERY authored column empties the array, the emptied array reads + * as "no columns were authored", and the derived set brings the redacted field + * straight back. Measured before the fix, real DOM cells, the card's fixture: + * + * [ 'Fix the pump', '90000' ] // `salary` was the redacted field + * + * ⇒ applying the control maximally is what switches it off. The repair pushes + * the policy down to the component that decides columns, so ALL THREE paths — + * authored, `highlightFields`, heuristic walk — are filtered by one policy. + * + * ## Scope fence (objectui#8793 is a DIFFERENT hole on the same seam) + * + * The block's fold keeps a member whose identity it cannot NAME + * (`colName` → `null` ⇒ kept), which is objectui#8793's subject and is not + * touched here: the filter added below is fail-open on an unnameable column, + * exactly like the `filterFLS` it sits next to. What it does cover is a column + * this component CAN name — including the table's own `accessorKey` spelling, + * because that is the identity this component renders through. + * + * FLS is NOT what leaks here, and that is load-bearing for the grade: the + * derived path re-applies `perms.checkField(...,'read')` — the identical + * predicate `useFieldPermissions().readableFields` is built from — so the + * permission boundary holds and only the block-level authoring preference was + * lost. Pinned as its own case below rather than asserted in prose. + */ + +import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { cleanup, render, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import * as React from 'react'; +import { RecordContextProvider } from '@object-ui/react'; +import { PermissionProvider } from '@object-ui/permissions'; +import type { ObjectPermissionConfig, RoleDefinition } from '@object-ui/types'; +import { RecordRelatedListRenderer } from '../renderers/record-related-list'; +import { RelatedList } from '../RelatedList'; + +/** + * Desktop, pinned rather than inherited (the objectui#8399 reason): under the + * 768 breakpoint a `type="table"` related list renders a card gallery with no + * cells at all, and every assertion here reads rendered CELLS. + */ +beforeAll(() => { + Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 }); +}); + +afterEach(() => cleanup()); + +/** The card's fixture: one child object, two declared fields, one row. */ +const FIELDS = { + subject: { type: 'text', label: 'Subject' }, + salary: { type: 'text', label: 'Salary' }, +}; + +const ROWS = [{ id: 't1', subject: 'Fix the pump', salary: '90000' }]; + +const makeDS = (schemaExtra: Record = {}) => ({ + find: vi.fn(async () => ROWS), + getObjectSchema: vi.fn(async (name: string) => ({ name, fields: FIELDS, ...schemaExtra })), +}); + +/** Every rendered body cell's text, in DOM order. */ +const cellTexts = () => screen.getAllByRole('cell').map((c) => (c.textContent || '').trim()); + +/** Render the BLOCK end to end over the real `RelatedList` and the real table. */ +function renderBlock(schema: Record, schemaExtra?: Record) { + return render( + + + , + ); +} + +/** + * Wait until `text` is on screen as a body CELL. + * + * Every "the redacted value is absent" read below is taken only after one of + * these resolves, and that ordering is the whole guard against a false green: + * the derived column set cannot exist before the object schema lands + * (`if (!objectSchema?.fields) return []`), so a table with any cell at all is + * a table whose columns are final. Waiting on a POSITIVE cell is therefore the + * cheapest available proof that the negative was measured on the settled DOM + * rather than on the render before it. + */ +const waitForCell = (text: string) => + waitFor(() => expect(cellTexts()).toContain(text)); + +describe('objectui#9053 — a redacted field must not come back through auto-derivation', () => { + it('MAXIMAL — redacting the ONLY authored column does not resurrect it', async () => { + // The card's measurement verbatim: one authored column, and it is redacted. + renderBlock({ + columns: [{ field: 'salary', label: 'Salary' }], + redactFields: ['salary'], + }); + await waitForCell('Fix the pump'); + + // Before the fix this read `[ 'Fix the pump', '90000' ]`. + expect(cellTexts()).not.toContain('90000'); + expect(screen.queryByText('Salary')).not.toBeInTheDocument(); + // Non-vacuity: the list really did render, with the field that was NOT + // redacted — so "90000 is absent" cannot be satisfied by an empty table. + expect(cellTexts()).toContain('Fix the pump'); + }); + + it('CONTROL (partial) — redacting SOME authored columns leaves the rest alone', async () => { + // The array stays non-empty, so the AUTHORED path is taken. This is the + // case redaction already handled, and the fix must not disturb it. + renderBlock({ + columns: [ + { field: 'subject', label: 'Subject' }, + { field: 'salary', label: 'Salary' }, + ], + redactFields: ['salary'], + }); + await waitForCell('Fix the pump'); + + expect(cellTexts()).not.toContain('90000'); + expect(screen.queryByText('Salary')).not.toBeInTheDocument(); + }); + + it('CONTROL (no redaction) — the same column renders when nothing is redacted', async () => { + // The positive control for every negative above: without `redactFields` + // the value IS on screen, so their absence measures redaction and not a + // broken fixture. + renderBlock({ columns: [{ field: 'salary', label: 'Salary' }] }); + await waitForCell('90000'); + + expect(screen.getByText('Salary')).toBeInTheDocument(); + }); + + it('DERIVED (heuristic walk) — a redacted field never enters the auto-derived set', async () => { + // No authored columns at all: the walk is the ONLY path, and the block's + // redaction list has to reach it for the key to mean anything here. + renderBlock({ redactFields: ['salary'] }); + await waitForCell('Fix the pump'); + + expect(cellTexts()).not.toContain('90000'); + }); + + it('DERIVED (highlightFields) — a redacted highlight field never leads the list', async () => { + // ADR-0085 prominence path. With its only member redacted the branch must + // yield nothing and fall through to the walk — which is itself filtered. + renderBlock({ redactFields: ['salary'] }, { highlightFields: ['salary'] }); + await waitForCell('Fix the pump'); + + expect(cellTexts()).not.toContain('90000'); + }); + + it('CONTROL (highlightFields, no redaction) — the prominence path still leads with it', async () => { + renderBlock({}, { highlightFields: ['salary'] }); + await waitForCell('90000'); + }); + + it('the policy holds on `RelatedList` itself, for both authored and derived columns', async () => { + // `RelatedList` is exported from this package's public entry, so the prop + // has to mean the same thing to a direct consumer: a named column does not + // render, whichever path produced it. A prop that only bound on the derived + // path would be a fresh instance of the defect this card is about. + const { rerender } = render( + , + ); + await waitForCell('Fix the pump'); + expect(cellTexts()).not.toContain('90000'); + + rerender( + , + ); + await waitForCell('Fix the pump'); + expect(cellTexts()).not.toContain('90000'); + }); +}); + +/** + * The grade's load-bearing measurement, asserted rather than inherited: FLS is + * re-applied on the derived path, so this card is a lost AUTHORING preference + * and not a permission bypass. If this case ever fails, objectui#9053's p2 + * grade and the shape of its remedy are both wrong. + */ +describe('objectui#9053 — the derived path re-applies field-level security', () => { + const roles: RoleDefinition[] = [{ name: 'restricted', label: 'Restricted' }]; + const permissions: ObjectPermissionConfig[] = [ + { + object: 'task', + roles: { + restricted: { + actions: ['read'], + fieldPermissions: [{ field: 'salary', read: false }], + }, + }, + }, + ]; + + it('an FLS-denied field stays out of the auto-derived set (no redactFields involved)', async () => { + render( + + + + + , + ); + await waitForCell('Fix the pump'); + + expect(cellTexts()).not.toContain('90000'); + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-related-list.tsx b/packages/plugin-detail/src/renderers/record-related-list.tsx index 70d3e5761d..7ea2f6df88 100644 --- a/packages/plugin-detail/src/renderers/record-related-list.tsx +++ b/packages/plugin-detail/src/renderers/record-related-list.tsx @@ -203,6 +203,15 @@ const RecordRelatedListBody: React.FC = ({ referenceField={schema.relationshipField} parentId={parentLinkValue as any} columns={filteredColumns as any} + // [objectui#9053] The same list, pushed down to the component that + // DECIDES columns. Filtering the authored array here only ever reached + // one of the three paths that decide them: redacting every authored + // column emptied this array, `RelatedList` read the empty array as "no + // columns were authored", and its auto-derivation — which this list + // never reached — brought the redacted field back. Passed by reference + // (and `undefined` when unauthored) so the column memo downstream keeps + // a stable dependency. + redactFields={redact.length > 0 ? redact : undefined} pageSize={ typeof schema.limit === 'number' && schema.limit > 0 ? schema.limit