mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 15:02:04 +00:00
feat(config): let the operator turn off the breach and update checks
Two requests left the server without anything being connected: the HaveIBeenPwned range query on every password set (registration, change, admin reset) and the daily GitHub releases request behind the About page and the admin version tile. Neither could be turned off, which made "nothing leaves" untrue for an instance that wanted it to be. PASSWORD_BREACH_CHECK_DISABLED and UPDATE_CHECK_DISABLED (1, true, yes or on) stop each request. Both stay on by default. The breach check keeps its fail-open contract: switched off, a password is simply not looked up. The update route answers status "unknown" with reason "disabled" and the admin tile says the check is off. The password routes reach the check only through the switch, and a test holds that import line. Both are on the compose whitelist, in the env manifest under a new "Outbound requests" group with IP_GEO_LOOKUP_DISABLED, and documented in both env examples and the getting-started guide.
This commit is contained in:
1 parent
228c5c9da6
commit
afcb37e76c
21 files changed
+248
-10
No files matched your search
@@ -0,0 +1,66 @@
|
||||
/**
|
||||
* The HaveIBeenPwned check sits behind one operator switch, and nothing
|
||||
* reaches it except through that switch.
|
||||
*
|
||||
* `PASSWORD_BREACH_CHECK_DISABLED` is how an operator stops the one request
|
||||
* a password change makes to a third party. A route that imported the raw
|
||||
* check from `@/lib/auth/hibp` would keep sending it with the switch on, so
|
||||
* the import itself is frozen: only `src/lib/password-breach-check.ts` (and
|
||||
* tests) may name that module.
|
||||
*/
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { walkSourceFiles } from "./helpers/source-files";
|
||||
|
||||
const checkPasswordBreach = vi.fn();
|
||||
vi.mock("@/lib/auth/hibp", () => ({
|
||||
checkPasswordBreach: (password: string) => checkPasswordBreach(password),
|
||||
}));
|
||||
|
||||
const SRC = join(process.cwd(), "src");
|
||||
const RAW_IMPORT = /from\s+["']@\/lib\/auth\/hibp["']|["']\.\/hibp["']/;
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
checkPasswordBreach.mockReset();
|
||||
});
|
||||
|
||||
describe("password breach check switch", () => {
|
||||
it("calls the range API when the switch is unset or empty", async () => {
|
||||
checkPasswordBreach.mockResolvedValue({ breached: true, count: 3 });
|
||||
const { checkPasswordBreachIfEnabled } =
|
||||
await import("@/lib/password-breach-check");
|
||||
vi.stubEnv("PASSWORD_BREACH_CHECK_DISABLED", "");
|
||||
await expect(
|
||||
checkPasswordBreachIfEnabled("correct horse battery"),
|
||||
).resolves.toEqual({ breached: true, count: 3 });
|
||||
expect(checkPasswordBreach).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it.each(["1", "true", "yes"])(
|
||||
"makes no request when PASSWORD_BREACH_CHECK_DISABLED=%s",
|
||||
async (value) => {
|
||||
vi.stubEnv("PASSWORD_BREACH_CHECK_DISABLED", value);
|
||||
const { checkPasswordBreachIfEnabled } =
|
||||
await import("@/lib/password-breach-check");
|
||||
await expect(
|
||||
checkPasswordBreachIfEnabled("correct horse battery"),
|
||||
).resolves.toBeNull();
|
||||
expect(checkPasswordBreach).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it("is the only non-test importer of the raw check", () => {
|
||||
const importers = walkSourceFiles(SRC, { floor: 3000 })
|
||||
.filter((rel) => !rel.startsWith("generated/"))
|
||||
.filter((rel) => !rel.includes("__tests__"))
|
||||
.filter((rel) => !rel.endsWith(".test.ts"))
|
||||
.filter((rel) => rel !== "lib/auth/hibp.ts")
|
||||
.filter((rel) => RAW_IMPORT.test(readFileSync(join(SRC, rel), "utf8")));
|
||||
// Non-empty by construction: the wrapper itself must match, or the
|
||||
// matcher went stale and this assertion would pass on nothing.
|
||||
expect(importers).toEqual(["lib/password-breach-check.ts"]);
|
||||
});
|
||||
});
|
||||
@@ -14,7 +14,7 @@ import { destroyAllSessions } from "@/lib/auth/session";
|
||||
import { revokeStepUpElevations } from "@/lib/auth/step-up";
|
||||
import { NextRequest, NextResponse } from "next/server";
|
||||
import { resolveServerLocale } from "@/lib/i18n/server-locale";
|
||||
import { checkPasswordBreach } from "@/lib/auth/hibp";
|
||||
import { checkPasswordBreachIfEnabled } from "@/lib/password-breach-check";
|
||||
import { getServerTranslator } from "@/lib/i18n/server-translator";
|
||||
|
||||
export const POST = apiHandler(
|
||||
@@ -70,7 +70,7 @@ export const POST = apiHandler(
|
||||
|
||||
// v1.23 — reject an admin-set password found in a known breach corpus
|
||||
// (HIBP k-anonymity). Fail-open on an unreachable HIBP.
|
||||
const breach = await checkPasswordBreach(password);
|
||||
const breach = await checkPasswordBreachIfEnabled(password);
|
||||
if (breach?.breached) {
|
||||
return apiError(
|
||||
getServerTranslator(locale).t("auth.passwordBreached"),
|
||||
|
||||
@@ -24,7 +24,7 @@ import { annotate } from "@/lib/logging/context";
|
||||
import { destroyAllSessions, createSession } from "@/lib/auth/session";
|
||||
import { revokeStepUpElevations } from "@/lib/auth/step-up";
|
||||
import { resolveServerLocale } from "@/lib/i18n/server-locale";
|
||||
import { checkPasswordBreach } from "@/lib/auth/hibp";
|
||||
import { checkPasswordBreachIfEnabled } from "@/lib/password-breach-check";
|
||||
import { getServerTranslator } from "@/lib/i18n/server-translator";
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
@@ -89,7 +89,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
// corpus (HIBP k-anonymity). Fail-open: a null result (HIBP unreachable)
|
||||
// never blocks the change. Only the user's chosen NEW password is checked —
|
||||
// existing credentials are never retroactively blocked at login.
|
||||
const breach = await checkPasswordBreach(newPassword);
|
||||
const breach = await checkPasswordBreachIfEnabled(newPassword);
|
||||
if (breach?.breached) {
|
||||
return apiError(
|
||||
getServerTranslator(locale).t("auth.passwordBreached"),
|
||||
|
||||
@@ -21,7 +21,7 @@ import { NextRequest, NextResponse } from "next/server";
|
||||
import { apiHandler } from "@/lib/api-handler";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { resolveServerLocale } from "@/lib/i18n/server-locale";
|
||||
import { checkPasswordBreach } from "@/lib/auth/hibp";
|
||||
import { checkPasswordBreachIfEnabled } from "@/lib/password-breach-check";
|
||||
import { getServerTranslator } from "@/lib/i18n/server-translator";
|
||||
import {
|
||||
isValidTimezone,
|
||||
@@ -173,7 +173,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
|
||||
// v1.23 — reject a registration password found in a known breach corpus
|
||||
// (HIBP k-anonymity). Fail-open on an unreachable HIBP.
|
||||
const breach = await checkPasswordBreach(password);
|
||||
const breach = await checkPasswordBreachIfEnabled(password);
|
||||
if (breach?.breached) {
|
||||
return apiError(
|
||||
getServerTranslator(locale).t("auth.passwordBreached"),
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
|
||||
requireAuth: vi.fn().mockResolvedValue({ user: { id: "u1" } }),
|
||||
}));
|
||||
|
||||
const safeFetchMock = vi.fn();
|
||||
vi.mock("@/lib/safe-fetch", () => ({
|
||||
safeFetch: (...args: unknown[]) => safeFetchMock(...args),
|
||||
}));
|
||||
|
||||
import { GET } from "../route";
|
||||
|
||||
const call = () => (GET as unknown as () => Promise<Response>)();
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
safeFetchMock.mockReset();
|
||||
});
|
||||
|
||||
describe("GET /api/version/check-updates", () => {
|
||||
it.each(["1", "true", "yes"])(
|
||||
"makes no outbound request when UPDATE_CHECK_DISABLED=%s",
|
||||
async (value) => {
|
||||
vi.stubEnv("UPDATE_CHECK_DISABLED", value);
|
||||
const body = (await (await call()).json()) as {
|
||||
data: { status: string; reason: string };
|
||||
};
|
||||
expect(body.data.status).toBe("unknown");
|
||||
expect(body.data.reason).toBe("disabled");
|
||||
expect(safeFetchMock).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it("asks the release feed when the switch is unset or empty", async () => {
|
||||
vi.stubEnv("UPDATE_CHECK_DISABLED", "");
|
||||
safeFetchMock.mockResolvedValue(
|
||||
Response.json({ tag_name: "v999.0.0", html_url: "https://x.test" }),
|
||||
);
|
||||
const body = (await (await call()).json()) as {
|
||||
data: { status: string };
|
||||
};
|
||||
expect(safeFetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(body.data.status).toBe("newer_available");
|
||||
});
|
||||
});
|
||||
@@ -1,5 +1,6 @@
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import { apiError, apiSuccess } from "@/lib/api-response";
|
||||
import { envFlag } from "@/lib/env";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { getEvent } from "@/lib/logging/context";
|
||||
import { safeFetch } from "@/lib/safe-fetch";
|
||||
@@ -27,6 +28,12 @@ export const dynamic = "force-dynamic";
|
||||
* repo missing the release manifest); UI surfaces a non-blocking
|
||||
* warning so the user can retry
|
||||
*
|
||||
* `UPDATE_CHECK_DISABLED` (1, true, yes or on) stops the request to GitHub
|
||||
* entirely: the route answers `unknown` with `reason: "disabled"` and makes
|
||||
* no outbound call. The About page and the admin version tile run this check
|
||||
* on their own once a day, so an instance that promises no third-party
|
||||
* requests needs the switch.
|
||||
*
|
||||
* Responses are not cached at the route level. The browser bears the
|
||||
* load with React Query's `staleTime`, so a single user spamming
|
||||
* "Check now" still hits the GH API at most every 30 seconds.
|
||||
@@ -78,6 +85,15 @@ export const GET = apiHandler(async () => {
|
||||
|
||||
const current = packageJson.version;
|
||||
|
||||
if (envFlag("UPDATE_CHECK_DISABLED")) {
|
||||
annotate({ meta: { outcome: "disabled" } });
|
||||
return apiSuccess({
|
||||
status: "unknown" as const,
|
||||
current,
|
||||
reason: "disabled",
|
||||
});
|
||||
}
|
||||
|
||||
let release: GithubReleaseShape | null = null;
|
||||
const callStart = Date.now();
|
||||
try {
|
||||
|
||||
@@ -80,6 +80,9 @@ export function VersionTileSection() {
|
||||
|
||||
const newer = update?.status === "newer_available";
|
||||
const upToDate = update?.status === "up_to_date";
|
||||
// The operator turned the GitHub request off (UPDATE_CHECK_DISABLED); say
|
||||
// so rather than leaving the slot empty like an unreachable feed.
|
||||
const checkOff = update?.status === "unknown" && update.reason === "disabled";
|
||||
const sha = shortSha(version?.buildSha ?? null);
|
||||
|
||||
return (
|
||||
@@ -110,6 +113,10 @@ export function VersionTileSection() {
|
||||
<CheckCircle2 className="h-3.5 w-3.5" aria-hidden="true" />
|
||||
{t("admin.overview.versionTileUpToDate")}
|
||||
</span>
|
||||
) : checkOff ? (
|
||||
<span className="text-muted-foreground text-xs">
|
||||
{t("admin.overview.versionTileCheckOff")}
|
||||
</span>
|
||||
) : null
|
||||
}
|
||||
/>
|
||||
|
||||
@@ -428,7 +428,8 @@ export const metaPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
description:
|
||||
"A server-side proxy for the project's public release feed. It exists because the production CSP allows almost nothing on `connect-src`, so a browser calling the feed directly is silently blocked — which is exactly how the first version of the “check for updates” button appeared to do nothing.\n\n" +
|
||||
"Authenticated (cookie or a wildcard Bearer token): there is no reason an anonymous caller should poll a third-party API through this instance. No credential is forwarded to the feed, so the request runs under the host's anonymous quota — roughly sixty an hour per address, shared by everyone on the instance.\n\n" +
|
||||
'It is deliberately hard to fail. A network error, an outage, or a rate-limited feed all return 200 with `status: "unknown"` and a machine-readable `reason`, so the UI can offer a retry instead of a red banner nobody can act on. The one genuine error is a feed that answers successfully with no version tag in it. Nothing is cached server-side; the client is expected to hold the result.',
|
||||
'It is deliberately hard to fail. A network error, an outage, or a rate-limited feed all return 200 with `status: "unknown"` and a machine-readable `reason`, so the UI can offer a retry instead of a red banner nobody can act on. The one genuine error is a feed that answers successfully with no version tag in it. Nothing is cached server-side; the client is expected to hold the result.\n\n' +
|
||||
'An operator who sets `UPDATE_CHECK_DISABLED` turns the outbound request off: the route then answers `status: "unknown"` with `reason: "disabled"` and contacts nobody.',
|
||||
responses: {
|
||||
"200": {
|
||||
description:
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
/**
|
||||
* The operator switch in front of the HaveIBeenPwned password check.
|
||||
*
|
||||
* `checkPasswordBreach()` (src/lib/auth/hibp.ts) sends the first five hex
|
||||
* characters of the password's SHA-1 to api.pwnedpasswords.com whenever a
|
||||
* password is set: registration, a password change, and an admin reset. The
|
||||
* range API is k-anonymous, so neither the password nor its full hash leaves
|
||||
* the server, but the request itself does, and an instance that promises no
|
||||
* third-party calls needs a way to stop it. `PASSWORD_BREACH_CHECK_DISABLED`
|
||||
* (1, true, yes or on) turns the check off; the result is then "unknown",
|
||||
* which every caller already treats as fail-open.
|
||||
*
|
||||
* Callers import this module, never `@/lib/auth/hibp` directly;
|
||||
* `src/__tests__/password-breach-check-switch.test.ts` holds that line.
|
||||
*/
|
||||
import { checkPasswordBreach, type BreachCheckResult } from "@/lib/auth/hibp";
|
||||
import { envFlag } from "@/lib/env";
|
||||
|
||||
export function passwordBreachCheckEnabled(): boolean {
|
||||
return !envFlag("PASSWORD_BREACH_CHECK_DISABLED");
|
||||
}
|
||||
|
||||
/** The breach status of `password`, or `null` when disabled or unknown. */
|
||||
export async function checkPasswordBreachIfEnabled(
|
||||
password: string,
|
||||
): Promise<BreachCheckResult | null> {
|
||||
if (!passwordBreachCheckEnabled()) return null;
|
||||
return checkPasswordBreach(password);
|
||||
}
|
||||
Reference in new issue
Block a user