diff --git a/internal/cmdpolicy/engine.go b/internal/cmdpolicy/engine.go index d46a39bcd..c2e7e0162 100644 --- a/internal/cmdpolicy/engine.go +++ b/internal/cmdpolicy/engine.go @@ -121,16 +121,16 @@ func (e *Engine) EvaluateOne(cmd *cobra.Command) Decision { return Decision{ Allowed: false, ReasonCode: "risk_not_annotated", - Reason: "command has no risk_level annotation; required when a Rule is active (set rule.allow_unannotated=true to opt out during gradual adoption)", + Reason: "command has no risk_level annotation; rule denies unannotated commands", } } // Axis 1: Deny has priority. - if matchesAny(r.Deny, path) { + if matched, ok := firstMatch(r.Deny, path); ok { return Decision{ Allowed: false, ReasonCode: "command_denylisted", - Reason: "command denied by rule deny list", + Reason: fmt.Sprintf("command path %q matched deny pattern %q", path, matched), } } @@ -139,7 +139,7 @@ func (e *Engine) EvaluateOne(cmd *cobra.Command) Decision { return Decision{ Allowed: false, ReasonCode: "domain_not_allowed", - Reason: "command path not in rule allow list", + Reason: fmt.Sprintf("command path %q not in allow list %v", path, r.Allow), } } @@ -151,7 +151,7 @@ func (e *Engine) EvaluateOne(cmd *cobra.Command) Decision { return Decision{ Allowed: false, ReasonCode: reasonCodeForRisk(cmdRisk), - Reason: "command risk exceeds rule max_risk", + Reason: fmt.Sprintf("command risk %q exceeds rule max_risk %q", cmdRisk, r.MaxRisk), } } } @@ -163,7 +163,7 @@ func (e *Engine) EvaluateOne(cmd *cobra.Command) Decision { return Decision{ Allowed: false, ReasonCode: "identity_mismatch", - Reason: "command identities do not intersect rule identities", + Reason: fmt.Sprintf("command supports identities %v; rule allows %v", cmdIdents, r.Identities), } } } @@ -348,12 +348,20 @@ func reasonCodeForRisk(risk platform.Risk) string { // Invalid globs are skipped here -- they're rejected upstream by // ValidateRule when the rule first enters the system. func matchesAny(globs []string, path string) bool { + _, ok := firstMatch(globs, path) + return ok +} + +// firstMatch returns the first glob in globs that matches path. Used by +// command_denylisted so the envelope can name the specific deny pattern +// that fired. +func firstMatch(globs []string, path string) (string, bool) { for _, g := range globs { if ok, err := doublestar.Match(g, path); err == nil && ok { - return true + return g, true } } - return false + return "", false } // hasIdentityIntersection reports whether the rule's typed identities diff --git a/internal/cmdpolicy/engine_test.go b/internal/cmdpolicy/engine_test.go index c102b58fd..3d35d3647 100644 --- a/internal/cmdpolicy/engine_test.go +++ b/internal/cmdpolicy/engine_test.go @@ -324,6 +324,64 @@ func TestEvaluate_identitiesIntersection(t *testing.T) { } } +// Reason strings must carry both the attempted value and the rule's +// constraint so the envelope is self-contained for AI consumers. +// Asserting on substrings (not exact match) leaves room for minor wording +// tweaks while pinning the value-carrying behaviour. +func TestEvaluate_reasonCarriesAttemptAndConstraint(t *testing.T) { + root := buildTree() + + cases := []struct { + name string + rule *platform.Rule + path string + wantInReason []string + }{ + { + name: "identity_mismatch surfaces both identity sets", + rule: &platform.Rule{Identities: []platform.Identity{"bot"}}, + path: "docs/+update", // identities=[user] + wantInReason: []string{"[user]", "[bot]"}, + }, + { + name: "domain_not_allowed surfaces path and allow list", + rule: &platform.Rule{Allow: []string{"docs/**"}}, + path: "im/+send", + wantInReason: []string{`"im/+send"`, "docs/**"}, + }, + { + name: "command_denylisted surfaces matched deny pattern", + rule: &platform.Rule{Deny: []string{"docs/+delete-*"}}, + path: "docs/+delete-doc", + wantInReason: []string{`"docs/+delete-doc"`, `"docs/+delete-*"`}, + }, + { + name: "risk_too_high surfaces cmd risk and max_risk", + rule: &platform.Rule{MaxRisk: "write"}, + path: "docs/+delete-doc", // risk=high-risk-write + wantInReason: []string{`"high-risk-write"`, `"write"`}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := cmdpolicy.New(tc.rule).EvaluateAll(root) + d, ok := got[tc.path] + if !ok { + t.Fatalf("no decision for %q", tc.path) + } + if d.Allowed { + t.Fatalf("%q should have been denied", tc.path) + } + for _, sub := range tc.wantInReason { + if !strings.Contains(d.Reason, sub) { + t.Errorf("reason %q missing %q", d.Reason, sub) + } + } + }) + } +} + // Unknown identities defaults to ALLOW. A command with risk annotated // but without supportedIdentities passes any identity filter. func TestEvaluate_unknownIdentitiesIsAllow(t *testing.T) {