mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
Support direct Guardian review of patches invoked through exec_command (#58931)
## Why Direct Guardian review rejected recorded `exec_command` calls intercepted as `apply_patch`, even though normal execution could review those patches. ## What changed Share intercepted patch preparation between execution and direct review so Guardian assesses the verified patch and its approval requirement without applying it. ## Testing Extend the direct patch review scenario to cover `exec_command` heredocs alongside native `apply_patch` calls, asserting identical live and direct review inputs and no file writes after direct approval. GitOrigin-RevId: af62cf66a3f01c65a65c15cfad3fc9cdd1f2bf19
This commit is contained in:
@@ -115,19 +115,6 @@ fn is_implicit_patch(argv: &[String], cwd: &PathUri) -> bool {
|
||||
body.is_some_and(|body| parse_patch(body).is_ok())
|
||||
}
|
||||
|
||||
impl MaybeApplyPatch {
|
||||
/// Whether exec intercepts this command, without reading any patch target files.
|
||||
pub fn intercepts_command(argv: &[String], cwd: &PathUri) -> bool {
|
||||
if is_implicit_patch(argv, cwd) {
|
||||
return true;
|
||||
}
|
||||
match maybe_parse_apply_patch(argv, cwd) {
|
||||
Self::Body(_) | Self::PatchParseError(_) => true,
|
||||
Self::ShellParseError(_) | Self::NotApplyPatch => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: make private once we remove tests in lib.rs
|
||||
/// `cwd` supplies the path convention used to interpret the shell executable in `argv`.
|
||||
pub fn maybe_parse_apply_patch(argv: &[String], cwd: &PathUri) -> MaybeApplyPatch {
|
||||
@@ -516,26 +503,6 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classifies_intercepted_commands_without_a_filesystem() {
|
||||
let cwd = PathUri::parse("file:///workspace").expect("valid POSIX test cwd");
|
||||
let patch = wrap_patch("*** Delete File: target");
|
||||
let explicit = format!("apply_patch <<'PATCH'\n{patch}\nPATCH");
|
||||
let malformed = "apply_patch <<'PATCH'\ninvalid patch\nPATCH";
|
||||
for (script, intercepted) in [
|
||||
(explicit.as_str(), true),
|
||||
(patch.as_str(), true),
|
||||
(malformed, true),
|
||||
("echo apply_patch", false),
|
||||
] {
|
||||
assert_eq!(
|
||||
MaybeApplyPatch::intercepts_command(&args_bash(script), &cwd),
|
||||
intercepted,
|
||||
"{script}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_implicit_patch_single_arg_is_error() {
|
||||
let patch = "*** Begin Patch\n*** Add File: foo\n+hi\n*** End Patch".to_string();
|
||||
|
||||
Reference in New Issue
Block a user