fix(security): purge credentials from settings backups

Write the newly protected settings payload to both sync and async config backups so disabling credential persistence or losing encryption availability cannot leave previously stored provider secrets behind.

Replace content-based prefix detection with a typed, versioned safeStorage envelope, preserve arbitrary legacy plaintext for migration, and emit field-only diagnostics for encryption failures without exposing credentials or ciphertext. Add regression coverage for both save paths, both persistence-off conditions, and prefix-shaped plaintext values.
This commit is contained in:
Sucukdeluxe
2026-08-11 21:50:02 +02:00
parent 97ad90ad4f
commit 5af7b5c32f
4 changed files with 192 additions and 46 deletions
+47 -16
View File
@@ -1,4 +1,5 @@
import { AppSettings } from "../shared/types"; import { AppSettings } from "../shared/types";
import { logger } from "./logger";
export interface CredentialProtector { export interface CredentialProtector {
isEncryptionAvailable(): boolean; isEncryptionAvailable(): boolean;
@@ -6,7 +7,6 @@ export interface CredentialProtector {
decryptString(value: Buffer): string; decryptString(value: Buffer): string;
} }
const PROTECTED_VALUE_PREFIX = "mdd-safe-storage:v1:";
const MASKED_CREDENTIAL = "••••••••"; const MASKED_CREDENTIAL = "••••••••";
const CREDENTIAL_KEYS = [ const CREDENTIAL_KEYS = [
"token", "token",
@@ -24,6 +24,17 @@ const CREDENTIAL_KEYS = [
"linkSnappyLogin", "linkSnappyLogin",
"linkSnappyPassword" "linkSnappyPassword"
] as const satisfies readonly (keyof AppSettings)[]; ] as const satisfies readonly (keyof AppSettings)[];
type CredentialKey = typeof CREDENTIAL_KEYS[number];
interface PersistedCredentialEnvelope {
type: "safe-storage";
version: 1;
payload: string;
}
export type PersistedAppSettings = Omit<AppSettings, CredentialKey> & {
[K in CredentialKey]: string | PersistedCredentialEnvelope;
};
let credentialProtector: CredentialProtector = { let credentialProtector: CredentialProtector = {
isEncryptionAvailable: () => false, isEncryptionAvailable: () => false,
@@ -39,12 +50,20 @@ function isEncryptionAvailable(): boolean {
try { try {
return credentialProtector.isEncryptionAvailable(); return credentialProtector.isEncryptionAvailable();
} catch { } catch {
logger.warn("Credential-Verschlüsselungsverfügbarkeit konnte nicht ermittelt werden");
return false; return false;
} }
} }
function isProtectedValue(value: string): boolean { function isPersistedCredentialEnvelope(value: unknown): value is PersistedCredentialEnvelope {
return value.startsWith(PROTECTED_VALUE_PREFIX); if (!value || typeof value !== "object" || Array.isArray(value)) {
return false;
}
const envelope = value as Partial<PersistedCredentialEnvelope>;
return envelope.type === "safe-storage"
&& envelope.version === 1
&& typeof envelope.payload === "string"
&& envelope.payload.length > 0;
} }
function clearCredentials(settings: AppSettings): AppSettings { function clearCredentials(settings: AppSettings): AppSettings {
@@ -55,61 +74,73 @@ function clearCredentials(settings: AppSettings): AppSettings {
return cleared; return cleared;
} }
export function protectPersistedSettings(settings: AppSettings): AppSettings { export function protectPersistedSettings(settings: AppSettings): PersistedAppSettings {
if (settings.rememberToken === false || !isEncryptionAvailable()) { if (settings.rememberToken === false || !isEncryptionAvailable()) {
return clearCredentials(settings); return clearCredentials(settings);
} }
const protectedSettings = { ...settings }; const protectedSettings = { ...settings } as PersistedAppSettings;
for (const key of CREDENTIAL_KEYS) { for (const key of CREDENTIAL_KEYS) {
const value = typeof settings[key] === "string" ? settings[key] : ""; const value = typeof settings[key] === "string" ? settings[key] : "";
if (!value || isProtectedValue(value)) { if (!value) {
protectedSettings[key] = value; protectedSettings[key] = value;
continue; continue;
} }
try { try {
protectedSettings[key] = `${PROTECTED_VALUE_PREFIX}${credentialProtector.encryptString(value).toString("base64")}`; protectedSettings[key] = {
type: "safe-storage",
version: 1,
payload: credentialProtector.encryptString(value).toString("base64")
};
} catch { } catch {
protectedSettings[key] = ""; protectedSettings[key] = "";
logger.warn(`Credential-Verschlüsselung fehlgeschlagen: ${key}`);
} }
} }
return protectedSettings; return protectedSettings;
} }
export function restorePersistedSettings(settings: AppSettings): AppSettings { export function restorePersistedSettings(settings: AppSettings | PersistedAppSettings): AppSettings {
if (settings.rememberToken === false) { if (settings.rememberToken === false) {
return clearCredentials(settings); return clearCredentials(settings as AppSettings);
} }
const restored = { ...settings }; const restored = { ...settings } as AppSettings;
for (const key of CREDENTIAL_KEYS) { for (const key of CREDENTIAL_KEYS) {
const value = typeof settings[key] === "string" ? settings[key] : ""; const value = settings[key];
if (!isProtectedValue(value)) { if (typeof value === "string") {
restored[key] = value; restored[key] = value;
continue; continue;
} }
if (!isPersistedCredentialEnvelope(value)) {
restored[key] = "";
continue;
}
if (!isEncryptionAvailable()) { if (!isEncryptionAvailable()) {
restored[key] = ""; restored[key] = "";
continue; continue;
} }
try { try {
restored[key] = credentialProtector.decryptString(Buffer.from(value.slice(PROTECTED_VALUE_PREFIX.length), "base64")); restored[key] = credentialProtector.decryptString(Buffer.from(value.payload, "base64"));
} catch { } catch {
restored[key] = ""; restored[key] = "";
logger.warn(`Credential-Entschlüsselung fehlgeschlagen: ${key}`);
} }
} }
return restored; return restored;
} }
export function needsPersistedSettingsRewrite(settings: AppSettings): boolean { export function needsPersistedSettingsRewrite(settings: AppSettings | PersistedAppSettings): boolean {
const values = CREDENTIAL_KEYS.map((key) => typeof settings[key] === "string" ? settings[key] : "").filter(Boolean); const values = CREDENTIAL_KEYS.map((key) => settings[key]).filter((value) => {
return typeof value === "string" ? value.length > 0 : value !== null && value !== undefined;
});
if (values.length === 0) { if (values.length === 0) {
return false; return false;
} }
if (settings.rememberToken === false || !isEncryptionAvailable()) { if (settings.rememberToken === false || !isEncryptionAvailable()) {
return true; return true;
} }
return values.some((value) => !isProtectedValue(value)); return values.some((value) => !isPersistedCredentialEnvelope(value));
} }
export function projectSettingsForRenderer(settings: AppSettings): AppSettings { export function projectSettingsForRenderer(settings: AppSettings): AppSettings {
+39 -28
View File
@@ -1050,40 +1050,51 @@ function readSessionFile(filePath: string): SessionState | null {
} }
export function saveSettings(paths: StoragePaths, settings: AppSettings): void { export function saveSettings(paths: StoragePaths, settings: AppSettings): void {
syncSettingsSaveGeneration += 1; syncSettingsSaveGeneration += 1;
ensureBaseDir(paths.baseDir); ensureBaseDir(paths.baseDir);
if (fs.existsSync(paths.configFile)) {
try {
fs.copyFileSync(paths.configFile, `${paths.configFile}.bak`);
} catch {
}
}
const payload = settingsPayload(settings); const payload = settingsPayload(settings);
const tempPath = `${paths.configFile}.tmp`; if (fs.existsSync(paths.configFile)) {
try { writeSettingsFileAtomically(`${paths.configFile}.bak`, payload);
fs.writeFileSync(tempPath, payload, "utf8"); }
syncRenameWithExdevFallback(tempPath, paths.configFile); writeSettingsFileAtomically(paths.configFile, payload);
} catch (error) { }
try { fs.rmSync(tempPath, { force: true }); } catch { }
throw error;
}
}
let asyncSettingsSaveRunning = false; let asyncSettingsSaveRunning = false;
let asyncSettingsSaveQueued: { paths: StoragePaths; payload: string; generation: number } | null = null; let asyncSettingsSaveQueued: { paths: StoragePaths; payload: string; generation: number } | null = null;
let syncSettingsSaveGeneration = 0; let syncSettingsSaveGeneration = 0;
async function writeSettingsPayload(paths: StoragePaths, payload: string, generation: number): Promise<void> { async function writeSettingsPayload(paths: StoragePaths, payload: string, generation: number): Promise<void> {
await fs.promises.mkdir(paths.baseDir, { recursive: true }); await fs.promises.mkdir(paths.baseDir, { recursive: true });
await fsp.copyFile(paths.configFile, `${paths.configFile}.bak`).catch(() => {}); const tempPath = `${paths.configFile}.settings.tmp`;
const tempPath = `${paths.configFile}.settings.tmp`; await fsp.writeFile(tempPath, payload, "utf8");
await fsp.writeFile(tempPath, payload, "utf8"); if (generation < syncSettingsSaveGeneration) {
if (generation < syncSettingsSaveGeneration) { await fsp.rm(tempPath, { force: true }).catch(() => {});
await fsp.rm(tempPath, { force: true }).catch(() => {}); return;
return; }
} if (fs.existsSync(paths.configFile)) {
try { const backupTempPath = `${paths.configFile}.bak.settings.tmp`;
await fsp.rename(tempPath, paths.configFile); await fsp.writeFile(backupTempPath, payload, "utf8");
if (generation < syncSettingsSaveGeneration) {
await Promise.all([
fsp.rm(tempPath, { force: true }).catch(() => {}),
fsp.rm(backupTempPath, { force: true }).catch(() => {})
]);
return;
}
try {
await fsp.rename(backupTempPath, `${paths.configFile}.bak`);
} catch (renameError: unknown) {
if (renameError && typeof renameError === "object" && "code" in renameError && (renameError as NodeJS.ErrnoException).code === "EXDEV") {
await fsp.copyFile(backupTempPath, `${paths.configFile}.bak`);
await fsp.rm(backupTempPath, { force: true }).catch(() => {});
} else {
await fsp.rm(backupTempPath, { force: true }).catch(() => {});
throw renameError;
}
}
}
try {
await fsp.rename(tempPath, paths.configFile);
} catch (renameError: unknown) { } catch (renameError: unknown) {
if (renameError && typeof renameError === "object" && "code" in renameError && (renameError as NodeJS.ErrnoException).code === "EXDEV") { if (renameError && typeof renameError === "object" && "code" in renameError && (renameError as NodeJS.ErrnoException).code === "EXDEV") {
if (generation < syncSettingsSaveGeneration) { if (generation < syncSettingsSaveGeneration) {
+53 -1
View File
@@ -1,5 +1,6 @@
import { beforeEach, describe, expect, it } from "vitest"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { defaultSettings } from "../src/main/constants"; import { defaultSettings } from "../src/main/constants";
import { logger } from "../src/main/logger";
import { import {
configureCredentialProtector, configureCredentialProtector,
CredentialProtector, CredentialProtector,
@@ -21,6 +22,10 @@ describe("credential protection", () => {
configureCredentialProtector(createProtector()); configureCredentialProtector(createProtector());
}); });
afterEach(() => {
vi.restoreAllMocks();
});
it("protects remembered provider values and restores them for the main process", () => { it("protects remembered provider values and restores them for the main process", () => {
const input = { const input = {
...defaultSettings(), ...defaultSettings(),
@@ -53,6 +58,53 @@ describe("credential protection", () => {
expect(protectPersistedSettings(restorePersistedSettings(input)).token).not.toBe(input.token); expect(protectPersistedSettings(restorePersistedSettings(input)).token).not.toBe(input.token);
}); });
it("protects plaintext credentials that begin with the legacy marker text", () => {
const input = {
...defaultSettings(),
rememberToken: true,
token: "mdd-safe-storage:v1:literal-credential"
};
const persisted = protectPersistedSettings(input);
expect(persisted.token).not.toBe(input.token);
expect(JSON.stringify(persisted)).not.toContain(input.token);
expect(restorePersistedSettings(persisted).token).toBe(input.token);
});
it("reports encryption failures without logging credential data", () => {
const value = "credential-value-not-for-logs";
const warn = vi.spyOn(logger, "warn").mockImplementation(() => {});
configureCredentialProtector({
isEncryptionAvailable: () => true,
encryptString: () => { throw new Error("encryption failed"); },
decryptString: () => ""
});
const persisted = protectPersistedSettings({ ...defaultSettings(), token: value });
expect(persisted.token).toBe("");
expect(warn).toHaveBeenCalledWith("Credential-Verschlüsselung fehlgeschlagen: token");
expect(JSON.stringify(warn.mock.calls)).not.toContain(value);
});
it("reports decryption failures without logging persisted data", () => {
const persisted = protectPersistedSettings({ ...defaultSettings(), token: "value-to-encrypt" });
const serialized = JSON.stringify(persisted.token);
const warn = vi.spyOn(logger, "warn").mockImplementation(() => {});
configureCredentialProtector({
isEncryptionAvailable: () => true,
encryptString: () => Buffer.alloc(0),
decryptString: () => { throw new Error("decryption failed"); }
});
const restored = restorePersistedSettings(persisted);
expect(restored.token).toBe("");
expect(warn).toHaveBeenCalledWith("Credential-Entschlüsselung fehlgeschlagen: token");
expect(JSON.stringify(warn.mock.calls)).not.toContain(serialized);
});
it("does not persist provider values when encryption is unavailable", () => { it("does not persist provider values when encryption is unavailable", () => {
configureCredentialProtector(createProtector(false)); configureCredentialProtector(createProtector(false));
const persisted = protectPersistedSettings({ const persisted = protectPersistedSettings({
+53 -1
View File
@@ -8,7 +8,7 @@ import { getProviderUsageDayKey } from "../src/shared/provider-daily-limits";
import { AppSettings } from "../src/shared/types"; import { AppSettings } from "../src/shared/types";
import { defaultSettings } from "../src/main/constants"; import { defaultSettings } from "../src/main/constants";
import { configureCredentialProtector } from "../src/main/credential-protection"; import { configureCredentialProtector } from "../src/main/credential-protection";
import { addHistoryEntryForRetention, createStoragePaths, emptySession, loadHistory, loadHistoryForRetention, loadSession, loadSettings, normalizeLoadedSession, normalizeSettings, resetHistoryForRetention, saveHistory, saveSession, saveSessionAsync, saveSettings } from "../src/main/storage"; import { addHistoryEntryForRetention, createStoragePaths, emptySession, loadHistory, loadHistoryForRetention, loadSession, loadSettings, normalizeLoadedSession, normalizeSettings, resetHistoryForRetention, saveHistory, saveSession, saveSessionAsync, saveSettings, saveSettingsAsync } from "../src/main/storage";
const tempDirs: string[] = []; const tempDirs: string[] = [];
@@ -195,6 +195,58 @@ describe("settings storage", () => {
expect(loaded.allDebridToken).toBe("all-token"); expect(loaded.allDebridToken).toBe("all-token");
}); });
it.each(["sync", "async"] as const)("clears previously stored credentials from the backup when remembering is disabled during a %s save", async (mode) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-store-"));
tempDirs.push(dir);
const paths = createStoragePaths(dir);
const remembered = {
...defaultSettings(),
rememberToken: true,
token: "stored-value-before-clear"
};
saveSettings(paths, remembered);
const cleared = { ...remembered, rememberToken: false };
if (mode === "sync") {
saveSettings(paths, cleared);
} else {
await saveSettingsAsync(paths, cleared);
}
const primary = JSON.parse(fs.readFileSync(paths.configFile, "utf8")) as Record<string, unknown>;
const backup = JSON.parse(fs.readFileSync(`${paths.configFile}.bak`, "utf8")) as Record<string, unknown>;
expect(primary.token).toBe("");
expect(backup.token).toBe("");
});
it.each(["sync", "async"] as const)("clears previously stored credentials from the backup when encryption is unavailable during a %s save", async (mode) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-store-"));
tempDirs.push(dir);
const paths = createStoragePaths(dir);
const remembered = {
...defaultSettings(),
rememberToken: true,
token: "stored-value-before-unavailable"
};
saveSettings(paths, remembered);
configureCredentialProtector({
isEncryptionAvailable: () => false,
encryptString: () => Buffer.alloc(0),
decryptString: () => ""
});
if (mode === "sync") {
saveSettings(paths, remembered);
} else {
await saveSettingsAsync(paths, remembered);
}
const primary = JSON.parse(fs.readFileSync(paths.configFile, "utf8")) as Record<string, unknown>;
const backup = JSON.parse(fs.readFileSync(`${paths.configFile}.bak`, "utf8")) as Record<string, unknown>;
expect(primary.token).toBe("");
expect(backup.token).toBe("");
});
it("migrates remembered plaintext provider values without retaining plaintext in config backups", () => { it("migrates remembered plaintext provider values without retaining plaintext in config backups", () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-store-")); const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-store-"));
tempDirs.push(dir); tempDirs.push(dir);