Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

### Added

- **Built-in policy presets** (issue #104): `strict`, `permissive`, and `shell_safe`. Select via `Project.spec.defaults.policy`, by referencing a preset name on agents/workflows, or with `Policy.spec.preset` (local rules layer on top). Presets expand during [NormalizeProjectGraph]; `strict`/`permissive` materialize approval flags, while `shell_safe` sets `ResolvedPreset` and relies on runtime token classification plus tool safety metadata for plan risk.
- **`shell_safe` token classification** for native `command.run` / `run` / `exec` / `shell` operations: read-only first tokens (`ls`, `cat`, …) run unattended when the command contains no shell metacharacters (`;|&$`, newlines, `` ` ``, `$(…)`); risky tokens, unknown tokens, and side-effecting non-shell tools require `--approve`. **Heuristic only — not a sandbox.**
- **`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`).
Expand Down
30 changes: 30 additions & 0 deletions internal/cli/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"os"
"path/filepath"
"testing"

"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
)

func TestInit_thenValidateSucceeds(t *testing.T) {
Expand Down Expand Up @@ -35,6 +37,34 @@ func TestInit_thenValidateSucceeds(t *testing.T) {
}
}

func TestInit_defaultPolicyExpandsShellSafePreset(t *testing.T) {
parent := t.TempDir()
name := "shellsafe"

ResetGlobalsForTest()
cmd := NewRootCmd()
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
cmd.SetArgs([]string{"init", name, "--parent-dir", parent})
if err := cmd.Execute(); err != nil {
t.Fatal(err)
}

ResetGlobalsForTest()
g := &Global{ProjectRoot: filepath.Join(parent, name)}
graph, _, err := prepareProjectGraph(g.ProjectRoot, g)
if err != nil {
t.Fatal(err)
}
pr, ok := graph.Policies["default"]
if !ok || pr == nil {
t.Fatal("expected default policy")
}
if pr.Spec.ResolvedPreset != spec.PresetShellSafe {
t.Fatalf("default policy ResolvedPreset = %q want %s", pr.Spec.ResolvedPreset, spec.PresetShellSafe)
}
}

func TestInit_rejectsExistingDir(t *testing.T) {
parent := t.TempDir()
name := "dup"
Expand Down
1 change: 1 addition & 0 deletions internal/cli/initembed/policies/default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ kind: Policy
metadata:
name: default
spec:
preset: shell_safe
execution:
maxWallClockSeconds: 300
maxTotalCostUsd: 5
2 changes: 1 addition & 1 deletion internal/engine/steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func parseAgentJSONObject(content string) (map[string]any, error) {

func (e *Executor) runToolStep(ctx context.Context, pol policy.PolicyEvaluator, runID string, step spec.WorkflowStep, with map[string]any, pctx policy.RunContext) (map[string]any, tools.ToolCallMeta, error) {
uses := strings.TrimSpace(step.Uses)
if err := pol.CheckToolCall(ctx, policy.ToolCallContext{Run: pctx, StepID: step.ID, Uses: uses}); err != nil {
if err := pol.CheckToolCall(ctx, policy.ToolCallContext{Run: pctx, StepID: step.ID, Uses: uses, With: with}); err != nil {
if e.Trace != nil {
if d, ok := policy.AsDenied(err); ok {
_, _ = e.Trace.Append(ctx, runID, step.ID, trace.EventPolicyDenied, d.TraceData())
Expand Down
2 changes: 2 additions & 0 deletions internal/policy/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,6 @@ type ToolCallContext struct {
Run RunContext
StepID string
Uses string
// With is the interpolated workflow step input (used by shell_safe token classification).
With map[string]any
}
26 changes: 26 additions & 0 deletions internal/policy/derive.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,32 @@ func Derive(safety spec.ResolvedToolSafety) Decision {
func EffectiveToolDecision(graph *spec.ProjectGraph, pol *spec.PolicySpec, toolName string) ToolDecision {
toolName = strings.TrimSpace(toolName)
safety := resolvedSafetyForTool(graph, toolName)
if pol != nil && spec.ResolvedPresetName(pol) == spec.PresetShellSafe {
// shell_safe plan risk is tool-granular (conservative); runtime uses per-command classification.
if safety.RequiresApproval || safety.SideEffects {
return ToolDecision{
Decision: DecisionRequireApproval,
Source: SourceExplicitPolicyRule,
Safety: safety,
}
}
}
if pol != nil && pol.Approvals != nil {
if spec.ApprovalPermissive(pol.Approvals) {
return ToolDecision{
Decision: DecisionAllow,
Source: SourceExplicitPolicyRule,
Safety: safety,
}
}
if spec.ApprovalRequireAllTools(pol.Approvals) {
return ToolDecision{
Decision: DecisionRequireApproval,
Source: SourceExplicitPolicyRule,
Safety: safety,
}
}
}
if pol != nil && pol.Approvals != nil {
prefix := toolUsesPrefix(toolName)
for _, r := range pol.Approvals.RequiredFor {
Expand Down
7 changes: 7 additions & 0 deletions internal/policy/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
// Run context should carry elapsed wall clock, accumulated cost, and repeated --approve action strings
// matching policy approvals.requiredFor entries.
//
// Built-in policy presets (issue #104): strict, permissive, and shell_safe. Select via
// Project.spec.defaults.policy, a Policy resource spec.preset, or by referencing a preset name
// as the workflow/agent policy. [spec.ExpandPresetsInGraph] materializes effective rules during normalize.
//
// shell_safe uses first-token heuristics plus metacharacter fail-closed checks — not a sandbox.
// Plan risk for shell_safe is tool-granular (conservative); runtime applies per-command classification.
//
// When no explicit approvals.requiredFor rule matches a tool call, [Derive] consults
// [spec.ResolveToolSafety] metadata (fail-closed defaults; issue #103).
//
Expand Down
45 changes: 43 additions & 2 deletions internal/policy/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,50 @@ func (e *evaluator) CheckToolCall(ctx context.Context, call ToolCallContext) err
if err := checkKnownTool(e.graph, call.Uses, p.Tools); err != nil {
return err
}
if approvalRequired(call.Uses, p.Approvals) {
return checkApprovalGranted(call.Uses, p.Approvals, call.Run.ApprovedActions)
if p.Approvals != nil && spec.ApprovalPermissive(p.Approvals) {
return nil
}
}
switch {
case p != nil && spec.ResolvedPresetName(p) == spec.PresetShellSafe:
needApproval := shellSafeRequiresApproval(e.graph, call) || approvalRequired(call.Uses, p.Approvals)
if needApproval {
if actionApproved(call.Uses, call.Run.ApprovedActions) {
return nil
}
return toolCallApprovalDenied(call, p)
}
return nil
case requiresToolCallApproval(e.graph, p, call):
if actionApproved(call.Uses, call.Run.ApprovedActions) {
return nil
}
return toolCallApprovalDenied(call, p)
}
if p != nil && approvalRequired(call.Uses, p.Approvals) {
return checkApprovalGranted(call.Uses, p.Approvals, call.Run.ApprovedActions)
}
return checkSafetyDerived(e.graph, call)
}

func requiresToolCallApproval(graph *spec.ProjectGraph, pol *spec.PolicySpec, call ToolCallContext) bool {
if pol == nil || pol.Approvals == nil {
return false
}
return spec.ApprovalRequireAllTools(pol.Approvals)
}

func toolCallApprovalDenied(call ToolCallContext, pol *spec.PolicySpec) error {
extra := map[string]any{"requiredFor": call.Uses}
if pol != nil {
if preset := spec.ResolvedPresetName(pol); preset != "" {
extra["preset"] = preset
}
}
return denied(
ReasonApprovalRequired,
"policy: action requires explicit approval (--approve)",
call.Uses,
extra,
)
}
Loading
Loading