Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-grid): guard useRowColor's object-literal lookups with hasOwnProperty - #6381
Merged
Merged
Conversation
…OwnProperty `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`.
…color-prototype-guard
Contributor
✅ 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
|
os-support-ai
marked this pull request as ready for review
August 25, 2026 18:58
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#6295
useRowColorreached two plain object literals with a bare index, and both inheritObject.prototype. Both reads are now guarded withObject.prototype.hasOwnProperty.call, mirroring the guard #6178 / PR #6294 applied to the analogous lookup inpackages/plugin-detail/src/headerColor.ts.The two halves
config.colors[value](:67) — the loud half. Indexed with record data. A record whose colour field heldconstructor/toString/valueOf/hasOwnPropertyresolved to an inherited function;if (!color)passed it (functions are truthy) andcolorToClassthen called.startsWithon it —TypeError: color.startsWith is not a function, thrown inside the row-className resolver during render. A grid crash triggered by data rather than by metadata. The authored map does not have to mention the value: an emptycolors: {}crashes just the same, which this PR pins.COLOR_TO_CLASS[lower](:52) — the quiet half. Same shape one call deeper, reached with the authored colour value. It never threw; it returned the inherited member, so a function left the resolver as the row'sclassNameand reached React as a class attribute. Fixing only the thrower would have left this.Both now resolve to
undefined, exactly as an undeclared value always did.Object.hasOwnis ES2022 and is not available here — verified rather than assumed:tsconfig.json(whichpackages/plugin-grid/tsconfig.jsonextends without overriding) sets"lib": ["ES2020", "DOM", "DOM.Iterable"]. Worth noting for the next reader:packages/plugin-grid/tsconfig.test.jsondeliberately sits one notch above atES2022, soObject.hasOwnwould have compiled in a test while failing in shipped source.Ghost-assertion guard — both readings
New assertions run against unmodified
origin/main(fix not yet applied), then against the fix. Same file, same command.Before — 13 failed | 3 passed (16):
TypeError: color.startsWith is not a functionappears 4× in that run. All 13 failures are in the two defect describes; the 3 that passed are the control below.After — 16 passed (16):
Degenerate control
The control fixture is
CONTROL_CONFIG—{ field: 'status', colors: { open: 'green', closed: 'red', urgent: 'bg-red-200' } }— inuseRowColor — ordinary lookups still work (degenerate control). Its three tests (open→bg-green-100,closed→bg-red-100, thebg-*pass-through, and an undeclared value →undefined) are the 3 that were green both before and after, so the guard is shown not to have switched ordinary lookups off.One deliberate asymmetry, called out so nobody "helpfully" widens it into ghost assertions: the quiet half asserts only
constructorand__proto__, not the full eight.colorToClasslower-cases before indexing, sotoStringarrives astostring,valueOfasvalueof, and so on — none of which is an inherited member. Measured on unmodifiedmain, those six returnundefinedthere too, so asserting them would assert nothing. Only the two already-lower-case names actually reachObject.prototype. The loud half indexes the record value verbatim, so all eight apply there and all eight are pinned.Verification
All readings below are from the final commit,
e9f263574(after mergingorigin/main).pnpm exec vitest run packages/plugin-grid/Test Files 91 passed (91)/Tests 853 passed (853), exit 0pnpm --filter @object-ui/plugin-grid type-checktsc --noEmit && tsc -p tsconfig.test.json)pnpm --filter @object-ui/plugin-grid lint✖ 691 problems (0 errors, 691 warnings), exit 0node scripts/check-changeset-presence.mjs✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)Lint delta measured, not assumed.
useRowColor.tscarries two warnings (react-hooks/preserve-manual-memoization,@typescript-eslint/no-explicit-any). Both are pre-existing: eslint--format jsonon the file atorigin/mainand at this branch returns identical per-rule counts ({preserve-manual-memoization: 1, no-explicit-any: 1},errorCount=0both). This change introduces no new lint finding.Typecheck coverage proven, not inferred.
packages/plugin-grid/tsconfig.jsonexcludes**/__tests__/**, so "typecheck clean" could have been true while saying nothing about the new test.tsc --listFilesconfirms both changed files are real program inputs —useRowColor.prototypeGuard.test.tsxin thetsconfig.test.jsonprogram,useRowColor.tsin the build program.Declared narrowing. Local verification is scoped to
@object-ui/plugin-grid(its full suite, its typecheck, its lint) plus the repo-wide changeset-presence check — not a repo-widepnpm lint/pnpm test. CI runs the full farm regardless; this is a declared narrowing, not a skipped check.Scope
packages/plugin-grid/src/useRowColor.ts(the two index reads only), one new test file, one changeset. The colour-resolution flow, the authoredcolorsmap shape, andplugin-detail's already-fixed sibling are untouched. No edits tocontent/docs/releases/.Draft, and staying draft — the PM lands it.
Generated by Claude Code