feat(spec,policy): tool safety metadata + fail-closed policy derivation#124
Merged
Conversation
Introduce spec.safety (trusted, sideEffects, requiresApproval) with ResolveToolSafety defaults, NormalizeProjectGraph materialization, and MCP meta.mcp_flags mapping helpers for issue #103. Co-authored-by: Cursor <cursoragent@cursor.com>
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>
Surface effective decision and source in plan risk summaries. Update fixtures with explicit safety on demo and test tools so read-only native helpers and trusted GitHub demos keep expected run behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
ReviewGate [WARN]
|
Automated reviewSummaryEnhances tool safety metadata and policy derivation mechanisms with significant changes. Findings
|
- Fix NewEvaluator/Engine.Evaluator godoc for nil-policy safety enforcement - Add CHANGELOG [Unreleased] with breaking-change migration guide - Document tool-level trusted vs requiredFor; plan prefix vs runtime exact match - Mark MCP SafetyFromMCPMeta/MergeToolSafety as not wired yet - Validate non-empty spec.safety blocks; export spec.BoolPtr - Add run_safety CLI fixture: exit 5 from safety only (no Policy) - Revert testGraphWithTools to production fail-closed defaults - Add prefix vs exact approval tests; CONTRIBUTING CHANGELOG note Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #103: tools self-describe blast radius via
spec.safety, and the policy layer derives approval requirements when no explicitPolicyrule matches.internal/spec:ToolSafetyblock (trusted,sideEffects,requiresApproval), fail-closedResolveToolSafety,NormalizeProjectGraphmaterialization, andSafetyFromMCPMeta/MergeToolSafetyfor MCPmeta.mcp_flagsprecedence.internal/policy:Derivetruth table,CheckToolCallsafety fallback (exit code 5 /approval_requiredwhen unattended), explicitapprovals.requiredForremains authoritative.internal/plan: risk summary flags tools that will require approval with decision + source (explicit_policy_rule,safety_metadata,fail_closed_default).sideEffects: false; GitHub demo tools settrusted: trueso policy can still gate write operations.Test plan
make ci(verify-fmt, vet,go test -race ./...)ResolveToolSafetytruth table,Derive, policy evaluator safety fallback, plan risk safety messagesrun_policyexit 5, pr-review-demo blockspost_comment, demo/hello workflows succeed with updated safety metadataMade with Cursor