mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Remove path-bearing fields from Guardian review analytics (#44352)
## What changed Replace full additional permission profiles with network-only metadata in Guardian reviewed actions, and remove the `program` field from `execve` analytics. Preserve absent and empty permission requests and the optional `network.enabled` value. ## Testing Add serialization tests for enabled and disabled network permissions, absent and empty permission requests, and an unset network permission value. GitOrigin-RevId: 9904e5f5bd6723d0c09fb423b4818cb8bf12d397
This commit is contained in:
@@ -22,6 +22,7 @@ use crate::events::CodexRuntimeMetadata;
|
||||
use crate::events::CodexToolItemEventBase;
|
||||
use crate::events::CodexTurnEventRequest;
|
||||
use crate::events::FinalApprovalOutcome;
|
||||
use crate::events::GuardianAdditionalPermissions;
|
||||
use crate::events::GuardianApprovalRequestSource;
|
||||
use crate::events::GuardianReviewDecision;
|
||||
use crate::events::GuardianReviewEventParams;
|
||||
@@ -172,9 +173,12 @@ use codex_protocol::approvals::NetworkApprovalProtocol;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile as CorePermissionProfile;
|
||||
use codex_protocol::models::SandboxPermissions;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::GuardianCommandSource;
|
||||
use codex_protocol::protocol::HookEventName;
|
||||
use codex_protocol::protocol::HookExecutionMode;
|
||||
use codex_protocol::protocol::HookHandlerType;
|
||||
@@ -2520,6 +2524,81 @@ async fn compaction_event_ingests_custom_fact() {
|
||||
assert_eq!(payload[0]["event_params"]["status"], "failed");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn execve_serializes_enabled_network_permissions() {
|
||||
let permissions: AdditionalPermissionProfile = serde_json::from_value(json!({
|
||||
"network": { "enabled": true },
|
||||
}))
|
||||
.expect("network permissions");
|
||||
|
||||
let action = GuardianReviewedAction::Execve {
|
||||
source: GuardianCommandSource::UnifiedExec,
|
||||
additional_permissions: Some(GuardianAdditionalPermissions::from(&permissions)),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
serde_json::to_value(action).expect("serialize action"),
|
||||
json!({
|
||||
"type": "execve",
|
||||
"source": "unified_exec",
|
||||
"additional_permissions": {
|
||||
"network": { "enabled": true },
|
||||
},
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unified_exec_serializes_disabled_network_permissions() {
|
||||
let permissions: AdditionalPermissionProfile = serde_json::from_value(json!({
|
||||
"network": { "enabled": false },
|
||||
}))
|
||||
.expect("network permissions");
|
||||
|
||||
let action = GuardianReviewedAction::UnifiedExec {
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
additional_permissions: Some(GuardianAdditionalPermissions::from(&permissions)),
|
||||
tty: false,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
serde_json::to_value(action).expect("serialize action"),
|
||||
json!({
|
||||
"type": "unified_exec",
|
||||
"sandbox_permissions": "with_additional_permissions",
|
||||
"additional_permissions": {
|
||||
"network": { "enabled": false },
|
||||
},
|
||||
"tty": false,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_metadata_preserves_absent_and_empty_requests() {
|
||||
for (input, expected) in [
|
||||
(json!(null), json!(null)),
|
||||
(json!({}), json!({ "network": null })),
|
||||
(
|
||||
json!({ "network": { "enabled": null } }),
|
||||
json!({
|
||||
"network": { "enabled": null },
|
||||
}),
|
||||
),
|
||||
] {
|
||||
let permissions: Option<AdditionalPermissionProfile> =
|
||||
serde_json::from_value(input).expect("optional permissions");
|
||||
let metadata = permissions
|
||||
.as_ref()
|
||||
.map(GuardianAdditionalPermissions::from);
|
||||
|
||||
assert_eq!(
|
||||
serde_json::to_value(metadata).expect("serialize permissions"),
|
||||
expected,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn guardian_review_event_ingests_custom_fact_with_optional_target_item() {
|
||||
let mut reducer = AnalyticsReducer::default();
|
||||
|
||||
@@ -337,16 +337,40 @@ pub enum GuardianApprovalRequestSource {
|
||||
DelegatedSubagent,
|
||||
}
|
||||
|
||||
/// Path-free permission metadata for a Guardian reviewed action.
|
||||
#[derive(Clone, Debug, Serialize)]
|
||||
pub struct GuardianAdditionalPermissions {
|
||||
network: Option<GuardianNetworkPermissions>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Serialize)]
|
||||
struct GuardianNetworkPermissions {
|
||||
enabled: Option<bool>,
|
||||
}
|
||||
|
||||
impl From<&AdditionalPermissionProfile> for GuardianAdditionalPermissions {
|
||||
fn from(permissions: &AdditionalPermissionProfile) -> Self {
|
||||
Self {
|
||||
network: permissions
|
||||
.network
|
||||
.as_ref()
|
||||
.map(|network| GuardianNetworkPermissions {
|
||||
enabled: network.enabled,
|
||||
}),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Serialize)]
|
||||
#[serde(tag = "type", rename_all = "snake_case")]
|
||||
pub enum GuardianReviewedAction {
|
||||
Shell {
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<GuardianAdditionalPermissions>,
|
||||
},
|
||||
UnifiedExec {
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<GuardianAdditionalPermissions>,
|
||||
tty: bool,
|
||||
},
|
||||
WriteStdin {
|
||||
@@ -354,8 +378,7 @@ pub enum GuardianReviewedAction {
|
||||
},
|
||||
Execve {
|
||||
source: GuardianCommandSource,
|
||||
program: String,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
additional_permissions: Option<GuardianAdditionalPermissions>,
|
||||
},
|
||||
ApplyPatch {},
|
||||
NetworkAccess {
|
||||
|
||||
@@ -14,6 +14,7 @@ use std::time::UNIX_EPOCH;
|
||||
pub use accepted_lines::fingerprint_hash;
|
||||
pub use client::AnalyticsEventsClient;
|
||||
pub use events::AppServerRpcTransport;
|
||||
pub use events::GuardianAdditionalPermissions;
|
||||
pub use events::GuardianApprovalRequestSource;
|
||||
pub use events::GuardianReviewAnalyticsResult;
|
||||
pub use events::GuardianReviewDecision;
|
||||
|
||||
@@ -511,7 +511,7 @@ pub(crate) fn guardian_reviewed_action(
|
||||
..
|
||||
} => GuardianReviewedAction::UnifiedExec {
|
||||
sandbox_permissions: *sandbox_permissions,
|
||||
additional_permissions: additional_permissions.clone(),
|
||||
additional_permissions: additional_permissions.as_ref().map(Into::into),
|
||||
tty: *tty,
|
||||
},
|
||||
GuardianApprovalRequest::WriteStdin { tty, .. } => {
|
||||
@@ -520,13 +520,11 @@ pub(crate) fn guardian_reviewed_action(
|
||||
#[cfg(unix)]
|
||||
GuardianApprovalRequest::Execve {
|
||||
source,
|
||||
program,
|
||||
additional_permissions,
|
||||
..
|
||||
} => GuardianReviewedAction::Execve {
|
||||
source: *source,
|
||||
program: program.clone(),
|
||||
additional_permissions: additional_permissions.clone(),
|
||||
additional_permissions: additional_permissions.as_ref().map(Into::into),
|
||||
},
|
||||
GuardianApprovalRequest::ApplyPatch { .. } => GuardianReviewedAction::ApplyPatch {},
|
||||
GuardianApprovalRequest::NetworkAccess { protocol, port, .. } => {
|
||||
|
||||
Reference in New Issue
Block a user