diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 7155457..c9ed36d 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -55,6 +55,17 @@ omitted to keep the file minimal. Scan-target inputs (paths, `--git-url`, GitHub/GitLab/etc. flags, S3/GCS buckets) are stripped — they describe *what* this run scans and don't belong in shared project policy. +## Webhook URL policy + +`alerts.webhooks[].url` (and `--alert-webhook URL`) **must use `https://`**. +Webhook URLs typically embed a secret token in the path and the alert +payload contains finding metadata, so cleartext transport is never the right +default. `http://` is allowed only when the host is a loopback address +(`localhost`, `127.0.0.0/8`, `::1`) — useful for local development against an +on-host receiver. Loopback decisions are made on the literal hostname / IP +in the URL; we do not consult DNS, so a resolver cannot trick the validator +into permitting `http://` for a remote host. + ## Caveats - **`scan.jobs` and the Tokio runtime.** The Tokio runtime is sized from the diff --git a/src/alerts/discord.rs b/src/alerts/discord.rs index 8066e09..58598a5 100644 --- a/src/alerts/discord.rs +++ b/src/alerts/discord.rs @@ -56,13 +56,16 @@ pub fn build_payload( if let Some(t) = &summary.target { fields.push(json!({ "name": "Target", - "value": format!("`{}`", truncate(t, 1000)), + "value": format!("`{}`", escape_for_code_span(&truncate(t, 1000))), "inline": false, })); } if !summary.by_rule.is_empty() { - let lines: Vec = - summary.by_rule.iter().map(|(rule, count)| format!("• `{rule}` — {count}")).collect(); + let lines: Vec = summary + .by_rule + .iter() + .map(|(rule, count)| format!("• `{}` — {count}", escape_for_code_span(rule))) + .collect(); fields.push(json!({ "name": "Top rules", "value": truncate(&lines.join("\n"), 1000), diff --git a/src/alerts/mattermost.rs b/src/alerts/mattermost.rs index a2b6582..dda633b 100644 --- a/src/alerts/mattermost.rs +++ b/src/alerts/mattermost.rs @@ -54,11 +54,18 @@ pub fn build_payload( json!({ "short": true, "title": "Unknown", "value": summary.unknown.to_string() }), ]; if let Some(t) = &summary.target { - fields.push(json!({ "short": false, "title": "Target", "value": format!("`{t}`") })); + fields.push(json!({ + "short": false, + "title": "Target", + "value": format!("`{}`", escape_for_code_span(t)), + })); } if !summary.by_rule.is_empty() { - let lines: Vec = - summary.by_rule.iter().map(|(rule, count)| format!("• `{rule}` — {count}")).collect(); + let lines: Vec = summary + .by_rule + .iter() + .map(|(rule, count)| format!("• `{}` — {count}", escape_for_code_span(rule))) + .collect(); fields.push(json!({ "short": false, "title": "Top rules", diff --git a/src/alerts/mod.rs b/src/alerts/mod.rs index dccef16..e92420f 100644 --- a/src/alerts/mod.rs +++ b/src/alerts/mod.rs @@ -204,24 +204,66 @@ fn host_matches(host: &str, suffix: &str) -> bool { host == suffix || host.ends_with(&format!(".{suffix}")) } -/// Validate a webhook URL: must parse, must use http(s) scheme, must have a -/// host. Returns the parsed URL on success. +/// Validate a webhook URL. +/// +/// Webhook URLs typically embed a secret token in the path (e.g. +/// `hooks.slack.com/services/T0/B0/`) and the payload contains +/// finding metadata, so the transport must protect both. Default policy: +/// +/// * Must parse and have a non-empty host. +/// * Scheme must be `https`. +/// * `http` is allowed *only* when the host is a loopback address +/// (`localhost`, `127.0.0.0/8`, `::1`) — useful for local development and +/// on-host webhook receivers without exposing webhooks-in-the-clear on a +/// network. pub fn validate_webhook_url(url: &str) -> Result<()> { let parsed = url::Url::parse(url) .with_context(|| format!("invalid webhook URL `{}`", redact_for_log(url)))?; let scheme = parsed.scheme(); - if scheme != "https" && scheme != "http" { - anyhow::bail!( - "webhook URL `{}` uses unsupported scheme `{scheme}` (only http/https are allowed)", - redact_for_log(url) - ); - } - if parsed.host_str().is_none_or(|h| h.is_empty()) { + let host = parsed.host_str().unwrap_or(""); + if host.is_empty() { anyhow::bail!("webhook URL `{}` has no host", redact_for_log(url)); } + match scheme { + "https" => {} + "http" if is_loopback_host(host) => {} + "http" => { + anyhow::bail!( + "webhook URL `{}` uses cleartext `http://`; webhook tokens and finding \ + metadata must not traverse the network unencrypted. Use `https://`, or a \ + loopback host (`localhost`/`127.0.0.1`/`::1`) for local testing.", + redact_for_log(url) + ); + } + _ => { + anyhow::bail!( + "webhook URL `{}` uses unsupported scheme `{scheme}` (only `https` is \ + allowed; `http` is allowed only for loopback hosts)", + redact_for_log(url) + ); + } + } Ok(()) } +/// True when `host` resolves unambiguously to the local machine — i.e. the +/// loopback hostname or any IPv4 in `127.0.0.0/8` or the IPv6 loopback `::1`. +/// We deliberately do not consult DNS; only literal hostnames and IP +/// literals count, so a malicious resolver cannot trick us into accepting +/// `http://` for a remote host. +fn is_loopback_host(host: &str) -> bool { + if host.eq_ignore_ascii_case("localhost") { + return true; + } + // `url::Url::host_str` keeps the surrounding `[...]` on IPv6 literals; + // `IpAddr::from_str` rejects that form, so strip the brackets first. + let trimmed = host.strip_prefix('[').and_then(|s| s.strip_suffix(']')).unwrap_or(host); + if let Ok(ip) = trimmed.parse::() { + return ip.is_loopback(); + } + false +} + fn redact_for_log(url: &str) -> String { redact_webhook(url) } @@ -406,6 +448,42 @@ mod tests { assert_eq!(r, ""); } + #[test] + fn validate_webhook_accepts_https() { + validate_webhook_url("https://hooks.slack.com/services/T0/B0/XXX").unwrap(); + } + + #[test] + fn validate_webhook_rejects_remote_http() { + let err = validate_webhook_url("http://example.com/hook").unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("cleartext `http://`"), "got: {msg}"); + } + + #[test] + fn validate_webhook_allows_http_localhost() { + validate_webhook_url("http://localhost:8080/hook").unwrap(); + validate_webhook_url("http://127.0.0.1:9000/hook").unwrap(); + validate_webhook_url("http://[::1]:9000/hook").unwrap(); + } + + #[test] + fn validate_webhook_rejects_unknown_scheme() { + let err = validate_webhook_url("ftp://example.com/hook").unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("unsupported scheme"), "got: {msg}"); + } + + #[test] + fn validate_webhook_rejects_no_host() { + // url::Url::parse on a relative-style file URL leaves no host. + let err = validate_webhook_url("file:///etc/passwd").unwrap_err(); + let msg = format!("{err:#}"); + // Either "no host" or "unsupported scheme" is acceptable; both are + // hard rejections. + assert!(msg.contains("no host") || msg.contains("unsupported scheme"), "got: {msg}"); + } + #[test] fn infer_format_slack() { assert_eq!( diff --git a/src/alerts/slack.rs b/src/alerts/slack.rs index 736b607..6cc2162 100644 --- a/src/alerts/slack.rs +++ b/src/alerts/slack.rs @@ -35,7 +35,7 @@ pub fn build_payload( "type": "section", "text": { "type": "mrkdwn", - "text": format!("*Target:* `{}`", escape_mrkdwn(target)) + "text": format!("*Target:* `{}`", escape_for_code_span(target)) } })); } @@ -44,7 +44,7 @@ pub fn build_payload( let lines: Vec = summary .by_rule .iter() - .map(|(rule_id, count)| format!("• `{}` — {}", escape_mrkdwn(rule_id), count)) + .map(|(rule_id, count)| format!("• `{}` — {}", escape_for_code_span(rule_id), count)) .collect(); blocks.push(json!({ "type": "section", @@ -66,12 +66,12 @@ pub fn build_payload( }; detail_lines.push(format!( "• `{}` at `{}:{}` — `{}` (validation: {}) — fp:`{}`", - escape_mrkdwn(&f.rule.id), - escape_mrkdwn(&f.finding.path), + escape_for_code_span(&f.rule.id), + escape_for_code_span(&f.finding.path), f.finding.line, snippet, escape_mrkdwn(&f.finding.validation.status), - escape_mrkdwn(&f.finding.fingerprint), + escape_for_code_span(&f.finding.fingerprint), )); } if findings.len() > take { @@ -98,12 +98,18 @@ pub fn build_payload( } if let Some(url) = &summary.report_url { + // Render as a Block Kit `actions` block with a `button` element. The + // button takes the URL via a separate JSON field, so we sidestep the + // mrkdwn `` link syntax entirely — a URL containing `|` or + // `>` cannot inject markup or break the link the way it could in + // `<{}|Full report →>`. blocks.push(json!({ - "type": "section", - "text": { - "type": "mrkdwn", - "text": format!("<{}|Full report →>", url) - } + "type": "actions", + "elements": [{ + "type": "button", + "text": { "type": "plain_text", "text": "Full report" }, + "url": url, + }] })); }