mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(nightscout): assert the public-host floor before persisting the URL
Egress was already correct — the client fetches through safeFetch with requirePublicHost pinned off the user's opt-in, manual redirects and a timeout — but the check on the value we STORE was only a side effect of the live probe the connect route happens to run first. That makes the guard contingent on the probe's placement rather than on the input. Run isPublicUrl at input time, gated on the same self-hoster opt-in, so the URL that gets encrypted onto the row is the URL that was validated. Cover the alt-notation classes the helper knows about; each case asserts the probe was never reached, which is what distinguishes the input-time refusal from the fetch-time one.
This commit is contained in:
1 parent
0fca5ca207
commit
fd6a80a921
3 files changed
+88
-5
No files matched your search
@@ -140,6 +140,63 @@ describe("POST /api/nightscout/connect", () => {
|
||||
expect(userUpdate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Input-time SSRF floor. Every case below leaves the default happy-path
|
||||
// `fetchSgvEntries` mock in place — it RESOLVES — so the only thing that
|
||||
// can produce a 422 is the check that runs before the probe. Asserting the
|
||||
// probe was never called is what separates this from the fetch-time guard:
|
||||
// a URL that never reaches the egress layer cannot have been refused by it.
|
||||
describe("refuses private / non-public hosts before the probe runs", () => {
|
||||
const cases: Array<[string, string]> = [
|
||||
["loopback", "http://127.0.0.1:1337"],
|
||||
["loopback by name", "http://localhost:1337"],
|
||||
["link-local", "http://169.254.1.1"],
|
||||
["cloud metadata endpoint", "http://169.254.169.254/latest/meta-data/"],
|
||||
["RFC1918 class A", "http://10.0.0.5"],
|
||||
["RFC1918 class C", "http://192.168.1.5"],
|
||||
["CGNAT", "http://100.64.0.1"],
|
||||
["IPv4-mapped IPv6 loopback", "http://[::ffff:127.0.0.1]"],
|
||||
["IPv6 loopback", "http://[::1]:1337"],
|
||||
["IPv6 unique-local", "http://[fd00::1]"],
|
||||
["octal IPv4 loopback", "http://0177.0.0.1"],
|
||||
["hex IPv4 loopback", "http://0x7f000001"],
|
||||
["decimal IPv4 loopback", "http://2130706433"],
|
||||
[".internal suffix", "http://cgm.internal"],
|
||||
[".local suffix", "http://cgm.local"],
|
||||
];
|
||||
|
||||
it.each(cases)("refuses %s", async (_label, url) => {
|
||||
const res = await post(req({ url, token: "tok" }));
|
||||
|
||||
expect(res.status).toBe(422);
|
||||
expect(String(res.error)).toMatch(/private network/i);
|
||||
expect(fetchSgvEntriesMock).not.toHaveBeenCalled();
|
||||
expect(userUpdate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still lets the explicit self-hoster opt-in through to the probe", async () => {
|
||||
// The floor is gated, not absolute: a LAN instance stays reachable for
|
||||
// the operator who asked for it. Without this the opt-in is dead code.
|
||||
const res = await post(
|
||||
req({
|
||||
url: "http://192.168.1.5:1337",
|
||||
token: "tok",
|
||||
allowPrivateHost: true,
|
||||
}),
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(fetchSgvEntriesMock).toHaveBeenCalledTimes(1);
|
||||
expect(userUpdate).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("lets an ordinary public instance through", async () => {
|
||||
const res = await post(req({ url: "https://ns.example.com" }));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(fetchSgvEntriesMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("rate-limits the connect surface", async () => {
|
||||
rateLimitMock.mockResolvedValue({
|
||||
allowed: false,
|
||||
|
||||
@@ -12,6 +12,7 @@ import { markReconnected } from "@/lib/integrations/status";
|
||||
import { SafeFetchError } from "@/lib/safe-fetch";
|
||||
import { fetchSgvEntries, NightscoutApiError } from "@/lib/nightscout/client";
|
||||
import { nightscoutConnectSchema } from "@/lib/validations/nightscout";
|
||||
import { isPublicUrl } from "@/lib/validations/notifications";
|
||||
|
||||
/**
|
||||
* Connect (or update) the user's Nightscout instance (v1.17.0).
|
||||
@@ -49,6 +50,25 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
|
||||
const { url, token, allowPrivateHost } = result.data;
|
||||
|
||||
// SSRF floor at INPUT time, not merely as a side effect of the live probe
|
||||
// below. `fetchSgvEntries` does route through `safeFetch` with the same
|
||||
// public-host pin, so today both paths refuse the same URLs — but that
|
||||
// makes the guard on the value we PERSIST contingent on the probe staying
|
||||
// exactly where it is. Reorder it, make it best-effort, or add a "save
|
||||
// without testing" affordance and the stored URL is suddenly unchecked.
|
||||
// Assert it here so the thing that gets encrypted onto the row is the
|
||||
// thing that was validated.
|
||||
//
|
||||
// Gated on the same explicit opt-in the fetch path uses: a self-hoster who
|
||||
// deliberately points at a LAN instance has asked for the private host and
|
||||
// must still be able to connect.
|
||||
if (!allowPrivateHost && !isPublicUrl(url)) {
|
||||
return apiError(
|
||||
"That instance is on a private network. Enable the private-network option to connect to it.",
|
||||
422,
|
||||
);
|
||||
}
|
||||
|
||||
// Live validation: pull a single SGV entry. A connection that can't even
|
||||
// fetch one row is not worth storing.
|
||||
try {
|
||||
|
||||
@@ -7,11 +7,17 @@ import { z } from "zod/v4";
|
||||
* on `User` (`nightscoutUrlEncrypted` / `nightscoutTokenEncrypted`); the
|
||||
* private-host opt-in maps to `nightscoutAllowPrivateHost`.
|
||||
*
|
||||
* The URL is validated to be a parseable http(s) origin at input time. The
|
||||
* SSRF floor (public-host requirement, unless `allowPrivateHost` is set) is
|
||||
* enforced at fetch time by the client through `safeFetch` — input-shape
|
||||
* validation here only rejects garbage early so the connect route can return
|
||||
* a clear 422 instead of letting a malformed string reach the egress layer.
|
||||
* The URL is validated to be a parseable http(s) origin here. The SSRF floor
|
||||
* (public-host requirement, unless `allowPrivateHost` is set) is asserted
|
||||
* twice on purpose: the connect route runs `isPublicUrl` before it persists
|
||||
* anything, and `safeFetch` re-checks plus pins the resolved IP at connect
|
||||
* time. The shape check here only rejects garbage early so the route can
|
||||
* return a clear 422 instead of letting a malformed string reach either.
|
||||
*
|
||||
* The floor is NOT expressed as a Zod refinement because it depends on the
|
||||
* sibling `allowPrivateHost` field and needs its own error message; folding
|
||||
* it in here would collapse "malformed URL" and "private host, opt in to
|
||||
* reach it" into one opaque 422.
|
||||
*/
|
||||
export const nightscoutConnectSchema = z.object({
|
||||
// Trimmed: a trailing space / newline from a copy button reaches the
|
||||
|
||||
Reference in new issue
Block a user