Guard object lookups against inherited prototype properties#19725
Guard object lookups against inherited prototype properties#19725RobinMalfait merged 4 commits intomainfrom
Conversation
When user-controlled candidate values like "constructor" are used as keys to look up values in plain objects (staticValues, plugin values, modifiers, config), they can match inherited Object.prototype properties instead of returning undefined. This caused crashes like "V.map is not a function" when scanning source files containing strings like "row-constructor". Use Object.hasOwn() checks before all user-keyed object lookups in: - utilities.ts (staticValues lookup) - plugin-api.ts (values, modifiers, and variant values lookups) - plugin-functions.ts (get() config traversal function) Fixes #19721 https://claude.ai/code/session_011CYSGw3DLh2Z8xnuyoaCgC
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughReplaces fragile property lookups with explicit own-property checks (Object.hasOwn) and adds null/undefined/non-object guards during path traversal to avoid indexing into non-object values. Converts certain static value maps to null-prototype objects before lookup to prevent inherited properties from Object.prototype. Adds tests for plugin and utility APIs using names matching Object.prototype keys (e.g. constructor, hasOwnProperty, toString, valueOf, __proto__) that assert no CSS output or errors. No public API signatures were changed. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/tailwindcss/src/utilities.test.ts (1)
1650-1655: Consider adding__proto__(and related prototype members) to the test cases.The four cases added here are the right set for the reported issue, and the structure is correct. However,
__proto__is a special accessor onObject.prototypethat is equally dangerous as a keyed lookup — it's absent from the coverage. TheObject.hasOwn()guard introduced inutilities.tscorrectly returnsfalsefor it, but having an explicit test would prevent regressions if the guard were ever relaxed or refactored.🧪 Proposed additional test candidates
'row-constructor', 'row-hasOwnProperty', 'row-toString', 'row-valueOf', + 'row-__proto__', + 'row-isPrototypeOf', + 'row-propertyIsEnumerable',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tailwindcss/src/utilities.test.ts` around lines 1650 - 1655, Add an explicit test candidate for "__proto__" alongside the existing entries ('row-constructor', 'row-hasOwnProperty', 'row-toString', 'row-valueOf') in the candidates list in utilities.test.ts so the Object.hasOwn() guard in utilities.ts is exercised for the special accessor; update the test array to include "__proto__" (and any related prototype accessor names you want covered) to ensure the guard continues to return false and prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/tailwindcss/src/compat/plugin-api.ts`:
- Around line 426-427: The call to values.__BARE_VALUE__ is not type-safe
because Object.hasOwn(values, '__BARE_VALUE__') does not narrow to a callable;
guard the invocation by checking the property is a function before calling it
(e.g. if (typeof values.__BARE_VALUE__ === 'function') { value =
values.__BARE_VALUE__(candidate.value) ?? null }) so TypeScript recognizes the
callable; reference symbols: __BARE_VALUE__, values, candidate.value, value,
Object.hasOwn.
---
Nitpick comments:
In `@packages/tailwindcss/src/utilities.test.ts`:
- Around line 1650-1655: Add an explicit test candidate for "__proto__"
alongside the existing entries ('row-constructor', 'row-hasOwnProperty',
'row-toString', 'row-valueOf') in the candidates list in utilities.test.ts so
the Object.hasOwn() guard in utilities.ts is exercised for the special accessor;
update the test array to include "__proto__" (and any related prototype accessor
names you want covered) to ensure the guard continues to return false and
prevent regressions.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/tailwindcss/src/compat/plugin-api.test.tspackages/tailwindcss/src/compat/plugin-api.tspackages/tailwindcss/src/compat/plugin-functions.tspackages/tailwindcss/src/utilities.test.tspackages/tailwindcss/src/utilities.ts
Otherwise the `Object.hasOwn` check runs for every utility where static values have been defined and uses bare values (e.g. `row-constructor`) but in most cases you won't hit prototype specific keys. This takes it out of the hot path
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/tailwindcss/src/compat/plugin-api.ts (1)
418-430:values.DEFAULTat line 415 is the one remaining direct access withouthasOwn.All user-controlled key paths now use
Object.hasOwn, but the!candidate.valuebranch still doesvalues.DEFAULT ?? nullvia plain property access. While'DEFAULT'is not anObject.prototypekey and the lookup is not driven by user input, adding ahasOwnguard would make this block fully consistent with the rest of the change.♻️ Optional consistency fix
- if (!candidate.value) { - value = values.DEFAULT ?? null - } else if (candidate.value.kind === 'arbitrary') { + if (!candidate.value) { + value = Object.hasOwn(values, 'DEFAULT') ? values.DEFAULT : null + } else if (candidate.value.kind === 'arbitrary') {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tailwindcss/src/compat/plugin-api.ts` around lines 418 - 430, The branch that handles the !candidate.value case currently reads values.DEFAULT via direct property access; change it to first check Object.hasOwn(values, 'DEFAULT') and only then set value = values.DEFAULT ?? null (otherwise fall back to null), keeping any existing ignoreModifier logic intact; update the code around the candidate.value / values lookup (the same area using Object.hasOwn for fraction and value keys) to use Object.hasOwn for 'DEFAULT' for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/tailwindcss/src/compat/plugin-api.ts`:
- Around line 418-430: The branch that handles the !candidate.value case
currently reads values.DEFAULT via direct property access; change it to first
check Object.hasOwn(values, 'DEFAULT') and only then set value = values.DEFAULT
?? null (otherwise fall back to null), keeping any existing ignoreModifier logic
intact; update the code around the candidate.value / values lookup (the same
area using Object.hasOwn for fraction and value keys) to use Object.hasOwn for
'DEFAULT' for consistency.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/tailwindcss/src/compat/plugin-api.tspackages/tailwindcss/src/utilities.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/tailwindcss/src/utilities.ts
RobinMalfait
left a comment
RobinMalfait
left a comment
There was a problem hiding this comment.
Made a few small changes, but looks good otherwise.
3 participants
When user-controlled candidate values like "constructor" are used as
keys to look up values in plain objects (staticValues, plugin values,
modifiers, config), they can match inherited Object.prototype properties
instead of returning undefined. This caused crashes like "V.map is not
a function" when scanning source files containing strings like
"row-constructor".
Use Object.hasOwn() checks before all user-keyed object lookups in:
Fixes #19721
https://claude.ai/code/session_011CYSGw3DLh2Z8xnuyoaCgC