fix!: Scope auth transports to allowed origins

[huynhtrungcsc]

Summary

• Scope BasicAuthTransport and UnauthenticatedRateLimitedTransport credentials to AllowedOrigins.
• Default the allowlist to the public GitHub API and upload origins.
• Suppress Authorization and X-GitHub-OTP outside the allowlist.
• Cover direct absolute URLs and multi-hop redirect chains.

Fixes #4365

[alexandear]

Could you create an issue first with the problem you're trying to resolve, and once we've confirmed it, we can proceed with a review?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.53%. Comparing base (ed10621) to head (34049a2).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4364   +/-   ##
=======================================
  Coverage   97.52%   97.53%           
=======================================
  Files         193      193           
  Lines       19668    19695   +27     
=======================================
+ Hits        19182    19209   +27     
+ Misses        269      268    -1     
- Partials      217      218    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[huynhtrungcsc]

Thanks for the guidance. I opened an issue describing the behavior this PR addresses:

• Auth transports add credentials after cross-origin redirects #4365

I also added focused coverage for the redirect-origin helper so the new behavior is covered more directly. I’m happy to adjust the PR based on the maintainers’ preferred direction.

[gmlewis]

Thank you, @huynhtrungcsc!
LGTM with the same comments I made in #4363.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @alexandear - @Not-Dhananjay-Mishra

[gmlewis]

Since you added defaultPort (is this function really necessary?!?), your code coverage went down, so if you want to keep it, it needs its own unit test.

[huynhtrungcsc]

Added direct unit coverage for defaultPort in c5be7ac.

Local verification:

• go test ./github -run "TestIsCrossOriginRedirect|TestSameRedirectOrigin|TestDefaultPort|TestUnauthenticatedRateLimitedTransport_doesNotAuthorizeCrossOriginRedirect|TestBasicAuthTransport_doesNotAuthorizeCrossOriginRedirect" -count=1
• go test ./github -coverprofile=/tmp/go-github-4364.cover -count=1 now reports 100% coverage for isCrossOriginRedirect, sameRedirectOrigin, and defaultPort.

[stevehipwell]

I have the same issue with this as I do with #4363, this need further discussion around the actual context rather than a rushed PR.

[huynhtrungcsc]

Understood. I agree this belongs in the same broader discussion as #4363 before changing transport behavior.

I will pause further implementation changes here as well and use #4365 to clarify the expected redirect/auth boundary first. Once that direction is settled, I can update this PR accordingly or close it if maintainers prefer a different approach.

[JamBalaya56562]

No action needed while this is paused — just noting the part I think has to change once #4366 settles:

isCrossOriginRedirect compares req.URL against req.Response.Request.URL and returns false when req.Response is nil, so the guard only fires on a redirect. A direct absolute-URL request through BasicAuthTransport or UnauthenticatedRateLimitedTransport still gets credentials attached — the same leak as #4366, via a different transport.

Making it origin-based rather than hop-based needs the configured origins, which neither transport can see (both are user-constructed via github.NewClient(tp.Client()) and neither struct has a URL field). That likely means an explicit allowed-origins field on both — new public API, so possibly its own PR rather than a change here.

[huynhtrungcsc]

Reworked the transport guard to check every request against a fixed origin allowlist instead of comparing redirect hops. This covers direct absolute URLs and multi-hop redirect chains, and suppresses Basic auth, OTP, and OAuth app credentials outside AllowedOrigins.

Validated with script/fmt.sh, script/test.sh, and script/lint.sh.