diff --git a/ci/publish-release.sh b/ci/publish-release.sh index 5ae3649..5531dc7 100755 --- a/ci/publish-release.sh +++ b/ci/publish-release.sh @@ -65,6 +65,35 @@ tested_wc=$(ci_plugin_header 'WC tested up to') body=$(printf '%s\n\n---\n\nTested up to: %s\nWC tested up to: %s\n' \ "$notes" "${tested_wp:-unknown}" "${tested_wc:-unknown}") +# Read one top-level field out of a JSON response. +# +# Not sed. A greedy `.*"id":([0-9]+)` matches the LAST id in the payload, and a +# Gitea release carries both an embedded "author" object and, once anything is +# attached, an "assets" array — each with an id of its own. Measured on this +# repo: the four published releases have ids 416/417/419/421, and the greedy +# pattern returns 1/2/3/4, which are the asset ids. +# +# Those four releases were nonetheless published correctly, and only by luck: +# the author id on this instance is **-1**, and the minus sign stops `[0-9]+` +# from matching, so on a create response — which has no assets yet — the last +# numeric id happened to be the release's. A real author id, or a re-run +# against a release that already has its zip, and it picks the wrong number. +# +# Found by the session porting this into subscriptions, where it picked the +# author id and the upload 404'd. python3 is already a required tool here. +json_field() { # field (JSON on stdin) + python3 -c ' +import json, sys +try: + data = json.load( sys.stdin ) +except Exception: + sys.exit( 0 ) +value = data.get( sys.argv[1] ) +if value is not None: + print( value ) +' "$1" +} + api() { # method path [data] curl -fsS -X "$1" \ -H "Authorization: token $GH_TOKEN" \ @@ -94,20 +123,42 @@ print(json.dumps({ # A re-run of a release must not fail on "already exists". existing=$(curl -fsS -H "Authorization: token $GH_TOKEN" \ "$GITHUB_API_URL/repos/$owner/$repo/releases/tags/$TAG" 2>/dev/null || true) - id=$(printf '%s' "$existing" | sed -n -E 's/.*"id":[[:space:]]*([0-9]+).*/\1/p' | head -1) + id=$(printf '%s' "$existing" | json_field id) if [ -z "$id" ]; then created=$(api POST "$GITHUB_API_URL/repos/$owner/$repo/releases" "$payload") - id=$(printf '%s' "$created" | sed -n -E 's/.*"id":[[:space:]]*([0-9]+).*/\1/p' | head -1) + id=$(printf '%s' "$created" | json_field id) fi [ -n "$id" ] || { echo "Could not create or find the release for $TAG." >&2; exit 1; } echo " release id $id" - curl -fsS -X POST \ - -H "Authorization: token $GH_TOKEN" \ - -F "attachment=@$zip" \ - "$GITHUB_API_URL/repos/$owner/$repo/releases/$id/assets?name=$PLUGIN_SLUG.zip" >/dev/null - echo " attached $PLUGIN_SLUG.zip" + # Before uploading anything to it, confirm the id really is this tag's. The + # failure being guarded against is not a 404 — that is loud — but an id that + # exists and belongs to something else, which would attach this plugin's zip + # to an unrelated release with every check still green. + release=$(curl -fsS -H "Authorization: token $GH_TOKEN" \ + "$GITHUB_API_URL/repos/$owner/$repo/releases/$id") + actual=$(printf '%s' "$release" | json_field tag_name) + if [ "$actual" != "$TAG" ]; then + echo "Release id $id is tagged '$actual', not '$TAG'. Refusing to upload the zip to it." >&2 + exit 1 + fi + + # A re-run must not fail on the half that already succeeded: the upload + # rejects a duplicate asset name. + if printf '%s' "$release" | python3 -c ' +import json, sys +data = json.load( sys.stdin ) +sys.exit( 0 if any( a.get( "name" ) == sys.argv[1] for a in data.get( "assets", [] ) ) else 1 ) +' "$PLUGIN_SLUG.zip"; then + echo " $PLUGIN_SLUG.zip is already attached — leaving it alone" + else + curl -fsS -X POST \ + -H "Authorization: token $GH_TOKEN" \ + -F "attachment=@$zip" \ + "$GITHUB_API_URL/repos/$owner/$repo/releases/$id/assets?name=$PLUGIN_SLUG.zip" >/dev/null + echo " attached $PLUGIN_SLUG.zip" + fi release_api="$GITHUB_API_URL/repos/$owner/$repo/releases/latest" fi diff --git a/docs/REUSE.md b/docs/REUSE.md index bc93110..648df72 100644 --- a/docs/REUSE.md +++ b/docs/REUSE.md @@ -139,6 +139,15 @@ and a release with no zip attached both produce a page that looks perfect to anyone logged in, and an update that no shop is ever offered. A check that sends a token is the false positive that hides exactly that. +**Do not parse a forge's JSON with `sed`.** A greedy `.*"id":([0-9]+)` matches +the *last* id in the payload, and a release object carries an embedded +`author` and, once anything is attached, an `assets` array — each with an id of +its own. `ci/publish-release.sh` uses a four-line `json_field()` over `python3` +instead. Measured here: the greedy pattern returns the asset ids 1/2/3/4 for +releases whose real ids are 416/417/419/421. Four releases were published +correctly anyway, purely because this instance's author id is **-1** and the +minus sign stops `[0-9]+` from matching. That is not a safety margin. + **A forge's own download URL is not the address your users can reach.** `browser_download_url` is built from the forge's configured root URL; `gitea.nolimit.dk` reports `http://57.129.89.117:25001/...` there. The updater