From fdef29f6b87f39fbbc1e699d750071efe6d15368 Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Tue, 1 Sep 2026 20:01:54 +0200 Subject: [PATCH 1/3] Ask the release lists whether the record still says what they say [#225] data/releases.json is what the landing table and every plugin page compute the shipping state from, and it is taken by hand: `go run . releases` writes it and nothing runs that verb. So the record aged with nothing reading it, while the landing page went on stating the day it was taken as though a reader could rely on it. A repository publishing its first finished release turns a `shell` row into a shipping one, and nothing here would have said so. The verb gains one argument. `go run . releases check` asks the same question of the same repositories, compares what came back against the record, names every repository the two disagree about with what it moved from and to, writes nothing, and refuses while any of them disagree. The reporting loop is shared with the writing mode, so a red schedule and the change that repairs it describe the same difference in the same words. A record that could not be read is refused rather than reported and carried on from, which is the opposite of what the writing mode does with the same failure: that one is about to replace the file, and this one would be reporting no difference having compared nothing. The workflow beside it asks on the weekly cadence the other three comparisons against something published elsewhere use, and proves after every run, red or green, that the tree it checked out is unchanged. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- .github/workflows/releases.yml | 70 +++++++++++++++++++ internal/releases/releases.go | 71 +++++++++++++++---- internal/releases/releases_test.go | 108 +++++++++++++++++++++++++++++ main.go | 23 ++++-- 4 files changed, 256 insertions(+), 16 deletions(-) create mode 100644 .github/workflows/releases.yml diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml new file mode 100644 index 0000000..5d2d82d --- /dev/null +++ b/.github/workflows/releases.yml @@ -0,0 +1,70 @@ +# The comparison between what data/releases.json records about published +# releases and what the release lists say today. +# +# It runs on a schedule rather than on a pull request, for the reason the pins, +# token and roster comparisons beside it do: what it reads is what somebody else +# has published, so its verdict moves when they publish rather than when this +# tree changes, and a merge blocked by that is a gate punishing the wrong +# change. The half that needs no network is the build, which reads the record on +# every run of the gate. +# +# The record it judges is unlike the copies those three compare. Those hold a +# file with an authority somewhere else and a difference is evidence about that +# file; this one is the answer itself, taken once and committed, so nothing +# published anywhere can be read to find out whether it has gone stale. Until +# this ran, nothing did: the file aged in silence while the landing page stated +# the day it was taken as though a reader could rely on it, and a repository +# publishing its first finished release went on being rendered as a shell. +# +# It reports and does not write. The verb this asks for has a mode that would +# rewrite the record, and a run that took it would destroy the difference that +# is the evidence for the change somebody makes, on the one file here where a +# machine could write the answer instead of reporting it. The last step is what +# holds it to that. +name: Release record + +on: + schedule: + # Weekly, on the cadence the other comparisons against something published + # elsewhere use, so the whole set is looked at on one rhythm rather than on + # several nobody can hold in their head. A day later than the roster it + # reads its repository names from. + - cron: "51 5 * * 3" + workflow_dispatch: + +# Deny everything at the top level; the job below grants the one scope it needs. +permissions: {} + +jobs: + releases: + name: Compare the release record against what is published + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read # checkout only; nothing here writes + steps: + - name: Checkout Repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install the toolchain go.mod pins + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + cache: false + + - name: Compare every recorded repository against its release list + run: go run . releases check + + - name: Prove the run wrote nothing + # After the comparison whether it passed or failed, because the verb + # this asks for can write that file and a run that rewrote it on its way + # to a red verdict is exactly the failure that would be missed. + if: always() + shell: bash + run: | + set -euo pipefail + git status --porcelain=v1 + git diff --exit-code + test -z "$(git status --porcelain=v1)" diff --git a/internal/releases/releases.go b/internal/releases/releases.go index 13ed7fa..7d716f9 100644 --- a/internal/releases/releases.go +++ b/internal/releases/releases.go @@ -231,36 +231,83 @@ func Run(root string, repositories []string, taken, command string, fetch Fetche return fmt.Errorf("writing %s: %w", File, err) } + moved := report(before, rec, out) + + fmt.Fprintf(out, "releases: %d repository(s) recorded as taken on %s, %d entry(s) moved, written to %s\n", + len(rec.Repositories), rec.Taken, moved, File) + return nil +} + +// Check asks the same question Run asks and writes nothing, refusing while the +// record no longer says what the release lists say. +// +// It is the route a schedule takes. Nothing else re-takes the record, so +// without one the file ages in silence while the landing page states the day it +// was taken as though a reader could rely on it, and a repository that publishes +// its first finished release goes on being rendered as a shell. +// +// It refuses on a record it could not read, which is the opposite of what Run +// does with the same failure and is the same reason in both directions. Run is +// about to replace the file, so an unreadable one is a state it may report and +// carry on from; this one has nothing to compare against, and a run that +// reported no difference because it read no record would be the green mark over +// something that did not happen. +func Check(root string, repositories []string, fetch Fetcher, out io.Writer) error { + before, err := Load(root) + if err != nil { + return fmt.Errorf("releases: %w", err) + } + + // The moment and the command are carried over from the record rather + // than taken again, because what this compares is the counts and a + // fresh date on one side would be a difference in every run. + now, err := Refresh(repositories, before.Taken, before.Command, fetch) + if err != nil { + return fmt.Errorf("releases: %w", err) + } + + moved := report(before, now, out) + + fmt.Fprintf(out, "releases: %d repository(s) compared against %s, taken on %s, %d entry(s) moved, nothing written\n", + len(now.Repositories), File, before.Taken, moved) + if moved > 0 { + return fmt.Errorf("releases: %d entry(s) in %s no longer say what the release lists say, and the pages render that file; re-take it with `go run . releases`", moved, File) + } + return nil +} + +// report names every repository the two records disagree about and answers with +// how many they were. It is shared by the two verbs above so that a schedule and +// a refresh describe the same difference in the same words, rather than a reader +// meeting one vocabulary in a red run and another in the change that repairs it. +func report(before, now Record, out io.Writer) int { moved := 0 - names := make([]string, 0, len(rec.Repositories)) - for n := range rec.Repositories { + names := make([]string, 0, len(now.Repositories)) + for n := range now.Repositories { names = append(names, n) } sort.Strings(names) for _, n := range names { - now := rec.Repositories[n] + current := now.Repositories[n] was, had := before.Repositories[n] switch { case !had: moved++ - fmt.Fprintf(out, " %s: NEW, %s\n", n, said(now)) - case !same(was, now): + fmt.Fprintf(out, " %s: NEW, %s\n", n, said(current)) + case !same(was, current): moved++ - fmt.Fprintf(out, " %s: MOVED, was %s, now %s\n", n, said(was), said(now)) + fmt.Fprintf(out, " %s: MOVED, was %s, now %s\n", n, said(was), said(current)) default: - fmt.Fprintf(out, " %s: unchanged, %s\n", n, said(now)) + fmt.Fprintf(out, " %s: unchanged, %s\n", n, said(current)) } } for n := range before.Repositories { - if _, still := rec.Repositories[n]; !still { + if _, still := now.Repositories[n]; !still { moved++ fmt.Fprintf(out, " %s: GONE, the roster no longer names it and this run did not ask about it\n", n) } } - - fmt.Fprintf(out, "releases: %d repository(s) recorded as taken on %s, %d entry(s) moved, written to %s\n", - len(rec.Repositories), rec.Taken, moved, File) - return nil + return moved } // said is one repository's entry in the words the run reports it in. The diff --git a/internal/releases/releases_test.go b/internal/releases/releases_test.go index 943655c..0815a7e 100644 --- a/internal/releases/releases_test.go +++ b/internal/releases/releases_test.go @@ -337,3 +337,111 @@ func TestTheVerbOverATreeWithNoRecordSaysThereWasNothingThere(t *testing.T) { t.Errorf("the run does not report the repository as new; it said:\n%s", log.String()) } } + +// The comparison over a record that still says what the release lists say, and +// the same record with one count edited. It is one case rather than two so that +// the green half and the red half are the same bytes apart from the edit that +// is supposed to decide them: a comparison proved only against a record it +// refuses says nothing about whether it ever passes, and one proved only +// against a record it accepts says nothing about whether it ever bites. +func TestTheComparisonBitesOnAnEditAndPassesWithoutIt(t *testing.T) { + const current = `{"taken":"2026-01-02","command":"a command","repositories":{ + "a/b":{"finished":1,"prereleases":2,"generations":["10.11"]}, + "c/d":{"finished":0,"prereleases":0}}}` + const edited = `{"taken":"2026-01-02","command":"a command","repositories":{ + "a/b":{"finished":1,"prereleases":2,"generations":["10.11"]}, + "c/d":{"finished":3,"prereleases":0,"generations":["12.0"]}}}` + + published := func(repository string) (Repository, error) { + if repository == "a/b" { + return Repository{Finished: 1, Prereleases: 2, Generations: []string{"10.11"}}, nil + } + return Repository{}, nil + } + + root, name := treeHolding(t, current) + before := bytesOf(t, name) + var log strings.Builder + if err := Check(root, []string{"a/b", "c/d"}, published, &log); err != nil { + t.Fatalf("the comparison refused a record that still says what was published: %v", err) + } + for _, want := range []string{ + "a/b: unchanged, 1 finished, 2 prerelease(s), for 10.11", + "c/d: unchanged, 0 finished, 0 prerelease(s)", + "2 repository(s) compared against " + File, + "0 entry(s) moved, nothing written", + } { + if !strings.Contains(log.String(), want) { + t.Errorf("the passing run does not say %q; it said:\n%s", want, log.String()) + } + } + if got := bytesOf(t, name); got != before { + t.Error("the passing run rewrote the record it was asked to compare against") + } + + root, name = treeHolding(t, edited) + before = bytesOf(t, name) + log.Reset() + err := Check(root, []string{"a/b", "c/d"}, published, &log) + if err == nil { + t.Fatal("the comparison passed a record that no longer says what was published") + } + if !strings.Contains(err.Error(), File) || !strings.Contains(err.Error(), "go run . releases") { + t.Errorf("the refusal reads %q, which does not name the file or what re-takes it", err) + } + if want := "c/d: MOVED, was 3 finished, 0 prerelease(s), for 12.0, now 0 finished, 0 prerelease(s)"; !strings.Contains(log.String(), want) { + t.Errorf("the refusing run does not say %q; it said:\n%s", want, log.String()) + } + if !strings.Contains(log.String(), "1 entry(s) moved, nothing written") { + t.Errorf("the refusing run does not count what moved; it said:\n%s", log.String()) + } + if got := bytesOf(t, name); got != before { + t.Error("the refusing run rewrote the record on its way to a red verdict") + } +} + +// A tree with no record is refused rather than compared against nothing. Run +// reports the same state and carries on, because it is about to write the file; +// this one would be reporting no difference having read no record, which is the +// green mark over something that did not happen. +func TestTheComparisonOverATreeWithNoRecordIsRefused(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, filepath.Dir(filepath.FromSlash(File))), 0o755); err != nil { + t.Fatalf("preparing the tree: %v", err) + } + var log strings.Builder + err := Check(root, []string{"a/b"}, + func(string) (Repository, error) { return Repository{}, nil }, &log) + if err == nil { + t.Fatal("the comparison passed a tree holding no record") + } + if strings.Contains(log.String(), "unchanged") { + t.Errorf("the run reported an entry as unchanged with nothing to compare against; it said:\n%s", log.String()) + } +} + +// treeHolding is a tree carrying one record, which both halves of the case +// above need and neither may share with the other. +func treeHolding(t *testing.T, record string) (root, name string) { + t.Helper() + root = t.TempDir() + if err := os.MkdirAll(filepath.Join(root, filepath.Dir(filepath.FromSlash(File))), 0o755); err != nil { + t.Fatalf("preparing the tree: %v", err) + } + name = filepath.Join(root, filepath.FromSlash(File)) + if err := os.WriteFile(name, []byte(record), 0o644); err != nil { + t.Fatalf("writing the record: %v", err) + } + return root, name +} + +// bytesOf is what the file holds, read as a string so that a case can say the +// run left it alone. +func bytesOf(t *testing.T, name string) string { + t.Helper() + body, err := os.ReadFile(name) + if err != nil { + t.Fatalf("reading %s: %v", name, err) + } + return string(body) +} diff --git a/main.go b/main.go index 8661f50..7da1fae 100644 --- a/main.go +++ b/main.go @@ -116,9 +116,19 @@ func run(args []string, out, errOut io.Writer) error { // every run. return freshness.Run(".", freshness.Publisher, out) case "releases": - if len(args) != 1 { + // The one argument this verb takes chooses between writing the + // answer and comparing against it. They are one verb rather than + // two because they ask the same question of the same repositories + // and differ only in what they do with what came back, and a second + // verb name would let the two drift apart. + compare := false + switch { + case len(args) == 1: + case len(args) == 2 && args[1] == "check": + compare = true + default: usage(errOut) - return errors.New("releases takes no argument") + return errors.New("releases takes no argument, or the argument check") } // Deliberately not a leg of the gate, for the reason the pins and // tokens verbs are not: what it reads is somebody else's release @@ -135,6 +145,9 @@ func run(args []string, out, errOut io.Writer) error { if err != nil { return err } + if compare { + return releases.Check(root, named, github.Fetch, out) + } return releases.Run(root, named, time.Now().UTC().Format(time.DateOnly), github.Command, github.Fetch, out) case "version": if len(args) != 1 { @@ -198,10 +211,12 @@ func usage(w io.Writer) { go run . roster compare the pinned roster against the published one, naming every row that differs, and report OFF rather than green where nothing is published to compare against - go run . releases + go run . releases [check] ask each repository the roster names what it has published and write `+releases.File+`, which is what the shipping state - and the roster's repositories are read from + and the roster's repositories are read from; with check, ask + the same question and report what has moved since that file + was taken, writing nothing go run . version print the version this repository releases under, alone on one line, which is what the release run builds its tag from go run . changelog From e9139e25750c2da349d7e9457adbfa4a1396637d Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Tue, 1 Sep 2026 20:10:31 +0200 Subject: [PATCH 2/3] Ask with a token where the environment holds one [#225] The comparison asks about twelve repositories in one run, and an anonymous caller from a shared address does not get that far: a run taken on 2026-09-01 answered 403 for the fifth repository it asked about. A run that could not ask says nothing about the record, which is the opposite of what a scheduled comparison is for. So the request that reads a release list carries a token where one is in the environment, under the name a workflow already puts it, and the workflow hands it the read-only one. No token is not an error: a contributor asking once is inside the anonymous rate and should not have to hold a credential to run a verb, and the refusal that matters is the one the request itself answers with. The metadata beside a release carries no token. It is fetched from wherever that release names, which is not this interface and is not a host this repository chose, and a credential sent there is one handed to a third party. The case beside the header proves both directions. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- .github/workflows/releases.yml | 7 ++++ internal/releases/github/github.go | 34 +++++++++++++++ internal/releases/github/github_test.go | 55 +++++++++++++++++++++++++ 3 files changed, 96 insertions(+) diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 5d2d82d..02b0416 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -55,6 +55,13 @@ jobs: cache: false - name: Compare every recorded repository against its release list + # The token is what an anonymous caller does not have, and twelve + # repositories from a shared address exhaust the anonymous rate: a run + # taken that way answered 403 for the fifth repository it asked about. + # A run that could not ask says nothing about the record rather than + # reading it and finding it current, and the scope below is read-only. + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: go run . releases check - name: Prove the run wrote nothing diff --git a/internal/releases/github/github.go b/internal/releases/github/github.go index 01a1577..5572d6d 100644 --- a/internal/releases/github/github.go +++ b/internal/releases/github/github.go @@ -16,6 +16,7 @@ import ( "fmt" "net/http" "net/url" + "os" "strings" "time" @@ -53,6 +54,7 @@ func Fetch(repository string) (releases.Repository, error) { return releases.Repository{}, err } req.Header.Set("Accept", "application/vnd.github+json") + authorise(req) client := &http.Client{Timeout: 30 * time.Second} resp, err := client.Do(req) @@ -243,3 +245,35 @@ func escapeRepository(repository string) string { } return url.PathEscape(owner) + "/" + url.PathEscape(name) } + +// TokenVariable is the environment variable a token is read from. It is the +// name a workflow already puts a token under, so a run inside one asks with it +// by having it in the environment rather than by being told to. +const TokenVariable = "GITHUB_TOKEN" + +// authorise puts a token on the request where the environment holds one. +// +// Without it the request is anonymous, and an anonymous caller is held to a +// rate the twelve repositories here exhaust: a scheduled run from a shared +// address answered 403 for the fifth repository it asked about on 2026-09-01, +// which is a run that says nothing about the record rather than one that read +// it and found it current. +// +// It is optional rather than required, because a contributor asking about +// twelve repositories once is inside the anonymous rate and should not have to +// hold a credential to run a verb. A token that is not there is not an error +// and the run says nothing about it: what it would report is the state of +// somebody's environment, and the refusal that matters is the one the request +// itself answers with. +// +// Only this request carries it. The metadata beside a release is fetched from +// wherever the release list says, which is not this interface and is not a host +// this repository chose, and a credential sent there is a credential handed to +// a third party. +func authorise(req *http.Request) { + token := strings.TrimSpace(os.Getenv(TokenVariable)) + if token == "" { + return + } + req.Header.Set("Authorization", "Bearer "+token) +} diff --git a/internal/releases/github/github_test.go b/internal/releases/github/github_test.go index c2dd0fe..20db49a 100644 --- a/internal/releases/github/github_test.go +++ b/internal/releases/github/github_test.go @@ -163,3 +163,58 @@ func TestMetadataThatCouldNotBeReadIsAFailureRatherThanTheThirdState(t *testing. } } } + +// The token reaches the request that asks this interface and nothing else. A +// credential put on the request for the metadata beside a release would be +// handed to whichever host that release names, which is not this interface and +// is not a host this repository chose. +func TestTheTokenReachesTheReleaseListAndNothingElse(t *testing.T) { + t.Setenv(TokenVariable, " a-token ") + + req, err := http.NewRequest(http.MethodGet, API+"a/b/releases?per_page=100", nil) + if err != nil { + t.Fatalf("building the request: %v", err) + } + authorise(req) + if got := req.Header.Get("Authorization"); got != "Bearer a-token" { + t.Errorf("the request carries the authorisation %q", got) + } + + // The metadata is reached through the getter below, and a case that + // sees a header on it is a case that has caught the leak. + asked := "" + _, _, err = generationsOf(func(address string) (*http.Response, error) { + asked = address + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{}, + Body: io.NopCloser(strings.NewReader(`{"targetAbi":"10.11.0.0"}`)), + }, nil + }, []release{{Tag: "1.0", Assets: []struct { + Name string `json:"name"` + URL string `json:"browser_download_url"` + }{{Name: "a" + metadataSuffix, URL: "https://elsewhere.example/a.meta.json"}}}}) + if err != nil { + t.Fatalf("reading the generation: %v", err) + } + if asked != "https://elsewhere.example/a.meta.json" { + t.Errorf("the metadata was read from %q", asked) + } +} + +// No token in the environment is not an error, and the request goes anonymous +// rather than carrying an empty credential. A contributor asking about twelve +// repositories once is inside the anonymous rate and should not have to hold +// one. +func TestNoTokenLeavesTheRequestAnonymous(t *testing.T) { + t.Setenv(TokenVariable, " ") + + req, err := http.NewRequest(http.MethodGet, API+"a/b/releases?per_page=100", nil) + if err != nil { + t.Fatalf("building the request: %v", err) + } + authorise(req) + if _, carried := req.Header["Authorization"]; carried { + t.Errorf("the request carries an authorisation header with no token behind it: %q", req.Header.Get("Authorization")) + } +} From fd08b94e080c7d91c7434c9e06a592930d3c4071 Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Tue, 1 Sep 2026 20:11:10 +0200 Subject: [PATCH 3/3] Re-take the release record, which had gone six days out of date [#225] The record the pages compute the shipping state from was taken on 2026-08-26, and the landing page states that day to a reader as the moment the data was read. Two entries had moved under it since: `requests` published a second finished release and `sso` published eight more and eight more prereleases. Nothing had said so, which is what the comparison landing beside this exists to end. It is re-taken here rather than left for the schedule to report, so that the route lands green over a record that reproduces what is published rather than red over one nothing had re-taken. go run . releases | tail -1 releases: 12 repository(s) recorded as taken on 2026-09-01, 2 entry(s) moved, written to data/releases.json Neither move changes a state word any page prints. The move that would is the one nothing would have reported either. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- data/releases.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/data/releases.json b/data/releases.json index a6cfe4f..e9d841e 100644 --- a/data/releases.json +++ b/data/releases.json @@ -1,5 +1,5 @@ { - "taken": "2026-08-26", + "taken": "2026-09-01", "command": "go run . releases, which reads https://api.github.com/repos/OWNER/NAME/releases?per_page=100 for each repository the roster names, and the .meta.json asset of each finished release for the server generation it targets", "repositories": { "Flowfin/jellyfin-plugin-discover": { @@ -15,7 +15,7 @@ "prereleases": 0 }, "Flowfin/jellyfin-plugin-requests": { - "finished": 1, + "finished": 2, "prereleases": 0, "generations": [ "10.11" @@ -34,8 +34,8 @@ "prereleases": 0 }, "Flowfin/jellyfin-plugin-sso": { - "finished": 29, - "prereleases": 53, + "finished": 37, + "prereleases": 61, "generations": [ "10.11" ],