Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion agents/hermes/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,11 @@ RUN chmod -R a+rX /opt/nemoclaw-blueprint/
COPY scripts/lib/sandbox-init.sh /usr/local/lib/nemoclaw/sandbox-init.sh
COPY agents/hermes/start.sh /usr/local/bin/nemoclaw-start
COPY agents/hermes/validate-env-secret-boundary.py /usr/local/lib/nemoclaw/validate-hermes-env-secret-boundary.py
RUN chmod 755 /usr/local/bin/nemoclaw-start /usr/local/lib/nemoclaw/sandbox-init.sh /usr/local/lib/nemoclaw/validate-hermes-env-secret-boundary.py
# Host recovery restores these guards from this immutable image path before it
# relaunches a stopped gateway. Keep the contract aligned with the OpenClaw image.
COPY nemoclaw-blueprint/scripts/*.js /usr/local/lib/nemoclaw/preloads/
RUN chmod 755 /usr/local/bin/nemoclaw-start /usr/local/lib/nemoclaw/sandbox-init.sh /usr/local/lib/nemoclaw/validate-hermes-env-secret-boundary.py \
&& find /usr/local/lib/nemoclaw/preloads -type f -name '*.js' -exec chmod 644 {} +

# Wrap the hermes CLI so the runtime env secret boundary is enforced for
# `hermes gateway` no matter how it is invoked. The entrypoint guard alone left
Expand Down
62 changes: 62 additions & 0 deletions agents/hermes/start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,58 @@ else
_SANDBOX_HOME="${HOME:-/sandbox}"
fi

# ── Node preload guards (#2478) — best-effort, MUST NEVER abort startup ──────────
# The OpenClaw entrypoint (scripts/nemoclaw-start.sh) installs the sandbox-safety-net
# and ciao-network-guard NODE_OPTIONS preloads; the Hermes entrypoint did not. The
# gateway-recovery path refuses to relaunch unless NODE_OPTIONS advertises both
# guards ("proxy-env present but NODE_OPTIONS missing safety-net preload or ciao
# preload — refusing unguarded gateway relaunch", #2478), so a Hermes gateway that
# stops can never be brought back with `recover`. Install the same guards here.
#
# This script runs `set -euo pipefail`, so the install is written to be incapable of
# aborting the entrypoint: it is invoked as `… || true` (errexit suppressed through
# the whole function), every fallible command sits inside an `if`, all vars are
# `${x:-}`-safe, and a guard is only added to NODE_OPTIONS once its /tmp copy exists
# and is non-empty. Worst case it logs a warning and the gateway still starts.
# The optional positional argument is a test seam. Production always reads the
# image-owned recovery directory populated by agents/hermes/Dockerfile.
install_nemoclaw_node_guards() {
_SANDBOX_SAFETY_NET=""
_CIAO_GUARD_SCRIPT=""
_nemoclaw_guard_dir=""
local _nemoclaw_guard_dirs="${1:-/usr/local/lib/nemoclaw/preloads}"
# Production uses an image-owned, root-read-only directory only. Never accept
# an environment-selected path here: the sandbox user could plant a malicious
# preload that the entrypoint would --require into the gateway.
# shellcheck disable=SC2086 # intentional word-split over the candidate dir list
for _gd in $_nemoclaw_guard_dirs; do
if [ -f "$_gd/sandbox-safety-net.js" ] && [ -f "$_gd/ciao-network-guard.js" ]; then
_nemoclaw_guard_dir="$_gd"
break
fi
done
if [ -z "$_nemoclaw_guard_dir" ]; then
echo "[gateway] WARNING: NemoClaw preload guards not found — recover may refuse relaunch (#2478)" >&2
return 0
fi
if emit_sandbox_sourced_file /tmp/nemoclaw-sandbox-safety-net.js <"$_nemoclaw_guard_dir/sandbox-safety-net.js" 2>/dev/null && [ -s /tmp/nemoclaw-sandbox-safety-net.js ]; then
_SANDBOX_SAFETY_NET=/tmp/nemoclaw-sandbox-safety-net.js
export NODE_OPTIONS="${NODE_OPTIONS:+$NODE_OPTIONS }--require $_SANDBOX_SAFETY_NET"
else
echo "[gateway] WARNING: could not install sandbox-safety-net preload (#2478)" >&2
fi
if emit_sandbox_sourced_file /tmp/nemoclaw-ciao-network-guard.js <"$_nemoclaw_guard_dir/ciao-network-guard.js" 2>/dev/null && [ -s /tmp/nemoclaw-ciao-network-guard.js ]; then
_CIAO_GUARD_SCRIPT=/tmp/nemoclaw-ciao-network-guard.js
export NODE_OPTIONS="${NODE_OPTIONS:+$NODE_OPTIONS }--require $_CIAO_GUARD_SCRIPT"
else
echo "[gateway] WARNING: could not install ciao-network-guard preload (#2478)" >&2
fi
return 0
}
# Invoked with `|| true` so set -e is suppressed for the whole function — a guard
# failure can never take the gateway down.
install_nemoclaw_node_guards || true

# SECURITY FIX: Write proxy config to a standalone file via
# emit_sandbox_sourced_file() (444, root-owned when running as root) instead of
# appending inline to .bashrc/.profile. The old approach rewrote files under
Expand Down Expand Up @@ -792,6 +844,16 @@ hermes() {
}
# nemoclaw-configure-guard end
GUARDENVEOF
# Node preload guards for connect + recovery sessions (#2478). The gateway-
# recovery path sources this file and refuses to relaunch unless NODE_OPTIONS
# advertises the safety-net + ciao guards, so re-export them here (only if
# install_nemoclaw_node_guards actually staged them).
if [ -n "${_SANDBOX_SAFETY_NET:-}" ]; then
echo "export NODE_OPTIONS=\"\${NODE_OPTIONS:+\$NODE_OPTIONS }--require ${_SANDBOX_SAFETY_NET}\""
fi
if [ -n "${_CIAO_GUARD_SCRIPT:-}" ]; then
echo "export NODE_OPTIONS=\"\${NODE_OPTIONS:+\$NODE_OPTIONS }--require ${_CIAO_GUARD_SCRIPT}\""
fi
} | emit_sandbox_sourced_file "$_PROXY_ENV_FILE"
}

Expand Down
106 changes: 106 additions & 0 deletions test/hermes-node-guard-install.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { spawnSync } from "node:child_process";
import fs from "node:fs";
import os from "node:os";
import path from "node:path";

import { describe, expect, it } from "vitest";

const ROOT = path.resolve(import.meta.dirname, "..");
const START_SCRIPT = path.join(ROOT, "agents", "hermes", "start.sh");
const DOCKERFILE = path.join(ROOT, "agents", "hermes", "Dockerfile");

function extractGuardInstaller(source: string): string {
const start = source.indexOf("install_nemoclaw_node_guards() {");
const end = source.indexOf("\n# Invoked with", start);
expect(start).toBeGreaterThanOrEqual(0);
expect(end).toBeGreaterThan(start);
return source.slice(start, end);
}

function shellQuote(value: string): string {
return `'${value.replaceAll("'", "'\\''")}'`;
}

function runGuardInstaller(sourceDir: string, emitFunction: string) {
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "hermes-node-guards-"));
const safetyTarget = path.join(tempDir, "sandbox-safety-net.js");
const ciaoTarget = path.join(tempDir, "ciao-network-guard.js");
const source = fs
.readFileSync(START_SCRIPT, "utf8")
.replaceAll("/tmp/nemoclaw-sandbox-safety-net.js", safetyTarget)
.replaceAll("/tmp/nemoclaw-ciao-network-guard.js", ciaoTarget);
const script = [
"set -euo pipefail",
emitFunction,
extractGuardInstaller(source),
'NODE_OPTIONS=""',
`install_nemoclaw_node_guards ${shellQuote(sourceDir)}`,
'printf "NODE_OPTIONS=%s\\n" "$NODE_OPTIONS"',
].join("\n");
const result = spawnSync("bash", ["--noprofile", "--norc", "-c", script], {
encoding: "utf8",
timeout: 5000,
});
return { ciaoTarget, result, safetyTarget, tempDir };
}

describe("Hermes Node guard installation", () => {
it("stages both image-owned guards and exports them through NODE_OPTIONS", () => {

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add (#2478) suffix to test titles per guideline.

These tests directly cover the recovery-refusal / guard-preload behavior for issue #2478 (as referenced in the start.sh warning strings), but their titles lack the local issue-reference suffix required for **/*.test.ts files.

📝 Proposed fix
-  it("stages both image-owned guards and exports them through NODE_OPTIONS", () => {
+  it("stages both image-owned guards and exports them through NODE_OPTIONS (`#2478`)", () => {
...
-  it("keeps startup alive when guard staging fails", () => {
+  it("keeps startup alive when guard staging fails (`#2478`)", () => {
...
-  it("keeps startup alive when the image guard directory is missing", () => {
+  it("keeps startup alive when the image guard directory is missing (`#2478`)", () => {

As per coding guidelines, "**/*.test.ts: Write behavior-oriented test titles, and put local issue references in a final (#1234) suffix."

Also applies to: 68-68, 84-84

🤖 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-node-guard-install.test.ts` at line 51, Update the affected test
titles in the `hermes-node-guard-install` suite to follow the `**/*.test.ts`
guideline by appending the local issue reference suffix `(`#2478`)`. Specifically,
adjust the `it(...)` descriptions in the test cases covering the guard-preload
and recovery-refusal behavior so they remain behavior-oriented but end with the
required `(`#2478`)` tag. Use the existing test title strings in
`hermes-node-guard-install.test.ts` as the unique anchors for the change.

Source: Coding guidelines

const sourceDir = fs.mkdtempSync(path.join(os.tmpdir(), "hermes-node-guard-source-"));
fs.writeFileSync(path.join(sourceDir, "sandbox-safety-net.js"), "// safety\n");
fs.writeFileSync(path.join(sourceDir, "ciao-network-guard.js"), "// ciao\n");
const harness = runGuardInstaller(sourceDir, 'emit_sandbox_sourced_file() { cat >"$1"; }');
try {
expect(harness.result.status, harness.result.stderr).toBe(0);
expect(fs.readFileSync(harness.safetyTarget, "utf8")).toBe("// safety\n");
expect(fs.readFileSync(harness.ciaoTarget, "utf8")).toBe("// ciao\n");
expect(harness.result.stdout).toContain(`--require ${harness.safetyTarget}`);
expect(harness.result.stdout).toContain(`--require ${harness.ciaoTarget}`);
} finally {
fs.rmSync(sourceDir, { force: true, recursive: true });
fs.rmSync(harness.tempDir, { force: true, recursive: true });
}
});

it("keeps startup alive when guard staging fails", () => {
const sourceDir = fs.mkdtempSync(path.join(os.tmpdir(), "hermes-node-guard-source-"));
fs.writeFileSync(path.join(sourceDir, "sandbox-safety-net.js"), "// safety\n");
fs.writeFileSync(path.join(sourceDir, "ciao-network-guard.js"), "// ciao\n");
const harness = runGuardInstaller(sourceDir, "emit_sandbox_sourced_file() { return 1; }");
try {
expect(harness.result.status, harness.result.stderr).toBe(0);
expect(harness.result.stdout).toBe("NODE_OPTIONS=\n");
expect(harness.result.stderr).toContain("could not install sandbox-safety-net preload");
expect(harness.result.stderr).toContain("could not install ciao-network-guard preload");
} finally {
fs.rmSync(sourceDir, { force: true, recursive: true });
fs.rmSync(harness.tempDir, { force: true, recursive: true });
}
});

it("keeps startup alive when the image guard directory is missing", () => {
const missingDir = path.join(os.tmpdir(), `missing-hermes-guards-${process.pid}`);
const harness = runGuardInstaller(missingDir, 'emit_sandbox_sourced_file() { cat >"$1"; }');
try {
expect(harness.result.status, harness.result.stderr).toBe(0);
expect(harness.result.stdout).toBe("NODE_OPTIONS=\n");
expect(harness.result.stderr).toContain("NemoClaw preload guards not found");
} finally {
fs.rmSync(harness.tempDir, { force: true, recursive: true });
}
});

it("copies the recovery preloads into the Hermes image contract", () => {
const dockerfile = fs.readFileSync(DOCKERFILE, "utf8");
expect(dockerfile).toContain(
"COPY nemoclaw-blueprint/scripts/*.js /usr/local/lib/nemoclaw/preloads/",
);
expect(dockerfile).toContain(
"find /usr/local/lib/nemoclaw/preloads -type f -name '*.js' -exec chmod 644 {} +",
);
expect(fs.readFileSync(START_SCRIPT, "utf8")).not.toContain("NEMOCLAW_GUARD_DIRS");
});
});