From bb474f0f8c6a5e4c13a951ad7d368bd362b18cbf Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 9 Aug 2026 13:59:16 +0200 Subject: [PATCH] fix(stalker): reject stale repairs before discovery --- CLAUDE.md | 2 +- docs/architecture/stalker-portal.md | 8 ++- .../lib/stalker-portal-repair.service.spec.ts | 72 +++++++++++-------- .../src/lib/stalker-portal-repair.service.ts | 20 ++++-- 4 files changed, 67 insertions(+), 35 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c063d4caa..df7d0b1de 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1263,7 +1263,7 @@ engine` (restart required) or - Portal mode (full vs. simple) follows OBSERVED behavior, never a URL substring. The single predicate is `isFullStalkerPortalPlaylist()` / `isFullStalkerPortalUrl()` in `@iptvnator/shared/interfaces` (`stalker-portal-mode.util.ts`): the persisted `Playlist.isFullStalkerPortal` flag is authoritative and the URL shape is a fallback for legacy rows only. Three diverging copies of this rule used to exist and shipped broken configurations (#850/#686/#755) — never re-implement it. A token-enforcing `portal.php` panel is a full portal; a `server/load.php` endpoint that answers without a token is a simple one. - Import requires an explicit HTTP(S) scheme but accepts a bare host, `/c`, or a concrete `.php` address. It probes candidates in order (a pasted `.php` endpoint first, then `/portal.php` → `/server/load.php` → `/stalker_portal/server/load.php`) and classifies each by behavior — a token-less `itv/get_genres` returning data proves a token-free panel; the plain-text auth failure proves a full portal, confirmed by a real handshake + `get_profile`. `StalkerPortalDiscoveryService` (`libs/portal/stalker/data-access`) persists and displays the proven endpoint and mode. An unreachable panel-style import remains allowed with a warning; a bare host falls back to `/portal.php`, while canonical-shaped unreachable addresses still abort. - The playlist-info Edit dialog loads the complete persisted Stalker row before enabling the form, because Electron's startup metadata projection omits payload-only serial/device/signature/mode fields; a summarized row must never render and then persist an empty portal identity. It preserves an unchanged connection byte-for-byte and skips discovery. Changing URL, MAC, credentials, serial, device IDs or signatures blocks duplicate saves, disables dialog closure for the validation window, and runs the existing discovery service through the app-provided `STALKER_PLAYLIST_CONNECTION_EDITOR` token, keeping Stalker data-access out of `playlist-shared-ui`. Before discovery it reserves the playlist ID; a second Edit cannot replace that owner. The reservation blocks every new authentication (including fingerprint-equivalent URL edits) and repair, drains existing work, and rechecks ownership after every asynchronous drain/rebase; failure releases it without changing the saved or runtime connection. If navigation or another owner closes/destroys the dialog while discovery is in flight, the UI discards a later successful result through `discardResolvedConnection()` and releases both reservations without persisting it. Success uses one awaited write to atomically replace endpoint, mode, normalized identity and session metadata, then feeds its complete merged row into the state-only NgRx update and active `StalkerStore`/session/watchdog replacement before another same-route request can use the old connection. This preserves playback headers and other metadata absent from the form. Runtime session authority may rebase a changed fingerprint only when the persisted row proves that it owns the same playlist ID, so delete/restore and backup merge remain usable. The transient `PlaylistMetaUpdate.stalkerSessionPatch` preserves on absence, clears on `null`, and fully replaces from an object before storage; it is projected onto existing flat playlist fields and never changes the DB or backup shape. -- `executeStalkerRequest()` (`stores/utils/stalker-request.utils.ts`) is the choke point for catalog, content and playback requests: mode routing, the in-session repair override, and retry-once all live there. Four callers are deliberately outside it because they run below or before the thing it routes on — `StalkerAuthApi` (handshake/`get_profile`/`do_auth`, which the full-portal branch is built from; routing them back would recurse), `StalkerPortalDiscoveryService` (probes precede the mode they determine), `StalkerAccountInfoService.fetchViaProfile()`, and `StreamResolverService` for a collection item with no playlist row. They are exempt from the routing, not from the repair it hooks, but only `fetchViaProfile()` wires `StalkerPortalRepairService` itself: discovery is what repair _drives_, the row-less resolver branch has no playlist to repair, and the auth layer needs nothing — a terminal handshake failure propagates out of the full-portal branch into whichever `executeStalkerRequest()` call triggered the authentication, which is why terminal handshake failures are a repair trigger. Anything new that is not auth or discovery belongs on `executeStalkerRequest()`. Existing playlists are repaired LAZILY (`StalkerPortalRepairService`) — only after a request fails with a shape a wrong endpoint/mode produces, at most once per source configuration per playlist per session, persisted through the atomic `PlaylistsService.transformPlaylistMeta`. There is deliberately **no eager one-shot migration**: a portal that works is never re-probed. +- `executeStalkerRequest()` (`stores/utils/stalker-request.utils.ts`) is the choke point for catalog, content and playback requests: mode routing, the in-session repair override, and retry-once all live there. Four callers are deliberately outside it because they run below or before the thing it routes on — `StalkerAuthApi` (handshake/`get_profile`/`do_auth`, which the full-portal branch is built from; routing them back would recurse), `StalkerPortalDiscoveryService` (probes precede the mode they determine), `StalkerAccountInfoService.fetchViaProfile()`, and `StreamResolverService` for a collection item with no playlist row. They are exempt from the routing, not from the repair it hooks, but only `fetchViaProfile()` wires `StalkerPortalRepairService` itself: discovery is what repair _drives_, the row-less resolver branch has no playlist to repair, and the auth layer needs nothing — a terminal handshake failure propagates out of the full-portal branch into whichever `executeStalkerRequest()` call triggered the authentication, which is why terminal handshake failures are a repair trigger. Anything new that is not auth or discovery belongs on `executeStalkerRequest()`. Existing playlists are repaired LAZILY (`StalkerPortalRepairService`) — only after a request fails with a shape a wrong endpoint/mode produces, at most once per source configuration per playlist per session, persisted through the atomic `PlaylistsService.transformPlaylistMeta`. Before an unrecorded repair calls discovery, it verifies that the persisted row still owns the failing source, so a late pre-Edit request cannot authenticate against the old portal after Edit commits and invalidate the newly saved token. There is deliberately **no eager one-shot migration**: a portal that works is never re-probed. - Explicit Edit advances the repair generation before installing its resolved session. A lazy repair that started earlier is discarded even if it had already verified its row, so it cannot restore an older endpoint, mode or token after Edit. - Both transports build the wire format from the same shared builders in `@iptvnator/shared/interfaces` — `buildStalkerRequestUrl()`, `buildStalkerIdentityRequestContext()`, `encodeStalkerCmdValue()` — so the Electron and PWA legs cannot drift. The mock's `/stalker` mirror shares the identity builder only — it dispatches in-process, so there is no portal URL to build and it mirrors the `JsHttpRequest` default by hand. Never fork any of them. - Simple portals skip the auth lifecycle (no handshake, token or watchdog) but their requests are not stripped to a bare cookie: they still carry everything the shared builder derives from a MAC alone (`mac`/`stb_lang`/`timezone` cookie, MAG `User-Agent`/`X-User-Agent`, `Accept` set). They do NOT carry the serial — `dispatchStalkerRequest()`'s direct branch forwards only `url`/`macAddress`/`params`, so no `SN` header and no serial-derived `__cfduid`, whatever the playlist stores. That gate is on API requests only: `buildStalkerExternalPlaybackHeaders()` reads the serial off the playlist row with no mode check, so the same simple-mode playlist does send `SN`/`__cfduid` with a portal-owned stream. diff --git a/docs/architecture/stalker-portal.md b/docs/architecture/stalker-portal.md index f72e43ae2..b891d3e6e 100644 --- a/docs/architecture/stalker-portal.md +++ b/docs/architecture/stalker-portal.md @@ -251,8 +251,12 @@ gate), and terminal handshake/profile errors — never timeouts or other network failures), at most once per SOURCE CONFIGURATION (endpoint + mode + MAC + identity fingerprint) per playlist per session — an edited configuration may probe when it fails, while every already-probed one stays latched for -the session — and persists only a configuration discovery has proven to -answer, and only when it differs from the failing one. A repaired configuration is applied immediately via an +the session. Before an unrecorded probe makes any portal request, it verifies +that the persisted row still carries the failing source; a request that failed +late after Edit committed is declined before discovery can authenticate against +the old portal and invalidate the edited session. Repair persists only a +configuration discovery has proven to answer, and only when it differs from the +failing one. A repaired configuration is applied immediately via an in-session override inside `executeStalkerRequest()` (stale store snapshots keep working) and persisted through `PlaylistsService.transformPlaylistMeta` — the verification and the patch run in ONE slot of the per-playlist write diff --git a/libs/portal/stalker/data-access/src/lib/stalker-portal-repair.service.spec.ts b/libs/portal/stalker/data-access/src/lib/stalker-portal-repair.service.spec.ts index 976e6e95f..1d8d50af5 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-portal-repair.service.spec.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-portal-repair.service.spec.ts @@ -353,12 +353,14 @@ describe('StalkerPortalRepairService', () => { // without the credentials the probe reports `login-required` and // the source could never be repaired. discover.mockResolvedValue({ status: 'unreachable' }); - - await service.repairPortal({ + const playlistWithCredentials = { ...MISCLASSIFIED, username: 'user', password: 'secret', - }); + } as PlaylistMeta; + persistedRow = playlistWithCredentials as Playlist; + + await service.repairPortal(playlistWithCredentials); expect(discover).toHaveBeenCalledWith( expect.any(String), @@ -377,21 +379,17 @@ describe('StalkerPortalRepairService', () => { expect(discover).toHaveBeenCalledTimes(1); }); - it('re-enters for an edited configuration after awaiting a pending probe', async () => { - // A's probe is in flight when a request from the EDITED config B - // fails: after A settles (and is discarded by the row guard), B - // must get its own probe instead of inheriting A's outcome. + it('re-enters for an edited configuration after declining a pending stale source', async () => { + // A's persisted-row preflight is in flight when a request from + // the EDITED config B fails: after stale A is declined, B must + // get its own probe instead of inheriting A's outcome. const edited = { ...MISCLASSIFIED, portalUrl: 'http://edited.example/portal.php', } as PlaylistMeta; persistedRow = edited as Playlist; - let resolveFirstDiscovery!: (value: unknown) => void; - discover.mockReturnValueOnce( - new Promise((resolve) => (resolveFirstDiscovery = resolve)) - ); - discover.mockResolvedValueOnce({ + discover.mockResolvedValue({ status: 'resolved', portalUrl: 'http://edited.example/server/load.php', isFullStalkerPortal: true, @@ -400,15 +398,9 @@ describe('StalkerPortalRepairService', () => { const oldRepair = service.repairPortal(MISCLASSIFIED); const editedRepair = service.repairPortal(edited); - resolveFirstDiscovery({ - status: 'resolved', - portalUrl: MISCLASSIFIED.portalUrl, - isFullStalkerPortal: true, - }); - expect(await oldRepair).toBeNull(); const repaired = await editedRepair; - expect(discover).toHaveBeenCalledTimes(2); + expect(discover).toHaveBeenCalledTimes(1); expect(repaired?.portalUrl).toBe( 'http://edited.example/server/load.php' ); @@ -531,6 +523,29 @@ describe('StalkerPortalRepairService', () => { await fence; }); + it('does not probe a stale source after explicit Edit has committed', async () => { + const edited = { + ...MISCLASSIFIED, + portalUrl: 'http://edited.example/server/load.php', + isFullStalkerPortal: true, + } as Playlist; + await service.fenceForPlaylistEdit(MISCLASSIFIED._id); + persistedRow = edited; + service.commitPlaylistEdit(MISCLASSIFIED._id); + discover.mockResolvedValue({ + status: 'resolved', + portalUrl: MISCLASSIFIED.portalUrl, + isFullStalkerPortal: true, + token: 'STALE_REPAIR_TOKEN', + }); + + await expect( + service.repairPortal(MISCLASSIFIED) + ).resolves.toBeNull(); + expect(discover).not.toHaveBeenCalled(); + expect(adoptDiscoveredSession).not.toHaveBeenCalled(); + }); + it('discards a repair verified before explicit Edit but completed after it', async () => { const wrongEndpoint = { ...MISCLASSIFIED, @@ -724,9 +739,9 @@ describe('StalkerPortalRepairService', () => { }); it('re-probes a DISCARDED configuration once the row is restored to it', async () => { - // A's probe was discarded because the row moved to B mid-probe; - // after the user restores the row to A, A's next failure must - // probe again instead of staying dead for the session. + // A's probe was discarded by the persisted-row preflight because + // the row had moved to B; after the user restores the row to A, + // A's next failure must probe instead of staying dead. discover.mockResolvedValue({ status: 'resolved', portalUrl: MISCLASSIFIED.portalUrl, @@ -737,12 +752,12 @@ describe('StalkerPortalRepairService', () => { portalUrl: 'http://edited.example/portal.php', } as Playlist; expect(await service.repairPortal(MISCLASSIFIED)).toBeNull(); - expect(discover).toHaveBeenCalledTimes(1); + expect(discover).not.toHaveBeenCalled(); // Row restored to A → the discarded marker yields to a new probe. persistedRow = MISCLASSIFIED as Playlist; const repaired = await service.repairPortal(MISCLASSIFIED); - expect(discover).toHaveBeenCalledTimes(2); + expect(discover).toHaveBeenCalledTimes(1); expect(repaired).toMatchObject({ isFullStalkerPortal: true }); }); @@ -752,7 +767,8 @@ describe('StalkerPortalRepairService', () => { portalUrl: 'http://edited.example/portal.php', } as PlaylistMeta; - // Probe of the OLD config lands after the user edit → discarded. + // The OLD config is declined before discovery because the row + // already carries the user edit. discover.mockResolvedValue({ status: 'resolved', portalUrl: MISCLASSIFIED.portalUrl, @@ -760,12 +776,12 @@ describe('StalkerPortalRepairService', () => { }); persistedRow = edited as Playlist; expect(await service.repairPortal(MISCLASSIFIED)).toBeNull(); - expect(discover).toHaveBeenCalledTimes(1); + expect(discover).not.toHaveBeenCalled(); // A stale snapshot of the already-probed config must NOT loop // the probe… expect(await service.repairPortal(MISCLASSIFIED)).toBeNull(); - expect(discover).toHaveBeenCalledTimes(1); + expect(discover).not.toHaveBeenCalled(); // …but the EDITED configuration failing later must be allowed // to repair without an application restart. @@ -775,7 +791,7 @@ describe('StalkerPortalRepairService', () => { isFullStalkerPortal: true, }); const repaired = await service.repairPortal(edited); - expect(discover).toHaveBeenCalledTimes(2); + expect(discover).toHaveBeenCalledTimes(1); expect(repaired?.portalUrl).toBe( 'http://edited.example/server/load.php' ); diff --git a/libs/portal/stalker/data-access/src/lib/stalker-portal-repair.service.ts b/libs/portal/stalker/data-access/src/lib/stalker-portal-repair.service.ts index b6736461d..24a8b3dfd 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-portal-repair.service.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-portal-repair.service.ts @@ -387,6 +387,18 @@ export class StalkerPortalRepairService implements StalkerPortalRepairApi { playlist: PlaylistMeta, editGeneration: number ): Promise { + // A request sent before an explicit Edit can fail only after that Edit + // commits and releases its fence. Verify the caller still owns the + // persisted source before discovery: confirmation may authenticate + // against the old portal and invalidate the freshly edited session + // even though the atomic transform below would reject its write. + if ( + !(await this.rowCurrentlyMatches(playlist)) || + this.editGenerationChanged(playlist._id, editGeneration) + ) { + return this.discardSupersededRepair(playlist); + } + const outcome = await this.discovery.discover( playlist.portalUrl ?? '', playlist.macAddress ?? '', @@ -541,10 +553,10 @@ export class StalkerPortalRepairService implements StalkerPortalRepairApi { } /** - * Cheap gate for retrying a DISCARDED configuration: reads the current - * row and reports whether it now carries the caller's configuration. - * Only avoids pointless discovery runs — the authoritative check stays - * the atomic transform. + * Cheap preflight for an unrecorded or DISCARDED configuration: reads the + * current row and reports whether it still carries the caller's source. + * This prevents stale discovery from authenticating against an edited + * portal; the authoritative write guard remains the atomic transform. */ private async rowCurrentlyMatches( playlist: PlaylistMeta