diff --git a/package.json b/package.json index 92ae762d..73767958 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ "scripts": { "preinstall": "node -e \"var v = +process.versions.node.split('.')[0]; if (v < 22) { console.error('Node 22 or newer required (got ' + process.versions.node + '). Run: nvm use 22'); process.exit(1); }\"", "gen:index": "bun run ./scripts/gen-index.ts", + "fix:env": "bun run ./scripts/fix-env.ts", "gen:ecosystem": "bun run --env-file=.env ./scripts/gen-ecosystem.ts", "predev": "bun run ./scripts/gen-index.ts", "dev": "bun --env-file=.env --watch src/server.tsx", diff --git a/scripts/fix-env.ts b/scripts/fix-env.ts new file mode 100644 index 00000000..9cb9d34b --- /dev/null +++ b/scripts/fix-env.ts @@ -0,0 +1,14 @@ +// `bun run fix:env` — give every variable in `.env` the `OFFICERDEV_` prefix. +// +// A thin front for `servers/env-prefix.ts`, which is where the rules live. Deliberately that way round: +// the same conversion runs on every restart from the server settings screen, and a router reaching into +// `scripts/` to do it would make a maintenance script part of the request path. + +import { join } from 'node:path'; +import { fixEnvFile } from '../src/servers/env-prefix'; + +const path = process.argv[2] ?? join(process.cwd(), '.env'); +const { changed, alreadyPrefixed } = fixEnvFile(path); + +if (changed.length) console.log(`[fix:env] prefixed ${changed.length}: ${changed.join(', ')}`); +else console.log(`[fix:env] nothing to do (${alreadyPrefixed} already prefixed)`); diff --git a/scripts/gen-index.ts b/scripts/gen-index.ts index 14443f5e..0b1ad5a1 100644 --- a/scripts/gen-index.ts +++ b/scripts/gen-index.ts @@ -27,17 +27,23 @@ const output = join(root, 'src/apps/officer-web/index.gen.html'); const argUrl = process.argv[2]?.trim(); const envPath = join(root, '.env'); -if (!argUrl && !process.env.PUBLIC_URL && existsSync(envPath)) { +// Prefixed first, bare as the fallback — the same rule every other consumer follows. This script reads +// the file itself rather than relying on the environment, because it is run by hand as often as by the +// setup, and `bun run gen:index` in a bare shell has no `.env` loaded. +const envValue = (): string | undefined => process.env.OFFICERDEV_PUBLIC_URL ?? process.env.PUBLIC_URL; + +if (!argUrl && !envValue() && existsSync(envPath)) { for (const line of (await Bun.file(envPath).text()).split('\n')) { - const match = line.match(/^\s*PUBLIC_URL\s*=\s*(.*)$/); - if (match) process.env.PUBLIC_URL = match[1]!.trim().replace(/^["']|["']$/g, ''); + // Either spelling, so a file that has been through `bun run fix:env` and one that has not both work. + const match = line.match(/^\s*(?:OFFICERDEV_)?PUBLIC_URL\s*=\s*(.*)$/); + if (match) process.env.OFFICERDEV_PUBLIC_URL = match[1]!.trim().replace(/^["']|["']$/g, ''); } } -const publicUrl = (argUrl || process.env.PUBLIC_URL || '').replace(/\/+$/, ''); +const publicUrl = (argUrl || envValue() || '').replace(/\/+$/, ''); if (!publicUrl) { console.error('[gen-index] no public URL. Pass one — `bun gen:index https://officer.example.com` —'); - console.error('[gen-index] or set PUBLIC_URL in .env.'); + console.error('[gen-index] or set OFFICERDEV_PUBLIC_URL in .env.'); process.exit(1); } diff --git a/scripts/rename-capabilities-to-permissions.ts b/scripts/rename-capabilities-to-permissions.ts index 8ccdec84..a1ba55f7 100644 --- a/scripts/rename-capabilities-to-permissions.ts +++ b/scripts/rename-capabilities-to-permissions.ts @@ -37,7 +37,7 @@ import postgres from 'postgres'; // `role === 'Super Admin'` before it reads the table at all, so the account that can fix things can // always sign in. Non-owners degrade to core-only until the rename completes. -const url = process.env.POSTGRES_URL; +const url = process.env.OFFICERDEV_POSTGRES_URL ?? process.env.POSTGRES_URL; if (!url) { console.error(' POSTGRES_URL is not set. Run from the platform directory so Bun loads .env.'); process.exit(1); diff --git a/scripts/setup/officer-setup/lib/env.sh b/scripts/setup/officer-setup/lib/env.sh index b4d7ec1a..d7a1b8e2 100644 --- a/scripts/setup/officer-setup/lib/env.sh +++ b/scripts/setup/officer-setup/lib/env.sh @@ -60,13 +60,22 @@ write_env() { # # Everything Officer reads at runtime. Kept at 0600 and owned by ${USERNAME}: it # holds the token-signing secret and the database credential. +# +# EVERY NAME IS PREFIXED \`OFFICERDEV_\`. Bun auto-loads this file into every +# process started in the platform directory, so a bare PORT or POSTGRES_URL +# reaches the sidecars, the tools they spawn and anything an agent runs from +# here — names half the software in the world already uses. Prefixed, they +# cannot be picked up by accident. +# +# Every consumer reads \`OFFICERDEV_X ?? X\`, so an older unprefixed file still +# works. \`bun run fix:env\` converts one in place. -PORT="${ENV_PORT}" +OFFICERDEV_PORT="${ENV_PORT}" # Where Officer is reached from a browser. Not derivable — see the section. -PUBLIC_URL="${ENV_PUBLIC_URL}" +OFFICERDEV_PUBLIC_URL="${ENV_PUBLIC_URL}" -POSTGRES_URL="${POSTGRES_URL}" +OFFICERDEV_POSTGRES_URL="${POSTGRES_URL}" ENVF diff --git a/src/databases/marketplace_db/drizzle.config.ts b/src/databases/marketplace_db/drizzle.config.ts index efa10430..673f0ef2 100644 --- a/src/databases/marketplace_db/drizzle.config.ts +++ b/src/databases/marketplace_db/drizzle.config.ts @@ -20,7 +20,7 @@ try { // because drizzle-kit loads this file outside the workspace's module resolution. const url = (() => { if (process.env.MARKETPLACE_POSTGRES_URL) return process.env.MARKETPLACE_POSTGRES_URL; - const base = process.env.POSTGRES_URL; + const base = process.env.OFFICERDEV_POSTGRES_URL ?? process.env.POSTGRES_URL; if (!base) throw new Error('MARKETPLACE_POSTGRES_URL or POSTGRES_URL is required'); const parsed = new URL(base); parsed.pathname = '/officer_marketplace'; diff --git a/src/databases/marketplace_db/src/db.ts b/src/databases/marketplace_db/src/db.ts index 205a92be..bf37f7d7 100644 --- a/src/databases/marketplace_db/src/db.ts +++ b/src/databases/marketplace_db/src/db.ts @@ -31,7 +31,7 @@ export function marketplaceUrl(): string { const explicit = process.env.MARKETPLACE_POSTGRES_URL; if (explicit) return explicit; - const base = process.env.POSTGRES_URL; + const base = process.env.OFFICERDEV_POSTGRES_URL ?? process.env.POSTGRES_URL; if (!base) throw new Error('MARKETPLACE_POSTGRES_URL or POSTGRES_URL is required'); // Swap only the path. Parsing rather than string-replacing, so a password containing the database diff --git a/src/databases/officer_db/drizzle.config.ts b/src/databases/officer_db/drizzle.config.ts index e9c8e431..0c9a7f11 100644 --- a/src/databases/officer_db/drizzle.config.ts +++ b/src/databases/officer_db/drizzle.config.ts @@ -20,6 +20,6 @@ export default defineConfig({ out: './migrations', dialect: 'postgresql', dbCredentials: { - url: process.env.POSTGRES_URL!, + url: (process.env.OFFICERDEV_POSTGRES_URL ?? process.env.POSTGRES_URL)!, }, }); diff --git a/src/databases/officer_db/src/db.ts b/src/databases/officer_db/src/db.ts index 1bbc2485..50e565f3 100644 --- a/src/databases/officer_db/src/db.ts +++ b/src/databases/officer_db/src/db.ts @@ -2,7 +2,11 @@ import { drizzle } from 'drizzle-orm/postgres-js'; import postgres from 'postgres'; import * as schema from './schema'; -const { POSTGRES_URL } = process.env; +// Prefixed first, bare as the fallback. Bun auto-loads `.env` into every process started in this +// directory, so an unprefixed POSTGRES_URL reaches sidecars and anything an agent spawns — none of +// which has business holding a database credential. `bun run fix:env` does the rename; the fallback +// is what lets an install that has not run it keep working. +const POSTGRES_URL = process.env.OFFICERDEV_POSTGRES_URL ?? process.env.POSTGRES_URL; if (!POSTGRES_URL) { throw new Error('POSTGRES_URL environment variable is required'); diff --git a/src/servers/api/dav/router.ts b/src/servers/api/dav/router.ts index 02b56924..42a2f77e 100644 --- a/src/servers/api/dav/router.ts +++ b/src/servers/api/dav/router.ts @@ -43,7 +43,7 @@ davRouter.post('/provision/ios', async (ctx) => { // The profile has to name a host, and PUBLIC_URL is the only place that knows the one a phone can // actually reach — the request's own Host header is whatever the reverse proxy forwarded. - const publicUrl = process.env.PUBLIC_URL; + const publicUrl = process.env.OFFICERDEV_PUBLIC_URL ?? process.env.PUBLIC_URL; if (!publicUrl) throw errors.INTERNAL_SERVER_ERROR('PUBLIC_URL is not set; cannot build a profile'); let url: URL; diff --git a/src/servers/api/server-settings/server-settings.ts b/src/servers/api/server-settings/server-settings.ts index ff926c5d..8179700f 100644 --- a/src/servers/api/server-settings/server-settings.ts +++ b/src/servers/api/server-settings/server-settings.ts @@ -1,6 +1,7 @@ import { createRouter } from '../../create-router'; import { rebuildFrontend } from '../../plugins/generate'; import { isKnownToPm2, restartProcess } from '../../app-store/pm2'; +import { fixEnvFile } from '../../env-prefix'; import { PLATFORM_DIR } from '../../data-path'; import { readdirSync, existsSync } from 'node:fs'; import { join } from 'node:path'; @@ -66,10 +67,27 @@ serverSettingsRouter.post('/restart', async (ctx) => { return ctx.json({ ok: false, error: 'not running under PM2 — restart it the way you started it' }, 400); } + // Convert `.env` to the `OFFICERDEV_` prefix on the way past. + // + // Here rather than at boot, because a restart is the one moment the file can be rewritten and the + // change take effect in the same breath — the process about to start is the one that reads it. Doing + // it at boot would rewrite a file the running process had already loaded. + // + // Idempotent and non-fatal: an already-converted file is not even written, and a failure must not stop + // a restart the owner asked for. Every consumer still reads the unprefixed name as a fallback, so the + // worst case is that nothing changed. + let envFixed: string[] = []; + try { + envFixed = fixEnvFile(join(PLATFORM_DIR, '.env')).changed; + if (envFixed.length) console.log(`[env] prefixed on restart: ${envFixed.join(', ')}`); + } catch (err) { + console.error('[env] could not prefix .env —', err instanceof Error ? err.message : err); + } + setTimeout(() => { void restartProcess('officer', PLATFORM_DIR); }, 250); - return ctx.json({ ok: true, restartingIn: 250 }); + return ctx.json({ ok: true, restartingIn: 250, envFixed }); }); serverSettingsRouter.get('/settings', async (ctx) => { diff --git a/src/servers/env-prefix.test.ts b/src/servers/env-prefix.test.ts new file mode 100644 index 00000000..30c48f61 --- /dev/null +++ b/src/servers/env-prefix.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, test } from 'bun:test'; +import { prefixEnvText } from './env-prefix'; + +// This rewrites the file the whole platform reads its database credential from, and it runs on every +// restart from the settings screen. The tests are about what it must NOT do. + +describe('prefixing', () => { + test('renames an ordinary variable', () => { + expect(prefixEnvText('PORT="9000"').text).toBe('OFFICERDEV_PORT="9000"'); + }); + + test('leaves the value completely alone, including an = inside it', () => { + const line = 'POSTGRES_URL="postgresql://u:p=a=ss@127.0.0.1:5432/officer"'; + expect(prefixEnvText(line).text).toBe(`OFFICERDEV_${line}`); + }); + + test('is idempotent — it runs on every restart', () => { + const once = prefixEnvText('PORT="9000"').text; + const twice = prefixEnvText(once); + expect(twice.text).toBe(once); + expect(twice.changed).toEqual([]); + expect(twice.alreadyPrefixed).toBe(1); + }); +}); + +describe('what it must not touch', () => { + test('comments and blank lines survive', () => { + const text = '# the port\n\nPORT="9000"\n# trailing note\n'; + const out = prefixEnvText(text).text; + expect(out).toContain('# the port'); + expect(out).toContain('# trailing note'); + expect(out.split('\n').length).toBe(text.split('\n').length); + }); + + test('a commented-out assignment stays commented out', () => { + expect(prefixEnvText('# PORT="9000"').text).toBe('# PORT="9000"'); + }); + + test('variables belonging to other software are left alone', () => { + // Prefixing one of these does not rename a variable, it deletes it: nothing reads OFFICERDEV_PATH. + for (const key of ['NODE_ENV', 'PATH', 'HOME', 'TZ']) { + expect(prefixEnvText(`${key}=x`).text).toBe(`${key}=x`); + } + }); + + test('an `export` prefix is preserved', () => { + expect(prefixEnvText('export PORT=9000').text).toBe('export OFFICERDEV_PORT=9000'); + }); + + test('a line that is not an assignment passes through', () => { + expect(prefixEnvText('this is not an env line').text).toBe('this is not an env line'); + }); +}); diff --git a/src/servers/env-prefix.ts b/src/servers/env-prefix.ts new file mode 100644 index 00000000..5e61985f --- /dev/null +++ b/src/servers/env-prefix.ts @@ -0,0 +1,79 @@ +import { existsSync, readFileSync, writeFileSync } from 'node:fs'; + +// Giving every variable in `.env` the `OFFICERDEV_` prefix. +// +// ── Why prefix at all ── +// +// `PORT`, `PUBLIC_URL` and `POSTGRES_URL` are names half the software in the world uses. Bun auto-loads +// `.env` into EVERY process started in the platform directory, so a bare `PORT` reaches the sidecars, +// the tools they spawn and anything an agent runs from here — and a bare `POSTGRES_URL` reaches things +// with no business holding a database credential. Prefixed, they cannot be picked up by accident. +// +// ── Why every consumer still reads both ── +// +// `process.env.OFFICERDEV_X ?? process.env.X`, everywhere. An install that has not been converted keeps +// working, which is what makes this safe to ship before every machine has run it. The unprefixed name is +// the fallback, never the source. +// +// ── Idempotent, because it runs unattended ── +// +// This runs on every restart from the server settings screen, not only when somebody types +// `bun run fix:env`. An already-prefixed line is left exactly as it is, and a file that needs no change +// is not written at all — writing an identical file still moves its mtime. + +const PREFIX = 'OFFICERDEV_'; + +/** + * Names that must NOT be prefixed. + * + * These are read by other software rather than by Officer: `NODE_ENV` is the runtime's, `PATH` and + * `HOME` are the operating system's. Prefixing one does not rename a variable, it deletes it. + */ +const LEAVE_ALONE = new Set(['NODE_ENV', 'PATH', 'HOME', 'SHELL', 'USER', 'LANG', 'TZ', 'TMPDIR', 'XDG_DATA_HOME']); + +export type FixEnvResult = { changed: string[]; alreadyPrefixed: number; path: string }; + +/** Rewrite one env file's text. Pure, so the rules are testable without a filesystem. */ +export function prefixEnvText(text: string): { text: string; changed: string[]; alreadyPrefixed: number } { + const changed: string[] = []; + let alreadyPrefixed = 0; + + const out = text.split('\n').map((line) => { + // Comments, blanks and anything that is not `KEY=…` pass through untouched. A `.env` is usually + // commented, and losing that would be a poor trade for a rename. + // + // `export` is captured rather than skipped so it can be put back — dropping it would change what the + // line does in a shell that sources this file. + const match = line.match(/^(\s*)(export\s+)?([A-Za-z_][A-Za-z0-9_]*)(\s*=)(.*)$/); + if (!match) return line; + + const [, indent, exported, key, eq, value] = match as unknown as [ + string, + string, + string | undefined, + string, + string, + string, + ]; + + if (key.startsWith(PREFIX)) { + alreadyPrefixed += 1; + return line; + } + if (LEAVE_ALONE.has(key)) return line; + + changed.push(key); + return `${indent}${exported ?? ''}${PREFIX}${key}${eq}${value}`; + }); + + return { text: out.join('\n'), changed, alreadyPrefixed }; +} + +/** Apply it to a file. Returns what changed; writes nothing when nothing did. */ +export function fixEnvFile(path: string): FixEnvResult { + if (!existsSync(path)) return { changed: [], alreadyPrefixed: 0, path }; + const before = readFileSync(path, 'utf8'); + const { text, changed, alreadyPrefixed } = prefixEnvText(before); + if (text !== before) writeFileSync(path, text); + return { changed, alreadyPrefixed, path }; +} diff --git a/src/servers/officer-url.mjs b/src/servers/officer-url.mjs index 0ff14fc1..23158689 100644 --- a/src/servers/officer-url.mjs +++ b/src/servers/officer-url.mjs @@ -20,7 +20,7 @@ // // So: no default anywhere, and this throws. Same posture as jwt.ts with JWT_SECRET. -const raw = process.env.PORT; +const raw = process.env.OFFICERDEV_PORT ?? process.env.PORT; if (!raw) { throw new Error( diff --git a/src/servers/os-user-postgres.ts b/src/servers/os-user-postgres.ts index e0c2d367..e1375b41 100644 --- a/src/servers/os-user-postgres.ts +++ b/src/servers/os-user-postgres.ts @@ -223,7 +223,7 @@ const TEMPLATE_ISOLATION_SQL = [ * their side. */ export async function ensureTemplateIsolation(): Promise<{ ok: true } | { ok: false; error: string }> { - const url = process.env.POSTGRES_URL; + const url = (process.env.OFFICERDEV_POSTGRES_URL ?? process.env.POSTGRES_URL); if (!url) return { ok: false, error: 'POSTGRES_URL is not set' }; let target: string; @@ -315,7 +315,7 @@ export async function provisionPostgresRole(params: { } if (passwordSet && password) { - const { host, port } = pgpassTarget(process.env.POSTGRES_URL); + const { host, port } = pgpassTarget((process.env.OFFICERDEV_POSTGRES_URL ?? process.env.POSTGRES_URL)); // Written twice, on purpose, and they are not redundant. // @@ -428,7 +428,7 @@ const DB_NAME_RE = /^[a-zA-Z0-9._-]{1,63}$/; export async function dropPostgresRole(osUser: string): Promise { if (!ROLE_NAME_RE.test(osUser)) return { ok: false, error: `'${osUser}' is not a usable Postgres role name` }; - const url = process.env.POSTGRES_URL; + const url = (process.env.OFFICERDEV_POSTGRES_URL ?? process.env.POSTGRES_URL); if (!url) return { ok: false, error: 'POSTGRES_URL is not set' }; try {