-
Notifications
You must be signed in to change notification settings - Fork 368
Add issues: read to update-project GitHub App token permissions
#27837
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -161,3 +161,44 @@ Test workflow with discussions permission. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, stepsStr, "permission-contents: read", "GitHub App token should include contents read permission") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, stepsStr, "permission-issues: write", "GitHub App token should include issues write permission (create-discussion falls back to issue)") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TestSafeOutputsAppTokenUpdateProjectIssuesReadPermission tests that issues read permission | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // is included in the GitHub App token minting step when update-project is configured. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestSafeOutputsAppTokenUpdateProjectIssuesReadPermission(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| compiler := NewCompiler(WithVersion("1.0.0")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| markdown := `--- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| on: issues | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| safe-outputs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| update-project: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project: "https://github.com/orgs/my-org/projects/1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| github-app: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| app-id: ${{ vars.APP_ID }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private-key: ${{ secrets.APP_PRIVATE_KEY }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Test Workflow | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Test workflow with update-project permissions. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tmpDir := t.TempDir() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| testFile := filepath.Join(tmpDir, "test.md") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := os.WriteFile(testFile, []byte(markdown), 0644) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err, "Failed to write test file") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workflowData, err := compiler.ParseWorkflowFile(testFile) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err, "Failed to parse markdown content") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NotNil(t, workflowData.SafeOutputs.UpdateProjects, "UpdateProjects should not be nil") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| job, _, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, "main", testFile) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err, "Failed to build safe_outputs job") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NotNil(t, job, "Job should not be nil") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| stepsStr := strings.Join(job.Steps, "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, stepsStr, "permission-organization-projects: write", "GitHub App token should include organization projects write permission") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, stepsStr, "permission-issues: read", "GitHub App token should include issues read permission for issue-backed project items") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, stepsStr, "permission-contents: read", "GitHub App token should include contents read permission") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | |
| } | |
| // TestSafeOutputsAppConfigurationCreateProjectWithItemURL tests that create-project with item_url | |
| // requests issue read permission for GitHub App tokens. | |
| func TestSafeOutputsAppConfigurationCreateProjectWithItemURL(t *testing.T) { | |
| compiler := NewCompiler(WithVersion("1.0.0")) | |
| markdown := `--- | |
| on: issues | |
| safe-outputs: | |
| create-project: | |
| project: "https://github.com/orgs/my-org/projects/1" | |
| item_url: "https://github.com/my-org/my-repo/issues/123" | |
| github-app: | |
| app-id: ${{ vars.APP_ID }} | |
| private-key: ${{ secrets.APP_PRIVATE_KEY }} | |
| --- | |
| # Test Workflow | |
| Test workflow with create-project item_url permissions. | |
| ` | |
| tmpDir := t.TempDir() | |
| testFile := filepath.Join(tmpDir, "test.md") | |
| err := os.WriteFile(testFile, []byte(markdown), 0644) | |
| require.NoError(t, err, "Failed to write test file") | |
| workflowData, err := compiler.ParseWorkflowFile(testFile) | |
| require.NoError(t, err, "Failed to parse markdown content") | |
| require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") | |
| require.NotNil(t, workflowData.SafeOutputs.CreateProjects, "CreateProjects should not be nil") | |
| job, _, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, "main", testFile) | |
| require.NoError(t, err, "Failed to build safe_outputs job") | |
| require.NotNil(t, job, "Job should not be nil") | |
| stepsStr := strings.Join(job.Steps, "") | |
| assert.Contains(t, stepsStr, "permission-organization-projects: write", "GitHub App token should include organization projects write permission") | |
| assert.Contains(t, stepsStr, "permission-issues: read", "GitHub App token should include issues read permission when create-project resolves an issue from item_url") | |
| assert.Contains(t, stepsStr, "permission-contents: read", "GitHub App token should include contents read permission") | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -206,6 +206,7 @@ func ComputePermissionsForSafeOutputs(safeOutputs *SafeOutputsConfig) *Permissio | |
| if safeOutputs.UpdateProjects != nil && !isHandlerStaged(safeOutputs.Staged, safeOutputs.UpdateProjects.Staged) { | ||
| safeOutputsPermissionsLog.Print("Adding permissions for update-project") | ||
| permissions.Merge(NewPermissionsContentsReadProjectsWrite()) | ||
| permissions.Set(PermissionIssues, PermissionRead) | ||
| } | ||
|
Comment on lines
206
to
210
|
||
| if safeOutputs.CreateProjectStatusUpdates != nil && !isHandlerStaged(safeOutputs.Staged, safeOutputs.CreateProjectStatusUpdates.Staged) { | ||
| safeOutputsPermissionsLog.Print("Adding permissions for create-project-status-update") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new note documents
update-projectneedingissues: readfor issue-backed item resolution when usinggithub-app, butcreate-projectcan also resolve an issue whenitem_urlis set (it queriesrepository.issueto get the node id). To avoid users hitting the same permission-scoping problem on project creation, consider extending this documentation to mention thecreate-project+item_urlcase (or more generally that project item resolution requiresissues: read).