From 95d3b212bc6252c2d2bc083389b4cb58602b0024 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Tue, 14 Jul 2026 21:02:27 +0200 Subject: [PATCH] fix(electron): gate Jira IPC behind a one-shot capability (#9008) * docs(plugins): add microsoft 365 calendar provider plan * fix(jira): gate Electron requests behind one-shot capability Claim privileged Jira IPC before plugin startup and return responses through invoke instead of a broadcast event. Keep arbitrary HTTP(S) Jira hosts supported while rejecting redirects and bounding request resources. * fix(jira): enforce Electron request capability Bind privileged Jira IPC to a main-issued renderer-document token and strip raw Electron events from renderer callbacks. Scope image authentication by origin, base path, and resource type while preserving safe redirects and legacy configurations. * fix(electron): handle payload-only IPC lifecycle Clear Jira image authentication before replacement and when a new renderer document claims the capability. Parse before-close IDs from payload-only events so pending sync and finish-day hooks can complete. * fix(electron): address Jira IPC capability review findings - electron.effects: read ANY_FILE_DOWNLOADED payload at [0] after the payload-only IPC refactor (was [1], now undefined -> TypeError on every download); guard against a malformed payload - jira-capability: rotate the token on re-register so a renderer reload that reuses the WebFrameMain object is not permanently locked out of Jira; invalidates any stale token - document that the one-shot consumption order, not the bypassable main-frame check, is the real capability boundary - jira-electron-bridge: skip the no-op clearImgHeaders IPC round-trip when image auth was never set up (non-Jira detail-panel open/close) - jira-api: route a synchronous _toElectronRequestInit throw through _handleResponse instead of leaking a dangling request-log entry * test(electron): cover ANY_FILE_DOWNLOADED payload parsing Extract parseDownloadedFilePayload from ElectronEffects and add a regression spec pinning the payload-only shape ([file], not [event, file]) that caused a TypeError on every download. Hardened against non-array input surfaced by the new test. * fix(electron): restore Node ambient globals for frontend build --- electron/electronAPI.d.ts | 28 +- electron/ipc-handlers/jira.ts | 39 +- electron/jira-capability.test.cjs | 57 +++ electron/jira-capability.ts | 44 ++ electron/jira-image-auth.ts | 100 +++++ electron/jira-ipc.test.cjs | 107 +++++ electron/jira.test.cjs | 420 ++++++++++++++++++ electron/jira.ts | 327 +++++++++----- electron/main-window.ts | 2 + electron/preload-api-capability.test.cjs | 94 ++++ electron/preload.ts | 26 +- .../shared-with-frontend/ipc-events.const.ts | 3 +- .../jira-request.model.ts | 44 ++ .../one-shot-api-consumer.ts | 16 + electron/shared-with-frontend/preload-api.ts | 52 +++ .../core/electron/electron.effects.spec.ts | 32 ++ src/app/core/electron/electron.effects.ts | 32 +- .../electron/exec-before-close.service.ts | 5 +- src/app/core/ipc-events.spec.ts | 129 +----- src/app/core/ipc-events.ts | 38 +- src/app/core/startup/startup.service.spec.ts | 14 + src/app/core/startup/startup.service.ts | 13 +- .../issue/providers/jira/jira-api.service.ts | 71 ++- .../jira/jira-electron-bridge.service.ts | 53 +++ .../task-detail-panel.component.ts | 24 +- .../sync/oauth-callback-handler.service.ts | 2 +- 26 files changed, 1459 insertions(+), 313 deletions(-) create mode 100644 electron/jira-capability.test.cjs create mode 100644 electron/jira-capability.ts create mode 100644 electron/jira-image-auth.ts create mode 100644 electron/jira-ipc.test.cjs create mode 100644 electron/jira.test.cjs create mode 100644 electron/preload-api-capability.test.cjs create mode 100644 electron/shared-with-frontend/jira-request.model.ts create mode 100644 electron/shared-with-frontend/one-shot-api-consumer.ts create mode 100644 electron/shared-with-frontend/preload-api.ts create mode 100644 src/app/core/electron/electron.effects.spec.ts create mode 100644 src/app/features/issue/providers/jira/jira-electron-bridge.service.ts diff --git a/electron/electronAPI.d.ts b/electron/electronAPI.d.ts index 20b08f8e5f..523eedf763 100644 --- a/electron/electronAPI.d.ts +++ b/electron/electronAPI.d.ts @@ -1,11 +1,17 @@ -import { IpcRendererEvent } from 'electron'; +// This file is pulled into the frontend TS program (via src/app/core/window-ea.d.ts). +// That program sets `types: []`, so it has no ambient Node globals of its own and +// relied on the now-removed `import { IpcRendererEvent } from 'electron'` here to +// transitively supply them. Several frontend/shared modules still probe Node globals +// guarded at runtime (get-dist-channel's `process`/`NodeJS`, generate-client-id's +// `process`, create-task-placeholder's `NodeJS.Timeout`, user-profile's `require`), +// so re-expose them explicitly instead of by accident. +/// import { GlobalConfigState, TakeABreakConfig, TaskWidgetConfig, } from '../src/app/features/config/global-config.model'; import { KeyboardConfig } from './shared-with-frontend/keyboard-config.model'; -import { JiraCfg } from '../src/app/features/issue/providers/jira/jira.model'; import { AppDataCompleteLegacy } from '../src/app/imex/sync/sync.model'; import { Task } from '../src/app/features/tasks/task.model'; import { LocalBackupMeta } from '../src/app/imex/local-backup/local-backup.model'; @@ -16,12 +22,10 @@ import { LocalRestApiResponsePayload, } from './shared-with-frontend/local-rest-api.model'; import { ElectronDistChannel } from './shared-with-frontend/get-dist-channel'; +import { JiraElectronApi } from './shared-with-frontend/jira-request.model'; export interface ElectronAPI { - on( - channel: string, - listener: (event: IpcRendererEvent, ...args: unknown[]) => void, - ): void; + on(channel: string, listener: (...args: unknown[]) => void): void; // SYNC // ---- @@ -220,16 +224,6 @@ export interface ElectronAPI { showFullScreenBlocker(args: { msg?: string; takeABreakCfg: TakeABreakConfig }): void; - // TODO use invoke instead - makeJiraRequest(args: { - requestId: string; - url: string; - requestInit: RequestInit; - jiraCfg: JiraCfg; - }): void; - - jiraSetupImgHeaders(args: { jiraCfg: JiraCfg }): void; - backupAppData(args: { data: AppDataCompleteLegacy | AppDataComplete; maxBackupFiles?: number | null; @@ -250,6 +244,8 @@ export interface ElectronAPI { onSwitchTask(listener: (taskId: string) => void): void; + consumeJiraApi(): JiraElectronApi | null; + consumePluginNodeExecutionApi(): PluginNodeExecutionElectronApi | null; // Plugin OAuth diff --git a/electron/ipc-handlers/jira.ts b/electron/ipc-handlers/jira.ts index ac8bb527a6..f5663f1ee3 100644 --- a/electron/ipc-handlers/jira.ts +++ b/electron/ipc-handlers/jira.ts @@ -1,14 +1,41 @@ import { ipcMain } from 'electron'; import { IPC } from '../shared-with-frontend/ipc-events.const'; -import { JiraCfg } from '../../src/app/features/issue/providers/jira/jira.model'; -import { sendJiraRequest, setupRequestHeadersForImages } from '../jira'; +import { executeJiraRequest } from '../jira'; +import { JiraCapabilityRegistry } from '../jira-capability'; +import { + clearRequestHeadersForImages, + setupRequestHeadersForImages, +} from '../jira-image-auth'; + +const capabilityRegistry = new JiraCapabilityRegistry(); export const initJiraIpc = (): void => { - ipcMain.on(IPC.JIRA_SETUP_IMG_HEADERS, (ev, { jiraCfg }: { jiraCfg: JiraCfg }) => { - setupRequestHeadersForImages(jiraCfg); + ipcMain.handle(IPC.JIRA_REGISTER_CAPABILITY, (event) => { + // NOTE: this main-frame check is a secondary guard, not the real boundary: + // same-origin plugin iframes can reach window.top.ea, so their IPC also + // arrives with senderFrame === mainFrame. The actual protection is that + // trusted startup code consumes the one-shot capability before any plugin + // code runs (see JiraElectronBridgeService.initialize / StartupService). + if (event.senderFrame !== event.sender.mainFrame) { + return null; + } + const token = capabilityRegistry.register(event.senderFrame); + // A fresh document just claimed the capability — drop any stale image auth + // left over from the previous renderer document. + clearRequestHeadersForImages(); + return token; }); - ipcMain.on(IPC.JIRA_MAKE_REQUEST_EVENT, (ev, request) => { - sendJiraRequest(request); + ipcMain.handle(IPC.JIRA_SETUP_IMG_HEADERS, (event, envelope: unknown) => { + setupRequestHeadersForImages(capabilityRegistry.unwrap(event.senderFrame, envelope)); }); + + ipcMain.handle(IPC.JIRA_CLEAR_IMG_HEADERS, (event, envelope: unknown) => { + capabilityRegistry.unwrap(event.senderFrame, envelope); + clearRequestHeadersForImages(); + }); + + ipcMain.handle(IPC.JIRA_MAKE_REQUEST_EVENT, (event, envelope: unknown) => + executeJiraRequest(capabilityRegistry.unwrap(event.senderFrame, envelope)), + ); }; diff --git a/electron/jira-capability.test.cjs b/electron/jira-capability.test.cjs new file mode 100644 index 0000000000..64334f9d7f --- /dev/null +++ b/electron/jira-capability.test.cjs @@ -0,0 +1,57 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); + +require('ts-node/register/transpile-only'); + +const { JiraCapabilityRegistry } = require( + path.resolve(__dirname, 'jira-capability.ts'), +); + +test('rotates the Jira capability when a renderer document re-registers', () => { + let counter = 0; + const registry = new JiraCapabilityRegistry(() => `test-token-${(counter += 1)}`); + const frame = {}; + + const first = registry.register(frame); + assert.equal(first, 'test-token-1'); + + // A reload re-registers the same frame object: a fresh token is issued and + // the stale one is invalidated, so the reloaded document is never locked out. + const second = registry.register(frame); + assert.equal(second, 'test-token-2'); + assert.equal(registry.isAuthorized(frame, first), false); + assert.equal(registry.isAuthorized(frame, second), true); +}); + +test('only authorizes the issued token from the same renderer document', () => { + const registry = new JiraCapabilityRegistry(() => 'test-token'); + const registeredFrame = {}; + const otherFrame = {}; + registry.register(registeredFrame); + + assert.equal(registry.isAuthorized(registeredFrame, 'test-token'), true); + assert.equal(registry.isAuthorized(registeredFrame, 'wrong-token'), false); + assert.equal(registry.isAuthorized(otherFrame, 'test-token'), false); + assert.equal(registry.isAuthorized(registeredFrame, null), false); +}); + +test('unwraps only an authorized Jira capability envelope', () => { + const registry = new JiraCapabilityRegistry(() => 'test-token'); + const frame = {}; + registry.register(frame); + const payload = { requestId: 'request-1' }; + + assert.equal( + registry.unwrap(frame, { + capabilityToken: 'test-token', + payload, + }), + payload, + ); + assert.throws( + () => registry.unwrap(frame, { capabilityToken: 'wrong-token', payload }), + /unauthorized/i, + ); + assert.throws(() => registry.unwrap(frame, payload), /unauthorized/i); +}); diff --git a/electron/jira-capability.ts b/electron/jira-capability.ts new file mode 100644 index 0000000000..77da4cce67 --- /dev/null +++ b/electron/jira-capability.ts @@ -0,0 +1,44 @@ +import { randomBytes } from 'node:crypto'; +import { JiraCapabilityEnvelope } from './shared-with-frontend/jira-request.model'; + +const TOKEN_BYTES = 32; + +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + +export class JiraCapabilityRegistry { + private readonly _tokens = new WeakMap(); + + constructor( + private readonly _createToken: () => string = () => + randomBytes(TOKEN_BYTES).toString('base64url'), + ) {} + + register(frame: object): string { + // Always issue a fresh token, even when this frame object already has one. + // A renderer reload re-runs the preload and re-registers; if Electron hands + // back the same WebFrameMain object, returning null would leave the new + // document permanently without a capability (Jira broken until a full app + // restart). Rotating the token also invalidates any stale token still held + // by the previous document. + const token = this._createToken(); + this._tokens.set(frame, token); + return token; + } + + isAuthorized(frame: object, token: unknown): token is string { + return typeof token === 'string' && this._tokens.get(frame) === token; + } + + unwrap(frame: object, envelope: unknown): T { + if ( + !isRecord(envelope) || + !this.isAuthorized(frame, envelope.capabilityToken) || + !Object.prototype.hasOwnProperty.call(envelope, 'payload') + ) { + throw new Error('Unauthorized Jira IPC request'); + } + + return (envelope as unknown as JiraCapabilityEnvelope).payload; + } +} diff --git a/electron/jira-image-auth.ts b/electron/jira-image-auth.ts new file mode 100644 index 0000000000..c9f7acd5ef --- /dev/null +++ b/electron/jira-image-auth.ts @@ -0,0 +1,100 @@ +import { JiraImageAuthConfig } from './shared-with-frontend/jira-request.model'; + +interface JiraImageAuthState { + origin: string; + basePath: string; + authorization: string; +} + +let currentAuth: JiraImageAuthState | null = null; + +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); +const isNullableString = (value: unknown): value is string | null => + value === null || typeof value === 'string'; +const isOptionalNullableString = (value: unknown): value is string | null | undefined => + value === undefined || isNullableString(value); + +const parseImageAuthConfig = (config: unknown): JiraImageAuthConfig => { + if (!isRecord(config)) { + throw new Error('Invalid Jira image authentication config'); + } + const { host, userName, password, usePAT } = config; + + if ( + typeof host !== 'string' || + host.trim().length === 0 || + !isNullableString(userName) || + !isOptionalNullableString(password) || + (usePAT !== undefined && typeof usePAT !== 'boolean') + ) { + throw new Error('Invalid Jira image authentication config'); + } + + return { + host, + userName, + password, + usePAT: usePAT === true, + }; +}; + +// TODO simplify and do encoding in frontend service +export const setupRequestHeadersForImages = (rawConfig: unknown): void => { + currentAuth = null; + const config = parseImageAuthConfig(rawConfig); + const parsedUrl = new URL( + /^[a-z][a-z\d+.-]*:\/\//i.test(config.host as string) + ? (config.host as string) + : `https://${config.host}`, + ); + if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') { + throw new Error('Jira URL must use HTTP or HTTPS'); + } + + const password = config.password || ''; + const encoded = Buffer.from(`${config.userName || ''}:${password}`).toString('base64'); + const trimmedPath = parsedUrl.pathname.replace(/\/+$/, ''); + + currentAuth = { + origin: parsedUrl.origin, + basePath: trimmedPath || '/', + authorization: config.usePAT ? `Bearer ${password}` : `Basic ${encoded}`, + }; +}; + +export const applyJiraImageAuth = ( + rawUrl: string, + requestHeaders: Record, + resourceType: string, +): void => { + if (!currentAuth || resourceType !== 'image') { + return; + } + + let requestUrl: URL; + try { + requestUrl = new URL(rawUrl); + } catch { + return; + } + + const isInBasePath = + currentAuth.basePath === '/' || + requestUrl.pathname === currentAuth.basePath || + requestUrl.pathname.startsWith(`${currentAuth.basePath}/`); + if (requestUrl.origin !== currentAuth.origin || !isInBasePath) { + return; + } + + for (const headerName of Object.keys(requestHeaders)) { + if (headerName.toLowerCase() === 'authorization') { + delete requestHeaders[headerName]; + } + } + requestHeaders.authorization = currentAuth.authorization; +}; + +export const clearRequestHeadersForImages = (): void => { + currentAuth = null; +}; diff --git a/electron/jira-ipc.test.cjs b/electron/jira-ipc.test.cjs new file mode 100644 index 0000000000..dd4acb37a6 --- /dev/null +++ b/electron/jira-ipc.test.cjs @@ -0,0 +1,107 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); +const Module = require('node:module'); + +require('ts-node/register/transpile-only'); + +const handlers = new Map(); +const jiraImageAuth = require(path.resolve(__dirname, 'jira-image-auth.ts')); +const originalModuleLoad = Module._load; +Module._load = function patchedLoad(request, parent, isMain) { + if (request === 'electron') { + return { + ipcMain: { + handle: (channel, handler) => handlers.set(channel, handler), + on: () => undefined, + }, + }; + } + if (request === '../jira-image-auth') { + return jiraImageAuth; + } + return originalModuleLoad.call(this, request, parent, isMain); +}; + +const { initJiraIpc } = require(path.resolve(__dirname, 'ipc-handlers/jira.ts')); +const { IPC } = require( + path.resolve(__dirname, 'shared-with-frontend/ipc-events.const.ts'), +); +const { applyJiraImageAuth, setupRequestHeadersForImages } = jiraImageAuth; +Module._load = originalModuleLoad; + +initJiraIpc(); + +test('Jira IPC issues a capability only to the main renderer frame', () => { + const mainFrame = {}; + const event = { senderFrame: mainFrame, sender: { mainFrame } }; + const register = handlers.get(IPC.JIRA_REGISTER_CAPABILITY); + + const token = register(event); + assert.equal(typeof token, 'string'); + // Re-registering (e.g. after a renderer reload) rotates the token instead of + // locking the frame out. + const rotated = register(event); + assert.equal(typeof rotated, 'string'); + assert.notEqual(rotated, token); + + const subFrame = {}; + assert.equal(register({ senderFrame: subFrame, sender: { mainFrame } }), null); +}); + +test('Jira IPC rejects a request without the document capability', () => { + const mainFrame = {}; + const event = { senderFrame: mainFrame, sender: { mainFrame } }; + const register = handlers.get(IPC.JIRA_REGISTER_CAPABILITY); + const makeRequest = handlers.get(IPC.JIRA_MAKE_REQUEST_EVENT); + register(event); + + assert.throws( + () => + makeRequest(event, { + capabilityToken: 'forged-token', + payload: { requestId: 'request-1' }, + }), + /unauthorized/i, + ); +}); + +test('Jira IPC unwraps an authorized request before validation', async () => { + const mainFrame = {}; + const event = { senderFrame: mainFrame, sender: { mainFrame } }; + const register = handlers.get(IPC.JIRA_REGISTER_CAPABILITY); + const makeRequest = handlers.get(IPC.JIRA_MAKE_REQUEST_EVENT); + const token = register(event); + + const result = await makeRequest(event, { + capabilityToken: token, + payload: { requestId: 'request-1' }, + }); + + assert.deepEqual(result, { + requestId: 'request-1', + error: { message: 'Invalid Jira URL' }, + }); +}); + +test('issuing a capability to a new renderer document revokes stale image auth', () => { + setupRequestHeadersForImages({ + host: 'https://jira.example.com/jira', + userName: 'user', + password: 'pass', + usePAT: false, + }); + + const mainFrame = {}; + const event = { senderFrame: mainFrame, sender: { mainFrame } }; + const register = handlers.get(IPC.JIRA_REGISTER_CAPABILITY); + assert.equal(typeof register(event), 'string'); + + const requestHeaders = {}; + applyJiraImageAuth( + 'https://jira.example.com/jira/image.png', + requestHeaders, + 'image', + ); + assert.deepEqual(requestHeaders, {}); +}); diff --git a/electron/jira.test.cjs b/electron/jira.test.cjs new file mode 100644 index 0000000000..de595d9e0c --- /dev/null +++ b/electron/jira.test.cjs @@ -0,0 +1,420 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); + +require('ts-node/register/transpile-only'); + +const { + executeJiraRequest, +} = require( + path.resolve(__dirname, 'jira.ts'), +); +const { + applyJiraImageAuth, + clearRequestHeadersForImages, + setupRequestHeadersForImages, +} = require(path.resolve(__dirname, 'jira-image-auth.ts')); + +const makeRequest = (overrides = {}) => ({ + requestId: 'request-1', + url: 'https://jira.example.com/rest/api/latest/myself', + requestInit: { + method: 'GET', + headers: { + authorization: 'Basic secret', + 'Content-Type': 'application/json', + }, + }, + allowSelfSignedCertificate: false, + ...overrides, +}); + +test('allows Jira hosted on localhost and applies non-overridable fetch limits', async () => { + let fetchedUrl; + let fetchedInit; + const fetchStub = async (url, init) => { + fetchedUrl = url; + fetchedInit = init; + return { + ok: true, + text: async () => '{"ok":true}', + }; + }; + + const result = await executeJiraRequest( + makeRequest({ + url: 'http://127.0.0.1:8080/jira/rest/api/latest/myself', + requestInit: { + method: 'POST', + headers: { authorization: 'Bearer secret' }, + body: '{"query":"test"}', + redirect: 'follow', + timeout: 0, + size: 0, + }, + }), + fetchStub, + () => undefined, + ); + + assert.equal(fetchedUrl, 'http://127.0.0.1:8080/jira/rest/api/latest/myself'); + assert.deepEqual( + { + method: fetchedInit.method, + headers: fetchedInit.headers, + body: fetchedInit.body, + redirect: fetchedInit.redirect, + timeout: fetchedInit.timeout, + size: fetchedInit.size, + }, + { + method: 'POST', + headers: { authorization: 'Bearer secret' }, + body: '{"query":"test"}', + redirect: 'manual', + timeout: 20_000, + size: 25 * 1024 * 1024, + }, + ); + assert.deepEqual(result, { + requestId: 'request-1', + response: { ok: true }, + }); +}); + +test('rejects non-HTTP Jira URLs before fetch', async () => { + let fetchCalled = false; + const result = await executeJiraRequest( + makeRequest({ url: 'file:///etc/passwd' }), + async () => { + fetchCalled = true; + throw new Error('must not run'); + }, + () => undefined, + ); + + assert.equal(fetchCalled, false); + assert.deepEqual(result, { + requestId: 'request-1', + error: { message: 'Jira URL must use HTTP or HTTPS' }, + }); +}); + +test('rejects methods outside the Jira API contract', async () => { + let fetchCalled = false; + const result = await executeJiraRequest( + makeRequest({ requestInit: { method: 'DELETE', headers: {} } }), + async () => { + fetchCalled = true; + throw new Error('must not run'); + }, + () => undefined, + ); + + assert.equal(fetchCalled, false); + assert.deepEqual(result, { + requestId: 'request-1', + error: { message: 'Invalid Jira request method' }, + }); +}); + +test('returns an HTTP error without copying the remote response body', async () => { + let responseBodyDestroyed = false; + const result = await executeJiraRequest( + makeRequest(), + async () => ({ + ok: false, + status: 401, + statusText: 'Unauthorized', + body: { destroy: () => (responseBodyDestroyed = true) }, + text: async () => 'Access denied', + }), + () => undefined, + ); + + assert.deepEqual(result, { + requestId: 'request-1', + error: { + message: 'HTTP 401', + status: 401, + }, + }); + assert.equal(responseBodyDestroyed, true); + assert.equal('stack' in result.error, false); +}); + +test('treats a missing legacy certificate setting as false', async () => { + let allowSelfSignedCertificate; + const request = makeRequest(); + delete request.allowSelfSignedCertificate; + + const result = await executeJiraRequest( + request, + async () => ({ + ok: true, + text: async () => '{"ok":true}', + }), + (_url, allowSelfSigned) => { + allowSelfSignedCertificate = allowSelfSigned; + return undefined; + }, + ); + + assert.equal(allowSelfSignedCertificate, false); + assert.deepEqual(result.response, { ok: true }); +}); + +test('follows a same-origin Jira redirect', async () => { + const fetchedUrls = []; + let redirectBodyDestroyed = false; + const result = await executeJiraRequest( + makeRequest(), + async (url) => { + fetchedUrls.push(url); + if (fetchedUrls.length === 1) { + return { + ok: false, + status: 302, + statusText: 'Found', + body: { destroy: () => (redirectBodyDestroyed = true) }, + headers: { get: (name) => (name === 'location' ? '/jira/login' : null) }, + text: async () => '', + }; + } + return { + ok: true, + status: 200, + headers: { get: () => null }, + text: async () => '{"redirected":true}', + }; + }, + () => undefined, + ); + + assert.deepEqual(fetchedUrls, [ + 'https://jira.example.com/rest/api/latest/myself', + 'https://jira.example.com/jira/login', + ]); + assert.deepEqual(result.response, { redirected: true }); + assert.equal(redirectBodyDestroyed, true); +}); + +test('allows an HTTP to HTTPS redirect on the configured Jira hostname', async () => { + const fetchedUrls = []; + const result = await executeJiraRequest( + makeRequest({ url: 'http://jira.example.com:8080/rest/api/latest/myself' }), + async (url) => { + fetchedUrls.push(url); + if (fetchedUrls.length === 1) { + return { + ok: false, + status: 308, + statusText: 'Permanent Redirect', + headers: { + get: (name) => + name === 'location' + ? 'https://jira.example.com:8443/rest/api/latest/myself' + : null, + }, + text: async () => '', + }; + } + return { + ok: true, + status: 200, + headers: { get: () => null }, + text: async () => '{"upgraded":true}', + }; + }, + () => undefined, + ); + + assert.deepEqual(fetchedUrls, [ + 'http://jira.example.com:8080/rest/api/latest/myself', + 'https://jira.example.com:8443/rest/api/latest/myself', + ]); + assert.deepEqual(result.response, { upgraded: true }); +}); + +test('rejects redirects to a different hostname', async () => { + const fetchedUrls = []; + const result = await executeJiraRequest( + makeRequest(), + async (url) => { + fetchedUrls.push(url); + return { + ok: false, + status: 302, + statusText: 'Found', + headers: { + get: (name) => + name === 'location' ? 'https://internal.example.test/secret' : null, + }, + text: async () => '', + }; + }, + () => undefined, + ); + + assert.deepEqual(fetchedUrls, [ + 'https://jira.example.com/rest/api/latest/myself', + ]); + assert.deepEqual(result, { + requestId: 'request-1', + error: { message: 'Unsafe Jira redirect blocked' }, + }); +}); + +test('stops after five safe Jira redirects', async () => { + let fetchCalls = 0; + const result = await executeJiraRequest( + makeRequest(), + async () => { + fetchCalls += 1; + return { + ok: false, + status: 302, + statusText: 'Found', + headers: { get: () => `/redirect-${fetchCalls}` }, + text: async () => '', + }; + }, + () => undefined, + ); + + assert.equal(fetchCalls, 6); + assert.deepEqual(result, { + requestId: 'request-1', + error: { message: 'Too many Jira redirects' }, + }); +}); + +test('does not expose thrown error details beyond the message', async () => { + const thrown = Object.assign(new Error('network failed'), { + secret: 'do not return', + }); + const result = await executeJiraRequest( + makeRequest(), + async () => { + throw thrown; + }, + () => undefined, + ); + + assert.deepEqual(result, { + requestId: 'request-1', + error: { message: 'network failed' }, + }); +}); + +test('scopes image authentication to a custom Jira origin with port and base path', () => { + setupRequestHeadersForImages({ + host: 'http://localhost:8080/jira', + userName: 'user', + password: 'pass', + usePAT: false, + }); + + const matchingHeaders = { accept: 'image/png' }; + applyJiraImageAuth( + 'http://localhost:8080/jira/secure/attachment/1/image.png', + matchingHeaders, + 'image', + ); + assert.deepEqual(matchingHeaders, { + accept: 'image/png', + authorization: `Basic ${Buffer.from('user:pass').toString('base64')}`, + }); + + const outsidePathHeaders = {}; + applyJiraImageAuth( + 'http://localhost:8080/other/image.png', + outsidePathHeaders, + 'image', + ); + assert.deepEqual(outsidePathHeaders, {}); + + const prefixCollisionHeaders = {}; + applyJiraImageAuth( + 'http://localhost:8080/jira-evil/image.png', + prefixCollisionHeaders, + 'image', + ); + assert.deepEqual(prefixCollisionHeaders, {}); + + const xhrHeaders = {}; + applyJiraImageAuth('http://localhost:8080/jira/rest/api/latest/issue/1', xhrHeaders, 'xhr'); + assert.deepEqual(xhrHeaders, {}); +}); + +test('treats a missing legacy PAT setting as basic authentication', () => { + setupRequestHeadersForImages({ + host: 'https://jira.example.com/jira/', + userName: 'user', + password: 'pass', + }); + + const requestHeaders = {}; + applyJiraImageAuth( + 'https://jira.example.com/jira/image.png', + requestHeaders, + 'image', + ); + assert.equal( + requestHeaders.authorization, + `Basic ${Buffer.from('user:pass').toString('base64')}`, + ); +}); + +test('clears Jira image authentication when it is no longer needed', () => { + clearRequestHeadersForImages(); + + const requestHeaders = {}; + applyJiraImageAuth( + 'https://jira.example.com/jira/image.png', + requestHeaders, + 'image', + ); + assert.deepEqual(requestHeaders, {}); +}); + +test('clears previous image authentication when replacement config is invalid', () => { + setupRequestHeadersForImages({ + host: 'https://jira-a.example.com/jira', + userName: 'user', + password: 'pass', + usePAT: false, + }); + + assert.throws( + () => + setupRequestHeadersForImages({ + host: null, + userName: 'other-user', + password: 'other-pass', + usePAT: false, + }), + /Invalid Jira image authentication config/, + ); + + const requestHeaders = {}; + applyJiraImageAuth( + 'https://jira-a.example.com/jira/image.png', + requestHeaders, + 'image', + ); + assert.deepEqual(requestHeaders, {}); +}); + +test('rejects a non-HTTP Jira image authentication origin', () => { + assert.throws( + () => + setupRequestHeadersForImages({ + host: 'file:///tmp/jira', + userName: 'user', + password: 'pass', + usePAT: false, + }), + /HTTP or HTTPS/, + ); +}); diff --git a/electron/jira.ts b/electron/jira.ts index 421e447173..1f4b7608b6 100644 --- a/electron/jira.ts +++ b/electron/jira.ts @@ -1,131 +1,230 @@ -import { getWin } from './main-window'; -import { IPC } from './shared-with-frontend/ipc-events.const'; -import { session } from 'electron'; -import { JiraCfg } from '../src/app/features/issue/providers/jira/jira.model'; -import fetch, { RequestInit } from 'node-fetch'; -import { error, log } from 'electron-log/main'; +import fetch, { RequestInit, Response } from 'node-fetch'; import { createProxyAwareAgent } from './proxy-agent'; +import { + JiraElectronRequest, + JiraElectronResponse, + JIRA_MAIN_REQUEST_TIMEOUT_MS, + JIRA_MAX_RESPONSE_BYTES, +} from './shared-with-frontend/jira-request.model'; -export const sendJiraRequest = ({ - requestId, - requestInit, - url, - jiraCfg, -}: { - requestId: string; - requestInit: RequestInit; - url: string; - jiraCfg: JiraCfg; -}): void => { - const mainWin = getWin(); - const agent = createProxyAwareAgent(url, jiraCfg?.isAllowSelfSignedCertificate); +const MAX_REQUEST_ID_LENGTH = 256; +const MAX_URL_LENGTH = 16 * 1024; +const MAX_HEADER_BYTES = 64 * 1024; +const MAX_BODY_BYTES = 5 * 1024 * 1024; +const MAX_ERROR_MESSAGE_LENGTH = 2_000; +const MAX_REDIRECTS = 5; +const REDIRECT_STATUS_CODES = new Set([301, 302, 303, 307, 308]); - fetch(url, { - ...requestInit, - ...(agent ? { agent } : {}), - } as RequestInit) - .then(async (response) => { - // log('JIRA_RAW_RESPONSE', response); - if (!response.ok) { - error('Jira Error Error Response ELECTRON: ', response); - try { - log(JSON.stringify(response)); - } catch (e) {} +type FetchImplementation = (url: string, init: RequestInit) => Promise; +type AgentFactory = typeof createProxyAwareAgent; - let errText; - try { - errText = await response.text(); - } catch (e2) { - throw Error(response.statusText); - } - throw Error(errText || response.statusText); - } - return response; - }) - .then((res) => res.text()) - .then((text) => { - try { - return text ? JSON.parse(text) : {}; - } catch (e) { - console.error('Error: Cannot parse json'); - console.log('Error: text response', text); - // throw new Error(e); - return text; - } - }) - .then((response) => { - mainWin.webContents.send(IPC.JIRA_CB_EVENT, { - response, - requestId, - }); - }) - .catch((err: unknown) => { - mainWin.webContents.send(IPC.JIRA_CB_EVENT, { - error: err, - requestId, - }); - }); -}; +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); -// TODO simplify and do encoding in frontend service -export const setupRequestHeadersForImages = (jiraCfg: JiraCfg): void => { - const { host, protocol } = parseHostAndPort(jiraCfg); +const getRequestId = (request: unknown): string => + isRecord(request) && typeof request.requestId === 'string' + ? request.requestId.slice(0, MAX_REQUEST_ID_LENGTH) + : ''; - // TODO export to util fn - const _b64EncodeUnicode = (str: string): string => { - return Buffer.from(str || '').toString('base64'); - }; - const encoded = _b64EncodeUnicode(`${jiraCfg.userName}:${jiraCfg.password}`); - const filter = { - urls: [`${protocol}://${host}/*`], - }; +const validateHeaders = (headers: unknown): Record => { + if (!isRecord(headers)) { + throw new Error('Invalid Jira request headers'); + } - // thankfully only the last attached listener will be used - // @see: https://electronjs.org/docs/api/web-request - session.defaultSession.webRequest.onBeforeSendHeaders(filter, (details, callback) => { - if (jiraCfg.usePAT) { - details.requestHeaders.authorization = `Bearer ${jiraCfg.password}`; - } else { - details.requestHeaders.authorization = `Basic ${encoded}`; + const entries = Object.entries(headers); + let byteLength = 0; + for (const [name, value] of entries) { + if (!name || typeof value !== 'string') { + throw new Error('Invalid Jira request headers'); } - callback({ requestHeaders: details.requestHeaders }); - }); + byteLength += Buffer.byteLength(name) + Buffer.byteLength(value); + } + + if (byteLength > MAX_HEADER_BYTES) { + throw new Error('Jira request headers are too large'); + } + + return Object.fromEntries(entries) as Record; }; -const MATCH_PROTOCOL_REG_EX = /(^[^:]+):\/\//; -const MATCH_PORT_REG_EX = /:\d{2,4}/; - -const parseHostAndPort = ( - config: JiraCfg, -): { host: string; protocol: string; port: number | undefined } => { - let host: string = config.host as string; - let protocol; - let port; - - if (!host) { - throw new Error('No host given'); +const validateJiraRequest = (request: unknown): JiraElectronRequest => { + if (!isRecord(request)) { + throw new Error('Invalid Jira request'); } - // parse port from host and remove it - if (host.match(MATCH_PORT_REG_EX)) { - const match = MATCH_PORT_REG_EX.exec(host) as RegExpExecArray; - host = host.replace(MATCH_PORT_REG_EX, ''); - port = parseInt(match[0].replace(':', ''), 10); + if ( + typeof request.requestId !== 'string' || + request.requestId.length === 0 || + request.requestId.length > MAX_REQUEST_ID_LENGTH + ) { + throw new Error('Invalid Jira request id'); } - // parse protocol from host and remove it - if (host.match(MATCH_PROTOCOL_REG_EX)) { - const match = MATCH_PROTOCOL_REG_EX.exec(host); - host = host - .replace(MATCH_PROTOCOL_REG_EX, '') - // remove trailing slash just in case - .replace(/\/$/, ''); - - protocol = (match as any)[1]; - } else { - protocol = 'https'; + if ( + typeof request.url !== 'string' || + request.url.length === 0 || + request.url.length > MAX_URL_LENGTH + ) { + throw new Error('Invalid Jira URL'); } - // log({host, protocol, port}); - return { host, protocol, port }; + let parsedUrl: URL; + try { + parsedUrl = new URL(request.url); + } catch { + throw new Error('Invalid Jira URL'); + } + if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') { + throw new Error('Jira URL must use HTTP or HTTPS'); + } + + if (!isRecord(request.requestInit)) { + throw new Error('Invalid Jira request options'); + } + const { method, headers, body } = request.requestInit; + if (method !== 'GET' && method !== 'POST' && method !== 'PUT') { + throw new Error('Invalid Jira request method'); + } + if (body !== undefined && typeof body !== 'string') { + throw new Error('Invalid Jira request body'); + } + if (typeof body === 'string' && Buffer.byteLength(body) > MAX_BODY_BYTES) { + throw new Error('Jira request body is too large'); + } + if ( + request.allowSelfSignedCertificate !== undefined && + typeof request.allowSelfSignedCertificate !== 'boolean' + ) { + throw new Error('Invalid Jira certificate setting'); + } + + return { + requestId: request.requestId, + url: parsedUrl.href, + requestInit: { + method, + headers: validateHeaders(headers), + ...(typeof body === 'string' ? { body } : {}), + }, + allowSelfSignedCertificate: request.allowSelfSignedCertificate === true, + }; +}; + +const errorMessage = (error: unknown): string => { + const message = error instanceof Error ? error.message : 'Jira request failed'; + return message.slice(0, MAX_ERROR_MESSAGE_LENGTH) || 'Jira request failed'; +}; + +const discardResponseBody = (response: Response): void => { + const body = response.body as unknown; + if (isRecord(body) && typeof body.destroy === 'function') { + body.destroy.call(body); + } +}; + +export const executeJiraRequest = async ( + rawRequest: unknown, + fetchImplementation: FetchImplementation = fetch, + agentFactory: AgentFactory = createProxyAwareAgent, +): Promise => { + const requestId = getRequestId(rawRequest); + + try { + const request = validateJiraRequest(rawRequest); + const response = await fetchWithSafeRedirects( + request, + fetchImplementation, + agentFactory, + ); + + if (!response.ok) { + discardResponseBody(response); + return { + requestId: request.requestId, + error: { + // Response bodies can contain issue data or internal server details. + // Keep them out of the renderer and its exportable logs. + message: `HTTP ${response.status}`, + status: response.status, + }, + }; + } + + const text = await response.text(); + let parsedResponse: unknown = {}; + if (text) { + try { + parsedResponse = JSON.parse(text) as unknown; + } catch { + parsedResponse = text; + } + } + + return { + requestId: request.requestId, + response: parsedResponse, + }; + } catch (error) { + return { + requestId, + error: { message: errorMessage(error) }, + }; + } +}; + +const isSafeRedirect = (currentUrl: URL, nextUrl: URL): boolean => + nextUrl.origin === currentUrl.origin || + (currentUrl.protocol === 'http:' && + nextUrl.protocol === 'https:' && + nextUrl.hostname === currentUrl.hostname); + +const fetchWithSafeRedirects = async ( + request: JiraElectronRequest, + fetchImplementation: FetchImplementation, + agentFactory: AgentFactory, +): Promise => { + let currentUrl = request.url; + let method = request.requestInit.method; + let body = request.requestInit.body; + + for (let redirectCount = 0; ; redirectCount += 1) { + const agent = agentFactory(currentUrl, request.allowSelfSignedCertificate); + const response = await fetchImplementation(currentUrl, { + method, + headers: request.requestInit.headers, + ...(body !== undefined ? { body } : {}), + ...(agent ? { agent } : {}), + redirect: 'manual', + timeout: JIRA_MAIN_REQUEST_TIMEOUT_MS, + size: JIRA_MAX_RESPONSE_BYTES, + }); + + if (!REDIRECT_STATUS_CODES.has(response.status)) { + return response; + } + + const location = response.headers?.get('location'); + if (!location) { + return response; + } + discardResponseBody(response); + if (redirectCount >= MAX_REDIRECTS) { + throw new Error('Too many Jira redirects'); + } + + const currentParsedUrl = new URL(currentUrl); + const nextUrl = new URL(location, currentParsedUrl); + if (!isSafeRedirect(currentParsedUrl, nextUrl)) { + throw new Error('Unsafe Jira redirect blocked'); + } + + if ( + response.status === 303 || + ((response.status === 301 || response.status === 302) && method === 'POST') + ) { + method = 'GET'; + body = undefined; + } + currentUrl = nextUrl.href; + } }; diff --git a/electron/main-window.ts b/electron/main-window.ts index c1d2796b6a..8105c31dea 100644 --- a/electron/main-window.ts +++ b/electron/main-window.ts @@ -31,6 +31,7 @@ import { loadSimpleStoreAll } from './simple-store'; import { SimpleStoreKey } from './shared-with-frontend/simple-store.const'; import { markGpuStartupSuccess } from './gpu-startup-guard'; import { isAppOriginUrl } from './navigation-guard'; +import { applyJiraImageAuth } from './jira-image-auth'; let mainWin: BrowserWindow; @@ -244,6 +245,7 @@ export const createWindow = async ({ ) { removeKeyInAnyCase(requestHeaders, 'User-Agent'); } + applyJiraImageAuth(details.url, requestHeaders, details.resourceType); callback({ requestHeaders }); }); diff --git a/electron/preload-api-capability.test.cjs b/electron/preload-api-capability.test.cjs new file mode 100644 index 0000000000..bee0fb4c22 --- /dev/null +++ b/electron/preload-api-capability.test.cjs @@ -0,0 +1,94 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); + +require('ts-node/register/transpile-only'); + +const { IPC } = require( + path.resolve(__dirname, 'shared-with-frontend/ipc-events.const.ts'), +); +const { + createJiraPreloadApiConsumer, + toPayloadOnlyIpcListener, +} = require(path.resolve(__dirname, 'shared-with-frontend/preload-api.ts')); + +test('renderer listeners receive payloads without the Electron event object', () => { + const received = []; + const listener = toPayloadOnlyIpcListener((...args) => received.push(args)); + const rawEvent = { + sender: { + invoke: () => { + throw new Error('must not be exposed'); + }, + }, + }; + + listener(rawEvent, { safe: true }, 'payload'); + + assert.deepEqual(received, [[{ safe: true }, 'payload']]); + assert.notEqual(received[0][0], rawEvent); +}); + +test('Jira preload capability registers eagerly and can only be consumed once', async () => { + const invocations = []; + const invoke = async (channel, ...args) => { + invocations.push([channel, ...args]); + if (channel === IPC.JIRA_REGISTER_CAPABILITY) { + return 'main-issued-token'; + } + return { requestId: 'request-1', response: { ok: true } }; + }; + + const consume = createJiraPreloadApiConsumer(invoke); + assert.deepEqual(invocations, [[IPC.JIRA_REGISTER_CAPABILITY]]); + + const api = consume(); + assert.ok(api); + assert.equal(consume(), null); + assert.equal(consume(), null); + + const request = { + requestId: 'request-1', + url: 'https://jira.example.com/rest/api/latest/myself', + requestInit: { method: 'GET', headers: {} }, + allowSelfSignedCertificate: false, + }; + await api.makeRequest(request); + await api.clearImgHeaders(); + + assert.deepEqual(invocations[1], [ + IPC.JIRA_MAKE_REQUEST_EVENT, + { + capabilityToken: 'main-issued-token', + payload: request, + }, + ]); + assert.deepEqual(invocations[2], [ + IPC.JIRA_CLEAR_IMG_HEADERS, + { + capabilityToken: 'main-issued-token', + payload: null, + }, + ]); +}); + +test('Jira preload API refuses calls when main does not issue a capability', async () => { + const invocations = []; + const invoke = async (channel, ...args) => { + invocations.push([channel, ...args]); + return null; + }; + const api = createJiraPreloadApiConsumer(invoke)(); + + await assert.rejects( + () => + api.makeRequest({ + requestId: 'request-1', + url: 'https://jira.example.com', + requestInit: { method: 'GET', headers: {} }, + allowSelfSignedCertificate: false, + }), + /unavailable/i, + ); + assert.deepEqual(invocations, [[IPC.JIRA_REGISTER_CAPABILITY]]); +}); diff --git a/electron/preload.ts b/electron/preload.ts index e6e081d942..ee8b8428aa 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -1,10 +1,4 @@ -import { - ipcRenderer, - IpcRendererEvent, - webFrame, - contextBridge, - webUtils, -} from 'electron'; +import { ipcRenderer, webFrame, contextBridge, webUtils } from 'electron'; import { ElectronAPI } from './electronAPI.d'; import { IS_GNOME_DESKTOP, IS_GNOME_WAYLAND } from './common.const'; import { IPC, IPCEventValue } from './shared-with-frontend/ipc-events.const'; @@ -21,6 +15,10 @@ import { LocalRestApiRequestPayload, LocalRestApiResponsePayload, } from './shared-with-frontend/local-rest-api.model'; +import { + createJiraPreloadApiConsumer, + toPayloadOnlyIpcListener, +} from './shared-with-frontend/preload-api'; let pluginNodeExecutionApiConsumed = false; @@ -31,13 +29,12 @@ const _invoke: (channel: IPCEventValue, ...args: unknown[]) => Promise ...args ) => ipcRenderer.invoke(channel, ...args); +const consumeJiraApi = createJiraPreloadApiConsumer(_invoke); + const ea: ElectronAPI = { - on: ( - channel: string, - listener: (event: IpcRendererEvent, ...args: unknown[]) => void, - ) => { + on: (channel: string, listener: (...args: unknown[]) => void) => { // NOTE: there is no proper way to unsubscribe apart from unsubscribing all - ipcRenderer.on(channel, listener); + ipcRenderer.on(channel, toPayloadOnlyIpcListener(listener)); }, // SYNC // ---- @@ -205,9 +202,6 @@ const ea: ElectronAPI = { _send('REGISTER_GLOBAL_SHORTCUTS', keyboardCfg), showFullScreenBlocker: (args) => _send('FULL_SCREEN_BLOCKER', args), - makeJiraRequest: (args) => _send('JIRA_MAKE_REQUEST_EVENT', args), - jiraSetupImgHeaders: (args) => _send('JIRA_SETUP_IMG_HEADERS', args), - backupAppData: (appData) => _send('BACKUP', appData), updateCurrentTask: ( @@ -236,6 +230,8 @@ const ea: ElectronAPI = { ipcRenderer.on('SWITCH_TASK', (_: any, taskId: string) => listener(taskId)); }, + consumeJiraApi: () => consumeJiraApi(), + // Plugin API consumePluginNodeExecutionApi: () => { if (pluginNodeExecutionApiConsumed) { diff --git a/electron/shared-with-frontend/ipc-events.const.ts b/electron/shared-with-frontend/ipc-events.const.ts index 8cc3e2165d..e79d0093c7 100644 --- a/electron/shared-with-frontend/ipc-events.const.ts +++ b/electron/shared-with-frontend/ipc-events.const.ts @@ -4,9 +4,10 @@ export enum IPC { EXIT = 'EXIT', RELAUNCH = 'RELAUNCH', - JIRA_CB_EVENT = 'JIRA_RESPONSE', JIRA_MAKE_REQUEST_EVENT = 'JIRA_MAKE_REQUEST_EVENT', JIRA_SETUP_IMG_HEADERS = 'JIRA_SETUP_IMG_HEADERS', + JIRA_CLEAR_IMG_HEADERS = 'JIRA_CLEAR_IMG_HEADERS', + JIRA_REGISTER_CAPABILITY = 'JIRA_REGISTER_CAPABILITY', REGISTER_GLOBAL_SHORTCUTS_EVENT = 'REGISTER_GLOBAL_SHORTCUTS', IDLE_TIME = 'IDLE_TIME', RESUME = 'RESUME', diff --git a/electron/shared-with-frontend/jira-request.model.ts b/electron/shared-with-frontend/jira-request.model.ts new file mode 100644 index 0000000000..08a900da2f --- /dev/null +++ b/electron/shared-with-frontend/jira-request.model.ts @@ -0,0 +1,44 @@ +export const JIRA_MAIN_REQUEST_TIMEOUT_MS = 20_000; +export const JIRA_MAX_RESPONSE_BYTES = 25 * 1024 * 1024; + +export type JiraRequestMethod = 'GET' | 'POST' | 'PUT'; + +export interface JiraElectronRequestInit { + method: JiraRequestMethod; + headers: Record; + body?: string; +} + +export interface JiraElectronRequest { + requestId: string; + url: string; + requestInit: JiraElectronRequestInit; + allowSelfSignedCertificate: boolean; +} + +export interface JiraElectronResponse { + requestId: string; + response?: unknown; + error?: { + status?: number; + message: string; + }; +} + +export interface JiraCapabilityEnvelope { + capabilityToken: string; + payload: TPayload; +} + +export interface JiraImageAuthConfig { + host: string | null; + userName: string | null; + password?: string | null; + usePAT: boolean; +} + +export interface JiraElectronApi { + makeRequest(request: JiraElectronRequest): Promise; + setupImgHeaders(config: JiraImageAuthConfig): Promise; + clearImgHeaders(): Promise; +} diff --git a/electron/shared-with-frontend/one-shot-api-consumer.ts b/electron/shared-with-frontend/one-shot-api-consumer.ts new file mode 100644 index 0000000000..998fb85fde --- /dev/null +++ b/electron/shared-with-frontend/one-shot-api-consumer.ts @@ -0,0 +1,16 @@ +/** + * Creates a preload API consumer whose capability is returned exactly once. + * Trusted startup code claims the capability before untrusted plugin code runs. + */ +export const createOneShotApiConsumer = (factory: () => T): (() => T | null) => { + let isConsumed = false; + + return () => { + if (isConsumed) { + return null; + } + + isConsumed = true; + return factory(); + }; +}; diff --git a/electron/shared-with-frontend/preload-api.ts b/electron/shared-with-frontend/preload-api.ts new file mode 100644 index 0000000000..220449e04b --- /dev/null +++ b/electron/shared-with-frontend/preload-api.ts @@ -0,0 +1,52 @@ +import { IPC, IPCEventValue } from './ipc-events.const'; +import { + JiraCapabilityEnvelope, + JiraElectronApi, + JiraElectronResponse, +} from './jira-request.model'; +import { createOneShotApiConsumer } from './one-shot-api-consumer'; + +type Invoke = (channel: IPCEventValue, ...args: unknown[]) => Promise; +type PayloadListener = (...args: unknown[]) => void; + +export const toPayloadOnlyIpcListener = + (listener: PayloadListener) => + (_event: unknown, ...args: unknown[]): void => + listener(...args); + +export const createJiraPreloadApiConsumer = ( + invoke: Invoke, +): (() => JiraElectronApi | null) => { + // Register before renderer code runs. The token remains inside this isolated + // preload closure and is never exposed through the context bridge. + const capabilityTokenPromise = invoke(IPC.JIRA_REGISTER_CAPABILITY).then((token) => + typeof token === 'string' && token.length > 0 ? token : null, + ); + + const invokeWithCapability = async ( + channel: IPCEventValue, + payload: TPayload, + ): Promise => { + const capabilityToken = await capabilityTokenPromise; + if (!capabilityToken) { + throw new Error('Jira Electron API is unavailable'); + } + const envelope: JiraCapabilityEnvelope = { + capabilityToken, + payload, + }; + return invoke(channel, envelope) as Promise; + }; + + return createOneShotApiConsumer(() => ({ + makeRequest: (request) => + invokeWithCapability( + IPC.JIRA_MAKE_REQUEST_EVENT, + request, + ), + setupImgHeaders: (config) => + invokeWithCapability(IPC.JIRA_SETUP_IMG_HEADERS, config), + clearImgHeaders: () => + invokeWithCapability(IPC.JIRA_CLEAR_IMG_HEADERS, null), + })); +}; diff --git a/src/app/core/electron/electron.effects.spec.ts b/src/app/core/electron/electron.effects.spec.ts new file mode 100644 index 0000000000..3144a57486 --- /dev/null +++ b/src/app/core/electron/electron.effects.spec.ts @@ -0,0 +1,32 @@ +import { parseDownloadedFilePayload } from './electron.effects'; + +describe('parseDownloadedFilePayload', () => { + it('reads the download path from the payload-only args ([file], not [event, file])', () => { + // Regression for the ANY_FILE_DOWNLOADED consumer: after the payload-only + // IPC listener stripped the Electron event, the file moved from args[1] to + // args[0]. Reading the old index yielded undefined -> TypeError on every + // download. + expect( + parseDownloadedFilePayload([{ path: '/home/u/Downloads/report.pdf' }]), + ).toEqual({ + fileName: 'report.pdf', + dir: '/home/u/Downloads/', + }); + }); + + it('does not match the pre-strip [event, file] shape (file must be at index 0)', () => { + const eventFirst = [{ sender: {} }, { path: '/home/u/Downloads/report.pdf' }]; + expect(parseDownloadedFilePayload(eventFirst)).toBeNull(); + }); + + it('returns null for a missing payload instead of throwing', () => { + expect(parseDownloadedFilePayload([])).toBeNull(); + expect(parseDownloadedFilePayload(undefined)).toBeNull(); + }); + + it('returns null when the payload has no string path', () => { + expect(parseDownloadedFilePayload([{}])).toBeNull(); + expect(parseDownloadedFilePayload([{ path: 123 }])).toBeNull(); + expect(parseDownloadedFilePayload([null])).toBeNull(); + }); +}); diff --git a/src/app/core/electron/electron.effects.ts b/src/app/core/electron/electron.effects.ts index 3f76dd9b38..ece8b1272b 100644 --- a/src/app/core/electron/electron.effects.ts +++ b/src/app/core/electron/electron.effects.ts @@ -7,6 +7,26 @@ import { SnackService } from '../snack/snack.service'; import { T } from '../../t.const'; import { ipcAnyFileDownloaded$ } from '../ipc-events'; +/** + * Extracts the downloaded file's name and directory from the ANY_FILE_DOWNLOADED + * payload. The payload-only IPC listener strips the raw Electron event, so the + * file is the first arg (it was [1] before the event was stripped). Returns null + * for a missing/malformed payload so the effect never throws. + */ +export const parseDownloadedFilePayload = ( + args: unknown, +): { fileName: string; dir: string } | null => { + const fileParam = Array.isArray(args) ? (args[0] as { path?: unknown }) : undefined; + const path = typeof fileParam?.path === 'string' ? fileParam.path : null; + if (!path) { + return null; + } + return { + fileName: path.replace(/^.*[\\\/]/, ''), + dir: path.replace(/[^\/]*$/, ''), + }; +}; + @Injectable() export class ElectronEffects { private _snackService = inject(SnackService); @@ -17,21 +37,21 @@ export class ElectronEffects { () => ipcAnyFileDownloaded$.pipe( tap((args) => { - const fileParam = (args as any)[1]; - const path = fileParam.path; - const fileName = path.replace(/^.*[\\\/]/, ''); - const dir = path.replace(/[^\/]*$/, ''); + const file = parseDownloadedFilePayload(args); + if (!file) { + return; + } this._snackService.open({ ico: 'file_download', // ico: 'file_download_done', // ico: 'download_done', msg: T.GLOBAL_SNACK.FILE_DOWNLOADED, translateParams: { - fileName, + fileName: file.fileName, }, actionStr: T.GLOBAL_SNACK.FILE_DOWNLOADED_BTN, actionFn: () => { - window.ea.openPath(dir); + window.ea.openPath(file.dir); }, }); }), diff --git a/src/app/core/electron/exec-before-close.service.ts b/src/app/core/electron/exec-before-close.service.ts index 43a77de300..b25a6bb73f 100644 --- a/src/app/core/electron/exec-before-close.service.ts +++ b/src/app/core/electron/exec-before-close.service.ts @@ -1,14 +1,11 @@ import { Injectable } from '@angular/core'; import { EMPTY, Observable } from 'rxjs'; -import { map } from 'rxjs/operators'; import { IS_ELECTRON } from '../../app.constants'; import { ipcNotifyOnClose$ } from '../ipc-events'; @Injectable({ providedIn: 'root' }) export class ExecBeforeCloseService { - onBeforeClose$: Observable = IS_ELECTRON - ? ipcNotifyOnClose$.pipe(map(([, ids]: any) => ids)) - : EMPTY; + onBeforeClose$: Observable = IS_ELECTRON ? ipcNotifyOnClose$ : EMPTY; constructor() {} diff --git a/src/app/core/ipc-events.spec.ts b/src/app/core/ipc-events.spec.ts index 4ce873fd9b..cb9b79dff4 100644 --- a/src/app/core/ipc-events.spec.ts +++ b/src/app/core/ipc-events.spec.ts @@ -1,115 +1,34 @@ -import { of } from 'rxjs'; -import { map, filter } from 'rxjs/operators'; +import { parseAddTaskFromAppUriPayload, parseBeforeCloseIdsPayload } from './ipc-events'; -describe('ipcAddTaskFromAppUri$ validation logic', () => { - it('should handle valid data with title', (done) => { - const validData = { title: 'Test Task' }; - - // Test the validation logic that we implemented - const testObservable = of([{}, validData]).pipe( - map(([ev, data]: any[]) => { - // This is the exact validation logic from our implementation - if ( - !data || - typeof data !== 'object' || - typeof (data as any).title !== 'string' - ) { - console.error(`Validation failed for:`, data); - return null; - } - return data as { title: string }; - }), - filter((data: any): data is { title: string } => data !== null), - ); - - testObservable.subscribe((data) => { - expect(data).toEqual({ title: 'Test Task' }); - done(); +describe('parseAddTaskFromAppUriPayload', () => { + it('accepts a payload with a title', () => { + expect(parseAddTaskFromAppUriPayload({ title: 'Test Task' })).toEqual({ + title: 'Test Task', }); }); - it('should filter out undefined data', (done) => { - const testObservable = of([{}, undefined]).pipe( - map(([ev, data]: any[]) => { - if ( - !data || - typeof data !== 'object' || - typeof (data as any).title !== 'string' - ) { - return null; - } - return data as { title: string }; - }), - filter((data: any): data is { title: string } => data !== null), - ); - - const emissions: any[] = []; - const subscription = testObservable.subscribe((data) => { - emissions.push(data); - }); - - setTimeout(() => { - expect(emissions.length).toBe(0); - subscription.unsubscribe(); - done(); - }, 50); + it('rejects missing payload data', () => { + expect(parseAddTaskFromAppUriPayload(undefined)).toBeNull(); }); - it('should filter out data without title property', (done) => { - const invalidData = { notTitle: 'Test Task' }; - - const testObservable = of([{}, invalidData]).pipe( - map(([ev, data]: any[]) => { - if ( - !data || - typeof data !== 'object' || - typeof (data as any).title !== 'string' - ) { - return null; - } - return data as { title: string }; - }), - filter((data: any): data is { title: string } => data !== null), - ); - - const emissions: any[] = []; - const subscription = testObservable.subscribe((data) => { - emissions.push(data); - }); - - setTimeout(() => { - expect(emissions.length).toBe(0); - subscription.unsubscribe(); - done(); - }, 50); + it('rejects a payload without a title', () => { + expect(parseAddTaskFromAppUriPayload({ notTitle: 'Test Task' })).toBeNull(); }); - it('should filter out data with non-string title', (done) => { - const invalidData = { title: 123 }; - - const testObservable = of([{}, invalidData]).pipe( - map(([ev, data]: any[]) => { - if ( - !data || - typeof data !== 'object' || - typeof (data as any).title !== 'string' - ) { - return null; - } - return data as { title: string }; - }), - filter((data: any): data is { title: string } => data !== null), - ); - - const emissions: any[] = []; - const subscription = testObservable.subscribe((data) => { - emissions.push(data); - }); - - setTimeout(() => { - expect(emissions.length).toBe(0); - subscription.unsubscribe(); - done(); - }, 50); + it('rejects a non-string title', () => { + expect(parseAddTaskFromAppUriPayload({ title: 123 })).toBeNull(); + }); +}); + +describe('parseBeforeCloseIdsPayload', () => { + it('reads ids from the payload without an Electron event argument', () => { + expect(parseBeforeCloseIdsPayload(['SYNC_BEFORE_CLOSE'])).toEqual([ + 'SYNC_BEFORE_CLOSE', + ]); + }); + + it('rejects malformed close payloads', () => { + expect(parseBeforeCloseIdsPayload(undefined)).toEqual([]); + expect(parseBeforeCloseIdsPayload(['valid', 123])).toEqual([]); }); }); diff --git a/src/app/core/ipc-events.ts b/src/app/core/ipc-events.ts index 01010af4e0..0e09ec57c3 100644 --- a/src/app/core/ipc-events.ts +++ b/src/app/core/ipc-events.ts @@ -3,18 +3,33 @@ import { IPC } from '../../../electron/shared-with-frontend/ipc-events.const'; import { map, filter } from 'rxjs/operators'; import { EMPTY, Observable } from 'rxjs'; import { IS_ELECTRON } from '../app.constants'; -import { devError } from '../util/dev-error'; + +export const parseAddTaskFromAppUriPayload = ( + data: unknown, +): { title: string } | null => { + if ( + !data || + typeof data !== 'object' || + typeof (data as { title?: unknown }).title !== 'string' + ) { + return null; + } + return data as { title: string }; +}; + +export const parseBeforeCloseIdsPayload = (data: unknown): string[] => + Array.isArray(data) && data.every((id) => typeof id === 'string') ? data : []; export const ipcIdleTime$: Observable = IS_ELECTRON - ? ipcEvent$(IPC.IDLE_TIME).pipe(map(([ev, idleTimeInMs]) => idleTimeInMs as number)) + ? ipcEvent$(IPC.IDLE_TIME).pipe(map(([idleTimeInMs]) => idleTimeInMs as number)) : EMPTY; export const ipcAnyFileDownloaded$: Observable = IS_ELECTRON ? ipcEvent$(IPC.ANY_FILE_DOWNLOADED).pipe() : EMPTY; -export const ipcNotifyOnClose$: Observable = IS_ELECTRON - ? ipcEvent$(IPC.NOTIFY_ON_CLOSE).pipe() +export const ipcNotifyOnClose$: Observable = IS_ELECTRON + ? ipcEvent$(IPC.NOTIFY_ON_CLOSE).pipe(map(([ids]) => parseBeforeCloseIdsPayload(ids))) : EMPTY; export const ipcResume$: Observable = IS_ELECTRON @@ -37,20 +52,7 @@ export const ipcShowAddTaskBar$: Observable = IS_ELECTRON export const ipcAddTaskFromAppUri$: Observable<{ title: string } | null> = IS_ELECTRON ? ipcEvent$(IPC.ADD_TASK_FROM_APP_URI).pipe( - map(([ev, data]) => { - // Validate that data exists and has required properties - if ( - !data || - typeof data !== 'object' || - typeof (data as { title: string }).title !== 'string' - ) { - devError( - `ipcAddTaskFromAppUri$ received invalid data: ${JSON.stringify(data)}`, - ); - return null; - } - return data as { title: string }; - }), + map(([data]) => parseAddTaskFromAppUriPayload(data)), filter((data): data is { title: string } => data !== null), ) : EMPTY; diff --git a/src/app/core/startup/startup.service.spec.ts b/src/app/core/startup/startup.service.spec.ts index f942f4f5c3..51a7b41a89 100644 --- a/src/app/core/startup/startup.service.spec.ts +++ b/src/app/core/startup/startup.service.spec.ts @@ -22,11 +22,13 @@ import { provideMockStore } from '@ngrx/store/testing'; import { selectSyncConfig } from '../../features/config/store/global-config.reducer'; import { selectEnabledIssueProviders } from '../../features/issue/store/issue-provider.selectors'; import { RatePromptService } from '../../features/dialog-please-rate/rate-prompt.service'; +import { JiraElectronBridgeService } from '../../features/issue/providers/jira/jira-electron-bridge.service'; describe('StartupService', () => { let service: StartupService; let pluginService: jasmine.SpyObj; let ratePromptService: jasmine.SpyObj; + let jiraElectronBridge: jasmine.SpyObj; beforeEach(() => { // Mock localStorage @@ -76,6 +78,9 @@ describe('StartupService', () => { const pluginServiceSpy = jasmine.createSpyObj('PluginService', ['initializePlugins']); pluginServiceSpy.initializePlugins.and.returnValue(Promise.resolve()); + const jiraElectronBridgeSpy = jasmine.createSpyObj('JiraElectronBridgeService', [ + 'initialize', + ]); const syncWrapperServiceSpy = jasmine.createSpyObj('SyncWrapperService', [ 'isSyncInProgressSync', @@ -128,6 +133,7 @@ describe('StartupService', () => { { provide: SnackService, useValue: snackServiceSpy }, { provide: RatePromptService, useValue: ratePromptServiceSpy }, { provide: PluginService, useValue: pluginServiceSpy }, + { provide: JiraElectronBridgeService, useValue: jiraElectronBridgeSpy }, { provide: SyncWrapperService, useValue: syncWrapperServiceSpy }, { provide: BannerService, useValue: bannerServiceSpy }, { provide: UiHelperService, useValue: uiHelperServiceSpy }, @@ -154,12 +160,20 @@ describe('StartupService', () => { ratePromptService = TestBed.inject( RatePromptService, ) as jasmine.SpyObj; + jiraElectronBridge = TestBed.inject( + JiraElectronBridgeService, + ) as jasmine.SpyObj; }); describe('init', () => { // Note: Full init() testing requires complex BroadcastChannel mocking // These tests cover the testable parts + it('claims the Jira Electron capability before deferred plugin initialization', () => { + expect(jiraElectronBridge.initialize).toHaveBeenCalledOnceWith(); + expect(pluginService.initializePlugins).not.toHaveBeenCalled(); + }); + it('should check for stray backups during initialization', fakeAsync(() => { // Mock BroadcastChannel to prevent multi-instance blocking const mockChannel = { diff --git a/src/app/core/startup/startup.service.ts b/src/app/core/startup/startup.service.ts index 4ce370d88b..aa4c707c48 100644 --- a/src/app/core/startup/startup.service.ts +++ b/src/app/core/startup/startup.service.ts @@ -27,7 +27,6 @@ import { selectSyncConfig } from '../../features/config/store/global-config.redu import { selectEnabledIssueProviders } from '../../features/issue/store/issue-provider.selectors'; import { SyncProviderId } from '../../op-log/sync-providers/provider.const'; import { IPC } from '../../../../electron/shared-with-frontend/ipc-events.const'; -import { IpcRendererEvent } from 'electron'; import { environment } from '../../../environments/environment'; import { TrackingReminderService } from '../../features/tracking-reminder/tracking-reminder.service'; import { CapacitorPlatformService } from '../platform/capacitor-platform.service'; @@ -37,6 +36,7 @@ import { OnboardingHintService } from '../../features/onboarding/onboarding-hint import { LocalRestApiHandlerService } from '../electron/local-rest-api-handler.service'; import { CustomThemeService } from '../theme/custom-theme.service'; import { UpdateCheckService } from '../update-check/update-check.service'; +import { JiraElectronBridgeService } from '../../features/issue/providers/jira/jira-electron-bridge.service'; const w = window as Window & { productivityTips?: string[][]; randomIndex?: number }; @@ -76,12 +76,21 @@ export class StartupService { private _dataInitStateService = inject(DataInitStateService); private _injector = inject(Injector); private _customThemeService = inject(CustomThemeService); + private _jiraElectronBridge = inject(JiraElectronBridgeService); constructor() { + // Claim the privileged Jira IPC capability here, in trusted startup code, + // before any untrusted renderer code (plugins) is loaded. This one-shot + // ordering — not the main-frame IPC check — is the real security boundary: + // same-origin plugin iframes can reach window.top.ea, so the frame check + // alone is bypassable. Once consumed, consumeJiraApi() returns null to + // everyone else. Do NOT move plugin/3rd-party loading before this call. + this._jiraElectronBridge.initialize(); + // Initialize electron error handler in an effect if (IS_ELECTRON) { effect(() => { - window.ea.on(IPC.ERROR, (ev: IpcRendererEvent, ...args: unknown[]) => { + window.ea.on(IPC.ERROR, (...args: unknown[]) => { const data = args[0] as { error: unknown; stack: unknown; diff --git a/src/app/features/issue/providers/jira/jira-api.service.ts b/src/app/features/issue/providers/jira/jira-api.service.ts index 2c4605f34a..2be89d6d6d 100644 --- a/src/app/features/issue/providers/jira/jira-api.service.ts +++ b/src/app/features/issue/providers/jira/jira-api.service.ts @@ -22,7 +22,6 @@ import { JiraOriginalUser, } from './jira-api-responses'; import { JiraCfg } from './jira.model'; -import { IPC } from '../../../../../../electron/shared-with-frontend/ipc-events.const'; import { SnackService } from '../../../../core/snack/snack.service'; import { HANDLED_ERROR_PROP_STR, IS_ELECTRON } from '../../../../app.constants'; import { from, Observable, of, throwError } from 'rxjs'; @@ -45,7 +44,6 @@ import { T } from '../../../../t.const'; import { getErrorTxt } from '../../../../util/get-error-text'; import { isOnline } from '../../../../util/is-online'; import { GlobalProgressBarService } from '../../../../core-ui/global-progress-bar/global-progress-bar.service'; -import { IpcRendererEvent } from 'electron'; import { SS } from '../../../../core/persistence/storage-keys.const'; import { MatDialog } from '@angular/material/dialog'; import { DialogPromptComponent } from '../../../../ui/dialog-prompt/dialog-prompt.component'; @@ -53,12 +51,18 @@ import { stripTrailing } from '../../../../util/strip-trailing'; import { IS_ANDROID_WEB_VIEW } from '../../../../util/is-android-web-view'; import { formatJiraDate } from '../../../../util/format-jira-date'; import { IssueLog } from '../../../../core/log'; +import { JiraElectronBridgeService } from './jira-electron-bridge.service'; +import { + JiraElectronRequestInit, + JiraRequestMethod, +} from '../../../../../../electron/shared-with-frontend/jira-request.model'; const BLOCK_ACCESS_KEY = 'SUP_BLOCK_JIRA_ACCESS'; const API_VERSION = 'latest'; interface JiraCallbackResponse { requestId?: string; + response?: unknown; error?: { statusCode?: number; status?: number; @@ -106,6 +110,7 @@ export class JiraApiService { private _snackService = inject(SnackService); private _bannerService = inject(BannerService); private _matDialog = inject(MatDialog); + private _jiraElectronBridge = inject(JiraElectronBridgeService); private _requestsLog: { [key: string]: JiraRequestLogItem } = {}; private _isBlockAccess: boolean = !!sessionStorage.getItem(BLOCK_ACCESS_KEY); @@ -123,13 +128,6 @@ export class JiraApiService { ); constructor() { - // set up callback listener for electron - if (IS_ELECTRON) { - window.ea.on(IPC.JIRA_CB_EVENT, (ev: IpcRendererEvent, ...args: unknown[]) => { - this._handleResponse(args[0] as JiraCallbackResponse); - }); - } - this._chromeExtensionInterfaceService.onReady$.subscribe(() => { this._isExtension = true; this._chromeExtensionInterfaceService.addEventListener( @@ -685,10 +683,25 @@ export class JiraApiService { const requestToSend = { requestId, requestInit, url }; if (IS_ELECTRON) { - window.ea.makeJiraRequest({ - ...requestToSend, - jiraCfg, - }); + // Wrap in Promise.resolve().then so a synchronous throw from + // _toElectronRequestInit is routed through _handleResponse (which clears + // the logged request) rather than escaping past the .catch. + void Promise.resolve() + .then(() => + this._jiraElectronBridge.makeRequest({ + requestId, + url, + requestInit: this._toElectronRequestInit(requestInit), + allowSelfSignedCertificate: jiraCfg.isAllowSelfSignedCertificate === true, + }), + ) + .then((response) => this._handleResponse(response)) + .catch((error: unknown) => + this._handleResponse({ + requestId, + error: { message: getErrorTxt(error) }, + }), + ); } else if (this._isExtension) { this._chromeExtensionInterfaceService.dispatchEvent( 'SP_JIRA_REQUEST', @@ -701,10 +714,9 @@ export class JiraApiService { this._globalProgressBarService.countUp(url); return from(promise).pipe( catchError((err) => { - IssueLog.log(err); - IssueLog.log(getErrorTxt(err)); const errTxt = `Jira: ${getErrorTxt(err)}`; const status = extractHttpStatus(err); + IssueLog.err('Jira request failed', { status }); if (!suppressErrorSnack && !(err as { jiraBlocked?: boolean }).jiraBlocked) { this._snackService.open({ type: 'ERROR', msg: errTxt }); } @@ -747,10 +759,9 @@ export class JiraApiService { [HANDLED_ERROR_PROP_STR]: 'Jira: Request timed out', })); } - IssueLog.log(err); - IssueLog.log(getErrorTxt(err)); const errTxt = `Jira: ${getErrorTxt(err)}`; const status = extractHttpStatus(err); + IssueLog.err('Jira request failed', { status }); if (!suppressErrorSnack && !(err as { jiraBlocked?: boolean }).jiraBlocked) { this._snackService.open({ type: 'ERROR', msg: errTxt }); } @@ -785,6 +796,19 @@ export class JiraApiService { }; } + private _toElectronRequestInit(requestInit: RequestInit): JiraElectronRequestInit { + const method = requestInit.method || 'GET'; + if (method !== 'GET' && method !== 'POST' && method !== 'PUT') { + throw new Error('Invalid Jira request method'); + } + + return { + method: method as JiraRequestMethod, + headers: requestInit.headers as Record, + ...(typeof requestInit.body === 'string' ? { body: requestInit.body } : {}), + }; + } + private async _checkSetWonkyCookie(cfg: JiraCfg): Promise { const ssVal = sessionStorage.getItem(SS.JIRA_WONKY_COOKIE); if (ssVal && ssVal.length > 0) { @@ -866,7 +890,10 @@ export class JiraApiService { // NOTE: never log `currentRequest` — it holds `jiraCfg` (plaintext // password) and `requestInit.headers.authorization`. The log history // is exportable, so credentials must never reach it. - IssueLog.err('JIRA_RESPONSE_ERROR', res?.error, res?.requestId); + IssueLog.err('JIRA_RESPONSE_ERROR', { + requestId: res?.requestId, + status: res?.error?.status, + }); // let msg = const blocked = res?.error && isUnauthorizedError(res.error) ? this._blockAccess() : undefined; @@ -878,9 +905,9 @@ export class JiraApiService { // data can be invalid, that's why we check try { currentRequest.resolve(currentRequest.transform(res, currentRequest.jiraCfg)); - } catch (e) { + } catch { // Do not log `currentRequest` (contains jiraCfg + auth header). - IssueLog.err('JIRA_TRANSFORM_ERROR', res?.requestId, e); + IssueLog.err('JIRA_TRANSFORM_ERROR', { requestId: res?.requestId }); this._snackService.open({ type: 'ERROR', msg: T.F.JIRA.S.INVALID_RESPONSE, @@ -990,8 +1017,8 @@ async function streamToJsonIfPossible(stream: ReadableStream): Promise const text = await streamToString(stream); try { return JSON.parse(text); - } catch (e) { - IssueLog.err('Jira: Could not parse response', text); + } catch { + IssueLog.err('Jira: Could not parse response'); return text; } } diff --git a/src/app/features/issue/providers/jira/jira-electron-bridge.service.ts b/src/app/features/issue/providers/jira/jira-electron-bridge.service.ts new file mode 100644 index 0000000000..672526dbfc --- /dev/null +++ b/src/app/features/issue/providers/jira/jira-electron-bridge.service.ts @@ -0,0 +1,53 @@ +import { Injectable } from '@angular/core'; +import { IS_ELECTRON } from '../../../../app.constants'; +import { + JiraElectronApi, + JiraElectronRequest, + JiraElectronResponse, + JiraImageAuthConfig, +} from '../../../../../../electron/shared-with-frontend/jira-request.model'; + +@Injectable({ providedIn: 'root' }) +export class JiraElectronBridgeService { + #api: JiraElectronApi | null | undefined; + // Whether image auth has been set up in the main process. Lets the (common) + // non-Jira path skip a no-op clear IPC round-trip on every detail-panel + // open/close. + #hasImgHeaders = false; + + initialize(): void { + if (this.#api !== undefined) { + return; + } + + // Claim the one-shot capability during trusted app startup, before plugin + // code is loaded into the renderer. + this.#api = IS_ELECTRON ? window.ea.consumeJiraApi() : null; + } + + makeRequest(request: JiraElectronRequest): Promise { + this.initialize(); + if (!this.#api) { + return Promise.reject(new Error('Jira Electron API is unavailable')); + } + return this.#api.makeRequest(request); + } + + setupImgHeaders(config: JiraImageAuthConfig): Promise { + this.initialize(); + if (!this.#api) { + return Promise.resolve(); + } + this.#hasImgHeaders = true; + return this.#api.setupImgHeaders(config); + } + + clearImgHeaders(): Promise { + this.initialize(); + if (!this.#api || !this.#hasImgHeaders) { + return Promise.resolve(); + } + this.#hasImgHeaders = false; + return this.#api.clearImgHeaders(); + } +} diff --git a/src/app/features/tasks/task-detail-panel/task-detail-panel.component.ts b/src/app/features/tasks/task-detail-panel/task-detail-panel.component.ts index 910e1869fa..34deb7d896 100644 --- a/src/app/features/tasks/task-detail-panel/task-detail-panel.component.ts +++ b/src/app/features/tasks/task-detail-panel/task-detail-panel.component.ts @@ -88,6 +88,7 @@ import { clipboardHasText } from '../../../util/clipboard-has-text'; import { checkKeyCombo } from '../../../util/check-key-combo'; import { IS_MAC } from '../../../util/is-mac'; import { ClipboardImageService } from '../../../core/clipboard-image/clipboard-image.service'; +import { JiraElectronBridgeService } from '../../issue/providers/jira/jira-electron-bridge.service'; import { DropPasteIcons } from '../../../core/drop-paste-input/drop-paste.model'; import { AddSubtaskInputComponent, @@ -140,6 +141,7 @@ export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestro private _clipboardImageService = inject(ClipboardImageService); private _globalConfigService = inject(GlobalConfigService); private _issueService = inject(IssueService); + private _jiraElectronBridge = inject(JiraElectronBridgeService); private _taskRepeatCfgService = inject(TaskRepeatCfgService); private _matDialog = inject(MatDialog); private _store = inject(Store); @@ -441,8 +443,8 @@ export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestro ) .pipe( // Orphan issueProviderId — see #7135. - catchError((err: unknown) => { - IssueLog.warn('Jira header setup skipped', err); + catchError(() => { + IssueLog.warn('Jira header setup skipped'); return of(null); }), ) @@ -452,7 +454,18 @@ export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestro ) .subscribe((jiraCfg) => { if (jiraCfg?.isEnabled) { - window.ea.jiraSetupImgHeaders({ jiraCfg }); + void this._jiraElectronBridge + .setupImgHeaders({ + host: jiraCfg.host, + userName: jiraCfg.userName, + password: jiraCfg.password, + usePAT: jiraCfg.usePAT === true, + }) + .catch(() => IssueLog.err('Jira image authentication setup failed')); + } else { + void this._jiraElectronBridge + .clearImgHeaders() + .catch(() => IssueLog.err('Jira image authentication cleanup failed')); } }) : null; @@ -583,6 +596,11 @@ export class TaskDetailPanelComponent implements OnInit, AfterViewInit, OnDestro } ngOnDestroy(): void { + if (IS_ELECTRON) { + void this._jiraElectronBridge + .clearImgHeaders() + .catch(() => IssueLog.err('Jira image authentication cleanup failed')); + } if (window.history.state?.[HISTORY_STATE.TASK_DETAIL_PANEL]) { window.history.back(); } diff --git a/src/app/imex/sync/oauth-callback-handler.service.ts b/src/app/imex/sync/oauth-callback-handler.service.ts index c55cb0a6d2..490ecbc17b 100644 --- a/src/app/imex/sync/oauth-callback-handler.service.ts +++ b/src/app/imex/sync/oauth-callback-handler.service.ts @@ -77,7 +77,7 @@ export class OAuthCallbackHandlerService implements OnDestroy { } private _setupElectronOAuthListener(): void { - window.ea.on(IPC.OAUTH_CALLBACK, (_event, payload) => { + window.ea.on(IPC.OAUTH_CALLBACK, (payload) => { if (this._isDestroyed) { return; }