From bb7bab8be7254f6a690599c5076d812150d50014 Mon Sep 17 00:00:00 2001 From: Aaro Koinsaari <89689072+koinsaari@users.noreply.github.com> Date: Sat, 1 Aug 2026 19:49:24 +0300 Subject: [PATCH 1/3] feat: rate-limit public read endpoints GET /v1/places and GET /v1/places/{id} had no rate limiting at the app layer, only authenticated writes did. Adds a per-IP token-bucket limiter (20 req/s, burst 20) ahead of the OpenAPI validator, skipped for requests carrying X-API-Key since those already have their own per-key limiter. --- cmd/api/main.go | 17 ++-- internal/middleware/public_rate_limit.go | 25 ++++++ internal/middleware/public_rate_limit_test.go | 84 +++++++++++++++++++ 3 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 internal/middleware/public_rate_limit.go create mode 100644 internal/middleware/public_rate_limit_test.go diff --git a/cmd/api/main.go b/cmd/api/main.go index cbe2a98..46f2479 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -54,8 +54,9 @@ type Server struct { db *gorm.DB places *place.Repository engine *a11y.Engine - regLimiter *middleware.RateLimiter - keyLimiter *middleware.RateLimiter + regLimiter *middleware.RateLimiter + keyLimiter *middleware.RateLimiter + readLimiter *middleware.RateLimiter } func main() { @@ -95,11 +96,12 @@ func main() { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) srv := &Server{ - db: gormDB, - places: place.NewRepository(gormDB), - engine: &a11y.Engine{}, - regLimiter: middleware.NewRateLimiter(ctx, rate.Every(20*time.Minute), 3), - keyLimiter: middleware.NewRateLimiter(ctx, rate.Every(time.Second), 60), + db: gormDB, + places: place.NewRepository(gormDB), + engine: &a11y.Engine{}, + regLimiter: middleware.NewRateLimiter(ctx, rate.Every(20*time.Minute), 3), + keyLimiter: middleware.NewRateLimiter(ctx, rate.Every(time.Second), 60), + readLimiter: middleware.NewRateLimiter(ctx, rate.Limit(20), 20), } // v1Mux holds only /v1/* routes so the spec validator only wraps those. @@ -138,6 +140,7 @@ func main() { srv.validationErrorHandler(w, r, err) }, })(v1Mux) + v1Handler = middleware.PublicRateLimit(srv.readLimiter)(v1Handler) v1Handler = middleware.CORS(getEnv("CORS_ALLOWED_ORIGIN", ""))(v1Handler) mux := http.NewServeMux() diff --git a/internal/middleware/public_rate_limit.go b/internal/middleware/public_rate_limit.go new file mode 100644 index 0000000..0365d64 --- /dev/null +++ b/internal/middleware/public_rate_limit.go @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2026 InWheel Contributors + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package middleware + +import ( + "encoding/json" + "net/http" +) + +func PublicRateLimit(rl *RateLimiter) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("X-API-Key") == "" && !rl.Allow(ClientIP(r)) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + json.NewEncoder(w).Encode(map[string]string{"error": "rate limit exceeded"}) + return + } + next.ServeHTTP(w, r) + }) + } +} diff --git a/internal/middleware/public_rate_limit_test.go b/internal/middleware/public_rate_limit_test.go new file mode 100644 index 0000000..5780963 --- /dev/null +++ b/internal/middleware/public_rate_limit_test.go @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2026 InWheel Contributors + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "golang.org/x/time/rate" +) + +func TestPublicRateLimit_AllowsUnderLimit(t *testing.T) { + t.Parallel() + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { called = true }) + + rl := &RateLimiter{r: 1, b: 1} + handler := PublicRateLimit(rl)(next) + r := httptest.NewRequest(http.MethodGet, "/v1/places", nil) + r.RemoteAddr = "1.2.3.4:9999" + w := httptest.NewRecorder() + handler.ServeHTTP(w, r) + + if !called { + t.Fatal("next was not called") + } +} + +func TestPublicRateLimit_BlocksOverLimit(t *testing.T) { + t.Parallel() + calls := 0 + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { calls++ }) + + rl := &RateLimiter{r: rate.Every(time.Hour), b: 1} + handler := PublicRateLimit(rl)(next) + + r1 := httptest.NewRequest(http.MethodGet, "/v1/places", nil) + r1.RemoteAddr = "1.2.3.4:9999" + handler.ServeHTTP(httptest.NewRecorder(), r1) // consumes the one token + + r2 := httptest.NewRequest(http.MethodGet, "/v1/places", nil) + r2.RemoteAddr = "1.2.3.4:9999" + w := httptest.NewRecorder() + handler.ServeHTTP(w, r2) + + if calls != 1 { + t.Fatalf("next was called %d times, want 1 (only the first request)", calls) + } + if w.Code != http.StatusTooManyRequests { + t.Errorf("status = %d, want %d", w.Code, http.StatusTooManyRequests) + } +} + +func TestPublicRateLimit_SkipsRequestsWithAPIKey(t *testing.T) { + t.Parallel() + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { called = true }) + + rl := &RateLimiter{r: rate.Every(time.Hour), b: 1} + handler := PublicRateLimit(rl)(next) + + r1 := httptest.NewRequest(http.MethodPost, "/v1/places", nil) + r1.RemoteAddr = "1.2.3.4:9999" + r1.Header.Set("X-API-Key", "iwk_test") + handler.ServeHTTP(httptest.NewRecorder(), r1) + + r2 := httptest.NewRequest(http.MethodPost, "/v1/places", nil) + r2.RemoteAddr = "1.2.3.4:9999" + r2.Header.Set("X-API-Key", "iwk_test") + w := httptest.NewRecorder() + handler.ServeHTTP(w, r2) + + if !called { + t.Fatal("next was not called for a request carrying X-API-Key") + } + if w.Code != http.StatusOK { + t.Errorf("status = %d, want %d (no status set by middleware)", w.Code, http.StatusOK) + } +} From 2b6d1475e0938bb78f97458968014c67a0e3e734 Mon Sep 17 00:00:00 2001 From: Aaro Koinsaari <89689072+koinsaari@users.noreply.github.com> Date: Sat, 1 Aug 2026 19:50:29 +0300 Subject: [PATCH 2/3] feat: deploy to Fly.io on merge to main Adds fly.toml (cmd/api, Frankfurt region, scale-to-zero) and a CI job that runs flyctl deploy only after all required checks pass on push to main, so a failing build/test/lint/scan never reaches production. Also ignores the local Supabase CLI's session cache directory. --- .github/workflows/ci.yml | 14 ++++++++++++++ .gitignore | 1 + fly.toml | 24 ++++++++++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 fly.toml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed7d31e..c233e2c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -119,3 +119,17 @@ jobs: with: repo-checkout: false + deploy: + name: Deploy + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + needs: [lint, generated, unit, integration, docker, vulncheck] + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: superfly/flyctl-actions/setup-flyctl@ed8efb33836e8b2096c7fd3ba1c8afe303ebbff1 # 1.6 + - name: Deploy to Fly.io + run: flyctl deploy --remote-only -a inwheel-api + env: + FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} + diff --git a/.gitignore b/.gitignore index 8adea7c..2f6937a 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ docs/superpowers/ .claude/ !.claude/settings.json graphify-out/ +supabase/.temp/ diff --git a/fly.toml b/fly.toml new file mode 100644 index 0000000..922cc0d --- /dev/null +++ b/fly.toml @@ -0,0 +1,24 @@ +# fly.toml app configuration file generated for inwheel-api on 2026-08-01T19:12:01+03:00 +# +# See https://fly.io/docs/reference/configuration/ for information about how to use this file. +# + +app = 'inwheel-api' +primary_region = 'fra' + +[build] + dockerfile = 'cmd/api/Dockerfile' + +[http_service] + internal_port = 8080 + force_https = true + auto_stop_machines = 'stop' + auto_start_machines = true + min_machines_running = 0 + processes = ['app'] + +[[vm]] + memory = '1gb' + cpu_kind = 'shared' + cpus = 1 + memory_mb = 1024 From 73101c6704ef7a5712457e0087e2c7527bc7251b Mon Sep 17 00:00:00 2001 From: Aaro Koinsaari <89689072+koinsaari@users.noreply.github.com> Date: Sat, 1 Aug 2026 20:40:12 +0300 Subject: [PATCH 3/3] fix: public read limiter bypassable via unverified X-API-Key header Ran before the OpenAPI validator/authenticate and only checked header presence, not validity. Now runs after the validator, keyed off APIKeyIDFromCtx, which is only set once a key is verified. Extracts buildV1Handler so a test covers the real composed chain instead of a hand-copied approximation. Also drops the redundant memory_mb in fly.toml. --- cmd/api/main.go | 75 ++++++++++--------- cmd/api/rate_limit_wiring_test.go | 45 +++++++++++ fly.toml | 1 - internal/middleware/public_rate_limit.go | 9 ++- internal/middleware/public_rate_limit_test.go | 52 +++++++++---- 5 files changed, 129 insertions(+), 53 deletions(-) create mode 100644 cmd/api/rate_limit_wiring_test.go diff --git a/cmd/api/main.go b/cmd/api/main.go index 46f2479..0893293 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -104,45 +104,12 @@ func main() { readLimiter: middleware.NewRateLimiter(ctx, rate.Limit(20), 20), } - // v1Mux holds only /v1/* routes so the spec validator only wraps those. - // Unversioned routes (/healthz, /readyz, /openapi.yaml) are registered on - // the outer mux and never go through the validator. - v1Mux := http.NewServeMux() - - swagger, err := apiv1.GetSpec() + v1Handler, err := buildV1Handler(srv, getEnv("CORS_ALLOWED_ORIGIN", "")) if err != nil { - slog.Error("Failed to load OpenAPI spec", "error", err) + slog.Error("Failed to build v1 handler", "error", err) os.Exit(1) } - strictHandler := apiv1.NewStrictHandlerWithOptions(srv, []apiv1.StrictMiddlewareFunc{injectRequest()}, apiv1.StrictHTTPServerOptions{ - RequestErrorHandlerFunc: srv.validationErrorHandler, - ResponseErrorHandlerFunc: func(w http.ResponseWriter, _ *http.Request, err error) { - slog.Error("handler error", "error", err) - writeJSON(w, map[string]string{"error": "internal server error"}, http.StatusInternalServerError) - }, - }) - apiv1.HandlerWithOptions(strictHandler, apiv1.StdHTTPServerOptions{ - BaseURL: "/v1", - BaseRouter: v1Mux, - ErrorHandlerFunc: srv.validationErrorHandler, - Middlewares: []apiv1.MiddlewareFunc{ - bodySizeLimiter(1 << 20), - }, - }) - - v1Handler := nethttp_middleware.OapiRequestValidatorWithOptions(swagger, &nethttp_middleware.Options{ - SilenceServersWarning: true, - Options: openapi3filter.Options{ - AuthenticationFunc: srv.authenticate, - }, - ErrorHandlerWithOpts: func(_ context.Context, err error, w http.ResponseWriter, r *http.Request, _ nethttp_middleware.ErrorHandlerOpts) { - srv.validationErrorHandler(w, r, err) - }, - })(v1Mux) - v1Handler = middleware.PublicRateLimit(srv.readLimiter)(v1Handler) - v1Handler = middleware.CORS(getEnv("CORS_ALLOWED_ORIGIN", ""))(v1Handler) - mux := http.NewServeMux() mux.HandleFunc("GET /healthz", srv.handleHealthz) mux.HandleFunc("GET /readyz", srv.handleReadyz) @@ -180,6 +147,44 @@ func main() { cancel() } +func buildV1Handler(srv *Server, corsOrigin string) (http.Handler, error) { + v1Mux := http.NewServeMux() + + swagger, err := apiv1.GetSpec() + if err != nil { + return nil, err + } + + strictHandler := apiv1.NewStrictHandlerWithOptions(srv, []apiv1.StrictMiddlewareFunc{injectRequest()}, apiv1.StrictHTTPServerOptions{ + RequestErrorHandlerFunc: srv.validationErrorHandler, + ResponseErrorHandlerFunc: func(w http.ResponseWriter, _ *http.Request, err error) { + slog.Error("handler error", "error", err) + writeJSON(w, map[string]string{"error": "internal server error"}, http.StatusInternalServerError) + }, + }) + apiv1.HandlerWithOptions(strictHandler, apiv1.StdHTTPServerOptions{ + BaseURL: "/v1", + BaseRouter: v1Mux, + ErrorHandlerFunc: srv.validationErrorHandler, + Middlewares: []apiv1.MiddlewareFunc{ + bodySizeLimiter(1 << 20), + }, + }) + + v1Handler := nethttp_middleware.OapiRequestValidatorWithOptions(swagger, &nethttp_middleware.Options{ + SilenceServersWarning: true, + Options: openapi3filter.Options{ + AuthenticationFunc: srv.authenticate, + }, + ErrorHandlerWithOpts: func(_ context.Context, err error, w http.ResponseWriter, r *http.Request, _ nethttp_middleware.ErrorHandlerOpts) { + srv.validationErrorHandler(w, r, err) + }, + })(middleware.PublicRateLimit(srv.readLimiter)(v1Mux)) + v1Handler = middleware.CORS(corsOrigin)(v1Handler) + + return v1Handler, nil +} + func bodySizeLimiter(maxBytes int64) apiv1.MiddlewareFunc { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/api/rate_limit_wiring_test.go b/cmd/api/rate_limit_wiring_test.go new file mode 100644 index 0000000..4f4fe94 --- /dev/null +++ b/cmd/api/rate_limit_wiring_test.go @@ -0,0 +1,45 @@ +/* + * Copyright (C) 2026 InWheel Contributors + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package main + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/InWheelOrg/inwheel-api/internal/middleware" + "golang.org/x/time/rate" +) + +func TestBuildV1Handler_UnverifiedAPIKeyHeaderDoesNotBypassReadLimit(t *testing.T) { + srv := &Server{ + readLimiter: middleware.NewRateLimiter(t.Context(), rate.Every(time.Hour), 1), + } + handler, err := buildV1Handler(srv, "") + if err != nil { + t.Fatalf("buildV1Handler: %v", err) + } + + newReq := func() *http.Request { + r := httptest.NewRequest(http.MethodGet, "/v1/places?lng=1", nil) + r.RemoteAddr = "1.2.3.4:9999" + r.Header.Set("X-API-Key", "garbage") + return r + } + + w1 := httptest.NewRecorder() + handler.ServeHTTP(w1, newReq()) + if w1.Code != http.StatusBadRequest { + t.Fatalf("first request status = %d, want %d (invalid proximity params, no DB touch)", w1.Code, http.StatusBadRequest) + } + + w2 := httptest.NewRecorder() + handler.ServeHTTP(w2, newReq()) + if w2.Code != http.StatusTooManyRequests { + t.Fatalf("second request status = %d, want %d (an unverified X-API-Key header must not bypass the read limiter)", w2.Code, http.StatusTooManyRequests) + } +} diff --git a/fly.toml b/fly.toml index 922cc0d..8a0c9ed 100644 --- a/fly.toml +++ b/fly.toml @@ -21,4 +21,3 @@ primary_region = 'fra' memory = '1gb' cpu_kind = 'shared' cpus = 1 - memory_mb = 1024 diff --git a/internal/middleware/public_rate_limit.go b/internal/middleware/public_rate_limit.go index 0365d64..610f07e 100644 --- a/internal/middleware/public_rate_limit.go +++ b/internal/middleware/public_rate_limit.go @@ -7,16 +7,21 @@ package middleware import ( "encoding/json" + "log/slog" "net/http" + "strconv" ) func PublicRateLimit(rl *RateLimiter) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Header.Get("X-API-Key") == "" && !rl.Allow(ClientIP(r)) { + if APIKeyIDFromCtx(r.Context()) == "" && !rl.Allow(ClientIP(r)) { w.Header().Set("Content-Type", "application/json") + w.Header().Set("Retry-After", strconv.Itoa(rl.RetryAfterSeconds())) w.WriteHeader(http.StatusTooManyRequests) - json.NewEncoder(w).Encode(map[string]string{"error": "rate limit exceeded"}) + if err := json.NewEncoder(w).Encode(map[string]string{"error": "rate limit exceeded"}); err != nil { + slog.Error("PublicRateLimit: encode failed", "error", err) + } return } next.ServeHTTP(w, r) diff --git a/internal/middleware/public_rate_limit_test.go b/internal/middleware/public_rate_limit_test.go index 5780963..8fdb87e 100644 --- a/internal/middleware/public_rate_limit_test.go +++ b/internal/middleware/public_rate_limit_test.go @@ -56,29 +56,51 @@ func TestPublicRateLimit_BlocksOverLimit(t *testing.T) { } } -func TestPublicRateLimit_SkipsRequestsWithAPIKey(t *testing.T) { +func TestPublicRateLimit_SkipsAuthenticatedRequests(t *testing.T) { t.Parallel() - called := false - next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { called = true }) + calls := 0 + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { calls++ }) rl := &RateLimiter{r: rate.Every(time.Hour), b: 1} handler := PublicRateLimit(rl)(next) - r1 := httptest.NewRequest(http.MethodPost, "/v1/places", nil) - r1.RemoteAddr = "1.2.3.4:9999" - r1.Header.Set("X-API-Key", "iwk_test") - handler.ServeHTTP(httptest.NewRecorder(), r1) + authed := func() *http.Request { + r := httptest.NewRequest(http.MethodPost, "/v1/places", nil) + r.RemoteAddr = "1.2.3.4:9999" + return r.WithContext(WithAPIKeyID(r.Context(), "key-id")) + } - r2 := httptest.NewRequest(http.MethodPost, "/v1/places", nil) - r2.RemoteAddr = "1.2.3.4:9999" - r2.Header.Set("X-API-Key", "iwk_test") + handler.ServeHTTP(httptest.NewRecorder(), authed()) + handler.ServeHTTP(httptest.NewRecorder(), authed()) + + if calls != 2 { + t.Fatalf("next was called %d times, want 2 (authenticated requests bypass the read limiter)", calls) + } +} + +func TestPublicRateLimit_DoesNotTrustUnverifiedAPIKeyHeader(t *testing.T) { + t.Parallel() + calls := 0 + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { calls++ }) + + rl := &RateLimiter{r: rate.Every(time.Hour), b: 1} + handler := PublicRateLimit(rl)(next) + + unauthed := func() *http.Request { + r := httptest.NewRequest(http.MethodGet, "/v1/places", nil) + r.RemoteAddr = "1.2.3.4:9999" + r.Header.Set("X-API-Key", "garbage") + return r + } + + handler.ServeHTTP(httptest.NewRecorder(), unauthed()) // consumes the one token w := httptest.NewRecorder() - handler.ServeHTTP(w, r2) + handler.ServeHTTP(w, unauthed()) - if !called { - t.Fatal("next was not called for a request carrying X-API-Key") + if calls != 1 { + t.Fatalf("next was called %d times, want 1 (a bare X-API-Key header must not bypass the limiter)", calls) } - if w.Code != http.StatusOK { - t.Errorf("status = %d, want %d (no status set by middleware)", w.Code, http.StatusOK) + if w.Code != http.StatusTooManyRequests { + t.Errorf("status = %d, want %d", w.Code, http.StatusTooManyRequests) } }