mirror of
https://github.com/warmbly/warmbly.git
synced 2026-10-19 16:03:14 +00:00
Merge pull request #853 from warmbly/devin/1791288149-api-response-no-store
Harden API query validation, private caching and dashboard CORS
This commit is contained in:
@@ -5,6 +5,8 @@ description: Learn how to authenticate with the Warmbly API using API keys.
|
||||
|
||||
The Warmbly API uses API keys for authentication. Each API key has specific permissions that control what operations it can perform.
|
||||
|
||||
API responses use `Cache-Control: no-store` by default to instruct browsers and intermediaries not to retain private data. Public uploaded assets keep their explicit caching policy.
|
||||
|
||||
## API key format
|
||||
|
||||
API keys follow this format:
|
||||
|
||||
@@ -80,6 +80,8 @@ When the body cannot be read, the `message` says why:
|
||||
|
||||
Fields are named by their JSON key, with nested fields as a dotted path (`inner.name`). The `code` is `bad_request` in every case, so branch on `code` and show or log `message`.
|
||||
|
||||
Versioned API routes check every query parameter name and value before the endpoint runs, including endpoints that read filters directly. The legacy `/invitations/lookup` alias applies the same validation. NUL characters, invalid UTF-8 and malformed query encoding return `400` rather than sending unsupported text to the database. For example, `?q=%00` returns `Query parameter "q[0]" contains a NUL character, which is not supported.` Valid Unicode text and properly encoded punctuation remain unchanged.
|
||||
|
||||
**How to fix:**
|
||||
- Check that your JSON is valid
|
||||
- Verify all required fields are present
|
||||
|
||||
@@ -491,6 +491,8 @@ Both dashboards point `WARMBLY_API_URL` at the same API and set `WARMBLY_APP_URL
|
||||
|
||||
The customer dashboard also generates a Content Security Policy from the same deployment settings. Static bootstrap scripts and styles use build-generated hashes; scripts and styles injected as markup are not generally allowed. API, Sentry and PostHog origins are derived from their configured URLs. A realtime service on a different origin needs that exact origin in `WARMBLY_CSP_CONNECT_ORIGINS`, for example `wss://realtime.example.com`. Changing these settings requires a container restart or a Pages rebuild. Pages remains fully static and needs no Function.
|
||||
|
||||
The Pages dashboard removes Cloudflare's default `Access-Control-Allow-Origin: *` header from its static responses. Dashboard assets are not a cross-origin public API. This does not change the backend's `CORS_ALLOW_ORIGINS` or the public image URLs used in customer emails.
|
||||
|
||||
Email bodies render in a separate static, script-disabled sandbox. That document permits email styling and the reader's remote-content choice without relaxing the dashboard policy. Dashboard images load from its own origin, the configured API, data URLs or temporary blob URLs. External UI images and editor previews use the backend's image-preview endpoint, which allows HTTPS public destinations only, checks every connection and redirect, and caps images at 5 MiB and requests at ten seconds. Owned public images are read through the configured blob store, so filesystem storage and private MinIO endpoints need no public-network exception. The unsafe-webhook development flag does not relax image-fetching restrictions.
|
||||
|
||||
This proxy is display-only. Existing records need no migration, and saved and sent email HTML retains its original image URLs, links and styles. Recipients do not use the dashboard proxy or need a session to load email images. Incoming messages keep their separate remote-content consent and do not use this proxy. Dashboard inline style attributes remain permitted for editor-generated email designs; inline scripts and arbitrary style blocks do not. Development uses fresh per-response nonces for Vite's script and stylesheet injection.
|
||||
|
||||
@@ -313,7 +313,7 @@ func (h *Handler) RevokeAIToolPolicy(c *gin.Context) {
|
||||
func sseEmitter(c *gin.Context) func(aiagent.StreamEvent) {
|
||||
h := c.Writer.Header()
|
||||
h.Set("Content-Type", "text/event-stream")
|
||||
h.Set("Cache-Control", "no-cache")
|
||||
h.Set("Cache-Control", "no-cache, no-store")
|
||||
h.Set("Connection", "keep-alive")
|
||||
h.Set("X-Accel-Buffering", "no") // disable proxy buffering so deltas flush
|
||||
c.Writer.WriteHeader(http.StatusOK)
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
func TestSSEEmitterDoesNotStoreEvents(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest(http.MethodPost, "/v1/ai/agent", nil)
|
||||
sseEmitter(c)
|
||||
if got := w.Header().Get("Cache-Control"); got != "no-cache, no-store" {
|
||||
t.Errorf("Cache-Control = %q, want no-cache, no-store", got)
|
||||
}
|
||||
if !w.Flushed {
|
||||
t.Error("event stream should flush immediately")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,23 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/warmbly/warmbly/internal/errx"
|
||||
)
|
||||
|
||||
func QueryValidation() gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
values, err := url.ParseQuery(c.Request.URL.RawQuery)
|
||||
if err == nil {
|
||||
err = errx.ValidateText(values)
|
||||
}
|
||||
if err != nil {
|
||||
c.Abort()
|
||||
errx.JSON(c, errx.InvalidQuery(err))
|
||||
return
|
||||
}
|
||||
c.Next()
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,116 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-contrib/cors"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
func TestQueryValidationRejectsUnsupportedText(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
for _, query := range []string{"q=%00", "q=valid&q=%00", "%00=value", "cursor=%00", "folder=%00", "tag=%00", "q=%FF", "%FF=value", "q=%ZZ", "q=unescaped;semicolon"} {
|
||||
t.Run(query, func(t *testing.T) {
|
||||
called := false
|
||||
r := gin.New()
|
||||
r.Use(SecurityHeaders(), RequestIDMiddleware(), cors.New(cors.Config{AllowOrigins: []string{"https://dashboard.example.com"}, AllowMethods: []string{http.MethodGet}}), QueryValidation())
|
||||
r.GET("/v1/campaigns", func(c *gin.Context) {
|
||||
called = true
|
||||
c.Status(http.StatusOK)
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/v1/campaigns?"+query, nil)
|
||||
req.Header.Set("Origin", "https://dashboard.example.com")
|
||||
r.ServeHTTP(w, req)
|
||||
if called || w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("called = %t, status = %d; want false, 400", called, w.Code)
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), `"code":"bad_request"`) || !strings.Contains(w.Body.String(), `"request_id":"`) {
|
||||
t.Errorf("missing structured error or request ID: %s", w.Body.String())
|
||||
}
|
||||
if w.Header().Get("Cache-Control") != "no-store" {
|
||||
t.Error("invalid queries must retain cache protection")
|
||||
}
|
||||
if w.Header().Get("Access-Control-Allow-Origin") != "https://dashboard.example.com" {
|
||||
t.Error("validation errors must be readable by the configured dashboard origin")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestQueryValidationPreservesValidQuery(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
for _, query := range []string{"", "q=security&limit=5", "q=%27%22%3Cscript%3E", "q=hello%3Bworld", "q=%E4%BD%A0%E5%A5%BD", "q=one&q=two"} {
|
||||
t.Run(query, func(t *testing.T) {
|
||||
r := gin.New()
|
||||
r.Use(QueryValidation())
|
||||
r.GET("/v1/emails", func(c *gin.Context) {
|
||||
if c.Request.URL.RawQuery != query {
|
||||
t.Errorf("query was changed to %q", c.Request.URL.RawQuery)
|
||||
}
|
||||
c.Status(http.StatusOK)
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/v1/emails?"+query, nil))
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("valid query returned %d", w.Code)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestQueryValidationProtectsLegacyInvitationLookup(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
token := strings.Repeat("a", 64)
|
||||
for _, tc := range []struct {
|
||||
query string
|
||||
valid bool
|
||||
}{
|
||||
{"token=" + token, true},
|
||||
{"token=" + token + "&ref=%E4%BD%A0%E5%A5%BD", true},
|
||||
{"token=%00", false},
|
||||
{"token=%FF", false},
|
||||
{"token=%ZZ", false},
|
||||
{"%00=value", false},
|
||||
{"token=" + token + "&unused=%00", false},
|
||||
} {
|
||||
t.Run(tc.query, func(t *testing.T) {
|
||||
rateLimiterCalled, handlerCalled := false, false
|
||||
r := gin.New()
|
||||
r.Use(SecurityHeaders(), RequestIDMiddleware())
|
||||
r.GET("/invitations/lookup", QueryValidation(), func(c *gin.Context) {
|
||||
rateLimiterCalled = true
|
||||
c.Next()
|
||||
}, func(c *gin.Context) {
|
||||
handlerCalled = true
|
||||
if c.Query("token") != token || c.Request.URL.RawQuery != tc.query {
|
||||
t.Error("valid invitation query was changed")
|
||||
}
|
||||
c.Status(http.StatusOK)
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/invitations/lookup?"+tc.query, nil)
|
||||
req.Header.Set("Origin", "https://dashboard.example.com")
|
||||
r.ServeHTTP(w, req)
|
||||
wantStatus := http.StatusBadRequest
|
||||
if tc.valid {
|
||||
wantStatus = http.StatusOK
|
||||
}
|
||||
if w.Code != wantStatus || rateLimiterCalled != tc.valid || handlerCalled != tc.valid {
|
||||
t.Fatalf("status = %d, limiter = %t, handler = %t; want %d, %t, %t", w.Code, rateLimiterCalled, handlerCalled, wantStatus, tc.valid, tc.valid)
|
||||
}
|
||||
if !tc.valid && (!strings.Contains(w.Body.String(), `"code":"bad_request"`) || !strings.Contains(w.Body.String(), `"request_id":"`)) {
|
||||
t.Errorf("missing structured validation error: %s", w.Body.String())
|
||||
}
|
||||
if w.Header().Get("Cache-Control") != "no-store" || w.Header().Get("X-Request-Id") == "" {
|
||||
t.Error("invitation responses must retain cache protection and request IDs")
|
||||
}
|
||||
if w.Header().Get("Access-Control-Allow-Origin") != "" {
|
||||
t.Error("legacy invitation validation must not change CORS behavior")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -31,6 +31,7 @@ func SecurityHeaders() gin.HandlerFunc {
|
||||
h.Set("Cross-Origin-Opener-Policy", "same-origin")
|
||||
h.Set("Cross-Origin-Resource-Policy", "same-site")
|
||||
h.Set("Permissions-Policy", "camera=(), microphone=(), geolocation=(), interest-cohort=()")
|
||||
h.Set("Cache-Control", "no-store")
|
||||
|
||||
// default-src 'none' suits a JSON API. The handful of routes that
|
||||
// return HTML (the OAuth bouncers, the unsubscribe page) set their own
|
||||
|
||||
@@ -18,6 +18,7 @@ func TestSecurityHeadersAreSet(t *testing.T) {
|
||||
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/x", nil))
|
||||
|
||||
want := map[string]string{
|
||||
"Cache-Control": "no-store",
|
||||
"X-Content-Type-Options": "nosniff",
|
||||
"X-Frame-Options": "DENY",
|
||||
"Referrer-Policy": "strict-origin-when-cross-origin",
|
||||
@@ -34,6 +35,37 @@ func TestSecurityHeadersAreSet(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSecurityHeadersPreventCachingErrors(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
for _, status := range []int{http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound, http.StatusInternalServerError} {
|
||||
t.Run(http.StatusText(status), func(t *testing.T) {
|
||||
r := gin.New()
|
||||
r.Use(SecurityHeaders())
|
||||
r.GET("/x", func(c *gin.Context) { c.AbortWithStatus(status) })
|
||||
w := httptest.NewRecorder()
|
||||
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/x", nil))
|
||||
if got := w.Header().Get("Cache-Control"); got != "no-store" {
|
||||
t.Errorf("Cache-Control = %q, want no-store", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandlerPublicCachePolicyWins(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
r := gin.New()
|
||||
r.Use(SecurityHeaders())
|
||||
r.GET("/public/image.png", func(c *gin.Context) {
|
||||
c.Header("Cache-Control", "public, max-age=31536000, immutable")
|
||||
c.Status(http.StatusOK)
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/public/image.png", nil))
|
||||
if got := w.Header().Get("Cache-Control"); got != "public, max-age=31536000, immutable" {
|
||||
t.Errorf("handler cache policy should survive, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// HSTS over plain HTTP is ignored by browsers and would pin a self-hosted LAN
|
||||
// instance to a scheme it does not serve, so it is only sent on a TLS request.
|
||||
func TestHSTSOnlyOverTLS(t *testing.T) {
|
||||
|
||||
@@ -146,7 +146,7 @@ func Run(
|
||||
|
||||
// Public invitation preview for the /invite landing page. Unauthenticated:
|
||||
// the secret token in the query is the capability.
|
||||
r.GET("/invitations/lookup", m.PublicIPRateLimitMiddleware(), h.PreviewInvitation)
|
||||
r.GET("/invitations/lookup", middleware.QueryValidation(), m.PublicIPRateLimitMiddleware(), h.PreviewInvitation)
|
||||
|
||||
// On-demand TLS gate for the reverse proxy in front of this instance
|
||||
// (Caddy's `ask`). Unauthenticated because the proxy has no credential to
|
||||
@@ -299,6 +299,7 @@ func Run(
|
||||
|
||||
r.Use(cors.New(corsConfig))
|
||||
r.Use(middleware.DashboardOriginMiddleware())
|
||||
r.Use(middleware.QueryValidation())
|
||||
|
||||
// Limit request body size to 10MB to prevent OOM. The contact file uploads
|
||||
// apply their own, larger cap in the handler before reading.
|
||||
|
||||
+19
-6
@@ -9,6 +9,7 @@ import (
|
||||
"reflect"
|
||||
"strings"
|
||||
"time"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/gin-gonic/gin/binding"
|
||||
"github.com/go-playground/validator/v10"
|
||||
@@ -33,7 +34,7 @@ func (v rejectingTextValidator) ValidateStruct(value any) error {
|
||||
if err := v.delegate.ValidateStruct(value); err != nil {
|
||||
return err
|
||||
}
|
||||
return unsupportedText(reflect.ValueOf(value), "")
|
||||
return ValidateText(value)
|
||||
}
|
||||
|
||||
func (v rejectingTextValidator) Engine() any {
|
||||
@@ -41,11 +42,17 @@ func (v rejectingTextValidator) Engine() any {
|
||||
}
|
||||
|
||||
type unsupportedTextError struct {
|
||||
field string
|
||||
field string
|
||||
reason string
|
||||
}
|
||||
|
||||
func (e *unsupportedTextError) Error() string {
|
||||
return fmt.Sprintf("field %q contains a NUL character", e.field)
|
||||
return fmt.Sprintf("field %q %s", e.field, e.reason)
|
||||
}
|
||||
|
||||
// ValidateText rejects text PostgreSQL cannot store in a request value.
|
||||
func ValidateText(value any) error {
|
||||
return unsupportedText(reflect.ValueOf(value), "")
|
||||
}
|
||||
|
||||
func unsupportedText(value reflect.Value, path string) error {
|
||||
@@ -61,11 +68,17 @@ func unsupportedText(value reflect.Value, path string) error {
|
||||
|
||||
switch value.Kind() {
|
||||
case reflect.String:
|
||||
var reason string
|
||||
if strings.ContainsRune(value.String(), '\x00') {
|
||||
reason = "contains a NUL character"
|
||||
} else if !utf8.ValidString(value.String()) {
|
||||
reason = "contains invalid UTF-8"
|
||||
}
|
||||
if reason != "" {
|
||||
if path == "" {
|
||||
path = "value"
|
||||
}
|
||||
return &unsupportedTextError{field: path}
|
||||
return &unsupportedTextError{field: path, reason: reason}
|
||||
}
|
||||
case reflect.Struct:
|
||||
typeOf := value.Type()
|
||||
@@ -157,7 +170,7 @@ func InvalidBody(err error) *Error {
|
||||
case errors.As(err, &timeErr):
|
||||
return New(BadRequest, fmt.Sprintf("The request body has a time that is not RFC 3339 (such as 2026-10-01T09:00:00Z): %q.", strings.Trim(timeErr.Value, `"`)))
|
||||
case errors.As(err, &textErr):
|
||||
return New(BadRequest, fmt.Sprintf("Field %q contains a NUL character, which is not supported.", textErr.field))
|
||||
return New(BadRequest, fmt.Sprintf("Field %q %s, which is not supported.", textErr.field, textErr.reason))
|
||||
case strings.HasPrefix(err.Error(), "invalid UUID"):
|
||||
return New(BadRequest, "The request body has a value that should be a UUID and is not one.")
|
||||
}
|
||||
@@ -171,7 +184,7 @@ func InvalidBody(err error) *Error {
|
||||
func InvalidQuery(err error) *Error {
|
||||
var textErr *unsupportedTextError
|
||||
if errors.As(err, &textErr) {
|
||||
return New(BadRequest, fmt.Sprintf("Query parameter %q contains a NUL character, which is not supported.", textErr.field))
|
||||
return New(BadRequest, fmt.Sprintf("Query parameter %q %s, which is not supported.", textErr.field, textErr.reason))
|
||||
}
|
||||
return New(BadRequest, "The query parameters are invalid. Check them against the API reference.")
|
||||
}
|
||||
|
||||
@@ -104,6 +104,21 @@ func TestInvalidQueryRejectsNUL(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestInvalidQueryRejectsInvalidUTF8(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
c, _ := gin.CreateTestContext(httptest.NewRecorder())
|
||||
c.Request = httptest.NewRequest(http.MethodGet, "/?q=%FF", nil)
|
||||
var query bindQuery
|
||||
err := c.ShouldBindQuery(&query)
|
||||
if err == nil {
|
||||
t.Fatal("binding invalid UTF-8 succeeded")
|
||||
}
|
||||
got := InvalidQuery(err)
|
||||
if got.Code != BadRequest || !strings.Contains(got.Message, `Query parameter "q" contains invalid UTF-8`) {
|
||||
t.Fatalf("InvalidQuery() = %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInvalidBodyCapsTheList(t *testing.T) {
|
||||
type many struct {
|
||||
A string `json:"a" binding:"required"`
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
#
|
||||
# build:pages fills the built policy's hashes and configured resource origins.
|
||||
/*
|
||||
! Access-Control-Allow-Origin
|
||||
X-Content-Type-Options: nosniff
|
||||
X-Frame-Options: DENY
|
||||
Referrer-Policy: strict-origin-when-cross-origin
|
||||
|
||||
@@ -87,6 +87,12 @@ describe("dashboard CSP", () => {
|
||||
expect(readFileSync(path.join(directory, "nginx-mail-preview-headers.conf"), "utf8")).toContain(`Content-Security-Policy "${mailPreviewCsp}"`);
|
||||
});
|
||||
|
||||
it("removes wildcard CORS from dashboard static responses", () => {
|
||||
const headers = readFileSync(path.join(directory, "public/_headers"), "utf8");
|
||||
expect(headers).toContain("/*\n ! Access-Control-Allow-Origin\n");
|
||||
expect(headers).not.toMatch(/^\s+Access-Control-Allow-Origin:\s*\*/m);
|
||||
});
|
||||
|
||||
it("derives exact resource origins and permits separately configured realtime", () => {
|
||||
expect(configuredSources({ VITE_API_URL: "https://api.example.test/v1", VITE_POSTHOG_HOST: "https://eu.i.posthog.com", WARMBLY_CSP_CONNECT_ORIGINS: "wss://realtime.example.test/socket" })).toEqual({
|
||||
connections: ["https://api.example.test", "wss://api.example.test", "wss://realtime.example.test"],
|
||||
@@ -119,6 +125,7 @@ describe("dashboard CSP", () => {
|
||||
expect(policy).not.toContain("__");
|
||||
for (const hash of [...scripts, ...styles]) expect(policy).toContain(hash);
|
||||
const headers = readFileSync(path.join(temporary, "_headers"), "utf8");
|
||||
expect(headers).toContain("/*\n ! Access-Control-Allow-Origin\n");
|
||||
expect(headers).toContain(`Content-Security-Policy: ${policy}`);
|
||||
expect(headers).toContain(`Content-Security-Policy: ${mailPreviewCsp}`);
|
||||
execFileSync("sh", [path.join(directory, "docker-entrypoint.sh")], { env });
|
||||
|
||||
Reference in New Issue
Block a user