From 43816fdecc7bb9d3b0212463994f23224fb0d387 Mon Sep 17 00:00:00 2001 From: kristianwind Date: Wed, 16 Sep 2026 13:47:33 +0200 Subject: [PATCH] Read the release id out of JSON, not out of a greedy regex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ci/publish-release.sh found the release id with sed -n -E 's/.*"id":[[:space:]]*([0-9]+).*/\1/p' | head -1 `.*` is greedy, the response is one line, and a Gitea release payload carries an embedded "author" object and — once anything is attached — an "assets" array, each with an id of its own. So it matches the LAST id, not the release's. Measured against the four published releases here: their real ids are 416/417/419/421, and that pattern returns 1/2/3/4, which are the asset ids. All four were nonetheless published correctly, and only by luck. The author id on this instance is -1, the minus sign stops `[0-9]+` matching, and a create response has no assets yet — so the last numeric id happened to be the release's. A real author id, a Gitea that orders its fields differently, or a re-run against a release that already has its zip, and it picks a number that belongs to something else. Release 4 does not exist in this repository, so a re-run of v1.4.3 today would 404 loudly rather than attach the zip to a stranger's release, but that is the blast radius by accident, not by design. Found by the session porting this into subscriptions, where the same line picked the author id and the upload 404'd on its first real release. json_field() reads one top-level field with python3, which release.yml already requires. Verified against the live responses: 416 and 421 where the sed gives 1 and 4, and empty rather than a traceback on empty input, on "Not found." and on an error object. Two guards added with it, both verified against the live v1.4.3 release: - the id's tag_name must equal the tag before anything is uploaded to it. The failure worth guarding 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. - a re-run skips the upload when the zip is already attached, instead of failing on the half that previously succeeded. Co-Authored-By: Claude Opus 5 (1M context) --- ci/publish-release.sh | 65 ++++++++++++++++++++++++++++++++++++++----- docs/REUSE.md | 9 ++++++ 2 files changed, 67 insertions(+), 7 deletions(-) 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