Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/notify-kit/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ The format is based on *Keep a Changelog*, and this project adheres to *Semantic
- `notify-kit` 的 webhook/API sinks 现在共享一个内部 outbound transport helper,统一 `HttpClientProfile` 创建、公网 IP 选择和 strict eager 校验入口,后续新增 transport 级约束时不再需要在每个 sink 里散弹式重复接线。

### Fixed
- `GenericWebhookConfig::with_public_ip_check(false)` 不再与默认构造路径形成不可达契约:`GenericWebhookSink::new(...)` 现在允许关闭公网 IP 校验,但前提是调用方显式提供 `allowed_hosts` 这类安全边界;默认从 URL 派生出的 host/path 约束不再被误当成显式授权,而 `new_strict(...)` 仍继续拒绝关闭公网 IP 校验。
- `Hub::send` 在 sink 进入 poisoned 状态后不再静默跳过并返回成功;后续发送会继续返回结构化 sink failure,避免“假成功”掩盖已失效的通知通道。
- tests: add a crate-level regression check that `notify_kit::core` continues to match the root compatibility re-exports, so the review-root `core`-module regression stays mechanically covered.
- `notify_kit::core` 现在补上一段最小 rustdoc 编译示例,显式锁住 `core` 入口与 crate root 兼容 re-export 会同时保持可用,避免 review 命中过的“`core` 模块被删但入口未同步”回归再次悄悄漂进门禁前。
Expand Down
43 changes: 38 additions & 5 deletions crates/notify-kit/src/sinks/generic_webhook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,17 @@ fn normalize_config(

let url = parse_and_validate_https_url_basic(&url)?;
let payload_field = normalize_payload_field(payload_field)?;
let explicit_allowed_hosts = normalize_nonempty_trimmed_vec(allowed_hosts.clone());
let explicit_path_prefix = path_prefix
.as_ref()
.and_then(|prefix| normalize_optional_trimmed(prefix.clone()));
let (allowed_hosts, path_prefix) =
normalize_security_scope(&url, allowed_hosts, path_prefix, mode)?;

validate_security_requirements(
enforce_public_ip,
&explicit_allowed_hosts,
explicit_path_prefix.as_deref(),
&allowed_hosts,
path_prefix.as_deref(),
mode,
Expand Down Expand Up @@ -267,6 +273,8 @@ fn normalize_allowed_hosts(

fn validate_security_requirements(
enforce_public_ip: bool,
explicit_allowed_hosts: &[String],
explicit_path_prefix: Option<&str>,
allowed_hosts: &[String],
path_prefix: Option<&str>,
mode: GenericWebhookValidationMode,
Expand All @@ -277,15 +285,15 @@ fn validate_security_requirements(
anyhow::anyhow!("generic webhook strict mode requires public ip check").into(),
);
}
if mode == GenericWebhookValidationMode::StrictByDefault {
if explicit_allowed_hosts.is_empty() {
return Err(anyhow::anyhow!(
"generic webhook default constructor requires public ip check"
"generic webhook disabling public ip check requires explicit allowed_hosts"
)
.into());
}
if allowed_hosts.is_empty() {
if explicit_path_prefix.is_none() && path_prefix.is_none() {
return Err(anyhow::anyhow!(
"generic webhook disabling public ip check requires allowed_hosts"
"generic webhook disabling public ip check requires path_prefix"
)
.into());
}
Expand Down Expand Up @@ -409,7 +417,32 @@ mod tests {
let cfg =
GenericWebhookConfig::new("https://example.com/webhook").with_public_ip_check(false);
let err = GenericWebhookSink::new(cfg).expect_err("expected invalid config");
assert!(err.to_string().contains("public ip"), "{err:#}");
assert!(
err.to_string().contains("explicit allowed_hosts"),
"{err:#}"
);
}

#[test]
fn disabling_public_ip_check_accepts_explicit_allowlist() {
let cfg = GenericWebhookConfig::new("https://example.com/hooks/notify")
.with_public_ip_check(false)
.with_allowed_hosts(vec!["example.com".to_string()]);
let sink = GenericWebhookSink::new(cfg).expect("build sink");
assert_eq!(sink.url.host_str().unwrap_or(""), "example.com");
assert!(sink.url.path().starts_with("/hooks/"));
}

#[test]
fn disabling_public_ip_check_rejects_derived_allowlist_only() {
let cfg = GenericWebhookConfig::new("https://example.com/hooks/notify")
.with_public_ip_check(false)
.with_allowed_hosts(vec![" ".to_string()]);
let err = GenericWebhookSink::new(cfg).expect_err("expected invalid config");
assert!(
err.to_string().contains("explicit allowed_hosts"),
"{err:#}"
);
}

#[test]
Expand Down