Address SPDD daily spec review: DriftRecord conformance tests, guard-policy dry-run report, spec gaps

pkg/cli/compile_guard_policy_report.go

@@ -0,0 +1,149 @@
+package cli
+
+import (
+	"fmt"
+	"os"
+	"sort"
+	"strings"
+
+	"github.com/github/gh-aw/pkg/console"
+	"github.com/github/gh-aw/pkg/workflow"
+)
+
+// guardPolicyDryRunReport summarizes the effective GitHub guard-policy
+// configuration for a single compiled workflow. It is produced in --strict
+// mode as a compile-time dry-run of which repositories the guard policy
+// would permit or deny, addressing Open Question #4 in
+// scratchpad/guard-policies-specification.md ("Should we add a 'dry-run'
+// mode to test policies before enforcement?").
+type guardPolicyDryRunReport struct {
+	Workflow       string
+	Lockdown       bool
+	PermittedRepos string
+	MinIntegrity   string
+	BlockedUsers   []string
+	TrustedUsers   []string
+	ApprovalLabels []string
+}
+
+// hasGuardPolicyFields reports whether the GitHub tool config has any
+// guard-policy fields configured (allowed-repos, min-integrity, blocked-users,
+// trusted-users, approval-labels).
+func hasGuardPolicyFields(github *workflow.GitHubToolConfig) bool {
+	if github == nil {
+		return false
+	}
+	hasRepos := github.AllowedRepos != nil || github.Repos != nil
+	hasMinIntegrity := github.MinIntegrity != ""
+	hasBlockedUsers := len(github.BlockedUsers) > 0 || github.BlockedUsersExpr != ""
+	hasApprovalLabels := len(github.ApprovalLabels) > 0 || github.ApprovalLabelsExpr != ""
+	hasTrustedUsers := len(github.TrustedUsers) > 0 || github.TrustedUsersExpr != ""
+	return hasRepos || hasMinIntegrity || hasBlockedUsers || hasApprovalLabels || hasTrustedUsers
+}
+
+// formatGuardPolicyReposScope renders a GitHubReposScope value ("all",
+// "public", or an array of repository patterns) as a human-readable string
+// for the dry-run report.
+func formatGuardPolicyReposScope(scope workflow.GitHubReposScope) string {
+	switch v := scope.(type) {
+	case nil:
+		return "all (default)"
+	case string:
+		if v == "" {
+			return "all (default)"
+		}
+		return v
+	case []any:
+		patterns := make([]string, 0, len(v))
+		for _, item := range v {
+			if s, ok := item.(string); ok && s != "" {
+				patterns = append(patterns, s)
+			}
+		}
+		sort.Strings(patterns)
+		if len(patterns) == 0 {
+			return "all (default)"
+		}
+		return strings.Join(patterns, ", ")
+	case []string:
+		patterns := append([]string(nil), v...)
+		sort.Strings(patterns)
+		if len(patterns) == 0 {
+			return "all (default)"
+		}
+		return strings.Join(patterns, ", ")
+	default:
+		return fmt.Sprintf("%v", v)
+	}
+}
+
+// buildGuardPolicyDryRunReport builds a guardPolicyDryRunReport for a
+// workflow's GitHub tool configuration. Returns nil when no guard-policy
+// fields are configured (nothing to report).
+func buildGuardPolicyDryRunReport(workflowName string, github *workflow.GitHubToolConfig) *guardPolicyDryRunReport {
+	if github == nil || !hasGuardPolicyFields(github) {
+		return nil
+	}
+
+	repos := github.AllowedRepos
+	if repos == nil {
+		repos = github.Repos
+	}
+
+	minIntegrity := string(github.MinIntegrity)
+	if minIntegrity == "" {
+		minIntegrity = "none (default)"
+	}
+
+	return &guardPolicyDryRunReport{
+		Workflow:       workflowName,
+		Lockdown:       github.Lockdown,
+		PermittedRepos: formatGuardPolicyReposScope(repos),
+		MinIntegrity:   minIntegrity,
+		BlockedUsers:   github.BlockedUsers,
+		TrustedUsers:   github.TrustedUsers,
+		ApprovalLabels: github.ApprovalLabels,
+	}
+}
+
+// formatGuardPolicyDryRunReport renders a guardPolicyDryRunReport as a
+// human-readable multi-line string for --strict compile output.
+func formatGuardPolicyDryRunReport(report *guardPolicyDryRunReport) string {
+	if report == nil {
+		return ""
+	}
+
+	var b strings.Builder
+	fmt.Fprintf(&b, "guard policy dry-run report for %s:\n", report.Workflow)
+	if report.Lockdown {
+		b.WriteString("  lockdown: true (guard-policy fields below are not evaluated at runtime)\n")
+	}
+	fmt.Fprintf(&b, "  allowed-repos: %s\n", report.PermittedRepos)
+	fmt.Fprintf(&b, "  min-integrity: %s\n", report.MinIntegrity)
+	if len(report.BlockedUsers) > 0 {
+		fmt.Fprintf(&b, "  blocked-users: %s\n", strings.Join(report.BlockedUsers, ", "))
+	}
+	if len(report.TrustedUsers) > 0 {
+		fmt.Fprintf(&b, "  trusted-users: %s\n", strings.Join(report.TrustedUsers, ", "))
+	}
+	if len(report.ApprovalLabels) > 0 {
+		fmt.Fprintf(&b, "  approval-labels: %s\n", strings.Join(report.ApprovalLabels, ", "))
+	}
+	return strings.TrimRight(b.String(), "\n")
+}
+
+// printGuardPolicyDryRunReport prints a compile-time guard-policy dry-run
+// report to stderr when --strict is set and the workflow has a GitHub guard
+// policy configured. It is a no-op otherwise.
+func printGuardPolicyDryRunReport(workflowName string, workflowData *workflow.WorkflowData, strict bool) {
+	if !strict || workflowData == nil || workflowData.ParsedTools == nil {
+		return
+	}
+
+	report := buildGuardPolicyDryRunReport(workflowName, workflowData.ParsedTools.GitHub)
+	if report == nil {
+		return
+	}
+
+	fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(formatGuardPolicyDryRunReport(report)))
+}

pkg/cli/compile_guard_policy_report_test.go

@@ -0,0 +1,142 @@
+//go:build !integration
+
+package cli
+
+import (
+	"io"
+	"os"
+	"testing"
+
+	"github.com/github/gh-aw/pkg/workflow"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// captureStderrForGuardPolicyReportTest captures stderr output produced while
+// running fn, for use by the guard-policy dry-run report tests in this file.
+func captureStderrForGuardPolicyReportTest(fn func()) string {
+	old := os.Stderr
+	r, w, err := os.Pipe()
+	if err != nil {
+		panic(err)
+	}
+	os.Stderr = w
+
+	fn()
+
+	if err := w.Close(); err != nil {
+		panic(err)
+	}
+	os.Stderr = old
+
+	out, err := io.ReadAll(r)
+	if err != nil {
+		panic(err)
+	}
+	return string(out)
+}
+
+// TestBuildGuardPolicyDryRunReport_NoGuardPolicy verifies that no report is
+// produced when no guard-policy fields are configured.
+func TestBuildGuardPolicyDryRunReport_NoGuardPolicy(t *testing.T) {
+	assert.Nil(t, buildGuardPolicyDryRunReport("test.md", nil))
+	assert.Nil(t, buildGuardPolicyDryRunReport("test.md", &workflow.GitHubToolConfig{}))
+}
+
+// TestBuildGuardPolicyDryRunReport_AllowedReposAndMinIntegrity verifies that a
+// dry-run report is produced summarizing allowed-repos and min-integrity.
+func TestBuildGuardPolicyDryRunReport_AllowedReposAndMinIntegrity(t *testing.T) {
+	github := &workflow.GitHubToolConfig{
+		AllowedRepos: []any{"owner/repo-b", "owner/repo-a"},
+		MinIntegrity: workflow.GitHubIntegrityApproved,
+	}
+
+	report := buildGuardPolicyDryRunReport("test-workflow.md", github)
+	require.NotNil(t, report)
+	assert.Equal(t, "test-workflow.md", report.Workflow)
+	assert.False(t, report.Lockdown)
+	assert.Equal(t, "owner/repo-a, owner/repo-b", report.PermittedRepos)
+	assert.Equal(t, "approved", report.MinIntegrity)
+}
+
+// TestBuildGuardPolicyDryRunReport_AllScope verifies that an "all" scope
+// (or omitted allowed-repos) is reported as such.
+func TestBuildGuardPolicyDryRunReport_AllScope(t *testing.T) {
+	github := &workflow.GitHubToolConfig{
+		MinIntegrity: workflow.GitHubIntegrityNone,
+	}
+
+	report := buildGuardPolicyDryRunReport("test-workflow.md", github)
+	require.NotNil(t, report)
+	assert.Equal(t, "all (default)", report.PermittedRepos)
+	assert.Equal(t, "none", report.MinIntegrity)
+}
+
+// TestBuildGuardPolicyDryRunReport_BlockedTrustedApproval verifies that
+// blocked-users, trusted-users, and approval-labels are surfaced in the report.
+func TestBuildGuardPolicyDryRunReport_BlockedTrustedApproval(t *testing.T) {
+	github := &workflow.GitHubToolConfig{
+		MinIntegrity:   workflow.GitHubIntegrityApproved,
+		BlockedUsers:   []string{"spam-bot"},
+		TrustedUsers:   []string{"trusted-user"},
+		ApprovalLabels: []string{"human-reviewed"},
+	}
+
+	report := buildGuardPolicyDryRunReport("test-workflow.md", github)
+	require.NotNil(t, report)
+	assert.Equal(t, []string{"spam-bot"}, report.BlockedUsers)
+	assert.Equal(t, []string{"trusted-user"}, report.TrustedUsers)
+	assert.Equal(t, []string{"human-reviewed"}, report.ApprovalLabels)
+
+	rendered := formatGuardPolicyDryRunReport(report)
+	assert.Contains(t, rendered, "blocked-users: spam-bot")
+	assert.Contains(t, rendered, "trusted-users: trusted-user")
+	assert.Contains(t, rendered, "approval-labels: human-reviewed")
+}
+
+// TestBuildGuardPolicyDryRunReport_Lockdown verifies that lockdown is
+// surfaced in the report to make clear the guard-policy fields are ignored
+// at runtime per §9.5 of scratchpad/github-mcp-access-control-specification.md.
+func TestBuildGuardPolicyDryRunReport_Lockdown(t *testing.T) {
+	github := &workflow.GitHubToolConfig{
+		Lockdown:     true,
+		AllowedRepos: "all",
+		MinIntegrity: workflow.GitHubIntegrityApproved,
+	}
+
+	report := buildGuardPolicyDryRunReport("test-workflow.md", github)
+	require.NotNil(t, report)
+	assert.True(t, report.Lockdown)
+
+	rendered := formatGuardPolicyDryRunReport(report)
+	assert.Contains(t, rendered, "lockdown: true")
+}
+
+// TestPrintGuardPolicyDryRunReport_OnlyWhenStrict verifies that the dry-run
+// report is only emitted when --strict is set, and only when guard-policy
+// fields are configured.
+func TestPrintGuardPolicyDryRunReport_OnlyWhenStrict(t *testing.T) {
+	workflowData := &workflow.WorkflowData{
+		ParsedTools: &workflow.Tools{
+			GitHub: &workflow.GitHubToolConfig{
+				MinIntegrity: workflow.GitHubIntegrityApproved,
+			},
+		},
+	}
+
+	stderrOutput := captureStderrForGuardPolicyReportTest(func() {
+		printGuardPolicyDryRunReport("test-workflow.md", workflowData, false)
+	})
+	assert.Empty(t, stderrOutput, "no report should be emitted when --strict is not set")
+
+	stderrOutput = captureStderrForGuardPolicyReportTest(func() {
+		printGuardPolicyDryRunReport("test-workflow.md", workflowData, true)
+	})
+	assert.Contains(t, stderrOutput, "guard policy dry-run report for test-workflow.md")
+
+	noGuardPolicyData := &workflow.WorkflowData{ParsedTools: &workflow.Tools{}}
+	stderrOutput = captureStderrForGuardPolicyReportTest(func() {
+		printGuardPolicyDryRunReport("test-workflow.md", noGuardPolicyData, true)
+	})
+	assert.Empty(t, stderrOutput, "no report should be emitted without guard-policy fields")
+}

pkg/cli/compile_workflow_processor.go

@@ -185,6 +185,11 @@ func compileWorkflowFile(
 	// Collect labels for JSON output (used by create-labels maintenance operation)
 	result.validationResult.Labels = extractSafeOutputLabels(workflowData)
 
+	// Emit a compile-time guard-policy dry-run report in --strict mode.
+	if !opts.jsonOutput {
+		printGuardPolicyDryRunReport(filepath.Base(resolvedFile), workflowData, opts.strict)
+	}
+
 	compileWorkflowProcessorLog.Printf("Successfully processed workflow file: %s", resolvedFile)
 	return result
 }