From 9a16fe6590484345f72b844a0d0ed467a9fa0bd2 Mon Sep 17 00:00:00 2001 From: jpoehnelt-bot Date: Tue, 17 Mar 2026 17:09:58 -0600 Subject: [PATCH 1/4] 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 --- .changeset/output-hygiene.md | 5 + src/error.rs | 33 +---- src/helpers/gmail/triage.rs | 5 +- src/helpers/gmail/watch.rs | 17 ++- src/main.rs | 1 + src/output.rs | 259 +++++++++++++++++++++++++++++++++++ src/validate.rs | 41 +----- 7 files changed, 289 insertions(+), 72 deletions(-) create mode 100644 .changeset/output-hygiene.md create mode 100644 src/output.rs diff --git a/.changeset/output-hygiene.md b/.changeset/output-hygiene.md new file mode 100644 index 00000000..da066051 --- /dev/null +++ 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/error.rs b/src/error.rs index 2fd4d795..ba6295eb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -148,36 +148,9 @@ 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() -} +// Re-export from the consolidated output module so existing callers +// that import from `crate::error` continue to work. +pub(crate) 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/helpers/gmail/triage.rs b/src/helpers/gmail/triage.rs index 8bc58804..3c275e1f 100644 --- 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 index 5785fa7a..a8e2571f 100644 --- a/src/helpers/gmail/watch.rs +++ b/src/helpers/gmail/watch.rs @@ -2,6 +2,7 @@ use super::*; use crate::auth::AccessTokenProvider; use crate::error::sanitize_for_terminal; use crate::helpers::PUBSUB_API_BASE; +use crate::output::colorize; 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/main.rs b/src/main.rs index 2fe5efc7..41dcc1e1 100644 --- 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 new file mode 100644 index 00000000..1be9b77e --- /dev/null +++ b/src/output.rs @@ -0,0 +1,259 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Shared output helpers for terminal sanitization, coloring, and stderr +//! messaging. +//! +//! Every function that prints untrusted content to the terminal should use +//! these helpers to prevent escape-sequence injection, Unicode spoofing, +//! and to respect `NO_COLOR` / non-TTY environments. + +use crate::error::GwsError; + +// ── Dangerous character detection ───────────────────────────────────── + +/// Returns `true` for Unicode characters that are dangerous in terminal +/// output but not caught by `char::is_control()`: zero-width chars, bidi +/// overrides, Unicode line/paragraph separators, and directional isolates. +/// +/// Using `matches!` with char ranges gives O(1) per character instead of the +/// O(M) linear scan that a slice `.contains()` would require. +pub(crate) fn is_dangerous_unicode(c: char) -> bool { + matches!(c, + // zero-width: ZWSP, ZWNJ, ZWJ, BOM/ZWNBSP + '\u{200B}'..='\u{200D}' | '\u{FEFF}' | + // bidi: LRE, RLE, PDF, LRO, RLO + '\u{202A}'..='\u{202E}' | + // line / paragraph separators + '\u{2028}'..='\u{2029}' | + // directional isolates: LRI, RLI, FSI, PDI + '\u{2066}'..='\u{2069}' + ) +} + +// ── Sanitization ────────────────────────────────────────────────────── + +/// Strip dangerous characters from untrusted text before printing to the +/// terminal. Removes ASCII control characters (except `\n` and `\t`, +/// which are preserved for readability) and dangerous Unicode characters +/// (bidi overrides, zero-width chars, line/paragraph separators). +pub(crate) fn sanitize_for_terminal(text: &str) -> String { + text.chars() + .filter(|&c| { + if c == '\n' || c == '\t' { + return true; + } + if c.is_control() { + return false; + } + !is_dangerous_unicode(c) + }) + .collect() +} + +/// Rejects strings containing null bytes, ASCII control characters +/// (including DEL, 0x7F), or dangerous Unicode characters such as +/// zero-width chars, bidi overrides, and Unicode line/paragraph separators. +/// +/// Used for validating CLI argument values at the parse boundary. +pub(crate) fn reject_dangerous_chars(value: &str, flag_name: &str) -> Result<(), GwsError> { + for c in value.chars() { + if (c as u32) < 0x20 || c as u32 == 0x7F { + return Err(GwsError::Validation(format!( + "{flag_name} contains invalid control characters" + ))); + } + if is_dangerous_unicode(c) { + return Err(GwsError::Validation(format!( + "{flag_name} contains invalid Unicode characters" + ))); + } + } + Ok(()) +} + +// ── Color ───────────────────────────────────────────────────────────── + +/// Returns true when stderr is connected to an interactive terminal and +/// `NO_COLOR` is not set, meaning ANSI color codes will be visible. +pub(crate) 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 or `NO_COLOR` +/// is set. +pub(crate) 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() + } +} + +// ── Stderr helpers ──────────────────────────────────────────────────── + +/// Print a status message to stderr. The message is sanitized before +/// printing to prevent terminal injection. +#[allow(dead_code)] +pub(crate) fn status(msg: &str) { + eprintln!("{}", sanitize_for_terminal(msg)); +} + +/// Print a warning to stderr with a colored prefix. The message is +/// sanitized before printing. +#[allow(dead_code)] +pub(crate) fn warn(msg: &str) { + let prefix = colorize("warning:", "33"); // yellow + eprintln!("{prefix} {}", sanitize_for_terminal(msg)); +} + +/// Print an informational message to stderr. The message is sanitized +/// before printing. +#[allow(dead_code)] +pub(crate) fn info(msg: &str) { + eprintln!("{}", sanitize_for_terminal(msg)); +} + +#[cfg(test)] +mod tests { + use super::*; + + // ── sanitize_for_terminal ───────────────────────────────────── + + #[test] + fn sanitize_strips_ansi_escape_sequences() { + let input = "normal \x1b[31mred text\x1b[0m end"; + let sanitized = sanitize_for_terminal(input); + assert_eq!(sanitized, "normal [31mred text[0m end"); + assert!(!sanitized.contains('\x1b')); + } + + #[test] + fn sanitize_preserves_newlines_and_tabs() { + let input = "line1\nline2\ttab"; + assert_eq!(sanitize_for_terminal(input), "line1\nline2\ttab"); + } + + #[test] + fn sanitize_strips_bell_and_backspace() { + let input = "hello\x07bell\x08backspace"; + assert_eq!(sanitize_for_terminal(input), "hellobellbackspace"); + } + + #[test] + fn sanitize_strips_carriage_return() { + let input = "real\rfake"; + assert_eq!(sanitize_for_terminal(input), "realfake"); + } + + #[test] + fn sanitize_strips_bidi_overrides() { + let input = "hello\u{202E}dlrow"; + assert_eq!(sanitize_for_terminal(input), "hellodlrow"); + } + + #[test] + fn sanitize_strips_zero_width_chars() { + assert_eq!(sanitize_for_terminal("foo\u{200B}bar"), "foobar"); + assert_eq!(sanitize_for_terminal("foo\u{FEFF}bar"), "foobar"); + } + + #[test] + fn sanitize_strips_line_separators() { + assert_eq!(sanitize_for_terminal("line1\u{2028}line2"), "line1line2"); + assert_eq!(sanitize_for_terminal("para1\u{2029}para2"), "para1para2"); + } + + #[test] + fn sanitize_strips_directional_isolates() { + assert_eq!(sanitize_for_terminal("a\u{2066}b\u{2069}c"), "abc"); + } + + #[test] + fn sanitize_preserves_normal_unicode() { + assert_eq!(sanitize_for_terminal("日本語 café αβγ"), "日本語 café αβγ"); + } + + // ── reject_dangerous_chars ──────────────────────────────────── + + #[test] + fn reject_clean_string() { + assert!(reject_dangerous_chars("hello/world", "test").is_ok()); + } + + #[test] + fn reject_tab() { + assert!(reject_dangerous_chars("hello\tworld", "test").is_err()); + } + + #[test] + fn reject_newline() { + assert!(reject_dangerous_chars("hello\nworld", "test").is_err()); + } + + #[test] + fn reject_del() { + assert!(reject_dangerous_chars("hello\x7Fworld", "test").is_err()); + } + + #[test] + fn reject_zero_width_space() { + assert!(reject_dangerous_chars("foo\u{200B}bar", "test").is_err()); + } + + #[test] + fn reject_bom() { + assert!(reject_dangerous_chars("foo\u{FEFF}bar", "test").is_err()); + } + + #[test] + fn reject_rtl_override() { + assert!(reject_dangerous_chars("foo\u{202E}bar", "test").is_err()); + } + + #[test] + fn reject_line_separator() { + assert!(reject_dangerous_chars("foo\u{2028}bar", "test").is_err()); + } + + #[test] + fn reject_paragraph_separator() { + assert!(reject_dangerous_chars("foo\u{2029}bar", "test").is_err()); + } + + #[test] + fn reject_zero_width_joiner() { + assert!(reject_dangerous_chars("foo\u{200D}bar", "test").is_err()); + } + + #[test] + fn reject_preserves_normal_unicode() { + assert!(reject_dangerous_chars("日本語", "test").is_ok()); + assert!(reject_dangerous_chars("café", "test").is_ok()); + assert!(reject_dangerous_chars("αβγ", "test").is_ok()); + } + + // ── colorize ────────────────────────────────────────────────── + + #[test] + fn colorize_returns_text_in_no_color_mode() { + // In test environment, stderr is typically not a TTY + let result = colorize("hello", "31"); + // Either plain text (no TTY) or colored (TTY) — we just verify + // it contains the original text + assert!(result.contains("hello")); + } +} diff --git a/src/validate.rs b/src/validate.rs index 0de4a312..e4513107 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -21,24 +21,8 @@ use crate::error::GwsError; use std::path::{Path, PathBuf}; -/// Returns `true` for Unicode characters that are dangerous but not caught by -/// ASCII-range byte checks or `char::is_control()`: zero-width chars, bidi -/// overrides, Unicode line/paragraph separators, and directional isolates. -/// -/// Using `matches!` with char ranges gives O(1) per character instead of the -/// O(M) linear scan that a slice `.contains()` would require. -fn is_rejected_unicode(c: char) -> bool { - matches!(c, - // zero-width: ZWSP, ZWNJ, ZWJ, BOM/ZWNBSP - '\u{200B}'..='\u{200D}' | '\u{FEFF}' | - // bidi: LRE, RLE, PDF, LRO, RLO - '\u{202A}'..='\u{202E}' | - // line / paragraph separators - '\u{2028}'..='\u{2029}' | - // directional isolates: LRI, RLI, FSI, PDI - '\u{2066}'..='\u{2069}' - ) -} +// Re-export from the consolidated output module. +pub(crate) use crate::output::reject_dangerous_chars as reject_control_chars; /// Validates that `dir` is a safe output directory. /// @@ -210,24 +194,7 @@ fn normalize_dotdot(path: &Path) -> PathBuf { out } -/// Rejects strings containing null bytes, ASCII control characters -/// (including DEL, 0x7F), or dangerous Unicode characters such as -/// zero-width chars, bidi overrides, and Unicode line/paragraph separators. -pub(crate) fn reject_control_chars(value: &str, flag_name: &str) -> Result<(), GwsError> { - for c in value.chars() { - if (c as u32) < 0x20 || c as u32 == 0x7F { - return Err(GwsError::Validation(format!( - "{flag_name} contains invalid control characters" - ))); - } - if is_rejected_unicode(c) { - return Err(GwsError::Validation(format!( - "{flag_name} contains invalid Unicode characters" - ))); - } - } - Ok(()) -} +// reject_control_chars is now a re-export from crate::output (see top of file) /// Resolves a path that may not exist yet by canonicalizing the existing /// prefix and appending remaining components. @@ -302,7 +269,7 @@ pub fn validate_resource_name(s: &str) -> Result<&str, GwsError> { ))); } if s.chars() - .any(|c| c == '\0' || c.is_control() || is_rejected_unicode(c)) + .any(|c| c == '\0' || c.is_control() || crate::output::is_dangerous_unicode(c)) { return Err(GwsError::Validation(format!( "Resource name contains invalid characters: {s}" From 1d169b504e7807fc71548484ce389839a9c0ae30 Mon Sep 17 00:00:00 2001 From: jpoehnelt-bot Date: Tue, 17 Mar 2026 17:16:38 -0600 Subject: [PATCH 2/4] 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. --- src/credential_store.rs | 2 +- src/error.rs | 4 +--- src/executor.rs | 3 ++- src/generate_skills.rs | 3 ++- src/helpers/events/subscribe.rs | 2 +- src/helpers/gmail/mod.rs | 2 +- src/helpers/gmail/read.rs | 12 +++++------- src/helpers/gmail/watch.rs | 2 +- src/helpers/workflows.rs | 3 ++- src/setup.rs | 3 ++- src/token_storage.rs | 2 +- src/validate.rs | 3 +-- 12 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/credential_store.rs b/src/credential_store.rs index 7770fe0e..52958e2b 100644 --- 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 index ba6295eb..40b14ba6 100644 --- a/src/error.rs +++ b/src/error.rs @@ -148,9 +148,7 @@ impl GwsError { } } -// Re-export from the consolidated output module so existing callers -// that import from `crate::error` continue to work. -pub(crate) use crate::output::{colorize, sanitize_for_terminal}; +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 index b6b8dadf..46f31ac4 100644 --- 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 index 07cf41fa..121d4c79 100644 --- 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 index 3a1c8e13..00474ac7 100644 --- 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 index b9ed7c8e..a8a079c1 100644 --- 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 index 71b24fed..c09d0858 100644 --- 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/watch.rs b/src/helpers/gmail/watch.rs index a8e2571f..43707ee6 100644 --- a/src/helpers/gmail/watch.rs +++ b/src/helpers/gmail/watch.rs @@ -1,8 +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"; diff --git a/src/helpers/workflows.rs b/src/helpers/workflows.rs index 6831f889..b921d864 100644 --- 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/setup.rs b/src/setup.rs index c3302c78..e11957b5 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -22,7 +22,8 @@ use std::process::Command; use serde_json::json; -use crate::error::{sanitize_for_terminal, GwsError}; +use crate::error::GwsError; +use crate::output::sanitize_for_terminal; use crate::setup_tui::{PickerResult, SelectItem, SetupWizard, StepStatus}; diff --git a/src/token_storage.rs b/src/token_storage.rs index 1cfa5bf1..8b63e61b 100644 --- a/src/token_storage.rs +++ b/src/token_storage.rs @@ -18,7 +18,7 @@ use std::sync::Arc; use tokio::sync::Mutex; use yup_oauth2::storage::{TokenInfo, TokenStorage, TokenStorageError}; -use crate::error::sanitize_for_terminal; +use crate::output::sanitize_for_terminal; /// A custom token storage implementation for `yup-oauth2` that encrypts /// the cached tokens at rest using AES-256-GCM encryption. diff --git a/src/validate.rs b/src/validate.rs index e4513107..2e0b5a96 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -21,8 +21,7 @@ use crate::error::GwsError; use std::path::{Path, PathBuf}; -// Re-export from the consolidated output module. -pub(crate) use crate::output::reject_dangerous_chars as reject_control_chars; +use crate::output::reject_dangerous_chars as reject_control_chars; /// Validates that `dir` is a safe output directory. /// From 7bcb63a2b139badf4c70c0eae7df67416f677b83 Mon Sep 17 00:00:00 2001 From: jpoehnelt-bot Date: Tue, 17 Mar 2026 17:18:30 -0600 Subject: [PATCH 3/4] fix: use char::is_control() in reject_dangerous_chars for C1 coverage Address PR review: the manual (c as u32) < 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. --- src/output.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/output.rs b/src/output.rs index 1be9b77e..0ddcc37d 100644 --- a/src/output.rs +++ b/src/output.rs @@ -62,14 +62,14 @@ pub(crate) fn sanitize_for_terminal(text: &str) -> String { .collect() } -/// Rejects strings containing null bytes, ASCII control characters -/// (including DEL, 0x7F), or dangerous Unicode characters such as -/// zero-width chars, bidi overrides, and Unicode line/paragraph separators. +/// Rejects strings containing control characters (C0: U+0000–U+001F, +/// C1: U+0080–U+009F, and DEL: U+007F) or dangerous Unicode characters +/// such as zero-width chars, bidi overrides, and line/paragraph separators. /// /// Used for validating CLI argument values at the parse boundary. pub(crate) fn reject_dangerous_chars(value: &str, flag_name: &str) -> Result<(), GwsError> { for c in value.chars() { - if (c as u32) < 0x20 || c as u32 == 0x7F { + if c.is_control() { return Err(GwsError::Validation(format!( "{flag_name} contains invalid control characters" ))); @@ -246,6 +246,13 @@ mod tests { assert!(reject_dangerous_chars("αβγ", "test").is_ok()); } + #[test] + fn reject_c1_control_csi() { + // U+009B is the C1 "Control Sequence Introducer" — can inject + // terminal escape sequences just like ESC+[ + assert!(reject_dangerous_chars("foo\u{009B}bar", "test").is_err()); + } + // ── colorize ────────────────────────────────────────────────── #[test] From db5713dca6119a13537440311813b0e69696a4f8 Mon Sep 17 00:00:00 2001 From: jpoehnelt-bot Date: Wed, 18 Mar 2026 09:20:57 -0600 Subject: [PATCH 4/4] 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. --- src/output.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/output.rs b/src/output.rs index 0ddcc37d..3aa0fec0 100644 --- a/src/output.rs +++ b/src/output.rs @@ -96,7 +96,7 @@ pub(crate) fn stderr_supports_color() -> bool { /// Returns the plain text unchanged when stderr is not a TTY or `NO_COLOR` /// is set. pub(crate) fn colorize(text: &str, ansi_color: &str) -> String { - if stderr_supports_color() { + if stderr_supports_color() && ansi_color.chars().all(|c| c.is_ascii_digit()) { format!("\x1b[1;{ansi_color}m{text}\x1b[0m") } else { text.to_string()