17284: Redact RailsAPI host:port in error messages.
authorTom Clegg <tom@curii.com>
Mon, 26 Apr 2021 13:40:46 +0000 (09:40 -0400)
committerTom Clegg <tom@curii.com>
Mon, 26 Apr 2021 13:40:46 +0000 (09:40 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

lib/controller/handler_test.go
lib/controller/localdb/conn.go
lib/controller/rpc/conn.go

index 935208fc4e621c7c8040e09a1381e39c949e0232..2911a4f031cdac7aef14a80ecff42f349ddd0011 100644 (file)
@@ -344,3 +344,19 @@ func (s *HandlerSuite) TestGetObjects(c *check.C) {
                s.CheckObjectType(c, "/arvados/v1/"+url, arvadostest.AdminToken, skippedFields)
        }
 }
+
+func (s *HandlerSuite) TestRedactRailsAPIHostFromErrors(c *check.C) {
+       req := httptest.NewRequest("GET", "https://0.0.0.0:1/arvados/v1/collections/zzzzz-4zz18-abcdefghijklmno", nil)
+       req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+       resp := httptest.NewRecorder()
+       s.handler.ServeHTTP(resp, req)
+       c.Check(resp.Code, check.Equals, http.StatusNotFound)
+       var jresp struct {
+               Errors []string
+       }
+       c.Log(resp.Body.String())
+       c.Assert(json.NewDecoder(resp.Body).Decode(&jresp), check.IsNil)
+       c.Assert(jresp.Errors, check.HasLen, 1)
+       c.Check(jresp.Errors[0], check.Matches, `.*//railsapi\.internal/arvados/v1/collections/.*: 404 Not Found.*`)
+       c.Check(jresp.Errors[0], check.Not(check.Matches), `(?ms).*127.0.0.1.*`)
+}
index 04f85cb5a9f54c2cd2286e33645fc4a62cb400ca..a90deded593ab59c31599e6bcde3ca833a961349 100644 (file)
@@ -24,6 +24,7 @@ type Conn struct {
 
 func NewConn(cluster *arvados.Cluster) *Conn {
        railsProxy := railsproxy.NewConn(cluster)
+       railsProxy.RedactHostInErrors = true
        var conn Conn
        conn = Conn{
                cluster:    cluster,
index 61d20de78a824e869677e15f8c9937b69e9e4121..19e2d32d2cb01979317fff7dd74a16d3842bfe0e 100644 (file)
@@ -39,7 +39,9 @@ func PassthroughTokenProvider(ctx context.Context) ([]string, error) {
 }
 
 type Conn struct {
-       SendHeader    http.Header
+       SendHeader         http.Header
+       RedactHostInErrors bool
+
        clusterID     string
        httpClient    http.Client
        baseURL       url.URL
@@ -148,7 +150,21 @@ func (conn *Conn) requestAndDecode(ctx context.Context, dst interface{}, ep arva
                path = strings.Replace(path, "/{uuid}", "/"+uuid, 1)
                delete(params, "uuid")
        }
-       return aClient.RequestAndDecodeContext(ctx, dst, ep.Method, path, body, params)
+       err = aClient.RequestAndDecodeContext(ctx, dst, ep.Method, path, body, params)
+       if err != nil && conn.RedactHostInErrors {
+               redacted := strings.Replace(err.Error(), conn.baseURL.String(), "//railsapi.internal", -1)
+               if strings.HasPrefix(redacted, "request failed: ") {
+                       redacted = strings.Replace(redacted, "request failed: ", "", -1)
+               }
+               if redacted != err.Error() {
+                       if err, ok := err.(httpStatusError); ok {
+                               return wrapHTTPStatusError(err, redacted)
+                       } else {
+                               return errors.New(redacted)
+                       }
+               }
+       }
+       return err
 }
 
 func (conn *Conn) BaseURL() url.URL {
@@ -629,3 +645,26 @@ func (conn *Conn) UserAuthenticate(ctx context.Context, options arvados.UserAuth
        err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
        return resp, err
 }
+
+// httpStatusError is an error with an HTTP status code that can be
+// propagated by lib/controller/router, etc.
+type httpStatusError interface {
+       error
+       HTTPStatus() int
+}
+
+// wrappedHTTPStatusError is used to augment/replace an error message
+// while preserving the HTTP status code indicated by the original
+// error.
+type wrappedHTTPStatusError struct {
+       httpStatusError
+       message string
+}
+
+func wrapHTTPStatusError(err httpStatusError, message string) httpStatusError {
+       return wrappedHTTPStatusError{err, message}
+}
+
+func (err wrappedHTTPStatusError) Error() string {
+       return err.message
+}