mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:40:44 +00:00
fix(media): handle canonical inbound media paths
This commit is contained in:
@@ -127,6 +127,13 @@ beforeEach(() => {
|
||||
relativePath,
|
||||
maxBytes: options?.maxBytes,
|
||||
}),
|
||||
copyIn: async (relativePath: string, sourcePath: string, options?: { maxBytes?: number }) =>
|
||||
await rootCopyFromForTest({
|
||||
sourcePath,
|
||||
rootDir,
|
||||
relativePath,
|
||||
maxBytes: options?.maxBytes,
|
||||
}),
|
||||
}));
|
||||
mediaRootMocks.resolveChannelRemoteInboundAttachmentRoots
|
||||
.mockReset()
|
||||
|
||||
@@ -12,6 +12,7 @@ import { normalizeScpRemoteHost, normalizeScpRemotePath } from "../../infra/scp-
|
||||
import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js";
|
||||
import { resolveChannelRemoteInboundAttachmentRoots } from "../../media/channel-inbound-roots.js";
|
||||
import { isInboundPathAllowed } from "../../media/inbound-path-policy.js";
|
||||
import { resolveInboundMediaReference } from "../../media/media-reference.js";
|
||||
import { getMediaDir, MEDIA_MAX_BYTES } from "../../media/store.js";
|
||||
import { normalizeOptionalString } from "../../shared/string-coerce.js";
|
||||
import { CONFIG_DIR } from "../../utils.js";
|
||||
@@ -99,8 +100,9 @@ export async function stageSandboxMedia(params: {
|
||||
maxBytes: STAGED_MEDIA_MAX_BYTES,
|
||||
});
|
||||
} else {
|
||||
const copySource = await fs.realpath(source).catch(() => source);
|
||||
await stageLocalFileIntoRoot({
|
||||
sourcePath: source,
|
||||
sourcePath: copySource,
|
||||
rootDir: effectiveWorkspaceDir,
|
||||
relativeDestPath: relativeDest,
|
||||
maxBytes: STAGED_MEDIA_MAX_BYTES,
|
||||
@@ -213,21 +215,27 @@ async function isAllowedSourcePath(params: {
|
||||
}
|
||||
return true;
|
||||
}
|
||||
const inboundReference = await resolveInboundMediaReference(params.source).catch(() => null);
|
||||
if (inboundReference) {
|
||||
return true;
|
||||
}
|
||||
const mediaDir = getMediaDir();
|
||||
const canonicalMediaDir = await fs.realpath(mediaDir).catch(() => mediaDir);
|
||||
if (
|
||||
!isInboundPathAllowed({
|
||||
filePath: params.source,
|
||||
roots: [mediaDir],
|
||||
roots: [mediaDir, canonicalMediaDir],
|
||||
})
|
||||
) {
|
||||
logVerbose(`Blocking attempt to stage media from outside media directory: ${params.source}`);
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
const canonicalSource = await fs.realpath(params.source).catch(() => params.source);
|
||||
await assertSandboxPath({
|
||||
filePath: params.source,
|
||||
cwd: mediaDir,
|
||||
root: mediaDir,
|
||||
filePath: canonicalSource,
|
||||
cwd: canonicalMediaDir,
|
||||
root: canonicalMediaDir,
|
||||
});
|
||||
return true;
|
||||
} catch {
|
||||
|
||||
@@ -19,12 +19,12 @@ async function withBlockedLocalAttachmentFallback(
|
||||
run: (params: { cache: MediaAttachmentCache; fallbackUrl: string }) => Promise<void>,
|
||||
) {
|
||||
await withTempDir({ prefix }, async (base) => {
|
||||
const attachmentRoot = path.join(base, "attachment");
|
||||
const allowedRoot = path.join(base, "allowed");
|
||||
const blockedRoot = path.join(base, "blocked");
|
||||
const attachmentPath = path.join(blockedRoot, "voice-note.m4a");
|
||||
const attachmentPath = path.join(attachmentRoot, "voice-note.m4a");
|
||||
const fallbackUrl = "https://example.com/fallback.jpg";
|
||||
await fs.mkdir(attachmentRoot, { recursive: true });
|
||||
await fs.mkdir(allowedRoot, { recursive: true });
|
||||
await fs.mkdir(blockedRoot, { recursive: true });
|
||||
await fs.writeFile(attachmentPath, "ok");
|
||||
|
||||
const cache = new MediaAttachmentCache(
|
||||
@@ -33,20 +33,12 @@ async function withBlockedLocalAttachmentFallback(
|
||||
localPathRoots: [allowedRoot],
|
||||
},
|
||||
);
|
||||
const originalRealpath = fs.realpath.bind(fs);
|
||||
fetchRemoteMediaMock.mockResolvedValue({
|
||||
buffer: Buffer.from("fallback-buffer"),
|
||||
contentType: "image/jpeg",
|
||||
fileName: "fallback.jpg",
|
||||
});
|
||||
|
||||
vi.spyOn(fs, "realpath").mockImplementation(async (candidatePath) => {
|
||||
if (String(candidatePath) === attachmentPath) {
|
||||
throw new Error("EACCES");
|
||||
}
|
||||
return await originalRealpath(candidatePath);
|
||||
});
|
||||
|
||||
await run({ cache, fallbackUrl });
|
||||
});
|
||||
}
|
||||
|
||||
@@ -22,15 +22,15 @@ 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);
|
||||
const secureRoot = await fs.realpath(resolvePreferredOpenClawTmpDir());
|
||||
|
||||
await getImageMetadata(Buffer.from("image"));
|
||||
|
||||
expect(fs.mkdtemp).toHaveBeenCalledTimes(1);
|
||||
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);
|
||||
const [prefix] = vi.mocked(fs.mkdtemp).mock.calls[0] ?? [];
|
||||
expect(prefix).toEqual(expect.stringMatching(/^.+openclaw-img-[0-9a-f-]+-$/u));
|
||||
expect(path.dirname(prefix ?? "")).toBe(secureRoot);
|
||||
expect(createdTempDir.startsWith(prefix ?? "")).toBe(true);
|
||||
await expect(fs.access(createdTempDir)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { safeFileURLToPath } from "../infra/local-file-access.js";
|
||||
import { resolveUserPath } from "../utils.js";
|
||||
@@ -86,6 +87,14 @@ function maybeLocalPathFromSource(source: string): string | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
async function resolvePathForContainment(candidate: string): Promise<string> {
|
||||
try {
|
||||
return await fs.realpath(candidate);
|
||||
} catch {
|
||||
return path.resolve(candidate);
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveInboundMediaUri(
|
||||
normalizedSource: string,
|
||||
): Promise<InboundMediaReference | null> {
|
||||
@@ -148,9 +157,16 @@ export async function resolveInboundMediaReference(
|
||||
return null;
|
||||
}
|
||||
|
||||
const inboundDir = path.resolve(getMediaDir(), "inbound");
|
||||
const resolvedPath = path.resolve(localPath);
|
||||
const rel = path.relative(inboundDir, resolvedPath);
|
||||
const rawInboundDir = path.resolve(getMediaDir(), "inbound");
|
||||
const rawResolvedPath = path.resolve(localPath);
|
||||
const rawRel = path.relative(rawInboundDir, rawResolvedPath);
|
||||
const rel =
|
||||
rawRel && !rawRel.startsWith("..") && !path.isAbsolute(rawRel)
|
||||
? rawRel
|
||||
: path.relative(
|
||||
await resolvePathForContainment(rawInboundDir),
|
||||
await resolvePathForContainment(localPath),
|
||||
);
|
||||
if (!rel || rel.startsWith("..") || path.isAbsolute(rel) || rel.includes(path.sep)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user