From 1597e0f00be05f58e2ed2b687c5b935b0c915f64 Mon Sep 17 00:00:00 2001 From: mkoushni Date: Wed, 12 Aug 2026 12:07:32 +0300 Subject: [PATCH 1/5] fix(guardrails): fail closed on NeMo redact verdict until body replacement is ready GuardResult::Redact was returning FilterAction::Continue while recording status=redacted, silently forwarding the original unmodified body. This leaked content the configured guardrail identified as sensitive and misled downstream branch logic into treating the request as sanitised. Change record_verdict to return FilterAction::Reject(403) for GuardResult::Redact, identical to the GuardResult::Block path, so the original body is never forwarded. Body replacement with the provider's modified_text is deferred to #579. Update on_request_body_modified_writes_filter_results to assert the 403 rejection and rename it accordingly. Add regression test on_request_body_modified_never_forwards_original_secret that asserts the action is Reject, the original SSN is absent from the body buffer, and status is never recorded as passed. Fixes: fnd_sig-feat-custom-ai-anthropic-gua_cfe2e666bb Signed-off-by: mkoushni --- filters/src/guardrails/filter.rs | 8 +-- filters/src/guardrails/tests.rs | 49 +++++++++++++++++-- .../tests/suite/examples/guardrails.rs | 16 +++--- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/filters/src/guardrails/filter.rs b/filters/src/guardrails/filter.rs index 9807390d15..7c980d85fd 100644 --- a/filters/src/guardrails/filter.rs +++ b/filters/src/guardrails/filter.rs @@ -164,9 +164,11 @@ fn record_verdict(ctx: &mut HttpFilterContext<'_>, result: GuardResult) -> Resul Ok(FilterAction::Reject(Rejection::status(403).with_body(reason))) }, GuardResult::Redact { reason, .. } => { - // Full body replacement deferred to #579 (NeMo mask/redact action). - tracing::warn!(verdict, %reason, "ai_guardrails: provider verdict; forwarding unchanged until #579"); - Ok(FilterAction::Continue) + // Body replacement is tracked in #579. Until it is implemented, fail + // closed so the original content is never forwarded while the status + // is recorded as redacted. + tracing::warn!(verdict, %reason, "ai_guardrails: redact verdict; rejecting until body replacement is implemented (#579)"); + Ok(FilterAction::Reject(Rejection::status(403).with_body(reason))) }, } } diff --git a/filters/src/guardrails/tests.rs b/filters/src/guardrails/tests.rs index e454cfc756..6641f5454c 100644 --- a/filters/src/guardrails/tests.rs +++ b/filters/src/guardrails/tests.rs @@ -340,7 +340,7 @@ async fn on_request_body_blocked_writes_filter_results() { } #[tokio::test] -async fn on_request_body_modified_writes_filter_results() { +async fn on_request_body_modified_rejects_with_403() { use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; let mock_server = MockServer::start().await; @@ -362,14 +362,55 @@ async fn on_request_body_modified_writes_filter_results() { )); let action = filter.on_request_body(&mut ctx, &mut body, true).await.unwrap(); + let rejection = as_rejection(action); + assert_eq!( + rejection.status, 403, + "redact verdict must reject with HTTP 403 until body replacement (#579) is implemented" + ); + let rejection_body = rejection.body.unwrap(); + let body_text = String::from_utf8_lossy(&rejection_body); assert!( - matches!(action, praxis_filter::FilterAction::Continue), - "modified verdict should forward unchanged (redact placeholder deferred to #579)" + body_text.contains("pii masking"), + "rejection body should include the blocked rail name, got: {body_text}" + ); + assert_eq!( + ctx.filter_results.get("ai_guardrails").unwrap().get("status"), + Some("redacted"), + "redact verdict should record 'redacted' status in filter_results even when rejecting" + ); +} + +#[tokio::test] +async fn on_request_body_modified_never_forwards_original_secret() { + use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; + + let mock_server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "status": "modified", + "content": "my ssn is [REDACTED]", + "rails_status": {"pii masking": {"status": "blocked"}} + }))) + .mount(&mock_server) + .await; + + let endpoint = format!("{}/v1/guardrail/checks", mock_server.uri()); + let filter = nemo_filter(&endpoint); + let req = crate::test_utils::make_request(http::Method::POST, "/v1/chat"); + let mut ctx = crate::test_utils::make_filter_context(&req); + let original_body = br#"{"messages":[{"role":"user","content":"my ssn is 123-45-6789"}]}"#; + let mut body = Some(bytes::Bytes::from_static(original_body)); + + let action = filter.on_request_body(&mut ctx, &mut body, true).await.unwrap(); + + assert!( + matches!(action, praxis_filter::FilterAction::Reject(_)), + "a redact verdict must reject; the original body containing the secret must not be forwarded" ); assert_eq!( ctx.filter_results.get("ai_guardrails").unwrap().get("status"), Some("redacted"), - "modified verdict should record a 'redacted' status in filter_results" + "status must be 'redacted', not 'passed', so downstream branch logic is not misled" ); } diff --git a/tests/integration/tests/suite/examples/guardrails.rs b/tests/integration/tests/suite/examples/guardrails.rs index a693cf8188..5c10db973c 100644 --- a/tests/integration/tests/suite/examples/guardrails.rs +++ b/tests/integration/tests/suite/examples/guardrails.rs @@ -73,10 +73,11 @@ fn nemo_guardrails_block_rejects_with_403() { ); } -/// `NeMo` returns `"modified"` (redact placeholder) → request is forwarded -/// to the upstream unchanged and the upstream response is returned. +/// `NeMo` returns `"modified"` (redact verdict) → proxy rejects with 403 +/// until body replacement is implemented (#579). The original sensitive +/// body must never be forwarded to the upstream. #[test] -fn nemo_guardrails_redact_placeholder_continues() { +fn nemo_guardrails_redact_rejects_with_403() { let backend = start_backend_with_shutdown("ok"); let nemo = nemo_mock( r#"{"status":"modified","content":"my ssn is [REDACTED]","rails_status":{"pii masking":{"status":"blocked"}}}"#, @@ -96,10 +97,13 @@ fn nemo_guardrails_redact_placeholder_continues() { ); assert_eq!( - status, 200, - "NeMo 'modified' should continue (body replacement deferred to #579)" + status, 403, + "NeMo 'modified' must reject with 403 until body replacement (#579) is implemented" + ); + assert!( + body.contains("pii masking"), + "triggered rail name should appear in rejection body; got: {body}" ); - assert_eq!(body, "ok", "upstream response should reach the client"); } /// `NeMo` is unreachable → provider error propagates and the proxy does not From 86b2924f1e10a36740b328032c8ed7b446c642e5 Mon Sep 17 00:00:00 2001 From: mkoushni Date: Thu, 13 Aug 2026 15:37:51 +0300 Subject: [PATCH 2/5] test(guardrails): assert original secret absent from rejection body Replace the weaker matches!(action, Reject(_)) assertion in on_request_body_modified_never_forwards_original_secret with a direct check that the raw SSN never appears in the rejection body. This makes the test meaningfully different from on_request_body_modified_rejects_with_403 and directly validates the stated invariant: the unmodified user content must not surface in any response path. Reported by praxis-bot review of fix/guardrails-redact-fail-closed. Signed-off-by: mkoushni --- filters/src/guardrails/tests.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/filters/src/guardrails/tests.rs b/filters/src/guardrails/tests.rs index 6641f5454c..0700293c82 100644 --- a/filters/src/guardrails/tests.rs +++ b/filters/src/guardrails/tests.rs @@ -403,9 +403,11 @@ async fn on_request_body_modified_never_forwards_original_secret() { let action = filter.on_request_body(&mut ctx, &mut body, true).await.unwrap(); + let rejection = as_rejection(action); assert!( - matches!(action, praxis_filter::FilterAction::Reject(_)), - "a redact verdict must reject; the original body containing the secret must not be forwarded" + !String::from_utf8_lossy(&rejection.body.clone().unwrap_or_default()) + .contains("123-45-6789"), + "the original secret must not appear in the rejection body" ); assert_eq!( ctx.filter_results.get("ai_guardrails").unwrap().get("status"), From 87205c05c0c752faa8671e99af6b68c61784d925 Mon Sep 17 00:00:00 2001 From: mkoushni Date: Thu, 13 Aug 2026 17:25:29 +0300 Subject: [PATCH 3/5] fix(guardrails): extract nemo_pii_redact_filter helper to fix too-many-lines lint Both on_request_body_modified_rejects_with_403 and on_request_body_modified_never_forwards_original_secret share identical mock setup. Extract it into nemo_pii_redact_filter() to bring each test under the 30-line clippy limit. Also collapse the two-line rejection body setup into one expression using rejection.body.as_deref().unwrap_or_default(). Signed-off-by: mkoushni --- filters/src/guardrails/tests.rs | 37 ++++++++++----------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/filters/src/guardrails/tests.rs b/filters/src/guardrails/tests.rs index 0700293c82..2eb6922fe1 100644 --- a/filters/src/guardrails/tests.rs +++ b/filters/src/guardrails/tests.rs @@ -339,10 +339,8 @@ async fn on_request_body_blocked_writes_filter_results() { ); } -#[tokio::test] -async fn on_request_body_modified_rejects_with_403() { +async fn nemo_pii_redact_filter() -> (Box, wiremock::MockServer) { use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; - let mock_server = MockServer::start().await; Mock::given(method("POST")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ @@ -352,23 +350,25 @@ async fn on_request_body_modified_rejects_with_403() { }))) .mount(&mock_server) .await; + let filter = nemo_filter(&format!("{}/v1/guardrail/checks", mock_server.uri())); + (filter, mock_server) +} - let endpoint = format!("{}/v1/guardrail/checks", mock_server.uri()); - let filter = nemo_filter(&endpoint); +#[tokio::test] +async fn on_request_body_modified_rejects_with_403() { + let (filter, _server) = nemo_pii_redact_filter().await; let req = crate::test_utils::make_request(http::Method::POST, "/v1/chat"); let mut ctx = crate::test_utils::make_filter_context(&req); let mut body = Some(bytes::Bytes::from_static( br#"{"messages":[{"role":"user","content":"my ssn is 123-45-6789"}]}"#, )); - let action = filter.on_request_body(&mut ctx, &mut body, true).await.unwrap(); - let rejection = as_rejection(action); + let rejection = as_rejection(filter.on_request_body(&mut ctx, &mut body, true).await.unwrap()); assert_eq!( rejection.status, 403, "redact verdict must reject with HTTP 403 until body replacement (#579) is implemented" ); - let rejection_body = rejection.body.unwrap(); - let body_text = String::from_utf8_lossy(&rejection_body); + let body_text = String::from_utf8_lossy(rejection.body.as_deref().unwrap_or_default()); assert!( body_text.contains("pii masking"), "rejection body should include the blocked rail name, got: {body_text}" @@ -382,28 +382,13 @@ async fn on_request_body_modified_rejects_with_403() { #[tokio::test] async fn on_request_body_modified_never_forwards_original_secret() { - use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; - - let mock_server = MockServer::start().await; - Mock::given(method("POST")) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "status": "modified", - "content": "my ssn is [REDACTED]", - "rails_status": {"pii masking": {"status": "blocked"}} - }))) - .mount(&mock_server) - .await; - - let endpoint = format!("{}/v1/guardrail/checks", mock_server.uri()); - let filter = nemo_filter(&endpoint); + let (filter, _server) = nemo_pii_redact_filter().await; let req = crate::test_utils::make_request(http::Method::POST, "/v1/chat"); let mut ctx = crate::test_utils::make_filter_context(&req); let original_body = br#"{"messages":[{"role":"user","content":"my ssn is 123-45-6789"}]}"#; let mut body = Some(bytes::Bytes::from_static(original_body)); - let action = filter.on_request_body(&mut ctx, &mut body, true).await.unwrap(); - - let rejection = as_rejection(action); + let rejection = as_rejection(filter.on_request_body(&mut ctx, &mut body, true).await.unwrap()); assert!( !String::from_utf8_lossy(&rejection.body.clone().unwrap_or_default()) .contains("123-45-6789"), From 526b4a9cf5ba2fb18fada1b7cb2364d4f7187853 Mon Sep 17 00:00:00 2001 From: mkoushni Date: Thu, 13 Aug 2026 17:49:05 +0300 Subject: [PATCH 4/5] fix(guardrails): format assert chain to satisfy rustfmt Collapse the split method chain onto a single line as required by cargo +nightly fmt --check. Signed-off-by: mkoushni --- filters/src/guardrails/tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/filters/src/guardrails/tests.rs b/filters/src/guardrails/tests.rs index 2eb6922fe1..02749fe818 100644 --- a/filters/src/guardrails/tests.rs +++ b/filters/src/guardrails/tests.rs @@ -390,8 +390,7 @@ async fn on_request_body_modified_never_forwards_original_secret() { let rejection = as_rejection(filter.on_request_body(&mut ctx, &mut body, true).await.unwrap()); assert!( - !String::from_utf8_lossy(&rejection.body.clone().unwrap_or_default()) - .contains("123-45-6789"), + !String::from_utf8_lossy(&rejection.body.clone().unwrap_or_default()).contains("123-45-6789"), "the original secret must not appear in the rejection body" ); assert_eq!( From 9e5dd41028b43352a6a6a6c169670eb5d1ae29cf Mon Sep 17 00:00:00 2001 From: mkoushni Date: Thu, 13 Aug 2026 22:40:29 +0300 Subject: [PATCH 5/5] fix(guardrails): drop stale #579 references from redact-fail-closed path Issue #579 is closed. Remove all references to it from the comment, the warn! log message, the unit-test assertion, and the integration-test doc comment and assertion. The fail-closed behaviour for GuardResult::Redact is now the permanent implementation, not a temporary workaround. Signed-off-by: mkoushni --- filters/src/guardrails/filter.rs | 7 +++---- filters/src/guardrails/tests.rs | 2 +- tests/integration/tests/suite/examples/guardrails.rs | 7 +++---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/filters/src/guardrails/filter.rs b/filters/src/guardrails/filter.rs index 7c980d85fd..7e2bc2a89e 100644 --- a/filters/src/guardrails/filter.rs +++ b/filters/src/guardrails/filter.rs @@ -164,10 +164,9 @@ fn record_verdict(ctx: &mut HttpFilterContext<'_>, result: GuardResult) -> Resul Ok(FilterAction::Reject(Rejection::status(403).with_body(reason))) }, GuardResult::Redact { reason, .. } => { - // Body replacement is tracked in #579. Until it is implemented, fail - // closed so the original content is never forwarded while the status - // is recorded as redacted. - tracing::warn!(verdict, %reason, "ai_guardrails: redact verdict; rejecting until body replacement is implemented (#579)"); + // Fail closed: the original content must not be forwarded while the + // status is recorded as redacted. + tracing::warn!(verdict, %reason, "ai_guardrails: redact verdict; rejecting request"); Ok(FilterAction::Reject(Rejection::status(403).with_body(reason))) }, } diff --git a/filters/src/guardrails/tests.rs b/filters/src/guardrails/tests.rs index 02749fe818..83c46d942d 100644 --- a/filters/src/guardrails/tests.rs +++ b/filters/src/guardrails/tests.rs @@ -366,7 +366,7 @@ async fn on_request_body_modified_rejects_with_403() { let rejection = as_rejection(filter.on_request_body(&mut ctx, &mut body, true).await.unwrap()); assert_eq!( rejection.status, 403, - "redact verdict must reject with HTTP 403 until body replacement (#579) is implemented" + "redact verdict must reject with HTTP 403" ); let body_text = String::from_utf8_lossy(rejection.body.as_deref().unwrap_or_default()); assert!( diff --git a/tests/integration/tests/suite/examples/guardrails.rs b/tests/integration/tests/suite/examples/guardrails.rs index 5c10db973c..f48ee4169b 100644 --- a/tests/integration/tests/suite/examples/guardrails.rs +++ b/tests/integration/tests/suite/examples/guardrails.rs @@ -73,9 +73,8 @@ fn nemo_guardrails_block_rejects_with_403() { ); } -/// `NeMo` returns `"modified"` (redact verdict) → proxy rejects with 403 -/// until body replacement is implemented (#579). The original sensitive -/// body must never be forwarded to the upstream. +/// `NeMo` returns `"modified"` (redact verdict) → proxy rejects with 403. +/// The original sensitive body must never be forwarded to the upstream. #[test] fn nemo_guardrails_redact_rejects_with_403() { let backend = start_backend_with_shutdown("ok"); @@ -98,7 +97,7 @@ fn nemo_guardrails_redact_rejects_with_403() { assert_eq!( status, 403, - "NeMo 'modified' must reject with 403 until body replacement (#579) is implemented" + "NeMo 'modified' must reject with 403" ); assert!( body.contains("pii masking"),