Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions apps/electron-backend/src/app/app.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const mockClearStorageData = jest.fn();

jest.mock('electron', () => ({
app: {
getPath: jest.fn(() => '/tmp'),
Expand All @@ -10,6 +12,11 @@ jest.mock('electron', () => ({
screen: {
getPrimaryDisplay: jest.fn(),
},
session: {
defaultSession: {
clearStorageData: mockClearStorageData,
},
},
shell: {
openExternal: jest.fn(),
},
Expand All @@ -24,12 +31,48 @@ jest.mock('./services/store.service', () => ({
}));

import {
clearElectronServiceWorkerStorage,
getMainWindowWebPreferences,
isExternalBrowserUrl,
isTrustedRendererNavigationUrl,
} from './app';
import App from './app';
import { app as electronApp } from 'electron';

type MockMainWindow = {
loadFile: jest.Mock<Promise<void>, [string]>;
loadURL: jest.Mock<Promise<void>, [string]>;
webContents: {
openDevTools: jest.Mock<void, []>;
};
};

function createMockMainWindow(): MockMainWindow {
return {
loadFile: jest.fn<Promise<void>, [string]>().mockResolvedValue(),
loadURL: jest.fn<Promise<void>, [string]>().mockResolvedValue(),
webContents: {
openDevTools: jest.fn<void, []>(),
},
};
}

type AppInternals = {
mainWindow: MockMainWindow;
loadMainWindow: () => Promise<void>;
};

function getAppInternals(): AppInternals {
return App as unknown as AppInternals;
}

describe('Electron app security helpers', () => {
beforeEach(() => {
jest.clearAllMocks();
delete process.env.ELECTRON_IS_DEV;
(electronApp as unknown as { isPackaged: boolean }).isPackaged = false;
});

it('creates an explicitly hardened BrowserWindow webPreferences object', () => {
expect(getMainWindowWebPreferences()).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -90,4 +133,53 @@ describe('Electron app security helpers', () => {
isTrustedRendererNavigationUrl('https://example.com', false)
).toBe(false);
});

it('clears Electron service worker storage before loading the packaged renderer', async () => {
const appInternals = getAppInternals();
const mainWindow = createMockMainWindow();
appInternals.mainWindow = mainWindow;
(electronApp as unknown as { isPackaged: boolean }).isPackaged = true;

await appInternals.loadMainWindow();

expect(mockClearStorageData).toHaveBeenCalledWith({
storages: ['serviceworkers', 'cachestorage'],
});
expect(mainWindow.loadFile).toHaveBeenCalledWith(
expect.stringContaining('index.html')
);
expect(
mockClearStorageData.mock.invocationCallOrder[0]
).toBeLessThan(mainWindow.loadFile.mock.invocationCallOrder[0]);
});

it('continues packaged renderer loading when Electron service worker cleanup fails', async () => {
const appInternals = getAppInternals();
const mainWindow = createMockMainWindow();
const warnSpy = jest.spyOn(console, 'warn').mockImplementation();

appInternals.mainWindow = mainWindow;
(electronApp as unknown as { isPackaged: boolean }).isPackaged = true;
mockClearStorageData.mockRejectedValueOnce(new Error('cleanup failed'));

await appInternals.loadMainWindow();

expect(mainWindow.loadFile).toHaveBeenCalledWith(
expect.stringContaining('index.html')
);
expect(warnSpy).toHaveBeenCalledWith(
'Failed to clear Electron service worker storage:',
expect.any(Error)
);

warnSpy.mockRestore();
});

it('clears only service worker registrations and cache storage', async () => {
await clearElectronServiceWorkerStorage();

expect(mockClearStorageData).toHaveBeenCalledWith({
storages: ['serviceworkers', 'cachestorage'],
});
});
});
41 changes: 36 additions & 5 deletions apps/electron-backend/src/app/app.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { app, BrowserWindow, Menu, screen, shell } from 'electron';
import { app, BrowserWindow, Menu, screen, session, shell } from 'electron';
import { WINDOW_STATE_CHANGED } from '@iptvnator/shared/interfaces';
import { join, resolve } from 'path';
import { fileURLToPath } from 'url';
import { rendererAppName, rendererAppPort } from './constants';
import {
isStartupTraceEnabled,
isRendererConsoleTraceEnabled,
isWindowTraceEnabled,
trace,
Expand Down Expand Up @@ -88,6 +89,30 @@ export function getMainWindowWebPreferences(): Electron.BrowserWindowConstructor
};
}

export async function clearElectronServiceWorkerStorage(
electronSession: Pick<Electron.Session, 'clearStorageData'> = session.defaultSession
): Promise<void> {
try {
await electronSession.clearStorageData({
storages: ['serviceworkers', 'cachestorage'],
});

if (isStartupTraceEnabled()) {
trace('startup', 'electron-service-worker-storage:cleared');
}
} catch (error) {
console.warn('Failed to clear Electron service worker storage:', error);

if (isStartupTraceEnabled()) {
trace(
'startup',
'electron-service-worker-storage:clear-failed',
error
);
}
}
}

function attachWindowTrace(mainWindow: Electron.BrowserWindow): void {
if (!isWindowTraceEnabled()) {
return;
Expand Down Expand Up @@ -211,7 +236,9 @@ export default class App {
// Some APIs can only be used after this event occurs.
if (rendererAppName) {
App.initMainWindow();
App.loadMainWindow();
void App.loadMainWindow().catch((error) => {
console.error('Failed to load main window:', error);
});
}
}

Expand Down Expand Up @@ -382,15 +409,19 @@ export default class App {
});
}

private static loadMainWindow() {
private static async loadMainWindow(): Promise<void> {
// load the index.html of the app.
if (App.isDevelopmentMode()) {
App.mainWindow.loadURL(`http://localhost:${rendererAppPort}`);
const loadPromise = App.mainWindow.loadURL(
`http://localhost:${rendererAppPort}`
);
if (App.shouldOpenDevTools()) {
App.mainWindow.webContents.openDevTools();
}
await loadPromise;
} else {
App.mainWindow.loadFile(getPackagedRendererIndexPath());
await clearElectronServiceWorkerStorage();
await App.mainWindow.loadFile(getPackagedRendererIndexPath());
}
}

Expand Down
38 changes: 38 additions & 0 deletions apps/web/src/app/services/runtime-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,42 @@ describe('runtime config helpers', () => {
).toBe(false);
expect(shouldEnableServiceWorker(true, {} as Navigator)).toBe(false);
});

it('disables service worker for Electron runtime', () => {
expect(
shouldEnableServiceWorker(
true,
{ serviceWorker: {} } as Navigator,
{
electronBridge: {},
protocol: 'file:',
}
)
).toBe(false);
});
Comment on lines +40 to +51

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing coverage: electronBridge-only guard

The Electron test always supplies both electronBridge and protocol: 'file:', so neither condition is ever isolated. If an Electron build ever uses a non-file: scheme (e.g. app:// or a custom iptvnator:// protocol), the only guard that fires is !electronBridge, but no test exercises that path. Adding a case where electronBridge is set and protocol is 'https:' (or any non-file: value) would pin that contract.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/src/app/services/runtime-config.spec.ts
Line: 40-51

Comment:
**Missing coverage: `electronBridge`-only guard**

The Electron test always supplies both `electronBridge` and `protocol: 'file:'`, so neither condition is ever isolated. If an Electron build ever uses a non-`file:` scheme (e.g. `app://` or a custom `iptvnator://` protocol), the only guard that fires is `!electronBridge`, but no test exercises that path. Adding a case where `electronBridge` is set and `protocol` is `'https:'` (or any non-`file:` value) would pin that contract.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


it('disables service worker for Electron runtime on non-file origins', () => {
expect(
shouldEnableServiceWorker(
true,
{ serviceWorker: {} } as Navigator,
{
electronBridge: {},
protocol: 'https:',
}
)
).toBe(false);
});

it('disables service worker for file origins without an Electron bridge', () => {
expect(
shouldEnableServiceWorker(
true,
{ serviceWorker: {} } as Navigator,
{
protocol: 'file:',
}
)
).toBe(false);
});
});
28 changes: 26 additions & 2 deletions apps/web/src/app/services/runtime-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,33 @@ export function getRuntimeBackendUrl(): string {
);
}

export interface ServiceWorkerRuntimeContext {
readonly electronBridge?: unknown;
readonly protocol?: string;
}

function getDefaultServiceWorkerRuntimeContext(): ServiceWorkerRuntimeContext {
const browserWindow = globalThis.window as
| (Window & { electron?: unknown })
| undefined;

return {
electronBridge: browserWindow?.electron,
protocol:
browserWindow?.location?.protocol ?? globalThis.location?.protocol,
};
}

export function shouldEnableServiceWorker(
production = AppConfig.production,
navigatorRef: Navigator | undefined = globalThis.navigator
navigatorRef: Navigator | undefined = globalThis.navigator,
runtimeContext = getDefaultServiceWorkerRuntimeContext()
): boolean {
return production && !!navigatorRef && 'serviceWorker' in navigatorRef;
return (
production &&
!!navigatorRef &&
'serviceWorker' in navigatorRef &&
!runtimeContext.electronBridge &&
runtimeContext.protocol !== 'file:'
Comment on lines +48 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Clear existing Electron service workers before disabling registration

When a user upgrades from a desktop version that already registered ngsw-worker.js, returning false here only prevents this version's Angular initializer from registering again; it does not unregister an active registration or clear NGSW caches. The old file-origin worker can control the first file:// navigation and serve the previous index.html/chunks before this new runtime check ever runs, so affected Electron installs can remain pinned to stale UI until userData is cleared. Add cleanup from the Electron main/session side, or an enabled renderer cleanup path, for existing registrations/caches.

Useful? React with 👍 / 👎.

);
}
9 changes: 9 additions & 0 deletions docs/architecture/pwa-self-hosted.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ Angular also emits hashed font and media assets under `dist/apps/web/media/`.
Keep `/media/**` in `ngsw-config.json` so the PWA service worker can cache
bundled fonts, including Material Icons.

The Angular service worker is a browser/PWA feature only. Packaged Electron
loads the same Angular production bundle from `file://.../app.asar/web`, but it
must not register `ngsw-worker.js`; otherwise a desktop update can leave the
first Electron window controlled by a stale file-origin service worker and serve
old chunks from Electron `userData`. Electron clears legacy `serviceworkers` and
`cachestorage` storage from its default session before loading the packaged
renderer so existing desktop installs recover on the next startup without
clearing unrelated app storage.

`web:serve-static` serves `dist/apps/web` and builds with `web:build:pwa`, so it
exercises the same output layout as Docker. If Nx daemon state returns stale
service worker outputs while changing build options, run:
Expand Down
Loading