From 9bfe2a1a08d541b8f697a816cb3148b0867fda73 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 6 Jun 2026 17:50:23 +0200 Subject: [PATCH] fix(ci): gate http-client tests on mock readiness; address semgrep findings (refs #451) The integration suite flaked in CI: with three `go run` mocks now compiling concurrently, the spotify/amazon mocks weren't listening within the fixed `sleep 10`, so the registration requests at the start of the suite hit a connection-refused and the "Account exists" assertions (and the cascading amazon oauth token test) failed. Locally it passed because the mock builds were warm. Replace the fixed sleep with real readiness gating: - Add a /healthz endpoint to the spotify, amazon and tunein mocks. - Give all four CI services (the three mocks + soundtouch-service) a compose healthcheck (busybox wget; all images are alpine-based), and make the service depend_on the mocks being service_healthy. - `docker compose up -d --build --wait` blocks until everything is healthy, so the JetBrains client only runs against a fully-ready stack. Also clear the two semgrep advisories on the new TuneIn mock: - cmd/mock-*: annotate the intentional plaintext ListenAndServe with nosemgrep (throwaway loopback/CI test servers, never production). - pkg/testutils/tunein: sanitize the query-supplied guide id to a safe charset before interpolating it into the JSON/XML response (raw-html-format). make test-http-client: 73 requests, 0 failed (clean testdata, healthcheck-gated). Co-Authored-By: Claude Opus 4.8 (1M context) --- Makefile | 6 ++---- cmd/mock-amazon/main.go | 3 +++ cmd/mock-spotify/main.go | 3 +++ cmd/mock-tunein/main.go | 3 +++ docker-compose.ci.yml | 33 +++++++++++++++++++++++++++++++ pkg/testutils/amazon/handlers.go | 3 +++ pkg/testutils/spotify/handlers.go | 3 +++ pkg/testutils/tunein/handlers.go | 16 +++++++++++++++ 8 files changed, 66 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 29dc188..e06ccce 100644 --- a/Makefile +++ b/Makefile @@ -164,10 +164,8 @@ test-http-client-rotate: fi test-http-client: - @echo "Starting services with docker compose..." - @docker compose -f docker-compose.yml -f docker-compose.ci.yml up -d --build - @echo "Waiting for services to start..." - @sleep 10 + @echo "Starting services with docker compose (waiting for healthchecks)..." + @docker compose -f docker-compose.yml -f docker-compose.ci.yml up -d --build --wait @echo "Running .http tests..." @docker run --rm --network soundtouch-test-net \ -v "$(PWD)/tests/integration/http-client:/workdir" \ diff --git a/cmd/mock-amazon/main.go b/cmd/mock-amazon/main.go index 662bb1b..6d50cda 100644 --- a/cmd/mock-amazon/main.go +++ b/cmd/mock-amazon/main.go @@ -17,6 +17,9 @@ func main() { log.Printf("Starting mock Amazon LWA server on port %d", *port) + // Plaintext HTTP is intentional: this is a throwaway test mock that only + // runs on the loopback / CI compose network, never in production. + // nosemgrep: go.lang.security.audit.net.use-tls.use-tls if err := http.ListenAndServe(fmt.Sprintf(":%d", *port), amazon.NewAmazonHandler()); err != nil { log.Fatal(err) } diff --git a/cmd/mock-spotify/main.go b/cmd/mock-spotify/main.go index 81e7118..c40cc29 100644 --- a/cmd/mock-spotify/main.go +++ b/cmd/mock-spotify/main.go @@ -17,6 +17,9 @@ func main() { log.Printf("Starting mock Spotify server on port %d", *port) + // Plaintext HTTP is intentional: this is a throwaway test mock that only + // runs on the loopback / CI compose network, never in production. + // nosemgrep: go.lang.security.audit.net.use-tls.use-tls if err := http.ListenAndServe(fmt.Sprintf(":%d", *port), spotify.NewSpotifyHandler()); err != nil { log.Fatal(err) } diff --git a/cmd/mock-tunein/main.go b/cmd/mock-tunein/main.go index 8e22ed2..58003f8 100644 --- a/cmd/mock-tunein/main.go +++ b/cmd/mock-tunein/main.go @@ -17,6 +17,9 @@ func main() { log.Printf("Starting mock TuneIn server on port %d", *port) + // Plaintext HTTP is intentional: this is a throwaway test mock that only + // runs on the loopback / CI compose network, never in production. + // nosemgrep: go.lang.security.audit.net.use-tls.use-tls if err := http.ListenAndServe(fmt.Sprintf(":%d", *port), tunein.NewTuneInHandler()); err != nil { log.Fatal(err) } diff --git a/docker-compose.ci.yml b/docker-compose.ci.yml index b79d7e4..559f758 100644 --- a/docker-compose.ci.yml +++ b/docker-compose.ci.yml @@ -18,6 +18,21 @@ services: - AMAZON_PROFILE_URL=http://amazon-mock:8080/user/profile - TUNEIN_OPML_URL=http://tunein-mock:8080 - TUNEIN_API_URL=http://tunein-mock:8080 + # Start only once every mock is actually listening (the mocks are `go run`, + # so cold compilation can take a while); see depends_on below. + depends_on: + spotify-mock: + condition: service_healthy + amazon-mock: + condition: service_healthy + tunein-mock: + condition: service_healthy + healthcheck: + test: ["CMD", "wget", "-q", "-O", "/dev/null", "http://localhost:8000/health"] + interval: 3s + timeout: 3s + retries: 30 + start_period: 3s spotify-mock: image: golang:1.26.4-alpine @@ -30,6 +45,12 @@ services: - "8081:8080" networks: - soundtouch-test-net + healthcheck: + test: ["CMD", "wget", "-q", "-O", "/dev/null", "http://localhost:8080/healthz"] + interval: 3s + timeout: 3s + retries: 30 + start_period: 3s amazon-mock: image: golang:1.26.4-alpine @@ -42,6 +63,12 @@ services: - "8082:8080" networks: - soundtouch-test-net + healthcheck: + test: ["CMD", "wget", "-q", "-O", "/dev/null", "http://localhost:8080/healthz"] + interval: 3s + timeout: 3s + retries: 30 + start_period: 3s tunein-mock: image: golang:1.26.4-alpine @@ -54,6 +81,12 @@ services: - "8083:8080" networks: - soundtouch-test-net + healthcheck: + test: ["CMD", "wget", "-q", "-O", "/dev/null", "http://localhost:8080/healthz"] + interval: 3s + timeout: 3s + retries: 30 + start_period: 3s networks: soundtouch-test-net: diff --git a/pkg/testutils/amazon/handlers.go b/pkg/testutils/amazon/handlers.go index e64440a..084a4d9 100644 --- a/pkg/testutils/amazon/handlers.go +++ b/pkg/testutils/amazon/handlers.go @@ -17,6 +17,9 @@ func NewAmazonHandler() http.Handler { // LWA User Profile Endpoint mux.HandleFunc("/user/profile", HandleProfile) + // Readiness probe (used by the CI compose healthcheck) + mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) + return mux } diff --git a/pkg/testutils/spotify/handlers.go b/pkg/testutils/spotify/handlers.go index 86cd0e3..279495e 100644 --- a/pkg/testutils/spotify/handlers.go +++ b/pkg/testutils/spotify/handlers.go @@ -18,6 +18,9 @@ func NewSpotifyHandler() http.Handler { mux.HandleFunc("/v1/me", HandleMe) mux.HandleFunc("/me", HandleMe) + // Readiness probe (used by the CI compose healthcheck) + mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) + return mux } diff --git a/pkg/testutils/tunein/handlers.go b/pkg/testutils/tunein/handlers.go index 3645454..b3dc5e1 100644 --- a/pkg/testutils/tunein/handlers.go +++ b/pkg/testutils/tunein/handlers.go @@ -16,13 +16,25 @@ import ( "fmt" "log" "net/http" + "regexp" ) +// unsafeGuideIDChars matches anything outside the TuneIn guide-id charset +// (e.g. s166521, p290778, t472593281). Stripping them before the id is +// interpolated into the JSON/XML response keeps a caller from injecting markup +// or breaking the document (the input is attacker-controlled query data). +var unsafeGuideIDChars = regexp.MustCompile(`[^A-Za-z0-9._-]`) + +func safeGuideID(id string) string { + return unsafeGuideIDChars.ReplaceAllString(id, "") +} + // NewTuneInHandler returns an http.Handler configured with the mocked TuneIn // OPML endpoints. func NewTuneInHandler() http.Handler { mux := http.NewServeMux() + mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) mux.HandleFunc("/Tune.ashx", HandleTune) mux.HandleFunc("/describe.ashx", HandleDescribe) mux.HandleFunc("/", HandleCatchAll) @@ -43,6 +55,8 @@ func HandleTune(w http.ResponseWriter, r *http.Request) { return } + id = safeGuideID(id) + body := fmt.Sprintf(`{"head":{"status":"200"},"body":[`+ `{"url":"http://192.0.2.20:8000/%s/stream-1.mp3","media_type":"mp3","reliability":99,"bitrate":128,"is_direct":true},`+ `{"url":"http://192.0.2.20:8000/%s/stream-2.mp3","media_type":"mp3","reliability":95,"bitrate":128,"is_direct":true}`+ @@ -64,6 +78,8 @@ func HandleDescribe(w http.ResponseWriter, r *http.Request) { return } + id = safeGuideID(id) + body := fmt.Sprintf(``+ ``+ `%s200`+