From d2b0325138dbd0f526ccac48be22cb78dbaf3e74 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 13 Oct 2026 23:04:07 +0200 Subject: [PATCH] fix(sources): decode nested URLs before classifying; narrow Xtream paths - A URL nested in a parameter or path segment is percent-decoded one round at a time until it reads as a URL or parameter list, so one encoded twice is cleaned too; a value still encoded after the last round is dropped. The name check unwraps nested values the same way. - The two segments after `/live/` and its siblings count as an account only when a stream follows them, so a playlist path such as `/live/news/list.m3u` keeps its file name in the URL and the title. - Adding a playlist from the phone drawer closes the drawer first, so two focus-trapped modals never stack. Co-Authored-By: Claude Opus 5.5 --- .../lib/source-display/source-display.spec.ts | 14 ++++ .../src/lib/source-display/source-display.ts | 25 ++++--- .../strip-url-credentials.spec.ts | 49 ++++++++++++ .../source-display/strip-url-credentials.ts | 74 ++++++++++++++++--- ...ce-sources-filters-panel.component.spec.ts | 15 +++- ...rkspace-sources-filters-panel.component.ts | 3 + 6 files changed, 155 insertions(+), 25 deletions(-) diff --git a/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts b/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts index 3b0fb9f1a..867fcfaae 100644 --- a/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts +++ b/libs/playlist/shared/util/src/lib/source-display/source-display.spec.ts @@ -110,6 +110,20 @@ describe('sourceDisplayName', () => { ).toBe('host.test'); }); + it('keeps a playlist file name from a path that names no account', () => { + const url = 'https://cdn.example/live/news/list.m3u'; + expect(sourceDisplayName({ title: 'list.m3u', url })).toBe('list.m3u'); + }); + + it('drops a name holding a secret of a nested URL encoded twice', () => { + const url = `https://proxy.test/list?url=${encodeURIComponent( + encodeURIComponent('http://joe:hunter2@origin.test/a.m3u') + )}`; + for (const title of ['Mine hunter2', 'Mine joe']) { + expect(sourceDisplayName({ title, url })).toBe('proxy.test'); + } + }); + it('drops a name holding a secret of a URL nested in a path segment', () => { const url = `https://proxy.test/fetch/${encodeURIComponent( 'http://origin.test/live/joe/hunter2/1.ts' diff --git a/libs/playlist/shared/util/src/lib/source-display/source-display.ts b/libs/playlist/shared/util/src/lib/source-display/source-display.ts index 0eb571f4f..843c92b98 100644 --- a/libs/playlist/shared/util/src/lib/source-display/source-display.ts +++ b/libs/playlist/shared/util/src/lib/source-display/source-display.ts @@ -1,14 +1,15 @@ -import { - isSensitiveKey, - XTREAM_CREDENTIAL_PATH_SEGMENTS, -} from '@iptvnator/shared/logging'; +import { isSensitiveKey } from '@iptvnator/shared/logging'; import { PlaylistMeta, playlistDisplayLabel, playlistNameHasCredentials, resolvePlaylistSourceKind, } from '@iptvnator/shared/interfaces'; -import { stripUrlCredentials } from './strip-url-credentials'; +import { + isXtreamAccountPath, + stripUrlCredentials, + unwrapNestedValue, +} from './strip-url-credentials'; export type SourceDisplayFields = Partial< Pick< @@ -94,7 +95,11 @@ function collectUrlSecrets( if (depth > 0) { secrets.push(safeDecode(url.username)); } - const visitNested = (value: string): void => { + const visitNested = (raw: string): void => { + const value = unwrapNestedValue(raw); + if (value === null) { + return; + } const nested = ABSOLUTE_URL.test(value.trim()) ? parseUrl(value) : null; if (nested) { collectUrlSecrets(nested, secrets, depth + 1); @@ -113,14 +118,12 @@ function collectUrlSecrets( } visitNested(value); }); - // An Xtream stream path carries the account: `/live///`. + // An Xtream stream path carries the account: + // `/live///`. // A proxy may carry a whole URL in one segment. const segments = url.pathname.split('/').map(safeDecode); segments.forEach((segment, index) => { - if ( - XTREAM_CREDENTIAL_PATH_SEGMENTS.has(segment.toLowerCase()) && - index + 2 < segments.length - ) { + if (isXtreamAccountPath(segments, index)) { secrets.push(segments[index + 2]); if (depth > 0) { secrets.push(segments[index + 1]); diff --git a/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.spec.ts b/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.spec.ts index f955008cf..f08b95fe4 100644 --- a/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.spec.ts +++ b/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.spec.ts @@ -83,6 +83,55 @@ describe('stripUrlCredentials', () => { expect(stripUrlCredentials(nested)).not.toMatch(/hunter2|joe/); }); + it('keeps a playlist path that only starts like an Xtream stream', () => { + // No stream follows `/news/list.m3u`: it names no account. + expect(stripUrlCredentials('https://cdn.test/live/news/list.m3u')).toBe( + 'https://cdn.test/live/news/list.m3u' + ); + expect( + stripUrlCredentials('https://cdn.test/live/joe/hunter2/list.m3u') + ).toBe('https://cdn.test/live/list.m3u'); + }); + + it('cleans a nested URL encoded more than once', () => { + const origin = 'https://joe:hunter2@origin.test/a.m3u?token=tk98765'; + for (const rounds of [1, 2, 3]) { + let value = origin; + for (let round = 0; round < rounds; round += 1) { + value = encodeURIComponent(value); + } + const shown = stripUrlCredentials( + `https://proxy.test/list?url=${value}&hd=1` + ); + expect(shown).not.toMatch(/hunter2|joe|tk98765/); + expect(new URL(shown ?? '').searchParams.get('url')).toBe( + 'https://origin.test/a.m3u' + ); + } + // Still encoded after every round it decodes: not shown at all. + let deep = origin; + for (let round = 0; round < 8; round += 1) { + deep = encodeURIComponent(deep); + } + expect( + stripUrlCredentials(`https://proxy.test/list?url=${deep}&hd=1`) + ).toBe('https://proxy.test/list?hd=1'); + }); + + it('keeps a nested URL’s own encoded parameters as written', () => { + const nested = `https://origin.test/a.m3u?q=${encodeURIComponent( + 'a&b=c' + )}`; + const shown = stripUrlCredentials( + `https://proxy.test/list?url=${encodeURIComponent(nested)}` + ); + expect(new URL(shown ?? '').searchParams.get('url')).toBe(nested); + // Plain text with a stray percent sign is not an escape. + expect( + stripUrlCredentials('https://proxy.test/list?note=100%25&hd=1') + ).toBe('https://proxy.test/list?note=100%25&hd=1'); + }); + it('drops a kept part that still names a secret', () => { expect( stripUrlCredentials( diff --git a/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.ts b/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.ts index dded0fb5f..2162b9505 100644 --- a/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.ts +++ b/libs/playlist/shared/util/src/lib/source-display/strip-url-credentials.ts @@ -12,6 +12,8 @@ const SHORT_ACCOUNT_KEYS = new Set(['user', 'usr', 'pass', 'pas']); /** How deep URLs nested in URLs are cleaned; anything deeper is dropped. */ const MAX_NESTING = 4; +/** Percent-decoding rounds a nested value gets before it is classified. */ +const MAX_DECODE_ROUNDS = 4; // Linear on any input: a match starts only where a name can start (the // lookbehinds), and every name part is bounded, so a long run of letters // cannot make a scan retry from each position. @@ -21,6 +23,53 @@ const EMBEDDED_URL = /(?]+/giu; const BARE_USERINFO = /(?:^|[\s/])[^\s/:@]{1,256}:[^\s/@]{0,256}@[^\s/@]/u; const KEYED_PAIR = /(?//.ts`); a playlist + * path such as `/live/news/list.m3u` names no account. + */ +export function isXtreamAccountPath( + segments: readonly string[], + index: number +): boolean { + return ( + XTREAM_CREDENTIAL_PATH_SEGMENTS.has(segments[index].toLowerCase()) && + index + 3 < segments.length + ); +} + function isDisplaySensitiveKey(key: string): boolean { return SHORT_ACCOUNT_KEYS.has(key.toLowerCase()) || isSensitiveKey(key); } @@ -42,23 +91,26 @@ function hasSensitivePair(value: string): boolean { * when it cannot be shown safely and must be dropped. */ function sanitizeNested(value: string, depth: number): string | null { - if (depth >= MAX_NESTING) { + const decoded = unwrapNestedValue(value); + if (depth >= MAX_NESTING || decoded === null) { return null; } - const trimmed = value.trim(); + const trimmed = decoded.trim(); if (ABSOLUTE_URL.test(trimmed)) { return stripUrl(trimmed, depth + 1); } if (looksLikeSearchParams(trimmed)) { return withoutSensitiveParams(new URLSearchParams(trimmed), depth + 1); } - if (BARE_USERINFO.test(value) || hasSensitivePair(value)) { + if (BARE_USERINFO.test(decoded) || hasSensitivePair(decoded)) { return null; } - return value.replace( + const cleaned = decoded.replace( EMBEDDED_URL, (candidate) => stripUrl(candidate, depth + 1) ?? '' ); + // Nothing to clean: keep the value as it was written. + return cleaned === decoded ? value : cleaned; } function withoutSensitiveParams( @@ -98,19 +150,17 @@ function stripUrl(value: string, depth: number): string | null { url.password = ''; const segments = url.pathname.split('/'); + const decodedSegments = segments.map(decodeSegment); const kept: string[] = []; for (let index = 0; index < segments.length; index += 1) { const segment = segments[index]; - const decoded = decodeSegment(segment); + const decoded = decodedSegments[index]; const clean = sanitizeNested(decoded, depth); if (clean !== null) { kept.push(clean === decoded ? segment : encodeURIComponent(clean)); } // `/LIVE/…` and `/%6Cive/…` mark account segments as `/live/…` does. - if ( - XTREAM_CREDENTIAL_PATH_SEGMENTS.has(decoded.toLowerCase()) && - index + 2 < segments.length - ) { + if (isXtreamAccountPath(decodedSegments, index)) { index += 2; } } @@ -139,9 +189,9 @@ function stripUrl(value: string, depth: number): string | null { * a marker, this drops them: the userinfo, every query or fragment parameter * the redactor treats as sensitive (`username`, `password`, `token`, `mac`, * …), and the two account segments after `/live/`, `/movie/`, `/series/` or - * `/timeshift/` in an Xtream stream path. A URL or parameter list nested in - * a kept parameter or path segment (a proxy's `?url=…`) is cleaned the same - * way, and a part that still names a secret (`joe:hunter2@…`, `token=…`) is + * `/timeshift/` when a stream follows them. A URL or parameter list nested + * in a kept parameter or path segment (a proxy's `?url=…`, even encoded + * twice) is cleaned the same way, and a part that still names a secret (`joe:hunter2@…`, `token=…`) is * dropped. A list can then say where a source points without any part of * its credentials. * diff --git a/libs/workspace/shell/feature/src/lib/workspace-sources-filters-panel/workspace-sources-filters-panel.component.spec.ts b/libs/workspace/shell/feature/src/lib/workspace-sources-filters-panel/workspace-sources-filters-panel.component.spec.ts index 38ac80ad2..4fcbe7851 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-sources-filters-panel/workspace-sources-filters-panel.component.spec.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-sources-filters-panel/workspace-sources-filters-panel.component.spec.ts @@ -7,7 +7,10 @@ import { SourceStatusService } from '@iptvnator/portal/shared/data-access'; import { SourceRemovalService } from '@iptvnator/playlist/shared/ui'; import { SourcesViewStateService } from '@iptvnator/playlist/shared/util'; import { PlaylistMeta } from '@iptvnator/shared/interfaces'; -import { WORKSPACE_SHELL_ACTIONS } from '@iptvnator/workspace/shell/util'; +import { + WORKSPACE_SHELL_ACTIONS, + WorkspaceShellContextDrawerService, +} from '@iptvnator/workspace/shell/util'; import { WorkspaceSourcesFiltersPanelComponent } from './workspace-sources-filters-panel.component'; const PLAYLISTS = [ @@ -137,7 +140,13 @@ describe('WorkspaceSourcesFiltersPanelComponent', () => { expect(rows(element)).toHaveLength(4); }); - it('adds a playlist from the header', () => { + it('adds a playlist from the header, closing the phone drawer first', () => { + const drawer = TestBed.inject(WorkspaceShellContextDrawerService); + drawer.open(); + let drawerOpenAtDialog: boolean | null = null; + openAddPlaylistDialog.mockImplementation(() => { + drawerOpenAtDialog = drawer.isOpen(); + }); const element = render(); element .querySelector( @@ -145,5 +154,7 @@ describe('WorkspaceSourcesFiltersPanelComponent', () => { ) ?.click(); expect(openAddPlaylistDialog).toHaveBeenCalled(); + // Two focus-trapped modals must not stack. + expect(drawerOpenAtDialog).toBe(false); }); }); diff --git a/libs/workspace/shell/feature/src/lib/workspace-sources-filters-panel/workspace-sources-filters-panel.component.ts b/libs/workspace/shell/feature/src/lib/workspace-sources-filters-panel/workspace-sources-filters-panel.component.ts index ae4633726..fbeaa0523 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-sources-filters-panel/workspace-sources-filters-panel.component.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-sources-filters-panel/workspace-sources-filters-panel.component.ts @@ -137,6 +137,9 @@ export class WorkspaceSourcesFiltersPanelComponent { } addPlaylist(): void { + // On a phone the drawer is a focus-trapped modal of its own: it must + // not stay open under the dialog. + this.contextDrawer?.close(); this.workspaceActions?.openAddPlaylistDialog(); } }