test(images): add security revision container verifier#7289
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 WalkthroughWalkthroughAdds an opt-in Vitest Docker E2E suite that probes an installed OpenClaw image, enforces a hardened container boundary, validates an exact dependency remediation graph, and records structured evidence. ChangesOpenClaw security revision validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Vitest
participant Docker
participant Verifier
participant OpenClaw
participant npm
participant EvidenceArtifact
Vitest->>Docker: Run opted-in hardened container
Docker->>Verifier: Execute embedded probe
Verifier->>OpenClaw: Run openclaw --version
Verifier->>npm: Run npm ls
Verifier-->>Vitest: Return structured evidence
Vitest->>Vitest: Assert exact remediation graph
Vitest->>EvidenceArtifact: Write evidence JSON
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 48459d4 in the TypeScript / code-coverage/cliThe overall coverage in commit 48459d4 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / high 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: None This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
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/openclaw-security-revision-container-e2e.test.ts`:
- Around line 73-81: Extend ProbeEvidence.shrinkwrap with a jszip: LockedPackage
entry, then update the shrinkwrap probe assertions to validate jszip’s version,
resolved URL, and integrity using the same checks as braceExpansion, fsSafe, and
tar. Preserve the existing root-dependency and other package assertions.
🪄 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: 8497cec1-82f0-4e3d-a17f-b2e8633a8ba9
📒 Files selected for processing (1)
test/openclaw-security-revision-container-e2e.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Post-#7286 refresh is complete on exact head
Current-head CI and human review remain in progress; there are no unresolved review threads. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
Latest-head validation complete for
This head is conflict-free and mergeable; human approval is the remaining gate. |
cjagwani
left a comment
There was a problem hiding this comment.
Security review complete. Exact head 48459d4 requires an immutable named @sha256 image reference, runs offline/read-only as the sandbox user with all capabilities dropped, no privilege escalation, bounded CPU/memory/PIDs/files, and no host mounts or Docker socket. The exact self-hosted CI image nemoclaw-production@sha256:a075375409e3c81d248beafe2b627c6bd2ee5ea95f8f36a7b1c0fdb02850bc4c (OCI revision acfa261) passed 4/4 live verifier cases. All 53 current checks are green; DCO and all commits are verified.
senthilr-nv
left a comment
There was a problem hiding this comment.
Reviewed the exact current head and base. Product scope, security, CI, contributor, and test gates pass.
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `v0.0.91` changelog entry that was missed before the release tag was cut. Correct the custom-image compatibility guidance because the tagged code still accepts the legacy inference route selector instead of removing it in v0.0.91. ## Changes - Add `docs/changelog/2026-07-22.mdx` with the release summary and detailed security, rebuild, Hermes, DGX Station, and historical-validation changes from the published v0.0.91 announcement. - Link shipped behavior to the most specific published OpenClaw and Hermes documentation routes. - Correct the v0.0.90 changelog and command reference so they match the compatibility fallback present in the tagged v0.0.91 code without inventing a new removal version. - Keep the immutable v0.0.91 release tag unchanged; this is the documented post-release recovery path. Source summary: - [#7332](#7332), [#7289](#7289), and [#7294](#7294) -> `docs/changelog/2026-07-22.mdx`: Summarize completed-image `node-tar` remediation and current and historical container verification. - [#7213](#7213), [#7363](#7363), [#7366](#7366), and [#7369](#7369) -> `docs/changelog/2026-07-22.mdx`: Summarize trusted base preparation, backup reuse, deletion convergence, and rebuild confidence. - [#7212](#7212) -> `docs/changelog/2026-07-22.mdx`: Summarize the Hermes API bearer-token lifecycle and supported retrieval command. - [#7327](#7327) and [#7328](#7328) -> `docs/changelog/2026-07-22.mdx`: Summarize qualified DGX Station guidance and reproducible coding-agent installation instructions. - [#7355](#7355), [#7360](#7360), [#7362](#7362), and [#7364](#7364) -> `docs/changelog/2026-07-22.mdx`: Summarize restored historical OpenClaw upgrade and Hermes rebuild validation. - [#7189](#7189) -> `docs/changelog/2026-07-20.mdx`, `docs/reference/commands.mdx`: Correct its forward-looking removal deadline after v0.0.91 shipped with the documented legacy fallback still present. - [#7282](#7282), [#7306](#7306), and [#7341](#7341) need no additional user-guide update because they already update their owned contributor or user-facing text directly. ## 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 dated changelog structure, SPDX syntax, version ordering, and published routes. - [ ] 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: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this documentation-only PR does not change Station preparation or runtime behavior. - 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` (6 passed). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not run; this is a focused documentation-only 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) — completed with 0 errors and 2 existing site-wide 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 native changelog entry uses the required parser-safe MDX SPDX comment and intentionally has no frontmatter. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated v0.0.91 guidance for custom Dockerfiles, including continued legacy compatibility and recommended migration to `NEMOCLAW_INFERENCE_PROVIDER_ID`. * Added release notes covering image security scanning, safer rebuild behavior, token management, DGX Station guidance, and deterministic release validation. * Clarified that existing custom images may continue using the legacy selector temporarily, with fallback removal planned for a future release. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: cjagwani <cjagwani@nvidia.com>
## Summary Reopen follow-up for #6520: `nemoclaw uninstall --yes` followed by a curl reinstall hard-aborts at the installer strict pre-upgrade backup. The uninstall removes the gateway registration and the sandbox container but preserves `sandboxes.json`, so the stranded record can only ever be skipped — and the strict gate (`NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS=1`, #6114) exits 1 before the installer recovery phase (`recover_preexisting_sandboxes_before_onboard`), the phase that knows how to surface orphans, ever runs. The #6539 orphan classifier is wired only into `upgrade-sandboxes`, which is downstream of the aborting backup. ## What changed - `backupAll()` now classifies stranded records with the existing `classifyOrphanedRegistrySandboxes` (unobserved on the selected gateway AND persisted binding resolving to that same gateway), gated on the new `isSandboxContainerDefinitivelyAbsent()`. Stranded records are tracked separately from `skipped`, so the strict gate keeps failing closed for every genuine skip, and the run ends with `orphanedRegistrySummary` + `orphanedRegistryRemediation` (same destroy/onboard guidance as the recovery phase). End-to-end, the installer now proceeds to its recovery phase, which reports the orphans and yields "completed with warnings" instead of a hard abort. - The exemption is two-phase (PRA-1): after the backup loop, a confirming second pinned listing re-checks every stranded candidate — the same #6114 confirmation idiom as `upgrade-sandboxes` — and any candidate the gateway observes again reverts to a genuine strict skip. Container absence is itself checked per candidate at skip time. - `isSandboxContainerDefinitivelyAbsent()` fails closed everywhere absence cannot be proven: non-docker or unknown driver (including a throwing registry read), and a failed or timed-out labeled listing. The listing is status-checked (`docker ps -a` with the OpenShell labels) rather than reusing `findLabeledSandboxContainers`, which swallows docker errors — a dead daemon yields `null`, never "absent" — and passes `ignoreError` so the probe cannot `process.exit` the run. - `backup-all` now pins its sandbox listing to the selected gateway (same #6114 rationale and idiom as `upgrade-sandboxes`): an unpinned list taken from a sibling gateway selection must not feed a fail-open stranded decision. - The exemption carries an in-code source-of-truth review block (PRA-2): source boundary (`nemoclaw uninstall` preserves `sandboxes.json` by design), source-fix constraint (backup-all must not reconcile the registry; record removal is owned by the recovery phase destroy/onboard flow), and removal condition (install/uninstall registry reconciliation, or running the recovery phase before the strict backup). ## Constraints preserved (pinned by tests) - Strict gate still aborts on any genuine skip (#6114 cases untouched and green). - A sandbox bound to a sibling gateway is never claimed stranded, even when its container is absent on this host. - An unobserved sandbox whose container still exists keeps the strict abort (reconnect race). - A stranded candidate the confirming listing observes again reverts to a strict skip (lifecycle race). - Real backup failures (EACCES, non-manifest paths) still re-throw/abort. - A registry row with a null/unknown driver keeps the strict abort — fail closed, since absence cannot be proven for it. - `ORPHANED_SANDBOX_MARKER` / summary / remediation wording unchanged; install.sh greps the marker only from the recovery-phase log, which this change does not touch. ## Review responses - **PR Review Advisor PRA-1** (revalidate absence before exempting the gate): addressed by the confirming second pinned listing after the backup loop plus per-candidate absence checks at skip time; new test pins the reappeared-candidate revert. - **PR Review Advisor PRA-2** (bound the workaround to its source fix): addressed with the in-code source-of-truth review block (source boundary / fix constraint / removal condition). - `reviewed-npm-audit` failure is upstream-wide (also fails on #7289) and is being remediated by #7286; this PR adds no dependencies. ## Verification - `vitest`: `maintenance.test.ts` (32), `stopped-sandbox-backup.test.ts` (24), `orphan-detection.test.ts`, `upgrade-sandboxes-recovery.test.ts`, `openshell-sandbox-list.test.ts` — 108 tests green; installer lane `test/install-orphaned-sandbox-recovery.test.ts` (8, drives strict backup through recovery and the real install.sh orphan grep) green; `typecheck:cli`, `lint`, and `biome check` clean. The new maintenance tests pin `GATEWAY_PORT` so they stay green under an exported `NEMOCLAW_GATEWAY_PORT`. - Live end-to-end on a workstation (real docker daemon, real registry file, openshell CLI shimmed to a healthy empty gateway, real built CLI): stranded record → warning + `0 skipped` + exit 0 under strict mode (through both listings); same record bound to `gatewayPort: 9999` → strict abort exit 1 with no orphan claim; same record with a labeled container present → strict abort exit 1 with no orphan claim. Refs #6520 Signed-off-by: Dongni Yang <dongniy@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved backup-all orphan handling: potentially stranded sandboxes are skipped when their containers are definitively absent, then rechecked with the same selected gateway before remediation occurs. * Tightened strict-mode “fail closed” behavior for confirmed absent containers, including mismatched-gateway scenarios and reappearing candidates. * Reduced false “container absent” results by treating Docker/registry uncertainties as unknown rather than absent. * **Tests** * Expanded coverage for stranded-orphan detection and definitive container absence (including fail-closed, reappearance, and gateway-pinning scenarios) with improved Docker mock observability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Takeover review response - The PR Review Advisor handoff warning is addressed by an installer boundary regression that drives strict `backup-all`, orphan recovery, and the completed-with-warnings result in order. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Reviewed the complete five-file diff at `71a02caebe9f`. No documentation paths changed; existing docs already cover strict backup and stranded-record recovery and remediation. - Agent: Codex Desktop - PR: #7290 <!-- docs-review-head-sha: 71a02ca --> <!-- docs-review-agents-blob-sha: 560ff38 --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> --------- Signed-off-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Julie Yaunches <jyaunches@nvidia.com>
Summary
Add a standalone container verifier for the current OpenClaw security remediation produced by #7286. The verifier inspects the installed OpenClaw package graph in an offline, read-only, unprivileged container and rejects vulnerable, stale, or internally inconsistent dependency state.
Related Issue
Related to #7272. The real-container lane validates the final image produced after #7286 lands; this PR remains independently based on
mainand has no dependency on #7276.Changes
openclaw@2026.6.10graph.tar@7.5.19,brace-expansion@5.0.7,@openclaw/fs-safe@0.3.0, andjszip@3.10.1.@openclaw/fs-safeoptional dependencies to be absent, reject nested vulnerable copies, validatenpm ls, and verify the OpenClaw executable resolves to the reviewed installation.Type of Change
Quality Gates
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 unavailablefs-safe, and lock metadata.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Additional checks passed: CLI typecheck, exact Vitest project membership, test-title style, source-shape budget, test-file-size budget, Biome, repository checks, and secret scanning.
Merge Dependencies
This is a downstream verifier and does not block #7286 from merging. Human review can proceed now, but this PR must not merge until all items below are complete:
main.mainand run the opt-in real-container lane against the exact final built image.Final image evidence:
2ad613d6e9abc26859b59e5a876ddd0ee40c7ca148459d489c929076b47b41550b035cc90093bee1nemoclaw-production@sha256:a075375409e3c81d248beafe2b627c6bd2ee5ea95f8f36a7b1c0fdb02850bc4cacfa2613c7a645ae1bff21914f25d824e5fbcf62Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit