fix(security): rewrite two ReDoS-able regular expressions flagged by code scanning - #604
devin-ai-integration[bot] wants to merge 4 commits into
Conversation
- lab/events/limits.ts: the path-body alternation (\/|[^/\0\r\n]+)+ could repartition a non-slash run exponentially; rewritten as a strictly alternating segment/slash pattern accepting the same strings. - exec-tool-result-normalize.ts: adjacent \n+/\s* quantifiers in EMPTY_EXEC_OUTPUT_REGEX could repartition a newline block combinatorially; each run is now pinned to its maximal match via the lookahead-capture idiom. Verified same-language by fuzzing against the previous patterns (200k randomized cases each, zero mismatches) and by quadratic growth in the old pattern (80k newlines: 3.9s -> <1ms). Co-Authored-By: Epinephrine <luvs01@hanmail.net>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository: luvs01/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Coverage for the code-scanning fixes: section-combination acceptance for EMPTY_EXEC_OUTPUT_REGEX plus a pathological-whitespace case, and near-4KiB pathological inputs for RAW_POSIX_PATH_RE exercised through enforceEventStructureLimits. Co-Authored-By: Epinephrine <luvs01@hanmail.net>
|
✅ Deterministic PR hygiene checks passed. |
|
Regression coverage added in 675d795:
|
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
…test.ts banner (20-min cancel, zero test lines)
Summary
GitHub code scanning (CodeQL
javascript-code-scanningsuite) flags two regular expressions as ReDoS-able on uncontrolled input. Both are rewritten to be unambiguous while accepting the same language:src/lab/events/limits.ts—RAW_POSIX_PATH_RE(error severity). The path body(?:\/|[^/\0\r\n]+)+could repartition a run of non-slash characters exponentially, and the trailing\/?let a slash run split two ways. It now alternates strictly between non-slash segments and slash runs:[^/\0\r\n]+(?:\/+[^/\0\r\n]+)*\/*— one parse per string. The regex runs on client-supplied lab-event string fields, so a crafted value is a CPU-exhaustion vector on the ingest path.src/adapters/exec-tool-result-normalize.ts—EMPTY_EXEC_OUTPUT_REGEX(warning severity). Adjacent\n+and\s*quantifiers could split a newline block between sections combinatorially; each wildcard run is now pinned to its maximal match via the(?=(X))\1lookahead-capture idiom. Measured: 80k-newline input went from ~3.9s to <1ms (quadratic → linear).All other code-scanning findings on this ruleset were triaged and found to be false positives (SHA-256 key fingerprinting reported as password hashing,
Math.random()session-affinity IDs, gated URL substring checks, test-file matches) and are not touched here.Verification
/,//in path, trailing slashes, blank-line-separated exec wrappers) — zero mismatches.bun test tests/adapters/exec-tool-result-normalize.test.ts tests/lab/lab-post-merge-hardening.test.ts tests/lab/lab-evidence-ledger.test.ts tests/providers/kiro/kiro-adapter.test.ts tests/adapters/tool-catalog-nudge.test.ts tests/responses/openai-responses-passthrough.test.ts— 386 pass, 0 fail.bun run typecheck,bun run privacy:scan— clean.bun run test:changed— 3 failures intests/vision/vision-routed.test.ts(EBUSYtemp-dir cleanup →SPEND_LEDGER_OWNER_HOME_CONFLICT); reproduced identically on a cleandevworktree, so unrelated to this change (Windows file-locking environment issue).Checklist
Link to Devin session: https://app.devin.ai/sessions/18173a51bcf04824a2e66ca1d2e42ccf
Open in Devin Desktop: https://app.devin.ai/desktop/session/18173a51bcf04824a2e66ca1d2e42ccf?variant=devin
Requested by: @luvs01
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.