From 0dacf7955de5cec8e1a84cbe2d93edaf392c4bbf Mon Sep 17 00:00:00 2001
From: "@permadeath.com" <did:plc:nlzmjyfv6loqtxyzvdcznwgf>
Date: Fri, 11 Sep 2026 21:02:52 -0400
Subject: [PATCH] fix(serve): cap the e-stop admin socket's command line length

A client that never sent a newline grew a buffer here for as long as it
kept writing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Change-Id: I21865ada3c892c0627ebcb63b706a491220bcd0e
---
 crates/didbot-serve/src/estop_admin.rs | 89 ++++++++++++++++++++++++--
 1 file changed, 84 insertions(+), 5 deletions(-)

diff --git a/crates/didbot-serve/src/estop_admin.rs b/crates/didbot-serve/src/estop_admin.rs
index c33d94f0..c7d33a47 100644
--- a/crates/didbot-serve/src/estop_admin.rs
+++ b/crates/didbot-serve/src/estop_admin.rs
@@ -78,7 +78,7 @@ use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use didbot_pds::{Estop, EstopMode, ServerLifecycle};
-use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
+use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader};
 use tokio::net::{UnixListener, UnixStream};
 use tokio::task::JoinHandle;
 use tracing::{info, warn};
@@ -200,6 +200,14 @@ fn tighten_socket_permissions(path: &Path) -> std::io::Result<()> {
     std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))
 }
 
+/// The longest command line this socket reads, in bytes.
+///
+/// Every command is one short word, so the bound is generous by orders of
+/// magnitude and still keeps a client that never sends a newline from growing
+/// a buffer here. Reaching this socket already takes host access, so this is
+/// a bound on a mistake as much as on anything else.
+const MAX_COMMAND_LINE: usize = 1024;
+
 async fn serve(
     listener: UnixListener,
     estop: Arc<Estop>,
@@ -232,13 +240,39 @@ async fn handle_connection(
     relay: Option<&RelayTarget>,
 ) -> std::io::Result<()> {
     let (reader, mut writer) = stream.into_split();
-    let mut lines = BufReader::new(reader).lines();
-    while let Some(line) = lines.next_line().await? {
-        let reply = handle_command(line.trim(), estop, Some(lifecycle), relay).await;
+    let mut reader = BufReader::new(reader);
+    let mut line = Vec::new();
+    loop {
+        line.clear();
+        // One byte past the bound, so a line that reaches that length is
+        // known to have exceeded it without ever holding more than it.
+        let read = (&mut reader)
+            .take(MAX_COMMAND_LINE as u64 + 1)
+            .read_until(b'\n', &mut line)
+            .await?;
+        if read == 0 {
+            return Ok(());
+        }
+        if line.len() > MAX_COMMAND_LINE {
+            warn!(
+                max = MAX_COMMAND_LINE,
+                "an e-stop admin client sent a command line over the length bound"
+            );
+            writer
+                .write_all(
+                    format!("ERR a command line is at most {MAX_COMMAND_LINE} bytes\n").as_bytes(),
+                )
+                .await?;
+            // Closed rather than resynchronised: the rest of that line is
+            // whatever the client was sending, and reading on would parse it
+            // as the commands that follow.
+            return Ok(());
+        }
+        let text = String::from_utf8_lossy(&line);
+        let reply = handle_command(text.trim(), estop, Some(lifecycle), relay).await;
         writer.write_all(reply.as_bytes()).await?;
         writer.write_all(b"\n").await?;
     }
-    Ok(())
 }
 
 /// Handles one command line, returning the reply line (without its `\n`).
@@ -680,6 +714,51 @@ mod tests {
         std::fs::remove_dir_all(&dir).ok();
     }
 
+    /// A client that sends and sends without ever ending the line is refused
+    /// at the bound and hung up on, rather than growing a buffer here for as
+    /// long as it cares to keep typing.
+    #[tokio::test]
+    async fn a_command_line_past_the_bound_is_refused_and_the_connection_closed() {
+        let dir = TempDir(std::env::temp_dir().join(format!(
+            "didbot-estop-admin-bound-test-{}-{}",
+            std::process::id(),
+            line!()
+        )));
+        std::fs::create_dir_all(&dir.0).unwrap();
+        let path = dir.0.join("estop.sock");
+
+        let estop = Arc::new(Estop::default());
+        let admin = spawn(&path, estop.clone(), ready_lifecycle(), None).expect("binds");
+
+        let stream = UnixStream::connect(&path).await.expect("connects");
+        let (reader, mut writer) = stream.into_split();
+        let mut lines = BufReader::new(reader).lines();
+
+        // A command that has not ended, and by now never will.
+        writer
+            .write_all(&vec![b'A'; MAX_COMMAND_LINE + 1])
+            .await
+            .unwrap();
+        let reply = lines
+            .next_line()
+            .await
+            .unwrap()
+            .expect("the socket answers rather than reading on");
+        assert!(
+            reply.starts_with("ERR a command line is at most"),
+            "a line past the bound was taken as a command: {reply}"
+        );
+        assert_eq!(
+            lines.next_line().await.unwrap(),
+            None,
+            "the connection stayed open after a line past the bound"
+        );
+        assert_eq!(estop.status().mode, None, "and nothing on it was run");
+
+        drop(writer);
+        admin.stop().await;
+    }
+
     /// A directory `permissions_of` reads back, cleaned up on drop so a
     /// failing assertion does not leak a fixture into the next test run.
     struct TempDir(PathBuf);
-- 
2.51.2