Fix direct URL relative destination not resolved against DOWNLOADS_DIR#19
Merged
loganbuilt merged 1 commit intosudoStacks:mainfrom Apr 15, 2026
Conversation
_run_direct_url_with_cli received the destination from the UI as a relative path (e.g. 'Singles') but passed it directly to ensure_dir() and finalize_download_artifact(), causing os.makedirs to resolve it against the process CWD (/app) instead of DOWNLOADS_DIR (/downloads). The job queue path correctly calls resolve_dir(raw, paths.single_downloads_dir) for this reason; apply the same logic here: if the destination is relative, join it against DOWNLOADS_DIR before use.
Collaborator
|
Thanks for catching this!! The bug is real, but before merging I want this path to use the same resolution logic as the queue path, including rejecting destinations that escape DOWNLOADS_DIR via inputs like ../.... Can you update this to use the shared resolve_dir(...) helper and add a regression test for both a relative destination like Singles and an escape case? I’m happy to re-review after that. I appreciate your contribution! |
loganbuilt
approved these changes
Apr 15, 2026
Collaborator
loganbuilt
left a comment
There was a problem hiding this comment.
Will merge as-is for to main and will implement the hard fix on the latest dev branch
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.
Summary
_run_direct_url_with_clireceivesdestinationfrom the UI as a relative path (e.g.Singles) but passed it directly toensure_dir()andfinalize_download_artifact(), causingos.makedirsto resolve it against the process CWD (/app) instead ofDOWNLOADS_DIR(/downloads)PermissionError: [Errno 13] Permission denied: 'Singles'; when running as root it silently writes files inside the container rather than the mounted downloads volumeresolve_dir(raw, paths.single_downloads_dir)for this reason — this fix applies the same logic to the direct URL CLI pathTest plan
single_download_folderconfigured (e.g.Singles) — file should land in$DOWNLOADS_DIR/Singles/instead of erroring$DOWNLOADS_DIR/as before