Skip to content

Commit 391c497

Browse files
committed
This patch re-enables masking with a few backend changes
- Add should_redact_header() to aggressively neutralize sensitive headers. - Add mask_url() using safe rfind('/') path truncation. - Apply consistent masking across Slack, Webhook, and AlertManager types. - Fix AlertManager JSON type key from "webhook" to "alertManager". - Add strict unit tests enforcing that secrets never hit the serializer.
1 parent c849704 commit 391c497

2 files changed

Lines changed: 123 additions & 32 deletions

File tree

src/alerts/target.rs

Lines changed: 118 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -195,51 +195,73 @@ pub struct Target {
195195
}
196196

197197
impl Target {
198+
/// Evaluates an HTTP header name to determine if its value is likely a secret.
199+
/// Uses a strict allowlist of known sensitive headers (Authorization, Cookies)
200+
/// combined with a broad blocklist of dangerous substrings (token, secret, key)
201+
/// to catch custom headers like `X-Api-Token`.
202+
fn should_redact_header(name: &str) -> bool {
203+
let lower = name.to_lowercase();
204+
matches!(
205+
lower.as_str(),
206+
"authorization" | "proxy-authorization" | "cookie" | "set-cookie"
207+
) || lower.contains("token")
208+
|| lower.contains("secret")
209+
|| lower.contains("key")
210+
}
211+
/// Sanitizes a URL by obliterating the trailing path segment.
212+
fn mask_url(url: &Url) -> String {
213+
let url_str = url.to_string();
214+
if let Some(pos) = url_str.rfind('/') {
215+
let mut masked = url_str;
216+
masked.truncate(pos + 1);
217+
masked.push_str("********");
218+
masked
219+
} else {
220+
"********".to_string()
221+
}
222+
}
223+
198224
pub fn mask(self) -> Value {
199225
match self.target {
200226
TargetType::Slack(slack_web_hook) => {
201-
let endpoint = slack_web_hook.endpoint.to_string();
202-
let masked_endpoint = if endpoint.len() > 20 {
203-
format!("{}********", &endpoint[..20])
204-
} else {
205-
"********".to_string()
206-
};
227+
let endpoint = Self::mask_url(&slack_web_hook.endpoint);
207228
json!({
208229
"name":self.name,
209230
"type":"slack",
210-
"endpoint":masked_endpoint,
231+
"endpoint": endpoint,
211232
"id":self.id
212233
})
213234
}
214235
TargetType::Other(other_web_hook) => {
215-
let endpoint = other_web_hook.endpoint.to_string();
216-
let masked_endpoint = if endpoint.len() > 20 {
217-
format!("{}********", &endpoint[..20])
218-
} else {
219-
"********".to_string()
220-
};
236+
let endpoint = Self::mask_url(&other_web_hook.endpoint);
237+
let safe_headers: HashMap<String, String> = other_web_hook
238+
.headers
239+
.into_iter()
240+
.map(|(k, v)| {
241+
if Self::should_redact_header(&k) {
242+
(k, "********".to_string())
243+
} else {
244+
(k, v)
245+
}
246+
})
247+
.collect();
221248
json!({
222249
"name":self.name,
223250
"type":"webhook",
224-
"endpoint":masked_endpoint,
225-
"headers":other_web_hook.headers,
251+
"endpoint":endpoint,
252+
"headers":safe_headers,
226253
"skipTlsCheck":other_web_hook.skip_tls_check,
227254
"id":self.id
228255
})
229256
}
230257
TargetType::AlertManager(alert_manager) => {
231-
let endpoint = alert_manager.endpoint.to_string();
232-
let masked_endpoint = if endpoint.len() > 20 {
233-
format!("{}********", &endpoint[..20])
234-
} else {
235-
"********".to_string()
236-
};
258+
let endpoint = Self::mask_url(&alert_manager.endpoint);
237259
if let Some(auth) = alert_manager.auth {
238260
let password = "********";
239261
json!({
240262
"name":self.name,
241263
"type":"webhook",
242-
"endpoint":masked_endpoint,
264+
"endpoint":endpoint,
243265
"username":auth.username,
244266
"password":password,
245267
"skipTlsCheck":alert_manager.skip_tls_check,
@@ -249,7 +271,7 @@ impl Target {
249271
json!({
250272
"name":self.name,
251273
"type":"webhook",
252-
"endpoint":masked_endpoint,
274+
"endpoint":endpoint,
253275
"username":Value::Null,
254276
"password":Value::Null,
255277
"skipTlsCheck":alert_manager.skip_tls_check,
@@ -652,3 +674,76 @@ pub struct Auth {
652674
username: String,
653675
password: String,
654676
}
677+
678+
#[cfg(test)]
679+
mod tests {
680+
use super::*;
681+
682+
#[test]
683+
fn test_masked_target_hides_secrets() {
684+
let target = Target {
685+
name: "test-target".into(),
686+
id: Ulid::new(),
687+
tenant: None,
688+
target: TargetType::AlertManager(AlertManager {
689+
endpoint: "https://internal.corp/api".parse().unwrap(),
690+
auth: Some(Auth {
691+
username: "admin".into(),
692+
password: "SuperSecretPassword123".into(),
693+
}),
694+
skip_tls_check: false,
695+
}),
696+
};
697+
698+
let masked = target.mask();
699+
700+
let masked_str = serde_json::to_string(&masked).unwrap();
701+
702+
// These assertions MUST pass, or the build fails
703+
assert!(
704+
!masked_str.contains("SuperSecretPassword123"),
705+
"Password leaked in masked response!"
706+
);
707+
assert!(
708+
masked_str.contains("********"),
709+
"Expected redaction marker not found!"
710+
);
711+
}
712+
713+
#[test]
714+
fn test_masked_webhook_hides_auth_headers() {
715+
let mut headers = HashMap::new();
716+
headers.insert("Authorization".into(), "Bearer super-secret-token".into());
717+
headers.insert("Content-Type".into(), "application/json".into());
718+
719+
let target = Target {
720+
name: "slack-hook".into(),
721+
id: Ulid::new(),
722+
tenant: None,
723+
target: TargetType::Other(OtherWebHook {
724+
endpoint: "https://hooks.slack.com/services/T00/B00/SECRET"
725+
.parse()
726+
.unwrap(),
727+
headers,
728+
skip_tls_check: false,
729+
}),
730+
};
731+
732+
let masked = target.mask();
733+
let masked_str = serde_json::to_string(&masked).unwrap();
734+
735+
assert!(
736+
!masked_str.contains("super-secret-token"),
737+
"Bearer token leaked in masked response!"
738+
);
739+
assert!(
740+
!masked_str.contains("SECRET"),
741+
"Webhook URL secret leaked in masked response!"
742+
);
743+
// Non-sensitive headers should survive
744+
assert!(
745+
masked_str.contains("application/json"),
746+
"Non-sensitive header was incorrectly redacted!"
747+
);
748+
}
749+
}

src/handlers/http/targets.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ pub async fn post(
4242
// add to the map
4343
TARGETS.update(target.clone()).await?;
4444

45-
// Ok(web::Json(target.mask()))
46-
Ok(web::Json(target))
45+
Ok(web::Json(target.mask()))
4746
}
4847

4948
// GET /targets
@@ -54,7 +53,7 @@ pub async fn list(req: HttpRequest) -> Result<impl Responder, AlertError> {
5453
.list(&tenant_id)
5554
.await?
5655
.into_iter()
57-
// .map(|t| t.mask())
56+
.map(|t| t.mask())
5857
.collect_vec();
5958

6059
Ok(web::Json(list))
@@ -66,8 +65,7 @@ pub async fn get(req: HttpRequest, target_id: Path<Ulid>) -> Result<impl Respond
6665
let tenant_id = get_tenant_id_from_request(&req);
6766
let target = TARGETS.get_target_by_id(&target_id, &tenant_id).await?;
6867

69-
// Ok(web::Json(target.mask()))
70-
Ok(web::Json(target))
68+
Ok(web::Json(target.mask()))
7169
}
7270

7371
// PUT /targets/{target_id}
@@ -95,8 +93,7 @@ pub async fn update(
9593
// add to the map
9694
TARGETS.update(target.clone()).await?;
9795

98-
// Ok(web::Json(target.mask()))
99-
Ok(web::Json(target))
96+
Ok(web::Json(target.mask()))
10097
}
10198

10299
// DELETE /targets/{target_id}
@@ -105,6 +102,5 @@ pub async fn delete(req: HttpRequest, target_id: Path<Ulid>) -> Result<impl Resp
105102
let tenant_id = get_tenant_id_from_request(&req);
106103
let target = TARGETS.delete(&target_id, &tenant_id).await?;
107104

108-
// Ok(web::Json(target.mask()))
109-
Ok(web::Json(target))
105+
Ok(web::Json(target.mask()))
110106
}

0 commit comments

Comments
 (0)