fix: redos protection - #1032
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens schema validation against regex denial-of-service (ReDoS) by replacing several backtracking-prone OpenAPI pattern regexes with structurally simpler, bounded expressions, and by introducing tests + an ADR documenting the rationale and tradeoffs.
Changes:
- Replace multiple regex patterns in
src/openapi/definitions.yamlwith safer, anchored variants and addmaxLengthconstraints to cap input size. - Add a Jest test suite that validates pattern behavior and attempts to detect catastrophic backtracking using adversarial inputs in a sandboxed Node process.
- Document the decision and implications in a new ADR.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/patterns.test.ts | Adds unit tests for regex patterns plus a ReDoS-focused execution harness using child processes and timeouts. |
| src/openapi/definitions.yaml | Updates several validation regexes to reduce backtracking risk and adds max length constraints. |
| docs/adr/0001-regex-change.md | Captures the motivation, replacements, and consequences of the regex changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
j-zimnowoda
left a comment
There was a problem hiding this comment.
Great improvement against ReDOS attacks!
Few comments about escaping characters and about test improvement request.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/patterns.test.ts:75
- The child-process timeout (1s) is much shorter than the Jest timeout for the same test case (5s). On slower CI runners, process spawn + startup can exceed 1s even when the regex itself is safe, causing flaky failures.
timeout: 1_000,
No description provided.