diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index 462cb00..bf180cf 100644 --- a/.github/workflows/build-windows.yml +++ b/.github/workflows/build-windows.yml @@ -16,6 +16,8 @@ jobs: steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v6 + with: + go-version-file: go.mod - name: Build run: go build ./cmd/gobrew diff --git a/gobrew.go b/gobrew.go index 7950c00..e7d27d5 100644 --- a/gobrew.go +++ b/gobrew.go @@ -312,7 +312,10 @@ func (gb *GoBrew) Uninstall(version string) { color.Errorf("[Error] Version: %s you are trying to remove is your current version. Please use a different version first before uninstalling the current version\n", version) os.Exit(1) } - gb.cleanVersionDir(version) + if err := gb.cleanVersionDir(version); err != nil { + color.Errorf("==> [Error] Failed to uninstall version: %s: %s\n", version, err) + return + } color.Successf("==> [Success] Version: %s uninstalled\n", version) } diff --git a/gobrew_test.go b/gobrew_test.go index 7018c00..ddeb9f3 100644 --- a/gobrew_test.go +++ b/gobrew_test.go @@ -154,6 +154,31 @@ func TestPrune(t *testing.T) { t.Log("test finished") } +// TestPruneReadOnlyDir reproduces issue #226: a read-only directory inside an +// installed version (e.g. a Go module-cache style tree with mode 0555) made +// os.RemoveAll fail with "permission denied", so prune reported success while +// the version stayed on disk. +func TestPruneReadOnlyDir(t *testing.T) { + t.Parallel() + ts := httptest.NewServer(http.FileServer(http.Dir("testdata"))) + defer ts.Close() + gb := setupGobrew(t, ts) + gb.Install("1.20") + gb.Install("1.19") + gb.Use("1.19") + + // Inject a read-only directory holding a read-only file into version 1.20. + roDir := filepath.Join(gb.getVersionDir("1.20"), "go", "readonly") + assert.NoError(t, os.MkdirAll(roDir, 0o755)) + assert.NoError(t, os.WriteFile(filepath.Join(roDir, "f"), []byte("x"), 0o600)) + assert.NoError(t, os.Chmod(roDir, 0o555)) + + gb.Prune() + assert.Equal(t, false, gb.existsVersion("1.20")) + assert.Equal(t, true, gb.existsVersion("1.19")) + t.Log("test finished") +} + func TestGoBrew_CurrentVersion(t *testing.T) { t.Parallel() ts := httptest.NewServer(http.FileServer(http.Dir("testdata"))) diff --git a/helpers.go b/helpers.go index a258618..3198feb 100644 --- a/helpers.go +++ b/helpers.go @@ -153,8 +153,8 @@ func (gb *GoBrew) existsVersion(version string) bool { return err == nil } -func (gb *GoBrew) cleanVersionDir(version string) { - _ = os.RemoveAll(gb.getVersionDir(version)) +func (gb *GoBrew) cleanVersionDir(version string) error { + return utils.RemoveAll(gb.getVersionDir(version)) } func (gb *GoBrew) cleanDownloadsDir() { @@ -263,7 +263,7 @@ func (gb *GoBrew) downloadAndExtract(version string) { err = gb.extract(srcTar, dstDir) if err != nil { // clean up dir - gb.cleanVersionDir(version) + _ = gb.cleanVersionDir(version) color.Errorln("==> [Info] Extract failed:", err) os.Exit(1) } diff --git a/utils/utils.go b/utils/utils.go index 506a6ee..0c3db4a 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -3,9 +3,11 @@ package utils import ( "fmt" "io" + "io/fs" "net/http" "os" "path" + "path/filepath" "github.com/gookit/color" "github.com/schollz/progressbar/v3" @@ -52,6 +54,27 @@ func DownloadWithProgress(url string, tarName string, destFolder string) (err er return nil } +// RemoveAll removes path and all its children, making any read-only +// directories writable first. A plain os.RemoveAll fails with +// "permission denied" when the tree contains read-only directories +// (e.g. Go module-cache style trees with mode 0555), leaving the +// directory in place. Making directories writable before removal avoids +// silently failing to delete an installed Go version. +func RemoveAll(path string) error { + _ = filepath.WalkDir(path, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return nil + } + // WalkDir does not follow symlinks and d.IsDir() is false for + // symlinked dirs, so p stays within gobrew's version tree. + if d.IsDir() { + _ = os.Chmod(p, 0o755) //nolint:gosec // path is confined to the walked tree + } + return nil + }) + return os.RemoveAll(path) +} + func CheckError(err error, format string) { if err != nil { color.Errorf(format+": %s", err)