Create .git-credentials to allow secure auth when cloning private repos

[andrewjeffree]

Currently when Atlantis is running in a docker container it cannot access private repositories without people either pushing ssh private keys into the container with commands at launch or in some cases forking off their own copy where they embed the ssh key/secrets in the image. I'm sure there are a myriad of other ways people are doing it also. None of these are particularly nice options at least personally as then you're providing two sets of secrets instead of one.

GitHub, GitLab and Bitbucket all support push/pull via HTTPS you just need to provide the respective token. git will and does prompt you for the username and password required to authenticate over HTTPS however it is not cached to disk.

Terraform itself doesn't support any sort of environment variables either which would allow it to take care of this. As such we need to use git-credentials-store to configure this as a once off when we run the container. We do this by writing out a .git-credentials file into the atlantis users home directory with the relevant information that is already provided to the container as environment variables when launched and then running git config --global credential.helper store as the atlantis user so that it's aware of the .git-credentials file.

This will only affect/work for those who use HTTPS sources.

I've conducted some basic testing on all three providers and didn't run into any issues.

Happy to enhance/tweak as required.

[codecov]

Codecov Report

Merging #711 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #711   +/-   ##
=======================================
  Coverage   72.45%   72.45%           
=======================================
  Files          61       61           
  Lines        4679     4679           
=======================================
  Hits         3390     3390           
  Misses       1039     1039           
  Partials      250      250

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8180f35...0e99363. Read the comment docs.

[codecov]

Codecov Report

Merging #711 into master will decrease coverage by 0.04%.
The diff coverage is 54.54%.

@@            Coverage Diff             @@
##           master     #711      +/-   ##
==========================================
- Coverage   72.37%   72.32%   -0.05%     
==========================================
  Files          61       62       +1     
  Lines        4695     4712      +17     
==========================================
+ Hits         3398     3408      +10     
- Misses       1044     1052       +8     
+ Partials      253      252       -1

Impacted Files	Coverage Δ
cmd/server.go	79.42% <ø> (ø)	⬆️
server/user_config.go	100% <ø> (ø)	⬆️
server/server.go	63.87% <0%> (-3.02%)	⬇️
server/events/git_cred_writer.go	90% <90%> (ø)
server/events/runtime/runtime.go	78.94% <0%> (ø)	⬆️
server/events/terraform/terraform_client.go	79.03% <0%> (+0.97%)	⬆️
server/events/runtime/run_step_runner.go	100% <0%> (+10%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f9412b...6120974. Read the comment docs.

[lkysow]

Hi Andrew, thanks for the PR! I'm wondering if we should make this optional based on an environment variable because it's writing the credentials in plaintext to disk?

[andrewjeffree]

Hey. I can see the logic behind that, and am happy to implement that. Do you have any preferences around the variable name?

[lkysow]

Hmm, maybe ATLANTIS_GIT_CREDS=true?

[lkysow]

Hmm, I'm wondering if we should actually implement this within Atlantis itself on startup? Because this requires you use environment variables and our Docker container.

What about a flag --write-git-creds=true?

[andrewjeffree]

Implementing it into Atlantis itself is probably a better solution. I’m happy to give it a shot though I have next to no experience with go. I like --write-git-creds=true

[lkysow]

• See Add flag for setting Terraform Enterprise hostname #706 for how to add a CLI flag.
• I'd put the implementation into a new file called server/events/git_cred_writer.go
• It can be a function without a struct/class, ex

  func writeGitCreds(ghUser, ghToken, ghHostname, glUser ...)

  should be similar in function to https://github.com/runatlantis/atlantis/blob/master/server/events/terraform/terraform_client.go#L401
• Can be called in the NewServer() method of server/server.go
• Will need tests, see the tests for generateRCFiles here https://github.com/runatlantis/atlantis/blob/master/server/events/terraform/terraform_client_internal_test.go#L17

[andrewjeffree]

So I'm just in the process of figuring out where to put the logic in the NewServer() method of server/server.go and I can see two ways of going about it.

I put something like the below in the respective GitHub, GitLab and Bitbucket blocks here

if userConfig.WriteGitCreds == true {
    events.writeGitCreds(userConfig.User, userConfig.Token, userConfig.Host)
 }

Or I split it out to its own logic just below it and duplicate the logic around determining which git provider is being used.

Do you have any preferences around this?

[andrewjeffree]

I think I've got the basic logic in place now. I have no idea why the testing is saying writeGitCreds is undefined I'm a bit out of my depth here, unfortunately. Would appreciate some pointers and/or assistance in getting this working.

[andrewjeffree]

Well, I figured it out and it's passing now after some swearing on my end. I've squashed it to make it easier to review/merge as desired. Let me know if you want adjustments made.

[andrewjeffree]

@lkysow I think it's ready for review. I'm not sure how to fix the coverage with server.go though.

[lkysow]

Nice! Don't worry about the server.go coverage. I'll try to get to the review asap but I've got a lot on my plate so you might need to wait a while.

[lkysow]

This is awesome! Final feedback:

• some small changes requested
• please squash your commits

Not in this PR but if you're down to do another one I'd like to:

1. Get ssh private cloning working as well. All you need to do is write an additional line to the .gitconfig:

   [url "https://[email protected]"]
      insteadOf = ssh:https://[email protected]
   

2. Get this working if using more than one VCS provider. Right now if you're using github and gitlab then it will fail because it tries to overwrite itself.

[andrewjeffree]

* some small changes requested

* please squash your commits

Changes made and commits squashed.

Not in this PR but if you're down to do another one I'd like to:

1. Get ssh private cloning working as well. All you need to do is write an additional line to the .gitconfig:
   ```
   [url "https://[email protected]"]
      insteadOf = ssh:https://[email protected]
   ```

2. Get this working if using more than one VCS provider. Right now if you're using github and gitlab then it will fail because it tries to overwrite itself.

Sure. If you create an issue and assign it to me I'll give it a shot.

[chadasapp]

1. Get ssh private cloning working as well. All you need to do is write an additional line to the .gitconfig:

   [url "https://[email protected]"]
      insteadOf = ssh:https://[email protected]
   

I wouldn't suggest doing that, actually. We shouldn't assume that all repos available over SSH are accessible to this user with their token provided. The Atlantis user is only required to have read access to the repository/repositories being acted upon. Other git operations as a consequence (I'm looking at you, modules), are not necessarily accessible to that Github user.

I could see a scenario in which the Atlantis user doesn't even have an SSH key, but various modules that need accessing are given deploy keys that are loaded into the ssh-agent that Atlantis uses. Compartmentalizing access isn't a bad idea, after all :)

I'd stick to just HTTPS with this patch and leave SSH as-is.

2. Get this working if using more than one VCS provider. Right now if you're using github and gitlab then it will fail because it tries to overwrite itself.

Github Enterprise is being forgotten here. Please vary all config based on ATLANTIS_GH_HOSTNAME where appropriate.

[chadasapp]

Generally speaking, I think we can do a little better here to help avoid the "malicious PRs could exfiltrate credentials" issue.

1. Don't worry about keeping the username private, its already known to the people submitting PRs anyway. So you can get away with: git config --global credential.https://github.com.username $GIT_USER (vary on ATLANTIS_GH_HOSTNAME, as in my prior comment, please)
2. Write the token to a bash script on disk when git clone/fetch operations happen, then kill it afterwords. Use GIT_ASKPASS to point to said script. That way the token doesn't have to exist on-disk when Terraform operations are happening.

Another approach could include messing with the credential cache times -- basically set it to infinity so it's in memory and so after first authentication we can kill the on-disk credential entirely.

[andrewjeffree]

wouldn't suggest doing that, actually. We shouldn't assume that all repos available over SSH are accessible to this user with their token provided. The Atlantis user is only required to have read access to the repository/repositories being acted upon. Other git operations as a consequence (I'm looking at you, modules), are not necessarily accessible to that Github user.

How do you expect Terraform to be able to checkout modules via terraform init without the user itself having access to those modules? All my modules are private git repos. If the Atlantis user doesn't have access to the modules I'm unsure how you'd actually be making effective use of Atlantis. Perhaps if you could share more detail on your workflow?

I could see a scenario in which the Atlantis user doesn't even have an SSH key, but various modules that need accessing are given deploy keys that are loaded into the ssh-agent that Atlantis uses. Compartmentalizing access isn't a bad idea, after all :)

But then you're storing secrets (ssh keys) in your terraform repo/git? Perhaps I'm misunderstanding your intended workflow here.

I'd stick to just HTTPS with this patch and leave SSH as-is.

I believe the request was to look at doing SSH in a separate PR. I certainly have no intention of doing anything apart from HTTPS in this PR at this point in time. Perhaps in a future PR if/when SSH is looked at it could be added as another flag to alivieate your concerns here.

Github Enterprise is being forgotten here. Please vary all config based on ATLANTIS_GH_HOSTNAME where appropriate.

The code should work fine with Github Enterprise and already makes use of the ATLANTIS_GH_HOSTNAME value. We pass this value to the function on every usage when GitHub user is actually set.

https://github.com/ImperialXT/atlantis/blob/master/server/server.go#L161-L165

By default, the Github hostname value is github.com unless overridden via the CLI fags or environment variable ATLANTIS_GH_HOSTNAME.
https://github.com/ImperialXT/atlantis/blob/master/cmd/server.go#L125-L127
https://github.com/ImperialXT/atlantis/blob/master/cmd/server.go#L80

Write the token to a bash script on disk when git clone/fetch operations happen, then kill it afterwords. Use GIT_ASKPASS to point to said script. That way the token doesn't have to exist on-disk when Terraform operations are happening.

That could work. I think the only place it'd need to be called would be in server/events/runtime/init_step_runner.go @lkysow What are your thoughts on implementing it this way instead?

[chadasapp]

wouldn't suggest doing that, actually. We shouldn't assume that all repos available over SSH are accessible to this user with their token provided. The Atlantis user is only required to have read access to the repository/repositories being acted upon. Other git operations as a consequence (I'm looking at you, modules), are not necessarily accessible to that Github user.

How do you expect Terraform to be able to checkout modules via terraform init without the user itself having access to those modules? All my modules are private git repos. If the Atlantis user doesn't have access to the modules I'm unsure how you'd actually be making effective use of Atlantis. Perhaps if you could share more detail on your workflow?

If the repos are accessed over SSH instead of HTTPS, they could be done with deploy keys as I say below, which are injected into the ssh-agent. The Atlantis Github user doesn't have any relation to these keys at all, hence why rewriting SSH to HTTPS would break things.

This isn't actually my workflow, but it doesn't seem implausible and rewriting protocols just struck me as likely to cause issues for someone so I was just playing hypotheticals.

I could see a scenario in which the Atlantis user doesn't even have an SSH key, but various modules that need accessing are given deploy keys that are loaded into the ssh-agent that Atlantis uses. Compartmentalizing access isn't a bad idea, after all :)

But then you're storing secrets (ssh keys) in your terraform repo/git? Perhaps I'm misunderstanding your intended workflow here.

The public key would be a deploy key on the repo on Github, the private key would be injected into the ssh-agent where Atlantis is running (be that Docker container or otherwise). At my last job, we actually wrote some software to allow for SSH keys to be used without exposing the private key to the caller, so this isn't entirely implausible either.

I'd stick to just HTTPS with this patch and leave SSH as-is.

I believe the request was to look at doing SSH in a separate PR. I certainly have no intention of doing anything apart from HTTPS in this PR at this point in time. Perhaps in a future PR if/when SSH is looked at it could be added as another flag to alivieate your concerns here.

I misunderstood! Carry on then :)

Github Enterprise is being forgotten here. Please vary all config based on ATLANTIS_GH_HOSTNAME where appropriate.

The code should work fine with Github Enterprise and already makes use of the ATLANTIS_GH_HOSTNAME value. We pass this value to the function on every usage when GitHub user is actually set.

https://github.com/ImperialXT/atlantis/blob/master/server/server.go#L161-L165

By default, the Github hostname value is github.com unless overridden via the CLI fags or environment variable ATLANTIS_GH_HOSTNAME.
https://github.com/ImperialXT/atlantis/blob/master/cmd/server.go#L125-L127
https://github.com/ImperialXT/atlantis/blob/master/cmd/server.go#L80

I must've misread something or misunderstood the comments, I guess! Again, carry on!

Write the token to a bash script on disk when git clone/fetch operations happen, then kill it afterwords. Use GIT_ASKPASS to point to said script. That way the token doesn't have to exist on-disk when Terraform operations are happening.

That could work. I think the only place it'd need to be called would be in server/events/runtime/init_step_runner.go @lkysow What are your thoughts on implementing it this way instead?

Oh cool, I wasn't sure if that'd be feasible or not. That'd be a great route and would alleviate a lot of the concerns with exfiltration we see right now.

[lkysow]

Thanks for the great PR!

I'd be happy to take a look at a PR that implements the ASK_PASS suggestions but this is a nice improvement for now and is optional for users that don't want to enable.