Convert cmd escaping to support linear regexp engine#2383
Merged
ericcornelissen merged 15 commits intomainfrom Mar 6, 2026
Merged
Convert cmd escaping to support linear regexp engine#2383ericcornelissen merged 15 commits intomainfrom
cmd escaping to support linear regexp engine#2383ericcornelissen merged 15 commits intomainfrom
Conversation
Update `cmd.js` to use the lregexp package to transparently create regular expressions that use the linear engine in order to avoid any ReDoS vulnerabilities.This commit target the logic behind Shescape's `escape(All)` API only. The refactoring is mostly straightforward with the exception of the logic for `"` (double quotes). In particular, the regular expression for `"` previously used a negative lookbehind. This has been replaced with a capturing group matching the start of the string or any character not matching the characters found previously in the negative lookbehind. This capturing group is put back in to the output string verbatim, thus having the same effect as a negative lookbehind.
ericcornelissen
added
the
refactor
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
Mar 5, 2026 11:38a.m. | Review ↗ |
Update `cmd.js` to use the lregexp package to transparently create regular expressions that use the linear engine in order to avoid any ReDoS vulnerabilities. This commit target the logic behind Shescape's `quote(All)` API only. The refactoring is mostly straightforward with the exception of the logic for `\` (backslashes). Like f79a6b3, the negative lookbehind, and in this case positive lookahead too, is replaced by a capturing group that the replacement puts back in the string. This doesn't change the behavior of the replacing because what is in the capturing group never needs to be captured again in the same pass (it can't be another `\`).
2 kill 2 mutants uncovered by the changes in the previous commit, in particular the start of string anchor in the backslashes regexp and the global flag in the backslashes regexp by testing backslashes at the start of the string and multiple backslashes in a string resp. Additionally, add various test cases that ensure the order of the replacements for quoting is correct. In particular, backslashes must be escaped **after** special characters because the latter introduces characters that require escaping the former.
ericcornelissen
force-pushed
the
lregexp-cmd
branch
from
ec8228b to
7794166
Compare
Taken from `win/no-shell.js`.
This comment was marked as resolved.
This comment was marked as resolved.
![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: 1dc7703557
ℹ️ 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".
Rework escaping for CMD so that escaping a `"` works as follows 1. Escape the `"` itself (as `\\^"`) and inject a marker as a unique anchor for the required additional escaping of backslashes. The marker is guaranteed to be unique because we previously removed it indiscriminately from the argument. 2. Use the marker for further replacements. If it's preceded by `\\` it means the `\\` proceeds a `"` in the original string. If it's not it should still be removed.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as resolved.
This comment was marked as resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #2122