fix: the signed-out preview never worked, because /api was gated wholesale
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 52s
Forgejo Docker Build / Root app tests (push) Successful in 52s
Forgejo Android APK / Build signed APK (push) Successful in 2m5s
Forgejo Docker Build / Build Docker image (push) Successful in 17s
Forgejo Docker Build / Deploy to the host (push) Failing after 0s
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 52s
Forgejo Docker Build / Root app tests (push) Successful in 52s
Forgejo Android APK / Build signed APK (push) Successful in 2m5s
Forgejo Docker Build / Build Docker image (push) Successful in 17s
Forgejo Docker Build / Deploy to the host (push) Failing after 0s
Eleven routers are mounted on '/api' and called router.use(authMiddleware) with no path. Mounted that way, the gate applies to every /api request that reaches the router — including routes belonging to routers mounted further down server.js. extensions.js did it from line 295; the assistant is mounted at 305. So a signed-out request to /api/clinical-assistant/status was refused ten lines before the preview middleware could look at it, whatever the admin setting said. server.js line 250 already warned about this shape. Each gate now names its own prefix, so a router protects its own routes and nothing else. Verified afterwards that every namespace which must stay shut still answers 401 signed out: extensions, encounters, memories, notes, diagrams, generated images, image jobs, documents, audio backups, ED encounters, don't-miss, patient education, billing, well visit, admin, transcribe and the rest. Two of these routers were gating routes nobody realised they were gating. Second defect in the same path: authMiddleware only ever looks for a token, so calling it unconditionally after the preview identity had been assigned rejected exactly the requests preview exists to serve. Only that identity may skip it; authMiddleware stays strict everywhere else. Preview now answers with a real cited answer, and stays as narrow as it was designed to be — four allow-listed paths, no identity, nothing ownable. A test now walks every /api router and fails on a blanket gate, which is how the last six were found. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
parent
ca0be0a98e
commit
cc76c66953
13 changed files with 101 additions and 12 deletions
|
|
@ -28,7 +28,10 @@ var upload = multer({
|
|||
limits: { fileSize: 25 * 1024 * 1024 },
|
||||
});
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. Mounted on /api, a bare
|
||||
// router.use(authMiddleware) gated every /api path — including routes owned by
|
||||
// routers mounted after it in server.js.
|
||||
router.use('/audio-backups', authMiddleware);
|
||||
|
||||
// ── POST save audio backup (compressed) ──────────────────────────────────
|
||||
router.post('/audio-backups', upload.single('audio'), async function(req, res) {
|
||||
|
|
|
|||
|
|
@ -79,7 +79,17 @@ router.use(async function(req, res, next) {
|
|||
next();
|
||||
});
|
||||
|
||||
router.use(authMiddleware);
|
||||
// The middleware above may have assigned the anonymous preview identity.
|
||||
// authMiddleware knows nothing about that — it only looks for a token — so
|
||||
// calling it unconditionally here rejected exactly the requests preview exists
|
||||
// to allow, and the feature never worked at all. authMiddleware itself stays
|
||||
// strict: it is used everywhere else and must keep refusing anyone without a
|
||||
// credential. Only the preview identity, which this router assigns on
|
||||
// allow-listed paths when an admin has opted in, may pass.
|
||||
router.use(function(req, res, next) {
|
||||
if (req.user && req.user.preview) return next();
|
||||
return authMiddleware(req, res, next);
|
||||
});
|
||||
|
||||
var MAX_SAVED_CHATS_PER_USER = 100;
|
||||
var MAX_SAVED_CHAT_TITLE = 160;
|
||||
|
|
|
|||
|
|
@ -11,7 +11,11 @@ var { authMiddleware } = require('../middleware/auth');
|
|||
var logger = require('../utils/logger');
|
||||
var cryptoUtil = require('../utils/crypto');
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. These routers are mounted on /api, so a
|
||||
// bare router.use(authMiddleware) gated every /api path — including routes
|
||||
// belonging to routers mounted after it. That is what kept the signed-out
|
||||
// assistant preview returning 401 no matter what the admin setting said.
|
||||
router.use('/diagrams', authMiddleware);
|
||||
|
||||
var MAX_TITLE = 200;
|
||||
var MAX_SOURCE = 50000;
|
||||
|
|
|
|||
|
|
@ -10,7 +10,10 @@ var db = require('../db/database');
|
|||
var { authMiddleware } = require('../middleware/auth');
|
||||
var logger = require('../utils/logger');
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. Mounted on /api, a bare
|
||||
// router.use(authMiddleware) gated every /api path — including routes owned by
|
||||
// routers mounted after it in server.js.
|
||||
router.use('/documents', authMiddleware);
|
||||
|
||||
var upload = multer({
|
||||
storage: multer.memoryStorage(),
|
||||
|
|
|
|||
|
|
@ -13,7 +13,10 @@ var { authMiddleware } = require('../middleware/auth');
|
|||
var logger = require('../utils/logger');
|
||||
var { wrapUserText, INJECTION_GUARD } = require('../utils/promptSafe');
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. Mounted on /api, a bare
|
||||
// router.use(authMiddleware) gated every /api path — including routes owned by
|
||||
// routers mounted after it in server.js.
|
||||
router.use('/dont-miss', authMiddleware);
|
||||
|
||||
function extractJson(raw) {
|
||||
var t = String(raw || '').trim();
|
||||
|
|
|
|||
|
|
@ -19,7 +19,10 @@ var { authMiddleware } = require('../middleware/auth');
|
|||
var logger = require('../utils/logger');
|
||||
var { wrapUserText, INJECTION_GUARD } = require('../utils/promptSafe');
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. Mounted on /api, a bare
|
||||
// router.use(authMiddleware) gated every /api path — including routes owned by
|
||||
// routers mounted after it in server.js.
|
||||
router.use('/ed-encounters', authMiddleware);
|
||||
|
||||
// Models sometimes wrap JSON in ```json fences or prepend prose. Strip the
|
||||
// fence and recover the JSON object between the first { and last }.
|
||||
|
|
|
|||
|
|
@ -9,7 +9,11 @@ var { authMiddleware } = require('../middleware/auth');
|
|||
var logger = require('../utils/logger');
|
||||
var cryptoUtil = require('../utils/crypto');
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. These routers are mounted on /api, so a
|
||||
// bare router.use(authMiddleware) gated every /api path — including routes
|
||||
// belonging to routers mounted after it. That is what kept the signed-out
|
||||
// assistant preview returning 401 no matter what the admin setting said.
|
||||
router.use('/encounters', authMiddleware);
|
||||
|
||||
// Normalise partial_data to a string before encrypting so JSON objects and
|
||||
// raw strings round-trip identically. Decryption returns the original string.
|
||||
|
|
|
|||
|
|
@ -14,7 +14,11 @@ var transfer = require('../utils/extensionTransfer');
|
|||
|
||||
var upload = multer({ storage: multer.memoryStorage(), limits: { fileSize: 1024 * 1024 } });
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. These routers are mounted on /api, so a
|
||||
// bare router.use(authMiddleware) gated every /api path — including routes
|
||||
// belonging to routers mounted after it. That is what kept the signed-out
|
||||
// assistant preview returning 401 no matter what the admin setting said.
|
||||
router.use('/extensions', authMiddleware);
|
||||
|
||||
function sanitizeType(t) {
|
||||
return transfer.sanitizeType(t);
|
||||
|
|
|
|||
|
|
@ -2,7 +2,13 @@ const router = require('express').Router();
|
|||
const { authMiddleware, moderatorMiddleware, adminMiddleware } = require('../middleware/auth');
|
||||
const images = require('../utils/generatedImages');
|
||||
const db = require('../db/database');
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefixes. Mounted on /api, a bare
|
||||
// router.use(authMiddleware) gated every /api path, including routes owned by
|
||||
// routers mounted after it in server.js — documents, audio backups, billing
|
||||
// and well visits all sat behind this line by accident.
|
||||
['/generated-images', '/image-jobs', '/admin/image-settings'].forEach(function (prefix) {
|
||||
router.use(prefix, authMiddleware);
|
||||
});
|
||||
function fail(res, e) { res.status(e.statusCode || 503).json({ error: e.statusCode ? e.message : 'Image service unavailable' }); }
|
||||
async function sendAsset(req, res, download) {
|
||||
// ?w= serves a stored preview of the SAME asset. Permission is checked against
|
||||
|
|
|
|||
|
|
@ -9,7 +9,11 @@ var { authMiddleware } = require('../middleware/auth');
|
|||
var logger = require('../utils/logger');
|
||||
var cryptoUtil = require('../utils/crypto');
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. These routers are mounted on /api, so a
|
||||
// bare router.use(authMiddleware) gated every /api path — including routes
|
||||
// belonging to routers mounted after it. That is what kept the signed-out
|
||||
// assistant preview returning 401 no matter what the admin setting said.
|
||||
router.use('/memories', authMiddleware);
|
||||
router.use('/memories', require('../utils/policy').requireFeature('memories'));
|
||||
|
||||
// Decrypt a row's user-facing fields. Safe against legacy plaintext rows —
|
||||
|
|
|
|||
|
|
@ -36,7 +36,11 @@ function toHtmlBody(s) {
|
|||
}
|
||||
}
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. These routers are mounted on /api, so a
|
||||
// bare router.use(authMiddleware) gated every /api path — including routes
|
||||
// belonging to routers mounted after it. That is what kept the signed-out
|
||||
// assistant preview returning 401 no matter what the admin setting said.
|
||||
router.use('/notes', authMiddleware);
|
||||
|
||||
var MAX_TITLE = 200;
|
||||
var MAX_BODY = 50000; // 50 KB of rich-text HTML is plenty for a clinical note
|
||||
|
|
|
|||
|
|
@ -11,7 +11,10 @@ var { authMiddleware } = require('../middleware/auth');
|
|||
var logger = require('../utils/logger');
|
||||
var { wrapUserText, INJECTION_GUARD } = require('../utils/promptSafe');
|
||||
|
||||
router.use(authMiddleware);
|
||||
// Scoped to this router's own prefix. Mounted on /api, a bare
|
||||
// router.use(authMiddleware) gated every /api path — including routes owned by
|
||||
// routers mounted after it in server.js.
|
||||
router.use('/patient-education', authMiddleware);
|
||||
|
||||
// ── POST /patient-education ─────────────────────────────────
|
||||
// Body:
|
||||
|
|
|
|||
|
|
@ -554,3 +554,41 @@ test('actual native admin script disables selected default, displays backend rep
|
|||
assertDisabledAbsent();
|
||||
assert.ok(Array.from(select.options).some(o => o.value === 'other'), 'enabled custom model retained');
|
||||
});
|
||||
|
||||
const readSource = file => fs.readFileSync(path.join(root, file), 'utf8');
|
||||
|
||||
test('a router mounted on /api must not gate the whole namespace', () => {
|
||||
// These routers are all mounted on '/api', so `router.use(authMiddleware)`
|
||||
// with no path applies to every /api request that reaches them — including
|
||||
// routes owned by routers mounted further down server.js. extensions.js did
|
||||
// exactly that from line 295, which is why the signed-out assistant preview
|
||||
// returned 401 no matter what the admin setting said: the request never got
|
||||
// as far as the preview middleware. Each gate must name its own prefix.
|
||||
const server = readSource('server.js');
|
||||
const mounted = [...server.matchAll(/app\.use\('\/api', require\('\.\/src\/routes\/([\w-]+)'\)\)/g)]
|
||||
.map(m => m[1]);
|
||||
assert.ok(mounted.length > 10, 'expected the /api routers to be found');
|
||||
for (const name of mounted) {
|
||||
let source;
|
||||
try { source = readSource('src/routes/' + name + '.js'); } catch (e) { continue; }
|
||||
assert.doesNotMatch(source, /^router\.use\(\s*authMiddleware\s*\)/m,
|
||||
name + '.js gates every /api path; scope it, e.g. router.use(\'/' + name + '\', authMiddleware)');
|
||||
}
|
||||
});
|
||||
|
||||
test('the signed-out preview is reachable, and stays narrow', () => {
|
||||
const route = readSource('src/routes/clinicalAssistant.js');
|
||||
// authMiddleware only ever looks for a token, so calling it unconditionally
|
||||
// after the preview identity was assigned rejected the very requests preview
|
||||
// exists to serve. Only the preview identity may skip it.
|
||||
assert.match(route, /if \(req\.user && req\.user\.preview\) return next\(\);/);
|
||||
assert.match(route, /return authMiddleware\(req, res, next\);/);
|
||||
// Allow-listed by exact path: a route added later is private unless someone
|
||||
// puts it on this list deliberately.
|
||||
const list = route.slice(route.indexOf('var PREVIEW_PATHS'), route.indexOf('var PREVIEW_USER'));
|
||||
assert.match(list, /'\/clinical-assistant\/chat'/);
|
||||
assert.match(list, /'\/clinical-assistant\/chat\/stream'/);
|
||||
assert.doesNotMatch(list, /saved-chats|\/config|\/images/);
|
||||
// A preview visitor has no identity, so nothing can be owned or billed.
|
||||
assert.match(route, /PREVIEW_USER = Object\.freeze\(\{ id: null, preview: true/);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue