fix: prevent URL masks from retaining credentials

[afourniernv]

Overview

Fixes the built-in PII URL mask so it preserves only the scheme, host, and optional port. URL credentials are removed, and pathless query or fragment data is masked instead of being returned unchanged.

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

Details

• Split the URL authority from paths, queries, fragments, and backslash path separators.
• Remove user:password@ data before rebuilding the masked URL.
• Keep host-only URLs unchanged and collapse any suffix to /*.
• Expand the external component test with credentials, queries, fragments, backslash paths, ports, and IPv6 hosts, proving sensitive values do not reach emitted tool events.

Validation:

• cargo test -p nemo-relay-pii-redaction -- --test-threads=1
• just test-rust (4,677 workspace tests and all plugin-example suites)
• cargo clippy --workspace --all-targets -- -D warnings
• uv run pre-commit run --all-files

Where should the reviewer start?

crates/pii-redaction/src/detectors.rs, in mask_url; its component-level coverage is in crates/pii-redaction/tests/unit/component_tests.rs.

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

• Closes PII URL masking preserves embedded credentials #1014

Summary by CodeRabbit

• Bug Fixes

  • URL masking now removes embedded credentials and masks paths, query strings, fragments, and backslash-delimited suffixes.
  • Hosts, ports, IPv6 addresses, and URLs without suffixes are preserved correctly.
  • Malformed or empty authorities receive full masking.

• Tests

  • Expanded coverage for credential removal, query and fragment masking, paths, backslashes, ports, IPv6 hosts, pathless URLs, and malformed authorities.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: ef52b47c-935b-444a-9c1f-f5366646207f

📥 Commits

Reviewing files that changed from the base of the PR and between d29c6c5 and 3f57a08.

📒 Files selected for processing (1)

• crates/pii-redaction/tests/unit/component_tests.rs

Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (22)

Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.

⚙️ CodeRabbit configuration file

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

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

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

Keep async behavior on the existing tokio-based model.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

[ ] Do all bindings expose the same logical knobs and semantics?

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

If any Rust code changed, always run `just test-rust`.

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

**Formatting**: `cargo fmt` (rustfmt defaults) **Linting**: `cargo clippy -- -D warnings` -- all warnings are treated as errors

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

If any Rust code changed, also run `cargo fmt --all`.

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

Use `Json = serde_json::Value` in Rust-facing runtime APIs where the existing code expects JSON payloads.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

[ ] Branch scope is coherent and reviewable [ ] Relevant tests passed under `validate-change` [ ] Docs and examples updated for any public behavior changes [ ] Pull request title follows Conventional Commit style and uses the correct type U...

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

Format changed files with the language-native formatter before the final lint/test pass.

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

Keep NeMo Relay optional Use stable, documented framework or plugin APIs Wrap tool and LLM paths at the correct framework boundary Preserve the framework's original behavior when NeMo Relay is absent Integration uses public framework or plu...

📄 CodeRabbit inference engine (.agents/skills/contribute-integration/SKILL.md)

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

Tool execution callbacks and each execution-intercept `next` continuation return the canonical `ToolExecutionResult { result, annotation }`.

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

Keep SPDX headers on source, docs, scripts, and configuration files.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

**Validation** Run the validation matrix from the `validate-change` skill for the affected surfaces.

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

Use `test-ffi-surface`.

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

[ ] Any Rust change ran `just test-rust` [ ] Any Rust change ran `cargo fmt --all` [ ] Any Rust change ran `cargo clippy --workspace --all-targets -- -D warnings`

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

Follow binding naming conventions: Rust and Python `snake_case`, C FFI exports prefixed `nemo_relay_`, Go `PascalCase` for public APIs, Node.js `camelCase`.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

[ ] SPDX license header on any new files

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

Update docs and examples in the same branch.

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

Files:

• crates/pii-redaction/tests/unit/component_tests.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

📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)

Files:

• crates/pii-redaction/tests/unit/component_tests.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...

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

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`

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

Files:

• crates/pii-redaction/tests/unit/component_tests.rs

🔇 Additional comments (1)

crates/pii-redaction/tests/unit/component_tests.rs (1)

949-949: LGTM!

Also applies to: 1007-1007, 1017-1017, 1077-1077, 2646-2646, 2742-2742

---

Walkthrough

The URL detector removes embedded credentials, preserves the scheme and host, and masks path, backslash, query, and fragment suffixes. Tests cover credentials, host-only URLs, ports, IPv6 hosts, and malformed authorities.

Changes

URL PII masking

Layer / File(s)	Summary
URL masking behavior and regression coverage crates/pii-redaction/src/detectors.rs, crates/pii-redaction/tests/unit/component_tests.rs	mask_url removes userinfo and masks URL suffixes introduced by /, \, ?, or #. Table-driven tests verify host and port preservation across supported URL forms.

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 3f57a

URL redaction now removes credentials and masks URL suffixes that could contain sensitive data, with regression coverage for the supported URL forms. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description includes all required sections, completed overview confirmations, implementation details, reviewer guidance, validation steps, and a valid related issue reference.
Linked Issues check	✅ Passed	The changes satisfy issue #1014 by removing URL userinfo, preserving the scheme, host, and optional port, and masking path, query, fragment, and backslash-separated suffixes.
Out of Scope Changes check	✅ Passed	The implementation and expanded regression tests remain within the scope of the built-in PII URL detector requirements in issue #1014.
Title check	✅ Passed	The title follows Conventional Commits format, uses the allowed lowercase type "fix", provides a concise imperative summary, and stays within 72 characters.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[willkill07]

LGTM, but need to move tests out of detectors.rs

[afourniernv]

Moved the URL-mask coverage out of detectors.rs and into the existing external component test. The production file no longer contains a test module. The revised PII suite, workspace clippy, and targeted pre-commit checks pass.

[willkill07]

/ok to test d29c6c5

[willkill07]

/ok to test 3f57a08

[afourniernv]

/merge