Arcana 1.0 upgrade + 5 agent tools (scheduler/files/web/search_chat/todos) + security hardening - #2
Merged
Merged
Conversation
The pipe-to-shell danger rules were anchored on a download command (curl|wget|fetch|http) before the first pipe, so a payload decoded locally and piped into a shell slipped past the gate entirely: echo … | base64 -d | sh xxd -r -p x | sh openssl enc -d -in x | bash cat payload | sh base64 -d <<< … | python3 Only curl|bash was caught — the most realistic prompt-injection landing pad was wide open. Add generalized, source-agnostic pipe-to-shell / pipe-to-interpreter rules plus a process-substitution-into-shell rule. The download-anchored rules stay first so curl/wget cases keep their descriptive reason. New patterns allow an optional sudo/nohup/env/exec wrapper and /abs/path/, and use \b after the shell name so 'bash_completion' / 'script.sh | wc' don't false-positive. Also: wrap _log_audit in try/except so an audit write failure (disk full / RO fs) can't turn a just-approved command into a silent rejection, and refresh the stale D2 origin-tracking WARN comment. 11 red-team cases + 7 false-positive guards added; 139 guard tests green.
adapters/telegram_bot.py registered a tool_guard broadcaster but never a soul_review one. So on a Telegram-only deployment, ROBOOT_SOUL_REVIEW=confirm silently degraded to LOG for every Telegram-driven soul.md write: the gate found no broadcaster registered and fell back to log-and-proceed. The user believed soul.md writes were gated behind an approval modal when they were just being audited after the fact — an 'I think I'm protected but I'm not' gap. Mirror the tool_approval wiring exactly: - _broadcast_soul_review DMs the triggering user (current_tg_user) the proposed diff with an inline keyboard; bails if the contextvar is unset (let the gate time out → REJECTED) rather than DM'ing every allowed user. - soul_ok:/soul_no: callback prefixes resolve soul_review's pending future, with the same cross-user owner guard (_pending_soul_owner) as tool calls. - register the broadcaster in _post_init alongside the tool_guard one. 5 tests added (broadcaster targeting, owner binding, callback resolve, cross-user reject, end-to-end review_write round trip); 16 telegram-wiring tests green.
server.py / run.py hard-import tools.vision, which hard-imported tools.face_db, which imported numpy at module top. On the Linux CI runners (install .[telegram], NOT .[vision]) numpy is absent, so 'import server' / 'import run' would crash with ModuleNotFoundError. It only stayed green because no test imported those entry points — a latent landmine (the telegram path was already patched in f822eca, but at the consumer side, not the root). Fix at the producer: face_db imports numpy lazily inside recognize() (the only runtime user — enroll() just calls .tolist() on its arg, and the np.ndarray annotations are strings under future-annotations). Now the whole import chain is vision-extra-free. Add tests/test_import_without_vision_extras.py: a subprocess with a meta_path finder that blocks numpy/cv2/face_recognition/dlib, asserting the entry points still import — reproduces the CI environment so this can't silently regress again.
arcana-agent 1.0.0 is on PyPI — the first release with a semver promise.
Its only breaking change drops the multi-agent surface (team()/collaborate),
which Roboot has explicitly decided not to use; 0.9.0's ToolError contract
rename isn't referenced either (only appears in our comments). The whole
API surface Roboot actually depends on is intact in 1.0.0, verified by
introspection:
- arcana.Runtime / RuntimeConfig / Budget / tool
- arcana.contracts.llm.Message/MessageRole (canonical) AND
arcana.runtime.conversation.Message/MessageRole (re-export)
- arcana.ChatSession (+ new public seed_history(list[Message]|list[dict]))
- runtime._tool_gateway.confirmation_callback / .registry
Pin moved >=0.8.2,<0.9 -> >=1.0,<2. Pure dependency change, zero business
code edits. Full suite: 393 passed (370 baseline + 23 added this branch).
Lands the agent runtime on a semver-stable floor.
…fect) Two cleanups unlocked by the 1.0 upgrade: 1. memory._seed_messages now prefers the public ChatSession.seed_history() over poking the private _messages list. seed_history(list[Message]) is new in 1.0 and does exactly our cold-start restore (append after the system prompt, before any send()). The private-_messages path stays as a graceful fallback for pre-1.0 envs; the user-only replay policy (drop assistant turns that may carry hallucinated success claims) is unchanged. Collapses Roboot's dependency on an Arcana private surface to one fallback branch. 2. tools/shell.py side_effect 'read' -> 'write'. shell executes arbitrary commands; 'read' let Arcana schedule it concurrently with genuine read tools. 'write' serializes it like claude_code send/create and soul writes. The approval gate already fires via requires_confirmation either way — this just stops the metadata from claiming shell is side-effect-free. Added a test for the seed_history-preferred path. Full suite: 394 passed.
New tools/scheduler.py gives the agent schedule_reminder / list_reminders /
cancel_reminder, turning it from pure request-response into something that
can fire '15分钟后提醒我看build'. Reminders persist to .reminders.db
(sqlite, WAL, gitignored) so they survive a daemon restart — same
sync-via-to_thread shape as chat_store.
A dispatcher loop per long-running process delivers due reminders for the
surfaces it owns; origins are disjoint (daemon owns {local,relay,''},
Telegram will own {telegram}) so two dispatchers never double-fire, with an
atomic UPDATE...SET fired=1 claim as belt-and-suspenders. Recurring
reminders reschedule to the next future slot (skipping a missed backlog).
server.py starts the daemon dispatcher and delivers via the existing
local-WS + relay notify fan-out.
The wait between polls is a sleep-slice loop, NOT asyncio.wait_for(event.
wait()): the latter can leave a task un-finalized on cancellation (caught
by the dispatcher tests, which hung until this was fixed) — a bug that
would otherwise have shipped into the daemon dispatcher.
Also folds in two flagged debt items in the same file touch:
- remove the dead duplicate _relay_broadcast in server.py (the 2nd def
shadowed the 1st; prerequisite the audit called out before adding the
reminder broadcast path).
- .gitignore the at-rest/agent dirs: .reminders.db*, .tool_audit/, .claude/.
14 scheduler tests added; full suite 404 passed.
Register schedule_reminder/list_reminders/cancel_reminder in the bot's
ALL_TOOLS and start a dispatcher for origin {telegram} in _post_init.
schedule_reminder already stamps origin='telegram' + target=<user_id> (via
the current_origin / current_tg_user contextvars), so _deliver_reminder_
telegram DMs the reminder back to whoever set it. Disjoint origins from the
daemon dispatcher → no double-fire. Without this, a Telegram-set reminder
would persist but never be delivered (no dispatcher owned its origin) — a
half-feature that violates remote==local parity.
2 delivery tests added; 18 telegram tests green.
Two new capability sets, registered on both the daemon and Telegram runtimes.
tools/files.py — read_file (line-paginated, no 4000-char shell truncation),
write_file, edit_file. They bypass shell, so they enforce their own path
policy on the *resolved* absolute path (so ../ and relative paths can't dodge
it), holding even when ROBOOT_TOOL_APPROVAL=off:
- Roboot secrets are never readable/writable (config.yaml, .identity, .auth,
chat/reminder DBs, .faces, .voice_prefs, .tool_audit) — reading
config.yaml would leak API keys into the transcript.
- soul.md is readable but not writable here — self-mod must keep going
through the soul_review gate, not this raw writer.
- OS credential/system paths (~/.ssh, ~/.aws, ~/.gnupg, /etc, ...) refused.
write_file/edit_file are also added to tool_guard's always-confirm set (keyed
on the path), so CONFIRM-mode deployments get an approval modal for writes;
defense-in-depth on top of the hard deny list.
tools/web.py — web_fetch + web_search (keyless DuckDuckGo), both side_effect=
'read'. SSRF defense for an LLM that can be prompt-injected from a Telegram
message or a fetched page: scheme must be http/https and every resolved IP
must be globally routable (blocks localhost, 127/10/172.16/192.168, ::1, and
the 169.254.169.254 metadata endpoint). Redirects are followed manually,
re-validating each hop's host so a public URL can't 302 onto an internal one.
HTML is reduced to readable text (script/style stripped). httpx added as a
direct dep (was already transitive via arcana).
99 tests added across files/web/gating; full suite 456 passed.
…ed gate - Agent section: Arcana 0.8.2 -> 1.0.0, semver-stable, seed_history note. - tools/ map: add scheduler.py, files.py, web.py. - Tool Approval Gate: gated set now includes write_file/edit_file (path-keyed) and notes the base64/decode-pipe-to-shell detection added this branch.
… /var/db
Adversarial review found the deny-policy was bypassable on the default
macOS boot volume: Path.resolve() is case-PRESERVING but the FS is
case-INSENSITIVE, so 'Config.yaml' resolved to a distinct string that
missed the lower-case deny entry yet the OS opened the real config.yaml.
Exploits (all confirmed): read_file('Config.yaml') leaks API keys/bot
token into the transcript; write_file('Soul.md', …) overwrites soul.md
bypassing the soul_review gate; write_file('.Identity/daemon.ed25519.key')
bricks relay pairing; read_file('~/.SSH/id_rsa') leaks the ssh key.
Fix: every comparison in _deny_reason is now casefolded, and the
repo-membership test uses a casefolded per-component match (_repo_top) so
even a case-variant of the repo ROOT path maps in. Also add /private/var/db
alongside /var/db (the /var->/private/var symlink form was unguarded).
11 regression cases added (Config.yaml, .IDENTITY/, Soul.md, ~/.SSH, /var/db
both symlink forms); 36 files tests green.
…/6to4 Adversarial review found the SSRF guard was bypassable by DNS rebinding: _validate_url resolved the host, then httpx independently re-resolved it at connect time. An attacker controlling DNS for a domain the (prompt-injectable) agent fetches returns a public IP for the check, then 127.0.0.1 / 169.254.169.254 / an internal IP for the connect — reaching the daemon, cloud metadata, and internal services. Fix: resolve+validate ONCE per hop and pin the vetted IP for the actual connection via a custom httpx transport that rewrites the URL host to the IP while keeping the Host header and TLS SNI = the real hostname (so HTTPS cert verification is unaffected). Check-time and connect-time IP are now identical; re-pinned on every redirect hop. Also from the review: - Stream the body and stop at MAX_BODY_BYTES instead of buffering the whole response (OOM / event-loop stall on a multi-GB or slow-drip body); plus an early reject when Content-Length already exceeds the cap, and an explicit connect+read timeout. - _ip_is_safe now unwraps IPv4-mapped / NAT64 (64:ff9b::/96) / 6to4 (2002::/16) IPv6 forms and re-checks the embedded IPv4 (residual SSRF on NAT64/6to4 hosts). 6 tests added (tunnel unwrap, pinned-transport host/SNI preservation, content-length reject); 28 web tests green.
Adversarial review found three scheduler defects: 1. (high) Non-recurring reminders were marked fired=1 at CLAIM time, before delivery. If no surface was connected (the common case: set a reminder, close the tab), delivery reached zero clients, returned success, and the row stayed fired=1 forever -- the reminder was silently lost. Fix: split claim from consume. _claim_due_sync only takes the row (fired=1 to stop a second dispatcher); the new _finalize_sync decides its fate AFTER delivery. deliver() now returns whether it reached >=1 surface (server: any local WS or a paired relay client; telegram: the DM succeeded). Not delivered -> un-fire and retry on a RETRY_BACKOFF (due_at pushed forward so the dispatcher sleeps instead of hot-looping on an overdue row), bounded by MAX_RETRIES (~24h) before dropping. Survives a closed tab / offline phone and fires when it reconnects. 2. (medium) No PRAGMA busy_timeout on a deliberately multi-process DB -- a contended writer failed instantly with "database is locked". Added busy_timeout=5000 so writers wait for the lock. 3. (medium) _connect connections were never closed (a bare with-block on a sqlite connection only manages the transaction), leaking fds until cyclic GC. Every op now uses contextlib.closing(). Added attempts column + retry/backoff/give-up tests and a "retries when no surface" dispatcher test (which also caught a hot-loop in the first cut of the fix). Full suite 478 passed.
mlx-whisper hard-declares torch, but torch is used ONLY by mlx_whisper/torch_whisper.py -- the OpenAI-checkpoint -> MLX *conversion* path. Roboot downloads pre-converted `-mlx` models from HF and never converts, so the runtime STT path (load_models/transcribe/audio/decoding) imports and runs without torch (verified, plus full suite green). Exclude it with uv override-dependencies = ["torch ; sys_platform == 'never'"]. The project .venv drops 968M -> 558M (torch 353M + sympy and other torch-only transitives). Remove the override if you ever need mlx-whisper's torch conversion utility. Also gitignore .venv/ explicitly (was untracked but not listed).
The agent can now answer "我们之前聊过 X 吗 / 上次关于 Y 我怎么说的" instead of
only seeing the current turn. New @arcana.tool search_chat (tools/chat_search.py,
read-only, registered on both server + telegram) backed by an FTS5 index in
chat_store.py.
- external-content FTS5 (content='messages') over message content, tokenize=
'trigram' (the only built-in tokenizer that works on unsegmented Chinese;
unicode61 needs spaces). INSERT/DELETE/UPDATE triggers keep it in lockstep
with the base table (delete feeds old.content back, the external-content
quirk), so _purge_old and wipe_all clear the index correctly.
- search_messages(): trigram FTS for queries >=3 chars (relevance-ranked +
snippet), LIKE substring fallback for 1-2 char queries (below the trigram
floor). The query is wrapped as a quoted FTS5 phrase so user text can't be
interpreted as FTS operators (", *, OR, NEAR, column:).
- One-time backfill for a pre-existing .chat_history.db (rows that predate the
index): guarded by an fts_meta 'built' marker. NB: can't detect an empty
index via SELECT count(*) FROM messages_fts — external-content FTS proxies
count() to the content table and reports the message count even when nothing
is indexed (this bug was caught by the backfill test).
No new deps (bundled SQLite has FTS5+trigram). Read-only, not gated. 12 tests
added (Chinese/ascii match, short-query fallback, FTS-operator-injection
literal, wipe sync, backfill rebuild, tool formatting).
New tools/todos.py gives the agent a checkable todo list — soul.md is for soft knowledge, todos are discrete items. Registered on both server + telegram. - Own .todos.db, same autocommit+WAL+busy_timeout=5000+closing() shape as scheduler. add_todo / list_todos / complete_todo / cancel_todo. - A todo with due_seconds hands a reminder to the SCHEDULER's existing dispatcher (scheduler._add_sync + wake) — no new delivery path, so a due todo fires on the surface it was created from, local==remote for free. complete/cancel also cancel the linked pending reminder. - Per-origin isolation (reuses scheduler._current_origin/_current_target): list_todos shows only the caller's surface; complete/cancel are origin- guarded so a Telegram user can't check off a console todo. Security: writes are local sqlite, side_effect="write" but NOT in tool_guard's always-confirm set (benign, same as schedule_reminder). .todos.db added to tools/files.py _SECRET_REPO_PATHS so read_file/write_file can't bypass the per-origin isolation, and to .gitignore. 9 tests (add/list/validate, due->linked reminder, complete/cancel cancel the reminder, cross-origin guard, .todos.db is secret). Full suite 498 passed.
…, 4 low) HIGH — search_chat cross-user/cross-surface data exfiltration: _search_sync ran an UNSCOPED query over the single messages table, which holds every surface's transcript (local console, each relay client, each Telegram user). A Telegram user — or a prompt-injected agent — could dump the console owner's and other users' private history (pasted passwords, etc.). Now search_messages takes (source, label) and JOINs sessions to scope; search_chat derives them from the caller's origin, mapping origin 'relay'->source 'remote' (the deliberate naming mismatch) and scoping Telegram to source='telegram' AND label=<user_id>. Fails closed (unknown origin / telegram-without-user -> matches nothing). MEDIUM — every int tool param was typed "string": `from __future__ import annotations` made arcana's schema generator see the annotation as the string "int" and fall back to type:"string", so the model's numeric arg was rejected by the validator (or a string arg crashed on '600' < 0). Affected schedule_reminder/cancel_reminder, add_todo/complete_todo/cancel_todo, web_search, read_file, search_chat. Fix: drop future-annotations from the five tool modules (requires-python>=3.11, X|None is native) so params type as "integer"; plus a defensive coerce_int at the arithmetic tools. MEDIUM — cross-Telegram-user todo/reminder isolation: list/complete/cancel guarded only on origin='telegram', so user B could read/complete/cancel user A's items. Added a per-user `target` scope to scheduler._list_pending_sync/ _cancel_sync and todos._list_open_sync/_close_sync, threaded from the tools. MEDIUM — add_todo orphaned a reminder if the todo INSERT failed (reminder created first). Reordered: insert todo (reminder_id NULL) -> create reminder -> back-link; reminder failure leaves the todo with no due, never an orphan. LOW: chat_store busy_timeout=5000 (the one-time FTS rebuild holds the writer lock); _search_sync uses closing(); todo complete/cancel cancel the linked reminder best-effort with logging; files.py denies the .db -wal/-shm/-journal sidecars (write_file could corrupt a live DB). 20 regression tests added (cross-surface + per-user isolation, string-int coercion, sidecar deny). Full suite 513 passed.
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.
What this does
Upgrades Arcana to the first semver-stable release and adds five new agent tools, with two rounds of adversarial security review baked in. 16 commits, 513 pytest green, ~3.1k LOC.
Core upgrade
ee12569). Only breaking change is the multi-agent surface drop, which we never used.memory.pyswitched to the publicChatSession.seed_history();shellmoved toside_effect="write"(1959e0e).uvoverride — torch is only mlx-whisper's conversion path, never the runtime STT path. Main.venv968M → 558M (e7b9f1d).d146f24):face_dblazy-imports numpy soimport server/runwork without[vision]extras.5 new agent tools (all registered on both server + telegram surfaces)
tools/scheduler.py— one-shot + recurring reminders (.reminders.db), per-origin dispatcher, Telegram parity (reminders set via Telegram fire on Telegram). Delivery-aware retry so reminders aren't lost on no-surface.tools/files.py— read/write/edit with a casefold path deny-policy (closes the macOS case-insensitive-FS bypass). Writes go through the tool-approval gate.tools/web.py— fetch/search, SSRF-guarded (pins validated IP against DNS-rebinding, body cap, NAT64/6to4), keyless DuckDuckGo.tools/chat_search.py—search_chatFTS5 over chat_store (trigram for Chinese + LIKE fallback), per-surface scoped (telegram by user_id, fail-closed).tools/todos.py— add/list/complete/cancel, due items hand off to the scheduler dispatcher, per-origin + per-telegram-user isolation.Security (2 original High + 2 adversarial-review rounds, ~20 real bugs)
a7ec4c5tool_guard catches locally-decoded pipe-to-shell (base64 -d | sh,xxd -r | sh,cat x | sh), not justcurl | bash.0961a96Telegram registers a soul_review broadcaster (was silently degrading confirm→log).f805848): search_chat cross-user exfiltration (now scoped, fail-closed); cross-telegram-user todo/reminder isolation; add_todo orphan-reminder; busy_timeout + conn-closing.Testing
uv run pytest -q→ 513 passed. New suites: scheduler, files, web, chat_search, todos, tool_guard_telegram, import-without-vision-extras, history-replay.Notes
from __future__ import annotations— it makes the@arcana.toolschema generator seeintas a string and fall back totype:"string", breaking numeric args. Eager annotations only (fine onrequires-python>=3.11).