fix(security): address SSL fallback, ZIP slip, and unbounded download vulnerabilities

[wjc2821296948]

This PR fixes four security vulnerabilities found during a security audit:

Summary of Fixes

P0 - Silent TLS Verification Fallback in Download Helpers (io.download_file(), io.download_image_by_url(), dashboard_assets._download_package())

Risk: A network attacker could intercept HTTPS connections and serve malicious content (plugin/dashboard assets, chat media) without triggering any user-visible warning.

Fix:

• Changed default of download_file() allow_insecure_ssl_fallback=True → False
• Changed default of dashboard_assets._download_package() allow_insecure_ssl_fallback=True → False
• Removed silent ssl.CERT_NONE retry in download_image_by_url() entirely
• SSL errors now propagate to callers instead of being masked

Code References:

• astrbot/core/utils/io.py:209 - download_file() default changed
• astrbot/core/utils/io.py:283 - download_image_by_url() fallback removed
• astrbot/core/dashboard_assets.py:103 - _download_package() default changed

---

P0 - Silent TLS Verification Fallback in Misskey URL Uploads (misskey_api.upload_and_find_file())

Risk: A network attacker could intercept the connection to a user-supplied URL and serve arbitrary bytes that would then be uploaded to the Misskey instance as if they came from the legitimate URL.

Fix: Dropped the CERT_NONE retry entirely so certificate errors propagate to the caller.

Code References:

• astrbot/core/platform/sources/misskey/misskey_api.py:126 - Removed silent retry with ssl_verify=False

---

P1 - ZIP Slip (Path Traversal) in Archive Extraction (_RepoZipUpdater._extract_archive(), _PluginUpdater._extract_plugin_archive())

Risk: A crafted plugin or update archive could include members like ../../<outside path>/evil.py, which zipfile writes outside target_dir during extractall. The later commonpath() check rejects the move but the file is already on disk.

Fix: Implemented _extract_zip_safely() helper that validates each member's destination against target_dir using os.path.commonpath before delegating to ZipFile.extract(). Handles cross-drive ValueError on Windows.

Code References:

• astrbot/core/zip_updater.py:20 - New _extract_zip_safely() helper
• astrbot/core/zip_updater.py:345 - _extract_archive() uses helper
• astrbot/core/star/updater.py:463 - _extract_plugin_archive() uses helper

---

P1 - Unbounded Streamed Download Size (Disk-Fill DoS) in io.download_file()

Risk: A malicious or misconfigured remote could advertise a huge Content-Length or stream unlimited bytes, exhausting free disk space before any other validation runs.

Fix: Added _DOWNLOAD_MAX_BYTES = 2 GiB cap with:

• Pre-check on Content-Length header before writing any bytes
• Streaming accumulation size check that aborts as soon as running total crosses cap
• Partial file cleanup when size limit exceeded

Code References:

• astrbot/core/utils/io.py:17 - New DownloadTooLargeError exception and _DOWNLOAD_MAX_BYTES constant
• astrbot/core/utils/io.py:226 - Content-Length pre-check
• astrbot/core/utils/io.py:242 - Streaming size check with partial file cleanup

---

Testing

• All existing tests pass (46/46 in test_updater_socks.py + test_security_fixes.py)
• Ruff linting and formatting pass
• Test fakes updated to support new _extract_zip_safely() interface

Co-authored-by

All commits include Co-authored-by: cgsdn <chaogeshuodiannao@users.noreply.github.com>

Summary by Sourcery

Harden network and archive handling to address multiple security issues, including unsafe TLS fallbacks, ZIP slip during extraction, and unbounded HTTP download sizes.

Bug Fixes:

• Disable insecure SSL fallback by default in generic file and dashboard package downloads so certificate failures no longer silently downgrade to insecure connections.
• Remove the non-verifying TLS retry path in Misskey URL-based uploads so certificate errors surface instead of allowing potential man-in-the-middle content injection.
• Introduce a size cap for streamed HTTP downloads that rejects oversized responses and cleans up partial files when the limit is exceeded.
• Prevent ZIP slip path traversal when extracting updater and plugin archives by validating member paths stay within the target directory.

Enhancements:

• Clarify documentation for TLS fallback parameters to emphasize the security risks of disabling certificate verification.

Tests:

• Update test fakes and existing tests to cover the new safe ZIP extraction behavior and ensure all security changes pass the test suite.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[wjc2821296948]

P0 - Silent TLS Verification Fallback in Download Helpers

Risk: A network attacker could intercept HTTPS connections and serve malicious content (plugin/dashboard assets, chat media) without triggering any user-visible warning.

Fix:

• Changed default of →
• Changed default of →
• Removed silent retry in entirely
• SSL errors now propagate to callers instead of being masked

Code References:

• • default changed
• • fallback removed
• • default changed

[wjc2821296948]

P0 - Silent TLS Verification Fallback in Download Helpers

Risk: A network attacker could intercept HTTPS connections and serve malicious content (plugin/dashboard assets, chat media) without triggering any user-visible warning.

Fix:

• Changed default of download_file() allow_insecure_ssl_fallback=True → False
• Changed default of dashboard_assets._download_package() allow_insecure_ssl_fallback=True → False
• Removed silent ssl.CERT_NONE retry in download_image_by_url() entirely
• SSL errors now propagate to callers instead of being masked

Code References:

• astrbot/core/utils/io.py:209 - download_file() default changed
• astrbot/core/utils/io.py:283 - download_image_by_url() fallback removed
• astrbot/core/dashboard_assets.py:103 - _download_package() default changed

[wjc2821296948]

P0 - Silent TLS Verification Fallback in Misskey URL Uploads

Risk: A network attacker could intercept the connection to a user-supplied URL and serve arbitrary bytes that would then be uploaded to the Misskey instance as if they came from the legitimate URL.

Fix: Dropped the CERT_NONE retry entirely so certificate errors propagate to the caller.

Code References:

• astrbot/core/platform/sources/misskey/misskey_api.py:126 - Removed silent retry with ssl_verify=False

[wjc2821296948]

P1 - ZIP Slip (Path Traversal) in Archive Extraction

Risk: A crafted plugin or update archive could include members like ../../<outside path>/evil.py, which zipfile writes outside target_dir during extractall. The later commonpath() check rejects the move but the file is already on disk.

Fix: Implemented _extract_zip_safely() helper that validates each member's destination against target_dir using os.path.commonpath before delegating to ZipFile.extract(). Handles cross-drive ValueError on Windows.

Code References:

• astrbot/core/zip_updater.py:20 - New _extract_zip_safely() helper
• astrbot/core/zip_updater.py:345 - _extract_archive() uses helper
• astrbot/core/star/updater.py:463 - _extract_plugin_archive() uses helper

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• Consider applying the same _DOWNLOAD_MAX_BYTES limiting logic (or a dedicated cap) to download_image_by_url() so image downloads cannot be used for an unbounded disk-fill DoS while other HTTP downloads are protected.
• The _DOWNLOAD_MAX_BYTES safety cap is currently hard-coded at 2 GiB; if this is likely to vary across deployments, you may want to make it configurable via settings/env so operators can tune it without code changes.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider applying the same `_DOWNLOAD_MAX_BYTES` limiting logic (or a dedicated cap) to `download_image_by_url()` so image downloads cannot be used for an unbounded disk-fill DoS while other HTTP downloads are protected.
- The `_DOWNLOAD_MAX_BYTES` safety cap is currently hard-coded at 2 GiB; if this is likely to vary across deployments, you may want to make it configurable via settings/env so operators can tune it without code changes.

## Individual Comments

### Comment 1
<location path="astrbot/core/utils/io.py" line_range="25-28" />
<code_context>

+# 2 GiB safety cap for streamed HTTP downloads. Content-Length headers above this
+# threshold cause the download to abort before any bytes are written to disk.
+_DOWNLOAD_MAX_BYTES = 2 * 1024 * 1024 * 1024
+
+
+class DownloadTooLargeError(RuntimeError):
+    """Raised when an HTTP download exceeds the configured size cap."""
+
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider reusing the download size cap for `download_image_by_url` to avoid unbounded reads.

`download_image_by_url` still calls `await resp.read()` and writes the full body without any size check, so large responses can bypass the new cap and lead to excessive memory/disk usage. To keep download limits consistent, either apply `_DOWNLOAD_MAX_BYTES` there as well or have it reuse `_download_response_to_file` (or a similar streaming helper).

Suggested implementation:

```python
async def download_image_by_url(session: aiohttp.ClientSession, url: str) -> bytes:
    async with session.get(url) as resp:
        resp.raise_for_status()

        # Enforce the same maximum download size as for streamed HTTP downloads.
        total_read = 0
        chunks: list[bytes] = []

        async for chunk in resp.content.iter_chunked(64 * 1024):
            if not chunk:
                break

            total_read += len(chunk)
            if total_read > _DOWNLOAD_MAX_BYTES:
                raise DownloadTooLargeError(
                    f"image download exceeded size cap of {_DOWNLOAD_MAX_BYTES} bytes"
                )

            chunks.append(chunk)

        return b"{}".join(chunks)

```

I assumed `download_image_by_url` currently does a plain `await resp.read()`. If its signature or body differ, apply the same pattern:

1. Replace `await resp.read()` with an async loop over `resp.content.iter_chunked(...)`.
2. Track `total_read` and raise `DownloadTooLargeError` when it exceeds `_DOWNLOAD_MAX_BYTES`.
3. If you already have a helper like `_download_response_to_file(resp, path)` that enforces `_DOWNLOAD_MAX_BYTES`, consider refactoring `download_image_by_url` to reuse it instead of duplicating the chunked read logic (e.g. download to a temp file and then load the image from disk).
4. Ensure `DownloadTooLargeError` and `_DOWNLOAD_MAX_BYTES` are in scope in this module and, if needed, add tests that cover oversized image responses (both with and without `Content-Length` headers).
</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.}

[wjc2821296948]

P1 - Unbounded Streamed Download Size (Disk-Fill DoS)

Risk: A malicious or misconfigured remote could advertise a huge Content-Length or stream unlimited bytes, exhausting free disk space before any other validation runs.

Fix: Added _DOWNLOAD_MAX_BYTES = 2 GiB cap with:

• Pre-check on Content-Length header before writing any bytes
• Streaming accumulation size check that aborts as soon as running total crosses cap
• Partial file cleanup when size limit exceeded

Code References:

• astrbot/core/utils/io.py:17 - New DownloadTooLargeError exception and _DOWNLOAD_MAX_BYTES constant
• astrbot/core/utils/io.py:226 - Content-Length pre-check
• astrbot/core/utils/io.py:242 - Streaming size check with partial file cleanup