fix(hermes): document API bearer token lifecycle and retrieval#7212
Conversation
Signed-off-by: harjoth <harjoth.khara@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughHermes documentation defines bearer-token lifecycle and retrieval behavior. Rebuild recovery emits a token-change notice, while tests cover messaging, token rotation, documentation, failure paths, and preservation of replacement-home credentials during state restoration. ChangesHermes token lifecycle
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant Rebuild
participant Gateway
participant Notice
Operator->>Rebuild: run sandbox rebuild
Rebuild->>Gateway: restore and verify Hermes state
Gateway-->>Rebuild: return post-restore status
Rebuild->>Notice: pass sandbox and Hermes agent
Notice-->>Operator: display token changed and gateway-token command
Operator->>Gateway: retrieve replacement token
Gateway-->>Operator: return new bearer token
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/commands.mdx`:
- Line 1553: Qualify the ordinary-restart bug-report condition so it only
reports token changes without sandbox replacement when the prior API server key
was present and valid. Preserve the expected rotation behavior for restarts with
a missing or malformed API_SERVER_KEY, while retaining the existing identity and
redacted mint-log collection for true bug reports.
🪄 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: 7ae4fe38-6f2b-44b0-a1fc-4459ecaddf17
📒 Files selected for processing (7)
docs/get-started/quickstart-hermes.mdxdocs/manage-sandboxes/recover-rebuild-sandboxes.mdxdocs/reference/commands.mdxsrc/lib/actions/sandbox/rebuild-post-restore-phase.test.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.tstest/hermes-api-key-lifecycle-docs.test.tstest/snapshot.test.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
A restart with a missing or malformed API_SERVER_KEY rotates the token by design, so the bug-report guidance now applies only when the prior key was present and valid. Signed-off-by: harjoth <harjoth.khara@gmail.com>
Signed-off-by: harjoth <harjoth.khara@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/e2e/live/rebuild-hermes.test.ts`:
- Around line 167-177: Update the token retrieval command in the rebuild test to
invoke the configured CLI path from HostCliClient.commandPath instead of
hard-coding nemoclaw. Preserve the existing token validation and hashing
behavior while ensuring NEMOCLAW_CLI_BIN-selected artifacts are used.
In `@test/hermes-api-key-lifecycle-docs.test.ts`:
- Line 46: Update the lifecycle assertion to inspect the Hermes subsection
returned by readHermesGatewayTokenSection() instead of the full commands value,
while preserving the existing expected text.
🪄 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: 4eedac69-9b59-44f6-bce0-92c56f402568
📒 Files selected for processing (6)
docs/reference/commands.mdxsrc/lib/actions/sandbox/rebuild-post-restore-phase.test.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.tstest/e2e/live/rebuild-hermes.test.tstest/hermes-api-key-lifecycle-docs.test.tstest/snapshot.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/actions/sandbox/rebuild-post-restore-phase.ts
- docs/reference/commands.mdx
The digest helper hard-coded `nemoclaw`, so a NEMOCLAW_CLI_BIN-selected test artifact was bypassed and the rotation check could validate a different installed binary. Interpolate host.commandPath like the other live tests. Signed-off-by: harjoth <harjoth.khara@gmail.com>
|
✨ Thanks for the fix. This documents the Hermes API bearer token lifecycle and retrieval path. Ready for maintainer review. Related open issues: Related open issues: |
|
I'll come back and trigger CI after the conflict is resolved. |
|
The documentation updates look good to me. Since this PR also introduces new E2E test coverage, I’ll defer that portion—and any related CI follow-up—to the development team. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
left a comment
There was a problem hiding this comment.
Validated the documentation contract, Hermes-only rebuild messaging, credential-boundary coverage, and the merged live-E2E integration on exact head 856e6ad. Local verification: 59 targeted tests passed, 4 Hermes rebuild support tests passed, docs built with 0 errors, and all commit/pre-push hooks passed. Sensitive-path review complete.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/manage-sandboxes/recover-rebuild-sandboxes.mdx (2)
142-149: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winDocument the token-handling boundary beside the retrieval command.
The workflow explains how to retrieve the replacement token but does not state that it is a password or that operators must not scrape or edit implementation-owned
.envfiles.Proposed fix
```bash $$nemoclaw <sandbox-name> gateway-token --quiet+Treat the bearer token as a password.
+Use this command instead of scraping or editing implementation-owned.envfiles.
+
Before post-restore repairs, NemoClaw verifies that the recreated sandbox still identifies as Hermes and exits nonzero if its identity does not match the rebuild target.</details> As per path instructions, keep this user-facing recovery workflow clear and task-oriented, including safe credential handling. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/manage-sandboxes/recover-rebuild-sandboxes.mdxaround lines 142 - 149,
Add the token-handling guidance immediately after the gateway-token retrieval
command: state that the bearer token must be treated as a password, and instruct
operators to use the command rather than scraping or editing
implementation-owned .env files. Keep the surrounding recovery workflow
unchanged and task-oriented.</details> <!-- cr-comment:v1:3effe602185a6d612526dce1 --> _Source: Path instructions_ --- `124-126`: _📐 Maintainability & Code Quality_ | _🟡 Minor_ | _⚡ Quick win_ **Format literal filesystem paths as code.** Wrap `/sandbox/.openclaw`, `/sandbox/.hermes`, and `/sandbox/user-data` in backticks so readers can distinguish literal paths from prose. <details> <summary>Proposed fix</summary> ```diff - such as /sandbox/.openclaw or /sandbox/.hermes. - It does not preserve files outside that recorded path, including /sandbox/user-data. + such as `/sandbox/.openclaw` or `/sandbox/.hermes`. + It does not preserve files outside that recorded path, including `/sandbox/user-data`.As per coding guidelines, use
codeformatting for commands, paths, flags, environment variables, file names, and literal values.🤖 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 `@docs/manage-sandboxes/recover-rebuild-sandboxes.mdx` around lines 124 - 126, Update the legacy sandbox recovery documentation so the literal paths `/sandbox/.openclaw`, `/sandbox/.hermes`, and `/sandbox/user-data` use inline code formatting, while leaving the surrounding prose unchanged.Source: Coding guidelines
test/e2e/live/rebuild-hermes.test.ts (2)
667-675: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBootstrap the CLI before requiring its entrypoint.
The existence assertion runs before
ensureRebuildHermesHostTools(), so a checkout lacking the CLI artifact fails immediately and the newnpm run build:clifallback is unreachable. Run the bootstrap first, then assert the configured CLI path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/live/rebuild-hermes.test.ts` around lines 667 - 675, Reorder the setup in the rebuild-Hermes test so ensureRebuildHermesHostTools(host) runs before the fs.existsSync(CLI_ENTRYPOINT) assertion. Keep the existing CLI path assertion afterward, ensuring the build:cli fallback can create the entrypoint before it is validated.
61-61: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winScope and restore the checked-out CLI override.
Line 61 mutates
process.envat module load without restoration, leaking into later tests;??=also preserves an inherited controller CLI path instead of pinning this test toCLI_ENTRYPOINT. Set the exact value in test-scoped setup and restore the prior value in teardown.As per coding guidelines, deterministic tests must “undo stubbed environments and globals.”
🤖 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/e2e/live/rebuild-hermes.test.ts` at line 61, Update the test setup surrounding the module-level NEMOCLAW_CLI_BIN assignment to set it explicitly to CLI_ENTRYPOINT rather than using ??=, then capture the previous environment value and restore or delete it during teardown. Keep the override scoped to this test suite so later tests retain their original environment.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@docs/manage-sandboxes/recover-rebuild-sandboxes.mdx`:
- Around line 142-149: Add the token-handling guidance immediately after the
gateway-token retrieval command: state that the bearer token must be treated as
a password, and instruct operators to use the command rather than scraping or
editing implementation-owned .env files. Keep the surrounding recovery workflow
unchanged and task-oriented.
- Around line 124-126: Update the legacy sandbox recovery documentation so the
literal paths `/sandbox/.openclaw`, `/sandbox/.hermes`, and `/sandbox/user-data`
use inline code formatting, while leaving the surrounding prose unchanged.
In `@test/e2e/live/rebuild-hermes.test.ts`:
- Around line 667-675: Reorder the setup in the rebuild-Hermes test so
ensureRebuildHermesHostTools(host) runs before the fs.existsSync(CLI_ENTRYPOINT)
assertion. Keep the existing CLI path assertion afterward, ensuring the
build:cli fallback can create the entrypoint before it is validated.
- Line 61: Update the test setup surrounding the module-level NEMOCLAW_CLI_BIN
assignment to set it explicitly to CLI_ENTRYPOINT rather than using ??=, then
capture the previous environment value and restore or delete it during teardown.
Keep the override scoped to this test suite so later tests retain their original
environment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 65ff42c8-27c8-4ab7-bf87-0985021cdff3
📒 Files selected for processing (4)
docs/manage-sandboxes/recover-rebuild-sandboxes.mdxdocs/reference/commands.mdxtest/e2e/live/rebuild-hermes-host-tools.tstest/e2e/live/rebuild-hermes.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/reference/commands.mdx
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `v0.0.91` changelog entry that was missed before the release tag was cut. Correct the custom-image compatibility guidance because the tagged code still accepts the legacy inference route selector instead of removing it in v0.0.91. ## Changes - Add `docs/changelog/2026-07-22.mdx` with the release summary and detailed security, rebuild, Hermes, DGX Station, and historical-validation changes from the published v0.0.91 announcement. - Link shipped behavior to the most specific published OpenClaw and Hermes documentation routes. - Correct the v0.0.90 changelog and command reference so they match the compatibility fallback present in the tagged v0.0.91 code without inventing a new removal version. - Keep the immutable v0.0.91 release tag unchanged; this is the documented post-release recovery path. Source summary: - [#7332](#7332), [#7289](#7289), and [#7294](#7294) -> `docs/changelog/2026-07-22.mdx`: Summarize completed-image `node-tar` remediation and current and historical container verification. - [#7213](#7213), [#7363](#7363), [#7366](#7366), and [#7369](#7369) -> `docs/changelog/2026-07-22.mdx`: Summarize trusted base preparation, backup reuse, deletion convergence, and rebuild confidence. - [#7212](#7212) -> `docs/changelog/2026-07-22.mdx`: Summarize the Hermes API bearer-token lifecycle and supported retrieval command. - [#7327](#7327) and [#7328](#7328) -> `docs/changelog/2026-07-22.mdx`: Summarize qualified DGX Station guidance and reproducible coding-agent installation instructions. - [#7355](#7355), [#7360](#7360), [#7362](#7362), and [#7364](#7364) -> `docs/changelog/2026-07-22.mdx`: Summarize restored historical OpenClaw upgrade and Hermes rebuild validation. - [#7189](#7189) -> `docs/changelog/2026-07-20.mdx`, `docs/reference/commands.mdx`: Correct its forward-looking removal deadline after v0.0.91 shipped with the documented legacy fallback still present. - [#7282](#7282), [#7306](#7306), and [#7341](#7341) need no additional user-guide update because they already update their owned contributor or user-facing text directly. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates dated changelog structure, SPDX syntax, version ordering, and published routes. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this documentation-only PR does not change Station preparation or runtime behavior. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` (6 passed). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not run; this is a focused documentation-only change. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — completed with 0 errors and 2 existing site-wide warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) — the native changelog entry uses the required parser-safe MDX SPDX comment and intentionally has no frontmatter. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated v0.0.91 guidance for custom Dockerfiles, including continued legacy compatibility and recommended migration to `NEMOCLAW_INFERENCE_PROVIDER_ID`. * Added release notes covering image security scanning, safer rebuild behavior, token management, DGX Station guidance, and deterministic release validation. * Clarified that existing custom images may continue using the legacy selector temporarily, with fallback removal planned for a future release. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: cjagwani <cjagwani@nvidia.com>
Summary
Documents the Hermes
API_SERVER_KEYlifecycle so operators can tell ordinary restart stability apart from expected rebuild rotation, and adds a rebuild-time notice pointing at the supported retrieval command. Before this change the reference described whatgateway-tokenprints but not when the value changes, and the headless quickstart told operators to read the token out of the generated Hermes environment — which contradicts the supported retrieval path.Related Issue
Fixes #7175
Changes
gateway-tokenHermes reference section: generated once per sandbox home, distinct across sandbox homes, preserved across a gateway restart, a sandbox stop and start, and a host OpenShell gateway restart.API_SERVER_KEYthat is missing or is not exactly 64 lowercase hexadecimal characters. Ordinary restarts that change the token without replacing the sandbox stay a bug, and the page says what evidence to collect.gateway-token --quietinstead of reading or editing.hermes/.env.rebuildreference section.gateway-tokenheading boundary rather than a whole-file match, so a correct phrase in the wrong section fails.API_SERVER_KEY, which is the structural guarantee behind rotation.Scope note: the issue also mentions a planned headless/server deployment guide. That page does not exist yet, so it is out of scope here.
Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts test/hermes-api-key-lifecycle-docs.test.ts test/snapshot.test.ts— 3 files, 57 tests passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only) — 0 errors. The 2 reported warnings are pre-existing and unrelated to this change: an unauthenticated redirect check that is skipped withoutFERN_TOKEN, and the site-wide light-mode accent contrast ratio.Coverage limitation
Rebuild rotation is proven structurally, not end to end. Two tests together establish it: state restore does not carry
.hermes/.envinto a replacement home, and separate sandbox homes mint distinct keys. No test drives a real rebuild and compares the token before and after. A live-E2E rotation and retrieval check intest/e2e/live/rebuild-hermes.test.tsis the natural follow-up;e2e-liveis opt-in and outside the required gate for code-changing PRs, so it is not included here. Ordinary-restart preservation keeps its existing direct coverage intest/hermes-runtime-api-key.test.ts.Signed-off-by: harjoth harjoth.khara@gmail.com
Summary by CodeRabbit
gateway-token --quietcommand, and suppresses the notice on certain verification/restore failure paths.--quietusage (not environment/dashboard tokens).