diff --git a/src/app.ts b/src/app.ts index abd39fec..fa1a7cd9 100644 --- a/src/app.ts +++ b/src/app.ts @@ -43,7 +43,6 @@ import * as Sentry from '@sentry/node'; import { get } from 'es-toolkit/compat'; import { Hono, type Context as HonoContext, type Next } from 'hono'; import { cors } from 'hono/cors'; -import { behindProxy } from 'x-forwarded-fetch'; import type { Account } from '@/account/account.entity'; import { AccountBlockedEvent } from '@/account/events/account-blocked.event'; @@ -103,6 +102,7 @@ import type { FeedUpdateService } from '@/feed/feed-update.service'; import type { FlagService } from '@/flag/flag.service'; import type { GhostPostService } from '@/ghost/ghost-post.service'; import { getTraceContext } from '@/helpers/context-header'; +import { isLocalEnvironment } from '@/helpers/environment'; import { AccountController } from '@/http/api/account.controller'; import { BlockController } from '@/http/api/block.controller'; import { BlueskyController } from '@/http/api/bluesky.controller'; @@ -123,6 +123,7 @@ import type { SiteController } from '@/http/api/site.controller'; import { TopicController } from '@/http/api/topic.controller'; import type { WebFingerController } from '@/http/api/webfinger.controller'; import type { WebhookController } from '@/http/api/webhook.controller'; +import { createFetchHandler } from '@/http/fetch-handler'; import type { HostDataContextLoader } from '@/http/host-data-context-loader'; import { createDeploymentHeadersMiddleware } from '@/http/middleware/deployment-headers'; import { createHostDataContextMiddleware } from '@/http/middleware/host-data-context'; @@ -1037,16 +1038,9 @@ app.onError((err, c) => { return c.text('Internal Server Error', 500); }); -function forceAcceptHeader(fn: (req: Request) => unknown) { - return (request: Request) => { - request.headers.set('accept', 'application/activity+json'); - return fn(request); - }; -} - serve( { - fetch: forceAcceptHeader(behindProxy(app.fetch)), + fetch: createFetchHandler(process.env.NODE_ENV, app.fetch), port: Number.parseInt(process.env.PORT || '8080', 10), }, (info) => { @@ -1084,13 +1078,13 @@ async function gracefulShutdown(signal: 'SIGINT' | 'SIGTERM') { } process.on('SIGINT', () => { - if (['development', 'testing'].includes(process.env.NODE_ENV || '')) { + if (isLocalEnvironment(process.env.NODE_ENV)) { process.exit(0); } void gracefulShutdown('SIGINT'); }); process.on('SIGTERM', () => { - if (['development', 'testing'].includes(process.env.NODE_ENV || '')) { + if (isLocalEnvironment(process.env.NODE_ENV)) { process.exit(0); } void gracefulShutdown('SIGTERM'); diff --git a/src/configuration/registrations.ts b/src/configuration/registrations.ts index 8db234c7..c53b1ed3 100644 --- a/src/configuration/registrations.ts +++ b/src/configuration/registrations.ts @@ -48,6 +48,7 @@ import { FeedService } from '@/feed/feed.service'; import { FeedUpdateService } from '@/feed/feed-update.service'; import { FlagService } from '@/flag/flag.service'; import { GhostPostService } from '@/ghost/ghost-post.service'; +import { isLocalEnvironment } from '@/helpers/environment'; import { getSiteSettings } from '@/helpers/ghost'; import { AccountController } from '@/http/api/account.controller'; import { BlockController } from '@/http/api/block.controller'; @@ -256,14 +257,10 @@ export function registerDependencies( circuitBreaker: false, skipSignatureVerification: process.env.SKIP_SIGNATURE_VERIFICATION === 'true' && - ['development', 'testing'].includes( - process.env.NODE_ENV || '', - ), + isLocalEnvironment(process.env.NODE_ENV), allowPrivateAddress: process.env.ALLOW_PRIVATE_ADDRESS === 'true' && - ['development', 'testing'].includes( - process.env.NODE_ENV || '', - ), + isLocalEnvironment(process.env.NODE_ENV), firstKnock: 'draft-cavage-http-signatures-12', }); }).singleton(), diff --git a/src/helpers/environment.ts b/src/helpers/environment.ts new file mode 100644 index 00000000..f7ae9062 --- /dev/null +++ b/src/helpers/environment.ts @@ -0,0 +1,17 @@ +/** + * Environments that run over plain http (local development and CI). Code that + * follows the request scheme (e.g. the JWKS lookup in the role middleware) + * relies on requests staying http in these environments, and the serve + * boundary must not force the scheme to https for them (see + * `createFetchHandler`). + * + * Every other environment — including an unset or unrecognised `NODE_ENV` — + * is treated as being served over https, matching the canonical account URLs + * which are always created with an https scheme + * (`AccountService.createInternalAccount`). + */ +const LOCAL_ENVIRONMENTS = ['development', 'testing']; + +export function isLocalEnvironment(environment: string | undefined): boolean { + return LOCAL_ENVIRONMENTS.includes(environment || ''); +} diff --git a/src/helpers/environment.unit.test.ts b/src/helpers/environment.unit.test.ts new file mode 100644 index 00000000..a98d9dd0 --- /dev/null +++ b/src/helpers/environment.unit.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest'; + +import { isLocalEnvironment } from './environment'; + +describe('isLocalEnvironment', () => { + it('should return true for local environments', () => { + expect(isLocalEnvironment('development')).toBe(true); + expect(isLocalEnvironment('testing')).toBe(true); + }); + + it('should return false for deployed environments', () => { + expect(isLocalEnvironment('staging')).toBe(false); + expect(isLocalEnvironment('production')).toBe(false); + }); + + it('should return false when the environment is unset or unrecognised', () => { + expect(isLocalEnvironment(undefined)).toBe(false); + expect(isLocalEnvironment('')).toBe(false); + expect(isLocalEnvironment('prod')).toBe(false); + }); +}); diff --git a/src/http/fetch-handler.ts b/src/http/fetch-handler.ts new file mode 100644 index 00000000..3820f13e --- /dev/null +++ b/src/http/fetch-handler.ts @@ -0,0 +1,61 @@ +import { behindProxy } from 'x-forwarded-fetch'; + +import { isLocalEnvironment } from '@/helpers/environment'; + +type FetchHandler = (request: Request) => Response | Promise; + +/** + * Decorate a `fetch()` function so that the request is always treated as + * having been made over https. + * + * Canonical URLs are always https (see `AccountEntity.draft` / + * `AccountService.createInternalAccount`), but Fedify derives generated URIs + * (e.g. the actor's `publicKey.id`) from the incoming request URL. A reverse + * proxy that omits the `X-Forwarded-Proto` header would leave the request URL + * as http, producing http URIs that contradict the stored https actor IDs and + * breaking HTTP signature verification on remote servers. + * + * Fedify's `origin` option on `createFederation` would be the first-class way + * to pin generated URIs, but it takes a single static origin — unusable here, + * where the host varies per tenant. + * + * This must wrap a `fetch()` function already decorated with `behindProxy` + * (i.e. run before it), as `behindProxy` is what applies the header to the + * request URL — `createFetchHandler` owns that composition. + */ +function forceHttps(fetch: FetchHandler): FetchHandler { + return (request: Request) => { + request.headers.set('x-forwarded-proto', 'https'); + return fetch(request); + }; +} + +function forceAcceptHeader(fetch: FetchHandler): FetchHandler { + return (request: Request) => { + request.headers.set('accept', 'application/activity+json'); + return fetch(request); + }; +} + +/** + * Build the `fetch()` function passed to `serve()`: applies `X-Forwarded-*` + * headers to the request URL and forces the accept header, and — outside + * local environments, which serve plain http — forces the https scheme so + * generated URIs match the stored https canonical URLs regardless of proxy + * configuration. + * + * @param environment `process.env.NODE_ENV` + * @param fetch The app's `fetch()` function + */ +export function createFetchHandler( + environment: string | undefined, + fetch: FetchHandler, +): FetchHandler { + const proxiedFetch = behindProxy(fetch); + + return forceAcceptHeader( + isLocalEnvironment(environment) + ? proxiedFetch + : forceHttps(proxiedFetch), + ); +} diff --git a/src/http/fetch-handler.unit.test.ts b/src/http/fetch-handler.unit.test.ts new file mode 100644 index 00000000..78e4914f --- /dev/null +++ b/src/http/fetch-handler.unit.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest'; + +import { createFetchHandler } from './fetch-handler'; + +async function dispatch(environment: string | undefined, request: Request) { + let receivedRequest: Request | undefined; + + const fetch = createFetchHandler(environment, (request: Request) => { + receivedRequest = request; + return new Response(); + }); + + await fetch(request); + + if (!receivedRequest) { + throw new Error('Expected the wrapped fetch to be called'); + } + + return receivedRequest; +} + +describe('createFetchHandler', () => { + for (const environment of ['staging', 'production']) { + it(`should force an https request URL in ${environment} when x-forwarded-proto is missing`, async () => { + const request = await dispatch( + environment, + new Request('http://example.com/foo'), + ); + + expect(request.url).toBe('https://example.com/foo'); + }); + + it(`should force an https request URL in ${environment} when x-forwarded-proto is http`, async () => { + const request = await dispatch( + environment, + new Request('http://example.com/foo', { + headers: { + 'x-forwarded-proto': 'http', + }, + }), + ); + + expect(request.url).toBe('https://example.com/foo'); + }); + } + + it('should force an https request URL when NODE_ENV is unset or unrecognised', async () => { + for (const environment of [undefined, '', 'prod']) { + const request = await dispatch( + environment, + new Request('http://example.com/foo'), + ); + + expect(request.url).toBe('https://example.com/foo'); + } + }); + + for (const environment of ['development', 'testing']) { + it(`should keep an http request URL in ${environment}`, async () => { + const request = await dispatch( + environment, + new Request('http://example.com/foo'), + ); + + expect(request.url).toBe('http://example.com/foo'); + }); + + it(`should still honour x-forwarded-proto in ${environment}`, async () => { + const request = await dispatch( + environment, + new Request('http://example.com/foo', { + headers: { + 'x-forwarded-proto': 'https', + }, + }), + ); + + expect(request.url).toBe('https://example.com/foo'); + }); + } + + it('should apply x-forwarded-host to the request URL', async () => { + const request = await dispatch( + 'production', + new Request('http://internal.host/foo', { + headers: { + 'x-forwarded-host': 'example.com', + }, + }), + ); + + expect(request.url).toBe('https://example.com/foo'); + }); + + it('should force the accept header in every environment', async () => { + for (const environment of ['development', 'production']) { + const request = await dispatch( + environment, + new Request('http://example.com/foo', { + headers: { + accept: 'text/html', + }, + }), + ); + + expect(request.headers.get('accept')).toBe( + 'application/activity+json', + ); + } + }); +}); diff --git a/src/http/middleware/role-guard.ts b/src/http/middleware/role-guard.ts index c338ce40..8037651b 100644 --- a/src/http/middleware/role-guard.ts +++ b/src/http/middleware/role-guard.ts @@ -4,6 +4,8 @@ import type { Context as HonoContext, Next } from 'hono'; import jwt from 'jsonwebtoken'; import jose from 'node-jose'; +import { isLocalEnvironment } from '@/helpers/environment'; + export enum GhostRole { Anonymous = 'Anonymous', Owner = 'Owner', @@ -21,9 +23,9 @@ function getJwksURL(host: string, ctx: HonoContext) { const GHOST_JWKS_ENDPOINT = '/ghost/.well-known/jwks.json'; let protocol = 'https'; - // We allow insecure requests when not in production for things like testing + // We allow insecure requests in local environments for things like testing if ( - !['staging', 'production'].includes(process.env.NODE_ENV || '') && + isLocalEnvironment(process.env.NODE_ENV) && !ctx.req.raw.url.startsWith('https') ) { protocol = 'http'; diff --git a/src/lookup-helpers.ts b/src/lookup-helpers.ts index 582e4260..8a776a0e 100644 --- a/src/lookup-helpers.ts +++ b/src/lookup-helpers.ts @@ -10,6 +10,7 @@ import { lookupWebFinger } from '@fedify/webfinger'; import type { FedifyContext } from '@/app'; import { error, ok, type Result } from '@/core/result'; +import { isLocalEnvironment } from '@/helpers/environment'; export type LookupError = 'no-links-found' | 'no-self-link' | 'lookup-error'; @@ -82,7 +83,7 @@ export async function lookupActorProfile( const webfingerData = await lookupWebFinger(resource, { allowPrivateAddress: process.env.ALLOW_PRIVATE_ADDRESS === 'true' && - ['development', 'testing'].includes(process.env.NODE_ENV || ''), + isLocalEnvironment(process.env.NODE_ENV), }); if (!webfingerData?.links) {