Repository navigation
fix(middleware): make crawler detection better and isolate bot traffic - #949
Open
thostetler wants to merge 6 commits into
Open
thostetler wants to merge 6 commits into
thostetler wants to merge 6 commits into
Conversation
Bundle sizeShared by all pages: 627.8 kB (+0.1 kB) ⚪ No route changed by more than 1 kB. ✅ First load = polyfills + shared |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #949 +/- ##
========================================
+ Coverage 55.2% 56.1% +1.0%
========================================
Files 378 383 +5
Lines 11618 11841 +223
Branches 2554 2606 +52
========================================
+ Hits 6406 6639 +233
+ Misses 4574 4559 -15
- Partials 638 643 +5
🚀 New features to boost your workflow:
|
thostetler
changed the title
thostetler
marked this pull request as ready for review
October 8, 2026 17:58
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Bot isolation and shared-credential handling still have unresolved correctness and security issues.
4 open findings
What changed in this PR
This security-sensitive PR improves crawler verification and separates bot traffic from normal user sessions.
Changes:
- Adds broader crawler detection, trusted-IP handling, signed checks, and corrected DNS verification.
- Persists dedicated bot sessions while withholding shared credentials from client responses.
- Adds automated IP-range refreshes and regression coverage.
| File | Description |
|---|---|
| vitest.config.js | Excludes build output from tests. |
| src/types/global.d.ts | Types the trusted-IP header setting. |
| src/ssr-utils.ts | Omits bot credentials from client state. |
| src/pages/api/user.ts | Withholds bot credentials from responses. |
| src/pages/api/isBot.ts | Authenticates checks and delegates verification. |
| src/pages/api/auth/logout.ts | Clears the bot-session flag. |
| src/pages/api/auth/login.ts | Clears the bot-session flag. |
| src/middlewares/reverseDns.ts | Adds forward-confirmed reverse DNS. |
| src/middlewares/initSession.ts | Persists bot sessions without bootstrap. |
| src/middlewares/crawlers.ts | Centralizes crawler classification. |
| src/middlewares/crawler-ranges.generated.json | Stores published crawler IP ranges. |
| src/middlewares/clientIp.ts | Distinguishes trusted IP sources. |
| src/middlewares/cidr.ts | Implements IP parsing and range matching. |
| src/middlewares/botCheckSignature.ts | Signs and validates bot checks. |
| src/middlewares/botCheck.ts | Selects expiring bot-tier credentials. |
| src/middlewares/__tests__/reverseDns.test.ts | Tests DNS verification. |
| src/middlewares/__tests__/initSession.integration.test.ts | Tests bot-session transitions. |
| src/middlewares/__tests__/crawlers.test.ts | Tests crawler classification. |
| src/middlewares/__tests__/crawler-ranges.test.ts | Validates range snapshots and freshness. |
| src/middlewares/__tests__/crawler-breadth.test.ts | Measures crawler detection coverage. |
| src/middlewares/__tests__/clientIp.test.ts | Tests IP-source trust handling. |
| src/middlewares/__tests__/cidr.test.ts | Tests address parsing and matching. |
| src/middlewares/__tests__/botCheck.test.ts | Tests bot checks and token selection. |
| src/__tests__/ssr-utils.test.ts | Tests credential omission during serialization. |
| src/__tests__/api-isBot.test.ts | Tests signed requests and classification. |
| scripts/fetch-crawler-ranges.mjs | Refreshes published IP ranges. |
| pnpm-lock.yaml | Locks added dependencies. |
| package.json | Adds detection and test dependencies. |
| .github/workflows/crawler-ranges.yml | Schedules range-refresh pull requests. |
| .env.local.sample | Adds trusted-IP header configuration. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| const groupCount = head.length + tail.length; | ||
| const trailingGroups = trailing === null ? 0 : 2; | ||
| if (halves.length === 2 ? groupCount + trailingGroups > 8 : groupCount + trailingGroups !== 8) { |
| const botToken = await botCheck(req); | ||
|
|
||
| if (botToken) { | ||
| session.token = botToken; |
| return res.json({ | ||
| isAuthenticated: session.isAuthenticated, | ||
| user: session.token, | ||
| user: session.bot ? undefined : session.token, |
|
|
||
| let ptrRecords: readonly string[]; | ||
| try { | ||
| ptrRecords = await lookupPtr(remoteIp); |
Crawler user agents were matched by exact map lookup against the whole UA string, so no real crawler ever matched. Fixing that exposed three more defects: IP checks compared bare addresses instead of CIDR ranges, reverse DNS accepted any host with a PTR record, and classification was discarded by a second iron-session write. Verified-tier classification now requires TRUSTED_CLIENT_IP_HEADER, since X-Forwarded-For's leftmost entry is client-supplied. Without that header, matched crawlers are capped at the unverifiable tier instead of getting an elevated-quota token.
/api/isBot sits outside the middleware matcher, so it was reachable unauthenticated and would drive up to six resolver lookups per request on a caller-supplied IP. The hop is now signed with an HMAC over the timestamp, IP and user agent, keyed on COOKIE_SECRET, and the handler rejects anything unsigned, stale or altered before touching DNS. The response status is checked too, so a rejected hop no longer reads a 403 body as a HUMAN verdict. A successful bootstrap also clears session.bot. Without it a session that was once classified as a bot kept the flag after its token expired and was replaced by a human one, and isUserIdentifiedAsBot then skipped the apiCookieHash comparison for the rest of that token's life.
A zero run must stand for at least one group, but the parser also accepted eight explicit groups alongside it. 0:0:0:0:0:ffff:14ab:ce07:: and 0:0:0:0:0:ffff::20.171.206.7 both resolved to 20.171.206.7 and matched a published GPTBot prefix.
parseIp collapses an IPv4-mapped address to four bytes, which is what selects the A record family, but the PTR lookup still received the original form and queried ip6.arpa. A legitimate crawler arriving IPv4-mapped therefore failed verification and was handed the malicious tier. The existing tests mocked the lookup to resolve regardless of its argument, so they did not catch it.
The session and user-data debug payloads carried access_token and neither logger redacted it. Bot tier tokens are shared across all crawler traffic and rotate only on redeploy, so the exposure matters more than it would for a per-session token. Both call sites now log summaries rather than the objects, because pino redaction is inert in browser mode and that is the mode the edge logger runs in. Redaction is added to the Node logger, where it works, covering paths not enumerated here.
Withholding the bot token from the client still left a direct browser call to the bootstrap endpoint, so a crawler that executes JavaScript picked up an ordinary anonymous token and left the bot tier for every client-side request. The session endpoint now reports the classification and the client stops negotiating once it knows. Client-side queries fail for those sessions instead of succeeding on the anonymous quota.
thostetler
force-pushed
the
fix/bot-detection-ua-match
branch
from
October 14, 2026 16:11
7c5e0c6 to
9aa1cd6
Compare
This branch has not been deployed
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
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.
Crawler user agents were matched by exact map lookup against the whole UA string, which was probably making some bots not match