From a3be4e485545aff2036db2fa8161a55535e1e5de Mon Sep 17 00:00:00 2001 From: Kevin Cui Date: Sat, 24 Oct 2026 02:42:59 -0400 Subject: [PATCH 1/3] build(deps-dev): bump electron to 45.0.0 Electron 42 bundles Node.js 24.19.0, whose libuv reports mangled filenames from `fs.watch()` when the watched directory contains 8.3 short names (libuv/libuv#5010). GitHub Actions sets TEMP to such a path (C:\Users\RUNNER~1\...), so the "watches for the types file to be updated" test in _tests/main/electron-types.spec.ts_ never saw its change event and failed on every Windows run. Electron 45 bundles Node.js 24.22.0, which carries the fix. Electron 44 dropped Windows ia32 and Linux armv7l binaries, so the release workflow no longer builds those targets, and the test and build matrix entries that already ran on arm64 hosts are relabeled arm64. The synchronous `safeStorage` methods are deprecated in Electron 45, so token storage in _src/main/github.ts_ moves to `encryptStringAsync()`, `decryptStringAsync()` and `isAsyncEncryptionAvailable()`. Tokens saved by earlier versions still decrypt, and a token is re-encrypted when `shouldReEncrypt` is reported. Signed-off-by: Kevin Cui --- .github/workflows/build.yml | 2 +- .github/workflows/release.yml | 3 -- .github/workflows/test.yml | 2 +- package.json | 2 +- src/main/github.ts | 18 +++++++----- tests/main/github.spec.ts | 54 +++++++++++++++++++++++------------ tests/mocks/electron.ts | 15 +++++++--- yarn.lock | 10 +++---- 8 files changed, 65 insertions(+), 41 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7ce23d4f3a..3232dd2b44 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -20,7 +20,7 @@ jobs: - os: macos-latest arch: arm64 - os: ubuntu-24.04-arm - arch: armv7l + arch: arm64 runs-on: '${{ matrix.os }}' steps: - run: git config --global core.autocrlf input diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 33d5e5bec0..5744a2ae28 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -24,8 +24,6 @@ jobs: include: - os: macos-latest-xlarge arch: arm64 - - os: ubuntu-24.04-arm - arch: armv7l - os: ubuntu-24.04-arm arch: arm64 runs-on: "${{ matrix.os }}" @@ -85,7 +83,6 @@ jobs: matrix: arch: - x64 - - ia32 runs-on: windows-latest permissions: actions: write diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1a2ef46557..87705fb4c3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,7 +20,7 @@ jobs: - os: macos-latest arch: arm64 - os: ubuntu-24.04-arm - arch: armv7l + arch: arm64 runs-on: '${{ matrix.os }}' steps: - run: git config --global core.autocrlf input diff --git a/package.json b/package.json index b074b76b2f..519defaf1a 100644 --- a/package.json +++ b/package.json @@ -101,7 +101,7 @@ "@vitest/coverage-v8": "5.0.1", "copy-webpack-plugin": "^14.0.0", "css-loader": "^6.7.1", - "electron": "^42.11.3", + "electron": "^45.0.0", "electron-devtools-installer": "^4.0.0", "fork-ts-checker-webpack-plugin": "^8.0.0", "husky": "^9.0.11", diff --git a/src/main/github.ts b/src/main/github.ts index 244faf6648..16e7ca2bf9 100644 --- a/src/main/github.ts +++ b/src/main/github.ts @@ -90,16 +90,20 @@ function getCredentialsPath(): string { return pathJoin(app.getPath('userData'), CREDENTIALS_FILE); } -function saveToken(token: string): void { - const encrypted = safeStorage.encryptString(token); +async function saveToken(token: string): Promise { + const encrypted = await safeStorage.encryptStringAsync(token); fs.writeFileSync(getCredentialsPath(), encrypted, { mode: 0o600 }); } -function loadToken(): string | null { +async function loadToken(): Promise { const credPath = getCredentialsPath(); try { const encrypted = fs.readFileSync(credPath); - return safeStorage.decryptString(encrypted); + const { result, shouldReEncrypt } = + await safeStorage.decryptStringAsync(encrypted); + // The encryption key was rotated or upgraded, so store it with the new key + if (shouldReEncrypt) await saveToken(result); + return result; } catch { return null; } @@ -134,7 +138,7 @@ async function handleTokenSignIn( if (!isValidToken(token)) return { success: false, error: 'Invalid token format.' }; - if (!safeStorage.isEncryptionAvailable()) { + if (!(await safeStorage.isAsyncEncryptionAvailable())) { return { success: false, error: @@ -154,7 +158,7 @@ async function handleTokenSignIn( 'Token is missing the "gist" scope. Please generate a new token with gist permissions.', }; - saveToken(token); + await saveToken(token); octokit_ = testOctokit; return { success: true, login: response.data.login }; @@ -175,7 +179,7 @@ async function handleTokenSignOut(_event: IpcMainInvokeEvent): Promise { async function handleTokenCheckAuth( _event: IpcMainInvokeEvent, ): Promise { - const token = loadToken(); + const token = await loadToken(); if (!token) return { login: null, hasToken: false }; try { diff --git a/tests/main/github.spec.ts b/tests/main/github.spec.ts index 97cc87aac9..0525b35ca8 100644 --- a/tests/main/github.spec.ts +++ b/tests/main/github.spec.ts @@ -123,7 +123,7 @@ describe('github', () => { app.setPath('userData', userDataPath); // Confirm that the folder we just created will hold the credentials file expect(getCredentialsPath().startsWith(userDataPath)).toBe(true); - expect(loadToken()).toBeNull(); + await expect(loadToken()).resolves.toBeNull(); }); afterEach(async () => { @@ -134,14 +134,14 @@ describe('github', () => { describe('handleTokenSignIn()', () => { it('saves encrypted tokens to a permission-protected userData file', async () => { mockOctokitInstance(); - expect(loadToken()).toBeNull(); + await expect(loadToken()).resolves.toBeNull(); for (const token of [VALID_GHP_TOKEN, VALID_PAT_TOKEN]) { const result = await handleTokenSignIn(MOCK_EVENT, token); expect(result).toEqual({ success: true, login: MOCK_LOGIN }); - const encrypted = safeStorage.encryptString(token); - expect(loadToken()).toBe(token); + const encrypted = await safeStorage.encryptStringAsync(token); + await expect(loadToken()).resolves.toBe(token); expect(fs.readFileSync(getCredentialsPath())).toEqual(encrypted); // POSIX permission bits aren't meaningful on Windows if (process.platform !== 'win32') @@ -172,7 +172,9 @@ describe('github', () => { }); it('rejects when encryption is unavailable', async () => { - vi.mocked(safeStorage.isEncryptionAvailable).mockReturnValue(false); + vi.mocked(safeStorage.isAsyncEncryptionAvailable).mockResolvedValue( + false, + ); const result = await handleTokenSignIn(MOCK_EVENT, VALID_GHP_TOKEN); expect(result.success).toBe(false); expect(result.error).toContain('Encryption is not available'); @@ -211,25 +213,39 @@ describe('github', () => { describe('handleTokenSignOut()', () => { it('deletes the stored token', async () => { // setup: set a token & confirm it loads - saveToken(VALID_GHP_TOKEN); - expect(loadToken()).toBe(VALID_GHP_TOKEN); + await saveToken(VALID_GHP_TOKEN); + await expect(loadToken()).resolves.toBe(VALID_GHP_TOKEN); await expect(handleTokenSignOut(MOCK_EVENT)).resolves.toBeUndefined(); - expect(loadToken()).toBeNull(); + await expect(loadToken()).resolves.toBeNull(); }); }); describe('handleTokenCheckAuth()', () => { it('returns null when decryption fails', async () => { - saveToken(VALID_GHP_TOKEN); - vi.mocked(safeStorage.decryptString).mockImplementationOnce(() => { - throw new Error('corrupt'); - }); + await saveToken(VALID_GHP_TOKEN); + vi.mocked(safeStorage.decryptStringAsync).mockRejectedValueOnce( + new Error('corrupt'), + ); const result = await handleTokenCheckAuth(MOCK_EVENT); expect(result).toEqual({ login: null, hasToken: false }); }); + it('re-encrypts the stored token when the key has changed', async () => { + fs.writeFileSync(getCredentialsPath(), 'stale-encrypted-token'); + vi.mocked(safeStorage.decryptStringAsync).mockResolvedValueOnce({ + shouldReEncrypt: true, + result: VALID_GHP_TOKEN, + }); + mockOctokitInstance(); + const result = await handleTokenCheckAuth(MOCK_EVENT); + expect(result).toEqual({ login: MOCK_LOGIN, hasToken: true }); + expect(fs.readFileSync(getCredentialsPath())).toEqual( + await safeStorage.encryptStringAsync(VALID_GHP_TOKEN), + ); + }); + it('returns login when a valid token is stored', async () => { - saveToken(VALID_GHP_TOKEN); + await saveToken(VALID_GHP_TOKEN); mockOctokitInstance(); const result = await handleTokenCheckAuth(MOCK_EVENT); expect(result).toEqual({ login: MOCK_LOGIN, hasToken: true }); @@ -237,14 +253,14 @@ describe('github', () => { it('returns null when no token is stored', async () => { // setup: confirm there's no token - expect(loadToken()).toBeNull(); + await expect(loadToken()).resolves.toBeNull(); const result = await handleTokenCheckAuth(MOCK_EVENT); expect(result).toEqual({ login: null, hasToken: false }); }); it('cleans up and returns null for expired tokens', async () => { - saveToken(VALID_GHP_TOKEN); + await saveToken(VALID_GHP_TOKEN); mockOctokitInstance({ users: { getAuthenticated: vi @@ -257,11 +273,11 @@ describe('github', () => { const result = await handleTokenCheckAuth(MOCK_EVENT); expect(result).toEqual({ login: null, hasToken: false }); - expect(loadToken()).toBeNull(); + await expect(loadToken()).resolves.toBeNull(); }); it('preserves the token and reports hasToken for transient failures', async () => { - saveToken(VALID_GHP_TOKEN); + await saveToken(VALID_GHP_TOKEN); mockOctokitInstance({ users: { getAuthenticated: vi.fn().mockRejectedValue(new Error('offline')), @@ -271,11 +287,11 @@ describe('github', () => { const result = await handleTokenCheckAuth(MOCK_EVENT); expect(result).toEqual({ login: null, hasToken: true }); - expect(loadToken()).toBe(VALID_GHP_TOKEN); + await expect(loadToken()).resolves.toBe(VALID_GHP_TOKEN); }); it('keeps octokit_ usable after a transient failure', async () => { - saveToken(VALID_GHP_TOKEN); + await saveToken(VALID_GHP_TOKEN); mockOctokitInstance({ users: { getAuthenticated: vi.fn().mockRejectedValue(new Error('offline')), diff --git a/tests/mocks/electron.ts b/tests/mocks/electron.ts index 559181d211..0f943f5d2d 100644 --- a/tests/mocks/electron.ts +++ b/tests/mocks/electron.ts @@ -189,11 +189,18 @@ const systemPreferences = { }; const safeStorage = { - isEncryptionAvailable: vi.fn(() => true), - encryptString: vi.fn((text: string) => Buffer.from(`encrypted:${text}`)), - decryptString: vi.fn((buffer: Buffer) => { + isAsyncEncryptionAvailable: vi.fn(async () => true), + encryptStringAsync: vi.fn(async (text: string) => + Buffer.from(`encrypted:${text}`), + ), + decryptStringAsync: vi.fn(async (buffer: Buffer) => { const str = buffer.toString(); - return str.startsWith('encrypted:') ? str.slice('encrypted:'.length) : str; + return { + shouldReEncrypt: false, + result: str.startsWith('encrypted:') + ? str.slice('encrypted:'.length) + : str, + }; }), }; diff --git a/yarn.lock b/yarn.lock index 306a127488..dea4343f49 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4938,7 +4938,7 @@ __metadata: classnames: "npm:^2.2.6" copy-webpack-plugin: "npm:^14.0.0" css-loader: "npm:^6.7.1" - electron: "npm:^42.11.3" + electron: "npm:^45.0.0" electron-devtools-installer: "npm:^4.0.0" electron-squirrel-startup: "npm:^1.0.0" fork-ts-checker-webpack-plugin: "npm:^8.0.0" @@ -5108,9 +5108,9 @@ __metadata: languageName: node linkType: hard -"electron@npm:^42.11.3": - version: 42.11.3 - resolution: "electron@npm:42.11.3" +"electron@npm:^45.0.0": + version: 45.0.0 + resolution: "electron@npm:45.0.0" dependencies: "@electron-internal/extract-zip": "npm:^1.0.1" "@electron/get": "npm:^5.0.0" @@ -5118,7 +5118,7 @@ __metadata: bin: electron: cli.js install-electron: install.js - checksum: 10c0/42ec2174199864f193e0bf74f7a35e040f51e2749482c6e1a6d0277deff19c423175063496938dcb8ab3e00b4e084105a4788dcb40ff863169c3b6f1f319404d + checksum: 10c0/53dc878a72815e89aadccb125d137d1e2dbeabbe9b8c0f2c1a49c114e85c4635aa32c83af6074f11e6ccbe433069c6c57299a5b473a63322e7957da1348c7d8d languageName: node linkType: hard From 29419c4e3b1b5604a7041112973abbcf1686c1d5 Mon Sep 17 00:00:00 2001 From: Kevin Cui Date: Sat, 24 Oct 2026 02:53:04 -0400 Subject: [PATCH 2/3] test: await hoisted factory in devtools spec The `vi.hoisted()` factory is async but was never awaited, so its `import('../utils.js')` raced with the test module's own imports. Under the load of a full parallel run the devmode automock was sometimes not registered in time, `isDevMode` resolved to the real function, and all three tests failed with `mockReturnValue is not a function`. This flaked on Windows and Linux arm64 CI. Awaiting the factory at the top level makes the hoisted import finish before the module imports are evaluated. Signed-off-by: Kevin Cui --- tests/main/devtools.spec.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/main/devtools.spec.ts b/tests/main/devtools.spec.ts index cd0d3aff27..e0baed4f10 100644 --- a/tests/main/devtools.spec.ts +++ b/tests/main/devtools.spec.ts @@ -1,14 +1,16 @@ /** * @vitest-environment node */ -import { type Mock, beforeAll, describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { setupDevTools } from '../../src/main/devtools'; import { isDevMode } from '../../src/main/utils/devmode'; vi.mock('../../src/main/utils/devmode'); -const installExtensionPromise = vi.hoisted(async () => { +// Await the hoisted factory so its dynamic import finishes before the module +// imports below are evaluated, otherwise the devmode mock can lose the race. +const installExtension = await vi.hoisted(async () => { const { mockRequire } = await import('../utils.js'); const installExtension = vi.fn(); @@ -22,12 +24,6 @@ const installExtensionPromise = vi.hoisted(async () => { }); describe('devtools', () => { - let installExtension: Mock; - - beforeAll(async () => { - installExtension = await installExtensionPromise; - }); - it('does not set up developer tools if not in dev mode', async () => { vi.mocked(isDevMode).mockReturnValue(false); await setupDevTools(); From 6b3aa6da66f34f8fc13b5a36d6b40a5f8e50cb3a Mon Sep 17 00:00:00 2001 From: Kevin Cui Date: Sat, 24 Oct 2026 02:56:36 -0400 Subject: [PATCH 3/3] fix(github): guard auth state across async token decryption Moving to the async `safeStorage` API added an await between reading the stored token and assigning `octokit_`. A sign-out during the startup auth check (for example while a keychain prompt is open) was then undone when decryption resolved, and with `shouldReEncrypt` the deleted credentials file was written back. Sign-in and sign-out now bump `authGeneration_`, and a check that sees it change reports a result the renderer ignores and skips the re-encrypt write. Re-encrypting after a key rotation is now best-effort, so a failed write no longer turns a valid token into a signed-out state. Signed-off-by: Kevin Cui --- src/main/github.ts | 44 +++++++++++++++++++++++++++++++-------- tests/main/github.spec.ts | 41 ++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 9 deletions(-) diff --git a/src/main/github.ts b/src/main/github.ts index 16e7ca2bf9..355d202586 100644 --- a/src/main/github.ts +++ b/src/main/github.ts @@ -90,23 +90,41 @@ function getCredentialsPath(): string { return pathJoin(app.getPath('userData'), CREDENTIALS_FILE); } -async function saveToken(token: string): Promise { - const encrypted = await safeStorage.encryptStringAsync(token); +// Bumped on every sign-in and sign-out, so work that awaited the async +// safeStorage API can tell that the stored credentials changed meanwhile. +let authGeneration_ = 0; + +function writeCredentials(encrypted: Buffer): void { fs.writeFileSync(getCredentialsPath(), encrypted, { mode: 0o600 }); } +async function saveToken(token: string): Promise { + writeCredentials(await safeStorage.encryptStringAsync(token)); +} + async function loadToken(): Promise { - const credPath = getCredentialsPath(); + const generation = authGeneration_; + let decrypted: Electron.DecryptStringAsyncReturnValue; try { - const encrypted = fs.readFileSync(credPath); - const { result, shouldReEncrypt } = - await safeStorage.decryptStringAsync(encrypted); - // The encryption key was rotated or upgraded, so store it with the new key - if (shouldReEncrypt) await saveToken(result); - return result; + const encrypted = fs.readFileSync(getCredentialsPath()); + decrypted = await safeStorage.decryptStringAsync(encrypted); } catch { return null; } + + // The encryption key was rotated or upgraded, so store the token with the + // new key. This is best-effort, since the decrypted token is still valid, + // and it must not recreate credentials that a sign-out has just deleted. + if (decrypted.shouldReEncrypt) { + try { + const encrypted = await safeStorage.encryptStringAsync(decrypted.result); + if (generation === authGeneration_) writeCredentials(encrypted); + } catch (error) { + console.warn('Failed to re-encrypt the stored GitHub token', error); + } + } + + return decrypted.result; } function deleteToken(): void { @@ -158,6 +176,7 @@ async function handleTokenSignIn( 'Token is missing the "gist" scope. Please generate a new token with gist permissions.', }; + authGeneration_++; await saveToken(token); octokit_ = testOctokit; @@ -172,6 +191,7 @@ async function handleTokenSignIn( } async function handleTokenSignOut(_event: IpcMainInvokeEvent): Promise { + authGeneration_++; deleteToken(); octokit_ = null; } @@ -179,7 +199,13 @@ async function handleTokenSignOut(_event: IpcMainInvokeEvent): Promise { async function handleTokenCheckAuth( _event: IpcMainInvokeEvent, ): Promise { + const generation = authGeneration_; const token = await loadToken(); + + // A sign-in or sign-out finished while the token was being decrypted, so + // this result is stale. Report it like an offline check, which the renderer + // ignores, instead of overwriting the newer auth state. + if (generation !== authGeneration_) return { login: null, hasToken: true }; if (!token) return { login: null, hasToken: false }; try { diff --git a/tests/main/github.spec.ts b/tests/main/github.spec.ts index 0525b35ca8..f9fd8c7378 100644 --- a/tests/main/github.spec.ts +++ b/tests/main/github.spec.ts @@ -244,6 +244,47 @@ describe('github', () => { ); }); + it('keeps a valid token when re-encrypting it fails', async () => { + await saveToken(VALID_GHP_TOKEN); + vi.mocked(safeStorage.decryptStringAsync).mockResolvedValueOnce({ + shouldReEncrypt: true, + result: VALID_GHP_TOKEN, + }); + vi.mocked(safeStorage.encryptStringAsync).mockRejectedValueOnce( + new Error('keychain locked'), + ); + mockOctokitInstance(); + const result = await handleTokenCheckAuth(MOCK_EVENT); + expect(result).toEqual({ login: MOCK_LOGIN, hasToken: true }); + }); + + it('does not restore auth when signing out during decryption', async () => { + await saveToken(VALID_GHP_TOKEN); + mockOctokitInstance(); + let resolveDecrypt!: ( + value: Electron.DecryptStringAsyncReturnValue, + ) => void; + vi.mocked(safeStorage.decryptStringAsync).mockReturnValueOnce( + new Promise((resolve) => { + resolveDecrypt = resolve; + }), + ); + + const check = handleTokenCheckAuth(MOCK_EVENT); + await handleTokenSignOut(MOCK_EVENT); + resolveDecrypt({ shouldReEncrypt: true, result: VALID_GHP_TOKEN }); + + await expect(check).resolves.toEqual({ login: null, hasToken: true }); + expect(fs.existsSync(getCredentialsPath())).toBe(false); + await expect( + handleGistCreate(MOCK_EVENT, { + description: 'Test', + files: VALID_FILES, + isPublic: false, + }), + ).rejects.toThrow('Not authenticated'); + }); + it('returns login when a valid token is stored', async () => { await saveToken(VALID_GHP_TOKEN); mockOctokitInstance();