From 4f4a0e7c94ae9294ae9d1312118d291df8bf4ced Mon Sep 17 00:00:00 2001 From: tngus6007 Date: Mon, 11 May 2026 18:42:56 +0900 Subject: [PATCH 1/2] fix(api): harden webhook and store metadata fetches --- .../_backend/private/website_preview.ts | 36 +-------- .../_backend/public/app/store_metadata.ts | 8 +- supabase/functions/_backend/utils/response.ts | 39 ++++++++++ supabase/functions/_backend/utils/webhook.ts | 11 ++- tests/store-metadata.unit.test.ts | 75 +++++++++++++++++++ tests/webhook-delivery-security.unit.test.ts | 33 ++++++++ 6 files changed, 166 insertions(+), 36 deletions(-) create mode 100644 supabase/functions/_backend/utils/response.ts create mode 100644 tests/store-metadata.unit.test.ts diff --git a/supabase/functions/_backend/private/website_preview.ts b/supabase/functions/_backend/private/website_preview.ts index 19e3b62025..b0f90f83ed 100644 --- a/supabase/functions/_backend/private/website_preview.ts +++ b/supabase/functions/_backend/private/website_preview.ts @@ -1,5 +1,6 @@ import type { Context } from 'hono' import { createHono, middlewareAuth, parseBody, quickError, useCors } from '../utils/hono.ts' +import { readResponseBytesWithLimit } from '../utils/response.ts' import { version } from '../utils/version.ts' import { getWebhookUrlValidationError } from '../utils/webhook.ts' @@ -220,40 +221,9 @@ async function getPublicHostnameValidationError(c: Context, urlString: string) { } async function readResponseTextWithLimit(response: Response, limit: number) { - const contentLength = Number.parseInt(response.headers.get('content-length') ?? '', 10) - if (Number.isFinite(contentLength) && contentLength > limit) + const bytes = await readResponseBytesWithLimit(response, limit) + if (!bytes) return null - - if (!response.body) - return await response.text() - - const reader = response.body.getReader() - const chunks: Uint8Array[] = [] - let total = 0 - - while (true) { - const { done, value } = await reader.read() - if (done) - break - - if (!value) - continue - - total += value.byteLength - if (total > limit) { - await reader.cancel() - return null - } - chunks.push(value) - } - - const bytes = new Uint8Array(total) - let offset = 0 - for (const chunk of chunks) { - bytes.set(chunk, offset) - offset += chunk.byteLength - } - return new TextDecoder().decode(bytes) } diff --git a/supabase/functions/_backend/public/app/store_metadata.ts b/supabase/functions/_backend/public/app/store_metadata.ts index 9cf4f24d7d..ac89f69a52 100644 --- a/supabase/functions/_backend/public/app/store_metadata.ts +++ b/supabase/functions/_backend/public/app/store_metadata.ts @@ -1,11 +1,14 @@ import type { Context } from 'hono' import type { MiddlewareKeyVariables } from '../../utils/hono.ts' import { quickError } from '../../utils/hono.ts' +import { readResponseBytesWithLimit } from '../../utils/response.ts' export interface FetchStoreMetadataBody { url?: string } +const MAX_ICON_BYTES = 512 * 1024 + interface AppleLookupResult { trackName?: string artworkUrl512?: string @@ -51,7 +54,10 @@ async function fetchIconDataUrl(iconUrl: string | null) { return null const contentType = response.headers.get('content-type')?.split(';')[0]?.trim() || 'image/png' - const bytes = new Uint8Array(await response.arrayBuffer()) + const bytes = await readResponseBytesWithLimit(response, MAX_ICON_BYTES) + if (!bytes) + return null + return `data:${contentType};base64,${uint8ArrayToBase64(bytes)}` } catch { diff --git a/supabase/functions/_backend/utils/response.ts b/supabase/functions/_backend/utils/response.ts new file mode 100644 index 0000000000..44f9602b68 --- /dev/null +++ b/supabase/functions/_backend/utils/response.ts @@ -0,0 +1,39 @@ +export async function readResponseBytesWithLimit(response: Response, limit: number) { + const contentLength = Number.parseInt(response.headers.get('content-length') ?? '', 10) + if (Number.isFinite(contentLength) && contentLength > limit) + return null + + if (!response.body) { + const buffer = await response.arrayBuffer() + return buffer.byteLength > limit ? null : new Uint8Array(buffer) + } + + const reader = response.body.getReader() + const chunks: Uint8Array[] = [] + let total = 0 + + while (true) { + const { done, value } = await reader.read() + if (done) + break + + if (!value) + continue + + total += value.byteLength + if (total > limit) { + await reader.cancel() + return null + } + chunks.push(value) + } + + const bytes = new Uint8Array(total) + let offset = 0 + for (const chunk of chunks) { + bytes.set(chunk, offset) + offset += chunk.byteLength + } + + return bytes +} diff --git a/supabase/functions/_backend/utils/webhook.ts b/supabase/functions/_backend/utils/webhook.ts index 96fdb85a03..d773adbe0b 100644 --- a/supabase/functions/_backend/utils/webhook.ts +++ b/supabase/functions/_backend/utils/webhook.ts @@ -52,6 +52,10 @@ function allowLocalWebhookUrls(c: Context): boolean { return getEnv(c, 'CAPGO_ALLOW_LOCAL_WEBHOOK_URLS') === 'true' } +function isLocalWebhookProtocol(protocol: string): boolean { + return protocol === 'http:' || protocol === 'https:' +} + function normalizeHostname(hostname: string): string { return hostname.replace(/\.$/, '').toLowerCase() } @@ -73,8 +77,11 @@ export function getWebhookUrlValidationError(c: Context, urlString: string): str return 'Webhook URL is invalid' } - if (allowLocalWebhookUrls(c)) - return null + if (allowLocalWebhookUrls(c)) { + return isLocalWebhookProtocol(url.protocol) + ? null + : 'Webhook URL must use HTTP or HTTPS' + } // We intentionally stop at syntactic/public-host checks: webhook delivery runs // entirely from serverless infrastructure, so private/internal addresses are not diff --git a/tests/store-metadata.unit.test.ts b/tests/store-metadata.unit.test.ts new file mode 100644 index 0000000000..0f60684aae --- /dev/null +++ b/tests/store-metadata.unit.test.ts @@ -0,0 +1,75 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { fetchStoreMetadata } from '../supabase/functions/_backend/public/app/store_metadata.ts' + +function createContext() { + return { + json: (body: unknown) => Response.json(body), + } as any +} + +function storeHtml(iconUrl: string) { + return ` + + + + + + + + ` +} + +function mockStoreMetadataFetch(iconResponse: Response) { + const iconUrl = 'https://play-lh.googleusercontent.com/icon.png' + const fetchMock = vi.fn(async (url: string) => { + if (url.startsWith('https://play.google.com/')) { + return new Response(storeHtml(iconUrl), { + headers: { 'content-type': 'text/html' }, + }) + } + + return iconResponse + }) + vi.stubGlobal('fetch', fetchMock) + return fetchMock +} + +async function fetchPlayStoreMetadata() { + const response = await fetchStoreMetadata(createContext(), { + url: 'https://play.google.com/store/apps/details?id=app.capgo.test', + }) + return response.json() as Promise<{ icon_data_url: string | null }> +} + +describe('store metadata icon fetching', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('does not buffer oversized icon responses', async () => { + const fetchMock = mockStoreMetadataFetch( + new Response('too large', { + headers: { + 'content-length': String(512 * 1024 + 1), + 'content-type': 'image/png', + }, + }), + ) + const body = await fetchPlayStoreMetadata() + + expect(body.icon_data_url).toBeNull() + expect(fetchMock).toHaveBeenCalledTimes(2) + }) + + it('keeps small allowlisted icon responses', async () => { + mockStoreMetadataFetch( + new Response(new Uint8Array([1, 2, 3]), { + headers: { 'content-type': 'image/png' }, + }), + ) + const body = await fetchPlayStoreMetadata() + + expect(body.icon_data_url).toBe('data:image/png;base64,AQID') + }) +}) diff --git a/tests/webhook-delivery-security.unit.test.ts b/tests/webhook-delivery-security.unit.test.ts index 4375d8d22d..6fb8f577bc 100644 --- a/tests/webhook-delivery-security.unit.test.ts +++ b/tests/webhook-delivery-security.unit.test.ts @@ -22,6 +22,17 @@ function createContext() { } as any } +function enableLocalWebhookUrls(enabled: boolean) { + mockGetEnv.mockImplementation((_c: unknown, key: string) => + key === 'CAPGO_ALLOW_LOCAL_WEBHOOK_URLS' && enabled ? 'true' : '', + ) +} + +async function validateWebhookUrl(url: string) { + const { getWebhookUrlValidationError } = await import('../supabase/functions/_backend/utils/webhook.ts') + return getWebhookUrlValidationError(createContext(), url) +} + afterEach(() => { vi.restoreAllMocks() vi.unstubAllGlobals() @@ -104,3 +115,25 @@ describe('webhook delivery redirect handling', () => { expect(fetchMock).toHaveBeenCalledOnce() }) }) + +describe('webhook URL validation', () => { + it('allows HTTP and HTTPS webhooks when local webhook URLs are enabled', async () => { + enableLocalWebhookUrls(true) + + await expect(validateWebhookUrl('http://localhost:3000/webhook')).resolves.toBeNull() + await expect(validateWebhookUrl('https://localhost/webhook')).resolves.toBeNull() + }) + + it('rejects unsupported URL schemes even when local webhook URLs are enabled', async () => { + enableLocalWebhookUrls(true) + + await expect(validateWebhookUrl('ftp://localhost/webhook')).resolves.toBe('Webhook URL must use HTTP or HTTPS') + await expect(validateWebhookUrl('file:///tmp/webhook')).resolves.toBe('Webhook URL must use HTTP or HTTPS') + }) + + it('continues to reject HTTP URLs when local webhook URLs are disabled', async () => { + enableLocalWebhookUrls(false) + + await expect(validateWebhookUrl('http://example.com/webhook')).resolves.toBe('Webhook URL must use HTTPS') + }) +}) From 1c4231f733f30f9ede50388b391f863c33397cce Mon Sep 17 00:00:00 2001 From: tngus6007 Date: Tue, 12 May 2026 01:51:27 +0900 Subject: [PATCH 2/2] fix(api): avoid logging blocked webhook URLs --- supabase/functions/_backend/utils/webhook.ts | 22 ++++++++- tests/webhook-delivery-security.unit.test.ts | 52 ++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/supabase/functions/_backend/utils/webhook.ts b/supabase/functions/_backend/utils/webhook.ts index d773adbe0b..80e7817437 100644 --- a/supabase/functions/_backend/utils/webhook.ts +++ b/supabase/functions/_backend/utils/webhook.ts @@ -68,6 +68,26 @@ function isIpLiteral(hostname: string): boolean { return IPV4_REGEX.test(hostname) || hostname.includes(':') } +function getWebhookUrlLogInfo(urlString: string) { + try { + const url = new URL(urlString) + return { + protocol: url.protocol, + hasHostname: url.hostname !== '', + hostnameLength: url.hostname.length, + pathSegmentCount: url.pathname.split('/').filter(Boolean).length, + hasQuery: url.search !== '', + hasCredentials: url.username !== '' || url.password !== '', + } + } + catch { + return { + invalid: true, + length: urlString.length, + } + } +} + export function getWebhookUrlValidationError(c: Context, urlString: string): string | null { let url: URL try { @@ -229,7 +249,7 @@ export async function deliverWebhook( requestId: c.get('requestId'), message: 'Webhook delivery blocked by URL validation', deliveryId, - url, + urlInfo: getWebhookUrlLogInfo(url), error: urlValidationError, duration, }) diff --git a/tests/webhook-delivery-security.unit.test.ts b/tests/webhook-delivery-security.unit.test.ts index 6fb8f577bc..a40ac569fe 100644 --- a/tests/webhook-delivery-security.unit.test.ts +++ b/tests/webhook-delivery-security.unit.test.ts @@ -114,6 +114,46 @@ describe('webhook delivery redirect handling', () => { expect(result.status).toBe(302) expect(fetchMock).toHaveBeenCalledOnce() }) + + it('does not log raw blocked webhook URLs with query secrets', async () => { + mockGetEnv.mockReturnValue('') + + const { deliverWebhook } = await import('../supabase/functions/_backend/utils/webhook.ts') + const result = await deliverWebhook( + createContext(), + 'delivery-3', + 'http://localhost/webhook?token=secret-token', + { + event: 'app_versions.INSERT', + event_id: 'event-3', + timestamp: new Date().toISOString(), + org_id: 'org-1', + data: { + table: 'app_versions', + operation: 'INSERT', + record_id: 'record-3', + old_record: null, + new_record: null, + changed_fields: null, + }, + }, + 'secret', + ) + + expect(result.success).toBe(false) + expect(mockCloudlogErr).toHaveBeenCalledOnce() + const logged = JSON.stringify(mockCloudlogErr.mock.calls) + expect(logged).not.toContain('secret-token') + expect(logged).not.toContain('http://localhost/webhook') + expect(mockCloudlogErr.mock.calls[0]?.[0]).toMatchObject({ + message: 'Webhook delivery blocked by URL validation', + urlInfo: { + protocol: 'http:', + hasHostname: true, + hasQuery: true, + }, + }) + }) }) describe('webhook URL validation', () => { @@ -136,4 +176,16 @@ describe('webhook URL validation', () => { await expect(validateWebhookUrl('http://example.com/webhook')).resolves.toBe('Webhook URL must use HTTPS') }) + + it('allows public HTTPS URLs when local webhook URLs are enabled', async () => { + enableLocalWebhookUrls(true) + + await expect(validateWebhookUrl('https://example.com/webhook')).resolves.toBeNull() + }) + + it('rejects localhost URLs when local webhook URLs are disabled', async () => { + enableLocalWebhookUrls(false) + + await expect(validateWebhookUrl('https://localhost/webhook')).resolves.toBe('Webhook URL must point to a public host') + }) })