Skip to content

Errors caused by internal HTTP client's Do() don't have client_secret redacted. #522

@dmitshur

Description

@dmitshur

go-github tries to redact client_secret URL query parameter value from errors that it returns, to prevent leaking sensitive information in case they errors are exposed to users. See:

  • go-github/github/github.go

    Lines 552 to 564 in 1739081

    // sanitizeURL redacts the client_secret parameter from the URL which may be
    // exposed to the user, specifically in the ErrorResponse error message.
    func sanitizeURL(uri *url.URL) *url.URL {
    if uri == nil {
    return nil
    }
    params := uri.Query()
    if len(params.Get("client_secret")) > 0 {
    params.Set("client_secret", "REDACTED")
    uri.RawQuery = params.Encode()
    }
    return uri
    }
  • r.Response.Request.Method, sanitizeURL(r.Response.Request.URL),
  • errorResponse := &ErrorResponse{Response: r}
  • err = CheckResponse(resp)

However, there exists a possible path where an error is returned without sanitization. See:

return nil, err

The underlying HTTP client's Do() can return an error, for example, if GitHub servers are down or having connectivity issues. The returned error can be a *url.Error which contains the original query URL with the secret.

E.g.:

image

(Actual status.github.com graph spike from last night.)

Get https://api.github.com/...?client_id=xxx&client_secret=yyy: dial tcp 192.30.253.117:443 i/o timeout

Since we're already sanitizing those errors in most cases, for consistency, we should also do it here, to prevent client_secret being included in the returned error and potentially displayed to users.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions