Merge master and address PR review findings

Harden LocalFile saves, keep legacy file image IPC removed, avoid pre-save cached-image deletion, and add user-facing reselect warnings for LocalFile/background images.
This commit is contained in:
Johannes Millan 2026-06-10 19:17:54 +02:00
commit ecd5ec7077
25 changed files with 290 additions and 146 deletions

View file

@ -66,18 +66,28 @@ test.describe('Add subtask with detail panel open', () => {
}) => {
const parent = await addParentAndOpenPanel(page, workViewPage, taskPage);
// Let the panel's deferred open-time auto-focus land first, otherwise it can
// fire *after* we move focus to the row below and silently steal it back.
await expect
.poll(async () =>
page.evaluate(() => !!document.activeElement?.closest('task-detail-panel')),
)
.toBe(true);
// Put focus back on the main-list task row (as if the user clicked or
// arrow-navigated it with the panel open) — the path that goes through the
// global shortcut handler and previously left the new subtask unfocused.
await parent.focus();
await expect
.poll(async () =>
page.evaluate(() => {
// Re-apply focus on each retry: under load the row may not accept focus on
// the first try, and .poll() alone would never re-focus it.
await expect(async () => {
await parent.focus();
expect(
await page.evaluate(() => {
const a = document.activeElement as HTMLElement | null;
return a?.tagName?.toLowerCase() === 'task' && !a.closest('task-detail-panel');
}),
)
.toBe(true);
).toBe(true);
}).toPass();
await page.keyboard.press('a');

View file

@ -95,13 +95,11 @@ export interface ElectronAPI {
/**
* Open the native image picker, copy the chosen file into the main-owned
* cache, and return an opaque id. The renderer never holds the absolute
* path. Pass `replacesId` to garbage-collect the previous cached image
* the renderer is about to overwrite in its config. Returns null when the
* user cancels or the picked file fails validation.
* path. Returns null when the user cancels and a safe Error when the picked
* file fails validation/import. Old cached images are not deleted here,
* because the surrounding config save may still fail or be cancelled.
*/
imagePickAndImport(args?: {
replacesId?: string;
}): Promise<{ id: string; mimeType: string } | null | Error>;
imagePickAndImport(): Promise<{ id: string; mimeType: string } | null | Error>;
/**
* Resolve a cached image id to a `data:` URL the renderer can use as a

View file

@ -124,6 +124,40 @@ test('FILE_SYNC_SAVE rejects an absolute renderer-supplied relativePath', async
assert.ok(result instanceof Error);
});
test('FILE_SYNC_SAVE rejects the sync root and does not create a sibling temp file', async () => {
await configureSyncFolder(externalDir);
const siblingTemp = `${externalDir}.tmp`;
const result = await handlers['FILE_SYNC_SAVE'](
{},
{ relativePath: '', dataStr: 'x', localRev: null },
);
assert.ok(result instanceof Error);
assert.equal(fs.existsSync(siblingTemp), false, 'must not write outside sync root');
});
test('FILE_SYNC_SAVE does not write through a predictable .tmp symlink', async () => {
await configureSyncFolder(externalDir);
const outside = path.join(userDataDir, 'outside-target');
fs.writeFileSync(outside, 'keep');
try {
fs.symlinkSync(outside, path.join(externalDir, 'main.json.tmp'), 'file');
} catch (e) {
if (process.platform === 'win32') {
return;
}
throw e;
}
const result = await handlers['FILE_SYNC_SAVE'](
{},
{ relativePath: 'main.json', dataStr: '{"ok":1}', localRev: null },
);
assert.ok(!(result instanceof Error), 'normal save should still work');
assert.equal(fs.readFileSync(outside, 'utf8'), 'keep');
assert.equal(fs.readFileSync(path.join(externalDir, 'main.json'), 'utf8'), '{"ok":1}');
});
test('PICK_DIRECTORY rejects a folder inside userData', async () => {
// A folder equal to (or inside) userData is rejected at pick time so the
// user never ends up with a "configured" folder that resolveSyncPath then
@ -307,7 +341,7 @@ test('IMAGE_PICK_AND_IMPORT returns a safe Error when validation fails (no path
assert.equal(result.stack, undefined, 'stack stripped, no main-bundle paths');
});
test('IMAGE_PICK_AND_IMPORT garbage-collects the prior cached image on replace', async () => {
test('IMAGE_PICK_AND_IMPORT does not delete the prior cached image during replace', async () => {
const oldPicked = path.join(externalDir, 'old.png');
fs.writeFileSync(oldPicked, 'oldbytes');
nextDialogResult = { canceled: false, filePaths: [oldPicked] };
@ -319,13 +353,14 @@ test('IMAGE_PICK_AND_IMPORT garbage-collects the prior cached image on replace',
const newPicked = path.join(externalDir, 'new.png');
fs.writeFileSync(newPicked, 'newbytes');
nextDialogResult = { canceled: false, filePaths: [newPicked] };
const newImport = await handlers['IMAGE_PICK_AND_IMPORT'](
{},
{ replacesId: oldImport.id },
);
const newImport = await handlers['IMAGE_PICK_AND_IMPORT']({});
assert.ok(newImport);
assert.notEqual(newImport.id, oldImport.id);
assert.equal(fs.existsSync(oldCachePath), false, 'old file should be removed');
assert.equal(
fs.existsSync(oldCachePath),
true,
'old file must remain until config persistence makes it unreachable',
);
});
test('IMAGE_CACHE_GET_DATA_URL returns null for unknown ids', async () => {

View file

@ -1,5 +1,6 @@
import { IPC } from './shared-with-frontend/ipc-events.const';
import { SimpleStoreKey } from './shared-with-frontend/simple-store.const';
import { randomBytes } from 'crypto';
import {
readdirSync,
readFileSync,
@ -9,13 +10,14 @@ import {
writeFileSync,
unlinkSync,
} from 'fs';
import * as path from 'path';
import { error, log } from 'electron-log/main';
import { app, dialog, ipcMain } from 'electron';
import { getWin } from './main-window';
import { resolveSyncPath } from './sync-path-resolver';
import { resolveSyncPath, type ResolvedSyncPath } from './sync-path-resolver';
import { loadSimpleStoreAll, saveSimpleStore } from './simple-store';
import { assertPathOutside } from './file-path-guard';
import { getImageDataUrl, importImage, removeCachedImage } from './image-cache';
import { getImageDataUrl, importImage } from './image-cache';
// SECURITY: file-sync must never read/write/list inside the app's private dir,
// which holds settings/grants/db — touching it is a privilege-escalation
@ -68,12 +70,55 @@ const setSyncFolderPath = async (rawPath: string): Promise<string> => {
// so the five handlers below don't repeat the same incantation. Returns the
// absolute path on success; throws PathNotAllowedError on rejection (the
// existing handler try/catch funnels both into a safe IPC error).
const _resolveRelative = async (relativePath: string | undefined): Promise<string> =>
const _resolveRelative = async (
relativePath: string | undefined,
): Promise<ResolvedSyncPath> =>
resolveSyncPath(
(await getSyncFolderPath()) ?? undefined,
relativePath ?? '',
getAppPrivateDir(),
).absolutePath;
);
const _pathNotAllowed = (): Error => {
const e = new Error('Path not allowed for the sync folder');
e.name = 'PathNotAllowedError';
delete e.stack;
return e;
};
const _isEnoent = (e: unknown): boolean =>
typeof e === 'object' &&
e !== null &&
'code' in e &&
(e as { code?: unknown }).code === 'ENOENT';
const _assertSaveTargetIsFilePath = (resolved: ResolvedSyncPath): void => {
if (resolved.isRoot) {
throw _pathNotAllowed();
}
try {
if (statSync(resolved.absolutePath).isDirectory()) {
throw _pathNotAllowed();
}
} catch (e) {
if (!_isEnoent(e)) {
throw e;
}
}
};
const _resolveTempPathForSave = (resolved: ResolvedSyncPath): string => {
const tempName = [
`.${path.basename(resolved.absolutePath)}`,
process.pid,
Date.now(),
randomBytes(8).toString('hex'),
'tmp',
].join('.');
const candidate = path.join(path.dirname(resolved.absolutePath), tempName);
const tempRelative = path.relative(resolved.root, candidate);
return resolveSyncPath(resolved.root, tempRelative, getAppPrivateDir()).absolutePath;
};
export const initLocalFileSyncAdapter = (): void => {
ipcMain.handle(
@ -90,8 +135,11 @@ export const initLocalFileSyncAdapter = (): void => {
localRev: string | null;
},
): Promise<string | Error> => {
let tempPath: string | null = null;
try {
const filePath = await _resolveRelative(relativePath);
const resolved = await _resolveRelative(relativePath);
_assertSaveTargetIsFilePath(resolved);
const filePath = resolved.absolutePath;
log(IPC.FILE_SYNC_SAVE, {
dataLength: dataStr.length,
hasData: dataStr.length > 0,
@ -100,12 +148,20 @@ export const initLocalFileSyncAdapter = (): void => {
// Atomic write: write to temp file first, then rename.
// renameSync is atomic on ext4/APFS/NTFS, so a crash mid-write
// won't corrupt the original file.
const tempPath = filePath + '.tmp';
writeFileSync(tempPath, dataStr);
tempPath = _resolveTempPathForSave(resolved);
writeFileSync(tempPath, dataStr, { encoding: 'utf8', flag: 'wx' });
renameSync(tempPath, filePath);
tempPath = null;
return getRev(filePath);
} catch (e) {
if (tempPath) {
try {
unlinkSync(tempPath);
} catch {
// Best-effort cleanup; the safe IPC error below is the important part.
}
}
error('Local file sync save failed', getSafeErrorMeta(e));
return createSafeIpcError(IPC.FILE_SYNC_SAVE, e);
}
@ -125,7 +181,7 @@ export const initLocalFileSyncAdapter = (): void => {
},
): Promise<{ rev: string; dataStr: string | undefined } | Error> => {
try {
const filePath = await _resolveRelative(relativePath);
const filePath = (await _resolveRelative(relativePath)).absolutePath;
log(IPC.FILE_SYNC_LOAD, {
hasLocalRev: !!localRev,
});
@ -155,7 +211,7 @@ export const initLocalFileSyncAdapter = (): void => {
},
): Promise<void | Error> => {
try {
const filePath = await _resolveRelative(relativePath);
const filePath = (await _resolveRelative(relativePath)).absolutePath;
log(IPC.FILE_SYNC_REMOVE);
unlinkSync(filePath);
return;
@ -179,7 +235,7 @@ export const initLocalFileSyncAdapter = (): void => {
try {
// Default to the sync root itself (relativePath = '') so the legacy
// "is the configured sync folder reachable?" check still works.
const dirPath = await _resolveRelative(relativePath);
const dirPath = (await _resolveRelative(relativePath)).absolutePath;
const dirEntries = readdirSync(dirPath);
log(IPC.CHECK_DIR_EXISTS, {
dirEntryCount: dirEntries.length,
@ -208,7 +264,7 @@ export const initLocalFileSyncAdapter = (): void => {
},
): Promise<string[] | Error> => {
try {
const dirPath = await _resolveRelative(relativePath);
const dirPath = (await _resolveRelative(relativePath)).absolutePath;
return readdirSync(dirPath);
} catch (e) {
error('Local file sync list files failed', getSafeErrorMeta(e));
@ -256,10 +312,7 @@ export const initLocalFileSyncAdapter = (): void => {
ipcMain.handle(
IPC.IMAGE_PICK_AND_IMPORT,
async (
_,
args?: { replacesId?: string },
): Promise<{ id: string; mimeType: string } | null | Error> => {
async (): Promise<{ id: string; mimeType: string } | null | Error> => {
// SECURITY: the dialog + import are atomic and run together in main.
// The renderer never holds the absolute path — it cannot trigger an
// image read without a user clicking through the native picker.
@ -294,13 +347,6 @@ export const initLocalFileSyncAdapter = (): void => {
// (from a raw `e.stack`) are not leaked.
throw new Error('Selected image could not be imported');
}
if (typeof args?.replacesId === 'string' && args.replacesId) {
// GC: drop the file the renderer is about to overwrite in its config.
// Renderer-supplied id is opaque; worst case is removing a cached
// image that wasn't actually orphaned, which the user can recover by
// re-picking.
await removeCachedImage(args.replacesId);
}
return imported;
} catch (e) {
error('Image pick-and-import failed', getSafeErrorMeta(e));

View file

@ -90,8 +90,8 @@ const ea: ElectronAPI = {
filters?: { name: string; extensions: string[] }[];
}) => _invoke('SHOW_OPEN_DIALOG', options) as Promise<string[] | undefined>,
imagePickAndImport: (args) =>
_invoke('IMAGE_PICK_AND_IMPORT', args) as Promise<
imagePickAndImport: () =>
_invoke('IMAGE_PICK_AND_IMPORT') as Promise<
| {
id: string;
mimeType: string;

View file

@ -37,9 +37,8 @@ export class LocalFileSyncElectron extends LocalFileSyncBase {
if (!folderPath) {
// Migration breadcrumb (#8228): a pre-fix install may still have a
// syncFolderPath in the renderer credential store. Log so the dev
// console shows why isReady=false until the user re-picks. A
// user-facing snack is the right shape but i18n infrastructure
// belongs in a UX follow-up, not the security fix.
// console shows why isReady=false until the user re-picks. The host
// app surfaces a sticky user-facing warning from its provider manager.
const legacyCfg = await this.privateCfg.load().catch(() => null);
if (legacyCfg?.syncFolderPath) {
this.logger.critical(

View file

@ -174,6 +174,7 @@ export class AppComponent implements OnDestroy, AfterViewInit {
readonly _store = inject(Store);
private _sectionService = inject(SectionService);
private _browserTitleService = inject(BrowserTitleService);
private _hasShownLegacyFileBgSnack = false;
readonly T = T;
readonly TODAY_TAG_ID = TODAY_TAG.id;
readonly isShowMobileButtonNav = this.layoutService.isShowMobileBottomNav;
@ -283,6 +284,18 @@ export class AppComponent implements OnDestroy, AfterViewInit {
effect(() => {
const bgImage = this._globalThemeService.backgroundImg();
const currentRequestId = ++bgResolveRequestId;
if (
typeof bgImage === 'string' &&
bgImage.startsWith('file://') &&
!this._hasShownLegacyFileBgSnack
) {
this._hasShownLegacyFileBgSnack = true;
this._snackService.open({
msg: T.F.PROJECT.FORM_THEME.S_BACKGROUND_IMAGE_RESELECT_REQUIRED,
type: 'WARNING',
config: { duration: 0 },
});
}
void resolveBgImageToDataUrl(bgImage).then((resolved) => {
// Ignore stale resolutions when the source changed mid-read.
if (currentRequestId === bgResolveRequestId) {

View file

@ -46,9 +46,8 @@ export const resolveBgImageToDataUrl = async (
if (bgImage.startsWith('file://')) {
// Legacy shape — picker now produces `image:<id>`. No safe IPC to read
// an arbitrary renderer-supplied path remains; user must re-pick.
// TODO(#8228 follow-up): one-time snack to nudge re-pick. Translation
// infrastructure is the only blocker; for now we log so devs notice.
// Static message only — never log the URL (it's user content).
// Static message only — never log the URL (it's user content). AppComponent
// shows the user-facing re-pick snack once per session.
if (!_hasWarnedAboutLegacyFileUrl) {
_hasWarnedAboutLegacyFileUrl = true;
Log.warn(

View file

@ -342,6 +342,11 @@ describe('JiraApiService', () => {
beforeEach(() => {
service = setupService(new Subject<boolean>());
fetchSpy = spyOn(window, 'fetch');
// findAutoImportIssues$ → _sendRequest$ bails out with "Jira Offline" when
// !isOnline(). These tests assert pagination, not connectivity, so pin the
// online state instead of depending on the runner's real navigator.onLine
// (false under headless/offline CI). Jasmine restores the spy per test.
spyOnProperty(navigator, 'onLine').and.returnValue(true);
});
it('fetches all Jira Cloud search/jql pages via nextPageToken', (done) => {

View file

@ -6,8 +6,8 @@
>
@if (note.imgUrl) {
<img
[enlargeImg]="note.imgUrl"
[src]="note.imgUrl"
[enlargeImg]="safeImgUrl"
[src]="safeImgUrl"
(longPress)="editFullscreen($event)"
class="note-img"
/>

View file

@ -38,6 +38,7 @@ import { DEFAULT_PROJECT_COLOR } from '../../work-context/work-context.const';
import { DEFAULT_PROJECT_ICON } from '../../project/project.const';
import { ClipboardImageService } from '../../../core/clipboard-image/clipboard-image.service';
import { RenderLinksPipe } from '../../../ui/pipes/render-links.pipe';
import { isPathSafeToOpen } from '../../../../../electron/shared-with-frontend/is-external-url-allowed';
@Component({
selector: 'note',
@ -69,10 +70,16 @@ export class NoteComponent implements OnChanges {
note!: Note;
// The <img> src auto-loads on render (no click), so a synced remote file:// /
// UNC imgUrl would silently leak the user's NTLM hash. Only a safe URL reaches
// the [src]/[enlargeImg] bindings. See GHSA-hr87-735w-hfq3.
safeImgUrl?: string;
// TODO: Skipped for migration because:
// Accessor inputs cannot be migrated as they are too complex.
@Input('note') set noteSet(v: Note) {
this.note = v;
this.safeImgUrl = isPathSafeToOpen(v?.imgUrl) ? v.imgUrl : undefined;
this._note$.next(v);
this._updateNoteTxt();
}

View file

@ -8,6 +8,7 @@ import { DateAdapter } from '@angular/material/core';
import { MatDialog } from '@angular/material/dialog';
import { ScheduleComponent } from './schedule.component';
import { CalendarEventActionsService } from '../../calendar-integration/calendar-event-actions.service';
import { TaskService } from '../../tasks/task.service';
import { LayoutService } from '../../../core-ui/layout/layout.service';
import { ScheduleService } from '../schedule.service';
@ -109,6 +110,25 @@ describe('issue #7383 — NG0701 race on /schedule', () => {
{ provide: LayoutService, useValue: mockLayoutService },
{ provide: ScheduleService, useValue: mockScheduleService },
{ provide: MatDialog, useValue: jasmine.createSpyObj('MatDialog', ['open']) },
{
// ScheduleComponent's children (schedule-week/schedule-event) eagerly
// inject the root-provided CalendarEventActionsService, whose real DI
// chain pulls in IssueService → SnackService → LOCAL_ACTIONS (needs NgRx
// Actions, which provideMockStore does not supply). Without a mock the
// deferred CD pass after the test hits NG0201 and the async error kills
// the whole Karma session. Mock it to keep the injector self-contained.
provide: CalendarEventActionsService,
useValue: jasmine.createSpyObj('CalendarEventActionsService', [
'hasEventUrl',
'isPluginEvent',
'canMoveEvent',
'openEventLink',
'reschedule',
'createAsTask',
'hideForever',
'deleteEvent',
]),
},
{
provide: GlobalTrackingIntervalService,
useValue: mockGlobalTrackingIntervalService,

View file

@ -175,7 +175,7 @@ export const WORK_CONTEXT_THEME_CONFIG_FORM_CONFIG: ConfigFormSection<WorkContex
type: 'image-input',
templateOptions: {
label: T.F.PROJECT.FORM_THEME.L_BACKGROUND_IMAGE_DARK,
description: '* https://some/cool.jpg, file:///home/user/bg.png',
description: '* https://some/cool.jpg',
},
},
{
@ -183,7 +183,7 @@ export const WORK_CONTEXT_THEME_CONFIG_FORM_CONFIG: ConfigFormSection<WorkContex
type: 'image-input',
templateOptions: {
label: T.F.PROJECT.FORM_THEME.L_BACKGROUND_IMAGE_LIGHT,
description: '* https://some/cool.jpg, file:///home/user/bg.png',
description: '* https://some/cool.jpg',
},
},
{

View file

@ -12,6 +12,8 @@ import { Store } from '@ngrx/store';
import { selectSyncConfig } from '../../features/config/store/global-config.reducer';
import { DataInitStateService } from '../../core/data-init/data-init-state.service';
import { SyncLog } from '../../core/log';
import { SnackService } from '../../core/snack/snack.service';
import { T } from '../../t.const';
import { SyncProviderId, toSyncProviderId } from './provider.const';
import { SyncProviderBase } from './provider.interface';
import {
@ -41,6 +43,7 @@ export type SyncStatusChangePayload =
export class SyncProviderManager {
private _dataInitStateService = inject(DataInitStateService);
private _store = inject(Store);
private _snackService = inject(SnackService);
// Lazily loaded providers (cached after first load)
private _providers: SyncProviderBase<SyncProviderId>[] | null = null;
@ -72,6 +75,7 @@ export class SyncProviderManager {
// Emits whenever provider config is updated via setProviderConfig()
private _providerConfigChanged$ = new Subject<void>();
private _hasShownLocalFileReselectSnack = false;
/**
* Observable for whether the sync provider is enabled and ready
@ -249,6 +253,7 @@ export class SyncProviderManager {
// Re-check readiness
const ready = await provider.isReady();
this._isProviderReady$.next(ready);
this._maybeShowLocalFileReselectSnack(providerId, ready, config);
}
}
@ -325,6 +330,7 @@ export class SyncProviderManager {
}
this._isProviderReady$.next(ready);
this._currentProviderPrivateCfg$.next({ providerId, privateCfg });
this._maybeShowLocalFileReselectSnack(providerId, ready, privateCfg);
SyncLog.normal(`SyncProviderManager: Active provider set to ${providerId}`);
})
.catch((e) => {
@ -334,4 +340,29 @@ export class SyncProviderManager {
}
});
}
private _maybeShowLocalFileReselectSnack(
providerId: SyncProviderId,
isReady: boolean,
privateCfg: unknown,
): void {
if (
this._hasShownLocalFileReselectSnack ||
isReady ||
providerId !== SyncProviderId.LocalFile
) {
return;
}
const legacyPath = (privateCfg as { syncFolderPath?: unknown } | null)
?.syncFolderPath;
if (typeof legacyPath !== 'string' || !legacyPath) {
return;
}
this._hasShownLocalFileReselectSnack = true;
this._snackService.open({
msg: T.F.SYNC.S.LOCAL_FILE_RESELECT_REQUIRED,
type: 'WARNING',
config: { duration: 0 },
});
}
}

View file

@ -1,4 +1,3 @@
/* eslint-disable @typescript-eslint/naming-convention */
/**
* Integration test for the #8077 own-op-replay regression.
*

View file

@ -1,5 +1,4 @@
import { Injectable } from '@angular/core';
import { cleanupAllPluginIframeUrls } from './util/plugin-iframe.util';
/**
* Simplified cleanup service following KISS principles.
@ -34,8 +33,5 @@ export class PluginCleanupService {
cleanupAll(): void {
// Just clear all references - let Angular manage iframe DOM lifecycle
this._pluginIframes.clear();
// Clean up all blob URLs to prevent memory leaks
cleanupAllPluginIframeUrls();
}
}

View file

@ -59,16 +59,19 @@
}
<!-- Plugin iframe -->
@if (iframeSrc() && !error()) {
@if (iframeSrcdoc() && !error()) {
<div
class="iframe-container"
[class.is-resizing]="isResizing()"
>
<!-- sandbox must stay a static attribute and mirror PLUGIN_IFRAME_SANDBOX:
Angular forbids binding security-sensitive iframe attributes (throws NG0910). -->
Angular forbids binding security-sensitive iframe attributes (throws NG0910).
No allow-same-origin: the iframe runs on an opaque origin so it cannot reach
window.parent.ea. Loaded via srcdoc (not a blob: URL) so it still renders on
packaged file:// builds, where an opaque-origin blob: fetch would be blocked. -->
<iframe
#iframe
[src]="iframeSrc()"
[srcdoc]="iframeSrcdoc()"
data-plugin-iframe
[attr.data-plugin-id]="pluginId()"
sandbox="allow-scripts allow-forms allow-popups allow-modals"

View file

@ -10,7 +10,7 @@ import {
ViewChild,
} from '@angular/core';
import { ActivatedRoute, Router } from '@angular/router';
import { DomSanitizer, SafeResourceUrl } from '@angular/platform-browser';
import { DomSanitizer, SafeHtml } from '@angular/platform-browser';
import { Subscription } from 'rxjs';
import { trigger, transition, style, animate } from '@angular/animations';
import { PluginService } from '../../plugin.service';
@ -18,9 +18,8 @@ import { PluginBridgeService } from '../../plugin-bridge.service';
import { PluginCleanupService } from '../../plugin-cleanup.service';
import {
PluginIframeConfig,
createPluginIframeUrl,
buildPluginIframeHtml,
handlePluginMessage,
cleanupPluginIframeUrl,
} from '../../util/plugin-iframe.util';
import {
MatCard,
@ -93,12 +92,11 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
readonly pluginId = signal<string>('');
readonly isLoading = signal<boolean>(true);
readonly error = signal<string | null>(null);
readonly iframeSrc = signal<SafeResourceUrl | null>(null);
readonly iframeSrcdoc = signal<SafeHtml | null>(null);
readonly isResizing = this._layoutService.isPanelResizing;
private _messageListener?: EventListener;
private _routeSubscription?: Subscription;
private _currentIframeUrl: string | null = null;
async ngOnInit(): Promise<void> {
// If directPluginId is provided, load that plugin directly
@ -235,11 +233,8 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
boundMethods: this._pluginBridge.createBoundMethods(pluginId, plugin.manifest),
};
// Create iframe URL using blob URL
const iframeUrl = createPluginIframeUrl(config);
// Store the URL for cleanup
this._currentIframeUrl = iframeUrl;
// Build the iframe document (loaded via srcdoc, not a blob: URL)
const iframeHtml = buildPluginIframeHtml(config);
// Store message handler for cleanup
// Filter by event.source so messages from other plugin iframes (side panel,
@ -265,17 +260,12 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
// Set up message communication
window.addEventListener('message', this._messageListener);
// Create safe URL and set iframe source
const safeUrl = this._sanitizer.bypassSecurityTrustResourceUrl(iframeUrl);
PluginLog.log(
`Setting iframe src for plugin ${pluginId}:`,
iframeUrl.startsWith('blob:')
? `blob:${iframeUrl.split(':')[1].substring(0, 20)}...`
: iframeUrl.substring(0, 100) + '...',
);
this.iframeSrc.set(safeUrl);
// Set the iframe document. `srcdoc` is a SecurityContext.HTML sink, so the
// host-built document (which contains the inline API bridge <script>) must
// be marked trusted or Angular's HTML sanitizer would strip the script.
this.iframeSrcdoc.set(this._sanitizer.bypassSecurityTrustHtml(iframeHtml));
this.isLoading.set(false);
PluginLog.log(`Plugin ${pluginId} iframe src set, loading complete`);
PluginLog.log(`Plugin ${pluginId} iframe srcdoc set, loading complete`);
}
private _createBridgeToken(): string {
@ -295,13 +285,6 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
PluginLog.log(`Removed message listener for plugin: ${currentPluginId}`);
}
// Cleanup blob URL if it exists
if (this._currentIframeUrl) {
cleanupPluginIframeUrl(this._currentIframeUrl);
PluginLog.log(`Cleaned up blob URL for plugin: ${currentPluginId}`);
this._currentIframeUrl = null;
}
// Clear iframe reference from cleanup service (but don't remove from DOM).
// Skip when embedding (work-view, side panel) so the plugin keeps its hook
// and button registrations across embed mount/unmount cycles.
@ -310,13 +293,11 @@ export class PluginIndexComponent implements OnInit, OnDestroy {
PluginLog.log(`Cleaned up plugin references for: ${currentPluginId}`);
}
// Set iframe to empty data URL to stop execution but keep iframe in DOM
this.iframeSrc.set(
this._sanitizer.bypassSecurityTrustResourceUrl(
'data:text/html,<html><body></body></html>',
),
// Reset iframe to an empty document to stop execution but keep it in DOM
this.iframeSrcdoc.set(
this._sanitizer.bypassSecurityTrustHtml('<html><body></body></html>'),
);
PluginLog.log(`Set iframe to empty data URL for plugin: ${currentPluginId}`);
PluginLog.log(`Reset iframe to empty document for plugin: ${currentPluginId}`);
}
onIframeLoad(): void {

View file

@ -5,6 +5,7 @@ import {
} from '@super-productivity/plugin-api';
import { PluginBridgeService } from '../plugin-bridge.service';
import {
buildPluginIframeHtml,
createPluginApiScript,
handlePluginMessage,
PluginIframeConfig,
@ -310,6 +311,34 @@ describe('handlePluginMessage()', () => {
expect(PLUGIN_IFRAME_SANDBOX).not.toContain('allow-same-origin');
});
describe('buildPluginIframeHtml()', () => {
const pluginBridge = {
createBoundMethods: () => ({}),
} as unknown as PluginBridgeService;
it('returns an inline HTML document (not a blob: URL) for srcdoc', () => {
const html = buildPluginIframeHtml({
...createConfig(pluginBridge),
indexHtml: '<html><head></head><body><div id="app"></div></body></html>',
});
expect(typeof html).toBe('string');
expect(html.startsWith('blob:')).toBe(false);
// plugin content is preserved
expect(html).toContain('<div id="app"></div>');
});
it('injects the API bridge script (with the bridge token) before </body>', () => {
const html = buildPluginIframeHtml({
...createConfig(pluginBridge),
indexHtml: '<html><head></head><body></body></html>',
});
expect(html).toContain('const bridgeToken = "test-bridge-token"');
expect(html).toContain('window.PluginAPI');
// script is injected inside the body, before its closing tag
expect(html.indexOf('window.PluginAPI')).toBeLessThan(html.indexOf('</body>'));
});
});
it('rejects raw iframe calls to bridge methods outside the iframe API allowlist', async () => {
const sourceWindow = jasmine.createSpyObj<{ postMessage: jasmine.Spy }>(
'sourceWindow',

View file

@ -533,13 +533,18 @@ export const createPluginApiScript = (config: PluginIframeConfig): string => {
`;
};
// Store blob URLs for cleanup
const activeBlobUrls = new Set<string>();
/**
* Create iframe URL with injected API using blob URLs instead of data URLs
* Build the full HTML document for a plugin UI iframe: the plugin's index.html
* with the host CSS and the postMessage API bridge injected.
*
* Returned as a string for `iframe.srcdoc`, NOT a `blob:` URL. srcdoc content is
* parsed inline, so the iframe loads under its sandbox-assigned opaque origin
* even on packaged `file://` builds. A `blob:` URL created by the host inherits
* the host origin, and an opaque-origin iframe (no `allow-same-origin`) cannot
* fetch it which blanks the plugin UI on desktop. srcdoc avoids that
* cross-origin fetch entirely. See PLUGIN_IFRAME_SANDBOX.
*/
export const createPluginIframeUrl = (config: PluginIframeConfig): string => {
export const buildPluginIframeHtml = (config: PluginIframeConfig): string => {
const apiScript = createPluginApiScript(config);
const cssInjection = createPluginCssInjection();
const fullCssInjection =
@ -566,34 +571,7 @@ export const createPluginIframeUrl = (config: PluginIframeConfig): string => {
html = html + apiScript;
}
// Create blob URL instead of data URL
const blob = new Blob([html], { type: 'text/html' });
const blobUrl = URL.createObjectURL(blob);
// Track the blob URL for cleanup
activeBlobUrls.add(blobUrl);
return blobUrl;
};
/**
* Clean up a blob URL when iframe is no longer needed
*/
export const cleanupPluginIframeUrl = (url: string): void => {
if (url.startsWith('blob:') && activeBlobUrls.has(url)) {
URL.revokeObjectURL(url);
activeBlobUrls.delete(url);
}
};
/**
* Clean up all active blob URLs (useful for cleanup on destroy)
*/
export const cleanupAllPluginIframeUrls = (): void => {
activeBlobUrls.forEach((url) => {
URL.revokeObjectURL(url);
});
activeBlobUrls.clear();
return html;
};
/**

View file

@ -1023,6 +1023,8 @@ const T = {
BTN_SELECT_IMAGE: 'F.PROJECT.FORM_THEME.BTN_SELECT_IMAGE',
S_BACKGROUND_IMAGE_READ_ERROR:
'F.PROJECT.FORM_THEME.S_BACKGROUND_IMAGE_READ_ERROR',
S_BACKGROUND_IMAGE_RESELECT_REQUIRED:
'F.PROJECT.FORM_THEME.S_BACKGROUND_IMAGE_RESELECT_REQUIRED',
S_BACKGROUND_IMAGE_TOO_LARGE: 'F.PROJECT.FORM_THEME.S_BACKGROUND_IMAGE_TOO_LARGE',
L_THEME_COLOR: 'F.PROJECT.FORM_THEME.L_THEME_COLOR',
TITLE: 'F.PROJECT.FORM_THEME.TITLE',
@ -1597,6 +1599,7 @@ const T = {
LOCAL_DATA_REPLACE_UNDO: 'F.SYNC.S.LOCAL_DATA_REPLACE_UNDO',
LOCK_TIMEOUT_ERROR: 'F.SYNC.S.LOCK_TIMEOUT_ERROR',
LWW_CONFLICTS_AUTO_RESOLVED: 'F.SYNC.S.LWW_CONFLICTS_AUTO_RESOLVED',
LOCAL_FILE_RESELECT_REQUIRED: 'F.SYNC.S.LOCAL_FILE_RESELECT_REQUIRED',
MIGRATION_FAILED: 'F.SYNC.S.MIGRATION_FAILED',
NETWORK_ERROR: 'F.SYNC.S.NETWORK_ERROR',
NEWER_VERSION_AVAILABLE: 'F.SYNC.S.NEWER_VERSION_AVAILABLE',

View file

@ -138,11 +138,11 @@ describe('FormlyImageInputComponent', () => {
await component.openFileExplorer();
expect(imagePickAndImport).toHaveBeenCalledWith(undefined);
expect(imagePickAndImport).toHaveBeenCalledWith();
expect(setValueSpy).toHaveBeenCalledWith(`image:${'a'.repeat(32)}`);
});
it('passes the replacesId so main can garbage-collect the old cached image', async () => {
it('does not pass the previous image id before the form save is durable', async () => {
(component as any).IS_ELECTRON = true;
formControl.setValue(`image:${'b'.repeat(32)}`);
@ -153,7 +153,7 @@ describe('FormlyImageInputComponent', () => {
await component.openFileExplorer();
expect(imagePickAndImport).toHaveBeenCalledWith({ replacesId: 'b'.repeat(32) });
expect(imagePickAndImport).toHaveBeenCalledWith();
});
it('shows a snack on validation failure (Error return) but not on cancel (null)', async () => {
@ -194,9 +194,8 @@ describe('FormlyImageInputComponent', () => {
(window as any).ea = { imagePickAndImport };
const firstClick = component.openFileExplorer();
// Second click while the first is awaiting — must be a no-op so the
// form's `replacesId` for the second IPC doesn't still point at the
// old value and orphan the first import.
// Second click while the first is awaiting must be a no-op so the form
// does not queue multiple imports and orphan all but the last selected one.
await component.openFileExplorer();
expect(imagePickAndImport).toHaveBeenCalledTimes(1);
expect(component.isPickerBusy()).toBe(true);

View file

@ -48,9 +48,8 @@ export class FormlyImageInputComponent extends FieldType<FormlyFieldConfig> {
readonly IS_ELECTRON = IS_ELECTRON;
// Guard against double-click while the main-side dialog + import is in
// flight. A second click would otherwise queue a second IPC that opens
// a second dialog after the first resolves, and (because `replacesId` is
// computed from the form value at click time) leak the first import as
// an orphan in `userData/bg-images/`.
// a second dialog after the first resolves and orphan all but the last
// selected import in `userData/bg-images/`.
readonly isPickerBusy = signal(false);
get isUnsplashAvailable(): boolean {
@ -63,18 +62,11 @@ export class FormlyImageInputComponent extends FieldType<FormlyFieldConfig> {
}
// Post-#8228: dialog + import are atomic in main. The renderer never
// sees the absolute path and cannot trigger an import without a real
// user-driven dialog interaction. We pass the current cached id (if
// any) so main can garbage-collect the file we're about to replace.
const current = this.formControl.value;
const replacesId =
typeof current === 'string' && current.startsWith('image:')
? current.substring('image:'.length)
: undefined;
// user-driven dialog interaction. Old cached images are not deleted here:
// the surrounding form save can still be cancelled or fail.
this.isPickerBusy.set(true);
try {
const result = await window.ea.imagePickAndImport(
replacesId ? { replacesId } : undefined,
);
const result = await window.ea.imagePickAndImport();
if (result instanceof Error) {
// Validation failure (extension, size cap, etc.). User-cancel
// returns null. See electron/local-file-sync.ts IMAGE_PICK_AND_IMPORT.

View file

@ -1004,6 +1004,7 @@
"L_IS_DISABLE_BACKGROUND_TINT": "Disable colored background tint",
"BTN_SELECT_IMAGE": "Select",
"S_BACKGROUND_IMAGE_READ_ERROR": "Could not read selected image file.",
"S_BACKGROUND_IMAGE_RESELECT_REQUIRED": "Local background image files need to be selected again after a security update.",
"S_BACKGROUND_IMAGE_TOO_LARGE": "Selected image is too large. Please choose a file smaller than {{maxSizeKb}} KB.",
"L_THEME_COLOR": "Theme Color",
"TITLE": "Theme"
@ -1552,6 +1553,7 @@
"LOCAL_DATA_REPLACE_UNDO": "Your local data was replaced with the server's data.",
"LOCK_TIMEOUT_ERROR": "Sync timed out waiting for a previous operation to finish. Please try again.",
"LWW_CONFLICTS_AUTO_RESOLVED": "Sync conflicts auto-resolved: {{localWins}} local win(s), {{remoteWins}} remote win(s)",
"LOCAL_FILE_RESELECT_REQUIRED": "Local file sync needs the sync folder to be selected again after a security update. Open Sync settings and choose the folder once more.",
"MIGRATION_FAILED": "Some sync data could not be migrated and was skipped. This may indicate data corruption or a bug.",
"NETWORK_ERROR": "Sync request failed because of a temporary network problem. Your changes remain local and sync will retry.",
"NEWER_VERSION_AVAILABLE": "Some changes are from a newer app version. Consider updating for full compatibility.",

View file

@ -70,7 +70,6 @@ if (asyncActionProto && typeof asyncActionProto.execute === 'function') {
if (this.closed) {
if (!warnedOnce) {
warnedOnce = true;
// eslint-disable-next-line no-console
console.warn(
'[test] Suppressed a leaked rxjs scheduler action (a cancelled ' +
"action's timer fired after teardown). A spec is not tearing down " +