chore(sync): absorb upstream 4.28.1 window - #203
Conversation
…otDevs#10082) Wire build_provider_headers into the fork provider sources, including openai_chat_completions_source and openai_responses_source. Keep Edge TTS as a lazy import. Tests live under tests/unit/provider/. Upstream-Commit: d524b87 Upstream-Author: Soulter <37870767+Soulter@users.noreply.github.com> Upstream-PR: AstrBotDevs#10082 Sync-Disposition: adapt Fork-Adaptation: Map openai_source.py onto openai_chat_completions_source.py and openai_responses_source.py; keep Edge TTS importlib loading; omit missing dashscope embedding and TEI sources; tests under tests/unit/provider/. Tested: uv run pytest tests/unit/provider/test_provider_user_agent.py tests/unit/provider/test_anthropic_kimi_code_provider.py tests/unit/provider/test_openai_chat_completions_source.py tests/unit/provider/test_openai_responses_source.py tests/unit/provider/test_openai_embedding_source.py tests/unit/provider/test_opencode_go_providers.py tests/unit/provider/test_ssycloud_source.py tests/unit/provider/test_mirarouter_source.py -q --test-profile blocking AI-Generated: true Generated-At: 2026-09-16T06:51:43Z
…ing (AstrBotDevs#10069) Close every open C2C stream segment even when the tail buffer is empty or contains only an empty Plain, using the fork-owned stream delta helper. Upstream-Commit: 31d7989 Upstream-Author: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Upstream-PR: AstrBotDevs#10069 Sync-Disposition: adapt Fork-Adaptation: Keep _append_stream_delta and PlatformSendResult; add regressions next to existing C2C stream tests. Tested: uv run pytest tests/unit/platform/test_qqofficial_group_message_create.py -q --test-profile blocking AI-Generated: true Generated-At: 2026-09-16T06:54:09Z
…strBotDevs#10084) Group the model picker by provider source, add a source filter, and keep sticky headers with virtualized rows. Preserve fork trigger labels, async provider drawers, capability badges, and connectivity tests. Upstream-Commit: 56cbc4a Upstream-Author: Soulter <37870767+Soulter@users.noreply.github.com> Upstream-PR: AstrBotDevs#10084 Sync-Disposition: adapt Fork-Adaptation: Keep buttonText, defineAsyncComponent drawers, and input-variant stored model names; omit ja/ru locales. Tested: cd dashboard && pnpm exec vue-tsc --noEmit && pnpm exec vitest run --config vitest.config.ts tests/configProductization.vitest.ts && pnpm i18n:check AI-Generated: true Generated-At: 2026-09-16T06:58:59Z
…evs#10079) Treat an empty api_base string as unset so the OpenAI SDK keeps its default official endpoint. Upstream-Commit: 7ee03f2 Upstream-Author: Wei Chengqian <wcqqq1214@gmail.com> Upstream-PR: AstrBotDevs#10079 Sync-Disposition: adapt Fork-Adaptation: Apply the two-line base_url fix on openai_chat_completions_source.py. Tested: uv run pytest tests/unit/provider/test_openai_chat_completions_source.py::test_empty_api_base_uses_sdk_default_endpoint tests/unit/provider/test_openai_chat_completions_source.py::test_null_api_version_uses_regular_openai_client -q --test-profile blocking AI-Generated: true Generated-At: 2026-09-16T07:00:06Z
…otDevs#10027) (AstrBotDevs#10046) Telegram long-polling floods the log in two ways: 1. python-telegram-bot performs getUpdates through httpx, which logs every request at INFO level. httpx/httpcore were missing from LogManager._NOISY_LOGGER_LEVELS, so those per-request lines (one every ~10s) leaked through. 2. _on_polling_error logged a full traceback for every polling error, including non-network errors such as Conflict (two bot instances polling the same token). Log a concise one-line message for non-network errors and keep the traceback only for NetworkError, which still needs it for recovery. (cherry picked from commit e79c0d7)
…sized images (AstrBotDevs#10071) Skip still-image resize for CUA sandbox sessions at the request-preparation choke point so pixel coordinates stay 1:1. Keep always-JPEG conversion, drop images above the 20 MB cap, and warn above 5 MB without resize. Upstream-Commit: 02fef48 Upstream-Author: 氕氙 <piexian@foxmail.com> Upstream-PR: AstrBotDevs#10071 Sync-Disposition: adapt Fork-Adaptation: Implement CUA no-resize in image_compress_args_from_settings and request_preparation; keep always-JPEG and dhash animation sampling; document in bilingual astrbot-config.md instead of image-formats.md. Tested: uv run pytest tests/unit/test_provider_request_preparation.py -q --test-profile blocking AI-Generated: true Generated-At: 2026-09-16T07:04:01Z
Record dispositions for bd046ed..ab42c0d: adapt AstrBotDevs#10082, AstrBotDevs#10069, AstrBotDevs#10084, AstrBotDevs#10079, and AstrBotDevs#10071; cherry-pick AstrBotDevs#10046; skip AstrBotDevs#10085. Advance the cursor to ab42c0d and note absorbed 4.28.1 changelog entries. AI-Generated: true Generated-At: 2026-09-16T07:05:28Z
Use the upstream 4.28.1 date (2026-09-14), start the next fork version as WIP, and keep pyproject.toml, astrbot.__version__, and uv.lock in sync. AI-Generated: true Generated-At: 2026-09-16T07:17:28Z
Add member/admin Local permission policies, OS process sandboxes, and Dashboard matrix controls. Keep ComputerRuntime ownership, authorization actions, regex command blocking, and Python 3.14-only search fallbacks. Upstream-Commit: 06261c5 Upstream-Author: Soulter <37870767+Soulter@users.noreply.github.com> Upstream-PR: AstrBotDevs#9472 Sync-Disposition: adapt Fork-Adaptation: Keep ComputerRuntime instance booters and async authorization; map instance_operator+ to the admin policy row; do not restore computer_use_require_admin or python-ripgrep; regenerate OpenAPI; map tests to tests/unit/. Tested: uv run pytest tests/unit/test_local_permission_policy.py tests/unit/test_local_sandbox_access.py tests/unit/test_process_sandbox.py tests/unit/test_computer.py tests/unit/test_python_tools.py -q AI-Generated: true Generated-At: 2026-09-16T07:55:15Z
Report disabled local network policy in Shell/Python tool results and clarify file-access scope hints in the permission matrix. Upstream-Commit: fce1bbc Upstream-Author: Ruochen Pan <badbatch0x01@gmail.com> Upstream-PR: AstrBotDevs#10096 Sync-Disposition: adapt Fork-Adaptation: Keep JSON managed-shell results with policy_notice; skip ja-JP/ru-RU catalogs this fork does not ship; preserve ExecuteShellTool background schema. Tested: uv run pytest tests/unit/test_python_tools.py tests/unit/test_func_tool_manager.py -q AI-Generated: true Generated-At: 2026-09-16T07:59:34Z
…s#10088) Translate the pnpm/action-setup 6.1.0 and github/codeql-action 4.38.0 bumps to immutable commit SHAs across every matching fork workflow. Upstream-Commit: 5950d7c Upstream-Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Upstream-PR: AstrBotDevs#10088 Sync-Disposition: adapt Fork-Adaptation: Pin peeled SHAs instead of floating tags; skip missing release.yml; update extra fork workflows that share the same action pins. Tested: git grep -n 'pnpm/action-setup@\\|github/codeql-action' .github/workflows AI-Generated: true Generated-At: 2026-09-16T08:00:40Z
Add old-to-new WebUI entry mappings and keep contributor docs aligned with in-app /help/ documentation. Upstream-Commit: 553096b Upstream-Author: Soulter <37870767+Soulter@users.noreply.github.com> Upstream-PR: AstrBotDevs#10093 Sync-Disposition: adapt Fork-Adaptation: Keep fork AGENTS.md/CONTRIBUTING.md and More → Data labels; skip deleted provider/deploy pages, v4.28.0 changelog with docs.astrbot.app links, and restoring missing ja/ru catalogs. Tested: rg -n '菜单与旧入口对照|Navigation and previous entry points' docs/zh/use/webui.md docs/en/use/webui.md AI-Generated: true Generated-At: 2026-09-16T08:04:00Z
…trBotDevs#10104) Keep the creator-only edit/delete rule, but describe other-member and dashboard-created jobs instead of pretending they disappeared. Upstream-Commit: 763a5ee Upstream-Author: AstrBot <soulter618@gmail.com> Upstream-PR: AstrBotDevs#10104 Sync-Disposition: adapt Fork-Adaptation: Apply the ownership replies on the extracted edit/delete/list methods instead of inlining them into call(). Tested: uv run pytest tests/unit/test_cron_tools.py -q AI-Generated: true Generated-At: 2026-09-16T08:09:00Z
Record dispositions for upstream AstrBotDevs#9472, AstrBotDevs#10096, AstrBotDevs#10088, AstrBotDevs#10093, and AstrBotDevs#10104, and advance the cursor to upstream/master. AI-Generated: true Generated-At: 2026-09-16T08:13:57Z
Keep Gemini UA stripping defensive for SDK stubs, type Azure base_url without passing None, and wire request_headers into constructor-skipping tests. Also document the Edge TTS ImportError and avoid a mixed import in request-preparation tests. AI-Generated: true Generated-At: 2026-09-16T08:28:52Z
Replace Lucide with MDI, satisfy CodeQL protocol/except/permission alerts, and keep Local permission tests aligned with the current APIs. AI-Generated: true Generated-At: 2026-09-16T09:57:54Z
Open workspace-scoped files without POSIX dir_fd/pread so Windows CI and Local members can still read and write inside allowed roots. AI-Generated: true Generated-At: 2026-09-16T10:58:40Z
Independent AI-assisted review of the 4.28.1 upstream-sync window, as required by AI_POLICY.md before merge. Records the scope mismatch between the PR body and the branch, the Local permission-role fallback that makes file tools evaluate the member policy (reproduced), and 16 further findings across the new sandbox modules, the config-save probe, and the provider header changes. No fixes applied. AI-Generated: true Generated-At: 2026-09-16T12:21:43Z
43673f1 to
ffd965b
Compare
PR #203 audit —
|
| Reviewed head | ffd965bc3d9f1e2c02ebc5694201054b913bf64a |
| PR base | 4479f89f5de91f1492f29114ecc9650640539546 |
| Commits / files | 17 commits, 115 files, +5595 / −532 |
| Method | full diff read; every claim in the PR body re-run; findings reproduced or derived from the shipped code |
| Fixes applied | none (per request) |
This review contains no remediation commits; every finding is left for the authors to resolve.
Summary
The absorbed upstream changes themselves are small and look faithful: the provider User-Agent work (AstrBotDevs#10082), the OpenAI api_base fix (AstrBotDevs#10079), the QQ C2C closing frame (AstrBotDevs#10069), the Telegram log-noise change (AstrBotDevs#10046), the CUA still-image mode (AstrBotDevs#10071), the model-selector grouping (AstrBotDevs#10084), the cron ownership messages (AstrBotDevs#10104), and the action-pin bumps (AstrBotDevs#10088) all behave as their upstream commits intend, and the fork's own invariants (always-JPEG preparation, openai_chat_completions_source.py, C2C _append_stream_delta, MDI drawers, no BTW surface touched, no docs.astrbot.app links restored) are preserved.
The risk in this PR is concentrated in AstrBotDevs#9472 (role-based Local computer permissions), which is the single largest change and the only one that introduces a security boundary: four new modules, a permission matrix, OS sandbox launchers, and a Dashboard control. It is also the part the PR body says is not in the PR, so a reviewer following the body would not look at it.
Two findings block merge in my view:
- A1 (High) — the Local file tools resolve the caller's role from a marker that only the execution tools ever set, so an administrator's file tools are evaluated against the member policy, or denied outright on Windows defaults, depending on which tool the model happened to call first in a message.
- A2 (High) — the PR body materially misdescribes the scope of the change, and the merge-gate reviewer would be reviewing a different PR than the one on the branch.
make check / make quality were not run by the author and are not run here either as a single gate, but each gate was run individually: ruff format --check, ruff check, pyright --project pyrightconfig.quality.json (clean on Linux; the Windows-only diagnostics are discussed in A3), vue-tsc, vitest, i18n:check, and the full blocking pytest profile (5655 passed, 26 skipped, 0 failed).
Scope of the change vs. the PR body
The body says the branch absorbs "the first 4.28.1 upstream window (bd046ed..ab42c0d9, 7 commits)" and then states:
This window does not absorb later upstream commits
#9472,#10096,#10088,#10093, or#10104.
and in Compatibility and risk:
Remaining upstream Local sandbox permissions (
#9472) are not in this PR.
All five of those commits are on the branch (77e4d26b5, 9b7e9ca7c, b682ddbe3, cb4dc22e8, 8cf6d6791), plus three fork-authored follow-ups (968eae830, 6eaa37d92, ffd965bc3). #9472 is not merely present — it is the bulk of the diff (local_file_security.py, process_sandbox/*, LocalPermissionMatrix.vue, the permission matrix in config/default.py, the Dashboard runtime probe).
The repository's own ledger agrees with the branch, not with the body: upstream-sync.yaml sets last_synced.commit: 763a5ee9 with source_pr: '9472,10096,10088,10093,10104', and upstream-decisions.jsonl carries an #9472 record (2026-09-16T08:13:57Z-06261c532a64-1, reason_codes includes security). The body reads as a description of an earlier revision of the branch that was never updated.
Other body/ledger drift
| Claim in the body | Actual |
|---|---|
| "The upstream version-bump commit is skipped because the fork already ships 4.28.1" | ddf60a07d bumps astrbot/__init__.py, pyproject.toml and uv.lock to 4.28.2 and adds changelogs/v4.28.2.md; CHANGELOG.md dates 4.28.1 at 2026-09-14 |
| "Ledger validates (151 events, 129 commits)" | upstream_decisions.py validate → 156 events, 134 commits |
| "109 focused pytest tests passed" | the stated command yields 110 passed |
upstream-decisions.jsonl record …ab42c0d9b726-1, disposition skip, "Keep the existing fork 4.28.1 version files" |
contradicts the branch, which moves the version to 4.28.2 |
None of these are code defects, but the ledger is this fork's mechanism for justifying upstream integration, and a skip record that the branch contradicts is exactly the kind of drift the ledger exists to prevent. The version bump itself is fine and consistent (pyproject.toml, uv.lock, astrbot/__init__.py, CHANGELOG.md, changelogs/v4.28.2.md all agree); only the record is stale.
Findings
A1 — High — Local file tools resolve the caller's role from an order-dependent marker
Where: astrbot/core/tools/computer_tools/util.py:62-75 (_local_permission_role), util.py:119-139 (check_local_file_permission), util.py:184-186 (only writer of the marker); call sites astrbot/core/tools/computer_tools/fs.py:299, 388, 489, 717.
get_local_permission_policy picks the permission-matrix row like this:
event = context.context.event
if getattr(event, "role", None) == "admin":
return "admin"
stored = getattr(event, "_computer_permission_role", None)
if stored in {"admin", "member"}:
return stored
return "member"_computer_permission_role is written in exactly one place — check_admin_permission — which is called only by the execution tools (shell.py:114, shell.py:287, python.py:148, plus upload/download). The file tools (FileReadTool, FileWriteTool, FileEditTool, GrepTool) call check_local_file_permission, which never calls check_admin_permission. So for a file-tool call the marker is unset, and the policy silently degrades to the member row.
The event.role branch cannot rescue this in production: AstrMessageEvent has no role attribute (it exposes platform_member_role, set by set_platform_member_role, and is_admin() deliberately returns False). role is injected only by the new tests (tests/unit/test_computer_fs_tools.py:49, tests/unit/test_local_permission_policy.py:34), which inject it directly and therefore cannot observe the fallback.
Reproduced against the PR head with the Windows defaults:
windows defaults: {'member': {..., 'filesystem_scope': 'none'}, 'admin': {..., 'filesystem_scope': 'host'}}
resolved policy : LocalPermissionPolicy(allow_execution=False, allow_network=False, filesystem_scope='none')
file tool result : error: Permission denied. Local computer tools are disabled for this user role. ...
(linux defaults resolve to the member workspace row and return None.)
Impact. On Windows the documented intent is "member access disabled, admins unrestricted" (get_local_permission_defaults docstring, test_local_permission_defaults_disable_windows_member_access). In reality every file-tool call is evaluated as a member and denied, including for administrators, unless an execution tool happened to run earlier in the same message and set the marker. This is also the exact Windows surface that ffd965bc3 ("keep restricted local file tools working on windows") set out to fix — it fixed the descriptor/pread mechanics but not this. On Linux/macOS the same fallback silently downgrades an administrator configured for host scope to workspace-only file access (and to the skill/temp read roots), so the effective policy can differ from the configured one per tool call.
The failure direction is closed (a member cannot escalate: INSTANCE_TOOL_ROLES is {instance_operator, operator, root}), so this is a functional/authorization-consistency defect rather than an escalation path.
Direction. Derive the role where it is used rather than from mutable event state: resolve it in the file tools through the same authorization decision, or read the caller's role from the auth context, instead of relying on a marker that a different tool may or may not have set earlier in the message. Tests should exercise the production shape (no event.role), not the injected one.
A2 — High — PR body misstates scope
Covered in Scope of the change vs. the PR body. The merge gate requires a human review plus an AI-assisted review of the change; a body that says the security-sensitive half is absent makes a correct review unlikely. Also note the checklist item "The change is focused and does not include unrelated product refactors" — #9472 is a feature-sized security surface, and chore(sync) hides it.
A3 — Low — make quality was not run; pyright reports 9 new diagnostics on Windows, 0 on Linux
uv run pyright --project pyrightconfig.quality.json on the PR head reports 16 errors, 1 warning on Windows, against 7 errors, 1 warning at 4479f89f5. The 9 new ones are all in the new POSIX-only modules:
local.py:408 "_TemporaryFileWrapper[bytes]" not assignable to "BinaryIO"
local_file_security.py:90,278,278,303 os.pread / os.O_DIRECTORY / os.O_NOFOLLOW
unix.py:69,78,159,161 signal.SIGKILL / os.killpg / read(int | None)
Running the same command with --pythonplatform Linux — what quality-gates.yml (ubuntu-latest) actually does — gives 0 errors, 1 warning on the head. The remaining Windows diagnostics are typeshed artifacts of POSIX-only APIs and follow the pre-existing pattern (project_workspace.py:134, data_file_service.py:225/227/274/1136, runtime_instance_lock.py:82, local.py:146 at base). So this PR does not break the CI gate, but it does add new Windows-host pyright noise in modules that are nominally cross-platform. Worth a sys.platform guard or a targeted # type: ignore if Windows contributors are expected to keep make quality green locally.
A4 — Medium — the sandbox-availability gate cannot see a missing Linux backend
Where: astrbot/core/computer/process_sandbox/__init__.py:21-38 (create_process_sandbox), astrbot/core/computer/process_sandbox/bubblewrap.py:31-35, astrbot/core/tools/computer_tools/util.py:222-231.
check_local_execution_permission probes availability with create_process_sandbox() and turns a RuntimeError into the curated message "Restricted Local execution is unavailable: … Select Third-party sandbox…". But create_process_sandbox() only dispatches on sys.platform — the shutil.which("bwrap") check lives in BubblewrapProcessSandbox._build_command. On a Linux host without bubblewrap the probe therefore succeeds and the curated error is never produced; the failure surfaces later as a raw RuntimeError from _build_command, through the tool's generic except Exception handler.
detect_local_runtime_info() (__init__.py:41-92) does check which("bwrap") and drives the Dashboard status and the config-save gate, so the two gates can disagree about whether the same host can run a sandbox.
A5 — Medium — configuration saves run a real sandbox probe synchronously
Where: astrbot/dashboard/services/config_service.py:805, config_service.py:545-628, astrbot/dashboard/services/stat_service.py:71, astrbot/core/computer/process_sandbox/__init__.py:66-87.
save_config_async calls detect_local_runtime_info() on every save. That function creates a temporary directory and launches a real sandbox (bwrap or seatbelt plus a Python wrapper) with a 5 s timeout, synchronously, inside an async request handler — the event loop is blocked for the duration of a process launch on every config save. StatService.__init__ does the same at startup. On macOS the profile builder can additionally shell out to /usr/bin/otool -L up to 64 times (seatbelt.py:182-213) when the interpreter is not under a standard prefix.
ConfigProfileService also keeps a separate, construction-time snapshot (config_service.py:877), so the status the Dashboard displays can be older than the status the save-time validation computed.
A6 — Medium — unbounded reads in the local document/image path
Where: astrbot/core/computer/file_read_utils.py:310-326 (_read_local_image_base64), :329-341 (_read_local_file_bytes), reached from :457-493.
_parse_local_supported_document reads the entire file into memory before any size check, for any path ending in .pdf, for .epub/.docx, and for anything whose first 512 bytes look like a ZIP container. _read_local_image_base64 likewise reads the whole image. The thresholds in this module (_MAX_TEXT_FILE_FULL_READ_BYTES = 256 KiB, _MAX_FILE_READ_BYTES = 128 KiB, the 20 MB preparation cap) are all applied after the bytes are already resident, and the preparation cap applies to the request path, not here. A multi-gigabyte .pdf in the workspace is enough to exhaust memory.
A7 — Medium — the ripgrep fallback has no timeout and no size bound
Where: astrbot/core/computer/booters/local.py:1049-1101.
When rg is not on PATH, LocalFileSystemComponent.search_files falls back to a pure-Python scan that rglobs every file under the search root, reads each one with read_text(encoding="utf-8", errors="ignore"), and applies re.compile(pattern) built from the model-supplied pattern. The 30 s timeout in the same method covers only the rg subprocess branch. There is no timeout, no file-count or file-size bound, and no regex-complexity guard, so a single catastrophic-backtracking pattern over a large file pins a worker thread indefinitely. In restricted local mode the fallback search roots are the member's read roots (fs.py:676), which include shared temporary directories.
A8 — Medium — members can read other sessions' artifacts through the shared temp roots
Where: astrbot/core/tools/computer_tools/fs.py:126-144 (_read_allowed_roots / _write_allowed_roots).
Both get_astrbot_temp_path() (data/temp) and get_astrbot_system_tmp_path() (<system tmp>/.astrbot) are granted as read and write roots to every member, and they are shared across sessions:
FileDownloadToolwrites downloaded sandbox files there assandbox_<4 hex>_<original name>(astrbot/core/tools/computer_tools/fs.py:892-894).- Background shell output is written there as
astrbot_shell_stdout_<8 hex>.log(shell.py:42-49).
Because the name is derived from the original filename plus a short random prefix, another member can list the directory and read a peer's downloaded file or background command output. This is inherited from the upstream matrix rather than introduced by the fork, and it is worth deciding deliberately rather than by accident.
A9 — Medium — _is_safe_command is not an effective control
Where: astrbot/core/computer/booters/local.py:43-61, relied on at :171 and :290.
The blocklist is described in the code as the safety basis for the unsandboxed path ("Safety relies on _is_safe_command() and the allowed-root checks", local.py:206-220). Probed against the shipped implementation:
False rm -rf /
True /bin/rm -rf /
True \rm -rf /
True find / -delete
True python -c "import shutil; shutil.rmtree('/')"
The patterns are anchored on (^|[;&|() ]), so any command that reaches the binary by path, or through a shell builtin/wrapper, or via a language runtime, is not matched. The homoglyph/case normalisation is not the issue; the anchoring is. I am deliberately not enumerating further bypasses here — see the disclosure note at the end.
The blocklist also produces false positives on ordinary content (a quoted string containing dd if= or the word sudo, rm with an -r-containing flag in an unrelated word) and applies inside the sandbox too (exec_managed re-checks at local.py:290), so members cannot rm -rf their own workspace even though the sandbox is precisely the boundary that would make that safe.
Direction. Decide what this list is for. If it is a UX guard, say so and stop citing it as safety. If the unsandboxed path needs a real guarantee, it needs a mechanism, not a pattern list.
A10 — Medium — resource limits are ceilings that ordinary Python work will hit
Where: astrbot/core/computer/process_sandbox/base.py:22-26 (SandboxLimits defaults), unix.py:223-256 (build_resource_limited_argv).
Defaults are cpu_seconds=300, file_size_bytes=100 MiB, memory_bytes=1 GiB, open_files=256, processes=256. On Linux the wrapper applies RLIMIT_AS = memory_bytes (line 240) — a virtual address space limit, not RSS. Python scientific stacks reserve far more address space than they touch (numpy/OpenBLAS thread arenas, polars, numba), so astrbot_execute_python inside the workspace sandbox will commonly fail with MemoryError on imports that work fine on the host. RLIMIT_FSIZE = 100 MiB similarly turns a larger write into SIGXFSZ rather than a truncation warning. Neither is wrong as a policy, but 1 GiB of VA is low enough to make the sandboxed Python tool unreliable for the data tasks it advertises, and there is no way for an operator to raise it.
RLIMIT_NPROC/RLIMIT_AS are Linux-only by design (the macOS comment at unix.py:235-238 explains why), which is correct, but it means macOS sandboxes enforce only CPU, file size, open files and core.
A11 — Low — sandbox process termination can signal a recycled PID
Where: astrbot/core/computer/process_sandbox/unix.py:71-80, compared with astrbot/core/computer/booters/local.py:138-149.
UnixSandboxProcess._send_signal calls os.killpg(self.pid, signal) and catches only ProcessLookupError. Its counterpart in local.py catches ProcessLookupError and PermissionError, with a docstring noting that macOS reports an already-reaped group as PermissionError. _terminate_process (local.py:736-739) intentionally does not short-circuit for sandboxed sessions, so an exit-then-reap-then-terminate sequence is reachable. The consequence of a recycled PGID is a signal delivered to an unrelated process group; the window is small but the mitigation (re-check returncode / tolerate PermissionError) is cheap and already written next door.
A12 — Low — dead code and unreachable branches
astrbot/core/computer/booters/local.py:926-1001— thesandboxed=/sandbox_root=branch ofsearch_files(checks forrg, requiressandbox_root, runscreate_process_sandbox().run(...)) is unreachable: no caller passes either argument. The only caller isGrepTool(fs.py:745-751), which runs the host-side path. ~50 lines of untested, unexercised sandbox code.astrbot/core/computer/booters/local.py:159—disk_quota_bytesis declared and never read; it is a leftover from the on-disk output design that315(tempfile.TemporaryFile) replaced, and the per-owner quota directory logic is gone.astrbot/core/tools/computer_tools/shell.py:111—kwargs.get("timeout_seconds")is dead: the tool schema (shell.py:81-85) names the parametertimeout.
A13 — Low — authorization-mapping details
astrbot/core/tools/computer_tools/util.py:148-160—check_admin_permissionmaps a dozen operation names to(action, resource)and falls back to("tool.local_exec", "shell-execution")for anything unrecognised. A future call site that forgets to register gets silently checked against the wrong resource rather than failing loudly.util.py:169-186— the_computer_permission_rolemarker is written beforeif not decision.allowed, so a denied operation still stamps the role. Harmless today (roles cannot be forged —INSTANCE_TOOL_ROLESis{instance_operator, operator, root}), but it means "the role was recorded" does not imply "the operation was authorised".astrbot/core/tools/computer_tools/shell.py:291-294—ShellSessionTooldeliberately skipscheck_local_execution_permissionwhenaction == "terminate"(and skips the local-runtime check). Intentional cleanup behaviour, worth confirming: a caller whose Local execution has been revoked can still terminate sessions. Ownership is enforced insideLocalShellComponent._get_owned_session(matched onruntime_id,umo,sender_id), so this is limited to the caller's own sessions.
A14 — Low — provider header details
- Precedence is inconsistent.
mimo_stt_api_source.py:92andmimo_tts_api_source.py:115merge as{**build_headers(key), **request_headers}(a custom header overridesAuthorization), whilenvidia_embedding_source.py:58,nvidia_rerank_source.py:45,ollama_embedding_source.py:51andbailian_rerank_source.py:78merge as{**request_headers, **headers}(auth wins). A user can overrideAuthorizationfor MiMo but not for NVIDIA. - Global third-party mutation.
edge_tts_source.py:178-185writesedge_tts.constants.WSS_HEADERS["User-Agent"] = DEFAULT_USER_AGENTon every call. That is a process-wide mutation of a third-party module, not thread-safe, and it ignorescustom_headers(the module's ownrequest_headers). Theexcept ImportErrorguard does not cover the case its comment names — a build that hasedge_tts.constantsbut noWSS_HEADERSraisesAttributeError, which is uncaught. (Noteedge_ttsis an optional dependency and is not installed in CI, so this path has no test coverage.) - Private attribute poke.
xinference_rerank_source.py:52andxinference_stt_provider.py:47mutateself.client._headersof the third-partyxinference_client. Verified present in the pinned version, but it is private API. - Default UA is now sent everywhere.
AbstractProvider.__init__(provider.py:40) always setsrequest_headers = {"User-Agent": "astrbot/<version>"}, and OpenAPI-family sources now pass it asdefault_headerseven when the user configured none (openai_chat_completions_source.py:466,openai_responses_source.py:177), so the SDK's own UA is replaced for every OpenAI-compatible endpoint, not only for the Kimi/MiniMax/Xiaomi templates the body mentions. That is the intent of fix: standardize provider user agents with the AstrBot version AstrBotDevs/AstrBot#10082, but it is a wider behavioural change than the body describes, and any gateway that keys off the SDK User-Agent is affected. Existing installations keep their storedcustom_headers(the templates only affect new configurations), so fresh installs and upgrades will behave differently.
A15 — Low — CUA warning is not CUA-scoped
Where: astrbot/core/agent/request_preparation.py:33, 249-259.
if not image_compress_enabled and _CUA_IMAGE_WARN_BYTES < len(jpeg_bytes) <= max_bytes:
logger.warning("Prepared image is %.1f MB without resize; ...")image_compress_enabled is also False when an operator deliberately sets image_compress_enabled: false (request_preparation.py:103-116), so the "without resize … may exceed provider image upload limits" warning fires for non-CUA configurations too, despite the constant name and the CUA-specific wording added to docs/{zh,en}/dev/astrbot-config.md. The threshold logic itself matches the documentation (warn above 5 MB, drop above 20 MB).
A16 — Low — Dashboard permission matrix details
LocalPermissionMatrix.vue:277-292derives its defaults fromruntime.os === 'windows', andruntimeisnulluntil/stats/versionresolves (and staysnullif the request fails). In that state a Windows host is shown the non-Windows defaults (workspace). The controls are locked whileruntimeis null (permissionLocks), so this is display-only, but it is the wrong row.LocalPermissionMatrix.vue:238-249, 347-355hardcodeswindowsPermissionDefaultsand reuseswindowsPermissionDefaults.member/.adminas thenone/fullpresets in the unsupported-platform view. These are a hand-copy ofget_local_permission_defaults()inastrbot/core/config/default.py; the two must be kept in sync with no test enforcing it.config_service.py:1015-1021callscomputer_runtime.get_local_booter()on profile update. That method lazily constructs aLocalBooteras a side effect (computer_client.py:756-766) and raisesRuntimeErrorif the runtime has been terminated; the call is not guarded, so a save racing a restart can fail.
A17 — Low — docs/public/openapi.json not refreshed
openspec/openapi-v1.yaml gains the RuntimeInfo component and a typed /api/v1/stats/version response, and the generated client is committed in step (types.gen.ts RuntimeInfo, index.ts, sdk.gen.ts). docs/public/openapi.json — the derived artifact the repository's own instructions say to refresh (CONTRIBUTING.md, AGENTS.md) — is not updated, so the published reference will not show the new response shape. This file is already ~293 lines out of sync at 4479f89f5, so it is not a regression introduced here; I am recording it only so the omission is a decision rather than an oversight.
Verified non-issues
Recorded so the review is not read as uniformly negative, and so later reviewers do not repeat the work.
pyrightis clean on Linux. 0 errors with--pythonplatform Linux; see A3.ruff format --check .→ "1602 files already formatted".ruff check .→ "All checks passed!".- Full blocking suite →
5655 passed, 26 skipped, 1 deselected(skips are the sandbox tests, which requirebwrap/sandbox-exec). - The PR's stated focused command → 110 passed (body says 109).
- Dashboard:
vue-tsc --noEmitclean;vitest run tests/configProductization.vitest.ts4 passed;i18n:checkpassed (2013 static keys, 4078 catalog keys). Thelocal_permissionsi18n block is structurally identical inzh-CNanden-US, andLocalPermissionMatrix.vue:265-267looks up exactly the path that was added. pnpm generate:apidrift is pre-existing. Regenerating on4479f89f5produces essentially the same diff as on the head (base 5657/10359 changed lines vs. head 5678/10381 — the delta is theRuntimeInfoaddition). The committed generated files were formatted at some point beforedashboard/src/api/generated/was added to.prettierignore, sogenerate:apino longer reproduces them byte-for-byte. Not caused by this PR.- The ledger validates.
uv run python .agents/skills/sync-upstream/scripts/upstream_decisions.py validate→valid: upstream-decisions.jsonl (156 events, 134 commits). upstream-sync.yamlcursor is consistent.last_synced.commit: 763a5ee9is the#10104upstream commit, and the fivesource_prentries match the five upstream commits in this window.- Fork invariants preserved. No
btwfile is touched; C2C_append_stream_deltaandPlatformSendResultremain in the QQ adapter; always-JPEG conversion is preserved (prepare_images_for_provider(..., resize=image_compress_enabled),request_preparation.py:236-241);openai_chat_completions_source.pysurvives as a fork file; the#10021ChatUI revert is not undone. Nodocs.astrbot.applink is restored — the two hits are ledger prose describing that decision.CODE_OF_CONDUCT.mdis untouched. seatbelt.py:194except OSError, subprocess.TimeoutExpired:is valid on Python 3.14 (PEP 758) andrequires-python = ">=3.14"; it parses,ruffaccepts it, andpyrightis clean. Worth knowing that the new sandbox modules depend on 3.14-only grammar.- Existing-test changes are benign. The only relaxed assertion is
tests/unit/test_local_shell_component.py:548, which now also acceptsoutput_limitedalongsidecompleted/terminated/failedfor the bounded-output session test — the added status is what the new reader sets at the byte limit.
Verification log
Run on the PR head (ffd965bc3), Python 3.14.1, Windows 11.
uv run pytest --test-profile blocking -q
-> 5655 passed, 26 skipped, 1 deselected in 631.75s
uv run pytest tests/unit/provider/test_provider_user_agent.py \
tests/unit/platform/test_qqofficial_group_message_create.py \
"tests/unit/provider/test_openai_chat_completions_source.py::test_empty_api_base_uses_sdk_default_endpoint" \
"tests/unit/platform/test_telegram_adapter.py::test_telegram_polling_error_requests_rebuild_after_threshold" \
tests/unit/test_provider_request_preparation.py -q --test-profile blocking
-> 110 passed
uv run pytest tests/unit/test_local_sandbox_access.py tests/unit/test_process_sandbox.py \
tests/unit/test_local_permission_policy.py tests/unit/test_local_filesystem_component.py \
tests/unit/test_computer_fs_tools.py tests/unit/test_local_shell_component.py \
tests/unit/test_python_tools.py tests/unit/test_cron_tools.py \
tests/unit/test_message_tools.py tests/unit/test_dashboard_config_service.py -q --test-profile blocking
-> 139 passed, 20 skipped
uv run ruff format --check . -> 1602 files already formatted
uv run ruff check . -> All checks passed!
uv run pyright --project pyrightconfig.quality.json -> 16 errors, 1 warning (Windows)
uv run pyright --project pyrightconfig.quality.json --pythonplatform Linux -> 0 errors, 1 warning (CI)
uv run python .agents/skills/sync-upstream/scripts/upstream_decisions.py validate
-> valid: upstream-decisions.jsonl (156 events, 134 commits)
cd dashboard && pnpm exec vue-tsc --noEmit -> clean
cd dashboard && pnpm exec vitest run --config vitest.config.ts tests/configProductization.vitest.ts
-> 4 passed
cd dashboard && pnpm i18n:check -> passed
The A1 reproduction is a standalone script calling get_local_permission_policy and check_local_file_permission with a SimpleNamespace event that exposes neither role nor _computer_permission_role — i.e. the production shape.
make check and make quality were run as their constituent targets rather than as single commands (check-md, check-yaml, check-toml and check-shell were not run, as this change does not touch markdown outside changelogs/, YAML outside the workflows and ledger, TOML, or shell scripts).
Disclosure note
Per this fork's AI_POLICY.md, security reports are meant to go through SECURITY.md rather than a public PR. Three items below are security-relevant and are written here as control-effectiveness findings with impact, not as exploitation recipes:
- A8 — cross-session readability of shared temporary directories.
- A9 — the command blocklist is not an effective control.
- A6 / A7 / A10 — resource bounds that a caller can exceed (memory, thread time, limits).
If the maintainers judge any of these to be vulnerabilities rather than defects, they should be triaged through SECURITY.md and their detail removed from this comment. A1, A2, A4, A5, A11-A17 are ordinary correctness, robustness and process findings and can stay public.
Reviewer note
Reviewed by an AI agent (Claude Code, Opus 5) at the maintainer's request, as the AI-assisted review required by the merge gate in AI_POLICY.md. Every functional claim above was reproduced or read from the shipped code; the reproduction for A1 and the raw tool output for the checks are summarised in the verification log. No fixes were applied. This review does not substitute for the human maintainer review the policy also requires, and the author agent's own note does not satisfy the AI-assisted review requirement — this one is a separate pass by a different agent, but the maintainer should confirm it meets the policy's intent.
Resolve Local file-tool role lookup from authorization, isolate session temp roots, bound document reads and grep fallbacks, and fail closed when the sandbox launcher is missing. AI-Generated: true Generated-At: 2026-09-16T14:32:41Z
Keep os.open ownership in the opening function so failed validation cannot leak descriptors, and replace empty excepts with explicit handlers CodeQL can see. AI-Generated: true Generated-At: 2026-09-16T16:07:17Z
The pattern list is not a security boundary. Sandboxed sessions already isolate the workspace, so members can manage their own files there. AI-Generated: true Generated-At: 2026-09-16T16:07:27Z
| outside = tmp_path / "secret.txt" | ||
| outside.write_text("nope", encoding="utf-8") | ||
| with pytest.raises(PermissionError, match="outside restricted roots"): | ||
| open_file_in_allowed_roots(str(outside), (allowed,), access="read") |
| nested = allowed / "dir" | ||
| nested.mkdir() | ||
| with pytest.raises(IsADirectoryError): | ||
| open_file_in_allowed_roots(str(nested), (allowed,), access="read") |
| outside = tmp_path / "secret.txt" | ||
| outside.write_text("nope", encoding="utf-8") | ||
| with pytest.raises(PermissionError, match="outside restricted roots"): | ||
| open_file_in_allowed_roots(str(outside), (allowed,), access="read") |
Summary
Absorb upstream through
763a5ee9(4.28.1 remainder plus the next window onto this fork). Provider User-Agent, QQ Official C2C closing frames, model-selector source groups, empty OpenAIapi_base, Telegram log noise, CUA still-image geometry, role-based Local computer permissions (#9472), sandbox policy feedback, GitHub Actions pin bumps, WebUI navigation docs, and cron ownership messages are integrated. The upstream version-bump commit#10085is skipped; this fork already dated 4.28.1 and opened 4.28.2 WIP.Related issue
Related: none
Surfaces
astrbot/core/provider/headers and sourcesastrbot/core/platform/sources/qqofficial/astrbot/core/computer/Local file security, process sandbox, bootersastrbot/core/tools/computer_tools/permission matrix and file/shell/python toolsdashboard/src/components/shared/ProviderSelectMenu.vue,LocalPermissionMatrix.vue, zh/en i18nastrbot/core/agent/request_preparation.pyastrbot/core/log.py, Telegram adapterchangelogs/v4.28.1.md,changelogs/v4.28.2.md,upstream-sync.yaml,upstream-decisions.jsonlImplementation notes
Default method is per-commit cherry-pick/adapt. This branch absorbs
#10082,#10069,#10084,#10079,#10046,#10071, then#9472,#10096,#10088,#10093, and#10104. Adaptations keep fork always-JPEG request preparation,openai_chat_completions_source.py, MDI provider drawers, C2C_append_stream_delta, ComputerRuntime ownership, and async authorization. Local file tools resolve the permission-matrix row from authorization (instance_operator+→ admin), not fromevent.roleor an execution-tool marker. Restricted temp roots are per-UMO. Document/image reads and the Python grep fallback are bounded.create_process_sandbox()fails closed whenbwrap/sandbox-execis missing. Config saves do not launch a live sandbox probe. Upstream changelog text from#10085is not copied.Validation
Focused pytest on the review follow-up passed. Dashboard vue-tsc, vitest, and i18n:check were previously run for the model-selector change. Cursor is
763a5ee9. Ledger validates.make check/make qualitywere not run as a single umbrella for this window.Compatibility and risk
astrbot/<version>unless custom headers override it. Kimi/MiniMax/Xiaomi templates no longer forceclaude-code/0.1.0. OpenCode Go keepsAstrBot/<version>.#9472) are in this PR. Windows members default tofilesystem_scope=none; admins default tohost. Restricted Local file tools no longer sharedata/tempacross sessions._is_safe_commandis a UX blocklist, not a security boundary; the OS sandbox is.Checklist
chore,build,ops, ortest(notci).pyproject.toml,requirements.txt, anduv.locktogether.Agent note
Goal: absorb upstream through
763a5ee9and close the AI review findings on Local permissions. Tools: local git,uv run pytest,ruff,pyright,gh. Residual risk: fullmake check/make qualitynot run as one umbrella; publicdocs/public/openapi.jsonstill omitsStatsby tag filter. Do not merge this PR; a human maintainer review and a separate AI-assisted review are required.