From ad2cacf0949f9d034919a6efac828c41a382a385 Mon Sep 17 00:00:00 2001 From: William Mantly Date: Wed, 15 Jul 2026 00:41:16 -0400 Subject: [PATCH 1/3] Make basic auth and SSO mutually exclusive per host; fix silently-broken validation errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Auth tab is now a single choice (Off / Basic auth / SSO) instead of two independent toggles that could both be on at once, which made it ambiguous which gate actually protected a request. Enforced both in the UI and server-side (POST/PUT), accounting for partial PUT updates against the existing record. - Add per-user basic-auth management (change password, delete) so an admin no longer has to blow away and retype the whole user list to remove or rotate one account. - Fix: `Model.errors.ObjectValidateError(...)` is a constructor and was being called without `new` everywhere in this codebase. Without `new`, `this` inside it was the module's shared `errors` object (mutated in place) and the call evaluated to `undefined` — so every `throw Model.errors.ObjectValidateError(...)` actually threw `undefined`, which Express's `next(undefined)` treats as "no error" and silently falls through to the catch-all 404 handler. Every host/user/group/ permission/dns-provider validation error (bad hostname, bad IP, etc.) was showing a confusing "Page not found" instead of the real message. Co-Authored-By: Claude Sonnet 5 --- nodejs/models/dns_provider.js | 2 +- nodejs/models/host.js | 2 +- nodejs/models/local_group.js | 4 +- nodejs/models/permission.js | 4 +- nodejs/routes/host.js | 61 ++++++++++- nodejs/routes/render.js | 10 +- nodejs/routes/user.js | 2 +- nodejs/views/hosts.ejs | 190 ++++++++++++++++++++++++---------- 8 files changed, 209 insertions(+), 66 deletions(-) diff --git a/nodejs/models/dns_provider.js b/nodejs/models/dns_provider.js index 172317e..636aab9 100644 --- a/nodejs/models/dns_provider.js +++ b/nodejs/models/dns_provider.js @@ -176,7 +176,7 @@ class DnsProvider extends Table{ for(let key in Provider._keyMap){ keys.push({'key': key, message: 'Invalid Key'}) } - throw this.errors.ObjectValidateError(keys, "API rejected key"); + throw new this.errors.ObjectValidateError(keys, "API rejected key"); } // Don't swallow other failures (e.g. a domain-sync validation error): // returning undefined here made the route crash on `item.id` with an diff --git a/nodejs/models/host.js b/nodejs/models/host.js index e6d7fb7..445bf06 100755 --- a/nodejs/models/host.js +++ b/nodejs/models/host.js @@ -189,7 +189,7 @@ class Host extends Table{ }catch(error){ console.log('validateWildcardCreate error', error) if(error.status === 404) error.message = "No matching DNS provider registered" - throw this.errors.ObjectValidateError([{key: 'host', message: error.message}]); + throw new this.errors.ObjectValidateError([{key: 'host', message: error.message}]); } } diff --git a/nodejs/models/local_group.js b/nodejs/models/local_group.js index f8a83f1..fa3c75f 100644 --- a/nodejs/models/local_group.js +++ b/nodejs/models/local_group.js @@ -30,7 +30,7 @@ class LocalGroup extends Table{ static async create(data){ data.name = this.slug(data.name); if(!data.name){ - throw this.errors.ObjectValidateError([{key: 'name', message: 'A group name is required.'}]); + throw new this.errors.ObjectValidateError([{key: 'name', message: 'A group name is required.'}]); } if(!Array.isArray(data.members)) data.members = []; return super.create(data); @@ -39,7 +39,7 @@ class LocalGroup extends Table{ async addMember(username){ username = String(username || '').trim(); if(!username){ - throw this.constructor.errors.ObjectValidateError([{key: 'username', message: 'A username is required.'}]); + throw new this.constructor.errors.ObjectValidateError([{key: 'username', message: 'A username is required.'}]); } let members = Array.isArray(this.members) ? this.members : []; if(members.includes(username)) return this; diff --git a/nodejs/models/permission.js b/nodejs/models/permission.js index 2287366..99b2c09 100644 --- a/nodejs/models/permission.js +++ b/nodejs/models/permission.js @@ -55,10 +55,10 @@ class Permission extends Table{ static async create(data){ if(!this.roles.includes(data.role)){ - throw this.errors.ObjectValidateError([{key: 'role', message: `role must be one of ${this.roles.join(', ')}`}]); + throw new this.errors.ObjectValidateError([{key: 'role', message: `role must be one of ${this.roles.join(', ')}`}]); } if(!['user', 'group'].includes(data.subjectType)){ - throw this.errors.ObjectValidateError([{key: 'subjectType', message: `subjectType must be 'user' or 'group'`}]); + throw new this.errors.ObjectValidateError([{key: 'subjectType', message: `subjectType must be 'user' or 'group'`}]); } if(data.scope === 'global') data.domain = '*'; data.id = this.mkId(data); diff --git a/nodejs/routes/host.js b/nodejs/routes/host.js index ccfe75d..c93e1f4 100755 --- a/nodejs/routes/host.js +++ b/nodejs/routes/host.js @@ -6,7 +6,7 @@ const {Host, Domain, User} = require('../models').models; const {LocalGroup} = require('../models/local_group'); const {Permission} = require('../models/permission'); const authz = require('../middleware/authz'); -const {normalizeHostFeatures} = require('../utils/host_features'); +const {normalizeHostFeatures, sanitizeBasicAuthObject} = require('../utils/host_features'); const {collectHostFieldErrors} = require('../utils/hostname_validate'); const {hashBasicAuthUsers} = require('../utils/basicauth'); @@ -16,7 +16,22 @@ const Model = Host; // ObjectValidateError (per-field keys) that the frontend surfaces inline. function validateHostFields(body){ let errors = collectHostFieldErrors(body); - if(errors.length) throw Model.errors.ObjectValidateError(errors); + if(errors.length) throw new Model.errors.ObjectValidateError(errors); +} + +// Basic auth and SSO are mutually exclusive per host (having both enabled +// invites confusion about which gate actually protected a request). `existing` +// is the current record (undefined on create), so a partial PUT that only +// touches one of the two fields is still checked against the other's current +// value. +function validateAuthExclusivity(body, existing){ + let basic = 'basicauth_enabled' in body ? body.basicauth_enabled : (existing ? existing.basicauth_enabled : false); + let sso = 'sso_enabled' in body ? body.sso_enabled : (existing ? existing.sso_enabled : false); + if(basic && sso){ + throw new Model.errors.ObjectValidateError([ + {key: 'sso_enabled', message: 'Basic auth and SSO cannot both be enabled for the same host — pick one.'}, + ]); + } } // After normalizeHostFeatures has parsed basic-auth creds to {user: plaintext}, @@ -73,6 +88,7 @@ router.post('/', authz.requireDomainRole('manager', authz.resolve.hostBody), asy req.body.created_by = authz.reqUsername(req); validateHostFields(req.body); normalizeHostFeatures(req.body); + validateAuthExclusivity(req.body); hashHostSecrets(req.body); let item = await Model.create(req.body); @@ -139,9 +155,10 @@ router.put('/:item', authz.requireDomainRole('manager', authz.resolve.hostParam) req.body.updated_by = authz.reqUsername(req); validateHostFields(req.body); normalizeHostFeatures(req.body); + let existing = await Model.get(req.params.item); + validateAuthExclusivity(req.body, existing); hashHostSecrets(req.body); - let item = await Model.get(req.params.item); - item = await item.update(req.body); + let item = await existing.update(req.body); return res.json({ message: `"${req.params.item}" updated.`, @@ -170,6 +187,42 @@ router.delete('/:item', authz.requireDomainRole('manager', authz.resolve.hostPar } }); +// Manage a single basic-auth user without replacing the whole list — the bulk +// PUT /:item endpoint always replaces basicauth_users wholesale (an empty +// textarea there means "leave existing users untouched", see +// normalizeHostFeatures), which makes deleting or rotating one user's +// password error-prone from that form. These two routes touch exactly one key. +router.put('/:item/basicauth-user/:username', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ + try{ + let item = await Model.get(req.params.item); + let sanitized = sanitizeBasicAuthObject({[req.params.username]: req.body.password}); + let username = Object.keys(sanitized)[0]; + if(!username){ + throw new Model.errors.ObjectValidateError([{key: 'password', message: 'Invalid username or empty password.'}]); + } + + let users = Object.assign({}, item.basicauth_users, hashBasicAuthUsers(sanitized)); + item = await item.update({basicauth_users: users, updated_by: authz.reqUsername(req)}); + + return res.json({message: `User "${username}" saved.`, ...item}); + }catch(error){ + next(error); + } +}); + +router.delete('/:item/basicauth-user/:username', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ + try{ + let item = await Model.get(req.params.item); + let users = Object.assign({}, item.basicauth_users); + delete users[req.params.username]; + item = await item.update({basicauth_users: users, updated_by: authz.reqUsername(req)}); + + return res.json({message: `User "${req.params.username}" removed.`, ...item}); + }catch(error){ + next(error); + } +}); + router.put('/:item/renew', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ let item = await Model.get(req.params.item); diff --git a/nodejs/routes/render.js b/nodejs/routes/render.js index f3dea9c..17235ba 100644 --- a/nodejs/routes/render.js +++ b/nodejs/routes/render.js @@ -19,13 +19,17 @@ const frontEndModules = ['bootstrap', 'mustache', 'jquery', '@fortawesome', // Server front end modules // https://stackoverflow.com/a/55700773/3140931 +// Vendor libraries only change when package versions are bumped (a rebuild), +// so they're safe to cache aggressively; ETag/Last-Modified (on by default) +// still cover that rare case with a cheap 304 instead of a stale asset. frontEndModules.forEach(dep => { - router.use(`/static-modules/${dep}`, express.static(path.join(__dirname, `../node_modules/${dep}`))) + router.use(`/static-modules/${dep}`, express.static(path.join(__dirname, `../node_modules/${dep}`), {maxAge: '7d'})) }); // Have express server static content( images, CSS, browser JS) from the public -// local folder. -router.use('/static', express.static(path.join(__dirname, '../public'))) +// local folder. Shorter maxAge than /static-modules since this is the app's +// own JS/CSS, which changes on every deploy and isn't cache-busted/fingerprinted. +router.use('/static', express.static(path.join(__dirname, '../public'), {maxAge: '1h'})) router.get('/', (req, res) => { res.redirect(301, '/hosts'); diff --git a/nodejs/routes/user.js b/nodejs/routes/user.js index 434d41c..d89e4d1 100755 --- a/nodejs/routes/user.js +++ b/nodejs/routes/user.js @@ -9,7 +9,7 @@ const {passwordError} = require('../utils/password_policy'); // per-field key the frontend surfaces inline. function validatePassword(password){ let message = passwordError(password); - if(message) throw User.errors.ObjectValidateError([{key: 'password', message}]); + if(message) throw new User.errors.ObjectValidateError([{key: 'password', message}]); } // User management is global-admin-only, except the self-service routes below diff --git a/nodejs/views/hosts.ejs b/nodejs/views/hosts.ejs index 8add303..fefc6e7 100755 --- a/nodejs/views/hosts.ejs +++ b/nodejs/views/hosts.ejs @@ -110,6 +110,66 @@ input.focus(); } + // Host name of the record currently open in the edit modal, or null when + // adding a new host (basic-auth user management needs a saved host to + // attach users to). + let hostFormCurrentHost = null; + + // The auth_mode radios aren't real form fields (no [name]); this keeps the + // two hidden basicauth_enabled/sso_enabled inputs — the ones actually + // submitted — in sync so only one can ever be true, and shows/hides the + // matching field group. + function hostAuthModeChanged(mode){ + $('#basicauth_enabled-hidden').val(mode === 'basic' ? 'true' : 'false'); + $('#sso_enabled-hidden').val(mode === 'sso' ? 'true' : 'false'); + $('#hostTab-auth-basicFields').toggle(mode === 'basic'); + $('#hostTab-auth-ssoFields').toggle(mode === 'sso'); + $('#hostTab-auth-basicUsersMgmt').toggle(mode === 'basic' && !!hostFormCurrentHost); + } + + // Per-user basic-auth management (delete / change password) for the host + // currently open in the edit modal. Only shown once a host exists to attach + // users to (not on "Add host", before it's been saved). + function hostRenderBasicAuthUsers(host, users){ + let $rows = $('#basicAuthUserRows').empty(); + let usernames = Object.keys(users || {}); + if(!usernames.length){ + $rows.append('No basic-auth users yet.'); + return; + } + for(let username of usernames){ + let $tr = $(''); + $tr.append($('').text(username)); + let $pass = $(''); + $tr.append($('').append($pass)); + let $actions = $(''); + let $save = $(''); + $save.on('click', function(){ + let password = $pass.val(); + if(!password) return; + app.api.put('host/' + encodeURIComponent(host) + '/basicauth-user/' + encodeURIComponent(username), {password}, function(error, data){ + if(error) return app.util.actionMessage((data && data.message) || 'Failed to update password', $rows, 'danger'); + $pass.val(''); + app.util.actionMessage('Password updated for "' + username + '".', $rows, 'success'); + }); + }); + // No confirm step, matching this form's existing "Delete" button + // (host deletion itself is also a single click, no dialog — see the + // host row actions above). + let $del = $(''); + $del.on('click', function(){ + app.api.delete('host/' + encodeURIComponent(host) + '/basicauth-user/' + encodeURIComponent(username), function(error, data){ + if(error) return app.util.actionMessage((data && data.message) || 'Failed to delete user', $rows, 'danger'); + $tr.remove(); + $('.basicauth-current').text(Object.keys((data && data.basicauth_users) || {}).join(', ') || 'none'); + }); + }); + $actions.append($save).append($del); + $tr.append($actions); + $rows.append($tr); + } + } + // Fill the user/group datalists that back the allow-list autocomplete. function hostLoadAuthSuggestions(){ app.api.get('host/auth-suggestions', function(error, data){ @@ -135,6 +195,8 @@ .addClass('challengeType-container'); $('#challengeType-child-relatedHost').text(''); $('.basicauth-current').text('none'); + hostFormCurrentHost = null; + hostAuthModeChanged('none'); hostShowTab('hostTab-general-btn'); } @@ -175,6 +237,13 @@ $f.find("textarea[name='basicauth_users']").val(''); $('.basicauth-current').text(Object.keys(h.basicauth_users || {}).join(', ') || 'none'); + // Auth: one radio drives both mutually-exclusive booleans. + hostFormCurrentHost = host; + let authMode = h.sso_enabled ? 'sso' : (h.basicauth_enabled ? 'basic' : 'none'); + $f.find('#auth_mode-' + authMode).prop('checked', true); + hostAuthModeChanged(authMode); + hostRenderBasicAuthUsers(host, h.basicauth_users); + // The host name is the key; it can't change on edit. Wildcard hosts can // still toggle their matching mode. $f.find('[name=host]').prop('disabled', true); @@ -637,71 +706,88 @@

- Basic auth and SSO are OR'd — if either is enabled, a request - is allowed when it passes either one. Leave both off for a - public host. + Pick one authentication method for this host — basic auth and + SSO can't both be enabled, to avoid ambiguity about which one + actually protected a request. Choose "Off" for a public host.

-
Basic authentication
+
-
- - -
-
- - - - Current: none. - Passwords are stored hashed and never shown here. Leave blank to keep - the current users; entering any lines replaces the whole list. - + + + + + -
-
Single sign-on (SSO)
-
-
-
- Gates the site behind the same identity provider the admin app uses. Empty allow-lists below mean any authenticated user is allowed. + -
- -
- - + -
- -
- - -
-
From a19ff81c76b3721c37a57b8dea85acbd7f2d3c96 Mon Sep 17 00:00:00 2001 From: William Mantly Date: Wed, 15 Jul 2026 00:41:31 -0400 Subject: [PATCH 2/3] Fix a worker-blocking Lua socket call and add gzip/caching for static assets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ops/nginx_conf/targetinfo.lua's wildcard-subdomain lookup fallback used classic LuaSocket (require("socket.unix")) instead of an OpenResty cosocket. LuaSocket is blocking, and called from an nginx worker it stalls the ENTIRE worker — every other in-flight connection on it — for the round-trip to the Node app. Worse, the Node side never newline-terminated its response, so the old blocking receive() only ever returned via its read-timeout-then-partial-read fallback, meaning every single cache-miss lookup paid a fixed timeout penalty while blocking the whole worker. Replaced with an ngx.socket.tcp() cosocket (unix-domain via "unix:/path", the only cosocket API this lua-nginx-module ships) and newline-terminated the Node service's responses so receive() actually completes instead of timing out. Verified against a live container: previously this crashed OpenResty's Lua VM entirely (ngx.socket.unix doesn't exist); fixed version resolves fresh wildcard subdomains in ~2ms. - Add gzip compression (`compression` middleware) and far-future Cache-Control on static assets (7d for vendor libs under /static-modules, 1h for the app's own /static JS/CSS, which isn't cache-busted). The admin UI is a traditional multi-page app that loads ~13 separate vendor/app JS+CSS files on every full navigation; previously none of them were compressed and Cache-Control was `max-age=0` (Express's default), forcing a revalidation round-trip for every asset on every page view. Co-Authored-By: Claude Sonnet 5 --- nodejs/app.js | 10 +++- nodejs/package-lock.json | 84 ++++++++++++++++++++++++++++++++++ nodejs/package.json | 1 + nodejs/services/host_lookup.js | 12 +++-- ops/nginx_conf/targetinfo.lua | 56 ++++++++++++++--------- 5 files changed, 138 insertions(+), 25 deletions(-) diff --git a/nodejs/app.js b/nodejs/app.js index 1330fdd..bbffc5a 100755 --- a/nodejs/app.js +++ b/nodejs/app.js @@ -3,6 +3,7 @@ const path = require('path'); const ejs = require('ejs') const express = require('express'); +const compression = require('compression'); // Set up the express app. const app = express(); @@ -52,8 +53,15 @@ app.onListen.push(function(){ }); }); +// Gzip text responses (HTML/JS/CSS/JSON). The admin UI loads ~13 separate, +// uncompressed vendor JS/CSS files on every full page navigation (a +// traditional multi-page app, not an SPA) — this alone meaningfully cuts +// bytes-over-the-wire and perceived load time on a real network, where it +// matters far more than on localhost. +app.use(compression()); + // load the JSON parser middleware. Express will parse JSON into native objects -// for any request that has JSON in its content type. +// for any request that has JSON in its content type. app.use(express.json()); // Set up the templating engine to build HTML for the front end. diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index 00bcaff..9ec8e1c 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -16,6 +16,7 @@ "axios": "^1.13.5", "bcrypt": "^6.0.0", "bootstrap": "^5.3.8", + "compression": "^1.8.1", "ejs": "^6.0.1", "express": "^5.2.1", "express-rate-limit": "^8.5.2", @@ -609,6 +610,60 @@ "node": ">= 0.8" } }, + "node_modules/compressible": { + "version": "2.0.18", + "resolved": "https://registry.npmjs.org/compressible/-/compressible-2.0.18.tgz", + "integrity": "sha512-AF3r7P5dWxL8MxyITRMlORQNaOA2IkAFaTr4k7BUumjPtRpGDTZpl0Pb1XCO6JeDCBdp126Cgs9sMxqSjgYyRg==", + "license": "MIT", + "dependencies": { + "mime-db": ">= 1.43.0 < 2" + }, + "engines": { + "node": ">= 0.6" + } + }, + "node_modules/compression": { + "version": "1.8.1", + "resolved": "https://registry.npmjs.org/compression/-/compression-1.8.1.tgz", + "integrity": "sha512-9mAqGPHLakhCLeNyxPkK4xVo746zQ/czLH1Ky+vkitMnWfWZps8r0qXuwhwizagCRttsL4lfG4pIOvaWLpAP0w==", + "license": "MIT", + "dependencies": { + "bytes": "3.1.2", + "compressible": "~2.0.18", + "debug": "2.6.9", + "negotiator": "~0.6.4", + "on-headers": "~1.1.0", + "safe-buffer": "5.2.1", + "vary": "~1.1.2" + }, + "engines": { + "node": ">= 0.8.0" + } + }, + "node_modules/compression/node_modules/debug": { + "version": "2.6.9", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", + "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", + "license": "MIT", + "dependencies": { + "ms": "2.0.0" + } + }, + "node_modules/compression/node_modules/ms": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", + "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==", + "license": "MIT" + }, + "node_modules/compression/node_modules/negotiator": { + "version": "0.6.4", + "resolved": "https://registry.npmjs.org/negotiator/-/negotiator-0.6.4.tgz", + "integrity": "sha512-myRT3DiWPHqho5PrJaIRyaMv2kgYf0mUVgBNOYMuCH5Ki1yEiQaf/ZJuQ62nvpc44wL5WDbTX7yGJi1Neevw8w==", + "license": "MIT", + "engines": { + "node": ">= 0.6" + } + }, "node_modules/content-disposition": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-1.0.1.tgz", @@ -1569,6 +1624,15 @@ "node": ">= 0.8" } }, + "node_modules/on-headers": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/on-headers/-/on-headers-1.1.0.tgz", + "integrity": "sha512-737ZY3yNnXy37FHkQxPzt4UZ2UWPWiCZWLvFZ4fu5cueciegX0zGPnrlY6bwRg4FdQOe9YU8MkmJwGhoMybl8A==", + "license": "MIT", + "engines": { + "node": ">= 0.8" + } + }, "node_modules/once": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", @@ -1754,6 +1818,26 @@ "node": ">= 18" } }, + "node_modules/safe-buffer": { + "version": "5.2.1", + "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz", + "integrity": "sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/feross" + }, + { + "type": "patreon", + "url": "https://www.patreon.com/feross" + }, + { + "type": "consulting", + "url": "https://feross.org/support" + } + ], + "license": "MIT" + }, "node_modules/safer-buffer": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", diff --git a/nodejs/package.json b/nodejs/package.json index 379923c..e365158 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -27,6 +27,7 @@ "axios": "^1.13.5", "bcrypt": "^6.0.0", "bootstrap": "^5.3.8", + "compression": "^1.8.1", "ejs": "^6.0.1", "express": "^5.2.1", "express-rate-limit": "^8.5.2", diff --git a/nodejs/services/host_lookup.js b/nodejs/services/host_lookup.js index 1e28c02..1157713 100644 --- a/nodejs/services/host_lookup.js +++ b/nodejs/services/host_lookup.js @@ -33,7 +33,7 @@ const socket = new SocketServerJson({ let parentHost = Host.lookUp(data['domain']); // If we don't have a match, return empty object - if(!parentHost) return clientSocket.write(JSON.stringify({})); + if(!parentHost) return clientSocket.write(JSON.stringify({}) + '\n'); // lookUp returns the live #record object stored inside the shared // lookup tree. Everything below mutates parentHost (sets @@ -50,7 +50,7 @@ const socket = new SocketServerJson({ // subdomain and must not be routed to the wildcard parent. if(parentHost.is_wildcard && !parentHost.wildcard_matchAny && parentHost.host !== data['domain']){ - return clientSocket.write(JSON.stringify({})); + return clientSocket.write(JSON.stringify({}) + '\n'); } // If the matched host belongs to a wildcard domain, set wildcard_parent @@ -66,7 +66,13 @@ const socket = new SocketServerJson({ parentHost[key] = String(value); } - clientSocket.write(JSON.stringify(parentHost)); + // Terminate with a newline: the Lua client (ops/nginx_conf/targetinfo.lua) + // reads a single line per lookup via a cosocket receive() -- without a + // delimiter it would block for the full read timeout on every request + // waiting for a newline that never arrives (this was masked before by + // blocking LuaSocket's timeout+partial-read behavior, which silently + // paid that same timeout on every single lookup). + clientSocket.write(JSON.stringify(parentHost) + '\n'); }catch(error){ console.error('services/host_lookup onData error', error); } diff --git a/ops/nginx_conf/targetinfo.lua b/ops/nginx_conf/targetinfo.lua index befe6de..cb4fb9c 100644 --- a/ops/nginx_conf/targetinfo.lua +++ b/ops/nginx_conf/targetinfo.lua @@ -1,12 +1,39 @@ local M = {} --- Function to connect to a Unix socket -local function connect(path) - local socket = require("socket.unix")() - assert(socket:settimeout(.1)) - local status, err = pcall(function() assert(socket:connect(path)) end) - if status then return true end - return false +-- Query the Node app's host-lookup service for a domain that missed the +-- Redis fast path (wildcard subdomains not yet cached, see Host.addCache). +-- Uses an OpenResty cosocket rather than the classic LuaSocket socket.unix() +-- the previous version of this file used: LuaSocket's API is blocking and, +-- called from an nginx worker, stalls the ENTIRE worker (every other +-- in-flight connection on it) for the round-trip -- a real source of +-- intermittent request latency for any wildcard host whose on-demand cache +-- entry (1h TTL, conf.cacheTTL) had expired. resty.redis (used just above) +-- is cosocket-based already and works fine from both the phases this module +-- is called from (access_by_lua_block and the SSL request_domain callback), +-- so a unix-domain cosocket is safe here too. +local function unixLookup(json, domain) + -- The ngx_lua cosocket API has no separate ngx.socket.unix -- a plain + -- ngx.socket.tcp() connects to a unix domain socket when given a + -- "unix:/path" address instead of a host/port pair. + local sock = ngx.socket.tcp() + sock:settimeouts(100, 100, 100) -- connect, send, read (ms) + + local ok = sock:connect("unix:/var/run/proxy_lookup.socket") + if not ok then return nil end + + local ok = sock:send(json.encode({domain = domain})) + if not ok then + sock:close() + return nil + end + + local line = sock:receive() + sock:close() + if not line then return nil end + + local decodeOk, decoded = pcall(json.decode, line) + if not decodeOk then return nil end + return decoded end print("In targetInfo module") @@ -61,20 +88,7 @@ function M.get(ngx, domain, targetInfo) end if not res["ip"] then - if connect("/var/run/proxy_lookup.socket") then - local socket = require("socket.unix")() - assert(socket:settimeout(.1)) - assert(socket:connect("/var/run/proxy_lookup.socket")) - assert(socket:send(json.encode({domain = domain}))) - while true do - local s, status, partial = socket:receive() - if partial then - res = json.decode(partial) - socket:close() - break - end - end - end + res = unixLookup(json, domain) or res end if not res["ip"] then From 1026f18c085e72ecbac8db860dfe6476948f5bca Mon Sep 17 00:00:00 2001 From: William Mantly Date: Wed, 15 Jul 2026 00:45:24 -0400 Subject: [PATCH 3/3] Rate-limit host-mutating routes CodeQL flagged POST/PUT/DELETE /api/host* as missing rate limiting despite performing authorization -- same authLimiter pattern routes/auth.js already uses, applied here with a higher ceiling since legitimate admin work (bulk edits) is expected on these routes. CodeQL also flagged utils/basicauth.js's SHA-1 hashing as reachable from the new basicauth-user route -- this is the existing, documented htpasswd- compatible {SHA} scheme (see the comment on hashPassword), not something this PR changes; left as-is per that comment's existing "follow-up" note, since swapping it requires a coordinated change to ops/nginx_conf/hostfeatures.lua's verification and a migration path for already-stored hashes. Co-Authored-By: Claude Sonnet 5 --- nodejs/routes/host.js | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/nodejs/routes/host.js b/nodejs/routes/host.js index c93e1f4..775d3d4 100755 --- a/nodejs/routes/host.js +++ b/nodejs/routes/host.js @@ -1,6 +1,7 @@ 'use strict'; const router = require('express').Router(); +const {rateLimit} = require('express-rate-limit'); const conf = require('@simpleworkjs/conf'); const {Host, Domain, User} = require('../models').models; const {LocalGroup} = require('../models/local_group'); @@ -12,6 +13,20 @@ const {hashBasicAuthUsers} = require('../utils/basicauth'); const Model = Host; +// Throttle host-mutating endpoints (create/update/delete a host, manage a +// basic-auth user's password) per IP. These already require an authenticated, +// authorized manager/admin, but a compromised or careless session shouldn't +// be able to hammer them unboundedly — same pattern as routes/auth.js's +// authLimiter, just a higher ceiling since legitimate admin work (bulk edits) +// is expected here. +const mutateLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 300, // 300 mutations per IP per window + standardHeaders: true, + legacyHeaders: false, + message: {name: 'TooManyRequests', message: 'Too many requests, please try again later.'}, +}); + // Reject a malformed host/target before it reaches the model. Throws a 422 // ObjectValidateError (per-field keys) that the frontend surfaces inline. function validateHostFields(body){ @@ -83,7 +98,7 @@ router.get('/', async function(req, res, next){ } }); -router.post('/', authz.requireDomainRole('manager', authz.resolve.hostBody), async function(req, res, next){ +router.post('/', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostBody), async function(req, res, next){ try{ req.body.created_by = authz.reqUsername(req); validateHostFields(req.body); @@ -125,7 +140,7 @@ router.get('/lookupobj', authz.requireAdmin, async function(req, res, next){ } }); -router.delete('/cache', authz.requireAdmin, async function(req, res, next){ +router.delete('/cache', mutateLimiter, authz.requireAdmin, async function(req, res, next){ try{ let count = await Model.clearCache(); @@ -150,7 +165,7 @@ router.get('/:item', authz.requireDomainRole('viewer', authz.resolve.hostParam), } }); -router.put('/:item', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.put('/:item', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ req.body.updated_by = authz.reqUsername(req); validateHostFields(req.body); @@ -172,7 +187,7 @@ router.put('/:item', authz.requireDomainRole('manager', authz.resolve.hostParam) } }); -router.delete('/:item', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.delete('/:item', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ let item = await Model.get(req.params.item); let count = await item.remove(); @@ -192,7 +207,7 @@ router.delete('/:item', authz.requireDomainRole('manager', authz.resolve.hostPar // textarea there means "leave existing users untouched", see // normalizeHostFeatures), which makes deleting or rotating one user's // password error-prone from that form. These two routes touch exactly one key. -router.put('/:item/basicauth-user/:username', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.put('/:item/basicauth-user/:username', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ let item = await Model.get(req.params.item); let sanitized = sanitizeBasicAuthObject({[req.params.username]: req.body.password}); @@ -210,7 +225,7 @@ router.put('/:item/basicauth-user/:username', authz.requireDomainRole('manager', } }); -router.delete('/:item/basicauth-user/:username', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.delete('/:item/basicauth-user/:username', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ let item = await Model.get(req.params.item); let users = Object.assign({}, item.basicauth_users); @@ -223,7 +238,7 @@ router.delete('/:item/basicauth-user/:username', authz.requireDomainRole('manage } }); -router.put('/:item/renew', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ +router.put('/:item/renew', mutateLimiter, authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){ try{ let item = await Model.get(req.params.item); item.createWildcardCert();