feat(security): stop the app origin serving anything it does not recognise, and make the uploads origin configurable (#103) #173
+22
-2
@@ -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 ?? '')
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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]
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
@@ -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'));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
@@ -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[]> {
|
||||
|
||||
@@ -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'}>
|
||||
|
||||
@@ -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>
|
||||
|
||||
@@ -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>
|
||||
|
||||
@@ -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}`;
|
||||
}
|
||||
Reference in New Issue
Block a user