fix(utils): Remove dottie in favor of undot. update undot with security enhancements.#18137
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughReplaced use of the external Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR removes the dottie dependency from @sequelize/core by switching nested-path read/write behavior to the internal undot utilities, and adds prototype-pollution protections to undot to block dangerous key segments.
Changes:
- Add prototype-pollution guards to
tokenizePath,precompileKeys,setByPathArray, and introducegetByPathArrayinundot. - Update
Model#setJSON nested-field handling to useundotinstead ofdottie. - Remove
dottiefrompackages/coredependencies and add unit tests for the new protections.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/utils/undot.ts | Adds dangerous-segment detection, new getByPathArray, and guards in path tokenization + setter. |
| packages/core/src/model.js | Replaces Dottie.get/set with getByPathArray / setByPathArray + tokenizePath for JSON dotted keys. |
| packages/core/test/unit/utils/undot.test.ts | Adds unit tests targeting prototype-pollution prevention across undot helpers. |
| packages/core/package.json | Removes the dottie dependency from core. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/model.js (1)
3677-3681: Repeated tokenization of the same key.
tokenizePath(key)is called twice for the samekeyvalue. While not a significant performance issue in practice (this code path handles JSON attribute updates which are relatively infrequent), you could cache the result if this becomes a hot path.Optional: Cache tokenized path
if (key.includes('.') && jsonAttributeNames.has(key.split('.')[0])) { - const previousNestedValue = getByPathArray(this.dataValues, tokenizePath(key)); + const path = tokenizePath(key); + const previousNestedValue = getByPathArray(this.dataValues, path); if (!isEqual(previousNestedValue, value)) { - setByPathArray(this.dataValues, tokenizePath(key), value); + setByPathArray(this.dataValues, path, value); this.changed(key.split('.')[0], true); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/model.js` around lines 3677 - 3681, tokenizePath(key) is being called twice for the same key in the JSON update logic (used with getByPathArray and setByPathArray); cache its result in a local variable (e.g., const path = tokenizePath(key)) and reuse that variable when calling getByPathArray(this.dataValues, path) and setByPathArray(this.dataValues, path), leaving the this.changed(key.split('.')[0], true) call unchanged.
🤖 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/core/test/unit/utils/undot.test.ts`:
- Around line 621-629: The test name and behavior are mismatched: update the
spec for transformRowWithPrecompiled / precompileKeys to either reflect that
this is a normal dotted-key transformation (e.g., "should transform normal
dotted keys without pollution") or add a new test that actually exercises
blocking of "__proto__" (call precompileKeys with a key containing "__proto__"
like "user.__proto__.pollute" or a top-level "__proto__" dotted key, run
transformRowWithPrecompiled on a row that would attempt prototype pollution, and
assert the prototype was not polluted). Ensure the new or renamed test
references the same helpers (precompileKeys and transformRowWithPrecompiled) and
keeps the existing pollution assertion style.
---
Nitpick comments:
In `@packages/core/src/model.js`:
- Around line 3677-3681: tokenizePath(key) is being called twice for the same
key in the JSON update logic (used with getByPathArray and setByPathArray);
cache its result in a local variable (e.g., const path = tokenizePath(key)) and
reuse that variable when calling getByPathArray(this.dataValues, path) and
setByPathArray(this.dataValues, path), leaving the
this.changed(key.split('.')[0], true) call unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/package.jsonpackages/core/src/model.jspackages/core/src/utils/undot.tspackages/core/test/unit/utils/undot.test.ts
💤 Files with no reviewable changes (1)
- packages/core/package.json
- Replace external dottie dependency with internal undot utility for dot-path object access in model.js - Add getByPathArray to undot.ts for nested value retrieval - Guard against prototype pollution (__proto__, constructor, prototype) at all path positions in tokenizePath, precompileKeys, setByPathArray, and getByPathArray (CVE-2026-27837) - Add comprehensive prototype pollution prevention tests - Remove dottie from package.json dependencies
This pull request adds important security enhancement to the core package
undotby replacing thedottiedependency withundotfor handling nested object paths, and by implementing the same protections found in the newdottiebuild against prototype pollution attacks. The changes affect both the implementation and testing of nested path access and mutation logic, ensuring that dangerous object keys cannot be used to manipulate object prototypes.Dottie fixes: https://github.com/mickhansen/dottie.js/pull/43/changes
Pull Request Checklist
Description of Changes
###Security improvements
dottiepackage inmodel.jswith custom functionsgetByPathArray,setByPathArray, andtokenizePathfromutils/undot.js, eliminating an external dependency and allowing for tighter control over path parsing and setting. [1] [2] [3]tokenizePath,getByPathArray, andsetByPathArrayto detect and block dangerous key segments (__proto__,constructor,prototype) at any position, preventing prototype pollution vulnerabilities. [1] [2] [3] [4] [5]###Testing and verification
undot.test.tsto verify that prototype pollution attempts are blocked and that normal nested paths still work as expected. These tests cover various edge cases and ensure that the new logic is robust. [1] [2]These changes add the security changes of dottie to undot by preventing known attack vectors while maintaining full functionality for safe keys. Then removes the dottie dependency in favor of undot.
List of Breaking Changes
Summary by CodeRabbit
Bug Fixes
Refactor
Tests