From a48d0b913fe73bd2ff6d494ccafab3c385f161e1 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 13 Oct 2026 20:27:24 +0200 Subject: [PATCH] fix(sources): keep fragment and nested URL secrets out of source names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile bot (PR review, P1 security): an unnamed import of https://host.test/list.m3u#api_key=secret123 stored list.m3u#api_key=… as its title. isUrlDerivedName only knew names ending in a query, and sourceSecrets only read the top-level query, so the row and the toolbar showed the secret while the displayed URL hid it. A name that ends the URL with a query or a fragment is now URL-derived, and shows the host. The secrets a typed name is checked against now include sensitive fragment parameters and, as stripUrlCredentials cleans them, the credentials of URLs and parameter lists nested in parameters (nested account names included). Co-Authored-By: Claude Opus 5.5 --- .../lib/source-display/source-display.spec.ts | 21 ++++++ .../src/lib/source-display/source-display.ts | 67 +++++++++++++++---- 2 files changed, 75 insertions(+), 13 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 d4a9bda7f..61a0d678f 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 @@ -76,6 +76,27 @@ describe('sourceDisplayName', () => { expect(sourceDisplayName({ title: 'joe', url: M3U_URL })).toBe('joe'); }); + it('shows the host for a name an import took from a URL fragment', () => { + const url = 'https://host.test/list.m3u#api_key=secret123'; + expect( + sourceDisplayName({ title: 'list.m3u#api_key=secret123', url }) + ).toBe('host.test'); + // A typed name holding the fragment's secret collapses too. + expect(sourceDisplayName({ title: 'My secret123', url })).toBe( + 'host.test' + ); + }); + + it('drops a name holding a secret of a URL nested in the source URL', () => { + const url = `https://proxy.test/list?url=${encodeURIComponent( + 'http://joe:hunter2@origin.test/get.php?password=pw4567&type=m3u' + )}`; + for (const title of ['Mine hunter2', 'Mine pw4567', 'Mine joe']) { + expect(sourceDisplayName({ title, url })).toBe('proxy.test'); + } + expect(sourceDisplayName({ title: 'Mine', url })).toBe('Mine'); + }); + it('returns null for a local file without a name', () => { expect(sourceDisplayName({ title: '' })).toBeNull(); expect(sourceDisplayName({ title: '', filename: 'list.m3u' })).toBe( 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 9af934e79..590386d10 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 @@ -24,6 +24,7 @@ export type SourceDisplayFields = Partial< const SHOWN_ACCOUNT_KEYS = new Set(['username', 'login']); /** Shorter values match ordinary words, not secrets. */ const MIN_SECRET_LENGTH = 3; +const ABSOLUTE_URL = /^[a-z][a-z0-9+.-]*:\/\//i; function parseUrl(value: string | undefined): URL | null { if (!value) { @@ -67,22 +68,62 @@ export function sourceHost( return parseUrl(sourceEndpoint(playlist))?.host || null; } +/** How deep URLs nested in the source URL are searched for secrets. */ +const MAX_SECRET_NESTING = 4; + +/** + * The secrets a URL carries: its userinfo password, every sensitive query or + * fragment parameter (the source's own account name excepted), and the same + * again in any URL or parameter list nested in another parameter, whose + * account names count as secrets too. + */ +function collectUrlSecrets( + url: URL, + secrets: (string | undefined)[], + depth: number +): void { + if (depth >= MAX_SECRET_NESTING) { + return; + } + secrets.push(safeDecode(url.password)); + if (depth > 0) { + secrets.push(safeDecode(url.username)); + } + const visit = (params: URLSearchParams): void => + params.forEach((value, key) => { + const normalized = key.toLowerCase().replace(/[^a-z0-9]/g, ''); + if (isSensitiveKey(key)) { + if (depth > 0 || !SHOWN_ACCOUNT_KEYS.has(normalized)) { + secrets.push(value); + } + return; + } + const nested = ABSOLUTE_URL.test(value.trim()) + ? parseUrl(value) + : null; + if (nested) { + collectUrlSecrets(nested, secrets, depth + 1); + } else if (value.includes('=')) { + visit(new URLSearchParams(value)); + } + }); + visit(url.searchParams); + const fragment = url.hash.slice(1); + if (fragment.includes('=')) { + visit(new URLSearchParams(fragment)); + } +} + /** * Values that must never be on screen for this source: the password, the - * MAC address, and every sensitive parameter of its URL except the account - * name, in both decoded and encoded form. + * MAC address, and every secret its URL carries (`collectUrlSecrets`), in + * both decoded and encoded form. */ function sourceSecrets(playlist: SourceDisplayFields): string[] { const secrets = [playlist.password, playlist.macAddress]; const url = parseUrl(sourceEndpoint(playlist)); if (url) { - secrets.push(safeDecode(url.password)); - url.searchParams.forEach((value, key) => { - const normalized = key.toLowerCase().replace(/[^a-z0-9]/g, ''); - if (isSensitiveKey(key) && !SHOWN_ACCOUNT_KEYS.has(normalized)) { - secrets.push(value); - } - }); + collectUrlSecrets(url, secrets, 0); } const shown: string[] = []; for (const secret of secrets) { @@ -102,14 +143,14 @@ export function containsSourceSecret( /** * A name an import made from the URL rather than one the user typed: the - * URL itself, or the last path segment of a URL with a query, such as - * `get.php?username=…&password=…`. + * URL itself, or the end of a URL with a query or fragment, such as + * `get.php?username=…&password=…` or `list.m3u#api_key=…`. */ function isUrlDerivedName(name: string, url: string | undefined): boolean { - if (/^[a-z][a-z0-9+.-]*:\/\//i.test(name)) { + if (ABSOLUTE_URL.test(name)) { return true; } - return !!url && name.includes('?') && url.trim().endsWith(name); + return !!url && /[?#]/.test(name) && url.trim().endsWith(name); } /**