fix(security): backport libssh2 and Python fixes - #7667
Conversation
|
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 patched libssh2 and Python HTMLParser Debian packages, builds them in dedicated Docker stages, installs them into base images, verifies package and runtime integrity, updates security inventories, and expands tests and workflow triggers. ChangesNative Security Remediation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant NativeBuilder
participant BaseImage
participant RuntimeVerification
NativeBuilder->>BaseImage: Produce and copy native security .deb artifacts
BaseImage->>BaseImage: Install patched packages
BaseImage->>RuntimeVerification: Run package, checksum, parser, and libssh2 checks
RuntimeVerification->>BaseImage: Write verified security inventory
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
senthilr-nv
added
bug-fix
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence 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: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
ae266f3 to
41fc369
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/native-security-packages.test.ts (1)
37-70: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftBind these security tests to executable package-install behavior.
The current assertions accept text appearing anywhere in a Dockerfile stage, including comments or unrelated commands. They do not prove the patched packages build, install, or remain arguments of the relevant
apt-get installinvocation.
test/native-security-packages.test.ts#L37-L70: supplement literaltoContainchecks with a package-contract test that builds or executes the native-package stage and validates produced package metadata and patched runtime behavior.test/perl-critical-cve-remediation.test.ts#L134-L136: extract the apt-install command and assert both Perl package paths are actual arguments to that command.🤖 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/native-security-packages.test.ts` around lines 37 - 70, The native security tests must validate executable package installation rather than matching arbitrary Dockerfile text. In test/native-security-packages.test.ts:37-70, supplement the assertions with a package-contract test that builds or executes the native-security-builder stage, verifies produced package metadata and installation, and confirms patched runtime behavior. In test/perl-critical-cve-remediation.test.ts:134-136, extract the relevant apt-get install command and assert both Perl package paths are actual arguments to that command.Source: Path instructions
scripts/security/build-native-security-packages.sh (1)
185-201: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winVerify the patched
parser.pyhash before packaging, not just downstream.
verify_sha256checks the unmodified input at line 178, but after applying the patch there's no check that the resulting file matches the documented "fixed identity" hash (33a7eeead8...in the security review doc and all three Dockerfiles). A build-time regression here would only surface later, in the image's ownsha256sum -ccheck — moving that verification into this script gives a fail-fast signal exactly where the patch is applied.♻️ Proposed fix
readonly PYTHON_PARSER_SHA256="f91ec3de6331206bbe2ec3e54a05f646bd23d3c61a18d4a01b25164e070bacc9" +readonly PYTHON_PARSER_FIXED_SHA256="33a7eeead8d1ccb04efd282502b766e44c36cca17bbb44d9e6fa3911fd8f226f" @@ git -C "${package_root}" apply \ "${patch_dir}/python3.13-htmlparser-cve-2026-15308.patch" + verify_sha256 "${PYTHON_PARSER_FIXED_SHA256}" "${package_root}/${parser_path}"🤖 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 `@scripts/security/build-native-security-packages.sh` around lines 185 - 201, After applying the patch in the Python 3.13 HTMLParser packaging flow, use verify_sha256 on the resulting parser.py and the documented fixed identity hash 33a7eeead8... before generating the DEBIAN control file. Keep the existing pre-patch verification and fail the build immediately if the post-patch hash does not match.Dockerfile.base (1)
88-89: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMove the artifact copy later to let the two long builds run in parallel.
COPY --from=native-security-builderat line 88 sits before perl-builder's own independentapt-get install/curl/./Configure/make/make test_harnesssequence (90-150). Since that RUN chain doesn't actually need the copied.debs until the finaldpkg-deb --buildstep (181-184), this ordering forces BuildKit to serialize native-security-builder's libssh2 build+test behind (or ahead of) the equally expensive Perl build+test, instead of running them concurrently.♻️ Proposed fix
ARG PERL_VERSION ARG PERL_SHA256 ARG PERL_PACKAGE_REVISION -COPY --from=native-security-builder /out /tmp/security-packages - RUN apt-get update && apt-get install -y --no-install-recommends \ build-essential=12.12 \ ca-certificates=20250419 \ curl=8.14.1-2+deb13u4 \ netbase=6.5 \ xz-utils=5.8.1-1+deb13u1 \ && rm -rf /var/lib/apt/lists/* @@ && make install DESTDIR=/tmp/perl-root +COPY --from=native-security-builder /out /tmp/security-packages + RUN package_version="${PERL_VERSION}-${PERL_PACKAGE_REVISION}" \🤖 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 88 - 89, Move the COPY from native-security-builder that populates /tmp/security-packages to immediately before the final dpkg-deb --build step, after the independent perl-builder install, Configure, make, and make test_harness sequence. Keep the copied artifacts available for the packaging commands while allowing both long builder stages to execute concurrently.
🤖 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.
Nitpick comments:
In `@Dockerfile.base`:
- Around line 88-89: Move the COPY from native-security-builder that populates
/tmp/security-packages to immediately before the final dpkg-deb --build step,
after the independent perl-builder install, Configure, make, and make
test_harness sequence. Keep the copied artifacts available for the packaging
commands while allowing both long builder stages to execute concurrently.
In `@scripts/security/build-native-security-packages.sh`:
- Around line 185-201: After applying the patch in the Python 3.13 HTMLParser
packaging flow, use verify_sha256 on the resulting parser.py and the documented
fixed identity hash 33a7eeead8... before generating the DEBIAN control file.
Keep the existing pre-patch verification and fail the build immediately if the
post-patch hash does not match.
In `@test/native-security-packages.test.ts`:
- Around line 37-70: The native security tests must validate executable package
installation rather than matching arbitrary Dockerfile text. In
test/native-security-packages.test.ts:37-70, supplement the assertions with a
package-contract test that builds or executes the native-security-builder stage,
verifies produced package metadata and installation, and confirms patched
runtime behavior. In test/perl-critical-cve-remediation.test.ts:134-136, extract
the relevant apt-get install command and assert both Perl package paths are
actual arguments to that command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 46a99d20-8509-4247-8023-eb4a7cab14a1
📒 Files selected for processing (18)
.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.tstest/helpers/base-apt-security-functions.tstest/helpers/onboard-script-mocks.cjstest/hermes-share-mount-deps.test.tstest/native-security-packages.test.tstest/perl-critical-cve-remediation.test.tstest/sandbox-base-security-packages.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/native-security-packages.test.ts (1)
58-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep this test behavior-oriented.
This test only reads Dockerfiles and checks independent substrings; it does not install packages or execute image verification. Rename it as a structural wiring check, or make it drive an executable package-contract test so a stale/commented/wrong-stage command cannot satisfy the assertions.
As per path instructions, tests should favor observable outcomes over source-text implementation lock-in.
🤖 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/native-security-packages.test.ts` around lines 58 - 72, The test named “installs and proves both native packages” only performs Dockerfile text matching and does not install or verify packages. Rename it and its assertions to describe a structural Dockerfile wiring check, or replace the substring-only checks with an executable package-contract test that validates the built packages and rejects stale, commented, or wrong-stage commands.Source: Path instructions
🤖 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/perl-critical-cve-remediation.test.ts`:
- Around line 134-136: Update the runtime assertions in the Perl remediation
test to verify one bounded apt-get install command contains both
/tmp/nemoclaw-native-security/perl-base.deb and
/tmp/nemoclaw-native-security/perl.deb paths, rather than checking the package
paths independently. Preserve the existing install flags and ensure the
assertion cannot pass when the packages are only copied.
In `@test/sandbox-base-security-packages.test.ts`:
- Around line 70-77: Preserve distinct Debian and native artifact roots in both
test helpers: in test/sandbox-base-security-packages.test.ts lines 70-77, create
separate fixture directories and map /tmp/nemoclaw-debian-security and
/tmp/nemoclaw-native-security independently; apply the same separation to the
Hermes apt-layer command in test/hermes-share-mount-deps.test.ts lines 215-221.
Do not use one shared directory for either test.
---
Nitpick comments:
In `@test/native-security-packages.test.ts`:
- Around line 58-72: The test named “installs and proves both native packages”
only performs Dockerfile text matching and does not install or verify packages.
Rename it and its assertions to describe a structural Dockerfile wiring check,
or replace the substring-only checks with an executable package-contract test
that validates the built packages and rejects stale, commented, or wrong-stage
commands.
🪄 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: 0435b3ed-aa86-4ff4-b3c3-9ffe4d0f102a
📒 Files selected for processing (18)
.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.tstest/helpers/base-apt-security-functions.tstest/helpers/onboard-script-mocks.cjstest/hermes-share-mount-deps.test.tstest/native-security-packages.test.tstest/perl-critical-cve-remediation.test.tstest/sandbox-base-security-packages.test.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- .github/workflows/base-image.yaml
- agents/hermes/Dockerfile
- docs/security/sandbox-base-2026-07-27-native-package-review.md
- Dockerfile
- agents/hermes/Dockerfile.base
- scripts/security/patches/python3.13-htmlparser-cve-2026-15308.patch
- Dockerfile.base
- test/helpers/base-apt-security-functions.ts
- test/helpers/onboard-script-mocks.cjs
- scripts/security/build-native-security-packages.sh
- agents/langchain-deepagents-code/Dockerfile
- agents/langchain-deepagents-code/Dockerfile.base
41fc369 to
514615d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/native-security-packages.test.ts (1)
37-56: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftSupplement source-text checks with boundary behavior validation.
These assertions only prove that hashes, snippets, and package strings occur somewhere in source files. They can pass while a patch is unapplied or while installation/runtime checks exist only in a builder stage, comment, or other non-final stage. Keep the lightweight guards if useful, but add or reference execution-level tests for the built package/final image, or narrow the test names to static contract checks.
As per path instructions, tests should prefer observable outcomes through the public boundary over source-text assertions.
Also applies to: 58-73
🤖 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/native-security-packages.test.ts` around lines 37 - 56, Supplement the static assertions in the native security package tests with execution-level validation through the public package or final-image boundary, covering the patched libssh2 and Python behaviors. Ensure the checks verify observable runtime outcomes in the built artifact rather than only source text or intermediate builder stages; otherwise rename the tests to explicitly identify them as static contract checks.Source: Path instructions
🤖 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 `@scripts/security/patches/python3.13-htmlparser-cve-2026-15308.patch`:
- Around line 30-32: Update the pending-data handling around _pending_len and
_pending so zero-length data is treated as a no-op before appending. Skip empty
chunks while preserving the existing length accounting and threshold behavior
for non-empty input.
In `@test/sandbox-base-security-packages.test.ts`:
- Around line 54-77: Replace the placeholder fixedParser contents with the real
patched parser.py fixture bytes and configure fakePython3/fakePythonLink to run
a real interpreter that imports that fixture, rather than a no-op shell script;
update test/sandbox-base-security-packages.test.ts lines 54-77 accordingly. At
lines 102-113, reuse the same verified parser fixture via Python’s import path
so the HTMLParser behavior assertion executes against the SHA-256-validated
file, not merely a substituted checksum path.
---
Nitpick comments:
In `@test/native-security-packages.test.ts`:
- Around line 37-56: Supplement the static assertions in the native security
package tests with execution-level validation through the public package or
final-image boundary, covering the patched libssh2 and Python behaviors. Ensure
the checks verify observable runtime outcomes in the built artifact rather than
only source text or intermediate builder stages; otherwise rename the tests to
explicitly identify them as static contract checks.
🪄 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: d7e92fe2-c72c-424b-86b4-8fc2401f7fe8
📒 Files selected for processing (18)
.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.tstest/helpers/base-apt-security-functions.tstest/helpers/onboard-script-mocks.cjstest/hermes-share-mount-deps.test.tstest/native-security-packages.test.tstest/perl-critical-cve-remediation.test.tstest/sandbox-base-security-packages.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/lib/onboard/base-image.ts
- docs/security/sandbox-base-2026-07-27-native-package-review.md
- test/perl-critical-cve-remediation.test.ts
- agents/langchain-deepagents-code/Dockerfile.base
- agents/hermes/Dockerfile.base
- Dockerfile
- test/helpers/base-apt-security-functions.ts
- scripts/security/patches/libssh2-1.11.1-cve-2026.patch
- scripts/security/build-native-security-packages.sh
- agents/hermes/Dockerfile
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Requesting changes on exact head 514615d64e7e0de7f82a58469e6d677bf7bd2b59.
The security remediation is directionally sound, but the current head has blocking correctness and validation gaps:
scripts/security/patches/python3.13-htmlparser-cve-2026-15308.patchappends everyfeed("")call to_pendingwhile_pending_lenremains zero. Repeated empty feeds can therefore grow the list without bound. Treat empty input as a no-op, add a regression, and refresh every derived patched-parser hash/version receipt.test/sandbox-base-security-packages.test.tssubstitutes placeholder parser bytes and a no-op Python executable. The claimed checksum and HTMLParser behavior are not exercised against the same patched artifact. Use the real patched fixture and a real interpreter/import path.- The sandbox and Hermes fixtures map
/tmp/nemoclaw-debian-securityand/tmp/nemoclaw-native-securityto one directory. Preserve distinct roots so wrong-path and wrong-glob wiring fails. - The Perl package checks only find package paths independently; assert that one bounded
apt-get installcommand actually contains both native package paths. - The current directory-level
COPY scripts/securitywiring is breaking exact-head CLI contracts: shard 8 rejects the non-literal base-image publication path and the old-base context copier receives a directory (EISDIR); shard 2 also reports stale Dockerfile/dashboard-context assumptions. Copy/list the three required security files explicitly in each base Dockerfile/workflow context, then update the affected fixtures and rerun shards 2, 7, and 8.
The current WSL failure is unrelated—the shared deb.nodesource.com/setup_22.x request receives HTTP 403 before tests—and the Terra Advisor lane stopped in analysis infrastructure. Those two failures are not attributed to this PR’s code, but the three CLI shard failures and findings above are.
Please refresh the exact-head security and documentation receipts after the fixes. I did not modify this Senthil-owned branch.
|
Follow-up detail for item 5 on exact head
Focused local repro on this exact head: dashboard lifecycle 1/34 failed, sandbox provisioning 2/62 failed, and dcode workflow 1/4 failed. |
|
ACK — I reviewed the findings on exact head |
7823c2b to
214ded5
Compare
|
Addressed the acknowledged review findings on exact head
Validation on the final conflict-free rebase: 7 focused files / 77 tests passed; documentation validation completed with 0 errors and 2 existing warnings; exact upstream-range pre-commit, commitlint, and pre-push gates passed, including gitleaks, shellcheck, hadolint, source-shape, test-size, and CLI type-checking. Both rebased commits are GitHub Verified. Ready for re-review. |
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 `@agents/hermes/Dockerfile.base`:
- Around line 160-165: The parser verification commands in
agents/hermes/Dockerfile.base lines 160-165 and agents/hermes/Dockerfile lines
780-785 must verify the imported module matches the hashed file. In each Python
stress-test command, import html.parser and pathlib.Path, assert
Path(html.parser.__file__).resolve() points to
/usr/lib/python3.13/html/parser.py, then retain the existing parser stress
checks.
🪄 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: 995da057-44c9-46a2-9923-ef5fe6abdc95
📒 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
🚧 Files skipped from review as they are similar to previous changes (12)
- .github/workflows/base-image.yaml
- src/lib/onboard/base-image.ts
- agents/langchain-deepagents-code/Dockerfile.base
- test/perl-critical-cve-remediation.test.ts
- Dockerfile.base
- scripts/security/patches/libssh2-1.11.1-cve-2026.patch
- Dockerfile
- scripts/security/patches/python3.13-htmlparser-cve-2026-15308.patch
- test/helpers/onboard-script-mocks.cjs
- test/helpers/base-apt-security-functions.ts
- scripts/security/build-native-security-packages.sh
- agents/langchain-deepagents-code/Dockerfile
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/helpers/python-parser-security-fixture.ts (1)
11-12: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert parser callbacks instead of private state.
The probe locks the test to
_pendingandrawdata. Use anHTMLParsersubclass that recordshandle_comment()output afterclose(); retain the existing stress/timeout boundary for the performance claim.🤖 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/python-parser-security-fixture.ts` around lines 11 - 12, Update HTML_PARSER_SECURITY_PROBE to define an HTMLParser subclass that records handle_comment() output, then assert the expected callback result after close() instead of inspecting private _pending or rawdata state. Preserve the existing feed stress loops and timeout/performance boundary.Source: Path instructions
🤖 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.
Nitpick comments:
In `@test/helpers/python-parser-security-fixture.ts`:
- Around line 11-12: Update HTML_PARSER_SECURITY_PROBE to define an HTMLParser
subclass that records handle_comment() output, then assert the expected callback
result after close() instead of inspecting private _pending or rawdata state.
Preserve the existing feed stress loops and timeout/performance boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d2d21283-7a11-4e8d-927b-3f65c4c7bb27
📒 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
🚧 Files skipped from review as they are similar to previous changes (16)
- src/lib/onboard/dockerfile-remote-dashboard-bind-contract.ts
- src/lib/onboard/base-image.ts
- .github/workflows/base-image.yaml
- scripts/security/patches/python3.13-htmlparser-cve-2026-15308.patch
- test/perl-critical-cve-remediation.test.ts
- docs/security/sandbox-base-2026-07-27-native-package-review.md
- Dockerfile.base
- scripts/security/patches/libssh2-1.11.1-cve-2026.patch
- test/helpers/base-apt-security-functions.ts
- scripts/security/build-native-security-packages.sh
- test/helpers/onboard-script-mocks.cjs
- agents/langchain-deepagents-code/Dockerfile
- agents/langchain-deepagents-code/Dockerfile.base
- agents/hermes/Dockerfile.base
- agents/hermes/Dockerfile
- Dockerfile
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
214ded5 to
fdf47d3
Compare
|
@cv PR #7667 is approved on exact head |
|
Superseded by #7679, which uses the identical reviewed head SHA on an internal NVIDIA/NemoClaw branch. Required checks are not bypassed. |
<!-- markdownlint-disable MD041 --> ## 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 - Build libssh2 1.11.1 with the reviewed upstream fixes for CVE-2026-66032, CVE-2026-66033, CVE-2026-66034, and CVE-2026-66035 after verifying the source archive and original Debian runtime package. - Run the full upstream libssh2 test suite, preserve the `libssh2.so.1` soname, and reject any build that removes an exported Debian runtime symbol. - Package the reviewed CPython 3.13 fix for CVE-2026-15308 as a narrow replacement for `html/parser.py`, bound to the exact Debian standard-library version and fixed file hash. - Install and verify both packages in the OpenClaw, Hermes, and Deep Agents Code base images and repeat the package, inventory, file, runtime, and `dpkg --audit` checks in the completed images. - Rebuild the base images when the shared builder or reviewed patches change. - Add focused package-contract tests and a public review record with immutable identities, validation requirements, and removal conditions. ## 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: Codex Desktop completed the repository's nine-category security review on exact SHA `df11dfcd2`; all categories passed with no findings. The builder accepts no user-controlled network or package identity input, verifies every downloaded artifact before use, and preserves the existing container privilege 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: `docs/security/sandbox-base-2026-07-27-native-package-review.md` remains accurate. The follow-up only registers the already-reviewed final-image verification instruction with the fail-closed dashboard contract. - Agent: Codex Desktop <!-- docs-review-head-sha: df11dfc --> <!-- docs-review-agents-blob-sha: be20a09 --> ## 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] 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 — command/result or justification: Six focused integration files passed 100 tests on the remediation head; native arm64 built both packages and passed all 25 top-level libssh2 cases including 18 nested algorithm subcases. On exact head `df11dfcd2`, the dashboard lifecycle contract passed 34/34 tests and normal commit and pre-push hooks passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Native amd64 and arm64 builds for all three managed images remain the required CI gate. - [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 existing 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) — the new non-navigated Markdown review record has the required SPDX header; adjacent review records do not use Fern frontmatter. --- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security** * Added and pinned new native security packages (**libssh2-1t64** and **nemoclaw-python3.13-htmlparser-fix**) across base and production images. * Hardened immutable inventory and runtime integrity validation with stricter package/version checks, file checksum verification, and libssh2/html.parser behavior assertions. * **Documentation** * Added a sandbox base native package security review record with verification and removal criteria. * **Tests** * Expanded native security remediation and sandbox/provisioning test coverage with stronger fixture and end-to-end validation. * **Chores** * Updated CI build triggers to include additional security build artifacts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
3 participants
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.
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
fdf47d388; all categories passed with no findings. The builder accepts no user-controlled network or package identity input, verifies every downloaded artifact before use, and preserves the existing container privilege boundary.Documentation Writer Review
docs-updateddocs/security/sandbox-base-2026-07-27-native-package-review.mdaccurately documents that every base and completed image resolveshtml.parserto the checksum-verified file before running the behavior probe.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm 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 the required CI gate.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