From 92382052d82b93afd4180bd83179d0ecc3e2892a Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 27 Sep 2026 13:08:59 +0200 Subject: [PATCH] fix(settings): refuse lock removals that commit after a relock and keep the index stale until the store write lands Lock edits that take a lock away now commit only while the session is unlocked, checked inside the write queue at commit time, so an editor save still in flight (or queued) when the app relocks cannot remove locks. Adding locks stays allowed. The Xtream category dialog drops its lock draft after a relock, and a backup restore re-asks the PIN only when it would remove a lock. Clearing a playlist's last lock keeps its index stale until the store write has landed. Co-Authored-By: Claude Opus 5.5 --- docs/architecture/parental-lock.md | 21 ++++-- ...tegory-management-dialog.component.spec.ts | 17 +++++ .../category-management-dialog.component.ts | 5 +- .../parental-lock-lock-store.service.spec.ts | 68 ++++++++++++++++++ .../parental-lock-lock-store.service.ts | 70 +++++++++++++------ .../parental-lock-store.util.spec.ts | 34 +++++++++ .../parental-lock/parental-lock-store.util.ts | 39 +++++++++++ .../parental-lock/parental-lock.service.ts | 3 + .../src/lib/playlist-backup.service.ts | 15 ++-- 9 files changed, 239 insertions(+), 33 deletions(-) create mode 100644 libs/services/src/lib/parental-lock/parental-lock-store.util.spec.ts diff --git a/docs/architecture/parental-lock.md b/docs/architecture/parental-lock.md index bf90bd3a0..31a83928b 100644 --- a/docs/architecture/parental-lock.md +++ b/docs/architecture/parental-lock.md @@ -401,7 +401,16 @@ by the index alone: clear or the store write fails, the index is re-stamped from the previous locks at once — title matching and multi-source discovery query the worker directly and trust the index, so a stale flag alone would not - protect them. + protect them. The playlist stays stale until the store write has landed + too: while it is pending the index is already cleared but the store + still holds the lock. +- An edit that takes a lock away commits only while the session is + unlocked, checked inside the write queue at commit time + (`ParentalLockLockStore.setRemovalGate`, set by `ParentalLockService`). + An editor opened while unlocked may still be saving, or queued behind + another write, when the app relocks. Adding locks is always allowed. + Playlist deletion and "Remove all playlists" are not edits and are not + gated. - Every launch re-derives the index from the store for each playlist that has locks, and a store recovered by a later successful read marks its playlists stale the same way. The reconcile is awaited inside the store's @@ -436,11 +445,11 @@ never runs against the empty fail-closed snapshot. 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. That answer can go -stale during a long import (idle relock, "Lock now"), so each merge asks -again right before it replaces locks if the app has relocked; a refusal -fails that entry and keeps its previous locks. A newly created playlist -starts without locks, so its restore can only add some and is not asked -again. Locks are restored LAST +stale during a long import (idle relock, "Lock now"), so an entry whose +restore would take a lock away asks again right before it replaces locks +if the app has relocked; a refusal fails that entry and keeps its previous +locks. A restore that only adds locks is not asked again. 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 diff --git a/libs/portal/xtream/feature/src/lib/category-management-dialog/category-management-dialog.component.spec.ts b/libs/portal/xtream/feature/src/lib/category-management-dialog/category-management-dialog.component.spec.ts index bc4969e42..0501c2805 100644 --- a/libs/portal/xtream/feature/src/lib/category-management-dialog/category-management-dialog.component.spec.ts +++ b/libs/portal/xtream/feature/src/lib/category-management-dialog/category-management-dialog.component.spec.ts @@ -208,6 +208,23 @@ describe('CategoryManagementDialogComponent', () => { expect(parentalLock.setXtreamLocks).not.toHaveBeenCalled(); }); + it('drops the lock draft when the session relocks during a pending save', async () => { + parentalLock.enabled.set(true); + fixture.detectChanges(); + let finishVisibility: () => void = () => undefined; + db.updateCategoryVisibility.mockImplementationOnce( + () => new Promise((resolve) => (finishVisibility = resolve)) + ); + component.deselectAll(); + + const saving = component.save(); + parentalLock.active.set(true); + finishVisibility(); + await saving; + + expect(parentalLock.setXtreamLocks).not.toHaveBeenCalled(); + }); + it('discards pending bulk changes on cancel', () => { component.searchTerm.set('FR'); component.selectAll(); diff --git a/libs/portal/xtream/feature/src/lib/category-management-dialog/category-management-dialog.component.ts b/libs/portal/xtream/feature/src/lib/category-management-dialog/category-management-dialog.component.ts index af434db89..ceddfd01a 100644 --- a/libs/portal/xtream/feature/src/lib/category-management-dialog/category-management-dialog.component.ts +++ b/libs/portal/xtream/feature/src/lib/category-management-dialog/category-management-dialog.component.ts @@ -240,7 +240,10 @@ export class CategoryManagementDialogComponent implements OnInit { await this.dbService.updateCategoryVisibility(toShow, false); } - if (this.showLocks()) { + // A relock while the visibility writes ran closed the dialog: + // its lock draft is dropped. The lock store refuses a removal + // that still commits after a relock (it checks at commit time). + if (this.showLocks() && !this.parentalLock.active()) { const saved = await this.parentalLock.setXtreamLocks( this.data.playlistId, this.getDbType(), 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 67c5ddde8..28060bf1e 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 @@ -404,4 +404,72 @@ describe('ParentalLockLockStore', () => { expect(setCategoryLocks).toHaveBeenCalledWith('pl-1', 'live', [7, 9]); expect(store.readable()).toBe(true); }); + + it('stays not readable until the store write of a cleared last lock lands', async () => { + await store.load(); + await store.ensureReadable(); + let finishWrite: (ok: boolean) => void = () => undefined; + storage.writeLocks.mockImplementationOnce( + () => new Promise((resolve) => (finishWrite = resolve)) + ); + const revision = store.revision(); + + const clearing = store.setXtreamLocks('pl-1', 'live', []); + await waitFor(() => storage.writeLocks.mock.calls.length > 0); + // The index is already cleared; the store still holds the lock. + expect(setCategoryLocks).toHaveBeenCalledWith('pl-1', 'live', []); + expect(store.readable()).toBe(false); + expect(store.revision()).toBe(revision); + + finishWrite(false); + await expect(clearing).resolves.toBe(false); + // Restored from the store, which kept the lock. + expect(setCategoryLocks).toHaveBeenLastCalledWith('pl-1', 'live', [7]); + expect(store.readable()).toBe(true); + expect(store.lockedXtreamIds('pl-1', 'live')).toEqual([7]); + }); + it('refuses, at commit time, an edit that removes a lock while removal is gated', async () => { + await store.load(); + await store.ensureReadable(); + let unlocked = true; + store.setRemovalGate(() => unlocked); + jest.spyOn(console, 'warn').mockImplementation(() => undefined); + // An earlier write holds the queue while the app relocks: the edit + // queued behind it was built unlocked but commits locked. + let finishFirst: (ok: boolean) => void = () => undefined; + storage.writeLocks.mockImplementationOnce( + () => new Promise((resolve) => (finishFirst = resolve)) + ); + const adding = store.setM3uLocks('pl-1', ['Adults']); + const removing = store.setXtreamLocks('pl-1', 'live', []); + await waitFor(() => storage.writeLocks.mock.calls.length > 0); + unlocked = false; + finishFirst(true); + + await expect(adding).resolves.toBe(true); + await expect(removing).resolves.toBe(false); + expect(store.lockedXtreamIds('pl-1', 'live')).toEqual([7]); + + // Adding stays allowed while locked; removing again once unlocked. + await expect( + store.setXtreamLocks('pl-1', 'live', [7, 9]) + ).resolves.toBe(true); + await expect( + store.replacePlaylistLocks('pl-1', { + xtream: [], + stalker: [], + m3u: [], + }) + ).resolves.toBe(false); + unlocked = true; + await expect(store.setM3uLocks('pl-1', [])).resolves.toBe(true); + expect(store.lockedGroupTitles('pl-1')).toEqual([]); + }); }); + +async function waitFor(condition: () => boolean): Promise { + for (let i = 0; i < 50 && !condition(); i++) { + await Promise.resolve(); + } + expect(condition()).toBe(true); +} 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 086a1c616..7f83f7247 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 @@ -14,6 +14,7 @@ import { DatabaseService } from '../database-electron.service'; import { RuntimeCapabilitiesService } from '../runtime-capabilities.service'; import { ParentalLockStorageService } from './parental-lock-storage'; import { + isLockRemovalAllowed, withM3uLocks, withStalkerLocks, withXtreamLocks, @@ -58,6 +59,13 @@ export class ParentalLockLockStore { private readonly revisionState = signal(0); private readonly loadedState = signal(false); private loading: Promise | null = null; + /** + * Whether an edit may REMOVE a lock right now; set by + * `ParentalLockService` to "the session is not locked". Asked inside + * the write queue, at commit time: an editor opened while unlocked may + * still be saving (or queued) when the app relocks. + */ + private mayRemoveLocks: () => boolean = () => true; /** * Mutations run one at a time: each rewrites the WHOLE persisted store * from the in-memory copy, so two overlapping edits would snapshot the @@ -191,6 +199,10 @@ export class ParentalLockLockStore { } } + setRemovalGate(gate: () => boolean): void { + this.mayRemoveLocks = gate; + } + locksFor(playlistId: string): ParentalLockPlaylistLocks { return ( this.locks()[playlistId] ?? createEmptyParentalLockPlaylistLocks() @@ -239,6 +251,9 @@ export class ParentalLockLockStore { this.lockedXtreamIds(playlistId, categoryType) ) ); + if (!isLockRemovalAllowed(previous, next, this.mayRemoveLocks)) { + return false; + } return this.commitLocks(playlistId, previous, next, [categoryType]); }); } @@ -252,17 +267,19 @@ export class ParentalLockLockStore { if (!(await this.ensureReadable())) { return false; } - return this.persistPlaylistLocks( - playlistId, - withStalkerLocks( - this.locksFor(playlistId), - categoryType, - applyLockListEdit( - categoryIds, - this.lockedStalkerIds(playlistId, categoryType) - ) + const previous = this.locksFor(playlistId); + const next = withStalkerLocks( + previous, + categoryType, + applyLockListEdit( + categoryIds, + this.lockedStalkerIds(playlistId, categoryType) ) ); + return ( + isLockRemovalAllowed(previous, next, this.mayRemoveLocks) && + this.persistPlaylistLocks(playlistId, next) + ); }); } @@ -274,16 +291,18 @@ export class ParentalLockLockStore { if (!(await this.ensureReadable())) { return false; } - return this.persistPlaylistLocks( - playlistId, - withM3uLocks( - this.locksFor(playlistId), - applyLockListEdit( - groupTitles, - this.lockedGroupTitles(playlistId) - ) + const previous = this.locksFor(playlistId); + const next = withM3uLocks( + previous, + applyLockListEdit( + groupTitles, + this.lockedGroupTitles(playlistId) ) ); + return ( + isLockRemovalAllowed(previous, next, this.mayRemoveLocks) && + this.persistPlaylistLocks(playlistId, next) + ); }); } @@ -296,9 +315,13 @@ export class ParentalLockLockStore { if (!(await this.ensureReadable())) { return false; } + const previous = this.locksFor(playlistId); + if (!isLockRemovalAllowed(previous, locks, this.mayRemoveLocks)) { + return false; + } return this.commitLocks( playlistId, - this.locksFor(playlistId), + previous, locks, XTREAM_CATEGORY_TYPES ); @@ -369,6 +392,10 @@ export class ParentalLockLockStore { if (!(await this.ensureReadable())) { return false; } + // Stale until the STORE write has landed too: while it is + // pending the index is already cleared but the store still + // holds the lock, and a relock in that window must not reload + // through the cleared index. this.markStaleWhileStamping(playlistId); if ( (await this.stampXtreamLocks( @@ -376,9 +403,12 @@ export class ParentalLockLockStore { categoryTypes, normalizedNext )) && - (this.unmarkIndexStale(playlistId), - await this.persistPlaylistLocks(playlistId, normalizedNext)) + (await this.persistPlaylistLocks(playlistId, normalizedNext, { + publish: false, + })) ) { + this.unmarkIndexStale(playlistId); + this.revisionState.update((value) => value + 1); return true; } // The clear (partly) reached the index but the store still holds diff --git a/libs/services/src/lib/parental-lock/parental-lock-store.util.spec.ts b/libs/services/src/lib/parental-lock/parental-lock-store.util.spec.ts new file mode 100644 index 000000000..d054ae180 --- /dev/null +++ b/libs/services/src/lib/parental-lock/parental-lock-store.util.spec.ts @@ -0,0 +1,34 @@ +import { removesParentalLocks } from './parental-lock-store.util'; + +describe('removesParentalLocks', () => { + const previous = { + xtream: [{ categoryType: 'live' as const, xtreamId: 7 }], + stalker: [{ categoryType: 'vod' as const, categoryId: '3' }], + m3u: ['Adults'], + }; + + it('is false when every previous lock is kept, extra locks or not', () => { + expect(removesParentalLocks(previous, previous)).toBe(false); + expect( + removesParentalLocks(previous, { + ...previous, + m3u: ['Adults', 'News'], + }) + ).toBe(false); + }); + + it('is true when any portal list drops a lock', () => { + expect( + removesParentalLocks(previous, { ...previous, xtream: [] }) + ).toBe(true); + expect( + removesParentalLocks(previous, { + ...previous, + stalker: [{ categoryType: 'itv', categoryId: '3' }], + }) + ).toBe(true); + expect(removesParentalLocks(previous, { ...previous, m3u: [] })).toBe( + true + ); + }); +}); diff --git a/libs/services/src/lib/parental-lock/parental-lock-store.util.ts b/libs/services/src/lib/parental-lock/parental-lock-store.util.ts index 511be0a08..951ee0674 100644 --- a/libs/services/src/lib/parental-lock/parental-lock-store.util.ts +++ b/libs/services/src/lib/parental-lock/parental-lock-store.util.ts @@ -54,3 +54,42 @@ export function withM3uLocks( ): ParentalLockPlaylistLocks { return { ...current, m3u: [...new Set(groupTitles)] }; } + +/** Whether `next` drops any lock that `previous` holds. */ +export function removesParentalLocks( + previous: ParentalLockPlaylistLocks, + next: ParentalLockPlaylistLocks +): boolean { + const xtream = new Set( + next.xtream.map((lock) => `${lock.categoryType}:${lock.xtreamId}`) + ); + const stalker = new Set( + next.stalker.map((lock) => `${lock.categoryType}:${lock.categoryId}`) + ); + const m3u = new Set(next.m3u); + return ( + previous.xtream.some( + (lock) => !xtream.has(`${lock.categoryType}:${lock.xtreamId}`) + ) || + previous.stalker.some( + (lock) => !stalker.has(`${lock.categoryType}:${lock.categoryId}`) + ) || + previous.m3u.some((title) => !m3u.has(title)) + ); +} + +/** + * Whether an edit from `previous` to `next` may commit: adding locks always + * may, removing one only while `mayRemove()` says so (logged when refused). + */ +export function isLockRemovalAllowed( + previous: ParentalLockPlaylistLocks, + next: ParentalLockPlaylistLocks, + mayRemove: () => boolean +): boolean { + if (!removesParentalLocks(previous, next) || mayRemove()) { + return true; + } + console.warn('A parental lock edit was refused: the app is locked.'); + return false; +} diff --git a/libs/services/src/lib/parental-lock/parental-lock.service.ts b/libs/services/src/lib/parental-lock/parental-lock.service.ts index e631b96a4..91db0aa0c 100644 --- a/libs/services/src/lib/parental-lock/parental-lock.service.ts +++ b/libs/services/src/lib/parental-lock/parental-lock.service.ts @@ -122,6 +122,9 @@ export class ParentalLockService { ); constructor() { + // Lock edits that take a lock away need an unlocked session at the + // moment they commit, not only when their editor opened. + this.locks.setRemovalGate(() => !this.active()); effect(() => { const active = this.active(); if (!this.settingsReady()) { diff --git a/libs/services/src/lib/playlist-backup.service.ts b/libs/services/src/lib/playlist-backup.service.ts index 3cf084785..3d332f5c7 100644 --- a/libs/services/src/lib/playlist-backup.service.ts +++ b/libs/services/src/lib/playlist-backup.service.ts @@ -7,6 +7,7 @@ import { VodSourcePinService } from './vod-source-pin.service'; import { PlaybackPositionService } from './playback-position.service'; import { XtreamPendingRestoreService } from './xtream-pending-restore.service'; import { ParentalLockService } from './parental-lock/parental-lock.service'; +import { removesParentalLocks } from './parental-lock/parental-lock-store.util'; import { isParentalLockPlaylistLocksEmpty, isM3uRecentlyViewedItem, @@ -1186,14 +1187,16 @@ export class PlaylistBackupService { }; } - // A merge REPLACES the playlist's locks, possibly with fewer. The PIN - // asked at the start may be stale by now (idle relock or "Lock now" - // during a long import): ask again if the session relocked. A new - // playlist starts without locks, so its restore can only add some. + // Taking a lock away needs an unlocked session when it commits. The + // PIN asked at the start may be stale by now (idle relock or "Lock + // now" during a long import): ask again if the session relocked. + // A restore that only adds locks is not asked. if ( next && - isMerge && - !staleOnNewId && + removesParentalLocks( + this.parentalLock.locksFor(playlistId), + next + ) && !(await this.parentalLock.requestUnlock()) ) { throw new PlaylistBackupError(