fix(config): trim whitespace around NODE_ENV, LOG_LEVEL and DEFAULT_LANGUAGE once at boot

NODE_ENV="production " passed the boot check but matched none of the
exact production comparisons, so the web UI was never served and every
page answered {"error":"Cannot GET /"}, while the banner showed a clean
"production". The boot import now trims these three variables in
process.env before anything else reads them, so Express, the plugin
children and readEnv() agree, and reports each change as a process
warning. Casing stays as it was. A padded "test" is only reported, so an
instance cannot switch to an in-memory database after an update. The
banner prints NODE_ENV as set instead of lowercasing it.

Refs #2565
This commit is contained in:
Maurice
2026-10-12 09:27:19 +02:00
parent 7397e33c31
commit 1ea72dca97
7 changed files with 124 additions and 7 deletions
+4 -2
View File
@@ -8,10 +8,10 @@ the pre-init Express middleware.
```
env.schema.ts Zod catalog of the whole env surface; fail-fast at boot
derive.ts pure (raw env) → typed namespace functions, exact per-site coercions
env.ts readEnv() live accessor + validateEnvAtBoot()
env.ts readEnv() live accessor + normalizeEnvAtBoot() + validateEnvAtBoot()
app-url.ts getAppUrl()/getMcpSafeUrl() instance base-URL resolution
data-paths.ts resolveDataPaths(): data/, uploads/, backups, tmp, logs, key files, DB file
boot-validate.ts side-effect import used by index.ts (validates before other modules load)
boot-validate.ts side-effect import used by index.ts (trims, then validates before other modules load)
parsers.ts shared coercion helpers (boolTrueLoose, numberOr, csvList, …)
```
@@ -200,3 +200,5 @@ The exemption list is enforced by the `no-restricted-syntax` ban on
- NODE_ENV comparisons are case-sensitive at some sites (HSTS activation,
platform statics, spa-fallback, authService dev_mode, oidcService
frontendUrl) and case-insensitive at others.
Surrounding whitespace is stripped once at boot (`normalizeEnvAtBoot`,
together with LOG_LEVEL and DEFAULT_LANGUAGE), the casing is not.
+7 -3
View File
@@ -1,12 +1,16 @@
/**
* Side-effect module: validates the environment the moment it is imported.
* Side-effect module: normalizes and validates the environment the moment it
* is imported.
*
* index.ts imports this immediately after `import 'dotenv/config'` — as a
* side-effect import (not a function call) so validation runs BEFORE the rest
* of the import graph executes its module-load side effects (config.ts key
* resolution, ...). A plain call in index.ts's body
* would run after all of those, because imports hoist.
* would run after all of those, because imports hoist. The trim runs first,
* so everything after it, Express and the plugin children included, reads the
* cleaned value.
*/
import { validateEnvAtBoot } from './env';
import { normalizeEnvAtBoot, validateEnvAtBoot } from './env';
normalizeEnvAtBoot();
validateEnvAtBoot();
+1
View File
@@ -60,6 +60,7 @@ export const envSchema = z.object({
HOST: anyString,
// NODE_ENV and TZ are deliberately unvalidated: non-standard values (e.g.
// NODE_ENV=staging) behave as "not production" today and must keep booting.
// Surrounding whitespace on NODE_ENV is trimmed before this runs (env.ts).
NODE_ENV: anyString,
TZ: anyString,
LOG_LEVEL: oneOf(['error', 'warn', 'info', 'debug']),
+39
View File
@@ -39,6 +39,45 @@ function reportedValue(key: string, value: string | undefined): string {
return SECRET_ENV_KEYS.has(key) ? '***' : JSON.stringify(value);
}
/**
* Variables whose surrounding whitespace is stripped once at boot. The schema
* already validates them trimmed, but every reader compares the raw string:
* NODE_ENV="production " served no web UI at all (#2565), LOG_LEVEL="debug "
* fell back to info, DEFAULT_LANGUAGE="de " to English. Deliberately a short
* list, not every variable: a secret may carry its spaces on purpose.
*/
const TRIMMED_AT_BOOT = ['NODE_ENV', 'LOG_LEVEL', 'DEFAULT_LANGUAGE'] as const;
/**
* Strips the whitespace a .env line or a compose file left around the values in
* TRIMMED_AT_BOOT and writes the result back, so Express (`app.get('env')`),
* the plugin child processes and any library reading NODE_ENV itself see the
* same value as readEnv(). Casing is left alone. Each change is reported, since
* the banner cannot show a trailing space.
*
* NODE_ENV="test " is the one exception: trimmed it would switch a running
* instance onto an in-memory database without crons, so it is only reported.
*/
export function normalizeEnvAtBoot(raw: RawEnv = process.env as RawEnv): void {
for (const key of TRIMMED_AT_BOOT) {
const value = raw[key];
if (value === undefined) continue;
const trimmed = value.trim();
if (trimmed === value) continue;
if (key === 'NODE_ENV' && trimmed === 'test') {
bootWarning(`${key}=${JSON.stringify(value)} has surrounding whitespace and is not treated as "test".`);
continue;
}
raw[key] = trimmed;
bootWarning(`${key}=${JSON.stringify(value)} has surrounding whitespace, using ${JSON.stringify(trimmed)}.`);
}
}
/** Runs before the file logger exists, so it goes to stderr as a process warning. */
function bootWarning(message: string): void {
process.emitWarning(message, { code: 'TREK_ENV_WHITESPACE' });
}
/**
* Fail-fast startup validation: a variable that is PRESENT but malformed aborts
* boot with an aggregated report; unset/blank variables always pass (their
+1 -1
View File
@@ -53,7 +53,7 @@ const onListen = () => {
...(HOST ? [` Host: ${HOST}`] : []),
` Container Port: ${PORT}`,
` App URL: ${appUrl}`,
` Environment: ${env.app.nodeEnv?.toLowerCase() || 'development'}`,
` Environment: ${env.app.nodeEnv || 'development'}`,
` Timezone: ${tz}`,
` Origins: ${origins}`,
` Log level: ${LOG_LVL}`,
@@ -0,0 +1,70 @@
import { normalizeEnvAtBoot, readEnv } from '../../../src/app-config/env';
import { describe, it, expect, vi, afterEach } from 'vitest';
/** Everything one normalisation printed as a warning. */
function warnings(raw: Record<string, string | undefined>): string {
const warn = vi.spyOn(process, 'emitWarning').mockImplementation(() => {});
normalizeEnvAtBoot(raw);
const printed = warn.mock.calls.map((c) => String(c[0])).join('\n');
warn.mockRestore();
return printed;
}
describe('normalizeEnvAtBoot (#2565)', () => {
afterEach(() => {
vi.restoreAllMocks();
vi.unstubAllEnvs();
vi.resetModules();
});
it('trims a padded NODE_ENV in place and says so', () => {
for (const padded of ['production ', ' production', 'production\r', '\tproduction\n']) {
const raw: Record<string, string | undefined> = { NODE_ENV: padded };
const printed = warnings(raw);
expect(raw.NODE_ENV, JSON.stringify(padded)).toBe('production');
expect(printed).toContain(`NODE_ENV=${JSON.stringify(padded)}`);
expect(printed).toContain('"production"');
}
});
it('trims LOG_LEVEL and DEFAULT_LANGUAGE the same way', () => {
const raw: Record<string, string | undefined> = { LOG_LEVEL: 'debug ', DEFAULT_LANGUAGE: ' de' };
const printed = warnings(raw);
expect(raw.LOG_LEVEL).toBe('debug');
expect(raw.DEFAULT_LANGUAGE).toBe('de');
expect(printed).toContain('LOG_LEVEL="debug "');
expect(printed).toContain('DEFAULT_LANGUAGE=" de"');
});
it('leaves clean, unset and differently cased values alone and stays quiet', () => {
const raw: Record<string, string | undefined> = { NODE_ENV: 'Production', LOG_LEVEL: 'info' };
expect(warnings(raw)).toBe('');
expect(raw).toEqual({ NODE_ENV: 'Production', LOG_LEVEL: 'info' });
});
it('does not turn a padded "test" into the test runtime', () => {
// NODE_ENV=test means an in-memory database and no crons. An instance that
// has been running on "test " with its file database must not wake up on
// an empty one after an update, so this one is only reported.
const raw: Record<string, string | undefined> = { NODE_ENV: 'test ' };
const printed = warnings(raw);
expect(raw.NODE_ENV).toBe('test ');
expect(printed).toContain('NODE_ENV="test "');
});
it('never touches other variables, secrets keep their surrounding spaces', () => {
const raw: Record<string, string | undefined> = { SMTP_PASS: ' pass ', TZ: 'UTC ' };
expect(warnings(raw)).toBe('');
expect(raw).toEqual({ SMTP_PASS: ' pass ', TZ: 'UTC ' });
});
it('runs from the boot import, so everything reading process.env sees the trimmed value', async () => {
vi.stubEnv('NODE_ENV', 'production ');
vi.spyOn(process, 'emitWarning').mockImplementation(() => {});
await import('../../../src/app-config/boot-validate');
expect(process.env.NODE_ENV).toBe('production');
expect(readEnv().app.nodeEnv).toBe('production');
expect(readEnv().app.isProduction).toBe(true);
});
});
+2 -1
View File
@@ -38,7 +38,8 @@ aborting, because `reset-admin.js` — the way back into a locked-out instance
to keep working. The three plugin caps `TREK_PLUGIN_AI_PER_DAY`, `TREK_PLUGIN_NOTIFY_PER_DAY` and
`TREK_PLUGIN_AUDIT_MAX_ROWS` are read by the plugin host rather than by the boot schema, so a malformed value there
falls back to the default without a warning. `NODE_ENV` and `TZ` are not validated at all, so a non-standard value
like `NODE_ENV=staging` still boots.
like `NODE_ENV=staging` still boots. Spaces around `NODE_ENV`, `LOG_LEVEL` and `DEFAULT_LANGUAGE` are trimmed at startup with a
warning in the log; the web UI is only served with `NODE_ENV=production` in lowercase.
---