Skip to content

fix: colorize stderr warnings and errors on TTY terminals #394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
seang1121 wants to merge 5 commits into
base: main
Choose a base branch
from
+43 −4
Open

fix: colorize stderr warnings and errors on TTY terminals #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
598e79d
fix: colorize stderr warnings and errors on TTY terminals
seang1121 Mar 11, 2026
4efadd8
refactor(error): extract colorize helper to reduce duplication
seang1121 Mar 11, 2026
9267f3f
fix(executor): prevent ANSI injection by colorizing only static prefix
seang1121 Mar 11, 2026
3c34fb0
fix(error): sanitize ESC chars in colorize, remove redundant tests
seang1121 Mar 11, 2026
417cba4
fix(error): strip all control chars in colorize to prevent terminal i…
seang1121 Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/colorize-stderr-tty.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

Colorize stderr warnings and errors with ANSI codes when output is a TTY terminal
35 changes: 34 additions & 1 deletion src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,29 @@
use serde_json::json;
use thiserror::Error;

pub(crate) fn is_tty() -> bool {
use std::io::IsTerminal;
std::io::stderr().is_terminal()
}
Comment on lines +18 to +21
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The is_tty() function is called every time a string is colorized. This can be inefficient, especially when logging messages in a loop (e.g., during pagination), as it results in a repeated syscall. Since the TTY status of stderr will not change during the execution of the program, this value should be cached.

You can use std::sync::OnceLock (available since Rust 1.70, which you are already targeting) to compute this value only once.

Here's an example of how you could implement this:

pub(crate) fn is_tty() -> bool {
    static IS_TTY: std::sync::OnceLock<bool> = std::sync::OnceLock::new();
    *IS_TTY.get_or_init(|| {
        use std::io::IsTerminal;
        std::io::stderr().is_terminal()
    })
}


fn colorize(s: &str, code: &str) -> String {
if is_tty() {
// Strip control characters to prevent terminal escape injection.
// We allow newline and tab for basic formatting.
let sanitized: String = s
.chars()
.filter(|&c| !c.is_control() || c == '\n' || c == '\t')
.collect();
format!("\x1b[{code}m{sanitized}\x1b[0m")
} else {
s.to_string()
}
}
Comment on lines +23 to +35
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The colorize function directly includes the input string s in the formatted output. If a string containing ANSI escape codes is passed to yellow, red, or bold, it could lead to terminal escape injection. This would allow an attacker to manipulate the terminal's output, or in some terminals, execute commands. While the current usages in this PR are safe as they use static strings, future uses with dynamic or user-controlled data could introduce a vulnerability. To mitigate this, the input string should be sanitized to remove any existing escape codes before being wrapped in new ones.

Suggested change
fn colorize(s: &str, code: &str) -> String {
if is_tty() {
format!("\x1b[{code}m{s}\x1b[0m")
} else {
s.to_string()
}
}
fn colorize(s: &str, code: &str) -> String {
if is_tty() {
// Basic sanitization to prevent terminal escape injection.
// This removes the ESC character, which is the start of most control sequences.
let sanitized_s = s.replace('\x1b', "");
format!("\x1b[{code}m{sanitized_s}\x1b[0m")
} else {
s.to_string()
}
}

Comment on lines +23 to +35
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current sanitization logic only removes the ESC (\x1b) character. This is insufficient to prevent all forms of terminal escape injection. Other control characters (like \r for carriage return or \b for backspace) can be used to manipulate the terminal output, potentially hiding or spoofing important information in error messages.

To make this utility safer for future use with dynamic data, it's better to strip all control characters except for known-safe ones like newline and tab.

Suggested change
fn colorize(s: &str, code: &str) -> String {
if is_tty() {
// Strip ESC characters to prevent terminal escape injection.
let sanitized = s.replace('\x1b', "");
format!("\x1b[{code}m{sanitized}\x1b[0m")
} else {
s.to_string()
}
}
fn colorize(s: &str, code: &str) -> String {
if is_tty() {
// Strip control characters to prevent terminal escape injection.
// We allow newline and tab for basic formatting.
let sanitized: String = s
.chars()
.filter(|&c| !c.is_control() || c == '\n' || c == '\t')
.collect();
format!("\x1b[{code}m{sanitized}\x1b[0m")
} else {
s.to_string()
}
}


pub(crate) fn yellow(s: &str) -> String { colorize(s, "33") }
pub(crate) fn red(s: &str) -> String { colorize(s, "31") }
pub(crate) fn bold(s: &str) -> String { colorize(s, "1") }

#[derive(Error, Debug)]
pub enum GwsError {
#[error("{message}")]
Expand Down Expand Up @@ -111,7 +134,7 @@ pub fn print_error_json(err: &GwsError) {
{
if reason == "accessNotConfigured" {
eprintln!();
eprintln!("💡 API not enabled for your GCP project.");
eprintln!("{}", yellow("💡 API not enabled for your GCP project."));
if let Some(url) = enable_url {
eprintln!(" Enable it at: {url}");
} else {
Expand All @@ -126,6 +149,16 @@ pub fn print_error_json(err: &GwsError) {
mod tests {
use super::*;

#[test]
fn test_color_helpers_no_ansi_when_non_tty() {
// In test runners, stderr is not a TTY — helpers must return the raw string.
if !is_tty() {
assert_eq!(yellow("x"), "x");
assert_eq!(red("x"), "x");
assert_eq!(bold("x"), "x");
}
}

#[test]
fn test_error_to_json_api() {
let err = GwsError::Api {
Expand Down
4 changes: 2 additions & 2 deletions src/executor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ async fn handle_json_response(
Ok(result) => {
let is_match = result.filter_match_state == "MATCH_FOUND";
if is_match {
eprintln!("⚠️ Model Armor: prompt injection detected (filterMatchState: MATCH_FOUND)");
eprintln!("{}", crate::error::yellow("⚠️ Model Armor: prompt injection detected (filterMatchState: MATCH_FOUND)"));
}

if is_match && *sanitize_mode == crate::helpers::modelarmor::SanitizeMode::Block
Expand All @@ -254,7 +254,7 @@ async fn handle_json_response(
}
}
Err(e) => {
eprintln!("⚠️ Model Armor sanitization failed: {e}");
eprintln!("{} {e}", crate::error::yellow("⚠️ Model Armor sanitization failed:"));
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ async fn run() -> Result<(), GwsError> {
Ok(fmt) => fmt,
Err(unknown) => {
eprintln!(
"warning: unknown output format '{unknown}'; falling back to json (valid options: json, table, yaml, csv)"
"{} unknown output format '{unknown}'; falling back to json (valid options: json, table, yaml, csv)",
crate::error::yellow("warning:")
);
formatter::OutputFormat::Json
}
Expand Down