Skip to content

fix(agent): fail-closed subagent tool allowlist (tinyhumansai#4452) - #9

Closed
senamakel wants to merge 1 commit into
agent/vendor-tinyagentsfrom
agent/fix-4452
Closed

senamakel wants to merge 1 commit into
agent/vendor-tinyagentsfrom
agent/fix-4452

Conversation

@senamakel

Copy link
Copy Markdown
Owner

Summary

Fixes a fail-OPEN sub-agent tool allowlist (prompt-injection privilege escalation). Source issue: tinyhumansai#4452.

The registration predicate in src/openhuman/tinyagents/mod.rs was allowed.is_empty() || allowed.contains(name) — an empty allowlist registered every tool across tool_sets (which includes the parent's shell, file-write, and spawn/delegate tools). A deliberately tool-less sub-agent (tools = [] archetype, a zero-match skill_filter, or a named list of unresolvable names) therefore silently inherited the parent's full surface.

Fix

  • Plumb the allowlist as Option<HashSet<String>> end-to-end (subagent_runner/ops/runner.rs → run_subagent_via_graph / AgentTurnRequest → assemble_turn_harness → run_turn_via_tinyagents_shared), so registration distinguishes:
    • None → no filter supplied, register every candidate (parent/channel/chat turns);
    • Some(set) → register only names in set;
    • Some({}) → register nothing (deny-all).
  • Sub-agents always resolve to Some(set). ToolScope::Named([]) and zero-match skill_filter produce Some(empty) = deny-all (v0.58.7 parity). Parent channel/chat turns map an empty visibility set to None (all tools), preserving current behavior there.
  • Defense in depth: strip spawn/delegate names (is_subagent_spawn_tool + spawn_worker_thread) at registration regardless of the allowlist, so the "sub-agents must never spawn" invariant holds even under a widened/None allowlist.
  • Warn-log ([subagent] tool allowlist resolved empty …) when the resolved allowlist is empty.
  • The shadow tool-exposure middleware mirrors the same None/Some(set)/Some(empty) semantics.

Acceptance criteria

  • tools = [] / zero-match filters register zero tools; the sub-agent still completes text-only turns.
  • No sub-agent configuration can implicitly inherit the parent's full tool surface.
  • Spawn/delegate tools are never registered to a sub-agent regardless of allowlist content.
  • Unit tests over the registration gate for None, Some(empty), Some({named}), named-but-unresolvable, plus spawn-always-stripped; e2e that a Some(empty) sub-agent over a shell-bearing surface has no shell.

Verified

  • cargo check --manifest-path Cargo.toml --lib → clean finish (EXIT=0).
  • cargo check --manifest-path Cargo.toml --lib --tests → clean finish (EXIT=0), so the new tests compile.
  • cargo fmt applied.

Not verified

  • Tests were not executed here (cargo test link step is too heavy for this memory-constrained box); relying on PR CI to run them. They are written and compile.
  • No changes to vendor/tinyagents (submodule pointer unchanged); the fix lives entirely in the openhuman seam.

https://claude.ai/code/session_019j5TLsRLHsM3kqAYFyH4hR

An empty sub-agent tool allowlist was fail-OPEN: the registration predicate
`allowed.is_empty() || allowed.contains(name)` treated an empty set as "all
tools visible", so a `tools = []` archetype, a zero-match `skill_filter`, or a
`named` list of unresolvable names silently inherited the parent's FULL surface
(shell, file-write, spawn/delegate) — a prompt-injection privilege escalation
for a deliberately tool-less summarizer processing untrusted content.

Change the plumbed allowlist type from `HashSet<String>` to
`Option<HashSet<String>>` end-to-end (subagent_runner ops/runner.rs ->
run_subagent_via_graph / AgentTurnRequest -> assemble_turn_harness ->
run_turn_via_tinyagents_shared) so registration distinguishes:
  * None      -> no filter supplied, register every candidate (parent/channel/
                 chat turns);
  * Some(set) -> register ONLY names in `set`;
  * Some({})  -> register NOTHING (deny-all).

Sub-agents always resolve to `Some(set)` (never None): `ToolScope::Named([])`
and a zero-match `skill_filter` now produce `Some(empty)` = deny-all, matching
v0.58.7 semantics. Parent channel/chat turns map an empty visibility set to
`None` (all tools), preserving today's behavior on those paths.

Defense in depth: strip spawn/delegate tool names (is_subagent_spawn_tool +
`spawn_worker_thread`) at registration regardless of the allowlist, so the
"sub-agents must never spawn" invariant holds even under a widened/None
allowlist — not only via `allowed_indices`. Warn-log when the resolved
allowlist is empty so misconfigured agents are diagnosable.

The shadow tool-exposure middleware mirrors the same None/Some(set)/Some(empty)
semantics.

Tests: unit tests over the registration gate (None / Some(empty) /
Some({named}) / named-unresolvable / spawn-always-stripped) plus an e2e that a
tool-less (`Some(empty)`) sub-agent over a shell-bearing surface registers no
tools yet completes a text-only turn.

Claude-Session: https://claude.ai/code/session_019j5TLsRLHsM3kqAYFyH4hR
@cursor

cursor Bot commented Jul 4, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@senamakel senamakel closed this Jul 4, 2026
@senamakel
senamakel deleted the agent/fix-4452 branch July 4, 2026 02:48
senamakel pushed a commit that referenced this pull request Oct 16, 2026
- #6: the service trusts every files folder (`tiny_store::file_roots`), so
  an artifact made in an earlier one can still be revised.
- #8: a failed revision keeps the artifact: the record keeps its versions
  and the card shows the newest of them again.
- #9: a failure the observer could not show fails the card directly, and a
  completion that records nothing fails the record too; no card stays
  "generating".
- #10: listing a thread's artifacts (the row backfill, the UI) queries the
  `by_thread` index instead of reading the whole workspace.
- #4: the open preview re-reads the artifact when a new version is saved
  (an agent revision) unless an edit is pending; the panel is keyed per
  artifact.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant