fix(deps): add npm override to resolve ajv ReDoS alert - #160
Conversation
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
83-83: ⚡ Quick winPrefer a bounded override range to avoid unintended future major upgrades.
Using
>=8.18.0can pullajv@9+later under this subtree. Consider^8.18.0(or an exact 8.x pin) to keep this security fix stable while avoiding surprise compatibility drift.Suggested change
- "ajv": ">=8.18.0" + "ajv": "^8.18.0"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 83, The AJV dependency version range in package.json is unbounded ( "ajv": ">=8.18.0" ); change it to a bounded 8.x range such as "ajv": "^8.18.0" (or pin to an exact 8.x version) to prevent accidental upgrades to AJV v9 while preserving the security fix—update the "ajv" entry accordingly and run dependency install/update to lock the new range.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 81-85: You added an overrides entry for
"@rushstack/node-core-library" to force "ajv" >=8.18.0 but did not regenerate
the lockfile; run npm install to update package-lock.json so the override is
applied (ensuring `@rushstack/node-core-library`'s transitive ajv is bumped to
8.18.0+), verify the lockfile now lists the updated ajv version, and commit the
updated package-lock.json alongside the package.json change.
---
Nitpick comments:
In `@package.json`:
- Line 83: The AJV dependency version range in package.json is unbounded (
"ajv": ">=8.18.0" ); change it to a bounded 8.x range such as "ajv": "^8.18.0"
(or pin to an exact 8.x version) to prevent accidental upgrades to AJV v9 while
preserving the security fix—update the "ajv" entry accordingly and run
dependency install/update to lock the new range.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: aa877d11-2aaa-4f75-89b6-6416f66112e2
📒 Files selected for processing (1)
package.json
Add a scoped override for @rushstack/node-core-library to force ajv >=8.18.0, resolving the Dependabot ReDoS alert (CVE in ajv >=7.0.0-alpha.0 <8.18.0 when the $data option is used). ESLint's ajv@6 tree is unaffected because the override is scoped to the rushstack subtree only. Run `npm install` after merging to regenerate the lockfile with the resolved version. Closes #156 Part of #150 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch override key from nested-object form (which npm 10 does not honour against a ~8.13.0 constraint) to the flat version-selector form `ajv@~8.13.0`, which correctly forces hoisting to ^8.18.0. The nested `@rushstack/node-core-library/node_modules/ajv` entry is removed from the lockfile; ajv@8.20.0 is now shared from the root. `npm audit` reports 0 vulnerabilities for the ajv ReDoS advisory. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Both CodeRabbit points addressed: Range bounded to ^8.18.0 — with a structural correction The original override used the nested-object form Switched to the flat version-selector form instead: "overrides": {
"ajv@~8.13.0": "^8.18.0"
}This matches only packages requesting Lockfile regenerated — advisory cleared After a clean install with the corrected override:
Commit: 11d09c9 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…ons.md PR refs Update the console.error spy in app.test.ts to use util.format so it correctly captures formatted output when production code calls console.error with a format-string + args (the secure CodeQL-safe form). Fix decisions.md to consistently reference PR #160 (the fix) and issue #156 (the Dependabot alert) in the ajv ReDoS override decision entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…163) * fix(security): close remaining Epic #150 CodeQL alerts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(squad): log Epic #150 session, merge decisions inbox Logged Epic #150 (CodeQL + Dependabot security alerts) session with work summary: - Brand: #151 gitleaks, #155 k3d hardening, #156 ajv override (6 PRs merged) - Data: #154 keycloak-jwt, #153 format-string, #152 rate-limit, #163 cleanup - Lead: triage, routing, CodeRabbit shepherding, alert audit Merged 4 decisions from inbox: 1. npm override selector syntax rule (nested-object fails on tilde-pins in npm 10.9.4) 2. CodeQL false-positive dismissal pattern for dev-only scripts with loopback guards 3. Rate-limit policy table (auth 5/15min, write 100/15min, read 300/15min) 4. Rate-limit zero-semantics wrapper (limit=0 via makeRateLimiter preserves disable) Dismissed alerts: CodeQL #5 + #6 (k3d SSRF false positives, dev-only loopback proxy). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix(rate-limit): suppress trust-proxy validation warning express-rate-limit v8 emits a ValidationError to stderr when trust proxy is enabled, polluting captured log output in tests. Trust-proxy is intentional in this k3d deployment; add validate: { trustProxy: false } to rateLimitDefaults in both apps to suppress the runtime warning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(control-plane): use util.format in console.error spy; fix decisions.md PR refs Update the console.error spy in app.test.ts to use util.format so it correctly captures formatted output when production code calls console.error with a format-string + args (the secure CodeQL-safe form). Fix decisions.md to consistently reference PR #160 (the fix) and issue #156 (the Dependabot alert) in the ajv ReDoS override decision entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
overridesblock in rootpackage.jsonto forceajv >= 8.18.0under the@rushstack/node-core-librarysubtreeajv >= 7.0.0-alpha.0 < 8.18.0when the$dataoption is used)ajv@6tree is unaffected — the override is scoped to@rushstack/node-core-libraryonly, not applied globallyContext
The vulnerable
ajv@8.13.0is pulled in transitively by:The
$dataoption (required to trigger the vulnerability) is not used by this project or any of its toolchain dependencies, so the practical risk is low.Lockfile note
@rushstack/node-core-library@5.13.0declares"ajv": "~8.13.0"— a narrow pin. The npm override forcibly overrides this. The lockfile (package-lock.json) will be regenerated on the nextnpm installrun. After merging, runnpm installlocally and commit the updated lockfile, or wait for Dependabot to regenerate it.Test plan
npm ls ajv --allshows noajvin the>=7.0.0-alpha.0 <8.18.0range after lockfile is regeneratednpm run lintpassesnpm run test:cipassesnpm audit --productionreports no high/critical issues related to ajvCloses #156
Part of epic #150
Summary by CodeRabbit