From 43df43d0d53f5428ebf48c34fc996e3cb2825fa3 Mon Sep 17 00:00:00 2001 From: "buf0-bot[bot]" <252831055+buf0-bot[bot]@users.noreply.github.com> Date: Wed, 10 Jun 2026 23:56:17 -0700 Subject: [PATCH] fix: address pr-bug-scan validated finding from #4683 (#5151) Isolated CredentialDecryptionError per-item in Linear getClients (client.ts:518) and Jira getClients (client.ts:373) on the 'all' selection so one bad credential no longer collapses healthy workspaces Co-authored-by: orca-bug-scan-bot --- src/main/jira/client.test.ts | 72 +++++++++++++++++++++++++++++ src/main/jira/client.ts | 23 ++++++++-- src/main/linear/client.test.ts | 83 ++++++++++++++++++++++++++++++++++ src/main/linear/client.ts | 23 ++++++++-- 4 files changed, 191 insertions(+), 10 deletions(-) diff --git a/src/main/jira/client.test.ts b/src/main/jira/client.test.ts index 00b2087e342..e211f94dc2b 100644 --- a/src/main/jira/client.test.ts +++ b/src/main/jira/client.test.ts @@ -50,6 +50,37 @@ function writeJiraFiles(siteId: string, token: string | Buffer): void { writeFileSync(tokenPathForSite(siteId), token) } +function writeMultiSiteFiles( + sites: { id: string; token: string | Buffer }[], + selectedSiteId: string +): void { + const orcaDir = join(tempHome, '.orca') + mkdirSync(join(orcaDir, 'jira-tokens'), { recursive: true }) + writeFileSync( + join(orcaDir, 'jira-sites.json'), + JSON.stringify( + { + version: 1, + activeSiteId: sites[0]?.id ?? null, + selectedSiteId, + sites: sites.map((site) => ({ + id: site.id, + siteUrl: `https://${site.id}.atlassian.net`, + email: `${site.id}@example.com`, + displayName: site.id, + accountId: `account-${site.id}` + })) + }, + null, + 2 + ), + { encoding: 'utf-8' } + ) + for (const site of sites) { + writeFileSync(tokenPathForSite(site.id), site.token) + } +} + async function loadClientModule(options: SafeStorageMockOptions = {}) { vi.resetModules() vi.doMock('electron', () => ({ @@ -237,4 +268,45 @@ describe('Jira client credential storage', () => { expect(fetchMock).not.toHaveBeenCalled() expect(jira.getStatus()).toMatchObject({ connected: false }) }) + + it('keeps healthy sites under the "all" selection when one site cannot be decrypted', async () => { + writeMultiSiteFiles( + [ + { id: 'good', token: 'token-good' }, + { id: 'bad', token: Buffer.from([0x76, 0x31, 0x30, 0xff, 0xfe]) } + ], + 'all' + ) + const jira = await loadClientModule({ + encryptionAvailable: true, + // Why: only the binary "bad" token throws on decrypt; the plaintext + // "good" token falls back through the legacy path. + decryptString: () => { + throw new Error('userCanceledErr') + } + }) + + const clients = jira.getClients('all') + expect(clients.map((client) => client.site.id)).toEqual(['good']) + // The bad site's decrypt error is still recorded for the status banner. + expect(jira.getStatus().credentialError).toContain('Could not decrypt') + }) + + it('rethrows the decrypt error for a specific site selection', async () => { + writeMultiSiteFiles( + [ + { id: 'good', token: 'token-good' }, + { id: 'bad', token: Buffer.from([0x76, 0x31, 0x30, 0xff, 0xfe]) } + ], + 'bad' + ) + const jira = await loadClientModule({ + encryptionAvailable: true, + decryptString: () => { + throw new Error('userCanceledErr') + } + }) + + expect(() => jira.getClients('bad')).toThrow('Could not decrypt') + }) }) diff --git a/src/main/jira/client.ts b/src/main/jira/client.ts index 468c5d93636..a29c8a807d6 100644 --- a/src/main/jira/client.ts +++ b/src/main/jira/client.ts @@ -373,13 +373,26 @@ export async function jiraRequest( export function getClients(selection?: JiraSiteSelection | null): JiraClientForSite[] { const file = getSiteFile() const selected = selection ?? file.selectedSiteId ?? file.activeSiteId - const sites = - selected === 'all' - ? file.sites - : file.sites.filter((site) => site.id === (selected ?? file.activeSiteId)) + const isAllSelection = selected === 'all' + const sites = isAllSelection + ? file.sites + : file.sites.filter((site) => site.id === (selected ?? file.activeSiteId)) return sites.flatMap((site) => { - const token = readToken(site.id) + let token: string | null + try { + token = readToken(site.id) + } catch (error) { + // Why: under an 'all' selection one un-decryptable site must not collapse + // reads for the healthy ones. readToken already recorded the per-site + // credentialError for getStatus to surface, so skip this site like a + // missing token. A specific-site selection still rethrows so the renderer + // can surface the decrypt banner promptly. + if (isAllSelection && error instanceof CredentialDecryptionError) { + return [] + } + throw error + } return token ? [{ site, authorization: authHeader(site.email, token) }] : [] }) } diff --git a/src/main/linear/client.test.ts b/src/main/linear/client.test.ts index d566c087c0b..bead383dcd2 100644 --- a/src/main/linear/client.test.ts +++ b/src/main/linear/client.test.ts @@ -34,6 +34,42 @@ function writeLegacyLinearToken(token: string | Buffer, viewer: Record ({ + id: workspace.id, + organizationId: workspace.id, + organizationName: workspace.id, + displayName: 'Ada', + email: 'ada@example.com' + })) + }), + { encoding: 'utf-8' } + ) + for (const workspace of workspaces) { + writeFileSync(workspaceTokenPath(workspace.id), workspace.token) + } +} + async function loadClientModule(options: SafeStorageMockOptions = {}) { vi.resetModules() linearClientMock = vi.fn(function LinearClient( @@ -302,4 +338,51 @@ describe('Linear client workspace storage', () => { expect(linearClientMock).not.toHaveBeenCalled() expect(linear.getStatus()).toMatchObject({ connected: false }) }) + + it('keeps healthy workspaces under the "all" selection when one cannot be decrypted', async () => { + writeMultiWorkspaceFiles( + [ + { id: 'good', token: 'token-good' }, + { id: 'bad', token: Buffer.from([0x76, 0x31, 0x30, 0xff, 0xfe]) } + ], + 'all' + ) + fixtures.set('token-good', { + displayName: 'Ada', + email: 'ada@example.com', + organizationId: 'good', + organizationName: 'good', + organizationUrlKey: 'good' + }) + const linear = await loadClientModule({ + encryptionAvailable: true, + // Why: the plaintext "token-good" falls back through the legacy path; + // the binary "bad" token throws CredentialDecryptionError. + decryptString: () => { + throw new Error('userCanceledErr') + } + }) + + const clients = linear.getClients('all') + expect(clients.map((client) => client.workspace.id)).toEqual(['good']) + expect(linear.getStatus().credentialError).toContain('Could not decrypt') + }) + + it('rethrows the decrypt error for a specific workspace selection', async () => { + writeMultiWorkspaceFiles( + [ + { id: 'good', token: 'token-good' }, + { id: 'bad', token: Buffer.from([0x76, 0x31, 0x30, 0xff, 0xfe]) } + ], + 'bad' + ) + const linear = await loadClientModule({ + encryptionAvailable: true, + decryptString: () => { + throw new Error('userCanceledErr') + } + }) + + expect(() => linear.getClients('bad')).toThrow('Could not decrypt') + }) }) diff --git a/src/main/linear/client.ts b/src/main/linear/client.ts index 56d8a9a5ac4..8e8bc17e7d8 100644 --- a/src/main/linear/client.ts +++ b/src/main/linear/client.ts @@ -519,14 +519,27 @@ export function getClients( workspaceId?: LinearWorkspaceSelection | null ): LinearClientForWorkspace[] { const state = getWorkspaceState() - const selectedWorkspaces = - workspaceId === 'all' - ? state.workspaces - : state.workspaces.filter((workspace) => workspace.id === resolveWorkspaceId(workspaceId)) + const isAllSelection = workspaceId === 'all' + const selectedWorkspaces = isAllSelection + ? state.workspaces + : state.workspaces.filter((workspace) => workspace.id === resolveWorkspaceId(workspaceId)) const clients: LinearClientForWorkspace[] = [] for (const workspace of selectedWorkspaces) { - const token = loadToken({ force: true, workspaceId: workspace.id }) + let token: string | null + try { + token = loadToken({ force: true, workspaceId: workspace.id }) + } catch (error) { + // Why: under an 'all' selection one un-decryptable workspace must not + // collapse reads for the healthy ones. loadToken already recorded the + // per-workspace credentialError for getStatus to surface, so skip this + // workspace like a missing token. A specific-workspace selection still + // rethrows so the renderer can surface the decrypt banner promptly. + if (isAllSelection && error instanceof CredentialDecryptionError) { + continue + } + throw error + } if (!token) { continue }