mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
fix(notifications): redact mixed-case webhook credential keys
Diagnostic query masking recognised only lowercase credential names, leaving mixed-case and encoded variants visible in URL and transport-error diagnostics. Match decoded names without case while preserving query spelling, unrelated parameters and the original request URL. Add seven regression cases covering diagnostics and wrapped error identity. Change-source: pulse-maintainer
This commit is contained in:
@@ -822,12 +822,19 @@ outside this bounded change.
|
||||
|
||||
### Bounded diagnostic confidentiality: query representations and bypass callers
|
||||
|
||||
Recognised query names are exactly token, apikey, api_key, key, secret and
|
||||
password after one URL query decode. Every repeated occurrence is masked,
|
||||
including mixed literal/escaped names. Unrelated names, ordering and values
|
||||
Recognised query names are token, apikey, api_key, key, secret and
|
||||
password, matched case-insensitively after one URL query decode. Every repeated
|
||||
occurrence is masked, including mixed literal/escaped names and mixed-case
|
||||
spellings. Case folding is limited to diagnostic matching: original query-name
|
||||
spelling is retained, and configured outbound URLs are not normalised. Unrelated names, ordering and values
|
||||
remain intact; invalid name escapes fail closed. This is diagnostic projection,
|
||||
not mutation of configured destinations or a claim to recognise arbitrary secrets.
|
||||
|
||||
`TestRedactWebhookMixedCaseQuerySecrets` covers all six recognised names plus
|
||||
an escaped mixed-case name through URL, embedded-message and transport-error
|
||||
projection. It preserves unrelated lookalikes, the original request URL and
|
||||
wrapped error identity; these synthetic cases do not establish customer exposure.
|
||||
|
||||
Resolved ntfy must apply the same transport-error projection before both its
|
||||
error log and returned error. Common HTTP execution preserves payload bytes,
|
||||
event identity and error causes; URLs containing userinfo remain rejected by
|
||||
|
||||
@@ -63,6 +63,8 @@ func RedactWebhookURLSecrets(urlString string) string {
|
||||
// Decode names exactly once, as net/url does, but retain the original
|
||||
// spelling, order and unrelated values in diagnostic URLs. Inspect every
|
||||
// occurrence rather than Query().Get(), which would miss repeated keys.
|
||||
// Mask recognised credential names regardless of case for diagnostics only;
|
||||
// the destination retains the original, potentially case-sensitive query.
|
||||
parts := strings.Split(parsed.RawQuery, "&")
|
||||
changed := false
|
||||
for i, part := range parts {
|
||||
@@ -71,7 +73,7 @@ func RedactWebhookURLSecrets(urlString string) string {
|
||||
if err != nil {
|
||||
return invalidWebhookURLDiagnostic
|
||||
}
|
||||
switch decoded {
|
||||
switch strings.ToLower(decoded) {
|
||||
case "token", "apikey", "api_key", "key", "secret", "password":
|
||||
if hasValue {
|
||||
parts[i] = name + "=REDACTED"
|
||||
|
||||
@@ -81,6 +81,32 @@ func TestRedactWebhookURLSecrets(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Diagnostic masking is deliberately conservative even though a destination
|
||||
// may treat query names as case-sensitive. It must not change the request URL.
|
||||
func TestRedactWebhookMixedCaseQuerySecrets(t *testing.T) {
|
||||
for _, key := range []string{"TOKEN", "ApiKey", "Api_Key", "KEY", "Secret", "PassWord", "%54oKeN"} {
|
||||
t.Run(key, func(t *testing.T) {
|
||||
raw := "https://example.test/hook?" + key + "=synthetic-secret&token=second-secret&extra_TOKEN=visible&channel=ops"
|
||||
want := "https://example.test/hook?" + key + "=REDACTED&token=REDACTED&extra_TOKEN=visible&channel=ops"
|
||||
if got := RedactWebhookURLSecrets(raw); got != want {
|
||||
t.Errorf("URL = %q, want %q", got, want)
|
||||
}
|
||||
if got := RedactWebhookDiagnosticSecrets("post " + raw + " failed"); got != "post "+want+" failed" {
|
||||
t.Errorf("diagnostic = %q", got)
|
||||
}
|
||||
cause := errors.New("connection refused")
|
||||
original := &url.Error{Op: "Post", URL: raw, Err: cause}
|
||||
redacted := redactWebhookTransportError(original)
|
||||
if strings.Contains(redacted.Error(), "synthetic-secret") || strings.Contains(redacted.Error(), "second-secret") {
|
||||
t.Errorf("transport diagnostic exposed synthetic credential: %v", redacted)
|
||||
}
|
||||
if original.URL != raw || !errors.Is(redacted, cause) {
|
||||
t.Fatal("redaction changed request URL or error identity")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRedactWebhookDiagnosticSecrets(t *testing.T) {
|
||||
tests := map[string]struct {
|
||||
input string
|
||||
|
||||
Reference in New Issue
Block a user