fix(core) disable command_might_be_dangerous when unsandboxed#15036
fix(core) disable command_might_be_dangerous when unsandboxed#15036dylan-hurd-oai merged 4 commits intomainfrom
Conversation
| AskForApproval::Never => Decision::Forbidden, | ||
| AskForApproval::Never => { | ||
| if sandbox_is_explicitly_disabled { | ||
| // If the sandbox is explicitly disabled, we should allow the command to run |
There was a problem hiding this comment.
This could also be because no sandbox is available, like for Windows users who have not installed our sandbox?
There was a problem hiding this comment.
Ah I might be misunderstanding the intent of FileSystemSandboxKind. Should we instead use the following logic?
let sandbox_is_explicitly_disabled = matches!(
sandbox_policy,
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. },
)
I've updated to switch to this instead
|
Here's something that has been on my personal backlog:
Currently, because we cannot distinguish between Also, the point about |
|
re: prefix_rule(), I don't think this change supersedes any ExecPolicy decisions, since this change is in |
| ); | ||
| } | ||
|
|
||
| struct ExecApprovalRequirementScenario { |
There was a problem hiding this comment.
I find this a bit confusing since most of the fields are "inputs" to the assertion whereas ExecApprovalRequirement is the only output.
What about dropping expected_requirement from the struct and doing:
async fn assert_exec_approval_requirement_for_command(
scenario: ExecApprovalRequirementScenario,
expected_requirement: ExecApprovalRequirement,
) {I think that makes it clearer from the call site what is being asserted.
Or even have assert_exec_approval_requirement_for_command() return ExecApprovalRequirement and then have an assert_eq!() so you can pass a string as the third argument explaining the assertion.
bbf12a8 to
cd4a4e0
Compare
Summary
If we are in a mode that is already explicitly un-sandboxed, then
ApprovalPolicy::Nevershould not block dangerous commands.Testing