Conversation
…issions Allow users to specify org-level permissions (e.g., members: read) under tools.github.github-app.permissions. These are merged into the minted GitHub App installation access token with nested permissions winning over job-level permissions for overlapping scopes. Fixes: Can not fetch members from org when using an app token Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8134923b-eeed-4351-bd5e-531c7dc8f3ce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
tools.github.github-app token for org-level API access
| "description": "Optional extra permissions to merge into the minted token. These override the job-level permissions for the token (nested wins). Use this to add org-level permissions (e.g., members: read) that are not valid GitHub Actions scopes but are supported by GitHub Apps.", | ||
| "additionalProperties": { | ||
| "type": "string", | ||
| "enum": ["read", "write", "none"] |
| "permissions": { | ||
| "type": "object", | ||
| "description": "Optional extra permissions to merge into the minted token. These override the job-level permissions for the token (nested wins). Use this to add org-level permissions (e.g., members: read) that are not valid GitHub Actions scopes but are supported by GitHub Apps.", | ||
| "additionalProperties": { |
There was a problem hiding this comment.
@copilot reuse list of permissions already present in the JSON format
There was a problem hiding this comment.
Fixed in a879f39 — replaced additionalProperties with an explicit properties map enumerating all 30 GitHub App-only permission scopes (matching the pattern already used in the main permissions object). additionalProperties: false prevents unknown scopes.
… read/none only - Enumerate all GitHub App-only scopes explicitly instead of using additionalProperties - Only allow 'read' and 'none' values (not 'write') - Update override test to use a valid GitHub App-only scope (vulnerability-alerts)" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3e32b3bc-7c8b-4f55-a88f-3d940a2ae8e6 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot refactor permissions type in JSON schema and reuse where appropriate |
- Add $defs.github_actions_permissions (the full GitHub Actions permissions object) - Add $defs.github_app_permissions (the GitHub App-only permissions object) - Replace inline definitions with $ref references at both usage points Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8f5e79a1-f47d-4d4a-bfd6-0e5f8edd76cc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in a6d3df9. Both permissions map definitions are now extracted into named
Both usage points now use Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| }, | ||
| "codespaces": { | ||
| "type": "string", | ||
| "enum": ["read", "none"], |
There was a problem hiding this comment.
@copilot add the "write" entry but have a compiler checks that refuses it as an informative error.
There was a problem hiding this comment.
Done in 3335321. "write" is now present in the enum for all scopes in $defs.github_app_permissions (so editors offer it as a completion and schema validation passes), and a new compiler check validateGitHubMCPAppPermissionsNoWrite rejects it with an informative error listing the offending scopes. Example error:
"write" is not allowed in tools.github.github-app.permissions.
GitHub App-only scopes in this section must be declared as "read" or "none".
Write operations must be performed via safe-outputs, not through declared permissions.
The following scopes were declared with "write" access:
- members
Change the permission level to "read" for read-only access, or remove the entry.
… compile time - Add "write" to all enum values in $defs.github_app_permissions so editors can offer it as a completion and users get a clear compiler error instead of a silent schema validation pass-through - Add validateGitHubMCPAppPermissionsNoWrite() to github_app_permissions_validation.go with an informative error listing the offending scopes - Wire the new validator into compiler.go alongside the existing GitHub App permission checks - Add TestGitHubMCPAppTokenExtraPermissionsWriteRejected to cover the error path Agent-Logs-Url: https://github.com/github/gh-aw/sessions/17aa2920-f546-4f22-b7aa-992c63901959 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Enables tools.github.github-app to request additional GitHub App-only permission scopes (not representable in GitHub Actions permissions:) so org-level API calls can succeed when using the minted GitHub App token.
Changes:
- Added
permissionsmap support to GitHub App config parsing and schema validation. - Merged
tools.github.github-app.permissionsinto the permissions passed toactions/create-github-app-token, with warnings for unknown scopes. - Added compiler validation + tests to reject
"write"values intools.github.github-app.permissions.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_outputs_app_config.go | Extends GitHubAppConfig + parsing to include an optional permissions map. |
| pkg/workflow/compiler_github_mcp_steps.go | Merges extra github-app.permissions into the permissions forwarded to the GitHub MCP App token minting step. |
| pkg/workflow/github_app_permissions_validation.go | Adds compiler validation to reject "write" in tools.github.github-app.permissions. |
| pkg/workflow/compiler.go | Wires the new validation into the compiler validation pipeline. |
| pkg/workflow/github_mcp_app_token_test.go | Adds tests for merging extra permissions, overrides, and write rejection. |
| pkg/parser/schemas/main_workflow_schema.json | Adds permissions to the github_app schema and introduces $defs for GitHub Actions vs GitHub App permission maps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| appConfig.Permissions[key] = valStr | ||
| } | ||
| } |
There was a problem hiding this comment.
parseAppConfig silently drops non-string entries under github-app.permissions (and also ignores a non-object permissions value) without any warning or error. This can lead to confusing behavior where a user configures permissions but they are simply not applied. Consider logging a warning (including the key/type) or returning a validation error when permissions is present but not a map[string]string.
| appConfig.Permissions[key] = valStr | |
| } | |
| } | |
| appConfig.Permissions[key] = valStr | |
| } else { | |
| safeOutputsAppLog.Printf("Ignoring github-app.permissions[%q]: expected string value, got %T", key, val) | |
| } | |
| } | |
| } else { | |
| safeOutputsAppLog.Printf("Ignoring github-app.permissions: expected map[string]string-compatible object, got %T", perms) |
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg)) | ||
| continue | ||
| } | ||
| permissions.Set(scope, PermissionLevel(val)) |
There was a problem hiding this comment.
Extra permissions from github-app.permissions are applied with PermissionLevel(val) without validating that val is an allowed level (expected "read" or "none"; "write" is rejected elsewhere). If schema validation is bypassed (e.g. non-workflow files or imported JSON), this can generate invalid permission-* inputs and fail at runtime. Consider validating/normalizing the level here (trim/lowercase) and failing compilation for unknown values.
| permissions.Set(scope, PermissionLevel(val)) | |
| level := strings.ToLower(strings.TrimSpace(val)) | |
| if level != "read" && level != "none" { | |
| msg := fmt.Sprintf("Unknown permission level %q for scope %q in tools.github.github-app.permissions. Valid levels are: read, none.", val, key) | |
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg)) | |
| continue | |
| } | |
| permissions.Set(scope, PermissionLevel(level)) |
| var writeScopes []string | ||
| for scope, level := range app.Permissions { | ||
| if level == string(PermissionWrite) { | ||
| writeScopes = append(writeScopes, scope) | ||
| } | ||
| } | ||
| if len(writeScopes) == 0 { | ||
| return nil | ||
| } | ||
| sort.Strings(writeScopes) | ||
|
|
||
| var lines []string | ||
| lines = append(lines, `"write" is not allowed in tools.github.github-app.permissions.`) | ||
| lines = append(lines, "GitHub App-only scopes in this section must be declared as \"read\" or \"none\".") | ||
| lines = append(lines, "Write operations must be performed via safe-outputs, not through declared permissions.") | ||
| lines = append(lines, "") | ||
| lines = append(lines, "The following scopes were declared with \"write\" access:") | ||
| lines = append(lines, "") | ||
| for _, s := range writeScopes { | ||
| lines = append(lines, " - "+s) | ||
| } | ||
| lines = append(lines, "") | ||
| lines = append(lines, "Change the permission level to \"read\" for read-only access, or remove the entry.") |
There was a problem hiding this comment.
validateGitHubMCPAppPermissionsNoWrite only checks for the literal string "write". If the permissions map contains other invalid values (e.g. "Read", "none ", or any unexpected string) they will pass validation and later be emitted into permission-* inputs. Consider normalizing values (trim/lowercase) and validating that each level is exactly "read" or "none" (with a dedicated error message) rather than only rejecting "write".
| var writeScopes []string | |
| for scope, level := range app.Permissions { | |
| if level == string(PermissionWrite) { | |
| writeScopes = append(writeScopes, scope) | |
| } | |
| } | |
| if len(writeScopes) == 0 { | |
| return nil | |
| } | |
| sort.Strings(writeScopes) | |
| var lines []string | |
| lines = append(lines, `"write" is not allowed in tools.github.github-app.permissions.`) | |
| lines = append(lines, "GitHub App-only scopes in this section must be declared as \"read\" or \"none\".") | |
| lines = append(lines, "Write operations must be performed via safe-outputs, not through declared permissions.") | |
| lines = append(lines, "") | |
| lines = append(lines, "The following scopes were declared with \"write\" access:") | |
| lines = append(lines, "") | |
| for _, s := range writeScopes { | |
| lines = append(lines, " - "+s) | |
| } | |
| lines = append(lines, "") | |
| lines = append(lines, "Change the permission level to \"read\" for read-only access, or remove the entry.") | |
| var ( | |
| invalidScopes []string | |
| writeScopes []string | |
| ) | |
| for scope, level := range app.Permissions { | |
| normalized := strings.ToLower(strings.TrimSpace(level)) | |
| switch normalized { | |
| case "read", "none": | |
| // valid levels | |
| continue | |
| default: | |
| // Track all invalid scopes | |
| invalidScopes = append(invalidScopes, scope+" (level: "+level+")") | |
| // Track scopes that explicitly requested write access (in any casing/whitespace) | |
| if normalized == strings.ToLower(string(PermissionWrite)) { | |
| writeScopes = append(writeScopes, scope) | |
| } | |
| } | |
| } | |
| if len(invalidScopes) == 0 { | |
| return nil | |
| } | |
| sort.Strings(invalidScopes) | |
| sort.Strings(writeScopes) | |
| var lines []string | |
| lines = append(lines, "Invalid permission levels in tools.github.github-app.permissions.") | |
| lines = append(lines, "Each GitHub App permission level must be exactly \"read\" or \"none\" (case-sensitive, no extra whitespace).") | |
| if len(writeScopes) > 0 { | |
| lines = append(lines, "") | |
| lines = append(lines, `"write" is not allowed in tools.github.github-app.permissions.`) | |
| lines = append(lines, "Write operations must be performed via safe-outputs, not through declared permissions.") | |
| lines = append(lines, "") | |
| lines = append(lines, "The following scopes were declared with \"write\" access:") | |
| for _, s := range writeScopes { | |
| lines = append(lines, " - "+s) | |
| } | |
| } | |
| lines = append(lines, "") | |
| lines = append(lines, "The following scopes were declared with invalid permission levels:") | |
| for _, s := range invalidScopes { | |
| lines = append(lines, " - "+s) | |
| } | |
| lines = append(lines, "") | |
| lines = append(lines, "Change the permission level to \"read\" for read-only access, or set it to \"none\" / remove the entry.") |
| }, | ||
| "permissions": { | ||
| "$ref": "#/$defs/github_app_permissions" |
There was a problem hiding this comment.
The schema adds a permissions property to the shared $defs.github_app object, which is referenced by multiple features (e.g. safe-outputs.github-app, checkout.github-app, qmd search github-app). The implementation in this PR only merges github-app.permissions into the GitHub MCP token minting step, so users may think permissions applies in those other contexts when it does not. Consider scoping this schema change to tools.github.github-app only, or emitting a compile-time warning/error when permissions is provided in unsupported github-app contexts.
| }, | |
| "permissions": { | |
| "$ref": "#/$defs/github_app_permissions" |
|
@copilot review comments |
- parseAppConfig: log warnings for non-string permission values and non-map permissions entries instead of silently dropping them - compiler_github_mcp_steps: normalize permission level (trim+lowercase) and emit a warning for any level that isn't "read" or "none" before skipping it - validateGitHubMCPAppPermissionsNoWrite: broaden validation to reject all invalid levels (not just "write") after normalization; list both write-specific and generic invalid scopes with separate explanations in the error message - warnGitHubAppPermissionsUnsupportedContexts: new function that emits a compile-time warning when permissions is set in safe-outputs.github-app, on.github-app, or the top-level github-app fallback (where it has no effect); called from compiler.go after the no-write check - schema: keep permissions in $defs.github_app (allOf+additionalProperties:false conflict prevents scoping via allOf) but update its description to clarify it only takes effect for tools.github.github-app; scoping enforced at runtime via the new warning function - test: update write-rejection assertion strings to match the new error format Agent-Logs-Url: https://github.com/github/gh-aw/sessions/293c96f3-a7d6-461b-88c3-8ded806c935c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed all four review comments in 77fbde1:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The compiler only forwarded job-level
permissions:to the minted GitHub App token, with no mechanism to add org-level scopes (e.g.members: read) that are invalid as GitHub Actions permissions but valid for GitHub App tokens. Org-scoped API calls likeGET /orgs/{org}/teams/{slug}/membersreturned null silently even with a correctly-configured app.Changes
GitHubAppConfig: AddedPermissions map[string]stringfieldparseAppConfig: Parsespermissionsmap from frontmatter; logs debug warnings for non-string values and non-mappermissionsentries instead of silently dropping themgenerateGitHubMCPAppTokenMintingStep: Appliesapp.Permissionson top of job-level permissions after conversion — nested entries win for overlapping scopes; levels are normalized (trimmed + lowercased); emits user-visible warnings for unrecognized scope names or invalid levelsmain_workflow_schema.json: Addedpermissionsproperty togithub_appdefinition with all 30 GitHub App-only permission scopes enumerated explicitly (additionalProperties: false). Both permission map definitions are extracted into named$defs(github_actions_permissions,github_app_permissions) and referenced via$ref."write"is included in the enum so editors offer it as a completion, but is rejected at compile time with an informative error. The description clearly states thatpermissionsonly takes effect fortools.github.github-app.validateGitHubMCPAppPermissionsNoWrite: Compiler validation that rejects all invalid permission levels (not just the literal"write") after normalization, with a structured error listing write-specific scopes and other invalid scopes separately.warnGitHubAppPermissionsUnsupportedContexts: Emits a compile-time warning whenpermissionsis set onsafe-outputs.github-app,on.github-app, or the top-levelgithub-appfallback, where it has no effect."none"overrides a job-level GitHub App-only scope, and any invalid level (including"write") intools.github.github-app.permissionsis rejected with an informative error.Usage
The compiled token minting step will now include
permission-members: readalongside the standard job-levelpermission-*fields.Specifying an invalid level (e.g.
"write") for any scope undertools.github.github-app.permissionsproduces a compiler error: