diff --git a/internal/opa/opa.go b/internal/opa/opa.go index 60786da4..1e797255 100644 --- a/internal/opa/opa.go +++ b/internal/opa/opa.go @@ -67,6 +67,12 @@ func Middleware(ctx context.Context, cfg Config, next http.Handler) (http.Handle } if !allowed { logger.Warn("OPA denied request", "method", r.Method, "path", r.URL.Path, "remote_addr", r.RemoteAddr) + // The unread body is still on the wire, so the connection can't be reused. Saying so + // up front also stops net/http reading up to 256KiB of the body before replying, which + // it does because otelhttp replaces r.Body and hides the unread length from it. + if r.ContentLength != 0 { + w.Header().Set("Connection", "close") + } http.Error(w, "forbidden", http.StatusForbidden) return } diff --git a/internal/opa/opa_test.go b/internal/opa/opa_test.go index 065b2e49..8ade95d2 100644 --- a/internal/opa/opa_test.go +++ b/internal/opa/opa_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/alecthomas/assert/v2" @@ -365,6 +366,27 @@ test_delete_allowed if authz.allow with input as {"method": "DELETE"} } } +func TestMiddlewareDeniedUploadClosesConnection(t *testing.T) { + policy := `package cachew.authz +default allow := false +` + next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}) + handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next) + assert.NoError(t, err) + + upload := httptest.NewRequest(http.MethodPost, "/api/v1/object/ns/key", strings.NewReader("data")) + upload = upload.WithContext(logging.ContextWithLogger(upload.Context(), slog.Default())) + w := httptest.NewRecorder() + handler.ServeHTTP(w, upload) + assert.Equal(t, http.StatusForbidden, w.Code) + assert.Equal(t, "close", w.Header().Get("Connection")) + + w = httptest.NewRecorder() + handler.ServeHTTP(w, newRequest(http.MethodGet, "/api/v1/object/ns/key")) + assert.Equal(t, http.StatusForbidden, w.Code) + assert.Equal(t, "", w.Header().Get("Connection")) +} + func TestMiddlewareEmptyPolicyDeniesAll(t *testing.T) { policy := `package cachew.authz `