fix: require approval for force delete on Windows #8568
Conversation
![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.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Classic `cmd /c start https://...` ShellExecute path. | ||
| if first_cmd_lc == "start" { | ||
| return args_have_url(cmd_args); | ||
| } | ||
|
|
||
| // Parity with Unix `rm -f`: CMD `del/erase /f` forces deletion of read-only files. | ||
| if matches!(first_cmd_lc.as_str(), "del" | "erase") { | ||
| return cmd_args.iter().any(|a| a.eq_ignore_ascii_case("/f")); | ||
| } |
There was a problem hiding this comment.
With this change, is_dangerous_cmd only treats start and del/erase /f as dangerous and returns false for any other command body. That drops the previous args_have_url fallback, so cmd /c msedge https://example.com, cmd /c rundll32 url.dll,fileprotocolhandler https://..., etc. are no longer flagged even though they launch URLs via cmd. This is a regression because the cmd wrapper is the executable being inspected, so is_direct_gui_launch won’t catch the nested browser invocation; as a result the approval gate can be bypassed for URL launches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let mut nested: Vec<String> = Vec::with_capacity(1 + cmd_args.len()); | ||
| nested.push(first_cmd.to_string()); | ||
| nested.extend(cmd_args.iter().cloned()); | ||
| if is_direct_gui_launch(&nested) { | ||
| return true; |
There was a problem hiding this comment.
After parsing cmd /c …, this branch only treats start, del/erase /f, or direct GUI launchers as dangerous. A command like cmd /c powershell -Command "Start-Process https://example.com" (or pwsh) contains a URL but won’t match is_direct_gui_launch, so is_dangerous_cmd returns false and approval is bypassed for a URL launch via PowerShell. This was previously caught by the generic URL-arg fallback and now regresses URL gating for nested PowerShell invocations on Windows.
Useful? React with 👍 / 👎.
|
close it, since duplicated with #8590 |
2 participants
Fix force delete approval bypass on Windows. Ensures ask-for-approval setting is respected when executing force delete operations, aligning behavior with Linux platform.
close #8567