fix: prevent prototype pollution in setByDotNotation #2952
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds path segment validation to setByDotNotation to block prototype-polluting keys and introduces a regression test ensuring Object.prototype cannot be polluted via dot-notation paths. Flow diagram for updated setByDotNotation path validationflowchart TD
A[start setByDotNotation
obj, path, value, merge] --> B[check path is falsy or length is 0]
B -->|yes| C[return undefined]
B -->|no| D[check path contains unsafe keys
__proto__, constructor, prototype]
D -->|yes| E[return undefined]
D -->|no| F{is path a string}
F -->|yes| G[recurse setByDotNotation
obj, path split by '.', value, merge]
F -->|no| H[normal traversal and assignment
using array path segments]
G --> I[end]
H --> I[end]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @Bitshifter-9, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a critical security enhancement by patching a Prototype Pollution vulnerability in the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded a prototype-pollution guard to setByDotNotation that blocks path segments Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The new
path.includes('__proto__' | 'constructor' | 'prototype')check on string paths will also block legitimate keys that merely contain these substrings (e.g.'my_constructor_field'); consider parsing string paths into segments and only rejecting when a segment exactly matches one of the dangerous keys. - The guard currently treats string and array paths identically, but array paths are already segmented; for better precision and future extensibility, consider checking each array segment for exact equality with the dangerous keys rather than using
includeson the whole path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `path.includes('__proto__' | 'constructor' | 'prototype')` check on string paths will also block legitimate keys that merely contain these substrings (e.g. `'my_constructor_field'`); consider parsing string paths into segments and only rejecting when a segment exactly matches one of the dangerous keys.
- The guard currently treats string and array paths identically, but array paths are already segmented; for better precision and future extensibility, consider checking each array segment for exact equality with the dangerous keys rather than using `includes` on the whole path.
## Individual Comments
### Comment 1
<location> `packages/altair-core/src/utils/dot-notation.ts:31-29` </location>
<code_context>
if (!path || path.length === 0) {
return undefined;
}
+ if (path.includes('__proto__') || path.includes('constructor') || path.includes('prototype')) {
+ return undefined;
+ }
if (typeof path === 'string') {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Prototype-pollution guard may block legitimate keys due to substring matching.
Because `includes` runs on the entire `path` value, it will also block keys that only contain these words as substrings, e.g. `user.constructorId` or `['user', 'myPrototypeFlag']`. Instead, normalize to discrete path segments (e.g. split the string when `typeof path === 'string'`) and check segment equality (`segment === '__proto__'`, `'constructor'`, `'prototype'`) to avoid false positives while keeping the protection in place.
Suggested implementation:
```typescript
if (!path || path.length === 0) {
return undefined;
}
if (typeof path === 'string') {
const pathSegments = path.split('.');
if (
pathSegments.some(
(segment) =>
segment === '__proto__' ||
segment === 'constructor' ||
segment === 'prototype',
)
) {
return undefined;
}
return setByDotNotation(
obj,
```
To fully harden this function against prototype pollution while following the same “segment equality” rule for non-string paths, you should also:
1. Locate the code path that handles array/object-style paths after the shown `if (typeof path === 'string') { ... }` branch.
2. Add an analogous guard for non-string paths, e.g.:
```ts
const pathSegments = Array.isArray(path) ? path : [path];
if (
pathSegments.some(
(segment) =>
segment === '__proto__' ||
segment === 'constructor' ||
segment === 'prototype',
)
) {
return undefined;
}
```
Place this before any mutation logic for non-string paths. This ensures `['user', 'constructorId']` is allowed, but paths containing a segment strictly equal to `'constructor'`, `'__proto__'`, or `'prototype'` are blocked.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a critical prototype pollution vulnerability in the setByDotNotation utility. The approach of blocking paths containing __proto__, constructor, or prototype is correct. However, the current implementation has a flaw where it performs a substring match on string paths, which can block valid paths (e.g., a property named 'myconstructor'). I've left a suggestion to correct this behavior. Additionally, I've recommended expanding the test suite to cover more attack vectors and to verify that the fix doesn't introduce regressions for valid use cases. Overall, a good security fix with minor adjustments needed.
| if (path.includes('__proto__') || path.includes('constructor') || path.includes('prototype')) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
The current implementation to prevent prototype pollution is a bit too broad. When path is a string, path.includes() performs a substring search. This means a valid path like 'thisisaconstructorstring' would be incorrectly blocked because it contains the substring "constructor". The check should only validate individual path segments against the blocklist. To fix this, you can ensure this check only runs when path is an array of segments.
| if (path.includes('__proto__') || path.includes('constructor') || path.includes('prototype')) { | |
| return undefined; | |
| } | |
| if (Array.isArray(path) && (path.includes('__proto__') || path.includes('constructor') || path.includes('prototype'))) { | |
| return undefined; | |
| } |
| // prevent prototype pollution | ||
| const polluted: any = {}; | ||
| setByDotNotation(polluted, '__proto__.polluted', 'polluted'); | ||
| expect((polluted as any).polluted).toBeUndefined(); | ||
| expect(({} as any).polluted).toBeUndefined(); |
There was a problem hiding this comment.
This test is a great start for verifying the prototype pollution fix. To make it more comprehensive, I suggest expanding it to cover all the blocked keywords (constructor, prototype) and also to ensure that valid property names that happen to contain these keywords as substrings are not incorrectly blocked.
// prevent prototype pollution
const victim: any = {};
setByDotNotation(victim, '__proto__.polluted', 'yes');
expect((victim as any).polluted).toBeUndefined();
expect(({} as any).polluted).toBeUndefined();
setByDotNotation(victim, 'constructor.prototype.polluted', 'yes');
expect((victim as any).polluted).toBeUndefined();
expect(({} as any).polluted).toBeUndefined();
// ensure valid paths with substrings are not blocked
const validObj: any = {};
setByDotNotation(validObj, 'myconstructor', 'is-ok');
expect(validObj.myconstructor).toBe('is-ok');![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/altair-core/src/utils/dot-notation.spec.ts (1)
45-49: Tighten test assertions and consider broader coverage for the new guard
The core
__proto__test looks good and validates the fix.Minor cleanup:
pollutedis already typed asany, so the assertion cast is redundant:- expect((polluted as any).polluted).toBeUndefined(); + expect(polluted.polluted).toBeUndefined();
- To fully exercise the new guard, consider adding tests that:
- Use paths containing
constructorandprototype, e.g.:
setByDotNotation(polluted, 'constructor.prototype.polluted', 'polluted')setByDotNotation(polluted, 'a.constructor.prototype.polluted', 'polluted')- Use array‑based paths, e.g.:
setByDotNotation(polluted, ['__proto__', 'polluted'], 'polluted')setByDotNotation(polluted, ['constructor', 'prototype', 'polluted'], 'polluted')and assert both the target object and
({} as any).pollutedremainundefined. This will better guard against regressions in the sharedpackages/altair-coreutility. Based on learnings, ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/altair-core/src/utils/dot-notation.spec.ts(1 hunks)packages/altair-core/src/utils/dot-notation.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
{**/__tests__/**/*.ts,**/*.{spec,test}.ts}
📄 CodeRabbit inference engine (.github/instructions/app-testing.instructions.md)
{**/__tests__/**/*.ts,**/*.{spec,test}.ts}: Use Jest as the testing framework for all tests
Organize tests next to the code under test: use a tests folder or .test.ts/.spec.ts files alongside sources
Use clear, descriptive test names explaining what is being verified
Mock dependencies with Jest to isolate the unit under test
Leverage TypeScript types in tests; define interfaces/types for expected data shapes
Files:
packages/altair-core/src/utils/dot-notation.spec.ts
**/*.ts
📄 CodeRabbit inference engine (.github/instructions/app-testing.instructions.md)
Follow project code style using ESLint and Prettier
**/*.ts: Use explicit type annotations for function parameters and return types
Prefer interfaces over type aliases for object shapes
Use union types and literal types for better type safety
Leverage generic types for reusable components
Group imports: external libraries first, then internal modules
Use absolute imports from package roots when possible
Prefer named exports over default exports
Use custom error classes that extend Error
Implement proper error boundaries and handling
Log errors with sufficient context for debugging
Use observables (RxJS) for reactive programming patterns where appropriate
Manage subscriptions to avoid memory leaks
Use appropriate RxJS operators for data transformation
Handle errors in observable streams
Use async/await for sequential operations
Handle promise rejections properly
Use Promise.all() for concurrent operations
Implement timeout handling for long-running operations
Dispose of resources properly (subscriptions, event listeners)
Use weak references where appropriate
Avoid creating unnecessary objects in hot paths
Profile memory usage for performance-critical code
Use tree-shaking-friendly imports
Lazy load heavy modules when possible
Monitor bundle size impacts of new dependencies
Use dynamic imports for code splitting
Validate and sanitize all user inputs
Implement proper XSS and injection prevention
Validate API responses before processing
Sanitize sensitive data in logs
Follow secure coding practices
Group related functionality in modules
Keep files focused and not too large
Use consistent naming conventions
Organize imports and exports clearly
Write JSDoc comments for public APIs
Keep documentation up to date with code changes (inline docs)
Use meaningful variable and function names
Handle environment-specific APIs properly
Use TypeScript features appropriate for the configured version
Files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Use TypeScript for implementation across the codebase
Files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
**/*.{spec,test}.{ts,tsx,js}
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Write and maintain tests; Jest is used for most testing
Files:
packages/altair-core/src/utils/dot-notation.spec.ts
packages/altair-core/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Modify shared logic/types/utilities in packages/altair-core carefully, as changes impact multiple packages; ensure thorough testing
Files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
**/*.{test,spec}.{ts,js}
📄 CodeRabbit inference engine (.github/instructions/testing.instructions.md)
**/*.{test,spec}.{ts,js}: Follow the Arrange-Act-Assert (AAA) pattern in tests
Write descriptive test names that explain expected behavior
Keep tests focused and independent
Use consistent naming conventions across all test files
Group related tests using describe blocks
Use nested describe blocks for different methods or scenarios
Place setup code in beforeEach or beforeAll hooks
Clean up resources in afterEach or afterAll hooks
Mock external dependencies to isolate units under test
Use Jest's mocking capabilities effectively
Create reusable mock factories for common dependencies
Verify interactions with mocked dependencies when necessary
Use async/await for testing promises
Test both success and error scenarios in async code
Handle timeouts appropriately in async tests
Test concurrent operations when relevant
For NestJS controllers, test HTTP handling, response formatting, auth, and error/status codes; mock service dependencies
For NestJS services, test business logic, data transformations, error handling/validation, and verify logging/monitoring calls
For API integration tests, test endpoints end-to-end, use test DB/transactions, test auth flows, and verify API contracts/responses
For browser extensions, mock browser APIs (chrome., browser.), test message passing, content scripts, and verify manifest configuration
Write performance tests for critical code paths and set performance budgets/thresholds
Monitor test execution times and profile memory usage in tests
Load test API endpoints, verify graceful degradation, check for resource cleanup/memory leaks, and monitor performance metrics
E2E tests should focus on critical user journeys, use realistic data, test cross-browser, and verify integrations
Use dedicated test environments, mock external services appropriately, ensure data consistency, and clean up test artifacts
Create reusable test data factories and use realistic but anonymized data; version fixtures with code and clean up after tests
Maintain high t...
Files:
packages/altair-core/src/utils/dot-notation.spec.ts
**/*.{spec,test}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{spec,test}.ts: Write unit tests focusing on business logic and behavior
Use descriptive test names
Mock external dependencies appropriately in tests
Test edge cases and error conditions
Files:
packages/altair-core/src/utils/dot-notation.spec.ts
🧠 Learnings (18)
📚 Learning: 2025-10-03T16:41:45.498Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2025-10-03T16:41:45.498Z
Learning: Applies to packages/altair-app/**/*.{test,spec}.ts : In components, test methods, business logic, event emissions, state changes, lifecycle, and integration with injected services
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Test state management and property changes of components
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:39:27.517Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/main.instructions.md:0-0
Timestamp: 2025-10-03T16:39:27.517Z
Learning: Applies to packages/altair-core/**/*.ts : Modify shared logic/types/utilities in packages/altair-core carefully, as changes impact multiple packages; ensure thorough testing
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Do not test third-party library behavior
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Do not test Angular framework internals
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-13T05:15:52.113Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/angular-components.instructions.md:0-0
Timestamp: 2025-10-13T05:15:52.113Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.component.spec.ts : Test event emissions, state changes, and method behavior in unit tests
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/auth/**/*.spec.ts : Test authentication and authorization scenarios
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.spec.ts : Mock dependencies properly in tests
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:36:10.024Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/angular-services.instructions.md:0-0
Timestamp: 2025-10-03T16:36:10.024Z
Learning: Applies to packages/altair-app/src/**/*.service.spec.ts : Test public methods and observable streams of services
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Avoid fragile emission index tracking across multiple actions in a single test; isolate scenarios per it block
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.controller.ts : Protect routes with NestJS guards (e.g., AuthGuard) on controller handlers
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.{guard,decorator}.ts : Use NestJS guards for route protection and implement role-based access control
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.service.ts : Ensure proper SQL injection protection in database access code
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:39:27.517Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/main.instructions.md:0-0
Timestamp: 2025-10-03T16:39:27.517Z
Learning: Applies to packages/altair-db/**/*.ts : Keep database-related utilities in TypeScript aligned with Prisma usage in packages/altair-db
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:36:10.024Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/angular-services.instructions.md:0-0
Timestamp: 2025-10-03T16:36:10.024Z
Learning: Applies to packages/altair-app/src/**/*.service.ts : Avoid side effects within utility service methods
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/config/**/*.ts : Use type-safe configuration classes
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.{dto,pipe}.ts : Validate all input data
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.controller.ts : Keep API documentation up to date with endpoints
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
🧬 Code graph analysis (1)
packages/altair-core/src/utils/dot-notation.spec.ts (1)
packages/altair-core/src/utils/dot-notation.ts (1)
setByDotNotation(19-72)
🪛 GitHub Check: SonarCloud Code Analysis
packages/altair-core/src/utils/dot-notation.spec.ts
[warning] 48-48: This assertion is unnecessary since it does not change the type of the expression.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/altair-core/src/utils/dot-notation.spec.ts (1)
45-53: Good coverage of main pollution vectors.The tests correctly verify that
__proto__andconstructor.prototypepaths don't pollute the target or global prototypes.Consider expanding test coverage for additional edge cases:
// Test individual dangerous segments setByDotNotation(validObj, 'constructor', 'blocked'); expect(validObj.constructor).not.toBe('blocked'); setByDotNotation(validObj, 'prototype', 'blocked'); expect((validObj as any).prototype).not.toBe('blocked'); // Test with array path input directly setByDotNotation(validObj, ['__proto__', 'polluted'], 'blocked'); expect((validObj as any).polluted).toBeUndefined(); expect(({} as any).polluted).toBeUndefined(); // Test dangerous keys at different depths setByDotNotation(validObj, 'safe.path.__proto__.polluted', 'blocked'); expect((validObj as any).safe?.path?.__proto__?.polluted).toBeUndefined();Based on learnings, test edge cases and error conditions, and ensure thorough testing for shared code in packages/altair-core.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/altair-core/src/utils/dot-notation.spec.ts(1 hunks)packages/altair-core/src/utils/dot-notation.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
{**/__tests__/**/*.ts,**/*.{spec,test}.ts}
📄 CodeRabbit inference engine (.github/instructions/app-testing.instructions.md)
{**/__tests__/**/*.ts,**/*.{spec,test}.ts}: Use Jest as the testing framework for all tests
Organize tests next to the code under test: use a tests folder or .test.ts/.spec.ts files alongside sources
Use clear, descriptive test names explaining what is being verified
Mock dependencies with Jest to isolate the unit under test
Leverage TypeScript types in tests; define interfaces/types for expected data shapes
Files:
packages/altair-core/src/utils/dot-notation.spec.ts
**/*.ts
📄 CodeRabbit inference engine (.github/instructions/app-testing.instructions.md)
Follow project code style using ESLint and Prettier
**/*.ts: Use explicit type annotations for function parameters and return types
Prefer interfaces over type aliases for object shapes
Use union types and literal types for better type safety
Leverage generic types for reusable components
Group imports: external libraries first, then internal modules
Use absolute imports from package roots when possible
Prefer named exports over default exports
Use custom error classes that extend Error
Implement proper error boundaries and handling
Log errors with sufficient context for debugging
Use observables (RxJS) for reactive programming patterns where appropriate
Manage subscriptions to avoid memory leaks
Use appropriate RxJS operators for data transformation
Handle errors in observable streams
Use async/await for sequential operations
Handle promise rejections properly
Use Promise.all() for concurrent operations
Implement timeout handling for long-running operations
Dispose of resources properly (subscriptions, event listeners)
Use weak references where appropriate
Avoid creating unnecessary objects in hot paths
Profile memory usage for performance-critical code
Use tree-shaking-friendly imports
Lazy load heavy modules when possible
Monitor bundle size impacts of new dependencies
Use dynamic imports for code splitting
Validate and sanitize all user inputs
Implement proper XSS and injection prevention
Validate API responses before processing
Sanitize sensitive data in logs
Follow secure coding practices
Group related functionality in modules
Keep files focused and not too large
Use consistent naming conventions
Organize imports and exports clearly
Write JSDoc comments for public APIs
Keep documentation up to date with code changes (inline docs)
Use meaningful variable and function names
Handle environment-specific APIs properly
Use TypeScript features appropriate for the configured version
Files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Use TypeScript for implementation across the codebase
Files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
**/*.{spec,test}.{ts,tsx,js}
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Write and maintain tests; Jest is used for most testing
Files:
packages/altair-core/src/utils/dot-notation.spec.ts
packages/altair-core/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/main.instructions.md)
Modify shared logic/types/utilities in packages/altair-core carefully, as changes impact multiple packages; ensure thorough testing
Files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
**/*.{test,spec}.{ts,js}
📄 CodeRabbit inference engine (.github/instructions/testing.instructions.md)
**/*.{test,spec}.{ts,js}: Follow the Arrange-Act-Assert (AAA) pattern in tests
Write descriptive test names that explain expected behavior
Keep tests focused and independent
Use consistent naming conventions across all test files
Group related tests using describe blocks
Use nested describe blocks for different methods or scenarios
Place setup code in beforeEach or beforeAll hooks
Clean up resources in afterEach or afterAll hooks
Mock external dependencies to isolate units under test
Use Jest's mocking capabilities effectively
Create reusable mock factories for common dependencies
Verify interactions with mocked dependencies when necessary
Use async/await for testing promises
Test both success and error scenarios in async code
Handle timeouts appropriately in async tests
Test concurrent operations when relevant
For NestJS controllers, test HTTP handling, response formatting, auth, and error/status codes; mock service dependencies
For NestJS services, test business logic, data transformations, error handling/validation, and verify logging/monitoring calls
For API integration tests, test endpoints end-to-end, use test DB/transactions, test auth flows, and verify API contracts/responses
For browser extensions, mock browser APIs (chrome., browser.), test message passing, content scripts, and verify manifest configuration
Write performance tests for critical code paths and set performance budgets/thresholds
Monitor test execution times and profile memory usage in tests
Load test API endpoints, verify graceful degradation, check for resource cleanup/memory leaks, and monitor performance metrics
E2E tests should focus on critical user journeys, use realistic data, test cross-browser, and verify integrations
Use dedicated test environments, mock external services appropriately, ensure data consistency, and clean up test artifacts
Create reusable test data factories and use realistic but anonymized data; version fixtures with code and clean up after tests
Maintain high t...
Files:
packages/altair-core/src/utils/dot-notation.spec.ts
**/*.{spec,test}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{spec,test}.ts: Write unit tests focusing on business logic and behavior
Use descriptive test names
Mock external dependencies appropriately in tests
Test edge cases and error conditions
Files:
packages/altair-core/src/utils/dot-notation.spec.ts
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/main.instructions.md:0-0
Timestamp: 2025-10-03T16:39:27.517Z
Learning: Applies to packages/altair-core/**/*.ts : Modify shared logic/types/utilities in packages/altair-core carefully, as changes impact multiple packages; ensure thorough testing
📚 Learning: 2025-10-03T16:41:45.498Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2025-10-03T16:41:45.498Z
Learning: Applies to packages/altair-app/**/*.{test,spec}.ts : In components, test methods, business logic, event emissions, state changes, lifecycle, and integration with injected services
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Test state management and property changes of components
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:39:27.517Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/main.instructions.md:0-0
Timestamp: 2025-10-03T16:39:27.517Z
Learning: Applies to packages/altair-core/**/*.ts : Modify shared logic/types/utilities in packages/altair-core carefully, as changes impact multiple packages; ensure thorough testing
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-13T05:15:52.113Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/angular-components.instructions.md:0-0
Timestamp: 2025-10-13T05:15:52.113Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.component.spec.ts : Test event emissions, state changes, and method behavior in unit tests
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.spec.ts : Mock dependencies properly in tests
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Do not test third-party library behavior
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Do not test Angular framework internals
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/auth/**/*.spec.ts : Test authentication and authorization scenarios
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.spec.ts : Write unit tests for services and controllers
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to packages/altair-app/src/app/modules/altair/components/**/*.spec.ts : Always cover edge cases (null, rapid calls, invalid inputs) in component tests
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:41:45.498Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2025-10-03T16:41:45.498Z
Learning: Applies to **/*.{test,spec}.{ts,js} : Verify interactions with mocked dependencies when necessary
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:38:42.919Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/javascript.instructions.md:0-0
Timestamp: 2025-10-03T16:38:42.919Z
Learning: Applies to **/*.test.{js,mjs,cjs} : Test error conditions and edge cases
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-09-24T19:40:17.905Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/app-testing.instructions.md:0-0
Timestamp: 2025-09-24T19:40:17.905Z
Learning: Applies to {**/__tests__/**/*.ts,**/*.{spec,test}.ts} : Use clear, descriptive test names explaining what is being verified
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:41:45.498Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2025-10-03T16:41:45.498Z
Learning: Applies to **/*.{test,spec}.{ts,js} : Review test code as carefully as production code; refactor for maintainability; remove flaky tests; keep tests simple and focused
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:41:45.498Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2025-10-03T16:41:45.498Z
Learning: Applies to **/*.{test,spec}.{ts,js} : For browser extensions, mock browser APIs (chrome.*, browser.*), test message passing, content scripts, and verify manifest configuration
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:42:34.287Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-10-03T16:42:34.287Z
Learning: Applies to **/*.{spec,test}.ts : Test edge cases and error conditions
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.tspackages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:41:45.498Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2025-10-03T16:41:45.498Z
Learning: Applies to **/*.{test,spec}.{ts,js} : Create reusable test data factories and use realistic but anonymized data; version fixtures with code and clean up after tests
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:41:45.498Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2025-10-03T16:41:45.498Z
Learning: Applies to **/*.{test,spec}.{ts,js} : Write descriptive test names that explain expected behavior
Applied to files:
packages/altair-core/src/utils/dot-notation.spec.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.controller.ts : Protect routes with NestJS guards (e.g., AuthGuard) on controller handlers
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.{guard,decorator}.ts : Use NestJS guards for route protection and implement role-based access control
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:40:52.954Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/nestjs-api.instructions.md:0-0
Timestamp: 2025-10-03T16:40:52.954Z
Learning: Applies to packages/altair-api/src/**/*.service.ts : Ensure proper SQL injection protection in database access code
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:36:10.024Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/angular-services.instructions.md:0-0
Timestamp: 2025-10-03T16:36:10.024Z
Learning: Applies to packages/altair-app/src/**/*.service.ts : Avoid side effects within utility service methods
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:38:42.919Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/javascript.instructions.md:0-0
Timestamp: 2025-10-03T16:38:42.919Z
Learning: Applies to {bin,scripts}/**/*.{js,mjs,cjs} : Avoid code injection vulnerabilities
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:42:34.287Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-10-03T16:42:34.287Z
Learning: Applies to **/*.ts : Follow secure coding practices
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:38:42.919Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/javascript.instructions.md:0-0
Timestamp: 2025-10-03T16:38:42.919Z
Learning: Applies to {bin,scripts}/**/*.{js,mjs,cjs} : Use secure methods for file operations
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
📚 Learning: 2025-10-03T16:38:42.919Z
Learnt from: CR
Repo: altair-graphql/altair PR: 0
File: .github/instructions/javascript.instructions.md:0-0
Timestamp: 2025-10-03T16:38:42.919Z
Learning: Applies to **/*.{js,mjs,cjs} : Implement proper error handling in legacy code
Applied to files:
packages/altair-core/src/utils/dot-notation.ts
🧬 Code graph analysis (1)
packages/altair-core/src/utils/dot-notation.spec.ts (1)
packages/altair-core/src/utils/dot-notation.ts (1)
setByDotNotation(19-77)
🪛 GitHub Check: SonarCloud Code Analysis
packages/altair-core/src/utils/dot-notation.spec.ts
[warning] 48-48: This assertion is unnecessary since it does not change the type of the expression.
[warning] 52-52: This assertion is unnecessary since it does not change the type of the expression.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (4)
packages/altair-core/src/utils/dot-notation.ts (2)
32-38: LGTM! Prototype pollution guard correctly implemented for string paths.The implementation properly splits the path into segments and checks for exact equality with dangerous keys (
__proto__,constructor,prototype). This avoids the false positives from substring matching while effectively blocking prototype pollution vectors.
44-46: LGTM! Array path protection complete.The guard correctly blocks dangerous keys in array paths using exact segment equality. Given this is shared code in
packages/altair-core, the thorough test coverage in the spec file is critical.Based on learnings, modify shared logic/types/utilities in packages/altair-core carefully, as changes impact multiple packages; ensure thorough testing.
packages/altair-core/src/utils/dot-notation.spec.ts (2)
55-61: Excellent verification of false positive prevention.These tests confirm that legitimate keys containing dangerous substrings (like
myconstructor,some_prototype_value) are correctly allowed, addressing the main concern from previous reviews about substring matching.
48-48: Type assertions are acceptable in tests.SonarCloud flags these type assertions as unnecessary, but they're a common pattern in tests to access properties that TypeScript doesn't know about. The current code is fine and maintains test clarity.
Also applies to: 52-52
1 participant
Overview
This PR fixes a Prototype Pollution vulnerability in the setByDotNotation utility function located in packages/altair-core/src/utils/dot-notation.ts. The previous implementation did not validate path segments, allowing attackers to modify Object.prototype using keys such as proto, constructor, or prototype.
Root Cause
The setByDotNotation function recursively assigns values based on user-controlled dot-notation paths without filtering dangerous keys that can mutate JavaScript prototypes.
What This PR Changes
Adds validation to block unsafe keys: proto, prototype, and constructor.
Prevents prototype mutation at any depth during path traversal.
Ensures safe nested object assignment without affecting global prototypes.
Impact of the Vulnerability (before fix)
An attacker could pollute Object.prototype, which may lead to:
Denial of Service (DoS) by overwriting native Object methods
Logic bypasses and unintended behavior
Potential Remote Code Execution (RCE) in specific gadget-based scenarios
Application-wide instability or unexpected side effects
Steps to Reproduce (before fix)
import { setByDotNotation } from './dot-notation';
const victim = {};
setByDotNotation(victim, 'proto.polluted', 'yes');
const test = {};
console.log((test as any).polluted);
// Output: "yes" (vulnerable)
Recommendation / Fix Strategy
Reject unsafe path segments proactively.
Prevent assignments to proto, constructor, or prototype.
Maintain compatibility for all valid dot-notation paths.
Severity
High — The vulnerability allows prototype pollution, which can compromise the entire application.
Notes
The fix is minimal and targeted. Additional hardening or refactoring can be done in future updates if needed.
#2951
Summary by Sourcery
Prevent prototype pollution in the setByDotNotation utility by rejecting unsafe path segments.
Bug Fixes:
Tests:
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.