mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Centralize Guardian policy resolution in config and protocol (#45957)
## What changed - Extend `GuardianModelPolicy` with controls for uncategorized tools, unscored actions, the initial computer-use call allowance, and sandboxed command coverage. - Add `GuardianPolicyLoader` in `codex-config` to translate legacy settings, preserve catalog policy precedence, and enforce reviewer requirements. Apply live model review requirements through `ConfigRequirements::constrain_guardian_policy`. - Use the shared model policy for Guardian scoring and approval, replacing the extension-local policy wrapper while retaining legacy defaults. ## Testing Add configuration tests for catalog precedence, legacy scope fallback, required-model constraints, and legacy computer-use opt-in and feature gating. Adapt existing extension tests to consume the shared policy. GitOrigin-RevId: 6fed1c3a831964ad28ea7de5cb19e74e323c1204
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -3595,6 +3595,7 @@ dependencies = [
|
||||
"anyhow",
|
||||
"codex-analytics",
|
||||
"codex-api",
|
||||
"codex-config",
|
||||
"codex-core",
|
||||
"codex-extension-api",
|
||||
"codex-features",
|
||||
|
||||
106
codex-rs/config/src/guardian.rs
Normal file
106
codex-rs/config/src/guardian.rs
Normal file
@@ -0,0 +1,106 @@
|
||||
//! Translates legacy Guardian settings and applies live managed constraints.
|
||||
//! Catalog policies take precedence over legacy scope and feature settings.
|
||||
|
||||
use codex_features::FeatureToml;
|
||||
use codex_features::GuardianV2ConfigToml;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::openai_models::GuardianModelPolicy;
|
||||
use codex_protocol::openai_models::GuardianReviewMode;
|
||||
use codex_protocol::openai_models::GuardianUnscoredAction;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
|
||||
use crate::ConfigRequirements;
|
||||
|
||||
/// Load-boundary adapter for configuration that predates model-owned policies.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct GuardianPolicyLoader {
|
||||
default: GuardianModelPolicy,
|
||||
force_synchronous_review: bool,
|
||||
scoring_disabled: bool,
|
||||
}
|
||||
|
||||
impl GuardianPolicyLoader {
|
||||
pub fn new(
|
||||
legacy: Option<&FeatureToml<GuardianV2ConfigToml>>,
|
||||
requirements: &ConfigRequirements,
|
||||
) -> Self {
|
||||
let scope = match legacy {
|
||||
Some(FeatureToml::Config(config)) => config.review_scope.as_ref(),
|
||||
Some(FeatureToml::Enabled(_)) | None => None,
|
||||
};
|
||||
let computer_use_only = scope
|
||||
.and_then(|scope| scope.computer_use_only)
|
||||
.unwrap_or(/*default*/ true);
|
||||
let other = if computer_use_only {
|
||||
GuardianReviewMode::Synchronous
|
||||
} else {
|
||||
GuardianReviewMode::Adaptive
|
||||
};
|
||||
let default = GuardianModelPolicy {
|
||||
computer_use: Some(GuardianReviewMode::Adaptive),
|
||||
shell: Some(other),
|
||||
file_changes: Some(other),
|
||||
mcp: Some(other),
|
||||
network: Some(other),
|
||||
permissions: Some(other),
|
||||
other_tools: other,
|
||||
unscored_action: if computer_use_only {
|
||||
GuardianUnscoredAction::Ignore
|
||||
} else {
|
||||
GuardianUnscoredAction::AgeScore
|
||||
},
|
||||
initial_cua_call: Some(computer_use_only),
|
||||
sandboxed_exec_commands: Some(
|
||||
!computer_use_only
|
||||
&& scope
|
||||
.and_then(|scope| scope.sandboxed_exec_commands)
|
||||
.unwrap_or(/*default*/ false),
|
||||
),
|
||||
};
|
||||
let enabled = legacy.and_then(FeatureToml::enabled);
|
||||
Self {
|
||||
default,
|
||||
scoring_disabled: !enabled.unwrap_or(/*default*/ false),
|
||||
force_synchronous_review: requirements
|
||||
.approvals_reviewer
|
||||
.can_set(&ApprovalsReviewer::User)
|
||||
.is_err(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn resolve(&self, model: Option<&ModelInfo>) -> GuardianModelPolicy {
|
||||
let catalog = model.and_then(|model| model.guardian.as_ref());
|
||||
let mut policy = catalog.cloned().unwrap_or_else(|| self.default.clone());
|
||||
if catalog.is_none()
|
||||
&& policy.allows_initial_cua_call()
|
||||
&& model.is_some_and(|model| !model.node_repl_auto_review_required)
|
||||
{
|
||||
policy.computer_use = Some(GuardianReviewMode::Synchronous);
|
||||
policy.initial_cua_call = Some(false);
|
||||
policy.unscored_action = GuardianUnscoredAction::InvalidateScore;
|
||||
}
|
||||
// Preserve the legacy feature gate: explicit catalog policies bypass it.
|
||||
if self.force_synchronous_review || self.scoring_disabled && catalog.is_none() {
|
||||
policy.disable_scoring();
|
||||
}
|
||||
policy
|
||||
}
|
||||
}
|
||||
|
||||
impl ConfigRequirements {
|
||||
/// Applies live administrator requirements to an already resolved model policy.
|
||||
pub fn constrain_guardian_policy(&self, policy: &mut GuardianModelPolicy, model: &str) {
|
||||
if self.auto_review_required_for_model(model) {
|
||||
let computer_use = policy.computer_use;
|
||||
let initial_cua_call = policy.allows_initial_cua_call();
|
||||
policy.disable_scoring();
|
||||
if initial_cua_call {
|
||||
policy.computer_use = computer_use;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "guardian_tests.rs"]
|
||||
mod tests;
|
||||
124
codex-rs/config/src/guardian_tests.rs
Normal file
124
codex-rs/config/src/guardian_tests.rs
Normal file
@@ -0,0 +1,124 @@
|
||||
use super::*;
|
||||
use crate::FeatureRequirementsToml;
|
||||
use crate::RequirementSource;
|
||||
use crate::Sourced;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeSet;
|
||||
|
||||
#[test]
|
||||
fn explicit_catalog_policy_preserves_precedence_over_the_legacy_switch() {
|
||||
let catalog: GuardianModelPolicy = serde_json::from_value(json!({
|
||||
"computer_use": "adaptive", "shell": "disabled"
|
||||
}))
|
||||
.unwrap();
|
||||
for legacy in [json!(false), json!({"enabled": false})] {
|
||||
let legacy = serde_json::from_value(legacy).unwrap();
|
||||
for managed in [None, Some(false), Some(true)] {
|
||||
let requirements = ConfigRequirements {
|
||||
feature_requirements: managed.map(|enabled| {
|
||||
Sourced::new(
|
||||
FeatureRequirementsToml {
|
||||
entries: [("guardianv2".to_owned(), enabled)].into(),
|
||||
},
|
||||
RequirementSource::Unknown,
|
||||
)
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
let mut model = test_model();
|
||||
model.guardian = Some(catalog.clone());
|
||||
assert_eq!(
|
||||
GuardianPolicyLoader::new(Some(&legacy), &requirements).resolve(Some(&model)),
|
||||
catalog
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_scope_applies_only_to_models_without_catalog_policy() {
|
||||
let legacy = serde_json::from_value(json!({
|
||||
"enabled": true,
|
||||
"review_scope": {"computer_use_only": false, "sandboxed_exec_commands": true}
|
||||
}))
|
||||
.unwrap();
|
||||
let loader = GuardianPolicyLoader::new(Some(&legacy), &ConfigRequirements::default());
|
||||
let expected = GuardianModelPolicy {
|
||||
computer_use: Some(GuardianReviewMode::Adaptive),
|
||||
shell: Some(GuardianReviewMode::Adaptive),
|
||||
file_changes: Some(GuardianReviewMode::Adaptive),
|
||||
mcp: Some(GuardianReviewMode::Adaptive),
|
||||
network: Some(GuardianReviewMode::Adaptive),
|
||||
permissions: Some(GuardianReviewMode::Adaptive),
|
||||
other_tools: GuardianReviewMode::Adaptive,
|
||||
unscored_action: GuardianUnscoredAction::AgeScore,
|
||||
initial_cua_call: Some(false),
|
||||
sandboxed_exec_commands: Some(true),
|
||||
};
|
||||
assert_eq!(loader.resolve(/*model*/ None), expected);
|
||||
let catalog: GuardianModelPolicy =
|
||||
serde_json::from_value(json!({"computer_use": "adaptive"})).unwrap();
|
||||
let mut model = test_model();
|
||||
model.guardian = Some(catalog.clone());
|
||||
assert_eq!(loader.resolve(Some(&model)), catalog);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn required_models_preserve_cua_allowance_and_disable_all_tools_scoring() {
|
||||
let requirements = ConfigRequirements {
|
||||
auto_review_required_models: Some(Sourced::new(
|
||||
BTreeSet::from(["protected-model".to_owned()]),
|
||||
RequirementSource::Unknown,
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let legacy = serde_json::from_value(json!({
|
||||
"enabled": true, "review_scope": {"computer_use_only": false}
|
||||
}))
|
||||
.unwrap();
|
||||
let cua_only = FeatureToml::Enabled(true);
|
||||
for legacy in [Some(&cua_only), Some(&legacy)] {
|
||||
let loader = GuardianPolicyLoader::new(legacy, &requirements);
|
||||
let ordinary = loader.resolve(/*model*/ None);
|
||||
let mut expected = ordinary.clone();
|
||||
expected.disable_scoring();
|
||||
if ordinary.allows_initial_cua_call() {
|
||||
expected.computer_use = ordinary.computer_use;
|
||||
}
|
||||
let mut actual = ordinary;
|
||||
requirements.constrain_guardian_policy(&mut actual, "provider/protected-model");
|
||||
assert_eq!(actual, expected);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_cua_opt_in_and_disabled_feature_preserve_scoring_behavior() {
|
||||
for legacy in [
|
||||
json!(false),
|
||||
json!({"review_scope": {"computer_use_only": true}}),
|
||||
json!(true),
|
||||
] {
|
||||
let enabled = legacy == json!(true);
|
||||
let legacy = serde_json::from_value(legacy).unwrap();
|
||||
let loader = GuardianPolicyLoader::new(Some(&legacy), &ConfigRequirements::default());
|
||||
for required in [false, true] {
|
||||
let mut model = test_model();
|
||||
model.node_repl_auto_review_required = required;
|
||||
let policy = loader.resolve(Some(&model));
|
||||
assert_eq!(policy.scoring_enabled(), enabled && required);
|
||||
assert_eq!(model.computer_use_review_required(), required);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn test_model() -> ModelInfo {
|
||||
serde_json::from_value(json!({
|
||||
"slug": "test-model", "display_name": "test-model",
|
||||
"supported_reasoning_levels": [], "shell_type": "shell_command",
|
||||
"visibility": "list", "supported_in_api": true, "priority": 0,
|
||||
"support_verbosity": false, "truncation_policy": {"mode": "bytes", "limit": 10000},
|
||||
"experimental_supported_tools": []
|
||||
}))
|
||||
.unwrap()
|
||||
}
|
||||
@@ -14,6 +14,7 @@ mod constraint;
|
||||
mod diagnostics;
|
||||
mod filesystem_constraints;
|
||||
mod fingerprint;
|
||||
mod guardian;
|
||||
mod hook_config;
|
||||
mod host_name;
|
||||
mod in_app_browser_requirements;
|
||||
@@ -141,6 +142,7 @@ pub use diagnostics::format_config_error;
|
||||
pub use diagnostics::format_config_error_with_source;
|
||||
pub use diagnostics::io_error_from_config_error;
|
||||
pub use fingerprint::version_for_toml;
|
||||
pub use guardian::GuardianPolicyLoader;
|
||||
pub use hook_config::HookEventsToml;
|
||||
pub use hook_config::HookHandlerConfig;
|
||||
pub use hook_config::HookStateToml;
|
||||
|
||||
@@ -16,6 +16,7 @@ workspace = true
|
||||
anyhow = { workspace = true }
|
||||
codex-analytics = { workspace = true }
|
||||
codex-api = { workspace = true }
|
||||
codex-config = { workspace = true }
|
||||
codex-core = { workspace = true }
|
||||
codex-extension-api = { workspace = true }
|
||||
codex-features = { workspace = true }
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
|
||||
use super::authorization::ScoreAuthorization;
|
||||
use super::config::GuardianV2Config;
|
||||
use super::coverage::GuardianPolicy;
|
||||
use super::metrics::TOOL_CALL_LAG_METRIC;
|
||||
use super::metrics::record_fast_decision;
|
||||
use super::parent_compaction::select_parent_compaction;
|
||||
@@ -18,6 +17,7 @@ use codex_extension_api::ApprovalReviewContributor;
|
||||
use codex_extension_api::ExtensionFuture;
|
||||
use codex_protocol::approvals::GuardianReviewReason;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::openai_models::GuardianModelPolicy;
|
||||
use codex_protocol::openai_models::GuardianReviewMode;
|
||||
use codex_protocol::openai_models::GuardianScope;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -77,18 +77,22 @@ impl GuardianApprovalReviewer {
|
||||
.map_or_else(|| GuardianV2Config::resolve(&config), Ok)
|
||||
.ok();
|
||||
let mut policy = guardian_config.as_ref().map_or_else(
|
||||
|| GuardianPolicy::from_legacy(/*scope*/ None).for_model(model.as_deref()),
|
||||
|| {
|
||||
codex_config::GuardianPolicyLoader::new(
|
||||
Some(&codex_features::FeatureToml::Enabled(true)),
|
||||
&codex_config::ConfigRequirements::default(),
|
||||
)
|
||||
.resolve(model.as_deref())
|
||||
},
|
||||
|config| config.policy_for_model(model.as_deref()),
|
||||
);
|
||||
if model.as_ref().is_some_and(|model| {
|
||||
if let Some(model) = model.as_ref() {
|
||||
config
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.auto_review_required_for_model(&model.slug)
|
||||
}) {
|
||||
policy.enforce_required_model();
|
||||
.constrain_guardian_policy(&mut policy, &model.slug);
|
||||
}
|
||||
let mode = policy.mode(input.category);
|
||||
let mode = policy.review_mode(input.category);
|
||||
if mode != GuardianReviewMode::Adaptive {
|
||||
record_fast_decision(input.metrics.as_deref(), "deferred", "out_of_scope");
|
||||
}
|
||||
@@ -127,7 +131,7 @@ async fn cached_evidence(
|
||||
thread: &CodexThread,
|
||||
input: &ApprovalDecisionInput<'_>,
|
||||
config: &GuardianV2Config,
|
||||
policy: &GuardianPolicy,
|
||||
policy: &GuardianModelPolicy,
|
||||
) -> Result<(), GuardianReviewReason> {
|
||||
let store = input.thread_store;
|
||||
let metrics = input.metrics.as_deref();
|
||||
@@ -180,7 +184,7 @@ async fn cached_evidence(
|
||||
}
|
||||
let action = input.action;
|
||||
if input.category == GuardianScope::ComputerUse
|
||||
&& policy.initial_cua_call
|
||||
&& policy.allows_initial_cua_call()
|
||||
&& action.get("tool_name").and_then(serde_json::Value::as_str) == Some("js")
|
||||
&& action
|
||||
.get("connector_id")
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
use codex_config::GuardianPolicyLoader;
|
||||
use codex_core::config::Config;
|
||||
use codex_features::FeatureToml;
|
||||
use codex_features::GuardianV2ConfigToml;
|
||||
use codex_features::GuardianV2TranscriptConfigToml;
|
||||
use codex_protocol::openai_models::GuardianModelPolicy;
|
||||
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;
|
||||
@@ -38,9 +39,7 @@ 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(super) policy: GuardianPolicy,
|
||||
force_synchronous_review: bool,
|
||||
scoring_disabled: bool,
|
||||
pub(super) policy: GuardianPolicyLoader,
|
||||
pub(crate) transcript: TranscriptConfig,
|
||||
}
|
||||
|
||||
@@ -64,28 +63,22 @@ impl GuardianV2Config {
|
||||
None => GuardianV2ConfigToml::default(),
|
||||
};
|
||||
|
||||
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);
|
||||
let mut resolved = Self::from_overrides(configured.clone())?;
|
||||
// Config.features can be changed after loading, including for reviewer threads.
|
||||
let legacy = FeatureToml::Config(GuardianV2ConfigToml {
|
||||
enabled: Some(config.features.enabled(codex_features::Feature::GuardianV2)),
|
||||
..configured
|
||||
});
|
||||
resolved.policy =
|
||||
GuardianPolicyLoader::new(Some(&legacy), config.config_layer_stack.requirements());
|
||||
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
|
||||
) -> GuardianModelPolicy {
|
||||
self.policy.resolve(model)
|
||||
}
|
||||
|
||||
pub(crate) fn with_model_defaults(
|
||||
@@ -166,8 +159,6 @@ 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)
|
||||
}
|
||||
|
||||
@@ -249,7 +240,13 @@ impl GuardianV2Config {
|
||||
);
|
||||
}
|
||||
|
||||
let policy = GuardianPolicy::from_legacy(configured.review_scope.as_ref());
|
||||
let policy = GuardianPolicyLoader::new(
|
||||
Some(&FeatureToml::Config(GuardianV2ConfigToml {
|
||||
enabled: Some(true),
|
||||
..configured.clone()
|
||||
})),
|
||||
&codex_config::ConfigRequirements::default(),
|
||||
);
|
||||
Ok(Self {
|
||||
local_overrides: configured.clone(),
|
||||
persist_scores: configured.persist_scores.unwrap_or(false),
|
||||
@@ -266,8 +263,6 @@ impl GuardianV2Config {
|
||||
reuse_parent_compaction: configured.reuse_parent_compaction.unwrap_or(true),
|
||||
max_parent_compaction_tokens,
|
||||
policy,
|
||||
force_synchronous_review: false,
|
||||
scoring_disabled: false,
|
||||
transcript: TranscriptConfig {
|
||||
sources: transcript_config
|
||||
.and_then(|transcript| transcript.sources.clone())
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use codex_features::GuardianV2ConfigToml;
|
||||
use codex_features::GuardianV2ReviewScopeConfigToml;
|
||||
use codex_features::GuardianV2TranscriptConfigToml;
|
||||
use codex_protocol::openai_models::GuardianV2ModelConfig;
|
||||
use codex_protocol::openai_models::GuardianV2TranscriptModelConfig;
|
||||
@@ -9,50 +8,8 @@ use pretty_assertions::assert_eq;
|
||||
use super::CLASSIFICATION_OUTPUT_INSTRUCTIONS;
|
||||
use super::DEFAULT_CLASSIFIER_INSTRUCTIONS;
|
||||
use super::GuardianV2Config;
|
||||
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.policy, GuardianPolicy::from_legacy(/*scope*/ None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandboxed_exec_commands_can_be_included() {
|
||||
let config = GuardianV2Config::from_overrides(GuardianV2ConfigToml {
|
||||
review_scope: Some(GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(false),
|
||||
sandboxed_exec_commands: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
config.policy,
|
||||
GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(false),
|
||||
sandboxed_exec_commands: Some(true),
|
||||
}))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn computer_use_only_takes_precedence_over_sandboxed_exec_commands() {
|
||||
let config = GuardianV2Config::from_overrides(GuardianV2ConfigToml {
|
||||
review_scope: Some(GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(true),
|
||||
sandboxed_exec_commands: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(config.policy, GuardianPolicy::from_legacy(/*scope*/ None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn template_policy_is_substituted_before_the_single_truncation() {
|
||||
for max_tokens in [256, 1_000, 2_000] {
|
||||
|
||||
@@ -1,151 +1,29 @@
|
||||
//! 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.
|
||||
//! Model policies review nested actions; legacy tool coverage remains unchanged.
|
||||
//! Selects tool calls for classification using the resolved model policy.
|
||||
|
||||
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::GuardianReviewMode::Adaptive;
|
||||
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),
|
||||
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.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.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 scores_tool(
|
||||
policy: &GuardianModelPolicy,
|
||||
tool: &ToolName,
|
||||
payload: &ToolPayload,
|
||||
scope: Option<GuardianScope>,
|
||||
) -> bool {
|
||||
if scope.map_or(policy.other_tools, |scope| policy.review_mode(scope)) != Adaptive {
|
||||
return false;
|
||||
}
|
||||
if policy.sandboxed_exec_commands.unwrap_or(/*default*/ true)
|
||||
|| !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")
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -62,14 +62,12 @@ impl ThreadLifecycleContributor<Config> for GuardianV2Extension {
|
||||
}
|
||||
};
|
||||
let mut policy = guardian_config.policy_for_model(model.as_deref());
|
||||
if model.as_ref().is_some_and(|model| {
|
||||
if let Some(model) = model.as_ref() {
|
||||
input
|
||||
.config
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.auto_review_required_for_model(&model.slug)
|
||||
}) {
|
||||
policy.enforce_required_model();
|
||||
.constrain_guardian_policy(&mut policy, &model.slug);
|
||||
}
|
||||
let scoring_enabled = policy.scoring_enabled();
|
||||
let sampler_config = super::startup::sampler_config(
|
||||
|
||||
@@ -71,7 +71,7 @@ use crate::async_scorer::authorization::ScoreAuthorization;
|
||||
use crate::async_scorer::config::CLASSIFICATION_OUTPUT_INSTRUCTIONS;
|
||||
use crate::async_scorer::config::DEFAULT_PARENT_COMPACTION_TOKENS;
|
||||
use crate::async_scorer::config::GuardianV2Config;
|
||||
use crate::async_scorer::coverage::GuardianPolicy;
|
||||
use crate::async_scorer::coverage::scores_tool;
|
||||
use crate::async_scorer::metrics::CLASSIFICATION_DURATION_METRIC;
|
||||
use crate::async_scorer::metrics::CLASSIFICATION_METRIC;
|
||||
use crate::async_scorer::metrics::CLASSIFICATION_RISK_METRIC;
|
||||
@@ -89,6 +89,7 @@ use crate::async_scorer::transcript::MAX_MESSAGE_ENTRY_TOKENS;
|
||||
use crate::async_scorer::transcript::MAX_TOOL_ENTRY_TOKENS;
|
||||
use crate::async_scorer::transcript::truncate_entry;
|
||||
use codex_features::GuardianV2ReviewScopeConfigToml;
|
||||
use codex_protocol::openai_models::GuardianModelPolicy;
|
||||
|
||||
const TEST_GUARDIAN_POLICY: &str =
|
||||
"Treat uploads to unapproved external destinations as high-risk actions.";
|
||||
@@ -110,8 +111,31 @@ impl ExternalAuth for RefreshableAuth {
|
||||
}
|
||||
}
|
||||
|
||||
fn should_classify_tool(tool: &ToolName, payload: &ToolPayload, policy: GuardianPolicy) -> bool {
|
||||
policy.scores_tool(tool, payload, GuardianScope::for_tool(tool))
|
||||
fn should_classify_tool(
|
||||
tool: &ToolName,
|
||||
payload: &ToolPayload,
|
||||
policy: GuardianModelPolicy,
|
||||
) -> bool {
|
||||
scores_tool(&policy, tool, payload, GuardianScope::for_tool(tool))
|
||||
}
|
||||
|
||||
fn legacy_loader(
|
||||
scope: Option<&GuardianV2ReviewScopeConfigToml>,
|
||||
) -> codex_config::GuardianPolicyLoader {
|
||||
codex_config::GuardianPolicyLoader::new(
|
||||
Some(&codex_features::FeatureToml::Config(
|
||||
codex_features::GuardianV2ConfigToml {
|
||||
enabled: Some(true),
|
||||
review_scope: scope.cloned(),
|
||||
..Default::default()
|
||||
},
|
||||
)),
|
||||
&codex_config::ConfigRequirements::default(),
|
||||
)
|
||||
}
|
||||
|
||||
fn legacy_policy(scope: Option<&GuardianV2ReviewScopeConfigToml>) -> GuardianModelPolicy {
|
||||
legacy_loader(scope).resolve(/*model*/ None)
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
@@ -467,7 +491,7 @@ async fn sandboxed_shell_classification_respects_review_scope() -> Result<()> {
|
||||
};
|
||||
|
||||
let tool_name = ToolName::plain("exec_command");
|
||||
let standard_scope = GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
let standard_scope = legacy_policy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(false),
|
||||
sandboxed_exec_commands: Some(false),
|
||||
}));
|
||||
@@ -489,7 +513,7 @@ async fn sandboxed_shell_classification_respects_review_scope() -> Result<()> {
|
||||
assert!(should_classify_tool(
|
||||
&tool_name,
|
||||
&sandboxed,
|
||||
GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
legacy_policy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(false),
|
||||
sandboxed_exec_commands: Some(true),
|
||||
})),
|
||||
@@ -587,11 +611,7 @@ fn computer_use_only_classification_recognizes_direct_and_code_mode_tools() {
|
||||
(ToolName::plain("exec_command"), false),
|
||||
] {
|
||||
assert_eq!(
|
||||
should_classify_tool(
|
||||
&tool_name,
|
||||
&payload,
|
||||
GuardianPolicy::from_legacy(/*scope*/ None)
|
||||
),
|
||||
should_classify_tool(&tool_name, &payload, legacy_policy(/*scope*/ None)),
|
||||
expected,
|
||||
"unexpected classification scope for {tool_name}"
|
||||
);
|
||||
@@ -612,7 +632,7 @@ async fn computer_use_only_scores_cannot_approve_other_actions() -> Result<()> {
|
||||
.expect("Guardian v2 should have initialized")
|
||||
.as_ref()
|
||||
.clone();
|
||||
config.policy = GuardianPolicy::from_legacy(/*scope*/ None);
|
||||
config.policy = legacy_loader(/*scope*/ None);
|
||||
thread_store.insert(config);
|
||||
thread_store.insert(SecurityRiskScore {
|
||||
scores: BTreeMap::from([("action_risk".to_owned(), 0.25)]),
|
||||
@@ -2202,7 +2222,7 @@ async fn contributor_skips_required_models_in_standard_scope() -> Result<()> {
|
||||
.expect("Guardian v2 should have initialized")
|
||||
.as_ref()
|
||||
.clone();
|
||||
guardian_config.policy = GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
guardian_config.policy = legacy_loader(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(false),
|
||||
sandboxed_exec_commands: Some(false),
|
||||
}));
|
||||
@@ -2431,7 +2451,7 @@ async fn assert_compaction_approval_policy(thread_context_enabled: bool) -> Resu
|
||||
.get::<GuardianV2Config>()
|
||||
.expect("Guardian configuration"))
|
||||
.clone();
|
||||
config.policy = GuardianPolicy::from_legacy(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
config.policy = legacy_loader(Some(&GuardianV2ReviewScopeConfigToml {
|
||||
computer_use_only: Some(computer_use_only),
|
||||
sandboxed_exec_commands: Some(true),
|
||||
}));
|
||||
@@ -3324,7 +3344,10 @@ async fn cached_approval(
|
||||
let action = serde_json::from_str(action).unwrap_or(serde_json::Value::Null);
|
||||
let category = match review_scope(&action) {
|
||||
Some(category) => category,
|
||||
None if store.get::<super::GuardianV2Config>()?.policy.other_tools
|
||||
None if store
|
||||
.get::<super::GuardianV2Config>()?
|
||||
.policy_for_model(store.get::<ModelInfo>().as_deref())
|
||||
.other_tools
|
||||
== codex_protocol::openai_models::GuardianReviewMode::Adaptive =>
|
||||
{
|
||||
codex_protocol::openai_models::GuardianScope::Shell
|
||||
|
||||
@@ -28,7 +28,7 @@ use super::action::GuardianAction;
|
||||
use super::authorization::ScoreAuthorization;
|
||||
use super::classification::Classification;
|
||||
use super::config::GuardianV2Config;
|
||||
use super::coverage::UnscoredAction;
|
||||
use super::coverage::scores_tool;
|
||||
use super::extension::GuardianV2Extension;
|
||||
use super::metrics::record_classification;
|
||||
use super::parent_compaction::ParentCompactionError;
|
||||
@@ -36,6 +36,7 @@ use super::parent_compaction::select_parent_compaction;
|
||||
use super::sampler::LunaSampler;
|
||||
use super::score::GuardianV2ScoreProgress;
|
||||
use super::score::record_fail_closed_score;
|
||||
use codex_protocol::openai_models::GuardianUnscoredAction as UnscoredAction;
|
||||
|
||||
impl GuardianV2Extension {
|
||||
pub(super) async fn score_tool(&self, input: ToolStartInput<'_>) {
|
||||
@@ -75,7 +76,7 @@ impl GuardianV2Extension {
|
||||
{
|
||||
return;
|
||||
}
|
||||
if !policy.scores_tool(input.tool_name, input.payload, scope) {
|
||||
if !scores_tool(&policy, input.tool_name, input.payload, scope) {
|
||||
match policy.unscored_action {
|
||||
UnscoredAction::Ignore => {}
|
||||
UnscoredAction::AgeScore => {
|
||||
@@ -183,7 +184,7 @@ impl GuardianV2Extension {
|
||||
return;
|
||||
}
|
||||
// A required model keeps synchronous review outside its CUA allowance.
|
||||
if !(scope == Some(GuardianScope::ComputerUse) && policy.initial_cua_call)
|
||||
if !(scope == Some(GuardianScope::ComputerUse) && policy.allows_initial_cua_call())
|
||||
&& parent_model.as_ref().is_some_and(|model| {
|
||||
config
|
||||
.config_layer_stack
|
||||
|
||||
@@ -39,6 +39,7 @@ mod guardian;
|
||||
pub use guardian::GuardianModelPolicy;
|
||||
pub use guardian::GuardianReviewMode;
|
||||
pub use guardian::GuardianScope;
|
||||
pub use guardian::GuardianUnscoredAction;
|
||||
|
||||
#[path = "openai_models/guardian_v2.rs"]
|
||||
mod guardian_v2;
|
||||
|
||||
@@ -12,9 +12,10 @@ 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)]
|
||||
#[derive(Debug, Default, Serialize, Deserialize, Clone, Copy, PartialEq, Eq, TS, JsonSchema)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum GuardianReviewMode {
|
||||
#[default]
|
||||
Disabled,
|
||||
Synchronous,
|
||||
/// Use a current low-risk score; otherwise run synchronous review.
|
||||
@@ -23,6 +24,17 @@ pub enum GuardianReviewMode {
|
||||
Unknown,
|
||||
}
|
||||
|
||||
/// How actions outside adaptive coverage affect cached classification evidence.
|
||||
#[derive(Debug, Default, Serialize, Deserialize, Clone, Copy, PartialEq, Eq, TS, JsonSchema)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum GuardianUnscoredAction {
|
||||
Ignore,
|
||||
AgeScore,
|
||||
#[default]
|
||||
#[serde(other)]
|
||||
InvalidateScore,
|
||||
}
|
||||
|
||||
/// A complete model policy. Omitted scopes are disabled; unknown fields are ignored.
|
||||
/// Code Mode wrappers have no approval scope; their nested tools follow this policy.
|
||||
#[derive(Debug, Default, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
|
||||
@@ -39,6 +51,17 @@ pub struct GuardianModelPolicy {
|
||||
pub network: Option<GuardianReviewMode>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub permissions: Option<GuardianReviewMode>,
|
||||
/// Coverage for tools without an approval category.
|
||||
#[serde(default)]
|
||||
pub other_tools: GuardianReviewMode,
|
||||
#[serde(default)]
|
||||
pub unscored_action: GuardianUnscoredAction,
|
||||
/// Omission retains the existing allowance for adaptive computer use.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub initial_cua_call: Option<bool>,
|
||||
/// Whether adaptive shell coverage includes ordinary sandboxed commands.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub sandboxed_exec_commands: Option<bool>,
|
||||
}
|
||||
|
||||
impl GuardianModelPolicy {
|
||||
@@ -53,6 +76,39 @@ impl GuardianModelPolicy {
|
||||
}
|
||||
.unwrap_or(GuardianReviewMode::Disabled)
|
||||
}
|
||||
|
||||
pub fn scoring_enabled(&self) -> bool {
|
||||
[
|
||||
self.computer_use,
|
||||
self.shell,
|
||||
self.file_changes,
|
||||
self.mcp,
|
||||
self.network,
|
||||
self.permissions,
|
||||
]
|
||||
.contains(&Some(GuardianReviewMode::Adaptive))
|
||||
}
|
||||
|
||||
pub fn disable_scoring(&mut self) {
|
||||
for mode in [
|
||||
&mut self.computer_use,
|
||||
&mut self.shell,
|
||||
&mut self.file_changes,
|
||||
&mut self.mcp,
|
||||
&mut self.network,
|
||||
&mut self.permissions,
|
||||
] {
|
||||
if *mode == Some(GuardianReviewMode::Adaptive) {
|
||||
*mode = Some(GuardianReviewMode::Synchronous);
|
||||
}
|
||||
}
|
||||
self.other_tools = GuardianReviewMode::Synchronous;
|
||||
}
|
||||
|
||||
pub fn allows_initial_cua_call(&self) -> bool {
|
||||
self.initial_cua_call
|
||||
.unwrap_or(self.computer_use == Some(GuardianReviewMode::Adaptive))
|
||||
}
|
||||
}
|
||||
|
||||
/// Approval categories understood by this client. Future catalog keys are ignored.
|
||||
|
||||
Reference in New Issue
Block a user