Repository navigation
Conversation
Add GET /api/v1/logs/export, which packs the log files and the in-memory logs into a zip archive. The files come from StorageCleaner, so log and trace log paths configured outside data/logs are included too, and the in-memory cache is split into memory/logs.txt and memory/traces.jsonl. Add an Export Logs button to the console page and to the log and cache cleanup panel in Settings. Closes AstrBotDevs#10407
…orts Only pack regular files, so a symlink in a log directory cannot pull in a file from elsewhere and a FIFO cannot block the export. Skipped files and files that disappear while packing are listed in manifest.json with the reason, together with the AstrBot version and the in-memory log counts. Also keep same-named log files from different directories apart, and remove archives left behind by cancelled downloads.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 10 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/services/log_service.py" line_range="124" />
<code_context>
+ try:
+ with zipfile.ZipFile(archive_path, "w", zipfile.ZIP_DEFLATED) as archive:
+ for file_path in sorted(
+ StorageCleaner(self.config).collect_log_files()
+ ):
+ if file_path.is_relative_to(data_dir):
</code_context>
<issue_to_address>
**Symlink targets enter archives**
When a configured log path or one of its parent directories is a symlink, `StorageCleaner._resolve_log_path()` resolves the configured path before `export_logs()` checks it, so `lstat()` sees the target as a regular file and the archive includes its contents, potentially from outside the log locations.
Preserve the unresolved configured path and reject symlinks before resolving or archiving it.
Also at `astrbot/dashboard/services/log_service.py:139-140`.
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/services/log_service.py" line_range="139-146" />
<code_context>
+ # lstat() and the resolve() check keep symlinks from
+ # pulling in files outside the log locations.
+ if (
+ not stat.S_ISREG(file_path.lstat().st_mode)
+ or file_path.resolve() != file_path
+ ):
+ manifest["skipped"].append(
+ {"name": arcname, "reason": "not a regular file"}
+ )
+ continue
+ archive.write(file_path, arcname)
+ manifest["files"].append(arcname)
+ except OSError as exc:
</code_context>
<issue_to_address>
**Replaced paths expose or block exports**
When another process replaces a collected log path after validation but before it is opened, `archive.write()` reopens the path after `lstat()` and `resolve()` checks, so a replacement symlink exposes its target and a replacement FIFO blocks the worker.
Open each file safely without following symlinks, verify the opened file is regular, and pass that open file to the archive writer.
</issue_to_address>
### Comment 3
<location path="astrbot/dashboard/services/log_service.py" line_range="105" />
<code_context>
+ traces = [item for item in cached if item.get("type") == "trace"]
+ data_dir = Path(get_astrbot_data_path())
+ export_dir = Path(get_astrbot_temp_path()) / "log_exports"
+ export_dir.mkdir(parents=True, exist_ok=True)
+ # The response removes its archive, except when the download is cancelled.
+ for stale in export_dir.glob("*.zip"):
</code_context>
<issue_to_address>
**Temporary archive exposes private logs**
When astrBot runs with a typical permissive umask in a data directory traversable by other local users, and an export is still in progress or its download is cancelled, `export_dir.mkdir()` and `zipfile.ZipFile()` create the export directory and archive using normal process permissions, so local users who can traverse the data directory can read the archive while it is being sent or after a cancelled download. It contains in-memory chat logs and traces as well as log files.
Create the export directory with owner-only permissions and create each archive with owner-only file permissions, applying those modes even when the directory already exists.
Also at `astrbot/dashboard/services/log_service.py:122`.
</issue_to_address>
### Comment 4
<location path="astrbot/core/utils/storage_cleaner.py" line_range="161-162" />
<code_context>
+ The log files, including rotated ones of the configured log and
+ trace log paths, even when those are outside data/logs.
+ """
files = set(self._iter_files(self._data_dir / "logs"))
for log_path in self._configured_log_paths():
files.update(self._iter_log_family_files(log_path))
</code_context>
<issue_to_address>
**Disabled logging still exports disk logs**
When file logging is disabled but files from earlier logging remain under `data/logs` or at configured paths, `collect_log_files()` includes those files regardless of whether file logging is enabled, so the archive contains persisted or stale logs despite the UI and guides saying only in-memory logs are included.
Have `collect_log_files()` exclude disk log paths when file logging is disabled.
Also at `astrbot/dashboard/services/log_service.py:124`, `astrbot/dashboard/services/log_service.py:127`, `docs/en/use/webui.md:139`, `docs/zh/use/webui.md:139`.
</issue_to_address>
### Comment 5
<location path="astrbot/dashboard/services/log_service.py" line_range="122-146" />
<code_context>
+ with zipfile.ZipFile(archive_path, "w", zipfile.ZIP_DEFLATED) as archive:
</code_context>
<issue_to_address>
**Large exports exhaust temporary disk**
When logs are large or multiple export requests run concurrently on a disk with limited free space, `ZipFile` writes a full archive for every request without a size limit or concurrency bound. Large log files or simultaneous exports can fill the temp filesystem, disrupting other AstrBot writes as well as failing exports.
Bound export size and concurrency, or otherwise prevent exports from consuming the remaining application disk space.
</issue_to_address>
### Comment 6
<location path="astrbot/dashboard/services/log_service.py" line_range="146" />
<code_context>
+ {"name": arcname, "reason": "not a regular file"}
+ )
+ continue
+ archive.write(file_path, arcname)
+ manifest["files"].append(arcname)
+ except OSError as exc:
</code_context>
<issue_to_address>
**Cleanup truncates exported logs**
When log cleanup overlaps an export reading its source files, `archive.write()` reads live log paths, so cleanup can truncate an active file while it is being read and the archive can contain an empty or partial log listed as packed.
Coordinate cleanup with exports or copy the source files to a stable snapshot before packing them.
</issue_to_address>
### Comment 7
<location path="astrbot/dashboard/services/log_service.py" line_range="104" />
<code_context>
+ ]
+ traces = [item for item in cached if item.get("type") == "trace"]
+ data_dir = Path(get_astrbot_data_path())
+ export_dir = Path(get_astrbot_temp_path()) / "log_exports"
+ export_dir.mkdir(parents=True, exist_ok=True)
+ # The response removes its archive, except when the download is cancelled.
</code_context>
<issue_to_address>
**Cache cleanup removes export archives**
When cache cleanup overlaps an export or its download, `StorageCleaner._collect_cache_files()` includes the temp directory, so cleanup can unlink the ZIP before `FileResponse` opens it or while it is being sent, causing the download to fail or be interrupted.
Exclude active log exports from cache cleanup or store them in a location that cleanup does not remove.
</issue_to_address>
### Comment 8
<location path="astrbot/dashboard/services/log_service.py" line_range="127" />
<code_context>
+ StorageCleaner(self.config).collect_log_files()
+ ):
+ if file_path.is_relative_to(data_dir):
+ arcname = file_path.relative_to(data_dir).as_posix()
+ else:
+ arcname = f"external/{file_path.name}"
</code_context>
<issue_to_address>
**Archive entries have duplicate names**
When a source path maps to a name already used by another file or by generated archive content, the data-directory branch does not check `used_names`, and generated entries also use fixed names, so the ZIP can contain duplicate members and readers cannot reliably retrieve the affected files.
Reserve generated names and ensure every source file gets a unique archive name across both data-directory and external paths.
Also at `astrbot/dashboard/services/log_service.py:157-167`.
</issue_to_address>
### Comment 9
<location path="astrbot/dashboard/services/log_service.py" line_range="105" />
<code_context>
+ traces = [item for item in cached if item.get("type") == "trace"]
+ data_dir = Path(get_astrbot_data_path())
+ export_dir = Path(get_astrbot_temp_path()) / "log_exports"
+ export_dir.mkdir(parents=True, exist_ok=True)
+ # The response removes its archive, except when the download is cancelled.
+ for stale in export_dir.glob("*.zip"):
</code_context>
<issue_to_address>
**Directory errors bypass export handling**
When the temp directory cannot be created, such as when its filesystem is read-only or full, `export_dir.mkdir()` raises `OSError` before the method’s `try` block, so `export_logs()` does not translate it to `LogServiceError` and the route returns a generic server error.
Move directory creation inside the handled block and translate its `OSError` to `LogServiceError`.
</issue_to_address>
### Comment 10
<location path="tests/test_fastapi_v1_dashboard.py" line_range="4332" />
<code_context>
+ (logs_dir / "event_loop_watchdog.log").write_text("watchdog", encoding="utf-8")
+ external_dir = tmp_path / "external"
+ external_dir.mkdir()
+ (external_dir / "custom.log").write_text("current", encoding="utf-8")
+ (external_dir / "custom.1.log").write_text("rotated", encoding="utf-8")
+ other_dir = tmp_path / "other"
</code_context>
<issue_to_address>
**Rotated log contents go unchecked**
When the exporter includes `external/custom.1.log` but writes incorrect bytes for it, `test_v1_log_export_packs_log_files_and_memory_logs` checks that `external/custom.1.log` is present by name at lines 4361–4369, but never reads its contents; an exporter that writes empty or corrupted bytes for rotated files still passes.
Assert that `archive.read("external/custom.1.log")` equals `b"rotated"`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 10 findings to address first, and if the export authorization or file selection is wrong, a system-scoped caller could download chat messages, user IDs, or configured external logs, and downloaded copies outlive a revert. Reverting would prevent future exports but cannot recall archives that were already downloaded.
Blocking findings: astrbot/dashboard/services/log_service.py:124, astrbot/dashboard/services/log_service.py:146, astrbot/dashboard/services/log_service.py:105, astrbot/core/utils/storage_cleaner.py:162, astrbot/dashboard/services/log_service.py:146, and 5 more
Open each log file without following symlinks or blocking on FIFOs and check the opened descriptor, so a file swapped after the lstat() check cannot get into the archive. Run one export at a time and check the free temp space before packing. Reserve the generated archive entries and dedupe every file name, move the temp directory setup into the handled block, and say in the tooltip and docs that without file logging the archive mostly has in-memory logs, since older log files on disk are still included.
# Conflicts: # dashboard/src/api/generated/openapi-v1/sdk.gen.ts
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #10407.
Adds a way to export logs from the WebUI, so users can hand them to developers when reporting a problem. As planned in the issue, the archive covers the log files and the in-memory logs and traces.
Modifications / 改动点
Backend
GET /api/v1/logs/export(systemscope, like the other log routes) returns a zip archive. It's built in a worker thread underdata/temp/log_exports/and deleted once the response has been sent. Archives left behind by a cancelled download are removed by the next export once they are an hour old.StorageCleaner.collect_log_files()(renamed from_collect_log_files()): everything underdata/logs, plus the configuredlog_file_path/trace_log_pathand their rotated files, even when those point outsidedata/logs. So the export covers the same files the log and cache cleanup panel cleans.logs/astrbot.log), files configured elsewhere go underexternal/, and same-named ones from different directories becomeexternal/2/...instead of overwriting each other.lstat(), plus the resolved path has to match), so a link in a log directory can't pull in a file from elsewhere and a FIFO can't block the export.LogBroker.log_cache, the last 500 entries, shared by logs and traces) is copied once and split intomemory/logs.txtandmemory/traces.jsonl.manifest.jsonlists the packed files, the skipped ones with the reason (not a regular file, rotated away while packing, no permission, ...), the AstrBot version and the in-memory log and trace counts. A file that can't be read is skipped instead of failing the whole export.Frontend
LogExportButton.vue, used in the console page header and in the log and cache cleanup panel in Settings, next to the clean button. It saves the archive under the filename fromContent-Disposition.openspec/openapi-v1.yamlhas the new route, and the client was regenerated withgenerate:api(onlyexportLogsand its types are added).Docs:
docs/zh/use/webui.mdanddocs/en/use/webui.mdnow describe the button, what the archive contains, and that file logging needs to be on for full logs.Tests in
tests/test_fastapi_v1_dashboard.py:test_v1_log_export_packs_log_files_and_memory_logs: a file underdata/logs, a configured log file outside it plus its rotated file, a same-named file from another directory, memory logs and traces, the manifest (with no host paths in it), and the temp archive being removed.test_v1_log_export_skips_files_that_are_not_regular: a symlink indata/logspointing outside is left out and listed as skipped. Where symlinks can't be created (Windows without developer mode), it fakeslstat()to take the same path.test_v1_log_export_keeps_archive_names_unique: configured log files named like the generated entries get their own names.test_v1_log_export_refuses_when_disk_space_is_shortandtest_v1_log_export_runs_one_export_at_a_time: the export fails cleanly instead of filling the disk or running twice.test_v1_log_export_requires_authentication: 401 without a token.The suggestions from @LIghtJUNction in the issue are covered by the second commit, and the Sourcery review by the third.
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Console page, with the tooltip shown on hover:
Settings → General → Cache, in the log and cache cleanup panel:
Verification steps:
On a local instance, both buttons downloaded
astrbot-logs-<time>.zip. With the final code, the archive was:{ "astrbot_version": "4.29.0-beta.1", "exported_at": "2026-10-06 16:38:28 +0800", "files": [ "logs/astrbot.log", "logs/astrbot.trace.log" ], "skipped": [], "memory_logs": 25, "memory_traces": 0 }memory/logs.txthad the chat lines (e.g.member/10001: 天气真好啊), anddata/temp/log_exports/was empty after the download. The trace files are empty only because that instance had no model configured, so nothing got traced; the tests cover the trace part.Tests (Windows 11, Python 3.13, Node 22):
The 2 failures are
tests/test_fastapi_v1_dashboard.py::test_config_update_revokes_only_affected_shell_sessions(member_executionandadmin_removal). They fail the same way on master without this change.Checklist / 检查清单
😊 This is the feature requested in [Feature] 增加日志导出 #10407. I posted the plan in the issue before starting.
/ 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。
📚 The new button is described in
docs/zh/use/webui.mdanddocs/en/use/webui.md. No entry point was renamed, moved or merged, so no old-to-new mapping is needed./ 我已对照变化后的 WebUI 入口、页面结构和术语，核对并在本 PR 中更新
docs/zh和docs/en的相关操作说明与截图（或说明无需更新文档的原因）。入口改名、移动或合并时，已在文档和 changelog 中补充 旧入口 → 新入口 对照。🤓 No new dependencies are introduced.
/ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Enable users to export runtime logs and traces from the WebUI for troubleshooting.
New Features:
Enhancements:
Documentation:
Tests: