From f0dcdd50cae175be02bb480a7d241db5653003bb Mon Sep 17 00:00:00 2001 From: Daniel Legt Date: Sat, 23 May 2026 19:07:11 +0300 Subject: [PATCH] feat: bypass security for health checks and support HEAD downloads - Allow the `/health` endpoint to bypass the security middleware, ensuring container health checks succeed even if the proxy IP is banned. - Add a test to verify health checks from banned IPs. - Register a HEAD route for file downloads. - Refactor admin alert status checks to use a new `isUnacknowledgedAlert` helper. - Update the security runbook documentation with clearer instructions and examples for trusted proxy configuration. --- docs/security-runbook.md | 8 +- lib/routing/routes.go | 1 + lib/server/admin.go | 10 ++- lib/server/admin_alerts_test.go | 38 ++++++++++ lib/server/admin_security.go | 4 + lib/server/admin_security_test.go | 21 ++++++ lib/server/downloads.go | 44 ++++++++++- lib/server/downloads_test.go | 120 ++++++++++++++++++++++++++++++ lib/server/ip_test.go | 11 +++ templates/admin/security.html | 4 +- 10 files changed, 250 insertions(+), 11 deletions(-) create mode 100644 lib/server/admin_alerts_test.go create mode 100644 lib/server/downloads_test.go diff --git a/docs/security-runbook.md b/docs/security-runbook.md index 2735377..7256fbf 100644 --- a/docs/security-runbook.md +++ b/docs/security-runbook.md @@ -2,26 +2,26 @@ ## Trusted Proxy Setup (Caddy) -Set `WARPBOX_TRUSTED_PROXY_CIDRS` to only the CIDRs of your reverse proxies/load balancers. +Set `WARPBOX_TRUSTED_PROXY_CIDRS` to only the CIDRs of your reverse proxies/load balancers. Without this, WarpBox intentionally ignores forwarding headers and every request may appear to come from the proxy/container bridge, such as `172.30.0.1`. Example: ```bash -WARPBOX_TRUSTED_PROXY_CIDRS=10.0.0.0/8,192.168.0.0/16 +WARPBOX_TRUSTED_PROXY_CIDRS=172.30.0.1/32 ``` Caddy example: ```caddyfile :443 { - reverse_proxy 127.0.0.1:8080 { + reverse_proxy warpbox:8080 { header_up X-Forwarded-For {http.request.remote.host} header_up X-Real-IP {http.request.remote.host} } } ``` -WarpBox will trust `X-Forwarded-For` only if the direct remote IP is inside `WARPBOX_TRUSTED_PROXY_CIDRS`. +WarpBox will trust `X-Forwarded-For` only if the direct remote IP is inside `WARPBOX_TRUSTED_PROXY_CIDRS`. Prefer the exact proxy IP as a `/32` when it is stable. If Caddy is on a changing Docker/Podman network, use that network's CIDR instead. You can find it with `docker network inspect ` or `podman network inspect `. ## IP Ban Operations diff --git a/lib/routing/routes.go b/lib/routing/routes.go index 1768f5b..d5b35bb 100644 --- a/lib/routing/routes.go +++ b/lib/routing/routes.go @@ -57,6 +57,7 @@ func Register(router *gin.Engine, handlers Handlers) { router.GET("/box/:id/download", handlers.DownloadBox) router.GET("/box/:id/files/:filename", handlers.DownloadFile) router.GET("/box/:id/thumbnails/:file_id", handlers.DownloadThumbnail) + router.HEAD("/box/:id/files/:filename", handlers.DownloadFile) router.POST("/box", handlers.CreateBox) router.POST("/box/:id/login", handlers.BoxLoginPost) diff --git a/lib/server/admin.go b/lib/server/admin.go index a027de8..56a97c4 100644 --- a/lib/server/admin.go +++ b/lib/server/admin.go @@ -287,7 +287,7 @@ func (app *App) buildAdminDashboardView() adminDashboardView { } for _, alert := range alertsList { - if alert.Status != alerts.StatusClosed { + if isUnacknowledgedAlert(alert) { view.OpenAlerts++ switch alert.Severity { case "high": @@ -474,10 +474,10 @@ func (app *App) handleAdminAlerts(ctx *gin.Context) { case "closed": closedCount++ } - if alert.Severity == "high" && string(alert.Status) != "closed" { + if alert.Severity == "high" && isUnacknowledgedAlert(alert) { highCount++ } - if alert.Severity == "medium" && string(alert.Status) != "closed" { + if alert.Severity == "medium" && isUnacknowledgedAlert(alert) { mediumCount++ } } @@ -495,3 +495,7 @@ func (app *App) handleAdminAlerts(ctx *gin.Context) { "AlertChipLabel": adminAlertChipLabel(openCount), }) } + +func isUnacknowledgedAlert(alert alerts.Alert) bool { + return alert.Status == "" || alert.Status == alerts.StatusOpen +} diff --git a/lib/server/admin_alerts_test.go b/lib/server/admin_alerts_test.go new file mode 100644 index 0000000..2631347 --- /dev/null +++ b/lib/server/admin_alerts_test.go @@ -0,0 +1,38 @@ +package server + +import ( + "path/filepath" + "testing" + + "warpbox/lib/alerts" + "warpbox/lib/config" +) + +func TestAdminDashboardCountsOnlyUnacknowledgedAlerts(t *testing.T) { + store := alerts.NewStore(filepath.Join(t.TempDir(), "alerts.json")) + for _, alert := range []alerts.Alert{ + {ID: "open-high", Title: "Open high", Severity: "high", Status: alerts.StatusOpen}, + {ID: "acked-high", Title: "Acked high", Severity: "high", Status: alerts.StatusAcked}, + {ID: "closed-medium", Title: "Closed medium", Severity: "medium", Status: alerts.StatusClosed}, + } { + if err := store.Add(alert); err != nil { + t.Fatalf("Add returned error: %v", err) + } + } + + app := &App{ + config: &config.Config{}, + alertStore: store, + } + view := app.buildAdminDashboardView() + + if view.OpenAlerts != 1 { + t.Fatalf("expected only unacknowledged alerts in dashboard count, got %d", view.OpenAlerts) + } + if view.HighAlerts != 1 || view.MediumAlerts != 0 || view.LowAlerts != 0 { + t.Fatalf("expected only open alert severities, got high=%d medium=%d low=%d", view.HighAlerts, view.MediumAlerts, view.LowAlerts) + } + if len(view.Alerts) != 1 || view.Alerts[0].ID != "open-high" { + t.Fatalf("expected only open alert in dashboard inbox, got %#v", view.Alerts) + } +} diff --git a/lib/server/admin_security.go b/lib/server/admin_security.go index 5fbb3a9..31b9652 100644 --- a/lib/server/admin_security.go +++ b/lib/server/admin_security.go @@ -103,6 +103,10 @@ func (app *App) createAlert(title string, severity string, group string, code st func (app *App) securityMiddleware() gin.HandlerFunc { return func(ctx *gin.Context) { + if ctx.Request != nil && ctx.Request.URL != nil && ctx.Request.URL.Path == "/health" { + ctx.Next() + return + } if !app.securityFeaturesEnabled() { ctx.Next() return diff --git a/lib/server/admin_security_test.go b/lib/server/admin_security_test.go index 3634cfb..f8eda46 100644 --- a/lib/server/admin_security_test.go +++ b/lib/server/admin_security_test.go @@ -16,6 +16,27 @@ import ( "warpbox/lib/security" ) +func TestSecurityMiddlewareAllowsHealthCheckFromBannedIP(t *testing.T) { + app := &App{ + config: &config.Config{SecurityEnabled: true}, + securityGuard: security.NewGuard(), + } + app.securityGuard.Ban("172.30.0.1", 300) + + router := gin.New() + router.Use(app.securityMiddleware()) + router.GET("/health", app.handleHealth) + + request := httptest.NewRequest(http.MethodGet, "/health", nil) + request.RemoteAddr = "172.30.0.1:12345" + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + + if response.Code != http.StatusOK { + t.Fatalf("expected health check to pass, got %d", response.Code) + } +} + func TestAdminSecurityActionsWriteAuditTrail(t *testing.T) { app, router := setupAdminSecurityTest(t) diff --git a/lib/server/downloads.go b/lib/server/downloads.go index 9bdbea9..b1930e7 100644 --- a/lib/server/downloads.go +++ b/lib/server/downloads.go @@ -4,8 +4,10 @@ import ( "archive/zip" "fmt" "io" + "mime" "net/http" "os" + "strings" "sync" "github.com/gin-gonic/gin" @@ -233,7 +235,8 @@ func (app *App) handleDownloadFile(ctx *gin.Context) { return } - if _, err := os.Stat(path); err != nil { + info, err := os.Stat(path) + if err != nil { ctx.String(http.StatusNotFound, "File not found") return } @@ -242,12 +245,49 @@ func (app *App) handleDownloadFile(ctx *gin.Context) { return } - ctx.FileAttachment(path, filename) + if !app.serveDownloadFile(ctx, path, filename, info) { + return + } if hasManifest && app.config.RenewOnDownloadEnabled { boxstore.RenewManifest(boxID, manifest.RetentionSecs) } } +func (app *App) serveDownloadFile(ctx *gin.Context, path string, filename string, info os.FileInfo) bool { + file, err := os.Open(path) + if err != nil { + ctx.String(http.StatusInternalServerError, "Could not read file") + return false + } + defer file.Close() + + mimeType := helpers.MimeTypeForFile(path, filename) + ctx.Header("Content-Type", mimeType) + ctx.Header("Content-Disposition", contentDispositionForDownload(filename, mimeType)) + ctx.Header("X-Content-Type-Options", "nosniff") + http.ServeContent(ctx.Writer, ctx.Request, filename, info.ModTime(), file) + return true +} + +func contentDispositionForDownload(filename string, mimeType string) string { + disposition := "attachment" + if isEmbeddableMimeType(mimeType) { + disposition = "inline" + } + return mime.FormatMediaType(disposition, map[string]string{"filename": filename}) +} + +func isEmbeddableMimeType(mimeType string) bool { + baseType := strings.ToLower(strings.TrimSpace(strings.Split(mimeType, ";")[0])) + if strings.HasPrefix(baseType, "video/") || strings.HasPrefix(baseType, "audio/") { + return true + } + if strings.HasPrefix(baseType, "image/") { + return baseType != "image/svg+xml" + } + return baseType == "application/pdf" || baseType == "text/plain" +} + func (app *App) handleDownloadThumbnail(ctx *gin.Context) { boxID := ctx.Param("id") fileID := ctx.Param("file_id") diff --git a/lib/server/downloads_test.go b/lib/server/downloads_test.go new file mode 100644 index 0000000..6cd60ca --- /dev/null +++ b/lib/server/downloads_test.go @@ -0,0 +1,120 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + + "warpbox/lib/boxstore" + "warpbox/lib/config" + "warpbox/lib/models" +) + +const downloadTestBoxID = "abcdefabcdefabcdefabcdefabcdefab" + +func TestDownloadFileServesEmbeddableMediaInlineWithRangeSupport(t *testing.T) { + app := setupDownloadFileTest(t, "clip.mp4", []byte("0123456789")) + + response := performDownloadFile(app, http.MethodGet, "/box/"+downloadTestBoxID+"/files/clip.mp4", map[string]string{ + "Range": "bytes=0-3", + }) + + if response.Code != http.StatusPartialContent { + t.Fatalf("expected ranged download to return 206, got %d", response.Code) + } + if got := response.Header().Get("Content-Disposition"); !strings.HasPrefix(got, "inline;") || !strings.Contains(got, "filename=clip.mp4") { + t.Fatalf("expected inline content disposition for embeddable media, got %q", got) + } + if got := response.Header().Get("Content-Type"); !strings.HasPrefix(got, "video/mp4") { + t.Fatalf("expected video content type, got %q", got) + } + if got := response.Header().Get("Content-Range"); got != "bytes 0-3/10" { + t.Fatalf("expected byte range header, got %q", got) + } + if got := response.Body.String(); got != "0123" { + t.Fatalf("expected ranged body, got %q", got) + } +} + +func TestDownloadFileServesUnsafeInlineTypesAsAttachments(t *testing.T) { + app := setupDownloadFileTest(t, "page.html", []byte("")) + + response := performDownloadFile(app, http.MethodGet, "/box/"+downloadTestBoxID+"/files/page.html", nil) + + if response.Code != http.StatusOK { + t.Fatalf("expected download to return 200, got %d", response.Code) + } + if got := response.Header().Get("Content-Disposition"); !strings.HasPrefix(got, "attachment;") || !strings.Contains(got, "filename=page.html") { + t.Fatalf("expected attachment content disposition for html, got %q", got) + } +} + +func TestDownloadFileSupportsHeadRequests(t *testing.T) { + app := setupDownloadFileTest(t, "clip.mp4", []byte("0123456789")) + + response := performDownloadFile(app, http.MethodHead, "/box/"+downloadTestBoxID+"/files/clip.mp4", nil) + + if response.Code != http.StatusOK { + t.Fatalf("expected HEAD download to return 200, got %d", response.Code) + } + if got := response.Header().Get("Content-Disposition"); !strings.HasPrefix(got, "inline;") { + t.Fatalf("expected inline content disposition for HEAD request, got %q", got) + } + if response.Body.Len() != 0 { + t.Fatalf("expected HEAD response body to be empty, got %d bytes", response.Body.Len()) + } +} + +func setupDownloadFileTest(t *testing.T, filename string, body []byte) *App { + t.Helper() + gin.SetMode(gin.TestMode) + + restoreUploadRoot := boxstore.UploadRoot() + t.Cleanup(func() { boxstore.SetUploadRoot(restoreUploadRoot) }) + boxstore.SetUploadRoot(t.TempDir()) + + if err := os.MkdirAll(boxstore.BoxPath(downloadTestBoxID), 0755); err != nil { + t.Fatalf("MkdirAll returned error: %v", err) + } + path, ok := boxstore.SafeBoxFilePath(downloadTestBoxID, filename) + if !ok { + t.Fatal("SafeBoxFilePath rejected test file") + } + if err := os.WriteFile(path, body, 0644); err != nil { + t.Fatalf("WriteFile returned error: %v", err) + } + + manifest := models.BoxManifest{ + Files: []models.BoxFile{{ + ID: "0123456789abcdef", + Name: filename, + Size: int64(len(body)), + MimeType: "", + Status: models.FileStatusReady, + }}, + CreatedAt: time.Now().UTC(), + } + if err := boxstore.WriteManifest(downloadTestBoxID, manifest); err != nil { + t.Fatalf("WriteManifest returned error: %v", err) + } + + return &App{config: &config.Config{}} +} + +func performDownloadFile(app *App, method string, path string, headers map[string]string) *httptest.ResponseRecorder { + router := gin.New() + router.GET("/box/:id/files/:filename", app.handleDownloadFile) + router.HEAD("/box/:id/files/:filename", app.handleDownloadFile) + request := httptest.NewRequest(method, path, nil) + for key, value := range headers { + request.Header.Set(key, value) + } + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + return response +} diff --git a/lib/server/ip_test.go b/lib/server/ip_test.go index 5678104..7e2dc5f 100644 --- a/lib/server/ip_test.go +++ b/lib/server/ip_test.go @@ -32,6 +32,17 @@ func TestClientIPTrustedProxyChain(t *testing.T) { } } +func TestClientIPTrustedDockerBridgeProxy(t *testing.T) { + app := &App{config: &config.Config{TrustedProxyCIDRs: "172.30.0.1/32"}} + ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx.Request = httptest.NewRequest(http.MethodGet, "/", nil) + ctx.Request.RemoteAddr = "172.30.0.1:8080" + ctx.Request.Header.Set("X-Forwarded-For", "198.51.100.55") + if got := app.clientIP(ctx); got != "198.51.100.55" { + t.Fatalf("expected forwarded client IP from trusted docker bridge, got %q", got) + } +} + func TestClientIPSpoofedHeaderFromUntrustedRemote(t *testing.T) { app := &App{config: &config.Config{TrustedProxyCIDRs: "10.0.0.0/8"}} ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) diff --git a/templates/admin/security.html b/templates/admin/security.html index 98a38c9..d95c464 100644 --- a/templates/admin/security.html +++ b/templates/admin/security.html @@ -141,10 +141,10 @@
Security Runbookops quick reference

Reverse Proxy and Trusted CIDRs

-

Set WARPBOX_TRUSTED_PROXY_CIDRS to the CIDRs of your proxy nodes only. WarpBox will trust forwarding headers only when the direct remote IP is in this list.

+

Set WARPBOX_TRUSTED_PROXY_CIDRS to the CIDRs of your proxy nodes only. Without this, all traffic can appear as the proxy or bridge IP, such as 172.30.0.1.

Caddyfile
 :443 {
-  reverse_proxy 127.0.0.1:8080 {
+  reverse_proxy warpbox:8080 {
     header_up X-Forwarded-For {http.request.remote.host}
     header_up X-Real-IP {http.request.remote.host}
   }