[codex] fix plugin capability definitions

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)
 			}
 		}
 	}
@@ -474,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()
@@ -488,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
 }

app/allowlist_body_filter_test.go

@@ -53,6 +53,15 @@ func TestBodyArrayMatching(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 fail an array rule")
+	}
+}
+
 func TestBodyObjectMatching(t *testing.T) {
 	body := []byte(`{"foo":"bar","num":1,"extra":true}`)
 	tests := []struct {

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)
@@ -483,6 +516,12 @@ func TestMatchValueNotOkBranches(t *testing.T) {
 	}
 }
 
+func TestMatchValueArrayRuleRejectsScalarValue(t *testing.T) {
+	if matchValue("C123", []interface{}{"C123", "C456"}) {
+		t.Fatal("expected scalar value to fail an array rule")
+	}
+}
+
 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)

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)
 			}
 		}
 	}
@@ -122,8 +123,20 @@ 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)
 	}
 	return nil
 }
+
+func hasMatchingConstraint(existing []RequestConstraint, cons RequestConstraint) bool {
+	for _, prev := range existing {
+		if reflect.DeepEqual(prev, cons) {
+			return true
+		}
+	}
+	return false
+}

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",
@@ -261,6 +307,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",

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
 		},
 	})
 }