From b73a2de9f6137a01f9cac5d94b164cce7f798367 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 18 Feb 2026 17:38:11 +0000 Subject: [PATCH] refactor(infra): reuse shared home prefix expansion --- src/infra/exec-approvals-analysis.ts | 21 ++++------------- src/infra/exec-approvals.test.ts | 35 +++++++++++++++++++++++++--- src/infra/exec-approvals.ts | 23 ++++-------------- 3 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/infra/exec-approvals-analysis.ts b/src/infra/exec-approvals-analysis.ts index 0a8340eef53..a41d3347717 100644 --- a/src/infra/exec-approvals-analysis.ts +++ b/src/infra/exec-approvals-analysis.ts @@ -1,24 +1,11 @@ import fs from "node:fs"; -import os from "node:os"; import path from "node:path"; import { splitShellArgs } from "../utils/shell-argv.js"; import type { ExecAllowlistEntry } from "./exec-approvals.js"; +import { expandHomePrefix } from "./home-dir.js"; export const DEFAULT_SAFE_BINS = ["jq", "grep", "cut", "sort", "uniq", "head", "tail", "tr", "wc"]; -function expandHome(value: string): string { - if (!value) { - return value; - } - if (value === "~") { - return os.homedir(); - } - if (value.startsWith("~/")) { - return path.join(os.homedir(), value.slice(2)); - } - return value; -} - export type CommandResolution = { rawExecutable: string; resolvedPath?: string; @@ -58,7 +45,7 @@ function parseFirstToken(command: string): string | null { } function resolveExecutablePath(rawExecutable: string, cwd?: string, env?: NodeJS.ProcessEnv) { - const expanded = rawExecutable.startsWith("~") ? expandHome(rawExecutable) : rawExecutable; + const expanded = rawExecutable.startsWith("~") ? expandHomePrefix(rawExecutable) : rawExecutable; if (expanded.includes("/") || expanded.includes("\\")) { if (path.isAbsolute(expanded)) { return isExecutableFile(expanded) ? expanded : undefined; @@ -172,7 +159,7 @@ function matchesPattern(pattern: string, target: string): boolean { if (!trimmed) { return false; } - const expanded = trimmed.startsWith("~") ? expandHome(trimmed) : trimmed; + const expanded = trimmed.startsWith("~") ? expandHomePrefix(trimmed) : trimmed; const hasWildcard = /[*?]/.test(expanded); let normalizedPattern = expanded; let normalizedTarget = target; @@ -200,7 +187,7 @@ export function resolveAllowlistCandidatePath( if (!raw) { return undefined; } - const expanded = raw.startsWith("~") ? expandHome(raw) : raw; + const expanded = raw.startsWith("~") ? expandHomePrefix(raw) : raw; if (!expanded.includes("/") && !expanded.includes("\\")) { return undefined; } diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index bbb73a4a040..0a7e1784c8c 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -1,7 +1,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { analyzeArgvCommand, analyzeShellCommand, @@ -19,6 +19,8 @@ import { resolveCommandResolution, resolveExecApprovals, resolveExecApprovalsFromFile, + resolveExecApprovalsPath, + resolveExecApprovalsSocketPath, type ExecAllowlistEntry, type ExecApprovalsFile, } from "./exec-approvals.js"; @@ -110,6 +112,28 @@ describe("mergeExecApprovalsSocketDefaults", () => { }); }); +describe("resolve exec approvals defaults", () => { + it("expands home-prefixed default file and socket paths", () => { + const dir = makeTempDir(); + const prevOpenClawHome = process.env.OPENCLAW_HOME; + try { + process.env.OPENCLAW_HOME = dir; + expect(path.normalize(resolveExecApprovalsPath())).toBe( + path.normalize(path.join(dir, ".openclaw", "exec-approvals.json")), + ); + expect(path.normalize(resolveExecApprovalsSocketPath())).toBe( + path.normalize(path.join(dir, ".openclaw", "exec-approvals.sock")), + ); + } finally { + if (prevOpenClawHome === undefined) { + delete process.env.OPENCLAW_HOME; + } else { + process.env.OPENCLAW_HOME = prevOpenClawHome; + } + } + }); +}); + describe("exec approvals safe shell command builder", () => { it("quotes only safeBins segments (leaves other segments untouched)", () => { if (process.platform === "win32") { @@ -599,9 +623,10 @@ describe("exec approvals policy helpers", () => { describe("exec approvals wildcard agent", () => { it("merges wildcard allowlist entries with agent entries", () => { const dir = makeTempDir(); - const homedirSpy = vi.spyOn(os, "homedir").mockReturnValue(dir); + const prevOpenClawHome = process.env.OPENCLAW_HOME; try { + process.env.OPENCLAW_HOME = dir; const approvalsPath = path.join(dir, ".openclaw", "exec-approvals.json"); fs.mkdirSync(path.dirname(approvalsPath), { recursive: true }); fs.writeFileSync( @@ -625,7 +650,11 @@ describe("exec approvals wildcard agent", () => { "/usr/bin/uname", ]); } finally { - homedirSpy.mockRestore(); + if (prevOpenClawHome === undefined) { + delete process.env.OPENCLAW_HOME; + } else { + process.env.OPENCLAW_HOME = prevOpenClawHome; + } } }); }); diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index 56daa99e582..b2d425e4313 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -1,8 +1,8 @@ import crypto from "node:crypto"; import fs from "node:fs"; -import os from "node:os"; import path from "node:path"; import { DEFAULT_AGENT_ID } from "../routing/session-key.js"; +import { expandHomePrefix } from "./home-dir.js"; import { requestJsonlSocket } from "./jsonl-socket.js"; export * from "./exec-approvals-analysis.js"; export * from "./exec-approvals-allowlist.js"; @@ -98,25 +98,12 @@ function hashExecApprovalsRaw(raw: string | null): string { .digest("hex"); } -function expandHome(value: string): string { - if (!value) { - return value; - } - if (value === "~") { - return os.homedir(); - } - if (value.startsWith("~/")) { - return path.join(os.homedir(), value.slice(2)); - } - return value; -} - export function resolveExecApprovalsPath(): string { - return expandHome(DEFAULT_FILE); + return expandHomePrefix(DEFAULT_FILE); } export function resolveExecApprovalsSocketPath(): string { - return expandHome(DEFAULT_SOCKET); + return expandHomePrefix(DEFAULT_SOCKET); } function normalizeAllowlistPattern(value: string | undefined): string | null { @@ -370,7 +357,7 @@ export function resolveExecApprovals( agentId, overrides, path: resolveExecApprovalsPath(), - socketPath: expandHome(file.socket?.path ?? resolveExecApprovalsSocketPath()), + socketPath: expandHomePrefix(file.socket?.path ?? resolveExecApprovalsSocketPath()), token: file.socket?.token ?? "", }); } @@ -421,7 +408,7 @@ export function resolveExecApprovalsFromFile(params: { ]; return { path: params.path ?? resolveExecApprovalsPath(), - socketPath: expandHome( + socketPath: expandHomePrefix( params.socketPath ?? file.socket?.path ?? resolveExecApprovalsSocketPath(), ), token: params.token ?? file.socket?.token ?? "",