Skip to content

fix(ssrf): guard explicit proxy connections - #9554

Open
wunianze666-netizen wants to merge 3 commits into
invoke-ai:mainfrom
wunianze666-netizen:codex/fix-download-proxy-ssrf
Open

wunianze666-netizen wants to merge 3 commits into
invoke-ai:mainfrom
wunianze666-netizen:codex/fix-download-proxy-ssrf

Conversation

@wunianze666-netizen

Copy link
Copy Markdown
Contributor

Summary

  • install InvokeAI's guarded HTTP/HTTPS connection pools on Requests proxy managers
  • prevent an explicit download_proxy from bypassing the connected-peer SSRF check
  • add a regression test using a loopback proxy

Related Issues / Discussions

Refs #9493 (download-queue SSRF follow-up, item 2).

QA Instructions

  • Verified the pre-fix behavior locally: an explicit loopback proxy returned HTTP 200 despite the guarded session.
  • Verified the new behavior locally: the same request is rejected with UnsafeDownloadURLException.
  • python -m ruff check invokeai/app/util/ssrf.py tests/app/util/test_ssrf.py
  • python -m ruff format --check invokeai/app/util/ssrf.py tests/app/util/test_ssrf.py
  • Python compilation and git diff --check pass.

Merge Plan

This is intentionally limited to the proxy-manager path described in #9493. It does not change the explicit private-download opt-out or the separate injected-session follow-up.

Checklist

  • I have performed a self-review of my own code
  • I have added or updated focused tests
  • I have checked the diff for secrets and unrelated files

@github-actions github-actions Bot added python PRs that change python files python-tests PRs that change python tests labels Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Synced this branch with current main (ef832d1aa) in merge commit 1b66e1299. The PR diff remains limited to ssrf.py and its focused test. Revalidation on the updated tree: the explicit-loopback-proxy regression passed; 68 additional SSRF tests passed; Ruff check/format, Python compilation, and git diff --check passed. One unrelated host-specific test (test_no_warning_without_a_proxy) sees the Windows system proxy registry and fails even after proxy environment variables are cleared; I reproduced the identical failure on a clean detached origin/main, so it is not introduced by this PR. Fresh full CI is now running.

@wunianze666-netizen

Copy link
Copy Markdown
Contributor Author

Added HTTPS CONNECT-tunnel coverage in e92a43935 so the regression now proves the connected-peer guard rejects a loopback explicit proxy for both plain HTTP requests and HTTPS tunnels. Focused proxy cases: 2/2 passed. The broader SSRF file passed 69 tests with only the previously documented host-dependent system-proxy case excluded; Ruff check/format and git diff --check passed.

@wunianze666-netizen

wunianze666-netizen commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

The latest Windows 3.11 matrix failed in the unrelated PID decoder test TestNameEvidence::test_one_component_naming_two_backbones_decides_nothing (DID NOT RAISE NotAMatchError); the job otherwise completed 5,603 tests. Windows 3.12 was then cancelled by fail-fast. This PR's latest commit changes only the SSRF proxy test, and the same merged base passed both Windows matrices on the immediately preceding head, so no PID/model-manager code is in the diff.

I also reran that exact failing test locally on Windows with the repository's full dependency environment; it passed (1 passed in 0.27s). I attempted to rerun the failed Actions jobs, but fork authors do not have the repository permission required. Could a maintainer rerun the failed workflow when convenient?

Copy link
Copy Markdown
Contributor Author

CI follow-up for e92a43935:

  • Python 3.11/3.12 passed on Linux and macOS.
  • Windows Python 3.11 completed with 5,603 passed and one failure in tests/backend/model_manager/configs/test_pid_decoder_config.py::TestNameEvidence::test_one_component_naming_two_backbones_decides_nothing (NotAMatchError was not raised).
  • Windows Python 3.12 was cancelled after 5,514 tests had passed when the matrix failed.
  • All eight non-test workflows passed.

The failing test is outside this PR's two changed files (invokeai/app/util/ssrf.py and tests/app/util/test_ssrf.py). I also cannot rerun the upstream workflow from the fork integration (GitHub returns 403). Could a maintainer rerun the failed Python jobs? I do not see a source change in this PR that would address this unrelated model-identification failure.

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

Labels

python PRs that change python files python-tests PRs that change python tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant