From d91ec6d6dd75b282fb4676686eb192125bb51c09 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Fri, 6 Nov 2026 21:11:54 +0000 Subject: [PATCH 1/5] refactor: export HTTPStatus for use outside the requests package Other packages report HTTP errors by bare code and are about to render them the same way as the API client does, so the helper needs to be reachable from them. --- internal/requests/requests.go | 12 ++++++------ internal/requests/requests_test.go | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/requests/requests.go b/internal/requests/requests.go index f094b3336..8829dd7d1 100644 --- a/internal/requests/requests.go +++ b/internal/requests/requests.go @@ -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) { From 49e9fbe6c4aded9e993c20f49099b7adfce3cf64 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Fri, 6 Nov 2026 21:12:37 +0000 Subject: [PATCH 2/5] fix: name the status in upload errors An upload refused by the storage bucket read "upload refused with HTTP 403". It now reads "HTTP 403 Forbidden", like the API client's errors. --- cmd/kosli/attestationSubmit_test.go | 2 +- internal/requests/requests.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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/internal/requests/requests.go b/internal/requests/requests.go index 8829dd7d1..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 { From 08345f1efb848019c2b24fad7f46c5f1d2305111 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Fri, 6 Nov 2026 21:13:40 +0000 Subject: [PATCH 3/5] fix: name the status in kosli evaluate errors A refused policy fetch and a server-side evaluation the API turned down were reported by bare code, e.g. "the Kosli server answered 403". They now read "HTTP 403 Forbidden", like the API client's errors. --- cmd/kosli/evaluateHelpers.go | 6 +++--- cmd/kosli/evaluateHelpers_test.go | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) 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..44c904b53 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,21 @@ 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") +} From ba6119922878bbcbb6ecdb08aedaccf5ee631e87 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Fri, 6 Nov 2026 21:14:06 +0000 Subject: [PATCH 4/5] fix: name the status in Bitbucket pull request errors A pull request or commit list that came back with a success other than 200 was reported as "got HTTP status 202". It now reads "got HTTP 202 Accepted", like the API client's errors. The fake Bitbucket server in the tests can now answer a reply with a chosen status, to reach these paths. --- internal/bitbucket/bitbucket.go | 4 ++-- internal/bitbucket/odd_replies_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/internal/bitbucket/bitbucket.go b/internal/bitbucket/bitbucket.go index d412476e5..f2c2ea9e8 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. Please review repository permissions", 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", 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..7191a0af2 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", + "pullrequest_commits_1": "failed to get PR commits, got HTTP 202 Accepted", + } { + t.Run(name, func(t *testing.T) { + _, err := runOddRepliesWithStatus(t, 2, oddBaselineReplies(), map[string]int{name: http.StatusAccepted}) + require.ErrorContains(t, err, want) + }) + } +} From 0506d11e1ccc615a5e5fcd06f9e724e8359995dd Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Fri, 6 Nov 2026 21:19:40 +0000 Subject: [PATCH 5/5] fix: say what Bitbucket should have answered instead of blaming permissions The pull request details error reached only on a 201 or 202 told the user to review repository permissions, which a success status does not suggest. Both Bitbucket errors on that path now say the reply was not the 200 OK the code reads. The tests match the whole message. Also pins the evaluate error for a server message that is present but empty, which reports the status alone. --- cmd/kosli/evaluateHelpers_test.go | 3 +++ internal/bitbucket/bitbucket.go | 4 ++-- internal/bitbucket/odd_replies_test.go | 6 +++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cmd/kosli/evaluateHelpers_test.go b/cmd/kosli/evaluateHelpers_test.go index 44c904b53..cd107ebde 100644 --- a/cmd/kosli/evaluateHelpers_test.go +++ b/cmd/kosli/evaluateHelpers_test.go @@ -41,4 +41,7 @@ func TestServerSideRequestErrorNamesTheStatus(t *testing.T) { 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 f2c2ea9e8..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 %s. Please review repository permissions", requests.HTTPStatus(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 %s", requests.HTTPStatus(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 7191a0af2..e093de5fd 100644 --- a/internal/bitbucket/odd_replies_test.go +++ b/internal/bitbucket/odd_replies_test.go @@ -438,12 +438,12 @@ func TestOddReplies_CommitWithNoHashFails(t *testing.T) { // 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", - "pullrequest_commits_1": "failed to get PR commits, got HTTP 202 Accepted", + "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.ErrorContains(t, err, want) + require.EqualError(t, err, want) }) } }