fix(access-policy): validate scripts rules/deny, tighten looksLikeFile, explicit bwrap /tmp guard

src/infra/access-policy.test.ts

@@ -104,6 +104,18 @@ describe("validateAccessPolicyConfig", () => {
     expect(errs[0]).toMatch(/auto-expanded/);
   });
 
+  it("auto-expands non-existent versioned directory names (v1.0, app-2.3) in deny[]", () => {
+    // Versioned names like "v1.0" or "pkg-1.0.0" look like files via naive dot-detection
+    // but are almost always directories. The tightened heuristic requires the extension
+    // to contain at least one letter — digits-only extensions (like ".0") are treated as
+    // directory-like and expanded to /**.
+    const base = os.tmpdir();
+    const versionedDir = path.join(base, `openclaw-test-v1.0-${Date.now()}`);
+    const config: AccessPolicyConfig = { deny: [versionedDir] };
+    validateAccessPolicyConfig(config);
+    expect(config.deny?.[0]).toBe(`${versionedDir}/**`); // must be expanded
+  });
+
   it("does NOT auto-expand a non-existent deny[] path that looks like a file (has extension)", () => {
     // "~/future-secrets.key" doesn't exist yet but the extension heuristic should
     // prevent expansion to "~/future-secrets.key/**" — the user intends to protect
@@ -133,6 +145,32 @@ describe("validateAccessPolicyConfig", () => {
     expect(checkAccessPolicy(file, "read", config)).toBe("deny");
   });
 
+  it("validates scripts[].rules perm strings and emits diagnostics for bad ones", () => {
+    // A typo like "rwX" in a script's rules must produce a diagnostic, not silently
+    // fail closed (which would deny exec with no operator-visible error).
+    const config: AccessPolicyConfig = {
+      scripts: {
+        "/usr/local/bin/deploy.sh": {
+          rules: { "~/deploy/**": "rwX" }, // invalid: uppercase X
+        },
+      },
+    };
+    const errs = validateAccessPolicyConfig(config);
+    expect(errs.some((e) => e.includes("rwX") && e.includes("scripts"))).toBe(true);
+  });
+
+  it("validates scripts[].deny entries and emits diagnostics for empty patterns", () => {
+    const config: AccessPolicyConfig = {
+      scripts: {
+        "/usr/local/bin/deploy.sh": {
+          deny: ["", "~/.secrets/**"], // first entry is invalid empty string
+        },
+      },
+    };
+    const errs = validateAccessPolicyConfig(config);
+    expect(errs.some((e) => e.includes("scripts") && e.includes("deny"))).toBe(true);
+  });
+
   it("accepts valid 'rwx' and '---' perm strings", () => {
     expect(validateAccessPolicyConfig({ default: "rwx" })).toEqual([]);
     expect(validateAccessPolicyConfig({ default: "---" })).toEqual([]);

src/infra/access-policy.ts

@@ -75,6 +75,29 @@ export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[]
     }
   }
 
+  if (config.scripts) {
+    for (const [scriptPath, entry] of Object.entries(config.scripts)) {
+      if (entry.rules) {
+        for (const [pattern, perm] of Object.entries(entry.rules)) {
+          if (!PERM_STR_RE.test(perm)) {
+            errors.push(
+              `access-policy.scripts["${scriptPath}"].rules["${pattern}"] "${perm}" is invalid: must be exactly 3 chars (e.g. "rwx", "r--", "---")`,
+            );
+          }
+        }
+      }
+      if (entry.deny) {
+        for (let i = 0; i < entry.deny.length; i++) {
+          if (!entry.deny[i]) {
+            errors.push(
+              `access-policy.scripts["${scriptPath}"].deny[${i}] must be a non-empty glob pattern`,
+            );
+          }
+        }
+      }
+    }
+  }
+
   if (config.deny) {
     for (let i = 0; i < config.deny.length; i++) {
       const pattern = config.deny[i];
@@ -110,8 +133,11 @@ export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[]
               .replace(/[/\\]$/, "")
               .split(/[/\\]/)
               .pop() ?? "";
-          // Has a dot that is not the leading dot (dotfile), and has chars after the dot.
-          looksLikeFile = /[^.]\.[^/\\]+$/.test(lastName);
+          // Has a dot that is not the leading dot (dotfile), and the extension
+          // contains at least one letter — this excludes version-like suffixes
+          // (.0, .3, -1.0, app-2.3) which look like versioned directory names.
+          // Examples: "secrets.key" → file; "v1.0" → directory; ".ssh" → directory.
+          looksLikeFile = /[^.]\.[a-zA-Z][^/\\]*$/.test(lastName);
         }
         if (!isExistingFile && !looksLikeFile) {
           const fixed = `${pattern}/**`;

src/infra/exec-sandbox-bwrap.test.ts

@@ -115,6 +115,17 @@ describe("generateBwrapArgs", () => {
     expect(tmpfsMounts).toContain("/tmp");
   });
 
+  it("does not add --tmpfs /tmp in restrictive mode even with no explicit /tmp rule", () => {
+    // Regression guard: the defaultAllowsRead guard on the /tmp block must prevent
+    // a writable tmpfs being mounted under default:"---" when no /tmp rule exists.
+    // explicitTmpPerm === null is true (no rule), but defaultAllowsRead is false,
+    // so the entire /tmp block must be skipped.
+    const config: AccessPolicyConfig = { default: "---" };
+    const args = generateBwrapArgs(config, HOME);
+    const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
+    expect(tmpfsMounts).not.toContain("/tmp");
+  });
+
   it("skips --tmpfs /tmp in permissive mode when policy explicitly restricts /tmp writes", () => {
     // A rule "/tmp/**": "r--" means the user wants /tmp read-only; the base --ro-bind / /
     // already makes it readable. Adding --tmpfs /tmp would silently grant write access.

src/infra/exec-sandbox-bwrap.ts

@@ -143,14 +143,6 @@ export function generateBwrapArgs(
     // mount /proc so programs that read /proc/self/*, /proc/cpuinfo, etc. work
     // correctly inside the sandbox (shells, Python, most build tools need this).
     args.push("--proc", "/proc");
-    // Add writable /tmp tmpfs unless the policy has an explicit rule that denies
-    // write on /tmp. Without an explicit rule, /tmp is writable by default (needed
-    // by most processes for temp files). With an explicit "r--" or "---" rule on
-    // /tmp/**, respect it — /tmp remains read-only via the --ro-bind / / base.
-    const explicitTmpPerm = findBestRule("/tmp/.", config.rules ?? {}, homeDir);
-    if (explicitTmpPerm === null || explicitTmpPerm[1] === "w") {
-      args.push("--tmpfs", "/tmp");
-    }
     args.push("--dev", "/dev");
   } else {
     // Restrictive base: only bind system paths needed to run processes.
@@ -165,6 +157,18 @@ export function generateBwrapArgs(
     // the enclosed process genuinely needs it.
   }
 
+  // Writable /tmp tmpfs — only in permissive mode AND only when the policy does not
+  // explicitly restrict writes on /tmp. Keeping this outside the if/else block above
+  // makes the defaultAllowsRead guard self-evident and not implicit from nesting.
+  // In restrictive mode (default:"---"), /tmp is intentionally omitted so rules
+  // control tmpfs access explicitly (e.g. "/tmp/**":"rwx" is handled by the rules loop).
+  if (defaultAllowsRead) {
+    const explicitTmpPerm = findBestRule("/tmp/.", config.rules ?? {}, homeDir);
+    if (explicitTmpPerm === null || explicitTmpPerm[1] === "w") {
+      args.push("--tmpfs", "/tmp");
+    }
+  }
+
   // Apply rules: upgrade paths with w bit to read-write binds.
   // Sort by concrete path length ascending so less-specific mounts are applied
   // first — bwrap applies mounts in order, and later mounts win for overlapping