Repository navigation
Conversation
Expose the /api/v1/backups endpoints to developer API keys through a new
backup scope:
- Add backup to the OpenAPI scope list and grant it by default alongside
the other base scopes
- Replace the dashboard-only system scope check on /api/v1/backups*
routes with a ScopeDependency(backup), so JWT callers keep working and
backup-scoped API keys are now accepted
- Allow backup-scoped API keys to download backups without the
short-lived dashboard JWT download token
- Require an explicit backup:import sensitive sub-scope (following the
config:edit_admin and chat:admin pattern) for POST
/api/v1/backups/{filename}/import, which overwrites the live data
directory
- Document the scopes in openspec/openapi-v1.yaml, regenerate the
public OpenAPI JSON and scope reference docs, list them in the WebUI
API key scope selector with localized descriptions, and regenerate
the frontend API client
- Cover the scope and sub-scope with API key integration tests
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/api/backups.py" line_range="240" />
<code_context>
async def init_backup_upload(
payload: BackupUploadInitRequest,
- auth: AuthContext = Depends(require_system_scope),
+ auth: AuthContext = Depends(require_backup_scope),
service: BackupService = Depends(get_service),
):
</code_context>
<issue_to_address>
**Retried completion can delete upload**
When an API-key client retries `/backups/upload/complete` while its first request is still assembling, two concurrent `upload_complete` calls for the same session both pass `get_session` and assemble into the same destination. One assembly can clean up the session while the other is reading its chunks; the latter then fails and `assemble` unlinks the destination, deleting the completed backup.
Serialize completion per upload session and retain an idempotent completed result so retries cannot assemble into or remove the same destination concurrently.
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/api/backups.py" line_range="154" />
<code_context>
page: int = Query(default=1),
page_size: int = Query(default=20),
- _auth: AuthContext = Depends(require_system_scope),
+ _auth: AuthContext = Depends(require_backup_scope),
service: BackupService = Depends(get_service),
):
</code_context>
<issue_to_address>
**Concurrent uploads overwrite backups**
When two backup-scoped API-key callers upload the same source filename within one second, `BackupService.upload_backup` and `upload_init` both derive the destination name from the sanitized source filename and a timestamp precise to one second. Two newly permitted API-key uploads with the same source name in that second target the same path, so one uploaded backup overwrites the other.
Generate collision-resistant upload names and reserve them atomically before writing the uploaded files.
Also at `astrbot/dashboard/api/backups.py:179`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and adding backup to the default API-key scopes grants default-scoped keys access to download and delete backups; if that default is too broad, the permission decision applies silently, and any exposed or deleted backup cannot be undone by reverting. Backup import can also overwrite live data, though this change gates it behind the explicit backup:import scope.
Blocking findings: astrbot/dashboard/api/backups.py:240, astrbot/dashboard/api/backups.py:154
| async def complete_backup_upload( | ||
| payload: BackupUploadSessionRequest, | ||
| auth: AuthContext = Depends(require_system_scope), | ||
| auth: AuthContext = Depends(require_backup_scope), |
There was a problem hiding this comment.
🟡 Medium · Retried completion can delete upload
When an API-key client retries /backups/upload/complete while its first request is still assembling, two concurrent upload_complete calls for the same session both pass get_session and assemble into the same destination. One assembly can clean up the session while the other is reading its chunks; the latter then fails and assemble unlinks the destination, deleting the completed backup.
Serialize completion per upload session and retain an idempotent completed result so retries cannot assemble into or remove the same destination concurrently.
Prompt for AI agents
In `astrbot/dashboard/api/backups.py` at line 240:
**Retried completion can delete upload**
When an API-key client retries `/backups/upload/complete` while its first request is still assembling, two concurrent `upload_complete` calls for the same session both pass `get_session` and assemble into the same destination. One assembly can clean up the session while the other is reading its chunks; the latter then fails and `assemble` unlinks the destination, deleting the completed backup.
Serialize completion per upload session and retain an idempotent completed result so retries cannot assemble into or remove the same destination concurrently.| async def upload_backup( | ||
| file: UploadFile = File(...), | ||
| _auth: AuthContext = Depends(require_system_scope), | ||
| _auth: AuthContext = Depends(require_backup_scope), |
There was a problem hiding this comment.
🟡 Medium · Concurrent uploads overwrite backups
When two backup-scoped API-key callers upload the same source filename within one second, BackupService.upload_backup and upload_init both derive the destination name from the sanitized source filename and a timestamp precise to one second. Two newly permitted API-key uploads with the same source name in that second target the same path, so one uploaded backup overwrites the other.
Generate collision-resistant upload names and reserve them atomically before writing the uploaded files.
Also at astrbot/dashboard/api/backups.py:179.
Prompt for AI agents
In `astrbot/dashboard/api/backups.py` at line 154:
**Concurrent uploads overwrite backups**
When two backup-scoped API-key callers upload the same source filename within one second, `BackupService.upload_backup` and `upload_init` both derive the destination name from the sanitized source filename and a timestamp precise to one second. Two newly permitted API-key uploads with the same source name in that second target the same path, so one uploaded backup overwrites the other.
Generate collision-resistant upload names and reserve them atomically before writing the uploaded files.
Also at `astrbot/dashboard/api/backups.py:179`.
|
Thanks @sourcery-ai for the careful review. I verified both findings against the code — both races are real, but they are pre-existing issues in code this PR does not touch:
Both races were equally reachable by dashboard JWT callers before this PR; opening the endpoints to explicitly-scoped API keys only widens who can reach them, it does not introduce the races. Following the project's guidance to keep unrelated changes out of a PR, I'd like to keep this one scoped to the auth change and track the hardening work — serializing completion per upload session with an idempotent result, and collision-resistant upload naming — in a follow-up issue. Happy to implement those fixes in a separate PR. |
Motivation
Open the backup-related APIs to developer API keys so that automated and scheduled backup workflows (for example, periodically exporting and downloading backups with external schedulers or CI jobs) become possible. Scheduled/automatic backup support has already been requested in #8719; exposing the existing endpoints through the OpenAPI scope system is the API-level enabler for it.
Currently every
/api/v1/backups*endpoint requires thesystemscope, which is not a grantable API key scope, so all API keys are rejected with 403 and the endpoints are only reachable with a dashboard JWT. This is the same whitelist mechanism reported for the knowledge-base endpoints in #10322. This PR adds abackupscope so the backup endpoints follow the same explicit scope model as every other/api/v1resource.Modifications
backuptop-level scope to the OpenAPI scope list, granted by default alongside the other base scopes.systemscope check on all/api/v1/backups*routes withScopeDependency("backup"): dashboard JWT callers keep working unchanged, and API keys explicitly grantedbackupcan now list, create, download, upload, rename, delete, check, and import backups.backup-scoped API keys to download backups without the short-lived dashboard JWT download token;prepare_downloadgains arequire_tokenflag with the previous behavior preserved as the default (token is still strictly validated when provided).POST /api/v1/backups/{filename}/importwith an explicit sensitive sub-scopebackup:import, following the existingconfig:edit_admin/chat:adminpattern, because importing overwrites the live data directory. Keys that grantbackup:importwithout the parentbackupscope are rejected at creation time; abackup-scoped key without the sub-scope gets 403 on import.openspec/openapi-v1.yaml, regenerate the public OpenAPI JSON and the zh/en scope reference docs, add the new scopes to the WebUI API key scope selector with localized (en/zh/ja/ru) descriptions, and regenerate the frontend API client.This is NOT a breaking change: scopes are stored per API key, so existing keys are unaffected; JWT/dashboard behavior and the legacy
/api/backup/*dashboard routes are unchanged.Screenshots or Test Results
uv run pytest tests/test_api_key_open_api.py→ 15 passed, including the newtest_backup_scope_api_key: a backup-scoped key can list and download backups without a JWT token, gets 403 on import withoutbackup:import, passes the scope check with it; keys without the backup scope get 403 on list and download.uv run pytest tests/test_backup.py tests/test_fastapi_v1_dashboard.py→ 264 passed.ruff format --check .andruff check .with the CI-pinned ruff 0.15.22 → all checks passed (524 files).pnpm typecheck(vue-tsc) andpnpm i18n:checkindashboard/→ passed.Checklist
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试，并已在上方提供了"验证步骤"和"运行截图"。
📚 I checked the affected WebUI instructions and screenshots in
docs/zhanddocs/enagainst the changed navigation, page structure, and labels, and updated them in this PR (or explained why no documentation update is needed). For renamed, moved, or merged entry points, I included an old entry → new entry mapping in the documentation and changelog./ 我已对照变化后的 WebUI 入口、页面结构和术语，核对并在本 PR 中更新
docs/zh和docs/en的相关操作说明与截图（或说明无需更新文档的原因）。入口改名、移动或合并时，已在文档和 changelog 中补充 旧入口 → 新入口 对照。🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Enable scoped developer API-key access to OpenAPI backup endpoints while requiring explicit permission for backup imports.
New Features:
backupscope, including API-key downloads without dashboard download tokens.backup:importsub-scope for restoring backups.],Enhancements:
Documentation:
Tests: