diff --git a/docs/changes/unreleased/1832-one-release-repository.md b/docs/changes/unreleased/1832-one-release-repository.md new file mode 100644 index 0000000000..330c1ea278 --- /dev/null +++ b/docs/changes/unreleased/1832-one-release-repository.md @@ -0,0 +1,15 @@ +--- +kind: removed +title: releases come from one repository, and a 404 there is final +pr: 1832 +surface: [build] +invalidates: + - "After a 404 from Agent-Field/codeaf, the updater (selecting a release, looking up a tag's publish moment, downloading) and scripts/install.sh asked the repository's former name as a second source. They no longer do: a 404 from Agent-Field/codeaf ends the selection or install, and no other repository is asked." + - "The installer used whatever repository a caller wrote on the release. It now always downloads from Agent-Field/codeaf, so no caller can point an install at a second origin." +--- + +The former name is only GitHub's redirect to the renamed repository (the two +report one repository id), so it never held a release the real one lacked; the +fallback could only ever have reached a repository created under that name +later. The former ASSET name is still tried inside the same release, because +v0.1.0, the one stable published before the rename, carries it. diff --git a/internal/release/install_test.go b/internal/release/install_test.go index 6b524fc9ee..91f5c181d1 100644 --- a/internal/release/install_test.go +++ b/internal/release/install_test.go @@ -291,25 +291,37 @@ func TestInstallerGetsLatestStableAndFinishesWithVersion(t *testing.T) { } } -// H10: a 404 from the current repository makes release discovery retry the -// former repository and complete the install from there. -func TestH10InstallerFallsBackToLegacyRepository(t *testing.T) { - github := newInstallGitHub(t, "v1.2.3") - github.oldRepositoryOnly = true - run := runInstaller(t, github, nil, "CODEAF_NO_MODIFY_PATH=1") - if run.code != 0 { - t.Fatalf("fallback install exit %d:\n%s", run.code, run.output) - } +// onlyTheReleaseRepository fails the test for any request that named a +// repository other than the one releases come from. +func onlyTheReleaseRepository(t *testing.T, github *installGitHub) { + t.Helper() github.mu.Lock() defer github.mu.Unlock() - var current, legacy bool + if len(github.requests) == 0 { + t.Fatal("the installer asked nothing") + } for _, request := range github.requests { - current = current || request.URL.Path == "/repos/Agent-Field/codeaf/releases/latest" - legacy = legacy || request.URL.Path == "/repos/Agent-Field/aforge-v2/releases/latest" // legacy-name + path := request.URL.Path + if !strings.HasPrefix(path, "/repos/Agent-Field/codeaf/") && !strings.HasPrefix(path, "/Agent-Field/codeaf/") { + t.Fatalf("the installer asked another repository: %s", path) + } } - if !current || !legacy { - t.Fatalf("repository requests current=%v legacy=%v", current, legacy) +} + +// THE INSTALLER ASKS ONE REPOSITORY. The fake answers under the former name +// too, as a repository squatting on it could; a 404 from the real one still +// ends the install, and nothing is asked of the other. +func TestInstallerAsksOnlyTheReleaseRepository(t *testing.T) { + github := newInstallGitHub(t, "v1.2.3") + github.oldRepositoryOnly = true + run := runInstaller(t, github, nil, "CODEAF_NO_MODIFY_PATH=1") + if run.code == 0 { + t.Fatalf("an install with no release in the release repository succeeded:\n%s", run.output) } + if _, err := os.Stat(filepath.Join(run.installDir, "codeaf")); err == nil { + t.Fatal("a binary was written although the release repository had none") + } + onlyTheReleaseRepository(t, github) } // H10: a release containing only the former asset spelling is installed under @@ -327,24 +339,27 @@ func TestH10InstallerFallsBackToLegacyAssetName(t *testing.T) { } } -func TestPinnedFormerReleaseKeepsAssetAndChecksumInTheFormerRepository(t *testing.T) { - github := newInstallGitHub(t, "v0.2.0") - github.oldRepositoryOnly = true +// A release published before the rename carries the former asset name; pinning +// it still installs it, with its asset and checksum both read from the release +// repository. +func TestPinnedFormerReleaseTakesItsFormerAssetFromTheReleaseRepository(t *testing.T) { + github := newInstallGitHub(t, "v0.1.0") github.legacyAssetOnly = true - run := runInstaller(t, github, []string{"--version", "v0.2.0"}, "CODEAF_NO_MODIFY_PATH=1") + run := runInstaller(t, github, []string{"--version", "v0.1.0"}, "CODEAF_NO_MODIFY_PATH=1") if run.code != 0 { - t.Fatalf("pinned compatibility install exit %d:\n%s", run.code, run.output) + t.Fatalf("pinned former release exit %d:\n%s", run.code, run.output) } github.mu.Lock() - defer github.mu.Unlock() var asset, checksum bool for _, request := range github.requests { - asset = asset || strings.Contains(request.URL.Path, "/Agent-Field/aforge-v2/releases/download/v0.2.0/aforge-") // legacy-name - checksum = checksum || request.URL.Path == "/Agent-Field/aforge-v2/releases/download/v0.2.0/checksums.txt" // legacy-name + asset = asset || strings.Contains(request.URL.Path, "/Agent-Field/codeaf/releases/download/v0.1.0/aforge-") // legacy-name + checksum = checksum || request.URL.Path == "/Agent-Field/codeaf/releases/download/v0.1.0/checksums.txt" } + github.mu.Unlock() if !asset || !checksum { - t.Fatalf("former repository asset=%v checksum=%v", asset, checksum) + t.Fatalf("release repository asset=%v checksum=%v", asset, checksum) } + onlyTheReleaseRepository(t, github) } func TestInstallerRunsAdoptionBeforeCreatingTheStateRoot(t *testing.T) { diff --git a/internal/update/client.go b/internal/update/client.go index 7d91acc00a..a3dc167d8a 100644 --- a/internal/update/client.go +++ b/internal/update/client.go @@ -79,8 +79,13 @@ const ( downloadCeiling = 15 * time.Minute ) +// primaryRepository is THE ONE ORIGIN a release is selected from and +// downloaded from. The repository's former name is not a second one: GitHub +// answers it only as a redirect to this repository, so it never held a release +// this one lacks, and asking it after a 404 here could only ever reach a +// repository somebody created under that name later — a stranger to every +// checksum this program trusts. const primaryRepository = "Agent-Field/codeaf" -const legacyRepository = "Agent-Field/aforge-v2" // legacy-name // Client talks to the release API and release asset host. type Client struct { @@ -318,7 +323,9 @@ type Choice struct { Published time.Time } -// Release is one selected GitHub release and the repository that answered. +// Release is one selected GitHub release. Repository names where it was +// selected and is always [primaryRepository]; it is a record of that fact and +// never an address — nothing reads it to decide where to ask. type Release struct { Tag string Repository string @@ -354,16 +361,7 @@ func (c *Client) Select(ctx context.Context, choice Choice) (Release, error) { if channel != "stable" && channel != "rc" && channel != "dev" && channel != "staging" { return Release{}, fmt.Errorf("channel must be stable, rc, dev, or staging") } - for index, repository := range []string{primaryRepository, legacyRepository} { // legacy-name - release, err := c.selectRepository(ctx, repository, channel, strings.TrimSpace(choice.Running)) - if err == nil { - return release, nil - } - if !isStatus(err, http.StatusNotFound) || index == 1 { - return Release{}, err - } - } - return Release{}, errors.New("no release repository answered") + return c.selectRelease(ctx, channel, strings.TrimSpace(choice.Running)) } // TagPublished asks the release API for ONE exact tag's publish moment, and it @@ -385,34 +383,28 @@ func (c *Client) TagPublished(ctx context.Context, tag string) (time.Time, bool) if strings.TrimSpace(c.APIBase) == "" { return time.Time{}, false } - for index, repository := range []string{primaryRepository, legacyRepository} { // legacy-name - rawURL := strings.TrimRight(c.APIBase, "/") + "/repos/" + repository + "/releases/tags/" + url.PathEscape(tag) - body, err := c.get(ctx, rawURL, "application/vnd.github+json", true, "release API") - if err != nil { - if isStatus(err, http.StatusNotFound) && index == 0 { - continue - } - return time.Time{}, false - } - var row apiRelease - if json.Unmarshal(body, &row) != nil || strings.TrimSpace(row.TagName) != tag { - return time.Time{}, false - } - moment := row.stamp() - if moment.IsZero() { - return time.Time{}, false - } - return moment, true + rawURL := strings.TrimRight(c.APIBase, "/") + "/repos/" + primaryRepository + "/releases/tags/" + url.PathEscape(tag) + body, err := c.get(ctx, rawURL, "application/vnd.github+json", true, "release API") + if err != nil { + return time.Time{}, false + } + var row apiRelease + if json.Unmarshal(body, &row) != nil || strings.TrimSpace(row.TagName) != tag { + return time.Time{}, false + } + moment := row.stamp() + if moment.IsZero() { + return time.Time{}, false } - return time.Time{}, false + return moment, true } -func (c *Client) selectRepository(ctx context.Context, repository, channel, running string) (Release, error) { +func (c *Client) selectRelease(ctx context.Context, channel, running string) (Release, error) { suffix := "releases/latest" if channel != "stable" { suffix = "releases?per_page=100" } - rawURL := strings.TrimRight(c.APIBase, "/") + "/repos/" + repository + "/" + suffix + rawURL := strings.TrimRight(c.APIBase, "/") + "/repos/" + primaryRepository + "/" + suffix body, err := c.get(ctx, rawURL, "application/vnd.github+json", true, "release API") if err != nil { return Release{}, err @@ -425,7 +417,7 @@ func (c *Client) selectRepository(ctx context.Context, repository, channel, runn if Kind(row.TagName) != "stable" { return Release{}, fmt.Errorf("the latest release did not name a stable codeaf tag") } - return Release{Tag: row.TagName, Repository: repository, PublishedAt: row.stamp()}, nil + return Release{Tag: row.TagName, Repository: primaryRepository, PublishedAt: row.stamp()}, nil } var rows []apiRelease if err := json.Unmarshal(body, &rows); err != nil { @@ -448,12 +440,15 @@ func (c *Client) selectRepository(ctx context.Context, repository, channel, runn return Release{}, fmt.Errorf("no %s build has been published yet", channel) } return Release{ - Tag: newest.TagName, Repository: repository, PublishedAt: newest.stamp(), + Tag: newest.TagName, Repository: primaryRepository, PublishedAt: newest.stamp(), RunningPublishedAt: runningPublished, }, nil } +// assetURL names one file of a release on the download host. The repository in +// it is the constant, not anything carried on the release, so no caller can +// point a download at a second origin. func (c *Client) assetURL(release Release, name string) string { base := strings.TrimRight(c.DownloadBase, "/") - return base + "/" + release.Repository + "/releases/download/" + url.PathEscape(release.Tag) + "/" + url.PathEscape(name) + return base + "/" + primaryRepository + "/releases/download/" + url.PathEscape(release.Tag) + "/" + url.PathEscape(name) } diff --git a/internal/update/install.go b/internal/update/install.go index 7abb701004..6c6a49694c 100644 --- a/internal/update/install.go +++ b/internal/update/install.go @@ -76,9 +76,6 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error) if release.Tag == "" { return InstallResult{}, errors.New("the release tag is empty") } - if release.Repository == "" { - release.Repository = primaryRepository - } // ONE ACQUIRER, INSIDE THE SHARED INSTALLER. Every road that replaces this // file — the chat surface's background install and `codeaf update` alike — // comes through here, so the lock and the on-disk re-read cannot be @@ -144,21 +141,12 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error) return InstallResult{}, &RefusalError{Reason: reason} } } - var ( - body []byte - checksums []byte - err error - ) - for index, repository := range installRepositories(release.Repository) { - release.Repository = repository - body, asset, checksums, err = options.Client.downloadRelease(ctx, release, asset, goos, goarch, extension) - if err == nil { - break - } - if !isStatus(err, http.StatusNotFound) || index == 1 { - return InstallResult{}, err - } - } + // THE BYTES AND THE CHECKSUM COME FROM THE ONE ORIGIN ([primaryRepository]), + // whatever repository a caller wrote on the release ([Client.assetURL] + // reads only the constant): a 404 there is an answer, never a reason to go + // and ask somewhere else. The result records it. + release.Repository = primaryRepository + body, asset, checksums, err := options.Client.downloadRelease(ctx, release, asset, goos, goarch, extension) if err != nil { return InstallResult{}, err } @@ -198,13 +186,11 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error) return InstallResult{Release: release, Path: target}, nil } -func installRepositories(first string) []string { - if first == legacyRepository { - return []string{legacyRepository} - } - return []string{primaryRepository, legacyRepository} // legacy-name -} - +// downloadRelease fetches the asset and its checksums from one release. The +// former asset name is still tried, IN THE SAME RELEASE, for the one stable +// published before the rename (v0.1.0): a person may still name that tag to roll +// back to it, and checksums.txt beside it is what vouches for whichever name +// answered. func (c *Client) downloadRelease(ctx context.Context, release Release, asset, goos, goarch, extension string) ([]byte, string, []byte, error) { shownAsset := asset body, err := c.downloadAsset(ctx, c.assetURL(release, asset), shownAsset) diff --git a/internal/update/update_test.go b/internal/update/update_test.go index 525b9faeb3..225a2992c2 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -12,6 +12,7 @@ import ( "path/filepath" "runtime" "strings" + "sync" "sync/atomic" "testing" "time" @@ -845,18 +846,87 @@ func TestC10RedirectsNeverCarryTheAPITokenToAnotherHost(t *testing.T) { } } -// TestC11RepositoryAndAssetFallbacksKeepOldReleasesInstallable proves C11. -func TestC11RepositoryAndAssetFallbacksKeepOldReleasesInstallable(t *testing.T) { +// requestLog is the paths a fake server was asked, kept under a lock because +// the server's handler goroutines write while the test reads. +type requestLog struct { + mu sync.Mutex + paths []string +} + +func (l *requestLog) record(path string) { + l.mu.Lock() + defer l.mu.Unlock() + l.paths = append(l.paths, path) +} + +func (l *requestLog) list() []string { + l.mu.Lock() + defer l.mu.Unlock() + return append([]string(nil), l.paths...) +} + +// onlyTheReleaseRepository fails the test for any request that names a +// repository other than the one origin, in the API's spelling or the download +// host's. +func onlyTheReleaseRepository(t *testing.T, paths []string) { + t.Helper() + for _, path := range paths { + if !strings.HasPrefix(path, "/repos/"+primaryRepository+"/") && !strings.HasPrefix(path, "/"+primaryRepository+"/") { + t.Fatalf("a request left the release repository: %s\nall requests:\n%s", path, strings.Join(paths, "\n")) + } + } +} + +// TestANotFoundFromTheReleaseRepositoryIsTheAnswer proves there is ONE origin. +// A 404 from it ends the selection, the moment lookup and the download alike, +// and nothing is asked of any other repository — not even when the caller +// wrote another one on the release. +func TestANotFoundFromTheReleaseRepositoryIsTheAnswer(t *testing.T) { + requests := &requestLog{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.record(r.URL.Path) + http.NotFound(w, r) + })) + defer server.Close() + client := releaseClient(server, "v0.1.1") + for _, channel := range []string{"stable", "dev"} { + if release, err := client.Select(context.Background(), Choice{Channel: channel}); err == nil || !isStatus(err, http.StatusNotFound) { + t.Fatalf("%s selection = %+v, %v; want the repository's 404", channel, release, err) + } + } + if moment, found := client.TagPublished(context.Background(), "dev-20261001-0123456789ab"); found { + t.Fatalf("a 404 answered a publish moment %s", moment) + } + target := filepath.Join(t.TempDir(), "codeaf") + if err := os.WriteFile(target, []byte("old"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := Install(context.Background(), InstallOptions{ + Client: client, Target: target, Release: Release{Tag: "v0.2.0", Repository: "Agent-Field/elsewhere"}, + }); err == nil { + t.Fatal("an install found bytes nowhere and still succeeded") + } + if got, _ := os.ReadFile(target); string(got) != "old" { + t.Fatalf("the original was replaced: %q", got) + } + paths := requests.list() + if len(paths) == 0 { + t.Fatal("the server was never asked") + } + onlyTheReleaseRepository(t, paths) +} + +// TestTheFormerAssetNameStillInstallsTheOneOldRelease proves C11's surviving +// half: v0.1.0 was published before the rename under the former asset name, and +// a person naming that tag still gets it — from the same release, vouched for +// by the checksums.txt beside it. +func TestTheFormerAssetNameStillInstallsTheOneOldRelease(t *testing.T) { asset := []byte("old named release") digest := sha256.Sum256(asset) - var paths []string + requests := &requestLog{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - paths = append(paths, r.URL.Path) + requests.record(r.URL.Path) switch { - case strings.Contains(r.URL.Path, "/repos/"+primaryRepository+"/"): - http.NotFound(w, r) - case strings.Contains(r.URL.Path, "/repos/"+legacyRepository+"/"): - fmt.Fprint(w, `{"tag_name":"v0.2.0"}`) case strings.HasSuffix(r.URL.Path, "/aforge-"+runtime.GOOS+"-"+runtime.GOARCH): // legacy-name _, _ = w.Write(asset) case strings.HasSuffix(r.URL.Path, "/checksums.txt"): @@ -866,24 +936,27 @@ func TestC11RepositoryAndAssetFallbacksKeepOldReleasesInstallable(t *testing.T) } })) defer server.Close() - client := releaseClient(server, "v0.1.1") - release, err := client.Select(context.Background(), Choice{Channel: "stable"}) - if err != nil || release.Repository != legacyRepository { - t.Fatalf("release = %+v, error %v", release, err) - } + client := releaseClient(server, "v0.2.0") target := filepath.Join(t.TempDir(), "codeaf") - if err := os.WriteFile(target, []byte("old"), 0o755); err != nil { + if err := os.WriteFile(target, []byte("new"), 0o755); err != nil { t.Fatal(err) } - if _, err := Install(context.Background(), InstallOptions{Client: client, Release: release, Target: target}); err != nil { + if _, err := Install(context.Background(), InstallOptions{ + Client: client, Target: target, Release: Release{Tag: "v0.1.0"}, AllowDowngrade: true, + }); err != nil { t.Fatal(err) } + if got, _ := os.ReadFile(target); string(got) != string(asset) { + t.Fatalf("installed %q, want the release under its former asset name", got) + } + paths := requests.list() joined := strings.Join(paths, "\n") - for _, want := range []string{"/repos/" + primaryRepository, "/repos/" + legacyRepository, "/codeaf-", "/aforge-"} { // legacy-name + for _, want := range []string{"/codeaf-", "/aforge-"} { // legacy-name if !strings.Contains(joined, want) { t.Fatalf("requests do not contain %q:\n%s", want, joined) } } + onlyTheReleaseRepository(t, paths) } // TestAssetFailureNamesTheCurrentAssetAndStatusWithoutExposingFallbacks proves D9. diff --git a/scripts/install.sh b/scripts/install.sh index 7b57809152..b28a2d10ef 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -2,8 +2,9 @@ set -euo pipefail +# The one repository releases come from. Its former name is only GitHub's +# redirect to it, so a 404 here is the answer: no other repository is asked. REPOSITORY="Agent-Field/codeaf" -LEGACY_REPOSITORY="Agent-Field/aforge-v2" # Remove after the one-release repository fallback. # legacy-name CHANNEL="${CHANNEL:-stable}" INSTALL_NAME="${CODEAF_INSTALL_NAME:-codeaf}" VERSION="${VERSION:-}" @@ -327,17 +328,7 @@ api_problem() { release_api_get() { local suffix="$1" local destination="$2" - if http_get "$GITHUB_API/repos/$REPOSITORY/$suffix" "$destination" "application/vnd.github+json" 1; then - return 0 - fi - if [[ "$HTTP_STATUS" == "404" && "$REPOSITORY" != "$LEGACY_REPOSITORY" ]]; then - # Remove after the renamed repository has carried releases for one release. - if http_get "$GITHUB_API/repos/$LEGACY_REPOSITORY/$suffix" "$destination" "application/vnd.github+json" 1; then - REPOSITORY="$LEGACY_REPOSITORY" - return 0 - fi - fi - return 1 + http_get "$GITHUB_API/repos/$REPOSITORY/$suffix" "$destination" "application/vnd.github+json" 1 } extract_tags() { @@ -533,17 +524,8 @@ if [[ "$VERBOSE" == "1" ]]; then fi fi spin_label "Downloading codeaf $TAG for $OS/$ARCH" -DOWNLOAD_REPOSITORY="$REPOSITORY" -if ! download_release "$DOWNLOAD_REPOSITORY"; then - if [[ "$HTTP_STATUS" == "404" && "$DOWNLOAD_REPOSITORY" != "$LEGACY_REPOSITORY" ]]; then - # Remove after the renamed repository has carried releases for one release. - DOWNLOAD_REPOSITORY="$LEGACY_REPOSITORY" - if ! download_release "$DOWNLOAD_REPOSITORY"; then - fail "no codeaf-${OS}-${ARCH}${extension} in release ${TAG}; check the tag on the Releases page" - fi - else - fail "no codeaf-${OS}-${ARCH}${extension} in release ${TAG}; check the tag on the Releases page" - fi +if ! download_release "$REPOSITORY"; then + fail "no codeaf-${OS}-${ARCH}${extension} in release ${TAG}; check the tag on the Releases page" fi expected=$(awk -v name="$ASSET" '$2 == name || $2 == "*" name {print $1; exit}' "$TMP_ROOT/checksums.txt")