diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml new file mode 100644 index 0000000..02b0416 --- /dev/null +++ b/.github/workflows/releases.yml @@ -0,0 +1,77 @@ +# 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 + # 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 + # 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/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" ], 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")) + } +} 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