Skip to content

fix: validate WebUI dist before reuse #8785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
he-yufeng wants to merge 1 commit into
base: master
Choose a base branch
from
+56 −17
Open

fix: validate WebUI dist before reuse #8785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 29 additions & 16 deletions main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,25 +79,38 @@ async def check_dashboard_files(webui_dir: str | None = None):

data_dist_path = os.path.join(get_astrbot_data_path(), "dist")
if os.path.exists(data_dist_path):
v = await get_dashboard_version()
if should_use_bundled_dashboard_dist(data_dist_path, VERSION):
bundled_dist = get_bundled_dashboard_dist_path()
logger.info(
"Using bundled WebUI because data/dist is older than core version v%s.",
VERSION,
index_html = os.path.join(data_dist_path, "index.html")
assets_dir = os.path.join(data_dist_path, "assets")
version_file = os.path.join(assets_dir, "version")
if not (
os.path.isfile(index_html)
and os.path.isdir(assets_dir)
and os.path.isfile(version_file)
):
logger.warning(
"WebUI directory is incomplete: %s. Downloading a fresh copy.",
data_dist_path,
)
Comment on lines +90 to 93

@gemini-code-assist gemini-code-assist Bot Jun 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If the WebUI directory is incomplete or corrupted, simply logging a warning and proceeding to download might lead to extraction conflicts (e.g., if data/dist is actually a file instead of a directory, or if there are leftover orphaned files from a previous failed installation). Cleaning up the incomplete path before downloading ensures a clean and reliable extraction state.

Suggested change
logger.warning(
"WebUI directory is incomplete: %s. Downloading a fresh copy.",
data_dist_path,
)
logger.warning(
"WebUI directory is incomplete: %s. Downloading a fresh copy.",
data_dist_path,
)
import shutil
if os.path.isdir(data_dist_path):
shutil.rmtree(data_dist_path, ignore_errors=True)
else:
try:
os.remove(data_dist_path)
except Exception:
pass

return str(bundled_dist)
if v is not None:
# 存在文件
if v == f"v{VERSION}":
logger.info("WebUI is up to date.")
else:
logger.warning(
"WebUI version mismatch: %s, expected v%s.",
v,
else:
v = await get_dashboard_version()
if should_use_bundled_dashboard_dist(data_dist_path, VERSION):
bundled_dist = get_bundled_dashboard_dist_path()
logger.info(
"Using bundled WebUI because data/dist is older than core version v%s.",
VERSION,
)
return data_dist_path
return str(bundled_dist)
if v is not None:
# 存在文件
if v == f"v{VERSION}":
logger.info("WebUI is up to date.")
else:
logger.warning(
"WebUI version mismatch: %s, expected v%s.",
v,
VERSION,
)
return data_dist_path

logger.info(
"Downloading WebUI. If it fails, download dist.zip from https://github.com/AstrBotDevs/AstrBot/releases/latest and extract dist to data/.",
Expand Down
28 changes: 27 additions & 1 deletion tests/test_main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import pytest

from astrbot.core.utils.io import should_use_bundled_dashboard_dist
from main import check_dashboard_files, check_env
from main import VERSION, check_dashboard_files, check_env


class _version_info:
Expand Down Expand Up @@ -158,6 +158,8 @@ async def test_check_dashboard_files_exists_and_version_match(monkeypatch):
"""Tests that dashboard is not downloaded when it exists and version matches."""
# Mock os.path.exists to return True
monkeypatch.setattr(os.path, "exists", lambda x: True)
monkeypatch.setattr(os.path, "isfile", lambda x: True)
monkeypatch.setattr(os.path, "isdir", lambda x: True)
Comment on lines +161 to +162

@sourcery-ai sourcery-ai Bot Jun 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Avoid globally returning True for os.path.isfile/isdir in these tests to better reflect real filesystem state

In test_check_dashboard_files_exists_and_version_match and test_check_dashboard_files_exists_but_version_mismatch, os.path.isfile/os.path.isdir are patched to always return True, which can hide issues if more path checks are added later. Instead, either:

  1. Build a minimal dist structure under tmp_path (as in test_check_dashboard_files_uses_bundled_dist_when_data_dist_is_stale) and avoid patching isfile/isdir, or
  2. Patch them to return True only for the specific data/dist/index.html, data/dist/assets, and data/dist/assets/version paths, delegating to the real implementation otherwise.

This will keep the tests aligned with the actual filesystem checks and reduce false positives as the implementation evolves.

Suggested implementation:

    orig_isfile = os.path.isfile

    def _test_isfile(path):
        path_str = os.fspath(path)
        if path_str.endswith(os.path.join("data", "dist", "index.html")):
            return True
        if path_str.endswith(os.path.join("data", "dist", "assets", "version")):
            return True
        return orig_isfile(path)

    monkeypatch.setattr(os.path, "isfile", _test_isfile)
    orig_isdir = os.path.isdir

    def _test_isdir(path):
        path_str = os.fspath(path)
        if path_str.endswith(os.path.join("data", "dist", "assets")):
            return True
        return orig_isdir(path)

    monkeypatch.setattr(os.path, "isdir", _test_isdir)

Comment on lines +161 to +162

@gemini-code-assist gemini-code-assist Bot Jun 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Globally mocking os.path.isfile and os.path.isdir to always return True can cause unexpected side effects in other parts of the test suite or standard library calls. It is safer to restrict the mock to return True only for paths related to the dashboard distribution.

Suggested change
monkeypatch.setattr(os.path, "isfile", lambda x: True)
monkeypatch.setattr(os.path, "isdir", lambda x: True)
monkeypatch.setattr(os.path, "isfile", lambda x: "dist" in str(x))
monkeypatch.setattr(os.path, "isdir", lambda x: "dist" in str(x))


# Mock get_dashboard_version to return the current version
with mock.patch("main.get_dashboard_version") as mock_get_version:
Expand All @@ -176,6 +178,8 @@ async def test_check_dashboard_files_exists_and_version_match(monkeypatch):
async def test_check_dashboard_files_exists_but_version_mismatch(monkeypatch):
"""Tests that a warning is logged when dashboard version mismatches."""
monkeypatch.setattr(os.path, "exists", lambda x: True)
monkeypatch.setattr(os.path, "isfile", lambda x: True)
monkeypatch.setattr(os.path, "isdir", lambda x: True)
Comment on lines +181 to +182

@gemini-code-assist gemini-code-assist Bot Jun 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Globally mocking os.path.isfile and os.path.isdir to always return True can cause unexpected side effects in other parts of the test suite or standard library calls. It is safer to restrict the mock to return True only for paths related to the dashboard distribution.

Suggested change
monkeypatch.setattr(os.path, "isfile", lambda x: True)
monkeypatch.setattr(os.path, "isdir", lambda x: True)
monkeypatch.setattr(os.path, "isfile", lambda x: "dist" in str(x))
monkeypatch.setattr(os.path, "isdir", lambda x: "dist" in str(x))


with mock.patch(
"main.get_dashboard_version", mock.AsyncMock(return_value="v0.0.1")
Expand Down Expand Up @@ -225,6 +229,9 @@ async def test_check_dashboard_files_uses_bundled_dist_when_data_dist_is_stale(
data_dist = data_dir / "dist"
bundled_dist = tmp_path / "bundled-dist"
data_dist.mkdir(parents=True)
(data_dist / "index.html").write_text("", encoding="utf-8")
(data_dist / "assets").mkdir()
(data_dist / "assets" / "version").write_text("v0.0.1", encoding="utf-8")
bundled_dist.mkdir()

with mock.patch("main.get_astrbot_data_path", return_value=str(data_dir)):
Expand All @@ -245,6 +252,25 @@ async def test_check_dashboard_files_uses_bundled_dist_when_data_dist_is_stale(
mock_download.assert_not_called()


@pytest.mark.asyncio
async def test_check_dashboard_files_redownloads_incomplete_data_dist(tmp_path):
"""Tests that a partial data/dist does not skip dashboard download."""
data_dir = tmp_path / "data"
data_dist = data_dir / "dist"
data_dist.mkdir(parents=True)

with mock.patch("main.get_astrbot_data_path", return_value=str(data_dir)):
with mock.patch("main.get_dashboard_version") as mock_get_version:
with mock.patch(
"main.download_dashboard", mock.AsyncMock()
) as mock_download:
result = await check_dashboard_files()

assert result == str(data_dist)
mock_get_version.assert_not_called()
mock_download.assert_awaited_once_with(version=f"v{VERSION}", latest=False)


@pytest.mark.asyncio
async def test_check_dashboard_files_with_webui_dir_arg(monkeypatch):
"""Tests that providing a valid webui_dir skips all checks."""
Expand Down
Loading