Skip to content

Commit f307c49

Browse files
authored
Restore and harden credential masking for /targets (#1698)
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 f307c49

2 files changed

Lines changed: 52 additions & 32 deletions

File tree

src/alerts/target.rs

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -198,48 +198,36 @@ impl Target {
198198
pub fn mask(self) -> Value {
199199
match self.target {
200200
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-
};
207201
json!({
208202
"name":self.name,
209203
"type":"slack",
210-
"endpoint":masked_endpoint,
204+
"endpoint":format!("{}://********", slack_web_hook.endpoint.scheme()),
211205
"id":self.id
212206
})
213207
}
214208
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-
};
209+
let safe_headers: HashMap<String, String> = other_web_hook
210+
.headers
211+
.into_keys()
212+
.map(|k| (k, "********".to_string()))
213+
.collect();
221214
json!({
222215
"name":self.name,
223216
"type":"webhook",
224-
"endpoint":masked_endpoint,
225-
"headers":other_web_hook.headers,
217+
"endpoint": format!("{}://********", other_web_hook.endpoint.scheme()),
218+
"headers":safe_headers,
226219
"skipTlsCheck":other_web_hook.skip_tls_check,
227220
"id":self.id
228221
})
229222
}
230223
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-
};
224+
let endpoint = format!("{}://********", alert_manager.endpoint.scheme());
237225
if let Some(auth) = alert_manager.auth {
238226
let password = "********";
239227
json!({
240228
"name":self.name,
241229
"type":"webhook",
242-
"endpoint":masked_endpoint,
230+
"endpoint":endpoint,
243231
"username":auth.username,
244232
"password":password,
245233
"skipTlsCheck":alert_manager.skip_tls_check,
@@ -249,7 +237,7 @@ impl Target {
249237
json!({
250238
"name":self.name,
251239
"type":"webhook",
252-
"endpoint":masked_endpoint,
240+
"endpoint":endpoint,
253241
"username":Value::Null,
254242
"password":Value::Null,
255243
"skipTlsCheck":alert_manager.skip_tls_check,
@@ -652,3 +640,39 @@ pub struct Auth {
652640
username: String,
653641
password: String,
654642
}
643+
644+
#[cfg(test)]
645+
mod tests {
646+
use super::*;
647+
648+
#[test]
649+
fn test_masked_target_hides_secrets() {
650+
let target = Target {
651+
name: "test-target".into(),
652+
id: Ulid::new(),
653+
tenant: None,
654+
target: TargetType::AlertManager(AlertManager {
655+
endpoint: "https://internal.corp/api".parse().unwrap(),
656+
auth: Some(Auth {
657+
username: "admin".into(),
658+
password: "SuperSecretPassword123".into(),
659+
}),
660+
skip_tls_check: false,
661+
}),
662+
};
663+
664+
let masked = target.mask();
665+
666+
let masked_str = serde_json::to_string(&masked).unwrap();
667+
668+
// These assertions MUST pass, or the build fails
669+
assert!(
670+
!masked_str.contains("SuperSecretPassword123"),
671+
"Password leaked in masked response!"
672+
);
673+
assert!(
674+
masked_str.contains("********"),
675+
"Expected redaction marker not found!"
676+
);
677+
}
678+
}

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)