fix(security): upgrade vulnerable production dependencies - #8126
Conversation
sandl99
added
the
area: security
📝 WalkthroughWalkthroughThe PR updates Undici from ChangesUndici dependency update
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-8126.docs.buildwithfern.com/nemoclaw |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
3 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. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
There was a problem hiding this comment.
Security review: FAIL for commit SHA 3ab72f3ddabb9806f9e76b77f8db261753cf0f84.
- Validate the complete production lock before npm can make a network request.
materializeSourceGraphchecks file type and the whole-lock digest, copies the PR-controlled lock, and then runsnpm ci. Registry-signature validation occurs later. A future PR can therefore make the trusted job fetch an arbitrary URL before trust is established.
Before invoking npm, require credential-free HTTPS, the configured registry origin, sha512 integrity, an exact package identity, and no file, Git, link, conflicting-identity, or unsafe nested source. Add a regression that proves npm is not invoked for a rejected lock. Verify installed package identities after installation.
- The required GitHub check does not execute the new trusted audit path. The workflow at base SHA
4cd4d64fechecks out and runs the audit driver from that base SHA. That driver has no root-production-graph path. Run 30810450136 therefore cannot provide evidence for the new path.
The preferred remediation is a prerequisite PR with the corrected verifier and tests. Merge it, then refresh #8126 so its base SHA owns and executes the verifier. A same-PR bootstrap requires an earlier verified commit containing the complete corrected verifier, an exact PR and repository guard, a capability check that selects it despite the old schema being available, and independent review.
- Retain or link the 16-range dependency concern and provenance ledger. Record each tag and SHA, release state, behavior and test change, license, runtime effect, and disposition.
brace-expansion@2.1.4has no npmgitHeador attestation. Its reviewed tarball integrity matches the lock and its five shipped files match tagv2.1.4; retain that manual comparison and the missing cryptographic producer binding.
The new policy-enforcement unit test is useful, but it does not close either trust-boundary finding. The PR body also has no independent sensitive-path review link. Secrets, authorization, cryptography, and configuration categories pass. Input validation, dependency security, security testing, and system security fail.
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Security review: changes requested for commit fb42b51.
Thank you for moving root-lock origin and integrity validation before npm access. The following blockers remain:
- After npm ci, the source-graph path does not call verifyInstalledNpmLock. A same-origin archive can still install a package identity that differs from the reviewed lock. Add post-install identity verification and a mismatch regression.
- Root-lock validation still permits hasShrinkwrap. Reject nested shrinkwrap delegation before npm access and prove the install callback is not invoked.
- The trusted audit driver still comes from the base SHA, so no required GitHub check for this commit exercises the proposed root-graph path. Land the base-owned audit support first, then refresh this PR.
- Preserve the audited per-tag and commit-SHA provenance ledger, including the brace-expansion 2.1.4 manual tag/file comparison and producer binding.
The PR currently records base SHA 4cd4d64; current main is c614c0d, so earlier check evidence also requires refresh.
|
To keep the user remediation independent from the new trusted audit-gate design, please split this PR as follows:
#8156 remains the managed-image/OpenClaw remediation and does not replace this root CLI fix. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Security review for commit : PASS for the dependency-only change.\n\n| Category | Result | Evidence |\n|---|---|---|\n| Input validation | PASS | The lock retains exact package versions, credential-free HTTPS registry URLs, and SHA-512 integrity values. |\n| Secrets | PASS | No credentials, tokens, or secret-bearing configuration changed. |\n| Authentication and authorization | PASS | No identity, permission, or access-control path changed. |\n| Cryptography | PASS | Registry integrity fields remain pinned; the installed production graph reports 130 verified registry signatures and 7 attestations. |\n| Dependency security | PASS | Installed identities resolve to brace-expansion 5.0.9 and 2.1.4, fast-uri 3.1.5, and js-yaml 4.3.1. # npm audit report
brace-expansion 2.0.0 - 2.1.3 || 3.0.0 - 5.0.8
Severity: high
brace-expansion: Large numeric range defeats documented max DoS protection - GHSA-jxxr-4gwj-5jf2
brace-expansion: DoS via exponential-time expansion of consecutive non-expanding {} groups - GHSA-3jxr-9vmj-r5cp
brace-expansion: DoS via exponential-time expansion of consecutive non-expanding {} groups - GHSA-3jxr-9vmj-r5cp
brace-expansion: DoS via unbounded expansion length causing an out-of-memory process crash - GHSA-mh99-v99m-4gvg
brace-expansion: DoS via unbounded expansion length causing an out-of-memory process crash - GHSA-mh99-v99m-4gvg
brace-expansion: DoS via unbounded intermediate arrays, bypassing the CVE-2026-14257 mitigation - GHSA-rgw5-rvv9-x895
brace-expansion: DoS via unbounded intermediate arrays, bypassing the CVE-2026-14257 mitigation - GHSA-rgw5-rvv9-x895
fix available via npm audit fix
node_modules/brace-expansion
node_modules/filelist/node_modules/brace-expansion
js-yaml 4.0.0 - 4.2.0
Severity: high
JS-YAML: Quadratic-complexity DoS in merge key handling via repeated aliases - GHSA-h67p-54hq-rp68
js-yaml: YAML merge-key chains can force quadratic CPU consumption - GHSA-52cp-r559-cp3m
fix available via npm audit fix
node_modules/js-yaml
2 high severity vulnerabilities
To address all issues, run:
npm audit fix reports zero vulnerabilities. |\n| Sensitive data | PASS | No logging, persistence, or data-handling path changed. |\n| Privilege boundaries | PASS | No process, filesystem-permission, container, or host boundary changed. |\n| Security testing | PASS | 96 focused manifest, policy, YAML, schema, and brace-expansion compatibility tests pass; repository checks and CLI build/typecheck pass. |\n| System security | PASS | The effective diff is limited to the production lockfile; the proposed trusted audit enforcement was removed for its separate prerequisite PR. |\n\nThe documentation writer classified this as because no command, configuration, default, output, workflow, policy schema, or support claim changes. The dependency provenance ledger remains in the PR description, including the recorded manual comparison for brace-expansion 2.1.4.
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Correction to my preceding review: shell quoting inserted unrelated local command output into that comment. Disregard the preceding review in full; this is the authoritative security assessment.
Security review for commit 06e4ae1: PASS for the dependency-only change.
| Category | Result | Evidence |
|---|---|---|
| Input validation | PASS | The lock retains specific package versions, credential-free HTTPS registry URLs, and SHA-512 integrity values. |
| Secrets | PASS | No credentials, tokens, or secret-bearing configuration changed. |
| Authentication and authorization | PASS | No identity, permission, or access-control path changed. |
| Cryptography | PASS | Registry integrity fields remain pinned; the installed production graph reports 130 verified registry signatures and 7 attestations. |
| Dependency security | PASS | Installed identities resolve to brace-expansion 5.0.9 and 2.1.4, fast-uri 3.1.5, and js-yaml 4.3.1. The production dependency audit reports zero vulnerabilities. |
| Sensitive data | PASS | No logging, persistence, or data-handling path changed. |
| Privilege boundaries | PASS | No process, filesystem-permission, container, or host boundary changed. |
| Security testing | PASS | 96 focused manifest, policy, YAML, schema, and brace-expansion compatibility tests pass; repository checks and CLI build/typecheck pass. |
| System security | PASS | The effective diff is limited to the production lockfile; the proposed trusted audit enforcement was removed for its separate prerequisite PR. |
The documentation writer classified this as no-docs-needed because no command, configuration, default, output, workflow, policy schema, or support claim changes. The dependency provenance ledger remains in the PR description, including the recorded manual comparison for brace-expansion 2.1.4.
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
Status: the scope split is complete at The current Planned sequence: merge #8156, refresh and land trusted audit prerequisite #8131, then refresh #8126 so the base-owned gate exercises the remediated root graph before security re-review. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Security review: PASS for revision 0bd5c09.
The production graph now resolves the affected root packages to brace-expansion 5.0.9 and 2.1.4, fast-uri 3.1.5, js-yaml 4.3.1, and undici 8.10.0. The direct undici manifest pin, trusted PR Review Advisor runtime pin, workflow boundary validator, and package contract agree. The patched undici engine requirement remains within NemoClaw's existing Node.js requirement. Registry origins and SHA-512 integrity remain locked, npm reports zero production vulnerabilities, and all 130 production packages have verified registry signatures; seven also have attestations.
No blocker was found across secrets, input validation, authorization, third-party provenance, logging, cryptography, configuration, testing, or system hardening. Repository checks, builds, type checks, hooks, and 33 focused trusted-workflow tests pass. The package-contract test requires the repository's configured package registry to complete its packed-install phase and remains for CI to validate. Documentation review found no user-facing update necessary.
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Close the high-severity npm audit gaps in the managed OpenClaw and mcporter production boundaries. This keeps the existing OpenClaw 2026.7.1 product scope and replaces only reviewed vulnerable dependency identities. PR #8126 is merged into this branch so San Dang and Apurv Kumaria retain their original root-remediation commits and authorship. ## Changes - Upgrade the OpenClaw locked runtime to `brace-expansion@5.0.9`, `fast-uri@3.1.5`, `undici@8.10.0`, and `ip-address@10.3.1` with exact registry URLs and SRI values. - Upgrade the mcporter lock to `fast-uri@3.1.5` and `ip-address@10.3.1` through exact manifest overrides. - Remediate Discord's direct and bundled `undici@8.5.0` across its manifest, shrinkwrap, `bundledDependencies`, and physical `node_modules` tree. - Fail closed when any reviewed source identity, dependency edge, engine, replacement archive, deterministic tree digest, or committed lock digest changes. - Update the security review records and regression contracts for the final advisory set and audit evidence. ## 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: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [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: nine-category review completed on the exact diff; dependency provenance warnings are recorded below and no code-security blocker was found. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: no exception is currently recorded; required CI is authoritative on exact head `1d28516eac51effbd690f8bf1847e59571ac5118`. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `agents/openclaw/dependency-review.md`, `docs/security/openclaw-2026.7.1-dependency-review.md`, and `docs/security/sandbox-base-2026-07-25-dependency-review.md`; their content is unchanged from the exact head where `npm run docs` passed. The final doctor-boundary test passed 1/1, the load-timed-out setup test passed 1/1 in isolation, and normal pre-commit and commit-msg hooks passed. The final test-only hardening requires no additional documentation. - Agent: Codex Desktop <!-- docs-review-head-sha: 1d28516 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## Dependency Review | Concern | Verdict | Evidence | |---|---|---| | `brace-expansion@5.0.9` | PASS | Exact registry archive, SRI, corrected Node engine, source range, and deterministic lock/tree boundaries verified. | | `fast-uri@3.1.5` | WARNING | Full source range and exact BSD-3-Clause no-dependency archive verified; registry signature is valid, but the archive has no registry attestation. | | `undici@8.10.0` | WARNING | Exact MIT no-dependency archive, signatures, SLSA provenance, and source ranges verified. Upstream release CI had one flaky Node 24 test; 1,502 tests and the Node 22, types, lint, fuzz, release, and Scorecard lanes passed. NemoClaw's exact runtime tests and audit provide compensating proof. | | `ip-address@10.3.1` | WARNING | Full non-shallow `v10.2.0..v10.3.1` range, exact MIT no-dependency archive, two registry signatures, SLSA provenance, content-identical source build, and 3,571 upstream tests verified. The annotated tag is unsigned. | | Trusted lock transition | PASS | PR #8165 merged the governance-approved transition into `main`. The OpenClaw lock `759b31779f40867f35f15065b582eb1d3efb8fddb1fe43c207507c905fa2a421` and mcporter lock `962dee34f6b0a493521d1619d1cf030e2630cbdfce8bf0598217202f57078793` are now each the sole trusted `lockSha256`; temporary successor fields were removed. | Licenses remain compatible: MIT, BSD-3-Clause, and the existing graph licenses. No replacement adds a runtime dependency or lifecycle script. ## Security Review | Category | Verdict | Evidence | |---|---|---| | Secrets and Credentials | PASS | No secret-bearing values or credential paths changed; gitleaks passed. | | Input Validation and Data Sanitization | PASS | Archive paths, package identities, dependency edges, engines, bundled trees, deterministic tree hashes, and lock hashes reject drift. | | Authentication and Authorization | PASS | No authentication or authorization behavior changed. | | Dependencies and Third-Party Libraries | WARNING | Every affected production graph is outside the disclosed high-severity ranges; upstream signing and release-CI limitations are recorded in the concern ledger. | | Error Handling and Logging | PASS | Failures identify the exact source, replacement, tree, or lock boundary without exposing secrets. | | Cryptography and Data Protection | PASS | SHA-512 artifact integrity and SHA-256 lock binding remain mandatory; registry signatures and available provenance were verified. | | Configuration and Security Headers | PASS | Docker guards and reviewed-audit policy use the exact new locks; users, permissions, ports, and headers are unchanged. | | Security Testing | PASS | Source-shape rejection, physical bundled-tree replacement, deterministic tree integrity, installed-lock verification, registry audit, and signature checks passed. | | System Security | PASS | Managed images remove the high findings without weakening sandbox isolation or image hardening. | ## 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 pushed 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 — 105/105 exact-head focused integration tests passed after merging #8165; 57/57 dependency and audit-documentation tests passed again after the final remote/main reconciliation. The package-contract probe also passed after increasing its Mac-local npm pack timeout from 90 to 120 seconds. - [ ] 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 reviewed dependency and integrity-boundary update; 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) — 0 errors and 2 existing hidden Fern warnings. - [x] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional validation: - Exact Node 22.23.1 reviewed npm audit exited 0: - reviewed archive graph: 0 high, 0 critical, 13 moderate; - OpenClaw locked graph: 0 high, 0 critical, 7 moderate; - mcporter locked graph: 0 high, 0 critical, 1 moderate. - Registry signature checks completed for both locked graphs; mcporter reports 120/120 signed packages and 14 attestations. - `npm run build:cli`, `npm run typecheck:cli`, `npm run checks:repository`, focused documentation tests, and `npm run docs` passed. - Normal pre-commit and commit-message hooks passed, including Biome, hadolint, gitleaks, markdownlint, repository checks, and source/test budgets. - Live E2E was not run locally. Required image-build CI provides the platform-specific proof. - PR #8165 merged the base-trusted audit transition. This head removes the temporary successor hashes and retains each remediated lock as the sole trusted `lockSha256`. --- 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 bundled and runtime dependencies to patched versions, including `brace-expansion`, `fast-uri`, `undici`, and `ip-address`. * Expanded protection for additional runtime and Discord-related dependencies. * Refreshed vulnerability reviews, integrity metadata, provenance records, and audit results. * **Bug Fixes** * Improved dependency validation and fail-closed checks for affected package versions. * **Tests** * Expanded coverage for patched packages, lockfile integrity, remediation behavior, and supply-chain validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> Co-authored-by: San Dang <sdang@nvidia.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com>
|
Closed as superseded by #8156. The merged change includes the same production-dependency remediation and preserves San Dang’s contribution through the integrated commits and squash attribution. Comparing all six #8126 files with main at 15069f9 shows five identical files; the remaining test keeps the same undici assertion and increases the package-build timeout from 90 to 120 seconds. No work remains to transfer. |
5 participants
Summary
Upgrade root production dependencies affected by current high-severity advisories. The change remains dependency-focused; trusted root-audit enforcement stays separately scoped in PR #8131.
Related Issue
Fixes #8116
Changes
brace-expansion5.0.9 and 2.1.4,fast-uri3.1.5,js-yaml4.3.1, andundici8.10.0.undicimanifest pin to 8.10.0; its Node.js requirement remains within NemoClaw's existing runtime requirement.undici8.10.0.Type of Change
Quality Gates
main. Secrets, validation, authentication, authorization, error handling, cryptography, configuration, security testing, and system security pass; the dependency provenance limitations are recorded below.Documentation Writer Review
no-docs-neededundici@8.10.0retains the>=22.19.0Node.js requirement already documented by NemoClaw.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every published commit appears asVerifiedin GitHubnpm audit --omit=devreports zero vulnerabilitiesbrace-expansion5.0.9 and 2.1.4,fast-uri3.1.5,js-yaml4.3.1, andundici8.10.0npm run checks:repository, CLI build, CLI type check, and diff checks passnpm run docsbuilds without warnings (doc changes only)The packed CLI contract was not counted as local passing evidence because a live AWS SDK publication wave temporarily exposed successive not-yet-propagated transitive versions during its fresh omit-dev install. This is unrelated to the reviewed diff; required CI remains authoritative.
Dependency provenance
brace-expansion2.1.4 lacks an npm producer attestation; the recorded manual comparison found its reviewed tarball integrity matches the lock and its five shipped files match release tagv2.1.4.Signed-off-by: San Dang sdang@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Signed-off-by: Prekshi Vyas prekshiv@nvidia.com