fix(core): isolate release repair from tagged build code - #71
Merged
Conversation
Releases 0.6.0 and 0.7.0 published with an empty asset list: the release page exists, the run was green, and every download link 404s. There was no way to fill those holes. Re-dispatching release.yml at an old tag is not one: GitHub compiles a dispatched run from the workflow file at the dispatched ref, and tags before 0.7.1 carry no workflow_dispatch trigger at all. Worse, release.yml rewrites the live `latest` release, so running it at an old tag would drag the stable releases/download/latest/*.jar URLs backwards and silently downgrade every server owner using them. release-repair.yml dispatches from the default branch, checks the tag out itself, builds at the JDK that tag's own release.yml pinned (guessing a toolchain would produce a false red about whether a tag builds), and uploads only the assets that release is missing, under that era's own naming convention. The safety is a capability boundary, not an `if:` somebody can get wrong: the workflow holds `contents: write` and nothing else, consumes no secret beyond GITHUB_TOKEN, and never names a release other than the tag under repair - so publishing a rebuilt old jar anywhere users pull from by default is unrepresentable here rather than merely discouraged. ReleaseRepairCapabilityTest asserts that, plus: the pointer tags are refused outright, --clobber can only reach a hole and never a published good asset, and the completeness guard runs before the build rather than after it. Both verification steps assert on landed facts instead of on the steps that just ran - the same defect the release.yml guard closed. The asset check re-reads the published release from the API and range-requests a jar to prove it is served, not merely listed. The pointer check records `latest` and `latest-prerelease` before the build and diffs them after the upload, so "this workflow cannot move a pointer" is a check that can fail rather than an argument in a comment.
![no-mistakes[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
… jobs
Scoping GH_TOKEN to individual steps does not contain a repair. A repair
necessarily executes old, unreviewed tagged source, and within a single job
that build can append to $GITHUB_PATH or $GITHUB_ENV so that a later,
token-bearing step in the same job executes a tool it planted. Step-level
`env:` is not a sandbox; the job is the boundary. The previous shape put
the tagged Gradle build and every write-scoped step in one `repair` job, so
the containment it claimed was not real.
Now the write capability lives where no tag code can reach it:
build - permissions: contents: read. Checks the tag out, runs its own
pinned toolchain and Gradle build, hands the staged assets on
as an artifact. Nothing it plants can be escalated, because
there is no write capability in the job to escalate into. No
step at or after the build carries a credential at all.
publish - permissions: contents: write, needs: build. Checks out
NOTHING, runs no tag code, downloads the artifact and uploads
it to the one release named by the dispatch input.
Top-level permissions are now `{}`, so neither job inherits a scope it was
not explicitly granted.
The artifact crossing that split was produced by the untrusted job, so its
file NAMES are untrusted input to the publish job - they become public
release asset names. The publish job re-validates them against
connect-<platform>*.jar plus LICENSE and rejects symlinks, nested paths and
empty files before anything is uploaded.
Every previously pinned guard is kept and re-verified against the new
shape: the pointer-tag refusal is now the first step of BOTH jobs (it
belongs where the capability is), the completeness guard still runs before
the build, the toolchain is still pinned from the tag with a hard failure
when unreadable, absent names still upload without --clobber so GitHub's
own rejection is the atomic conditional, and the landed checks still
require a real plugin jar and prove the pointers did not move.
ReleaseRepairCapabilityTest gains three tests for the split itself and the
suite goes red on each of: granting the build job a write scope, adding a
checkout to the publish job, placing a token-bearing step after the tagged
build, and collapsing Gradle back into the publish job.
robinbraemer
changed the title
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.
Intent
Close the $GITHUB_PATH privilege-escalation vector in the release-repair workflow on PR #71 by splitting it into two jobs, per an explicit ruling from the programme owner on the identical vector in a sibling repo (gate #941 landed the same split).
WHY THIS CHANGE EXISTS - the previous shape on this branch was WRONG and must not be restored:
THE MANDATED SHAPE (do not propose collapsing it back to one job for simplicity):
DELIBERATE DECISIONS a reviewer reading only the diff would not know:
VERIFICATION ALREADY DONE LOCALLY, so a finding contradicting it needs strong evidence: full ./gradlew build green on JDK 17; shellcheck clean across all 12 run steps; and 8 mutations each turn ReleaseRepairCapabilityTest red - granting the build job contents:write, adding a checkout to the publish job, placing a token-bearing step after the tagged build, collapsing gradle back into the publish job, adding --clobber to the absent upload, weakening the jar allowlist, widening top-level permissions from {}, and removing the publish-side asset-name validation.
Context on scope: connect-java's two zero-asset releases (0.6.0, 0.7.0) are separately proven unrecoverable - their bungeecord transitive snapshot deps are 404 upstream - so this workflow lands for future value and is not exercised against them.
What Changed
release-repairworkflow with a read-only tagged build job and separate write-only publish job.ReleaseRepairCapabilityTest, workflow input tracking, and repair guidance covering asset naming and unrecoverable zero-asset releases.Risk Assessment
✅ Low: The workflow implements the mandated read-only build/write-only publish boundary and preserves the required artifact, race, pointer, and landed-verification guards.
Testing
Focused capability tests and end-user-oriented artifact validation passed; supplied baseline build, shellcheck, and mutation evidence was supplemented, and transient outputs were removed.
Evidence: Release-repair validation evidence
Valid plugin jar + LICENSE accepted; nested archive, empty jar, and symlink rejected by the actual workflow validation step.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
./gradlew :core:test --tests com.minekube.connect.release.ReleaseRepairCapabilityTestParsed the workflow and confirmed thebuild/publishjob boundaryExecuted the actual publish asset-validation step with valid and hostile artifactsRemoved transient Gradle outputs and verified a clean worktree✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.