Skip to content

fix(hermes): remove stale openclaw state dir - #5882

Merged
cv merged 24 commits into
mainfrom
fix/hermes-openclaw-dir-cleanup
Jun 28, 2026
Merged

fix(hermes): remove stale openclaw state dir#5882
cv merged 24 commits into
mainfrom
fix/hermes-openclaw-dir-cleanup

Conversation

@ericksoa

@ericksoa ericksoa commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Hermes final image layout cleanup now removes inherited /sandbox/.openclaw state from stale Hermes base images. Dockerfile.base remains the source of truth for new base-image layout; this final-image repair only covers invalid .openclaw state already baked into older published Hermes base layers, where runtime migration cannot depend on root privileges after OpenShell starts the sandbox as the sandbox user. The .openclaw cleanup can be removed once the minimum supported Hermes base digest is newer than the stale layout.

Changes

  • Remove stale /sandbox/.openclaw during Hermes final-image layout repair.
  • Reject symlinked stale OpenClaw state before cleanup and assert the path is gone afterward.
  • Enforce the stale-layout workaround removal trigger by checking the default published Hermes base digest at build time.
  • Add scripts/verify-hermes-stale-openclaw-image.sh plus a self-hosted PR CI job that builds real synthetic stale-directory and stale-symlink Hermes base images, validates the final-image runtime layout, and proves the symlink path fails closed.
  • Harden the new self-hosted validation job by disabling checkout credential persistence and validating the verifier's resolved base-image input.
  • Keep existing Hermes provisioning coverage verifying stale .openclaw removal while Hermes state permissions remain correct.

Type of Change

  • 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

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: internal Hermes image layout cleanup; no user-facing behavior or commands changed.
  • 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: author self-review; cleanup refuses symlinked /sandbox/.openclaw, removes only stale non-symlink .openclaw state baked into older Hermes base images, asserts removal, adds a default-base digest gate for workaround removal, and does not expand credential, policy, or runtime egress behavior.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Verification commands run:

  • bash -n scripts/verify-hermes-stale-openclaw-image.sh
  • shellcheck scripts/verify-hermes-stale-openclaw-image.sh
  • npx prek run --all-files --stage pre-push --skip tsc-plugin --skip tsc-js --skip tsc-cli --skip version-tag-sync --skip test-cli --skip test-plugin --skip source-shape-test-budget --skip test-file-size-budget --skip test-skills-yaml
  • NEMOCLAW_HERMES_BASE_IMAGE=ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@sha256:60333c1982ad855d55887b4488e867eb343f3930a30aa8e0268e5397fc6f2926 bash scripts/verify-hermes-stale-openclaw-image.sh
  • npm test -- test/hermes-doctor-config-hash.test.ts test/pr-workflow-contract.test.ts test/sandbox-provisioning.test.ts
  • npm test -- test/pr-workflow-contract.test.ts test/sandbox-provisioning.test.ts
  • npm run source-shape:check
  • npm run test-size:check
  • npm run test-conditionals:scan -- --top 25
  • git diff --check
  • npm test -- test/sandbox-provisioning.test.ts
  • npm run build:cli
  • npm test -- src/lib/onboard/sandbox-create-launch.test.ts src/lib/onboard/openclaw-runtime-env.test.ts test/hermes-doctor-config-hash.test.ts
  • wc -l test/sandbox-provisioning.test.ts
  • gh api /repos/NVIDIA/NemoClaw/commits/2837307a0 --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/98ea6fa72 --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/396471a1b --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/9c48e0607 --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/6e0acc56b --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/2ca0288f8 --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/989dc46c3 --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/f69014248 --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/b4bc2a1ab --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/060b3f3d2 --jq '.commit.verification'
  • gh api /repos/NVIDIA/NemoClaw/commits/a6d8b17d8 --jq '.commit.verification'

Note: npx prek run --from-ref origin/main --to-ref HEAD was started and visible checks passed through gitleaks/markdownlint skip, but the command stopped producing output and was interrupted rather than claimed as passed.


Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

  • New Features

    • Added a self-hosted CI job and a Hermes verifier to validate “stale OpenClaw” cleanup behavior (directory cleanup and symlink detection).
  • Bug Fixes

    • Strengthened Hermes stale OpenClaw repair/guardrails with digest-based enforcement for supported pinned bases.
    • Improved Hermes base-image resolution to prefer immutable image@sha256: digests when available.
  • Tests

    • Expanded Hermes stale OpenClaw regression coverage with new provisioning options and reference-guarding assertions.
    • Updated workflow/contract tests to verify the new job and base resolver behavior.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 56e7e08d-a628-4c10-b6ef-8fe0e03229b9

📥 Commits

Reviewing files that changed from the base of the PR and between e6cec21 and ef9fa34.

📒 Files selected for processing (1)
  • agents/hermes/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (1)
  • agents/hermes/Dockerfile

📝 Walkthrough

Walkthrough

Hermes now validates stale .openclaw state during image builds, adds a CI verification job and script for directory and symlink cases, and updates sandbox provisioning tests to seed and assert those cases.

Changes

Hermes OpenClaw cleanup

Layer / File(s) Summary
Dockerfile sandbox cleanup
agents/hermes/Dockerfile
BASE_IMAGE is redeclared after FROM, NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST is added, and the stale-base repair step validates the digest and removes /sandbox/.openclaw unless it is a symlink.
Resolver and CI wiring
.github/actions/resolve-hermes-base-image/action.yaml, .github/workflows/pr-self-hosted.yaml, test/pr-workflow-contract.test.ts
The Hermes base-image resolver now exports a digest form, the self-hosted PR workflow adds build-hermes-stale-openclaw-image, and the workflow contract test checks the job and action commands.
Verification script
scripts/verify-hermes-stale-openclaw-image.sh
The new verifier validates Hermes base-image references, builds stale-directory and stale-symlink base images, checks final-image behavior, and supports a ref-only validation mode with cleanup of temporary artifacts.
Test helpers and guard tests
test/helpers/hermes-dockerfile-run.ts, test/hermes-stale-openclaw-guard.test.ts
The sandbox helper exports the stale-base digest, can precreate stale .openclaw directory or symlink state, and the Hermes guard tests cover digest mismatch, unsupported refs, verifier safety checks, and stale-path cleanup.
Sandbox provisioning tests
test/sandbox-provisioning.test.ts
The sandbox provisioning helper passes the pinned stale digest into Docker shells, can seed stale OpenClaw layouts, and the Hermes provisioning tests assert symlink failure and stale-path removal.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

area: ci, area: e2e

Suggested reviewers

  • cv
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: Hermes cleanup now removes stale OpenClaw state from inherited base images.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/hermes-openclaw-dir-cleanup

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the fix/hermes-openclaw-... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/hermes-openclaw-... ef9fa34 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the fix/hermes-openclaw-... branch is 68%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/hermes-openclaw-... ef9fa34 +/-
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 79%
src/lib/actions...dbox/rebuild.ts 74%
src/lib/state/sandbox.ts 72%
src/lib/shields/index.ts 70%
src/lib/onboard/preflight.ts 69%
src/lib/actions...licy-channel.ts 59%
src/lib/onboard...er-gpu-patch.ts 59%
src/lib/policy/index.ts 52%
src/lib/onboard.ts 20%

Updated June 28, 2026 18:29 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/sandbox-provisioning.test.ts (1)

1308-1312: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering the symlink fail-closed branch too.

The fixture seeds .openclaw as a regular directory, so only the happy-path removal is exercised. The Dockerfile's security-relevant if [ -L "$openclaw_dir" ]; ... exit 1 branch (a symlinked stale state must abort the build) is never tested, so a regression that follows the symlink could go unnoticed. A second scenario that seeds .openclaw as a symlink and asserts the run fails would lock down that behavior.

Want me to draft the symlink-rejection test case?

🤖 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 1308 - 1312, Add a second
sandbox-provisioning test scenario in test/sandbox-provisioning.test.ts that
seeds .openclaw as a symlink instead of a normal directory and verifies the
build/run fails closed. Reuse the existing sandbox fixture setup around
precreateStaleOpenclaw, but create the symlinked stale state and assert the
Dockerfile path that checks if [ -L "$openclaw_dir" ] exits with an error, so
the symlink-rejection branch is covered alongside the current happy-path removal
case.
🤖 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/sandbox-provisioning.test.ts`:
- Around line 1308-1312: Add a second sandbox-provisioning test scenario in
test/sandbox-provisioning.test.ts that seeds .openclaw as a symlink instead of a
normal directory and verifies the build/run fails closed. Reuse the existing
sandbox fixture setup around precreateStaleOpenclaw, but create the symlinked
stale state and assert the Dockerfile path that checks if [ -L "$openclaw_dir" ]
exits with an error, so the symlink-rejection branch is covered alongside the
current happy-path removal case.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 17f63828-3104-4974-ae4a-da0f432bf095

📥 Commits

Reviewing files that changed from the base of the PR and between 9f48114 and a6d8b17.

📒 Files selected for processing (2)
  • agents/hermes/Dockerfile
  • test/sandbox-provisioning.test.ts

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: build-hermes-stale-openclaw-image, hermes-root-entrypoint-smoke-vitest, hermes-sandbox-secret-boundary-vitest, hermes-e2e-vitest
Optional E2E: hermes-dashboard-vitest

Dispatch hint: hermes-root-entrypoint-smoke-vitest,hermes-sandbox-secret-boundary-vitest,hermes-e2e-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • build-hermes-stale-openclaw-image (medium): This self-hosted PR job is the direct Docker-level validation for the changed stale /sandbox/.openclaw cleanup and immutable Hermes base-image contract. It builds synthetic stale directory and symlink bases and verifies the final image removes safe stale state while failing closed on symlinks.
  • hermes-root-entrypoint-smoke-vitest (medium): Builds/probes the real Hermes image and root entrypoint path. Required because the Dockerfile final-image cleanup and layout changes can break root-entrypoint startup behavior before the sandbox drops privileges.
  • hermes-sandbox-secret-boundary-vitest (medium): Exercises the real Hermes sandbox image and startup secret-boundary contracts. Required because the PR changes Dockerfile cleanup and base-image provenance checks in a security-sensitive area.
  • hermes-e2e-vitest (high): Runs the hosted-inference Hermes live assistant flow against a built sandbox. Required to catch regressions where the final-image layout cleanup succeeds syntactically but breaks Hermes onboarding/startup or an actual assistant turn.

Optional E2E

  • hermes-dashboard-vitest (high): Useful adjacent coverage because this Dockerfile also contains Hermes dashboard prebuild/layout logic near the modified final-image provisioning path, but the PR does not directly change dashboard behavior.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: .github/workflows/e2e-vitest-scenarios.yaml
  • jobs input: hermes-root-entrypoint-smoke-vitest,hermes-sandbox-secret-boundary-vitest,hermes-e2e-vitest

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: hermes-root-entrypoint-smoke-vitest, hermes-sandbox-secret-boundary-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • None.

Optional Vitest E2E scenarios

  • hermes-root-entrypoint-smoke-vitest: Adjacent Docker-focused Hermes coverage: builds the Hermes image and verifies root entrypoint/runtime layout behavior, which is close to the changed final-image cleanup block.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-root-entrypoint-smoke-vitest
  • hermes-sandbox-secret-boundary-vitest: Adjacent Hermes image/startup hardening coverage that rebuilds the Hermes image and checks final-image and startup secret-boundary contracts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-sandbox-secret-boundary-vitest

Relevant changed files

  • agents/hermes/Dockerfile

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-5: Verify local/verifier base exemption in digest guard is intentional and tested; then add or justify PRA-T1.
Open items: 1 required · 2 warnings · 5 suggestions · 5 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

  • PRA-5 Fix: Verify local/verifier base exemption in digest guard is intentional and tested in agents/hermes/Dockerfile:279
  • PRA-2 Resolve or justify: Document/verify self-hosted runner docker daemon isolation for new job in .github/workflows/pr-self-hosted.yaml:118
  • PRA-6 Resolve or justify: Document why Hermes resolver is stricter than sandbox resolver and verify local fallback in .github/actions/resolve-hermes-base-image/action.yaml:45
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Evaluate consolidating dockerRunCommandContaining with dockerRunCommandBetween
  • PRA-T5 Add or justify test follow-up: Add explicit types for test-specific env vars after @ts-nocheck removal
  • PRA-1 In-scope improvement: Add $() command substitution to unsafe character class in require_safe_image_ref in scripts/verify-hermes-stale-openclaw-image.sh:58
  • PRA-3 In-scope improvement: Document bash requirement in verifier script shebang comment in scripts/verify-hermes-stale-openclaw-image.sh:1
  • PRA-4 In-scope improvement: Add CI enforcement for REMOVAL_CHECKLIST instead of comment-only in agents/hermes/Dockerfile:268
  • PRA-7 In-scope improvement: Evaluate consolidating dockerRunCommandContaining with dockerRunCommandBetween in test/helpers/hermes-dockerfile-run.ts:100
  • PRA-8 In-scope improvement: Add explicit types for test-specific env vars after @ts-nocheck removal in test/sandbox-provisioning.test.ts:1

Findings index

ID Severity Category Location Required action
PRA-1 Improvement security scripts/verify-hermes-stale-openclaw-image.sh:58 Add '\$\(' to the unsafe character pattern in the case statement: *[[:space:]]* | *[;\`"'$()\\]*
PRA-2 Resolve/justify security .github/workflows/pr-self-hosted.yaml:118 Confirm self-hosted runner docker daemon isolation is documented in runner onboarding docs. Add a comment in the workflow noting the docker privilege requirement and isolation expectation.
PRA-3 Improvement security scripts/verify-hermes-stale-openclaw-image.sh:1 Add a comment near the shebang noting 'Requires bash for process substitution and extended test operators'
PRA-4 Improvement architecture agents/hermes/Dockerfile:268 Add a check in verify_dockerfile_base_digest_contract() or a new test that compares the default BASE_IMAGE digest against NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST and warns/fails if the default is newer but cleanup remains
PRA-5 Required acceptance agents/hermes/Dockerfile:279 The current design is intentional per comments (local/verifier bases are exempt). Verify the verify script's synthetic base construction correctly replicates the stale layout from whatever BASE_IMAGE is provided. This appears correct - build_stale_dir_base and build_stale_link_base both use the resolved BASE_IMAGE as FROM.
PRA-6 Resolve/justify acceptance .github/actions/resolve-hermes-base-image/action.yaml:45 Add a comment in the action explaining why Hermes resolver is stricter than sandbox resolver (stale cleanup requires digest pinning). Verify the local build fallback path works by testing with no GHCR digests available.
PRA-7 Improvement tests test/helpers/hermes-dockerfile-run.ts:100 Evaluate if dockerRunCommandBetween can be refactored to use dockerRunCommandContaining with marker signatures, reducing duplication
PRA-8 Improvement tests test/sandbox-provisioning.test.ts:1 Add type declarations for NEMOCLAW_FAKE_DOCKER_LOG and other test-specific env vars, or use a typed env helper

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-5 Required — Verify local/verifier base exemption in digest guard is intentional and tested

  • Location: agents/hermes/Dockerfile:279
  • Category: acceptance
  • Problem: The stale cleanup block validates BASE_IMAGE against the tracked stale digest, but the case statement allows 'nemoclaw-hermes-base-local' and verifier tags unconditionally. If a local build is done from a base that has the stale layout but a different digest, the cleanup would run without the digest guard. The verify script builds synthetic stale bases from the resolved BASE_IMAGE, so this is covered in CI, but the Dockerfile alone could accept a mismatched local base.
  • Impact: Local builds with mismatched stale-layout bases could bypass the digest guard, though CI verifier catches this by building synthetic stale bases from the resolved BASE_IMAGE.
  • Required action: The current design is intentional per comments (local/verifier bases are exempt). Verify the verify script's synthetic base construction correctly replicates the stale layout from whatever BASE_IMAGE is provided. This appears correct - build_stale_dir_base and build_stale_link_base both use the resolved BASE_IMAGE as FROM.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check verify script lines 95-115 (build_stale_dir_base, build_stale_link_base) - they use ${BASE_IMAGE} as the FROM image
  • Missing regression test: The verify script already covers this by building synthetic stale bases from the resolved BASE_IMAGE
  • Done when: The required change is committed and verification passes: Check verify script lines 95-115 (build_stale_dir_base, build_stale_link_base) - they use ${BASE_IMAGE} as the FROM image.
  • Evidence: Dockerfile lines 291-294: case statement allows nemoclaw-hermes-base-local and verifier tags without digest check; verify script lines 95-115 build synthetic bases from resolved BASE_IMAGE
Review findings by urgency: 1 required fix, 2 items to resolve/justify, 5 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-2 Resolve/justify — Document/verify self-hosted runner docker daemon isolation for new job

  • Location: .github/workflows/pr-self-hosted.yaml:118
  • Category: security
  • Problem: The new build-hermes-stale-openclaw-image job runs on self-hosted runner 'linux-amd64-cpu4' with docker daemon access. While persist-credentials: false is set on checkout, the job runs docker build which requires privileged docker access. The runner environment's docker daemon isolation from other workloads should be verified/documented.
  • Impact: If docker daemon is shared across workloads, a compromised build could affect other jobs or access cached layers from other projects.
  • Recommended action: Confirm self-hosted runner docker daemon isolation is documented in runner onboarding docs. Add a comment in the workflow noting the docker privilege requirement and isolation expectation.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Review .github/workflows/pr-self-hosted.yaml lines 118-145 and runner onboarding documentation at https://docs.gha-runners.nvidia.com/
  • Missing regression test: N/A - infrastructure concern; verify via runner documentation review
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Review .github/workflows/pr-self-hosted.yaml lines 118-145 and runner onboarding documentation at https://docs.gha-runners.nvidia.com/.
  • Evidence: Job build-hermes-stale-openclaw-image at lines 118-145 uses runs-on: linux-amd64-cpu4 with docker build commands

PRA-6 Resolve/justify — Document why Hermes resolver is stricter than sandbox resolver and verify local fallback

  • Location: .github/actions/resolve-hermes-base-image/action.yaml:45
  • Category: acceptance
  • Problem: The resolver action now requires immutable GHCR digests (rejects :latest and tag refs when stale cleanup is active). This is a behavior change from the sandbox base resolver which allows tags. Ensure this is documented and that the fallback to local build works correctly when no compatible digest is found.
  • Impact: If no compatible GHCR digest exists, the action falls back to local build. This path should be tested to ensure it works correctly.
  • Recommended action: Add a comment in the action explaining why Hermes resolver is stricter than sandbox resolver (stale cleanup requires digest pinning). Verify the local build fallback path works by testing with no GHCR digests available.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare .github/actions/resolve-hermes-base-image/action.yaml with .github/actions/resolve-sandbox-base-image/action.yaml
  • Missing regression test: Add an integration test that simulates no GHCR digests available and verifies local build fallback
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare .github/actions/resolve-hermes-base-image/action.yaml with .github/actions/resolve-sandbox-base-image/action.yaml.
  • Evidence: Hermes resolver lines 45-48: requires digest_ref with ${image}@sha256: pattern; sandbox resolver allows tags via try_image()

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-1 Improvement — Add $() command substitution to unsafe character class in require_safe_image_ref

  • Location: scripts/verify-hermes-stale-openclaw-image.sh:58
  • Category: security
  • Problem: The unsafe character check rejects backticks, semicolons, quotes, and backslashes but not $() command substitution syntax. While the subsequent allowlist restricts to known-safe patterns, adding $() provides defense-in-depth against shell injection via image refs.
  • Impact: Low-risk defense-in-depth gap; allowlist already restricts to safe patterns but explicit rejection of $() would catch injection attempts earlier.
  • Suggested action: Add '\$\(' to the unsafe character pattern in the case statement: *[[:space:]]* | *[;\`"'$()\\]*
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check line 58 in verify-hermes-stale-openclaw-image.sh for the case statement pattern
  • Missing regression test: Add a test case with 'ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@sha256:$(id)' to the unsafeRefs array in test/hermes-stale-openclaw-guard.test.ts
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 58: case "$ref" in *[[:space:]]* | *[;\`"'\\]*)

PRA-3 Improvement — Document bash requirement in verifier script shebang comment

  • Location: scripts/verify-hermes-stale-openclaw-image.sh:1
  • Category: security
  • Problem: The verify script uses bash-specific features (process substitution 'exec > >(tee...)', [[ ]] extended tests, arrays). The shebang is #!/bin/bash which is correct, but the requirement should be explicitly documented for future maintainers.
  • Impact: Maintainability risk if script is ever run with sh instead of bash; process substitution would fail.
  • Suggested action: Add a comment near the shebang noting 'Requires bash for process substitution and extended test operators'
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check line 1 of verify-hermes-stale-openclaw-image.sh
  • Missing regression test: N/A - documentation only
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 1: #!/bin/bash with process substitution at line 13: exec > >(tee -a "$LOG_PATH") 2>&1

PRA-4 Improvement — Add CI enforcement for REMOVAL_CHECKLIST instead of comment-only

  • Location: agents/hermes/Dockerfile:268
  • Category: architecture
  • Problem: The REMOVAL_CHECKLIST in the Dockerfile (lines 268-278) is excellent for technical debt management but exists only as a comment. A CI check should fail when the default BASE_IMAGE digest advances past the stale digest but the cleanup block hasn't been removed.
  • Impact: Technical debt could persist indefinitely without automated enforcement; the workaround should be removed automatically when criteria are met.
  • Suggested action: Add a check in verify_dockerfile_base_digest_contract() or a new test that compares the default BASE_IMAGE digest against NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST and warns/fails if the default is newer but cleanup remains
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check Dockerfile lines 1-6 (ARGs) and 268-278 (REMOVAL_CHECKLIST comment)
  • Missing regression test: Add a test that parses both ARGs from Dockerfile and asserts cleanup removal when default digest > stale digest
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Dockerfile lines 268-278: REMOVAL_CHECKLIST with 4 criteria but no automated enforcement

PRA-7 Improvement — Evaluate consolidating dockerRunCommandContaining with dockerRunCommandBetween

  • Location: test/helpers/hermes-dockerfile-run.ts:100
  • Category: tests
  • Problem: The new dockerRunCommandContaining function extracts Dockerfile RUN commands by signature. This is a useful helper but duplicates some logic with dockerRunCommandBetween. Consider if both are needed or if one can be generalized.
  • Impact: Code duplication in test helpers; maintenance burden if Dockerfile parsing logic diverges.
  • Suggested action: Evaluate if dockerRunCommandBetween can be refactored to use dockerRunCommandContaining with marker signatures, reducing duplication
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare both functions in test/helpers/hermes-dockerfile-run.ts
  • Missing regression test: N/A - code quality
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Lines 55-85 (dockerRunCommandBetween) vs lines 100-130 (dockerRunCommandContaining) - similar parsing logic

PRA-8 Improvement — Add explicit types for test-specific env vars after @ts-nocheck removal

  • Location: test/sandbox-provisioning.test.ts:1
  • Category: tests
  • Problem: The @ts-nocheck directive was removed from sandbox-provisioning.test.ts (good). However, the file still uses some type-unsafe patterns (e.g., process.env access without guards for NEMOCLAW_FAKE_DOCKER_LOG). Consider adding explicit types for env vars used in tests.
  • Impact: Type safety gaps in test code; could miss typos in env var names.
  • Suggested action: Add type declarations for NEMOCLAW_FAKE_DOCKER_LOG and other test-specific env vars, or use a typed env helper
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check test/sandbox-provisioning.test.ts for process.env usage (e.g., line 161: process.env.NEMOCLAW_FAKE_DOCKER_LOG)
  • Missing regression test: N/A - code quality
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: @ts-nocheck removed at line 1; process.env.NEMOCLAW_FAKE_DOCKER_LOG used at line 161 without type guard
Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-4 shrink (agents/hermes/Dockerfile:268): REMOVAL_CHECKLIST comment-only enforcement
    • Replacement: Automated check in verify_dockerfile_base_digest_contract() or new test
    • Net: 10 lines
    • Safety boundary: Must not weaken the existing digest guard case statement that validates BASE_IMAGE at build time
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Resolver action integration test: verify resolve-hermes-base-image correctly exports immutable digest to HERMES_BASE_IMAGE env when GHCR digest exists. Unit tests thoroughly cover logic (digest guard, allowlist, symlink refusal, cleanup success). Runtime validation recommended for resolver action integration (GHCR pull -> digest inspect -> env export) and self-hosted workflow job execution (docker build + verifier script).
  • PRA-T2 Runtime validation — Workflow job integration test: verify build-hermes-stale-openclaw-image job passes with real stale base from GHCR. Unit tests thoroughly cover logic (digest guard, allowlist, symlink refusal, cleanup success). Runtime validation recommended for resolver action integration (GHCR pull -> digest inspect -> env export) and self-hosted workflow job execution (docker build + verifier script).
  • PRA-T3 Runtime validation — REMOVAL_CHECKLIST enforcement test: fail when BASE_IMAGE default digest > NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST but cleanup block remains. Unit tests thoroughly cover logic (digest guard, allowlist, symlink refusal, cleanup success). Runtime validation recommended for resolver action integration (GHCR pull -> digest inspect -> env export) and self-hosted workflow job execution (docker build + verifier script).
  • PRA-T4 Evaluate consolidating dockerRunCommandContaining with dockerRunCommandBetween — Evaluate if dockerRunCommandBetween can be refactored to use dockerRunCommandContaining with marker signatures, reducing duplication
  • PRA-T5 Add explicit types for test-specific env vars after @ts-nocheck removal — Add type declarations for NEMOCLAW_FAKE_DOCKER_LOG and other test-specific env vars, or use a typed env helper
Since last review details

Current findings, using the urgency labels above:

PRA-1 Improvement — Add $() command substitution to unsafe character class in require_safe_image_ref

  • Location: scripts/verify-hermes-stale-openclaw-image.sh:58
  • Category: security
  • Problem: The unsafe character check rejects backticks, semicolons, quotes, and backslashes but not $() command substitution syntax. While the subsequent allowlist restricts to known-safe patterns, adding $() provides defense-in-depth against shell injection via image refs.
  • Impact: Low-risk defense-in-depth gap; allowlist already restricts to safe patterns but explicit rejection of $() would catch injection attempts earlier.
  • Suggested action: Add '\$\(' to the unsafe character pattern in the case statement: *[[:space:]]* | *[;\`"'$()\\]*
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check line 58 in verify-hermes-stale-openclaw-image.sh for the case statement pattern
  • Missing regression test: Add a test case with 'ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@sha256:$(id)' to the unsafeRefs array in test/hermes-stale-openclaw-guard.test.ts
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 58: case "$ref" in *[[:space:]]* | *[;\`"'\\]*)

PRA-2 Resolve/justify — Document/verify self-hosted runner docker daemon isolation for new job

  • Location: .github/workflows/pr-self-hosted.yaml:118
  • Category: security
  • Problem: The new build-hermes-stale-openclaw-image job runs on self-hosted runner 'linux-amd64-cpu4' with docker daemon access. While persist-credentials: false is set on checkout, the job runs docker build which requires privileged docker access. The runner environment's docker daemon isolation from other workloads should be verified/documented.
  • Impact: If docker daemon is shared across workloads, a compromised build could affect other jobs or access cached layers from other projects.
  • Recommended action: Confirm self-hosted runner docker daemon isolation is documented in runner onboarding docs. Add a comment in the workflow noting the docker privilege requirement and isolation expectation.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Review .github/workflows/pr-self-hosted.yaml lines 118-145 and runner onboarding documentation at https://docs.gha-runners.nvidia.com/
  • Missing regression test: N/A - infrastructure concern; verify via runner documentation review
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Review .github/workflows/pr-self-hosted.yaml lines 118-145 and runner onboarding documentation at https://docs.gha-runners.nvidia.com/.
  • Evidence: Job build-hermes-stale-openclaw-image at lines 118-145 uses runs-on: linux-amd64-cpu4 with docker build commands

PRA-3 Improvement — Document bash requirement in verifier script shebang comment

  • Location: scripts/verify-hermes-stale-openclaw-image.sh:1
  • Category: security
  • Problem: The verify script uses bash-specific features (process substitution 'exec > >(tee...)', [[ ]] extended tests, arrays). The shebang is #!/bin/bash which is correct, but the requirement should be explicitly documented for future maintainers.
  • Impact: Maintainability risk if script is ever run with sh instead of bash; process substitution would fail.
  • Suggested action: Add a comment near the shebang noting 'Requires bash for process substitution and extended test operators'
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check line 1 of verify-hermes-stale-openclaw-image.sh
  • Missing regression test: N/A - documentation only
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 1: #!/bin/bash with process substitution at line 13: exec > >(tee -a "$LOG_PATH") 2>&1

PRA-4 Improvement — Add CI enforcement for REMOVAL_CHECKLIST instead of comment-only

  • Location: agents/hermes/Dockerfile:268
  • Category: architecture
  • Problem: The REMOVAL_CHECKLIST in the Dockerfile (lines 268-278) is excellent for technical debt management but exists only as a comment. A CI check should fail when the default BASE_IMAGE digest advances past the stale digest but the cleanup block hasn't been removed.
  • Impact: Technical debt could persist indefinitely without automated enforcement; the workaround should be removed automatically when criteria are met.
  • Suggested action: Add a check in verify_dockerfile_base_digest_contract() or a new test that compares the default BASE_IMAGE digest against NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST and warns/fails if the default is newer but cleanup remains
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check Dockerfile lines 1-6 (ARGs) and 268-278 (REMOVAL_CHECKLIST comment)
  • Missing regression test: Add a test that parses both ARGs from Dockerfile and asserts cleanup removal when default digest > stale digest
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Dockerfile lines 268-278: REMOVAL_CHECKLIST with 4 criteria but no automated enforcement

PRA-5 Required — Verify local/verifier base exemption in digest guard is intentional and tested

  • Location: agents/hermes/Dockerfile:279
  • Category: acceptance
  • Problem: The stale cleanup block validates BASE_IMAGE against the tracked stale digest, but the case statement allows 'nemoclaw-hermes-base-local' and verifier tags unconditionally. If a local build is done from a base that has the stale layout but a different digest, the cleanup would run without the digest guard. The verify script builds synthetic stale bases from the resolved BASE_IMAGE, so this is covered in CI, but the Dockerfile alone could accept a mismatched local base.
  • Impact: Local builds with mismatched stale-layout bases could bypass the digest guard, though CI verifier catches this by building synthetic stale bases from the resolved BASE_IMAGE.
  • Required action: The current design is intentional per comments (local/verifier bases are exempt). Verify the verify script's synthetic base construction correctly replicates the stale layout from whatever BASE_IMAGE is provided. This appears correct - build_stale_dir_base and build_stale_link_base both use the resolved BASE_IMAGE as FROM.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check verify script lines 95-115 (build_stale_dir_base, build_stale_link_base) - they use ${BASE_IMAGE} as the FROM image
  • Missing regression test: The verify script already covers this by building synthetic stale bases from the resolved BASE_IMAGE
  • Done when: The required change is committed and verification passes: Check verify script lines 95-115 (build_stale_dir_base, build_stale_link_base) - they use ${BASE_IMAGE} as the FROM image.
  • Evidence: Dockerfile lines 291-294: case statement allows nemoclaw-hermes-base-local and verifier tags without digest check; verify script lines 95-115 build synthetic bases from resolved BASE_IMAGE

PRA-6 Resolve/justify — Document why Hermes resolver is stricter than sandbox resolver and verify local fallback

  • Location: .github/actions/resolve-hermes-base-image/action.yaml:45
  • Category: acceptance
  • Problem: The resolver action now requires immutable GHCR digests (rejects :latest and tag refs when stale cleanup is active). This is a behavior change from the sandbox base resolver which allows tags. Ensure this is documented and that the fallback to local build works correctly when no compatible digest is found.
  • Impact: If no compatible GHCR digest exists, the action falls back to local build. This path should be tested to ensure it works correctly.
  • Recommended action: Add a comment in the action explaining why Hermes resolver is stricter than sandbox resolver (stale cleanup requires digest pinning). Verify the local build fallback path works by testing with no GHCR digests available.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare .github/actions/resolve-hermes-base-image/action.yaml with .github/actions/resolve-sandbox-base-image/action.yaml
  • Missing regression test: Add an integration test that simulates no GHCR digests available and verifies local build fallback
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare .github/actions/resolve-hermes-base-image/action.yaml with .github/actions/resolve-sandbox-base-image/action.yaml.
  • Evidence: Hermes resolver lines 45-48: requires digest_ref with ${image}@sha256: pattern; sandbox resolver allows tags via try_image()

PRA-7 Improvement — Evaluate consolidating dockerRunCommandContaining with dockerRunCommandBetween

  • Location: test/helpers/hermes-dockerfile-run.ts:100
  • Category: tests
  • Problem: The new dockerRunCommandContaining function extracts Dockerfile RUN commands by signature. This is a useful helper but duplicates some logic with dockerRunCommandBetween. Consider if both are needed or if one can be generalized.
  • Impact: Code duplication in test helpers; maintenance burden if Dockerfile parsing logic diverges.
  • Suggested action: Evaluate if dockerRunCommandBetween can be refactored to use dockerRunCommandContaining with marker signatures, reducing duplication
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare both functions in test/helpers/hermes-dockerfile-run.ts
  • Missing regression test: N/A - code quality
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Lines 55-85 (dockerRunCommandBetween) vs lines 100-130 (dockerRunCommandContaining) - similar parsing logic

PRA-8 Improvement — Add explicit types for test-specific env vars after @ts-nocheck removal

  • Location: test/sandbox-provisioning.test.ts:1
  • Category: tests
  • Problem: The @ts-nocheck directive was removed from sandbox-provisioning.test.ts (good). However, the file still uses some type-unsafe patterns (e.g., process.env access without guards for NEMOCLAW_FAKE_DOCKER_LOG). Consider adding explicit types for env vars used in tests.
  • Impact: Type safety gaps in test code; could miss typos in env var names.
  • Suggested action: Add type declarations for NEMOCLAW_FAKE_DOCKER_LOG and other test-specific env vars, or use a typed env helper
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check test/sandbox-provisioning.test.ts for process.env usage (e.g., line 161: process.env.NEMOCLAW_FAKE_DOCKER_LOG)
  • Missing regression test: N/A - code quality
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: @ts-nocheck removed at line 1; process.env.NEMOCLAW_FAKE_DOCKER_LOG used at line 161 without type guard

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 28261421550
Target ref: a6d8b17d8c54e52dda82e2acba3acb418df24521
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ⚠️ Run cancelled — no signal

Run: 28261609498
Target ref: ``
Requested jobs: onboard-negative-paths-e2e,messaging-providers-e2e,channels-add-remove-e2e,hermes-discord-e2e
Summary: 0 passed, 0 failed, 4 cancelled, 0 skipped

Job Result
channels-add-remove-e2e ⚠️ cancelled
hermes-discord-e2e ⚠️ cancelled
messaging-providers-e2e ⚠️ cancelled
onboard-negative-paths-e2e ⚠️ cancelled

ericksoa added 5 commits June 26, 2026 14:18
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 28266450648
Target ref: 989dc46c3fe51e55a6f188854338ef8d1405a68d
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 28266673637
Target ref: 2ca0288f8fdc98b611d583ca909eceafe16c9b5a
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e,hermes-secret-boundary-e2e
Summary: 2 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ✅ success
hermes-secret-boundary-e2e ✅ success

ericksoa added 2 commits June 26, 2026 14:45
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
scripts/verify-hermes-stale-openclaw-image.sh (1)

90-97: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Avoid dropping errexit for the expected-failure build.

Capture the non-zero docker build with an if instead of toggling set +e; that keeps the script fail-fast if anything else gets added to this block later. As per path instructions, the workflow docs call for Bash hardening with explicit error handling.

Suggested change
-  set +e
-  docker build -f "${REPO_ROOT}/agents/hermes/Dockerfile" \
-    --build-arg "BASE_IMAGE=${STALE_LINK_BASE}" \
-    -t "$STALE_LINK_IMAGE" \
-    "$REPO_ROOT" \
-    >"$SYMLINK_BUILD_LOG" 2>&1
-  local status="$?"
-  set -e
-
-  if [ "$status" -eq 0 ]; then
+  if docker build -f "${REPO_ROOT}/agents/hermes/Dockerfile" \
+    --build-arg "BASE_IMAGE=${STALE_LINK_BASE}" \
+    -t "$STALE_LINK_IMAGE" \
+    "$REPO_ROOT" \
+    >"$SYMLINK_BUILD_LOG" 2>&1; then
     cat "$SYMLINK_BUILD_LOG" >&2
     fail "Hermes final image unexpectedly built from stale-symlink base"
   fi
🤖 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/verify-hermes-stale-openclaw-image.sh` around lines 90 - 97, The
expected-failure build in verify-hermes-stale-openclaw-image.sh is disabling
errexit with set +e, which weakens fail-fast behavior for the whole block.
Update the docker build handling in the stale image check to use an explicit if
around docker build (capturing the non-zero status from that command only) and
remove the temporary errexit toggle so the script remains hardened if more
commands are added later.

Sources: Path instructions, Linters/SAST tools

test/pr-workflow-contract.test.ts (1)

641-649: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the failure-log artifact step in this contract test too.

The workflow change also adds failure diagnostics, but this test only locks in the runner and main script path. Add an assertion for the upload-artifact step so regressions to the log-upload contract get caught here as well. As per path instructions, failure diagnostics should upload scoped logs with if-no-files-found: ignore.

Suggested change
   it("runs Hermes stale OpenClaw image validation in self-hosted PR CI", () => {
     const job = prSelfHostedWorkflow.jobs["build-hermes-stale-openclaw-image"];
     const runs = stepRuns(job).join("\n");
+    const uploadStep = (job.steps ?? []).find(
+      (step) => step.name === "Upload Hermes stale OpenClaw image log on failure",
+    );
 
     expect(job["runs-on"]).toBe("linux-amd64-cpu4");
     expect(job["timeout-minutes"]).toBe(30);
     expect(stepUses(job)).toContain("./.github/actions/resolve-hermes-base-image");
     expect(runs).toContain("bash scripts/verify-hermes-stale-openclaw-image.sh");
+    expect(uploadStep?.if).toBe("failure()");
+    expect(uploadStep?.uses).toBe(
+      "actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a",
+    );
+    expect(uploadStep?.with?.["if-no-files-found"]).toBe("ignore");
   });
🤖 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/pr-workflow-contract.test.ts` around lines 641 - 649, The contract test
for build-hermes-stale-openclaw-image only checks the runner and main
verification script, so it misses the new failure-log upload behavior. Update
the test around prSelfHostedWorkflow.jobs["build-hermes-stale-openclaw-image"]
and stepRuns/stepUses to also assert the upload-artifact step is present and
configured for scoped logs with if-no-files-found set to ignore. Use the
existing job inspection helpers in this test file to locate the artifact step
and lock in the log-upload contract.

Source: Path instructions

.github/workflows/pr-self-hosted.yaml (1)

94-95: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Disable checkout credential persistence for this job.

This job only needs the workspace contents, so keeping the default persisted token is unnecessary. Setting persist-credentials: false aligns with the workflow guidance to avoid passing tokens unless needed. As per path instructions, “avoid passing GITHUB_TOKEN unless needed”; the zizmor hint here is pointing at that same gap.

Suggested change
       - name: Checkout
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
+        with:
+          persist-credentials: false
🤖 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 @.github/workflows/pr-self-hosted.yaml around lines 94 - 95, The Checkout
step in the self-hosted workflow is persisting credentials unnecessarily; update
the actions/checkout usage in this job to disable credential persistence. Keep
the workspace checkout behavior the same, but set persist-credentials to false
on the Checkout step so the job does not retain the default token.

Sources: Path instructions, Linters/SAST tools

🤖 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 @.github/workflows/pr-self-hosted.yaml:
- Around line 94-95: The Checkout step in the self-hosted workflow is persisting
credentials unnecessarily; update the actions/checkout usage in this job to
disable credential persistence. Keep the workspace checkout behavior the same,
but set persist-credentials to false on the Checkout step so the job does not
retain the default token.

In `@scripts/verify-hermes-stale-openclaw-image.sh`:
- Around line 90-97: The expected-failure build in
verify-hermes-stale-openclaw-image.sh is disabling errexit with set +e, which
weakens fail-fast behavior for the whole block. Update the docker build handling
in the stale image check to use an explicit if around docker build (capturing
the non-zero status from that command only) and remove the temporary errexit
toggle so the script remains hardened if more commands are added later.

In `@test/pr-workflow-contract.test.ts`:
- Around line 641-649: The contract test for build-hermes-stale-openclaw-image
only checks the runner and main verification script, so it misses the new
failure-log upload behavior. Update the test around
prSelfHostedWorkflow.jobs["build-hermes-stale-openclaw-image"] and
stepRuns/stepUses to also assert the upload-artifact step is present and
configured for scoped logs with if-no-files-found set to ignore. Use the
existing job inspection helpers in this test file to locate the artifact step
and lock in the log-upload contract.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 31bdd849-306f-4986-8c2d-35075e4b0d87

📥 Commits

Reviewing files that changed from the base of the PR and between 6e0acc5 and 9c48e06.

📒 Files selected for processing (4)
  • .github/workflows/pr-self-hosted.yaml
  • agents/hermes/Dockerfile
  • scripts/verify-hermes-stale-openclaw-image.sh
  • test/pr-workflow-contract.test.ts

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 28269063627
Target ref: 9c48e06079be272ce29f9190f43dbdd925eed32f
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 28269247005
Target ref: 396471a1b9e4ef47df66793181b3c725d08d4061
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ✅ success

ericksoa added 4 commits June 26, 2026 15:56
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/hermes-stale-openclaw-guard.test.ts`:
- Around line 16-39: The helper dockerRunCommandContaining in the test file is
introducing extra if statements that trip the test-conditionals:scan check. Move
this parsing logic into a non-.test.ts utility module and keep the test focused
on assertions, or rewrite the helper to avoid the new conditionals entirely
while preserving the same behavior. Use dockerRunCommandContaining as the main
symbol to relocate or simplify.
🪄 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: a6b14f8b-c579-449d-bada-95c439bee434

📥 Commits

Reviewing files that changed from the base of the PR and between 2837307 and 9e453b9.

📒 Files selected for processing (5)
  • .github/actions/resolve-hermes-base-image/action.yaml
  • agents/hermes/Dockerfile
  • scripts/verify-hermes-stale-openclaw-image.sh
  • test/hermes-stale-openclaw-guard.test.ts
  • test/pr-workflow-contract.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • agents/hermes/Dockerfile
  • scripts/verify-hermes-stale-openclaw-image.sh

Comment thread test/hermes-stale-openclaw-guard.test.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ⚠️ Run cancelled — no signal

Run: 28270862837
Target ref: 9e453b9eda9b3ad491d211f00a15b86192bf59c3
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e
Summary: 0 passed, 0 failed, 1 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ⚠️ cancelled

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 28272918335
Target ref: 9642941630db6edbfbaab327332aadb962fcac99
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e,hermes-secret-boundary-e2e
Summary: 0 passed, 2 failed, 0 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ❌ failure
hermes-secret-boundary-e2e ❌ failure

Failed jobs: hermes-root-entrypoint-smoke-e2e, hermes-secret-boundary-e2e. Check run artifacts for logs.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
test/helpers/hermes-dockerfile-run.ts (1)

77-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make dockerRunCommandContaining() find the enclosing RUN, not one exact prologue.

Line 82 only matches RUN set -eu;, so a harmless Dockerfile change like set -euo pipefail will break these guard tests even if the target signature is still in the same RUN block.

Suggested change
 export function dockerRunCommandContaining(dockerfile: string, signature: string): string {
   const signatureIndex = dockerfile.indexOf(signature);
   if (signatureIndex === -1) {
     throw new Error(`Expected Dockerfile RUN signature: ${signature}`);
   }
-  const runIndex = dockerfile.lastIndexOf("RUN set -eu;", signatureIndex);
-  if (runIndex === -1) {
+  const previousRunIndex = dockerfile.lastIndexOf("\nRUN ", signatureIndex);
+  const runIndex =
+    previousRunIndex === -1 && dockerfile.startsWith("RUN ")
+      ? 0
+      : previousRunIndex + 1;
+  if (runIndex <= 0 && !dockerfile.startsWith("RUN ")) {
     throw new Error(`Expected RUN instruction before ${signature}`);
   }
   const linesAfterRun = dockerfile.slice(runIndex).split("\n");
🤖 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/hermes-dockerfile-run.ts` around lines 77 - 97,
dockerRunCommandContaining() is too strict because it only searches for the
exact prologue "RUN set -eu;" before the target signature, so valid Dockerfile
variations can fail. Update the helper to locate the enclosing RUN instruction
more flexibly by identifying the nearest preceding RUN block around the
signature, rather than matching one exact shell prefix; keep the behavior in
dockerRunCommandContaining() and its related parsing logic intact.
test/hermes-stale-openclaw-guard.test.ts (1)

23-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse runDockerShell() here so root-only commands stay mocked.

This test hand-rolls its own Bash wrapper, so it drops the helper’s chown stub. If the cleanup block ever moves a privileged step before the digest check, this assertion will start failing for permissions instead of the digest guard.

Suggested change
-    const script = [
-      "#!/usr/bin/env bash",
-      "set -euo pipefail",
-      `BASE_IMAGE=${JSON.stringify(`ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@${DIFFERENT_DIGEST}`)}`,
-      `NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST=${JSON.stringify(STALE_DIGEST)}`,
-      cleanupCommand.replaceAll("/sandbox", sandboxRoot),
-    ].join("\n");
-    const scriptPath = path.join(tmp, "run-cleanup.sh");
     fs.mkdirSync(sandboxRoot, { recursive: true });
-    fs.writeFileSync(scriptPath, script, { mode: 0o700 });
 
     try {
-      const result = spawnSync("bash", [scriptPath], { encoding: "utf-8", timeout: 5000 });
+      const { result } = runDockerShell(
+        [
+          `BASE_IMAGE=${JSON.stringify(`ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@${DIFFERENT_DIGEST}`)}`,
+          `NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST=${JSON.stringify(STALE_DIGEST)}`,
+          cleanupCommand,
+        ].join("; "),
+        sandboxRoot,
+      );
       expect(result.status).toBe(1);
       expect(result.stderr).toContain("remove stale Hermes .openclaw cleanup or update");
       expect(result.stderr).toContain(DIFFERENT_DIGEST);
🤖 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/hermes-stale-openclaw-guard.test.ts` around lines 23 - 46, The stale
digest guard test is bypassing the shared Docker shell mock setup by
hand-rolling its own Bash wrapper, which leaves root-only commands like chown
unstubbed. Update the Hermes stale cleanup test to use runDockerShell() and its
existing mocking behavior, while still injecting the BASE_IMAGE and
NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST values and asserting the digest-guard
failure through dockerRunCommandContaining and STALE_CLEANUP_SIGNATURE.
🤖 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/hermes-dockerfile-run.ts`:
- Around line 77-97: dockerRunCommandContaining() is too strict because it only
searches for the exact prologue "RUN set -eu;" before the target signature, so
valid Dockerfile variations can fail. Update the helper to locate the enclosing
RUN instruction more flexibly by identifying the nearest preceding RUN block
around the signature, rather than matching one exact shell prefix; keep the
behavior in dockerRunCommandContaining() and its related parsing logic intact.

In `@test/hermes-stale-openclaw-guard.test.ts`:
- Around line 23-46: The stale digest guard test is bypassing the shared Docker
shell mock setup by hand-rolling its own Bash wrapper, which leaves root-only
commands like chown unstubbed. Update the Hermes stale cleanup test to use
runDockerShell() and its existing mocking behavior, while still injecting the
BASE_IMAGE and NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST values and asserting the
digest-guard failure through dockerRunCommandContaining and
STALE_CLEANUP_SIGNATURE.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 85723319-7b5d-43bc-b4a8-7e7ee2f72278

📥 Commits

Reviewing files that changed from the base of the PR and between 2837307 and 08f50e2.

📒 Files selected for processing (7)
  • .github/actions/resolve-hermes-base-image/action.yaml
  • agents/hermes/Dockerfile
  • scripts/verify-hermes-stale-openclaw-image.sh
  • test/helpers/hermes-dockerfile-run.ts
  • test/hermes-stale-openclaw-guard.test.ts
  • test/pr-workflow-contract.test.ts
  • test/sandbox-provisioning.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • .github/actions/resolve-hermes-base-image/action.yaml
  • test/pr-workflow-contract.test.ts
  • agents/hermes/Dockerfile
  • test/sandbox-provisioning.test.ts

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 28273631331
Target ref: e6cec21143a065d04004498629e4349068a07b19
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ❌ failure

Failed jobs: hermes-root-entrypoint-smoke-e2e. Check run artifacts for logs.

@ericksoa

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ericksoa ericksoa added bug-fix PR fixes a bug or regression area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery area: packaging Packages, images, registries, installers, or distribution integration: hermes Hermes integration behavior integration: openclaw OpenClaw integration behavior platform: container Affects Docker, containerd, Podman, or images v0.0.70 labels Jun 27, 2026
@cv
cv enabled auto-merge (squash) June 28, 2026 18:25
@cv
cv merged commit 9f4bf23 into main Jun 28, 2026
46 checks passed
@cv
cv deleted the fix/hermes-openclaw-dir-cleanup branch June 28, 2026 18:29
cv added a commit that referenced this pull request Jun 29, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Advance the default Hermes final-image base to the newly published
post-stale-layout digest and retire the temporary digest-coupled repair
that intentionally rejected newer published bases. This restores Hermes
onboarding and live E2E builds after the `latest` base moved, while
keeping current-state fail-closed layout checks and the older
`.hermes-data` compatibility migration.

## Related Issue

Unblocks #5947. Follow-up to #5882.

## Changes

- Pin the default Hermes base to
`sha256:8dad3b989a9ed1e601743310b97be21be5f59f89f7913a47d04f3ec3c40b8ce6`,
whose published ARM64 image has neither `/sandbox/.openclaw` nor
`/sandbox/.hermes-data`.
- Remove the fired `NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST` guard and
temporary `/sandbox/.openclaw` repair; replace it with a small
fail-closed invariant that rejects retired OpenClaw state rather than
maintaining it.
- Keep published-image provenance in the resolver: export immutable
official GHCR digests and reject candidates containing `.openclaw` or
`.hermes-data` state, while preserving local rebuild and caller-selected
base refs in the final Dockerfile.
- Delete the dedicated stale-layout verifier job, script, helpers, and
tests now that their documented removal trigger has fired.
- Add a focused final-image layout suite covering OpenClaw-state
refusal, the retained `.hermes-data` migration, and symlink refusal;
keep immutable published-ref coverage at the resolver boundary and
extend live secret-boundary inspection to both retired paths.
- Reject nested symlinks before copying retained `.hermes-data`
compatibility state so migration cannot preserve links outside the
legacy tree.

## 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
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [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: internal published-image
lifecycle cleanup; CLI behavior, configuration, supported workflows, and
the documented Hermes state location are unchanged.
- [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: pending independent
review of the Hermes sandbox image boundary.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [ ] 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)

Verification evidence:

- Pulled and inspected the published ARM64 `8dad…` image;
`/sandbox/.openclaw` and `/sandbox/.hermes-data` are absent.
- Built `agents/hermes/Dockerfile` against the exact `8dad…` digest and
verified sandbox-user readability, runtime permissions, runtime
symlinks, and absence of both stale paths.
- Focused integration run passed 61 tests across
`hermes-final-image-layout`, `hermes-doctor-config-hash`,
`pr-workflow-contract`, and `sandbox-provisioning`.
- `npm run build:cli`, `npm run typecheck:cli`, `npm run checks`, and
`npm run test:projects:check` passed.
- Normal commit and push hooks passed for all commits, including the
full CLI test lane, shellcheck, hadolint, repository checks,
source-shape and test-size budgets, gitleaks, commitlint, and CLI
TypeScript checks.
- [Vitest E2E run
28339846140](https://github.com/NVIDIA/NemoClaw/actions/runs/28339846140)
passed Hermes install/inference, root entrypoint, and secret-boundary
jobs, then exposed that Dockerfile-level registry validation rejected
the forced local `:latest` base-cache rebuild. Commit `ce85e762a` moves
provenance back to the resolver boundary; [exact-head rerun
28340627350](https://github.com/NVIDIA/NemoClaw/actions/runs/28340627350)
passed all four jobs, including stale-base rebuild.
- Downloaded exact-head artifacts have no credential-shaped matches;
every command result in the rebuild artifact is zero, post-rebuild
inference returned `PONG`, backup scanning found no leaks, and all
scenario cleanups report no failures.
- Required documentation writer assessment, rerun after the fail-closed
review fixes, reported no user-facing docs changes needed.

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@miyoungc miyoungc mentioned this pull request Jun 30, 2026
21 tasks
jyaunches pushed a commit that referenced this pull request Jun 30, 2026
## Summary
Refreshes the v0.0.70 release docs from the release announcement and the
`v0.0.69..v0.0.70` commit range.
It also documents the `channels start` policy restoration behavior that
was missing from the shared OpenClaw and Hermes command references, and
bumps the Fern CLI version used for docs validation.

## Changes
- Replaced the stale `v0.0.70` release-notes entry with the actual
release themes, including CLI, onboarding, inference, messaging,
Windows, documentation, and release-validation changes.
- Documented that `channels start` reapplies the matching built-in
network policy preset before rebuild and rolls back to disabled if
policy restoration fails.
- Bumped `fern/fern.config.json` from `5.55.0` to `5.59.0` for the docs
refresh.
- Source summary:
- #5754 -> `docs/about/release-notes.mdx`: Notes Docker Desktop gateway
bridge retry behavior during onboarding.
- #5930 -> `docs/about/release-notes.mdx`: Links `nemoclaw use` default
sandbox selection to the command reference.
- #5948 -> `docs/about/release-notes.mdx`: Links reasoning-compatible
endpoint validation to inference documentation.
- #5950 -> `docs/about/release-notes.mdx`: Links Windows bootstrap WSL
recovery behavior to Windows preparation and troubleshooting docs.
- #5856 -> `docs/about/release-notes.mdx`: Notes rebuilt policy preset
registry repair.
- #5882 and #5949 -> `docs/about/release-notes.mdx`: Notes Hermes stale
base-image state repair.
- #6016 -> `docs/reference/commands.mdx`,
`docs/reference/commands-nemohermes.mdx`, and
`docs/manage-sandboxes/messaging-channels.mdx`: Documents channel policy
restoration and rollback on `channels start`.
- #5859 -> `docs/about/release-notes.mdx`: Links quickstart network
approval guidance.
- #5863 -> `docs/about/release-notes.mdx`: Links Teams allowlist
guidance in the messaging page.
- #5756, #5926, #6010, and #6011 -> `docs/about/release-notes.mdx`:
Summarizes the Vitest E2E validation cutover.

## 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
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: doc-only prose refresh with
no runtime behavior change.
- [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:

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)
- [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)

`npm run docs` exited 0 and Fern reported one existing light-mode accent
contrast warning.
`fern check --warnings` confirmed the warning is the site theme contrast
ratio, not content introduced by this PR.

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Hermes final image layout cleanup now removes inherited
`/sandbox/.openclaw` state from stale Hermes base images.
`Dockerfile.base` remains the source of truth for new base-image layout;
this final-image repair only covers invalid `.openclaw` state already
baked into older published Hermes base layers, where runtime migration
cannot depend on root privileges after OpenShell starts the sandbox as
the sandbox user. The `.openclaw` cleanup can be removed once the
minimum supported Hermes base digest is newer than the stale layout.

## Changes
- Remove stale `/sandbox/.openclaw` during Hermes final-image layout
repair.
- Reject symlinked stale OpenClaw state before cleanup and assert the
path is gone afterward.
- Enforce the stale-layout workaround removal trigger by checking the
default published Hermes base digest at build time.
- Add `scripts/verify-hermes-stale-openclaw-image.sh` plus a self-hosted
PR CI job that builds real synthetic stale-directory and stale-symlink
Hermes base images, validates the final-image runtime layout, and proves
the symlink path fails closed.
- Harden the new self-hosted validation job by disabling checkout
credential persistence and validating the verifier's resolved base-image
input.
- Keep existing Hermes provisioning coverage verifying stale `.openclaw`
removal while Hermes state permissions remain correct.

## 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: internal Hermes image layout
cleanup; no user-facing behavior or commands changed.
- [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: author self-review;
cleanup refuses symlinked `/sandbox/.openclaw`, removes only stale
non-symlink `.openclaw` state baked into older Hermes base images,
asserts removal, adds a default-base digest gate for workaround removal,
and does not expand credential, policy, or runtime egress behavior.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [ ] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)

Verification commands run:
- `bash -n scripts/verify-hermes-stale-openclaw-image.sh`
- `shellcheck scripts/verify-hermes-stale-openclaw-image.sh`
- `npx prek run --all-files --stage pre-push --skip tsc-plugin --skip
tsc-js --skip tsc-cli --skip version-tag-sync --skip test-cli --skip
test-plugin --skip source-shape-test-budget --skip test-file-size-budget
--skip test-skills-yaml`
-
`NEMOCLAW_HERMES_BASE_IMAGE=ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@sha256:60333c1982ad855d55887b4488e867eb343f3930a30aa8e0268e5397fc6f2926
bash scripts/verify-hermes-stale-openclaw-image.sh`
- `npm test -- test/hermes-doctor-config-hash.test.ts
test/pr-workflow-contract.test.ts test/sandbox-provisioning.test.ts`
- `npm test -- test/pr-workflow-contract.test.ts
test/sandbox-provisioning.test.ts`
- `npm run source-shape:check`
- `npm run test-size:check`
- `npm run test-conditionals:scan -- --top 25`
- `git diff --check`
- `npm test -- test/sandbox-provisioning.test.ts`
- `npm run build:cli`
- `npm test -- src/lib/onboard/sandbox-create-launch.test.ts
src/lib/onboard/openclaw-runtime-env.test.ts
test/hermes-doctor-config-hash.test.ts`
- `wc -l test/sandbox-provisioning.test.ts`
- `gh api /repos/NVIDIA/NemoClaw/commits/2837307a0 --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/98ea6fa72 --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/396471a1b --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/9c48e0607 --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/6e0acc56b --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/2ca0288f8 --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/989dc46c3 --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/f69014248 --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/b4bc2a1ab --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/060b3f3d2 --jq
'.commit.verification'`
- `gh api /repos/NVIDIA/NemoClaw/commits/a6d8b17d8 --jq
'.commit.verification'`

Note: `npx prek run --from-ref origin/main --to-ref HEAD` was started
and visible checks passed through gitleaks/markdownlint skip, but the
command stopped producing output and was interrupted rather than claimed
as passed.

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added a self-hosted CI job and a Hermes verifier to validate “stale
OpenClaw” cleanup behavior (directory cleanup and symlink detection).

* **Bug Fixes**
* Strengthened Hermes stale OpenClaw repair/guardrails with digest-based
enforcement for supported pinned bases.
* Improved Hermes base-image resolution to prefer immutable
`image@sha256:` digests when available.

* **Tests**
* Expanded Hermes stale OpenClaw regression coverage with new
provisioning options and reference-guarding assertions.
* Updated workflow/contract tests to verify the new job and base
resolver behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Advance the default Hermes final-image base to the newly published
post-stale-layout digest and retire the temporary digest-coupled repair
that intentionally rejected newer published bases. This restores Hermes
onboarding and live E2E builds after the `latest` base moved, while
keeping current-state fail-closed layout checks and the older
`.hermes-data` compatibility migration.

## Related Issue

Unblocks NVIDIA#5947. Follow-up to NVIDIA#5882.

## Changes

- Pin the default Hermes base to
`sha256:8dad3b989a9ed1e601743310b97be21be5f59f89f7913a47d04f3ec3c40b8ce6`,
whose published ARM64 image has neither `/sandbox/.openclaw` nor
`/sandbox/.hermes-data`.
- Remove the fired `NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST` guard and
temporary `/sandbox/.openclaw` repair; replace it with a small
fail-closed invariant that rejects retired OpenClaw state rather than
maintaining it.
- Keep published-image provenance in the resolver: export immutable
official GHCR digests and reject candidates containing `.openclaw` or
`.hermes-data` state, while preserving local rebuild and caller-selected
base refs in the final Dockerfile.
- Delete the dedicated stale-layout verifier job, script, helpers, and
tests now that their documented removal trigger has fired.
- Add a focused final-image layout suite covering OpenClaw-state
refusal, the retained `.hermes-data` migration, and symlink refusal;
keep immutable published-ref coverage at the resolver boundary and
extend live secret-boundary inspection to both retired paths.
- Reject nested symlinks before copying retained `.hermes-data`
compatibility state so migration cannot preserve links outside the
legacy tree.

## 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
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [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: internal published-image
lifecycle cleanup; CLI behavior, configuration, supported workflows, and
the documented Hermes state location are unchanged.
- [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: pending independent
review of the Hermes sandbox image boundary.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [ ] 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)

Verification evidence:

- Pulled and inspected the published ARM64 `8dad…` image;
`/sandbox/.openclaw` and `/sandbox/.hermes-data` are absent.
- Built `agents/hermes/Dockerfile` against the exact `8dad…` digest and
verified sandbox-user readability, runtime permissions, runtime
symlinks, and absence of both stale paths.
- Focused integration run passed 61 tests across
`hermes-final-image-layout`, `hermes-doctor-config-hash`,
`pr-workflow-contract`, and `sandbox-provisioning`.
- `npm run build:cli`, `npm run typecheck:cli`, `npm run checks`, and
`npm run test:projects:check` passed.
- Normal commit and push hooks passed for all commits, including the
full CLI test lane, shellcheck, hadolint, repository checks,
source-shape and test-size budgets, gitleaks, commitlint, and CLI
TypeScript checks.
- [Vitest E2E run
28339846140](https://github.com/NVIDIA/NemoClaw/actions/runs/28339846140)
passed Hermes install/inference, root entrypoint, and secret-boundary
jobs, then exposed that Dockerfile-level registry validation rejected
the forced local `:latest` base-cache rebuild. Commit `ce85e762a` moves
provenance back to the resolver boundary; [exact-head rerun
28340627350](https://github.com/NVIDIA/NemoClaw/actions/runs/28340627350)
passed all four jobs, including stale-base rebuild.
- Downloaded exact-head artifacts have no credential-shaped matches;
every command result in the rebuild artifact is zero, post-rebuild
inference returned `PONG`, backup scanning found no leaks, and all
scenario cleanups report no failures.
- Required documentation writer assessment, rerun after the fail-closed
review fixes, reported no user-facing docs changes needed.

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Refreshes the v0.0.70 release docs from the release announcement and the
`v0.0.69..v0.0.70` commit range.
It also documents the `channels start` policy restoration behavior that
was missing from the shared OpenClaw and Hermes command references, and
bumps the Fern CLI version used for docs validation.

## Changes
- Replaced the stale `v0.0.70` release-notes entry with the actual
release themes, including CLI, onboarding, inference, messaging,
Windows, documentation, and release-validation changes.
- Documented that `channels start` reapplies the matching built-in
network policy preset before rebuild and rolls back to disabled if
policy restoration fails.
- Bumped `fern/fern.config.json` from `5.55.0` to `5.59.0` for the docs
refresh.
- Source summary:
- NVIDIA#5754 -> `docs/about/release-notes.mdx`: Notes Docker Desktop gateway
bridge retry behavior during onboarding.
- NVIDIA#5930 -> `docs/about/release-notes.mdx`: Links `nemoclaw use` default
sandbox selection to the command reference.
- NVIDIA#5948 -> `docs/about/release-notes.mdx`: Links reasoning-compatible
endpoint validation to inference documentation.
- NVIDIA#5950 -> `docs/about/release-notes.mdx`: Links Windows bootstrap WSL
recovery behavior to Windows preparation and troubleshooting docs.
- NVIDIA#5856 -> `docs/about/release-notes.mdx`: Notes rebuilt policy preset
registry repair.
- NVIDIA#5882 and NVIDIA#5949 -> `docs/about/release-notes.mdx`: Notes Hermes stale
base-image state repair.
- NVIDIA#6016 -> `docs/reference/commands.mdx`,
`docs/reference/commands-nemohermes.mdx`, and
`docs/manage-sandboxes/messaging-channels.mdx`: Documents channel policy
restoration and rollback on `channels start`.
- NVIDIA#5859 -> `docs/about/release-notes.mdx`: Links quickstart network
approval guidance.
- NVIDIA#5863 -> `docs/about/release-notes.mdx`: Links Teams allowlist
guidance in the messaging page.
- NVIDIA#5756, NVIDIA#5926, NVIDIA#6010, and NVIDIA#6011 -> `docs/about/release-notes.mdx`:
Summarizes the Vitest E2E validation cutover.

## 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
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: doc-only prose refresh with
no runtime behavior change.
- [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:

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)
- [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)

`npm run docs` exited 0 and Fern reported one existing light-mode accent
contrast warning.
`fern check --warnings` confirmed the warning is the site theme contrast
ratio, not content introduced by this PR.

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: packaging Packages, images, registries, installers, or distribution area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior integration: openclaw OpenClaw integration behavior platform: container Affects Docker, containerd, Podman, or images

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants