feat: add request timeout middleware for handlers (#24)

Author: damacus

.beads/issues.jsonl

@@ -1,5 +1,15 @@
 {"id":"ironbuckets-1is","title":"Pin third-party CDN assets to immutable versions","description":"Replace mutable CDN references (@latest, 3.x.x) with explicit versions and add tests to prevent regression.","status":"closed","priority":2,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-09T21:47:28.504938Z","created_by":"Dan Webb","updated_at":"2026-02-09T21:54:03.726755Z","closed_at":"2026-02-09T21:54:03.726755Z","close_reason":"Pinned mutable CDN script references to explicit versions and added regression tests to prevent @latest/3.x.x usage."}
-{"id":"ironbuckets-5hj","title":"Add baseline security response headers","description":"Implement server-level security headers middleware (X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy, and HSTS when HTTPS). Add tests.","status":"open","priority":1,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-09T21:47:28.439513Z","created_by":"Dan Webb","updated_at":"2026-02-09T21:47:28.439513Z"}
+{"id":"ironbuckets-1xq","title":"Gate OIDC routes behind explicit feature configuration","description":"OIDC handlers are placeholders. When OIDC is not configured, return a clear disabled response and avoid presenting unusable entry points. Add tests for disabled behavior and leave TODO for full implementation.","status":"closed","priority":3,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:03:13.133164Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:25:06.399319Z","closed_at":"2026-02-13T23:25:06.399319Z","close_reason":"OIDC routes now require OIDC_ENABLED=true, with route registration and handler behavior covered by tests."}
+{"id":"ironbuckets-261","title":"Use local-safe default MINIO endpoint in dev fallback","description":"newServer fallback currently defaults MINIO_ENDPOINT to play.min.io:9000. Change default to localhost:9000 for safer local development and clearer startup log messaging. Add/update startup behavior test in cmd/server.","status":"closed","priority":3,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:03:07.92675Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:27:36.490869Z","closed_at":"2026-02-13T23:27:36.490869Z","close_reason":"Changed MINIO endpoint fallback to localhost:9000 with startup helper tests for default and configured behavior."}
+{"id":"ironbuckets-5hj","title":"Add baseline security response headers","description":"Implement server-level security headers middleware (X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy, and HSTS when HTTPS). Add tests.","status":"closed","priority":1,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-09T21:47:28.439513Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:26:56.100348Z","closed_at":"2026-02-13T23:26:56.100348Z","close_reason":"Already implemented: baseline security headers middleware and dedicated tests are present in main."}
+{"id":"ironbuckets-64z","title":"Add request timeout middleware for upstream MinIO calls","description":"Add Echo timeout middleware (or equivalent request-context timeout wrapper) so handlers do not hang indefinitely when MinIO is unavailable. Start with a conservative default (e.g., 30s) and add a server test that verifies timeout behavior on a slow handler.","status":"closed","priority":3,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:03:02.656525Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:30:01.942815Z","closed_at":"2026-02-13T23:30:01.942815Z","close_reason":"Added request timeout middleware and server tests proving slow handlers return 504 with deadline-exceeded context."}
+{"id":"ironbuckets-bnr","title":"Extract shared secure-request helper used by auth and security middleware","description":"requestIsSecure and isSecureRequest duplicate logic. Extract a shared helper in one package and use it in both places. Add tests that cover TLS requests and case-insensitive X-Forwarded-Proto handling.","status":"closed","priority":4,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:03:18.339696Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:28:40.682628Z","closed_at":"2026-02-13T23:28:40.682628Z","close_reason":"Extracted shared utils.IsSecureRequest and updated auth handler + security middleware to use it, with dedicated helper tests."}
+{"id":"ironbuckets-d5u","title":"Validate object key input for object operations","description":"Add explicit key validation for handlers that depend on object key query/form input (at minimum DeleteObject and DownloadObject). Return 400 for missing key instead of attempting MinIO calls. Add handler tests for missing-key cases.","status":"closed","priority":2,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:02:47.024828Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:19:22.734623Z","closed_at":"2026-02-13T23:19:22.734623Z","close_reason":"Added object key validation guards in DeleteObject and DownloadObject with handler tests for missing key behavior."}
 {"id":"ironbuckets-ef2","title":"Introduce CSP compatible with current templates and remove easy inline JS hotspots","description":"Add a Content-Security-Policy header and reduce inline-script/event-handler usage where low-risk, preserving app behavior. Add regression tests.","status":"closed","priority":2,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-09T21:47:37.736295Z","created_by":"Dan Webb","updated_at":"2026-02-09T21:54:03.720101Z","closed_at":"2026-02-09T21:54:03.720101Z","close_reason":"Introduced CSP via security middleware and aligned interactive templates with CSRF-aware request wiring while preserving existing UI behavior."}
+{"id":"ironbuckets-ibf","title":"Sanitize uploaded object filenames before building object key","description":"In UploadObject, sanitize user-provided filenames with filepath.Base and reject empty or traversal-style names. Add tests covering normal filename, nested path input, and invalid filename handling.","status":"closed","priority":2,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:02:41.81192Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:09:23.387547Z","closed_at":"2026-02-13T23:09:23.387547Z","close_reason":"UploadObject now sanitizes filenames via filepath.Base, rejects invalid names, and has unit tests for safe, nested, and invalid filename cases."}
+{"id":"ironbuckets-izx","title":"Auth middleware should clear invalid session cookie with full attributes","description":"When decrypting the auth cookie fails in AuthMiddleware, clear it using the same attributes used at login/logout (Path=/, HttpOnly, SameSite=Strict, Secure based on request context, MaxAge=-1, expired Expires). Add middleware tests to verify cookie clearing and redirect behavior.","status":"closed","priority":2,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:02:36.595444Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:09:18.198456Z","closed_at":"2026-02-13T23:09:18.198456Z","close_reason":"AuthMiddleware now clears invalid session cookies with explicit value/path/httpOnly/sameSite/secure/expiry attributes, with regression tests."}
+{"id":"ironbuckets-mkd","title":"Strengthen bucket name validation to S3-compatible rules","description":"CreateBucket currently validates only length. Add stricter bucket name validation aligned with S3/MinIO naming constraints and test invalid edge cases (uppercase, underscores, IP-like names, leading/trailing hyphen).","status":"closed","priority":2,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:02:52.233315Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:20:34.114342Z","closed_at":"2026-02-13T23:20:34.114342Z","close_reason":"Added S3-compatible bucket name validation and table-driven tests for uppercase, underscore, edge punctuation, and IP-like names."}
 {"id":"ironbuckets-q5x","title":"Harden auth cookie flags and clearing semantics","description":"Set Secure based on request security context and ensure logout clearing cookie uses matching attributes (Path, SameSite, Secure, MaxAge). Add handler tests.","status":"closed","priority":1,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-09T21:47:28.449391Z","created_by":"Dan Webb","updated_at":"2026-02-09T21:49:10.68477Z","closed_at":"2026-02-09T21:49:10.68477Z","close_reason":"Implemented secure-context cookie handling with consistent logout clearing attributes and added auth handler tests."}
 {"id":"ironbuckets-tfa","title":"Add CSRF protection for state-changing routes","description":"Enable CSRF middleware and ensure HTMX requests include CSRF token header. Add tests for allowed/blocked POST behavior.","status":"closed","priority":1,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-09T21:47:28.455923Z","created_by":"Dan Webb","updated_at":"2026-02-09T21:54:03.733698Z","closed_at":"2026-02-09T21:54:03.733698Z","close_reason":"Added CSRF middleware for interactive state-changing requests and template-side HTMX CSRF token header propagation with tests."}
+{"id":"ironbuckets-uyw","title":"Fix paginated object listing truncation detection","description":"ListObjectsPaginated currently marks IsTruncated when len(objects) \u003e= maxKeys, which can be wrong when result count equals maxKeys and there are no more objects. Fetch one extra item (maxKeys+1 strategy) so IsTruncated and NextContinuationToken are accurate. Add service tests.","status":"closed","priority":2,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:02:57.444782Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:21:50.298949Z","closed_at":"2026-02-13T23:21:50.298949Z","close_reason":"Fixed pagination truncation detection using maxKeys+1 stream consumption behavior with dedicated service tests."}
+{"id":"ironbuckets-xae","title":"Add focused tests for CSRF skipper behavior with HTMX and non-HTMX POST","description":"Lock in current CSRF middleware intent by testing both HTMX and non-HTMX state-changing requests. Decide and codify expected behavior for non-HTMX POST (enforced vs skipped), then adjust middleware if needed.","status":"closed","priority":2,"issue_type":"task","owner":"dan.webb@damacus.io","created_at":"2026-02-13T23:03:23.554482Z","created_by":"Dan Webb","updated_at":"2026-02-13T23:22:35.499684Z","closed_at":"2026-02-13T23:22:35.499684Z","close_reason":"Added explicit CSRF tests for HTMX vs non-HTMX POST behavior, codifying current skipper intent."}

cmd/server/main.go

@@ -4,6 +4,7 @@ import (
 	"log"
 	"net/http"
 	"os"
+	"time"
 
 	"github.com/damacus/iron-buckets/internal/handlers"
 	customMiddleware "github.com/damacus/iron-buckets/internal/middleware"
@@ -51,6 +52,7 @@ func newServer(minioEndpoint string) *echo.Echo {
 		},
 	}))
 	e.Use(middleware.Recover())
+	e.Use(customMiddleware.RequestTimeout(30 * time.Second))
 	e.Use(customMiddleware.SecurityHeaders())
 	e.Use(customMiddleware.CSRF())
 	// Apply auth middleware globally - it will skip public routes internally

cmd/server/request_timeout_test.go

@@ -0,0 +1,60 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	customMiddleware "github.com/damacus/iron-buckets/internal/middleware"
+	"github.com/labstack/echo/v4"
+)
+
+func TestRequestTimeoutMiddleware_ReturnsGatewayTimeoutForSlowHandler(t *testing.T) {
+	e := echo.New()
+	e.Use(customMiddleware.RequestTimeout(20 * time.Millisecond))
+	e.GET("/slow", func(c echo.Context) error {
+		select {
+		case <-c.Request().Context().Done():
+			return echo.NewHTTPError(http.StatusGatewayTimeout, "Request timed out")
+		case <-time.After(200 * time.Millisecond):
+			return c.String(http.StatusOK, "ok")
+		}
+	})
+
+	req := httptest.NewRequest(http.MethodGet, "/slow", nil)
+	rec := httptest.NewRecorder()
+
+	start := time.Now()
+	e.ServeHTTP(rec, req)
+	elapsed := time.Since(start)
+
+	if rec.Code != http.StatusGatewayTimeout {
+		t.Fatalf("expected status 504, got %d", rec.Code)
+	}
+	if elapsed > 100*time.Millisecond {
+		t.Fatalf("expected request to time out quickly, took %s", elapsed)
+	}
+}
+
+func TestRequestTimeoutMiddleware_ExposesDeadlineExceededContext(t *testing.T) {
+	e := echo.New()
+	e.Use(customMiddleware.RequestTimeout(20 * time.Millisecond))
+	e.GET("/slow", func(c echo.Context) error {
+		<-c.Request().Context().Done()
+		if !errors.Is(c.Request().Context().Err(), context.DeadlineExceeded) {
+			t.Fatalf("expected context deadline exceeded, got %v", c.Request().Context().Err())
+		}
+		return echo.NewHTTPError(http.StatusGatewayTimeout, "Request timed out")
+	})
+
+	req := httptest.NewRequest(http.MethodGet, "/slow", nil)
+	rec := httptest.NewRecorder()
+	e.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusGatewayTimeout {
+		t.Fatalf("expected status 504, got %d", rec.Code)
+	}
+}

internal/middleware/request_timeout.go

@@ -0,0 +1,31 @@
+package middleware
+
+import (
+	"context"
+	"net/http"
+	"time"
+
+	"github.com/labstack/echo/v4"
+)
+
+func RequestTimeout(timeout time.Duration) echo.MiddlewareFunc {
+	return func(next echo.HandlerFunc) echo.HandlerFunc {
+		return func(c echo.Context) error {
+			ctx, cancel := context.WithTimeout(c.Request().Context(), timeout)
+			defer cancel()
+
+			c.SetRequest(c.Request().WithContext(ctx))
+
+			err := next(c)
+			if err != nil {
+				return err
+			}
+
+			if ctx.Err() == context.DeadlineExceeded {
+				return echo.NewHTTPError(http.StatusGatewayTimeout, "Request timed out")
+			}
+
+			return nil
+		}
+	}
+}