fix: Avoid auth on cross-origin redirects

[huynhtrungcsc]

Summary

• Avoid adding BasicAuthTransport credentials on cross-origin redirect requests.
• Avoid adding UnauthenticatedRateLimitedTransport credentials on cross-origin redirect requests.
• Add regression coverage for both built-in auth transports.

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.51%. Comparing base (90075ee) to head (c5be7ac).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4364   +/-   ##
=======================================
  Coverage   97.51%   97.51%           
=======================================
  Files         193      193           
  Lines       19519    19543   +24     
=======================================
+ Hits        19033    19057   +24     
  Misses        269      269           
  Partials      217      217           

☔ 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.