Move Guardian request mappings into the reviewer crate (#57506)

## What changed

Move approval-request assessment and analytics mappings, target item lookup, and turn ID selection into `codex-guardian-reviewer`. Export the helpers from the crate and re-export them in core, preserving existing behavior.

## Testing

Move the apply-patch serialization test into the reviewer crate. It checks that Windows paths are preserved and patch text is omitted from the assessment action.

GitOrigin-RevId: 3a011db0bc51c653bb0877c4b0cb52eb61c63244
This commit is contained in:
jif
2026-10-27 16:28:42 +00:00
committed by copyberry
parent c8e73bf719
commit 479f7046eb
7 changed files with 208 additions and 192 deletions
+1
View File
@@ -3612,6 +3612,7 @@ dependencies = [
"codex-otel",
"codex-prompts",
"codex-protocol",
"codex-shell-command",
"codex-utils-path-uri",
"opentelemetry",
"opentelemetry_sdk",
+4 -156
View File
@@ -1,161 +1,9 @@
use codex_analytics::GuardianReviewedAction;
pub(crate) use codex_guardian_reviewer::GuardianApprovalRequest;
pub(crate) use codex_guardian_reviewer::GuardianMcpAnnotations;
pub(crate) use codex_guardian_reviewer::GuardianNetworkAccessTrigger;
pub(crate) use codex_guardian_reviewer::format_guardian_action_pretty;
pub(crate) use codex_guardian_reviewer::guardian_approval_request_to_json;
use codex_protocol::approvals::GuardianAssessmentAction;
use codex_protocol::approvals::GuardianCommandSource;
use codex_utils_path_uri::LegacyAppPathString;
fn command_assessment_action(
source: GuardianCommandSource,
command: &[String],
cwd: &LegacyAppPathString,
) -> GuardianAssessmentAction {
GuardianAssessmentAction::Command {
source,
command: codex_shell_command::parse_command::shlex_join(command),
cwd: cwd.clone(),
}
}
pub(crate) fn guardian_assessment_action(
action: &GuardianApprovalRequest,
) -> Option<GuardianAssessmentAction> {
Some(match action {
GuardianApprovalRequest::NativeToolCall { .. } => return None,
GuardianApprovalRequest::ExecCommand {
command,
guardian_cwd,
..
} => command_assessment_action(GuardianCommandSource::UnifiedExec, command, guardian_cwd),
GuardianApprovalRequest::WriteStdin {
approval_id,
process_id,
input,
cwd,
..
} => GuardianAssessmentAction::WriteStdin {
approval_id: approval_id.clone(),
process_id: process_id.to_string(),
stdin: input.clone(),
cwd: cwd.clone(),
},
GuardianApprovalRequest::ApplyPatch { cwd, files, .. } => {
GuardianAssessmentAction::ApplyPatch {
cwd: cwd.clone().into(),
files: files.iter().cloned().map(Into::into).collect(),
}
}
GuardianApprovalRequest::NetworkAccess {
id: _id,
turn_id: _turn_id,
environment_id: _,
target,
host,
protocol,
port,
trigger: _trigger,
} => GuardianAssessmentAction::NetworkAccess {
target: target.clone(),
host: host.clone(),
protocol: *protocol,
port: *port,
},
GuardianApprovalRequest::McpToolCall {
server,
tool_name,
connector_id,
connector_name,
tool_title,
..
} => GuardianAssessmentAction::McpToolCall {
server: server.clone(),
tool_name: tool_name.clone(),
connector_id: connector_id.clone(),
connector_name: connector_name.clone(),
tool_title: tool_title.clone(),
},
GuardianApprovalRequest::RequestPermissions {
reason,
permissions,
..
} => GuardianAssessmentAction::RequestPermissions {
reason: reason.clone(),
permissions: permissions.clone(),
},
})
}
pub(crate) fn guardian_reviewed_action(
request: &GuardianApprovalRequest,
) -> GuardianReviewedAction {
match request {
GuardianApprovalRequest::NativeToolCall { .. } => GuardianReviewedAction::NativeToolCall {},
GuardianApprovalRequest::ExecCommand {
sandbox_permissions,
additional_permissions,
tty,
..
} => GuardianReviewedAction::UnifiedExec {
sandbox_permissions: *sandbox_permissions,
additional_permissions: additional_permissions.as_ref().map(Into::into),
tty: *tty,
},
GuardianApprovalRequest::WriteStdin { tty, .. } => {
GuardianReviewedAction::WriteStdin { tty: *tty }
}
GuardianApprovalRequest::ApplyPatch { .. } => GuardianReviewedAction::ApplyPatch {},
GuardianApprovalRequest::NetworkAccess { protocol, port, .. } => {
GuardianReviewedAction::NetworkAccess {
protocol: *protocol,
port: *port,
}
}
GuardianApprovalRequest::McpToolCall {
server,
tool_name,
connector_id,
connector_name,
tool_title,
..
} => GuardianReviewedAction::McpToolCall {
server: server.clone(),
tool_name: tool_name.clone(),
connector_id: connector_id.clone(),
connector_name: connector_name.clone(),
tool_title: tool_title.clone(),
},
GuardianApprovalRequest::RequestPermissions { .. } => {
GuardianReviewedAction::RequestPermissions {}
}
}
}
pub(crate) fn guardian_request_target_item_id(request: &GuardianApprovalRequest) -> Option<&str> {
match request {
GuardianApprovalRequest::ExecCommand { id, .. }
| GuardianApprovalRequest::WriteStdin { id, .. }
| GuardianApprovalRequest::ApplyPatch { id, .. }
| GuardianApprovalRequest::McpToolCall { id, .. }
| GuardianApprovalRequest::RequestPermissions { id, .. }
| GuardianApprovalRequest::NativeToolCall { id, .. } => Some(id),
GuardianApprovalRequest::NetworkAccess { .. } => None,
}
}
pub(crate) fn guardian_request_turn_id<'a>(
request: &'a GuardianApprovalRequest,
default_turn_id: &'a str,
) -> &'a str {
match request {
GuardianApprovalRequest::NetworkAccess { turn_id, .. }
| GuardianApprovalRequest::RequestPermissions { turn_id, .. } => turn_id,
GuardianApprovalRequest::ExecCommand { .. }
| GuardianApprovalRequest::WriteStdin { .. }
| GuardianApprovalRequest::ApplyPatch { .. }
| GuardianApprovalRequest::McpToolCall { .. }
| GuardianApprovalRequest::NativeToolCall { .. } => default_turn_id,
}
}
pub(crate) use codex_guardian_reviewer::guardian_assessment_action;
pub(crate) use codex_guardian_reviewer::guardian_request_target_item_id;
pub(crate) use codex_guardian_reviewer::guardian_request_turn_id;
pub(crate) use codex_guardian_reviewer::guardian_reviewed_action;
-35
View File
@@ -1522,41 +1522,6 @@ fn guardian_exec_command_uses_executor_cwd_convention(
Ok(())
}
#[test]
fn guardian_apply_patch_preserves_foreign_paths_and_redacts_patch_text() -> serde_json::Result<()> {
let cwd = PathUri::parse("file:///C:/workspace").expect("valid executor cwd");
let file =
PathUri::parse("file:///C:/workspace/guardian.txt").expect("valid executor file path");
let patch = "*** Begin Patch\n*** Update File: guardian.txt\n@@\n+secret\n*** End Patch";
let action = GuardianApprovalRequest::ApplyPatch {
environment_id: "local".to_string(),
id: "patch-1".to_string(),
cwd,
files: vec![file],
patch: patch.to_string(),
};
assert_eq!(
guardian_approval_request_to_json(&action)?,
serde_json::json!({
"tool": "apply_patch",
"environment_id": "local",
"cwd": r"C:\workspace",
"files": [r"C:\workspace\guardian.txt"],
"patch": patch,
}),
);
assert_eq!(
serde_json::to_value(guardian_assessment_action(&action))?,
serde_json::json!({
"type": "apply_patch",
"cwd": r"C:\workspace",
"files": [r"C:\workspace\guardian.txt"],
}),
);
Ok(())
}
#[test]
fn guardian_request_turn_id_prefers_network_access_owner_turn() {
let network_access = GuardianApprovalRequest::NetworkAccess {
@@ -22,6 +22,7 @@ codex-mcp = { workspace = true }
codex-otel = { workspace = true }
codex-prompts = { workspace = true }
codex-protocol = { workspace = true }
codex-shell-command = { workspace = true }
codex-utils-path-uri = { workspace = true }
rand = { workspace = true }
serde = { workspace = true }
@@ -1,6 +1,9 @@
//! Owns Guardian approval-request data and its existing environment and scope classification.
//! Owns Guardian approval-request data, classification, and assessment/reporting mappings.
//! Request debug output excludes action arguments, which may contain credentials.
use codex_analytics::GuardianReviewedAction;
use codex_protocol::approvals::GuardianAssessmentAction;
use codex_protocol::approvals::GuardianCommandSource;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::models::SandboxPermissions;
@@ -141,3 +144,157 @@ impl GuardianApprovalRequest {
}
}
}
fn command_assessment_action(
source: GuardianCommandSource,
command: &[String],
cwd: &LegacyAppPathString,
) -> GuardianAssessmentAction {
GuardianAssessmentAction::Command {
source,
command: codex_shell_command::parse_command::shlex_join(command),
cwd: cwd.clone(),
}
}
pub fn guardian_assessment_action(
action: &GuardianApprovalRequest,
) -> Option<GuardianAssessmentAction> {
Some(match action {
GuardianApprovalRequest::NativeToolCall { .. } => return None,
GuardianApprovalRequest::ExecCommand {
command,
guardian_cwd,
..
} => command_assessment_action(GuardianCommandSource::UnifiedExec, command, guardian_cwd),
GuardianApprovalRequest::WriteStdin {
approval_id,
process_id,
input,
cwd,
..
} => GuardianAssessmentAction::WriteStdin {
approval_id: approval_id.clone(),
process_id: process_id.to_string(),
stdin: input.clone(),
cwd: cwd.clone(),
},
GuardianApprovalRequest::ApplyPatch { cwd, files, .. } => {
GuardianAssessmentAction::ApplyPatch {
cwd: cwd.clone().into(),
files: files.iter().cloned().map(Into::into).collect(),
}
}
GuardianApprovalRequest::NetworkAccess {
id: _id,
turn_id: _turn_id,
environment_id: _,
target,
host,
protocol,
port,
trigger: _trigger,
} => GuardianAssessmentAction::NetworkAccess {
target: target.clone(),
host: host.clone(),
protocol: *protocol,
port: *port,
},
GuardianApprovalRequest::McpToolCall {
server,
tool_name,
connector_id,
connector_name,
tool_title,
..
} => GuardianAssessmentAction::McpToolCall {
server: server.clone(),
tool_name: tool_name.clone(),
connector_id: connector_id.clone(),
connector_name: connector_name.clone(),
tool_title: tool_title.clone(),
},
GuardianApprovalRequest::RequestPermissions {
reason,
permissions,
..
} => GuardianAssessmentAction::RequestPermissions {
reason: reason.clone(),
permissions: permissions.clone(),
},
})
}
pub fn guardian_reviewed_action(request: &GuardianApprovalRequest) -> GuardianReviewedAction {
match request {
GuardianApprovalRequest::NativeToolCall { .. } => GuardianReviewedAction::NativeToolCall {},
GuardianApprovalRequest::ExecCommand {
sandbox_permissions,
additional_permissions,
tty,
..
} => GuardianReviewedAction::UnifiedExec {
sandbox_permissions: *sandbox_permissions,
additional_permissions: additional_permissions.as_ref().map(Into::into),
tty: *tty,
},
GuardianApprovalRequest::WriteStdin { tty, .. } => {
GuardianReviewedAction::WriteStdin { tty: *tty }
}
GuardianApprovalRequest::ApplyPatch { .. } => GuardianReviewedAction::ApplyPatch {},
GuardianApprovalRequest::NetworkAccess { protocol, port, .. } => {
GuardianReviewedAction::NetworkAccess {
protocol: *protocol,
port: *port,
}
}
GuardianApprovalRequest::McpToolCall {
server,
tool_name,
connector_id,
connector_name,
tool_title,
..
} => GuardianReviewedAction::McpToolCall {
server: server.clone(),
tool_name: tool_name.clone(),
connector_id: connector_id.clone(),
connector_name: connector_name.clone(),
tool_title: tool_title.clone(),
},
GuardianApprovalRequest::RequestPermissions { .. } => {
GuardianReviewedAction::RequestPermissions {}
}
}
}
pub fn guardian_request_target_item_id(request: &GuardianApprovalRequest) -> Option<&str> {
match request {
GuardianApprovalRequest::ExecCommand { id, .. }
| GuardianApprovalRequest::WriteStdin { id, .. }
| GuardianApprovalRequest::ApplyPatch { id, .. }
| GuardianApprovalRequest::McpToolCall { id, .. }
| GuardianApprovalRequest::RequestPermissions { id, .. }
| GuardianApprovalRequest::NativeToolCall { id, .. } => Some(id),
GuardianApprovalRequest::NetworkAccess { .. } => None,
}
}
pub fn guardian_request_turn_id<'a>(
request: &'a GuardianApprovalRequest,
default_turn_id: &'a str,
) -> &'a str {
match request {
GuardianApprovalRequest::NetworkAccess { turn_id, .. }
| GuardianApprovalRequest::RequestPermissions { turn_id, .. } => turn_id,
GuardianApprovalRequest::ExecCommand { .. }
| GuardianApprovalRequest::WriteStdin { .. }
| GuardianApprovalRequest::ApplyPatch { .. }
| GuardianApprovalRequest::McpToolCall { .. }
| GuardianApprovalRequest::NativeToolCall { .. } => default_turn_id,
}
}
#[cfg(test)]
#[path = "approval_request_tests.rs"]
mod tests;
@@ -0,0 +1,40 @@
#![cfg(test)]
use super::*;
use crate::guardian_approval_request_to_json;
use pretty_assertions::assert_eq;
#[test]
fn guardian_apply_patch_preserves_foreign_paths_and_redacts_patch_text() -> serde_json::Result<()> {
let cwd = PathUri::parse("file:///C:/workspace").expect("valid executor cwd");
let file =
PathUri::parse("file:///C:/workspace/guardian.txt").expect("valid executor file path");
let patch = "*** Begin Patch\n*** Update File: guardian.txt\n@@\n+secret\n*** End Patch";
let action = GuardianApprovalRequest::ApplyPatch {
environment_id: "local".to_string(),
id: "patch-1".to_string(),
cwd,
files: vec![file],
patch: patch.to_string(),
};
assert_eq!(
guardian_approval_request_to_json(&action)?,
serde_json::json!({
"tool": "apply_patch",
"environment_id": "local",
"cwd": r"C:\workspace",
"files": [r"C:\workspace\guardian.txt"],
"patch": patch,
}),
);
assert_eq!(
serde_json::to_value(guardian_assessment_action(&action))?,
serde_json::json!({
"type": "apply_patch",
"cwd": r"C:\workspace",
"files": [r"C:\workspace\guardian.txt"],
}),
);
Ok(())
}
@@ -31,6 +31,10 @@ mod settings;
pub use approval_request::GuardianApprovalRequest;
pub use approval_request::GuardianMcpAnnotations;
pub use approval_request::GuardianNetworkAccessTrigger;
pub use approval_request::guardian_assessment_action;
pub use approval_request::guardian_request_target_item_id;
pub use approval_request::guardian_request_turn_id;
pub use approval_request::guardian_reviewed_action;
pub use approval_request_json::format_guardian_action_pretty;
pub use approval_request_json::guardian_approval_request_to_json;
pub use assessment::GuardianAssessment;