From 847cfd2d2b80cc035f6b97c09dda363daf3758dd Mon Sep 17 00:00:00 2001 From: wtclaude Date: Fri, 7 Aug 2026 20:09:56 -0500 Subject: [PATCH] feat(theming): brand-asset overrides and a cached, settings-aware HTML shell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5 of docs/website/THEMING_AND_NAV.md: uploaded logo/hero/favicon overrides on top of the BRAND_* env defaults, delivered through an HTML shell that is no longer built once at boot. - utils/htmlShell.js owns the shell lifecycle: rendered lazily, cached per process, invalidated on a brand_assets/theme_visual write with a 5-minute TTL so other workers converge. A settings-read failure renders the env-only shell and caches that, so a DB outage is not a failing query per page view, and with no rows the output is byte-identical to what app.js served before. - POST /admin/settings/brand-asset/:slot uploads one asset and writes the row in the same call, so an upload never leaves an unreferenced file. It reuses the shared multer allowlist and only tightens it per slot: favicons are PNG-only and capped at 512 KB, logos at 1 MB, heroes at 8 MB. Refused files are unlinked before the response. - utils/brandAssets.js constrains a stored asset to a same-origin path under /uploads, /brand or /assets — these are the only settings values written straight into the page as a URL. Strict on write, forgiving on read. - The shell also carries the resolved theme as a ` : '' +} + +/** + * Provide the built index.html. Called once at boot by app.js; a separate step + * from get() so the file read stays synchronous and startup still fails loudly + * if the client build is unreadable. + */ +function init(html) { + template = html + cached = null + inflight = null + generation += 1 +} + +/** Drop the cached shell. Called after any write that can change it. */ +function invalidate() { + cached = null + inflight = null + generation += 1 +} + +/** + * The current shell. Renders on a cold or expired cache, otherwise returns the + * cached string. Never rejects: a settings read that fails yields the env-only + * shell. + * + * @returns {Promise} + */ +async function get() { + if (template === null) throw new Error('htmlShell.init() was never called') + if (cached && Date.now() - cached.at < TTL_MS) return cached.html + if (inflight) return inflight + + const startedAt = generation + const run = (async () => { + let overrides = {} + try { + // Required lazily: this module is loaded by app.js at boot, and the + // settings model pulls in the DB pool. Requiring it at the top would make + // the HTML shell a startup-time dependency of the database. + // eslint-disable-next-line global-require + const settings = require('../model/settings/settings.model') + overrides = await settings.getShellBrand() + } catch { + // A DB fault must never fail the page (§4.3). Fall back to the env-only + // shell — the pre-feature behaviour — and cache it, so an outage does not + // mean a failing query per page view. + overrides = {} + } + const html = render(template, overrides) + // An invalidation that landed while this read was in flight means the value + // we just read may already be stale. Serve it, but do not cache it. + if (generation === startedAt) cached = { html, at: Date.now() } + // Only retire our own registration: an invalidation during the read may have + // already started a newer render, and clearing that one would cost an extra + // render on the next request. + if (inflight === run) inflight = null + return html + })() + inflight = run + return run +} + +module.exports = { init, get, invalidate, render, TTL_MS, THEME_STYLE_ID } diff --git a/server/swagger/swagger-output.json b/server/swagger/swagger-output.json index 7f862d8..31e95e3 100644 --- a/server/swagger/swagger-output.json +++ b/server/swagger/swagger-output.json @@ -3532,6 +3532,132 @@ } } }, + "/api/v1/admin/settings/brand-asset/{slot}": { + "post": { + "tags": [ + "Admin · Settings" + ], + "summary": "Upload a brand asset and set it as the override (admin only)", + "description": "Stores the image and writes the brand_assets settings row in one call, so an upload never leaves an unreferenced file. Favicons must be PNG (max 512 KB); logos max 1 MB; heroes max 8 MB. Absent slots keep falling back to the BRAND_* env defaults — uploading a logo does not clear a hero.", + "parameters": [ + { + "name": "slot", + "in": "path", + "required": true, + "schema": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "enum": { + "type": "array", + "example": [ + "logo", + "hero", + "favicon" + ], + "items": { + "type": "string" + } + } + } + }, + "description": "Which asset to replace" + } + ], + "responses": { + "201": { + "description": "Stored file URL and the updated overrides", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "url": { + "type": "string", + "example": "/uploads/1712345678901-ab12cd34.png" + }, + "brand_assets": { + "type": "object", + "properties": { + "logo": { + "type": "string" + }, + "hero": { + "type": "string" + }, + "favicon": { + "type": "string" + } + } + } + } + } + } + } + }, + "400": { + "description": "No file, unknown slot, disallowed type, or over the slot size cap", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "401": { + "description": "Not authenticated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Admin role required", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "multipart/form-data": { + "schema": { + "type": "object", + "properties": { + "image": { + "type": "string", + "format": "binary" + } + } + } + } + } + } + } + }, "/api/v1/admin/settings/{key}": { "delete": { "tags": [ diff --git a/server/test/brandAssets.test.js b/server/test/brandAssets.test.js new file mode 100644 index 0000000..773f62a --- /dev/null +++ b/server/test/brandAssets.test.js @@ -0,0 +1,352 @@ +// Point the DB at a closed port BEFORE the pool is built, and the upload +// directory at a throwaway one BEFORE imageUpload.js resolves it — both are read +// at require time. Every model call is monkeypatched, so no query runs. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const os = require('os') +const path = require('path') +const fs = require('fs') + +const UPLOAD_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-brand-assets-')) +process.env.UPLOAD_DIR = UPLOAD_DIR + +const { test, after, afterEach } = require('node:test') +const assert = require('node:assert/strict') + +// Phase 5 of docs/website/THEMING_AND_NAV.md: the brand-asset overrides. Two +// halves are worth locking — what a stored value is allowed to be (these values +// are written straight into HTML as URLs) and the upload route's per-slot rules, +// which tighten the shared allowlist without ever widening it (§9). +const { startApp } = require('./_helper') +const { isSafeAssetPath, validateBrandAssets, resolveBrandAssets, SLOTS } = require('../src/utils/brandAssets') +const settingsRouter = require('../src/router/v1/admin/settings.router') +const settingsDb = require('../src/model/settings/settings.db') +const sessionService = require('../src/auth/session.service') +const { requireAuth } = require('../src/auth/session.middleware') +const users = require('../src/model/users/users.model') +const activity = require('../src/model/activity/activity.model') +const htmlShell = require('../src/utils/htmlShell') +const db = require('../src/utils/db') + +after(() => { + db.close() + fs.rmSync(UPLOAD_DIR, { recursive: true, force: true }) +}) + +const originals = { + validateSession: sessionService.validateSession, + isSessionRevoked: sessionService.isSessionRevoked, + sessionMeta: sessionService.sessionMeta, + getById: users.getById, + get: settingsDb.get, + set: settingsDb.set, + log: activity.log, +} +afterEach(() => { + Object.assign(sessionService, { + validateSession: originals.validateSession, + isSessionRevoked: originals.isSessionRevoked, + sessionMeta: originals.sessionMeta, + }) + users.getById = originals.getById + settingsDb.get = originals.get + settingsDb.set = originals.set + activity.log = originals.log +}) + +function signInAs(user) { + sessionService.validateSession = () => ({ userId: user.id, sessionId: 's1', createdAt: Date.now(), authMethod: 'jwt' }) + sessionService.isSessionRevoked = async () => false + sessionService.sessionMeta = () => ({}) + users.getById = async () => user + activity.log = async () => {} +} + +// ── What a stored asset path may be ─────────────────────────────────── + +test('only same-origin paths under the directories this server serves are accepted', () => { + for (const ok of ['/uploads/1-a.png', '/brand/logo.svg', '/assets/img/runic-emblem.png']) { + assert.equal(isSafeAssetPath(ok), true, `${ok} should be accepted`) + } + const rejected = [ + 'https://evil.example/x.png', // off-origin: an the operator did not choose + '//evil.example/x.png', // protocol-relative — looks like a path, loads off-origin + 'javascript:alert(1)', // no scheme survives the prefix check, but be explicit + '/uploads/../../etc/passwd', // climbing out of the served directory + '/uploads/a b.png', // whitespace is the raw material for smuggling + '/uploads/"onerror="alert(1)', // quote would break out of the attribute + '/etc/passwd', // a path, but not one we serve + 'uploads/1-a.png', // relative to the current route, not to the origin + '', + null, + 42, + ] + for (const bad of rejected) { + assert.equal(isSafeAssetPath(bad), false, `${String(bad)} should be rejected`) + } +}) + +// Strict on write: the admin gets told which field is wrong, rather than saving +// something that silently never renders. +test('a write naming an unknown slot or an unusable path is rejected by field', () => { + assert.equal(validateBrandAssets({ logo: '/uploads/a.png', hero: null }).ok, true) + assert.equal(validateBrandAssets(null).ok, true) // clearing every slot + + const unknown = validateBrandAssets({ banner: '/uploads/a.png' }) + assert.equal(unknown.ok, false) + assert.match(unknown.message, /banner/) + + const offsite = validateBrandAssets({ favicon: 'https://evil.example/f.png' }) + assert.equal(offsite.ok, false) + assert.match(offsite.message, /favicon/) + + assert.equal(validateBrandAssets(['/uploads/a.png']).ok, false) +}) + +// Forgiving on read: one hand-edited slot must not cost the admin the other two. +test('a bad stored slot is dropped and its neighbours are kept', () => { + const resolved = resolveBrandAssets({ logo: '/uploads/a.png', hero: 'https://evil.example/h.png', favicon: null }) + assert.deepEqual(resolved, { logo: '/uploads/a.png' }) +}) + +test('resolve is also how a cleared slot stops being stored', () => { + // '' and null are how the UI clears a slot; neither may survive into the row, + // or "the field is absent" would stop being the single meaning of "use env". + assert.deepEqual(resolveBrandAssets({ logo: '', hero: null }), {}) + assert.deepEqual(resolveBrandAssets(null), {}) + assert.deepEqual(SLOTS, ['logo', 'hero', 'favicon']) +}) + +// ── POST /admin/settings/brand-asset/:slot ──────────────────────────── + +// A 1x1 PNG and a 1x1 GIF, small enough to inline and real enough for multer to +// accept by mimetype (which is what the shared allowlist keys off). +const PNG = Buffer.from( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==', + 'base64', +) +const GIF = Buffer.from('R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7', 'base64') + +function form(buffer, { filename = 'x.png', type = 'image/png' } = {}) { + const fd = new FormData() + fd.append('image', new Blob([buffer], { type }), filename) + return fd +} + +const startSettingsApp = () => + startApp((a) => a.use('/api/v1/admin/settings', requireAuth, settingsRouter)) + +const filesInUploadDir = () => fs.readdirSync(UPLOAD_DIR) + +test('uploading a slot stores the file and points brand_assets at it', async () => { + signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' }) + settingsDb.get = async () => null // never set before + let stored = null + settingsDb.set = async (key, value) => { + stored = { key, value } + } + const before = filesInUploadDir().length + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings/brand-asset/logo`, { + method: 'POST', + body: form(PNG), + }) + assert.equal(res.status, 201) + const body = await res.json() + assert.match(body.url, /^\/uploads\/\d+-[0-9a-f]{16}\.png$/) + assert.deepEqual(body.brand_assets, { logo: body.url }) + assert.equal(stored.key, 'brand_assets') + assert.deepEqual(JSON.parse(stored.value), { logo: body.url }) + assert.equal(filesInUploadDir().length, before + 1, 'the file is kept') + } finally { + await app.close() + } +}) + +// §6.3: uploading a logo does not force the admin to also pick a hero — and must +// not silently discard the hero they picked last week. +test('an upload merges into the existing overrides rather than replacing them', async () => { + signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' }) + settingsDb.get = async () => JSON.stringify({ hero: '/uploads/existing-hero.png' }) + let stored = null + settingsDb.set = async (key, value) => { + stored = value + } + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings/brand-asset/favicon`, { + method: 'POST', + body: form(PNG), + }) + assert.equal(res.status, 201) + const saved = JSON.parse(stored) + assert.equal(saved.hero, '/uploads/existing-hero.png', 'the hero survives') + assert.match(saved.favicon, /^\/uploads\//) + } finally { + await app.close() + } +}) + +// §4.10: .ico would mean adding a type to MIME_EXT, and the stored extension +// coming from that map is what makes the upload path safe. PNG only, and the +// rejected file does not stay on disk. +test('a favicon that is not a PNG is refused and the file is discarded', async () => { + signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' }) + settingsDb.set = async () => assert.fail('a refused upload must not write the row') + const before = filesInUploadDir().length + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings/brand-asset/favicon`, { + method: 'POST', + body: form(GIF, { filename: 'x.gif', type: 'image/gif' }), + }) + assert.equal(res.status, 400) + assert.match((await res.json()).message, /PNG/) + assert.equal(filesInUploadDir().length, before, 'no orphan file left behind') + } finally { + await app.close() + } +}) + +test('a file over the slot cap is refused and discarded', async () => { + signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' }) + settingsDb.set = async () => assert.fail('a refused upload must not write the row') + // Valid PNG header, then padding past the favicon's 512 KB cap — the shared + // multer limit is 8 MB, so only the per-slot rule can reject this. + const big = Buffer.concat([PNG, Buffer.alloc(600 * 1024)]) + const before = filesInUploadDir().length + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings/brand-asset/favicon`, { + method: 'POST', + body: form(big), + }) + assert.equal(res.status, 400) + assert.match((await res.json()).message, /512 KB or smaller/) + assert.equal(filesInUploadDir().length, before) + } finally { + await app.close() + } +}) + +test('the same file is accepted for a slot with a bigger cap', async () => { + signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' }) + settingsDb.get = async () => null + settingsDb.set = async () => {} + const big = Buffer.concat([PNG, Buffer.alloc(600 * 1024)]) + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings/brand-asset/hero`, { + method: 'POST', + body: form(big), + }) + assert.equal(res.status, 201) + } finally { + await app.close() + } +}) + +test('an unknown slot is refused and the file is discarded', async () => { + signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' }) + settingsDb.set = async () => assert.fail('an unknown slot must not write the row') + const before = filesInUploadDir().length + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings/brand-asset/banner`, { + method: 'POST', + body: form(PNG), + }) + assert.equal(res.status, 400) + assert.equal(filesInUploadDir().length, before) + } finally { + await app.close() + } +}) + +// The generic POST /admin/uploads is reachable by editors. The site's identity +// is not theirs to change, so this route carries the same admin gate as the +// settings row it writes. +test('an editor cannot upload a brand asset', async () => { + signInAs({ id: 2, username: 'e', role: 'editor', status: 'active' }) + settingsDb.set = async () => assert.fail('an editor must not write brand_assets') + const before = filesInUploadDir().length + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings/brand-asset/logo`, { + method: 'POST', + body: form(PNG), + }) + assert.equal(res.status, 403) + assert.equal(filesInUploadDir().length, before, 'the gate runs before multer writes') + } finally { + await app.close() + } +}) + +// ── PUT /admin/settings { brand_assets } — how a slot is CLEARED ────── +// +// There is no per-slot delete route: clearing the logo is a write of the +// remaining slots, and clearing the last one is the existing reset-by-delete. + +test('clearing a slot through the settings write drops it from the row', async () => { + signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' }) + let stored = null + settingsDb.set = async (key, value) => { + stored = value + } + settingsDb.getAll = async () => [] + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ brand_assets: { logo: '/uploads/a.png', hero: null, favicon: '' } }), + }) + assert.equal(res.status, 200) + assert.deepEqual(JSON.parse(stored), { logo: '/uploads/a.png' }, 'no null fields survive into the row') + } finally { + await app.close() + } +}) + +test('a settings write carrying an off-origin asset URL is rejected by field', async () => { + signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' }) + settingsDb.set = async () => assert.fail('an invalid brand_assets must not be stored') + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ brand_assets: { logo: 'https://tracker.example/pixel.png' } }), + }) + assert.equal(res.status, 400) + assert.match((await res.json()).message, /brand_assets\.logo/) + } finally { + await app.close() + } +}) + +test('a successful upload invalidates the cached HTML shell', async () => { + signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' }) + settingsDb.get = async () => null + settingsDb.set = async () => {} + let invalidated = 0 + const realInvalidate = htmlShell.invalidate + htmlShell.invalidate = () => { + invalidated += 1 + } + const app = await startSettingsApp() + try { + const res = await fetch(`${app.url}/api/v1/admin/settings/brand-asset/logo`, { + method: 'POST', + body: form(PNG), + }) + assert.equal(res.status, 201) + assert.equal(invalidated, 1, 'the favicon an admin just uploaded must not wait for the TTL') + } finally { + htmlShell.invalidate = realInvalidate + await app.close() + } +}) diff --git a/server/test/htmlShell.test.js b/server/test/htmlShell.test.js new file mode 100644 index 0000000..03b5b4d --- /dev/null +++ b/server/test/htmlShell.test.js @@ -0,0 +1,229 @@ +// Point the DB at a closed port before the pool is built; the settings read is +// monkeypatched in every test that reaches it, so no query runs. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' +// A brand URL, so the og:image absolutization of an uploaded path is exercised +// rather than being dead code in the test environment. +process.env.BRAND_URL = process.env.BRAND_URL || 'https://shard.example' +// BRAND_LOGO defaults to empty (no logo image rendered), which would make the +// "og:image still comes from env" assertions below pass vacuously. +process.env.BRAND_LOGO = process.env.BRAND_LOGO || '/brand/logo.png' + +const { test, afterEach, after } = require('node:test') +const assert = require('node:assert/strict') + +// The cached, settings-aware HTML shell (docs/website/THEMING_AND_NAV.md §4.3). +// Three properties are load-bearing enough to lock here: that an untouched +// instance gets byte-for-byte the shell it got before this feature existed, that +// a DB fault still serves a page, and that the steady state is one cached string +// rather than a settings read per page view. +const htmlShell = require('../src/utils/htmlShell') +const settings = require('../src/model/settings/settings.model') +const brand = require('../src/config/brand') +const db = require('../src/utils/db') + +after(() => db.close()) + +const originalGetShellBrand = settings.getShellBrand +afterEach(() => { + settings.getShellBrand = originalGetShellBrand +}) + +// A stand-in for the built client/dist/index.html: the two tags the shell +// rewrites plus the stylesheet link the theme block has to follow. +const TEMPLATE = ` + + + + Vite App + + + +
+` + +// The shell app.js served BEFORE this phase, reproduced verbatim. The point of +// the test is that this string and the new renderer's output are identical for +// an instance with no brand_assets and no theme_visual row (§9), so it is copied +// rather than imported. +function legacyRenderIndexHtml(html) { + const htmlEscape = (s) => + String(s).replace( + /[&<>"']/g, + (c) => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }[c]), + ) + const title = htmlEscape(brand.name) + const desc = htmlEscape(brand.description) + const tags = [ + ``, + ``, + '', + brand.url ? `` : '', + brand.logo ? `` : '', + '', + ``, + ``, + brand.favicon ? `` : '', + ] + .filter(Boolean) + .join('\n ') + return html + .replace(/[\s\S]*?<\/title>/i, `<title>${title}`) + .replace(/()/i, `$1${desc}$2`) + .replace(/<\/head>/i, ` ${tags}\n `) +} + +// ── The byte-identical guarantee (§9) ───────────────────────────────── + +test('with no overrides the shell is byte-identical to the pre-feature one', () => { + assert.equal(htmlShell.render(TEMPLATE, {}), legacyRenderIndexHtml(TEMPLATE)) +}) + +test('an empty theme and empty assets are the same as no overrides at all', () => { + // A row that parsed to nothing usable resolves to null/undefined rather than + // to an empty block, or "reset" would leave a `', 'color': 'red' }, + }) + assert.match(html, /