diff --git a/filters/src/guardrails/filter.rs b/filters/src/guardrails/filter.rs index 9807390d15..7e2bc2a89e 100644 --- a/filters/src/guardrails/filter.rs +++ b/filters/src/guardrails/filter.rs @@ -164,9 +164,10 @@ 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) + // 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 e454cfc756..83c46d942d 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_writes_filter_results() { +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,24 +350,53 @@ async fn on_request_body_modified_writes_filter_results() { }))) .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(filter.on_request_body(&mut ctx, &mut body, true).await.unwrap()); + assert_eq!( + rejection.status, 403, + "redact verdict must reject with HTTP 403" + ); + let body_text = String::from_utf8_lossy(rejection.body.as_deref().unwrap_or_default()); 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() { + 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 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"), + "the original secret must not appear in the rejection body" ); 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..f48ee4169b 100644 --- a/tests/integration/tests/suite/examples/guardrails.rs +++ b/tests/integration/tests/suite/examples/guardrails.rs @@ -73,10 +73,10 @@ 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. +/// 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 +96,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" + ); + 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