Skip to content

fix: redact known LLM credential headers in core - #563

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
willkill07:wkk_relay-572-core-credential-redaction
Jul 27, 2026
Merged

fix: redact known LLM credential headers in core#563
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
willkill07:wkk_relay-572-core-credential-redaction

Conversation

@willkill07

@willkill07 willkill07 commented Jul 27, 2026

Copy link
Copy Markdown
Member

Overview

Move standard credential-header filtering into the Rust core LLM event path.

  • I confirm this contribution is my own work, or I have the right to submit it under this project license.
  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Details

  • Remove known credential headers case-insensitively from the event-only LLM request copy before sanitize-request guardrails run.
  • Include the Gemini credential header x-goog-api-key.
  • Preserve provider-bound request headers for manual, buffered, and streaming execution.
  • Add regression coverage for mixed-case credential headers and update middleware/API documentation.

Where should the reviewer start?

Start with crates/core/src/api/llm.rs: the event-only clone is filtered before the LLM sanitizer chain. crates/core/tests/unit/llm_api_tests.rs verifies the manual, buffered, and streaming paths, including the Gemini header.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

Validation

  • cargo fmt --all
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo deny check (passed; existing duplicate-dependency warnings only)
  • Focused core credential-header regression test passed, including mixed-case x-goog-api-key.
  • just test-python (556 passed)
  • just test-node (290 passed)
  • just test-go
  • just docs
  • uv run pre-commit run --all-files, plus changed-file hooks after the Gemini follow-up.
  • The full just test-rust command is blocked locally by an unrelated CLI integration test reading existing user-level dynamic-plugin state; details are recorded on RELAY-572.

Summary by CodeRabbit

Summary

  • Bug Fixes

    • Standard credential headers are now removed from the request snapshot used for sanitization and observability events.
    • Credential header matching is now case-insensitive for more reliable redaction.
    • Provider-facing LLM execution requests remain unchanged.
  • Tests

    • Added coverage to verify sanitized requests and LLM start event payloads omit credential headers.
  • Documentation

    • Updated middleware docs to clarify the middleware order and the exact headers removed, plus guidance for custom credential header names.

Signed-off-by: Will Killian <wkillian@nvidia.com>
@willkill07
willkill07 requested review from a team as code owners July 27, 2026 14:50
@github-actions github-actions Bot added size:M PR is medium Bug issue describes bug; PR fixes bug lang:rust PR changes/introduces Rust code labels Jul 27, 2026
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

LLM observability now removes standard credential headers from an event-only request copy before sanitization and event emission. Provider-bound requests remain unchanged. Tests cover case-insensitive redaction across manual, buffered, and streaming calls, and documentation describes the behavior.

Changes

Credential observability sanitization

Layer / File(s) Summary
Observable request filtering and sanitization
crates/core/src/api/llm.rs, docs/about-nemo-relay/concepts/middleware.mdx
Standard credential headers are removed case-insensitively from the event-only request copy before sanitizer guardrails and LLM start events receive it; execution continues using the original request.
Redaction and provider-request validation
crates/core/tests/unit/llm_api_tests.rs
Tests verify redaction for mixed-case headers across manual, buffered, and streaming calls, while provider invocations retain the original headers.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • NVIDIA/NeMo-Relay#546 — Both changes modify the LLM event-emission sanitization path in crates/core/src/api/llm.rs.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change sanitizes emitted LLM event headers case-insensitively without altering provider requests, matching #560's main objective.
Out of Scope Changes check ✅ Passed Only core logic, regression tests, and related docs were changed, and they all support the stated header-redaction goal.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title follows Conventional Commits and accurately summarizes the core credential-header redaction change.
Description check ✅ Passed The description matches the template well, with overview, details, reviewer start, related issues, and validation included.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@crates/core/src/api/llm.rs`:
- Around line 45-52: Update OBSERVABILITY_CREDENTIAL_HEADERS to include
"x-goog-api-key", then mirror this header in the related documentation and tests
so Gemini credentials are redacted before sanitize callbacks and emitted events.
🪄 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: b3dea67b-3a62-4014-999d-62b3d715e3e8

📥 Commits

Reviewing files that changed from the base of the PR and between 42c7655 and 002994e.

📒 Files selected for processing (3)
  • crates/core/src/api/llm.rs
  • crates/core/tests/unit/llm_api_tests.rs
  • docs/about-nemo-relay/concepts/middleware.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Check / Run
  • GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (23)
**/*.mdx

📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)

MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)

In MDX files, top-of-file comments must use JSX comment delimiters ({/* to open and */} to close); do not use HTML comments for MDX SPDX headers

Files:

  • docs/about-nemo-relay/concepts/middleware.mdx
**/*.{md,mdx}

📄 CodeRabbit inference engine (AGENTS.md)

Update README.md, fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.

**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such as RELEASING.md, not as user-facing docs pages or CHANGELOG.md
Keep stable user-facing wrappers at scripts/ root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, and grpc-v1 protocol details on separate pages

If links in documentation change, run just docs-linkcheck.

Files:

  • docs/about-nemo-relay/concepts/middleware.mdx
**/*.{md,markdown,mdx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.

Files:

  • docs/about-nemo-relay/concepts/middleware.mdx
{docs,examples}/**/*

📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)

Update docs and examples.

Files:

  • docs/about-nemo-relay/concepts/middleware.mdx
**/*

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, use maintain-dynamic-plugins and include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, prefer uv run pre-commit run --files <changed files...>.
Before review or handoff, run uv run pre-commit run --all-files.

Files:

  • docs/about-nemo-relay/concepts/middleware.mdx
  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
docs/**/*

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If documentation examples or commands under docs/ change, run the targeted docs checks appropriate to the change.

Files:

  • docs/about-nemo-relay/concepts/middleware.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}

⚙️ CodeRabbit configuration file

{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.

Files:

  • docs/about-nemo-relay/concepts/middleware.mdx
**/*.rs

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

When changing the core Rust runtime or Rust-facing API surface, format Rust code with cargo fmt (rustfmt defaults), keep cargo clippy -- -D warnings clean, and satisfy cargo deny check per deny.toml.

**/*.rs: If any Rust code changed, always run just test-rust.
If any Rust code changed, also run cargo fmt --all.
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, run cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings even if relying on pre-commit.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
{crates/core,crates/adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

Changes to crates/core or crates/adaptive must run the full language matrix

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
crates/core/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions in Rust and Python: use snake_case.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,py,js,mjs,cjs,ts,tsx}: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
**/*.{rs,py,go,js,ts,c,h}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use language-appropriate naming conventions: Rust snake_case, C FFI exports prefixed nemo_relay_, Go PascalCase, Node.js camelCase, and Python snake_case.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
**/*.{rs,go,js,ts}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding // comment form.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
crates/{core,adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If crates/core or crates/adaptive changed, run the full validation matrix across Rust, Python, Go, and Node.js.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
**/*.{rs,py,go,js,ts}

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If a language surface changed, always run that language's test target even when Rust core did not change.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}

📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)

Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
crates/{core,adaptive}/**/*.rs

⚙️ CodeRabbit configuration file

crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
  • crates/core/src/api/llm.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}

⚙️ CodeRabbit configuration file

{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.

Files:

  • crates/core/tests/unit/llm_api_tests.rs
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Implement the new or changed public runtime behavior first in the Rust core, especially under crates/core/src/api/ and related core modules such as crates/core/src/api/runtime/, crates/core/src/codec/, and crates/core/src/json.rs.

Files:

  • crates/core/src/api/llm.rs
{crates/**/src/**/*.rs,python/**/*.py}

📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)

Do not add tests under src; Rust tests belong in crate tests/ trees, and Python SDK tests belong under python/tests.

Files:

  • crates/core/src/api/llm.rs
crates/core/src/api/{tool,llm,shared,scope}.rs

📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)

Wire the new middleware chain into the appropriate lifecycle owner and pipeline stage: tool and LLM execution paths use tool.rs or llm.rs; shared mark and scope event sanitization uses shared.rs and is called from scope.rs.

Files:

  • crates/core/src/api/llm.rs
crates/core/src/api/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)

Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.

Files:

  • crates/core/src/api/llm.rs
🔇 Additional comments (1)
crates/core/src/api/llm.rs (1)

442-450: 📐 Maintainability & Code Quality

Provide required core-Rust validation results.

Please attach successful cargo fmt --all, cargo clippy --workspace --all-targets -- -D warnings, just test-rust, cargo deny check, validate-change, and the required Rust/Python/Go/Node validation matrix.

As per coding guidelines, “Changes to crates/core or crates/adaptive must run the full language matrix” and core runtime changes must satisfy cargo deny check.

Source: Coding guidelines

Comment thread crates/core/src/api/llm.rs Outdated
@github-actions

Copy link
Copy Markdown

@willkill07 willkill07 self-assigned this Jul 27, 2026
@willkill07 willkill07 added this to the 0.7 milestone Jul 27, 2026
Signed-off-by: Will Killian <wkillian@nvidia.com>
@willkill07

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit b3f5b37 into NVIDIA:main Jul 27, 2026
70 checks passed
@willkill07
willkill07 deleted the wkk_relay-572-core-credential-redaction branch August 4, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug issue describes bug; PR fixes bug lang:rust PR changes/introduces Rust code size:M PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: PII redaction leaves LLM request headers in emitted events

2 participants