Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/rowcolor-prototype-guard-6295.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
@@ -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');
}
});
});
22 changes: 20 additions & 2 deletions packages/plugin-grid/src/useRowColor.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
}

/**
Expand All@@ -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);
Expand Down
Loading