From 64f63f36942bdb1a7349d66450bd62236282811c Mon Sep 17 00:00:00 2001 From: StrandedTurtle Date: Wed, 1 Jul 2026 22:58:29 +0100 Subject: [PATCH] Harden auth limiter, fix logout cookie path, document Cloudflare Tunnel setup - Bound the in-memory login-attempt map (sweep expired entries past 10k IPs) so a wide scan can't grow it forever now that internet exposure via a tunnel is a documented deployment. - Clear the session cookie with the same path it was set with, so logout works when the app is served under BASE_PATH. - Fix the stale urlguard.js header: the strict SSRF guards are kept but unused; the active policy (LAN targets allowed, admin-only) matches SECURITY.md. - README/SECURITY.md: Cloudflare Tunnel guidance (Cloudflare Access in front, TRUST_PROXY=1, https BASE_URL) and a README typo fix. Co-Authored-By: Claude Fable 5 --- README.md | 9 ++++++++- SECURITY.md | 5 +++++ server/src/auth.js | 16 +++++++++++++++- server/src/urlguard.js | 21 ++++++++++++--------- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 243d3a8..453439a 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,13 @@ To update DockPull itself: `docker compose pull dockpull && docker compose up -d `BASE_PATH=/dockpull` and build the client with the same value (`BASE_PATH=/dockpull docker compose build`). +**Behind a Cloudflare Tunnel?** A tunnel makes DockPull reachable from the +public internet, and DockPull controls your Docker socket — the built-in +password alone is not enough there. Put a [Cloudflare Access](https://developers.cloudflare.com/cloudflare-one/policies/access/) +policy (Zero Trust login) in front of the hostname, and set `TRUST_PROXY=1` +and `BASE_URL=https://your-hostname` so rate-limiting sees real client IPs and +the session cookie is marked `Secure`. + --- ## Troubleshooting @@ -157,6 +164,6 @@ To update DockPull itself: `docker compose pull dockpull && docker compose up -d public or `docker login ghcr.io`. --- -DISCLAIMER: This porject was made using Claude +DISCLAIMER: This project was made using Claude Endpoint/field reference: [`API_CONTRACT.md`](./API_CONTRACT.md) · Development setup: [`CONTRIBUTING.md`](./CONTRIBUTING.md) · License: MIT. diff --git a/SECURITY.md b/SECURITY.md index c2e4efc..4072b9a 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -35,6 +35,11 @@ DockPull is built for a **trusted LAN / homelab** behind authentication. It is - **Keep it off the open internet.** Put it on your LAN, a VPN (WireGuard / Tailscale), or behind an authenticating reverse proxy / tunnel. +- **Using a Cloudflare Tunnel?** That *is* internet exposure. Require a + [Cloudflare Access](https://developers.cloudflare.com/cloudflare-one/policies/access/) + policy on the hostname (so visitors authenticate to Cloudflare before they + ever reach DockPull), and set `TRUST_PROXY=1` + `BASE_URL=https://…`. Don't + rely on the single app password as the only gate to a Docker-socket app. - **Use a strong `ADMIN_PASSWORD`** and a random `SESSION_SECRET` (`openssl rand -hex 32`). - **Serve it over HTTPS** (set `BASE_URL=https://…`) so the session cookie gets diff --git a/server/src/auth.js b/server/src/auth.js index 3c81e57..d4870b7 100644 --- a/server/src/auth.js +++ b/server/src/auth.js @@ -27,12 +27,25 @@ const LOCKOUT_MS = 15 * 60 * 1000; // how long a lockout lasts once tripped const loginAttempts = new Map(); // ip -> { count, firstAt, lockedUntil } +// Bound the map: once it grows past this, expired entries are swept on the +// next recorded failure, so a wide scan from many IPs can't grow it forever. +const MAX_TRACKED_IPS = 10000; + +function pruneLoginAttempts(now) { + for (const [ip, a] of loginAttempts) { + const windowExpired = now - a.firstAt > FAILURE_WINDOW_MS; + const lockoutExpired = !a.lockedUntil || a.lockedUntil <= now; + if (windowExpired && lockoutExpired) loginAttempts.delete(ip); + } +} + export function isLockedOut(ip, now = Date.now()) { const a = loginAttempts.get(ip); return Boolean(a && a.lockedUntil && a.lockedUntil > now); } export function recordLoginFailure(ip, now = Date.now()) { + if (loginAttempts.size >= MAX_TRACKED_IPS) pruneLoginAttempts(now); let a = loginAttempts.get(ip); if (!a || now - a.firstAt > FAILURE_WINDOW_MS) { a = { count: 0, firstAt: now, lockedUntil: 0 }; @@ -127,7 +140,8 @@ export function loginHandler(req, res) { * cookie that may not even be valid is harmless. */ export function logoutHandler(req, res) { - res.clearCookie(SESSION_COOKIE, { path: '/' }); + // Must match the path the cookie was set with, or the browser won't clear it. + res.clearCookie(SESSION_COOKIE, { path: config.BASE_PATH || '/' }); return res.status(200).json({ ok: true }); } diff --git a/server/src/urlguard.js b/server/src/urlguard.js index c9d5bdf..ab572a4 100644 --- a/server/src/urlguard.js +++ b/server/src/urlguard.js @@ -1,18 +1,21 @@ /** - * SSRF guards for the user-supplied Discord webhook URL. + * URL validation for the user-supplied notification target. * - * The webhook URL is fetched server-side (scheduled notify + "send test"), so - * an unrestricted URL would let an authenticated user probe the host's internal - * network or cloud metadata endpoints. We defend in two layers: + * The active check is `isValidNotifyUrl`: a well-formed http(s) URL, with + * private/LAN hosts deliberately allowed — a self-hosted ntfy/Gotify on your + * network is a normal target, and the field is admin-only (behind login). + * See SECURITY.md ("Notification URL is admin-only"). + * + * Two stricter guards are kept (tested, currently unused) in case a future + * feature fetches URLs that should never reach internal hosts: * * - `isSafeWebhookUrl(url)` — synchronous, cheap. Requires https and rejects * URLs whose host is a literal loopback/private/link-local/reserved IP or an - * obviously-internal name (localhost / *.local). Used when validating input - * before storing it and in the test endpoint. + * obviously-internal name (localhost / *.local). * - `assertPublicWebhookUrl(url)` — async. Does the sync checks, then resolves - * the hostname via DNS and rejects if *any* resolved address is private. - * This closes the gap where a public hostname points at an internal IP (or a - * DNS-rebind). Called right before the network request in notify.js. + * the hostname via DNS and rejects if *any* resolved address is private, + * closing the gap where a public hostname points at an internal IP (or a + * DNS-rebind). */ import dns from 'node:dns/promises';