cmd/dist: save and restore original permissions in makeGOROOTUnwritable
Also log a message and skip the Chmods if running as root.
Updates #30316
Change-Id: Ifb68d06ce845275a72d64c808407e8609df270bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/206757
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
index e0fa51f..84ad5fd 100644
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -192,8 +192,13 @@
}
// On a few builders, make GOROOT unwritable to catch tests writing to it.
+ restoreGOROOT := func() {}
if strings.HasPrefix(os.Getenv("GO_BUILDER_NAME"), "linux-") {
- t.makeGOROOTUnwritable()
+ if os.Getuid() == 0 {
+ log.Printf("Not making GOROOT unwritable: running as root, so permissions would have no effect.")
+ } else {
+ restoreGOROOT = t.makeGOROOTUnwritable()
+ }
}
for _, dt := range t.tests {
@@ -208,12 +213,15 @@
if t.keepGoing {
log.Printf("Failed: %v", err)
} else {
+ restoreGOROOT()
log.Fatalf("Failed: %v", err)
}
}
}
t.runPending(nil)
+ restoreGOROOT()
timelog("end", "dist test")
+
if t.failed {
fmt.Println("\nFAILED")
os.Exit(1)
@@ -1423,32 +1431,45 @@
// makeGOROOTUnwritable makes all $GOROOT files & directories non-writable to
// check that no tests accidentally write to $GOROOT.
-func (t *tester) makeGOROOTUnwritable() {
- if os.Getenv("GO_BUILDER_NAME") == "" {
- panic("not a builder")
- }
- if os.Getenv("GOROOT") == "" {
+func (t *tester) makeGOROOTUnwritable() (undo func()) {
+ dir := os.Getenv("GOROOT")
+ if dir == "" {
panic("GOROOT not set")
}
- err := filepath.Walk(os.Getenv("GOROOT"), func(name string, fi os.FileInfo, err error) error {
- if err != nil {
- return err
+
+ type pathMode struct {
+ path string
+ mode os.FileMode
+ }
+ var dirs []pathMode // in lexical order
+
+ undo = func() {
+ for i := range dirs {
+ os.Chmod(dirs[i].path, dirs[i].mode) // best effort
}
- if !fi.Mode().IsRegular() && !fi.IsDir() {
- return nil
- }
- mode := fi.Mode()
- newMode := mode & ^os.FileMode(0222)
- if newMode != mode {
- if err := os.Chmod(name, newMode); err != nil {
- return err
+ }
+
+ filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+ if err == nil {
+ mode := info.Mode()
+ if mode&0222 != 0 && (mode.IsDir() || mode.IsRegular()) {
+ dirs = append(dirs, pathMode{path, mode})
}
}
return nil
})
- if err != nil {
- log.Fatalf("making builder's files read-only: %v", err)
+
+ // Run over list backward to chmod children before parents.
+ for i := len(dirs) - 1; i >= 0; i-- {
+ err := os.Chmod(dirs[i].path, dirs[i].mode&^0222)
+ if err != nil {
+ dirs = dirs[i:] // Only undo what we did so far.
+ undo()
+ log.Fatalf("failed to make GOROOT read-only: %v", err)
+ }
}
+
+ return undo
}
// shouldUsePrecompiledStdTest reports whether "dist test" should use
