openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-29 19:08:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Preserve plugin attribution across command approvals (#35029)

Browse Source 
## What changed

- Add optional `plugin_id` and `script_path` fields to execution approval and guardian assessment events.
- Propagate validated plugin attribution through delegated approvals, guardian-reviewed command items, app-server notifications, thread history, and rollout traces.
- Preserve attribution on both started and completed command items, including declined commands.

## Testing

- Extend core, app-server, thread-history, and rollout-trace tests to cover plugin attribution propagation and serialization.

GitOrigin-RevId: 723684d010cab04142918d7a95e06ed95d008da1
This commit is contained in:
Kyle Brown
2026-07-23 23:13:59 +00:00
committed by copyberry
parent 091e4a5d7c
commit 9fc4e5a7aa
23 changed files with 363 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex_delegate.rs
+31 -6
View File
@@ -5,7 +5,10 @@ use async_channel::Receiver;
use async_channel::Sender;
use codex_analytics::GuardianApprovalRequestSource;
use codex_async_utils::OrCancelExt;
use codex_core_plugins::PluginCommandAttribution;
use codex_extension_api::LoadedUserInstructions;
use codex_plugin::PluginId;
use codex_protocol::items::is_safe_plugin_relative_path;
use codex_protocol::protocol::ApplyPatchApprovalRequestEvent;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
@@ -34,6 +37,7 @@ use tokio_util::sync::CancellationToken;
use crate::config::Config;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::GuardianReviewOptions;
use crate::guardian::new_guardian_review_id;
use crate::guardian::routes_approval_to_guardian;
use crate::guardian::routes_approval_to_guardian_with_reviewer;
@@ -492,6 +496,8 @@ async fn handle_exec_approval(
let approval_id_for_op = event.effective_approval_id();
let ExecApprovalRequestEvent {
call_id,
plugin_id,
script_path,
approval_id,
environment_id,
command,
@@ -503,6 +509,15 @@ async fn handle_exec_approval(
available_decisions,
..
} = event;
let plugin_attribution = plugin_id
.zip(script_path)
.and_then(|(plugin_id, script_path)| {
let plugin_id = PluginId::parse(&plugin_id).ok()?;
is_safe_plugin_relative_path(&script_path).then_some(PluginCommandAttribution {
plugin_id,
normalized_relative_path: script_path,
})
});
let decision = if routes_approval_to_guardian(parent_ctx) {
let review_cancel = cancel_token.child_token();
let review_rx = spawn_approval_request_review(
@@ -522,8 +537,11 @@ async fn handle_exec_approval(
justification: None,
},
reason,
GuardianApprovalRequestSource::DelegatedSubagent,
review_cancel.clone(),
GuardianReviewOptions {
plugin_attribution_override: plugin_attribution.clone(),
approval_request_source: GuardianApprovalRequestSource::DelegatedSubagent,
external_cancel: Some(review_cancel.clone()),
},
);
await_approval_with_cancel(
receive_approval_review(review_rx),
@@ -547,6 +565,7 @@ async fn handle_exec_approval(
proposed_execpolicy_amendment,
additional_permissions,
available_decisions,
plugin_attribution,
),
parent_session,
&approval_id_for_op,
@@ -630,8 +649,11 @@ async fn handle_patch_approval(
patch,
},
reason.clone(),
GuardianApprovalRequestSource::DelegatedSubagent,
review_cancel.clone(),
GuardianReviewOptions {
plugin_attribution_override: None,
approval_request_source: GuardianApprovalRequestSource::DelegatedSubagent,
external_cancel: Some(review_cancel.clone()),
},
);
Some(
await_approval_with_cancel(
@@ -746,8 +768,11 @@ async fn maybe_auto_review_mcp_request_user_input(
new_guardian_review_id(),
build_guardian_mcp_tool_review_request(&event.call_id, &invocation, metadata.as_ref()),
/*retry_reason*/ None,
GuardianApprovalRequestSource::DelegatedSubagent,
review_cancel.clone(),
GuardianReviewOptions {
plugin_attribution_override: None,
approval_request_source: GuardianApprovalRequestSource::DelegatedSubagent,
external_cancel: Some(review_cancel.clone()),
},
);
let decision = await_approval_with_cancel(
receive_approval_review(review_rx),
codex-rs/core/src/codex_delegate_tests.rs
+10
View File
@@ -363,6 +363,8 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
&parent_ctx,
ExecApprovalRequestEvent {
call_id: "command-item-1".to_string(),
plugin_id: Some("sample@openai-curated".to_string()),
script_path: Some("scripts/run.py".to_string()),
approval_id: Some("callback-approval-1".to_string()),
turn_id: "child-turn-1".to_string(),
environment_id: Some("remote".to_string()),
@@ -406,6 +408,14 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
assessment_event.target_item_id.as_deref(),
Some("command-item-1")
);
assert_eq!(
assessment_event.plugin_id.as_deref(),
Some("sample@openai-curated")
);
assert_eq!(
assessment_event.script_path.as_deref(),
Some("scripts/run.py")
);
assert_eq!(assessment_event.turn_id, parent_ctx.sub_id);
assert_eq!(
assessment_event.status,
codex-rs/core/src/guardian/mod.rs
+1
View File
@@ -28,6 +28,7 @@ pub(crate) use approval_request::GuardianMcpAnnotations;
pub(crate) use approval_request::GuardianNetworkAccessTrigger;
#[cfg(test)]
pub(crate) use approval_request::guardian_approval_request_to_json;
pub(crate) use review::GuardianReviewOptions;
pub(crate) use review::guardian_timeout_message;
pub(crate) use review::is_guardian_reviewer_source;
pub(crate) use review::new_guardian_review_id;
codex-rs/core/src/guardian/review.rs
+64 -12
View File
@@ -5,6 +5,7 @@ use codex_analytics::GuardianReviewFailureReason;
use codex_analytics::GuardianReviewTerminalStatus;
use codex_analytics::GuardianReviewTrackContext;
use codex_analytics::GuardianReviewedAction;
use codex_core_plugins::PluginCommandAttribution;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::CodexErrorInfo;
@@ -63,6 +64,32 @@ const GUARDIAN_TIMEOUT_INSTRUCTIONS: &str = concat!(
const GUARDIAN_REVIEW_MAX_ATTEMPTS: i64 = 3;
fn plugin_attribution_for_guardian_request(
turn: &TurnContext,
request: &GuardianApprovalRequest,
) -> Option<PluginCommandAttribution> {
match request {
GuardianApprovalRequest::Shell { command, cwd, .. }
| GuardianApprovalRequest::ExecCommand { command, cwd, .. } => {
turn.plugin_attribution_for_command(command, cwd)
}
#[cfg(unix)]
GuardianApprovalRequest::Execve {
program, argv, cwd, ..
} => {
let command = if argv.is_empty() {
vec![program.clone()]
} else {
std::iter::once(program.clone())
.chain(argv.iter().skip(1).cloned())
.collect()
};
turn.plugin_attribution_for_command(&command, cwd)
}
_ => None,
}
}
pub(crate) fn new_guardian_review_id() -> String {
uuid::Uuid::new_v4().to_string()
}
@@ -261,11 +288,21 @@ async fn run_guardian_review(
review_id: String,
request: GuardianApprovalRequest,
retry_reason: Option<String>,
approval_request_source: GuardianApprovalRequestSource,
external_cancel: Option<CancellationToken>,
options: GuardianReviewOptions,
) -> ReviewDecision {
let GuardianReviewOptions {
plugin_attribution_override,
approval_request_source,
external_cancel,
} = options;
let target_item_id = guardian_request_target_item_id(&request).map(str::to_string);
let assessment_turn_id = guardian_request_turn_id(&request, &turn.sub_id).to_string();
let plugin_attribution = plugin_attribution_override
.or_else(|| plugin_attribution_for_guardian_request(turn.as_ref(), &request));
let (plugin_id, script_path) = plugin_attribution
.as_ref()
.map(PluginCommandAttribution::serialized_fields)
.unzip();
let action_summary = guardian_assessment_action(&request);
let reviewed_action = guardian_reviewed_action(&request);
let review_tracking = GuardianReviewTrackContext::new(
@@ -284,6 +321,8 @@ async fn run_guardian_review(
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: review_id.clone(),
target_item_id: target_item_id.clone(),
plugin_id: plugin_id.clone(),
script_path: script_path.clone(),
turn_id: assessment_turn_id.clone(),
started_at_ms,
completed_at_ms: None,
@@ -321,6 +360,8 @@ async fn run_guardian_review(
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: review_id,
target_item_id,
plugin_id: plugin_id.clone(),
script_path: script_path.clone(),
turn_id: assessment_turn_id.clone(),
started_at_ms,
completed_at_ms: Some(completed_at_ms),
@@ -414,6 +455,8 @@ async fn run_guardian_review(
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: review_id,
target_item_id,
plugin_id: plugin_id.clone(),
script_path: script_path.clone(),
turn_id: assessment_turn_id.clone(),
started_at_ms,
completed_at_ms: Some(completed_at_ms),
@@ -449,6 +492,8 @@ async fn run_guardian_review(
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: review_id,
target_item_id,
plugin_id: plugin_id.clone(),
script_path: script_path.clone(),
turn_id: assessment_turn_id.clone(),
started_at_ms,
completed_at_ms: Some(completed_at_ms),
@@ -535,6 +580,8 @@ async fn run_guardian_review(
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: review_id,
target_item_id,
plugin_id: plugin_id.clone(),
script_path: script_path.clone(),
turn_id: assessment_turn_id.clone(),
started_at_ms,
completed_at_ms: Some(completed_at_ms),
@@ -568,6 +615,12 @@ async fn run_guardian_review(
}
}
pub(crate) struct GuardianReviewOptions {
pub(crate) plugin_attribution_override: Option<PluginCommandAttribution>,
pub(crate) approval_request_source: GuardianApprovalRequestSource,
pub(crate) external_cancel: Option<CancellationToken>,
}
/// Public entrypoint for approval requests that should be reviewed by guardian.
pub(crate) async fn review_approval_request(
session: &Arc<Session>,
@@ -584,8 +637,11 @@ pub(crate) async fn review_approval_request(
review_id,
request,
retry_reason,
GuardianApprovalRequestSource::MainTurn,
/*external_cancel*/ None,
GuardianReviewOptions {
plugin_attribution_override: None,
approval_request_source: GuardianApprovalRequestSource::MainTurn,
external_cancel: None,
},
))
.await
}
@@ -596,8 +652,7 @@ pub(crate) async fn review_approval_request_with_cancel(
review_id: String,
request: GuardianApprovalRequest,
retry_reason: Option<String>,
approval_request_source: GuardianApprovalRequestSource,
cancel_token: CancellationToken,
options: GuardianReviewOptions,
) -> ReviewDecision {
run_guardian_review(
Arc::clone(session),
@@ -605,8 +660,7 @@ pub(crate) async fn review_approval_request_with_cancel(
review_id,
request,
retry_reason,
approval_request_source,
Some(cancel_token),
options,
)
.await
}
@@ -617,8 +671,7 @@ pub(crate) fn spawn_approval_request_review(
review_id: String,
request: GuardianApprovalRequest,
retry_reason: Option<String>,
approval_request_source: GuardianApprovalRequestSource,
cancel_token: CancellationToken,
options: GuardianReviewOptions,
) -> oneshot::Receiver<ReviewDecision> {
let (tx, rx) = oneshot::channel();
let spawn_result = std::thread::Builder::new()
@@ -639,8 +692,7 @@ pub(crate) fn spawn_approval_request_review(
review_id,
request,
retry_reason,
approval_request_source,
cancel_token,
options,
));
let _ = tx.send(decision);
});
codex-rs/core/src/guardian/tests.rs
+5 -2
View File
@@ -1167,8 +1167,11 @@ async fn cancelled_guardian_review_emits_terminal_abort_without_warning() {
.to_string(),
},
/*retry_reason*/ None,
GuardianApprovalRequestSource::MainTurn,
cancel_token,
GuardianReviewOptions {
plugin_attribution_override: None,
approval_request_source: GuardianApprovalRequestSource::MainTurn,
external_cancel: Some(cancel_token),
},
)
.await;
codex-rs/core/src/session/elicitation_holders_tests.rs
+13 -1
View File
@@ -1,5 +1,7 @@
use std::collections::HashMap;
use codex_core_plugins::PluginCommandAttribution;
use codex_plugin::PluginId;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionProfile;
@@ -34,6 +36,10 @@ async fn command_approval_holds_an_elicitation_until_response() {
let mut pause_state = session.subscribe_elicitation_pause_state();
#[allow(deprecated)]
let cwd = turn_context.cwd.clone();
let plugin_attribution = PluginCommandAttribution {
plugin_id: PluginId::parse("sample@openai-curated").expect("valid plugin id"),
normalized_relative_path: "scripts/run.py".to_string(),
};
let request = tokio::spawn({
let session = session.clone();
@@ -52,12 +58,18 @@ async fn command_approval_holds_an_elicitation_until_response() {
/*proposed_execpolicy_amendment*/ None,
/*additional_permissions*/ None,
/*available_decisions*/ None,
/*plugin_attribution_override*/ Some(plugin_attribution),
)
.await
}
});
events.recv().await.expect("approval event");
let event = events.recv().await.expect("approval event");
let codex_protocol::protocol::EventMsg::ExecApprovalRequest(event) = event.msg else {
panic!("expected command approval event");
};
assert_eq!(event.plugin_id.as_deref(), Some("sample@openai-curated"));
assert_eq!(event.script_path.as_deref(), Some("scripts/run.py"));
wait_until_held(&mut pause_state).await;
session
.notify_approval("call-1", ReviewDecision::Approved)
codex-rs/core/src/session/mod.rs
+17 -2
View File
@@ -306,6 +306,7 @@ pub(crate) struct PreviousTurnSettings {
use crate::SkillMetadata;
use crate::SkillsService;
use crate::exec_policy::ExecPolicyUpdateError;
use crate::guardian::GuardianReviewOptions;
use crate::guardian::GuardianReviewSessionManager;
use crate::mcp::McpManager;
use crate::network_policy_decision::execpolicy_network_rule_amendment;
@@ -334,6 +335,7 @@ use crate::turn_timing::TurnTimingState;
use crate::turn_timing::record_turn_ttfm_metric;
use crate::unified_exec::UnifiedExecProcessManager;
use crate::windows_sandbox::WindowsSandboxLevelExt;
use codex_core_plugins::PluginCommandAttribution;
use codex_core_plugins::PluginsManager;
use codex_core_plugins::RecommendedPluginCandidatesInput;
use codex_git_utils::get_git_repo_root;
@@ -2276,6 +2278,7 @@ impl Session {
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
additional_permissions: Option<AdditionalPermissionProfile>,
available_decisions: Option<Vec<ReviewDecision>>,
plugin_attribution_override: Option<PluginCommandAttribution>,
) -> ReviewDecision {
let _elicitation = self.services.elicitations.register();
// command-level approvals use `call_id`.
@@ -2318,8 +2321,16 @@ impl Session {
additional_permissions.as_ref(),
)
});
let plugin_attribution = plugin_attribution_override
.or_else(|| turn_context.plugin_attribution_for_command(&command, &cwd));
let (plugin_id, script_path) = plugin_attribution
.as_ref()
.map(PluginCommandAttribution::serialized_fields)
.unzip();
let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
call_id,
plugin_id,
script_path,
approval_id,
turn_id: turn_context.sub_id.clone(),
environment_id,
@@ -2448,8 +2459,12 @@ impl Session {
review_id,
request,
/*retry_reason*/ None,
codex_analytics::GuardianApprovalRequestSource::MainTurn,
cancellation_token.clone(),
GuardianReviewOptions {
plugin_attribution_override: None,
approval_request_source:
codex_analytics::GuardianApprovalRequestSource::MainTurn,
external_cancel: Some(cancellation_token.clone()),
},
);
let decision = tokio::select! {
biased;
codex-rs/core/src/tools/network_approval.rs
+1
View File
@@ -687,6 +687,7 @@ impl NetworkApprovalService {
/*proposed_execpolicy_amendment*/ None,
/*additional_permissions*/ None,
available_decisions,
/*plugin_attribution_override*/ None,
)
.await
};
codex-rs/core/src/tools/runtimes/shell.rs
+1
View File
@@ -170,6 +170,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
.cloned(),
req.additional_permissions.clone(),
available_decisions,
/*plugin_attribution_override*/ None,
)
.await
})
codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
+1
View File
@@ -507,6 +507,7 @@ impl CoreShellActionProvider {
/*proposed_execpolicy_amendment*/ None,
additional_permissions,
Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
/*plugin_attribution_override*/ None,
)
.await
})
codex-rs/core/src/tools/runtimes/unified_exec.rs
+1
View File
@@ -209,6 +209,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
.cloned(),
req.additional_permissions.clone(),
available_decisions,
/*plugin_attribution_override*/ None,
)
.await
})