feat: replace OPA allow rule with deny-reasons protocol

README.md

@@ -51,7 +51,7 @@ opa {
 }
 ```
 
-Policies must be written under `package cachew.authz` and define a boolean `allow` rule. The input document available to policies contains:
+Policies must be written under `package cachew.authz` and define a `deny` rule that collects human-readable reason strings. If the deny set is empty the request is allowed; otherwise it is rejected and the reasons are included in the response body and server logs. The input document available to policies contains:
 
 | Field | Type | Description |
 |---|---|---|
@@ -63,7 +63,16 @@ Policies must be written under `package cachew.authz` and define a boolean `allo
 Since `remote_addr` includes the port, use `startswith` to match by IP:
 
 ```rego
-allow if startswith(input.remote_addr, "127.0.0.1:")
+deny contains "remote address not allowed" if not startswith(input.remote_addr, "127.0.0.1:")
+```
+
+Example policy that requires authentication and blocks writes:
+
+```rego
+package cachew.authz
+deny contains "unauthenticated" if not input.headers["authorization"]
+deny contains "writes are not allowed" if input.method == "PUT"
+deny contains "deletes are not allowed" if input.method == "DELETE"
 ```
 
 Policies can reference external data that becomes available as `data.*` in Rego. Provide it inline via `data` or from a file via `data-file`:
@@ -90,8 +99,7 @@ opa {
 
 ```rego
 package cachew.authz
-default allow := false
-allow if net.cidr_contains(data.allowed_cidrs[_], input.remote_addr)
+deny contains "address not in allowed CIDR" if not net.cidr_contains(data.allowed_cidrs[_], input.remote_addr)
 ```
 
 If `data-file` is not set, `data.*` is empty but policies can still use `http.send` to fetch data at evaluation time.

internal/opa/opa.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"os"
+	"sort"
 	"strings"
 
 	"github.com/alecthomas/errors"
@@ -14,14 +15,11 @@ import (
 	"github.com/block/cachew/internal/logging"
 )
 
-// DefaultPolicy allows only GET and HEAD requests.
+// DefaultPolicy allows only GET and HEAD requests from localhost.
 const DefaultPolicy = `package cachew.authz
 
-default allow := false
-
-allow if input.method == "GET"
-allow if input.method == "HEAD"
-allow if startswith(input.remote_addr, "127.0.0.1:")
+deny contains "method not allowed" if not input.method in {"GET", "HEAD"}
+deny contains "remote address not allowed" if not startswith(input.remote_addr, "127.0.0.1:")
 `
 
 // Config for OPA policy evaluation. If neither Policy nor PolicyFile is set,
@@ -34,49 +32,99 @@ type Config struct {
 }
 
 // Middleware returns an http.Handler that evaluates OPA policy before delegating to next.
-// The policy must define a boolean "allow" rule under package cachew.authz.
+// The policy must define a set "deny" rule under package cachew.authz whose elements
+// are human-readable reason strings (e.g. `deny contains "unauthenticated" if ...`).
+// If the deny set is empty, the request is allowed. Otherwise it is rejected and
+// the reasons are included in the response body and server logs.
 func Middleware(ctx context.Context, cfg Config, next http.Handler) (http.Handler, error) {
 	policy, err := loadPolicy(cfg)
 	if err != nil {
 		return nil, err
 	}
 
-	opts := []func(*rego.Rego){
-		rego.Query("data.cachew.authz.allow"),
-		rego.Module("policy.rego", policy),
-	}
-
-	if cfg.Data != "" || cfg.DataFile != "" {
-		opaData, err := loadData(cfg)
-		if err != nil {
-			return nil, err
-		}
-		opts = append(opts, rego.Data(opaData))
+	dataOpts, err := dataOptions(cfg)
+	if err != nil {
+		return nil, err
 	}
 
-	prepared, err := rego.New(opts...).PrepareForEval(ctx)
+	prepared, err := prepareQuery(ctx, "data.cachew.authz.deny", policy, dataOpts)
 	if err != nil {
-		return nil, errors.Errorf("compile OPA policy: %w", err)
+		return nil, errors.Errorf("compile OPA deny query: %w", err)
 	}
 
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		input := buildInput(r)
 		logger := logging.FromContext(r.Context())
-		results, err := prepared.Eval(r.Context(), rego.EvalInput(input))
+
+		reasons, err := evalDeny(r.Context(), prepared, input)
 		if err != nil {
 			logger.Error("OPA evaluation failed", "error", err)
 			http.Error(w, "policy evaluation error", http.StatusInternalServerError)
 			return
 		}
-		if !results.Allowed() {
-			logger.Warn("OPA denied request", "method", r.Method, "path", r.URL.Path, "remote_addr", r.RemoteAddr)
-			http.Error(w, "forbidden", http.StatusForbidden)
+		if len(reasons) > 0 {
+			logger.Warn("OPA denied request", "method", r.Method, "path", r.URL.Path, "remote_addr", r.RemoteAddr, "reasons", reasons)
+			http.Error(w, "forbidden: "+strings.Join(reasons, "; "), http.StatusForbidden)
 			return
 		}
+
 		next.ServeHTTP(w, r)
 	}), nil
 }
 
+// prepareQuery compiles a single Rego query against the given policy and data options.
+func prepareQuery(ctx context.Context, query, policy string, dataOpts []func(*rego.Rego)) (rego.PreparedEvalQuery, error) {
+	opts := make([]func(*rego.Rego), 0, 2+len(dataOpts))
+	opts = append(opts, rego.Query(query), rego.Module("policy.rego", policy))
+	opts = append(opts, dataOpts...)
+	prepared, err := rego.New(opts...).PrepareForEval(ctx)
+	if err != nil {
+		return prepared, errors.Errorf("prepare query: %w", err)
+	}
+	return prepared, nil
+}
+
+// dataOptions returns rego options for loading external data, if configured.
+func dataOptions(cfg Config) ([]func(*rego.Rego), error) {
+	if cfg.Data == "" && cfg.DataFile == "" {
+		return nil, nil
+	}
+	opaData, err := loadData(cfg)
+	if err != nil {
+		return nil, err
+	}
+	return []func(*rego.Rego){rego.Data(opaData)}, nil
+}
+
+// evalDeny evaluates the prepared deny query and returns any denial reason strings.
+// If the policy produces no deny reasons, nil is returned.
+func evalDeny(ctx context.Context, prepared rego.PreparedEvalQuery, input map[string]any) ([]string, error) {
+	results, err := prepared.Eval(ctx, rego.EvalInput(input))
+	if err != nil {
+		return nil, errors.Errorf("evaluate deny query: %w", err)
+	}
+	if len(results) == 0 || len(results[0].Expressions) == 0 {
+		return nil, nil
+	}
+	val := results[0].Expressions[0].Value
+	// OPA represents sets as []interface{} in the Go bindings.
+	set, isSet := val.([]any)
+	if !isSet {
+		return nil, nil
+	}
+	if len(set) == 0 {
+		return nil, nil
+	}
+	reasons := make([]string, 0, len(set))
+	for _, v := range set {
+		if s, isString := v.(string); isString {
+			reasons = append(reasons, s)
+		}
+	}
+	sort.Strings(reasons) // deterministic order for logging/testing
+	return reasons, nil
+}
+
 func loadPolicy(cfg Config) (string, error) {
 	if cfg.Policy != "" && cfg.PolicyFile != "" {
 		return "", errors.New("OPA config: only one of policy or policy-file may be set")

internal/opa/opa_test.go

@@ -39,17 +39,33 @@ func TestMiddlewareDefaultPolicy(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.Name, func(t *testing.T) {
 			r := newRequest(test.Method, "/some/path")
+			// Default policy requires localhost; httptest uses 192.0.2.1, so
+			// non-localhost requests that are also non-GET/HEAD get two reasons.
+			// Override RemoteAddr so we only test the method rule.
+			r.RemoteAddr = "127.0.0.1:12345"
 			w := httptest.NewRecorder()
 			handler.ServeHTTP(w, r)
 			assert.Equal(t, test.ExpectedStatus, w.Code)
 		})
 	}
 }
 
+func TestMiddlewareDefaultPolicyDeniesNonLocalhost(t *testing.T) {
+	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
+	handler, err := opa.Middleware(t.Context(), opa.Config{}, next)
+	assert.NoError(t, err)
+
+	r := newRequest(http.MethodGet, "/some/path")
+	r.RemoteAddr = "10.0.0.1:9999"
+	w := httptest.NewRecorder()
+	handler.ServeHTTP(w, r)
+	assert.Equal(t, http.StatusForbidden, w.Code)
+	assert.Contains(t, w.Body.String(), "remote address not allowed")
+}
+
 func TestMiddlewareInlinePolicy(t *testing.T) {
 	policy := `package cachew.authz
-default allow := false
-allow if input.method == "POST"
+deny contains "only POST allowed" if input.method != "POST"
 `
 	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
 	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
@@ -75,8 +91,7 @@ allow if input.method == "POST"
 
 func TestMiddlewarePolicyFile(t *testing.T) {
 	policy := `package cachew.authz
-default allow := false
-allow if input.path[0] == "public"
+deny contains "private path" if input.path[0] != "public"
 `
 	dir := t.TempDir()
 	path := filepath.Join(dir, "policy.rego")
@@ -106,9 +121,10 @@ allow if input.path[0] == "public"
 
 func TestMiddlewarePathBasedPolicy(t *testing.T) {
 	policy := `package cachew.authz
-default allow := false
-allow if input.path[0] == "api"
-allow if input.path[0] == "_liveness"
+deny contains "path not allowed" if {
+	not input.path[0] == "api"
+	not input.path[0] == "_liveness"
+}
 `
 	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
 	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
@@ -135,8 +151,7 @@ allow if input.path[0] == "_liveness"
 
 func TestMiddlewareInlineData(t *testing.T) {
 	policy := `package cachew.authz
-default allow := false
-allow if data.allowed_methods[input.method]
+deny contains "method not in allowed set" if not data.allowed_methods[input.method]
 `
 	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
 	handler, err := opa.Middleware(t.Context(), opa.Config{
@@ -178,8 +193,7 @@ func TestMiddlewareInlineDataInvalidJSON(t *testing.T) {
 
 func TestMiddlewareDataFile(t *testing.T) {
 	policy := `package cachew.authz
-default allow := false
-allow if data.allowed_methods[input.method]
+deny contains "method not in allowed set" if not data.allowed_methods[input.method]
 `
 	dataJSON := `{"allowed_methods": {"POST": true, "PUT": true}}`
 
@@ -246,3 +260,91 @@ func TestMiddlewareInvalidPolicy(t *testing.T) {
 	_, err := opa.Middleware(t.Context(), opa.Config{Policy: "not valid rego {"}, next)
 	assert.Error(t, err)
 }
+
+func TestMiddlewareDenyReasons(t *testing.T) {
+	policy := `package cachew.authz
+deny contains "writes are not allowed" if input.method == "PUT"
+deny contains "deletes are not allowed" if input.method == "DELETE"
+`
+	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
+	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
+	assert.NoError(t, err)
+
+	tests := []struct {
+		Name           string
+		Method         string
+		ExpectedStatus int
+		ExpectedBody   string
+	}{
+		{"GETAllowed", http.MethodGet, http.StatusOK, ""},
+		{"PUTDenied", http.MethodPut, http.StatusForbidden, "forbidden: writes are not allowed\n"},
+		{"DELETEDenied", http.MethodDelete, http.StatusForbidden, "forbidden: deletes are not allowed\n"},
+	}
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {
+			r := newRequest(test.Method, "/")
+			w := httptest.NewRecorder()
+			handler.ServeHTTP(w, r)
+			assert.Equal(t, test.ExpectedStatus, w.Code)
+			if test.ExpectedBody != "" {
+				assert.Equal(t, test.ExpectedBody, w.Body.String())
+			}
+		})
+	}
+}
+
+func TestMiddlewareDenyUnauthenticated(t *testing.T) {
+	policy := `package cachew.authz
+deny contains "unauthenticated" if not input.headers["authorization"]
+`
+	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
+	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
+	assert.NoError(t, err)
+
+	// Without Authorization header: denied.
+	r := newRequest(http.MethodGet, "/")
+	w := httptest.NewRecorder()
+	handler.ServeHTTP(w, r)
+	assert.Equal(t, http.StatusForbidden, w.Code)
+	assert.Equal(t, "forbidden: unauthenticated\n", w.Body.String())
+
+	// With Authorization header: allowed.
+	r = newRequest(http.MethodGet, "/")
+	r.Header.Set("Authorization", "Bearer token")
+	w = httptest.NewRecorder()
+	handler.ServeHTTP(w, r)
+	assert.Equal(t, http.StatusOK, w.Code)
+}
+
+func TestMiddlewareDenyMultipleReasons(t *testing.T) {
+	policy := `package cachew.authz
+deny contains "reason-a" if input.method == "POST"
+deny contains "reason-b" if input.method == "POST"
+`
+	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
+	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
+	assert.NoError(t, err)
+
+	r := newRequest(http.MethodPost, "/")
+	w := httptest.NewRecorder()
+	handler.ServeHTTP(w, r)
+	assert.Equal(t, http.StatusForbidden, w.Code)
+	// Reasons are sorted deterministically.
+	assert.Equal(t, "forbidden: reason-a; reason-b\n", w.Body.String())
+}
+
+func TestMiddlewareEmptyDenyAllowsAll(t *testing.T) {
+	// A policy with no deny rules allows everything.
+	policy := `package cachew.authz
+`
+	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
+	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
+	assert.NoError(t, err)
+
+	for _, method := range []string{http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete} {
+		r := newRequest(method, "/any/path")
+		w := httptest.NewRecorder()
+		handler.ServeHTTP(w, r)
+		assert.Equal(t, http.StatusOK, w.Code)
+	}
+}