Skip to content

Commit c4bd014

Browse files
authored
fix(sandbox/recover): enforce Hermes env-file secret boundary on probe path (#5530)
## Summary `nemohermes <name> recover` did not re-evaluate the documented Hermes secret boundary when the gateway was already running. `recover` → `connect --probe-only` → `checkAndRecoverSandboxProcesses` took the `if (running)` branch and returned after refreshing the port-forward, never invoking `buildHermesEnvFileBoundaryGuard()` (only the relaunch path embeds it). A raw `TELEGRAM_BOT_TOKEN` injected into `/sandbox/.hermes/.env` survived recovery and the gateway kept serving. The probe path now executes a standalone secret-boundary check via the root `openshell sandbox exec` channel (so the kill snippet has authority over the gateway-user process), brings the gateway down on refusal, and surfaces a non-zero exit on `recover`. ## Related Issue Fixes #5525 ## Changes - `hermes-recovery-boundary.ts` exports `buildHermesEnvFileBoundaryStandaloneCheck()` plus `SECRET_BOUNDARY_{OK,REFUSED,VALIDATOR_MISSING}_MARKER` constants. The standalone snippet runs the existing validator on `/sandbox/.hermes/.env`, kills any running Hermes gateway/dashboard on refusal, and emits a single stdout marker. - `process-recovery.ts` adds `enforceHermesSecretBoundaryOnRunningGateway`, invoked from `checkAndRecoverSandboxProcesses` before the `if (running)` early-return when the active agent is Hermes. Refusal short-circuits the function with `secretBoundaryRefused: true` and skips the forward-refresh path. - `connect.ts` `runSandboxConnectProbe` surfaces a refusal as exit 1 with a clear remediation line pointing at the `openshell:resolve:env:<name>` placeholder. - `docs/reference/commands.mdx` documents the Hermes-only secret-boundary re-evaluation on `recover`; the nemohermes variant is regenerated. - New tests: 4 in `process-recovery.test.ts` (refused / passed / validator-missing / OpenClaw no-op); 3 in `runtime-hermes-secret-boundary-behavioural.test.ts` exercising the standalone snippet against real bash with stubbed `python3`/`pkill`. ## 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) ## 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 - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `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) --- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated `recover` command documentation to describe secret boundary re-evaluation behavior. * **New Features** * Added Hermes secret boundary validation during the recovery process that checks for improperly configured raw secrets in environment files. * **Bug Fixes** * Improved error detection with clearer guidance to replace raw secret values with proper placeholders before recovery proceeds. * **Tests** * Added test coverage for secret boundary validation scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
1 parent adf91da commit c4bd014

8 files changed

Lines changed: 900 additions & 28 deletions

File tree

docs/reference/commands-nemohermes.mdx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,11 @@ If the gateway is already running, the command exits zero with a probe message a
501501
nemohermes my-assistant recover
502502
```
503503

504+
`recover` re-evaluates the documented Hermes secret boundary against `/sandbox/.hermes/.env` on every run, including when the gateway is already healthy.
505+
If the file contains raw secret-shaped values (for example a pasted Telegram, Discord, or Slack bot token in place of the expected `openshell:resolve:env:<name>` placeholder), the command stops the running gateway, exits non-zero, and prints the offending key.
506+
Replace each flagged value with the `openshell:resolve:env:<name>` placeholder and re-run.
507+
Older Hermes sandbox images that predate the standalone validator are detected and left untouched: `recover` prints a `[boundary]` warning naming the sandbox and noting that `/sandbox/.hermes/.env` was not re-evaluated, then proceeds with the rest of the recovery path rather than blocking the gateway. Re-image the sandbox to a current Hermes build to enable the per-run boundary re-evaluation described above.
508+
504509
### `nemohermes <name> status`
505510

506511
Show sandbox status, health, and inference configuration.

docs/reference/commands.mdx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,15 @@ If the gateway is already running, the command exits zero with a probe message a
630630
$$nemoclaw my-assistant recover
631631
```
632632

633+
<AgentOnly variant="hermes">
634+
635+
`recover` re-evaluates the documented Hermes secret boundary against `/sandbox/.hermes/.env` on every run, including when the gateway is already healthy.
636+
If the file contains raw secret-shaped values (for example a pasted Telegram, Discord, or Slack bot token in place of the expected `openshell:resolve:env:<name>` placeholder), the command stops the running gateway, exits non-zero, and prints the offending key.
637+
Replace each flagged value with the `openshell:resolve:env:<name>` placeholder and re-run.
638+
Older Hermes sandbox images that predate the standalone validator are detected and left untouched: `recover` prints a `[boundary]` warning naming the sandbox and noting that `/sandbox/.hermes/.env` was not re-evaluated, then proceeds with the rest of the recovery path rather than blocking the gateway. Re-image the sandbox to a current Hermes build to enable the per-run boundary re-evaluation described above.
639+
640+
</AgentOnly>
641+
633642
### `$$nemoclaw <name> status`
634643

635644
Show sandbox status, health, and inference configuration.

src/lib/actions/sandbox/connect-flow.test.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ type ConnectHarnessOptions = {
2929
wasRunning?: boolean;
3030
recovered?: boolean;
3131
forwardRecovered?: boolean;
32+
secretBoundaryRefused?: boolean;
33+
secretBoundaryReason?: "raw-secret" | "inconclusive";
3234
};
3335
spawnStatus?: number | null;
3436
};
@@ -222,4 +224,110 @@ describe("connectSandbox flow", () => {
222224
);
223225
expect(exitSpy).toHaveBeenCalledWith(1);
224226
});
227+
228+
it("probe-only mode exits with raw-secret remediation when the Hermes boundary refuses recovery", async () => {
229+
const harness = createConnectHarness({
230+
processCheck: {
231+
checked: true,
232+
wasRunning: true,
233+
recovered: false,
234+
forwardRecovered: false,
235+
secretBoundaryRefused: true,
236+
secretBoundaryReason: "raw-secret",
237+
},
238+
});
239+
const agentRuntime = requireDist("../../../../dist/lib/agent/runtime.js");
240+
vi.spyOn(agentRuntime, "getSessionAgent").mockReturnValue({ name: "hermes" });
241+
vi.spyOn(agentRuntime, "getAgentDisplayName").mockReturnValue("Hermes");
242+
const errorSpy = vi.spyOn(console, "error");
243+
244+
await expect(harness.connectSandbox("alpha", { probeOnly: true })).rejects.toThrow(
245+
"process.exit(1)",
246+
);
247+
248+
expect(harness.runAutoPairSpy).not.toHaveBeenCalled();
249+
expect(harness.spawnSyncSpy).not.toHaveBeenCalledWith(
250+
"openshell",
251+
["sandbox", "connect", "alpha"],
252+
expect.any(Object),
253+
);
254+
const errorOutput = errorSpy.mock.calls.map((call) => String(call[0] ?? "")).join("\n");
255+
expect(errorOutput).toContain(
256+
"Probe failed: refused to confirm Hermes gateway in 'alpha' — /sandbox/.hermes/.env contains raw secret-shaped values.",
257+
);
258+
expect(errorOutput).toContain(
259+
"Replace raw secret values with openshell:resolve:env:<name> placeholders and re-run.",
260+
);
261+
const logOutput = harness.logSpy.mock.calls.map((call) => String(call[0] ?? "")).join("\n");
262+
expect(logOutput).not.toContain("Probe complete");
263+
expect(exitSpy).toHaveBeenCalledWith(1);
264+
});
265+
266+
it("non-probe connect exits before Ollama/inference-route/auto-pair when the Hermes boundary refuses", async () => {
267+
const harness = createConnectHarness({
268+
processCheck: {
269+
checked: true,
270+
wasRunning: true,
271+
recovered: false,
272+
forwardRecovered: false,
273+
secretBoundaryRefused: true,
274+
secretBoundaryReason: "raw-secret",
275+
},
276+
});
277+
const agentRuntime = requireDist("../../../../dist/lib/agent/runtime.js");
278+
vi.spyOn(agentRuntime, "getSessionAgent").mockReturnValue({ name: "hermes" });
279+
vi.spyOn(agentRuntime, "getAgentDisplayName").mockReturnValue("Hermes");
280+
const errorSpy = vi.spyOn(console, "error");
281+
282+
await expect(harness.connectSandbox("alpha")).rejects.toThrow("process.exit(1)");
283+
284+
expect(harness.ensureOllamaAuthProxySpy).not.toHaveBeenCalled();
285+
expect(harness.runAutoPairSpy).not.toHaveBeenCalled();
286+
expect(harness.spawnSyncSpy).not.toHaveBeenCalledWith(
287+
"openshell",
288+
["sandbox", "connect", "alpha"],
289+
expect.any(Object),
290+
);
291+
const errorOutput = errorSpy.mock.calls.map((call) => String(call[0] ?? "")).join("\n");
292+
expect(errorOutput).toContain(
293+
"Connect failed: refused to confirm Hermes gateway in 'alpha' — /sandbox/.hermes/.env contains raw secret-shaped values.",
294+
);
295+
expect(errorOutput).toContain(
296+
"Replace raw secret values with openshell:resolve:env:<name> placeholders and re-run.",
297+
);
298+
expect(exitSpy).toHaveBeenCalledWith(1);
299+
});
300+
301+
it("probe-only mode exits with inconclusive guidance when the Hermes boundary check could not run", async () => {
302+
const harness = createConnectHarness({
303+
processCheck: {
304+
checked: true,
305+
wasRunning: true,
306+
recovered: false,
307+
forwardRecovered: false,
308+
secretBoundaryRefused: true,
309+
secretBoundaryReason: "inconclusive",
310+
},
311+
});
312+
const agentRuntime = requireDist("../../../../dist/lib/agent/runtime.js");
313+
vi.spyOn(agentRuntime, "getSessionAgent").mockReturnValue({ name: "hermes" });
314+
vi.spyOn(agentRuntime, "getAgentDisplayName").mockReturnValue("Hermes");
315+
const errorSpy = vi.spyOn(console, "error");
316+
317+
await expect(harness.connectSandbox("alpha", { probeOnly: true })).rejects.toThrow(
318+
"process.exit(1)",
319+
);
320+
321+
expect(harness.runAutoPairSpy).not.toHaveBeenCalled();
322+
const errorOutput = errorSpy.mock.calls.map((call) => String(call[0] ?? "")).join("\n");
323+
expect(errorOutput).toContain(
324+
"Probe failed: secret-boundary check did not complete for Hermes gateway in 'alpha'.",
325+
);
326+
expect(errorOutput).toContain(
327+
"Inspect the validator output above and re-run `nemoclaw <sandbox> recover`.",
328+
);
329+
const logOutput = harness.logSpy.mock.calls.map((call) => String(call[0] ?? "")).join("\n");
330+
expect(logOutput).not.toContain("Probe complete");
331+
expect(exitSpy).toHaveBeenCalledWith(1);
332+
});
225333
});

src/lib/actions/sandbox/connect.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import { findReachableOllamaHost, probeLocalProviderHealth } from "../../inferen
2727
import { ensureOllamaAuthProxy, probeOllamaAuthProxyHealth } from "../../inference/ollama/proxy";
2828
import { LOCAL_INFERENCE_TIMEOUT_SECS } from "../../onboard/env";
2929
import { resolveSandboxGatewayName } from "../../onboard/gateway-binding";
30-
import { getSandboxTargetGatewayName } from "./gateway-target";
3130
import { isWsl } from "../../platform";
3231
import { ROOT } from "../../runner";
3332
import * as sandboxVersion from "../../sandbox/version";
@@ -53,6 +52,7 @@ import {
5352
import { preflightVllmModelEnvOrExit } from "./connect-vllm-preflight";
5453
import { isDockerRuntimeDown, printDockerRuntimeDownGuidance } from "./gateway-failure-classifier";
5554
import { ensureLiveSandboxOrExit, printGatewayLifecycleHint } from "./gateway-state";
55+
import { getSandboxTargetGatewayName } from "./gateway-target";
5656
import { printGatewayWedgeDiagnostics } from "./gateway-wedge-diagnostics";
5757
import { checkAndRecoverSandboxProcesses, executeSandboxExecCommand } from "./process-recovery";
5858
import { applyOpenShellVmDnsMonkeypatch, shouldApplyVmDnsMonkeypatch } from "./vm-dns-monkeypatch";
@@ -183,6 +183,33 @@ export function parseSandboxConnectArgs(
183183
return options;
184184
}
185185

186+
function exitOnSecretBoundaryRefusal(
187+
sandboxName: string,
188+
agentName: string,
189+
processCheck: Record<string, unknown>,
190+
contextLabel: "Probe" | "Connect",
191+
): never {
192+
console.error("");
193+
const reason =
194+
"secretBoundaryReason" in processCheck
195+
? (processCheck.secretBoundaryReason as "raw-secret" | "inconclusive" | undefined)
196+
: undefined;
197+
if (reason === "raw-secret") {
198+
console.error(
199+
` ${contextLabel} failed: refused to confirm ${agentName} gateway in '${sandboxName}' — /sandbox/.hermes/.env contains raw secret-shaped values.`,
200+
);
201+
console.error(
202+
" Replace raw secret values with openshell:resolve:env:<name> placeholders and re-run.",
203+
);
204+
} else {
205+
console.error(
206+
` ${contextLabel} failed: secret-boundary check did not complete for ${agentName} gateway in '${sandboxName}'.`,
207+
);
208+
console.error(" Inspect the validator output above and re-run `nemoclaw <sandbox> recover`.");
209+
}
210+
process.exit(1);
211+
}
212+
186213
function runSandboxConnectProbe(sandboxName: string): void {
187214
const processCheck = checkAndRecoverSandboxProcesses(sandboxName, { quiet: true });
188215
const agent = agentRuntime.getSessionAgent(sandboxName);
@@ -193,6 +220,9 @@ function runSandboxConnectProbe(sandboxName: string): void {
193220
);
194221
process.exit(1);
195222
}
223+
if ("secretBoundaryRefused" in processCheck && processCheck.secretBoundaryRefused) {
224+
exitOnSecretBoundaryRefusal(sandboxName, agentName, processCheck, "Probe");
225+
}
196226
if (processCheck.wasRunning) {
197227
ensureSandboxInferenceRoute(sandboxName, { quiet: true });
198228
// Defense-in-depth scope-upgrade approval on the probe-only / `recover`
@@ -861,7 +891,11 @@ export async function connectSandbox(
861891
/* non-fatal — don't block connect on session detection failure */
862892
}
863893

864-
checkAndRecoverSandboxProcesses(sandboxName);
894+
const processCheck = checkAndRecoverSandboxProcesses(sandboxName);
895+
if ("secretBoundaryRefused" in processCheck && processCheck.secretBoundaryRefused) {
896+
const agentName = agentRuntime.getAgentDisplayName(agentRuntime.getSessionAgent(sandboxName));
897+
exitOnSecretBoundaryRefusal(sandboxName, agentName, processCheck, "Connect");
898+
}
865899
// Ensure Ollama auth proxy is running (recovers from host reboots)
866900
ensureOllamaAuthProxy();
867901

src/lib/actions/sandbox/process-recovery.ts

Lines changed: 123 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ import {
1111
runOpenshell,
1212
} from "../../adapters/openshell/runtime";
1313
import { OPENSHELL_PROBE_TIMEOUT_MS } from "../../adapters/openshell/timeouts";
14+
import {
15+
buildHermesEnvFileBoundaryStandaloneCheck,
16+
SECRET_BOUNDARY_OK_MARKER,
17+
SECRET_BOUNDARY_REFUSED_MARKER,
18+
SECRET_BOUNDARY_VALIDATOR_MISSING_MARKER,
19+
} from "../../agent/hermes-recovery-boundary";
1420
import * as agentRuntime from "../../agent/runtime";
1521
import { G, R } from "../../cli/terminal-style";
1622
import { DASHBOARD_PORT } from "../../core/ports";
@@ -482,12 +488,115 @@ function recoverHermesDashboardProcessIfEnabled(sandboxName: string): boolean |
482488
return recoverHermesDashboardProcess(sandboxName, { executeCommand: executeSandboxCommand });
483489
}
484490

491+
function isHermesAgent(agent: ReturnType<typeof agentRuntime.getSessionAgent>): boolean {
492+
return !!agent && agent.name === "hermes";
493+
}
494+
495+
type SecretBoundaryRefusalReason = "raw-secret" | "inconclusive";
496+
497+
type HermesSecretBoundaryEnforcement =
498+
| { refused: false }
499+
| { refused: true; reason: SecretBoundaryRefusalReason; stderr: string };
500+
501+
function printValidatorStderr(stderr: string): void {
502+
if (!stderr.trim()) return;
503+
for (const line of stderr.split(/\r?\n/)) {
504+
if (line.trim()) console.error(` ${line}`);
505+
}
506+
}
507+
508+
/**
509+
* Re-run the Hermes env-file secret-boundary validator against a running
510+
* gateway, before the probe path returns control to the caller. The
511+
* relaunch path already runs the same validator inline as part of
512+
* `buildRecoveryScript`, but the probe path returns early as soon as the
513+
* gateway is reported healthy, so a poisoned `.env` injected after cold
514+
* start would otherwise never be re-evaluated. The check is invoked via
515+
* `openshell sandbox exec` (root) so the validator's kill snippet can
516+
* actually signal the gateway-user process when refusing — a sandbox-user
517+
* SSH shell cannot (test/e2e-gateway-isolation.sh test 13). Every
518+
* refusal diagnostic — validator `[SECURITY]` stderr, the helper's own
519+
* context line, and the remediation hint — is written to `console.error`
520+
* unconditionally, so the offending key (e.g. `TELEGRAM_BOT_TOKEN (line
521+
* N)`) and the reason for refusal always reach the operator, including
522+
* on the quiet probe/recover path. Returns `null` only when the persisted
523+
* sandbox registry entry is not Hermes (no boundary to enforce). When
524+
* the registry says Hermes but the in-memory agent definition failed to
525+
* load (`getSessionAgent()` returned `null` from its catch path), the
526+
* helper fails safe with an inconclusive refusal rather than silently
527+
* skipping the boundary. A running Hermes gateway whose root exec
528+
* channel is unreachable is also treated as a fail-safe inconclusive
529+
* refusal rather than a healthy path. Non-zero validator status without
530+
* a `SECRET_BOUNDARY_REFUSED` marker is reported as inconclusive, not as
531+
* a raw-secret refusal, so a shell or validator crash does not
532+
* masquerade as a poisoned env file.
533+
*/
534+
function enforceHermesSecretBoundaryOnRunningGateway(
535+
sandboxName: string,
536+
agent: ReturnType<typeof agentRuntime.getSessionAgent>,
537+
): HermesSecretBoundaryEnforcement | null {
538+
const persistedAgent = registry.getSandbox(sandboxName)?.agent;
539+
if (persistedAgent !== "hermes") return null;
540+
if (!isHermesAgent(agent)) {
541+
console.error("");
542+
console.error(
543+
` ${R}Hermes agent definition could not be loaded for sandbox '${sandboxName}'.${R}`,
544+
);
545+
console.error(" Refusing recovery to keep the validator-enforced boundary intact.");
546+
return { refused: true, reason: "inconclusive", stderr: "" };
547+
}
548+
const script = buildHermesEnvFileBoundaryStandaloneCheck();
549+
const result = executeSandboxExecCommand(sandboxName, script, 30000);
550+
if (!result) {
551+
console.error("");
552+
console.error(
553+
` ${R}Secret-boundary check could not run against the Hermes gateway in '${sandboxName}'.${R}`,
554+
);
555+
console.error(" Refusing recovery to keep the validator-enforced boundary intact.");
556+
return { refused: true, reason: "inconclusive", stderr: "" };
557+
}
558+
const stdoutMarker = result.stdout
559+
.split(/\r?\n/)
560+
.reverse()
561+
.find((line) => line.trim().startsWith("SECRET_BOUNDARY_"));
562+
if (stdoutMarker === SECRET_BOUNDARY_REFUSED_MARKER) {
563+
printValidatorStderr(result.stderr);
564+
console.error("");
565+
console.error(
566+
` ${R}Secret-boundary check refused recovery of Hermes gateway in '${sandboxName}'.${R}`,
567+
);
568+
console.error(" /sandbox/.hermes/.env contains raw secret-shaped values. Replace them with");
569+
console.error(
570+
" openshell:resolve:env:<name> placeholders and re-run `nemoclaw <sandbox> recover`.",
571+
);
572+
return { refused: true, reason: "raw-secret", stderr: result.stderr };
573+
}
574+
if (stdoutMarker === SECRET_BOUNDARY_OK_MARKER) {
575+
return { refused: false };
576+
}
577+
if (stdoutMarker === SECRET_BOUNDARY_VALIDATOR_MISSING_MARKER) {
578+
console.error(
579+
` [boundary] Hermes secret-boundary validator missing in sandbox '${sandboxName}'; recover proceeded without re-evaluating /sandbox/.hermes/.env. Re-image the sandbox to enable per-run enforcement.`,
580+
);
581+
return { refused: false };
582+
}
583+
printValidatorStderr(result.stderr);
584+
console.error("");
585+
console.error(
586+
` ${R}Secret-boundary check did not complete cleanly for Hermes gateway in '${sandboxName}'.${R}`,
587+
);
588+
console.error(
589+
" Refusing recovery; inspect the validator output above before re-running `nemoclaw <sandbox> recover`.",
590+
);
591+
return { refused: true, reason: "inconclusive", stderr: result.stderr };
592+
}
593+
485594
/**
486595
* Detect and recover from a sandbox that survived a gateway restart but
487596
* whose OpenClaw processes are not running. Also re-establishes the
488597
* host-side dashboard port-forward when it has gone dead independently
489598
* of the gateway. Returns an object describing the outcome:
490-
* `{ checked, wasRunning, recovered, forwardRecovered }`.
599+
* `{ checked, wasRunning, recovered, forwardRecovered, secretBoundaryRefused?, secretBoundaryReason? }`.
491600
*/
492601
export function checkAndRecoverSandboxProcesses(
493602
sandboxName: string,
@@ -499,6 +608,19 @@ export function checkAndRecoverSandboxProcesses(
499608
}
500609
const recoveryAgent = agentRuntime.getSessionAgent(sandboxName);
501610
const recoveryPort = resolveSandboxDashboardPort(sandboxName);
611+
if (running) {
612+
const enforcement = enforceHermesSecretBoundaryOnRunningGateway(sandboxName, recoveryAgent);
613+
if (enforcement?.refused) {
614+
return {
615+
checked: true,
616+
wasRunning: true,
617+
recovered: false,
618+
forwardRecovered: false,
619+
secretBoundaryRefused: true,
620+
secretBoundaryReason: enforcement.reason,
621+
};
622+
}
623+
}
502624
if (running) {
503625
// Gateway is alive but the host-side forward can still be dead or
504626
// owned by another sandbox. Probe and re-establish only when

0 commit comments

Comments
 (0)