fix: resolve fs-safe post-land fallout

This commit is contained in:
Peter Steinberger
2026-05-06 02:41:31 +01:00
parent 71cd132f1f
commit 20163313af
14 changed files with 255 additions and 61 deletions

View File

@@ -74,6 +74,7 @@ const rootBundledPluginRuntimeDependencies = [
const config = {
ignoreFiles: [
"scripts/**",
"packages/*/dist/**",
"**/__tests__/**",
"src/test-utils/**",
"**/test-helpers/**",
@@ -134,6 +135,7 @@ const config = {
bundledPluginFile("msteams", "src/polls-store-memory.ts"),
bundledPluginFile("voice-call", "src/providers/index.ts"),
],
ignore: ["packages/*/dist/**"],
workspaces: {
".": {
entry: rootEntries,
@@ -155,6 +157,10 @@ const config = {
entry: ["index.html!", "src/main.ts!", "vite.config.ts!", "vitest*.ts!"],
project: ["src/**/*.{ts,tsx}!"],
},
"packages/sdk": {
entry: ["src/index.ts!"],
project: ["src/**/*.ts!"],
},
"packages/*": {
entry: ["index.js!", "scripts/postinstall.js!"],
project: ["index.js!", "scripts/**/*.js!"],

View File

@@ -36,6 +36,7 @@
import * as crypto from "node:crypto";
import type { FileHandle } from "node:fs/promises";
import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime";
import type { MediaSource, OpenedLocalFile } from "../messaging/media-source.js";
import { openLocalFile } from "../messaging/media-source.js";
import {
@@ -564,30 +565,38 @@ async function putToPresignedUrl(
) as ArrayBuffer;
const startTime = Date.now();
const response = await fetch(presignedUrl, {
method: "PUT",
body: new Blob([ab]),
headers: { "Content-Length": String(data.length) },
const { response, release } = await fetchWithSsrFGuard({
url: presignedUrl,
auditContext: "qqbot-media-part-upload",
init: {
method: "PUT",
body: new Blob([ab]),
headers: { "Content-Length": String(data.length) },
},
signal: controller.signal,
});
const elapsed = Date.now() - startTime;
const requestId = response.headers.get("x-cos-request-id") ?? "-";
const etag = response.headers.get("ETag") ?? "-";
try {
const elapsed = Date.now() - startTime;
const requestId = response.headers.get("x-cos-request-id") ?? "-";
const etag = response.headers.get("ETag") ?? "-";
if (!response.ok) {
const body = await response.text().catch(() => "");
logger?.error?.(
`${prefix} PUT part ${partIndex}/${totalParts}: HTTP ${response.status} ${response.statusText} (${elapsed}ms, requestId=${requestId}) body=${body.slice(0, 160)}`,
);
throw new Error(
`COS PUT failed: ${response.status} ${response.statusText} - ${body.slice(0, 120)}`,
if (!response.ok) {
const body = await response.text().catch(() => "");
logger?.error?.(
`${prefix} PUT part ${partIndex}/${totalParts}: HTTP ${response.status} ${response.statusText} (${elapsed}ms, requestId=${requestId}) body=${body.slice(0, 160)}`,
);
throw new Error(
`COS PUT failed: ${response.status} ${response.statusText} - ${body.slice(0, 120)}`,
);
}
logger?.debug?.(
`${prefix} PUT part ${partIndex}/${totalParts} OK (${elapsed}ms ETag=${etag} requestId=${requestId})`,
);
return;
} finally {
await release();
}
logger?.debug?.(
`${prefix} PUT part ${partIndex}/${totalParts} OK (${elapsed}ms ETag=${etag} requestId=${requestId})`,
);
return;
} catch (err) {
lastError = err instanceof Error ? err : new Error(String(err));
if (lastError.name === "AbortError") {

View File

@@ -331,7 +331,9 @@ export async function startSlackSocketAndWaitForDisconnect(params: {
}
if (err === undefined || err === null || err === "") {
const suffix = disconnect ? ` after ${disconnect.event}` : "";
throw new Error(`Slack Socket Mode start failed${suffix} without error detail`);
throw new Error(`Slack Socket Mode start failed${suffix} without error detail`, {
cause: err,
});
}
throw err;
}

View File

@@ -1695,7 +1695,7 @@
"@mariozechner/pi-tui": "0.73.0",
"@modelcontextprotocol/sdk": "1.29.0",
"@mozilla/readability": "^0.6.0",
"@openclaw/fs-safe": "^0.1.0",
"@openclaw/fs-safe": "^0.1.1",
"@slack/bolt": "^4.7.2",
"@slack/types": "^2.21.0",
"@slack/web-api": "^7.15.2",

View File

@@ -1,4 +1,4 @@
import "../../../../src/infra/fs-safe-defaults.js";
import { configureFsSafePython } from "@openclaw/fs-safe/config";
export { isPathInside } from "@openclaw/fs-safe/path";
export {
readRegularFile,
@@ -7,6 +7,13 @@ export {
} from "@openclaw/fs-safe/advanced";
export { walkDirectory, type WalkDirectoryEntry } from "@openclaw/fs-safe/walk";
const hasPythonModeOverride =
process.env.FS_SAFE_PYTHON_MODE != null || process.env.OPENCLAW_FS_SAFE_PYTHON_MODE != null;
if (!hasPythonModeOverride) {
configureFsSafePython({ mode: "off" });
}
export function isFileMissingError(
err: unknown,
): err is NodeJS.ErrnoException & { code: "ENOENT" } {

10
pnpm-lock.yaml generated
View File

@@ -104,8 +104,8 @@ importers:
specifier: ^0.6.0
version: 0.6.0
'@openclaw/fs-safe':
specifier: ^0.1.0
version: 0.1.0
specifier: ^0.1.1
version: 0.1.1
'@slack/bolt':
specifier: ^4.7.2
version: 4.7.2(@types/express@5.0.6)
@@ -3233,8 +3233,8 @@ packages:
cpu: [x64]
os: [win32]
'@openclaw/fs-safe@0.1.0':
resolution: {integrity: sha512-ByFeXAA+7EXje15eaI3mqee9STFcglTkfdhenZQ2V4OGAkhkHzb2yxe3hInuWXYImG9gDolbccNEnwlsoTayaA==}
'@openclaw/fs-safe@0.1.1':
resolution: {integrity: sha512-+50LBpW7nKWzu3wJb4C5X9BQAcAxmj3oNRd/ZqZK+YO1ZdiikOPZ6fy5ta631xsV13+m+Ap5s0Gb3zPSl+0kOQ==}
engines: {node: '>=20.11'}
'@opentelemetry/api-logs@0.216.0':
@@ -10005,7 +10005,7 @@ snapshots:
'@openai/codex@0.128.0-win32-x64':
optional: true
'@openclaw/fs-safe@0.1.0':
'@openclaw/fs-safe@0.1.1':
optionalDependencies:
jszip: 3.10.1
tar: 7.5.13

View File

@@ -120,7 +120,7 @@ beforeEach(() => {
childProcessMocks.spawn.mockClear();
fsSafeMocks.rootCopyFrom.mockReset().mockImplementation(rootCopyFromForTest);
fsSafeMocks.root.mockReset().mockImplementation(async (rootDir: string) => ({
copyFrom: async (sourcePath: string, relativePath: string, options?: { maxBytes?: number }) =>
copyIn: async (relativePath: string, sourcePath: string, options?: { maxBytes?: number }) =>
await rootCopyFromForTest({
sourcePath,
rootDir,

View File

@@ -19,6 +19,7 @@ const hoisted = await vi.hoisted(async () => {
),
mkdirMock: vi.fn(async (_filePath: string, _options?: { recursive?: boolean }) => undefined),
accessMock: vi.fn(async (_filePath: string) => undefined),
pathExistsMock: vi.fn(async (_filePath: string) => true),
exportHtmlTemplateContents: new Map<string, string>(),
};
});
@@ -37,6 +38,10 @@ vi.mock("./commands-system-prompt.js", () => ({
resolveCommandsSystemPromptBundle: hoisted.resolveCommandsSystemPromptBundleMock,
}));
vi.mock("../../infra/fs-safe.js", () => ({
pathExists: hoisted.pathExistsMock,
}));
vi.mock("node:fs", async () => {
const actual = await vi.importActual<typeof import("node:fs")>("node:fs");
const mockedFs = {
@@ -146,6 +151,7 @@ describe("buildExportSessionReply", () => {
sandboxRuntime: { sandboxed: false, mode: "off" },
});
hoisted.accessMock.mockResolvedValue(undefined);
hoisted.pathExistsMock.mockResolvedValue(true);
hoisted.exportHtmlTemplateContents.clear();
});

View File

@@ -245,7 +245,7 @@ describe("archive utils", () => {
});
it.runIf(process.platform !== "win32")(
"rejects zip extraction when a hardlink appears after atomic rename",
"rejects zip extraction when a hardlink appears during destination verification",
async () => {
await withArchiveCase("zip", async ({ workDir, archivePath, extractDir }) => {
const outsideDir = path.join(workDir, "outside");
@@ -256,15 +256,20 @@ describe("archive utils", () => {
const zip = new JSZip();
zip.file("package/payload.bin", "owned");
await fs.writeFile(archivePath, await zip.generateAsync({ type: "nodebuffer" }));
const extractedRealPath = path.join(
await fs.realpath(extractDir),
"package",
"payload.bin",
);
const realRename = fs.rename.bind(fs);
const realLstat = fs.lstat.bind(fs);
let linked = false;
const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (...args) => {
await realRename(...args);
if (!linked) {
const lstatSpy = vi.spyOn(fs, "lstat").mockImplementation(async (...args) => {
if (!linked && String(args[0]) === extractedRealPath) {
await fs.link(extractedRealPath, outsideAlias);
linked = true;
await fs.link(String(args[1]), outsideAlias);
}
return await realLstat(...args);
});
try {
@@ -278,10 +283,10 @@ describe("archive utils", () => {
code: "destination-symlink-traversal",
} satisfies Partial<ArchiveSecurityError>);
} finally {
renameSpy.mockRestore();
lstatSpy.mockRestore();
}
await expect(fs.readFile(outsideAlias, "utf8")).resolves.toBe("owned");
await expect(fs.readFile(outsideAlias, "utf8")).resolves.toBe("");
await expect(fs.stat(extractedPath)).rejects.toMatchObject({ code: "ENOENT" });
});
},

View File

@@ -1,26 +1,175 @@
import "./fs-safe-defaults.js";
import { resolveSecureTempRoot, type ResolveSecureTempRootOptions } from "@openclaw/fs-safe/temp";
import fs from "node:fs";
import { tmpdir as getOsTmpDir } from "node:os";
import path from "node:path";
export const POSIX_OPENCLAW_TMP_DIR = "/tmp/openclaw";
type ResolvePreferredOpenClawTmpDirOptions = Omit<
ResolveSecureTempRootOptions,
| "fallbackPrefix"
| "preferredDir"
| "skipPreferredOnWindows"
| "unsafeFallbackLabel"
| "warningPrefix"
>;
type MaybeNodeError = { code?: string };
type SecureDirStat = {
isDirectory(): boolean;
isSymbolicLink(): boolean;
mode?: number;
uid?: number;
};
export type ResolvePreferredOpenClawTmpDirOptions = {
accessSync?: (path: string, mode?: number) => void;
chmodSync?: (path: string, mode: number) => void;
getuid?: () => number | undefined;
lstatSync?: (path: string) => SecureDirStat;
mkdirSync?: (path: string, opts: { recursive: boolean; mode?: number }) => void;
platform?: NodeJS.Platform;
tmpdir?: () => string;
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
);
}
export function resolvePreferredOpenClawTmpDir(
options: ResolvePreferredOpenClawTmpDirOptions = {},
): string {
return resolveSecureTempRoot({
...options,
fallbackPrefix: "openclaw",
preferredDir: POSIX_OPENCLAW_TMP_DIR,
skipPreferredOnWindows: true,
unsafeFallbackLabel: "OpenClaw temp dir",
warningPrefix: "[openclaw]",
});
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;
}
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 = POSIX_OPENCLAW_TMP_DIR;
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();
}
}

View File

@@ -20,9 +20,11 @@ async function withBlockedLocalAttachmentFallback(
) {
await withTempDir({ prefix }, async (base) => {
const allowedRoot = path.join(base, "allowed");
const attachmentPath = path.join(allowedRoot, "voice-note.m4a");
const blockedRoot = path.join(base, "blocked");
const attachmentPath = path.join(blockedRoot, "voice-note.m4a");
const fallbackUrl = "https://example.com/fallback.jpg";
await fs.mkdir(allowedRoot, { recursive: true });
await fs.mkdir(blockedRoot, { recursive: true });
await fs.writeFile(attachmentPath, "ok");
const cache = new MediaAttachmentCache(

View File

@@ -23,12 +23,14 @@ describe("image-ops temp dir", () => {
it("creates sips temp dirs under the secured OpenClaw tmp root", async () => {
const secureRoot = resolvePreferredOpenClawTmpDir();
const secureRootReal = await fs.realpath(secureRoot);
await getImageMetadata(Buffer.from("image"));
expect(fs.mkdtemp).toHaveBeenCalledTimes(1);
expect(fs.mkdtemp).toHaveBeenCalledWith(path.join(secureRoot, "openclaw-img-"));
expect(createdTempDir.startsWith(path.join(secureRoot, "openclaw-img-"))).toBe(true);
const mkdtempPrefix = vi.mocked(fs.mkdtemp).mock.calls[0]?.[0];
expect(mkdtempPrefix.startsWith(path.join(secureRootReal, "openclaw-img-"))).toBe(true);
expect(createdTempDir.startsWith(path.join(secureRootReal, "openclaw-img-"))).toBe(true);
await expect(fs.access(createdTempDir)).rejects.toMatchObject({ code: "ENOENT" });
});
});

View File

@@ -46,12 +46,13 @@ describe("media reference helpers", () => {
const filePath = path.join(stateDir, "media", "inbound", id);
await fs.mkdir(path.dirname(filePath), { recursive: true });
await fs.writeFile(filePath, Buffer.from("png"));
const realFilePath = await fs.realpath(filePath);
try {
await expect(resolveInboundMediaReference(`media://inbound/${id}`)).resolves.toMatchObject({
id,
normalizedSource: `media://inbound/${id}`,
physicalPath: filePath,
physicalPath: realFilePath,
sourceType: "uri",
});
} finally {
@@ -65,9 +66,12 @@ describe("media reference helpers", () => {
const filePath = path.join(stateDir, "media", "inbound", id);
await fs.mkdir(path.dirname(filePath), { recursive: true });
await fs.writeFile(filePath, Buffer.from("png"));
const realFilePath = await fs.realpath(filePath);
try {
await expect(resolveMediaReferenceLocalPath(`media://inbound/${id}`)).resolves.toBe(filePath);
await expect(resolveMediaReferenceLocalPath(`media://inbound/${id}`)).resolves.toBe(
realFilePath,
);
await expect(resolveMediaReferenceLocalPath(" MEDIA: ./out.png")).resolves.toBe("./out.png");
} finally {
await fs.rm(filePath, { force: true });
@@ -80,11 +84,12 @@ describe("media reference helpers", () => {
const filePath = path.join(stateDir, "media", "inbound", id);
await fs.mkdir(path.dirname(filePath), { recursive: true });
await fs.writeFile(filePath, Buffer.from("png"));
const realFilePath = await fs.realpath(filePath);
try {
await expect(resolveInboundMediaReference(filePath)).resolves.toMatchObject({
id,
physicalPath: filePath,
physicalPath: realFilePath,
sourceType: "path",
});
await expect(

View File

@@ -106,13 +106,14 @@ describe("production lint suppressions", () => {
"src/hooks/module-loader.ts|typescript/no-unnecessary-type-parameters|1",
"src/infra/channel-runtime-context.ts|typescript/no-unnecessary-type-parameters|1",
"src/infra/exec-approvals-effective.ts|typescript/no-unnecessary-type-parameters|1",
"src/infra/json-file.ts|typescript/no-unnecessary-type-parameters|1",
"src/infra/json-file.ts|typescript-eslint/no-unnecessary-type-parameters|1",
"src/infra/outbound/send-deps.ts|typescript/no-unnecessary-type-parameters|1",
"src/node-host/invoke.ts|typescript/no-unnecessary-type-parameters|1",
"src/plugin-sdk/channel-config-helpers.ts|typescript/no-unnecessary-type-parameters|1",
"src/plugin-sdk/channel-entry-contract.ts|typescript/no-unnecessary-type-parameters|1",
"src/plugin-sdk/facade-loader.ts|typescript/no-unnecessary-type-parameters|1",
"src/plugin-sdk/facade-runtime.ts|typescript/no-unnecessary-type-parameters|3",
"src/plugin-sdk/json-store.ts|typescript-eslint/no-unnecessary-type-parameters|1",
"src/plugin-sdk/qa-runner-runtime.ts|typescript/no-unnecessary-type-parameters|1",
"src/plugin-sdk/test-helpers/package-manifest-contract.ts|typescript/no-unnecessary-type-parameters|1",
"src/plugin-sdk/test-helpers/public-surface-loader.ts|typescript/no-unnecessary-type-parameters|1",