refactor(tmp): harden temp boundary guardrails

This commit is contained in:
Peter Steinberger
2026-02-24 23:51:01 +00:00
parent de586373e0
commit def993dbd8
6 changed files with 84 additions and 27 deletions

View File

@@ -171,6 +171,10 @@ Security boundary notes:
- Sandbox media validation allows absolute temp paths only under the OpenClaw-managed temp root. - 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. - 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. - 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 ## Operational Guidance

View File

@@ -47,7 +47,8 @@ async function collectTypeScriptFiles(dir) {
return out; return out;
} }
function collectNodeOsImports(sourceFile) { function collectOsTmpdirImports(sourceFile) {
const osModuleSpecifiers = new Set(["node:os", "os"]);
const osNamespaceOrDefault = new Set(); const osNamespaceOrDefault = new Set();
const namedTmpdir = new Set(); const namedTmpdir = new Set();
for (const statement of sourceFile.statements) { for (const statement of sourceFile.statements) {
@@ -57,7 +58,7 @@ function collectNodeOsImports(sourceFile) {
if (!statement.importClause || !ts.isStringLiteral(statement.moduleSpecifier)) { if (!statement.importClause || !ts.isStringLiteral(statement.moduleSpecifier)) {
continue; continue;
} }
if (statement.moduleSpecifier.text !== "node:os") { if (!osModuleSpecifiers.has(statement.moduleSpecifier.text)) {
continue; continue;
} }
const clause = statement.importClause; const clause = statement.importClause;
@@ -101,7 +102,7 @@ function unwrapExpression(expression) {
export function findMessagingTmpdirCallLines(content, fileName = "source.ts") { export function findMessagingTmpdirCallLines(content, fileName = "source.ts") {
const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true);
const { osNamespaceOrDefault, namedTmpdir } = collectNodeOsImports(sourceFile); const { osNamespaceOrDefault, namedTmpdir } = collectOsTmpdirImports(sourceFile);
const lines = []; const lines = [];
const visit = (node) => { const visit = (node) => {

View File

@@ -66,35 +66,48 @@ export function resolvePreferredOpenClawTmpDir(
return path.join(base, suffix); return path.join(base, suffix);
}; };
try { const isTrustedPreferredDir = (st: {
const preferred = lstatSync(POSIX_OPENCLAW_TMP_DIR); isDirectory(): boolean;
if (!preferred.isDirectory() || preferred.isSymbolicLink()) { isSymbolicLink(): boolean;
return fallback(); mode?: number;
} uid?: number;
accessSync(POSIX_OPENCLAW_TMP_DIR, fs.constants.W_OK | fs.constants.X_OK); }): boolean => {
if (!isSecureDirForUser(preferred)) { return st.isDirectory() && !st.isSymbolicLink() && isSecureDirForUser(st);
return fallback(); };
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; return POSIX_OPENCLAW_TMP_DIR;
} catch (err) { }
if (!isNodeErrorWithCode(err, "ENOENT")) { if (existingPreferredState === "invalid") {
return fallback(); return fallback();
}
} }
try { try {
accessSync("/tmp", fs.constants.W_OK | fs.constants.X_OK); accessSync("/tmp", fs.constants.W_OK | fs.constants.X_OK);
// Create with a safe default; subsequent callers expect it exists. // Create with a safe default; subsequent callers expect it exists.
mkdirSync(POSIX_OPENCLAW_TMP_DIR, { recursive: true, mode: 0o700 }); mkdirSync(POSIX_OPENCLAW_TMP_DIR, { recursive: true, mode: 0o700 });
try { if (resolvePreferredState(true) !== "available") {
const preferred = lstatSync(POSIX_OPENCLAW_TMP_DIR);
if (!preferred.isDirectory() || preferred.isSymbolicLink()) {
return fallback();
}
if (!isSecureDirForUser(preferred)) {
return fallback();
}
} catch {
return fallback(); return fallback();
} }
return POSIX_OPENCLAW_TMP_DIR; return POSIX_OPENCLAW_TMP_DIR;

View File

@@ -4,9 +4,25 @@ import type { OpenClawConfig } from "../config/config.js";
import { resolveStateDir } from "../config/paths.js"; import { resolveStateDir } from "../config/paths.js";
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.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 resolvedStateDir = path.resolve(stateDir);
const preferredTmpDir = resolvePreferredOpenClawTmpDir(); const preferredTmpDir = options.preferredTmpDir ?? resolveCachedPreferredTmpDir();
return [ return [
preferredTmpDir, preferredTmpDir,
path.join(resolvedStateDir, "media"), path.join(resolvedStateDir, "media"),

View File

@@ -31,6 +31,15 @@ function resolveTempRoot(tmpDir?: string): string {
return tmpDir ?? resolvePreferredOpenClawTmpDir(); 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: { export function buildRandomTempFilePath(params: {
prefix: string; prefix: string;
extension?: string; extension?: string;
@@ -64,6 +73,12 @@ export async function withTempDownloadPath<T>(
try { try {
return await fn(tmpPath); return await fn(tmpPath);
} finally { } 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)}`);
}
}
} }
} }

View File

@@ -18,6 +18,14 @@ describe("check-no-random-messaging-tmp", () => {
expect(findMessagingTmpdirCallLines(source)).toEqual([3]); 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", () => { it("ignores mentions in comments and strings", () => {
const source = ` const source = `
// os.tmpdir() // os.tmpdir()