Repository navigation
fix(guardrails): restore PII masking and contain output failures - #358
rapids-bot[bot] merged 7 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
🧰 Additional context used📓 Path-based instructions (3)**/*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*test*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*config*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (1)
WalkthroughGuardrails middleware now converts execution, traversal, adaptation, and streaming failures into safe refusals while synchronizing guarded workflow outputs. The project also adds PII dependencies, Docker validation, packaging checks, documentation, and regression tests. ChangesGuardrails failure safety
PII runtime dependency validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant GuardrailsMixin
participant OutputRails
participant WorkflowOutput
Agent->>GuardrailsMixin: invoke or stream output
GuardrailsMixin->>OutputRails: evaluate selected content
OutputRails-->>GuardrailsMixin: masked, blocked, or failed result
GuardrailsMixin->>WorkflowOutput: synchronize buffered and terminal text
GuardrailsMixin-->>Agent: sanitized output or refusal-only response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Tanner Leach <tleach@nvidia.com>
Signed-off-by: Tanner Leach <tleach@nvidia.com>
Signed-off-by: Tanner Leach <tleach@nvidia.com>
23076e8 to
6f22960
Compare
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Signed-off-by: Tanner Leach <tleach@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/aiq_agent/guardrails/interface/middleware.py`:
- Around line 126-137: Move consumption of call_next into the same try block as
_apply_output_rails_to_structured_stream by buffering it with an explicit async
for, so producer exceptions—including failures after protected chunks—enter the
refusal path. When refusing, use the first buffered chunk when available and
handle an empty buffer safely while preserving the existing logging and
response-yield behavior.
In `@src/aiq_agent/guardrails/workflow/middleware.py`:
- Around line 168-175: Change the input-target extraction flow used by
pre_invoke so exceptions from traversing or reading the input are handled by its
existing failure boundary and propagate as _GUARDRAILS_FAILURE_REFUSAL, rather
than being caught and converted to an empty target list. Remove the local
swallowing behavior around the extraction logic, and add a regression test
covering an input property that raises.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 892e5e29-4c40-447e-baa5-4a3b716832ca
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
deploy/Dockerfiledocs/source/customization/guardrails.mdpyproject.tomlsrc/aiq_agent/guardrails/deep_agent/middleware.pysrc/aiq_agent/guardrails/interface/middleware.pysrc/aiq_agent/guardrails/shallow_agent/middleware.pysrc/aiq_agent/guardrails/workflow/middleware.pytests/aiq_agent/guardrails/test_deep_agent_guardrails.pytests/aiq_agent/guardrails/test_shallow_agent_guardrails.pytests/aiq_agent/guardrails/test_workflow_guardrails.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Run Harbor skill eval
- GitHub Check: Lint and Hooks
- GitHub Check: Script Validation
- GitHub Check: Pytest and Coverage
🧰 Additional context used
📓 Path-based instructions (7)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
src/aiq_agent/guardrails/shallow_agent/middleware.pytests/aiq_agent/guardrails/test_shallow_agent_guardrails.pysrc/aiq_agent/guardrails/deep_agent/middleware.pysrc/aiq_agent/guardrails/workflow/middleware.pytests/aiq_agent/guardrails/test_deep_agent_guardrails.pytests/aiq_agent/guardrails/test_workflow_guardrails.pysrc/aiq_agent/guardrails/interface/middleware.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/guardrails/shallow_agent/middleware.pysrc/aiq_agent/guardrails/deep_agent/middleware.pysrc/aiq_agent/guardrails/workflow/middleware.pysrc/aiq_agent/guardrails/interface/middleware.py
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}: Review automation and packaging changes for least-privilege permissions, pinned versions where appropriate,
copy-pr-bot pull-request/ branch behavior, reproducible uv/npm setup, secret handling, and consistency with
the documented validation matrix.
Files:
pyproject.toml
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/customization/guardrails.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/customization/guardrails.md
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/guardrails/test_shallow_agent_guardrails.pytests/aiq_agent/guardrails/test_deep_agent_guardrails.pytests/aiq_agent/guardrails/test_workflow_guardrails.py
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
deploy/Dockerfile
🪛 ast-grep (0.44.1)
tests/aiq_agent/guardrails/test_workflow_guardrails.py
[info] 941-941: use jsonify instead of json.dumps for JSON output
Context: json.dumps([item.model_dump(mode="json") for item in results])
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1077-1077: use jsonify instead of json.dumps for JSON output
Context: json.dumps([item if isinstance(item, str) else item.model_dump(mode="json") for item in results])
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Trivy (0.72.0)
deploy/Dockerfile
[error] 79-82: 'apt-get' missing '--no-install-recommends'
'--no-install-recommends' flag is missed: 'apt-get update && apt-get install -y curl ca-certificates && rm -rf /var/lib/apt/lists/*'
Rule: DS-0029
(IaC/Dockerfile)
🔇 Additional comments (11)
pyproject.toml (1)
56-59: LGTM!deploy/Dockerfile (2)
79-79: LGTM!
93-99: LGTM!src/aiq_agent/guardrails/interface/middleware.py (1)
52-56: LGTM!Also applies to: 73-86, 164-214, 228-256
src/aiq_agent/guardrails/deep_agent/middleware.py (1)
28-28: LGTM!Also applies to: 138-145
src/aiq_agent/guardrails/shallow_agent/middleware.py (1)
27-27: LGTM!Also applies to: 138-145
src/aiq_agent/guardrails/workflow/middleware.py (1)
29-33: LGTM!Also applies to: 103-152
tests/aiq_agent/guardrails/test_deep_agent_guardrails.py (1)
22-26: LGTM!Also applies to: 385-442
tests/aiq_agent/guardrails/test_shallow_agent_guardrails.py (1)
22-26: LGTM!Also applies to: 397-454
tests/aiq_agent/guardrails/test_workflow_guardrails.py (1)
29-34: LGTM!Also applies to: 76-80, 643-853, 900-1090, 1138-1232
docs/source/customization/guardrails.md (1)
37-38: LGTM!Also applies to: 45-50
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mcp/tests/test_config_and_packaging.py`:
- Around line 153-161: The URL-source branch in the lockfile test should also
validate the package version. Within the existing assertions in
test_config_and_packaging, add an assertion that package["version"] equals
"3.8.0", while preserving the current package name and URL checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 067e6328-1ecb-4ef9-856d-ca6f4ffb1ac7
⛔ Files ignored due to path filters (1)
mcp/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
mcp/tests/test_config_and_packaging.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Validate skill-eval specs
- GitHub Check: Script Validation
- GitHub Check: Pytest and Coverage
- GitHub Check: Lint and Hooks
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
mcp/tests/test_config_and_packaging.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
mcp/tests/test_config_and_packaging.py
**/*config*.py
📄 CodeRabbit inference engine (AGENTS.md)
Config schemas must inherit from FunctionBaseConfig and YAML _type names must come from the registered config class
Files:
mcp/tests/test_config_and_packaging.py
KyleZheng1284
left a comment
KyleZheng1284
left a comment
There was a problem hiding this comment.
left a few comments otherwise LGTM
Move workflow input-target extraction into the existing fail-closed boundary so unexpected traversal errors cannot bypass input rails. Add regression coverage proving the workflow and rail backend remain uninvoked and sensitive exception details stay out of logs. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Verify the root lock records en-core-web-lg at version 3.8.0 in addition to checking its package name and exact wheel URL. Signed-off-by: Tanner Leach <tleach@nvidia.com>
|
/merge |
a4223b2
into
NVIDIA-AI-Blueprints:release/2.2
Overview
Restores supported PII masking in the AI-Q 2.2 release image and makes output guardrail failures fail closed instead of escaping as generic internal errors.
The release runtime previously included NeMo Guardrails without the Presidio, spaCy, and language-model dependencies required by
sensitive_data_detection. Because those imports are lazy, the service could start healthy but fail every response once output PII masking ran. Output-rail exceptions could also escape the middleware boundary.What changed
piiruntime extra containing the NAT Security defense dependencies and pinneden-core-web-lg==3.8.0model.piiextra in the release image and execute the public NeMomask_sensitive_dataaction during the Docker build.workflow_outcome.resultso later chunks cannot retain unfiltered content./app/datawritable by runtime UID 1000.DCO sign-off for the squash commit
Signed-off-by: Tanner Leach tleach@nvidia.com
Validation
uv run ruff check .uv run ruff format --check .— 370 filesuv run pytest -q tests/aiq_agent/guardrails— 99 passeduv run pytest -q— 1788 passed, 13 skippeduv run nat --helpanduv run aiq-research --helpmask_sensitive_dataaction returnedContact <EMAIL_ADDRESS>/app/dataTanner Leach <tleach@nvidia.com>Formal QA cases
26.07_GUARD_001and26.07_GUARD_002remain to be run against a published corrected image.Where should reviewers start?
src/aiq_agent/guardrails/interface/middleware.py— shared exception containment and buffered stream handling.src/aiq_agent/guardrails/workflow/middleware.py— workflow-shaped emergency refusals and terminal-outcome synchronization.tests/aiq_agent/guardrails/test_workflow_guardrails.py— multi-chunk, mixed-order stream, synchronization-failure, no-target, and PII-safe logging regressions.pyproject.tomlanddeploy/Dockerfile— PII extra and release-image verification.Related Issues
Summary by CodeRabbit