Skip to content

Commit 3b3d41a

Browse files
Prevent corrupt JSON state from blocking startup (#258)
Co-authored-by: Owen McGirr <o.a.mcgirr@gmail.com>
1 parent 905debe commit 3b3d41a

8 files changed

Lines changed: 386 additions & 16 deletions

src/main/cursor-overlay-settings-store.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
1+
import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs';
22
import { tmpdir } from 'node:os';
33
import { join } from 'node:path';
44
import { afterEach, describe, expect, it, vi } from 'vitest';
@@ -70,4 +70,21 @@ describe('JsonCursorOverlaySettingsStore', () => {
7070
color: 'blue'
7171
});
7272
});
73+
74+
it('creates parent directories when saving', () => {
75+
tempDir = mkdtempSync(join(tmpdir(), 'switchify-cursor-overlay-'));
76+
const settingsFile = join(tempDir, 'nested', 'cursor-overlay-settings.json');
77+
78+
new JsonCursorOverlaySettingsStore(settingsFile).save(DEFAULT_CURSOR_OVERLAY_SETTINGS);
79+
80+
expect(JSON.parse(readFileSync(settingsFile, 'utf8'))).toEqual(DEFAULT_CURSOR_OVERLAY_SETTINGS);
81+
});
82+
83+
it('leaves no temp files after saving', () => {
84+
const settingsStore = store();
85+
86+
settingsStore.save(DEFAULT_CURSOR_OVERLAY_SETTINGS);
87+
88+
expect(readdirSync(tempDir!).filter((name) => name.endsWith('.tmp'))).toEqual([]);
89+
});
7390
});

src/main/cursor-overlay-settings-store.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import { mkdirSync, readFileSync, writeFileSync } from 'node:fs';
2-
import { dirname } from 'node:path';
1+
import { readFileSync } from 'node:fs';
32
import {
43
DEFAULT_CURSOR_OVERLAY_SETTINGS,
54
normalizeCursorOverlaySettings,
65
type CursorOverlaySettings
76
} from '../shared/cursor-overlay-settings';
7+
import { writeJsonFileAtomicSync } from './json-file-store';
88

99
export class JsonCursorOverlaySettingsStore {
1010
constructor(private readonly filePath: string) {}
@@ -25,8 +25,7 @@ export class JsonCursorOverlaySettingsStore {
2525

2626
save(settings: CursorOverlaySettings): CursorOverlaySettings {
2727
const normalized = normalizeCursorOverlaySettings(settings);
28-
mkdirSync(dirname(this.filePath), { recursive: true });
29-
writeFileSync(this.filePath, `${JSON.stringify(normalized, null, 2)}\n`, 'utf8');
28+
writeJsonFileAtomicSync(this.filePath, `${JSON.stringify(normalized, null, 2)}\n`);
3029
return normalized;
3130
}
3231
}

src/main/json-file-store.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs';
2+
import { tmpdir } from 'node:os';
3+
import { basename, join } from 'node:path';
4+
import { afterEach, describe, expect, it } from 'vitest';
5+
import { backupCorruptJsonFile, writeJsonFileAtomic, writeJsonFileAtomicSync } from './json-file-store';
6+
7+
describe('json file store helpers', () => {
8+
let tempDir: string | null = null;
9+
10+
afterEach(() => {
11+
if (tempDir) {
12+
rmSync(tempDir, { recursive: true, force: true });
13+
tempDir = null;
14+
}
15+
});
16+
17+
it('writes async JSON content to the target path', async () => {
18+
const filePath = path('state.json');
19+
20+
await writeJsonFileAtomic(filePath, '{ "ok": true }\n');
21+
22+
expect(readFileSync(filePath, 'utf8')).toBe('{ "ok": true }\n');
23+
expect(readdirSync(tempDir!).filter((name) => name.endsWith('.tmp'))).toEqual([]);
24+
});
25+
26+
it('creates parent directories for async writes', async () => {
27+
const filePath = path('nested', 'state.json');
28+
29+
await writeJsonFileAtomic(filePath, '{}\n');
30+
31+
expect(readFileSync(filePath, 'utf8')).toBe('{}\n');
32+
});
33+
34+
it('removes async temp files on failure before rename', async () => {
35+
const parentFile = path('not-a-directory');
36+
writeFileSync(parentFile, 'file', 'utf8');
37+
38+
await expect(writeJsonFileAtomic(join(parentFile, 'state.json'), '{}\n')).rejects.toThrow();
39+
expect(readdirSync(tempDir!).filter((name) => name.endsWith('.tmp'))).toEqual([]);
40+
});
41+
42+
it('writes sync JSON content to the target path', () => {
43+
const filePath = path('sync-state.json');
44+
45+
writeJsonFileAtomicSync(filePath, '{ "ok": true }\n');
46+
47+
expect(readFileSync(filePath, 'utf8')).toBe('{ "ok": true }\n');
48+
expect(readdirSync(tempDir!).filter((name) => name.endsWith('.tmp'))).toEqual([]);
49+
});
50+
51+
it('creates parent directories for sync writes', () => {
52+
const filePath = path('sync-nested', 'state.json');
53+
54+
writeJsonFileAtomicSync(filePath, '{}\n');
55+
56+
expect(readFileSync(filePath, 'utf8')).toBe('{}\n');
57+
});
58+
59+
it('backs up corrupt JSON files with a safe filename', async () => {
60+
const filePath = path('pairing-state.json');
61+
writeFileSync(filePath, '\0\0', 'utf8');
62+
63+
const result = await backupCorruptJsonFile(filePath);
64+
65+
expect(result.backupPath).toMatch(/pairing-state\.corrupt-\d{8}T\d{9}Z\.json$/);
66+
expect(basename(result.backupPath!)).not.toContain(':');
67+
expect(readFileSync(result.backupPath!, 'utf8')).toBe('\0\0');
68+
});
69+
70+
it('returns null backup path for a missing corrupt file', async () => {
71+
await expect(backupCorruptJsonFile(path('missing.json'))).resolves.toEqual({ backupPath: null });
72+
});
73+
74+
function path(...parts: string[]): string {
75+
if (!tempDir) {
76+
tempDir = mkdtempSync(join(tmpdir(), 'switchify-json-store-'));
77+
}
78+
79+
return join(tempDir, ...parts);
80+
}
81+
});

src/main/json-file-store.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { constants } from 'node:fs';
2+
import { mkdir, open, rename, unlink } from 'node:fs/promises';
3+
import { closeSync, existsSync, fsyncSync, mkdirSync, openSync, renameSync, unlinkSync, writeFileSync } from 'node:fs';
4+
import { dirname, extname, join, basename } from 'node:path';
5+
6+
export type CorruptJsonBackupResult = {
7+
backupPath: string | null;
8+
};
9+
10+
export async function writeJsonFileAtomic(filePath: string, content: string): Promise<void> {
11+
await mkdir(dirname(filePath), { recursive: true });
12+
const tempPath = tempPathFor(filePath);
13+
let handle: Awaited<ReturnType<typeof open>> | null = null;
14+
let renamed = false;
15+
16+
try {
17+
handle = await open(tempPath, 'w', 0o600);
18+
await handle.writeFile(content, 'utf8');
19+
await handle.sync();
20+
await handle.close();
21+
handle = null;
22+
await rename(tempPath, filePath);
23+
renamed = true;
24+
} finally {
25+
if (handle) {
26+
await handle.close().catch(() => undefined);
27+
}
28+
29+
if (!renamed) {
30+
await unlink(tempPath).catch(() => undefined);
31+
}
32+
}
33+
}
34+
35+
export function writeJsonFileAtomicSync(filePath: string, content: string): void {
36+
mkdirSync(dirname(filePath), { recursive: true });
37+
const tempPath = tempPathFor(filePath);
38+
let fd: number | null = null;
39+
let renamed = false;
40+
41+
try {
42+
fd = openSync(tempPath, constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC, 0o600);
43+
writeFileSync(fd, content, 'utf8');
44+
fsyncSync(fd);
45+
closeSync(fd);
46+
fd = null;
47+
renameSync(tempPath, filePath);
48+
renamed = true;
49+
} finally {
50+
if (fd !== null) {
51+
try {
52+
closeSync(fd);
53+
} catch {
54+
// Best effort cleanup.
55+
}
56+
}
57+
58+
if (!renamed) {
59+
try {
60+
unlinkSync(tempPath);
61+
} catch {
62+
// Best effort cleanup.
63+
}
64+
}
65+
}
66+
}
67+
68+
export async function backupCorruptJsonFile(filePath: string): Promise<CorruptJsonBackupResult> {
69+
if (!existsSync(filePath)) {
70+
return { backupPath: null };
71+
}
72+
73+
const backupPath = corruptBackupPathFor(filePath);
74+
75+
try {
76+
await rename(filePath, backupPath);
77+
return { backupPath };
78+
} catch (error) {
79+
if (isMissingFileError(error)) {
80+
return { backupPath: null };
81+
}
82+
83+
throw error;
84+
}
85+
}
86+
87+
function tempPathFor(filePath: string): string {
88+
return `${filePath}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp`;
89+
}
90+
91+
function corruptBackupPathFor(filePath: string, now = new Date()): string {
92+
const extension = extname(filePath);
93+
const name = basename(filePath, extension);
94+
return join(dirname(filePath), `${name}.corrupt-${sanitizeTimestamp(now.toISOString())}${extension}`);
95+
}
96+
97+
function sanitizeTimestamp(value: string): string {
98+
return value.replace(/[-:.]/g, '');
99+
}
100+
101+
function isMissingFileError(error: unknown): boolean {
102+
return (
103+
error !== null &&
104+
typeof error === 'object' &&
105+
'code' in error &&
106+
(error as { code?: unknown }).code === 'ENOENT'
107+
);
108+
}

src/main/pairing/pairing-store.test.ts

Lines changed: 141 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
import { describe, expect, it } from 'vitest';
2-
import { removePairedDevice, toPairedDeviceViews, type PairingState } from './pairing-store';
1+
import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs';
2+
import { tmpdir } from 'node:os';
3+
import { join } from 'node:path';
4+
import { afterEach, describe, expect, it, vi } from 'vitest';
5+
import { JsonPairingStore, removePairedDevice, toPairedDeviceViews, type PairingState } from './pairing-store';
36

47
describe('toPairedDeviceViews', () => {
58
it('removes shared tokens from paired device metadata', () => {
@@ -60,6 +63,142 @@ describe('removePairedDevice', () => {
6063
});
6164
});
6265

66+
describe('JsonPairingStore', () => {
67+
let tempDir: string | null = null;
68+
69+
afterEach(() => {
70+
vi.restoreAllMocks();
71+
if (tempDir) {
72+
rmSync(tempDir, { recursive: true, force: true });
73+
tempDir = null;
74+
}
75+
});
76+
77+
it('creates and saves fresh pairing state when the file is missing', async () => {
78+
const filePath = pairingPath();
79+
80+
const state = await new JsonPairingStore(filePath).load();
81+
82+
expect(state.desktopId).toMatch(/[0-9a-f-]{36}/);
83+
expect(state.pairedDevices).toEqual([]);
84+
expect(JSON.parse(readFileSync(filePath, 'utf8'))).toEqual(state);
85+
});
86+
87+
it('loads valid pairing state', async () => {
88+
const filePath = pairingPath();
89+
writeFileSync(filePath, JSON.stringify(createState()), 'utf8');
90+
91+
await expect(new JsonPairingStore(filePath).load()).resolves.toEqual(createState());
92+
});
93+
94+
it('saves formatted JSON and creates parent directories', async () => {
95+
const filePath = nestedPairingPath();
96+
97+
await new JsonPairingStore(filePath).save(createState());
98+
99+
expect(readFileSync(filePath, 'utf8')).toBe(`${JSON.stringify(createState(), null, 2)}\n`);
100+
});
101+
102+
it('leaves no temp files after a successful save', async () => {
103+
const filePath = pairingPath();
104+
105+
await new JsonPairingStore(filePath).save(createState());
106+
107+
expect(readdirSync(tempDir!).filter((name) => name.endsWith('.tmp'))).toEqual([]);
108+
});
109+
110+
it('backs up invalid JSON and replaces it with fresh valid state', async () => {
111+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
112+
const filePath = pairingPath();
113+
writeFileSync(filePath, '{', 'utf8');
114+
115+
const state = await new JsonPairingStore(filePath).load();
116+
117+
expect(state.pairedDevices).toEqual([]);
118+
expect(JSON.parse(readFileSync(filePath, 'utf8'))).toEqual(state);
119+
expect(corruptBackups()).toHaveLength(1);
120+
expect(warn.mock.calls.flat().join('\n')).not.toContain('{');
121+
});
122+
123+
it('backs up NUL-byte files and replaces them with fresh valid state', async () => {
124+
const filePath = pairingPath();
125+
writeFileSync(filePath, '\0'.repeat(562), 'utf8');
126+
127+
const state = await new JsonPairingStore(filePath).load();
128+
129+
expect(state.desktopId).toMatch(/[0-9a-f-]{36}/);
130+
expect(state.pairedDevices).toEqual([]);
131+
expect(readFileSync(filePath, 'utf8')).toContain('"pairedDevices": []');
132+
expect(corruptBackups()).toHaveLength(1);
133+
});
134+
135+
it('backs up invalid pairing state schema and replaces it with fresh valid state', async () => {
136+
const filePath = pairingPath();
137+
writeFileSync(filePath, JSON.stringify({ desktopId: 1, pairedDevices: [] }), 'utf8');
138+
139+
const state = await new JsonPairingStore(filePath).load();
140+
141+
expect(state.pairedDevices).toEqual([]);
142+
expect(corruptBackups()).toHaveLength(1);
143+
});
144+
145+
it('backs up invalid paired-device entries and replaces them with fresh valid state', async () => {
146+
const filePath = pairingPath();
147+
writeFileSync(
148+
filePath,
149+
JSON.stringify({
150+
desktopId: 'desktop-1',
151+
pairedDevices: [{ deviceId: 'android-1', token: 'secret-token' }]
152+
}),
153+
'utf8'
154+
);
155+
156+
const state = await new JsonPairingStore(filePath).load();
157+
158+
expect(state.pairedDevices).toEqual([]);
159+
expect(corruptBackups()).toHaveLength(1);
160+
});
161+
162+
it('does not log tokens or corrupt pairing contents during recovery', async () => {
163+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
164+
const filePath = pairingPath();
165+
writeFileSync(
166+
filePath,
167+
JSON.stringify({
168+
desktopId: 'desktop-1',
169+
pairedDevices: [{ deviceId: 'android-1', token: 'secret-token' }]
170+
}),
171+
'utf8'
172+
);
173+
174+
await new JsonPairingStore(filePath).load();
175+
176+
const warningText = warn.mock.calls.flat().join('\n');
177+
expect(warningText).not.toContain('secret-token');
178+
expect(warningText).not.toContain('android-1');
179+
});
180+
181+
function pairingPath(): string {
182+
if (!tempDir) {
183+
tempDir = mkdtempSync(join(tmpdir(), 'switchify-pairing-store-'));
184+
}
185+
186+
return join(tempDir, 'pairing-state.json');
187+
}
188+
189+
function nestedPairingPath(): string {
190+
if (!tempDir) {
191+
tempDir = mkdtempSync(join(tmpdir(), 'switchify-pairing-store-'));
192+
}
193+
194+
return join(tempDir, 'nested', 'pairing-state.json');
195+
}
196+
197+
function corruptBackups(): string[] {
198+
return readdirSync(tempDir!).filter((name) => name.startsWith('pairing-state.corrupt-'));
199+
}
200+
});
201+
63202
function createState(): PairingState {
64203
return {
65204
desktopId: 'desktop-1',

0 commit comments

Comments
 (0)