terraform-aws-atlantis icon indicating copy to clipboard operation
terraform-aws-atlantis copied to clipboard

fix: Unintentionally exposed endpoints when enabling github webhooks

Open casperbiering opened this issue 3 years ago β€’ 4 comments

Description

When following the guide to allow access for github webhooks, then you're actually incorrectly exposing endpoints in the following ways:

  1. All endpoints is allowed from github IPs
  2. /events endpoint is allowed from the entire world

Motivation and Context

Limiting exposed endpoints is a good security practice.

Breaking Changes

It shouldn't except:

  1. abusing the rules to have github webhooks hit other endpoints than /events
  2. abusing the allow_github_webhooks option to allow webhooks from other systems than github

How Has This Been Tested?

  • [x] I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • [x] I have tested and validated these changes using one or more of the provided examples/* projects
  • [x] I have executed pre-commit run -a on my pull request

casperbiering avatar Jul 06 '22 14:07 casperbiering

This PR has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 10 days

github-actions[bot] avatar Aug 06 '22 00:08 github-actions[bot]

This is still relevant

casperbiering avatar Aug 08 '22 09:08 casperbiering

This PR has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 10 days

github-actions[bot] avatar Sep 08 '22 00:09 github-actions[bot]

This is still relevant

casperbiering avatar Sep 08 '22 11:09 casperbiering

@bryantbiggs @antonbabenko Can you guys review please?

casperbiering avatar Oct 07 '22 12:10 casperbiering

I think the overall intent of this PR is valid, but doesn't appear to work the way I was expecting and after a bit of experimentation, I think I understand why.

The default behavior of the ALB routing rules is to forward traffic if you don't specify an ALB authentication method. image

So any forwarding rules limiting /events to GitHub only IPs is ignored because by default all traffic is allowed. The only way I think this can be resolved is to have a fixed response (e.g., 401 Unauthorized) either as the default response or as a low priority listener rule.

dynamike avatar Oct 14 '22 16:10 dynamike

This PR has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 10 days

github-actions[bot] avatar Nov 14 '22 00:11 github-actions[bot]

This PR was automatically closed because of stale in 10 days

github-actions[bot] avatar Nov 25 '22 00:11 github-actions[bot]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Dec 25 '22 02:12 github-actions[bot]