From e37b18aa5079765fe91a3583e80b4fd08ae6b5d2 Mon Sep 17 00:00:00 2001 From: Alec Thomas Date: Fri, 20 Mar 2026 22:57:20 +1100 Subject: [PATCH] feat: replace OPA allow rule with deny-reasons protocol --- README.md | 16 +++-- internal/opa/opa.go | 96 ++++++++++++++++++++++-------- internal/opa/opa_test.go | 124 +++++++++++++++++++++++++++++++++++---- 3 files changed, 197 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index b750236..a9688fb 100644 --- a/README.md +++ b/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. diff --git a/internal/opa/opa.go b/internal/opa/opa.go index 5f82e8d..ce19b49 100644 --- a/internal/opa/opa.go +++ b/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") diff --git a/internal/opa/opa_test.go b/internal/opa/opa_test.go index c88f23b..5e24bbd 100644 --- a/internal/opa/opa_test.go +++ b/internal/opa/opa_test.go @@ -39,6 +39,10 @@ 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) @@ -46,10 +50,22 @@ func TestMiddlewareDefaultPolicy(t *testing.T) { } } +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) + } +}