From 590ed405d28da67ef9689a137ba9dbc5d314f457 Mon Sep 17 00:00:00 2001 From: Albin Cassirer Date: Sun, 19 Apr 2026 14:56:32 -0700 Subject: [PATCH] [observability] Add field policy gate Introduce FieldPolicy as the small shared primitive that decides whether a sink may inspect a field before serialization. The policy combines the field detail limit with the allowed data classes, keeping destination-specific rules outside the event definitions. Add tests for the basic remote-safe shape and for the important safety invariant: denied content and secret-risk fields are filtered before serialization, proven with fields that panic if a visitor tries to serialize them. --- codex-rs/observability/src/lib.rs | 70 ++++++++++++++++++++ codex-rs/observability/tests/derive.rs | 89 ++++++++++++++++++++++++++ 2 files changed, 159 insertions(+) diff --git a/codex-rs/observability/src/lib.rs b/codex-rs/observability/src/lib.rs index bef7314bed..ca4ac8514e 100644 --- a/codex-rs/observability/src/lib.rs +++ b/codex-rs/observability/src/lib.rs @@ -83,6 +83,41 @@ impl FieldMeta { } } +/// Decides whether a sink may read an observation field. +/// +/// Policies are checked before serialization. This matters because denied +/// fields may contain content, secrets, or large trace payloads that remote +/// sinks must not materialize even transiently. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct FieldPolicy { + max_detail: DetailLevel, + allowed_classes: &'static [DataClass], +} + +impl FieldPolicy { + /// Creates a policy that permits fields at or below the configured detail + /// limit and whose data class is present in the allowed class list. + pub const fn new(max_detail: DetailLevel, allowed_classes: &'static [DataClass]) -> Self { + Self { + max_detail, + allowed_classes, + } + } + + /// Returns true when a sink may inspect and serialize a field. + pub fn allows(self, meta: FieldMeta) -> bool { + let detail_allowed = match self.max_detail { + DetailLevel::Basic => matches!(meta.detail, DetailLevel::Basic), + DetailLevel::Detailed => { + matches!(meta.detail, DetailLevel::Basic | DetailLevel::Detailed) + } + DetailLevel::Trace => true, + }; + + detail_allowed && self.allowed_classes.contains(&meta.class) + } +} + /// Coarse detail level for a field. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum DetailLevel { @@ -124,4 +159,39 @@ mod tests { } ); } + + #[test] + fn field_policy_requires_allowed_detail_and_class() { + let policy = FieldPolicy::new( + DetailLevel::Basic, + &[DataClass::Identifier, DataClass::Operational], + ); + let cases = [ + ( + FieldMeta::new(DetailLevel::Basic, DataClass::Identifier), + true, + ), + ( + FieldMeta::new(DetailLevel::Basic, DataClass::Operational), + true, + ), + ( + FieldMeta::new(DetailLevel::Detailed, DataClass::Operational), + false, + ), + ( + FieldMeta::new(DetailLevel::Basic, DataClass::Content), + false, + ), + ( + FieldMeta::new(DetailLevel::Basic, DataClass::SecretRisk), + false, + ), + ]; + + assert_eq!( + cases.map(|(meta, _expected)| policy.allows(meta)), + cases.map(|(_meta, expected)| expected) + ); + } } diff --git a/codex-rs/observability/tests/derive.rs b/codex-rs/observability/tests/derive.rs index 201b15433f..7506676052 100644 --- a/codex-rs/observability/tests/derive.rs +++ b/codex-rs/observability/tests/derive.rs @@ -1,9 +1,12 @@ use codex_observability::DataClass; use codex_observability::DetailLevel; use codex_observability::FieldMeta; +use codex_observability::FieldPolicy; use codex_observability::Observation; use codex_observability::ObservationFieldVisitor; use pretty_assertions::assert_eq; +use serde::Serialize; +use serde::Serializer; use serde_json::Value; #[derive(Observation)] @@ -19,6 +22,33 @@ struct TurnConfigResolved<'a> { model: &'a str, } +#[derive(Observation)] +#[observation(name = "test.policy_filtered")] +struct PolicyFiltered<'a> { + #[obs(level = "basic", class = "identifier")] + thread_id: &'a str, + + #[obs(level = "basic", class = "operational")] + status: &'a str, + + #[obs(level = "trace", class = "content")] + raw_prompt: PanicsIfSerialized, + + #[obs(level = "basic", class = "secret_risk")] + api_key: PanicsIfSerialized, +} + +struct PanicsIfSerialized; + +impl Serialize for PanicsIfSerialized { + fn serialize(&self, _serializer: S) -> Result + where + S: Serializer, + { + panic!("denied observation field should not be serialized") + } +} + #[derive(Debug, PartialEq)] struct CapturedField { name: &'static str, @@ -31,6 +61,11 @@ struct CapturingVisitor { fields: Vec, } +struct PolicyCapturingVisitor { + policy: FieldPolicy, + fields: Vec, +} + impl ObservationFieldVisitor for CapturingVisitor { fn field( &mut self, @@ -46,6 +81,25 @@ impl ObservationFieldVisitor for CapturingVisitor { } } +impl ObservationFieldVisitor for PolicyCapturingVisitor { + fn field( + &mut self, + name: &'static str, + meta: FieldMeta, + value: &T, + ) { + if !self.policy.allows(meta) { + return; + } + + let value = match serde_json::to_value(value) { + Ok(value) => value, + Err(err) => panic!("allowed field should serialize: {err}"), + }; + self.fields.push(CapturedField { name, meta, value }); + } +} + #[test] fn derive_visits_annotated_fields_with_metadata() { let event = TurnConfigResolved { @@ -79,3 +133,38 @@ fn derive_visits_annotated_fields_with_metadata() { ] ); } + +#[test] +fn policy_visitor_does_not_serialize_denied_fields() { + let event = PolicyFiltered { + thread_id: "thread-1", + status: "completed", + raw_prompt: PanicsIfSerialized, + api_key: PanicsIfSerialized, + }; + let mut visitor = PolicyCapturingVisitor { + policy: FieldPolicy::new( + DetailLevel::Basic, + &[DataClass::Identifier, DataClass::Operational], + ), + fields: Vec::new(), + }; + + event.visit_fields(&mut visitor); + + assert_eq!( + visitor.fields, + vec![ + CapturedField { + name: "thread_id", + meta: FieldMeta::new(DetailLevel::Basic, DataClass::Identifier), + value: Value::String("thread-1".to_string()), + }, + CapturedField { + name: "status", + meta: FieldMeta::new(DetailLevel::Basic, DataClass::Operational), + value: Value::String("completed".to_string()), + }, + ] + ); +}