From 20feaa6c91d67cd62ada3b584090534fa74d2f40 Mon Sep 17 00:00:00 2001 From: Jasper Van Date: Tue, 28 Jul 2026 10:03:47 -0400 Subject: [PATCH] fix(image-hosting): bypass auth on custom domains (#532) * fix(image-hosting): bypass auth on custom domains * test(image-hosting): cover traffic compensation --- .../traffic-metering.integration.test.ts | 69 ++++++++ server/middleware/error-handler.test.ts | 23 ++- server/middleware/error-handler.ts | 20 +++ server/middleware/image-hosting-domain.ts | 148 ++++++++++++------ workers/bootstrap-image-domain.cf-test.ts | 66 ++++++++ workers/bootstrap.ts | 37 +++++ 6 files changed, 313 insertions(+), 50 deletions(-) create mode 100644 workers/bootstrap-image-domain.cf-test.ts diff --git a/server/http/store/traffic-metering.integration.test.ts b/server/http/store/traffic-metering.integration.test.ts index dfe2049b..a947e77d 100644 --- a/server/http/store/traffic-metering.integration.test.ts +++ b/server/http/store/traffic-metering.integration.test.ts @@ -497,6 +497,75 @@ describe('public redirect cloud traffic reporting', () => { ]) }) + it('denies custom-domain images and refunds traffic when Cloud rejects usage', async () => { + const { app, db } = await createTestApp() + await seedTrafficBinding(db) + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue(makeCloudResponse({ error: { code: 'insufficient_credits' } }, 402)), + ) + await authedHeaders(app) + await insertStorage(db) + const orgId = await getOrgId(db) + await insertImage(db, orgId, 'ih-cloud-domain-blocked', 'ih_clouddomainblocked', 'blog/domain-blocked.png') + await insertImageConfig(db, orgId, 'img-blocked.example.com') + await setTrafficQuota(db, orgId) + + const res = await app.request('https://img-blocked.example.com/blog/domain-blocked.png', { + headers: { host: 'img-blocked.example.com' }, + redirect: 'manual', + }) + + expect(res.status).toBe(402) + await expect(res.json()).resolves.toMatchObject({ + error: { + details: [{ reason: 'INSUFFICIENT_CREDITS', metadata: { resource: 'storage_egress' } }], + }, + }) + const quotaRows = await db.all<{ trafficUsed: number }>( + sql`SELECT traffic_used AS trafficUsed FROM org_quotas WHERE org_id = ${orgId}`, + ) + expect(quotaRows).toEqual([{ trafficUsed: 25 }]) + await expect(trafficReports(db)).resolves.toMatchObject([{ status: 'blocked', error: 'insufficient_credits' }]) + const failures = await db.all<{ reason: string }>(sql` + SELECT json_extract(metadata, '$.reason') AS reason + FROM audit_events + WHERE action = 'download_failed' AND target_id = 'ih-cloud-domain-blocked' + `) + expect(failures).toEqual([{ reason: 'insufficient_credits' }]) + }) + + it('reverses custom-domain traffic when issuing the download fails', async () => { + const { app, db, deps } = await createTestApp() + await seedTrafficBinding(db) + vi.stubGlobal('fetch', vi.fn().mockImplementation(acceptedUsageResponse)) + await authedHeaders(app) + await insertStorage(db) + const orgId = await getOrgId(db) + await insertImage(db, orgId, 'ih-cloud-domain-confirm-fail', 'ih_clouddomainconfirmfail', 'blog/confirm-fail.png') + await insertImageConfig(db, orgId, 'img-confirm-fail.example.com') + await setTrafficQuota(db, orgId) + vi.spyOn(deps.cloudTrafficReports, 'markIssued').mockRejectedValueOnce(new Error('confirm failed')) + + const res = await app.request('https://img-confirm-fail.example.com/blog/confirm-fail.png', { + headers: { host: 'img-confirm-fail.example.com' }, + redirect: 'manual', + }) + + expect(res.status).toBe(500) + const quotaRows = await db.all<{ trafficUsed: number }>( + sql`SELECT traffic_used AS trafficUsed FROM org_quotas WHERE org_id = ${orgId}`, + ) + expect(quotaRows).toEqual([{ trafficUsed: 25 }]) + await expect(trafficReports(db)).resolves.toMatchObject([{ status: 'reversed' }]) + const failures = await db.all<{ reason: string }>(sql` + SELECT json_extract(metadata, '$.reason') AS reason + FROM audit_events + WHERE action = 'download_failed' AND target_id = 'ih-cloud-domain-confirm-fail' + `) + expect(failures).toEqual([{ reason: 'internal' }]) + }) + it('still redirects custom-domain images when access-count recording fails after local traffic queue', async () => { const { app, db } = await createTestApp() await seedTrafficBinding(db) diff --git a/server/middleware/error-handler.test.ts b/server/middleware/error-handler.test.ts index 4029ec6e..84816a53 100644 --- a/server/middleware/error-handler.test.ts +++ b/server/middleware/error-handler.test.ts @@ -2,7 +2,7 @@ import type { Context } from 'hono' import { Hono } from 'hono' import { describe, expect, it } from 'vitest' import { AppError, NameConflictError } from '../usecases/ports' -import { isHandledError, jsonError } from './error-handler' +import { isHandledError, jsonError, standaloneJsonError } from './error-handler' import type { Env } from './platform' // Build a real Context so jsonError's c.json / c.set behave as in production. @@ -58,3 +58,24 @@ describe('isHandledError', () => { expect(isHandledError(null)).toBe(false) }) }) + +describe('standaloneJsonError', () => { + it('renders an AppError without a Hono context', async () => { + const res = standaloneJsonError( + new AppError(429, 'Try later', { reason: 'RATE_LIMITED', headers: { 'Retry-After': '5' } }), + ) + expect(res.status).toBe(429) + expect(res.headers.get('retry-after')).toBe('5') + expect(await res.json()).toMatchObject({ + error: { message: 'Try later', status: 'RESOURCE_EXHAUSTED' }, + }) + }) + + it('does not expose unexpected errors', async () => { + const res = standaloneJsonError(new Error('database password leaked')) + expect(res.status).toBe(500) + expect(await res.json()).toMatchObject({ + error: { message: 'Internal Server Error' }, + }) + }) +}) diff --git a/server/middleware/error-handler.ts b/server/middleware/error-handler.ts index 4ceb2025..00e1e9f7 100644 --- a/server/middleware/error-handler.ts +++ b/server/middleware/error-handler.ts @@ -39,6 +39,26 @@ export function jsonError(c: Context, err: unknown): Response { return c.json(buildErrorBody(500, 'Internal Server Error', { reason: 'INTERNAL' }), 500) } +// Render the same JSON error outside Hono's request pipeline. Cloudflare's +// image-domain fast path runs before the full app (and Better Auth) exists. +export function standaloneJsonError(err: unknown): Response { + if (err instanceof AppError) { + return Response.json( + buildErrorBody(err.httpStatus, err.message, { + reason: err.meta.reason, + status: err.meta.canonicalStatus, + metadata: err.meta.metadata, + }), + { status: err.httpStatus, headers: err.meta.headers }, + ) + } + + const mapped = mapDomainError(err) + if (mapped) return Response.json(mapped.json, { status: mapped.status }) + + return Response.json(buildErrorBody(500, 'Internal Server Error', { reason: 'INTERNAL' }), { status: 500 }) +} + // True when `jsonError` would translate `err` into a specific (non-500) result. // Lets `app.onError` log only genuinely unhandled errors as `http.unhandled_error`. export function isHandledError(err: unknown): boolean { diff --git a/server/middleware/image-hosting-domain.ts b/server/middleware/image-hosting-domain.ts index 4428589d..d18dd57e 100644 --- a/server/middleware/image-hosting-domain.ts +++ b/server/middleware/image-hosting-domain.ts @@ -1,11 +1,17 @@ import type { Context, Next } from 'hono' +import { ZPAN_CLOUD_URL_DEFAULT } from '../../shared/constants' import { imageHostingNotFound } from '../http/image-hosting-not-found' import { PRESIGN_TTL_SECS } from '../http/share-utils' -import { reportTrafficForDownload } from '../http/store/traffic-metering' import type { Env } from '../middleware/platform' +import type { Platform } from '../platform/interface' +import type { Deps } from '../usecases/deps' import { cacheVerifiedImageDomain, resolveCachedImageDomain } from '../usecases/image-hosting/domain-cache' -import { forbidden, notFound, quotaExceeded, storageNotFound } from '../usecases/ports' -import { confirmDownloadTraffic, reverseDownloadTraffic } from '../usecases/store/traffic-metering' +import { forbidden, insufficientCredits, notFound, quotaExceeded, storageNotFound } from '../usecases/ports' +import { + confirmDownloadTraffic, + reportDownloadEgress, + reverseDownloadTraffic, +} from '../usecases/store/traffic-metering' import { createTrafficEventId, recordDownloadFailure, recordDownloadIssued } from '../usecases/transfer-activity' function stripPort(host: string): string { @@ -40,37 +46,44 @@ function checkReferer(refererAllowlist: string[], refererHeader: string | null): } } -async function handleImageByPath(c: Context, orgId: string, virtualPath: string): Promise { - const resolved = await c.get('deps').imageHosting.resolveActiveByOrgPath(orgId, virtualPath) - if (!resolved) return imageHostingNotFound(c.req.raw) +async function handleImageByPath( + request: Request, + deps: Deps, + platform: Platform, + orgId: string, + virtualPath: string, +): Promise { + const resolved = await deps.imageHosting.resolveActiveByOrgPath(orgId, virtualPath) + if (!resolved) return imageHostingNotFound(request) const { image, refererAllowlist } = resolved - const refererHeader = c.req.header('Referer') ?? null + const refererHeader = request.headers.get('Referer') if (!checkReferer(refererAllowlist, refererHeader)) { throw forbidden('forbidden referer') } - const storage = await c.get('deps').storages.get(image.storageId) + const storage = await deps.storages.get(image.storageId) if (!storage) throw storageNotFound('Storage not found') - const trafficAllowed = await c.get('deps').quota.consumeTrafficIfQuotaAllows(image.orgId, image.size) + const trafficAllowed = await deps.quota.consumeTrafficIfQuotaAllows(image.orgId, image.size) if (!trafficAllowed) { - await recordImageDownloadFailure(c, image, 'quota_exceeded') + await recordImageDownloadFailure(deps, image, 'quota_exceeded') throw quotaExceeded('Traffic quota exceeded') } let url: string try { - url = await c.get('deps').s3.presignInline(storage, image.storageKey, image.mime, PRESIGN_TTL_SECS) + url = await deps.s3.presignInline(storage, image.storageKey, image.mime, PRESIGN_TTL_SECS) } catch (e) { - await c.get('deps').quota.refundTraffic(image.orgId, image.size) - await recordImageDownloadFailure(c, image, 'presign_failed') + await deps.quota.refundTraffic(image.orgId, image.size) + await recordImageDownloadFailure(deps, image, 'presign_failed') throw e } const trafficEventId = createTrafficEventId() - const trafficReportError = await reportTrafficForDownload(c, { + const trafficOutcome = await reportDownloadEgress(deps, { + cloudBaseUrl: platform.getEnv('ZPAN_CLOUD_URL') ?? ZPAN_CLOUD_URL_DEFAULT, orgId: image.orgId, bytes: image.size, storage, @@ -78,29 +91,29 @@ async function handleImageByPath(c: Context, orgId: string, virtualPath: st sourceId: image.id, eventId: trafficEventId, }) - if (trafficReportError) { - await recordImageDownloadFailure(c, image, 'insufficient_credits') - return trafficReportError + if (!trafficOutcome.ok) { + await recordImageDownloadFailure(deps, image, 'insufficient_credits') + throw insufficientCredits('Insufficient credits', { metadata: { resource: 'storage_egress' } }) } try { - await confirmDownloadTraffic(c.get('deps'), { eventId: trafficEventId }) + await confirmDownloadTraffic(deps, { eventId: trafficEventId }) } catch (error) { - await reverseDownloadTraffic(c.get('deps'), { + await reverseDownloadTraffic(deps, { orgId: image.orgId, bytes: image.size, eventId: trafficEventId, }) - await recordImageDownloadFailure(c, image, 'internal') + await recordImageDownloadFailure(deps, image, 'internal') throw error } try { - await c.get('deps').imageHosting.incrementAccessCount(image.id) + await deps.imageHosting.incrementAccessCount(image.id) } catch (error) { console.error('[image-hosting-domain] incrementAccessCount failed:', error) } await recordDownloadIssued( - c.get('deps'), + deps, { userId: null, actorType: 'anonymous', actorRef: null }, 'image_hosting_download', { @@ -114,18 +127,22 @@ async function handleImageByPath(c: Context, orgId: string, virtualPath: st }, trafficEventId, ) - const res = c.redirect(url, 302) - res.headers.set('Cache-Control', 'no-store') - return res + return new Response(null, { + status: 302, + headers: { + 'Cache-Control': 'no-store', + Location: url, + }, + }) } function recordImageDownloadFailure( - c: Context, + deps: Deps, image: { id: string; orgId: string; path: string; size: number; storageId: string }, reason: string, ): Promise { return recordDownloadFailure( - c.get('deps'), + deps, { userId: null, actorType: 'anonymous', actorRef: null }, { orgId: image.orgId, @@ -140,31 +157,40 @@ function recordImageDownloadFailure( ) } -// biome-ignore lint/suspicious/noConfusingVoidType: Next returns void; union with Response is intentional -export async function imageHostingDomain(c: Context, next: Next): Promise { - if (isApplicationPath(c.req.path)) return next() +export interface ImageHostingDomainRequestOptions { + request: Request + deps: Deps + platform: Platform + appHosts: string[] + webDavMountPath: string +} - const rawHost = c.req.header('host') - if (!rawHost) return next() +export async function handleImageHostingDomainRequest({ + request, + deps, + platform, + appHosts, + webDavMountPath, +}: ImageHostingDomainRequestOptions): Promise { + const path = requestPath(request) + if (isApplicationPath(path)) return null + const rawHost = request.headers.get('host') ?? new URL(request.url).host const host = normalizeHost(rawHost) - if (!host) return next() + if (!host || webDavMountPath === '') return null - if (c.get('webDavMountPath') === '') return next() - - const appHosts = getAppHostCandidates(c) if ( appHosts.some((candidate) => host === candidate || (candidate === 'workers.dev' && host.endsWith('.workers.dev'))) ) { - return next() + return null } const verificationPrefix = '/.well-known/zpan-domain-verification/' - if (c.req.method === 'GET' && c.req.path.startsWith(verificationPrefix)) { - const token = c.req.path.slice(verificationPrefix.length) + if (request.method === 'GET' && path.startsWith(verificationPrefix)) { + const token = path.slice(verificationPrefix.length) const [row, provider] = await Promise.all([ - c.get('deps').imageHostingConfigs.getByDomain(host), - c.get('deps').imageDomains.getConfig(), + deps.imageHostingConfigs.getByDomain(host), + deps.imageDomains.getConfig(), ]) if ( row?.customDomain && @@ -176,27 +202,51 @@ export async function imageHostingDomain(c: Context, next: Next): Promise, next: Next): Promise { + const response = await handleImageHostingDomainRequest({ + request: c.req.raw, + deps: c.get('deps'), + platform: c.get('platform'), + appHosts: getAppHostCandidates(c), + webDavMountPath: c.get('webDavMountPath'), + }) + return response ?? next() } function isApplicationPath(path: string): boolean { return path === '/api' || path.startsWith('/api/') || path === '/dav' || path.startsWith('/dav/') } + +function requestPath(request: Request): string { + const path = new URL(request.url).pathname + try { + return decodeURI(path) + } catch { + return path + } +} diff --git a/workers/bootstrap-image-domain.cf-test.ts b/workers/bootstrap-image-domain.cf-test.ts new file mode 100644 index 00000000..e740c3aa --- /dev/null +++ b/workers/bootstrap-image-domain.cf-test.ts @@ -0,0 +1,66 @@ +import { env } from 'cloudflare:workers' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const createAuthMock = vi.hoisted(() => + vi.fn(() => { + throw new Error('Better Auth must not initialize for image-domain requests') + }), +) + +vi.mock('../server/auth', async () => ({ + ...(await vi.importActual('../server/auth')), + createAuth: createAuthMock, +})) + +import worker, { isImageDomainFastPathRequest } from './bootstrap' + +const testEnv = { + ...env, + BETTER_AUTH_SECRET: env.BETTER_AUTH_SECRET || 'ci-test-secret-that-is-at-least-32-chars', + BETTER_AUTH_URL: 'https://drive.fast-path.test', +} + +describe('[CF] image-domain Worker fast path', () => { + beforeEach(() => { + createAuthMock.mockClear() + }) + + it('recognizes only rewritten custom-domain requests', () => { + expect(isImageDomainFastPathRequest(new Request('https://img.fast-path.test/ih/folder/image.png'), testEnv)).toBe( + true, + ) + expect(isImageDomainFastPathRequest(new Request('https://drive.fast-path.test/ih/folder/image.png'), testEnv)).toBe( + false, + ) + expect(isImageDomainFastPathRequest(new Request('https://preview.workers.dev/ih/image.png'), testEnv)).toBe(false) + expect(isImageDomainFastPathRequest(new Request('https://img.fast-path.test/api/health'), testEnv)).toBe(false) + }) + + it('serves repeated custom-domain requests without initializing Better Auth', async () => { + const suffix = Date.now().toString(36) + const orgId = `fast-path-org-${suffix}` + const domain = `img-${suffix}.fast-path.test` + const now = Date.now() + + await env.DB.prepare( + `INSERT INTO organization (id, name, slug, created_at, updated_at) + VALUES (?, 'Fast Path', ?, ?, ?)`, + ) + .bind(orgId, orgId, now, now) + .run() + await env.DB.prepare( + `INSERT INTO image_hosting_configs + (org_id, custom_domain, domain_status, domain_verified_at, created_at, updated_at) + VALUES (?, ?, 'verified', ?, ?, ?)`, + ) + .bind(orgId, domain, now, now, now) + .run() + + for (let attempt = 0; attempt < 3; attempt += 1) { + const response = await worker.fetch(new Request(`https://${domain}/ih/missing.png`), testEnv) + expect(response.status).toBe(404) + expect(await response.text()).toContain('Image not found') + } + expect(createAuthMock).not.toHaveBeenCalled() + }) +}) diff --git a/workers/bootstrap.ts b/workers/bootstrap.ts index ffa52ba8..3db62a86 100644 --- a/workers/bootstrap.ts +++ b/workers/bootstrap.ts @@ -8,6 +8,8 @@ import type { Auth } from '../server/auth' import { createAuth } from '../server/auth' import { createDeps } from '../server/composition' import { isPotentialWebDavPublicRequest } from '../server/domain/webdav-public-url' +import { isHandledError, standaloneJsonError } from '../server/middleware/error-handler' +import { handleImageHostingDomainRequest } from '../server/middleware/image-hosting-domain' import { createCloudflarePlatform } from '../server/platform/cloudflare' import { platformContext } from '../server/platform/context' import type { Deps } from '../server/usecases/deps' @@ -105,6 +107,8 @@ export default { throw new Error('BETTER_AUTH_SECRET is not configured for this deployment.') } const runtime = runtimeFor(env) + const imageDomainResponse = await handleImageDomainBeforeAuth(request, env, runtime) + if (imageDomainResponse) return imageDomainResponse const edgeCached = await matchConfigzResponseCache(request, runtime.cache) if (edgeCached) return edgeCached const app = await appForRequest(runtime, request, env) @@ -137,6 +141,39 @@ export default { }, } +async function handleImageDomainBeforeAuth( + request: Request, + env: Env, + runtime: WorkerRuntime, +): Promise { + if (!isImageDomainFastPathRequest(request, env)) return null + + try { + return await platformContext.run(runtime.platform, () => + handleImageHostingDomainRequest({ + request, + deps: runtime.deps, + platform: runtime.platform, + appHosts: [new URL(env.BETTER_AUTH_URL!).hostname.toLowerCase(), 'workers.dev'], + webDavMountPath: '/dav', + }), + ) + } catch (error) { + if (!isHandledError(error)) console.error(`image_domain.unhandled_error code=${String(error)}`) + return standaloneJsonError(error) + } +} + +export function isImageDomainFastPathRequest(request: Request, env: Pick): boolean { + if (!env.BETTER_AUTH_URL) return false + const url = new URL(request.url) + if (url.pathname !== '/ih' && !url.pathname.startsWith('/ih/')) return false + + const host = (request.headers.get('host') ?? url.host).replace(/:\d+$/, '').toLowerCase() + const appHost = new URL(env.BETTER_AUTH_URL).hostname.toLowerCase() + return host !== appHost && !host.endsWith('.workers.dev') +} + function configzCacheKey(request: Request, includeValidators = false): Request | null { if (request.method !== 'GET') return null const url = new URL(request.url)