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..355d202586 100644 --- a/src/main/github.ts +++ b/src/main/github.ts @@ -90,19 +90,41 @@ function getCredentialsPath(): string { return pathJoin(app.getPath('userData'), CREDENTIALS_FILE); } -function saveToken(token: string): void { - const encrypted = safeStorage.encryptString(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 }); } -function loadToken(): string | null { - const credPath = getCredentialsPath(); +async function saveToken(token: string): Promise { + writeCredentials(await safeStorage.encryptStringAsync(token)); +} + +async function loadToken(): Promise { + const generation = authGeneration_; + let decrypted: Electron.DecryptStringAsyncReturnValue; try { - const encrypted = fs.readFileSync(credPath); - return safeStorage.decryptString(encrypted); + 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 { @@ -134,7 +156,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 +176,8 @@ async function handleTokenSignIn( 'Token is missing the "gist" scope. Please generate a new token with gist permissions.', }; - saveToken(token); + authGeneration_++; + await saveToken(token); octokit_ = testOctokit; return { success: true, login: response.data.login }; @@ -168,6 +191,7 @@ async function handleTokenSignIn( } async function handleTokenSignOut(_event: IpcMainInvokeEvent): Promise { + authGeneration_++; deleteToken(); octokit_ = null; } @@ -175,7 +199,13 @@ async function handleTokenSignOut(_event: IpcMainInvokeEvent): Promise { async function handleTokenCheckAuth( _event: IpcMainInvokeEvent, ): Promise { - const token = loadToken(); + 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/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(); diff --git a/tests/main/github.spec.ts b/tests/main/github.spec.ts index 97cc87aac9..f9fd8c7378 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,80 @@ 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('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 () => { - 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 +294,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 +314,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 +328,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