[feature] Protect atlantis.yaml

[so0k]

Atm, any developer can add pre_get, pre_init, pre/post_plan commands to expose secrets via atlantis.yaml - as Atlantis requires quite significant permissions, this might be a concern.

Drone used to have a signing step required for the drone.yaml file, ensuring unauthorized modifications to atlantis.yaml can be prevented

[so0k]

My mistake, output of these hooks isn't printed to comments

[lkysow]

I think this is still an issue actually. If there's an error while running the scripts then we print out the output if you run the command with --verbose.

[lkysow]

I think the best way to fix this is to only execute the atlantis.yaml file that's on the branch being pull requested into.

[eriksw]

Using old atlantis.yaml seems problematic. The most trivial example that springs to mind is a PR that changes what version of terraform is being used, with simultaneous changes to atlantis.yaml and .tf changes that require the new version of terraform.

[lkysow]

@eriksw in that case you'd have to do 2 pull requests. The first would be the version change and the second would be the tf changes.

It's kind of annoying but if we use the atlantis.yaml on the pull request it seems like we open ourselves up to lots of issues that circumvent the code review and authorization aspect of Atlantis, like the example the issue @so0k mentioned.

I anticipate that atlantis.yaml won't be changing too much. Basically you set up your repo and then you're good to go. So it might be okay.

[eriksw]

I can't say I'm a fan of having to go through a two-PR process every time there's a new version of terraform. (Pinning in both .tf terraform { .. } and atlantis.yaml.)

The simplicity of requiring a simple detached hmac "signature" of atlantis.yaml is pretty appealing in comparison.

[lkysow]

@eriksw you're right that that is a poor workflow. Thank you for your concerns. As a result I've spent some more time looking into how drone.io deals with these issues and why they removed the signing step. This thread is particularly illuminating: harness/harness#1935

My conclusions:

• Requiring signing atlantis.yaml would be a poor user experience for the reasons experienced by drone. It would also require some changes I don't like very much. Either starting atlantis server with a secret signing key option, ex. -signing-key=<secret> that then everyone needs to know in order to sign the atlantis.yaml or having some sort of login system into Atlantis which is more than I think the project should bite off at this moment.
• Only executing the atlantis.yaml from the base branch (requiring a pull request merge to make changes) is also a poor user experience for the reasons Erik mentioned and also for testing new files.
• Thus I'm leaning more towards the solution that drone settled on: if we detect a change in atlantis.yaml we won't run any commands unless the user doing the pull request is in the list of admins for the project OR the PR is approved. In a later change, we will need to implement something like a gatekeeper endpoint (see drone's here: RFC improved gating capabilities, whitelists harness/harness#1971 (comment)) or hook into an identity system to make this more configurable.

[so0k]

I like the conclusion - in Drone we do provide a list of admins as part of the configuration, but you will get this from the repo attributes?

Does this mean atlantis plan won't work for PRs that include changes to atlantis.yaml unless the PR is "trusted" - where trusted means the user creating the PR is an admin collaborator or belongs to a team that have admin rights?

[lkysow]

but you will get this from the repo attributes

Yes, for now until I have time to work on a more robust authentication/authorization system

Does this mean atlantis plan won't work for PRs that include changes to atlantis.yaml unless the
PR is "trusted"

Yes, exactly–because otherwise you could put malicious changes in the plan step

[grobie]

Another use case:

• mono repo with hundreds of commits per hour
• pushing to master directly can't be forbidden

The proposed solution there won't work, as a user could simply change atlantis.yml in master and then create a PR. An additionally solution for such mono repo use cases: separate atlantis.yml from the terraform files and load it directly on the (secured) server.

[atlantisbot]

This issue was migrated to runatlantis/atlantis#47. Read about why here.