Skip to content

fix: unify on.permissions schema with shared github_actions_permissions ref #40064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Jun 18, 2026
+76 −65
Merged

fix: unify on.permissions schema with shared github_actions_permissions ref #40064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64f52be
Initial plan
Copilot Jun 18, 2026
3ef3ec5
fix: unify on.permissions schema with shared github_actions_permissio…
Copilot Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/importinpututil/spec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func TestSpec_PublicAPI_FormatResolvedValue_MarshalFailure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
s, ok := importinpututil.FormatResolvedValue(tt.value)
assert.False(t, ok, "FormatResolvedValue should return ok=false when JSON marshalling fails: %s", tt.name)
assert.Equal(t, "", s, "FormatResolvedValue should return empty string when JSON marshalling fails: %s", tt.name)
assert.Empty(t, s, "FormatResolvedValue should return empty string when JSON marshalling fails: %s", tt.name)
})
}
}
Expand Down
22 changes: 15 additions & 7 deletions pkg/parser/schema_errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,31 +306,39 @@ func findFrontmatterBounds(lines []string) (startIdx int, endIdx int, frontmatte
// of the valid values / children for that field. Used to append helpful hints when an
// additionalProperties error occurs on these fields so users quickly know what is allowed.
//
// The permissions scope list mirrors the properties defined in main_workflow_schema.json
// under permissions.oneOf[1].properties. Update this list when the schema changes.
// Both /permissions and /on/permissions mirror #/$defs/github_actions_permissions in
// main_workflow_schema.json. Update this list when the schema changes.
var knownFieldValidValues = map[string]string{
// This list mirrors permissions.oneOf[1].properties in main_workflow_schema.json.
// Both entries mirror $defs/github_actions_permissions in main_workflow_schema.json.
// Update both when the schema changes.
"/permissions": "Valid permission scopes: actions, all, attestations, checks, copilot-requests, contents, deployments, discussions, id-token, issues, metadata, models, organization-projects, packages, pages, pull-requests, repository-projects, security-events, statuses, vulnerability-alerts",
"/permissions": "Valid permission scopes: actions, all, attestations, checks, copilot-requests, contents, deployments, discussions, id-token, issues, metadata, models, organization-projects, packages, pages, pull-requests, repository-projects, security-events, statuses, vulnerability-alerts",
"/on/permissions": "Valid permission scopes: actions, all, attestations, checks, copilot-requests, contents, deployments, discussions, id-token, issues, metadata, models, organization-projects, packages, pages, pull-requests, repository-projects, security-events, statuses, vulnerability-alerts",
}
Comment on lines +309 to 316

// knownFieldScopes maps well-known JSON schema paths to a slice of valid scope names.
// This enables spell-check ("Did you mean?") suggestions for unknown-property errors.
//
// The permissions scope list mirrors permissions.oneOf[1].properties in main_workflow_schema.json.
// Update both when the schema changes.
// Both /permissions and /on/permissions mirror #/$defs/github_actions_permissions in
// main_workflow_schema.json. Update this list when the schema changes.
var knownFieldScopes = map[string][]string{
"/permissions": {
"actions", "all", "attestations", "checks", "copilot-requests", "contents", "deployments",
"discussions", "id-token", "issues", "metadata", "models",
"organization-projects", "packages", "pages", "pull-requests",
"repository-projects", "security-events", "statuses", "vulnerability-alerts",
},
"/on/permissions": {

@github-actions github-actions Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/improve-codebase-architecture] The scope slice for /on/permissions is byte-for-byte identical to the /permissions slice, and both comments now say they mirror the same $ref. When a new scope is added to github_actions_permissions, there are four co-ordinated edits needed: two entries in knownFieldValidValues and two in knownFieldScopes.

Since the schema consolidation is the whole point of this PR, this is a good moment to consolidate the Go side too.

💡 Suggested refactor 
// githubActionsPermissionScopes is the canonical scope list mirroring
// $defs/github_actions_permissions in main_workflow_schema.json.
// Update here when the schema changes.
var githubActionsPermissionScopes = []string{
    "actions", "all", "attestations", "checks", "copilot-requests", "contents", "deployments",
    "discussions", "id-token", "issues", "metadata", "models",
    "organization-projects", "packages", "pages", "pull-requests",
    "repository-projects", "security-events", "statuses", "vulnerability-alerts",
}

var knownFieldScopes = map[string][]string{
    "/permissions":    githubActionsPermissionScopes,
    "/on/permissions": githubActionsPermissionScopes,
}

Similarly, the long string in knownFieldValidValues could be built from this slice with strings.Join.

"actions", "all", "attestations", "checks", "copilot-requests", "contents", "deployments",
"discussions", "id-token", "issues", "metadata", "models",
"organization-projects", "packages", "pages", "pull-requests",
"repository-projects", "security-events", "statuses", "vulnerability-alerts",
},
}

// knownFieldDocs maps well-known JSON schema paths to documentation URLs.
var knownFieldDocs = map[string]string{
"/permissions": "https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/controlling-permissions-for-github_token",
"/permissions": "https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/controlling-permissions-for-github_token",
"/on/permissions": "https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/controlling-permissions-for-github_token",
}

// unknownPropertyPattern extracts the property name(s) from a rewritten "Unknown property(ies):" message.
Expand Down
43 changes: 43 additions & 0 deletions pkg/parser/schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1975,3 +1975,46 @@ func TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_AwfApiProxyTargets
}
})
}

// TestValidateMainWorkflowFrontmatter_OnPermissionsVulnerabilityAlerts validates that
// vulnerability-alerts is accepted as a scope in on.permissions (regression for #40063).
func TestValidateMainWorkflowFrontmatter_OnPermissionsVulnerabilityAlerts(t *testing.T) {
frontmatter := map[string]any{
"on": map[string]any{
"schedule": []any{map[string]any{"cron": "0 0 * * *"}},
"workflow_dispatch": nil,
"permissions": map[string]any{
"vulnerability-alerts": "read",

@github-actions github-actions Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The acceptance test pins vulnerability-alerts (the scope from the bug report), which is the right regression anchor. However, attestations, copilot-requests, id-token, metadata, models, and organization-projects were also blocked by the old inline schema and are now valid through the shared $ref — none of them have acceptance coverage.

Adding one or two to the acceptance test (e.g., attestations: write) would turn this into a broader schema-drift guard rather than a single-scope pin.

💡 Suggested addition 
// covers a second scope that was previously blocked by the inline schema
"attestations": "write",

Or use a short table-driven loop over []string{"vulnerability-alerts", "attestations", "copilot-requests"} inside the test to keep it compact.

},
"steps": []any{
map[string]any{
"id": "check",
"run": "echo checking",
},
},
},
"engine": "copilot",
}
err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter, "/tmp/gh-aw/on-permissions-vulnerability-alerts-test.md")
if err != nil {
t.Errorf("vulnerability-alerts: read should be accepted in on.permissions, got error: %v", err)
}
}

// TestValidateMainWorkflowFrontmatter_OnPermissionsUnknownScopeRejected validates that
// unknown scopes in on.permissions are still rejected.
func TestValidateMainWorkflowFrontmatter_OnPermissionsUnknownScopeRejected(t *testing.T) {
frontmatter := map[string]any{
"on": map[string]any{
"workflow_dispatch": nil,
"permissions": map[string]any{
"unknown-scope": "read",
},
},
"engine": "copilot",
}
err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter, "/tmp/gh-aw/on-permissions-unknown-scope-test.md")
if err == nil {

@github-actions github-actions Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The rejection test confirms that err != nil, but does not verify that the error message actually contains the "Did you mean?" hints or docs URL added in schema_errors.go for /on/permissions.

If path detection is broken — e.g., the JSON schema path surfaced for an on.permissions violation is not exactly /on/permissions — the new knownFieldScopes and knownFieldDocs entries would be dead code, yet this test would still pass.

💡 Suggested assertion

Consider asserting the error message text, for example:

if err == nil {
    t.Error("unknown scope in on.permissions should be rejected")
}
if !strings.Contains(err.Error(), "vulnerability-alerts") {
    t.Errorf("expected \"Did you mean?\" hint with valid scopes in error, got: %v", err)
}

Or at minimum use assert.ErrorContains(t, err, "Valid permission scopes") to confirm the hint path fires.

t.Error("unknown scope in on.permissions should be rejected by schema validation")
}
}
61 changes: 4 additions & 57 deletions pkg/parser/schemas/main_workflow_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2130,70 +2130,17 @@
},
"permissions": {
"description": "Additional permissions for the pre-activation job. Use to declare extra scopes required by on.steps (e.g., issues: read for GitHub API calls in steps).",
"oneOf": [
{
"type": "object",
"description": "Map of permission scope to level",
"properties": {
"actions": {
"type": "string",
"enum": ["read", "write", "none"]
},
"checks": {
"type": "string",
"enum": ["read", "write", "none"]
},
"contents": {
"type": "string",
"enum": ["read", "write", "none"]
},
"deployments": {
"type": "string",
"enum": ["read", "write", "none"]
},
"discussions": {
"type": "string",
"enum": ["read", "write", "none"]
},
"issues": {
"type": "string",
"enum": ["read", "write", "none"]
},
"packages": {
"type": "string",
"enum": ["read", "write", "none"]
},
"pages": {
"type": "string",
"enum": ["read", "write", "none"]
},
"pull-requests": {
"type": "string",
"enum": ["read", "write", "none"]
},
"repository-projects": {
"type": "string",
"enum": ["read", "write", "none"]
},
"security-events": {
"type": "string",
"enum": ["read", "write", "none"]
},
"statuses": {
"type": "string",
"enum": ["read", "write", "none"]
}
},
"additionalProperties": false
}
],
"$ref": "#/$defs/github_actions_permissions",
"examples": [
Comment on lines 2131 to 2134
{
"issues": "read"
},
{
"issues": "read",
"pull-requests": "read"
},
{
"vulnerability-alerts": "read"
}
]
},
Expand Down
13 changes: 13 additions & 0 deletions pkg/workflow/on_steps_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,19 @@ func TestExtractOnPermissions(t *testing.T) {
expectNil: false,
expectScopes: map[string]string{"issues": "read", "pull-requests": "read"},
},
{
name: "on_permissions_vulnerability_alerts",
frontmatter: map[string]any{
"on": map[string]any{
"workflow_dispatch": nil,
"permissions": map[string]any{
"vulnerability-alerts": "read",
},
},
},
expectNil: false,
expectScopes: map[string]string{"vulnerability-alerts": "read"},
},
}

for _, tt := range tests {
Expand Down
Loading