fix(screenscraper): inject user credentials for all standard media downloads#3438
Merged
gantoine merged 5 commits intoMay 26, 2026
Merged
Conversation
Contributor
Author
|
Sorry for needing to iterate on this one last time. I think we're truly covering all media types now however. Also, I don't quite understand why this is failing but it seems to be related to PR #3437 |
…reenshot downloads Standard media fields (url_cover, url_manual, url_screenshots) were downloaded using the stored credential-less URLs, causing them to count against the anonymous IP quota instead of the user's SS account. Apply add_ss_auth_to_url() at each download call site in the scan and ROM update paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> fix(screenscraper): guard add_ss_auth_to_url against non-SS URLs Only inject ssid/sspassword into screenscraper.fr URLs to prevent leaking user credentials to third-party sources (IGDB, LaunchBox, etc.) when url_cover/url_manual/url_screenshots originate from other providers. Add tests for the non-SS no-op and empty-string edge cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> test(screenscraper): verify SS credentials injected for all media download paths - TestAddSsAuthToUrl: add guards for non-SS URLs (IGDB, LaunchBox) and empty string inputs - test_update_rom: verify ssid/sspassword appear in url_cover and url_manual args passed to get_cover/get_manual for screenscraper.fr URLs; verify IGDB URLs are NOT decorated with SS credentials - TestScanCredentialInjection: verify the scan-path ternary pattern correctly applies add_ss_auth_to_url to cover and screenshot URLs, and that a None cover URL passes through without error Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> test(screenscraper): empirical audit — every SS request carries ssid/sspassword Intercepts both HTTP clients at the transport/session level to verify that every outgoing screenscraper.fr request is decorated with the user's ssid and sspassword credentials: aiohttp (API calls via auth_middleware): - jeuInfos.php, jeuRecherche.php, ssinfraInfos.php, ssuserInfos.php httpx (media downloads via FSResourcesHandler): - get_cover → url_cover - get_manual → url_manual - get_rom_screenshots → url_screenshots (each URL) - store_media_file → extra media (fanart, bezel, etc.) Also verifies the domain guard: IGDB URLs passed through add_ss_auth_to_url are NOT decorated with SS credentials. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Spinnich
force-pushed
the
fix/screenscraper-std-media-credentials
branch
2 times, most recently
from
4ca595a to
3c2f421
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
gantoine
approved these changes
May 26, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to ensure ScreenScraper media downloads use user credentials at download time while keeping stored URLs credential-free, and includes a scan logging fix plus dependency resolution metadata updates.
Changes:
- Applies
add_ss_auth_to_url()to standard cover, manual, and screenshot downloads in scan/update paths. - Adds a ScreenScraper URL guard before injecting credentials.
- Updates scan logging to avoid detached ORM property access and adjusts uv locking configuration.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
backend/handler/metadata/ss_handler.py |
Adds domain gating to ScreenScraper credential injection helper. |
backend/endpoints/sockets/scan.py |
Injects ScreenScraper auth for standard media during scan and tightens special media handling. |
backend/endpoints/roms/__init__.py |
Injects ScreenScraper auth for standard media during ROM updates and tightens special media handling. |
backend/handler/scan_handler.py |
Uses filesystem scan metadata instead of detached ROM properties for nested file logging. |
pyproject.toml |
Adds a package-specific uv exclude-newer override for Starlette. |
uv.lock |
Updates uv exclude-newer lock metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
PR #3414 added
add_ss_auth_to_url()and wired it up for extra mediadownloads (
store_media_file— fanart, bezels, etc.), but the three standardmedia fields —
url_cover,url_manual, andurl_screenshots— were leftunguarded in both the scan socket and the ROM update endpoint. Those downloads
were hitting screenscraper.fr without
ssid/sspassword, consuming the sharedanonymous/dev quota instead of the user's quota.
ss_handler.py— adds ascreenscraper.frdomain guard toadd_ss_auth_to_urlso it is safe to call unconditionally on any URL;non-SS URLs (IGDB, LaunchBox, etc.) are returned unchanged, preventing
credential leakage to third-party servers
endpoints/sockets/scan.py— appliesadd_ss_auth_to_urltourl_cover,url_manual, andurl_screenshotsin_identify_romendpoints/roms/__init__.py— same at all three call sites inupdate_romhandler/scan_handler.py— fixes aDetachedInstanceErrorcaused byaccessing the deferred
multi_filecolumn_property on a detached Rominstance; replaces
rom.has_nested_single_file or rom.has_multiple_fileswith
fs_rom["nested"], which carries the same meaning and is alwayspopulated from the filesystem scan
Checklist