fix(deps): clear MCP runtime audit advisories - #8175
Merged
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThe change adds an HTTPS registry fetch cancellation test. It aborts the request after proxy tunnel establishment, checks for ChangesProxy abort testing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit ade9f9c in the TypeScript / code-coverage/cliThe overall coverage in commit ade9f9c in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
2 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 1 semantic terminology decisionTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
This was referenced Aug 3, 2026
2 tasks
23 tasks
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
23 tasks
<!-- markdownlint-disable MD041 --> ## Summary The managed registry proxy test now exercises the production-like HTTPS proxy route used by the registry client. Undici 8.10.0 sends an HTTP target as a plain proxy request, so the previous synthetic HTTP target could never reach the test's CONNECT handler and timed out in coverage shard 8. ## Changes - Use an HTTPS registry target so the loopback proxy observes a CONNECT tunnel. - Abort the test request once that stable behavior boundary is observed. - Assert both the `AbortError` rejection and `registry.invalid:443` authority. - Destroy the accepted test socket during deterministic `finally` cleanup. - Leave production registry-fetch behavior unchanged. ## Stacked landing This test-only PR is temporarily based on #8175 so CI evaluates the MCP audit fix and this Undici test correction together without creating a circular red dependency. Its public diff remains one test file. Once this PR is approved and green, it should merge into #8175; #8175 can then be revalidated and merged to `main` when its refreshed exact head is approved and green. Auto-merge is intentionally disabled. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: This changes one deterministic test route and does not change a public API, CLI, configuration, default, error, or supported product behavior. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: The nine-category exact-head review passed at `405bdd6c09e60205003cb032b5a1f7f088c9799d`; the signed synchronization commit is empty and preserves reviewed tree `c3ad47e26a5387ce62fe95116730cc2777456719`. This is a loopback-only test change with no production, dependency, credential, or external-network behavior. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: At exact head `405bdd6c09e60205003cb032b5a1f7f088c9799d`, the PR still changes only the loopback HTTPS CONNECT fixture. The signed synchronization commit is empty and has the same tree as reviewed parent `21ed6d13a`. No production or user-visible behavior changed; no explanatory comments or test titles changed; the writing review found no findings. `git diff --check` passed and the focused Vitest file passed 5/5. A docs build is not applicable. - Agent: Codex Desktop <!-- docs-review-head-sha: 405bdd6 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — exact-head focused Vitest passed 5/5; the prior exact behavior correction passed coverage shard 8 in 8m47s. - [x] Exact-head CLI TypeScript check and `git diff --check` passed. - [ ] Applicable broad gate passed — exact-head stacked CI is authoritative and pending. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> Signed-off-by: Julie Yaunches <jmyaunch@gmail.com> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@src/lib/onboard/managed-image-registry-fetch.test.ts`:
- Line 47: Update the server-side connect handler in the managed image registry
fetch test so it writes the 200 Connection Established response before calling
abortController.abort(). Keep the existing assertion at the fetchImpl public
boundary.
🪄 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: CHILL
Plan: Enterprise
Run ID: c1f821a7-f681-4311-94f5-d0908e79ad54
📒 Files selected for processing (1)
src/lib/onboard/managed-image-registry-fetch.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
pushed a commit
that referenced
this pull request
Aug 4, 2026
<!-- markdownlint-disable MD041 --> ## Summary Refreshes the reviewed MCP tool-discovery runtime lock so exact-source image builds no longer fail on the `fast-uri`, Hono, and `ip-address` advisories reported against NemoClaw `v0.0.100`. The strict `npm audit --omit=dev --audit-level=low` gate remains unchanged. ## Related Issue Fixes #8177 ## Changes - Pin `@hono/node-server@2.0.12`, `fast-uri@3.1.5`, `hono@4.12.34`, and `ip-address@10.3.1` through the runtime's existing override mechanism and regenerate the immutable lock. - Record the reviewed package integrities, upstream ranges, runtime exposure, and exact verification evidence. - Bind the reviewed manifest, lock metadata, review record, and fail-closed audit command in the image contract regression. - Update the managed-image proxy test for Undici 8.10's forward-proxy request behavior so the CLI shard terminates deterministically. PR #8172 overlaps this dependency graph but also changes audit policy and remains blocked on separate sequencing. PR #8175 independently applies the same three version pins and lock metadata; this PR additionally binds the exact reviewed graph and fail-closed installer command in the deterministic regression required by #8177. No material work was transferred between the branches. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: No public API, CLI, configuration, workflow, default, or supported behavior changes. The internal dependency review was updated with the exact graph and audit evidence. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Exact-head nine-category security review found no findings. The change advances exact MIT-licensed registry pins, retains integrity binding, adds regressions, and preserves the fail-closed audit boundary. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: All six changed files were reviewed on exact head `330b13d1cc79e61bcfa0eee1f96963143deac55b`. `tools/mcp-tool-discovery-runtime/dependency-review.md` aligns with the exact manifest, lock integrities, installer audit boundary, and bundle scope. The reviewer independently confirmed the `@hono/node-server@2.0.12` registry identity, unsigned tag and release commit, three-commit source range, and SLSA attestation binding to commit `a813b6cdaa15baac3ead84e9e6ed5b72b2353c96`. The final proxy fixture matches Undici 8.10's forward-proxy behavior, changes no production behavior, and needs no user-facing `docs/` update. The reviewed tree `2504a6b78e4447af9734dd34b6ebe81da047d207` matches the validated maintainer tree, and `git diff --check` passed. - Agent: Codex Desktop, independent documentation-writer subagent <!-- docs-review-head-sha: 330b13d --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable - Result: Not applicable - Supporting evidence: Not applicable ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — focused Vitest passed 9/9 on exact head, including the proxy contract 5/5; exact standalone `install-reviewed-runtime.sh` completed with 98 package signatures, 12 provenance attestations, runtime tests, type-check, bundle verification, and 0 audit vulnerabilities. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to the isolated lock refresh and proxy test correction. `npm run checks:repository`, `npm run build:cli`, `npm run typecheck:cli`, Biome, and normal pre-commit and commit-message hooks passed. Required CI remains authoritative. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## Security Review No findings on exact head `330b13d1cc79e61bcfa0eee1f96963143deac55b`. | Category | Verdict | Evidence | |---|---|---| | Secrets and Credentials | PASS | No credentials, secret-bearing values, or credential paths changed. | | Input Validation and Data Sanitization | PASS | Exact manifest, version, archive URL, integrity, review record, and installer command assertions reject dependency-graph drift. | | Authentication and Authorization | PASS | No authentication or authorization behavior changed. | | Dependencies and Third-Party Libraries | PASS | Exact MIT-licensed pins move the reviewed production graph outside the reported advisory ranges; the standalone audit reports 0 vulnerabilities. | | Error Handling and Logging | PASS | No production logging or error surface changed. | | Cryptography and Data Protection | PASS | SHA-512 package integrity remains locked; 98 registry signatures and 12 attestations verified. | | Configuration and Security Headers | PASS | The image's fail-closed low-severity audit remains unchanged; ports, permissions, users, and headers are unaffected. | | Security Testing | PASS | The deterministic image contract binds the reviewed graph and audit command; the proxy regression covers the Undici 8.10 behavior that failed CI. | | System Security | PASS | The change removes vulnerable runtime versions without weakening image or sandbox controls. | --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security** * Updated security-reviewed runtime components and pinned versions to address known advisory concerns. * Strengthened dependency integrity, provenance, and installation auditing requirements. * Added automated checks covering dependency versions, integrity verification, manifests, lockfiles, and audit procedures. * **Documentation** * Updated security review records with refreshed validation evidence and audit results. * Confirmed the production bundle remains unchanged and free of reported vulnerabilities. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
23 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add deterministic regression coverage for cancelling an HTTPS registry request after its configured proxy has completed the CONNECT handshake.
The independent MCP runtime audit fix is already merged on
mainthrough #8180. This branch now integrates that exact base and intentionally carries no dependency, lockfile, review-record, or production-code delta.Changes
main's plain HTTP forward-proxy coverage.200 Connection Establishedbefore aborting the request signal.AbortError, the tunnel targetsregistry.invalid:443, and the accepted socket is cleaned up infinally.Type of Change
Quality Gates
ade9f9cfc588124d0ffac8ced6e594cc0b40aba7.Documentation Writer Review
no-docs-needed1ee723a3cto headade9f9cfc. It adds only a 32-line HTTPS proxy CONNECT cancellation regression test. No production or user-visible behavior changed. The test title follows the writing guide. Findings: none.git diff --checkpassed, and the focused Vitest file passed 6/6.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passednpm run docsbuilds without warnings (doc changes only)Security Review
No findings at exact head
ade9f9cfc588124d0ffac8ced6e594cc0b40aba7.The effective diff is one test file with 32 additions. The fixture is loopback-only, completes the CONNECT handshake before cancellation, asserts the abort-specific public error, and deterministically destroys the socket and closes the dispatcher in
finally. No dependencies, production configuration, authentication, TLS trust, credentials, or runtime controls change. All nine review categories pass.Signed-off-by: Aaron Erickson aerickson@nvidia.com