Skip to content

[BUG]: New client leaks concurrency permits on non-2xx REST responses #3603

Description

@RoFz

Expected Behavior

With legacy_client = false, every concurrency permit acquired for a GitHub REST request should be released after the response has been consumed, including when GitHub returns a non-2xx response.

Expected responses such as 404 Not Found from the vulnerability-alerts endpoint should not progressively exhaust the provider's concurrency limit. A Terraform plan managing more than 100 repositories should complete without requests becoming permanently blocked inside the provider.

Actual Behavior

The new client introduced in v6.13.0 leaks one concurrency permit whenever go-github processes a non-2xx REST response.

After 100 such responses, all permits in the provider's shared semaphore are exhausted. Subsequent requests block in semaphore.Acquire before reaching the network. They eventually fail after the provider's five-minute client timeout with:

Get "https://api.github.com/repos/example-org/example-repository": (http.RoundTripper).RoundTrip failed: context deadline exceeded (Client.Timeout exceeded while awaiting headers)

This was reproduced on both:

  • Terraform v1.14.4 on linux_amd64, running on a self-hosted GitHub Actions runner
  • Terraform v1.14.4 on darwin_arm64

The affected configuration manages 331 existing github_repository resources using GitHub App authentication.

The repeated trigger in this configuration is:

GET /repos/{owner}/{repository}/vulnerability-alerts

GitHub returns 404 Not Found when vulnerability alerts are disabled. go-github intentionally converts that response to false without returning an error, but the concurrency permit has already been leaked.

Root cause

The v6.13.0 throttler:

  1. Acquires one semaphore permit.
  2. Wraps the original response body in throttlerReadCloser.
  3. Releases the permit only when that wrapper's Close method is called.

For non-2xx responses, go-github.CheckResponse reads the wrapped body and replaces r.Body with a new io.NopCloser, without closing the original body.

bareDo subsequently closes only the replacement body. The original throttlerReadCloser is no longer referenced, so its Close method is never called and the permit is never released.

The provider's concurrency limit is exactly 100, and its client timeout is five minutes: internal/ghclient/const.go.

For the observed trigger, GetVulnerabilityAlerts calls client.Do and then parseBoolResponse, which hides the 404 and returns false, nil. The plan therefore continues while silently losing one permit for each repository with vulnerability alerts disabled.

Control tests established that this is not a network-connectivity or retry problem:

  • Direct authenticated curl requests from the same runner completed successfully over HTTP/2.
  • The provider process had no outbound TCP socket for the requests waiting on the exhausted semaphore.
  • max_retries = 0 produced the same behavior.
  • legacy_client = true with parallel_requests = true completed the same plan successfully.
  • The issue occurs with legacy_client = false, where parallel_requests is documented as ignored.

Terraform Version

Terraform v1.14.4
on linux_amd64
+ provider registry.terraform.io/integrations/github v6.13.0

Also reproduced with Terraform v1.14.4 on darwin_arm64.

GitHub Installation Type

  • GitHub.com (Free, Pro, or Team)
  • GitHub Enterprise Server (on-premises)
  • GitHub Enterprise Cloud with Personal Accounts (github.com)
  • GitHub Enterprise Cloud with Managed Users/EMU (github.com)
  • GitHub Enterprise Cloud with Data Residency (*.ghe.com)
  • I don't know

Affected Resource(s)

  • github_repository
  • Potentially every resource or data source using the new REST client, because the defect is in the shared internal/ghclient transport

Terraform Configuration Files

terraform {
  required_providers {
    github = {
      source  = "integrations/github"
      version = "6.13.0"
    }
  }
}

provider "github" {
  owner         = var.github_owner
  legacy_client = false
  max_retries   = 0

  app_auth {
    id              = var.github_app_id
    installation_id = var.github_app_installation_id
    pem_file        = var.github_app_pem
  }
}

resource "github_repository" "existing" {
  for_each = toset(var.existing_repository_names)

  name = each.value
}

The production configuration uses modules, but the failure occurs in the provider's shared HTTP transport rather than module evaluation.

Steps to Reproduce

A deterministic unit test can reproduce the leak without contacting GitHub.

Add the following test under internal/ghclient at tag v6.13.0:

package ghclient

import (
	"io"
	"net/http"
	"strings"
	"testing"

	"github.com/google/go-github/v88/github"
	"golang.org/x/sync/semaphore"
)

type notFoundTransport struct{}

func (t *notFoundTransport) RoundTrip(req *http.Request) (*http.Response, error) {
	return &http.Response{
		Status:     "404 Not Found",
		StatusCode: http.StatusNotFound,
		Header:     make(http.Header),
		Body:       io.NopCloser(strings.NewReader(`{"message":"Not Found"}`)),
		Request:    req,
	}, nil
}

func TestThrottlerReleasesPermitAfterErrorResponse(t *testing.T) {
	sema := semaphore.NewWeighted(1)

	client, err := github.NewClient(
		github.WithTransport(&throttler{
			sema:  sema,
			inner: &notFoundTransport{},
		}),
		github.WithDisableRateLimitCheck(),
	)
	if err != nil {
		t.Fatal(err)
	}

	req, err := client.NewRequest(
		t.Context(),
		http.MethodGet,
		"repos/example/missing",
		nil,
	)
	if err != nil {
		t.Fatal(err)
	}

	if _, err := client.Do(req, nil); err == nil {
		t.Fatal("expected GitHub API error")
	}

	if !sema.TryAcquire(1) {
		t.Fatal("semaphore permit leaked after non-2xx response")
	}
	sema.Release(1)
}

Run:

go test ./internal/ghclient \
  -run TestThrottlerReleasesPermitAfterErrorResponse \
  -count=1

The test fails on v6.13.0 with:

semaphore permit leaked after non-2xx response

A Terraform-level reproduction is:

  1. Configure integrations/github v6.13.0 with legacy_client = false.
  2. Manage or refresh more than 100 repositories for which the vulnerability-alerts check returns 404.
  3. Run TF_LOG=DEBUG terraform plan.
  4. Observe that the provider initially refreshes resources successfully.
  5. After the semaphore permits are exhausted, subsequent API operations stop reaching the network.
  6. After five minutes, blocked requests begin returning Client.Timeout exceeded while awaiting headers.
  7. Repeat with legacy_client = true and parallel_requests = true; the plan completes.

Debug Output

[ERROR] vertex "github_repository.example" error:
Get "https://api.github.com/repos/example-org/example-repository":
(http.RoundTripper).RoundTrip failed:
context deadline exceeded
(Client.Timeout exceeded while awaiting headers)

At the time of the timeout, the provider process remained alive but had no TCP connection corresponding to the blocked request. Direct API requests from the same runner continued to succeed.

Additional Context

The new client and explicit concurrency control were introduced in v6.13.0. The previous release, v6.12.1, did not contain internal/ghclient, the 100-permit semaphore, or the legacy_client option.

One possible correction would be for throttlerReadCloser to release its permit when reading reaches io.EOF as well as on Close, using the existing sync.Once. Another option is to ensure that the original wrapped response body is explicitly closed before go-github replaces it. The key requirement is that all response paths, including non-2xx responses, return the acquired permit exactly once.

Code of Conduct

  • I agree to follow this project's Code of Conduct

Metadata

Metadata

Assignees

Labels

Type: BugSomething isn't working as documented

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions