The sanitized operational line promised for a header/request timeout was never actually emitted. Driving a real slow-header and slow-body connection through a real socket against the pinned Node 24 runtime shows the general server 'timeout' event never fires for headersTimeout/requestTimeout — that event belongs to server.setTimeout()'s idle-socket timer, which this server never arms. Both surface as 'clientError' carrying err.code === "ERR_HTTP_REQUEST_TIMEOUT". bindServer now observes 'clientError'. Because registering any such listener suppresses Node's own default response entirely, the handler reproduces that default byte-for-byte — 408 for a timeout, 431 for a header overflow, 400 otherwise — writing it only when the socket is still writable and no response has already begun, then destroying the socket exactly as Node would. Exactly one fixed sanitized line is logged per event, never the caught error's own message, which for a parse failure derives from attacker-controlled input. Proof moves to the real transport boundary: raw-socket scenarios assert the exact response bytes, exactly one status line, socket closure, handler non-invocation, and exactly one sanitized log for a slow-header connection, a slow-body connection, a malformed request line, and an oversized header block. The prior unit test, which manually emitted 'timeout', is removed as misleading. Co-Authored-By: Claude Opus 5 <[email protected]>
19 KiB
0017: Public request perimeter hardening — client address, signup gating, body limits, timeouts
Context
An independent security audit against main (Phase 5C checkpoint, commit 5e093300) found that
apps/api's public HTTP perimeter had four related gaps before any Surface work or additional
module-platform capability could responsibly proceed:
- Better Auth's rate limiter resolved client identity from a caller-controlled
X-Forwarded-Forheader by default (ADR 0011 explicitly left trusted-proxy configuration unset, but the pinned release's own fallback still trusts a single-value header with no proxy policy at all). A caller could rotate the header value on every request to make each attempt look like a new, unlimited client, or collapse unrelated callers into one bucket when no header was present. - Public registration (ADR 0011) had no verified email, CAPTCHA, invitation, or durable distributed limiter, and — more immediately — no way to disable it at all. Every deployment was permanently open to unlimited account creation.
- Neither Better Auth's endpoints, Core's grant-mutation endpoint, nor any module route enforced
a request-body size limit.
core/src/grants-routes.ts'sc.req.json()and any module handler reading a body could be driven with an arbitrarily large or indefinitely streamed payload. - The Node HTTP server had no configured
headersTimeout/requestTimeout, leaving Node's own generous defaults (60s/300s) as the only bound on a slow-header or slow-body connection.
This ADR records the pass-1 fix for all four, as one bounded slice (Phase 5D pass 1). Passes 2–5 (identity/database transport hardening, authorization integrity, module/runtime/migration correctness, and deployment/supply-chain closure) remain separate, later work.
Decision
One platform-owned client-address resolution boundary
core/src/identity/client-address.ts defines a framework-agnostic policy —
ClientAddressConfig ({mode: "direct"} or {mode: "trusted-proxy", header, trustedProxies}),
createClientAddressConfig (strict validation, safe default), and
createClientAddressResolver (the actual per-request resolution algorithm) — built only on
node:net and the standard Headers type, so it stays importable by Core without pulling Hono
into it. It never reads a request header for "direct" mode: it trusts only the value it is given
as the actual immediate socket peer address. For "trusted-proxy" mode, a forwarded header is
consulted only when the actual immediate peer matches a configured trusted-proxy
address/CIDR; otherwise the peer address itself is used, exactly as if no header existed. A
forwarded chain (comma-separated) is walked right to left: trailing hops matching a trusted proxy
are skipped, and the first non-matching hop from the right is the resolved address. A missing,
empty, malformed, or entirely-trusted-hops chain from an otherwise-trusted peer fails closed
(PEER_ADDRESS_UNAVAILABLE / FORWARDED_HEADER_MISSING / FORWARDED_HEADER_MALFORMED /
FORWARDED_CHAIN_EXHAUSTED) rather than guessing or falling back to a shared bucket.
Node's own runtime cannot be told about the real socket peer from inside Hono — only the raw
IncomingMessage (@hono/node-server's HttpBindings.incoming) has it. apps/api/src/ client-address.ts therefore adds exactly one Hono-adjacent piece, wrapFetchWithClientAddress,
which wraps the fetch callback passed to @hono/node-server's serve() in apps/api/src/ server.ts: it resolves the trusted address from env.incoming.socket.remoteAddress before the
wrapped callback (and therefore every route, including Better Auth's own handler) ever sees the
request, then unconditionally overwrites — never appends to — any externally supplied copy of a
fixed internal header (TRUSTED_CLIENT_ADDRESS_HEADER = "x-ekklesia-client-address",
core/src/identity/runtime.ts) with the resolved value. A resolution failure returns a sanitized
503 {"code":"CLIENT_ADDRESS_UNRESOLVED"} directly from this wrapper — the wrapped callback,
Better Auth, Core, and every module handler are all unreachable in that case.
Better Auth is configured with advanced.ipAddress.ipAddressHeaders: [TRUSTED_CLIENT_ADDRESS_HEADER]
and no trustedProxies of its own: the pinned release's own trustedProxies option only inspects
header content for proxy-shaped addresses and cannot verify who actually sent the request (its
own doc comment says as much), so the real trust decision is made exactly once, at the transport
boundary, using the genuine socket peer — never asked of Better Auth a second time. Core never
reinterprets an untrusted proxy header itself; it only ever receives the one already-resolved,
already-sanitized header apps/api produced.
This closes the original reproduction directly: rotating a spoofed X-Forwarded-For on every
request no longer changes the rate-limit bucket in direct mode (the default), because the header
is never consulted at all. The in-memory rate-limit store remains per-process (core/src/identity/ runtime.ts, unchanged from ADR 0011) — this pass does not add distributed storage; a genuinely
multi-instance deployment still needs a shared or edge limiter, reserved for pass 5.
Public signup is disabled by default, and cannot be enabled in production at all
IdentityConfig.publicSignupEnabled (core/src/identity/config.ts) is a strict boolean:
undefined input means false (disabled); only an explicit boolean or the exact strings
"true"/"false" are accepted, so an absent, misspelled, or loosely parsed environment variable
can never enable it.
Production refuses enablement outright. validatePublicSignupEnabled takes the same
IdentityEnvironment the rest of this config already validates against, and throws the stable
PUBLIC_SIGNUP_UNSUPPORTED_IN_PRODUCTION error whenever the parsed value is true in production.
The durable signup-abuse prerequisites a public registration endpoint requires — verified email or
an invitation requirement, plus durable shared/edge throttling rather than this release's
per-process in-memory limiter — do not exist in this repository, so there is no production
configuration in which the flag could be honored safely; permitting a deliberate opt-in would be
offering a switch whose safe position does not exist yet. Parsing is kept separate from this policy
so a malformed value still reports PUBLIC_SIGNUP_ENABLED_INVALID in every environment, and an
operator who typo'd the flag is never told the misleading "unsupported in production" story
instead. Because apps/api's executable entry point calls createApiRuntimeConfig before
startApi, such a deployment fails before any PostgreSQL pool, Better Auth instance, module
runtime, or listener exists. pnpm doctor surfaces the same state as a blocking NOT READY
failure (via its existing IdentityConfigError path), never a warning. Development and test
composition may still enable it explicitly through the identical typed path — that is how this
repository's own integration suite creates accounts over HTTP. When the prerequisites are actually
implemented and proven, this one function is the single place that changes.
core/src/identity/runtime.ts wires the validated flag directly to Better Auth's own
emailAndPassword.disableSignUp flag: when disabled, POST /sign-up/email rejects every call
with a fixed 400 {"code":"EMAIL_PASSWORD_SIGN_UP_DISABLED"} before Better Auth ever looks up the
email, so a disabled deployment cannot leak whether an address is already registered, and
apps/api/src/app.ts now forces Cache-Control: no-store on every /api/auth/* response
(previously unset on the success path). Sign-in, session resolution, and sign-out are entirely
unaffected — this is Better Auth's own native, already-audited gate, not a second, competing
platform-owned check, since the pinned release's own mechanism already satisfies every required
property (no email-existence leak, no business logic invoked, a stable code). Since email/password
is the only enabled provider (ADR 0011), there is no second signup entry point to separately guard.
This amends ADR 0011's framing ("Anyone may create an Ekklesia account") for production entirely —
public registration remains the intended eventual product policy, but is currently reachable only
in development/test composition (publicSignupEnabled: true with environment: "development");
ADR 0011's
architectural description of what a registered account is (an authentication identity, not a
membership record) is otherwise unchanged and still authoritative. This pass adds no email
verification, CAPTCHA, invitation, or durable abuse control — those remain explicitly deferred
prerequisites for a real future public-enrollment mode.
One platform-owned request-body policy, and a module route's own conscious body contract
apps/api/src/body-limits.ts centralizes three named, typed values built on Hono's pinned
hono/body-limit middleware (verified directly against its source: it rejects an oversized
declared Content-Length synchronously from the header alone, and rejects an oversized
declared-length-free body by reading and counting the stream incrementally, aborting as soon as
the running total exceeds the limit — never buffering the whole body first):
AUTH_REQUEST_MAX_BODY_BYTES= 8 KiB, applied to every/api/auth/*request before Better Auth's handler ever runs.CORE_MUTATION_MAX_BODY_BYTES= 16 KiB, applied toPOST /api/v1/grantsandPOST /api/v1/grants/:grantId/revokeafterrequireTrustedOrigin(so a rejected cross-origin mutation's body is never read) but before permission evaluation.MODULE_ROUTE_MAX_BODY_BYTES_CEILING= 1 MiB (re-exported from@ekklesia/contracts), the hard ceiling no module route may exceed and the platform's overall body-size ceiling.
Every hono/body-limit instance shares one sanitized public contract,
requestBodyTooLargeResponse — a stable 413 {"code":"REQUEST_BODY_TOO_LARGE"} with
Cache-Control: no-store — so auth, Core, and module rejections are indistinguishable in shape.
A module route's body contract becomes a fifth, required field on the existing route-definition
contract (packages/contracts/src/module-http.ts): maxBodyBytes: number, where 0 means the
route accepts no body at all (a non-empty declared or streamed body is rejected) and any other
value is the exact byte maximum, validated as a non-negative, finite integer no greater than
MODULE_ROUTE_MAX_BODY_BYTES_CEILING (MODULE_ROUTE_MAX_BODY_BYTES_INVALID otherwise). This is a
security contract, not a manifest capability: it does not touch manifestVersion or the static
manifest at all, only the executable http.route(...) call every module already makes during
register. apps/api/src/module-http-routes.ts applies platformBodyLimit(route.maxBodyBytes)
to every mounted route, positioned identically to the Core mutation routes — after
requireTrustedOrigin for a state-changing method, before requirePermission — so an oversized
request never reaches authorization work, a module handler, or module database behavior, and a
rejected cross-origin mutation's body is never read for size at all.
This is a deliberate, minimal public-contract change to an existing executable contract (not the
manifest): every existing module fixture, test, and the template (templates/module/src/server/ index.ts) are updated in this same pass to declare it explicitly, per module-authoring
convention documented in Module development.
Finite Node header/request timeouts, and two non-obvious operational details
apps/api/src/server.ts configures @hono/node-server's serverOptions (passed straight through
to http.createServer) with headersTimeout: 20s, requestTimeout: 30s, and
keepAliveTimeout: 5s (Node's own default, made explicit and typed). Node's own documented
behavior for requestTimeout/headersTimeout — verified directly against the pinned Node 24
runtime with a standalone reproduction — is to respond 408 and close the connection without
ever invoking a request listener, so a timed-out connection cannot invoke a handler or receive a
second response by construction; this pass does not reimplement that behavior.
That same reproduction surfaced a non-obvious operational requirement: Node's http.Server only
checks for a headersTimeout/requestTimeout violation when its periodic
connectionsCheckingInterval fires, and that option's own default is 30 seconds — leaving it
unset would silently let a violating connection sit for up to a further 30 seconds past its
configured timeout before Node ever closes it, on top of the configured value. NodeServerTimeouts
therefore also sets connectionsCheckingIntervalMs: 5s explicitly, so a violation is detected
promptly after it actually elapses rather than after an unrelated, much longer default interval.
The second detail concerns observing those timeouts, and corrects a claim this ADR originally
made. Node's general server 'timeout' event belongs to server.setTimeout()'s idle-socket timer,
which this server never arms — it does not fire for headersTimeout/requestTimeout. Driving
a real slow-header and a real slow-body connection through a real socket against the pinned runtime
shows the only server-level event either produces is 'clientError', carrying
err.code === "ERR_HTTP_REQUEST_TIMEOUT". A server.on("timeout", ...) listener therefore never
fires in production, and the sanitized operational line it was meant to emit was never actually
emitted.
apps/api/src/server.ts consequently observes 'clientError' instead. That carries an obligation:
registering any 'clientError' listener suppresses Node's own default response entirely
(_http_server.js falls back only when server.emit("clientError", ...) returns false), so a
listener that merely watched would silently turn every 408/400/431 into a bare connection
reset. The handler therefore reproduces Node's default byte-for-byte — 408 Request Timeout for
ERR_HTTP_REQUEST_TIMEOUT, 431 Request Header Fields Too Large for HPE_HEADER_OVERFLOW, 400 Bad Request otherwise — writing it only when the socket is still writable and no response has
already begun for the in-flight request (mirroring Node's own _httpMessage/headersSent guard, so
a handler that already responded before an unread body timed out never receives a second, corrupting
status line), then destroys the socket exactly as Node would.
Exactly one fixed, sanitized line is logged per event — one message for a timeout, one for any other
malformed or oversized request. Neither ever contains the request path, query, body, cookies,
authorization value, client-address header, forwarding chain, raw peer address, stack, or the caught
error's own message, since a parse failure's message is derived directly from attacker-controlled
input. This is proven at the real transport boundary rather than by manual event emission: raw-socket
scenarios in apps/api/tests/api-http.integration.ts drive a genuine slow-header connection, a
genuine slow-body connection, a malformed request line, and an oversized header block, each
asserting the exact response bytes, exactly one status line, socket closure, non-invocation of any
handler, and exactly one sanitized log.
Bounded graceful-shutdown deadlines for module stop() hooks are a related but distinct concern,
explicitly deferred to pass 4 and not touched here.
Alternatives
- Setting Better Auth's
advanced.ipAddress.trustedProxiesalone was rejected: that option only inspects the forwarded header's own content for proxy-shaped addresses and can never verify who actually connected, so any direct caller could still choose its own bucket by crafting a header that merely looks trusted-proxy-shaped. - Resolving the client address inside Core's identity runtime directly from headers was
rejected: Core has no access to the real socket peer (it only ever sees a Fetch
Request), and duplicating proxy-trust logic inside Core would create a second, divergent implementation of exactly the policy this ADR centralizes once, at the transport boundary. - A separate CAPTCHA, email-verification, or invitation system to gate signup in this pass was rejected as out of scope: pass 1 closes the default-open exposure; the deeper abuse-control prerequisites for a real public-enrollment mode are reserved, honestly, for later work.
- Adding
maxBodyBytesto the static module manifest instead of the executable route definition was rejected, matching ADR 0015's own reasoning for keeping routes executable data: a manifest field can't be enforced without also trusting the manifest as a security boundary the platform doesn't otherwise treat that way, and it would create a second, potentially-drifting place to declare the same route. - A single blanket global body-limit middleware applied before per-route logic was rejected in
favor of positioning
platformBodyLimitindividually within each route's own middleware chain: a single global instance mounted ahead ofrequireTrustedOriginwould read (or begin reading) a rejected cross-origin mutation's body before the origin check ever ran, which this ADR's ordering requirement forbids. - Leaving Node's
connectionsCheckingIntervalat its default was rejected once the reproduction showed it would silently widen both configured timeouts by up to 30 additional seconds — a security-relevant gap, not merely a latency one.
Consequences
Direct mode (the default) trusts only the real socket peer for every request; explicit
trusted-proxy mode requires a header name and a concrete, non-empty trusted-proxy list, validated
the same way in core/src/identity/client-address.ts and surfaced by Doctor
(identity.client-address), which distinguishes safe direct mode, an explicitly configured proxy
mode, and an invalid configuration. Public signup is closed by default everywhere, and production
cannot open it at all until the durable abuse-control prerequisites exist — only development and
test composition may enable it. Every body-consuming public route — auth, Core, and every module
route — now has an explicit, enforced byte ceiling with one sanitized 413 contract. The Node
server itself now bounds a slow-header or slow-body connection instead of relying on Node's
generous multi-minute defaults. None of this constitutes distributed rate limiting, a durable
signup-abuse control, or a claim that the origin is unreachable except through a configured proxy
— the deployment's own reverse proxy and firewall remain responsible for that, and durable/shared
throttling for a multi-instance or public-signup deployment remains explicitly reserved for pass 5.