diff --git a/.github/workflows/daily-team-status.lock.yml b/.github/workflows/daily-team-status.lock.yml index 13f017fe93..43d703d315 100644 --- a/.github/workflows/daily-team-status.lock.yml +++ b/.github/workflows/daily-team-status.lock.yml @@ -2375,7 +2375,7 @@ jobs: timeout-minutes: 10 run: | set -o pipefail - sudo -E awf --env-all --container-workdir "${GITHUB_WORKSPACE}" --mount /tmp:/tmp:rw --mount "${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}:rw" --mount /usr/bin/date:/usr/bin/date:ro --mount /usr/bin/gh:/usr/bin/gh:ro --mount /usr/bin/yq:/usr/bin/yq:ro --mount /usr/local/bin/copilot:/usr/local/bin/copilot:ro --allow-domains api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.individual.githubcopilot.com,github.com,host.docker.internal,raw.githubusercontent.com,registry.npmjs.org --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs \ + sudo -E awf --env-all --container-workdir "${GITHUB_WORKSPACE}" --mount /tmp:/tmp:rw --mount "${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}:rw" --mount /usr/bin/date:/usr/bin/date:ro --mount /usr/bin/gh:/usr/bin/gh:ro --mount /usr/bin/yq:/usr/bin/yq:ro --mount /usr/local/bin/copilot:/usr/local/bin/copilot:ro --allow-domains api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,github.com,host.docker.internal,raw.githubusercontent.com,registry.npmjs.org --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs \ -- /usr/local/bin/copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/sandbox/agent/logs/ --add-dir "${GITHUB_WORKSPACE}" --disable-builtin-mcps --allow-tool github --allow-tool safeoutputs --prompt "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"${GH_AW_MODEL_AGENT_COPILOT:+ --model "$GH_AW_MODEL_AGENT_COPILOT"} \ 2>&1 | tee /tmp/gh-aw/agent-stdio.log env: @@ -2519,7 +2519,7 @@ jobs: uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }} - GH_AW_ALLOWED_DOMAINS: "api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.individual.githubcopilot.com,github.com,host.docker.internal,raw.githubusercontent.com,registry.npmjs.org" + GH_AW_ALLOWED_DOMAINS: "api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,github.com,host.docker.internal,raw.githubusercontent.com,registry.npmjs.org" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} with: diff --git a/docs/src/content/docs/labs.mdx b/docs/src/content/docs/labs.mdx index 708998d43d..010015d013 100644 --- a/docs/src/content/docs/labs.mdx +++ b/docs/src/content/docs/labs.mdx @@ -66,6 +66,7 @@ These are experimental agentic workflows used by the GitHub Next team to learn, | [Glossary Maintainer](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/glossary-maintainer.md) | copilot | [![Glossary Maintainer](https://github.com/githubnext/gh-aw/actions/workflows/glossary-maintainer.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/glossary-maintainer.lock.yml) | `0 10 * * 1-5` | - | | [Go Fan](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/go-fan.md) | claude | [![Go Fan](https://github.com/githubnext/gh-aw/actions/workflows/go-fan.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/go-fan.lock.yml) | `0 7 * * 1-5` | - | | [Go File Size Reduction Campaign](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/go-file-size-reduction.campaign.g.md) | copilot | [![Go File Size Reduction Campaign](https://github.com/githubnext/gh-aw/actions/workflows/go-file-size-reduction.campaign.g.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/go-file-size-reduction.campaign.g.lock.yml) | `0 18 * * *` | - | +| [Go File Size Reduction Campaign (Project 64)](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/go-file-size-reduction-project64.campaign.g.md) | copilot | [![Go File Size Reduction Campaign (Project 64)](https://github.com/githubnext/gh-aw/actions/workflows/go-file-size-reduction-project64.campaign.g.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/go-file-size-reduction-project64.campaign.g.lock.yml) | `0 18 * * *` | - | | [Go Logger Enhancement](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/go-logger.md) | claude | [![Go Logger Enhancement](https://github.com/githubnext/gh-aw/actions/workflows/go-logger.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/go-logger.lock.yml) | - | - | | [Go Pattern Detector](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/go-pattern-detector.md) | claude | [![Go Pattern Detector](https://github.com/githubnext/gh-aw/actions/workflows/go-pattern-detector.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/go-pattern-detector.lock.yml) | `0 14 * * 1-5` | - | | [Grumpy Code Reviewer 🔥](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/grumpy-reviewer.md) | copilot | [![Grumpy Code Reviewer 🔥](https://github.com/githubnext/gh-aw/actions/workflows/grumpy-reviewer.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/grumpy-reviewer.lock.yml) | - | - | @@ -77,6 +78,7 @@ These are experimental agentic workflows used by the GitHub Next team to learn, | [Issue Monster](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/issue-monster.md) | copilot | [![Issue Monster](https://github.com/githubnext/gh-aw/actions/workflows/issue-monster.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/issue-monster.lock.yml) | - | - | | [Issue Summary to Notion](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/notion-issue-summary.md) | copilot | [![Issue Summary to Notion](https://github.com/githubnext/gh-aw/actions/workflows/notion-issue-summary.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/notion-issue-summary.lock.yml) | - | - | | [Issue Triage Agent](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/issue-triage-agent.md) | copilot | [![Issue Triage Agent](https://github.com/githubnext/gh-aw/actions/workflows/issue-triage-agent.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/issue-triage-agent.lock.yml) | - | - | +| [jsweep - JavaScript Unbloater](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/jsweep.md) | copilot | [![jsweep - JavaScript Unbloater](https://github.com/githubnext/gh-aw/actions/workflows/jsweep.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/jsweep.lock.yml) | - | - | | [Layout Specification Maintainer](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/layout-spec-maintainer.md) | copilot | [![Layout Specification Maintainer](https://github.com/githubnext/gh-aw/actions/workflows/layout-spec-maintainer.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/layout-spec-maintainer.lock.yml) | `0 7 * * 1-5` | - | | [Lockfile Statistics Analysis Agent](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/lockfile-stats.md) | claude | [![Lockfile Statistics Analysis Agent](https://github.com/githubnext/gh-aw/actions/workflows/lockfile-stats.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/lockfile-stats.lock.yml) | - | - | | [MCP Inspector Agent](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/mcp-inspector.md) | copilot | [![MCP Inspector Agent](https://github.com/githubnext/gh-aw/actions/workflows/mcp-inspector.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/mcp-inspector.lock.yml) | - | - | diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index d84cf2ee73..f41732d702 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -134,13 +134,13 @@ func (c *Compiler) CompileWorkflowData(workflowData *WorkflowData, markdownPath // Validate permissions against GitHub MCP toolsets log.Printf("Validating permissions for GitHub MCP toolsets") - if githubTool, hasGitHub := workflowData.Tools["github"]; hasGitHub { + if workflowData.ParsedTools != nil && workflowData.ParsedTools.GitHub != nil { // Parse permissions from the workflow data // WorkflowData.Permissions contains the raw YAML string (including "permissions:" prefix) permissions := NewPermissionsParser(workflowData.Permissions).ToPermissions() - // Validate permissions - validationResult := ValidatePermissions(permissions, githubTool) + // Validate permissions using the typed GitHub tool configuration + validationResult := ValidatePermissions(permissions, workflowData.ParsedTools.GitHub) if validationResult.HasValidationIssues { // Format the validation message diff --git a/pkg/workflow/permissions_validator.go b/pkg/workflow/permissions_validator.go index a432b4cfdb..467d93958e 100644 --- a/pkg/workflow/permissions_validator.go +++ b/pkg/workflow/permissions_validator.go @@ -81,6 +81,35 @@ func GetToolsetsData() GitHubToolsetsData { return data } +// ValidatableTool represents a tool configuration that can be validated for permissions +// This interface abstracts the tool configuration structure to enable type-safe permission validation +type ValidatableTool interface { + // GetToolsets returns the comma-separated list of toolsets configured for this tool + GetToolsets() string + // IsReadOnly returns whether the tool is configured in read-only mode + IsReadOnly() bool +} + +// GetToolsets implements ValidatableTool for GitHubToolConfig +func (g *GitHubToolConfig) GetToolsets() string { + if g == nil { + // Should not happen - ValidatePermissions checks for nil before calling this + return "" + } + // Convert toolset array to comma-separated string + // If empty, expandDefaultToolset will apply defaults + toolsetsStr := strings.Join(g.Toolset, ",") + return expandDefaultToolset(toolsetsStr) +} + +// IsReadOnly implements ValidatableTool for GitHubToolConfig +func (g *GitHubToolConfig) IsReadOnly() bool { + if g == nil { + return true // default to read-only for security + } + return g.ReadOnly +} + // PermissionsValidationResult contains the result of permissions validation type PermissionsValidationResult struct { MissingPermissions map[PermissionScope]PermissionLevel // Permissions required but not granted @@ -95,21 +124,14 @@ type PermissionsValidationResult struct { // // Parameters: // - permissions: The workflow's declared permissions -// - githubTool: The GitHub tool configuration (uses any type because the structure varies -// based on the engine and toolsets being used. This is an appropriate use of any for -// dynamic configuration data that cannot be known at compile time.) +// - githubTool: The GitHub tool configuration implementing ValidatableTool interface // // Returns: // - A validation result indicating any missing permissions and which toolsets require them // -// Type Pattern Note: githubTool uses 'any' because tool configuration structure is dynamic -// and varies based on engine (copilot, claude, codex) and mode (local, remote). This is -// parsed from YAML frontmatter where the structure is not known until runtime. -// See specs/go-type-patterns.md for guidance on when to use 'any' types. -// // Use ValidatePermissions (this function) for general permission validation against GitHub MCP toolsets. // Use ValidateIncludedPermissions (in imports.go) when validating permissions from included/imported workflow files. -func ValidatePermissions(permissions *Permissions, githubTool any) *PermissionsValidationResult { +func ValidatePermissions(permissions *Permissions, githubTool ValidatableTool) *PermissionsValidationResult { permissionsValidatorLog.Print("Starting permissions validation") result := &PermissionsValidationResult{ @@ -118,14 +140,21 @@ func ValidatePermissions(permissions *Permissions, githubTool any) *PermissionsV } // If GitHub tool is not configured, no validation needed + // Check both for nil interface and nil concrete type if githubTool == nil { - permissionsValidatorLog.Print("No GitHub tool configured, skipping validation") + permissionsValidatorLog.Print("No GitHub tool configured (nil interface), skipping validation") + return result + } + + // Check if concrete type is nil (interface wrapping nil pointer) + if config, ok := githubTool.(*GitHubToolConfig); ok && config == nil { + permissionsValidatorLog.Print("No GitHub tool configured (nil concrete type), skipping validation") return result } // Extract toolsets from GitHub tool configuration - toolsetsStr := getGitHubToolsets(githubTool) - readOnly := getGitHubReadOnly(githubTool) + toolsetsStr := githubTool.GetToolsets() + readOnly := githubTool.IsReadOnly() result.ReadOnlyMode = readOnly permissionsValidatorLog.Printf("Validating toolsets: %s, read-only: %v", toolsetsStr, readOnly) diff --git a/pkg/workflow/permissions_validator_test.go b/pkg/workflow/permissions_validator_test.go index 723ed40fc2..8c4b6cbef5 100644 --- a/pkg/workflow/permissions_validator_test.go +++ b/pkg/workflow/permissions_validator_test.go @@ -123,7 +123,7 @@ func TestValidatePermissions_MissingPermissions(t *testing.T) { tests := []struct { name string permissions *Permissions - githubTool any + githubToolConfig *GitHubToolConfig expectMissing map[PermissionScope]PermissionLevel expectMissingCount int expectHasIssues bool @@ -131,7 +131,7 @@ func TestValidatePermissions_MissingPermissions(t *testing.T) { { name: "No GitHub tool configured", permissions: NewPermissions(), - githubTool: nil, + githubToolConfig: nil, expectMissing: map[PermissionScope]PermissionLevel{}, expectMissingCount: 0, expectHasIssues: false, @@ -139,8 +139,8 @@ func TestValidatePermissions_MissingPermissions(t *testing.T) { { name: "Default toolsets with no permissions", permissions: NewPermissions(), - githubTool: map[string]any{ - "toolsets": []string{"default"}, + githubToolConfig: &GitHubToolConfig{ + Toolset: []string{"default"}, }, expectMissingCount: 3, // contents, issues, pull-requests expectHasIssues: true, @@ -152,9 +152,9 @@ func TestValidatePermissions_MissingPermissions(t *testing.T) { PermissionIssues: PermissionWrite, PermissionPullRequests: PermissionWrite, }), - githubTool: map[string]any{ - "toolsets": []string{"default"}, - "read-only": false, + githubToolConfig: &GitHubToolConfig{ + Toolset: []string{"default"}, + ReadOnly: false, }, expectMissingCount: 0, expectHasIssues: false, @@ -166,9 +166,9 @@ func TestValidatePermissions_MissingPermissions(t *testing.T) { PermissionIssues: PermissionRead, PermissionPullRequests: PermissionRead, }), - githubTool: map[string]any{ - "toolsets": []string{"default"}, - "read-only": false, // Need write permissions + githubToolConfig: &GitHubToolConfig{ + Toolset: []string{"default"}, + ReadOnly: false, // Need write permissions }, expectMissingCount: 3, // All need write expectHasIssues: true, @@ -180,9 +180,9 @@ func TestValidatePermissions_MissingPermissions(t *testing.T) { PermissionIssues: PermissionRead, PermissionPullRequests: PermissionRead, }), - githubTool: map[string]any{ - "toolsets": []string{"default"}, - "read-only": true, + githubToolConfig: &GitHubToolConfig{ + Toolset: []string{"default"}, + ReadOnly: true, }, expectMissingCount: 0, expectHasIssues: false, @@ -192,9 +192,9 @@ func TestValidatePermissions_MissingPermissions(t *testing.T) { permissions: NewPermissionsFromMap(map[PermissionScope]PermissionLevel{ PermissionContents: PermissionWrite, }), - githubTool: map[string]any{ - "toolsets": []string{"repos", "issues"}, - "read-only": false, + githubToolConfig: &GitHubToolConfig{ + Toolset: []string{"repos", "issues"}, + ReadOnly: false, }, expectMissingCount: 1, // Missing issues: write expectHasIssues: true, @@ -204,8 +204,8 @@ func TestValidatePermissions_MissingPermissions(t *testing.T) { permissions: NewPermissionsFromMap(map[PermissionScope]PermissionLevel{ PermissionActions: PermissionRead, }), - githubTool: map[string]any{ - "toolsets": []string{"actions"}, + githubToolConfig: &GitHubToolConfig{ + Toolset: []string{"actions"}, }, expectMissingCount: 0, expectHasIssues: false, @@ -214,7 +214,7 @@ func TestValidatePermissions_MissingPermissions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := ValidatePermissions(tt.permissions, tt.githubTool) + result := ValidatePermissions(tt.permissions, tt.githubToolConfig) if len(result.MissingPermissions) != tt.expectMissingCount { t.Errorf("Expected %d missing permissions, got %d: %v", @@ -336,17 +336,17 @@ func TestToolsetPermissionsMapping(t *testing.T) { func TestValidatePermissions_ComplexScenarios(t *testing.T) { tests := []struct { - name string - permissions *Permissions - githubTool any - expectMsg []string + name string + permissions *Permissions + githubToolConfig *GitHubToolConfig + expectMsg []string }{ { name: "Shorthand read-all with default toolsets", permissions: NewPermissionsReadAll(), - githubTool: map[string]any{ - "toolsets": []string{"default"}, - "read-only": false, + githubToolConfig: &GitHubToolConfig{ + Toolset: []string{"default"}, + ReadOnly: false, }, expectMsg: []string{ "Missing required permissions for github toolsets:", @@ -358,9 +358,9 @@ func TestValidatePermissions_ComplexScenarios(t *testing.T) { { name: "All: read with discussions toolset", permissions: NewPermissionsAllRead(), - githubTool: map[string]any{ - "toolsets": []string{"discussions"}, - "read-only": false, + githubToolConfig: &GitHubToolConfig{ + Toolset: []string{"discussions"}, + ReadOnly: false, }, expectMsg: []string{ "Missing required permissions for github toolsets:", @@ -371,7 +371,7 @@ func TestValidatePermissions_ComplexScenarios(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := ValidatePermissions(tt.permissions, tt.githubTool) + result := ValidatePermissions(tt.permissions, tt.githubToolConfig) message := FormatValidationMessage(result, false) for _, expected := range tt.expectMsg { diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index 90b194015b..8538cc794f 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -344,19 +344,25 @@ func NewTools(toolsMap map[string]any) *Tools { func parseGitHubTool(val any) *GitHubToolConfig { if val == nil { toolsTypesLog.Print("GitHub tool enabled with default configuration") - return &GitHubToolConfig{} + return &GitHubToolConfig{ + ReadOnly: true, // default to read-only for security + } } // Handle string type (simple enable) if _, ok := val.(string); ok { toolsTypesLog.Print("GitHub tool enabled with string configuration") - return &GitHubToolConfig{} + return &GitHubToolConfig{ + ReadOnly: true, // default to read-only for security + } } // Handle map type (detailed configuration) if configMap, ok := val.(map[string]any); ok { toolsTypesLog.Print("Parsing GitHub tool detailed configuration") - config := &GitHubToolConfig{} + config := &GitHubToolConfig{ + ReadOnly: true, // default to read-only for security + } if allowed, ok := configMap["allowed"].([]any); ok { config.Allowed = make([]string, 0, len(allowed)) @@ -387,6 +393,7 @@ func parseGitHubTool(val any) *GitHubToolConfig { if readOnly, ok := configMap["read-only"].(bool); ok { config.ReadOnly = readOnly } + // else: defaults to true (set above) if token, ok := configMap["github-token"].(string); ok { config.GitHubToken = token @@ -416,7 +423,9 @@ func parseGitHubTool(val any) *GitHubToolConfig { return config } - return &GitHubToolConfig{} + return &GitHubToolConfig{ + ReadOnly: true, // default to read-only for security + } } // parseBashTool converts raw bash tool configuration to BashToolConfig