- add cargo fmt / clippy to actions build. - add common unit tests. -

improved security sanitization - security spcecific unit tests - add
unit tests to workflow build - add unami analytics.

src/analytics.rs

@@ -0,0 +1,287 @@
+// Copyright (c) 2024 Jason Witty <jasonpwitty+socktop@proton.me>.
+// All rights reserved.
+//
+// Umami analytics integration for tracking terminal events
+
+use anyhow::Result;
+use std::sync::Arc;
+use tokio::sync::Mutex;
+use umami_metrics::Umami;
+
+/// Umami analytics tracker
+pub struct Analytics {
+    client: Arc<Mutex<Option<Umami>>>,
+    enabled: bool,
+}
+
+impl Analytics {
+    /// Create a new Analytics instance
+    ///
+    /// # Arguments
+    /// * `website_id` - The Umami website ID
+    /// * `endpoint` - The Umami instance endpoint (e.g., "http://unami.wittyoneoff.com")
+    pub fn new(website_id: String, endpoint: String) -> Self {
+        let client = Umami::new(website_id, endpoint);
+
+        Self {
+            client: Arc::new(Mutex::new(Some(client))),
+            enabled: true,
+        }
+    }
+
+    /// Create a disabled Analytics instance (no-op)
+    pub fn disabled() -> Self {
+        Self {
+            client: Arc::new(Mutex::new(None)),
+            enabled: false,
+        }
+    }
+
+    /// Track a terminal command event
+    ///
+    /// # Arguments
+    /// * `command` - The command that was typed (will be sanitized)
+    /// * `user_agent` - Optional user agent string
+    pub async fn track_command(&self, command: &str, user_agent: Option<String>) -> Result<()> {
+        if !self.enabled {
+            return Ok(());
+        }
+
+        let client = self.client.lock().await;
+
+        if let Some(umami) = client.as_ref() {
+            // Sanitize the command for analytics
+            let sanitized_command = sanitize_command(command);
+
+            let ua = user_agent.unwrap_or_else(|| "unknown".to_string());
+
+            // Track as an event
+            match umami
+                .event(
+                    "/terminal".to_string(),
+                    "command_typed".to_string(),
+                    ua,
+                    "unknown".to_string(), // hostname
+                    "unknown".to_string(), // language
+                    sanitized_command,     // event_data (the command)
+                )
+                .await
+            {
+                Ok(_) => log::debug!("Tracked command event"),
+                Err(e) => log::warn!("Failed to track command event: {:?}", e),
+            }
+        }
+
+        Ok(())
+    }
+
+    /// Track a page view
+    ///
+    /// # Arguments
+    /// * `path` - The page path
+    /// * `user_agent` - Optional user agent string
+    pub async fn track_pageview(&self, path: &str, user_agent: Option<String>) -> Result<()> {
+        if !self.enabled {
+            return Ok(());
+        }
+
+        let client = self.client.lock().await;
+
+        if let Some(umami) = client.as_ref() {
+            let ua = user_agent.unwrap_or_else(|| "unknown".to_string());
+
+            match umami
+                .pageview(
+                    path.to_string(),
+                    "pageview".to_string(),
+                    ua,
+                    "unknown".to_string(), // hostname
+                    "unknown".to_string(), // language
+                )
+                .await
+            {
+                Ok(_) => log::debug!("Tracked pageview: {}", path),
+                Err(e) => log::warn!("Failed to track pageview: {:?}", e),
+            }
+        }
+
+        Ok(())
+    }
+
+    /// Track a terminal session start
+    pub async fn track_session_start(&self, user_agent: Option<String>) -> Result<()> {
+        if !self.enabled {
+            return Ok(());
+        }
+
+        let client = self.client.lock().await;
+
+        if let Some(umami) = client.as_ref() {
+            let ua = user_agent.unwrap_or_else(|| "unknown".to_string());
+
+            match umami
+                .event(
+                    "/terminal".to_string(),
+                    "session_start".to_string(),
+                    ua,
+                    "unknown".to_string(),
+                    "unknown".to_string(),
+                    "terminal_session".to_string(),
+                )
+                .await
+            {
+                Ok(_) => log::debug!("Tracked session start"),
+                Err(e) => log::warn!("Failed to track session start: {:?}", e),
+            }
+        }
+
+        Ok(())
+    }
+
+    /// Track a terminal session end
+    pub async fn track_session_end(&self, user_agent: Option<String>) -> Result<()> {
+        if !self.enabled {
+            return Ok(());
+        }
+
+        let client = self.client.lock().await;
+
+        if let Some(umami) = client.as_ref() {
+            let ua = user_agent.unwrap_or_else(|| "unknown".to_string());
+
+            match umami
+                .event(
+                    "/terminal".to_string(),
+                    "session_end".to_string(),
+                    ua,
+                    "unknown".to_string(),
+                    "unknown".to_string(),
+                    "terminal_session".to_string(),
+                )
+                .await
+            {
+                Ok(_) => log::debug!("Tracked session end"),
+                Err(e) => log::warn!("Failed to track session end: {:?}", e),
+            }
+        }
+
+        Ok(())
+    }
+}
+
+impl Clone for Analytics {
+    fn clone(&self) -> Self {
+        Self {
+            client: Arc::clone(&self.client),
+            enabled: self.enabled,
+        }
+    }
+}
+
+/// Sanitize a command for analytics tracking
+///
+/// This removes potentially sensitive information like:
+/// - Passwords in commands (e.g., mysql -p password)
+/// - URLs with credentials
+/// - SSH keys
+/// - File paths (replaced with generic placeholders)
+///
+/// Returns a sanitized version of the command safe for analytics
+fn sanitize_command(command: &str) -> String {
+    let trimmed = command.trim();
+
+    // If empty, return as-is
+    if trimmed.is_empty() {
+        return "empty".to_string();
+    }
+
+    // Split into words
+    let words: Vec<&str> = trimmed.split_whitespace().collect();
+
+    if words.is_empty() {
+        return "empty".to_string();
+    }
+
+    // Get the base command (first word)
+    let base_cmd = words[0];
+
+    // For sensitive commands, only track the command name
+    let sensitive_commands = [
+        "ssh", "scp", "sftp", "rsync", "mysql", "psql", "mongo", "curl", "wget", "git", "docker",
+        "kubectl", "aws", "gcloud", "sudo", "su", "passwd", "chpasswd", "openssl", "gpg",
+    ];
+
+    if sensitive_commands.iter().any(|&cmd| base_cmd.contains(cmd)) {
+        return format!("{} [REDACTED]", base_cmd);
+    }
+
+    // For common safe commands, keep the command and count of args
+    let safe_commands = [
+        "ls", "cd", "pwd", "cat", "less", "more", "head", "tail", "echo", "grep", "find", "which",
+        "whoami", "date", "cal", "clear", "exit", "history", "man", "help", "top", "htop", "ps",
+        "kill", "df", "du", "free", "uptime", "uname",
+    ];
+
+    if safe_commands.contains(&base_cmd) {
+        if words.len() > 1 {
+            return format!("{} +{} args", base_cmd, words.len() - 1);
+        } else {
+            return base_cmd.to_string();
+        }
+    }
+
+    // For other commands, just return the base command
+    base_cmd.to_string()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_sanitize_empty() {
+        assert_eq!(sanitize_command(""), "empty");
+        assert_eq!(sanitize_command("   "), "empty");
+    }
+
+    #[test]
+    fn test_sanitize_safe_commands() {
+        assert_eq!(sanitize_command("ls"), "ls");
+        assert_eq!(sanitize_command("ls -la"), "ls +1 args");
+        assert_eq!(sanitize_command("cd /tmp"), "cd +1 args");
+        assert_eq!(sanitize_command("pwd"), "pwd");
+    }
+
+    #[test]
+    fn test_sanitize_sensitive_commands() {
+        assert_eq!(sanitize_command("ssh user@host"), "ssh [REDACTED]");
+        assert_eq!(
+            sanitize_command("mysql -u root -p password"),
+            "mysql [REDACTED]"
+        );
+        assert_eq!(
+            sanitize_command("curl https://api.com/secret"),
+            "curl [REDACTED]"
+        );
+        assert_eq!(sanitize_command("sudo rm -rf /"), "sudo [REDACTED]");
+    }
+
+    #[test]
+    fn test_sanitize_unknown_commands() {
+        assert_eq!(sanitize_command("customcmd arg1 arg2"), "customcmd");
+        assert_eq!(sanitize_command("./script.sh"), "./script.sh");
+    }
+
+    #[test]
+    fn test_analytics_disabled() {
+        let analytics = Analytics::disabled();
+        assert!(!analytics.enabled);
+    }
+
+    #[tokio::test]
+    async fn test_track_command_disabled() {
+        let analytics = Analytics::disabled();
+        let result = analytics.track_command("ls -la", None).await;
+        assert!(result.is_ok());
+    }
+}

src/lib.rs

@@ -47,16 +47,24 @@ const CLIENT_TIMEOUT: Duration = Duration::from_secs(10);
 const IDLE_TIMEOUT: Duration = Duration::from_secs(300); // 5 minutes
 const IDLE_CHECK_INTERVAL: Duration = Duration::from_secs(30); // Check every 30 seconds
 
-mod event;
-mod terminado;
+pub mod analytics;
+pub mod event;
+pub mod security;
+pub mod terminado;
 
 use event::{ChildDied, TerminadoMessage, IO};
 
+// Re-export for public API
+pub use analytics::Analytics;
+pub use security::{validate_command, validate_env_value, ValidationError};
+pub use terminado::ParseError;
+
 /// Actix WebSocket actor
 pub struct Websocket {
     cons: Option<Addr<Terminal>>,
     hb: Instant,
     command: Option<Command>,
+    analytics: Option<Analytics>,
 }
 
 impl Actor for Websocket {
@@ -71,8 +79,14 @@ impl Actor for Websocket {
             .take()
             .expect("command was None at start of WebSocket.");
 
-        // Start PTY
-        self.cons = Some(Terminal::new(ctx.address(), command).start());
+        // Start PTY with analytics if available
+        let terminal = if let Some(analytics) = self.analytics.clone() {
+            Terminal::with_analytics(ctx.address(), command, analytics)
+        } else {
+            Terminal::new(ctx.address(), command)
+        };
+
+        self.cons = Some(terminal.start());
 
         log::trace!("Started WebSocket");
     }
@@ -127,6 +141,16 @@ impl Websocket {
             hb: Instant::now(),
             cons: None,
             command: Some(command),
+            analytics: None,
+        }
+    }
+
+    pub fn with_analytics(command: Command, analytics: Analytics) -> Self {
+        Self {
+            hb: Instant::now(),
+            cons: None,
+            command: Some(command),
+            analytics: Some(analytics),
         }
     }
 
@@ -204,6 +228,7 @@ pub struct Terminal {
     command: Command,
     last_activity: Instant,
     idle_timeout: Duration,
+    analytics: Option<Analytics>,
 }
 
 impl Terminal {
@@ -216,6 +241,20 @@ impl Terminal {
             command,
             last_activity: Instant::now(),
             idle_timeout: IDLE_TIMEOUT,
+            analytics: None,
+        }
+    }
+
+    pub fn with_analytics(ws: Addr<Websocket>, command: Command, analytics: Analytics) -> Self {
+        Self {
+            pty_master: None,
+            pty_writer: None,
+            child: None,
+            ws,
+            command,
+            last_activity: Instant::now(),
+            idle_timeout: IDLE_TIMEOUT,
+            analytics: Some(analytics),
         }
     }
 }
@@ -378,6 +417,15 @@ impl Handler<TerminadoMessage> for Terminal {
                 // Reset idle timer on user input
                 self.last_activity = Instant::now();
 
+                // Track command in analytics
+                if let Some(analytics) = &self.analytics {
+                    let command = String::from_utf8_lossy(&io.0).to_string();
+                    let analytics_clone = analytics.clone();
+                    actix::spawn(async move {
+                        let _ = analytics_clone.track_command(&command, None).await;
+                    });
+                }
+
                 let writer = match &mut self.pty_writer {
                     Some(w) => w,
                     None => {
@@ -459,10 +507,19 @@ where
     {
         self.route(
             endpoint,
-            web::get().to(move |req: HttpRequest, stream: web::Payload| {
-                let cmd = handler(&req);
-                async move { ws::start(Websocket::new(cmd), &req, stream) }
-            }),
+            web::get().to(
+                move |req: HttpRequest,
+                      stream: web::Payload,
+                      analytics: Option<web::Data<Analytics>>| {
+                    let cmd = handler(&req);
+                    let ws = if let Some(analytics_data) = analytics {
+                        Websocket::with_analytics(cmd, analytics_data.as_ref().clone())
+                    } else {
+                        Websocket::new(cmd)
+                    };
+                    async move { ws::start(ws, &req, stream) }
+                },
+            ),
         )
     }
 

src/security.rs

@@ -0,0 +1,283 @@
+// Copyright (c) 2024 Jason Witty <jasonpwitty+socktop@proton.me>.
+// All rights reserved.
+//
+// Security validation for command execution
+
+use std::path::Path;
+
+/// Error type for command validation failures
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ValidationError {
+    pub message: String,
+}
+
+impl ValidationError {
+    fn new(message: impl Into<String>) -> Self {
+        Self {
+            message: message.into(),
+        }
+    }
+}
+
+impl std::fmt::Display for ValidationError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "Command validation error: {}", self.message)
+    }
+}
+
+impl std::error::Error for ValidationError {}
+
+/// Whitelist of allowed shell commands
+const ALLOWED_SHELLS: &[&str] = &[
+    "/bin/sh",
+    "/bin/bash",
+    "/bin/zsh",
+    "/bin/dash",
+    "/usr/bin/bash",
+    "/usr/bin/zsh",
+    "/usr/bin/fish",
+];
+
+/// Maximum allowed command path length
+const MAX_COMMAND_LENGTH: usize = 4096;
+
+/// Validates a command path for security concerns
+///
+/// # Security Checks
+/// - Must be an absolute path
+/// - Must not contain path traversal sequences (..)
+/// - Must not contain shell metacharacters
+/// - Must not contain null bytes
+/// - Must be ASCII only
+/// - Must not be in user-writable directories
+/// - Must be in the whitelist (if whitelist checking is enabled)
+/// - Must not exceed maximum length
+///
+/// # Example
+/// ```
+/// use webterm::security::validate_command;
+///
+/// // Valid command
+/// assert!(validate_command("/bin/sh", true).is_ok());
+///
+/// // Invalid: shell injection attempt
+/// assert!(validate_command("/bin/sh; rm -rf /", true).is_err());
+///
+/// // Invalid: path traversal
+/// assert!(validate_command("../../bin/sh", true).is_err());
+/// ```
+pub fn validate_command(command: &str, check_whitelist: bool) -> Result<(), ValidationError> {
+    // Check for empty command
+    if command.is_empty() {
+        return Err(ValidationError::new("Command cannot be empty"));
+    }
+
+    // Check length
+    if command.len() > MAX_COMMAND_LENGTH {
+        return Err(ValidationError::new(format!(
+            "Command path too long: {} bytes (max: {})",
+            command.len(),
+            MAX_COMMAND_LENGTH
+        )));
+    }
+
+    // Must be absolute path
+    if !command.starts_with('/') {
+        return Err(ValidationError::new(
+            "Command must be an absolute path starting with '/'",
+        ));
+    }
+
+    // Check for path traversal
+    if command.contains("..") {
+        return Err(ValidationError::new(
+            "Command path contains '..' (path traversal attempt)",
+        ));
+    }
+
+    // Check for shell metacharacters
+    let dangerous_chars = [';', '&', '|', '`', '$', '\n', '\r', '\0', '<', '>'];
+    for ch in dangerous_chars {
+        if command.contains(ch) {
+            return Err(ValidationError::new(format!(
+                "Command contains dangerous character: {:?}",
+                ch
+            )));
+        }
+    }
+
+    // Check for null bytes
+    if command.contains('\0') {
+        return Err(ValidationError::new("Command contains null byte"));
+    }
+
+    // Must be ASCII only (avoid Unicode tricks)
+    if !command.is_ascii() {
+        return Err(ValidationError::new("Command must be ASCII only"));
+    }
+
+    // Check for control characters (except common ones that might be in paths)
+    for ch in command.chars() {
+        if ch.is_control() {
+            return Err(ValidationError::new(format!(
+                "Command contains control character: {:?}",
+                ch
+            )));
+        }
+    }
+
+    // Must not be in user-writable directories
+    let dangerous_prefixes = ["/tmp/", "/var/tmp/", "/home/", "/Users/", "/root/"];
+    for prefix in dangerous_prefixes {
+        if command.starts_with(prefix) {
+            return Err(ValidationError::new(format!(
+                "Command in user-writable directory: {}",
+                prefix
+            )));
+        }
+    }
+
+    // Check against whitelist if enabled
+    if check_whitelist && !ALLOWED_SHELLS.contains(&command) {
+        return Err(ValidationError::new(format!(
+            "Command '{}' not in whitelist. Allowed: {:?}",
+            command, ALLOWED_SHELLS
+        )));
+    }
+
+    // Verify the command exists (if not checking whitelist, we should at least verify it's a file)
+    if !check_whitelist {
+        let path = Path::new(command);
+        if !path.exists() {
+            return Err(ValidationError::new(format!(
+                "Command path does not exist: {}",
+                command
+            )));
+        }
+        if !path.is_file() {
+            return Err(ValidationError::new(format!(
+                "Command path is not a file: {}",
+                command
+            )));
+        }
+    }
+
+    Ok(())
+}
+
+/// Validates an environment variable value for security
+///
+/// # Security Checks
+/// - Must not contain shell metacharacters
+/// - Must not contain null bytes
+/// - Must not contain newlines
+/// - Must be reasonable length
+///
+/// # Example
+/// ```
+/// use webterm::security::validate_env_value;
+///
+/// assert!(validate_env_value("xterm").is_ok());
+/// assert!(validate_env_value("xterm; rm -rf /").is_err());
+/// ```
+pub fn validate_env_value(value: &str) -> Result<(), ValidationError> {
+    // Check length
+    if value.len() > 4096 {
+        return Err(ValidationError::new("Environment value too long"));
+    }
+
+    // Check for dangerous characters
+    let dangerous_chars = [';', '&', '|', '`', '$', '\n', '\r', '\0'];
+    for ch in dangerous_chars {
+        if value.contains(ch) {
+            return Err(ValidationError::new(format!(
+                "Environment value contains dangerous character: {:?}",
+                ch
+            )));
+        }
+    }
+
+    Ok(())
+}
+
+/// Returns the list of allowed shells
+pub fn allowed_shells() -> &'static [&'static str] {
+    ALLOWED_SHELLS
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_valid_command() {
+        assert!(validate_command("/bin/sh", true).is_ok());
+        assert!(validate_command("/bin/bash", true).is_ok());
+    }
+
+    #[test]
+    fn test_reject_relative_path() {
+        assert!(validate_command("bin/sh", true).is_err());
+        assert!(validate_command("./bin/sh", true).is_err());
+    }
+
+    #[test]
+    fn test_reject_path_traversal() {
+        assert!(validate_command("/../bin/sh", true).is_err());
+        assert!(validate_command("/bin/../sh", true).is_err());
+    }
+
+    #[test]
+    fn test_reject_shell_metacharacters() {
+        assert!(validate_command("/bin/sh;", true).is_err());
+        assert!(validate_command("/bin/sh&", true).is_err());
+        assert!(validate_command("/bin/sh|", true).is_err());
+        assert!(validate_command("/bin/sh`", true).is_err());
+        assert!(validate_command("/bin/sh$", true).is_err());
+    }
+
+    #[test]
+    fn test_reject_null_bytes() {
+        assert!(validate_command("/bin/sh\0", true).is_err());
+    }
+
+    #[test]
+    fn test_reject_tmp_directory() {
+        assert!(validate_command("/tmp/malicious.sh", true).is_err());
+        assert!(validate_command("/var/tmp/evil.sh", true).is_err());
+    }
+
+    #[test]
+    fn test_reject_non_ascii() {
+        assert!(validate_command("/bin/sh™", true).is_err());
+    }
+
+    #[test]
+    fn test_reject_empty_command() {
+        assert!(validate_command("", true).is_err());
+    }
+
+    #[test]
+    fn test_reject_too_long() {
+        let long_command = format!("/{}", "a".repeat(5000));
+        assert!(validate_command(&long_command, true).is_err());
+    }
+
+    #[test]
+    fn test_whitelist_enforcement() {
+        assert!(validate_command("/bin/sh", true).is_ok());
+        assert!(validate_command("/usr/local/bin/custom", true).is_err());
+    }
+
+    #[test]
+    fn test_valid_env_value() {
+        assert!(validate_env_value("xterm").is_ok());
+        assert!(validate_env_value("xterm-256color").is_ok());
+    }
+
+    #[test]
+    fn test_reject_env_with_metacharacters() {
+        assert!(validate_env_value("xterm; rm -rf /").is_err());
+        assert!(validate_env_value("xterm && curl evil.com").is_err());
+    }
+}

src/server.rs

@@ -1,6 +1,6 @@
 use actix_web::{App, HttpServer};
 use clap::Parser;
-use webterm::WebTermExt;
+use webterm::{validate_command, Analytics, WebTermExt};
 
 use std::process::Command;
 
@@ -19,6 +19,18 @@ struct Opt {
     /// The command to execute
     #[arg(short, long, default_value = "/bin/sh")]
     command: String,
+
+    /// Enable Umami analytics tracking
+    #[arg(long, default_value = "true")]
+    enable_analytics: bool,
+
+    /// Umami instance endpoint
+    #[arg(long, default_value = "http://unami.wittyoneoff.com")]
+    umami_endpoint: String,
+
+    /// Umami website ID
+    #[arg(long, default_value = "caefa16f-86af-4835-8b82-c8649aea0e2a")]
+    umami_website_id: String,
 }
 
 #[actix_web::main]
@@ -27,6 +39,15 @@ async fn main() -> std::io::Result<()> {
 
     let opt = Opt::parse();
 
+    // Validate command for security before starting server
+    if let Err(e) = validate_command(&opt.command, false) {
+        eprintln!("Error: Invalid command '{}': {}", opt.command, e);
+        eprintln!("Command failed security validation.");
+        std::process::exit(1);
+    }
+
+    log::info!("Command validated: {}", opt.command);
+
     // Normalize common hostnames that sometimes resolve to IPv6-only addresses
     // which can cause platform-specific bind failures. Mapping `localhost` to
     // 127.0.0.1 makes behavior predictable on systems where `::1` would otherwise
@@ -40,10 +61,24 @@ async fn main() -> std::io::Result<()> {
     let bind_addr = format!("{}:{}", host, opt.port);
     println!("Starting webterm server on http://{}", bind_addr);
 
+    // Initialize analytics
+    let analytics = if opt.enable_analytics {
+        log::info!(
+            "Analytics enabled: {} (website_id: {})",
+            opt.umami_endpoint,
+            opt.umami_website_id
+        );
+        Analytics::new(opt.umami_website_id.clone(), opt.umami_endpoint.clone())
+    } else {
+        log::info!("Analytics disabled");
+        Analytics::disabled()
+    };
+
     let command = opt.command.clone();
 
     HttpServer::new(move || {
         let cmd = command.clone();
+        let analytics_clone = analytics.clone();
         App::new()
             .service(actix_files::Files::new("/assets", "./static"))
             .service(actix_files::Files::new("/static", "./node_modules"))
@@ -53,6 +88,7 @@ async fn main() -> std::io::Result<()> {
                 command
             })
             .webterm_ui("/", "/websocket", "/static")
+            .app_data(actix_web::web::Data::new(analytics_clone.clone()))
     })
     .bind(&bind_addr)?
     .run()

src/terminado.rs

@@ -4,12 +4,34 @@ use log::error;
 use libc::c_ushort;
 
 use std::convert::TryFrom;
+use std::fmt;
 
 use serde::ser::SerializeSeq;
 use serde::{Serialize, Serializer};
 
 use crate::event::IO;
 
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ParseError {
+    message: String,
+}
+
+impl ParseError {
+    fn new(message: impl Into<String>) -> Self {
+        Self {
+            message: message.into(),
+        }
+    }
+}
+
+impl fmt::Display for ParseError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "Terminado parse error: {}", self.message)
+    }
+}
+
+impl std::error::Error for ParseError {}
+
 impl Message for TerminadoMessage {
     type Result = ();
 }
@@ -22,77 +44,97 @@ pub enum TerminadoMessage {
 }
 
 impl TerminadoMessage {
-    pub fn from_json(json: &str) -> Result<Self, ()> {
-        let value: serde_json::Value = serde_json::from_str(json).map_err(|_| {
-            error!("Invalid Terminado message: Invalid JSON");
+    pub fn from_json(json: &str) -> Result<Self, ParseError> {
+        let value: serde_json::Value = serde_json::from_str(json).map_err(|e| {
+            let msg = format!("Invalid JSON: {}", e);
+            error!("Invalid Terminado message: {}", msg);
+            ParseError::new(msg)
         })?;
 
         let list: &Vec<serde_json::Value> = value.as_array().ok_or_else(|| {
-            error!("Invalid Terminado message: Needs to be an array!");
+            let msg = "Needs to be an array";
+            error!("Invalid Terminado message: {}", msg);
+            ParseError::new(msg)
         })?;
 
         match list
             .first()
             .ok_or_else(|| {
-                error!("Invalid Terminado message: Empty array!");
+                let msg = "Empty array";
+                error!("Invalid Terminado message: {}", msg);
+                ParseError::new(msg)
             })?
             .as_str()
             .ok_or_else(|| {
-                error!("Invalid Terminado message: Type field not a string!");
+                let msg = "Type field not a string";
+                error!("Invalid Terminado message: {}", msg);
+                ParseError::new(msg)
             })? {
             "stdin" => {
                 if list.len() != 2 {
-                    error!(r#"Invalid Terminado message: "stdin" length != 2"#);
-                    return Err(());
+                    let msg = r#""stdin" length != 2"#;
+                    error!("Invalid Terminado message: {}", msg);
+                    return Err(ParseError::new(msg));
                 }
 
                 Ok(TerminadoMessage::Stdin(IO::from(
                     list[1].as_str().ok_or_else(|| {
-                        error!(r#"Invalid Terminado message: "stdin" needs to be a String"#);
+                        let msg = r#""stdin" needs to be a String"#;
+                        error!("Invalid Terminado message: {}", msg);
+                        ParseError::new(msg)
                     })?,
                 )))
             }
             "stdout" => {
                 if list.len() != 2 {
-                    error!(r#"Invalid Terminado message: "stdout" length != 2"#);
-                    return Err(());
+                    let msg = r#""stdout" length != 2"#;
+                    error!("Invalid Terminado message: {}", msg);
+                    return Err(ParseError::new(msg));
                 }
 
                 Ok(TerminadoMessage::Stdout(IO::from(
                     list[1].as_str().ok_or_else(|| {
-                        error!(r#"Invalid Terminado message: "stdout" needs to be a String"#);
+                        let msg = r#""stdout" needs to be a String"#;
+                        error!("Invalid Terminado message: {}", msg);
+                        ParseError::new(msg)
                     })?,
                 )))
             }
             "set_size" => {
                 if list.len() != 3 {
-                    error!(r#"Invalid Terminado message: "set_size" length != 2"#);
-                    return Err(());
+                    let msg = r#""set_size" length != 3"#;
+                    error!("Invalid Terminado message: {}", msg);
+                    return Err(ParseError::new(msg));
                 }
 
                 let rows: u16 = u16::try_from(list[1].as_u64().ok_or_else(|| {
-                    error!(
-                        r#"Invalid Terminado message: "set_size" element 1 needs to be an integer"#
-                    );
+                    let msg = r#""set_size" element 1 needs to be an integer"#;
+                    error!("Invalid Terminado message: {}", msg);
+                    ParseError::new(msg)
                 })?)
                 .map_err(|_| {
-                    error!(r#"Invalid Terminado message. "set_size" rows out of range."#);
+                    let msg = r#""set_size" rows out of range"#;
+                    error!("Invalid Terminado message: {}", msg);
+                    ParseError::new(msg)
                 })?;
 
                 let cols: u16 = u16::try_from(list[2].as_u64().ok_or_else(|| {
-                    error!(
-                        r#"Invalid Terminado message: "set_size" element 2 needs to be an integer"#
-                    );
+                    let msg = r#""set_size" element 2 needs to be an integer"#;
+                    error!("Invalid Terminado message: {}", msg);
+                    ParseError::new(msg)
                 })?)
                 .map_err(|_| {
-                    error!(r#"Invalid Terminado message. "set_size" cols out of range."#);
+                    let msg = r#""set_size" cols out of range"#;
+                    error!("Invalid Terminado message: {}", msg);
+                    ParseError::new(msg)
                 })?;
 
                 Ok(TerminadoMessage::Resize { rows, cols })
             }
             v => {
-                error!("Invalid Terminado message: Unknown type {:?}", v);
-                Err(())
+                let msg = format!("Unknown type {:?}", v);
+                error!("Invalid Terminado message: {}", msg);
+                Err(ParseError::new(msg))
             }
         }
     }