mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Redact secrets from app-server command execution items (#36893)
## Why Client-facing command execution items can include recognizable secrets in the rendered command or parsed command actions. ## What changed - Redact secrets in `commandExecution.command` and `commandActions` for live items, completed items, and replayed thread history. - Keep command approval requests backed by the original executable command and parsed actions while using the redacted representation for display items. - Document that execution item commands are redacted display values rather than executable commands. ## Testing - Cover redaction for command conversion, parsed search actions, legacy replay, approval flows, completion notifications, and rejected commands. GitOrigin-RevId: 2ad056f21882bf5166182ad1a4ff0bf6471d9c0e
This commit is contained in:
1 parent
6d4d9442c7
commit
fcf636a41d
10 files changed
+238
-70
No files matched your search
@@ -35,6 +35,7 @@ use codex_protocol::protocol::PatchApplyEndEvent;
|
||||
use codex_protocol::protocol::ReviewOutputEvent;
|
||||
use codex_protocol::review_format::REVIEW_FALLBACK_MESSAGE;
|
||||
use codex_protocol::review_format::render_review_output_text;
|
||||
use codex_secrets::redact_secrets;
|
||||
use codex_shell_command::parse_command::parse_command;
|
||||
use codex_shell_command::parse_command::shlex_join;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -42,6 +43,24 @@ use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use tracing::warn;
|
||||
|
||||
/// Client-facing command and parsed actions projected from a raw command.
|
||||
pub struct CommandExecutionPresentation {
|
||||
/// Shell-formatted command with recognizable secrets redacted.
|
||||
pub command: String,
|
||||
/// Parsed command actions with recognizable secrets redacted.
|
||||
pub command_actions: Vec<CommandAction>,
|
||||
}
|
||||
|
||||
impl CommandExecutionPresentation {
|
||||
/// Projects a raw command into its client-facing representation.
|
||||
pub fn from_raw(command: &[String], parsed_cmd: &[ParsedCommand], cwd: &PathUri) -> Self {
|
||||
Self {
|
||||
command: redact_secrets(shlex_join(command)),
|
||||
command_actions: command_actions_for_path_uri(parsed_cmd, cwd),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn review_output_text(output: Option<&ReviewOutputEvent>) -> String {
|
||||
output
|
||||
.map(render_review_output_text)
|
||||
@@ -75,17 +94,18 @@ pub fn build_file_change_end_item(payload: &PatchApplyEndEvent) -> ThreadItem {
|
||||
}
|
||||
|
||||
pub fn build_command_execution_begin_item(payload: &ExecCommandBeginEvent) -> ThreadItem {
|
||||
let command_actions = command_actions_for_path_uri(&payload.parsed_cmd, &payload.cwd);
|
||||
let presentation =
|
||||
CommandExecutionPresentation::from_raw(&payload.command, &payload.parsed_cmd, &payload.cwd);
|
||||
ThreadItem::CommandExecution {
|
||||
id: payload.call_id.clone(),
|
||||
plugin_id: payload.plugin_id.clone(),
|
||||
script_path: payload.script_path.clone(),
|
||||
command: shlex_join(&payload.command),
|
||||
command: presentation.command,
|
||||
cwd: payload.cwd.clone().into(),
|
||||
process_id: payload.process_id.clone(),
|
||||
source: payload.source.into(),
|
||||
status: CommandExecutionStatus::InProgress,
|
||||
command_actions,
|
||||
command_actions: presentation.command_actions,
|
||||
aggregated_output: None,
|
||||
exit_code: None,
|
||||
duration_ms: None,
|
||||
@@ -99,28 +119,26 @@ pub fn build_command_execution_end_item(payload: &ExecCommandEndEvent) -> Thread
|
||||
Some(payload.aggregated_output.clone())
|
||||
};
|
||||
let duration_ms = i64::try_from(payload.duration.as_millis()).unwrap_or(i64::MAX);
|
||||
let command_actions = command_actions_for_path_uri(&payload.parsed_cmd, &payload.cwd);
|
||||
let presentation =
|
||||
CommandExecutionPresentation::from_raw(&payload.command, &payload.parsed_cmd, &payload.cwd);
|
||||
|
||||
ThreadItem::CommandExecution {
|
||||
id: payload.call_id.clone(),
|
||||
plugin_id: payload.plugin_id.clone(),
|
||||
script_path: payload.script_path.clone(),
|
||||
command: shlex_join(&payload.command),
|
||||
command: presentation.command,
|
||||
cwd: payload.cwd.clone().into(),
|
||||
process_id: payload.process_id.clone(),
|
||||
source: payload.source.into(),
|
||||
status: (&payload.status).into(),
|
||||
command_actions,
|
||||
command_actions: presentation.command_actions,
|
||||
aggregated_output,
|
||||
exit_code: Some(payload.exit_code),
|
||||
duration_ms: Some(duration_ms),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn command_actions_for_path_uri(
|
||||
parsed_cmd: &[ParsedCommand],
|
||||
cwd: &PathUri,
|
||||
) -> Vec<CommandAction> {
|
||||
fn command_actions_for_path_uri(parsed_cmd: &[ParsedCommand], cwd: &PathUri) -> Vec<CommandAction> {
|
||||
parsed_cmd
|
||||
.iter()
|
||||
.cloned()
|
||||
@@ -131,7 +149,7 @@ pub(crate) fn command_actions_for_path_uri(
|
||||
// genuinely opaque cwd would require executor-native state unavailable here.
|
||||
match cwd.join(path.to_string_lossy().as_ref()) {
|
||||
Ok(path) => Some(CommandAction::Read {
|
||||
command: cmd,
|
||||
command: redact_secrets(cmd),
|
||||
name,
|
||||
path: path.into(),
|
||||
}),
|
||||
@@ -147,15 +165,18 @@ pub(crate) fn command_actions_for_path_uri(
|
||||
}
|
||||
}
|
||||
}
|
||||
ParsedCommand::ListFiles { cmd, path } => {
|
||||
Some(CommandAction::ListFiles { command: cmd, path })
|
||||
}
|
||||
ParsedCommand::Search { cmd, query, path } => Some(CommandAction::Search {
|
||||
command: cmd,
|
||||
query,
|
||||
ParsedCommand::ListFiles { cmd, path } => Some(CommandAction::ListFiles {
|
||||
command: redact_secrets(cmd),
|
||||
path,
|
||||
}),
|
||||
ParsedCommand::Unknown { cmd } => Some(CommandAction::Unknown { command: cmd }),
|
||||
ParsedCommand::Search { cmd, query, path } => Some(CommandAction::Search {
|
||||
command: redact_secrets(cmd),
|
||||
query: query.map(redact_secrets),
|
||||
path,
|
||||
}),
|
||||
ParsedCommand::Unknown { cmd } => Some(CommandAction::Unknown {
|
||||
command: redact_secrets(cmd),
|
||||
}),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ use serde_json::json;
|
||||
|
||||
#[test]
|
||||
fn read_command_actions_preserve_native_and_foreign_paths() {
|
||||
let api_key = "sk-abcdefghijklmnopqrstuvwxyz123456";
|
||||
for (cwd_uri, relative_path, expected_path) in [
|
||||
(
|
||||
"file:///home/alice/repo",
|
||||
@@ -38,6 +39,11 @@ fn read_command_actions_preserve_native_and_foreign_paths() {
|
||||
cmd: "ls".to_string(),
|
||||
path: Some("subdir".to_string()),
|
||||
},
|
||||
ParsedCommand::Search {
|
||||
cmd: format!("rg {api_key}"),
|
||||
query: Some(api_key.to_string()),
|
||||
path: Some("src".to_string()),
|
||||
},
|
||||
ParsedCommand::Search {
|
||||
cmd: "rg needle".to_string(),
|
||||
query: Some("needle".to_string()),
|
||||
@@ -60,6 +66,12 @@ fn read_command_actions_preserve_native_and_foreign_paths() {
|
||||
"command": "ls",
|
||||
"path": "subdir",
|
||||
},
|
||||
{
|
||||
"type": "search",
|
||||
"command": "rg [REDACTED_SECRET]",
|
||||
"query": "[REDACTED_SECRET]",
|
||||
"path": "src",
|
||||
},
|
||||
{
|
||||
"type": "search",
|
||||
"command": "rg needle",
|
||||
|
||||
@@ -1649,6 +1649,7 @@ mod tests {
|
||||
use codex_protocol::protocol::CompactedItem;
|
||||
use codex_protocol::protocol::DynamicToolCallResponseEvent;
|
||||
use codex_protocol::protocol::EnteredReviewModeEvent;
|
||||
use codex_protocol::protocol::ExecCommandBeginEvent;
|
||||
use codex_protocol::protocol::ExecCommandEndEvent;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
use codex_protocol::protocol::ExitedReviewModeEvent;
|
||||
@@ -2111,19 +2112,28 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_command_plugin_id_across_legacy_upsert() {
|
||||
fn preserves_command_plugin_id_and_redacts_secrets_across_legacy_upsert() {
|
||||
let turn_id = "turn-1";
|
||||
let thread_id = ThreadId::new();
|
||||
let command = vec![
|
||||
"git".to_string(),
|
||||
"-c".to_string(),
|
||||
"http.extraHeader=Authorization: Bearer example_synthetic_bearer_token_123456"
|
||||
.to_string(),
|
||||
"push".to_string(),
|
||||
];
|
||||
let parsed_cmd = vec![ParsedCommand::Unknown {
|
||||
cmd: "git -c 'http.extraHeader=Authorization: Bearer example_synthetic_bearer_token_123456' push"
|
||||
.to_string(),
|
||||
}];
|
||||
let command_item = CoreTurnItem::CommandExecution(CoreCommandExecutionItem {
|
||||
id: "exec-1".to_string(),
|
||||
plugin_id: Some("sample@openai-curated".to_string()),
|
||||
script_path: Some("scripts/run.py".to_string()),
|
||||
process_id: Some("pid-1".to_string()),
|
||||
command: vec!["echo".to_string(), "hello world".to_string()],
|
||||
command: command.clone(),
|
||||
cwd: test_path_buf("/tmp").abs().into(),
|
||||
parsed_cmd: vec![ParsedCommand::Unknown {
|
||||
cmd: "echo hello world".to_string(),
|
||||
}],
|
||||
parsed_cmd: parsed_cmd.clone(),
|
||||
source: ExecCommandSource::Agent,
|
||||
interaction_input: None,
|
||||
status: CoreCommandExecutionStatus::Completed,
|
||||
@@ -2142,6 +2152,19 @@ mod tests {
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
}),
|
||||
EventMsg::ExecCommandBegin(ExecCommandBeginEvent {
|
||||
call_id: "exec-1".to_string(),
|
||||
plugin_id: Some("sample@openai-curated".to_string()),
|
||||
script_path: Some("scripts/run.py".to_string()),
|
||||
process_id: Some("pid-1".to_string()),
|
||||
turn_id: turn_id.to_string(),
|
||||
started_at_ms: 0,
|
||||
command: command.clone(),
|
||||
cwd: test_path_buf("/tmp").abs().into(),
|
||||
parsed_cmd: parsed_cmd.clone(),
|
||||
source: ExecCommandSource::Agent,
|
||||
interaction_input: None,
|
||||
}),
|
||||
EventMsg::ItemCompleted(ItemCompletedEvent {
|
||||
thread_id,
|
||||
turn_id: turn_id.to_string(),
|
||||
@@ -2156,11 +2179,9 @@ mod tests {
|
||||
process_id: Some("pid-1".to_string()),
|
||||
turn_id: turn_id.to_string(),
|
||||
completed_at_ms: 1_000,
|
||||
command: vec!["echo".to_string(), "hello world".to_string()],
|
||||
command,
|
||||
cwd: test_path_buf("/tmp").abs().into(),
|
||||
parsed_cmd: vec![ParsedCommand::Unknown {
|
||||
cmd: "echo hello world".to_string(),
|
||||
}],
|
||||
parsed_cmd,
|
||||
source: ExecCommandSource::Agent,
|
||||
interaction_input: None,
|
||||
stdout: "hello world\n".to_string(),
|
||||
@@ -2186,6 +2207,29 @@ mod tests {
|
||||
.into_iter()
|
||||
.map(RolloutItem::EventMsg)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(
|
||||
build_turns_from_rollout_items(&items[..2])[0].items,
|
||||
vec![ThreadItem::CommandExecution {
|
||||
id: "exec-1".to_string(),
|
||||
plugin_id: Some("sample@openai-curated".to_string()),
|
||||
script_path: Some("scripts/run.py".to_string()),
|
||||
command: "git -c 'http.extraHeader=Authorization: Bearer [REDACTED_SECRET]' push"
|
||||
.to_string(),
|
||||
cwd: test_path_buf("/tmp").abs().into(),
|
||||
process_id: Some("pid-1".to_string()),
|
||||
source: CommandExecutionSource::Agent,
|
||||
status: CommandExecutionStatus::InProgress,
|
||||
command_actions: vec![CommandAction::Unknown {
|
||||
command:
|
||||
"git -c 'http.extraHeader=Authorization: Bearer [REDACTED_SECRET]' push"
|
||||
.to_string(),
|
||||
}],
|
||||
aggregated_output: None,
|
||||
exit_code: None,
|
||||
duration_ms: None,
|
||||
}]
|
||||
);
|
||||
let turns = build_turns_from_rollout_items(&items);
|
||||
|
||||
assert_eq!(turns.len(), 1);
|
||||
@@ -2195,13 +2239,16 @@ mod tests {
|
||||
id: "exec-1".to_string(),
|
||||
plugin_id: Some("sample@openai-curated".to_string()),
|
||||
script_path: Some("scripts/run.py".to_string()),
|
||||
command: "echo 'hello world'".to_string(),
|
||||
command: "git -c 'http.extraHeader=Authorization: Bearer [REDACTED_SECRET]' push"
|
||||
.to_string(),
|
||||
cwd: test_path_buf("/tmp").abs().into(),
|
||||
process_id: Some("pid-1".to_string()),
|
||||
source: CommandExecutionSource::Agent,
|
||||
status: CommandExecutionStatus::Completed,
|
||||
command_actions: vec![CommandAction::Unknown {
|
||||
command: "echo hello world".to_string(),
|
||||
command:
|
||||
"git -c 'http.extraHeader=Authorization: Bearer [REDACTED_SECRET]' push"
|
||||
.to_string(),
|
||||
}],
|
||||
aggregated_output: Some("hello world\n".to_string()),
|
||||
exit_code: Some(0),
|
||||
|
||||
@@ -10,7 +10,7 @@ use super::UserInput;
|
||||
use super::shared::v2_enum_from_core;
|
||||
use crate::JsonSchema;
|
||||
use crate::TS;
|
||||
use crate::protocol::item_builders::command_actions_for_path_uri;
|
||||
use crate::protocol::item_builders::CommandExecutionPresentation;
|
||||
use crate::protocol::item_builders::convert_patch_changes;
|
||||
use crate::protocol::item_builders::review_output_text;
|
||||
use codex_experimental_api_macros::ExperimentalApi;
|
||||
@@ -43,7 +43,6 @@ use codex_protocol::protocol::GuardianUserAuthorization as CoreGuardianUserAutho
|
||||
use codex_protocol::protocol::PatchApplyStatus as CorePatchApplyStatus;
|
||||
use codex_protocol::protocol::ReviewDecision as CoreReviewDecision;
|
||||
use codex_protocol::protocol::SubAgentActivityKind as CoreSubAgentActivityKind;
|
||||
use codex_shell_command::parse_command::shlex_join;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use serde::Deserialize;
|
||||
@@ -841,24 +840,31 @@ impl From<CoreTurnItem> for ThreadItem {
|
||||
summary: reasoning.summary_text,
|
||||
content: reasoning.raw_content,
|
||||
},
|
||||
CoreTurnItem::CommandExecution(command) => ThreadItem::CommandExecution {
|
||||
id: command.id,
|
||||
plugin_id: command.plugin_id,
|
||||
script_path: command.script_path,
|
||||
command: shlex_join(&command.command),
|
||||
cwd: command.cwd.clone().into(),
|
||||
process_id: command.process_id,
|
||||
source: command.source.into(),
|
||||
status: command.status.into(),
|
||||
command_actions: command_actions_for_path_uri(&command.parsed_cmd, &command.cwd),
|
||||
aggregated_output: command
|
||||
.aggregated_output
|
||||
.filter(|output| !output.is_empty()),
|
||||
exit_code: command.exit_code,
|
||||
duration_ms: command
|
||||
.duration
|
||||
.and_then(|duration| i64::try_from(duration.as_millis()).ok()),
|
||||
},
|
||||
CoreTurnItem::CommandExecution(command) => {
|
||||
let presentation = CommandExecutionPresentation::from_raw(
|
||||
&command.command,
|
||||
&command.parsed_cmd,
|
||||
&command.cwd,
|
||||
);
|
||||
ThreadItem::CommandExecution {
|
||||
id: command.id,
|
||||
plugin_id: command.plugin_id,
|
||||
script_path: command.script_path,
|
||||
command: presentation.command,
|
||||
cwd: command.cwd.clone().into(),
|
||||
process_id: command.process_id,
|
||||
source: command.source.into(),
|
||||
status: command.status.into(),
|
||||
command_actions: presentation.command_actions,
|
||||
aggregated_output: command
|
||||
.aggregated_output
|
||||
.filter(|output| !output.is_empty()),
|
||||
exit_code: command.exit_code,
|
||||
duration_ms: command
|
||||
.duration
|
||||
.and_then(|duration| i64::try_from(duration.as_millis()).ok()),
|
||||
}
|
||||
}
|
||||
CoreTurnItem::DynamicToolCall(call) => ThreadItem::DynamicToolCall {
|
||||
id: call.id,
|
||||
namespace: call.namespace,
|
||||
|
||||
@@ -2927,10 +2927,19 @@ fn core_turn_item_into_thread_item_converts_supported_variants() {
|
||||
plugin_id: Some("sample@openai-curated".to_string()),
|
||||
script_path: Some("scripts/run.py".to_string()),
|
||||
process_id: Some("pid-1".to_string()),
|
||||
command: vec!["echo".to_string(), "done".to_string()],
|
||||
command: vec![
|
||||
"git".to_string(),
|
||||
"-c".to_string(),
|
||||
"http.extraHeader=Authorization: Bearer example_synthetic_bearer_token_123456"
|
||||
.to_string(),
|
||||
"-c".to_string(),
|
||||
"http.extraHeader=X-Trace:example".to_string(),
|
||||
"push".to_string(),
|
||||
],
|
||||
cwd: PathUri::from_abs_path(&test_path_buf("/tmp").abs()),
|
||||
parsed_cmd: vec![codex_protocol::parse_command::ParsedCommand::Unknown {
|
||||
cmd: "echo done".to_string(),
|
||||
cmd: "git -c 'http.extraHeader=Authorization: Bearer example_synthetic_bearer_token_123456' -c http.extraHeader=X-Trace:example push"
|
||||
.to_string(),
|
||||
}],
|
||||
source: CoreExecCommandSource::Agent,
|
||||
interaction_input: None,
|
||||
@@ -2949,13 +2958,15 @@ fn core_turn_item_into_thread_item_converts_supported_variants() {
|
||||
id: "exec-1".to_string(),
|
||||
plugin_id: Some("sample@openai-curated".to_string()),
|
||||
script_path: Some("scripts/run.py".to_string()),
|
||||
command: "echo done".to_string(),
|
||||
command: "git -c 'http.extraHeader=Authorization: Bearer [REDACTED_SECRET]' -c 'http.extraHeader=X-Trace:example' push"
|
||||
.to_string(),
|
||||
cwd: LegacyAppPathString::from_abs_path(&test_path_buf("/tmp").abs()),
|
||||
process_id: Some("pid-1".to_string()),
|
||||
source: CommandExecutionSource::Agent,
|
||||
status: CommandExecutionStatus::Completed,
|
||||
command_actions: vec![CommandAction::Unknown {
|
||||
command: "echo done".to_string(),
|
||||
command: "git -c 'http.extraHeader=Authorization: Bearer [REDACTED_SECRET]' -c http.extraHeader=X-Trace:example push"
|
||||
.to_string(),
|
||||
}],
|
||||
aggregated_output: Some("done\n".to_string()),
|
||||
exit_code: Some(0),
|
||||
|
||||
Reference in new issue
Block a user