From 323e4a69e2c1c21c44e618b93c0ccbd52cc989e6 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Thu, 23 Apr 2026 20:34:13 -0700 Subject: [PATCH 1/4] fix plugin capability definitions --- app/allowlist.go | 12 +- app/allowlist_body_filter_test.go | 15 ++ app/allowlist_test.go | 9 + app/allowlist_validate.go | 3 + app/allowlist_validate_test.go | 33 ++++ .../plugins/monday/capabilities.go | 32 +++- .../plugins/monday/monday_test.go | 35 +++- .../plugins/sendgrid/capabilities.go | 14 +- .../plugins/sendgrid/sendgrid_test.go | 14 +- app/integrations/registry.go | 3 + app/integrations/registry_test.go | 16 ++ app/secrets/plugins/aws/plugin.go | 8 +- cmd/allowlist/main.go | 143 +++++++++++++-- cmd/allowlist/main_test.go | 173 +++++++++++++++--- cmd/allowlist/plugins/asana.go | 7 - .../plugins/dangerously_full_access.go | 5 - cmd/allowlist/plugins/github.go | 7 - cmd/allowlist/plugins/jira.go | 7 - cmd/allowlist/plugins/linear.go | 7 - cmd/allowlist/plugins/monday.go | 7 - cmd/allowlist/plugins/okta.go | 7 - cmd/allowlist/plugins/registry.go | 57 +++++- cmd/allowlist/plugins/registry_test.go | 82 +++++++++ cmd/allowlist/plugins/sendgrid.go | 7 - cmd/allowlist/plugins/servicenow.go | 7 - cmd/allowlist/plugins/slack.go | 7 - cmd/allowlist/plugins/stripe.go | 7 - cmd/allowlist/plugins/twilio.go | 7 - cmd/allowlist/plugins/zendesk.go | 7 - cmd/integrations/plugins/builders_test.go | 15 +- cmd/integrations/plugins/servicenow.go | 16 +- cmd/integrations/plugins/zendesk.go | 16 +- docs/capabilities.md | 10 +- docs/cli.md | 12 +- docs/integration-plugins.md | 5 +- docs/secret-backends.md | 6 +- 36 files changed, 623 insertions(+), 195 deletions(-) delete mode 100644 cmd/allowlist/plugins/asana.go delete mode 100644 cmd/allowlist/plugins/dangerously_full_access.go delete mode 100644 cmd/allowlist/plugins/github.go delete mode 100644 cmd/allowlist/plugins/jira.go delete mode 100644 cmd/allowlist/plugins/linear.go delete mode 100644 cmd/allowlist/plugins/monday.go delete mode 100644 cmd/allowlist/plugins/okta.go delete mode 100644 cmd/allowlist/plugins/sendgrid.go delete mode 100644 cmd/allowlist/plugins/servicenow.go delete mode 100644 cmd/allowlist/plugins/slack.go delete mode 100644 cmd/allowlist/plugins/stripe.go delete mode 100644 cmd/allowlist/plugins/twilio.go delete mode 100644 cmd/allowlist/plugins/zendesk.go diff --git a/app/allowlist.go b/app/allowlist.go index 5b4b5fb3..8c249b82 100644 --- a/app/allowlist.go +++ b/app/allowlist.go @@ -365,7 +365,12 @@ func matchValueReason(data, rule interface{}, path string) (bool, string) { case []interface{}: da, ok := data.([]interface{}) if !ok { - return false, fmt.Sprintf("body field %s not array", path) + for _, want := range rv { + if ok2, _ := matchValueReason(data, want, path); ok2 { + return true, "" + } + } + return false, fmt.Sprintf("body field %s value not in allowed set", path) } for _, want := range rv { found := false @@ -415,6 +420,11 @@ func matchValue(data, rule interface{}) bool { case []interface{}: da, ok := data.([]interface{}) if !ok { + for _, want := range rv { + if matchValue(data, want) { + return true + } + } return false } for _, want := range rv { diff --git a/app/allowlist_body_filter_test.go b/app/allowlist_body_filter_test.go index 67aafa4c..f97c789d 100644 --- a/app/allowlist_body_filter_test.go +++ b/app/allowlist_body_filter_test.go @@ -53,6 +53,21 @@ func TestBodyArrayMatching(t *testing.T) { } } +func TestBodyArrayRuleMatchesScalarAllowedSet(t *testing.T) { + body := []byte(`{"channel":"C123"}`) + rule := map[string]interface{}{"channel": []interface{}{"C123", "C456"}} + r := req(http.MethodPost, body) + if !validateRequest(r, RequestConstraint{Body: rule}) { + t.Fatal("expected scalar body value to match one allowed rule value") + } + + rule = map[string]interface{}{"channel": []interface{}{"C999"}} + r = req(http.MethodPost, body) + if validateRequest(r, RequestConstraint{Body: rule}) { + t.Fatal("expected scalar body value outside allowed set to fail") + } +} + func TestBodyObjectMatching(t *testing.T) { body := []byte(`{"foo":"bar","num":1,"extra":true}`) tests := []struct { diff --git a/app/allowlist_test.go b/app/allowlist_test.go index 2bd9ac8f..9493583b 100644 --- a/app/allowlist_test.go +++ b/app/allowlist_test.go @@ -483,6 +483,15 @@ func TestMatchValueNotOkBranches(t *testing.T) { } } +func TestMatchValueScalarAllowedSet(t *testing.T) { + if !matchValue("C123", []interface{}{"C123", "C456"}) { + t.Fatal("expected scalar value to match one allowed value") + } + if matchValue("C999", []interface{}{"C123", "C456"}) { + t.Fatal("expected scalar value outside allowed set to fail") + } +} + func TestMatchValueReasonNotOkBranches(t *testing.T) { if ok, reason := matchValueReason("not-a-map", map[string]interface{}{"a": 1}, ""); ok { t.Fatalf("expected map type mismatch to fail, got reason: %s", reason) diff --git a/app/allowlist_validate.go b/app/allowlist_validate.go index 1601bc8e..8172640a 100644 --- a/app/allowlist_validate.go +++ b/app/allowlist_validate.go @@ -122,6 +122,9 @@ func validateCapability(integration string, cap integrationplugins.CapabilityCon return fmt.Errorf("unknown param %s for capability %s", p, cap.Name) } } + if spec.Generate == nil { + return fmt.Errorf("capability %s has no rule generator", cap.Name) + } if _, err := spec.Generate(cap.Params); err != nil { return fmt.Errorf("invalid params for capability %s: %v", cap.Name, err) } diff --git a/app/allowlist_validate_test.go b/app/allowlist_validate_test.go index 79e778a7..d2748d38 100644 --- a/app/allowlist_validate_test.go +++ b/app/allowlist_validate_test.go @@ -261,6 +261,39 @@ func TestValidateAllowlistEntriesGlobalCapability(t *testing.T) { } } +func TestValidateAllowlistEntriesCapabilityWithoutGenerator(t *testing.T) { + orig := make(map[string]map[string]integrationplugins.CapabilitySpec) + for integ, caps := range integrationplugins.AllCapabilities() { + m := make(map[string]integrationplugins.CapabilitySpec, len(caps)) + for name, spec := range caps { + m[name] = spec + } + orig[integ] = m + } + t.Cleanup(func() { + reg := integrationplugins.AllCapabilities() + for k := range reg { + delete(reg, k) + } + for k, v := range orig { + reg[k] = v + } + }) + + integrationplugins.RegisterCapability("nogenerator", "cap", integrationplugins.CapabilitySpec{}) + entries := []AllowlistEntry{{ + Integration: "nogenerator", + Callers: []CallerConfig{{ + ID: "c", + Capabilities: []integrationplugins.CapabilityConfig{{Name: "cap"}}, + }}, + }} + err := validateAllowlistEntries(entries) + if err == nil || !strings.Contains(err.Error(), "has no rule generator") { + t.Fatalf("expected no rule generator error, got %v", err) + } +} + func TestCopyAllowlistCallersSkipsEmptyMethod(t *testing.T) { callers := []CallerConfig{{ ID: "c", diff --git a/app/integrations/plugins/monday/capabilities.go b/app/integrations/plugins/monday/capabilities.go index cd8e5954..a83aeb1d 100644 --- a/app/integrations/plugins/monday/capabilities.go +++ b/app/integrations/plugins/monday/capabilities.go @@ -2,25 +2,45 @@ package monday import integrationplugins "github.com/winhowes/AuthTranslator/app/integrations" +func operationName(p map[string]interface{}, fallback string) string { + if p == nil { + return fallback + } + name, _ := p["operationName"].(string) + if name == "" { + return fallback + } + return name +} + +func operationRule(name string) integrationplugins.CallRule { + return integrationplugins.CallRule{ + Path: "/v2", + Methods: map[string]integrationplugins.RequestConstraint{ + "POST": {Body: map[string]interface{}{"operationName": name}}, + }, + } +} + func init() { integrationplugins.RegisterCapability("monday", "create_item", integrationplugins.CapabilitySpec{ + Params: []string{"operationName"}, Generate: func(p map[string]interface{}) ([]integrationplugins.CallRule, error) { - rule := integrationplugins.CallRule{Path: "/v2", Methods: map[string]integrationplugins.RequestConstraint{"POST": {}}} - return []integrationplugins.CallRule{rule}, nil + return []integrationplugins.CallRule{operationRule(operationName(p, "create_item"))}, nil }, }) integrationplugins.RegisterCapability("monday", "update_status", integrationplugins.CapabilitySpec{ + Params: []string{"operationName"}, Generate: func(p map[string]interface{}) ([]integrationplugins.CallRule, error) { - rule := integrationplugins.CallRule{Path: "/v2", Methods: map[string]integrationplugins.RequestConstraint{"POST": {}}} - return []integrationplugins.CallRule{rule}, nil + return []integrationplugins.CallRule{operationRule(operationName(p, "update_status"))}, nil }, }) integrationplugins.RegisterCapability("monday", "add_comment", integrationplugins.CapabilitySpec{ + Params: []string{"operationName"}, Generate: func(p map[string]interface{}) ([]integrationplugins.CallRule, error) { - rule := integrationplugins.CallRule{Path: "/v2", Methods: map[string]integrationplugins.RequestConstraint{"POST": {}}} - return []integrationplugins.CallRule{rule}, nil + return []integrationplugins.CallRule{operationRule(operationName(p, "add_comment"))}, nil }, }) } diff --git a/app/integrations/plugins/monday/monday_test.go b/app/integrations/plugins/monday/monday_test.go index 5beb2e7d..3d782378 100644 --- a/app/integrations/plugins/monday/monday_test.go +++ b/app/integrations/plugins/monday/monday_test.go @@ -14,13 +14,14 @@ func TestMondayCapabilities(t *testing.T) { } tests := []struct { - name string - path string - method string + name string + path string + method string + operationName string }{ - {"create_item", "/v2", "POST"}, - {"update_status", "/v2", "POST"}, - {"add_comment", "/v2", "POST"}, + {"create_item", "/v2", "POST", "create_item"}, + {"update_status", "/v2", "POST", "update_status"}, + {"add_comment", "/v2", "POST", "add_comment"}, } for _, tt := range tests { @@ -39,8 +40,28 @@ func TestMondayCapabilities(t *testing.T) { if r.Path != tt.path { t.Errorf("%s path mismatch: %s", tt.name, r.Path) } - if _, ok := r.Methods[tt.method]; !ok { + rc, ok := r.Methods[tt.method] + if !ok { t.Errorf("%s missing method %s", tt.name, tt.method) } + if rc.Body["operationName"] != tt.operationName { + t.Errorf("%s operationName mismatch: %v", tt.name, rc.Body["operationName"]) + } + + rules, err = spec.Generate(map[string]interface{}{}) + if err != nil { + t.Fatalf("generate empty params failed: %v", err) + } + if got := rules[0].Methods[tt.method].Body["operationName"]; got != tt.operationName { + t.Errorf("%s empty operationName fallback mismatch: %v", tt.name, got) + } + + rules, err = spec.Generate(map[string]interface{}{"operationName": "customOp"}) + if err != nil { + t.Fatalf("generate custom operation failed: %v", err) + } + if got := rules[0].Methods[tt.method].Body["operationName"]; got != "customOp" { + t.Errorf("%s custom operationName mismatch: %v", tt.name, got) + } } } diff --git a/app/integrations/plugins/sendgrid/capabilities.go b/app/integrations/plugins/sendgrid/capabilities.go index 13d55a76..4b20ade1 100644 --- a/app/integrations/plugins/sendgrid/capabilities.go +++ b/app/integrations/plugins/sendgrid/capabilities.go @@ -15,11 +15,15 @@ func init() { return nil, fmt.Errorf("from parameter required") } reply, replyOK := p["replyTo"] - body := map[string]interface{}{"from": from} - if replyOK { - body["reply_to"] = reply - } else { - body["reply_to"] = nil + body := map[string]interface{}{ + "from": map[string]interface{}{"email": from}, + } + if replyOK && reply != nil { + replyTo, ok := reply.(string) + if !ok || replyTo == "" { + return nil, fmt.Errorf("replyTo must be a non-empty string or null") + } + body["reply_to"] = map[string]interface{}{"email": replyTo} } rule := integrationplugins.CallRule{Path: "/v3/mail/send", Methods: map[string]integrationplugins.RequestConstraint{"POST": {Body: body}}} return []integrationplugins.CallRule{rule}, nil diff --git a/app/integrations/plugins/sendgrid/sendgrid_test.go b/app/integrations/plugins/sendgrid/sendgrid_test.go index a0e8324d..bbc5b09c 100644 --- a/app/integrations/plugins/sendgrid/sendgrid_test.go +++ b/app/integrations/plugins/sendgrid/sendgrid_test.go @@ -49,10 +49,11 @@ func TestSendgridCapabilities(t *testing.T) { continue } if tt.name == "send_email" { - if rc.Body["from"] != "me@example.com" { + from, ok := rc.Body["from"].(map[string]interface{}) + if !ok || from["email"] != "me@example.com" { t.Errorf("from not propagated") } - if rc.Body["reply_to"] != nil { + if _, ok := rc.Body["reply_to"]; ok { t.Errorf("reply_to default unexpected: %#v", rc.Body["reply_to"]) } } @@ -68,7 +69,8 @@ func TestSendgridCapabilities(t *testing.T) { t.Fatalf("unexpected error: %v", err) } rc := rules[0].Methods["POST"] - if rc.Body["reply_to"] != "r@example.com" { + replyTo, ok := rc.Body["reply_to"].(map[string]interface{}) + if !ok || replyTo["email"] != "r@example.com" { t.Errorf("reply_to value not propagated") } @@ -77,7 +79,11 @@ func TestSendgridCapabilities(t *testing.T) { t.Fatalf("unexpected error: %v", err) } rc = rules[0].Methods["POST"] - if rc.Body["reply_to"] != nil { + if _, ok := rc.Body["reply_to"]; ok { t.Errorf("reply_to nil not set") } + + if _, err := spec.Generate(map[string]interface{}{"from": "me@example.com", "replyTo": 123}); err == nil { + t.Errorf("expected error for non-string replyTo") + } } diff --git a/app/integrations/registry.go b/app/integrations/registry.go index a12861fc..c3ad2481 100644 --- a/app/integrations/registry.go +++ b/app/integrations/registry.go @@ -50,6 +50,9 @@ func ExpandCapabilities(integration string, callers []CallerConfig) []CallerConf if !ok { continue } + if spec.Generate == nil { + continue + } rules, err := spec.Generate(cap.Params) if err != nil { continue diff --git a/app/integrations/registry_test.go b/app/integrations/registry_test.go index 29ca696a..2002c252 100644 --- a/app/integrations/registry_test.go +++ b/app/integrations/registry_test.go @@ -78,6 +78,22 @@ func TestExpandCapabilitiesGenerateError(t *testing.T) { } } +func TestExpandCapabilitiesNilGenerator(t *testing.T) { + orig := capabilityRegistry + capabilityRegistry = map[string]map[string]CapabilitySpec{} + t.Cleanup(func() { capabilityRegistry = orig }) + RegisterCapability("e", "cap", CapabilitySpec{}) + + callers := []CallerConfig{{ID: "c", Capabilities: []CapabilityConfig{{Name: "cap"}}}} + res := ExpandCapabilities("e", callers) + if len(res) != 1 || len(res[0].Rules) != 0 { + t.Fatalf("expected no rules without generator") + } + if len(res[0].Capabilities) != 0 { + t.Fatalf("capabilities not cleared") + } +} + func TestCapabilitiesHelpers(t *testing.T) { // Save registry and restore after test orig := capabilityRegistry diff --git a/app/secrets/plugins/aws/plugin.go b/app/secrets/plugins/aws/plugin.go index 8044cb13..8179d413 100644 --- a/app/secrets/plugins/aws/plugin.go +++ b/app/secrets/plugins/aws/plugin.go @@ -18,10 +18,10 @@ var ( newGCM = cipher.NewGCM ) -// awsKMSPlugin decrypts secrets using a symmetric key provided via the -// AWS_KMS_KEY environment variable. The ciphertext must be base64 encoded and -// include a 12 byte nonce prefix followed by the encrypted data. This is not a -// real AWS KMS integration but provides basic encryption semantics for tests. +// awsKMSPlugin decrypts legacy local AES-GCM envelope values using a symmetric +// key provided via the AWS_KMS_KEY environment variable. The ciphertext must be +// base64 encoded and include a 12 byte nonce prefix followed by the encrypted +// data. This is not a real AWS KMS integration. type awsKMSPlugin struct { once sync.Once key []byte diff --git a/cmd/allowlist/main.go b/cmd/allowlist/main.go index 7391f5ac..73f05b28 100644 --- a/cmd/allowlist/main.go +++ b/cmd/allowlist/main.go @@ -2,10 +2,12 @@ package main import ( "bytes" + "encoding/json" "flag" "fmt" yaml "gopkg.in/yaml.v3" "os" + "sort" "strings" "github.com/winhowes/AuthTranslator/cmd/allowlist/plugins" @@ -45,9 +47,22 @@ func main() { } func listCaps() { - for integ, caps := range plugins.List() { + list := plugins.List() + integrations := make([]string, 0, len(list)) + for integ := range list { + integrations = append(integrations, integ) + } + sort.Strings(integrations) + for _, integ := range integrations { + caps := list[integ] fmt.Println(integ + ":") - for name, spec := range caps { + names := make([]string, 0, len(caps)) + for name := range caps { + names = append(names, name) + } + sort.Strings(names) + for _, name := range names { + spec := caps[name] fmt.Printf(" %s (params: %s)\n", name, strings.Join(spec.Params, ",")) } } @@ -69,21 +84,16 @@ func addEntry(args []string) { fs.Usage() return } - var params map[string]interface{} - if *paramList != "" { - params = make(map[string]interface{}) - for _, kv := range strings.Split(*paramList, ",") { - kv = strings.TrimSpace(kv) - if kv == "" { - continue - } - parts := strings.SplitN(kv, "=", 2) - if len(parts) == 2 { - k := strings.TrimSpace(parts[0]) - v := strings.TrimSpace(parts[1]) - params[k] = v - } - } + params, err := parseParams(*paramList) + if err != nil { + fmt.Fprintln(os.Stderr, err) + return + } + + wantName := strings.ToLower(*integ) + if err := plugins.ValidateCapability(wantName, plugins.CapabilityConfig{Name: *capName, Params: params}); err != nil { + fmt.Fprintln(os.Stderr, err) + return } // load file @@ -100,7 +110,6 @@ func addEntry(args []string) { } } // find integration - wantName := strings.ToLower(*integ) var entry *plugins.AllowlistEntry for i := range entries { if strings.ToLower(entries[i].Integration) == wantName { @@ -151,6 +160,104 @@ func addEntry(args []string) { } } +func parseParams(paramList string) (map[string]interface{}, error) { + if strings.TrimSpace(paramList) == "" { + return nil, nil + } + params := make(map[string]interface{}) + items, err := splitParamList(paramList) + if err != nil { + return nil, err + } + for _, kv := range items { + kv = strings.TrimSpace(kv) + if kv == "" { + continue + } + parts := strings.SplitN(kv, "=", 2) + if len(parts) != 2 || strings.TrimSpace(parts[0]) == "" { + return nil, fmt.Errorf("invalid param %q, expected key=value", kv) + } + value, err := parseParamValue(strings.TrimSpace(parts[1])) + if err != nil { + return nil, fmt.Errorf("invalid value for param %s: %w", strings.TrimSpace(parts[0]), err) + } + params[strings.TrimSpace(parts[0])] = value + } + return params, nil +} + +func splitParamList(paramList string) ([]string, error) { + var parts []string + start := 0 + depth := 0 + inQuote := false + escaped := false + for i, r := range paramList { + if inQuote { + if escaped { + escaped = false + continue + } + switch r { + case '\\': + escaped = true + case '"': + inQuote = false + } + continue + } + switch r { + case '"': + inQuote = true + case '[', '{': + depth++ + case ']', '}': + if depth == 0 { + return nil, fmt.Errorf("invalid params: unmatched %q", r) + } + depth-- + case ',': + if depth == 0 { + parts = append(parts, paramList[start:i]) + start = i + 1 + } + } + } + if inQuote { + return nil, fmt.Errorf("invalid params: unterminated quoted value") + } + if depth != 0 { + return nil, fmt.Errorf("invalid params: unmatched bracket or brace") + } + parts = append(parts, paramList[start:]) + return parts, nil +} + +func parseParamValue(raw string) (interface{}, error) { + if raw == "" { + return "", nil + } + switch raw[0] { + case '[', '{', '"': + var v interface{} + if err := json.Unmarshal([]byte(raw), &v); err != nil { + return nil, err + } + return v, nil + } + switch raw { + case "null": + return nil, nil + case "true": + return true, nil + case "false": + return false, nil + default: + return raw, nil + } +} + func removeEntry(args []string) { fs := flag.NewFlagSet("remove", flag.ExitOnError) fs.Usage = func() { diff --git a/cmd/allowlist/main_test.go b/cmd/allowlist/main_test.go index 4c9d8907..6a777059 100644 --- a/cmd/allowlist/main_test.go +++ b/cmd/allowlist/main_test.go @@ -16,6 +16,8 @@ import ( "github.com/winhowes/AuthTranslator/cmd/allowlist/plugins" ) +const fullAccessCapability = "dangerously_allow_full_access" + // helper to capture stdout from f func captureOutput(f func()) string { r, w, err := os.Pipe() @@ -51,7 +53,7 @@ func TestAddEntryNewFile(t *testing.T) { *file = path t.Cleanup(func() { *file = old }) - addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "cap"}) + addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", fullAccessCapability}) data, err := os.ReadFile(path) if err != nil { @@ -65,7 +67,7 @@ func TestAddEntryNewFile(t *testing.T) { { Integration: "foo", Callers: []plugins.CallerConfig{ - {ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: "cap", Params: nil}}}, + {ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: fullAccessCapability, Params: nil}}}, }, }, } @@ -79,7 +81,7 @@ func TestAddEntryUpdateExisting(t *testing.T) { path := filepath.Join(tmpDir, "allow.yaml") initial := []plugins.AllowlistEntry{ - {Integration: "foo", Callers: []plugins.CallerConfig{{ID: "u1"}}}, + {Integration: "github", Callers: []plugins.CallerConfig{{ID: "u1"}}}, } data, _ := yaml.Marshal(initial) if err := os.WriteFile(path, data, 0644); err != nil { @@ -90,7 +92,7 @@ func TestAddEntryUpdateExisting(t *testing.T) { *file = path t.Cleanup(func() { *file = old }) - addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "cap2", "-params", "k=v"}) + addEntry([]string{"-integration", "github", "-caller", "u1", "-capability", "comment", "-params", "repo=org/repo"}) out, err := os.ReadFile(path) if err != nil { @@ -102,8 +104,8 @@ func TestAddEntryUpdateExisting(t *testing.T) { } want := []plugins.AllowlistEntry{ { - Integration: "foo", - Callers: []plugins.CallerConfig{{ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: "cap2", Params: map[string]interface{}{"k": "v"}}}}}, + Integration: "github", + Callers: []plugins.CallerConfig{{ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: "comment", Params: map[string]interface{}{"repo": "org/repo"}}}}}, }, } if !reflect.DeepEqual(entries, want) { @@ -127,7 +129,7 @@ func TestAddEntryIntegrationCaseInsensitive(t *testing.T) { *file = path t.Cleanup(func() { *file = old }) - addEntry([]string{"-integration", "FOO", "-caller", "u1", "-capability", "cap"}) + addEntry([]string{"-integration", "FOO", "-caller", "u1", "-capability", fullAccessCapability}) out, err := os.ReadFile(path) if err != nil { @@ -140,7 +142,7 @@ func TestAddEntryIntegrationCaseInsensitive(t *testing.T) { want := []plugins.AllowlistEntry{ { Integration: "foo", - Callers: []plugins.CallerConfig{{ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: "cap", Params: nil}}}}, + Callers: []plugins.CallerConfig{{ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: fullAccessCapability, Params: nil}}}}, }, } if !reflect.DeepEqual(entries, want) { @@ -304,7 +306,7 @@ func TestAddEntryNewCaller(t *testing.T) { *file = path t.Cleanup(func() { *file = old }) - addEntry([]string{"-integration", "foo", "-caller", "u2", "-capability", "cap2"}) + addEntry([]string{"-integration", "foo", "-caller", "u2", "-capability", fullAccessCapability}) out, err := os.ReadFile(path) if err != nil { @@ -319,7 +321,7 @@ func TestAddEntryNewCaller(t *testing.T) { Integration: "foo", Callers: []plugins.CallerConfig{ {ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: "cap1", Params: nil}}}, - {ID: "u2", Capabilities: []plugins.CapabilityConfig{{Name: "cap2", Params: nil}}}, + {ID: "u2", Capabilities: []plugins.CapabilityConfig{{Name: fullAccessCapability, Params: nil}}}, }, }, } @@ -336,9 +338,9 @@ func TestAddEntryDuplicateCapability(t *testing.T) { initial := []plugins.AllowlistEntry{ { - Integration: "foo", + Integration: "github", Callers: []plugins.CallerConfig{ - {ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: "cap", Params: map[string]interface{}{"k": "v1"}}}}, + {ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: "comment", Params: map[string]interface{}{"repo": "org/old"}}}}, }, }, } @@ -351,7 +353,7 @@ func TestAddEntryDuplicateCapability(t *testing.T) { *file = path t.Cleanup(func() { *file = old }) - addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "cap", "-params", "k=v2"}) + addEntry([]string{"-integration", "github", "-caller", "u1", "-capability", "comment", "-params", "repo=org/new"}) out, err := os.ReadFile(path) if err != nil { @@ -363,9 +365,9 @@ func TestAddEntryDuplicateCapability(t *testing.T) { } want := []plugins.AllowlistEntry{ { - Integration: "foo", + Integration: "github", Callers: []plugins.CallerConfig{ - {ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: "cap", Params: map[string]interface{}{"k": "v2"}}}}, + {ID: "u1", Capabilities: []plugins.CapabilityConfig{{Name: "comment", Params: map[string]interface{}{"repo": "org/new"}}}}, }, }, } @@ -382,7 +384,7 @@ func TestAddEntryParamTrim(t *testing.T) { *file = path t.Cleanup(func() { *file = old }) - addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "cap", "-params", "k=v1, bar = baz "}) + addEntry([]string{"-integration", "sendgrid", "-caller", "u1", "-capability", "send_email", "-params", "from=me@example.com, replyTo = reply@example.com "}) data, err := os.ReadFile(path) if err != nil { @@ -393,7 +395,7 @@ func TestAddEntryParamTrim(t *testing.T) { t.Fatalf("failed to unmarshal: %v", err) } params := entries[0].Callers[0].Capabilities[0].Params - if params["k"] != "v1" || params["bar"] != "baz" { + if params["from"] != "me@example.com" || params["replyTo"] != "reply@example.com" { t.Fatalf("params not trimmed: %#v", params) } } @@ -405,7 +407,7 @@ func TestAddEntryIgnoresEmptyParams(t *testing.T) { *file = path t.Cleanup(func() { *file = old }) - addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "cap", "-params", "k=v1,, ,other=v2"}) + addEntry([]string{"-integration", "sendgrid", "-caller", "u1", "-capability", "send_email", "-params", "from=me@example.com,, ,replyTo=reply@example.com"}) data, err := os.ReadFile(path) if err != nil { @@ -416,11 +418,132 @@ func TestAddEntryIgnoresEmptyParams(t *testing.T) { t.Fatalf("failed to unmarshal: %v", err) } params := entries[0].Callers[0].Capabilities[0].Params - if len(params) != 2 || params["k"] != "v1" || params["other"] != "v2" { + if len(params) != 2 || params["from"] != "me@example.com" || params["replyTo"] != "reply@example.com" { t.Fatalf("unexpected params: %#v", params) } } +func TestAddEntryParsesStructuredParams(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "allow.yaml") + old := *file + *file = path + t.Cleanup(func() { *file = old }) + + addEntry([]string{"-integration", "slack", "-caller", "u1", "-capability", "post_channels", "-params", `channels=["C123","C456"]`}) + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed reading file: %v", err) + } + var entries []plugins.AllowlistEntry + if err := yaml.Unmarshal(data, &entries); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + params := entries[0].Callers[0].Capabilities[0].Params + channels, ok := params["channels"].([]interface{}) + if !ok || len(channels) != 2 || channels[0] != "C123" || channels[1] != "C456" { + t.Fatalf("channels not parsed as JSON array: %#v", params["channels"]) + } +} + +func TestAddEntryRejectsMalformedParams(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "allow.yaml") + old := *file + *file = path + t.Cleanup(func() { *file = old }) + + out := captureStderr(func() { + addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", fullAccessCapability, "-params", "repo"}) + }) + if !strings.Contains(out, "invalid param") { + t.Fatalf("unexpected error output: %s", out) + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("file should not be created") + } +} + +func TestParseParamsStructuredValues(t *testing.T) { + params, err := parseParams(`repo=org/repo,object={"labels":["bug"]},quoted="a\"b",empty=,none=null,enabled=true,disabled=false`) + if err != nil { + t.Fatalf("parseParams failed: %v", err) + } + + want := map[string]interface{}{ + "repo": "org/repo", + "object": map[string]interface{}{"labels": []interface{}{"bug"}}, + "quoted": `a"b`, + "empty": "", + "none": nil, + "enabled": true, + "disabled": false, + } + if !reflect.DeepEqual(params, want) { + t.Fatalf("params mismatch:\n%#v\nwant\n%#v", params, want) + } +} + +func TestParseParamsErrors(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {name: "invalid param", input: "repo", want: "invalid param"}, + {name: "invalid json", input: "channels=[broken]", want: "invalid value for param channels"}, + {name: "unmatched close", input: "channels=]", want: "unmatched"}, + {name: "unterminated quote", input: `text="unterminated`, want: "unterminated quoted value"}, + {name: "unmatched bracket", input: "channels=[", want: "unmatched bracket or brace"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := parseParams(tt.input) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Fatalf("expected error containing %q, got %v", tt.want, err) + } + }) + } +} + +func TestAddEntryRejectsUnknownCapability(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "allow.yaml") + old := *file + *file = path + t.Cleanup(func() { *file = old }) + + out := captureStderr(func() { + addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "missing"}) + }) + if !strings.Contains(out, "unknown capability missing") { + t.Fatalf("unexpected error output: %s", out) + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("file should not be created") + } +} + +func TestAddEntryRejectsInvalidCapabilityParam(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "allow.yaml") + old := *file + *file = path + t.Cleanup(func() { *file = old }) + + out := captureStderr(func() { + addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", fullAccessCapability, "-params", "extra=value"}) + }) + if !strings.Contains(out, "unknown param extra") { + t.Fatalf("unexpected error output: %s", out) + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("file should not be created") + } +} + func TestAddEntryMissingArgs(t *testing.T) { tmpDir := t.TempDir() path := filepath.Join(tmpDir, "allow.yaml") @@ -655,7 +778,7 @@ func TestAddEntryInvalidYAML(t *testing.T) { t.Cleanup(func() { *file = old }) out := captureStderr(func() { - addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "c"}) + addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", fullAccessCapability}) }) if out == "" { t.Fatalf("expected error output") @@ -675,7 +798,7 @@ func TestAddEntryReadFileError(t *testing.T) { t.Cleanup(func() { *file = old }) out := captureStderr(func() { - addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "c"}) + addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", fullAccessCapability}) }) if out == "" { t.Fatalf("expected error output") @@ -701,9 +824,9 @@ func TestMainAddRemoveCommands(t *testing.T) { t.Cleanup(func() { flag.CommandLine = oldFS; file = oldFile }) origArgs := os.Args - os.Args = []string{"allowlist", "add", "-integration", "foo", "-caller", "u1", "-capability", "cap"} + os.Args = []string{"allowlist", "add", "-integration", "foo", "-caller", "u1", "-capability", fullAccessCapability} main() - os.Args = []string{"allowlist", "remove", "-integration", "foo", "-caller", "u1", "-capability", "cap"} + os.Args = []string{"allowlist", "remove", "-integration", "foo", "-caller", "u1", "-capability", fullAccessCapability} main() os.Args = origArgs @@ -723,7 +846,7 @@ func TestAddEntryHelper(t *testing.T) { cfg := os.Getenv("CFG") flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) file = flag.CommandLine.String("file", cfg, "allowlist file") - addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "cap"}) + addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", fullAccessCapability}) os.Exit(0) } @@ -760,7 +883,7 @@ func TestAddEntryMarshalError(t *testing.T) { } }() - addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", "cap"}) + addEntry([]string{"-integration", "foo", "-caller", "u1", "-capability", fullAccessCapability}) } func TestRemoveEntryMarshalError(t *testing.T) { diff --git a/cmd/allowlist/plugins/asana.go b/cmd/allowlist/plugins/asana.go deleted file mode 100644 index 652d2be6..00000000 --- a/cmd/allowlist/plugins/asana.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("asana", "create_task", CapabilitySpec{}) - RegisterCapability("asana", "update_status", CapabilitySpec{}) - RegisterCapability("asana", "add_comment", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/dangerously_full_access.go b/cmd/allowlist/plugins/dangerously_full_access.go deleted file mode 100644 index a658cbd2..00000000 --- a/cmd/allowlist/plugins/dangerously_full_access.go +++ /dev/null @@ -1,5 +0,0 @@ -package plugins - -func init() { - RegisterCapability("*", "dangerously_allow_full_access", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/github.go b/cmd/allowlist/plugins/github.go deleted file mode 100644 index 044ea3bf..00000000 --- a/cmd/allowlist/plugins/github.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("github", "comment", CapabilitySpec{ - Params: []string{"repo"}, - }) -} diff --git a/cmd/allowlist/plugins/jira.go b/cmd/allowlist/plugins/jira.go deleted file mode 100644 index 418d87f2..00000000 --- a/cmd/allowlist/plugins/jira.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("jira", "create_task", CapabilitySpec{}) - RegisterCapability("jira", "update_status", CapabilitySpec{}) - RegisterCapability("jira", "add_comment", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/linear.go b/cmd/allowlist/plugins/linear.go deleted file mode 100644 index 76af60a9..00000000 --- a/cmd/allowlist/plugins/linear.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("linear", "create_task", CapabilitySpec{}) - RegisterCapability("linear", "update_status", CapabilitySpec{}) - RegisterCapability("linear", "add_comment", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/monday.go b/cmd/allowlist/plugins/monday.go deleted file mode 100644 index 740de911..00000000 --- a/cmd/allowlist/plugins/monday.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("monday", "create_item", CapabilitySpec{}) - RegisterCapability("monday", "update_status", CapabilitySpec{}) - RegisterCapability("monday", "add_comment", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/okta.go b/cmd/allowlist/plugins/okta.go deleted file mode 100644 index b1c96190..00000000 --- a/cmd/allowlist/plugins/okta.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("okta", "create_user", CapabilitySpec{}) - RegisterCapability("okta", "update_user", CapabilitySpec{}) - RegisterCapability("okta", "deactivate_user", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/registry.go b/cmd/allowlist/plugins/registry.go index 1ee40dc9..c5ef7466 100644 --- a/cmd/allowlist/plugins/registry.go +++ b/cmd/allowlist/plugins/registry.go @@ -1,19 +1,56 @@ package plugins -// CapabilitySpec describes a capability's parameters. -type CapabilitySpec struct { - Params []string +import ( + "fmt" + + integrationplugins "github.com/winhowes/AuthTranslator/app/integrations" + _ "github.com/winhowes/AuthTranslator/app/integrations/plugins" +) + +// CapabilitySpec describes a capability's parameters and rule generator. +type CapabilitySpec = integrationplugins.CapabilitySpec + +func List() map[string]map[string]CapabilitySpec { + return integrationplugins.AllCapabilities() } -var registry = map[string]map[string]CapabilitySpec{} +func Capability(integration, name string) (CapabilitySpec, bool) { + if caps := integrationplugins.CapabilitiesFor(integration); caps != nil { + if spec, ok := caps[name]; ok { + return spec, true + } + } + if caps := integrationplugins.CapabilitiesFor(integrationplugins.GlobalIntegration); caps != nil { + spec, ok := caps[name] + return spec, ok + } + return CapabilitySpec{}, false +} -func RegisterCapability(integration, name string, spec CapabilitySpec) { - if registry[integration] == nil { - registry[integration] = map[string]CapabilitySpec{} +func ValidateCapability(integration string, cap CapabilityConfig) error { + spec, ok := Capability(integration, cap.Name) + if !ok { + return fmt.Errorf("unknown capability %s for integration %s", cap.Name, integration) + } + for param := range cap.Params { + if !knownParam(spec.Params, param) { + return fmt.Errorf("unknown param %s for capability %s", param, cap.Name) + } + } + if spec.Generate == nil { + return fmt.Errorf("capability %s has no rule generator", cap.Name) } - registry[integration][name] = spec + if _, err := spec.Generate(cap.Params); err != nil { + return fmt.Errorf("invalid params for capability %s: %w", cap.Name, err) + } + return nil } -func List() map[string]map[string]CapabilitySpec { - return registry +func knownParam(params []string, name string) bool { + for _, param := range params { + if param == name { + return true + } + } + return false } diff --git a/cmd/allowlist/plugins/registry_test.go b/cmd/allowlist/plugins/registry_test.go index 797db613..cc5df361 100644 --- a/cmd/allowlist/plugins/registry_test.go +++ b/cmd/allowlist/plugins/registry_test.go @@ -2,7 +2,10 @@ package plugins import ( "reflect" + "strings" "testing" + + integrationplugins "github.com/winhowes/AuthTranslator/app/integrations" ) // Test that plugin init functions register their capabilities into the registry. @@ -40,4 +43,83 @@ func TestRegistryInitialization(t *testing.T) { } else if !reflect.DeepEqual(spec.Params, []string{"repo"}) { t.Fatalf("unexpected github comment params: %v", spec.Params) } + if spec, ok := github["create_issue"]; !ok { + t.Fatalf("github create_issue capability missing") + } else if !reflect.DeepEqual(spec.Params, []string{"repo"}) { + t.Fatalf("unexpected github create_issue params: %v", spec.Params) + } + + openai, ok := list["openai"] + if !ok { + t.Fatalf("openai capabilities missing") + } + if _, ok := openai["chat_completion"]; !ok { + t.Fatalf("openai chat_completion capability missing") + } +} + +func TestValidateCapability(t *testing.T) { + if err := ValidateCapability("github", CapabilityConfig{Name: "comment", Params: map[string]interface{}{"repo": "org/repo"}}); err != nil { + t.Fatalf("expected valid capability: %v", err) + } + if err := ValidateCapability("github", CapabilityConfig{Name: integrationplugins.DangerouslyAllowFullAccess}); err != nil { + t.Fatalf("expected global capability to be valid: %v", err) + } + if err := ValidateCapability("github", CapabilityConfig{Name: "missing"}); err == nil { + t.Fatalf("expected unknown capability error") + } + if err := ValidateCapability("github", CapabilityConfig{Name: "comment", Params: map[string]interface{}{"bogus": "x"}}); err == nil { + t.Fatalf("expected unknown param error") + } + if err := ValidateCapability("github", CapabilityConfig{Name: "comment"}); err == nil { + t.Fatalf("expected missing repo error") + } + if err := ValidateCapability("github", CapabilityConfig{Name: "comment", Params: map[string]interface{}{"repo": "org/repo"}}); err != nil { + t.Fatalf("expected valid capability after errors: %v", err) + } +} + +func TestCapabilityNoGlobalRegistryFallback(t *testing.T) { + restore := snapshotRegistry(t) + defer restore() + + reg := integrationplugins.AllCapabilities() + delete(reg, integrationplugins.GlobalIntegration) + + if _, ok := Capability("missing", "missing"); ok { + t.Fatal("expected missing capability when integration and global registry are absent") + } +} + +func TestValidateCapabilityWithoutGenerator(t *testing.T) { + restore := snapshotRegistry(t) + defer restore() + + integrationplugins.RegisterCapability("registrytest", "nogenerator", integrationplugins.CapabilitySpec{}) + err := ValidateCapability("registrytest", CapabilityConfig{Name: "nogenerator"}) + if err == nil || !strings.Contains(err.Error(), "has no rule generator") { + t.Fatalf("expected no rule generator error, got %v", err) + } +} + +func snapshotRegistry(t *testing.T) func() { + t.Helper() + + orig := make(map[string]map[string]integrationplugins.CapabilitySpec) + for integ, caps := range integrationplugins.AllCapabilities() { + m := make(map[string]integrationplugins.CapabilitySpec, len(caps)) + for name, spec := range caps { + m[name] = spec + } + orig[integ] = m + } + return func() { + reg := integrationplugins.AllCapabilities() + for k := range reg { + delete(reg, k) + } + for k, v := range orig { + reg[k] = v + } + } } diff --git a/cmd/allowlist/plugins/sendgrid.go b/cmd/allowlist/plugins/sendgrid.go deleted file mode 100644 index 65148396..00000000 --- a/cmd/allowlist/plugins/sendgrid.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("sendgrid", "send_email", CapabilitySpec{}) - RegisterCapability("sendgrid", "manage_contacts", CapabilitySpec{}) - RegisterCapability("sendgrid", "update_template", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/servicenow.go b/cmd/allowlist/plugins/servicenow.go deleted file mode 100644 index 1b855d66..00000000 --- a/cmd/allowlist/plugins/servicenow.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("servicenow", "open_ticket", CapabilitySpec{}) - RegisterCapability("servicenow", "update_ticket", CapabilitySpec{}) - RegisterCapability("servicenow", "query_status", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/slack.go b/cmd/allowlist/plugins/slack.go deleted file mode 100644 index 2cd034f2..00000000 --- a/cmd/allowlist/plugins/slack.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("slack", "post_as", CapabilitySpec{Params: []string{"username"}}) - RegisterCapability("slack", "post_channels_as", CapabilitySpec{Params: []string{"username", "channels"}}) - RegisterCapability("slack", "post_channels", CapabilitySpec{Params: []string{"channels"}}) -} diff --git a/cmd/allowlist/plugins/stripe.go b/cmd/allowlist/plugins/stripe.go deleted file mode 100644 index 5abc3c6c..00000000 --- a/cmd/allowlist/plugins/stripe.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("stripe", "create_charge", CapabilitySpec{}) - RegisterCapability("stripe", "refund_charge", CapabilitySpec{}) - RegisterCapability("stripe", "create_customer", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/twilio.go b/cmd/allowlist/plugins/twilio.go deleted file mode 100644 index 801a4d32..00000000 --- a/cmd/allowlist/plugins/twilio.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("twilio", "send_sms", CapabilitySpec{}) - RegisterCapability("twilio", "make_call", CapabilitySpec{}) - RegisterCapability("twilio", "query_message", CapabilitySpec{}) -} diff --git a/cmd/allowlist/plugins/zendesk.go b/cmd/allowlist/plugins/zendesk.go deleted file mode 100644 index 78a88511..00000000 --- a/cmd/allowlist/plugins/zendesk.go +++ /dev/null @@ -1,7 +0,0 @@ -package plugins - -func init() { - RegisterCapability("zendesk", "open_ticket", CapabilitySpec{}) - RegisterCapability("zendesk", "update_ticket", CapabilitySpec{}) - RegisterCapability("zendesk", "query_status", CapabilitySpec{}) -} diff --git a/cmd/integrations/plugins/builders_test.go b/cmd/integrations/plugins/builders_test.go index 6bf24940..33f22b3c 100644 --- a/cmd/integrations/plugins/builders_test.go +++ b/cmd/integrations/plugins/builders_test.go @@ -25,14 +25,14 @@ func TestBuilders(t *testing.T) { {"okta", []string{"-name", "ok", "-domain", "okta.example.com", "-token", "tok"}, Okta("ok", "okta.example.com", "tok")}, {"sendgrid", []string{"-name", "sg", "-token", "tok"}, SendGrid("sg", "tok")}, {"trufflehog", []string{"-name", "th", "-token", "tok"}, TruffleHog("th", "tok")}, - {"servicenow", []string{"-name", "sn", "-token", "tok"}, ServiceNow("sn", "tok")}, + {"servicenow", []string{"-name", "sn", "-domain", "sn.example.com", "-token", "tok"}, ServiceNow("sn", "sn.example.com", "tok")}, {"slack", []string{"-name", "sl", "-token", "tok", "-signing-secret", "sec"}, Slack("sl", "tok", "sec")}, {"stripe", []string{"-name", "st", "-token", "tok"}, Stripe("st", "tok")}, {"twilio", []string{"-name", "tw", "-token", "tok"}, Twilio("tw", "tok")}, {"workday", []string{"-name", "wd", "-domain", "work.example.com", "-token", "tok"}, Workday("wd", "work.example.com", "tok")}, {"openai", []string{"-name", "oa", "-token", "tok"}, OpenAI("oa", "tok")}, {"pagerduty", []string{"-name", "pd", "-token", "tok"}, PagerDuty("pd", "tok")}, - {"zendesk", []string{"-name", "zd", "-token", "tok"}, Zendesk("zd", "tok")}, + {"zendesk", []string{"-name", "zd", "-domain", "zd.example.com", "-token", "tok"}, Zendesk("zd", "zd.example.com", "tok")}, } for _, tt := range tests { @@ -95,6 +95,17 @@ func TestBuilderErrors(t *testing.T) { } } +func TestTenantDomainIntegrations(t *testing.T) { + serviceNow := ServiceNow("sn", "sn.example.com/", "tok") + if serviceNow.Destination != "https://sn.example.com" { + t.Fatalf("unexpected ServiceNow destination: %s", serviceNow.Destination) + } + zendesk := Zendesk("zd", "https://zd.example.com/", "tok") + if zendesk.Destination != "https://zd.example.com" { + t.Fatalf("unexpected Zendesk destination: %s", zendesk.Destination) + } +} + func TestBuilderParseError(t *testing.T) { names := []string{ "asana", "confluence", "datadog", "ghe", "github", "gitlab", "jira", diff --git a/cmd/integrations/plugins/servicenow.go b/cmd/integrations/plugins/servicenow.go index c59b4ef5..858ed046 100644 --- a/cmd/integrations/plugins/servicenow.go +++ b/cmd/integrations/plugins/servicenow.go @@ -3,13 +3,18 @@ package plugins import ( "flag" "fmt" + "strings" ) // ServiceNow returns an Integration configured for the ServiceNow API. -func ServiceNow(name, tokenRef string) Integration { +func ServiceNow(name, domain, tokenRef string) Integration { + if !strings.HasPrefix(domain, "https://") && !strings.HasPrefix(domain, "http://") { + domain = "https://" + domain + } + dest := strings.TrimSuffix(domain, "/") return Integration{ Name: name, - Destination: "https://api.servicenow.com", + Destination: dest, InRateLimit: 100, OutRateLimit: 100, OutgoingAuth: []AuthPluginConfig{{ @@ -28,12 +33,13 @@ func init() { Register("servicenow", servicenowBuilder) } func servicenowBuilder(args []string) (Integration, error) { fs := flag.NewFlagSet("servicenow", flag.ContinueOnError) name := fs.String("name", "servicenow", "integration name") + domain := fs.String("domain", "", "ServiceNow instance domain, e.g. example.service-now.com") token := fs.String("token", "", "secret reference for API token") if err := fs.Parse(args); err != nil { return Integration{}, err } - if *token == "" { - return Integration{}, fmt.Errorf("-token is required") + if *token == "" || *domain == "" { + return Integration{}, fmt.Errorf("-token and -domain are required") } - return ServiceNow(*name, *token), nil + return ServiceNow(*name, *domain, *token), nil } diff --git a/cmd/integrations/plugins/zendesk.go b/cmd/integrations/plugins/zendesk.go index 7f8de506..f67d34bf 100644 --- a/cmd/integrations/plugins/zendesk.go +++ b/cmd/integrations/plugins/zendesk.go @@ -3,13 +3,18 @@ package plugins import ( "flag" "fmt" + "strings" ) // Zendesk returns an Integration configured for the Zendesk API. -func Zendesk(name, tokenRef string) Integration { +func Zendesk(name, domain, tokenRef string) Integration { + if !strings.HasPrefix(domain, "https://") && !strings.HasPrefix(domain, "http://") { + domain = "https://" + domain + } + dest := strings.TrimSuffix(domain, "/") return Integration{ Name: name, - Destination: "https://api.zendesk.com", + Destination: dest, InRateLimit: 100, OutRateLimit: 100, OutgoingAuth: []AuthPluginConfig{{ @@ -28,12 +33,13 @@ func init() { Register("zendesk", zendeskBuilder) } func zendeskBuilder(args []string) (Integration, error) { fs := flag.NewFlagSet("zendesk", flag.ContinueOnError) name := fs.String("name", "zendesk", "integration name") + domain := fs.String("domain", "", "Zendesk domain, e.g. example.zendesk.com") token := fs.String("token", "", "secret reference for API token") if err := fs.Parse(args); err != nil { return Integration{}, err } - if *token == "" { - return Integration{}, fmt.Errorf("-token is required") + if *token == "" || *domain == "" { + return Integration{}, fmt.Errorf("-token and -domain are required") } - return Zendesk(*name, *token), nil + return Zendesk(*name, *domain, *token), nil } diff --git a/docs/capabilities.md b/docs/capabilities.md index e1d72a0c..3c60b5ce 100644 --- a/docs/capabilities.md +++ b/docs/capabilities.md @@ -30,9 +30,9 @@ Run `go run ./cmd/allowlist list` to list capabilities from your build. For quic | linear | add_comment | – | | linear | create_task | – | | linear | update_status | – | -| monday | add_comment | – | -| monday | create_item | – | -| monday | update_status | – | +| monday | add_comment | operationName | +| monday | create_item | operationName | +| monday | update_status | operationName | | okta | create_user | – | | okta | deactivate_user | – | | okta | update_user | – | @@ -63,5 +63,7 @@ Run `go run ./cmd/allowlist list` to list capabilities from your build. For quic | zendesk | query_status | – | | zendesk | update_ticket | – | -For SendGrid `send_email`, if `replyTo` is omitted the reply address is empty. Provide `null` explicitly to leave the reply-to header unset. +For Monday capabilities, `operationName` is optional and defaults to the capability name. Requests must include the matching GraphQL `operationName` field so the single `/v2` endpoint can still be constrained by action. + +For SendGrid `send_email`, `from` maps to `from.email` and `replyTo`, when provided, maps to `reply_to.email`. Omit `replyTo` or pass `null` to leave `reply_to` unconstrained. Capabilities not listed above may be added by custom plugins. Use the CLI to discover them in your build. diff --git a/docs/cli.md b/docs/cli.md index 123d1228..1f4846a6 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -99,10 +99,14 @@ command is purely a discovery tool to help you decide which capability name and parameter keys to pass to `add`. The `-params` flag accepts a comma-separated list such as -`username=bot-123,channel=C123`. Each value is stored as a string in the YAML. -When a capability requires structured data (for example, the Slack plug-in's -`channels` parameter expects a list), run `add` with the closest shape you can and -then touch up the generated YAML manually to insert arrays or nested objects. +`username=bot-123,repo=org/repo`. Plain values are stored as strings. Values that +start with `[`, `{`, or `"` are parsed as JSON, so structured capability params can +be written directly: + +```bash +go run ./cmd/allowlist add -integration slack -caller bot \ + -capability post_channels -params 'channels=["C123","C456"]' +``` --- diff --git a/docs/integration-plugins.md b/docs/integration-plugins.md index 20a46636..83579e77 100644 --- a/docs/integration-plugins.md +++ b/docs/integration-plugins.md @@ -27,14 +27,14 @@ Think of it as a *cookie‑cutter* that stamps out a ready‑to‑run block in y | `openai` | `https://api.openai.com` | `token` | | `pagerduty` | `https://api.pagerduty.com` | `token` | | `sendgrid` | `https://api.sendgrid.com` | `token` | -| `servicenow` | `https://api.servicenow.com` | `token` | +| `servicenow` | `https://.service-now.com` (configurable) | `token` | | `slack` | `https://slack.com/` | `token` | | `gdrive` | `https://www.googleapis.com/drive/v3` | `gcp_token` | | `stripe` | `https://api.stripe.com` | `token` | | `trufflehog` | `https://trufflehog.cloud/api` | `token` | | `twilio` | `https://api.twilio.com` | `basic` | | `workday` | `https:///api` (configurable) | `token` | -| `zendesk` | `https://api.zendesk.com` | `token` | +| `zendesk` | `https://.zendesk.com` (configurable) | `token` | *(Full list lives under **[`cmd/integrations/plugins/`](../cmd/integrations/plugins/)**)* See [**Built-in Capabilities**](capabilities.md) for the convenience permissions @@ -107,4 +107,3 @@ From here you can tweak fields—e.g. turn on `tls_insecure_skip_verify` for a d * **Set sane timeouts** SaaS APIs differ; codify them so callers don’t guess. * **Keep zero secrets** Integration plugins should only reference secret URIs, never raw tokens. - diff --git a/docs/secret-backends.md b/docs/secret-backends.md index 28030578..93faf0ce 100644 --- a/docs/secret-backends.md +++ b/docs/secret-backends.md @@ -24,8 +24,8 @@ outgoing_auth: | `file` | `file:///etc/secrets/slack_token` | Kubernetes **secret volume** or Docker bind‑mount. Append `:KEY` to read key/value files; omit it to load the entire file. | | `k8s` | `k8s:default/mysecret#token` | In‑cluster secret via the Kubernetes API. | | `gcp` | `gcp:projects/acme/locations/global/keyRings/auth/cryptoKeys/token:ciphertext` | Running on GKE / Cloud Run; decrypt via **Cloud KMS**. | -| `aws` | `aws:Ci0KU29tZUNpcGhlcnRleHQ=` | AES‑GCM encrypted values decrypted using `AWS_KMS_KEY`. | -| `azure` | `azure:https://kv-name.vault.azure.net/secrets/secret-name` | AKS or VM SS with **Managed Identity**. | +| `aws` | `aws:Ci0KU29tZUNpcGhlcnRleHQ=` | Legacy local AES‑GCM envelope values decrypted using `AWS_KMS_KEY`; this is not AWS KMS. | +| `azure` | `azure:https://kv-name.vault.azure.net/secrets/secret-name` | Azure Key Vault using service-principal client credentials. | | `vault` | `vault:secret/data/slack` | Self‑hosted **HashiCorp Vault** cluster. | | `keychain` | `keychain:github-cli#octocat` | macOS hosts with secrets in Keychain (`service#account`). | | `secretservice` | `secretservice:service=slack,user=bot` | Linux desktops/servers with D-Bus Secret Service (`secret-tool`). | @@ -62,7 +62,7 @@ Some schemes rely on environment variables for authentication or decryption: | `env` | Names referenced in the configuration (e.g. `env:IN_TOKEN`) | Secrets are read directly from those variables. | `env:IN_TOKEN` resolves to `$IN_TOKEN` | | `file` | _none_ | Reads file contents from disk for `file:` secrets. Append `:KEY` to select entries from `KEY=value` files; omit it to load the whole file. | `file:/etc/secrets.env:SLACK_SECRET` | | `k8s` | `KUBERNETES_SERVICE_HOST`, `KUBERNETES_SERVICE_PORT` | Provided by Kubernetes; used with the in-cluster service account. | `k8s:default/mysecret#token` | -| `aws` | `AWS_KMS_KEY` | Base64 encoded 32 byte key for decrypting `aws:` secrets. | `aws:Ci0KU29tZUNpcGhlcnRleHQ=` | +| `aws` | `AWS_KMS_KEY` | Base64 encoded 32 byte local AES-GCM key for decrypting legacy `aws:` secrets. | `aws:Ci0KU29tZUNpcGhlcnRleHQ=` | | `azure` | `AZURE_TENANT_ID`, `AZURE_CLIENT_ID`, `AZURE_CLIENT_SECRET` | Credentials for fetching `azure:` secrets from Key Vault. | `azure:https://kv-name.vault.azure.net/secrets/token` | | `gcp` | _none_ | Uses the GCP metadata service when resolving `gcp:` secrets. | `gcp:projects/p/locations/l/keyRings/r/cryptoKeys/k:cipher` | | `vault` | `VAULT_ADDR`, `VAULT_TOKEN` | Fetches secrets from HashiCorp Vault via its HTTP API. | `vault:secret/data/api` reads from Vault | From de4624a929e4a67102bf981ebd953c09ba8e35d2 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Thu, 23 Apr 2026 20:59:39 -0700 Subject: [PATCH 2/4] preserve body array type checks --- app/allowlist.go | 12 +------ app/allowlist_body_filter_test.go | 10 ++---- app/allowlist_test.go | 9 ++---- .../plugins/slack/capabilities.go | 32 ++++++++++++------- app/integrations/plugins/slack/slack_test.go | 32 ++++++++++--------- 5 files changed, 43 insertions(+), 52 deletions(-) diff --git a/app/allowlist.go b/app/allowlist.go index 8c249b82..5b4b5fb3 100644 --- a/app/allowlist.go +++ b/app/allowlist.go @@ -365,12 +365,7 @@ func matchValueReason(data, rule interface{}, path string) (bool, string) { case []interface{}: da, ok := data.([]interface{}) if !ok { - for _, want := range rv { - if ok2, _ := matchValueReason(data, want, path); ok2 { - return true, "" - } - } - return false, fmt.Sprintf("body field %s value not in allowed set", path) + return false, fmt.Sprintf("body field %s not array", path) } for _, want := range rv { found := false @@ -420,11 +415,6 @@ func matchValue(data, rule interface{}) bool { case []interface{}: da, ok := data.([]interface{}) if !ok { - for _, want := range rv { - if matchValue(data, want) { - return true - } - } return false } for _, want := range rv { diff --git a/app/allowlist_body_filter_test.go b/app/allowlist_body_filter_test.go index f97c789d..9c76e9c5 100644 --- a/app/allowlist_body_filter_test.go +++ b/app/allowlist_body_filter_test.go @@ -53,18 +53,12 @@ func TestBodyArrayMatching(t *testing.T) { } } -func TestBodyArrayRuleMatchesScalarAllowedSet(t *testing.T) { +func TestBodyArrayRuleRejectsScalarValue(t *testing.T) { body := []byte(`{"channel":"C123"}`) rule := map[string]interface{}{"channel": []interface{}{"C123", "C456"}} r := req(http.MethodPost, body) - if !validateRequest(r, RequestConstraint{Body: rule}) { - t.Fatal("expected scalar body value to match one allowed rule value") - } - - rule = map[string]interface{}{"channel": []interface{}{"C999"}} - r = req(http.MethodPost, body) if validateRequest(r, RequestConstraint{Body: rule}) { - t.Fatal("expected scalar body value outside allowed set to fail") + t.Fatal("expected scalar body value to fail an array rule") } } diff --git a/app/allowlist_test.go b/app/allowlist_test.go index 9493583b..8efd066b 100644 --- a/app/allowlist_test.go +++ b/app/allowlist_test.go @@ -483,12 +483,9 @@ func TestMatchValueNotOkBranches(t *testing.T) { } } -func TestMatchValueScalarAllowedSet(t *testing.T) { - if !matchValue("C123", []interface{}{"C123", "C456"}) { - t.Fatal("expected scalar value to match one allowed value") - } - if matchValue("C999", []interface{}{"C123", "C456"}) { - t.Fatal("expected scalar value outside allowed set to fail") +func TestMatchValueArrayRuleRejectsScalarValue(t *testing.T) { + if matchValue("C123", []interface{}{"C123", "C456"}) { + t.Fatal("expected scalar value to fail an array rule") } } diff --git a/app/integrations/plugins/slack/capabilities.go b/app/integrations/plugins/slack/capabilities.go index bacc8bc1..947d9a8c 100644 --- a/app/integrations/plugins/slack/capabilities.go +++ b/app/integrations/plugins/slack/capabilities.go @@ -27,12 +27,7 @@ func init() { if user == "" || len(ch) == 0 { return nil, fmt.Errorf("username and channels required") } - allowed := make([]interface{}, len(ch)) - for i, c := range ch { - allowed[i] = c - } - rule := integrationplugins.CallRule{Path: "/api/chat.postMessage", Methods: map[string]integrationplugins.RequestConstraint{"POST": {Body: map[string]interface{}{"username": user, "channel": allowed}}}} - return []integrationplugins.CallRule{rule}, nil + return channelRules(map[string]interface{}{"username": user}, ch), nil }, }) @@ -43,12 +38,25 @@ func init() { if len(ch) == 0 { return nil, fmt.Errorf("channels required") } - allowed := make([]interface{}, len(ch)) - for i, c := range ch { - allowed[i] = c - } - rule := integrationplugins.CallRule{Path: "/api/chat.postMessage", Methods: map[string]integrationplugins.RequestConstraint{"POST": {Body: map[string]interface{}{"channel": allowed}}}} - return []integrationplugins.CallRule{rule}, nil + return channelRules(nil, ch), nil }, }) } + +func channelRules(base map[string]interface{}, channels []interface{}) []integrationplugins.CallRule { + rules := make([]integrationplugins.CallRule, 0, len(channels)) + for _, channel := range channels { + body := make(map[string]interface{}, len(base)+1) + for k, v := range base { + body[k] = v + } + body["channel"] = channel + rules = append(rules, integrationplugins.CallRule{ + Path: "/api/chat.postMessage", + Methods: map[string]integrationplugins.RequestConstraint{ + "POST": {Body: body}, + }, + }) + } + return rules +} diff --git a/app/integrations/plugins/slack/slack_test.go b/app/integrations/plugins/slack/slack_test.go index a2d9fb9b..9f32d5b8 100644 --- a/app/integrations/plugins/slack/slack_test.go +++ b/app/integrations/plugins/slack/slack_test.go @@ -1,7 +1,6 @@ package slack_test import ( - "reflect" "testing" integrationplugins "github.com/winhowes/AuthTranslator/app/integrations" @@ -54,17 +53,21 @@ func TestSlackCapabilities(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if len(rules) != 1 { - t.Fatalf("expected 1 rule, got %d", len(rules)) - } - rule = rules[0] - rc, ok = rule.Methods["POST"] - if !ok { - t.Fatalf("missing POST method") - } - chVal, ok := rc.Body["channel"].([]interface{}) - if !ok || !reflect.DeepEqual(chVal, []interface{}{"c1", "c2"}) { - t.Errorf("channels not propagated: %v", rc.Body["channel"]) + if len(rules) != 2 { + t.Fatalf("expected 2 rules, got %d", len(rules)) + } + for i, want := range []string{"c1", "c2"} { + rule = rules[i] + rc, ok = rule.Methods["POST"] + if !ok { + t.Fatalf("rule %d missing POST method", i) + } + if got := rc.Body["username"]; got != "bot" { + t.Errorf("rule %d username not propagated: %v", i, got) + } + if got := rc.Body["channel"]; got != want { + t.Errorf("rule %d channel not propagated: %v", i, got) + } } // missing fields should error @@ -91,9 +94,8 @@ func TestSlackCapabilities(t *testing.T) { if !ok { t.Fatalf("missing POST method") } - chVal, ok = rc.Body["channel"].([]interface{}) - if !ok || !reflect.DeepEqual(chVal, []interface{}{"c1"}) { - t.Errorf("channels not propagated: %v", rc.Body["channel"]) + if got := rc.Body["channel"]; got != "c1" { + t.Errorf("channels not propagated: %v", got) } // missing channels should error From b4e51943bec4ea3d7b5cb26c093753e4b706d78d Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Fri, 24 Apr 2026 17:29:18 -0700 Subject: [PATCH 3/4] allow constrained duplicate routes --- app/allowlist.go | 13 +++++----- app/allowlist_validate.go | 20 +++++++++++---- app/allowlist_validate_test.go | 46 ++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/app/allowlist.go b/app/allowlist.go index 5b4b5fb3..fef77a9a 100644 --- a/app/allowlist.go +++ b/app/allowlist.go @@ -57,17 +57,18 @@ func validateAllowlist(name string, callers []CallerConfig) error { } seenIDs[id] = struct{}{} - // track path+method combos to prevent duplicates - ruleSeen := make(map[string]map[string]struct{}) + // Track exact rule duplicates while allowing same route+method + // combinations with distinct request constraints. + ruleSeen := make(map[string]map[string][]RequestConstraint) for ri, r := range c.Rules { if ruleSeen[r.Path] == nil { - ruleSeen[r.Path] = make(map[string]struct{}) + ruleSeen[r.Path] = make(map[string][]RequestConstraint) } - for m := range r.Methods { - if _, dup := ruleSeen[r.Path][m]; dup { + for m, cons := range r.Methods { + if hasMatchingConstraint(ruleSeen[r.Path][m], cons) { return fmt.Errorf("duplicate rule for caller %q path %q method %s (index %d rule %d)", id, r.Path, m, ci, ri) } - ruleSeen[r.Path][m] = struct{}{} + ruleSeen[r.Path][m] = append(ruleSeen[r.Path][m], cons) } } } diff --git a/app/allowlist_validate.go b/app/allowlist_validate.go index 8172640a..d5cea908 100644 --- a/app/allowlist_validate.go +++ b/app/allowlist_validate.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "reflect" "strings" integrationplugins "github.com/winhowes/AuthTranslator/app/integrations" @@ -41,7 +42,7 @@ func validateAllowlistEntry(name string, callers []CallerConfig) error { if len(c.Rules) == 0 && len(c.Capabilities) == 0 { return fmt.Errorf("caller %q has no rules or capabilities", id) } - ruleSeen := make(map[string]map[string]struct{}) + ruleSeen := make(map[string]map[string][]RequestConstraint) for _, cap := range c.Capabilities { if err := validateCapability(name, cap); err != nil { return err @@ -56,18 +57,18 @@ func validateAllowlistEntry(name string, callers []CallerConfig) error { } normPath := strings.Join(splitPath(r.Path), "/") if ruleSeen[normPath] == nil { - ruleSeen[normPath] = make(map[string]struct{}) + ruleSeen[normPath] = make(map[string][]RequestConstraint) } - for m := range r.Methods { + for m, cons := range r.Methods { trimmed := strings.TrimSpace(m) if trimmed == "" { return fmt.Errorf("caller %q rule %d invalid method %q", id, ri, m) } upper := strings.ToUpper(trimmed) - if _, dup := ruleSeen[normPath][upper]; dup { + if hasMatchingConstraint(ruleSeen[normPath][upper], cons) { return fmt.Errorf("duplicate rule for caller %q path %q method %s", id, r.Path, upper) } - ruleSeen[normPath][upper] = struct{}{} + ruleSeen[normPath][upper] = append(ruleSeen[normPath][upper], cons) } } } @@ -130,3 +131,12 @@ func validateCapability(integration string, cap integrationplugins.CapabilityCon } return nil } + +func hasMatchingConstraint(existing []RequestConstraint, cons RequestConstraint) bool { + for _, prev := range existing { + if reflect.DeepEqual(prev, cons) { + return true + } + } + return false +} diff --git a/app/allowlist_validate_test.go b/app/allowlist_validate_test.go index d2748d38..4dfaee48 100644 --- a/app/allowlist_validate_test.go +++ b/app/allowlist_validate_test.go @@ -169,6 +169,52 @@ func TestValidateAllowlistEntriesDuplicateRule(t *testing.T) { } } +func TestValidateAllowlistEntriesAllowsSameRouteDifferentConstraints(t *testing.T) { + entries := []AllowlistEntry{{ + Integration: "test", + Callers: []CallerConfig{{ + ID: "c", + Rules: []CallRule{ + {Path: "/x", Methods: map[string]RequestConstraint{"POST": {Body: map[string]interface{}{"channel": "c1"}}}}, + {Path: "/x", Methods: map[string]RequestConstraint{"POST": {Body: map[string]interface{}{"channel": "c2"}}}}, + }, + }}, + }} + if err := validateAllowlistEntries(entries); err != nil { + t.Fatalf("unexpected error for distinct constraints: %v", err) + } +} + +func TestValidateAllowlistEntriesAllowsComposableCapabilities(t *testing.T) { + entries := []AllowlistEntry{{ + Integration: "slack", + Callers: []CallerConfig{{ + ID: "c", + Capabilities: []integrationplugins.CapabilityConfig{{ + Name: "post_channels", + Params: map[string]interface{}{"channels": []interface{}{"c1", "c2"}}, + }}, + }}, + }} + if err := validateAllowlistEntries(entries); err != nil { + t.Fatalf("unexpected slack capability error: %v", err) + } + + entries = []AllowlistEntry{{ + Integration: "monday", + Callers: []CallerConfig{{ + ID: "c", + Capabilities: []integrationplugins.CapabilityConfig{ + {Name: "create_item"}, + {Name: "update_status"}, + }, + }}, + }} + if err := validateAllowlistEntries(entries); err != nil { + t.Fatalf("unexpected monday capability error: %v", err) + } +} + func TestValidateAllowlistEntriesDuplicateNormalizedPath(t *testing.T) { entries := []AllowlistEntry{{ Integration: "test", From 9490d74924630853d027c1cb68cc6414bad52ad6 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Fri, 24 Apr 2026 18:04:30 -0700 Subject: [PATCH 4/4] try alternate route constraints --- app/allowlist.go | 76 ++++++++++++++++++++++++++++--------------- app/allowlist_test.go | 33 +++++++++++++++++++ app/main.go | 22 +++++-------- 3 files changed, 91 insertions(+), 40 deletions(-) diff --git a/app/allowlist.go b/app/allowlist.go index fef77a9a..e76504af 100644 --- a/app/allowlist.go +++ b/app/allowlist.go @@ -475,9 +475,7 @@ func toFloat(v interface{}) (float64, bool) { } } -// findConstraint returns the RequestConstraint for the given caller, path and -// method if one exists. -func findConstraint(i *Integration, callerID, pth, method string) (RequestConstraint, bool) { +func findConstraintCandidates(i *Integration, callerID, pth, method string) []RequestConstraint { segments := splitPath(pth) allowlists.RLock() @@ -489,39 +487,63 @@ func findConstraint(i *Integration, callerID, pth, method string) (RequestConstr allowlists.RUnlock() if ok { - if len(c.Capabilities) > 0 { - c = integrationplugins.ExpandCapabilities(i.Name, []CallerConfig{c})[0] - for ri := range c.Rules { - normalizeRule(&c.Rules[ri]) - } - } - for _, r := range c.Rules { - if matchSegments(r.Segments, segments) { - if m, ok := r.Methods[method]; ok { - return m, true - } - } + if candidates := constraintCandidatesForCaller(i.Name, c, segments, method); len(candidates) > 0 { + return candidates } // Identified callers with explicit entries should not fall back to wildcard // rules when their own rules do not match. if callerID != "*" && callerID != "" { - return RequestConstraint{}, false + return nil } } if hasWildcard && (!ok || callerID == "*" || callerID == "") { - if len(wildcard.Capabilities) > 0 { - wildcard = integrationplugins.ExpandCapabilities(i.Name, []CallerConfig{wildcard})[0] - for ri := range wildcard.Rules { - normalizeRule(&wildcard.Rules[ri]) - } + return constraintCandidatesForCaller(i.Name, wildcard, segments, method) + } + return nil +} + +func constraintCandidatesForCaller(integration string, c CallerConfig, segments []string, method string) []RequestConstraint { + if len(c.Capabilities) > 0 { + c = integrationplugins.ExpandCapabilities(integration, []CallerConfig{c})[0] + for ri := range c.Rules { + normalizeRule(&c.Rules[ri]) } - for _, r := range wildcard.Rules { - if matchSegments(r.Segments, segments) { - if m, ok := r.Methods[method]; ok { - return m, true - } + } + + var candidates []RequestConstraint + for _, r := range c.Rules { + if matchSegments(r.Segments, segments) { + if m, ok := r.Methods[method]; ok { + candidates = append(candidates, m) } } } - return RequestConstraint{}, false + return candidates +} + +// findConstraint returns the first RequestConstraint for the given caller, +// path and method if one exists. +func findConstraint(i *Integration, callerID, pth, method string) (RequestConstraint, bool) { + candidates := findConstraintCandidates(i, callerID, pth, method) + if len(candidates) == 0 { + return RequestConstraint{}, false + } + return candidates[0], true +} + +func findMatchingConstraint(i *Integration, callerID, pth, method string, r *http.Request) (RequestConstraint, bool, string) { + candidates := findConstraintCandidates(i, callerID, pth, method) + if len(candidates) == 0 { + return RequestConstraint{}, false, "" + } + + firstReason := "" + for _, cons := range candidates { + if ok, reason := validateRequestReason(r, cons); ok { + return cons, true, "" + } else if firstReason == "" { + firstReason = reason + } + } + return RequestConstraint{}, false, firstReason } diff --git a/app/allowlist_test.go b/app/allowlist_test.go index 8efd066b..ec5eebce 100644 --- a/app/allowlist_test.go +++ b/app/allowlist_test.go @@ -282,6 +282,39 @@ func TestSetAllowlistDuplicateRule(t *testing.T) { } } +func TestFindMatchingConstraintTriesAlternateConstraints(t *testing.T) { + allowlists.Lock() + allowlists.m = make(map[string]map[string]CallerConfig) + allowlists.Unlock() + + if _, ok, reason := findMatchingConstraint(&Integration{Name: "missing"}, "*", "/post", http.MethodPost, httptest.NewRequest(http.MethodPost, "http://missing/post", nil)); ok || reason != "" { + t.Fatalf("expected missing allowlist to have no candidate, got ok=%v reason=%q", ok, reason) + } + + if err := SetAllowlist("multi", []CallerConfig{{ + ID: "*", + Rules: []CallRule{ + {Path: "/post", Methods: map[string]RequestConstraint{"POST": {Body: map[string]interface{}{"channel": "c1"}}}}, + {Path: "/post", Methods: map[string]RequestConstraint{"POST": {Body: map[string]interface{}{"channel": "c2"}}}}, + }, + }}); err != nil { + t.Fatalf("failed to set allowlist: %v", err) + } + + integ := &Integration{Name: "multi"} + req := httptest.NewRequest(http.MethodPost, "http://multi/post", strings.NewReader(`{"channel":"c2"}`)) + req.Header.Set("Content-Type", "application/json") + if _, ok, reason := findMatchingConstraint(integ, "*", "/post", http.MethodPost, req); !ok { + t.Fatalf("expected second constraint to match, got reason %q", reason) + } + + req = httptest.NewRequest(http.MethodPost, "http://multi/post", strings.NewReader(`{"channel":"c3"}`)) + req.Header.Set("Content-Type", "application/json") + if _, ok, reason := findMatchingConstraint(integ, "*", "/post", http.MethodPost, req); ok || reason == "" { + t.Fatalf("expected all constraints to fail with a reason, got ok=%v reason=%q", ok, reason) + } +} + func TestSetAllowlistMethodNormalization(t *testing.T) { allowlists.Lock() allowlists.m = make(map[string]map[string]CallerConfig) diff --git a/app/main.go b/app/main.go index 865fe05c..371b8b27 100644 --- a/app/main.go +++ b/app/main.go @@ -1402,20 +1402,16 @@ func proxyHandler(w http.ResponseWriter, r *http.Request) { callers := GetAllowlist(integ.Name) if len(callers) > 0 { - cons, ok := findConstraint(integ, callerID, r.URL.Path, r.Method) + _, ok, reason := findMatchingConstraint(integ, callerID, r.URL.Path, r.Method, r) if !ok { - reason := "no allowlist match" - logger.Warn("request blocked", "integration", integ.Name, "caller_id", callerID, "reason", reason) - metrics.IncInternalResponse(integ.Name, http.StatusForbidden, internalReasonNoAllowlistMatch) - w.Header().Set("X-AT-Error-Reason", reason) - w.Header().Set("X-AT-Upstream-Error", "false") - w.Header().Set("Content-Type", "text/plain; charset=utf-8") - http.Error(w, fmt.Sprintf("Forbidden: %s", reason), http.StatusForbidden) - return - } - if ok2, reason := validateRequestReason(r, cons); !ok2 { - logger.Warn("request failed constraints", "integration", integ.Name, "caller_id", callerID, "reason", reason) - metrics.IncInternalResponse(integ.Name, http.StatusForbidden, internalReasonConstraintFailure) + if reason == "" { + reason = "no allowlist match" + logger.Warn("request blocked", "integration", integ.Name, "caller_id", callerID, "reason", reason) + metrics.IncInternalResponse(integ.Name, http.StatusForbidden, internalReasonNoAllowlistMatch) + } else { + logger.Warn("request failed constraints", "integration", integ.Name, "caller_id", callerID, "reason", reason) + metrics.IncInternalResponse(integ.Name, http.StatusForbidden, internalReasonConstraintFailure) + } w.Header().Set("X-AT-Error-Reason", reason) w.Header().Set("X-AT-Upstream-Error", "false") w.Header().Set("Content-Type", "text/plain; charset=utf-8")