Skip to content

Conversation

@lpbedard
Copy link
Contributor

@lpbedard lpbedard commented Jun 13, 2025

Github recently added a new subdomain release-assets to their githubusercontent.com creating an issue when validating the incoming requests.

15.79 │ │        │                                                                 │ │
15.79 │ │        query='sp=r&sv=2018-11-09&sr=b&spr=https&se=2025-06-12T20%3A29%3… │ │
15.79 │ │        │   fragment=''                                                   │ │
15.79 │ │        )                                                                 │ │
15.79 │ │ self = <github.Requester.Requester object at 0xffffaed27bc0>             │ │
15.79 │ │  url = 'https://release-assets.githubusercontent.com/github-production-… │ │
15.79 │ ╰──────────────────────────────────────────────────────────────────────────╯ │
15.79 ╰──────────────────────────────────────────────────────────────────────────────╯
15.79 AssertionError: release-assets.githubusercontent.com
------

This PR updates the validation on the domain name instead of the full subdomain.domain

Message from github:

Thank you for reaching out to GitHub Support!
I checked in with our engineering teams about this.
That’s correct, we have recently turned on a feature flag that adds the address release-assets.githubusercontent.com for release assets.
This change comes a part of some internal service updates and is planned to be permanent.
The *.githubusercontent.com address is listed as a wildcard in the GitHub META API Endpoint and depending on the activity performed you may see connections to many different githubusercontent.com subdomains, with some of them being new from time to time.

Fixes #3315.

Copy link
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

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

I think this is a great improvement which voids future work when new sub-domains appear.

@EnricoMi EnricoMi changed the title fix: use wildcard for github domains validation Use wildcard for github domains validation Jul 15, 2025
lpbedard and others added 2 commits July 15, 2025 11:15
Return None if hostname is None

Co-authored-by: Enrico Minack <github@enrico.minack.dev>
lpbedard added 2 commits July 16, 2025 15:05
…icmethod and is tested separately, split github.com and githubusercontent.com checks
@tomatoprinx
Copy link

Hi, just wanted to check in on this PR. Will it be merged and included in the next release? Also, do you have an estimated timeline for that release? Thanks!

@lpbedard lpbedard requested a review from EnricoMi July 22, 2025 18:11
Copy link
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

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

LGTM!

@EnricoMi EnricoMi enabled auto-merge July 30, 2025 16:40
@github-actions
Copy link

Test Results

     8 files  ± 0       8 suites  ±0   4m 44s ⏱️ -14s
 1 002 tests + 2     996 ✅  -  4  0 💤 ±0   6 ❌ + 6 
11 091 runs  +16  11 042 ✅  - 32  1 💤 ±0  48 ❌ +48 

For more details on these failures, see this check.

Results for commit 71f404a. ± Comparison against base commit 2d4785d.

@EnricoMi
Copy link
Collaborator

My suggestion above should fix the tests.

Co-authored-by: Enrico Minack <github@enrico.minack.dev>
auto-merge was automatically disabled August 4, 2025 13:40

Head branch was pushed to by a user without write access

@EnricoMi
Copy link
Collaborator

EnricoMi commented Aug 8, 2025

I took another attempt to fix this: #3329.

I think the code got messed up by an earlier commit of mine where I misinterpreted the semantics of this code. I think it makes sense to move the logic into a separate method.

I have kept your changes to account for your contribution.

@EnricoMi
Copy link
Collaborator

Fixed in #3329. Thanks for your help!

@EnricoMi EnricoMi closed this Aug 25, 2025
EnricoMi added a commit that referenced this pull request Aug 25, 2025
Cleans redirection URL check that got extended in the wrong way by #3182.

Supersedes (and includes for contribution attribution) #3300. Fixes #3315.

---------

Co-authored-by: Louis-Philippe Bedard <lpbedard@coveo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Downloading release assets no longer working

4 participants