diff --git a/util/git/creds.go b/util/git/creds.go index 18698449082bf..e0ca584ebb7ad 100644 --- a/util/git/creds.go +++ b/util/git/creds.go @@ -277,6 +277,9 @@ func (c SSHCreds) Environ() (io.Closer, []string, error) { if err != nil { return nil, nil, err } + + sshCloser := sshPrivateKeyFile(file.Name()) + defer func() { if err = file.Close(); err != nil { log.WithFields(log.Fields{ @@ -288,6 +291,7 @@ func (c SSHCreds) Environ() (io.Closer, []string, error) { _, err = file.WriteString(c.sshPrivateKey + "\n") if err != nil { + sshCloser.Close() return nil, nil, err } @@ -310,6 +314,7 @@ func (c SSHCreds) Environ() (io.Closer, []string, error) { if c.proxy != "" { parsedProxyURL, err := url.Parse(c.proxy) if err != nil { + sshCloser.Close() return nil, nil, fmt.Errorf("failed to set environment variables related to socks5 proxy, could not parse proxy URL '%s': %w", c.proxy, err) } args = append(args, "-o", fmt.Sprintf("ProxyCommand='connect-proxy -S %s:%s -5 %%h %%p'", @@ -324,7 +329,7 @@ func (c SSHCreds) Environ() (io.Closer, []string, error) { } env = append(env, []string{fmt.Sprintf("GIT_SSH_COMMAND=%s", strings.Join(args, " "))}...) env = append(env, proxyEnv...) - return sshPrivateKeyFile(file.Name()), env, nil + return sshCloser, env, nil } // GitHubAppCreds to authenticate as GitHub application diff --git a/util/git/creds_test.go b/util/git/creds_test.go index 23a705ed33574..69c1150db881b 100644 --- a/util/git/creds_test.go +++ b/util/git/creds_test.go @@ -17,6 +17,7 @@ import ( "github.com/argoproj/argo-cd/v2/util/cert" "github.com/argoproj/argo-cd/v2/util/io" + argoio "github.com/argoproj/gitops-engine/pkg/utils/io" ) type cred struct { @@ -302,6 +303,37 @@ func Test_SSHCreds_Environ_WithProxyUserNamePassword(t *testing.T) { } } +func Test_SSHCreds_Environ_TempFileCleanupOnInvalidProxyURL(t *testing.T) { + + // Previously, if the proxy URL was invalid, a temporary file would be left in /dev/shm. This ensures the file is cleaned up in this case. + + // countDev returns the number of files in /dev/shm (argoio.TempDir) + countFilesInDevShm := func() int { + entries, err := os.ReadDir(argoio.TempDir) + require.NoError(t, err) + + return len(entries) + } + + for _, insecureIgnoreHostKey := range []bool{false, true} { + tempDir := t.TempDir() + caFile := path.Join(tempDir, "caFile") + err := os.WriteFile(caFile, []byte(""), os.FileMode(0600)) + require.NoError(t, err) + creds := NewSSHCreds("sshPrivateKey", caFile, insecureIgnoreHostKey, &NoopCredsStore{}, ":invalid-proxy-url") + + filesInDevShmBeforeInvocation := countFilesInDevShm() + + _, _, err = creds.Environ() + require.Error(t, err) + + filesInDevShmAfterInvocation := countFilesInDevShm() + + assert.Equal(t, filesInDevShmBeforeInvocation, filesInDevShmAfterInvocation, "no temporary files should leak if the proxy url cannot be parsed") + + } +} + const gcpServiceAccountKeyJSON = `{ "type": "service_account", "project_id": "my-google-project",