feat: allow CMP plugins to preserve repo files mode (argoproj#12940)

* feat: allow CMP plugins to preserve repo files mode

Signed-off-by: Alexander Matyushentsev <[email protected]>

* implement missing e2e test

Signed-off-by: Alexander Matyushentsev <[email protected]>

---------

Signed-off-by: Alexander Matyushentsev <[email protected]>

cmpserver/plugin/config.go

@@ -22,11 +22,12 @@ type PluginConfig struct {
 }
 
 type PluginConfigSpec struct {
-	Version    string     `json:"version"`
-	Init       Command    `json:"init,omitempty"`
-	Generate   Command    `json:"generate"`
-	Discover   Discover   `json:"discover"`
-	Parameters Parameters `yaml:"parameters"`
+	Version          string     `json:"version"`
+	Init             Command    `json:"init,omitempty"`
+	Generate         Command    `json:"generate"`
+	Discover         Discover   `json:"discover"`
+	Parameters       Parameters `yaml:"parameters"`
+	PreserveFileMode bool       `json:"preserveFileMode,omitempty"`
 }
 
 // Discover holds find and fileName

cmpserver/plugin/plugin.go

@@ -113,7 +113,7 @@ func runCommand(ctx context.Context, command Command, path string, env []string)
 	}
 	if len(output) == 0 {
 		log.WithFields(log.Fields{
-			"stderr": stderr,
+			"stderr":  stderr,
 			"command": command,
 		}).Warn("Plugin command returned zero output")
 	}
@@ -213,7 +213,7 @@ func (s *Service) generateManifestGeneric(stream GenerateManifestStream) error {
 	}
 	defer cleanup()
 
-	metadata, err := cmp.ReceiveRepoStream(ctx, stream, workDir)
+	metadata, err := cmp.ReceiveRepoStream(ctx, stream, workDir, s.initConstants.PluginConfig.Spec.PreserveFileMode)
 	if err != nil {
 		return fmt.Errorf("generate manifest error receiving stream: %w", err)
 	}
@@ -297,7 +297,7 @@ func (s *Service) matchRepositoryGeneric(stream MatchRepositoryStream) error {
 	}
 	defer cleanup()
 
-	metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir)
+	metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir, s.initConstants.PluginConfig.Spec.PreserveFileMode)
 	if err != nil {
 		return fmt.Errorf("match repository error receiving stream: %w", err)
 	}
@@ -375,7 +375,7 @@ func (s *Service) GetParametersAnnouncement(stream apiclient.ConfigManagementPlu
 	}
 	defer cleanup()
 
-	metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir)
+	metadata, err := cmp.ReceiveRepoStream(bufferedCtx, stream, workDir, s.initConstants.PluginConfig.Spec.PreserveFileMode)
 	if err != nil {
 		return fmt.Errorf("parameters announcement error receiving stream: %w", err)
 	}

docs/operator-manual/config-management-plugins.md

@@ -117,6 +117,10 @@ spec:
       # The command is run in an Application's source directory. Standard output must be JSON matching the schema of the
       # static parameter announcements list.
       command: [echo, '[{"name": "example-param", "string": "default-string-value"}]']
+
+    # If set to then the plugin receives repository files with original file mode. Dangerous since the repository
+    # might have executable files. Set to true only if you trust the CMP plugin authors.
+    preserveFileMode: false
 ```
 
 !!! note
@@ -484,3 +488,29 @@ After installing the plugin as a sidecar [according to the directions above](#in
 test it out on a few Applications before migrating all of them to the sidecar plugin.
 
 Once tests have checked out, remove the plugin entry from your argocd-cm ConfigMap.
+
+### Additional Settings
+
+#### Preserve repository files mode
+
+By default, config management plugin receives source repository files with reset file mode. This is done for security
+reasons. If you want to preserve original file mode, you can set `preserveFileMode` to `true` in the plugin spec:
+
+!!! warning
+    Make sure you trust the plugin you are using. If you set `preserveFileMode` to `true` then the plugin might receive
+    files with executable permissions which can be a security risk.
+
+```yaml
+apiVersion: argoproj.io/v1alpha1
+kind: ConfigManagementPlugin
+metadata:
+  name: pluginName
+spec:
+  init:
+    command: ["sample command"]
+    args: ["sample args"]
+  generate:
+    command: ["sample command"]
+    args: ["sample args"]
+  preserveFileMode: true
+```

test/e2e/custom_tool_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/argoproj/gitops-engine/pkg/health"
 	. "github.com/argoproj/gitops-engine/pkg/sync/common"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	. "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	. "github.com/argoproj/argo-cd/v2/test/e2e/fixture"
@@ -156,7 +157,7 @@ func TestCustomToolWithEnv(t *testing.T) {
 		})
 }
 
-//make sure we can sync and diff with --local
+// make sure we can sync and diff with --local
 func TestCustomToolSyncAndDiffLocal(t *testing.T) {
 	ctx := Given(t)
 	ctx.
@@ -203,7 +204,7 @@ func startCMPServer(configFile string) {
 	FailOnErr(RunWithStdin("", "", "../../dist/argocd", "--config-dir-path", configFile))
 }
 
-//Discover by fileName
+// Discover by fileName
 func TestCMPDiscoverWithFileName(t *testing.T) {
 	pluginName := "cmp-fileName"
 	Given(t).
@@ -222,7 +223,7 @@ func TestCMPDiscoverWithFileName(t *testing.T) {
 		Expect(HealthIs(health.HealthStatusHealthy))
 }
 
-//Discover by Find glob
+// Discover by Find glob
 func TestCMPDiscoverWithFindGlob(t *testing.T) {
 	Given(t).
 		And(func() {
@@ -240,7 +241,7 @@ func TestCMPDiscoverWithFindGlob(t *testing.T) {
 		Expect(HealthIs(health.HealthStatusHealthy))
 }
 
-//Discover by Plugin Name
+// Discover by Plugin Name
 func TestCMPDiscoverWithPluginName(t *testing.T) {
 	Given(t).
 		And(func() {
@@ -261,7 +262,7 @@ func TestCMPDiscoverWithPluginName(t *testing.T) {
 		Expect(HealthIs(health.HealthStatusHealthy))
 }
 
-//Discover by Find command
+// Discover by Find command
 func TestCMPDiscoverWithFindCommandWithEnv(t *testing.T) {
 	pluginName := "cmp-find-command"
 	ctx := Given(t)
@@ -329,3 +330,23 @@ func TestPruneResourceFromCMP(t *testing.T) {
 			assert.Error(t, err)
 		})
 }
+
+func TestPreserveFileModeForCMP(t *testing.T) {
+	Given(t).
+		And(func() {
+			go startCMPServer("./testdata/cmp-preserve-file-mode")
+			time.Sleep(1 * time.Second)
+			os.Setenv("ARGOCD_BINARY_NAME", "argocd")
+		}).
+		Path("cmp-preserve-file-mode").
+		When().
+		CreateFromFile(func(app *Application) {
+			app.Spec.Source.Plugin = &ApplicationSourcePlugin{Name: "cmp-preserve-file-mode-v1.0"}
+		}).
+		Refresh(RefreshTypeNormal).
+		Then().
+		And(func(app *Application) {
+			require.Len(t, app.Status.Resources, 1)
+			assert.Equal(t, "ConfigMap", app.Status.Resources[0].Kind)
+		})
+}

test/e2e/testdata/cmp-preserve-file-mode/plugin.yaml

@@ -0,0 +1,11 @@
+apiVersion: argoproj.io/v1alpha1
+kind: ConfigManagementPlugin
+metadata:
+  name: cmp-preserve-file-mode
+spec:
+  version: v1.0
+  generate:
+    command: [sh, -c , "./script.sh"]
+  discovery:
+  - glob: "**/*"
+  preserveFileMode: true

test/e2e/testdata/cmp-preserve-file-mode/script.sh

@@ -0,0 +1,8 @@
+cat << EOF
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: test-cm
+data:
+  foo: bar
+EOF

util/cmp/stream.go

@@ -12,11 +12,12 @@ import (
 	"path/filepath"
 	"strings"
 
+	log "github.com/sirupsen/logrus"
+
 	pluginclient "github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
 	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/util/io/files"
 	"github.com/argoproj/argo-cd/v2/util/tgzstream"
-	log "github.com/sirupsen/logrus"
 )
 
 // StreamSender defines the contract to send App files over stream
@@ -33,7 +34,7 @@ type StreamReceiver interface {
 // ReceiveRepoStream will receive the repository files and save them
 // in destDir. Will return the stream metadata if no error. Metadata
 // will be nil in case of errors.
-func ReceiveRepoStream(ctx context.Context, receiver StreamReceiver, destDir string) (*pluginclient.ManifestRequestMetadata, error) {
+func ReceiveRepoStream(ctx context.Context, receiver StreamReceiver, destDir string, preserveFileMode bool) (*pluginclient.ManifestRequestMetadata, error) {
 	header, err := receiver.Recv()
 	if err != nil {
 		return nil, fmt.Errorf("error receiving stream header: %w", err)
@@ -47,7 +48,7 @@ func ReceiveRepoStream(ctx context.Context, receiver StreamReceiver, destDir str
 	if err != nil {
 		return nil, fmt.Errorf("error receiving tgz file: %w", err)
 	}
-	err = files.Untgz(destDir, tgzFile, math.MaxInt64)
+	err = files.Untgz(destDir, tgzFile, math.MaxInt64, preserveFileMode)
 	if err != nil {
 		return nil, fmt.Errorf("error decompressing tgz file: %w", err)
 	}

util/cmp/stream_test.go

@@ -62,7 +62,7 @@ func TestReceiveApplicationStream(t *testing.T) {
 		go streamMock.sendFile(context.Background(), t, appDir, streamMock, []string{"env1", "env2"}, []string{"DUMMY.md", "dum*"})
 
 		// when
-		env, err := cmp.ReceiveRepoStream(context.Background(), streamMock, workdir)
+		env, err := cmp.ReceiveRepoStream(context.Background(), streamMock, workdir, false)
 
 		// then
 		require.NoError(t, err)

util/io/files/tar.go

@@ -58,7 +58,7 @@ func Tgz(srcPath string, inclusions []string, exclusions []string, writers ...io
 //   - a full path
 //   - points to an empty directory or
 //   - points to a non existing directory
-func Untgz(dstPath string, r io.Reader, maxSize int64) error {
+func Untgz(dstPath string, r io.Reader, maxSize int64, preserveFileMode bool) error {
 	if !filepath.IsAbs(dstPath) {
 		return fmt.Errorf("dstPath points to a relative path: %s", dstPath)
 	}
@@ -90,9 +90,14 @@ func Untgz(dstPath string, r io.Reader, maxSize int64) error {
 			return fmt.Errorf("illegal filepath in archive: %s", target)
 		}
 
+		var mode os.FileMode = 0755
+		if preserveFileMode {
+			mode = os.FileMode(header.Mode)
+		}
+
 		switch header.Typeflag {
 		case tar.TypeDir:
-			err := os.MkdirAll(target, 0755)
+			err := os.MkdirAll(target, mode)
 			if err != nil {
 				return fmt.Errorf("error creating nested folders: %w", err)
 			}
@@ -118,7 +123,7 @@ func Untgz(dstPath string, r io.Reader, maxSize int64) error {
 				return fmt.Errorf("error creating nested folders: %w", err)
 			}
 
-			f, err := os.Create(target)
+			f, err := os.OpenFile(target, os.O_RDWR|os.O_CREATE|os.O_TRUNC, mode)
 			if err != nil {
 				return fmt.Errorf("error creating file %q: %w", target, err)
 			}

util/io/files/tar_test.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"math"
 	"os"
+	"path"
 	"path/filepath"
 	"testing"
 
@@ -168,7 +169,7 @@ func TestUntgz(t *testing.T) {
 		destDir := filepath.Join(tmpDir, "untgz1")
 
 		// when
-		err := files.Untgz(destDir, tgzFile, math.MaxInt64)
+		err := files.Untgz(destDir, tgzFile, math.MaxInt64, false)
 
 		// then
 		require.NoError(t, err)
@@ -191,12 +192,32 @@ func TestUntgz(t *testing.T) {
 		destDir := filepath.Join(tmpDir, "untgz2")
 
 		// when
-		err := files.Untgz(destDir, tgzFile, math.MaxInt64)
+		err := files.Untgz(destDir, tgzFile, math.MaxInt64, false)
 
 		// then
 		assert.Error(t, err)
 		assert.Contains(t, err.Error(), "illegal filepath in symlink")
 	})
+
+	t.Run("preserves file mode", func(t *testing.T) {
+		// given
+		tmpDir := createTmpDir(t)
+		defer deleteTmpDir(t, tmpDir)
+		tgzFile := createTgz(t, filepath.Join(getTestDataDir(t), "executable"), tmpDir)
+		defer tgzFile.Close()
+
+		destDir := filepath.Join(tmpDir, "untgz1")
+
+		// when
+		err := files.Untgz(destDir, tgzFile, math.MaxInt64, false)
+		require.NoError(t, err)
+
+		// then
+
+		scriptFileInfo, err := os.Stat(path.Join(destDir, "script.sh"))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0755), scriptFileInfo.Mode())
+	})
 }
 
 // read returns a map with the filename as key. In case

util/io/files/testdata/executable/script.sh

@@ -0,0 +1 @@
+echo hello world

util/manifeststream/stream.go

@@ -9,11 +9,12 @@ import (
 	"io"
 	"os"
 
+	log "github.com/sirupsen/logrus"
+
 	applicationpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
 	"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
 	"github.com/argoproj/argo-cd/v2/util/io/files"
 	"github.com/argoproj/argo-cd/v2/util/tgzstream"
-	log "github.com/sirupsen/logrus"
 )
 
 // Defines the contract for the application sender, i.e. the CLI
@@ -183,7 +184,7 @@ func ReceiveManifestFileStream(ctx context.Context, receiver RepoStreamReceiver,
 	if err != nil {
 		return nil, nil, fmt.Errorf("error receiving tgz file: %w", err)
 	}
-	err = files.Untgz(destDir, tgzFile, maxExtractedSize)
+	err = files.Untgz(destDir, tgzFile, maxExtractedSize, false)
 	if err != nil {
 		return nil, nil, fmt.Errorf("error decompressing tgz file: %w", err)
 	}