forked from mirrors/kingfisher
preparing for v1.99.0
This commit is contained in:
parent
b28f15252c
commit
bacdca6a52
5 changed files with 130 additions and 25 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<String> =
|
||||
summary.by_rule.iter().map(|(rule, count)| format!("• `{rule}` — {count}")).collect();
|
||||
let lines: Vec<String> = 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),
|
||||
|
|
|
|||
|
|
@ -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<String> =
|
||||
summary.by_rule.iter().map(|(rule, count)| format!("• `{rule}` — {count}")).collect();
|
||||
let lines: Vec<String> = summary
|
||||
.by_rule
|
||||
.iter()
|
||||
.map(|(rule, count)| format!("• `{}` — {count}", escape_for_code_span(rule)))
|
||||
.collect();
|
||||
fields.push(json!({
|
||||
"short": false,
|
||||
"title": "Top rules",
|
||||
|
|
|
|||
|
|
@ -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/<secret>`) 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::<std::net::IpAddr>() {
|
||||
return ip.is_loopback();
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
fn redact_for_log(url: &str) -> String {
|
||||
redact_webhook(url)
|
||||
}
|
||||
|
|
@ -406,6 +448,42 @@ mod tests {
|
|||
assert_eq!(r, "<unparseable webhook url>");
|
||||
}
|
||||
|
||||
#[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!(
|
||||
|
|
|
|||
|
|
@ -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<String> = 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 `<url|text>` 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,
|
||||
}]
|
||||
}));
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue