From d0694c34f808561fad0a0e61aaa3791e8d2bbe03 Mon Sep 17 00:00:00 2001 From: Varun Nuthalapati Date: Sun, 12 Apr 2026 00:17:50 -0700 Subject: [PATCH 1/3] fix(formatter): strip terminal escape sequences from non-JSON output API responses may contain user-generated content with embedded ANSI escape codes or C0/C1 control characters. JSON output is safe because serde escapes them as \uXXXX, but table/CSV/YAML formats passed strings through verbatim, allowing a malicious API value to inject terminal sequences into the user's terminal. Adds strip_control_chars() which removes CSI sequences (ESC [ ... final), OSC sequences (ESC ] ... BEL/ST), other Fe two-char sequences, and bare control characters (except tab and newline). Called from value_to_cell() so every string field rendered by format_value() in non-JSON modes is sanitized. Fixes #635 --- ...fix-formatter-sanitize-terminal-escapes.md | 11 +++ crates/google-workspace-cli/src/formatter.rs | 99 ++++++++++++++++++- 2 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 .changeset/fix-formatter-sanitize-terminal-escapes.md diff --git a/.changeset/fix-formatter-sanitize-terminal-escapes.md b/.changeset/fix-formatter-sanitize-terminal-escapes.md new file mode 100644 index 00000000..48e6fda4 --- /dev/null +++ b/.changeset/fix-formatter-sanitize-terminal-escapes.md @@ -0,0 +1,11 @@ +--- +"@googleworkspace/cli": patch +--- + +fix(formatter): strip terminal escape sequences from non-JSON output + +API responses may contain user-generated content with ANSI escape codes or +other control characters. JSON output is safe because serde escapes them as +\uXXXX, but table/CSV/YAML formats passed strings verbatim, allowing a +malicious API value to inject terminal sequences. Adds strip_control_chars() +which is applied to every string cell in value_to_cell(). diff --git a/crates/google-workspace-cli/src/formatter.rs b/crates/google-workspace-cli/src/formatter.rs index 08d4d287..2a3180e1 100644 --- a/crates/google-workspace-cli/src/formatter.rs +++ b/crates/google-workspace-cli/src/formatter.rs @@ -418,10 +418,63 @@ fn csv_escape(s: &str) -> String { } } +/// Strips ANSI/VT terminal escape sequences and C0/C1 control characters +/// (except `\t` and `\n`) from a string. +/// +/// API responses may contain user-generated content with embedded escape codes. +/// JSON output is safe because serde serialises control characters as `\uXXXX`, +/// but table/CSV/YAML formats pass strings through verbatim, which would allow +/// a malicious API value to inject terminal sequences into the user's terminal. +fn strip_control_chars(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + let mut chars = s.chars().peekable(); + while let Some(c) = chars.next() { + match c { + // ESC-prefixed sequences: consume until the sequence terminator. + '\x1b' => { + match chars.peek() { + // CSI sequences: ESC [ ... + Some('[') => { + chars.next(); + for ch in chars.by_ref() { + if ('\x40'..='\x7e').contains(&ch) { + break; + } + } + } + // OSC sequences: ESC ] ... ST (BEL or ESC \) + Some(']') => { + chars.next(); + while let Some(ch) = chars.next() { + if ch == '\x07' { + break; // BEL string terminator + } + if ch == '\x1b' { + chars.next(); // consume the \ of ESC \ + break; + } + } + } + // Other Fe sequences: ESC <0x40–0x5F> — consume one char. + Some(&ch) if ('\x40'..='\x5f').contains(&ch) => { + chars.next(); + } + _ => {} + } + } + // Allow tab and newline; strip all other C0/C1 control characters. + '\t' | '\n' => out.push(c), + c if c.is_control() => {} + c => out.push(c), + } + } + out +} + fn value_to_cell(value: &Value) -> String { match value { Value::Null => String::new(), - Value::String(s) => s.clone(), + Value::String(s) => strip_control_chars(s), Value::Bool(b) => b.to_string(), Value::Number(n) => n.to_string(), Value::Array(arr) => { @@ -629,6 +682,50 @@ mod tests { assert_eq!(csv_escape("has\"quote"), "\"has\"\"quote\""); } + #[test] + fn test_strip_control_chars_clean_string() { + assert_eq!(strip_control_chars("hello world"), "hello world"); + assert_eq!(strip_control_chars("tab\there"), "tab\there"); + assert_eq!(strip_control_chars("line\nbreak"), "line\nbreak"); + } + + #[test] + fn test_strip_control_chars_csi_sequence() { + // SGR colour code: ESC [ 31 m + assert_eq!(strip_control_chars("\x1b[31mred\x1b[0m"), "red"); + // Bold: ESC [ 1 m + assert_eq!(strip_control_chars("\x1b[1mbold\x1b[m"), "bold"); + } + + #[test] + fn test_strip_control_chars_osc_sequence() { + // OSC title injection: ESC ] 0 ; malicious BEL + assert_eq!( + strip_control_chars("\x1b]0;malicious\x07clean"), + "clean" + ); + // OSC terminated by ESC \ + assert_eq!( + strip_control_chars("\x1b]2;title\x1b\\clean"), + "clean" + ); + } + + #[test] + fn test_strip_control_chars_c0_control() { + // NUL, BEL, BS, CR stripped; tab and newline kept + assert_eq!(strip_control_chars("a\x00b"), "ab"); + assert_eq!(strip_control_chars("a\x07b"), "ab"); + assert_eq!(strip_control_chars("a\x08b"), "ab"); + assert_eq!(strip_control_chars("a\rb"), "ab"); + } + + #[test] + fn test_value_to_cell_sanitizes_escape_sequences() { + let val = Value::String("\x1b[31mred\x1b[0m".to_string()); + assert_eq!(value_to_cell(&val), "red"); + } + #[test] fn test_format_yaml() { let val = json!({"name": "test", "count": 42}); From 3710151719fdd37deb1e09ae7f294d03b2f88044 Mon Sep 17 00:00:00 2001 From: Varun Nuthalapati Date: Sun, 12 Apr 2026 10:00:54 -0700 Subject: [PATCH 2/3] fix(formatter): handle DCS/SOS/PM/APC sequences and sanitize YAML strings Two issues from review: 1. DCS (ESC P), SOS (ESC X), PM (ESC ^), and APC (ESC _) sequences were not handled. The previous 'Other Fe sequences' arm consumed only the introducer byte, leaving the sequence body and ST terminator in the output. Each now calls consume_until_st() (extracted helper shared with OSC) which drains chars until BEL or ESC-backslash ST. 2. The YAML formatter (json_to_yaml) built strings directly from the raw API value without going through value_to_cell, so escape sequences survived in YAML output. Apply strip_control_chars() at the top of the String branch in json_to_yaml so all three non-JSON formats are covered. Adds tests for DCS/SOS/PM/APC stripping and YAML sanitization. --- crates/google-workspace-cli/src/formatter.rs | 92 +++++++++++++++++--- 1 file changed, 78 insertions(+), 14 deletions(-) diff --git a/crates/google-workspace-cli/src/formatter.rs b/crates/google-workspace-cli/src/formatter.rs index 2a3180e1..d673d5ab 100644 --- a/crates/google-workspace-cli/src/formatter.rs +++ b/crates/google-workspace-cli/src/formatter.rs @@ -281,6 +281,7 @@ fn json_to_yaml(value: &Value, indent: usize) -> String { Value::Bool(b) => b.to_string(), Value::Number(n) => n.to_string(), Value::String(s) => { + let s = strip_control_chars(s); if s.contains('\n') { // Genuine multi-line content: block scalar is the most readable choice. format!( @@ -425,15 +426,39 @@ fn csv_escape(s: &str) -> String { /// JSON output is safe because serde serialises control characters as `\uXXXX`, /// but table/CSV/YAML formats pass strings through verbatim, which would allow /// a malicious API value to inject terminal sequences into the user's terminal. +/// +/// Sequences handled: +/// - CSI `ESC [` … final-byte (SGR colours, cursor movement, …) +/// - OSC `ESC ]` … BEL or ST (window title, hyperlinks, …) +/// - DCS `ESC P` … ST (device-control strings) +/// - SOS `ESC X` … ST (start-of-string) +/// - PM `ESC ^` … ST (privacy message) +/// - APC `ESC _` … ST (application-program command) +/// - Other two-char Fe sequences (`ESC` + 0x40–0x5F, not in the above) +/// - Bare C0/C1 control characters (NUL, BEL, BS, CR, …; tab/newline kept) fn strip_control_chars(s: &str) -> String { let mut out = String::with_capacity(s.len()); let mut chars = s.chars().peekable(); + + /// Consume chars until ST (BEL `\x07` or `ESC \`), used for OSC/DCS/SOS/PM/APC. + fn consume_until_st(chars: &mut std::iter::Peekable>) { + while let Some(ch) = chars.next() { + if ch == '\x07' { + break; + } + if ch == '\x1b' { + chars.next(); // consume the `\` of ESC \ + break; + } + } + } + while let Some(c) = chars.next() { match c { // ESC-prefixed sequences: consume until the sequence terminator. '\x1b' => { - match chars.peek() { - // CSI sequences: ESC [ ... + match chars.peek().copied() { + // CSI: ESC [ … Some('[') => { chars.next(); for ch in chars.by_ref() { @@ -442,21 +467,33 @@ fn strip_control_chars(s: &str) -> String { } } } - // OSC sequences: ESC ] ... ST (BEL or ESC \) + // OSC: ESC ] … ST Some(']') => { chars.next(); - while let Some(ch) = chars.next() { - if ch == '\x07' { - break; // BEL string terminator - } - if ch == '\x1b' { - chars.next(); // consume the \ of ESC \ - break; - } - } + consume_until_st(&mut chars); + } + // DCS: ESC P … ST + Some('P') => { + chars.next(); + consume_until_st(&mut chars); + } + // SOS: ESC X … ST + Some('X') => { + chars.next(); + consume_until_st(&mut chars); } - // Other Fe sequences: ESC <0x40–0x5F> — consume one char. - Some(&ch) if ('\x40'..='\x5f').contains(&ch) => { + // PM: ESC ^ … ST + Some('^') => { + chars.next(); + consume_until_st(&mut chars); + } + // APC: ESC _ … ST + Some('_') => { + chars.next(); + consume_until_st(&mut chars); + } + // Other Fe two-char sequences: ESC <0x40–0x5F> — consume one char. + Some(ch) if ('\x40'..='\x5f').contains(&ch) => { chars.next(); } _ => {} @@ -720,12 +757,39 @@ mod tests { assert_eq!(strip_control_chars("a\rb"), "ab"); } + #[test] + fn test_strip_control_chars_dcs_sequence() { + // DCS: ESC P … ST (BEL-terminated) + assert_eq!(strip_control_chars("\x1bPdcs-payload\x07clean"), "clean"); + // DCS: ESC P … ST (ESC \-terminated) + assert_eq!(strip_control_chars("\x1bPdcs-payload\x1b\\clean"), "clean"); + } + + #[test] + fn test_strip_control_chars_sos_pm_apc_sequences() { + // SOS: ESC X … ST + assert_eq!(strip_control_chars("\x1bXsos-payload\x07clean"), "clean"); + // PM: ESC ^ … ST + assert_eq!(strip_control_chars("\x1b^pm-payload\x07clean"), "clean"); + // APC: ESC _ … ST + assert_eq!(strip_control_chars("\x1b_apc-payload\x07clean"), "clean"); + } + #[test] fn test_value_to_cell_sanitizes_escape_sequences() { let val = Value::String("\x1b[31mred\x1b[0m".to_string()); assert_eq!(value_to_cell(&val), "red"); } + #[test] + fn test_format_yaml_sanitizes_escape_sequences() { + // YAML strings must also be sanitized. + let val = json!({"title": "\x1b]0;injected\x07safe"}); + let output = format_value(&val, &OutputFormat::Yaml); + assert!(output.contains("safe")); + assert!(!output.contains("\x1b")); + } + #[test] fn test_format_yaml() { let val = json!({"name": "test", "count": 42}); From 27bd7b62611e1c6a7ff6524adab17afb04ca27e5 Mon Sep 17 00:00:00 2001 From: Varun Nuthalapati Date: Sun, 12 Apr 2026 10:26:36 -0700 Subject: [PATCH 3/3] fix(formatter): precise ESC-backslash check in consume_until_st Previously, any ESC byte inside a control string caused the next character to be consumed unconditionally. If the ESC was not followed by a backslash (e.g., a malformed or nested sequence), that character would be silently dropped. Now peek() checks for '\' before consuming, so only the valid ESC \ String Terminator is consumed; other ESC bytes cause an immediate break without over-consuming. --- crates/google-workspace-cli/src/formatter.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/google-workspace-cli/src/formatter.rs b/crates/google-workspace-cli/src/formatter.rs index d673d5ab..7dee2deb 100644 --- a/crates/google-workspace-cli/src/formatter.rs +++ b/crates/google-workspace-cli/src/formatter.rs @@ -444,10 +444,12 @@ fn strip_control_chars(s: &str) -> String { fn consume_until_st(chars: &mut std::iter::Peekable>) { while let Some(ch) = chars.next() { if ch == '\x07' { - break; + break; // BEL string terminator } if ch == '\x1b' { - chars.next(); // consume the `\` of ESC \ + if let Some('\\') = chars.peek() { + chars.next(); // consume the `\` of ESC \ + } break; } }