fix: prevent SSRF via unvalidated Bitbucket Cloud pagination next URLs (CodeQL #35)

.github/workflows/test.yml

@@ -172,13 +172,19 @@ jobs:
             git config --global user.name "atlantis[bot]"
           fi
 
-      - run: |
-          make build-service
-          ./scripts/e2e.sh
+      - name: Build service
+        run: make build-service
+      - name: Run e2e tests
+        run: ./scripts/e2e.sh
   e2e-gitlab:
+    needs: [e2e-github]
     runs-on: ubuntu-24.04
     # dont run e2e tests on forked PRs
-    if: github.event.pull_request.head.repo.fork == false
+    # always() ensures this job runs even when e2e-github fails, while still
+    # respecting the fork check; sequential execution prevents ngrok auth
+    # token conflicts (both jobs share the same token and ngrok only allows
+    # one active tunnel per token at a time).
+    if: always() && github.event.pull_request.head.repo.fork == false
     env:
       ATLANTIS_GITLAB_USER: ${{ secrets.ATLANTISBOT_GITLAB_USERNAME }}
       ATLANTIS_GITLAB_TOKEN: ${{ secrets.ATLANTISBOT_GITLAB_TOKEN }}
@@ -213,6 +219,7 @@ jobs:
           git config --global user.email "maintainers@runatlantis.io"
           git config --global user.name "atlantisbot"
 
-      - run: |
-          make build-service
-          ./scripts/e2e.sh
+      - name: Build service
+        run: make build-service
+      - name: Run e2e tests
+        run: ./scripts/e2e.sh

e2e/gitlab.go

@@ -126,22 +126,25 @@ func (g GitlabClient) CreatePullRequest(ctx context.Context, title, branchName s
 }
 
 func (g GitlabClient) GetAtlantisStatus(ctx context.Context, branchName string) (string, error) {
-
-	pipelineInfos, _, err := g.client.MergeRequests.ListMergeRequestPipelines(g.projectId, g.branchToMR[branchName])
+	mr, _, err := g.client.MergeRequests.GetMergeRequest(g.projectId, g.branchToMR[branchName], nil)
 	if err != nil {
 		return "", err
 	}
-	// Possible todo: determine which status in the pipeline we care about?
-	if len(pipelineInfos) != 1 {
-		return "", fmt.Errorf("unexpected pipelines: %d", len(pipelineInfos))
-	}
-	pipelineInfo := pipelineInfos[0]
-	pipeline, _, err := g.client.Pipelines.GetPipeline(g.projectId, pipelineInfo.ID)
+
+	// By default (all=false) GitLab returns only the latest status per name+ref
+	// combination, so statuses[0] is the most recent "atlantis/plan" status.
+	statuses, _, err := g.client.Commits.GetCommitStatuses(g.projectId, mr.SHA, &gitlab.GetCommitStatusesOptions{
+		Name: gitlab.Ptr("atlantis/plan"),
+	})
 	if err != nil {
 		return "", err
 	}
-
-	return pipeline.Status, nil
+	// Return "" while Atlantis hasn't processed the webhook yet; the caller
+	// treats an empty string as "not started" and continues polling.
+	if len(statuses) == 0 {
+		return "", nil
+	}
+	return statuses[0].Status, nil
 }
 
 func (g GitlabClient) ClosePullRequest(ctx context.Context, pullRequestNumber int) error {
@@ -166,9 +169,9 @@ func (g GitlabClient) DeleteBranch(ctx context.Context, branchName string) error
 }
 
 func (g GitlabClient) IsAtlantisInProgress(state string) bool {
-	// From https://docs.gitlab.com/api/pipelines/
-	// created, waiting_for_resource, preparing, pending, running, success, failed, canceled, skipped, manual, scheduled
-	for _, s := range []string{"success", "failed", "canceled", "skipped"} {
+	// From https://docs.gitlab.com/ee/api/commits.html#list-the-statuses-of-a-commit
+	// GitLab commit status states: pending, running, success, failed, canceled
+	for _, s := range []string{"success", "failed", "canceled"} {
 		if state == s {
 			return false
 		}

server/events/vcs/bitbucketcloud/client.go

@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"strings"
 	"unicode/utf8"
 
@@ -88,10 +89,11 @@ func (b *Client) GetModifiedFiles(logger logging.SimpleLogging, repo models.Repo
 		if diffStat.Next == nil || *diffStat.Next == "" {
 			break
 		}
+		if err := b.validateNextPageURL(*diffStat.Next); err != nil {
+			return nil, fmt.Errorf("getting modified files: %w", err)
+		}
 		nextPageURL = *diffStat.Next
 	}
-
-	// Now ensure all files are unique.
 	hash := make(map[string]bool)
 	var unique []string
 	for _, f := range files {
@@ -266,6 +268,9 @@ func (b *Client) PullIsMergeable(logger logging.SimpleLogging, repo models.Repo,
 		if diffStat.Next == nil || *diffStat.Next == "" {
 			break
 		}
+		if err := b.validateNextPageURL(*diffStat.Next); err != nil {
+			return models.MergeableStatus{}, fmt.Errorf("checking pull mergeability: %w", err)
+		}
 		nextPageURL = *diffStat.Next
 	}
 	return models.MergeableStatus{
@@ -393,3 +398,21 @@ func (b *Client) GetCloneURL(_ logging.SimpleLogging, _ models.VCSHostType, _ st
 func (b *Client) GetPullLabels(_ logging.SimpleLogging, _ models.Repo, _ models.PullRequest) ([]string, error) {
 	return nil, fmt.Errorf("not yet implemented")
 }
+
+// validateNextPageURL checks that a pagination URL returned by the Bitbucket
+// API has the same host as the configured base URL, preventing SSRF attacks
+// where a malicious server response could redirect requests to internal hosts.
+func (b *Client) validateNextPageURL(nextPageURL string) error {
+	parsedNext, err := url.Parse(nextPageURL)
+	if err != nil {
+		return fmt.Errorf("parsing next page URL %q: %w", nextPageURL, err)
+	}
+	parsedBase, err := url.Parse(b.BaseURL)
+	if err != nil {
+		return fmt.Errorf("parsing base URL %q: %w", b.BaseURL, err)
+	}
+	if parsedNext.Host != parsedBase.Host {
+		return fmt.Errorf("next page URL %q host %q does not match base URL host %q", nextPageURL, parsedNext.Host, parsedBase.Host)
+	}
+	return nil
+}

server/events/vcs/bitbucketcloud/client_test.go

@@ -370,6 +370,82 @@ func TestClient_PullIsMergeable(t *testing.T) {
 
 }
 
+// TestClient_GetModifiedFiles_SSRFPrevented verifies that a malicious "next"
+// pagination URL pointing to a different host is rejected to prevent SSRF.
+func TestClient_GetModifiedFiles_SSRFPrevented(t *testing.T) {
+	logger := logging.NewNoopLogger(t)
+	resp := `{
+		"pagelen": 1,
+		"values": [
+			{
+				"status": "modified",
+				"old": {"path": "file1.txt"},
+				"new": {"path": "file1.txt"}
+			}
+		],
+		"next": "http://internal.evil.host/steal-secrets"
+	}`
+
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.RequestURI {
+		case diffstatURL:
+			w.Write([]byte(resp)) // nolint: errcheck
+		default:
+			t.Errorf("got unexpected request at %q", r.RequestURI)
+			http.Error(w, "not found", http.StatusNotFound)
+		}
+	}))
+	defer testServer.Close()
+
+	client := bitbucketcloud.New(http.DefaultClient, "user", "pass", "", "runatlantis.io")
+	client.BaseURL = testServer.URL
+
+	_, err := client.GetModifiedFiles(logger, models.Repo{
+		FullName: "owner/repo",
+		VCSHost:  models.VCSHost{Type: models.BitbucketCloud, Hostname: "bitbucket.org"},
+	}, models.PullRequest{Num: 1})
+	Assert(t, err != nil, "expected error for malicious next page URL")
+	Assert(t, strings.Contains(err.Error(), "does not match base URL host"), "error should mention host mismatch, got: %s", err.Error())
+}
+
+// TestClient_PullIsMergeable_SSRFPrevented verifies that a malicious "next"
+// pagination URL pointing to a different host is rejected to prevent SSRF.
+func TestClient_PullIsMergeable_SSRFPrevented(t *testing.T) {
+	logger := logging.NewNoopLogger(t)
+	resp := `{
+		"pagelen": 1,
+		"values": [
+			{
+				"status": "modified",
+				"old": {"path": "main.tf"},
+				"new": {"path": "main.tf"}
+			}
+		],
+		"next": "http://internal.evil.host/steal-secrets"
+	}`
+
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.RequestURI {
+		case diffstatURL:
+			w.Write([]byte(resp)) // nolint: errcheck
+		default:
+			t.Errorf("got unexpected request at %q", r.RequestURI)
+			http.Error(w, "not found", http.StatusNotFound)
+		}
+	}))
+	defer testServer.Close()
+
+	client := bitbucketcloud.New(http.DefaultClient, "user", "pass", "", "runatlantis.io")
+	client.BaseURL = testServer.URL
+
+	_, err := client.PullIsMergeable(logger, models.Repo{
+		FullName: "owner/repo",
+		VCSHost:  models.VCSHost{Type: models.BitbucketCloud, Hostname: "bitbucket.org"},
+	}, models.PullRequest{Num: 1}, "", []string{})
+	Assert(t, err != nil, "expected error for malicious next page URL")
+	Assert(t, strings.Contains(err.Error(), "does not match base URL host"), "error should mention host mismatch, got: %s", err.Error())
+}
+
 func TestClient_MarkdownPullLink(t *testing.T) {
 	client := bitbucketcloud.New(http.DefaultClient, "user", "pass", "", "runatlantis.io")
 	pull := models.PullRequest{Num: 1}