Skip to content

Commit

Permalink
fix: prevent leak of files to /dev/shm in corner cases (#17658) (#17659)
Browse files Browse the repository at this point in the history
Signed-off-by: Jonathan West <[email protected]>
  • Loading branch information
jgwest authored Apr 11, 2024
1 parent 0cf6fdb commit 5ac8d05
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
7 changes: 6 additions & 1 deletion util/git/creds.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
}

Expand All @@ -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'",
Expand All @@ -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
Expand Down
32 changes: 32 additions & 0 deletions util/git/creds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 5ac8d05

Please sign in to comment.