Simplify isOrUnderDir function.

The previous implementation worked but was difficult to follow. The new version
is more explicit and easy to understand at a glance.

Signed-off-by: Erik Sipsma <[email protected]>

Author: sipsma

agent/drive_handler.go

@@ -184,12 +184,7 @@ func (dh driveHandler) MountDrive(ctx context.Context, req *drivemount.MountDriv
 	}
 
 	for _, systemDir := range bannedSystemDirs {
-		isUnder, err := isOrUnderDir(filepath.Clean(resolvedDest), filepath.Clean(systemDir))
-		if err != nil {
-			return nil, errors.Errorf("failed to check if %q is under dir %q", resolvedDest, systemDir)
-		}
-
-		if isUnder {
+		if isOrUnderDir(resolvedDest, systemDir) {
 			return nil, errors.Errorf(
 				"drive mount destination %q resolves to path %q under banned system directory %q",
 				req.DestinationPath, resolvedDest, systemDir,
@@ -256,11 +251,17 @@ func evalAnySymlinks(path string) (string, error) {
 }
 
 // returns whether the given path is the provided baseDir or is under it
-func isOrUnderDir(path, baseDir string) (bool, error) {
-	relPath, err := filepath.Rel(baseDir, path)
-	if err != nil {
-		return false, err
+func isOrUnderDir(path, baseDir string) bool {
+	path = filepath.Clean(path)
+	baseDir = filepath.Clean(baseDir)
+
+	if baseDir == "/" {
+		return true
+	}
+
+	if path == baseDir {
+		return true
 	}
 
-	return !strings.HasPrefix(relPath, "../") && relPath != "..", nil
+	return strings.HasPrefix(path, baseDir+"/")
 }

agent/drive_handler_test.go

@@ -63,131 +63,105 @@ func TestIsOrUnderDir(t *testing.T) {
 		baseDir      string
 		path         string
 		expectedTrue bool
-		expectedErr  bool
 	}
 
 	for _, tc := range []testcase{
 		{
 			baseDir:      "/foo",
 			path:         "/foo/bar",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo/bar",
 			path:         "/foo/bar/baz",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "/foo",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo/bar",
 			path:         "/foo/bar",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "/foobar",
 			expectedTrue: false,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "/bar",
 			expectedTrue: false,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo/bar",
 			path:         "/bar",
 			expectedTrue: false,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo/bar",
 			path:         "/foo",
 			expectedTrue: false,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo/bar",
 			path:         "/bar/bar",
 			expectedTrue: false,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "foo",
 			expectedTrue: false,
-			expectedErr:  true,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "bar",
 			expectedTrue: false,
-			expectedErr:  true,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "/foo/../foo",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo/bar",
 			path:         "/foo/../foo/bar",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "/foo/../bar",
 			expectedTrue: false,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "/foo/..bar",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "/foo/..bar/baz",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/",
 			path:         "/",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/foo",
 			path:         "/",
 			expectedTrue: false,
-			expectedErr:  false,
 		},
 		{
 			baseDir:      "/",
 			path:         "/foo",
 			expectedTrue: true,
-			expectedErr:  false,
 		},
 	} {
-		isUnder, err := isOrUnderDir(tc.path, tc.baseDir)
-		assert.Equalf(t, tc.expectedTrue, isUnder, "unexpected output for isOrUnderDir case %+v", tc)
-		if tc.expectedErr {
-			assert.Errorf(t, err, "unexpected error from isOrUnderDir case %+v", tc)
-		} else {
-			assert.NoErrorf(t, err, "unexpected error from isOrUnderDir case %+v", tc)
-		}
+		assert.Equalf(t, tc.expectedTrue, isOrUnderDir(tc.path, tc.baseDir), "unexpected output for isOrUnderDir case %+v", tc)
 	}
 }