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

fix(github): prevent null pointer dereferencing when using AllowMergeableBypassApply with no required checks on branch protection #3672

Merged
merged 3 commits into from
Aug 15, 2023

Conversation

arturhoo
Copy link
Contributor

@arturhoo arturhoo commented Aug 15, 2023

what

Handle the case where Atlantis is using the feature flag AllowMergeableBypassApply, but a repository does not have a required check under its branch protection rules (it might have, for example, a requirement for at least one approval).

Currently, the code assumes that if a response was returned from https://docs.github.com/en/rest/branches/branch-protection?apiVersion=2022-11-28#get-branch-protection, then a required check will always be defined, which is not the case, leading to the panic seen in the linked issues:

runtime error: invalid memory address or nil pointer dereference
runtime/panic.go:260 (0x44e11c)
runtime/signal_unix.go:837 (0x44e0ec)
github.com/runatlantis/atlantis/server/events/vcs/github_client.go:438 (0xd91157)
github.com/runatlantis/atlantis/server/events/vcs/github_client.go:507 (0xd91717)
github.com/runatlantis/atlantis/server/events/vcs/instrumented_client.go:206 (0xd98beb)
github.com/runatlantis/atlantis/server/events/vcs/proxy.go:80 (0xd9a984)
github.com/runatlantis/atlantis/server/events/vcs/pull_status_fetcher.go:32 (0xd9b255)
github.com/runatlantis/atlantis/server/events/apply_command_runner.go:105 (0xf92cb4)
github.com/runatlantis/atlantis/server/events/command_runner.go:298 (0xf97f43)
runtime/asm_amd64.s:1598 (0x46b2e0)

why

See issues #2663, #3563

tests

I have implemented a new test case that covers the case where:

  • Check suite has runs
  • Check runs contains runs that have completed with success
  • But none of the checks in the suite is a required check

Happy to add more cases if deemed necessary.

references

closes #2663
closes #3563

@arturhoo arturhoo requested a review from a team as a code owner August 15, 2023 13:29
@github-actions github-actions bot added go Pull requests that update Go code provider/github labels Aug 15, 2023
@arturhoo arturhoo changed the title Fix 2663 Fix null pointer dereferencing when using AllowMergeableBypassApply with no required checks on branch protection Aug 15, 2023
@arturhoo arturhoo changed the title Fix null pointer dereferencing when using AllowMergeableBypassApply with no required checks on branch protection fix(github): prevent null pointer dereferencing when using AllowMergeableBypassApply with no required checks on branch protection Aug 15, 2023
Copy link
Contributor

@jamengual jamengual left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks @arturhoo for the contribution

@jamengual jamengual merged commit 049c146 into runatlantis:main Aug 15, 2023
12 checks passed
@arturhoo arturhoo deleted the fix-2663 branch August 15, 2023 18:05
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…ableBypassApply with no required checks on branch protection (runatlantis#3672)

* Implement test for allow mergeable bypass apply with no branch protection checks

* Report true combined status if no branch protection required status checks

* Remove warning on feature flag utilization
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…ableBypassApply with no required checks on branch protection (runatlantis#3672)

* Implement test for allow mergeable bypass apply with no branch protection checks

* Report true combined status if no branch protection required status checks

* Remove warning on feature flag utilization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation go Pull requests that update Go code provider/github
Projects
None yet
2 participants