From 4d33a1a1b44a4b8ce8ecfc6111ea57d626d6e0b9 Mon Sep 17 00:00:00 2001 From: RealDiligent Date: Thu, 16 Jul 2026 11:03:09 +0800 Subject: [PATCH] fix(selfhost): log blob-store path-traversal gets distinctly from misses get() still returns null for escaped keys (safe miss for /loopover/shot), but emits a structured warn instead of swallowing pathFor failures like ordinary ENOENT. Closes #6283 Co-authored-by: Cursor --- src/selfhost/blob-store.ts | 20 +++++++++++++++-- test/unit/selfhost-blob-store.test.ts | 32 ++++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/selfhost/blob-store.ts b/src/selfhost/blob-store.ts index 71bc611051..479b1018a3 100644 --- a/src/selfhost/blob-store.ts +++ b/src/selfhost/blob-store.ts @@ -19,10 +19,26 @@ export function createFsBlobStore(baseDir: string): R2Bucket { return full; }; const store = { - /** Stream a stored object's bytes, or null on a miss (ENOENT / unreadable). The serve route reads `.body`. */ + /** Stream a stored object's bytes, or null on a miss (ENOENT / unreadable). The serve route reads `.body`. + * A path-traversal key still returns null (safe miss for `/loopover/shot`), but is logged distinctly from + * an ordinary miss so probes are visible (#6283) — put/delete keep throwing the same check unguarded. */ async get(key: string): Promise { + let path: string; try { - const bytes = await readFile(pathFor(key)); + path = pathFor(key); + } catch (error) { + console.warn( + JSON.stringify({ + level: "warn", + event: "selfhost_blob_key_escapes_base_dir", + key: typeof key === "string" ? key.slice(0, 200) : String(key).slice(0, 200), + message: error instanceof Error ? error.message.slice(0, 200) : String(error).slice(0, 200), + }), + ); + return null; + } + try { + const bytes = await readFile(path); return { body: new Response(bytes).body } as unknown as R2ObjectBody; } catch { return null; diff --git a/test/unit/selfhost-blob-store.test.ts b/test/unit/selfhost-blob-store.test.ts index 289b97cb61..e2833f646d 100644 --- a/test/unit/selfhost-blob-store.test.ts +++ b/test/unit/selfhost-blob-store.test.ts @@ -1,7 +1,7 @@ import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createFsBlobStore } from "../../src/selfhost/blob-store"; describe("createFsBlobStore (#10 — self-host visual screenshot persistence)", () => { @@ -36,8 +36,34 @@ describe("createFsBlobStore (#10 — self-host visual screenshot persistence)", it("rejects a key that escapes the base dir — put throws, get is a safe miss (no traversal)", async () => { const store = createFsBlobStore(dir); - await expect(store.put("../escape.png", new Uint8Array([1]))).rejects.toThrow(/escapes base dir/); - expect(await store.get("../../etc/passwd")).toBeNull(); // the pathFor throw is caught inside get → safe miss + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(store.put("../escape.png", new Uint8Array([1]))).rejects.toThrow(/escapes base dir/); + expect(await store.get("../../etc/passwd")).toBeNull(); // the pathFor throw is caught inside get → safe miss + } finally { + warn.mockRestore(); + } + }); + + it("logs a path-traversal get distinctly from an ordinary miss (#6283)", async () => { + const store = createFsBlobStore(dir); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(await store.get("gittensory/shots/missing.png")).toBeNull(); + expect(warn).not.toHaveBeenCalled(); + + expect(await store.get("../../etc/passwd")).toBeNull(); + expect(warn).toHaveBeenCalledTimes(1); + const payload = JSON.parse(String(warn.mock.calls[0]?.[0])); + expect(payload).toMatchObject({ + level: "warn", + event: "selfhost_blob_key_escapes_base_dir", + key: "../../etc/passwd", + message: expect.stringMatching(/escapes base dir/i), + }); + } finally { + warn.mockRestore(); + } }); it("delete removes a stored object — a subsequent get is a miss", async () => {