From b719ed23cd43d6d48f59318f8c3ce0177ffee3eb Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sun, 19 Jul 2026 07:48:44 +0200 Subject: [PATCH 1/3] fix(playback): position embedded MPV native view correctly on scaled displays (#1206) * fix(playback): position embedded MPV native view correctly on scaled displays Renderer bounds are measured in CSS pixels, but the native-view engines position OS windows: SetWindowPos (win32) and XMoveResizeWindow (linux) expect physical pixels, NSView setFrame (macOS) expects points. The raw values landed the video toward the window's top-left corner at 1/scale of its size on any display scale or page zoom other than 100%, windowed and fullscreen alike. The main process now converts native-view bounds (x page zoom everywhere, x display scale factor on win32/linux) with edge-based rounding; frame-copy bounds stay unscaled because the adapter owns its render scale. The session controller re-syncs bounds when devicePixelRatio changes, covering moves to a display with a different scale that keep the CSS layout identical. Closes #1145 Co-Authored-By: Claude Fable 5 * fix(playback): keep CSS bounds unrounded until native scaling Review feedback on #1206: measureBounds() rounded the CSS edges in the renderer, before the main-process CSS-to-native conversion, so fractional layout positions could drift by a pixel per scale factor (a 10.49px edge at 200% must land on 21 physical px, not 20). The renderer now sends raw getBoundingClientRect() edges and rounding happens exactly once, after scaling. Also pins process.platform explicitly in the macOS wiring test instead of relying on the suite default. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- CLAUDE.md | 2 +- .../services/embedded-mpv-bounds.util.spec.ts | 133 +++++++++++ .../app/services/embedded-mpv-bounds.util.ts | 60 +++++ .../embedded-mpv-native.service.spec.ts | 105 ++++++++- .../services/embedded-mpv-native.service.ts | 50 +++- docs/architecture/embedded-mpv-native.md | 38 +++- .../embedded-mpv-format.utils.spec.ts | 9 +- .../embedded-mpv-format.utils.ts | 15 +- ...mbedded-mpv-session-controller.dpr.spec.ts | 213 ++++++++++++++++++ .../embedded-mpv-session-controller.spec.ts | 4 +- .../embedded-mpv-session-controller.ts | 27 +++ 11 files changed, 637 insertions(+), 19 deletions(-) create mode 100644 apps/electron-backend/src/app/services/embedded-mpv-bounds.util.spec.ts create mode 100644 apps/electron-backend/src/app/services/embedded-mpv-bounds.util.ts create mode 100644 libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.dpr.spec.ts diff --git a/CLAUDE.md b/CLAUDE.md index 42cd4b7fe..16fc9a0e2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -616,7 +616,7 @@ This project uses modern Angular signal-based APIs and patterns. **ALWAYS** use - Built-in web players: HTML5+hls.js, Video.js, and ArtPlayer - External players: MPV, VLC (via IPC to Electron backend) -- Embedded MPV (experimental, macOS/Windows/Linux): renders mpv video inside the Electron window through a native addon. macOS uses the libmpv render API in an `NSOpenGLView`; Windows uses in-process libmpv with `--wid` against an app-owned child `HWND`; Linux spawns an out-of-process `mpv --wid=` controlled over a JSON IPC socket (X11/XWayland only, requires system `mpv` on PATH; subtitles/speed/aspect/recording are not exported there). mpv's own screensaver inhibition does not apply to any of these paths, so `EmbeddedMpvNativeService` holds an Electron `powerSaveBlocker` (`prevent-display-sleep`) whenever any session's status is `playing`, and releases it on pause, dispose, or shutdown. Service: `apps/electron-backend/src/app/services/embedded-mpv-native.service.ts`; full architecture: `docs/architecture/embedded-mpv-native.md`. +- Embedded MPV (experimental, macOS/Windows/Linux): renders mpv video inside the Electron window through a native addon. macOS uses the libmpv render API in an `NSOpenGLView`; Windows uses in-process libmpv with `--wid` against an app-owned child `HWND`; Linux spawns an out-of-process `mpv --wid=` controlled over a JSON IPC socket (X11/XWayland only, requires system `mpv` on PATH; subtitles/speed/aspect/recording are not exported there). mpv's own screensaver inhibition does not apply to any of these paths, so `EmbeddedMpvNativeService` holds an Electron `powerSaveBlocker` (`prevent-display-sleep`) whenever any session's status is `playing`, and releases it on pause, dispose, or shutdown. Renderer bounds are CSS pixels; the service converts them to native units in the main process (`embedded-mpv-bounds.util.ts`: × page zoom everywhere, × display scale on Windows/Linux whose child windows are positioned in physical pixels; frame-copy bounds stay unscaled), and the session controller re-syncs bounds when `devicePixelRatio` changes. Service: `apps/electron-backend/src/app/services/embedded-mpv-native.service.ts`; full architecture: `docs/architecture/embedded-mpv-native.md`. - Embedded MPV frame-copy engine (experimental, macOS Apple Silicon + Linux x64 + Windows; enabled via `Settings > Playback > Embedded MPV: frame-copy engine` (restart required) or diff --git a/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.spec.ts b/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.spec.ts new file mode 100644 index 000000000..03d3ee95a --- /dev/null +++ b/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.spec.ts @@ -0,0 +1,133 @@ +import { EmbeddedMpvBounds } from '@iptvnator/shared/interfaces'; +import { + NativeViewBoundsContext, + toNativeViewBounds, +} from './embedded-mpv-bounds.util'; + +const CSS_BOUNDS: EmbeddedMpvBounds = { x: 372, y: 60, width: 578, height: 330 }; + +function context( + overrides: Partial = {} +): NativeViewBoundsContext { + return { + platform: 'linux', + zoomFactor: 1, + displayScaleFactor: 1, + ...overrides, + }; +} + +describe('toNativeViewBounds', () => { + it('returns the input untouched at 100% zoom and 100% display scale', () => { + const result = toNativeViewBounds(CSS_BOUNDS, context()); + + expect(result).toBe(CSS_BOUNDS); + }); + + // Regression for #1145: CSS bounds were handed to XMoveResizeWindow as-is, + // so on a 140%-scaled Linux Mint desktop the mpv window landed at ~71% of + // the expected position and size, toward the window's top-left corner. + it('scales linux bounds by the display scale factor', () => { + const result = toNativeViewBounds( + CSS_BOUNDS, + context({ platform: 'linux', displayScaleFactor: 1.4 }) + ); + + expect(result).toEqual({ x: 521, y: 84, width: 809, height: 462 }); + }); + + it('scales win32 bounds by the display scale factor', () => { + const result = toNativeViewBounds( + { x: 100, y: 50, width: 640, height: 360 }, + context({ platform: 'win32', displayScaleFactor: 1.25 }) + ); + + expect(result).toEqual({ x: 125, y: 63, width: 800, height: 450 }); + }); + + it('combines page zoom with the display scale factor', () => { + const result = toNativeViewBounds( + { x: 100, y: 50, width: 640, height: 360 }, + context({ + platform: 'win32', + zoomFactor: 1.2, + displayScaleFactor: 1.5, + }) + ); + + expect(result).toEqual({ x: 180, y: 90, width: 1152, height: 648 }); + }); + + it('ignores the display scale on macOS (NSView frames are in points)', () => { + const result = toNativeViewBounds( + CSS_BOUNDS, + context({ platform: 'darwin', displayScaleFactor: 2 }) + ); + + expect(result).toBe(CSS_BOUNDS); + }); + + it('applies page zoom on macOS', () => { + const result = toNativeViewBounds( + { x: 100, y: 50, width: 640, height: 360 }, + context({ + platform: 'darwin', + zoomFactor: 1.5, + displayScaleFactor: 2, + }) + ); + + expect(result).toEqual({ x: 150, y: 75, width: 960, height: 540 }); + }); + + it('rounds fractional CSS edges only after scaling', () => { + // A 10.49px CSS edge at 200% renders at 21 physical pixels; edges + // rounded before scaling would send 20 and shift the video by 1px. + const result = toNativeViewBounds( + { x: 10.49, y: 0.5, width: 100.02, height: 50 }, + context({ platform: 'win32', displayScaleFactor: 2 }) + ); + + expect(result).toEqual({ x: 21, y: 1, width: 200, height: 100 }); + }); + + it('keeps vertically adjacent rects seamless under fractional scales', () => { + // 42 × 1.25 and 153 × 1.25 both land on .5/.25 fractions: rounding + // x/y/width/height independently would misplace the shared edge by + // 1px, while edge-based rounding keeps the rects flush. + const scale = context({ displayScaleFactor: 1.25 }); + const upper = toNativeViewBounds( + { x: 0, y: 42, width: 500, height: 111 }, + scale + ); + const lower = toNativeViewBounds( + { x: 0, y: 153, width: 500, height: 90 }, + scale + ); + + expect(upper.y + upper.height).toBe(lower.y); + }); + + it('keeps hidden bounds offscreen and at least 1x1', () => { + const result = toNativeViewBounds( + { x: -100000, y: -100000, width: 1, height: 1 }, + context({ displayScaleFactor: 1.5 }) + ); + + expect(result.x).toBeLessThanOrEqual(-100000); + expect(result.y).toBeLessThanOrEqual(-100000); + expect(result.width).toBeGreaterThanOrEqual(1); + expect(result.height).toBeGreaterThanOrEqual(1); + }); + + it('treats non-finite or non-positive factors as 100%', () => { + for (const zoomFactor of [Number.NaN, 0, -1, Number.POSITIVE_INFINITY]) { + expect( + toNativeViewBounds( + CSS_BOUNDS, + context({ zoomFactor, displayScaleFactor: zoomFactor }) + ) + ).toBe(CSS_BOUNDS); + } + }); +}); diff --git a/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.ts b/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.ts new file mode 100644 index 000000000..7c7015dd1 --- /dev/null +++ b/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.ts @@ -0,0 +1,60 @@ +import { EmbeddedMpvBounds } from '@iptvnator/shared/interfaces'; + +export interface NativeViewBoundsContext { + platform: NodeJS.Platform; + /** Page zoom factor of the main window's webContents (1 = 100%). */ + zoomFactor: number; + /** Scale factor of the display hosting the main window (1 = 96 dpi). */ + displayScaleFactor: number; +} + +/** + * Converts renderer-measured bounds (CSS pixels from + * getBoundingClientRect()) into the coordinate space the native-view + * engines position their OS windows in: physical pixels for the win32 + * child HWND (SetWindowPos) and the linux child X11 window + * (XMoveResizeWindow), points — device-independent pixels — for the macOS + * NSView (setFrame). CSS pixels match points only at 100% page zoom and + * match physical pixels only at 100% page zoom AND 100% display scale, so + * every platform scales by the zoom factor and win32/linux additionally by + * the display scale factor (#1145). + * + * Bounds arrive with unrounded CSS edges and are rounded exactly once here, + * after scaling: edges first, then width/height derived from them. Rounding + * any earlier (or per-field) lets fractional CSS layouts drift by a pixel + * per scale factor and open 1px seams between the native video window and + * the surrounding DOM UI. + */ +export function toNativeViewBounds( + bounds: EmbeddedMpvBounds, + context: NativeViewBoundsContext +): EmbeddedMpvBounds { + const scale = resolveNativeViewScale(context); + if (scale === 1) { + return bounds; + } + + const left = Math.round(bounds.x * scale); + const top = Math.round(bounds.y * scale); + const right = Math.round((bounds.x + bounds.width) * scale); + const bottom = Math.round((bounds.y + bounds.height) * scale); + + return { + x: left, + y: top, + width: Math.max(1, right - left), + height: Math.max(1, bottom - top), + }; +} + +function resolveNativeViewScale(context: NativeViewBoundsContext): number { + const zoomFactor = sanitizeFactor(context.zoomFactor); + if (context.platform === 'darwin') { + return zoomFactor; + } + return zoomFactor * sanitizeFactor(context.displayScaleFactor); +} + +function sanitizeFactor(value: number): number { + return Number.isFinite(value) && value > 0 ? value : 1; +} diff --git a/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts b/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts index 1f85402b2..0568a9f0c 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts @@ -41,20 +41,29 @@ const appMock = { commandLine: commandLineMock, }; +const screenGetDisplayMatchingMock = jest.fn(); + jest.mock('electron', () => ({ app: appMock, powerSaveBlocker: powerSaveBlockerMock, + screen: { getDisplayMatching: screenGetDisplayMatchingMock }, })); const mainWindowSendMock = jest.fn(); const mainWindowWebContentsOnMock = jest.fn(); +const mainWindowGetZoomFactorMock = jest.fn(); const mainWindowGetNativeWindowHandleMock = jest.fn(() => Buffer.alloc(8) ); const mainWindowMock = { isDestroyed: () => false, getNativeWindowHandle: mainWindowGetNativeWindowHandleMock, - webContents: { send: mainWindowSendMock, on: mainWindowWebContentsOnMock }, + getBounds: () => ({ x: 0, y: 0, width: 1280, height: 720 }), + webContents: { + send: mainWindowSendMock, + on: mainWindowWebContentsOnMock, + getZoomFactor: mainWindowGetZoomFactorMock, + }, }; jest.mock('../app', () => ({ @@ -155,6 +164,10 @@ describe('EmbeddedMpvNativeService power blocker', () => { mainWindowGetNativeWindowHandleMock.mockReturnValue(Buffer.alloc(8)); mainWindowSendMock.mockReset(); mainWindowWebContentsOnMock.mockReset(); + mainWindowGetZoomFactorMock.mockReset(); + mainWindowGetZoomFactorMock.mockReturnValue(1); + screenGetDisplayMatchingMock.mockReset(); + screenGetDisplayMatchingMock.mockReturnValue({ scaleFactor: 1 }); appMock.isPackaged = true; tempDirs = []; @@ -440,6 +453,96 @@ describe('EmbeddedMpvNativeService power blocker', () => { }); }); + describe('native view bounds scaling', () => { + afterEach(() => { + delete process.env.IPTVNATOR_ENABLE_EMBEDDED_MPV_FRAME_COPY; + }); + + it('converts CSS bounds to physical pixels for the native engine on scaled displays', () => { + // Regression for #1145: the win32/linux engines position their + // child window in physical pixels, so renderer CSS bounds must + // be multiplied by the display scale before reaching the addon. + Object.defineProperty(process, 'platform', { value: 'linux' }); + screenGetDisplayMatchingMock.mockReturnValue({ scaleFactor: 1.5 }); + addon.createSession.mockReturnValueOnce('s-scaled'); + addon.getSessionSnapshot.mockReturnValue(snapshot('loading')); + + const cssBounds = { x: 100, y: 40, width: 640, height: 360 }; + service.createSession(cssBounds, '', 1); + service.setBounds('s-scaled', cssBounds); + + const physicalBounds = { x: 150, y: 60, width: 960, height: 540 }; + expect(addon.createSession).toHaveBeenCalledWith( + expect.any(Buffer), + physicalBounds, + '', + 1 + ); + expect(addon.setBounds).toHaveBeenCalledWith( + 's-scaled', + physicalBounds + ); + }); + + it('applies page zoom but not the display scale on macOS', () => { + // NSView frames are in points (device-independent pixels): only + // the webContents zoom factor separates them from CSS pixels. + Object.defineProperty(process, 'platform', { value: 'darwin' }); + screenGetDisplayMatchingMock.mockReturnValue({ scaleFactor: 2 }); + mainWindowGetZoomFactorMock.mockReturnValue(1.25); + addon.createSession.mockReturnValueOnce('s-zoom'); + addon.getSessionSnapshot.mockReturnValue(snapshot('loading')); + + service.createSession({ x: 0, y: 0, width: 100, height: 100 }, '', 1); + + expect(addon.createSession).toHaveBeenCalledWith( + expect.any(Buffer), + { x: 0, y: 0, width: 125, height: 125 }, + '', + 1 + ); + }); + + it('passes frame-copy bounds through unscaled', () => { + // The frame-copy engine paints into a DOM canvas laid out in CSS + // pixels; its adapter applies the display scale to the render + // size itself, so a second scaling pass here would double it. + Object.defineProperty(process, 'platform', { value: 'linux' }); + process.env.IPTVNATOR_ENABLE_EMBEDDED_MPV_FRAME_COPY = '1'; + mockIsFrameCopyRuntimeUsable.mockReturnValue(true); + mockGetFrameCopyRuntimeAvailability.mockReturnValue({ + usable: true, + }); + screenGetDisplayMatchingMock.mockReturnValue({ scaleFactor: 1.5 }); + const frameCopyAddon = createMockAddon(); + frameCopyAddon.createSession.mockReturnValueOnce('s-fc-bounds'); + frameCopyAddon.getSessionSnapshot.mockReturnValue( + snapshot('loading') + ); + ( + service as unknown as { frameCopyAdapter: MockAddon } + ).frameCopyAdapter = frameCopyAddon; + + service.createSession(BOUNDS, '', 1); + service.setBounds('s-fc-bounds', BOUNDS); + + expect(frameCopyAddon.createSession).toHaveBeenCalledWith( + Buffer.alloc(0), + BOUNDS, + '', + 1 + ); + expect(frameCopyAddon.setBounds).toHaveBeenCalledWith( + 's-fc-bounds', + BOUNDS + ); + + // Dispose while the frame-copy env is still set so teardown + // dispatches to the adapter that owns the session. + service.disposeSession('s-fc-bounds'); + }); + }); + it('does not acquire a blocker for a loading session', () => { startSession('s1', snapshot('loading')); expect(powerSaveBlockerMock.start).not.toHaveBeenCalled(); diff --git a/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts b/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts index d30ad596f..8142fb66b 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts @@ -27,6 +27,7 @@ import { EMBEDDED_MPV_SESSION_UPDATE, ResolvedPortalPlayback, } from '@iptvnator/shared/interfaces'; +import { toNativeViewBounds } from './embedded-mpv-bounds.util'; import { EmbeddedMpvFrameCopyAdapter } from './embedded-mpv-frame-copy.adapter'; import { getFrameCopyRuntimeAvailability, @@ -201,6 +202,36 @@ export class EmbeddedMpvNativeService { } } + private getMainWindowZoomFactor(): number { + try { + if (!App.mainWindow || App.mainWindow.isDestroyed()) { + return 1; + } + return App.mainWindow.webContents.getZoomFactor(); + } catch { + return 1; + } + } + + /** + * Renderer bounds arrive in CSS pixels; the native-view engines position + * OS windows in physical pixels (win32/linux) or points (macOS), so at + * page zoom or display scale ≠ 100% the raw values land the video toward + * the window's top-left corner at a fraction of its size (#1145). The + * frame-copy engine must bypass this: it paints into a DOM canvas laid + * out in CSS pixels, and its adapter already applies the display scale + * to the render size itself. + */ + private scaleBoundsForNativeView( + bounds: EmbeddedMpvBounds + ): EmbeddedMpvBounds { + return toNativeViewBounds(bounds, { + platform: process.platform, + zoomFactor: this.getMainWindowZoomFactor(), + displayScaleFactor: this.getMainWindowScaleFactor(), + }); + } + private detectCapabilities(): EmbeddedMpvCapabilities { if (this.isFrameCopyEngineActive()) { return { @@ -419,14 +450,15 @@ export class EmbeddedMpvNativeService { // embed into the window at all. Derive the skip from the dispatched // addon rather than re-evaluating the engine gate, so the two // decisions cannot disagree. - const windowHandle = - this.frameCopyAdapter && addon === this.frameCopyAdapter - ? Buffer.alloc(0) - : this.getMainWindowHandle(); + const usesFrameCopyAddon = + this.frameCopyAdapter !== null && addon === this.frameCopyAdapter; + const windowHandle = usesFrameCopyAddon + ? Buffer.alloc(0) + : this.getMainWindowHandle(); const startedAt = new Date().toISOString(); const sessionId = addon.createSession( windowHandle, - bounds, + usesFrameCopyAddon ? bounds : this.scaleBoundsForNativeView(bounds), title, initialVolume ); @@ -478,7 +510,13 @@ export class EmbeddedMpvNativeService { setBounds(sessionId: string, bounds: EmbeddedMpvBounds): void { this.assertEmbeddedMpvEnabled(); - this.getAddon().setBounds(sessionId, bounds); + const addon = this.getAddon(); + const usesFrameCopyAddon = + this.frameCopyAdapter !== null && addon === this.frameCopyAdapter; + addon.setBounds( + sessionId, + usesFrameCopyAddon ? bounds : this.scaleBoundsForNativeView(bounds) + ); } setPaused(sessionId: string, paused: boolean): EmbeddedMpvSession | null { diff --git a/docs/architecture/embedded-mpv-native.md b/docs/architecture/embedded-mpv-native.md index 19feed60d..458368467 100644 --- a/docs/architecture/embedded-mpv-native.md +++ b/docs/architecture/embedded-mpv-native.md @@ -150,9 +150,10 @@ The flow is: native-view, it starts `mpv --wid=` in a separate process with a private JSON IPC socket. Frame-copy instead uses the per-session helper described below. -9. Resize, scroll, and fullscreen changes are measured in Angular and sent - through bounds sync. Native-view uses them to align the platform host; - frame-copy uses them to resize helper rendering and the canvas frame source. +9. Resize, scroll, fullscreen, and devicePixelRatio changes are measured in + Angular and sent through bounds sync. Native-view uses them to align the + platform host; frame-copy uses them to resize helper rendering and the + canvas frame source. 10. Playback controls remain IPTVnator-owned Angular UI. Frame-copy uses the shared `app-player-controls` overlay through `EmbeddedMpvControlsAdapter`; native-view keeps its compositor-safe fixed @@ -595,6 +596,37 @@ there is no `HIDDEN_BOUNDS`, popover cutout, or reserved dock height. Dialogs and controls layer naturally over the canvas, while bounds sync still updates the helper's render size. +### Coordinate spaces (CSS → native units) + +The renderer measures bounds in CSS pixels (`getBoundingClientRect()`), but +the native-view engines position OS windows, not DOM nodes: the win32 child +`HWND` (`SetWindowPos`) and the Linux child X11 window (`XMoveResizeWindow`) +live in physical pixels, and the macOS `NSView` (`setFrame`) lives in points +(device-independent pixels). CSS values match points only at 100% page zoom +and match physical pixels only at 100% page zoom AND 100% display scale. +`EmbeddedMpvNativeService` therefore converts every native-view bounds payload +in the main process (`toNativeViewBounds` in `embedded-mpv-bounds.util.ts`): +all platforms scale by the webContents zoom factor, win32/linux additionally +by the scale factor of the display hosting the window. The renderer sends +unrounded CSS edges (`measureBounds` does not round) and the conversion +rounds exactly once, after scaling — edges first, width/height derived from +them — so fractional CSS layouts and fractional scales cannot open 1px +seams against the surrounding DOM UI. Skipping this conversion is issue #1145: on scaled +displays (Windows 125%, Linux fractional scaling, HiDPI TVs) the video landed +toward the window's top-left corner at `1/scale` of its size, in windowed and +fullscreen mode alike. + +Frame-copy bounds bypass the conversion: the canvas is laid out by the DOM in +CSS pixels, and the frame-copy adapter already multiplies the render size by +the display scale factor itself. + +Because a monitor change can rescale this mapping without resizing the host +element (moving the window to a display with a different scale keeps the DIP +layout), the session controller also watches `devicePixelRatio` through a +re-armed `matchMedia('(resolution: …dppx)')` query and re-syncs bounds when +it changes; page zoom changes are covered by the same watch plus the ordinary +resize-driven syncs. + ### Controls ownership by engine `EmbeddedMpvPlayerComponent` selects one control owner from diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.spec.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.spec.ts index 9b80e2dfb..0c250d405 100644 --- a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.spec.ts +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.spec.ts @@ -59,7 +59,10 @@ describe('embedded MPV format utilities', () => { expect(volumeLabel(0.755)).toBe('Volume 76%'); }); - it('rounds host bounds and keeps minimum native view dimensions', () => { + it('preserves fractional host edges and keeps minimum native view dimensions', () => { + // Rounding happens once in the main process, after CSS→native + // scaling — pre-rounded edges would drift by up to 1px per scale + // factor on scaled displays. const host = { getBoundingClientRect: () => ({ left: 10.4, @@ -70,8 +73,8 @@ describe('embedded MPV format utilities', () => { } as HTMLElement; expect(measureBounds(host)).toEqual({ - x: 10, - y: 21, + x: 10.4, + y: 20.6, width: 1, height: 1, }); diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.ts index 6678c291b..2e8635b6d 100644 --- a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.ts +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.ts @@ -120,12 +120,19 @@ export function persistVolume(value: number): void { localStorage.setItem('volume', String(value)); } +/** + * Measures the host element in CSS pixels without rounding. The main process + * converts these bounds to native units (page zoom × display scale) and + * rounds exactly once, after scaling — pre-rounding here would bake up to + * ±0.5px of CSS error that the scale factor then amplifies into visible + * off-by-one seams (e.g. a 10.49px edge at 200% renders at 21px, not 20px). + */ export function measureBounds(host: HTMLElement): EmbeddedMpvBounds { const rect = host.getBoundingClientRect(); return { - x: Math.round(rect.left), - y: Math.round(rect.top), - width: Math.max(1, Math.round(rect.width)), - height: Math.max(1, Math.round(rect.height)), + x: rect.left, + y: rect.top, + width: Math.max(1, rect.width), + height: Math.max(1, rect.height), }; } diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.dpr.spec.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.dpr.spec.ts new file mode 100644 index 000000000..79cf74df6 --- /dev/null +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.dpr.spec.ts @@ -0,0 +1,213 @@ +import { TestBed } from '@angular/core/testing'; +import { + EmbeddedMpvSession, + ResolvedPortalPlayback, +} from '@iptvnator/shared/interfaces'; +import { EmbeddedMpvSessionController } from './embedded-mpv-session-controller'; + +/** + * Moving the window to a display with a different scale (or changing the + * page zoom) rescales the CSS→native mapping the backend applies to bounds, + * without necessarily resizing the host element. The controller watches + * devicePixelRatio through a re-armed matchMedia query and re-syncs bounds + * when it changes (#1145). + */ +describe('EmbeddedMpvSessionController devicePixelRatio watch', () => { + class FakeMediaQueryList { + private readonly listeners = new Set<() => void>(); + + constructor(readonly media: string) {} + + addEventListener(_type: 'change', listener: () => void): void { + this.listeners.add(listener); + } + + removeEventListener(_type: 'change', listener: () => void): void { + this.listeners.delete(listener); + } + + fire(): void { + for (const listener of [...this.listeners]) { + listener(); + } + } + + get listenerCount(): number { + return this.listeners.size; + } + } + + let electron: { + platform: string; + getEmbeddedMpvSupport: jest.Mock; + prepareEmbeddedMpv: jest.Mock; + createEmbeddedMpvSession: jest.Mock; + loadEmbeddedMpvPlayback: jest.Mock; + disposeEmbeddedMpvSession: jest.Mock; + setEmbeddedMpvBounds: jest.Mock; + onEmbeddedMpvSessionUpdate: jest.Mock; + }; + let mediaQueries: FakeMediaQueryList[]; + + beforeEach(() => { + electron = { + platform: 'win32', + getEmbeddedMpvSupport: jest + .fn() + .mockResolvedValue({ supported: true, platform: 'win32' }), + prepareEmbeddedMpv: jest + .fn() + .mockResolvedValue({ supported: true, platform: 'win32' }), + createEmbeddedMpvSession: jest + .fn() + .mockResolvedValue(createSession()), + loadEmbeddedMpvPlayback: jest.fn().mockResolvedValue(undefined), + disposeEmbeddedMpvSession: jest.fn().mockResolvedValue(undefined), + setEmbeddedMpvBounds: jest.fn().mockResolvedValue(undefined), + onEmbeddedMpvSessionUpdate: jest.fn(() => jest.fn()), + }; + Object.defineProperty(window, 'electron', { + configurable: true, + value: electron, + }); + + mediaQueries = []; + Object.defineProperty(window, 'matchMedia', { + configurable: true, + value: (media: string) => { + const query = new FakeMediaQueryList(media); + mediaQueries.push(query); + return query; + }, + }); + Object.defineProperty(window, 'devicePixelRatio', { + configurable: true, + value: 1, + }); + Object.defineProperty(globalThis, 'ResizeObserver', { + configurable: true, + value: class MockResizeObserver { + observe = jest.fn(); + disconnect = jest.fn(); + }, + }); + Object.defineProperty(window, 'requestAnimationFrame', { + configurable: true, + value: (callback: FrameRequestCallback) => + window.setTimeout(() => callback(0), 0), + }); + Object.defineProperty(window, 'cancelAnimationFrame', { + configurable: true, + value: (handle: number) => window.clearTimeout(handle), + }); + + TestBed.configureTestingModule({ + providers: [EmbeddedMpvSessionController], + }); + }); + + afterEach(() => { + TestBed.resetTestingModule(); + delete (window as unknown as { electron?: unknown }).electron; + delete (window as unknown as { matchMedia?: unknown }).matchMedia; + jest.restoreAllMocks(); + }); + + it('re-syncs bounds and re-arms the query when devicePixelRatio changes', async () => { + const controller = TestBed.inject(EmbeddedMpvSessionController); + const teardown = controller.startSession( + createHost(), + createPlayback(), + 0.5 + ); + await waitFor( + () => controller.sessionId() === 'mpv-1', + 'session to start' + ); + + expect(mediaQueries.length).toBe(1); + expect(mediaQueries[0].media).toBe('(resolution: 1dppx)'); + electron.setEmbeddedMpvBounds.mockClear(); + + // Simulate a move to a 150%-scaled display. + Object.defineProperty(window, 'devicePixelRatio', { + configurable: true, + value: 1.5, + }); + mediaQueries[0].fire(); + await waitFor( + () => electron.setEmbeddedMpvBounds.mock.calls.length > 0, + 'bounds re-sync after dPR change' + ); + + // The stale query is released and a new one tracks the new ratio. + expect(mediaQueries[0].listenerCount).toBe(0); + expect(mediaQueries.length).toBe(2); + expect(mediaQueries[1].media).toBe('(resolution: 1.5dppx)'); + + // A second display change must fire through the re-armed query. + electron.setEmbeddedMpvBounds.mockClear(); + mediaQueries[1].fire(); + await waitFor( + () => electron.setEmbeddedMpvBounds.mock.calls.length > 0, + 'bounds re-sync after second dPR change' + ); + + teardown(); + expect(mediaQueries[mediaQueries.length - 1].listenerCount).toBe(0); + }); +}); + +function createHost(): HTMLElement { + return { + getBoundingClientRect: () => ({ + left: 10, + top: 20, + width: 640, + height: 360, + }), + } as HTMLElement; +} + +function createPlayback(): ResolvedPortalPlayback { + return { + streamUrl: 'https://example.com/movie.mp4', + title: 'Example Movie', + }; +} + +function createSession(): EmbeddedMpvSession { + return { + id: 'mpv-1', + title: 'Example Movie', + streamUrl: 'https://example.com/movie.mp4', + status: 'playing', + positionSeconds: 0, + durationSeconds: null, + volume: 0.5, + audioTracks: [], + selectedAudioTrackId: null, + subtitleTracks: [], + selectedSubtitleTrackId: null, + playbackSpeed: 1, + aspectOverride: 'no', + recording: { active: false }, + startedAt: '2026-07-19T00:00:00.000Z', + updatedAt: '2026-07-19T00:00:01.000Z', + }; +} + +async function waitFor( + condition: () => boolean, + description: string +): Promise { + const deadline = Date.now() + 1_000; + while (Date.now() < deadline) { + if (condition()) { + return; + } + await Promise.resolve(); + await new Promise((resolve) => window.setTimeout(resolve, 0)); + } + throw new Error(`Timed out waiting for ${description}`); +} diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.spec.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.spec.ts index d2a60a690..b70512184 100644 --- a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.spec.ts +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.spec.ts @@ -141,8 +141,10 @@ describe('EmbeddedMpvSessionController', () => { ); expect(electron.prepareEmbeddedMpv).toHaveBeenCalled(); + // Fractional CSS edges stay unrounded: the main process rounds once, + // after converting them to native units. expect(electron.createEmbeddedMpvSession).toHaveBeenCalledWith( - { x: 11, y: 21, width: 640, height: 360 }, + { x: 10.6, y: 20.5, width: 640, height: 360 }, 'Example Movie', 0.7 ); diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.ts index 36e6e11cb..f760bec97 100644 --- a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.ts +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.ts @@ -151,6 +151,31 @@ export class EmbeddedMpvSessionController { window.addEventListener('resize', scheduleBoundsSync); window.addEventListener('scroll', scheduleBoundsSync, true); + // Page zoom and monitor DPI rescale the CSS→native-pixel mapping the + // backend applies to these bounds. Moving the window to a display + // with a different scale can keep the CSS layout identical (no + // resize, no ResizeObserver), so watch devicePixelRatio through a + // re-armed matchMedia query and re-sync when it changes. + let detachDprWatch: (() => void) | null = null; + const watchDevicePixelRatio = () => { + detachDprWatch?.(); + detachDprWatch = null; + const query = window.matchMedia?.( + `(resolution: ${window.devicePixelRatio}dppx)` + ); + if (!query) { + return; + } + const onChange = () => { + watchDevicePixelRatio(); + scheduleBoundsSync(); + }; + query.addEventListener('change', onChange); + detachDprWatch = () => + query.removeEventListener('change', onChange); + }; + watchDevicePixelRatio(); + const create = async () => { this.session.set(createLoadingSession(playback, initialVolume)); await waitForStartupPaint(); @@ -237,6 +262,8 @@ export class EmbeddedMpvSessionController { resizeObserver.disconnect(); window.removeEventListener('resize', scheduleBoundsSync); window.removeEventListener('scroll', scheduleBoundsSync, true); + detachDprWatch?.(); + detachDprWatch = null; if (this.activeBoundsSync === scheduleBoundsSync) { this.activeBoundsSync = null; From 29e624b0dd68660051d4859d2c733e7f7bfa5e83 Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sun, 19 Jul 2026 07:59:53 +0200 Subject: [PATCH 2/3] ci(release): make test draft releases traceable and self-cleaning (#1202) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ci(release): make test draft releases traceable and self-cleaning Every PR and master build created a draft named "Release v" with tag test-, so 70+ identical drafts piled up and PR drafts were untraceable (for pull_request events github.sha is the ephemeral merge-commit SHA that resolves to nothing in the repo). - Title test drafts as "v — PR # @ [test]" / "v — master @ [test]"; tag releases keep "Release v" - Prepend a context header (PR, head commit, workflow run links) to the auto-generated release notes - Use the PR head SHA and pass target_commitish so generated notes actually cover the PR commits - Use stable tags (test-pr-, test-master) so action-gh-release updates one rolling draft in place instead of creating a new one per push - Mark all non-tag drafts as prerelease - Cancel superseded in-progress PR builds via a concurrency group - Delete a PR's rolling draft when the PR closes (new workflow) Co-Authored-By: Claude Fable 5 * ci(release): close review-bot race windows in draft release flow - Move concurrency from workflow level to job level: cancelling a whole run could interrupt action-gh-release mid-asset-replacement and leave the rolling draft incomplete. Build slots still cancel superseded PR work (matrix-aware groups); the release job gets its own serializing, never-cancelling group. - Re-check the live PR state in the release job right before touching the draft, so a build that outlives its PR cannot recreate the draft after cleanup deleted it. - In the cleanup workflow, cancel still-running builds of the closed PR (dead work anyway) and wait for them to settle before deleting. - Emit an explicit empty `body=` output for tag builds instead of a blank-line heredoc. Addresses Codex and Greptile review feedback on #1202. Co-Authored-By: Claude Fable 5 * ci(release): grant actions:write so PR-close cleanup can cancel builds gh run cancel needs the actions scope; with only contents: write the cancellation 403s silently and the settle-poll burns its full window. Also skip the cleanup job for fork PRs entirely: they never get a draft and their token is read-only regardless of the permissions block. Addresses Greptile P1 / Codex P2 follow-up on #1202. Co-Authored-By: Claude Fable 5 * ci(release): re-assert rolling draft title after asset upload action-gh-release@v2 updates name/body/target_commitish on the normal draft-reuse path, but in a rare race (release listing transiently missing the draft) it uploads assets to the canonical oldest draft without refreshing its metadata. PATCH the title and commitish on the release id the action actually used, so the draft title always names the current head SHA; the body is left alone to preserve generated notes. Addresses Codex round-2 feedback on #1202. Co-Authored-By: Claude Fable 5 * ci(release): prune stale assets before updating a rolling draft The release action only replaces same-name assets, so a PR that bumps the app version would leave old-version installers beside the new set in its rolling draft. Delete all existing assets of the matched draft before the upload; the action re-uploads the full current set right after. Published releases are never touched. Addresses Codex round-3 feedback on #1202. Co-Authored-By: Claude Fable 5 * ci(release): rebuild full draft metadata after asset upload Extend the post-upload metadata step to also rebuild the body (context header + notes from the same generate-notes API the action uses), not just title/commitish. The rolling draft now ends up with correct metadata regardless of which internal action-gh-release path ran, including the rare canonicalize-duplicate fallback. If notes generation fails, the body is left as the action set it. Addresses Codex round-4 feedback on #1202. Co-Authored-By: Claude Fable 5 * ci(release): only cancel pull_request runs when cleaning up a closed PR A manually dispatched build on the same head branch is not the PR's work; filter the cancellation list by event so PR-close cleanup cannot abort it. Addresses Codex round-5 feedback on #1202. Co-Authored-By: Claude Fable 5 * ci(release): guard PR-close cleanup against close-reopen races Re-check the live PR state at the start of the cleanup job and again right before deleting the draft, so a PR that is reopened while the cleanup is queued or waiting keeps its rolling draft and its fresh reopened-run builds are not cancelled. Addresses Codex round-6 feedback on #1202. Co-Authored-By: Claude Fable 5 * ci(release): keep tag_name when patching rolling draft metadata PATCHing a draft release without tag_name makes GitHub drop the pending tag (the draft turns into untagged-), so the next run cannot find the rolling draft by tag and creates a duplicate — observed live on this PR's own drafts. Include tag_name in both PATCH payloads of the metadata step. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- .github/workflows/build-and-make.yaml | 181 ++++++++++++++++++++++++- .github/workflows/cleanup-pr-draft.yml | 92 +++++++++++++ 2 files changed, 270 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/cleanup-pr-draft.yml diff --git a/.github/workflows/build-and-make.yaml b/.github/workflows/build-and-make.yaml index 8df8b6e81..14fd1cc94 100644 --- a/.github/workflows/build-and-make.yaml +++ b/.github/workflows/build-and-make.yaml @@ -16,6 +16,13 @@ jobs: name: Build pinned Linux Embedded MPV runtime runs-on: ubuntu-22.04 timeout-minutes: 120 + # Concurrency lives on the build jobs, not the workflow: cancelling a + # whole run could interrupt action-gh-release mid-update and leave the + # rolling draft with missing assets. Build slots cancel superseded PR + # work; the release job only serializes and is never cancelled. + concurrency: + group: ${{ github.workflow }}-build-linux-runtime-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} steps: - name: Checkout code @@ -304,6 +311,9 @@ jobs: name: Build on ${{ matrix.os }} ${{ matrix.arch }} runs-on: ${{ matrix.runner }} timeout-minutes: 120 + concurrency: + group: ${{ github.workflow }}-build-${{ matrix.os }}-${{ matrix.arch }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} strategy: fail-fast: false matrix: @@ -1200,6 +1210,9 @@ jobs: needs: linux-embedded-mpv-runtime runs-on: ${{ matrix.runner }} timeout-minutes: 120 + concurrency: + group: ${{ github.workflow }}-build-${{ matrix.os }}-${{ matrix.linux_profile }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} strategy: fail-fast: false matrix: @@ -1235,6 +1248,9 @@ jobs: - build-linux if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} runs-on: ubuntu-latest + concurrency: + group: ${{ github.workflow }}-release-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: false permissions: contents: write @@ -1367,13 +1383,117 @@ jobs: id: package-version run: echo "version=$(node -p "require('./package.json').version")" >> $GITHUB_OUTPUT + # Test drafts (PR/master) get a self-describing title plus a context + # header linking the PR, real head commit, and workflow run. A stable + # tag per PR (test-pr-) / branch (test-master) makes the action + # update one rolling draft in place instead of piling up a new draft + # for every push. PR builds must not use github.sha here: that is the + # ephemeral merge-commit SHA, which resolves to nothing in the repo. + - name: Compose release metadata + id: release-meta + shell: bash + env: + VERSION: ${{ steps.package-version.outputs.version }} + EVENT_NAME: ${{ github.event_name }} + IS_TAG_BUILD: ${{ startsWith(github.ref, 'refs/tags/') }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_TITLE: ${{ github.event.pull_request.title }} + PR_URL: ${{ github.event.pull_request.html_url }} + HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + SOURCE_BRANCH: ${{ github.head_ref || github.ref_name }} + REPO_URL: ${{ github.server_url }}/${{ github.repository }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + + SHORT_SHA="${HEAD_SHA:0:7}" + SAFE_BRANCH="${SOURCE_BRANCH//\//-}" + + if [ "${IS_TAG_BUILD}" = "true" ]; then + NAME="Release v${VERSION}" + TAG="${GITHUB_REF_NAME}" + BODY="" + elif [ "${EVENT_NAME}" = "pull_request" ]; then + NAME="v${VERSION} — PR #${PR_NUMBER} @ ${SHORT_SHA} [test]" + TAG="test-pr-${PR_NUMBER}" + BODY="$(printf '🧪 Test build for PR [#%s](%s) — %s\n\nCommit [`%s`](%s/commit/%s) · branch `%s` · [workflow run](%s)' \ + "${PR_NUMBER}" "${PR_URL}" "${PR_TITLE}" \ + "${SHORT_SHA}" "${REPO_URL}" "${HEAD_SHA}" "${SOURCE_BRANCH}" "${RUN_URL}")" + else + NAME="v${VERSION} — ${SAFE_BRANCH} @ ${SHORT_SHA} [test]" + TAG="test-${SAFE_BRANCH}" + BODY="$(printf '🧪 Test build from `%s` — commit [`%s`](%s/commit/%s) · [workflow run](%s)' \ + "${SOURCE_BRANCH}" "${SHORT_SHA}" "${REPO_URL}" "${HEAD_SHA}" "${RUN_URL}")" + fi + + { + echo "name=${NAME}" + echo "tag=${TAG}" + echo "commitish=${HEAD_SHA}" + } >> "${GITHUB_OUTPUT}" + + if [ -n "${BODY}" ]; then + { + echo "body<> "${GITHUB_OUTPUT}" + else + echo "body=" >> "${GITHUB_OUTPUT}" + fi + + # A PR can be closed while this workflow is still running; the + # cleanup workflow deletes the PR draft on close. Re-check the live + # PR state right before touching the draft so a late-finishing run + # cannot recreate a draft for a closed PR. + - name: Check PR is still open + if: github.event_name == 'pull_request' + id: pr-state + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + echo "state=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${{ github.event.pull_request.number }}" --jq '.state')" >> "${GITHUB_OUTPUT}" + + # The rolling draft keeps assets across runs and the release action + # only replaces same-name files. If the app version changes between + # pushes, old-version installers would linger beside the new set, + # so drop every existing asset first — the action re-uploads the + # full current set right after. Only drafts are pruned; published + # releases are never touched. + - name: Prune stale draft assets + if: github.event_name != 'pull_request' || steps.pr-state.outputs.state == 'open' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RELEASE_TAG: ${{ steps.release-meta.outputs.tag }} + run: | + set -euo pipefail + + release_id="$(gh api "repos/${GITHUB_REPOSITORY}/releases?per_page=100" --paginate | + jq -s --arg tag "${RELEASE_TAG}" \ + 'add | [.[] | select(.draft and .tag_name == $tag)][0].id // empty')" + + if [ -z "${release_id}" ]; then + echo "No existing draft for ${RELEASE_TAG}; nothing to prune." + exit 0 + fi + + gh api "repos/${GITHUB_REPOSITORY}/releases/${release_id}/assets?per_page=100" --paginate --jq '.[].id' | + xargs -r -n1 -I{} gh api -X DELETE "repos/${GITHUB_REPOSITORY}/releases/assets/{}" + + echo "Pruned assets from draft ${release_id} (${RELEASE_TAG})." + - name: Create Draft Release + id: draft-release + if: github.event_name != 'pull_request' || steps.pr-state.outputs.state == 'open' uses: softprops/action-gh-release@v2 with: draft: true - prerelease: ${{ github.event_name == 'pull_request' }} - name: Release v${{ steps.package-version.outputs.version }} - tag_name: ${{ startsWith(github.ref, 'refs/tags/') && github.ref_name || format('test-{0}', github.sha) }} + prerelease: ${{ !startsWith(github.ref, 'refs/tags/') }} + name: ${{ steps.release-meta.outputs.name }} + tag_name: ${{ steps.release-meta.outputs.tag }} + target_commitish: ${{ steps.release-meta.outputs.commitish }} + body: ${{ steps.release-meta.outputs.body }} generate_release_notes: true files: | artifacts/macos-x64-artifacts/*-x64.dmg @@ -1400,3 +1520,58 @@ jobs: artifacts/windows-artifacts/*.blockmap env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + # Rare action-gh-release path: when the release listing transiently + # misses the rolling draft, the action creates a duplicate, deletes + # it in favor of the canonical (oldest) draft, and uploads assets + # there WITHOUT refreshing that draft's metadata. Rebuild the full + # metadata (title, commitish, and body = context header + notes + # from the same generate-notes API the action uses) on the release + # id the action actually used, so the draft ends up correct no + # matter which internal path ran. If notes generation fails, the + # body is left as the action set it and only title/commitish are + # re-asserted. + - name: Ensure draft metadata is current + if: steps.draft-release.outputs.id != '' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RELEASE_ID: ${{ steps.draft-release.outputs.id }} + RELEASE_TAG: ${{ steps.release-meta.outputs.tag }} + RELEASE_NAME: ${{ steps.release-meta.outputs.name }} + RELEASE_COMMITISH: ${{ steps.release-meta.outputs.commitish }} + RELEASE_BODY: ${{ steps.release-meta.outputs.body }} + run: | + set -euo pipefail + + GENERATED_NOTES="$(gh api -X POST "repos/${GITHUB_REPOSITORY}/releases/generate-notes" \ + -f tag_name="${RELEASE_TAG}" \ + -f target_commitish="${RELEASE_COMMITISH}" \ + --jq '.body' || true)" + + # tag_name MUST be included in every PATCH: updating a draft + # without it makes GitHub drop the pending tag (the draft + # becomes "untagged-"), which breaks the rolling-draft + # lookup on the next run. + if [ -z "${GENERATED_NOTES}" ]; then + jq -n \ + --arg tag "${RELEASE_TAG}" \ + --arg name "${RELEASE_NAME}" \ + --arg commitish "${RELEASE_COMMITISH}" \ + '{tag_name: $tag, name: $name, target_commitish: $commitish}' | + gh api -X PATCH "repos/${GITHUB_REPOSITORY}/releases/${RELEASE_ID}" --input - > /dev/null + exit 0 + fi + + if [ -n "${RELEASE_BODY}" ]; then + FULL_BODY="$(printf '%s\n\n%s' "${RELEASE_BODY}" "${GENERATED_NOTES}")" + else + FULL_BODY="${GENERATED_NOTES}" + fi + + jq -n \ + --arg tag "${RELEASE_TAG}" \ + --arg name "${RELEASE_NAME}" \ + --arg commitish "${RELEASE_COMMITISH}" \ + --arg body "${FULL_BODY}" \ + '{tag_name: $tag, name: $name, target_commitish: $commitish, body: ($body | .[0:120000])}' | + gh api -X PATCH "repos/${GITHUB_REPOSITORY}/releases/${RELEASE_ID}" --input - > /dev/null diff --git a/.github/workflows/cleanup-pr-draft.yml b/.github/workflows/cleanup-pr-draft.yml new file mode 100644 index 000000000..695685255 --- /dev/null +++ b/.github/workflows/cleanup-pr-draft.yml @@ -0,0 +1,92 @@ +name: Cleanup PR Draft Release + +on: + pull_request: + types: [closed] + +# contents: write — delete the draft release; actions: write — cancel the +# closed PR's still-running build workflow before deleting. +permissions: + actions: write + contents: write + +jobs: + delete-draft: + name: Delete PR draft release + # Fork PRs never get a draft (the release job skips them) and their + # GITHUB_TOKEN is read-only regardless of the permissions block, so + # there is nothing to cancel or delete. + if: github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + steps: + # A closed PR can be reopened while this job is still queued or + # waiting; a reopened PR's fresh build must not be cancelled and + # its draft must not be deleted. Check the live state up front + # (and again right before deleting below). + - name: Check PR is still closed + id: pr-state + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set -euo pipefail + echo "state=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" --jq '.state')" >> "${GITHUB_OUTPUT}" + + # A build for this PR may still be running and would recreate the + # rolling draft after we delete it. Cancel those runs (dead work + # for a closed PR anyway) and wait for them to wind down. The + # release job additionally re-checks the live PR state, so this + # wait is defense in depth, not the only guard. + - name: Cancel in-progress builds for the closed PR + if: steps.pr-state.outputs.state == 'closed' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + HEAD_BRANCH: ${{ github.event.pull_request.head.ref }} + run: | + set -euo pipefail + + # --event pull_request: a manually dispatched build on the + # same branch is not this PR's work and must not be cancelled. + list_active_runs() { + gh run list --repo "${GITHUB_REPOSITORY}" \ + --workflow 'Build and Make Electron App' \ + --branch "${HEAD_BRANCH}" \ + --event pull_request \ + --json databaseId,status \ + --jq '.[] | select(.status == "queued" or .status == "in_progress" or .status == "waiting" or .status == "requested" or .status == "pending") | .databaseId' + } + + for run_id in $(list_active_runs); do + echo "Cancelling run ${run_id}" + gh run cancel "${run_id}" --repo "${GITHUB_REPOSITORY}" || true + done + + # Cancellation is asynchronous; poll until the runs settle. + for _ in $(seq 1 18); do + if [ -z "$(list_active_runs)" ]; then + break + fi + sleep 10 + done + + # Draft releases have no real git tag, so a lookup via + # releases/tags/ returns 404. List releases and match the + # draft by its stored tag_name (test-pr-) instead. The PR state + # is re-checked one last time right before deleting, in case the + # PR was reopened during the cancellation wait above. + - name: Delete draft release for closed PR + if: steps.pr-state.outputs.state == 'closed' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set -euo pipefail + + if [ "$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" --jq '.state')" != "closed" ]; then + echo "PR #${PR_NUMBER} was reopened; keeping its draft." + exit 0 + fi + + gh api "repos/${GITHUB_REPOSITORY}/releases?per_page=100" --paginate \ + --jq ".[] | select(.draft and .tag_name == \"test-pr-${PR_NUMBER}\") | .id" | + xargs -r -n1 -I{} gh api -X DELETE "repos/${GITHUB_REPOSITORY}/releases/{}" From 5cae310430966b065a67f20d722b767dea4af25d Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sun, 19 Jul 2026 08:30:21 +0200 Subject: [PATCH 3/3] fix(logging): redact sensitive portal and Electron diagnostics (#1182) * fix: redact sensitive log data * fix(ci): keep logging preload self-contained * fix(logging): preserve shared diagnostics * fix(logging): close trace redaction gaps * fix(logging): redact Xtream path credentials * fix(logging): close credential redaction gaps * fix(logging): suppress external player arguments * fix(logging): harden URL and date redaction * fix(logging): redact map keys and URL fragments * fix(logging): redact sensitive map values * fix(logging): redact credentials in diagnostic text * fix(logging): close remaining credential leaks --- AGENTS.md | 4 + CLAUDE.md | 5 + .../app/events/portal-debug.events.spec.ts | 33 ++ .../src/app/events/portal-debug.events.ts | 114 +---- .../src/app/events/settings.events.spec.ts | 70 ++++ .../src/app/events/settings.events.ts | 6 +- .../src/app/events/stalker.events.ts | 6 +- .../src/app/events/xtream.events.ts | 11 +- .../src/app/services/debug-trace.spec.ts | 47 +++ .../src/app/services/debug-trace.ts | 8 +- .../src/app/services/electron.service.spec.ts | 20 + apps/web/src/app/services/electron.service.ts | 26 +- apps/web/src/app/services/pwa.service.ts | 13 +- docs/architecture/electron-security.md | 20 + .../portal/shared/util/src/lib/logger.spec.ts | 47 ++- libs/portal/shared/util/src/lib/logger.ts | 23 +- .../src/lib/stalker-session.service.spec.ts | 35 +- .../src/lib/stalker-session.service.ts | 27 +- .../src/lib/services/xtream-api.service.ts | 4 +- libs/shared/logging/jest.config.ts | 13 + libs/shared/logging/project.json | 20 + libs/shared/logging/src/index.ts | 5 + .../src/lib/redact-sensitive-data.spec.ts | 357 ++++++++++++++++ .../logging/src/lib/redact-sensitive-data.ts | 395 ++++++++++++++++++ libs/shared/logging/tsconfig.json | 19 + libs/shared/logging/tsconfig.lib.json | 10 + libs/shared/logging/tsconfig.spec.json | 15 + tools/coverage/coverage-policy.json | 7 + tsconfig.base.json | 1 + 29 files changed, 1203 insertions(+), 158 deletions(-) create mode 100644 apps/electron-backend/src/app/events/settings.events.spec.ts create mode 100644 apps/electron-backend/src/app/services/debug-trace.spec.ts create mode 100644 libs/shared/logging/jest.config.ts create mode 100644 libs/shared/logging/project.json create mode 100644 libs/shared/logging/src/index.ts create mode 100644 libs/shared/logging/src/lib/redact-sensitive-data.spec.ts create mode 100644 libs/shared/logging/src/lib/redact-sensitive-data.ts create mode 100644 libs/shared/logging/tsconfig.json create mode 100644 libs/shared/logging/tsconfig.lib.json create mode 100644 libs/shared/logging/tsconfig.spec.json diff --git a/AGENTS.md b/AGENTS.md index 7371074f5..fb252fee7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -73,6 +73,10 @@ IPTVNATOR_TRACE_STARTUP=1 nx serve electron-backend - `IPTVNATOR_TRACE_PLAYER=1` traces external-player activity and bounded Embedded MPV runtime-probe stderr - `IPTVNATOR_TRACE_RENDERER_CONSOLE=1` mirrors renderer console output into the Electron terminal +- Settings, portal request/response, and trace payloads must use + `@iptvnator/shared/logging` or the redacting portal logger before reaching + `console.*`; never log raw credentials while debugging. + - GPU/compositor debugging: ```bash diff --git a/CLAUDE.md b/CLAUDE.md index 16fc9a0e2..d3d18d399 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -130,6 +130,10 @@ Useful narrower flags: - `IPTVNATOR_TRACE_PLAYER=1` traces external-player activity and bounded Embedded MPV runtime-probe stderr - `IPTVNATOR_TRACE_RENDERER_CONSOLE=1` mirrors renderer console logs into the Electron terminal +Settings, portal request/response, and trace payloads must use +`@iptvnator/shared/logging` or the redacting portal logger before reaching +`console.*`; never log raw credentials while debugging. + For GPU/compositor debugging: ```bash @@ -238,6 +242,7 @@ This is an Nx monorepo with the following structure: - **portal/shared/{data-access,ui,util}** - Cross-portal shared code - **services** - Abstract DataService contract and shared app services (incl. the TMDB metadata enrichment module in `lib/tmdb/`) - **shared/interfaces** - TypeScript interfaces and types (incl. `ElectronBridgeApi`) + - **shared/logging** - Dependency-free structured redaction for diagnostic logs - **shared/database** - Canonical Drizzle schema and DB connection (used by the Electron backend) - **shared/m3u-utils** - M3U playlist utilities - **shared/testing** - Shared test helpers diff --git a/apps/electron-backend/src/app/events/portal-debug.events.spec.ts b/apps/electron-backend/src/app/events/portal-debug.events.spec.ts index 9912c78f3..b64c4ead5 100644 --- a/apps/electron-backend/src/app/events/portal-debug.events.spec.ts +++ b/apps/electron-backend/src/app/events/portal-debug.events.spec.ts @@ -87,4 +87,37 @@ describe('sanitizePortalDebugEvent', () => { }, }); }); + + it('redacts credentials in request, response, errors, and URL query params', () => { + const secrets = { + username: 'main-user-secret', + password: 'main-password-secret', + token: 'main-token-secret', + authorization: 'main-authorization-secret', + mac: 'main-mac-secret', + }; + const event = sanitizePortalDebugEvent({ + requestId: 'req-redaction', + provider: 'stalker', + operation: 'get_profile', + transport: 'electron-main', + startedAt: new Date().toISOString(), + durationMs: 5, + status: 'error', + request: { + params: secrets, + url: `https://example.com/portal?token=${secrets.token}&action=get_profile`, + }, + error: new Error( + `Request failed: https://example.com/portal?authorization=${secrets.authorization}&action=get_profile` + ), + }); + + const output = JSON.stringify(event); + for (const secret of Object.values(secrets)) { + expect(output).not.toContain(secret); + } + expect(output).toContain('get_profile'); + expect(output).toContain('req-redaction'); + }); }); diff --git a/apps/electron-backend/src/app/events/portal-debug.events.ts b/apps/electron-backend/src/app/events/portal-debug.events.ts index 153efee15..c9fb68142 100644 --- a/apps/electron-backend/src/app/events/portal-debug.events.ts +++ b/apps/electron-backend/src/app/events/portal-debug.events.ts @@ -1,117 +1,19 @@ import { BrowserWindow } from 'electron'; -import { PORTAL_DEBUG_EVENT, PortalDebugEvent } from '@iptvnator/shared/interfaces'; +import { + PORTAL_DEBUG_EVENT, + PortalDebugEvent, +} from '@iptvnator/shared/interfaces'; +import { redactSensitiveData } from '@iptvnator/shared/logging'; import { environment } from '../../environments/environment'; -const MAX_DEBUG_DEPTH = 6; - -function sanitizePortalDebugValue( - value: unknown, - seen = new WeakSet(), - depth = 0 -): unknown { - if ( - value == null || - typeof value === 'string' || - typeof value === 'number' || - typeof value === 'boolean' - ) { - return value; - } - - if (typeof value === 'bigint') { - return value.toString(); - } - - if ( - typeof value === 'function' || - typeof value === 'symbol' - ) { - return undefined; - } - - if (depth >= MAX_DEBUG_DEPTH) { - return '[MaxDepth]'; - } - - if (value instanceof Error) { - const baseError = { - name: value.name, - message: value.message, - stack: value.stack, - } as Record; - - for (const [key, entry] of Object.entries( - value as unknown as Record - )) { - baseError[key] = sanitizePortalDebugValue( - entry, - seen, - depth + 1 - ); - } - - return baseError; - } - - if (value instanceof Date) { - return value.toISOString(); - } - - if (value instanceof URL) { - return value.toString(); - } - - if (Array.isArray(value)) { - return value.map((entry) => - sanitizePortalDebugValue(entry, seen, depth + 1) - ); - } - - if (value instanceof Map) { - return Object.fromEntries( - [...value.entries()].map(([key, entry]) => [ - String(key), - sanitizePortalDebugValue(entry, seen, depth + 1), - ]) - ); - } - - if (value instanceof Set) { - return [...value].map((entry) => - sanitizePortalDebugValue(entry, seen, depth + 1) - ); - } - - if (typeof value === 'object') { - if (seen.has(value)) { - return '[Circular]'; - } - - seen.add(value); - - const entries = Object.entries(value as Record).map( - ([key, entry]) => [ - key, - sanitizePortalDebugValue(entry, seen, depth + 1), - ] - ); - - seen.delete(value); - - return Object.fromEntries(entries); - } - - return String(value); -} - export function sanitizePortalDebugEvent( event: PortalDebugEvent ): PortalDebugEvent { return { ...event, - request: sanitizePortalDebugValue(event.request), - response: sanitizePortalDebugValue(event.response), - error: sanitizePortalDebugValue(event.error), + request: redactSensitiveData(event.request), + response: redactSensitiveData(event.response), + error: redactSensitiveData(event.error), }; } diff --git a/apps/electron-backend/src/app/events/settings.events.spec.ts b/apps/electron-backend/src/app/events/settings.events.spec.ts new file mode 100644 index 000000000..fa0825ea3 --- /dev/null +++ b/apps/electron-backend/src/app/events/settings.events.spec.ts @@ -0,0 +1,70 @@ +const handlers = new Map unknown>(); + +jest.mock('electron', () => ({ + ipcMain: { + handle: jest.fn( + (channel: string, handler: (...args: unknown[]) => unknown) => { + handlers.set(channel, handler); + } + ), + }, +})); + +jest.mock('../services/store.service', () => ({ + MPV_PLAYER_ARGUMENTS: 'mpvPlayerArguments', + MPV_REUSE_INSTANCE: 'mpvReuseInstance', + VLC_PLAYER_ARGUMENTS: 'vlcPlayerArguments', + VLC_REUSE_INSTANCE: 'vlcReuseInstance', + store: { get: jest.fn(), set: jest.fn() }, +})); + +jest.mock('../server/http-server', () => ({ + httpServer: { updateSettings: jest.fn() }, +})); + +describe('SETTINGS_UPDATE logging', () => { + beforeEach(async () => { + jest.spyOn(console, 'log').mockImplementation(() => undefined); + await import('./settings.events'); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('does not print a TMDB apiKey while retaining useful fields', () => { + const apiKey = 'tmdb-settings-api-key-secret'; + const handler = handlers.get('SETTINGS_UPDATE'); + + expect(handler).toBeDefined(); + handler?.({}, { language: 'de', tmdb: { apiKey, enabled: true } }); + + const output = JSON.stringify((console.log as jest.Mock).mock.calls); + expect(output).not.toContain(apiKey); + expect(output).toContain('language'); + expect(output).toContain('de'); + expect(output).toContain('enabled'); + }); + + it('does not print credentials embedded in external player arguments', () => { + const authorizationSecret = 'player-authorization-secret'; + const cookieSecret = 'player-cookie-secret'; + const handler = handlers.get('SETTINGS_UPDATE'); + + expect(handler).toBeDefined(); + handler?.( + {}, + { + language: 'de', + mpvPlayerArguments: `--http-header-fields=Authorization: Bearer ${authorizationSecret}`, + vlcPlayerArguments: `--http-referrer=https://example.com --http-cookie=Cookie: ${cookieSecret}`, + } + ); + + const output = JSON.stringify((console.log as jest.Mock).mock.calls); + expect(output).not.toContain(authorizationSecret); + expect(output).not.toContain(cookieSecret); + expect(output).toContain('language'); + expect(output).toContain('de'); + }); +}); diff --git a/apps/electron-backend/src/app/events/settings.events.ts b/apps/electron-backend/src/app/events/settings.events.ts index 5eb290f2c..d05ccdcf0 100644 --- a/apps/electron-backend/src/app/events/settings.events.ts +++ b/apps/electron-backend/src/app/events/settings.events.ts @@ -1,5 +1,6 @@ import { ipcMain } from 'electron'; import { normalizeExternalPlayerArguments } from '@iptvnator/shared/interfaces'; +import { redactSensitiveData } from '@iptvnator/shared/logging'; import { EMBEDDED_MPV_FRAME_COPY, MPV_PLAYER_ARGUMENTS, @@ -17,7 +18,10 @@ export default class SettingsEvents { } ipcMain.handle('SETTINGS_UPDATE', (_event, arg) => { - console.log('Received SETTINGS_UPDATE with data:', arg); + console.log( + 'Received SETTINGS_UPDATE with data:', + redactSensitiveData(arg) + ); if (arg.mpvPlayerArguments !== undefined) { store.set( diff --git a/apps/electron-backend/src/app/events/stalker.events.ts b/apps/electron-backend/src/app/events/stalker.events.ts index d16fd8cba..a46a039f5 100644 --- a/apps/electron-backend/src/app/events/stalker.events.ts +++ b/apps/electron-backend/src/app/events/stalker.events.ts @@ -9,6 +9,7 @@ import { PortalDebugEvent, STALKER_REQUEST, } from '@iptvnator/shared/interfaces'; +import { redactSensitiveData } from '@iptvnator/shared/logging'; import { rememberStalkerPlaybackContext } from '../services/stalker-playback-context.service'; import { emitPortalDebugEvent } from './portal-debug.events'; import { buildStalkerIdentityRequestContext } from './stalker-identity'; @@ -186,7 +187,10 @@ ipcMain.handle( emitPortalDebugEvent(debugEvent); } - console.error('[StalkerEvents] Request error:', error); + console.error( + '[StalkerEvents] Request error:', + redactSensitiveData(error) + ); // Format error response if (axios.isAxiosError(error)) { diff --git a/apps/electron-backend/src/app/events/xtream.events.ts b/apps/electron-backend/src/app/events/xtream.events.ts index da96caa5e..0d63e0523 100644 --- a/apps/electron-backend/src/app/events/xtream.events.ts +++ b/apps/electron-backend/src/app/events/xtream.events.ts @@ -10,6 +10,7 @@ import { XTREAM_CANCEL_SESSION, normalizeXtreamServerUrl, } from '@iptvnator/shared/interfaces'; +import { redactSensitiveData } from '@iptvnator/shared/logging'; import { emitPortalDebugEvent } from './portal-debug.events'; import { UnsafeUrlError } from './url-safety'; import { requestWithValidatedRedirects } from '../util/validated-axios'; @@ -212,10 +213,12 @@ ipcMain.handle( if (!payload.suppressErrorLog) { console.error( '[XTREAM_REQUEST] Failed', - formatXtreamError( - error, - requestUrlForLog, - payload.params?.action + redactSensitiveData( + formatXtreamError( + error, + requestUrlForLog, + payload.params?.action + ) ) ); } diff --git a/apps/electron-backend/src/app/services/debug-trace.spec.ts b/apps/electron-backend/src/app/services/debug-trace.spec.ts new file mode 100644 index 000000000..91da7daed --- /dev/null +++ b/apps/electron-backend/src/app/services/debug-trace.spec.ts @@ -0,0 +1,47 @@ +import { trace } from './debug-trace'; + +describe('debug trace redaction', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('does not serialize credentials from nested payloads or URLs', () => { + jest.spyOn(console, 'log').mockImplementation(() => undefined); + const secrets = { + password: 'trace-password-secret', + token: 'trace-token-secret', + authorization: 'trace-authorization-secret', + mac: 'trace-mac-secret', + }; + + trace('portal', 'request', { + params: secrets, + url: `https://example.com/portal?token=${secrets.token}&action=get_profile`, + requestId: 'diagnostic-request-id', + }); + + const output = JSON.stringify((console.log as jest.Mock).mock.calls); + for (const secret of Object.values(secrets)) { + expect(output).not.toContain(secret); + } + expect(output).toContain('diagnostic-request-id'); + expect(output).toContain('get_profile'); + }); + + it('redacts serialized credentials before truncating trace strings', () => { + jest.spyOn(console, 'log').mockImplementation(() => undefined); + const secret = 'long-trace-json-password-secret'; + const diagnostic = JSON.stringify({ + password: secret, + operation: 'get_profile', + padding: 'x'.repeat(300), + }); + + trace('portal', 'request', { diagnostic }); + + const output = JSON.stringify((console.log as jest.Mock).mock.calls); + expect(output).not.toContain(secret); + expect(output).toContain('[Redacted]'); + expect(output).toContain('get_profile'); + }); +}); diff --git a/apps/electron-backend/src/app/services/debug-trace.ts b/apps/electron-backend/src/app/services/debug-trace.ts index 19a727ea3..f134dc7f3 100644 --- a/apps/electron-backend/src/app/services/debug-trace.ts +++ b/apps/electron-backend/src/app/services/debug-trace.ts @@ -1,3 +1,5 @@ +import { redactSensitiveData } from '@iptvnator/shared/logging'; + const TRACE_ENV_TRUE_VALUES = new Set(['1', 'true', 'yes', 'on']); const TRACE_PREFIX = '[IPTVnator Trace]'; const MAX_TRACE_ARRAY_ITEMS = 5; @@ -146,10 +148,10 @@ export function summarizeForTrace(value: unknown, depth = 0): unknown { export function safeStringifyForTrace(payload: unknown): string { try { - return JSON.stringify(payload); + return JSON.stringify(redactSensitiveData(payload)); } catch (error) { return JSON.stringify({ - fallback: summarizeForTrace(payload), + fallback: summarizeForTrace(redactSensitiveData(payload)), stringifyError: error instanceof Error ? truncateString(error.message) @@ -166,7 +168,7 @@ export function trace(scope: string, message: string, payload?: unknown): void { console.log( `${TRACE_PREFIX}[${scope}] ${message} ${safeStringifyForTrace( - summarizeForTrace(payload) + summarizeForTrace(redactSensitiveData(payload)) )}` ); } diff --git a/apps/web/src/app/services/electron.service.spec.ts b/apps/web/src/app/services/electron.service.spec.ts index ad0c5015b..e7000a6e4 100644 --- a/apps/web/src/app/services/electron.service.spec.ts +++ b/apps/web/src/app/services/electron.service.spec.ts @@ -16,6 +16,7 @@ describe('ElectronService', () => { const session = { id: 'session-1' }; let electronBridge: { fetchPlaylistByUrl: jest.Mock; + onPlayerError: jest.Mock; openInMpv: jest.Mock; openInVlc: jest.Mock; }; @@ -24,9 +25,11 @@ describe('ElectronService', () => { beforeEach(() => { jest.spyOn(console, 'log').mockImplementation(() => undefined); + jest.spyOn(console, 'error').mockImplementation(() => undefined); electronBridge = { fetchPlaylistByUrl: jest.fn(), + onPlayerError: jest.fn(), openInMpv: jest.fn().mockResolvedValue(session), openInVlc: jest.fn().mockResolvedValue(session), }; @@ -99,6 +102,23 @@ describe('ElectronService', () => { expect(electronBridge.fetchPlaylistByUrl).not.toHaveBeenCalled(); }); + it('redacts credentials from backend player errors before logging', () => { + const secret = 'player-error-token-secret'; + const listener = electronBridge.onPlayerError.mock.calls[0][0]; + + listener({ + player: 'MPV', + error: 'Playback failed', + originalError: `Request failed: token=${secret}&channel=news`, + }); + + const output = JSON.stringify( + (console.error as jest.Mock).mock.calls + ); + expect(output).not.toContain(secret); + expect(output).toContain('channel=news'); + }); + it('shows the trust-host action for Electron-wrapped security errors', async () => { const securityPayload = { code: ELECTRON_BRIDGE_SECURITY_ERROR_CODES.InvalidTlsCertificate, diff --git a/apps/web/src/app/services/electron.service.ts b/apps/web/src/app/services/electron.service.ts index 00f3d96af..7ffdaf6f5 100644 --- a/apps/web/src/app/services/electron.service.ts +++ b/apps/web/src/app/services/electron.service.ts @@ -7,7 +7,6 @@ import { DialogService } from '@iptvnator/ui/components'; import { DataService, SettingsStore } from '@iptvnator/services'; import { AUTO_UPDATE_PLAYLISTS, - createDevLogger, ELECTRON_BRIDGE_SECURITY_ERROR_CODES, ERROR, normalizeHost, @@ -22,6 +21,7 @@ import { } from '@iptvnator/shared/interfaces'; import { AppConfig } from '../../environments/environment'; import { + createLogger, createPortalDebugRequestContext, logPortalDebugEvent, } from '@iptvnator/portal/shared/util'; @@ -54,7 +54,7 @@ export class ElectronService extends DataService { private readonly store = inject(Store); private readonly settingsStore = inject(SettingsStore); private readonly translateService = inject(TranslateService); - private readonly debugLog = createDevLogger('ElectronService'); + private readonly logger = createLogger('ElectronService'); private readonly silentXtreamActions = new Set([ XtreamCodeActions.GetAccountInfo, XtreamCodeActions.GetLiveCategories, @@ -67,7 +67,6 @@ export class ElectronService extends DataService { constructor() { super(); - this.debugLog('Electron service initialized...'); this.setupPlayerErrorListener(); this.setupPortalDebugListener(); } @@ -81,7 +80,10 @@ export class ElectronService extends DataService { error: string; originalError: string; }) => { - console.error(`${data.player} Error:`, data.originalError); + this.logger.error( + `${data.player} Error:`, + data.originalError + ); this.snackBar.open( `${data.player} Error: ${data.error}`, 'Close', @@ -182,7 +184,7 @@ export class ElectronService extends DataService { duration: 5000, } ); - console.error('MPV launch error:', error); + this.logger.error('MPV launch error:', error); throw error; } } @@ -211,7 +213,7 @@ export class ElectronService extends DataService { duration: 5000, } ); - console.error('VLC launch error:', error); + this.logger.error('VLC launch error:', error); throw error; } } @@ -237,7 +239,7 @@ export class ElectronService extends DataService { return playlists as T; } - this.debugLog('Unknown IPC event type:', type); + this.logger.debug('Unknown IPC event type:', type); return undefined as T; } @@ -265,7 +267,7 @@ export class ElectronService extends DataService { return response; } catch (err: unknown) { const errorInfo = this.getErrorDetails(err); - console.error('Stalker request error:', err); + this.logger.error('Stalker request error:', err); this.snackBar.open( `Error: ${errorInfo?.message ?? ' Not found'}, status: ${errorInfo?.status ?? 404}`, 'Close', @@ -362,7 +364,7 @@ export class ElectronService extends DataService { data.title ); } else { - console.error( + this.logger.error( 'Either url or filePath must be provided, but not both.' ); return; @@ -387,7 +389,7 @@ export class ElectronService extends DataService { { duration: 2000 } ); } catch (error: unknown) { - console.error('Playlist refresh error:', error); + this.logger.error('Playlist refresh error:', error); if ( data.url && this.handlePlaylistSecurityError(error, () => { @@ -590,12 +592,12 @@ export class ElectronService extends DataService { // Log error to console if (isSilentAction) { - this.debugLog( + this.logger.debug( `Background Xtream action failed (${action ?? 'unknown'}):`, normalizedMessage ); } else { - console.error('Xtream request error:', normalizedMessage); + this.logger.error('Xtream request error:', normalizedMessage); } // Only show snackbar for user-triggered Xtream requests diff --git a/apps/web/src/app/services/pwa.service.ts b/apps/web/src/app/services/pwa.service.ts index e4f05f161..3204b3262 100644 --- a/apps/web/src/app/services/pwa.service.ts +++ b/apps/web/src/app/services/pwa.service.ts @@ -15,7 +15,6 @@ import { } from 'rxjs'; import { DataService } from '@iptvnator/services'; import { - createDevLogger, ERROR, Playlist, PLAYLIST_PARSE_BY_URL, @@ -32,6 +31,7 @@ import { createPortalDebugSuccessEvent, logPortalDebugEvent, logPortalDebugRequest, + createLogger, } from '@iptvnator/portal/shared/util'; import { getRuntimeBackendUrl } from './runtime-config'; @@ -71,7 +71,7 @@ export class PwaService extends DataService { private readonly store = inject(Store); private readonly swUpdate = inject(SwUpdate); private readonly translateService = inject(TranslateService); - private readonly debugLog = createDevLogger('PwaService'); + private readonly logger = createLogger('PwaService'); private readonly providerTargetIds = new Map>(); private readonly silentXtreamActions = new Set([ XtreamCodeActions.GetAccountInfo, @@ -88,7 +88,6 @@ export class PwaService extends DataService { constructor() { super(); - this.debugLog('PWA service initialized...'); } /** Uses service worker mechanism to check for available application updates */ @@ -358,7 +357,7 @@ export class PwaService extends DataService { ); if (isSilentAction) { - this.debugLog( + this.logger.debug( `Background Xtream action failed (${action ?? 'unknown'}):`, normalizedMessage ); @@ -398,7 +397,7 @@ export class PwaService extends DataService { // Log error to console if (isSilentAction) { - this.debugLog( + this.logger.debug( `Background Xtream action failed (${action ?? 'unknown'}):`, normalizedMessage ); @@ -409,7 +408,7 @@ export class PwaService extends DataService { }; } - console.error('Xtream request error:', normalizedMessage); + this.logger.error('Xtream request error:', normalizedMessage); this.snackBar.open( `Xtream request failed: ${normalizedMessage}`, 'Close', @@ -533,7 +532,7 @@ export class PwaService extends DataService { } catch (err: unknown) { const errorInfo = this.getErrorDetails(err); logPortalDebugEvent(createPortalDebugErrorEvent(context, err)); - console.error('Stalker request error:', err); + this.logger.error('Stalker request error:', err); this.snackBar.open( `Error: ${errorInfo?.message ?? ' Not found'}, status: ${errorInfo?.status ?? 404}`, diff --git a/docs/architecture/electron-security.md b/docs/architecture/electron-security.md index 47332c781..0f33d310f 100644 --- a/docs/architecture/electron-security.md +++ b/docs/architecture/electron-security.md @@ -188,6 +188,26 @@ playlist or EPG source host. The `IPTVNATOR_ALLOW_INSECURE_TLS=1` escape hatch is only for explicitly trusted providers with invalid or self-signed certificates when the host-scoped UI path is not available. +## Sensitive Diagnostic Logging + +Settings, portal requests/responses, IPC trace payloads, and remote-request +errors can contain provider credentials. Code at those boundaries must pass +structured values through `redactSensitiveData` from +`@iptvnator/shared/logging`, or through the portal `createLogger`/portal-debug +helpers that apply it. Do not send a raw settings object, request params, +response, or `Error` directly to `console.*`. + +The redactor preserves non-sensitive diagnostic fields while replacing +credential fields case-insensitively, including usernames, passwords, tokens, +API keys, authorization/cookie headers, and MAC addresses. It also sanitizes +URL query parameters, serialized JSON, nested query values, errors, arrays, +and cyclic objects without mutating the original value. Depth, collection, +object-key, and string limits keep opt-in debug traces bounded. + +When adding a new logging boundary, extend the closest regression test with a +synthetic secret and assert that the exact value is absent from captured log +output. Never use a real provider credential to validate logging. + ## Filesystem Capabilities Renderer IPC payloads are not filesystem authorization. diff --git a/libs/portal/shared/util/src/lib/logger.spec.ts b/libs/portal/shared/util/src/lib/logger.spec.ts index 1691cae71..393f5f27e 100644 --- a/libs/portal/shared/util/src/lib/logger.spec.ts +++ b/libs/portal/shared/util/src/lib/logger.spec.ts @@ -14,7 +14,9 @@ describe('portal debug logger', () => { beforeEach(() => { globalWithNgDevMode.ngDevMode = true; - jest.spyOn(console, 'groupCollapsed').mockImplementation(() => undefined); + jest.spyOn(console, 'groupCollapsed').mockImplementation( + () => undefined + ); jest.spyOn(console, 'groupEnd').mockImplementation(() => undefined); jest.spyOn(console, 'log').mockImplementation(() => undefined); jest.spyOn(console, 'error').mockImplementation(() => undefined); @@ -36,7 +38,9 @@ describe('portal debug logger', () => { }); logPortalDebugRequest(context); - logPortalDebugEvent(createPortalDebugSuccessEvent(context, { ok: true })); + logPortalDebugEvent( + createPortalDebugSuccessEvent(context, { ok: true }) + ); expect(console.groupCollapsed).not.toHaveBeenCalled(); expect(console.log).not.toHaveBeenCalled(); @@ -85,4 +89,43 @@ describe('portal debug logger', () => { nowSpy.mockRestore(); }); + + it('redacts portal request and response credentials before logging', () => { + const secrets = { + username: 'portal-user-secret', + password: 'portal-password-secret', + token: 'portal-token-secret', + authorization: 'portal-authorization-secret', + mac: 'portal-mac-secret', + }; + const context = createPortalDebugRequestContext({ + provider: 'stalker', + operation: 'get_profile', + transport: 'pwa-http', + request: { + params: secrets, + url: `https://example.com/portal?token=${secrets.token}&action=get_profile`, + diagnosticId: 'request-42', + }, + }); + + logPortalDebugRequest(context); + logPortalDebugEvent( + createPortalDebugSuccessEvent(context, { + user_info: secrets, + status: 'ok', + }) + ); + + const output = JSON.stringify([ + ...(console.log as jest.Mock).mock.calls, + ...(console.error as jest.Mock).mock.calls, + ]); + for (const secret of Object.values(secrets)) { + expect(output).not.toContain(secret); + } + expect(output).toContain('request-42'); + expect(output).toContain('get_profile'); + expect(output).toContain('status'); + }); }); diff --git a/libs/portal/shared/util/src/lib/logger.ts b/libs/portal/shared/util/src/lib/logger.ts index 439b449e3..16578937a 100644 --- a/libs/portal/shared/util/src/lib/logger.ts +++ b/libs/portal/shared/util/src/lib/logger.ts @@ -3,6 +3,7 @@ import { PortalDebugProvider, PortalDebugTransport, } from '@iptvnator/shared/interfaces'; +import { redactSensitiveData } from '@iptvnator/shared/logging'; export interface Logger { debug: (...args: unknown[]) => void; @@ -26,19 +27,31 @@ export function createLogger(scope: string): Logger { return { debug: (...args: unknown[]) => { if (debugEnabled) { - console.debug(prefix, ...args); + console.debug( + prefix, + ...args.map((arg) => redactSensitiveData(arg)) + ); } }, info: (...args: unknown[]) => { if (debugEnabled) { - console.info(prefix, ...args); + console.info( + prefix, + ...args.map((arg) => redactSensitiveData(arg)) + ); } }, warn: (...args: unknown[]) => { - console.warn(prefix, ...args); + console.warn( + prefix, + ...args.map((arg) => redactSensitiveData(arg)) + ); }, error: (...args: unknown[]) => { - console.error(prefix, ...args); + console.error( + prefix, + ...args.map((arg) => redactSensitiveData(arg)) + ); }, }; } @@ -78,7 +91,7 @@ function logPortalDebugSection( method: 'log' | 'error' = 'log' ): void { if (typeof console[method] === 'function') { - console[method](label, value); + console[method](label, redactSensitiveData(value)); } } diff --git a/libs/portal/stalker/data-access/src/lib/stalker-session.service.spec.ts b/libs/portal/stalker/data-access/src/lib/stalker-session.service.spec.ts index 51810adb6..8ed39a0de 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-session.service.spec.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-session.service.spec.ts @@ -24,7 +24,8 @@ type GetProfileWithIdentity = ( ) => Promise; describe('StalkerSessionService identity payloads', () => { - const portalUrl = 'https://portal.example.com/stalker_portal/server/load.php'; + const portalUrl = + 'https://portal.example.com/stalker_portal/server/load.php'; const macAddress = '00:1A:79:AA:BB:CC'; let service: StalkerSessionService; @@ -56,6 +57,10 @@ describe('StalkerSessionService identity payloads', () => { service = TestBed.inject(StalkerSessionService); }); + afterEach(() => { + jest.restoreAllMocks(); + }); + it('omits SN, device IDs, and signatures from get_profile when identity is blank', async () => { const getProfile = service.getProfile as unknown as GetProfileWithIdentity; @@ -180,6 +185,34 @@ describe('StalkerSessionService identity payloads', () => { expect(handshakePayload.serialNumber).toBe('CUSTOMSN123'); }); + it('does not log credentials from portal request errors', async () => { + const token = 'stalker-error-token-secret'; + const consoleError = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined); + dataService.sendIpcEvent.mockRejectedValue( + new Error( + `Request failed: https://portal.example/api?token=${token}&action=get_profile` + ) + ); + + await expect( + service.getProfile(portalUrl, macAddress, token, {}, 'random-1') + ).rejects.toThrow('Request failed'); + + const output = consoleError.mock.calls + .flatMap((call) => + call.map((value) => + value instanceof Error + ? `${value.message}\n${value.stack ?? ''}` + : JSON.stringify(value) + ) + ) + .join('\n'); + expect(output).not.toContain(token); + expect(output).toContain('get_profile'); + }); + function lastStalkerPayload(): { params: Record; serialNumber?: string; diff --git a/libs/portal/stalker/data-access/src/lib/stalker-session.service.ts b/libs/portal/stalker/data-access/src/lib/stalker-session.service.ts index e32498aca..91f1ea741 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-session.service.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-session.service.ts @@ -1,10 +1,7 @@ import { Injectable, inject } from '@angular/core'; -import { - createDevLogger, - Playlist, - STALKER_REQUEST, -} from '@iptvnator/shared/interfaces'; +import { Playlist, STALKER_REQUEST } from '@iptvnator/shared/interfaces'; import { DataService } from '@iptvnator/services'; +import { createLogger } from '@iptvnator/portal/shared/util'; import { getStalkerPortalIdentityFromPlaylist, LEGACY_DEFAULT_STALKER_SERIAL, @@ -91,7 +88,7 @@ interface StalkerAuthConfirmationResponse { }) export class StalkerSessionService { private dataService = inject(DataService); - private readonly debugLog = createDevLogger('StalkerSession'); + private readonly logger = createLogger('StalkerSession'); // In-memory token cache for current session (keyed by playlist ID) private tokenCache = new Map(); @@ -234,7 +231,7 @@ export class StalkerSessionService { ); } catch (error) { // Keep failures non-fatal; next interval can recover after token refresh. - console.warn('[StalkerSession] Watchdog ping failed:', error); + this.logger.warn('Watchdog ping failed:', error); } finally { this.watchdogInFlight.delete(playlistId); } @@ -281,10 +278,10 @@ export class StalkerSessionService { }; } - console.error('[StalkerSession] No token in response'); + this.logger.error('No token in response'); throw new Error('Handshake failed: No token received'); } catch (error) { - console.error('[StalkerSession] Handshake error:', error); + this.logger.error('Handshake error:', error); throw error; } } @@ -364,7 +361,7 @@ export class StalkerSessionService { return response; } catch (error) { - console.error('[StalkerSession] Get profile error:', error); + this.logger.error('Get profile error:', error); throw error; } } @@ -404,7 +401,7 @@ export class StalkerSessionService { return false; } catch (error) { - console.error('[StalkerSession] do_auth error:', error); + this.logger.error('do_auth error:', error); throw error; } } @@ -447,7 +444,7 @@ export class StalkerSessionService { profileResponse.js.msg || profileResponse.js.block_msg || 'Unknown profile error'; - console.error('[StalkerSession] Profile error:', errorMsg); + this.logger.error('Profile error:', errorMsg); throw new Error(`Profile error: ${errorMsg}`); } @@ -457,7 +454,7 @@ export class StalkerSessionService { }; } catch (error) { // Profile fetch failed - this is a real error, propagate it - console.error('[StalkerSession] Profile fetch failed:', error); + this.logger.error('Profile fetch failed:', error); throw error; } } @@ -487,14 +484,14 @@ export class StalkerSessionService { // This prevents race conditions when multiple resources request a token simultaneously const pendingPromise = this.pendingAuth.get(playlist._id); if (pendingPromise) { - this.debugLog('Waiting for pending authentication...'); + this.logger.debug('Waiting for pending authentication...'); return pendingPromise; } // No cached token - need to do full authentication (handshake + get_profile) // Don't trust stored tokens as they may be from a different session if (!playlist.portalUrl || !playlist.macAddress) { - console.error('[StalkerSession] Missing portal URL or MAC address'); + this.logger.error('Missing portal URL or MAC address'); throw new Error('Portal URL and MAC address are required'); } const portalUrl = playlist.portalUrl; diff --git a/libs/portal/xtream/data-access/src/lib/services/xtream-api.service.ts b/libs/portal/xtream/data-access/src/lib/services/xtream-api.service.ts index ec5a7db01..d85aa843b 100644 --- a/libs/portal/xtream/data-access/src/lib/services/xtream-api.service.ts +++ b/libs/portal/xtream/data-access/src/lib/services/xtream-api.service.ts @@ -1,5 +1,6 @@ import { inject, Injectable } from '@angular/core'; import { DataService } from '@iptvnator/services'; +import { createLogger } from '@iptvnator/portal/shared/util'; import { EpgItem, XtreamCategory, @@ -78,6 +79,7 @@ interface EpgResponse { @Injectable({ providedIn: 'root' }) export class XtreamApiService { private readonly dataService = inject(DataService); + private readonly logger = createLogger('XtreamApiService'); async cancelSession(sessionId: string): Promise { if ( @@ -91,7 +93,7 @@ export class XtreamApiService { const result = await window.electron.xtreamCancelSession(sessionId); return result.success; } catch (error) { - console.error('Failed to cancel Xtream session:', error); + this.logger.error('Failed to cancel Xtream session:', error); return false; } } diff --git a/libs/shared/logging/jest.config.ts b/libs/shared/logging/jest.config.ts new file mode 100644 index 000000000..c2ec796ae --- /dev/null +++ b/libs/shared/logging/jest.config.ts @@ -0,0 +1,13 @@ +export default { + displayName: 'shared-logging', + preset: '../../../jest.preset.js', + testEnvironment: 'node', + transform: { + '^.+\\.[tj]s$': [ + 'ts-jest', + { tsconfig: '/tsconfig.spec.json' }, + ], + }, + moduleFileExtensions: ['ts', 'js'], + coverageDirectory: '../../../coverage/libs/shared/logging', +}; diff --git a/libs/shared/logging/project.json b/libs/shared/logging/project.json new file mode 100644 index 000000000..031ec83b4 --- /dev/null +++ b/libs/shared/logging/project.json @@ -0,0 +1,20 @@ +{ + "name": "shared-logging", + "$schema": "../../../node_modules/nx/schemas/project-schema.json", + "sourceRoot": "libs/shared/logging/src", + "projectType": "library", + "tags": ["scope:shared", "domain:shared-runtime", "type:util"], + "targets": { + "test": { + "executor": "@nx/jest:jest", + "outputs": ["{workspaceRoot}/coverage/{projectRoot}"], + "options": { + "jestConfig": "libs/shared/logging/jest.config.ts", + "tsConfig": "libs/shared/logging/tsconfig.spec.json" + } + }, + "lint": { + "executor": "@nx/eslint:lint" + } + } +} diff --git a/libs/shared/logging/src/index.ts b/libs/shared/logging/src/index.ts new file mode 100644 index 000000000..1a61e0b26 --- /dev/null +++ b/libs/shared/logging/src/index.ts @@ -0,0 +1,5 @@ +export { + REDACTED_VALUE, + redactSensitiveData, +} from './lib/redact-sensitive-data'; +export type { RedactionOptions } from './lib/redact-sensitive-data'; diff --git a/libs/shared/logging/src/lib/redact-sensitive-data.spec.ts b/libs/shared/logging/src/lib/redact-sensitive-data.spec.ts new file mode 100644 index 000000000..75d6efcb6 --- /dev/null +++ b/libs/shared/logging/src/lib/redact-sensitive-data.spec.ts @@ -0,0 +1,357 @@ +import { REDACTED_VALUE, redactSensitiveData } from './redact-sensitive-data'; + +const TEST_SECRETS = [ + 'settings-api-key-secret', + 'nested-user-secret', + 'nested-password-secret', + 'nested-token-secret', + 'nested-auth-secret', + 'nested-mac-secret', + 'query-password-secret', + 'query-token-secret', +]; + +function serialized(value: unknown): string { + return JSON.stringify(value); +} + +describe('redactSensitiveData', () => { + it('recursively redacts credentials while retaining diagnostic fields', () => { + const input = { + operation: 'get_profile', + settings: { tmdb: { apiKey: TEST_SECRETS[0] } }, + params: { + username: TEST_SECRETS[1], + PASSWORD: TEST_SECRETS[2], + access_token: TEST_SECRETS[3], + headers: { Authorization: `Bearer ${TEST_SECRETS[4]}` }, + macAddress: TEST_SECRETS[5], + }, + }; + + const result = redactSensitiveData(input); + const output = serialized(result); + + for (const secret of TEST_SECRETS) { + expect(output).not.toContain(secret); + } + expect(result).toEqual({ + operation: 'get_profile', + settings: { tmdb: { apiKey: REDACTED_VALUE } }, + params: { + username: REDACTED_VALUE, + PASSWORD: REDACTED_VALUE, + access_token: REDACTED_VALUE, + headers: { Authorization: REDACTED_VALUE }, + macAddress: REDACTED_VALUE, + }, + }); + }); + + it('redacts Stalker identity credentials while retaining request diagnostics', () => { + const identitySecrets = { + sn: 'stalker-sn-secret', + serialNumber: 'stalker-serial-number-secret', + device_id: 'stalker-device-id-secret', + deviceId1: 'stalker-device-id1-secret', + device_id2: 'stalker-device-id2-secret', + signature: 'stalker-signature-secret', + signature1: 'stalker-signature1-secret', + signature2: 'stalker-signature2-secret', + stalkerSerialNumber: 'playlist-stalker-serial-number-secret', + stalkerDeviceId1: 'playlist-stalker-device-id1-secret', + stalkerDeviceId2: 'playlist-stalker-device-id2-secret', + stalkerSignature1: 'playlist-stalker-signature1-secret', + stalkerSignature2: 'playlist-stalker-signature2-secret', + prehash: 'stalker-prehash-secret', + }; + + const result = redactSensitiveData({ + action: 'get_profile', + requestId: 'stalker-request-id', + params: identitySecrets, + }); + const output = serialized(result); + + for (const secret of Object.values(identitySecrets)) { + expect(output).not.toContain(secret); + } + expect(output).toContain('get_profile'); + expect(output).toContain('stalker-request-id'); + }); + + it('redacts credentials embedded in URL, URLSearchParams, errors, and serialized strings', () => { + const url = new URL( + `https://user:pass@example.com/live?password=${TEST_SECRETS[6]}&token=${TEST_SECRETS[7]}&action=get_live_streams` + ); + const params = new URLSearchParams({ + authorization: TEST_SECRETS[4], + category: 'news', + }); + const error = new Error( + `Request failed: https://example.com/api?token=${TEST_SECRETS[3]}&action=profile` + ); + const json = JSON.stringify({ + refreshToken: TEST_SECRETS[3], + status: 401, + }); + + const output = serialized( + redactSensitiveData({ url, params, error, json }) + ); + + for (const secret of TEST_SECRETS) { + expect(output).not.toContain(secret); + } + expect(output).toContain('action=get_live_streams'); + expect(output).toContain('category=news'); + expect(output).toContain('status'); + expect(output).toContain('401'); + }); + + it('redacts credentials in non-HTTP stream URL strings', () => { + const username = 'rtsp-user-secret'; + const password = 'rtsp-password-secret'; + const token = 'rtmp-token-secret'; + + const output = serialized( + redactSensitiveData([ + `rtsp://${username}:${password}@stream.example/live?token=${token}&channel=news`, + `Playback failed: rtmp://stream.example/live?password=${password}&channel=sports`, + ]) + ); + + expect(output).not.toContain(username); + expect(output).not.toContain(password); + expect(output).not.toContain(token); + expect(output).toContain('channel=news'); + expect(output).toContain('channel=sports'); + }); + + it('redacts a credential from a single-parameter string', () => { + const password = 'single-param-password-secret'; + const token = 'single-param-token-secret'; + + const output = serialized( + redactSensitiveData([ + `password=${password}`, + `token=${token}`, + 'operation=get_profile', + ]) + ); + + expect(output).not.toContain(password); + expect(output).not.toContain(token); + expect(output).toContain('get_profile'); + }); + + it('redacts credentials embedded in diagnostic text', () => { + const token = 'diagnostic-token-secret'; + const authorization = 'diagnostic-authorization-secret'; + const authorizationAssignment = + 'diagnostic-authorization-assignment-secret'; + const stalkerDeviceId = 'diagnostic-stalker-device-id-secret'; + const diagnostic = [ + `Request failed: token=${token}&action=get_profile`, + `Upstream response Authorization: Bearer ${authorization}; status=401`, + `Retrying with authorization=Bearer ${authorizationAssignment}, attempt=2`, + `Identity rejected: stalkerDeviceId1: ${stalkerDeviceId}; action=handshake`, + ]; + + const output = serialized(redactSensitiveData(diagnostic)); + + expect(output).not.toContain(token); + expect(output).not.toContain(authorization); + expect(output).not.toContain(authorizationAssignment); + expect(output).not.toContain(stalkerDeviceId); + expect(output).toContain('get_profile'); + expect(output).toContain('status=401'); + expect(output).toContain('attempt=2'); + expect(output).toContain('action=handshake'); + }); + + it('does not repeat a redacted Error message secret in its stack', () => { + const secret = 'error-stack-password-secret'; + const error = new Error(`password=${secret}&operation=get_profile`); + + const output = serialized(redactSensitiveData(error)); + + expect(output).not.toContain(secret); + expect(output).toContain(encodeURIComponent(REDACTED_VALUE)); + expect(output).toContain('get_profile'); + }); + + it('redacts credentials nested inside non-sensitive query values', () => { + const nestedUrl = `https://identity.example/callback?token=${TEST_SECRETS[3]}&step=authorize`; + const url = new URL('https://example.com/portal'); + url.searchParams.set('redirect', nestedUrl); + url.searchParams.set( + 'payload', + JSON.stringify({ password: TEST_SECRETS[2], action: 'profile' }) + ); + + const output = serialized(redactSensitiveData(url)); + + expect(output).not.toContain(TEST_SECRETS[2]); + expect(output).not.toContain(TEST_SECRETS[3]); + expect(output).toContain('authorize'); + expect(output).toContain('profile'); + }); + + it('redacts credentials from URL fragments while retaining diagnostics', () => { + const token = 'fragment-token-secret'; + const url = new URL( + `https://example.com/callback#access_token=${token}&state=diagnostic` + ); + + const output = serialized(redactSensitiveData(url)); + + expect(output).not.toContain(token); + expect(output).toContain('state=diagnostic'); + }); + + it('redacts credentials from Map keys while retaining values', () => { + const username = 'map-key-user-secret'; + const password = 'map-key-password-secret'; + const token = 'map-key-token-secret'; + const requests = new Map([ + [ + `https://example.com/live/${username}/${password}/101.ts?token=${token}`, + { status: 200 }, + ], + ]); + + const output = serialized(redactSensitiveData(requests)); + + expect(output).not.toContain(username); + expect(output).not.toContain(password); + expect(output).not.toContain(token); + expect(output).toContain('101.ts'); + expect(output).toContain('"status":200'); + }); + + it('redacts Map values selected by sensitive keys', () => { + const authorization = 'map-authorization-secret'; + const password = 'map-password-secret'; + const headers = new Map([ + ['Authorization', `Bearer ${authorization}`], + ['password', password], + ['requestId', 'diagnostic-request-id'], + ]); + + const result = redactSensitiveData(headers); + const output = serialized(result); + + expect(output).not.toContain(authorization); + expect(output).not.toContain(password); + expect(output).toContain('diagnostic-request-id'); + expect(result).toEqual({ + Authorization: REDACTED_VALUE, + password: REDACTED_VALUE, + requestId: 'diagnostic-request-id', + }); + }); + + it('redacts Xtream credentials in playback URL paths', () => { + const username = 'xtream-path-user-secret'; + const password = 'xtream-path-password-secret'; + const urls = [ + new URL( + `https://example.com/base/live/${username}/${password}/101.ts` + ), + new URL( + `https://example.com/movie/${username}/${password}/202.mkv` + ), + new URL( + `https://example.com/series/${username}/${password}/303.mp4` + ), + new URL( + `https://example.com/timeshift/${username}/${password}/60/2026-07-18:12-00/404.ts` + ), + ]; + const embedded = `Playback failed for https://example.com/live/${username}/${password}/505.m3u8`; + const ordinaryLiveUrl = 'https://example.com/live/channel.m3u8'; + + const output = serialized( + redactSensitiveData({ urls, embedded, ordinaryLiveUrl }) + ); + + expect(output).not.toContain(username); + expect(output).not.toContain(password); + expect(output).toContain('101.ts'); + expect(output).toContain('202.mkv'); + expect(output).toContain('303.mp4'); + expect(output).toContain('404.ts'); + expect(output).toContain('505.m3u8'); + expect(output).toContain(ordinaryLiveUrl); + }); + + it('redacts Xtream path credentials when no resource segment follows', () => { + const username = 'terminal-xtream-user-secret'; + const password = 'terminal-xtream-password-secret'; + + const output = serialized( + redactSensitiveData( + new URL(`https://example.com/live/${username}/${password}`) + ) + ); + + expect(output).not.toContain(username); + expect(output).not.toContain(password); + }); + + it('does not mutate input and safely bounds cycles, depth, arrays, objects, and strings', () => { + const input: Record = { + status: 'ok', + password: TEST_SECRETS[2], + items: [1, 2, 3, 4], + long: 'abcdefghij', + nested: { level: { value: 'too deep' } }, + extraA: 'a', + extraB: 'b', + }; + input['self'] = input; + const originalItems = input['items']; + + const result = redactSensitiveData(input, { + maxArrayItems: 2, + maxDepth: 2, + maxObjectKeys: 6, + maxStringLength: 8, + }); + + expect(input['password']).toBe(TEST_SECRETS[2]); + expect(input['items']).toBe(originalItems); + expect(result).not.toBe(input); + expect(() => serialized(result)).not.toThrow(); + expect(serialized(result)).not.toContain(TEST_SECRETS[2]); + expect(serialized(result)).toContain('[Truncated'); + }); + + it('serializes invalid dates without throwing', () => { + const invalidDate = new Date(Number.NaN); + + expect(() => redactSensitiveData(invalidDate)).not.toThrow(); + expect(redactSensitiveData([invalidDate])).toEqual(['[Invalid Date]']); + }); + + it('preserves repeated non-circular references while still redacting them', () => { + const shared = { + operation: 'get_profile', + password: TEST_SECRETS[2], + }; + + const result = redactSensitiveData({ first: shared, second: shared }); + + expect(result).toEqual({ + first: { + operation: 'get_profile', + password: REDACTED_VALUE, + }, + second: { + operation: 'get_profile', + password: REDACTED_VALUE, + }, + }); + }); +}); diff --git a/libs/shared/logging/src/lib/redact-sensitive-data.ts b/libs/shared/logging/src/lib/redact-sensitive-data.ts new file mode 100644 index 000000000..cc42a4db2 --- /dev/null +++ b/libs/shared/logging/src/lib/redact-sensitive-data.ts @@ -0,0 +1,395 @@ +export const REDACTED_VALUE = '[Redacted]'; + +const CIRCULAR_VALUE = '[Circular]'; +const MAX_DEPTH_VALUE = '[MaxDepth]'; +const DEFAULT_MAX_DEPTH = 6; +const DEFAULT_MAX_ARRAY_ITEMS = 50; +const DEFAULT_MAX_OBJECT_KEYS = 50; +const DEFAULT_MAX_STRING_LENGTH = 2_000; + +const SENSITIVE_KEY_NAMES = new Set([ + 'apikey', + 'auth', + 'authorization', + 'cookie', + 'credentials', + 'deviceid', + 'deviceid2', + 'login', + 'mac', + 'macaddress', + 'mpvplayerarguments', + 'passwd', + 'password', + 'pwd', + 'secret', + 'setcookie', + 'signature', + 'signature2', + 'sn', + 'token', + 'username', + 'vlcplayerarguments', +]); + +const SENSITIVE_KEY_SUFFIXES = [ + 'apikey', 'authorization', 'cookie', + 'deviceid', 'deviceid1', 'deviceid2', + 'macaddress', 'passwd', 'password', 'prehash', + 'serialnumber', 'signature', 'signature1', 'signature2', + 'secret', 'token', 'username', +]; + +const XTREAM_CREDENTIAL_PATH_SEGMENTS = new Set([ + 'live', + 'movie', + 'series', + 'timeshift', +]); + +export interface RedactionOptions { + maxDepth?: number; + maxArrayItems?: number; + maxObjectKeys?: number; + maxStringLength?: number; +} + +interface ResolvedRedactionOptions { + maxDepth: number; + maxArrayItems: number; + maxObjectKeys: number; + maxStringLength: number; +} + +function normalizeKey(key: string): string { + return key.toLowerCase().replace(/[^a-z0-9]/g, ''); +} + +function isSensitiveKey(key: string): boolean { + const normalized = normalizeKey(key); + return ( + SENSITIVE_KEY_NAMES.has(normalized) || + SENSITIVE_KEY_SUFFIXES.some((suffix) => normalized.endsWith(suffix)) + ); +} + +function resolveOptions(options: RedactionOptions): ResolvedRedactionOptions { + return { + maxDepth: Math.max(0, options.maxDepth ?? DEFAULT_MAX_DEPTH), + maxArrayItems: Math.max( + 0, + options.maxArrayItems ?? DEFAULT_MAX_ARRAY_ITEMS + ), + maxObjectKeys: Math.max( + 0, + options.maxObjectKeys ?? DEFAULT_MAX_OBJECT_KEYS + ), + maxStringLength: Math.max( + 0, + options.maxStringLength ?? DEFAULT_MAX_STRING_LENGTH + ), + }; +} + +function truncateString(value: string, maxLength: number): string { + if (value.length <= maxLength) { + return value; + } + + const omitted = value.length - maxLength; + return `${value.slice(0, maxLength)}[Truncated ${omitted} chars]`; +} + +function redactSearchParams( + params: URLSearchParams, + sanitizeValue: (value: string) => string +): URLSearchParams { + const redacted = new URLSearchParams(); + + params.forEach((value, key) => { + redacted.append( + key, + isSensitiveKey(key) ? REDACTED_VALUE : sanitizeValue(value) + ); + }); + + return redacted; +} + +function redactUrl( + value: URL, + sanitizeValue: (value: string) => string +): string { + const redacted = new URL(value.toString()); + + if (redacted.username) { + redacted.username = REDACTED_VALUE; + } + if (redacted.password) { + redacted.password = REDACTED_VALUE; + } + + const pathSegments = redacted.pathname.split('/'); + for (let index = 0; index < pathSegments.length - 2; index += 1) { + if (XTREAM_CREDENTIAL_PATH_SEGMENTS.has(pathSegments[index])) { + pathSegments[index + 1] = REDACTED_VALUE; + pathSegments[index + 2] = REDACTED_VALUE; + } + } + redacted.pathname = pathSegments.join('/'); + + const search = redactSearchParams( + redacted.searchParams, + sanitizeValue + ).toString(); + redacted.search = search ? `?${search}` : ''; + + const fragment = redacted.hash.slice(1); + if (looksLikeSearchParams(fragment)) { + const hash = redactSearchParams( + new URLSearchParams(fragment), + sanitizeValue + ).toString(); + redacted.hash = hash ? `#${hash}` : ''; + } + + return redacted.toString(); +} + +function redactUrlStrings( + value: string, + sanitizeValue: (value: string) => string +): string { + return value.replace( + /[a-z][a-z0-9+.-]*:\/\/[^\s"'<>]+/giu, + (candidate) => { + try { + return redactUrl(new URL(candidate), sanitizeValue); + } catch { + return candidate; + } + } + ); +} + +function redactEmbeddedSensitivePairs(value: string): string { + const redactedAssignments = value.replace( + /\b([a-z][a-z0-9_.-]*)(\s*=\s*)((?:Bearer\s+)?[^&\s,;]+)/giu, + (match, key: string, separator: string) => + isSensitiveKey(key) + ? `${key}${separator}${REDACTED_VALUE}` + : match + ); + return redactedAssignments.replace( + /\b([a-z0-9_.-]*(?:api[-_.]?key|auth(?:orization)?|cookie|credentials|device[-_.]?id[12]?|login|mac(?:[-_.]?address)?|passwd|password|prehash|pwd|secret|serial[-_.]?number|set[-_.]?cookie|signature[12]?|sn|token|username))(\s*:\s*)(?:Bearer\s+)?[^;,\r\n]+/giu, + (_match, key: string, separator: string) => + `${key}${separator}${REDACTED_VALUE}` + ); +} + +function looksLikeSearchParams(value: string): boolean { + return /^[^=&\s]+=[^&]*(?:&[^=&\s]+=[^&]*)*$/u.test(value); +} + +export function redactSensitiveData( + value: unknown, + options: RedactionOptions = {} +): unknown { + const resolved = resolveOptions(options); + const seen = new WeakSet(); + const visitString = (input: string, depth: number): string => { + const trimmed = input.trim(); + if (depth >= resolved.maxDepth) { + return MAX_DEPTH_VALUE; + } + if ( + (trimmed.startsWith('{') && trimmed.endsWith('}')) || + (trimmed.startsWith('[') && trimmed.endsWith(']')) + ) { + try { + return truncateString( + JSON.stringify(visit(JSON.parse(trimmed), depth + 1)), + resolved.maxStringLength + ); + } catch { + // Keep processing malformed or non-JSON diagnostic strings. + } + } + if (/^[a-z][a-z0-9+.-]*:\/\//iu.test(trimmed)) { + try { + return truncateString( + redactUrl(new URL(trimmed), (entry) => + visitString(entry, depth + 1) + ), + resolved.maxStringLength + ); + } catch { + // Continue with embedded URL handling for malformed URLs. + } + } + + if (looksLikeSearchParams(trimmed)) { + return truncateString( + redactSearchParams(new URLSearchParams(trimmed), (entry) => + visitString(entry, depth + 1) + ).toString(), + resolved.maxStringLength + ); + } + + const redactedText = redactEmbeddedSensitivePairs(input); + return truncateString( + redactUrlStrings(redactedText, (entry) => + visitString(entry, depth + 1) + ), + resolved.maxStringLength + ); + }; + + const visitObject = ( + input: Record, + depth: number + ): Record => { + const output: Record = {}; + const keys = Object.keys(input); + + for (const key of keys.slice(0, resolved.maxObjectKeys)) { + if (isSensitiveKey(key)) { + output[key] = REDACTED_VALUE; + continue; + } + + try { + output[key] = visit(input[key], depth + 1); + } catch { + output[key] = '[Unserializable]'; + } + } + + if (keys.length > resolved.maxObjectKeys) { + output['__truncatedKeys'] = keys.length - resolved.maxObjectKeys; + } + + return output; + }; + + const visitError = ( + error: Error, + depth: number + ): Record => { + const output = visitObject( + error as Error & Record, + depth + ); + output['name'] = visitString(error.name, depth + 1); + output['message'] = visitString(error.message, depth + 1); + if (error.stack) { + const [, ...stackFrames] = error.stack.split('\n'); + output['stack'] = visitString( + [`${output['name']}: ${output['message']}`, ...stackFrames].join( + '\n' + ), + depth + 1 + ); + } + if ('cause' in error) { + output['cause'] = visit(error.cause, depth + 1); + } + return output; + }; + + const visit = (input: unknown, depth: number): unknown => { + if ( + input == null || + typeof input === 'boolean' || + typeof input === 'number' + ) { + return input; + } + if (typeof input === 'string') { + return visitString(input, depth); + } + if (typeof input === 'bigint') { + return input.toString(); + } + if (typeof input === 'symbol') { + return input.toString(); + } + if (typeof input === 'function') { + return undefined; + } + if (depth >= resolved.maxDepth) { + return MAX_DEPTH_VALUE; + } + + const object = input as object; + if (seen.has(object)) { + return CIRCULAR_VALUE; + } + seen.add(object); + + try { + if (input instanceof URL) { + return redactUrl(input, (entry) => + visitString(entry, depth + 1) + ); + } + if (input instanceof URLSearchParams) { + return redactSearchParams(input, (entry) => + visitString(entry, depth + 1) + ).toString(); + } + if (input instanceof Error) { + return visitError(input, depth); + } + if (input instanceof Date) { + return Number.isNaN(input.getTime()) + ? '[Invalid Date]' + : input.toISOString(); + } + if (Array.isArray(input)) { + const output = input + .slice(0, resolved.maxArrayItems) + .map((entry) => visit(entry, depth + 1)); + if (input.length > resolved.maxArrayItems) { + output.push( + `[Truncated ${ + input.length - resolved.maxArrayItems + } items]` + ); + } + return output; + } + if (input instanceof Map) { + const entries: Record = {}; + const mapEntries = Array.from(input).slice( + 0, + resolved.maxObjectKeys + ); + for (const [key, entry] of mapEntries) { + const stringKey = String(key); + const redactedKey = visitString(stringKey, depth + 1); + entries[redactedKey] = + isSensitiveKey(stringKey) && + !/[/:?=&]/u.test(stringKey) + ? REDACTED_VALUE + : visit(entry, depth + 1); + } + if (input.size > resolved.maxObjectKeys) { + entries['__truncatedKeys'] = + input.size - resolved.maxObjectKeys; + } + return entries; + } + if (input instanceof Set) { + return visit(Array.from(input), depth); + } + + return visitObject(input as Record, depth); + } finally { + seen.delete(object); + } + }; + + return visit(value, 0); +} diff --git a/libs/shared/logging/tsconfig.json b/libs/shared/logging/tsconfig.json new file mode 100644 index 000000000..ab8e5af25 --- /dev/null +++ b/libs/shared/logging/tsconfig.json @@ -0,0 +1,19 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "module": "commonjs", + "forceConsistentCasingInFileNames": true, + "strict": true, + "importHelpers": true, + "noImplicitOverride": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true, + "noPropertyAccessFromIndexSignature": true + }, + "files": [], + "include": [], + "references": [ + { "path": "./tsconfig.lib.json" }, + { "path": "./tsconfig.spec.json" } + ] +} diff --git a/libs/shared/logging/tsconfig.lib.json b/libs/shared/logging/tsconfig.lib.json new file mode 100644 index 000000000..163d90724 --- /dev/null +++ b/libs/shared/logging/tsconfig.lib.json @@ -0,0 +1,10 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../../../dist/out-tsc", + "declaration": true, + "types": ["node"] + }, + "include": ["src/**/*.ts"], + "exclude": ["jest.config.ts", "src/**/*.spec.ts", "src/**/*.test.ts"] +} diff --git a/libs/shared/logging/tsconfig.spec.json b/libs/shared/logging/tsconfig.spec.json new file mode 100644 index 000000000..4b0383fc4 --- /dev/null +++ b/libs/shared/logging/tsconfig.spec.json @@ -0,0 +1,15 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../../../dist/out-tsc", + "module": "commonjs", + "moduleResolution": "node10", + "types": ["jest", "node"] + }, + "include": [ + "jest.config.ts", + "src/**/*.test.ts", + "src/**/*.spec.ts", + "src/**/*.d.ts" + ] +} diff --git a/tools/coverage/coverage-policy.json b/tools/coverage/coverage-policy.json index d29c4c0a7..baadc91f0 100644 --- a/tools/coverage/coverage-policy.json +++ b/tools/coverage/coverage-policy.json @@ -149,6 +149,13 @@ "validationCommand": "pnpm nx test shared-interfaces", "e2eTags": ["@m3u", "@xtream", "@stalker"] }, + { + "name": "shared-logging", + "root": "libs/shared/logging", + "sourceRoot": "libs/shared/logging/src", + "validationCommand": "pnpm nx test shared-logging", + "e2eTags": ["@electron", "@xtream", "@stalker", "@settings"] + }, { "name": "m3u-utils", "root": "libs/shared/m3u-utils", diff --git a/tsconfig.base.json b/tsconfig.base.json index 2e8cc3c8c..05edde3ed 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -80,6 +80,7 @@ "libs/shared/m3u-utils/src/index.ts" ], "@iptvnator/shared/testing": ["libs/shared/testing/src/index.ts"], + "@iptvnator/shared/logging": ["libs/shared/logging/src/index.ts"], "@iptvnator/services": ["libs/services/src/index.ts"], "@iptvnator/shared/interfaces": [ "libs/shared/interfaces/src/index.ts"