Conversation
- Add TestIntegration_Cloudflare for IPv4 IP list - Add TestIntegration_GoogleSpecial for special crawlers - Update meta-externalagent to use RDNS (no public IP list available) - Remove TestIntegration_MetaExternalAgent (no valid URL)
- Add tfbnw.net and facebook.com to verified domains - Meta crawlers use multiple domain suffixes
- mj12bot: mj12bot.com (not majestic.com) - yandexbot: yandex.net - semrushbot: bl.bot.semrush.com - semrushbot-ba: bl.bot.semrush.com - sogou: crawl.sogou.com - pinterestbot: pinimg.com - google-storebot: googlebot.com, rate-limited-proxy.google.com - chrome-lighthouse: googlebot.com, rate-limited-proxy.google.com - linkedinbot: fwd.linkedin.com - chatgpt-user: chat.openai.com
Contributor
Reviewer's GuideAdds new integration tests for Cloudflare IP ranges and Google special crawlers, and tightens multiple bot verification configurations by updating RDNS/domain rules to better match official sources. Sequence diagram for updated bot RDNS and domain verificationsequenceDiagram
actor Client
participant WebServer
participant BotDetector
participant RDNSResolver
participant BotConfigStore
Client->>WebServer: HTTP request with UserAgent and IP
WebServer->>BotDetector: Classify request
BotDetector->>BotConfigStore: Load bot config for UserAgent
BotConfigStore-->>BotDetector: Config with rdns flag and domains list
alt rdns is true
BotDetector->>RDNSResolver: Reverse DNS lookup for IP
RDNSResolver-->>BotDetector: Hostname
BotDetector->>BotDetector: Check hostname ends with allowed domains
alt hostname matches allowed domains
BotDetector-->>WebServer: Mark as verified bot
else hostname does not match
BotDetector-->>WebServer: Mark as unverified bot
end
else rdns is false
BotDetector-->>WebServer: Use other parsing logic
end
WebServer-->>Client: Response with bot-aware handling
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new integration tests treat network issues differently (Cloudflare fails the test while GoogleSpecial skips on error); consider making their behavior consistent so that transient network problems don’t cause flaky test runs.
- Both new tests convert the downloaded bytes to string and back into a reader; you can avoid this extra allocation by passing bytes.NewReader(data) directly to the parser.
- Several bot configs now share very similar rdns domain lists (e.g., google-related domains for Chrome-Lighthouse and GoogleStoreBot); if the config system allows it, consider centralizing or reusing these to reduce duplication and drift over time.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new integration tests treat network issues differently (Cloudflare fails the test while GoogleSpecial skips on error); consider making their behavior consistent so that transient network problems don’t cause flaky test runs.
- Both new tests convert the downloaded bytes to string and back into a reader; you can avoid this extra allocation by passing bytes.NewReader(data) directly to the parser.
- Several bot configs now share very similar rdns domain lists (e.g., google-related domains for Chrome-Lighthouse and GoogleStoreBot); if the config system allows it, consider centralizing or reusing these to reduce duplication and drift over time.
## Individual Comments
### Comment 1
<location> `bots/conf.d/meta-externalagent.yaml:6-10` </location>
<code_context>
rdns: true
domains:
- "openai.com"
</code_context>
<issue_to_address>
**question (bug_risk):** Replacing parser/url-based validation with rdns+domain-only checks may broaden what gets classified as Meta-ExternalAgent.
The previous implementation relied on a canonical JSON definition via the `google` parser, which tightly constrained the IP ranges considered valid. Switching to `rdns: true` with these domains removes that single source of truth and may classify a wider set of IPs as Meta-ExternalAgent if those domains are reused elsewhere. Please confirm that this broader rdns-based behavior is intended, or consider retaining the JSON-based definition (e.g., as a fallback) to keep the scope constrained.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12 +/- ##
==========================================
- Coverage 72.76% 71.85% -0.92%
==========================================
Files 15 17 +2
Lines 661 732 +71
==========================================
+ Hits 481 526 +45
- Misses 136 152 +16
- Partials 44 54 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Benchmark Results |
- Removed domains that are covered by shorter suffixes: - rate-limited-proxy.google.com (covered by google.com) - fwd.linkedin.com (covered by linkedin.com) - chat.openai.com (covered by openai.com) - bl.bot.semrush.com (covered by semrush.com) - crawl.sogou.com (covered by sogou.com)
- Use googlebot.json for IP range verification (faster than RDNS) - Update parser to google style - Add official documentation reference
- Use t.Fatalf for network errors in both Cloudflare and GoogleSpecial tests - Use bytes.NewReader instead of strings.NewReader to avoid extra allocation
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by Sourcery
Update bot configuration domains and add new integration tests for external IP sources.
Bug Fixes:
Enhancements:
Tests: