Only remove symlinks to vendor dirs on non-Windows OSes

On Windows, we're unable to distinguish between symlinks and junctions. Deleting junctions is dangerous and appears to be able to delete the underlying folder, not just the junction that's pointing to a folder. That's unacceptably dangerous: a malicious repo could contain a junction named 'vendor' pointing at 'C:\\' and we'd delete all the user's data.
sdboyer · Apr 10, 2017 · 06bcb96 · 06bcb96
1 parent 6eda647
commit 06bcb96
Show file tree

Hide file tree

Showing 7 changed files with 421 additions and 225 deletions.
diff --git a/result.go b/result.go
@@ -72,34 +72,3 @@ func (r solution) Attempts() int {
 func (r solution) InputHash() []byte {
 	return r.hd
 }
-
-func stripVendor(path string, info os.FileInfo, err error) error {
-	if info.Name() == "vendor" {
-		if _, err := os.Lstat(path); err == nil {
-			symlink := (info.Mode() & os.ModeSymlink) != 0
-			dir := info.IsDir()
-
-			switch {
-			case symlink && dir:
-				// This must be a windows junction directory. Support for these in the
-				// standard library is spotty, and we could easily delete an important
-				// folder if we called os.Remove or os.RemoveAll. Just skip these.
-				return filepath.SkipDir
-
-			case symlink:
-				realInfo, err := os.Stat(path)
-				if err != nil {
-					return err
-				}
-				if realInfo.IsDir() {
-					return os.Remove(path)
-				}
-
-			case dir:
-				return removeAll(path)
-			}
-		}
-	}
-
-	return nil
-}
diff --git a/result_test.go b/result_test.go
@@ -140,197 +140,3 @@ func BenchmarkCreateVendorTree(b *testing.B) {
 	sm.Release()
 	os.RemoveAll(tmp) // comment this to leave temp dir behind for inspection
 }
-
-func TestStripVendor(t *testing.T) {
-	t.Run("vendor directory", stripVendorTestCase(fsTestCase{
-		before: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-				fsPath{"package", "vendor"},
-			},
-		},
-		after: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-			},
-		},
-	}))
-
-	t.Run("vendor file", stripVendorTestCase(fsTestCase{
-		before: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-			},
-			files: []fsPath{
-				fsPath{"package", "vendor"},
-			},
-		},
-		after: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-			},
-			files: []fsPath{
-				fsPath{"package", "vendor"},
-			},
-		},
-	}))
-
-	t.Run("vendor symlink", stripVendorTestCase(fsTestCase{
-		before: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-				fsPath{"package", "_vendor"},
-			},
-			links: []fsLink{
-				fsLink{
-					path: fsPath{"package", "vendor"},
-					to:   "_vendor",
-				},
-			},
-		},
-		after: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-				fsPath{"package", "_vendor"},
-			},
-		},
-	}))
-
-	t.Run("nonvendor symlink", stripVendorTestCase(fsTestCase{
-		before: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-				fsPath{"package", "_vendor"},
-			},
-			links: []fsLink{
-				fsLink{
-					path: fsPath{"package", "link"},
-					to:   "_vendor",
-				},
-			},
-		},
-		after: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-				fsPath{"package", "_vendor"},
-			},
-			links: []fsLink{
-				fsLink{
-					path: fsPath{"package", "link"},
-					to:   "_vendor",
-				},
-			},
-		},
-	}))
-
-	t.Run("vendor symlink to file", stripVendorTestCase(fsTestCase{
-		before: filesystemState{
-			files: []fsPath{
-				fsPath{"file"},
-			},
-			links: []fsLink{
-				fsLink{
-					path: fsPath{"vendor"},
-					to:   "file",
-				},
-			},
-		},
-		after: filesystemState{
-			files: []fsPath{
-				fsPath{"file"},
-			},
-			links: []fsLink{
-				fsLink{
-					path: fsPath{"vendor"},
-					to:   "file",
-				},
-			},
-		},
-	}))
-
-	t.Run("chained symlinks", stripVendorTestCase(fsTestCase{
-		before: filesystemState{
-			dirs: []fsPath{
-				fsPath{"_vendor"},
-			},
-			links: []fsLink{
-				fsLink{
-					path: fsPath{"vendor"},
-					to:   "vendor2",
-				},
-				fsLink{
-					path: fsPath{"vendor2"},
-					to:   "_vendor",
-				},
-			},
-		},
-		after: filesystemState{
-			dirs: []fsPath{
-				fsPath{"_vendor"},
-			},
-			links: []fsLink{
-				fsLink{
-					path: fsPath{"vendor2"},
-					to:   "_vendor",
-				},
-			},
-		},
-	}))
-
-	t.Run("circular symlinks", stripVendorTestCase(fsTestCase{
-		before: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-			},
-			links: []fsLink{
-				fsLink{
-					path: fsPath{"package", "link1"},
-					to:   "link2",
-				},
-				fsLink{
-					path: fsPath{"package", "link2"},
-					to:   "link1",
-				},
-			},
-		},
-		after: filesystemState{
-			dirs: []fsPath{
-				fsPath{"package"},
-			},
-			links: []fsLink{
-				fsLink{
-					path: fsPath{"package", "link1"},
-					to:   "link2",
-				},
-				fsLink{
-					path: fsPath{"package", "link2"},
-					to:   "link1",
-				},
-			},
-		},
-	}))
-}
-
-func stripVendorTestCase(tc fsTestCase) func(*testing.T) {
-	return func(t *testing.T) {
-		tempDir, err := ioutil.TempDir("", "TestStripVendor")
-		if err != nil {
-			t.Fatalf("ioutil.TempDir err=%q", err)
-		}
-		defer func() {
-			if err := os.RemoveAll(tempDir); err != nil {
-				t.Errorf("os.RemoveAll(%q) err=%q", tempDir, err)
-			}
-		}()
-		tc.before.root = tempDir
-		tc.after.root = tempDir
-
-		tc.before.setup(t)
-
-		if err := filepath.Walk(tempDir, stripVendor); err != nil {
-			t.Errorf("filepath.Walk err=%q", err)
-		}
-
-		tc.after.assert(t)
-	}
-}
diff --git a/strip_vendor.go b/strip_vendor.go
@@ -0,0 +1,26 @@
+//+build !windows
+
+package gps
+
+import "os"
+
+func stripVendor(path string, info os.FileInfo, err error) error {
+	if info.Name() == "vendor" {
+		if _, err := os.Lstat(path); err == nil {
+			if (info.Mode() & os.ModeSymlink) != 0 {
+				realInfo, err := os.Stat(path)
+				if err != nil {
+					return err
+				}
+				if realInfo.IsDir() {
+					return os.Remove(path)
+				}
+			}
+			if info.IsDir() {
+				return removeAll(path)
+			}
+		}
+	}
+
+	return nil
+}