-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Use wildcard for github domains validation #3300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
EnricoMi
left a comment
There was a problem hiding this 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.
Return None if hostname is None Co-authored-by: Enrico Minack <github@enrico.minack.dev>
…icmethod and is tested separately, split github.com and githubusercontent.com checks
|
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! |
EnricoMi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Test Results 8 files ± 0 8 suites ±0 4m 44s ⏱️ -14s For more details on these failures, see this check. Results for commit 71f404a. ± Comparison against base commit 2d4785d. |
|
My suggestion above should fix the tests. |
Co-authored-by: Enrico Minack <github@enrico.minack.dev>
Head branch was pushed to by a user without write access
|
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. |
|
Fixed in #3329. Thanks for your help! |
Github recently added a new subdomain
release-assetsto theirgithubusercontent.comcreating an issue when validating the incoming requests.This PR updates the validation on the domain name instead of the full subdomain.domain
Message from github:
Fixes #3315.