Skip to content

Add JIT access fallback for GitHub API requests - #176

Open
thavaahariharangit wants to merge 3 commits into
mainfrom
harry/fix-github-api-jit-access-fallback
Open

Add JIT access fallback for GitHub API requests#176
thavaahariharangit wants to merge 3 commits into
mainfrom
harry/fix-github-api-jit-access-fallback

Conversation

@thavaahariharangit

@thavaahariharangit thavaahariharangit commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What are you trying to accomplish?

Add JIT access fallback support to GitHubAPIHandler.

When static credentials cannot access a repository and GitHub returns 401, 403, or 404, the handler now requests a repository-scoped JIT credential and retries the API request. This allows Dependabot to retrieve metadata, such as release information, from internal repositories permitted by the organization’s repository access policy.

Successful JIT credentials are cached for subsequent API requests to the same repository.

Anything you want to highlight for special attention from reviewers?

The implementation follows the existing JIT authentication pattern used by GitServerHandler:

  • Static credentials are attempted first.
  • JIT access is requested only after authentication fails.
  • JIT credentials are scoped and cached per repository.
  • JIT fallback only runs for requests that the proxy authenticated with static credentials.
  • The existing API client is shared with GitHubAPIHandler.
    Reviewers should pay particular attention to treating 404 as a potential authentication failure, since GitHub returns it when an authenticated token cannot access a private or internal repository.

How will you know you've accomplished your goal?

Tests verify that:

  • A GitHub API release request returning 404 falls back to JIT authentication.
  • The request succeeds when retried with the JIT credential.
  • Requests without matching static credentials do not trigger JIT fallback.
  • Cached JIT credentials are reused without requesting another token.
  • Existing GitHub API authentication behavior remains unchanged.

The full test suite passes with:

go test ./... -count=1

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

Rollout and failure behavior

  • GitHub API JIT fallback is disabled by default and can be enabled with PROXY_GITHUB_API_JIT_ACCESS=true.
  • Fallback remains limited to requests authenticated by the proxy.
  • GitHub API 404 responses remain potential authentication failures because private repositories can be hidden behind a 404; when enabled, an ordinary 404 can therefore cause one JIT attempt for that repository.
  • Repositories that exhaust all authentication paths are negatively cached so subsequent API requests do not repeat JIT access requests.

@thavaahariharangit
thavaahariharangit requested a review from a team as a code owner July 30, 2026 08:13
Copilot AI review requested due to automatic review settings July 30, 2026 08:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds JIT access fallback support to GitHubAPIHandler so GitHub API GET/HEAD requests that fail with 401/403/404 can be retried using repository-scoped JIT credentials (requested via the existing API client) and then cached for later requests.

Changes:

  • Extend GitHubAPIHandler to parse jit_access credentials and request repo-scoped JIT tokens after auth failures, retrying the API request.
  • Wire the shared API client into GitHubAPIHandler from proxy.go.
  • Add tests covering 404-triggered JIT fallback, JIT-only configs, and JIT token caching behavior.
Show a summary per file
File Description
proxy.go Passes the shared API client into GitHubAPIHandler so it can request JIT access.
internal/handlers/github_api.go Adds JIT access configuration, fallback retry logic, and JIT token caching into the existing credential store.
internal/handlers/github_api_test.go Adds test coverage for JIT fallback behavior and caching.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment thread internal/handlers/github_api.go
Comment thread internal/handlers/github_api.go Outdated
Copilot AI review requested due to automatic review settings July 30, 2026 08:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review details

Comments suppressed due to low confidence (1)

internal/handlers/github_api.go:162

  • The JIT retry always sets an Authorization: token ... header and does not clear any existing auth headers. If JIT ever returns the Proxima service identity (or if the original request used X-GitHub-PSI-JWT), the immediate retry will send the wrong/ambiguous auth headers and may not actually exercise the JIT credential.

Mirror the existing credential handling logic here: clear both auth headers on the cloned request and choose between X-GitHub-PSI-JWT vs Authorization based on jitCreds.username.

		newReq := ctx.Req.Clone(ctx.Req.Context())
		logging.RequestLogf(ctx, "* auth'd github api request failed authentication, retrying with jit access auth")
		newReq.Header.Set("Authorization", "token "+jitCreds.password)
		newRsp, err := ctx.RoundTrip(newReq)
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Low

@honeyankit

Copy link
Copy Markdown
Contributor

@thavaahariharangit Helm smoke test is failing.

@jurre

jurre commented Jul 30, 2026

Copy link
Copy Markdown
Member

I'm digging through wether this opens up any access to resources/endpoints that we would otherwise expect to be shielded off for users. I think it's probably fine, but I want to make sure that I fully understand how this changes the access that Dependabot gets to these private repositories.

@jurre jurre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we put all of this behind a feature flag so we can test it in isolation, roll it out to customers slowly and revert it quickly if needed?

@jurre jurre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Commented by Copilot from a review-session with Jurre.

The JIT plumbing mirrors GitServerHandler closely and the happy-path tests are solid. My concerns are all about what happens when JIT fails rather than when it succeeds. GitServerHandler guards against retry storms with reposAlreadyTried; this handler has no equivalent, and combined with treating 404 as an auth failure that turns routine GitHub API 404s into JIT round trips.

Comment thread internal/handlers/github_api.go
Comment thread internal/handlers/github_api.go
Comment thread internal/handlers/github_api.go Outdated
Comment thread internal/handlers/github_api.go Outdated
Comment thread internal/handlers/github_api.go Outdated
Comment thread internal/handlers/github_api_test.go
Copilot AI review requested due to automatic review settings July 31, 2026 07:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review details

Suppressed comments (1)

internal/handlers/github_api.go:154

  • In HandleResponse, the code uses ctx.Req.BasicAuth() to avoid retrying the same credentials, but GitHubAPIHandler authenticates via the Authorization/X-GitHub-PSI-JWT headers (not BasicAuth). As a result, the first retry can repeat the exact same token/identity, and retries can also end up sending both Authorization and X-GitHub-PSI-JWT on the same request because the alternate-auth loop sets one header without clearing the other. This can cause ambiguous auth precedence and unnecessary extra round-trips.

Consider detecting the original auth from headers (or storing the selected credential in ctxdata like GitServerHandler does) and always clearing both headers before setting the new auth.

	username, password, reqWasAuthed := ctx.Req.BasicAuth()
	for _, creds := range getCredentialsForRequest(ctx.Req, h.credentials, gitHubAPIExtractOrgAndRepo) {
		// don't retry the request with the same auth that was previously used
		if reqWasAuthed && creds.username == username && creds.password == password {
			continue
		}

		newReq := ctx.Req.Clone(ctx.Req.Context())
		if creds.username == reservedProximaIdentity {
			logging.RequestLogf(ctx, "* auth'd github api request failed authentication, retrying with alternate identity")
			newReq.Header.Set("X-GitHub-PSI-JWT", creds.password)
		} else {
			logging.RequestLogf(ctx, "* auth'd github api request failed authentication, retrying with alternate provided auth")
			newReq.Header.Set("Authorization", "token "+creds.password)
		}
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants