mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
fix(notifications): retain context around redacted delivery URLs
PR #1959 exposed that the URL-only redactor was called with a complete delivery error, causing safe but unhelpful replacement of the whole diagnostic. Redact embedded webhook URLs separately so credentials stay masked and non-secret failure context remains available; malformed URLs continue to fail closed. Change-source: pulse-maintainer
This commit is contained in:
@@ -700,6 +700,10 @@ actual rate-limit log output. These queue-free tests establish local diagnostic
|
|||||||
redaction, not destination receipt, installed recovery or release qualification.
|
redaction, not destination receipt, installed recovery or release qualification.
|
||||||
No claim is made that arbitrary custom path/query secrets are recognised.
|
No claim is made that arbitrary custom path/query secrets are recognised.
|
||||||
|
|
||||||
|
Delivery-log errors use `RedactWebhookDiagnosticSecrets` so URLs embedded in
|
||||||
|
otherwise useful error text receive the same masking without discarding the
|
||||||
|
surrounding status context. Malformed embedded URLs still fail closed.
|
||||||
|
|
||||||
### Slack webhook diagnostic path confidentiality
|
### Slack webhook diagnostic path confidentiality
|
||||||
|
|
||||||
The same helper masks paths on the exact `hooks.slack.com` and
|
The same helper masks paths on the exact `hooks.slack.com` and
|
||||||
|
|||||||
@@ -1076,7 +1076,7 @@ func (h *NotificationHandlers) GetDeliveryLog(w http.ResponseWriter, r *http.Req
|
|||||||
// can embed credentials.
|
// can embed credentials.
|
||||||
for i := range entries {
|
for i := range entries {
|
||||||
if entries[i].ErrorMessage != "" {
|
if entries[i].ErrorMessage != "" {
|
||||||
entries[i].ErrorMessage = notifications.RedactWebhookURLSecrets(entries[i].ErrorMessage)
|
entries[i].ErrorMessage = notifications.RedactWebhookDiagnosticSecrets(entries[i].ErrorMessage)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -6,6 +6,8 @@ import (
|
|||||||
"strings"
|
"strings"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
const invalidWebhookURLDiagnostic = "[invalid webhook URL]"
|
||||||
|
|
||||||
// RedactWebhookURLSecrets masks credentials commonly embedded in webhook URLs
|
// RedactWebhookURLSecrets masks credentials commonly embedded in webhook URLs
|
||||||
// while preserving the URL shape needed for operator diagnostics.
|
// while preserving the URL shape needed for operator diagnostics.
|
||||||
func RedactWebhookURLSecrets(urlString string) string {
|
func RedactWebhookURLSecrets(urlString string) string {
|
||||||
@@ -14,7 +16,7 @@ func RedactWebhookURLSecrets(urlString string) string {
|
|||||||
// URLs rather than returning unparsed credentials to diagnostic callers.
|
// URLs rather than returning unparsed credentials to diagnostic callers.
|
||||||
parsed, err := url.Parse(urlString)
|
parsed, err := url.Parse(urlString)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "[invalid webhook URL]"
|
return invalidWebhookURLDiagnostic
|
||||||
}
|
}
|
||||||
if parsed.User != nil {
|
if parsed.User != nil {
|
||||||
parsed.User = url.User("REDACTED")
|
parsed.User = url.User("REDACTED")
|
||||||
@@ -91,6 +93,49 @@ func RedactWebhookURLSecrets(urlString string) string {
|
|||||||
return urlString
|
return urlString
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// RedactWebhookDiagnosticSecrets masks webhook URLs embedded in diagnostic
|
||||||
|
// text while retaining the non-secret context around them. A malformed URL
|
||||||
|
// still fails closed rather than returning potentially sensitive text.
|
||||||
|
func RedactWebhookDiagnosticSecrets(message string) string {
|
||||||
|
lowerMessage := strings.ToLower(message)
|
||||||
|
cursor := 0
|
||||||
|
foundURL := false
|
||||||
|
var redacted strings.Builder
|
||||||
|
|
||||||
|
for cursor < len(message) {
|
||||||
|
httpOffset := strings.Index(lowerMessage[cursor:], "http://")
|
||||||
|
httpsOffset := strings.Index(lowerMessage[cursor:], "https://")
|
||||||
|
offset := httpOffset
|
||||||
|
if offset == -1 || (httpsOffset != -1 && httpsOffset < offset) {
|
||||||
|
offset = httpsOffset
|
||||||
|
}
|
||||||
|
if offset == -1 {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
|
||||||
|
start := cursor + offset
|
||||||
|
end := len(message)
|
||||||
|
if whitespace := strings.IndexAny(message[start:], " \t\r\n"); whitespace != -1 {
|
||||||
|
end = start + whitespace
|
||||||
|
}
|
||||||
|
|
||||||
|
redactedURL := RedactWebhookURLSecrets(message[start:end])
|
||||||
|
if redactedURL == invalidWebhookURLDiagnostic {
|
||||||
|
return invalidWebhookURLDiagnostic
|
||||||
|
}
|
||||||
|
redacted.WriteString(message[cursor:start])
|
||||||
|
redacted.WriteString(redactedURL)
|
||||||
|
cursor = end
|
||||||
|
foundURL = true
|
||||||
|
}
|
||||||
|
|
||||||
|
if !foundURL {
|
||||||
|
return RedactWebhookURLSecrets(message)
|
||||||
|
}
|
||||||
|
redacted.WriteString(message[cursor:])
|
||||||
|
return redacted.String()
|
||||||
|
}
|
||||||
|
|
||||||
func redactWebhookTransportError(err error) error {
|
func redactWebhookTransportError(err error) error {
|
||||||
if err == nil {
|
if err == nil {
|
||||||
return nil
|
return nil
|
||||||
|
|||||||
@@ -72,6 +72,38 @@ func TestRedactWebhookURLSecrets(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRedactWebhookDiagnosticSecrets(t *testing.T) {
|
||||||
|
tests := map[string]struct {
|
||||||
|
input string
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
"embedded query credential": {
|
||||||
|
input: "post https://hooks.example.test/notify?token=supersecret returned 401",
|
||||||
|
want: "post https://hooks.example.test/notify?token=REDACTED returned 401",
|
||||||
|
},
|
||||||
|
"embedded userinfo": {
|
||||||
|
input: "request to https://hook-user:hook-password@example.test/hook failed",
|
||||||
|
want: "request to https://REDACTED@example.test/hook failed",
|
||||||
|
},
|
||||||
|
"plain diagnostic": {
|
||||||
|
input: "connection refused",
|
||||||
|
want: "connection refused",
|
||||||
|
},
|
||||||
|
"malformed embedded URL": {
|
||||||
|
input: "post https://hook-user:hook-password@example.test/%zz failed",
|
||||||
|
want: invalidWebhookURLDiagnostic,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for name, test := range tests {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
if got := RedactWebhookDiagnosticSecrets(test.input); got != test.want {
|
||||||
|
t.Fatalf("RedactWebhookDiagnosticSecrets() = %q, want %q", got, test.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestRedactWebhookTransportErrorPreservesBehaviorWithoutToken(t *testing.T) {
|
func TestRedactWebhookTransportErrorPreservesBehaviorWithoutToken(t *testing.T) {
|
||||||
cause := errors.New("connection refused")
|
cause := errors.New("connection refused")
|
||||||
original := &url.Error{
|
original := &url.Error{
|
||||||
|
|||||||
Reference in New Issue
Block a user