From e013ec0f88ba74f1b639e0c525216b6068297550 Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 28 Jul 2026 23:08:20 +0100 Subject: [PATCH] chore: all HTTP requests now come with a context, timeout, etc. --- service/go.mod | 2 +- service/internal/auth/local_bearer_test.go | 10 +++++----- service/internal/auth/otjwt/jwt_test.go | 10 +++++++++- .../internal/auth/otoauth2/restapi_auth_oauth2.go | 13 ++++++++++++- .../auth/otoauth2/restapi_auth_oauth2_test.go | 2 +- service/internal/updatecheck/updateCheck.go | 8 +++++++- .../internal/webhooks/matcher_justification_test.go | 4 ++-- service/scripts/find-flakey-tests-inf/main.go | 7 ++++++- 8 files changed, 43 insertions(+), 13 deletions(-) diff --git a/service/go.mod b/service/go.mod index 5d15066..f0a9915 100644 --- a/service/go.mod +++ b/service/go.mod @@ -28,7 +28,6 @@ require ( github.com/sirupsen/logrus v1.9.4 github.com/stretchr/testify v1.11.1 go.akshayshah.org/connectproto v0.6.0 - golang.org/x/exp v0.0.0-20260718201538-764159d718ef golang.org/x/oauth2 v0.36.0 golang.org/x/sys v0.47.0 google.golang.org/protobuf v1.36.11 @@ -290,6 +289,7 @@ require ( go.uber.org/zap v1.28.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/crypto v0.54.0 // indirect + golang.org/x/exp v0.0.0-20260718201538-764159d718ef // indirect golang.org/x/exp/typeparams v0.0.0-20260718201538-764159d718ef // indirect golang.org/x/mod v0.38.0 // indirect golang.org/x/net v0.57.0 // indirect diff --git a/service/internal/auth/local_bearer_test.go b/service/internal/auth/local_bearer_test.go index 65ce8c5..0426c04 100644 --- a/service/internal/auth/local_bearer_test.go +++ b/service/internal/auth/local_bearer_test.go @@ -21,7 +21,7 @@ func TestCheckUserFromLocalBearerApiKey_Match_LowercaseBearerScheme(t *testing.T ApiKey: "secret-api-key", }} - req := httptest.NewRequest("POST", "/", nil) + req := httptest.NewRequestWithContext(t.Context(), "POST", "/", nil) req.Header.Set("Authorization", "bearer secret-api-key") ctx := &authpublic.AuthCheckingContext{Request: req, Config: cfg} @@ -43,7 +43,7 @@ func TestCheckUserFromLocalBearerApiKey_Match(t *testing.T) { ApiKey: "secret-api-key", }} - req := httptest.NewRequest("POST", "/", nil) + req := httptest.NewRequestWithContext(t.Context(), "POST", "/", nil) req.Header.Set("Authorization", "Bearer secret-api-key") ctx := &authpublic.AuthCheckingContext{Request: req, Config: cfg} @@ -64,7 +64,7 @@ func TestCheckUserFromLocalBearerApiKey_WrongKey(t *testing.T) { ApiKey: "secret-api-key", }} - req := httptest.NewRequest("POST", "/", nil) + req := httptest.NewRequestWithContext(t.Context(), "POST", "/", nil) req.Header.Set("Authorization", "Bearer wrong") ctx := &authpublic.AuthCheckingContext{Request: req, Config: cfg} @@ -81,7 +81,7 @@ func TestCheckUserFromLocalBearerApiKey_DisabledLocalUsers(t *testing.T) { ApiKey: "secret-api-key", }} - req := httptest.NewRequest("POST", "/", nil) + req := httptest.NewRequestWithContext(t.Context(), "POST", "/", nil) req.Header.Set("Authorization", "Bearer secret-api-key") ctx := &authpublic.AuthCheckingContext{Request: req, Config: cfg} @@ -98,7 +98,7 @@ func TestCheckUserFromLocalBearerApiKey_NoBearerPrefix(t *testing.T) { ApiKey: "secret-api-key", }} - req := httptest.NewRequest("POST", "/", nil) + req := httptest.NewRequestWithContext(t.Context(), "POST", "/", nil) req.Header.Set("Authorization", "secret-api-key") ctx := &authpublic.AuthCheckingContext{Request: req, Config: cfg} diff --git a/service/internal/auth/otjwt/jwt_test.go b/service/internal/auth/otjwt/jwt_test.go index 6dc6cb1..219fcb5 100644 --- a/service/internal/auth/otjwt/jwt_test.go +++ b/service/internal/auth/otjwt/jwt_test.go @@ -132,7 +132,14 @@ func testJwkValidationWithAudience(t *testing.T, expire int64, expectCode int, c defer srv.Close() res := makeJWTRequest(t, srv, tokenStr) + verifyJWTResponse(t, res, expectCode) + + err := res.Body.Close() + + if err != nil { + t.Error("Could not close response body", err) + } } func TestJWTSignatureVerificationSucceeds(t *testing.T) { @@ -167,7 +174,7 @@ func createJWTTokenWithGroups(t *testing.T, privateKey *rsa.PrivateKey, groups i } func makeJWTRequest(t *testing.T, srv *httptest.Server, tokenStr string) *http.Response { - req, err := http.NewRequest("GET", srv.URL, nil) + req, err := http.NewRequestWithContext(t.Context(), "GET", srv.URL, nil) if err != nil { t.Fatalf("failed to create request: %v", err) } @@ -177,6 +184,7 @@ func makeJWTRequest(t *testing.T, srv *httptest.Server, tokenStr string) *http.R if err != nil { t.Fatalf("Client err: %+v", err) } + return res } diff --git a/service/internal/auth/otoauth2/restapi_auth_oauth2.go b/service/internal/auth/otoauth2/restapi_auth_oauth2.go index ddae1ec..5c38a39 100644 --- a/service/internal/auth/otoauth2/restapi_auth_oauth2.go +++ b/service/internal/auth/otoauth2/restapi_auth_oauth2.go @@ -342,7 +342,18 @@ type UserInfo struct { func getUserInfo(cfg *config.Config, client *http.Client, provider *config.OAuth2Provider) *UserInfo { ret := &UserInfo{} - res, err := client.Get(provider.WhoamiUrl) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + + defer cancel() + + req, err := http.NewRequestWithContext(ctx, "GET", provider.WhoamiUrl, nil) + + if err != nil { + log.Error("Could not construct user data request", err) + return ret + } + + res, err := http.DefaultClient.Do(req) if err != nil { log.Errorf("Failed to get user data: %v", err) diff --git a/service/internal/auth/otoauth2/restapi_auth_oauth2_test.go b/service/internal/auth/otoauth2/restapi_auth_oauth2_test.go index 2b6ef4f..f11768d 100644 --- a/service/internal/auth/otoauth2/restapi_auth_oauth2_test.go +++ b/service/internal/auth/otoauth2/restapi_auth_oauth2_test.go @@ -56,7 +56,7 @@ func TestHandleOAuthLoginRejectsWhenStateMapFull(t *testing.T) { } } - req := httptest.NewRequest(http.MethodGet, "/oauth/login?provider=test", nil) + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/oauth/login?provider=test", nil) rec := httptest.NewRecorder() h.HandleOAuthLogin(rec, req) diff --git a/service/internal/updatecheck/updateCheck.go b/service/internal/updatecheck/updateCheck.go index 54a697f..a0c6353 100644 --- a/service/internal/updatecheck/updateCheck.go +++ b/service/internal/updatecheck/updateCheck.go @@ -1,6 +1,7 @@ package updatecheck import ( + "context" "encoding/json" "github.com/Masterminds/semver" config "github.com/OliveTin/OliveTin/internal/config" @@ -10,6 +11,7 @@ import ( "io" "net/http" "os" + "time" ) type versionMapType struct { @@ -84,7 +86,11 @@ func parseIfVersionIsLater(currentString string, latestString string) string { } func doRequest() string { - req, err := http.NewRequest("GET", "http://update-check.olivetin.app/versions.json", nil) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + + defer cancel() + + req, err := http.NewRequestWithContext(ctx, "GET", "http://update-check.olivetin.app/versions.json", nil) if err != nil { log.Errorf("Update check failed %v", err) diff --git a/service/internal/webhooks/matcher_justification_test.go b/service/internal/webhooks/matcher_justification_test.go index 970f0a8..dd63395 100644 --- a/service/internal/webhooks/matcher_justification_test.go +++ b/service/internal/webhooks/matcher_justification_test.go @@ -12,7 +12,7 @@ import ( func TestExtractJustificationFromWebhookBody(t *testing.T) { body := []byte(`{"message":"deploy production","repo":"my-app"}`) - req, err := http.NewRequest(http.MethodPost, "/webhooks/deploy", nil) + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, "/webhooks/deploy", nil) require.NoError(t, err) matcher := NewWebhookMatcher(config.WebhookConfig{ @@ -25,7 +25,7 @@ func TestExtractJustificationFromWebhookBody(t *testing.T) { } func TestExtractJustificationEmptyWhenNotConfigured(t *testing.T) { - req, err := http.NewRequest(http.MethodPost, "/webhooks/deploy", nil) + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, "/webhooks/deploy", nil) require.NoError(t, err) matcher := NewWebhookMatcher(config.WebhookConfig{}, req, []byte(`{}`)) diff --git a/service/scripts/find-flakey-tests-inf/main.go b/service/scripts/find-flakey-tests-inf/main.go index 34af824..73a863f 100644 --- a/service/scripts/find-flakey-tests-inf/main.go +++ b/service/scripts/find-flakey-tests-inf/main.go @@ -2,6 +2,7 @@ package main import ( "bufio" + "context" "encoding/json" "errors" "fmt" @@ -323,7 +324,11 @@ func finishTestCommand(cmd *exec.Cmd, state *testRunState) (int, runSummary, []t } func runTestsOnce(rootDir string) (int, runSummary, []testFailure, error) { - cmd := exec.Command("go", "test", "./...", "-count=1", "-json") + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + + defer cancel() + + cmd := exec.CommandContext(ctx, "go", "test", "./...", "-count=1", "-json") cmd.Dir = rootDir stdout, err := cmd.StdoutPipe()