Skip to content

Commit 1c22410

Browse files
jeremy-clerkwobsoriano
authored andcommitted
feat(electron): harden token storage encryption (#8791)
1 parent 61ccd5d commit 1c22410

2 files changed

Lines changed: 450 additions & 33 deletions

File tree

‎packages/electron/src/storage/__tests__/index.test.ts‎

Lines changed: 246 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ const storeGet = vi.fn();
88
const storeSet = vi.fn();
99
const storeDelete = vi.fn();
1010

11+
// `safeStorage` starts empty; each test installs only the methods for the backend it exercises.
12+
// This mirrors reality: Electron < 42 has no async methods at all, so `resolveCipher` only takes the
13+
// async path when both the methods exist and `isAsyncEncryptionAvailable()` confirms it.
1114
vi.mock('electron', () => ({
12-
safeStorage: {
13-
decryptString: vi.fn(),
14-
encryptString: vi.fn(),
15-
},
15+
safeStorage: {},
1616
}));
1717

1818
vi.mock('electron-store', () => ({
@@ -23,10 +23,32 @@ vi.mock('electron-store', () => ({
2323
})),
2424
}));
2525

26-
describe('storage', () => {
27-
beforeEach(() => {
28-
vi.clearAllMocks();
29-
});
26+
const ss = safeStorage as unknown as Record<string, unknown>;
27+
28+
/** Installs the synchronous `safeStorage` API. */
29+
function installSync({ available = true }: { available?: boolean } = {}) {
30+
ss.isEncryptionAvailable = vi.fn(() => available);
31+
ss.encryptString = vi.fn((value: string) => Buffer.from(`enc(${value})`));
32+
ss.decryptString = vi.fn();
33+
}
34+
35+
/** Adds the Electron 42+ asynchronous `safeStorage` API. */
36+
function installAsync({ available = true }: { available?: boolean } = {}) {
37+
ss.isAsyncEncryptionAvailable = vi.fn(() => Promise.resolve(available));
38+
ss.encryptStringAsync = vi.fn((value: string) => Promise.resolve(Buffer.from(`enc(${value})`)));
39+
ss.decryptStringAsync = vi.fn();
40+
}
41+
42+
beforeEach(() => {
43+
vi.clearAllMocks();
44+
// Reset the mocked `safeStorage` shape so backend detection starts from a clean slate.
45+
for (const key of Object.keys(ss)) {
46+
delete ss[key];
47+
}
48+
});
49+
50+
describe('storage options', () => {
51+
beforeEach(() => installSync());
3052

3153
it('creates an electron-store instance with the default store name', () => {
3254
storage();
@@ -40,38 +62,239 @@ describe('storage', () => {
4062
expect(Store).toHaveBeenCalledWith({ name: 'custom-clerk-tokens' });
4163
});
4264

43-
it('returns null when a token is missing', () => {
65+
it('forwards a custom path as electron-store `cwd`', () => {
66+
storage({ path: '/tmp/clerk' });
67+
68+
expect(Store).toHaveBeenCalledWith({ name: 'clerk-tokens', cwd: '/tmp/clerk' });
69+
});
70+
71+
it('omits `cwd` when no path is provided', () => {
72+
storage();
73+
74+
expect(Store).toHaveBeenCalledWith(expect.not.objectContaining({ cwd: expect.anything() }));
75+
});
76+
});
77+
78+
describe('getItem', () => {
79+
it('returns null when a token is missing', async () => {
80+
installSync();
4481
storeGet.mockReturnValue(undefined);
4582

46-
expect(storage().getItem('token-key')).toBeNull();
83+
await expect(storage().getItem('token-key')).resolves.toBeNull();
84+
});
85+
86+
it('returns an unencrypted (raw:) value as-is without decrypting', async () => {
87+
installSync();
88+
storeGet.mockReturnValue('raw:jwt');
89+
90+
await expect(storage().getItem('token-key')).resolves.toBe('jwt');
91+
expect(safeStorage.decryptString).not.toHaveBeenCalled();
92+
});
93+
94+
it('deletes and returns null for an unrecognized value format', async () => {
95+
installSync();
96+
storeGet.mockReturnValue('garbage-without-prefix');
97+
98+
await expect(storage().getItem('token-key')).resolves.toBeNull();
99+
expect(storeDelete).toHaveBeenCalledWith('token-key');
100+
});
101+
102+
it('preserves the entry and returns null when no OS encryption is available', async () => {
103+
installSync({ available: false });
104+
storeGet.mockReturnValue(`enc:${Buffer.from('cipher').toString('base64')}`);
105+
106+
await expect(storage().getItem('token-key')).resolves.toBeNull();
107+
expect(storeDelete).not.toHaveBeenCalled();
108+
});
109+
110+
describe('sync backend', () => {
111+
it('decrypts stored tokens', async () => {
112+
installSync();
113+
storeGet.mockReturnValue(`enc:${Buffer.from('cipher').toString('base64')}`);
114+
vi.mocked(safeStorage.decryptString).mockReturnValue('jwt');
115+
116+
await expect(storage().getItem('token-key')).resolves.toBe('jwt');
117+
expect(safeStorage.decryptString).toHaveBeenCalledWith(Buffer.from('cipher'));
118+
});
119+
120+
it('returns null without deleting the entry when decryption fails', async () => {
121+
installSync();
122+
storeGet.mockReturnValue(`enc:${Buffer.from('cipher').toString('base64')}`);
123+
vi.mocked(safeStorage.decryptString).mockImplementation(() => {
124+
throw new Error('decrypt failed');
125+
});
126+
127+
await expect(storage().getItem('token-key')).resolves.toBeNull();
128+
expect(storeDelete).not.toHaveBeenCalled();
129+
});
130+
});
131+
132+
describe('async backend', () => {
133+
it('decrypts stored tokens', async () => {
134+
installSync();
135+
installAsync();
136+
storeGet.mockReturnValue(`enc:${Buffer.from('cipher').toString('base64')}`);
137+
vi.mocked(safeStorage.decryptStringAsync).mockResolvedValue({ result: 'jwt', shouldReEncrypt: false });
138+
139+
await expect(storage().getItem('token-key')).resolves.toBe('jwt');
140+
expect(safeStorage.decryptStringAsync).toHaveBeenCalledWith(Buffer.from('cipher'));
141+
});
142+
143+
it('re-encrypts and re-saves when the OS key has rotated (shouldReEncrypt)', async () => {
144+
installSync();
145+
installAsync();
146+
storeGet.mockReturnValue(`enc:${Buffer.from('old-cipher').toString('base64')}`);
147+
vi.mocked(safeStorage.decryptStringAsync).mockResolvedValue({ result: 'jwt', shouldReEncrypt: true });
148+
149+
await expect(storage().getItem('token-key')).resolves.toBe('jwt');
150+
expect(safeStorage.encryptStringAsync).toHaveBeenCalledWith('jwt');
151+
expect(storeSet).toHaveBeenCalledWith('token-key', `enc:${Buffer.from('enc(jwt)').toString('base64')}`);
152+
});
153+
154+
it('returns null without deleting the entry when decryption rejects', async () => {
155+
installSync();
156+
installAsync();
157+
storeGet.mockReturnValue(`enc:${Buffer.from('cipher').toString('base64')}`);
158+
vi.mocked(safeStorage.decryptStringAsync).mockRejectedValue(new Error('decrypt failed'));
159+
160+
await expect(storage().getItem('token-key')).resolves.toBeNull();
161+
expect(storeDelete).not.toHaveBeenCalled();
162+
});
163+
164+
it('falls back to the sync API (never calling the async crypto) when async encryption is unavailable', async () => {
165+
installSync();
166+
installAsync({ available: false });
167+
storeGet.mockReturnValue(`enc:${Buffer.from('cipher').toString('base64')}`);
168+
vi.mocked(safeStorage.decryptString).mockReturnValue('jwt');
169+
170+
await expect(storage().getItem('token-key')).resolves.toBe('jwt');
171+
expect(safeStorage.decryptString).toHaveBeenCalledWith(Buffer.from('cipher'));
172+
// Critical: calling the async crypto while unavailable crashes the process on Electron 42.x.
173+
expect(safeStorage.decryptStringAsync).not.toHaveBeenCalled();
174+
});
175+
});
176+
});
177+
178+
describe('setItem', () => {
179+
it('encrypts tokens before storing them (sync backend)', async () => {
180+
installSync();
181+
182+
await storage().setItem('token-key', 'jwt');
183+
184+
expect(safeStorage.encryptString).toHaveBeenCalledWith('jwt');
185+
expect(storeSet).toHaveBeenCalledWith('token-key', `enc:${Buffer.from('enc(jwt)').toString('base64')}`);
186+
});
187+
188+
it('encrypts tokens before storing them (async backend)', async () => {
189+
installSync();
190+
installAsync();
191+
192+
await storage().setItem('token-key', 'jwt');
193+
194+
expect(safeStorage.encryptStringAsync).toHaveBeenCalledWith('jwt');
195+
expect(storeSet).toHaveBeenCalledWith('token-key', `enc:${Buffer.from('enc(jwt)').toString('base64')}`);
196+
});
197+
198+
it('falls back to the sync API (never calling the async crypto) when async encryption is unavailable', async () => {
199+
installSync();
200+
installAsync({ available: false });
201+
202+
await storage().setItem('token-key', 'jwt');
203+
204+
expect(safeStorage.encryptString).toHaveBeenCalledWith('jwt');
205+
expect(safeStorage.encryptStringAsync).not.toHaveBeenCalled();
206+
expect(storeSet).toHaveBeenCalledWith('token-key', `enc:${Buffer.from('enc(jwt)').toString('base64')}`);
207+
});
208+
209+
it('does not persist when no encryption is available and no fallback is configured', async () => {
210+
installSync({ available: false });
211+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
212+
213+
await storage().setItem('token-key', 'jwt');
214+
215+
expect(storeSet).not.toHaveBeenCalled();
216+
expect(warn).toHaveBeenCalledOnce();
217+
218+
warn.mockRestore();
47219
});
48220

49-
it('decrypts stored tokens', () => {
50-
storeGet.mockReturnValue(Buffer.from('encrypted-token').toString('base64'));
51-
vi.mocked(safeStorage.decryptString).mockReturnValue('jwt');
221+
it('persists unencrypted when no encryption is available and unencryptedFallback is enabled', async () => {
222+
installSync({ available: false });
223+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
224+
225+
await storage({ unencryptedFallback: true }).setItem('token-key', 'jwt');
52226

53-
expect(storage().getItem('token-key')).toBe('jwt');
54-
expect(safeStorage.decryptString).toHaveBeenCalledWith(Buffer.from('encrypted-token'));
227+
expect(storeSet).toHaveBeenCalledWith('token-key', 'raw:jwt');
228+
expect(warn).toHaveBeenCalledOnce();
229+
230+
warn.mockRestore();
55231
});
56232

57-
it('returns null when token decryption fails', () => {
58-
storeGet.mockReturnValue(Buffer.from('encrypted-token').toString('base64'));
59-
vi.mocked(safeStorage.decryptString).mockImplementation(() => {
60-
throw new Error('decrypt failed');
233+
it('only warns once across repeated saves', async () => {
234+
installSync({ available: false });
235+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
236+
237+
const adapter = storage();
238+
await adapter.setItem('a', '1');
239+
await adapter.setItem('b', '2');
240+
241+
expect(warn).toHaveBeenCalledOnce();
242+
243+
warn.mockRestore();
244+
});
245+
246+
it('does not downgrade to plaintext when encryption is available but encrypt() fails', async () => {
247+
ss.isEncryptionAvailable = vi.fn(() => true);
248+
ss.encryptString = vi.fn(() => {
249+
throw new Error('encrypt failed');
61250
});
251+
ss.decryptString = vi.fn();
252+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
253+
254+
// Even with the fallback enabled, a *failed encrypt* (vs. unavailable encryption) must not be
255+
// persisted in the clear.
256+
await storage({ unencryptedFallback: true }).setItem('token-key', 'jwt');
257+
258+
expect(storeSet).not.toHaveBeenCalled();
259+
expect(warn).toHaveBeenCalledOnce();
260+
261+
warn.mockRestore();
262+
});
62263

63-
expect(storage().getItem('token-key')).toBeNull();
264+
it('resolves the cipher lazily and retries when it was initially unavailable (e.g. before app ready)', async () => {
265+
// Unavailable on the first probe (pre-`ready`), available afterwards.
266+
ss.isEncryptionAvailable = vi.fn().mockReturnValueOnce(false).mockReturnValue(true);
267+
ss.encryptString = vi.fn((value: string) => Buffer.from(`enc(${value})`));
268+
ss.decryptString = vi.fn();
269+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
270+
271+
const adapter = storage();
272+
273+
await adapter.setItem('token-key', 'jwt');
274+
expect(storeSet).not.toHaveBeenCalled(); // not persisted while unavailable
275+
276+
await adapter.setItem('token-key', 'jwt');
277+
expect(storeSet).toHaveBeenCalledWith('token-key', `enc:${Buffer.from('enc(jwt)').toString('base64')}`);
278+
279+
warn.mockRestore();
64280
});
65281

66-
it('encrypts tokens before storing them', async () => {
67-
vi.mocked(safeStorage.encryptString).mockReturnValue(Buffer.from('encrypted-token'));
282+
it('ignores a partial async surface and uses the sync API', async () => {
283+
// `encryptStringAsync` exists but the rest of the async trio does not — must not take the async
284+
// path (and must not call the async crypto).
285+
ss.encryptStringAsync = vi.fn();
286+
ss.isEncryptionAvailable = vi.fn(() => true);
287+
ss.encryptString = vi.fn((value: string) => Buffer.from(`enc(${value})`));
288+
ss.decryptString = vi.fn();
68289

69290
await storage().setItem('token-key', 'jwt');
70291

71292
expect(safeStorage.encryptString).toHaveBeenCalledWith('jwt');
72-
expect(storeSet).toHaveBeenCalledWith('token-key', Buffer.from('encrypted-token').toString('base64'));
293+
expect(safeStorage.encryptStringAsync).not.toHaveBeenCalled();
73294
});
295+
});
74296

297+
describe('removeItem', () => {
75298
it('removes stored tokens', async () => {
76299
await storage().removeItem('token-key');
77300

0 commit comments

Comments
 (0)