mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Skip sandboxed shell commands in Guardian v2 by default (#39631)
## What changed - Exclude sandboxed `exec_command` and `shell_command` calls from Guardian v2 classification by default while continuing to classify calls that request `require_escalated` permissions. - Add `guardianv2.review_scope.sandboxed_exec_commands` to opt sandboxed shell commands back into classification. - Keep other tools and namespaced shell tools in scope, and advance tool-call progress when a call is skipped. ## Testing - Cover the default and configured review scopes, tool namespaces, permission modes, and skipped-call progress tracking. GitOrigin-RevId: 32fb540c69959b9a82569f0f2fc76b5517496e6b
This commit is contained in:
@@ -1299,6 +1299,9 @@
|
||||
"reasoning_effort": {
|
||||
"$ref": "#/definitions/ReasoningEffort"
|
||||
},
|
||||
"review_scope": {
|
||||
"$ref": "#/definitions/GuardianV2ReviewScopeConfigToml"
|
||||
},
|
||||
"review_threshold": {
|
||||
"format": "double",
|
||||
"maximum": 1.0,
|
||||
@@ -1311,6 +1314,17 @@
|
||||
},
|
||||
"type": "object"
|
||||
},
|
||||
"GuardianV2ReviewScopeConfigToml": {
|
||||
"additionalProperties": false,
|
||||
"description": "Optional tool-call categories available to the Guardian v2 classifier.",
|
||||
"properties": {
|
||||
"sandboxed_exec_commands": {
|
||||
"description": "Include sandboxed shell command calls in Guardian v2 classification.",
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"type": "object"
|
||||
},
|
||||
"GuardianV2TranscriptConfigToml": {
|
||||
"additionalProperties": false,
|
||||
"description": "Bounds and optional sources for the Guardian v2 conversation transcript.",
|
||||
|
||||
@@ -33,6 +33,7 @@ pub(crate) struct GuardianV2Config {
|
||||
pub(crate) max_action_tokens: usize,
|
||||
pub(crate) max_classifier_instruction_tokens: usize,
|
||||
pub(crate) max_parent_compaction_tokens: usize,
|
||||
pub(crate) sandboxed_exec_commands: bool,
|
||||
pub(crate) transcript: TranscriptConfig,
|
||||
}
|
||||
|
||||
@@ -233,6 +234,11 @@ impl GuardianV2Config {
|
||||
max_action_tokens,
|
||||
max_classifier_instruction_tokens,
|
||||
max_parent_compaction_tokens,
|
||||
sandboxed_exec_commands: configured
|
||||
.review_scope
|
||||
.as_ref()
|
||||
.and_then(|review_scope| review_scope.sandboxed_exec_commands)
|
||||
.unwrap_or(false),
|
||||
transcript: TranscriptConfig {
|
||||
sources: transcript_config
|
||||
.and_then(|transcript| transcript.sources.clone())
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use codex_features::GuardianV2ConfigToml;
|
||||
use codex_features::GuardianV2ReviewScopeConfigToml;
|
||||
use codex_protocol::openai_models::GuardianV2ModelConfig;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::TruncationPolicy;
|
||||
@@ -8,6 +9,26 @@ use super::DEFAULT_CLASSIFIER_INSTRUCTIONS;
|
||||
use super::GuardianV2Config;
|
||||
use crate::async_scorer::transcript::truncate_entry;
|
||||
|
||||
#[test]
|
||||
fn sandboxed_exec_commands_are_excluded_by_default() {
|
||||
let config = GuardianV2Config::from_overrides(GuardianV2ConfigToml::default()).unwrap();
|
||||
|
||||
assert!(!config.sandboxed_exec_commands);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandboxed_exec_commands_can_be_included() {
|
||||
let config = GuardianV2Config::from_overrides(GuardianV2ConfigToml {
|
||||
review_scope: Some(GuardianV2ReviewScopeConfigToml {
|
||||
sandboxed_exec_commands: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert!(config.sandboxed_exec_commands);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn template_policy_is_substituted_before_the_single_truncation() {
|
||||
for max_tokens in [256, 1_000, 2_000] {
|
||||
|
||||
@@ -49,6 +49,32 @@ struct GuardianAction {
|
||||
payload: ToolPayload,
|
||||
}
|
||||
|
||||
fn should_classify_tool(
|
||||
tool_name: &ToolName,
|
||||
payload: &ToolPayload,
|
||||
sandboxed_exec_commands: bool,
|
||||
) -> bool {
|
||||
if sandboxed_exec_commands
|
||||
|| !tool_name.is_default_namespace()
|
||||
|| !matches!(tool_name.name.as_str(), "exec_command" | "shell_command")
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
matches!(
|
||||
payload,
|
||||
ToolPayload::Function { arguments }
|
||||
if serde_json::from_str::<serde_json::Value>(arguments)
|
||||
.ok()
|
||||
.is_some_and(|arguments| {
|
||||
arguments
|
||||
.get("sandbox_permissions")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
== Some("require_escalated")
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
impl GuardianAction {
|
||||
fn render(self, max_action_tokens: usize) -> serde_json::Result<String> {
|
||||
let arguments = match self.payload {
|
||||
@@ -401,6 +427,16 @@ impl GuardianV2Extension {
|
||||
let Some(score_progress) = input.thread_store.get::<GuardianV2ScoreProgress>() else {
|
||||
return;
|
||||
};
|
||||
if !should_classify_tool(
|
||||
input.tool_name,
|
||||
input.payload,
|
||||
guardian_config.sandboxed_exec_commands,
|
||||
) {
|
||||
score_progress
|
||||
.latest_tool_call
|
||||
.fetch_add(/*val*/ 1, Ordering::Relaxed);
|
||||
return;
|
||||
}
|
||||
let metrics = score_progress.metrics.clone();
|
||||
let sampled_at = SystemTime::now();
|
||||
let parent_model = input.thread_store.get::<ModelInfo>();
|
||||
|
||||
@@ -57,6 +57,7 @@ use super::REVIEW_FALLBACK_METRIC;
|
||||
use super::StrictReviewReason;
|
||||
use super::TOOL_CALL_LAG_METRIC;
|
||||
use super::encrypted_parent_compaction;
|
||||
use super::should_classify_tool;
|
||||
use crate::async_scorer::config::DEFAULT_MODEL_CONTEXT_ITEM_TOKENS;
|
||||
use crate::async_scorer::config::DEFAULT_PARENT_COMPACTION_TOKENS;
|
||||
use crate::async_scorer::sampler::CLASSIFICATION_TOKEN_USAGE_METRIC;
|
||||
@@ -256,6 +257,91 @@ fn fail_closed_score_preserves_classification_order() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn sandboxed_shell_classification_respects_review_scope() -> Result<()> {
|
||||
let sandboxed = ToolPayload::Function {
|
||||
arguments: r#"{"cmd":"pwd"}"#.to_owned(),
|
||||
};
|
||||
let additional_permissions = ToolPayload::Function {
|
||||
arguments: r#"{"cmd":"pwd","sandbox_permissions":"with_additional_permissions"}"#
|
||||
.to_owned(),
|
||||
};
|
||||
let unsandboxed = ToolPayload::Function {
|
||||
arguments: r#"{"cmd":"pwd","sandbox_permissions":"require_escalated"}"#.to_owned(),
|
||||
};
|
||||
|
||||
for tool_name in ["exec_command", "shell_command"] {
|
||||
let tool_name = ToolName::plain(tool_name);
|
||||
assert!(!should_classify_tool(
|
||||
&tool_name, &sandboxed, /*sandboxed_exec_commands*/ false
|
||||
));
|
||||
assert!(!should_classify_tool(
|
||||
&tool_name,
|
||||
&additional_permissions,
|
||||
/*sandboxed_exec_commands*/ false
|
||||
));
|
||||
assert!(should_classify_tool(
|
||||
&tool_name,
|
||||
&unsandboxed,
|
||||
/*sandboxed_exec_commands*/ false
|
||||
));
|
||||
assert!(should_classify_tool(
|
||||
&tool_name, &sandboxed, /*sandboxed_exec_commands*/ true
|
||||
));
|
||||
}
|
||||
assert!(should_classify_tool(
|
||||
&ToolName::plain("read_file"),
|
||||
&sandboxed,
|
||||
/*sandboxed_exec_commands*/ false
|
||||
));
|
||||
assert!(should_classify_tool(
|
||||
&ToolName::namespaced("mcp", "exec_command"),
|
||||
&sandboxed,
|
||||
/*sandboxed_exec_commands*/ false
|
||||
));
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let fixture = GuardianFailureFixture::new().await?;
|
||||
let thread_store = fixture.test.codex.thread_extension_data();
|
||||
let score_progress = thread_store
|
||||
.get::<GuardianV2ScoreProgress>()
|
||||
.expect("Guardian v2 should track score progress per thread");
|
||||
let latest_scored_tool_call = score_progress
|
||||
.latest_scored_tool_call
|
||||
.load(Ordering::Acquire);
|
||||
let turn_store = ExtensionData::new("turn-1");
|
||||
let tool_name = ToolName::plain("exec_command");
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: r#"{"cmd":"pwd"}"#.to_owned(),
|
||||
};
|
||||
|
||||
fixture.registry.tool_lifecycle_contributors()[0]
|
||||
.on_tool_start(ToolStartInput {
|
||||
session_store: &fixture.session_store,
|
||||
thread_store,
|
||||
turn_store: &turn_store,
|
||||
turn_id: "turn-1",
|
||||
call_id: "call-2",
|
||||
tool_name: &tool_name,
|
||||
payload: &payload,
|
||||
conversation_history: Arc::new(TestConversationHistory(Vec::new())),
|
||||
source: ToolCallSource::Direct,
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
score_progress.latest_tool_call.load(Ordering::Acquire),
|
||||
latest_scored_tool_call + 1
|
||||
);
|
||||
assert_eq!(
|
||||
score_progress
|
||||
.latest_scored_tool_call
|
||||
.load(Ordering::Acquire),
|
||||
latest_scored_tool_call
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn encrypted_parent_compaction_preserves_the_latest_valid_item() {
|
||||
let older = ResponseItem::Compaction {
|
||||
|
||||
@@ -108,6 +108,15 @@ pub struct GuardianV2TranscriptConfigToml {
|
||||
pub max_recent_non_user_entries: Option<usize>,
|
||||
}
|
||||
|
||||
/// Optional tool-call categories available to the Guardian v2 classifier.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct GuardianV2ReviewScopeConfigToml {
|
||||
/// Include sandboxed shell command calls in Guardian v2 classification.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub sandboxed_exec_commands: Option<bool>,
|
||||
}
|
||||
|
||||
/// User-configurable prompt, approval, and context settings for Guardian v2.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
@@ -133,6 +142,8 @@ pub struct GuardianV2ConfigToml {
|
||||
#[schemars(range(min = 100, max = 100000))]
|
||||
pub max_parent_compaction_tokens: Option<usize>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub review_scope: Option<GuardianV2ReviewScopeConfigToml>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub transcript: Option<GuardianV2TranscriptConfigToml>,
|
||||
}
|
||||
|
||||
|
||||
@@ -22,6 +22,7 @@ pub use feature_configs::CurrentTimeReminderConfigToml;
|
||||
pub use feature_configs::CurrentTimeReminderDeliveryMode;
|
||||
pub use feature_configs::CurrentTimeSource;
|
||||
pub use feature_configs::GuardianV2ConfigToml;
|
||||
pub use feature_configs::GuardianV2ReviewScopeConfigToml;
|
||||
pub use feature_configs::GuardianV2TranscriptConfigToml;
|
||||
pub use feature_configs::GuardianV2TranscriptSource;
|
||||
pub use feature_configs::MultiAgentV2ConfigToml;
|
||||
|
||||
@@ -162,6 +162,9 @@ max_action_tokens = 512
|
||||
max_classifier_instruction_tokens = 256
|
||||
max_parent_compaction_tokens = 384
|
||||
|
||||
[guardianv2.review_scope]
|
||||
sandboxed_exec_commands = true
|
||||
|
||||
[guardianv2.transcript]
|
||||
sources = ["tool_outputs", "reasoning"]
|
||||
include_images = true
|
||||
@@ -185,6 +188,9 @@ max_recent_non_user_entries = 12
|
||||
max_action_tokens: Some(512),
|
||||
max_classifier_instruction_tokens: Some(256),
|
||||
max_parent_compaction_tokens: Some(384),
|
||||
review_scope: Some(crate::GuardianV2ReviewScopeConfigToml {
|
||||
sandboxed_exec_commands: Some(true),
|
||||
}),
|
||||
transcript: Some(crate::GuardianV2TranscriptConfigToml {
|
||||
sources: Some(vec![
|
||||
crate::GuardianV2TranscriptSource::ToolOutputs,
|
||||
|
||||
Reference in New Issue
Block a user