diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index fe354ff0..6a363766 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -123,3 +123,143 @@ jobs: **Commit:** ${{ github.sha }} ⚠️ This is a development build and may be unstable. + + # A release that publishes no downloadable asset is indistinguishable + # from a healthy one: the run is green, the release page exists, and + # the hole only surfaces later when a server owner's download link + # 404s. Nothing here ever re-read the release that actually landed, + # so an upload that silently shipped nothing looked exactly like a + # successful release. + # + # This re-reads the PUBLISHED release from the API instead of trusting + # the upload steps above. Trusting the steps we just ran would rebuild + # the same "green run, empty artifact" defect one layer up: the upload + # action can skip files, partially fail, or be silently gated off, and + # only the landed release tells the truth. Assert on the fact. + - name: Verify published release assets + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RELEASE_TAG: ${{ steps.release-tag.outputs.tag }} + run: | + set -euo pipefail + + # Verify every release this run publishes. The release path writes + # both the version tag and the stable "latest" release the download + # links point at; the push path writes "latest-prerelease". + if [ "$GITHUB_EVENT_NAME" = "push" ]; then + TARGETS="latest-prerelease" + else + if [ -z "$RELEASE_TAG" ]; then + echo "::error::Release tag is empty; cannot verify the published release." + exit 1 + fi + TARGETS="$RELEASE_TAG latest" + fi + + # A non-empty asset list is NOT proof of a usable release. A release + # carrying only LICENSE, a checksum manifest, a signature bundle or + # an SBOM has a positive asset count and still offers nothing anyone + # can run. Counting assets is the same defect as trusting a green run + # without checking what it produced, so classify by name/type and + # require a real downloadable BUILD artifact (here: the plugin jars). + BUILD_FILTER='[.assets[] + | select(.state == "uploaded") + | select(.name | test( + "^checksums\\.txt$|^SHA(256|512)SUMS$|^LICENSE|^README|" + + "\\.sig$|\\.sigstore\\.json$|\\.attest\\.spdx\\.json$|" + + "\\.spdx\\.json$|\\.asc$|\\.pem$|\\.sha256$|" + + "\\.h$|\\.hpp$|\\.md$|\\.txt$" + ) | not) + | select(.size > 0)]' + + verify_release() { + local tag="$1" + local attempt RELEASE_JSON ASSET_COUNT BUILD_COUNT BUILD_NAMES + local PROBE PROBE_URL PROBE_CODE + + RELEASE_JSON="" + ASSET_COUNT=0 + BUILD_COUNT=0 + BUILD_NAMES="" + PROBE="" + PROBE_URL="" + PROBE_CODE="" + + # Asset visibility is eventually consistent right after upload, so + # poll briefly before declaring the release empty. + for attempt in $(seq 1 12); do + if ! RELEASE_JSON=$(gh api "/repos/$GITHUB_REPOSITORY/releases/tags/$tag"); then + echo "Release $tag is not readable yet (attempt $attempt); waiting..." + RELEASE_JSON="" + sleep 10 + continue + fi + + ASSET_COUNT=$(echo "$RELEASE_JSON" | jq '[.assets[] | select(.state == "uploaded")] | length') + BUILD_COUNT=$(echo "$RELEASE_JSON" | jq "$BUILD_FILTER | length") + BUILD_NAMES=$(echo "$RELEASE_JSON" | jq -r "$BUILD_FILTER | .[].name") + + PROBE="" + PROBE_URL="" + PROBE_CODE="" + if [ "$BUILD_COUNT" -gt 0 ]; then + PROBE=$(echo "$BUILD_NAMES" | head -n1) + PROBE_URL="$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/releases/download/$tag/$PROBE" + if ! PROBE_CODE=$(curl -sSL -o /dev/null -w '%{http_code}' -r 0-0 "$PROBE_URL"); then + PROBE_CODE="000" + fi + fi + + if [ "$ASSET_COUNT" -gt 0 ] && [ "$BUILD_COUNT" -gt 0 ]; then + case "$PROBE_CODE" in + 200|206) break ;; + esac + fi + + echo "Published assets are incomplete on $tag (attempt $attempt); waiting..." + sleep 10 + done + + echo "--- $tag ---" + if [ -n "$RELEASE_JSON" ]; then + echo "$RELEASE_JSON" | jq -r '.assets[] | "\(.name)\t\(.size)\t\(.state)"' + fi + echo "Real build artifacts published ($BUILD_COUNT):" + echo "${BUILD_NAMES:- (none)}" + + if [ -z "$RELEASE_JSON" ]; then + echo "::error::Release $tag could not be read back from the API; the release did not land." + exit 1 + fi + + if [ "$ASSET_COUNT" -eq 0 ]; then + echo "::error::Release $tag published with ZERO downloadable assets." + echo "::error::A release with no artifact silently did not happen. Failing loudly." + exit 1 + fi + + # Wrong-artifact-type is its own failure mode, distinct from empty. + # It is the more dangerous one because the release looks populated. + if [ "$BUILD_COUNT" -eq 0 ]; then + echo "::error::Release $tag has $ASSET_COUNT asset(s) but NO real build artifact." + echo "::error::Only LICENSE/checksums/signatures/SBOMs were published - no plugin jar." + echo "::error::A positive asset count is not a release; a downloadable build is." + exit 1 + fi + + # Finally prove one build is actually served, not merely listed by + # the API. Range-request the first byte: a released asset that 404s + # or is empty fails here rather than in a server owner's download. + if [ "$PROBE_CODE" != "200" ] && [ "$PROBE_CODE" != "206" ]; then + echo "::error::Build artifact $PROBE is listed on $tag but not downloadable (HTTP $PROBE_CODE); the build is undownloadable." + echo "::error::$PROBE_URL" + exit 1 + fi + + echo "OK: $tag publishes $BUILD_COUNT real build artifact(s) of $ASSET_COUNT assets;" + echo "OK: $PROBE downloads (HTTP $PROBE_CODE)." + } + + for target in $TARGETS; do + verify_release "$target" + done diff --git a/AGENTS.md b/AGENTS.md index df0fdf58..bc5dc530 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,6 +36,13 @@ gh -R minekube/connect-java release view --json tagName,targetCommitis curl -I -L --fail https://github.com/minekube/connect-java/releases/download//connect-velocity.jar ``` +- `release.yml`'s "Verify published release assets" step now enforces this in CI: + it re-reads each published release from the API (never the upload step's own + output) and fails when no real downloadable build jar landed, so metadata-only + assets like `LICENSE` cannot pass as a release. Pinned by + `core/.../release/ReleaseAssetVerificationTest`; keep that test's step and + upload-step names in sync when editing `release.yml`. + ## Injector Scoping (config availability) - The parent injector binds `ConfigHolder`; `ConnectPlatform.init()` populates it diff --git a/core/build.gradle.kts b/core/build.gradle.kts index 82ddfac9..cb6b5363 100644 --- a/core/build.gradle.kts +++ b/core/build.gradle.kts @@ -33,11 +33,19 @@ dependencies { testImplementation("io.netty", "netty-transport", Versions.nettyVersion) testImplementation("io.netty", "netty-codec", Versions.nettyVersion) testImplementation("com.squareup.okhttp3:mockwebserver:4.9.3") + // Parses .github/workflows/release.yml in ReleaseAssetVerificationTest. + testImplementation("org.yaml:snakeyaml:1.27") testRuntimeOnly("org.junit.platform:junit-platform-launcher") } tasks.test { useJUnitPlatform() + + // ReleaseAssetVerificationTest asserts on the release workflow, so an edit + // to that workflow must re-run the tests instead of being served from cache. + inputs.file(rootProject.file(".github/workflows/release.yml")) + .withPropertyName("releaseWorkflow") + .withPathSensitivity(PathSensitivity.RELATIVE) } relocate("org.bstats") diff --git a/core/src/test/java/com/minekube/connect/release/ReleaseAssetVerificationTest.java b/core/src/test/java/com/minekube/connect/release/ReleaseAssetVerificationTest.java new file mode 100644 index 00000000..77cb658f --- /dev/null +++ b/core/src/test/java/com/minekube/connect/release/ReleaseAssetVerificationTest.java @@ -0,0 +1,241 @@ +/* + * Copyright (c) 2019-2022 Minekube. https://minekube.com + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + * @author Minekube + * @link https://github.com/minekube/connect-java + */ + +package com.minekube.connect.release; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.regex.Pattern; +import org.junit.jupiter.api.Test; +import org.yaml.snakeyaml.Yaml; + +/** + * The release workflow used to end at "upload the artifacts and hope": nothing ever re-read the + * release that actually landed, so a release that published zero assets - or only LICENSE - looked + * exactly like a healthy one. These tests pin the guard that closes that gap. + * + *

This is the same fail-loud guard proven written in geyserlite (minekube/geyserlite#136), + * transplanted here and adapted to connect-java's build artifacts (the plugin jars). + */ +class ReleaseAssetVerificationTest { + + private static final String VERIFY_ASSETS_STEP = "Verify published release assets"; + + /** Every step that publishes assets; the guard must run after all of them. */ + private static final List UPLOAD_STEPS = Arrays.asList( + "Upload Release Artifacts", + "Update Latest Release", + "Update Pre-Release"); + + private static final Path WORKFLOW_PATH = + Paths.get("..", ".github", "workflows", "release.yml"); + private static final Path REPOSITORY_GIT_PATH = Paths.get("..", ".git"); + + @SuppressWarnings("unchecked") + private static List> readBuildJobSteps() throws Exception { + if (!Files.exists(WORKFLOW_PATH)) { + if (Files.exists(REPOSITORY_GIT_PATH)) { + throw new AssertionError( + WORKFLOW_PATH + " is missing from the repository checkout"); + } else { + assumeTrue(false, + WORKFLOW_PATH + " is unavailable outside a repository checkout"); + return List.of(); + } + } + + Map workflow; + try (InputStream in = Files.newInputStream(WORKFLOW_PATH)) { + workflow = new Yaml().load(in); + } + + Map jobs = (Map) workflow.get("jobs"); + assertTrue(jobs != null && jobs.containsKey("build"), "build job is missing"); + + Map build = (Map) jobs.get("build"); + List> steps = (List>) build.get("steps"); + assertTrue(steps != null && !steps.isEmpty(), "build job has no steps"); + return steps; + } + + private static int stepIndex(List> steps, String name) { + for (int i = 0; i < steps.size(); i++) { + if (name.equals(steps.get(i).get("name"))) { + return i; + } + } + return -1; + } + + private static String verifyScript(List> steps) { + int at = stepIndex(steps, VERIFY_ASSETS_STEP); + assertTrue(at >= 0, "build job is missing the \"" + VERIFY_ASSETS_STEP + + "\" step; a release that publishes no asset would ship green again"); + Object run = steps.get(at).get("run"); + assertTrue(run instanceof String && !((String) run).isEmpty(), + "\"" + VERIFY_ASSETS_STEP + "\" must be a run step that asserts on the " + + "published release"); + return (String) run; + } + + /** + * The core regression guard: the release job must fail when the published release carries no + * downloadable artifact. + */ + @Test + void releaseVerifiesPublishedAssets() throws Exception { + List> steps = readBuildJobSteps(); + verifyScript(steps); + + // Unlike the upload steps, the guard is not gated on the event: it verifies whichever + // release the run published, so it can never be skipped into silence. + Object condition = steps.get(stepIndex(steps, VERIFY_ASSETS_STEP)).get("if"); + assertTrue(condition == null, + "\"" + VERIFY_ASSETS_STEP + "\" is conditional (if: " + condition + + "); the guard must always run"); + } + + /** + * The point of the guard. Asserting against the upload step's own output would rebuild the exact + * "trust the run, not the artifact" defect one layer up, so the check has to go back to the + * GitHub API and read what actually landed. + */ + @Test + void releaseVerificationReadsPublishedRelease() throws Exception { + String script = verifyScript(readBuildJobSteps()); + + List required = Arrays.asList( + "/releases/tags/", // re-reads the published release by tag + "\"uploaded\"", // only fully-uploaded assets count + "releases/download/", // proves a build is actually served, not just listed + "gh api"); // reads the API, not the local build output + + List missing = new ArrayList<>(); + for (String want : required) { + if (!script.contains(want)) { + missing.add(want); + } + } + assertTrue(missing.isEmpty(), "\"" + VERIFY_ASSETS_STEP + "\" script does not reference " + + missing + "; it must assert on the published release, not on local build output"); + + // The guard must not be satisfied by inspecting the local build/libs directories: + // those jars exist even when the upload never happened. + assertTrue(!script.contains("build/libs"), + "verification must read the published release, not local build output"); + } + + /** + * Pins the second failure condition. A non-empty asset list is not proof of a usable release: a + * release carrying only LICENSE, a checksum manifest, a signature bundle or an SBOM has a + * positive asset count and still offers nothing anyone can run. So the guard must classify by + * name/type rather than count. + */ + @Test + void releaseVerificationRequiresRealBuildArtifact() throws Exception { + String script = verifyScript(readBuildJobSteps()); + + // The metadata types that must NOT satisfy the guard on their own. + List excluded = Arrays.asList( + "^checksums\\\\.txt$", // checksum manifests + "^SHA(256|512)SUMS$", // checksum manifests + "^LICENSE", // license metadata - connect-java uploads this + "^README", // readme metadata + "\\\\.sig$", // detached signatures + "\\\\.sigstore\\\\.json$", // signature bundles + "\\\\.attest\\\\.spdx\\\\.json$", // SBOM attestations + "\\\\.spdx\\\\.json$", // SBOM metadata + "\\\\.asc$", // armored signatures + "\\\\.pem$", // certificate metadata + "\\\\.sha256$", // checksum sidecars + "\\\\.h$", // C headers + "\\\\.hpp$", // C++ headers + "\\\\.md$", // markdown metadata + "\\\\.txt$"); // text metadata + + List notExcluded = new ArrayList<>(); + for (String pattern : excluded) { + if (!script.contains(pattern)) { + notExcluded.add(pattern); + } + } + assertTrue(notExcluded.isEmpty(), "build-artifact classifier does not exclude " + + notExcluded + "; a release of pure metadata would pass the guard"); + + // And it must actually gate on the classified count, not just compute it. + assertTrue(Pattern.compile("BUILD_COUNT\"\\s*-eq\\s*0").matcher(script).find(), + "guard does not fail on the classified zero build-artifact count"); + } + + /** + * Pins the ordering. The guard is meaningless before the upload: run first, it would always see + * an empty release and pass. + */ + @Test + void releaseVerificationRunsAfterUpload() throws Exception { + List> steps = readBuildJobSteps(); + + int verifyAt = stepIndex(steps, VERIFY_ASSETS_STEP); + assertTrue(verifyAt >= 0, "build job is missing the \"" + VERIFY_ASSETS_STEP + "\" step"); + + for (String uploadStep : UPLOAD_STEPS) { + int uploadAt = stepIndex(steps, uploadStep); + assertTrue(uploadAt >= 0, "expected publishing step \"" + uploadStep + + "\" in the build job"); + assertTrue(verifyAt > uploadAt, "\"" + VERIFY_ASSETS_STEP + "\" runs before \"" + + uploadStep + "\"; it would always see an empty release"); + } + } + + /** + * Both release targets must be verified. The release path publishes the version tag and + * the stable {@code latest} release whose download URLs the release body advertises; the push + * path publishes {@code latest-prerelease}. A guard that checked only one of them would leave + * the others free to ship empty. + */ + @Test + void releaseVerificationCoversEveryPublishedTarget() throws Exception { + String script = verifyScript(readBuildJobSteps()); + + assertTrue(Pattern.compile("(?m)^\\s*TARGETS=\"\\$RELEASE_TAG latest\"\\s*$") + .matcher(script).find(), + "guard does not verify the version-tag and latest release targets"); + assertTrue(Pattern.compile("(?m)^\\s*TARGETS=\"latest-prerelease\"\\s*$") + .matcher(script).find(), + "guard does not verify the latest-prerelease release target"); + assertTrue(Pattern.compile("(?m)^\\s*for target in \\$TARGETS; do\\s*$") + .matcher(script).find(), + "guard does not iterate over its assigned release targets"); + } +}