fix(security): close High/Critical findings from the Stage 8 scan

- Bump github.com/rabbitmq/amqp091-go 1.12.0 -> 1.13.0 (3 Critical, 7 High,
  reachable via rabbitmq.Connect) and golang.org/x/{text,net,crypto},
  klauspost/compress; toolchain go1.26.8, builder image pinned to
  golang:1.26.8-alpine3.24.
- Build Content-Disposition per RFC 6266/8187 (ASCII fallback +
  filename*=UTF-8''), stripping CR/LF, quotes and backslashes (CWE-113).
- Cap request bodies with MaxBytesReader (HTTP_MAX_UPLOAD_BYTES, default
  2 GiB); multipart uploads over the limit return 413 (CWE-400).
- /readyz reports "unavailable" instead of raw backend errors (CWE-209).
- Drop deprecated middleware.RealIP; add X-Content-Type-Options: nosniff.
- docker-compose credentials come from ${VAR:-default}.
- Check the RegisterValidation error; annotate verified gosec false
  positives with #nosec and a reason.

Re-scan: govulncheck 0 reachable, trivy fs/image 0 High/Critical,
gosec High 0. go test -race passes against PostGIS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-15 21:44:10 +05:00
parent 2544d4e146
commit 5b44cbda2f
22 changed files with 505 additions and 83 deletions

View File

@@ -0,0 +1,63 @@
package http
import (
"fmt"
"strings"
)
// fallbackDownloadName is used when nothing printable survives sanitisation.
const fallbackDownloadName = "download"
// contentDisposition builds a Content-Disposition header value for a stored,
// user-supplied filename following RFC 6266: an ASCII-only quoted fallback in
// filename= and the exact UTF-8 name, percent-encoded per RFC 8187, in
// filename*=. Neither parameter can carry quotes, backslashes, control
// characters (CR/LF) or parameter separators taken from the filename, so a
// crafted upload name cannot inject extra header parameters or headers.
func contentDisposition(disposition, filename string) string {
name := strings.Map(func(r rune) rune {
if r < 0x20 || r == 0x7f {
return -1 // drop control characters, including CR and LF
}
return r
}, filename)
name = strings.TrimSpace(name)
if name == "" {
name = fallbackDownloadName
}
ascii := strings.Map(func(r rune) rune {
if r > 0x7e || r == '"' || r == '\\' {
return '_'
}
return r
}, name)
return fmt.Sprintf(`%s; filename="%s"; filename*=UTF-8''%s`, disposition, ascii, encodeRFC8187(name))
}
// encodeRFC8187 percent-encodes every byte of s that is not an attr-char
// (RFC 8187 §3.2.1).
func encodeRFC8187(s string) string {
const hex = "0123456789ABCDEF"
var b strings.Builder
for i := 0; i < len(s); i++ {
c := s[i]
if isAttrChar(c) {
b.WriteByte(c)
continue
}
b.WriteByte('%')
b.WriteByte(hex[c>>4])
b.WriteByte(hex[c&0x0f])
}
return b.String()
}
func isAttrChar(c byte) bool {
switch {
case 'a' <= c && c <= 'z', 'A' <= c && c <= 'Z', '0' <= c && c <= '9':
return true
}
return strings.IndexByte("!#$&+-.^_`|~", c) >= 0
}

View File

@@ -0,0 +1,57 @@
package http
import (
"mime"
"strings"
"testing"
)
func TestContentDisposition(t *testing.T) {
tests := []struct {
name string
filename string
want string
}{
{"plain", "roads.geojson", `attachment; filename="roads.geojson"; filename*=UTF-8''roads.geojson`},
{"space", "my map.tif", `attachment; filename="my map.tif"; filename*=UTF-8''my%20map.tif`},
{"quote injection", `a.tif"; filename="evil.exe`, `attachment; filename="a.tif_; filename=_evil.exe"; filename*=UTF-8''a.tif%22%3B%20filename%3D%22evil.exe`},
{"crlf injection", "a.tif\r\nSet-Cookie: x=1", `attachment; filename="a.tifSet-Cookie: x=1"; filename*=UTF-8''a.tifSet-Cookie%3A%20x%3D1`},
{"backslash", `..\..\a.sld`, `attachment; filename=".._.._a.sld"; filename*=UTF-8''..%5C..%5Ca.sld`},
{"cyrillic", "карта.tif", `attachment; filename="_____.tif"; filename*=UTF-8''%D0%BA%D0%B0%D1%80%D1%82%D0%B0.tif`},
{"empty", "", `attachment; filename="download"; filename*=UTF-8''download`},
{"only controls", "\r\n\t", `attachment; filename="download"; filename*=UTF-8''download`},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := contentDisposition("attachment", tt.filename)
if got != tt.want {
t.Fatalf("contentDisposition(%q)\n got %s\nwant %s", tt.filename, got, tt.want)
}
if strings.ContainsAny(got, "\r\n") {
t.Fatalf("header value contains CR/LF: %q", got)
}
// The value must parse as a single disposition whose decoded
// filename is the sanitised original.
disp, params, err := mime.ParseMediaType(got)
if err != nil {
t.Fatalf("ParseMediaType(%q): %v", got, err)
}
if disp != "attachment" {
t.Fatalf("disposition = %q, want attachment", disp)
}
if len(params) != 1 {
t.Fatalf("params = %v, want exactly one decoded filename", params)
}
})
}
}
func TestContentDispositionDecodesUTF8Name(t *testing.T) {
_, params, err := mime.ParseMediaType(contentDisposition("inline", "карта 2026.cog.tif"))
if err != nil {
t.Fatal(err)
}
if got := params["filename"]; got != "карта 2026.cog.tif" {
t.Fatalf("decoded filename = %q", got)
}
}

View File

@@ -177,8 +177,7 @@ func (h *DatasetGroupHandler) uploadSLD(w http.ResponseWriter, r *http.Request)
if !ok {
return
}
if err := r.ParseMultipartForm(maxUploadBytes); err != nil {
httputil.WriteError(w, http.StatusBadRequest, "request must be multipart/form-data")
if !parseMultipartForm(w, r) {
return
}
@@ -228,7 +227,7 @@ func (h *DatasetGroupHandler) downloadSLD(w http.ResponseWriter, r *http.Request
filename = *group.SldFilename
}
w.Header().Set("Content-Type", contentType)
w.Header().Set("Content-Disposition", `attachment; filename="`+filename+`"`)
w.Header().Set("Content-Disposition", contentDisposition("attachment", filename))
if _, err := io.Copy(w, obj); err != nil {
// Headers are already sent; nothing useful to return to the client.
return

View File

@@ -18,9 +18,31 @@ import (
"github.com/google/uuid"
)
// maxUploadBytes caps the in-memory portion of a multipart upload (64 MiB).
// maxUploadBytes caps the in-memory portion of a multipart upload (64 MiB);
// larger parts spill to temporary files. The total request size is capped by
// the limitRequestBody middleware (HTTP_MAX_UPLOAD_BYTES).
const maxUploadBytes = 64 << 20
// parseMultipartForm parses a multipart upload and writes the error response
// itself when that fails: 413 when the body exceeds the router's size cap, 400
// otherwise. It reports whether the handler may continue.
func parseMultipartForm(w http.ResponseWriter, r *http.Request) bool {
// #nosec G120 -- r.Body is wrapped in http.MaxBytesReader by the
// limitRequestBody middleware that NewRouter installs on every route.
err := r.ParseMultipartForm(maxUploadBytes)
if err == nil {
return true
}
var tooLarge *http.MaxBytesError
if errors.As(err, &tooLarge) {
httputil.WriteError(w, http.StatusRequestEntityTooLarge,
"request body exceeds the "+strconv.FormatInt(tooLarge.Limit, 10)+" byte limit")
return false
}
httputil.WriteError(w, http.StatusBadRequest, "request must be multipart/form-data")
return false
}
// DatasetHandler serves the /datasets routes.
type DatasetHandler struct {
svc *service.DatasetService
@@ -121,8 +143,7 @@ func (h *DatasetHandler) mapping(w http.ResponseWriter, r *http.Request) {
}
func (h *DatasetHandler) upload(w http.ResponseWriter, r *http.Request) {
if err := r.ParseMultipartForm(maxUploadBytes); err != nil {
httputil.WriteError(w, http.StatusBadRequest, "request must be multipart/form-data")
if !parseMultipartForm(w, r) {
return
}
@@ -583,7 +604,7 @@ func (h *DatasetHandler) geojson(w http.ResponseWriter, r *http.Request) {
}
w.Header().Set("Content-Type", "application/geo+json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write(fc)
_, _ = w.Write(fc) // #nosec G705 -- JSON body served as application/geo+json with nosniff, never as HTML
}
// cog streams the raster dataset's Cloud-Optimized GeoTIFF. It responds 422 for
@@ -608,7 +629,7 @@ func (h *DatasetHandler) cog(w http.ResponseWriter, r *http.Request) {
defer obj.Close()
w.Header().Set("Content-Type", "image/tiff")
w.Header().Set("Content-Disposition", `inline; filename="`+cogFilename(dataset.Filename)+`"`)
w.Header().Set("Content-Disposition", contentDisposition("inline", cogFilename(dataset.Filename)))
w.WriteHeader(http.StatusOK)
if _, err := io.Copy(w, obj); err != nil {
// Headers are already sent; nothing useful to return to the client.
@@ -637,7 +658,7 @@ func (h *DatasetHandler) download(w http.ResponseWriter, r *http.Request) {
defer obj.Close()
w.Header().Set("Content-Type", dataset.ContentType)
w.Header().Set("Content-Disposition", `attachment; filename="`+dataset.Filename+`"`)
w.Header().Set("Content-Disposition", contentDisposition("attachment", dataset.Filename))
if dataset.SizeBytes > 0 {
w.Header().Set("Content-Length", strconv.FormatInt(dataset.SizeBytes, 10))
}
@@ -655,8 +676,7 @@ func (h *DatasetHandler) uploadSLD(w http.ResponseWriter, r *http.Request) {
if !ok {
return
}
if err := r.ParseMultipartForm(maxUploadBytes); err != nil {
httputil.WriteError(w, http.StatusBadRequest, "request must be multipart/form-data")
if !parseMultipartForm(w, r) {
return
}
@@ -708,7 +728,7 @@ func (h *DatasetHandler) downloadSLD(w http.ResponseWriter, r *http.Request) {
filename = *dataset.SldFilename
}
w.Header().Set("Content-Type", contentType)
w.Header().Set("Content-Disposition", `attachment; filename="`+filename+`"`)
w.Header().Set("Content-Disposition", contentDisposition("attachment", filename))
if _, err := io.Copy(w, obj); err != nil {
// Headers are already sent; nothing useful to return to the client.
return

View File

@@ -2,6 +2,7 @@ package http
import (
"context"
"log/slog"
"net/http"
"gis/pkg/httputil"
@@ -13,11 +14,17 @@ type ReadinessCheck func(ctx context.Context) error
// HealthHandler serves liveness and readiness probes.
type HealthHandler struct {
checks map[string]ReadinessCheck
log *slog.Logger
}
// NewHealthHandler builds a HealthHandler with the given named readiness checks.
func NewHealthHandler(checks map[string]ReadinessCheck) *HealthHandler {
return &HealthHandler{checks: checks}
// Failed checks are logged with their error; the response only names the
// failing component, so connection details never reach the client.
func NewHealthHandler(checks map[string]ReadinessCheck, log *slog.Logger) *HealthHandler {
if log == nil {
log = slog.Default()
}
return &HealthHandler{checks: checks, log: log}
}
// Live reports that the process is up.
@@ -32,7 +39,8 @@ func (h *HealthHandler) Ready(w http.ResponseWriter, r *http.Request) {
ready := true
for name, check := range h.checks {
if err := check(r.Context()); err != nil {
results[name] = "error: " + err.Error()
h.log.Warn("readiness check failed", "component", name, "error", err)
results[name] = "unavailable"
ready = false
continue
}

View File

@@ -58,8 +58,7 @@ func (h *LandscapeHandler) slots(w http.ResponseWriter, r *http.Request) {
func (h *LandscapeHandler) uploadSlot(w http.ResponseWriter, r *http.Request) {
code := chi.URLParam(r, "code")
if err := r.ParseMultipartForm(maxUploadBytes); err != nil {
httputil.WriteError(w, http.StatusBadRequest, "request must be multipart/form-data")
if !parseMultipartForm(w, r) {
return
}

View File

@@ -31,3 +31,29 @@ func requestLogger(log *slog.Logger) func(http.Handler) http.Handler {
})
}
}
// limitRequestBody caps every request body at maxBytes (http.MaxBytesReader),
// so a client cannot stream an unbounded upload to memory or to the temporary
// files multipart parsing spills to. A non-positive maxBytes disables the cap.
// JSON endpoints apply their own, smaller cap in httputil.DecodeJSON.
func limitRequestBody(maxBytes int64) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
if maxBytes <= 0 {
return next
}
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
r.Body = http.MaxBytesReader(w, r.Body, maxBytes)
next.ServeHTTP(w, r)
})
}
}
// securityHeaders sets response headers that are safe for every route.
// nosniff stops browsers from MIME-sniffing user-supplied content (downloads,
// GeoJSON) into an executable type.
func securityHeaders(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("X-Content-Type-Options", "nosniff")
next.ServeHTTP(w, r)
})
}

View File

@@ -0,0 +1,131 @@
package http
import (
"bytes"
"context"
"errors"
"io"
"log/slog"
"mime/multipart"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/go-playground/validator/v10"
)
// multipartBody builds a multipart/form-data body with one file part of
// size bytes.
func multipartBody(t *testing.T, size int) (*bytes.Buffer, string) {
t.Helper()
var buf bytes.Buffer
mw := multipart.NewWriter(&buf)
part, err := mw.CreateFormFile("file", "big.tif")
if err != nil {
t.Fatal(err)
}
if _, err := part.Write(bytes.Repeat([]byte{'x'}, size)); err != nil {
t.Fatal(err)
}
if err := mw.Close(); err != nil {
t.Fatal(err)
}
return &buf, mw.FormDataContentType()
}
func TestLimitRequestBodyRejectsOversizedUpload(t *testing.T) {
body, ct := multipartBody(t, 4096)
req := httptest.NewRequest(http.MethodPost, "/landscape/slots/lp-z-lcr", body)
req.Header.Set("Content-Type", ct)
rec := httptest.NewRecorder()
h := limitRequestBody(1024)(http.HandlerFunc(newLandscapeTestHandler().uploadSlot))
h.ServeHTTP(rec, req)
if rec.Code != http.StatusRequestEntityTooLarge {
t.Fatalf("status = %d, want %d (body %s)", rec.Code, http.StatusRequestEntityTooLarge, rec.Body.String())
}
if !strings.Contains(rec.Body.String(), "1024 byte limit") {
t.Fatalf("body %s does not state the limit", rec.Body.String())
}
}
func TestLimitRequestBodyPassesSmallBody(t *testing.T) {
var got []byte
h := limitRequestBody(1024)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var err error
got, err = io.ReadAll(r.Body)
if err != nil {
t.Errorf("read: %v", err)
}
}))
h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/", strings.NewReader("hello")))
if string(got) != "hello" {
t.Fatalf("body = %q, want hello", got)
}
}
func TestLimitRequestBodyDisabled(t *testing.T) {
h := limitRequestBody(0)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if _, err := io.ReadAll(r.Body); err != nil {
t.Errorf("read with cap disabled: %v", err)
}
}))
h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(make([]byte, 1<<20))))
}
func newTestRouter() http.Handler {
v := validator.New(validator.WithRequiredStructEnabled())
return NewRouter(RouterDeps{
Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
Health: NewHealthHandler(nil, nil),
Categories: NewCategoryHandler(nil, v),
Datasets: NewDatasetHandler(nil, v),
DatasetGroups: NewDatasetGroupHandler(nil, v),
Landscape: NewLandscapeHandler(nil, v),
MaxBodyBytes: 1024,
})
}
func TestRouterSetsNosniff(t *testing.T) {
rec := httptest.NewRecorder()
newTestRouter().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/healthz", nil))
if got := rec.Header().Get("X-Content-Type-Options"); got != "nosniff" {
t.Fatalf("X-Content-Type-Options = %q, want nosniff", got)
}
}
func TestRouterCapsUploadBody(t *testing.T) {
body, ct := multipartBody(t, 4096)
req := httptest.NewRequest(http.MethodPost, "/landscape/slots/lp-z-lcr", body)
req.Header.Set("Content-Type", ct)
rec := httptest.NewRecorder()
newTestRouter().ServeHTTP(rec, req)
if rec.Code != http.StatusRequestEntityTooLarge {
t.Fatalf("status = %d, want %d (body %s)", rec.Code, http.StatusRequestEntityTooLarge, rec.Body.String())
}
}
func TestReadyDoesNotLeakCheckErrors(t *testing.T) {
h := NewHealthHandler(map[string]ReadinessCheck{
"postgres": func(context.Context) error {
return errors.New("dial tcp 10.1.2.3:5432: connect: connection refused (user=gis)")
},
"s3": func(context.Context) error { return nil },
}, slog.New(slog.NewTextHandler(io.Discard, nil)))
rec := httptest.NewRecorder()
h.Ready(rec, httptest.NewRequest(http.MethodGet, "/readyz", nil))
if rec.Code != http.StatusServiceUnavailable {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusServiceUnavailable)
}
body := rec.Body.String()
if strings.Contains(body, "10.1.2.3") || strings.Contains(body, "dial tcp") {
t.Fatalf("readiness body leaks internal error details: %s", body)
}
if !strings.Contains(body, `"postgres":"unavailable"`) || !strings.Contains(body, `"s3":"ok"`) {
t.Fatalf("unexpected readiness body: %s", body)
}
}

View File

@@ -19,16 +19,23 @@ type RouterDeps struct {
// Landscape serves the /landscape routes.
Landscape *LandscapeHandler
OpenAPISpec []byte
// MaxBodyBytes caps every request body (uploads included); <= 0 disables
// the cap.
MaxBodyBytes int64
}
// NewRouter builds the application's HTTP handler with middleware and routes.
func NewRouter(deps RouterDeps) http.Handler {
r := chi.NewRouter()
// middleware.RealIP is deliberately not used: it rewrites r.RemoteAddr
// from client-controlled headers (X-Forwarded-For, X-Real-IP,
// True-Client-IP) and is deprecated upstream as an IP-spoofing vector.
r.Use(middleware.RequestID)
r.Use(middleware.RealIP)
r.Use(requestLogger(deps.Logger))
r.Use(middleware.Recoverer)
r.Use(securityHeaders)
r.Use(limitRequestBody(deps.MaxBodyBytes))
// Health/readiness endpoints.
r.Get("/healthz", deps.Health.Live)