From 643a9fa9c8d89c00e5ab82cf3efb05e08b50cf89 Mon Sep 17 00:00:00 2001
From: subrih <skovilmadam@gmail.com>
Date: Fri, 13 Mar 2026 20:29:30 -0700
Subject: [PATCH] fix(access-policy): denyVerdict default rwx, bwrap /tmp
 policy-gated, file-like path auto-expand

---
 src/agents/bash-tools.exec-runtime.ts |  4 ++++
 src/infra/access-policy.test.ts       | 10 ++++++++++
 src/infra/access-policy.ts            | 18 ++++++++++++++++--
 src/infra/exec-sandbox-bwrap.test.ts  |  9 +++++++++
 src/infra/exec-sandbox-bwrap.ts       | 11 +++++++++--
 5 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/src/agents/bash-tools.exec-runtime.ts b/src/agents/bash-tools.exec-runtime.ts
index b146f9acaee..67cd18ced2e 100644
--- a/src/agents/bash-tools.exec-runtime.ts
+++ b/src/agents/bash-tools.exec-runtime.ts
@@ -387,8 +387,12 @@ export async function runExecProcess(opts: {
       Object.keys(_scripts).some(
         (k) => k.startsWith("~") && k.replace(/^~(?=$|[/\\])/, os.homedir()) === argv0,
       );
+    // Use default:"rwx" so only actual deny-pattern hits produce "deny".
+    // Without a default, permAllows(undefined, "exec") → false → every path
+    // not matched by a deny pattern would be incorrectly blocked.
     const denyVerdict = checkAccessPolicy(argv0, "exec", {
       deny: opts.permissions.deny,
+      default: "rwx",
     });
     if (denyVerdict === "deny") {
       throw new Error(`exec denied by access policy: ${argv0}`);
diff --git a/src/infra/access-policy.test.ts b/src/infra/access-policy.test.ts
index b4b6b8ffdbf..d9d0970630a 100644
--- a/src/infra/access-policy.test.ts
+++ b/src/infra/access-policy.test.ts
@@ -104,6 +104,16 @@ describe("validateAccessPolicyConfig", () => {
     expect(errs[0]).toMatch(/auto-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
+    // the file itself, not a subtree of non-existent children.
+    const fileLikePath = path.join(os.tmpdir(), `openclaw-test-${Date.now()}.key`);
+    const config: AccessPolicyConfig = { deny: [fileLikePath] };
+    validateAccessPolicyConfig(config);
+    expect(config.deny?.[0]).toBe(fileLikePath); // must NOT be expanded to /**
+  });
+
   it("does NOT auto-expand a bare deny[] entry that is an existing file", () => {
     // A specific file like "~/.ssh/id_rsa" must stay as an exact-match pattern.
     // Expanding it to "~/.ssh/id_rsa/**" would only match non-existent children,
diff --git a/src/infra/access-policy.ts b/src/infra/access-policy.ts
index c86aa2c2cd1..3a15374e3d4 100644
--- a/src/infra/access-policy.ts
+++ b/src/infra/access-policy.ts
@@ -94,12 +94,26 @@ export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[]
           ? pattern.replace(/^~(?=$|[/\\])/, os.homedir())
           : pattern;
         let isExistingFile = false;
+        // For non-existent paths: treat as a future file (skip /**-expansion) when
+        // the last segment looks like a filename — has a dot but is not a dotfile-only
+        // name (e.g. ".ssh") and has a non-empty extension (e.g. "secrets.key").
+        // This preserves the intent of "deny: ['~/future-secrets.key']" where the
+        // user wants to protect that specific file once it is created.
+        // Plain names without an extension (e.g. "myfolder") are still treated as
+        // future directories and expanded to /**.
+        let looksLikeFile = false;
         try {
           isExistingFile = !fs.statSync(expandedForStat).isDirectory();
         } catch {
-          // Path does not exist — treat as a future directory and expand to /**.
+          const lastName =
+            expandedForStat
+              .replace(/[/\\]$/, "")
+              .split(/[/\\]/)
+              .pop() ?? "";
+          // Has a dot that is not the leading dot (dotfile), and has chars after the dot.
+          looksLikeFile = /[^.]\.[^/\\]+$/.test(lastName);
         }
-        if (!isExistingFile) {
+        if (!isExistingFile && !looksLikeFile) {
           const fixed = `${pattern}/**`;
           config.deny[i] = fixed;
           if (!_autoExpandedWarned.has(`deny:${pattern}`)) {
diff --git a/src/infra/exec-sandbox-bwrap.test.ts b/src/infra/exec-sandbox-bwrap.test.ts
index df771b76f10..dae956bdcff 100644
--- a/src/infra/exec-sandbox-bwrap.test.ts
+++ b/src/infra/exec-sandbox-bwrap.test.ts
@@ -115,6 +115,15 @@ describe("generateBwrapArgs", () => {
     expect(tmpfsMounts).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.
+    const config: AccessPolicyConfig = { default: "r--", rules: { "/tmp/**": "r--" } };
+    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("does not add --tmpfs /tmp in restrictive mode (default: ---)", () => {
     const config: AccessPolicyConfig = { default: "---" };
     const args = generateBwrapArgs(config, HOME);
diff --git a/src/infra/exec-sandbox-bwrap.ts b/src/infra/exec-sandbox-bwrap.ts
index 0c570255597..452e06d44a6 100644
--- a/src/infra/exec-sandbox-bwrap.ts
+++ b/src/infra/exec-sandbox-bwrap.ts
@@ -3,6 +3,7 @@ import fs from "node:fs";
 import os from "node:os";
 import { promisify } from "node:util";
 import type { AccessPolicyConfig, PermStr } from "../config/types.tools.js";
+import { findBestRule } from "./access-policy.js";
 import { shellEscape } from "./shell-escape.js";
 
 const execFileAsync = promisify(execFile);
@@ -142,8 +143,14 @@ 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");
-    // Upgrade /tmp to writable tmpfs and overlay a real /dev for normal process operation.
-    args.push("--tmpfs", "/tmp");
+    // 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.