pediatric-ai-scribe-v3/test/backend-hardening.test.js
Daniel 15a8b399ba
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 57s
Forgejo Docker Build / Root app tests (push) Successful in 53s
Forgejo Android APK / Build signed APK (push) Successful in 1m59s
Forgejo Docker Build / Build Docker image (push) Has been cancelled
Forgejo Docker Build / Deploy to the host (push) Has been cancelled
feat: slides are built by pandoc from markdown, with a reference template
pptxgenjs is gone, and with it 269 lines of hand-rolled markdown parsing.

It stretched every image. Reading the slide XML it emitted shows why: it writes
the target box verbatim with <a:stretch/> and a no-op srcRect, so a 200x800
image handed an 11.8x3.9 box came out 1:4 squashed to 3:1. It could not do
better — it never measures an image, and its own getSizeFromImage is commented
out and marked "currently unused", reaching for a package called sizeof that
does not exist. pandoc measures them: a 300x175 source renders at aspect 1.714
and a 160x360 at 0.445, verified by rendering the deck to PDF and looking at it.

Tables, ordered and unordered lists, bold, italic and subscripts all come out
natively, and the fonts, palette and slide layouts come from
assets/learning/slides-reference.pptx. Design now lives in that file: restyling
the decks means editing it in PowerPoint, not editing this route.

Only images the requester owns can reach a deck. pandoc resolves an image link
against the filesystem, so a markdown link naming any local path would read that
file into the presentation. Images are fetched by id through the ownership
check, written into a per-request temporary directory under names we choose, and
every image link that did not resolve is removed rather than passed through. The
directory is removed in a finally block, and the conversion has a 60s timeout so
it cannot hang a request.

pandoc is in the image rather than a sidecar, because an export must not fail
for reasons outside this container. It costs 197MB (307 -> 504).

Removing pptxgenjs also removed image-size, and with it both high-severity
advisories — GHSA-w3rx-r6r6-pgpr and GHSA-5p2g-fcmc-qvqq, ICNS/JXL/HEIF parser
denial of service, ranged <=2.0.2 with no fixed release to upgrade to. npm audit
goes from 2 high and 2 moderate to 2 moderate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
2026-09-11 13:25:24 +02:00

332 lines
18 KiB
JavaScript

const { test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
function read(relativePath) {
return fs.readFileSync(path.join(__dirname, '..', relativePath), 'utf8');
}
test('redactor removes PHI and common secret patterns from logs', () => {
const { redact } = require('../src/utils/redact');
const input = 'Authorization: Bearer abc.def.ghi password=secret123 token=tok_1234567890 email test@example.com MRN 123456789';
const output = redact(input);
assert.doesNotMatch(output, /abc\.def\.ghi/);
assert.doesNotMatch(output, /secret123/);
assert.doesNotMatch(output, /tok_1234567890/);
assert.doesNotMatch(output, /test@example\.com/);
assert.doesNotMatch(output, /123456789/);
assert.match(output, /\[REDACTED\]|\[JWT\]/);
});
test('URL safety helper blocks private network targets', () => {
const { isPrivateIp } = require('../src/utils/urlSafety');
assert.equal(isPrivateIp('127.0.0.1'), true);
assert.equal(isPrivateIp('10.0.0.5'), true);
assert.equal(isPrivateIp('172.16.0.1'), true);
assert.equal(isPrivateIp('192.168.1.10'), true);
assert.equal(isPrivateIp('169.254.169.254'), true);
assert.equal(isPrivateIp('100.64.0.1'), true);
assert.equal(isPrivateIp('::1'), true);
assert.equal(isPrivateIp('fe80::1'), true);
assert.equal(isPrivateIp('8.8.8.8'), false);
assert.equal(isPrivateIp('2606:4700:4700::1111'), false);
});
test('Nextcloud/WebDAV routes enforce SSRF guard and redirect blocking', () => {
const nextcloud = read('src/routes/nextcloud.js');
const learningAI = read('src/routes/learningAI.js');
assert.match(nextcloud, /assertSafeHttpsUrl\(cleanUrl, 'Nextcloud URL'\)/);
assert.match(nextcloud, /assertSafeHttpsUrl\(user\.nextcloud_url, 'Nextcloud URL'\)/);
assert.match(nextcloud, /maxRedirects: 0/);
assert.match(nextcloud, /encodeURIComponent\(username\)/);
assert.match(learningAI, /assertSafeHttpsUrl\(user\.nextcloud_url, 'Nextcloud URL'\)/);
assert.match(learningAI, /maxRedirects: 0/);
assert.match(learningAI, /encodeURIComponent\(user\.nextcloud_user\)/);
});
test('logs and audits avoid unbounded limits and PHI-prone details', () => {
const logs = read('src/routes/logs.js');
const encounters = read('src/routes/encounters.js');
const documents = read('src/routes/documents.js');
const learningAI = read('src/routes/learningAI.js');
assert.match(logs, /function clampLimit/);
assert.match(logs, /clampLimit\(req\.query\.limit, 50, 200\)/);
assert.match(logs, /logger\.warn\('client_error'/);
assert.match(logs, /redact\(trimField\(e\.stack, 1200\)\)/);
assert.doesNotMatch(logs, /console\.error\('\[CLIENT ERROR\]'/);
assert.doesNotMatch(encounters, /Saved encounter: ' \+ \(label/);
assert.doesNotMatch(encounters, /Loaded encounter: ' \+ \(row\.label/);
assert.doesNotMatch(documents, /Uploaded: ' \+ \(req\.file/);
assert.doesNotMatch(learningAI, /Char context/);
});
// adminMiddleware only checks req.user.role; without authMiddleware having run,
// req.user is undefined. These routes were protected only because adminConfig
// happens to be mounted on /api/admin first and guards the whole path.
test('admin routers state their own authentication, not mount order', () => {
const fs = require('node:fs');
const path = require('node:path');
const root = path.join(__dirname, '..');
for (const file of ['adminMilestones.js', 'admin.js', 'adminConfig.js', 'adminDocs.js']) {
const src = fs.readFileSync(path.join(root, 'src/routes', file), 'utf8');
assert.match(src, /router\.use\(authMiddleware\);/, file + ' authenticates every route it serves');
}
const auth = fs.readFileSync(path.join(root, 'src/middleware/auth.js'), 'utf8');
assert.match(auth, /async function adminMiddleware\(req, res, next\) \{\s*\n\s*if \(!req\.user \|\| req\.user\.role !== 'admin'\)/,
'and the role check stays a role check, so it fails closed on its own');
});
// Three S3 schemes grew separately — S3_*, GENERATED_IMAGES_S3_*, and later
// audio backups — which is why pointing the app at a different MinIO meant
// hunting through three files. One resolver answers it for every purpose.
test('object storage resolves the same way for every purpose', () => {
const storage = require('../src/utils/objectStorage');
// One endpoint plus a bucket name per purpose is enough for all of them.
const shared = { S3_ENDPOINT: 'https://s3.example.com', S3_ACCESS_KEY: 'AK', S3_SECRET_KEY: 'SK',
S3_BUCKET_AUDIO_BACKUPS: 'audio', S3_BUCKET_GENERATED_IMAGES: 'images', S3_BUCKET: 'docs' };
for (const [purpose, bucket] of [['audio-backups', 'audio'], ['generated-images', 'images'], ['documents', 'docs']]) {
const resolved = storage.settingsFor(purpose, shared);
assert.equal(resolved.bucket, bucket, purpose + ' finds its bucket');
assert.equal(resolved.endpoint, 'https://s3.example.com');
assert.deepEqual(resolved.credentials, { accessKeyId: 'AK', secretAccessKey: 'SK' });
}
// A purpose that needs its own account still overrides everything.
const overridden = storage.settingsFor('audio-backups',
Object.assign({}, shared, { AUDIO_BACKUPS_S3_ENDPOINT: 'http://assets:9000', AUDIO_BACKUPS_S3_BUCKET: 'audio-backups' }));
assert.equal(overridden.endpoint, 'http://assets:9000');
assert.equal(overridden.bucket, 'audio-backups');
// Existing deployments keep working untouched, including the old key names.
const legacy = storage.settingsFor('documents',
{ S3_BUCKET: 'd', S3_REGION: 'us-west-004', S3_ENDPOINT: 'https://b2', S3_ACCESS_KEY_ID: 'A', S3_SECRET_ACCESS_KEY: 'B' });
assert.equal(legacy.region, 'us-west-004');
assert.deepEqual(legacy.credentials, { accessKeyId: 'A', secretAccessKey: 'B' });
// Documents defaulted path-style off; a Backblaze endpoint must keep working.
assert.equal(legacy.forcePathStyle, false);
assert.equal(storage.settingsFor('audio-backups', { AUDIO_BACKUPS_S3_BUCKET: 'a', AUDIO_BACKUPS_S3_ENDPOINT: 'http://assets:9000' }).forcePathStyle, true,
'but MinIO needs it, so a custom endpoint turns it on where there is no older default');
// No bucket means "not configured" — never an error, since all of this is optional.
assert.equal(storage.settingsFor('audio-backups', {}), null);
assert.equal(storage.isConfigured('audio-backups', {}), false);
// A mounted secret must not be overridden by an inherited environment value.
const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path');
const keyFile = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'objstore-')), 'key');
fs.writeFileSync(keyFile, 'from-file\n');
const fileWins = storage.settingsFor('audio-backups',
{ AUDIO_BACKUPS_S3_BUCKET: 'a', AUDIO_BACKUPS_S3_ACCESS_KEY: 'inline', AUDIO_BACKUPS_S3_ACCESS_KEY_FILE: keyFile, AUDIO_BACKUPS_S3_SECRET_KEY: 'S' });
assert.equal(fileWins.credentials.accessKeyId, 'from-file');
});
// .env.example listed 18 of the 67 variables the app reads, so anyone setting
// up a deployment had to find the rest by reading source.
test('.env.example documents every variable the app reads', () => {
const fs = require('node:fs');
const path = require('node:path');
const root = path.join(__dirname, '..');
const read = f => fs.readFileSync(path.join(root, f), 'utf8');
const sources = ['server.js'].concat(
fs.readdirSync(path.join(root, 'src'), { recursive: true })
.filter(f => String(f).endsWith('.js')).map(f => path.join('src', String(f))));
const used = new Set();
for (const file of sources) {
for (const m of read(file).matchAll(/process\.env\.([A-Z_0-9]+)/g)) used.add(m[1]);
}
// Set by node:test in its own child processes, never by a deployment.
used.delete('NODE_TEST_CONTEXT');
const documented = new Set(
[...read('.env.example').matchAll(/^#?\s*([A-Z_0-9]+)=/gm)].map(m => m[1]));
const missing = [...used].filter(name => !documented.has(name)).sort();
assert.deepEqual(missing, [], 'undocumented variables: ' + missing.join(', '));
});
// registration_enabled is open-or-closed. Invite-only is the middle setting,
// and it has to hold up against someone probing codes.
test('registration invites are single-use, expiring, and safe to store', () => {
const fs = require('node:fs');
const path = require('node:path');
const root = path.join(__dirname, '..');
const read = f => fs.readFileSync(path.join(root, f), 'utf8');
const invites = require('../src/utils/registrationInvites');
const migration = read('migrations/1780100000000_registration-invites.js');
const auth = read('src/routes/auth.js');
// A code must not be recoverable from a database dump.
assert.match(migration, /code_hash TEXT NOT NULL UNIQUE/);
assert.doesNotMatch(migration, /\bcode TEXT\b/, 'the code itself is never stored');
assert.equal(invites.hash('abcd-efgh'), invites.hash('ABCDEFGH'), 'hashing normalises case and separators');
assert.notEqual(invites.hash('a'), 'a');
// Generated codes avoid characters that are misread when typed from a screen.
const code = invites.generateCode();
assert.match(code, /^[0-9A-Z]{4}-[0-9A-Z]{4}-[0-9A-Z]{4}-[0-9A-Z]{4}$/);
assert.doesNotMatch(code, /[ILOU]/, 'no I, L, O or U');
assert.notEqual(invites.generateCode(), invites.generateCode());
// Expiry is bounded, and a nonsense value falls back rather than throwing.
assert.equal(invites.ttlDays(undefined), invites.DEFAULT_TTL_DAYS);
assert.equal(invites.ttlDays(0), invites.DEFAULT_TTL_DAYS);
assert.equal(invites.ttlDays('nonsense'), invites.DEFAULT_TTL_DAYS);
assert.equal(invites.ttlDays(10000), invites.MAX_TTL_DAYS);
// The claim is one conditional UPDATE, so two registrations racing the same
// code cannot both succeed.
const claim = read('src/utils/registrationInvites.js');
assert.match(claim, /UPDATE registration_invites SET used_at = NOW\(\), used_by = \$1 /);
assert.match(claim, /WHERE code_hash = \$2 AND used_at IS NULL AND revoked_at IS NULL AND expires_at > NOW\(\)/);
assert.match(claim, /RETURNING id/);
// Losing that race must not leave a free account behind.
assert.match(auth, /if \(!claimedInvite\) \{[\s\S]{0,120}DELETE FROM users WHERE id = \?/);
// And the rejection must not say which of the four reasons applied.
assert.match(auth, /may have expired, been revoked, or already been used/);
});
// With several admins, everything in the panel was editable by all of them.
// Lockdown separates running the service from changing how it behaves.
test('admin lockdown refuses configuration writes at the server', () => {
const lockdown = require('../src/utils/adminLockdown');
const off = {};
const on = { ADMIN_LOCKDOWN: 'true' };
// It is an environment variable, not a setting: a setting could be switched
// off by the very admin it restrains.
assert.equal(lockdown.enabled(off), false);
assert.equal(lockdown.enabled({ ADMIN_LOCKDOWN: 'false' }), false);
assert.equal(lockdown.enabled(on), true);
// Off, nothing is locked.
assert.equal(lockdown.isLocked('prompt.hpi', off), false);
// On, configuration is locked and day-to-day operation is not.
for (const key of ['prompt.hpi', 'clinical_assistant.chat_model', 'models.default',
'tts.voice', 'stt.model', 'embeddings.model', 'smtp.host', 'email.verify.subject']) {
assert.equal(lockdown.isLocked(key, on), true, key + ' is locked');
}
for (const key of ['announcement.text', 'registration_enabled',
'registration_invite_only', 'feature.memories', 'site.name']) {
assert.equal(lockdown.isLocked(key, on), false, key + ' stays editable');
}
// A setting added later is locked until someone decides otherwise, rather
// than defaulting to open.
assert.equal(lockdown.isLocked('something.invented.later', on), true);
const fs = require('node:fs');
const path = require('node:path');
const admin = fs.readFileSync(path.join(__dirname, '..', 'src/routes/adminConfig.js'), 'utf8');
// One gate, not a check per route: a route added later cannot miss it.
assert.match(admin, /if \(!lockdown\.enabled\(\) \|\| req\.method === 'GET' \|\| req\.method === 'HEAD'\) return next\(\);/);
assert.match(admin, /if \(OPERATIONAL_WRITE\.test\(req\.path\)\) return next\(\);/);
assert.match(admin, /return res\.status\(403\)\.json\(\{ error: lockdown\.refusal/);
// And the per-key rule for the generic settings route.
assert.match(admin, /if \(lockdown\.isLocked\(key\)\) \{\s*\n\s*return res\.status\(403\)/);
// The panel learns the state from a response it already fetches, rather than
// adding a request of its own on every admin open.
assert.match(admin, /invites: await invites\.list\(\), inviteOnly: await invites\.inviteOnly\(\), lockdown: lockdown\.state\(\)/);
const panel = fs.readFileSync(path.join(__dirname, '..', 'public/js/admin.js'), 'utf8');
assert.match(panel, /window\.applyAdminLockdown\(data\.lockdown\)/);
});
// A citation that resolves to nothing is never rendered as a link, so without
// counting it, nobody ever learns it happened.
test('citation quality is measured on the server, where answer and sources both exist', () => {
const audit = require('../src/utils/citationAudit');
const sources = [{ number: 1 }, { number: 2 }, { number: 3 }];
const good = audit.audit('Bronchiolitis is managed supportively [1]. Steroids do not help [2, 3].', sources);
assert.equal(good.cited, 3);
assert.deepEqual(good.unverifiableNumbers, []);
// The case that motivated this: a number no source matched.
const over = audit.audit('Supportive care [1] and also this [7].', sources);
assert.equal(over.cited, 2);
assert.deepEqual(over.unverifiableNumbers, [7]);
// Repeats are one problem, not three.
assert.deepEqual(audit.audit('[7] and [7] and [7]', sources).unverifiableNumbers, [7]);
// A bracketed number inside code is content, not a citation.
assert.deepEqual(audit.audit('```\nconst a = arr[7];\n```', sources).unverifiableNumbers, []);
assert.deepEqual(audit.audit('Use `arr[7]` here.', sources).unverifiableNumbers, []);
// Sources are matched on their assigned number, the same way rendering does.
assert.deepEqual(audit.audit('[5]', [{ number: 5 }]).unverifiableNumbers, []);
// Recording must never be able to fail an answer.
assert.doesNotThrow(() => audit.record(null, null));
assert.doesNotThrow(() => audit.record('[1]', undefined));
const fs = require('node:fs');
const path = require('node:path');
const route = fs.readFileSync(path.join(__dirname, '..', 'src/routes/clinicalAssistant.js'), 'utf8');
// Not awaited: the clinician is waiting for the answer.
// One tracker for both answer paths — the streaming one and the fallback the
// client uses when streaming fails. Auditing only the first would make
// exactly the answers produced under failure invisible.
assert.match(route, /function trackCitations\(req, question, answer, sources\)/);
assert.match(route, /trackCitations\(req, prepared\.message, answer, safeSources\);/, 'streaming');
assert.match(route, /trackCitations\(req, prepared\.message, answer, fallbackSources\);/, 'fallback');
// Loaded on demand and allowed to be absent: observation must never be able
// to fail an answer.
assert.match(route, /function citationTracker\(\)/);
assert.match(route, /if \(!tracker\) return;/);
assert.match(route, /tracker\.store\(req\.user\.id, question, result, sources\);/);
});
test('slides are built by pandoc, from markdown, with a reference template', () => {
// pptxgenjs is gone. It stretched every image — reading the slide XML it
// emitted showed the target box verbatim with <a:stretch/>, so a 200x800
// image in an 11.8x3.9 box came out 1:4 squashed to 3:1 — and it could not
// do better, because it never measures an image; its own getSizeFromImage is
// commented out as unused. pandoc measures them: rendered and inspected, a
// 300x175 source produced aspect 1.714 and a 160x360 produced 0.445.
//
// Removing it also took image-size with it, and with that both high-severity
// advisories (GHSA-w3rx-r6r6-pgpr, GHSA-5p2g-fcmc-qvqq), which had no fixed
// release to upgrade to.
const src = read('src/routes/learningAI.js');
assert.doesNotMatch(src, /pptxgenjs|PptxGenJS/, 'nothing builds slides by hand any more');
assert.match(src, /execFile\('pandoc', \['deck\.md', '--reference-doc=' \+ reference, '-o', 'deck\.pptx'\]/);
assert.match(src, /timeout: 60000/, 'a conversion cannot hang the request');
// Design lives in the template, so restyling is editing a file in PowerPoint.
assert.match(src, /assets', 'learning', 'slides-reference\.pptx'/);
assert.ok(require('node:fs').existsSync(
require('node:path').join(__dirname, '..', 'assets', 'learning', 'slides-reference.pptx')),
'and that template ships with the app');
// pandoc must be in the image, or every export fails at runtime.
assert.match(read('Dockerfile'), /apk add --no-cache ffmpeg curl jq pandoc-cli/);
const pkg = JSON.parse(read('package.json'));
assert.ok(!pkg.dependencies['pptxgenjs'], 'and the library is no longer a dependency');
assert.ok(!pkg.dependencies['image-size'], 'nor the parser it dragged in');
});
test('a deck can only embed images the requester owns', () => {
// pandoc resolves an image link against the filesystem, so a markdown link
// naming any local path would read that file into the deck. Only the images
// fetched by id for this user are written into the working directory, and
// every other image link is dropped rather than passed through.
const src = read('src/routes/learningAI.js');
assert.match(src, /var image = await require\('\.\.\/utils\/generatedImages'\)\.service\(\)\.asset\(id, req\.user\);/,
'ownership is checked when fetching');
assert.match(src, /return local \? '!\[' \+ alt \+ '\]\(' \+ local \+ '\)' : '';/,
'an unresolved link is removed, not passed to pandoc');
// The working directory is per request and always cleaned up.
assert.match(src, /workdir = await fsp\.mkdtemp\(/);
assert.match(src, /\} finally \{[\s\S]{0,200}rm\(workdir, \{ recursive: true, force: true \}\)/);
});