From b28f15252cd684ba7db410252e509c3c7b15e41f Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Mon, 4 May 2026 18:03:29 -0700 Subject: [PATCH] preparing for v1.99.0 --- src/alerts/discord.rs | 37 ++++++++++++++++++++++++++------ src/alerts/googlechat.rs | 31 ++++++++++++++++++++------- src/alerts/mattermost.rs | 29 +++++++++++++++++++------ src/alerts/slack.rs | 16 +++++++++++--- src/alerts/teams.rs | 29 +++++++++++++++++++------ src/update.rs | 46 +++++++++++++++++++++++++++++++++++----- 6 files changed, 154 insertions(+), 34 deletions(-) diff --git a/src/alerts/discord.rs b/src/alerts/discord.rs index 541f22c..8066e09 100644 --- a/src/alerts/discord.rs +++ b/src/alerts/discord.rs @@ -82,18 +82,18 @@ pub fn build_payload( let mut detail = String::new(); for f in findings.iter().take(take) { let snippet = if include_secret { - truncate(&f.finding.snippet, 32) + escape_for_code_span(&truncate(&f.finding.snippet, 32)) } else { - "".to_string() + "redacted".to_string() }; detail.push_str(&format!( "• `{}` at `{}:{}` — `{}` (validation: {}) — fp:`{}`\n", - f.rule.id, - f.finding.path, + escape_for_code_span(&f.rule.id), + escape_for_code_span(&f.finding.path), f.finding.line, snippet, - f.finding.validation.status, - f.finding.fingerprint, + escape_md(&f.finding.validation.status), + escape_for_code_span(&f.finding.fingerprint), )); } if findings.len() > take { @@ -137,6 +137,31 @@ fn truncate(s: &str, n: usize) -> String { format!("{prefix}…") } +/// Escape a value before embedding it in a backtick code span. Replace +/// backticks with U+02CB so a user-controlled value cannot terminate the +/// span and inject Discord markup, and normalize newlines so a single +/// finding does not fragment the bullet list. +fn escape_for_code_span(s: &str) -> String { + s.replace('`', "\u{02CB}").replace(['\n', '\r'], " ") +} + +/// Escape Discord markdown metacharacters in fields rendered outside a code +/// span (e.g. validation status). Backslash-escapes the common formatters. +fn escape_md(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for ch in s.chars() { + match ch { + '\\' | '*' | '_' | '~' | '`' | '|' | '>' | '<' | '[' | ']' | '(' | ')' => { + out.push('\\'); + out.push(ch); + } + '\n' | '\r' => out.push(' '), + _ => out.push(ch), + } + } + out +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/alerts/googlechat.rs b/src/alerts/googlechat.rs index 4b5d665..9e7e2db 100644 --- a/src/alerts/googlechat.rs +++ b/src/alerts/googlechat.rs @@ -41,14 +41,14 @@ pub fn build_payload( ]; if let Some(t) = &summary.target { summary_widgets.push(json!({ - "decoratedText": { "topLabel": "Target", "text": t } + "decoratedText": { "topLabel": "Target", "text": escape_html(t) } })); } if !summary.by_rule.is_empty() { let lines: Vec = summary .by_rule .iter() - .map(|(rule, count)| format!("• {rule} — {count}")) + .map(|(rule, count)| format!("• {} — {count}", escape_html(rule))) .collect(); summary_widgets.push(json!({ "textParagraph": { "text": format!("Top rules
{}", lines.join("
")) } @@ -65,18 +65,18 @@ pub fn build_payload( let mut detail = String::new(); for f in findings.iter().take(take) { let snippet = if include_secret { - truncate(&f.finding.snippet, 32) + escape_html(&truncate(&f.finding.snippet, 32)) } else { - "".to_string() + "redacted".to_string() }; detail.push_str(&format!( "• {} at {}:{} — {} (validation: {}) — fp:{}
", - f.rule.id, - f.finding.path, + escape_html(&f.rule.id), + escape_html(&f.finding.path), f.finding.line, snippet, - f.finding.validation.status, - f.finding.fingerprint, + escape_html(&f.finding.validation.status), + escape_html(&f.finding.fingerprint), )); } if findings.len() > take { @@ -137,6 +137,21 @@ fn truncate(s: &str, n: usize) -> String { format!("{prefix}…") } +/// Google Chat `textParagraph.text` is HTML-ish — ``, ``, `
`, etc. +/// are rendered as markup. Any user-controlled value (rule id, path, snippet, +/// fingerprint, validation status) must be HTML-escaped before interpolation, +/// otherwise an unescaped `<` could break out of a `` span and inject +/// arbitrary chat markup. Newlines are normalized to spaces so a single +/// finding does not fragment the bullet list. +fn escape_html(s: &str) -> String { + s.replace('&', "&") + .replace('<', "<") + .replace('>', ">") + .replace('"', """) + .replace('\'', "'") + .replace(['\n', '\r'], " ") +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/alerts/mattermost.rs b/src/alerts/mattermost.rs index 462e585..a2b6582 100644 --- a/src/alerts/mattermost.rs +++ b/src/alerts/mattermost.rs @@ -78,18 +78,18 @@ pub fn build_payload( let mut details = String::new(); for f in findings.iter().take(take) { let snippet = if include_secret { - truncate(&f.finding.snippet, 32) + escape_for_code_span(&truncate(&f.finding.snippet, 32)) } else { - "".to_string() + "redacted".to_string() }; details.push_str(&format!( "- **{}** at `{}:{}` — `{}` (validation: {}) — fp:`{}`\n", - f.rule.id, - f.finding.path, + escape_bold(&f.rule.id), + escape_for_code_span(&f.finding.path), f.finding.line, snippet, - f.finding.validation.status, - f.finding.fingerprint, + escape_bold(&f.finding.validation.status), + escape_for_code_span(&f.finding.fingerprint), )); } if findings.len() > take { @@ -127,6 +127,23 @@ fn plural(n: usize) -> &'static str { if n == 1 { "" } else { "s" } } +/// Escape a value before embedding it in a backtick code span. Replace +/// backticks with U+02CB so a user-controlled value cannot terminate the +/// span and inject markdown, and collapse newlines so a single finding +/// does not fragment the bullet list. +fn escape_for_code_span(s: &str) -> String { + s.replace('`', "\u{02CB}").replace(['\n', '\r'], " ") +} + +/// Escape values rendered inside a `**bold**` span — strip embedded `**` +/// and `_` so a user-controlled value cannot end the bold or start a link. +fn escape_bold(s: &str) -> String { + s.replace("**", "\u{02CB}\u{02CB}") + .replace('_', "\\_") + .replace('|', "\\|") + .replace(['\n', '\r'], " ") +} + fn truncate(s: &str, n: usize) -> String { if s.chars().count() <= n { return s.to_string(); diff --git a/src/alerts/slack.rs b/src/alerts/slack.rs index 363f410..736b607 100644 --- a/src/alerts/slack.rs +++ b/src/alerts/slack.rs @@ -60,12 +60,12 @@ pub fn build_payload( let mut detail_lines: Vec = Vec::with_capacity(take); for f in findings.iter().take(take) { let snippet = if include_secret { - truncate(&f.finding.snippet, 32) + escape_for_code_span(&truncate(&f.finding.snippet, 32)) } else { - "".to_string() + "redacted".to_string() }; detail_lines.push(format!( - "• `{}` at `{}:{}` — {} (validation: {}) — fp:`{}`", + "• `{}` at `{}:{}` — `{}` (validation: {}) — fp:`{}`", escape_mrkdwn(&f.rule.id), escape_mrkdwn(&f.finding.path), f.finding.line, @@ -131,6 +131,16 @@ fn escape_mrkdwn(s: &str) -> String { s.replace('&', "&").replace('<', "<").replace('>', ">") } +/// Sanitize a value before it goes inside a backtick code span. We escape +/// the same `<>&` mrkdwn metacharacters and replace embedded backticks with +/// a similar-looking U+02CB (modifier letter grave accent) so a user-controlled +/// value cannot break out of the span and inject Slack markup or `` +/// links. Newlines are normalized to spaces so a single finding does not +/// fragment the bullet list. +fn escape_for_code_span(s: &str) -> String { + escape_mrkdwn(s).replace('`', "\u{02CB}").replace(['\n', '\r'], " ") +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/alerts/teams.rs b/src/alerts/teams.rs index c695bbe..5e42c8d 100644 --- a/src/alerts/teams.rs +++ b/src/alerts/teams.rs @@ -55,18 +55,18 @@ pub fn build_payload( let mut details = String::new(); for f in findings.iter().take(take) { let snippet = if include_secret { - truncate(&f.finding.snippet, 32) + escape_for_code_span(&truncate(&f.finding.snippet, 32)) } else { - "".to_string() + "redacted".to_string() }; details.push_str(&format!( "- **{}** at `{}:{}` — `{}` (validation: {}) — fp:`{}`\n", - f.rule.id, - f.finding.path, + escape_bold(&f.rule.id), + escape_for_code_span(&f.finding.path), f.finding.line, snippet, - f.finding.validation.status, - f.finding.fingerprint, + escape_bold(&f.finding.validation.status), + escape_for_code_span(&f.finding.fingerprint), )); } if findings.len() > take { @@ -111,6 +111,23 @@ fn plural(n: usize) -> &'static str { if n == 1 { "" } else { "s" } } +/// Escape a value before embedding it in a backtick code span. Replace +/// backticks with U+02CB so a user-controlled value cannot terminate the +/// span and inject Teams markdown, and collapse newlines so a single +/// finding does not fragment the bullet list. +fn escape_for_code_span(s: &str) -> String { + s.replace('`', "\u{02CB}").replace(['\n', '\r'], " ") +} + +/// Escape values rendered inside a `**bold**` span — strip embedded `**` +/// and `_` so a user-controlled value cannot end the bold or start a link. +fn escape_bold(s: &str) -> String { + s.replace("**", "\u{02CB}\u{02CB}") + .replace('_', "\\_") + .replace('|', "\\|") + .replace(['\n', '\r'], " ") +} + fn truncate(s: &str, n: usize) -> String { if s.chars().count() <= n { return s.to_string(); diff --git a/src/update.rs b/src/update.rs index 5e5c036..e231aed 100644 --- a/src/update.rs +++ b/src/update.rs @@ -349,6 +349,11 @@ pub fn rewrite_argv_for_reexec(argv: impl IntoIterator) -> Vec< let mut out: Vec = Vec::new(); let mut already_has_no_update_check = false; let mut hit_double_dash = false; + // Index of the `--` separator inside `out`, if seen. When set, we must + // insert any synthesized flags (like `--no-update-check`) *before* this + // index — clap stops parsing flags at `--` and treats everything after + // as positional. + let mut double_dash_idx: Option = None; let had_argv0; if let Some(argv0) = iter.next() { @@ -367,6 +372,7 @@ pub fn rewrite_argv_for_reexec(argv: impl IntoIterator) -> Vec< if tok == "--" { hit_double_dash = true; + double_dash_idx = Some(out.len()); out.push(tok); continue; } @@ -386,11 +392,19 @@ pub fn rewrite_argv_for_reexec(argv: impl IntoIterator) -> Vec< out.push(tok); } - // Only append --no-update-check when we actually preserved an argv[0]. In the - // theoretical empty-input case, returning an empty Vec keeps argv shape-consistent - // and lets the caller decide what to do. + // Only inject `--no-update-check` when we actually preserved an argv[0]. + // In the theoretical empty-input case, returning an empty Vec keeps the + // argv shape consistent and lets the caller decide what to do. + // + // If a `--` separator was present, insert the flag *before* it — anything + // after `--` is positional, so appending at the end would silently fail + // to suppress the update check on the re-exec'd process. if had_argv0 && !already_has_no_update_check { - out.push(OsString::from("--no-update-check")); + let flag = OsString::from("--no-update-check"); + match double_dash_idx { + Some(idx) => out.insert(idx, flag), + None => out.push(flag), + } } out @@ -462,6 +476,9 @@ mod tests { #[test] fn rewrite_argv_preserves_tokens_after_double_dash() { // --self-update appearing AFTER `--` is a positional and must be preserved. + // The synthesized --no-update-check has to land BEFORE `--`; otherwise it + // would be parsed as a positional and the re-exec'd process would still + // perform an update check. let result = rewrite_argv_for_reexec(argv(&[ "kingfisher", "scan", @@ -472,7 +489,26 @@ mod tests { ])); assert_eq!( result, - argv(&["kingfisher", "scan", "--", "--self-update", "--update", "--no-update-check"]) + argv(&["kingfisher", "scan", "--no-update-check", "--", "--self-update", "--update"]) + ); + } + + #[test] + fn rewrite_argv_does_not_duplicate_no_update_check_when_already_present_before_double_dash() { + // If --no-update-check is already present before `--`, the function is + // a no-op for that flag (no duplicate insertion) and tokens after `--` + // pass through verbatim. + let result = rewrite_argv_for_reexec(argv(&[ + "kingfisher", + "scan", + "--no-update-check", + "--self-update", + "--", + "--self-update", + ])); + assert_eq!( + result, + argv(&["kingfisher", "scan", "--no-update-check", "--", "--self-update"]) ); }