Compare commits

..

1 Commits

Author SHA1 Message Date
MechaCat02
0a3877be51 chore: full hop-by-hop header strip and 60s timeout on /api/* proxy
The SvelteKit proxy was only stripping host + content-length; the rest
of RFC 7230 §6.1 (connection, keep-alive, proxy-authenticate,
proxy-authorization, te, trailer, transfer-encoding, upgrade) leaked
through to axum. Axum doesn't emit them so the impact is theoretical,
but the proxy should be RFC-conformant. Also adds an AbortController
with a configurable 60s timeout (BACKEND_PROXY_TIMEOUT_MS) so a
wedged backend can't hang the browser request indefinitely — failures
surface as the standard 502 upstream_unavailable envelope.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-28 20:07:39 +02:00
5 changed files with 141 additions and 67 deletions

View File

@@ -51,3 +51,8 @@ MAX_FILE_BYTES=20971520
# internal docker network. Override only if you're running the
# frontend container against a backend somewhere else.
BACKEND_URL=http://backend:8080
# Per-request wall-clock cap for the /api/* reverse proxy (milliseconds).
# Default 300000 (5 min) covers a typical 200 MiB chapter upload over
# 25 Mbps; raise for users on slower upstream links or lower if a
# tighter front proxy already bounds the request lifetime.
BACKEND_PROXY_TIMEOUT_MS=300000

View File

@@ -19,49 +19,12 @@ COPY migrations ./migrations
RUN touch src/main.rs src/lib.rs && cargo build --locked --release
FROM debian:bookworm-slim
# `curl` is for the container HEALTHCHECK; `ca-certificates` is for
# outbound HTTPS (crawler covers/pages).
RUN apt-get update \
&& apt-get install -y --no-install-recommends ca-certificates curl \
&& apt-get install -y --no-install-recommends ca-certificates \
&& rm -rf /var/lib/apt/lists/*
# Non-root runtime user. The API binary doesn't need any root
# privilege; the crawler daemon's Chromium launcher uses --no-sandbox
# precisely because user-namespace sandboxing is fragile, so dropping
# privileges costs nothing operationally and shrinks the blast radius
# of any RCE.
ARG APP_UID=10001
ARG APP_GID=10001
RUN groupadd --system --gid ${APP_GID} app \
&& useradd --system --uid ${APP_UID} --gid app --home-dir /home/app --create-home --shell /usr/sbin/nologin app
WORKDIR /app
COPY --from=builder /app/target/release/mangalord /usr/local/bin/mangalord
COPY --from=builder /app/migrations /app/migrations
ENV STORAGE_DIR=/var/lib/mangalord/storage
# Pre-create the storage dir so the entrypoint doesn't need to
# mkdir-as-root and so the named volume mount inherits the right
# ownership.
#
# UPGRADE NOTE for operators: if you're moving from an older image
# that ran as root, the existing `storage-data` volume has files owned
# by UID 0 and the new UID-10001 user can't write them. Run once
# before the upgrade:
# docker compose run --rm --user 0 backend \
# chown -R 10001:10001 /var/lib/mangalord/storage
# (Postgres is unaffected — that image's `postgres` user UID hasn't
# changed.)
RUN mkdir -p ${STORAGE_DIR} \
&& chown -R app:app ${STORAGE_DIR} /app /home/app
USER app
EXPOSE 8080
# `--start-period` is generous because first boot runs sqlx::migrate
# against postgres which can take a few seconds; subsequent restarts
# are sub-second.
HEALTHCHECK --interval=30s --timeout=5s --start-period=20s --retries=3 \
CMD curl -fsS http://localhost:8080/api/v1/health > /dev/null || exit 1
CMD ["mangalord"]

View File

@@ -1,11 +1,7 @@
FROM node:22-alpine AS builder
WORKDIR /app
COPY package.json package-lock.json* ./
# `npm ci` installs the locked versions exactly; `npm install` would
# silently rewrite package-lock.json mid-build. CI (.gitea/workflows)
# also uses `npm ci`, so this keeps the image build deterministic and
# matches what the test job validated.
RUN npm ci
RUN npm install
COPY . .
RUN npm run build
@@ -14,20 +10,8 @@ WORKDIR /app
ENV NODE_ENV=production
ENV HOST=0.0.0.0
ENV PORT=3000
# node:22-alpine ships a `node` user (UID 1000); use it instead of
# running the SvelteKit server as root.
COPY --from=builder --chown=node:node /app/build ./build
COPY --from=builder --chown=node:node /app/node_modules ./node_modules
COPY --from=builder --chown=node:node /app/package.json ./
USER node
COPY --from=builder /app/build ./build
COPY --from=builder /app/node_modules ./node_modules
COPY --from=builder /app/package.json ./
EXPOSE 3000
# Alpine's busybox `wget` is the canonical lightweight HTTP probe.
# `--spider` doesn't follow redirects; `node build` serves a 200 on
# `/` for the homepage so this works without a dedicated /health.
HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \
CMD wget -q --spider http://localhost:3000/ || exit 1
CMD ["node", "build"]

View File

@@ -118,4 +118,77 @@ describe('hooks.server proxy', () => {
expect(body.error.code).toBe('upstream_unavailable');
expect(errSpy).toHaveBeenCalled();
});
it('strips every hop-by-hop header listed in RFC 7230 §6.1', async () => {
// Defence in depth: axum doesn't emit these, but a future
// middleware that did would otherwise leak per-connection
// state across the proxy boundary.
fetchSpy.mockResolvedValueOnce(new Response('[]', { status: 200 }));
const resolve = vi.fn();
await handle({
event: makeEvent('/api/v1/health', {
headers: {
host: 'app.example.com',
'content-length': '0',
connection: 'keep-alive',
'keep-alive': 'timeout=5',
'proxy-authenticate': 'Basic realm=x',
'proxy-authorization': 'Basic xyz',
te: 'trailers',
trailer: 'Expires',
'transfer-encoding': 'chunked',
upgrade: 'websocket',
// A non-hop-by-hop header to ensure non-targets
// aren't accidentally stripped.
'x-custom': 'pass-through'
}
}),
resolve
});
const init = fetchSpy.mock.calls[0][1] as RequestInit;
const headers = init.headers as Headers;
for (const h of [
'host',
'content-length',
'connection',
'keep-alive',
'proxy-authenticate',
'proxy-authorization',
'te',
'trailer',
'transfer-encoding',
'upgrade'
]) {
expect(headers.get(h), `${h} should be stripped`).toBeNull();
}
expect(headers.get('x-custom')).toBe('pass-through');
});
it('aborts and returns 502 when the upstream stalls past the timeout', async () => {
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
// Simulate an aborted fetch (AbortController.abort() raises a
// DOMException with name 'AbortError' on Node's fetch). The
// handler should treat it as the same upstream_unavailable
// 502 it uses for any other network failure.
const abortErr = new DOMException('aborted', 'AbortError');
fetchSpy.mockRejectedValueOnce(abortErr);
const resolve = vi.fn();
const resp = await handle({ event: makeEvent('/api/v1/slow'), resolve });
expect(resp.status).toBe(502);
const body = await resp.json();
expect(body.error.code).toBe('upstream_unavailable');
expect(errSpy).toHaveBeenCalled();
});
it('attaches an AbortSignal to the upstream fetch so it can time out', async () => {
fetchSpy.mockResolvedValueOnce(new Response('[]', { status: 200 }));
const resolve = vi.fn();
await handle({ event: makeEvent('/api/v1/health'), resolve });
const init = fetchSpy.mock.calls[0][1] as RequestInit;
expect(init.signal).toBeInstanceOf(AbortSignal);
// The signal hasn't fired (handler returned in time), but its
// presence is the contract this test is pinning.
expect(init.signal?.aborted).toBe(false);
});
});

View File

@@ -12,20 +12,66 @@ import type { Handle } from '@sveltejs/kit';
const BACKEND_URL = process.env.BACKEND_URL ?? 'http://localhost:8080';
/**
* Hop-by-hop headers per RFC 7230 §6.1. These are scoped to a single
* transport-level connection and must not be forwarded by a proxy.
* Plus `host` and `content-length`: `host` would mislead the backend
* about its origin, and `content-length` is recomputed by the upstream
* fetch from the body stream.
*/
const HOP_BY_HOP_HEADERS = [
'host',
'content-length',
'connection',
'keep-alive',
'proxy-authenticate',
'proxy-authorization',
'te',
'trailer',
'transfer-encoding',
'upgrade'
];
/**
* Cap each proxied request at 5 minutes. The bound exists to surface
* a wedged backend (stuck on a slow DB query, deadlocked, etc.) as a
* 502 rather than letting the browser request hang indefinitely.
*
* The default leans toward the slow-upload end of the spectrum: at a
* 1 Mbps upstream, a 200 MiB chapter upload (the default
* `MAX_REQUEST_BYTES` cap) needs ~27 minutes; 300 s covers the more
* realistic 25 Mbps urban-broadband case (~64 s for the same upload)
* with comfortable headroom. Operators serving very slow clients
* should raise `BACKEND_PROXY_TIMEOUT_MS`; operators behind a
* tighter upstream proxy may want to lower it. A future improvement
* is an idle-based timeout (reset per chunk) instead of this
* wall-clock budget — that's a fair bit more code, deferred.
*/
const PROXY_TIMEOUT_MS = (() => {
const raw = process.env.BACKEND_PROXY_TIMEOUT_MS;
const n = raw ? Number(raw) : 300_000;
return Number.isFinite(n) && n > 0 ? n : 300_000;
})();
export const handle: Handle = async ({ event, resolve }) => {
if (event.url.pathname.startsWith('/api/')) {
const target = `${BACKEND_URL}${event.url.pathname}${event.url.search}`;
// Strip hop-by-hop headers — `host` would mislead the backend
// about the origin, and `content-length` will be recomputed.
const headers = new Headers(event.request.headers);
headers.delete('host');
headers.delete('content-length');
for (const h of HOP_BY_HOP_HEADERS) headers.delete(h);
// AbortController times the upstream fetch out so a backend
// wedged on a slow DB query doesn't keep the browser request
// hanging forever. The `signal` is also wired into the
// RequestInit so the body stream is cancelled cleanly.
const ctrl = new AbortController();
const timeoutHandle = setTimeout(() => ctrl.abort(), PROXY_TIMEOUT_MS);
const init: RequestInit & { duplex?: 'half' } = {
method: event.request.method,
headers,
redirect: 'manual'
redirect: 'manual',
signal: ctrl.signal
};
if (event.request.method !== 'GET' && event.request.method !== 'HEAD') {
init.body = event.request.body;
@@ -39,11 +85,13 @@ export const handle: Handle = async ({ event, resolve }) => {
upstream = await fetch(target, init);
} catch (e) {
// Network-layer failure (DNS / connection refused / TLS
// handshake) — most commonly "backend container restarting".
// SvelteKit's default 500 would be an HTML page that
// client.ts can't .json(), which masks the real cause. Emit
// the standard envelope with a dedicated code instead.
// handshake / abort by timeout) — most commonly "backend
// container restarting". SvelteKit's default 500 would be
// an HTML page that client.ts can't .json(), which masks
// the real cause. Emit the standard envelope with a
// dedicated code instead.
console.error('Proxy to backend failed:', e);
clearTimeout(timeoutHandle);
return new Response(
JSON.stringify({
error: {
@@ -58,6 +106,7 @@ export const handle: Handle = async ({ event, resolve }) => {
);
}
clearTimeout(timeoutHandle);
return new Response(upstream.body, {
status: upstream.status,
statusText: upstream.statusText,