Skip to content

security: enforce multifile download path boundary#9258

Open
Ersa-tech wants to merge 5 commits into
invoke-ai:mainfrom
Ersa-tech:security/enforce-multifile-download-relative-paths
Open

security: enforce multifile download path boundary#9258
Ersa-tech wants to merge 5 commits into
invoke-ai:mainfrom
Ersa-tech:security/enforce-multifile-download-relative-paths

Conversation

@Ersa-tech

Copy link
Copy Markdown

Summary

  • Replace an assert-based multifile download path guard with an explicit runtime ValueError.
  • Resolve the destination root and candidate download path before checking containment.
  • Add regression coverage for parent-directory traversal in remote file paths.

Why

RemoteModelFile.path is documented as relative to the model root. The multifile download service should enforce that boundary at runtime, including production/optimized Python runs where assert statements are disabled. Resolving both paths also catches .. traversal that a lexical path check can miss.

Testing

  • python -m compileall invokeai/app/services/download/download_default.py tests/app/services/download/test_download_queue.py

Focused pytest could not start in this local environment because project test imports require additional runtime dependencies not installed here; after installing blake3 and picklescan, collection next failed on missing onnxruntime.

@github-actions github-actions Bot added python PRs that change python files services PRs that change app services python-tests PRs that change python tests labels May 31, 2026
@lstein lstein added the 6.14.x label Jun 3, 2026
@lstein lstein moved this to 6.14.x Theme: USER EXPERIENCE in Invoke - Community Roadmap Jun 3, 2026

@JPPhoto JPPhoto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ersa-tech Thanks for your PR! I would like you to take a look at this issue, please:

  • invokeai/app/services/download/download_default.py:183 and invokeai/app/services/download/download_default.py:686

    multifile_download() now resolves each child part path before storing it on the child DownloadJob, but leaves the parent MultiFileDownloadJob.dest unchanged. When a caller passes a relative dest, the child starts with an absolute download_path, then _mfd_started() tries to make that absolute path relative to the original relative parent path. That raises ValueError.

    A direct service call such as multifile_download(parts=[RemoteModelFile(..., path=Path("sub/file.bin"))], dest=Path("relative-dest")) reaches this path. The PR changes the child path to (dest / part.path).resolve() at invokeai/app/services/download/download_default.py:195 and stores it in DownloadJob.dest at line 200, while _mfd_started() still uses download_job.download_path.relative_to(mf_job.dest) at line 686. Since _execute_cb() catches and logs callback exceptions at invokeai/app/services/download/download_default.py:775, the child download can complete while the parent job never gets its download_path and the caller's parent on_start callback is skipped.

    I reproduced this against the PR worktree. The job completed, but the log showed ValueError: '/tmp/relative-dest-pr9258/sub/file.bin' is not in the subpath of 'relative-dest-pr9258'.

    To expose this issue, add a test that calls multifile_download() with a relative dest, waits for completion, and asserts the parent MultiFileDownloadJob.download_path is populated and the parent on_start callback ran.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.14.x python PRs that change python files python-tests PRs that change python tests services PRs that change app services

Projects

Status: 6.14.x Theme: USER EXPERIENCE

Development

Successfully merging this pull request may close these issues.

4 participants