Repository navigation
Restrict workflow pushes by bot identity, not by our App's name - #118
Merged
Merged
Conversation
App names are globally unique, so every adopter's App resolves to a different bot login and the hardcoded comparison was false for all of them. The preflight only ran for the one installation that needed it least, and everyone else discovered the refusal at push time, after a full loop and its model spend. Any *[bot] login is restricted: the Actions runtime always authenticates as an App and that App deliberately holds no workflows permission. An empty login stays unrestricted, because that is a local run under a human's own credential and a human can push workflow files. The escalation message named our bot in prose too, so an adopter's correct escalation pointed at an account they never installed. It now names the authenticated identity, or says 'a GitHub App' when there isn't one. Closes #111
8 tasks
prepare() requires the Azure variables, and the two new subtests relied on them being present in the shell. They passed locally and failed on a clean runner, which is the failure mode the tests exist to prevent. Matches the pattern the surrounding prepare tests already use.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Codecov flagged internal/loop at 70% patch coverage. The uncovered branch was the new one: naming the authenticated identity. The existing tests only ever ran with an empty login, so they exercised the generic wording and never the adopter case. Adds coverage for both escalation paths naming a real bot, the generic form when no login is known, and the two push-refusal branches that must stay silent. Also covers the preflight deciding NOT to block. That path had no test at all, and it is the exact behaviour that misfired on the run for this issue (#119): a change touching no workflow file must reach a pull request, and a failure to decide must not be reported as a permission problem. internal/loop goes from 86.0% to 89.3%.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #111.
The fix
simplycubedAppLoginis gone.WorkflowRestrictedPushis nowisBotLogin(forge.Self)— any*[bot]login, since the Actions runtime always authenticates as an App and that App holds noworkflowspermission by design.An empty login stays unrestricted. That is a local run under a human's own credential, and a human can push workflow files.
main.goalready documented empty-means-human; nothing pinned it until now.workflowPermissionReasontook our bot's name in prose, so an adopter's correct escalation pointed at an account they had never installed. It now takes the authenticated login, falling back to "a GitHub App" when there is none. That meant threadingSelfLoginthroughapp.Depsandloop.Config, and givingworkflowPushReasona receiver.Tests
Two cases, neither previously covered:
acme-code[bot]) is restricted — the case that was broken, and the one that cannot reproduce in this repositoryInjected with
SIMPLYCUBED_SELF_LOGINthroughprepare(), so it is hermetic and needs no network.Why a human opened this
The agent was given
sc:goon #111 and produced essentially this change, then blocked with "the change touches.github/workflows/". It does not. The identical change here modifies four Go files,git status --porcelain -- .github/workflowsreturns empty, andmake checkpasses.That escalation was a false positive inside the agent's worktree, and the same run reported
go buildfailing there witherror obtaining VCS status: exit status 128. Filed separately — it is an environment bug, not this one.