adds a windows-specific method to check if a command is safe #4119
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
iceweasel-oai
force-pushed
the
iceweasel/windows-command-safety
branch
from
19aedc5 to
6561d4c
Compare
|
I have read the CLA Document and I hereby sign the CLA |
iceweasel-oai
force-pushed
the
iceweasel/windows-command-safety
branch
2 times, most recently
from
04aaa69 to
9c2223d
Compare
This reverts commit 7035522.
iceweasel-oai
force-pushed
the
iceweasel/windows-command-safety
branch
from
9c2223d to
13f0876
Compare
| #[test] | ||
| fn everything_is_unsafe() { | ||
| for cmd in [ | ||
| vec_str(&["powershell.exe", "-NoLogo", "-Command", "echo hello"]), |
There was a problem hiding this comment.
I know this is a WIP, but wouldn't this be safe?
There was a problem hiding this comment.
yeah, this is more just meant to test the current "everything is unsafe" behavior. It'll change shortly
| return true; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This is making me question whether we should be doing is_safe_to_call_with_exec() as it works today on Windows. Possibly only if you have Git Bash enabled or whatever it is on Windows and we're actually going to run the command through it? What do you think?
There was a problem hiding this comment.
I originally had a version where only the new windows function is called and nothing after it. But there are some tests that are depending on the other safety checks being called (ie, Windows tests that use commands such as "ls"). I'm not sure if those tests are valid, but I left this for now to be conservative and not regress anything until I can learn more
There was a problem hiding this comment.
ls works in PowerShell by default, apparently. It works in cmd.exe on my machine, but probably due to Git Bash.
refactors command_safety files into its own package, so we can add platform-specific ones
Also creates a windows-specific of
is_known_safe_commandthat just returns false always, since that is what happens today.