Address reviewer's comment:

P1 Badge Avoid tracing when filter resolution errors occur

If resolving a scope’s filter fails, scope_resolution logs the error
and returns None without marking the code object as skipped.
should_trace_code treats a None resolution as “trace”, so subsequent
argument/local capture runs with value_policy=None, meaning values
are recorded unfiltered even though the user requested filtering. A
failure to read co_filename/co_qualname or any internal engine error
therefore silently disables all filtering for that frame and can leak
sensitive data. The safer behaviour would be to skip the scope or disable
tracing on resolution errors.

Author: tzanko-matev

codetracer-python-recorder/src/runtime/mod.rs

@@ -22,7 +22,7 @@ use value_capture::{
 use std::collections::{hash_map::Entry, HashMap, HashSet};
 use std::fs;
 use std::path::{Path, PathBuf};
-#[cfg(feature = "integration-test")]
+#[cfg(any(feature = "integration-test", test))]
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 #[cfg(feature = "integration-test")]
@@ -117,6 +117,13 @@ enum ShouldTrace {
     SkipAndDisable,
 }
 
+#[derive(Debug)]
+enum ScopeResolutionState {
+    NotConfigured,
+    Resolved(Arc<ScopeResolution>),
+    Error,
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 enum FailureStage {
     PyStart,
@@ -190,6 +197,14 @@ static FAILURE_MODE: OnceLock<Option<FailureMode>> = OnceLock::new();
 #[cfg(feature = "integration-test")]
 static FAILURE_TRIGGERED: AtomicBool = AtomicBool::new(false);
 
+#[cfg(test)]
+static FORCE_SCOPE_RESOLUTION_ERROR: AtomicBool = AtomicBool::new(false);
+
+#[cfg(test)]
+fn take_forced_scope_resolution_error() -> bool {
+    FORCE_SCOPE_RESOLUTION_ERROR.swap(false, Ordering::SeqCst)
+}
+
 #[cfg(feature = "integration-test")]
 fn configured_failure_mode() -> Option<FailureMode> {
     *FAILURE_MODE.get_or_init(|| {
@@ -392,12 +407,28 @@ impl RuntimeTracer {
         &mut self,
         py: Python<'_>,
         code: &CodeObjectWrapper,
-    ) -> Option<Arc<ScopeResolution>> {
-        let engine = self.trace_filter.as_ref()?;
+    ) -> ScopeResolutionState {
+        let engine = match self.trace_filter.as_ref() {
+            Some(engine) => engine,
+            None => return ScopeResolutionState::NotConfigured,
+        };
         let code_id = code.id();
 
         if let Some(existing) = self.scope_cache.get(&code_id) {
-            return Some(existing.clone());
+            return ScopeResolutionState::Resolved(existing.clone());
+        }
+
+        #[cfg(test)]
+        if take_forced_scope_resolution_error() {
+            with_error_code(ErrorCode::Unknown, || {
+                let _mute = ScopedMuteIoCapture::new();
+                log::error!(
+                    "[RuntimeTracer] trace filter resolution failed for code id {}: forced test error",
+                    code_id
+                );
+            });
+            record_dropped_event("filter_resolution_error");
+            return ScopeResolutionState::Error;
         }
 
         match engine.resolve(py, code) {
@@ -407,7 +438,7 @@ impl RuntimeTracer {
                 } else {
                     self.scope_cache.remove(&code_id);
                 }
-                Some(resolution)
+                ScopeResolutionState::Resolved(resolution)
             }
             Err(err) => {
                 let message = err.to_string();
@@ -421,7 +452,7 @@ impl RuntimeTracer {
                     );
                 });
                 record_dropped_event("filter_resolution_error");
-                None
+                ScopeResolutionState::Error
             }
         }
     }
@@ -650,8 +681,8 @@ impl RuntimeTracer {
             return ShouldTrace::SkipAndDisable;
         }
 
-        if let Some(resolution) = self.scope_resolution(py, code) {
-            match resolution.exec() {
+        match self.scope_resolution(py, code) {
+            ScopeResolutionState::Resolved(resolution) => match resolution.exec() {
                 ExecDecision::Skip => {
                     self.scope_cache.remove(&code_id);
                     self.filter_stats.record_skip();
@@ -662,6 +693,15 @@ impl RuntimeTracer {
                 ExecDecision::Trace => {
                     // already cached for future use
                 }
+            },
+            ScopeResolutionState::Error => {
+                self.scope_cache.remove(&code_id);
+                self.filter_stats.record_skip();
+                self.ignored_code_ids.insert(code_id);
+                return ShouldTrace::SkipAndDisable;
+            }
+            ScopeResolutionState::NotConfigured => {
+                // Continue without filtering when no engine is configured.
             }
         }
 
@@ -1699,6 +1739,67 @@ def emit_return(value):
         fs::write(path, contents.trim_start()).expect("write filter");
     }
 
+    #[test]
+    fn scope_resolution_error_disables_tracing() {
+        Python::with_gil(|py| {
+            ensure_test_module(py);
+
+            let project = tempfile::tempdir().expect("project dir");
+            let project_root = project.path();
+            let filters_dir = project_root.join(".codetracer");
+            fs::create_dir(&filters_dir).expect("create .codetracer");
+            let filter_path = filters_dir.join("filters.toml");
+            write_filter(
+                &filter_path,
+                r#"
+                [meta]
+                name = "test"
+                version = 1
+
+                [scope]
+                default_exec = "trace"
+                default_value_action = "allow"
+                "#,
+            );
+            let config = TraceFilterConfig::from_paths(&[filter_path]).expect("load filter");
+            let engine = Arc::new(TraceFilterEngine::new(config));
+
+            let mut tracer = RuntimeTracer::new(
+                "app/module.py",
+                &[],
+                TraceEventsFileFormat::Json,
+                None,
+                Some(engine),
+            );
+
+            let compile_fn = py
+                .import("builtins")
+                .expect("import builtins")
+                .getattr("compile")
+                .expect("fetch compile");
+            let compiled = compile_fn
+                .call1(("snapshot()", "app/module.py", "exec"))
+                .expect("compile code object");
+            let code_obj = compiled.downcast::<PyCode>().expect("downcast code object");
+            let wrapper = CodeObjectWrapper::new(py, &code_obj);
+
+            FORCE_SCOPE_RESOLUTION_ERROR.store(true, Ordering::SeqCst);
+            assert_eq!(
+                tracer.should_trace_code(py, &wrapper),
+                ShouldTrace::SkipAndDisable
+            );
+            assert!(tracer.ignored_code_ids.contains(&wrapper.id()));
+            assert!(tracer.scope_cache.get(&wrapper.id()).is_none());
+            assert_eq!(tracer.filter_stats.skipped_scopes, 1);
+
+            // Subsequent checks should keep the scope skipped without re-triggering the error.
+            assert_eq!(
+                tracer.should_trace_code(py, &wrapper),
+                ShouldTrace::SkipAndDisable
+            );
+        });
+    }
+
     #[test]
     fn trace_filter_redacts_values() {
         Python::with_gil(|py| {

codetracer-python-recorder/src/trace_filter/engine.rs

@@ -239,7 +239,7 @@ impl TraceFilterEngine {
 
         let mut exec = self.default_exec;
         let mut value_default = self.default_value_action;
-        let mut patterns: Arc<[CompiledValuePattern]> = Arc::from(Vec::new());
+        let mut patterns: Vec<CompiledValuePattern> = Vec::new();
         let mut matched_rule_index = None;
         let mut matched_rule_source = context.source_id;
         let mut matched_rule_reason = None;
@@ -253,15 +253,20 @@ impl TraceFilterEngine {
                     value_default = rule_value;
                 }
                 if !rule.value_patterns.is_empty() {
-                    patterns = rule.value_patterns.clone();
+                    let mut merged =
+                        Vec::with_capacity(rule.value_patterns.len() + patterns.len());
+                    merged.extend(rule.value_patterns.iter().cloned());
+                    merged.extend(patterns.into_iter());
+                    patterns = merged;
                 }
                 matched_rule_index = Some(rule.index);
                 matched_rule_source = Some(rule.source_id);
                 matched_rule_reason = rule.reason.clone();
             }
         }
 
-        let value_policy = Arc::new(ValuePolicy::new(value_default, patterns));
+        let compiled_patterns: Arc<[CompiledValuePattern]> = patterns.into();
+        let value_policy = Arc::new(ValuePolicy::new(value_default, compiled_patterns));
 
         Ok(ScopeResolution {
             exec,
@@ -568,6 +573,146 @@ mod tests {
         })
     }
 
+    #[test]
+    fn overlapping_scope_rules_should_merge_value_patterns() -> RecorderResult<()> {
+        let (config, file_path) = filter_with_pkg_rule(
+            r#"
+            [scope]
+            default_exec = "trace"
+            default_value_action = "redact"
+
+            [[scope.rules]]
+            selector = "pkg:app.foo"
+            value_default = "redact"
+
+            [[scope.rules.value_patterns]]
+            selector = "local:literal:secret_token"
+            action = "redact"
+
+            [[scope.rules]]
+            selector = "pkg:app.foo"
+            value_default = "allow"
+
+            [[scope.rules.value_patterns]]
+            selector = "local:literal:debug_switch"
+            action = "allow"
+            "#,
+        )?;
+
+        Python::with_gil(|py| -> RecorderResult<()> {
+            let module = load_module(
+                py,
+                "app.foo",
+                &file_path,
+                "def foo(secret_token, debug_switch):\n    return secret_token\n",
+            )?;
+            let code_obj = get_code(&module, "foo")?;
+            let wrapper = CodeObjectWrapper::new(py, &code_obj);
+
+            let engine = TraceFilterEngine::new(config);
+            let resolution = engine.resolve(py, &wrapper)?;
+            let policy = resolution.value_policy();
+
+            assert_eq!(resolution.matched_rule_index(), Some(1));
+            assert_eq!(policy.default_action(), ValueAction::Allow);
+            assert_eq!(
+                policy.decide(ValueKind::Local, "secret_token"),
+                ValueAction::Redact
+            );
+            assert_eq!(
+                policy.decide(ValueKind::Local, "debug_switch"),
+                ValueAction::Allow
+            );
+            Ok(())
+        })
+    }
+
+    #[test]
+    fn later_scope_rule_can_restore_trace_execution() -> RecorderResult<()> {
+        let (config, file_path) = filter_with_pkg_rule(
+            r#"
+            [scope]
+            default_exec = "trace"
+            default_value_action = "allow"
+
+            [[scope.rules]]
+            selector = "pkg:app.foo"
+            exec = "skip"
+
+            [[scope.rules]]
+            selector = "pkg:app.foo"
+            exec = "trace"
+            "#,
+        )?;
+
+        Python::with_gil(|py| -> RecorderResult<()> {
+            let module = load_module(py, "app.foo", &file_path, "def ping():\n    return 1\n")?;
+            let code_obj = get_code(&module, "ping")?;
+            let wrapper = CodeObjectWrapper::new(py, &code_obj);
+
+            let engine = TraceFilterEngine::new(config);
+            let resolution = engine.resolve(py, &wrapper)?;
+
+            assert_eq!(resolution.exec(), ExecDecision::Trace);
+            assert_eq!(resolution.matched_rule_index(), Some(1));
+            Ok(())
+        })
+    }
+
+    #[test]
+    fn catch_all_allow_patterns_can_clear_prior_redactions() -> RecorderResult<()> {
+        let (config, file_path) = filter_with_pkg_rule(
+            r#"
+            [scope]
+            default_exec = "trace"
+            default_value_action = "redact"
+
+            [[scope.rules]]
+            selector = "pkg:app.foo"
+            value_default = "redact"
+
+            [[scope.rules.value_patterns]]
+            selector = "local:literal:secret_token"
+            action = "redact"
+
+            [[scope.rules]]
+            selector = "pkg:app.foo"
+            value_default = "allow"
+
+            [[scope.rules.value_patterns]]
+            selector = "local:glob:*"
+            action = "allow"
+            "#,
+        )?;
+
+        Python::with_gil(|py| -> RecorderResult<()> {
+            let module = load_module(
+                py,
+                "app.foo",
+                &file_path,
+                "def foo(secret_token, other):\n    return secret_token\n",
+            )?;
+            let code_obj = get_code(&module, "foo")?;
+            let wrapper = CodeObjectWrapper::new(py, &code_obj);
+
+            let engine = TraceFilterEngine::new(config);
+            let resolution = engine.resolve(py, &wrapper)?;
+            let policy = resolution.value_policy();
+
+            assert_eq!(resolution.matched_rule_index(), Some(1));
+            assert_eq!(policy.default_action(), ValueAction::Allow);
+            assert_eq!(
+                policy.decide(ValueKind::Local, "secret_token"),
+                ValueAction::Allow
+            );
+            assert_eq!(
+                policy.decide(ValueKind::Local, "other"),
+                ValueAction::Allow
+            );
+            Ok(())
+        })
+    }
+
     #[test]
     fn object_rule_overrides_package_rule() -> RecorderResult<()> {
         let (config, file_path) = filter_with_pkg_rule(

codetracer-python-recorder/tests/python/perf/test_trace_filter_perf.py

@@ -199,7 +199,7 @@ def test_trace_filter_perf_smoke(tmp_path: Path) -> None:
         assert glob.duration_seconds > 0
         assert regex.duration_seconds > 0
 
-        assert baseline.filter_names == ["bench-baseline"]
+        assert baseline.filter_names == ["builtin-default", "bench-baseline"]
         assert "bench-glob" in glob.filter_names
         assert "bench-regex" in regex.filter_names