diff --git a/SECURITY.md b/SECURITY.md index dcda446ad90..fea3cda8357 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -171,6 +171,10 @@ Security boundary notes: - Sandbox media validation allows absolute temp paths only under the OpenClaw-managed temp root. - Arbitrary host tmp paths are not treated as trusted media roots. - Plugin/extension code should use OpenClaw temp helpers (`resolvePreferredOpenClawTmpDir`, `buildRandomTempFilePath`, `withTempDownloadPath`) rather than raw `os.tmpdir()` defaults when handling media files. +- Enforcement reference points: + - temp root resolver: `src/infra/tmp-openclaw-dir.ts` + - SDK temp helpers: `src/plugin-sdk/temp-path.ts` + - messaging/channel tmp guardrail: `scripts/check-no-random-messaging-tmp.mjs` ## Operational Guidance diff --git a/scripts/check-no-random-messaging-tmp.mjs b/scripts/check-no-random-messaging-tmp.mjs index c2d6395f4dd..af7b56a371f 100644 --- a/scripts/check-no-random-messaging-tmp.mjs +++ b/scripts/check-no-random-messaging-tmp.mjs @@ -47,7 +47,8 @@ async function collectTypeScriptFiles(dir) { return out; } -function collectNodeOsImports(sourceFile) { +function collectOsTmpdirImports(sourceFile) { + const osModuleSpecifiers = new Set(["node:os", "os"]); const osNamespaceOrDefault = new Set(); const namedTmpdir = new Set(); for (const statement of sourceFile.statements) { @@ -57,7 +58,7 @@ function collectNodeOsImports(sourceFile) { if (!statement.importClause || !ts.isStringLiteral(statement.moduleSpecifier)) { continue; } - if (statement.moduleSpecifier.text !== "node:os") { + if (!osModuleSpecifiers.has(statement.moduleSpecifier.text)) { continue; } const clause = statement.importClause; @@ -101,7 +102,7 @@ function unwrapExpression(expression) { export function findMessagingTmpdirCallLines(content, fileName = "source.ts") { const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); - const { osNamespaceOrDefault, namedTmpdir } = collectNodeOsImports(sourceFile); + const { osNamespaceOrDefault, namedTmpdir } = collectOsTmpdirImports(sourceFile); const lines = []; const visit = (node) => { diff --git a/src/infra/tmp-openclaw-dir.ts b/src/infra/tmp-openclaw-dir.ts index d2377f57961..1e8250b3210 100644 --- a/src/infra/tmp-openclaw-dir.ts +++ b/src/infra/tmp-openclaw-dir.ts @@ -66,35 +66,48 @@ export function resolvePreferredOpenClawTmpDir( return path.join(base, suffix); }; - try { - const preferred = lstatSync(POSIX_OPENCLAW_TMP_DIR); - if (!preferred.isDirectory() || preferred.isSymbolicLink()) { - return fallback(); - } - accessSync(POSIX_OPENCLAW_TMP_DIR, fs.constants.W_OK | fs.constants.X_OK); - if (!isSecureDirForUser(preferred)) { - return fallback(); + const isTrustedPreferredDir = (st: { + isDirectory(): boolean; + isSymbolicLink(): boolean; + mode?: number; + uid?: number; + }): boolean => { + return st.isDirectory() && !st.isSymbolicLink() && isSecureDirForUser(st); + }; + + const resolvePreferredState = ( + requireWritableAccess: boolean, + ): "available" | "missing" | "invalid" => { + try { + const preferred = lstatSync(POSIX_OPENCLAW_TMP_DIR); + if (!isTrustedPreferredDir(preferred)) { + return "invalid"; + } + if (requireWritableAccess) { + accessSync(POSIX_OPENCLAW_TMP_DIR, fs.constants.W_OK | fs.constants.X_OK); + } + return "available"; + } catch (err) { + if (isNodeErrorWithCode(err, "ENOENT")) { + return "missing"; + } + return "invalid"; } + }; + + const existingPreferredState = resolvePreferredState(true); + if (existingPreferredState === "available") { return POSIX_OPENCLAW_TMP_DIR; - } catch (err) { - if (!isNodeErrorWithCode(err, "ENOENT")) { - return fallback(); - } + } + if (existingPreferredState === "invalid") { + return fallback(); } try { accessSync("/tmp", fs.constants.W_OK | fs.constants.X_OK); // Create with a safe default; subsequent callers expect it exists. mkdirSync(POSIX_OPENCLAW_TMP_DIR, { recursive: true, mode: 0o700 }); - try { - const preferred = lstatSync(POSIX_OPENCLAW_TMP_DIR); - if (!preferred.isDirectory() || preferred.isSymbolicLink()) { - return fallback(); - } - if (!isSecureDirForUser(preferred)) { - return fallback(); - } - } catch { + if (resolvePreferredState(true) !== "available") { return fallback(); } return POSIX_OPENCLAW_TMP_DIR; diff --git a/src/media/local-roots.ts b/src/media/local-roots.ts index 8f203d15f7b..51476200ca1 100644 --- a/src/media/local-roots.ts +++ b/src/media/local-roots.ts @@ -4,9 +4,25 @@ import type { OpenClawConfig } from "../config/config.js"; import { resolveStateDir } from "../config/paths.js"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; -function buildMediaLocalRoots(stateDir: string): string[] { +type BuildMediaLocalRootsOptions = { + preferredTmpDir?: string; +}; + +let cachedPreferredTmpDir: string | undefined; + +function resolveCachedPreferredTmpDir(): string { + if (!cachedPreferredTmpDir) { + cachedPreferredTmpDir = resolvePreferredOpenClawTmpDir(); + } + return cachedPreferredTmpDir; +} + +function buildMediaLocalRoots( + stateDir: string, + options: BuildMediaLocalRootsOptions = {}, +): string[] { const resolvedStateDir = path.resolve(stateDir); - const preferredTmpDir = resolvePreferredOpenClawTmpDir(); + const preferredTmpDir = options.preferredTmpDir ?? resolveCachedPreferredTmpDir(); return [ preferredTmpDir, path.join(resolvedStateDir, "media"), diff --git a/src/plugin-sdk/temp-path.ts b/src/plugin-sdk/temp-path.ts index f0ca73b2109..c418fe9f664 100644 --- a/src/plugin-sdk/temp-path.ts +++ b/src/plugin-sdk/temp-path.ts @@ -31,6 +31,15 @@ function resolveTempRoot(tmpDir?: string): string { return tmpDir ?? resolvePreferredOpenClawTmpDir(); } +function isNodeErrorWithCode(err: unknown, code: string): boolean { + return ( + typeof err === "object" && + err !== null && + "code" in err && + (err as { code?: string }).code === code + ); +} + export function buildRandomTempFilePath(params: { prefix: string; extension?: string; @@ -64,6 +73,12 @@ export async function withTempDownloadPath( try { return await fn(tmpPath); } finally { - await rm(dir, { recursive: true, force: true }).catch(() => {}); + try { + await rm(dir, { recursive: true, force: true }); + } catch (err) { + if (!isNodeErrorWithCode(err, "ENOENT")) { + console.warn(`temp-path cleanup failed for ${dir}: ${String(err)}`); + } + } } } diff --git a/test/scripts/check-no-random-messaging-tmp.test.ts b/test/scripts/check-no-random-messaging-tmp.test.ts index 01495b2b09b..276a19962af 100644 --- a/test/scripts/check-no-random-messaging-tmp.test.ts +++ b/test/scripts/check-no-random-messaging-tmp.test.ts @@ -18,6 +18,14 @@ describe("check-no-random-messaging-tmp", () => { expect(findMessagingTmpdirCallLines(source)).toEqual([3]); }); + it("finds tmpdir calls imported from os", () => { + const source = ` + import os from "os"; + const dir = os.tmpdir(); + `; + expect(findMessagingTmpdirCallLines(source)).toEqual([3]); + }); + it("ignores mentions in comments and strings", () => { const source = ` // os.tmpdir()