diff --git a/pkg/intent/intent_formal_test.go b/pkg/intent/intent_formal_test.go index 52472fd3e24..270a8c4e187 100644 --- a/pkg/intent/intent_formal_test.go +++ b/pkg/intent/intent_formal_test.go @@ -392,3 +392,76 @@ func TestFormal_AllowedToolsIntersection(t *testing.T) { "P12: intersection with deny-all must yield deny-all") }) } + +// TestFormal_SeedRuleEnumValidation (P13 — SeedEnumsValidated) +// Invariant: an unrecognized Autonomy or WriteScope on the rule that seeds the +// accumulator must not be carried into the compiled policy; it falls back to the +// safest default for that field (fail-closed). +func TestFormal_SeedRuleEnumValidation(t *testing.T) { + repo := intent.RepositoryContext{Owner: "owner", Name: "repo"} + // Use a mapped record (labels required for non-unlinked status). + rec := matchingResolver().ResolvePullRequest(intent.PullRequestData{ + ClosingIssues: []intent.RootReference{ + {NodeID: "I_1", Labels: []string{"security"}}, + }, + }) + + t.Run("typo_in_seed_rule_falls_back_to_safest", func(t *testing.T) { + rule := intent.PolicyRule{ + ID: "typo", + Set: intent.ExecutionPolicy{ + Autonomy: "boundeed", // typo for "bounded" + WriteScope: "any_branchh", // typo for "any_branch" + }, + } + compiler := intent.PolicyCompiler{Rules: []intent.PolicyRule{rule}} + policy := compiler.Compile(rec, repo) + + assert.Equal(t, "propose_only", policy.Autonomy, + "P13: unrecognized autonomy must fall back to the safest default") + assert.Equal(t, "none", policy.WriteScope, + "P13: unrecognized write scope must fall back to the safest default") + }) + + t.Run("valid_seed_values_preserved", func(t *testing.T) { + rule := intent.PolicyRule{ + ID: "valid", + Set: intent.ExecutionPolicy{Autonomy: "bounded", WriteScope: "any_branch"}, + } + compiler := intent.PolicyCompiler{Rules: []intent.PolicyRule{rule}} + policy := compiler.Compile(rec, repo) + + assert.Equal(t, "bounded", policy.Autonomy, "P13: recognized autonomy must be preserved") + assert.Equal(t, "any_branch", policy.WriteScope, "P13: recognized write scope must be preserved") + }) + + t.Run("unset_seed_values_remain_unspecified", func(t *testing.T) { + rule := intent.PolicyRule{ + ID: "unset", + Set: intent.ExecutionPolicy{MaxAttempts: 3}, + } + compiler := intent.PolicyCompiler{Rules: []intent.PolicyRule{rule}} + policy := compiler.Compile(rec, repo) + + assert.Empty(t, policy.Autonomy, "P13: unset autonomy must stay unspecified for later merges") + assert.Empty(t, policy.WriteScope, "P13: unset write scope must stay unspecified for later merges") + }) + + t.Run("invalid_seed_cannot_be_relaxed_by_later_rule", func(t *testing.T) { + seed := intent.PolicyRule{ + ID: "typo-seed", + Set: intent.ExecutionPolicy{Autonomy: "boundeed", WriteScope: "any_branchh"}, + } + later := intent.PolicyRule{ + ID: "permissive", + Set: intent.ExecutionPolicy{Autonomy: "bounded", WriteScope: "any_branch"}, + } + compiler := intent.PolicyCompiler{Rules: []intent.PolicyRule{seed, later}} + policy := compiler.Compile(rec, repo) + + assert.Equal(t, "propose_only", policy.Autonomy, + "P13: sanitized seed must not be relaxed by a later permissive rule") + assert.Equal(t, "none", policy.WriteScope, + "P13: sanitized seed must not be relaxed by a later permissive rule") + }) +} diff --git a/pkg/intent/policy.go b/pkg/intent/policy.go index 444c09c464f..7af77942d38 100644 --- a/pkg/intent/policy.go +++ b/pkg/intent/policy.go @@ -101,8 +101,10 @@ type PolicyCompiler struct { // ExecutionPolicy. Unlinked and ambiguous records always receive the safest // policy regardless of configured rules (fail-closed). For all other statuses // the first matching rule seeds the accumulator directly; subsequent matching -// rules are merged with stricter-wins semantics. If no rules match, the safest -// default policy is returned. +// rules are merged with stricter-wins semantics. Autonomy and WriteScope values +// that are not recognized by the rank tables are replaced with the safest default +// for that field when seeding. If no rules match, the safest default policy is +// returned. func (c PolicyCompiler) Compile(rec IntentRecord, repo RepositoryContext) ExecutionPolicy { policyLog.Printf("Compiling policy: status=%s rules=%d", rec.Status, len(c.Rules)) // Fail-closed for indeterminate statuses: unlinked and ambiguous records @@ -123,10 +125,10 @@ func (c PolicyCompiler) Compile(rec IntentRecord, repo RepositoryContext) Execut // rule's policy so that permissive values (e.g. auto_merge: true, // max_attempts: 5) are not silently discarded by the safest-default // base, and so that pointer/slice fields cannot alias rule.Set. - accumulated = deepCopyPolicy(rule.Set) + accumulated = sanitizeSeedPolicy(deepCopyPolicy(rule.Set), rule.ID) accumulated.RuleIDs = []string{rule.ID} matched = true - policyLog.Printf("First matching rule: id=%s autonomy=%s write_scope=%s", rule.ID, rule.Set.Autonomy, rule.Set.WriteScope) + policyLog.Printf("First matching rule: id=%s autonomy=%s write_scope=%s", rule.ID, accumulated.Autonomy, accumulated.WriteScope) } else { accumulated = mergePolicy(accumulated, rule.Set) accumulated.RuleIDs = append(accumulated.RuleIDs, rule.ID) @@ -141,6 +143,26 @@ func (c PolicyCompiler) Compile(rec IntentRecord, repo RepositoryContext) Execut return accumulated } +// sanitizeSeedPolicy validates the Autonomy and WriteScope values of the policy +// that seeds the accumulator in Compile. Because the seed is copied verbatim (it +// is not merged through the rank tables), an unrecognized value would otherwise +// be carried into the compiled policy untouched and rank as 0 (least restrictive) +// in every later comparison — a fail-open outcome. Any value that is not present +// in the rank tables is replaced with the safest default for that field. An empty +// value means "unspecified" and is left as-is, matching mergePolicy's semantics. +func sanitizeSeedPolicy(p ExecutionPolicy, ruleID string) ExecutionPolicy { + safest := safestDefaultPolicy() + if _, ok := autonomyRank[p.Autonomy]; !ok && p.Autonomy != "" { + policyLog.Printf("Rule %s has unrecognized autonomy %q, falling back to %s", ruleID, p.Autonomy, safest.Autonomy) + p.Autonomy = safest.Autonomy + } + if _, ok := writeScopeRank[p.WriteScope]; !ok && p.WriteScope != "" { + policyLog.Printf("Rule %s has unrecognized write scope %q, falling back to %s", ruleID, p.WriteScope, safest.WriteScope) + p.WriteScope = safest.WriteScope + } + return p +} + // safestDefaultPolicy returns the most restrictive execution policy: propose-only, // no write scope, human approval required, auto-merge denied, and a single attempt. func safestDefaultPolicy() ExecutionPolicy { diff --git a/specs/intent-attribution-compliance/explicit-intent-wins.yaml b/specs/intent-attribution-compliance/explicit-intent-wins.yaml index 57ff9ae5b54..73d6dc72d35 100644 --- a/specs/intent-attribution-compliance/explicit-intent-wins.yaml +++ b/specs/intent-attribution-compliance/explicit-intent-wins.yaml @@ -21,7 +21,7 @@ expected: status: mapped intent_key: security policy: - autonomy: bounded_autonomous - write_scope: limited + autonomy: bounded + write_scope: feature_branch human_approval_required: false auto_merge_allowed: true