diff --git a/src/runtime/server/og-image/core/plugins/imageSrc.ts b/src/runtime/server/og-image/core/plugins/imageSrc.ts index 8214060d..bb6ecc05 100644 --- a/src/runtime/server/og-image/core/plugins/imageSrc.ts +++ b/src/runtime/server/og-image/core/plugins/imageSrc.ts @@ -8,7 +8,7 @@ import { fetchLocalAsset } from '../../../util/fetchLocalAsset' import { getFetchTimeout } from '../../../util/fetchTimeout' import { logger } from '../../../util/logger' import { getOgImageOrigin } from '../../../util/origin' -import { fetchWithRedirectValidation, isBlockedUrl, parseSameOriginPath } from '../../../util/ssrf' +import { fetchWithRedirectValidation, isBlockedUrl, parseSameOriginPath, resolveSameOriginUrl } from '../../../util/ssrf' import { getImageDimensions } from '../../utils/image-detector' import { defineTransformer } from '../plugins' @@ -89,6 +89,8 @@ async function doResolveSrcToBuffer( } if (src.startsWith('/')) { + if (!resolveSameOriginUrl(src, getOgImageOrigin(e))) + return { blocked: true } let buffer: BufferSource | undefined if (import.meta.prerender || import.meta.dev) { const srcWithoutBase = src.replace(runtimeConfig.app.baseURL, '/') @@ -191,17 +193,12 @@ export default defineTransformer([ applyImageDimensions(node, result.buffer) return } - // relative src couldn't be fetched — fall back to an absolute URL so - // satori/takumi may attempt to resolve at render time (against the app's - // own origin, not an attacker-controlled one) - if (isRelative) { + // In development, renderers may retry unresolved assets against the app origin. + if (isRelative && import.meta.dev) { node.props.src = withBase(src, `${getOgImageOrigin(ctx.e)}`) return } - // Absolute external URL whose validated fetch failed. Drop it in - // production: letting it survive would invite the renderer (satori - // fetches URLs internally; takumi fetches via extractResourceUrls) - // to re-issue the request without SSRF/redirect validation. + // Production removes unresolved sources so renderers cannot fetch them without validation. if (!import.meta.dev) delete node.props.src }, @@ -223,9 +220,8 @@ export default defineTransformer([ node.props.style!.backgroundImage = `url(${toBufferSourceAsBase64(result.buffer)})` return } - // Same reasoning as the branch: an absolute external URL that - // didn't inline must not be left for the renderer to fetch unvalidated. - if (!import.meta.dev && !src.startsWith('/')) + // Production removes unresolved sources so renderers cannot fetch them without validation. + if (!import.meta.dev) delete node.props.style!.backgroundImage }, }, diff --git a/src/runtime/server/util/cloudflareAssets.ts b/src/runtime/server/util/cloudflareAssets.ts index a1e769ca..ded84ca7 100644 --- a/src/runtime/server/util/cloudflareAssets.ts +++ b/src/runtime/server/util/cloudflareAssets.ts @@ -43,7 +43,7 @@ export async function tryCloudflareAssetsFetch( return const origin = getRequestURL(event).origin const url = new URL(path, origin).href - const res = await assets.fetch(url, signal ? { signal } : undefined).catch(() => { + const res = await assets.fetch(url, { signal, redirect: 'manual' }).catch(() => { // Binding failures fall through to the caller's other asset sources. return null }) diff --git a/src/runtime/server/util/fetchLocalAsset.ts b/src/runtime/server/util/fetchLocalAsset.ts index fbaccdeb..c7b722f7 100644 --- a/src/runtime/server/util/fetchLocalAsset.ts +++ b/src/runtime/server/util/fetchLocalAsset.ts @@ -1,8 +1,8 @@ import type { H3Event } from '#nuxtseo/h3' -import { $fetch } from 'ofetch' import { fetchWithEvent } from '#nuxtseo/nitro' import { tryCloudflareAssetsFetch } from './cloudflareAssets' import { getOgImageOrigin } from './origin' +import { fetchWithRedirectValidation, resolveSameOriginUrl } from './ssrf' export interface FetchLocalAssetOptions { fetchTimeout: number @@ -21,7 +21,7 @@ export interface FetchLocalAssetOptions { * Resolve a same-origin asset path to bytes, trying (in order): * 1. Cloudflare Workers ASSETS binding * 2. Nitro localFetch through the event-aware bridge, which resolves dynamic routes too - * 3. External `$fetch` to the Nitro origin (opt-in) + * 3. External fetch with redirect validation (opt-in) * * All steps share a single `AbortSignal.timeout(fetchTimeout)` so a broken * URL can't burn N× the configured budget. @@ -33,6 +33,11 @@ export async function fetchLocalAsset( path: string, options: FetchLocalAssetOptions, ): Promise { + const absolute = resolveSameOriginUrl(path, getOgImageOrigin(event)) + if (!path.startsWith('/') || !absolute) + return + const url = new URL(absolute) + path = `${url.pathname}${url.search}` const { fetchTimeout, headers, includeExternalFallback = false, onStepFailure } = options const deadline = AbortSignal.timeout(fetchTimeout) @@ -58,9 +63,14 @@ export async function fetchLocalAsset( result = await race(fetchWithEvent(event, path, { responseType: 'arrayBuffer', + redirect: 'manual', signal: deadline, timeout: fetchTimeout, headers, + onResponse({ response }) { + if (!response.ok) + throw new Error(response.statusText || String(response.status)) + }, }).catch((err: unknown) => { onStepFailure?.(path, err) return undefined @@ -68,14 +78,10 @@ export async function fetchLocalAsset( if (result || expired || !includeExternalFallback) return result - const absolute = `${getOgImageOrigin(event)}${path}` - return await race($fetch(absolute, { - responseType: 'arrayBuffer', + return await race(fetchWithRedirectValidation(absolute, { signal: deadline, timeout: fetchTimeout, headers, - }).catch((err: unknown) => { - onStepFailure?.(absolute, err) - return undefined - }) as Promise) + trustedHost: url.host, + }).then(buffer => buffer ?? undefined)) } diff --git a/test/unit/local-asset-redirects.test.ts b/test/unit/local-asset-redirects.test.ts new file mode 100644 index 00000000..3aaf35f9 --- /dev/null +++ b/test/unit/local-asset-redirects.test.ts @@ -0,0 +1,64 @@ +import type { AddressInfo } from 'node:net' +import type { H3Event } from '#nuxtseo/h3' +import { createServer } from 'node:http' +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' +import { fetchLocalAsset } from '../../src/runtime/server/util/fetchLocalAsset' + +vi.mock('#nuxtseo/nitro', () => ({ fetchWithEvent: async () => undefined })) +vi.mock('#nuxtseo/h3', () => ({ getRequestURL: (event: { context: { origin: string } }) => new URL(event.context.origin) })) +vi.mock('#site-config/server/composables', () => ({ getNitroOrigin: (event: { context: { origin: string } }) => event.context.origin })) + +describe('local asset external fallback', () => { + let targetHits = 0 + let origin = '' + let assetRequest: { path?: string, header?: string } = {} + const target = createServer((_, response) => { + targetHits++ + response.end('fixture bytes') + }) + const app = createServer((request, response) => { + if (request.url === '/redirect.png') { + response.writeHead(302, { location: `http://127.0.0.1:${(target.address() as AddressInfo).port}/asset.png` }) + response.end() + return + } + assetRequest = { path: request.url, header: request.headers['x-nuxt-og-image'] as string | undefined } + response.end('asset bytes') + }) + + beforeAll(async () => { + for (const server of [target, app]) { + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', resolve) + }) + } + origin = `http://127.0.0.1:${(app.address() as AddressInfo).port}` + }) + + afterAll(async () => { + await Promise.all([target, app].map(server => new Promise((resolve, reject) => { + server.close(error => error ? reject(error) : resolve()) + }))) + }) + + function event(): H3Event { + return { context: { origin } } as unknown as H3Event + } + + it('does not follow a redirect to another local listener', async () => { + const bytes = await fetchLocalAsset(event(), '/redirect.png', { fetchTimeout: 500, includeExternalFallback: true }) + expect(targetHits).toBe(0) + expect(bytes).toBeUndefined() + }) + + it('fetches app assets with their query and headers', async () => { + const bytes = await fetchLocalAsset(event(), '/asset.png?width=32', { + fetchTimeout: 500, + includeExternalFallback: true, + headers: { 'x-nuxt-og-image': '1' }, + }) + expect(new TextDecoder().decode(bytes)).toBe('asset bytes') + expect(assetRequest).toEqual({ path: '/asset.png?width=32', header: '1' }) + }) +}) diff --git a/test/unit/local-asset-responses.test.ts b/test/unit/local-asset-responses.test.ts new file mode 100644 index 00000000..8a186623 --- /dev/null +++ b/test/unit/local-asset-responses.test.ts @@ -0,0 +1,35 @@ +import type { H3Event } from '#nuxtseo/h3' +import { createFetch } from 'ofetch' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { fetchLocalAsset } from '../../src/runtime/server/util/fetchLocalAsset' + +const transport = vi.hoisted(() => ({ external: vi.fn() })) +vi.mock('#nuxtseo/nitro', () => ({ + fetchWithEvent: (event: { $fetch: (path: string, options: unknown) => unknown }, path: string, options: unknown) => event.$fetch(path, options), +})) +vi.mock('#nuxtseo/h3', () => ({ getRequestURL: () => new URL('https://site.example/render') })) +vi.mock('#site-config/server/composables', () => ({ getNitroOrigin: () => 'https://site.example' })) +vi.mock('#og-image/bindings/fetch', () => ({ guardedFetch: transport.external })) + +beforeEach(() => { + vi.clearAllMocks() + transport.external.mockResolvedValue(new Response('asset bytes')) +}) + +describe('local asset response', () => { + it.each([301, 302, 303, 307, 308, 404, 500])('uses the validated fallback after status %i', async (status) => { + const local = createFetch({ fetch: async () => new Response('response body', { status, headers: { location: '/actual.png' } }) }) + const event = { context: {}, $fetch: local } as unknown as H3Event + const bytes = await fetchLocalAsset(event, '/logo.png', { fetchTimeout: 100, includeExternalFallback: true }) + expect(new TextDecoder().decode(bytes)).toBe('asset bytes') + expect(transport.external).toHaveBeenCalledOnce() + }) + + it('uses a successful local response without an external fetch', async () => { + const local = createFetch({ fetch: async () => new Response('local bytes') }) + const event = { context: {}, $fetch: local } as unknown as H3Event + const bytes = await fetchLocalAsset(event, '/logo.png', { fetchTimeout: 100, includeExternalFallback: true }) + expect(new TextDecoder().decode(bytes)).toBe('local bytes') + expect(transport.external).not.toHaveBeenCalled() + }) +}) diff --git a/test/unit/local-assets.test.ts b/test/unit/local-assets.test.ts new file mode 100644 index 00000000..d2a95483 --- /dev/null +++ b/test/unit/local-assets.test.ts @@ -0,0 +1,95 @@ +import type { H3Event } from '#nuxtseo/h3' +import type { OgImageRenderEventContext, VNode } from '../../src/runtime/types' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { walkTree } from '../../src/runtime/server/og-image/core/plugins' +import imageSrc from '../../src/runtime/server/og-image/core/plugins/imageSrc' +import { fetchLocalAsset } from '../../src/runtime/server/util/fetchLocalAsset' + +const transport = vi.hoisted(() => ({ local: vi.fn(), external: vi.fn() })) +vi.mock('#nuxtseo/nitro', () => ({ + fetchWithEvent: transport.local, + useStorage: () => ({ hasItem: async () => false }), +})) +vi.mock('#nuxtseo/h3', () => ({ getRequestURL: () => new URL('https://site.example/render') })) +vi.mock('#site-config/server/composables', () => ({ getNitroOrigin: () => 'https://site.example' })) +vi.mock('#og-image/bindings/fetch', () => ({ guardedFetch: transport.external })) + +const crossOriginPaths = ['//other.example/logo.png', '/\\other.example/logo.png', '/\t/other.example/logo.png', '/\n/other.example/logo.png'] + +function createEvent(assets?: { fetch: (url: string) => Promise }): H3Event { + return { context: { ASSETS: assets } } as unknown as H3Event +} + +function createContext(): OgImageRenderEventContext { + return { + e: createEvent(), + publicStoragePath: 'public', + runtimeConfig: { app: { baseURL: '/' }, security: { imageFetchTimeout: 100 } }, + timings: { + measure: async (_name: string, work: () => T | Promise) => work(), + start: () => () => {}, + }, + } as unknown as OgImageRenderEventContext +} + +beforeEach(() => { + vi.clearAllMocks() + transport.local.mockResolvedValue(undefined) + transport.external.mockResolvedValue(new Response(null, { status: 404 })) +}) + +describe('local asset fetch', () => { + it.each(crossOriginPaths)('keeps %j outside local transports', async (path) => { + const assets = { fetch: vi.fn(async () => new Response('asset')) } + expect(await fetchLocalAsset(createEvent(assets), path, { fetchTimeout: 20, includeExternalFallback: true })).toBeUndefined() + expect(assets.fetch).not.toHaveBeenCalled() + expect(transport.local).not.toHaveBeenCalled() + expect(transport.external).not.toHaveBeenCalled() + }) + + it('keeps asset query parameters when using the local transport', async () => { + const bytes = new ArrayBuffer(4) + transport.local.mockResolvedValue(bytes) + const event = createEvent() + expect(await fetchLocalAsset(event, '/logo.png?width=32', { fetchTimeout: 20 })).toBe(bytes) + expect(transport.local).toHaveBeenCalledWith(event, '/logo.png?width=32', expect.objectContaining({ redirect: 'manual' })) + }) + + it('reads a same-origin asset from the binding', async () => { + const assets = { fetch: vi.fn(async () => new Response('asset')) } + const bytes = await fetchLocalAsset(createEvent(assets), '/logo.png?width=32', { fetchTimeout: 20 }) + expect(new TextDecoder().decode(bytes)).toBe('asset') + expect(assets.fetch).toHaveBeenCalledWith('https://site.example/logo.png?width=32', expect.objectContaining({ redirect: 'manual' })) + expect(transport.local).not.toHaveBeenCalled() + }) +}) + +describe('image asset transformation', () => { + it.each(crossOriginPaths)('removes %j from image nodes', async (src) => { + const node: VNode = { type: 'img', props: { src } } + await Promise.all(walkTree(createContext(), node, [imageSrc])) + expect(node.props.src).toBeUndefined() + expect(transport.local).not.toHaveBeenCalled() + expect(transport.external).not.toHaveBeenCalled() + }) + + it.each(crossOriginPaths)('removes %j from background images', async (src) => { + const node: VNode = { type: 'div', props: { style: { backgroundImage: `url(${src})` } } } + await Promise.all(walkTree(createContext(), node, [imageSrc])) + expect(node.props.style?.backgroundImage).toBeUndefined() + expect(transport.local).not.toHaveBeenCalled() + expect(transport.external).not.toHaveBeenCalled() + }) + + it('removes an unavailable local image', async () => { + const node: VNode = { type: 'img', props: { src: '/logo.png?width=32' } } + await Promise.all(walkTree(createContext(), node, [imageSrc])) + expect(node.props.src).toBeUndefined() + }) + + it('removes an unavailable local background image', async () => { + const node: VNode = { type: 'div', props: { style: { backgroundImage: 'url(/logo.png?width=32)' } } } + await Promise.all(walkTree(createContext(), node, [imageSrc])) + expect(node.props.style?.backgroundImage).toBeUndefined() + }) +})