Token leak round 2: artist endpoint + playlist sync + URL-encoded redaction
The first token-leak fix scrubbed the artwork URL fixer's own log
calls. This catches three more sites that ALSO leaked tokens, plus
one upstream gap that let URL-encoded tokens slip through the
redactor.
Three sites in `web_server.py` (artist endpoint at line 8765-8773):
- "Artist image before fix: '...'" -- logged the raw image_url with
the auth token in plain form.
- "Artist image after fix: '...'" -- logged the URL-encoded form
after it had been wrapped in the image proxy
(`/api/image-proxy?url=<percent-encoded-token>`).
- "Final artist data being sent: {...}" -- dumped the entire
artist_info dict on every render, including the image_url field.
All three were dev-time debug noise. Removed entirely. The "No
artist image URL found" warning at line 8770 stays (no URL, just
the artist name).
One site in `core/discovery/sync.py:402`:
- "[PLAYLIST IMAGE] image_url=..." -- logged the playlist poster URL
during sync. Same auth-token leak risk for Plex / Jellyfin
playlists. Changed to log only `has_image=True/False`.
Upstream gap in `_redact_url_secrets`:
- The original regex only matched plain query params (`?key=value`).
When an auth-bearing URL gets wrapped inside another URL's query
string (our `/api/image-proxy?url=<encoded>` flow) the auth params
end up percent-encoded -- `%3FX-Plex-Token%3D...` -- and slipped
through.
- New second pattern catches the URL-encoded form. Both passes run
on every redact call; idempotent.
Verified manually:
/api/image-proxy?url=...%3FX-Plex-Token%3DABC...
-> /api/image-proxy?url=...%3FX-Plex-Token%3D***REDACTED***
6 artwork tests pass.
This commit is contained in:
Broque Thomas
parent
2fe1926074
commit
d9529fc801
4 changed files with 39 additions and 14 deletions
|
|
@ -397,9 +397,12 @@ def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, p
|
|||
}
|
||||
logger.info(f"Sync finished for {playlist_id} - state updated")
|
||||
|
||||
# Set playlist poster image if available (Plex, Jellyfin, Emby)
|
||||
# Set playlist poster image if available (Plex, Jellyfin, Emby).
|
||||
# Don't log the URL itself — it may carry an auth token (Plex
|
||||
# X-Plex-Token / Jellyfin X-Emby-Token / Subsonic auth) that we
|
||||
# don't want persisted to app.log.
|
||||
_synced = getattr(result, 'synced_tracks', 0)
|
||||
logger.info(f"[PLAYLIST IMAGE] image_url={playlist_image_url!r}, synced_tracks={_synced}")
|
||||
logger.info(f"[PLAYLIST IMAGE] has_image={bool(playlist_image_url)}, synced_tracks={_synced}")
|
||||
if playlist_image_url and _synced > 0:
|
||||
try:
|
||||
active_server = deps.config_manager.get_active_media_server()
|
||||
|
|
|
|||
|
|
@ -37,22 +37,43 @@ _REDACT_QUERY_KEYS = (
|
|||
'x-plex-token', 'x-emby-token', 'api_key', 'apikey',
|
||||
't', 's', 'p', 'token', 'password',
|
||||
)
|
||||
# Anchor on `?` / `&` (or string start) so short keys like `t` only
|
||||
# match at parameter boundaries — not as a substring of `format=Jpg`.
|
||||
_REDACT_KEYS_ALT = '|'.join(re.escape(k) for k in _REDACT_QUERY_KEYS)
|
||||
# Plain form: `?key=value` or `&key=value`. Anchored on `?` / `&` (or
|
||||
# string start) so short keys like `t` only match at parameter
|
||||
# boundaries — not as a substring of `format=Jpg`.
|
||||
_REDACT_QUERY_RE = re.compile(
|
||||
r'(?i)(?P<lead>^|[?&])(?P<key>' + '|'.join(re.escape(k) for k in _REDACT_QUERY_KEYS) + r')=([^&\s]+)'
|
||||
r'(?i)(?P<lead>^|[?&])(?P<key>' + _REDACT_KEYS_ALT + r')=(?P<val>[^&\s]+)'
|
||||
)
|
||||
# URL-encoded form: `%3Fkey%3Dvalue` or `%26key%3Dvalue`. The image
|
||||
# proxy wraps the original URL via `?url=<encoded>`, so the auth
|
||||
# params end up encoded inside another URL. Without this second pass
|
||||
# the encoded form survives plain redaction and ships to logs intact.
|
||||
_REDACT_QUERY_RE_ENCODED = re.compile(
|
||||
r'(?i)(?P<lead>%3F|%26)(?P<key>' + _REDACT_KEYS_ALT + r')%3D(?P<val>[^%&\s]+?)(?=%26|&|\s|$)'
|
||||
)
|
||||
|
||||
|
||||
def _redact_url_secrets(url: str | None) -> str:
|
||||
"""Mask sensitive query parameters in a URL so the result is safe
|
||||
to log. Returns ``''`` for None/empty input. Idempotent."""
|
||||
to log. Handles both the plain form (``?token=abc``) and the URL-
|
||||
encoded form (``%3Ftoken%3Dabc``) — the latter shows up when an
|
||||
auth-bearing URL is wrapped inside another URL's query string
|
||||
(e.g. our `/api/image-proxy?url=<encoded-plex-url>` flow).
|
||||
|
||||
Returns ``''`` for None/empty input. Idempotent (safe to call on
|
||||
already-redacted strings)."""
|
||||
if not url:
|
||||
return ''
|
||||
return _REDACT_QUERY_RE.sub(
|
||||
out = str(url)
|
||||
out = _REDACT_QUERY_RE.sub(
|
||||
lambda m: f"{m.group('lead')}{m.group('key')}=***REDACTED***",
|
||||
str(url),
|
||||
out,
|
||||
)
|
||||
out = _REDACT_QUERY_RE_ENCODED.sub(
|
||||
lambda m: f"{m.group('lead')}{m.group('key')}%3D***REDACTED***",
|
||||
out,
|
||||
)
|
||||
return out
|
||||
|
||||
|
||||
def normalize_image_url(thumb_url: str | None) -> str | None:
|
||||
|
|
|
|||
|
|
@ -8761,17 +8761,17 @@ def get_artist_detail(artist_id):
|
|||
|
||||
logger.info(f"Found artist: {artist_info['name']} with {len(owned_releases['albums'])} albums")
|
||||
|
||||
# Fix artist image URL
|
||||
logger.info(f"Artist image before fix: '{artist_info.get('image_url')}'")
|
||||
# Fix artist image URL.
|
||||
# NOTE: don't log image_url or the full artist_info dict here.
|
||||
# The fixed URL embeds the media-server token (and the proxy
|
||||
# variant URL-encodes it), so logging at INFO writes the token
|
||||
# straight into app.log. Issue: tokens leaked to disk on every
|
||||
# artist-page render until this was scrubbed.
|
||||
if artist_info.get('image_url'):
|
||||
artist_info['image_url'] = fix_artist_image_url(artist_info['image_url'])
|
||||
logger.info(f"Artist image after fix: '{artist_info['image_url']}'")
|
||||
else:
|
||||
logger.warning(f"No artist image URL found for {artist_info['name']}")
|
||||
|
||||
# Debug final artist data being sent
|
||||
logger.info(f"Final artist data being sent: {artist_info}")
|
||||
|
||||
# Fix image URLs for all albums
|
||||
for album in owned_releases['albums']:
|
||||
if album.get('image_url'):
|
||||
|
|
|
|||
|
|
@ -3416,6 +3416,7 @@ const WHATS_NEW = {
|
|||
'2.5.2': [
|
||||
// --- May 13, 2026 — 2.5.2 release ---
|
||||
{ date: 'May 13, 2026 — 2.5.2 release' },
|
||||
{ title: 'Token Leak Round 2: URL-Encoded Form In Artist Endpoint + Playlist Sync', desc: 'security follow-up to the prior token-leak fix. found three sites in `web_server.py` (artist endpoint) that logged the full `image_url` and the entire artist_info dict at INFO on every artist-page render — the dict contained the `image_url` field routed through the image proxy (`/api/image-proxy?url=<encoded>`), URL-encoding the X-Plex-Token / X-Emby-Token / Subsonic auth straight into the log line. also one site in `core/discovery/sync.py` logged the playlist poster URL during sync. fixes: dropped the three artist-endpoint dev-time debug log lines entirely (before-fix, after-fix, "Final artist data being sent"). playlist-image log now logs `has_image=True/False`, not the URL. strengthened `_redact_url_secrets` with a second regex pattern that matches the URL-encoded form (`%3FX-Plex-Token%3D...`) so any future log-through-redactor catches both plain and encoded shapes. wipe your existing app.log if it captured tokens in either form, and rotate Plex / Jellyfin / Navidrome credentials.', page: 'settings' },
|
||||
{ title: 'Stop Leaking Plex / Jellyfin / Navidrome Tokens Into app.log', desc: 'security: artwork URL fixer was logging full media-server URLs (including the X-Plex-Token / X-Emby-Token / Subsonic auth params) at INFO level on every cover-art lookup. tokens piled up in app.log on disk — anyone with read access to the log file gained full read access to the user\'s media server. fix: log lines moved to DEBUG (so they don\'t persist by default) and routed through a new `_redact_url_secrets` helper that masks the values of `X-Plex-Token` / `X-Emby-Token` / `api_key` / `apikey` / Subsonic `t` / `s` / `p` / generic `token` / `password` query params. anchor regex on `?` or `&` boundary so short keys like `t` don\'t false-match inside `format=Jpg`. also dropped the noisy per-call "Plex/Jellyfin/Navidrome config - base_url: ..., token: ..." INFO lines that fired on every thumbnail. wipe your existing app.log if your config has been logged.', page: 'settings' },
|
||||
{ title: 'AcoustID + Quarantine Modal: Three Bug Fixes', desc: 'github issues #607 + #608. (1) live recordings no longer false-quarantine when AcoustID returns the live recording with a bare title — common case where venue/live annotation lives on the release entity, not the recording entity itself ("Clarity (Live at ...)" expected, AcoustID returns bare "Clarity"). new pure helper `core/matching/version_mismatch.py:is_acceptable_version_mismatch` accepts the mismatch only when one-sided live + bare AND fingerprint score >= 0.85 AND bare titles agree (>=0.7) AND artist matches (>=0.6). other version mismatches (instrumental, remix, acoustic, demo) stay strict — those have distinct fingerprints + MB always annotates them in the recording title. 23 boundary tests pin every shape; existing test_acoustid_version_mismatch suite passes unchanged. (2) audio-mismatch failure message no longer reports "identified as \'\' by \'\' (artist=100%)" when AcoustID returns multiple recordings — prior code mixed recordings[0]\'s strings (which can be empty) with best_rec\'s scores. now uses matched_title/matched_artist consistently in both the high-confidence-skip path and the final fail message. (3) quarantine modal Approve/Delete buttons no longer silently no-op when filename contains an apostrophe — id is now wrapped via `escapeHtml(JSON.stringify(id))` so quotes / backslashes / unicode all round-trip safely through the HTML attribute → JS string boundary. (4) bonus UX: quarantine entry expanded view now shows source uploader (username) + original soulseek filename when the sidecar carries that context, helping trace which uploader the bad file came from.', page: 'downloads' },
|
||||
{ title: 'Reorganize: Read Embedded Tags Instead Of Metadata API', desc: 'github issue #592: optional reorganize mode that uses each file\'s embedded tags as the canonical source of truth instead of doing a fresh metadata-source api lookup. useful for well-tagged libraries where api drift can produce inconsistent renames. zero api calls. opt-in via new "metadata mode" dropdown in the per-album reorganize modal + bulk reorganize-all modal — default stays "api" so existing pipelines are untouched. tag-mode reads via the same mutagen path the audit-trail modal uses (id3 / vorbis / mp4 all covered). honors id3-style "5/12" track and disc shapes, multi-value Artists tags, year normalization across 5 date formats, releasetype canonical tokens (album/single/ep/compilation). partial-album reorganize respects "totaldiscs" tag so disc 1 of a 2-disc set still routes into "Disc 1/" subfolder. plan items carry per-item `api_album` so each file\'s own album metadata flows through post-process, not a shared one. logic lifted to pure helper `core/library/reorganize_tag_source.py` with 49 boundary tests + 6 planner-level integration tests pinning every shape: missing essentials, multi-artist split across 9 separators, defensive paths, api-mode regression guard. 3171 tests pass — no regression.', page: 'tools' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue