mirror of https://github.com/openclaw/openclaw.git
fix(access-policy): denyVerdict default rwx, bwrap /tmp policy-gated, file-like path auto-expand
This commit is contained in:
parent
a7fb92a606
commit
643a9fa9c8
|
|
@ -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}`);
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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}`)) {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Reference in New Issue