Skip to content

Commit b159aee

Browse files
authored
fix: prune fails to remove versions with read-only dirs (#226) (#227)
* fix: prune fails to remove versions containing read-only dirs (#226) os.RemoveAll returns 'permission denied' and leaves the directory in place when a version tree contains read-only directories (e.g. Go module-cache style trees with mode 0555). cleanVersionDir swallowed the error, so prune/uninstall reported success while every version stayed on disk and a second prune produced identical output. Add utils.RemoveAll, which makes directories writable before removing, return the error from cleanVersionDir, and report a real failure from Uninstall instead of a false success. * ci: pin windows build to go.mod toolchain version The windows build used actions/setup-go without a version, so it ran the runner's preinstalled Go (1.24.13) against go.mod's 'go 1.26.0' directive with GOTOOLCHAIN=local, failing with 'go.mod requires go >= 1.26.0'. Read the version from go.mod so the job tracks the module requirement.
1 parent a3cd541 commit b159aee

5 files changed

Lines changed: 57 additions & 4 deletions

File tree

.github/workflows/build-windows.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ jobs:
1616
steps:
1717
- uses: actions/checkout@v4
1818
- uses: actions/setup-go@v6
19+
with:
20+
go-version-file: go.mod
1921

2022
- name: Build
2123
run: go build ./cmd/gobrew

gobrew.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,10 @@ func (gb *GoBrew) Uninstall(version string) {
312312
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)
313313
os.Exit(1)
314314
}
315-
gb.cleanVersionDir(version)
315+
if err := gb.cleanVersionDir(version); err != nil {
316+
color.Errorf("==> [Error] Failed to uninstall version: %s: %s\n", version, err)
317+
return
318+
}
316319
color.Successf("==> [Success] Version: %s uninstalled\n", version)
317320
}
318321

gobrew_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,31 @@ func TestPrune(t *testing.T) {
154154
t.Log("test finished")
155155
}
156156

157+
// TestPruneReadOnlyDir reproduces issue #226: a read-only directory inside an
158+
// installed version (e.g. a Go module-cache style tree with mode 0555) made
159+
// os.RemoveAll fail with "permission denied", so prune reported success while
160+
// the version stayed on disk.
161+
func TestPruneReadOnlyDir(t *testing.T) {
162+
t.Parallel()
163+
ts := httptest.NewServer(http.FileServer(http.Dir("testdata")))
164+
defer ts.Close()
165+
gb := setupGobrew(t, ts)
166+
gb.Install("1.20")
167+
gb.Install("1.19")
168+
gb.Use("1.19")
169+
170+
// Inject a read-only directory holding a read-only file into version 1.20.
171+
roDir := filepath.Join(gb.getVersionDir("1.20"), "go", "readonly")
172+
assert.NoError(t, os.MkdirAll(roDir, 0o755))
173+
assert.NoError(t, os.WriteFile(filepath.Join(roDir, "f"), []byte("x"), 0o600))
174+
assert.NoError(t, os.Chmod(roDir, 0o555))
175+
176+
gb.Prune()
177+
assert.Equal(t, false, gb.existsVersion("1.20"))
178+
assert.Equal(t, true, gb.existsVersion("1.19"))
179+
t.Log("test finished")
180+
}
181+
157182
func TestGoBrew_CurrentVersion(t *testing.T) {
158183
t.Parallel()
159184
ts := httptest.NewServer(http.FileServer(http.Dir("testdata")))

helpers.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,8 @@ func (gb *GoBrew) existsVersion(version string) bool {
153153
return err == nil
154154
}
155155

156-
func (gb *GoBrew) cleanVersionDir(version string) {
157-
_ = os.RemoveAll(gb.getVersionDir(version))
156+
func (gb *GoBrew) cleanVersionDir(version string) error {
157+
return utils.RemoveAll(gb.getVersionDir(version))
158158
}
159159

160160
func (gb *GoBrew) cleanDownloadsDir() {
@@ -263,7 +263,7 @@ func (gb *GoBrew) downloadAndExtract(version string) {
263263
err = gb.extract(srcTar, dstDir)
264264
if err != nil {
265265
// clean up dir
266-
gb.cleanVersionDir(version)
266+
_ = gb.cleanVersionDir(version)
267267
color.Errorln("==> [Info] Extract failed:", err)
268268
os.Exit(1)
269269
}

utils/utils.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ package utils
33
import (
44
"fmt"
55
"io"
6+
"io/fs"
67
"net/http"
78
"os"
89
"path"
10+
"path/filepath"
911

1012
"github.com/gookit/color"
1113
"github.com/schollz/progressbar/v3"
@@ -52,6 +54,27 @@ func DownloadWithProgress(url string, tarName string, destFolder string) (err er
5254
return nil
5355
}
5456

57+
// RemoveAll removes path and all its children, making any read-only
58+
// directories writable first. A plain os.RemoveAll fails with
59+
// "permission denied" when the tree contains read-only directories
60+
// (e.g. Go module-cache style trees with mode 0555), leaving the
61+
// directory in place. Making directories writable before removal avoids
62+
// silently failing to delete an installed Go version.
63+
func RemoveAll(path string) error {
64+
_ = filepath.WalkDir(path, func(p string, d fs.DirEntry, err error) error {
65+
if err != nil {
66+
return nil
67+
}
68+
// WalkDir does not follow symlinks and d.IsDir() is false for
69+
// symlinked dirs, so p stays within gobrew's version tree.
70+
if d.IsDir() {
71+
_ = os.Chmod(p, 0o755) //nolint:gosec // path is confined to the walked tree
72+
}
73+
return nil
74+
})
75+
return os.RemoveAll(path)
76+
}
77+
5578
func CheckError(err error, format string) {
5679
if err != nil {
5780
color.Errorf(format+": %s", err)

0 commit comments

Comments
 (0)