diff --git a/bugsnag.go b/bugsnag.go index 498c3b91..8909b960 100644 --- a/bugsnag.go +++ b/bugsnag.go @@ -237,7 +237,7 @@ func init() { AppVersion: "", AutoCaptureSessions: true, ReleaseStage: "", - ParamsFilters: []string{"password", "secret", "authorization", "cookie"}, + ParamsFilters: []string{"password", "secret", "authorization", "cookie", "access_token"}, SourceRoot: sourceRoot, ProjectPackages: []string{"main*"}, NotifyReleaseStages: nil, diff --git a/request_extractor.go b/request_extractor.go index 29d959e1..db723b69 100644 --- a/request_extractor.go +++ b/request_extractor.go @@ -3,6 +3,7 @@ package bugsnag import ( "context" "net/http" + "net/url" "strings" ) @@ -32,19 +33,54 @@ func extractRequestInfo(ctx context.Context) (*RequestJSON, *http.Request) { // understands from the given HTTP request. Returns the sub-object supported by // the notify API. func extractRequestInfoFromReq(req *http.Request) *RequestJSON { - proto := "http://" - if req.TLS != nil { - proto = "https://" - } return &RequestJSON{ ClientIP: req.RemoteAddr, HTTPMethod: req.Method, - URL: proto + req.Host + req.RequestURI, + URL: sanitizeURL(req), Referer: req.Referer(), Headers: parseRequestHeaders(req.Header), } } +// sanitizeURL will build up the URL matching the request. It will filter query parameters to remove sensitive fields. +// The query part of the URL might appear differently (different order of parameters) if any filtering was done. +func sanitizeURL(req *http.Request) string { + scheme := "http" + if req.TLS != nil { + scheme = "https" + } + + rawQuery := req.URL.RawQuery + parsedQuery, err := url.ParseQuery(req.URL.RawQuery) + + if err != nil { + return scheme + "://" + req.Host + req.RequestURI + } + + changed := false + for key, values := range parsedQuery { + if contains(Config.ParamsFilters, key) { + for i := range values { + values[i] = "BUGSNAG_URL_FILTERED" + changed = true + } + } + } + + if changed { + rawQuery = parsedQuery.Encode() + rawQuery = strings.Replace(rawQuery, "BUGSNAG_URL_FILTERED", "[FILTERED]", -1) + } + + u := url.URL{ + Scheme: scheme, + Host: req.Host, + Path: req.URL.Path, + RawQuery: rawQuery, + } + return u.String() +} + func parseRequestHeaders(header map[string][]string) map[string]string { headers := make(map[string]string) for k, v := range header { diff --git a/request_extractor_test.go b/request_extractor_test.go index 646e2690..4b4c537f 100644 --- a/request_extractor_test.go +++ b/request_extractor_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "net/http/httptest" + "net/url" "strings" "testing" ) @@ -54,6 +55,42 @@ func TestRequestExtractorCanHandleAbsentContext(t *testing.T) { } } +func TestExtractRequestInfoFromReq_RedactURL(t *testing.T) { + testCases := []struct { + in url.URL + exp string + }{ + {in: url.URL{}, exp: "http://example.com"}, + {in: url.URL{Path: "/"}, exp: "http://example.com/"}, + {in: url.URL{Path: "/foo.html"}, exp: "http://example.com/foo.html"}, + {in: url.URL{Path: "/foo.html", RawQuery: "q=something&bar=123"}, exp: "http://example.com/foo.html?q=something&bar=123"}, + {in: url.URL{Path: "/foo.html", RawQuery: "foo=1&foo=2&foo=3"}, exp: "http://example.com/foo.html?foo=1&foo=2&foo=3"}, + + // Invalid query string. + {in: url.URL{Path: "/foo", RawQuery: "%"}, exp: "http://example.com/foo?%"}, + + // Query params contain secrets + {in: url.URL{Path: "/foo.html", RawQuery: "access_token=something"}, exp: "http://example.com/foo.html?access_token=[FILTERED]"}, + {in: url.URL{Path: "/foo.html", RawQuery: "access_token=something&access_token=&foo=bar"}, exp: "http://example.com/foo.html?access_token=[FILTERED]&access_token=[FILTERED]&foo=bar"}, + } + + for _, tc := range testCases { + requestURI := tc.in.Path + if tc.in.RawQuery != "" { + requestURI += "?" + tc.in.RawQuery + } + req := &http.Request{ + Host: "example.com", + URL: &tc.in, + RequestURI: requestURI, + } + result := extractRequestInfoFromReq(req) + if result.URL != tc.exp { + t.Errorf("expected URL to be '%s' but was '%s'", tc.exp, result.URL) + } + } +} + func TestParseHeadersWillSanitiseIllegalParams(t *testing.T) { headers := make(map[string][]string) headers["password"] = []string{"correct horse battery staple"}