[StevenLi-phoenix]

Refs #3302

This is the v4 (manager-v4) half. The v3 handler on main gets the same fix in #3404.

Problem

The legacy GET /v2/manager/notice handler downloads the GitHub wiki News page and shows its text and links in the legacy Manager dialog. It built its connector with aiohttp.TCPConnector(verify_ssl=False). Certificate verification was therefore off for every user, whatever the bypass_ssl setting said. Anyone who can intercept the connection can present any certificate for github.com and choose the notice text and links. The sanitizer from #3297 limits the markup, but it cannot authenticate the content.

Also, a connection error made the handler raise, so the notice area showed aiohttp's "500 Internal Server Error" page text.

Fix

• Build the connector with TCPConnector(verify_ssl=not manager_util.bypass_ssl). This is the expression common/manager_util.get_data already uses. Verification is on by default. bypass_ssl = True in config.ini still turns it off. trust_env=True is unchanged, so HTTP(S)_PROXY and SSL_CERT_FILE keep working.
• Catch aiohttp.ClientError and asyncio.TimeoutError around the request. The handler logs a warning with the cause and returns the existing Unable to retrieve Notice response (status 200), the same response it already gives for a non-200 reply.

I used verify_ssl= instead of ssl= on purpose. On aiohttp 3.8.x, TCPConnector(ssl=True) gives an unverified context. I checked this with the new test: with ssl=not manager_util.bypass_ssl, the untrusted-certificate case fails on aiohttp 3.8.6 and passes on 3.14. verify_ssl=True verifies on every 3.x release and gives no deprecation warning.

Most of the diff is re-indentation for the new try:. With git diff -w the handler change is 5 lines.

Behavior change

Users behind a TLS-intercepting proxy whose CA is not trusted now see Unable to retrieve Notice instead of the notice. They get it back by trusting the proxy's CA (for example through SSL_CERT_FILE) or by setting bypass_ssl = True. Manager's other fetches through get_data already behave this way.

Scope

comfyui_manager/glob/share_3rdparty.py and comfyui_manager/legacy/share_3rdparty.py still build their connectors with TCPConnector(verify_ssl=False) (4 call sites each). I left them alone on purpose, because #3302 covers notice retrieval only. They could be a follow-up.

Tests

• New tests/test_notice_tls.py. Each case runs the real legacy get_notice in a child process, loaded through legacy_html_testutil.load_functions. The child sets HTTPS_PROXY to a local CONNECT proxy. The proxy forwards the tunnel to a local HTTPS server that presents a github.com certificate from a throwaway CA made with the openssl CLI. The cases are: an untrusted certificate is rejected, a CA trusted through SSL_CERT_FILE is accepted, bypass_ssl = True still accepts the certificate, and an unreachable upstream returns Unable to retrieve Notice. All traffic stays on 127.0.0.1. The module is skipped if there is no openssl CLI.
• tests/test_notice_html_sanitizer.py: the stub namespace now provides manager_util, because the handler reads manager_util.bypass_ssl.

Before the fix:

$ python -m pytest tests/test_notice_tls.py -q
E         aiohttp.client_exceptions.ClientProxyConnectionError: Cannot connect to host 127.0.0.1:55008 ssl:default [Connect call failed ...]
E       assert (200, '<p>Ser...ager: 4.test') == (200, 'Unable...rieve Notice')
FAILED tests/test_notice_tls.py::test_untrusted_certificate_is_rejected_before_its_content_is_used
FAILED tests/test_notice_tls.py::test_unreachable_upstream_returns_the_unavailable_notice
2 failed, 2 passed

After the fix:

$ python -m pytest tests/test_notice_tls.py tests/test_notice_html_sanitizer.py -q
28 passed
$ python -m pytest tests -q --ignore=tests/e2e --ignore=tests/playwright
3 failed, 557 passed, 28 skipped
$ ruff check .
All checks passed!

The 3 failures are tests/common/test_git_helper.py::test_single_collision, test_multi_collision and test_uuid_fallback_on_full_collision. They fail the same way on manager-v4 without this change, and they do not touch the notice code.

Other checks: the new tests also pass on Python 3.10 with aiohttp 3.8.6, and with the LibreSSL 3.3.6 openssl that ships with macOS. I did not run the e2e or Playwright suites, which need a prepared ComfyUI environment.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: b657e0b9-4d13-4d3e-8e64-601a6b0a1ab2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}