From 4bd4bf2fc262a8ef5eb9415e841fb5f0a718da29 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sat, 26 Sep 2026 11:10:48 +0200 Subject: [PATCH] fix(settings): gate the workspace on parental lock init, edit only a readable lock store, guard the deferred reload, validate backup lock entries - The workspace route resolver awaits ParentalLockService.initialize() next to the settings load, so no route or catalog activates before the PIN and lock store are known. - Every lock write re-reads a failed store before building its edit, so a recovered store is edited rather than overwritten. - The deferred hydration reload runs under the publish guard of the request that deferred it. - Backup import validates every parental lock entry and rejects a damaged list instead of erasing the persisted locks on restore. Co-Authored-By: Claude Fable 5.1 --- apps/web/src/app/app.routes.spec.ts | 57 ++++++++++++- apps/web/src/app/app.routes.ts | 18 ++++- docs/architecture/parental-lock.md | 17 +++- .../with-content.feature.reload.spec.ts | 27 +++++++ .../stores/features/with-content.feature.ts | 12 ++- .../parental-lock-lock-store.service.spec.ts | 15 ++++ .../parental-lock-lock-store.service.ts | 18 +++++ .../lib/playlist-backup.service.pins.spec.ts | 6 +- .../playlist-backup.service.roundtrip.spec.ts | 8 +- .../src/lib/playlist-backup.service.ts | 30 ++++++- ...list-backup.service.xtream-restore.spec.ts | 30 ++++--- .../interfaces/src/lib/parental-lock.util.ts | 79 +++++++++++-------- 12 files changed, 257 insertions(+), 60 deletions(-) diff --git a/apps/web/src/app/app.routes.spec.ts b/apps/web/src/app/app.routes.spec.ts index 691f79699..00969c2fb 100644 --- a/apps/web/src/app/app.routes.spec.ts +++ b/apps/web/src/app/app.routes.spec.ts @@ -1,5 +1,5 @@ import { TestBed } from '@angular/core/testing'; -import { SettingsStore } from '@iptvnator/services'; +import { ParentalLockService, SettingsStore } from '@iptvnator/services'; describe('app routes', () => { let workspaceRoute: import('@angular/router').Route | undefined; @@ -67,6 +67,52 @@ describe('app routes', () => { TestBed.resetTestingModule(); }); + it('waits for the parental lock state before activating workspace children', async () => { + let releaseLock!: () => void; + const lockPending = new Promise((resolve) => { + releaseLock = resolve; + }); + const initialize = jest.fn(() => lockPending); + TestBed.configureTestingModule({ + providers: [ + { + provide: SettingsStore, + useValue: { + loadSettings: jest.fn().mockResolvedValue(undefined), + }, + }, + { provide: ParentalLockService, useValue: { initialize } }, + ], + }); + const settingsReadyResolver = + workspaceRoute?.resolve?.['settingsReady']; + if (typeof settingsReadyResolver !== 'function') { + throw new Error('resolver missing'); + } + let resolved = false; + const resolution = TestBed.runInInjectionContext(() => + Promise.resolve( + ( + settingsReadyResolver as import('@angular/router').ResolveFn + )( + {} as import('@angular/router').ActivatedRouteSnapshot, + {} as import('@angular/router').RouterStateSnapshot + ) + ) + ).then(() => { + resolved = true; + }); + await Promise.resolve(); + await Promise.resolve(); + + expect(initialize).toHaveBeenCalled(); + expect(resolved).toBe(false); + + releaseLock(); + await resolution; + expect(resolved).toBe(true); + }); + it('waits for settings before activating workspace children', async () => { let releaseSettings!: () => void; const settingsPending = new Promise((resolve) => { @@ -79,6 +125,12 @@ describe('app routes', () => { provide: SettingsStore, useValue: { loadSettings }, }, + { + provide: ParentalLockService, + useValue: { + initialize: jest.fn().mockResolvedValue(undefined), + }, + }, ], }); const settingsReadyResolver = @@ -120,8 +172,7 @@ describe('app routes', () => { (route) => route.path === 'playlists/:id' ); const loadChildren = playlistRoute?.loadChildren as - | (() => Promise) - | undefined; + (() => Promise) | undefined; const m3uRoutes = (await loadChildren?.()) ?? []; const defaultRoute = m3uRoutes.find((route) => route.path === ''); const favoritesRoute = m3uRoutes.find( diff --git a/apps/web/src/app/app.routes.ts b/apps/web/src/app/app.routes.ts index c02ee01da..123f51372 100644 --- a/apps/web/src/app/app.routes.ts +++ b/apps/web/src/app/app.routes.ts @@ -1,10 +1,24 @@ import { inject } from '@angular/core'; import { Router, Routes } from '@angular/router'; -import { RuntimeCapabilitiesService, SettingsStore } from '@iptvnator/services'; +import { + ParentalLockService, + RuntimeCapabilitiesService, + SettingsStore, +} from '@iptvnator/services'; import { WorkspaceStartupPreferencesService } from '@iptvnator/workspace/shell/util'; import { settingsUnsavedChangesGuard } from './settings/settings-unsaved-changes.guard'; -const settingsReadyResolver = () => inject(SettingsStore).loadSettings(); +// The workspace activates only once settings AND the parental lock state +// (PIN, lock store) are known: before that the lock reads as off and a +// slower IndexedDB read would let the catalogs admit protected rows. +const settingsReadyResolver = async () => { + const settingsStore = inject(SettingsStore); + const parentalLock = inject(ParentalLockService); + await Promise.all([ + settingsStore.loadSettings(), + parentalLock.initialize(), + ]); +}; const workspaceEntryRedirect = async () => inject(WorkspaceStartupPreferencesService).resolveInitialWorkspacePath(); diff --git a/docs/architecture/parental-lock.md b/docs/architecture/parental-lock.md index 80efbe5d0..6cf6698bc 100644 --- a/docs/architecture/parental-lock.md +++ b/docs/architecture/parental-lock.md @@ -129,7 +129,13 @@ retry the read first, and a write is refused while it still fails, since it would be built on an empty in-memory store and wipe the persisted locks). The window before the initial read settles is treated the same way (`ParentalLockLockStore.readable` is false until then): settings can report -the feature as on before the locks are known. The feature switch itself is +the feature as on before the locks are known — and the workspace route's +`settingsReady` resolver awaits `ParentalLockService.initialize()` next to +the settings load, so no route or catalog activates before the PIN and the +lock store are known (in the PWA the settings read can be the slower one). +Every lock write re-reads a failed store BEFORE building its edit, so a +recovered store is edited, never overwritten by an edit built on the empty +fail-closed one. The feature switch itself is persisted through one guarded path (`persistEnabled`): `updateSettings` patches memory before it writes, so a failed write is undone in memory and `setupPin`/`disable` report false (whether to persist is decided from the @@ -178,7 +184,8 @@ on either side. hydration then publishes empty lists in place of the rows it read under the previous lock state (categories included), and the filtered reload of categories and content runs as soon as the hydration settles, on - every path that marks the content initialized. + every path that marks the content initialized, under the publish guard + of the request that deferred it. Both reloads fail closed: a category reload that rejects empties the three category lists, and a per-type content reload that rejects empties that type and sets it back to `idle` so the next @@ -310,7 +317,11 @@ on either side. M3U group titles travel verbatim (`normalizeParentalLockGroupTitles`, exact dedup): locks match `channel.group.title` exactly, so the trimming `uniqueStrings` used for favorites would weaken a lock on a title with -surrounding whitespace. +surrounding whitespace. A backup's lock lists are validated entry by entry +on import (`isWellFormedParentalLock*` in the shared contract): restore +treats them as authoritative and the normalizer drops what it does not +understand, so a damaged list is rejected rather than erasing the +playlist's persisted protection. `lockedGroupTitles` (M3U), `lockedCategories` (Xtream `{categoryType, xtreamId}`, Stalker `{categoryType, categoryId}`) travel in each entry's diff --git a/libs/portal/xtream/data-access/src/lib/stores/features/with-content.feature.reload.spec.ts b/libs/portal/xtream/data-access/src/lib/stores/features/with-content.feature.reload.spec.ts index 13e7246de..ae581d853 100644 --- a/libs/portal/xtream/data-access/src/lib/stores/features/with-content.feature.reload.spec.ts +++ b/libs/portal/xtream/data-access/src/lib/stores/features/with-content.feature.reload.spec.ts @@ -146,6 +146,33 @@ describe('withContent parental-lock reloads', () => { expect(store.isContentInitialized()).toBe(true); }); + it("carries the deferred request's publish guard into the reload", async () => { + const live = createDeferred(); + let calls = 0; + dataSource.getContent.mockImplementation(() => { + calls += 1; + return calls === 1 + ? live.promise + : Promise.resolve([{ xtream_id: calls }]); + }); + const initialization = store.initializeContent(); + await waitForCondition(() => calls === 1); + + let current = true; + await store.reloadCachedContent(() => current); + // The lock moved on again before the hydration settled. + current = false; + live.resolve([{ xtream_id: 1 }]); + await initialization; + + // The reload stops at its first guard check instead of publishing + // the older rows; the superseding apply reads everything again. + expect(calls).toBe(4); + expect(store.liveStreams()).toEqual([]); + expect(store.vodStreams()).toEqual([]); + expect(store.serialStreams()).toEqual([]); + }); + it('empties the category lists when their reload fails', async () => { dataSource.getCategories.mockResolvedValue([{ category_id: 'x' }]); await store.reloadCategories(); diff --git a/libs/portal/xtream/data-access/src/lib/stores/features/with-content.feature.ts b/libs/portal/xtream/data-access/src/lib/stores/features/with-content.feature.ts index 3f1a773a2..78db6ae3c 100644 --- a/libs/portal/xtream/data-access/src/lib/stores/features/with-content.feature.ts +++ b/libs/portal/xtream/data-access/src/lib/stores/features/with-content.feature.ts @@ -226,13 +226,20 @@ export function withContent() { * the reload runs once the hydration has settled. */ let reloadAfterInitialization = false; + /** The publish guard of the LATEST deferred request (see below). */ + let deferredPublishGuard: () => boolean = () => true; const runDeferredReload = async (): Promise => { if (!reloadAfterInitialization) { return; } reloadAfterInitialization = false; - await methods.reloadCategories(); - await methods.reloadCachedContent(); + // The requester's guard travels with the deferred reload: a + // relock during these reads supersedes them exactly as it + // would an ordinary reload. + const shouldPublish = deferredPublishGuard; + deferredPublishGuard = () => true; + await methods.reloadCategories(shouldPublish); + await methods.reloadCachedContent(shouldPublish); }; const dataService = inject(DataService); const databaseService = inject(DatabaseService); @@ -1598,6 +1605,7 @@ export function withContent() { // read under the previous lock state: withhold those // and reload once it has settled. reloadAfterInitialization = true; + deferredPublishGuard = shouldPublish; return; } const loadStates = store.contentLoadStateByType(); diff --git a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts index e146c7d58..45dae7052 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.spec.ts @@ -80,6 +80,21 @@ describe('ParentalLockLockStore', () => { expect(store.readable()).toBe(true); }); + it('builds an edit on the recovered store, never on the empty fail-closed one', async () => { + storage.readLocks.mockResolvedValueOnce(null); + await store.load(); + + await expect(store.setM3uLocks('pl-1', ['Adult'])).resolves.toBe(true); + + expect(storage.writeLocks).toHaveBeenLastCalledWith({ + 'pl-1': { + xtream: [{ categoryType: 'live', xtreamId: 7 }], + stalker: [], + m3u: ['Adult'], + }, + }); + }); + it('re-derives the index from a store recovered after a failed read', async () => { storage.readLocks.mockResolvedValueOnce(null); await store.load(); diff --git a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts index c10cd0e8f..036e57ec5 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-lock-store.service.ts @@ -182,11 +182,20 @@ export class ParentalLockLockStore { return this.locks()[playlistId]?.m3u ?? []; } + /** + * Every mutation re-reads a store that failed to read BEFORE building + * the edit: `locksFor` on the empty fail-closed store would otherwise + * turn one type's edit into the playlist's entire lock set once the + * write goes through, discarding its other locks. + */ async setXtreamLocks( playlistId: string, categoryType: ParentalLockXtreamCategoryType, xtreamIds: number[] ): Promise { + if (!(await this.ensureReadable())) { + return false; + } const previous = this.locksFor(playlistId); const next = withXtreamLocks(previous, categoryType, xtreamIds); return this.commitLocks(playlistId, previous, next, [categoryType]); @@ -197,6 +206,9 @@ export class ParentalLockLockStore { categoryType: ParentalLockStalkerCategoryType, categoryIds: string[] ): Promise { + if (!(await this.ensureReadable())) { + return false; + } return this.persistPlaylistLocks( playlistId, withStalkerLocks( @@ -211,6 +223,9 @@ export class ParentalLockLockStore { playlistId: string, groupTitles: string[] ): Promise { + if (!(await this.ensureReadable())) { + return false; + } return this.persistPlaylistLocks( playlistId, withM3uLocks(this.locksFor(playlistId), groupTitles) @@ -222,6 +237,9 @@ export class ParentalLockLockStore { playlistId: string, locks: ParentalLockPlaylistLocks ): Promise { + if (!(await this.ensureReadable())) { + return false; + } return this.commitLocks( playlistId, this.locksFor(playlistId), diff --git a/libs/services/src/lib/playlist-backup.service.pins.spec.ts b/libs/services/src/lib/playlist-backup.service.pins.spec.ts index ea7c7efca..92cfee42c 100644 --- a/libs/services/src/lib/playlist-backup.service.pins.spec.ts +++ b/libs/services/src/lib/playlist-backup.service.pins.spec.ts @@ -69,8 +69,7 @@ describe('PlaylistBackupService Xtream source pins', () => { const backup = await service.exportBackup(); - const entry = backup.manifest - .playlists[0] as XtreamPlaylistBackupEntry; + const entry = backup.manifest.playlists[0] as XtreamPlaylistBackupEntry; // Without this every "main source" choice vanishes on restore, with // nothing in the archive to say it was ever made. expect(entry.userState.sourcePins).toEqual([ @@ -161,7 +160,8 @@ describe('PlaylistBackupService Xtream source pins', () => { await Promise.all([firstImport, secondImport]); expect( replacePins.mock.calls.map( - ([, pins]) => (pins as Array<{ contentId: number }>)[0].contentId + ([, pins]) => + (pins as Array<{ contentId: number }>)[0].contentId ) ).toEqual([501, 502]); }); diff --git a/libs/services/src/lib/playlist-backup.service.roundtrip.spec.ts b/libs/services/src/lib/playlist-backup.service.roundtrip.spec.ts index 16a1c6b58..eb7ccfe9f 100644 --- a/libs/services/src/lib/playlist-backup.service.roundtrip.spec.ts +++ b/libs/services/src/lib/playlist-backup.service.roundtrip.spec.ts @@ -229,9 +229,11 @@ describe('PlaylistBackupService export → import round-trip', () => { }), ]); expect(state.epgUrls).toEqual(['https://epg.example.com/guide.xml']); - expect( - state.playlists.map((playlist) => playlist._id) - ).toEqual(['m3u-1', 'xtream-1', 'stalker-1']); + expect(state.playlists.map((playlist) => playlist._id)).toEqual([ + 'm3u-1', + 'xtream-1', + 'stalker-1', + ]); // Exporting the restored state must reproduce the original // manifest byte for byte (modulo the export timestamp): any field diff --git a/libs/services/src/lib/playlist-backup.service.ts b/libs/services/src/lib/playlist-backup.service.ts index 238f72095..88967eb35 100644 --- a/libs/services/src/lib/playlist-backup.service.ts +++ b/libs/services/src/lib/playlist-backup.service.ts @@ -26,6 +26,9 @@ import { XtreamBackupSourcePin, XtreamPendingRestoreState, createRandomId, + isWellFormedParentalLockGroupTitles, + isWellFormedParentalLockStalkerCategories, + isWellFormedParentalLockXtreamCategories, normalizeParentalLockGroupTitles, normalizeParentalLockStalkerCategories, normalizeParentalLockXtreamCategories, @@ -525,6 +528,19 @@ export class PlaylistBackupService { `M3U backup "${entry.title}" is missing raw playlist data.` ); } + // Restore replaces the playlist's locks with the archive's, + // and the normalizer drops what it does not understand — a + // damaged lock list would erase the persisted protection. + if ( + entry.userState?.lockedGroupTitles !== undefined && + !isWellFormedParentalLockGroupTitles( + entry.userState.lockedGroupTitles + ) + ) { + throw new PlaylistBackupError( + `M3U backup "${entry.title}" has invalid parental locks.` + ); + } break; case 'xtream': if ( @@ -554,7 +570,9 @@ export class PlaylistBackupService { if ( entry.userState.lockedCategories !== undefined && - !Array.isArray(entry.userState.lockedCategories) + !isWellFormedParentalLockXtreamCategories( + entry.userState.lockedCategories + ) ) { throw new PlaylistBackupError( `Xtream backup "${entry.title}" has invalid parental locks.` @@ -581,6 +599,16 @@ export class PlaylistBackupService { `Stalker backup "${entry.title}" is missing connection metadata.` ); } + if ( + entry.userState?.lockedCategories !== undefined && + !isWellFormedParentalLockStalkerCategories( + entry.userState.lockedCategories + ) + ) { + throw new PlaylistBackupError( + `Stalker backup "${entry.title}" has invalid parental locks.` + ); + } break; default: throw new PlaylistBackupError( diff --git a/libs/services/src/lib/playlist-backup.service.xtream-restore.spec.ts b/libs/services/src/lib/playlist-backup.service.xtream-restore.spec.ts index a708a7566..72fd1a7c1 100644 --- a/libs/services/src/lib/playlist-backup.service.xtream-restore.spec.ts +++ b/libs/services/src/lib/playlist-backup.service.xtream-restore.spec.ts @@ -41,8 +41,7 @@ describe('PlaylistBackupService Xtream hidden categories (issue #1017)', () => { const backup = await service.exportBackup(); - const entry = backup.manifest - .playlists[0] as XtreamPlaylistBackupEntry; + const entry = backup.manifest.playlists[0] as XtreamPlaylistBackupEntry; const expectedHiddenCategories = [ { categoryType: 'live', xtreamId: 101 }, { categoryType: 'movies', xtreamId: 201 }, @@ -92,9 +91,7 @@ describe('PlaylistBackupService Xtream hidden categories (issue #1017)', () => { 'xtream-1', expect.objectContaining({ state: expect.objectContaining({ - hiddenCategories: [ - { categoryType: 'live', xtreamId: 101 }, - ], + hiddenCategories: [{ categoryType: 'live', xtreamId: 101 }], }), }), expect.any(Function) @@ -107,11 +104,21 @@ describe('PlaylistBackupService Xtream hidden categories (issue #1017)', () => { ); }); + it('rejects a damaged parental lock list instead of erasing the persisted locks', async () => { + const collaborators = createRestoreCollaborators(); + const service = createPlaylistBackupService(collaborators); + const manifest = createXtreamManifest([]); + ( + manifest.playlists[0].userState as { lockedCategories?: unknown } + ).lockedCategories = [{}]; - - - - + await expect( + service.importBackup(JSON.stringify(manifest)) + ).rejects.toThrow(/invalid parental locks/); + expect( + collaborators.databaseService.updateCategoryVisibility + ).not.toHaveBeenCalled(); + }); it('rejects entries with missing user-state collections instead of wiping user data', async () => { const collaborators = createRestoreCollaborators(); @@ -121,9 +128,8 @@ describe('PlaylistBackupService Xtream hidden categories (issue #1017)', () => { // treated as an authoritative "empty" state: the merge path would // unhide every category and delete favorites/recent/positions. const manifest = createXtreamManifest([]); - delete ( - manifest.playlists[0] as unknown as { userState?: unknown } - ).userState; + delete (manifest.playlists[0] as unknown as { userState?: unknown }) + .userState; await expect( service.importBackup(JSON.stringify(manifest)) diff --git a/libs/shared/interfaces/src/lib/parental-lock.util.ts b/libs/shared/interfaces/src/lib/parental-lock.util.ts index 9f166b056..3afd2ac08 100644 --- a/libs/shared/interfaces/src/lib/parental-lock.util.ts +++ b/libs/shared/interfaces/src/lib/parental-lock.util.ts @@ -152,21 +152,58 @@ export function normalizeParentalLockStalkerCategories( return result; } -function isWellFormedList( - value: unknown, - isWellFormedEntry: (entry: unknown) => boolean +/** A list whose every entry the matching normalizer would KEEP. */ +export function isWellFormedParentalLockXtreamCategories( + value: unknown ): boolean { - return Array.isArray(value) && value.every(isWellFormedEntry); + return ( + Array.isArray(value) && + value.every( + (entry) => + isRecord(entry) && + PARENTAL_LOCK_XTREAM_CATEGORY_TYPES.includes( + entry['categoryType'] as ParentalLockXtreamCategoryType + ) && + normalizeNumericId(entry['xtreamId']) !== null + ) + ); +} + +export function isWellFormedParentalLockStalkerCategories( + value: unknown +): boolean { + return ( + Array.isArray(value) && + value.every( + (entry) => + isRecord(entry) && + PARENTAL_LOCK_STALKER_CATEGORY_TYPES.includes( + entry['categoryType'] as ParentalLockStalkerCategoryType + ) && + ((typeof entry['categoryId'] === 'string' && + entry['categoryId'].trim() !== '' && + entry['categoryId'].trim() !== '*') || + (typeof entry['categoryId'] === 'number' && + Number.isFinite(entry['categoryId']))) + ) + ); +} + +export function isWellFormedParentalLockGroupTitles(value: unknown): boolean { + return ( + Array.isArray(value) && + value.every((entry) => typeof entry === 'string') + ); } /** * Whether a persisted store has the shape `writeLocks` produces: every * playlist entry an object whose `xtream`/`stalker`/`m3u` lists are all * present (a missing list is corruption, not "empty") and hold only - * entries the normalizers accept. The normalizers DROP what they - * do not understand, which is right for user-supplied backups but turns a - * corrupted persisted store into "nothing is locked"; a store that fails - * this check is treated as unreadable instead. + * entries the normalizers accept. The normalizers DROP what they do not + * understand; on a persisted store (and on a backup's lock lists, which a + * restore treats as authoritative) that would turn corruption into + * "nothing is locked", so such data is rejected instead. */ export function isWellFormedParentalLockStore(value: unknown): boolean { if (!isRecord(value)) { @@ -175,29 +212,9 @@ export function isWellFormedParentalLockStore(value: unknown): boolean { return Object.values(value).every( (locks) => isRecord(locks) && - isWellFormedList( - locks['xtream'], - (entry) => - isRecord(entry) && - PARENTAL_LOCK_XTREAM_CATEGORY_TYPES.includes( - entry['categoryType'] as ParentalLockXtreamCategoryType - ) && - normalizeNumericId(entry['xtreamId']) !== null - ) && - isWellFormedList( - locks['stalker'], - (entry) => - isRecord(entry) && - PARENTAL_LOCK_STALKER_CATEGORY_TYPES.includes( - entry['categoryType'] as ParentalLockStalkerCategoryType - ) && - ((typeof entry['categoryId'] === 'string' && - entry['categoryId'].trim() !== '' && - entry['categoryId'].trim() !== '*') || - (typeof entry['categoryId'] === 'number' && - Number.isFinite(entry['categoryId']))) - ) && - isWellFormedList(locks['m3u'], (entry) => typeof entry === 'string') + isWellFormedParentalLockXtreamCategories(locks['xtream']) && + isWellFormedParentalLockStalkerCategories(locks['stalker']) && + isWellFormedParentalLockGroupTitles(locks['m3u']) ); }