mirrors/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-28 02:11:08 +03:00
Code Issues Packages Projects Releases Wiki Activity

Trim PermissionRequest hook inputs

Browse Source 
Keep PermissionRequest hook payloads focused on tool identity and the actionable command details. For Bash and exec_command hooks, plumb request justification into tool_input.description when present. For NetworkAccess hooks, pass the originating command and a network-access <domain> description instead of the old approval context envelope.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Abhinav Vedmala
2026-04-13 16:24:45 -07:00
parent 2563661366
commit 04294e0038
12 changed files with 83 additions and 321 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
codex-rs/core/src/hook_runtime.rs
View File

@@ -160,8 +160,8 @@ pub(crate) async fn run_permission_request_hooks(
request_id: &str,
payload: PermissionRequestPayload,
approval_attempt: PermissionRequestApprovalAttempt,
retry_reason: Option<String>,
network_approval_context: Option<NetworkApprovalContext>,
_retry_reason: Option<String>,
_network_approval_context: Option<NetworkApprovalContext>,
) -> Option<PermissionRequestDecision> {
let request = PermissionRequestRequest {
session_id: sess.conversation_id,
@@ -173,12 +173,7 @@ pub(crate) async fn run_permission_request_hooks(
tool_name: payload.tool_name,
run_id_suffix: format!("{request_id}:{}", approval_attempt.as_wire_value()),
command: payload.command,
sandbox_permissions: payload.sandbox_permissions,
additional_permissions: payload.additional_permissions,
justification: payload.justification,
approval_attempt,
retry_reason,
network_approval_context,
description: payload.description,
};
let preview_runs = sess.hooks().preview_permission_request(&request);
emit_hook_started_events(sess, turn_context, preview_runs).await;

17
codex-rs/core/src/tools/network_approval.rs
View File

@@ -7,7 +7,6 @@ use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::hook_runtime::run_permission_request_hooks;
use crate::network_policy_decision::denied_network_policy_message;
use crate::sandboxing::SandboxPermissions;
use crate::tools::sandboxing::PermissionRequestPayload;
use crate::tools::sandboxing::ToolError;
use codex_hooks::PermissionRequestApprovalAttempt;
@@ -50,6 +49,7 @@ pub(crate) enum NetworkApprovalMode {
pub(crate) struct NetworkApprovalSpec {
pub network: Option<NetworkProxy>,
pub mode: NetworkApprovalMode,
pub command: String,
}
#[derive(Clone, Debug)]
@@ -179,6 +179,7 @@ impl PendingHostApproval {
struct ActiveNetworkApprovalCall {
registration_id: String,
turn_id: String,
command: String,
}
pub(crate) struct NetworkApprovalService {
@@ -211,7 +212,7 @@ impl NetworkApprovalService {
other_approved_hosts.extend(approved_hosts.iter().cloned());
}
async fn register_call(&self, registration_id: String, turn_id: String) {
async fn register_call(&self, registration_id: String, turn_id: String, command: String) {
let mut active_calls = self.active_calls.lock().await;
let key = registration_id.clone();
active_calls.insert(
@@ -219,6 +220,7 @@ impl NetworkApprovalService {
Arc::new(ActiveNetworkApprovalCall {
registration_id,
turn_id,
command,
}),
);
}
@@ -379,16 +381,17 @@ impl NetworkApprovalService {
let owner_call = self.resolve_single_active_call().await;
let guardian_approval_id = Self::approval_id_for_key(&key);
let prompt_command = vec!["network-access".to_string(), target.clone()];
let command = owner_call
.as_ref()
.map_or_else(|| prompt_command.join(" "), |call| call.command.clone());
if let Some(permission_request_decision) = run_permission_request_hooks(
&session,
&turn_context,
&guardian_approval_id,
PermissionRequestPayload {
tool_name: NETWORK_ACCESS_HOOK_TOOL_NAME.to_string(),
command: prompt_command.join(" "),
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
justification: Some(prompt_reason.clone()),
command,
description: Some(format!("network-access {}", request.host)),
},
PermissionRequestApprovalAttempt::Initial,
/*retry_reason*/ None,
@@ -637,7 +640,7 @@ pub(crate) async fn begin_network_approval(
session
.services
.network_approval
.register_call(registration_id.clone(), turn_id.to_string())
.register_call(registration_id.clone(), turn_id.to_string(), spec.command)
.await;
Some(ActiveNetworkApproval {

24
codex-rs/core/src/tools/network_approval_tests.rs
View File

@@ -211,7 +211,11 @@ fn denied_blocked_request(host: &str) -> BlockedRequest {
async fn record_blocked_request_sets_policy_outcome_for_owner_call() {
let service = NetworkApprovalService::default();
service
.register_call("registration-1".to_string(), "turn-1".to_string())
.register_call(
"registration-1".to_string(),
"turn-1".to_string(),
"curl http://example.com".to_string(),
)
.await;
service
@@ -230,7 +234,11 @@ async fn record_blocked_request_sets_policy_outcome_for_owner_call() {
async fn blocked_request_policy_does_not_override_user_denial_outcome() {
let service = NetworkApprovalService::default();
service
.register_call("registration-1".to_string(), "turn-1".to_string())
.register_call(
"registration-1".to_string(),
"turn-1".to_string(),
"curl http://example.com".to_string(),
)
.await;
service
@@ -250,10 +258,18 @@ async fn blocked_request_policy_does_not_override_user_denial_outcome() {
async fn record_blocked_request_ignores_ambiguous_unattributed_blocked_requests() {
let service = NetworkApprovalService::default();
service
.register_call("registration-1".to_string(), "turn-1".to_string())
.register_call(
"registration-1".to_string(),
"turn-1".to_string(),
"curl http://example.com".to_string(),
)
.await;
service
.register_call("registration-2".to_string(), "turn-1".to_string())
.register_call(
"registration-2".to_string(),
"turn-1".to_string(),
"gh api /foo".to_string(),
)
.await;
service

5
codex-rs/core/src/tools/runtimes/shell.rs
View File

@@ -203,9 +203,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
Some(PermissionRequestPayload {
tool_name: "Bash".to_string(),
command: req.hook_command.clone(),
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
description: req.justification.clone(),
})
}
@@ -224,6 +222,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
Some(NetworkApprovalSpec {
network: req.network.clone(),
mode: NetworkApprovalMode::Immediate,
command: req.hook_command.clone(),
})
}

4
codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
View File

@@ -403,9 +403,7 @@ impl CoreShellActionProvider {
let permission_request = PermissionRequestPayload {
tool_name: "Bash".to_string(),
command: codex_shell_command::parse_command::shlex_join(&command),
sandbox_permissions: self.approval_sandbox_permissions,
additional_permissions: additional_permissions.clone(),
justification: None,
description: None,
};
let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone());
match run_permission_request_hooks(

7
codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs
View File

@@ -448,11 +448,8 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul
expected_hook_command
);
assert_eq!(
hook_inputs[0]["approval_context"]["attempt"],
serde_json::json!({
"stage": "initial",
"retryReason": null,
})
hook_inputs[0]["tool_input"]["description"],
serde_json::Value::Null
);
Ok(())

5
codex-rs/core/src/tools/runtimes/unified_exec.rs
View File

@@ -186,9 +186,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
Some(PermissionRequestPayload {
tool_name: "Bash".to_string(),
command: req.hook_command.clone(),
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
description: req.justification.clone(),
})
}
@@ -207,6 +205,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
Some(NetworkApprovalSpec {
network: req.network.clone(),
mode: NetworkApprovalMode::Deferred,
command: req.hook_command.clone(),
})
}

5
codex-rs/core/src/tools/sandboxing.rs
View File

@@ -14,7 +14,6 @@ use codex_network_proxy::NetworkProxy;
use codex_protocol::approvals::ExecPolicyAmendment;
use codex_protocol::approvals::NetworkApprovalContext;
use codex_protocol::error::CodexErr;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
@@ -136,9 +135,7 @@ pub(crate) struct ApprovalCtx<'a> {
pub(crate) struct PermissionRequestPayload {
pub tool_name: String,
pub command: String,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<PermissionProfile>,
pub justification: Option<String>,
pub description: Option<String>,
}
// Specifies what tool orchestrator should do with a given tool call.

98
codex-rs/core/tests/suite/hooks.rs
View File

@@ -1172,20 +1172,15 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() ->
assert_eq!(hook_inputs[0]["tool_name"], "Bash");
assert_eq!(hook_inputs[0]["tool_input"]["command"], command);
assert_eq!(
hook_inputs[0]["approval_context"],
serde_json::json!({
"attempt": {
"stage": "initial",
"retryReason": null,
},
"policy": {
"sandboxPermissions": "use_default",
"additionalPermissions": null,
},
"justification": null,
"resource": {},
})
hook_inputs[0]["tool_input"]["description"],
serde_json::Value::Null
);
assert!(hook_inputs[0].get("approval_attempt").is_none());
assert!(hook_inputs[0].get("sandbox_permissions").is_none());
assert!(hook_inputs[0].get("additional_permissions").is_none());
assert!(hook_inputs[0].get("justification").is_none());
assert!(hook_inputs[0].get("host").is_none());
assert!(hook_inputs[0].get("protocol").is_none());
assert!(
hook_inputs[0].get("tool_use_id").is_none(),
"PermissionRequest input should not include a tool_use_id",
@@ -1207,9 +1202,12 @@ async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> {
let call_id = "permissionrequest-exec-command";
let marker = std::env::temp_dir().join("permissionrequest-exec-command-marker");
let command = format!("rm -f {}", marker.display());
let justification = "remove the temporary marker";
let args = serde_json::json!({
"cmd": command,
"login": true,
"sandbox_permissions": "require_escalated",
"justification": justification,
});
let responses = mount_sse_sequence(
&server,
@@ -1261,7 +1259,7 @@ async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> {
test.submit_turn_with_policies(
"run the exec command after hook approval",
AskForApproval::OnRequest,
codex_protocol::protocol::SandboxPolicy::DangerFullAccess,
codex_protocol::protocol::SandboxPolicy::new_read_only_policy(),
)
.await?;
@@ -1278,21 +1276,13 @@ async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> {
assert_eq!(hook_inputs[0]["hook_event_name"], "PermissionRequest");
assert_eq!(hook_inputs[0]["tool_name"], "Bash");
assert_eq!(hook_inputs[0]["tool_input"]["command"], command);
assert_eq!(
hook_inputs[0]["approval_context"],
serde_json::json!({
"attempt": {
"stage": "initial",
"retryReason": null,
},
"policy": {
"sandboxPermissions": "use_default",
"additionalPermissions": null,
},
"justification": null,
"resource": {},
})
);
assert_eq!(hook_inputs[0]["tool_input"]["description"], justification);
assert!(hook_inputs[0].get("approval_attempt").is_none());
assert!(hook_inputs[0].get("sandbox_permissions").is_none());
assert!(hook_inputs[0].get("additional_permissions").is_none());
assert!(hook_inputs[0].get("justification").is_none());
assert!(hook_inputs[0].get("host").is_none());
assert!(hook_inputs[0].get("protocol").is_none());
Ok(())
}
@@ -1446,28 +1436,17 @@ allow_local_binding = true
assert_eq!(hook_inputs.len(), 1);
assert_eq!(hook_inputs[0]["hook_event_name"], "PermissionRequest");
assert_eq!(hook_inputs[0]["tool_name"], "NetworkAccess");
assert_eq!(hook_inputs[0]["tool_input"]["command"], command);
assert_eq!(
hook_inputs[0]["tool_input"]["command"],
"network-access http://codex-network-test.invalid:80"
);
assert_eq!(
hook_inputs[0]["approval_context"],
serde_json::json!({
"attempt": {
"stage": "initial",
"retryReason": null,
},
"policy": {
"sandboxPermissions": "use_default",
"additionalPermissions": null,
},
"justification": "codex-network-test.invalid is not in the allowed_domains",
"resource": {
"host": "codex-network-test.invalid",
"protocol": "http",
},
})
hook_inputs[0]["tool_input"]["description"],
"network-access codex-network-test.invalid"
);
assert!(hook_inputs[0].get("approval_attempt").is_none());
assert!(hook_inputs[0].get("sandbox_permissions").is_none());
assert!(hook_inputs[0].get("additional_permissions").is_none());
assert!(hook_inputs[0].get("justification").is_none());
assert!(hook_inputs[0].get("host").is_none());
assert!(hook_inputs[0].get("protocol").is_none());
test.codex.submit(Op::Shutdown {}).await?;
wait_for_event(&test.codex, |event| {
@@ -1550,20 +1529,15 @@ async fn permission_request_hook_sees_retry_context_after_sandbox_denial() -> Re
assert_eq!(hook_inputs[0]["hook_event_name"], "PermissionRequest");
assert_eq!(hook_inputs[0]["tool_input"]["command"], command);
assert_eq!(
hook_inputs[0]["approval_context"],
serde_json::json!({
"attempt": {
"stage": "retry",
"retryReason": "command failed; retry without sandbox?",
},
"policy": {
"sandboxPermissions": "use_default",
"additionalPermissions": null,
},
"justification": null,
"resource": {},
})
hook_inputs[0]["tool_input"]["description"],
serde_json::Value::Null
);
assert!(hook_inputs[0].get("approval_attempt").is_none());
assert!(hook_inputs[0].get("sandbox_permissions").is_none());
assert!(hook_inputs[0].get("additional_permissions").is_none());
assert!(hook_inputs[0].get("justification").is_none());
assert!(hook_inputs[0].get("host").is_none());
assert!(hook_inputs[0].get("protocol").is_none());
Ok(())
}

154
codex-rs/hooks/schema/generated/permission-request.command.input.schema.json
View File

@@ -2,176 +2,29 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"additionalProperties": false,
"definitions": {
"AbsolutePathBuf": {
"description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.",
"type": "string"
},
"FileSystemPermissions": {
"properties": {
"read": {
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": "array"
},
"write": {
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
"type": "array"
}
},
"type": "object"
},
"NetworkApprovalProtocol": {
"enum": [
"http",
"https",
"socks5_tcp",
"socks5_udp"
],
"type": "string"
},
"NetworkPermissions": {
"properties": {
"enabled": {
"type": "boolean"
}
},
"type": "object"
},
"NullableString": {
"type": [
"string",
"null"
]
},
"PermissionProfile": {
"properties": {
"file_system": {
"$ref": "#/definitions/FileSystemPermissions"
},
"network": {
"$ref": "#/definitions/NetworkPermissions"
}
},
"type": "object"
},
"PermissionRequestApprovalAttempt": {
"enum": [
"initial",
"retry"
],
"type": "string"
},
"PermissionRequestApprovalContext": {
"additionalProperties": false,
"properties": {
"attempt": {
"$ref": "#/definitions/PermissionRequestAttemptContext"
},
"justification": {
"type": "string"
},
"policy": {
"$ref": "#/definitions/PermissionRequestPolicyContext"
},
"resource": {
"$ref": "#/definitions/PermissionRequestResourceContext"
}
},
"required": [
"attempt",
"policy",
"resource"
],
"type": "object"
},
"PermissionRequestAttemptContext": {
"additionalProperties": false,
"properties": {
"retryReason": {
"type": "string"
},
"stage": {
"$ref": "#/definitions/PermissionRequestApprovalAttempt"
}
},
"required": [
"stage"
],
"type": "object"
},
"PermissionRequestPolicyContext": {
"additionalProperties": false,
"properties": {
"additionalPermissions": {
"$ref": "#/definitions/PermissionProfile"
},
"sandboxPermissions": {
"$ref": "#/definitions/SandboxPermissions"
}
},
"required": [
"sandboxPermissions"
],
"type": "object"
},
"PermissionRequestResourceContext": {
"additionalProperties": false,
"properties": {
"host": {
"type": "string"
},
"protocol": {
"$ref": "#/definitions/NetworkApprovalProtocol"
}
},
"type": "object"
},
"PermissionRequestToolInput": {
"additionalProperties": false,
"properties": {
"command": {
"type": "string"
},
"description": {
"type": "string"
}
},
"required": [
"command"
],
"type": "object"
},
"SandboxPermissions": {
"description": "Controls the per-command sandbox override requested by a shell-like tool call.",
"oneOf": [
{
"description": "Run with the turn's configured sandbox policy unchanged.",
"enum": [
"use_default"
],
"type": "string"
},
{
"description": "Request to run outside the sandbox.",
"enum": [
"require_escalated"
],
"type": "string"
},
{
"description": "Request to stay in the sandbox while widening permissions for this command only.",
"enum": [
"with_additional_permissions"
],
"type": "string"
}
]
}
},
"properties": {
"approval_context": {
"$ref": "#/definitions/PermissionRequestApprovalContext"
},
"cwd": {
"type": "string"
},
@@ -214,7 +67,6 @@
}
},
"required": [
"approval_context",
"cwd",
"hook_event_name",
"model",

32
codex-rs/hooks/src/events/permission_request.rs
View File

@@ -16,9 +16,6 @@
use std::path::PathBuf;
use codex_protocol::ThreadId;
use codex_protocol::approvals::NetworkApprovalContext;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::SandboxPermissions;
use codex_protocol::protocol::HookCompletedEvent;
use codex_protocol::protocol::HookEventName;
use codex_protocol::protocol::HookOutputEntry;
@@ -35,11 +32,7 @@ use crate::engine::ConfiguredHandler;
use crate::engine::command_runner::CommandRunResult;
use crate::engine::dispatcher;
use crate::engine::output_parser;
use crate::schema::PermissionRequestApprovalContext;
use crate::schema::PermissionRequestAttemptContext;
use crate::schema::PermissionRequestCommandInput;
use crate::schema::PermissionRequestPolicyContext;
use crate::schema::PermissionRequestResourceContext;
use crate::schema::PermissionRequestToolInput;
#[derive(Debug, Clone)]
@@ -53,12 +46,7 @@ pub struct PermissionRequestRequest {
pub tool_name: String,
pub run_id_suffix: String,
pub command: String,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<PermissionProfile>,
pub justification: Option<String>,
pub approval_attempt: PermissionRequestApprovalAttempt,
pub retry_reason: Option<String>,
pub network_approval_context: Option<NetworkApprovalContext>,
pub description: Option<String>,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
@@ -198,11 +186,6 @@ fn resolve_permission_request_decision<'a>(
}
fn build_command_input(request: &PermissionRequestRequest) -> PermissionRequestCommandInput {
let (host, protocol) = request
.network_approval_context
.as_ref()
.map(|context| (Some(context.host.clone()), Some(context.protocol)))
.unwrap_or((None, None));
PermissionRequestCommandInput {
session_id: request.session_id.to_string(),
turn_id: request.turn_id.clone(),
@@ -214,18 +197,7 @@ fn build_command_input(request: &PermissionRequestRequest) -> PermissionRequestC
tool_name: request.tool_name.clone(),
tool_input: PermissionRequestToolInput {
command: request.command.clone(),
},
approval_context: PermissionRequestApprovalContext {
attempt: PermissionRequestAttemptContext {
stage: request.approval_attempt,
retry_reason: request.retry_reason.clone(),
},
policy: PermissionRequestPolicyContext {
sandbox_permissions: request.sandbox_permissions,
additional_permissions: request.additional_permissions.clone(),
},
justification: request.justification.clone(),
resource: PermissionRequestResourceContext { host, protocol },
description: request.description.clone(),
},
}
}

42
codex-rs/hooks/src/schema.rs
View File

@@ -12,12 +12,6 @@ use serde_json::Value;
use std::path::Path;
use std::path::PathBuf;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::SandboxPermissions;
use crate::events::permission_request::PermissionRequestApprovalAttempt;
const GENERATED_DIR: &str = "generated";
const POST_TOOL_USE_INPUT_FIXTURE: &str = "post-tool-use.command.input.schema.json";
const POST_TOOL_USE_OUTPUT_FIXTURE: &str = "post-tool-use.command.output.schema.json";
@@ -248,41 +242,8 @@ pub(crate) struct PreToolUseCommandInput {
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestToolInput {
pub command: String,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestAttemptContext {
pub stage: PermissionRequestApprovalAttempt,
pub retry_reason: Option<String>,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestPolicyContext {
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<PermissionProfile>,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestResourceContext {
#[serde(skip_serializing_if = "Option::is_none")]
pub host: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub protocol: Option<NetworkApprovalProtocol>,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestApprovalContext {
pub attempt: PermissionRequestAttemptContext,
pub policy: PermissionRequestPolicyContext,
pub justification: Option<String>,
pub resource: PermissionRequestResourceContext,
pub description: Option<String>,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
@@ -302,7 +263,6 @@ pub(crate) struct PermissionRequestCommandInput {
#[schemars(schema_with = "permission_request_tool_name_schema")]
pub tool_name: String,
pub tool_input: PermissionRequestToolInput,
pub approval_context: PermissionRequestApprovalContext,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]