From eb92615825666918fa092dd6e50563281a362d44 Mon Sep 17 00:00:00 2001
From: Carolyn Van Slyck <me@carolynvanslyck.com>
Date: Thu, 9 Jun 2022 08:54:06 -0500
Subject: [PATCH] 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 <me@carolynvanslyck.com>
---
 pkg/porter/porter_integration_test.go | 23 +++++++++++++++++++++++
 pkg/porter/storage.go                 | 19 +++++++++++++++++--
 tests/helpers.go                      |  2 +-
 3 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/pkg/porter/porter_integration_test.go b/pkg/porter/porter_integration_test.go
index 20632fcbd..ff506c801 100644
--- a/pkg/porter/porter_integration_test.go
+++ b/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)
+}
diff --git a/pkg/porter/storage.go b/pkg/porter/storage.go
index 7b5a46971..50a46e9ad 100644
--- a/pkg/porter/storage.go
+++ b/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)
diff --git a/tests/helpers.go b/tests/helpers.go
index 4a450ea92..5a8e655ed 100644
--- a/tests/helpers.go
+++ b/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)
 }