fix(security): update sandbox Perl runtime#7504
Conversation
Signed-off-by: Apurv Kumaria <akumaria@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 base image now builds pinned Perl from source into Debian packages, installs and validates those packages, and adds static Dockerfile contract tests. Sandbox image build timeouts and corresponding workflow validation expectations increase from 15 to 45 minutes. ChangesPerl CVE remediation
Sandbox build timeout updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant perl_builder
participant MainImage
participant RuntimeChecks
perl_builder->>perl_builder: Download and verify pinned Perl source
perl_builder->>perl_builder: Configure, compile, and run make test
perl_builder->>MainImage: Copy generated perl-base.deb and perl.deb
MainImage->>MainImage: Install packages and remove temporary files
MainImage->>RuntimeChecks: Verify Perl, Socket, Storable, and dpkg state
RuntimeChecks-->>MainImage: Return validation results
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit dad76f5 in the TypeScript / code-coverage/cliThe overall coverage in commit dad76f5 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / medium 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: 1 optional E2E recommendation
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
test/sandbox-provisioning.test.ts (1)
31-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate stage-extraction helper across two new test files.
This is functionally the same "slice from the last
FROM" logic added intest/perl-critical-cve-remediation.test.ts'scompletedStage(). See the consolidated comment for a shared-helper suggestion.🤖 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/sandbox-provisioning.test.ts` around lines 31 - 35, Remove the duplicate completedDockerStage helper and reuse the existing shared stage-extraction helper introduced as completedStage in perl-critical-cve-remediation.test.ts. Update callers in sandbox-provisioning.test.ts to use that shared symbol while preserving the behavior of slicing from the last “FROM” stage.test/perl-critical-cve-remediation.test.ts (1)
61-76: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winProve the superseded Perl packages are actually removed, not just declared conflicting.
This test only checks the
.debcontrol-file directives anddpkg --audit; it never asserts thatlibperl5.40/perl-modules-5.40are actually gone from the completed image (e.g.dpkg -s libperl5.40failing). As per path instructions, "Migration tests must prove the superseded path is unreachable or removed, not merely prove that the new path also works."♻️ Example negative-path assertion
expect(runtime).toContain('test -z "$(dpkg --audit)"'); + expect(runtime).toContain( + '! dpkg -s libperl5.40 perl-modules-5.40 >/dev/null 2>&1', + );(requires a matching check added to the Dockerfile's install RUN block)
🤖 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/perl-critical-cve-remediation.test.ts` around lines 61 - 76, Add a negative-path assertion to the completed Perl runtime migration: update the Dockerfile install RUN block used by the runtime stage to verify `dpkg -s libperl5.40` and `dpkg -s perl-modules-5.40` fail after installation, then assert those checks in the `replaces the vulnerable distro packages without breaking dpkg ownership` test. Keep the existing control-file and `dpkg --audit` assertions.Source: Path instructions
Dockerfile.base (1)
98-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winperl-base package declares no runtime
Depends.The control file only carries migration fields (Provides/Conflicts/Breaks/Replaces); there's no
Dependson the shared libraries this binary actually links against (libc, and likely zlib/bz2/db/gdbm for bundled XS modules). This contradicts the stated goal in the comment above ("so dpkg dependencies... describe the runtime that is actually used") — today's build-time runtime checks would catch a missing.soat build time, but dpkg itself has no record that perl needs these libs, so a futureapt-get autoremoveor base-image change could silently drop them.Consider generating an accurate
Dependsline withdpkg-shlibdeps(standard Debian tooling) against the staged binaries before writing the control file, rather than leaving it empty.🤖 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 98 - 129, Update the perl-base packaging flow in the RUN block so its control metadata includes runtime shared-library dependencies. Stage or otherwise expose the packaged Perl binaries and use dpkg-shlibdeps to derive the dependency set before writing /tmp/perl-root/DEBIAN/control, then emit the generated values in a Depends field while preserving the existing migration fields and perl metapackage dependency.
🤖 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 `@Dockerfile.base`:
- Around line 171-172: Update the Perl regex probe in the Dockerfile build check
to generate more than 65,535 distinct fixed-string branches, ensuring the
expression exercises the CVE-2026-13221 threshold and fails on vulnerable Perl
while passing on fixed versions. Keep the existing probe structure and
build-gating behavior unchanged.
In `@test/perl-critical-cve-remediation.test.ts`:
- Around line 78-87: Update the test around completedStage() to assert the
Dockerfile exercises the 65,535-branch regex overflow boundary required by
CVE-2026-13221, rather than merely checking for the copied "fnord" regex
literal. Preserve the existing assertions for the Perl and module versions,
pack_ip_mreq_source, and short-source rejection.
---
Nitpick comments:
In `@Dockerfile.base`:
- Around line 98-129: Update the perl-base packaging flow in the RUN block so
its control metadata includes runtime shared-library dependencies. Stage or
otherwise expose the packaged Perl binaries and use dpkg-shlibdeps to derive the
dependency set before writing /tmp/perl-root/DEBIAN/control, then emit the
generated values in a Depends field while preserving the existing migration
fields and perl metapackage dependency.
In `@test/perl-critical-cve-remediation.test.ts`:
- Around line 61-76: Add a negative-path assertion to the completed Perl runtime
migration: update the Dockerfile install RUN block used by the runtime stage to
verify `dpkg -s libperl5.40` and `dpkg -s perl-modules-5.40` fail after
installation, then assert those checks in the `replaces the vulnerable distro
packages without breaking dpkg ownership` test. Keep the existing control-file
and `dpkg --audit` assertions.
In `@test/sandbox-provisioning.test.ts`:
- Around line 31-35: Remove the duplicate completedDockerStage helper and reuse
the existing shared stage-extraction helper introduced as completedStage in
perl-critical-cve-remediation.test.ts. Update callers in
sandbox-provisioning.test.ts to use that shared symbol while preserving the
behavior of slicing from the last “FROM” stage.
🪄 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: bc6acd88-804f-496f-b036-748966c8cc18
📒 Files selected for processing (4)
Dockerfile.baseci/source-shape-test-budget.jsontest/perl-critical-cve-remediation.test.tstest/sandbox-provisioning.test.ts
senthilr-nv
left a comment
There was a problem hiding this comment.
Blocking: the new leading perl-builder stage makes the existing test/runner.test.ts runtime-stage ordering check inspect the builder stage. CI shard 6 fails at test/runner.test.ts:1043 with expected 6311 to be less than 3358, and the focused test reproduces locally. Update this test to scope Dockerfile.base to the completed runtime stage, as this PR already does for the affected sandbox-provisioning checks, then rerun the focused test. The security and implementation review is otherwise clean: the official CPAN archive matches the pinned SHA-256, Perl 5.44.0 release notes cover all three CVEs, package replacement is fail-closed, and the nine changed-contract tests pass.
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
@senthilr-nv The runtime-stage ordering blocker is fixed in verified signed commit eafa6e6 ( |
|
Exact-head refresh for |
senthilr-nv
left a comment
There was a problem hiding this comment.
Approved current head 68e8f2c. The prior runtime-stage ordering blocker is fixed. Security review found no blocking issue: the official Perl source is version- and checksum-pinned, upstream tests and fail-closed runtime/package checks cover the remediation, no secrets or unsafe input/privilege changes are introduced, the changed-contract tests pass, and the tree composes cleanly with #7502. Pending GitHub checks remain required and are not waived. Non-blocking follow-ups: consider deriving shared-library Depends metadata and documenting the QEMU override removal condition.
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Addressed the remaining advisor warning in signed commit |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical pre-tag `## v0.0.95` release entry to `docs/changelog/2026-07-24.mdx`, before the existing v0.0.94 entry. The entry summarizes approved user-visible changes merged since v0.0.94 and excludes internal-only prerequisites. ## Changes - Adds the v0.0.95 summary and detailed bullets for gateway lifecycle, recovery, state transfer, inference compatibility, sandbox security, Discord policy, and E2E evidence. - Links each user-facing theme to the most specific published documentation. - Records the release entry in the shared native changelog used by the OpenClaw, Hermes, and Deep Agents guides. Source summary: - [#7246](#7246), [#7228](#7228), [#7267](#7267), [#7489](#7489), [#7509](#7509), [#7351](#7351), and [#7290](#7290) -> `docs/changelog/2026-07-24.mdx`: Gateway authority, forward teardown and retry, managed recovery, Hermes restart recovery, scoped uninstall, and orphan-aware backup behavior. - [#7344](#7344) and [#7416](#7416) -> `docs/changelog/2026-07-24.mdx`: Atomic SQLite restore and host download verification. - [#7476](#7476), [#7347](#7347), [#7281](#7281), [#7485](#7485), [#7491](#7491), and [#7422](#7422) -> `docs/changelog/2026-07-24.mdx`: Windows Ollama reuse, CDI fallback, bounded OpenRouter connection setup, Nemotron-3 request compatibility, and managed Deep Agents retry and provider-error behavior. - [#6884](#6884), [#7481](#7481), [#6878](#6878), [#7467](#7467), [#7502](#7502), [#7503](#7503), [#7504](#7504), and [#7486](#7486) -> `docs/changelog/2026-07-24.mdx`: Trusted base-image overrides, local rebuild images, runtime validation, config preservation, reviewed package updates, and fewer final-image payload layers. - [#7303](#7303) -> `docs/changelog/2026-07-24.mdx`: Scoped Discord application-command management. - [#7488](#7488), [#7465](#7465), [#7497](#7497), [#7464](#7464), [#7501](#7501), [#7494](#7494), and [#7493](#7493) -> `docs/changelog/2026-07-24.mdx`: Selected-test risk signals, retry cleanup, full root-image validation, direct-main Hermes setup, executed PR-gate evidence, nightly history, and runner wait reporting. - [#7447](#7447) is an internal pinned-runtime prerequisite and is intentionally excluded from canonical supported-integration documentation. - [#7370](#7370) adds maintainer-only advisory reconciliation tooling and does not change supported user behavior. - [#7495](#7495) updates existing documentation and does not add a new v0.0.95 behavior claim. ## 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 structure, heading uniqueness, and published links. - [ ] 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: `docs/changelog/2026-07-24.mdx`; writing rules, documentation style, factual release meaning, and published links reviewed at exact head `58b02f2bf`. - Agent: Codex documentation writer reviewer <!-- docs-review-head-sha: 58b02f2 --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## 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: `npx vitest run test/changelog-docs.test.ts` passed 6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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) — the build passed with 0 errors and 2 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) --- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new v0.0.95 changelog entry above v0.0.94. * Documented improved externally supervised gateway lifecycle ownership. * Improved snapshot restore reliability and SQLite state handling. * Tightened CLI `backup-all` behavior and host artifact verification. * Updated Windows onboarding guidance (including Ollama service reuse and CDI directory fallback). * Noted inference compatibility fixes, deeper agent failure classification, stricter base-image validation, updated Discord bot command permissions, and refined E2E release automation evidence handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary OpenClaw base-image publication runs its full Perl regression suite for arm64 under QEMU, which can exceed the 90-minute job timeout. This change builds each platform on a native GitHub-hosted runner and publishes user-facing tags only after both immutable digests succeed. ## Changes - Build the OpenClaw amd64 and arm64 images on native runners with separate 60-minute timeouts and cache namespaces. The GHCR publisher needs the complete Perl suite from #7504; increasing the QEMU timeout does not remove the emulation bottleneck. - Transfer immutable platform digests to a 10-minute manifest job. The manifest job preserves the existing required-check name and updates release, `latest`, and SHA tags only after both builds pass. - Keep the Hermes and Deep Agents Code QEMU publisher and its 90-minute timeout unchanged. - Update the base-image workflow, Perl remediation, build-argument, and publication-gate contracts for the native and atomic publication behavior. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: This changes the internal image publication workflow. It does not change a CLI, configuration, API, policy, documented behavior, or image content. - [x] 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: `no-docs-needed` - Evidence: Internal base-image publication now builds OpenClaw on native amd64 and arm64 runners and publishes the manifest after both builds pass. No CLI, configuration, API, policy, documented behavior, or image content changes. - Agent: Codex Desktop documentation writer subagent <!-- docs-review-head-sha: 5f0bbb8 --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## 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 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: Integration workflow, Perl remediation, and build-argument tests passed (18/18). The E2E-support base-image publication tests passed (33/33). - [ ] 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; focused workflow contracts cover this change. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * OpenClaw sandbox images are now built natively for both AMD64 and ARM64 platforms. * Multi-platform manifests are validated before publishing, ensuring only the expected architectures are included. * Version and image digest validation has been strengthened for safer releases. * **Documentation** * Updated build documentation to clarify cross-platform configuration and test execution. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Summary
The published OpenClaw sandbox base currently inherits Debian trixie's vulnerable Perl 5.40 runtime. This change installs checksum-pinned Perl 5.44.0, which contains the upstream fixes for CVE-2026-12087, CVE-2026-13221, and CVE-2026-57433, while preserving Debian package ownership and runtime dependencies.
Related Issue
Related to #7338. This PR remediates the current Critical findings; it does not close the broader early-detection and rescan work tracked there.
Changes
perl-baseandperlDebian packages sodpkg, dependency resolution, and vulnerability inventory describe the installed runtime.libperl5.40andperl-modules-5.40packages during installation.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededDGX 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 unavailableperl-critical-cve-remediation(4/4), runner contracts (55/55), sandbox workflow boundary (12/12), sandbox provisioning (61/61), adjacent Dockerfile contracts (24/24), and base-image publication contracts (60/60) passed. The corrected regex probe fails against vulnerable Perl 5.40 withregexp memory corruptionand passes against fixed Perl 5.44. The full arm64 image build passed 2,763 upstream scripts / 1,391,429 tests, package audit, CVE behavior probes, exact runtime version checks, and Git compatibility. After the latest main refresh, 78 focused security/integration tests and the test-size, project-membership, source-shape, secret-scan, and commit-message guardrails passed.npm run checkspassed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Security
Tests
Chores / CI