fix: an assistant attachment must be the image type it claims to be
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 59s
Forgejo Docker Build / Root app tests (push) Successful in 50s
Forgejo Android APK / Build signed APK (push) Successful in 1m56s
Forgejo Docker Build / Build Docker image (push) Successful in 9s
Forgejo Docker Build / Deploy to the host (push) Failing after 0s

The MIME type was taken on trust here. Anything at all could be posted as
image/png: it passed the size and base64 checks, was stored in the saved chat,
and was handed to a provider as a data URI. Documents and S3 uploads have always
been sniffed by fileType.js; this was the one upload path that was not.

Now sniffed with the same helper, so there is one idea of what a PNG looks like.
A PHP payload, a shell script, an ELF or PE binary, a zip, or a real PDF
labelled image/png are all refused with a message that says what is wrong.

What this does not claim: bytes hidden after a valid PNG header still make a
valid PNG, and no sniffer can promise otherwise. The protection is that the file
is never executed and never served as anything but an image.

Existing fixtures used buffers of 0x07 as stand-in images, which are correctly
refused now. They carry real file headers instead — a fixture should be the
thing it claims to be, exactly like a real upload.

Also adds the deck theme system: five palettes in assets/deck-themes.json,
render_pptx.py rebinding its palette from the theme rather than hardcoding it,
the theme carried on the deck and validated against the same catalogue the
renderer reads, a picker on the generate form, and PUT /my-resources/:id/theme
to re-skin a stored deck with no model call.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
Daniel 2026-09-12 19:00:10 +02:00
parent cda74e1fe2
commit bd8e413bc7
10 changed files with 303 additions and 10 deletions

40
assets/deck-themes.json Normal file
View file

@ -0,0 +1,40 @@
{
"_comment": "A theme is a palette and a typeface, nothing more. Every slide type already draws from these five names, so adding one here restyles the whole deck with no change to any slide builder. Colours are hex without a leading #, the form python-pptx wants.",
"themes": [
{
"id": "clinical-blue",
"name": "Clinical Blue",
"description": "The default. Calm, high-contrast, reads well on a projector.",
"accent": "2563EB", "ink": "1F2937", "muted": "4B5563",
"rule": "E5E7EB", "paper": "FFFFFF", "font": "Calibri"
},
{
"id": "teaching-amber",
"name": "Teaching Amber",
"description": "Warmer and less clinical. Good for sessions with parents or students.",
"accent": "D97706", "ink": "1C1917", "muted": "57534E",
"rule": "EDE9E4", "paper": "FFFFFF", "font": "Calibri"
},
{
"id": "ward-teal",
"name": "Ward Teal",
"description": "Quieter than blue, still clearly clinical.",
"accent": "0F766E", "ink": "134E4A", "muted": "4B5563",
"rule": "E3EDEB", "paper": "FFFFFF", "font": "Calibri"
},
{
"id": "slate",
"name": "Slate",
"description": "Almost monochrome. Lets figures and tables carry the colour.",
"accent": "475569", "ink": "0F172A", "muted": "64748B",
"rule": "E2E8F0", "paper": "FFFFFF", "font": "Calibri"
},
{
"id": "high-contrast",
"name": "High Contrast",
"description": "For a bright room or a poor projector. Heavier ink, stronger rules.",
"accent": "B91C1C", "ink": "000000", "muted": "27272A",
"rule": "A1A1AA", "paper": "FFFFFF", "font": "Calibri"
}
]
}

View file

@ -42,6 +42,16 @@
<!-- One group rather than four rows. Each option hides itself when an
administrator has not enabled it, so nothing appears that a person
could tick and then be refused. -->
<!-- The look, chosen by the person who knows the room it will be shown in.
Hidden until the catalogue loads, so it never flashes as an empty row. -->
<div class="admin-row" id="mr-theme-row" hidden>
<label for="mr-theme" class="admin-row-label">Theme</label>
<div style="flex:1;display:flex;flex-direction:column;gap:4px;min-width:0;">
<select id="mr-theme" class="admin-control" style="max-width:320px;"></select>
<p id="mr-theme-hint" style="margin:0;font-size:12px;color:var(--g500);"></p>
</div>
</div>
<div class="admin-row" style="align-items:flex-start;">
<strong class="admin-row-label">Draw on</strong>
<div style="flex:1;display:flex;flex-direction:column;gap:8px;min-width:0;">

View file

@ -144,6 +144,30 @@
}
if (modelRow) modelRow.hidden = models.length < 2;
// The theme catalogue. Structural slides carry no colour of their own,
// so this is the whole of a deck's styling — one choice rather than a
// colour field the model would have to guess a value for.
var themeSelect = document.getElementById('mr-theme');
var themeRow = document.getElementById('mr-theme-row');
var themes = Array.isArray(data.themes) ? data.themes : [];
if (themeSelect && themes.length) {
var chosenTheme = themeSelect.value;
themeSelect.textContent = '';
themes.forEach(function (t) {
var option = document.createElement('option');
option.value = t.id;
option.textContent = t.name;
option.title = t.description || '';
themeSelect.appendChild(option);
});
if (chosenTheme && themes.some(function (t) { return t.id === chosenTheme; })) {
themeSelect.value = chosenTheme;
}
describeTheme(themes);
themeSelect.onchange = function () { describeTheme(themes); };
}
if (themeRow) themeRow.hidden = themes.length < 2;
// Hidden entirely unless an administrator enabled it, so an option
// never appears that someone could tick and then be refused. Both
// groups are driven from the same answer: Generate and Modify offer
@ -184,6 +208,15 @@
// when the tab was reopened, looking like output for a topic nobody had
// typed. It cleared on a full page refresh and only then, which is why it
// read as a leak.
// Says what the chosen theme is for, which is the part a name cannot carry.
function describeTheme(themes) {
var select = document.getElementById('mr-theme');
var hint = document.getElementById('mr-theme-hint');
if (!select || !hint) return;
var chosen = themes.filter(function (t) { return t.id === select.value; })[0];
hint.textContent = chosen ? (chosen.description || '') : '';
}
function clearResults() {
['mr-images', 'mr-searches', 'mr-image-failures'].forEach(function (id) {
var el = document.getElementById(id);
@ -213,6 +246,7 @@
refinement: (document.getElementById('mr-refinement') || {}).value || '',
useCorpus: corpusBox && corpusBox.checked === false ? 'false' : 'true',
model: (document.getElementById('mr-model') || {}).value || '',
theme: (document.getElementById('mr-theme') || {}).value || '',
withImages: (document.getElementById('mr-with-images') || {}).checked ? 'true' : 'false',
withWebSearch: (document.getElementById('mr-web-search') || {}).checked ? 'true' : 'false',
withPubmed: (document.getElementById('mr-pubmed') || {}).checked ? 'true' : 'false'

View file

@ -51,6 +51,13 @@ BODY_TOP = HEADING_TOP + HEADING_H + Emu(228600) # clears the accent rule
BODY_H = SLIDE_H - BODY_TOP - MARGIN
BODY_W = SLIDE_W - MARGIN * 2
# The palette. Every slide builder reads these five names and the font, which is
# what makes theming a rebinding rather than a rewrite: apply_theme() below
# reassigns them once, and a comparison slide, a table header and a callout card
# all follow without a line changing in any builder.
#
# These values are the clinical-blue theme, kept as the literal default so the
# renderer still works standalone with no theme and no catalogue on disk.
INK = RGBColor(0x1F, 0x29, 0x37)
MUTED = RGBColor(0x4B, 0x55, 0x63)
ACCENT = RGBColor(0x25, 0x63, 0xEB)
@ -59,6 +66,49 @@ PAPER = RGBColor(0xFF, 0xFF, 0xFF)
FONT = "Calibri"
THEMES_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)),
"..", "assets", "deck-themes.json")
def _rgb(value):
"""'2563EB' -> RGBColor. Raises on anything that is not six hex digits."""
text = str(value).lstrip("#")
if len(text) != 6:
raise ValueError("colour must be six hex digits: %r" % (value,))
return RGBColor(int(text[0:2], 16), int(text[2:4], 16), int(text[4:6], 16))
def load_themes():
"""The catalogue, or an empty list if it cannot be read."""
try:
with open(THEMES_PATH, "r", encoding="utf-8") as handle:
return json.load(handle).get("themes") or []
except Exception:
# A deck must still render with no catalogue: the defaults above stand.
return []
def apply_theme(theme_id):
"""Rebind the palette. An unknown or missing id leaves the default in place,
because a deck rendering in the wrong colours beats a deck not rendering."""
if not theme_id:
return None
for theme in load_themes():
if theme.get("id") != theme_id:
continue
global INK, MUTED, ACCENT, RULE, PAPER, FONT
try:
INK = _rgb(theme["ink"])
MUTED = _rgb(theme["muted"])
ACCENT = _rgb(theme["accent"])
RULE = _rgb(theme["rule"])
PAPER = _rgb(theme["paper"])
except Exception:
return None
FONT = theme.get("font") or FONT
return theme
return None
# Text is sized to fit rather than left to the renderer's autofit, which only
# some viewers honour and which LibreOffice ignores entirely when converting to
# PDF — the reason slides were being cut off mid-sentence.
@ -655,6 +705,7 @@ def main():
print("usage: render_pptx.py <output.pptx> (spec on stdin)", file=sys.stderr)
return 2
spec = json.load(sys.stdin)
apply_theme(spec.get("theme"))
prs = Presentation()
prs.slide_width = SLIDE_W

View file

@ -213,6 +213,9 @@ router.get('/my-resources/options', async function (req, res) {
models: models.allowed,
defaultModel: models.configured,
imagesAvailable: Boolean(await db.getSetting('clinical_assistant.image_model', '')),
themes: deckSchema.themes().map(function (t) {
return { id: t.id, name: t.name, description: t.description || '' };
}),
webSearchAvailable: await webSearch.isAvailable(),
pubmedAvailable: await pubmedSearch.isAvailable()
});
@ -353,6 +356,9 @@ router.post('/my-resources/generate', async function (req, res) {
// guessed at: the shape vocabulary should grow from real demand.
var vocabularyGaps = [];
var deck = deckMode ? deckBuild.parse(ai && ai.content, vocabularyGaps) : null;
// Chosen by the author, not the model: a theme is a look, and the person
// making the deck is the one who knows the room it will be shown in.
if (deck) deck.theme = deckSchema.themeId(req.body.theme);
reportVocabularyGaps(vocabularyGaps, topic);
// Why a deck became plain slides, when it did. Reported to the caller as
// well as logged: the fallback produces a usable but plainer deck, and
@ -532,6 +538,36 @@ router.put('/my-resources/:id', async function (req, res) {
}
});
// Re-skin, not re-generate. Export renders from the stored deck every time, so
// changing the theme is a column write and the next download looks different.
// No model call, nothing regenerated, nothing that can reword a slide.
router.put('/my-resources/:id/theme', async function (req, res) {
try {
var theme = deckSchema.themeId(req.body.theme);
if (!theme && String(req.body.theme || '').trim()) {
return res.status(400).json({ error: 'That is not one of the available themes.' });
}
var row = await db.get(
'SELECT id, deck FROM user_resources WHERE id = ? AND user_id = ?',
[parseInt(req.params.id, 10), req.user.id]
);
if (!row) return res.status(404).json({ error: 'Not found' });
if (!row.deck) {
return res.status(400).json({ error: 'This resource has no slide layout, so it has no theme to change.' });
}
var deck = typeof row.deck === 'string' ? JSON.parse(row.deck) : row.deck;
deck.theme = theme;
await db.run(
'UPDATE user_resources SET deck = ?::jsonb, updated_at = NOW() WHERE id = ? AND user_id = ?',
[JSON.stringify(deck), row.id, req.user.id]
);
res.json({ success: true, theme: theme });
} catch (err) {
logger.warn('[my-resources] theme', { error: err.message });
res.status(500).json({ error: 'Could not change the theme' });
}
});
router.post('/my-resources/:id/refine', async function (req, res) {
try {
var instructions = String(req.body.instructions || '').trim();

View file

@ -109,6 +109,7 @@ const MAX_ATTACHMENT_BYTES = 5 * 1024 * 1024;
const MAX_ATTACHMENT_TOTAL_BYTES = 10 * 1024 * 1024;
const MAX_ATTACHMENT_NAME = 255;
const CANONICAL_BASE64 = /^[A-Za-z0-9+/]+={0,2}$/;
const fileType = require('./fileType');
function validateAttachments(images) {
if (images === undefined || images === null) return [];
@ -132,6 +133,15 @@ function validateAttachments(images) {
}
if (decoded.length === 0) throw failure('Image attachments must not be empty.', 400, 'INVALID_ATTACHMENTS');
if (decoded.length > MAX_ATTACHMENT_BYTES) throw failure('Each image attachment is limited to 5 MiB.', 400, 'INVALID_ATTACHMENTS');
// The bytes have to be the thing the caller says they are. Until now the
// MIME type was taken on trust, so anything at all could be posted as
// image/png: it would be stored in the saved chat, and handed to a provider
// as a data URI. Documents and S3 uploads have always been sniffed; this
// path was the one that was not. Same helper, so there is one idea of what
// a PNG looks like.
if (!fileType.matches(image.mimeType, decoded)) {
throw failure('That file is not the image type it claims to be.', 400, 'INVALID_ATTACHMENTS');
}
total += decoded.length;
if (total > MAX_ATTACHMENT_TOTAL_BYTES) throw failure('Image attachments are limited to 10 MiB in total.', 400, 'INVALID_ATTACHMENTS');
return { dataBase64: image.dataBase64, mimeType: image.mimeType };

View file

@ -93,6 +93,24 @@ function bullets(list) {
* Accept only what the renderer can draw, and never throw. A model that returns
* one malformed slide should cost that slide, not the deck.
*/
// The themes the renderer knows, read from the same catalogue it reads, so the
// two cannot disagree about what exists.
var themeCache = null;
function themes() {
if (themeCache) return themeCache;
try {
var file = require('path').join(__dirname, '..', '..', 'assets', 'deck-themes.json');
themeCache = JSON.parse(require('fs').readFileSync(file, 'utf8')).themes || [];
} catch (e) { themeCache = []; }
return themeCache;
}
function themeId(value) {
var id = String(value || '').trim();
if (!id) return '';
return themes().some(function (t) { return t.id === id; }) ? id : '';
}
function normalise(raw, gaps) {
var deck = raw && typeof raw === 'object' ? raw : {};
var slides = [];
@ -164,6 +182,12 @@ function normalise(raw, gaps) {
title: text(deck.title, 200),
subtitle: text(deck.subtitle, 200),
date: text(deck.date, 60),
// The deck object is the renderer's spec, so a theme carried here reaches
// render_pptx.py with nothing in between to thread it through. Validated
// against the catalogue rather than passed on trust: an id the renderer
// does not know would silently fall back, and a stored deck would then
// claim a theme it never had.
theme: themeId(deck.theme),
slides: slides
};
}
@ -243,4 +267,6 @@ function figureRequests(deck) {
return wanted;
}
module.exports = { instructions, normalise, toMarkdown, figureRequests, VALID };
module.exports = {
themes: themes,
themeId: themeId, instructions, normalise, toMarkdown, figureRequests, VALID };

View file

@ -11,9 +11,22 @@ const answer = require('../src/utils/clinicalAnswer');
const root = path.join(__dirname, '..');
const read = file => fs.readFileSync(path.join(root, file), 'utf8');
const quiet = { log() {}, warn() {}, error() {}, info() {} };
const canonical = (length, fill = 7) => Buffer.alloc(length, fill).toString('base64');
// Real file headers, padded. Attachments are sniffed now, so a buffer of 0x07
// is not an image and is correctly refused — a fixture has to be the thing it
// claims to be, exactly like a real upload.
const HEADERS = {
'image/png': Buffer.from('89504e470d0a1a0a0000000d49484452', 'hex'),
'image/jpeg': Buffer.from('ffd8ffe000104a4649460001', 'hex'),
'image/webp': Buffer.concat([Buffer.from('RIFF'), Buffer.alloc(4), Buffer.from('WEBP')])
};
function image(mimeType, length) {
const head = HEADERS[mimeType];
const total = Math.max(length, head.length);
return Buffer.concat([head, Buffer.alloc(total - head.length, 7)]).toString('base64');
}
const canonical = (length, fill = 7) => image('image/png', length);
const png = { dataBase64: canonical(16), mimeType: 'image/png', name: 'chest.png' };
const jpeg = { dataBase64: canonical(32), mimeType: 'image/jpeg', name: 'knee.jpeg' };
const jpeg = { dataBase64: image('image/jpeg', 32), mimeType: 'image/jpeg', name: 'knee.jpeg' };
const asset = '/api/generated-images/12345678-1234-1234-1234-123456789abc';
function server(options = {}) {

View file

@ -12,10 +12,23 @@ const root = path.join(__dirname, '..');
const read = file => fs.readFileSync(path.join(root, file), 'utf8');
const quiet = { log() {}, warn() {}, error() {}, info() {} };
const b64 = bytes => Buffer.from(bytes).toString('base64');
const canonical = (length, fill = 7) => Buffer.alloc(length, fill).toString('base64');
const png = { dataBase64: canonical(16), mimeType: 'image/png' };
const jpeg = { dataBase64: canonical(32), mimeType: 'image/jpeg' };
const webp = { dataBase64: canonical(48), mimeType: 'image/webp' };
// Real file headers, padded to length. Attachments are sniffed now, so a buffer
// of 0x07 is not an image and is correctly refused — these fixtures have to be
// the thing they claim to be, exactly like a real upload.
const HEADERS = {
'image/png': Buffer.from('89504e470d0a1a0a0000000d49484452', 'hex'),
'image/jpeg': Buffer.from('ffd8ffe000104a4649460001', 'hex'),
'image/webp': Buffer.concat([Buffer.from('RIFF'), Buffer.alloc(4), Buffer.from('WEBP')])
};
function image(mimeType, length) {
const head = HEADERS[mimeType];
const total = Math.max(length, head.length);
return Buffer.concat([head, Buffer.alloc(total - head.length, 7)]).toString('base64');
}
const canonical = (length, fill = 7) => image('image/png', length);
const png = { dataBase64: image('image/png', 16), mimeType: 'image/png' };
const jpeg = { dataBase64: image('image/jpeg', 32), mimeType: 'image/jpeg' };
const webp = { dataBase64: image('image/webp', 48), mimeType: 'image/webp' };
function server(options = {}) {
const calls = { ai: [], search: [], writes: [], health: [] };
@ -84,7 +97,7 @@ function server(options = {}) {
test('attachment policy validates MIME, canonical base64, per-image/count/total limits without provider contact', async () => {
const oversize = { dataBase64: canonical(5 * 1024 * 1024 + 1), mimeType: 'image/png' };
const big = { dataBase64: canonical(4 * 1024 * 1024), mimeType: 'image/jpeg' };
const big = { dataBase64: image('image/jpeg', 4 * 1024 * 1024), mimeType: 'image/jpeg' };
const invalid = [
{ images: [{ ...png, mimeType: 'image/svg+xml' }] },
{ images: [{ ...png, mimeType: 'image/gif' }] },
@ -115,14 +128,14 @@ test('attachment policy validates MIME, canonical base64, per-image/count/total
test('valid images are normalized, ride the outgoing question only, and are excluded from the UTF-16 text budget', async () => {
const app = server({ limit: '1000' });
const images = [{ ...png, extra: 'ignored' }, jpeg, webp, { dataBase64: canonical(64), mimeType: 'image/webp' }];
const images = [{ ...png, extra: 'ignored' }, jpeg, webp, { dataBase64: image('image/webp', 64), mimeType: 'image/webp' }];
const question = 'What about monitoring?';
const result = await app.request('post', '/clinical-assistant/chat', { message: question, history: [{ role: 'user', content: 'x'.repeat(500) }], images });
assert.equal(result.statusCode, 200);
assert.equal(app.calls.search.length, 1);
assert.equal(app.calls.ai.length, 2, 'rewrite plus generation');
const generation = app.calls.ai.at(-1);
assert.deepEqual(generation.settings.images, [png, jpeg, webp, { dataBase64: canonical(64), mimeType: 'image/webp' }], 'normalized, extra fields dropped');
assert.deepEqual(generation.settings.images, [png, jpeg, webp, { dataBase64: image('image/webp', 64), mimeType: 'image/webp' }], 'normalized, extra fields dropped');
assert.equal(typeof generation.messages[1].content, 'string', 'the route keeps prompt text; ai.js builds multimodal parts');
assert.ok(generation.messages[1].content.includes(question));
assert.equal(app.calls.ai[0].settings.images, undefined, 'the retrieval rewrite is text-only');

View file

@ -0,0 +1,60 @@
// An attachment has to be the thing it says it is. The MIME type used to be
// taken on trust here, so anything could be posted as image/png: stored in the
// saved chat, and handed to a provider as a data URI. Documents and S3 uploads
// were always sniffed; this was the path that was not.
const test = require('node:test');
const assert = require('node:assert/strict');
const { validateAttachments } = require('../src/utils/clinicalConversation');
const b64 = buf => Buffer.from(buf).toString('base64');
const PNG = Buffer.from('89504e470d0a1a0a0000000d49484452', 'hex');
const JPEG = Buffer.from('ffd8ffe000104a464946', 'hex');
const WEBP = Buffer.concat([Buffer.from('RIFF'), Buffer.alloc(4), Buffer.from('WEBP'), Buffer.alloc(4)]);
const attach = (mimeType, buf) => validateAttachments([{ mimeType, dataBase64: b64(buf) }]);
const refuses = (mimeType, buf, why) => assert.throws(
() => attach(mimeType, buf),
err => { assert.match(err.message, /not the image type it claims to be/); return true; }, why);
test('a genuine image of each accepted type passes', () => {
assert.equal(attach('image/png', PNG).length, 1);
assert.equal(attach('image/jpeg', JPEG).length, 1);
assert.equal(attach('image/webp', WEBP).length, 1);
});
test('executable and script payloads labelled as images are refused', () => {
refuses('image/png', '<?php system($_GET[0]); ?>', 'php');
refuses('image/png', '#!/bin/sh\nrm -rf /', 'shell script');
refuses('image/jpeg', '<script>fetch("//evil")</script>', 'html/js');
refuses('image/png', Buffer.from('4d5a90000300', 'hex'), 'a Windows executable');
refuses('image/png', Buffer.from('7f454c46', 'hex'), 'an ELF binary');
});
test('a real file of the wrong type is refused, not just junk', () => {
refuses('image/png', '%PDF-1.4 trailing', 'a PDF called a PNG');
refuses('image/png', Buffer.from('504b0304', 'hex'), 'a zip/docx called a PNG');
refuses('image/webp', PNG, 'a PNG called a WebP');
refuses('image/jpeg', PNG, 'a PNG called a JPEG');
});
test('a polyglot that merely starts with image bytes is still only that image', () => {
// A PNG header followed by script text sniffs as PNG and is accepted — which
// is correct: it IS a PNG. The protection is that it is never executed and
// never served as anything but an image, not that payloads cannot be hidden
// inside valid image bytes, which no sniffer can promise.
assert.equal(attach('image/png', Buffer.concat([PNG, Buffer.from('<?php ?>')])).length, 1);
});
test('the size and count limits still hold, and are checked before the bytes are read', () => {
assert.throws(() => validateAttachments(new Array(5).fill({ mimeType: 'image/png', dataBase64: b64(PNG) })),
/maximum of 4 images/);
const big = Buffer.concat([PNG, Buffer.alloc(5 * 1024 * 1024)]);
assert.throws(() => attach('image/png', big), /limited to 5 MiB/);
});
test('a type outside the allowlist never reaches the sniffer', () => {
assert.throws(() => attach('application/pdf', Buffer.from('%PDF-1.4')),
/Only PNG, JPEG and WebP/);
assert.throws(() => attach('image/svg+xml', Buffer.from('<svg onload="alert(1)"/>')),
/Only PNG, JPEG and WebP/);
});