diff --git a/cmpserver/plugin/plugin.go b/cmpserver/plugin/plugin.go index 08be235315dde..ca67ccecf214a 100644 --- a/cmpserver/plugin/plugin.go +++ b/cmpserver/plugin/plugin.go @@ -24,6 +24,7 @@ import ( "github.com/argoproj/argo-cd/v2/util/io/files" "github.com/argoproj/gitops-engine/pkg/utils/kube" + "github.com/cyphar/filepath-securejoin" "github.com/mattn/go-zglob" log "github.com/sirupsen/logrus" ) @@ -182,7 +183,7 @@ func getTempDirMustCleanup(baseDir string) (workDir string, cleanup func(), err if err := os.RemoveAll(workDir); err != nil { log.WithFields(map[string]interface{}{ common.SecurityField: common.SecurityHigh, - common.SecurityCWEField: 459, + common.SecurityCWEField: common.SecurityCWEIncompleteCleanup, }).Errorf("Failed to clean up temp directory: %s", err) } } @@ -302,7 +303,7 @@ func (s *Service) matchRepositoryGeneric(stream MatchRepositoryStream) error { return fmt.Errorf("match repository error receiving stream: %w", err) } - isSupported, isDiscoveryEnabled, err := s.matchRepository(bufferedCtx, workDir, metadata.GetEnv()) + isSupported, isDiscoveryEnabled, err := s.matchRepository(bufferedCtx, workDir, metadata.GetEnv(), metadata.GetAppRelPath()) if err != nil { return fmt.Errorf("match repository error: %w", err) } @@ -315,12 +316,20 @@ func (s *Service) matchRepositoryGeneric(stream MatchRepositoryStream) error { return nil } -func (s *Service) matchRepository(ctx context.Context, workdir string, envEntries []*apiclient.EnvEntry) (isSupported bool, isDiscoveryEnabled bool, err error) { +func (s *Service) matchRepository(ctx context.Context, workdir string, envEntries []*apiclient.EnvEntry, appRelPath string) (isSupported bool, isDiscoveryEnabled bool, err error) { config := s.initConstants.PluginConfig + appPath, err := securejoin.SecureJoin(workdir, appRelPath) + if err != nil { + log.WithFields(map[string]interface{}{ + common.SecurityField: common.SecurityHigh, + common.SecurityCWEField: common.SecurityCWEIncompleteCleanup, + }).Errorf("error joining workdir %q and appRelPath %q: %v", workdir, appRelPath, err) + } + if config.Spec.Discover.FileName != "" { log.Debugf("config.Spec.Discover.FileName is provided") - pattern := filepath.Join(workdir, config.Spec.Discover.FileName) + pattern := filepath.Join(appPath, config.Spec.Discover.FileName) matches, err := filepath.Glob(pattern) if err != nil { e := fmt.Errorf("error finding filename match for pattern %q: %w", pattern, err) @@ -332,7 +341,7 @@ func (s *Service) matchRepository(ctx context.Context, workdir string, envEntrie if config.Spec.Discover.Find.Glob != "" { log.Debugf("config.Spec.Discover.Find.Glob is provided") - pattern := filepath.Join(workdir, config.Spec.Discover.Find.Glob) + pattern := filepath.Join(appPath, config.Spec.Discover.Find.Glob) // filepath.Glob doesn't have '**' support hence selecting third-party lib // https://github.com/golang/go/issues/11862 matches, err := zglob.Glob(pattern) @@ -348,7 +357,7 @@ func (s *Service) matchRepository(ctx context.Context, workdir string, envEntrie if len(config.Spec.Discover.Find.Command.Command) > 0 { log.Debugf("Going to try runCommand.") env := append(os.Environ(), environ(envEntries)...) - find, err := runCommand(ctx, config.Spec.Discover.Find.Command, workdir, env) + find, err := runCommand(ctx, config.Spec.Discover.Find.Command, appPath, env) if err != nil { return false, true, fmt.Errorf("error running find command: %w", err) } diff --git a/cmpserver/plugin/plugin_test.go b/cmpserver/plugin/plugin_test.go index 3096e6736bc23..936a38caba934 100644 --- a/cmpserver/plugin/plugin_test.go +++ b/cmpserver/plugin/plugin_test.go @@ -100,7 +100,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.NoError(t, err) @@ -115,7 +115,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.NoError(t, err) @@ -130,7 +130,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - _, _, err := f.service.matchRepository(context.Background(), f.path, f.env) + _, _, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.ErrorContains(t, err, "syntax error") @@ -145,7 +145,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.NoError(t, err) @@ -162,7 +162,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.NoError(t, err) @@ -179,7 +179,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - _, _, err := f.service.matchRepository(context.Background(), f.path, f.env) + _, _, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.ErrorContains(t, err, "error finding glob match for pattern") @@ -196,7 +196,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.NoError(t, err) @@ -215,7 +215,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.NoError(t, err) assert.False(t, match) @@ -233,7 +233,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.NoError(t, err) @@ -253,7 +253,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.NoError(t, err) @@ -272,7 +272,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.Error(t, err) @@ -285,7 +285,7 @@ func TestMatchRepository(t *testing.T) { f := setup(t, withDiscover(d)) // when - match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env) + match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".") // then assert.NoError(t, err) diff --git a/common/common.go b/common/common.go index 0a13e4ccb8f89..1c01710539472 100644 --- a/common/common.go +++ b/common/common.go @@ -309,13 +309,16 @@ const ( // Security severity logging const ( - SecurityField = "security" - SecurityCWEField = "CWE" - SecurityEmergency = 5 // Indicates unmistakably malicious events that should NEVER occur accidentally and indicates an active attack (i.e. brute forcing, DoS) - SecurityCritical = 4 // Indicates any malicious or exploitable event that had a side effect (i.e. secrets being left behind on the filesystem) - SecurityHigh = 3 // Indicates likely malicious events but one that had no side effects or was blocked (i.e. out of bounds symlinks in repos) - SecurityMedium = 2 // Could indicate malicious events, but has a high likelihood of being user/system error (i.e. access denied) - SecurityLow = 1 // Unexceptional entries (i.e. successful access logs) + SecurityField = "security" + // SecurityCWEField is the logs field for the CWE associated with a log line. CWE stands for Common Weakness Enumeration. See https://cwe.mitre.org/ + SecurityCWEField = "CWE" + SecurityCWEIncompleteCleanup = 459 + SecurityCWEMissingReleaseOfFileDescriptor = 775 + SecurityEmergency = 5 // Indicates unmistakably malicious events that should NEVER occur accidentally and indicates an active attack (i.e. brute forcing, DoS) + SecurityCritical = 4 // Indicates any malicious or exploitable event that had a side effect (i.e. secrets being left behind on the filesystem) + SecurityHigh = 3 // Indicates likely malicious events but one that had no side effects or was blocked (i.e. out of bounds symlinks in repos) + SecurityMedium = 2 // Could indicate malicious events, but has a high likelihood of being user/system error (i.e. access denied) + SecurityLow = 1 // Unexceptional entries (i.e. successful access logs) ) // Common error messages diff --git a/docs/operator-manual/config-management-plugins.md b/docs/operator-manual/config-management-plugins.md index 0334b2c168faa..792cd7ca53a54 100644 --- a/docs/operator-manual/config-management-plugins.md +++ b/docs/operator-manual/config-management-plugins.md @@ -67,8 +67,8 @@ spec: # Only one of fileName, find.glob, or find.command should be specified. If multiple are specified then only the # first (in that order) is evaluated. discover: - # fileName is a glob pattern (https://pkg.go.dev/path/filepath#Glob) that is applied to the repository's root - # directory (not the Application source directory). If there is a match, this plugin may be used for the repository. + # fileName is a glob pattern (https://pkg.go.dev/path/filepath#Glob) that is applied to the Application's source + # directory. If there is a match, this plugin may be used for the Application. fileName: "./subdir/s*.yaml" find: # This does the same thing as fileName, but it supports double-start (nested directory) glob patterns. diff --git a/go.mod b/go.mod index eb58fbe522a1d..04bed2b5ab02b 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/bradleyfalzon/ghinstallation/v2 v2.1.0 github.com/casbin/casbin/v2 v2.60.0 github.com/chai2010/gettext-go v0.0.0-20170215093142-bf70f2a70fb1 // indirect + github.com/cyphar/filepath-securejoin v0.2.3 github.com/dustin/go-humanize v1.0.0 github.com/evanphx/json-patch v5.6.0+incompatible github.com/fsnotify/fsnotify v1.6.0 diff --git a/go.sum b/go.sum index 814049f492ecd..ab57b15d73df7 100644 --- a/go.sum +++ b/go.sum @@ -252,6 +252,7 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46t github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/creack/pty v1.1.11 h1:07n33Z8lZxZ2qwegKbObQohDhXDQxiMMz1NOUGYlesw= github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/cyphar/filepath-securejoin v0.2.3 h1:YX6ebbZCZP7VkM3scTTokDgBL2TY741X51MTk3ycuNI= github.com/cyphar/filepath-securejoin v0.2.3/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= github.com/davecgh/go-spew v0.0.0-20161028175848-04cdfd42973b/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/test/e2e/testdata/cmp-fileName/plugin.yaml b/test/e2e/testdata/cmp-fileName/plugin.yaml index 1263c60054880..766278c79e773 100644 --- a/test/e2e/testdata/cmp-fileName/plugin.yaml +++ b/test/e2e/testdata/cmp-fileName/plugin.yaml @@ -7,4 +7,4 @@ spec: generate: command: [sh, -c, 'echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"'] discover: - fileName: "cmp-fileName/subdir/s*.yaml" + fileName: "subdir/s*.yaml" diff --git a/util/app/discovery/discovery.go b/util/app/discovery/discovery.go index 67cc58112bb7c..e29cbf2fa7ce1 100644 --- a/util/app/discovery/discovery.go +++ b/util/app/discovery/discovery.go @@ -93,7 +93,7 @@ func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, plugin pluginSockFilePath := common.GetPluginSockFilePath() log.WithFields(log.Fields{ common.SecurityField: common.SecurityLow, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Debugf("pluginSockFilePath is: %s", pluginSockFilePath) if pluginName != "" { @@ -160,7 +160,7 @@ func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fil if err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("error dialing to cmp-server for plugin %s, %v", fileName, err) return nil, nil, false } @@ -169,7 +169,7 @@ func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fil if err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("repository %s is not the match because %v", repoPath, err) io.Close(conn) return nil, nil, false @@ -182,7 +182,7 @@ func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fil } log.WithFields(log.Fields{ common.SecurityField: common.SecurityLow, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Debugf("Reponse from socket file %s does not support %v", fileName, repoPath) io.Close(conn) return nil, nil, false diff --git a/util/cert/cert.go b/util/cert/cert.go index c7ac284c20314..3826c72b7d6e7 100644 --- a/util/cert/cert.go +++ b/util/cert/cert.go @@ -135,7 +135,7 @@ func ParseTLSCertificatesFromPath(sourceFile string) ([]string, error) { if err = fileHandle.Close(); err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("error closing file %q: %v", fileHandle.Name(), err) } }() @@ -199,7 +199,7 @@ func ParseSSHKnownHostsFromPath(sourceFile string) ([]string, error) { if err = fileHandle.Close(); err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("error closing file %q: %v", fileHandle.Name(), err) } }() diff --git a/util/git/creds.go b/util/git/creds.go index 76655de34913c..c3d09574eeb84 100644 --- a/util/git/creds.go +++ b/util/git/creds.go @@ -279,7 +279,7 @@ func (c SSHCreds) Environ() (io.Closer, []string, error) { if err = file.Close(); err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("error closing file %q: %v", file.Name(), err) } }() diff --git a/util/gpg/gpg.go b/util/gpg/gpg.go index 140fe4932336e..681c22d310e23 100644 --- a/util/gpg/gpg.go +++ b/util/gpg/gpg.go @@ -170,7 +170,7 @@ func writeKeyToFile(keyData string) (string, error) { if err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("error closing file %q: %v", f.Name(), err) } }() @@ -275,7 +275,7 @@ func InitializeGnuPG() error { if err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("error closing file %q: %v", f.Name(), err) } }() @@ -302,7 +302,7 @@ func ImportPGPKeysFromString(keyData string) ([]*appsv1.GnuPGPublicKey, error) { if err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("error closing file %q: %v", f.Name(), err) } }() @@ -430,7 +430,7 @@ func SetPGPTrustLevel(pgpKeys []*appsv1.GnuPGPublicKey, trustLevel string) error if err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("error closing file %q: %v", f.Name(), err) } }() diff --git a/util/helm/cmd.go b/util/helm/cmd.go index db98c0fce7853..2be56b26d5ce8 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -176,7 +176,7 @@ func writeToTmp(data []byte) (string, argoio.Closer, error) { if err = file.Close(); err != nil { log.WithFields(log.Fields{ common.SecurityField: common.SecurityMedium, - common.SecurityCWEField: 775, + common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor, }).Errorf("error closing file %q: %v", file.Name(), err) } }()