Merge pull request 'feat(security): stop the app origin serving anything it does not recognise, and make the uploads origin configurable (#103)' (#173) from feature/103-uploads-origin into main

Reviewed-on: #173

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 ?? '')
   });
 });
 

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]
   };
 }

backend/src/uploadTypes.ts

@@ -37,6 +37,22 @@ const EXTENSION_FOR_TYPE: Readonly<Record<string, string>> = {
   '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);
 }

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 `<img>` 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
+        // `<img>` load. Harmless while the origin is shared.
+        res.set('Cross-Origin-Resource-Policy', 'cross-origin');
+      }
+    })
+  );
+
+  return router;
+}

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('<!doctype html><script>alert(document.cookie)</script>', 'utf8');
+const HOSTILE_SVG = Buffer.from(
+  '<svg xmlns="http://www.w3.org/2000/svg"><script>alert(document.cookie)</script></svg>',
+  'utf8'
+);
+
+const written: string[] = [];
+
+async function place(name: string, bytes: Buffer): Promise<string> {
+  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 <img> 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');
+  });
+});

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<string, string> {
@@ -79,7 +94,7 @@ function environmentEntries(source: string): Map<string, string> {
     // 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;

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'));
+  });
+});

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();
+  });
+});

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

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] ? (
           <span style={{ position: 'relative', display: 'inline-block' }}>
-            <img src={images[0].image_path} alt="" style={{ width: 60 }} />
+            <img src={uploadUrl(images[0].image_path)} alt="" style={{ width: 60 }} />
             {images.length > 1 && (
               <Tag style={{ position: 'absolute', bottom: -4, right: -8, fontSize: 10 }}>+{images.length - 1}</Tag>
             )}
@@ -312,7 +313,7 @@ function Inventory() {
                 <Space wrap>
                   {editingItem.images.map(img => (
                     <div key={img.id} style={{ position: 'relative' }}>
-                      <AntImage src={img.image_path} width={80} height={80} style={{ objectFit: 'cover' }} />
+                      <AntImage src={uploadUrl(img.image_path)} width={80} height={80} style={{ objectFit: 'cover' }} />
                       <Button size="small" danger icon={<DeleteOutlined />} style={{ position: 'absolute', top: 0, right: 0 }}
                         onClick={() => handleDeleteImage(editingItem.id, img.id)} />
                     </div>

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<SiteConfig> {
   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<Item[]> {

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 => (
                 <List.Item actions={[<Button key="remove" danger size="small" onClick={() => handleRemove(item.item_id)}>Remove</Button>]}>
                   <List.Item.Meta
-                    avatar={item.images[0] && <img src={item.images[0].image_path} alt="" style={{ width: 60, height: 60, objectFit: 'cover' }} />}
+                    avatar={item.images[0] && <img src={uploadUrl(item.images[0].image_path)} alt="" style={{ width: 60, height: 60, objectFit: 'cover' }} />}
                     title={item.name}
                     description={
                       <Text type={isExpiringSoon(item.added_at, item.expires_at, now) ? 'danger' : 'secondary'}>

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<
       <Carousel ref={carouselRef} dots={hasMultiple}>
         {item.images.map(img => (
           <div key={img.id}>
-            <img src={img.image_path} alt={item.name} className="card-cover-img" />
+            <img src={uploadUrl(img.image_path)} alt={item.name} className="card-cover-img" />
           </div>
         ))}
       </Carousel>

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(
   <React.StrictMode>
     <ThemeModeProvider>

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/<id>.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}`;
+}