From 99d7f484c5f67e15b0abb1845e5d31192952fd5c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 17:03:37 +0000 Subject: [PATCH 1/2] fix(daemon): auto-download missing shelltime-daemon binary on install When `shelltime daemon install/reinstall` cannot find the daemon binary locally, fetch the matching release archive from GitHub and extract shelltime-daemon to ~/.shelltime/bin/. This recovers curl-installer users who installed before commit shelltime/installation@36a09e9 (where the daemon was renamed to .bak and download URLs 404'd) without forcing them to rerun the installer. - Add model.EnsureDaemonBinary: resolves locally first; on miss downloads the archive for the CLI's own version (falling back to latest for dev builds or 404'd tags), extracts shelltime-daemon, places it at ~/.shelltime/bin/shelltime-daemon via existing ReplaceBinary. - Refuse auto-download for Homebrew installs (point users at `brew reinstall`) and for Windows (no daemon binary built). - Make github base URLs swappable for tests in updater.go. - Wire the new helper into commands/daemon.install.go after the existing ResolveDaemonBinaryPath() call so the .bak restore path still runs. https://claude.ai/code/session_01ThKSJA9sGXQm8jMeCZZdYY --- commands/daemon.install.go | 24 ++- model/daemon_fetch.go | 124 ++++++++++++ model/daemon_fetch_test.go | 376 +++++++++++++++++++++++++++++++++++++ model/updater.go | 17 +- 4 files changed, 530 insertions(+), 11 deletions(-) create mode 100644 model/daemon_fetch.go create mode 100644 model/daemon_fetch_test.go diff --git a/commands/daemon.install.go b/commands/daemon.install.go index 4a6beb1..8c2fd3f 100644 --- a/commands/daemon.install.go +++ b/commands/daemon.install.go @@ -39,15 +39,27 @@ func commandDaemonInstall(c *cli.Context) error { } } - // Resolve daemon binary (Homebrew/PATH preferred, curl-installer fallback) + // Resolve daemon binary (Homebrew/PATH preferred, curl-installer fallback). + // On miss, try to auto-download a matching daemon archive from GitHub + // releases so curl-installer users from before the daemon-bundling fix + // don't have to rerun the installer. daemonBinPath, err := model.ResolveDaemonBinaryPath() if err != nil { - color.Red.Println("❌ shelltime-daemon binary not found.") - color.Yellow.Println("Install via Homebrew: brew install shelltime/tap/shelltime") - color.Yellow.Println("Or via curl installer: curl -sSL https://shelltime.xyz/i | bash") - return fmt.Errorf("shelltime-daemon binary not found: %w", err) + color.Yellow.Println("⚠️ shelltime-daemon binary not found locally.") + cliPath, _ := model.ResolveCLIBinaryPath() + color.Yellow.Println("⬇️ Attempting to download matching daemon from GitHub releases...") + daemonBinPath, err = model.EnsureDaemonBinary(c.Context, cliPath, commitID) + if err != nil { + color.Red.Println("❌ shelltime-daemon binary not found and auto-download failed.") + color.Yellow.Printf(" reason: %v\n", err) + color.Yellow.Println("Install via Homebrew: brew install shelltime/tap/shelltime") + color.Yellow.Println("Or via curl installer: curl -sSL https://shelltime.xyz/i | bash") + return fmt.Errorf("shelltime-daemon binary not found: %w", err) + } + color.Green.Printf("✅ Downloaded daemon binary to: %s\n", daemonBinPath) + } else { + color.Green.Printf("✅ Found daemon binary at: %s\n", daemonBinPath) } - color.Green.Printf("✅ Found daemon binary at: %s\n", daemonBinPath) // If we picked a system-managed binary but a stale curl-installer copy // still lives under ~/.shelltime/bin, remove it so future resolution diff --git a/model/daemon_fetch.go b/model/daemon_fetch.go new file mode 100644 index 0000000..d583ad2 --- /dev/null +++ b/model/daemon_fetch.go @@ -0,0 +1,124 @@ +package model + +import ( + "context" + "fmt" + "os" + "path/filepath" + "runtime" + "strings" +) + +// daemonFetchGOOS is a swappable indirection over runtime.GOOS so tests can +// exercise the Windows-aborts branch on Linux/Darwin runners. +var daemonFetchGOOS = runtime.GOOS + +// EnsureDaemonBinary returns the path to a usable shelltime-daemon binary, +// downloading it from GitHub releases into the curl-installer location +// (~/.shelltime/bin/shelltime-daemon) when no existing binary is found. +// +// cliBinPath is the resolved path of the running CLI; it is consulted only +// to refuse auto-download for Homebrew installs (those should use +// `brew reinstall`). cliVersion is the CLI's own version (typically +// commands.commitID); when empty or "dev" we fall back to the latest +// GitHub release tag. +func EnsureDaemonBinary(ctx context.Context, cliBinPath, cliVersion string) (string, error) { + if p, err := ResolveDaemonBinaryPath(); err == nil { + return p, nil + } + + if daemonFetchGOOS == "windows" { + return "", fmt.Errorf("shelltime-daemon is not built for Windows; please use WSL or run the CLI without the daemon") + } + + if cliBinPath != "" && DetectInstallKind(cliBinPath) == InstallKindHomebrew { + return "", fmt.Errorf("shelltime-daemon missing for Homebrew install; run: brew reinstall shelltime/tap/shelltime") + } + + tag := normalizeDaemonTag(cliVersion) + if tag == "" { + latest, err := FetchLatestVersion(ctx) + if err != nil { + return "", fmt.Errorf("cannot determine release tag (no CLI version, latest lookup failed): %w", err) + } + tag = latest + } + + archiveName, err := BuildArchiveName(runtime.GOOS, runtime.GOARCH) + if err != nil { + return "", err + } + + destPath, err := fetchDaemonToCurlPath(ctx, tag, archiveName) + if err == nil { + return destPath, nil + } + + // Fall back once to latest if the tagged release 404'd (deleted/yanked). + if strings.Contains(err.Error(), "status 404") { + latest, lerr := FetchLatestVersion(ctx) + if lerr == nil && latest != "" && latest != tag { + if destPath, ferr := fetchDaemonToCurlPath(ctx, latest, archiveName); ferr == nil { + return destPath, nil + } + } + } + return "", err +} + +// fetchDaemonToCurlPath downloads, verifies, extracts, and installs the +// shelltime-daemon binary from release `tag` to GetCurlInstallerDaemonPath(). +func fetchDaemonToCurlPath(ctx context.Context, tag, archiveName string) (string, error) { + downloadURL := BuildDownloadURL(tag, archiveName) + + sum, _, _ := FetchChecksum(ctx, tag, archiveName) + // FetchChecksum errors and absent checksums are both non-fatal; we proceed + // without verification, matching the `shelltime update` behavior. + + tmpDir, err := os.MkdirTemp("", "shelltime-daemon-fetch-*") + if err != nil { + return "", fmt.Errorf("create temp dir: %w", err) + } + defer os.RemoveAll(tmpDir) + + archivePath := filepath.Join(tmpDir, archiveName) + if err := DownloadAndVerify(ctx, downloadURL, sum, archivePath); err != nil { + return "", fmt.Errorf("download daemon archive: %w", err) + } + + extractDir := filepath.Join(tmpDir, "extracted") + if err := os.MkdirAll(extractDir, 0o755); err != nil { + return "", err + } + binaries, err := ExtractBinaries(archivePath, extractDir) + if err != nil { + return "", fmt.Errorf("extract archive: %w", err) + } + daemonSrc, ok := binaries["shelltime-daemon"] + if !ok { + return "", fmt.Errorf("archive %s did not contain shelltime-daemon", archiveName) + } + + destPath := GetCurlInstallerDaemonPath() + if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { + return "", fmt.Errorf("create bin dir: %w", err) + } + if err := ReplaceBinary(daemonSrc, destPath); err != nil { + return "", fmt.Errorf("install daemon binary: %w", err) + } + return destPath, nil +} + +// normalizeDaemonTag returns "" for empty/"dev" inputs (signaling the caller +// to fetch the latest tag), otherwise ensures the result has a leading "v" +// to match goreleaser's tag template. +func normalizeDaemonTag(v string) string { + v = strings.TrimSpace(v) + if v == "" || v == "dev" { + return "" + } + if !strings.HasPrefix(v, "v") { + v = "v" + v + } + return v +} diff --git a/model/daemon_fetch_test.go b/model/daemon_fetch_test.go new file mode 100644 index 0000000..fa5e48d --- /dev/null +++ b/model/daemon_fetch_test.go @@ -0,0 +1,376 @@ +package model + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// daemonFetchFixture is a fake GitHub release server. Routes: +// +// GET /repos///releases/latest → {"tag_name": LatestTag} +// GET ///releases/download// → archive bytes (200) or 404 +// GET ///releases/download//checksums.txt → checksum line +type daemonFetchFixture struct { + t *testing.T + LatestTag string + Archives map[string][]byte // key: archiveName + NotFoundTag string // if a request comes in for this tag, respond 404 once + hits404 atomic.Int32 + releaseHits atomic.Int32 + apiHits atomic.Int32 +} + +func newDaemonFetchFixture(t *testing.T) *daemonFetchFixture { + t.Helper() + return &daemonFetchFixture{t: t, Archives: map[string][]byte{}} +} + +func (f *daemonFetchFixture) start(t *testing.T) (apiURL, releaseURL string) { + t.Helper() + apiSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.apiHits.Add(1) + if strings.HasSuffix(r.URL.Path, "/releases/latest") { + fmt.Fprintf(w, `{"tag_name": %q}`, f.LatestTag) + return + } + http.NotFound(w, r) + })) + t.Cleanup(apiSrv.Close) + + relSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.releaseHits.Add(1) + parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + // expected: //releases/download// + if len(parts) < 6 { + http.NotFound(w, r) + return + } + tag := parts[4] + file := parts[5] + if f.NotFoundTag != "" && tag == f.NotFoundTag { + f.hits404.Add(1) + http.NotFound(w, r) + return + } + if file == "checksums.txt" { + var lines []string + for name, body := range f.Archives { + sum := sha256.Sum256(body) + lines = append(lines, fmt.Sprintf("%s %s", hex.EncodeToString(sum[:]), name)) + } + fmt.Fprintln(w, strings.Join(lines, "\n")) + return + } + body, ok := f.Archives[file] + if !ok { + http.NotFound(w, r) + return + } + _, _ = w.Write(body) + })) + t.Cleanup(relSrv.Close) + return apiSrv.URL, relSrv.URL +} + +// pointURLsAt overrides the package-level GitHub base URLs for the duration +// of the test. +func pointURLsAt(t *testing.T, apiURL, releaseURL string) { + t.Helper() + prevAPI := githubAPIBaseURL + prevRel := githubReleaseBaseURL + githubAPIBaseURL = apiURL + githubReleaseBaseURL = releaseURL + t.Cleanup(func() { + githubAPIBaseURL = prevAPI + githubReleaseBaseURL = prevRel + }) +} + +// makeArchiveWithDaemon returns archive bytes containing `shelltime-daemon` +// (and optionally `shelltime`) in the platform-appropriate format. +func makeArchiveWithDaemon(t *testing.T, includeDaemon bool) (name string, body []byte) { + t.Helper() + archiveName, err := BuildArchiveName(runtime.GOOS, runtime.GOARCH) + require.NoError(t, err) + + if strings.HasSuffix(archiveName, ".zip") { + return archiveName, makeZip(t, includeDaemon) + } + return archiveName, makeTarGz(t, includeDaemon) +} + +func makeZip(t *testing.T, includeDaemon bool) []byte { + t.Helper() + tmp := filepath.Join(t.TempDir(), "out.zip") + f, err := os.Create(tmp) + require.NoError(t, err) + zw := zip.NewWriter(f) + cli, _ := zw.Create("shelltime") + _, _ = cli.Write([]byte("CLI")) + if includeDaemon { + d, _ := zw.Create("shelltime-daemon") + _, _ = d.Write([]byte("DAEMON_BODY")) + } + require.NoError(t, zw.Close()) + require.NoError(t, f.Close()) + body, err := os.ReadFile(tmp) + require.NoError(t, err) + return body +} + +func makeTarGz(t *testing.T, includeDaemon bool) []byte { + t.Helper() + tmp := filepath.Join(t.TempDir(), "out.tar.gz") + f, err := os.Create(tmp) + require.NoError(t, err) + gw := gzip.NewWriter(f) + tw := tar.NewWriter(gw) + write := func(name, body string) { + require.NoError(t, tw.WriteHeader(&tar.Header{ + Name: name, Mode: 0o755, Size: int64(len(body)), Typeflag: tar.TypeReg, + })) + _, err := tw.Write([]byte(body)) + require.NoError(t, err) + } + write("shelltime", "CLI") + if includeDaemon { + write("shelltime-daemon", "DAEMON_BODY") + } + require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + require.NoError(t, f.Close()) + body, err := os.ReadFile(tmp) + require.NoError(t, err) + return body +} + +// isolateDaemonFetchEnv mirrors withIsolatedDaemonResolution but exposes the +// home dir for assertions and resets daemonFetchGOOS for the test. +func isolateDaemonFetchEnv(t *testing.T) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", "") + + prevHomebrew := daemonHomebrewSearchPaths + daemonHomebrewSearchPaths = nil + prevGOOS := daemonFetchGOOS + daemonFetchGOOS = runtime.GOOS + t.Cleanup(func() { + daemonHomebrewSearchPaths = prevHomebrew + daemonFetchGOOS = prevGOOS + }) + return home +} + +func TestEnsureDaemon_ExistingBinaryShortCircuits(t *testing.T) { + home := isolateDaemonFetchEnv(t) + existing := writeFakeDaemon(t, filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin")) + + fx := newDaemonFetchFixture(t) + apiURL, relURL := fx.start(t) + pointURLsAt(t, apiURL, relURL) + + got, err := EnsureDaemonBinary(context.Background(), "/usr/bin/shelltime", "v0.1.83") + require.NoError(t, err) + assert.Equal(t, existing, got) + assert.Zero(t, fx.apiHits.Load(), "no API hit expected when binary already present") + assert.Zero(t, fx.releaseHits.Load(), "no download hit expected when binary already present") +} + +func TestEnsureDaemon_DownloadsWhenMissing(t *testing.T) { + home := isolateDaemonFetchEnv(t) + + fx := newDaemonFetchFixture(t) + fx.LatestTag = "v9.9.9" + name, body := makeArchiveWithDaemon(t, true) + fx.Archives[name] = body + apiURL, relURL := fx.start(t) + pointURLsAt(t, apiURL, relURL) + + cliPath := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime") + got, err := EnsureDaemonBinary(context.Background(), cliPath, "v0.1.83") + require.NoError(t, err) + expected := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime-daemon") + assert.Equal(t, expected, got) + + info, err := os.Stat(expected) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o755), info.Mode().Perm()) + contents, _ := os.ReadFile(expected) + assert.Equal(t, "DAEMON_BODY", string(contents)) +} + +func TestEnsureDaemon_HomebrewAborts(t *testing.T) { + isolateDaemonFetchEnv(t) + + fx := newDaemonFetchFixture(t) + apiURL, relURL := fx.start(t) + pointURLsAt(t, apiURL, relURL) + + _, err := EnsureDaemonBinary(context.Background(), "/opt/homebrew/bin/shelltime", "v0.1.83") + require.Error(t, err) + assert.Contains(t, err.Error(), "brew reinstall") + assert.Zero(t, fx.releaseHits.Load(), "no download attempt expected for Homebrew install") +} + +func TestEnsureDaemon_WindowsAborts(t *testing.T) { + isolateDaemonFetchEnv(t) + daemonFetchGOOS = "windows" + + fx := newDaemonFetchFixture(t) + apiURL, relURL := fx.start(t) + pointURLsAt(t, apiURL, relURL) + + _, err := EnsureDaemonBinary(context.Background(), "C:\\Users\\me\\.shelltime\\bin\\shelltime.exe", "v0.1.83") + require.Error(t, err) + assert.Contains(t, err.Error(), "Windows") + assert.Zero(t, fx.releaseHits.Load()) + assert.Zero(t, fx.apiHits.Load()) +} + +func TestEnsureDaemon_UsesCliVersionWhenSet(t *testing.T) { + home := isolateDaemonFetchEnv(t) + + fx := newDaemonFetchFixture(t) + name, body := makeArchiveWithDaemon(t, true) + fx.Archives[name] = body + + var seenTag atomic.Value + apiSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + t.Cleanup(apiSrv.Close) + relSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + if len(parts) >= 6 { + seenTag.Store(parts[4]) + } + file := parts[len(parts)-1] + if file == "checksums.txt" { + fmt.Fprintln(w) + return + } + if b, ok := fx.Archives[file]; ok { + _, _ = w.Write(b) + return + } + http.NotFound(w, r) + })) + t.Cleanup(relSrv.Close) + pointURLsAt(t, apiSrv.URL, relSrv.URL) + + cliPath := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime") + _, err := EnsureDaemonBinary(context.Background(), cliPath, "v0.94.5") + require.NoError(t, err) + assert.Equal(t, "v0.94.5", seenTag.Load()) +} + +func TestEnsureDaemon_PrefixesVForUnprefixedVersion(t *testing.T) { + assert.Equal(t, "v0.94.5", normalizeDaemonTag("0.94.5")) + assert.Equal(t, "v0.94.5", normalizeDaemonTag("v0.94.5")) + assert.Equal(t, "", normalizeDaemonTag("")) + assert.Equal(t, "", normalizeDaemonTag("dev")) + assert.Equal(t, "", normalizeDaemonTag(" ")) +} + +func TestEnsureDaemon_FallsBackToLatestForDev(t *testing.T) { + home := isolateDaemonFetchEnv(t) + + fx := newDaemonFetchFixture(t) + fx.LatestTag = "v9.9.9" + name, body := makeArchiveWithDaemon(t, true) + fx.Archives[name] = body + apiURL, relURL := fx.start(t) + pointURLsAt(t, apiURL, relURL) + + cliPath := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime") + _, err := EnsureDaemonBinary(context.Background(), cliPath, "dev") + require.NoError(t, err) + assert.Equal(t, int32(1), fx.apiHits.Load(), "expected one API hit to resolve latest tag") +} + +func TestEnsureDaemon_NetworkErrorReturnsHelpful(t *testing.T) { + home := isolateDaemonFetchEnv(t) + + // Servers immediately closed → connections fail. + apiSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + relSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + apiSrv.Close() + relSrv.Close() + pointURLsAt(t, apiSrv.URL, relSrv.URL) + + cliPath := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime") + _, err := EnsureDaemonBinary(context.Background(), cliPath, "v0.1.83") + require.Error(t, err) +} + +func TestEnsureDaemon_ArchiveMissingDaemonBinary(t *testing.T) { + home := isolateDaemonFetchEnv(t) + + fx := newDaemonFetchFixture(t) + name, body := makeArchiveWithDaemon(t, false) // CLI only, no daemon + fx.Archives[name] = body + apiURL, relURL := fx.start(t) + pointURLsAt(t, apiURL, relURL) + + cliPath := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime") + _, err := EnsureDaemonBinary(context.Background(), cliPath, "v0.1.83") + require.Error(t, err) + assert.Contains(t, err.Error(), "did not contain shelltime-daemon") +} + +func TestEnsureDaemon_CreatesBinDir(t *testing.T) { + home := isolateDaemonFetchEnv(t) + // Deliberately don't create ~/.shelltime/bin upfront. + _, err := os.Stat(filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin")) + require.True(t, os.IsNotExist(err)) + + fx := newDaemonFetchFixture(t) + fx.LatestTag = "v9.9.9" + name, body := makeArchiveWithDaemon(t, true) + fx.Archives[name] = body + apiURL, relURL := fx.start(t) + pointURLsAt(t, apiURL, relURL) + + cliPath := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime") + got, err := EnsureDaemonBinary(context.Background(), cliPath, "v0.1.83") + require.NoError(t, err) + _, err = os.Stat(got) + require.NoError(t, err) +} + +func TestEnsureDaemon_404FallsBackToLatest(t *testing.T) { + home := isolateDaemonFetchEnv(t) + + fx := newDaemonFetchFixture(t) + fx.LatestTag = "v9.9.9" + fx.NotFoundTag = "v0.1.83" // pretend the tagged release was yanked + name, body := makeArchiveWithDaemon(t, true) + fx.Archives[name] = body + apiURL, relURL := fx.start(t) + pointURLsAt(t, apiURL, relURL) + + cliPath := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime") + got, err := EnsureDaemonBinary(context.Background(), cliPath, "v0.1.83") + require.NoError(t, err) + expected := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime-daemon") + assert.Equal(t, expected, got) + assert.GreaterOrEqual(t, fx.hits404.Load(), int32(1), "expected at least one 404 before falling back") +} diff --git a/model/updater.go b/model/updater.go index e935a2d..cd3eab9 100644 --- a/model/updater.go +++ b/model/updater.go @@ -37,6 +37,13 @@ const ( updaterDownloadTimeout = 5 * time.Minute ) +// Base URLs for GitHub. Exposed as vars so tests can point them at an +// httptest.Server. +var ( + githubAPIBaseURL = "https://api.github.com" + githubReleaseBaseURL = "https://github.com" +) + // Binary names extracted from release archives. var allowedArchiveBinaries = map[string]bool{ "shelltime": true, @@ -67,7 +74,7 @@ func updaterUserAgent() string { // FetchLatestVersion calls the GitHub API for the latest stable release tag. func FetchLatestVersion(ctx context.Context) (string, error) { - url := fmt.Sprintf("https://api.github.com/repos/%s/%s/releases/latest", githubReleasesOwner, githubReleasesRepo) + url := fmt.Sprintf("%s/repos/%s/%s/releases/latest", githubAPIBaseURL, githubReleasesOwner, githubReleasesRepo) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { @@ -135,16 +142,16 @@ func BuildArchiveName(goos, goarch string) (string, error) { // BuildDownloadURL returns the direct release-asset URL for a specific tag. func BuildDownloadURL(tag, archiveName string) string { return fmt.Sprintf( - "https://github.com/%s/%s/releases/download/%s/%s", - githubReleasesOwner, githubReleasesRepo, tag, archiveName, + "%s/%s/%s/releases/download/%s/%s", + githubReleaseBaseURL, githubReleasesOwner, githubReleasesRepo, tag, archiveName, ) } // BuildChecksumsURL returns the checksums.txt URL for a specific tag. func BuildChecksumsURL(tag string) string { return fmt.Sprintf( - "https://github.com/%s/%s/releases/download/%s/checksums.txt", - githubReleasesOwner, githubReleasesRepo, tag, + "%s/%s/%s/releases/download/%s/checksums.txt", + githubReleaseBaseURL, githubReleasesOwner, githubReleasesRepo, tag, ) } From ca9a087d1063dd43918b4652ec0d42348ee47967 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 20 May 2026 02:58:00 +0000 Subject: [PATCH 2/2] fix(daemon): propagate FetchChecksum errors during auto-download MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously fetchDaemonToCurlPath discarded the FetchChecksum error, treating any failure the same as a legitimately-absent checksum (404 / no entry for this archive). That meant a transient 5xx from the GitHub CDN — or an attacker blocking/tampering with checksums.txt — could silently downgrade the install to an unverified download. Now only ("", false, nil) is treated as absent. Real errors propagate out of EnsureDaemonBinary, the temp dir is cleaned via defer, and no daemon binary is written to ~/.shelltime/bin/. https://claude.ai/code/session_01ThKSJA9sGXQm8jMeCZZdYY --- model/daemon_fetch.go | 11 ++++++++--- model/daemon_fetch_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/model/daemon_fetch.go b/model/daemon_fetch.go index d583ad2..c718d8d 100644 --- a/model/daemon_fetch.go +++ b/model/daemon_fetch.go @@ -71,9 +71,14 @@ func EnsureDaemonBinary(ctx context.Context, cliBinPath, cliVersion string) (str func fetchDaemonToCurlPath(ctx context.Context, tag, archiveName string) (string, error) { downloadURL := BuildDownloadURL(tag, archiveName) - sum, _, _ := FetchChecksum(ctx, tag, archiveName) - // FetchChecksum errors and absent checksums are both non-fatal; we proceed - // without verification, matching the `shelltime update` behavior. + sum, _, err := FetchChecksum(ctx, tag, archiveName) + if err != nil { + return "", fmt.Errorf("fetch checksum: %w", err) + } + // Empty sum (404 / no entry for this archive) is legitimately absent — + // proceed without verification. Non-nil errors (5xx, network, MITM) MUST + // propagate so an attacker can't silently downgrade us to an unverified + // download. tmpDir, err := os.MkdirTemp("", "shelltime-daemon-fetch-*") if err != nil { diff --git a/model/daemon_fetch_test.go b/model/daemon_fetch_test.go index fa5e48d..4d47187 100644 --- a/model/daemon_fetch_test.go +++ b/model/daemon_fetch_test.go @@ -282,6 +282,42 @@ func TestEnsureDaemon_UsesCliVersionWhenSet(t *testing.T) { assert.Equal(t, "v0.94.5", seenTag.Load()) } +func TestEnsureDaemon_ChecksumServer5xxAborts(t *testing.T) { + home := isolateDaemonFetchEnv(t) + + name, body := makeArchiveWithDaemon(t, true) + apiSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + t.Cleanup(apiSrv.Close) + relSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + file := parts[len(parts)-1] + if file == "checksums.txt" { + // Simulate a transient 5xx (or a tampered/blocked checksum + // endpoint): the caller must NOT silently downgrade to an + // unverified download. + http.Error(w, "boom", http.StatusServiceUnavailable) + return + } + if file == name { + _, _ = w.Write(body) + return + } + http.NotFound(w, r) + })) + t.Cleanup(relSrv.Close) + pointURLsAt(t, apiSrv.URL, relSrv.URL) + + cliPath := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime") + _, err := EnsureDaemonBinary(context.Background(), cliPath, "v0.1.83") + require.Error(t, err) + assert.Contains(t, err.Error(), "fetch checksum") + // And the binary must not have been written. + _, statErr := os.Stat(filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime-daemon")) + assert.True(t, os.IsNotExist(statErr), "daemon binary should not be written on checksum failure") +} + func TestEnsureDaemon_PrefixesVForUnprefixedVersion(t *testing.T) { assert.Equal(t, "v0.94.5", normalizeDaemonTag("0.94.5")) assert.Equal(t, "v0.94.5", normalizeDaemonTag("v0.94.5"))