feat(spec,policy): tool safety metadata + fail-closed policy derivation

CHANGELOG.md

@@ -0,0 +1,33 @@
+# Changelog
+
+All notable changes to this project are documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
+
+## [Unreleased]
+
+### Added
+
+- **`spec.safety` on Tool resources** (issue #103): optional `trusted`, `sideEffects`, and `requiresApproval` fields. [NormalizeProjectGraph] materializes fail-closed defaults on load.
+- **Policy safety fallback**: when no `approvals.requiredFor` entry matches the exact `uses` string, [policy.Derive] consults resolved safety metadata. Unattended mutating tools require `--approve` (exit code **5**, `approval_required`).
+- **Plan risk hints** for tools that will require approval at run, including decision source (`explicit_policy_rule`, `safety_metadata`, `fail_closed_default`).
+
+### Changed
+
+- **Breaking — tool calls without explicit policy are no longer unrestricted.** Previously, `CheckToolCall` with a nil [spec.PolicySpec] allowed all tools. Now fail-closed safety always applies from the project graph (even when the workflow omits `spec.policy` or the Policy resource is missing).
+- Tools with **no** `spec.safety` block behave as **untrusted with side effects** after normalization → require `--approve` unless an explicit `approvals.requiredFor` rule matches.
+
+### Migration
+
+1. For **read-only** native or mock tools (echo, fetch, identity), add:
+   ```yaml
+   spec:
+     safety:
+       sideEffects: false
+   ```
+2. For tools where you accept **tool-wide** unattended use but still gate specific operations, set `trusted: true` and list write operations under `Policy.spec.approvals.requiredFor` (exact `uses` strings).
+3. Do **not** set `trusted: true` unless you intend every operation on that tool to run without safety-derived approval; per-action gating remains `requiredFor` only (exact match at runtime).
+
+### Not yet wired
+
+- MCP discovery does **not** yet apply [spec.SafetyFromMCPMeta] / [spec.MergeToolSafety]; author-set `spec.safety` in YAML is the source of truth until MCP merge lands (tracked separately from #103).

CONTRIBUTING.md

@@ -22,6 +22,8 @@ Run **`make`** or **`make help`** for a full list of targets.
 
 ## Before you open a pull request
 
+User-visible behavior changes should include an entry under **[Unreleased]** in [`CHANGELOG.md`](CHANGELOG.md) (especially breaking changes and migrations).
+
 1. **Format** — `make fmt` or ensure `gofmt -l .` prints nothing (same check as CI).
 2. **Static analysis** — `make vet` (or `go vet ./...`).
 3. **Tests** — `make test` (`go test ./... -race`).

examples/example1/tools/helper.yaml

@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false

examples/pr-review-demo/tools/github.yaml

@@ -4,3 +4,5 @@ metadata:
   name: github
 spec:
   type: native
+  safety:
+    trusted: true

examples/pr-review-github-actions/tools/github.yaml

@@ -4,3 +4,5 @@ metadata:
   name: github
 spec:
   type: native
+  safety:
+    trusted: true

examples/pr-review-github/tools/github.yaml

@@ -4,3 +4,5 @@ metadata:
   name: github
 spec:
   type: native
+  safety:
+    trusted: true

internal/cli/initembed/tools/helper.yaml

@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false

internal/cli/run_test.go

@@ -29,6 +29,16 @@ func runPolicyRoot(t *testing.T) string {
 	return abs
 }
 
+func runSafetyRoot(t *testing.T) string {
+	t.Helper()
+	p := filepath.Join("testdata", "run_safety")
+	abs, err := filepath.Abs(p)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return abs
+}
+
 func TestRun_demo_integration_succeeds(t *testing.T) {
 	db := filepath.Join(t.TempDir(), "run-cli.db")
 	root := runProjRoot(t)
@@ -54,6 +64,53 @@ func TestRun_demo_integration_succeeds(t *testing.T) {
 	}
 }
 
+func TestRun_safetyOnlyDenial_exit5(t *testing.T) {
+	db := filepath.Join(t.TempDir(), "run-safety.db")
+	root := runSafetyRoot(t)
+
+	ResetGlobalsForTest()
+	cmd := NewRootCmd()
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	cmd.SetArgs([]string{
+		"run", "workflow/echo",
+		"--project", root,
+		"--state", db,
+		"--input", "topic=x",
+	})
+	err := cmd.Execute()
+	if err == nil {
+		t.Fatal("expected safety-derived policy denial")
+	}
+	if ExitCodeOf(err) != ExitPolicyDenied {
+		t.Fatalf("exit=%d want %d err=%v", ExitCodeOf(err), ExitPolicyDenied, err)
+	}
+}
+
+func TestRun_safetyOnly_withApprove_succeeds(t *testing.T) {
+	db := filepath.Join(t.TempDir(), "run-safety-ok.db")
+	root := runSafetyRoot(t)
+
+	ResetGlobalsForTest()
+	var out bytes.Buffer
+	cmd := NewRootCmd()
+	cmd.SetOut(&out)
+	cmd.SetErr(&out)
+	cmd.SetArgs([]string{
+		"run", "workflow/echo",
+		"--project", root,
+		"--state", db,
+		"--input", "topic=x",
+		"--approve", "tool.helper.echo",
+	})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(out.String(), "Status: succeeded") {
+		t.Fatalf("output:\n%s", out.String())
+	}
+}
+
 func TestRun_policyDenial_exit5(t *testing.T) {
 	db := filepath.Join(t.TempDir(), "run-pol.db")
 	root := runPolicyRoot(t)

internal/cli/testdata/fmt_messy/tool.yaml

@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false

internal/cli/testdata/plan_project/tool.yaml

@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false

internal/cli/testdata/run_policy/tools.yaml

@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false

internal/cli/testdata/run_safety/project.yaml

@@ -0,0 +1,12 @@
+apiVersion: agentic.dev/v0
+kind: Project
+metadata:
+  name: safetyproj
+spec:
+  imports:
+    - ./tools.yaml
+    - ./workflow.yaml
+  providers:
+    models:
+      mock:
+        type: mock

internal/cli/testdata/run_safety/schemas/in.json

@@ -0,0 +1,9 @@
+{
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "type": "object",
+  "required": ["topic"],
+  "properties": {
+    "topic": { "type": "string" }
+  },
+  "additionalProperties": true
+}

internal/cli/testdata/run_safety/tools.yaml

@@ -0,0 +1,6 @@
+apiVersion: agentic.dev/v0
+kind: Tool
+metadata:
+  name: helper
+spec:
+  type: native

internal/cli/testdata/run_safety/workflow.yaml

@@ -0,0 +1,15 @@
+apiVersion: agentic.dev/v0
+kind: Workflow
+metadata:
+  name: echo
+spec:
+  input:
+    schema: ./schemas/in.json
+  steps:
+    - id: s1
+      uses: tool.helper.echo
+      with:
+        topic: "${input.topic}"
+  output:
+    value:
+      out: "${steps.s1.output.echo}"

internal/cli/testdata/validate_ok/tool.yaml

@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false

internal/cli/testdata/wf_tests/tool.yaml

@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false

internal/engine/execution_test.go

@@ -47,7 +47,10 @@ func TestRun_sequentialToolAndAgent_mockModel(t *testing.T) {
 				APIVersion: spec.APIVersionV0,
 				Kind:       spec.KindTool,
 				Metadata:   spec.Metadata{Name: "helper"},
-				Spec:       spec.ToolSpec{Type: "native"},
+				Spec: spec.ToolSpec{
+					Type:   "native",
+					Safety: &spec.ToolSafety{SideEffects: spec.BoolPtr(false)},
+				},
 			},
 		},
 		Agents: map[string]*spec.AgentResource{

internal/plan/planner.go

@@ -106,7 +106,7 @@ func (p *Planner) ComputePlan(ctx context.Context, env string, g *spec.ProjectGr
 
 	sortOperations(ops)
 
-	risk := summarizeRisks(appliedByID, desiredByID, ops)
+	risk := summarizeRisks(g, appliedByID, desiredByID, ops)
 	fp, err := DeploymentStateFingerprint(ctx, p.Deploy, env, projectName)
 	if err != nil {
 		return nil, err

internal/plan/risk.go

@@ -6,6 +6,7 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/policy"
 	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
 	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/state"
 )
@@ -47,13 +48,19 @@ type toolSpecRisk struct {
 	Permissions *struct {
 		Allow []string `json:"allow"`
 	} `json:"permissions"`
+	Safety *struct {
+		Trusted          *bool `json:"trusted"`
+		SideEffects      *bool `json:"sideEffects"`
+		RequiresApproval *bool `json:"requiresApproval"`
+	} `json:"safety"`
 }
 
 type jsonEnvelope struct {
 	Spec json.RawMessage `json:"spec"`
 }
 
 func summarizeRisks(
+	g *spec.ProjectGraph,
 	appliedByID map[string]state.AppliedResource,
 	desiredByID map[string]desiredRow,
 	ops []Operation,
@@ -88,7 +95,7 @@ func summarizeRisks(
 		case spec.KindAgent:
 			summarizeAgentRisk(add, op, oldJSON, des.json, hadPrev)
 		case spec.KindTool:
-			summarizeToolRisk(add, op, oldJSON, des.json, hadPrev)
+			summarizeToolRisk(add, g, op, oldJSON, des.json, hadPrev)
 		}
 	}
 
@@ -156,12 +163,13 @@ func summarizeAgentRisk(add func(string), op Operation, oldJSON, newJSON string,
 	}
 }
 
-func summarizeToolRisk(add func(string), op Operation, oldJSON, newJSON string, hadPrev bool) {
+func summarizeToolRisk(add func(string), g *spec.ProjectGraph, op Operation, oldJSON, newJSON string, hadPrev bool) {
 	newTool, ok := parseToolSpec(newJSON)
 	if !ok {
 		return
 	}
 	newAllows := toolAllows(newTool)
+	newDecision := toolPlanDecisionFromGraph(g, op.Target.Name)
 
 	if op.Action == ActionCreate || !hadPrev {
 		for _, a := range newAllows {
@@ -170,6 +178,7 @@ func summarizeToolRisk(add func(string), op Operation, oldJSON, newJSON string,
 				break
 			}
 		}
+		addToolSafetyRisk(add, op.Target.Name, newDecision, nil)
 		return
 	}
 
@@ -195,6 +204,67 @@ func summarizeToolRisk(add func(string), op Operation, oldJSON, newJSON string,
 			break
 		}
 	}
+	oldDecision := toolDecisionFromParsed(g, op.Target.Name, oldTool)
+	addToolSafetyRisk(add, op.Target.Name, newDecision, &oldDecision)
+}
+
+func toolPlanDecisionFromGraph(g *spec.ProjectGraph, toolName string) policy.ToolDecision {
+	if g != nil {
+		for _, pr := range g.Policies {
+			if pr == nil {
+				continue
+			}
+			td := policy.EffectiveToolDecision(g, &pr.Spec, toolName)
+			if td.Decision == policy.DecisionRequireApproval {
+				return td
+			}
+		}
+	}
+	return policy.EffectiveToolDecision(g, nil, toolName)
+}
+
+func toolDecisionFromParsed(g *spec.ProjectGraph, toolName string, parsed *toolSpecRisk) policy.ToolDecision {
+	if g != nil {
+		for _, pr := range g.Policies {
+			if pr == nil {
+				continue
+			}
+			td := policy.EffectiveToolDecision(g, &pr.Spec, toolName)
+			if td.Decision == policy.DecisionRequireApproval {
+				return td
+			}
+		}
+	}
+	var safety *spec.ToolSafety
+	src := policy.SourceFailClosedDefault
+	if parsed != nil && parsed.Safety != nil {
+		safety = &spec.ToolSafety{
+			Trusted:          parsed.Safety.Trusted,
+			SideEffects:      parsed.Safety.SideEffects,
+			RequiresApproval: parsed.Safety.RequiresApproval,
+		}
+		src = policy.SourceSafetyMetadata
+	}
+	resolved := spec.ResolveToolSafety(safety)
+	return policy.ToolDecision{
+		Decision: policy.Derive(resolved),
+		Source:   src,
+		Safety:   resolved,
+	}
+}
+
+func addToolSafetyRisk(add func(string), toolName string, cur policy.ToolDecision, prev *policy.ToolDecision) {
+	if cur.Decision != policy.DecisionRequireApproval {
+		return
+	}
+	if prev != nil && prev.Decision == policy.DecisionRequireApproval {
+		return
+	}
+	// Plan uses prefix match on tool.<name>. for explicit requiredFor (conservative); runtime matches exact uses.
+	add(fmt.Sprintf(
+		"Tool/%s will require approval at run (decision=%s, source=%s).",
+		toolName, cur.Decision, cur.Source,
+	))
 }
 
 func parsePolicySpec(resourceJSON string) (*policySpecRisk, bool) {