mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-23 23:46:35 +00:00
fix(security): refuse an ENCRYPTION_KEY that disagrees with the key file
An env value different from data/.encryption_key used to overwrite the file on boot, destroying the only copy of the key the stored secrets were encrypted with. The server now refuses to start and names the rotation script, which re-encrypts the AI Parsing addon key as well and moves the key file to the new key. Under test the suite's fixed key no longer overwrites the developer's key file.
This commit is contained in:
@@ -530,6 +530,34 @@ async function main() {
|
||||
}
|
||||
}
|
||||
|
||||
// --- addons: the AI Parsing addon's instance-wide API key ---
|
||||
// Stored encrypted as `apiKey` inside the addon's JSON config
|
||||
// (llm-parse/llm-config.ts). Left out, LLM parsing quietly runs without a
|
||||
// key after the rotation, since a failed decrypt reads as "no key".
|
||||
if (tableExists('addons')) {
|
||||
const addonRows = db
|
||||
.prepare('SELECT id, config FROM addons WHERE config IS NOT NULL')
|
||||
.all() as { id: string; config: string }[];
|
||||
for (const row of addonRows) {
|
||||
let config: unknown;
|
||||
try {
|
||||
config = JSON.parse(row.config);
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
if (!config || typeof config !== 'object' || Array.isArray(config)) continue;
|
||||
const apiKey = (config as Record<string, unknown>).apiKey;
|
||||
if (typeof apiKey !== 'string' || !apiKey) continue;
|
||||
const newVal = migrateApiKeyValue(apiKey, `addons[${row.id}].config.apiKey`);
|
||||
if (newVal !== null) {
|
||||
db.prepare('UPDATE addons SET config = ? WHERE id = ?').run(
|
||||
JSON.stringify({ ...(config as Record<string, unknown>), apiKey: newVal }),
|
||||
row.id,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// --- trip_album_links: passphrase ---
|
||||
const albumLinks = db.prepare('SELECT id, passphrase FROM trip_album_links WHERE passphrase IS NOT NULL').all() as { id: number; passphrase: string }[];
|
||||
for (const row of albumLinks) {
|
||||
@@ -618,6 +646,16 @@ async function main() {
|
||||
process.exit(1);
|
||||
} else {
|
||||
console.log('\nAll secrets successfully re-encrypted.');
|
||||
// The server refuses to start when ENCRYPTION_KEY and the key file disagree
|
||||
// (src/config.ts), so the file follows the rotation. Only an existing file
|
||||
// next to the database is touched; anything else is left to the operator.
|
||||
const keyFile = path.join(path.dirname(dbPath), '.encryption_key');
|
||||
if (fs.existsSync(keyFile)) {
|
||||
fs.writeFileSync(keyFile, newKey, { mode: 0o600 });
|
||||
console.log(`Key file updated: ${keyFile}`);
|
||||
} else {
|
||||
console.log('No key file next to the database. Store the new key wherever TREK reads it from (ENCRYPTION_KEY or data/.encryption_key).');
|
||||
}
|
||||
console.log(`Backup retained at: ${backupPath}`);
|
||||
}
|
||||
}
|
||||
|
||||
+34
-4
@@ -23,15 +23,45 @@ const jwtSecretFile = path.join(dataDir, '.jwt_secret');
|
||||
const encKeyFile = path.join(dataDir, '.encryption_key');
|
||||
let _encryptionKey: string = process.env.ENCRYPTION_KEY || '';
|
||||
|
||||
if (_encryptionKey) {
|
||||
// Env var is set explicitly — persist it to file so the value survives
|
||||
// container restarts even if the env var is later removed.
|
||||
// Under test the suite sets its own fixed ENCRYPTION_KEY. Comparing it with, or
|
||||
// writing it over, the developer's own data/.encryption_key would either stop
|
||||
// every test run or quietly replace the key a local dev server encrypts with.
|
||||
const keyFileFollowsEnv = process.env.NODE_ENV !== 'test';
|
||||
|
||||
/**
|
||||
* Persist an explicit ENCRYPTION_KEY to the key file, so the value survives
|
||||
* container restarts even if the env var is later removed.
|
||||
*
|
||||
* A key file that disagrees with the env var means every stored secret was
|
||||
* encrypted with the file's key. Booting would make them unreadable and the
|
||||
* write would destroy the only copy of that key, which is exactly what a
|
||||
* regenerated Helm secret or a typo in the variable used to do in silence. The
|
||||
* rotation script updates the file, so a real rotation never lands here.
|
||||
*/
|
||||
function persistEnvKey(envKey: string): void {
|
||||
let storedKey = '';
|
||||
try {
|
||||
storedKey = fs.readFileSync(encKeyFile, 'utf8').trim();
|
||||
} catch {
|
||||
// No readable file yet: nothing to protect, the env value becomes the file.
|
||||
}
|
||||
if (storedKey && storedKey !== envKey.trim()) {
|
||||
console.error(`FATAL: ENCRYPTION_KEY does not match ${encKeyFile}.`);
|
||||
console.error('The stored secrets are encrypted with the key in that file, so TREK will not start with a different one.');
|
||||
console.error('To change the key, run scripts/migrate-encryption.ts first; it re-encrypts the secrets and updates the file.');
|
||||
console.error('Otherwise set ENCRYPTION_KEY back to the value in the file, or remove the variable.');
|
||||
process.exit(1);
|
||||
}
|
||||
try {
|
||||
if (!fs.existsSync(dataDir)) fs.mkdirSync(dataDir, { recursive: true });
|
||||
fs.writeFileSync(encKeyFile, _encryptionKey, { mode: 0o600 });
|
||||
fs.writeFileSync(encKeyFile, envKey, { mode: 0o600 });
|
||||
} catch {
|
||||
// Non-fatal: env var is the source of truth when set.
|
||||
}
|
||||
}
|
||||
|
||||
if (_encryptionKey) {
|
||||
if (keyFileFollowsEnv) persistEnvKey(_encryptionKey);
|
||||
} else {
|
||||
// Try the dedicated key file first (covers all installs after first start).
|
||||
try {
|
||||
|
||||
@@ -106,4 +106,64 @@ describe('config — encryption key resolution', () => {
|
||||
expect(exit).not.toHaveBeenCalled();
|
||||
expect(config.ENCRYPTION_KEY).toMatch(/^[0-9a-f]{64}$/);
|
||||
});
|
||||
|
||||
describe('an explicit ENCRYPTION_KEY outside the test suite', () => {
|
||||
const realNodeEnv = process.env.NODE_ENV;
|
||||
beforeEach(() => {
|
||||
process.env.NODE_ENV = 'production';
|
||||
});
|
||||
afterEach(() => {
|
||||
process.env.NODE_ENV = realNodeEnv;
|
||||
});
|
||||
|
||||
it('CFGKEY-004: refuses to boot when it disagrees with the key file, and leaves the file alone', async () => {
|
||||
process.env.ENCRYPTION_KEY = 'a-new-key';
|
||||
const exit = vi.spyOn(process, 'exit').mockImplementation((() => {
|
||||
throw new Error('process.exit called');
|
||||
}) as never);
|
||||
const write = vi.spyOn(fs, 'writeFileSync').mockImplementation(() => undefined);
|
||||
const error = vi.spyOn(console, 'error').mockImplementation(() => undefined);
|
||||
stubKeyFileRead(() => 'the-original-key\n');
|
||||
|
||||
await expect(import('../../src/config')).rejects.toThrow('process.exit called');
|
||||
expect(exit).toHaveBeenCalledWith(1);
|
||||
expect(write).not.toHaveBeenCalledWith(ENC_KEY_FILE, expect.anything(), expect.anything());
|
||||
expect(error.mock.calls.flat().join(' ')).toContain('migrate-encryption.ts');
|
||||
});
|
||||
|
||||
it('CFGKEY-005: boots and keeps the file in step when both agree', async () => {
|
||||
process.env.ENCRYPTION_KEY = 'the-original-key';
|
||||
const exit = vi.spyOn(process, 'exit').mockImplementation((() => {
|
||||
throw new Error('process.exit called');
|
||||
}) as never);
|
||||
const write = vi.spyOn(fs, 'writeFileSync').mockImplementation(() => undefined);
|
||||
stubKeyFileRead(() => 'the-original-key\n');
|
||||
|
||||
const config = await import('../../src/config');
|
||||
expect(exit).not.toHaveBeenCalled();
|
||||
expect(config.ENCRYPTION_KEY).toBe('the-original-key');
|
||||
expect(write).toHaveBeenCalledWith(ENC_KEY_FILE, 'the-original-key', { mode: 0o600 });
|
||||
});
|
||||
|
||||
it('CFGKEY-006: writes the key file on the first start that has one in the environment', async () => {
|
||||
process.env.ENCRYPTION_KEY = 'first-key';
|
||||
const write = vi.spyOn(fs, 'writeFileSync').mockImplementation(() => undefined);
|
||||
stubKeyFileRead(() => {
|
||||
throw enoent();
|
||||
});
|
||||
|
||||
await import('../../src/config');
|
||||
expect(write).toHaveBeenCalledWith(ENC_KEY_FILE, 'first-key', { mode: 0o600 });
|
||||
});
|
||||
});
|
||||
|
||||
it('CFGKEY-007: under test, an explicit key neither checks nor overwrites the developer key file', async () => {
|
||||
process.env.ENCRYPTION_KEY = 'suite-key';
|
||||
const write = vi.spyOn(fs, 'writeFileSync').mockImplementation(() => undefined);
|
||||
stubKeyFileRead(() => 'a-developer-key');
|
||||
|
||||
const config = await import('../../src/config');
|
||||
expect(config.ENCRYPTION_KEY).toBe('suite-key');
|
||||
expect(write).not.toHaveBeenCalledWith(ENC_KEY_FILE, expect.anything(), expect.anything());
|
||||
});
|
||||
});
|
||||
|
||||
@@ -391,4 +391,36 @@ describe('migrate-encryption.ts rotates the stores the app writes', () => {
|
||||
expect(output).toMatch(/Already on new key:\s+1\b/);
|
||||
expect(output).toContain('All secrets successfully re-encrypted.');
|
||||
}, 30000);
|
||||
|
||||
it("ROTPAR-013: re-encrypts the AI Parsing addon key and moves the key file to the new key", () => {
|
||||
const seed = new Database(dbPath);
|
||||
seed.exec(BASE_SCHEMA);
|
||||
seed.exec('CREATE TABLE addons (id TEXT PRIMARY KEY, config TEXT)');
|
||||
seed.prepare('INSERT INTO addons (id, config) VALUES (?, ?)').run(
|
||||
'llm_parse',
|
||||
JSON.stringify({ provider: 'openai', apiKey: encryptWith('old-key', 'sk-addon') }),
|
||||
);
|
||||
seed.prepare('INSERT INTO addons (id, config) VALUES (?, ?)').run('packing', '{"x":1}');
|
||||
seed.close();
|
||||
const keyFile = path.join(path.dirname(dbPath), '.encryption_key');
|
||||
fs.writeFileSync(keyFile, 'old-key');
|
||||
|
||||
execFileSync(process.execPath, ['--import', 'tsx', 'scripts/migrate-encryption.ts'], {
|
||||
cwd: SERVER_ROOT,
|
||||
env: { ...process.env, DB_PATH: dbPath },
|
||||
input: ['old-key', 'new-key', 'yes', ''].join(String.fromCharCode(10)),
|
||||
stdio: 'pipe',
|
||||
encoding: 'utf8',
|
||||
});
|
||||
|
||||
const after = new Database(dbPath, { readonly: true });
|
||||
const llm = JSON.parse((after.prepare("SELECT config AS v FROM addons WHERE id = 'llm_parse'").get() as { v: string }).v);
|
||||
expect(decryptWith('new-key', llm.apiKey)).toBe('sk-addon');
|
||||
expect(llm.provider).toBe('openai');
|
||||
const other = (after.prepare("SELECT config AS v FROM addons WHERE id = 'packing'").get() as { v: string }).v;
|
||||
expect(other).toBe('{"x":1}');
|
||||
after.close();
|
||||
// The server refuses an ENCRYPTION_KEY that disagrees with this file.
|
||||
expect(fs.readFileSync(keyFile, 'utf8')).toBe('new-key');
|
||||
}, 30000);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user