Repository navigation
fix(startup): honor documented env loading #369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
88cc53a
da39e14
e3a2bd7
b19e6a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,64 @@ let serverPort = 20128; | |
|
|
||
| const getServerUrl = () => `http://localhost:${serverPort}`; | ||
|
|
||
| function resolveDataDir(overridePath, env = process.env) { | ||
| if (overridePath && overridePath.trim()) return path.resolve(overridePath); | ||
|
|
||
| const configured = env.DATA_DIR?.trim(); | ||
| if (configured) return path.resolve(configured); | ||
|
|
||
| if (process.platform === "win32") { | ||
| const appData = env.APPDATA || path.join(require("os").homedir(), "AppData", "Roaming"); | ||
| return path.join(appData, "omniroute"); | ||
| } | ||
|
|
||
| const xdg = env.XDG_CONFIG_HOME?.trim(); | ||
| if (xdg) return path.join(path.resolve(xdg), "omniroute"); | ||
|
|
||
| return path.join(require("os").homedir(), ".omniroute"); | ||
| } | ||
|
|
||
| function getPreferredEnvFilePath(env = process.env) { | ||
| const candidates = []; | ||
|
|
||
| if (env.DATA_DIR?.trim()) { | ||
| candidates.push(path.join(path.resolve(env.DATA_DIR.trim()), ".env")); | ||
| } | ||
|
|
||
| candidates.push(path.join(resolveDataDir(), ".env")); | ||
| candidates.push(path.join(process.cwd(), ".env")); | ||
|
|
||
| return candidates.find((filePath) => fs.existsSync(filePath)) || null; | ||
| } | ||
|
|
||
| function hasEncryptedCredentials(dbPath) { | ||
| if (!fs.existsSync(dbPath)) return false; | ||
|
|
||
| try { | ||
| const Database = require("better-sqlite3"); | ||
| const db = new Database(dbPath, { readonly: true, fileMustExist: true }); | ||
| try { | ||
| const row = db | ||
| .prepare( | ||
| `SELECT 1 | ||
| FROM provider_connections | ||
| WHERE access_token LIKE 'enc:v1:%' | ||
| OR refresh_token LIKE 'enc:v1:%' | ||
| OR api_key LIKE 'enc:v1:%' | ||
| OR id_token LIKE 'enc:v1:%' | ||
| LIMIT 1` | ||
| ) | ||
| .get(); | ||
| return !!row; | ||
| } finally { | ||
| db.close(); | ||
| } | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| throw new Error(`Unable to inspect existing database at ${dbPath}: ${message}`); | ||
| } | ||
| } | ||
|
Comment on lines
+67
to
+123
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These helper functions ( To avoid this, consider extracting these shared functions into a separate CommonJS file (e.g.,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keeping this PR scoped to the startup/key-safety fix. I agree the duplication is a maintenance cost, but extracting a shared helper across the Electron CJS path and the bootstrap ESM path would broaden the runtime/module-loading surface for this PR. I would prefer to handle that as a separate follow-up refactor once the startup regression is merged. |
||
|
|
||
| // ── Auto-Updater Configuration ────────────────────────────── | ||
| autoUpdater.autoDownload = false; | ||
| autoUpdater.autoInstallOnAppQuit = true; | ||
|
|
@@ -386,12 +444,10 @@ function startNextServer() { | |
| // ── Zero-config bootstrap: auto-generate required secrets ───────────────── | ||
| // Electron uses CJS — cannot dynamically import ESM bootstrap-env.mjs. | ||
| // This mirrors bootstrap-env.mjs logic synchronously: | ||
| // 1. Read persisted secrets from userData/server.env | ||
| // 1. Read persisted secrets from the resolved DATA_DIR/server.env | ||
| // 2. Generate missing secrets with crypto.randomBytes() | ||
| // 3. Persist back to userData/server.env for future restarts | ||
| // 3. Persist back to DATA_DIR/server.env for future restarts | ||
| const crypto = require("crypto"); | ||
| const userDataDir = app.getPath("userData"); | ||
| const serverEnvPath = path.join(userDataDir, "server.env"); | ||
|
|
||
| // Parse a simple KEY=VALUE file | ||
| function parseEnvFile(filePath) { | ||
|
|
@@ -407,8 +463,12 @@ function startNextServer() { | |
| return env; | ||
| } | ||
|
|
||
| const preferredEnvPath = getPreferredEnvFilePath(process.env); | ||
| const preferredEnv = preferredEnvPath ? parseEnvFile(preferredEnvPath) : {}; | ||
| const dataDir = resolveDataDir(null, { ...preferredEnv, ...process.env }); | ||
|
kfiramar marked this conversation as resolved.
Resolved
|
||
| const serverEnvPath = path.join(dataDir, "server.env"); | ||
| const persisted = parseEnvFile(serverEnvPath); | ||
| const serverEnv = { ...process.env, ...persisted }; | ||
| const serverEnv = { ...persisted, ...preferredEnv, ...process.env }; | ||
| let changed = false; | ||
|
|
||
| if (!serverEnv.JWT_SECRET) { | ||
|
|
@@ -417,6 +477,16 @@ function startNextServer() { | |
| console.log("[Electron] ✨ JWT_SECRET auto-generated"); | ||
| } | ||
| if (!serverEnv.STORAGE_ENCRYPTION_KEY) { | ||
| if (hasEncryptedCredentials(path.join(dataDir, "storage.sqlite"))) { | ||
| console.error( | ||
| `[Electron] Refusing to auto-generate STORAGE_ENCRYPTION_KEY: encrypted credentials already exist in ${path.join( | ||
| dataDir, | ||
| "storage.sqlite" | ||
| )}. Restore the key via ${preferredEnvPath || "an appropriate .env file"}, ${serverEnvPath}, or process.env.` | ||
| ); | ||
| sendToRenderer("server-status", { status: "error", port: serverPort }); | ||
| return; | ||
| } | ||
| serverEnv.STORAGE_ENCRYPTION_KEY = persisted.STORAGE_ENCRYPTION_KEY = crypto | ||
| .randomBytes(32) | ||
| .toString("hex"); | ||
|
|
@@ -432,7 +502,7 @@ function startNextServer() { | |
| if (changed) { | ||
| serverEnv.OMNIROUTE_BOOTSTRAPPED = "true"; | ||
| try { | ||
| fs.mkdirSync(userDataDir, { recursive: true }); | ||
| fs.mkdirSync(dataDir, { recursive: true }); | ||
| const lines = [ | ||
| "# Auto-generated by OmniRoute bootstrap", | ||
| "", | ||
|
|
@@ -454,6 +524,7 @@ function startNextServer() { | |
| cwd: NEXT_SERVER_PATH, | ||
| env: { | ||
| ...serverEnv, | ||
| DATA_DIR: dataDir, | ||
| PORT: String(serverPort), | ||
| NODE_ENV: "production", | ||
| }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| import test from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
| import fs from "node:fs"; | ||
| import os from "node:os"; | ||
| import path from "node:path"; | ||
| import Database from "better-sqlite3"; | ||
|
|
||
| import { bootstrapEnv } from "../../scripts/bootstrap-env.mjs"; | ||
|
|
||
| function withTempEnv(fn) { | ||
| const originalCwd = process.cwd(); | ||
| const originalEnv = { ...process.env }; | ||
| const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-bootstrap-test-")); | ||
| const tempCwd = path.join(tempRoot, "cwd"); | ||
| const tempHome = path.join(tempRoot, "home"); | ||
|
|
||
| fs.mkdirSync(tempCwd, { recursive: true }); | ||
| fs.mkdirSync(tempHome, { recursive: true }); | ||
|
|
||
| delete process.env.DATA_DIR; | ||
| delete process.env.XDG_CONFIG_HOME; | ||
| delete process.env.APPDATA; | ||
| delete process.env.JWT_SECRET; | ||
| delete process.env.STORAGE_ENCRYPTION_KEY; | ||
| delete process.env.STORAGE_ENCRYPTION_KEY_VERSION; | ||
| delete process.env.API_KEY_SECRET; | ||
| delete process.env.INITIAL_PASSWORD; | ||
| process.env.HOME = tempHome; | ||
| process.chdir(tempCwd); | ||
|
|
||
| try { | ||
| fn({ tempRoot, tempCwd, tempHome, dataDir: path.join(tempHome, ".omniroute") }); | ||
| } finally { | ||
| process.chdir(originalCwd); | ||
| for (const key of Object.keys(process.env)) { | ||
| if (!(key in originalEnv)) delete process.env[key]; | ||
| } | ||
| for (const [key, value] of Object.entries(originalEnv)) { | ||
| process.env[key] = value; | ||
| } | ||
| fs.rmSync(tempRoot, { recursive: true, force: true }); | ||
| } | ||
| } | ||
|
|
||
| test("bootstrapEnv prefers ~/.omniroute/.env over server.env", () => { | ||
| withTempEnv(({ dataDir }) => { | ||
| fs.mkdirSync(dataDir, { recursive: true }); | ||
| fs.writeFileSync( | ||
| path.join(dataDir, ".env"), | ||
| "STORAGE_ENCRYPTION_KEY=from-dot-env\nJWT_SECRET=jwt-from-dot-env\n", | ||
| "utf8" | ||
| ); | ||
| fs.writeFileSync( | ||
| path.join(dataDir, "server.env"), | ||
| "STORAGE_ENCRYPTION_KEY=from-server-env\nJWT_SECRET=jwt-from-server-env\n", | ||
| "utf8" | ||
| ); | ||
|
|
||
| const env = bootstrapEnv({ quiet: true }); | ||
|
|
||
| assert.equal(env.STORAGE_ENCRYPTION_KEY, "from-dot-env"); | ||
| assert.equal(env.JWT_SECRET, "jwt-from-dot-env"); | ||
| }); | ||
| }); | ||
|
|
||
| test("bootstrapEnv refuses to generate a new key over encrypted data", () => { | ||
| withTempEnv(({ dataDir }) => { | ||
| fs.mkdirSync(dataDir, { recursive: true }); | ||
| const db = new Database(path.join(dataDir, "storage.sqlite")); | ||
| try { | ||
| db.exec(` | ||
| CREATE TABLE provider_connections ( | ||
| id TEXT PRIMARY KEY, | ||
| access_token TEXT, | ||
| refresh_token TEXT, | ||
| api_key TEXT, | ||
| id_token TEXT | ||
| ); | ||
| `); | ||
| db.prepare("INSERT INTO provider_connections (id, access_token) VALUES (?, ?)") | ||
| .run("conn-1", "enc:v1:deadbeef:feedface:cafebabe"); | ||
| } finally { | ||
| db.close(); | ||
| } | ||
|
|
||
| assert.throws( | ||
| () => bootstrapEnv({ quiet: true }), | ||
| /Refusing to auto-generate STORAGE_ENCRYPTION_KEY/ | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| test("bootstrapEnv fails closed when existing database cannot be inspected", () => { | ||
| withTempEnv(({ dataDir }) => { | ||
| fs.mkdirSync(path.join(dataDir, "storage.sqlite"), { recursive: true }); | ||
|
|
||
| assert.throws( | ||
| () => bootstrapEnv({ quiet: true }), | ||
| /Unable to inspect existing database/ | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| test("bootstrapEnv ignores blank dataDirOverride values", () => { | ||
| withTempEnv(({ dataDir }) => { | ||
| fs.mkdirSync(dataDir, { recursive: true }); | ||
| fs.writeFileSync(path.join(dataDir, ".env"), "JWT_SECRET=jwt-from-dot-env\n", "utf8"); | ||
|
|
||
| const env = bootstrapEnv({ dataDirOverride: " ", quiet: true }); | ||
|
|
||
| assert.equal(env.JWT_SECRET, "jwt-from-dot-env"); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.