mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Honor model-provided Guardian review policies (#42744)
## What changed - Add optional per-model Guardian modes for computer use, shell, code mode, file changes, MCP, network, and permission requests. - Apply disabled, synchronous, or adaptive review by action category while preserving legacy configuration when model policy is absent. - Bind cached risk scores to the active model policy, thread settings, and environment selections so outdated scores cannot approve later actions. - Keep unknown review modes on the synchronous path and ignore unknown policy fields for forward compatibility. ## Testing - Cover policy serialization, legacy fallback, scoped tool classification, disabled categories, and score invalidation after policy changes. GitOrigin-RevId: f334a5f4fffb198da66cde3a400233e0df650516
This commit is contained in:
@@ -69,6 +69,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo {
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -116,6 +116,10 @@ async fn resumed_thread_does_not_wait_for_guardian_websocket_warmup() -> Result<
|
||||
.with_extra_config("[features.guardianv2]\nenabled = true")
|
||||
.enable_feature(Feature::GuardianApproval)
|
||||
.write(codex_home.path())?;
|
||||
let config = load_default_config_for_test(&codex_home).await;
|
||||
let mut model = codex_core::test_support::construct_model_info_offline(MODEL, &config);
|
||||
model.node_repl_auto_review_required = true;
|
||||
write_models_cache_with_models(codex_home.path(), vec![model])?;
|
||||
let thread_id = create_fake_rollout(
|
||||
codex_home.path(),
|
||||
"2025-01-05T12-00-00",
|
||||
|
||||
@@ -102,6 +102,7 @@ async fn models_client_hits_models_endpoint() {
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: true,
|
||||
node_repl_disabled: true,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -832,6 +832,9 @@ impl Session {
|
||||
thread_extension_init.insert(codex_extension_api::ThreadOriginator(
|
||||
session_configuration.originator.clone(),
|
||||
));
|
||||
// Publish the already resolved model before extensions make startup decisions.
|
||||
// Turn construction refreshes this attachment when the selected model changes.
|
||||
thread_extension_init.insert(model_info);
|
||||
let mcp_thread_init = thread_extension_init.clone();
|
||||
let thread_extension_data = codex_extension_api::ExtensionData::new_with_init(
|
||||
thread_id.to_string(),
|
||||
|
||||
@@ -107,12 +107,18 @@ fn check_legacy_model_safety(
|
||||
// TurnMetadataState pins both node REPL flags. Guardian prompt/evidence
|
||||
// construction also reads node_repl_auto_review_required from the turn.
|
||||
if retained_models.iter().any(|model| {
|
||||
model.node_repl_auto_review_required != destination.node_repl_auto_review_required
|
||||
model.computer_use_review_required() != destination.computer_use_review_required()
|
||||
}) {
|
||||
return Err(
|
||||
"the destination changes the admitted node REPL review requirement".to_string(),
|
||||
);
|
||||
}
|
||||
if retained_models
|
||||
.iter()
|
||||
.any(|model| model.guardian != destination.guardian)
|
||||
{
|
||||
return Err("the destination changes the admitted Guardian coverage".to_string());
|
||||
}
|
||||
if retained_models
|
||||
.iter()
|
||||
.any(|model| model.node_repl_disabled != destination.node_repl_disabled)
|
||||
@@ -130,7 +136,7 @@ fn check_legacy_model_safety(
|
||||
return Err("the destination changes the explicit Guardian reviewer model".to_string());
|
||||
}
|
||||
|
||||
if admitted_config.features.enabled(Feature::GuardianV2)
|
||||
if (admitted_config.features.enabled(Feature::GuardianV2) || destination.guardian.is_some())
|
||||
&& admitted_config.features.enabled(Feature::GuardianApproval)
|
||||
{
|
||||
// GuardianV2Extension::on_tool_start reads the parent ModelInfo from
|
||||
|
||||
@@ -1069,14 +1069,7 @@ async fn track_turn_resolved_config_analytics(
|
||||
.services
|
||||
.thread_extension_data
|
||||
.get::<codex_extension_api::GuardianV2Enabled>()
|
||||
.is_some_and(|state| {
|
||||
state.computer_use_only
|
||||
|| !turn_context
|
||||
.config
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.auto_review_required_for_model(&turn_context.model_info().slug)
|
||||
}),
|
||||
.is_some(),
|
||||
sandbox_network_access: turn_context.network_sandbox_policy().is_enabled(),
|
||||
collaboration_mode: turn_context.mode(),
|
||||
personality: turn_context.personality(),
|
||||
|
||||
@@ -650,6 +650,7 @@ fn remote_model_with_auto_review_override(slug: &str, review_model: &str) -> Mod
|
||||
supports_image_detail_original: false,
|
||||
context_window: Some(272_000),
|
||||
max_context_window: None,
|
||||
guardian: None,
|
||||
auto_compact_token_limit: None,
|
||||
comp_hash: None,
|
||||
effective_context_window_percent: 95,
|
||||
|
||||
@@ -121,6 +121,7 @@ fn test_model_info(
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
@@ -1358,6 +1359,7 @@ async fn model_switch_to_smaller_model_updates_token_context_window() -> Result<
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -539,6 +539,7 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo {
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -673,6 +673,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
@@ -803,6 +804,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -610,6 +610,7 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> {
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
@@ -879,6 +880,7 @@ async fn remote_models_apply_legacy_instructions() -> Result<()> {
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
@@ -1466,6 +1468,7 @@ fn test_remote_model_with_policy(
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -2771,6 +2771,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -91,6 +91,7 @@ fn test_model_info(
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -1623,6 +1623,7 @@ async fn view_image_tool_returns_unsupported_message_for_text_only_model() -> an
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -11,10 +11,7 @@ use crate::ConversationHistorySnapshot;
|
||||
use crate::ExtensionData;
|
||||
|
||||
/// Thread-local state installed only after Guardian V2's async classifier initializes.
|
||||
pub struct GuardianV2Enabled {
|
||||
/// Computer-use classification remains active for models requiring synchronous review.
|
||||
pub computer_use_only: bool,
|
||||
}
|
||||
pub struct GuardianV2Enabled;
|
||||
|
||||
/// Classification returned by an approval reviewer.
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
|
||||
@@ -1,12 +1,15 @@
|
||||
//! Binds cached classifier results to the user authorization they evaluated.
|
||||
//! Binds cached classifier results to the user authorization and model policy they evaluated.
|
||||
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::GuardianAuthorizationVersion;
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub(super) struct ScoreAuthorization {
|
||||
pub(super) settings: codex_protocol::protocol::ThreadSettingsSnapshot,
|
||||
pub(super) environments: Vec<codex_protocol::protocol::TurnEnvironmentSelection>,
|
||||
pub(super) local: GuardianAuthorizationVersion,
|
||||
pub(super) root: Option<GuardianAuthorizationVersion>,
|
||||
pub(super) model: Option<std::sync::Arc<codex_protocol::openai_models::ModelInfo>>,
|
||||
}
|
||||
|
||||
impl ScoreAuthorization {
|
||||
@@ -16,8 +19,17 @@ impl ScoreAuthorization {
|
||||
.await
|
||||
.map(|snapshot| snapshot.authorization_version);
|
||||
Self {
|
||||
settings: thread.thread_settings_snapshot().await,
|
||||
environments: thread
|
||||
.config_snapshot()
|
||||
.await
|
||||
.environment_selections()
|
||||
.to_vec(),
|
||||
local: thread.guardian_authorization_version().await,
|
||||
root,
|
||||
model: thread
|
||||
.thread_extension_data()
|
||||
.get::<codex_protocol::openai_models::ModelInfo>(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ use codex_features::GuardianV2TranscriptConfigToml;
|
||||
use codex_protocol::openai_models::GuardianV2ModelConfig;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
|
||||
use super::coverage::GuardianPolicy;
|
||||
use super::transcript::MAX_MESSAGE_ENTRY_TOKENS;
|
||||
use super::transcript::MAX_MESSAGE_TRANSCRIPT_TOKENS;
|
||||
use super::transcript::MAX_RECENT_NON_USER_ENTRIES;
|
||||
@@ -24,12 +25,6 @@ const DEFAULT_MAX_TOOL_CALL_LAG: usize = 2;
|
||||
pub(crate) const DEFAULT_CLASSIFIER_INSTRUCTIONS: &str = include_str!("classifier_instructions.md");
|
||||
pub(crate) const CLASSIFICATION_OUTPUT_INSTRUCTIONS: &str = "Your first output token is the entire classification: `high` for high risk or `low` for low risk. Output that token immediately and nothing else.";
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub(crate) enum GuardianV2ReviewScope {
|
||||
Standard { sandboxed_exec_commands: bool },
|
||||
ComputerUseOnly,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub(crate) struct GuardianV2Config {
|
||||
local_overrides: GuardianV2ConfigToml,
|
||||
@@ -43,7 +38,9 @@ pub(crate) struct GuardianV2Config {
|
||||
pub(crate) max_classifier_instruction_tokens: Option<usize>,
|
||||
pub(crate) reuse_parent_compaction: bool,
|
||||
pub(crate) max_parent_compaction_tokens: usize,
|
||||
pub(crate) review_scope: GuardianV2ReviewScope,
|
||||
pub(super) policy: GuardianPolicy,
|
||||
force_synchronous_review: bool,
|
||||
scoring_disabled: bool,
|
||||
pub(crate) transcript: TranscriptConfig,
|
||||
}
|
||||
|
||||
@@ -67,7 +64,28 @@ impl GuardianV2Config {
|
||||
None => GuardianV2ConfigToml::default(),
|
||||
};
|
||||
|
||||
Self::from_overrides(configured)
|
||||
let mut resolved = Self::from_overrides(configured)?;
|
||||
resolved.force_synchronous_review = config
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.approvals_reviewer
|
||||
.can_set(&codex_protocol::config_types::ApprovalsReviewer::User)
|
||||
.is_err();
|
||||
resolved.scoring_disabled = !config.features.enabled(codex_features::Feature::GuardianV2);
|
||||
Ok(resolved)
|
||||
}
|
||||
|
||||
pub(super) fn policy_for_model(
|
||||
&self,
|
||||
model: Option<&codex_protocol::openai_models::ModelInfo>,
|
||||
) -> GuardianPolicy {
|
||||
let mut policy = self.policy.for_model(model);
|
||||
if self.force_synchronous_review
|
||||
|| self.scoring_disabled && model.is_none_or(|model| model.guardian.is_none())
|
||||
{
|
||||
policy.disable_scoring();
|
||||
}
|
||||
policy
|
||||
}
|
||||
|
||||
pub(crate) fn with_model_defaults(
|
||||
@@ -147,6 +165,9 @@ impl GuardianV2Config {
|
||||
|
||||
let mut resolved = Self::from_overrides(configured)?;
|
||||
resolved.local_overrides = self.local_overrides.clone();
|
||||
resolved.policy = self.policy.clone();
|
||||
resolved.force_synchronous_review = self.force_synchronous_review;
|
||||
resolved.scoring_disabled = self.scoring_disabled;
|
||||
Ok(resolved)
|
||||
}
|
||||
|
||||
@@ -228,6 +249,7 @@ impl GuardianV2Config {
|
||||
);
|
||||
}
|
||||
|
||||
let policy = GuardianPolicy::from_legacy(configured.review_scope.as_ref());
|
||||
Ok(Self {
|
||||
local_overrides: configured.clone(),
|
||||
persist_scores: configured.persist_scores.unwrap_or(false),
|
||||
@@ -243,22 +265,9 @@ impl GuardianV2Config {
|
||||
max_classifier_instruction_tokens,
|
||||
reuse_parent_compaction: configured.reuse_parent_compaction.unwrap_or(true),
|
||||
max_parent_compaction_tokens,
|
||||
review_scope: if configured
|
||||
.review_scope
|
||||
.as_ref()
|
||||
.and_then(|review_scope| review_scope.computer_use_only)
|
||||
.unwrap_or(true)
|
||||
{
|
||||
GuardianV2ReviewScope::ComputerUseOnly
|
||||
} else {
|
||||
GuardianV2ReviewScope::Standard {
|
||||
sandboxed_exec_commands: configured
|
||||
.review_scope
|
||||
.as_ref()
|
||||
.and_then(|review_scope| review_scope.sandboxed_exec_commands)
|
||||
.unwrap_or(false),
|
||||
}
|
||||
},
|
||||
policy,
|
||||
force_synchronous_review: false,
|
||||
scoring_disabled: false,
|
||||
transcript: TranscriptConfig {
|
||||
sources: transcript_config
|
||||
.and_then(|transcript| transcript.sources.clone())
|
||||
|
||||
@@ -9,14 +9,14 @@ use pretty_assertions::assert_eq;
|
||||
use super::CLASSIFICATION_OUTPUT_INSTRUCTIONS;
|
||||
use super::DEFAULT_CLASSIFIER_INSTRUCTIONS;
|
||||
use super::GuardianV2Config;
|
||||
use super::GuardianV2ReviewScope;
|
||||
use crate::async_scorer::coverage::GuardianPolicy;
|
||||
use crate::async_scorer::transcript::truncate_entry;
|
||||
|
||||
#[test]
|
||||
fn review_scope_is_computer_use_only_by_default() {
|
||||
let config = GuardianV2Config::from_overrides(GuardianV2ConfigToml::default()).unwrap();
|
||||
|
||||
assert_eq!(config.review_scope, GuardianV2ReviewScope::ComputerUseOnly);
|
||||
assert_eq!(config.policy, GuardianPolicy::from_legacy(/*scope*/ None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -31,10 +31,11 @@ fn sandboxed_exec_commands_can_be_included() {
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
config.review_scope,
|
||||
GuardianV2ReviewScope::Standard {
|
||||
sandboxed_exec_commands: true,
|
||||
}
|
||||
config.policy,
|
||||
GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(false),
|
||||
sandboxed_exec_commands: Some(true),
|
||||
}))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -49,7 +50,7 @@ fn computer_use_only_takes_precedence_over_sandboxed_exec_commands() {
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(config.review_scope, GuardianV2ReviewScope::ComputerUseOnly);
|
||||
assert_eq!(config.policy, GuardianPolicy::from_legacy(/*scope*/ None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
//! Owns Guardian's category policy and translates legacy config at the boundary.
|
||||
//! Scoring and approval consume the same policy; host-required reviews still take precedence.
|
||||
|
||||
use codex_extension_api::ToolPayload;
|
||||
use codex_features::GuardianV2ReviewScopeConfigToml;
|
||||
use codex_protocol::ToolName;
|
||||
use codex_protocol::openai_models::GuardianModelPolicy;
|
||||
use codex_protocol::openai_models::GuardianReviewMode;
|
||||
use codex_protocol::openai_models::GuardianScope;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
|
||||
use GuardianReviewMode::Adaptive;
|
||||
use GuardianReviewMode::Disabled;
|
||||
use GuardianReviewMode::Synchronous;
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq)]
|
||||
pub(super) enum UnscoredAction {
|
||||
Ignore,
|
||||
AgeScore,
|
||||
InvalidateScore,
|
||||
}
|
||||
|
||||
/// Canonical runtime policy, including the existing scorer's compatibility behavior.
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub(super) struct GuardianPolicy {
|
||||
pub(super) categories: GuardianModelPolicy,
|
||||
pub(super) other_tools: GuardianReviewMode,
|
||||
pub(super) unscored_action: UnscoredAction,
|
||||
pub(super) initial_cua_call: bool,
|
||||
sandboxed_exec_commands: bool,
|
||||
}
|
||||
|
||||
impl GuardianPolicy {
|
||||
pub(super) fn from_legacy(scope: Option<&GuardianV2ReviewScopeConfigToml>) -> Self {
|
||||
let computer_use_only = scope
|
||||
.and_then(|scope| scope.computer_use_only)
|
||||
.unwrap_or(/*default*/ true);
|
||||
let other = if computer_use_only {
|
||||
Synchronous
|
||||
} else {
|
||||
Adaptive
|
||||
};
|
||||
Self {
|
||||
categories: GuardianModelPolicy {
|
||||
computer_use: Some(Adaptive),
|
||||
shell: Some(other),
|
||||
code_mode: Some(other),
|
||||
file_changes: Some(other),
|
||||
mcp: Some(other),
|
||||
network: Some(other),
|
||||
permissions: Some(other),
|
||||
},
|
||||
other_tools: other,
|
||||
unscored_action: if computer_use_only {
|
||||
UnscoredAction::Ignore
|
||||
} else {
|
||||
UnscoredAction::AgeScore
|
||||
},
|
||||
initial_cua_call: computer_use_only,
|
||||
sandboxed_exec_commands: !computer_use_only
|
||||
&& scope
|
||||
.and_then(|scope| scope.sandboxed_exec_commands)
|
||||
.unwrap_or(/*default*/ false),
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn for_model(&self, model: Option<&ModelInfo>) -> Self {
|
||||
match model.and_then(|model| model.guardian.as_ref()) {
|
||||
Some(categories) => Self {
|
||||
categories: categories.clone(),
|
||||
other_tools: Disabled,
|
||||
unscored_action: UnscoredAction::InvalidateScore,
|
||||
initial_cua_call: categories.computer_use == Some(Adaptive),
|
||||
sandboxed_exec_commands: true,
|
||||
},
|
||||
None => {
|
||||
let mut policy = self.clone();
|
||||
if policy.initial_cua_call
|
||||
&& model.is_some_and(|model| !model.node_repl_auto_review_required)
|
||||
{
|
||||
policy.categories.computer_use = Some(Synchronous);
|
||||
policy.initial_cua_call = false;
|
||||
policy.unscored_action = UnscoredAction::InvalidateScore;
|
||||
}
|
||||
policy
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn mode(&self, scope: GuardianScope) -> GuardianReviewMode {
|
||||
self.categories.review_mode(scope)
|
||||
}
|
||||
|
||||
pub(super) fn scoring_enabled(&self) -> bool {
|
||||
[
|
||||
self.categories.computer_use,
|
||||
self.categories.shell,
|
||||
self.categories.code_mode,
|
||||
self.categories.file_changes,
|
||||
self.categories.mcp,
|
||||
self.categories.network,
|
||||
self.categories.permissions,
|
||||
]
|
||||
.contains(&Some(Adaptive))
|
||||
}
|
||||
|
||||
pub(super) fn disable_scoring(&mut self) {
|
||||
for mode in [
|
||||
&mut self.categories.computer_use,
|
||||
&mut self.categories.shell,
|
||||
&mut self.categories.code_mode,
|
||||
&mut self.categories.file_changes,
|
||||
&mut self.categories.mcp,
|
||||
&mut self.categories.network,
|
||||
&mut self.categories.permissions,
|
||||
] {
|
||||
if *mode == Some(Adaptive) {
|
||||
*mode = Some(Synchronous);
|
||||
}
|
||||
}
|
||||
self.other_tools = Synchronous;
|
||||
}
|
||||
|
||||
pub(super) fn enforce_required_model(&mut self) {
|
||||
let computer_use = self.categories.computer_use;
|
||||
self.disable_scoring();
|
||||
if self.initial_cua_call {
|
||||
self.categories.computer_use = computer_use;
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn scores_tool(
|
||||
&self,
|
||||
tool: &ToolName,
|
||||
payload: &ToolPayload,
|
||||
scope: Option<GuardianScope>,
|
||||
) -> bool {
|
||||
if scope.map_or(self.other_tools, |scope| self.mode(scope)) != Adaptive {
|
||||
return false;
|
||||
}
|
||||
if self.sandboxed_exec_commands
|
||||
|| !tool.is_default_namespace()
|
||||
|| tool.name != "exec_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")
|
||||
}))
|
||||
}
|
||||
|
||||
pub(super) fn review_scope(action: &serde_json::Value) -> Option<GuardianScope> {
|
||||
match action.get("tool").and_then(serde_json::Value::as_str)? {
|
||||
"mcp_tool_call" => action
|
||||
.get("server")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(GuardianScope::for_mcp_server),
|
||||
"network_access" => Some(GuardianScope::Network),
|
||||
tool => GuardianScope::for_tool(&ToolName::plain(tool)),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -31,8 +31,6 @@ use codex_extension_api::ThreadOriginator;
|
||||
use codex_extension_api::ThreadStartInput;
|
||||
use codex_extension_api::ToolLifecycleContributor;
|
||||
use codex_extension_api::ToolLifecycleFuture;
|
||||
use codex_extension_api::ToolName;
|
||||
use codex_extension_api::ToolPayload;
|
||||
use codex_extension_api::ToolStartInput;
|
||||
use codex_features::Feature;
|
||||
use codex_guardian_context::ContextTarget;
|
||||
@@ -43,7 +41,8 @@ use codex_model_provider::create_model_provider;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::mcp::is_node_repl_backed_server;
|
||||
use codex_protocol::mcp::is_node_repl_backed_tool;
|
||||
use codex_protocol::openai_models::GuardianReviewMode;
|
||||
use codex_protocol::openai_models::GuardianScope;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::TruncationPolicy;
|
||||
@@ -54,7 +53,8 @@ use super::action::GuardianAction;
|
||||
use super::action::RenderedAction;
|
||||
use super::authorization::ScoreAuthorization;
|
||||
use super::config::GuardianV2Config;
|
||||
use super::config::GuardianV2ReviewScope;
|
||||
use super::coverage::GuardianPolicy;
|
||||
use super::coverage::UnscoredAction;
|
||||
use super::metrics::REVIEW_FALLBACK_METRIC;
|
||||
use super::metrics::TOOL_CALL_LAG_METRIC;
|
||||
use super::metrics::record_classification;
|
||||
@@ -71,38 +71,6 @@ use super::trusted_skills::TrustedSkillInvocations;
|
||||
use super::trusted_skills::TrustedSkillRoots;
|
||||
use super::trusted_tools::trusted_tool_context;
|
||||
|
||||
fn should_classify_tool(
|
||||
tool_name: &ToolName,
|
||||
payload: &ToolPayload,
|
||||
review_scope: GuardianV2ReviewScope,
|
||||
) -> bool {
|
||||
let GuardianV2ReviewScope::Standard {
|
||||
sandboxed_exec_commands,
|
||||
} = review_scope
|
||||
else {
|
||||
return is_node_repl_backed_tool(&tool_name.name, tool_name.namespace.as_deref());
|
||||
};
|
||||
if sandboxed_exec_commands
|
||||
|| !tool_name.is_default_namespace()
|
||||
|| tool_name.name != "exec_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")
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
/// Explains why Guardian v2 requires synchronous approval review.
|
||||
#[derive(Debug, Eq, PartialEq)]
|
||||
pub enum StrictReviewReason {
|
||||
@@ -141,12 +109,11 @@ impl ThreadLifecycleContributor<Config> for GuardianV2Extension {
|
||||
input: ThreadStartInput<'a, Config>,
|
||||
) -> ExtensionFuture<'a, ()> {
|
||||
Box::pin(async move {
|
||||
if !input.config.features.enabled(Feature::GuardianV2)
|
||||
|| !input.config.features.enabled(Feature::GuardianApproval)
|
||||
{
|
||||
if !input.config.features.enabled(Feature::GuardianApproval) {
|
||||
return;
|
||||
}
|
||||
|
||||
let model = input.thread_store.get::<ModelInfo>();
|
||||
let thread_id = input.thread_store.level_id().to_string();
|
||||
let guardian_config = match GuardianV2Config::resolve(input.config) {
|
||||
Ok(config) => config,
|
||||
@@ -159,6 +126,17 @@ impl ThreadLifecycleContributor<Config> for GuardianV2Extension {
|
||||
return;
|
||||
}
|
||||
};
|
||||
let mut policy = guardian_config.policy_for_model(model.as_deref());
|
||||
if model.as_ref().is_some_and(|model| {
|
||||
input
|
||||
.config
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.auto_review_required_for_model(&model.slug)
|
||||
}) {
|
||||
policy.enforce_required_model();
|
||||
}
|
||||
let scoring_enabled = policy.scoring_enabled();
|
||||
let luna_compaction_hash = if let Some(thread_manager) = self.thread_manager.upgrade() {
|
||||
thread_manager
|
||||
.get_models_manager()
|
||||
@@ -192,7 +170,7 @@ impl ThreadLifecycleContributor<Config> for GuardianV2Extension {
|
||||
metrics: input.extension_metrics.clone(),
|
||||
};
|
||||
|
||||
if guardian_config.transcript.include_images {
|
||||
if scoring_enabled && guardian_config.transcript.include_images {
|
||||
input
|
||||
.thread_store
|
||||
.get_or_init(NodeReplReviewEvidence::default)
|
||||
@@ -202,10 +180,6 @@ impl ThreadLifecycleContributor<Config> for GuardianV2Extension {
|
||||
let sampler = input
|
||||
.thread_store
|
||||
.get_or_init(|| LunaSampler::new(sampler_config));
|
||||
let guardian_v2_enabled = GuardianV2Enabled {
|
||||
computer_use_only: guardian_config.review_scope
|
||||
== GuardianV2ReviewScope::ComputerUseOnly,
|
||||
};
|
||||
input.thread_store.insert(guardian_config);
|
||||
input.thread_store.insert(GuardianV2ScoreProgress {
|
||||
metrics: input.extension_metrics.clone(),
|
||||
@@ -218,11 +192,14 @@ impl ThreadLifecycleContributor<Config> for GuardianV2Extension {
|
||||
input
|
||||
.thread_store
|
||||
.insert(TrustedSkillRoots::from_config(input.config));
|
||||
input.thread_store.insert(guardian_v2_enabled);
|
||||
if scoring_enabled {
|
||||
input.thread_store.insert(GuardianV2Enabled);
|
||||
}
|
||||
|
||||
// Keep the sampler available for later automatic review, but do not
|
||||
// prewarm while User approval mode or Full Access is selected.
|
||||
if input.config.approvals_reviewer == ApprovalsReviewer::AutoReview
|
||||
if scoring_enabled
|
||||
&& input.config.approvals_reviewer == ApprovalsReviewer::AutoReview
|
||||
&& !has_full_access(
|
||||
input.config.permissions.approval_policy.value(),
|
||||
&input.config.permissions.effective_permission_profile(),
|
||||
@@ -273,30 +250,22 @@ impl ApprovalReviewContributor for GuardianV2Extension {
|
||||
extension_metrics: Option<Arc<dyn ExtensionMetrics>>,
|
||||
) -> ExtensionFuture<'a, Option<ReviewDecision>> {
|
||||
Box::pin(async move {
|
||||
thread_store.get::<GuardianV2Enabled>()?;
|
||||
let model = thread_store.get::<ModelInfo>();
|
||||
let guardian_config = thread_store.get::<GuardianV2Config>()?;
|
||||
let policy = guardian_config.policy_for_model(model.as_deref());
|
||||
let action = serde_json::from_str::<serde_json::Value>(prompt).unwrap_or_default();
|
||||
let scope = GuardianPolicy::review_scope(&action);
|
||||
if scope.map_or(policy.other_tools, |scope| policy.mode(scope))
|
||||
!= GuardianReviewMode::Adaptive
|
||||
{
|
||||
record_fast_decision(extension_metrics.as_deref(), "deferred", "out_of_scope");
|
||||
return None;
|
||||
}
|
||||
let guardian_evidence = thread_store.get_or_init(GuardianReviewEvidence::default);
|
||||
let thread_context_enabled = guardian_evidence.uses_thread_owned_context();
|
||||
let mut initial_cua_call = false;
|
||||
if guardian_config.review_scope == GuardianV2ReviewScope::ComputerUseOnly {
|
||||
let Ok(action) = serde_json::from_str::<serde_json::Value>(prompt) else {
|
||||
record_fast_decision(extension_metrics.as_deref(), "deferred", "out_of_scope");
|
||||
return None;
|
||||
};
|
||||
if action.get("tool").and_then(serde_json::Value::as_str) != Some("mcp_tool_call")
|
||||
|| !action
|
||||
.get("server")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.is_some_and(is_node_repl_backed_server)
|
||||
|| !thread_store
|
||||
.get::<ModelInfo>()
|
||||
.is_some_and(|model| model.node_repl_auto_review_required)
|
||||
{
|
||||
record_fast_decision(extension_metrics.as_deref(), "deferred", "out_of_scope");
|
||||
return None;
|
||||
}
|
||||
// Legacy mode keeps the first REPL execution's review bypass.
|
||||
// Thread-owned context first checks whether Luna can use the checkpoint.
|
||||
if scope == Some(GuardianScope::ComputerUse) && policy.initial_cua_call {
|
||||
// Thread-owned context checks checkpoint compatibility before the first CUA allowance.
|
||||
initial_cua_call = action.get("tool_name").and_then(serde_json::Value::as_str)
|
||||
== Some("js")
|
||||
&& action
|
||||
@@ -319,7 +288,14 @@ impl ApprovalReviewContributor for GuardianV2Extension {
|
||||
} else if thread_store.get::<ModelInfo>().is_some() {
|
||||
let manager = self.thread_manager.upgrade()?;
|
||||
let thread_id = ThreadId::from_string(thread_store.level_id()).ok()?;
|
||||
let thread = manager.get_thread(thread_id).await.ok()?;
|
||||
let Ok(thread) = manager.get_thread(thread_id).await else {
|
||||
record_fast_decision(
|
||||
extension_metrics.as_deref(),
|
||||
"deferred",
|
||||
"scoring_failure",
|
||||
);
|
||||
return None;
|
||||
};
|
||||
let config = thread.config().await;
|
||||
let model = thread_store.get::<ModelInfo>()?;
|
||||
if config
|
||||
@@ -345,10 +321,6 @@ impl ApprovalReviewContributor for GuardianV2Extension {
|
||||
record_fast_decision(extension_metrics.as_deref(), "deferred", "scoring_failure");
|
||||
return None;
|
||||
};
|
||||
let root_authorization = thread
|
||||
.guardian_root_snapshot()
|
||||
.await
|
||||
.map(|snapshot| snapshot.authorization_version);
|
||||
let history = thread.conversation_history_snapshot().await;
|
||||
if thread_context_enabled {
|
||||
let sampler = thread_store.get::<LunaSampler>()?;
|
||||
@@ -366,10 +338,7 @@ impl ApprovalReviewContributor for GuardianV2Extension {
|
||||
record_fast_decision(extension_metrics.as_deref(), "approved", "initial_cua_call");
|
||||
return Some(ReviewDecision::Approved);
|
||||
}
|
||||
let current_authorization = ScoreAuthorization {
|
||||
local: guardian_evidence.authorization_version(history.as_ref()),
|
||||
root: root_authorization,
|
||||
};
|
||||
let current_authorization = ScoreAuthorization::current(&thread).await;
|
||||
if !current_authorization.local.retained_context_complete
|
||||
|| current_authorization
|
||||
.root
|
||||
@@ -473,6 +442,10 @@ impl GuardianV2Extension {
|
||||
}
|
||||
|
||||
async fn score_tool(&self, input: ToolStartInput<'_>) {
|
||||
// Polling a code cell does not introduce another action or age its score.
|
||||
if input.tool_name.is_default_namespace() && input.tool_name.name == "wait" {
|
||||
return;
|
||||
}
|
||||
let classification_started_at = Instant::now();
|
||||
let Some(sampler) = input.thread_store.get::<LunaSampler>() else {
|
||||
return;
|
||||
@@ -483,11 +456,34 @@ impl GuardianV2Extension {
|
||||
let Some(score_progress) = input.thread_store.get::<GuardianV2ScoreProgress>() else {
|
||||
return;
|
||||
};
|
||||
if !should_classify_tool(input.tool_name, input.payload, guardian_config.review_scope) {
|
||||
if guardian_config.review_scope != GuardianV2ReviewScope::ComputerUseOnly {
|
||||
score_progress
|
||||
.latest_tool_call
|
||||
.fetch_add(/*val*/ 1, Ordering::Relaxed);
|
||||
let parent_model = input.thread_store.get::<ModelInfo>();
|
||||
let policy = guardian_config.policy_for_model(parent_model.as_deref());
|
||||
if !policy.scoring_enabled() {
|
||||
input.thread_store.remove::<GuardianV2Enabled>();
|
||||
}
|
||||
let mcp_server = input
|
||||
.mcp_tool
|
||||
.map(|tool| tool.tool_info().server_name.as_str());
|
||||
let scope = mcp_server
|
||||
.map(GuardianScope::for_mcp_server)
|
||||
.or_else(|| GuardianScope::for_tool(input.tool_name));
|
||||
if !policy.scores_tool(input.tool_name, input.payload, scope) {
|
||||
match policy.unscored_action {
|
||||
UnscoredAction::Ignore => {}
|
||||
UnscoredAction::AgeScore => {
|
||||
score_progress
|
||||
.latest_tool_call
|
||||
.fetch_add(/*val*/ 1, Ordering::Relaxed);
|
||||
}
|
||||
UnscoredAction::InvalidateScore => {
|
||||
let index = score_progress
|
||||
.latest_tool_call
|
||||
.fetch_add(/*val*/ 1, Ordering::Relaxed)
|
||||
.saturating_add(/*rhs*/ 1);
|
||||
score_progress
|
||||
.latest_failed_tool_call
|
||||
.fetch_max(index, Ordering::Release);
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
@@ -505,7 +501,7 @@ impl GuardianV2Extension {
|
||||
let tool_call_index = score_progress
|
||||
.latest_tool_call
|
||||
.fetch_add(/*val*/ 1, Ordering::Relaxed)
|
||||
.saturating_add(1);
|
||||
.saturating_add(/*rhs*/ 1);
|
||||
let event_sink = Arc::clone(&self.event_sink);
|
||||
let thread_id = input.thread_store.level_id().to_owned();
|
||||
let turn_id = input.turn_id.to_owned();
|
||||
@@ -546,23 +542,17 @@ impl GuardianV2Extension {
|
||||
};
|
||||
// Use the live reviewer, not the startup config or per-app reviewer overrides.
|
||||
let snapshot = thread.config_snapshot().await;
|
||||
let parent_model = input.thread_store.get::<ModelInfo>();
|
||||
if snapshot.full_access
|
||||
|| thread.approvals_reviewer_for_turn(input.turn_id).await == ApprovalsReviewer::User
|
||||
|| (guardian_config.review_scope == GuardianV2ReviewScope::ComputerUseOnly
|
||||
&& !parent_model
|
||||
.as_ref()
|
||||
.is_some_and(|model| model.node_repl_auto_review_required))
|
||||
{
|
||||
// A skipped call invalidates older scores, including ones still in flight
|
||||
// when switching to a model that does not require REPL review.
|
||||
// A skipped call invalidates older scores, including ones still in flight.
|
||||
score_progress
|
||||
.latest_failed_tool_call
|
||||
.fetch_max(tool_call_index, Ordering::Release);
|
||||
return;
|
||||
}
|
||||
// Computer-use-only scores cannot approve other tools for required models.
|
||||
if guardian_config.review_scope != GuardianV2ReviewScope::ComputerUseOnly
|
||||
// A required model keeps synchronous review outside its CUA allowance.
|
||||
if !(scope == Some(GuardianScope::ComputerUse) && policy.initial_cua_call)
|
||||
&& parent_model.as_ref().is_some_and(|model| {
|
||||
config
|
||||
.config_layer_stack
|
||||
@@ -573,6 +563,7 @@ impl GuardianV2Extension {
|
||||
input.thread_store.remove::<SecurityRiskScore>();
|
||||
return;
|
||||
}
|
||||
input.thread_store.insert(GuardianV2Enabled);
|
||||
let model_defaults = parent_model
|
||||
.as_ref()
|
||||
.and_then(|model| model.model_messages.as_ref())
|
||||
@@ -689,6 +680,7 @@ impl GuardianV2Extension {
|
||||
.transcript
|
||||
.images(input.conversation_history.review_items(), node_repl_images);
|
||||
|
||||
let score_authorization = ScoreAuthorization::current(&thread).await;
|
||||
tokio::spawn(async move {
|
||||
let mut truncations = ClassificationTruncations::default();
|
||||
let trusted_tool_context = match mcp_tool.as_ref() {
|
||||
@@ -715,6 +707,8 @@ impl GuardianV2Extension {
|
||||
let score_authorization = ScoreAuthorization {
|
||||
local: authorization_version,
|
||||
root: root_authorization_version,
|
||||
model: parent_model.clone(),
|
||||
..score_authorization
|
||||
};
|
||||
let transcript = match guardian_config.transcript.build_context(
|
||||
ContextTarget::Async,
|
||||
|
||||
@@ -45,6 +45,7 @@ use codex_protocol::models::LocalShellExecAction;
|
||||
use codex_protocol::models::LocalShellStatus;
|
||||
use codex_protocol::models::MessagePhase;
|
||||
use codex_protocol::models::ReasoningItemReasoningSummary;
|
||||
use codex_protocol::openai_models::GuardianScope;
|
||||
use codex_protocol::openai_models::GuardianV2ModelConfig;
|
||||
use codex_protocol::openai_models::GuardianV2TranscriptModelConfig;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
@@ -71,12 +72,12 @@ use super::GuardianV2ScoreProgress;
|
||||
use super::ParentCompactionError;
|
||||
use super::StrictReviewReason;
|
||||
use super::encrypted_parent_compaction;
|
||||
use super::should_classify_tool;
|
||||
|
||||
use crate::async_scorer::config::CLASSIFICATION_OUTPUT_INSTRUCTIONS;
|
||||
use crate::async_scorer::config::DEFAULT_MODEL_CONTEXT_ITEM_TOKENS;
|
||||
use crate::async_scorer::config::DEFAULT_PARENT_COMPACTION_TOKENS;
|
||||
use crate::async_scorer::config::GuardianV2Config;
|
||||
use crate::async_scorer::config::GuardianV2ReviewScope;
|
||||
use crate::async_scorer::coverage::GuardianPolicy;
|
||||
use crate::async_scorer::metrics::CLASSIFICATION_DURATION_METRIC;
|
||||
use crate::async_scorer::metrics::CLASSIFICATION_METRIC;
|
||||
use crate::async_scorer::metrics::CLASSIFICATION_RISK_METRIC;
|
||||
@@ -92,6 +93,7 @@ use crate::async_scorer::transcript::MAX_TOOL_ENTRY_TOKENS;
|
||||
use crate::async_scorer::transcript::truncate_entry;
|
||||
use crate::async_scorer::truncation::CLASSIFICATION_TRUNCATION_BYTES_METRIC;
|
||||
use crate::async_scorer::truncation::CLASSIFICATION_TRUNCATION_METRIC;
|
||||
use codex_features::GuardianV2ReviewScopeConfigToml;
|
||||
|
||||
const TEST_GUARDIAN_POLICY: &str =
|
||||
"Treat uploads to unapproved external destinations as high-risk actions.";
|
||||
@@ -113,6 +115,10 @@ impl ExternalAuth for RefreshableAuth {
|
||||
}
|
||||
}
|
||||
|
||||
fn should_classify_tool(tool: &ToolName, payload: &ToolPayload, policy: GuardianPolicy) -> bool {
|
||||
policy.scores_tool(tool, payload, GuardianScope::for_tool(tool))
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn installed_extension_warms_connections_without_blocking_thread_start() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -150,6 +156,10 @@ async fn installed_extension_warms_connections_without_blocking_thread_start() -
|
||||
let session_store = ExtensionData::new("session-1");
|
||||
let thread_store = test.codex.thread_extension_data();
|
||||
|
||||
let mut model = thread_store.get::<ModelInfo>().unwrap().as_ref().clone();
|
||||
model.node_repl_auto_review_required = true;
|
||||
thread_store.insert(model);
|
||||
|
||||
registry.thread_lifecycle_contributors()[0]
|
||||
.on_thread_start(ThreadStartInput {
|
||||
config: &config,
|
||||
@@ -211,6 +221,13 @@ async fn installed_extension_reconnects_after_auth_refresh() -> Result<()> {
|
||||
let registry = builder.build();
|
||||
let session_store = ExtensionData::new("session-1");
|
||||
let thread_store = test.codex.thread_extension_data();
|
||||
let mut model = test
|
||||
.thread_manager
|
||||
.get_models_manager()
|
||||
.get_model_info("gpt-5.5", &config.to_models_manager_config())
|
||||
.await;
|
||||
model.node_repl_auto_review_required = true;
|
||||
thread_store.insert(model);
|
||||
registry.thread_lifecycle_contributors()[0]
|
||||
.on_thread_start(ThreadStartInput {
|
||||
config: &config,
|
||||
@@ -231,13 +248,6 @@ async fn installed_extension_reconnects_after_auth_refresh() -> Result<()> {
|
||||
let progress = thread_store
|
||||
.get::<GuardianV2ScoreProgress>()
|
||||
.expect("Guardian v2 should initialize");
|
||||
let mut model = test
|
||||
.thread_manager
|
||||
.get_models_manager()
|
||||
.get_model_info("gpt-5.5", &config.to_models_manager_config())
|
||||
.await;
|
||||
model.node_repl_auto_review_required = true;
|
||||
thread_store.insert(model);
|
||||
let turn_store = ExtensionData::new("turn-1");
|
||||
let tool_name = ToolName::namespaced("mcp__node_repl__", "js");
|
||||
let payload = ToolPayload::Function {
|
||||
@@ -455,40 +465,42 @@ async fn sandboxed_shell_classification_respects_review_scope() -> Result<()> {
|
||||
};
|
||||
|
||||
let tool_name = ToolName::plain("exec_command");
|
||||
let standard_scope = GuardianV2ReviewScope::Standard {
|
||||
sandboxed_exec_commands: false,
|
||||
};
|
||||
let standard_scope = GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(false),
|
||||
sandboxed_exec_commands: Some(false),
|
||||
}));
|
||||
assert!(!should_classify_tool(
|
||||
&tool_name,
|
||||
&sandboxed,
|
||||
standard_scope,
|
||||
standard_scope.clone(),
|
||||
));
|
||||
assert!(!should_classify_tool(
|
||||
&tool_name,
|
||||
&additional_permissions,
|
||||
standard_scope,
|
||||
standard_scope.clone(),
|
||||
));
|
||||
assert!(should_classify_tool(
|
||||
&tool_name,
|
||||
&unsandboxed,
|
||||
standard_scope,
|
||||
standard_scope.clone(),
|
||||
));
|
||||
assert!(should_classify_tool(
|
||||
&tool_name,
|
||||
&sandboxed,
|
||||
GuardianV2ReviewScope::Standard {
|
||||
sandboxed_exec_commands: true,
|
||||
},
|
||||
GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(false),
|
||||
sandboxed_exec_commands: Some(true),
|
||||
})),
|
||||
));
|
||||
assert!(should_classify_tool(
|
||||
&ToolName::plain("read_file"),
|
||||
&sandboxed,
|
||||
standard_scope,
|
||||
standard_scope.clone(),
|
||||
));
|
||||
assert!(should_classify_tool(
|
||||
&ToolName::namespaced("mcp", "exec_command"),
|
||||
&sandboxed,
|
||||
standard_scope,
|
||||
standard_scope.clone(),
|
||||
));
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -550,7 +562,11 @@ fn computer_use_only_classification_recognizes_direct_and_code_mode_tools() {
|
||||
(ToolName::plain("exec_command"), false),
|
||||
] {
|
||||
assert_eq!(
|
||||
should_classify_tool(&tool_name, &payload, GuardianV2ReviewScope::ComputerUseOnly),
|
||||
should_classify_tool(
|
||||
&tool_name,
|
||||
&payload,
|
||||
GuardianPolicy::from_legacy(/*scope*/ None)
|
||||
),
|
||||
expected,
|
||||
"unexpected classification scope for {tool_name}"
|
||||
);
|
||||
@@ -563,12 +579,7 @@ async fn computer_use_only_scores_cannot_approve_other_actions() -> Result<()> {
|
||||
|
||||
let fixture = GuardianFailureFixture::new().await?;
|
||||
let thread_store = fixture.test.codex.thread_extension_data();
|
||||
let mut model = fixture
|
||||
.test
|
||||
.thread_manager
|
||||
.get_models_manager()
|
||||
.get_model_info("gpt-5.5", &fixture.test.config.to_models_manager_config())
|
||||
.await;
|
||||
let mut model = thread_store.get::<ModelInfo>().unwrap().as_ref().clone();
|
||||
model.node_repl_auto_review_required = true;
|
||||
thread_store.insert(model);
|
||||
let mut config = thread_store
|
||||
@@ -576,7 +587,7 @@ async fn computer_use_only_scores_cannot_approve_other_actions() -> Result<()> {
|
||||
.expect("Guardian v2 should have initialized")
|
||||
.as_ref()
|
||||
.clone();
|
||||
config.review_scope = GuardianV2ReviewScope::ComputerUseOnly;
|
||||
config.policy = GuardianPolicy::from_legacy(/*scope*/ None);
|
||||
thread_store.insert(config);
|
||||
thread_store.insert(SecurityRiskScore {
|
||||
scores: BTreeMap::from([("action_risk".to_owned(), 0.25)]),
|
||||
@@ -588,6 +599,10 @@ async fn computer_use_only_scores_cannot_approve_other_actions() -> Result<()> {
|
||||
let progress = thread_store
|
||||
.get::<GuardianV2ScoreProgress>()
|
||||
.expect("Guardian v2 should track score progress per thread");
|
||||
// The seeded low score belongs to the model selected above.
|
||||
let authorization =
|
||||
super::super::authorization::ScoreAuthorization::current(&fixture.test.codex).await;
|
||||
*progress.authorization.lock().unwrap() = Some(authorization);
|
||||
let latest_tool_call = progress.latest_tool_call.load(Ordering::Acquire);
|
||||
let turn_store = ExtensionData::new("turn-1");
|
||||
let ordinary_tool = ToolName::namespaced("mcp__ordinary__", "write_record");
|
||||
@@ -665,22 +680,31 @@ async fn computer_use_only_scores_cannot_approve_other_actions() -> Result<()> {
|
||||
None,
|
||||
"malformed approval actions must not reuse a browser score"
|
||||
);
|
||||
thread_store.remove::<SecurityRiskScore>();
|
||||
let mut changed_model = fixture
|
||||
.test
|
||||
.thread_manager
|
||||
.get_models_manager()
|
||||
.get_model_info("gpt-5.5", &fixture.test.config.to_models_manager_config())
|
||||
.await;
|
||||
changed_model.guardian = Some(codex_protocol::openai_models::GuardianModelPolicy {
|
||||
computer_use: Some(codex_protocol::openai_models::GuardianReviewMode::Adaptive),
|
||||
..Default::default()
|
||||
});
|
||||
thread_store.insert(changed_model);
|
||||
assert_eq!(
|
||||
fixture
|
||||
.registry
|
||||
.fast_approval_decision(
|
||||
&fixture.session_store,
|
||||
thread_store,
|
||||
&json!({"tool": "mcp_tool_call", "server": "node_repl"}).to_string(),
|
||||
thread_store
|
||||
.get::<RecordingMetrics>()
|
||||
.map(|metrics| metrics as Arc<dyn ExtensionMetrics>),
|
||||
&json!({"tool": "mcp_tool_call", "server": "node_repl", "tool_name": "js"})
|
||||
.to_string(),
|
||||
/*extension_metrics*/ None,
|
||||
)
|
||||
.await,
|
||||
None
|
||||
None,
|
||||
"a score from the previous model policy must not approve a call"
|
||||
);
|
||||
|
||||
let fast_decisions = thread_store
|
||||
.get::<RecordingMetrics>()
|
||||
.unwrap()
|
||||
@@ -704,11 +728,11 @@ async fn computer_use_only_scores_cannot_approve_other_actions() -> Result<()> {
|
||||
fast_decision_metric("deferred", "out_of_scope"),
|
||||
fast_decision_metric("deferred", "out_of_scope"),
|
||||
fast_decision_metric("deferred", "out_of_scope"),
|
||||
fast_decision_metric("deferred", "missing_score"),
|
||||
]
|
||||
);
|
||||
|
||||
let mut model = thread_store.get::<ModelInfo>().unwrap().as_ref().clone();
|
||||
model.guardian = None; // Exercise the legacy fallback after the catalog case above.
|
||||
model.node_repl_auto_review_required = false;
|
||||
thread_store.insert(model.clone());
|
||||
fixture
|
||||
@@ -1100,6 +1124,58 @@ struct GuardianFailureFixture {
|
||||
session_store: ExtensionData,
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn disabled_code_mode_invalidates_cached_scores() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let fixture = GuardianFailureFixture::new().await?;
|
||||
let thread_store = fixture.test.codex.thread_extension_data();
|
||||
let mut model = thread_store
|
||||
.get::<codex_protocol::openai_models::ModelInfo>()
|
||||
.expect("resolved model")
|
||||
.as_ref()
|
||||
.clone();
|
||||
model.guardian = Some(codex_protocol::openai_models::GuardianModelPolicy {
|
||||
computer_use: Some(codex_protocol::openai_models::GuardianReviewMode::Adaptive),
|
||||
..Default::default()
|
||||
});
|
||||
thread_store.insert(model);
|
||||
let progress = thread_store
|
||||
.get::<GuardianV2ScoreProgress>()
|
||||
.expect("score progress");
|
||||
let before = (
|
||||
progress.latest_tool_call.load(Ordering::Acquire),
|
||||
progress.latest_failed_tool_call.load(Ordering::Acquire),
|
||||
);
|
||||
fixture.score_tool(ToolName::plain("wait")).await;
|
||||
assert_eq!(
|
||||
(
|
||||
progress.latest_tool_call.load(Ordering::Acquire),
|
||||
progress.latest_failed_tool_call.load(Ordering::Acquire),
|
||||
),
|
||||
before,
|
||||
);
|
||||
fixture.score_tool(ToolName::plain("exec")).await;
|
||||
assert_eq!(
|
||||
(
|
||||
progress.latest_tool_call.load(Ordering::Acquire),
|
||||
progress.latest_failed_tool_call.load(Ordering::Acquire),
|
||||
),
|
||||
(before.0 + 1, before.0 + 1),
|
||||
);
|
||||
// An MCP tool with the same name remains in the MCP category.
|
||||
fixture
|
||||
.score_tool(ToolName::namespaced("mcp__ordinary", "wait"))
|
||||
.await;
|
||||
assert_eq!(
|
||||
(
|
||||
progress.latest_tool_call.load(Ordering::Acquire),
|
||||
progress.latest_failed_tool_call.load(Ordering::Acquire),
|
||||
),
|
||||
(before.0 + 2, before.0 + 2),
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
impl GuardianFailureFixture {
|
||||
async fn new() -> Result<Self> {
|
||||
Self::with_config("").await
|
||||
@@ -2289,9 +2365,10 @@ async fn contributor_skips_required_models_in_standard_scope() -> Result<()> {
|
||||
.expect("Guardian v2 should have initialized")
|
||||
.as_ref()
|
||||
.clone();
|
||||
guardian_config.review_scope = GuardianV2ReviewScope::Standard {
|
||||
sandboxed_exec_commands: false,
|
||||
};
|
||||
guardian_config.policy = GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(false),
|
||||
sandboxed_exec_commands: Some(false),
|
||||
}));
|
||||
thread_store.insert(guardian_config);
|
||||
|
||||
let mut model_info = test
|
||||
@@ -2501,16 +2578,18 @@ async fn assert_compaction_approval_policy(thread_context_enabled: bool) -> Resu
|
||||
fixture.test.codex.guardian_authorization_version().await,
|
||||
authorization
|
||||
);
|
||||
let score_authorization = super::ScoreAuthorization::current(&fixture.test.codex).await;
|
||||
*thread_store
|
||||
.get::<GuardianV2ScoreProgress>()
|
||||
.expect("score progress")
|
||||
.authorization
|
||||
.lock()
|
||||
.unwrap() = Some(score_authorization);
|
||||
// No new sample runs: only the enabled path rejects cached and initial-call approvals.
|
||||
for (scope, prompt) in [
|
||||
for (computer_use_only, prompt) in [
|
||||
(false, "review action"),
|
||||
(
|
||||
GuardianV2ReviewScope::Standard {
|
||||
sandboxed_exec_commands: true,
|
||||
},
|
||||
"review action",
|
||||
),
|
||||
(
|
||||
GuardianV2ReviewScope::ComputerUseOnly,
|
||||
true,
|
||||
r#"{"tool":"mcp_tool_call","server":"node_repl","connector_id":"node_repl","tool_name":"js"}"#,
|
||||
),
|
||||
] {
|
||||
@@ -2518,7 +2597,10 @@ async fn assert_compaction_approval_policy(thread_context_enabled: bool) -> Resu
|
||||
.get::<GuardianV2Config>()
|
||||
.expect("Guardian configuration"))
|
||||
.clone();
|
||||
config.review_scope = scope;
|
||||
config.policy = GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(computer_use_only),
|
||||
sandboxed_exec_commands: Some(true),
|
||||
}));
|
||||
thread_store.insert(config);
|
||||
thread_store
|
||||
.get::<GuardianV2ScoreProgress>()
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
mod action;
|
||||
mod authorization;
|
||||
mod config;
|
||||
mod coverage;
|
||||
mod extension;
|
||||
mod metrics;
|
||||
mod review_evidence;
|
||||
|
||||
@@ -179,6 +179,7 @@ pub fn model_info_from_slug(slug: &str) -> ModelInfo {
|
||||
used_fallback_model_metadata: true, // this is the fallback model metadata
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -33,6 +33,12 @@ use crate::config_types::ServiceTier;
|
||||
use crate::config_types::Verbosity;
|
||||
use crate::protocol::MultiAgentVersion;
|
||||
|
||||
#[path = "openai_models/guardian.rs"]
|
||||
mod guardian;
|
||||
pub use guardian::GuardianModelPolicy;
|
||||
pub use guardian::GuardianReviewMode;
|
||||
pub use guardian::GuardianScope;
|
||||
|
||||
#[path = "openai_models/guardian_v2.rs"]
|
||||
mod guardian_v2;
|
||||
|
||||
@@ -390,6 +396,11 @@ const fn is_true(value: &bool) -> bool {
|
||||
/// Model metadata returned by the Codex backend `/models` endpoint.
|
||||
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
|
||||
pub struct ModelInfo {
|
||||
/// Model-owned approval coverage. Absent preserves legacy settings; an empty map disables
|
||||
/// ordinary Guardian review. Keys are computer_use, shell, code_mode, file_changes, mcp, network,
|
||||
/// and permissions. This does not override mandatory safety or administrator requirements.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub guardian: Option<GuardianModelPolicy>,
|
||||
pub slug: String,
|
||||
pub display_name: String,
|
||||
pub description: Option<String>,
|
||||
@@ -961,7 +972,7 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
fn test_model(spec: Option<ModelMessages>) -> ModelInfo {
|
||||
pub(super) fn test_model(spec: Option<ModelMessages>) -> ModelInfo {
|
||||
ModelInfo {
|
||||
slug: "test-model".to_string(),
|
||||
display_name: "Test Model".to_string(),
|
||||
@@ -999,6 +1010,7 @@ mod tests {
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
guardian: None,
|
||||
node_repl_auto_review_required: false,
|
||||
node_repl_disabled: false,
|
||||
auto_review_model_override: None,
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
//! Model-owned Guardian coverage. Missing policy preserves legacy behavior;
|
||||
//! unknown modes retain synchronous review and never enable the fast path.
|
||||
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use ts_rs::TS;
|
||||
|
||||
use super::ModelInfo;
|
||||
use crate::ToolName;
|
||||
use crate::mcp::is_node_repl_backed_server;
|
||||
use crate::mcp::is_node_repl_backed_tool;
|
||||
|
||||
/// How Guardian handles an action when the user selects automatic approval.
|
||||
#[derive(Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq, TS, JsonSchema)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum GuardianReviewMode {
|
||||
Disabled,
|
||||
Synchronous,
|
||||
/// Use a current low-risk score; otherwise run synchronous review.
|
||||
Adaptive,
|
||||
#[serde(other)]
|
||||
Unknown,
|
||||
}
|
||||
|
||||
/// A complete model policy. Omitted scopes are disabled; unknown fields are ignored.
|
||||
#[derive(Debug, Default, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
|
||||
pub struct GuardianModelPolicy {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub computer_use: Option<GuardianReviewMode>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub shell: Option<GuardianReviewMode>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub code_mode: Option<GuardianReviewMode>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub file_changes: Option<GuardianReviewMode>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub mcp: Option<GuardianReviewMode>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub network: Option<GuardianReviewMode>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub permissions: Option<GuardianReviewMode>,
|
||||
}
|
||||
|
||||
impl GuardianModelPolicy {
|
||||
pub fn review_mode(&self, scope: GuardianScope) -> GuardianReviewMode {
|
||||
match scope {
|
||||
GuardianScope::ComputerUse => self.computer_use,
|
||||
GuardianScope::Shell => self.shell,
|
||||
GuardianScope::CodeMode => self.code_mode,
|
||||
GuardianScope::FileChanges => self.file_changes,
|
||||
GuardianScope::Mcp => self.mcp,
|
||||
GuardianScope::Network => self.network,
|
||||
GuardianScope::Permissions => self.permissions,
|
||||
}
|
||||
.unwrap_or(GuardianReviewMode::Disabled)
|
||||
}
|
||||
}
|
||||
|
||||
/// Approval categories understood by this client. Future catalog keys are ignored.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum GuardianScope {
|
||||
ComputerUse,
|
||||
Shell,
|
||||
CodeMode,
|
||||
FileChanges,
|
||||
Mcp,
|
||||
Network,
|
||||
Permissions,
|
||||
}
|
||||
|
||||
impl GuardianScope {
|
||||
pub fn for_mcp_server(server: &str) -> Self {
|
||||
if is_node_repl_backed_server(server) {
|
||||
Self::ComputerUse
|
||||
} else {
|
||||
Self::Mcp
|
||||
}
|
||||
}
|
||||
|
||||
pub fn for_tool(tool: &ToolName) -> Option<Self> {
|
||||
if is_node_repl_backed_tool(&tool.name, tool.namespace.as_deref()) {
|
||||
return Some(Self::ComputerUse);
|
||||
}
|
||||
if tool
|
||||
.namespace
|
||||
.as_deref()
|
||||
.is_some_and(|namespace| namespace.starts_with("mcp__"))
|
||||
|| tool.name.starts_with("mcp__")
|
||||
{
|
||||
return Some(Self::Mcp);
|
||||
}
|
||||
if !tool.is_default_namespace() {
|
||||
return None;
|
||||
}
|
||||
match tool.name.as_str() {
|
||||
"shell" | "shell_command" | "exec_command" | "write_stdin" | "execve" => {
|
||||
Some(Self::Shell)
|
||||
}
|
||||
"exec" | "wait" => Some(Self::CodeMode),
|
||||
"apply_patch" => Some(Self::FileChanges),
|
||||
"request_permissions" => Some(Self::Permissions),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ModelInfo {
|
||||
/// An absent map uses legacy config; an omitted scope in a supplied map is disabled.
|
||||
pub fn guardian_review_mode(&self, scope: GuardianScope) -> Option<GuardianReviewMode> {
|
||||
self.guardian
|
||||
.as_ref()
|
||||
.map(|policy| policy.review_mode(scope))
|
||||
}
|
||||
|
||||
/// The legacy metadata bit remains the transport understood by older CUA servers.
|
||||
pub fn computer_use_review_required(&self) -> bool {
|
||||
self.guardian_review_mode(GuardianScope::ComputerUse)
|
||||
.map_or(self.node_repl_auto_review_required, |mode| {
|
||||
mode != GuardianReviewMode::Disabled
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "guardian_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,48 @@
|
||||
use super::super::tests::test_model;
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn guardian_policy_is_optional_and_tolerates_future_config() -> anyhow::Result<()> {
|
||||
let model = test_model(/*spec*/ None);
|
||||
let mut wire = serde_json::to_value(&model)?;
|
||||
assert!(
|
||||
!wire
|
||||
.as_object()
|
||||
.expect("model object")
|
||||
.contains_key("guardian")
|
||||
);
|
||||
assert_eq!(serde_json::from_value::<ModelInfo>(wire.clone())?, model);
|
||||
|
||||
wire["guardian"] = serde_json::json!({
|
||||
"computer_use": "adaptive", "shell": "future_mode", "future_scope": {"version": 2},
|
||||
"mcp": null, "code_mode": "adaptive",
|
||||
});
|
||||
let model: ModelInfo = serde_json::from_value(wire)?;
|
||||
assert_eq!(
|
||||
model.guardian,
|
||||
Some(GuardianModelPolicy {
|
||||
computer_use: Some(GuardianReviewMode::Adaptive),
|
||||
shell: Some(GuardianReviewMode::Unknown),
|
||||
code_mode: Some(GuardianReviewMode::Adaptive),
|
||||
..Default::default()
|
||||
})
|
||||
);
|
||||
assert!(model.computer_use_review_required());
|
||||
assert_eq!(
|
||||
model.guardian_review_mode(GuardianScope::Mcp),
|
||||
Some(GuardianReviewMode::Disabled)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn guardian_policy_overrides_the_legacy_cua_bit_only_when_present() {
|
||||
let mut model = test_model(/*spec*/ None);
|
||||
model.node_repl_auto_review_required = true;
|
||||
assert!(model.computer_use_review_required());
|
||||
model.guardian = Some(GuardianModelPolicy::default());
|
||||
assert!(!model.computer_use_review_required());
|
||||
model.guardian.as_mut().expect("policy").computer_use = Some(GuardianReviewMode::Synchronous);
|
||||
assert!(model.computer_use_review_required());
|
||||
}
|
||||
Reference in New Issue
Block a user