diff --git a/.env.example b/.env.example index eed72f13e1..e43e480dcf 100644 --- a/.env.example +++ b/.env.example @@ -308,6 +308,33 @@ APNS_BUNDLE_ID=ai.pagespace.ios # FLY_MACHINES_ORG_TOKEN=your_fly_org_token_here # PUBLISHED_APPS_NETWORK=published-apps # +# Serving tier (the router). PUBLISHED_APPS_APEX is REQUIRED once +# APP_HOSTING_ENABLED=true — validateEnv() refuses to boot without it, on +# purpose. Published apps run customer-authored SERVER code on subdomains of +# this apex, so it must be a deliberate choice that has completed Public Suffix +# List registration, never a value inherited from a default: without a PSL entry +# one published app can set a cookie for the whole apex that every other +# published app then sends. See ROUTING.md for the submission checklist, and note +# that PSL listing is NOT retroactive — it ships inside browser releases. +# APP_ROUTER_FLY_APP_NAME is the Fly app that terminates the apex and holds +# custom-domain certs; it falls back to FLY_PROXY_APP_NAME and then to +# 'pagespace-proxy', and it MUST have been created on PUBLISHED_APPS_NETWORK +# because fly-replay cannot cross 6PN networks. +# PUBLISHED_APPS_APEX=pagespace.app +# APP_ROUTER_FLY_APP_NAME=pagespace-proxy +# +# Router secrets. BOTH must be >=32 characters when set, and both fail CLOSED: +# unset (or below the floor) means the router refuses rather than skipping the +# check. APP_ROUTER_PROXY_SECRET is what proves a request came from the edge +# proxy — the route is mounted on the web app, which also answers at +# pagespace.ai/api/..., so without it any internet caller could hand us a +# published-app hostname and collect a fly-replay header, waking and billing any +# app they can name. Set the SAME value as a secret on the proxy app. +# APP_REPLAY_SECRET derives the per-app fly-replay `state` key. +# Generate each with: openssl rand -hex 32 +# APP_REPLAY_SECRET=your_app_replay_secret_here_generate_with_openssl_rand_hex_32 +# APP_ROUTER_PROXY_SECRET=your_router_proxy_secret_here_generate_with_openssl_rand_hex_32 +# # Build & deploy pipeline (processor). APP_BUILD_SOURCE_ROOT is the directory # every build context is resolved UNDER — a source ref that escapes it is # refused — and leaving it unset means this processor is simply not a build diff --git a/CHANGELOG.md b/CHANGELOG.md index eb6df7fc16..6db7655400 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -205,6 +205,19 @@ All notable user-facing changes to PageSpace are documented here. Format follows ### Fixed +- **A custom domain stuck on SSL now tells you which DNS record to add** — when a certificate is + waiting on an ownership record, domain settings name it outright: the `_fly-ownership` TXT record, + where it goes, and every value that satisfies it — Fly accepts an app-scoped or an org-scoped + value, and whichever ones it offers are the ones you are shown. Previously that domain simply sat at "provisioning" + indefinitely with nothing to act on, because through the certificate's status alone "the + certificate has not issued yet" and "you were never told to add a DNS record" look identical — and + only one of them ever resolves on its own. The domain also stays healthy while it waits instead of + being marked failed, so a site already being served keeps serving. "Check SSL" now does more than + re-read a cached answer: once the record is visible in DNS it asks the certificate authority to + look again, rather than leaving you to wait out its own polling schedule. And removing a domain + now detaches its certificate, which previously kept billing after the domain was gone. Deleting an + entire drive does not yet do this, so remove its domains individually first if you want their + certificates released. - **An older AI conversation keeps its controls** — opening an AI page on a past conversation could drop the whole bar above the chat: no agent name, and no "+" to start a new conversation, so the only way to begin one was to go to the History tab and find the button there. Which of the two the diff --git a/apps/web/src/__tests__/middleware.test.ts b/apps/web/src/__tests__/middleware.test.ts index 5b3bc02005..71acb0afd5 100644 --- a/apps/web/src/__tests__/middleware.test.ts +++ b/apps/web/src/__tests__/middleware.test.ts @@ -31,8 +31,13 @@ vi.mock('@/middleware/security-headers', () => ({ // Real predicate logic — middleware passes its result as `skipCSP` so the // handoff-bridge OAuth callbacks don't get a middleware CSP layered on top of // their own (see the isHandoffBridgeRoute describe block below). - isHandoffBridgeRoute: (pathname: string) => - pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback', + // Real predicate logic, matching security-headers.ts: the handoff-bridge + // callbacks plus the published-app router all deliver their own CSP. + APP_ROUTER_ROUTE_PATH: '/api/app-hosting/router', + routeOwnsItsOwnCsp: (pathname: string) => + pathname === '/api/auth/google/callback' || + pathname === '/api/auth/apple/callback' || + pathname === '/api/app-hosting/router', isPublicPageRoute: () => false, isPublishedSiteHost: () => false, isSecureRequest: () => true, @@ -123,6 +128,99 @@ describe('middleware — /api/public/forms carve-outs', () => { }); }); +// Regression coverage for a real bug found while building the published-app +// routing tier: the router endpoint is called by pagespace-proxy with NO session +// and no user — it authenticates via the APP_ROUTER_PROXY_SECRET shared secret +// checked inside the route. Without a middleware carve-out, every such call is +// 401'd before route.ts ever runs, which does not fail any handler test (those +// invoke the route directly) but makes EVERY published app unreachable in a real +// deployment. The route's own tests cannot see this; only this one can. +describe('middleware — published-app router carve-out', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockGetSessionFromCookies.mockReturnValue(undefined); + }); + + it('skips the session-cookie check for a proxy call carrying no session', async () => { + mockValidateOriginForMiddleware.mockReturnValue({ valid: true, origin: null, skipped: true, reason: 'no origin' }); + mockIsOriginValidationBlocking.mockReturnValue(true); + + const request = buildRequest('/api/app-hosting/router'); + const response = await middleware(request); + + expect(response.status).not.toBe(401); + // createSecureResponse is mocked to always return 200, so the status alone + // would not catch the carve-out being removed — assert the session lookup + // was never reached. + expect(mockGetSessionFromCookies).not.toHaveBeenCalled(); + }); + + it('lets the route own its CSP, so the parked page keeps its inline styles', async () => { + // The API CSP is `default-src 'none'`, which falls style-src back to 'none'; + // browsers enforce the intersection of every delivered policy, so without + // the skip the customer-facing "app paused" page renders unstyled. + mockValidateOriginForMiddleware.mockReturnValue({ valid: true, origin: null, skipped: true, reason: 'no origin' }); + mockIsOriginValidationBlocking.mockReturnValue(true); + + await middleware(buildRequest('/api/app-hosting/router')); + + expect(mockCreateSecureResponse).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ skipCSP: true }), + ); + }); + + it('never runs origin validation — published apps have unbounded origins', async () => { + // A published app's own fetch carries its own origin, which is not and can + // never be in our allowlist. If origin validation ran here in blocking mode, + // every non-GET request a published app made to itself would 403. + mockValidateOriginForMiddleware.mockReturnValue({ + valid: false, + origin: 'https://acme.pagespace.app', + skipped: false, + reason: 'origin not in allowlist', + }); + mockIsOriginValidationBlocking.mockReturnValue(true); + + const response = await middleware( + buildRequest('/api/app-hosting/router', { origin: 'https://acme.pagespace.app' }), + ); + + expect(response.status).not.toBe(403); + expect(mockValidateOriginForMiddleware).not.toHaveBeenCalled(); + }); + + it('lets an OPTIONS preflight reach the route instead of answering it with our CORS policy', async () => { + // A preflight for a published app belongs to THAT app and must be replayed to + // it. The Bearer-API short-circuit would answer 204 with our own + // Access-Control-Allow-Headers, so a published app could never allow a custom + // request header on a cross-origin call. + mockValidateOriginForMiddleware.mockReturnValue({ valid: true, origin: null, skipped: true, reason: 'no origin' }); + mockIsOriginValidationBlocking.mockReturnValue(true); + + const response = await middleware( + buildRequest('/api/app-hosting/router', {}, 'OPTIONS'), + ); + + // The mocked createSecureResponse returns 200; the CORS short-circuit would + // have returned a 204 carrying Access-Control-Allow-Methods. + expect(response.headers.get('Access-Control-Allow-Methods')).toBeNull(); + expect(response.status).not.toBe(204); + }); + + it('does not extend the carve-out to sibling app-hosting paths', async () => { + // Exact match only: a future authenticated /api/app-hosting/* route must not + // inherit an exemption meant for the one endpoint the proxy calls. + mockValidateOriginForMiddleware.mockReturnValue({ valid: true, origin: null, skipped: true, reason: 'no origin' }); + mockIsOriginValidationBlocking.mockReturnValue(true); + + await middleware(buildRequest('/api/app-hosting/apps')); + + expect(mockGetSessionFromCookies).toHaveBeenCalled(); + }); +}); + // Regression coverage for a real bug: middleware.ts used to hand-duplicate two // of the three bearer prefixes `@/lib/auth` actually authenticates (mcp_, // ps_sess_), silently missing ps_at_ (OAuth access tokens, `pagespace login`). diff --git a/apps/web/src/app/api/__tests__/security-audit-coverage.test.ts b/apps/web/src/app/api/__tests__/security-audit-coverage.test.ts index 5cd7bce2e6..11a67c4a44 100644 --- a/apps/web/src/app/api/__tests__/security-audit-coverage.test.ts +++ b/apps/web/src/app/api/__tests__/security-audit-coverage.test.ts @@ -47,6 +47,7 @@ const AUDIT_EXEMPT_ROUTES = new Map([ ['internal/*', 'Internal service-to-service endpoints'], ['cron/scheduled-backups', 'HMAC-signed internal cron job — no user session, authenticated by shared secret, executes pre-authorized backup schedules'], ['memory/cron', 'Internal memory cron job'], + ['app-hosting/router', 'Published-app serving edge, called by pagespace-proxy on EVERY request to a published app (no fly-replay-cache on the metered tier, by design) — there is no user session to attribute, the caller is authenticated by the APP_ROUTER_PROXY_SECRET shared secret like the HMAC cron routes above, and one audit row per served asset would swamp the audit log with routing decisions. The security-relevant outcomes are counted at the edge instead: a refused caller answers 404 and a credit-exhausted app answers 402, both distinguishable in proxy logs.'], ['desktop-bridge/status', 'Desktop app connection status check'], ['provisioning-status/[slug]', 'Tenant provisioning status polling'], diff --git a/apps/web/src/app/api/app-hosting/router/__tests__/route.test.ts b/apps/web/src/app/api/app-hosting/router/__tests__/route.test.ts new file mode 100644 index 0000000000..df17305b05 --- /dev/null +++ b/apps/web/src/app/api/app-hosting/router/__tests__/route.test.ts @@ -0,0 +1,299 @@ +/** + * Contract tests for the published-app router endpoint. + * + * This route is the only thing standing between a hostname and a billable + * machine start, and it is mounted on the same web app that answers at + * `pagespace.ai/api/...`. So the boundary property matters as much as the + * routing one: an unauthenticated caller must not be able to hand us a + * published-app hostname and collect a `fly-replay` header — that would turn + * our own web app into a general-purpose replay emitter for the whole Fly org + * and let anyone wake, and therefore bill, any published app they can name. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; + +vi.mock('server-only', () => ({})); +vi.mock('@pagespace/lib/logging/logger-config', () => ({ + loggers: { api: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() } }, +})); + +const resolveAppRoute = vi.fn(); +vi.mock('@pagespace/lib/services/app-hosting/router', () => ({ + resolveAppRoute: (...args: unknown[]) => resolveAppRoute(...args), +})); + +import { GET, POST, HEAD } from '../route'; + +// >=32 chars: resolveAppRouterProxySecret reads anything shorter as unset. +const PROXY_SECRET = 'proxy-secret-value-padded-to-32ch'; +const HOST = 'acme.pagespace.app'; + +function request( + headers: Record = {}, + init: RequestInit = {}, +): Request { + return new Request('https://pagespace.ai/api/app-hosting/router', { + ...init, + headers: { + 'x-pagespace-app-router-key': PROXY_SECRET, + 'x-pagespace-app-host': HOST, + ...headers, + }, + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + process.env.APP_ROUTER_PROXY_SECRET = PROXY_SECRET; + resolveAppRoute.mockResolvedValue({ + kind: 'replay', + flyAppName: 'pgs-app-abc', + state: 'ff00', + timeoutMs: 1500, + }); +}); + +afterEach(() => { + delete process.env.APP_ROUTER_PROXY_SECRET; +}); + +describe('the endpoint answers only the edge proxy', () => { + it('given no proxy key, should 404 without resolving anything', async () => { + const res = await GET( + new Request('https://pagespace.ai/api/app-hosting/router', { + headers: { 'x-pagespace-app-host': HOST }, + }), + ); + expect(res.status).toBe(404); + expect(resolveAppRoute).not.toHaveBeenCalled(); + }); + + it('given a WRONG proxy key, should 404 and never emit a replay', async () => { + const res = await GET(request({ 'x-pagespace-app-router-key': 'not-the-secret' })); + expect(res.status).toBe(404); + expect(res.headers.get('fly-replay')).toBeNull(); + expect(resolveAppRoute).not.toHaveBeenCalled(); + }); + + it('given the secret is UNSET, should refuse everything rather than skip the check', async () => { + // The fail-closed direction: an unconfigured secret must not silently + // disable the protection that stops this endpoint being world-callable. + delete process.env.APP_ROUTER_PROXY_SECRET; + const res = await GET(request()); + expect(res.status).toBe(404); + expect(resolveAppRoute).not.toHaveBeenCalled(); + }); + + it('given the correct key, should route', async () => { + const res = await GET(request()); + expect(res.status).toBe(204); + expect(resolveAppRoute).toHaveBeenCalledWith(HOST); + }); +}); + +describe('the hostname the decision is made about', () => { + it('given the explicit host header, should prefer it over Host', async () => { + await GET(request({ 'x-pagespace-app-host': 'real.pagespace.app', host: 'internal.flycast' })); + expect(resolveAppRoute).toHaveBeenCalledWith('real.pagespace.app'); + }); + + it('given no explicit header, should fall back to Host for a direct-to-web deployment', async () => { + const res = new Request('https://pagespace.ai/api/app-hosting/router', { + headers: { 'x-pagespace-app-router-key': PROXY_SECRET, host: 'fallback.pagespace.app' }, + }); + await GET(res); + expect(resolveAppRoute).toHaveBeenCalledWith('fallback.pagespace.app'); + }); +}); + +describe('a replay decision', () => { + it('given a replay, should emit the fly-replay header with the timeout and no body', async () => { + const res = await GET(request()); + expect(res.status).toBe(204); + expect(res.headers.get('fly-replay')).toBe('app=pgs-app-abc;state=ff00;timeout=1500'); + expect(await res.text()).toBe(''); + }); + + it('given a replay, should NOT set fly-replay-cache — the cache would skip the balance gate', async () => { + const res = await GET(request()); + expect(res.headers.get('fly-replay-cache')).toBeNull(); + }); + + it('given a replay, should forbid caching the decision', async () => { + const res = await GET(request()); + expect(res.headers.get('Cache-Control')).toContain('no-store'); + }); + + it('given a decision carrying header grammar, should refuse rather than emit a redirectable header', async () => { + resolveAppRoute.mockResolvedValue({ + kind: 'replay', + flyAppName: 'pgs-app;app=victim', + state: 'ff00', + timeoutMs: 1500, + }); + const res = await GET(request()); + expect(res.status).toBe(503); + expect(res.headers.get('fly-replay')).toBeNull(); + }); +}); + +describe('a refusal is served here, and starts no machine', () => { + it('given a parked app, should answer 402 with a page and no replay', async () => { + resolveAppRoute.mockResolvedValue({ kind: 'parked', reason: 'out_of_credits' }); + const res = await GET(request()); + expect(res.status).toBe(402); + expect(res.headers.get('fly-replay')).toBeNull(); + expect(await res.text()).toMatch(/credits/i); + }); + + it('given a deploying app, should answer 503 with a Retry-After', async () => { + resolveAppRoute.mockResolvedValue({ kind: 'unavailable', reason: 'deploying' }); + const res = await GET(request()); + expect(res.status).toBe(503); + expect(res.headers.get('Retry-After')).toBe('15'); + }); + + it('given no such app, should answer 404 with no Retry-After', async () => { + resolveAppRoute.mockResolvedValue({ kind: 'not_found', reason: 'no_such_app' }); + const res = await GET(request()); + expect(res.status).toBe(404); + expect(res.headers.get('Retry-After')).toBeNull(); + }); + + it('given any served page, should carry its own hardening headers', async () => { + resolveAppRoute.mockResolvedValue({ kind: 'parked', reason: 'out_of_credits' }); + const res = await GET(request()); + expect(res.headers.get('X-Content-Type-Options')).toBe('nosniff'); + expect(res.headers.get('Content-Security-Policy')).toContain("frame-ancestors 'none'"); + }); + + it('given the parked page, should allow the inline styles it is actually built from', async () => { + // The page is a single self-contained document styled with `style=` + // attributes. Middleware skips its own CSP for this path precisely so this + // policy is the one that applies; a policy without style-src would render + // the customer-facing enforcement page as unstyled text. + resolveAppRoute.mockResolvedValue({ kind: 'parked', reason: 'out_of_credits' }); + const res = await GET(request()); + const csp = res.headers.get('Content-Security-Policy') ?? ''; + expect(csp).toContain("style-src 'unsafe-inline'"); + expect(await res.text()).toContain('style='); + // Everything else stays shut. + expect(csp).toContain("default-src 'none'"); + expect(csp).not.toContain('script-src'); + }); +}); + +describe('the 1MB replay ceiling is named, not discovered', () => { + it('given a body past the limit, should answer 413 rather than let Fly 502', async () => { + const res = await POST(request({ 'content-length': String(1_048_577) }, { method: 'POST' })); + expect(res.status).toBe(413); + expect(resolveAppRoute).not.toHaveBeenCalled(); + }); + + // The figure in the page has to come FROM the constant. A hardcoded "1 MiB" + // beside it would keep claiming 1 MiB after somebody changed the number, which + // is the drift this whole limit already has one instance of at the proxy. + it('given an oversized body, should name the limit in MiB and bytes, both derived', async () => { + const res = await POST(request({ 'content-length': String(1_048_577) }, { method: 'POST' })); + const body = await res.text(); + + expect(body).toContain('1 MiB'); + expect(body).toContain('1,048,576 bytes'); + // Not "MB": a size parser reads that as 1,000,000, and the proxy mirroring + // this cap has to agree with it exactly. + expect(body).not.toMatch(/\d\s?MB\b/); + }); + + it('given a body at the limit, should route normally', async () => { + const res = await POST(request({ 'content-length': String(1_048_576) }, { method: 'POST' })); + expect(res.status).toBe(204); + }); + + /** + * A body with no `Content-Length`, delivered as a stream. Not merely HTTP/1.1 + * `chunked`: HTTP/2 forbids that encoding and carries content in DATA frames + * with no length at all, so this is the ordinary shape on a modern edge as well + * as the default for a streaming upload. Before the streamed check it walked + * straight past the header gate into a `fly-replay` Fly could not perform — + * surfacing as an opaque 502 rather than the 413 this edge exists to give. + */ + const lengthlessRequest = (totalBytes: number): Request => { + const chunk = 64 * 1024; + let sent = 0; + const body = new ReadableStream({ + pull(controller) { + if (sent >= totalBytes) { + controller.close(); + return; + } + const size = Math.min(chunk, totalBytes - sent); + sent += size; + controller.enqueue(new Uint8Array(size)); + }, + }); + return request({}, { method: 'POST', body, duplex: 'half' } as RequestInit); + }; + + it('given a lengthless body past the limit, should answer 413 rather than emit an unreplayable fly-replay', async () => { + const res = await POST(lengthlessRequest(1_048_577)); + expect(res.status).toBe(413); + expect(res.headers.get('fly-replay')).toBeNull(); + expect(resolveAppRoute).not.toHaveBeenCalled(); + }); + + it('given a lengthless body within the limit, should route normally', async () => { + const res = await POST(lengthlessRequest(128 * 1024)); + expect(res.status).toBe(204); + expect(res.headers.get('fly-replay')).toBe('app=pgs-app-abc;state=ff00;timeout=1500'); + }); + + it('given a bodyless GET, should route without paying for a stream read', async () => { + const res = await GET(request()); + expect(res.status).toBe(204); + }); + + // Measuring the body is the only step on this path that can throw, and a body + // that fails mid-read is ordinary at a serving edge: a client hangs up, an + // upload truncates. Before this was handled it propagated out of the handler + // as an unhandled 500 with a stack trace, on the hottest route in the system. + it('given a body that errors mid-read, should answer 400 rather than throw', async () => { + const erroring = new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array(1024)); + }, + pull(controller) { + controller.error(new TypeError('terminated')); + }, + }); + + const res = await POST( + request({}, { method: 'POST', body: erroring, duplex: 'half' } as RequestInit), + ); + + expect(res.status).toBe(400); + // Refused before any routing decision, and with no replay emitted — we could + // not establish the size, so Fly must not be handed the request. + expect(res.headers.get('fly-replay')).toBeNull(); + expect(resolveAppRoute).not.toHaveBeenCalled(); + }); +}); + +describe('an outage reads as an outage', () => { + it('given the resolver throws, should answer 503 rather than teach crawlers the app is gone', async () => { + resolveAppRoute.mockRejectedValue(new Error('connection terminated')); + const res = await GET(request()); + expect(res.status).toBe(503); + expect(res.status).not.toBe(404); + expect(res.headers.get('Retry-After')).toBe('30'); + }); +}); + +describe('every method the proxy might forward is routable', () => { + it.each([ + ['GET', GET], + ['HEAD', HEAD], + ['POST', POST], + ])('given a %s request, should reach the same decision', async (method, handler) => { + const res = await handler(request({}, { method: method === 'HEAD' ? 'HEAD' : method })); + expect(res.status).toBe(204); + }); +}); diff --git a/apps/web/src/app/api/app-hosting/router/route.ts b/apps/web/src/app/api/app-hosting/router/route.ts new file mode 100644 index 0000000000..b8f85e7718 --- /dev/null +++ b/apps/web/src/app/api/app-hosting/router/route.ts @@ -0,0 +1,248 @@ +/** + * The published-app serving edge's decision endpoint. + * + * The edge proxy (`pagespace-proxy`, Caddy) cannot make this decision: it needs a + * `published_apps` row and a credit balance. So the proxy forwards every request + * for the published-apps apex here — rewritten to this path, with the real + * hostname in a header — and this route answers one of two ways: + * + * • `fly-replay` header → Fly's proxy replays the ORIGINAL request (not this + * rewritten one) to the target app, auto-starting its machine if stopped. + * The target's response goes straight back to the client and NEVER passes + * through us or through Caddy — see `services/app-hosting/router.ts`. + * • a page → parked / unavailable / not-found, served from here, with no + * machine started. + * + * ⚠️ THIS ROUTE IS THE ONLY THING STANDING BETWEEN A HOSTNAME AND A BILLABLE + * MACHINE START. Everything it refuses to replay is a machine that stays off. + * That is why it authenticates the caller (below), why it is not cacheable, and + * why an unknown app status resolves to "unavailable" rather than "replay". + */ + +import { NextResponse } from 'next/server'; +import { loggers } from '@pagespace/lib/logging/logger-config'; +import { secureCompare } from '@pagespace/lib/auth/secure-compare'; +import { + APP_ROUTER_HOST_HEADER, + APP_ROUTER_KEY_HEADER, + resolveAppRouterProxySecret, +} from '@pagespace/lib/services/app-hosting/routing-env'; +import { resolveAppRoute } from '@pagespace/lib/services/app-hosting/router'; +import { + buildFlyReplayHeader, + exceedsReplayableBody, + exceedsStreamedBody, + MAX_REPLAYABLE_BODY_BYTES, +} from '@pagespace/lib/services/app-hosting/router-core'; +import { + renderAppRouterPage, + retryAfterFor, + statusCodeFor, +} from '@pagespace/lib/services/app-hosting/parked-page'; + +// A routing decision is per-request state (an app's status and its payer's +// balance both change under us), so nothing here may be statically rendered or +// revalidated. +export const dynamic = 'force-dynamic'; +export const runtime = 'nodejs'; + +/** + * No response from this route may be cached ANYWHERE — not by a browser, not by + * an intermediary. A cached parked page outlives the top-up that should have + * cleared it, and a cached 404 outlives the publish that should have filled it. + * The metered tier already forgoes `fly-replay-cache` for the same reason; this + * is the same rule one layer out. + */ +const NO_STORE = 'no-store, no-cache, must-revalidate, private'; + +function htmlResponse(body: string, status: number, extraHeaders: Record = {}): Response { + return new NextResponse(body, { + status, + headers: { + 'Content-Type': 'text/html; charset=utf-8', + 'Cache-Control': NO_STORE, + // The served app controls its own headers (its response bypasses us + // entirely); these apply only to the pages this route itself renders. + 'X-Content-Type-Options': 'nosniff', + // This route OWNS its CSP — middleware skips its own for this path + // (`routeOwnsItsOwnCsp`), because the API default of `default-src 'none'` + // falls style-src back to 'none' and browsers enforce the intersection of + // every delivered policy, which would render the parked page as unstyled + // text. `'unsafe-inline'` here covers style ATTRIBUTES only: the page is a + // single self-contained document with no script, no external asset and no + // form, and every other directive stays shut. + 'Content-Security-Policy': + "default-src 'none'; style-src 'unsafe-inline'; frame-ancestors 'none'; base-uri 'none'; form-action 'none'", + 'Referrer-Policy': 'no-referrer', + ...extraHeaders, + }, + }); +} + +/** + * Whether this request really came from the edge proxy. + * + * The route is mounted on `pagespace-web`, which also answers at + * `pagespace.ai/api/...`. Without this check, any internet caller could hand us + * a published-app hostname and collect a `fly-replay` header — turning our own + * web app into a general-purpose replay emitter for the whole Fly org, and + * letting anyone wake (and therefore bill) any published app they can name. + * + * An UNSET secret refuses everything. That is the fail-closed direction and it + * is deliberate: the alternative reading ("no secret configured, so skip the + * check") disables exactly the protection that stops the endpoint being + * world-callable, at precisely the moment nobody has configured it. + */ +function isFromEdgeProxy(request: Request): boolean { + const expected = resolveAppRouterProxySecret(); + if (expected.length === 0) return false; + const presented = request.headers.get(APP_ROUTER_KEY_HEADER); + if (!presented) return false; + return secureCompare(presented, expected); +} + +/** + * The hostname the client actually asked for. + * + * The proxy sets it explicitly rather than relying on `Host`, because a rewrite + * plus an internal `flycast` hop is exactly the sort of path where `Host` gets + * rewritten by something in the middle. `Host` remains the fallback so a + * direct-to-web deployment (no separate proxy) still routes. + * + * Both are attacker-influenced input by nature; nothing downstream trusts them + * for anything but a lookup, and the request is authenticated as proxy-origin + * before we get here. + */ +function requestedHost(request: Request): string { + return request.headers.get(APP_ROUTER_HOST_HEADER) ?? request.headers.get('host') ?? ''; +} + +async function handle(request: Request): Promise { + if (!isFromEdgeProxy(request)) { + // Deliberately terse and deliberately 404, not 403: an endpoint that + // confirms its own existence to an unauthenticated caller is an invitation. + return new NextResponse(null, { status: 404, headers: { 'Cache-Control': NO_STORE } }); + } + + const host = requestedHost(request); + + // Fly cannot replay a body over 1MB. Answering here with a clear 413 that + // names the limit is the difference between a documented constraint and a + // mystery 502 from the platform. Upload paths are supposed to go direct to + // Tigris via presigned URLs and never reach this edge at all. + // + // Two checks, because there are two ways to arrive. A request that declares + // its size is refused on the header alone and costs nothing. A request that + // sends no Content-Length has its body measured — bounded at the limit, and + // only ever for the request that gave us no length to read. Without the second + // check the limit is trivially bypassed by omitting Content-Length, which is + // the default shape of a streaming upload AND of every HTTP/2 request, since + // HTTP/2 forbids Transfer-Encoding and carries content in DATA frames. + const declaredLength = request.headers.get('content-length'); + let tooLarge: boolean; + try { + tooLarge = declaredLength + ? exceedsReplayableBody(declaredLength) + : await exceedsStreamedBody(request.body); + } catch { + // The body failed mid-read — a client that hung up, or a truncated upload. + // Measuring it is the ONLY step on this path that can throw, and letting it + // propagate would answer the hottest route in the system with an unhandled + // 500 and a stack trace. Refusing is also the safe answer on the merits: we + // could not establish the size, so emitting `fly-replay` would hand Fly a + // body it may not be able to replay. 400 rather than 413 because the body + // did not exceed anything — it did not arrive. + // + // Deliberately not logged: a client hanging up mid-request is routine at a + // serving edge, and this route runs once per ASSET of every published page, + // so logging it would bury the genuine failures the two `error` calls below + // exist to surface. + return htmlResponse( + 'Bad request

The request body could not be read.', + 400, + ); + } + if (tooLarge) { + // Both units, and BOTH DERIVED from the constant. Naming the mebibyte + // matters here for the same reason it matters in the proxy config: "1 MB" + // reads as 1,000,000 to a size parser and to half the people who see it, + // and this limit is 1,048,576. Hardcoding "1 MiB" beside the constant would + // just move the drift — the text would keep claiming 1 MiB after somebody + // changed the number. + const limitMiB = MAX_REPLAYABLE_BODY_BYTES / 1024 / 1024; + return htmlResponse( + `Payload too large

Request bodies above ${limitMiB} MiB (${MAX_REPLAYABLE_BODY_BYTES.toLocaleString('en-US')} bytes) cannot be routed to a published app. Upload directly to storage instead.`, + 413, + ); + } + + let decision; + try { + decision = await resolveAppRoute(host); + } catch (error) { + // A genuine failure (the database is unreachable) is an OUTAGE, and it must + // read as one. Reporting it as "no such app" would hand every published site + // a 404 during an incident and teach crawlers the apps are gone. + loggers.api.error('Published-app router failed to resolve a route', { + host, + error: error instanceof Error ? error.message : 'unknown error', + }); + return htmlResponse( + renderAppRouterPage({ kind: 'unavailable', reason: 'failed' }, host), + 503, + { 'Retry-After': '30' }, + ); + } + + if (decision.kind === 'replay') { + let replay: string; + try { + replay = buildFlyReplayHeader({ + flyAppName: decision.flyAppName, + state: decision.state, + timeoutMs: decision.timeoutMs, + }); + } catch (error) { + loggers.api.error('Published-app router built an invalid fly-replay header', { + host, + flyAppName: decision.flyAppName, + error: error instanceof Error ? error.message : 'unknown error', + }); + return htmlResponse( + renderAppRouterPage({ kind: 'unavailable', reason: 'failed' }, host), + 503, + { 'Retry-After': '30' }, + ); + } + // 204 with no body: Fly's proxy consumes this response and replays the + // original request, so the client never sees it. A body here would be pure + // waste on the hottest path in the system. + // + // NO `fly-replay-cache`. The cache skips this hop on subsequent requests, + // and this hop IS the balance gate — a cached replay would keep a machine + // awake for a payer we would refuse today. Only the flat-rate dedicated + // tier may ever set it (see `replayCachePolicyFor`). + return new NextResponse(null, { + status: 204, + headers: { 'fly-replay': replay, 'Cache-Control': NO_STORE }, + }); + } + + const retryAfter = retryAfterFor(decision); + return htmlResponse( + renderAppRouterPage(decision, host), + statusCodeFor(decision), + retryAfter === null ? {} : { 'Retry-After': String(retryAfter) }, + ); +} + +// Every method the proxy might forward. A published app may serve any of them, +// and the routing decision does not depend on which — so they all take the same +// path rather than one being quietly unroutable. +export const GET = handle; +export const HEAD = handle; +export const POST = handle; +export const PUT = handle; +export const PATCH = handle; +export const DELETE = handle; +export const OPTIONS = handle; diff --git a/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/__tests__/route.test.ts b/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/__tests__/route.test.ts index 9953e6f292..4ceef6b399 100644 --- a/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/__tests__/route.test.ts +++ b/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/__tests__/route.test.ts @@ -27,6 +27,9 @@ vi.mock('@pagespace/lib/logging/logger-config', () => ({ const addCertificate = vi.fn(); vi.mock('@/lib/fly/certs', () => ({ addCertificate: (...args: unknown[]) => addCertificate(...args), + // Faithful to the real predicate: either credential counts. + hasFlyCertCredential: () => + Boolean(process.env.FLY_API_TOKEN || process.env.FLY_MACHINES_ORG_TOKEN), })); const dbSelect = vi.fn(); diff --git a/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/route.ts b/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/route.ts index 9f0843bb19..021e600666 100644 --- a/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/route.ts +++ b/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/cert/refresh/route.ts @@ -6,6 +6,7 @@ import { isPrincipalDriveOwnerOrAdmin, } from '@/lib/auth'; import { loggers } from '@pagespace/lib/logging/logger-config'; +import { hasFlyCertCredential } from '@/lib/fly/certs'; import { auditRequest } from '@pagespace/lib/audit/audit-log'; import { isCertEligible } from '@pagespace/lib/canvas/cert-action'; import { db } from '@pagespace/db/db'; @@ -47,15 +48,20 @@ export async function POST( ); } - if (!process.env.FLY_API_TOKEN) { - loggers.api.error('FLY_API_TOKEN not set — cert provisioning unavailable'); - return NextResponse.json({ error: 'SSL provisioning is not configured (ops: set FLY_API_TOKEN)' }, { status: 503 }); + // Same predicate the certs module uses, so a deployment carrying only + // FLY_MACHINES_ORG_TOKEN is not told SSL is unconfigured when it is. + if (!hasFlyCertCredential()) { + loggers.api.error('No Fly API credential set — cert provisioning unavailable'); + return NextResponse.json( + { error: 'SSL provisioning is not configured (ops: set FLY_API_TOKEN or FLY_MACHINES_ORG_TOKEN)' }, + { status: 503 }, + ); } // Advance the cert one step via the shared service (also used by the lazy // reconcile on the domains-list GET). It commits the status, then runs the // active/cert_failed side effects best-effort. - const { status: nextStatus, action } = await reconcileCustomDomainCert({ + const { status: nextStatus, action, ownershipInstruction } = await reconcileCustomDomainCert({ id: domain.id, driveId, hostname: domain.hostname, @@ -75,7 +81,11 @@ export async function POST( }, }); - return NextResponse.json({ status: nextStatus, action }); + // `ownershipInstruction` is the actionable half of a cert that is stuck: + // without it the UI can only say "still provisioning" for a hostname that + // will never provision until the customer publishes a TXT record nobody has + // told them about. Null whenever nothing is waiting on them. + return NextResponse.json({ status: nextStatus, action, ownershipInstruction: ownershipInstruction ?? null }); } catch (error) { loggers.api.error('Error refreshing cert:', error as Error); return NextResponse.json({ error: 'Failed to refresh cert' }, { status: 500 }); diff --git a/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/route.ts b/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/route.ts index cf78e2eaeb..1c097f24dd 100644 --- a/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/route.ts +++ b/apps/web/src/app/api/drives/[driveId]/domains/[domainId]/route.ts @@ -14,6 +14,8 @@ import { customDomains } from '@pagespace/db/schema/custom-domains'; import { clearCustomHost, mirrorDriveToCustomHost } from '@/lib/canvas/custom-domain-mirror'; import { regeneratePublishedSiteFiles, republishDriveCanonical, renderDomainNotFoundOverride } from '@/lib/canvas/publish-page'; import { isServingStatus } from '@pagespace/lib/canvas/cert-action'; +import { resolveAppRouterFlyAppName } from '@pagespace/lib/services/app-hosting/routing-env'; +import { removeCertificate } from '@/lib/fly/certs'; import { isValidDriveNotFoundPage } from '@pagespace/lib/services/drive-service'; const AUTH_OPTIONS = { allow: ['session', 'mcp'] as const, requireCSRF: true }; @@ -240,6 +242,53 @@ export async function DELETE( }); } + // Detach the hostname from the router app at Fly. Certificates bill PER + // HOSTNAME ($0.10/mo past the first ten), and this row was the only record + // that the hostname was ever attached — so a delete that skips this leaves a + // charge with nothing in our database pointing at it, which is the same + // orphaned-billing-resource shape `app_hosting_reclaims` exists to prevent + // for Fly apps. Best-effort and fire-and-forget, like the storage cleanup + // above: `deleteCertificate` is idempotent (a hostname Fly does not have is + // already in the desired state), so a failed attempt is safely retried by + // re-adding and re-removing the domain, and a Fly outage must not block the + // user's removal. Platform-owned rows are skipped — their TLS comes from the + // app's own domain and Fly never issued a per-hostname cert for them. + if (!deleted.platformOwned) { + const routerApp = resolveAppRouterFlyAppName(); + // SCOPE LIMIT, stated because it is easy to read this as solved: this is the + // ONLY caller of `removeCertificate`, and `custom_domains.drive_id` cascades + // off `drives`. So deleting a DRIVE — or the 30-day GDPR purge, or the + // account-erasure worker — destroys every domain row without ever running + // this, stranding a per-hostname certificate charge whose only pointer is + // gone. That is the same shape `app_hosting_reclaims` exists to prevent for + // Fly apps, and per that table's own docblock the fix is NOT to guard each + // delete path ("unenforceable — there is always one more path", and it + // cannot work for erasure) but to invert the dependency with an AFTER DELETE + // trigger writing to a FK-less outbox. Certificates have no such outbox yet. + // Explicit removal detaching the cert is strictly better than the previous + // behaviour of never detaching it; it is not complete coverage. + const warnCertRemovalFailed = (error: string) => { + loggers.api.warn('Failed to remove Fly certificate after domain removal', { + hostname: deleted.hostname, + routerApp, + error, + }); + }; + // `removeCertificate` returns a discriminated result and does not reject + // today. The `.catch` is here because that is an invariant of ANOTHER + // module, not of this call site: a fire-and-forget promise that starts + // rejecting surfaces as an unhandled rejection, which fails the coverage + // job while every test still reports passing — a failure that would not + // point back at this line. + void removeCertificate(routerApp, deleted.hostname) + .then((result) => { + if (!result.ok) warnCertRemovalFailed(result.error); + }) + .catch((err: unknown) => { + warnCertRemovalFailed(err instanceof Error ? err.message : 'unknown error'); + }); + } + auditRequest(request, { eventType: 'data.delete', userId: auth.userId, diff --git a/apps/web/src/app/api/drives/[driveId]/domains/__tests__/route.test.ts b/apps/web/src/app/api/drives/[driveId]/domains/__tests__/route.test.ts index f959e30df8..1c018ba4c5 100644 --- a/apps/web/src/app/api/drives/[driveId]/domains/__tests__/route.test.ts +++ b/apps/web/src/app/api/drives/[driveId]/domains/__tests__/route.test.ts @@ -214,6 +214,55 @@ describe('GET /api/drives/[driveId]/domains', () => { }); } + // The instruction was computed on every list and then thrown away — the route + // destructured only `status`. That left a certificate stuck on an ownership TXT + // with no delivery to the customer except a transient toast behind a button + // they had no reason to press, for the one cert state that never resolves on + // its own. The list is where a customer actually looks, so it has to carry it. + it('returns the ownership instruction for a stuck cert instead of discarding it', async () => { + mockGetSelects([ + { id: 'd1', driveId: DRIVE_ID, hostname: 'stuck.com', status: 'provisioning', createdAt: new Date() }, + ]); + reconcileCustomDomainCert.mockResolvedValueOnce({ + status: 'provisioning', + action: 'poll-again', + ownershipInstruction: 'Add a TXT record at _fly-ownership.stuck.com with the value org-XYZ789', + }); + + const body = await (await GET(makeReq(), ctx())).json(); + + expect(body.domains[0].ownershipInstruction).toContain('_fly-ownership.stuck.com'); + expect(body.domains[0].ownershipInstruction).toContain('org-XYZ789'); + }); + + // The case that would rot silently: a domain that FIXED its DNS must stop + // showing the instruction on the very next load. That only holds because the + // value is computed per request instead of persisted — a stored column would + // still be serving the old string. A terminal-status row bypasses reconcile + // altogether and so carries no instruction at all, which is correct: it is not + // blocked on ownership, and "we did not ask" must not render as an instruction. + it('carries no instruction for a terminal-status domain, which never reconciles', async () => { + mockGetSelects([ + { id: 'd1', driveId: DRIVE_ID, hostname: 'active.com', status: 'active', createdAt: new Date() }, + ]); + + const body = await (await GET(makeReq(), ctx())).json(); + + expect(reconcileCustomDomainCert).not.toHaveBeenCalled(); + expect(body.domains[0].ownershipInstruction).toBeUndefined(); + }); + + it('reports no instruction as null rather than dropping the key', async () => { + mockGetSelects([ + { id: 'd1', driveId: DRIVE_ID, hostname: 'fine.com', status: 'provisioning', createdAt: new Date() }, + ]); + reconcileCustomDomainCert.mockResolvedValueOnce({ status: 'provisioning', action: 'poll-again' }); + + const body = await (await GET(makeReq(), ctx())).json(); + + expect(body.domains[0]).toHaveProperty('ownershipInstruction', null); + }); + it('reconciles verified and provisioning rows and returns the advanced status', async () => { mockGetSelects([ { id: 'd1', driveId: DRIVE_ID, hostname: 'verified.com', status: 'verified', createdAt: new Date() }, diff --git a/apps/web/src/app/api/drives/[driveId]/domains/route.ts b/apps/web/src/app/api/drives/[driveId]/domains/route.ts index c78b08c507..f6c66f0edb 100644 --- a/apps/web/src/app/api/drives/[driveId]/domains/route.ts +++ b/apps/web/src/app/api/drives/[driveId]/domains/route.ts @@ -76,8 +76,16 @@ export async function GET( domains.map(async (domain) => { if (!CERT_NON_TERMINAL.has(domain.status)) return domain; try { - const { status } = await reconcileCustomDomainCert(domain, { allowFailureTransition: false }); - return { ...domain, status }; + // `ownershipInstruction` is kept, not discarded: it is the actionable + // half of a certificate stuck on an ownership TXT, and that is the one + // cert state that never resolves on its own — somebody has to be told. + // Returned per request rather than stored, deliberately: a column + // holding it would be stale the moment the customer fixed their zone, + // whereas this is recomputed from live DNS on every list. + const { status, ownershipInstruction } = await reconcileCustomDomainCert(domain, { + allowFailureTransition: false, + }); + return { ...domain, status, ownershipInstruction: ownershipInstruction ?? null }; } catch (err) { loggers.api.warn('Lazy cert reconcile failed during domains list', { driveId, diff --git a/apps/web/src/app/dashboard/[driveId]/settings/domains/__tests__/page.test.tsx b/apps/web/src/app/dashboard/[driveId]/settings/domains/__tests__/page.test.tsx new file mode 100644 index 0000000000..ddf8d53482 --- /dev/null +++ b/apps/web/src/app/dashboard/[driveId]/settings/domains/__tests__/page.test.tsx @@ -0,0 +1,146 @@ +/** + * The domain row's ownership instruction. + * + * A certificate waiting on Fly's `_fly-ownership` TXT is the one cert state that + * never resolves on its own — somebody has to be told what to publish. That + * instruction used to reach a human only as a 30-second toast behind the manual + * "Check SSL" button, so a customer who never pressed it saw a domain sitting at + * "provisioning" with nothing to act on. + * + * The server half (the list route carrying the instruction rather than + * discarding it) is covered in the route's own suite. What is asserted HERE is + * the half that suite cannot see: that the row actually renders it, and that it + * disappears again the moment the instruction does. + */ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { SWRConfig } from 'swr'; + +vi.mock('next/navigation', () => ({ + useParams: () => ({ driveId: 'drive-1' }), + useRouter: () => ({ push: vi.fn(), replace: vi.fn(), refresh: vi.fn(), back: vi.fn() }), + usePathname: () => '/', + useSearchParams: () => new URLSearchParams(), +})); + +vi.mock('sonner', () => ({ + toast: { error: vi.fn(), success: vi.fn(), warning: vi.fn(), info: vi.fn() }, +})); + +const mockFetchWithAuth = vi.fn(); +vi.mock('@/lib/auth/auth-fetch', () => ({ + fetchWithAuth: (...a: unknown[]) => mockFetchWithAuth(...a), + del: vi.fn(), + patch: vi.fn(), + post: vi.fn(), +})); + +const drive = { + id: 'drive-1', + name: 'Acme', + isOwned: true, + role: 'OWNER', + notFoundPageId: null, + publishDefaultOgImageUrl: '', + publishFaviconUrl: '', +}; + +vi.mock('@/hooks/useDrive', () => ({ + useDriveStore: (selector: (s: unknown) => unknown) => + selector({ + drives: [drive], + isLoading: false, + fetchDrives: vi.fn(), + updateDrive: vi.fn(), + }), +})); + +vi.mock('@/hooks/useAuth', () => ({ useAuth: () => ({ user: { id: 'u1', role: 'user' } }) })); + +vi.mock('@/components/common/PagePickerPopover', () => ({ + PagePickerPopover: () =>

, +})); + +import DomainsSettingsPage from '../page'; + +const INSTRUCTION = + 'Add a TXT record at _fly-ownership.docs.acme.com with the value org-XYZ789 — Fly cannot verify ownership of this domain until it resolves.'; + +/** One provisioning domain, with or without an outstanding ownership record. */ +function domainsPayload(ownershipInstruction: string | null) { + return { + domains: [ + { + id: 'd1', + driveId: 'drive-1', + hostname: 'docs.acme.com', + status: 'provisioning', + isPrimary: false, + createdAt: new Date().toISOString(), + platformOwned: false, + publishLandingPageId: null, + publishNotFoundPageId: null, + ownershipInstruction, + }, + ], + limit: 5, + }; +} + +function serve(ownershipInstruction: string | null) { + mockFetchWithAuth.mockImplementation((url: string) => + Promise.resolve({ + ok: true, + json: () => + Promise.resolve( + url.includes('/subdomain') ? { subdomain: null } : domainsPayload(ownershipInstruction), + ), + }), + ); +} + +/** + * A FRESH SWR cache per render, not the module-global one. + * + * Without this the second test rendered the first test's cached response: SWR + * keys on the URL, both tests request the same URL, and the stale value is + * served before the new fetch resolves. That made the "nothing owed" assertion + * fail against an instruction the test never supplied — a suite lying because of + * shared state rather than because of the code under test. + */ +const renderPage = () => + render( + new Map(), dedupingInterval: 0 }}> + + , + ); + +describe('DomainsSettingsPage — a stuck certificate names the record it is waiting on', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('given a provisioning domain blocked on ownership, should show the record and value in the row', async () => { + serve(INSTRUCTION); + + renderPage(); + + // The whole point: visible without pressing "Check SSL" first. + expect(await screen.findByText(/_fly-ownership\.docs\.acme\.com/)).toBeInTheDocument(); + expect(screen.getByText(/org-XYZ789/)).toBeInTheDocument(); + }); + + // The case that would rot: the instruction is recomputed per request, never + // stored, so a customer who has just fixed their zone must stop seeing it on + // the very next load rather than being told to re-publish a record they have. + it('given the same domain with nothing owed, should show no instruction at all', async () => { + serve(null); + + // Wait for the row itself, so this is not asserting on an unrendered page. + renderPage(); + expect(await screen.findByText('docs.acme.com')).toBeInTheDocument(); + + expect(screen.queryByText(/_fly-ownership/)).not.toBeInTheDocument(); + expect(screen.queryByText(/waiting on a DNS record/i)).not.toBeInTheDocument(); + }); +}); diff --git a/apps/web/src/app/dashboard/[driveId]/settings/domains/page.tsx b/apps/web/src/app/dashboard/[driveId]/settings/domains/page.tsx index b976868a1a..29ec009eb0 100644 --- a/apps/web/src/app/dashboard/[driveId]/settings/domains/page.tsx +++ b/apps/web/src/app/dashboard/[driveId]/settings/domains/page.tsx @@ -38,6 +38,15 @@ interface CustomDomain { publishLandingPageId: string | null; /** Canvas page overriding this domain's 404.html; null = use the drive-wide 404 page. */ publishNotFoundPageId: string | null; + /** + * What the customer must publish in DNS for a certificate stuck on Fly's + * `_fly-ownership` TXT check, or null when nothing is owed. + * + * Recomputed by the list route on every load rather than stored — it would be + * stale the moment the customer fixed their zone. Optional because it is only + * present for a domain still in a non-terminal cert state. + */ + ownershipInstruction?: string | null; } interface DomainsResponse { @@ -237,7 +246,11 @@ export default function DomainsSettingsPage() { const res = await fetchWithAuth(`/api/drives/${driveId}/domains/${domainId}/cert/refresh`, { method: 'POST', }); - const data = await res.json().catch(() => ({})) as { status?: string; error?: string }; + const data = await res.json().catch(() => ({})) as { + status?: string; + error?: string; + ownershipInstruction?: string | null; + }; if (!res.ok) { if (res.status === 503) { toast.error('SSL provisioning is not yet configured'); @@ -249,6 +262,12 @@ export default function DomainsSettingsPage() { await mutateDomains(); if (data.status === 'active') { toast.success('SSL certificate is active'); + } else if (data.ownershipInstruction) { + // The certificate is blocked on a DNS record the customer has not + // published. "Check back in a few minutes" would be false here: this is + // the one waiting state that never resolves on its own, so it gets the + // actual instruction and a duration long enough to copy a record out of. + toast.warning(data.ownershipInstruction, { duration: 30_000 }); } else if (data.status === 'provisioning') { toast.success('SSL cert provisioned — check back in a few minutes'); } else if (data.status === 'cert_failed') { @@ -1077,6 +1096,35 @@ function DomainRow({

{verifyReason}

)} + {/* + A certificate waiting on an ownership TXT is the ONE cert state that never + resolves on its own, so the instruction has to be visible without the + customer first guessing to press "Check SSL". Rendered in the row, beside + the DNS records panel it mirrors, rather than only as a toast that takes + the record name and value away with it. + */} + {domain.status === 'provisioning' && domain.ownershipInstruction && ( +
+

+ SSL is waiting on a DNS record you still need to add: +

+ {/* + `break-words`, not `break-all`, and no `font-mono`: this is a prose + sentence with a hostname and a record value embedded in it, unlike the + DNS panel below, which is a table of bare field values. `break-all` + would chop ordinary words mid-character, and monospacing the whole + sentence makes it harder to read to save the few tokens that benefit. + `break-words` still wraps the long `_fly-ownership.` label rather + than letting it overflow the row. + */} +

{domain.ownershipInstruction}

+

+ Once it propagates, click Check SSL — the certificate cannot be issued until this + record resolves. +

+
+ )} + {showDns && (

diff --git a/apps/web/src/lib/canvas/__tests__/reconcile-cert.test.ts b/apps/web/src/lib/canvas/__tests__/reconcile-cert.test.ts index 24d7d851a0..2164cc61c6 100644 --- a/apps/web/src/lib/canvas/__tests__/reconcile-cert.test.ts +++ b/apps/web/src/lib/canvas/__tests__/reconcile-cert.test.ts @@ -13,8 +13,14 @@ vi.mock('@pagespace/lib/logging/logger-config', () => ({ })); const addCertificate = vi.fn(); +const recheckCertificate = vi.fn(); vi.mock('@/lib/fly/certs', () => ({ addCertificate: (...args: unknown[]) => addCertificate(...args), + recheckCertificate: (...args: unknown[]) => recheckCertificate(...args), + // Faithful to the real predicate — it accepts either credential. A stub that + // only looked at FLY_API_TOKEN would hide the very bug this replaced. + hasFlyCertCredential: () => + Boolean(process.env.FLY_API_TOKEN || process.env.FLY_MACHINES_ORG_TOKEN), })); const dbUpdate = vi.fn(); @@ -35,6 +41,11 @@ vi.mock('@/lib/canvas/custom-domain-mirror', () => ({ clearCustomHost: (...args: unknown[]) => clearCustomHost(...args), })); +const resolveTxtRecords = vi.fn().mockResolvedValue([]); +vi.mock('@/lib/publish/dns-resolver', () => ({ + resolveTxtRecords: (...args: unknown[]) => resolveTxtRecords(...args), +})); + const regeneratePublishedSiteFiles = vi.fn().mockResolvedValue(undefined); const renderDomainNotFoundOverride = vi.fn().mockResolvedValue(undefined); vi.mock('@/lib/canvas/publish-page', () => ({ @@ -53,7 +64,15 @@ function domain(status: string) { beforeEach(() => { vi.clearAllMocks(); + // A benign default: most cases never reach the re-check, and the ones that do + // set their own. Without a default the mock resolves undefined, which is a + // shape the real function can never return. + recheckCertificate.mockResolvedValue({ ok: false, error: 'recheck not stubbed' }); + resolveTxtRecords.mockResolvedValue([]); process.env.FLY_API_TOKEN = 'test-token'; + // Cleared so a test that exercises the fallback cannot leak it into the guard + // tests that follow, which assert the no-credential no-op. + delete process.env.FLY_MACHINES_ORG_TOKEN; process.env.FLY_PROXY_APP_NAME = 'pagespace-proxy'; setMock.mockReturnValue({ where: vi.fn().mockResolvedValue([]) }); dbUpdate.mockReturnValue({ set: setMock }); @@ -104,7 +123,7 @@ describe('reconcileCustomDomainCert — cert advance', () => { const result = await reconcileCustomDomainCert(domain('verified')); expect(addCertificate).toHaveBeenCalledWith('pagespace-proxy', 'docs.acme.com'); - expect(result).toEqual({ status: 'active', action: 'mark-active' }); + expect(result).toEqual({ status: 'active', action: 'mark-active', ownershipInstruction: null }); expect(setMock).toHaveBeenCalledWith({ status: 'active' }); expect(regeneratePublishedSiteFiles).toHaveBeenCalledWith(DRIVE_ID); expect(mirrorDriveToCustomHost).toHaveBeenCalledWith(DRIVE_ID, 'docs.acme.com', expect.any(Function)); @@ -116,7 +135,7 @@ describe('reconcileCustomDomainCert — cert advance', () => { const result = await reconcileCustomDomainCert(domain('verified')); - expect(result).toEqual({ status: 'provisioning', action: 'provision' }); + expect(result).toEqual({ status: 'provisioning', action: 'provision', ownershipInstruction: null }); expect(setMock).toHaveBeenCalledWith({ status: 'provisioning' }); expect(regeneratePublishedSiteFiles).not.toHaveBeenCalled(); expect(mirrorDriveToCustomHost).not.toHaveBeenCalled(); @@ -127,7 +146,7 @@ describe('reconcileCustomDomainCert — cert advance', () => { const result = await reconcileCustomDomainCert(domain('provisioning')); - expect(result).toEqual({ status: 'provisioning', action: 'poll-again' }); + expect(result).toEqual({ status: 'provisioning', action: 'poll-again', ownershipInstruction: null }); expect(mirrorDriveToCustomHost).not.toHaveBeenCalled(); }); @@ -146,7 +165,7 @@ describe('reconcileCustomDomainCert — cert advance', () => { const result = await reconcileCustomDomainCert(domain('verified')); - expect(result).toEqual({ status: 'cert_failed', action: 'mark-failed' }); + expect(result).toEqual({ status: 'cert_failed', action: 'mark-failed', ownershipInstruction: null }); expect(setMock).toHaveBeenCalledWith({ status: 'cert_failed' }); expect(clearCustomHost).toHaveBeenCalledWith('docs.acme.com'); expect(mirrorDriveToCustomHost).not.toHaveBeenCalled(); @@ -232,3 +251,180 @@ describe('reconcileCustomDomainCert — side effects never throw', () => { expect(result.status).toBe('active'); }); }); + +describe('reconcileCustomDomainCert — blocked on an _fly-ownership TXT', () => { + /** A pending cert for which Fly is asking for an ownership record. */ + const pendingWithOwnership = { + ok: true, + configured: false, + status: 'pending_validation', + ownership: { + name: '_fly-ownership.docs.acme.com', + appValue: 'app-ABC', + orgValue: 'org-XYZ', + }, + ownershipTxtConfigured: false, + }; + + it('given the record is not published, should stay provisioning and say what is missing', async () => { + addCertificate.mockResolvedValue(pendingWithOwnership); + resolveTxtRecords.mockResolvedValue([]); + + const result = await reconcileCustomDomainCert(domain('provisioning')); + + // Non-destructive: this is a domain that is FINE and merely waiting, so it + // must not be flipped to cert_failed or have its mirrored prefix cleared. + expect(result.status).toBe('provisioning'); + expect(result.action).toBe('blocked-on-ownership'); + expect(result.ownershipInstruction).toContain('_fly-ownership.docs.acme.com'); + expect(result.ownershipInstruction).toContain('app-ABC'); + expect(clearCustomHost).not.toHaveBeenCalled(); + }); + + it('given the record IS published, should carry no instruction and resume ordinary polling', async () => { + addCertificate.mockResolvedValue(pendingWithOwnership); + resolveTxtRecords.mockResolvedValue([['app-ABC']]); + + const result = await reconcileCustomDomainCert(domain('provisioning')); + + expect(result.action).toBe('poll-again'); + expect(result.ownershipInstruction).toBeNull(); + }); + + it('given the record is published at the ownership NAME, should resolve that name', async () => { + addCertificate.mockResolvedValue(pendingWithOwnership); + resolveTxtRecords.mockResolvedValue([['app-ABC']]); + + await reconcileCustomDomainCert(domain('provisioning')); + + expect(resolveTxtRecords).toHaveBeenCalledWith('_fly-ownership.docs.acme.com'); + }); + + it('given Fly has already SEEN the record, should skip the DNS read entirely', async () => { + addCertificate.mockResolvedValue({ ...pendingWithOwnership, ownershipTxtConfigured: true }); + + const result = await reconcileCustomDomainCert(domain('provisioning')); + + expect(resolveTxtRecords).not.toHaveBeenCalled(); + expect(result.action).toBe('poll-again'); + }); + + it('given Fly asked for no ownership record, should do no DNS work at all', async () => { + addCertificate.mockResolvedValue({ ok: true, configured: false }); + + await reconcileCustomDomainCert(domain('provisioning')); + + expect(resolveTxtRecords).not.toHaveBeenCalled(); + }); + + it('given the DNS lookup fails, should not turn a resolver outage into a cert failure', async () => { + addCertificate.mockResolvedValue(pendingWithOwnership); + resolveTxtRecords.mockRejectedValue(new Error('SERVFAIL')); + + const result = await reconcileCustomDomainCert(domain('provisioning')); + + expect(result.status).toBe('provisioning'); + expect(clearCustomHost).not.toHaveBeenCalled(); + }); + + it('given a LIVE certificate, should activate regardless of what any record says', async () => { + addCertificate.mockResolvedValue({ ...pendingWithOwnership, configured: true }); + + const result = await reconcileCustomDomainCert(domain('verified')); + + expect(result.status).toBe('active'); + expect(result.action).toBe('mark-active'); + }); +}); + +describe('reconcileCustomDomainCert — nudging Fly once the record is published', () => { + const pendingWithOwnership = { + ok: true, + configured: false, + status: 'pending_validation', + ownership: { name: '_fly-ownership.docs.acme.com', appValue: 'app-ABC', orgValue: 'org-XYZ' }, + ownershipTxtConfigured: false, + }; + + it('given our resolver sees the record but Fly has not, should ask Fly to re-read DNS', async () => { + addCertificate.mockResolvedValue(pendingWithOwnership); + resolveTxtRecords.mockResolvedValue([['app-ABC']]); + recheckCertificate.mockResolvedValue({ ...pendingWithOwnership, ownershipTxtConfigured: true }); + + await reconcileCustomDomainCert(domain('provisioning')); + + expect(recheckCertificate).toHaveBeenCalledWith('pagespace-proxy', 'docs.acme.com'); + }); + + it('given the re-check reports the cert now live, should activate on this pass', async () => { + addCertificate.mockResolvedValue(pendingWithOwnership); + resolveTxtRecords.mockResolvedValue([['app-ABC']]); + recheckCertificate.mockResolvedValue({ ok: true, configured: true, status: 'active' }); + + const result = await reconcileCustomDomainCert(domain('provisioning')); + + expect(result.status).toBe('active'); + }); + + it('given Fly has ALREADY seen the record, should not re-check', async () => { + addCertificate.mockResolvedValue({ ...pendingWithOwnership, ownershipTxtConfigured: true }); + + await reconcileCustomDomainCert(domain('provisioning')); + + expect(recheckCertificate).not.toHaveBeenCalled(); + }); + + it('given the record is still MISSING, should not re-check — there is nothing new to see', async () => { + addCertificate.mockResolvedValue(pendingWithOwnership); + resolveTxtRecords.mockResolvedValue([]); + + await reconcileCustomDomainCert(domain('provisioning')); + + expect(recheckCertificate).not.toHaveBeenCalled(); + }); + + it('given no ownership requirement at all, should not re-check', async () => { + addCertificate.mockResolvedValue({ ok: true, configured: false }); + + await reconcileCustomDomainCert(domain('provisioning')); + + expect(recheckCertificate).not.toHaveBeenCalled(); + }); + + it('given the re-check fails, should not let a Fly blip fail a healthy domain', async () => { + addCertificate.mockResolvedValue(pendingWithOwnership); + resolveTxtRecords.mockResolvedValue([['app-ABC']]); + recheckCertificate.mockResolvedValue({ ok: false, error: 'Fly API 500' }); + + const result = await reconcileCustomDomainCert(domain('provisioning')); + + expect(result.status).toBe('provisioning'); + expect(clearCustomHost).not.toHaveBeenCalled(); + }); +}); + +describe('reconcileCustomDomainCert — which Fly credential counts as configured', () => { + it('given only FLY_MACHINES_ORG_TOKEN, should still reconcile', async () => { + // The published-app deployment configures that token and not FLY_API_TOKEN. + // Gating on FLY_API_TOKEN alone made the documented fallback unreachable from + // here, so cert reconciliation silently never ran for exactly that setup. + delete process.env.FLY_API_TOKEN; + process.env.FLY_MACHINES_ORG_TOKEN = 'org-token'; + addCertificate.mockResolvedValue({ ok: true, configured: true }); + + const result = await reconcileCustomDomainCert(domain('verified')); + + expect(addCertificate).toHaveBeenCalled(); + expect(result.status).toBe('active'); + }); + + it('given NEITHER credential, should stay a no-op and never flip the domain', async () => { + delete process.env.FLY_API_TOKEN; + delete process.env.FLY_MACHINES_ORG_TOKEN; + + const result = await reconcileCustomDomainCert(domain('verified')); + + expect(addCertificate).not.toHaveBeenCalled(); + expect(result).toEqual({ status: 'verified', action: null }); + }); +}); diff --git a/apps/web/src/lib/canvas/reconcile-cert.ts b/apps/web/src/lib/canvas/reconcile-cert.ts index cc0905cd1f..9fb78a9e50 100644 --- a/apps/web/src/lib/canvas/reconcile-cert.ts +++ b/apps/web/src/lib/canvas/reconcile-cert.ts @@ -2,15 +2,54 @@ import 'server-only'; import { loggers } from '@pagespace/lib/logging/logger-config'; import { nextCertAction, certActionToDbStatus, isCertEligible } from '@pagespace/lib/canvas/cert-action'; -import type { CertAction, CertEligibleStatus } from '@pagespace/lib/canvas/cert-action'; -import { addCertificate } from '@/lib/fly/certs'; +import type { CertAction, CertEligibleStatus, FlyCertResponse } from '@pagespace/lib/canvas/cert-action'; +import { + describeOwnershipVerification, + verifyFlyOwnershipTxt, + type FlyOwnershipVerification, +} from '@pagespace/lib/validators/fly-ownership'; +import { resolveAppRouterFlyAppName } from '@pagespace/lib/services/app-hosting/routing-env'; +import { addCertificate, hasFlyCertCredential, recheckCertificate } from '@/lib/fly/certs'; +import { resolveTxtRecords } from '@/lib/publish/dns-resolver'; import { db } from '@pagespace/db/db'; import { eq } from '@pagespace/db/operators'; import { customDomains } from '@pagespace/db/schema/custom-domains'; import { mirrorDriveToCustomHost, clearCustomHost } from '@/lib/canvas/custom-domain-mirror'; import { regeneratePublishedSiteFiles, renderDomainNotFoundOverride } from '@/lib/canvas/publish-page'; -const FLY_APP_NAME = process.env.FLY_PROXY_APP_NAME ?? 'pagespace-proxy'; +/** + * Certs attach to the ROUTER app — the app Fly TLS-terminates for us. Resolved + * through `resolveAppRouterFlyAppName()` (which still falls back to + * `FLY_PROXY_APP_NAME`) rather than read here, so custom-domain certs and the + * published-app routing tier can never end up naming two different apps: a cert + * issued on app A does not terminate traffic arriving at app B. + */ +function routerAppName(): string { + return resolveAppRouterFlyAppName(); +} + +/** + * Pre-validate the `_fly-ownership` TXT record, when Fly asked for one. + * + * Runs ONLY when Fly reports an ownership requirement it has not already + * satisfied — the common case (a hostname whose A/AAAA already point at us + * validates by reachability) does no DNS work at all. Returns null when there is + * nothing to check, which `nextCertAction` reads as "no pre-validation was run" + * and treats exactly as it did before this existed. + * + * Never throws: a DNS failure here must not turn into a cert failure. The + * resolver already collapses NXDOMAIN/ENODATA/timeout to `[]`, and `[]` reads as + * `missing` — which is non-destructive (it maps to `provisioning`, see the + * `blocked-on-ownership` action) and merely means the next poll asks again. + */ +async function preValidateOwnership(flyCert: FlyCertResponse): Promise { + if (!flyCert.ok) return null; + if (flyCert.ownershipTxtConfigured) return null; + const requirement = flyCert.ownership ?? null; + if (!requirement) return null; + const records = await resolveTxtRecords(requirement.name).catch(() => [] as string[][]); + return verifyFlyOwnershipTxt({ requirement, records }); +} /** The minimal domain shape `reconcileCustomDomainCert` needs. */ export interface CertReconcileDomain { @@ -27,6 +66,15 @@ export interface CertReconcileResult { status: string; /** The cert action taken, or `null` when reconcile was a no-op. */ action: CertAction['action'] | null; + /** + * What the customer still has to do, when the certificate is blocked on a DNS + * record they have not published. Null whenever nothing is waiting on them. + * + * Returned rather than persisted: it is derived from Fly's live answer plus a + * live DNS read, so a column holding it would be stale the moment the customer + * fixed their zone — and the status column already records the state. + */ + ownershipInstruction?: string | null; } export interface CertReconcileOptions { @@ -81,15 +129,53 @@ export async function reconcileCustomDomainCert( return { status: domain.status, action: null }; } - if (!process.env.FLY_API_TOKEN) { + // Asked through the certs module so the FLY_MACHINES_ORG_TOKEN fallback it + // accepts is actually reachable from here; testing FLY_API_TOKEN directly made + // this bail in precisely the deployment the fallback exists for. + if (!hasFlyCertCredential()) { return { status: domain.status, action: null }; } if (!isCertEligible(domain.status)) { return { status: domain.status, action: null }; } - const flyCert = await addCertificate(FLY_APP_NAME, domain.hostname); - const action = nextCertAction(domain.status as CertEligibleStatus, flyCert); + const flyCert = await addCertificate(routerAppName(), domain.hostname); + let ownership = await preValidateOwnership(flyCert); + let cert = flyCert; + + // The customer published the record, but Fly has not noticed yet. + // + // This is the ONE state a re-check helps, and it is why `recheckCertificate` + // exists: `addCertificate` above resolves an existing hostname with a GET, and + // a GET does not make Fly re-read DNS — so without this the domain waits on + // Fly's own polling cadence even though everything it needs is already + // published. Narrow by construction: it fires only while our resolver can see + // an accepted value AND Fly still reports the TXT unconfigured, a window that + // closes as soon as Fly agrees. + if (ownership?.state === 'satisfied' && cert.ok && !cert.ownershipTxtConfigured) { + const rechecked = await recheckCertificate(routerAppName(), domain.hostname); + if (rechecked.ok) { + cert = rechecked; + ownership = await preValidateOwnership(rechecked); + } + // A failed re-check is deliberately ignored: it is an optimization on top of + // a state that already resolves on its own, and letting a Fly blip here turn + // into `mark-failed` would flip a healthy domain to cert_failed. + } + + const action = nextCertAction(domain.status as CertEligibleStatus, cert, ownership); + + // Blocked on a record the customer has not published: the domain is fine and + // the certificate will issue the moment it appears, so this stays at + // `provisioning` and clears nothing. Logged at warn because it is the one + // cert state that will NEVER resolve on its own — somebody has to be told. + if (action.action === 'blocked-on-ownership') { + loggers.api.warn('Fly certificate is waiting on an _fly-ownership TXT record', { + driveId: domain.driveId, + hostname: domain.hostname, + reason: action.reason, + }); + } // Non-destructive read path: a Fly error/timeout maps to `mark-failed`, which // would flip the domain to `cert_failed` and wipe its mirrored prefix. On the @@ -138,5 +224,9 @@ export async function reconcileCustomDomainCert( } } - return { status: nextStatus, action: action.action }; + return { + status: nextStatus, + action: action.action, + ownershipInstruction: ownership ? describeOwnershipVerification(ownership) : null, + }; } diff --git a/apps/web/src/lib/fly/__tests__/certs.test.ts b/apps/web/src/lib/fly/__tests__/certs.test.ts index abcbee3ca5..fc0acbc688 100644 --- a/apps/web/src/lib/fly/__tests__/certs.test.ts +++ b/apps/web/src/lib/fly/__tests__/certs.test.ts @@ -1,177 +1,278 @@ +/** + * apps/web's Fly certificate wrapper, after the port off GraphQL. + * + * The previous version of this file asserted the shape of hand-written GraphQL + * mutations against a stubbed global `fetch`. None of that survives the port: + * the transport is now the shared flaps client, so what is worth asserting is + * the BEHAVIOUR the callers depend on, which deliberately did not change — + * `FlyCertResponse`, its `configured` boolean, idempotence, and degrading to + * `ok: false` instead of throwing into a settings page. + * + * What is new, and is the reason for the port: `ownership` and `status` are + * carried through, so a certificate stuck on an `_fly-ownership` TXT can be + * told apart from one that is simply still validating. + */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -vi.stubGlobal('fetch', vi.fn()); +const { + getCertificateMock, + requestAcmeCertificateMock, + checkCertificateMock, + deleteCertificateMock, + StubFlapsError, +} = vi.hoisted(() => ({ + getCertificateMock: vi.fn(), + requestAcmeCertificateMock: vi.fn(), + checkCertificateMock: vi.fn(), + deleteCertificateMock: vi.fn(), + // Hoisted with the mocks: the module factory below references it, and a class + // declared at file scope is still in its temporal dead zone when that runs. + StubFlapsError: class extends Error { + constructor(message: string) { + super(message); + this.name = 'FlapsError'; + } + }, +})); + +vi.mock('@pagespace/lib/services/fly/flaps-client', () => ({ + FlapsError: StubFlapsError, + getCertificate: getCertificateMock, + requestAcmeCertificate: requestAcmeCertificateMock, + checkCertificate: checkCertificateMock, + deleteCertificate: deleteCertificateMock, +})); + +import { + addCertificate, + ownershipRequirementOf, + recheckCertificate, + removeCertificate, +} from '../certs'; -import { addCertificate, getCertificate } from '../certs'; - -const FLY_API_URL = 'https://api.fly.io/graphql'; -const TOKEN = 'fly-test-token'; const APP_NAME = 'pagespace-proxy'; const HOSTNAME = 'docs.acme.com'; -function mockFetchOk(body: unknown) { - (fetch as ReturnType).mockResolvedValueOnce({ - ok: true, - json: () => Promise.resolve(body), - } as Response); -} - -function mockFetchNetworkError() { - (fetch as ReturnType).mockRejectedValueOnce(new Error('network failure')); -} - -function mockFetchHttpError(status: number) { - (fetch as ReturnType).mockResolvedValueOnce({ - ok: false, - status, - json: () => Promise.resolve({ errors: [{ message: `HTTP ${status}` }] }), - } as unknown as Response); -} - -function addCertOk(clientStatus: string) { - return { - data: { addCertificate: { certificate: { configured: true, clientStatus, hostname: HOSTNAME } } }, - }; -} - -function getCertOk(clientStatus: string) { - return { - data: { app: { certificate: { configured: true, clientStatus, hostname: HOSTNAME } } }, - }; -} +const ACTIVE = { hostname: HOSTNAME, status: 'active', configured: true }; +const PENDING = { + hostname: HOSTNAME, + status: 'pending_validation', + configured: false, + dns_requirements: { + ownership: { name: `_fly-ownership.${HOSTNAME}`, app_value: 'app-ABC', org_value: 'org-XYZ' }, + }, + validation: { ownership_txt_configured: false }, +}; beforeEach(() => { vi.clearAllMocks(); - process.env.FLY_API_TOKEN = TOKEN; + process.env.FLY_API_TOKEN = 'fly-test-token'; }); afterEach(() => { delete process.env.FLY_API_TOKEN; + delete process.env.FLY_MACHINES_ORG_TOKEN; }); -describe('addCertificate', () => { - it('sends POST to Fly GraphQL with correct Authorization header', async () => { - mockFetchOk(addCertOk('Awaiting certificates')); +describe('addCertificate — reads before it writes', () => { + it('given a hostname Fly already has, should return its state without requesting a new cert', async () => { + getCertificateMock.mockResolvedValueOnce(ACTIVE); - await addCertificate(APP_NAME, HOSTNAME); + const result = await addCertificate(APP_NAME, HOSTNAME); - expect(fetch).toHaveBeenCalledOnce(); - const [url, init] = (fetch as ReturnType).mock.calls[0] as [string, RequestInit]; - expect(url).toBe(FLY_API_URL); - expect((init.headers as Record)['Authorization']).toBe(`Bearer ${TOKEN}`); - expect((init.headers as Record)['Content-Type']).toBe('application/json'); - expect(init.method).toBe('POST'); + expect(result).toEqual({ + ok: true, + configured: true, + status: 'active', + ownership: null, + ownershipTxtConfigured: false, + }); + // The poll cycle this serves runs on every domains-list load; it must not be + // a stream of mutations. + expect(requestAcmeCertificateMock).not.toHaveBeenCalled(); }); - it('passes a bounded AbortSignal (timeout) to fetch so a hung Fly response cannot block the caller', async () => { - mockFetchOk(addCertOk('Awaiting certificates')); + it('given a hostname Fly has never seen, should request an ACME certificate', async () => { + getCertificateMock.mockResolvedValueOnce(null); + requestAcmeCertificateMock.mockResolvedValueOnce(PENDING); - await addCertificate(APP_NAME, HOSTNAME); + const result = await addCertificate(APP_NAME, HOSTNAME); - const [, init] = (fetch as ReturnType).mock.calls[0] as [string, RequestInit]; - expect(init.signal).toBeInstanceOf(AbortSignal); + expect(requestAcmeCertificateMock).toHaveBeenCalledWith( + expect.objectContaining({ token: 'fly-test-token' }), + APP_NAME, + HOSTNAME, + ); + expect(result).toMatchObject({ ok: true, configured: false, status: 'pending_validation' }); }); - it('returns ok:false when the Fly request times out (aborted)', async () => { - (fetch as ReturnType).mockRejectedValueOnce( - Object.assign(new Error('The operation timed out.'), { name: 'TimeoutError' }), - ); + it('given a pending cert, should surface the ownership record the customer still owes', async () => { + getCertificateMock.mockResolvedValueOnce(PENDING); const result = await addCertificate(APP_NAME, HOSTNAME); - expect(result.ok).toBe(false); - if (!result.ok) expect(result.error).toMatch(/timed out/i); + expect(result).toEqual({ + ok: true, + configured: false, + status: 'pending_validation', + ownership: { name: `_fly-ownership.${HOSTNAME}`, appValue: 'app-ABC', orgValue: 'org-XYZ' }, + ownershipTxtConfigured: false, + }); }); - it('sends the mutation with appId (not appName) and hostname variables', async () => { - mockFetchOk(addCertOk('Awaiting certificates')); + it('given Fly reports the ownership TXT as seen, should say so', async () => { + getCertificateMock.mockResolvedValueOnce({ + ...PENDING, + validation: { ownership_txt_configured: true }, + }); - await addCertificate(APP_NAME, HOSTNAME); + const result = await addCertificate(APP_NAME, HOSTNAME); + expect(result).toMatchObject({ ok: true, ownershipTxtConfigured: true }); + }); - const [, init] = (fetch as ReturnType).mock.calls[0] as [string, RequestInit]; - const body = JSON.parse(init.body as string) as { query: string; variables: Record }; - expect(body.variables.appId).toBe(APP_NAME); - expect(body.variables.hostname).toBe(HOSTNAME); - expect(body.variables.appName).toBeUndefined(); - expect(body.query).toContain('addCertificate(appId: $appId'); + it('given a cert that is DNS-configured but not yet ISSUED, should not call it configured', async () => { + // The distinction the old `clientStatus === 'Ready'` check drew, preserved + // across the port: Fly's own `configured` boolean reflects DNS, and a + // hostname can be correctly configured for minutes before a certificate + // actually issues. Serving on the strength of it would mean serving without + // TLS. + getCertificateMock.mockResolvedValueOnce({ + hostname: HOSTNAME, + status: 'pending_validation', + configured: true, + }); + expect(await addCertificate(APP_NAME, HOSTNAME)).toMatchObject({ ok: true, configured: false }); }); - it('returns ok:true configured:false while the cert is not yet Ready', async () => { - mockFetchOk(addCertOk('Awaiting configuration')); - const result = await addCertificate(APP_NAME, HOSTNAME); - expect(result).toEqual({ ok: true, configured: false }); + it('given a Fly failure, should degrade to ok:false rather than throw into the settings page', async () => { + getCertificateMock.mockRejectedValueOnce(new StubFlapsError('Fly Machines API 403')); + + expect(await addCertificate(APP_NAME, HOSTNAME)).toEqual({ + ok: false, + error: 'Fly Machines API 403', + }); }); - it('returns ok:true configured:true when clientStatus is Ready', async () => { - mockFetchOk(addCertOk('Ready')); - const result = await addCertificate(APP_NAME, HOSTNAME); - expect(result).toEqual({ ok: true, configured: true }); + it('given a non-Error rejection, should still report an error response', async () => { + getCertificateMock.mockRejectedValueOnce('something odd'); + expect(await addCertificate(APP_NAME, HOSTNAME)).toEqual({ + ok: false, + error: 'Unknown Fly API error', + }); }); +}); - it('treats "Hostname already exists" as non-fatal and reads the existing cert status', async () => { - // 1st call: addCertificate → "already exists" error - mockFetchOk({ data: null, errors: [{ message: 'Hostname already exists on app' }] }); - // 2nd call: getCertificate → Ready - mockFetchOk(getCertOk('Ready')); +describe('the token is required before any request is attempted', () => { + it('given no token at all, should report it without spending a request on a guaranteed 401', async () => { + delete process.env.FLY_API_TOKEN; const result = await addCertificate(APP_NAME, HOSTNAME); - expect(result).toEqual({ ok: true, configured: true }); - expect(fetch).toHaveBeenCalledTimes(2); - const [, secondInit] = (fetch as ReturnType).mock.calls[1] as [string, RequestInit]; - const secondBody = JSON.parse(secondInit.body as string) as { query: string; variables: Record }; - expect(secondBody.query).toContain('app(name: $appName)'); - expect(secondBody.variables.appName).toBe(APP_NAME); + expect(result).toEqual({ + ok: false, + error: 'Neither FLY_API_TOKEN nor FLY_MACHINES_ORG_TOKEN is configured', + }); + expect(getCertificateMock).not.toHaveBeenCalled(); }); - it('returns ok:false when FLY_API_TOKEN is absent', async () => { + it('given only FLY_MACHINES_ORG_TOKEN, should use it as the fallback credential', async () => { delete process.env.FLY_API_TOKEN; - const result = await addCertificate(APP_NAME, HOSTNAME); - expect(result.ok).toBe(false); - expect(fetch).not.toHaveBeenCalled(); + process.env.FLY_MACHINES_ORG_TOKEN = 'org-token'; + getCertificateMock.mockResolvedValueOnce(ACTIVE); + + await addCertificate(APP_NAME, HOSTNAME); + + expect(getCertificateMock).toHaveBeenCalledWith( + expect.objectContaining({ token: 'org-token' }), + APP_NAME, + HOSTNAME, + ); }); +}); - it('returns ok:false on network error', async () => { - mockFetchNetworkError(); - const result = await addCertificate(APP_NAME, HOSTNAME); - expect(result.ok).toBe(false); - if (!result.ok) expect(result.error).toMatch(/network failure/); +describe('recheckCertificate — the endpoint behind "Check SSL"', () => { + it('given a hostname, should ask Fly to re-read its DNS', async () => { + checkCertificateMock.mockResolvedValueOnce(ACTIVE); + expect(await recheckCertificate(APP_NAME, HOSTNAME)).toMatchObject({ ok: true, configured: true }); + expect(checkCertificateMock).toHaveBeenCalledWith(expect.anything(), APP_NAME, HOSTNAME); }); - it('returns ok:false on HTTP error', async () => { - mockFetchHttpError(401); - const result = await addCertificate(APP_NAME, HOSTNAME); - expect(result.ok).toBe(false); + it('given Fly does not have the hostname, should report no certificate', async () => { + checkCertificateMock.mockResolvedValueOnce(null); + expect(await recheckCertificate(APP_NAME, HOSTNAME)).toEqual({ + ok: false, + error: 'Fly did not return a certificate', + }); }); +}); + +describe('removeCertificate — a hostname left attached bills forever', () => { + it('given an attached hostname, should detach it', async () => { + deleteCertificateMock.mockResolvedValueOnce(undefined); + expect(await removeCertificate(APP_NAME, HOSTNAME)).toEqual({ ok: true }); + }); + + it('given the delete fails, should report the failure rather than assume removal', async () => { + deleteCertificateMock.mockRejectedValueOnce(new StubFlapsError('Fly Machines API 403')); + expect(await removeCertificate(APP_NAME, HOSTNAME)).toEqual({ + ok: false, + error: 'Fly Machines API 403', + }); + }); + + it('given no token, should report it', async () => { + delete process.env.FLY_API_TOKEN; + expect(await removeCertificate(APP_NAME, HOSTNAME)).toEqual({ + ok: false, + error: 'Neither FLY_API_TOKEN nor FLY_MACHINES_ORG_TOKEN is configured', + }); + }); + + // The message is read by an operator deciding WHICH variable to set, and + // `resolveToken` accepts either. A message naming only FLY_API_TOKEN tells + // someone on a published-app deployment — which configures only + // FLY_MACHINES_ORG_TOKEN — that the variable they set was not the problem. + it.each([ + ['addCertificate', () => addCertificate(APP_NAME, HOSTNAME)], + ['removeCertificate', () => removeCertificate(APP_NAME, HOSTNAME)], + ])('given no credential, %s should name BOTH accepted variables', async (_label, call) => { + delete process.env.FLY_API_TOKEN; + delete process.env.FLY_MACHINES_ORG_TOKEN; + + const result = await call(); - it('returns ok:false on a non-"already exists" GraphQL error', async () => { - mockFetchOk({ data: null, errors: [{ message: 'app not found' }] }); - const result = await addCertificate(APP_NAME, HOSTNAME); expect(result.ok).toBe(false); - if (!result.ok) expect(result.error).toMatch(/app not found/); + const error = result.ok ? '' : result.error; + expect(error).toContain('FLY_API_TOKEN'); + expect(error).toContain('FLY_MACHINES_ORG_TOKEN'); }); }); -describe('getCertificate', () => { - it('queries app(name:) and maps Ready → configured:true', async () => { - mockFetchOk(getCertOk('Ready')); - const result = await getCertificate(APP_NAME, HOSTNAME); - expect(result).toEqual({ ok: true, configured: true }); - const [, init] = (fetch as ReturnType).mock.calls[0] as [string, RequestInit]; - const body = JSON.parse(init.body as string) as { variables: Record }; - expect(body.variables.appName).toBe(APP_NAME); - expect(body.variables.hostname).toBe(HOSTNAME); +describe('ownershipRequirementOf — a half-populated requirement names nothing actionable', () => { + it('given no ownership block, should return null', () => { + expect(ownershipRequirementOf(ACTIVE)).toBeNull(); }); - it('maps a non-Ready status to configured:false', async () => { - mockFetchOk(getCertOk('Awaiting configuration')); - const result = await getCertificate(APP_NAME, HOSTNAME); - expect(result).toEqual({ ok: true, configured: false }); + it('given a complete requirement, should normalize it', () => { + expect(ownershipRequirementOf(PENDING)).toEqual({ + name: `_fly-ownership.${HOSTNAME}`, + appValue: 'app-ABC', + orgValue: 'org-XYZ', + }); }); - it('returns ok:false when the app/cert is missing', async () => { - mockFetchOk({ data: { app: { certificate: null } } }); - const result = await getCertificate(APP_NAME, HOSTNAME); - expect(result.ok).toBe(false); + it('given only an org value, should still be actionable', () => { + expect( + ownershipRequirementOf({ + dns_requirements: { ownership: { name: 'n', org_value: 'org-XYZ' } }, + }), + ).toEqual({ name: 'n', appValue: '', orgValue: 'org-XYZ' }); + }); + + it.each([ + ['no name', { name: '', app_value: 'app-ABC' }], + ['no values', { name: 'n' }], + ])('given a requirement with %s, should return null rather than an instruction with blanks in it', (_l, ownership) => { + expect(ownershipRequirementOf({ dns_requirements: { ownership } })).toBeNull(); }); }); diff --git a/apps/web/src/lib/fly/certs.ts b/apps/web/src/lib/fly/certs.ts index f353d9ae3d..d134362388 100644 --- a/apps/web/src/lib/fly/certs.ts +++ b/apps/web/src/lib/fly/certs.ts @@ -1,120 +1,189 @@ -import type { FlyCertResponse } from '@pagespace/lib/canvas/cert-action'; +/** + * Fly TLS certificates for custom domains — the REST certificates resource. + * + * PORTED OFF GRAPHQL. This module used to POST hand-written mutations to + * `api.fly.io/graphql` (`addCertificate(appId:)`, `app(name:){certificate}`). + * Two things were wrong with that beyond it being the legacy API: + * + * 1. The GraphQL response is `{configured, clientStatus, hostname}` and nothing + * more, so a certificate stuck in validation was indistinguishable from one + * about to issue — the customer got "not configured yet" forever with no + * instruction. The REST resource returns `dns_requirements` and + * `validation`, which name the exact records that are missing, including + * the `_fly-ownership` TXT that has no GraphQL equivalent at all. + * 2. It carried its own bespoke `fetch`, timeout and error parsing, duplicating + * what `services/fly/flaps-client.ts` already does properly — including + * Fly's per-object rate limiting (~1 r/s, burst 3), which this path hit + * unprotected every time the domains list lazily reconciled several rows. + * + * So the transport is now the shared flaps client and the shape of the answer is + * unchanged: `FlyCertResponse` is still what `nextCertAction` consumes, extended + * additively with the ownership fields. Existing callers do not change. + * + * TOKEN: `FLY_API_TOKEN` stays the primary credential so no deployment has to be + * reconfigured for this port; `FLY_MACHINES_ORG_TOKEN` is accepted as a fallback + * because it is the same class of org-scoped credential and a deployment that has + * configured published-app hosting has already set it. + */ -const FLY_API_URL = 'https://api.fly.io/graphql'; +import type { FlyCertResponse } from '@pagespace/lib/canvas/cert-action'; +import type { FlyOwnershipRequirement } from '@pagespace/lib/validators/fly-ownership'; +import { + checkCertificate, + deleteCertificate, + getCertificate as getFlyCertificate, + requestAcmeCertificate, + FlapsError, + type FlapsTransport, + type FlyCertificate, +} from '@pagespace/lib/services/fly/flaps-client'; -// Bound the Fly request so a hung response can't block the caller indefinitely. -// addCertificate is invoked from the domains-list GET (lazy cert reconcile), so -// an unbounded fetch would stall the settings UI. On timeout the AbortSignal -// rejects the fetch and we degrade to an ok:false error response. -const FLY_API_TIMEOUT_MS = 10_000; +/** A certificate is live and servable when Fly reports its status as active. */ +const CERT_ACTIVE_STATUS = 'active'; -// Fly's GraphQL `addCertificate` takes `appId` (an ID! whose value is the app -// NAME), NOT `appName` — passing `appName` is rejected with a schema error, so -// this mutation never succeeded until this was corrected. -const ADD_CERTIFICATE_MUTATION = ` - mutation AddCertificate($appId: ID!, $hostname: String!) { - addCertificate(appId: $appId, hostname: $hostname) { - certificate { - configured - clientStatus - hostname - } - } - } -`; +function resolveToken(): string { + return process.env.FLY_API_TOKEN || process.env.FLY_MACHINES_ORG_TOKEN || ''; +} -// Reading an existing cert's status uses `app(name:)` (a String!, not the ID! -// that addCertificate wants). Used when a cert already exists on the app. -const GET_CERTIFICATE_QUERY = ` - query GetCertificate($appName: String!, $hostname: String!) { - app(name: $appName) { - certificate(hostname: $hostname) { - configured - clientStatus - hostname - } - } - } -`; +/** + * Whether a Fly credential is configured at all. + * + * Exported because callers gate on it BEFORE doing cert work — the lazy + * reconcile and the "Check SSL" route both bail early when Fly is unconfigured, + * rather than marking a healthy domain failed. Those checks must ask the same + * question {@link resolveToken} answers: they used to test `FLY_API_TOKEN` + * directly, which made the `FLY_MACHINES_ORG_TOKEN` fallback unreachable in + * exactly the published-app deployment that configures only that one. + */ +export function hasFlyCertCredential(): boolean { + return resolveToken().length > 0; +} -// A Fly cert is live/servable when its clientStatus is "Ready". The boolean -// `configured` field only reflects DNS configuration, not issuance, so we key -// "active" off clientStatus. -const CERT_READY_STATUS = 'Ready'; +/** + * The transport, or null when no credential is configured. + * + * Null rather than an empty-token transport: an unauthenticated request to Fly + * would spend the retry budget on three guaranteed 401s before reporting a + * failure that was knowable before the first one. + */ +function transportOrNull(): FlapsTransport | null { + const token = resolveToken(); + return token ? { token } : null; +} -type CertNode = { configured: boolean; clientStatus: string; hostname: string } | null; -type AddCertData = { addCertificate: { certificate: CertNode } | null }; -type GetCertData = { app: { certificate: CertNode } | null }; +/** + * The message for "Fly is not configured at all". + * + * Names BOTH variables, because {@link resolveToken} accepts either and the + * message is read by an operator deciding which one to set. Naming only + * `FLY_API_TOKEN` — as this did — tells someone debugging a leaked certificate + * charge on a published-app deployment, which configures only + * `FLY_MACHINES_ORG_TOKEN`, that the variable they set was not the missing + * piece. Stated once so the two call sites cannot drift back apart. + */ +const NO_CREDENTIAL_ERROR = + 'Neither FLY_API_TOKEN nor FLY_MACHINES_ORG_TOKEN is configured'; -async function flyGraphQL( - query: string, - variables: Record, -): Promise<{ data: T } | { error: string }> { - const token = process.env.FLY_API_TOKEN ?? null; - if (!token) { - return { error: 'FLY_API_TOKEN is not configured' }; - } +const NO_TOKEN: FlyCertResponse = { + ok: false, + error: NO_CREDENTIAL_ERROR, +}; - try { - const response = await fetch(FLY_API_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${token}`, - }, - body: JSON.stringify({ query, variables }), - signal: AbortSignal.timeout(FLY_API_TIMEOUT_MS), - }); +/** Normalize Fly's ownership requirement, dropping a half-populated one. */ +export function ownershipRequirementOf(cert: FlyCertificate): FlyOwnershipRequirement | null { + const ownership = cert.dns_requirements?.ownership; + if (!ownership) return null; + const name = typeof ownership.name === 'string' ? ownership.name : ''; + const appValue = typeof ownership.app_value === 'string' ? ownership.app_value : ''; + const orgValue = typeof ownership.org_value === 'string' ? ownership.org_value : ''; + // A requirement with no name and no value names nothing the customer can act + // on; reporting it would produce an instruction with blanks in it. + if (!name || (!appValue && !orgValue)) return null; + return { name, appValue, orgValue }; +} - if (!response.ok) { - let msg = `Fly API HTTP ${response.status}`; - try { - const body = (await response.json()) as { errors?: Array<{ message: string }> }; - if (body.errors?.[0]?.message) msg = body.errors[0].message; - } catch { - // ignore parse failure - } - return { error: msg }; - } +/** Map a Fly certificate onto the response shape `nextCertAction` consumes. */ +function certToResponse(cert: FlyCertificate | null): FlyCertResponse { + if (!cert) return { ok: false, error: 'Fly did not return a certificate' }; + return { + ok: true, + // Keyed off `status`, not the `configured` boolean: `configured` reflects DNS + // configuration, and a hostname can be correctly configured for minutes + // before a certificate is actually issued. This is the same distinction the + // GraphQL path drew with `clientStatus === 'Ready'`. + configured: cert.status === CERT_ACTIVE_STATUS, + status: typeof cert.status === 'string' ? cert.status : undefined, + ownership: ownershipRequirementOf(cert), + ownershipTxtConfigured: cert.validation?.ownership_txt_configured === true, + }; +} - const body = (await response.json()) as { data: T; errors?: Array<{ message: string }> }; - if (body.errors?.length) { - return { error: body.errors.map((e) => e.message).join('; ') }; - } +/** Turn a thrown Flaps failure into the module's error response. */ +function toErrorResponse(err: unknown): FlyCertResponse { + if (err instanceof FlapsError) return { ok: false, error: err.message }; + return { ok: false, error: err instanceof Error ? err.message : 'Unknown Fly API error' }; +} - return { data: body.data }; +/** + * Ensure a certificate exists for `hostname` on `appName`, and report its state. + * + * READS BEFORE IT WRITES, which the GraphQL version could not do cheaply: a GET + * that finds an existing certificate returns its full validation state without + * asking Fly to request anything, so the poll cycle this function serves (the + * domains-list lazy reconcile calls it on every load) stops being a stream of + * mutations. Only a hostname Fly has never seen reaches the ACME request. + * + * Idempotent either way — `requestAcmeCertificate` resolves an "already exists" + * race back to the existing certificate — so concurrent reconciles converge. + */ +export async function addCertificate(appName: string, hostname: string): Promise { + const transport = transportOrNull(); + if (!transport) return NO_TOKEN; + try { + const existing = await getFlyCertificate(transport, appName, hostname); + if (existing) return certToResponse(existing); + return certToResponse(await requestAcmeCertificate(transport, appName, hostname)); } catch (err) { - return { error: err instanceof Error ? err.message : 'Unknown Fly API error' }; + return toErrorResponse(err); } } -/** Map a Fly cert node to our response; `configured` means "Ready/live". */ -function certToResponse(cert: CertNode): FlyCertResponse { - if (!cert) return { ok: false, error: 'Fly did not return a certificate' }; - return { ok: true, configured: cert.clientStatus === CERT_READY_STATUS }; -} - /** - * Request a TLS certificate from Fly for the given hostname on the given app. + * Ask Fly to RE-READ the hostname's DNS and re-evaluate validation. * - * Idempotent: if the cert already exists (Fly returns "Hostname already exists - * on app"), that is NOT a failure — we read the existing cert's status instead, - * so re-provision / poll cycles converge to active. + * The endpoint behind "the customer says they added the record". Without it, a + * hostname whose DNS was fixed sits at Fly's own polling cadence; with it, the + * settings UI's "Check SSL" actually checks. */ -export async function addCertificate(appName: string, hostname: string): Promise { - const result = await flyGraphQL(ADD_CERTIFICATE_MUTATION, { appId: appName, hostname }); - if ('error' in result) { - if (/already exists/i.test(result.error)) { - return getCertificate(appName, hostname); - } - return { ok: false, error: result.error }; +export async function recheckCertificate(appName: string, hostname: string): Promise { + const transport = transportOrNull(); + if (!transport) return NO_TOKEN; + try { + return certToResponse(await checkCertificate(transport, appName, hostname)); + } catch (err) { + return toErrorResponse(err); } - return certToResponse(result.data.addCertificate?.certificate ?? null); } -/** Read the status of an existing cert for a hostname on the given app. */ -export async function getCertificate(appName: string, hostname: string): Promise { - const result = await flyGraphQL(GET_CERTIFICATE_QUERY, { appName, hostname }); - if ('error' in result) return { ok: false, error: result.error }; - return certToResponse(result.data.app?.certificate ?? null); +/** + * Detach a hostname from the router app. + * + * Certificates bill per hostname ($0.10/mo beyond the first ten), so a domain + * removed from a drive has to be removed from Fly too or it bills forever with + * nothing in our database pointing at it. Idempotent: a hostname Fly does not + * have is already in the desired state. + */ +export async function removeCertificate( + appName: string, + hostname: string, +): Promise<{ ok: true } | { ok: false; error: string }> { + const transport = transportOrNull(); + if (!transport) return { ok: false, error: NO_CREDENTIAL_ERROR }; + try { + await deleteCertificate(transport, appName, hostname); + return { ok: true }; + } catch (err) { + const mapped = toErrorResponse(err); + return mapped.ok ? { ok: true } : { ok: false, error: mapped.error }; + } } diff --git a/apps/web/src/lib/publish/__tests__/dns-resolver.test.ts b/apps/web/src/lib/publish/__tests__/dns-resolver.test.ts index b61558d059..b812ee49e7 100644 --- a/apps/web/src/lib/publish/__tests__/dns-resolver.test.ts +++ b/apps/web/src/lib/publish/__tests__/dns-resolver.test.ts @@ -72,6 +72,26 @@ beforeEach(() => { }); describe('resolveHostname — authoritative DNS', () => { + it('given a zone answering with a huge NS RRset, should follow only a bounded number of them', async () => { + // The NS RRset comes from a zone the requester controls, and its COUNT is as + // attacker-controlled as its content: each entry costs one outbound resolve4 + // that can sit for the full DNS timeout. Without a cap, one authenticated + // verify request fans out to as many queries as the zone cares to list. + const huge = Array.from({ length: 200 }, (_, i) => `ns${i}.hostile.test`); + resolveNsImpl.mockResolvedValue(huge); + resolve4Impl.mockImplementation(async (_servers, host) => + host.endsWith('.hostile.test') ? ['198.51.100.7'] : [], + ); + resolve6Impl.mockResolvedValue([]); + resolveCnameImpl.mockResolvedValue([]); + + await resolveHostname('hostile.test'); + + const nsLookups = resolve4Impl.mock.calls.filter(([, host]) => host.endsWith('.hostile.test')); + // Counted, not merely "fewer than 200": the point is a fixed ceiling. + expect(nsLookups).toHaveLength(8); + }); + it('queries the authoritative nameservers (not the public resolver) for the target', async () => { const NS_IPS = ['9.9.9.1', '9.9.9.2']; resolveNsImpl.mockResolvedValue(['ns1.registrar-servers.com', 'ns2.registrar-servers.com']); diff --git a/apps/web/src/lib/publish/dns-resolver.ts b/apps/web/src/lib/publish/dns-resolver.ts index 23617fbc66..9aa0d50e00 100644 --- a/apps/web/src/lib/publish/dns-resolver.ts +++ b/apps/web/src/lib/publish/dns-resolver.ts @@ -36,6 +36,12 @@ const PUBLIC_DNS_SERVERS = ['1.1.1.1', '8.8.8.8', '9.9.9.9']; /** Per-lookup timeout so a dead/slow NS can't hang the verify request. */ const DNS_TIMEOUT_MS = 5000; const DNS_TRIES = 2; +/** + * How many of a zone's nameservers we will follow. Real delegations carry a + * handful; the cap exists because the RRset comes from an attacker-controlled + * zone and each entry costs an outbound lookup. + */ +const MAX_NS_HOSTS = 8; function makeResolver(servers: string[]): Resolver { const resolver = new Resolver({ timeout: DNS_TIMEOUT_MS, tries: DNS_TRIES }); @@ -56,6 +62,13 @@ function makeResolver(servers: string[]): Resolver { * would otherwise turn this verifier into an SSRF vector that fires DNS queries * at internal hosts. Filtered-out IPs simply drop us to the public-resolver * fallback. + * + * The NS RRset is attacker-controlled in COUNT as well as content, and each + * entry costs a concurrent `resolve4` (up to DNS_TIMEOUT_MS x DNS_TRIES). A + * hostile zone answering with a large RRset would turn one authenticated verify + * request into that many outbound queries, so only the first MAX_NS_HOSTS are + * followed. That loses nothing real: a delegation needs a couple of nameservers + * to be reachable, not all of them, and any honest zone is far under the cap. */ async function resolveAuthoritativeNsIps(hostname: string, publicResolver: Resolver): Promise { const domain = registrableDomain(hostname); @@ -65,7 +78,7 @@ async function resolveAuthoritativeNsIps(hostname: string, publicResolver: Resol if (nsHosts.length === 0) return []; const ipLists = await Promise.all( - nsHosts.map((ns) => publicResolver.resolve4(ns).catch(() => [] as string[])), + nsHosts.slice(0, MAX_NS_HOSTS).map((ns) => publicResolver.resolve4(ns).catch(() => [] as string[])), ); // De-dupe, and only ever hand globally-routable public IPs to setServers. return [...new Set(ipLists.flat())].filter(isPublicIp); @@ -115,3 +128,38 @@ export async function resolveHostname(hostname: string): Promise255-byte value arrives chunked and only the caller knows whether + * the chunks concatenate or are separate values. Never throws: NXDOMAIN, + * ENODATA and timeouts all surface as `[]` ("not set yet"). + */ +export async function resolveTxtRecords(hostname: string): Promise { + const publicResolver = makeResolver(PUBLIC_DNS_SERVERS); + + const nsIps = await resolveAuthoritativeNsIps(hostname, publicResolver).catch(() => [] as string[]); + if (nsIps.length > 0) { + const authoritativeResolver = makeResolver(nsIps); + const records = await authoritativeResolver.resolveTxt(hostname).catch(() => [] as string[][]); + if (records.length > 0) return records; + } + + return publicResolver.resolveTxt(hostname).catch(() => [] as string[][]); +} diff --git a/apps/web/src/middleware.ts b/apps/web/src/middleware.ts index af1a24e69c..7013ad191d 100644 --- a/apps/web/src/middleware.ts +++ b/apps/web/src/middleware.ts @@ -6,7 +6,8 @@ import { createSecureResponse, createSecureRewrite, createSecureErrorResponse, - isHandoffBridgeRoute, + APP_ROUTER_ROUTE_PATH, + routeOwnsItsOwnCsp, isPublicPageRoute, isPublishedSiteHost, isSecureRequest, @@ -152,6 +153,38 @@ export async function middleware(req: NextRequest, event?: NextFetchEvent) { return response; } + // Published-app serving edge: pagespace-proxy calls this for EVERY request to + // a published app, with no session and no user — it authenticates via the + // APP_ROUTER_PROXY_SECRET shared secret checked inside the route, which + // refuses everything when that secret is unset. + // + // Returned HERE, above origin validation and above the Bearer-API OPTIONS + // short-circuit, and both of those positions are load-bearing: + // + // • Origin validation is INAPPLICABLE. Valid callers are arbitrary + // published-app hosts and their custom domains, with no fixed allowlist — + // a published app's own fetch carries its own origin, which is not and can + // never be in ours. Same rationale as the public-form route above. + // • OPTIONS must REACH the route rather than be answered by the preflight + // short-circuit below. A CORS preflight for a published app belongs to + // that app and has to be replayed to it; answering it here would hand the + // browser our CORS policy instead of the app's, so a published app could + // never allow a custom request header on a cross-origin call. + // + // Without this the middleware also 401s the proxy before route.ts runs, and + // no published app is reachable at all. + if (pathname === APP_ROUTER_ROUTE_PATH) { + // The route delivers its own CSP for the styled parked/unavailable pages it + // renders; ours would intersect with and clobber it. Asked through the + // shared predicate rather than hardcoded `true` so there is one list of + // self-CSP routes rather than two places to keep in step. + const { response } = createSecureResponse(isProduction, req, { + isAPIRoute: true, + skipCSP: routeOwnsItsOwnCsp(pathname), + }); + return response; + } + const ip = getClientIP(req); // CORS preflight for the Bearer-authenticated API surface (@pagespace/sdk and @@ -316,9 +349,12 @@ export async function middleware(req: NextRequest, event?: NextFetchEvent) { // Handoff-bridge OAuth callbacks (google/apple) return their own styled HTML // with a bespoke CSP — skip the middleware CSP so it doesn't intersect with // and clobber the route's policy (which allows the page's inline styles). + // Asked through `routeOwnsItsOwnCsp` so the set of self-CSP routes has one + // definition; the published-app router is also in that set but returns + // above and never reaches this branch. const { response } = createSecureResponse(isProduction, req, { isAPIRoute, - skipCSP: isHandoffBridgeRoute(pathname), + skipCSP: routeOwnsItsOwnCsp(pathname), }); return response; } diff --git a/apps/web/src/middleware/__tests__/app-router-path-seam.test.ts b/apps/web/src/middleware/__tests__/app-router-path-seam.test.ts new file mode 100644 index 0000000000..0a42f7918a --- /dev/null +++ b/apps/web/src/middleware/__tests__/app-router-path-seam.test.ts @@ -0,0 +1,48 @@ +/** + * The middleware carve-out has to name the route that actually exists. + * + * `APP_ROUTER_ROUTE_PATH` is the ONLY thing tying two independent decisions + * together: middleware.ts exempts that path from the session check, and + * `routeOwnsItsOwnCsp` exempts it from the API CSP. Because the exemption + * removes authentication entirely for that path, the route's own shared-secret + * check is the only gate left — so the constant pointing at the wrong place is + * not a cosmetic bug in either direction: + * + * • constant BROADER than the route → more paths lose their session check. + * Covered by middleware.test.ts's sibling-path test. + * • constant no longer matching the route's location → the real endpoint is + * 401'd by middleware and every published app goes dark. NOT covered + * anywhere, because the route's own tests invoke the handler directly and + * never traverse middleware. + * + * And every middleware suite `vi.mock`s this constant to a literal, so none of + * them would notice the real one changing. This file deliberately imports the + * REAL constant and checks it against the filesystem. + */ +import { describe, it, expect } from 'vitest'; +import { existsSync } from 'fs'; +import { join } from 'path'; +import { APP_ROUTER_ROUTE_PATH } from '../security-headers'; + +describe('APP_ROUTER_ROUTE_PATH names a route that exists', () => { + it('given the carve-out path, should find a route handler at exactly that location', () => { + // App Router maps /api/x/y to src/app/api/x/y/route.ts. + // + // Resolved from `__dirname`, NOT `process.cwd()`: this file sits at a known + // place in the tree, whereas the working directory depends on how the suite + // was invoked — `bun run --filter web test` and CI's `turbo run` do not agree + // about it. A cwd-relative path would make this assertion pass or fail on the + // invocation rather than on the thing it is supposed to be checking. Matches + // how `api/__tests__/security-audit-coverage.test.ts` walks the route tree. + const handler = join(__dirname, '../../app', APP_ROUTER_ROUTE_PATH, 'route.ts'); + + expect(existsSync(handler)).toBe(true); + }); + + // Pins the shape too: a constant that stopped being an absolute /api path + // would still "exist" under some join and quietly stop matching `pathname`. + it('given the constant, should be an absolute /api path with no trailing slash', () => { + expect(APP_ROUTER_ROUTE_PATH.startsWith('/api/')).toBe(true); + expect(APP_ROUTER_ROUTE_PATH.endsWith('/')).toBe(false); + }); +}); diff --git a/apps/web/src/middleware/__tests__/matcher.test.ts b/apps/web/src/middleware/__tests__/matcher.test.ts index a2846ced82..5e0a7bfd1c 100644 --- a/apps/web/src/middleware/__tests__/matcher.test.ts +++ b/apps/web/src/middleware/__tests__/matcher.test.ts @@ -9,7 +9,9 @@ vi.mock('@/middleware/monitoring', () => ({ monitoringMiddleware: vi.fn() })); vi.mock('@/middleware/security-headers', () => ({ createSecureResponse: vi.fn(), createSecureErrorResponse: vi.fn(), + APP_ROUTER_ROUTE_PATH: '/api/app-hosting/router', isHandoffBridgeRoute: vi.fn(), + routeOwnsItsOwnCsp: vi.fn(), isPublicPageRoute: vi.fn(), isPublishedSiteHost: vi.fn(), shouldDisableCOEP: vi.fn(), diff --git a/apps/web/src/middleware/__tests__/oauth-public-endpoints.test.ts b/apps/web/src/middleware/__tests__/oauth-public-endpoints.test.ts index e938afc921..102f8b35a0 100644 --- a/apps/web/src/middleware/__tests__/oauth-public-endpoints.test.ts +++ b/apps/web/src/middleware/__tests__/oauth-public-endpoints.test.ts @@ -13,7 +13,11 @@ const createSecureResponse = vi.fn(() => ({ response: { status: 200, headers: ne vi.mock('@/middleware/security-headers', () => ({ createSecureResponse, createSecureErrorResponse: vi.fn((body: unknown, status: number) => new Response(JSON.stringify(body), { status })), + APP_ROUTER_ROUTE_PATH: '/api/app-hosting/router', isHandoffBridgeRoute: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback'), + // Middleware asks this (not isHandoffBridgeRoute) for skipCSP: the routes that + // deliver their own CSP are the handoff bridges plus the published-app router. + routeOwnsItsOwnCsp: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback' || pathname === '/api/app-hosting/router'), isPublicPageRoute: vi.fn(() => false), isPublishedSiteHost: vi.fn(() => false), isSecureRequest: vi.fn(() => true), diff --git a/apps/web/src/middleware/__tests__/pre-session-and-asset-endpoints.test.ts b/apps/web/src/middleware/__tests__/pre-session-and-asset-endpoints.test.ts index 6866e698f2..e34e41dbb2 100644 --- a/apps/web/src/middleware/__tests__/pre-session-and-asset-endpoints.test.ts +++ b/apps/web/src/middleware/__tests__/pre-session-and-asset-endpoints.test.ts @@ -13,7 +13,11 @@ const createSecureResponse = vi.fn(() => ({ response: { status: 200, headers: ne vi.mock('@/middleware/security-headers', () => ({ createSecureResponse, createSecureErrorResponse: vi.fn((body: unknown, status: number) => new Response(JSON.stringify(body), { status })), + APP_ROUTER_ROUTE_PATH: '/api/app-hosting/router', isHandoffBridgeRoute: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback'), + // Middleware asks this (not isHandoffBridgeRoute) for skipCSP: the routes that + // deliver their own CSP are the handoff bridges plus the published-app router. + routeOwnsItsOwnCsp: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback' || pathname === '/api/app-hosting/router'), isPublicPageRoute: vi.fn(() => false), isPublishedSiteHost: vi.fn(() => false), isSecureRequest: vi.fn(() => true), diff --git a/apps/web/src/middleware/__tests__/security-headers.test.ts b/apps/web/src/middleware/__tests__/security-headers.test.ts index f30aaaffa6..34cd411b35 100644 --- a/apps/web/src/middleware/__tests__/security-headers.test.ts +++ b/apps/web/src/middleware/__tests__/security-headers.test.ts @@ -15,6 +15,7 @@ import { createSecureResponse, createSecureErrorResponse, isHandoffBridgeRoute, + routeOwnsItsOwnCsp, isPublicPageRoute, isPublishedSiteHost, shouldDisableCOEP, @@ -564,6 +565,23 @@ describe('Security Headers', () => { }); }); + describe('routeOwnsItsOwnCsp', () => { + it('covers the handoff bridges and the published-app router', () => { + expect(routeOwnsItsOwnCsp('/api/auth/google/callback')).toBe(true); + expect(routeOwnsItsOwnCsp('/api/auth/apple/callback')).toBe(true); + expect(routeOwnsItsOwnCsp('/api/app-hosting/router')).toBe(true); + }); + + it('is false for everything else, including app-hosting siblings', () => { + // The API CSP is the safe default; only a route that actually delivers its + // own policy may opt out, and only by exact path. + expect(routeOwnsItsOwnCsp('/api/app-hosting')).toBe(false); + expect(routeOwnsItsOwnCsp('/api/app-hosting/router/x')).toBe(false); + expect(routeOwnsItsOwnCsp('/api/auth/csrf')).toBe(false); + expect(routeOwnsItsOwnCsp('/api/foo')).toBe(false); + }); + }); + describe('createSecureErrorResponse', () => { it('returns response with correct status code', () => { const response = createSecureErrorResponse('Error', 401); diff --git a/apps/web/src/middleware/__tests__/signup-public-endpoints.test.ts b/apps/web/src/middleware/__tests__/signup-public-endpoints.test.ts index 308522a31d..63b45b3806 100644 --- a/apps/web/src/middleware/__tests__/signup-public-endpoints.test.ts +++ b/apps/web/src/middleware/__tests__/signup-public-endpoints.test.ts @@ -13,7 +13,11 @@ const createSecureResponse = vi.fn(() => ({ response: { status: 200, headers: ne vi.mock('@/middleware/security-headers', () => ({ createSecureResponse, createSecureErrorResponse: vi.fn((body: unknown, status: number) => new Response(JSON.stringify(body), { status })), + APP_ROUTER_ROUTE_PATH: '/api/app-hosting/router', isHandoffBridgeRoute: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback'), + // Middleware asks this (not isHandoffBridgeRoute) for skipCSP: the routes that + // deliver their own CSP are the handoff bridges plus the published-app router. + routeOwnsItsOwnCsp: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback' || pathname === '/api/app-hosting/router'), isPublicPageRoute: vi.fn(() => false), isPublishedSiteHost: vi.fn(() => false), isSecureRequest: vi.fn(() => true), diff --git a/apps/web/src/middleware/__tests__/webhook-public-endpoints.test.ts b/apps/web/src/middleware/__tests__/webhook-public-endpoints.test.ts index 32c39096d5..51fac219bb 100644 --- a/apps/web/src/middleware/__tests__/webhook-public-endpoints.test.ts +++ b/apps/web/src/middleware/__tests__/webhook-public-endpoints.test.ts @@ -13,7 +13,11 @@ const createSecureResponse = vi.fn(() => ({ response: { status: 200, headers: ne vi.mock('@/middleware/security-headers', () => ({ createSecureResponse, createSecureErrorResponse: vi.fn((body: unknown, status: number) => new Response(JSON.stringify(body), { status })), + APP_ROUTER_ROUTE_PATH: '/api/app-hosting/router', isHandoffBridgeRoute: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback'), + // Middleware asks this (not isHandoffBridgeRoute) for skipCSP: the routes that + // deliver their own CSP are the handoff bridges plus the published-app router. + routeOwnsItsOwnCsp: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback' || pathname === '/api/app-hosting/router'), isPublicPageRoute: vi.fn(() => false), isPublishedSiteHost: vi.fn(() => false), isSecureRequest: vi.fn(() => true), diff --git a/apps/web/src/middleware/__tests__/well-known-oauth-discovery.test.ts b/apps/web/src/middleware/__tests__/well-known-oauth-discovery.test.ts index 6a80d4c82f..cc42867a73 100644 --- a/apps/web/src/middleware/__tests__/well-known-oauth-discovery.test.ts +++ b/apps/web/src/middleware/__tests__/well-known-oauth-discovery.test.ts @@ -23,7 +23,11 @@ vi.mock('@/middleware/security-headers', () => ({ createSecureResponse, createSecureRewrite, createSecureErrorResponse: vi.fn(), + APP_ROUTER_ROUTE_PATH: '/api/app-hosting/router', isHandoffBridgeRoute: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback'), + // Middleware asks this (not isHandoffBridgeRoute) for skipCSP: the routes that + // deliver their own CSP are the handoff bridges plus the published-app router. + routeOwnsItsOwnCsp: vi.fn((pathname: string) => pathname === '/api/auth/google/callback' || pathname === '/api/auth/apple/callback' || pathname === '/api/app-hosting/router'), isPublicPageRoute: vi.fn(() => false), isPublishedSiteHost: vi.fn(() => false), shouldDisableCOEP: vi.fn(() => false), diff --git a/apps/web/src/middleware/security-headers.ts b/apps/web/src/middleware/security-headers.ts index c4ae7d8682..186e48e9be 100644 --- a/apps/web/src/middleware/security-headers.ts +++ b/apps/web/src/middleware/security-headers.ts @@ -315,6 +315,29 @@ export const isPublishedSiteHost = (host: string | null | undefined): boolean => export const isHandoffBridgeRoute = (pathname: string): boolean => (HANDOFF_BRIDGE_ROUTE_PATHS as readonly string[]).includes(pathname); +/** + * The published-app serving edge. It answers a routing decision either as a + * bodiless `fly-replay` (no CSP needed) or as its OWN styled parked / + * unavailable / not-found page, which carries a bespoke CSP allowing the inline + * style attributes it is built from — the page must be self-contained, because + * fetching a stylesheet to render "this app is paused" adds a dependency to the + * one response that has to work when things are broken. + * + * Same reasoning as isHandoffBridgeRoute: the API CSP's `default-src 'none'` + * falls style-src back to 'none', and browsers enforce the intersection of every + * delivered CSP — so without this the customer-facing enforcement page renders + * as unstyled text. + */ +export const APP_ROUTER_ROUTE_PATH = '/api/app-hosting/router'; + +/** + * Routes that deliver their own Content-Security-Policy and must not have the + * middleware's layered on top. One predicate so the middleware asks the question + * once rather than growing a chain of ORs. + */ +export const routeOwnsItsOwnCsp = (pathname: string): boolean => + isHandoffBridgeRoute(pathname) || pathname === APP_ROUTER_ROUTE_PATH; + export const shouldDisableCOEP = (pathname: string): boolean => pathname.startsWith('/settings/plan') || pathname.startsWith('/settings/billing') || diff --git a/knip.json b/knip.json index d1fe78e046..f1e9cbcd00 100644 --- a/knip.json +++ b/knip.json @@ -421,8 +421,13 @@ "src/services/agent-workspaces/workspace-status.ts", "src/services/agent-workspaces/shell-types.ts", "src/services/app-hosting/app-hosting-env.ts", + "src/services/app-hosting/app-replay-key.ts", + "src/services/app-hosting/parked-page.ts", "src/services/app-hosting/provisioner-core.ts", "src/services/app-hosting/provisioner.ts", + "src/services/app-hosting/router-core.ts", + "src/services/app-hosting/router.ts", + "src/services/app-hosting/routing-env.ts", "src/services/attachment-upload-core.ts", "src/services/attachment-upload-repository.ts", "src/services/attachment-upload.ts", @@ -510,6 +515,7 @@ "src/utils/utils.ts", "src/validators/custom-domain.ts", "src/validators/email.ts", + "src/validators/fly-ownership.ts", "src/validators/id-validators.ts", "src/validators/subdomain.ts" ], diff --git a/packages/lib/package.json b/packages/lib/package.json index 68346b4304..6e319689bd 100644 --- a/packages/lib/package.json +++ b/packages/lib/package.json @@ -2017,6 +2017,36 @@ "types": "./dist/services/agent-workspaces/workspace-membership-store.d.ts", "import": "./dist/services/agent-workspaces/workspace-membership-store.js", "require": "./dist/services/agent-workspaces/workspace-membership-store.js" + }, + "./services/app-hosting/routing-env": { + "types": "./dist/services/app-hosting/routing-env.d.ts", + "import": "./dist/services/app-hosting/routing-env.js", + "require": "./dist/services/app-hosting/routing-env.js" + }, + "./services/app-hosting/router-core": { + "types": "./dist/services/app-hosting/router-core.d.ts", + "import": "./dist/services/app-hosting/router-core.js", + "require": "./dist/services/app-hosting/router-core.js" + }, + "./services/app-hosting/router": { + "types": "./dist/services/app-hosting/router.d.ts", + "import": "./dist/services/app-hosting/router.js", + "require": "./dist/services/app-hosting/router.js" + }, + "./services/app-hosting/app-replay-key": { + "types": "./dist/services/app-hosting/app-replay-key.d.ts", + "import": "./dist/services/app-hosting/app-replay-key.js", + "require": "./dist/services/app-hosting/app-replay-key.js" + }, + "./services/app-hosting/parked-page": { + "types": "./dist/services/app-hosting/parked-page.d.ts", + "import": "./dist/services/app-hosting/parked-page.js", + "require": "./dist/services/app-hosting/parked-page.js" + }, + "./validators/fly-ownership": { + "types": "./dist/validators/fly-ownership.d.ts", + "import": "./dist/validators/fly-ownership.js", + "require": "./dist/validators/fly-ownership.js" } }, "typesVersions": { @@ -2770,6 +2800,24 @@ ], "canvas/cert-action": [ "./dist/canvas/cert-action.d.ts" + ], + "services/app-hosting/routing-env": [ + "./dist/services/app-hosting/routing-env.d.ts" + ], + "services/app-hosting/router-core": [ + "./dist/services/app-hosting/router-core.d.ts" + ], + "services/app-hosting/router": [ + "./dist/services/app-hosting/router.d.ts" + ], + "services/app-hosting/app-replay-key": [ + "./dist/services/app-hosting/app-replay-key.d.ts" + ], + "services/app-hosting/parked-page": [ + "./dist/services/app-hosting/parked-page.d.ts" + ], + "validators/fly-ownership": [ + "./dist/validators/fly-ownership.d.ts" ] } }, diff --git a/packages/lib/src/billing/__tests__/credit-balance.test.ts b/packages/lib/src/billing/__tests__/credit-balance.test.ts index 09c46fec81..de17859ff0 100644 --- a/packages/lib/src/billing/__tests__/credit-balance.test.ts +++ b/packages/lib/src/billing/__tests__/credit-balance.test.ts @@ -42,7 +42,7 @@ vi.mock('@pagespace/db/db', () => ({ }, })); -import { getCreditBalance, resolveTier } from '../credit-balance'; +import { getCreditBalance, readSpendableCents, resolveTier } from '../credit-balance'; const future = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000); const past = new Date(Date.now() - 24 * 60 * 60 * 1000); @@ -276,6 +276,60 @@ describe('getCreditBalance', () => { }); }); +describe('readSpendableCents — the routing edge lean read', () => { + /** A funded row with a live period. */ + const funded = (over: Record = {}) => [{ + monthlyRemainingCents: 500, + monthlyAllowanceCents: 500, + topupRemainingCents: 0, + debtCents: 0, + monthlyPeriodEnd: future, + ...over, + }]; + + it('never reads credit_holds — that aggregate is the reason this function exists', async () => { + balanceRows = funded(); + // A hold big enough to change the answer if it were subtracted. + holdRows = [{ reserved: 100_000 }]; + + expect(await readSpendableCents('u1', 'pro')).toBe(500); + }); + + it('agrees with the display read for a funded row', async () => { + balanceRows = funded(); + const [lean, display] = [await readSpendableCents('u1', 'pro'), await getCreditBalance('u1', 'pro')]; + expect(lean).toBe(display.spendable); + }); + + it('agrees with the display read when no row exists yet', async () => { + balanceRows = []; + const [lean, display] = [await readSpendableCents('u1', 'free'), await getCreditBalance('u1', 'free')]; + expect(lean).toBe(display.spendable); + expect(lean).toBeGreaterThan(0); + }); + + it('agrees with the display read for a free user whose window has lapsed', async () => { + // The rollover the gate applies lazily: the user must not read as broke for the + // gap between the reset being due and something performing it. + balanceRows = funded({ monthlyRemainingCents: 0, monthlyPeriodEnd: past }); + const [lean, display] = [await readSpendableCents('u1', 'free'), await getCreditBalance('u1', 'free')]; + expect(lean).toBe(display.spendable); + expect(lean).toBeGreaterThan(0); + }); + + it('agrees with the display read for a user in debt, and goes negative', async () => { + balanceRows = funded({ monthlyRemainingCents: 0, topupRemainingCents: 0, debtCents: 750 }); + const [lean, display] = [await readSpendableCents('u1', 'pro'), await getCreditBalance('u1', 'pro')]; + expect(lean).toBe(display.spendable); + expect(lean).toBeLessThan(0); + }); + + it('clamps at zero when there is no debt', async () => { + balanceRows = funded({ monthlyRemainingCents: 0, topupRemainingCents: 0, debtCents: 0, monthlyPeriodEnd: future }); + expect(await readSpendableCents('u1', 'pro')).toBe(0); + }); +}); + describe('resolveTier', () => { it('returns the stored subscription tier', async () => { userRows = [{ subscriptionTier: 'pro' }]; diff --git a/packages/lib/src/billing/__tests__/has-spendable-balance.test.ts b/packages/lib/src/billing/__tests__/has-spendable-balance.test.ts new file mode 100644 index 0000000000..4d5752dab5 --- /dev/null +++ b/packages/lib/src/billing/__tests__/has-spendable-balance.test.ts @@ -0,0 +1,138 @@ +/** + * hasSpendableBalance — the read-only twin of the credit gate. + * + * It exists for one caller shape: the published-app routing edge, which asks + * "could this payer spend?" once per HTTP REQUEST (the metered tier has no + * replay cache, by design). The whole reason it is not `canConsumeAI` is that + * `canConsumeAI` inserts a hold — right for one bounded AI call, catastrophic on + * a path that runs for every image and stylesheet a published page loads, where + * each hold would reserve spend against a run that has no settle to release it. + * + * So the two properties under test are: it reaches the same VERDICT the gate + * would (same floor, same comparison), and it WRITES NOTHING. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const mockIsBillingEnabled = vi.hoisted(() => vi.fn(() => true)); +const mockReadSpendableCents = vi.hoisted(() => vi.fn()); +const mockDb = vi.hoisted(() => ({ + select: vi.fn(), + insert: vi.fn(), + update: vi.fn(), + transaction: vi.fn(), +})); + +vi.mock('@pagespace/db/db', () => ({ db: mockDb })); +vi.mock('../../deployment-mode', () => ({ isBillingEnabled: mockIsBillingEnabled })); +vi.mock('../credit-balance', () => ({ readSpendableCents: mockReadSpendableCents })); +vi.mock('@pagespace/db/schema/credits', () => ({ + creditBalances: { userId: 'cb.userId' }, + creditHolds: { id: 'ch.id', userId: 'ch.userId', estCents: 'ch.est', expiresAt: 'ch.exp' }, + creditLedger: { + userId: 'cl.userId', + stripeRef: 'cl.stripeRef', + entryType: 'cl.entryType', + bucket: 'cl.bucket', + amountCents: 'cl.amount', + chargeMillicents: 'cl.charge', + consumeStatus: 'cl.consumeStatus', + createdAt: 'cl.createdAt', + }, +})); +vi.mock('@pagespace/db/operators', () => ({ + eq: vi.fn((a, b) => ({ op: 'eq', a, b })), + and: vi.fn((...a) => ({ op: 'and', a })), + gt: vi.fn((a, b) => ({ op: 'gt', a, b })), + lt: vi.fn((a, b) => ({ op: 'lt', a, b })), + gte: vi.fn((a, b) => ({ op: 'gte', a, b })), + lte: vi.fn((a, b) => ({ op: 'lte', a, b })), + sql: Object.assign( + vi.fn((strings: TemplateStringsArray, ...values: unknown[]) => ({ sql: [strings, values] })), + { raw: vi.fn((s: string) => ({ raw: s })) }, + ), +})); + +import { hasSpendableBalance } from '../credit-gate'; +import { RESERVE_FLOOR_CENTS } from '../credit-pricing'; + +/** The lean read returns the spendable figure itself. */ +function balance(spendable: number) { + return spendable; +} + +beforeEach(() => { + vi.clearAllMocks(); + mockIsBillingEnabled.mockReturnValue(true); +}); + +describe('hasSpendableBalance — the same floor the gate applies', () => { + it('given a balance comfortably above the floor, should allow', async () => { + mockReadSpendableCents.mockResolvedValue(balance(RESERVE_FLOOR_CENTS + 500)); + expect(await hasSpendableBalance('user_1', 'pro')).toBe(true); + }); + + it('given a balance exactly AT the floor, should refuse — the gate compares strictly greater', async () => { + mockReadSpendableCents.mockResolvedValue(balance(RESERVE_FLOOR_CENTS)); + expect(await hasSpendableBalance('user_1', 'pro')).toBe(false); + }); + + it('given a balance one cent above the floor, should allow', async () => { + mockReadSpendableCents.mockResolvedValue(balance(RESERVE_FLOOR_CENTS + 1)); + expect(await hasSpendableBalance('user_1', 'pro')).toBe(true); + }); + + it('given an exhausted balance, should refuse — this is the parked-page path', async () => { + mockReadSpendableCents.mockResolvedValue(balance(0)); + expect(await hasSpendableBalance('user_1', 'pro')).toBe(false); + }); + + it('given a balance pulled negative by debt, should refuse', async () => { + mockReadSpendableCents.mockResolvedValue(balance(-1200)); + expect(await hasSpendableBalance('user_1', 'pro')).toBe(false); + }); +}); + +describe('hasSpendableBalance — deployments without billing are unlimited', () => { + it('given billing is disabled, should allow without reading the ledger at all', async () => { + mockIsBillingEnabled.mockReturnValue(false); + expect(await hasSpendableBalance('user_1', 'pro')).toBe(true); + expect(mockReadSpendableCents).not.toHaveBeenCalled(); + }); +}); + +describe('hasSpendableBalance — reads only, on a per-request path', () => { + it('given any call, should never write: no hold, no insert, no update, no transaction', async () => { + mockReadSpendableCents.mockResolvedValue(balance(5000)); + + await hasSpendableBalance('user_1', 'pro'); + + // A hold per request would reserve spend against a run with no settle. + expect(mockDb.insert).not.toHaveBeenCalled(); + expect(mockDb.update).not.toHaveBeenCalled(); + expect(mockDb.transaction).not.toHaveBeenCalled(); + }); + + it('given a call, should not read credit_holds — the gate discards that figure anyway', async () => { + // The whole reason this does not go through getCreditBalance: that read also + // runs a SUM over active holds, which this gate throws away, on a path that + // executes once per image and per stylesheet of a published page. + mockReadSpendableCents.mockResolvedValue(balance(5000)); + + await hasSpendableBalance('user_1', 'pro'); + + expect(mockDb.select).not.toHaveBeenCalled(); + expect(mockReadSpendableCents).toHaveBeenCalledTimes(1); + }); + + it("given a tier, should judge the balance against that tier's allowance", async () => { + mockReadSpendableCents.mockResolvedValue(balance(5000)); + await hasSpendableBalance('user_1', 'pro'); + expect(mockReadSpendableCents).toHaveBeenCalledWith('user_1', 'pro'); + }); + + it('given no tier, should default to free rather than assume an allowance', async () => { + mockReadSpendableCents.mockResolvedValue(balance(5000)); + await hasSpendableBalance('user_1'); + expect(mockReadSpendableCents).toHaveBeenCalledWith('user_1', 'free'); + }); +}); diff --git a/packages/lib/src/billing/credit-balance.ts b/packages/lib/src/billing/credit-balance.ts index 703b2c093c..4d3e364163 100644 --- a/packages/lib/src/billing/credit-balance.ts +++ b/packages/lib/src/billing/credit-balance.ts @@ -94,6 +94,80 @@ function disabledSummary(): CreditBalanceSummary { }; } +/** The funded-balance columns both the display read and the routing gate need. */ +interface FundedBalanceRow { + monthlyRemainingCents: number; + monthlyAllowanceCents: number; + topupRemainingCents: number; + debtCents: number | null; + monthlyPeriodEnd: Date | null; +} + +/** + * The spendable figure, from a balance row alone. + * + * Extracted so the display read and the published-app routing gate cannot drift + * apart about what "spendable" means — they used to share it only by both + * calling {@link getCreditBalance}, which made the gate pay for the display's + * in-flight-holds aggregate on a per-request path. + * + * GROSS of in-flight holds, deliberately: `reserved` is reported separately and + * never netted out (see the file header). Clamped at 0 only when there is no + * debt — outstanding overage pulls the figure negative. + */ +function spendableCentsFor( + row: FundedBalanceRow | null, + tier: SubscriptionTier, + now: Date, +): number { + // No row yet: the gate lazy-inits from the tier allowance on the first call. + if (!row) return Math.max(0, allowanceFor(tier)); + + const allowance = row.monthlyAllowanceCents || allowanceFor(tier); + const expired = row.monthlyPeriodEnd === null || row.monthlyPeriodEnd < now; + // A free user whose window has lapsed gets the allowance the gate will apply on + // its next call, so they are not treated as broke for the gap between the + // rollover being due and something performing it. + const monthlyRemaining = + tier === 'free' && expired ? row.monthlyRemainingCents + allowance : row.monthlyRemainingCents; + const topupRemaining = row.topupRemainingCents; + const debt = row.debtCents ?? 0; + return debt > 0 + ? monthlyRemaining + topupRemaining - debt + : Math.max(0, monthlyRemaining + topupRemaining); +} + +/** + * Spendable cents from ONE indexed read — no in-flight-holds aggregate. + * + * For the published-app routing edge, which asks "can this payer spend?" once per + * HTTP REQUEST (the metered tier has no replay cache, by design). Going through + * {@link getCreditBalance} there meant every image and stylesheet also paid for a + * `SUM` over `credit_holds` whose result the caller then discarded. + * + * Same arithmetic as the display read, via {@link spendableCentsFor}. Never + * lazy-inits and never rolls the period: both are writes, and they belong to the + * gate that owns the row lock. + */ +export async function readSpendableCents( + userId: string, + tier: SubscriptionTier = 'free', +): Promise { + const [row] = await db + .select({ + monthlyRemainingCents: creditBalances.monthlyRemainingCents, + monthlyAllowanceCents: creditBalances.monthlyAllowanceCents, + topupRemainingCents: creditBalances.topupRemainingCents, + debtCents: creditBalances.debtCents, + monthlyPeriodEnd: creditBalances.monthlyPeriodEnd, + }) + .from(creditBalances) + .where(eq(creditBalances.userId, userId)) + .limit(1); + + return spendableCentsFor(row ?? null, tier, new Date()); +} + /** * Read a user's current prepaid credit balance for display. Pure read: no lazy-init, * no reset — those are owned by the gate. A user with no balance row yet is shown the @@ -132,7 +206,7 @@ export async function getCreditBalance( // so present that as the spendable monthly balance. if (!row) { const allowance = allowanceFor(tier); - const spendable = Math.max(0, allowance); + const spendable = spendableCentsFor(null, tier, now); return { billingEnabled: true, monthly: { remaining: allowance, allowance, periodEnd: null }, @@ -179,10 +253,8 @@ export async function getCreditBalance( // when there's no debt — outstanding overage pulls spendable negative so the widget // shows the red. Debt accrues only after both buckets are exhausted, so the negative // branch is effectively −debt. - const spendable = - debt > 0 - ? monthlyRemaining + topupRemaining - debt - : Math.max(0, monthlyRemaining + topupRemaining); + // Shared with the routing gate's lean read, so the two can never disagree. + const spendable = spendableCentsFor(row, tier, now); return { billingEnabled: true, diff --git a/packages/lib/src/billing/credit-gate.ts b/packages/lib/src/billing/credit-gate.ts index e5cbc20c6d..1fdf91e75f 100644 --- a/packages/lib/src/billing/credit-gate.ts +++ b/packages/lib/src/billing/credit-gate.ts @@ -36,6 +36,7 @@ import { MAX_FREE_INFLIGHT, dailyExposureCapForTier, } from './credit-pricing'; +import { readSpendableCents } from './credit-balance'; import type { SubscriptionTier } from '../services/subscription-utils'; // The partial unique index credit_ledger_stripe_ref_unique is defined WHERE @@ -528,3 +529,48 @@ export async function canConsumeAI( return result; } + +/** + * hasSpendableBalance — the READ-ONLY twin of {@link canConsumeAI}: "could this + * user spend right now?", asked without reserving anything. + * + * Exists for the published-app routing edge's BALANCE-CHECK-BEFORE-WAKE, and the + * difference from `canConsumeAI` is the whole reason it exists: `canConsumeAI` + * INSERTS A HOLD. That is right for an AI call — one gate check, one bounded + * unit of work, one settle — and catastrophic on a serving edge, where the gate + * runs once per HTTP request (the metered tier has no replay cache, by design) + * and would write a `credit_holds` row per image, per stylesheet, per favicon, + * each of them reserving spend against a run that has no settle to release it. + * + * The decision RULE is the one `evaluateGate` applies — spendable above the + * reserve floor, debt netted, billing-disabled deployments unlimited — reached + * through `readSpendableCents`, which shares its arithmetic with the display read + * (including the free-tier lapsed-window rollover the gate applies lazily, so a + * free user whose month has ticked over is not parked for the gap between the + * rollover being due and the next AI call performing it). + * + * It reads the funded-balance columns and NOTHING else: ONE indexed row, no + * aggregate. Going through `getCreditBalance` here would also run its `SUM` over + * active `credit_holds` — a figure this gate then discards — on a path that runs + * once per image and per stylesheet. + * + * The INPUT differs by one term, and deliberately: `evaluateGate` nets out + * `reserved` and this call's `estCost`, and this does not subtract in-flight AI + * holds at all. So the two can disagree for a user mid-stream, which is the + * intended behaviour rather than drift — those holds are reservations against + * chat calls, and a user with a stream running must not have their published + * site go dark for the duration. The awake-seconds meter + * settles separately, and overspend on this path is bounded by the metering + * cron parking the app — not by this read. + * + * Never lazy-inits and never rolls the period: this is a hot read-only path, and + * both of those writes belong to `canConsumeAI`, which owns the row lock. + */ +export async function hasSpendableBalance( + userId: string, + tier: SubscriptionTier = 'free', +): Promise { + if (!isBillingEnabled()) return true; + const spendable = await readSpendableCents(userId, tier); + return spendable > RESERVE_FLOOR_CENTS; +} diff --git a/packages/lib/src/canvas/__tests__/cert-action.test.ts b/packages/lib/src/canvas/__tests__/cert-action.test.ts index 842dc46d87..212d060f5c 100644 --- a/packages/lib/src/canvas/__tests__/cert-action.test.ts +++ b/packages/lib/src/canvas/__tests__/cert-action.test.ts @@ -97,6 +97,15 @@ describe('certActionToDbStatus', () => { it('mark-failed action maps to cert_failed DB status', () => { expect(certActionToDbStatus({ action: 'mark-failed', reason: 'boom' })).toBe('cert_failed'); }); + + it('blocked-on-ownership maps to provisioning, NOT cert_failed', () => { + // The domain is fine and the cert will issue the moment the record appears. + // Mapping this to cert_failed would clear the mirrored prefix of a working + // site because its customer had not yet been told to add a TXT record. + expect(certActionToDbStatus({ action: 'blocked-on-ownership', reason: 'add a TXT' })).toBe( + 'provisioning', + ); + }); }); describe('isCertEligible', () => { @@ -121,3 +130,53 @@ describe('isServingStatus', () => { it('returns false for failed (legacy)', () => expect(isServingStatus('failed')).toBe(false)); it('returns false for an unknown status', () => expect(isServingStatus('bogus')).toBe(false)); }); + +describe('nextCertAction — ownership pre-validation', () => { + const requirement = { + name: '_fly-ownership.docs.acme.com', + appValue: 'app-ABC', + orgValue: 'org-XYZ', + }; + + it('given no pre-validation was run, should behave exactly as before it existed', () => { + // The default argument is the compatibility contract: a caller that does not + // resolve DNS must get the answer it always got. + expect(nextCertAction('verified', ok(false))).toEqual({ action: 'provision' }); + expect(nextCertAction('provisioning', ok(false))).toEqual({ action: 'poll-again' }); + }); + + it.each([ + ['missing', { state: 'missing' as const, expected: requirement }], + ['mismatched', { state: 'mismatched' as const, expected: requirement, found: ['app-WRONG'] }], + ])('given the ownership record is %s, should report what is blocking rather than poll blindly', (_l, ownership) => { + const action = nextCertAction('provisioning', ok(false), ownership); + expect(action.action).toBe('blocked-on-ownership'); + if (action.action !== 'blocked-on-ownership') throw new Error('expected blocked-on-ownership'); + expect(action.reason).toContain('_fly-ownership.docs.acme.com'); + }); + + it.each([ + ['satisfied', { state: 'satisfied' as const }], + ['not_required', { state: 'not_required' as const }], + ])('given ownership is %s, should fall through to the ordinary decision', (_l, ownership) => { + expect(nextCertAction('provisioning', ok(false), ownership)).toEqual({ action: 'poll-again' }); + expect(nextCertAction('verified', ok(false), ownership)).toEqual({ action: 'provision' }); + }); + + it('given a LIVE certificate, should mark it active whatever the record says', () => { + // A live cert is live; ownership is a precondition for issuance, not a + // condition of serving. Checking ownership first would un-activate a + // working certificate whose customer later deleted the TXT record. + expect( + nextCertAction('provisioning', ok(true), { state: 'missing', expected: requirement }), + ).toEqual({ action: 'mark-active' }); + }); + + it('given a Fly error, should still mark failed rather than blame the record', () => { + const action = nextCertAction('verified', err('Fly API timeout'), { + state: 'missing', + expected: requirement, + }); + expect(action).toEqual({ action: 'mark-failed', reason: 'Fly API timeout' }); + }); +}); diff --git a/packages/lib/src/canvas/cert-action.ts b/packages/lib/src/canvas/cert-action.ts index 071df49f16..39629d2750 100644 --- a/packages/lib/src/canvas/cert-action.ts +++ b/packages/lib/src/canvas/cert-action.ts @@ -1,11 +1,46 @@ +import { + describeOwnershipVerification, + type FlyOwnershipRequirement, + type FlyOwnershipVerification, +} from '../validators/fly-ownership'; + export type FlyCertResponse = - | { ok: true; configured: boolean } + | { + ok: true; + /** Fly reports the certificate as live and servable for this hostname. */ + configured: boolean; + /** + * Fly's raw status string (`'pending_validation' | 'active'` on the REST + * certificates resource). Carried alongside `configured` rather than + * replacing it so the decision below stays a boolean question while the + * exact state is still available for logs and the settings UI. + */ + status?: string; + /** + * The `_fly-ownership` TXT record Fly wants published, when it wants one. + * `null`/absent means validation is proceeding by reachability instead. + * See `validators/fly-ownership.ts` for why the distinction matters. + */ + ownership?: FlyOwnershipRequirement | null; + /** Whether Fly has already SEEN an acceptable ownership TXT. */ + ownershipTxtConfigured?: boolean; + } | { ok: false; error: string }; export type CertAction = | { action: 'provision' } | { action: 'poll-again' } | { action: 'mark-active' } + /** + * Fly is waiting on an ownership TXT the customer has not published. NOT a + * failure — the domain is fine and the certificate will issue the moment the + * record appears — so this maps to the same `provisioning` status as + * `poll-again` and never clears mirrored content. It exists as its own action + * purely so the caller can surface WHAT is missing: through the certificate + * status alone, "still validating" and "blocked on a record nobody asked the + * customer for" are indistinguishable, and they need opposite responses. + */ + | { action: 'blocked-on-ownership'; reason: string } | { action: 'mark-failed'; reason: string }; export type CertEligibleStatus = 'verified' | 'provisioning' | 'active' | 'cert_failed'; @@ -35,16 +70,34 @@ export function isServingStatus(status: string): boolean { * * - Fly error → mark-failed (stop polling; surface the error) * - configured=true → mark-active (cert is live) + * - configured=false + an unsatisfied ownership TXT → blocked-on-ownership * - configured=false + verified|cert_failed → provision (request cert, move to provisioning) * - configured=false + provisioning|active → poll-again (still waiting) + * + * `ownership` is the result of pre-validating the `_fly-ownership` TXT + * (`validators/fly-ownership.ts`). It is OPTIONAL and defaults to "no + * pre-validation was run", which reproduces the previous behaviour exactly — + * a caller that does not resolve DNS gets the same answer it always did. + * Checked BEFORE the `configured` branch is not an option: a live certificate + * is live regardless of what any record says, so `mark-active` still wins. */ -export function nextCertAction(currentStatus: CertEligibleStatus, flyCert: FlyCertResponse): CertAction { +export function nextCertAction( + currentStatus: CertEligibleStatus, + flyCert: FlyCertResponse, + ownership: FlyOwnershipVerification | null = null, +): CertAction { if (!flyCert.ok) { return { action: 'mark-failed', reason: flyCert.error || 'Fly cert API error' }; } if (flyCert.configured) { return { action: 'mark-active' }; } + if (ownership && (ownership.state === 'missing' || ownership.state === 'mismatched')) { + return { + action: 'blocked-on-ownership', + reason: describeOwnershipVerification(ownership) ?? 'Fly is waiting on an ownership TXT record', + }; + } if (currentStatus === 'verified' || currentStatus === 'cert_failed') { return { action: 'provision' }; } @@ -56,6 +109,7 @@ export function certActionToDbStatus(action: CertAction): 'provisioning' | 'acti switch (action.action) { case 'provision': case 'poll-again': + case 'blocked-on-ownership': return 'provisioning'; case 'mark-active': return 'active'; diff --git a/packages/lib/src/config/__tests__/env-validation.test.ts b/packages/lib/src/config/__tests__/env-validation.test.ts index b5edfc42f3..5924d6b60e 100644 --- a/packages/lib/src/config/__tests__/env-validation.test.ts +++ b/packages/lib/src/config/__tests__/env-validation.test.ts @@ -536,6 +536,84 @@ describe('env-validation', () => { }); }); + describe('app hosting — the apex and the proxy secret are gated at boot', () => { + const bootable = () => { + process.env.DATABASE_URL = 'postgresql://user:pass@localhost:5432/db'; + process.env.CSRF_SECRET = 'b'.repeat(32); + process.env.ENCRYPTION_KEY = 'c'.repeat(32); + }; + + // The apex carries customer-authored SERVER code on its subdomains, so it + // must be on the PSL before it does — a prerequisite no code can check. + // What code CAN do is refuse to let a deployment inherit the apex silently: + // validateEnv() runs from instrumentation.ts and throws, so enabling hosting + // without naming the apex stops the process rather than serving on a default. + it('given APP_HOSTING_ENABLED=true and no PUBLISHED_APPS_APEX, should refuse to boot', () => { + bootable(); + process.env.APP_HOSTING_ENABLED = 'true'; + + expect(() => validateEnv()).toThrow(/PUBLISHED_APPS_APEX must be set explicitly/); + }); + + it.each([ + ['blank', ''], + ['whitespace only', ' '], + ])('given APP_HOSTING_ENABLED=true and a %s apex, should refuse to boot', (_label, value) => { + bootable(); + process.env.APP_HOSTING_ENABLED = 'true'; + process.env.PUBLISHED_APPS_APEX = value; + + expect(() => validateEnv()).toThrow(/PUBLISHED_APPS_APEX must be set explicitly/); + }); + + it('given APP_HOSTING_ENABLED=true and an explicit apex, should boot', () => { + bootable(); + process.env.APP_HOSTING_ENABLED = 'true'; + process.env.PUBLISHED_APPS_APEX = 'pagespace.app'; + + expect(() => validateEnv()).not.toThrow(); + }); + + // The gate is on ENABLING hosting, not on the variable: while hosting is + // dark the apex is unused, and requiring it would fail every deployment + // that has never heard of app hosting. + it.each([ + ['unset', undefined], + ['not exactly "true"', '1'], + ])('given APP_HOSTING_ENABLED is %s, should boot without an apex', (_label, value) => { + bootable(); + if (value === undefined) delete process.env.APP_HOSTING_ENABLED; + else process.env.APP_HOSTING_ENABLED = value; + delete process.env.PUBLISHED_APPS_APEX; + + expect(() => validateEnv()).not.toThrow(); + }); + + // A guessable proxy secret leaves the router endpoint a world-callable + // fly-replay emitter, so it is rejected rather than accepted-but-weak. The + // blank form still passes: that is read as "refuse everything", not "no check". + it('given a configured APP_ROUTER_PROXY_SECRET below 32 chars, should refuse to boot', () => { + bootable(); + process.env.APP_ROUTER_PROXY_SECRET = 'a'; + + expect(() => validateEnv()).toThrow(/APP_ROUTER_PROXY_SECRET/); + }); + + it('given a blank APP_ROUTER_PROXY_SECRET, should boot — the router reads it as refuse-everything', () => { + bootable(); + process.env.APP_ROUTER_PROXY_SECRET = ''; + + expect(() => validateEnv()).not.toThrow(); + }); + + it('given a 32-char APP_ROUTER_PROXY_SECRET, should boot', () => { + bootable(); + process.env.APP_ROUTER_PROXY_SECRET = 'p'.repeat(32); + + expect(() => validateEnv()).not.toThrow(); + }); + }); + describe('getEnvErrors', () => { it('given valid environment, should return empty array', () => { process.env.DATABASE_URL = 'postgresql://user:pass@localhost:5432/db'; diff --git a/packages/lib/src/config/env-validation.ts b/packages/lib/src/config/env-validation.ts index 619996a8db..52561947f6 100644 --- a/packages/lib/src/config/env-validation.ts +++ b/packages/lib/src/config/env-validation.ts @@ -178,6 +178,40 @@ export const serverEnvSchema = z // networks, so all published apps must share one. PUBLISHED_APPS_NETWORK: z.string().optional(), + // The apex published apps are served from (`.`). Optional in + // the schema, but REQUIRED by the superRefine below once APP_HOSTING_ENABLED + // is 'true' — unset only falls back to PUBLISHED_APPS_APEX_DEFAULT while + // hosting is dark. Deliberately a DIFFERENT + // apex from `*.pagespace.site`, which is not on the Public Suffix List — a + // published app runs customer server code on its own origin, so sharing a + // registrable domain with other published content would let one app set + // cookies every other one sends. Read via resolvePublishedAppsApex. + PUBLISHED_APPS_APEX: z.string().optional(), + + // The Fly app that terminates the published-apps apex, emits fly-replay, and + // holds custom-domain certs. Optional: falls back to FLY_PROXY_APP_NAME and + // then to APP_ROUTER_FLY_APP_DEFAULT. It MUST have been created on + // PUBLISHED_APPS_NETWORK — fly-replay cannot cross Fly 6PN networks, and an + // app's network is fixed at create time. See routing-env.ts. + APP_ROUTER_FLY_APP_NAME: z.string().optional(), + + // Server secret the per-app fly-replay `state` key is derived from (see + // services/app-hosting/app-replay-key.ts). A configured value must be >= 32 + // chars, but a blank placeholder is accepted — mirroring + // SANDBOX_SESSION_SECRET — so an empty value makes the router refuse to + // emit replays (fail closed) rather than failing app-wide env validation. + APP_REPLAY_SECRET: z.string().min(32).optional().or(z.literal('')), + + // Shared secret proving a router request came from the edge proxy. Optional + // and blank-tolerant for the same reason; the router treats an unset value + // as "refuse everything", never as "skip the check" — the endpoint would + // otherwise be a world-callable fly-replay emitter for the whole Fly org. + // A CONFIGURED value must clear the same >=32 floor as APP_REPLAY_SECRET: a + // guessable secret is not a weaker check, it is the absence of one, and this + // is the check that stops the endpoint being world-callable. Enforced again + // in resolveAppRouterProxySecret, which reads process.env directly. + APP_ROUTER_PROXY_SECRET: z.string().min(32).optional().or(z.literal('')), + // Sentry server/edge DSN. Fail-loud in production for cloud/tenant (see // superRefine below) — a missing DSN previously meant Sentry.init({dsn: // undefined}) silently no-op'd with zero alerts ever reaching a human. @@ -205,6 +239,24 @@ export const serverEnvSchema = z } } + // App hosting serves customer-authored SERVER code on subdomains of one apex. + // Because that apex must be on the Public Suffix List before it carries + // untrusted origins — otherwise one published app sets a `domain=` + // cookie every other published app then sends — the apex is not something a + // deployment may arrive at by default. PUBLISHED_APPS_APEX_DEFAULT stays the + // documented value and keeps resolvePublishedAppsApex from ever returning '' + // (an empty apex would make parseAppHost claim EVERY hostname), but turning + // hosting on requires naming the apex explicitly, so the PSL prerequisite has + // an owner who chose it rather than inheriting it silently. See ROUTING.md. + if (data.APP_HOSTING_ENABLED === 'true' && !data.PUBLISHED_APPS_APEX?.trim()) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + 'PUBLISHED_APPS_APEX must be set explicitly when APP_HOSTING_ENABLED=true — published apps run customer server code, so the apex they share has to be a deliberate, PSL-registered choice rather than a default', + path: ['PUBLISHED_APPS_APEX'], + }); + } + if (data.NODE_ENV === 'production' && !isOnPrem() && !data.SENTRY_DSN) { ctx.addIssue({ code: z.ZodIssueCode.custom, diff --git a/packages/lib/src/services/app-hosting/ROUTING.md b/packages/lib/src/services/app-hosting/ROUTING.md new file mode 100644 index 0000000000..989cc70969 --- /dev/null +++ b/packages/lib/src/services/app-hosting/ROUTING.md @@ -0,0 +1,230 @@ +# Published-app serving edge + +How a request for `.` becomes a response, what has to be +configured for it to work, and the two out-of-band actions (a PSL submission and +a Fly network choice) that this repo cannot perform for itself. + +Everything here ships behind `APP_HOSTING_ENABLED`. With the flag off the router +endpoint is inert: it answers `hosting_disabled` before reading the database. + +## The request path + +``` +client → pagespace-proxy (Caddy, terminates TLS for *.) + → POST/GET pagespace-web /api/app-hosting/router + with x-pagespace-app-host: + x-pagespace-app-router-key: + → resolveAppRoute() ── published_apps row + payer balance + ├─ replay → 204 + `fly-replay: app=…;state=…;timeout=1500` + │ Fly replays the ORIGINAL request to the target app, + │ auto-starting its machine if stopped. + └─ refusal → the parked / unavailable / not-found page, served here, + and NO machine is started. +``` + +Two consequences of this shape are easy to get wrong later: + +- **A replayed response never comes back through us.** Fly hands the request to + the target app and returns its response straight to the client, so no Caddy + header stanza and nothing in the router route applies to a published app's own + output. A published app owns its security headers. +- **The rewritten path is not what gets replayed.** The proxy rewrites to + `/api/app-hosting/router`, but `fly-replay` replays the request the client + actually made. + +## The route lives behind the web app's middleware + +`apps/web/src/middleware.ts` runs in front of every `/api` path, and the router +endpoint has to be carved out of it explicitly — in the right *place*, not just +at all. It returns alongside `/api/public/forms`, above origin validation and +above the Bearer-API `OPTIONS` short-circuit. There are **three ways to get it +wrong** — never carved out, carved out but without skipping the CSP, or carved +out too late — and they produce **four distinct symptoms**, because the last one +breaks in two independent places. None of the four fails a handler test: handler +tests invoke the route directly, and all four of these live above it. + +| missing/misplaced | symptom | +| --- | --- | +| not on the public list | 401 before `route.ts` runs — **no published app is reachable** | +| middleware CSP not skipped | `default-src 'none'` falls `style-src` back to `'none'`; the parked page renders unstyled | +| below origin validation | a published app's own fetch carries its own origin, which is never in our allowlist → 403 on every non-GET | +| below the `OPTIONS` short-circuit | a published app's CORS preflight is answered with *our* policy instead of being replayed to the app | + +`middleware.test.ts` guards all four symptoms; each is mutation-checked. If you add another +`/api/app-hosting/*` route, note the exemption is an **exact path match** and does +not extend to siblings — an authenticated route there should not inherit it. + +## Why every request pays a router hop + +The metered tier sets **no `fly-replay-cache`**. The cache exists to skip the +router hop on subsequent requests — and the router hop *is* the balance gate, so +a cached replay would keep a machine awake and billing for a payer we would +refuse today. + +Every asset of a published page therefore costs one hop plus, for a servable +metered app, three single-row indexed reads: the `published_apps` lookup, the +payer's tier, and the payer's funded balance. No aggregates — +`hasSpendableBalance` deliberately avoids `getCreditBalance`, which would add a +`SUM` over active `credit_holds` that the gate then discards. A refusal costs +fewer: an app refused on status alone never reaches the ledger. + +That is the price of the enforcement property, and it is bounded by the fact that +only the decision is ours; the bytes are not. + +The flat-rate **dedicated** tier is the only legitimate cache user, because it +has no balance gate to bypass. See `replayCachePolicyFor`. + +## Balance-check-before-wake + +`decideAppRoute` refuses to replay when the payer is out of credits, so the +machine is never started and never bills. Enforcement is "don't wake", not +clawback — there is no credit to claw back from an account that has none. + +Order is load-bearing: the persisted `parked` status is checked **before** the +live balance read, because un-parking (and restarting) belongs to the metering +cron, not to a router that never writes. **That cron lands separately** — the +awake-seconds metering work in PR #2493, not this branch — so nothing here +un-parks anything yet; both halves ship dark behind `APP_HOSTING_ENABLED`. Conversely a `running` row whose payer +has run out is refused anyway: the row lags by up to one cron tick, the balance +does not. + +An unrecognized status resolves to `unavailable`, never to `replay` — a status +added later must not start billing machines through a router that has never +heard of it. + +## Uploads: the 1MB replay ceiling + +Fly will not replay a request whose body exceeds 1MB. The router answers such a +request with a `413` naming the limit rather than letting it surface as an opaque +502 from the platform. + +**Upload paths must go direct to Tigris via presigned URLs** and never traverse +the replay edge. No upload plumbing is built here; `MAX_REPLAYABLE_BODY_BYTES`, +`exceedsReplayableBody` and `exceedsStreamedBody` exist so the constraint is +enforced and legible at the edge. + +The limit is checked two ways, because there are two ways to arrive: + +| Request declares | Checked by | Cost | +| --- | --- | --- | +| `Content-Length` | `exceedsReplayableBody` | a header read | +| no `Content-Length` | `exceedsStreamedBody` | the body, measured, bounded at the limit | + +The second is not redundant. A request that sends no `Content-Length` gives the +header check nothing to read, so it answers false and the request would reach +`fly-replay` — where Fly, unable to replay a body over the limit, fails it at the +platform and the client sees an opaque 502 instead of the 413. This is NOT only +HTTP/1.1 `Transfer-Encoding: chunked`: HTTP/2 forbids that header entirely and +carries request content in DATA frames with no length at all, so on a modern edge +the lengthless case is the norm rather than the exception. It is the default +shape of a streaming upload too, so the header check alone is bypassed by +accident as easily as on purpose. The measured read is bounded: it stops and +cancels at the first byte past the limit, and it only ever runs for a request +that gave us no length to read, so a request that declares its size still pays +nothing. + +The edge proxy carries the same cap as defence in depth — +`request_body { max_size 1MiB }` in the `@published_apps` block of +`fly/Caddyfile.fly` in **PageSpace-Deploy**. That is not redundant with the check +here: it stops an oversized body crossing the internet into the flycast hop and +being streamed into this route only to be refused. The route keeps its own check +because it is the layer that decides whether to emit `fly-replay` at all, and +because a direct-to-web deployment has no proxy in front of it. + +> **Write it `1MiB`, never `1MB`.** Caddy parses `MB` as 1,000,000 and `MiB` as +> 1,048,576, and `MAX_REPLAYABLE_BODY_BYTES` is 1,048,576. Written as `MB` the +> two layers would disagree about every body between those two figures: the proxy +> would refuse it while the router's own 413 page names a limit that allows it — +> a refusal the user cannot reconcile with the message explaining it. + +## Configuration + +| Variable | Required | Meaning | +| --- | --- | --- | +| `APP_HOSTING_ENABLED` | to serve at all | Kill switch. Exactly `"true"` enables. | +| `PUBLISHED_APPS_APEX` | **yes, once enabled** | Apex apps serve from. Normalized (`*.`/trailing dot/case). `validateEnv` refuses to boot with `APP_HOSTING_ENABLED=true` and no explicit value — see the PSL note below. `PUBLISHED_APPS_APEX_DEFAULT` remains the fallback only while hosting is dark, so `resolvePublishedAppsApex` never returns `''`. | +| `APP_ROUTER_FLY_APP_NAME` | no | Fly app that terminates the apex and holds custom-domain certs. Falls back to `FLY_PROXY_APP_NAME`, then `pagespace-proxy`. | +| `PUBLISHED_APPS_NETWORK` | yes, in practice | The shared 6PN network. See the invariant below. | +| `APP_REPLAY_SECRET` | to emit replays | ≥32 chars. Server-held secret the per-app `state` key is derived from. Unset ⇒ the router refuses to replay. | +| `APP_ROUTER_PROXY_SECRET` | to answer at all | ≥32 chars. Shared secret the proxy presents. Unset — **or shorter than the floor** — ⇒ the endpoint refuses **everything**. | + +Both secrets fail **closed**. An unset `APP_ROUTER_PROXY_SECRET` is read as +"refuse everything", never as "skip the check" — the route is mounted on the web +app, which also answers at `pagespace.ai/api/...`, so without the check any +internet caller could hand us a hostname and collect a `fly-replay` header, +turning our own web app into a general-purpose replay emitter for the Fly org and +letting anyone wake (and bill) any published app they can name. + +## Invariant: one Fly network + +**The router app and every published app must live on the same Fly 6PN network.** +fly-replay cannot cross networks — the proxy answers +`502 cross-network replays are not allowed` — and **a Fly app's network is fixed +at create time**, so this cannot be repaired by redeploying. + +Satisfy it one of two ways, both pure configuration: + +1. create published apps on the existing router's network + (`PUBLISHED_APPS_NETWORK=`), or +2. point `APP_ROUTER_FLY_APP_NAME` at a router app that was itself created on + `PUBLISHED_APPS_NETWORK`. + +Nothing in this repo can verify which holds — the network an app was created on +is a Fly-side fact. `describeRouterNetworkInvariant()` renders the pair that has +to agree so a 502 is one log line from its cause. + +## Custom domains + +Custom hostnames attach to the **router app**, never to an individual published +app: the router is what Fly TLS-terminates, and a replay target has no public IP +at all. Certificates go through the Machines API certificates resource +(`/v1/apps/{app}/certificates`) in `services/fly/flaps-client.ts` — not the +legacy GraphQL mutations — because those responses carry `dns_requirements` and +`validation`, which name the exact records a stuck hostname is waiting on. + +`_fly-ownership` TXT pre-validation (`validators/fly-ownership.ts`) exists +because, through a certificate's status alone, "Fly has not issued yet" and "the +customer was never told to publish a record" look identical and need opposite +responses. + +A custom hostname reaching the router today answers `not_found` with reason +`custom_host`: `custom_domains` carries a `driveId` and resolves to a drive's +static published site, with no column naming a `published_apps` row. Those hosts +are served by the proxy's own custom-domain block and do not reach this route. +Binding a custom domain to a published app needs that pointer first. + +## PSL: an out-of-band action, required before GA + +Published apps serve from a **different apex** than `*.pagespace.site`, where +static canvas sites live. This is a security requirement, not tidiness. + +`pagespace.site` is **not on the Public Suffix List**, so a document served from +`a.pagespace.site` can set a `domain=.pagespace.site` cookie that every other +published site then sends. Static canvas pages already carry that risk. A +published app is strictly worse: it runs arbitrary customer-authored **server** +code on its own origin, so it can set, read and act on those cookies without a +user ever visiting the victim site. + +`PUBLISHED_APPS_APEX_DEFAULT` is the wiring for that decision. **Submitting the +apex to the PSL is not performed by this repo.** Before GA: + +- [ ] Register the apex and point its wildcard at the router app. +- [ ] Submit it to the PSL as a private-section entry + ( — PR against `public_suffix_list.dat`, + with the `_psl` DNS TXT validation record in place). +- [ ] Wait for the entry to ship in browser releases. **Listing is not + retroactive** — until a browser's bundled copy contains it, the cookie + boundary does not exist for that browser, so treat the submission date as + the start of a months-long tail rather than the fix. +- [ ] Only then serve customer-authored server code from the apex to the public. + +Until that lands, published apps must not be exposed to untrusted end users on a +shared apex. `APP_HOSTING_ENABLED` is what holds that line. + +Because the checklist above is the only thing that can satisfy the PSL +requirement — no code can verify a browser's bundled list — the boot gate does +the one thing code *can* do: `validateEnv` refuses to start with +`APP_HOSTING_ENABLED=true` and no explicit `PUBLISHED_APPS_APEX`. That converts +"the apex is whatever the default is" into a value somebody typed, and therefore +owns. It does **not** verify PSL registration, and it is not a substitute for +working the list above. diff --git a/packages/lib/src/services/app-hosting/__tests__/app-replay-key.test.ts b/packages/lib/src/services/app-hosting/__tests__/app-replay-key.test.ts new file mode 100644 index 0000000000..a86f6fdd04 --- /dev/null +++ b/packages/lib/src/services/app-hosting/__tests__/app-replay-key.test.ts @@ -0,0 +1,144 @@ +/** + * The per-app fly-replay `state` key. + * + * The property that matters: a key that authenticates traffic to app A must not + * authenticate traffic to app B, and neither must be derivable from the app name + * alone (which is not secret — it is in our logs and Fly's). Everything else here + * guards the ways a derivation like this usually fails: a weak secret accepted + * silently, an ambiguous fold, or a bearer comparison that leaks a prefix. + */ +import { describe, expect, it } from 'vitest'; +import { derivePublishedAppReplayKey, verifyPublishedAppReplayKey } from '../app-replay-key'; +import { assert } from '../../../__tests__/riteway'; + +const SECRET = 'x'.repeat(32); +const OTHER_SECRET = 'y'.repeat(32); + +const key = (flyAppName: string, secret = SECRET) => + derivePublishedAppReplayKey({ flyAppName, secret }); + +describe('derivePublishedAppReplayKey — deterministic, per app, per secret', () => { + // The two derivations are bound to separate names on purpose: comparing the + // call expression against itself reads as a self-comparison to the linter, + // and the property under test is that two SEPARATE derivations agree. + const firstDerivation = key('pgs-app-abc'); + const secondDerivation = key('pgs-app-abc'); + + assert({ + given: 'the same app name and secret twice', + should: 'derive the same key, so the router and the guest agree without exchanging anything', + actual: firstDerivation === secondDerivation, + expected: true, + }); + + assert({ + given: 'two different apps under one secret', + should: 'derive different keys, so a leaked key cannot authenticate a sibling app', + actual: key('pgs-app-abc') === key('pgs-app-def'), + expected: false, + }); + + assert({ + given: 'one app under two different secrets', + should: 'derive different keys, so rotating the secret invalidates the old key', + actual: key('pgs-app-abc') === key('pgs-app-abc', OTHER_SECRET), + expected: false, + }); + + assert({ + given: 'any app name', + should: 'be hex, so the value cannot carry the fly-replay header grammar', + actual: /^[0-9a-f]{64}$/.test(key('pgs-app-abc')), + expected: true, + }); +}); + +describe('derivePublishedAppReplayKey — fails closed on weak or ambiguous input', () => { + it.each([ + ['unset', ''], + ['31 characters — one short of the floor', 'x'.repeat(31)], + ])('given a %s secret, should throw rather than derive from weak material', (_label, secret) => { + expect(() => key('pgs-app-abc', secret)).toThrow(/at least 32/); + }); + + it('given a secret exactly at the floor, should derive', () => { + expect(() => key('pgs-app-abc', 'x'.repeat(32))).not.toThrow(); + }); + + it('given an empty app name, should throw', () => { + expect(() => key('')).toThrow(); + }); + + it('given an app name carrying the NUL delimiter, should throw rather than fold ambiguously', () => { + expect(() => key('pgs-app\0evil')).toThrow(/NUL/); + }); +}); + +describe('verifyPublishedAppReplayKey — the guest side of the contract', () => { + assert({ + given: 'the key this app derives', + should: 'accept it', + actual: verifyPublishedAppReplayKey(key('pgs-app-abc'), { + flyAppName: 'pgs-app-abc', + secret: SECRET, + }), + expected: true, + }); + + assert({ + given: "a SIBLING app's key", + should: 'reject it — this is the property the per-app derivation exists for', + actual: verifyPublishedAppReplayKey(key('pgs-app-def'), { + flyAppName: 'pgs-app-abc', + secret: SECRET, + }), + expected: false, + }); + + assert({ + given: 'a key derived under a rotated-away secret', + should: 'reject it', + actual: verifyPublishedAppReplayKey(key('pgs-app-abc', OTHER_SECRET), { + flyAppName: 'pgs-app-abc', + secret: SECRET, + }), + expected: false, + }); + + it.each([ + ['null', null], + ['undefined', undefined], + ['empty', ''], + ])('given a %s presented value, should answer false', (_label, presented) => { + expect( + verifyPublishedAppReplayKey(presented, { flyAppName: 'pgs-app-abc', secret: SECRET }), + ).toBe(false); + }); + + assert({ + given: 'a malformed secret on the GUEST side', + should: 'answer false rather than throw — a throw at this boundary becomes a fail-open catch', + actual: verifyPublishedAppReplayKey('anything', { flyAppName: 'pgs-app-abc', secret: 'short' }), + expected: false, + }); + + assert({ + given: 'a correct key with one character changed', + should: 'reject it', + actual: verifyPublishedAppReplayKey(`0${key('pgs-app-abc').slice(1)}`, { + flyAppName: 'pgs-app-abc', + secret: SECRET, + }), + expected: false, + }); + + assert({ + given: 'a correct PREFIX of the key', + should: 'reject it, so the comparison is not a prefix oracle', + actual: verifyPublishedAppReplayKey(key('pgs-app-abc').slice(0, 32), { + flyAppName: 'pgs-app-abc', + secret: SECRET, + }), + expected: false, + }); +}); diff --git a/packages/lib/src/services/app-hosting/__tests__/parked-page.test.ts b/packages/lib/src/services/app-hosting/__tests__/parked-page.test.ts new file mode 100644 index 0000000000..f1eaa29b1c --- /dev/null +++ b/packages/lib/src/services/app-hosting/__tests__/parked-page.test.ts @@ -0,0 +1,125 @@ +/** + * The page the edge serves when it refuses to wake an app. + * + * Two things are worth asserting here and they are both about HTTP semantics + * rather than copy: parked answers 402 (terminal-until-you-act) and not 503, + * because a 503 is a lie that retry machinery believes — every monitor that + * honours it would come back and re-run the balance check for an account that + * has none. And the rendered page must be self-contained and escape the one + * value it interpolates, since that value is a request header. + */ +import { describe, expect, it } from 'vitest'; +import { renderAppRouterPage, retryAfterFor, statusCodeFor } from '../parked-page'; +import type { AppRouteDecision } from '../router-core'; +import { assert } from '../../../__tests__/riteway'; + +const parked: AppRouteDecision = { kind: 'parked', reason: 'out_of_credits' }; +const deploying: AppRouteDecision = { kind: 'unavailable', reason: 'deploying' }; +const failed: AppRouteDecision = { kind: 'unavailable', reason: 'failed' }; +const missing: AppRouteDecision = { kind: 'not_found', reason: 'no_such_app' }; + +describe('statusCodeFor — enforcement is countable, not blended into outages', () => { + assert({ + given: 'an app parked for want of credits', + should: 'answer 402 Payment Required rather than a 503 that invites retries', + actual: statusCodeFor(parked), + expected: 402, + }); + + assert({ + given: 'an app parked by the metering cron', + should: 'also answer 402', + actual: statusCodeFor({ kind: 'parked', reason: 'parked_status' }), + expected: 402, + }); + + assert({ + given: 'a genuinely transient state', + should: 'answer 503', + actual: statusCodeFor(deploying), + expected: 503, + }); + + assert({ + given: 'no app at this address', + should: 'answer 404', + actual: statusCodeFor(missing), + expected: 404, + }); + + assert({ + given: 'a replay', + should: 'answer 204 — Fly consumes the response and the client never sees a body', + actual: statusCodeFor({ kind: 'replay', flyAppName: 'a', state: 'b', timeoutMs: 1500 }), + expected: 204, + }); +}); + +describe('retryAfterFor — back a caller off by how long the state will last', () => { + assert({ + given: 'a deploy in flight', + should: 'invite a retry in seconds', + actual: retryAfterFor(deploying), + expected: 15, + }); + + assert({ + given: 'a failed app waiting on a reconciler or a human', + should: 'back the caller off much further', + actual: retryAfterFor(failed), + expected: 120, + }); + + it.each([ + ['a parked app — retrying cannot change the answer', parked], + ['a missing app', missing], + ])('given %s, should offer no Retry-After', (_label, decision) => { + expect(retryAfterFor(decision)).toBeNull(); + }); +}); + +// The page is PUBLIC — anyone who visits the hostname sees it — so it must not +// claim anything about the owner that is not true. The unavailable copy used to +// say "its owner has been able to see why"; two of the four producers of that +// state are route-level outages logged server-side and surfaced to nobody. +describe('the unavailable page does not promise the owner an explanation', () => { + it('given a failed app, should not claim the owner can see the reason', () => { + const html = renderAppRouterPage(failed, 'acme.pagespace.app'); + expect(html).not.toContain('has been able to see'); + expect(html).not.toMatch(/owner.{0,30}(see|knows) why/i); + }); + + it('given a failed app, should point the one reader who can act at where to look', () => { + expect(renderAppRouterPage(failed, 'acme.pagespace.app')).toContain('check its status in PageSpace'); + }); +}); + +describe('renderAppRouterPage — self-contained, and it escapes the host header', () => { + it('given any decision, should reference no external asset that would need fetching', () => { + const html = renderAppRouterPage(parked, 'acme.pagespace.app'); + expect(html).not.toMatch(/ { + const html = renderAppRouterPage(parked, ''); + expect(html).not.toContain(''); + expect(html).toContain('<script>'); + }); + + it('given a parked app, should explain that credits ran out and nothing was lost', () => { + const html = renderAppRouterPage(parked, 'acme.pagespace.app'); + expect(html).toMatch(/credits/i); + expect(html).toContain('acme.pagespace.app'); + }); + + it.each([ + ['parked', parked], + ['deploying', deploying], + ['failed', failed], + ['not_found', missing], + ])('given a %s decision, should render a complete document with a title', (_label, decision) => { + const html = renderAppRouterPage(decision, 'acme.pagespace.app'); + expect(html.startsWith('')).toBe(true); + expect(html).toMatch(/.+<\/title>/); + }); +}); diff --git a/packages/lib/src/services/app-hosting/__tests__/router-core.test.ts b/packages/lib/src/services/app-hosting/__tests__/router-core.test.ts new file mode 100644 index 0000000000..93058444b8 --- /dev/null +++ b/packages/lib/src/services/app-hosting/__tests__/router-core.test.ts @@ -0,0 +1,369 @@ +/** + * router-core — the enforcement property, tested without a database or a clock. + * + * The claim under test is narrow and load-bearing: **an app whose payer is out of + * credits is never replayed to**, so its machine is never auto-started, so it + * never bills. Every other case here exists to stop that claim being satisfied + * vacuously — by a router that also refuses solvent apps, or by one that reads + * an unknown status as servable. + */ +import { describe, expect, it } from 'vitest'; +import { + FLY_REPLAY_TIMEOUT_MS, + MAX_REPLAYABLE_BODY_BYTES, + buildFlyReplayHeader, + decideAppRoute, + exceedsReplayableBody, + exceedsStreamedBody, + normalizeRequestHost, + parseAppHost, + replayCachePolicyFor, + type RoutableApp, +} from '../router-core'; +import { assert } from '../../../__tests__/riteway'; + +const APEX = 'pagespace.app'; + +/** A servable, solvent, metered app — the baseline every case perturbs. */ +function app(overrides: Partial<RoutableApp> = {}): RoutableApp { + return { + flyAppName: 'pgs-app-abc123', + status: 'running', + tier: 'metered', + hasMachine: true, + ...overrides, + }; +} + +function route(appRow: RoutableApp | null, balanceOk = true) { + return decideAppRoute({ app: appRow, balanceOk, replayState: 'deadbeef' }); +} + +describe('decideAppRoute — the balance gate is the wake gate', () => { + assert({ + given: 'a running metered app whose payer is out of credits', + should: 'refuse to replay, so the machine is never started', + actual: route(app(), false), + expected: { kind: 'parked', reason: 'out_of_credits' }, + }); + + assert({ + given: 'a STOPPED metered app whose payer is out of credits', + should: 'still refuse — a stopped machine is exactly the one a replay would wake', + actual: route(app({ status: 'stopped' }), false), + expected: { kind: 'parked', reason: 'out_of_credits' }, + }); + + assert({ + given: 'a running metered app whose payer can still spend', + should: 'replay, so the gate is not passing by refusing everyone', + actual: route(app(), true), + expected: { + kind: 'replay', + flyAppName: 'pgs-app-abc123', + state: 'deadbeef', + timeoutMs: FLY_REPLAY_TIMEOUT_MS, + }, + }); + + assert({ + given: 'a DEDICATED app whose payer is out of credits', + should: 'replay anyway — a flat-rate app has no balance gate to fail', + actual: route(app({ tier: 'dedicated' }), false).kind, + expected: 'replay', + }); +}); + +describe('decideAppRoute — status precedes the live balance read', () => { + assert({ + given: 'a parked app whose payer has since topped up', + should: 'stay parked — un-parking belongs to the cron, not to a router that never writes', + actual: route(app({ status: 'parked' }), true), + expected: { kind: 'parked', reason: 'parked_status' }, + }); + + assert({ + given: 'an app mid-first-deploy with no machine yet', + should: 'answer deploying — fly-replay auto-starts a machine, it cannot create one', + actual: route(app({ hasMachine: false })), + expected: { kind: 'unavailable', reason: 'deploying' }, + }); + + it.each([ + ['destroying', 'destroying'], + ['failed', 'failed'], + ])('given status %s, should be unavailable', (status, reason) => { + expect(route(app({ status }))).toEqual({ kind: 'unavailable', reason }); + }); + + assert({ + given: 'a status this file has never been taught', + should: 'fail CLOSED to unavailable, so a status added later cannot start billing machines', + actual: route(app({ status: 'some_status_added_in_2027' })), + expected: { kind: 'unavailable', reason: 'deploying' }, + }); + + assert({ + given: 'no row for the hostname', + should: 'answer not_found', + actual: route(null), + expected: { kind: 'not_found', reason: 'no_such_app' }, + }); + + assert({ + given: 'a deploying app that already has a machine', + should: 'replay — a rolling deploy still serves the previous version', + actual: route(app({ status: 'deploying' })).kind, + expected: 'replay', + }); +}); + +describe('parseAppHost — only a single label under the apex is an app', () => { + assert({ + given: 'a single label under the apex', + should: 'resolve to that subdomain', + actual: parseAppHost('acme.pagespace.app', APEX), + expected: { kind: 'subdomain', subdomain: 'acme' }, + }); + + assert({ + given: 'a NESTED label under the apex', + should: 'refuse — the wildcard cert covers one level, and evil.acme.* must not present as an app', + actual: parseAppHost('evil.acme.pagespace.app', APEX), + expected: { kind: 'foreign', hostname: 'evil.acme.pagespace.app' }, + }); + + assert({ + given: 'the apex itself', + should: 'be its own outcome, not an app named ""', + actual: parseAppHost('pagespace.app', APEX), + expected: { kind: 'apex' }, + }); + + assert({ + given: 'a hostname that merely ENDS with the apex text but is not under it', + should: 'be foreign — notpagespace.app is a different registrable domain', + actual: parseAppHost('notpagespace.app', APEX), + expected: { kind: 'foreign', hostname: 'notpagespace.app' }, + }); + + assert({ + given: 'a custom domain', + should: 'be foreign, so the proxy custom-domain block keeps serving it', + actual: parseAppHost('docs.acme.com', APEX), + expected: { kind: 'foreign', hostname: 'docs.acme.com' }, + }); + + assert({ + given: 'an EMPTY apex (misconfiguration)', + should: 'treat the host as foreign rather than making every hostname an app', + actual: parseAppHost('acme.pagespace.app', '').kind, + expected: 'foreign', + }); + + assert({ + given: 'a label at the 63-character DNS limit', + should: 'be accepted', + actual: parseAppHost(`${'a'.repeat(63)}.pagespace.app`, APEX).kind, + expected: 'subdomain', + }); + + assert({ + given: 'a label one character past the DNS limit', + should: 'be refused', + actual: parseAppHost(`${'a'.repeat(64)}.pagespace.app`, APEX).kind, + expected: 'foreign', + }); + + it.each(['-lead.pagespace.app', 'trail-.pagespace.app', 'under_score.pagespace.app'])( + 'given the invalid label %s, should be foreign', + (host) => { + expect(parseAppHost(host, APEX).kind).toBe('foreign'); + }, + ); + + assert({ + given: 'an uppercased host with a port and a trailing dot', + should: 'normalize to the same subdomain', + actual: parseAppHost('ACME.PageSpace.app.:8080', APEX), + expected: { kind: 'subdomain', subdomain: 'acme' }, + }); + + assert({ + given: 'an apex configured with a leading wildcard and trailing dot', + should: 'still match, since resolvePublishedAppsApex normalizes both', + actual: parseAppHost('acme.pagespace.app', 'PageSpace.app'), + expected: { kind: 'subdomain', subdomain: 'acme' }, + }); +}); + +describe('normalizeRequestHost — an IPv6 literal must not be amputated', () => { + assert({ + given: 'a bracketed IPv6 literal with a port', + should: 'strip the port and keep the whole address', + actual: normalizeRequestHost('[2001:db8::1]:8080'), + expected: '[2001:db8::1]', + }); + + assert({ + given: 'a bracketed IPv6 literal with NO port', + should: 'leave the address intact rather than cutting at its last colon', + actual: normalizeRequestHost('[2001:db8::1]'), + expected: '[2001:db8::1]', + }); +}); + +describe('buildFlyReplayHeader — a value that can inject a directive is refused', () => { + assert({ + given: 'a server-derived app name and state', + should: 'render the header including the timeout that collapses the cold-start stall', + actual: buildFlyReplayHeader({ flyAppName: 'pgs-app-abc', state: 'ff00', timeoutMs: 1500 }), + expected: 'app=pgs-app-abc;state=ff00;timeout=1500', + }); + + it.each([ + ['a semicolon', 'pgs-app;state=evil'], + ['an equals sign', 'pgs-app=x'], + ['a comma', 'pgs-app,evil'], + ['whitespace', 'pgs app'], + ])('given an app name containing %s, should throw rather than emit a redirectable header', (_l, name) => { + expect(() => buildFlyReplayHeader({ flyAppName: name, state: 'ff', timeoutMs: 1500 })).toThrow(); + }); + + it('given a state carrying header grammar, should throw', () => { + expect(() => + buildFlyReplayHeader({ flyAppName: 'pgs-app', state: 'a;app=victim', timeoutMs: 1500 }), + ).toThrow(); + }); + + it.each([ + ['empty', ''], + ])('given a %s app name, should throw', (_l, name) => { + expect(() => buildFlyReplayHeader({ flyAppName: name, state: 'ff', timeoutMs: 1500 })).toThrow(); + }); + + it.each([0, -1, 1.5, Number.NaN])('given the invalid timeout %s, should throw', (timeoutMs) => { + expect(() => buildFlyReplayHeader({ flyAppName: 'a', state: 'b', timeoutMs })).toThrow(); + }); + + assert({ + given: "Fly's documented cold-start stall", + should: 'be collapsed to 1500ms rather than the ~7.5s default', + actual: FLY_REPLAY_TIMEOUT_MS, + expected: 1500, + }); +}); + +describe('exceedsReplayableBody — the 1MB replay ceiling', () => { + assert({ + given: 'a body exactly at the limit', + should: 'be replayable', + actual: exceedsReplayableBody(String(MAX_REPLAYABLE_BODY_BYTES)), + expected: false, + }); + + assert({ + given: 'a body one byte past the limit', + should: 'be refused at the edge rather than 502-ing at Fly', + actual: exceedsReplayableBody(String(MAX_REPLAYABLE_BODY_BYTES + 1)), + expected: true, + }); + + it.each([ + ['a request declaring no length', null], + ['an absent header', undefined], + ['a non-numeric header', 'banana'], + ['a negative length', '-5'], + ])('given %s, should answer false rather than guess', (_label, header) => { + expect(exceedsReplayableBody(header)).toBe(false); + }); + + assert({ + given: "Fly's replay body ceiling", + should: 'be 1MB, the value the upload-to-Tigris constraint is derived from', + actual: MAX_REPLAYABLE_BODY_BYTES, + expected: 1_048_576, + }); +}); + +describe('exceedsStreamedBody — the half the header check cannot see', () => { + /** A body delivered in pieces, the way a request with no Content-Length arrives. */ + const streamOf = (...sizes: number[]): ReadableStream<Uint8Array> => + new ReadableStream<Uint8Array>({ + start(controller) { + for (const size of sizes) controller.enqueue(new Uint8Array(size)); + controller.close(); + }, + }); + + it('given no body at all, should answer false without reading', async () => { + expect(await exceedsStreamedBody(null)).toBe(false); + expect(await exceedsStreamedBody(undefined)).toBe(false); + }); + + it('given a stream exactly at the limit, should be replayable', async () => { + expect(await exceedsStreamedBody(streamOf(MAX_REPLAYABLE_BODY_BYTES))).toBe(false); + }); + + it('given a stream one byte past the limit, should be refused', async () => { + expect(await exceedsStreamedBody(streamOf(MAX_REPLAYABLE_BODY_BYTES + 1))).toBe(true); + }); + + // The bypass this function exists to close: the total is what matters, and a + // sender that splits a large body into small chunks is the ordinary case, not + // an attack. Summing per chunk rather than judging any single one is the point. + it('given many small chunks summing past the limit, should be refused', async () => { + const chunk = 64 * 1024; + const chunks = Array.from({ length: MAX_REPLAYABLE_BODY_BYTES / chunk + 1 }, () => chunk); + expect(await exceedsStreamedBody(streamOf(...chunks))).toBe(true); + }); + + it('given an empty stream, should be replayable', async () => { + expect(await exceedsStreamedBody(streamOf())).toBe(false); + }); + + // Bounded, not buffered: the read stops at the first byte past the limit + // rather than draining a body of unknown size into memory. + it('given an endless stream, should terminate at the limit instead of reading forever', async () => { + let enqueued = 0; + const endless = new ReadableStream<Uint8Array>({ + pull(controller) { + enqueued += 1; + controller.enqueue(new Uint8Array(64 * 1024)); + }, + }); + + expect(await exceedsStreamedBody(endless)).toBe(true); + // 1MB / 64KB = 16 chunks to reach the limit, 17 to pass it. A read that kept + // going would be unbounded, so the assertion is that it stopped promptly. + expect(enqueued).toBeLessThanOrEqual(18); + }); + + it('given a custom limit, should honour it rather than the 1MB constant', async () => { + expect(await exceedsStreamedBody(streamOf(11), 10)).toBe(true); + expect(await exceedsStreamedBody(streamOf(10), 10)).toBe(false); + }); +}); + +describe('replayCachePolicyFor — the cache would skip the gate', () => { + assert({ + given: 'a metered app', + should: 'never be cacheable, because the router hop IS the balance check', + actual: replayCachePolicyFor('metered'), + expected: 'no-cache', + }); + + assert({ + given: 'a dedicated app', + should: 'be cacheable — flat-rate billing has no gate to bypass', + actual: replayCachePolicyFor('dedicated'), + expected: 'cacheable', + }); + + assert({ + given: 'an unrecognized tier', + should: 'fail closed to no-cache', + actual: replayCachePolicyFor('experimental'), + expected: 'no-cache', + }); +}); diff --git a/packages/lib/src/services/app-hosting/__tests__/router.test.ts b/packages/lib/src/services/app-hosting/__tests__/router.test.ts new file mode 100644 index 0000000000..666679aab2 --- /dev/null +++ b/packages/lib/src/services/app-hosting/__tests__/router.test.ts @@ -0,0 +1,303 @@ +/** + * router — the imperative shell, tested through injected deps. + * + * The pure decision is covered in `router-core.test.ts`. What this file asserts + * is what the SHELL adds, and each of those is a place the enforcement property + * could be lost without the pure test noticing: + * + * • the kill switch short-circuits BEFORE any database read; + * • the balance is asked about the row's OWN payer (`ownerId`), the same + * column the awake-seconds meter charges; + * • the ledger is not consulted at all for a dedicated app; + * • a router that cannot derive a replay key refuses rather than emitting a + * replay with a blank state; + * • a real failure (database down) propagates instead of reading as a miss. + */ +import { describe, expect, it, vi } from 'vitest'; + +vi.mock('@pagespace/db/db', () => ({ db: { select: vi.fn() } })); +vi.mock('@pagespace/db/schema/published-apps', () => ({ + publishedApps: { + id: 'id', + flyAppName: 'flyAppName', + status: 'status', + tier: 'tier', + machineId: 'machineId', + ownerId: 'ownerId', + subdomain: 'subdomain', + }, +})); +vi.mock('@pagespace/db/operators', () => ({ eq: (a: unknown, b: unknown) => ({ eq: [a, b] }) })); +// The billing modules are mocked rather than imported: every balance read in this +// file goes through an injected dep, so pulling in the real credit gate would drag +// the whole ledger schema into a suite that never calls it. Their own semantics are +// covered in `billing/__tests__/credit-gate.test.ts`. +vi.mock('../../../billing/credit-gate', () => ({ hasSpendableBalance: vi.fn() })); +vi.mock('../../../billing/credit-balance', () => ({ resolveTier: vi.fn() })); + +import { + defaultAppRouterDeps, + resolveAppRoute, + type AppRouterDeps, + type PublishedAppRouteRow, +} from '../router'; +import { db } from '@pagespace/db/db'; +import { publishedApps } from '@pagespace/db/schema/published-apps'; +import { hasSpendableBalance } from '../../../billing/credit-gate'; +import { resolveTier } from '../../../billing/credit-balance'; +import { isAppHostingEnabled } from '../app-hosting-env'; +import { resolveAppReplaySecret, resolvePublishedAppsApex } from '../routing-env'; + +const SECRET = 'a'.repeat(48); + +function row(overrides: Partial<PublishedAppRouteRow> = {}): PublishedAppRouteRow { + return { + id: 'app_1', + flyAppName: 'pgs-app-abc123', + status: 'running', + tier: 'metered', + machineId: 'm-1', + ownerId: 'user_payer', + ...overrides, + }; +} + +function deps(overrides: Partial<AppRouterDeps> = {}): AppRouterDeps { + return { + isEnabled: () => true, + apex: () => 'pagespace.app', + replaySecret: () => SECRET, + findAppBySubdomain: async () => row(), + resolveTier: async () => 'pro', + hasSpendableBalance: async () => true, + ...overrides, + }; +} + +describe('resolveAppRoute — the kill switch is checked before the database', () => { + it('given hosting is disabled, should answer hosting_disabled without any lookup', async () => { + const findAppBySubdomain = vi.fn(); + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ isEnabled: () => false, findAppBySubdomain }), + ); + expect(decision).toEqual({ kind: 'unavailable', reason: 'hosting_disabled' }); + expect(findAppBySubdomain).not.toHaveBeenCalled(); + }); +}); + +describe('resolveAppRoute — hostname resolution', () => { + it('given the apex itself, should answer not_found without a lookup', async () => { + const findAppBySubdomain = vi.fn(); + const decision = await resolveAppRoute('pagespace.app', deps({ findAppBySubdomain })); + expect(decision).toEqual({ kind: 'not_found', reason: 'apex' }); + expect(findAppBySubdomain).not.toHaveBeenCalled(); + }); + + it('given a custom domain, should answer custom_host so the proxy keeps serving it', async () => { + const findAppBySubdomain = vi.fn(); + const decision = await resolveAppRoute('docs.acme.com', deps({ findAppBySubdomain })); + expect(decision).toEqual({ kind: 'not_found', reason: 'custom_host' }); + expect(findAppBySubdomain).not.toHaveBeenCalled(); + }); + + it('given a subdomain with no row, should answer no_such_app', async () => { + const decision = await resolveAppRoute( + 'nobody.pagespace.app', + deps({ findAppBySubdomain: async () => null }), + ); + expect(decision).toEqual({ kind: 'not_found', reason: 'no_such_app' }); + }); + + it('given a host with a port, should look up the normalized label', async () => { + const findAppBySubdomain = vi.fn(async () => row()); + await resolveAppRoute('ACME.pagespace.app:443', deps({ findAppBySubdomain })); + expect(findAppBySubdomain).toHaveBeenCalledWith('acme'); + }); +}); + +describe('resolveAppRoute — the balance is asked about the row own payer', () => { + it("given a metered app, should ask about the row's ownerId, not any other user", async () => { + const hasSpendableBalance = vi.fn(async () => true); + const resolveTier = vi.fn(async () => 'pro'); + await resolveAppRoute( + 'acme.pagespace.app', + deps({ + findAppBySubdomain: async () => row({ ownerId: 'user_drive_owner' }), + resolveTier, + hasSpendableBalance, + }), + ); + expect(resolveTier).toHaveBeenCalledWith('user_drive_owner'); + expect(hasSpendableBalance).toHaveBeenCalledWith('user_drive_owner', 'pro'); + }); + + it('given an insolvent payer, should park rather than replay', async () => { + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ hasSpendableBalance: async () => false }), + ); + expect(decision).toEqual({ kind: 'parked', reason: 'out_of_credits' }); + }); + + it('given a DEDICATED app, should never touch the ledger at all', async () => { + const hasSpendableBalance = vi.fn(async () => false); + const resolveTier = vi.fn(async () => 'pro'); + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ + findAppBySubdomain: async () => row({ tier: 'dedicated' }), + resolveTier, + hasSpendableBalance, + }), + ); + expect(decision.kind).toBe('replay'); + expect(resolveTier).not.toHaveBeenCalled(); + expect(hasSpendableBalance).not.toHaveBeenCalled(); + }); + + it('given an app that is not servable anyway, should skip the balance read', async () => { + const hasSpendableBalance = vi.fn(async () => true); + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ findAppBySubdomain: async () => row({ status: 'parked' }), hasSpendableBalance }), + ); + expect(decision).toEqual({ kind: 'parked', reason: 'parked_status' }); + expect(hasSpendableBalance).not.toHaveBeenCalled(); + }); +}); + +describe('resolveAppRoute — the replay key must exist before traffic is replayed', () => { + it('given a solvent servable app, should emit a replay carrying a derived state key', async () => { + const decision = await resolveAppRoute('acme.pagespace.app', deps()); + expect(decision.kind).toBe('replay'); + if (decision.kind !== 'replay') throw new Error('expected a replay'); + expect(decision.flyAppName).toBe('pgs-app-abc123'); + // Derived, hex, and not the placeholder the provisional decision carries. + expect(decision.state).toMatch(/^[0-9a-f]{64}$/); + expect(decision.state).not.toBe('pending'); + }); + + it('given an UNSET replay secret, should refuse rather than replay with a blank state', async () => { + const decision = await resolveAppRoute('acme.pagespace.app', deps({ replaySecret: () => '' })); + expect(decision).toEqual({ kind: 'unavailable', reason: 'failed' }); + }); + + it('given a too-short replay secret, should refuse', async () => { + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ replaySecret: () => 'short' }), + ); + expect(decision).toEqual({ kind: 'unavailable', reason: 'failed' }); + }); + + it('given two different apps, should derive different state keys', async () => { + const a = await resolveAppRoute( + 'a.pagespace.app', + deps({ findAppBySubdomain: async () => row({ flyAppName: 'pgs-app-aaa' }) }), + ); + const b = await resolveAppRoute( + 'b.pagespace.app', + deps({ findAppBySubdomain: async () => row({ flyAppName: 'pgs-app-bbb' }) }), + ); + if (a.kind !== 'replay' || b.kind !== 'replay') throw new Error('expected replays'); + expect(a.state).not.toBe(b.state); + }); +}); + +describe('resolveAppRoute — an outage is not a miss', () => { + it('given the lookup throws, should propagate so the caller can answer 503', async () => { + await expect( + resolveAppRoute( + 'acme.pagespace.app', + deps({ + findAppBySubdomain: async () => { + throw new Error('connection terminated'); + }, + }), + ), + ).rejects.toThrow('connection terminated'); + }); + + it('given the balance read throws, should propagate rather than park the app', async () => { + await expect( + resolveAppRoute( + 'acme.pagespace.app', + deps({ + hasSpendableBalance: async () => { + throw new Error('ledger unavailable'); + }, + }), + ), + ).rejects.toThrow('ledger unavailable'); + }); +}); + + +/** + * The composition root. + * + * Every other test in this file injects its own deps, and the route test mocks + * this module wholesale — so `defaultAppRouterDeps`, the object that decides what + * ACTUALLY runs at the serving edge, was asserted by nothing. That is a worse gap + * than it sounds: a mistake here is invisible to every mutation check on the + * decision function, because the decision function is not what is wrong. + * + * Bind `hasSpendableBalance` to `getCreditBalance` instead of the read-only twin + * and the per-request `SUM` over `credit_holds` comes back — every test still + * passes. Point `isEnabled` at anything truthy and hosting serves while the flag + * says dark — every test still passes. So the wiring is asserted directly. + */ +describe('defaultAppRouterDeps — the real edge is wired to the real readers', () => { + it('binds the kill switch, the apex and the replay secret by identity', () => { + expect(defaultAppRouterDeps.isEnabled).toBe(isAppHostingEnabled); + expect(defaultAppRouterDeps.apex).toBe(resolvePublishedAppsApex); + expect(defaultAppRouterDeps.replaySecret).toBe(resolveAppReplaySecret); + }); + + // `resolveTier` and `hasSpendableBalance` are wrapped in arrows for the tier + // cast, so identity cannot be asserted for them — and unwrapping them just to + // make `toBe` work would delete the thing being checked. Asserted behaviourally + // instead: the wrapper must delegate to the real module, with its own arguments. + it('delegates the balance read to the read-only twin, with the arguments it was given', async () => { + vi.mocked(hasSpendableBalance).mockResolvedValue(true); + + const answer = await defaultAppRouterDeps.hasSpendableBalance('user_payer', 'metered'); + + expect(hasSpendableBalance).toHaveBeenCalledWith('user_payer', 'metered'); + expect(answer).toBe(true); + }); + + it('delegates the tier lookup, and returns what it answers', async () => { + vi.mocked(resolveTier).mockResolvedValue('pro'); + + const tier = await defaultAppRouterDeps.resolveTier('user_payer'); + + expect(resolveTier).toHaveBeenCalledWith('user_payer'); + expect(tier).toBe('pro'); + }); + + // The row reader is module-private, so it is pinned by what it queries: the + // published_apps table, keyed on `subdomain`, one row. + it('reads the published_apps row for the subdomain it is asked about', async () => { + const found = row(); + const limit = vi.fn().mockResolvedValue([found]); + const where = vi.fn(() => ({ limit })); + const from = vi.fn(() => ({ where })); + vi.mocked(db.select).mockReturnValue({ from } as never); + + const result = await defaultAppRouterDeps.findAppBySubdomain('acme'); + + expect(from).toHaveBeenCalledWith(publishedApps); + expect(where).toHaveBeenCalledWith({ eq: [publishedApps.subdomain, 'acme'] }); + expect(limit).toHaveBeenCalledWith(1); + expect(result).toEqual(found); + }); + + it('answers null when the subdomain matches no row, rather than undefined', async () => { + const limit = vi.fn().mockResolvedValue([]); + vi.mocked(db.select).mockReturnValue({ from: () => ({ where: () => ({ limit }) }) } as never); + + expect(await defaultAppRouterDeps.findAppBySubdomain('nope')).toBeNull(); + }); +}); diff --git a/packages/lib/src/services/app-hosting/__tests__/routing-env.test.ts b/packages/lib/src/services/app-hosting/__tests__/routing-env.test.ts new file mode 100644 index 0000000000..620731f2b0 --- /dev/null +++ b/packages/lib/src/services/app-hosting/__tests__/routing-env.test.ts @@ -0,0 +1,154 @@ +/** + * The serving edge's configuration surface. + * + * The failure modes worth pinning are all "an unset variable silently disables a + * protection": an empty apex would make every hostname look like a published-app + * subdomain, an unset replay secret would emit replays with a blank state, and an + * unset proxy secret would leave the router endpoint world-callable. Each of + * those resolves to a value that fails CLOSED, and that is what is asserted here. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import { + APP_ROUTER_FLY_APP_DEFAULT, + APP_ROUTER_HOST_HEADER, + APP_ROUTER_KEY_HEADER, + MIN_ROUTER_SECRET_LENGTH, + PUBLISHED_APPS_APEX_DEFAULT, + describeRouterNetworkInvariant, + resolveAppReplaySecret, + resolveAppRouterFlyAppName, + resolveAppRouterProxySecret, + resolvePublishedAppsApex, +} from '../routing-env'; +import { parseAppHost } from '../router-core'; +import { assert } from '../../../__tests__/riteway'; + +const ORIGINAL = { ...process.env }; +afterEach(() => { + process.env = { ...ORIGINAL }; +}); + +describe('resolvePublishedAppsApex — never empty, whatever the configuration', () => { + it('given PUBLISHED_APPS_APEX is unset, should fall back to the default apex', () => { + delete process.env.PUBLISHED_APPS_APEX; + expect(resolvePublishedAppsApex()).toBe(PUBLISHED_APPS_APEX_DEFAULT); + }); + + it.each([ + ['empty', ''], + ['whitespace only', ' '], + ])( + 'given a %s override, should fall back rather than yield "" (which would make every host an app)', + (_label, value) => { + process.env.PUBLISHED_APPS_APEX = value; + expect(resolvePublishedAppsApex()).toBe(PUBLISHED_APPS_APEX_DEFAULT); + // The reason the fallback matters, asserted end to end. + expect(parseAppHost('victim.example.com', resolvePublishedAppsApex()).kind).toBe('foreign'); + }, + ); + + it.each([ + ['a wildcard prefix', '*.apps.example.com'], + ['a leading dot', '.apps.example.com'], + ['a trailing dot', 'apps.example.com.'], + ['mixed case', 'Apps.Example.COM'], + ['surrounding whitespace', ' apps.example.com '], + ])('given %s, should normalize to the bare apex', (_label, value) => { + process.env.PUBLISHED_APPS_APEX = value; + expect(resolvePublishedAppsApex()).toBe('apps.example.com'); + }); + + assert({ + given: 'the default published-apps apex', + should: 'be a DIFFERENT registrable domain from the pagespace.site canvas apex (the PSL cookie risk)', + actual: PUBLISHED_APPS_APEX_DEFAULT.endsWith('pagespace.site'), + expected: false, + }); +}); + +describe('resolveAppRouterFlyAppName — one app, reachable under either variable', () => { + it('given neither variable, should fall back to the proxy app', () => { + delete process.env.APP_ROUTER_FLY_APP_NAME; + delete process.env.FLY_PROXY_APP_NAME; + expect(resolveAppRouterFlyAppName()).toBe(APP_ROUTER_FLY_APP_DEFAULT); + }); + + it('given only the legacy FLY_PROXY_APP_NAME, should use it, so existing deployments keep working', () => { + delete process.env.APP_ROUTER_FLY_APP_NAME; + process.env.FLY_PROXY_APP_NAME = 'legacy-proxy'; + expect(resolveAppRouterFlyAppName()).toBe('legacy-proxy'); + }); + + it('given both, should prefer the explicit APP_ROUTER_FLY_APP_NAME', () => { + process.env.APP_ROUTER_FLY_APP_NAME = 'app-router'; + process.env.FLY_PROXY_APP_NAME = 'legacy-proxy'; + expect(resolveAppRouterFlyAppName()).toBe('app-router'); + }); + + it('given a blank explicit value, should fall through rather than name an empty app', () => { + process.env.APP_ROUTER_FLY_APP_NAME = ' '; + process.env.FLY_PROXY_APP_NAME = 'legacy-proxy'; + expect(resolveAppRouterFlyAppName()).toBe('legacy-proxy'); + }); +}); + +describe('the two secrets fail closed when unset', () => { + it('given APP_REPLAY_SECRET is unset, should resolve to "" so no replay can be signed', () => { + delete process.env.APP_REPLAY_SECRET; + expect(resolveAppReplaySecret()).toBe(''); + }); + + it('given APP_ROUTER_PROXY_SECRET is unset, should resolve to "" — which the route reads as refuse-everything', () => { + delete process.env.APP_ROUTER_PROXY_SECRET; + expect(resolveAppRouterProxySecret()).toBe(''); + }); + + it('given the secrets are configured above the floor, should return them verbatim', () => { + process.env.APP_REPLAY_SECRET = 'r'.repeat(40); + process.env.APP_ROUTER_PROXY_SECRET = 'p'.repeat(40); + expect(resolveAppReplaySecret()).toBe('r'.repeat(40)); + expect(resolveAppRouterProxySecret()).toBe('p'.repeat(40)); + }); + + // A guessable proxy secret is not a weaker check, it is the absence of one: + // the route compares the header against this value, so a short secret leaves + // the endpoint a world-callable fly-replay emitter. It has to read as unset. + it.each([ + ['one character', 'a'], + ['one below the floor', 'p'.repeat(MIN_ROUTER_SECRET_LENGTH - 1)], + ])( + 'given APP_ROUTER_PROXY_SECRET is %s, should resolve to "" so the route refuses everything', + (_label, value) => { + process.env.APP_ROUTER_PROXY_SECRET = value; + expect(resolveAppRouterProxySecret()).toBe(''); + }, + ); + + it('given a secret exactly at the floor, should accept it', () => { + process.env.APP_ROUTER_PROXY_SECRET = 'p'.repeat(MIN_ROUTER_SECRET_LENGTH); + expect(resolveAppRouterProxySecret()).toBe('p'.repeat(MIN_ROUTER_SECRET_LENGTH)); + }); +}); + +describe('the header names the proxy and the route have to agree on', () => { + assert({ + given: 'the host and key header constants', + should: 'be lowercase, since that is how they are read back off a Request', + actual: [APP_ROUTER_HOST_HEADER, APP_ROUTER_KEY_HEADER], + expected: ['x-pagespace-app-host', 'x-pagespace-app-router-key'], + }); +}); + +describe('describeRouterNetworkInvariant — renders the pair that must agree', () => { + it('given a configured router app, should report it alongside the published-apps network', () => { + process.env.APP_ROUTER_FLY_APP_NAME = 'app-router'; + process.env.PUBLISHED_APPS_NETWORK = 'pagespace-apps'; + const described = describeRouterNetworkInvariant(); + expect(described.routerApp).toBe('app-router'); + expect(described.publishedAppsNetwork).toBe('pagespace-apps'); + // The note is what turns a 502 into a one-line diagnosis, so it has to name + // the actual failure rather than gesture at configuration. + expect(described.note).toMatch(/cross/i); + expect(described.note).toMatch(/fixed at create time/i); + }); +}); diff --git a/packages/lib/src/services/app-hosting/app-replay-key.ts b/packages/lib/src/services/app-hosting/app-replay-key.ts new file mode 100644 index 0000000000..d3131f7f18 --- /dev/null +++ b/packages/lib/src/services/app-hosting/app-replay-key.ts @@ -0,0 +1,105 @@ +/** + * Per-published-app fly-replay `state` key (pure). + * + * `fly-replay: app=<target>;state=<key>` carries an opaque string that Fly hands + * to the target app verbatim in `fly-replay-src`. Its purpose is authentication + * in ONE direction: a published app must be able to tell "this request was routed + * by our router, which checked status and balance" from "this request reached me + * some other way". Without it, anything inside the shared 6PN network could talk + * straight to a published app and consume awake-seconds the balance gate would + * have refused. + * + * So the key is PER APP, derived rather than stored: an HMAC over the Fly app + * name under a server-held secret. Per app because one leaked key must not + * authenticate traffic to a sibling app; derived because a stored column would + * be one more secret to rotate, migrate and leak, and there is nothing to store + * that the name plus the secret does not already determine. + * + * The unguessability comes from `APP_REPLAY_SECRET`, never from the app name — + * `pgs-app-<cuid2>` is not secret (it is in our own logs, and Fly's). Rotation is + * a secret change plus a redeploy of the router and the guest runtime; there is + * deliberately no per-app rotation, because a per-app key that can be rotated + * independently is a per-app key that has to be stored. + * + * Shape (namespace, NUL-delimited fold, sha3-256 HMAC, >=32-char secret floor) is + * copied from `drive-envs/env-sprite-key.ts` and `agent-workspaces/ + * workspace-sprite-key.ts`. The namespace is FRESH and that is a requirement, not + * a convention: a shared namespace would put replay keys and Sprite NAMES in one + * keyspace, where a value minted as an authentication token also names a machine. + */ + +import { createHmac } from 'crypto'; +import { secureCompare } from '../../auth/secure-compare'; + +const NAMESPACE_VERSION = 'published-app-replay:v1'; + +/** + * Re-checked here rather than trusted from the caller: the web env schema is not + * the only reader (the guest runtime and the realtime service bypass full + * validation), and a too-short secret must fail CLOSED — a denied route costs a + * 503, deriving from weak material silently weakens every app's key at once. + */ +const MIN_SECRET_LENGTH = 32; + +export interface PublishedAppReplayKeyInput { + /** `published_apps.flyAppName` — the replay target, and the identity fold. */ + flyAppName: string; + /** The server-held `APP_REPLAY_SECRET`; never user input. */ + secret: string; +} + +/** + * The `state=` value for this app's replays. Deterministic: the router and the + * guest runtime derive the same string from the same two inputs, with nothing + * exchanged between them. + * + * Hex, so the value is safe in the `fly-replay` header's `k=v;k=v` grammar — + * a key containing `;` or `=` would let a crafted app name inject a second + * directive into the header. + */ +export function derivePublishedAppReplayKey({ flyAppName, secret }: PublishedAppReplayKeyInput): string { + if (secret.length < MIN_SECRET_LENGTH) { + throw new Error( + `derivePublishedAppReplayKey requires a secret of at least ${MIN_SECRET_LENGTH} characters`, + ); + } + if (flyAppName.length === 0) { + throw new Error('derivePublishedAppReplayKey requires a non-empty flyAppName'); + } + // The NUL delimiter only makes the fold injective if no component can carry + // one. Fly app names cannot today, but this function is the boundary. + if (flyAppName.includes('\0')) { + throw new Error('derivePublishedAppReplayKey requires a flyAppName without the NUL delimiter'); + } + const payload = [NAMESPACE_VERSION, flyAppName].join('\0'); + // codeql[js/insufficient-password-hash] not a password hash — a keyed HMAC over APP_REPLAY_SECRET (a >=32-char server secret, never user input) deriving a deterministic per-app preshared key, same as drive-envs/env-sprite-key.ts + return createHmac('sha3-256', secret).update(payload).digest('hex'); +} + +/** + * Validate a `fly-replay-src` state value against this app's derived key. + * + * For the GUEST side of the contract — the published app's own runtime — which is + * not built in this task; it is exported here so the two halves can never drift + * to two derivations. Comparison goes through {@link secureCompare} (SHA3-256 + * both sides, then `timingSafeEqual`): the key is a bearer credential, and a + * naive `===` on it leaks a prefix oracle. + * + * A malformed or missing secret makes this return FALSE rather than throw — the + * guest's answer to "was this router traffic?" must be "no" when it cannot tell, + * and a throw at that boundary is an exception path that tends to become a + * fail-open catch. + */ +export function verifyPublishedAppReplayKey( + presented: string | null | undefined, + input: PublishedAppReplayKeyInput, +): boolean { + if (typeof presented !== 'string' || presented.length === 0) return false; + let expected: string; + try { + expected = derivePublishedAppReplayKey(input); + } catch { + return false; + } + return secureCompare(presented, expected); +} diff --git a/packages/lib/src/services/app-hosting/parked-page.ts b/packages/lib/src/services/app-hosting/parked-page.ts new file mode 100644 index 0000000000..a63b7c5f34 --- /dev/null +++ b/packages/lib/src/services/app-hosting/parked-page.ts @@ -0,0 +1,127 @@ +/** + * parked-page — what the edge SERVES when it refuses to wake an app (pure). + * + * The parked page is the visible half of the enforcement decision, and it is + * rendered HERE, at the router, from a self-contained string: no request to the + * app, no request to the web app's renderer, no asset fetch. That is the point — + * every one of those would either start the machine we are refusing to start, or + * add a dependency to the one response that has to work when things are broken. + * + * No CSS file, no image, no script: a single inline-styled document, so the page + * is exactly as reliable as the router itself. + */ + +import type { AppRouteDecision } from './router-core'; + +/** + * HTTP status per outcome. + * + * `parked` answers **402 Payment Required** — the one status that actually says + * what happened. A 503 would be a lie the retry machinery believes: crawlers and + * uptime monitors treat 503 as transient and come back, and each of those + * requests would re-run the balance check for an account that is out of credits. + * 402 is terminal-until-you-act, which is the truth, and it makes enforcement + * countable in edge logs rather than blended into every other outage. + * + * `unavailable` IS transient (a deploy in flight, a failed provision awaiting the + * reconciler), so it takes 503 plus a `Retry-After` — see {@link retryAfterFor}. + */ +export function statusCodeFor(decision: AppRouteDecision): number { + switch (decision.kind) { + case 'replay': + return 204; + case 'parked': + return 402; + case 'unavailable': + return 503; + case 'not_found': + return 404; + } +} + +/** Seconds for `Retry-After`, or null when the outcome is not "come back later". */ +export function retryAfterFor(decision: AppRouteDecision): number | null { + if (decision.kind !== 'unavailable') return null; + // A deploy is seconds-to-a-minute; a failed or destroying app is minutes at + // best and is waiting on a human or a reconciler, so back the caller further + // off rather than letting a monitor hammer a state no retry can change. + return decision.reason === 'deploying' ? 15 : 120; +} + +/** Escape text for interpolation into the HTML body. */ +function escapeHtml(value: string): string { + return value + .replace(/&/g, '&') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + +interface PageCopy { + title: string; + heading: string; + body: string; +} + +function copyFor(decision: AppRouteDecision): PageCopy { + switch (decision.kind) { + case 'parked': + return { + title: 'App paused', + heading: 'This app is paused', + body: + 'It ran out of credits, so it has been stopped rather than left running. ' + + 'The owner can bring it back by topping up their PageSpace account — nothing has been lost.', + }; + case 'unavailable': + return decision.reason === 'deploying' + ? { + title: 'App starting', + heading: 'This app is starting up', + body: 'A new version is being deployed. Refresh in a few seconds.', + } + : { + title: 'App unavailable', + heading: 'This app is unavailable', + // Says nothing about what the owner can see. The previous copy told + // the visitor "its owner has been able to see why", which is not + // true: two of the four things that produce this page are route-level + // outages (the database is unreachable, the replay header came out + // invalid) that are logged server-side and surfaced to nobody. There + // is no owner-facing view of the reason. Telling a stranger that + // somebody else already has an explanation is both wrong and useless + // to the person reading it, so this points the one reader who can act + // at the place where they can. + body: 'It is not currently able to serve requests. If this is your app, check its status in PageSpace.', + }; + case 'not_found': + return { + title: 'No app here', + heading: 'There is no app at this address', + body: 'The address may be misspelled, or the app may have been unpublished.', + }; + case 'replay': + // Not rendered — a replay produces a bodiless response. Present so the + // switch stays exhaustive under a future decision kind. + return { title: 'Routing', heading: 'Routing', body: '' }; + } +} + +/** + * Render the router's own response body. + * + * `host` is echoed so an operator reading a screenshot knows which hostname + * produced it; it is escaped because it comes from a request header. + */ +export function renderAppRouterPage(decision: AppRouteDecision, host: string): string { + const { title, heading, body } = copyFor(decision); + const safeHost = escapeHtml(host); + return `<!doctype html><html lang="en"><head><meta charset="utf-8"><meta name="viewport" content="width=device-width,initial-scale=1"><title>${escapeHtml( + title, + )}

${escapeHtml( + heading, + )}

${escapeHtml( + body, + )}

${safeHost}

`; +} diff --git a/packages/lib/src/services/app-hosting/router-core.ts b/packages/lib/src/services/app-hosting/router-core.ts new file mode 100644 index 0000000000..b7e9b03009 --- /dev/null +++ b/packages/lib/src/services/app-hosting/router-core.ts @@ -0,0 +1,309 @@ +/** + * router-core — the PURE serving-edge decision: hostname in, route out. + * + * The whole enforcement property of the metered tier lives in one function here + * ({@link decideAppRoute}) and is therefore testable without a database, a clock, + * Fly, or a network: **an app whose payer is out of credits is not replayed to, + * so its machine is never started, so it never bills.** Enforcement is + * "don't wake", not clawback — there is no credit to claw back from an account + * that has none, which is precisely why the check has to happen BEFORE the wake + * rather than after it. + * + * That is also why the metered tier runs with NO `fly-replay-cache`. The cache + * exists to skip the router hop on subsequent requests — which means skipping + * this decision, which means skipping the balance check. A cached replay would + * keep a machine awake and billing for a payer we would refuse today. The + * dedicated (flat-rate) tier is the only legitimate cache user, because it has + * no balance gate to bypass; wiring that is a later change and is deliberately + * not smuggled in here. See {@link replayCachePolicyFor}. + */ + +/** + * The `timeout=` on every emitted replay, in milliseconds. + * + * Fly's proxy auto-starts a stopped target, and the default wait for that start + * is long enough (~7.5s) that a cold published app reads as a hung page rather + * than a slow one. 1500ms collapses that stall: past it the proxy gives up and + * the caller sees a fast error it can retry, which for a scale-to-zero app is + * strictly better than a browser spinner — the second request lands on a machine + * the first one already started. + */ +export const FLY_REPLAY_TIMEOUT_MS = 1500; + +/** + * Fly will not replay a request whose body exceeds 1MB. + * + * This is a hard platform limit and it shapes the product, not just this file: + * an upload path routed through the replay edge breaks at 1MB with a + * platform-level error we cannot improve on. Upload paths therefore go + * DIRECT TO TIGRIS via presigned URLs, never through the router. No upload + * plumbing is built here — this constant and {@link exceedsReplayableBody} exist + * so the constraint is enforced and legible at the edge (a clear 413 naming the + * limit) instead of surfacing as an opaque 502 from Fly. + */ +export const MAX_REPLAYABLE_BODY_BYTES = 1_048_576; +// 1,048,576 — one MEBIbyte. Any proxy mirroring this cap must say `1MiB`, never +// `1MB`: Caddy (and most size parsers) read `MB` as 1,000,000, which would refuse +// every body between the two figures while this route's own 413 page names a +// limit that allows them. See `fly/Caddyfile.fly` in PageSpace-Deploy. + +/** A published app, reduced to exactly what the routing decision reads. */ +export interface RoutableApp { + /** `published_apps.flyAppName` — the replay target. */ + flyAppName: string; + /** `published_apps.status`. */ + status: string; + /** `published_apps.tier` — 'metered' is gated, 'dedicated' is not. */ + tier: string; + /** + * Whether the row has a `machineId`. + * + * A precondition for replaying, not a detail: `fly-replay` targets an APP, and + * Fly's proxy auto-starts a STOPPED machine — it does not create one. An app + * with no machine yet (mid-first-deploy) has nothing to auto-start, so a replay + * to it fails at the platform with no useful message. Answering "deploying" + * ourselves is both honest and diagnosable. + */ + hasMachine: boolean; +} + +/** + * What the edge should do with this request. + * + * `parked` is deliberately its own outcome rather than a flavour of + * `unavailable`: it is the ENFORCEMENT state, it is the one outcome that must + * never start a machine, and it is the number worth watching in metrics. + */ +export type AppRouteDecision = + | { kind: 'replay'; flyAppName: string; state: string; timeoutMs: number } + | { kind: 'parked'; reason: 'out_of_credits' | 'parked_status' } + | { kind: 'unavailable'; reason: 'deploying' | 'failed' | 'destroying' | 'hosting_disabled' } + | { kind: 'not_found'; reason: 'unknown_host' | 'apex' | 'custom_host' | 'no_such_app' }; + +/** A hostname resolved against the published-apps apex. */ +export type AppHost = + | { kind: 'subdomain'; subdomain: string } + | { kind: 'apex' } + /** A hostname that is not under the apex at all — a custom domain, or noise. */ + | { kind: 'foreign'; hostname: string }; + +/** One DNS label: alphanumeric, inner hyphens allowed, 1..63 chars. */ +const LABEL_PATTERN = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/; +const MAX_LABEL_LENGTH = 63; + +/** + * Normalize a `Host` header value: lowercase, strip a trailing dot, strip the + * port — including from a bracketed IPv6 literal, where a naive "cut at the last + * colon" would amputate the address instead. + */ +export function normalizeRequestHost(rawHost: string): string { + let host = rawHost.trim().toLowerCase(); + if (host.startsWith('[')) { + const close = host.indexOf(']'); + if (close !== -1) host = host.slice(0, close + 1); + } else { + const colon = host.lastIndexOf(':'); + if (colon !== -1) host = host.slice(0, colon); + } + if (host.endsWith('.')) host = host.slice(0, -1); + return host; +} + +/** + * Resolve a request hostname against the published-apps apex. + * + * Only a SINGLE label under the apex is a published app: `acme.pagespace.app` + * yes, `a.b.pagespace.app` no. That is not fussiness — the wildcard cert covers + * one level, so a deeper name is not TLS-terminated for us anyway, and admitting + * it would let `evil.acme.pagespace.app` present as the app `evil` while looking + * to a reader like a child of `acme`. + */ +export function parseAppHost(rawHost: string, apex: string): AppHost { + const host = normalizeRequestHost(rawHost); + const normalizedApex = apex.trim().toLowerCase(); + if (host.length === 0 || normalizedApex.length === 0) { + return { kind: 'foreign', hostname: host }; + } + if (host === normalizedApex) return { kind: 'apex' }; + const suffix = `.${normalizedApex}`; + if (!host.endsWith(suffix)) return { kind: 'foreign', hostname: host }; + + const label = host.slice(0, -suffix.length); + if (label.length === 0 || label.length > MAX_LABEL_LENGTH) { + return { kind: 'foreign', hostname: host }; + } + if (label.includes('.') || !LABEL_PATTERN.test(label)) { + return { kind: 'foreign', hostname: host }; + } + return { kind: 'subdomain', subdomain: label }; +} + +/** Statuses whose app has something live to serve. */ +const SERVABLE_STATUSES = new Set(['running', 'stopped', 'deploying']); + +export interface AppRouteInput { + /** The row the hostname resolved to, or null when nothing did. */ + app: RoutableApp | null; + /** + * Whether the app's PAYER can still spend — the balance-check-before-wake. + * Read only for a 'metered' app; a 'dedicated' app is billed flat and skips + * the gate by definition (`published_apps_parked_is_metered_only` enforces the + * same rule in the database). + */ + balanceOk: boolean; + /** This app's derived fly-replay `state` key. */ + replayState: string; +} + +/** + * The routing decision. + * + * ORDER IS LOAD-BEARING. The persisted `parked` status is checked BEFORE the + * live balance read: parking is an enforcement action the metering cron took, + * and a payer who has since topped up gets un-parked by that cron (which can + * also restart the machine), not by a router that silently forgives the state on + * the next request. + * + * That cron is NOT in this branch — it arrives with the awake-seconds metering + * work (PR #2493), which is why grepping for it here finds nothing. The ordering + * is built now because it is the router's half of the contract and retrofitting + * it later would mean revisiting every decision below; both halves ship dark + * behind `APP_HOSTING_ENABLED`, so neither is load-bearing until they meet. The router NEVER writes — a status write on a + * per-request path would put a database mutation in front of every asset a + * published page loads. + * + * The converse also matters: a `running` app whose payer has run out is refused + * here even though its row still says `running`. The row lags by up to one cron + * tick; the balance does not. + */ +export function decideAppRoute(input: AppRouteInput): AppRouteDecision { + const { app } = input; + if (!app) return { kind: 'not_found', reason: 'no_such_app' }; + + if (app.status === 'parked') return { kind: 'parked', reason: 'parked_status' }; + if (app.status === 'destroying') return { kind: 'unavailable', reason: 'destroying' }; + if (app.status === 'failed') return { kind: 'unavailable', reason: 'failed' }; + + // provisioning / building, and any status this file has not been taught, are + // "not serving yet". Defaulting an UNKNOWN status to unavailable rather than + // to replay is the fail-closed direction: a status added later must not start + // billing machines through a router that has never heard of it. + if (!SERVABLE_STATUSES.has(app.status)) return { kind: 'unavailable', reason: 'deploying' }; + if (!app.hasMachine) return { kind: 'unavailable', reason: 'deploying' }; + + if (app.tier === 'metered' && !input.balanceOk) { + return { kind: 'parked', reason: 'out_of_credits' }; + } + + return { + kind: 'replay', + flyAppName: app.flyAppName, + state: input.replayState, + timeoutMs: FLY_REPLAY_TIMEOUT_MS, + }; +} + +/** + * Render a `fly-replay` header value. + * + * Throws on a target or state carrying the header's own `;`/`=` grammar rather + * than emitting it: a value that can inject a second directive can redirect the + * replay to another app. Both inputs are server-derived today (a `pgs-app-` + * name and a hex digest), which is exactly the condition under which such a + * check is cheap and stays true. + */ +export function buildFlyReplayHeader(args: { + flyAppName: string; + state: string; + timeoutMs: number; +}): string { + for (const [field, value] of [['app', args.flyAppName], ['state', args.state]] as const) { + if (value.length === 0) throw new Error(`buildFlyReplayHeader requires a non-empty ${field}`); + if (/[;=,\s]/.test(value)) { + throw new Error(`buildFlyReplayHeader received a ${field} containing header-grammar characters`); + } + } + if (!Number.isInteger(args.timeoutMs) || args.timeoutMs <= 0) { + throw new Error('buildFlyReplayHeader requires a positive integer timeoutMs'); + } + return `app=${args.flyAppName};state=${args.state};timeout=${args.timeoutMs}`; +} + +/** + * Whether a request's DECLARED body size is past what Fly can replay. + * + * Reads `Content-Length` only, and is therefore only half the check: a request + * that sends no length makes this answer false. {@link exceedsStreamedBody} + * covers that case — see the note there for why the split is deliberate rather + * than an oversight. See {@link MAX_REPLAYABLE_BODY_BYTES} for where large + * payloads are supposed to go instead. + */ +export function exceedsReplayableBody(contentLengthHeader: string | null | undefined): boolean { + if (!contentLengthHeader) return false; + const bytes = Number(contentLengthHeader); + if (!Number.isFinite(bytes) || bytes < 0) return false; + return bytes > MAX_REPLAYABLE_BODY_BYTES; +} + +/** + * Whether a body with no declared length runs past what Fly can replay. + * + * A request without `Content-Length` gives the header check above nothing to + * read, so it would let the request through to `fly-replay` — where Fly, unable + * to replay a body over the limit, fails it at the platform. The client gets an + * opaque 502 instead of the 413 this edge exists to give them, and it happens on + * the one path nobody tests. + * + * Deliberately NOT called "chunked": that names an HTTP/1.1 transfer-encoding, + * and HTTP/2 forbids it outright, carrying request content in DATA frames with + * no length at all. Keying on the ABSENCE of `Content-Length` covers both, which + * matters because the edge in front of this serves HTTP/2. + * + * So the body is measured, but ONLY when there is no length to read, and only up + * to the limit: the read stops and the stream is cancelled at the first byte past + * it. That keeps the original design property — a request that declares its size + * pays nothing, which is nearly all of them — while closing the case that + * declares nothing. Cancelling loses no replayable data: the only bodies + * cancelled are ones already too large for Fly to replay. + * + * Returns false for a bodyless request (GET, HEAD, a POST with no body), which + * is the same answer measuring an empty stream would give, without the read. + */ +export async function exceedsStreamedBody( + body: ReadableStream | null | undefined, + limit: number = MAX_REPLAYABLE_BODY_BYTES, +): Promise { + if (!body) return false; + + const reader = body.getReader(); + let seen = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) return false; + seen += value?.byteLength ?? 0; + if (seen > limit) return true; + } + } finally { + // Signals the producer to stop sending. This matters on the `return true` + // path, which leaves the stream mid-flight by design: without it the sender + // keeps streaming a body we have already decided to refuse. On the `done` + // path the stream is already closed and this is a no-op. It does NOT release + // the reader's lock, and does not need to — nothing else reads this body. + // Swallowing the rejection is deliberate: we are refusing the request either + // way, and a failure to cancel must not become the error the caller sees. + await reader.cancel().catch(() => {}); + } +} + +/** + * Whether this app may use `fly-replay-cache`. + * + * Never for a metered app: the cache skips the router hop, and the router hop IS + * the balance gate (see the file header). Stated as a function so the rule is + * one call away wherever the cache is eventually wired for the dedicated tier, + * rather than a comment somebody has to remember to read. + */ +export function replayCachePolicyFor(tier: string): 'no-cache' | 'cacheable' { + return tier === 'dedicated' ? 'cacheable' : 'no-cache'; +} diff --git a/packages/lib/src/services/app-hosting/router.ts b/packages/lib/src/services/app-hosting/router.ts new file mode 100644 index 0000000000..e9d604803a --- /dev/null +++ b/packages/lib/src/services/app-hosting/router.ts @@ -0,0 +1,193 @@ +/** + * router — the imperative shell of the published-app serving edge. + * + * Hostname in, {@link AppRouteDecision} out. Everything that touches the world — + * the `published_apps` read, resolving who pays, the balance read, deriving the + * replay key — happens here; every rule about what those facts MEAN lives in the + * pure `router-core.ts` next door. + * + * WHERE THIS RUNS, and why it is an app endpoint rather than proxy config: the + * decision needs a database row and a credit balance, and the edge is Caddy. So + * the proxy forwards published-app requests to the web app's router route, which + * calls this, and answers either with a `fly-replay` header (Fly then replays the + * ORIGINAL request to the target app — the proxy's rewrite to the router path is + * not what gets replayed) or with the parked/unavailable page itself. + * + * THE COST OF THAT SHAPE IS REAL AND DELIBERATE: with no replay cache on the + * metered tier, EVERY request to a published app — every asset, not just the + * document — pays one hop to the web app plus, for a servable metered app, three + * single-row indexed reads: the `published_apps` lookup, the payer's tier, and + * the payer's funded balance. No aggregates: `hasSpendableBalance` deliberately + * does NOT go through `getCreditBalance`, which would add a `SUM` over active + * `credit_holds` whose result the gate discards. A refusal costs fewer — an app + * refused on status alone never reaches the ledger at all. + * + * That is the price of the enforcement property (see `router-core.ts`), and it is + * bounded by the fact that the replayed response never returns through us: only + * the decision is ours, the bytes are not. + * + * ⚠️ A REPLAYED RESPONSE BYPASSES THE EDGE ENTIRELY. Fly's proxy hands the + * request straight to the target app and returns its response to the client, so + * NONE of the Caddyfile's header stanzas apply to a published app's own output. + * A published app owns its security headers. Nothing at this layer can add, + * strip, or sanitize them — do not add a header here expecting it to reach a + * served app's pages, because it never will. + */ + +import { db } from '@pagespace/db/db'; +import { eq } from '@pagespace/db/operators'; +import { publishedApps } from '@pagespace/db/schema/published-apps'; +import { hasSpendableBalance } from '../../billing/credit-gate'; +import { resolveTier } from '../../billing/credit-balance'; +import { isAppHostingEnabled } from './app-hosting-env'; +import { derivePublishedAppReplayKey } from './app-replay-key'; +import { resolveAppReplaySecret, resolvePublishedAppsApex } from './routing-env'; +import { + decideAppRoute, + parseAppHost, + type AppRouteDecision, + type RoutableApp, +} from './router-core'; + +/** What the router needs from the world, injected so the shell is testable. */ +export interface AppRouterDeps { + isEnabled: () => boolean; + /** The published-apps apex the hostname is resolved against. */ + apex: () => string; + /** Server secret the per-app replay `state` key is derived from. */ + replaySecret: () => string; + /** `published_apps` row for a subdomain, or null. */ + findAppBySubdomain: (subdomain: string) => Promise; + /** The payer's subscription tier — the allowance the balance is judged against. */ + resolveTier: (userId: string) => Promise; + /** Whether the payer can still spend. */ + hasSpendableBalance: (userId: string, tier: string) => Promise; +} + +/** The columns the routing decision reads. Narrower than the row on purpose. */ +export interface PublishedAppRouteRow { + id: string; + flyAppName: string; + status: string; + tier: string; + machineId: string | null; + /** + * Who pays — `published_apps.ownerId`, denormalized at publish time to the + * drive owner (`resolveEnvPayerId` semantics). + * + * Read from the row rather than re-resolved through the env and drive on every + * request, and that is a correctness point as much as a performance one: the + * balance gate must ask about the SAME payer the awake-seconds meter charges, + * and the meter charges this column. Re-deriving the payer here could disagree + * with it mid-flight (a drive ownership transfer between the two reads) and + * park an app whose actual payer is solvent. + */ + ownerId: string; +} + +async function findAppBySubdomainRow(subdomain: string): Promise { + const [row] = await db + .select({ + id: publishedApps.id, + flyAppName: publishedApps.flyAppName, + status: publishedApps.status, + tier: publishedApps.tier, + machineId: publishedApps.machineId, + ownerId: publishedApps.ownerId, + }) + .from(publishedApps) + .where(eq(publishedApps.subdomain, subdomain)) + .limit(1); + return row ?? null; +} + +export const defaultAppRouterDeps: AppRouterDeps = { + isEnabled: isAppHostingEnabled, + apex: resolvePublishedAppsApex, + replaySecret: resolveAppReplaySecret, + findAppBySubdomain: findAppBySubdomainRow, + resolveTier: (userId) => resolveTier(userId), + hasSpendableBalance: (userId, tier) => + hasSpendableBalance(userId, tier as Parameters[1]), +}; + +/** + * Resolve one request hostname to a routing decision. + * + * Never throws for an ordinary miss — an unknown host, a disabled feature, a + * hostname that is not ours — because each of those is a normal answer at a + * serving edge, not an exception. A genuine failure (the database is down) + * still propagates: the caller turns it into a 503, which is honest, whereas + * swallowing it here would present an outage as "no such app". + */ +export async function resolveAppRoute( + rawHost: string, + deps: AppRouterDeps = defaultAppRouterDeps, +): Promise { + // The kill switch is checked FIRST, before anything reads the database. While + // hosting is dark, this endpoint must be inert rather than merely fruitless. + if (!deps.isEnabled()) return { kind: 'unavailable', reason: 'hosting_disabled' }; + + const host = parseAppHost(rawHost, deps.apex()); + if (host.kind === 'apex') return { kind: 'not_found', reason: 'apex' }; + if (host.kind === 'foreign') { + // A custom domain reaches the edge as a hostname that is not under our apex. + // Binding one to a published app needs a pointer that does not exist yet: + // `custom_domains` carries `driveId` and resolves to a drive's STATIC + // published site, with no column naming a `published_apps` row. Answering + // `not_found` here (rather than guessing a drive's app) is what keeps the + // existing custom-domain behaviour intact — those hosts are served by the + // proxy's own custom-domain block and never reach this route. + return { kind: 'not_found', reason: 'custom_host' }; + } + + const app = await deps.findAppBySubdomain(host.subdomain); + if (!app) return { kind: 'not_found', reason: 'no_such_app' }; + + const routable: RoutableApp = { + flyAppName: app.flyAppName, + status: app.status, + tier: app.tier, + hasMachine: app.machineId !== null, + }; + + // Decide as far as the ROW alone allows, with the balance optimistically OK. + // Anything already refused at this point — parked, destroying, failed, an + // unknown status, no machine yet — never reaches the ledger. That is not only + // an optimization: a parked app is exactly the one that keeps receiving + // crawler and monitor traffic, and charging each of those requests a balance + // read would make the cheapest possible answer the most expensive one. + const preliminary = decideAppRoute({ app: routable, balanceOk: true, replayState: 'pending' }); + if (preliminary.kind !== 'replay') return preliminary; + + // Only a servable METERED app is worth asking the ledger about; a dedicated + // app is billed flat and has no gate. The refusal returns from inside the + // branch, so past this block the payer is known to be able to spend — which is + // what lets the final decision below pass `balanceOk: true` as a fact rather + // than a hope. `decideAppRoute` still re-checks the tier itself, so the skip + // here can never quietly become the policy. + if (app.tier === 'metered') { + const tier = await deps.resolveTier(app.ownerId); + const balanceOk = await deps.hasSpendableBalance(app.ownerId, tier); + if (!balanceOk) { + return decideAppRoute({ app: routable, balanceOk: false, replayState: 'pending' }); + } + } + + // Derive the state key only once the route is otherwise decided-servable: an + // unset or too-short `APP_REPLAY_SECRET` throws, and a router that cannot + // authenticate its replays must refuse to emit them rather than send traffic + // to a published app with a blank state it has no way to distinguish from a + // direct 6PN caller. + let replayState: string; + try { + replayState = derivePublishedAppReplayKey({ + flyAppName: app.flyAppName, + secret: deps.replaySecret(), + }); + } catch { + return { kind: 'unavailable', reason: 'failed' }; + } + + return decideAppRoute({ app: routable, balanceOk: true, replayState }); +} diff --git a/packages/lib/src/services/app-hosting/routing-env.ts b/packages/lib/src/services/app-hosting/routing-env.ts new file mode 100644 index 0000000000..ddea0481ee --- /dev/null +++ b/packages/lib/src/services/app-hosting/routing-env.ts @@ -0,0 +1,169 @@ +/** + * routing-env — the configuration surface of the published-app SERVING EDGE. + * + * Reads `process.env` DIRECTLY, for the same reason `app-hosting-env.ts` does: + * these values are resolved from more than one service, and `getValidatedEnv()` + * THROWS in a service with a lean env (realtime, processor) — which would blank + * the apex and the router secret even when both are correctly configured. See + * that file's header; this module is its routing-tier sibling and deliberately + * copies its shape. + * + * ──────────────────────────────────────────────────────────────────────────── + * THE ONE INVARIANT THIS MODULE EXISTS TO NAME + * + * The ROUTER APP and every PUBLISHED APP must live on the SAME Fly 6PN network. + * + * fly-replay cannot cross networks — the proxy answers + * `502 cross-network replays are not allowed`, which is exactly what the Phase 0 + * spike found and why `resolvePublishedAppsNetwork()` exists as one shared + * constant rather than a per-app value. That constraint binds the router too: + * the app that EMITS the `fly-replay` header is `resolveAppRouterFlyAppName()`, + * and if it was created on a different network than `resolvePublishedAppsNetwork()` + * every replay 502s. + * + * A Fly app's network is FIXED AT CREATE TIME, so this cannot be repaired by + * redeploying. It is satisfied one of two ways, both of them pure configuration: + * (a) create published apps on the existing router's network + * (`PUBLISHED_APPS_NETWORK=`), or + * (b) point `APP_ROUTER_FLY_APP_NAME` at a router app that was itself created + * on `PUBLISHED_APPS_NETWORK`. + * + * Nothing in this repo can verify which of those is true — the network an app + * was created on is a Fly-side fact. {@link describeRouterNetworkInvariant} is + * therefore a documentation/diagnostics helper, not a check: it renders the pair + * that has to agree so a 502 is one log line away from its cause instead of a + * day of bisecting. + * ──────────────────────────────────────────────────────────────────────────── + */ + +import { resolvePublishedAppsNetwork } from './app-hosting-env'; + +/** + * The apex published apps are served from: an app answers at + * `.`. + * + * A SEPARATE apex from `*.pagespace.site` (where `drives.publishSubdomain` static + * canvas sites live), and that separation is a security requirement rather than + * tidiness. `pagespace.site` is not on the Public Suffix List, so a document + * served from `a.pagespace.site` can set a `domain=.pagespace.site` cookie that + * every other published site then sends. Static canvas pages already carry that + * risk; a published app is strictly worse, because it runs arbitrary + * customer-authored SERVER code on its own origin — it can set, read and act on + * those cookies without a user ever visiting the victim site. + * + * So published apps get their own apex, and that apex MUST be on the PSL before + * GA. This constant is the wiring for that decision; submitting the apex to the + * PSL is an out-of-band action that is NOT performed by this repo. See + * `ROUTING.md` in this directory for the submission checklist. + */ +export const PUBLISHED_APPS_APEX_DEFAULT = 'pagespace.app'; + +/** + * The apex, normalized: lowercased, trailing dot and any leading `*.`/`.` + * stripped, so `PUBLISHED_APPS_APEX=*.pagespace.app` and `pagespace.app.` both + * resolve to the same value. An empty or whitespace-only override falls back to + * the default rather than yielding `''` — an empty apex would make + * {@link parseAppHost} treat EVERY hostname as a published-app subdomain. + */ +export function resolvePublishedAppsApex(): string { + const configured = (process.env.PUBLISHED_APPS_APEX ?? '').trim().toLowerCase(); + const normalized = configured.replace(/^\*?\./, '').replace(/\.$/, ''); + return normalized.length > 0 ? normalized : PUBLISHED_APPS_APEX_DEFAULT; +} + +/** The Fly app that terminates the published-apps apex and emits fly-replay. */ +export const APP_ROUTER_FLY_APP_DEFAULT = 'pagespace-proxy'; + +/** + * Name of the Fly app that emits the replays and holds the custom-domain certs. + * + * Falls back through `FLY_PROXY_APP_NAME` — the variable `reconcile-cert.ts` + * already uses to name the app certs attach to — so a deployment that has only + * ever configured the one proxy keeps working, and a deployment that splits the + * router onto its own app (option (b) in the file header) sets exactly one new + * variable. The two must name the SAME app: a cert issued on app A does not + * TLS-terminate traffic arriving at app B. + */ +export function resolveAppRouterFlyAppName(): string { + const explicit = (process.env.APP_ROUTER_FLY_APP_NAME ?? '').trim(); + if (explicit.length > 0) return explicit; + const legacy = (process.env.FLY_PROXY_APP_NAME ?? '').trim(); + return legacy.length > 0 ? legacy : APP_ROUTER_FLY_APP_DEFAULT; +} + +/** + * Server-held secret the per-app fly-replay `state` key is derived from. + * + * Returns '' when unset, so the router fails CLOSED: `derivePublishedAppReplayKey` + * throws below its length floor, the route answers "unavailable", and no traffic + * is replayed WITHOUT a state key. Failing open here would hand every published + * app unauthenticated traffic it cannot distinguish from router-issued traffic — + * the exact property the key exists to provide. + */ +export function resolveAppReplaySecret(): string { + return process.env.APP_REPLAY_SECRET ?? ''; +} + +/** + * The length floor both router secrets have to clear. + * + * Matches the floor `derivePublishedAppReplayKey` already enforces on + * `APP_REPLAY_SECRET`, and the one `CSRF_SECRET` and `ENCRYPTION_KEY` are held to + * in `env-validation.ts`. Stated once so the two secrets cannot drift apart: + * both authenticate a caller by equality against a value nobody may guess, and + * that property is a function of length. + */ +export const MIN_ROUTER_SECRET_LENGTH = 32; + +/** + * Shared secret proving a router request actually came from the edge proxy. + * + * The router endpoint lives on `pagespace-web`, which is also reachable at + * `pagespace.ai/api/...`. Without this, ANY internet caller could hand the app + * an arbitrary published-app hostname and receive a `fly-replay` header — i.e. + * turn our own web app into a general-purpose replay emitter for the org, and + * wake (and bill) any published app they can name. So the router answers only + * requests carrying this value in `X-PageSpace-App-Router-Key`, set by the proxy + * from its own Fly secret. + * + * Returns '' when unset, and the router treats '' as "refuse everything" rather + * than "no check" — an unconfigured secret must not silently disable the check + * that stops the endpoint being world-callable. + * + * A configured value SHORTER than {@link MIN_ROUTER_SECRET_LENGTH} resolves to '' + * too, and therefore also refuses everything. A one-character secret is not a + * weaker version of this protection, it is the absence of it: the header is + * guessable, and the endpoint becomes the world-callable replay emitter the + * check exists to prevent. This module deliberately reads `process.env` directly + * rather than going through `validateEnv`, so the floor has to be enforced here + * as well as in the schema — a process that skipped validation must not end up + * with a weaker router than one that did. + */ +export function resolveAppRouterProxySecret(): string { + const configured = process.env.APP_ROUTER_PROXY_SECRET ?? ''; + return configured.length >= MIN_ROUTER_SECRET_LENGTH ? configured : ''; +} + +/** The request header the edge proxy carries the real published-app hostname in. */ +export const APP_ROUTER_HOST_HEADER = 'x-pagespace-app-host'; + +/** The request header the edge proxy carries {@link resolveAppRouterProxySecret} in. */ +export const APP_ROUTER_KEY_HEADER = 'x-pagespace-app-router-key'; + +/** + * The router/published-app network pair that has to agree, rendered for logs and + * ops docs. NOT a check — see the file header for why one is not possible here. + */ +export function describeRouterNetworkInvariant(): { + routerApp: string; + publishedAppsNetwork: string; + note: string; +} { + return { + routerApp: resolveAppRouterFlyAppName(), + publishedAppsNetwork: resolvePublishedAppsNetwork(), + note: + 'fly-replay cannot cross Fly 6PN networks: the router app must have been CREATED on publishedAppsNetwork, ' + + 'or every replay answers 502 "cross-network replays are not allowed". A Fly app\'s network is fixed at create time.', + }; +} diff --git a/packages/lib/src/services/fly/__tests__/flaps-certificates.test.ts b/packages/lib/src/services/fly/__tests__/flaps-certificates.test.ts new file mode 100644 index 0000000000..67f2084fab --- /dev/null +++ b/packages/lib/src/services/fly/__tests__/flaps-certificates.test.ts @@ -0,0 +1,178 @@ +/** + * The Machines API certificates resource. + * + * These four helpers replace hand-written GraphQL mutations, and the reason the + * port is worth doing is asserted here rather than only asserted in prose: the + * REST responses carry `dns_requirements` and `validation`, which name the exact + * records a stuck hostname is waiting on — including the `_fly-ownership` TXT + * that GraphQL had no equivalent for. + * + * The other property under test is CONVERGENCE. Certificates bill per hostname, + * and both the request and the delete run from poll cycles that retry, so a + * hostname already present must resolve to its existing certificate rather than + * failing, and a hostname already absent must read as success. + */ +import { describe, expect, it, vi } from 'vitest'; +import { + FlapsError, + checkCertificate, + deleteCertificate, + getCertificate, + requestAcmeCertificate, + type FlapsTransport, +} from '../flaps-client'; + +const APP = 'pagespace-proxy'; +const HOST = 'docs.acme.com'; + +interface Reply { + status: number; + body?: unknown; +} + +/** A transport that replays queued responses and records every request. */ +function stubTransport(replies: Reply[]): { + transport: FlapsTransport; + calls: Array<{ method: string; url: string; body: unknown }>; +} { + const calls: Array<{ method: string; url: string; body: unknown }> = []; + const queue = [...replies]; + const fetchImpl = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const reply = queue.shift(); + if (!reply) throw new Error('stub transport received an unexpected extra request'); + calls.push({ + method: init?.method ?? 'GET', + url: String(input), + body: typeof init?.body === 'string' ? JSON.parse(init.body) : undefined, + }); + return new Response(reply.body === undefined ? null : JSON.stringify(reply.body), { + status: reply.status, + headers: { 'content-type': 'application/json' }, + }); + }); + return { + calls, + transport: { token: 'test-token', fetchImpl: fetchImpl as unknown as typeof fetch, sleep: async () => {} }, + }; +} + +const PENDING_CERT = { + hostname: HOST, + status: 'pending_validation', + configured: false, + dns_requirements: { + ownership: { + name: `_fly-ownership.${HOST}`, + app_value: 'app-ABC123', + org_value: 'org-XYZ789', + }, + }, + validation: { ownership_txt_configured: false }, +}; + +const ACTIVE_CERT = { hostname: HOST, status: 'active', configured: true }; + +describe('requestAcmeCertificate — the ACME request', () => { + it('given a new hostname, should POST it to the app certificates/acme endpoint', async () => { + const { transport, calls } = stubTransport([{ status: 200, body: PENDING_CERT }]); + const cert = await requestAcmeCertificate(transport, APP, HOST); + + expect(calls[0].method).toBe('POST'); + expect(calls[0].url).toContain(`/v1/apps/${APP}/certificates/acme`); + expect(calls[0].body).toEqual({ hostname: HOST }); + expect(cert.status).toBe('pending_validation'); + }); + + it('given a pending certificate, should carry the ownership record the customer still owes', async () => { + const { transport } = stubTransport([{ status: 200, body: PENDING_CERT }]); + const cert = await requestAcmeCertificate(transport, APP, HOST); + // This is the whole reason for the port: GraphQL returned no equivalent. + expect(cert.dns_requirements?.ownership?.app_value).toBe('app-ABC123'); + expect(cert.validation?.ownership_txt_configured).toBe(false); + }); + + it('given a hostname Fly already has, should resolve to the existing certificate rather than fail', async () => { + const { transport, calls } = stubTransport([ + { status: 422, body: { error: 'Hostname already exists on app' } }, + { status: 200, body: ACTIVE_CERT }, + ]); + const cert = await requestAcmeCertificate(transport, APP, HOST); + expect(cert.status).toBe('active'); + expect(calls[1].method).toBe('GET'); + }); + + it('given a hostname needing escaping, should encode it into the path', async () => { + const { transport, calls } = stubTransport([{ status: 200, body: ACTIVE_CERT }]); + await getCertificate(transport, 'app/with slash', 'a b.com'); + expect(calls[0].url).toContain('app%2Fwith%20slash'); + expect(calls[0].url).toContain('a%20b.com'); + }); + + it('given a genuine failure, should throw rather than report a certificate', async () => { + const { transport } = stubTransport([{ status: 403, body: { error: 'unauthorized' } }]); + await expect(requestAcmeCertificate(transport, APP, HOST)).rejects.toThrow(FlapsError); + }); + + it('given a 2xx whose body is not an object, should throw rather than return a non-certificate', async () => { + const { transport } = stubTransport([{ status: 200, body: ['not', 'a', 'cert'] }]); + await expect(requestAcmeCertificate(transport, APP, HOST)).rejects.toThrow(FlapsError); + }); +}); + +describe('getCertificate — a 404 is an answer, not an error', () => { + it('given a hostname the app does not have, should return null', async () => { + const { transport } = stubTransport([{ status: 404, body: { error: 'not found' } }]); + expect(await getCertificate(transport, APP, HOST)).toBeNull(); + }); + + it('given an existing certificate, should return it without requesting anything', async () => { + const { transport, calls } = stubTransport([{ status: 200, body: ACTIVE_CERT }]); + const cert = await getCertificate(transport, APP, HOST); + expect(cert?.status).toBe('active'); + expect(calls).toHaveLength(1); + expect(calls[0].method).toBe('GET'); + }); + + it('given a server error, should still throw', async () => { + const { transport } = stubTransport([ + { status: 500, body: { error: 'boom' } }, + { status: 500, body: { error: 'boom' } }, + { status: 500, body: { error: 'boom' } }, + { status: 500, body: { error: 'boom' } }, + ]); + await expect(getCertificate(transport, APP, HOST)).rejects.toThrow(FlapsError); + }); +}); + +describe('checkCertificate — asking Fly to re-read DNS', () => { + it('given a hostname, should POST to its check endpoint', async () => { + const { transport, calls } = stubTransport([{ status: 200, body: ACTIVE_CERT }]); + await checkCertificate(transport, APP, HOST); + expect(calls[0].method).toBe('POST'); + expect(calls[0].url).toContain(`/certificates/${HOST}/check`); + }); + + it('given a hostname Fly does not have, should return null', async () => { + const { transport } = stubTransport([{ status: 404, body: { error: 'not found' } }]); + expect(await checkCertificate(transport, APP, HOST)).toBeNull(); + }); +}); + +describe('deleteCertificate — idempotent, because certs bill per hostname', () => { + it('given an attached hostname, should DELETE it', async () => { + const { transport, calls } = stubTransport([{ status: 204 }]); + await expect(deleteCertificate(transport, APP, HOST)).resolves.toBeUndefined(); + expect(calls[0].method).toBe('DELETE'); + expect(calls[0].url).toContain(`/certificates/${HOST}`); + }); + + it('given a hostname already gone, should treat the 404 as the desired end state', async () => { + const { transport } = stubTransport([{ status: 404, body: { error: 'not found' } }]); + await expect(deleteCertificate(transport, APP, HOST)).resolves.toBeUndefined(); + }); + + it('given a refusal, should throw so a billing hostname is never assumed removed', async () => { + const { transport } = stubTransport([{ status: 403, body: { error: 'forbidden' } }]); + await expect(deleteCertificate(transport, APP, HOST)).rejects.toThrow(FlapsError); + }); +}); diff --git a/packages/lib/src/services/fly/flaps-client.ts b/packages/lib/src/services/fly/flaps-client.ts index 37cb71755f..7374a9f0e6 100644 --- a/packages/lib/src/services/fly/flaps-client.ts +++ b/packages/lib/src/services/fly/flaps-client.ts @@ -718,3 +718,181 @@ export async function updateMachineConfig( assertOk(status, body, path); return asMachine(body, status, path); } + +// ── TLS certificates ───────────────────────────────────────────────────────── +// +// Custom hostnames are attached to the ROUTER app (`resolveAppRouterFlyAppName`), +// never to an individual published app: the router is what Fly TLS-terminates, +// and the replay target has no public IP at all. +// +// These live on the Machines API (`api.machines.dev`) rather than Fly's GraphQL, +// which is what `apps/web/src/lib/fly/certs.ts` used to call. The REST resource is +// the reason that port is worth doing: GraphQL's `addCertificate` returns a bare +// `{configured, clientStatus}` and nothing about WHY a cert is stuck, while +// `dns_requirements` / `validation` here name the exact records the customer is +// missing — including the `_fly-ownership` TXT, which has no GraphQL equivalent +// and is the only validation path available to a domain behind a CDN. + +/** The DNS records Fly needs in place before it can issue for a hostname. */ +export interface FlyCertificateDnsRequirements { + a?: string[]; + aaaa?: string[]; + cname?: string; + acme_challenge?: { name?: string; target?: string }; + /** + * The `_fly-ownership` TXT record. Present when Fly cannot validate by + * reachability — a CDN-fronted host, an imported certificate, or an apex the + * customer will not point at us until the cert exists. + */ + ownership?: { name?: string; app_value?: string; org_value?: string }; + [key: string]: unknown; +} + +/** Which validation methods Fly has confirmed for a hostname. */ +export interface FlyCertificateValidation { + dns_configured?: boolean; + alpn_configured?: boolean; + http_configured?: boolean; + ownership_txt_configured?: boolean; + [key: string]: unknown; +} + +/** + * A hostname's certificate state. + * + * Named fields are the ones we read; the index signature preserves everything + * else, for the same reason `MachineConfig` does — Fly adds fields, and a type + * that dropped them would make every future response lossy at the boundary. + */ +export interface FlyCertificate { + hostname?: string; + /** `'pending_validation' | 'active'` are the documented values. */ + status?: string; + configured?: boolean; + acme_requested?: boolean; + dns_provider?: string; + rate_limited_until?: string | null; + validation?: FlyCertificateValidation; + dns_requirements?: FlyCertificateDnsRequirements; + validation_errors?: unknown[]; + [key: string]: unknown; +} + +/** A 2xx whose body is not an object is not a certificate. */ +function asCertificate(body: unknown, status: number, endpoint: string): FlyCertificate { + if (typeof body !== 'object' || body === null || Array.isArray(body)) { + throw new FlapsError(`Fly Machines API ${endpoint} returned no certificate`, status, endpoint); + } + return body as FlyCertificate; +} + +function certificatesPath(appName: string): string { + return `/v1/apps/${encodeURIComponent(appName)}/certificates`; +} + +function certificatePath(appName: string, hostname: string): string { + return `${certificatesPath(appName)}/${encodeURIComponent(hostname)}`; +} + +/** + * Request a Let's Encrypt certificate for `hostname` on `appName`. + * `POST /v1/apps/{app}/certificates/acme`, body `{hostname}`. + * + * IDEMPOTENT BY KEY (the hostname): a hostname already registered on the app + * resolves to that existing certificate rather than failing, so a re-provision, + * a poll cycle, or a retried ambiguous request all converge instead of + * alternating between "created" and "already exists". This mirrors `createApp`, + * and it is what makes retrying a lost response safe. + * + * Returns the certificate's CURRENT state, which for a fresh request is + * `status: 'pending_validation'` with `dns_requirements` filled in — that is the + * useful part, not the status: it names the records the customer still has to + * publish. + */ +export async function requestAcmeCertificate( + transport: FlapsTransport, + appName: string, + hostname: string, +): Promise { + const path = `${certificatesPath(appName)}/acme`; + const { status, body } = await flapsRequest(transport, 'POST', path, { + body: { hostname }, + }); + if (isAlreadyExists(status, body)) { + const existing = await getCertificate(transport, appName, hostname); + if (existing) return existing; + } + assertOk(status, body, path); + return asCertificate(body, status, path); +} + +/** + * Read a hostname's certificate. `GET /v1/apps/{app}/certificates/{hostname}`. + * + * Returns null on 404 — "this app has no certificate for that hostname" is an + * ANSWER on this endpoint, and the whole point of calling it before requesting + * one. Every other non-2xx still throws. + */ +export async function getCertificate( + transport: FlapsTransport, + appName: string, + hostname: string, +): Promise { + const path = certificatePath(appName, hostname); + const { status, body } = await flapsRequest(transport, 'GET', path); + if (status === 404) return null; + assertOk(status, body, path); + return asCertificate(body, status, path); +} + +/** + * Force Fly to re-run validation for a hostname. + * `POST /v1/apps/{app}/certificates/{hostname}/check`. + * + * A POST that mutates nothing we own — it makes Fly re-read DNS — so it is safe + * to retry. Its whole purpose is TIMING: a customer who has just published the + * record would otherwise wait out Fly's own polling cadence before anything + * changed, and this is what makes the settings UI's "Check SSL" actually check. + * `reconcile-cert.ts` calls it in exactly that window — our resolver can already + * see an accepted ownership value while Fly still reports the TXT unconfigured. + * + * It is NOT the source of "we see your TXT, but it says X". That message comes + * from resolving the record with our own resolver and reporting `mismatched` + * with the values found — see {@link verifyFlyOwnershipTxt} in + * `validators/fly-ownership.ts`. Nothing here reads the response's DNS detail, + * and a second source for that message would be exactly the drift the shared + * `acceptedOwnershipValues` exists to prevent. + * + * Returns null on 404, same reasoning as {@link getCertificate}. + */ +export async function checkCertificate( + transport: FlapsTransport, + appName: string, + hostname: string, +): Promise { + const path = `${certificatePath(appName, hostname)}/check`; + const { status, body } = await flapsRequest(transport, 'POST', path); + if (status === 404) return null; + assertOk(status, body, path); + return asCertificate(body, status, path); +} + +/** + * Remove a hostname and all its certificates from the app. + * `DELETE /v1/apps/{app}/certificates/{hostname}` (204 on success). + * + * IDEMPOTENT: a 404 is success — the desired end state is "this app does not + * serve that hostname", and it already holds. Certs bill per hostname, so the + * delete path must converge rather than strand a charge behind a retry that + * refuses to run twice. + */ +export async function deleteCertificate( + transport: FlapsTransport, + appName: string, + hostname: string, +): Promise { + const path = certificatePath(appName, hostname); + const { status, body } = await flapsRequest(transport, 'DELETE', path); + if (status === 404) return; + assertOk(status, body, path); +} diff --git a/packages/lib/src/validators/__tests__/fly-ownership.test.ts b/packages/lib/src/validators/__tests__/fly-ownership.test.ts new file mode 100644 index 0000000000..074cb8d123 --- /dev/null +++ b/packages/lib/src/validators/__tests__/fly-ownership.test.ts @@ -0,0 +1,294 @@ +/** + * `_fly-ownership` TXT pre-validation. + * + * The distinction this module exists to draw: through a certificate's status + * alone, "Fly has not issued yet" and "the customer was never told to publish a + * record" look identical, and they need opposite responses. So the states here + * are asserted as four DIFFERENT answers, and in particular `not_required` is + * never conflated with `satisfied` — the second claims we verified something. + */ +import { describe, expect, it } from 'vitest'; +import { + FLY_OWNERSHIP_TXT_PREFIX, + acceptedOwnershipValues, + describeOwnershipVerification, + flyOwnershipTxtName, + parseOwnershipTxtValues, + verifyFlyOwnershipTxt, + type FlyOwnershipRequirement, + type FlyOwnershipVerification, +} from '../fly-ownership'; +import { assert } from '../../__tests__/riteway'; + +const requirement: FlyOwnershipRequirement = { + name: '_fly-ownership.docs.acme.com', + appValue: 'app-ABC123', + orgValue: 'org-XYZ789', +}; + +/** Fly names an org value and no app value — a state `verifyFlyOwnershipTxt` accepts. */ +const orgOnlyRequirement: FlyOwnershipRequirement = { + name: '_fly-ownership.docs.acme.com', + appValue: '', + orgValue: 'org-XYZ789', +}; + +/** The mirror case: an app value and no org value. */ +const appOnlyRequirement: FlyOwnershipRequirement = { + name: '_fly-ownership.docs.acme.com', + appValue: 'app-ABC123', + orgValue: '', +}; + +const verify = (records: string[][], req: FlyOwnershipRequirement | null = requirement) => + verifyFlyOwnershipTxt({ requirement: req, records }); + +describe('flyOwnershipTxtName — where the record must live', () => { + assert({ + given: 'a hostname', + should: 'prefix it with the record label Fly reads', + actual: flyOwnershipTxtName('docs.acme.com'), + expected: `${FLY_OWNERSHIP_TXT_PREFIX}.docs.acme.com`, + }); + + assert({ + given: 'an uppercased hostname with a trailing dot and whitespace', + should: 'normalize before prefixing', + actual: flyOwnershipTxtName(' Docs.ACME.com. '), + expected: '_fly-ownership.docs.acme.com', + }); +}); + +describe('parseOwnershipTxtValues — the two shapes a real TXT answer arrives in', () => { + assert({ + given: 'a value chunked at the 255-byte TXT limit', + should: 'concatenate the chunks with no separator, or a long value never matches', + actual: parseOwnershipTxtValues([['app-', 'ABC', '123']]), + expected: ['app-ABC123'], + }); + + assert({ + given: 'several ownership values sharing one record, as Fly documents', + should: 'split on the semicolon, so a hostname serving two Fly apps still validates', + actual: parseOwnershipTxtValues([['app-ABC123;app-DEF456']]), + expected: ['app-ABC123', 'app-DEF456'], + }); + + assert({ + given: 'a value stored with zone-file quoting by a DNS UI', + should: 'strip the surrounding quotes', + actual: parseOwnershipTxtValues([['"app-ABC123"']]), + expected: ['app-ABC123'], + }); + + assert({ + given: 'multiple records at the name, one of them unrelated', + should: 'return every candidate rather than only the first record', + actual: parseOwnershipTxtValues([['v=spf1 -all'], ['app-ABC123']]), + expected: ['v=spf1 -all', 'app-ABC123'], + }); + + assert({ + given: 'empty and whitespace-only segments', + should: 'drop them', + actual: parseOwnershipTxtValues([['', ' '], [';;']]), + expected: [], + }); +}); + +describe('verifyFlyOwnershipTxt — four distinguishable answers', () => { + assert({ + given: 'Fly asked for no ownership record at all', + should: 'report not_required — NOT satisfied, which would claim a check we never ran', + actual: verify([['anything']], null), + expected: { state: 'not_required' }, + }); + + assert({ + given: 'nothing resolves at the record name', + should: 'report missing, with the record the customer still has to publish', + actual: verify([]), + expected: { state: 'missing', expected: requirement }, + }); + + assert({ + given: 'the app-scoped value is published', + should: 'be satisfied', + actual: verify([['app-ABC123']]), + expected: { state: 'satisfied' }, + }); + + assert({ + given: 'the ORG-scoped value is published instead', + should: 'also be satisfied, because Fly accepts either', + actual: verify([['org-XYZ789']]), + expected: { state: 'satisfied' }, + }); + + assert({ + given: 'a case-folded value, as several DNS providers store it', + should: 'still be satisfied', + actual: verify([['APP-abc123']]), + expected: { state: 'satisfied' }, + }); + + assert({ + given: 'a record carrying only some other value', + should: 'report mismatched, and say what was actually found', + actual: verify([['app-WRONG']]), + expected: { state: 'mismatched', expected: requirement, found: ['app-WRONG'] }, + }); + + assert({ + given: 'the right value alongside an unrelated TXT record at the same name', + should: 'be satisfied — coexisting records are normal', + actual: verify([['v=spf1 -all'], ['app-ABC123']]), + expected: { state: 'satisfied' }, + }); + + assert({ + given: 'Fly asked for ownership but named no value to publish', + should: 'report mismatched rather than satisfied — it is a state nobody can act on', + actual: verify([['something']], { name: '_fly-ownership.x.com', appValue: '', orgValue: '' }), + expected: { + state: 'mismatched', + expected: { name: '_fly-ownership.x.com', appValue: '', orgValue: '' }, + found: ['something'], + }, + }); +}); + +describe('describeOwnershipVerification — an instruction only when one is owed', () => { + it.each([ + ['not_required', { state: 'not_required' } as const], + ['satisfied', { state: 'satisfied' } as const], + ])('given %s, should offer no instruction', (_label, result) => { + expect(describeOwnershipVerification(result)).toBeNull(); + }); + + it('given a missing record, should name the record and the value to publish', () => { + const message = describeOwnershipVerification({ state: 'missing', expected: requirement }); + expect(message).toContain('_fly-ownership.docs.acme.com'); + expect(message).toContain('app-ABC123'); + }); + + it('given a mismatch, should name both what was expected and what was found', () => { + const message = describeOwnershipVerification({ + state: 'mismatched', + expected: requirement, + found: ['app-WRONG'], + }); + expect(message).toContain('app-ABC123'); + expect(message).toContain('app-WRONG'); + }); + + it('given a requirement naming both values, should name both, since either satisfies Fly', () => { + const message = describeOwnershipVerification({ state: 'missing', expected: requirement }); + expect(message).toContain('app-ABC123'); + expect(message).toContain('org-XYZ789'); + }); + + // "with the value A or B" reads as ONE value whose text is "A or B" — a string + // a customer can paste into a TXT record verbatim. The phrasing has to say + // these are alternatives, and only when there is more than one. + it('given two accepted values, should present them as alternatives, not as one value', () => { + const message = describeOwnershipVerification({ state: 'missing', expected: requirement }); + expect(message).toContain('either of these values: app-ABC123 or org-XYZ789'); + expect(message).not.toContain('the value app-ABC123 or'); + }); + + // The gap that let a regression through: the mismatched assertions only checked + // that the values APPEAR. "expected" already supplies the noun, so reusing the + // instruction's phrasing produced "expected the value org-X" and "expected + // either of these values: app-X or org-Y; found …" — a colon and a semicolon + // inside one parenthesis. Both sentences are now asserted as sentences. + it('given a mismatch, should list the values bare — "expected" already supplies the noun', () => { + const both = describeOwnershipVerification({ + state: 'mismatched', + expected: requirement, + found: ['app-WRONG'], + }); + expect(both).toContain('(expected app-ABC123 or org-XYZ789; found app-WRONG)'); + expect(both).not.toContain('expected either of these values'); + expect(both).not.toContain('expected the value'); + }); + + it('given an org-only mismatch, should not say "expected the value"', () => { + const one = describeOwnershipVerification({ + state: 'mismatched', + expected: orgOnlyRequirement, + found: ['app-WRONG'], + }); + expect(one).toContain('(expected org-XYZ789; found app-WRONG)'); + expect(one).not.toContain('expected the value'); + }); + + it('given one accepted value, should still say "the value", not offer a choice', () => { + const message = describeOwnershipVerification({ state: 'missing', expected: orgOnlyRequirement }); + expect(message).toContain('the value org-XYZ789'); + expect(message).not.toContain('either of these values'); + }); + + // The regression: `verifyFlyOwnershipTxt` accepts an org-only requirement (it + // filters empty values), but the instruction used to print `appValue` + // unconditionally — so this state produced a message with a blank where the + // value belongs, in the one place the customer has nothing else to act on. + // Typed rather than `as const`: the tuple type checks each row against the real + // union (which is what `as const` was reaching for) without freezing `found` + // into a readonly tuple that no longer matches the shipped `string[]`. + it.each<[string, FlyOwnershipVerification]>([ + ['missing', { state: 'missing', expected: orgOnlyRequirement }], + ['mismatched', { state: 'mismatched', expected: orgOnlyRequirement, found: ['app-WRONG'] }], + ])('given an org-only requirement reported as %s, should name the org value', (_label, result) => { + const message = describeOwnershipVerification(result); + expect(message).toContain('org-XYZ789'); + expect(message).not.toContain('the value '); + expect(message).not.toContain('(expected ;'); + }); + + it('given an app-only requirement, should name the app value and not trail an empty alternative', () => { + const message = describeOwnershipVerification({ + state: 'missing', + expected: appOnlyRequirement, + }); + expect(message).toContain('app-ABC123'); + expect(message).not.toContain(' or '); + }); + + // Unreachable through `ownershipRequirementOf`, which returns null when Fly + // names neither value — asserted anyway because the type permits it and the + // alternative is an instruction telling the customer to publish nothing. + it('given a requirement naming no value at all, should say the requirement is unusable', () => { + const message = describeOwnershipVerification({ + state: 'mismatched', + expected: { name: '_fly-ownership.docs.acme.com', appValue: '', orgValue: '' }, + found: ['app-WRONG'], + }); + expect(message).toContain('without naming a value'); + }); +}); + +describe('acceptedOwnershipValues — one definition of acceptable, shared', () => { + assert({ + given: 'a requirement naming both an app and an org value', + should: 'offer both, because Fly accepts either', + actual: acceptedOwnershipValues(requirement), + expected: ['app-ABC123', 'org-XYZ789'], + }); + + assert({ + given: 'a requirement naming only an org value', + should: 'drop the empty app value rather than offering a blank', + actual: acceptedOwnershipValues(orgOnlyRequirement), + expected: ['org-XYZ789'], + }); + + // The property the shared helper exists for: whatever verification is willing + // to match against is exactly what the instruction is willing to print. + it('given an org-only requirement, should agree with what verification accepts', () => { + expect(verify([['org-XYZ789']], orgOnlyRequirement)).toEqual({ state: 'satisfied' }); + expect(describeOwnershipVerification({ state: 'missing', expected: orgOnlyRequirement })).toContain( + acceptedOwnershipValues(orgOnlyRequirement)[0], + ); + }); +}); diff --git a/packages/lib/src/validators/fly-ownership.ts b/packages/lib/src/validators/fly-ownership.ts new file mode 100644 index 0000000000..d28e4f3b3c --- /dev/null +++ b/packages/lib/src/validators/fly-ownership.ts @@ -0,0 +1,193 @@ +/** + * `_fly-ownership` TXT pre-validation (pure). + * + * Fly issues a certificate for a hostname only once it can prove the requester + * controls it. The usual proof is reachability — the hostname's A/AAAA/CNAME + * already point at Fly — which is what `verifyDnsRecords` in `./custom-domain` + * checks and what the existing custom-domain flow requires before it will ask + * for a cert at all. + * + * That proof is unavailable for a hostname the customer will not (or cannot) + * point at us yet: a domain behind a CDN that terminates TLS itself, an apex + * being migrated with no downtime, an imported certificate. For those, Fly asks + * for a TXT record at `_fly-ownership.` carrying the app's or the + * org's ownership value, and reports it back as + * `dns_requirements.ownership` / `validation.ownership_txt_configured`. + * + * PRE-validation, not post-mortem: the point of checking the record ourselves, + * before treating a stuck certificate as failed, is that "Fly has not issued + * yet" and "the customer never published the record" are the same observable + * state through the certificate status alone — and they need opposite responses. + * The first is waiting; the second is an instruction the customer has not been + * given. Resolving the TXT here separates them. + * + * Pure: TXT records come in as data. The resolution itself is `apps/web/src/lib/ + * publish/dns-resolver.ts`, which already owns the authoritative-then-recursive + * strategy and its SSRF guard. + */ + +/** The subdomain Fly reads the ownership proof from. */ +export const FLY_OWNERSHIP_TXT_PREFIX = '_fly-ownership'; + +/** Where the TXT record for a hostname must live. */ +export function flyOwnershipTxtName(hostname: string): string { + return `${FLY_OWNERSHIP_TXT_PREFIX}.${hostname.trim().toLowerCase().replace(/\.$/, '')}`; +} + +/** What Fly said it wants, normalized out of a certificate response. */ +export interface FlyOwnershipRequirement { + /** The record name, e.g. `_fly-ownership.example.com`. */ + name: string; + /** The app-scoped value, e.g. `app-XXXXXXXXXX`. */ + appValue: string; + /** The org-scoped value, e.g. `org-XXXXXXXXXX`. Either value satisfies Fly. */ + orgValue: string; +} + +export type FlyOwnershipVerification = + /** Fly did not ask for an ownership TXT — nothing to pre-validate. */ + | { state: 'not_required' } + /** The record is published and carries a value Fly will accept. */ + | { state: 'satisfied' } + /** Nothing resolves at the record name. */ + | { state: 'missing'; expected: FlyOwnershipRequirement } + /** Something resolves, but none of its values match. */ + | { state: 'mismatched'; expected: FlyOwnershipRequirement; found: string[] }; + +/** + * Split raw TXT strings into candidate ownership values. + * + * Two shapes have to survive this. A DNS TXT record is a LIST of character + * strings (Node's `resolveTxt` returns `string[][]`, one inner array per record, + * chunked at 255 bytes) — the chunks of one record concatenate with no + * separator, or a long value silently fails to match. And Fly documents that + * MULTIPLE ownership values may share one record, "separated with semicolons" — + * which is how a hostname serves two Fly apps at once, and why a strict equality + * test against the whole string would reject a correctly-configured domain. + * + * Surrounding quotes are stripped: several DNS UIs store the value with the + * quoting from the zone-file syntax included, and a resolver hands that back + * verbatim. + */ +export function parseOwnershipTxtValues(records: readonly (readonly string[])[]): string[] { + const values: string[] = []; + for (const chunks of records) { + const joined = chunks.join(''); + for (const part of joined.split(';')) { + const trimmed = part.trim().replace(/^"(.*)"$/s, '$1').trim(); + if (trimmed.length > 0) values.push(trimmed); + } + } + return values; +} + +/** + * The ownership values Fly will accept for this requirement, in preference order. + * + * Either value satisfies Fly, and either may be absent, so the empties are + * filtered here ONCE — both the comparison and the customer-facing instruction + * read this list, so they cannot disagree about what counts as acceptable. + */ +export function acceptedOwnershipValues(requirement: FlyOwnershipRequirement): string[] { + return [requirement.appValue, requirement.orgValue].filter((v) => v.length > 0); +} + +/** + * Compare the published TXT values against what Fly asked for. + * + * `null` requirement means Fly reported no ownership requirement at all, which + * is the common case (a hostname already pointing at us validates by + * reachability) — and it is reported as `not_required` rather than `satisfied` + * so a caller cannot read "we verified ownership" out of "we never checked". + * + * EITHER the app value or the org value is accepted, because Fly accepts either. + * Comparison is case-insensitive: these values travel through DNS UIs that + * normalize case, and a case-folded record still satisfies Fly. + */ +export function verifyFlyOwnershipTxt(args: { + requirement: FlyOwnershipRequirement | null; + records: readonly (readonly string[])[]; +}): FlyOwnershipVerification { + const { requirement } = args; + if (!requirement) return { state: 'not_required' }; + + const found = parseOwnershipTxtValues(args.records); + if (found.length === 0) return { state: 'missing', expected: requirement }; + + const accepted = acceptedOwnershipValues(requirement).map((v) => v.toLowerCase()); + // No accepted value means Fly asked for ownership but named nothing to publish. + // Treat that as mismatched rather than satisfied: it is a state we cannot act + // on, and calling it satisfied would let a cert be declared blocked-on-nothing. + if (accepted.length === 0) return { state: 'mismatched', expected: requirement, found }; + + const matched = found.some((value) => accepted.includes(value.toLowerCase())); + return matched ? { state: 'satisfied' } : { state: 'mismatched', expected: requirement, found }; +} + +/** + * What Fly says it will accept, rendered for a human. + * + * Reads the SAME list `verifyFlyOwnershipTxt` compares against, because the two + * had already drifted: verification accepts an app-only OR an org-only + * requirement (it filters empties), while the instruction always printed + * `appValue`. On an org-only requirement — Fly names an org value and no app + * value — that produced an instruction with a blank where the value belongs, in + * exactly the state where the customer has nothing but this message to act on. + * + * Both are shown when Fly names both, since either satisfies it and publishing + * the wrong one of a pair the message never mentioned is a failure mode of its + * own. + * + * The phrasing changes with the count, and that is not fussiness: "with the + * value A or B" reads as one value whose text is "A or B", which is a string a + * customer can and will paste into a TXT record. Naming them as alternatives + * removes the reading. + */ +function describeAcceptedValues(requirement: FlyOwnershipRequirement): string { + const accepted = acceptedOwnershipValues(requirement); + if (accepted.length === 0) return ''; + if (accepted.length === 1) return `the value ${accepted[0]}`; + return `either of these values: ${accepted.join(' or ')}`; +} + +/** + * The same values as a bare list, for a sentence that already supplies the noun. + * + * The `mismatched` message says "(expected X; found Y)", where "expected" + * already does the work "the value" does in the instruction. Reusing the + * instruction's phrasing there produced "expected the value org-X; found …" and + * "expected either of these values: app-X or org-Y; found …" — a colon and a + * semicolon fighting inside one parenthesis. One helper cannot serve both + * sentences, so it does not try to. + */ +function listAcceptedValues(requirement: FlyOwnershipRequirement): string { + return acceptedOwnershipValues(requirement).join(' or '); +} + +/** + * The message for a requirement that names no value at all. `verifyFlyOwnershipTxt` + * reports that state as `mismatched` rather than `satisfied`; the instruction has + * to match, because telling a customer to publish nothing is worse than telling + * them the requirement itself is unusable. + */ +const UNNAMED_OWNERSHIP_VALUE = + 'Fly reported an ownership requirement without naming a value to publish — retry the certificate check, and contact support if it persists.'; + +/** A human-readable instruction for a verification that is not yet satisfied. */ +export function describeOwnershipVerification(result: FlyOwnershipVerification): string | null { + switch (result.state) { + case 'not_required': + case 'satisfied': + return null; + case 'missing': { + const expected = describeAcceptedValues(result.expected); + if (!expected) return UNNAMED_OWNERSHIP_VALUE; + return `Add a TXT record at ${result.expected.name} with ${expected} — Fly cannot verify ownership of this domain until it resolves.`; + } + case 'mismatched': { + const expected = listAcceptedValues(result.expected); + if (!expected) return UNNAMED_OWNERSHIP_VALUE; + return `The TXT record at ${result.expected.name} does not carry an accepted ownership value (expected ${expected}; found ${result.found.join(', ')}).`; + } + } +}