Refresh the Nix vendorHash on Dependabot Go bumps - #19
Merged
Conversation
A Dependabot Go bump changes go.sum and leaves the flake's vendorHash in nix/package.nix stale, so the caller's "Nix flake builds" check fails with a fixed-output hash mismatch: the PR wedges where the check is required (hey-cli #427) and merges red where it is not, breaking main's next flake build (basecamp-cli #697, repaired by #701). This reusable workflow refreshes the hash on the Dependabot PR itself. A compute job with no secrets builds the flake at the PR's merge commit and takes the hash only from a go-modules fixed-output mismatch; a separate push job mints a one-hour App token scoped to the calling repository, rewrites exactly the vendorHash line, proves the diff is that one line, and commits through createCommitOnBranch with the captured head as expectedHeadOid. The App push re-triggers the PR's pull_request CI, whose Nix check verifies the new hash; the re-triggered run is actored by the App bot, so the Dependabot actor guard ends the loop. Every other path fails closed with nothing pushed. The header records why pushing into Dependabot PRs is sound for Go bumps even though it was abandoned for actions bumps (#11): the workflow code that re-runs is the default branch's, and the callers' pull_request jobs reference no secret a Go bump can reach.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Changes recommended
Auto-merge can precede refresh, and the head-only lease does not protect against merge-base changes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a reusable workflow to refresh stale Nix vendorHash values on Dependabot Go PRs.
Changes:
- Computes hashes from Nix merge builds.
- Commits guarded one-line updates using a GitHub App token.
- Verifies PR CI was retriggered.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
.github/workflows/dependabot-sync-nix-vendor-hash.yml |
Implements hash computation, validation, and guarded commit workflow. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The caller's path filter is an inclusion filter and workflow_dispatch bypasses it, so the reusable workflow now decides "Go bump" from the PR's changed files: anything outside go.mod, go.sum and the package file is refused before Nix is installed, and a PR touching neither go.mod nor go.sum is a no-op. That lets a caller run it on every Dependabot PR and sequence auto-merge after it, which is the second of the two ways a caller keeps a stale head from merging first; the header now states both. The classifier moves out of the run step into scripts/, embedded by heredoc like the sync-actions-comments updater, with fixtures for the go-modules mismatch, another fixed-output derivation before and after it, a bare `got:`, and malformed SRI values. Writing the fixtures found that a go-modules block without a `got:` line would take the next derivation's, so a mismatch for any other derivation now closes the block. check-embedded-sync.sh covers both embeddings.
At 37d260a the changed-files allowlist ran before the "is this a Go bump" decision, so an actions- or docker-only Dependabot PR was refused with an error rather than being the no-op the header promised. A caller that sequences auto-merge after this workflow with `needs:` would then lose auto-merge for docker bumps and show a failing job on every non-Go PR. The no-op decision now comes first; the allowlist applies to Go bumps, where it still refuses a mixed PR whoever dispatched it. The lease note stops claiming a required check holds the hash against a base whose go.sum moves after the build: required checks are not re-run when the base moves, so two Go bumps merging on the same day leave the second with a stale hash whatever the ruleset. That is the exposure every PR has to a moving base; the structural closure is a post-merge repair on main.
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
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.
Dependabot's Go bumps change
go.sumand leave the flake'svendorHashinnix/package.nixstale, so the caller's "Nix flake builds" check fails with a fixed-output hash mismatch. Where that check is required the PR wedges until a human runsmake update-nix-hashwith Docker (hey-cli #427 today); where it is not, auto-merge lands the PR red and main's next flake build is broken until someone notices (basecamp-cli #697 → #701 this morning). This adds a reusable workflow that refreshes the hash on the Dependabot PR itself, so the PR's own Nix check goes green and main is never broken.How it works
Two jobs on separate runners:
contents: read, no secrets. Builds the flake at the PR's merge commit (what the Nix check builds) and, only when Nix reports a fixed-output mismatch for the*-go-modules.drvderivation, takes the SRI hash from that diagnostic'sgot:line. A build that fails for any other reason fails the job with Nix's own output and nothing downstream runs. The classifier isscripts/extract-nix-vendor-hash.shin this repo (derived from hey-cli's), embedded by heredoc the way the sync-actions-comments updater is:check-embedded-sync.shkeeps the copy identical, andextract-nix-vendor-hash.test.mjsholds the fixtures (the real go-modules mismatch, another fixed-output derivation's mismatch before and after it, a baregot:, malformed SRI values). Before any of that,computelists the PR's changed files and refuses anything outsidego.mod,go.sumand the package file — the "Go bumps only" boundary is enforced on the diff itself, dispatch included — and is a no-op for a PR touching neithergo.modnorgo.sum.contents: write, fetches the package file at the captured head, rewrites exactly thevendorHash = "..."line, proves the diff is that one line (one removed, one added, both vendorHash lines — a rewrite that touched anything else refuses), and commits through GraphQLcreateCommitOnBranchwithexpectedHeadOidset to the captured head. That is a compare-and-swap: a Dependabot rebase mid-run makes the commit fail cleanly and the rebase's own run recomputes. No clone and no git credential ever touch a runner; the token exists only in this job, which runs no build.An App push is a normal push, so the caller's full
pull_requestCI re-runs on the new head — the Nix check there is the verification that the refreshed hash builds. (AGITHUB_TOKENpush would not re-trigger anything, and the required checks would never report on the new head.) The re-triggered run is actored by the App bot, notdependabot[bot], so the actor guard ends the loop.workflow_dispatchwith a PR number re-runs the refresh by hand; the PR still has to be an open, same-repo, Dependabot-authored one.Why this is sound for Go bumps when it was abandoned for actions bumps (#11)
After any non-Dependabot push, the re-triggered runs are actored by the pusher and see the caller's Actions secrets instead of the Dependabot sandbox. For an actions bump the PR's freshly bumped, unreviewed workflow code would run with those secrets — the reason #11 moved to post-merge repair. For a Go bump the workflow code that runs is the default branch's (Dependabot does not touch workflows; the commit pushed here is proven to touch one line of
nix/package.nix). The unreviewed content is Go dependency code inside reviewed jobs, which the Test workflow already executes on the same head. So the lift exposes exactly the secrets those reviewedpull_requestjobs reference. I audited both callers: hey-cli references none outside the actor-gated auto-merge job; basecamp-cli's only one (ANTHROPIC_API_KEY) sits in a job path-gated toskills/changes a Go bump never makes. That invariant is written into the header for the next reader. The App token grants the power every write-access member already has: the default branch is ruleset-protected and the App is not a bypass actor. Nopull_request_target.Ordering against auto-merge
Auto-merge waits for required checks only, and this build takes minutes, so a caller has to make the refresh land first; this workflow cannot do that on its own. The contract gives the two ways: keep "Nix flake builds" a required check (hey-cli — the stale head stays red until the push, and the App push, a write-access actor's, leaves auto-merge enabled for the new head), or sequence auto-merge after this workflow in one file with
needs:(basecamp-cli, basecamp/basecamp-cli#702 — the changed-files boundary makes non-Go bumps a no-op, so the caller runs it on every Dependabot PR with no path filter). A base branch whosego.summoves after the push is the exposure every PR's checks have, and only the required check holds against it; the head-onlyexpectedHeadOidlease claims no more than a clean failure on a Dependabot rebase mid-run.Alternatives weighed
vendorHash = nullwith a committedvendor/tree that Dependabot keeps in sync): the only option that removes the class outright, at the cost of vendoring third-party code into each repo and changing every developer's build. A product call, recorded rather than taken.Rollout
Callers follow, pinned to this branch's SHA and to be re-pinned after this merges: basecamp/hey-cli#432 and basecamp/basecamp-cli — the only fleet repos with a flake. Each caller needs the
cli-release-botApp installed on the repository withcontents: write, and its private key stored as a repository Dependabot secretRELEASE_APP_PRIVATE_KEY(basecamp-cli has it; hey-cli does not) plus, for the dispatch path, a repository Actions secret of the same name. The copiesrelease.ymluses are scoped to thereleaseenvironment and unreachable elsewhere (a probe on the hey-cli branch confirmedvars.RELEASE_CLIENT_IDis empty there), which is also why the client id is a literal input rather than avarslookup.actionlint1.7.12 andzizmor1.30.0 (regular persona, online) are clean; the one ignore is the samebot-conditionsdual-check reasoning the auto-merge workflows carry.