fix(proxy,auth,insights): close three drift gaps the audits left open

The proxy's seven early exits — worker-only 503, two legacy 301s, the demo-mode
403, and the login / onboarding / MFA redirects — returned before the security
header block, so they carried no HSTS, no framing refusal and no nosniff. Factor
the transport subset into a helper and apply it on every exit. The per-route CSP
stays where it is; it is nonce-bound and only meaningful for a response with a
body.

The WebAuthn relying-party candidate list always carried http://localhost:3000,
including in production. The signed rpIdHash still pinned the real domain, so
this was never a reachable bypass, but a configured production origin has no
reason to carry it. Gate the fallback on development, keeping it when nothing is
configured so getRpId() still resolves.

ORPHAN_ENUMS was a hand-maintained literal that drifted every time
ALLOWED_CHART_TOKENS grew — eighteen allowlisted metrics had no strip entry, so
a model writing 'Your PHQ9_SCORE is elevated' leaked the raw enum into rendered
prose. Derive it from the allowlist and pin the coupling with a test.
This commit is contained in:
Marc-André Bombeck committed 2026-07-19 13:31:29 +02:00
1 parent f5889affb7
commit 8ead442d44
6 files changed
+365 -93

No files matched your search

@@ -0,0 +1,120 @@
/**
* Security headers on the proxy's EARLY EXITS.
*
* The full header block sits at the bottom of `proxy()`, after the pass-through
* `NextResponse.next()`. Seven exits return before it — the worker-only 503,
* two legacy 301s, the demo-mode 403, and the login / onboarding / MFA
* redirects — and used to carry no HSTS, no framing refusal and no nosniff at
* all.
*
* The exposure was one hop (a redirect has no rendered body, and the follow-up
* navigation re-enters the proxy and does get the full set), but HSTS is worth
* the most on exactly that first hop over a hostile network. These assertions
* exist so a future exit added above the header block cannot silently rejoin
* the unprotected set.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { NextRequest } from "next/server";
const shouldRunWeb = vi.fn(() => true);
vi.mock("@/lib/process-type", () => ({
shouldRunWeb: () => shouldRunWeb(),
}));
import { proxy } from "../proxy";
function makeRequest(
pathname: string,
init: { cookies?: Record<string, string>; method?: string } = {},
): NextRequest {
const cookieHeader = Object.entries(init.cookies ?? {})
.map(([k, v]) => `${k}=${v}`)
.join("; ");
return new NextRequest(`http://localhost${pathname}`, {
method: init.method ?? "GET",
headers: cookieHeader ? { cookie: cookieHeader } : {},
});
}
const ORIGINAL_NODE_ENV = process.env.NODE_ENV;
const ORIGINAL_DEMO = process.env.DEMO_MODE;
beforeEach(() => {
shouldRunWeb.mockReturnValue(true);
process.env.NODE_ENV = "production";
delete process.env.DEMO_MODE;
});
afterEach(() => {
process.env.NODE_ENV = ORIGINAL_NODE_ENV;
if (ORIGINAL_DEMO === undefined) delete process.env.DEMO_MODE;
else process.env.DEMO_MODE = ORIGINAL_DEMO;
});
/** The transport subset every exit must carry. */
function expectBaselineHeaders(res: Response) {
expect(res.headers.get("X-Content-Type-Options")).toBe("nosniff");
expect(res.headers.get("X-Frame-Options")).toBe("DENY");
expect(res.headers.get("Referrer-Policy")).toBe(
"strict-origin-when-cross-origin",
);
expect(res.headers.get("Permissions-Policy")).toContain("camera=()");
expect(res.headers.get("Cross-Origin-Opener-Policy")).toBe("same-origin");
expect(res.headers.get("X-Permitted-Cross-Domain-Policies")).toBe("none");
expect(res.headers.get("Strict-Transport-Security")).toContain(
"max-age=31536000",
);
}
describe("proxy early exits carry the baseline security headers", () => {
it("worker-only 503", () => {
shouldRunWeb.mockReturnValue(false);
const res = proxy(makeRequest("/dashboard"));
expect(res.status).toBe(503);
expectBaselineHeaders(res);
});
it("legacy 301 redirect", () => {
const res = proxy(makeRequest("/stimmung"));
expect(res.status).toBe(301);
expectBaselineHeaders(res);
});
it("unauthenticated page redirect to /auth/login", () => {
const res = proxy(makeRequest("/dashboard"));
expect(res.headers.get("location")).toMatch(/\/auth\/login$/);
expectBaselineHeaders(res);
});
it("onboarding redirect", () => {
const res = proxy(
makeRequest("/", {
cookies: { healthlog_session: "sess-1", hl_onboarding: "pending" },
}),
);
expect(res.headers.get("location")).toMatch(/\/onboarding$/);
expectBaselineHeaders(res);
});
it("demo-mode 403 on a blocked mutation", () => {
process.env.DEMO_MODE = "true";
const res = proxy(
makeRequest("/api/measurements", {
method: "POST",
cookies: { healthlog_session: "sess-1" },
}),
);
expect(res.status).toBe(403);
expectBaselineHeaders(res);
});
it("omits HSTS in development, like the main header block", () => {
process.env.NODE_ENV = "development";
const res = proxy(makeRequest("/stimmung"));
expect(res.status).toBe(301);
expect(res.headers.get("Strict-Transport-Security")).toBeNull();
// The non-transport-dependent headers still attach.
expect(res.headers.get("X-Frame-Options")).toBe("DENY");
});
});
@@ -0,0 +1,88 @@
/**
* Relying-party origin resolution.
*
* The localhost fallback used to sit in the candidate list unconditionally, so
* a production deployment with a real `APP_URL` still accepted an assertion
* whose `clientDataJSON.origin` was `http://localhost:3000`. The signed
* `rpIdHash` pinned the real domain either way, so this never was a reachable
* bypass — but a configured production origin has no reason to carry it, and an
* unpinned second layer is exactly what regresses unnoticed.
*/
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import {
getConfiguredOrigins,
getExpectedOrigin,
getRpId,
} from "../webauthn-rp";
const ORIGINAL = {
NODE_ENV: process.env.NODE_ENV,
APP_URL: process.env.APP_URL,
NEXT_PUBLIC_APP_URL: process.env.NEXT_PUBLIC_APP_URL,
};
function setEnv(env: {
nodeEnv?: string;
appUrl?: string;
publicAppUrl?: string;
}) {
if (env.nodeEnv === undefined) delete process.env.NODE_ENV;
else process.env.NODE_ENV = env.nodeEnv;
if (env.appUrl === undefined) delete process.env.APP_URL;
else process.env.APP_URL = env.appUrl;
if (env.publicAppUrl === undefined) delete process.env.NEXT_PUBLIC_APP_URL;
else process.env.NEXT_PUBLIC_APP_URL = env.publicAppUrl;
}
beforeEach(() => {
setEnv({});
});
afterEach(() => {
setEnv({
nodeEnv: ORIGINAL.NODE_ENV,
appUrl: ORIGINAL.APP_URL,
publicAppUrl: ORIGINAL.NEXT_PUBLIC_APP_URL,
});
});
describe("getConfiguredOrigins", () => {
it("drops the localhost fallback in production once an app URL is configured", () => {
setEnv({ nodeEnv: "production", appUrl: "https://health.example" });
const origins = getConfiguredOrigins();
expect(origins).toEqual(["https://health.example"]);
expect(origins).not.toContain("http://localhost:3000");
});
it("drops it in production when only the public build-time URL is set", () => {
setEnv({ nodeEnv: "production", publicAppUrl: "https://health.example" });
expect(getConfiguredOrigins()).not.toContain("http://localhost:3000");
});
it("keeps the localhost fallback outside production", () => {
setEnv({ nodeEnv: "development", appUrl: "https://health.example" });
expect(getConfiguredOrigins()).toContain("http://localhost:3000");
});
it("keeps it in production when nothing is configured, so getRpId still resolves", () => {
setEnv({ nodeEnv: "production" });
expect(getConfiguredOrigins()).toEqual(["http://localhost:3000"]);
expect(getRpId()).toBe("localhost");
});
it("still pins the RP id to the configured domain in production", () => {
setEnv({ nodeEnv: "production", appUrl: "https://health.example" });
expect(getRpId()).toBe("health.example");
expect(getExpectedOrigin()).toBe("https://health.example");
});
it("de-duplicates when both URL vars carry the same origin", () => {
setEnv({
nodeEnv: "production",
appUrl: "https://health.example",
publicAppUrl: "https://health.example/",
});
expect(getConfiguredOrigins()).toEqual(["https://health.example"]);
});
});
+17 -5
View File
@@ -10,20 +10,32 @@
* build-time URL, then a localhost fallback for dev. A multi-origin deployment
* (rare for a single-tenant self-host, but supported) passes the whole set to
* SimpleWebAuthn's `expectedOrigin`.
*
* The localhost fallback is gated: it is offered in development, and in
* production ONLY when neither app URL is configured — otherwise a production
* deployment accepted an assertion whose `clientDataJSON.origin` was
* `http://localhost:3000`. The signed `rpIdHash` still pins the real domain via
* `getRpId()`, so this was defence-in-depth rather than a live bypass, but a
* configured production origin has no reason to carry localhost at all. The
* unconfigured case keeps the fallback so `getRpId()` still resolves instead of
* throwing on an empty candidate list.
*/
/** Human-readable relying-party name shown by the authenticator UI. */
export const RP_NAME = "HealthLog";
export function getConfiguredOrigins(): string[] {
const candidates = [
process.env.APP_URL,
process.env.NEXT_PUBLIC_APP_URL,
"http://localhost:3000",
]
const configured = [process.env.APP_URL, process.env.NEXT_PUBLIC_APP_URL]
.map((value) => value?.trim())
.filter((value): value is string => Boolean(value));
const allowLocalhost =
process.env.NODE_ENV !== "production" || configured.length === 0;
const candidates = allowLocalhost
? [...configured, "http://localhost:3000"]
: configured;
const validOrigins = candidates
.map((value) => {
try {
@@ -252,3 +252,43 @@ describe("ALLOWED_CHART_TOKENS — drift guard", () => {
expect(ALLOWED_CHART_TOKENS).not.toContain("metric:COMPLIANCE");
});
});
describe("orphan-enum stripping stays coupled to the allowlist", () => {
// `ORPHAN_ENUMS` used to be a hand-maintained literal and drifted every time
// `ALLOWED_CHART_TOKENS` grew — by v1.30 eighteen allowlisted metrics had no
// strip entry, so the raw enum leaked into rendered prose. It is now derived;
// this guard is what keeps it derived.
const PROSE_SAFE = new Set(["WEIGHT", "PULSE", "MOOD", "RESILIENCE"]);
it.each(
ALLOWED_CHART_TOKENS.map((t) => t.replace(/^metric:/, "")).filter(
(name) => !PROSE_SAFE.has(name),
),
)("strips the bare enum %s out of prose", (name) => {
const stripped = stripChartTokens(`Your ${name} is elevated.`);
expect(stripped).not.toContain(name);
});
it.each([...PROSE_SAFE])(
"leaves the prose-safe single word %s alone",
(word) => {
expect(stripChartTokens(`Your ${word} trend is stable.`)).toContain(word);
},
);
// The tokens the drift actually leaked, named so a regression is legible.
it.each([
"PHQ9_SCORE",
"GAD7_SCORE",
"WHO5_SCORE",
"SCI_SCORE",
"GRIP_STRENGTH",
"CARDIO_LOAD",
"ANS_CHARGE",
"BODY_FAT",
"BLOOD_GLUCOSE",
"OXYGEN_SATURATION",
])("strips %s, which the stale literal list had missed", (name) => {
expect(stripChartTokens(`Your ${name} is elevated.`)).not.toContain(name);
});
});
+33 -69
View File
@@ -188,80 +188,44 @@ const METRIC_WORD_REGEX = /\bMetric\s+[A-Za-z][A-Za-z0-9_]*\b/g;
// fixed alternation so we never accidentally cleave a real word
// (e.g. "MOOD") out of legitimate prose unless the model wrote it
// as the raw enum in upper-snake form.
const ORPHAN_ENUMS = [
"BLOOD_PRESSURE_SYS",
"BLOOD_PRESSURE_DIA",
const EXTRA_ORPHAN_ENUMS = [
"PULSE_BPM",
"MOOD_SCORE",
"MEDICATION_COMPLIANCE_PCT",
// v1.4.23 Apple Health additions — see ALLOWED_CHART_TOKENS above.
"HEART_RATE_VARIABILITY",
"RESTING_HEART_RATE",
"ACTIVE_ENERGY_BURNED",
"FLIGHTS_CLIMBED",
"WALKING_RUNNING_DISTANCE",
"VO2_MAX",
"BODY_TEMPERATURE",
"SLEEP_DURATION",
// v1.4.25 W5d Withings additions. Multi-word upper-snake enum names
// never appear in legitimate user-facing prose, so stripping them
// unconditionally is safe.
"FAT_FREE_MASS",
"FAT_MASS",
"MUSCLE_MASS",
"SKIN_TEMPERATURE",
"PULSE_WAVE_VELOCITY",
"VASCULAR_AGE",
"VISCERAL_FAT",
// v1.4.25 W8d Apple Health server-prep — multi-word upper-snake
// names that should never surface verbatim in user-facing prose.
"AUDIO_EXPOSURE_ENV",
"AUDIO_EXPOSURE_HEADPHONE",
"TIME_IN_DAYLIGHT",
// v1.5.5 iOS-coord additions — same shape, same posture.
"RESPIRATORY_RATE",
"BODY_MASS_INDEX",
"LEAN_BODY_MASS",
"WALKING_HEART_RATE_AVERAGE",
"WALKING_ASYMMETRY",
"WALKING_DOUBLE_SUPPORT",
// v1.5.5 iOS-coord follow-up — raw-SI gait pair, same shape.
"WALKING_STEP_LENGTH",
"WALKING_SPEED",
// v1.10.0 — additive HealthKit signals (WX-A), same shape.
"CARDIO_RECOVERY",
"WRIST_TEMPERATURE",
"FALL_COUNT",
"SIX_MINUTE_WALK_DISTANCE",
"STAIR_ASCENT_SPEED",
"STAIR_DESCENT_SPEED",
"BREATHING_DISTURBANCES",
// v1.10.0 — computed scores (WX-C), same shape. Strip the bare enum name
// if the model drops it into prose (mirrors the MOOD_SCORE precedent).
"RECOVERY_SCORE",
"STRESS_SCORE",
"STRAIN_SCORE",
// v1.11.0 — WHOOP-native score classes, same shape: strip the bare enum
// name if the model drops it into prose.
"HRV_RMSSD",
"DAY_STRAIN",
"WORKOUT_STRAIN",
"SLEEP_PERFORMANCE",
"SLEEP_EFFICIENCY",
"SLEEP_CONSISTENCY",
"SLEEP_NEED",
"ENERGY_EXPENDITURE_KJ",
// v1.12.8 — WHOOP cycle + sleep coverage completion, same shape: strip the
// bare enum name if the model drops it into prose.
"AVERAGE_HEART_RATE",
"MAX_HEART_RATE",
"SLEEP_DISTURBANCE_COUNT",
// v1.17.1 — Oura coverage completion, same shape: strip the bare enum name
// if the model drops it into prose.
"SLEEP_SCORE",
"BODY_TEMPERATURE_DEVIATION",
] as const;
/**
* Single words shared with the user-facing metric label. "Your WEIGHT trend is
* stable" is legitimate prose, so these stay out of the stripper. Every entry
* is a single word by construction — an upper-snake name is never prose.
*/
const PROSE_SAFE_SINGLE_WORDS = new Set<string>([
"WEIGHT",
"PULSE",
"MOOD",
"RESILIENCE",
]);
/**
* Bare enum names cleaved out of prose, DERIVED from `ALLOWED_CHART_TOKENS`
* rather than hand-maintained.
*
* The list used to be a literal array and drifted every time the allowlist
* grew: by v1.30 eighteen allowlisted metrics had no strip entry, so a model
* writing "Your PHQ9_SCORE is elevated" leaked the raw enum straight into
* rendered text — the exact class this list exists to catch. Deriving it makes
* that drift impossible.
*/
const ORPHAN_ENUMS: readonly string[] = [
...ALLOWED_CHART_TOKENS.map((token) => token.replace(/^metric:/, "")).filter(
(name) => !PROSE_SAFE_SINGLE_WORDS.has(name),
),
...EXTRA_ORPHAN_ENUMS,
]
// Longest first so the alternation never matches a shorter enum that is a
// prefix of a longer one.
.sort((a, b) => b.length - a.length);
// `\b` boundaries keep ordinary English prose untouched — "weight"
// (lowercase) or "BMI" (no underscore) never match. The fixed list
// also means we never accidentally strip a single capitalised word
+67 -19
View File
@@ -188,6 +188,40 @@ function generateNonce(): string {
return btoa(binary);
}
/**
* Transport-level security headers every response carries, including the early
* exits (worker-only 503, legacy 301s, demo-mode 403, and the login /
* onboarding / MFA redirects) that return before the full header block below.
*
* Those exits used to answer with no HSTS, no framing refusal and no nosniff at
* all. The gap was one hop — a redirect has no rendered body and the follow-up
* navigation re-enters this function and does get the full set — but "the next
* request will be protected" is not a reason for this one not to be, and HSTS
* in particular is worth the most on the first hop over a hostile network.
*
* Deliberately the transport subset, not the whole block: the per-route CSP
* below is nonce-bound and carries carve-outs for the document-serve and share
* routes that only make sense for a response with a body to protect.
*/
function applyBaselineSecurityHeaders(response: NextResponse): NextResponse {
response.headers.set("X-Content-Type-Options", "nosniff");
response.headers.set("X-Frame-Options", "DENY");
response.headers.set("Referrer-Policy", "strict-origin-when-cross-origin");
response.headers.set(
"Permissions-Policy",
"camera=(), microphone=(), geolocation=()",
);
response.headers.set("Cross-Origin-Opener-Policy", "same-origin");
response.headers.set("X-Permitted-Cross-Domain-Policies", "none");
if (process.env.NODE_ENV !== "development") {
response.headers.set(
"Strict-Transport-Security",
"max-age=31536000; includeSubDomains; preload",
);
}
return response;
}
export function proxy(request: NextRequest) {
const { pathname } = request.nextUrl;
@@ -195,26 +229,32 @@ export function proxy(request: NextRequest) {
// serving requests that would either crash (no DB pool ready for writes from
// the worker) or duplicate work that the dedicated web container is doing.
if (!shouldRunWeb()) {
return NextResponse.json(
{
data: null,
error:
"This container runs the worker only — point HTTP at the web service.",
},
{ status: 503, headers: { "X-HealthLog-Process-Type": "worker" } },
return applyBaselineSecurityHeaders(
NextResponse.json(
{
data: null,
error:
"This container runs the worker only — point HTTP at the web service.",
},
{ status: 503, headers: { "X-HealthLog-Process-Type": "worker" } },
),
);
}
// 301 redirects for renamed routes
const redirect = LEGACY_REDIRECTS[pathname];
if (redirect) {
return NextResponse.redirect(new URL(redirect, request.url), 301);
return applyBaselineSecurityHeaders(
NextResponse.redirect(new URL(redirect, request.url), 301),
);
}
// 301 for the v1.5 admin section-anchor → dynamic-route migration.
const legacyAdmin = LEGACY_ADMIN_ANCHORS[pathname];
if (legacyAdmin) {
return NextResponse.redirect(new URL(legacyAdmin, request.url), 301);
return applyBaselineSecurityHeaders(
NextResponse.redirect(new URL(legacyAdmin, request.url), 301),
);
}
// Demo mode: block all mutations except login
@@ -230,13 +270,15 @@ export function proxy(request: NextRequest) {
(entry) => pathname === entry.path && method === entry.method,
)
) {
return NextResponse.json(
{
data: null,
error: "Demo mode: modifications are disabled",
meta: { demo: true },
},
{ status: 403 },
return applyBaselineSecurityHeaders(
NextResponse.json(
{
data: null,
error: "Demo mode: modifications are disabled",
meta: { demo: true },
},
{ status: 403 },
),
);
}
}
@@ -252,7 +294,9 @@ export function proxy(request: NextRequest) {
if (isSessionGatedPage) {
const hasSession = request.cookies.has("healthlog_session");
if (!hasSession) {
return NextResponse.redirect(new URL("/auth/login", request.url));
return applyBaselineSecurityHeaders(
NextResponse.redirect(new URL("/auth/login", request.url)),
);
}
// v1.4.22 C4 — server-side onboarding redirect. Previously the
@@ -272,7 +316,9 @@ export function proxy(request: NextRequest) {
const isOnboardingSurface =
pathname === "/onboarding" || pathname.startsWith("/onboarding/");
if (onboardingPending && !isOnboardingSurface) {
return NextResponse.redirect(new URL("/onboarding", request.url));
return applyBaselineSecurityHeaders(
NextResponse.redirect(new URL("/onboarding", request.url)),
);
}
// v1.23 — admin-enforced MFA forced-enrollment gate. When the operator
@@ -289,7 +335,9 @@ export function proxy(request: NextRequest) {
const isMfaEnrollSurface =
pathname === "/enroll-mfa" || pathname.startsWith("/settings/security");
if (mfaEnrollRequired && !isMfaEnrollSurface && !isOnboardingSurface) {
return NextResponse.redirect(new URL("/enroll-mfa", request.url));
return applyBaselineSecurityHeaders(
NextResponse.redirect(new URL("/enroll-mfa", request.url)),
);
}
}