diff --git a/.github/workflows/ai-moderator.lock.yml b/.github/workflows/ai-moderator.lock.yml index 927b0afe26c..64ae5d24561 100644 --- a/.github/workflows/ai-moderator.lock.yml +++ b/.github/workflows/ai-moderator.lock.yml @@ -22,7 +22,7 @@ # For more information: https://github.github.com/gh-aw/introduction/overview/ # # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"c914e6148e6bd4fd2f3bf55bbf79dcfe6291fbc8c32ea6f9953192c581ac5d3a"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"14e92fd69590d29ed79eebb40f8ef8df9e15cd886bbe89479566694376c392a5"} name: "AI Moderator" "on": @@ -38,6 +38,7 @@ name: "AI Moderator" # forks: "*" # Fork filtering applied via job conditions types: - opened + roles: all # skip-bots: # Skip-bots processed as bot check in pre-activation job # - github-actions # Skip-bots processed as bot check in pre-activation job # - copilot # Skip-bots processed as bot check in pre-activation job diff --git a/.github/workflows/ai-moderator.md b/.github/workflows/ai-moderator.md index a74fe54dee4..e8ef6b2d362 100644 --- a/.github/workflows/ai-moderator.md +++ b/.github/workflows/ai-moderator.md @@ -1,7 +1,7 @@ --- timeout-minutes: 5 -roles: all on: + roles: all issues: types: [opened] lock-for-agent: true @@ -38,6 +38,7 @@ safe-outputs: allowed-reasons: [spam] threat-detection: false --- + # AI Moderator You are an AI-powered moderation system that automatically detects spam, link spam, and AI-generated content in GitHub issues and comments. @@ -131,4 +132,4 @@ Based on your analysis: - Technical discussions may naturally contain links to resources, documentation, or related issues - New contributors may have less polished writing - this doesn't necessarily indicate AI generation - Provide clear reasoning for each detection in your analysis -- Only take action if you have high confidence in the detection +- Only take action if you have high confidence in the detection \ No newline at end of file diff --git a/.github/workflows/poem-bot.lock.yml b/.github/workflows/poem-bot.lock.yml index 2b2bf41ead8..fb68c838019 100644 --- a/.github/workflows/poem-bot.lock.yml +++ b/.github/workflows/poem-bot.lock.yml @@ -28,7 +28,7 @@ # - shared/mood.md # - shared/reporting.md # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"bea51591fd002f37dca347dc9f8a310ca504787ad90f32fe81ee688a380028f3"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"81547dd4016c398ec096fdae259f7510c7cedd738270ca04908bfbed2afa1434"} name: "Poem Bot - A Creative Agentic Workflow" "on": @@ -37,6 +37,9 @@ name: "Poem Bot - A Creative Agentic Workflow" - opened - edited - reopened + roles: + - admin + - maintainer workflow_dispatch: inputs: poem_theme: diff --git a/.github/workflows/poem-bot.md b/.github/workflows/poem-bot.md index 683f9dd8884..986ea88535b 100644 --- a/.github/workflows/poem-bot.md +++ b/.github/workflows/poem-bot.md @@ -2,6 +2,9 @@ description: Generates creative poems on specified themes when invoked with /poem-bot command # Custom triggers: command with events filter, workflow_dispatch on: + roles: + - admin + - maintainer # Command trigger - responds to /poem-bot mentions slash_command: name: poem-bot @@ -15,11 +18,6 @@ on: required: false default: 'technology and automation' -# Restrict to admin/maintainer roles only -roles: - - admin - - maintainer - # Minimal permissions - safe-outputs handles write operations permissions: contents: read diff --git a/.github/workflows/q.lock.yml b/.github/workflows/q.lock.yml index 4605a73686b..92594ae2753 100644 --- a/.github/workflows/q.lock.yml +++ b/.github/workflows/q.lock.yml @@ -27,7 +27,7 @@ # Imports: # - shared/mood.md # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"b62e483e5601e35c59a7d7e4337e705aeac232b879669f48eb40590599d0d4d7"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"d7ffc854b27bffb4edd06d2b7b04a3a8a1a94f914dad44da5d7f379733d6bed6"} name: "Q" "on": @@ -57,6 +57,10 @@ name: "Q" types: - created - edited + roles: + - admin + - maintainer + - write permissions: {} @@ -69,7 +73,9 @@ jobs: activation: needs: pre_activation if: > - (needs.pre_activation.outputs.activated == 'true') && ((github.event_name == 'issues') && ((startsWith(github.event.issue.body, '/q ')) || + (needs.pre_activation.outputs.activated == 'true') && (((github.event_name == 'issues' || github.event_name == 'issue_comment' || + github.event_name == 'pull_request' || github.event_name == 'pull_request_review_comment' || github.event_name == 'discussion' || + github.event_name == 'discussion_comment') && ((github.event_name == 'issues') && ((startsWith(github.event.issue.body, '/q ')) || (github.event.issue.body == '/q')) || (github.event_name == 'issue_comment') && (((startsWith(github.event.comment.body, '/q ')) || (github.event.comment.body == '/q')) && (github.event.issue.pull_request == null)) || (github.event_name == 'issue_comment') && (((startsWith(github.event.comment.body, '/q ')) || (github.event.comment.body == '/q')) && (github.event.issue.pull_request != null)) || @@ -77,7 +83,9 @@ jobs: (github.event.comment.body == '/q')) || (github.event_name == 'pull_request') && ((startsWith(github.event.pull_request.body, '/q ')) || (github.event.pull_request.body == '/q')) || (github.event_name == 'discussion') && ((startsWith(github.event.discussion.body, '/q ')) || (github.event.discussion.body == '/q')) || (github.event_name == 'discussion_comment') && ((startsWith(github.event.comment.body, '/q ')) || - (github.event.comment.body == '/q'))) + (github.event.comment.body == '/q')))) || (!(github.event_name == 'issues' || github.event_name == 'issue_comment' || + github.event_name == 'pull_request' || github.event_name == 'pull_request_review_comment' || github.event_name == 'discussion' || + github.event_name == 'discussion_comment'))) runs-on: ubuntu-slim permissions: contents: read @@ -1243,7 +1251,9 @@ jobs: pre_activation: if: > - (github.event_name == 'issues') && ((startsWith(github.event.issue.body, '/q ')) || (github.event.issue.body == '/q')) || + ((github.event_name == 'issues' || github.event_name == 'issue_comment' || github.event_name == 'pull_request' || + github.event_name == 'pull_request_review_comment' || github.event_name == 'discussion' || github.event_name == 'discussion_comment') && + ((github.event_name == 'issues') && ((startsWith(github.event.issue.body, '/q ')) || (github.event.issue.body == '/q')) || (github.event_name == 'issue_comment') && (((startsWith(github.event.comment.body, '/q ')) || (github.event.comment.body == '/q')) && (github.event.issue.pull_request == null)) || (github.event_name == 'issue_comment') && (((startsWith(github.event.comment.body, '/q ')) || (github.event.comment.body == '/q')) && (github.event.issue.pull_request != null)) || (github.event_name == 'pull_request_review_comment') && @@ -1253,7 +1263,9 @@ jobs: (github.event_name == 'discussion') && ((startsWith(github.event.discussion.body, '/q ')) || (github.event.discussion.body == '/q')) || (github.event_name == 'discussion_comment') && - ((startsWith(github.event.comment.body, '/q ')) || (github.event.comment.body == '/q')) + ((startsWith(github.event.comment.body, '/q ')) || (github.event.comment.body == '/q')))) || (!(github.event_name == 'issues' || + github.event_name == 'issue_comment' || github.event_name == 'pull_request' || github.event_name == 'pull_request_review_comment' || + github.event_name == 'discussion' || github.event_name == 'discussion_comment')) runs-on: ubuntu-slim permissions: contents: read diff --git a/.github/workflows/q.md b/.github/workflows/q.md index 9b7ff94331b..403ef26511e 100644 --- a/.github/workflows/q.md +++ b/.github/workflows/q.md @@ -2,6 +2,7 @@ name: Q description: Intelligent assistant that answers questions, analyzes repositories, and can create PRs for workflow optimizations on: + roles: [admin, maintainer, write] slash_command: name: q reaction: rocket @@ -11,7 +12,6 @@ permissions: issues: read pull-requests: read discussions: read -roles: [admin, maintainer, write] engine: copilot tools: agentic-workflows: diff --git a/.github/workflows/release.lock.yml b/.github/workflows/release.lock.yml index a56f2d26754..1f6ce717971 100644 --- a/.github/workflows/release.lock.yml +++ b/.github/workflows/release.lock.yml @@ -27,10 +27,13 @@ # Imports: # - shared/mood.md # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"cc2b8c6b51e275896e8102fe283fdb0e690e468cd8e5693425a35d14bb622645"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"babf32bc0f17f143da2d3cc6c7216050da644b62975734638fce5fb7b16f3994"} name: "Release" "on": + roles: + - admin + - maintainer workflow_dispatch: inputs: release_type: diff --git a/.github/workflows/release.md b/.github/workflows/release.md index eb1d4b864e8..31a162b4f06 100644 --- a/.github/workflows/release.md +++ b/.github/workflows/release.md @@ -2,6 +2,9 @@ name: Release description: Build, test, and release gh-aw extension, then generate and prepend release highlights on: + roles: + - admin + - maintainer workflow_dispatch: inputs: release_type: @@ -17,9 +20,6 @@ permissions: pull-requests: read actions: read issues: read -roles: - - admin - - maintainer engine: copilot strict: false timeout-minutes: 20 @@ -509,4 +509,4 @@ safeoutputs/update_release( - Reference: `https://github.github.com/gh-aw/reference/` - Setup: `https://github.github.com/gh-aw/setup/` -Verify paths exist in `docs_files.txt` before linking. \ No newline at end of file +Verify paths exist in `docs_files.txt` before linking. diff --git a/.github/workflows/scout.lock.yml b/.github/workflows/scout.lock.yml index 05f05b6c2b5..482ef70fe21 100644 --- a/.github/workflows/scout.lock.yml +++ b/.github/workflows/scout.lock.yml @@ -34,7 +34,7 @@ # - shared/mood.md # - shared/reporting.md # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"5f21df90d95bad88fa40d8a04f659e6cda4363abf33fd54a37fa44884fbb0355"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"f4dcca3eea6547e9841ec07c4248de1cf7d8c54ad12cf47c63edca5a5fd3b472"} name: "Scout" "on": @@ -64,6 +64,10 @@ name: "Scout" types: - created - edited + roles: + - admin + - maintainer + - write workflow_dispatch: inputs: topic: diff --git a/.github/workflows/scout.md b/.github/workflows/scout.md index d3cc70e3aff..0fc10c80c23 100644 --- a/.github/workflows/scout.md +++ b/.github/workflows/scout.md @@ -2,6 +2,7 @@ name: Scout description: Performs deep research investigations using web search to gather and synthesize comprehensive information on any topic on: + roles: [admin, maintainer, write] slash_command: name: scout workflow_dispatch: @@ -13,7 +14,6 @@ permissions: contents: read issues: read pull-requests: read -roles: [admin, maintainer, write] engine: claude imports: - shared/mood.md @@ -187,4 +187,4 @@ Focus on the most relevant and actionable information. Avoid overwhelming detail - **Clarity**: Write for the intended audience (developers working on this repo) - **Attribution**: Always cite your sources with proper links -Remember: Your goal is to provide valuable, actionable intelligence that helps resolve the issue or improve the pull request. Make every search count and synthesize information effectively. +Remember: Your goal is to provide valuable, actionable intelligence that helps resolve the issue or improve the pull request. Make every search count and synthesize information effectively. \ No newline at end of file diff --git a/docs/src/content/docs/patterns/dispatchops.md b/docs/src/content/docs/patterns/dispatchops.md index fefc53ab861..a6c2a34ea9a 100644 --- a/docs/src/content/docs/patterns/dispatchops.md +++ b/docs/src/content/docs/patterns/dispatchops.md @@ -95,12 +95,12 @@ Deploy to the ${{ github.event.inputs.target_env }} environment. Manual workflow execution respects the same security model as other triggers: - **Repository permissions** - User must have write access or higher to trigger workflows -- **Role-based access** - Use the `roles:` field to restrict who can run workflows: +- **Role-based access** - Use the `on.roles` field to restrict who can run workflows: ```yaml on: workflow_dispatch: -roles: [admin, maintainer] + roles: [admin, maintainer] ``` - **Bot authorization** - Use the `bots:` field to allow specific bot accounts: diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 2c2301da99b..882381c1f75 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -3732,22 +3732,6 @@ secret-masking: # (optional) steps: [] -# Repository access roles required to trigger agentic workflows. Defaults to -# ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any -# authenticated user (⚠️ security consideration). -# (optional) -# This field supports multiple formats (oneOf): - -# Option 1: Allow any authenticated user to trigger the workflow (⚠️ disables -# permission checking entirely - use with caution) -roles: "all" - -# Option 2: List of repository permission levels that can trigger the workflow. -# Permission checks are automatically applied to potentially unsafe triggers. -roles: [] - # Array items: Repository permission level: 'admin' (full access), - # 'maintainer'/'maintain' (repository management), 'write' (push access), 'triage' - # (issue management) # Allow list of bot identifiers that can trigger the workflow even if they don't # meet the required role permissions. When the actor is in this list, the bot must diff --git a/docs/src/content/docs/reference/frontmatter.md b/docs/src/content/docs/reference/frontmatter.md index 45d011a9dfc..833d5bdbbee 100644 --- a/docs/src/content/docs/reference/frontmatter.md +++ b/docs/src/content/docs/reference/frontmatter.md @@ -22,7 +22,7 @@ tools: ## Frontmatter Elements -The frontmatter combines standard GitHub Actions properties (`on`, `permissions`, `run-name`, `runs-on`, `timeout-minutes`, `concurrency`, `env`, `environment`, `container`, `services`, `if`, `steps`, `cache`) with GitHub Agentic Workflows-specific elements (`description`, `source`, `imports`, `engine`, `strict`, `roles`, `features`, `plugins`, `runtimes`, `safe-inputs`, `safe-outputs`, `network`, `tools`). +The frontmatter combines standard GitHub Actions properties (`on`, `permissions`, `run-name`, `runs-on`, `timeout-minutes`, `concurrency`, `env`, `environment`, `container`, `services`, `if`, `steps`, `cache`) with GitHub Agentic Workflows-specific elements (`description`, `source`, `imports`, `engine`, `strict`, `features`, `plugins`, `runtimes`, `safe-inputs`, `safe-outputs`, `network`, `tools`). Tool configurations (such as `bash`, `edit`, `github`, `web-fetch`, `web-search`, `playwright`, `cache-memory`, and custom [Model Context Protocol](/gh-aw/reference/glossary/#mcp-model-context-protocol) (MCP) [servers](/gh-aw/reference/glossary/#mcp-server)) are specified under the `tools:` key. Custom inline tools can be defined with the [`safe-inputs:`](/gh-aw/reference/safe-inputs/) (custom tools defined inline) key. See [Tools](/gh-aw/reference/tools/) and [Safe Inputs](/gh-aw/reference/safe-inputs/) for complete documentation. @@ -35,6 +35,7 @@ The `on:` section uses standard GitHub Actions syntax to define workflow trigger - `stop-after:` - Automatically disable triggers after a deadline - `manual-approval:` - Require manual approval using environment protection rules - `forks:` - Configure fork filtering for pull_request triggers +- `roles:` - Control who can trigger the workflow based on repository permission level - `skip-roles:` - Skip workflow execution for specific repository roles - `skip-bots:` - Skip workflow execution for specific GitHub actors @@ -211,17 +212,30 @@ The compiler validates workflows have sufficient permissions for their configure **Strict mode** (`gh aw compile --strict`): Treats under-provisioned permissions as compilation errors. Use for production workflows requiring enhanced security validation. -### Repository Access Roles (`roles:`) +### Repository Access Roles (`on.roles`) Controls who can trigger agentic workflows based on repository permission level. Defaults to `[admin, maintainer, write]`. ```yaml wrap -roles: [admin, maintainer, write] # Default -roles: all # Allow any user (⚠️ use with caution) +on: + issues: + types: [opened] + roles: [admin, maintainer, write] # Default +``` + +```yaml wrap +on: + issues: + types: [opened] + roles: all # Allow any user (⚠️ use with caution) ``` Available roles: `admin`, `maintainer`, `write`, `read`, `all`. Workflows with unsafe triggers (`push`, `issues`, `pull_request`) automatically enforce permission checks. Failed checks cancel the workflow with a warning. +:::note[Migration from top-level roles] +The `roles` field was previously a top-level frontmatter field. It has been moved to `on.roles` and top-level `roles` is no longer supported. Use `gh aw fix` to automatically migrate existing workflows. +::: + ### Bot Filtering (`bots:`) Configure which GitHub bot accounts can trigger workflows. Useful for allowing specific automation bots while maintaining security controls. @@ -236,9 +250,9 @@ bots: **Behavior**: - When specified, only the listed bot accounts can trigger the workflow - The bot must be active (installed) on the repository to trigger the workflow -- Combine with `roles:` for comprehensive access control +- Combine with `on.roles` for comprehensive access control - Applies to all workflow triggers (`pull_request`, `issues`, etc.) -- When `roles: all` is set, bot filtering is not enforced +- When `on.roles: all` is set, bot filtering is not enforced **Common bot names**: - `dependabot[bot]` - GitHub Dependabot for dependency updates diff --git a/pkg/cli/codemod_roles.go b/pkg/cli/codemod_roles.go new file mode 100644 index 00000000000..591f194f363 --- /dev/null +++ b/pkg/cli/codemod_roles.go @@ -0,0 +1,209 @@ +package cli + +import ( + "fmt" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var rolesCodemodLog = logger.New("cli:codemod_roles") + +// getRolesToOnRolesCodemod creates a codemod for moving top-level 'roles' to 'on.roles' +func getRolesToOnRolesCodemod() Codemod { + return Codemod{ + ID: "roles-to-on-roles", + Name: "Move roles to on.roles", + Description: "Moves the top-level 'roles' field to 'on.roles' as per the new frontmatter structure", + IntroducedIn: "0.10.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + // Check if top-level roles exists + _, hasTopLevelRoles := frontmatter["roles"] + if !hasTopLevelRoles { + return content, false, nil + } + + // Check if on.roles already exists (shouldn't happen, but be safe) + if onValue, hasOn := frontmatter["on"]; hasOn { + if onMap, ok := onValue.(map[string]any); ok { + if _, hasOnRoles := onMap["roles"]; hasOnRoles { + rolesCodemodLog.Print("Both top-level 'roles' and 'on.roles' exist - skipping migration") + return content, false, nil + } + } + } + + // Parse frontmatter to get raw lines + frontmatterLines, markdown, err := parseFrontmatterLines(content) + if err != nil { + return content, false, err + } + + // Find roles line and on: block + var rolesLineIdx = -1 + var rolesLineValue string + var onBlockIdx = -1 + var onIndent string + + // First pass: find the roles line and on: block + for i, line := range frontmatterLines { + trimmedLine := strings.TrimSpace(line) + + // Find top-level roles + if isTopLevelKey(line) && strings.HasPrefix(trimmedLine, "roles:") { + rolesLineIdx = i + // Extract the value (could be on same line or on next lines) + parts := strings.SplitN(line, ":", 2) + if len(parts) == 2 { + rolesLineValue = strings.TrimSpace(parts[1]) + } + rolesCodemodLog.Printf("Found top-level roles at line %d", i+1) + } + + // Find on: block + if isTopLevelKey(line) && strings.HasPrefix(trimmedLine, "on:") { + onBlockIdx = i + onIndent = getIndentation(line) + rolesCodemodLog.Printf("Found 'on:' block at line %d", i+1) + } + } + + // If no roles found, nothing to do + if rolesLineIdx == -1 { + return content, false, nil + } + + // Determine how roles is formatted + var rolesLines []string + var rolesEndIdx int + + if rolesLineValue == "all" || strings.HasPrefix(rolesLineValue, "[") { + // roles: all or roles: [admin, write] - single line format + rolesLines = []string{frontmatterLines[rolesLineIdx]} + rolesEndIdx = rolesLineIdx + } else { + // Multi-line array format OR roles: with empty value + // Find all lines that are part of the roles block + rolesStartIndent := getIndentation(frontmatterLines[rolesLineIdx]) + rolesLines = append(rolesLines, frontmatterLines[rolesLineIdx]) + rolesEndIdx = rolesLineIdx + + for j := rolesLineIdx + 1; j < len(frontmatterLines); j++ { + line := frontmatterLines[j] + trimmed := strings.TrimSpace(line) + + // Empty lines or comments might be part of the block + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + rolesLines = append(rolesLines, line) + rolesEndIdx = j + continue + } + + // Check if still in the roles block (indented more than roles:) + if isNestedUnder(line, rolesStartIndent) { + rolesLines = append(rolesLines, line) + rolesEndIdx = j + } else { + // Exited the block + break + } + } + } + + rolesCodemodLog.Printf("Roles spans lines %d to %d (%d lines)", rolesLineIdx+1, rolesEndIdx+1, len(rolesLines)) + + // If no on: block found, we need to create one + result := make([]string, 0, len(frontmatterLines)) + modified := false + + if onBlockIdx == -1 { + // No on: block exists - create one with roles inside it + rolesCodemodLog.Print("No 'on:' block found - creating new one with roles") + + for i, line := range frontmatterLines { + if i >= rolesLineIdx && i <= rolesEndIdx { + // Skip the original roles lines - we'll add them to the new on: block + if i == rolesLineIdx { + // Add new on: block with roles inside + result = append(result, "on:") + // Add roles lines with proper indentation + for _, rolesLine := range rolesLines { + trimmed := strings.TrimSpace(rolesLine) + if trimmed == "" { + result = append(result, rolesLine) + } else if strings.HasPrefix(trimmed, "roles:") { + // roles: line gets 2 spaces (nested under on:) + result = append(result, " "+rolesLine) + } else { + // Array items get 4 spaces (nested under on: and roles:) + result = append(result, " "+trimmed) + } + } + modified = true + } + // Skip all other roles lines + continue + } + result = append(result, line) + } + } else { + // on: block exists - add roles to it + rolesCodemodLog.Print("Found 'on:' block - adding roles to it") + + // Determine indentation for items inside on: block + onItemIndent := onIndent + " " + + // Track if we've inserted roles + insertedRoles := false + + for i, line := range frontmatterLines { + // Skip the original roles lines + if i >= rolesLineIdx && i <= rolesEndIdx { + modified = true + continue + } + + // Add the line + result = append(result, line) + + // After the on: line, insert roles + if i == onBlockIdx && !insertedRoles { + // Add roles lines with proper indentation inside on: block + for _, rolesLine := range rolesLines { + trimmed := strings.TrimSpace(rolesLine) + if trimmed == "" { + result = append(result, rolesLine) + } else { + // Adjust indentation to be nested under on: + // Remove "roles:" prefix and re-add with proper indentation + if strings.HasPrefix(trimmed, "roles:") { + // roles: value or roles: + parts := strings.SplitN(trimmed, ":", 2) + if len(parts) == 2 { + result = append(result, fmt.Sprintf("%sroles:%s", onItemIndent, parts[1])) + } else { + result = append(result, fmt.Sprintf("%sroles:", onItemIndent)) + } + } else { + // Array item line (e.g., "- admin") + // These should be indented 2 more spaces than roles: to be nested under it + result = append(result, onItemIndent+" "+trimmed) + } + } + } + insertedRoles = true + } + } + } + + if !modified { + return content, false, nil + } + + // Reconstruct the content + newContent := reconstructContent(result, markdown) + rolesCodemodLog.Print("Successfully migrated top-level 'roles' to 'on.roles'") + return newContent, true, nil + }, + } +} diff --git a/pkg/cli/codemod_roles_test.go b/pkg/cli/codemod_roles_test.go new file mode 100644 index 00000000000..e4cef277044 --- /dev/null +++ b/pkg/cli/codemod_roles_test.go @@ -0,0 +1,237 @@ +//go:build !integration + +package cli + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetRolesToOnRolesCodemod(t *testing.T) { + codemod := getRolesToOnRolesCodemod() + + assert.Equal(t, "roles-to-on-roles", codemod.ID) + assert.Equal(t, "Move roles to on.roles", codemod.Name) + assert.NotEmpty(t, codemod.Description) + assert.Equal(t, "0.10.0", codemod.IntroducedIn) + require.NotNil(t, codemod.Apply) +} + +func TestRolesToOnRolesCodemod_SingleLineArray(t *testing.T) { + codemod := getRolesToOnRolesCodemod() + + content := `--- +on: + issues: + types: [opened] +roles: [admin, maintainer, write] +--- + +# Test workflow` + + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + }, + "roles": []any{"admin", "maintainer", "write"}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "on:") + assert.Contains(t, result, "roles: [admin, maintainer, write]") + assert.NotContains(t, result, "\nroles: [admin, maintainer, write]") + // Ensure roles is nested under on: + lines := strings.Split(result, "\n") + foundOn := false + foundRoles := false + for _, line := range lines { + if line == "on:" { + foundOn = true + } + if foundOn && strings.Contains(line, "roles:") { + foundRoles = true + // Check that roles line has indentation (nested under on:) + assert.Greater(t, len(line), len(strings.TrimSpace(line)), "roles should be indented under on:") + break + } + } + assert.True(t, foundRoles, "Should find roles nested under on:") +} + +func TestRolesToOnRolesCodemod_MultiLineArray(t *testing.T) { + codemod := getRolesToOnRolesCodemod() + + content := `--- +on: + issues: + types: [opened] +roles: + - admin + - maintainer + - write +--- + +# Test workflow` + + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + }, + "roles": []any{"admin", "maintainer", "write"}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "on:") + assert.Contains(t, result, "roles:") + assert.Contains(t, result, "- admin") + assert.Contains(t, result, "- maintainer") + assert.Contains(t, result, "- write") +} + +func TestRolesToOnRolesCodemod_AllValue(t *testing.T) { + codemod := getRolesToOnRolesCodemod() + + content := `--- +on: + issues: + types: [opened] +roles: all +--- + +# Test workflow` + + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + }, + "roles": "all", + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "roles: all") + // Ensure roles is nested under on: + lines := strings.Split(result, "\n") + foundOn := false + foundRoles := false + for _, line := range lines { + if line == "on:" { + foundOn = true + } + if foundOn && strings.Contains(line, "roles:") { + foundRoles = true + // Check that roles line has indentation + assert.Greater(t, len(line), len(strings.TrimSpace(line)), "roles should be indented under on:") + break + } + } + assert.True(t, foundRoles, "Should find roles nested under on:") +} + +func TestRolesToOnRolesCodemod_NoOnBlock(t *testing.T) { + codemod := getRolesToOnRolesCodemod() + + content := `--- +roles: [admin, write] +engine: copilot +--- + +# Test workflow` + + frontmatter := map[string]any{ + "roles": []any{"admin", "write"}, + "engine": "copilot", + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "on:") + assert.Contains(t, result, "roles: [admin, write]") + // Ensure on: block is created + lines := strings.Split(result, "\n") + foundOn := false + for _, line := range lines { + if line == "on:" { + foundOn = true + break + } + } + assert.True(t, foundOn, "Should create new on: block") +} + +func TestRolesToOnRolesCodemod_NoChange_NoRoles(t *testing.T) { + codemod := getRolesToOnRolesCodemod() + + content := `--- +on: + issues: + types: [opened] +engine: copilot +--- + +# Test workflow` + + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + }, + "engine": "copilot", + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestRolesToOnRolesCodemod_NoChange_OnRolesExists(t *testing.T) { + codemod := getRolesToOnRolesCodemod() + + content := `--- +on: + issues: + types: [opened] + roles: [admin, write] +roles: [admin, maintainer, write] +--- + +# Test workflow` + + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + "roles": []any{"admin", "write"}, + }, + "roles": []any{"admin", "maintainer", "write"}, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index 9f4d95d0455..0a0d8147f46 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -36,5 +36,6 @@ func GetAllCodemods() []Codemod { getInstallScriptURLCodemod(), getBashAnonymousRemovalCodemod(), // Replace bash: with bash: false getActivationOutputsCodemod(), // Transform needs.activation.outputs.* to steps.sanitized.outputs.* + getRolesToOnRolesCodemod(), // Move top-level roles to on.roles } } diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index a25f6199927..8cd68082e45 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -43,7 +43,7 @@ func TestGetAllCodemods_ReturnsAllCodemods(t *testing.T) { codemods := GetAllCodemods() // Verify we have the expected number of codemods - expectedCount := 18 + expectedCount := 19 assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount) // Verify all codemods have required fields @@ -122,6 +122,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) { "install-script-url-migration", "bash-anonymous-removal", "activation-outputs-to-sanitized-step", + "roles-to-on-roles", } require.Len(t, codemods, len(expectedOrder), "Should have expected number of codemods") diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 5d4b5020b6f..3513e09244d 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -1380,6 +1380,27 @@ ], "description": "Skip workflow execution for specific GitHub users. Useful for preventing workflows from running for specific accounts (e.g., bots, specific team members)." }, + "roles": { + "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (⚠️ security consideration).", + "oneOf": [ + { + "type": "string", + "enum": ["all"], + "description": "Allow any authenticated user to trigger the workflow (⚠️ disables permission checking entirely - use with caution)" + }, + { + "type": "array", + "description": "List of repository permission levels that can trigger the workflow. Permission checks are automatically applied to potentially unsafe triggers.", + "items": { + "type": "string", + "enum": ["admin", "maintainer", "maintain", "write", "triage"], + "description": "Repository permission level: 'admin' (full access), 'maintainer'/'maintain' (repository management), 'write' (push access), 'triage' (issue management)" + }, + "minItems": 1, + "maxItems": 50 + } + ] + }, "manual-approval": { "type": "string", "description": "Environment name that requires manual approval before the workflow can run. Must match a valid environment configured in the repository settings." @@ -6389,27 +6410,6 @@ }, "additionalProperties": false }, - "roles": { - "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (\u26a0\ufe0f security consideration).", - "oneOf": [ - { - "type": "string", - "enum": ["all"], - "description": "Allow any authenticated user to trigger the workflow (\u26a0\ufe0f disables permission checking entirely - use with caution)" - }, - { - "type": "array", - "description": "List of repository permission levels that can trigger the workflow. Permission checks are automatically applied to potentially unsafe triggers.", - "items": { - "type": "string", - "enum": ["admin", "maintainer", "maintain", "write", "triage"], - "description": "Repository permission level: 'admin' (full access), 'maintainer'/'maintain' (repository management), 'write' (push access), 'triage' (issue management)" - }, - "minItems": 1, - "maxItems": 50 - } - ] - }, "bots": { "type": "array", "description": "Allow list of bot identifiers that can trigger the workflow even if they don't meet the required role permissions. When the actor is in this list, the bot must be active (installed) on the repository to trigger the workflow.", diff --git a/pkg/workflow/bots_test.go b/pkg/workflow/bots_test.go index 2efe4d4eac2..acc6558e356 100644 --- a/pkg/workflow/bots_test.go +++ b/pkg/workflow/bots_test.go @@ -206,7 +206,7 @@ func TestBotsWithRolesAll(t *testing.T) { on: issues: types: [opened] -roles: all + roles: all bots: ["dependabot[bot]"] --- diff --git a/pkg/workflow/extract_roles_test.go b/pkg/workflow/extract_roles_test.go new file mode 100644 index 00000000000..2b1df5732a5 --- /dev/null +++ b/pkg/workflow/extract_roles_test.go @@ -0,0 +1,134 @@ +//go:build !integration + +package workflow + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestExtractRoles_OnRoles(t *testing.T) { + compiler := &Compiler{} + + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + "roles": []any{"admin", "write"}, + }, + } + + roles := compiler.extractRoles(frontmatter) + + assert.Equal(t, []string{"admin", "write"}, roles) +} + +func TestExtractRoles_OnRolesString(t *testing.T) { + compiler := &Compiler{} + + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + "roles": "all", + }, + } + + roles := compiler.extractRoles(frontmatter) + + assert.Equal(t, []string{"all"}, roles) +} + +func TestExtractRoles_TopLevelRoles_NotSupported(t *testing.T) { + compiler := &Compiler{} + + // Top-level roles is no longer supported - should return defaults + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + }, + "roles": []any{"admin", "maintainer", "write"}, + } + + roles := compiler.extractRoles(frontmatter) + + // Should return defaults since top-level roles is not recognized + assert.Equal(t, []string{"admin", "maintainer", "write"}, roles) +} + +func TestExtractRoles_TopLevelRoles_StringArray_NotSupported(t *testing.T) { + compiler := &Compiler{} + + // Top-level roles is no longer supported - should return defaults + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + }, + "roles": []string{"admin", "write"}, + } + + roles := compiler.extractRoles(frontmatter) + + // Should return defaults since top-level roles is not recognized + assert.Equal(t, []string{"admin", "maintainer", "write"}, roles) +} + +func TestExtractRoles_OnlyOnRolesSupported(t *testing.T) { + compiler := &Compiler{} + + // Only on.roles is supported now + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + "roles": []any{"admin"}, + }, + "roles": []any{"admin", "maintainer", "write"}, + } + + roles := compiler.extractRoles(frontmatter) + + // Should use on.roles, ignoring top-level + assert.Equal(t, []string{"admin"}, roles) +} + +func TestExtractRoles_Default(t *testing.T) { + compiler := &Compiler{} + + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + }, + } + + roles := compiler.extractRoles(frontmatter) + + assert.Equal(t, []string{"admin", "maintainer", "write"}, roles) +} + +func TestExtractRoles_AllValue(t *testing.T) { + compiler := &Compiler{} + + frontmatter := map[string]any{ + "on": map[string]any{ + "issues": map[string]any{ + "types": []any{"opened"}, + }, + "roles": "all", + }, + } + + roles := compiler.extractRoles(frontmatter) + + assert.Equal(t, []string{"all"}, roles) +} diff --git a/pkg/workflow/role_checks.go b/pkg/workflow/role_checks.go index 5ae43b5eab0..a326247072d 100644 --- a/pkg/workflow/role_checks.go +++ b/pkg/workflow/role_checks.go @@ -89,39 +89,54 @@ func (c *Compiler) generateRateLimitCheck(data *WorkflowData, steps []string) [] // extractRoles extracts the 'roles' field from frontmatter to determine permission requirements func (c *Compiler) extractRoles(frontmatter map[string]any) []string { - if rolesValue, exists := frontmatter["roles"]; exists { - switch v := rolesValue.(type) { - case string: - if v == "all" { - // Special case: "all" means no restrictions - roleLog.Print("Roles set to 'all' - no permission restrictions") - return []string{"all"} - } - // Single permission level as string - roleLog.Printf("Extracted single role: %s", v) - return []string{v} - case []any: - // Array of permission levels - var permissions []string - for _, item := range v { - if str, ok := item.(string); ok { - permissions = append(permissions, str) + // Check on.roles + if onValue, exists := frontmatter["on"]; exists { + if onMap, ok := onValue.(map[string]any); ok { + if rolesValue, hasRoles := onMap["roles"]; hasRoles { + roles := parseRolesValue(rolesValue, "on.roles") + if roles != nil { + return roles } } - roleLog.Printf("Extracted %d roles from array: %v", len(permissions), permissions) - return permissions - case []string: - // Already a string slice - roleLog.Printf("Extracted %d roles: %v", len(v), v) - return v } } + // Default: require admin, maintainer, or write permissions defaultRoles := []string{"admin", "maintainer", "write"} roleLog.Printf("No roles specified, using defaults: %v", defaultRoles) return defaultRoles } +// parseRolesValue parses a roles value from frontmatter (supports string, []any, []string) +func parseRolesValue(rolesValue any, fieldName string) []string { + switch v := rolesValue.(type) { + case string: + if v == "all" { + // Special case: "all" means no restrictions + roleLog.Printf("Roles in '%s' set to 'all' - no permission restrictions", fieldName) + return []string{"all"} + } + // Single permission level as string + roleLog.Printf("Extracted single role from '%s': %s", fieldName, v) + return []string{v} + case []any: + // Array of permission levels + var permissions []string + for _, item := range v { + if str, ok := item.(string); ok { + permissions = append(permissions, str) + } + } + roleLog.Printf("Extracted %d roles from '%s' array: %v", len(permissions), fieldName, permissions) + return permissions + case []string: + // Already a string slice + roleLog.Printf("Extracted %d roles from '%s': %v", len(v), fieldName, v) + return v + } + return nil +} + // extractBots extracts the 'bots' field from frontmatter to determine allowed bot identifiers func (c *Compiler) extractBots(frontmatter map[string]any) []string { if botsValue, exists := frontmatter["bots"]; exists { diff --git a/pkg/workflow/role_checks_test.go b/pkg/workflow/role_checks_test.go index 0d58b362dd5..b70dc6ecfbc 100644 --- a/pkg/workflow/role_checks_test.go +++ b/pkg/workflow/role_checks_test.go @@ -23,7 +23,7 @@ func TestRoleMembershipUsesGitHubToken(t *testing.T) { on: issues: types: [opened] -roles: [admin, maintainer] + roles: [admin, maintainer] --- # Test Workflow @@ -114,7 +114,7 @@ func TestRoleMembershipTokenWithBots(t *testing.T) { on: pull_request: types: [opened] -roles: [write] + roles: [write] bots: ["dependabot[bot]"] --- diff --git a/pkg/workflow/skip_roles_test.go b/pkg/workflow/skip_roles_test.go index 77327a15efa..da21460ab63 100644 --- a/pkg/workflow/skip_roles_test.go +++ b/pkg/workflow/skip_roles_test.go @@ -133,7 +133,7 @@ on: issues: types: [opened] skip-roles: [admin, write] -roles: [maintainer] + roles: [maintainer] engine: copilot ---