diff --git a/cmd/kosli/attestationSubmit_test.go b/cmd/kosli/attestationSubmit_test.go index 2faaca538..325e72105 100644 --- a/cmd/kosli/attestationSubmit_test.go +++ b/cmd/kosli/attestationSubmit_test.go @@ -321,7 +321,7 @@ func TestAttestationSubmitterAttestsNothingWhenTheBucketRefusesTheFile(t *testin err := submitter.submit(submission) require.EqualError(t, err, "failed to upload attachment testdata/file1: "+ - "upload refused with HTTP 403: BadDigest: The SHA256 you specified did not match what we received") + "upload refused with HTTP 403 Forbidden: BadDigest: The SHA256 you specified did not match what we received") require.Empty(t, fake.recorded) } diff --git a/cmd/kosli/evaluateHelpers.go b/cmd/kosli/evaluateHelpers.go index 259723ec4..db8abab25 100644 --- a/cmd/kosli/evaluateHelpers.go +++ b/cmd/kosli/evaluateHelpers.go @@ -202,7 +202,7 @@ func fetchRemotePolicy(remoteURL string) ([]byte, error) { } defer func() { _ = resp.Body.Close() }() if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return nil, fmt.Errorf("failed to fetch policy from %s: HTTP %d", remoteURL, resp.StatusCode) + return nil, fmt.Errorf("failed to fetch policy from %s: %s", remoteURL, requests.HTTPStatus(resp.StatusCode)) } body, err := io.ReadAll(io.LimitReader(resp.Body, policyMaxBytes+1)) @@ -421,9 +421,9 @@ func serverSideRequestError(err error) error { // Quoting what arrived instead would dress a proxy's page, or the decoder's // complaint about one, as the server's own reason. if !apiError.HasServerMessage || apiError.Message == "" { - return fmt.Errorf("the Kosli server answered %d", apiError.StatusCode) + return fmt.Errorf("the Kosli server answered %s", requests.HTTPStatus(apiError.StatusCode)) } - return fmt.Errorf("the Kosli server answered %d: %s", apiError.StatusCode, apiError.Message) + return fmt.Errorf("the Kosli server answered %s: %s", requests.HTTPStatus(apiError.StatusCode), apiError.Message) } // serverSideReadError reports a failure to read a verdict back. By this point diff --git a/cmd/kosli/evaluateHelpers_test.go b/cmd/kosli/evaluateHelpers_test.go index 5e331421d..cd107ebde 100644 --- a/cmd/kosli/evaluateHelpers_test.go +++ b/cmd/kosli/evaluateHelpers_test.go @@ -3,9 +3,12 @@ package main import ( "bytes" "encoding/json" + "net/http" + "net/http/httptest" "testing" "github.com/kosli-dev/cli/internal/evaluate" + "github.com/kosli-dev/cli/internal/requests" "github.com/stretchr/testify/require" ) @@ -21,3 +24,24 @@ func TestEvaluateResultKeysListsEveryKeyTheOutputCanHave(t *testing.T) { require.Contains(t, evaluateResultKeys, key) } } + +func TestFetchRemotePolicyNamesTheStatusItWasRefusedWith(t *testing.T) { + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + + _, err := fetchRemotePolicy(server.URL + "/policy.rego") + + require.EqualError(t, err, "failed to fetch policy from "+server.URL+"/policy.rego: HTTP 404 Not Found") +} + +func TestServerSideRequestErrorNamesTheStatus(t *testing.T) { + require.EqualError(t, + serverSideRequestError(&requests.APIError{StatusCode: http.StatusForbidden, Message: "not entitled", HasServerMessage: true}), + "the Kosli server answered HTTP 403 Forbidden: not entitled") + require.EqualError(t, + serverSideRequestError(&requests.APIError{StatusCode: http.StatusNotFound}), + "the Kosli server answered HTTP 404 Not Found") + require.EqualError(t, + serverSideRequestError(&requests.APIError{StatusCode: http.StatusBadRequest, HasServerMessage: true}), + "the Kosli server answered HTTP 400 Bad Request") +} diff --git a/internal/bitbucket/bitbucket.go b/internal/bitbucket/bitbucket.go index d412476e5..84d1b2f1f 100644 --- a/internal/bitbucket/bitbucket.go +++ b/internal/bitbucket/bitbucket.go @@ -244,7 +244,7 @@ func (c *Config) getPullRequestDetailsFromBitbucket(prApiUrl, prHtmlLink, commit } } } else { - return evidence, fmt.Errorf("failed to get PR details, got HTTP status %d. Please review repository permissions", response.Resp.StatusCode) + return evidence, fmt.Errorf("failed to get PR details, got %s instead of HTTP 200 OK", requests.HTTPStatus(response.Resp.StatusCode)) } return evidence, nil } @@ -332,7 +332,7 @@ func (c *Config) getPullRequestCommitsFromBitbucket(prID int) ([]types.Commit, e return nil, err } if response.Resp.StatusCode != 200 { - return nil, fmt.Errorf("failed to get PR commits, got HTTP status %d", response.Resp.StatusCode) + return nil, fmt.Errorf("failed to get PR commits, got %s instead of HTTP 200 OK", requests.HTTPStatus(response.Resp.StatusCode)) } var responseData map[string]any diff --git a/internal/bitbucket/odd_replies_test.go b/internal/bitbucket/odd_replies_test.go index 83df38da1..e093de5fd 100644 --- a/internal/bitbucket/odd_replies_test.go +++ b/internal/bitbucket/odd_replies_test.go @@ -160,11 +160,21 @@ func (rt redirectTransport) RoundTrip(req *http.Request) (*http.Response, error) // runOddReplies serves replies and collects evidence for oddCommit through // the given entry point: 1 for PREvidenceForCommitV1, 2 for V2. func runOddReplies(t *testing.T, version int, replies map[string]string) ([]*types.PREvidence, error) { + t.Helper() + return runOddRepliesWithStatus(t, version, replies, nil) +} + +// runOddRepliesWithStatus is runOddReplies with the status of each named reply +// set by statuses; any other reply is a 200. +func runOddRepliesWithStatus(t *testing.T, version int, replies map[string]string, statuses map[string]int) ([]*types.PREvidence, error) { t.Helper() var base string server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { for name, route := range oddRoutes { if r.URL.RequestURI() == route { + if status, ok := statuses[name]; ok { + w.WriteHeader(status) + } _, _ = w.Write([]byte(strings.ReplaceAll(replies[name], "{{base}}", base))) return } @@ -423,3 +433,17 @@ func TestOddReplies_CommitWithNoHashFails(t *testing.T) { }) } } + +// A 201 or 202 passes the client as a success but is not the reply the code +// reads, so it fails, naming the status. +func TestOddReplies_ASuccessOtherThanOKIsNamed(t *testing.T) { + for name, want := range map[string]string{ + "pullrequest": "failed to get PR details, got HTTP 202 Accepted instead of HTTP 200 OK", + "pullrequest_commits_1": "failed to get PR commits, got HTTP 202 Accepted instead of HTTP 200 OK", + } { + t.Run(name, func(t *testing.T) { + _, err := runOddRepliesWithStatus(t, 2, oddBaselineReplies(), map[string]int{name: http.StatusAccepted}) + require.EqualError(t, err, want) + }) + } +} diff --git a/internal/requests/requests.go b/internal/requests/requests.go index f094b3336..3589a55ec 100644 --- a/internal/requests/requests.go +++ b/internal/requests/requests.go @@ -498,9 +498,9 @@ func s3ErrorMessage(status int, body []byte) string { Message string `xml:"Message"` } if err := xml.Unmarshal(body, &s3Error); err != nil || s3Error.Code == "" { - return fmt.Sprintf("upload refused with HTTP %d", status) + return "upload refused with " + HTTPStatus(status) } - return fmt.Sprintf("upload refused with HTTP %d: %s: %s", status, s3Error.Code, s3Error.Message) + return fmt.Sprintf("upload refused with %s: %s: %s", HTTPStatus(status), s3Error.Code, s3Error.Message) } func (c *Client) PayloadOutput(req *http.Request, jsonFields map[string]any, message string) error { @@ -553,11 +553,11 @@ func isSuccess(status int) bool { func nonJSONErrorMessage(status int, body []byte, debug bool) string { switch { case len(bytes.TrimSpace(body)) == 0: - return httpStatus(status) + return HTTPStatus(status) case debug: - return httpStatus(status) + ": the response is not JSON" + return HTTPStatus(status) + ": the response is not JSON" default: - return httpStatus(status) + ": the response is not JSON, rerun with --debug to see it" + return HTTPStatus(status) + ": the response is not JSON, rerun with --debug to see it" } } @@ -569,7 +569,7 @@ func giveUpErrorHandler(resp *http.Response, err error, numTries int) (*http.Res if resp != nil { _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 4096)) _ = resp.Body.Close() - msg += ": " + httpStatus(resp.StatusCode) + msg += ": " + HTTPStatus(resp.StatusCode) } if err != nil { return nil, fmt.Errorf("%s: %w", msg, err) @@ -577,9 +577,9 @@ func giveUpErrorHandler(resp *http.Response, err error, numTries int) (*http.Res return nil, errors.New(msg) } -// httpStatus renders a code as "HTTP 409 Conflict", or "HTTP 599" for a code +// HTTPStatus renders a code as "HTTP 409 Conflict", or "HTTP 599" for a code // with no standard text. -func httpStatus(code int) string { +func HTTPStatus(code int) string { return strings.TrimSpace(fmt.Sprintf("HTTP %d %s", code, http.StatusText(code))) } diff --git a/internal/requests/requests_test.go b/internal/requests/requests_test.go index dd2bae34b..39f1501e9 100644 --- a/internal/requests/requests_test.go +++ b/internal/requests/requests_test.go @@ -693,8 +693,8 @@ func TestRequestsTestSuite(t *testing.T) { } func TestHTTPStatus(t *testing.T) { - require.Equal(t, "HTTP 429 Too Many Requests", httpStatus(http.StatusTooManyRequests)) - require.Equal(t, "HTTP 599", httpStatus(599)) + require.Equal(t, "HTTP 429 Too Many Requests", HTTPStatus(http.StatusTooManyRequests)) + require.Equal(t, "HTTP 599", HTTPStatus(599)) } func TestJitteredBackoff(t *testing.T) {