feat(security): stop the app origin serving anything it does not recognise, and make the uploads origin configurable (#103)

The uploads directory is the only place in this application where content someone else authored is served over HTTP. #95 stopped 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, a restore, or a file written before that validation existed.

Two halves, complementary rather than alternative.

The app's own origin now serves uploads defensively. An allowlist of the three extensions the upload path can produce, so a `.html` or a `.svg` on disk is simply not a file this application hands out — 404, the same answer as a file that is not there, so the response cannot be used to learn which paths exist. An allowlist rather than a denylist because a denylist has to anticipate every type a browser might execute, which is a moving target across browsers and years, while this only has to know three. The content type is stated explicitly from that same list rather than sniffed or guessed from a name someone else chose, paired with `nosniff`. `default-src 'none'; sandbox` gives a directly-navigated file no capabilities at all, which is the only way one of these can do harm — an `<img>` embed does not execute script. Writes get 405 rather than falling through to a 404 that suggests the path is wrong.

The other half is the separate origin, which is the real fix, because the origin is the whole unit of trust in a browser. That needs a hostname and a certificate, which live outside this repository, so what is here is the switch: `UPLOADS_BASE_URL`, sent to the frontend at runtime through `/api/config` and joined onto stored paths by `uploadUrl`. Empty means the app's own origin, which is the default and what local development has, so nothing changes until it is pointed somewhere.

Stored paths stay site-relative. A stored value outlives any hostname baked into it, and rewriting them would be a migration to undo the day the hostname changes.

Runtime rather than built in, so one image serves every environment — the same reason `paypalClientId` and `demoMode` are already there. `UPLOADS_BASE_URL` has a line in `docker-compose.prod.yml` while still empty, deliberately: a Portainer stack variable with no line there is substituted into the file and never reaches the container, which is exactly how `UPLOADS_DIR` went missing on 2026-08-23.

Unset warns at boot, in the same shape as the admin gate — a working configuration with one defence switched off is worth saying out loud. Set without a scheme is refused outright, because a bare hostname joins onto a stored path as if it were relative and breaks every image on the site rather than failing visibly.

The compose guard now resolves `${VAR:-default}` to its default, which is what the container actually receives when the stack variable behind it is unset. A bare `${VAR}` is still left opaque, so a required variable referenced that way goes on counting as present — that check is about the line existing, not about the stack being filled in.

Closes #103
This commit is contained in:
2026-08-24 17:38:55 -05:00
parent 5d72c1c88b
commit cf1680dbfb
15 changed files with 492 additions and 11 deletions
+3 -2
View File
@@ -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>
+9 -1
View File
@@ -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[]> {
+2 -1
View File
@@ -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'}>
+2 -1
View File
@@ -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>
+11
View File
@@ -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>
+43
View File
@@ -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}`;
}