From a4e4bcd88039f38b5944898c16e71ca9e7d82d89 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 13 Oct 2026 20:24:47 +0200 Subject: [PATCH] fix(sources): clean credentials nested in kept URL parts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex bot (PR review, P1): stripUrlCredentials copied every non-sensitive parameter value unchanged, so a URL nested in one (a proxy's ?url=http%3A%2F%2Fjoe%3Ahunter2%40origin.test%2F…) kept its userinfo and password and reached the Sources row through sourceDisplayUrl. Kept query and fragment values and path segments are now cleaned recursively, up to four levels: a nested URL loses its credentials the same way, a nested parameter list loses its sensitive keys, and a part that still names a secret (joe:hunter2@host, token=…) is dropped. Deeper nesting is dropped rather than shown. Co-Authored-By: Claude Opus 5.5 --- .../src/lib/strip-url-credentials.spec.ts | 56 +++++++++ .../logging/src/lib/strip-url-credentials.ts | 112 +++++++++++++++--- 2 files changed, 149 insertions(+), 19 deletions(-) diff --git a/libs/shared/logging/src/lib/strip-url-credentials.spec.ts b/libs/shared/logging/src/lib/strip-url-credentials.spec.ts index f768542ac..0fb45e2a1 100644 --- a/libs/shared/logging/src/lib/strip-url-credentials.spec.ts +++ b/libs/shared/logging/src/lib/strip-url-credentials.spec.ts @@ -40,6 +40,62 @@ describe('stripUrlCredentials', () => { } }); + it('cleans a credential-bearing URL nested in a kept parameter', () => { + const proxied = `https://proxy.test/list?url=${encodeURIComponent( + 'http://joe:hunter2@origin.test/get.php?username=joe&password=hunter2&type=m3u' + )}&quality=hd`; + const shown = stripUrlCredentials(proxied); + expect(shown).not.toMatch(/hunter2|joe/); + expect(new URL(shown ?? '').searchParams.get('url')).toBe( + 'http://origin.test/get.php?type=m3u' + ); + expect(new URL(shown ?? '').searchParams.get('quality')).toBe('hd'); + }); + + it('cleans nested parameter lists, fragments and path segments', () => { + expect( + stripUrlCredentials( + `https://proxy.test/list?next=${encodeURIComponent( + 'token=abc&page=2' + )}` + ) + ).toBe('https://proxy.test/list?next=page%3D2'); + expect( + stripUrlCredentials( + `https://proxy.test/list#src=${encodeURIComponent( + 'http://joe:hunter2@origin.test/a.m3u' + )}` + ) + ).not.toMatch(/hunter2/); + const inPath = `https://proxy.test/fetch/${encodeURIComponent( + 'http://joe:hunter2@origin.test/a.m3u' + )}`; + expect(stripUrlCredentials(inPath)).not.toMatch(/hunter2|joe/); + }); + + it('drops a kept part that still names a secret', () => { + expect( + stripUrlCredentials( + 'https://proxy.test/list?via=joe:hunter2@origin.test&hd=1' + ) + ).toBe('https://proxy.test/list?hd=1'); + expect( + stripUrlCredentials( + `https://proxy.test/list?note=${encodeURIComponent( + 'token: abc' + )}&hd=1` + ) + ).toBe('https://proxy.test/list?hd=1'); + }); + + it('drops URLs nested deeper than it cleans', () => { + let url = 'http://joe:hunter2@origin.test/a.m3u'; + for (let depth = 0; depth < 6; depth += 1) { + url = `https://hop${depth}.test/?u=${encodeURIComponent(url)}`; + } + expect(stripUrlCredentials(url)).not.toMatch(/hunter2/); + }); + it('returns null for text that is not an absolute URL', () => { expect(stripUrlCredentials('get.php?username=joe')).toBeNull(); expect(stripUrlCredentials('')).toBeNull(); diff --git a/libs/shared/logging/src/lib/strip-url-credentials.ts b/libs/shared/logging/src/lib/strip-url-credentials.ts index f2f51db81..a759d84da 100644 --- a/libs/shared/logging/src/lib/strip-url-credentials.ts +++ b/libs/shared/logging/src/lib/strip-url-credentials.ts @@ -10,32 +10,80 @@ import { */ 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; +const ABSOLUTE_URL = /^[a-z][a-z0-9+.-]*:\/\//iu; +const EMBEDDED_URL = /[a-z][a-z0-9+.-]*:\/\/[^\s"'<>]+/giu; +/** `joe:hunter2@origin.test`: an authority's userinfo without a scheme. */ +const BARE_USERINFO = /(?:^|[\s/])[^\s/:@]+:[^\s/@]*@[^\s/@]/u; +const KEYED_PAIR = /\b([a-z][a-z0-9_.-]*)\s*[=:]/giu; + function isDisplaySensitiveKey(key: string): boolean { return SHORT_ACCOUNT_KEYS.has(key.toLowerCase()) || isSensitiveKey(key); } -function withoutSensitiveParams(params: URLSearchParams): string { +/** Free text that names a secret, such as `token=…` or `password: …`. */ +function hasSensitivePair(value: string): boolean { + const pairs = new RegExp(KEYED_PAIR.source, KEYED_PAIR.flags); + for (let match = pairs.exec(value); match; match = pairs.exec(value)) { + if (isDisplaySensitiveKey(match[1])) { + return true; + } + } + return false; +} + +/** + * A kept URL part (a parameter value or a path segment) with any URL, + * parameter list or credential nested in it cleaned the same way, or null + * when it cannot be shown safely and must be dropped. + */ +function sanitizeNested(value: string, depth: number): string | null { + if (depth >= MAX_NESTING) { + return null; + } + const trimmed = value.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)) { + return null; + } + return value.replace( + EMBEDDED_URL, + (candidate) => stripUrl(candidate, depth + 1) ?? '' + ); +} + +function withoutSensitiveParams( + params: URLSearchParams, + depth: number +): string { const kept = new URLSearchParams(); params.forEach((value, key) => { - if (!isDisplaySensitiveKey(key)) { - kept.append(key, value); + if (isDisplaySensitiveKey(key)) { + return; + } + const clean = sanitizeNested(value, depth); + if (clean !== null) { + kept.append(key, clean); } }); return kept.toString(); } -/** - * A URL safe to show on screen. Where the log redactor replaces secrets with - * 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 list can then say where a source - * points without any part of its credentials. - * - * Returns null for text that is not an absolute URL, so a caller never - * renders the unparsed original by mistake. - */ -export function stripUrlCredentials(value: string): string | null { +function decodeSegment(segment: string): string { + try { + return decodeURIComponent(segment); + } catch { + return segment; + } +} + +function stripUrl(value: string, depth: number): string | null { let url: URL; try { url = new URL(value.trim()); @@ -49,9 +97,14 @@ export function stripUrlCredentials(value: string): string | null { const segments = url.pathname.split('/'); const kept: string[] = []; for (let index = 0; index < segments.length; index += 1) { - kept.push(segments[index]); + const segment = segments[index]; + const decoded = decodeSegment(segment); + const clean = sanitizeNested(decoded, depth); + if (clean !== null) { + kept.push(clean === decoded ? segment : encodeURIComponent(clean)); + } if ( - XTREAM_CREDENTIAL_PATH_SEGMENTS.has(segments[index]) && + XTREAM_CREDENTIAL_PATH_SEGMENTS.has(segment) && index + 2 < segments.length ) { index += 2; @@ -59,14 +112,35 @@ export function stripUrlCredentials(value: string): string | null { } url.pathname = kept.join('/'); - const search = withoutSensitiveParams(url.searchParams); + const search = withoutSensitiveParams(url.searchParams, depth); url.search = search ? `?${search}` : ''; const fragment = url.hash.slice(1); if (looksLikeSearchParams(fragment)) { - const hash = withoutSensitiveParams(new URLSearchParams(fragment)); + const hash = withoutSensitiveParams( + new URLSearchParams(fragment), + depth + ); url.hash = hash ? `#${hash}` : ''; } return url.toString(); } + +/** + * A URL safe to show on screen. Where the log redactor replaces secrets with + * 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 + * dropped. A list can then say where a source points without any part of + * its credentials. + * + * Returns null for text that is not an absolute URL, so a caller never + * renders the unparsed original by mistake. + */ +export function stripUrlCredentials(value: string): string | null { + return stripUrl(value, 0); +}