Commit 2acffbeb by Marcus Efraimsson Committed by GitHub

CLI: Fix installing plugins on windows (#19061)

Fixes #19022
parent 7b7b9534
......@@ -199,14 +199,14 @@ func extractFiles(body []byte, pluginName string, filePath string, allowSymlinks
}
for _, zf := range r.File {
newFileName := RemoveGitBuildFromName(pluginName, zf.Name)
if !isPathSafe(newFileName, path.Join(filePath, pluginName)) {
if !isPathSafe(newFileName, filepath.Join(filePath, pluginName)) {
return xerrors.Errorf("filepath: %v tries to write outside of plugin directory: %v. This can be a security risk.", zf.Name, path.Join(filePath, pluginName))
}
newFile := path.Join(filePath, newFileName)
if zf.FileInfo().IsDir() {
err := os.Mkdir(newFile, 0755)
if permissionsError(err) {
if os.IsPermission(err) {
return fmt.Errorf(permissionsDeniedMessage, newFile)
}
} else {
......@@ -234,10 +234,6 @@ func extractFiles(body []byte, pluginName string, filePath string, allowSymlinks
return nil
}
func permissionsError(err error) bool {
return err != nil && strings.Contains(err.Error(), "permission denied")
}
func isSymlink(file *zip.File) bool {
return file.Mode()&os.ModeSymlink == os.ModeSymlink
}
......@@ -269,7 +265,7 @@ func extractFile(file *zip.File, filePath string) (err error) {
dst, err := os.OpenFile(filePath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fileMode)
if err != nil {
if permissionsError(err) {
if os.IsPermission(err) {
return xerrors.Errorf(permissionsDeniedMessage, filePath)
}
return errutil.Wrap("Failed to open file", err)
......
......@@ -48,6 +48,7 @@ func TestFoldernameReplacement(t *testing.T) {
func TestExtractFiles(t *testing.T) {
t.Run("Should preserve file permissions for plugin backend binaries for linux and darwin", func(t *testing.T) {
skipWindows(t)
pluginDir, del := setupFakePluginsDir(t)
defer del()
......@@ -95,6 +96,7 @@ func TestExtractFiles(t *testing.T) {
})
t.Run("Should extract symlinks if allowed", func(t *testing.T) {
skipWindows(t)
pluginDir, del := setupFakePluginsDir(t)
defer del()
......@@ -119,16 +121,18 @@ func TestInstallPluginCommand(t *testing.T) {
}
func TestIsPathSafe(t *testing.T) {
dest := fmt.Sprintf("%stest%spath", string(os.PathSeparator), string(os.PathSeparator))
t.Run("Should be true on nested destinations", func(t *testing.T) {
assert.True(t, isPathSafe("dest", "/test/path"))
assert.True(t, isPathSafe("dest/one", "/test/path"))
assert.True(t, isPathSafe("../path/dest/one", "/test/path"))
assert.True(t, isPathSafe("dest", dest))
assert.True(t, isPathSafe("dest/one", dest))
assert.True(t, isPathSafe("../path/dest/one", dest))
})
t.Run("Should be false on destinations outside of path", func(t *testing.T) {
assert.False(t, isPathSafe("../dest", "/test/path"))
assert.False(t, isPathSafe("../../", "/test/path"))
assert.False(t, isPathSafe("../../test", "/test/path"))
assert.False(t, isPathSafe("../dest", dest))
assert.False(t, isPathSafe("../../", dest))
assert.False(t, isPathSafe("../../test", dest))
})
}
......@@ -189,3 +193,9 @@ func setupFakePluginsDir(t *testing.T) (string, func()) {
assert.Nil(t, err)
}
}
func skipWindows(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Skipping test on Windows")
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment