Skip to content

fix(hermes): normalize image executable modes for the metadata gate - #7721

Merged
jyaunches merged 1 commit into
mainfrom
fix/7708-hermes-image-mode-normalize
Jul 28, 2026
Merged

fix(hermes): normalize image executable modes for the metadata gate#7721
jyaunches merged 1 commit into
mainfrom
fix/7708-hermes-image-mode-normalize

Conversation

@Dongni-Yang

@Dongni-Yang Dongni-Yang commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

v0.0.96 Hermes onboarding aborts at the sandbox image build: the final-stage check_metadata gate fails with metadata is root:root 775, expected root:root 755 on hermes-wrapper.py and node-tar-image-scan.mts. Both files are git-tracked 100755 but check out group-writable (0775) under a umask-002 checkout, and COPY preserves the host file mode under the classic (non-BuildKit) builder the OpenShell gateway uses. Every other check_metadata-asserted file is normalized with an explicit chmod in the final stage; these two were the only ones relying on COPY mode preservation. This adds the missing normalization.

Related Issue

Closes #7708

Changes

  • agents/hermes/Dockerfile: add RUN chmod 0755 /usr/local/lib/nemoclaw/hermes-wrapper.py /scripts/checks/node-tar-image-scan.mts between the scan COPY and the check_metadata gate, matching the established "normalize in a RUN, verify with a later check_metadata" pattern (e.g. chmod 755 at line 211 → assertion at the gate). COPY --chmod is not used because it is BuildKit-only and the gateway's classic builder ignores it; a RUN chmod is builder-independent. The content-hash gate over hermes-wrapper.py is over file bytes, so the mode change does not affect it.
  • test/hermes-final-image-layout.test.ts: extend the existing Dockerfile-string contract test to assert the normalizing chmod 0755 RUN is present and ordered after the scan COPY and before check_metadata. Red before the Dockerfile change, green after.

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: build-internal image-layout fix; no user-facing docs surface
  • 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 maintainer review; the change only tightens two executables from 0775 to 0755 (removes the group-write bit) to satisfy the existing image-metadata gate, and does not relax any assertion
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: no user-facing docs surface; the Dockerfile comment and commit message were reviewed for accuracy and repo voice
  • Agent: Claude Code

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project integration test/hermes-final-image-layout.test.ts → 8 pass; the new contract assertion fails on the pre-fix Dockerfile (red→green verified). Root cause and fix confirmed with a local DOCKER_BUILDKIT=0 docker build: COPY yields root:root 775, chmod 0755 corrects it to root:root 755. hadolint clean (0 findings via hadolint/hadolint image).
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • 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)

Signed-off-by: Dongni Yang dongniy@nvidia.com

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Corrected file permissions in the Hermes sandbox image to ensure required tools run reliably.
    • Improved image validation by enforcing the expected permission settings before metadata checks.
  • Tests

    • Added coverage to verify permission normalization occurs in the correct build sequence.

The final-stage check_metadata gate asserts root:root 755 on
hermes-wrapper.py and node-tar-image-scan.mts, but both check out
group-writable (0775) under umask 002 and COPY carries that source mode
into the image, so onboarding aborts at the sandbox image build with
"metadata is root:root 775, expected root:root 755". Every other
metadata-gated file is normalized with an explicit chmod in the final
stage; these two were the only ones relying on COPY mode preservation.

Add the missing chmod before the gate, matching the existing pattern.
A RUN chmod is builder-independent; COPY --chmod requires BuildKit and
the OpenShell gateway builds with the classic engine. Pin the RUN and
its ordering in the Dockerfile-layout contract test.

Closes #7708

Signed-off-by: Dongni Yang <dongniy@nvidia.com>
@Dongni-Yang Dongni-Yang self-assigned this Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 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: 354b2a14-4811-43bd-930b-9dc485704a13

📥 Commits

Reviewing files that changed from the base of the PR and between 45da24e and 0709678.

📒 Files selected for processing (2)
  • agents/hermes/Dockerfile
  • test/hermes-final-image-layout.test.ts

📝 Walkthrough

Walkthrough

The Hermes Dockerfile now normalizes executable permissions before metadata checks. The final image layout test verifies the chmod 755 instruction targets both scripts and is ordered after scanning and before metadata validation.

Changes

Hermes image permissions

Layer / File(s) Summary
Permission normalization and validation
agents/hermes/Dockerfile, test/hermes-final-image-layout.test.ts
The Dockerfile applies chmod 755 to both executable scripts before metadata validation, and the contract test verifies the instruction’s placement.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested labels: integration: hermes, bug-fix, platform: container

Suggested reviewers: cv, aasthajh, brandonpelfrey

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: normalizing Hermes image executable modes before the metadata gate.
Linked Issues check ✅ Passed The Dockerfile and test changes directly address #7708 by normalizing both executables to 0755 before metadata checks.
Out of Scope Changes check ✅ Passed The PR stays narrowly focused on the Hermes mode normalization fix and its layout test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/7708-hermes-image-mode-normalize

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

@github-code-quality

github-code-quality Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 0709678 in the fix/7708-hermes-imag... branch remains at 96%, unchanged from commit 45da24e in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 0709678 in the fix/7708-hermes-imag... branch remains at 81%, unchanged from commit 45da24e in the main branch.

Show a code coverage summary of the most impacted files.
File main 45da24e fix/7708-hermes-imag... 0709678 +/-
src/lib/domain/.../connect-env.ts 97% 89% -8%
src/lib/sandbox...rce-identity.ts 88% 88% 0%
src/lib/state/m...-acquisition.ts 75% 75% 0%

Updated July 28, 2026 08:57 UTC

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 1 warning · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized E2E selections match; Nemotron reported the same number of blockers, 1 fewer warning, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-onboard, full-e2e, hermes-e2e, security-posture

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — No runtime regression covers copied executable modes

  • Location: test/hermes-final-image-layout.test.ts:210
  • Category: tests
  • Problem: The changed test verifies Dockerfile text ordering, but it does not build the final Hermes image or verify that the two copied executables are mode 0755 after Docker Engine cross-stage COPY.
  • Impact: A future change can preserve the source-shape ordering while Docker Engine image construction still produces group-writable executables or fails the metadata gate, blocking Hermes onboarding.
  • Recommendation: Add a runtime image-build regression that starts with group-writable payload executables and verifies the completed Hermes image passes the metadata gate with both paths owned by root and mode 0755.
  • Verification: Inspect a built Hermes image from the Docker Engine-compatible path and run stat on /usr/local/lib/nemoclaw/hermes-wrapper.py and /scripts/checks/node-tar-image-scan.mts.
  • Test coverage: Build the Hermes final image through the Docker Engine-compatible payload path with the two source executables at mode 0775, then assert the build succeeds and stat reports root:root 755 for both final-image paths.
  • Evidence: test/hermes-final-image-layout.test.ts:210 asserts the literal chmod instruction and its order only. agents/hermes/Dockerfile:707 requires both paths to satisfy root:root 755 in check_metadata. The deterministic risk plan lists platform-install and sandbox-boundary invariants for agents/hermes/Dockerfile.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@Dongni-Yang

Copy link
Copy Markdown
Contributor Author

On advisor warning PRA-1 (runtime regression for copied executable modes): the runtime guard it asks for already exists in the Dockerfile itself. check_metadata at the final stage asserts root:root 755 on both paths and fails the build on any mode drift — that failure is exactly the #7708 symptom this PR fixes. So Docker Engine producing group-writable executables cannot pass silently: it aborts the build, which is the regression signal, and the real onboarding e2e exercises that actual build.

The added assertion in test/hermes-final-image-layout.test.ts is a fast pre-build pin so the normalizing chmod (and its ordering before the gate) can't be silently dropped — it complements the gate rather than replacing it, and matches this file's established role as the Dockerfile-layout contract test. I did verify the runtime behavior directly with DOCKER_BUILDKIT=0 docker build: COPY yields root:root 775 and the chmod 755 corrects it to root:root 755. A dedicated docker-build unit test would duplicate check_metadata and isn't suitable for the unit lane, which has no guaranteed Docker daemon; a full Hermes image build belongs to the e2e lane (it needs the base image and ~15 min).

Signed-off-by: Dongni Yang dongniy@nvidia.com

@jyaunches
jyaunches merged commit 02b59e5 into main Jul 28, 2026
77 of 78 checks passed
@jyaunches
jyaunches deleted the fix/7708-hermes-image-mode-normalize branch July 28, 2026 12:51
@cjagwani cjagwani added v0.0.98 and removed v0.0.97 labels Jul 28, 2026
@cjagwani cjagwani mentioned this pull request Jul 28, 2026
23 tasks
cv pushed a commit that referenced this pull request Jul 28, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical dated changelog entry for NemoClaw v0.0.97 before the
release plan captures `origin/main`.
The entry groups the user-visible and maintainer-facing changes since
v0.0.96 while preserving the Deferred dual-Station status, experimental
runtime-identity boundary, and pending physical IGX validation.

## Changes

- Add `docs/changelog/2026-07-28.mdx` with the parser-safe MDX SPDX
comment and exact `## v0.0.97` heading.
- Summarize the 43 merged PRs in the release range, omitting
internal-only changes from the public entry and linking each grouped
change to its most specific published documentation.
- Keep the experimental Okta reference explicitly opt-in and outside
normal onboarding, keep the two-Station path Deferred, and state that
physical IGX Orin validation remains pending.

### Source summary

- [#7440](#7440),
[#7443](#7443), and
[#7445](#7445) ->
`docs/changelog/2026-07-28.mdx`: Document read-only host readiness
reports and fail-closed platform qualification.
- [#7030](#7030) ->
`docs/changelog/2026-07-28.mdx`: Document the Deferred trusted
two-Station vLLM evaluation.
- [#7265](#7265) ->
`docs/changelog/2026-07-28.mdx`: Document the bounded experimental
direct-runner Okta runtime-identity reference.
- [#7711](#7711) and
[#7648](#7648) ->
`docs/changelog/2026-07-28.mdx`: Document compatible-endpoint reasoning
effort and retired NVIDIA Build model paths.
- [#7746](#7746),
[#7763](#7763), and
[#7681](#7681) ->
`docs/changelog/2026-07-28.mdx`: Document safe compatible-provider
creation, replacement refusal, and narrow OpenShell bridge URL handling.
- [#7641](#7641),
[#7690](#7690),
[#7631](#7631), and
[#7710](#7710) ->
`docs/changelog/2026-07-28.mdx`: Document paused-container recovery,
recreation journaling, pre-mutation uninstall checks, and
source-checkout OpenShell selection.
- [#7624](#7624) and
[#7762](#7762) ->
`docs/changelog/2026-07-28.mdx`: Document Jetson release diagnostics and
bounded render-device group propagation.
- [#7639](#7639),
[#7760](#7760),
[#7721](#7721), and
[#7761](#7761) ->
`docs/changelog/2026-07-28.mdx`: Document Telegram, MCP media-type,
Hermes image-mode, and locked-restart fixes.
- [#7653](#7653) and
[#7680](#7680) ->
`docs/changelog/2026-07-28.mdx`: Document Deep Agents policy tasks and
the bounded Claude Code OAuth path.
- [#7679](#7679) ->
`docs/changelog/2026-07-28.mdx`: Document the checksum-bound libssh2 and
Python HTMLParser backports.
- [#7655](#7655),
[#7651](#7651),
[#7664](#7664),
[#7666](#7666),
[#7670](#7670),
[#7719](#7719), and
[#7741](#7741) ->
`docs/changelog/2026-07-28.mdx`: Document exact candidate E2E evidence,
Launchable selection, diagnostic consolidation, and trusted WSL
validation.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates the dated changelog contract,
MDX header, heading uniqueness, and release-entry structure.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: The committed `docs/changelog/2026-07-28.mdx` blob exactly
matches the reviewed file. Completeness, factual accuracy, link shape,
parser-safe MDX header, one-sentence-per-line style, `.docs-skip`
compliance, and bounded product claims passed.
- Agent: Codex Desktop documentation writer subagent
<!-- docs-review-head-sha: da6aa27 -->
<!-- docs-review-agents-blob-sha: be20a09 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; this PR changes only the dated
changelog.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run
test/changelog-docs.test.ts` passed 6/6.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable to this doc-only release
entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) —
completed with 0 errors and 2 pre-existing Fern warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)
— native changelog entries use the required parser-safe MDX SPDX comment
and intentionally have no frontmatter.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


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

## Summary by CodeRabbit

* **New Features**
* Added improved host readiness reporting and Jetson onboarding
guidance.
* Added controls for reasoning effort with compatible endpoints and
enhanced managed MCP discovery.
  * Improved Deep Agents task publication and preset support.
* **Bug Fixes**
* Hardened provider switching, sandbox recovery, uninstall behavior, and
Telegram connectivity.
* Improved container image integrity checks, media-type handling, and
checksum validation.
  * Enhanced vLLM evaluation behavior and release diagnostics.
* **Documentation**
  * Added the NemoClaw v0.0.97 changelog.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Linux][Onboard] v0.0.96 NemoHermes onboarding fails during sandbox image build

4 participants