fix(shields,state): keep gateway readable and runtime sessions writable under shields-up

docs/security/best-practices.mdx

@@ -200,7 +200,17 @@ In root mode, the gateway process still runs as the separate `gateway` user, but
 Writable agent state such as plugins, skills, hooks, and workspace metadata lives directly under `/sandbox/.openclaw`.
 
 By default, this directory starts writable so the agent can manage its own config, install skills, and write to standard home-directory paths natively.
-For sensitive workloads, use a reviewed host-side immutability workflow after initial setup so config and writable state entry points cannot be changed by the sandbox user.
+For sensitive workloads, use a reviewed host-side immutability workflow after initial setup so config and high-risk state entry points cannot be changed by the sandbox user.
+Shields-up locks the high-risk state directories (`skills`, `hooks`, `cron`, `agents`, `extensions`, `plugins`, `workspace`, `memory`, `devices`, `canvas`, `telegram`, `wechat`, `whatsapp`, `platforms`, `weixin`, `profiles`, `skins`) to `root:sandbox` with `chmod -R go-w`.
+The OpenClaw gateway (a member of the `sandbox` group) keeps read access to plugin and agent code; the sandbox user can no longer write them.
+Shields-up also locks the secret-bearing directories (`credentials`, `identity`, `pairing`) to `root:root 700` with `chmod -R go-rwX`.
+Neither the sandbox user nor the gateway can read those secrets while shields are up.
+Shields-down restores both groups to the mutable-default posture (`sandbox:sandbox 2770`).
+The list is the union of state directories declared by every shipped agent manifest; the lock helper silently skips dirs that aren't present in a given agent's config tree.
+Two exemption kinds keep runtime data writable.
+The lock inventory omits top-level Hermes runtime dirs (`sessions/`, `memories/`, `logs/`, `cache/`, `plans/`) and the image-build-regenerated `openclaw-weixin/`; the lock helper never touches those paths.
+Inside a locked tree, the helper restores `agents/<agent-id>/sessions/` to `sandbox:sandbox 2770` after the surrounding `agents/` lock so the OpenClaw TUI can create and write session metadata under an otherwise root-owned parent.
+If any high-risk state-dir root is a symlink when shields-up runs, the lock refuses to proceed and reports "Config not locked: state dir root is a symlink" rather than silently following the link with privileged `chown -R` / `chmod -R`.
 
 - **DAC permissions (default).** The sandbox user owns `/sandbox/.openclaw` with mode `2770` (setgid `sandbox:sandbox`) and `openclaw.json` with mode `660`, so the agent and its group can read and write config directly. A reviewed host-side immutability workflow should compare the intended ownership and mode with the live sandbox filesystem before treating the config tree as locked.
 - **Config integrity hash.** The image includes a SHA256 hash of `openclaw.json`. In the default mutable state, `.config-hash` is sandbox-owned and is not a tamper-proof trust anchor, so startup does not fail closed on that hash. When the hash is root-owned and read-only, startup enforces it and refuses to start if the hash does not match.

src/lib/shields/index.ts

@@ -53,6 +53,10 @@ const {
   isHashVerificationIssue,
   isSha256Hex,
 }: typeof import("./seal") = require("./seal");
+const {
+  applyStateDirLockMode,
+  preflightStateDirLock,
+}: typeof import("./state-dir-lock") = require("./state-dir-lock");
 
 const STATE_DIR = resolveNemoclawStateDir();
 
@@ -331,114 +335,18 @@ function isShieldsState(value: unknown): value is ShieldsState {
 }
 
 // ---------------------------------------------------------------------------
-// NC-2227-05: State directories locked by shields-up.
-//
-// During shields-up, these must be locked (root:root 755) so the sandbox
-// user cannot create new entries or modify existing ones. This covers both
-// executable state (skills, hooks, cron jobs, extensions, plugins, agent
-// definitions) and writable agent state entry points such as workspace and
-// memory, so a stale symlink bridge cannot bypass the lockdown.
-//
-// The list is a superset: directories that don't exist in a given agent's
-// config dir are silently skipped.
+// State-dir lock — adapter between this module's privileged-exec helpers and
+// the lock pipeline in ./state-dir-lock. The inventory of locked dirs, the
+// preflight/mutation/verification logic, and the `agents/*/sessions`
+// carve-out live in that sibling module so this file stays focused on
+// shields state transitions.
 // ---------------------------------------------------------------------------
 
-const HIGH_RISK_STATE_DIRS = [
-  "skills",
-  "hooks",
-  "cron",
-  "agents",
-  "extensions",
-  "plugins", // Hermes equivalent of extensions
-  "workspace",
-  "memory",
-  "credentials",
-  "identity",
-  "devices",
-  "canvas",
-  "telegram",
-];
-
-function applyStateDirLockMode(
-  sandboxName: string,
-  configDir: string,
-  owner: string,
-): void {
-  // Locking (shields-up) strips group + world write. Unlocking (shields-down)
-  // restores the same group-readable/writable + o-rwx mutable-default contract
-  // as startup, plus setgid so the gateway UID — now in the sandbox group via
-  // Dockerfile.base — can write to OpenClaw's mutable config tree (#2681).
-  //
-  // The unlock variant uses `g+rwX,o-rwx` because a prior lock can strip group
-  // access from descendants. Without re-adding group read/write explicitly,
-  // shields-down would leave nested files readable/writable only by owner.
-  const isLocking = owner === "root:root";
-  const recursiveMode = isLocking ? "go-w" : "g+rwX,o-rwx";
-  const dirMode = isLocking ? "755" : "2770";
-
-  for (const dirName of HIGH_RISK_STATE_DIRS) {
-    const dirPath = `${configDir}/${dirName}`;
-    try {
-      privilegedSandboxExec(sandboxName, ["chown", "-R", owner, dirPath]);
-    } catch {
-      // Directory may not exist for this agent — silently skip
-    }
-    try {
-      privilegedSandboxExec(sandboxName, ["chmod", dirMode, dirPath]);
-    } catch {
-      // Silently skip
-    }
-    if (isLocking) {
-      try {
-        privilegedSandboxExec(sandboxName, ["chmod", "g-s", dirPath]);
-      } catch {
-        // Best effort; do not skip recursive write stripping.
-      }
-    }
-    try {
-      privilegedSandboxExec(sandboxName, [
-        "chmod",
-        "-R",
-        recursiveMode,
-        dirPath,
-      ]);
-    } catch {
-      // Silently skip
-    }
-  }
-
-  // Multi-agent OpenClaw workspaces are named workspace-<agent>. They are
-  // discovered dynamically because they are configured by openclaw.json.
-  const clearSetgid = isLocking ? "1" : "0";
-  try {
-    privilegedSandboxExec(sandboxName, [
-      "sh",
-      "-c",
-      `
-set -u
-config_dir="$1"
-owner="$2"
-recursive_mode="$3"
-dir_mode="$4"
-clear_setgid="$5"
-for dir in "$config_dir"/workspace-*; do
-  [ -d "$dir" ] || continue
-  chown -R "$owner" "$dir" 2>/dev/null || true
-  chmod "$dir_mode" "$dir" 2>/dev/null || true
-  [ "$clear_setgid" = "1" ] && chmod g-s "$dir" 2>/dev/null || true
-  chmod -R "$recursive_mode" "$dir" 2>/dev/null || true
-done
-`,
-      "sh",
-      configDir,
-      owner,
-      recursiveMode,
-      dirMode,
-      clearSetgid,
-    ]);
-  } catch {
-    // Best effort; verification below catches the primary config lock.
-  }
+function stateDirLockExec(sandboxName: string) {
+  return {
+    exec: (cmd: string[]) => privilegedSandboxExec(sandboxName, cmd),
+    capture: (cmd: string[]) => privilegedSandboxExecCapture(sandboxName, cmd),
+  };
 }
 
 function legacyDataDirFor(configDir: string): string {
@@ -542,8 +450,18 @@ function unlockAgentConfig(
 
   // NC-2227-05: Restore sandbox ownership on locked state directories.
   // Use chown -R to restore the full tree (files within may have been
-  // locked to root:root by a prior shields-up).
-  applyStateDirLockMode(sandboxName, target.configDir, "sandbox:sandbox");
+  // locked to root:root by a prior shields-up). Surface fan-out issues
+  // so `shields down` cannot report success while a state dir is still
+  // root-owned or read-only.
+  const stateDirUnlockIssues = applyStateDirLockMode(
+    stateDirLockExec(sandboxName),
+    target.configDir,
+    "sandbox:sandbox",
+    false,
+  );
+  for (const issue of stateDirUnlockIssues) {
+    errors.push(`state dir unlock: ${issue}`);
+  }
 
   if (errors.length > 0) {
     console.error(
@@ -652,6 +570,18 @@ function lockAgentConfig(
   const errors: string[] = [];
   const filesToLock = [target.configPath, ...(target.sensitiveFiles || [])];
 
+  // Symlink preflight runs before any file or directory mutation: if a
+  // pre-lockdown agent swapped e.g. `extensions/` for a symlink to /etc,
+  // we abort before the privileged chmod/chown touches anything, so the
+  // tree is never half-mutated against an attacker-controlled host path.
+  const preflightIssues = preflightStateDirLock(
+    stateDirLockExec(sandboxName),
+    target.configDir,
+  );
+  if (preflightIssues.length > 0) {
+    throw new Error(`Config not locked: ${preflightIssues.join(", ")}`);
+  }
+
   for (const f of filesToLock) {
     try {
       privilegedSandboxExec(sandboxName, ["chmod", "444", f]);
@@ -692,10 +622,24 @@ function lockAgentConfig(
     }
   }
 
-  // NC-2227-05: Lock state directories. Root-own the directory and set 755 so
-  // the sandbox user can read/execute but cannot create new entries or modify
-  // existing ones.
-  applyStateDirLockMode(sandboxName, target.configDir, "root:root");
+  // Lock state directories. High-risk dirs use `root:sandbox` ownership so
+  // the gateway (in the sandbox group) can still read plugin/agent code while
+  // the sandbox user is denied write through `chmod -R go-w`. Secret-bearing
+  // dirs (CONFIDENTIALITY_STATE_DIRS in ./state-dir-lock) go to `root:root`
+  // 700/go-rwX so neither the sandbox user nor the gateway can read them
+  // while shields are up. Top-level configDir stays root:root.
+  const stateDirLockIssues = applyStateDirLockMode(
+    stateDirLockExec(sandboxName),
+    target.configDir,
+    "root:sandbox",
+    true,
+  );
+  if (stateDirLockIssues.length > 0) {
+    // Symlinked state-dir roots are a security-relevant violation:
+    // continuing would let shields-up report "locked" while a state
+    // dir still points at a writable host path. Refuse the lock.
+    throw new Error(`Config not locked: ${stateDirLockIssues.join(", ")}`);
+  }
 
   // OpenClaw's mutable-default config root is setgid (#2681). Clear setgid
   // after descendant locking so shields-up verifies the root config dir as