From 4aa5426acc53cef3ce09c689a3d803456238e132 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 18:37:46 +0000 Subject: [PATCH] fix(plugin-grid): guard useRowColor's object-literal lookups with hasOwnProperty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `useRowColor` reached two plain object literals with a bare index, and both inherit `Object.prototype`. `config.colors[value]` is indexed with RECORD DATA, so a record whose colour field held `constructor`, `toString`, `valueOf` or `hasOwnProperty` resolved to an inherited function. `if (!color)` passed it (functions are truthy) and `colorToClass` then called `.startsWith` on it — a TypeError thrown inside the row-className resolver during render, crashing the grid on data rather than on metadata. `COLOR_TO_CLASS[lower]` one call deeper has the same shape and is the quieter half: it never threw, it handed an `Object.prototype` member back as the row's `className`, which reached React as a class attribute. Both reads now go through `Object.prototype.hasOwnProperty.call` and resolve to `undefined`, exactly as an undeclared value always did. (`Object.hasOwn` is ES2022; this workspace compiles against the ES2020 lib.) Mirrors the guard objectui#6178 / PR objectui#6294 applied to the analogous lookup in `packages/plugin-detail/src/headerColor.ts`. --- .changeset/rowcolor-prototype-guard-6295.md | 15 ++ .../useRowColor.prototypeGuard.test.tsx | 139 ++++++++++++++++++ packages/plugin-grid/src/useRowColor.ts | 22 ++- 3 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 .changeset/rowcolor-prototype-guard-6295.md create mode 100644 packages/plugin-grid/src/__tests__/useRowColor.prototypeGuard.test.tsx diff --git a/.changeset/rowcolor-prototype-guard-6295.md b/.changeset/rowcolor-prototype-guard-6295.md new file mode 100644 index 0000000000..de76be8132 --- /dev/null +++ b/.changeset/rowcolor-prototype-guard-6295.md @@ -0,0 +1,15 @@ +--- +'@object-ui/plugin-grid': patch +--- + +Guard `useRowColor`'s two object-literal lookups with `Object.prototype.hasOwnProperty.call`. + +Both `config.colors` (the authored map) and the module's `COLOR_TO_CLASS` literal inherit +`Object.prototype`, and both were reached with a bare index. A record whose colour field +held `constructor`, `toString`, `valueOf` or `hasOwnProperty` resolved to an inherited +function: the `if (!color)` guard passed it (functions are truthy) and `colorToClass` then +called `.startsWith` on it, throwing a `TypeError` inside the row-className resolver during +render — a grid crash triggered by record data rather than by metadata. The same shape one +call deeper in `colorToClass` did not throw; it handed an `Object.prototype` member back as +the row's `className`, which reached React as a class attribute. Both now resolve to +`undefined`, as an undeclared value always did. diff --git a/packages/plugin-grid/src/__tests__/useRowColor.prototypeGuard.test.tsx b/packages/plugin-grid/src/__tests__/useRowColor.prototypeGuard.test.tsx new file mode 100644 index 0000000000..fe497d8484 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/useRowColor.prototypeGuard.test.tsx @@ -0,0 +1,139 @@ +/** + * 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. + */ + +import { describe, it, expect } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import type { RowColorConfig } from '@object-ui/types'; +import { useRowColor } from '../useRowColor'; + +/** + * objectui#6295 — `useRowColor` reached two plain object literals with a bare + * index, and both inherit `Object.prototype`. The two halves fail differently, + * so they are pinned separately below: + * + * :67 `config.colors[value]`, where `value` comes from RECORD DATA. An + * inherited member is a function, `if (!color)` passes it (truthy), and + * `colorToClass` calls `.startsWith` on it — a TypeError thrown inside + * the row-className resolver during render. LOUD: the grid crashes, and + * the trigger is data, not metadata. + * + * :52 `COLOR_TO_CLASS[lower]`, one call deeper, where `lower` comes from the + * AUTHORED colour value. This one never threw: it returned the inherited + * member, so a function left the resolver as the row's `className` and + * reached React as a class attribute. QUIET, and the same defect. + * + * Shape mirrors `packages/plugin-detail/src/headerColor.ts` (objectui#6178 / + * PR objectui#6294), which guarded the analogous lookup one package over. + */ + +/** Render the hook and hand back the row-className resolver it produces. */ +function resolverFor(config: RowColorConfig) { + return renderHook(() => useRowColor(config)).result.current; +} + +/** + * The names every plain object literal inherits from `Object.prototype`. + * + * All of them reach the `:67` read, which indexes the authored map with the + * record value VERBATIM. + */ +const INHERITED_MEMBERS: string[] = [ + 'constructor', + 'toString', + 'valueOf', + 'hasOwnProperty', + 'isPrototypeOf', + 'propertyIsEnumerable', + 'toLocaleString', + '__proto__', +]; + +/** + * The subset that reaches the `:52` read — and the reason this list is SHORTER + * than the one above, which is easy to "helpfully" expand back into ghost + * assertions. + * + * `colorToClass` lower-cases before indexing (`color.toLowerCase().trim()`), so + * `toString` arrives as `tostring`, `valueOf` as `valueof`, and so on — none of + * which is an inherited member. Only the two names that are ALREADY lower-case + * survive the transform and actually hit `Object.prototype`. Measured against + * unmodified `main`: the six camelCase names return `undefined` there too, so + * asserting them here would assert nothing. + */ +const LOWERCASE_INHERITED_MEMBERS: string[] = ['constructor', '__proto__']; + +/** + * The degenerate control. `open` -> `bg-green-100` is the fixture that proves + * the guard did not simply switch ordinary lookups off; it is asserted first, + * and it is green both before and after the fix. + */ +const CONTROL_CONFIG: RowColorConfig = { + field: 'status', + colors: { open: 'green', closed: 'red', urgent: 'bg-red-200' }, +}; + +describe('useRowColor — ordinary lookups still work (degenerate control)', () => { + it('resolves an authored colour name to its Tailwind class', () => { + const resolve = resolverFor(CONTROL_CONFIG); + expect(resolve({ status: 'open' })).toBe('bg-green-100'); + expect(resolve({ status: 'closed' })).toBe('bg-red-100'); + }); + + it('passes a value that is already a bg-* class through untouched', () => { + expect(resolverFor(CONTROL_CONFIG)({ status: 'urgent' })).toBe('bg-red-200'); + }); + + it('returns undefined for a value the author did not declare', () => { + expect(resolverFor(CONTROL_CONFIG)({ status: 'archived' })).toBeUndefined(); + }); +}); + +describe('useRowColor — record data naming an Object.prototype member (:67, the loud half)', () => { + it('does not throw when a record colour field holds `constructor`', () => { + // The exact chain the card reproduced. Before the fix this threw + // `TypeError: color.startsWith is not a function`. + const resolve = resolverFor(CONTROL_CONFIG); + expect(() => resolve({ status: 'constructor' })).not.toThrow(); + expect(resolve({ status: 'constructor' })).toBeUndefined(); + }); + + it.each(INHERITED_MEMBERS)('resolves to undefined for a record value of %s', (member) => { + expect(resolverFor(CONTROL_CONFIG)({ status: member })).toBeUndefined(); + }); + + it('is inert even when the author declared no colours at all', () => { + // The authored map does not have to mention the value for the crash: the + // inherited member is found whether or not anything was declared. + const resolve = resolverFor({ field: 'status', colors: {} }); + for (const member of INHERITED_MEMBERS) { + expect(() => resolve({ status: member })).not.toThrow(); + expect(resolve({ status: member })).toBeUndefined(); + } + }); +}); + +describe('useRowColor — COLOR_TO_CLASS cannot emit an inherited member (:52, the quiet half)', () => { + it.each(LOWERCASE_INHERITED_MEMBERS)( + 'returns undefined when the authored colour value is %s', + (member) => { + const resolve = resolverFor({ field: 'status', colors: { open: member } }); + expect(resolve({ status: 'open' })).toBeUndefined(); + }, + ); + + it('never hands back a non-string as the row className', () => { + // The property that matters at the call site: `getRowClassName(row)` feeds + // `rowClassName` in ObjectGrid, so anything but a string or undefined is a + // class attribute React should never have been handed. + for (const member of LOWERCASE_INHERITED_MEMBERS) { + const cls = resolverFor({ field: 'status', colors: { open: member } })({ status: 'open' }); + expect(cls === undefined || typeof cls === 'string').toBe(true); + expect(typeof cls).not.toBe('function'); + } + }); +}); diff --git a/packages/plugin-grid/src/useRowColor.ts b/packages/plugin-grid/src/useRowColor.ts index 6c50fcf527..5c0cfcfd98 100644 --- a/packages/plugin-grid/src/useRowColor.ts +++ b/packages/plugin-grid/src/useRowColor.ts @@ -49,7 +49,15 @@ function colorToClass(color: string): string | undefined { if (color.startsWith('bg-')) return color; const lower = color.toLowerCase().trim(); - return COLOR_TO_CLASS[lower]; + // `hasOwnProperty` rather than a bare index (objectui#6295, the quiet half): + // this object literal inherits `constructor`, `toString` and friends, so an + // authored colour value naming one of them used to hand the inherited member + // back as the row's className. That never threw — it reached React as a class + // attribute. (`Object.hasOwn` is ES2022; this workspace compiles against the + // ES2020 lib.) + return Object.prototype.hasOwnProperty.call(COLOR_TO_CLASS, lower) + ? COLOR_TO_CLASS[lower] + : undefined; } /** @@ -64,7 +72,17 @@ export function useRowColor(config: RowColorConfig | undefined) { if (!config?.field || !config.colors) return undefined; const value = String(row[config.field] ?? ''); - const color = config.colors[value]; + // The same guard one call out, and this read is reached with RECORD DATA + // (objectui#6295, the loud half): the authored map inherits + // `Object.prototype`, so a record whose colour field holds `constructor` / + // `toString` / `valueOf` / `hasOwnProperty` resolved to an inherited + // FUNCTION. `if (!color)` below passes it (functions are truthy) and + // `colorToClass` then called `.startsWith` on it — a TypeError thrown + // inside the row-className resolver during render, crashing the grid on + // data rather than on metadata. + const color = Object.prototype.hasOwnProperty.call(config.colors, value) + ? config.colors[value] + : undefined; if (!color) return undefined; return colorToClass(color);