fix(state): heal ~/.nemoclaw directory and file permissions on read path

src/lib/state/config-io.test.ts

@@ -137,6 +137,178 @@ describe("config-io", () => {
     }
   });
 
+  it("readConfigFile repairs a 755 parent directory to 700", () => {
+    const root = makeTempDir();
+    const dir = path.join(root, "loose-dir");
+    fs.mkdirSync(dir, { mode: 0o755 });
+    const file = path.join(dir, "config.json");
+    fs.writeFileSync(file, JSON.stringify({ repaired: true }), { mode: 0o600 });
+
+    const result = readConfigFile(file, null);
+
+    expect(result).toEqual({ repaired: true });
+    expect(fs.statSync(dir).mode & 0o777).toBe(0o700);
+  });
+
+  it("readConfigFile repairs a 644 file to 600", () => {
+    const dir = makeTempDir();
+    fs.chmodSync(dir, 0o700);
+    const file = path.join(dir, "config.json");
+    fs.writeFileSync(file, JSON.stringify({ tight: true }), { mode: 0o644 });
+
+    const result = readConfigFile(file, null);
+
+    expect(result).toEqual({ tight: true });
+    expect(fs.statSync(file).mode & 0o777).toBe(0o600);
+  });
+
+  it("ensureConfigDir heals every root-level file in the dir, not just the one being read (#4546)", () => {
+    // #4546 expects auto-repair across all root-level files. Most of those
+    // files (onboard-session.json, ollama-proxy-token, etc.) are written by
+    // code paths that don't flow through readConfigFile, so the read-time
+    // per-file heal alone misses them. The dir walk in ensureConfigDir is
+    // what covers them — verify by writing several siblings at 644 and
+    // confirming a single read tightens all of them to 600.
+    //
+    // The walk is scoped to the host ~/.nemoclaw root, so the test sets
+    // HOME to a temp dir and writes under <home>/.nemoclaw.
+    const fakeHome = makeTempDir();
+    withHome(fakeHome, () => {
+      const dir = path.join(fakeHome, ".nemoclaw");
+      fs.mkdirSync(dir, { mode: 0o700 });
+      const target = path.join(dir, "config.json");
+      fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 });
+
+      const siblings = [
+        "onboard-session.json",
+        "ollama-proxy-token",
+        "ollama-auth-proxy.pid",
+        "usage-notice.json",
+      ];
+      for (const name of siblings) {
+        fs.writeFileSync(path.join(dir, name), "stale", { mode: 0o644 });
+      }
+
+      readConfigFile(target, null);
+
+      for (const name of siblings) {
+        const mode = fs.statSync(path.join(dir, name)).mode & 0o777;
+        expect(mode, `${name} should be tightened to 600`).toBe(0o600);
+      }
+    });
+  });
+
+  it("ensureConfigDir skips symlinks during the root-level heal", () => {
+    // A chmod on a symlink follows to the target — if ~/.nemoclaw/X is a
+    // symlink to /etc/passwd, healing must NOT chmod /etc/passwd. lstat
+    // before chmod keeps the heal scoped to real files inside the dir.
+    //
+    // Positive control: a regular sibling at 0o644 proves the walker
+    // actually ran (it should be tightened to 0o600). Without the
+    // control, this test would pass vacuously if the walker were a no-op.
+    const fakeHome = makeTempDir();
+    withHome(fakeHome, () => {
+      const dir = path.join(fakeHome, ".nemoclaw");
+      fs.mkdirSync(dir, { mode: 0o700 });
+      const target = path.join(dir, "config.json");
+      fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 });
+
+      const sibling = path.join(dir, "should-be-healed.json");
+      fs.writeFileSync(sibling, "stale", { mode: 0o644 });
+
+      const outsideDir = makeTempDir();
+      const outside = path.join(outsideDir, "target");
+      fs.writeFileSync(outside, "outside", { mode: 0o644 });
+      const linkPath = path.join(dir, "rogue-link");
+      fs.symlinkSync(outside, linkPath);
+
+      readConfigFile(target, null);
+      expect(
+        fs.statSync(sibling).mode & 0o777,
+        "positive control: walker tightened the regular sibling",
+      ).toBe(0o600);
+      expect(
+        fs.statSync(outside).mode & 0o777,
+        "symlink target must not be chmodded through the link",
+      ).toBe(0o644);
+    });
+  });
+
+  it("readConfigFile does not chmod through a symlink even via the per-file heal", () => {
+    // Defensive duplicate of the symlink check, this time for the per-file
+    // heal in readConfigFile itself (not the dir walk in ensureConfigDir).
+    const dir = makeTempDir();
+    fs.chmodSync(dir, 0o700);
+
+    const outsideDir = makeTempDir();
+    const outside = path.join(outsideDir, "target.json");
+    fs.writeFileSync(outside, JSON.stringify({ outside: true }), { mode: 0o644 });
+    const symlinkPath = path.join(dir, "config.json");
+    fs.symlinkSync(outside, symlinkPath);
+
+    // Reading through the symlink should not chmod the target file.
+    readConfigFile(symlinkPath, null);
+    expect(fs.statSync(outside).mode & 0o777).toBe(0o644);
+    // Cleanup via afterEach (both dirs are tracked in tmpDirs).
+  });
+
+  // ── Scope-boundary tests (cv's PR #4628 feedback) ──────────────────────
+  // The 700/600 heal is HOST-state-only — it must not normalize mutable
+  // sandbox OpenClaw config trees (2770/660 per #4538) or arbitrary
+  // config directories that may have their own permission contracts.
+
+  function withHome<T>(home: string, fn: () => T): T {
+    const previous = process.env.HOME;
+    process.env.HOME = home;
+    try {
+      return fn();
+    } finally {
+      if (previous === undefined) delete process.env.HOME;
+      else process.env.HOME = previous;
+    }
+  }
+
+  it("ensureConfigDir does NOT heal siblings when dirPath is not the host ~/.nemoclaw root", () => {
+    // An arbitrary config dir (not the host nemoclaw state root) must
+    // leave sibling perms alone — otherwise a future caller pointing
+    // ensureConfigDir at a mutable-sandbox or third-party state dir
+    // would silently tighten files that have a different contract.
+    const fakeHome = makeTempDir();
+    withHome(fakeHome, () => {
+      const unrelatedDir = path.join(makeTempDir(), "other-tool-state");
+      fs.mkdirSync(unrelatedDir, { recursive: true, mode: 0o700 });
+      const target = path.join(unrelatedDir, "config.json");
+      fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 });
+      const sibling = path.join(unrelatedDir, "other.json");
+      fs.writeFileSync(sibling, "stale", { mode: 0o644 });
+
+      readConfigFile(target, null);
+
+      expect(
+        fs.statSync(sibling).mode & 0o777,
+        "sibling under an unrelated dir must keep its mode",
+      ).toBe(0o644);
+    });
+  });
+
+  it("ensureConfigDir DOES heal siblings when dirPath IS the host ~/.nemoclaw root", () => {
+    // Positive control for the scope boundary: when the path is the host
+    // nemoclaw state root, the walk fires as before (#4546 acceptance).
+    const fakeHome = makeTempDir();
+    withHome(fakeHome, () => {
+      const hostDir = path.join(fakeHome, ".nemoclaw");
+      fs.mkdirSync(hostDir, { mode: 0o700 });
+      const target = path.join(hostDir, "sandboxes.json");
+      fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 });
+      const sibling = path.join(hostDir, "onboard-session.json");
+      fs.writeFileSync(sibling, "stale", { mode: 0o644 });
+
+      readConfigFile(target, null);
+
+      expect(fs.statSync(sibling).mode & 0o777).toBe(0o600);
+    });
+  });
+
   it("supports both rich and legacy constructor forms", () => {
     const rich = new ConfigPermissionError("test error", "/some/path");
     expect(rich.name).toBe("ConfigPermissionError");

src/lib/state/config-io.ts

@@ -17,6 +17,32 @@ type JsonValue = JsonScalar | JsonObject | JsonValue[];
 type JsonObject = { [key: string]: JsonValue };
 type SerializableConfig = JsonScalar | JsonValue[] | object;
 
+// Host-state vs sandbox-internal scoping. `ensureConfigDir` is a general
+// helper, but the 700/600 perm-heal contract only applies to the host's
+// `~/.nemoclaw` state directory. The mutable-sandbox OpenClaw config tree
+// (`/sandbox/.openclaw`, `openclaw.json`) has a different contract — 2770
+// directory / 660 file — so silently normalizing it would reintroduce the
+// EACCES bug that #4538 / PR #4610 are fighting. These predicates let the
+// heal opt out cleanly when a caller routes a sandbox-internal path here.
+function hostNemoclawDir(): string {
+  const home = process.env.HOME ?? os.homedir();
+  return path.resolve(home, ".nemoclaw");
+}
+
+function isHostNemoclawRoot(dirPath: string): boolean {
+  return path.resolve(dirPath) === hostNemoclawDir();
+}
+
+const MUTABLE_SANDBOX_CONFIG_ROOT = "/sandbox/.openclaw";
+
+function isMutableSandboxConfigPath(targetPath: string): boolean {
+  const resolved = path.resolve(targetPath);
+  return (
+    resolved === MUTABLE_SANDBOX_CONFIG_ROOT ||
+    resolved.startsWith(`${MUTABLE_SANDBOX_CONFIG_ROOT}/`)
+  );
+}
+
 function toError(error: Error | string | number | boolean | null | undefined): Error {
   return error instanceof Error ? error : new Error(String(error));
 }
@@ -145,16 +171,56 @@ export function rejectSymlinksOnPath(dirPath: string): void {
   }
 }
 
+/**
+ * Tighten group/world bits on every regular file directly inside `dirPath`.
+ * Symlinks are skipped (we use `lstat`; a chmod on a symlink follows to the
+ * target, which would mutate something outside the config dir). Subdirectories
+ * are skipped — this is intentionally root-level only, matching the issue's
+ * acceptance criteria (#4546). Best-effort: a single file's chmod failure
+ * does not abort the walk.
+ */
+function healRootLevelFiles(dirPath: string): void {
+  let entries: fs.Dirent[];
+  try {
+    entries = fs.readdirSync(dirPath, { withFileTypes: true });
+  } catch {
+    // If we can't list (e.g. dir does not exist), nothing to heal.
+    return;
+  }
+  for (const entry of entries) {
+    if (entry.isSymbolicLink() || !entry.isFile()) continue;
+    const full = path.join(dirPath, entry.name);
+    try {
+      // lstat (not stat) so a TOCTOU-swapped symlink between readdir and
+      // chmod doesn't trick us into chmodding a target outside the dir.
+      const st = fs.lstatSync(full);
+      if (!st.isFile()) continue;
+      if ((st.mode & 0o077) !== 0) {
+        fs.chmodSync(full, 0o600);
+      }
+    } catch {
+      // Best effort — keep walking.
+    }
+  }
+}
+
 export function ensureConfigDir(dirPath: string): void {
   // SECURITY: Block symlink attacks before creating or writing to the directory.
   rejectSymlinksOnPath(dirPath);
 
+  // The 700/dir + 600/file contract is host-state only. Mutable sandbox
+  // OpenClaw config paths use 2770/660 (#4538) and must not be normalized
+  // here even if a caller routes them through ensureConfigDir.
+  const mutableSandboxPath = isMutableSandboxConfigPath(dirPath);
+
   try {
     fs.mkdirSync(dirPath, { recursive: true, mode: 0o700 });
 
-    const stat = fs.statSync(dirPath);
-    if ((stat.mode & 0o077) !== 0) {
-      fs.chmodSync(dirPath, 0o700);
+    if (!mutableSandboxPath) {
+      const stat = fs.statSync(dirPath);
+      if ((stat.mode & 0o077) !== 0) {
+        fs.chmodSync(dirPath, 0o700);
+      }
     }
   } catch (error) {
     const errnoError = error instanceof Error ? error : null;
@@ -181,11 +247,50 @@ export function ensureConfigDir(dirPath: string): void {
     }
     throw error;
   }
+
+  // Heal every root-level file ONLY when this is the host `~/.nemoclaw`
+  // root. #4546's acceptance criteria scope to that exact dir (sandboxes.json,
+  // onboard-session.json, ollama-auth-proxy.pid, ollama-proxy-token,
+  // usage-notice.json). Walking siblings of arbitrary config dirs could
+  // silently normalize a mutable-sandbox config tree (#4538) or any future
+  // contract where 600 is wrong, so the heal stays opt-in by path.
+  if (isHostNemoclawRoot(dirPath)) {
+    healRootLevelFiles(dirPath);
+  }
 }
 
 export function readConfigFile<T>(filePath: string, fallback: T): T {
   try {
-    return parseJson<T>(fs.readFileSync(filePath, "utf-8"));
+    ensureConfigDir(path.dirname(filePath));
+  } catch (error) {
+    if (error instanceof ConfigPermissionError) {
+      throw error;
+    }
+    // Directory doesn't exist and can't be created — fall through to let
+    // readFileSync produce the appropriate ENOENT / fallback path.
+  }
+
+  try {
+    const content = parseJson<T>(fs.readFileSync(filePath, "utf-8"));
+
+    // Heal file-level permission drift: tighten group/world bits. lstat
+    // (not stat) so we don't chmod through a symlink to a target outside
+    // the config dir. Skip mutable-sandbox OpenClaw config paths so a
+    // future caller reading openclaw.json doesn't accidentally tighten
+    // its 660 contract (#4538). Defensive duplicate of healRootLevelFiles
+    // for read paths whose dirname differs from what ensureConfigDir saw.
+    try {
+      if (!isMutableSandboxConfigPath(filePath)) {
+        const st = fs.lstatSync(filePath);
+        if (st.isFile() && (st.mode & 0o077) !== 0) {
+          fs.chmodSync(filePath, 0o600);
+        }
+      }
+    } catch {
+      // Best effort — don't fail the read if we can't heal permissions.
+    }
+
+    return content;
   } catch (error) {
     const errnoError = error instanceof Error ? error : null;
     if (isPermissionError(errnoError)) {