fix(security): remove SSL verification downgrade fallback

[Qixuan112]

Issue #9446 - SSL Verification Downgrade Vulnerability Fix

🔐 Security Fix

This PR fixes a critical security vulnerability where download_image_by_url() and download_file()
would silently downgrade SSL certificate verification to CERT_NONE on SSL errors, allowing
man-in-the-middle (MITM) attacks.

Vulnerability Details

Before Fix:

• When SSL certificate verification failed, the code would catch the exception and retry with ssl.CERT_NONE
• This fallback behavior was automatic and silent (only logged a warning)
• Attackers could exploit this by triggering certificate errors to force insecure connections

After Fix:

• SSL certificate verification is always enforced
• SSL errors raise exceptions immediately without fallback
• No automatic downgrade to insecure connections

🔧 Changes

Modified Functions

download_image_by_url() (lines 116-144)

• Removed: SSL fallback try-except block that would retry with CERT_NONE
• Behavior: SSL certificate errors now raise immediately instead of falling back to insecure mode
• Impact: Functions calling this must handle SSL exceptions explicitly

download_file() (lines 250-291)

• Removed: SSL fallback logic
• Changed: Default parameter value allow_insecure_ssl_fallback=False (was True)
• Updated: Parameter documentation to indicate it's deprecated and ignored
• Behavior: Always enforces SSL verification regardless of parameter value

download_dashboard() (lines 452-549)

• Updated: Parameter documentation for allow_insecure_ssl_fallback
• Documentation: Now indicates the parameter is deprecated and ignored
• Note: Parameter retained for backward compatibility but has no effect

Breaking Change ⚠️

Parameter default changed: download_file(allow_insecure_ssl_fallback=False) (was True)

Impact: Code that relied on automatic SSL verification downgrade will now raise exceptions
for certificate errors. This is intentional security hardening.

Migration Guide:

• If you encounter SSL certificate errors, do NOT disable verification
• For self-signed certificates: Add them to your system's CA certificate store
• For development/testing: Use proper certificate configuration instead of bypassing verification
• The allow_insecure_ssl_fallback parameter is now deprecated and ignored

✅ Testing

Test Coverage

• 13 new SSL verification tests in tests/unit/test_io_ssl_verification.py
• SSL error handling tests in tests/unit/test_io_download_file.py
• All existing download tests updated and passing

Test Results

• ✅ SSL errors raise immediately without retry
• ✅ HTTPS downloads with valid certificates work correctly
• ✅ HTTP (non-encrypted) URLs continue to work
• ✅ Both download_image_by_url() and download_file() enforce SSL verification
• ✅ Deprecated parameter is properly ignored

📝 Documentation Updates

Updated Documentation

1. download_file() - Parameter documentation updated to indicate deprecation
2. download_dashboard() - Parameter documentation updated to indicate deprecation
3. Test files - Include references to Issue [Bug]SSL verification fallback to CERT_NONE allows MITM content injection in download utilities #9446 with detailed explanations

Parameter Documentation

allow_insecure_ssl_fallback: Deprecated parameter, ignored. SSL certificate
    verification is always enforced for security.

🔍 Security Considerations

Why This Fix Matters

1. Prevents MITM Attacks: Ensures all HTTPS connections verify server identity
2. No Silent Failures: SSL errors are visible and must be handled explicitly
3. Defense in Depth: Removes automatic fallback to insecure mode
4. Compliance: Aligns with security best practices and standards

Affected Code Paths

• Dashboard downloads from official repositories
• Plugin downloads
• Image downloads from external URLs
• Any code using download_file() or download_image_by_url()

📋 Checklist

• Security vulnerability fixed
• Breaking change documented
• All tests passing (13 new SSL tests added)
• Documentation updated for affected functions
• Migration guide provided
• No new dependencies introduced
• Code reviewed for security implications

🔗 Related

• Issue: [Bug]SSL verification fallback to CERT_NONE allows MITM content injection in download utilities #9446
• Related commits: 218e88755 (earlier SSL error handling work)
• Security: High priority - MITM vulnerability

Summary by Sourcery

Enforce strict SSL certificate verification for all download utilities and remove insecure fallback behavior that silently downgraded verification on errors.

Bug Fixes:

• Fix SSL verification downgrade vulnerability in download_image_by_url and download_file that allowed insecure CERT_NONE fallbacks on certificate errors.

Enhancements:

• Deprecate and ignore the allow_insecure_ssl_fallback parameter in download_file and download_dashboard while keeping the signature for backward compatibility.

Documentation:

• Add PR_DESCRIPTION_9446 security note and update parameter documentation to state that SSL verification is always enforced and the insecure fallback option is deprecated.

Tests:

• Add a dedicated SSL verification test suite and update existing download_file tests to assert that SSL and certificate errors are raised immediately without insecure retries.

[dosubot]

📄 Knowledge review

Dosu skipped reviewing this PR because your organization has used its 200 included credits for the month. Your usage will reset on 2026-08-01. To have Dosu review this PR before then, ask your organization admin to upgrade to a pro account.

---

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• Since allow_insecure_ssl_fallback is now deprecated and ignored, consider emitting a runtime deprecation warning when it is explicitly passed to make this change more visible to existing callers.
• With the SSL fallback removed, you might want to log SSL-related connection failures at the call site (or inside these helpers) so that operational debugging still has clear visibility into certificate issues without silently relying on exceptions alone.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Since `allow_insecure_ssl_fallback` is now deprecated and ignored, consider emitting a runtime deprecation warning when it is explicitly passed to make this change more visible to existing callers.
- With the SSL fallback removed, you might want to log SSL-related connection failures at the call site (or inside these helpers) so that operational debugging still has clear visibility into certificate issues without silently relying on exceptions alone.

## Individual Comments

### Comment 1
<location path="astrbot/core/utils/io.py" line_range="128-137" />
<code_context>
-            cafile=certifi.where(),
-        )  # 使用 certifi 提供的 CA 证书
-        connector = aiohttp.TCPConnector(ssl=ssl_context)  # 使用 certifi 的根证书
-        async with aiohttp.ClientSession(
-            trust_env=True,
-            connector=connector,
-        ) as session:
-            if post:
-                async with session.post(url, json=post_data) as resp:
-                    if not path:
-                        return save_temp_img(await resp.read())
-                    with open(path, "wb") as f:
-                        f.write(await resp.read())
-                    return path
-            else:
-                async with session.get(url) as resp:
-                    if not path:
-                        return save_temp_img(await resp.read())
</code_context>
<issue_to_address>
**suggestion (performance):** Consider adding a timeout to image downloads to avoid hanging connections.

The updated `download_image_by_url` logic makes HTTP calls without a timeout, so a slow or stalled server can tie up this coroutine indefinitely. Please add an explicit timeout (similar to `download_file`) so that connections fail predictably and don’t consume resources indefinitely.

Suggested implementation:

```python
    path: str | None = None,
    timeout: float = 30.0,
) -> str:
    """下载图片, 返回 path

    :param url: 图片 URL
    :param path: 保存路径, 如果为 None 则保存到临时文件
    :param timeout: 下载超时时间(秒)
    :return: 图片文件路径
    """

```

To fully implement the timeout behavior consistently with `download_file`, you should also:

1. Locate the body of `download_image_by_url` where `aiohttp.ClientSession` is created and update it to use an explicit timeout, for example:

   - Add/ensure the import:
   ```python
   import aiohttp
   ```
   and if not already present:
   ```python
   from aiohttp import ClientTimeout
   ```

   - Wrap the session with a timeout derived from the new argument:
   ```python
   client_timeout = ClientTimeout(total=timeout)
   async with aiohttp.ClientSession(
       trust_env=True,
       connector=connector,
       timeout=client_timeout,
   ) as session:
   ```

2. If `download_file` uses a shared configuration or constant for timeouts (e.g. `DEFAULT_HTTP_TIMEOUT`), consider:
   - Replacing the literal `30.0` in the function signature with that constant.
   - Passing the same timeout value when constructing `ClientTimeout` so both helpers behave consistently.

3. Wherever `download_image_by_url` is called, verify whether a custom timeout should be passed or if the default is sufficient; adjust calls as needed to avoid type errors (new `timeout` parameter).
</issue_to_address>

### Comment 2
<location path="astrbot/core/utils/io.py" line_range="264" />
<code_context>
 async def download_file(
</code_context>
<issue_to_address>
**suggestion:** The deprecated `allow_insecure_ssl_fallback` parameter is now ignored; consider making this clearer in behavior or removing it.

The parameter is still present in the signature and its default is now `False`, which may incorrectly imply it affects behavior. Consider either removing it in a breaking change, or keeping it but emitting a warning/log when `True` is passed so the deprecation is clear at runtime.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}