mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
fix(media): harden saveMediaSource against symlink TOCTOU
This commit is contained in:
@@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Security/Media: harden local media ingestion against TOCTOU/symlink swap attacks by pinning reads to a single file descriptor with symlink rejection and inode/device verification in `saveMediaSource`. Thanks @dorjoos for reporting.
|
||||
- Agents/Streaming: keep assistant partial streaming active during reasoning streams, handle native `thinking_*` stream events consistently, dedupe mixed reasoning-end signals, and clear stale mutating tool errors after same-target retry success. (#20635) Thanks @obviyus.
|
||||
- Gateway/Auth: default unresolved gateway auth to token mode with startup auto-generation/persistence of `gateway.auth.token`, while allowing explicit `gateway.auth.mode: "none"` for intentional open loopback setups. (#20686) thanks @gumadeiras.
|
||||
- Browser/Relay: require gateway-token auth on both `/extension` and `/cdp`, and align Chrome extension setup to use a single `gateway.auth.token` input for relay authentication. Thanks @tdjackey for reporting.
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import JSZip from "jszip";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import JSZip from "jszip";
|
||||
import sharp from "sharp";
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { isPathWithinBase } from "../../test/helpers/paths.js";
|
||||
@@ -102,6 +102,17 @@ describe("media store", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")("rejects symlink sources", async () => {
|
||||
await withTempStore(async (store, home) => {
|
||||
const target = path.join(home, "sensitive.txt");
|
||||
const source = path.join(home, "source.txt");
|
||||
await fs.writeFile(target, "sensitive");
|
||||
await fs.symlink(target, source);
|
||||
|
||||
await expect(store.saveMediaSource(source)).rejects.toThrow("symlink");
|
||||
});
|
||||
});
|
||||
|
||||
it("cleans old media files in first-level subdirectories", async () => {
|
||||
await withTempStore(async (store) => {
|
||||
const saved = await store.saveMediaBuffer(Buffer.from("nested"), "text/plain", "inbound");
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import crypto from "node:crypto";
|
||||
import { constants as fsConstants } from "node:fs";
|
||||
import { createWriteStream } from "node:fs";
|
||||
import fs from "node:fs/promises";
|
||||
import { request as httpRequest } from "node:http";
|
||||
@@ -20,6 +21,12 @@ const defaultHttpRequestImpl: RequestImpl = httpRequest;
|
||||
const defaultHttpsRequestImpl: RequestImpl = httpsRequest;
|
||||
const defaultResolvePinnedHostnameImpl: ResolvePinnedHostnameImpl = resolvePinnedHostname;
|
||||
|
||||
const isNodeError = (err: unknown): err is NodeJS.ErrnoException =>
|
||||
Boolean(err && typeof err === "object" && "code" in (err as Record<string, unknown>));
|
||||
|
||||
const isSymlinkOpenError = (err: unknown) =>
|
||||
isNodeError(err) && (err.code === "ELOOP" || err.code === "EINVAL" || err.code === "ENOTSUP");
|
||||
|
||||
let httpRequestImpl: RequestImpl = defaultHttpRequestImpl;
|
||||
let httpsRequestImpl: RequestImpl = defaultHttpsRequestImpl;
|
||||
let resolvePinnedHostnameImpl: ResolvePinnedHostnameImpl = defaultResolvePinnedHostnameImpl;
|
||||
@@ -232,20 +239,41 @@ export async function saveMediaSource(
|
||||
return { id, path: finalDest, size, contentType: mime };
|
||||
}
|
||||
// local path
|
||||
const stat = await fs.stat(source);
|
||||
if (!stat.isFile()) {
|
||||
throw new Error("Media path is not a file");
|
||||
const supportsNoFollow = process.platform !== "win32" && "O_NOFOLLOW" in fsConstants;
|
||||
const flags = fsConstants.O_RDONLY | (supportsNoFollow ? fsConstants.O_NOFOLLOW : 0);
|
||||
let handle: Awaited<ReturnType<typeof fs.open>>;
|
||||
try {
|
||||
handle = await fs.open(source, flags);
|
||||
} catch (err) {
|
||||
if (isSymlinkOpenError(err)) {
|
||||
throw new Error("Media path must not be a symlink", { cause: err });
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
if (stat.size > MAX_BYTES) {
|
||||
throw new Error("Media exceeds 5MB limit");
|
||||
try {
|
||||
const [stat, lstat] = await Promise.all([handle.stat(), fs.lstat(source)]);
|
||||
if (lstat.isSymbolicLink()) {
|
||||
throw new Error("Media path must not be a symlink");
|
||||
}
|
||||
if (!stat.isFile()) {
|
||||
throw new Error("Media path is not a file");
|
||||
}
|
||||
if (stat.ino !== lstat.ino || stat.dev !== lstat.dev) {
|
||||
throw new Error("Media path changed during read");
|
||||
}
|
||||
if (stat.size > MAX_BYTES) {
|
||||
throw new Error("Media exceeds 5MB limit");
|
||||
}
|
||||
const buffer = await handle.readFile();
|
||||
const mime = await detectMime({ buffer, filePath: source });
|
||||
const ext = extensionForMime(mime) ?? path.extname(source);
|
||||
const id = ext ? `${baseId}${ext}` : baseId;
|
||||
const dest = path.join(dir, id);
|
||||
await fs.writeFile(dest, buffer, { mode: 0o600 });
|
||||
return { id, path: dest, size: stat.size, contentType: mime };
|
||||
} finally {
|
||||
await handle.close().catch(() => {});
|
||||
}
|
||||
const buffer = await fs.readFile(source);
|
||||
const mime = await detectMime({ buffer, filePath: source });
|
||||
const ext = extensionForMime(mime) ?? path.extname(source);
|
||||
const id = ext ? `${baseId}${ext}` : baseId;
|
||||
const dest = path.join(dir, id);
|
||||
await fs.writeFile(dest, buffer, { mode: 0o600 });
|
||||
return { id, path: dest, size: stat.size, contentType: mime };
|
||||
}
|
||||
|
||||
export async function saveMediaBuffer(
|
||||
|
||||
Reference in New Issue
Block a user