From 501e196456911b23116194baed104a1e632cf71d Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 27 Sep 2026 12:31:17 +0200 Subject: [PATCH] fix(settings): require the PIN for lock-bearing backup restores and fail closed during index re-stamps - A backup carrying lock lists replaces the matching playlists' locks, possibly with an emptier set; the import now asks for the PIN (after the file was chosen) and aborts when it is refused. - While a write re-stamps the SQLite index the playlist counts as stale, so a relock inside that window reloads fail-closed instead of through the old stamps. The internal store write now needs only a readable store, so a rollback can still land. Co-Authored-By: Claude Opus 5.5 --- docs/architecture/parental-lock.md | 10 ++++-- .../parental-lock-lock-store.service.spec.ts | 21 +++++++++++ .../parental-lock-lock-store.service.ts | 35 +++++++++++++++++-- .../playlist-backup.service.test-helpers.ts | 1 + .../src/lib/playlist-backup.service.ts | 18 ++++++++++ ...list-backup.service.xtream-restore.spec.ts | 32 +++++++++++++++++ 6 files changed, 112 insertions(+), 5 deletions(-) diff --git a/docs/architecture/parental-lock.md b/docs/architecture/parental-lock.md index 80decd4cf..e8d1f0315 100644 --- a/docs/architecture/parental-lock.md +++ b/docs/architecture/parental-lock.md @@ -382,7 +382,10 @@ by the index alone: reload on is published only once every touched type is stamped — also after a rollback — so a reload cannot read a later type through its old stamps. If the rollback write or its re-stamp fails too, the playlist is - marked stale and re-stamped on the next store access. + marked stale and re-stamped on the next store access. For the whole + re-stamp window — store changed, stamps not yet landed — the playlist + counts as stale, so `readable` is false and a relock inside the window + reloads fail-closed instead of through the old stamps. - A write that removes a playlist's LAST lock clears the index first and drops the key afterwards: the launch-time reconcile finds playlists only through their key, so an interruption must leave store-with-lock and @@ -422,7 +425,10 @@ never runs against the empty fail-closed snapshot. ## Backup -Locks are restored LAST for each entry, after the Xtream data restore, so a +A backup carrying lock lists replaces the matching playlists' locks, +possibly with an emptier set, so the import asks for the PIN first (after +the file was chosen) and aborts when it is refused. Locks are restored LAST +for each entry, after the Xtream data restore, so a failed merge leaves the playlist's previous locks in place. M3U group titles travel verbatim (`normalizeParentalLockGroupTitles`, exact dedup): locks match `channel.group.title` exactly, so the trimming 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 47af0b5ca..67c5ddde8 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 @@ -315,6 +315,27 @@ describe('ParentalLockLockStore', () => { expect(storage.writeLocks).toHaveBeenCalledWith({}); }); + it('is not readable while a write is re-stamping the index', async () => { + await store.load(); + await store.ensureReadable(); + const readableAtStamp: boolean[] = []; + setCategoryLocks.mockImplementation(async () => { + readableAtStamp.push(store.readable()); + return true; + }); + + await expect( + store.setXtreamLocks('pl-1', 'live', [7, 9]) + ).resolves.toBe(true); + await expect(store.setXtreamLocks('pl-1', 'live', [])).resolves.toBe( + true + ); + + expect(readableAtStamp.length).toBeGreaterThan(0); + expect(readableAtStamp.every((readable) => !readable)).toBe(true); + expect(store.readable()).toBe(true); + }); + it('rolls the store back when the index re-stamp fails', async () => { await store.load(); setCategoryLocks.mockResolvedValue(false); 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 ca37a71ac..086a1c616 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 @@ -161,6 +161,11 @@ export class ParentalLockLockStore { this.staleIndexCount.set(this.staleIndexPlaylists.size); } + private unmarkIndexStale(playlistId: string): void { + this.staleIndexPlaylists.delete(playlistId); + this.staleIndexCount.set(this.staleIndexPlaylists.size); + } + /** Re-stamps every playlist whose index may lag behind the store. */ private async reconcileXtreamIndex(): Promise { if (!this.runtime.supportsXtreamSqliteDataSource) { @@ -364,13 +369,15 @@ export class ParentalLockLockStore { if (!(await this.ensureReadable())) { return false; } + this.markStaleWhileStamping(playlistId); if ( (await this.stampXtreamLocks( playlistId, categoryTypes, normalizedNext )) && - (await this.persistPlaylistLocks(playlistId, normalizedNext)) + (this.unmarkIndexStale(playlistId), + await this.persistPlaylistLocks(playlistId, normalizedNext)) ) { return true; } @@ -392,7 +399,9 @@ export class ParentalLockLockStore { ) { return false; } + this.markStaleWhileStamping(playlistId); if (await this.stampXtreamLocks(playlistId, categoryTypes)) { + this.unmarkIndexStale(playlistId); this.revisionState.update((value) => value + 1); return true; } @@ -426,10 +435,24 @@ export class ParentalLockLockStore { if (!restored || !restamped) { console.error('Failed to roll back the parental lock index.'); this.markIndexStale(playlistId); + } else { + this.unmarkIndexStale(playlistId); } this.revisionState.update((value) => value + 1); } + /** + * From the store change until every stamp has landed the SQLite index + * does not agree with the store: `readable` must be false for that whole + * window, or a relock inside it would reload through the old stamps. + * Only Electron has the index. + */ + private markStaleWhileStamping(playlistId: string): void { + if (this.runtime.supportsXtreamSqliteDataSource) { + this.markIndexStale(playlistId); + } + } + /** Re-stamps the index from `locks`; marks it stale when that fails. */ private async restoreIndex( playlistId: string, @@ -439,8 +462,10 @@ export class ParentalLockLockStore { if (!(await this.stampXtreamLocks(playlistId, categoryTypes, locks))) { console.error('Failed to restore the parental lock index.'); this.markIndexStale(playlistId); - this.revisionState.update((value) => value + 1); + } else { + this.unmarkIndexStale(playlistId); } + this.revisionState.update((value) => value + 1); } /** @@ -472,7 +497,11 @@ export class ParentalLockLockStore { locks: ParentalLockPlaylistLocks, options: { publish?: boolean } = {} ): Promise { - if (!(await this.ensureReadable())) { + // Every public mutation runs `ensureReadable()` before it builds its + // edit. Here only the STORE must be known: the index of the + // playlist being written is deliberately stale while it is + // re-stamped, and a rollback must still be able to write. + if (this.unreadable()) { return false; } const normalized = normalizeParentalLockPlaylistLocks(locks); diff --git a/libs/services/src/lib/playlist-backup.service.test-helpers.ts b/libs/services/src/lib/playlist-backup.service.test-helpers.ts index 590de721d..f53ef0e19 100644 --- a/libs/services/src/lib/playlist-backup.service.test-helpers.ts +++ b/libs/services/src/lib/playlist-backup.service.test-helpers.ts @@ -124,6 +124,7 @@ export function createPlaylistBackupService( initialize: jest.fn().mockResolvedValue(undefined), locksReadable: jest.fn(() => true), ensureLocksReadable: jest.fn().mockResolvedValue(true), + requestUnlock: jest.fn().mockResolvedValue(true), locksFor: jest.fn(() => ({ xtream: [], stalker: [], m3u: [] })), replacePlaylistLocks: jest.fn().mockResolvedValue(true), }, diff --git a/libs/services/src/lib/playlist-backup.service.ts b/libs/services/src/lib/playlist-backup.service.ts index 01e5b4546..92d2fafc5 100644 --- a/libs/services/src/lib/playlist-backup.service.ts +++ b/libs/services/src/lib/playlist-backup.service.ts @@ -131,6 +131,24 @@ export class PlaylistBackupService { json: string ): Promise { const manifest = this.parseManifest(json); + // A backup carrying lock lists REPLACES the matching playlists' + // locks, possibly with an emptier set: that is a lock edit, and lock + // edits sit behind the PIN. Asked here, after the file was chosen. + const carriesLocks = manifest.playlists.some((entry) => { + const state = entry.userState as { + lockedCategories?: unknown; + lockedGroupTitles?: unknown; + }; + return ( + state?.lockedCategories !== undefined || + state?.lockedGroupTitles !== undefined + ); + }); + if (carriesLocks && !(await this.parentalLock.requestUnlock())) { + throw new PlaylistBackupError( + 'Enter the parental PIN to restore a backup that carries parental locks.' + ); + } const existingPlaylists = await firstValueFrom( this.playlistsService.getAllData() ); 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 bf78c0464..9d21398a9 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 @@ -113,6 +113,7 @@ describe('PlaylistBackupService Xtream hidden categories (issue #1017)', () => { initialize: jest.fn().mockResolvedValue(undefined), locksReadable: jest.fn(() => true), ensureLocksReadable: jest.fn().mockResolvedValue(true), + requestUnlock: jest.fn().mockResolvedValue(true), locksFor: jest.fn(() => ({ xtream: [], stalker: [], m3u: [] })), replacePlaylistLocks, }, @@ -140,6 +141,7 @@ describe('PlaylistBackupService Xtream hidden categories (issue #1017)', () => { initialize: jest.fn().mockResolvedValue(undefined), locksReadable: jest.fn(() => true), ensureLocksReadable: jest.fn().mockResolvedValue(true), + requestUnlock: jest.fn().mockResolvedValue(true), // A failed cleanup left locks under the id the restore reuses. locksFor: jest.fn(() => ({ xtream: [{ categoryType: 'live', xtreamId: 1 }], @@ -163,6 +165,36 @@ describe('PlaylistBackupService Xtream hidden categories (issue #1017)', () => { }); }); + it('asks for the PIN before restoring lock lists and aborts when it is refused', async () => { + const collaborators = createRestoreCollaborators(); + const requestUnlock = jest.fn().mockResolvedValue(false); + const replacePlaylistLocks = jest.fn().mockResolvedValue(true); + const service = createPlaylistBackupService({ + ...collaborators, + parentalLock: { + initialize: jest.fn().mockResolvedValue(undefined), + locksReadable: jest.fn(() => true), + ensureLocksReadable: jest.fn().mockResolvedValue(true), + requestUnlock, + locksFor: jest.fn(() => ({ xtream: [], stalker: [], m3u: [] })), + replacePlaylistLocks, + }, + }); + const manifest = createXtreamManifest([]); + ( + manifest.playlists[0].userState as { lockedCategories?: unknown } + ).lockedCategories = []; + + await expect( + service.importBackup(JSON.stringify(manifest)) + ).rejects.toThrow(/parental PIN/); + expect(requestUnlock).toHaveBeenCalled(); + expect(replacePlaylistLocks).not.toHaveBeenCalled(); + expect( + collaborators.playlistsService.addPlaylist + ).not.toHaveBeenCalled(); + }); + it('rejects a damaged parental lock list instead of erasing the persisted locks', async () => { const collaborators = createRestoreCollaborators(); const service = createPlaylistBackupService(collaborators);