ApprovedCommand: typed approval token on the way to the SSH transport, plus a guard against new exec call sites #830

Goal

An AI or MCP command reaches the SSH transport only as a value that only the approval path can create. New code that runs remote commands outside the known places fails a guard test instead of compiling silently.

Context

• The approval path for AI and MCP actions is handle_action_proposed → (auto-run or handle_user_decision, crates/app-logic/src/orchestration/action_exec.rs:1455) → execute_action (:1752) → execute_suggested_command(…, command: String, …) (:1905) → transport.start_exec(run_command, …) / execute_streaming(…) (:2005, :2014). execute_action is private, so today the invariant holds because the code is clean, not because the compiler enforces it.
• The transport API takes plain strings: SshTransport::execute(&str), execute_with_stdin, execute_cancellable, execute_with_stdin_cancellable, execute_streaming, start_exec, open_sftp_via_exec (crates/core/src/ssh/transport.rs:18-120), and Session::transport is pub (crates/app-logic/src/session.rs:699). Decision is a public enum with Serialize/Deserialize (crates/core/src/filter/types.rs:23-28). Any code holding a Session can therefore run a command without the filter.
• Known exec call sites in production code (all legitimate today):
  • crates/app-logic/src/orchestration/action_exec.rs:2005,2014 – the approved AI/MCP command
  • crates/app-logic/src/orchestration/remote_files.rs:325 – execute_privileged, sudo fallback after a confirmed WriteRemoteFile
  • crates/app-shell/src/commands/connect.rs:631,643 – fixed probes uname -a, id -u
  • crates/app-shell/src/elevated_sftp.rs:402,604 – user-initiated elevated file browser probe and open_sftp_via_exec (inputs validated, is_valid_target_user)
  • crates/ssh-transport/src/local.rs – the local transport implementation itself
  • not production: crates/app-logic/src/orchestration/new_chat.rs:326 and crates/app-shell/src/commands/connect.rs:3018-3119 sit inside inline #[cfg(test)] mod tests blocks, so the scanner must ignore inline test modules (e.g. cut each file at its #[cfg(test)] module)
• The terminal (terminal_input) uses the interactive shell, not exec, and is intentionally unfiltered (security invariant 14).
• Pattern for a type that cannot be built outside its module, with compile_fail doctests: SealedContent / RedactedBlock in crates/core/src/ai/extension_content.rs:214-229.
• Existing text-scan guards for reference: crates/app-shell/src/startup_gate_wiring.rs:228-490.

Requirements

• New type ApprovedCommand in a new module crates/app-logic/src/orchestration/approval.rs: pub type, private fields (the command text that was shown/approved), no Deserialize, no Default, no From<String>/From<&str>; read access only via as_str(). Constructors are pub(super) or narrower and are called only:
  • on the auto-run branch after the escalation chain (escalate, Escalation chain: one pure escalate() function shared by the decision path and the rule-suggestion verdict #826) returned AutoExec,
  • in handle_user_decision after the user approved (including an edited command that passed the re-evaluation).
• execute_action / execute_suggested_command take ApprovedCommand instead of String for SuggestCommand; the outgoing transform (super::outgoing::prepare_for_send) and the sudo rewrite keep operating on its text exactly as today. execute_privileged (remote_files.rs:315) takes the approval of the WriteRemoteFile action as proof (e.g. an ApprovedFileWrite token or a reference to the ApprovedCommand-equivalent for file writes) – choose the smallest form that makes the call impossible without an approval and document it.
• The SshTransport trait signatures stay unchanged (implementations and test doubles in other crates exist). The visibility of Session::transport stays unchanged in this issue.
• Guard test (text scan of all non-test .rs files under crates/, test modules and tests*.rs / test_support*.rs excluded): the method calls .start_exec(, .execute_streaming(, .execute_with_stdin(, .execute_cancellable(, .execute_with_stdin_cancellable(, .open_sftp_via_exec( and transport .execute( calls (match transport receivers, e.g. transport.execute( / transport.lock().await.execute(, so SQL .execute( is not caught) may occur only in an explicit allowlist of files. Each allowlist entry carries a one-line reason. The allowlist contains the call sites listed above and nothing else.
• Comments in new code in English.

Acceptance Criteria

• compile_fail doctest: an ApprovedCommand { … } struct literal outside its module does not compile.
• compile_fail doctest: calling the ApprovedCommand constructor from outside app-logic does not compile.
• execute_suggested_command no longer accepts a String/&str command; the only way to call it is with an ApprovedCommand.
• Guard test fails when a non-allowlisted file contains .start_exec( (counter-check in the test itself, e.g. by running the scanner on an in-memory sample, or documented in the PR with the failing output).
• Guard test fails when an allowlisted file is deleted or renamed but still listed (stale entries are reported), so the list cannot rot.
• All existing action_exec/tests_*.rs, remote_files and MCP tests pass without changes to their assertions.
• .claude/skills/smart-ssh-coder/references/security-invariants.md invariant 1 names ApprovedCommand and the guard test; AGENTS.md mentions the guard where it lists the other guards (if such a list exists).
• Relevant automated tests pass.

Technical Notes

• Security-critical area (execution path): read .claude/skills/smart-ssh-coder/references/security-invariants.md (invariants 1–4, 14, 15) and AGENTS.md. Review priority elevated.
• Builds on the pure escalate() function from Escalation chain: one pure escalate() function shared by the decision path and the rule-suggestion verdict #826; the auto-run constructor call sits right after it.
• No behaviour change: every command that runs today still runs with the same text, stdin and decision.
• FilterEngine::evaluate() is not touched.
• Do not route the fixed connect probes or the elevated file browser through ApprovedCommand; they are user-initiated or constant and are covered by the allowlist with a reason.

Spec impact: none

Open questions

• Placement in core instead of app-logic: a core type would need a public constructor for app-logic to call, which defeats the purpose. Recommendation: app-logic, as described.

Related Work

• Blocked by Escalation chain: one pure escalate() function shared by the decision path and the rule-suggestion verdict #826 (pure escalate())