disallow symlinks, add more tests

2 files changed, 63 insertions, 11 deletions

diff --git a/client/go/util/fix_fs.go b/client/go/util/fix_fs.go
index eaddedf4d00..a62b76bede5 100644
--- a/client/go/util/fix_fs.go
+++ b/client/go/util/fix_fs.go
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/vespa-engine/vespa/client/go/trace"
 )
@@ -27,19 +28,43 @@ func NewFixSpec() FixSpec {
 	}
 }
 
+func statNoSymlinks(path string) (info os.FileInfo, err error) {
+	components := strings.Split(path, "/")
+	var name string
+	for idx, x := range components {
+		if idx == 0 {
+			name = x
+			if x == "" {
+				continue
+			}
+		} else {
+			name = name + "/" + x
+		}
+		info, err = os.Lstat(name)
+		if err != nil {
+			return
+		}
+		if (info.Mode() & os.ModeSymlink) != 0 {
+			return nil, fmt.Errorf("the path '%s' is a symlink, not allowed", name)
+		}
+		trace.Debug("lstat", name, "=>", info.Mode(), err)
+	}
+	return info, err
+}
+
 // ensure directory exist with correct owner/permissions
 func (spec *FixSpec) FixDir(dirName string) {
-	info, err := os.Stat(dirName)
-	if err != nil {
+	info, err := statNoSymlinks(dirName)
+	if errors.Is(err, os.ErrNotExist) {
 		trace.Trace("mkdir: ", dirName)
 		err = os.MkdirAll(dirName, spec.DirMode)
 		if err != nil {
 			JustExitWith(err)
 		}
-		info, err = os.Stat(dirName)
-		if err != nil {
-			JustExitWith(err)
-		}
+		info, err = statNoSymlinks(dirName)
+	}
+	if err != nil {
+		JustExitWith(err)
 	}
 	if !info.IsDir() {
 		JustExitMsg(fmt.Sprintf("Not a directory: %s", dirName))
@@ -59,24 +84,24 @@ func (spec *FixSpec) FixDir(dirName string) {
 
 // ensure file gets correct owner/permissions if it exists
 func (spec *FixSpec) FixFile(fileName string) {
-	info, err := os.Stat(fileName)
+	info, err := statNoSymlinks(fileName)
 	if err != nil {
 		if !errors.Is(err, os.ErrNotExist) {
-			trace.Warning("stat error: ", err, "containing:", err.(*os.PathError).Unwrap())
+			trace.Warning("stat error: ", err)
 		}
 		return
 	}
 	if info.IsDir() {
-		panic(fmt.Errorf("Should not be a directory: %s", fileName))
+		JustExitMsg("Should not be a directory: " + fileName)
 	}
 	trace.Debug("chown: ", fileName, spec.UserId, spec.GroupId)
 	err = os.Chown(fileName, spec.UserId, spec.GroupId)
 	if err != nil {
-		panic(err)
+		JustExitWith(err)
 	}
 	trace.Debug("chmod: ", fileName, spec.FileMode)
 	err = os.Chmod(fileName, spec.FileMode)
 	if err != nil {
-		panic(err)
+		JustExitWith(err)
 	}
 }
diff --git a/client/go/util/fix_fs_test.go b/client/go/util/fix_fs_test.go
index 4b63ff7cb51..637aab59e9a 100644
--- a/client/go/util/fix_fs_test.go
+++ b/client/go/util/fix_fs_test.go
@@ -2,6 +2,7 @@
 package util
 
 import (
+	"fmt"
 	"os"
 	"os/user"
 	"strconv"
@@ -113,3 +114,29 @@ func TestSuperUserOnly(t *testing.T) {
 	}
 	testFixSpec(t, fixSpec)
 }
+
+func expectSimplePanic() {
+	if r := recover(); r != nil {
+		if jee, ok := r.(*JustExitError); ok {
+			fmt.Fprintln(os.Stderr, "got as expected:", jee)
+			return
+		}
+		panic(r)
+	}
+}
+
+func TestFailedFixdir(t *testing.T) {
+	tmpDir := setup(t)
+	spec := NewFixSpec()
+	defer expectSimplePanic()
+	spec.FixDir(tmpDir + "/a/bad/subdir")
+	assert.Equal(t, "", "should not be reached")
+}
+
+func TestFailedFixfile(t *testing.T) {
+	tmpDir := setup(t)
+	spec := NewFixSpec()
+	defer expectSimplePanic()
+	spec.FixFile(tmpDir + "/a")
+	assert.Equal(t, "", "should not be reached")
+}