From 3b017b91c90af3312935c03e6b94558e9e517ae1 Mon Sep 17 00:00:00 2001 From: strandedturtle Date: Tue, 23 Jun 2026 11:06:57 +0000 Subject: [PATCH] harden: self-exclusion, login rate-limit, SSE keepalive, graceful shutdown Review-pass hardening for shipping to other users: - docker.js: exclude the app's own container from the dashboard (best-effort via SELF_CONTAINER_NAME + hostname match) so it can't be told to update and thereby kill itself mid-update. - auth.js: per-client-IP failed-login rate limiting with a temporary lockout (returns 429 too_many_attempts); covered by new unit tests (52 total). - sse.js: 15s keepalive comments + X-Accel-Buffering:no so reverse proxies don't drop the log stream during long, sparse pulls; clear keepalive on finish/disconnect. - index.js: graceful SIGTERM/SIGINT shutdown (close server + SQLite), and disable x-powered-by. - Dockerfile: HEALTHCHECK against /api/health. - client: useUpdateRunner settles exactly once (a late stream-close error no longer overwrites a success); accurate empty-state copy; iOS PWA meta tags. - docs: LICENSE (MIT), env/contract/README updates for the above. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_013Lj6nYJQDtLaZFvvEQJGM4 --- .env.example | 7 +++++ API_CONTRACT.md | 2 ++ LICENSE | 21 ++++++++++++++ README.md | 11 +++++-- client/index.html | 4 +++ client/package.json | 1 + client/src/Dashboard.jsx | 2 +- client/src/hooks/useUpdateRunner.js | 43 +++++++++++++++------------ server/Dockerfile | 5 ++++ server/package.json | 1 + server/src/auth.js | 41 ++++++++++++++++++++++++++ server/src/config.js | 4 +++ server/src/docker.js | 19 ++++++++++++ server/src/index.js | 26 +++++++++++++++-- server/src/sse.js | 17 +++++++++++ server/test/auth-ratelimit.test.js | 45 +++++++++++++++++++++++++++++ 16 files changed, 225 insertions(+), 24 deletions(-) create mode 100644 LICENSE create mode 100644 server/test/auth-ratelimit.test.js diff --git a/.env.example b/.env.example index 6e74865..e728908 100644 --- a/.env.example +++ b/.env.example @@ -31,4 +31,11 @@ DIUN_WEBHOOK_TOKEN= SESSION_TTL=604800 # Public base URL of this app (used in logs / any absolute links). +# If this starts with https, the login cookie is marked Secure. BASE_URL=http://localhost:5000 + +# Name of this app's OWN container. It is excluded from the dashboard so it +# can't be told to update (and thereby restart) itself mid-update. Defaults to +# "diun-updater" (the container_name in the shipped docker-compose.yml); change +# it only if you rename the service. +SELF_CONTAINER_NAME=diun-updater diff --git a/API_CONTRACT.md b/API_CONTRACT.md index 018577c..0c51c86 100644 --- a/API_CONTRACT.md +++ b/API_CONTRACT.md @@ -29,6 +29,8 @@ All request/response bodies are JSON unless noted otherwise. - Response: - `200 { "ok": true }` + `Set-Cookie: diun_session=...` on success. - `401 { "error": "invalid_password" }` on bad password. + - `429 { "error": "too_many_attempts" }` after too many failed attempts + from one client IP (temporary lockout). ### `POST /api/auth/logout` diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..499edb7 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 strandedturtle + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index a912db6..a437b5e 100644 --- a/README.md +++ b/README.md @@ -309,6 +309,7 @@ All configuration is via environment variables (see `.env.example`). | `DATA_DIR` | `/data` | | SQLite (`app.db`) location; persist via a volume. | | `SESSION_TTL` | `604800` | | Login cookie lifetime in seconds (7 days). | | `BASE_URL` | `http://localhost:5000` | | Public URL; if `https`, the cookie is set `Secure`. | +| `SELF_CONTAINER_NAME` | `diun-updater` | | This app's own container name, excluded from the dashboard so it can't update itself. | The three required vars are enforced at startup — the server refuses to boot without them (a `SKIP_CONFIG_CHECK=1` escape hatch exists for skeleton @@ -329,9 +330,13 @@ smoke-tests only; never use it in production). by `DIUN_WEBHOOK_TOKEN` (constant-time compared). Treat that token like a password and don't expose the app publicly without a proxy if you can avoid it. - **Auth** is a single password compared in constant time, issuing a signed, - `httpOnly`, `SameSite=Lax` cookie (`Secure` when `BASE_URL` is https). There's - intentionally no rate-limiting on login yet — keep the app off the open - internet or front it with Access/basic-auth if that matters to you. + `httpOnly`, `SameSite=Lax` cookie (`Secure` when `BASE_URL` is https). Failed + logins are rate-limited per client IP (lockout after repeated failures) to + blunt brute-force — but this is not a substitute for keeping the app off the + open internet or fronting it with Cloudflare Access if exposure matters. +- **The app excludes its own container** from the dashboard (it can't safely + update itself). Update the updater the normal way: + `docker compose pull diun-updater && docker compose up -d diun-updater`. --- diff --git a/client/index.html b/client/index.html index 7cf0a00..65c22f5 100644 --- a/client/index.html +++ b/client/index.html @@ -4,6 +4,10 @@ + + + + Diun Updater diff --git a/client/package.json b/client/package.json index 2c6e417..f965d54 100644 --- a/client/package.json +++ b/client/package.json @@ -2,6 +2,7 @@ "name": "diun-updater-client", "version": "0.1.0", "private": true, + "license": "MIT", "type": "module", "scripts": { "dev": "vite", diff --git a/client/src/Dashboard.jsx b/client/src/Dashboard.jsx index 10dbe3f..b9ecc34 100644 --- a/client/src/Dashboard.jsx +++ b/client/src/Dashboard.jsx @@ -106,7 +106,7 @@ export default function Dashboard({ onPendingCountChange }) { {!loading && !error && containers.length === 0 && (
-

All up to date.

+

No containers found.

)} diff --git a/client/src/hooks/useUpdateRunner.js b/client/src/hooks/useUpdateRunner.js index a19203a..7f937fe 100644 --- a/client/src/hooks/useUpdateRunner.js +++ b/client/src/hooks/useUpdateRunner.js @@ -22,10 +22,25 @@ export function useUpdateRunner(name, onSettled) { const { lines, result, error: sseError, reset } = useSSE(name, streamActive); const resolveRef = useRef(null); + // Ensures we settle (resolve the run promise + notify the dashboard) exactly + // once per run, so a connection error arriving after the result can't + // overwrite a success or trigger a second re-fetch. + const settledRef = useRef(false); + + const settle = useCallback(() => { + if (settledRef.current) return; + settledRef.current = true; + onSettled(name); + if (resolveRef.current) { + resolveRef.current(); + resolveRef.current = null; + } + }, [name, onSettled]); const run = useCallback(() => { return new Promise((resolve) => { resolveRef.current = resolve; + settledRef.current = false; setStartError(''); setStatus({ type: '', message: '' }); reset(); @@ -37,15 +52,11 @@ export function useUpdateRunner(name, onSettled) { }) .catch((err) => { setStartError(err.message || 'Failed to start update'); - if (resolveRef.current) { - resolveRef.current(); - resolveRef.current = null; - } - onSettled(name); + settle(); }) .finally(() => setStarting(false)); }); - }, [name, reset, onSettled]); + }, [name, reset, settle]); useEffect(() => { if (!result) return; @@ -54,22 +65,18 @@ export function useUpdateRunner(name, onSettled) { type: result.success ? 'success' : 'error', message: result.message || (result.success ? 'Updated successfully' : 'Update failed'), }); - onSettled(name); - if (resolveRef.current) { - resolveRef.current(); - resolveRef.current = null; - } - }, [result, name, onSettled]); + settle(); + }, [result, settle]); useEffect(() => { if (!sseError) return; + // If the result already arrived, a subsequent stream error (e.g. the + // server closing the connection) is expected — don't clobber the result. + if (result || settledRef.current) return; + setStreamActive(false); setStatus({ type: 'error', message: sseError }); - onSettled(name); - if (resolveRef.current) { - resolveRef.current(); - resolveRef.current = null; - } - }, [sseError, name, onSettled]); + settle(); + }, [sseError, result, settle]); const busy = starting || streamActive; diff --git a/server/Dockerfile b/server/Dockerfile index f23e9e3..28a1a18 100644 --- a/server/Dockerfile +++ b/server/Dockerfile @@ -28,4 +28,9 @@ COPY --from=client-builder /app/client/dist ./client/dist EXPOSE 5000 +# Container health: hit the public /api/health endpoint. Node 22 ships a +# global fetch, so no extra tooling is needed. +HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ + CMD node -e "fetch('http://127.0.0.1:'+(process.env.PORT||5000)+'/api/health').then(r=>process.exit(r.ok?0:1)).catch(()=>process.exit(1))" + CMD ["node", "src/index.js"] diff --git a/server/package.json b/server/package.json index 8392b17..8ca5c43 100644 --- a/server/package.json +++ b/server/package.json @@ -2,6 +2,7 @@ "name": "diun-updater-server", "version": "0.1.0", "private": true, + "license": "MIT", "type": "module", "engines": { "node": ">=22" diff --git a/server/src/auth.js b/server/src/auth.js index f615463..3b63fc6 100644 --- a/server/src/auth.js +++ b/server/src/auth.js @@ -16,6 +16,39 @@ export const authRouter = express.Router(); const SESSION_COOKIE = 'diun_session'; +// --- Simple in-memory login rate limiting --------------------------------- +// Per-client-IP failed-attempt tracking with a lockout, to blunt brute-force +// of the single shared password. Not a substitute for keeping the app off the +// open internet, but a sane default for a tool that may be exposed. State is +// in-memory (resets on restart), which is fine for a single-instance app. +const MAX_FAILURES = 10; // failures allowed within the window before lockout +const FAILURE_WINDOW_MS = 15 * 60 * 1000; // rolling window for counting failures +const LOCKOUT_MS = 15 * 60 * 1000; // how long a lockout lasts once tripped + +const loginAttempts = new Map(); // ip -> { count, firstAt, lockedUntil } + +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()) { + let a = loginAttempts.get(ip); + if (!a || now - a.firstAt > FAILURE_WINDOW_MS) { + a = { count: 0, firstAt: now, lockedUntil: 0 }; + } + a.count += 1; + if (a.count >= MAX_FAILURES) { + a.lockedUntil = now + LOCKOUT_MS; + } + loginAttempts.set(ip, a); + return a; +} + +export function clearLoginFailures(ip) { + loginAttempts.delete(ip); +} + /** * Constant-time comparison of the supplied password against * `config.ADMIN_PASSWORD`. Guards against length mismatches @@ -62,12 +95,20 @@ export function isValidSession(req) { * success. */ export function loginHandler(req, res) { + const ip = req.ip || req.socket?.remoteAddress || 'unknown'; + + if (isLockedOut(ip)) { + return res.status(429).json({ error: 'too_many_attempts' }); + } + const password = req.body?.password; if (!isValidPassword(password)) { + recordLoginFailure(ip); return res.status(401).json({ error: 'invalid_password' }); } + clearLoginFailures(ip); const expiry = String(Date.now() + config.SESSION_TTL * 1000); res.cookie(SESSION_COOKIE, expiry, { signed: true, diff --git a/server/src/config.js b/server/src/config.js index 0d83b4b..6890138 100644 --- a/server/src/config.js +++ b/server/src/config.js @@ -19,6 +19,10 @@ export const config = { DIUN_WEBHOOK_TOKEN: process.env.DIUN_WEBHOOK_TOKEN || '', SESSION_TTL: envInt('SESSION_TTL', 604800), BASE_URL: process.env.BASE_URL || 'http://localhost:5000', + // Name of this app's own container, excluded from the dashboard so it + // can't try to update (and kill) itself. Defaults to the container_name + // used in the shipped docker-compose.yml; override if you rename it. + SELF_CONTAINER_NAME: process.env.SELF_CONTAINER_NAME || 'diun-updater', }; /** diff --git a/server/src/docker.js b/server/src/docker.js index 1f74c64..20eb13c 100644 --- a/server/src/docker.js +++ b/server/src/docker.js @@ -11,12 +11,24 @@ */ import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; import { spawn } from 'node:child_process'; import Docker from 'dockerode'; import { config } from './config.js'; import { normalizeRef } from './reconcile.js'; +// Best-effort identity of this app's own container, so listContainers can +// exclude it (you can't safely update the updater from within itself). By +// default Docker sets a container's hostname to its short id. +const SELF_HOSTNAME = os.hostname(); + +function isSelfContainer(name, id) { + if (config.SELF_CONTAINER_NAME && name === config.SELF_CONTAINER_NAME) return true; + if (SELF_HOSTNAME && id && id.startsWith(SELF_HOSTNAME)) return true; + return false; +} + // Constructing the client does not connect to the daemon — it just sets // up the socket path to dial on first request. Safe to do at import time. const docker = new Docker({ socketPath: config.DOCKER_SOCKET }); @@ -237,6 +249,13 @@ export async function listContainers() { const inspectData = await container.inspect(); const name = stripLeadingSlash(inspectData.Name); + + // Never list our own container — offering to update it would recreate + // the container running this process mid-update. + if (isSelfContainer(name, summary.Id)) { + continue; + } + const image = inspectData.Config?.Image; if (!image) { console.warn(`docker.js: container ${name} has no Config.Image, skipping`); diff --git a/server/src/index.js b/server/src/index.js index 94f2622..4b8980b 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -5,7 +5,7 @@ import express from 'express'; import cookieParser from 'cookie-parser'; import { config, assertRequiredConfig } from './config.js'; // Importing db creates the data dir + tables as a side effect on load. -import './db.js'; +import db from './db.js'; import { webhookRouter } from './webhook.js'; import { authRouter, requireAuth } from './auth.js'; import { apiRouter } from './routes/api.js'; @@ -25,6 +25,7 @@ if (process.env.SKIP_CONFIG_CHECK !== '1') { } const app = express(); +app.disable('x-powered-by'); app.use(express.json()); app.use(cookieParser(config.SESSION_SECRET)); @@ -66,6 +67,27 @@ if (clientDistExists) { console.warn(`No client build found at ${clientDistDir} — skipping static file serving.`); } -app.listen(config.PORT, () => { +const server = app.listen(config.PORT, () => { console.log(`Diun Updater server listening at ${config.BASE_URL} (port ${config.PORT})`); }); + +// Graceful shutdown: stop accepting connections and checkpoint/close SQLite +// so a `docker stop` doesn't leave the WAL or an in-flight write half-done. +let shuttingDown = false; +function shutdown(signal) { + if (shuttingDown) return; + shuttingDown = true; + console.log(`Received ${signal}, shutting down…`); + server.close(() => { + try { + db.close(); + } catch { + // already closed / nothing to do + } + process.exit(0); + }); + // Don't hang forever if a connection (e.g. an open SSE stream) won't close. + setTimeout(() => process.exit(0), 5000).unref(); +} +process.on('SIGTERM', () => shutdown('SIGTERM')); +process.on('SIGINT', () => shutdown('SIGINT')); diff --git a/server/src/sse.js b/server/src/sse.js index c3cf6cc..b439186 100644 --- a/server/src/sse.js +++ b/server/src/sse.js @@ -89,6 +89,7 @@ export function finish(name, result) { writeToSubscribers(session, evt); for (const res of session.subscribers) { try { + clearInterval(res.__sseKeepAlive); res.end(); } catch { // ignore — subscriber may already be gone @@ -117,6 +118,9 @@ export function subscribe(name, res, req) { 'Content-Type': 'text/event-stream', 'Cache-Control': 'no-cache', Connection: 'keep-alive', + // Tell nginx (and similar) not to buffer the stream, or log lines would + // be held back until the response closed. + 'X-Accel-Buffering': 'no', }); if (typeof res.flushHeaders === 'function') { res.flushHeaders(); @@ -147,7 +151,20 @@ export function subscribe(name, res, req) { session.subscribers.add(res); + // Heartbeat so reverse proxies don't drop an idle connection during a long + // pull with sparse output. SSE comment lines (": ...") are ignored by the + // EventSource client, so they don't show up as log lines. + const keepAlive = setInterval(() => { + try { + res.write(': keepalive\n\n'); + } catch { + // subscriber is gone; the close handler will clear this interval + } + }, 15_000); + res.__sseKeepAlive = keepAlive; + const cleanup = () => { + clearInterval(keepAlive); session.subscribers.delete(res); }; res.on('close', cleanup); diff --git a/server/test/auth-ratelimit.test.js b/server/test/auth-ratelimit.test.js new file mode 100644 index 0000000..4964cca --- /dev/null +++ b/server/test/auth-ratelimit.test.js @@ -0,0 +1,45 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { isLockedOut, recordLoginFailure, clearLoginFailures } from '../src/auth.js'; + +// Each test uses a unique IP because the limiter state is a shared module-level +// map (mirrors how it works at runtime: per-client-IP). + +test('not locked out before reaching the failure threshold', () => { + const ip = 'test-ip-under-threshold'; + for (let i = 0; i < 9; i += 1) recordLoginFailure(ip); + assert.equal(isLockedOut(ip), false); +}); + +test('locked out once failures reach the threshold (10)', () => { + const ip = 'test-ip-threshold'; + for (let i = 0; i < 10; i += 1) recordLoginFailure(ip); + assert.equal(isLockedOut(ip), true); +}); + +test('clearLoginFailures resets the counter', () => { + const ip = 'test-ip-clear'; + for (let i = 0; i < 10; i += 1) recordLoginFailure(ip); + assert.equal(isLockedOut(ip), true); + clearLoginFailures(ip); + assert.equal(isLockedOut(ip), false); +}); + +test('failures older than the window do not accumulate into a lockout', () => { + const ip = 'test-ip-window'; + const t0 = 1_000_000; + for (let i = 0; i < 9; i += 1) recordLoginFailure(ip, t0); + // One more, but well past the rolling window — should start a fresh count. + const later = t0 + 16 * 60 * 1000; + recordLoginFailure(ip, later); + assert.equal(isLockedOut(ip, later), false); +}); + +test('lockout expires after the lockout duration', () => { + const ip = 'test-ip-expiry'; + const t0 = 5_000_000; + for (let i = 0; i < 10; i += 1) recordLoginFailure(ip, t0); + assert.equal(isLockedOut(ip, t0), true); + // 16 minutes later (lockout is 15m) it should be clear again. + assert.equal(isLockedOut(ip, t0 + 16 * 60 * 1000), false); +});