Fix: stop the admin UI announcing saves that failed

CHANGELOG.md

@@ -36,6 +36,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Saving an app or widget no longer reports "Updated" when the write failed; the
+  editor stays open with the form intact.
+
+- A failed save no longer leaves the admin list showing a delete, reorder,
+  drag-drop or folder change the server never received.
+
+- Importing a config reports "Imported" only once the write has landed.
+
+- A failed label-visibility toggle now reports the error instead of failing
+  silently with the switch left in its new position.
+
 - The Books, Now Playing and Backup widgets now show why they failed, such as a
   missing API key or a rejected password, instead of a generic error.
 
@@ -89,6 +100,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - A failed connection test no longer writes the request path to the log, where
   a service that carries its API key in the path would have left it.
 
+- Allowing a self-signed certificate now applies only to addresses on your own
+  network, wherever the request came from.
+  A badge or health check pointed at a public address is verified again, so an
+  app relying on this for one must use a trusted certificate.
+
+- Importing from another dashboard asks before allowing self-signed
+  certificates, instead of taking the setting from the file.
+
+- Sessions now expire after 12 hours idle instead of 30 days, and a session in
+  use is extended. Override with `SESSION_MAX_AGE_DAYS`.
+
+- Requests that change something must state their origin.
+  A script calling the API directly now has to send an `Origin` header naming
+  the address it is calling.
+
+- The API listens on loopback only, so it can no longer be reached around Nginx.
+
+- The pre-login status reply no longer says whether a password has been set.
+
 ## [1.5.2] - 2026-08-09
 
 ### Changed

api/src/auth.js

@@ -182,9 +182,21 @@ function needsRehash(hash) {
 }
 
 /* The signed issued-at inside the token is the real control; the cookie Max-Age
-   is only a browser hint kept in sync with it. */
+   is only a browser hint kept in sync with it.
+
+   This is an idle window, not a fixed one: a session in use is reissued by
+   refreshSession below, so the age here is how long a session survives with
+   nobody using it. A token that leaks is therefore usable for hours rather than
+   the month it used to be, while somebody working in the dashboard is not
+   signed out mid-task. */
+const DEFAULT_MAX_AGE_HOURS = 12;
 const _maxAgeDays = Number(process.env.SESSION_MAX_AGE_DAYS);
-const SESSION_MAX_AGE_MS = (_maxAgeDays > 0 ? _maxAgeDays : 30) * 24 * 60 * 60 * 1000;
+const SESSION_MAX_AGE_MS = _maxAgeDays > 0 ? _maxAgeDays * 24 * 60 * 60 * 1000 : DEFAULT_MAX_AGE_HOURS * 60 * 60 * 1000;
+
+/* Reissue once the token is past halfway. Renewing on every request would set a
+   cookie on every response for no gain; waiting until it nearly expires would
+   leave a tab open across the boundary holding a token it cannot renew. */
+const RENEW_AFTER_MS = SESSION_MAX_AGE_MS / 2;
 
 /* `${sessionId}.${issuedAt}.${sig}`, where sig covers the first two. Signing the
    timestamp is what lets verifyToken enforce a max age with no session store. */
@@ -195,7 +207,12 @@ function makeToken(sessionId, secret) {
   return `${payload}.${sig}`;
 }
 
-function verifyToken(token, secret) {
+/** The session a token stands for, with the moment it was issued, or null when
+    it does not verify or has aged out. Callers that only need the identity use
+    verifyToken; the issued-at is what refreshSession needs to decide on renewal.
+    @param {string} token @param {string} secret
+    @returns {{ sessionId: string, iat: number }|null} */
+function readToken(token, secret) {
   const dot2 = token.lastIndexOf('.');
   if (dot2 === -1) return null;
   const sig = token.slice(dot2 + 1);
@@ -209,7 +226,11 @@ function verifyToken(token, secret) {
   const expected = crypto.createHmac('sha256', secret).update(rest).digest('hex');
   if (!crypto.timingSafeEqual(Buffer.from(sig, 'hex'), Buffer.from(expected, 'hex'))) return null;
   if (Date.now() - Number(iat) > SESSION_MAX_AGE_MS) return null;
-  return sessionId;
+  return { sessionId, iat: Number(iat) };
+}
+
+function verifyToken(token, secret) {
+  return readToken(token, secret)?.sessionId ?? null;
 }
 
 /* decodeURIComponent throws on an invalid escape, so a stray '%' in any cookie
@@ -337,6 +358,25 @@ function isAuthenticated(req) {
   return !!verifyToken(token, secret);
 }
 
+/* Extend a session that is still in use, so the shortened idle window does not
+   sign out somebody working in the dashboard. The identifier is carried over and
+   only the issued-at moves, so this lengthens a session rather than starting one.
+
+   A handler that sets its own cookie runs after this and replaces the header, so
+   logging out and rotating the secret both still win. */
+function refreshSession(req, res) {
+  const cfg = loadConfig();
+  if (!authActive(cfg)) return false;
+  const secret = cfg.settings.auth.secret;
+  if (!secret) return false;
+  const token = parseCookies(req).ds;
+  if (!token) return false;
+  const read = readToken(token, secret);
+  if (!read || Date.now() - read.iat < RENEW_AFTER_MS) return false;
+  setSessionCookie(res, makeToken(read.sessionId, secret), isSecureRequest(req));
+  return true;
+}
+
 /* Unlike isAuthenticated, requires a real session even when auth is off. For
    operations such as changing an existing password. */
 function hasValidSession(req) {
@@ -362,6 +402,8 @@ module.exports = {
   parseHash,
   makeToken,
   verifyToken,
+  readToken,
+  refreshSession,
   parseCookies,
   setSessionCookie,
   clearSessionCookie,
@@ -374,4 +416,5 @@ module.exports = {
   /* Tests exercise limits in sequence in one process. */
   _resetRateLimits: () => _rateBuckets.clear(),
   SESSION_MAX_AGE_MS,
+  RENEW_AFTER_MS,
 };

api/src/proxy.js

@@ -122,6 +122,54 @@ function shouldSkipTls(hostname, cfg) {
   return isInternalHost(hostname);
 }
 
+/* Whether to turn certificate checking off for this one request.
+
+   The answer is the same wherever the request came from: only a host on your own
+   network. A self-signed certificate is a thing you have on a box you run, so
+   the switch that tolerates one has no honest use against a public address,
+   where the only effect is to accept whoever answers. The server-wide setting
+   was already scoped this way; the per-app switch and the per-request flag were
+   not, and those are the paths that carry a stored credential.
+
+   `ignored` says the caller asked and was refused, so a certificate failure can
+   be explained rather than just reported.
+
+   @param {string} hostname @param {boolean|null|undefined} requested
+   @returns {{ skip: boolean, ignored: boolean }} */
+function resolveSkipTls(hostname, requested) {
+  let wanted;
+  if (requested != null) wanted = requested === true;
+  else {
+    try {
+      wanted = loadConfig().settings?.server?.skipTlsVerify === true;
+    } catch {
+      wanted = false;
+    }
+  }
+  if (!wanted) return { skip: false, ignored: false };
+  const internal = isInternalHost(hostname);
+  return { skip: internal, ignored: !internal };
+}
+
+/* Shown when a certificate failed on a host the skip could not apply to, so the
+   answer names the reason rather than looking like the switch did nothing.
+
+   Wording only: it carries no hostname, path or upstream text, which is what
+   makes it safe to vouch for and show. See api-error.js. */
+const SKIP_TLS_IGNORED_MESSAGE =
+  'The certificate could not be verified. Allowing a self-signed certificate only applies to addresses on your own ' +
+  'network, so it was not used here.';
+
+const TLS_ERROR_CODES = new Set([
+  'DEPTH_ZERO_SELF_SIGNED_CERT',
+  'SELF_SIGNED_CERT_IN_CHAIN',
+  'UNABLE_TO_VERIFY_LEAF_SIGNATURE',
+  'UNABLE_TO_GET_ISSUER_CERT_LOCALLY',
+  'CERT_HAS_EXPIRED',
+  'CERT_NOT_YET_VALID',
+  'ERR_TLS_CERT_ALTNAME_INVALID',
+]);
+
 function rewriteUrl(raw) {
   try {
     const cfg = loadConfig(),
@@ -229,7 +277,7 @@ function fetchJSON(raw, opts = {}) {
     const done = (fn, arg) => dl.settle(fn, arg);
     const lib = u.protocol === 'https:' ? https : http;
     const port = u.port || (u.protocol === 'https:' ? 443 : 80);
-    const skipTls = opts.skipTls != null ? opts.skipTls : shouldSkipTls(u.hostname, loadConfig());
+    const { skip: skipTls, ignored: skipIgnored } = resolveSkipTls(u.hostname, opts.skipTls);
     const bodyBuf = opts.body ? Buffer.from(opts.body) : null;
     const hdrs = Object.assign({}, opts.headers || {});
     if (bodyBuf) hdrs['Content-Length'] = bodyBuf.length;
@@ -299,7 +347,11 @@ function fetchJSON(raw, opts = {}) {
       req.destroy();
       done(reject, new Error('Timed out'));
     });
-    req.on('error', e => done(reject, e));
+    req.on('error', (/** @type {unknown} */ e) => {
+      if (skipIgnored && TLS_ERROR_CODES.has(errCode(e) ?? ''))
+        /** @type {{ vouchedMessage?: string }} */ (e).vouchedMessage = SKIP_TLS_IGNORED_MESSAGE;
+      done(reject, e);
+    });
     if (bodyBuf) req.write(bodyBuf);
     req.end();
   });
@@ -356,7 +408,7 @@ function pingUrl(raw, ms = PING_MS, skipTls, pinIp) {
     if (policy) return resolve({ ok: false, status: 0, error: policy });
     const lib = u.protocol === 'https:' ? https : http;
     const port = u.port || (u.protocol === 'https:' ? 443 : 80);
-    const skip = skipTls != null ? skipTls : shouldSkipTls(u.hostname, loadConfig());
+    const { skip, ignored: skipIgnored } = resolveSkipTls(u.hostname, skipTls);
     const pin = pinIp && pinIp !== u.hostname ? pinIp : null;
     const connectHost = pin || u.hostname.replace(/^\[|\]$/g, '');
     const opts = { hostname: connectHost, port, path: u.pathname || '/', timeout: ms, rejectUnauthorized: !skip };
@@ -392,7 +444,8 @@ function pingUrl(raw, ms = PING_MS, skipTls, pinIp) {
            operator's question is which service is unreachable, which the origin
            answers on its own. */
         log.warn('ping failed', { url: u.origin, error: errMessage(e) });
-        dl.settle(resolve, { ok: false, status: 0, error: pingErrorText(e), code: errCode(e) });
+        const text = skipIgnored && TLS_ERROR_CODES.has(errCode(e) ?? '') ? SKIP_TLS_IGNORED_MESSAGE : pingErrorText(e);
+        dl.settle(resolve, { ok: false, status: 0, error: text, code: errCode(e) });
       });
       req.end();
     };
@@ -460,6 +513,8 @@ module.exports = {
   rewriteUrl,
   getHostIp,
   shouldSkipTls,
+  resolveSkipTls,
+  SKIP_TLS_IGNORED_MESSAGE,
   isInternalHost,
   isDockerServiceName,
   bareHost,

api/src/router.js

@@ -1,4 +1,4 @@
-const { isAuthenticated } = require('./auth');
+const { isAuthenticated, refreshSession } = require('./auth');
 const log = require('./log');
 const { tryDecode } = require('./percent-decode');
 
@@ -36,6 +36,9 @@ function route(req, res) {
 
   if (!PUBLIC_PATHS.has(u.pathname)) {
     if (!isAuthenticated(req)) return json(res, 401, { error: 'Unauthorised', auth: true, kind: 'auth' });
+    /* Before the handler, so a handler that issues its own cookie replaces this
+       one rather than being overwritten by it. */
+    refreshSession(req, res);
   }
 
   for (const r of routes) {
@@ -116,15 +119,26 @@ function getIp(req) {
   }
   return peer || 'unknown';
 }
+/* Every browser sends Origin on a request that changes something, including a
+   same-origin one, so a write arriving without it did not come from a page on
+   this dashboard. It used to be allowed through, which left the check answering
+   only the requests that were already declaring themselves.
+
+   This is what a command-line client has to add to write to the API: an Origin
+   naming the address it is calling. */
 function checkOrigin(req, res) {
   const origin = req.headers['origin'];
-  if (!origin) return true;
-  try {
-    const originHost = new URL(origin).host;
-    const serverHost = req.headers['host'];
-    if (originHost === serverHost) return true;
-  } catch {}
-  json(res, 403, { error: 'Forbidden: origin mismatch', kind: 'invalid' });
+  if (origin) {
+    try {
+      if (new URL(origin).host === req.headers['host']) return true;
+    } catch {
+      /* not a URL, so it names no host and cannot match one */
+    }
+  }
+  json(res, 403, {
+    error: origin ? 'Forbidden: origin mismatch' : 'Forbidden: this request needs an Origin header',
+    kind: 'invalid',
+  });
   return false;
 }
 

api/src/routes/auth.js

@@ -22,17 +22,27 @@ const {
   needsRehash,
 } = require('../auth');
 
+/* The one route that answers before sign-in, so it says only what the login
+   screen has to decide: whether to ask for a password, and whether this caller
+   already has a session. The two setup fields describe how the install is
+   configured and are added only once the caller is through, which is every case
+   that reads them: the setup prompt runs with protection off, where isAuthenticated
+   is true for everyone. */
 on('GET', '/api/auth/check', (req, res) => {
   const cfg = loadConfig();
-  json(res, 200, {
+  const authenticated = isAuthenticated(req);
+  const body = {
     /* The effective state, not the stored flag. Auth on with no password is
        reported as off because that is how it behaves; showing it as on would put
        a toggle in the admin UI that does not match what the server does. */
     enabled: authActive(cfg),
-    authenticated: isAuthenticated(req),
-    passwordSet: !!cfg.settings?.auth?.passwordHash,
-    setupPrompted: !!cfg.settings?.auth?.setupPrompted,
-  });
+    authenticated,
+  };
+  if (authenticated) {
+    body.passwordSet = !!cfg.settings?.auth?.passwordHash;
+    body.setupPrompted = !!cfg.settings?.auth?.setupPrompted;
+  }
+  json(res, 200, body);
 });
 
 on('POST', '/api/auth/login', async (req, res) => {

api/src/server.js

@@ -27,7 +27,14 @@ require('./widget-data');
 
 const { dispatch } = require('./router');
 
-http.createServer(dispatch).listen(PORT, () => {
+/* Loopback only. nginx is the only thing that ever calls this port, and it does
+   so on 127.0.0.1, so binding every interface only added a way into the API that
+   goes around nginx: reachable from anything sharing the container's network,
+   and arriving without the client address nginx resolves, so rate limiting saw
+   one caller. */
+const HOST = '127.0.0.1';
+
+http.createServer(dispatch).listen(PORT, HOST, () => {
   const { CONFIG_PATH, ICONS_PATH } = require('./config');
   const { getRegistry } = require('./widgets');
   const pkg = require('../package.json');

api/test/environment-doc.test.js

@@ -107,7 +107,9 @@ test('the documented defaults are the ones in the code', () => {
 test('the documented session and hash defaults match the code', () => {
   const defaults = Object.fromEntries(documentedRows());
   const auth = read('api/src/auth.js');
-  assert.equal(defaults.SESSION_MAX_AGE_DAYS, /_maxAgeDays > 0 \? _maxAgeDays : (\d+)/.exec(auth)[1]);
+  /* Documented in hours, because the default is shorter than the day the
+     variable is named for. */
+  assert.equal(defaults.SESSION_MAX_AGE_DAYS, `${/const DEFAULT_MAX_AGE_HOURS = (\d+)/.exec(auth)[1]}h`);
   assert.equal(defaults.PASSWORD_HASH_MEMORY, /const DEFAULT_PROFILE = '([\w]+)'/.exec(auth)[1]);
 });