diff --git a/crates/pii-redaction/src/builtin.rs b/crates/pii-redaction/src/builtin.rs index 6e6d1dc5c..414fb13f6 100644 --- a/crates/pii-redaction/src/builtin.rs +++ b/crates/pii-redaction/src/builtin.rs @@ -74,6 +74,13 @@ impl CompiledBuiltinBackend { config: BuiltinBackendConfig, codec_name: Option, ) -> PluginResult { + 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 trajectory = match config.preset.as_deref() { Some("trajectory_context") => { if config.detector.is_some() @@ -608,6 +615,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 log_llm_payload_omitted(direction: &str, codec: &LlmCodecIdentity, reason: &str) { let codec_kind = match codec { LlmCodecIdentity::None => "none", diff --git a/crates/pii-redaction/src/component.rs b/crates/pii-redaction/src/component.rs index 2f6b9bfd3..1e21dfe43 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}; @@ -187,7 +187,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"`. @@ -927,6 +927,19 @@ fn validate_builtin_action_requirements( return; }; + 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(), + ); + } + } + if builtin.preset.is_some() { validate_builtin_preset_requirements(diagnostics, policy, plugin_config, builtin); return; diff --git a/crates/pii-redaction/tests/unit/component_tests.rs b/crates/pii-redaction/tests/unit/component_tests.rs index cd7ad4026..30aeb690d 100644 --- a/crates/pii-redaction/tests/unit/component_tests.rs +++ b/crates/pii-redaction/tests/unit/component_tests.rs @@ -1767,6 +1767,129 @@ 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 preset_does_not_bypass_malformed_target_path_validation() { + let _guard = crate::plugins::pii_redaction::test_mutex().lock().unwrap(); + reset_runtime(); + + let config = json!({ + "mode": "builtin", + "codec": "openai_chat", + "builtin": { + "preset": "trajectory_context", + "target_paths": ["messages/0/content"] + }, + "policy": { + "unsupported_value": "warn" + } + }); + let report = validate_plugin_config(&plugin_config(config.clone())); + assert!(report.diagnostics.iter().any(|diagnostic| { + diagnostic.field.as_deref() == Some("builtin.target_paths[0]") + && diagnostic.message.contains("RFC 6901") + })); + + setup_isolated_thread(); + let activation = futures::executor::block_on(initialize_plugins(plugin_config(config))); + let error = activation.expect_err("preset must not bypass malformed target path validation"); + 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 d10f84813..270375180 100644 --- a/docs/configure-plugins/pii-redaction/configuration.mdx +++ b/docs/configure-plugins/pii-redaction/configuration.mdx @@ -318,6 +318,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