fix(gl): only report registered: false on 404 in whoami (#220)

crates/gl/src/sync.rs

@@ -107,7 +107,7 @@ fn trigger_counts(resp: &serde_json::Value) -> (u64, u64) {
 /// Read at most `cap` bytes of a response body. Bounds the allocation from a
 /// hostile or broken node returning a huge error body — the display is capped
 /// separately, but the read itself must not be unbounded (INV-6, read half).
-async fn read_body_capped(mut resp: reqwest::Response, cap: usize) -> String {
+pub(crate) async fn read_body_capped(mut resp: reqwest::Response, cap: usize) -> String {
     let mut buf: Vec<u8> = Vec::new();
     while buf.len() < cap {
         match resp.chunk().await {
@@ -130,7 +130,7 @@ async fn read_body_capped(mut resp: reqwest::Response, cap: usize) -> String {
 /// reach the terminal verbatim (INV-6). We drop the C0/C1 control bytes (which
 /// defangs ANSI/OSC escapes) AND the Unicode bidi/format controls (which
 /// `char::is_control` does not cover — they can reorder the displayed line).
-fn sanitize_node_msg(s: &str) -> String {
+pub(crate) fn sanitize_node_msg(s: &str) -> String {
     s.chars()
         .filter(|c| !c.is_control() && !gitlawb_core::sanitize::is_bidi_format(*c))
         .take(200)

crates/gl/src/whoami.rs

@@ -1,12 +1,13 @@
 //! `gl whoami` — print current identity and optional node registration info.
 
-use anyhow::Result;
+use anyhow::{bail, Context, Result};
 use clap::Args;
 use serde_json::{json, Value};
 use std::path::PathBuf;
 
 use crate::http::NodeClient;
 use crate::identity::load_keypair_from_dir;
+use crate::sync::{read_body_capped, sanitize_node_msg};
 
 #[derive(Args)]
 pub struct WhoamiArgs {
@@ -22,6 +23,10 @@ pub struct WhoamiArgs {
 }
 
 pub async fn run(args: WhoamiArgs) -> Result<()> {
+    run_to_writer(args, &mut std::io::stdout()).await
+}
+
+pub(crate) async fn run_to_writer(args: WhoamiArgs, w: &mut impl std::io::Write) -> Result<()> {
     let keypair = load_keypair_from_dir(args.dir.as_deref())?;
     let did = keypair.did().to_string();
     let short = did.split(':').next_back().unwrap_or(&did).to_string();
@@ -41,7 +46,7 @@ pub async fn run(args: WhoamiArgs) -> Result<()> {
                 if let Some(caps) = info["capabilities"].as_array() {
                     capabilities = caps
                         .iter()
-                        .filter_map(|c| c.as_str().map(String::from))
+                        .filter_map(|c| c.as_str().map(sanitize_node_msg))
                         .collect();
                 }
                 // Try to get repo count
@@ -51,11 +56,31 @@ pub async fn run(args: WhoamiArgs) -> Result<()> {
                     }
                 }
             }
-            Ok(_) => {
+            Ok(resp) if resp.status().as_u16() == 404 => {
                 registered = Some(false);
             }
-            Err(_) => {
-                registered = Some(false);
+            Ok(resp) => {
+                let status = resp.status();
+                let raw = read_body_capped(resp, 8 * 1024).await;
+                let msg = serde_json::from_str::<Value>(&raw)
+                    .ok()
+                    .and_then(|v| {
+                        let non_empty = |m: Option<&Value>| {
+                            m.and_then(|m| m.as_str())
+                                .map(String::from)
+                                .filter(|s| !s.is_empty())
+                        };
+                        non_empty(v.get("message")).or_else(|| non_empty(v.get("error")))
+                    })
+                    .unwrap_or(raw);
+                bail!(
+                    "agent lookup failed ({status}): {}",
+                    sanitize_node_msg(&msg)
+                );
+            }
+            Err(e) => {
+                let msg = sanitize_node_msg(&e.to_string());
+                return Err(e).context(format!("agent lookup failed: {msg}"));
             }
         }
     }
@@ -77,21 +102,21 @@ pub async fn run(args: WhoamiArgs) -> Result<()> {
         if let Some(rc) = repo_count {
             out["repos"] = json!(rc);
         }
-        println!("{}", serde_json::to_string_pretty(&out)?);
+        writeln!(w, "{}", serde_json::to_string_pretty(&out)?)?;
     } else {
-        println!("DID:        {did}");
-        println!("Short:      {short}");
+        writeln!(w, "DID:        {did}")?;
+        writeln!(w, "Short:      {short}")?;
         if let Some(reg) = registered {
-            println!("Registered: {}", if reg { "yes" } else { "no" });
+            writeln!(w, "Registered: {}", if reg { "yes" } else { "no" })?;
         }
         if let Some(ts) = trust_score {
-            println!("Trust:      {ts:.2}");
+            writeln!(w, "Trust:      {ts:.2}")?;
         }
         if !capabilities.is_empty() {
-            println!("Caps:       {}", capabilities.join(", "));
+            writeln!(w, "Caps:       {}", capabilities.join(", "))?;
         }
         if let Some(rc) = repo_count {
-            println!("Repos:      {rc}");
+            writeln!(w, "Repos:      {rc}")?;
         }
     }
     Ok(())
@@ -192,7 +217,208 @@ mod tests {
             node: Some(server.url()),
             json: false,
         };
-        run(args).await.unwrap();
+        let mut out = Vec::new();
+        run_to_writer(args, &mut out).await.unwrap();
+        let out = String::from_utf8(out).unwrap();
+        assert!(out.contains("Registered: no"), "unexpected output: {out}");
+
+        let args = WhoamiArgs {
+            dir: Some(dir.path().to_path_buf()),
+            node: Some(server.url()),
+            json: true,
+        };
+        let mut out = Vec::new();
+        run_to_writer(args, &mut out).await.unwrap();
+        let out = String::from_utf8(out).unwrap();
+        assert!(
+            out.contains("\"registered\": false"),
+            "unexpected output: {out}"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_whoami_with_node_forbidden() {
+        let dir = TempDir::new().unwrap();
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let pem = kp.to_pem().unwrap();
+        std::fs::write(dir.path().join("identity.pem"), pem.as_bytes()).unwrap();
+        let did = kp.did().to_string();
+
+        let mut server = mockito::Server::new_async().await;
+        let _agent = server
+            .mock("GET", format!("/api/v1/agents/{did}").as_str())
+            .with_status(403)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"message":"forbidden"}"#)
+            .create_async()
+            .await;
+
+        let args = WhoamiArgs {
+            dir: Some(dir.path().to_path_buf()),
+            node: Some(server.url()),
+            json: false,
+        };
+        let err = run(args).await.unwrap_err();
+        let msg = format!("{err:?}");
+        assert!(msg.contains("403"), "expected 403 error, got: {msg}");
+        assert!(
+            msg.contains("forbidden"),
+            "expected 'forbidden' in error, got: {msg}"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_whoami_with_node_server_error() {
+        let dir = TempDir::new().unwrap();
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let pem = kp.to_pem().unwrap();
+        std::fs::write(dir.path().join("identity.pem"), pem.as_bytes()).unwrap();
+        let did = kp.did().to_string();
+
+        let mut server = mockito::Server::new_async().await;
+        let _agent = server
+            .mock("GET", format!("/api/v1/agents/{did}").as_str())
+            .with_status(500)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"message":"internal error"}"#)
+            .create_async()
+            .await;
+
+        let args = WhoamiArgs {
+            dir: Some(dir.path().to_path_buf()),
+            node: Some(server.url()),
+            json: false,
+        };
+        let err = run(args).await.unwrap_err();
+        let msg = format!("{err:?}");
+        assert!(msg.contains("500"), "expected 500 error, got: {msg}");
+        assert!(
+            msg.contains("internal error"),
+            "expected 'internal error' in error, got: {msg}"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_whoami_server_error_unusable_message_uses_error() {
+        let dir = TempDir::new().unwrap();
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let pem = kp.to_pem().unwrap();
+        std::fs::write(dir.path().join("identity.pem"), pem.as_bytes()).unwrap();
+        let did = kp.did().to_string();
+
+        let mut server = mockito::Server::new_async().await;
+        let _agent = server
+            .mock("GET", format!("/api/v1/agents/{did}").as_str())
+            .with_status(500)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"message":"","error":"boom"}"#)
+            .create_async()
+            .await;
+
+        let args = WhoamiArgs {
+            dir: Some(dir.path().to_path_buf()),
+            node: Some(server.url()),
+            json: false,
+        };
+        let err = run(args).await.unwrap_err();
+        let msg = format!("{err:?}");
+        assert!(
+            msg.contains("boom"),
+            "expected 'boom' from error field, got: {msg}"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_whoami_with_node_transport_error() {
+        let dir = TempDir::new().unwrap();
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let pem = kp.to_pem().unwrap();
+        std::fs::write(dir.path().join("identity.pem"), pem.as_bytes()).unwrap();
+
+        let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
+        let node = format!("http://{}", listener.local_addr().unwrap());
+        drop(listener);
+
+        let args = WhoamiArgs {
+            dir: Some(dir.path().to_path_buf()),
+            node: Some(node),
+            json: false,
+        };
+        let err = run(args).await.unwrap_err();
+        let msg = format!("{err:?}");
+        assert!(
+            msg.contains("agent lookup failed"),
+            "expected transport error, got: {msg}"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_whoami_server_error_body_display_bounded() {
+        let dir = TempDir::new().unwrap();
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let pem = kp.to_pem().unwrap();
+        std::fs::write(dir.path().join("identity.pem"), pem.as_bytes()).unwrap();
+        let did = kp.did().to_string();
+
+        let mut server = mockito::Server::new_async().await;
+        let _agent = server
+            .mock("GET", format!("/api/v1/agents/{did}").as_str())
+            .with_status(502)
+            .with_header("content-type", "application/json")
+            .with_body("x".repeat(100_000))
+            .create_async()
+            .await;
+
+        let args = WhoamiArgs {
+            dir: Some(dir.path().to_path_buf()),
+            node: Some(server.url()),
+            json: false,
+        };
+        let err = run(args).await.unwrap_err();
+        let msg = format!("{err:?}");
+        assert!(
+            msg.contains("502"),
+            "expected 502 error with bounded body, got: {msg}"
+        );
+        let display = format!("{err}");
+        assert!(
+            display.len() < 1000,
+            "error message too long ({} bytes) — body was not capped",
+            display.len()
+        );
+    }
+
+    #[tokio::test]
+    async fn test_whoami_server_error_sanitizes_controls() {
+        let dir = TempDir::new().unwrap();
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let pem = kp.to_pem().unwrap();
+        std::fs::write(dir.path().join("identity.pem"), pem.as_bytes()).unwrap();
+        let did = kp.did().to_string();
+
+        let mut server = mockito::Server::new_async().await;
+        let _agent = server
+            .mock("GET", format!("/api/v1/agents/{did}").as_str())
+            .with_status(500)
+            .with_header("content-type", "application/json")
+            .with_body("{\"message\":\"\\u001b[31mowned\\u0007\\u202eevil\"}")
+            .create_async()
+            .await;
+
+        let args = WhoamiArgs {
+            dir: Some(dir.path().to_path_buf()),
+            node: Some(server.url()),
+            json: false,
+        };
+        let err = run(args).await.unwrap_err();
+        let msg = format!("{err:?}");
+        assert!(
+            msg.contains("owned"),
+            "expected sanitized error body, got: {msg}"
+        );
+        assert!(!msg.contains('\u{1b}'), "ESC control char leaked: {msg}");
+        assert!(!msg.contains('\u{07}'), "BEL control char leaked: {msg}");
+        assert!(!msg.contains('\u{202e}'), "RTL override leaked: {msg}");
     }
 
     #[tokio::test]
@@ -230,4 +456,67 @@ mod tests {
         };
         run(args).await.unwrap();
     }
+
+    #[tokio::test]
+    async fn test_whoami_sanitizes_hostile_capabilities() {
+        let dir = TempDir::new().unwrap();
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let pem = kp.to_pem().unwrap();
+        std::fs::write(dir.path().join("identity.pem"), pem.as_bytes()).unwrap();
+        let did = kp.did().to_string();
+        let short = did.split(':').next_back().unwrap().to_string();
+
+        let mut server = mockito::Server::new_async().await;
+        let _agent = server
+            .mock("GET", format!("/api/v1/agents/{did}").as_str())
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(
+                "{\"trust_score\":0.5,\"capabilities\":[\"\\u001b]0;PWNED\\u0007repo:write\",\"\\u202egnitirw-tfel\"]}",
+            )
+            .create_async()
+            .await;
+        let _repos = server
+            .mock(
+                "GET",
+                mockito::Matcher::Regex(format!(r"^/api/v1/repos\?owner={short}")),
+            )
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body("[]")
+            .create_async()
+            .await;
+
+        // Human mode: no control bytes or bidi overrides reach the terminal
+        let mut buf = Vec::new();
+        let args = WhoamiArgs {
+            dir: Some(dir.path().to_path_buf()),
+            node: Some(server.url()),
+            json: false,
+        };
+        run_to_writer(args, &mut buf).await.unwrap();
+        let out = String::from_utf8(buf).unwrap();
+        assert!(!out.contains('\u{1b}'), "ESC leaked in human mode: {out:?}");
+        assert!(!out.contains('\u{07}'), "BEL leaked in human mode: {out:?}");
+        assert!(
+            !out.contains('\u{202e}'),
+            "RLO leaked in human mode: {out:?}"
+        );
+        assert!(out.contains("repo:write"), "benign text missing: {out:?}");
+        assert!(out.contains("tfel"), "reversed text missing: {out:?}");
+
+        // JSON mode: serde escapes C0 but passes bidi — ensure no U+202E
+        let mut buf = Vec::new();
+        let args = WhoamiArgs {
+            dir: Some(dir.path().to_path_buf()),
+            node: Some(server.url()),
+            json: true,
+        };
+        run_to_writer(args, &mut buf).await.unwrap();
+        let out = String::from_utf8(buf).unwrap();
+        assert!(
+            !out.contains('\u{202e}'),
+            "RLO leaked in JSON mode: {out:?}"
+        );
+    }
 }