mirror of
https://github.com/openai/codex.git
synced 2026-11-09 08:40:33 +00:00
Persist Guardian V2 risk scores without restoring them (#40884)
## What changed - Append accepted Guardian V2 classification results to rollout history for non-ephemeral threads. - Keep resumed and forked threads from restoring a persisted score into active Guardian state. ## Testing - Verify that asynchronous scoring records the resulting `SecurityRiskScore` rollout item. - Seed resume and fork tests with a persisted score and verify that approvals ignore it. GitOrigin-RevId: 40d63a2ea7e7cf6398402c6aed7d5e5727dc9d68
This commit is contained in:
1 parent
f5420174da
commit
10d5a603ae
6 files changed
+70
-23
No files matched your search
Generated
+1
@@ -3329,6 +3329,7 @@ dependencies = [
|
|||||||
"codex-core",
|
"codex-core",
|
||||||
"codex-extension-api",
|
"codex-extension-api",
|
||||||
"codex-features",
|
"codex-features",
|
||||||
|
"codex-history",
|
||||||
"codex-http-client",
|
"codex-http-client",
|
||||||
"codex-login",
|
"codex-login",
|
||||||
"codex-model-provider",
|
"codex-model-provider",
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
use std::io::Write;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use std::sync::Mutex;
|
use std::sync::Mutex;
|
||||||
use std::sync::atomic::AtomicUsize;
|
use std::sync::atomic::AtomicUsize;
|
||||||
@@ -8,6 +9,7 @@ use anyhow::Result;
|
|||||||
use app_test_support::MockResponsesConfig;
|
use app_test_support::MockResponsesConfig;
|
||||||
use app_test_support::TestAppServer;
|
use app_test_support::TestAppServer;
|
||||||
use app_test_support::create_fake_rollout;
|
use app_test_support::create_fake_rollout;
|
||||||
|
use app_test_support::rollout_path;
|
||||||
use axum::Json;
|
use axum::Json;
|
||||||
use axum::Router;
|
use axum::Router;
|
||||||
use axum::extract::State;
|
use axum::extract::State;
|
||||||
@@ -571,14 +573,34 @@ async fn guardian_v2_routes_scoped_tool_approvals(
|
|||||||
| ThreadLifecycle::RootUserRestriction
|
| ThreadLifecycle::RootUserRestriction
|
||||||
| ThreadLifecycle::RootUserInputRestriction
|
| ThreadLifecycle::RootUserInputRestriction
|
||||||
| ThreadLifecycle::RootUserInputHookBlocked => None,
|
| ThreadLifecycle::RootUserInputHookBlocked => None,
|
||||||
ThreadLifecycle::Resume | ThreadLifecycle::Fork => Some(create_fake_rollout(
|
ThreadLifecycle::Resume | ThreadLifecycle::Fork => {
|
||||||
codex_home.path(),
|
let thread_id = create_fake_rollout(
|
||||||
"2025-01-05T12-00-00",
|
codex_home.path(),
|
||||||
"2025-01-05T12:00:00Z",
|
"2025-01-05T12-00-00",
|
||||||
USER_CONTEXT,
|
"2025-01-05T12:00:00Z",
|
||||||
Some("mock_provider"),
|
USER_CONTEXT,
|
||||||
/*git_info*/ None,
|
Some("mock_provider"),
|
||||||
)?),
|
/*git_info*/ None,
|
||||||
|
)?;
|
||||||
|
let mut rollout = std::fs::OpenOptions::new().append(true).open(rollout_path(
|
||||||
|
codex_home.path(),
|
||||||
|
"2025-01-05T12-00-00",
|
||||||
|
&thread_id,
|
||||||
|
))?;
|
||||||
|
writeln!(
|
||||||
|
rollout,
|
||||||
|
"{}",
|
||||||
|
json!({
|
||||||
|
"timestamp": "2025-01-05T12:00:00Z",
|
||||||
|
"type": "security_risk_score",
|
||||||
|
"payload": {
|
||||||
|
"scores": { "action_risk": 0.0 },
|
||||||
|
"sampled_at": "2025-01-05T12:00:00Z",
|
||||||
|
},
|
||||||
|
})
|
||||||
|
)?;
|
||||||
|
Some(thread_id)
|
||||||
|
}
|
||||||
};
|
};
|
||||||
let mut app_server = TestAppServer::builder()
|
let mut app_server = TestAppServer::builder()
|
||||||
.with_codex_home(codex_home.path())
|
.with_codex_home(codex_home.path())
|
||||||
@@ -1143,7 +1165,7 @@ async fn guardian_v2_required_model_bypasses_scoring_and_runs_full_reviews() ->
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn resumed_thread_starts_without_guardian_score() -> Result<()> {
|
async fn resumed_thread_ignores_persisted_guardian_score() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
guardian_v2_routes_tool_approvals(
|
guardian_v2_routes_tool_approvals(
|
||||||
GuardianRisk::Low,
|
GuardianRisk::Low,
|
||||||
@@ -1156,7 +1178,7 @@ async fn resumed_thread_starts_without_guardian_score() -> Result<()> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn forked_thread_starts_without_guardian_score() -> Result<()> {
|
async fn forked_thread_ignores_persisted_guardian_score() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
guardian_v2_routes_tool_approvals(
|
guardian_v2_routes_tool_approvals(
|
||||||
GuardianRisk::Low,
|
GuardianRisk::Low,
|
||||||
|
|||||||
@@ -31,7 +31,6 @@ use codex_protocol::protocol::MultiAgentVersion;
|
|||||||
use codex_protocol::protocol::ThreadHistoryMode;
|
use codex_protocol::protocol::ThreadHistoryMode;
|
||||||
use codex_protocol::protocol::ThreadSource;
|
use codex_protocol::protocol::ThreadSource;
|
||||||
use codex_protocol::protocol::TurnEnvironmentSelections;
|
use codex_protocol::protocol::TurnEnvironmentSelections;
|
||||||
use codex_protocol::security_risk::SecurityRiskScore;
|
|
||||||
use codex_skills::SkillError;
|
use codex_skills::SkillError;
|
||||||
use std::sync::OnceLock;
|
use std::sync::OnceLock;
|
||||||
use tokio::sync::Semaphore;
|
use tokio::sync::Semaphore;
|
||||||
@@ -752,18 +751,6 @@ impl Session {
|
|||||||
config.current_time_reminder.as_ref(),
|
config.current_time_reminder.as_ref(),
|
||||||
external_time_provider,
|
external_time_provider,
|
||||||
)?;
|
)?;
|
||||||
if thread_extension_init.get::<SecurityRiskScore>().is_none()
|
|
||||||
&& let Some(score) = initial_history
|
|
||||||
.get_rollout_items()
|
|
||||||
.iter()
|
|
||||||
.rev()
|
|
||||||
.find_map(|item| match item {
|
|
||||||
RolloutItem::SecurityRiskScore(score) => Some(score),
|
|
||||||
_ => None,
|
|
||||||
})
|
|
||||||
{
|
|
||||||
thread_extension_init.insert(score.clone());
|
|
||||||
}
|
|
||||||
let selected_capability_roots =
|
let selected_capability_roots =
|
||||||
match thread_extension_init.get::<Vec<SelectedCapabilityRoot>>() {
|
match thread_extension_init.get::<Vec<SelectedCapabilityRoot>>() {
|
||||||
Some(roots) => roots.as_ref().clone(),
|
Some(roots) => roots.as_ref().clone(),
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ codex-api = { workspace = true }
|
|||||||
codex-core = { workspace = true }
|
codex-core = { workspace = true }
|
||||||
codex-extension-api = { workspace = true }
|
codex-extension-api = { workspace = true }
|
||||||
codex-features = { workspace = true }
|
codex-features = { workspace = true }
|
||||||
|
codex-history = { workspace = true }
|
||||||
codex-http-client = { workspace = true }
|
codex-http-client = { workspace = true }
|
||||||
codex-login = { workspace = true }
|
codex-login = { workspace = true }
|
||||||
codex-model-provider = { workspace = true }
|
codex-model-provider = { workspace = true }
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ use codex_extension_api::ToolName;
|
|||||||
use codex_extension_api::ToolPayload;
|
use codex_extension_api::ToolPayload;
|
||||||
use codex_extension_api::ToolStartInput;
|
use codex_extension_api::ToolStartInput;
|
||||||
use codex_features::Feature;
|
use codex_features::Feature;
|
||||||
|
use codex_history::RolloutItem;
|
||||||
use codex_login::AgentIdentityAuthPolicy;
|
use codex_login::AgentIdentityAuthPolicy;
|
||||||
use codex_login::AuthManager;
|
use codex_login::AuthManager;
|
||||||
use codex_model_provider::create_model_provider;
|
use codex_model_provider::create_model_provider;
|
||||||
@@ -796,6 +797,19 @@ impl GuardianV2Extension {
|
|||||||
.latest_scored_tool_call
|
.latest_scored_tool_call
|
||||||
.fetch_max(tool_call_index, Ordering::Release);
|
.fetch_max(tool_call_index, Ordering::Release);
|
||||||
classification_finished_at = Some(Instant::now());
|
classification_finished_at = Some(Instant::now());
|
||||||
|
if !config.ephemeral
|
||||||
|
&& let Err(error) = thread
|
||||||
|
.append_rollout_items(&[RolloutItem::SecurityRiskScore(score)])
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
tracing::warn!(
|
||||||
|
%thread_id,
|
||||||
|
%turn_id,
|
||||||
|
%call_id,
|
||||||
|
%error,
|
||||||
|
"failed to persist Guardian V2 classification result"
|
||||||
|
);
|
||||||
|
}
|
||||||
Ok("success")
|
Ok("success")
|
||||||
}
|
}
|
||||||
.await;
|
.await;
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ use codex_extension_api::ToolName;
|
|||||||
use codex_extension_api::ToolPayload;
|
use codex_extension_api::ToolPayload;
|
||||||
use codex_extension_api::ToolStartInput;
|
use codex_extension_api::ToolStartInput;
|
||||||
use codex_features::Feature;
|
use codex_features::Feature;
|
||||||
|
use codex_history::RolloutItem;
|
||||||
use codex_login::AuthManager;
|
use codex_login::AuthManager;
|
||||||
use codex_login::CodexAuth;
|
use codex_login::CodexAuth;
|
||||||
use codex_login::ExternalAuth;
|
use codex_login::ExternalAuth;
|
||||||
@@ -1726,6 +1727,27 @@ async fn contributor_samples_tool_calls_with_the_existing_luna_pool() -> Result<
|
|||||||
}
|
}
|
||||||
);
|
);
|
||||||
assert!(score.sampled_at.is_some());
|
assert!(score.sampled_at.is_some());
|
||||||
|
test.codex.ensure_rollout_materialized().await;
|
||||||
|
let persisted_score = tokio::time::timeout(Duration::from_secs(5), async {
|
||||||
|
loop {
|
||||||
|
if let Some(persisted_score) = test
|
||||||
|
.codex
|
||||||
|
.load_history(/*include_archived*/ false)
|
||||||
|
.await?
|
||||||
|
.items
|
||||||
|
.into_iter()
|
||||||
|
.find_map(|item| match item {
|
||||||
|
RolloutItem::SecurityRiskScore(score) => Some(score),
|
||||||
|
_ => None,
|
||||||
|
})
|
||||||
|
{
|
||||||
|
return Ok::<_, anyhow::Error>(persisted_score);
|
||||||
|
}
|
||||||
|
tokio::task::yield_now().await;
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.await??;
|
||||||
|
assert_eq!(&persisted_score, score.as_ref());
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.fast_approval_decision(
|
.fast_approval_decision(
|
||||||
|
|||||||
Reference in new issue
Block a user