Skip to content

Commit cc1dcdd

Browse files
committed
This patch re-enables masking with a few backend changes
- Add mask_url() using url based parsing - Apply consistent masking across Slack, Webhook, and AlertManager types. - Add strict unit tests enforcing that secrets never hit the serializer.
1 parent 595af02 commit cc1dcdd

2 files changed

Lines changed: 62 additions & 32 deletions

File tree

src/alerts/target.rs

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

197197
impl Target {
198+
/// Redacts userinfo, path, query, and fragment from a URL,
199+
/// preserving only scheme, host, and port.
200+
fn mask_url(url: &Url) -> String {
201+
let host = url.host_str().unwrap_or("");
202+
let port = url.port().map(|p| format!(":{}", p)).unwrap_or_default();
203+
format!("{}://{}{}/********", url.scheme(), host, port)
204+
}
205+
198206
pub fn mask(self) -> Value {
199207
match self.target {
200208
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-
};
209+
let endpoint = Self::mask_url(&slack_web_hook.endpoint);
207210
json!({
208211
"name":self.name,
209212
"type":"slack",
210-
"endpoint":masked_endpoint,
213+
"endpoint": endpoint,
211214
"id":self.id
212215
})
213216
}
214217
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-
};
218+
let endpoint = Self::mask_url(&other_web_hook.endpoint);
219+
let safe_headers: HashMap<String, String> = other_web_hook
220+
.headers
221+
.into_keys()
222+
.map(|k| (k, "********".to_string()))
223+
.collect();
221224
json!({
222225
"name":self.name,
223226
"type":"webhook",
224-
"endpoint":masked_endpoint,
225-
"headers":other_web_hook.headers,
227+
"endpoint":endpoint,
228+
"headers":safe_headers,
226229
"skipTlsCheck":other_web_hook.skip_tls_check,
227230
"id":self.id
228231
})
229232
}
230233
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-
};
234+
let endpoint = Self::mask_url(&alert_manager.endpoint);
237235
if let Some(auth) = alert_manager.auth {
238236
let password = "********";
239237
json!({
240238
"name":self.name,
241239
"type":"webhook",
242-
"endpoint":masked_endpoint,
240+
"endpoint":endpoint,
243241
"username":auth.username,
244242
"password":password,
245243
"skipTlsCheck":alert_manager.skip_tls_check,
@@ -249,7 +247,7 @@ impl Target {
249247
json!({
250248
"name":self.name,
251249
"type":"webhook",
252-
"endpoint":masked_endpoint,
250+
"endpoint":endpoint,
253251
"username":Value::Null,
254252
"password":Value::Null,
255253
"skipTlsCheck":alert_manager.skip_tls_check,
@@ -652,3 +650,39 @@ pub struct Auth {
652650
username: String,
653651
password: String,
654652
}
653+
654+
#[cfg(test)]
655+
mod tests {
656+
use super::*;
657+
658+
#[test]
659+
fn test_masked_target_hides_secrets() {
660+
let target = Target {
661+
name: "test-target".into(),
662+
id: Ulid::new(),
663+
tenant: None,
664+
target: TargetType::AlertManager(AlertManager {
665+
endpoint: "https://internal.corp/api".parse().unwrap(),
666+
auth: Some(Auth {
667+
username: "admin".into(),
668+
password: "SuperSecretPassword123".into(),
669+
}),
670+
skip_tls_check: false,
671+
}),
672+
};
673+
674+
let masked = target.mask();
675+
676+
let masked_str = serde_json::to_string(&masked).unwrap();
677+
678+
// These assertions MUST pass, or the build fails
679+
assert!(
680+
!masked_str.contains("SuperSecretPassword123"),
681+
"Password leaked in masked response!"
682+
);
683+
assert!(
684+
masked_str.contains("********"),
685+
"Expected redaction marker not found!"
686+
);
687+
}
688+
}

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)