From b0603cb443d58db38bf5b1dc3c00169d7631371e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Jul 2026 03:24:19 -0700 Subject: [PATCH] refactor(fs): adopt shared safe filesystem primitives (#113596) * refactor(fs): adopt fs-safe helpers * fix(zalouser): keep stable per-profile QR temp path * fix(fs): route adoption through owned facades --- .../src/browser/extension-relay/relay-auth.ts | 10 +- extensions/canvas/doctor-contract-api.ts | 10 +- extensions/canvas/src/tool.test.ts | 21 +- extensions/canvas/src/tool.ts | 7 +- extensions/google/oauth.credentials.ts | 11 +- .../policy/src/doctor/policy-runtime.ts | 8 +- extensions/vault/src/cli.ts | 10 +- extensions/zalouser/src/qr-temp-file.test.ts | 38 ++++ extensions/zalouser/src/qr-temp-file.ts | 4 +- src/agents/auth-profiles/source-check.ts | 21 +- src/agents/sessions/tools/path-utils.ts | 21 +- src/agents/shell-snapshot.ts | 68 ++++--- src/commands/doctor-auth-flat-profiles.ts | 7 +- .../workspace-sync-local.ts | 13 +- src/infra/path-guards.ts | 1 + src/infra/tmp-openclaw-dir.ts | 186 ++++-------------- src/transcripts/store-artifacts.ts | 3 +- 17 files changed, 180 insertions(+), 259 deletions(-) create mode 100644 extensions/zalouser/src/qr-temp-file.test.ts diff --git a/extensions/browser/src/browser/extension-relay/relay-auth.ts b/extensions/browser/src/browser/extension-relay/relay-auth.ts index 705bc8de3c8d..8d740ab77b09 100644 --- a/extensions/browser/src/browser/extension-relay/relay-auth.ts +++ b/extensions/browser/src/browser/extension-relay/relay-auth.ts @@ -10,6 +10,7 @@ import crypto from "node:crypto"; import fs from "node:fs"; import path from "node:path"; +import { tryReadSecretFileSync } from "openclaw/plugin-sdk/secret-file-runtime"; import { resolveOAuthDir } from "openclaw/plugin-sdk/state-paths"; const RELAY_SECRET_FILE = "browser-extension-relay.secret"; @@ -26,11 +27,10 @@ function normalizeToken(raw: string): string | null { /** Read the host-local relay token, or null when it has not been created yet. */ export function readExtensionRelayToken(env: NodeJS.ProcessEnv = process.env): string | null { - try { - return normalizeToken(fs.readFileSync(resolveExtensionRelaySecretPath(env), "utf8")); - } catch { - return null; - } + return normalizeToken( + tryReadSecretFileSync(resolveExtensionRelaySecretPath(env), "browser extension relay secret") ?? + "", + ); } /** diff --git a/extensions/canvas/doctor-contract-api.ts b/extensions/canvas/doctor-contract-api.ts index 5a48339852e7..9ccf3732f82b 100644 --- a/extensions/canvas/doctor-contract-api.ts +++ b/extensions/canvas/doctor-contract-api.ts @@ -3,6 +3,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { resolvePluginConfigObject } from "openclaw/plugin-sdk/plugin-config-runtime"; import type { PluginDoctorStateMigration } from "openclaw/plugin-sdk/runtime-doctor"; +import { pathExists } from "openclaw/plugin-sdk/security-runtime"; import { asOptionalRecord as readRecord, readStringValue as readString, @@ -36,15 +37,6 @@ async function listDocumentIds(documentsDir: string): Promise { } } -async function pathExists(candidate: string): Promise { - try { - await fs.access(candidate); - return true; - } catch { - return false; - } -} - export const stateMigrations: PluginDoctorStateMigration[] = [ { id: "canvas-custom-root-documents-to-core", diff --git a/extensions/canvas/src/tool.test.ts b/extensions/canvas/src/tool.test.ts index 5771d54674fa..c290cbf0a8bd 100644 --- a/extensions/canvas/src/tool.test.ts +++ b/extensions/canvas/src/tool.test.ts @@ -3,7 +3,7 @@ import { mkdtemp, mkdir, rm, symlink, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createCanvasTool } from "./tool.js"; +import { CANVAS_JSONL_MAX_BYTES, createCanvasTool } from "./tool.js"; const VALID_A2UI_V08_JSONL = [ JSON.stringify({ @@ -80,6 +80,25 @@ describe("Canvas tool", () => { }, ); + it("rejects jsonlPath files above the shared bounded-read limit", async () => { + tempRoot = await mkdtemp(path.join(os.tmpdir(), "openclaw-canvas-tool-")); + const workspaceDir = path.join(tempRoot, "workspace"); + await mkdir(workspaceDir); + await writeFile( + path.join(workspaceDir, "events.jsonl"), + Buffer.alloc(CANVAS_JSONL_MAX_BYTES + 1), + ); + const tool = createCanvasTool({ workspaceDir }); + + await expect( + tool.execute("tool-call-1", { + action: "a2ui_push", + jsonlPath: "events.jsonl", + }), + ).rejects.toThrow(`exceeds ${CANVAS_JSONL_MAX_BYTES} bytes`); + expect(mocks.callGatewayTool).not.toHaveBeenCalled(); + }); + it("applies configured image limits to canvas snapshots", async () => { mocks.callGatewayTool.mockResolvedValue({ payload: { diff --git a/extensions/canvas/src/tool.ts b/extensions/canvas/src/tool.ts index 8dc6626843fd..dbe28e25bd28 100644 --- a/extensions/canvas/src/tool.ts +++ b/extensions/canvas/src/tool.ts @@ -17,6 +17,7 @@ import { } from "openclaw/plugin-sdk/channel-actions"; import { readFiniteNumberParam, readPositiveIntegerParam } from "openclaw/plugin-sdk/param-readers"; import type { AnyAgentTool, OpenClawConfig } from "openclaw/plugin-sdk/plugin-entry"; +import { readRegularFile } from "openclaw/plugin-sdk/security-runtime"; import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; import { validateSupportedA2UIJsonl } from "./a2ui-jsonl.js"; import { normalizeCanvasSnapshotFileExtension, parseCanvasSnapshotPayload } from "./cli-helpers.js"; @@ -32,6 +33,8 @@ type CanvasImageSanitizationLimits = { maxDimensionPx?: number; }; +export const CANVAS_JSONL_MAX_BYTES = 16 * 1024 * 1024; + function readGatewayCallOptions(params: Record) { return { gatewayUrl: readStringParam(params, "gatewayUrl", { trim: false }), @@ -78,7 +81,9 @@ async function readJsonlFromPath(jsonlPath: string, workspaceDir?: string): Prom if (!isPathInsideRoot(workspaceReal, resolvedReal)) { throw new Error("jsonlPath outside workspace"); } - return await fs.readFile(resolvedReal, "utf8"); + return ( + await readRegularFile({ filePath: resolvedReal, maxBytes: CANVAS_JSONL_MAX_BYTES }) + ).buffer.toString("utf8"); } function resolveCanvasImageSanitizationLimits( diff --git a/extensions/google/oauth.credentials.ts b/extensions/google/oauth.credentials.ts index 694fea0bbcb6..611af1d66bfe 100644 --- a/extensions/google/oauth.credentials.ts +++ b/extensions/google/oauth.credentials.ts @@ -1,13 +1,14 @@ // Google plugin module implements oauth.credentials behavior. -import { existsSync, readFileSync, readdirSync, realpathSync } from "node:fs"; +import { existsSync, readdirSync, realpathSync } from "node:fs"; import type { Dirent } from "node:fs"; import { delimiter, dirname, join } from "node:path"; +import { readSecretFileSync } from "openclaw/plugin-sdk/secret-file-runtime"; import { lowercasePreservingWhitespace } from "openclaw/plugin-sdk/string-coerce-runtime"; import { CLIENT_ID_KEYS, CLIENT_SECRET_KEYS } from "./oauth.shared.js"; type CredentialFs = { existsSync: (path: Parameters[0]) => ReturnType; - readFileSync: (path: Parameters[0], encoding: "utf8") => string; + readFileSync: (path: string, encoding: "utf8") => string; realpathSync: (path: Parameters[0]) => string; readdirSync: ( path: Parameters[0], @@ -17,7 +18,11 @@ type CredentialFs = { const defaultFs: CredentialFs = { existsSync, - readFileSync, + readFileSync: (path) => + readSecretFileSync(path, "Gemini CLI OAuth credentials", { + maxBytes: 1024 * 1024, + rejectHardlinks: false, + }), realpathSync, readdirSync, }; diff --git a/extensions/policy/src/doctor/policy-runtime.ts b/extensions/policy/src/doctor/policy-runtime.ts index 428b58fa750e..ae2e85909648 100644 --- a/extensions/policy/src/doctor/policy-runtime.ts +++ b/extensions/policy/src/doctor/policy-runtime.ts @@ -28,7 +28,7 @@ export async function readPolicyFile( ocDocName: basename(displayName), }; } catch (err) { - if (isNotFound(err)) { + if (isNotFoundPathError(err)) { return null; } throw err; @@ -48,7 +48,7 @@ export async function readExecApprovalsFile( ocDocName: "exec-approvals.json", }; } catch (err) { - if (isNotFound(err)) { + if (isNotFoundPathError(err)) { return null; } throw err; @@ -64,7 +64,7 @@ export async function readWorkspaceFile( const fs = await loadFsPromisesModule(); return { raw: await fs.readFile(path, "utf-8"), path }; } catch (err) { - if (isNotFound(err)) { + if (isNotFoundPathError(err)) { return null; } throw err; @@ -129,7 +129,7 @@ function resolveWorkspacePath(ctx: HealthCheckContext, fileName: string): string return resolve(ctx.cwd ?? process.cwd(), fileName); } -function isNotFound(err: unknown): boolean { +function isNotFoundPathError(err: unknown): boolean { return typeof err === "object" && err !== null && "code" in err && err.code === "ENOENT"; } diff --git a/extensions/vault/src/cli.ts b/extensions/vault/src/cli.ts index f3b5dfcf9943..66c362b9ecdc 100644 --- a/extensions/vault/src/cli.ts +++ b/extensions/vault/src/cli.ts @@ -4,6 +4,7 @@ import { createInterface } from "node:readline/promises"; import { fileURLToPath } from "node:url"; import type { OpenClawConfig } from "openclaw/plugin-sdk/plugin-entry"; import { resolveSecretPlanTargetByPath } from "openclaw/plugin-sdk/secret-ref-runtime"; +import { pathExists } from "openclaw/plugin-sdk/security-runtime"; import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; import { parseVaultSecretId } from "../vault-secret-id.js"; @@ -202,15 +203,6 @@ function resolveStatusProviderAlias(config: OpenClawConfig, requestedAlias?: str return configuredAliases[0] ?? VAULT_PROVIDER_ALIAS; } -async function pathExists(filePath: string): Promise { - try { - await fs.access(filePath); - return true; - } catch { - return false; - } -} - function resolverScriptPathCandidates(baseUrl: string): [string, string] { return [ fileURLToPath(new URL("../vault-secret-ref-resolver.js", baseUrl)), diff --git a/extensions/zalouser/src/qr-temp-file.test.ts b/extensions/zalouser/src/qr-temp-file.test.ts new file mode 100644 index 000000000000..ecd3090f494c --- /dev/null +++ b/extensions/zalouser/src/qr-temp-file.test.ts @@ -0,0 +1,38 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; +import { afterEach, describe, expect, it } from "vitest"; +import { writeQrDataUrlToTempFile } from "./qr-temp-file.js"; + +describe("writeQrDataUrlToTempFile", () => { + const profile = `test/profile-${process.pid}`; + const expectedPath = path.join( + resolvePreferredOpenClawTmpDir(), + `openclaw-zalouser-qr-test-profile-${process.pid}.png`, + ); + + afterEach(async () => { + await fs.rm(expectedPath, { force: true }); + }); + + it("overwrites the stable per-profile path and enforces private mode", async () => { + const firstData = Buffer.from("first-qr-image"); + const secondData = Buffer.from("second-qr-image"); + const first = await writeQrDataUrlToTempFile( + `data:image/png;base64,${firstData.toString("base64")}`, + profile, + ); + expect(first).toBe(expectedPath); + await fs.chmod(expectedPath, 0o644); + + const second = await writeQrDataUrlToTempFile( + `data:image/png;base64,${secondData.toString("base64")}`, + profile, + ); + expect(second).toBe(expectedPath); + await expect(fs.readFile(expectedPath)).resolves.toEqual(secondData); + if (process.platform !== "win32") { + expect((await fs.stat(expectedPath)).mode & 0o777).toBe(0o600); + } + }); +}); diff --git a/extensions/zalouser/src/qr-temp-file.ts b/extensions/zalouser/src/qr-temp-file.ts index 7d94c21626d1..841e0e3a899b 100644 --- a/extensions/zalouser/src/qr-temp-file.ts +++ b/extensions/zalouser/src/qr-temp-file.ts @@ -14,10 +14,12 @@ export async function writeQrDataUrlToTempFile( return null; } const safeProfile = profile.replace(/[^a-zA-Z0-9_-]+/g, "-") || "default"; + // The stable private-root name lets QR refreshes overwrite instead of accumulating temp files. const filePath = path.join( resolvePreferredOpenClawTmpDir(), `openclaw-zalouser-qr-${safeProfile}.png`, ); - await fsp.writeFile(filePath, Buffer.from(base64, "base64")); + await fsp.writeFile(filePath, Buffer.from(base64, "base64"), { mode: 0o600 }); + await fsp.chmod(filePath, 0o600); return filePath; } diff --git a/src/agents/auth-profiles/source-check.ts b/src/agents/auth-profiles/source-check.ts index 7417d058f5c1..d4b8092c2e6d 100644 --- a/src/agents/auth-profiles/source-check.ts +++ b/src/agents/auth-profiles/source-check.ts @@ -3,6 +3,7 @@ * These checks intentionally avoid loading secret-bearing credential payloads. */ import fs from "node:fs"; +import { tryReadJsonSync } from "../../infra/json-files.js"; import { evaluateStoredCredentialEligibility } from "./credential-state.js"; import { resolveAuthStatePath, @@ -27,14 +28,6 @@ function hasStoredAuthProfileFiles(agentDir?: string): boolean { ); } -function readJsonFile(pathname: string): unknown { - try { - return JSON.parse(fs.readFileSync(pathname, "utf8")) as unknown; - } catch { - return null; - } -} - function normalizeProvider(provider: string): string { return provider.trim().toLowerCase(); } @@ -153,13 +146,17 @@ export function hasAuthProfileStoreSourceForProvider( return true; } if ( - rawStoreHasProviderProfile(readJsonFile(resolveAuthStorePath(agentDir)), provider, profileIds) + rawStoreHasProviderProfile( + tryReadJsonSync(resolveAuthStorePath(agentDir)), + provider, + profileIds, + ) ) { return true; } if ( rawStoreHasProviderProfile( - readJsonFile(resolveLegacyAuthStorePath(agentDir)), + tryReadJsonSync(resolveLegacyAuthStorePath(agentDir)), provider, profileIds, ) @@ -179,11 +176,11 @@ export function hasAuthProfileStoreSourceForProvider( if (runtimeStoreHasProviderProfile(mainRuntimeStore, provider, profileIds)) { return true; } - if (rawStoreHasProviderProfile(readJsonFile(resolveAuthStorePath()), provider, profileIds)) { + if (rawStoreHasProviderProfile(tryReadJsonSync(resolveAuthStorePath()), provider, profileIds)) { return true; } if ( - rawStoreHasProviderProfile(readJsonFile(resolveLegacyAuthStorePath()), provider, profileIds) + rawStoreHasProviderProfile(tryReadJsonSync(resolveLegacyAuthStorePath()), provider, profileIds) ) { return true; } diff --git a/src/agents/sessions/tools/path-utils.ts b/src/agents/sessions/tools/path-utils.ts index ac654c5eea6b..9da121111100 100644 --- a/src/agents/sessions/tools/path-utils.ts +++ b/src/agents/sessions/tools/path-utils.ts @@ -3,10 +3,10 @@ * * Expands user/file URL inputs and resolves read/write paths against the active cwd with macOS filename variants. */ -import { accessSync, constants } from "node:fs"; import * as os from "node:os"; import { isAbsolute, resolve as resolvePath } from "node:path"; import { fileURLToPath } from "node:url"; +import { pathExistsSync } from "../../../infra/fs-safe.js"; const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g; const NARROW_NO_BREAK_SPACE = "\u202F"; @@ -29,15 +29,6 @@ function tryCurlyQuoteVariant(filePath: string): string { return filePath.replace(/'/g, "\u2019"); } -function fileExists(filePath: string): boolean { - try { - accessSync(filePath, constants.F_OK); - return true; - } catch { - return false; - } -} - function normalizeAtPrefix(filePath: string): string { return filePath.startsWith("@") ? filePath.slice(1) : filePath; } @@ -75,31 +66,31 @@ export function resolveToCwd(filePath: string, cwd: string): string { export function resolveReadPath(filePath: string, cwd: string): string { const resolved = resolveToCwd(filePath, cwd); - if (fileExists(resolved)) { + if (pathExistsSync(resolved)) { return resolved; } // Try macOS AM/PM variant (narrow no-break space before AM/PM) const amPmVariant = tryMacOSScreenshotPath(resolved); - if (amPmVariant !== resolved && fileExists(amPmVariant)) { + if (amPmVariant !== resolved && pathExistsSync(amPmVariant)) { return amPmVariant; } // Try NFD variant (macOS stores filenames in NFD form) const nfdVariant = tryNFDVariant(resolved); - if (nfdVariant !== resolved && fileExists(nfdVariant)) { + if (nfdVariant !== resolved && pathExistsSync(nfdVariant)) { return nfdVariant; } // Try curly quote variant (macOS uses U+2019 in screenshot names) const curlyVariant = tryCurlyQuoteVariant(resolved); - if (curlyVariant !== resolved && fileExists(curlyVariant)) { + if (curlyVariant !== resolved && pathExistsSync(curlyVariant)) { return curlyVariant; } // Try combined NFD + curly quote (for French macOS screenshots like "Capture d'écran") const nfdCurlyVariant = tryCurlyQuoteVariant(nfdVariant); - if (nfdCurlyVariant !== resolved && fileExists(nfdCurlyVariant)) { + if (nfdCurlyVariant !== resolved && pathExistsSync(nfdCurlyVariant)) { return nfdCurlyVariant; } diff --git a/src/agents/shell-snapshot.ts b/src/agents/shell-snapshot.ts index 1d46dce73d89..63a39412869d 100644 --- a/src/agents/shell-snapshot.ts +++ b/src/agents/shell-snapshot.ts @@ -10,6 +10,8 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { resolveStateDir } from "../config/paths.js"; +import { withTempWorkspace } from "../infra/private-temp-workspace.js"; +import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { killProcessTree } from "../process/kill-tree.js"; const SNAPSHOT_VERSION = 1; @@ -260,39 +262,41 @@ async function validateSnapshot( async function captureShellSnapshot(opts: ShellSnapshotWrapOptions): Promise { const shellName = path.basename(opts.shell); - const captureOutputDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-shell-snapshot-")); - await fs.chmod(captureOutputDir, 0o700); - const captureOutputPath = path.join(captureOutputDir, "snapshot.out"); - const captureOutputFile = await fs.open(captureOutputPath, "wx", 0o600); - await captureOutputFile.close(); - const captureCommand = [ - "{", - buildStartupSourceScript(shellName), - `printf '\\n%s\\n' ${shQuote(CAPTURE_MARKER)}`, - buildAliasCaptureScript(shellName), - "(typeset -f 2>/dev/null || declare -f 2>/dev/null || true)", - `printf '\\n%s\\n' ${shQuote(ENV_MARKER)}`, - `${shQuote(process.execPath)} -e ${shQuote(ENV_CAPTURE_NODE_SCRIPT)}`, - `} > ${shQuote(captureOutputPath)}`, - ].join("\n"); + return await withTempWorkspace( + { + rootDir: resolvePreferredOpenClawTmpDir(), + prefix: "openclaw-shell-snapshot-", + dirMode: 0o700, + mode: 0o600, + }, + async (workspace) => { + const captureOutputPath = await workspace.writeText("snapshot.out", ""); + const captureCommand = [ + "{", + buildStartupSourceScript(shellName), + `printf '\\n%s\\n' ${shQuote(CAPTURE_MARKER)}`, + buildAliasCaptureScript(shellName), + "(typeset -f 2>/dev/null || declare -f 2>/dev/null || true)", + `printf '\\n%s\\n' ${shQuote(ENV_MARKER)}`, + `${shQuote(process.execPath)} -e ${shQuote(ENV_CAPTURE_NODE_SCRIPT)}`, + `} > ${shQuote(captureOutputPath)}`, + ].join("\n"); - try { - const result = await runShell({ - shell: opts.shell, - shellArgs: buildCaptureShellArgs(shellName, opts.shellArgs), - cwd: opts.cwd, - env: buildTrustedSnapshotCaptureEnv(opts.env), - command: captureCommand, - timeoutMs: 5_000, - }); - if (result.status !== 0) { - return null; - } - const stdout = await fs.readFile(captureOutputPath, "utf8"); - return buildSnapshotFile(stdout); - } finally { - await fs.rm(captureOutputDir, { force: true, recursive: true }); - } + const result = await runShell({ + shell: opts.shell, + shellArgs: buildCaptureShellArgs(shellName, opts.shellArgs), + cwd: opts.cwd, + env: buildTrustedSnapshotCaptureEnv(opts.env), + command: captureCommand, + timeoutMs: 5_000, + }); + if (result.status !== 0) { + return null; + } + const stdout = await fs.readFile(captureOutputPath, "utf8"); + return buildSnapshotFile(stdout); + }, + ); } function buildCaptureShellArgs(shellName: string, shellArgs: string[]): string[] { diff --git a/src/commands/doctor-auth-flat-profiles.ts b/src/commands/doctor-auth-flat-profiles.ts index e774aa062df0..6be4facbf736 100644 --- a/src/commands/doctor-auth-flat-profiles.ts +++ b/src/commands/doctor-auth-flat-profiles.ts @@ -35,6 +35,7 @@ import type { AuthProfileConfig } from "../config/types.auth.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { coerceSecretRef } from "../config/types.secrets.js"; import { loadJsonFile } from "../infra/json-file.js"; +import { writeJsonSync } from "../infra/json-files.js"; import { shortenHomePath } from "../utils.js"; import type { DoctorPrompter } from "./doctor-prompter.js"; @@ -739,10 +740,6 @@ function backupAndRemoveAuthProfileJson( return backupPath; } -function writeJsonFile(pathname: string, value: unknown): void { - fs.writeFileSync(pathname, `${JSON.stringify(value, null, 2)}\n`, "utf8"); -} - /** * Imports legacy auth profile JSON and state files into the per-agent SQLite store. * @@ -974,7 +971,7 @@ export async function maybeMigrateAuthProfileJsonStoresToSqlite(params: { if (fs.existsSync(candidate.authPath)) { if (unresolvedSidecarRawStore) { backups.push(backupAuthProfileJson(candidate.authPath, "sqlite-import", now)); - writeJsonFile(candidate.authPath, unresolvedSidecarRawStore); + writeJsonSync(candidate.authPath, unresolvedSidecarRawStore); } else { backups.push(backupAndRemoveAuthProfileJson(candidate.authPath, "sqlite-import", now)); } diff --git a/src/gateway/worker-environments/workspace-sync-local.ts b/src/gateway/worker-environments/workspace-sync-local.ts index 75395cb0dae6..65e79ff569c0 100644 --- a/src/gateway/worker-environments/workspace-sync-local.ts +++ b/src/gateway/worker-environments/workspace-sync-local.ts @@ -3,6 +3,7 @@ import { createReadStream } from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; import { sliceUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; +import { hasNodeErrorCode } from "../../infra/path-guards.js"; import { killProcessTree } from "../../process/kill-tree.js"; import { workerSshCommandOptions } from "./ssh.js"; import { isDerivedWorkspacePath } from "./workspace-path-exclusions.js"; @@ -151,16 +152,6 @@ export async function runLocalCommandToFile(params: { } } -function hasErrorCode(error: unknown, code: string): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - typeof error.code === "string" && - error.code === code - ); -} - export async function writeEligibleGitFiles(params: { gitRoot: string; eligiblePath: string; @@ -186,7 +177,7 @@ export async function writeEligibleGitFiles(params: { } const absolute = path.join(canonicalRoot, file); const stats = await fs.lstat(absolute).catch((error: unknown) => { - if (hasErrorCode(error, "ENOENT")) { + if (hasNodeErrorCode(error, "ENOENT")) { return undefined; } throw error; diff --git a/src/infra/path-guards.ts b/src/infra/path-guards.ts index e96245af4e3c..58d8ff6bd4be 100644 --- a/src/infra/path-guards.ts +++ b/src/infra/path-guards.ts @@ -4,6 +4,7 @@ import "./fs-safe-defaults.js"; // Generic path guard facade for containment checks and safe relative paths. export { + hasNodeErrorCode, isNotFoundPathError, isPathInside, normalizeWindowsPathForComparison, diff --git a/src/infra/tmp-openclaw-dir.ts b/src/infra/tmp-openclaw-dir.ts index 225b099bfac7..fa8c718553f7 100644 --- a/src/infra/tmp-openclaw-dir.ts +++ b/src/infra/tmp-openclaw-dir.ts @@ -1,13 +1,8 @@ // Creates temporary OpenClaw directories for runtime scratch work. -import fs from "node:fs"; -import { tmpdir as getOsTmpDir } from "node:os"; -import path from "node:path"; /** Preferred shared OpenClaw temp root on POSIX systems when ownership and permissions are safe. */ export const DEFAULT_POSIX_TMP_ROOT = "/tmp/openclaw"; -type MaybeNodeError = { code?: string }; - type SecureDirStat = { isDirectory(): boolean; isSymbolicLink(): boolean; @@ -27,155 +22,46 @@ export type ResolvePreferredOpenClawTmpDirOptions = { warn?: (message: string) => void; }; -function isNodeErrorWithCode(err: unknown, code: string): err is MaybeNodeError { - return ( - typeof err === "object" && - err !== null && - "code" in err && - (err as MaybeNodeError).code === code - ); +type ResolveSecureTempRoot = typeof import("@openclaw/fs-safe/temp").resolveSecureTempRoot; + +let resolveSecureTempRootRuntime: ResolveSecureTempRoot | undefined; + +function loadResolveSecureTempRoot(): ResolveSecureTempRoot { + if (resolveSecureTempRootRuntime) { + return resolveSecureTempRootRuntime; + } + // Keep this module browser-import safe: fs-safe's temp barrel owns Node-only + // workspaces, so load it only when the Node runtime actually resolves a temp root. + const getBuiltinModule = ( + process as NodeJS.Process & { + getBuiltinModule?: (id: string) => unknown; + } + ).getBuiltinModule; + if (typeof getBuiltinModule !== "function") { + throw new Error("Node module loading is unavailable for secure temp-root resolution"); + } + const moduleNamespace = getBuiltinModule("module") as { + createRequire?: (id: string) => NodeJS.Require; + }; + if (typeof moduleNamespace.createRequire !== "function") { + throw new Error("Node createRequire is unavailable for secure temp-root resolution"); + } + const require = moduleNamespace.createRequire(import.meta.url); + const fsSafeTemp = require("@openclaw/fs-safe/temp") as typeof import("@openclaw/fs-safe/temp"); + resolveSecureTempRootRuntime = fsSafeTemp.resolveSecureTempRoot; + return resolveSecureTempRootRuntime; } /** Resolves a safe OpenClaw temp root, falling back to user-scoped os.tmpdir paths when needed. */ export function resolvePreferredOpenClawTmpDir( options: ResolvePreferredOpenClawTmpDirOptions = {}, ): string { - const accessMode = fs.constants.W_OK | fs.constants.X_OK; - const accessSync = options.accessSync ?? fs.accessSync; - const chmodSync = options.chmodSync ?? fs.chmodSync; - const lstatSync = options.lstatSync ?? fs.lstatSync; - const mkdirSync = options.mkdirSync ?? fs.mkdirSync; - const warn = options.warn ?? ((message: string) => console.warn(message)); - const getuid = - options.getuid ?? - (() => { - try { - return typeof process.getuid === "function" ? process.getuid() : undefined; - } catch { - return undefined; - } - }); - const tmpdir = typeof options.tmpdir === "function" ? options.tmpdir : getOsTmpDir; - const platform = options.platform ?? process.platform; - const uid = getuid(); - - const isSecureDirForUser = (st: { mode?: number; uid?: number }): boolean => { - if (uid === undefined) { - return true; - } - if (typeof st.uid === "number" && st.uid !== uid) { - return false; - } - return typeof st.mode !== "number" || (st.mode & 0o022) === 0; - }; - - const fallback = (): string => { - const suffix = uid === undefined ? "openclaw" : `openclaw-${uid}`; - const joiner = platform === "win32" ? path.win32.join : path.join; - return joiner(tmpdir(), suffix); - }; - - const isTrustedTmpDir = (st: SecureDirStat): boolean => - st.isDirectory() && !st.isSymbolicLink() && isSecureDirForUser(st); - - const resolveDirState = (candidatePath: string): "available" | "missing" | "invalid" => { - try { - const candidate = lstatSync(candidatePath); - if (!isTrustedTmpDir(candidate)) { - return "invalid"; - } - accessSync(candidatePath, accessMode); - return "available"; - } catch (err) { - return isNodeErrorWithCode(err, "ENOENT") ? "missing" : "invalid"; - } - }; - - const tryRepairWritableBits = (candidatePath: string): boolean => { - try { - const st = lstatSync(candidatePath); - if (!st.isDirectory() || st.isSymbolicLink()) { - return false; - } - if (uid !== undefined && typeof st.uid === "number" && st.uid !== uid) { - return false; - } - if (typeof st.mode !== "number") { - return false; - } - if ((st.mode & 0o022) === 0) { - return resolveDirState(candidatePath) === "available"; - } - try { - chmodSync(candidatePath, 0o700); - } catch (chmodErr) { - if ( - isNodeErrorWithCode(chmodErr, "EPERM") || - isNodeErrorWithCode(chmodErr, "EACCES") || - isNodeErrorWithCode(chmodErr, "ENOENT") - ) { - return resolveDirState(candidatePath) === "available"; - } - throw chmodErr; - } - warn(`[openclaw] tightened permissions on temp dir: ${candidatePath}`); - return resolveDirState(candidatePath) === "available"; - } catch { - return false; - } - }; - - const ensureTrustedFallbackDir = (): string => { - const fallbackPath = fallback(); - const state = resolveDirState(fallbackPath); - if (state === "available") { - return fallbackPath; - } - if (state === "invalid") { - if (tryRepairWritableBits(fallbackPath)) { - return fallbackPath; - } - // Never continue with a symlinked, wrong-owner, or world-writable temp root; - // callers create executable/media artifacts under this path. - throw new Error(`Unsafe fallback OpenClaw temp dir: ${fallbackPath}`); - } - try { - mkdirSync(fallbackPath, { recursive: true, mode: 0o700 }); - chmodSync(fallbackPath, 0o700); - } catch { - throw new Error(`Unable to create fallback OpenClaw temp dir: ${fallbackPath}`); - } - if (resolveDirState(fallbackPath) !== "available" && !tryRepairWritableBits(fallbackPath)) { - throw new Error(`Unsafe fallback OpenClaw temp dir: ${fallbackPath}`); - } - return fallbackPath; - }; - - if (platform === "win32") { - return ensureTrustedFallbackDir(); - } - - const preferredDir = DEFAULT_POSIX_TMP_ROOT; - const preferredState = resolveDirState(preferredDir); - if (preferredState === "available") { - return preferredDir; - } - if (preferredState === "invalid") { - if (tryRepairWritableBits(preferredDir)) { - return preferredDir; - } - return ensureTrustedFallbackDir(); - } - - try { - accessSync(path.dirname(preferredDir), accessMode); - mkdirSync(preferredDir, { recursive: true, mode: 0o700 }); - chmodSync(preferredDir, 0o700); - if (resolveDirState(preferredDir) !== "available" && !tryRepairWritableBits(preferredDir)) { - return ensureTrustedFallbackDir(); - } - return preferredDir; - } catch { - return ensureTrustedFallbackDir(); - } + return loadResolveSecureTempRoot()({ + ...options, + preferredDir: DEFAULT_POSIX_TMP_ROOT, + fallbackPrefix: "openclaw", + warningPrefix: "[openclaw]", + unsafeFallbackLabel: "OpenClaw temp dir", + skipPreferredOnWindows: true, + }); } diff --git a/src/transcripts/store-artifacts.ts b/src/transcripts/store-artifacts.ts index 7d8cda7b9bc3..f3efe6bf2a2e 100644 --- a/src/transcripts/store-artifacts.ts +++ b/src/transcripts/store-artifacts.ts @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"; import fs from "node:fs/promises"; import path from "node:path"; import { sha256Hex } from "../infra/crypto-digest.js"; +import { removePathWithinRoot } from "../infra/fs-safe-remove.js"; import { writeExternalFileWithinRoot } from "../infra/fs-safe.js"; import type { TranscriptSessionDescriptor } from "./provider-types.js"; @@ -82,7 +83,7 @@ export async function writeTranscriptArtifact( } export async function removeTranscriptArtifact(rootDir: string, fileName: string): Promise { - await fs.rm(path.join(rootDir, fileName), { force: true }); + await removePathWithinRoot({ rootDir, relativePath: fileName, force: true }); } export async function isCaseSensitiveDirectory(directory: string): Promise {