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