diff --git a/CHANGELOG.md b/CHANGELOG.md index a92ea984..73b2a05b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ All notable changes to Quoll are documented here. +## 0.1.60 — 2026-07-29 + +### Fixed + +- Pasting images into a single document is now capped at a generous total size per editing session, guarding against runaway disk use from a misbehaving source. The limit is far above any normal paste workflow; if it is ever reached, further image pastes are declined with a one-time notice and resume when you reopen the document. + ## 0.1.59 — 2026-07-29 ### Fixed diff --git a/package.json b/package.json index de78c2eb..74d3fc8e 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "gfm", "MEO" ], - "version": "0.1.59", + "version": "0.1.60", "publisher": "mtskf", "type": "module", "engines": { diff --git a/src/extension/image/image-write-budget.ts b/src/extension/image/image-write-budget.ts new file mode 100644 index 00000000..da80c616 --- /dev/null +++ b/src/extension/image/image-write-budget.ts @@ -0,0 +1,84 @@ +// Per-session cumulative volume cap for the image-WRITE path. +// +// The per-message caps in image-write-service (the MAX_IMAGE_DATA_LENGTH string +// bound + the 10 MB decode limit) bound a SINGLE write; they do nothing against +// a compromised webview — the exact adversary image-write-wiring gates against — +// that loops `image-write` with distinct PNG-magic-prefixed random payloads. +// Each such payload content-addresses to a NEW /assets/ file, so the +// per-message caps are individually satisfied while the disk fills silently. +// +// Other webview→host channels are already count-bounded PER MESSAGE (e.g. +// MAX_LINT_DIAGNOSTICS caps diagnostics on each inbound lint message), but none +// keeps a running session-lifetime total — this budget is the FIRST such bound +// in the codebase, added because the image-write path is uniquely able to turn a +// message loop into unbounded disk growth. It is a generous cumulative byte +// ceiling scoped to the editor session (one budget per panel). Once crossed it +// rejects further writes and warns the user ONCE (a per-write toast would itself +// be a notification-flood vector). Real paste flows — even bulk multi-image +// paste — never approach it. + +/** Cumulative image bytes a single editor session may write to ./assets/. + * 512 MiB ≈ 50 full-size (10 MB) images — orders of magnitude beyond any real + * paste session, yet a hard stop against an unbounded write loop. A session is + * one open panel; reopening the document starts a fresh budget. */ +export const SESSION_IMAGE_WRITE_BUDGET_BYTES = 512 * 1024 * 1024; + +/** User-facing warning shown ONCE when the session budget is first exceeded. */ +export const SESSION_IMAGE_WRITE_BUDGET_TOAST = + "Quoll: image paste limit reached for this document — reopen it to insert more images."; + +export interface SessionVolumeBudget { + /** Charge `byteLength` against the session budget. Returns true (and records + * the spend) when the write may proceed; false once the cumulative cap would + * be exceeded — in which case `onExceeded` fires exactly once across the + * session's lifetime, no matter how many further writes are attempted. + * Reserve BEFORE the async write so concurrent (fire-and-forget) writes can't + * each independently pass the check and overshoot the cap. + * + * There is deliberately NO release/refund counterpart: a charge is never + * reversed, even when the subsequent write fails. This is the fail-SAFE + * choice for a disk-fill cap — `workspace.fs.writeFile` gives no atomicity + * guarantee, so a failed write can still leave a partial content-addressed + * file on disk; refunding it would let an attacker loop failing writes and + * grow the disk without bound while every reservation is returned. Counting + * the attempt keeps total disk growth bounded by the budget. The only cost is + * that a session hitting many genuine FS failures may reach the cap early + * (reopen the document to reset) — negligible for real use against a 512 MiB + * ceiling. */ + reserve(byteLength: number): boolean; +} + +// Guard the numeric domain the running total assumes (non-negative integer). +// The sole production caller passes a `Uint8Array.length`, always safe — but a +// NaN would poison `spent` permanently (every `NaN > cap` is false, silently +// disabling the cap: the exact failure this module exists to prevent), and a +// negative would rewind it. Fail loud on the contract violation instead. +function assertByteLength(byteLength: number): void { + if (!Number.isInteger(byteLength) || byteLength < 0) { + throw new RangeError( + `SessionVolumeBudget: byteLength must be a non-negative integer, got ${byteLength}` + ); + } +} + +export function createSessionVolumeBudget( + budgetBytes: number, + onExceeded: () => void +): SessionVolumeBudget { + let spent = 0; + let warned = false; + return { + reserve(byteLength) { + assertByteLength(byteLength); + if (spent + byteLength > budgetBytes) { + if (!warned) { + warned = true; + onExceeded(); + } + return false; + } + spent += byteLength; + return true; + }, + }; +} diff --git a/src/extension/image/image-write-service.ts b/src/extension/image/image-write-service.ts index 1f3e702e..82045986 100644 --- a/src/extension/image/image-write-service.ts +++ b/src/extension/image/image-write-service.ts @@ -5,6 +5,7 @@ import { MAX_IMAGE_DATA_LENGTH } from "../../shared/protocol.js"; import { decideImageWrite, type ImageRejectReason } from "./image-ingest.js"; +import type { SessionVolumeBudget } from "./image-write-budget.js"; export function imageRejectToast(reason: ImageRejectReason): string { switch (reason) { @@ -29,6 +30,16 @@ export type ImageWriteDeps = { showError: (message: string) => void; /** Post the image-write-result; `null` path ⇒ ok:false. */ postResult: (requestId: string, relativePath: string | null) => void; + /** Session cumulative-volume budget (the DoS cap this service enforces). + * REQUIRED, not optional: a security-load-bearing dep must never default to + * "unbounded" — callers that genuinely want no cap pass an all-permitting + * budget explicitly, so "unbounded" is a visible decision, not a silent one. + * reserve() is charged AFTER validation (only bytes past every per-message + * gate) and BEFORE the write; the charge is never refunded, so total disk + * growth stays bounded by the budget even when a write fails and leaves a + * partial file (see SessionVolumeBudget.reserve). The budget owns its own + * one-time warning, so a budget rejection posts ok:false WITHOUT a showError. */ + budget: SessionVolumeBudget; }; /** Validate + write a base64 image and post the result. Never throws — every @@ -64,14 +75,38 @@ export async function handleImageWrite( deps.postResult(requestId, null); return; } + // Session cumulative-volume gate: reserve the validated byte count (only bytes + // past every per-message gate above) BEFORE the async write so concurrent + // fire-and-forget writes can't each overshoot the cap. The charge is not + // refunded on failure (see SessionVolumeBudget.reserve — a failed write can + // still leave a partial file, so counting the attempt keeps disk bounded). The + // budget surfaces its own one-time warning, so this rejection just clears the + // webview's pending entry. + // + // The charge brackets the WHOLE write (in the wiring, writeImage does an + // idempotent createDirectory then writeFile), so a pre-write setup failure is + // charged too even though it wrote no bytes. Accepted deliberately: reserving + // here keeps the budget-rejection path cleanly separate from I/O failures, and + // the over-charge is immaterial — createDirectory only fails on a broken FS, + // which blocks all pastes regardless of the cap (a persistent failure is not a + // budget problem), while a transient blip barely dents a 512 MiB ceiling. + if (!deps.budget.reserve(decision.bytes.length)) { + deps.postResult(requestId, null); + return; + } + // Scope the try to the write ALONE: a throw here means the write failed. The + // success-path post is deliberately OUTSIDE it so a postResult that throws is + // not misread as a write failure (wrong toast + double post). + let relativePath: string; try { - const relativePath = await deps.writeImage(decision.filename, decision.bytes); - deps.postResult(requestId, relativePath); + relativePath = await deps.writeImage(decision.filename, decision.bytes); } catch (err) { console.error("[quoll] image write failed", err); deps.showError( "Quoll: failed to write the image file. See the extension host log for details." ); deps.postResult(requestId, null); + return; } + deps.postResult(requestId, relativePath); } diff --git a/src/extension/image/image-write-wiring.ts b/src/extension/image/image-write-wiring.ts index 89cfc132..7fe4da8f 100644 --- a/src/extension/image/image-write-wiring.ts +++ b/src/extension/image/image-write-wiring.ts @@ -18,6 +18,11 @@ import { Uri, workspace } from "vscode"; import type { HostToWebview } from "../../shared/protocol.js"; import { buildImageWriteResultMessage } from "../session/document-message.js"; +import { + createSessionVolumeBudget, + SESSION_IMAGE_WRITE_BUDGET_BYTES, + SESSION_IMAGE_WRITE_BUDGET_TOAST, +} from "./image-write-budget.js"; import { handleImageWrite } from "./image-write-service.js"; export interface ImageWriteWiringDeps { @@ -35,6 +40,10 @@ export interface ImageWriteWiringDeps { * (harness.writeImageFileOverride) — read PER WRITE (it can be set after * resolve). null routes to workspace.fs.writeFile. */ readonly writeFileOverride: () => ((uri: Uri, content: Uint8Array) => Thenable) | null; + /** Session cumulative-volume ceiling in bytes. Defaults to + * SESSION_IMAGE_WRITE_BUDGET_BYTES; overridable only so tests can trip the cap + * without allocating half a gigabyte. */ + readonly budgetBytes?: number; } export interface ImageWriteWiring { @@ -44,12 +53,19 @@ export interface ImageWriteWiring { } export function createImageWriteWiring(deps: ImageWriteWiringDeps): ImageWriteWiring { + // One budget per wiring instance = one per panel = one per editor session. It + // owns its own one-time warning (a per-write toast would be a flood vector). + const budget = createSessionVolumeBudget( + deps.budgetBytes ?? SESSION_IMAGE_WRITE_BUDGET_BYTES, + () => deps.showError(SESSION_IMAGE_WRITE_BUDGET_TOAST) + ); return { handle(requestId: string, data: string): void { void handleImageWrite( { canWrite: deps.canWrite, showError: deps.showError, + budget, postResult: (id, relativePath) => deps.post(buildImageWriteResultMessage(id, relativePath)), // writeImage creates /assets/ then writes — the explicit diff --git a/test/extension/image/image-write-budget.test.ts b/test/extension/image/image-write-budget.test.ts new file mode 100644 index 00000000..9499ef82 --- /dev/null +++ b/test/extension/image/image-write-budget.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it, vi } from "vitest"; + +import { createSessionVolumeBudget } from "../../../src/extension/image/image-write-budget.js"; + +describe("createSessionVolumeBudget", () => { + it("allows writes up to (and including) the budget, then rejects", () => { + const onExceeded = vi.fn(); + const budget = createSessionVolumeBudget(100, onExceeded); + + expect(budget.reserve(60)).toBe(true); // spent 60 + expect(budget.reserve(40)).toBe(true); // spent 100 — exactly at the cap + expect(budget.reserve(1)).toBe(false); // 101 > 100 — over budget + expect(onExceeded).toHaveBeenCalledOnce(); + }); + + it("warns exactly once no matter how many over-budget writes follow", () => { + const onExceeded = vi.fn(); + const budget = createSessionVolumeBudget(10, onExceeded); + + expect(budget.reserve(8)).toBe(true); // spent 8 + expect(budget.reserve(5)).toBe(false); // 13 > 10 — first over-budget, warns + expect(budget.reserve(5)).toBe(false); // still over — no second warning + expect(onExceeded).toHaveBeenCalledOnce(); + }); + + it("does not charge the budget for a rejected write (a rejected spend cannot exhaust it)", () => { + const onExceeded = vi.fn(); + const budget = createSessionVolumeBudget(10, onExceeded); + + expect(budget.reserve(8)).toBe(true); // spent 8 + expect(budget.reserve(5)).toBe(false); // 13 > 10 — rejected, NOT charged + expect(budget.reserve(2)).toBe(true); // 8 + 2 = 10 still fits + expect(budget.reserve(1)).toBe(false); // 11 > 10 + }); + + it("never refunds a charge: a spend stays spent for the life of the session", () => { + // No release/refund counterpart exists — a charged write is permanent even + // if the write later fails, so total disk growth stays bounded by the cap. + const budget = createSessionVolumeBudget(10, vi.fn()); + expect(budget.reserve(10)).toBe(true); // spent 10 — full + expect(budget.reserve(1)).toBe(false); // still full; the charge did not lapse + expect("release" in budget).toBe(false); // no refund API to reopen the cap + }); + + it("rejects a NaN or negative byteLength instead of poisoning the running total", () => { + const budget = createSessionVolumeBudget(10, vi.fn()); + expect(() => budget.reserve(Number.NaN)).toThrow(RangeError); + expect(() => budget.reserve(-1)).toThrow(RangeError); + expect(() => budget.reserve(1.5)).toThrow(RangeError); + // The cap is still intact after the rejected inputs (spent untouched). + expect(budget.reserve(10)).toBe(true); + }); +}); diff --git a/test/extension/image/image-write-service.test.ts b/test/extension/image/image-write-service.test.ts index e6aa977c..72a3e3b3 100644 --- a/test/extension/image/image-write-service.test.ts +++ b/test/extension/image/image-write-service.test.ts @@ -1,17 +1,29 @@ import { describe, expect, it, vi } from "vitest"; +import { createSessionVolumeBudget } from "../../../src/extension/image/image-write-budget.js"; import { handleImageWrite } from "../../../src/extension/image/image-write-service.js"; import { MAX_IMAGE_DATA_LENGTH } from "../../../src/shared/protocol.js"; const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00]); const pngBase64 = Buffer.from(PNG).toString("base64"); +// An all-permitting budget stub — the default so tests that don't exercise the +// cap read cleanly. Tests that care pass a spy via `overrides.budget`. +const unboundedBudget = () => ({ reserve: vi.fn(() => true) }); + function makeDeps(overrides: Partial[0]> = {}) { const writeImage = vi.fn(async (filename: string) => `./assets/${filename}`); const showError = vi.fn(); const postResult = vi.fn(); return { - deps: { canWrite: () => true, writeImage, showError, postResult, ...overrides }, + deps: { + canWrite: () => true, + writeImage, + showError, + postResult, + budget: unboundedBudget(), + ...overrides, + }, writeImage, showError, postResult, @@ -29,37 +41,98 @@ describe("handleImageWrite", () => { expect(showError).not.toHaveBeenCalled(); }); - it("rejects on a read-only document without writing", async () => { - const { deps, writeImage, postResult, showError } = makeDeps({ canWrite: () => false }); + it("rejects on a read-only document without writing OR charging the budget", async () => { + const budget = unboundedBudget(); + const { deps, writeImage, postResult, showError } = makeDeps({ + canWrite: () => false, + budget, + }); await handleImageWrite(deps, "r1", pngBase64); expect(writeImage).not.toHaveBeenCalled(); expect(postResult).toHaveBeenCalledWith("r1", null); expect(showError).toHaveBeenCalledTimes(1); + // The read-only guard precedes (and short-circuits) the budget charge — pins + // that ordering so a future reorder that charges before the guard is caught. + expect(budget.reserve).not.toHaveBeenCalled(); }); - it("rejects a non-image (svg) without writing", async () => { - const { deps, writeImage, postResult } = makeDeps(); + it("rejects a non-image (svg) without writing OR charging the budget", async () => { + const budget = unboundedBudget(); + const { deps, writeImage, postResult } = makeDeps({ budget }); const svg = Buffer.from("").toString("base64"); await handleImageWrite(deps, "r1", svg); expect(writeImage).not.toHaveBeenCalled(); expect(postResult).toHaveBeenCalledWith("r1", null); + // A per-message-cap rejection (unsupported type) must skip the budget entirely. + expect(budget.reserve).not.toHaveBeenCalled(); }); - it("surfaces an error and posts null when writeImage rejects", async () => { + it("surfaces an error and posts null when writeImage rejects, and keeps the charge (no refund)", async () => { const writeImage = vi.fn(async () => { throw new Error("disk full"); }); - const { deps, postResult, showError } = makeDeps({ writeImage }); + const budget = unboundedBudget(); + const { deps, postResult, showError } = makeDeps({ writeImage, budget }); await handleImageWrite(deps, "r1", pngBase64); expect(postResult).toHaveBeenCalledWith("r1", null); + // Exactly one result post — the success post is outside the try, so a write + // failure does not double-post. + expect(postResult).toHaveBeenCalledTimes(1); expect(showError).toHaveBeenCalledTimes(1); + // The charge stands even though the write failed: a failed write can still + // leave a partial file, so the budget must count the attempt (no refund API). + expect(budget.reserve).toHaveBeenCalledWith(PNG.length); + expect(budget).not.toHaveProperty("release"); + }); + + it("a run of failed writes deterministically exhausts the session cap (fail-safe, no refund)", async () => { + // Real (small) budget so the no-refund contract is exercised end-to-end: two + // failed writes still consume the cap, so the third is rejected before the + // write is even attempted — this is what bounds disk under a write-failure + // flood. PNG is 9 bytes; budget = 2 writes' worth. + const writeImage = vi.fn(async () => { + throw new Error("disk full"); + }); + const budget = createSessionVolumeBudget(PNG.length * 2, vi.fn()); + const { deps } = makeDeps({ writeImage, budget }); + await handleImageWrite(deps, "r1", pngBase64); // charged, write fails + await handleImageWrite(deps, "r2", pngBase64); // charged, write fails — cap full + await handleImageWrite(deps, "r3", pngBase64); // over budget — rejected pre-write + expect(writeImage).toHaveBeenCalledTimes(2); // r3 never reached the write }); - it("rejects an over-cap data string without decoding or writing", async () => { - const { deps, writeImage, postResult } = makeDeps(); + it("rejects an over-cap data string without decoding, writing, OR charging the budget", async () => { + const budget = unboundedBudget(); + const { deps, writeImage, postResult } = makeDeps({ budget }); const huge = "a".repeat(MAX_IMAGE_DATA_LENGTH + 1); await handleImageWrite(deps, "r1", huge); expect(writeImage).not.toHaveBeenCalled(); expect(postResult).toHaveBeenCalledWith("r1", null); + // The over-cap early exit precedes the budget charge — pins that an + // undecoded, oversized payload never touches the session budget. + expect(budget.reserve).not.toHaveBeenCalled(); + }); + + it("rejects a validated write when the session budget denies it (no showError — the budget warns)", async () => { + const budget = { reserve: vi.fn(() => false) }; + const { deps, writeImage, postResult, showError } = makeDeps({ budget }); + await handleImageWrite(deps, "r1", pngBase64); + // The image passed every per-message gate — the budget is charged with the + // validated byte length, and only then does it deny the write. + expect(budget.reserve).toHaveBeenCalledWith(PNG.length); + expect(writeImage).not.toHaveBeenCalled(); + expect(postResult).toHaveBeenCalledWith("r1", null); + // The budget owns its one-time warning; the service must NOT toast here. + expect(showError).not.toHaveBeenCalled(); + }); + + it("writes normally when the session budget allows it", async () => { + const budget = { reserve: vi.fn(() => true) }; + const { deps, writeImage, postResult } = makeDeps({ budget }); + await handleImageWrite(deps, "r1", pngBase64); + expect(budget.reserve).toHaveBeenCalledWith(PNG.length); + expect(writeImage).toHaveBeenCalledTimes(1); + const [filename] = writeImage.mock.calls[0]; + expect(postResult).toHaveBeenCalledWith("r1", `./assets/${filename}`); }); }); diff --git a/test/extension/image/image-write-wiring.test.ts b/test/extension/image/image-write-wiring.test.ts index e879fdb4..5564479a 100644 --- a/test/extension/image/image-write-wiring.test.ts +++ b/test/extension/image/image-write-wiring.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import { workspace } from "vscode"; +import { SESSION_IMAGE_WRITE_BUDGET_TOAST } from "../../../src/extension/image/image-write-budget.js"; import { createImageWriteWiring } from "../../../src/extension/image/image-write-wiring.js"; // A minimal valid PNG (8-byte signature) — decideImageWrite sniffs the magic @@ -76,6 +77,115 @@ describe("createImageWriteWiring", () => { createDirSpy.mockRestore(); }); + it("enforces the per-session cumulative volume cap: writes past the budget are rejected with one warning", async () => { + // PNG_BYTES is 8 bytes; a budget of 8 admits exactly the first write, then + // denies every subsequent one for the life of this wiring (= the session). + const write = vi.fn(async () => {}); + const post = vi.fn(); + const showError = vi.fn(); + const wiring = createImageWriteWiring({ + documentUri, + canWrite: () => true, + showError, + post, + writeFileOverride: () => write, + budgetBytes: PNG_BYTES.length, + }); + + // 1st write — fits the budget exactly, written + ok:true. + wiring.handle("req-1", PNG_BASE64); + await flush(); + // 2nd write — over budget, rejected without touching disk, ok:false + warning. + wiring.handle("req-2", PNG_BASE64); + await flush(); + // 3rd write — still over budget, still rejected, but NO second warning. + wiring.handle("req-3", PNG_BASE64); + await flush(); + + expect(write).toHaveBeenCalledOnce(); + expect(post).toHaveBeenCalledWith( + expect.objectContaining({ type: "image-write-result", requestId: "req-1", ok: true }) + ); + expect(post).toHaveBeenCalledWith( + expect.objectContaining({ type: "image-write-result", requestId: "req-2", ok: false }) + ); + expect(post).toHaveBeenCalledWith( + expect.objectContaining({ type: "image-write-result", requestId: "req-3", ok: false }) + ); + // Exactly one budget warning across the two rejected writes. + const budgetWarnings = showError.mock.calls.filter( + ([msg]) => msg === SESSION_IMAGE_WRITE_BUDGET_TOAST + ); + expect(budgetWarnings).toHaveLength(1); + }); + + it("scopes the budget per wiring instance: exhausting one panel's budget does not starve another", async () => { + // Two independent wirings (= two panels/sessions). The design claims one + // budget per panel; a regression that hoisted the budget to module scope + // would let panel A's flood reject panel B's first write — this pins it. + const makeWiring = () => { + const write = vi.fn(async () => {}); + const post = vi.fn(); + const wiring = createImageWriteWiring({ + documentUri, + canWrite: () => true, + showError: vi.fn(), + post, + writeFileOverride: () => write, + budgetBytes: PNG_BYTES.length, // admits exactly one write + }); + return { wiring, write, post }; + }; + + const a = makeWiring(); + const b = makeWiring(); + + // Exhaust panel A: 1st fits, 2nd is over budget. + a.wiring.handle("a-1", PNG_BASE64); + a.wiring.handle("a-2", PNG_BASE64); + await flush(); + // Panel B's FIRST write must still succeed — its budget is untouched. + b.wiring.handle("b-1", PNG_BASE64); + await flush(); + + expect(a.write).toHaveBeenCalledOnce(); // A got exactly one write + expect(b.write).toHaveBeenCalledOnce(); // B not starved by A's flood + expect(b.post).toHaveBeenCalledWith( + expect.objectContaining({ type: "image-write-result", requestId: "b-1", ok: true }) + ); + }); + + it("uses a generous default budget when budgetBytes is omitted (normal multi-image paste is unaffected)", async () => { + // Omitting budgetBytes falls back to SESSION_IMAGE_WRITE_BUDGET_BYTES. A + // handful of real (distinct) images must all write and none trip the cap or + // warn — the Done-when "normal single/multi-image paste flows unaffected". + const write = vi.fn(async () => {}); + const post = vi.fn(); + const showError = vi.fn(); + const wiring = createImageWriteWiring({ + documentUri, + canWrite: () => true, + showError, + post, + writeFileOverride: () => write, + // budgetBytes omitted → real SESSION_IMAGE_WRITE_BUDGET_BYTES default. + }); + + // Distinct payloads so each content-addresses to a new file (a real paste + // batch), all far under the 512 MiB default. + for (let i = 0; i < 5; i++) { + const bytes = Buffer.concat([PNG_BYTES, Buffer.from([i])]); + wiring.handle(`req-${i}`, bytes.toString("base64")); + } + await flush(); + + expect(write).toHaveBeenCalledTimes(5); + const warnings = showError.mock.calls.filter( + ([msg]) => msg === SESSION_IMAGE_WRITE_BUDGET_TOAST + ); + expect(warnings).toHaveLength(0); + }); + it("re-reads writeFileOverride per call (late-bound override, not captured at construction)", async () => { // The wiring reads deps.writeFileOverride() fresh inside the write closure on // every handle() — the e2e harness sets writeImageFileOverride AFTER the panel