Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'util/git/creds.go' will leak temporary files to '/dev/shm' if proxy URL cannot be parsed, or if write fails #17658

Closed
3 tasks done
jgwest opened this issue Mar 28, 2024 · 1 comment · Fixed by #17659
Closed
3 tasks done
Assignees
Labels
bug Something isn't working component:core Syncing, diffing, cluster state cache

Comments

@jgwest
Copy link
Member

jgwest commented Mar 28, 2024

Checklist:

  • I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
  • I've included steps to reproduce the bug.
  • I've pasted the output of argocd version.

Describe the bug

In util/git/creds.go, within (c SSHCreds) Environ(), there are a couple of corner cases where the temporary file that is created is not being cleaned up:

  • If the socks5 proxy settings URL cannot be parsed by url.Parse, the function will return without deleting the temporary file.
  • If a call to file.WriteString fails, the temporary file will not be cleaned up (but, admittedly, this would be very rare)

We've had customer reports of /dev/shm running out of space due to temporary files left hanging around, and I discovered the above issue while auditing Argo CD's use of /dev/shm.

I've submitted a PR which fixes both issues. The solution is the same one used by the other Environ() functions in creds.go

Version

master branch

@jgwest jgwest added the bug Something isn't working label Mar 28, 2024
@jgwest jgwest self-assigned this Mar 28, 2024
@jgwest jgwest added the component:core Syncing, diffing, cluster state cache label Mar 28, 2024
jgwest added a commit to jgwest/argo-cd that referenced this issue Mar 28, 2024
Signed-off-by: Jonathan West <jonwest@redhat.com>
jgwest added a commit to jgwest/argo-cd that referenced this issue Mar 28, 2024
Signed-off-by: Jonathan West <jonwest@redhat.com>
jgwest added a commit to jgwest/argo-cd that referenced this issue Mar 28, 2024
Signed-off-by: Jonathan West <jonwest@redhat.com>
@manidharanupoju24
Copy link

We are seeing some customer issues regarding the same. Hopefully, this PR fixes the same

jgwest added a commit to jgwest/argo-cd that referenced this issue Apr 11, 2024
Signed-off-by: Jonathan West <jonwest@redhat.com>
jgwest added a commit to jgwest/argo-cd that referenced this issue Apr 11, 2024
Signed-off-by: Jonathan West <jonwest@redhat.com>
jannfis pushed a commit that referenced this issue Apr 11, 2024
Signed-off-by: Jonathan West <jonwest@redhat.com>
mkieweg pushed a commit to mkieweg/argo-cd that referenced this issue Jun 11, 2024
Hariharasuthan99 pushed a commit to AmadeusITGroup/argo-cd that referenced this issue Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:core Syncing, diffing, cluster state cache
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants