Report shell ACL refusals

src/daemon.rs

@@ -4698,6 +4698,16 @@ async fn handle_shell_dial_socket_connection(
                 .await;
             }
             Err(error) => {
+                if mux::is_permanent_stream_denial(&error) {
+                    let message = format!("refused service \"shell\": {error}");
+                    let _ = shell::serve_shell_failure(
+                        &mut local,
+                        &message,
+                        shell::EXIT_SHELL_DISABLED,
+                    )
+                    .await;
+                    return Err(error).context("peer refused resumable shell");
+                }
                 if tunnel::is_permanent_failure(&error) {
                     return Err(error.context("peer refused resumable shell"));
                 }
@@ -4763,7 +4773,7 @@ async fn run_legacy_shell_after_selection(
         match connected {
             Ok(connection) => {
                 let (send, recv) = connection.open_bi().await?;
-                return pipe_unix_iroh(local, send, recv).await;
+                return pipe_framed_unix_iroh(local, send, recv).await;
             }
             Err(error) => {
                 let error = anyhow::Error::new(error);
@@ -5185,18 +5195,18 @@ async fn handle_raw_dial_socket_connection(
         }
     };
     if alpn == exec::EXEC_ALPN {
-        pipe_exec_unix_iroh(local, stream.send, stream.recv).await?;
+        pipe_framed_unix_iroh(local, stream.send, stream.recv).await?;
     } else {
         pipe_unix_iroh(local, stream.send, stream.recv).await?;
     }
     Ok(())
 }
 
-/// Keep receiving exec frames after the peer stops its receive direction.
+/// Keep receiving framed replies after the peer stops its receive direction.
 ///
 /// A policy refusal sends Error and Exit, then closes without reading the
-/// command frame. That stopped send direction must not cancel the useful reply.
-async fn pipe_exec_unix_iroh(
+/// request frames. That stopped send direction must not cancel the useful reply.
+async fn pipe_framed_unix_iroh(
     local: UnixStream,
     mut send: SendStream,
     mut recv: RecvStream,

src/main.rs

@@ -1018,11 +1018,14 @@ async fn main() -> Result<()> {
                     }
                 }
                 Commands::Shell { peer } => {
-                    let socket = match send_control(&home, ControlRequest::Shell { peer }).await? {
-                        ControlResponse::Shell { socket } => socket,
-                        response => bail!("unexpected daemon response: {response:?}"),
-                    };
-                    let code = run_shell_client(&socket).await?;
+                    let socket =
+                        match send_control(&home, ControlRequest::Shell { peer: peer.clone() })
+                            .await?
+                        {
+                            ControlResponse::Shell { socket } => socket,
+                            response => bail!("unexpected daemon response: {response:?}"),
+                        };
+                    let code = run_shell_client(&socket, &peer).await?;
                     std::process::exit(code);
                 }
                 Commands::Exec { peer, cmd } => {
@@ -2869,7 +2872,7 @@ async fn wait_for_daemon_ready(
     }
 }
 
-async fn run_shell_client(socket: &PathBuf) -> Result<i32> {
+async fn run_shell_client(socket: &PathBuf, peer: &str) -> Result<i32> {
     let stream = tokio::net::UnixStream::connect(socket).await?;
     let (mut read, write) = stream.into_split();
     let mut signals = ShellSignals::new()?;
@@ -2894,7 +2897,7 @@ async fn run_shell_client(socket: &PathBuf) -> Result<i32> {
 
     let mut stdout = tokio::io::stdout();
     let mut stderr = tokio::io::stderr();
-    let mut exit_code = 1;
+    let mut exit_code = None;
 
     loop {
         tokio::select! {
@@ -2908,6 +2911,9 @@ async fn run_shell_client(socket: &PathBuf) -> Result<i32> {
                         stdout.flush().await?;
                     }
                     ServerFrame::Error(message) => {
+                        stderr
+                            .write_all(format!("fabric: peer {peer:?} ").as_bytes())
+                            .await?;
                         stderr.write_all(message.as_bytes()).await?;
                         stderr.write_all(b"\n").await?;
                         stderr.flush().await?;
@@ -2918,7 +2924,7 @@ async fn run_shell_client(socket: &PathBuf) -> Result<i32> {
                         stderr.flush().await?;
                     }
                     ServerFrame::Exit(code) => {
-                        exit_code = normalize_exit_code(code);
+                        exit_code = Some(normalize_exit_code(code));
                         break;
                     }
                 }
@@ -2948,9 +2954,19 @@ async fn run_shell_client(socket: &PathBuf) -> Result<i32> {
     stdin_task.abort();
     let _ = stdin_task.await;
     terminal.restore()?;
+    if exit_code.is_none() {
+        stderr
+            .write_all(
+                format!(
+                    "fabric: peer {peer:?} closed service \"shell\" before it returned an exit status\n"
+                )
+                .as_bytes(),
+            )
+            .await?;
+    }
     stdout.flush().await?;
     stderr.flush().await?;
-    Ok(exit_code)
+    Ok(exit_code.unwrap_or(1))
 }
 
 /// Drive the client side of a `fabric exec` session over the daemon-provided

src/mux.rs

@@ -980,6 +980,10 @@ mod tests {
         ));
         assert!(is_stream_denied(&acl));
         assert!(is_permanent_stream_denial(&acl));
+
+        let transport = anyhow::anyhow!("connection timed out");
+        assert!(!is_stream_denied(&transport));
+        assert!(!is_permanent_stream_denial(&transport));
     }
 
     #[test]

src/shell.rs

@@ -26,6 +26,7 @@ const SERVER_OUTPUT: u8 = 17;
 const SERVER_EXIT: u8 = 18;
 const SERVER_ERROR: u8 = 19;
 const SERVER_STATUS: u8 = 20;
+pub(crate) const EXIT_SHELL_DISABLED: i32 = 126;
 
 #[derive(Debug)]
 pub enum ClientFrame {
@@ -46,14 +47,25 @@ pub async fn serve_shell_disabled<W>(send: &mut W) -> Result<()>
 where
     W: AsyncWrite + Unpin,
 {
-    write_server_frame(
+    serve_shell_failure(
         send,
-        ServerFrame::Error(
-            "remote shell is disabled; set allow_shell = true in peers.toml".to_string(),
-        ),
+        "refused service \"shell\": remote shell is disabled; set allow_shell = true in peers.toml",
+        EXIT_SHELL_DISABLED,
     )
-    .await?;
-    write_server_frame(send, ServerFrame::Exit(126)).await
+    .await
+}
+
+/// Send a complete failure response when a shell session cannot start.
+pub(crate) async fn serve_shell_failure<W>(
+    send: &mut W,
+    message: &str,
+    exit_code: i32,
+) -> Result<()>
+where
+    W: AsyncWrite + Unpin,
+{
+    write_server_frame(send, ServerFrame::Error(message.to_string())).await?;
+    write_server_frame(send, ServerFrame::Exit(exit_code)).await
 }
 
 pub async fn serve_shell_session<R, W>(recv: &mut R, send: &mut W, peer: &str) -> Result<()>

tests/shell.rs

@@ -75,6 +75,21 @@ impl ProtocolHandler for LegacyRawShell {
     }
 }
 
+#[derive(Debug, Clone)]
+struct LegacyDisabledShell;
+
+impl ProtocolHandler for LegacyDisabledShell {
+    async fn accept(&self, connection: Connection) -> Result<(), AcceptError> {
+        let (mut send, _recv) = connection.accept_bi().await?;
+        shell::serve_shell_disabled(&mut send)
+            .await
+            .map_err(|error| AcceptError::from_boxed(error.into_boxed_dyn_error()))?;
+        send.finish()?;
+        connection.closed().await;
+        Ok(())
+    }
+}
+
 /// The legacy ALPN is a wire contract with every released Fabric: it carries
 /// one-shot shell framing and never generic tunnel frames. A build that routes
 /// shell/0 through the tunnel session protocol makes an old peer reject the
@@ -134,6 +149,42 @@ async fn new_client_falls_back_to_legacy_raw_shell_zero() -> Result<()> {
     Ok(())
 }
 
+/// A legacy peer can send a refusal and stop reading at the same time. The
+/// stopped send direction must not cancel the Error and Exit frames already in
+/// the receive direction.
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+async fn legacy_shell_refusal_frames_survive_a_stopped_send_direction() -> Result<()> {
+    let legacy_endpoint = Endpoint::bind(presets::N0).await?;
+    let legacy = Router::builder(legacy_endpoint)
+        .accept(shell::SHELL_ALPN, LegacyDisabledShell)
+        .spawn();
+    legacy.endpoint().online().await;
+
+    let client_dir = TempDir::new()?;
+    let client_home = FabricHome::new(client_dir.path());
+    let client = FabricNode::start(client_home.clone()).await?;
+    trust_peer(
+        &client_home,
+        &client,
+        legacy.endpoint().id(),
+        Some("legacy-disabled"),
+        Some(legacy.endpoint().addr()),
+    )
+    .await?;
+
+    let output = run_shell(&client_home, "legacy-disabled", "")?;
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert_eq!(output.status.code(), Some(126), "stderr: {stderr}");
+    assert!(
+        stderr.contains("legacy-disabled") && stderr.contains("shell"),
+        "the legacy refusal omitted the peer or service: {stderr:?}"
+    );
+
+    client.shutdown().await?;
+    legacy.shutdown().await?;
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
 async fn unavailable_old_peer_later_falls_back_to_raw_shell_zero() -> Result<()> {
     let legacy_secret = iroh::SecretKey::generate();
@@ -853,6 +904,54 @@ async fn trusted_peer_without_allow_shell_is_refused() -> Result<()> {
     Ok(())
 }
 
+/// A per-peer ACL refusal happens before the resumable shell session starts.
+/// The local daemon must still send shell frames to the CLI. Otherwise the CLI
+/// returns status 1 with no reason, even though the peer is reachable.
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+async fn shell_names_the_peer_and_service_when_the_peer_acl_refuses_it() -> Result<()> {
+    let server_dir = TempDir::new()?;
+    let client_dir = TempDir::new()?;
+    let server_home = FabricHome::new(server_dir.path());
+    let client_home = FabricHome::new(client_dir.path());
+    let server = start_shell_server(server_home.clone()).await?;
+    let client = FabricNode::start(client_home.clone()).await?;
+
+    trust_peer_allowing(
+        &server_home,
+        &server,
+        client.id(),
+        Some("client"),
+        Some(client.addr()),
+        &["echo"],
+    )
+    .await?;
+    trust_peer(
+        &client_home,
+        &client,
+        server.id(),
+        Some("server"),
+        Some(server.addr()),
+    )
+    .await?;
+    assert_eq!(client.ping("server").await?.bytes, 32);
+
+    let output = run_shell(&client_home, "server", "")?;
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert_eq!(output.status.code(), Some(126), "stderr: {stderr}");
+    assert!(
+        stderr.contains("server"),
+        "the refusal omitted the peer: {stderr:?}"
+    );
+    assert!(
+        stderr.contains("shell"),
+        "the refusal omitted the service: {stderr:?}"
+    );
+
+    client.shutdown().await?;
+    server.shutdown().await?;
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
 async fn untrusted_peer_is_refused_even_when_shell_is_allowed() -> Result<()> {
     let node_a_dir = TempDir::new()?;