[WRONG BRANCH] fix(codex): prevent TOML string regex backtracking - #316
[WRONG BRANCH] fix(codex): prevent TOML string regex backtracking#316luvs01 wants to merge 1 commit into
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe TOML string pattern now handles escaped quotes and backslashes correctly. Tests verify escaped Windows-style values and reject unterminated strings for root and provider-table lookups. ChangesTOML parsing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR narrows TOML string matching to avoid excessive backtracking while preserving escaped-path decoding, with targeted regression coverage; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbeaadf014
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the raw bytes back returned a path that matched nothing on disk and made the | ||
| // journal's recorded catalog path un-restorable (#1798). | ||
| return new RegExp(`^\\s*${keyToken}\\s*=\\s*("(?:\\\\.|[^"])*"|'[^']*')\\s*(?:#.*)?$`); | ||
| return new RegExp(`^\\s*${keyToken}\\s*=\\s*("(?:\\\\.|[^"\\\\])*"|'[^']*')\\s*(?:#.*)?$`); |
There was a problem hiding this comment.
When config.toml contains an unterminated model_catalog_json basic string followed by a backslash run, this change protects only rootTomlString and providerTableString; readRootTomlString in src/codex/paths.ts:136 retains the same overlapping (?:\\.|[^"])* alternatives. The catalog and routing paths call that helper, so even a few dozen backslashes still cause exponential matching and can stall the proxy. Apply the same backslash exclusion to that matcher and cover the exported helper in the regression test.
Useful? React with 👍 / 👎.
1 participant
Motivation
Description
tomlStringPatternto exclude backslash so the pattern becomes"(?:\\.|[^"\\])*"instead of"(?:\\.|[^"])*".tests/codex-injected-marker.test.tswith regression coverage that decodes escaped Windows-style paths and rejects unterminated basic strings with long backslash runs.Testing
bun test tests/codex-injected-marker.test.tsand the two tests passed.bun run typecheckandbun run privacy:scanand both succeeded, andgit diff --checkreported no issues.Codex Task
Summary by CodeRabbit
Bug Fixes
Tests