diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index acc6c89f8f3..a2c1162c1bd 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -507,7 +507,7 @@ PY # file has to also rewrite the Dockerfile-committed hash, which reviewers gate). # Regenerate with `sha256sum agents/hermes/{hermes-wrapper.py,validate-env-secret-boundary.py,finalize-tirith-marker.py}`. ARG NEMOCLAW_HERMES_WRAPPER_SHA256=cd851746da14162ac4701d56c274dac20024ea6a11f6ffcf2ce7fb89dff388a0 -ARG NEMOCLAW_HERMES_VALIDATOR_SHA256=87c1b02e0fb2becd26f79ae5dec6e0d21f78c6b08862b880e926708faf46ecb3 +ARG NEMOCLAW_HERMES_VALIDATOR_SHA256=822c7e63d068c5d09f3291350771c1a42c9686f51bfa9bc9a1f41fbe15d163b1 ARG NEMOCLAW_HERMES_TIRITH_FINALIZER_SHA256=a1e6b1c53ab297569abb87c29d15c294d729e46005bfd022136b4c447a791819 # hadolint ignore=DL4006 RUN printf '%s %s\n' \ diff --git a/agents/hermes/runtime-config-guard.py b/agents/hermes/runtime-config-guard.py index c1e2fb14ec6..cf2cd0a4cac 100755 --- a/agents/hermes/runtime-config-guard.py +++ b/agents/hermes/runtime-config-guard.py @@ -1754,7 +1754,9 @@ def _mutable_nonroot_reconciliation_posture_is_allowed( # OpenShell can present the live non-root home as private 0700 after the # managed supervisor/dashboard has started. Shields-down transitions use # the canonical set-id 03770 form. Both are sandbox-owned mutable roots; - # the root-owned 0755/0444 shields-up posture must never be reconciled. + # a shields-up posture must never be reconciled here. The locked root also + # carries 03770 since #7865, so the sandbox-owner check below — not the + # mode — is what keeps the two apart. if ( hermes_meta.get("uid") != sandbox_uid or hermes_meta.get("gid") != sandbox_gid @@ -2914,6 +2916,27 @@ def _record_current_sealed_inodes( _write_restart_state(state_file, state_data, create=False) +def _is_locked_hermes_root(uid: object, gid: object, mode: object) -> bool: + """Report whether a recorded `.hermes` root is in the shields-locked posture. + + The current locked root is root-owned in the sandbox group and keeps the + set-id/sticky shape so Hermes can still write its top-level runtime state + (#7865). Sandboxes locked before that change recorded a root:root 0755 + root, so keep accepting it here: rollback and re-lock must classify an + already-locked sandbox correctly, and the host verifier reports the legacy + shape as drift so `shields up` repairs it. + """ + if uid != os.geteuid(): + return False + if gid == os.getegid() and mode == 0o755: + return True + try: + _, sandbox_gid = _sandbox_identity() + except UnsafePathError: + return False + return gid == sandbox_gid and mode == 0o3770 + + def _restart_state_was_locked(state_data: dict[str, object]) -> bool: recorded = state_data.get("original_locked") if isinstance(recorded, bool): @@ -2922,10 +2945,8 @@ def _restart_state_was_locked(state_data: dict[str, object]) -> bool: files = state_data.get("files") if not isinstance(hermes_meta, dict) or not isinstance(files, dict): raise UnsafePathError("refusing malformed Hermes restart seal metadata") - if ( - hermes_meta.get("uid") != os.geteuid() - or hermes_meta.get("gid") != os.getegid() - or hermes_meta.get("mode") != 0o755 + if not _is_locked_hermes_root( + hermes_meta.get("uid"), hermes_meta.get("gid"), hermes_meta.get("mode") ): return False for name in ("config.yaml", ".env"): @@ -3213,9 +3234,9 @@ def _seal_shields_locked( original_locked = ( not unavailable_reasons - and hermes_meta["uid"] == os.geteuid() - and hermes_meta["gid"] == os.getegid() - and hermes_meta["mode"] == 0o755 + and _is_locked_hermes_root( + hermes_meta["uid"], hermes_meta["gid"], hermes_meta["mode"] + ) and all( initial_stats[name] is not None and initial_stats[name].st_uid == os.geteuid() @@ -3724,8 +3745,17 @@ def _configure_shields_target_metadata( locked = mode == "locked" desired_uid = os.geteuid() if locked else sandbox_uid desired_gid = os.getegid() if locked else sandbox_gid - desired_dir_mode = 0o755 if locked else 0o3770 desired_file_mode = 0o444 if locked else 0o640 + # The config root keeps one set-id/sticky shape in both postures; only its + # owner changes. Hermes writes its top-level runtime state directly here — + # auth.json, the drain request, and the temporary files that back every + # atomic gateway_state/pid replace — so a root-owned root without group + # write stops every gateway launch and the supervisor quarantines relaunch + # until the sandbox is recreated (#7865). Root ownership plus the sticky + # bit is what protects the sealed entries under lockdown: the sandbox + # identity manages its own runtime files but cannot unlink or rename the + # root-owned config, which is the same trade `/sandbox` already makes. + desired_dir_mode = 0o3770 # `/sandbox` must remain a usable home, but its sticky root-owned entry # prevents the sandbox identity from renaming the root-owned `.hermes` # lock root out from under the protected files. @@ -3742,7 +3772,7 @@ def _configure_shields_target_metadata( FS_IMMUTABLE_FL | FS_APPEND_FL ) hermes_meta.update( - {"uid": desired_uid, "gid": desired_gid, "mode": desired_dir_mode} + {"uid": desired_uid, "gid": sandbox_gid, "mode": desired_dir_mode} ) state_data["hermes"] = hermes_meta state_data["hermes_flags"] = int(state_data.get("hermes_flags", 0)) & ~( @@ -3898,6 +3928,34 @@ def apply_shields_transition( ) os.fchown(parent_fd, os.geteuid(), os.getegid()) os.fchmod(parent_fd, 0o755) + # Re-apply the recorded config-root posture. The pending phase + # clamps this root to a transient root-only mode, so an interruption + # between publishing the applied phase and restoring the seal leaves + # the clamp in place; finish would then refuse the drifted root and + # wedge the transaction instead of converging. Repairing here keeps + # resume idempotent, and the inode pin below is what makes it safe. + hermes_meta = state_data.get("hermes") + if not isinstance(hermes_meta, dict): + raise UnsafePathError( + "refusing applied shields resume without .hermes metadata" + ) + resumed_fd = _open_child_directory( + parent_fd, _split_path(hermes_dir)[1], hermes_dir + ) + try: + if not _same_inode(os.fstat(resumed_fd), hermes_meta): + raise UnsafePathError( + "refusing applied shields resume because .hermes changed" + ) + _set_inode_flags( + resumed_fd, + _get_inode_flags(resumed_fd) & ~(FS_IMMUTABLE_FL | FS_APPEND_FL), + ) + # Chown can clear set-id bits, so the mode restore follows it. + os.fchown(resumed_fd, hermes_meta["uid"], hermes_meta["gid"]) + os.fchmod(resumed_fd, hermes_meta["mode"]) + finally: + os.close(resumed_fd) finally: os.close(parent_fd) hash_file = str(state_data.get("hash_file", "")) diff --git a/agents/hermes/start.sh b/agents/hermes/start.sh index 33fa02550cb..81b3a7a02d4 100755 --- a/agents/hermes/start.sh +++ b/agents/hermes/start.sh @@ -590,7 +590,14 @@ hermes_config_root_is_locked() { owner="$(stat -c '%U:%G' "$HERMES_DIR" 2>/dev/null || stat -f '%Su:%Sg' "$HERMES_DIR" 2>/dev/null || true)" mode="$(stat -c '%a' "$HERMES_DIR" 2>/dev/null || stat -f '%Lp' "$HERMES_DIR" 2>/dev/null || true)" + # The locked root is root-owned in the sandbox group and keeps the set-id and + # sticky bits so the gateway can still write its top-level runtime state while + # the sticky bit protects the sealed entries (#7865) — the same shape + # hermes_locked_parent_is_protected expects one level up. `root:root 755` is + # the pre-#7865 posture; keep detecting it so an existing shields-up sandbox + # still takes the locked branches until `shields up` repairs the root. case "${owner} ${mode}" in + "root:sandbox 3770" | "root:sandbox 03770") ;; "root:root 755" | "root:root 0755") ;; *) return 1 ;; esac @@ -1970,6 +1977,17 @@ refresh_hermes_provider_placeholders() { refresh_hermes_runtime_config_hashes() { local mode="${1:-strict}" + # A locked root seals config.yaml, .env, and .config-hash as root-owned, and + # the lock transaction already wrote a coherent hash for them. The compat + # refresh runs as the sandbox identity, which by design cannot replace a + # sealed hash: the sticky config root refuses the rename, so every launch + # under shields failed here and the supervisor stopped respawning (#7865). + # There is also nothing to refresh, because the sealed inputs cannot drift. + # The MCP integrity inspection that follows still validates the sealed hash, + # so a genuinely incoherent locked tree keeps failing closed. + if [ "$mode" = "compat" ] && hermes_config_root_is_locked; then + return 0 + fi local cmd=( "$_HERMES_PYTHON" -I "$_HERMES_RUNTIME_CONFIG_GUARD" refresh-hashes --hermes-dir "$HERMES_DIR" diff --git a/agents/hermes/validate-env-secret-boundary.py b/agents/hermes/validate-env-secret-boundary.py index ef23f63c28c..2fdc82061b6 100755 --- a/agents/hermes/validate-env-secret-boundary.py +++ b/agents/hermes/validate-env-secret-boundary.py @@ -186,6 +186,12 @@ def _validate_directory_descriptor(path: str, fd: int) -> tuple[int, int, int, i { (sandbox_uid, sandbox_gid, 0o700), (sandbox_uid, sandbox_gid, 0o3770), + # Shields-up config root: root-owned in the sandbox + # group with set-id/sticky, so Hermes keeps writing its + # top-level runtime state while the sticky bit stops the + # sandbox identity from unlinking the sealed root-owned + # config (#7865). Same shape `/sandbox` uses above. + (0, sandbox_gid, 0o3770), } ) if (st.st_uid, st.st_gid, mode) not in allowed: diff --git a/ci/source-shape-test-budget.json b/ci/source-shape-test-budget.json index 8b54699e676..08dbb6efa65 100644 --- a/ci/source-shape-test-budget.json +++ b/ci/source-shape-test-budget.json @@ -266,6 +266,11 @@ "test": "restores exact locked posture after root-separated repair and later failure (#7033)", "category": "security" }, + { + "file": "test/hermes-runtime-config-guard-topology.test.ts", + "test": "allows the sandbox identity to create runtime state but refuses sealed configuration writes, unlinks, and renames (#7865)", + "category": "security" + }, { "file": "test/hosted-runner-recovery-workflow.test.ts", "test": "locks recovery identities to the source workflows' runtime names (#7140)", diff --git a/docs/reference/troubleshooting.mdx b/docs/reference/troubleshooting.mdx index 3243c021d7d..2537d6f8fcb 100644 --- a/docs/reference/troubleshooting.mdx +++ b/docs/reference/troubleshooting.mdx @@ -2689,6 +2689,51 @@ nemohermes start Then run `nemohermes inference get` and verify Dashboard Chat uses the selected model. If the command succeeds because the dashboard profile is missing, the dashboard is disabled and no dashboard recovery is required. +### Shields Reports Drift for the Hermes Configuration Root + +The Hermes configuration root holds the agent's top-level runtime state, not only its configuration: `auth.json`, the drain request, and the temporary files that back every atomic `gateway_state.json` and `gateway.pid` replace are created directly in `/sandbox/.hermes`. +Lockdown therefore moves that directory to `root:sandbox` mode `3770`, keeping the set-id and sticky bits, so the gateway can still manage its own runtime files while the sticky bit stops the sandbox identity from unlinking or renaming the sealed root-owned configuration. +Run the following command to inspect a locked root: + +```bash +$$nemoclaw exec -- stat -c '%a %U:%G' /sandbox/.hermes +``` + +Expected output: + +```text +3770 root:sandbox +``` + +A sandbox locked by an older release carries a `755 root:root` root instead. +The gateway cannot write its runtime state there. +`$$nemoclaw shields status` reports the stale posture as drift. +Before restarting the gateway, repair the posture: + +```bash +$$nemoclaw shields up +``` + +If the command refuses the repair, follow its recovery guidance and do not restart the gateway. +Otherwise, verify the repaired posture: + +```bash +$$nemoclaw shields status +$$nemoclaw exec -- stat -c '%a %U:%G' /sandbox/.hermes +``` + +Continue only when status no longer reports drift and `stat` prints the expected output above. +Then restart the gateway: + +```bash +$$nemoclaw gateway restart +``` + +The restart must exit zero and report that the health check passed. +If restart or recovery already reports `relaunch quarantined`, repairing the directory cannot clear the supervisor quarantine. +Follow [Restart or recovery reports `relaunch quarantined`](#restart-or-recovery-reports-relaunch-quarantined) to rebuild the sandbox. +The sealed files are unchanged by the repair and stay `444 root:root`. + ### Hermes restart reports `config hash mismatch` A Hermes restart reports `config hash mismatch` when a strict root-owned hash is available and `/sandbox/.hermes/config.yaml` or `/sandbox/.hermes/.env` does not match it. diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 3000e1c2c0a..3202c998986 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -1471,8 +1471,11 @@ try: os.close(fd) if action == "lock": - os.fchown(config_fd, 0, 0) - os.fchmod(config_fd, 0o755) + # Root-owned in the sandbox group with set-id/sticky: Hermes keeps + # writing its top-level runtime state while the sticky bit stops the + # sandbox identity from unlinking the sealed root-owned files (#7865). + os.fchown(config_fd, 0, sandbox_gid) + os.fchmod(config_fd, 0o3770) os.fchown(parent_fd, 0, sandbox_gid) os.fchmod(parent_fd, 0o1775) else: diff --git a/src/lib/shields/legacy-hermes-compat.test.ts b/src/lib/shields/legacy-hermes-compat.test.ts index 70dc2602561..73788930924 100644 --- a/src/lib/shields/legacy-hermes-compat.test.ts +++ b/src/lib/shields/legacy-hermes-compat.test.ts @@ -365,7 +365,7 @@ describe("legacy Hermes shields compatibility", () => { return cmd.at(-1) === "/sandbox" ? "1775 root:sandbox" : cmd.at(-1) === "/sandbox/.hermes" - ? "755 root:root" + ? "3770 root:sandbox" : "444 root:root"; case cmd[0] === "lsattr": return `----i----------- ${cmd.at(-1)}`; @@ -405,7 +405,7 @@ describe("legacy Hermes shields compatibility", () => { return cmd.at(-1) === "/sandbox" ? "755 sandbox:sandbox" : cmd.at(-1) === "/sandbox/.hermes" - ? "755 root:root" + ? "3770 root:sandbox" : "444 root:root"; case cmd[0] === "lsattr": return `----i----------- ${cmd.at(-1)}`; diff --git a/src/lib/shields/legacy-hermes-transition.test.ts b/src/lib/shields/legacy-hermes-transition.test.ts index ad068e3d1a7..f36f80dc092 100644 --- a/src/lib/shields/legacy-hermes-transition.test.ts +++ b/src/lib/shields/legacy-hermes-transition.test.ts @@ -41,6 +41,10 @@ function legacyTransitionScriptForCurrentUser(): string { .replace( "os.fchown(parent_fd, 0, sandbox_gid)", "os.fchown(parent_fd, os.geteuid(), os.getegid())", + ) + .replace( + "os.fchown(config_fd, 0, sandbox_gid)", + "os.fchown(config_fd, os.geteuid(), os.getegid())", ); } @@ -144,7 +148,7 @@ describe("legacy Hermes config transition", () => { expect(afterStaleWrite.inode).toBe(lockedSnapshot.inode); expect(afterStaleWrite.bytes).toEqual(fixture.configBytes); expect(mode(fixture.parentDir)).toBe(0o1775); - expect(mode(fixture.configDir)).toBe(0o755); + expect(mode(fixture.configDir)).toBe(0o3770); expect(afterStaleWrite.mode).toBe(0o444); expect(mode(fixture.envPath)).toBe(0o444); expect(mode(fixture.compatPath)).toBe(0o444); @@ -156,7 +160,7 @@ describe("legacy Hermes config transition", () => { expect(refused.status).not.toBe(0); expect(refused.stderr).toContain("strict hash verification failed"); expect(mode(fixture.parentDir)).toBe(0o1775); - expect(mode(fixture.configDir)).toBe(0o755); + expect(mode(fixture.configDir)).toBe(0o3770); const afterRefusedUnlock = readRegularFileSnapshot(fixture.configPath); expect(afterRefusedUnlock.inode).toBe(afterStaleWrite.inode); expect(afterRefusedUnlock.bytes).toEqual(fixture.configBytes); diff --git a/src/lib/shields/verify-lock.test.ts b/src/lib/shields/verify-lock.test.ts index c059485c5d2..4c2917d2188 100644 --- a/src/lib/shields/verify-lock.test.ts +++ b/src/lib/shields/verify-lock.test.ts @@ -75,7 +75,10 @@ describe("verifyShieldsLockState", () => { "/sandbox/.hermes/config.yaml": "444 root:root", "/sandbox/.hermes/.env": "444 root:root", "/sandbox/.hermes/.config-hash": "444 root:root", - "/sandbox/.hermes": "755 root:root", + // Root-owned in the sandbox group with set-id/sticky: Hermes keeps + // writing top-level runtime state while the sticky bit stops the sandbox + // identity unlinking the sealed root-owned files (#7865). + "/sandbox/.hermes": "3770 root:sandbox", "/sandbox": "1775 root:sandbox", }); @@ -90,7 +93,7 @@ describe("verifyShieldsLockState", () => { "/sandbox/.hermes/config.yaml": "444 root:root", "/sandbox/.hermes/.env": "444 root:root", "/sandbox/.hermes/.config-hash": "444 root:root", - "/sandbox/.hermes": "755 root:root", + "/sandbox/.hermes": "3770 root:sandbox", "/sandbox": "755 sandbox:sandbox", }); const drifted = verifyShieldsLockState("hermes", hermesTarget, { @@ -106,6 +109,59 @@ describe("verifyShieldsLockState", () => { ); }); + it("reports the previous root-owned Hermes config root as drift (#7865)", async () => { + const { verifyShieldsLockState } = await loadVerifier(); + const hermesTarget = { + agentName: "hermes", + configPath: "/sandbox/.hermes/config.yaml", + configDir: "/sandbox/.hermes", + sensitiveFiles: ["/sandbox/.hermes/.env", "/sandbox/.hermes/.config-hash"], + }; + // A Hermes root left at the old 755 root:root cannot run a gateway at all, + // so it must surface as drift for the caller's re-lock to repair rather + // than passing as a healthy lock. + const legacyLocked = makeExec({ + "/sandbox/.hermes/config.yaml": "444 root:root", + "/sandbox/.hermes/.env": "444 root:root", + "/sandbox/.hermes/.config-hash": "444 root:root", + "/sandbox/.hermes": "755 root:root", + "/sandbox": "1775 root:sandbox", + }); + + const result = verifyShieldsLockState("hermes", hermesTarget, { + exec: legacyLocked, + verifyParentProtection: true, + }); + + expect(result.ok).toBe(false); + expect(result.issues).toEqual( + expect.arrayContaining([ + "dir mode=755 (expected 3770)", + "dir owner=root:root (expected root:sandbox)", + ]), + ); + }); + + it("keeps the OpenClaw locked config root at 755 root:root", async () => { + const { verifyShieldsLockState } = await loadVerifier(); + const openClawTarget = { + agentName: "openclaw", + configPath: "/sandbox/.openclaw/openclaw.json", + configDir: "/sandbox/.openclaw", + sensitiveFiles: ["/sandbox/.openclaw/.config-hash"], + }; + const exec = makeExec({ + "/sandbox/.openclaw/openclaw.json": "444 root:root", + "/sandbox/.openclaw/.config-hash": "444 root:root", + "/sandbox/.openclaw": "755 root:root", + }); + + expect(verifyShieldsLockState("openclaw", openClawTarget, { exec })).toEqual({ + ok: true, + issues: [], + }); + }); + it("requires the same protected parent for OpenClaw but does not impose it on custom agents", async () => { const { verifyShieldsLockState } = await loadVerifier(); const openClawTarget = { ...target, agentName: "openclaw" }; diff --git a/src/lib/shields/verify-lock.ts b/src/lib/shields/verify-lock.ts index 3ff3ae8c1c4..ee9cf8e6262 100644 --- a/src/lib/shields/verify-lock.ts +++ b/src/lib/shields/verify-lock.ts @@ -2,9 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 // Re-verify that the sandbox filesystem still matches what `shields up` -// established: 444 root:root on each locked file, 755 root:root on the -// config directory, no legacy state layout, and (when the caller knows -// chattr was applied) the immutable bit. When the caller supplies the +// established: 444 root:root on each locked file, the agent-specific +// locked ownership and mode on the config directory, no legacy state +// layout, and the immutable bit when the caller knows `chattr` was +// applied. When the caller supplies the // SHA-256 seal that was captured at lock time, also re-hash each file // and surface a content-drift entry on any mismatch. This catches the // host-root tamper pattern that defeats perm-only verification: chmod @@ -41,6 +42,23 @@ export type VerifyShieldsLockResult = { const EXPECTED_FILE_MODE = "444"; const EXPECTED_DIR_MODE = "755"; const EXPECTED_OWNER = "root:root"; +// Hermes writes its top-level runtime state inside the config root, so its +// locked root stays group-writable with set-id/sticky and only changes owner. +// The sticky bit is what stops the sandbox identity from unlinking the sealed +// root-owned files (#7865). A Hermes root still sitting at the pre-#7865 +// `755 root:root` cannot run a gateway at all, so report it as drift and let +// the caller's re-lock repair it rather than passing it as a healthy lock. +const HERMES_EXPECTED_DIR_MODE = "3770"; +const HERMES_EXPECTED_DIR_OWNER = "root:sandbox"; + +function expectedLockedDirPosture(agentName?: string): { + mode: string; + owner: string; +} { + return agentName === "hermes" + ? { mode: HERMES_EXPECTED_DIR_MODE, owner: HERMES_EXPECTED_DIR_OWNER } + : { mode: EXPECTED_DIR_MODE, owner: EXPECTED_OWNER }; +} function noopAssertLegacyLayout(_sandboxName: string, _configDir: string): void { // Production callers replace this with the real legacy-layout assertion; @@ -73,13 +91,14 @@ export function verifyShieldsLockState( } } + const expectedDir = expectedLockedDirPosture(target.agentName); try { const dirPerms = exec(["stat", "-c", "%a %U:%G", target.configDir]); const [dirMode, dirOwner] = dirPerms.split(" "); - if (dirMode !== EXPECTED_DIR_MODE) - issues.push(`dir mode=${dirMode} (expected ${EXPECTED_DIR_MODE})`); - if (dirOwner !== EXPECTED_OWNER) - issues.push(`dir owner=${dirOwner} (expected ${EXPECTED_OWNER})`); + if (dirMode !== expectedDir.mode) + issues.push(`dir mode=${dirMode} (expected ${expectedDir.mode})`); + if (dirOwner !== expectedDir.owner) + issues.push(`dir owner=${dirOwner} (expected ${expectedDir.owner})`); } catch (err) { const msg = err instanceof Error ? err.message : String(err); issues.push(`dir stat failed: ${msg}`); diff --git a/test/e2e/live/hermes-shields-config.test.ts b/test/e2e/live/hermes-shields-config.test.ts index 1da37c9795d..3d2b192fdb5 100644 --- a/test/e2e/live/hermes-shields-config.test.ts +++ b/test/e2e/live/hermes-shields-config.test.ts @@ -126,7 +126,7 @@ async function expectLockedPosture(sandbox: SandboxClient, cycle: number): Promi ); assertExitZero(result, `inspect Hermes locked posture after cycle ${cycle}`); expect(result.stdout).toContain("1775 root:sandbox /sandbox"); - expect(result.stdout).toContain(`755 root:root ${HERMES_DIR}`); + expect(result.stdout).toContain(`3770 root:sandbox ${HERMES_DIR}`); expect(result.stdout).toContain(`444 root:root ${CONFIG_PATH}`); expect(result.stdout).toContain(`444 root:root ${HERMES_DIR}/.env`); expect(result.stdout).toContain(`444 root:root ${HERMES_DIR}/.config-hash`); diff --git a/test/e2e/live/mcp-bridge-hermes-lifecycle.ts b/test/e2e/live/mcp-bridge-hermes-lifecycle.ts index a7b71652c49..0f9b7a06881 100644 --- a/test/e2e/live/mcp-bridge-hermes-lifecycle.ts +++ b/test/e2e/live/mcp-bridge-hermes-lifecycle.ts @@ -89,7 +89,7 @@ export async function assertHermesManagedAddSurvivesLockedGatewayRestartAndState [ "set -eu", "test \"$(stat -c '%a %U:%G' /sandbox)\" = '1775 root:sandbox'", - "test \"$(stat -c '%a %U:%G' /sandbox/.hermes)\" = '755 root:root'", + "test \"$(stat -c '%a %U:%G' /sandbox/.hermes)\" = '3770 root:sandbox'", "for path in /sandbox/.hermes/gateway /sandbox/.hermes/runtime; do", " test \"$(stat -c '%a %U:%G' \"$path\")\" = '2770 gateway:sandbox'", "done", diff --git a/test/hermes-restart-config-seal-hostile-input.test.ts b/test/hermes-restart-config-seal-hostile-input.test.ts index 48be1a82176..0b44fac6d80 100644 --- a/test/hermes-restart-config-seal-hostile-input.test.ts +++ b/test/hermes-restart-config-seal-hostile-input.test.ts @@ -183,7 +183,7 @@ describe.skipIf(process.platform === "win32")("Hermes mutable restart input seal const aborted = runShieldsTransactionAction(fixture, "abort-shields-transition", { token }); expect(aborted.status, aborted.stderr).toBe(0); - expect(mode(fixture.hermesDir)).toBe(0o755); + expect(mode(fixture.hermesDir)).toBe(0o3770); expect(mode(fixture.sandboxDir)).toBe(0o1775); } finally { fs.rmSync(fixture.root, { recursive: true, force: true }); diff --git a/test/hermes-restart-config-seal-recovery.test.ts b/test/hermes-restart-config-seal-recovery.test.ts index 649f8f0de2d..936a0584005 100644 --- a/test/hermes-restart-config-seal-recovery.test.ts +++ b/test/hermes-restart-config-seal-recovery.test.ts @@ -215,7 +215,7 @@ describe.skipIf(process.platform === "win32")("Hermes mutable restart input seal expect(readTextFileSnapshot(fixture.configPath)).toBe(fixture.trustedConfig); expect(mode(fixture.sandboxDir)).toBe(0o1775); - expect(mode(fixture.hermesDir)).toBe(0o755); + expect(mode(fixture.hermesDir)).toBe(0o3770); expect(mode(fixture.configPath)).toBe(0o444); expect(strictHashIsValid(fixture)).toBe(true); expect(fs.existsSync(fixture.statePath)).toBe(false); @@ -318,6 +318,9 @@ describe.skipIf(process.platform === "win32")("Hermes mutable restart input seal it("preserves an already trusted shields-up directory posture across seal and unseal", () => { const fixture = createRestartFixture(); + // 0755 is the pre-#7865 locked root. Sandboxes locked by an older CLI keep + // it until the next `shields up` repairs them, so seal/unseal must still + // classify and restore it rather than refusing the tree. fs.chmodSync(fixture.sandboxDir, 0o755); fs.chmodSync(fixture.hermesDir, 0o755); fs.chmodSync(fixture.configPath, 0o444); diff --git a/test/hermes-restart-config-seal-transition.test.ts b/test/hermes-restart-config-seal-transition.test.ts index d8922d6e92a..7aa80983de7 100644 --- a/test/hermes-restart-config-seal-transition.test.ts +++ b/test/hermes-restart-config-seal-transition.test.ts @@ -93,7 +93,7 @@ describe.skipIf(process.platform === "win32")("Hermes mutable restart input seal expect(fs.statSync(fixture.envPath).ino).not.toBe(envBefore.ino); expect(fs.statSync(fixture.compatHashPath).ino).not.toBe(compatBefore.ino); expect(mode(fixture.sandboxDir)).toBe(0o1775); - expect(mode(fixture.hermesDir)).toBe(0o755); + expect(mode(fixture.hermesDir)).toBe(0o3770); expect(mode(fixture.configPath)).toBe(0o444); expect(mode(fixture.envPath)).toBe(0o444); expect(mode(fixture.compatHashPath)).toBe(0o444); @@ -171,7 +171,7 @@ describe.skipIf(process.platform === "win32")("Hermes mutable restart input seal expect(fs.existsSync(path.join(fixture.hermesDir, ".nemoclaw-hermes-restart-seal"))).toBe( true, ); - expect(mode(fixture.hermesDir)).toBe(0o755); + expect(mode(fixture.hermesDir)).toBe(0o3770); expect(mode(fixture.configPath)).toBe(0o444); expect(mode(fixture.sandboxDir)).toBe(0o755); @@ -261,7 +261,7 @@ describe.skipIf(process.platform === "win32")("Hermes mutable restart input seal token, }); expect(finished.status, finished.stderr).toBe(0); - expect(mode(fixture.hermesDir)).toBe(0o755); + expect(mode(fixture.hermesDir)).toBe(0o3770); } finally { fs.rmSync(fixture.root, { recursive: true, force: true }); } @@ -297,7 +297,7 @@ describe.skipIf(process.platform === "win32")("Hermes mutable restart input seal }); expect(finished.status, finished.stderr).toBe(0); expect(fs.existsSync(fixture.statePath)).toBe(false); - expect(mode(fixture.hermesDir)).toBe(0o755); + expect(mode(fixture.hermesDir)).toBe(0o3770); } finally { fs.rmSync(fixture.root, { recursive: true, force: true }); } @@ -358,7 +358,7 @@ describe.skipIf(process.platform === "win32")("Hermes mutable restart input seal token: lockedToken, }); expect(finished.status, finished.stderr).toBe(0); - expect(mode(fixture.hermesDir)).toBe(0o755); + expect(mode(fixture.hermesDir)).toBe(0o3770); expect(mode(fixture.configPath)).toBe(0o444); expect(strictHashIsValid(fixture)).toBe(true); } finally { diff --git a/test/hermes-runtime-config-guard-topology.test.ts b/test/hermes-runtime-config-guard-topology.test.ts index bcc6966fd28..6523d4b3c03 100644 --- a/test/hermes-runtime-config-guard-topology.test.ts +++ b/test/hermes-runtime-config-guard-topology.test.ts @@ -16,6 +16,9 @@ const RUNTIME_CONFIG_GUARD = path.join( "hermes", "runtime-config-guard.py", ); +const SANDBOX_GID_EXPECTED = 12345; +const EACCES = 13; +const EPERM = 1; const ROOT_RUNTIME_IMAGE = "python:3.12-slim@sha256:cab2dbf575e971934a81e4622f5aba17aa7929719bd7e31033a3a83b97fd0464"; function dockerAvailable(): boolean { @@ -657,8 +660,13 @@ with tempfile.TemporaryDirectory() as tmp: "config.yaml": "0o444", }, finish_error: "simulated later validation failure", - hermes_gid: 0, - hermes_mode: "0o755", + // Rolling back to locked re-derives the locked target rather than + // replaying the captured original, so a tree that was locked before + // #7865 comes back on the group-writable + sticky root the gateway + // needs. Root ownership plus the sticky bit keeps the sealed files + // unlinkable only by root. + hermes_gid: 12345, + hermes_mode: "0o3770", hermes_uid: 0, lifecycle_marker: { content: "root-separated\n", @@ -677,4 +685,135 @@ with tempfile.TemporaryDirectory() as tmp: }); }, ); + + // source-shape-contract: security -- Executes the shipped guard as root so a real sandbox identity proves the locked-root capability split + it.skipIf(!shouldAttemptRootContainerContract)( + "allows the sandbox identity to create runtime state but refuses sealed configuration writes, unlinks, and renames (#7865)", + () => { + // Exercise the real kernel capabilities of a distinct sandbox identity + // against a genuinely locked tree instead of asserting modes: Hermes must + // be able to create the top-level runtime state it writes on every + // launch, and must not be able to modify, unlink, or rename the sealed + // config out from under the lock. + const result = runRootContainerHarness(`${loadGuardModule} +import json +import os +import stat +import tempfile + +SANDBOX_UID = 12345 +SANDBOX_GID = 12345 + +def as_sandbox(action): + """Run action() under the sandbox identity; return its errno or 0.""" + child = os.fork() + if child == 0: + os.setgid(SANDBOX_GID) + os.setuid(SANDBOX_UID) + try: + action() + except OSError as exc: + os._exit(exc.errno) + except Exception: + os._exit(255) + os._exit(0) + _pid, status = os.waitpid(child, 0) + return os.waitstatus_to_exitcode(status) + +with tempfile.TemporaryDirectory() as tmp: + os.chmod(tmp, 0o755) + sandbox = os.path.join(tmp, "sandbox") + hermes = os.path.join(sandbox, ".hermes") + os.makedirs(hermes) + config = os.path.join(hermes, "config.yaml") + env = os.path.join(hermes, ".env") + strict = os.path.join(tmp, "hermes.config-hash") + state = os.path.join(tmp, "restart-state.json") + + with open(config, "wb") as handle: + handle.write(b"model: test\\n") + with open(env, "wb") as handle: + handle.write(b"SAFE=1\\n") + initial_hash, _config_snapshot, _env_snapshot = guard._hash_text(config, env) + guard._write_hash(strict, initial_hash) + guard.refresh_hashes(hermes, strict, "both") + + os.chown(hermes, SANDBOX_UID, SANDBOX_GID) + os.chmod(hermes, 0o3770) + os.chmod(sandbox, 0o755) + + guard._get_inode_flags = lambda _fd: 0 + guard._set_inode_flags = lambda _fd, _flags: None + guard._sandbox_identity = lambda: (SANDBOX_UID, SANDBOX_GID) + guard._claim_transition_worker = ( + lambda state_path, _token, _purpose: guard._load_restart_state(state_path) + ) + + token, _original_locked = guard.begin_shields_transition( + hermes, strict, state, "locked", "mutable" + ) + # begin clamps the root to 0500 until the host finishes its recursive lock + # pass; apply refuses while that clamp is still in place. Emulate the + # completed pass so this test covers the committed locked posture. + os.chmod(hermes, 0o755) + guard.apply_shields_transition(hermes, state, token) + guard.finish_shields_transition(hermes, strict, state, token) + + hermes_st = os.stat(hermes) + probe = os.path.join(hermes, ".gateway_state_probe.tmp") + + def create_runtime_state(): + fd = os.open(probe, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + os.close(fd) + + create_errno = as_sandbox(create_runtime_state) + unlink_own_errno = as_sandbox(lambda: os.unlink(probe)) + modify_errno = as_sandbox(lambda: os.close(os.open(config, os.O_WRONLY))) + unlink_sealed_errno = as_sandbox(lambda: os.unlink(config)) + rename_sealed_errno = as_sandbox( + lambda: os.rename(config, os.path.join(hermes, "stolen.yaml")) + ) + + print(json.dumps({ + "hermes_uid": hermes_st.st_uid, + "hermes_gid": hermes_st.st_gid, + "hermes_mode": oct(stat.S_IMODE(hermes_st.st_mode)), + "sticky": bool(hermes_st.st_mode & stat.S_ISVTX), + "config_mode": oct(stat.S_IMODE(os.stat(config).st_mode)), + "config_uid": os.stat(config).st_uid, + "create_runtime_state": create_errno, + "unlink_own_runtime_state": unlink_own_errno, + "modify_sealed_config": modify_errno, + "unlink_sealed_config": unlink_sealed_errno, + "rename_sealed_config": rename_sealed_errno, + "config_still_present": os.path.exists(config), + })) +`); + + const failureDetails = [ + `status: ${String(result.status)}`, + `stderr: ${String(result.stderr)}`, + ].join("\n"); + expect(result.status, failureDetails).toBe(0); + expect(JSON.parse(String(result.stdout))).toEqual({ + // Root-owned in the sandbox group, set-id + sticky. + hermes_uid: 0, + hermes_gid: SANDBOX_GID_EXPECTED, + hermes_mode: "0o3770", + sticky: true, + // The seal itself is unchanged by this fix. + config_mode: "0o444", + config_uid: 0, + // Hermes can manage its own top-level runtime state... + create_runtime_state: 0, + unlink_own_runtime_state: 0, + // ...but cannot touch the sealed config: EACCES on write, EPERM from + // the sticky bit on unlink and rename. + modify_sealed_config: EACCES, + unlink_sealed_config: EPERM, + rename_sealed_config: EPERM, + config_still_present: true, + }); + }, + ); }); diff --git a/test/repro-2681-group-writable.test.ts b/test/repro-2681-group-writable.test.ts index 194e70d0340..f8c42b22cf6 100644 --- a/test/repro-2681-group-writable.test.ts +++ b/test/repro-2681-group-writable.test.ts @@ -198,7 +198,10 @@ function withMockedDockerExecFileSync( case "/sandbox/.openclaw": return "2770 sandbox:sandbox\n"; case "/sandbox/.hermes": - return options.hermesLockedTransaction ? "755 root:root\n" : "3770 sandbox:sandbox\n"; + // A locked Hermes root keeps the set-id/sticky shape and only + // changes owner, so the gateway can still write its top-level + // runtime state under lockdown (#7865). + return options.hermesLockedTransaction ? "3770 root:sandbox\n" : "3770 sandbox:sandbox\n"; } if (typeof target === "string" && target.startsWith("/sandbox/.hermes/")) { return options.hermesLockedTransaction ? "444 root:root\n" : "640 sandbox:sandbox\n";