From 059ed215162edae2c536f0d0da4d7df40c7c4d39 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:14:33 +0000 Subject: [PATCH 1/4] Initial plan From 9b79bb35e2c584410a6ad90de41e3ab1e17ff75a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:35:38 +0000 Subject: [PATCH 2/4] Implement SPDD intent and OTel contract updates Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- Makefile | 4 +- actions/setup/js/emit_outcome_spans.cjs | 4 +- actions/setup/js/emit_outcome_spans.test.cjs | 4 +- actions/setup/js/otel_contract.test.cjs | 96 ++++++++++ pkg/cli/mcp_intent_authorization.go | 167 ++++++++++++++++++ pkg/cli/mcp_intent_authorization_test.go | 64 +++++++ pkg/cli/mcp_server.go | 3 + pkg/intent/authz/authorizer.go | 65 +++++++ pkg/intent/authz/authorizer_test.go | 75 ++++++++ pkg/intent/policy.go | 13 +- .../otel_observability_formal_test.go | 38 ++++ specs/intent-attribution-agent-governance.md | 36 ++-- specs/otel-observability-spec.md | 7 + specs/replace-label-compliance/README.md | 10 ++ specs/replace-label-spec.md | 12 ++ 15 files changed, 574 insertions(+), 24 deletions(-) create mode 100644 pkg/cli/mcp_intent_authorization.go create mode 100644 pkg/cli/mcp_intent_authorization_test.go create mode 100644 pkg/intent/authz/authorizer.go create mode 100644 pkg/intent/authz/authorizer_test.go diff --git a/Makefile b/Makefile index f170557ccf8..b647f548d1d 100644 --- a/Makefile +++ b/Makefile @@ -1057,8 +1057,8 @@ validate-registry: # raw OTLP JSONL mirrors, and shipped GenAI compatibility attributes. .PHONY: validate-otel-contract validate-otel-contract: - @echo "Validating gh-aw OpenTelemetry compatibility contract..." - @go test ./pkg/parser ./pkg/workflow -run 'TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_OTLP(CustomAttributes|ResourceAttributes|GitHubAppImplicitOIDC)|TestInjectOTLPConfig|TestApplyTraceContextEnvToMap' -count=1 + @echo "Validating gh-aw OpenTelemetry compatibility contract (T-OT-001 through T-OT-011)..." + @go test ./pkg/parser ./pkg/workflow -run 'TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_OTLP(CustomAttributes|ResourceAttributes|GitHubAppImplicitOIDC)|TestInjectOTLPConfig|TestApplyTraceContextEnvToMap|TestFormal_OTelComplianceRuntimeContractSuiteIncludesLevel1IDs' -count=1 @cd actions/setup/js && npm run test:js -- otel_contract.test.cjs send_otlp_span.test.cjs --no-file-parallelism >/dev/null @echo "✓ OpenTelemetry compatibility contract validated" diff --git a/actions/setup/js/emit_outcome_spans.cjs b/actions/setup/js/emit_outcome_spans.cjs index 49d6fd0b74e..46d62893035 100644 --- a/actions/setup/js/emit_outcome_spans.cjs +++ b/actions/setup/js/emit_outcome_spans.cjs @@ -14,7 +14,7 @@ require("./shim.cjs"); * for per-workflow, per-type, and per-result drill-down. * * Span naming: - * - Per-item: gh-aw.outcome.evaluation + * - Per-item: gh-aw.outcome.evaluate * - Summary: gh-aw.outcome.summary * * Errors are non-fatal: export failures must never break the workflow. @@ -201,7 +201,7 @@ async function main() { traceId, spanId: generateSpanId(), parentSpanId: summarySpanId, - spanName: "gh-aw.outcome.evaluation", + spanName: "gh-aw.outcome.evaluate", startMs: evalEndMs - 1, // point-in-time span endMs: evalEndMs, attributes, diff --git a/actions/setup/js/emit_outcome_spans.test.cjs b/actions/setup/js/emit_outcome_spans.test.cjs index 29f0fb0ac0c..bf9a1be1831 100644 --- a/actions/setup/js/emit_outcome_spans.test.cjs +++ b/actions/setup/js/emit_outcome_spans.test.cjs @@ -285,14 +285,14 @@ describe("emit_outcome_spans.cjs", () => { ); expect(spans[1]).toEqual( expect.objectContaining({ - spanName: "gh-aw.outcome.evaluation", + spanName: "gh-aw.outcome.evaluate", parentSpanId: summarySpan.spanId, statusCode: 1, }) ); expect(spans[2]).toEqual( expect.objectContaining({ - spanName: "gh-aw.outcome.evaluation", + spanName: "gh-aw.outcome.evaluate", parentSpanId: summarySpan.spanId, statusCode: 0, }) diff --git a/actions/setup/js/otel_contract.test.cjs b/actions/setup/js/otel_contract.test.cjs index d7775101d3c..6f24407ab69 100644 --- a/actions/setup/js/otel_contract.test.cjs +++ b/actions/setup/js/otel_contract.test.cjs @@ -2,9 +2,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import fs from "fs"; const { sendJobSetupSpan, sendJobConclusionSpan, OTEL_JSONL_PATH } = await import("./send_otlp_span.cjs"); +const { main: emitOutcomeSpans } = await import("./emit_outcome_spans.cjs"); const MANAGED_ENV_VARS = [ "GH_AW_OTLP_ENDPOINTS", + "OTEL_EXPORTER_OTLP_ENDPOINT", "INPUT_JOB_NAME", "INPUT_TRACE_ID", "INPUT_PARENT_SPAN_ID", @@ -41,6 +43,10 @@ function firstSpan(payload) { return payload.resourceSpans[0].scopeSpans[0].spans[0]; } +function allSpans(payload) { + return payload.resourceSpans.flatMap(rs => rs.scopeSpans.flatMap(ss => ss.spans)); +} + describe("gh-aw OpenTelemetry compatibility contract", () => { let appendFileSyncSpy; let mkdirSyncSpy; @@ -55,6 +61,7 @@ describe("gh-aw OpenTelemetry compatibility contract", () => { } process.env.GH_AW_OTLP_ENDPOINTS = JSON.stringify([{ url: "https://traces.example.com" }]); + process.env.OTEL_EXPORTER_OTLP_ENDPOINT = "https://traces.example.com"; process.env.OTEL_SERVICE_NAME = "gh-aw.customer-contract"; process.env.GH_AW_INFO_WORKFLOW_NAME = "Customer OTEL Contract"; process.env.GITHUB_RUN_ID = "1234567890"; @@ -83,6 +90,21 @@ describe("gh-aw OpenTelemetry compatibility contract", () => { if (filePath === "/tmp/gh-aw/agent_output.json") { return JSON.stringify({ items: [], errors: [] }); } + if (filePath === "/tmp/gh-aw/outcome-evaluations.jsonl") { + return ( + JSON.stringify({ + type: "replace_label", + result: "accepted", + outcome_status: "accepted", + workflow: "replace-label", + run_id: 1234567890, + repo: "github/gh-aw", + }) + "\n" + ); + } + if (filePath === "/tmp/gh-aw/outcome-summary.json") { + return JSON.stringify({ total_outcomes: 1, accepted: 1, rejected: 0, pending: 0, ignored: 0 }); + } throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); }); @@ -154,4 +176,78 @@ describe("gh-aw OpenTelemetry compatibility contract", () => { expect(payload).not.toHaveProperty("payload"); } }); + + it("T-OT-008: emits gh-aw.job.name on built-in setup spans", async () => { + process.env.INPUT_JOB_NAME = "agent"; + + await sendJobSetupSpan({ + traceId: "a".repeat(32), + parentSpanId: "b".repeat(16), + startMs: 1_700_000_000_000, + }); + + const setupSpan = firstSpan(JSON.parse(String(appendFileSyncSpy.mock.calls[0][1]).trim())); + expect(setupSpan.name).toBe("gh-aw.agent.setup"); + expect(attrsByKey(setupSpan)["gh-aw.job.name"]).toBe("agent"); + }); + + it("T-OT-009: emits normalized gen_ai.system on built-in agent spans", async () => { + process.env.INPUT_JOB_NAME = "agent"; + + const setup = await sendJobSetupSpan({ + traceId: "a".repeat(32), + parentSpanId: "b".repeat(16), + startMs: 1_700_000_000_000, + }); + process.env.GITHUB_AW_OTEL_TRACE_ID = setup.traceId; + process.env.GITHUB_AW_OTEL_PARENT_SPAN_ID = setup.spanId; + + await sendJobConclusionSpan("gh-aw.agent.conclusion", { startMs: 1_700_000_001_000 }); + + const spans = appendFileSyncSpy.mock.calls.flatMap(([, line]) => allSpans(JSON.parse(String(line).trim()))); + const agentSpan = spans.find(span => span.name === "gh-aw.agent.agent"); + expect(agentSpan).toBeTruthy(); + expect(attrsByKey(agentSpan)["gen_ai.system"]).toBe("anthropic"); + }); + + it("T-OT-010: emits gh-aw.outcome.type on outcome-evaluation spans", async () => { + await emitOutcomeSpans(); + + const payload = JSON.parse(String(appendFileSyncSpy.mock.calls[0][1]).trim()); + const outcomeSpan = allSpans(payload).find(span => span.name === "gh-aw.outcome.evaluate"); + expect(outcomeSpan).toBeTruthy(); + const attrs = attrsByKey(outcomeSpan); + expect(attrs["gh-aw.outcome.type"]).toBe("replace_label"); + expect(attrs["gh-aw.outcome.result"]).toBe("accepted"); + }); + + it("T-OT-011: preserves v0.3.0 built-in span names and attribute inventory", async () => { + process.env.INPUT_JOB_NAME = "agent"; + + const setup = await sendJobSetupSpan({ + traceId: "a".repeat(32), + parentSpanId: "b".repeat(16), + startMs: 1_700_000_000_000, + }); + process.env.GITHUB_AW_OTEL_TRACE_ID = setup.traceId; + process.env.GITHUB_AW_OTEL_PARENT_SPAN_ID = setup.spanId; + await sendJobConclusionSpan("gh-aw.agent.conclusion", { startMs: 1_700_000_001_000 }); + + const spans = appendFileSyncSpy.mock.calls.flatMap(([, line]) => allSpans(JSON.parse(String(line).trim()))); + expect(spans.map(span => span.name)).toEqual(["gh-aw.agent.setup", "gh-aw.agent.agent", "gh-aw.agent.conclusion"]); + + const setupAttrs = attrsByKey(spans[0]); + for (const key of ["gh-aw.workflow.name", "gh-aw.job.name", "gh-aw.run.id", "gh-aw.repository", "gh-aw.engine.id", "gen_ai.system"]) { + expect(setupAttrs).toHaveProperty(key); + } + + const resourceAttrs = attrsByKey({ attributes: appendFileSyncSpy.mock.calls.map(([, line]) => JSON.parse(String(line).trim()))[0].resourceSpans[0].resource.attributes }); + for (const key of ["github.repository", "github.run_id", "github.run_attempt", "github.event_name", "github.job"]) { + expect(resourceAttrs).toHaveProperty(key); + } + + const agentAttrs = attrsByKey(spans[1]); + expect(agentAttrs).toHaveProperty("gen_ai.usage.total_tokens"); + expect(process.env.OTEL_EXPORTER_OTLP_ENDPOINT).toBe("https://traces.example.com"); + }); }); diff --git a/pkg/cli/mcp_intent_authorization.go b/pkg/cli/mcp_intent_authorization.go new file mode 100644 index 00000000000..16e45331198 --- /dev/null +++ b/pkg/cli/mcp_intent_authorization.go @@ -0,0 +1,167 @@ +package cli + +import ( + "context" + "encoding/json" + "errors" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + + "github.com/github/gh-aw/pkg/intent" + "github.com/github/gh-aw/pkg/intent/authz" + "github.com/github/gh-aw/pkg/logger" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +const ( + intentPolicyEnforcementEnv = "GH_AW_INTENT_POLICY_ENFORCEMENT" + intentPolicyPathEnv = "GH_AW_INTENT_POLICY_PATH" + intentLabelsEnv = "GH_AW_INTENT_LABELS" + intentHumanApprovedEnv = "GH_AW_INTENT_HUMAN_APPROVED" + intentPassedChecksEnv = "GH_AW_INTENT_REQUIRED_CHECKS_PASSED" + intentAttemptEnv = "GH_AW_INTENT_ATTEMPT" +) + +var mcpIntentAuthzLog = logger.New("mcp:intent_authorization") + +type intentPolicyFile struct { + Rules []intent.PolicyRule `json:"rules"` +} + +func intentPolicyEnforcementEnabled() bool { + return os.Getenv(intentPolicyEnforcementEnv) == "true" +} + +func intentAuthorizationMiddleware() mcp.Middleware { + compiler, err := loadIntentPolicyCompiler() + if err != nil { + mcpIntentAuthzLog.Printf("intent policy enforcement disabled: %v", err) + return passthroughMiddleware + } + authorizer := authz.Authorizer{} + return intentAuthorizationMiddlewareForPolicy(compiler, authorizer.AuthorizeTool) +} + +func intentAuthorizationMiddlewareForPolicy(compiler intent.PolicyCompiler, authorize func(intent.ExecutionPolicy, string, authz.ToolContext) error) mcp.Middleware { + return func(next mcp.MethodHandler) mcp.MethodHandler { + return func(ctx context.Context, method string, req mcp.Request) (mcp.Result, error) { + if method != "tools/call" { + return next(ctx, method, req) + } + toolName := extractMCPToolName(req) + policy := compiler.Compile(intent.IntentRecord{ + Status: intent.AttributionMapped, + Labels: splitCSVEnv(intentLabelsEnv), + }, currentRepositoryContext()) + if err := authorize(policy, toolName, toolContextForMCPTool(toolName)); err != nil { + return &mcp.CallToolResult{ + IsError: true, + Content: []mcp.Content{&mcp.TextContent{Text: err.Error()}}, + }, nil + } + return next(ctx, method, req) + } + } +} + +func passthroughMiddleware(next mcp.MethodHandler) mcp.MethodHandler { + return next +} + +func loadIntentPolicyCompiler() (intent.PolicyCompiler, error) { + path := os.Getenv(intentPolicyPathEnv) + if path == "" { + path = filepath.Join(".github", "intent-policy.json") + } + data, err := os.ReadFile(path) + if err != nil { + return intent.PolicyCompiler{}, err + } + var cfg intentPolicyFile + if err := json.Unmarshal(data, &cfg); err != nil { + return intent.PolicyCompiler{}, err + } + if len(cfg.Rules) == 0 { + return intent.PolicyCompiler{}, errors.New("intent policy has no rules") + } + return intent.PolicyCompiler{Rules: cfg.Rules}, nil +} + +func currentRepositoryContext() intent.RepositoryContext { + repo := os.Getenv("GITHUB_REPOSITORY") + owner, name, _ := strings.Cut(repo, "/") + return intent.RepositoryContext{Owner: owner, Org: owner, Name: name} +} + +func toolContextForMCPTool(toolName string) authz.ToolContext { + return authz.ToolContext{ + IsWrite: isIntentWriteTool(toolName), + IsAutoMerge: toolName == "merge_pull_request", + Branch: currentBranch(), + DefaultBranch: firstNonEmptyIntentAuthz(os.Getenv("GITHUB_DEFAULT_BRANCH"), "main"), + Approved: os.Getenv(intentHumanApprovedEnv) == "true", + PassedChecks: splitCSVEnv(intentPassedChecksEnv), + Attempt: intentAttempt(), + } +} + +func isIntentWriteTool(toolName string) bool { + switch toolName { + case "add", "update", "fix", "merge_pull_request", "create_or_update_file", "push_files", "delete_file": + return true + default: + return false + } +} + +func intentAttempt() int { + raw := os.Getenv(intentAttemptEnv) + if raw == "" { + return 1 + } + attempt, err := strconv.Atoi(raw) + if err != nil || attempt < 1 { + return 1 + } + return attempt +} + +func splitCSVEnv(name string) []string { + raw := os.Getenv(name) + if strings.TrimSpace(raw) == "" { + return nil + } + parts := strings.Split(raw, ",") + values := make([]string, 0, len(parts)) + for _, part := range parts { + if value := strings.TrimSpace(part); value != "" { + values = append(values, value) + } + } + return values +} + +func currentBranch() string { + for _, name := range []string{"GITHUB_HEAD_REF", "GITHUB_REF_NAME"} { + if value := os.Getenv(name); value != "" { + return value + } + } + out, err := exec.Command("git", "branch", "--show-current").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +func firstNonEmptyIntentAuthz(values ...string) string { + for _, value := range values { + if value != "" { + return value + } + } + return "" +} diff --git a/pkg/cli/mcp_intent_authorization_test.go b/pkg/cli/mcp_intent_authorization_test.go new file mode 100644 index 00000000000..d47a17c99ee --- /dev/null +++ b/pkg/cli/mcp_intent_authorization_test.go @@ -0,0 +1,64 @@ +package cli + +import ( + "context" + "testing" + + "github.com/github/gh-aw/pkg/intent" + "github.com/github/gh-aw/pkg/intent/authz" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIntentAuthorizationMiddlewareRejectsDeniedToolCall(t *testing.T) { + compiler := intent.PolicyCompiler{Rules: []intent.PolicyRule{{ + ID: "deny-add", + Set: intent.ExecutionPolicy{ + AllowedTools: []string{"compile", "add"}, + DeniedTools: []string{"add"}, + }, + }}} + middleware := intentAuthorizationMiddlewareForPolicy(compiler, (authz.Authorizer{}).AuthorizeTool) + called := false + handler := middleware(func(_ context.Context, _ string, _ mcp.Request) (mcp.Result, error) { + called = true + return &mcp.CallToolResult{}, nil + }) + + result, err := handler(context.Background(), "tools/call", fakeToolCallRequest("add")) + + require.NoError(t, err) + assert.False(t, called) + toolResult := result.(*mcp.CallToolResult) + assert.True(t, toolResult.IsError) + assert.Contains(t, toolResult.Content[0].(*mcp.TextContent).Text, "tool denied") +} + +func TestIntentAuthorizationMiddlewareReadsPolicyAtExecutionTime(t *testing.T) { + autoMerge := false + compiler := intent.PolicyCompiler{Rules: []intent.PolicyRule{{ + ID: "runtime-fields", + Set: intent.ExecutionPolicy{ + AllowedTools: []string{"merge_pull_request"}, + AutoMergeAllowed: &autoMerge, + MaxAttempts: 2, + }, + }}} + + var observedPolicy intent.ExecutionPolicy + middleware := intentAuthorizationMiddlewareForPolicy(compiler, func(policy intent.ExecutionPolicy, _ string, _ authz.ToolContext) error { + observedPolicy = policy + return nil + }) + handler := middleware(func(_ context.Context, _ string, _ mcp.Request) (mcp.Result, error) { + return &mcp.CallToolResult{}, nil + }) + + _, err := handler(context.Background(), "tools/call", fakeToolCallRequest("merge_pull_request")) + + require.NoError(t, err) + assert.Equal(t, 2, observedPolicy.MaxAttempts) + require.NotNil(t, observedPolicy.AutoMergeAllowed) + assert.False(t, *observedPolicy.AutoMergeAllowed) +} diff --git a/pkg/cli/mcp_server.go b/pkg/cli/mcp_server.go index ff1409cc31d..97e3ebf3023 100644 --- a/pkg/cli/mcp_server.go +++ b/pkg/cli/mcp_server.go @@ -94,6 +94,9 @@ func createMCPServer(cmdPath string, actor string, validateActor bool, manifestC // Add receiving middleware to transform raw JSON-schema "additional properties" // validation errors into helpful messages with "Did you mean?" suggestions. server.AddReceivingMiddleware(argumentValidationMiddleware(mcpToolParams())) + if intentPolicyEnforcementEnabled() { + server.AddReceivingMiddleware(intentAuthorizationMiddleware()) + } return server } diff --git a/pkg/intent/authz/authorizer.go b/pkg/intent/authz/authorizer.go new file mode 100644 index 00000000000..4e8cd5049cc --- /dev/null +++ b/pkg/intent/authz/authorizer.go @@ -0,0 +1,65 @@ +package authz + +import ( + "errors" + "fmt" + "slices" + + "github.com/github/gh-aw/pkg/intent" +) + +var ( + ErrToolDenied = errors.New("tool denied by intent policy") + ErrToolNotAllowed = errors.New("tool not allowed by intent policy") + ErrWriteScopeDenied = errors.New("write denied by intent policy") + ErrHumanApprovalNeeded = errors.New("human approval required by intent policy") + ErrRequiredChecks = errors.New("required checks missing by intent policy") + ErrAutoMergeDenied = errors.New("auto-merge denied by intent policy") + ErrMaxAttemptsExceeded = errors.New("max attempts exceeded by intent policy") +) + +type ToolContext struct { + IsWrite bool + IsAutoMerge bool + Branch string + DefaultBranch string + Approved bool + PassedChecks []string + Attempt int +} + +type Authorizer struct{} + +func (a Authorizer) AuthorizeTool(policy intent.ExecutionPolicy, tool string, ctx ToolContext) error { + if slices.Contains(policy.DeniedTools, tool) { + return fmt.Errorf("%w: %s", ErrToolDenied, tool) + } + if policy.AllowedTools != nil && !slices.Contains(policy.AllowedTools, tool) { + return fmt.Errorf("%w: %s", ErrToolNotAllowed, tool) + } + if policy.MaxAttempts > 0 && ctx.Attempt > policy.MaxAttempts { + return fmt.Errorf("%w: attempt %d exceeds max_attempts %d", ErrMaxAttemptsExceeded, ctx.Attempt, policy.MaxAttempts) + } + if ctx.IsWrite { + if policy.WriteScope == "none" || policy.Autonomy == "propose_only" { + return ErrWriteScopeDenied + } + if policy.WriteScope == "feature_branch" && ctx.Branch != "" && ctx.DefaultBranch != "" && ctx.Branch == ctx.DefaultBranch { + return fmt.Errorf("%w: feature_branch policy cannot write to %s", ErrWriteScopeDenied, ctx.Branch) + } + if policy.HumanApprovalRequired && !ctx.Approved { + return ErrHumanApprovalNeeded + } + } + if len(policy.RequiredChecks) > 0 { + for _, check := range policy.RequiredChecks { + if !slices.Contains(ctx.PassedChecks, check) { + return fmt.Errorf("%w: %s", ErrRequiredChecks, check) + } + } + } + if ctx.IsAutoMerge && policy.AutoMergeAllowed != nil && !*policy.AutoMergeAllowed { + return ErrAutoMergeDenied + } + return nil +} diff --git a/pkg/intent/authz/authorizer_test.go b/pkg/intent/authz/authorizer_test.go new file mode 100644 index 00000000000..8c2ca7d271e --- /dev/null +++ b/pkg/intent/authz/authorizer_test.go @@ -0,0 +1,75 @@ +package authz + +import ( + "testing" + + "github.com/github/gh-aw/pkg/intent" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAuthorizerAuthorizeToolRejectsDeniedTool(t *testing.T) { + err := (Authorizer{}).AuthorizeTool(intent.ExecutionPolicy{ + AllowedTools: []string{"compile", "add"}, + DeniedTools: []string{"add"}, + }, "add", ToolContext{}) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrToolDenied) +} + +func TestAuthorizerAuthorizeToolRejectsToolOutsideAllowedSet(t *testing.T) { + err := (Authorizer{}).AuthorizeTool(intent.ExecutionPolicy{ + AllowedTools: []string{"compile"}, + }, "add", ToolContext{}) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrToolNotAllowed) +} + +func TestAuthorizerAuthorizeToolEnforcesExecutionPolicyGates(t *testing.T) { + autoMergeDenied := false + policy := intent.ExecutionPolicy{ + Autonomy: "bounded", + AllowedTools: []string{"merge_pull_request"}, + WriteScope: "feature_branch", + HumanApprovalRequired: true, + RequiredChecks: []string{"unit-tests"}, + AutoMergeAllowed: &autoMergeDenied, + MaxAttempts: 2, + } + + t.Run("max attempts", func(t *testing.T) { + err := (Authorizer{}).AuthorizeTool(policy, "merge_pull_request", ToolContext{ + IsWrite: true, + IsAutoMerge: true, + Approved: true, + PassedChecks: []string{"unit-tests"}, + Attempt: 3, + }) + require.Error(t, err) + assert.ErrorIs(t, err, ErrMaxAttemptsExceeded) + }) + + t.Run("auto merge", func(t *testing.T) { + err := (Authorizer{}).AuthorizeTool(policy, "merge_pull_request", ToolContext{ + IsWrite: true, + IsAutoMerge: true, + Approved: true, + PassedChecks: []string{"unit-tests"}, + Attempt: 1, + }) + require.Error(t, err) + assert.ErrorIs(t, err, ErrAutoMergeDenied) + }) + + t.Run("required checks", func(t *testing.T) { + err := (Authorizer{}).AuthorizeTool(policy, "merge_pull_request", ToolContext{ + IsWrite: true, + Approved: true, + Attempt: 1, + }) + require.Error(t, err) + assert.ErrorIs(t, err, ErrRequiredChecks) + }) +} diff --git a/pkg/intent/policy.go b/pkg/intent/policy.go index 444c09c464f..98e36d42a80 100644 --- a/pkg/intent/policy.go +++ b/pkg/intent/policy.go @@ -31,10 +31,9 @@ var writeScopeRank = map[string]int{ // ExecutionPolicy governs what an agent may do for a given intent. // -// WARNING: PolicyCompiler is advisory only. All fields except Autonomy are -// compiled and recorded for audit but are NOT yet wired into runtime enforcement. -// Do not rely on this policy to gate actual tool calls or merge operations until -// Authorizer.AuthorizeTool is implemented and integrated into the execution path. +// When GH_AW_INTENT_POLICY_ENFORCEMENT=true, the MCP orchestrator loads the +// compiled policy and enforces tool, write, approval, check, auto-merge, and +// attempt gates before tool execution. type ExecutionPolicy struct { Autonomy string `json:"autonomy"` @@ -90,9 +89,9 @@ type PolicyCondition struct { // PolicyCompiler holds policy rules for callers that still exchange policy compiler // configuration data. // -// WARNING: the compiled policy is advisory only. Runtime enforcement is not yet -// wired to the orchestrator — see the intent-attribution-agent-governance spec for -// the required follow-up before treating compiled policies as a security gate. +// Runtime enforcement is feature-flagged in the MCP orchestrator; callers that +// need enforcement must enable GH_AW_INTENT_POLICY_ENFORCEMENT and provide an +// intent policy file. type PolicyCompiler struct { Rules []PolicyRule } diff --git a/pkg/workflow/otel_observability_formal_test.go b/pkg/workflow/otel_observability_formal_test.go index e5221994d3f..bc967355e44 100644 --- a/pkg/workflow/otel_observability_formal_test.go +++ b/pkg/workflow/otel_observability_formal_test.go @@ -1,6 +1,9 @@ package workflow import ( + "os" + "path/filepath" + "strings" "testing" "github.com/github/gh-aw/pkg/constants" @@ -409,3 +412,38 @@ func TestFormal_InstrumentationScopeNaming(t *testing.T) { t.Skip("pending: no production instrumentationScopeResolver implementation in pkg/workflow; " + "replace t.Skip with assertions against the real resolver once it lands") } + +func TestFormal_OTelComplianceRuntimeContractSuiteIncludesLevel1IDs(t *testing.T) { + root := findRepoRootForOTelTest(t) + contractTest, err := os.ReadFile(filepath.Join(root, "actions", "setup", "js", "otel_contract.test.cjs")) + require.NoError(t, err) + contract := string(contractTest) + + for _, testID := range []string{"T-OT-008", "T-OT-009", "T-OT-010", "T-OT-011"} { + assert.Contains(t, contract, testID) + } + assert.Contains(t, contract, "gh-aw.outcome.evaluate") + assert.Contains(t, contract, "gh-aw.outcome.type") + + makefile, err := os.ReadFile(filepath.Join(root, "Makefile")) + require.NoError(t, err) + assert.True(t, + strings.Contains(string(makefile), "T-OT-001 through T-OT-011") && + strings.Contains(string(makefile), "otel_contract.test.cjs"), + "validate-otel-contract must report and run the Level 1 OTEL contract test IDs", + ) +} + +func findRepoRootForOTelTest(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + require.NoError(t, err) + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir + } + parent := filepath.Dir(dir) + require.NotEqual(t, dir, parent, "could not find repository root") + dir = parent + } +} diff --git a/specs/intent-attribution-agent-governance.md b/specs/intent-attribution-agent-governance.md index 7b61b23494d..a20b01d5c26 100644 --- a/specs/intent-attribution-agent-governance.md +++ b/specs/intent-attribution-agent-governance.md @@ -128,6 +128,13 @@ label-to-intent selectors derived from `.github/objective-mapping.json` — duri or CI. Keys present in one source but not the other SHOULD surface as a sync warning or compliance failure so attribution and authorization stay aligned. +Migration tracking SHOULD record whether each repository is in one of three +states: `objective-mapping-only`, `dual-read`, or `intent-policy-primary`. +Repositories in `dual-read` MUST keep label dimensions and policy rule matchers +in sync until `.github/intent-policy.json` becomes primary and the legacy +`.github/objective-mapping.json` path is retained only for explicit +backward-compatibility reads. + **Escalation norm**: A sync warning that persists across **3 or more consecutive CI runs** without a corresponding corrective PR or explicit waiver **MUST** be escalated to a compliance failure. When a sync warning escalates, the CI check responsible for drift detection **MUST** @@ -937,23 +944,30 @@ The agent must not be able to modify or expand its own policy. ### `Authorizer.AuthorizeTool` Implementation Audit -The `AuthorizeTool` function as specified in this section is **not yet implemented** in the Go orchestrator. The following table documents which fields of `ExecutionPolicy` are wired to runtime enforcement and which remain unused. +The `AuthorizeTool` function is implemented in `pkg/intent/authz` and wired to +the Go MCP orchestrator behind `GH_AW_INTENT_POLICY_ENFORCEMENT=true`. The +following table documents which fields of `ExecutionPolicy` are enforced by +that feature-flagged path. | `ExecutionPolicy` field | Wired to enforcement? | Notes | |---|---|---| -| `AllowedTools` | **Not wired** | The `pkg/intent` package implements `PolicyCompiler.Compile()` and `mergePolicy()` for this field, but no orchestrator calls `AuthorizeTool` at tool-call time. | -| `DeniedTools` | **Not wired** | Same as `AllowedTools` — present in the spec and policy model, not enforced at runtime. | -| `Autonomy` | **Not wired** | The autonomy level is compiled into the policy but not checked against actual workflow capabilities at execution time. | -| `WriteScope` | **Not wired** | Defined in the policy model; no runtime enforcement in the Go orchestrator. | -| `HumanApprovalRequired` | **Not wired** | Defined in policy model; human approval gates are not currently tied to `ExecutionPolicy`. | -| `AutoMergeAllowed` | **Not wired** | Not enforced by the orchestrator. | -| `RequiredChecks` | **Not wired** | Not checked before workflow execution. | -| `MaxAttempts` | **Not wired** | Not enforced at the orchestrator level. | +| `AllowedTools` | **Feature-flagged MCP path** | `pkg/intent/authz.Authorizer.AuthorizeTool` rejects MCP tool calls not in the compiled allowed set. | +| `DeniedTools` | **Feature-flagged MCP path** | `AuthorizeTool` rejects any MCP tool call named in the compiled denied set. | +| `Autonomy` | **Feature-flagged MCP path** | `propose_only` rejects write-class MCP tools. | +| `WriteScope` | **Feature-flagged MCP path** | `none` rejects write-class MCP tools; `feature_branch` rejects writes to the default branch when branch context is available. | +| `HumanApprovalRequired` | **Feature-flagged MCP path** | Write-class MCP tools require the approval signal consumed by the orchestrator middleware. | +| `AutoMergeAllowed` | **Feature-flagged MCP path** | Auto-merge-class tool calls are rejected when the compiled policy denies auto-merge. | +| `RequiredChecks` | **Feature-flagged MCP path** | Tool calls require all compiled check names to appear in the orchestrator's passed-check signal. | +| `MaxAttempts` | **Feature-flagged MCP path** | Tool calls are rejected when the current execution attempt exceeds the compiled maximum. | | `RuleIDs` | **Provenance only** | Recorded in the policy for auditing; not used to gate execution. | -**Risk**: Policy constraints defined in `.github/intent-policy.json` (or the equivalent `rules` array) have no runtime effect until the orchestrator is wired to call `AuthorizeTool` and enforce `WriteScope`, `HumanApprovalRequired`, and `RequiredChecks`. Any policy compiled by `PolicyCompiler.Compile()` today is purely advisory. +**Risk**: Policy constraints defined in `.github/intent-policy.json` have no +runtime effect unless the feature-flagged MCP enforcement path is enabled and +the orchestrator can resolve current intent, approval, attempt, and check +signals. -**Required follow-up**: Implement `Authorizer.AuthorizeTool` in `pkg/intent` or a new `pkg/intent/authz` sub-package and wire it into the execution path. Gate enforcement behind a feature flag until the policy model is validated in production. +**Required follow-up**: Broaden policy-signal resolution beyond MCP tool calls +as additional orchestrator entry points adopt runtime authorization. Initial observable rules: diff --git a/specs/otel-observability-spec.md b/specs/otel-observability-spec.md index 9b29cf40fd9..c179ce7dcb7 100644 --- a/specs/otel-observability-spec.md +++ b/specs/otel-observability-spec.md @@ -865,6 +865,13 @@ Level 1 and Level 2 compatibility validation MUST cover the following behaviors: The repository enforcement entry point for these checks is `make validate-otel-contract`. This target MUST remain focused on the customer-facing compatibility contract rather than all possible OTEL-related tests. +### Safeguards + +Stubbed or otherwise unimplemented test IDs MUST NOT be counted toward a Level +1 conformance claim. A Level 1 claim is valid only for test IDs that are wired +into automated validation and assert the exported semantic payload, not merely +listed in this specification. + ### 17.1.1 Test ID Stubs: Level 1 Compliance The following test IDs are stubs for Level 1 (Stable Configuration and Export) compliance tests. Implementations MUST provide tests that correspond to each stub before claiming Level 1 conformance. diff --git a/specs/replace-label-compliance/README.md b/specs/replace-label-compliance/README.md index 5a0c60eda08..1870df85f9a 100644 --- a/specs/replace-label-compliance/README.md +++ b/specs/replace-label-compliance/README.md @@ -76,6 +76,16 @@ are implemented in: - `pkg/workflow/replace_label_formal_test.go` - `pkg/workflow/replace_label_transitions_formal_test.go` +### Sync Notes + +This Behavioral Coverage Map is the canonical predicate-to-formal-test index +for replace-label. The corresponding T-RL test ID to fixture-file index is +maintained in [`specs/replace-label-spec.md` §9.2.3](../replace-label-spec.md#923-label-validation-tests). + +Coverage parity check (2026-08-12): verified that this Behavioral Coverage Map +and `specs/replace-label-spec.md`'s fixture linkage table cross-reference each +other and cover the replace-label fixture/test tables bidirectionally. + ## Fixture Schema Each fixture file is a YAML document with the following top-level keys: diff --git a/specs/replace-label-spec.md b/specs/replace-label-spec.md index 572884bff6a..a5e31cc0647 100644 --- a/specs/replace-label-spec.md +++ b/specs/replace-label-spec.md @@ -571,6 +571,18 @@ Fixture linkage check (2026-08-01): - [x] T-RL-024 covered by `specs/replace-label-compliance/rl-003-blocklist-ordering.yaml` - [x] T-RL-025 covered by `specs/replace-label-compliance/rl-002-allowlist-enforcement.yaml` +### Sync Notes + +The bidirectional fixture and formal-test index for this section is maintained +in [`specs/replace-label-compliance/README.md`](replace-label-compliance/README.md). +Its Behavioral Coverage Map is the canonical cross-reference from predicate IDs +to Go formal tests; this section remains the canonical cross-reference from +T-RL test IDs to normative fixture files. + +Coverage parity check (2026-08-12): verified that the fixture linkage table +above and the compliance README Behavioral Coverage Map point to each other and +cover the replace-label fixture/test tables bidirectionally. + #### 9.2.4 Gate Check Tests - **T-RL-030**: Verify that an item satisfying all `required-labels` proceeds to the mutation stage. From 2baac13e1a65eb32cc985111d008af957c5781dc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:39:15 +0000 Subject: [PATCH 3/4] Address intent default branch review feedback Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/mcp_intent_authorization.go | 12 ++---------- specs/intent-attribution-agent-governance.md | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/pkg/cli/mcp_intent_authorization.go b/pkg/cli/mcp_intent_authorization.go index 16e45331198..a385d271d8b 100644 --- a/pkg/cli/mcp_intent_authorization.go +++ b/pkg/cli/mcp_intent_authorization.go @@ -23,6 +23,7 @@ const ( intentHumanApprovedEnv = "GH_AW_INTENT_HUMAN_APPROVED" intentPassedChecksEnv = "GH_AW_INTENT_REQUIRED_CHECKS_PASSED" intentAttemptEnv = "GH_AW_INTENT_ATTEMPT" + intentDefaultBranchEnv = "GH_AW_INTENT_DEFAULT_BRANCH" ) var mcpIntentAuthzLog = logger.New("mcp:intent_authorization") @@ -101,7 +102,7 @@ func toolContextForMCPTool(toolName string) authz.ToolContext { IsWrite: isIntentWriteTool(toolName), IsAutoMerge: toolName == "merge_pull_request", Branch: currentBranch(), - DefaultBranch: firstNonEmptyIntentAuthz(os.Getenv("GITHUB_DEFAULT_BRANCH"), "main"), + DefaultBranch: os.Getenv(intentDefaultBranchEnv), Approved: os.Getenv(intentHumanApprovedEnv) == "true", PassedChecks: splitCSVEnv(intentPassedChecksEnv), Attempt: intentAttempt(), @@ -156,12 +157,3 @@ func currentBranch() string { } return strings.TrimSpace(string(out)) } - -func firstNonEmptyIntentAuthz(values ...string) string { - for _, value := range values { - if value != "" { - return value - } - } - return "" -} diff --git a/specs/intent-attribution-agent-governance.md b/specs/intent-attribution-agent-governance.md index a20b01d5c26..2bd2ecee229 100644 --- a/specs/intent-attribution-agent-governance.md +++ b/specs/intent-attribution-agent-governance.md @@ -954,7 +954,7 @@ that feature-flagged path. | `AllowedTools` | **Feature-flagged MCP path** | `pkg/intent/authz.Authorizer.AuthorizeTool` rejects MCP tool calls not in the compiled allowed set. | | `DeniedTools` | **Feature-flagged MCP path** | `AuthorizeTool` rejects any MCP tool call named in the compiled denied set. | | `Autonomy` | **Feature-flagged MCP path** | `propose_only` rejects write-class MCP tools. | -| `WriteScope` | **Feature-flagged MCP path** | `none` rejects write-class MCP tools; `feature_branch` rejects writes to the default branch when branch context is available. | +| `WriteScope` | **Feature-flagged MCP path** | `none` rejects write-class MCP tools; `feature_branch` rejects writes to the default branch when branch context and `GH_AW_INTENT_DEFAULT_BRANCH` are available. | | `HumanApprovalRequired` | **Feature-flagged MCP path** | Write-class MCP tools require the approval signal consumed by the orchestrator middleware. | | `AutoMergeAllowed` | **Feature-flagged MCP path** | Auto-merge-class tool calls are rejected when the compiled policy denies auto-merge. | | `RequiredChecks` | **Feature-flagged MCP path** | Tool calls require all compiled check names to appear in the orchestrator's passed-check signal. | From 4af1f5e67d8201b629233170d11fcbf03163af24 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:42:53 +0000 Subject: [PATCH 4/4] Fail closed on intent policy load errors Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/otel_contract.test.cjs | 2 +- pkg/cli/mcp_intent_authorization.go | 23 +++++++++++++++++------ pkg/cli/mcp_intent_authorization_test.go | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/actions/setup/js/otel_contract.test.cjs b/actions/setup/js/otel_contract.test.cjs index 6f24407ab69..79e865e973c 100644 --- a/actions/setup/js/otel_contract.test.cjs +++ b/actions/setup/js/otel_contract.test.cjs @@ -248,6 +248,6 @@ describe("gh-aw OpenTelemetry compatibility contract", () => { const agentAttrs = attrsByKey(spans[1]); expect(agentAttrs).toHaveProperty("gen_ai.usage.total_tokens"); - expect(process.env.OTEL_EXPORTER_OTLP_ENDPOINT).toBe("https://traces.example.com"); + expect(fetchMock.mock.calls.map(([url]) => url)).toContain("https://traces.example.com/v1/traces"); }); }); diff --git a/pkg/cli/mcp_intent_authorization.go b/pkg/cli/mcp_intent_authorization.go index a385d271d8b..1ee436cbbcd 100644 --- a/pkg/cli/mcp_intent_authorization.go +++ b/pkg/cli/mcp_intent_authorization.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "os" "os/exec" "path/filepath" @@ -39,13 +40,27 @@ func intentPolicyEnforcementEnabled() bool { func intentAuthorizationMiddleware() mcp.Middleware { compiler, err := loadIntentPolicyCompiler() if err != nil { - mcpIntentAuthzLog.Printf("intent policy enforcement disabled: %v", err) - return passthroughMiddleware + mcpIntentAuthzLog.Printf("intent policy enforcement failed closed: %v", err) + return failedClosedIntentAuthorizationMiddleware(err) } authorizer := authz.Authorizer{} return intentAuthorizationMiddlewareForPolicy(compiler, authorizer.AuthorizeTool) } +func failedClosedIntentAuthorizationMiddleware(loadErr error) mcp.Middleware { + return func(next mcp.MethodHandler) mcp.MethodHandler { + return func(ctx context.Context, method string, req mcp.Request) (mcp.Result, error) { + if method != "tools/call" { + return next(ctx, method, req) + } + return &mcp.CallToolResult{ + IsError: true, + Content: []mcp.Content{&mcp.TextContent{Text: fmt.Sprintf("intent policy enforcement failed closed: %v", loadErr)}}, + }, nil + } + } +} + func intentAuthorizationMiddlewareForPolicy(compiler intent.PolicyCompiler, authorize func(intent.ExecutionPolicy, string, authz.ToolContext) error) mcp.Middleware { return func(next mcp.MethodHandler) mcp.MethodHandler { return func(ctx context.Context, method string, req mcp.Request) (mcp.Result, error) { @@ -68,10 +83,6 @@ func intentAuthorizationMiddlewareForPolicy(compiler intent.PolicyCompiler, auth } } -func passthroughMiddleware(next mcp.MethodHandler) mcp.MethodHandler { - return next -} - func loadIntentPolicyCompiler() (intent.PolicyCompiler, error) { path := os.Getenv(intentPolicyPathEnv) if path == "" { diff --git a/pkg/cli/mcp_intent_authorization_test.go b/pkg/cli/mcp_intent_authorization_test.go index d47a17c99ee..43b4dfbdd8d 100644 --- a/pkg/cli/mcp_intent_authorization_test.go +++ b/pkg/cli/mcp_intent_authorization_test.go @@ -2,6 +2,7 @@ package cli import ( "context" + "errors" "testing" "github.com/github/gh-aw/pkg/intent" @@ -35,6 +36,23 @@ func TestIntentAuthorizationMiddlewareRejectsDeniedToolCall(t *testing.T) { assert.Contains(t, toolResult.Content[0].(*mcp.TextContent).Text, "tool denied") } +func TestFailedClosedIntentAuthorizationMiddlewareRejectsToolCalls(t *testing.T) { + middleware := failedClosedIntentAuthorizationMiddleware(errors.New("missing policy")) + called := false + handler := middleware(func(_ context.Context, _ string, _ mcp.Request) (mcp.Result, error) { + called = true + return &mcp.CallToolResult{}, nil + }) + + result, err := handler(context.Background(), "tools/call", fakeToolCallRequest("compile")) + + require.NoError(t, err) + assert.False(t, called) + toolResult := result.(*mcp.CallToolResult) + assert.True(t, toolResult.IsError) + assert.Contains(t, toolResult.Content[0].(*mcp.TextContent).Text, "failed closed") +} + func TestIntentAuthorizationMiddlewareReadsPolicyAtExecutionTime(t *testing.T) { autoMerge := false compiler := intent.PolicyCompiler{Rules: []intent.PolicyRule{{