fix(security): backport libssh2 and Python fixes - #7679
Merged
Conversation
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
senthilr-nv
added
area: packaging
Contributor
|
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:
📝 WalkthroughWalkthroughThis change adds deterministic native security packages for libssh2 and Python 3.13 HTMLParser, integrates them into managed Docker images, extends image verification and inventories, and updates tests, fixtures, workflow triggers, security review documentation, and CI calibration. ChangesNative security package remediation
CI performance calibration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Builder as native-security-builder
participant BaseImage as Dockerfile.base
participant Inventory as security inventory
participant Runtime as final image runtime checks
Builder->>BaseImage: build libssh2 and HTMLParser .deb artifacts
BaseImage->>BaseImage: install and validate native packages
BaseImage->>Inventory: record package versions
BaseImage->>Runtime: run parser and libssh2 integrity checks
Runtime-->>BaseImage: pass or fail image build
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
23 tasks
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-7679.docs.buildwithfern.com/nemoclaw |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 8ee3753 in the TypeScript / code-coverage/cliThe overall coverage in commit 8ee3753 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
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/helpers/base-apt-security-functions.ts (1)
130-148: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict the one-file checksum path to
parser.py.
count == 1currently accepts a single checksum for any of the six Debian packages too. A Dockerfile that still downloads all packages but verifies only one archive would pass this mock. Track whether the matched record ispython3.13/html/parser.py, then allow one record only for that parser verification.🤖 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/helpers/base-apt-security-functions.ts` around lines 130 - 148, Update the mocked sha256sum function to track whether the matched record is the python3.13/html/parser.py entry, rather than allowing count == 1 for any package. Keep count == 6 valid for the six Debian packages, and permit count == 1 only when the parser.py record was matched.
🧹 Nitpick comments (1)
Dockerfile.base (1)
64-79: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftNative libssh2/Python package build is duplicated (and re-tested) once per image variant. All three Dockerfile.base files independently run the identical
native-security-builderstage — same apt packages, samebuild-native-security-packages.shinvocation, same full libssh2 upstreammake checksuite — to produce the exact same.debartifacts per architecture. This triples CI build time/compute for artifacts that don't vary by image.
Dockerfile.base#L64-L79: build/test libssh2 once (e.g. in a shared builder image published to the registry, or a single job inbase-image.yamlthat all three Dockerfiles pull the resulting.debs from viaCOPY --from=a shared image reference) instead of rebuilding here.agents/hermes/Dockerfile.base#L24-L39: consume the shared native-security artifact instead of re-running the builder stage.agents/langchain-deepagents-code/Dockerfile.base#L12-L27: consume the shared native-security artifact instead of re-running the builder stage.🤖 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 `@Dockerfile.base` around lines 64 - 79, Replace the duplicated native-security builder invoked by the Dockerfile.base RUN step with one shared artifact-producing build in base-image.yaml or a published shared builder image, preserving the required libssh2/Python package outputs. Update Dockerfile.base lines 64-79, agents/hermes/Dockerfile.base lines 24-39, and agents/langchain-deepagents-code/Dockerfile.base lines 12-27 to consume those shared .deb artifacts instead of rerunning build-native-security-packages.sh and its tests.
🤖 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 `@agents/hermes/Dockerfile.base`:
- Around line 22-44: Use the same pinned Node.js Trixie base image digest for
both the native-security-builder stage and the final stage in Dockerfile.base.
Update the builder’s FROM declaration to match the final stage, preserving the
existing package installation, native build, and COPY flow.
In `@Dockerfile`:
- Around line 1758-1759: Replace every assert-based validation in Dockerfile
lines 1758-1759, agents/hermes/Dockerfile lines 784-785, and
agents/langchain-deepagents-code/Dockerfile lines 259-260 with explicit
conditional failures that remain active under Python optimization. Preserve each
check’s existing validation logic and make failures terminate the Docker build
with a clear error.
In `@scripts/security/build-native-security-packages.sh`:
- Around line 111-117: Update the download URLs for both the libssh2 package
flow and the libpython3.13-stdlib flow in the build script to use the
repository’s existing snapshot.debian.org URL pattern instead of deb.debian.org.
Preserve the current version, architecture, checksum verification, extraction,
and package metadata checks.
---
Outside diff comments:
In `@test/helpers/base-apt-security-functions.ts`:
- Around line 130-148: Update the mocked sha256sum function to track whether the
matched record is the python3.13/html/parser.py entry, rather than allowing
count == 1 for any package. Keep count == 6 valid for the six Debian packages,
and permit count == 1 only when the parser.py record was matched.
---
Nitpick comments:
In `@Dockerfile.base`:
- Around line 64-79: Replace the duplicated native-security builder invoked by
the Dockerfile.base RUN step with one shared artifact-producing build in
base-image.yaml or a published shared builder image, preserving the required
libssh2/Python package outputs. Update Dockerfile.base lines 64-79,
agents/hermes/Dockerfile.base lines 24-39, and
agents/langchain-deepagents-code/Dockerfile.base lines 12-27 to consume those
shared .deb artifacts instead of rerunning build-native-security-packages.sh and
its tests.
🪄 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: 4f6d4d4a-6f2f-4e70-98e5-deb1648627a6
📒 Files selected for processing (23)
.github/workflows/base-image.yamlDockerfileDockerfile.baseagents/hermes/Dockerfileagents/hermes/Dockerfile.baseagents/langchain-deepagents-code/Dockerfileagents/langchain-deepagents-code/Dockerfile.basedocs/security/sandbox-base-2026-07-27-native-package-review.mdscripts/security/build-native-security-packages.shscripts/security/patches/libssh2-1.11.1-cve-2026.patchscripts/security/patches/python3.13-htmlparser-cve-2026-15308.patchsrc/lib/onboard/base-image.tssrc/lib/onboard/dockerfile-remote-dashboard-bind-contract.tstest/fixtures/security/python3.13-html-parser-fixed.txttest/helpers/base-apt-security-functions.tstest/helpers/onboard-script-mocks.cjstest/helpers/python-parser-security-fixture.tstest/hermes-share-mount-deps.test.tstest/native-security-packages.test.tstest/perl-critical-cve-remediation.test.tstest/sandbox-base-runtime-tools.test.tstest/sandbox-base-security-packages.test.tstest/sandbox-provisioning.test.ts
💤 Files with no reviewable changes (1)
- test/sandbox-provisioning.test.ts
apurvvkumaria
approved these changes
Jul 28, 2026
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Collaborator
Author
|
@coderabbitai ACK — the four actionable findings were valid and are addressed in
The libssh2 harness now executes all 25 top-level cases against a local OpenSSH fixture, including all 18 algorithm subcases, and fails on nested TAP errors. The exact Hermes arm64 builder passed and produced both packages; six focused integration files passed 100 tests, documentation passed, and all commit/push hooks passed. The shared-artifact suggestion is a useful performance optimization but is intentionally separate: implementing a published shared builder would add artifact ownership and lifecycle decisions beyond this narrowly scoped security remediation. |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
I agree that publishing a shared builder is a separate lifecycle/ownership design decision and should remain out of scope for this targeted security backport. 🐇✅ ✏️ Learnings added
|
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 `@test/native-security-packages.test.ts`:
- Around line 118-127: Update the assertions in the native security harness test
to compare the complete 25-element argumentsList in exact order, rather than
only its length and endpoints. Also compare the trimmed environment output
against the exact expected SSH configuration line, including
PerSourcePenalties=no and all required options.
🪄 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: b93507f6-fc02-4c5e-86b0-9a327d473ad7
📒 Files selected for processing (12)
DockerfileDockerfile.baseagents/hermes/Dockerfileagents/hermes/Dockerfile.baseagents/langchain-deepagents-code/Dockerfileagents/langchain-deepagents-code/Dockerfile.basedocs/security/sandbox-base-2026-07-27-native-package-review.mdscripts/security/build-native-security-packages.shtest/helpers/base-apt-security-functions.tstest/helpers/python-parser-security-fixture.tstest/hermes-share-mount-deps.test.tstest/native-security-packages.test.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- test/helpers/python-parser-security-fixture.ts
- agents/langchain-deepagents-code/Dockerfile
- agents/hermes/Dockerfile.base
- scripts/security/build-native-security-packages.sh
- docs/security/sandbox-base-2026-07-27-native-package-review.md
- Dockerfile.base
- agents/langchain-deepagents-code/Dockerfile.base
- Dockerfile
- test/helpers/base-apt-security-functions.ts
- agents/hermes/Dockerfile
- test/hermes-share-mount-deps.test.ts
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+69
to
+99
| const result = spawnSync( | ||
| "bash", | ||
| [ | ||
| "-c", | ||
| [ | ||
| "set -euo pipefail", | ||
| 'source "$1"', | ||
| 'calls="$HARNESS_LOG/calls"', | ||
| 'mapfile() { local target="$2" line; eval "$target=()"; while IFS= read -r line; do eval "$target+=(\\"\\$line\\")"; done; }', | ||
| 'chmod() { printf "chmod %s\\n" "$*" >>"$calls"; }', | ||
| "id() { return 0; }", | ||
| 'useradd() { printf "useradd %s\\n" "$*" >>"$calls"; }', | ||
| 'chpasswd() { cat >/dev/null; printf "chpasswd\\n" >>"$calls"; }', | ||
| 'install() { printf "install %s\\n" "$*" >>"$calls"; }', | ||
| 'sed() { printf "sed %s\\n" "$*" >>"$calls"; }', | ||
| 'make() { printf "make %s\\n" "$*" >>"$calls"; }', | ||
| 'run_libssh2_tests "$2"', | ||
| ].join("\n"), | ||
| "libssh2-harness", | ||
| BUILD_SCRIPT, | ||
| fixture, | ||
| ], | ||
| { | ||
| encoding: "utf-8", | ||
| env: { | ||
| ...process.env, | ||
| HARNESS_LOG: harnessLog, | ||
| NESTED_FAILURE: nestedFailure ? "1" : "0", | ||
| }, | ||
| }, | ||
| ); |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
apurvvkumaria
enabled auto-merge (squash)
July 28, 2026 04:33
…2-security-remediation
…2-security-remediation
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
…2-security-remediation Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Collaborator
Author
|
ACK PRA-1 — valid. The current calibration records the exact local-build trigger at adjustment scope, but it does not bind each allowance sample to trigger provenance or enforce a recalibration/removal condition. I will add those bounded evidence fields and contract assertions without changing the measured allowance. |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
…2-security-remediation Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Collaborator
Author
|
PRA-1 is addressed in |
cv
pushed a commit
that referenced
this pull request
Jul 28, 2026
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical dated changelog entry for NemoClaw v0.0.97 before the release plan captures `origin/main`. The entry groups the user-visible and maintainer-facing changes since v0.0.96 while preserving the Deferred dual-Station status, experimental runtime-identity boundary, and pending physical IGX validation. ## Changes - Add `docs/changelog/2026-07-28.mdx` with the parser-safe MDX SPDX comment and exact `## v0.0.97` heading. - Summarize the 43 merged PRs in the release range, omitting internal-only changes from the public entry and linking each grouped change to its most specific published documentation. - Keep the experimental Okta reference explicitly opt-in and outside normal onboarding, keep the two-Station path Deferred, and state that physical IGX Orin validation remains pending. ### Source summary - [#7440](#7440), [#7443](#7443), and [#7445](#7445) -> `docs/changelog/2026-07-28.mdx`: Document read-only host readiness reports and fail-closed platform qualification. - [#7030](#7030) -> `docs/changelog/2026-07-28.mdx`: Document the Deferred trusted two-Station vLLM evaluation. - [#7265](#7265) -> `docs/changelog/2026-07-28.mdx`: Document the bounded experimental direct-runner Okta runtime-identity reference. - [#7711](#7711) and [#7648](#7648) -> `docs/changelog/2026-07-28.mdx`: Document compatible-endpoint reasoning effort and retired NVIDIA Build model paths. - [#7746](#7746), [#7763](#7763), and [#7681](#7681) -> `docs/changelog/2026-07-28.mdx`: Document safe compatible-provider creation, replacement refusal, and narrow OpenShell bridge URL handling. - [#7641](#7641), [#7690](#7690), [#7631](#7631), and [#7710](#7710) -> `docs/changelog/2026-07-28.mdx`: Document paused-container recovery, recreation journaling, pre-mutation uninstall checks, and source-checkout OpenShell selection. - [#7624](#7624) and [#7762](#7762) -> `docs/changelog/2026-07-28.mdx`: Document Jetson release diagnostics and bounded render-device group propagation. - [#7639](#7639), [#7760](#7760), [#7721](#7721), and [#7761](#7761) -> `docs/changelog/2026-07-28.mdx`: Document Telegram, MCP media-type, Hermes image-mode, and locked-restart fixes. - [#7653](#7653) and [#7680](#7680) -> `docs/changelog/2026-07-28.mdx`: Document Deep Agents policy tasks and the bounded Claude Code OAuth path. - [#7679](#7679) -> `docs/changelog/2026-07-28.mdx`: Document the checksum-bound libssh2 and Python HTMLParser backports. - [#7655](#7655), [#7651](#7651), [#7664](#7664), [#7666](#7666), [#7670](#7670), [#7719](#7719), and [#7741](#7741) -> `docs/changelog/2026-07-28.mdx`: Document exact candidate E2E evidence, Launchable selection, diagnostic consolidation, and trusted WSL validation. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog contract, MDX header, heading uniqueness, and release-entry structure. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] 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: The committed `docs/changelog/2026-07-28.mdx` blob exactly matches the reviewed file. Completeness, factual accuracy, link shape, parser-safe MDX header, one-sentence-per-line style, `.docs-skip` compliance, and bounded product claims passed. - Agent: Codex Desktop documentation writer subagent <!-- docs-review-head-sha: da6aa27 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this PR changes only the dated changelog. - 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 check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` passed 6/6. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable to this doc-only release entry. - [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) — completed with 0 errors and 2 pre-existing Fern warnings. - [x] 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) — native changelog entries use the required parser-safe MDX SPDX comment and intentionally have no frontmatter. --- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added improved host readiness reporting and Jetson onboarding guidance. * Added controls for reasoning effort with compatible endpoints and enhanced managed MCP discovery. * Improved Deep Agents task publication and preset support. * **Bug Fixes** * Hardened provider switching, sandbox recovery, uninstall behavior, and Telegram connectivity. * Improved container image integrity checks, media-type handling, and checksum validation. * Enhanced vLLM evaluation behavior and release diagnostics. * **Documentation** * Added the NemoClaw v0.0.97 changelog. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
4 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
The managed sandbox base images now install checksum-bound native packages that backport reviewed upstream fixes for four libssh2 memory-safety vulnerabilities and Python HTMLParser's incremental parsing complexity vulnerability.
The change preserves the supported Debian runtime identities, fails closed when reviewed source inputs drift, and verifies the fixed packages in both the base and completed images.
This internal PR replaces #7667 and includes the validated review follow-up fixes.
Changes
libssh2.so.1soname, and reject any build that removes an exported Debian runtime symbol.html/parser.py, bound to the exact Debian standard-library version and fixed file hash.dpkg --auditchecks in the completed images.Type of Change
Quality Gates
8ee37535cbf19934fd6a26c8659e31f59dc7a36a; all categories passed with no findings. The refresh range-diff confirms every reviewed PR commit is unchanged against base8d3e728b9. The calibration evidence follow-up changes only trusted CI records and contract tests; it adds no runtime, input, network, credential, authorization, privilege, dependency, or secret surface. Published-image budgets and all functional and security assertions remain enforced. The builder accepts no user-controlled network or package identity input, verifies every downloaded artifact before use, and preserves the existing container privilege boundary. MCP lifecycle behavior remains unchanged.Documentation Writer Review
docs-updatedDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubnpm run check:diffpassed when hooks were skipped or unavailable8ee37535c, 14 calibration contract assertions passed; exact refresh range-diff and diff checks passed. Vitest project membership and source-shape gates passed. Normal JSON, repository, secret, formatting, test-size, commit, and push hooks passed. The prior head passed the MCP regression, CLI build and typecheck, all eight CLI shards, the aggregate CLI gate, amd64 and arm64 image builds, and all four self-hosted image E2E jobs. Earlier validation recorded 128 focused assertions across performance calibration and E2E workflow contracts, plus 131 focused assertions across native packages, completed-image security, Hermes dependencies, dashboard lifecycle, and provisioning contracts.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Native amd64 and arm64 builds for all three managed images remain enforced by current-head CI.npm run docsbuilds without warnings (doc changes only) — completed with 0 errors and 2 existing warnings.Signed-off-by: Senthil Ravichandran senthilr@nvidia.com
Summary by CodeRabbit
libssh2andhtml.parserchecks).