fix(adoption-analyzer): remove polynomial ReDoS in nx-projects path regex - #4965
Conversation
…egex Replace the trailing-slash regex /\/+$/ with a linear-time helper. The regex ran on project.root (parsed from project.json/package.json, treated as untrusted library input) and had O(n^2) backtracking on inputs with many trailing '/' not ending in '/', flagged by CodeQL js/polynomial-redos (alert #120, high). Behaviour is unchanged; adds a targeted test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Visit https://backpack.github.io/storybook-prs/4965 to see this build running in a browser. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Removes a CodeQL-reported polynomial-time ReDoS risk by replacing a trailing-slash regex on Nx project roots with a linear-time helper.
Changes:
- Added
stripTrailingSlashes()to remove trailing/characters without regex backtracking. - Updated
buildProjectIndex()to use the helper when buildingprefix. - Added a unit test to validate prefix normalization and project resolution with trailing slashes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| libs/backpack-adoption-analyzer/src/analysis/nx-projects.ts | Replaces the trailing-slash regex with a linear-time helper when building project prefixes. |
| libs/backpack-adoption-analyzer/src/analysis/nx-projects-test.ts | Adds a test covering trailing slash stripping and correct resolveProject behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| }); | ||
|
|
||
| describe("buildProjectIndex", () => { |
There was a problem hiding this comment.
nit: Adding a test case for root: "." → prefix: "" would make the test suite fully cover the conditional.
There was a problem hiding this comment.
Good call — added a case covering root: "." → prefix: "" (and that it resolves every path), so the conditional is now fully covered. Thanks!
Adds the test case suggested in review to fully cover the buildProjectIndex conditional (root "." maps to an empty prefix that resolves every path). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Visit https://backpack.github.io/storybook-prs/4965 to see this build running in a browser. |
1 similar comment
|
Visit https://backpack.github.io/storybook-prs/4965 to see this build running in a browser. |
What this solves
CodeQL code scanning alert #120:
js/polynomial-redos(security severity: high).Location:
libs/backpack-adoption-analyzer/src/analysis/nx-projects.ts:96project.rootis parsed byreadProjectFilefrom therootfield ofproject.json/package.json, which CodeQL treats as untrusted library input. In the regex/\/+$/, the\/+can start matching anywhere within a run of consecutive slashes, and combined with the trailing$anchor this causes catastrophic backtracking: for inputs shaped like "many slashes followed by a non-slash char" (e.g."////…x"), the engine re-scans the remaining slashes from every possible starting slash, giving O(n²) polynomial time complexity — a potential ReDoS (denial of service).What changed
nx-projects.ts: Replaced the regex with a linear-time (O(n), no backtracking)stripTrailingSlasheshelper. Behaviour is unchanged — it still strips all trailing slashes fromroot.nx-projects-test.ts: Added a targeted test verifyingroot: "apps/web///"→prefix: "apps/web/"and that the project still resolves correctly.Verification
jest: 6 passedtsctypecheck: cleaneslint: clean