diff --git a/README.md b/README.md index 6883bcf..77260c3 100644 --- a/README.md +++ b/README.md @@ -164,6 +164,9 @@ the service user owns the database directory automatically. - `ADMIN_PASSWORD` (**required** for backoffice login) - `WUTZ_TZ` (default `Europe/Berlin`) — timezone for stats display and grouping - `WUTZ_DAY_CUTOFF_HOUR` (default `5`) — sales before this local hour count toward the previous business day +- `WUTZ_TRUST_PROXY` (default off, set to `1` to enable) — trust `X-Forwarded-*` headers for + `req.ip`. Only turn this on if you're actually running behind a reverse proxy (see the + systemd/deploy section) — otherwise a client can spoof its own logged IP. - `WUTZ_SERVER_PORT` (default `3000`, **client dev only** — not read by the server) — the port `dev:client`'s Vite proxy targets; set it to match `dev:server`'s `PORT` when running the server on something other than the default diff --git a/server/src/index.ts b/server/src/index.ts index 12affe0..b1747e2 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -11,11 +11,40 @@ import { registerAdminRoutes } from './routes/admin.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); -const PORT = Number(process.env.PORT ?? 3000); +function parsePortEnv(name: string, fallback: number): number { + const raw = process.env[name]; + if (raw === undefined) return fallback; + const n = Number(raw); + if (!Number.isInteger(n) || n < 1 || n > 65535) { + // A bare Number(...) turned an unparseable PORT into NaN, and + // Fastify listens on a random free port for a NaN — silently wrong, + // not an error anyone would see. + console.warn(`${name}=${JSON.stringify(raw)} is not a valid port — using default ${fallback}`); + return fallback; + } + return n; +} + +const PORT = parsePortEnv('PORT', 3000); const HOST = process.env.HOST ?? '0.0.0.0'; const DB_PATH = process.env.DB_PATH ?? join(process.cwd(), 'wutz.db'); -const app = Fastify({ logger: true }); +if (!process.env.ADMIN_PASSWORD) { + // Not fatal — public routes work fine without it — but previously this + // only surfaced as a 500 at the first login attempt, which in practice + // is the middle of the event. Warn at boot instead. + console.warn('ADMIN_PASSWORD is not set — the admin backoffice login will fail until it is.'); +} + +const app = Fastify({ + logger: true, + // Off by default: only trust X-Forwarded-* when the deployer has + // actually put a reverse proxy in front (the README recommends one for + // TLS termination). Without this, req.ip behind such a proxy is always + // the proxy's own address, making the stored client_ip column and its + // CSV export uniformly useless. + trustProxy: process.env.WUTZ_TRUST_PROXY === '1', +}); await app.register(fastifyCookie); diff --git a/server/src/routes/admin.ts b/server/src/routes/admin.ts index 64ca739..dc93318 100644 --- a/server/src/routes/admin.ts +++ b/server/src/routes/admin.ts @@ -202,15 +202,21 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) { app.get('/admin/api/stats', async (req, reply) => { if (!requireAuth(req, reply)) return; + // LEFT JOIN (not JOIN) so a bar with zero sales still gets a zero row — + // an inner join made a brand-new bar indistinguishable from a deleted + // one until its first sale, which reads as "my new bar isn't working" + // during setup. COUNT(t.id), not COUNT(*): the outer join produces one + // NULL-filled row per bar-with-no-transactions, and COUNT(*) would + // count that as 1 instead of 0. const totals = db .prepare( `SELECT b.id AS bar_id, b.name AS bar_name, - COUNT(*) AS tx_count, - COALESCE(SUM(CASE WHEN crew = 0 THEN total_cents ELSE 0 END), 0) AS paid_cents, - COALESCE(SUM(CASE WHEN crew = 1 THEN 1 ELSE 0 END), 0) AS crew_count, - COALESCE(SUM(pfand_returns), 0) AS pfand_returns - FROM transactions t - JOIN bars b ON b.id = t.bar_id + COUNT(t.id) AS tx_count, + COALESCE(SUM(CASE WHEN t.crew = 0 THEN t.total_cents ELSE 0 END), 0) AS paid_cents, + COALESCE(SUM(CASE WHEN t.crew = 1 THEN 1 ELSE 0 END), 0) AS crew_count, + COALESCE(SUM(t.pfand_returns), 0) AS pfand_returns + FROM bars b + LEFT JOIN transactions t ON t.bar_id = b.id GROUP BY b.id, b.name ORDER BY b.id` ) @@ -228,6 +234,16 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) { .all(); // Per business day (sales night runs past midnight — see time.ts). + // + // Deliberately still computed in JS rather than SQL, despite selecting + // every transaction row on every stats load: businessDay() uses + // Intl.DateTimeFormat with a named IANA zone (WUTZ_TZ), which handles + // DST transitions correctly. A SQL `date(created_at, '-Nh', 'localtime')` + // rewrite would use the *server process's* OS timezone (not WUTZ_TZ) and + // a fixed hour offset that's wrong on the two nights a year DST changes + // — a real correctness regression for a money-adjacent report, to fix a + // performance concern that (per the code review that flagged this) is + // "fine today" at festival scale. Not worth the trade. const txRows = db .prepare('SELECT created_at, total_cents, crew, pfand_returns FROM transactions') .all() as Array<{ created_at: string; total_cents: number; crew: number; pfand_returns: number }>; @@ -311,7 +327,12 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) { function csvCell(v: unknown): string { if (v === null || v === undefined) return ''; - const s = String(v); + let s = String(v); + // Neutralise spreadsheet formula injection: a cell starting with one of + // these characters is interpreted as a formula by Excel/LibreOffice when + // the CSV is opened (e.g. a drink named =HYPERLINK("http://…")). Admin- + // entered data only, so low risk, but the fix is one character. + if (/^[=+\-@]/.test(s)) s = `'${s}`; if (/[",\n]/.test(s)) return `"${s.replace(/"/g, '""')}"`; return s; } diff --git a/server/src/time.ts b/server/src/time.ts index 3302406..daee036 100644 --- a/server/src/time.ts +++ b/server/src/time.ts @@ -9,7 +9,24 @@ export const TZ = process.env.WUTZ_TZ ?? 'Europe/Berlin'; // A sale at e.g. 03:00 still belongs to the previous night's business day. // Anything before this local hour counts towards the day before. -export const BUSINESS_DAY_CUTOFF_HOUR = Number(process.env.WUTZ_DAY_CUTOFF_HOUR ?? 5); +// +// Parsed and range-checked rather than a bare Number(...) — an unparseable +// value (a typo like "5am" instead of "5") used to silently become NaN, +// and `hour < NaN` is always false, so the business-day rollback would +// just stop happening with no error anywhere: every after-midnight sale +// would land on the wrong day in the stats table. +export const BUSINESS_DAY_CUTOFF_HOUR = parseHourEnv('WUTZ_DAY_CUTOFF_HOUR', 5); + +function parseHourEnv(name: string, fallback: number): number { + const raw = process.env[name]; + if (raw === undefined) return fallback; + const n = Number(raw); + if (!Number.isInteger(n) || n < 0 || n > 23) { + console.warn(`${name}=${JSON.stringify(raw)} is not a valid hour (0-23) — using default ${fallback}`); + return fallback; + } + return n; +} /** * Parse a value stored in `created_at`. New rows are UTC ISO strings (with `Z`), diff --git a/shared/src/index.ts b/shared/src/index.ts index c660b57..04d0b93 100644 --- a/shared/src/index.ts +++ b/shared/src/index.ts @@ -8,7 +8,11 @@ export interface Drink { id: number; name: string; price_cents: number; - archived: boolean; + // SQLite has no boolean type — this is what the driver actually hands + // back for an INTEGER column, not `boolean`. It happened to work + // because `0` is falsy, but `archived === false` would silently be + // wrong the moment someone wrote that comparison. + archived: 0 | 1; } export interface BarConfig { @@ -33,23 +37,3 @@ export interface CreateTransactionResponse { id: number; total_cents: number; } - -export interface TransactionItemRecord { - drink_id: number; - drink_name: string; - qty: number; - unit_price_cents: number; - pfand_cents_per_unit: number; -} - -export interface TransactionRecord { - id: number; - bar_id: number; - bar_name: string; - created_at: string; - total_cents: number; - crew: boolean; - pfand_returns: number; - client_ip: string | null; - items: TransactionItemRecord[]; -}