diff --git a/electron/local-rest-api.ts b/electron/local-rest-api.ts index 6cfff90f56..4e02f7df03 100644 --- a/electron/local-rest-api.ts +++ b/electron/local-rest-api.ts @@ -8,6 +8,7 @@ import { GlobalConfigState } from '../src/app/features/config/global-config.mode import { LOCAL_REST_API_HOST, LOCAL_REST_API_MAX_BODY_BYTES, + LOCAL_REST_API_MAX_CONCURRENT_REQUESTS, LOCAL_REST_API_PORT, LOCAL_REST_API_TIMEOUT_MS, LocalRestApiRequestPayload, @@ -107,10 +108,41 @@ const handleResponse = (_event: unknown, payload: LocalRestApiResponsePayload): pending.resolve(payload); }; +const ALLOWED_HOSTS = new Set([ + `${LOCAL_REST_API_HOST}:${LOCAL_REST_API_PORT}`, + `localhost:${LOCAL_REST_API_PORT}`, + LOCAL_REST_API_HOST, + 'localhost', +]); + const handleHttpRequest = async ( req: IncomingMessage, res: ServerResponse, ): Promise => { + // Block DNS rebinding: reject requests with unexpected Host headers + const host = req.headers.host; + if (!host || !ALLOWED_HOSTS.has(host)) { + writeJson(res, 403, { + ok: false, + error: { + code: 'FORBIDDEN', + message: 'Invalid Host header', + }, + }); + return; + } + + if (pendingRequests.size >= LOCAL_REST_API_MAX_CONCURRENT_REQUESTS) { + writeJson(res, 429, { + ok: false, + error: { + code: 'TOO_MANY_REQUESTS', + message: `Too many concurrent requests (limit: ${LOCAL_REST_API_MAX_CONCURRENT_REQUESTS})`, + }, + }); + return; + } + const requestUrl = new URL(req.url ?? '/', `http://${LOCAL_REST_API_HOST}`); const method = req.method ?? 'GET'; diff --git a/electron/shared-with-frontend/local-rest-api.model.ts b/electron/shared-with-frontend/local-rest-api.model.ts index 9106678f11..872a4f184c 100644 --- a/electron/shared-with-frontend/local-rest-api.model.ts +++ b/electron/shared-with-frontend/local-rest-api.model.ts @@ -2,6 +2,7 @@ export const LOCAL_REST_API_HOST = '127.0.0.1'; export const LOCAL_REST_API_PORT = 3876; export const LOCAL_REST_API_TIMEOUT_MS = 15000; export const LOCAL_REST_API_MAX_BODY_BYTES = 1024 * 1024; +export const LOCAL_REST_API_MAX_CONCURRENT_REQUESTS = 50; export interface LocalRestApiRequestPayload { requestId: string; diff --git a/src/app/core/electron/local-rest-api-handler.service.spec.ts b/src/app/core/electron/local-rest-api-handler.service.spec.ts index edf9d8cb3e..75884b19a6 100644 --- a/src/app/core/electron/local-rest-api-handler.service.spec.ts +++ b/src/app/core/electron/local-rest-api-handler.service.spec.ts @@ -339,6 +339,29 @@ describe('LocalRestApiHandlerService', () => { expect(response.body.ok).toBe(false); expect(response.status).toBe(400); }); + + it('should strip disallowed fields from the body', async () => { + Object.defineProperty(taskServiceMock, 'getByIdOnce$', { + get: () => (_id: string) => of(createMockTask('new-task-id')), + }); + + await sendRequestAndWait( + createRequest('POST', '/tasks', { + body: { + title: 'New Task', + notes: 'allowed', + id: 'injected-id', + subTaskIds: ['injected'], + parentId: 'injected-parent', + }, + }), + ); + + expect(taskServiceMock.add).toHaveBeenCalledWith('New Task', false, { + title: 'New Task', + notes: 'allowed', + }); + }); }); describe('GET /tasks/:id', () => { @@ -408,6 +431,28 @@ describe('LocalRestApiHandlerService', () => { expect(response.body.ok).toBe(false); expect(response.status).toBe(400); }); + + it('should strip disallowed fields from PATCH body', async () => { + const mockTask = createMockTask('task-1'); + Object.defineProperty(taskServiceMock, 'getByIdOnce$', { + get: () => (_id: string) => of(mockTask), + }); + + await sendRequestAndWait( + createRequest('PATCH', '/tasks/task-1', { + body: { + title: 'Updated', + id: 'injected-id', + subTaskIds: ['injected'], + parentId: 'injected-parent', + }, + }), + ); + + expect(taskServiceMock.update).toHaveBeenCalledWith('task-1', { + title: 'Updated', + }); + }); }); describe('DELETE /tasks/:id', () => { diff --git a/src/app/core/electron/local-rest-api-handler.service.ts b/src/app/core/electron/local-rest-api-handler.service.ts index c0ab246464..09be7af99d 100644 --- a/src/app/core/electron/local-rest-api-handler.service.ts +++ b/src/app/core/electron/local-rest-api-handler.service.ts @@ -13,6 +13,30 @@ import { const isRecord = (value: unknown): value is Record => typeof value === 'object' && value !== null && !Array.isArray(value); +/** Only these fields may be set via the REST API to prevent state corruption. */ +const ALLOWED_TASK_FIELDS = new Set([ + 'title', + 'notes', + 'isDone', + 'timeEstimate', + 'timeSpent', + 'projectId', + 'tagIds', + 'dueDay', + 'dueWithTime', + 'plannedAt', +]); + +const pickAllowedFields = (body: Record): Partial => { + const result: Record = {}; + for (const key of Object.keys(body)) { + if (ALLOWED_TASK_FIELDS.has(key)) { + result[key] = body[key]; + } + } + return result as Partial; +}; + const getQueryParam = ( query: Record, key: string, @@ -77,7 +101,7 @@ export class LocalRestApiHandlerService { private _isInitialized = false; init(): void { - if (this._isInitialized) { + if (this._isInitialized || !window.ea?.onLocalRestApiRequest) { return; } this._isInitialized = true; @@ -222,7 +246,11 @@ export class LocalRestApiHandlerService { const projectId = getQueryParam(query, 'projectId'); const tagId = getQueryParam(query, 'tagId'); const includeDone = getQueryParamAsBoolean(query, 'includeDone', false); - const source = (getQueryParam(query, 'source') as TaskSource) || 'active'; + const VALID_SOURCES: TaskSource[] = ['active', 'archived', 'all']; + const rawSource = getQueryParam(query, 'source') || 'active'; + const source: TaskSource = VALID_SOURCES.includes(rawSource as TaskSource) + ? (rawSource as TaskSource) + : 'active'; let tasks: Task[]; @@ -271,9 +299,8 @@ export class LocalRestApiHandlerService { } const title = body.title.trim(); - const additionalFields = { ...body }; - delete additionalFields.title; - const taskId = this._taskService.add(title, false, additionalFields as Partial); + const additionalFields = pickAllowedFields(body); + const taskId = this._taskService.add(title, false, additionalFields); const createdTask = await this._getTaskById(taskId); return createSuccessResponse(requestId, 201, createdTask); @@ -311,7 +338,7 @@ export class LocalRestApiHandlerService { return createErrorResponse(requestId, 404, 'TASK_NOT_FOUND', 'Task not found'); } - this._taskService.update(taskId, body as Partial); + this._taskService.update(taskId, pickAllowedFields(body)); return createSuccessResponse(requestId, 200, await this._getTaskById(taskId)); } diff --git a/src/app/features/calendar-integration/calendar-integration.service.ts b/src/app/features/calendar-integration/calendar-integration.service.ts index 712de67d2e..6964825b9f 100644 --- a/src/app/features/calendar-integration/calendar-integration.service.ts +++ b/src/app/features/calendar-integration/calendar-integration.service.ts @@ -374,7 +374,12 @@ export class CalendarIntegrationService { // filter out cached past entries .map((provider) => ({ ...provider, - items: provider.items.filter((item) => item.start + item.duration >= now), + items: provider.items + .filter((item) => item.start + item.duration >= now) + // Backfill issueProviderKey for events cached before it became required + .map((item) => + item.issueProviderKey ? item : { ...item, issueProviderKey: 'ICAL' }, + ), })) ); } diff --git a/src/app/features/calendar-integration/hidden-calendar-events.service.ts b/src/app/features/calendar-integration/hidden-calendar-events.service.ts index 2746918dad..428f8ede92 100644 --- a/src/app/features/calendar-integration/hidden-calendar-events.service.ts +++ b/src/app/features/calendar-integration/hidden-calendar-events.service.ts @@ -8,6 +8,8 @@ import { getCalendarEventIdCandidates } from './get-calendar-event-id-candidates providedIn: 'root', }) export class HiddenCalendarEventsService { + private static readonly _MAX_HIDDEN_EVENT_IDS = 500; + readonly hiddenEventIds$ = new BehaviorSubject(this._loadFromStorage()); hideEvent(calEv: CalendarIntegrationEvent): void { @@ -18,7 +20,12 @@ export class HiddenCalendarEventsService { return; } const current = this.hiddenEventIds$.getValue(); - const updated = [...current, ...idsToAdd.filter((id) => !current.includes(id))]; + let updated = [...current, ...idsToAdd.filter((id) => !current.includes(id))]; + if (updated.length > HiddenCalendarEventsService._MAX_HIDDEN_EVENT_IDS) { + updated = updated.slice( + updated.length - HiddenCalendarEventsService._MAX_HIDDEN_EVENT_IDS, + ); + } this.hiddenEventIds$.next(updated); this._saveToStorage(updated); } diff --git a/src/app/features/calendar-integration/store/calendar-integration.effects.ts b/src/app/features/calendar-integration/store/calendar-integration.effects.ts index c3e1e31d14..47bf23a527 100644 --- a/src/app/features/calendar-integration/store/calendar-integration.effects.ts +++ b/src/app/features/calendar-integration/store/calendar-integration.effects.ts @@ -17,7 +17,7 @@ import { isValidUrl } from '../../../util/is-valid-url'; import { getPluralKey } from '../../../util/get-plural-key'; import { distinctUntilChangedObject } from '../../../util/distinct-until-changed-object'; import { selectCalendarProviders } from '../../issue/store/issue-provider.selectors'; -import { IssueProviderCalendar } from '../../issue/issue.model'; +import { IssueProviderCalendar, IssueProviderKey } from '../../issue/issue.model'; import { IssueService } from '../../issue/issue.service'; import { DateService } from '../../../core/date/date.service'; import { TaskService } from '../../tasks/task.service'; @@ -97,7 +97,8 @@ export class CalendarIntegrationEffects { !matchesAnyCalendarEventId(calEv, allIssueIds) ) { this._issueService.addTaskFromIssue({ - issueProviderKey: 'ICAL', + issueProviderKey: + (calEv.issueProviderKey as IssueProviderKey) || 'ICAL', issueProviderId: calProvider.id, issueDataReduced: calEv, // from this context we should always add to the default project rather than current context @@ -235,7 +236,7 @@ export class CalendarIntegrationEffects { fn: () => { this._skipEv(calEv); this._issueService.addTaskFromIssue({ - issueProviderKey: 'ICAL', + issueProviderKey: (calEv.issueProviderKey as IssueProviderKey) || 'ICAL', issueProviderId: calProvider.id, issueDataReduced: calEv, // from the banner we should always add to the default project rather than current context diff --git a/src/app/features/calendar-integration/time-block/time-block-delete-sidecar.service.ts b/src/app/features/calendar-integration/time-block/time-block-delete-sidecar.service.ts index 25909a79eb..a2a2e03197 100644 --- a/src/app/features/calendar-integration/time-block/time-block-delete-sidecar.service.ts +++ b/src/app/features/calendar-integration/time-block/time-block-delete-sidecar.service.ts @@ -10,7 +10,7 @@ export class TimeBlockDeleteSidecarService { private _pendingTaskIds: string[] = []; set(taskIds: string[]): void { - this._pendingTaskIds = taskIds; + this._pendingTaskIds.push(...taskIds); } consume(): string[] { diff --git a/src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.ts b/src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.ts index a6d6bc271d..aedfbb9967 100644 --- a/src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.ts +++ b/src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.ts @@ -480,7 +480,7 @@ export class DialogEditIssueProviderComponent { const pluginConfig = (model as { pluginConfig?: Record }) .pluginConfig; if (!pluginConfig) return model; - // Deep-clone pluginConfig to avoid mutating store state + // Shallow-clone pluginConfig to avoid mutating store state const migrated = { ...pluginConfig }; // Migrate old single-calendar calendarId to multi-calendar config shape if ( diff --git a/src/app/features/schedule/ical/get-relevant-events-from-ical.ts b/src/app/features/schedule/ical/get-relevant-events-from-ical.ts index 1bde0ceb12..8fea589ad9 100644 --- a/src/app/features/schedule/ical/get-relevant-events-from-ical.ts +++ b/src/app/features/schedule/ical/get-relevant-events-from-ical.ts @@ -166,7 +166,12 @@ const generateGoogleCalendarEventUrl = ( try { const cleanId = eventId.replace(/@(group\.calendar\.)?google\.com.*$/, ''); const raw = `${cleanId} ${calendarEmail}`; - const eid = btoa(String.fromCharCode(...new TextEncoder().encode(raw))); + const bytes = new TextEncoder().encode(raw); + let binary = ''; + for (let i = 0; i < bytes.length; i++) { + binary += String.fromCharCode(bytes[i]); + } + const eid = btoa(binary); return `https://calendar.google.com/calendar/event?eid=${eid}`; } catch { return undefined; diff --git a/src/app/plugins/issue-provider/plugin-http.service.spec.ts b/src/app/plugins/issue-provider/plugin-http.service.spec.ts index 7843244566..9e3751433a 100644 --- a/src/app/plugins/issue-provider/plugin-http.service.spec.ts +++ b/src/app/plugins/issue-provider/plugin-http.service.spec.ts @@ -122,7 +122,7 @@ describe('PluginHttpService', () => { it('should block 169.254.x.x link-local range', async () => { const http = service.createHttpHelper(noopHeaders); await expectAsync(http.get('https://169.254.169.254/api')).toBeRejectedWithError( - /private\/local/, + /cloud metadata endpoints/, ); }); @@ -130,7 +130,7 @@ describe('PluginHttpService', () => { const http = service.createHttpHelper(noopHeaders); await expectAsync( http.get('https://metadata.google.internal/computeMetadata/v1/'), - ).toBeRejectedWithError(/private\/local/); + ).toBeRejectedWithError(/cloud metadata endpoints/); }); it('should block IPv6 ULA (fc00::)', async () => { diff --git a/src/app/plugins/issue-provider/plugin-http.service.ts b/src/app/plugins/issue-provider/plugin-http.service.ts index ab9110ffa9..1e9eeef5cb 100644 --- a/src/app/plugins/issue-provider/plugin-http.service.ts +++ b/src/app/plugins/issue-provider/plugin-http.service.ts @@ -15,6 +15,27 @@ const BLOCKED_HOSTNAMES = new Set([ 'metadata.google.internal', ]); +// Cloud metadata endpoints that must ALWAYS be blocked, even when allowPrivateNetwork is true +const ALLOWED_HTTP_METHODS = new Set([ + 'GET', + 'POST', + 'PUT', + 'PATCH', + 'DELETE', + 'HEAD', + 'OPTIONS', + 'PROPFIND', + 'REPORT', + 'MKCALENDAR', + 'PROPPATCH', +]); + +const CLOUD_METADATA_HOSTNAMES = new Set([ + '169.254.169.254', // AWS/Azure instance metadata + 'metadata.google.internal', // GCP instance metadata + 'fd00:ec2::254', // AWS IMDSv2 IPv6 +]); + const isPrivateIp = (hostname: string): boolean => { // IPv4 private ranges if ( @@ -83,6 +104,10 @@ export class PluginHttpService { getHeaders: () => Record | Promise>, allowPrivateNetwork: boolean, ): Promise { + const upperMethod = method.toUpperCase(); + if (!ALLOWED_HTTP_METHODS.has(upperMethod)) { + throw new Error(`[PluginHttp] HTTP method not allowed: ${method}`); + } this._validateUrl(url, allowPrivateNetwork); const authHeaders = await Promise.resolve(getHeaders()); @@ -103,7 +128,7 @@ export class PluginHttpService { const responseType = options?.responseType === 'text' ? 'text' : 'json'; const obs$ = this._http - .request(method, url, { + .request(upperMethod, url, { body, headers: new HttpHeaders(mergedHeaders), params, @@ -124,9 +149,17 @@ export class PluginHttpService { if (parsed.protocol !== 'https:' && parsed.protocol !== 'http:') { throw new Error(`[PluginHttp] Unsupported URL scheme: ${parsed.protocol}`); } + // Strip brackets from IPv6 addresses (URL.hostname may include them) + const hostname = parsed.hostname.replace(/^\[|\]$/g, ''); + + // Always block cloud metadata endpoints (SSRF protection), even when allowPrivateNetwork is true + if (CLOUD_METADATA_HOSTNAMES.has(hostname.toLowerCase())) { + throw new Error( + `[PluginHttp] Requests to cloud metadata endpoints are not allowed: ${hostname}`, + ); + } + if (!allowPrivateNetwork) { - // Strip brackets from IPv6 addresses (URL.hostname may include them) - const hostname = parsed.hostname.replace(/^\[|\]$/g, ''); if (BLOCKED_HOSTNAMES.has(hostname) || isPrivateIp(hostname)) { throw new Error( `[PluginHttp] Requests to private/local networks are not allowed: ${hostname}`,