fix(guardrails): fail closed on NeMo redact verdict until body replac… by mkoushni · Pull Request #721 · praxis-proxy/ai

mkoushni · 2026-08-12T09:55:07Z

fix(guardrails): fail closed on NeMo redact verdict until body replacement is ready

Summary

Addresses Clawpatch finding fnd_sig-feat-custom-ai-anthropic-gua_cfe2e666bb (severity: high / security / confidence: high).

GuardResult::Redact was returning FilterAction::Continue while recording
status=redacted, silently forwarding the original unmodified body containing
sensitive content. A request with my ssn is 123-45-6789 would be passed
upstream unchanged while the filter claimed it had been redacted.

Root Cause

// Before — VULNERABLE
GuardResult::Redact { reason, .. } => {
    tracing::warn!(..., "forwarding unchanged until #579");
    Ok(FilterAction::Continue)   // ← original body forwarded unmodified
},

The modified_text from the provider was discarded, the original body was
forwarded, and status=redacted was recorded — misleading downstream branch
logic and auditing into treating the request as sanitised.

Fix

// After — fail closed
GuardResult::Redact { reason, .. } => {
    tracing::warn!(..., "rejecting until body replacement is implemented (#579)");
    Ok(FilterAction::Reject(Rejection::status(403).with_body(reason)))
},

GuardResult::Redact now follows the same code path as GuardResult::Block:
reject with HTTP 403, record status=redacted, never forward the original body.
Body replacement with the provider's modified_text remains deferred to #579.

Changes

File	Change
`filters/src/guardrails/filter.rs:166-171`	`GuardResult::Redact` → `FilterAction::Reject(403)`
`filters/src/guardrails/tests.rs`	Updated existing test + added regression test

Tests:

on_request_body_modified_rejects_with_403 — replaces the stale
on_request_body_modified_writes_filter_results; asserts HTTP 403 and
status=redacted is still recorded even when rejecting.
on_request_body_modified_never_forwards_original_secret — regression test
per Clawpatch recommendation; asserts the action is Reject (runtime
discards the body) and status is never recorded as passed.

Test Results

test result: ok. 44 passed; 0 failed; 0 ignored (guardrails suite)
test result: ok. 1009 passed; 0 failed; 0 ignored (full filters crate)

Resolve

#673

leseb · 2026-08-12T09:59:47Z

Body replacement (forwarding the provider's modified_text instead of
rejecting) is tracked in #579. This PR is intentionally fail-closed until
that is implemented.

#579 has merged? so this is outdated.

mkoushni · 2026-08-12T13:57:56Z

Body replacement (forwarding the provider's modified_text instead of
rejecting) is tracked in #579. This PR is intentionally fail-closed until
that is implemented.

#579 has merged? so this is outdated.

No — the fix is still needed. main still has the original vulnerable code:

GuardResult::Redact { reason, .. } => {
// Full body replacement deferred to #579 (NeMo mask/redact action).
tracing::warn!(..., "forwarding unchanged until #579");
Ok(FilterAction::Continue) // ← still forwarding the original body
},
#579 merged something else — it did not implement body replacement inside record_verdict. The guardrails filter on main is still forwarding the unmodified request and claiming status=redacted.

My fix (FilterAction::Reject(403)) is still the correct interim solution. The PR should go in as-is.

I will fix the errors in the tests

praxis-bot

Security fix is correct: GuardResult::Redact now fails closed with 403 instead of silently forwarding the original body containing sensitive data. The status recording order in record_verdict (write "redacted" before the match) ensures filter_results remains accurate even on rejection. Unit tests, regression test, and integration test all updated consistently. Two medium items below.

praxis-bot

Security fix is correct and well-scoped. GuardResult::Redact now fails closed with HTTP 403, matching the Block path, so the original unmasked body never reaches the upstream. The reason field passed to Rejection::with_body comes from blocked_rail_names() (rail names only, not sensitive content), so no information leak in the 403 body. Status recording ("redacted") happens before the match arms, keeping filter_results accurate for downstream branch logic even on rejection. Regression test and integration test cover the critical invariant. One medium item below (not a duplicate of the prior review, which flagged the same class of issue in the integration test).

praxis-bot

The core security fix in record_verdict is correct and well-scoped: GuardResult::Redact now rejects with HTTP 403, matching the Block path, so the unmodified body containing sensitive content is never forwarded upstream. The reason field passed to Rejection::with_body originates from blocked_rail_names() (rail names only, not user content), avoiding information leakage in the 403 response. Status recording ("redacted") remains accurate because it is written before the match arms. Unit tests, regression test, and integration test updated consistently. One medium item below.

…ement 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 praxis-proxy#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 <mkoushni@redhat.com>

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 <mkoushni@redhat.com>

leseb · 2026-08-13T14:16:56Z

-            // 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


579 is closed so why still have a comment for it? the comment is wrong here

…y-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 <mkoushni@redhat.com>

Collapse the split method chain onto a single line as required by cargo +nightly fmt --check. Signed-off-by: mkoushni <mkoushni@redhat.com>

leseb

let's resolve this https://github.com/praxis-proxy/ai/pull/721/changes#r3776143828

mkoushni requested review from a team and alexsnaps August 12, 2026 09:55

mkoushni force-pushed the fix/guardrails-redact-fail-closed branch 2 times, most recently from 8fd8f4c to 946e2be Compare August 12, 2026 14:05

praxis-bot reviewed Aug 12, 2026

View reviewed changes

Comment thread tests/integration/tests/suite/examples/guardrails.rs

Comment thread filters/src/guardrails/tests.rs Outdated

praxis-bot reviewed Aug 12, 2026

View reviewed changes

Comment thread filters/src/guardrails/tests.rs

mkoushni force-pushed the fix/guardrails-redact-fail-closed branch 4 times, most recently from 0d529ea to e6f41c0 Compare August 13, 2026 10:02

praxis-bot reviewed Aug 13, 2026

View reviewed changes

Comment thread filters/src/guardrails/tests.rs Outdated

mkoushni added 2 commits August 13, 2026 15:38

mkoushni force-pushed the fix/guardrails-redact-fail-closed branch from e6f41c0 to b63d55b Compare August 13, 2026 12:39

Merge branch 'main' into fix/guardrails-redact-fail-closed

360b509

leseb reviewed Aug 13, 2026

View reviewed changes

mkoushni added 3 commits August 13, 2026 17:26

fix(guardrails): format assert chain to satisfy rustfmt

167d987

Collapse the split method chain onto a single line as required by cargo +nightly fmt --check. Signed-off-by: mkoushni <mkoushni@redhat.com>

Merge branch 'main' into fix/guardrails-redact-fail-closed

c4c31a9

leseb requested changes Aug 13, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(guardrails): fail closed on NeMo redact verdict until body replac… - #721

fix(guardrails): fail closed on NeMo redact verdict until body replac…#721
mkoushni wants to merge 6 commits into
praxis-proxy:mainfrom
mkoushni:fix/guardrails-redact-fail-closed

mkoushni commented Aug 12, 2026 •

edited

Loading

Uh oh!

leseb commented Aug 12, 2026

Uh oh!

mkoushni commented Aug 12, 2026

Uh oh!

praxis-bot left a comment

Uh oh!

Uh oh!

Uh oh!

praxis-bot left a comment

Uh oh!

Uh oh!

praxis-bot left a comment

Uh oh!

Uh oh!

leseb Aug 13, 2026

Uh oh!

leseb left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mkoushni commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

fix(guardrails): fail closed on NeMo redact verdict until body replacement is ready

Summary

Root Cause

Fix

Changes

Test Results

Resolve

Uh oh!

leseb commented Aug 12, 2026

Uh oh!

mkoushni commented Aug 12, 2026

Uh oh!

praxis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

praxis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

praxis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leseb Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

leseb left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mkoushni commented Aug 12, 2026 •

edited

Loading