mirror of
https://github.com/openai/codex.git
synced 2026-09-13 11:47:17 +00:00
Include approval context in permission request hooks
This commit is contained in:
@@ -14,8 +14,10 @@ use codex_hooks::UserPromptSubmitOutcome;
|
||||
use codex_hooks::UserPromptSubmitRequest;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::models::DeveloperInstructions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::models::SandboxPermissions;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::HookCompletedEvent;
|
||||
@@ -155,6 +157,9 @@ pub(crate) async fn run_permission_request_hooks(
|
||||
run_id_suffix: String,
|
||||
tool_name: String,
|
||||
command: String,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<PermissionProfile>,
|
||||
justification: Option<String>,
|
||||
guardian_review: Option<PermissionRequestGuardianReview>,
|
||||
) -> Option<PermissionRequestDecision> {
|
||||
let request = PermissionRequestRequest {
|
||||
@@ -167,6 +172,9 @@ pub(crate) async fn run_permission_request_hooks(
|
||||
tool_name,
|
||||
run_id_suffix,
|
||||
command,
|
||||
sandbox_permissions,
|
||||
additional_permissions,
|
||||
justification,
|
||||
guardian_review,
|
||||
};
|
||||
let preview_runs = sess.hooks().preview_permission_request(&request);
|
||||
|
||||
@@ -421,6 +421,9 @@ impl ToolOrchestrator {
|
||||
approval_ctx.call_id.to_string(),
|
||||
permission_request.tool_name,
|
||||
permission_request.command,
|
||||
permission_request.sandbox_permissions,
|
||||
permission_request.additional_permissions,
|
||||
permission_request.justification,
|
||||
guardian_review
|
||||
.as_ref()
|
||||
.map(|review| permission_request_guardian_review(review.review.clone())),
|
||||
|
||||
@@ -199,7 +199,12 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
}
|
||||
|
||||
fn permission_request_payload(&self, req: &ShellRequest) -> Option<PermissionRequestPayload> {
|
||||
Some(PermissionRequestPayload::bash(req.hook_command.clone()))
|
||||
Some(PermissionRequestPayload::bash(
|
||||
req.hook_command.clone(),
|
||||
req.sandbox_permissions,
|
||||
req.additional_permissions.clone(),
|
||||
req.justification.clone(),
|
||||
))
|
||||
}
|
||||
|
||||
fn guardian_approval_request(
|
||||
|
||||
@@ -185,7 +185,12 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
&self,
|
||||
req: &UnifiedExecRequest,
|
||||
) -> Option<PermissionRequestPayload> {
|
||||
Some(PermissionRequestPayload::bash(req.hook_command.clone()))
|
||||
Some(PermissionRequestPayload::bash(
|
||||
req.hook_command.clone(),
|
||||
req.sandbox_permissions,
|
||||
req.additional_permissions.clone(),
|
||||
req.justification.clone(),
|
||||
))
|
||||
}
|
||||
|
||||
fn guardian_approval_request(
|
||||
|
||||
@@ -15,6 +15,7 @@ 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;
|
||||
@@ -129,13 +130,24 @@ 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>,
|
||||
}
|
||||
|
||||
impl PermissionRequestPayload {
|
||||
pub(crate) fn bash(command: String) -> Self {
|
||||
pub(crate) fn bash(
|
||||
command: String,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<PermissionProfile>,
|
||||
justification: Option<String>,
|
||||
) -> Self {
|
||||
Self {
|
||||
tool_name: "Bash".to_string(),
|
||||
command,
|
||||
sandbox_permissions,
|
||||
additional_permissions,
|
||||
justification,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,27 @@
|
||||
"$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"
|
||||
},
|
||||
"GuardianRiskLevel": {
|
||||
"enum": [
|
||||
"low",
|
||||
@@ -20,12 +41,49 @@
|
||||
],
|
||||
"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"
|
||||
},
|
||||
"PermissionRequestApprovalContext": {
|
||||
"additionalProperties": false,
|
||||
"properties": {
|
||||
"additional_permissions": {
|
||||
"$ref": "#/definitions/PermissionProfile"
|
||||
},
|
||||
"justification": {
|
||||
"type": "string"
|
||||
},
|
||||
"sandbox_permissions": {
|
||||
"$ref": "#/definitions/SandboxPermissions"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"sandbox_permissions"
|
||||
],
|
||||
"type": "object"
|
||||
},
|
||||
"PermissionRequestGuardianReview": {
|
||||
"additionalProperties": false,
|
||||
"properties": {
|
||||
@@ -106,9 +164,38 @@
|
||||
"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"
|
||||
},
|
||||
@@ -158,6 +245,7 @@
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"approval_context",
|
||||
"cwd",
|
||||
"guardian_review",
|
||||
"hook_event_name",
|
||||
|
||||
@@ -8,6 +8,8 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
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;
|
||||
@@ -22,6 +24,7 @@ use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::engine::output_parser;
|
||||
use crate::permission_review::PermissionRequestGuardianReview;
|
||||
use crate::schema::PermissionRequestApprovalContext;
|
||||
use crate::schema::PermissionRequestCommandInput;
|
||||
use crate::schema::PermissionRequestToolInput;
|
||||
|
||||
@@ -40,6 +43,9 @@ pub struct PermissionRequestRequest {
|
||||
/// still needs stable begin/end ids for hook UI and transcript bookkeeping.
|
||||
pub run_id_suffix: String,
|
||||
pub command: String,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub additional_permissions: Option<PermissionProfile>,
|
||||
pub justification: Option<String>,
|
||||
/// Advisory approval context from Codex's automated reviewer, when one ran.
|
||||
///
|
||||
/// A hook can use this as another signal, but it is not bound by the
|
||||
@@ -105,20 +111,7 @@ pub(crate) async fn run(
|
||||
// `Bash` shape even though the request carries `tool_name`, so later
|
||||
// tool support has to choose its own explicit schema instead of
|
||||
// accidentally inheriting Bash fields.
|
||||
let input_json = match serde_json::to_string(&PermissionRequestCommandInput {
|
||||
session_id: request.session_id.to_string(),
|
||||
turn_id: request.turn_id.clone(),
|
||||
transcript_path: crate::schema::NullableString::from_path(request.transcript_path.clone()),
|
||||
cwd: request.cwd.display().to_string(),
|
||||
hook_event_name: "PermissionRequest".to_string(),
|
||||
model: request.model.clone(),
|
||||
permission_mode: request.permission_mode.clone(),
|
||||
tool_name: "Bash".to_string(),
|
||||
tool_input: PermissionRequestToolInput {
|
||||
command: request.command.clone(),
|
||||
},
|
||||
guardian_review: request.guardian_review,
|
||||
}) {
|
||||
let input_json = match serde_json::to_string(&build_command_input(&request)) {
|
||||
Ok(input_json) => input_json,
|
||||
Err(error) => {
|
||||
let hook_events = common::serialization_failure_hook_events_for_tool_use(
|
||||
@@ -162,6 +155,28 @@ pub(crate) async fn run(
|
||||
}
|
||||
}
|
||||
|
||||
fn build_command_input(request: &PermissionRequestRequest) -> PermissionRequestCommandInput {
|
||||
PermissionRequestCommandInput {
|
||||
session_id: request.session_id.to_string(),
|
||||
turn_id: request.turn_id.clone(),
|
||||
transcript_path: crate::schema::NullableString::from_path(request.transcript_path.clone()),
|
||||
cwd: request.cwd.display().to_string(),
|
||||
hook_event_name: "PermissionRequest".to_string(),
|
||||
model: request.model.clone(),
|
||||
permission_mode: request.permission_mode.clone(),
|
||||
tool_name: "Bash".to_string(),
|
||||
tool_input: PermissionRequestToolInput {
|
||||
command: request.command.clone(),
|
||||
},
|
||||
approval_context: PermissionRequestApprovalContext {
|
||||
sandbox_permissions: request.sandbox_permissions,
|
||||
additional_permissions: request.additional_permissions.clone(),
|
||||
justification: request.justification.clone(),
|
||||
},
|
||||
guardian_review: request.guardian_review.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_completed(
|
||||
handler: &ConfiguredHandler,
|
||||
run_result: CommandRunResult,
|
||||
@@ -269,3 +284,87 @@ fn parse_completed(
|
||||
data: PermissionRequestHandlerData { decision },
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::SandboxPermissions;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
use super::PermissionRequestRequest;
|
||||
use super::build_command_input;
|
||||
|
||||
#[test]
|
||||
fn command_input_includes_approval_context_alongside_guardian_review() {
|
||||
let request = PermissionRequestRequest {
|
||||
session_id: ThreadId::new(),
|
||||
turn_id: "turn-123".to_string(),
|
||||
cwd: PathBuf::from("/repo"),
|
||||
transcript_path: Some(PathBuf::from("/tmp/transcript.jsonl")),
|
||||
model: "gpt-5".to_string(),
|
||||
permission_mode: "on-request".to_string(),
|
||||
tool_name: "Bash".to_string(),
|
||||
run_id_suffix: "call-123".to_string(),
|
||||
command: "cargo test -p codex-core".to_string(),
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
additional_permissions: Some(PermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
file_system: None,
|
||||
}),
|
||||
justification: Some("Need network and target writes".to_string()),
|
||||
guardian_review: Some(crate::permission_review::PermissionRequestGuardianReview {
|
||||
status: crate::permission_review::PermissionRequestGuardianReviewStatus::Approved,
|
||||
decision: Some(
|
||||
crate::permission_review::PermissionRequestGuardianReviewDecision::Allow,
|
||||
),
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
rationale: Some("Scoped escalation looks reasonable".to_string()),
|
||||
}),
|
||||
};
|
||||
|
||||
let actual = serde_json::to_value(build_command_input(&request))
|
||||
.expect("serialize permission request input");
|
||||
|
||||
assert_eq!(
|
||||
actual,
|
||||
json!({
|
||||
"session_id": request.session_id.to_string(),
|
||||
"turn_id": "turn-123",
|
||||
"transcript_path": "/tmp/transcript.jsonl",
|
||||
"cwd": "/repo",
|
||||
"hook_event_name": "PermissionRequest",
|
||||
"model": "gpt-5",
|
||||
"permission_mode": "on-request",
|
||||
"tool_name": "Bash",
|
||||
"tool_input": {
|
||||
"command": "cargo test -p codex-core",
|
||||
},
|
||||
"approval_context": {
|
||||
"sandbox_permissions": "with_additional_permissions",
|
||||
"additional_permissions": {
|
||||
"file_system": null,
|
||||
"network": {
|
||||
"enabled": true,
|
||||
},
|
||||
},
|
||||
"justification": "Need network and target writes",
|
||||
},
|
||||
"guardian_review": {
|
||||
"status": "approved",
|
||||
"decision": "allow",
|
||||
"risk_level": null,
|
||||
"user_authorization": null,
|
||||
"rationale": "Scoped escalation looks reasonable",
|
||||
},
|
||||
})
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,6 +12,9 @@ use serde_json::Value;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::SandboxPermissions;
|
||||
|
||||
use crate::permission_review::PermissionRequestGuardianReview;
|
||||
|
||||
const GENERATED_DIR: &str = "generated";
|
||||
@@ -237,6 +240,14 @@ pub(crate) struct PermissionRequestToolInput {
|
||||
pub command: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub(crate) struct PermissionRequestApprovalContext {
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub additional_permissions: Option<PermissionProfile>,
|
||||
pub justification: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
#[schemars(rename = "permission-request.command.input")]
|
||||
@@ -254,6 +265,7 @@ pub(crate) struct PermissionRequestCommandInput {
|
||||
#[schemars(schema_with = "permission_request_tool_name_schema")]
|
||||
pub tool_name: String,
|
||||
pub tool_input: PermissionRequestToolInput,
|
||||
pub approval_context: PermissionRequestApprovalContext,
|
||||
#[schemars(
|
||||
schema_with = "crate::permission_review::nullable_permission_request_guardian_review_schema"
|
||||
)]
|
||||
|
||||
Reference in New Issue
Block a user