Skip to content

Fix formatter ReDoS regexes#210

Closed
Sean-Kenneth-Doherty wants to merge 1 commit into
yoavbls:mainfrom
Sean-Kenneth-Doherty:codex/fix-formatter-redos
Closed

Fix formatter ReDoS regexes#210
Sean-Kenneth-Doherty wants to merge 1 commit into
yoavbls:mainfrom
Sean-Kenneth-Doherty:codex/fix-formatter-redos

Conversation

@Sean-Kenneth-Doherty
Copy link
Copy Markdown
Contributor

@Sean-Kenneth-Doherty Sean-Kenneth-Doherty commented May 17, 2026

Summary

  • replace ReDoS-prone formatter regexes with bounded character classes and sentinels
  • keep existing quoted object-key and string-literal formatting behavior
  • add issue-sized regression coverage for the reported attack-string shapes

Fixes #187.

Verification

  • npm ci
  • npm run format:check
  • npm run build --workspace=@pretty-ts-errors/utils
  • npm run lint --workspace=@pretty-ts-errors/formatter
  • npm test --workspace=@pretty-ts-errors/formatter (29 tests)
  • npx --yes --package @vscode/vsce npm run build
  • git diff --check origin/main...HEAD && git diff --check

The formatter test suite includes the issue-sized ReDoS stress inputs for addMissingParentheses and quote-heavy prettifier messages.

Note: in a fresh checkout, @pretty-ts-errors/utils needs to be built before running the formatter TypeScript lint so the workspace package declarations are available.

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@kevinramharak
Copy link
Copy Markdown
Collaborator

@Sean-Kenneth-Doherty How does this relate to #208? I don't mind AI assisted PR's, but multiple PR's fixing the same issue is confusing.

Copy link
Copy Markdown
Contributor Author

Thanks, that is fair. #210 overlaps with #208 and leaving both open makes the review path confusing.

This PR was a later, smaller regex-only variant with broader attack-string fixtures. I folded the useful fixture coverage and the follow-up fixes into #208 at fc92d1f, so I am closing this duplicate and keeping the ReDoS discussion on #208.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ReDoS Vulnerable Regular Expressions - unlikely to be triggered

2 participants