Storage fix-permissions should handle missing config

When a config file is not present in PORTER_HOME, we don't do a safe job of detecting
that and then skipping trying to set permission on it. So we end up
calling fixFile on an empty string, which changes the permissions on the current
working directory, removing the x bit...

I've fixed the command to check if the config file exists before trying to set it
and also added more safety checks in fixFile to prevent us from changing permissions on a
directory, or to a path outside of PORTER_HOME.

Signed-off-by: Carolyn Van Slyck <[email protected]>

pkg/porter/porter_integration_test.go

@@ -1,3 +1,4 @@
+//go:build integration
 // +build integration
 
 package porter
@@ -36,7 +37,29 @@ func TestPorter_FixPermissions(t *testing.T) {
 			err := p.FixPermissions()
 			require.NoError(t, err)
 
+			// Check that all files in the directory have the correct permissions
 			tests.AssertDirectoryPermissionsEqual(t, dir, 0600)
 		})
 	}
 }
+
+func TestPorter_FixPermissions_NoConfigFile(t *testing.T) {
+	p := NewTestPorter(t)
+	p.SetupIntegrationTest()
+	defer p.CleanupIntegrationTest()
+
+	// Remember the original permissions on the current working directory
+	wd := p.Getwd()
+	wdInfo, err := p.FileSystem.Stat(wd)
+	require.NoError(t, err, "stat on the current working directory failed")
+	wantMode := wdInfo.Mode()
+
+	err = p.FixPermissions()
+	require.NoError(t, err)
+
+	// Check that the current working directory didn't have its permissions changed
+	wdInfo, err = p.FileSystem.Stat(wd)
+	require.NoError(t, err, "stat on the current working directory failed")
+	gotMode := wdInfo.Mode()
+	tests.AssertFilePermissionsEqual(t, wd, wantMode, gotMode)
+}

pkg/porter/storage.go

@@ -25,7 +25,11 @@ func (p *Porter) MigrateStorage() error {
 }
 
 func (p *Porter) FixPermissions() error {
-	home, _ := p.GetHomeDir()
+	home, err := p.GetHomeDir()
+	if err != nil {
+		return err
+	}
+
 	fmt.Fprintf(p.Out, "Resetting file permissions in %s...\n", home)
 
 	// Fix as many files as we can, and then report any errors
@@ -39,6 +43,14 @@ func (p *Porter) FixPermissions() error {
 			}
 		}
 
+		if info.IsDir() {
+			return fmt.Errorf("fixFile was called on a directory %s", path)
+		}
+
+		if _, err = filepath.Rel(home, path); err != nil {
+			return fmt.Errorf("fixFile was called on a path, %s, that isn't in the PORTER_HOME directory %s", path, home)
+		}
+
 		gotPerms := info.Mode().Perm()
 		if mode != gotPerms|mode {
 			if err := p.FileSystem.Chmod(path, mode); err != nil {
@@ -73,7 +85,10 @@ func (p *Porter) FixPermissions() error {
 	}
 
 	var bigErr *multierror.Error
-	dataFiles := []string{p.ConfigFilePath, filepath.Join(home, "schema.json")}
+	dataFiles := []string{filepath.Join(home, "schema.json")}
+	if p.ConfigFilePath != "" {
+		dataFiles = append(dataFiles, p.ConfigFilePath)
+	}
 	for _, file := range dataFiles {
 		if err := fixFile(file, 0600); err != nil {
 			bigErr = multierror.Append(bigErr, err)

tests/helpers.go

@@ -13,7 +13,7 @@ import (
 func AssertFilePermissionsEqual(t *testing.T, path string, want os.FileMode, got os.FileMode) bool {
 	want = want.Perm()
 	got = got.Perm()
-	return assert.Equal(t, want, got|want,
+	return assert.Equal(t, want, got,
 		"incorrect file permissions on %s. want: %o, got %o", path, want, got)
 }