Mask a target URL's query string when the URL has no path (closes #500)
check / check (push) Successful in 5m28s
check / check (push) Successful in 5m28s
The Redactor treated a target URL's request URI as a secret only when the URL had a path other than "/", so a response echoing the request line for https://example.com/?token=... or https://example.com?token=... showed the token on the event log and the event's page. urlSecrets now treats the query string, and the request URI that carries it, as secrets whenever the URL has one, whatever its path. With the event's query string passed on, only the target's own part is masked. Model: opus-5-5
This commit is contained in:
@@ -162,18 +162,22 @@ func targetSecrets(t *database.Target) []string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// urlSecrets returns the substrings of a destination URL that
|
// urlSecrets returns the substrings of a destination URL that
|
||||||
// must not survive into a rendered page: the whole URL, the
|
// must not survive into a rendered page: the whole URL; its
|
||||||
// parts of it MaskURL elides, and any userinfo.
|
// path, unless that is empty or "/"; its query string, and the
|
||||||
|
// request URI that carries it, which a remote echoing the
|
||||||
|
// request line shows even when the URL has no path; and its
|
||||||
|
// userinfo and password.
|
||||||
//
|
//
|
||||||
// No length floor is applied to the path, and none to the
|
// No length floor is applied to the path, the query string or
|
||||||
// userinfo. A short path or a four-byte username is treated as
|
// the userinfo. A short path or a four-byte username is
|
||||||
// a credential exactly like a long one, because the field takes
|
// treated as a credential exactly like a long one, because the
|
||||||
// an arbitrary URL and no part of it can be assumed non-secret —
|
// field takes an arbitrary URL and no part of it can be
|
||||||
// the same rule MaskURL applies. headerSecrets does carry a
|
// assumed non-secret — the same rule MaskURL applies.
|
||||||
// floor, and the difference is deliberate: a header is picked
|
// headerSecrets does carry a floor, and the difference is
|
||||||
// out by a name-shaped guess and its value may be ordinary
|
// deliberate: a header is picked out by a name-shaped guess
|
||||||
// text, whereas a URL's path and userinfo are credential
|
// and its value may be ordinary text, whereas a URL's path,
|
||||||
// material by position.
|
// query string and userinfo are credential material by
|
||||||
|
// position.
|
||||||
func urlSecrets(raw string) []string {
|
func urlSecrets(raw string) []string {
|
||||||
raw = strings.TrimSpace(raw)
|
raw = strings.TrimSpace(raw)
|
||||||
if raw == "" {
|
if raw == "" {
|
||||||
@@ -188,12 +192,11 @@ func urlSecrets(raw string) []string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if parsed.Path != "" && parsed.Path != "/" {
|
if parsed.Path != "" && parsed.Path != "/" {
|
||||||
requestURI := parsed.RequestURI()
|
secrets = append(secrets, parsed.EscapedPath())
|
||||||
secrets = append(secrets, requestURI)
|
}
|
||||||
|
|
||||||
if escaped := parsed.EscapedPath(); escaped != requestURI {
|
if parsed.RawQuery != "" {
|
||||||
secrets = append(secrets, escaped)
|
secrets = append(secrets, parsed.RequestURI(), parsed.RawQuery)
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if parsed.User != nil {
|
if parsed.User != nil {
|
||||||
|
|||||||
@@ -202,6 +202,48 @@ func TestRedactor_RemovesHTTPURLQueryAndUserinfo(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestRedactor_RemovesEchoedQueryOfURLWithoutPath covers an
|
||||||
|
// HTTP target URL whose credential is all in its query string.
|
||||||
|
// Written with or without the "/", the request line sends it
|
||||||
|
// as "/?token=…", and a target passing the event's query string
|
||||||
|
// on sends that after an "&". The event's part stays visible:
|
||||||
|
// the event's page shows it anyway.
|
||||||
|
func TestRedactor_RemovesEchoedQueryOfURLWithoutPath(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const secret = "s3cr3t"
|
||||||
|
|
||||||
|
marker := delivery.RedactionMarker
|
||||||
|
|
||||||
|
// An echoed request line, and what the event log shows of it.
|
||||||
|
echoes := map[string]string{
|
||||||
|
"POST /?token=" + secret + " HTTP/1.1": "POST " + marker +
|
||||||
|
" HTTP/1.1",
|
||||||
|
"POST ?token=" + secret + " HTTP/1.1": "POST ?" + marker +
|
||||||
|
" HTTP/1.1",
|
||||||
|
"POST /?token=" + secret + "&a=1&b=2 HTTP/1.1": "POST " +
|
||||||
|
marker + "&a=1&b=2 HTTP/1.1",
|
||||||
|
"POST ?token=" + secret + "&a=1&b=2 HTTP/1.1": "POST ?" +
|
||||||
|
marker + "&a=1&b=2 HTTP/1.1",
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, dest := range []string{
|
||||||
|
"https://example.com/?token=" + secret,
|
||||||
|
"https://example.com?token=" + secret,
|
||||||
|
} {
|
||||||
|
r := delivery.NewRedactor(&database.Target{
|
||||||
|
Type: database.TargetTypeHTTP,
|
||||||
|
Config: `{"url":"` + dest + `"}`,
|
||||||
|
})
|
||||||
|
|
||||||
|
for echoed, want := range echoes {
|
||||||
|
assert.Equal(
|
||||||
|
t, want, r.Redact(echoed), "%s: %s", dest, echoed,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestRedactor_LeavesUnrelatedTextAlone pins that the
|
// TestRedactor_LeavesUnrelatedTextAlone pins that the
|
||||||
// redactor matches literally: it does not guess at what a
|
// redactor matches literally: it does not guess at what a
|
||||||
// secret looks like, so ordinary response content survives.
|
// secret looks like, so ordinary response content survives.
|
||||||
|
|||||||
Reference in New Issue
Block a user