refactor: consolidate output hygiene into output.rs (#527)

jpoehnelt · jpoehnelt-bot · web-flow · commit 2e909ae9054f · 2026-03-18T09:34:20.000-06:00
* refactor: consolidate output hygiene into output.rs

Introduce src/output.rs that consolidates:
- sanitize_for_terminal (upgraded: now also strips bidi overrides,
  zero-width chars, directional isolates, line/paragraph separators)
- reject_dangerous_chars (renamed from reject_control_chars)
- is_dangerous_unicode predicate
- colorize + stderr_supports_color (NO_COLOR + TTY detection)
- status/warn/info stderr helpers (auto-sanitize)

Migrate existing callers via re-exports from error.rs and validate.rs.

Fix watch.rs:
- Sanitize raw API error body in eprintln (was high-risk injection vector)
- Replace 3 inline ANSI escape codes with colorize() for NO_COLOR support

Fix triage.rs:
- Sanitize user --query string in no_messages_msg output

* refactor: remove re-export indirection, import directly from output

Update all 10 caller files to import sanitize_for_terminal directly
from crate::output instead of going through crate::error re-exports.
Remove pub(crate) re-exports from error.rs and validate.rs.

* fix: use char::is_control() in reject_dangerous_chars for C1 coverage

Address PR review: the manual (c as u32) &lt; 0x20 check missed
C1 control characters (U+0080-U+009F), including CSI (U+009B) which
can inject terminal escape sequences. Using char::is_control() covers
both C0 and C1 ranges.

Add test for CSI rejection.

* fix: validate ansi_color in colorize() to prevent injection

Defense-in-depth: only emit ANSI escape codes when ansi_color
contains exclusively ASCII digits. Falls back to plain text if
an invalid color code is passed.

---------

Co-authored-by: jpoehnelt-bot &lt;jpoehnelt-bot@users.noreply.github.com&gt;
diff --git a/.changeset/output-hygiene.md b/.changeset/output-hygiene.md
@@ -0,0 +1,5 @@
+---
+"@googleworkspace/cli": patch
+---
+
+Consolidate terminal sanitization, coloring, and output helpers into a new `output.rs` module. Fixes raw ANSI escape codes in `watch.rs` that bypassed `NO_COLOR` and TTY detection, upgrades `sanitize_for_terminal` to also strip dangerous Unicode characters (bidi overrides, zero-width spaces, directional isolates), and sanitizes previously raw API error body and user query outputs.
diff --git a/src/credential_store.rs b/src/credential_store.rs
@@ -14,7 +14,7 @@
 
 use std::path::PathBuf;
 
-use crate::error::sanitize_for_terminal;
+use crate::output::sanitize_for_terminal;
 
 use aes_gcm::aead::{Aead, KeyInit, OsRng};
 use aes_gcm::{AeadCore, Aes256Gcm, Nonce};
diff --git a/src/error.rs b/src/error.rs
@@ -148,36 +148,7 @@ impl GwsError {
     }
 }
 
-/// Returns true when stderr is connected to an interactive terminal,
-/// meaning ANSI color codes will be visible to the user.
-fn stderr_supports_color() -> bool {
-    use std::io::IsTerminal;
-    std::io::stderr().is_terminal() && std::env::var_os("NO_COLOR").is_none()
-}
-
-/// Wrap `text` in ANSI bold + the given color code, resetting afterwards.
-/// Returns the plain text unchanged when stderr is not a TTY.
-fn colorize(text: &str, ansi_color: &str) -> String {
-    if stderr_supports_color() {
-        format!("\x1b[1;{ansi_color}m{text}\x1b[0m")
-    } else {
-        text.to_string()
-    }
-}
-
-/// Strip terminal control characters from `text` to prevent escape-sequence
-/// injection when printing untrusted content (API responses, user input) to
-/// stderr.  Preserves newlines and tabs for readability.
-pub(crate) fn sanitize_for_terminal(text: &str) -> String {
-    text.chars()
-        .filter(|&c| {
-            if c == '\n' || c == '\t' {
-                return true;
-            }
-            !c.is_control()
-        })
-        .collect()
-}
+use crate::output::{colorize, sanitize_for_terminal};
 
 /// Format a colored error label for the given error variant.
 fn error_label(err: &GwsError) -> String {
diff --git a/src/executor.rs b/src/executor.rs
@@ -28,7 +28,8 @@ use serde_json::{json, Map, Value};
 use tokio::io::AsyncWriteExt;
 
 use crate::discovery::{RestDescription, RestMethod};
-use crate::error::{sanitize_for_terminal, GwsError};
+use crate::error::GwsError;
+use crate::output::sanitize_for_terminal;
 
 /// Tracks what authentication method was used for the request.
 #[derive(Debug, Clone, PartialEq)]
diff --git a/src/generate_skills.rs b/src/generate_skills.rs
@@ -18,7 +18,8 @@
 
 use crate::commands;
 use crate::discovery;
-use crate::error::{sanitize_for_terminal, GwsError};
+use crate::error::GwsError;
+use crate::output::sanitize_for_terminal;
 use crate::services;
 use clap::Command;
 use std::path::Path;
diff --git a/src/helpers/events/subscribe.rs b/src/helpers/events/subscribe.rs
@@ -1,7 +1,7 @@
 use super::*;
 use crate::auth::AccessTokenProvider;
-use crate::error::sanitize_for_terminal;
 use crate::helpers::PUBSUB_API_BASE;
+use crate::output::sanitize_for_terminal;
 use std::path::PathBuf;
 
 #[derive(Debug, Clone, Default, Builder)]
diff --git a/src/helpers/gmail/mod.rs b/src/helpers/gmail/mod.rs
@@ -28,9 +28,9 @@ use triage::handle_triage;
 use watch::handle_watch;
 
 pub(super) use crate::auth;
-use crate::error::sanitize_for_terminal;
 pub(super) use crate::error::GwsError;
 pub(super) use crate::executor;
+use crate::output::sanitize_for_terminal;
 pub(super) use anyhow::Context;
 pub(super) use base64::{engine::general_purpose::URL_SAFE, Engine as _};
 pub(super) use clap::{Arg, ArgAction, ArgMatches, Command};
diff --git a/src/helpers/gmail/read.rs b/src/helpers/gmail/read.rs
@@ -70,7 +70,7 @@ pub(super) async fn handle_read(
                 continue;
             }
             // Replace newlines to prevent header spoofing in the output, then sanitize.
-            let sanitized_value = sanitize_terminal_output(&value.replace(['\r', '\n'], " "));
+            let sanitized_value = sanitize_for_terminal(&value.replace(['\r', '\n'], " "));
             writeln!(stdout, "{}: {}", name, sanitized_value)
                 .with_context(|| format!("Failed to write '{name}' header"))?;
         }
@@ -87,8 +87,7 @@ pub(super) async fn handle_read(
         &original.body_text
     };
 
-    writeln!(stdout, "{}", sanitize_terminal_output(body))
-        .context("Failed to write message body")?;
+    writeln!(stdout, "{}", sanitize_for_terminal(body)).context("Failed to write message body")?;
 
     Ok(())
 }
@@ -102,17 +101,16 @@ fn format_mailbox_list(mailboxes: &[Mailbox]) -> String {
         .join(", ")
 }
 
-/// Re-export the crate-wide terminal sanitizer for use in this module.
-use crate::error::sanitize_for_terminal as sanitize_terminal_output;
+use crate::output::sanitize_for_terminal;
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
     #[test]
-    fn test_sanitize_terminal_output() {
+    fn test_sanitize_for_terminal() {
         let malicious = "Subject: \x1b]0;MALICIOUS\x07Hello\nWorld\r\t";
-        let sanitized = sanitize_terminal_output(malicious);
+        let sanitized = sanitize_for_terminal(malicious);
         // ANSI escape sequences (control chars) should be removed
         assert!(!sanitized.contains('\x1b'));
         assert!(!sanitized.contains('\x07'));
diff --git a/src/helpers/gmail/triage.rs b/src/helpers/gmail/triage.rs
@@ -181,7 +181,10 @@ pub async fn handle_triage(matches: &ArgMatches) -> Result<(), GwsError> {
 /// Returns the human-readable "no messages" diagnostic string.
 /// Extracted so the test can reference the exact same message without duplication.
 fn no_messages_msg(query: &str) -> String {
-    format!("No messages found matching query: {query}")
+    format!(
+        "No messages found matching query: {}",
+        crate::output::sanitize_for_terminal(query)
+    )
 }
 
 #[cfg(test)]
diff --git a/src/helpers/gmail/watch.rs b/src/helpers/gmail/watch.rs
@@ -1,7 +1,8 @@
 use super::*;
 use crate::auth::AccessTokenProvider;
-use crate::error::sanitize_for_terminal;
 use crate::helpers::PUBSUB_API_BASE;
+use crate::output::colorize;
+use crate::output::sanitize_for_terminal;
 
 const GMAIL_API_BASE: &str = "https://gmail.googleapis.com/gmail/v1";
 
@@ -100,7 +101,7 @@ pub(super) async fn handle_watch(
                     "    --member=serviceAccount:gmail-api-push@system.gserviceaccount.com \\"
                 );
                 eprintln!("    --role=roles/pubsub.publisher");
-                eprintln!("Error: {body}");
+                eprintln!("Error: {}", sanitize_for_terminal(&body));
             }
 
             t
@@ -454,7 +455,11 @@ async fn fetch_and_output_messages(
                             }
                         }
                         Err(e) => {
-                            eprintln!("\x1b[33m[WARNING]\x1b[0m Model Armor sanitization failed for message {msg_id}: {}", sanitize_for_terminal(&e.to_string()));
+                            eprintln!(
+                                "{} Model Armor sanitization failed for message {msg_id}: {}",
+                                colorize("warning:", "33"),
+                                sanitize_for_terminal(&e.to_string())
+                            );
                         }
                     }
                 }
@@ -498,12 +503,16 @@ fn apply_sanitization_result(
         match sanitize_config.mode {
             crate::helpers::modelarmor::SanitizeMode::Block => {
                 eprintln!(
-                    "\x1b[31m[BLOCKED]\x1b[0m Message {msg_id} blocked by Model Armor (match found)"
+                    "{} Message {msg_id} blocked by Model Armor (match found)",
+                    colorize("blocked:", "31")
                 );
                 return None;
             }
             crate::helpers::modelarmor::SanitizeMode::Warn => {
-                eprintln!("\x1b[33m[WARNING]\x1b[0m Model Armor match found in message {msg_id}");
+                eprintln!(
+                    "{} Model Armor match found in message {msg_id}",
+                    colorize("warning:", "33")
+                );
                 full_msg["_sanitization"] = serde_json::json!({
                     "filterMatchState": result.filter_match_state,
                     "filterResults": result.filter_results,
diff --git a/src/helpers/workflows.rs b/src/helpers/workflows.rs
@@ -17,7 +17,8 @@
 
 use super::Helper;
 use crate::auth;
-use crate::error::{sanitize_for_terminal, GwsError};
+use crate::error::GwsError;
+use crate::output::sanitize_for_terminal;
 use clap::{Arg, ArgMatches, Command};
 use serde_json::{json, Value};
 use std::future::Future;
diff --git a/src/main.rs b/src/main.rs
@@ -33,6 +33,7 @@ mod generate_skills;
 mod helpers;
 mod logging;
 mod oauth_config;
+mod output;
 mod schema;
 mod services;
 mod setup;
diff --git a/src/output.rs b/src/output.rs
diff --git a/src/setup.rs b/src/setup.rs
diff --git a/src/token_storage.rs b/src/token_storage.rs
diff --git a/src/validate.rs b/src/validate.rs

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +"@googleworkspace/cli": patch
 +---
++
 +Consolidate terminal sanitization, coloring, and output helpers into a new `output.rs` module. Fixes raw ANSI escape codes in `watch.rs` that bypassed `NO_COLOR` and TTY detection, upgrades `sanitize_for_terminal` to also strip dangerous Unicode characters (bidi overrides, zero-width spaces, directional isolates), and sanitizes previously raw API error body and user query outputs.