fix!(policy/checker): make List and-like (#1217)
* fix!(policy/checker): make List and-like This has the potential to break user configs. Anubis lets you stack multiple checks at once with blocks like this: ```yaml name: allow-prometheus action: ALLOW user_agent_regex: ^prometheus-probe$ remote_addresses: - 192.168.2.0/24 ``` Previously, this only returned ALLOW if _any one_ of the conditions matched. This behaviour has changed to only return ALLOW if _all_ of the conditions match. I have marked this as a potentially breaking change because I'm absolutely certain that someone is relying on this behaviour due to spacebar heating. If this bites you, please let me know ASAP. Signed-off-by: Xe Iaso <me@xeiaso.net> Assisted-by: GPT-OSS 120b on local hardware * fix(policy/checker): more explicit short-circuit Signed-off-by: Xe Iaso <me@xeiaso.net> --------- Signed-off-by: Xe Iaso <me@xeiaso.net>
This commit is contained in:
parent
d7459de941
commit
6b1cd6120f
3 changed files with 84 additions and 5 deletions
|
|
@ -16,18 +16,24 @@ type Impl interface {
|
|||
|
||||
type List []Impl
|
||||
|
||||
// Check runs each checker in the list against the request.
|
||||
// It returns true only if *all* checkers return true (AND semantics).
|
||||
// If any checker returns an error, the function returns false and the error.
|
||||
func (l List) Check(r *http.Request) (bool, error) {
|
||||
for _, c := range l {
|
||||
ok, err := c.Check(r)
|
||||
if err != nil {
|
||||
return ok, err
|
||||
// Propagate the error; overall result is false.
|
||||
return false, err
|
||||
}
|
||||
if ok {
|
||||
return ok, nil
|
||||
if !ok {
|
||||
// One false means the combined result is false. Short-circuit
|
||||
// so we don't waste time.
|
||||
return false, err
|
||||
}
|
||||
}
|
||||
|
||||
return false, nil
|
||||
// Assume success until a checker says otherwise.
|
||||
return true, nil
|
||||
}
|
||||
|
||||
func (l List) Hash() string {
|
||||
|
|
|
|||
57
lib/policy/checker/checker_test.go
Normal file
57
lib/policy/checker/checker_test.go
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
package checker
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"net/http"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// Mock implements the Impl interface for testing.
|
||||
type Mock struct {
|
||||
result bool
|
||||
err error
|
||||
hash string
|
||||
}
|
||||
|
||||
func (m Mock) Check(r *http.Request) (bool, error) { return m.result, m.err }
|
||||
func (m Mock) Hash() string { return m.hash }
|
||||
|
||||
func TestListCheck_AndSemantics(t *testing.T) {
|
||||
req, _ := http.NewRequest(http.MethodGet, "http://example.com", nil)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
list List
|
||||
want bool
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "all true",
|
||||
list: List{Mock{true, nil, "a"}, Mock{true, nil, "b"}},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "one false",
|
||||
list: List{Mock{true, nil, "a"}, Mock{false, nil, "b"}},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "error propagates",
|
||||
list: List{Mock{true, nil, "a"}, Mock{true, errors.New("boom"), "b"}},
|
||||
want: false,
|
||||
wantErr: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got, err := tt.list.Check(req)
|
||||
if (err != nil) != tt.wantErr {
|
||||
t.Fatalf("unexpected error state: %v", err)
|
||||
}
|
||||
if got != tt.want {
|
||||
t.Fatalf("expected %v, got %v", tt.want, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue