diff --git a/electron/electronAPI.d.ts b/electron/electronAPI.d.ts index 4978630571..163c56b0fb 100644 --- a/electron/electronAPI.d.ts +++ b/electron/electronAPI.d.ts @@ -92,7 +92,7 @@ export interface ElectronAPI { */ imagePickAndImport(args?: { replacesId?: string; - }): Promise<{ id: string; mimeType: string } | null>; + }): Promise<{ id: string; mimeType: string } | null | Error>; /** * Resolve a cached image id to a `data:` URL the renderer can use as a diff --git a/electron/local-file-sync.test.cjs b/electron/local-file-sync.test.cjs index 6ef18c7c51..f80587349b 100644 --- a/electron/local-file-sync.test.cjs +++ b/electron/local-file-sync.test.cjs @@ -279,16 +279,17 @@ test('IMAGE_PICK_AND_IMPORT returns null when the user cancels', async () => { assert.equal(result, null); }); -test('IMAGE_PICK_AND_IMPORT throws when the picked file fails validation', async () => { - // File inside userData → importImage returns null → handler throws so the - // renderer can show an error snack (vs a silent null for user-cancel). +test('IMAGE_PICK_AND_IMPORT returns a safe Error when validation fails (no path leak)', async () => { + // File inside userData → importImage returns null → handler returns a + // safe Error so the renderer can show a snack (vs a silent null for cancel). + // The error message must not echo back the renderer-picked path. const planted = path.join(userDataDir, 'planted.png'); fs.writeFileSync(planted, 'fake'); nextDialogResult = { canceled: false, filePaths: [planted] }; - await assert.rejects( - () => handlers['IMAGE_PICK_AND_IMPORT']({}, undefined), - /could not be imported/, - ); + const result = await handlers['IMAGE_PICK_AND_IMPORT']({}, undefined); + assert.ok(result instanceof Error); + assert.ok(!String(result.message).includes(planted), 'no path in message'); + 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 () => { diff --git a/electron/local-file-sync.ts b/electron/local-file-sync.ts index baf7a058ac..c501e43aee 100644 --- a/electron/local-file-sync.ts +++ b/electron/local-file-sync.ts @@ -252,38 +252,48 @@ export const initLocalFileSyncAdapter = (): void => { async ( _, args?: { replacesId?: string }, - ): Promise<{ id: string; mimeType: string } | null> => { + ): 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. // (The previous shape exposed an `importImage(path)` IPC that the // renderer could call with any image-extension path it pleased; this // version closes that gap.) - const { canceled, filePaths } = (await dialog.showOpenDialog(getWin(), { - title: 'Select image', - buttonLabel: 'Select', - properties: ['openFile'], - filters: [{ name: 'Images', extensions: ['png', 'jpg', 'jpeg', 'gif', 'webp'] }], - })) as unknown as { canceled: boolean; filePaths: string[] }; - if (canceled || !filePaths[0]) { - // User cancelled — distinct from validation failure so the renderer - // can stay silent instead of showing a "couldn't read image" snack. - return null; + try { + const { canceled, filePaths } = (await dialog.showOpenDialog(getWin(), { + title: 'Select image', + buttonLabel: 'Select', + properties: ['openFile'], + filters: [ + { name: 'Images', extensions: ['png', 'jpg', 'jpeg', 'gif', 'webp'] }, + ], + })) as unknown as { canceled: boolean; filePaths: string[] }; + if (canceled || !filePaths[0]) { + // User cancelled — distinct from validation failure so the renderer + // can stay silent instead of showing a "couldn't read image" snack. + return null; + } + const imported = await importImage(filePaths[0]); + if (!imported) { + // Reject as an error: validation failed (extension, size, etc.) so + // the renderer surfaces the error path, not the cancel path. The + // safe-error wrapper below strips the stack and renames the error + // so it matches the FS handlers' contract — main bundle paths + // (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)); + return createSafeIpcError(IPC.IMAGE_PICK_AND_IMPORT, e); } - const imported = await importImage(filePaths[0]); - if (!imported) { - // Reject as an error: validation failed (extension, size, etc.) so - // the renderer surfaces the error path, not the cancel path. - 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; }, ); diff --git a/electron/preload.ts b/electron/preload.ts index f0817f666b..f0c782c7c2 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -91,10 +91,14 @@ const ea: ElectronAPI = { }) => _invoke('SHOW_OPEN_DIALOG', options) as Promise, imagePickAndImport: (args) => - _invoke('IMAGE_PICK_AND_IMPORT', args) as Promise<{ - id: string; - mimeType: string; - } | null>, + _invoke('IMAGE_PICK_AND_IMPORT', args) as Promise< + | { + id: string; + mimeType: string; + } + | null + | Error + >, imageCacheGetDataUrl: (id: string) => _invoke('IMAGE_CACHE_GET_DATA_URL', id) as Promise, // STANDARD diff --git a/packages/sync-providers/src/file-based/local-file/local-file.model.ts b/packages/sync-providers/src/file-based/local-file/local-file.model.ts index 5cb45aa4d9..eb6aecc084 100644 --- a/packages/sync-providers/src/file-based/local-file/local-file.model.ts +++ b/packages/sync-providers/src/file-based/local-file/local-file.model.ts @@ -8,10 +8,12 @@ export const PROVIDER_ID_LOCAL_FILE = 'LocalFile' as const; export interface LocalFileSyncPrivateCfg { encryptKey?: string; /** - * @deprecated Electron-selected sync folder path, kept for migration of - * pre-issue-#8228 configs. The authoritative copy now lives main-side in - * `electron/sync-folder-store.ts`; this field may be read once on upgrade - * to seed the main store, then becomes unused. + * @deprecated Electron-selected sync folder path, kept only so older + * persisted configs can still be parsed. Post-#8228 the authoritative + * sync folder lives main-side (inlined cache backed by `simple-store` + * inside `electron/local-file-sync.ts`); this field is read once on + * upgrade so the migration breadcrumb can log "needs re-pick", and is + * never written from new code paths. */ syncFolderPath?: string; /** Android Storage Access Framework folder URI. */ diff --git a/src/app/ui/formly-image-input/formly-image-input.component.html b/src/app/ui/formly-image-input/formly-image-input.component.html index 318f9de681..c277cc8e92 100644 --- a/src/app/ui/formly-image-input/formly-image-input.component.html +++ b/src/app/ui/formly-image-input/formly-image-input.component.html @@ -13,14 +13,14 @@ [formControl]="formControl" [formlyAttributes]="field" matInput - placeholder="https://... or file://..." + placeholder="https://..." /> @if (IS_ELECTRON) {