Skip to content

Commit 743c2f2

Browse files
authored
Merge pull request #126 from LAA-Software-Engineering/feat/policy-presets-104
feat(policy): built-in policy presets (strict | permissive | shell_safe)
2 parents 848451d + 46b4467 commit 743c2f2

21 files changed

Lines changed: 1216 additions & 11 deletions

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
88

99
### Added
1010

11+
- **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.
12+
- **`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.**
1113
- **`spec.safety` on Tool resources** (issue #103): optional `trusted`, `sideEffects`, and `requiresApproval` fields. [NormalizeProjectGraph] materializes fail-closed defaults on load.
1214
- **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`).
1315
- **Plan risk hints** for tools that will require approval at run, including decision source (`explicit_policy_rule`, `safety_metadata`, `fail_closed_default`).

internal/cli/init_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"os"
66
"path/filepath"
77
"testing"
8+
9+
"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
810
)
911

1012
func TestInit_thenValidateSucceeds(t *testing.T) {
@@ -35,6 +37,34 @@ func TestInit_thenValidateSucceeds(t *testing.T) {
3537
}
3638
}
3739

40+
func TestInit_defaultPolicyExpandsShellSafePreset(t *testing.T) {
41+
parent := t.TempDir()
42+
name := "shellsafe"
43+
44+
ResetGlobalsForTest()
45+
cmd := NewRootCmd()
46+
cmd.SetOut(io.Discard)
47+
cmd.SetErr(io.Discard)
48+
cmd.SetArgs([]string{"init", name, "--parent-dir", parent})
49+
if err := cmd.Execute(); err != nil {
50+
t.Fatal(err)
51+
}
52+
53+
ResetGlobalsForTest()
54+
g := &Global{ProjectRoot: filepath.Join(parent, name)}
55+
graph, _, err := prepareProjectGraph(g.ProjectRoot, g)
56+
if err != nil {
57+
t.Fatal(err)
58+
}
59+
pr, ok := graph.Policies["default"]
60+
if !ok || pr == nil {
61+
t.Fatal("expected default policy")
62+
}
63+
if pr.Spec.ResolvedPreset != spec.PresetShellSafe {
64+
t.Fatalf("default policy ResolvedPreset = %q want %s", pr.Spec.ResolvedPreset, spec.PresetShellSafe)
65+
}
66+
}
67+
3868
func TestInit_rejectsExistingDir(t *testing.T) {
3969
parent := t.TempDir()
4070
name := "dup"

internal/cli/initembed/policies/default.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ kind: Policy
33
metadata:
44
name: default
55
spec:
6+
preset: shell_safe
67
execution:
78
maxWallClockSeconds: 300
89
maxTotalCostUsd: 5

internal/engine/steps.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func parseAgentJSONObject(content string) (map[string]any, error) {
4646

4747
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) {
4848
uses := strings.TrimSpace(step.Uses)
49-
if err := pol.CheckToolCall(ctx, policy.ToolCallContext{Run: pctx, StepID: step.ID, Uses: uses}); err != nil {
49+
if err := pol.CheckToolCall(ctx, policy.ToolCallContext{Run: pctx, StepID: step.ID, Uses: uses, With: with}); err != nil {
5050
if e.Trace != nil {
5151
if d, ok := policy.AsDenied(err); ok {
5252
_, _ = e.Trace.Append(ctx, runID, step.ID, trace.EventPolicyDenied, d.TraceData())

internal/policy/context.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,6 @@ type ToolCallContext struct {
2525
Run RunContext
2626
StepID string
2727
Uses string
28+
// With is the interpolated workflow step input (used by shell_safe token classification).
29+
With map[string]any
2830
}

internal/policy/derive.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,32 @@ func Derive(safety spec.ResolvedToolSafety) Decision {
4949
func EffectiveToolDecision(graph *spec.ProjectGraph, pol *spec.PolicySpec, toolName string) ToolDecision {
5050
toolName = strings.TrimSpace(toolName)
5151
safety := resolvedSafetyForTool(graph, toolName)
52+
if pol != nil && spec.ResolvedPresetName(pol) == spec.PresetShellSafe {
53+
// shell_safe plan risk is tool-granular (conservative); runtime uses per-command classification.
54+
if safety.RequiresApproval || safety.SideEffects {
55+
return ToolDecision{
56+
Decision: DecisionRequireApproval,
57+
Source: SourceExplicitPolicyRule,
58+
Safety: safety,
59+
}
60+
}
61+
}
62+
if pol != nil && pol.Approvals != nil {
63+
if spec.ApprovalPermissive(pol.Approvals) {
64+
return ToolDecision{
65+
Decision: DecisionAllow,
66+
Source: SourceExplicitPolicyRule,
67+
Safety: safety,
68+
}
69+
}
70+
if spec.ApprovalRequireAllTools(pol.Approvals) {
71+
return ToolDecision{
72+
Decision: DecisionRequireApproval,
73+
Source: SourceExplicitPolicyRule,
74+
Safety: safety,
75+
}
76+
}
77+
}
5278
if pol != nil && pol.Approvals != nil {
5379
prefix := toolUsesPrefix(toolName)
5480
for _, r := range pol.Approvals.RequiredFor {

internal/policy/doc.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55
// Run context should carry elapsed wall clock, accumulated cost, and repeated --approve action strings
66
// matching policy approvals.requiredFor entries.
77
//
8+
// Built-in policy presets (issue #104): strict, permissive, and shell_safe. Select via
9+
// Project.spec.defaults.policy, a Policy resource spec.preset, or by referencing a preset name
10+
// as the workflow/agent policy. [spec.ExpandPresetsInGraph] materializes effective rules during normalize.
11+
//
12+
// shell_safe uses first-token heuristics plus metacharacter fail-closed checks — not a sandbox.
13+
// Plan risk for shell_safe is tool-granular (conservative); runtime applies per-command classification.
14+
//
815
// When no explicit approvals.requiredFor rule matches a tool call, [Derive] consults
916
// [spec.ResolveToolSafety] metadata (fail-closed defaults; issue #103).
1017
//

internal/policy/evaluator.go

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,50 @@ func (e *evaluator) CheckToolCall(ctx context.Context, call ToolCallContext) err
5858
if err := checkKnownTool(e.graph, call.Uses, p.Tools); err != nil {
5959
return err
6060
}
61-
if approvalRequired(call.Uses, p.Approvals) {
62-
return checkApprovalGranted(call.Uses, p.Approvals, call.Run.ApprovedActions)
61+
if p.Approvals != nil && spec.ApprovalPermissive(p.Approvals) {
62+
return nil
6363
}
6464
}
65+
switch {
66+
case p != nil && spec.ResolvedPresetName(p) == spec.PresetShellSafe:
67+
needApproval := shellSafeRequiresApproval(e.graph, call) || approvalRequired(call.Uses, p.Approvals)
68+
if needApproval {
69+
if actionApproved(call.Uses, call.Run.ApprovedActions) {
70+
return nil
71+
}
72+
return toolCallApprovalDenied(call, p)
73+
}
74+
return nil
75+
case requiresToolCallApproval(e.graph, p, call):
76+
if actionApproved(call.Uses, call.Run.ApprovedActions) {
77+
return nil
78+
}
79+
return toolCallApprovalDenied(call, p)
80+
}
81+
if p != nil && approvalRequired(call.Uses, p.Approvals) {
82+
return checkApprovalGranted(call.Uses, p.Approvals, call.Run.ApprovedActions)
83+
}
6584
return checkSafetyDerived(e.graph, call)
6685
}
86+
87+
func requiresToolCallApproval(graph *spec.ProjectGraph, pol *spec.PolicySpec, call ToolCallContext) bool {
88+
if pol == nil || pol.Approvals == nil {
89+
return false
90+
}
91+
return spec.ApprovalRequireAllTools(pol.Approvals)
92+
}
93+
94+
func toolCallApprovalDenied(call ToolCallContext, pol *spec.PolicySpec) error {
95+
extra := map[string]any{"requiredFor": call.Uses}
96+
if pol != nil {
97+
if preset := spec.ResolvedPresetName(pol); preset != "" {
98+
extra["preset"] = preset
99+
}
100+
}
101+
return denied(
102+
ReasonApprovalRequired,
103+
"policy: action requires explicit approval (--approve)",
104+
call.Uses,
105+
extra,
106+
)
107+
}

0 commit comments

Comments
 (0)