fix(storage): preserve sanitized settings backups
Restore previous-state recovery semantics by reading and normalizing the prior primary config, reapplying the current credential persistence policy, and atomically storing that sanitized prior state as the backup for both sync and async saves. Create a credential-free current-state fallback when the prior primary is absent or invalid, capture async settings before later mutations, and add matching regression coverage for recovery, credential removal, and safe fallback behavior across both save paths.
This commit is contained in:
+54
-34
@@ -944,6 +944,30 @@ function settingsPayload(settings: AppSettings): string {
|
|||||||
return JSON.stringify(protectPersistedSettings(normalizeSettings(settings)), safeJsonReplacer, 2);
|
return JSON.stringify(protectPersistedSettings(normalizeSettings(settings)), safeJsonReplacer, 2);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
interface SettingsSavePayloads {
|
||||||
|
primary: string;
|
||||||
|
backup: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
function credentialFreeSettingsPayload(settings: AppSettings): string {
|
||||||
|
return settingsPayload({ ...settings, rememberToken: false });
|
||||||
|
}
|
||||||
|
|
||||||
|
function createSettingsSavePayloads(paths: StoragePaths, settings: AppSettings): SettingsSavePayloads {
|
||||||
|
const previous = readSettingsFile(paths.configFile);
|
||||||
|
const backup = previous
|
||||||
|
? settingsPayload({ ...previous.settings, rememberToken: settings.rememberToken })
|
||||||
|
: credentialFreeSettingsPayload(settings);
|
||||||
|
return {
|
||||||
|
primary: settingsPayload(settings),
|
||||||
|
backup
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function captureSettings(settings: AppSettings): AppSettings {
|
||||||
|
return JSON.parse(JSON.stringify(normalizeSettings(settings))) as AppSettings;
|
||||||
|
}
|
||||||
|
|
||||||
function writeSettingsFileAtomically(filePath: string, payload: string): void {
|
function writeSettingsFileAtomically(filePath: string, payload: string): void {
|
||||||
const tempPath = `${filePath}.tmp`;
|
const tempPath = `${filePath}.tmp`;
|
||||||
try {
|
try {
|
||||||
@@ -1052,45 +1076,42 @@ 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);
|
||||||
const payload = settingsPayload(settings);
|
const payloads = createSettingsSavePayloads(paths, settings);
|
||||||
if (fs.existsSync(paths.configFile)) {
|
writeSettingsFileAtomically(`${paths.configFile}.bak`, payloads.backup);
|
||||||
writeSettingsFileAtomically(`${paths.configFile}.bak`, payload);
|
writeSettingsFileAtomically(paths.configFile, payloads.primary);
|
||||||
}
|
|
||||||
writeSettingsFileAtomically(paths.configFile, payload);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
let asyncSettingsSaveRunning = false;
|
let asyncSettingsSaveRunning = false;
|
||||||
let asyncSettingsSaveQueued: { paths: StoragePaths; payload: string; generation: number } | null = null;
|
let asyncSettingsSaveQueued: { paths: StoragePaths; settings: AppSettings; 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, settings: AppSettings, generation: number): Promise<void> {
|
||||||
await fs.promises.mkdir(paths.baseDir, { recursive: true });
|
await fs.promises.mkdir(paths.baseDir, { recursive: true });
|
||||||
|
const payloads = createSettingsSavePayloads(paths, settings);
|
||||||
const tempPath = `${paths.configFile}.settings.tmp`;
|
const tempPath = `${paths.configFile}.settings.tmp`;
|
||||||
await fsp.writeFile(tempPath, payload, "utf8");
|
await fsp.writeFile(tempPath, payloads.primary, "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)) {
|
const backupTempPath = `${paths.configFile}.bak.settings.tmp`;
|
||||||
const backupTempPath = `${paths.configFile}.bak.settings.tmp`;
|
await fsp.writeFile(backupTempPath, payloads.backup, "utf8");
|
||||||
await fsp.writeFile(backupTempPath, payload, "utf8");
|
if (generation < syncSettingsSaveGeneration) {
|
||||||
if (generation < syncSettingsSaveGeneration) {
|
await Promise.all([
|
||||||
await Promise.all([
|
fsp.rm(tempPath, { force: true }).catch(() => {}),
|
||||||
fsp.rm(tempPath, { force: true }).catch(() => {}),
|
fsp.rm(backupTempPath, { force: true }).catch(() => {})
|
||||||
fsp.rm(backupTempPath, { force: true }).catch(() => {})
|
]);
|
||||||
]);
|
return;
|
||||||
return;
|
}
|
||||||
}
|
try {
|
||||||
try {
|
await fsp.rename(backupTempPath, `${paths.configFile}.bak`);
|
||||||
await fsp.rename(backupTempPath, `${paths.configFile}.bak`);
|
} 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") {
|
await fsp.copyFile(backupTempPath, `${paths.configFile}.bak`);
|
||||||
await fsp.copyFile(backupTempPath, `${paths.configFile}.bak`);
|
await fsp.rm(backupTempPath, { force: true }).catch(() => {});
|
||||||
await fsp.rm(backupTempPath, { force: true }).catch(() => {});
|
} else {
|
||||||
} else {
|
await fsp.rm(backupTempPath, { force: true }).catch(() => {});
|
||||||
await fsp.rm(backupTempPath, { force: true }).catch(() => {});
|
throw renameError;
|
||||||
throw renameError;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
@@ -1110,14 +1131,14 @@ async function writeSettingsPayload(paths: StoragePaths, payload: string, genera
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async function saveSettingsPayloadAsync(paths: StoragePaths, payload: string, generation: number): Promise<void> {
|
async function saveSettingsPayloadAsync(paths: StoragePaths, settings: AppSettings, generation: number): Promise<void> {
|
||||||
if (asyncSettingsSaveRunning) {
|
if (asyncSettingsSaveRunning) {
|
||||||
asyncSettingsSaveQueued = { paths, payload, generation };
|
asyncSettingsSaveQueued = { paths, settings, generation };
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
asyncSettingsSaveRunning = true;
|
asyncSettingsSaveRunning = true;
|
||||||
try {
|
try {
|
||||||
await writeSettingsPayload(paths, payload, generation);
|
await writeSettingsPayload(paths, settings, generation);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
logger.error(`Async Settings-Save fehlgeschlagen: ${String(error)}`);
|
logger.error(`Async Settings-Save fehlgeschlagen: ${String(error)}`);
|
||||||
} finally {
|
} finally {
|
||||||
@@ -1125,15 +1146,14 @@ async function saveSettingsPayloadAsync(paths: StoragePaths, payload: string, ge
|
|||||||
if (asyncSettingsSaveQueued) {
|
if (asyncSettingsSaveQueued) {
|
||||||
const queued = asyncSettingsSaveQueued;
|
const queued = asyncSettingsSaveQueued;
|
||||||
asyncSettingsSaveQueued = null;
|
asyncSettingsSaveQueued = null;
|
||||||
void saveSettingsPayloadAsync(queued.paths, queued.payload, queued.generation);
|
void saveSettingsPayloadAsync(queued.paths, queued.settings, queued.generation);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function saveSettingsAsync(paths: StoragePaths, settings: AppSettings): Promise<void> {
|
export async function saveSettingsAsync(paths: StoragePaths, settings: AppSettings): Promise<void> {
|
||||||
const generation = syncSettingsSaveGeneration;
|
const generation = syncSettingsSaveGeneration;
|
||||||
const payload = settingsPayload(settings);
|
await saveSettingsPayloadAsync(paths, captureSettings(settings), generation);
|
||||||
await saveSettingsPayloadAsync(paths, payload, generation);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export function emptySession(): SessionState {
|
export function emptySession(): SessionState {
|
||||||
|
|||||||
+74
-12
@@ -11,6 +11,15 @@ import { configureCredentialProtector } from "../src/main/credential-protection"
|
|||||||
import { addHistoryEntryForRetention, createStoragePaths, emptySession, loadHistory, loadHistoryForRetention, loadSession, loadSettings, normalizeLoadedSession, normalizeSettings, resetHistoryForRetention, saveHistory, saveSession, saveSessionAsync, saveSettings, saveSettingsAsync } 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[] = [];
|
||||||
|
type SettingsSaveMode = "sync" | "async";
|
||||||
|
|
||||||
|
async function saveSettingsInMode(mode: SettingsSaveMode, paths: ReturnType<typeof createStoragePaths>, settings: AppSettings): Promise<void> {
|
||||||
|
if (mode === "sync") {
|
||||||
|
saveSettings(paths, settings);
|
||||||
|
} else {
|
||||||
|
await saveSettingsAsync(paths, settings);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
configureCredentialProtector({
|
configureCredentialProtector({
|
||||||
@@ -195,6 +204,32 @@ describe("settings storage", () => {
|
|||||||
expect(loaded.allDebridToken).toBe("all-token");
|
expect(loaded.allDebridToken).toBe("all-token");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.each(["sync", "async"] as const)("preserves the previous recoverable settings state during a %s save", async (mode) => {
|
||||||
|
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-store-"));
|
||||||
|
tempDirs.push(dir);
|
||||||
|
const paths = createStoragePaths(dir);
|
||||||
|
const previousToken = "previous-value-for-recovery";
|
||||||
|
const previous = {
|
||||||
|
...defaultSettings(),
|
||||||
|
packageName: "previous-package-name",
|
||||||
|
token: previousToken
|
||||||
|
};
|
||||||
|
saveSettings(paths, previous);
|
||||||
|
|
||||||
|
await saveSettingsInMode(mode, paths, {
|
||||||
|
...previous,
|
||||||
|
packageName: "current-package-name",
|
||||||
|
token: "current-value"
|
||||||
|
});
|
||||||
|
|
||||||
|
const backupText = fs.readFileSync(`${paths.configFile}.bak`, "utf8");
|
||||||
|
expect(backupText).not.toContain(previousToken);
|
||||||
|
fs.writeFileSync(paths.configFile, "{broken-current-config", "utf8");
|
||||||
|
const recovered = loadSettings(paths);
|
||||||
|
expect(recovered.packageName).toBe("previous-package-name");
|
||||||
|
expect(recovered.token).toBe(previousToken);
|
||||||
|
});
|
||||||
|
|
||||||
it.each(["sync", "async"] as const)("clears previously stored credentials from the backup when remembering is disabled during a %s save", async (mode) => {
|
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-"));
|
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-store-"));
|
||||||
tempDirs.push(dir);
|
tempDirs.push(dir);
|
||||||
@@ -202,21 +237,21 @@ describe("settings storage", () => {
|
|||||||
const remembered = {
|
const remembered = {
|
||||||
...defaultSettings(),
|
...defaultSettings(),
|
||||||
rememberToken: true,
|
rememberToken: true,
|
||||||
token: "stored-value-before-clear"
|
token: "stored-value-before-clear",
|
||||||
|
packageName: "previous-package-name"
|
||||||
};
|
};
|
||||||
saveSettings(paths, remembered);
|
saveSettings(paths, remembered);
|
||||||
|
|
||||||
const cleared = { ...remembered, rememberToken: false };
|
const cleared = { ...remembered, rememberToken: false, packageName: "current-package-name" };
|
||||||
if (mode === "sync") {
|
await saveSettingsInMode(mode, paths, cleared);
|
||||||
saveSettings(paths, cleared);
|
|
||||||
} else {
|
|
||||||
await saveSettingsAsync(paths, cleared);
|
|
||||||
}
|
|
||||||
|
|
||||||
const primary = JSON.parse(fs.readFileSync(paths.configFile, "utf8")) as Record<string, unknown>;
|
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>;
|
const backup = JSON.parse(fs.readFileSync(`${paths.configFile}.bak`, "utf8")) as Record<string, unknown>;
|
||||||
expect(primary.token).toBe("");
|
expect(primary.token).toBe("");
|
||||||
expect(backup.token).toBe("");
|
expect(backup.token).toBe("");
|
||||||
|
expect(primary.packageName).toBe("current-package-name");
|
||||||
|
expect(backup.packageName).toBe("previous-package-name");
|
||||||
|
expect(backup.rememberToken).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it.each(["sync", "async"] as const)("clears previously stored credentials from the backup when encryption is unavailable during a %s save", async (mode) => {
|
it.each(["sync", "async"] as const)("clears previously stored credentials from the backup when encryption is unavailable during a %s save", async (mode) => {
|
||||||
@@ -235,11 +270,7 @@ describe("settings storage", () => {
|
|||||||
decryptString: () => ""
|
decryptString: () => ""
|
||||||
});
|
});
|
||||||
|
|
||||||
if (mode === "sync") {
|
await saveSettingsInMode(mode, paths, remembered);
|
||||||
saveSettings(paths, remembered);
|
|
||||||
} else {
|
|
||||||
await saveSettingsAsync(paths, remembered);
|
|
||||||
}
|
|
||||||
|
|
||||||
const primary = JSON.parse(fs.readFileSync(paths.configFile, "utf8")) as Record<string, unknown>;
|
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>;
|
const backup = JSON.parse(fs.readFileSync(`${paths.configFile}.bak`, "utf8")) as Record<string, unknown>;
|
||||||
@@ -247,6 +278,37 @@ describe("settings storage", () => {
|
|||||||
expect(backup.token).toBe("");
|
expect(backup.token).toBe("");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
["sync", "absent"],
|
||||||
|
["async", "absent"],
|
||||||
|
["sync", "corrupt"],
|
||||||
|
["async", "corrupt"]
|
||||||
|
] as const)("uses a credential-free current fallback for a %s save with a %s prior primary", async (mode, priorState) => {
|
||||||
|
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-store-"));
|
||||||
|
tempDirs.push(dir);
|
||||||
|
const paths = createStoragePaths(dir);
|
||||||
|
if (priorState === "corrupt") {
|
||||||
|
fs.writeFileSync(paths.configFile, "{broken-prior-config", "utf8");
|
||||||
|
}
|
||||||
|
const currentToken = "current-value-for-safe-fallback";
|
||||||
|
|
||||||
|
await saveSettingsInMode(mode, paths, {
|
||||||
|
...defaultSettings(),
|
||||||
|
rememberToken: true,
|
||||||
|
packageName: "current-package-name",
|
||||||
|
token: currentToken
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(fs.existsSync(`${paths.configFile}.bak`)).toBe(true);
|
||||||
|
const backupText = fs.readFileSync(`${paths.configFile}.bak`, "utf8");
|
||||||
|
const backup = JSON.parse(backupText) as Record<string, unknown>;
|
||||||
|
expect(backup.packageName).toBe("current-package-name");
|
||||||
|
expect(backup.rememberToken).toBe(false);
|
||||||
|
expect(backup.token).toBe("");
|
||||||
|
expect(backupText).not.toContain(currentToken);
|
||||||
|
expect(loadSettings(paths).token).toBe(currentToken);
|
||||||
|
});
|
||||||
|
|
||||||
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);
|
||||||
|
|||||||
Reference in New Issue
Block a user