From a0f52bb40613139a6c5aff175988d4502a5a2473 Mon Sep 17 00:00:00 2001 From: Jim Vinson Date: Mon, 29 Jun 2026 04:11:31 -0700 Subject: [PATCH] fix(compose): free WASM images on throw + cap pixel/raw sizes (review M2/M3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two confirmed-Medium resource-safety findings in the compose path. M2 — packages/limner-core/src/compose/photon-ops.ts: every wrapper freed its PhotonImage(s) only on the success path. A caller-triggerable throw (out-of-bounds crop, undecodable bytes) orphaned the decoded WASM buffer (multi-MB) — the server catch keeps the worker alive, so the leak is silent and accumulates across requests in a warm isolate toward the 128 MB cap. Wrapped each op in try/finally so img/out (and watermark's two images) are freed on any path. Protects both the MCP and CMA consumers (shared core). M3 — compose handler (mcp tools/compose.ts + the cma-tools twin): width/height were validated only as positive ints with no pixel-count ceiling, so a request like 100000x100000 forced a multi-GB RGBA allocation inside the isolate (OOM). And encode built a Uint8ClampedArray from caller raw.data without checking its length matched width*height*4, so the codec could read past the buffer (corrupt output) or pair a tiny buffer with huge dimensions. Added a 10 MP cap (MAX_COMPOSE_PIXELS, the documented in-isolate ceiling; oversize -> cf-images) on resize/crop/renderText/encode, and an exact raw-length check on encode. Mirrored into @limner/cma-tools for parity (D-RA-12). Tests: mocked-photon free-on-throw for all 7 ops (incl. watermark frees both); oversize-resize and encode-length-mismatch rejected cleanly in both the MCP and CMA dispatchers. Suite 677 -> 692. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Jim Vinson --- .../limner-cma-tools/src/tools/compose.ts | 24 +++- .../test/tools/compose.test.ts | 24 ++++ .../limner-core/src/compose/photon-ops.ts | 107 +++++++++++------- .../test/compose/photon-ops.free.test.ts | 64 +++++++++++ packages/limner-mcp/src/tools/compose.ts | 37 +++++- .../limner-mcp/test/tools/compose.test.ts | 40 +++++++ 6 files changed, 252 insertions(+), 44 deletions(-) create mode 100644 packages/limner-core/test/compose/photon-ops.free.test.ts diff --git a/packages/limner-cma-tools/src/tools/compose.ts b/packages/limner-cma-tools/src/tools/compose.ts index aed961f..856094b 100644 --- a/packages/limner-cma-tools/src/tools/compose.ts +++ b/packages/limner-cma-tools/src/tools/compose.ts @@ -24,6 +24,17 @@ function base64ToBytes(b64: string): Uint8Array { return out; } +// M3 parity with @limner/mcp: cap the in-isolate output pixel count so a caller +// can't request a multi-GB RGBA allocation and OOM the 128 MB V8 isolate. +const MAX_COMPOSE_PIXELS = 10_000_000; +function assertPixelCap(width: number, height: number, op: string): void { + if (width * height > MAX_COMPOSE_PIXELS) { + throw new Error( + `compose.${op}: ${width}x${height} exceeds the ${MAX_COMPOSE_PIXELS}-pixel in-isolate limit. Use cfTransform/cfSmartCrop (Cloudflare Images) for larger output.`, + ); + } +} + function bytesToBase64(bytes: Uint8Array): string { let bin = ''; for (let i = 0; i < bytes.length; i++) bin += String.fromCharCode(bytes[i]!); @@ -70,11 +81,13 @@ export const composeMcpTool: CustomTool = defineTool({ switch (op) { case 'resize': { const i = input as { input: string; width: number; height: number; fit?: FitMode }; + assertPixelCap(i.width, i.height, 'resize'); const bytes = compose.resize(base64ToBytes(i.input), i.width, i.height, i.fit ?? 'cover'); return emitImage(env, bytes, 'image/png', 'resize'); } case 'crop': { const i = input as { input: string; x: number; y: number; width: number; height: number }; + assertPixelCap(i.width, i.height, 'crop'); const bytes = compose.crop(base64ToBytes(i.input), i.x, i.y, i.width, i.height); return emitImage(env, bytes, 'image/png', 'crop'); } @@ -110,8 +123,16 @@ export const composeMcpTool: CustomTool = defineTool({ format: 'jpeg' | 'png' | 'webp' | 'avif'; quality?: number; }; + assertPixelCap(i.raw.width, i.raw.height, 'encode'); + const rawBytes = base64ToBytes(i.raw.data); + const expected = i.raw.width * i.raw.height * 4; + if (rawBytes.length !== expected) { + throw new Error( + `compose.encode: raw.data is ${rawBytes.length} bytes but ${i.raw.width}x${i.raw.height} RGBA requires ${expected}.`, + ); + } const raw = { - data: new Uint8ClampedArray(base64ToBytes(i.raw.data).buffer), + data: new Uint8ClampedArray(rawBytes.buffer, rawBytes.byteOffset, rawBytes.byteLength), width: i.raw.width, height: i.raw.height, }; @@ -155,6 +176,7 @@ export const composeMcpTool: CustomTool = defineTool({ height: number; fonts?: Array<{ fontId: string; name?: string; weight?: number; style?: 'normal' | 'italic' }>; }; + assertPixelCap(i.width, i.height, 'renderText'); // Fonts resolve to server-side bytes by id (no inline base64; large // fonts truncate on the wire). Omitted fonts -> the default built-in. const specs = diff --git a/packages/limner-cma-tools/test/tools/compose.test.ts b/packages/limner-cma-tools/test/tools/compose.test.ts index 02a068a..4482ed7 100644 --- a/packages/limner-cma-tools/test/tools/compose.test.ts +++ b/packages/limner-cma-tools/test/tools/compose.test.ts @@ -59,6 +59,30 @@ describe('cma compose dispatch — photon op (resize)', () => { }); }); +describe('cma compose dispatch — resource guards (M3 parity)', () => { + test('resize rejects an oversize pixel count', async () => { + const { bucket, put } = mockBucket(); + await expect( + composeTool.run( + { op: 'resize', input: b64encode(fixture), width: 20000, height: 20000 }, + { env: { BUCKET: bucket } }, + ), + ).rejects.toThrow(/exceeds|pixel/i); + expect(put).not.toHaveBeenCalled(); + }); + + test('encode rejects a raw buffer whose length != width*height*4', async () => { + const { bucket, put } = mockBucket(); + await expect( + composeTool.run( + { op: 'encode', raw: { data: b64encode(new Uint8Array([1, 2, 3, 4])), width: 2, height: 2 }, format: 'png' }, + { env: { BUCKET: bucket } }, + ), + ).rejects.toThrow(/raw\.data|RGBA/i); + expect(put).not.toHaveBeenCalled(); + }); +}); + describe('cma compose dispatch — codec op (convert)', () => { test('routes through jsquash and uploads image/jpeg under compose-convert/', async () => { const { bucket, put } = mockBucket(); diff --git a/packages/limner-core/src/compose/photon-ops.ts b/packages/limner-core/src/compose/photon-ops.ts index c9c4ffc..ce564ce 100644 --- a/packages/limner-core/src/compose/photon-ops.ts +++ b/packages/limner-core/src/compose/photon-ops.ts @@ -55,12 +55,19 @@ export function resize( height: number, _fit: FitMode = 'cover', ): Uint8Array { - const img = PhotonImage.new_from_byteslice(input); - const out = photonResize(img, width, height, SamplingFilter.Lanczos3); - const bytes = out.get_bytes(); - img.free(); - out.free(); - return bytes; + // try/finally so the decoded WASM image is freed even when an op throws + // (undecodable bytes, out-of-bounds args); otherwise the allocation leaks and + // accumulates toward the 128 MB isolate cap across requests (M2). + let img: PhotonImage | undefined; + let out: PhotonImage | undefined; + try { + img = PhotonImage.new_from_byteslice(input); + out = photonResize(img, width, height, SamplingFilter.Lanczos3); + return out.get_bytes(); + } finally { + img?.free(); + out?.free(); + } } /** @@ -79,12 +86,16 @@ export function crop( width: number, height: number, ): Uint8Array { - const img = PhotonImage.new_from_byteslice(input); - const out = photonCrop(img, x, y, x + width, y + height); - const bytes = out.get_bytes(); - img.free(); - out.free(); - return bytes; + let img: PhotonImage | undefined; + let out: PhotonImage | undefined; + try { + img = PhotonImage.new_from_byteslice(input); + out = photonCrop(img, x, y, x + width, y + height); + return out.get_bytes(); + } finally { + img?.free(); + out?.free(); + } } /** @@ -92,11 +103,14 @@ export function crop( * positive brightens, negative darkens. Typical range: -50..+50. */ export function brightness(input: Uint8Array, delta: number): Uint8Array { - const img = PhotonImage.new_from_byteslice(input); - adjust_brightness(img, delta); - const bytes = img.get_bytes(); - img.free(); - return bytes; + let img: PhotonImage | undefined; + try { + img = PhotonImage.new_from_byteslice(input); + adjust_brightness(img, delta); + return img.get_bytes(); + } finally { + img?.free(); + } } /** @@ -104,22 +118,28 @@ export function brightness(input: Uint8Array, delta: number): Uint8Array { * >1 increases contrast, 0..1 decreases. Typical range: 0.5..2.0. */ export function contrast(input: Uint8Array, factor: number): Uint8Array { - const img = PhotonImage.new_from_byteslice(input); - adjust_contrast(img, factor); - const bytes = img.get_bytes(); - img.free(); - return bytes; + let img: PhotonImage | undefined; + try { + img = PhotonImage.new_from_byteslice(input); + adjust_contrast(img, factor); + return img.get_bytes(); + } finally { + img?.free(); + } } /** * Gaussian blur with a given pixel radius. Typical range: 1..20. */ export function blur(input: Uint8Array, radius: number): Uint8Array { - const img = PhotonImage.new_from_byteslice(input); - gaussian_blur(img, radius); - const bytes = img.get_bytes(); - img.free(); - return bytes; + let img: PhotonImage | undefined; + try { + img = PhotonImage.new_from_byteslice(input); + gaussian_blur(img, radius); + return img.get_bytes(); + } finally { + img?.free(); + } } /** @@ -127,11 +147,14 @@ export function blur(input: Uint8Array, radius: number): Uint8Array { * for finer control, multiple passes can be chained. */ export function sharpen(input: Uint8Array): Uint8Array { - const img = PhotonImage.new_from_byteslice(input); - photonSharpen(img); - const bytes = img.get_bytes(); - img.free(); - return bytes; + let img: PhotonImage | undefined; + try { + img = PhotonImage.new_from_byteslice(input); + photonSharpen(img); + return img.get_bytes(); + } finally { + img?.free(); + } } /** @@ -148,12 +171,16 @@ export function watermark( x: number, y: number, ): Uint8Array { - const baseImg = PhotonImage.new_from_byteslice(base); - const overlayImg = PhotonImage.new_from_byteslice(overlay); - // Photon's watermark takes BigInt for x,y (Rust u32 -> wasm i64 binding). - photonWatermark(baseImg, overlayImg, BigInt(x), BigInt(y)); - const bytes = baseImg.get_bytes(); - baseImg.free(); - overlayImg.free(); - return bytes; + let baseImg: PhotonImage | undefined; + let overlayImg: PhotonImage | undefined; + try { + baseImg = PhotonImage.new_from_byteslice(base); + overlayImg = PhotonImage.new_from_byteslice(overlay); + // Photon's watermark takes BigInt for x,y (Rust u32 -> wasm i64 binding). + photonWatermark(baseImg, overlayImg, BigInt(x), BigInt(y)); + return baseImg.get_bytes(); + } finally { + baseImg?.free(); + overlayImg?.free(); + } } diff --git a/packages/limner-core/test/compose/photon-ops.free.test.ts b/packages/limner-core/test/compose/photon-ops.free.test.ts new file mode 100644 index 0000000..b4ed26b --- /dev/null +++ b/packages/limner-core/test/compose/photon-ops.free.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, test, vi } from 'vitest'; + +// M2 (release review): every photon-ops wrapper must free its WASM-allocated +// PhotonImage(s) even when the op throws (out-of-bounds crop, undecodable +// bytes), or the decoded image buffer leaks and accumulates toward the 128 MB +// isolate cap. We mock @cf-wasm/photon so the op functions throw mid-way and +// assert .free() still runs (the try/finally). This file mocks the module, so +// it is kept separate from photon-ops.test.ts which exercises the real WASM. + +const freeSpy = vi.fn(); + +vi.mock('@cf-wasm/photon', () => { + class PhotonImage { + free = freeSpy; + get_bytes(): Uint8Array { + return new Uint8Array([1]); + } + static new_from_byteslice(): PhotonImage { + return new PhotonImage(); + } + } + const boom = () => { + throw new Error('photon boom'); + }; + return { + PhotonImage, + SamplingFilter: { Lanczos3: 0 }, + resize: boom, + crop: boom, + adjust_brightness: boom, + adjust_contrast: boom, + gaussian_blur: boom, + sharpen: boom, + watermark: boom, + }; +}); + +const { resize, crop, brightness, contrast, blur, sharpen, watermark } = await import( + '../../src/compose/photon-ops.js' +); + +const bytes = new Uint8Array([1, 2, 3]); + +describe('photon-ops free WASM allocations on throw (M2)', () => { + test.each([ + ['resize', () => resize(bytes, 10, 10)], + ['crop', () => crop(bytes, 0, 0, 10, 10)], + ['brightness', () => brightness(bytes, 10)], + ['contrast', () => contrast(bytes, 1.2)], + ['blur', () => blur(bytes, 2)], + ['sharpen', () => sharpen(bytes)], + ])('%s frees its image when the op throws', (_name, call) => { + freeSpy.mockClear(); + expect(call).toThrow('photon boom'); + expect(freeSpy).toHaveBeenCalled(); + }); + + test('watermark frees BOTH images when the op throws', () => { + freeSpy.mockClear(); + expect(() => watermark(bytes, bytes, 8, 8)).toThrow('photon boom'); + // base + overlay both allocated before the throw -> both freed. + expect(freeSpy).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/limner-mcp/src/tools/compose.ts b/packages/limner-mcp/src/tools/compose.ts index 9415129..b7042f0 100644 --- a/packages/limner-mcp/src/tools/compose.ts +++ b/packages/limner-mcp/src/tools/compose.ts @@ -238,6 +238,21 @@ function errorResult(message: string): CallToolResult { return { content: [{ type: 'text', text: message }], isError: true }; } +// M3: in-isolate ops decode/allocate raw RGBA in the 128 MB V8 isolate; photon +// documents a ~10 MP practical ceiling. Cap the output pixel count so a caller +// can't request a multi-GB allocation (e.g. 100000x100000) and OOM the isolate. +// Oversize work should route through the cf-images (network) ops. +const MAX_COMPOSE_PIXELS = 10_000_000; + +function pixelCapError(width: number, height: number, op: string): CallToolResult | null { + if (width * height > MAX_COMPOSE_PIXELS) { + return errorResult( + `compose.${op}: ${width}x${height} exceeds the ${MAX_COMPOSE_PIXELS}-pixel in-isolate limit. Use cfTransform/cfSmartCrop (Cloudflare Images) for larger output.`, + ); + } + return null; +} + // ---------------- handler ---------------- async function handle(raw: ComposeAdvertisedInput, ctx: ToolContext): Promise { @@ -252,9 +267,11 @@ async function handle(raw: ComposeAdvertisedInput, ctx: ToolContext): Promise the default built-in. diff --git a/packages/limner-mcp/test/tools/compose.test.ts b/packages/limner-mcp/test/tools/compose.test.ts index 9662854..ddb9969 100644 --- a/packages/limner-mcp/test/tools/compose.test.ts +++ b/packages/limner-mcp/test/tools/compose.test.ts @@ -268,6 +268,46 @@ describe('compose tool — cf-images ops (workers: with mock binding)', () => { }); }); +// M3 (release review): in-isolate ops must reject an oversize pixel count and a +// raw buffer whose length doesn't match its declared dimensions, before the +// codec attempts a multi-GB allocation inside the 128 MB isolate. +describe('compose tool — resource guards (M3)', () => { + test('resize rejects an oversize pixel count with a clean error', async () => { + const { client, close } = await connectedPair(localCtx); + try { + const result = await client.callTool({ + name: 'limner_compose', + arguments: { op: 'resize', input: b64encode(corefix('checker-64.png')), width: 20000, height: 20000 }, + }); + expect(result.isError).toBe(true); + const text = JSON.stringify(result.content); + expect(text).toMatch(/exceeds|pixel/i); + expect(text).not.toMatch(/undefined/); + } finally { + await close(); + } + }); + + test('encode rejects a raw buffer whose length != width*height*4', async () => { + const { client, close } = await connectedPair(localCtx); + try { + const result = await client.callTool({ + name: 'limner_compose', + arguments: { + op: 'encode', + // 2x2 RGBA needs 16 bytes; supply only 4. + raw: { data: b64encode(new Uint8Array([1, 2, 3, 4])), width: 2, height: 2 }, + format: 'png', + }, + }); + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toMatch(/raw\.data|bytes|RGBA/i); + } finally { + await close(); + } + }); +}); + describe('compose tool — schema validation', () => { test('unknown op -> isError', async () => { const { client, close } = await connectedPair(localCtx);