Skip to content

Commit 12bc883

Browse files
leo-aa88cursoragent
leo-aa88
and
cursoragent
committed
feat(policy): derive tool approval from safety when rules omit tool
Add Derive/EffectiveToolDecision and consult safety metadata in CheckToolCall after explicit approvals.requiredFor checks. Unattended mutating tools without policy or trusted metadata return exit code 5. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 527ba32 commit 12bc883

7 files changed

Lines changed: 268 additions & 12 deletions

File tree

‎internal/policy/approvals.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,8 @@ func checkApprovalGranted(uses string, approvals *spec.PolicyApprovals, approved
2323
if !approvalRequired(uses, approvals) {
2424
return nil
2525
}
26-
u := strings.TrimSpace(uses)
27-
for _, a := range approved {
28-
if strings.TrimSpace(a) == u {
29-
return nil
30-
}
26+
if actionApproved(uses, approved) {
27+
return nil
3128
}
3229
return denied(
3330
ReasonApprovalRequired,

‎internal/policy/derive.go‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package policy
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
7+
"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
8+
"github.com/LAA-Software-Engineering/agentic-control-plane/internal/tools"
9+
)
10+
11+
// Decision is the effective policy outcome for a tool call when explicit Policy rules do not apply.
12+
type Decision string
13+
14+
const (
15+
// DecisionAllow permits unattended execution.
16+
DecisionAllow Decision = "allow"
17+
// DecisionRequireApproval blocks until the full uses string is passed via --approve.
18+
DecisionRequireApproval Decision = "requireApproval"
19+
// DecisionDeny hard-blocks execution (explicit denylist only; metadata fallback uses requireApproval).
20+
DecisionDeny Decision = "deny"
21+
)
22+
23+
// DecisionSource explains why a [Decision] was chosen (plan risk surfacing, issue #103).
24+
type DecisionSource string
25+
26+
const (
27+
SourceExplicitPolicyRule DecisionSource = "explicit_policy_rule"
28+
SourceSafetyMetadata DecisionSource = "safety_metadata"
29+
SourceFailClosedDefault DecisionSource = "fail_closed_default"
30+
)
31+
32+
// ToolDecision bundles an effective decision and its provenance for a tool resource.
33+
type ToolDecision struct {
34+
Decision Decision
35+
Source DecisionSource
36+
Safety spec.ResolvedToolSafety
37+
}
38+
39+
// Derive maps resolved safety metadata to a fallback decision (issue #103 truth table).
40+
// Metadata fallback never returns [DecisionDeny]; use explicit policy denylists for hard denies.
41+
func Derive(safety spec.ResolvedToolSafety) Decision {
42+
if safety.RequiresApproval {
43+
return DecisionRequireApproval
44+
}
45+
return DecisionAllow
46+
}
47+
48+
// EffectiveToolDecision returns the decision for toolName under policy pol (explicit rules first).
49+
func EffectiveToolDecision(graph *spec.ProjectGraph, pol *spec.PolicySpec, toolName string) ToolDecision {
50+
toolName = strings.TrimSpace(toolName)
51+
safety := resolvedSafetyForTool(graph, toolName)
52+
if pol != nil && pol.Approvals != nil {
53+
prefix := "tool." + toolName + "."
54+
for _, r := range pol.Approvals.RequiredFor {
55+
r = strings.TrimSpace(r)
56+
if r == prefix || strings.HasPrefix(r, prefix) {
57+
return ToolDecision{
58+
Decision: DecisionRequireApproval,
59+
Source: SourceExplicitPolicyRule,
60+
Safety: safety,
61+
}
62+
}
63+
}
64+
}
65+
src := SourceSafetyMetadata
66+
if graph == nil || graph.Tools == nil {
67+
src = SourceFailClosedDefault
68+
} else if tr, ok := graph.Tools[toolName]; !ok || tr == nil || tr.Spec.Safety == nil {
69+
src = SourceFailClosedDefault
70+
}
71+
return ToolDecision{
72+
Decision: Derive(safety),
73+
Source: src,
74+
Safety: safety,
75+
}
76+
}
77+
78+
func resolvedSafetyForTool(graph *spec.ProjectGraph, toolName string) spec.ResolvedToolSafety {
79+
if graph == nil || graph.Tools == nil {
80+
return spec.ResolveToolSafety(nil)
81+
}
82+
tr, ok := graph.Tools[toolName]
83+
if !ok || tr == nil {
84+
return spec.ResolveToolSafety(nil)
85+
}
86+
return spec.ResolveToolSafety(tr.Spec.Safety)
87+
}
88+
89+
func checkSafetyDerived(graph *spec.ProjectGraph, call ToolCallContext) error {
90+
toolName, _, err := tools.ParseUses(call.Uses)
91+
if err != nil {
92+
return denied(ReasonInvalidUses, fmt.Sprintf("policy: %v", err), call.Uses, nil)
93+
}
94+
td := EffectiveToolDecision(graph, nil, toolName)
95+
switch td.Decision {
96+
case DecisionAllow:
97+
return nil
98+
case DecisionRequireApproval:
99+
if actionApproved(call.Uses, call.Run.ApprovedActions) {
100+
return nil
101+
}
102+
return denied(
103+
ReasonApprovalRequired,
104+
"policy: tool requires approval from safety metadata (--approve)",
105+
call.Uses,
106+
map[string]any{
107+
"tool": toolName,
108+
"source": string(td.Source),
109+
},
110+
)
111+
default:
112+
return denied(
113+
ReasonDenied,
114+
"policy: tool denied by safety metadata",
115+
call.Uses,
116+
map[string]any{"tool": toolName},
117+
)
118+
}
119+
}
120+
121+
func actionApproved(uses string, approved []string) bool {
122+
u := strings.TrimSpace(uses)
123+
for _, a := range approved {
124+
if strings.TrimSpace(a) == u {
125+
return true
126+
}
127+
}
128+
return false
129+
}

‎internal/policy/derive_test.go‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package policy
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
8+
)
9+
10+
func boolPtr(b bool) *bool { v := b; return &v }
11+
12+
func TestDerive_truthTable(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
in spec.ResolvedToolSafety
16+
want Decision
17+
}{
18+
{"trusted_no_explicit_approval", spec.ResolvedToolSafety{Trusted: true, SideEffects: true, RequiresApproval: false}, DecisionAllow},
19+
{"untrusted_read_only", spec.ResolvedToolSafety{Trusted: false, SideEffects: false, RequiresApproval: false}, DecisionAllow},
20+
{"untrusted_mutating", spec.ResolvedToolSafety{Trusted: false, SideEffects: true, RequiresApproval: true}, DecisionRequireApproval},
21+
{"explicit_requires_approval", spec.ResolvedToolSafety{Trusted: true, SideEffects: false, RequiresApproval: true}, DecisionRequireApproval},
22+
}
23+
for _, tt := range tests {
24+
t.Run(tt.name, func(t *testing.T) {
25+
if got := Derive(tt.in); got != tt.want {
26+
t.Fatalf("Derive() = %q want %q", got, tt.want)
27+
}
28+
})
29+
}
30+
}
31+
32+
func TestCheckToolCall_safetyFallback_requiresApprovalWithoutApprove(t *testing.T) {
33+
g := testGraphWithTools("slack")
34+
g.Tools["slack"].Spec.Safety = &spec.ToolSafety{Trusted: boolPtr(false), SideEffects: boolPtr(true)}
35+
ev := NewEvaluator(g, nil)
36+
err := ev.CheckToolCall(context.Background(), ToolCallContext{
37+
Run: RunContext{},
38+
Uses: "tool.slack.message.send",
39+
})
40+
if err == nil {
41+
t.Fatal("expected denial")
42+
}
43+
d, ok := AsDenied(err)
44+
if !ok || d.Reason != ReasonApprovalRequired {
45+
t.Fatalf("got %v", err)
46+
}
47+
}
48+
49+
func TestCheckToolCall_safetyFallback_trustedAllows(t *testing.T) {
50+
g := testGraphWithTools("slack")
51+
g.Tools["slack"].Spec.Safety = &spec.ToolSafety{Trusted: boolPtr(true)}
52+
ev := NewEvaluator(g, nil)
53+
err := ev.CheckToolCall(context.Background(), ToolCallContext{
54+
Run: RunContext{},
55+
Uses: "tool.slack.message.send",
56+
})
57+
if err != nil {
58+
t.Fatal(err)
59+
}
60+
}
61+
62+
func TestCheckToolCall_safetyFallback_approveGrants(t *testing.T) {
63+
g := testGraphWithTools("slack")
64+
ev := NewEvaluator(g, nil)
65+
err := ev.CheckToolCall(context.Background(), ToolCallContext{
66+
Run: RunContext{ApprovedActions: []string{"tool.slack.message.send"}},
67+
Uses: "tool.slack.message.send",
68+
})
69+
if err != nil {
70+
t.Fatal(err)
71+
}
72+
}
73+
74+
func TestCheckToolCall_explicitPolicyRuleBeforeSafety(t *testing.T) {
75+
g := testGraphWithTools("github")
76+
g.Tools["github"].Spec.Safety = &spec.ToolSafety{Trusted: boolPtr(true)}
77+
pol := &spec.PolicySpec{
78+
Approvals: &spec.PolicyApprovals{
79+
RequiredFor: []string{"tool.github.pull_request.merge"},
80+
},
81+
}
82+
ev := NewEvaluator(g, pol)
83+
err := ev.CheckToolCall(context.Background(), ToolCallContext{
84+
Run: RunContext{},
85+
Uses: "tool.github.pull_request.merge",
86+
})
87+
if err == nil {
88+
t.Fatal("expected explicit policy approval denial")
89+
}
90+
d, ok := AsDenied(err)
91+
if !ok || d.Reason != ReasonApprovalRequired {
92+
t.Fatalf("got %v", err)
93+
}
94+
// trusted safety would allow, but explicit rule wins
95+
err = ev.CheckToolCall(context.Background(), ToolCallContext{
96+
Run: RunContext{},
97+
Uses: "tool.github.pull_request.get",
98+
})
99+
if err != nil {
100+
t.Fatalf("trusted tool without explicit rule should allow: %v", err)
101+
}
102+
}
103+
104+
func TestEffectiveToolDecision_explicitVsDerived(t *testing.T) {
105+
g := testGraphWithTools("github")
106+
g.Tools["github"].Spec.Safety = &spec.ToolSafety{Trusted: boolPtr(true)}
107+
pol := &spec.PolicySpec{
108+
Approvals: &spec.PolicyApprovals{
109+
RequiredFor: []string{"tool.github.pull_request.merge"},
110+
},
111+
}
112+
td := EffectiveToolDecision(g, pol, "github")
113+
if td.Decision != DecisionRequireApproval || td.Source != SourceExplicitPolicyRule {
114+
t.Fatalf("prefix rule: %+v", td)
115+
}
116+
td = EffectiveToolDecision(g, nil, "github")
117+
if td.Decision != DecisionAllow || td.Source != SourceSafetyMetadata {
118+
t.Fatalf("trusted metadata: %+v", td)
119+
}
120+
}

‎internal/policy/doc.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,7 @@
44
// Use [NewEngine] with a loaded [spec.ProjectGraph], then [Engine.Evaluator] or [Engine.EvaluatorForSpec].
55
// Run context should carry elapsed wall clock, accumulated cost, and repeated --approve action strings
66
// matching policy approvals.requiredFor entries.
7+
//
8+
// When no explicit approvals.requiredFor rule matches a tool call, [Derive] consults
9+
// [spec.ResolveToolSafety] metadata (fail-closed defaults; issue #103).
710
package policy

‎internal/policy/errors.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const (
1313
ReasonUnknownTool = "unknown_tool"
1414
ReasonApprovalRequired = "approval_required"
1515
ReasonInvalidUses = "invalid_uses"
16+
ReasonDenied = "denied"
1617
)
1718

1819
// DeniedError is returned when a policy check fails (design doc section 12.2 H).

‎internal/policy/evaluator.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,13 @@ func (e *evaluator) CheckStep(ctx context.Context, step StepContext) error {
5252
func (e *evaluator) CheckToolCall(ctx context.Context, call ToolCallContext) error {
5353
_ = ctx
5454
p := e.spec()
55-
if p == nil {
56-
return nil
57-
}
58-
if err := checkKnownTool(e.graph, call.Uses, p.Tools); err != nil {
59-
return err
55+
if p != nil {
56+
if err := checkKnownTool(e.graph, call.Uses, p.Tools); err != nil {
57+
return err
58+
}
59+
if approvalRequired(call.Uses, p.Approvals) {
60+
return checkApprovalGranted(call.Uses, p.Approvals, call.Run.ApprovedActions)
61+
}
6062
}
61-
return checkApprovalGranted(call.Uses, p.Approvals, call.Run.ApprovedActions)
63+
return checkSafetyDerived(e.graph, call)
6264
}

‎internal/policy/evaluator_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,16 @@ import (
1414

1515
func testGraphWithTools(names ...string) *spec.ProjectGraph {
1616
tools := make(map[string]*spec.ToolResource)
17+
trusted := true
1718
for _, n := range names {
1819
tools[n] = &spec.ToolResource{
1920
APIVersion: spec.APIVersionV0,
2021
Kind: spec.KindTool,
2122
Metadata: spec.Metadata{Name: n},
22-
Spec: spec.ToolSpec{Type: "mock"},
23+
Spec: spec.ToolSpec{
24+
Type: "mock",
25+
Safety: &spec.ToolSafety{Trusted: &trusted},
26+
},
2327
}
2428
}
2529
return &spec.ProjectGraph{Tools: tools}

0 commit comments

Comments
 (0)