fix: redact PII in mark and generic scope events [2/6]#372
Conversation
License DiffCompared against Lockfile license changesLockfile License ChangesRustAdded
Removed
Updated/Changed
NodeAdded
Removed
Updated/Changed
PythonAdded
Removed
Updated/Changed
Status output |
WalkthroughThis PR adds a ChangesMark Field Sanitization Support
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant EventSanitizeCallback
participant CompiledBuiltinBackend
Runtime->>EventSanitizeCallback: sanitize(event, fields)
EventSanitizeCallback->>EventSanitizeCallback: check Event::Scope category
alt tool or llm scope
EventSanitizeCallback-->>Runtime: return fields unchanged
else other event
EventSanitizeCallback->>CompiledBuiltinBackend: sanitize_json_preorder_dfs(data, metadata)
EventSanitizeCallback->>CompiledBuiltinBackend: sanitize_serializable_with_backend(category_profile)
CompiledBuiltinBackend-->>EventSanitizeCallback: sanitized values
EventSanitizeCallback-->>Runtime: return sanitized fields
end
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Will Killian <wkillian@nvidia.com>
56b2944 to
9498178
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/pii-redaction/tests/unit/component_tests.rs (1)
695-708: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winMake metadata passthrough assertions sensitive to accidental event sanitization.
The metadata values match the configured regex, but the configured
target_pathsexclude/owner,/reviewer, and/audit_owner. If the generic event sanitizer accidentally runs for tool/LLM events, these assertions still pass because those metadata fields are out of scope.Proposed fix
- "target_paths": ["/api_key", "/nested/token", "/result/secret"] + "target_paths": ["/api_key", "/nested/token", "/result/secret", "/owner", "/reviewer"]- "target_paths": ["/messages/0/content", "/messages/1/content"] + "target_paths": ["/messages/0/content", "/messages/1/content", "/audit_owner"]- "target_paths": ["/choices/0/message/content"] + "target_paths": ["/choices/0/message/content", "/audit_owner"]As per path instructions, “Tests should cover the behavior promised by the changed API surface.”
Also applies to: 734-741, 2345-2369, 2435-2476
🤖 Prompt for 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. In `@crates/pii-redaction/tests/unit/component_tests.rs` around lines 695 - 708, The metadata passthrough tests around tool/LLM events are too permissive because the current values fall outside the configured target paths, so they won’t catch accidental sanitization. Update the affected assertions in the event test helpers (including the ToolCallEndParams and related LLM/tool event cases) to use metadata fields that are inside the sanitizer scope and expected to be preserved, so a generic event sanitizer running on these events will cause a failure. Keep the checks focused on the passthrough behavior of the metadata fields used by the changed API surface and mirror this adjustment in all referenced test blocks.Source: Path instructions
🤖 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/pii-redaction/src/builtin.rs`:
- Around line 278-280: The `fields.category_profile` sanitization in
`builtin.rs` currently falls back to `unwrap_or(profile)`, which returns the
original unsanitized `CategoryProfile` when `sanitize_serializable_with_backend`
fails. Update the `map` closure to fail closed by discarding the profile on any
sanitization/serialization error instead of restoring the raw value, and keep
the fix localized around the `fields.category_profile` assignment and
`sanitize_serializable_with_backend::<CategoryProfile>` call.
---
Outside diff comments:
In `@crates/pii-redaction/tests/unit/component_tests.rs`:
- Around line 695-708: The metadata passthrough tests around tool/LLM events are
too permissive because the current values fall outside the configured target
paths, so they won’t catch accidental sanitization. Update the affected
assertions in the event test helpers (including the ToolCallEndParams and
related LLM/tool event cases) to use metadata fields that are inside the
sanitizer scope and expected to be preserved, so a generic event sanitizer
running on these events will cause a failure. Keep the checks focused on the
passthrough behavior of the metadata fields used by the changed API surface and
mirror this adjustment in all referenced test blocks.
🪄 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: 35d1c38e-73a9-4a44-bd50-b22f64ee8010
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
crates/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/Cargo.tomlcrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/tests/unit/component_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/tests/unit/component_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/tests/unit/component_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin 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/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/tests/unit/component_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/tests/unit/component_tests.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/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/tests/unit/component_tests.rs
**/*
📄 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, usemaintain-dynamic-pluginsand 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, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/Cargo.tomlcrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/tests/unit/component_tests.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/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/tests/unit/component_tests.rs
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings,
documentation, integrations, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile skills/ # Published Codex/agent skills for NeMo Relay usage patternsPrerequisites
Insta...
Files:
crates/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/Cargo.tomlcrates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rscrates/pii-redaction/tests/unit/component_tests.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/cli/tests/coverage/plugins_tests.rscrates/pii-redaction/tests/unit/component_tests.rs
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.Keep Rust package names and workspace metadata in
Cargo.tomlinternally consistent across the project.
Files:
crates/pii-redaction/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all TOML files using the
#comment form.
Files:
crates/pii-redaction/Cargo.toml
{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 cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/pii-redaction/src/builtin.rscrates/pii-redaction/src/component.rs
🔇 Additional comments (7)
crates/pii-redaction/src/builtin.rs (2)
12-16: LGTM!Also applies to: 262-277, 282-284
12-16: 📐 Maintainability & Code QualityConfirm required Rust validation ran.
Please confirm this Rust change passed
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andjust test-rust. As per coding guidelines,**/*.rs: “Any Rust change must runjust test-rust”, “Any Rust change must runcargo fmt --all”, and “Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings”.Also applies to: 262-284
Source: Coding guidelines
crates/pii-redaction/src/component.rs (1)
84-86: LGTM!Also applies to: 118-118, 206-206, 431-431, 506-506, 734-772
crates/cli/tests/coverage/plugins_tests.rs (1)
299-299: 📐 Maintainability & Code QualityConfirm required Rust validation passed.
This Rust test-surface change needs the repo-mandated checks before handoff:
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andjust test-rust.As per coding guidelines, “Any Rust change must run
just test-rust”, “Any Rust change must runcargo fmt --all”, and “Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings”.Source: Coding guidelines
crates/pii-redaction/Cargo.toml (1)
29-33: LGTM!crates/pii-redaction/tests/unit/component_tests.rs (2)
8-20: LGTM!Also applies to: 31-35, 122-165, 179-179, 429-542
629-638: 🔒 Security & PrivacyNo issue: the snapshot already flushes subscribers.
AtifExporter::export(),OpenTelemetrySubscriber::force_flush(), andOpenInferenceSubscriber::force_flush()all callflush_subscribers()internally, andcaptured_events_snapshot()flushes again before cloning. The exporter outputs are not read before subscriber delivery drains.> Likely an incorrect or invalid review comment.
|
Need to expose the new config attribute in each language binding? I don't see a file change to python: NeMo-Relay/python/nemo_relay/pii_redaction.py Line 150 in 29403a7 Would also be relevant to node, Go |
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/pii-redaction/tests/unit/component_tests.rs (1)
700-782: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSanitize tool/LLM metadata as well
event_sanitize_callbackskipsEvent::Scopetool/llm events entirely, and the dedicated tool/LLM guardrails only sanitize the payload, notmetadata. These assertions lock in unredactedowner/reviewerfields on emitted events; either passmetadatathrough the sanitizer here or avoid attaching sensitive data to these events.🤖 Prompt for 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. In `@crates/pii-redaction/tests/unit/component_tests.rs` around lines 700 - 782, The tool event sanitizer is leaving `metadata` untouched on emitted tool/LLM events, so sensitive fields like `owner` and `reviewer` remain unredacted. Update the sanitization flow around `event_sanitize_callback` and the tool guardrails so `Event::Scope` tool/LLM events have their metadata passed through the same redaction logic as payloads. Preserve the existing payload sanitization in `builtin_backend_sanitizes_tool_start_and_end_payloads_with_preorder_targets`, but ensure metadata is also sanitized before emitting or adjust the event construction to avoid attaching sensitive metadata to these events.
🤖 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/pii-redaction/tests/unit/component_tests.rs`:
- Around line 167-201: The failure-path test for event_sanitize_callback
currently depends on an implicit CategoryProfile deserialization collision via
the annotated_request entry in extra, which is fragile and easy to break if the
schema changes. Make the test intent explicit by either adding a short comment
in event_sanitizer_discards_category_profile_when_sanitization_fails explaining
that this value conflicts with a typed CategoryProfile field during round-trip
deserialization, or by asserting the intermediate sanitization error before
checking that category_profile is dropped.
---
Outside diff comments:
In `@crates/pii-redaction/tests/unit/component_tests.rs`:
- Around line 700-782: The tool event sanitizer is leaving `metadata` untouched
on emitted tool/LLM events, so sensitive fields like `owner` and `reviewer`
remain unredacted. Update the sanitization flow around `event_sanitize_callback`
and the tool guardrails so `Event::Scope` tool/LLM events have their metadata
passed through the same redaction logic as payloads. Preserve the existing
payload sanitization in
`builtin_backend_sanitizes_tool_start_and_end_payloads_with_preorder_targets`,
but ensure metadata is also sanitized before emitting or adjust the event
construction to avoid attaching sensitive metadata to these 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: a4896d0e-9837-4087-97e0-d891867b70f8
📒 Files selected for processing (2)
crates/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin 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/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.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/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.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 cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/pii-redaction/src/builtin.rs
**/*
📄 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, usemaintain-dynamic-pluginsand 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, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.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/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.rs
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings,
documentation, integrations, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile skills/ # Published Codex/agent skills for NeMo Relay usage patternsPrerequisites
Insta...
Files:
crates/pii-redaction/src/builtin.rscrates/pii-redaction/tests/unit/component_tests.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/pii-redaction/tests/unit/component_tests.rs
🔇 Additional comments (3)
crates/pii-redaction/src/builtin.rs (2)
262-283: LGTM!Confirms the prior fail-open finding on
category_profilesanitization (raw profile leaking viaunwrap_or(profile)) is now fixed — errors now discard the profile viaand_then(...).ok()instead of restoring unsanitized data.
12-16: LGTM!crates/pii-redaction/tests/unit/component_tests.rs (1)
8-37: LGTM!
|
/merge |
#### Overview Exposes the mark and scope event sanitizer registries from PRs #371 and #372 through the Node.js binding and plugin context. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Adds runtime and scope-local N-API registrations. - Extends JavaScript `PluginContext` and TypeScript `EventSanitizeFields` declarations. - Adds `mark` to PII configuration typing and defaults. - Uses the direct/thread-safe callback paths required by synchronous and async runtimes. - Tests conversion, ordering, field removal, invalid callbacks, scope cleanup, plugin cleanup, and PII configuration. - Contains no documentation changes; documentation is isolated in a final independent PR. - Breaking changes: none. Validation: `just test-node` (251 tests), focused event sanitizer tests, 100% JavaScript wrapper coverage, formatting, docstring checks, Clippy, and pre-commit. #### Where should the reviewer start? Start with `crates/node/src/api/mod.rs`, `crates/node/src/callable.rs`, and `crates/node/tests/event_sanitizers_tests.mjs`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: RELAY-409 ## Summary by CodeRabbit * **New Features** * Added `mark` support to the default PII redaction configuration. * Added event sanitizer guardrails for mark sanitization and scope lifecycle (start/end), including scope-local guardrails. * Introduced an event sanitizer result payload for JS↔runtime sanitization. * **Bug Fixes** * Event sanitizers now “fail open” on invalid sanitizer results, preserving the original event payload. * Improved sanitizer cleanup so guardrails are removed when a plugin or scope is cleared. * **Tests** * Added/expanded coverage for sanitizer ordering, scope inheritance, tool execution paths, and failure handling. Authors: - Will Killian (https://github.com/willkill07) Approvers: - Bryan Bednarski (https://github.com/bbednarski9) URL: #374
#### Overview Exposes the mark and scope event sanitizer registries from PRs #371 and #372 through the raw C FFI and experimental Go binding. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Adds the C callback type plus global, scope-local, and plugin-context exports. - Regenerates the public FFI header through the repository workflow. - Adds Go `EventSanitizeFields`, callbacks, registrations, plugin methods, and guardrail shorthands. - Adds Go PII `mark` configuration parity. - Tests all fields and phases, inheritance, cleanup, rollback, invalid callback fail-open, invalid UUIDs, duplicate registration, and callback release. - Contains no documentation changes; documentation is isolated in a final independent PR. - Breaking changes: none. Validation: FFI unit/integration tests, FFI coverage (94.26% line; no uncovered executable lines in the new registry file), `just build-go`, `just test-go`, CI Go coverage (95% main package; 100% guardrail shorthand package), `just test-rust`, formatting, Clippy, vet, and pre-commit. #### Where should the reviewer start? Start with `crates/ffi/src/api/event_registry.rs`, `go/nemo_relay/event_sanitizers_test.go`, and the FFI registry coverage test. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: RELAY-409 ## Summary by CodeRabbit * **New Features** * Added event sanitizer guardrails for mark, scope-start, and scope-end events, with global, scope-local, and plugin-context registration/deregistration. * Introduced a sanitizer callback that can rewrite event observability fields. * Updated Go/C bindings to support registering and removing these guardrails. * Enabled mark-based behavior in the default PII redaction configuration. * **Bug Fixes** * Invalid or null sanitizer output no longer overwrites existing event fields. * Failed sanitizer registrations now clean up correctly without leaks. Authors: - Will Killian (https://github.com/willkill07) Approvers: - Bryan Bednarski (https://github.com/bbednarski9) URL: #375
#### Overview Exposes the mark and scope event sanitizer registries from PRs #371 and #372 through the Python binding and worker SDK. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Adds Python callback types and global/scope-local registration helpers. - Extends the in-process `PluginContext` and Python gRPC worker SDK. - Adds `mark` to typed PII configuration helpers. - Tests field conversion/removal, errors, ordering, scope inheritance, plugin rollback/cleanup, worker surfaces, and PII opt-out. - Contains no documentation changes; documentation is isolated in a final independent PR. - Breaking changes: none. Validation: `just test-python`, `just test-python-plugin`, focused native tests and coverage, Ruff, ty, and pre-commit. #### Where should the reviewer start? Start with `python/nemo_relay/guardrails.py`, `python/nemo_relay/scope_local.py`, and `python/tests/test_event_sanitizers.py`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: RELAY-409 ## Summary by CodeRabbit * **New Features** * Added Python event sanitization guardrails for mark events and scope start/end events, available globally and per-scope. * Introduced `EventSanitizeFields` and `EventSanitizeGuardrail` typing for sanitizer callbacks, plus new registration/deregistration helpers. * Extended the plugin registration surface and updated public typings to match. * Added a `mark` option to PII redaction configuration. * **Bug Fixes** * Improved “fail open” behavior: if a sanitizer raises or returns an invalid value, sanitization safely falls back to the original event fields. Authors: - Will Killian (https://github.com/willkill07) Approvers: - Bryan Bednarski (https://github.com/bbednarski9) URL: #373
#### Overview Documents the event sanitizer and PII redaction behavior implemented by the merged RELAY-409 stack (#371–#375). This docs-only PR is the final part of that six-PR stack. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Adds a dedicated event sanitizer reference covering callback replacement semantics, ordering, registration lifetimes, cross-language APIs, and dynamic-plugin protocol values. - Explains which mark and scope event fields sanitizers can replace and which lifecycle and identity fields remain immutable. - Documents PII `mark = true` defaults, opt-out behavior, generic scope `input` and `output` coverage, and subscriber/exporter privacy guarantees. - Updates related middleware, event, scope/mark, plugin-authoring, Python, Node.js, Go, and observability-skill guidance. - Contains no implementation changes. - Breaking changes: none. Validation: - `just docs` - `just docs-linkcheck` - `uv run pre-commit run --all-files` #### Where should the reviewer start? Start with `docs/reference/event-sanitizers.mdx`, then review `docs/configure-plugins/pii-redaction/configuration.mdx` for the privacy contract. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: RELAY-409 - Relates to: #371, #372, #373, #374, and #375 ## Summary by CodeRabbit - **New Features** - Added observability sanitization for **mark** and **scope** events (including scope-start and scope-end) before delivery to subscribers/exporters. - Sanitizers can rewrite observability fields (`data`, `category_profile`, `metadata`) while keeping core identity/context fields immutable. - Expanded privacy redaction support with a configurable `mark` option (default enabled) for mark and generic scope events. - **Documentation** - Added a new Event Sanitizers reference covering registration levels, ordering, callback contracts, and examples. - Updated middleware and language-binding guidance plus Node.js/Go/Python overviews to reflect mark/scope sanitizer capabilities. Authors: - Will Killian (https://github.com/willkill07) Approvers: - https://github.com/lvojtku - Bryan Bednarski (https://github.com/bbednarski9) URL: #376
Overview
Extends the built-in PII redaction policy onto the event sanitizer infrastructure from PR #371 so raw PII does not escape through marks or generic scope events.
Details
inputto LLM requests and non-tool/LLM scope starts.outputto LLM responses and non-tool/LLM scope ends.mark, defaulting totrue, withmark = falseas the opt-out.data,category_profile, andmetadataindependently.markis omitted.Validation:
nemo-relay-pii-redactiontests and coverage, relevant CLI/config tests, full Rust and binding matrices, and pre-commit.Where should the reviewer start?
Start with
crates/pii-redaction/src/component.rs,crates/pii-redaction/src/builtin.rs, and the cross-exporter PII regression in the component tests.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
mark), defaulting to enabled and configurable via the editor.Bug Fixes
Tests