diff --git a/tools/cropper.js b/tools/cropper.js index cede308..2fdd46a 100644 --- a/tools/cropper.js +++ b/tools/cropper.js @@ -70,6 +70,7 @@ export function createCropperRouter(cfg) { carvekitKey, removebgUrl = REMOVEBG_URL, noderedBase = 'https://nodered.a2loft.net', + domainCodes = new Set(), requireApi, cache: cropCache, } = cfg; @@ -95,6 +96,25 @@ export function createCropperRouter(cfg) { }; router.use(cropGate); + // Express 4 does not catch async rejections: an unhandled throw would hang + // the request (and, on Node's default, exit the process). Wrap every route + // handler so failures become a 500 via next(err). + for (const method of ['get', 'post']) { + const register = router[method].bind(router); + router[method] = (path, ...handlers) => + register(path, ...handlers.map((h) => (h.length >= 3 ? h : (req, res, next) => Promise.resolve(h(req, res, next)).catch(next)))); + } + + /** Reject malformed bodies up front instead of throwing on destructuring. */ + const requireMessageBody = (req, res) => { + if (!req.body || typeof req.body.message !== 'object' || req.body.message === null) { + res.status(400).json({ ok: false, message: 'A message body is required.' }); + return false; + } + return true; + }; + + const UPSTREAM_TIMEOUT_MS = 15000; const base = `${cfg.a2loftBase.replace(/\/+$/, '')}/identity`; const nodered = (path) => `${noderedBase.replace(/\/+$/, '')}${path}`; @@ -130,7 +150,12 @@ export function createCropperRouter(cfg) { if (!message) { return res.json({ ok: false, message: 'Queue is currently empty.' }); } - const data = JSON.parse(message.messageText); + let data; + try { + data = JSON.parse(message.messageText); + } catch { + return res.json({ ok: false, message: 'Queue item is malformed.' }); + } const { messageId, popReceipt, dequeueCount } = message; res.json({ ok: true, @@ -140,9 +165,12 @@ export function createCropperRouter(cfg) { router.get('/picture/load/:id', async (req, res) => { try { - const r = await fetchImpl(`${base}/dims/picture/${encodeURIComponent(req.params.id)}`); - const { data } = await r.json(); - if (!data) return res.json({ ok: false, message: 'No upload with the specified id.' }); + const r = await fetchImpl(`${base}/dims/picture/${encodeURIComponent(req.params.id)}`, { + signal: AbortSignal.timeout(UPSTREAM_TIMEOUT_MS), + }); + const body = await r.json().catch(() => null); + const data = body && body.data; + if (!r.ok || !data) return res.json({ ok: false, message: 'No upload with the specified id.' }); const { upload_id, file_name, caption, domain, raw_file_id } = data; return res.json({ ok: true, @@ -163,6 +191,7 @@ export function createCropperRouter(cfg) { const review = await ( await fetchImpl(nodered(`/idid/picture/review/${encodeURIComponent(req.params.review_id)}`), { headers: { authorization: `Basic ${Buffer.from(`${noderedAuth.username}:${noderedAuth.password}`).toString('base64')}` }, + signal: AbortSignal.timeout(UPSTREAM_TIMEOUT_MS), }) ).json(); if (!review.ok) { @@ -171,6 +200,7 @@ export function createCropperRouter(cfg) { const agents = await ( await fetchImpl(nodered('/unical/agents'), { headers: { authorization: `Basic ${Buffer.from(`${noderedAuth.username}:${noderedAuth.password}`).toString('base64')}` }, + signal: AbortSignal.timeout(UPSTREAM_TIMEOUT_MS), }) ).json(); const agent = (Array.isArray(agents) ? agents : []).find( @@ -184,9 +214,12 @@ export function createCropperRouter(cfg) { router.get('/picture/:file_name', async (req, res) => { const file_name = req.params.file_name.replace(/_/gi, '/'); + if (!file_name.startsWith('id-domains/') || file_name.includes('..')) { + return res.status(400).json({ ok: false, message: 'Invalid picture reference.' }); + } try { const url = await minio.presignedGetObject(BUCKET, file_name); - const r = await fetchImpl(url); + const r = await fetchImpl(url, { signal: AbortSignal.timeout(UPSTREAM_TIMEOUT_MS) }); const buf = Buffer.from(await r.arrayBuffer()); res.json({ ok: true, data: `data:image/png;base64,${buf.toString('base64')}` }); } catch (error) { @@ -195,13 +228,15 @@ export function createCropperRouter(cfg) { }); router.post('/picture/review', async (req, res) => { + if (!requireMessageBody(req, res)) return; const { - message: { pop_receipt, message_id }, + message: { pop_receipt, message_id } = {}, deleteFromQueue, ...body } = req.body; try { const r = await fetchImpl(nodered('/zeebe/picture/review'), { + signal: AbortSignal.timeout(UPSTREAM_TIMEOUT_MS), method: 'POST', headers: { 'content-type': 'application/json', @@ -230,8 +265,9 @@ export function createCropperRouter(cfg) { }); router.post('/dequeue', async (req, res) => { + if (!requireMessageBody(req, res)) return; const { - message: { pop_receipt, message_id }, + message: { pop_receipt, message_id } = {}, } = req.body; try { const result = await deleteMessage(message_id, pop_receipt); @@ -250,6 +286,7 @@ export function createCropperRouter(cfg) { const fd = new FormData(); fd.append('image_file_b64', req.body.image_file_b64); const r = await fetchImpl(removebgUrl, { + signal: AbortSignal.timeout(UPSTREAM_TIMEOUT_MS), method: 'POST', headers: { 'x-api-key': carvekitKey }, body: fd, @@ -271,6 +308,7 @@ export function createCropperRouter(cfg) { // ── save (the one a2loft call that needs the operator OTP) ────────────── router.post('/profile/picture', async (req, res) => { + if (!requireMessageBody(req, res)) return; if (!requireOtp(req, res)) return; let live; try { @@ -297,14 +335,18 @@ export function createCropperRouter(cfg) { message: { pop_receipt, message_id, - message: { upload_id, domain, agentDeviceHash }, + message: { upload_id, domain, agentDeviceHash } = {}, }, } = req.body; + if (!domainCodes.has(domain)) { + return res.status(400).json({ ok: false, message: 'Unknown domain.' }); + } const q = new URLSearchParams({ phone: req.session.phone, otp: req.query.otp }); try { const r = await fetchImpl( `${base}/${encodeURIComponent(domain)}/profile/picture?${q.toString()}`, { + signal: AbortSignal.timeout(UPSTREAM_TIMEOUT_MS), method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ diff --git a/tools/permit.js b/tools/permit.js index 7d54d70..b46ddc0 100644 --- a/tools/permit.js +++ b/tools/permit.js @@ -4,6 +4,8 @@ // Results are cached per operator phone for DOMAIN_TTL_MS. export const DOMAIN_TTL_MS = 5 * 60 * 1000; export const CACHE_FRESH = 'CACHE_FRESH'; +/** Outbound OpenFGA verify timeout — a hung portal must not pin a request. */ +export const VERIFY_TIMEOUT_MS = 5000; export function stripPhonePrefix(phone) { return (phone || '').replace(/^\+/, ''); @@ -19,6 +21,7 @@ async function verifyOnce(fetchImpl, portalUrl, phone, object, relation = 'captu relation, object, }), + signal: AbortSignal.timeout(VERIFY_TIMEOUT_MS), }); if (r.status !== 200) throw new Error(`verify status ${r.status}`); const out = await r.json(); diff --git a/tools/public/index.html b/tools/public/index.html index 24cb009..9a1ac15 100644 --- a/tools/public/index.html +++ b/tools/public/index.html @@ -76,11 +76,21 @@ .then(function (session) { var host = document.getElementById('nav-session'); if (!session) return; // keep the Sign in link - var first = (session.firstName || session.phone || '?').trim(); - host.innerHTML = - '' + first.slice(0, 2).toUpperCase() + '' + - (session.firstName || session.phone) + '' + - 'Sign out'; + var name = (session.firstName || session.phone || '?').trim(); + host.textContent = ''; + var chip = document.createElement('span'); + chip.className = 'user-chip'; + var avatar = document.createElement('span'); + avatar.className = 'avatar'; + avatar.textContent = name.slice(0, 2).toUpperCase(); // textContent, never innerHTML + var label = document.createElement('span'); + label.textContent = name; + chip.append(avatar, label); + var signOut = document.createElement('a'); + signOut.className = 'linklike'; + signOut.href = '/auth/logout'; + signOut.textContent = 'Sign out'; + host.append(chip, signOut); }) .catch(function () { /* keep Sign in */ }); diff --git a/tools/server.js b/tools/server.js index 5fed4f5..57081b2 100644 --- a/tools/server.js +++ b/tools/server.js @@ -70,7 +70,7 @@ function errorPage(res, status, title, message, actions = {}) { - ${title} · YOUreka Tools + ${esc(title)} · YOUreka Tools @@ -90,10 +90,10 @@ function errorPage(res, status, title, message, actions = {}) {
${ICONS[icon] || ICONS.alert}
-

${title}

-

${message}

- ${primary.label} - ${secondary ? `${secondary.label}` : ''} +

${esc(title)}

+

${esc(message)}

+ ${esc(primary.label)} + ${secondary ? `${esc(secondary.label)}` : ''}
@@ -107,6 +107,16 @@ const ICONS = { '', }; +/** HTML-escape interpolated values — error pages may carry upstream text. */ +function esc(value) { + return String(value ?? '') + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +}; + // ── app factory ────────────────────────────────────────────────────────────── export function createApp(overrides = {}) { const cfg = { @@ -127,7 +137,9 @@ export function createApp(overrides = {}) { const app = express(); app.disable('x-powered-by'); app.set('trust proxy', 1); // https termination upstream → correct Secure-cookie flag - app.use(express.json({ limit: '50mb' })); // cropper pictures travel as data URLs + // cropper pictures travel as data URLs (large); everything else stays small + app.use('/cropper', express.json({ limit: '50mb' })); + app.use(express.json({ limit: '1mb' })); const setSessionCookie = (req, res, value) => res.cookie('session', value, { @@ -147,6 +159,9 @@ export function createApp(overrides = {}) { maxAge: SESSION_MAX_AGE_MS, }); + // ── static assets (no session work per asset) ───────────────────────────── + app.use(express.static(path.join(__dirname, 'public'))); + // ── session middleware: recover / refresh the YOUreka Auth session ──────── app.use(async (req, res, next) => { req.session = null; @@ -181,7 +196,6 @@ export function createApp(overrides = {}) { }; // ── static assets & landing page ────────────────────────────────────────── - app.use(express.static(path.join(__dirname, 'public'))); // ── auth: YOUreka portal web-auth flow ──────────────────────────────────── app.get('/auth/login', async (req, res) => { @@ -339,6 +353,7 @@ export function createApp(overrides = {}) { createCropperRouter({ ...cropperClients, a2loftBase: cfg.a2loftBase, + domainCodes, removebgUrl: cfg.removebgUrl, noderedBase: cfg.noderedBase, ...cropGateContext, @@ -411,6 +426,22 @@ export function createApp(overrides = {}) { } }); + // ── error handler (async handlers are wrapped; this catches next(err)) ───── + // eslint-disable-next-line no-unused-vars + app.use((err, req, res, next) => { + const status = err.status || err.statusCode || 500; + if (status >= 500) console.error('[error]', req.method, req.path, err && err.message); + if (res.headersSent) return next(err); + const message = + status === 413 ? 'The request is too large.' + : status >= 500 ? 'An unexpected error occurred. Please try again.' + : err.message || 'The request could not be processed.'; + if (req.path === '/cropper' || req.path.startsWith('/auth/')) { + return errorPage(res, status, status === 413 ? 'Request too large' : 'Something went wrong', message); + } + res.status(status).json({ ok: false, message }); + }); + return app; } diff --git a/tools/test/cropper.test.js b/tools/test/cropper.test.js index a9ef58e..869f781 100644 --- a/tools/test/cropper.test.js +++ b/tools/test/cropper.test.js @@ -86,19 +86,21 @@ async function startFakeA2loft() { const fakeMinio = () => ({ presignedGetObject: async (_bucket, name) => `https://storage.example/${name}`, }); -const fakeQueue = () => { +const fakeQueue = (messageText) => { const calls = { getMessage: 0, deleteMessage: 0 }; const item = { messageId: 'm-1', popReceipt: 'pop-1', dequeueCount: 1, - messageText: JSON.stringify({ - upload_id: 'up-1', - file_name: 'id-domains/ucth/raw-pictures/x/img.jpg', - caption: '08068287222', - domain: 'ucth', - raw_file_id: 'raw-1', - }), + messageText: + messageText ?? + JSON.stringify({ + upload_id: 'up-1', + file_name: 'id-domains/ucth/raw-pictures/x/img.jpg', + caption: '08068287222', + domain: 'ucth', + raw_file_id: 'raw-1', + }), }; return { calls, @@ -155,7 +157,7 @@ async function startFakeNodered() { return { calls, server, stop: () => stopServer(server) }; } -async function boot({ cropAllowed = true, cropperClients } = {}) { +async function boot({ cropAllowed = true, cropperClients, queue } = {}) { const portal = await startFakePortal({ cropAllowed }); const a2loft = await startFakeA2loft(); const removebg = await startFakeRemovebg(); @@ -182,7 +184,7 @@ async function boot({ cropAllowed = true, cropperClients } = {}) { fetch: testFetch, cropperClients: cropperClients !== undefined ? cropperClients : { minio: fakeMinio(), - queue: fakeQueue(), + queue: queue || fakeQueue(), noderedAuth: { username: 'nr', password: 'nr-secret' }, carvekitKey: 'carve-key', }, @@ -418,3 +420,83 @@ test('baseUrl exposes the identity-prefixed a2loft base', async () => { await stop(); } }); + + +test('malformed request bodies are rejected, never hang the request', async () => { + const { base, a2loft, stop } = await boot(); + try { + const cookies = await login(base); + for (const path of ['/cropper/picture/review', '/cropper/dequeue', '/cropper/profile/picture?otp=1']) { + const res = await fetch(`${base}${path}`, { + method: 'POST', + headers: { cookie: cookies, 'content-type': 'application/json' }, + body: JSON.stringify({}), + }); + assert.equal(res.status, 400, `${path} → ${res.status}`); + const body = await res.json(); + assert.equal(body.ok, false); + } + assert.equal(a2loft.calls.filter((c) => c.url.includes('/profile/picture')).length, 0); + } finally { + await stop(); + } +}); + +test('a malformed queue item does not hang the request', async () => { + const { base, stop } = await boot({ queue: fakeQueue('not-json') }); + try { + const cookies = await login(base); + const res = await fetch(`${base}/cropper/picture/next`, { headers: { cookie: cookies } }); + assert.equal(res.status, 200); + const body = await res.json(); + assert.equal(body.ok, false); + assert.match(body.message, /malformed/i); + } finally { + await stop(); + } +}); + +test('save rejects an unknown domain without contacting a2loft', async () => { + const { base, a2loft, stop } = await boot(); + try { + const cookies = await login(base); + const res = await fetch(`${base}/cropper/profile/picture?otp=778899`, { + method: 'POST', + headers: { cookie: cookies, 'content-type': 'application/json' }, + body: JSON.stringify({ + pictureDataUrl: 'data:image/jpeg;base64,QUJD', + phone: '08068287222', + message: { + pop_receipt: 'custom', + message_id: 'm-1', + message: { upload_id: 'up-1', domain: '../../admin', agentDeviceHash: 'a' }, + }, + }), + }); + assert.equal(res.status, 400); + assert.equal(a2loft.calls.length, 0, 'a2loft never contacted'); + } finally { + await stop(); + } +}); + +test('picture fetches reject object keys outside the id-domains prefix', async () => { + const { base, stop } = await boot(); + try { + const cookies = await login(base); + // encoded traversal survives as a single path segment; a plain ../ is + // normalised away by the HTTP client before the route ever sees it + const traversal = await fetch(`${base}/cropper/picture/..%2F..%2Fsecret`, { headers: { cookie: cookies } }); + assert.equal(traversal.status, 400, `got ${traversal.status}`); + const escapeWithinPrefix = await fetch(`${base}/cropper/picture/id-domains_.._.._secret`, { + headers: { cookie: cookies }, + }); + assert.equal(escapeWithinPrefix.status, 400, `got ${escapeWithinPrefix.status}`); + const ok = await fetch(`${base}/cropper/picture/id-domains_ucth_raw-pictures_x_img.jpg`, { + headers: { cookie: cookies }, + }); + assert.equal(ok.status, 200); + } finally { + await stop(); + } +}); diff --git a/tools/test/server.test.js b/tools/test/server.test.js index 5636153..c082113 100644 --- a/tools/test/server.test.js +++ b/tools/test/server.test.js @@ -11,7 +11,7 @@ import { } from './helpers.js'; // Fake YOUreka portal: web-auth session + openfga verify. -async function startFakePortal({ cookieStatus = 'Activated', allowed = true } = {}) { +async function startFakePortal({ cookieStatus = 'Activated', allowed = true, authErrorMessage } = {}) { return startServer((req, res) => { let raw = ''; req.on('data', (c) => (raw += c)); @@ -23,6 +23,9 @@ async function startFakePortal({ cookieStatus = 'Activated', allowed = true } = if (req.method === 'POST' && req.url === '/api/web/auth') { const body = JSON.parse(raw || '{}'); if (!body.returnUrl) return json(200, { ok: false, message: 'returnUrl is required' }); + if (authErrorMessage) { + return json(200, { ok: false, message: authErrorMessage }); + } return json(200, { ok: true, sessionId: 's1', loginUrl: 'https://portal.example/login?return=1' }); } if (req.method === 'GET' && req.url.startsWith('/api/web/auth/s1/cookie')) { @@ -263,3 +266,17 @@ test('proxy validation: auth, otp, and domain are enforced before contacting a2l await stop(); } }); + + +test('upstream error text is HTML-escaped on the error page', async () => { + const { base, stop } = await bootApp({}, { authErrorMessage: 'portal said no' }); + try { + const res = await fetch(`${base}/auth/login`, manual); + assert.equal(res.status, 502); + const html = await res.text(); + assert.ok(!html.includes(''), 'script tag must not be interpolated raw'); + assert.match(html, /<script>alert\(1\)<\/script>/); + } finally { + await stop(); + } +});