diff --git a/backend/src/app.ts b/backend/src/app.ts index e55c9ce..a0027da 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -18,6 +18,7 @@ import clientErrorsRouter from './routes/clientErrors'; import { attachCustomer } from './middleware/customerAuth'; import { requireAdminGate } from './middleware/adminGate'; import { asyncRoute } from './asyncRoute'; +import { uploadsRouter } from './uploads'; const app = express(); // Express advertises itself in X-Powered-By by default, which hands an @@ -32,7 +33,14 @@ app.use(cookieParser()); // globally an unforwarded rejection here would hang every request in the app — // including the routes that wrap their own handlers correctly. app.use(asyncRoute(attachCustomer)); -app.use('/uploads', express.static(process.env.UPLOADS_DIR || '/app/uploads')); +app.use('/uploads', uploadsRouter(process.env.UPLOADS_DIR || '/app/uploads')); + +// Trimmed with a loop rather than a `/+$/` regex, which backtracks. +function trimTrailingSlashes(value: string): string { + let trimmed = value; + while (trimmed.endsWith('/')) trimmed = trimmed.slice(0, -1); + return trimmed; +} app.get('/api/config', (_req, res) => { const clientId = process.env.PAYPAL_CLIENT_ID; @@ -40,7 +48,19 @@ app.get('/api/config', (_req, res) => { res.json({ paypalClientId: isPlaceholder ? null : clientId, demoMode: process.env.DEMO_MODE !== 'false', - currency: process.env.SITE_CURRENCY || 'USD' + currency: process.env.SITE_CURRENCY || 'USD', + // Where uploaded images should be fetched from (#103). Empty means the + // app's own origin, which is both the default and what local development + // has — there is no second hostname on a laptop. Set it to a hostname of + // its own in production and user-supplied files stop sharing an origin with + // the application, which is the whole unit of trust in a browser. + // + // Sent at runtime rather than built in, so one image serves every + // environment, the same reason paypalClientId and demoMode are here. + // + // Trailing slash trimmed so callers can join with a stored path, which + // always begins with one, without producing a double. + uploadsBaseUrl: trimTrailingSlashes(process.env.UPLOADS_BASE_URL ?? '') }); }); diff --git a/backend/src/envValidation.ts b/backend/src/envValidation.ts index 2d6155d..568a876 100644 --- a/backend/src/envValidation.ts +++ b/backend/src/envValidation.ts @@ -162,16 +162,49 @@ function checkAdminGate(env: NodeJS.ProcessEnv): string[] { ]; } +// Optional, and the same shape as the admin gate above: unset is a working +// configuration with one defence switched off, which is worth saying out loud +// rather than leaving to be discovered. Set, it has to be an absolute origin — +// a value missing its scheme joins into a relative path and silently breaks +// every image on the site, which is a worse outcome than either extreme. +function checkUploadsOrigin(env: NodeJS.ProcessEnv): EnvValidation { + if (!isPresent(env, 'UPLOADS_BASE_URL')) { + return { + errors: [], + warnings: [ + 'UPLOADS_BASE_URL is not set — uploaded files are served from this application on its ' + + 'own origin, so anything reaching the uploads directory shares an origin with the site.' + ] + }; + } + + const value = (env.UPLOADS_BASE_URL ?? '').trim(); + if (!value.startsWith('https://') && !value.startsWith('http://')) { + return { + errors: [ + 'UPLOADS_BASE_URL must be an absolute origin including the scheme, such as ' + + 'https://uploads.example.com. Without one it joins into a relative path and every ' + + 'image on the site breaks.' + ], + warnings: [] + }; + } + + return { errors: [], warnings: [] }; +} + export function validateEnv(env: NodeJS.ProcessEnv): EnvValidation { const mail = checkMail(env); + const uploads = checkUploadsOrigin(env); return { errors: [ ...checkAlwaysRequired(env), ...checkDemoMode(env), ...checkPayPal(env), - ...mail.errors + ...mail.errors, + ...uploads.errors ], - warnings: [...mail.warnings, ...checkAdminGate(env)] + warnings: [...mail.warnings, ...checkAdminGate(env), ...uploads.warnings] }; } diff --git a/backend/src/uploadTypes.ts b/backend/src/uploadTypes.ts index 883e1ea..c479b93 100644 --- a/backend/src/uploadTypes.ts +++ b/backend/src/uploadTypes.ts @@ -37,6 +37,22 @@ const EXTENSION_FOR_TYPE: Readonly> = { 'image/webp': '.webp' }; +/** + * The content type a stored file should be served as, from its extension. + * + * The inverse of `extensionFor`, and derived from the same record so the two + * cannot drift. Returns null for anything else, which is what lets the uploads + * route refuse to serve a file it does not recognise — the case that matters is + * a file written before this validation existed, or one that arrived through a + * gap, since nothing the current upload path accepts can produce another + * extension. + */ +export function typeForExtension(extension: string): string | null { + const lowered = extension.toLowerCase(); + const found = Object.entries(EXTENSION_FOR_TYPE).find(([, ext]) => ext === lowered); + return found?.[0] ?? null; +} + export function isAllowedImageType(mimetype: string): boolean { return ALLOWED_IMAGE_TYPES.includes(mimetype); } diff --git a/backend/src/uploads.ts b/backend/src/uploads.ts new file mode 100644 index 0000000..8c050bc --- /dev/null +++ b/backend/src/uploads.ts @@ -0,0 +1,104 @@ +/** + * Serving user-uploaded files, defensively. + * + * Everything under the uploads directory was put there by someone other than + * the people who wrote this application, and it is served over HTTP. #95 stops + * a dangerous file being *stored*; this stops a stored file *doing damage* if + * one ever gets there anyway — through a gap, a path added later, or a file + * written before that validation existed. + * + * The real fix for that class is a separate origin, because the origin is the + * whole unit of trust in a browser (#103). That needs a hostname and a + * certificate, which live outside this repository, so what is here is the half + * that works either way: the app's own origin stops serving anything it does + * not recognise, and serves what it does recognise in a form that cannot be + * talked into executing. + * + * These two are complementary rather than alternatives. The separate origin + * still points at these same files, so the rules below apply there too. + */ + +import express, { Request, Response, NextFunction, Router } from 'express'; +import path from 'path'; +import { typeForExtension } from './uploadTypes'; + +/** + * A directly-navigated upload gets no capabilities at all. + * + * `default-src 'none'` leaves a document unable to load or run anything, and + * `sandbox` with no allowances drops it into an opaque origin, so even a file + * that somehow renders as markup cannot reach the site's cookies or DOM. + * + * This does nothing to an `` embed, which is the only way these files are + * legitimately used — a policy on an image response constrains the image's own + * (nonexistent) subresource loads, not the page displaying it. + */ +const UPLOAD_CSP = "default-src 'none'; sandbox"; + +/** + * Whether a request should reach the files at all. + * + * Only GET and HEAD: express.static ignores the rest anyway, but answering 405 + * says so rather than falling through to a 404 that suggests the path is wrong. + */ +function methodAllowed(method: string): boolean { + return method === 'GET' || method === 'HEAD'; +} + +export function uploadsRouter(directory: string): Router { + const router = express.Router(); + + router.use((req: Request, res: Response, next: NextFunction) => { + if (!methodAllowed(req.method)) { + res.set('Allow', 'GET, HEAD'); + res.status(405).json({ error: 'method not allowed' }); + return; + } + + // An allowlist rather than a denylist of dangerous extensions. A denylist + // has to anticipate every type a browser might execute, which is a moving + // target across browsers and years; this only has to know the three types + // the upload path can produce, and everything else — including a `.html` or + // a `.svg` sitting on disk from before there was any validation — is simply + // not a file this application will hand out. + const contentType = typeForExtension(path.extname(req.path)); + if (contentType === null) { + // 404 rather than 403: whether a file exists at that path is not + // something a stranger needs to be able to distinguish. + res.status(404).json({ error: 'not found' }); + return; + } + + // Set here rather than only in setHeaders below, so a request that never + // reaches a file still carries them. + res.set('X-Content-Type-Options', 'nosniff'); + res.set('Content-Security-Policy', UPLOAD_CSP); + next(); + }); + + router.use( + express.static(directory, { + // No directory listings and no index.html, both of which would be content + // this application did not write being served as if it had. + index: false, + // A dotfile in an upload directory is never something to hand out. + dotfiles: 'ignore', + setHeaders: (res: Response, filePath: string) => { + const contentType = typeForExtension(path.extname(filePath)); + if (contentType !== null) { + // Stated explicitly rather than left to express.static's extension + // lookup. Paired with nosniff, the type a browser sees is then the + // one this application chose, from a list of three, and never a guess + // made from the bytes. + res.set('Content-Type', contentType); + } + // Required once these are served from a hostname of their own: without + // it a resource-policy-conscious browser refuses the cross-origin + // `` load. Harmless while the origin is shared. + res.set('Cross-Origin-Resource-Policy', 'cross-origin'); + } + }) + ); + + return router; +} diff --git a/backend/tests/integration/uploadServing.integration.test.ts b/backend/tests/integration/uploadServing.integration.test.ts new file mode 100644 index 0000000..e280db4 --- /dev/null +++ b/backend/tests/integration/uploadServing.integration.test.ts @@ -0,0 +1,145 @@ +import request from 'supertest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { closeDb } from './setup/testDb'; + +/** + * What the application will and will not hand back out of the uploads + * directory. + * + * #95 covers what can be *stored*; this covers what can be *served*, which is a + * separate question because a file can reach that directory without going + * through the upload route — one written before the validation existed, one + * restored from a backup, one put there by a path added later. The uploads + * directory is the only place in this application where content someone else + * authored is served over HTTP, so it gets its own rules (#103). + * + * The files here are written straight to disk rather than uploaded, precisely + * because the interesting cases are the ones the upload route would refuse. + */ + +const UPLOADS_DIR = process.env.UPLOADS_DIR as string; + +const REAL_PNG = Buffer.from( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==', + 'base64' +); + +// What a stored cross-site scripting payload looks like: same-origin markup +// carrying script, reachable by navigating straight to it. +const HOSTILE_HTML = Buffer.from('', 'utf8'); +const HOSTILE_SVG = Buffer.from( + '', + 'utf8' +); + +const written: string[] = []; + +async function place(name: string, bytes: Buffer): Promise { + await fs.writeFile(path.join(UPLOADS_DIR, name), bytes); + written.push(name); + return `/uploads/${name}`; +} + +beforeAll(async () => { + await fs.mkdir(UPLOADS_DIR, { recursive: true }); +}); + +afterAll(async () => { + await Promise.all( + written.map((name) => fs.rm(path.join(UPLOADS_DIR, name), { force: true })) + ); + await pool.end(); + await closeDb(); +}); + +describe('serving uploaded files', () => { + it('serves a stored image', async () => { + const url = await place('serving-ok.png', REAL_PNG); + const res = await request(app).get(url); + expect(res.status).toBe(200); + expect(res.body).toEqual(REAL_PNG); + }); + + // Stated by this application from a list of three, rather than sniffed from + // the bytes or guessed from a name someone else chose. + it('states the content type explicitly', async () => { + const url = await place('serving-type.png', REAL_PNG); + const res = await request(app).get(url); + expect(res.headers['content-type']).toContain('image/png'); + expect(res.headers['x-content-type-options']).toBe('nosniff'); + }); + + // Not a header for the image's own sake — an image loads no subresources. + // It constrains the document a browser makes when someone navigates directly + // to the file, which is the only way one of these can do harm. + it('sends a policy that leaves a directly-navigated file with no capabilities', async () => { + const url = await place('serving-csp.png', REAL_PNG); + const res = await request(app).get(url); + expect(res.headers['content-security-policy']).toContain("default-src 'none'"); + expect(res.headers['content-security-policy']).toContain('sandbox'); + }); + + // Needed the moment these are served from a hostname of their own, or a + // browser refuses the cross-origin load. + it('allows the file to be embedded cross-origin', async () => { + const url = await place('serving-corp.png', REAL_PNG); + const res = await request(app).get(url); + expect(res.headers['cross-origin-resource-policy']).toBe('cross-origin'); + }); + + // The case the whole file exists for. Both of these execute script when + // navigated to, and neither can be produced by the upload route — so a file + // like this on disk means an earlier control failed, and this is the one that + // still holds. + it('refuses to serve markup that would run as the site', async () => { + const html = await place('serving-hostile.html', HOSTILE_HTML); + const svg = await place('serving-hostile.svg', HOSTILE_SVG); + + expect((await request(app).get(html)).status).toBe(404); + expect((await request(app).get(svg)).status).toBe(404); + }); + + it('refuses an extension it does not recognise, whatever the file contains', async () => { + // Genuinely a PNG, and still not served: the rule is about the name the + // browser will judge the response by, not about the bytes. + const url = await place('serving-mislabelled.txt', REAL_PNG); + expect((await request(app).get(url)).status).toBe(404); + }); + + // 404 rather than 403, so a stranger cannot use the response to learn which + // paths exist. + it('answers the same for a refused type as for a file that is not there', async () => { + const missing = await request(app).get('/uploads/serving-absent.png'); + const refused = await request(app).get('/uploads/serving-absent.html'); + expect(missing.status).toBe(404); + expect(refused.status).toBe(404); + }); + + it('accepts an extension in any case', async () => { + const url = await place('serving-shouted.PNG', REAL_PNG); + const res = await request(app).get(url); + expect(res.status).toBe(200); + expect(res.headers['content-type']).toContain('image/png'); + }); + + it('refuses to be written to', async () => { + const res = await request(app).post('/uploads/serving-ok.png').send('anything'); + expect(res.status).toBe(405); + expect(res.headers.allow).toBe('GET, HEAD'); + }); +}); + +describe('GET /api/config', () => { + // Runtime rather than built in, so one image serves every environment. Empty + // is the default and means the app's own origin, which is what local + // development has. + it('names the origin uploaded images should be fetched from', async () => { + const res = await request(app).get('/api/config'); + expect(res.status).toBe(200); + expect(res.body).toHaveProperty('uploadsBaseUrl'); + expect(typeof res.body.uploadsBaseUrl).toBe('string'); + }); +}); diff --git a/backend/tests/unit/composeEnvironment.test.ts b/backend/tests/unit/composeEnvironment.test.ts index a9fb363..6b998e8 100644 --- a/backend/tests/unit/composeEnvironment.test.ts +++ b/backend/tests/unit/composeEnvironment.test.ts @@ -63,6 +63,21 @@ const DEPLOYMENTS = [ } ] as const; +/** + * What the container receives for a `${VAR:-default}` entry when the stack + * variable behind it is unset — which is the case worth modelling, since a + * variable that is always set needs no default. + * + * A bare `${VAR}` is deliberately left alone. It stays an opaque non-empty + * string so that a required variable referenced that way still counts as + * present, which is the whole contract described below: what is checked here is + * that the *line exists*, not that the stack behind it is filled in. + */ +function resolveDefault(value: string): string { + const withDefault = /^\$\{[A-Z_0-9]+:-(.*)\}$/.exec(value); + return withDefault?.[1] ?? value; +} + // Only real environment entries — `- NAME=value` at an indented list position. // A mention inside a comment cannot match, because a comment line starts with #. function environmentEntries(source: string): Map { @@ -79,7 +94,7 @@ function environmentEntries(source: string): Map { // but RegExpExecArray cannot say so, and #101 made the compiler insist. const [, name, value] = match ?? []; if (name !== undefined && value !== undefined) { - entries.set(name, value.trim()); + entries.set(name, resolveDefault(value.trim())); } } return entries; diff --git a/backend/tests/unit/envValidation.test.ts b/backend/tests/unit/envValidation.test.ts index 1c41ae7..ef900ec 100644 --- a/backend/tests/unit/envValidation.test.ts +++ b/backend/tests/unit/envValidation.test.ts @@ -187,3 +187,32 @@ describe('validateEnv', () => { expect(errors.some((e) => e.includes('UPLOADS_DIR'))).toBe(true); }); }); + +// #103. Optional, like the admin gate: unset is a working configuration with +// one defence switched off, and set-but-wrong is worse than either. +describe('UPLOADS_BASE_URL', () => { + it('warns when it is unset, since the isolation is simply off', () => { + const { errors, warnings } = validateEnv(MINIMAL); + expect(errors).not.toContainEqual(expect.stringContaining('UPLOADS_BASE_URL')); + expect(warnings).toContainEqual(expect.stringContaining('UPLOADS_BASE_URL is not set')); + }); + + it('is satisfied by an absolute origin', () => { + const { errors, warnings } = validateEnv(withEnv({ UPLOADS_BASE_URL: 'https://uploads.example.com' })); + expect(errors).not.toContainEqual(expect.stringContaining('UPLOADS_BASE_URL')); + expect(warnings).not.toContainEqual(expect.stringContaining('UPLOADS_BASE_URL')); + }); + + // A hostname with no scheme joins onto a stored path as if it were relative, + // which breaks every image on the site rather than failing visibly. Refusing + // to start is the kinder outcome. + it('refuses a value with no scheme', () => { + const { errors } = validateEnv(withEnv({ UPLOADS_BASE_URL: 'uploads.example.com' })); + expect(errors).toContainEqual(expect.stringContaining('absolute origin')); + }); + + it('refuses a path rather than an origin', () => { + const { errors } = validateEnv(withEnv({ UPLOADS_BASE_URL: '/uploads' })); + expect(errors).toContainEqual(expect.stringContaining('absolute origin')); + }); +}); diff --git a/backend/tests/unit/uploadTypes.test.ts b/backend/tests/unit/uploadTypes.test.ts index b45dde4..5797360 100644 --- a/backend/tests/unit/uploadTypes.test.ts +++ b/backend/tests/unit/uploadTypes.test.ts @@ -3,7 +3,8 @@ import { SIGNATURE_BYTES, extensionFor, isAllowedImageType, - signatureMatches + signatureMatches, + typeForExtension } from '../../src/uploadTypes'; // Heads long enough to satisfy the WebP check, which needs twelve bytes. @@ -105,3 +106,42 @@ describe('signatureMatches', () => { expect(SIGNATURE_BYTES).toBeGreaterThanOrEqual(12); }); }); + +// The inverse of extensionFor, and the gate on what the uploads route will +// serve. #103: anything it does not recognise is not handed out, which is how a +// file written before there was any validation stops being reachable. +describe('typeForExtension', () => { + it('names the type for each extension the upload path can produce', () => { + expect(typeForExtension('.jpg')).toBe('image/jpeg'); + expect(typeForExtension('.png')).toBe('image/png'); + expect(typeForExtension('.webp')).toBe('image/webp'); + }); + + it('is the inverse of extensionFor for every allowed type', () => { + for (const type of ALLOWED_IMAGE_TYPES) { + const extension = extensionFor(type); + expect(extension).not.toBeNull(); + expect(typeForExtension(extension as string)).toBe(type); + } + }); + + it('matches case-insensitively, since an extension on disk may be shouted', () => { + expect(typeForExtension('.JPG')).toBe('image/jpeg'); + }); + + // The whole point. Each of these is a file type that executes when navigated + // to, and none of them can be produced by the current upload path — so + // finding one on disk means something went wrong earlier, and refusing to + // serve it is the last line. + it('refuses the types that would run as the site', () => { + expect(typeForExtension('.html')).toBeNull(); + expect(typeForExtension('.htm')).toBeNull(); + expect(typeForExtension('.svg')).toBeNull(); + expect(typeForExtension('.js')).toBeNull(); + expect(typeForExtension('.xml')).toBeNull(); + }); + + it('refuses a file with no extension at all', () => { + expect(typeForExtension('')).toBeNull(); + }); +}); diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 940e6bb..4348ed8 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -153,6 +153,20 @@ services: - USPS_CLIENT_SECRET=${USPS_CLIENT_SECRET} - ADMIN_GATE_SECRET=${ADMIN_GATE_SECRET} + + # The origin uploaded images are fetched from (#103). Empty means this + # application serves them from its own origin, which works and is what + # every environment does today — so this is safe to leave unset while the + # hostname below does not exist yet. + # + # Present as a line even while empty, deliberately: a Portainer stack + # variable with no line here is substituted into this file and never + # reaches the container, which is exactly how UPLOADS_DIR went missing. + # + # Setting it needs an Nginx Proxy Manager host for the name, pointing at + # this same container, and a certificate that covers it. Until then the + # server warns at boot that the defence is off rather than staying silent. + - UPLOADS_BASE_URL=${UPLOADS_BASE_URL:-} volumes: # Production's own uploads directory. QA writes to # /volume1/configs/redefined-designs-qa/uploads; sharing this one would diff --git a/frontend/src/admin/Admin.tsx b/frontend/src/admin/Admin.tsx index 7bcb6b9..23a0a44 100755 --- a/frontend/src/admin/Admin.tsx +++ b/frontend/src/admin/Admin.tsx @@ -37,6 +37,7 @@ import CategoryTreeSelect from './CategoryTreeSelect'; import ItemCard from '../components/ItemCard'; import InventoryFilters from './InventoryFilters'; import { ItemFilters, EMPTY_FILTERS } from '../filters'; +import { uploadUrl } from '../uploadUrl'; const { Header, Content } = Layout; const { Title } = Typography; @@ -195,7 +196,7 @@ function Inventory() { render: (images: Item['images']) => images[0] ? ( - + {images.length > 1 && ( +{images.length - 1} )} @@ -312,7 +313,7 @@ function Inventory() { {editingItem.images.map(img => (
- +
diff --git a/frontend/src/api.ts b/frontend/src/api.ts index ff4916f..680c02c 100755 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -1,5 +1,6 @@ import type { ItemFilters } from './filters'; import { filtersToSearchParams } from './filters'; +import { setUploadsBase } from './uploadUrl'; export interface ItemTag { id: number; @@ -44,11 +45,18 @@ export interface SiteConfig { paypalClientId: string | null; demoMode: boolean; currency: string; + /** Origin for uploaded images. Empty means the app's own — see uploadUrl. */ + uploadsBaseUrl: string; } export async function fetchConfig(): Promise { const res = await fetch('/api/config'); - return res.json(); + const config = (await res.json()) as SiteConfig; + // Applied here rather than by each caller, so no caller can fetch the config + // and forget to — the uploads origin is a property of the deployment, not of + // whichever screen happened to ask for it. + setUploadsBase(config.uploadsBaseUrl); + return config; } export async function fetchItems(filters?: ItemFilters): Promise { diff --git a/frontend/src/cart/Cart.tsx b/frontend/src/cart/Cart.tsx index e231c45..829eabc 100644 --- a/frontend/src/cart/Cart.tsx +++ b/frontend/src/cart/Cart.tsx @@ -27,6 +27,7 @@ import { useCart } from './CartContext'; import { useCustomerAuth } from '../customer/CustomerAuthContext'; import { useNow } from './useNow'; import { timeRemaining, isExpiringSoon, hasLapsedItem } from './reservation'; +import { uploadUrl } from '../uploadUrl'; // The display has one-minute resolution, so half a minute keeps it honest // without being busy. Once a second would be wasted work. @@ -197,7 +198,7 @@ export default function Cart() { renderItem={item => ( handleRemove(item.item_id)}>Remove]}> } + avatar={item.images[0] && } title={item.name} description={ diff --git a/frontend/src/components/ItemCard.tsx b/frontend/src/components/ItemCard.tsx index 67f7fbd..b02c1e5 100755 --- a/frontend/src/components/ItemCard.tsx +++ b/frontend/src/components/ItemCard.tsx @@ -18,6 +18,7 @@ import { useCustomerAuth } from '../customer/CustomerAuthContext'; import AuthPromptModal from '../customer/AuthPromptModal'; import { useFavorites } from '../customer/FavoritesContext'; import { addFavorite, removeFavorite, setFavoriteAlerts } from '../customer/favoritesApi'; +import { uploadUrl } from '../uploadUrl'; const { Text, Title } = Typography; @@ -142,7 +143,7 @@ export default function ItemCard({ item, onChanged, preview = false }: Readonly< {item.images.map(img => (
- {item.name} + {item.name}
))} diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index d760310..5d72c13 100755 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -25,6 +25,7 @@ import { CartProvider } from './cart/CartContext'; import { FavoritesProvider } from './customer/FavoritesContext'; import { ThemeModeProvider, useThemeMode } from './theme/ThemeContext'; import './styles.css'; +import { fetchConfig } from './api'; // The brand accent is monochrome, so it inverts between themes rather than // switching to a different hue. @@ -214,6 +215,16 @@ function Root() { ); } +// Fired at entry purely for its side effect: it sets the origin uploaded +// images are fetched from (#103). Not awaited, because nothing should wait on +// it — anything that renders first gets a site-relative path, which the app's +// own origin still serves. +// +// Rejections are swallowed on purpose. A config that cannot be fetched is a +// broken deployment which every other request will report; failing here would +// only replace the app with an error page before it has drawn anything. +void fetchConfig().catch(() => {}); + ReactDOM.createRoot(document.getElementById('root')!).render( diff --git a/frontend/src/uploadUrl.ts b/frontend/src/uploadUrl.ts new file mode 100644 index 0000000..eb538df --- /dev/null +++ b/frontend/src/uploadUrl.ts @@ -0,0 +1,43 @@ +/** + * Where an uploaded image is fetched from. + * + * Image paths are stored in the database as site-relative — `/uploads/.jpg` + * — and that is deliberate: a stored value outlives any hostname it might be + * baked with, and rewriting them would be a migration to undo the day the host + * changes. So the origin is joined on here instead, from a value the server + * sends at runtime (#103). + * + * Empty base means the app's own origin, which is the default and is what local + * development has — there is no second hostname on a laptop. Point it at one in + * production and user-supplied files stop sharing an origin with the + * application. + * + * The base is cached in a module variable rather than threaded through context, + * because it is a deployment constant: one value, fetched once, never changing + * while the tab is open. Anything rendered before the fetch resolves gets the + * relative path, which still works — the app's origin goes on serving these + * files, hardened, and the separate host points at the same directory. It + * simply misses the isolation for that first paint. + */ + +let base = ''; + +/** + * Called once, from the config the app already fetches at startup. Trailing + * slashes are trimmed on the server, and again here, so that a base configured + * either way joins cleanly with a path that always begins with one. + */ +export function setUploadsBase(value: string | undefined | null): void { + // Trimmed with a loop rather than a `/+$/` regex, which backtracks. + let trimmed = value ?? ''; + while (trimmed.endsWith('/')) trimmed = trimmed.slice(0, -1); + base = trimmed; +} + +export function uploadUrl(storedPath: string): string { + // Absolute already, or empty. Either way there is nothing to join: returning + // it untouched means a value that was somehow stored absolute keeps working + // rather than being mangled into a nonsense URL. + if (!base || !storedPath.startsWith('/')) return storedPath; + return `${base}${storedPath}`; +}