v0.8.68 batch: Android updater, Termux docs, perf consts, sub-agent tool sandbox

[Hmbown]

Summary

Coherent v0.8.68 milestone batch across four lanes:

#4241 — Teach updater to select Android assets on Termux

The generic format!("{prefix}-{os}-{arch}") path already produces correct Android asset stems (codewhale-android-arm64). The glibc preflight is already Linux-only via cfg!(target_os = "linux"). This adds explicit test coverage and a documentation comment — no production behavior changes.

#4238 — Make Android sandbox and secret-store behavior explicit

New docs/TERMUX.md documenting the Android platform posture: sandbox type=none (no Landlock/bwrap), file-backed secret storage fallback, approvals still apply, self-update selects Android assets. Verified against sandbox/mod.rs cfg guards, secrets crate FileKeyringStore, and update.rs preflight guard.

#4158 — Define consts for repeated string literals

Centralizes the "auto" sentinel into named constants where semantic meaning is shared: AUTO_SENTINEL in config/src/route/ids.rs (router sentinel), REASONING_EFFORT_AUTO in turn_loop.rs (reasoning tier). DEEPSEEK_* env var names unchanged.

#4042 — Sub-agent tool restriction inheritance (Phase 1)

Parent --disallowed-tools flows down to child agents by default. SpawnRequest carries disallowed_tools + inherit_disallowed_tools (default true). WorkerRuntimeProfile gains denied_tools with derive_child() union. SubAgentToolRegistry applies deny filtering in is_tool_allowed(), tools_for_model(), execute() — restricted tools never reach the model. Wildcard matching (exact, prefix *, case-insensitive). Fleet worker_runtime threads disallowed_tools.

22 new tests — all passing.

Harvested from PR #4096 by @JayBeest (rebased onto current main; original was dirty after #4262).

Verification

• cargo fmt --all --check ✅
• cargo check -p codewhale-tui --locked ✅
• cargo test -p codewhale-tui --bin codewhale-tui --locked disallowed_tools → 22 passed ✅
• cargo test -p codewhale-cli --locked android → 3 passed ✅
• cargo test -p codewhale-tui --bin codewhale-tui --locked resolve_auto_effort → 1 passed ✅

Refs #4241 Refs #4238 Refs #4158 Refs #4042

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @Hmbown's task in 4m 39s —— View job

---

Claude finished @hunter Bown's task

Todo list

• Gather context (diff, changed files, custom review instructions)
• Review crates/cli/src/update.rs (Android asset selection)
• Review docs/TERMUX.md
• Review config/src/route/ids.rs + turn_loop.rs (AUTO consts)
• Review sub-agent tool restriction inheritance (engine.rs, worker_runtime.rs, subagent/mod.rs, worker_profile.rs, tests.rs)
• Post final review summary

Summary

Reviewed all four lanes (#4241, #4238, #4158, #4042). #4241, #4238, and #4158 are clean — small, well-tested, no behavior change (#4241) or a pure rename-to-const (#4158). #4042 (sub-agent tool restriction inheritance) is the substantive change and has one bypass that undermines the feature's stated invariant.

🔴 Correctness/security — inherit_disallowed_tools: false is model-reachable and clears the parent's deny-list

crates/tui/src/tools/subagent/mod.rs:4278-4279:

if !spawn_request.inherit_disallowed_tools {
    child_runtime.worker_profile.denied_tools.clear();
}

inherit_disallowed_tools is parsed straight from the agent tool's raw JSON input (parse_spawn_request, mod.rs:6337-6340), defaulting to true only when absent. But AgentTool::input_schema() (mod.rs:3665-3739) never declares disallowed_tools or inherit_disallowed_tools as properties, and there's no additionalProperties: false or schema-stripping step anywhere in the tool-execution path (checked registry.rs, execute() call sites) — undeclared keys in a tool call still reach parse_spawn_request unfiltered. apply_session_spawn_policy and apply_spawn_profile (the two policy gates that run after parsing) never touch this field either.

Net effect: any agent — including the top-level model itself, or a prompt-injected/compromised sub-agent — can defeat the session's --disallowed-tools restriction for any child it spawns simply by including "inherit_disallowed_tools": false in its agent tool call, which isn't part of the documented/advertised schema so it wouldn't show up in a casual approval-prompt review either. This directly contradicts the invariant documented at worker_profile.rs:145-149 ("a descendant can never drop a restriction an ancestor imposed... the only way to start without the parent's list is an explicit inherit_disallowed_tools: false at spawn") — that "only way" is itself attacker/model-reachable, so the non-escalation guarantee doesn't actually hold across a spawn boundary.

There's test coverage confirming this is intentional behavior (tests.rs:7240-7259), so it reads as a deliberate escape hatch rather than an oversight — but nothing restricts who can invoke it. If this is meant for a trusted internal path (Fleet profile system, Workflow driver) rather than live model tool calls, it should be threaded that way instead of through the public agent tool's freeform input, or the option should require the caller to already hold the permission it's dropping.

Fix this →

🟡 Minor — duplicated deny-matching logic

SubAgentToolRegistry::is_tool_denied (crates/tui/src/tools/subagent/mod.rs:~7639) re-implements the exact wildcard/case-insensitive matching already in command_denies_tool (crates/tui/src/core/engine/turn_loop.rs:3111-3124) instead of calling it. Not a bug today (both are correct and tested), but the two will drift if one is changed without the other. command_denies_tool is currently pub(super) to core::engine, so reuse would need a visibility bump — worth doing given these are the two enforcement points for the same session-level concept.

What looks good

• #4241/#4158: no production behavior change, tests are accurate, AUTO_SENTINEL/REASONING_EFFORT_AUTO centralization is correctly scoped (other "auto" literals with different semantics correctly left alone).
• #4238: docs/TERMUX.md claims (sandbox type=none, Bionic vs glibc, file-backed secrets, approvals still apply) all check out against sandbox cfg guards, the secrets crate fallback, and update.rs.
• derive_child() union semantics for denied_tools (worker_profile.rs) are correct and well-tested for the inheritance path — the gap above is specifically the explicit opt-out at spawn time, not the derive/union logic.
• apply_exec_hardening's deny-list threading for Inherited tool profiles (worker_runtime.rs:698-708) correctly handles a gap filter_tool_profile can't narrow at spec time.
• Good test coverage overall (22 new tests for feat: Environment-level tool sandboxing for sub-agents (enforce tool_restrictions) #4042, 3 for v0.8.68: Teach updater to select Android assets on Termux #4241).

Note

I wasn't able to run cargo check/cargo test in this sandbox (tool execution was blocked for cargo/build commands), so I couldn't independently re-verify the PR's reported cargo test results — this review is based on static reading of the diff and surrounding code paths.

[claude]

Claude finished @Hmbown's task in 1m 53s —— View job

---

Claude finished @hunter Bown's task

Todo list

• Gather context (diff, changed files)
• Check whether prior review's finding (inherit_disallowed_tools: false bypass) was addressed
• Re-review crates/cli/src/update.rs, docs/TERMUX.md, config/src/route/ids.rs / turn_loop.rs
• Re-review sub-agent tool restriction inheritance changes
• Post final review summary

Summary

This is the same commit set (64c44a5, 9904036, 3119e87, f1d13a4) already reviewed in my prior pass — the diff against origin/main is unchanged (same 10 files, same +684/-3). I re-verified the code independently rather than assuming the earlier findings still hold.

🔴 Correctness/security — still open: inherit_disallowed_tools: false is model-reachable and clears the parent's deny-list

Confirmed still present and unaddressed:

• crates/tui/src/tools/subagent/mod.rs:4278-4280 clears child_runtime.worker_profile.denied_tools whenever spawn_request.inherit_disallowed_tools == false.
• parse_spawn_request (mod.rs:6337-6340) reads inherit_disallowed_tools/inheritDisallowedTools straight from the raw tool-call JSON, defaulting to true only when the key is absent.
• AgentTool::input_schema() (mod.rs:3658-3749) does not declare disallowed_tools or inherit_disallowed_tools as properties, and the schema has no additionalProperties: false. I checked the execution path for any schema-stripping step between tool-call parsing and parse_spawn_request — there is none, so an undeclared key still reaches this parser unfiltered.
• Neither apply_session_spawn_policy nor apply_spawn_profile (the policy gates that run after parsing) touch this field.

Net effect is unchanged from the prior review: any caller of the agent tool — including a prompt-injected or compromised sub-agent — can defeat the session's --disallowed-tools restriction for any child it spawns by passing "inherit_disallowed_tools": false, a key that isn't part of the advertised schema and so wouldn't surface in an approval-prompt review. This directly contradicts the invariant documented at worker_profile.rs:145-149 ("a descendant can never drop a restriction an ancestor imposed... the only way to start without the parent's list is an explicit inherit_disallowed_tools: false at spawn") — since that escape hatch is itself reachable by the same untrusted caller the deny-list is meant to constrain, the non-escalation guarantee doesn't hold across a spawn boundary. tests.rs:7240-7259 documents this as intentional, but nothing gates who can invoke it.

If this is meant only for a trusted internal path (Fleet profile system, Workflow driver) rather than live model tool calls, it should be threaded through that path instead of the public agent tool's freeform input — or the option should require the caller to already hold the permission it's dropping.

Fix this →

🟡 Minor — still open: duplicated deny-matching logic

SubAgentToolRegistry::is_tool_denied (mod.rs:~7639-7650) still re-implements the wildcard/case-insensitive matching already in command_denies_tool (crates/tui/src/core/engine/turn_loop.rs:3111-3124) rather than sharing it. Not a bug, but the two enforcement points will drift if one changes without the other; command_denies_tool would need a visibility bump (pub(super) → broader) to be reused directly.

What looks good (unchanged from prior pass)

• #4241 (update.rs): no production behavior change — the existing format!("{prefix}-{os}-{arch}") path already produced correct Android stems, and cfg!(target_os = "linux") already excluded Android from the glibc preflight. New tests (android_arm64_maps_to_android_release_assets, ensure_supported_release_target_accepts_android, android_release_assets_never_select_linux_arm64) are accurate and useful regression coverage.
• #4238 (docs/TERMUX.md): claims (sandbox type=none, Bionic vs glibc, file-backed secrets fallback, approvals still apply) check out against the sandbox cfg guards, secrets crate's FileKeyringStore, and update.rs's preflight guard.
• #4158: AUTO_SENTINEL (config/src/route/ids.rs) and REASONING_EFFORT_AUTO (turn_loop.rs) are behavior-preserving renames — Some(effort) if effort == REASONING_EFFORT_AUTO is equivalent to the prior Some("auto") match arm. Scoping is correct: other "auto" literals with different semantics (e.g. DEEPSEEK_* env vars) were correctly left untouched.
• #4042: derive_child() union semantics for denied_tools (worker_profile.rs) are correct and well-tested for the inheritance path — the gap is specifically the explicit opt-out at spawn time, not the derive/union logic. apply_exec_hardening's deny-list threading for Inherited tool profiles (worker_runtime.rs:698-708) correctly handles a gap filter_tool_profile can't narrow at spec time. Good test coverage overall (22 new tests for feat: Environment-level tool sandboxing for sub-agents (enforce tool_restrictions) #4042, 3 for v0.8.68: Teach updater to select Android assets on Termux #4241).

Note

I wasn't able to run cargo check/cargo test in this sandbox (cargo/build commands are blocked), so I couldn't independently re-verify the PR's reported cargo test results — this review is based on static reading of the diff and surrounding code paths, cross-checked against the actual current file contents (not just the prior review's claims).