Files
redefined-designs/backend/tests/unit/envValidation.test.ts
T
bermudalamb cf1680dbfb 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
2026-08-24 17:38:55 -05:00

219 lines
8.7 KiB
TypeScript

import { validateEnv } from '../../src/envValidation';
// The smallest environment that should boot: demo mode on, a database, and
// somewhere to put uploads. Everything else is optional or conditional.
const MINIMAL: NodeJS.ProcessEnv = {
DEMO_MODE: 'true',
PGHOST: 'localhost',
PGPORT: '5432',
PGUSER: 'someone',
PGPASSWORD: 'secret',
PGDATABASE: 'redefined',
UPLOADS_DIR: '/tmp/uploads'
};
const withEnv = (extra: NodeJS.ProcessEnv): NodeJS.ProcessEnv => ({ ...MINIMAL, ...extra });
// `undefined` removes a key rather than setting it to the string "undefined".
const without = (...names: string[]): NodeJS.ProcessEnv => {
const env = { ...MINIMAL };
for (const name of names) delete env[name];
return env;
};
describe('validateEnv', () => {
it('accepts the minimal environment local development already uses', () => {
expect(validateEnv(MINIMAL).errors).toEqual([]);
});
describe('variables that are always required', () => {
it.each(['PGHOST', 'PGPORT', 'PGUSER', 'PGPASSWORD', 'PGDATABASE', 'UPLOADS_DIR'])(
'refuses to boot without %s',
(name) => {
const { errors } = validateEnv(without(name));
expect(errors.some((e) => e.includes(name))).toBe(true);
}
);
// The fallback of '/app/uploads' is right inside the container and wrong
// everywhere else, which is why this one gets no reprieve.
it('names UPLOADS_DIR rather than silently accepting its fallback', () => {
const { errors } = validateEnv(without('UPLOADS_DIR'));
expect(errors.some((e) => e.includes('UPLOADS_DIR'))).toBe(true);
});
// Reporting one problem per boot makes fixing a fresh environment a
// sequence of restarts.
it('reports every problem at once rather than stopping at the first', () => {
const { errors } = validateEnv(without('PGHOST', 'PGUSER', 'UPLOADS_DIR'));
expect(errors).toHaveLength(3);
});
});
describe('DEMO_MODE', () => {
it('is required', () => {
const { errors } = validateEnv(without('DEMO_MODE'));
expect(errors.some((e) => e.includes('DEMO_MODE'))).toBe(true);
});
it.each(['true', 'false'])('accepts the exact value %s', (value) => {
const env = withEnv({
DEMO_MODE: value,
// Real payments need credentials; supplied so this case tests DEMO_MODE
// alone rather than tripping the PayPal rule.
PAYPAL_CLIENT_ID: 'id',
PAYPAL_CLIENT_SECRET: 'secret',
PAYPAL_WEBHOOK_ID: 'hook',
PAYPAL_ENV: 'sandbox'
});
expect(validateEnv(env).errors).toEqual([]);
});
// The whole point of this issue. Before, any value that was not exactly
// 'false' meant demo mode was on — so a typo silently stopped the shop
// charging anyone.
it.each(['False', 'FALSE', '0', 'no', 'flase', ''])(
'refuses %p rather than reading it as demo mode',
(value) => {
const { errors } = validateEnv(withEnv({ DEMO_MODE: value }));
expect(errors.some((e) => e.includes('DEMO_MODE'))).toBe(true);
}
);
it('quotes the value it was given, so the typo is visible in the message', () => {
const { errors } = validateEnv(withEnv({ DEMO_MODE: 'False' }));
expect(errors.find((e) => e.includes('DEMO_MODE'))).toContain("'False'");
});
});
describe('PayPal credentials', () => {
// QA runs with no PayPal on purpose, so this cannot be an unconditional
// requirement — it is tied to real payments being switched on.
it('are not required while demo mode is on', () => {
expect(validateEnv(withEnv({ DEMO_MODE: 'true' })).errors).toEqual([]);
});
it.each(['PAYPAL_CLIENT_ID', 'PAYPAL_CLIENT_SECRET', 'PAYPAL_WEBHOOK_ID', 'PAYPAL_ENV'])(
'are required when demo mode is off — missing %s',
(name) => {
const full = withEnv({
DEMO_MODE: 'false',
PAYPAL_CLIENT_ID: 'id',
PAYPAL_CLIENT_SECRET: 'secret',
PAYPAL_WEBHOOK_ID: 'hook',
PAYPAL_ENV: 'live'
});
delete full[name];
const { errors } = validateEnv(full);
expect(errors.some((e) => e.includes(name))).toBe(true);
}
);
});
describe('SMTP', () => {
it('is optional, and its absence is a warning rather than an error', () => {
const { errors, warnings } = validateEnv(MINIMAL);
expect(errors).toEqual([]);
expect(warnings.some((w) => w.includes('SMTP'))).toBe(true);
});
// Half-configured is worse than not configured: it looks set up and fails
// at send time.
it('refuses SMTP_USER without SMTP_PASSWORD', () => {
const { errors } = validateEnv(withEnv({ SMTP_USER: 'someone', PUBLIC_URL: 'https://x.test' }));
expect(errors.some((e) => e.includes('SMTP_PASSWORD'))).toBe(true);
});
it('refuses SMTP_PASSWORD without SMTP_USER', () => {
const { errors } = validateEnv(withEnv({ SMTP_PASSWORD: 'secret', PUBLIC_URL: 'https://x.test' }));
expect(errors.some((e) => e.includes('SMTP_USER'))).toBe(true);
});
it('accepts both together', () => {
const env = withEnv({
SMTP_USER: 'someone',
SMTP_PASSWORD: 'secret',
PUBLIC_URL: 'https://x.test',
MAIL_ALLOWLIST: 'someone@example.com'
});
expect(validateEnv(env).errors).toEqual([]);
});
});
describe('PUBLIC_URL', () => {
// It exists only to build links in email. A local environment that cannot
// send mail does not need it, and demanding it would break every existing
// local setup to prevent nothing.
it('is not required when no mail can be sent', () => {
expect(validateEnv(MINIMAL).errors).toEqual([]);
});
it('is required once SMTP is configured, because the links would read undefined', () => {
const env = withEnv({ SMTP_USER: 'someone', SMTP_PASSWORD: 'secret' });
const { errors } = validateEnv(env);
expect(errors.some((e) => e.includes('PUBLIC_URL'))).toBe(true);
});
});
describe('warnings that are not failures', () => {
// An environment that can send mail with no allowlist can reach real
// customers, which is what #87 exists to prevent.
it('warns when mail can be sent with no allowlist', () => {
const env = withEnv({ SMTP_USER: 'someone', SMTP_PASSWORD: 'secret', PUBLIC_URL: 'https://x.test' });
const { warnings } = validateEnv(env);
expect(warnings.some((w) => w.includes('MAIL_ALLOWLIST'))).toBe(true);
});
it('does not warn about the allowlist when there is no way to send mail', () => {
const { warnings } = validateEnv(MINIMAL);
expect(warnings.some((w) => w.includes('MAIL_ALLOWLIST'))).toBe(false);
});
it('warns when the admin gate is inactive', () => {
const { warnings } = validateEnv(MINIMAL);
expect(warnings.some((w) => w.includes('ADMIN_GATE_SECRET'))).toBe(true);
});
it('stays quiet about the admin gate once it is configured', () => {
const { warnings } = validateEnv(withEnv({ ADMIN_GATE_SECRET: 'a-secret' }));
expect(warnings.some((w) => w.includes('ADMIN_GATE_SECRET'))).toBe(false);
});
});
// A variable set to spaces is a configuration mistake, not a value.
it('treats a whitespace-only value as absent', () => {
const { errors } = validateEnv(withEnv({ UPLOADS_DIR: ' ' }));
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'));
});
});