LINE/Security: harden inbound media temp-file naming (#20792)
Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: f6f3eecdb3ce21ccdccd8b2c10a74ebd47ad809e Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky
This commit is contained in:
@@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai
|
||||
### Fixes
|
||||
|
||||
- Security/Feishu: prevent path traversal in Feishu inbound media temp-file writes by replacing key-derived temp filenames with UUID-based names. Thanks @allsmog for reporting.
|
||||
- LINE/Security: harden inbound media temp-file naming by using UUID-based temp paths for downloaded media instead of external message IDs. (#20792) Thanks @mbelinky.
|
||||
- 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.
|
||||
- Security/Lobster (Windows): for the next npm release, remove shell-based fallback when launching Lobster wrappers (`.cmd`/`.bat`) and switch to explicit argv execution with wrapper entrypoint resolution, preventing command injection while preserving Windows wrapper compatibility. Thanks @tdjackey 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.
|
||||
|
||||
69
src/line/download.test.ts
Normal file
69
src/line/download.test.ts
Normal file
@@ -0,0 +1,69 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const getMessageContentMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@line/bot-sdk", () => ({
|
||||
messagingApi: {
|
||||
MessagingApiBlobClient: class {
|
||||
getMessageContent(messageId: string) {
|
||||
return getMessageContentMock(messageId);
|
||||
}
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("../globals.js", () => ({
|
||||
logVerbose: () => {},
|
||||
}));
|
||||
|
||||
import { downloadLineMedia } from "./download.js";
|
||||
|
||||
async function* chunks(parts: Buffer[]): AsyncGenerator<Buffer> {
|
||||
for (const part of parts) {
|
||||
yield part;
|
||||
}
|
||||
}
|
||||
|
||||
describe("downloadLineMedia", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("does not derive temp file path from external messageId", async () => {
|
||||
const messageId = "a/../../../../etc/passwd";
|
||||
const jpeg = Buffer.from([0xff, 0xd8, 0xff, 0x00]);
|
||||
getMessageContentMock.mockResolvedValueOnce(chunks([jpeg]));
|
||||
|
||||
const writeSpy = vi.spyOn(fs.promises, "writeFile").mockResolvedValueOnce(undefined);
|
||||
|
||||
const result = await downloadLineMedia(messageId, "token");
|
||||
const writtenPath = writeSpy.mock.calls[0]?.[0];
|
||||
|
||||
expect(result.size).toBe(jpeg.length);
|
||||
expect(result.contentType).toBe("image/jpeg");
|
||||
expect(typeof writtenPath).toBe("string");
|
||||
if (typeof writtenPath !== "string") {
|
||||
throw new Error("expected string temp file path");
|
||||
}
|
||||
expect(result.path).toBe(writtenPath);
|
||||
expect(writtenPath).toContain("line-media-");
|
||||
expect(writtenPath).toMatch(/\.jpg$/);
|
||||
expect(writtenPath).not.toContain(messageId);
|
||||
expect(writtenPath).not.toContain("..");
|
||||
|
||||
const tmpRoot = path.resolve(os.tmpdir());
|
||||
const rel = path.relative(tmpRoot, path.resolve(writtenPath));
|
||||
expect(rel === ".." || rel.startsWith(`..${path.sep}`)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects oversized media before writing to disk", async () => {
|
||||
getMessageContentMock.mockResolvedValueOnce(chunks([Buffer.alloc(4), Buffer.alloc(4)]));
|
||||
const writeSpy = vi.spyOn(fs.promises, "writeFile").mockResolvedValue(undefined);
|
||||
|
||||
await expect(downloadLineMedia("mid", "token", 7)).rejects.toThrow(/Media exceeds/i);
|
||||
expect(writeSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -1,3 +1,4 @@
|
||||
import crypto from "node:crypto";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
@@ -10,6 +11,10 @@ interface DownloadResult {
|
||||
size: number;
|
||||
}
|
||||
|
||||
function buildLineTempMediaPath(extension: string): string {
|
||||
return path.join(os.tmpdir(), `line-media-${Date.now()}-${crypto.randomUUID()}${extension}`);
|
||||
}
|
||||
|
||||
export async function downloadLineMedia(
|
||||
messageId: string,
|
||||
channelAccessToken: string,
|
||||
@@ -39,10 +44,8 @@ export async function downloadLineMedia(
|
||||
const contentType = detectContentType(buffer);
|
||||
const ext = getExtensionForContentType(contentType);
|
||||
|
||||
// Write to temp file
|
||||
const tempDir = os.tmpdir();
|
||||
const fileName = `line-media-${messageId}-${Date.now()}${ext}`;
|
||||
const filePath = path.join(tempDir, fileName);
|
||||
// Use random temp names; never derive paths from external message identifiers.
|
||||
const filePath = buildLineTempMediaPath(ext);
|
||||
|
||||
await fs.promises.writeFile(filePath, buffer);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user