test(security): bind audit provenance to checked-in policy - #8172
Conversation
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds seven temporary npm audit exceptions, updates MCP discovery dependency overrides, and adds exact registry and runtime dependency validation. Security documentation records expiry, provenance, compensating controls, successor locks, and removal requirements. ChangesManaged npm audit transition
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RuntimeAudit
participant ExceptionRegistry
participant LockReview
participant ProvenanceTests
RuntimeAudit->>ExceptionRegistry: load seven exact-match exceptions
ExceptionRegistry->>LockReview: verify graph and lock provenance
LockReview-->>RuntimeAudit: accept bounded findings
RuntimeAudit->>ProvenanceTests: derive audit status and exception list
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit b8ebab7 in the TypeScript / code-coverage/cliThe overall coverage in commit b8ebab7 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: None This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Prekshi Vyas <prekshiv@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: 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 `@test/reviewed-npm-audit.test.ts`:
- Around line 122-141: Update the test around CHECKED_IN_POLICY to assert all
seven exception identities, checking each entry’s advisory, graph, package,
installedVersion, severity, decision, expires, and trackingIssue. Retain the
existing length assertion so additional entries fail, and replace the
insufficient single-entry/graph-set coverage without asserting implementation
details beyond the policy identities.
🪄 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: b9258c2b-20af-4a2e-98c4-446873dad5cb
📒 Files selected for processing (6)
agents/openclaw/dependency-review.mdci/npm-audit-exceptions.jsondocs/security/openclaw-2026.6.10-dependency-review.mddocs/security/openclaw-2026.7.1-dependency-review.mdtest/openclaw-dependency-review.test.tstest/reviewed-npm-audit.test.ts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
prekshivyas
changed the title
|
🌿 Preview your docs: https://nvidia-preview-pr-8172.docs.buildwithfern.com/nemoclaw |
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 `@test/mcp-tool-discovery-image-contract.test.ts`:
- Around line 24-52: Update the dependency-review contract around the manifest
and lock assertions to cover `@hono/node-server` as the fourth override. Add its
exact version, resolved URL, integrity, and applicable engines to
dependency-review.md, then assert the same metadata in its lock entry and
documented review text alongside fast-uri, hono, and ip-address.
🪄 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: 7a675e2e-9c55-4008-9d29-1c56054f625b
⛔ Files ignored due to path filters (1)
tools/mcp-tool-discovery-runtime/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
ci/npm-audit-exceptions.jsonci/source-shape-test-budget.jsondocs/security/openclaw-2026.7.1-dependency-review.mdtest/mcp-tool-discovery-image-contract.test.tstest/reviewed-npm-audit.test.tstools/mcp-tool-discovery-runtime/dependency-review.mdtools/mcp-tool-discovery-runtime/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- ci/npm-audit-exceptions.json
- docs/security/openclaw-2026.7.1-dependency-review.md
- test/reviewed-npm-audit.test.ts
| expect(manifest.overrides).toEqual({ | ||
| "@hono/node-server": "2.0.11", | ||
| "fast-uri": "3.1.5", | ||
| hono: "4.12.34", | ||
| "ip-address": "10.3.1", | ||
| }); | ||
| expect(lock.packages["node_modules/fast-uri"]).toMatchObject({ | ||
| version: "3.1.5", | ||
| resolved: "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.5.tgz", | ||
| integrity: | ||
| "sha512-gHwA1O9LDIcKunMKhObS/HimwtehO1nPUECKAu5TpKgaO19fcWEl4bliWe1jWxVFvIXztJjjQ4L8XQ1EU9f7Jw==", | ||
| }); | ||
| expect(lock.packages["node_modules/ip-address"]).toMatchObject({ | ||
| version: "10.3.1", | ||
| resolved: "https://registry.npmjs.org/ip-address/-/ip-address-10.3.1.tgz", | ||
| integrity: | ||
| "sha512-1e9d3kb97NHJTIJDZW9rKqW2h6+dFa50Dy0fpPSMQp2ADje5gvKsXmdiK6dwY5t76TaTt5+P5N1Y/LoToIxP6g==", | ||
| engines: { node: ">= 12" }, | ||
| }); | ||
| expect(lock.packages["node_modules/hono"]).toMatchObject({ | ||
| version: "4.12.34", | ||
| resolved: "https://registry.npmjs.org/hono/-/hono-4.12.34.tgz", | ||
| integrity: | ||
| "sha512-GqXJqY/xJkJmuloTrnV1ZEXG3fqte+VjkUqoRNZXcrUidiUOP4fMSIHHY4tsqZBK++kVyWmt/AAfSUuy57/eSA==", | ||
| engines: { node: ">=16.9.0" }, | ||
| }); | ||
| expect(review).toContain("`fast-uri@3.1.5`"); | ||
| expect(review).toContain("`hono@4.12.34`"); | ||
| expect(review).toContain("`ip-address@10.3.1`"); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate test ---'
cat -n test/mcp-tool-discovery-image-contract.test.ts
printf '%s\n' '--- related files ---'
rg -n -S 'mcp-tool-discovery|dependency-review|`@hono/node-server`|fast-uri|ip-address|hono' . \
-g '!node_modules' -g '!dist' -g '!build' | head -250Repository: NVIDIA/NemoClaw
Length of output: 35521
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- dependency review metadata ---'
sed -n '12,66p' tools/mcp-tool-discovery-runtime/dependency-review.md
printf '%s\n' '--- lock entries for all four overrides ---'
python3 - <<'PY'
import json
from pathlib import Path
root = Path("tools/mcp-tool-discovery-runtime")
manifest = json.loads((root / "package.json").read_text())
lock = json.loads((root / "package-lock.json"))
for name, version in manifest["overrides"].items():
key = f"node_modules/{name}"
print(name, key)
print(json.dumps(lock["packages"].get(key), indent=2, sort_keys=True))
PY
printf '%s\n' '--- tests that inspect this runtime review or lock ---'
rg -n -S 'tools/mcp-tool-discovery-runtime|dependency-review.md|node_modules/@hono/node-server|expectedHonoNodeServer' test tools \
-g '*.test.*' -g '*.ts' -g '*.md' | head -220Repository: NVIDIA/NemoClaw
Length of output: 4316
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
import json
from pathlib import Path
root = Path("tools/mcp-tool-discovery-runtime")
manifest = json.loads((root / "package.json").read_text())
lock = json.loads((root / "package-lock.json").read_text())
for name, version in manifest["overrides"].items():
key = f"node_modules/{name}"
print(f"--- {name} ({version}) ---")
print(json.dumps(lock["packages"].get(key), indent=2, sort_keys=True))
PY
printf '%s\n' '--- all exact metadata references in the runtime review ---'
rg -n -S '`@hono/node-server`|Registry tarball|Integrity:|Node.js engine|engines' \
tools/mcp-tool-discovery-runtime/dependency-review.mdRepository: NVIDIA/NemoClaw
Length of output: 3712
Bind the dependency review to all four lock identities.
Add @hono/node-server artifact metadata to dependency-review.md. Assert each override’s exact version, resolved, integrity, and applicable engines, and compare those values with both the lock entries and the documented metadata.
🤖 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 `@test/mcp-tool-discovery-image-contract.test.ts` around lines 24 - 52, Update
the dependency-review contract around the manifest and lock assertions to cover
`@hono/node-server` as the fourth override. Add its exact version, resolved URL,
integrity, and applicable engines to dependency-review.md, then assert the same
metadata in its lock entry and documented review text alongside fast-uri, hono,
and ip-address.
ericksoa
left a comment
ericksoa
left a comment
There was a problem hiding this comment.
Reviewed the exact transition boundary at 4d2d570. The seven entries bind the current advisory/graph/package/version/severity tuples, expire on 2026-08-10, fail closed when unused or mismatched, and are explicitly removed by #8156. The focused audit-policy, dependency-review, and MCP image-contract suite passes 50/50. This provides the base-trusted bridge without authorizing an unreviewed lock or bypassing a failed check.
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
Commit 4d2d570 cannot pass the normal reviewed npm audit gate. Job 91841906209 failed because the workflow checks out both the audit implementation and ci/npm-audit-exceptions.json from base commit 3f3eb61, where the exception list is empty. The seven candidate entries are therefore not consumed; the job reports one, four, and two unaccepted high findings across the three graphs, and the aggregate check requires that job to pass. I will not use a failed-check merge or bypass. This needs a separately merged base-owned policy rollout or another reviewed mechanism that safely validates candidate exception data. Two review items also remain after that sequencing issue is solved:
|
|
The exact-head CI surfaced one in-scope test mismatch in shard 7. Please derive the helper default from the checked-in mcporter policy (expected IDs |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Maintainer bootstrap exception — commit
|
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Maintainer bootstrap exception — commit
|
Signed-off-by: J. Yaunches <jmyaunch@gmail.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
changed the title
<!-- 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>
5 participants
Summary
Keep the reviewed npm-audit provenance tests fail-closed after #8156 merged the clean root, OpenClaw, and mcporter dependency graphs.
Derive the expected mcporter audit status and advisory list from the parsed checked-in exception policy instead of hard-coding one policy state. Keep generic audit tests independent from that production file while separately asserting that the checked-in registry remains empty.
Related Issue
Post-remediation contract follow-up to #8156. The separate MCP discovery runtime remediation remains owned by #8177 and #8180.
Changes
ci/npm-audit-exceptions.jsoncontains no entries.mainand fix(security): refresh MCP runtime audit locks #8180 owns the MCP fix.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededgit diff --checkpasses.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every pushed commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — not applicable to this two-test-file contract hardening; required CI remains authoritative.npm run docsbuilds without warnings (doc changes only) — not applicable; no documentation changes remain in the effective diff.Additional evidence:
15069f9262d52b74d8916b7a0d7969a9ae4d3ee1.test/openclaw-integrity-pin-suite.tsandtest/reviewed-npm-audit.test.tsonly.ci/npm-audit-exceptions.json: schema version 1 with an emptyexceptionsarray.Signed-off-by: Julie Yaunches jyaunches@nvidia.com
Signed-off-by: Prekshi Vyas prekshiv@nvidia.com