From 90566879d2fa7226b850349584ce97111112ee7f Mon Sep 17 00:00:00 2001 From: conico974 Date: Wed, 26 Aug 2026 15:55:57 +0200 Subject: [PATCH] Improve image signature handling and resizing functionality (#4511) --- .../src/lib/images/resizer/gitbook-service.ts | 34 ++++-- .../src/lib/images/resizer/resizeImage.ts | 21 +--- .../gitbook/src/lib/images/resizer/types.ts | 15 ++- .../gitbook/src/lib/images/signatures.test.ts | 99 ++++++++++++++++ packages/gitbook/src/lib/images/signatures.ts | 72 +++++++++--- packages/gitbook/src/lib/images/utils.ts | 24 ++++ packages/gitbook/src/routes/image.ts | 108 ++++++++++-------- 7 files changed, 285 insertions(+), 88 deletions(-) create mode 100644 packages/gitbook/src/lib/images/signatures.test.ts diff --git a/packages/gitbook/src/lib/images/resizer/gitbook-service.ts b/packages/gitbook/src/lib/images/resizer/gitbook-service.ts index c4d99be2a..798925c0f 100644 --- a/packages/gitbook/src/lib/images/resizer/gitbook-service.ts +++ b/packages/gitbook/src/lib/images/resizer/gitbook-service.ts @@ -1,4 +1,4 @@ -import type { CloudflareImageOptions } from './types'; +import type { CloudflareImageOptions, CloudflareResizeImageOptions } from './types'; import { copyImageResponse } from './utils'; import { GITBOOK_IMAGE_RESIZE_SALT, GITBOOK_IMAGE_RESIZE_URL } from '@/lib/env'; import { getLogger } from '@/lib/logger'; @@ -18,11 +18,10 @@ function sdbmHash(str: string): number { */ export async function resizeImageWithGitbookServices( input: string, - options: CloudflareImageOptions & { - signal?: AbortSignal; - } + options: CloudflareResizeImageOptions ): Promise { - const { signal, ...resizeOptions } = options; + // Only the resize options are serialized in the URL, everything else is transport-level. + const { signal, accept, bypassSkipCheck, ...resizeOptions } = options; if (!GITBOOK_IMAGE_RESIZE_SALT) { throw new Error( @@ -48,16 +47,33 @@ export async function resizeImageWithGitbookServices( return copyImageResponse( await fetch(resizeURL, { headers: { - Accept: - resizeOptions.format === 'json' - ? 'application/json' - : `image/${resizeOptions.format || 'jpeg'}`, + Accept: getAcceptHeader(resizeOptions.format, accept), }, signal, }) ); } +/** + * Accept header to send to the image service. + * With the default "auto" format, the service negotiates the output itself from the + * accept header of the original request. + */ +function getAcceptHeader( + format: CloudflareImageOptions['format'], + accept: string | undefined +): string { + if (format === 'json') { + return 'application/json'; + } + + if (format && format !== 'auto') { + return `image/${format}`; + } + + return accept || 'image/*'; +} + function stringifyOptions(options: CloudflareImageOptions & { signature: string }): string { return Object.entries({ ...options }).reduce((rest, [key, value]) => { return `${rest}${rest ? ',' : ''}${key}=${value}`; diff --git a/packages/gitbook/src/lib/images/resizer/resizeImage.ts b/packages/gitbook/src/lib/images/resizer/resizeImage.ts index 5c0ddb0cf..010936035 100644 --- a/packages/gitbook/src/lib/images/resizer/resizeImage.ts +++ b/packages/gitbook/src/lib/images/resizer/resizeImage.ts @@ -4,7 +4,11 @@ import assertNever from 'assert-never'; import { GITBOOK_IMAGE_RESIZE_MODE } from '../../env'; import { SizableImageAction, checkIsSizableImageURL } from '../checkIsSizableImageURL'; import { resizeImageWithGitbookServices } from './gitbook-service'; -import type { CloudflareImageJsonFormat, CloudflareImageOptions } from './types'; +import type { + CloudflareImageJsonFormat, + CloudflareImageOptions, + CloudflareResizeImageOptions, +} from './types'; import { getLogger } from '@/lib/logger'; /** @@ -41,15 +45,6 @@ export async function getImageSize( } } -export type CloudflareResizeImageOptions = CloudflareImageOptions & { - signal?: AbortSignal; - /** - * Bypass the check to see if the image can be resized. - * This is useful for some format that are not supported by @next/og and need to be transformed - */ - bypassSkipCheck?: boolean; -}; - /** * Execute a Cloudflare Image Resize operation on an image. */ @@ -64,12 +59,6 @@ export async function resizeImage( ); } - if (action === SizableImageAction.Passthrough) { - return fetch(input, { - signal: options.signal, - }); - } - switch (GITBOOK_IMAGE_RESIZE_MODE) { case 'cf-fetch': case 'gitbook-service': diff --git a/packages/gitbook/src/lib/images/resizer/types.ts b/packages/gitbook/src/lib/images/resizer/types.ts index 9cb370586..fe339285a 100644 --- a/packages/gitbook/src/lib/images/resizer/types.ts +++ b/packages/gitbook/src/lib/images/resizer/types.ts @@ -13,7 +13,7 @@ export interface CloudflareImageJsonFormat { * https://developers.cloudflare.com/images/image-resizing/resize-with-workers/ */ export interface CloudflareImageOptions { - format?: 'webp' | 'avif' | 'json' | 'jpeg' | 'png'; + format?: 'webp' | 'avif' | 'json' | 'jpeg' | 'png' | 'auto'; fit?: 'scale-down' | 'contain' | 'cover' | 'crop' | 'pad'; width?: number; height?: number; @@ -21,3 +21,16 @@ export interface CloudflareImageOptions { anim?: boolean; quality?: number; } + +export type CloudflareResizeImageOptions = CloudflareImageOptions & { + signal?: AbortSignal; + /** + * Bypass the check to see if the image can be resized. + * This is useful for some format that are not supported by @next/og and need to be transformed + */ + bypassSkipCheck?: boolean; + /** + * Accept header of the incoming request, forwarded to the image service for content negotiation. + */ + accept?: string; +}; diff --git a/packages/gitbook/src/lib/images/signatures.test.ts b/packages/gitbook/src/lib/images/signatures.test.ts new file mode 100644 index 000000000..2fec1f9ea --- /dev/null +++ b/packages/gitbook/src/lib/images/signatures.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it, mock } from 'bun:test'; + +// The signing key is read at module load, so it must be mocked before importing the module. +const realEnv = await import('@/lib/env'); +mock.module('@/lib/env', () => ({ + ...realEnv, + GITBOOK_IMAGE_RESIZE_SIGNING_KEY: 'test-signing-key', +})); + +const { + CURRENT_SIGNATURE_VERSION, + generateImageSignature, + isSignatureVersion, + verifyImageSignature, +} = await import('./signatures'); + +const input = { + url: 'https://example.com/image.png', + imagesContextId: 'example.com', +}; + +describe('generateImageSignature', () => { + it('should generate a v3 signature', async () => { + const { signature, version } = await generateImageSignature(input); + expect(version).toBe('3'); + expect(version).toBe(CURRENT_SIGNATURE_VERSION); + expect(signature).toMatch(/^[0-9a-f]{32}$/); + }); + + it('should be deterministic', async () => { + const first = await generateImageSignature(input); + const second = await generateImageSignature(input); + expect(first.signature).toBe(second.signature); + }); + + it('should generate a different signature for a different url', async () => { + const { signature } = await generateImageSignature(input); + const other = await generateImageSignature({ + ...input, + url: 'https://example.com/other.png', + }); + expect(other.signature).not.toBe(signature); + }); + + it('should generate a different signature for a different images context', async () => { + const { signature } = await generateImageSignature(input); + const other = await generateImageSignature({ ...input, imagesContextId: 'other.com' }); + expect(other.signature).not.toBe(signature); + }); +}); + +describe('verifyImageSignature', () => { + it('should verify a freshly generated signature', async () => { + const { signature, version } = await generateImageSignature(input); + expect(await verifyImageSignature(input, { signature, version })).toBe(true); + }); + + it('should reject a tampered signature', async () => { + const { signature, version } = await generateImageSignature(input); + const tampered = `${signature.slice(0, -1)}${signature.endsWith('a') ? 'b' : 'a'}`; + expect(await verifyImageSignature(input, { signature: tampered, version })).toBe(false); + }); + + it('should reject a signature generated for another url', async () => { + const { signature, version } = await generateImageSignature(input); + const verified = await verifyImageSignature( + { ...input, url: 'https://example.com/other.png' }, + { signature, version } + ); + expect(verified).toBe(false); + }); + + it('should reject a signature generated for another images context', async () => { + const { signature, version } = await generateImageSignature(input); + const verified = await verifyImageSignature( + { ...input, imagesContextId: 'other.com' }, + { signature, version } + ); + expect(verified).toBe(false); + }); + + // Older signatures still exist in previously generated and cached content. + it('should still verify a v2 signature', async () => { + expect(await verifyImageSignature(input, { signature: 'd52f183b', version: '2' })).toBe( + true + ); + }); +}); + +describe('isSignatureVersion', () => { + it('should accept all known versions', () => { + expect(['0', '1', '2', '3'].every(isSignatureVersion)).toBe(true); + }); + + it('should reject unknown versions', () => { + expect(isSignatureVersion('4')).toBe(false); + expect(isSignatureVersion('')).toBe(false); + }); +}); diff --git a/packages/gitbook/src/lib/images/signatures.ts b/packages/gitbook/src/lib/images/signatures.ts index a6587d4c2..2c6463324 100644 --- a/packages/gitbook/src/lib/images/signatures.ts +++ b/packages/gitbook/src/lib/images/signatures.ts @@ -10,12 +10,12 @@ import { getLogger } from '@/lib/logger'; * GitBook has supported different version of image signing in the past. To maintain backwards * compatibility, we retain the ability to verify older signatures. */ -export type SignatureVersion = '0' | '1' | '2'; +export type SignatureVersion = '0' | '1' | '2' | '3'; /** * The current version of the signature. */ -export const CURRENT_SIGNATURE_VERSION: SignatureVersion = '2'; +export const CURRENT_SIGNATURE_VERSION: SignatureVersion = '3'; type SignFnInput = { url: string; @@ -34,30 +34,57 @@ export async function verifyImageSignature( const generator = IMAGE_SIGNATURE_FUNCTIONS[version]; const generated = await generator(input); + const matches = safeCompare(generated, signature); + const logger = getLogger().subLogger('imageResizing'); - if (generated !== signature) { + if (!matches) { // We only log if the signature does not match, to avoid logging useless information logger.log( `comparing image signature for "${input.url}" on identifier "${input.imagesContextId}": "${generated}" (expected) === "${signature}" (actual)` ); } - return generated === signature; + return matches; } /** * Generate an image signature. Also returns the version of the image signing algorithm that was used. - * - * This function is sync. If you need to implement an async version of image signing, you'll need to change - * ths signature of this fn and where it's used. */ export async function generateImageSignature(input: SignFnInput): Promise<{ signature: string; version: SignatureVersion; }> { - const result = await generateSignatureV2(input); + const result = await generateSignatureV3(input); return { signature: result, version: CURRENT_SIGNATURE_VERSION }; } +/** + * The signature is truncated to 128 bits: enough to make forgery infeasible, while keeping + * image URLs short as a page can contain many of them. + */ +const SIGNATURE_V3_BYTES = 16; + +/** + * Same inputs as the v2 algorithm, but hashed with SHA-256 instead of FNV-1a. + * FNV-1a is not a cryptographic hash, so a v2 signature is not a sound authentication token. + * + * We use the Web Crypto API rather than `node:crypto` because signatures are verified from the + * middleware, which is bundled for the Cloudflare edge runtime where `node:crypto` is external. + */ +const generateSignatureV3: SignFn = async (input) => { + assert(GITBOOK_IMAGE_RESIZE_SIGNING_KEY, 'GITBOOK_IMAGE_RESIZE_SIGNING_KEY is not set'); + const all = [ + input.url, + input.imagesContextId, // The hostname is used to avoid serving images from other sites on the same domain + // Kept last so a known prefix can never be length-extended into a valid signature. + GITBOOK_IMAGE_RESIZE_SIGNING_KEY, + ] + .filter(Boolean) + .join(':'); + + const hash = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(all)); + return toHex(new Uint8Array(hash, 0, SIGNATURE_V3_BYTES)); +}; + // Reused buffer for FNV-1a hashing in the v2 algorithm const fnv1aUtf8Buffer = new Uint8Array(512); @@ -102,13 +129,15 @@ const generateSignatureV0: SignFn = async (input) => { assert(GITBOOK_IMAGE_RESIZE_SIGNING_KEY, 'GITBOOK_IMAGE_RESIZE_SIGNING_KEY is not set'); const all = [input.url, GITBOOK_IMAGE_RESIZE_SIGNING_KEY].filter(Boolean).join(':'); const hash = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(all)); - - // Convert ArrayBuffer to hex string - const hashArray = Array.from(new Uint8Array(hash)); - const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join(''); - return hashHex; + return toHex(new Uint8Array(hash)); }; +function toHex(bytes: Uint8Array): string { + return Array.from(bytes) + .map((b) => b.toString(16).padStart(2, '0')) + .join(''); +} + /** * A mapping of signature versions to signature functions. */ @@ -116,8 +145,25 @@ const IMAGE_SIGNATURE_FUNCTIONS: Record = { '0': generateSignatureV0, '1': generateSignatureV1, '2': generateSignatureV2, + '3': generateSignatureV3, }; export function isSignatureVersion(input: string): input is SignatureVersion { return Object.keys(IMAGE_SIGNATURE_FUNCTIONS).includes(input); } + +/** + * Compare two signatures in constant time, to avoid leaking a valid signature byte by byte. + * We can't use `node:crypto`'s `timingSafeEqual` as this also runs on the edge runtime. + */ +function safeCompare(expected: string, actual: string): boolean { + if (expected.length !== actual.length) { + return false; + } + + let diff = 0; + for (let i = 0; i < expected.length; i++) { + diff |= expected.charCodeAt(i) ^ actual.charCodeAt(i); + } + return diff === 0; +} diff --git a/packages/gitbook/src/lib/images/utils.ts b/packages/gitbook/src/lib/images/utils.ts index 23927f737..50518ad6d 100644 --- a/packages/gitbook/src/lib/images/utils.ts +++ b/packages/gitbook/src/lib/images/utils.ts @@ -15,3 +15,27 @@ export async function getResizedImageURL( return await getURL(options); } + +/** + * Header stamped on every image request the service refuses to serve, so a rejection can be + * told apart from a source error without parsing the body. + */ +export const IMAGE_REJECT_REASON_HEADER = 'x-gitbook-reject-reason'; + +/** + * Reason an image request was rejected, reported in {@link IMAGE_REJECT_REASON_HEADER}. + */ +export enum ImageRejectReason { + /** The request path is missing the resizing options or the source URL. */ + InvalidRequest = 'invalid-request', + /** The signature is missing or does not match the source URL. */ + InvalidSignature = 'invalid-signature', + /** The source URL is not a public http(s) host. */ + UnsafeSourceURL = 'unsafe-source-url', + /** The source responded with an error status. */ + UpstreamError = 'upstream-error', + /** The response is not something we are willing to serve as an image. */ + UnsupportedContentType = 'unsupported-content-type', + /** The request failed while being processed. */ + InternalError = 'internal-error', +} diff --git a/packages/gitbook/src/routes/image.ts b/packages/gitbook/src/routes/image.ts index 31aff6d53..78c08f2eb 100644 --- a/packages/gitbook/src/routes/image.ts +++ b/packages/gitbook/src/routes/image.ts @@ -2,7 +2,7 @@ import { NextResponse } from 'next/server'; import { CURRENT_SIGNATURE_VERSION, - type CloudflareImageOptions, + type CloudflareResizeImageOptions, type SignatureVersion, SizableImageAction, checkIsSizableImageURL, @@ -10,21 +10,9 @@ import { parseImageAPIURL, resizeImage, verifyImageSignature, + IMAGE_REJECT_REASON_HEADER, + ImageRejectReason, } from '@/lib/images'; -import type { CloudflareResizeImageOptions } from '@/lib/images/resizer'; - -const FORMATS = [ - { - format: 'avif' as const, - regexp: /image\/avif/, - maxAllowedEdge: 1600, - }, - { - format: 'webp' as const, - regexp: /image\/webp/, - maxAllowedEdge: 1920, - }, -]; /** * Resize an image using the Cloudflare Image API. @@ -87,12 +75,12 @@ export async function serveResizedImage( const defaultFormat = getOriginalFormatFromURL(url); - // Cloudflare-specific options are in the cf object. - const options: CloudflareImageOptions = { + const options: CloudflareResizeImageOptions = { fit: 'scale-down', - // For GIF, we will use webp as default format for resizing. - format: defaultFormat === 'gif' ? 'webp' : defaultFormat, + // Let the image service negotiate the output format from the accept header. + format: 'auto', quality: 100, + accept: request.headers.get('accept') ?? undefined, }; const width = requestURL.searchParams.get('width'); @@ -105,8 +93,6 @@ export async function serveResizedImage( options.height = Number(height); } - const longestEdgeValue = Math.max(options.width || 0, options.height || 0); - const dpr = requestURL.searchParams.get('dpr'); if (dpr) { options.dpr = Number(dpr); @@ -117,24 +103,6 @@ export async function serveResizedImage( options.quality = Number(quality); } - // Check the Accept header to handle content negotiation - const accept = request.headers.get('accept'); - - // We test if we can use AVIF based on the accept header and constraints from Cloudflare - // @see https://developers.cloudflare.com/images/transform-images/#limits-per-format - if (accept) { - for (const entry of FORMATS) { - if (entry.regexp.test(accept) && longestEdgeValue <= entry.maxAllowedEdge) { - const wantedDpr = options.dpr ?? 1; - const dpr = chooseDPR(longestEdgeValue, entry.maxAllowedEdge, wantedDpr); - if (dpr === wantedDpr) { - options.format = entry.format; - break; - } - } - } - } - return resizeImageWithFallback( url, options, @@ -155,8 +123,37 @@ async function resizeImageWithFallback( try { const response = await resizeImage(url, options); if (!response.ok) { + const rejectReason = response.headers.get(IMAGE_REJECT_REASON_HEADER); + if (rejectReason) { + switch (rejectReason) { + case ImageRejectReason.InvalidRequest: + return new Response('Invalid request', { status: 400 }); + case ImageRejectReason.InvalidSignature: + return new Response('Invalid signature', { status: 400 }); + case ImageRejectReason.UnsafeSourceURL: + return new Response('Unsafe source URL', { status: 400 }); + case ImageRejectReason.UpstreamError: + // this one can happen for a lot of reasons, so we fallback to a redirect to the original image + // It sometimes happen when upstream block fetch from our server + throw new Error('Upstream error, falling back to a redirect'); + case ImageRejectReason.UnsupportedContentType: + throw new Error('Unsupported content type, falling back to a redirect'); + case ImageRejectReason.InternalError: + throw new Error('Internal error, falling back to a redirect'); + default: + throw new Error( + `Unknown reject reason "${rejectReason}", falling back to a redirect` + ); + } + } throw new Error(`Failed to resize image, received status code ${response.status}`); } + + // The output format is negotiated from the accept header we forwarded. + if (!response.headers.get('vary')?.toLowerCase().includes('accept')) { + response.headers.append('vary', 'Accept'); + } + return response; } catch (error) { if (formatFallback && options.format !== formatFallback) { @@ -167,12 +164,35 @@ async function resizeImageWithFallback( ); } + // Never bounce the visitor to a GitBook-hosted URL: it would expose our internal + // asset URLs and hide a failure that is ours to fix. + if (isGitBookHostedURL(url)) { + console.warn('Error while resizing GitBook-hosted image', error); + return new Response('Failed to resize image', { status: 502 }); + } + // Redirect to the original image if resizing fails console.warn('Error while resizing image, redirecting to original', error); return NextResponse.redirect(url, 302); } } +const GITBOOK_HOSTED_DOMAINS = ['gitbook.com', 'gitbook.io']; + +/** + * Check if a URL is hosted on a GitBook domain. + */ +function isGitBookHostedURL(url: string): boolean { + if (!URL.canParse(url)) { + return false; + } + + const { hostname } = new URL(url); + return GITBOOK_HOSTED_DOMAINS.some( + (domain) => hostname === domain || hostname.endsWith(`.${domain}`) + ); +} + /** * Get the original format from URL. */ @@ -188,16 +208,6 @@ function getOriginalFormatFromURL(url: string) { return 'jpeg'; } -/** - * Choose the DPR allowed to resize an image on Cloudflare. - * @see https://developers.cloudflare.com/images/transform-images/#limits-per-format - */ -function chooseDPR(longestEdgeValue: number, maxAllowedEdge: number, wantedDpr: number): number { - const maxDprBySize = Math.floor(maxAllowedEdge / longestEdgeValue); - // Ensure that the DPR is within the allowed range - return Math.max(1, Math.min(maxDprBySize, wantedDpr)); -} - /** * Parse the image signature version from a query param. Returns null if the version is invalid. */