[iOS] Grant statuses:write so Danger can post required PR commit statuses#4079
[iOS] Grant statuses:write so Danger can post required PR commit statuses#4079sfdctaka wants to merge 1 commit into
Conversation
…R commit statuses
The static-analysis and test-orchestrator jobs run Danger to post the
danger/StaticAnalysis and danger/TestOrchestrator commit statuses that the
dev branch protection requires. Their job-level permission blocks granted
contents:read and pull-requests:write but not statuses:write, so Danger's
status calls were rejected ("Danger does not have write access to the PR
to set a PR status."). The required statuses never appeared, leaving every
PR perpetually blocked from merge even when all checks completed
successfully.
|
||||||||||||||
|
||||||||||||||||||||||||
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #4079 +/- ##
==========================================
- Coverage 70.83% 68.35% -2.49%
==========================================
Files 246 246
Lines 21495 21495
==========================================
- Hits 15227 14692 -535
- Misses 6268 6803 +535
🚀 New features to boost your workflow:
|
|
||||||||||||||||
|
What changed for it to stop working? |
In #4047 (2026-05-29, "fix security issues") explicit per-job permissions: blocks were added to harden the pull_request_target workflow: The intent was correct, but in GitHub Actions any job-level permissions: block replaces the default GITHUB_TOKEN scopes rather than adding to them — anything not listed is dropped. The default token had implicit statuses: write; the new block doesn't, so Danger silently lost the ability to post commit statuses. Its log line is: ▎ Danger does not have write access to the PR to set a PR status. The two required contexts (danger/StaticAnalysis, danger/TestOrchestrator) have not been posted on PR head commits since that merge. We didn't notice because dev branch protection has enforce_admins: false, so admin merges have been bypassing the missing required statuses — masking the regression for ~3 weeks. Non-admin PRs This PR adds statuses: write back to the two job permission blocks. Two-line change, no other behavior affected. |
bbirman
left a comment
bbirman
left a comment
There was a problem hiding this comment.
lgtm but I'd want @brandonpage's approval too
|
@brandonpage Can you check this PR please? |
3 participants
Summary
statuses: writeto thestatic-analysisandtest-orchestratorjob-level permission blocks in.github/workflows/pr.yaml.danger/StaticAnalysisanddanger/TestOrchestratorcommit statuses required bydev's branch protection. Withoutstatuses: write, Danger's status calls are rejected (Danger does not have write access to the PR to set a PR status.), so the required statuses never appear and PRs cannot merge even when every check completes successfully.Note (the irony)
This very PR will hit the same blocker until it merges. The required
danger/StaticAnalysisanddanger/TestOrchestratorcommit statuses will not appear on this PR's head commit, because the workflow run evaluating it is still running with the old (insufficient) permissions. The merge button will stay disabled even after approval. Someone with admin rights will likely need to bypass branch protection once to land this. After that, every subsequent PR run picks up the new permission and unblocks automatically.Test plan
danger/StaticAnalysisanddanger/TestOrchestratorcommit statuses on its head commit.