From 69bef14c1997bdef0b043febc3dfbb1ef9f36729 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Sat, 25 Jul 2026 20:53:39 -0700 Subject: [PATCH] fix(pii): reject malformed target paths Signed-off-by: Alex Fournier --- crates/pii-redaction/src/builtin.rs | 24 +++++ crates/pii-redaction/src/component.rs | 19 +++- .../tests/unit/component_tests.rs | 95 +++++++++++++++++++ .../pii-redaction/configuration.mdx | 6 ++ 4 files changed, 141 insertions(+), 3 deletions(-) diff --git a/crates/pii-redaction/src/builtin.rs b/crates/pii-redaction/src/builtin.rs index 43cc9cdb8..2a7da0687 100644 --- a/crates/pii-redaction/src/builtin.rs +++ b/crates/pii-redaction/src/builtin.rs @@ -115,6 +115,13 @@ impl CompiledBuiltinBackend { .to_string(), )); } + for (index, target_path) in config.target_paths.iter().enumerate() { + if !is_valid_json_pointer(target_path) { + return Err(PluginError::InvalidConfig(format!( + "builtin.target_paths[{index}] must be a valid RFC 6901 JSON pointer" + ))); + } + } let detector = config .detector .as_deref() @@ -399,6 +406,23 @@ pub(super) fn llm_sanitize_response_callback( }) } +pub(super) fn is_valid_json_pointer(pointer: &str) -> bool { + if pointer.is_empty() { + return true; + } + pointer.strip_prefix('/').is_some_and(|path| { + path.split('/').all(|segment| { + let mut characters = segment.chars(); + while let Some(character) = characters.next() { + if character == '~' && !matches!(characters.next(), Some('0' | '1')) { + return false; + } + } + true + }) + }) +} + fn render_json_pointer_path(path_segments: &[String]) -> String { if path_segments.is_empty() { return String::new(); diff --git a/crates/pii-redaction/src/component.rs b/crates/pii-redaction/src/component.rs index 0c4c3b415..567f673cb 100644 --- a/crates/pii-redaction/src/component.rs +++ b/crates/pii-redaction/src/component.rs @@ -18,8 +18,8 @@ use serde::{Deserialize, Serialize}; use serde_json::{Map, Value as Json}; use super::builtin::{ - CompiledBuiltinBackend, llm_sanitize_request_callback, llm_sanitize_response_callback, - tool_sanitize_callback, + CompiledBuiltinBackend, is_valid_json_pointer, llm_sanitize_request_callback, + llm_sanitize_response_callback, tool_sanitize_callback, }; #[cfg(test)] pub(crate) use super::builtin::{hex_sha256, mask_text}; @@ -186,7 +186,7 @@ pub struct BuiltinBackendConfig { )] #[cfg_attr(feature = "schema", schemars(schema_with = "builtin_action_schema"))] pub action: String, - /// Exact JSON-pointer paths to sanitize. Empty means every string leaf. + /// Exact RFC 6901 JSON-pointer paths to sanitize. Empty means every string leaf. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub target_paths: Vec, /// Regex pattern used when `action = "regex_replace"` or `action = "redact"`. @@ -1034,6 +1034,19 @@ fn validate_builtin_action_requirements( "builtin.mask_char must not be empty when builtin.action = 'mask'".to_string(), ); } + + for (index, target_path) in builtin.target_paths.iter().enumerate() { + if !is_valid_json_pointer(target_path) { + push_policy_diag( + diagnostics, + policy.unsupported_value, + "pii_redaction.unsupported_value", + Some(PII_REDACTION_PLUGIN_KIND.to_string()), + Some(format!("builtin.target_paths[{index}]")), + "builtin.target_paths entries must be valid RFC 6901 JSON pointers".to_string(), + ); + } + } } fn validate_builtin_preset_requirements( diff --git a/crates/pii-redaction/tests/unit/component_tests.rs b/crates/pii-redaction/tests/unit/component_tests.rs index 3ac54253e..6e60b4060 100644 --- a/crates/pii-redaction/tests/unit/component_tests.rs +++ b/crates/pii-redaction/tests/unit/component_tests.rs @@ -1511,6 +1511,101 @@ fn validate_rejects_invalid_builtin_pattern_regex() { })); } +#[test] +fn validate_rejects_malformed_builtin_target_paths() { + let _guard = crate::plugins::pii_redaction::test_mutex().lock().unwrap(); + reset_runtime(); + + let report = validate_plugin_config(&plugin_config(json!({ + "mode": "builtin", + "codec": "openai_chat", + "builtin": { + "action": "remove", + "target_paths": [ + "messages/0/content", + "/messages/~2content", + "/messages/trailing~" + ] + } + }))); + + for index in 0..3 { + let expected_field = format!("builtin.target_paths[{index}]"); + assert!(report.diagnostics.iter().any(|diagnostic| { + diagnostic.field.as_deref() == Some(expected_field.as_str()) + && diagnostic.message.contains("RFC 6901") + })); + } +} + +#[test] +fn validate_accepts_valid_builtin_target_paths() { + let _guard = crate::plugins::pii_redaction::test_mutex().lock().unwrap(); + reset_runtime(); + + let report = validate_plugin_config(&plugin_config(json!({ + "mode": "builtin", + "codec": "openai_chat", + "builtin": { + "action": "remove", + "target_paths": [ + "", + "/", + "//empty-segment", + "/messages/0/content", + "/metadata/~0owner", + "/metadata/a~1b" + ] + } + }))); + + assert!(!report.has_errors(), "{:#?}", report.diagnostics); +} + +#[test] +fn profile_validation_reports_malformed_target_path_index() { + let _guard = crate::plugins::pii_redaction::test_mutex().lock().unwrap(); + reset_runtime(); + + let report = validate_plugin_config(&plugin_config(json!({ + "codec": "openai_chat", + "profiles": [{ + "mode": "builtin", + "builtin": { + "action": "remove", + "target_paths": ["/messages/0/content", "message"] + } + }] + }))); + + assert!(report.diagnostics.iter().any(|diagnostic| { + diagnostic.field.as_deref() == Some("profiles[0].builtin.target_paths[1]") + && diagnostic.message.contains("RFC 6901") + })); +} + +#[test] +fn activation_rejects_malformed_target_path_when_diagnostic_is_downgraded() { + let _guard = crate::plugins::pii_redaction::test_mutex().lock().unwrap(); + reset_runtime(); + setup_isolated_thread(); + + let activation = futures::executor::block_on(initialize_plugins(plugin_config(json!({ + "mode": "builtin", + "codec": "openai_chat", + "builtin": { + "action": "remove", + "target_paths": ["messages/0/content"] + }, + "policy": { + "unsupported_value": "warn" + } + })))); + + let error = activation.expect_err("malformed target path must fail plugin activation"); + assert!(error.to_string().contains("builtin.target_paths[0]")); +} + #[test] fn validate_rejects_mask_with_empty_mask_char() { let _guard = crate::plugins::pii_redaction::test_mutex().lock().unwrap(); diff --git a/docs/configure-plugins/pii-redaction/configuration.mdx b/docs/configure-plugins/pii-redaction/configuration.mdx index 329f78892..8933d1332 100644 --- a/docs/configure-plugins/pii-redaction/configuration.mdx +++ b/docs/configure-plugins/pii-redaction/configuration.mdx @@ -286,6 +286,12 @@ When `detector` is set and you do not specify `unmasked_prefix` or `target_paths` are exact JSON-pointer matches. +Relay validates every configured path as RFC 6901 syntax before activating the +component. The empty string selects the document root. Every other pointer must +start with `/`, and `~` escapes must use `~0` for `~` or `~1` for `/`. Invalid +pointers prevent activation instead of silently leaving the intended values +unsanitized. + The plugin uses different payload boundaries for tools and LLMs: - Tools use JSON-native payloads. Paths point into the emitted tool args or