Add Domain.maybe_wildcard? to detect possible wilcard DNS records #203
+64
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use a subdomain with more randomness Return an error when a wildcard record is suspected Some affordances for unit tests in maybe_wildcard? also check that the wildcard leads back to GitHub lint Don't include wildcard warnings as an error wildcard_warning method to generate an error
There was a problem hiding this comment.
Pull Request Overview
Adds detection for potential wildcard DNS records in custom domains, warning users when a wildcard could allow unintended Pages subdomains.
- Introduces
Domain#maybe_wildcard?andDomain#wildcard_warningto detect and surface wildcard-record risks. - Adds a new
WildcardRecordErrorclass and updates the total error count in specs. - Bumps the
addressablegem dependency to~> 2.8.7.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| spec/github_pages_health_check/errors_spec.rb | Update expected error count to include the new wildcard error |
| lib/github-pages-health-check/errors/wildcard_record_error.rb | Add WildcardRecordError for wildcard DNS record warnings |
| lib/github-pages-health-check/domain.rb | Implement parent_domain, maybe_wildcard?, and wildcard_warning |
| github-pages-health-check.gemspec | Bump addressable dependency to ~> 2.8.7 |
Comments suppressed due to low confidence (2)
spec/github_pages_health_check/errors_spec.rb:7
- [nitpick] The spec currently only checks the total number of errors, which can be brittle. Consider adding a dedicated test to verify that
WildcardRecordErroris included and that itsmessageis as expected.
expect(GitHubPages::HealthCheck::Errors.all.count).to eql(11)
lib/github-pages-health-check/errors/wildcard_record_error.rb:17
- Consider using a
<<~MSGheredoc instead of<<-MSGto automatically strip indentation and avoid leading whitespace in the error message.
<<-MSG
tsusdere
approved these changes
Jun 5, 2025
| @maybe_wildcard = begin | ||
| wildcard_resolver = GitHubPages::HealthCheck::Resolver.new(sibling_domain, :nameservers => nameservers) | ||
|
|
||
| [Dnsruby::Types::A, Dnsruby::Types::AAAA].any? do |record_type| |
There was a problem hiding this comment.
Do we also want to check for CNAME and MX? Or is that something we don't worry about because it's a Pages domain?
There was a problem hiding this comment.
For this purpose, all that matters is whether the IP points to github, so just A and AAAA are sufficient.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Domain#maybe_wildcard?generates a random sibling domain and queries it, then checks if it returns an answer and if it points to github pages. If so, then it's reasonable to suspect that the record which points the domain to github pages is a wildcard record.This is worth warning about because a wildcard DNS record will allow anyone to host a pages site at a subdomain of the wildcard record. GitHub allows us to verify ownership of our domains, but it only works for the verified domain and direct subdomains (a.example.com, b.example.com), not subdomains arbitrarily many segments long (a.b.c.d.e.f.example.com).