Conversation
#534) * fix(security): allowed-roots gap-fill; thumbnail and download confined poster_renamerr.music_source_dirs and asset_renamerr's source/music/ destination dirs feed poster_cache but were invisible to path_safety, so the picker, preview and both poster file endpoints refused their own legitimate art. get_poster_thumbnail and download_poster now route through resolve_confined (bare realpath authorized nothing); escaping paths get the file's standard 403, malformed config surfaces as 500 instead of a masked generic error. poster_cleanarr.asset_dirs joins as the same class of CHUB-operated dirs. * fix(security): no config file means no authorized roots for file access load_config returns a default ChubConfig when config.yml is absent, and get_allowed_roots still contributes CONFIG_DIR and every auto-discovered container mount — so before first boot the poster file endpoints would have served anything under /kometa, /media or /data off a config nobody wrote. resolve_confined now refuses a config that came from that branch, which covers preview, thumbnail, download, delete and the cleanup passes in one owner. The picker keeps working: get_browse_roots is deliberately unguarded so first-boot setup can still browse to those mounts. The marker is a pydantic PrivateAttr, so it can never serialize into a saved config, and a hand-built ChubConfig stays trusted. * fix(security): strip CR/LF from the refused path before logging it py/log-injection 324, introduced by the previous commit: the refusal warning interpolated request data, so a newline in the path could forge a second log line. Stripped inline — the path stays in the message for debugging, on one line. Test asserts a forged record collapses to one. * test: pin that the first-boot mount really is an allowed root Without it a 403 could come from ordinary confinement rather than the absent-config guard, so the test would keep passing if the guard were removed. Answers CodeRabbit's unverified finding on the serving tests.
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
1 participant
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.
@coderabbitai ignore
develophas drifted behindmain(releases, fixes, dependency bumps). Do not merge this PR — it reports the drift, it does not fix it.developrequires branches be up to date, andhead:maincan never satisfy that without pulling develop's extension files into main, which the branch invariant forbids. Squash or rebase would also leavemainunreachable fromdevelop, so this workflow would just open another PR next push.Sync locally instead:
Then verify
git diff main developis added extension files plusdeploy/docker/Dockerfileonly. GitHub marks this PR merged on its own once develop contains main's tip. Opened by the sync-develop workflow.