diff --git a/CHANGELOG.md b/CHANGELOG.md index 2954449d23..b2f610e8ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -232,6 +232,18 @@ aggregate instead: an italic *Catalog* line at the end of the version section an ### Fixed +- **The bot analytics recorded nothing at all** — Plausible identifies crawler user agents and + discards their events, so forwarding the real one guaranteed an empty dashboard. Verified against + the live API: the same event sent as `Claude-User` never appears, sent as a browser agent it does, + and both are acknowledged with `202 ok` — Plausible always accepts, then drops. There is no + documented bypass, so machine-side events now travel under a neutral agent; nothing is lost, + because the user agent only feeds browser/OS detection, which is meaningless for a crawler, while + the identity rides in the `assistant` and `kind` properties. A second silent drop was found in the + same pass: the [Events API docs](https://plausible.io/docs/events-api) state that forwarding an + infrastructure address rather than the visitor's makes Plausible discard the event, and analytics + was reusing the rate limiter's IP resolver — which deliberately returns the *rightmost* forwarded + entry, ours. Analytics now has its own `visitor_ip`, and `api/request_context.py` records why the + two must not be merged (#10477). - **The daily bot-serving monitor had been red for ten days** — it greps for an exact home-page title, the copy changed to "anyplot.ai — AI-generated plot catalog for 15 libraries", and every scheduled run since 2026-08-09 failed on that one line. Nothing else was wrong, and nobody looked: diff --git a/api/analytics.py b/api/analytics.py index 88982f9594..4a74740073 100644 --- a/api/analytics.py +++ b/api/analytics.py @@ -13,7 +13,7 @@ import httpx from fastapi import Request -from api.request_context import client_ip as resolve_client_ip +from api.request_context import visitor_ip logger = logging.getLogger(__name__) @@ -28,6 +28,17 @@ # numbers clean while still answering "how often does an assistant read this". BOT_DOMAIN = "bots.anyplot.ai" +# Plausible discards events whose User-Agent it recognises as a bot, and it +# recognises all of them — verified against the live API: the same event sent +# as `Claude-User` never appears, sent as a browser UA it does. Forwarding the +# real crawler UA therefore guarantees the bot site records nothing at all. +# +# So machine-side events are sent under a neutral agent. Nothing is lost: the +# UA only feeds Plausible's browser/OS/device detection, which is meaningless +# for a crawler, while the identity that matters travels in the `assistant` and +# `kind` properties either way. +BOT_SENDER_UA = "anyplot-server/1.0" + # Which assistant, and on whose behalf. The distinction is the point: a # user-directed fetch means a person asked their assistant to open this page, # which is a reader; an index crawler is building a corpus with no one waiting. @@ -195,11 +206,14 @@ async def _send_plausible_event( domain: Plausible site to record against; BOT_DOMAIN keeps AI traffic out of the human numbers """ + # Events for the bot site travel under a neutral agent: Plausible drops + # anything it identifies as a bot, which is every UA this path carries. + sender_ua = BOT_SENDER_UA if domain == BOT_DOMAIN else user_agent try: async with httpx.AsyncClient(timeout=5.0) as client: await client.post( PLAUSIBLE_ENDPOINT, - headers={"User-Agent": user_agent, "X-Forwarded-For": client_ip, "Content-Type": "application/json"}, + headers={"User-Agent": sender_ua, "X-Forwarded-For": client_ip, "Content-Type": "application/json"}, json={"name": name, "url": url, "domain": domain, "props": props}, ) except Exception as e: @@ -241,7 +255,7 @@ def track_og_image( filters: Query params for filtered home page (e.g., {'lib': 'plotly', 'dom': 'statistics'}) """ user_agent = request.headers.get("user-agent", "") - client_ip = resolve_client_ip(request) + client_ip = visitor_ip(request) platform = detect_platform(user_agent) # Build URL based on page type. Spec routes follow /{spec}/{language}/{library}. @@ -316,13 +330,11 @@ def track_bot_fetch(request: Request, path: str) -> None: return assistant, kind = detected - # Resolve through the shared helper rather than reading the raw header: - # x-forwarded-for is a comma-separated chain once more than one proxy has - # appended to it, so the raw value is neither a valid single IP for - # Plausible nor the right one for geolocation. The helper also prefers - # cf-connecting-ip and takes the rightmost entry, which is the one a - # client cannot forge. - client_ip = resolve_client_ip(request) + # visitor_ip, not the rate limiter's client_ip: Plausible documents that it + # drops events carrying an infrastructure address rather than the real + # visitor's, and the rate limiter deliberately returns the rightmost + # forwarded entry — ours. See api/request_context.py for why the two differ. + client_ip = visitor_ip(request) props = {"assistant": assistant, "kind": kind, "path": path} url = f"https://anyplot.ai{path}" diff --git a/api/request_context.py b/api/request_context.py index 8a3ebba296..a77c154327 100644 --- a/api/request_context.py +++ b/api/request_context.py @@ -1,5 +1,7 @@ """Request-scoped helpers shared across routers.""" +import ipaddress + from fastapi import Request @@ -30,3 +32,51 @@ def client_ip(request: Request) -> str: if entry.strip(): return entry.strip() return request.client.host if request.client else "" + + +def visitor_ip(request: Request) -> str: + """Resolve the IP to report to analytics — deliberately not `client_ip`. + + The two answer opposite questions and must not be merged. + + `client_ip` keys rate limiting, so it takes the RIGHTMOST forwarded entry: + the leftmost is client-controlled, and trusting it let a caller poison + another user's bucket. Analytics needs the opposite — Plausible documents + that it uses "the first valid IP address from the list" and that "if you + forward a server, hosting provider, or CDN IP address instead of the actual + visitor IP, Plausible's bot filtering will drop the event". Handing it the + rightmost entry means handing it our own infrastructure's address, and the + event is silently discarded. + + Spoofing is not a concern in this direction: a forged value skews a + geolocation bucket, where forging the rate-limit key locked people out. + + Order: `cf-connecting-ip`, which Cloudflare overwrites on proxied traffic + and is therefore both real and unforgeable; then the leftmost non-empty + forwarded entry; then the socket peer. + """ + cf_ip = request.headers.get("cf-connecting-ip", "").strip() + if _is_ip(cf_ip): + return cf_ip + for entry in request.headers.get("x-forwarded-for", "").split(","): + candidate = entry.strip() + if _is_ip(candidate): + return candidate + return request.client.host if request.client else "" + + +def _is_ip(value: str) -> bool: + """Whether the token is a real address. + + Proxies do insert non-addresses — `unknown` is the classic — and Plausible + documents that it uses "the first **valid** IP address from the list". + Forwarding a non-address gets the event discarded or mis-located, so a + malformed entry is skipped rather than passed on. + """ + if not value: + return False + try: + ipaddress.ip_address(value) + except ValueError: + return False + return True diff --git a/docs/reference/plausible.md b/docs/reference/plausible.md index 44db34498a..ebb385e02e 100644 --- a/docs/reference/plausible.md +++ b/docs/reference/plausible.md @@ -256,6 +256,31 @@ machine-side and goes to `bots.anyplot.ai` carrying the same `assistant` and `kind` props as `bot_fetch`, so both events slice alike. Anything it does not recognise (Twitter, Facebook, Slack, WhatsApp, …) is a share and stays put. +#### Two things Plausible will silently drop + +Both were found by sending probe events against the live API and comparing what +appeared, and both make an event vanish with an HTTP `202 ok` — Plausible always +acknowledges, then discards. + +**A bot User-Agent.** Plausible identifies crawler agents and drops their +events. Verified: the same event sent as `Claude-User` never appears, sent as a +browser UA it does. There is no documented bypass. Machine-side events are +therefore sent under `BOT_SENDER_UA` (`anyplot-server/1.0`); nothing is lost, +because the UA only feeds Plausible's browser/OS/device detection — meaningless +for a crawler — while the identity travels in `assistant` and `kind`. + +**Our own IP instead of the visitor's.** The +[Events API docs](https://plausible.io/docs/events-api) state that Plausible +uses "the first valid IP address from the list" and that if you "forward a +server, hosting provider, or CDN IP address instead of the actual visitor IP, +Plausible's bot filtering will drop the event". Analytics therefore resolves the +IP with `visitor_ip`, **not** the rate limiter's `client_ip`: the latter +deliberately returns the *rightmost* forwarded entry, because the leftmost is +client-controlled and forging it once let callers poison another user's +rate-limit bucket. The rightmost entry is our own infrastructure, so reusing it +here would discard every event. `api/request_context.py` documents why the two +must stay separate. + Register the three properties on the **`bots.anyplot.ai`** site, not on `anyplot.ai` — property registration is per site, and without it the events still arrive but cannot be broken down, which is the whole point of collecting diff --git a/tests/unit/api/test_analytics.py b/tests/unit/api/test_analytics.py index 6677be04c1..b5423ddfd7 100644 --- a/tests/unit/api/test_analytics.py +++ b/tests/unit/api/test_analytics.py @@ -7,6 +7,7 @@ from api.analytics import ( BOT_DOMAIN, + BOT_SENDER_UA, DOMAIN, PLATFORM_PATTERNS, _detect_whatsapp_variant, @@ -400,12 +401,15 @@ async def test_carries_assistant_kind_and_public_path(self) -> None: } @pytest.mark.asyncio - async def test_resolves_a_forwarded_for_chain_to_one_address(self) -> None: - """Multiple proxies append to XFF, so the raw header is not an IP. - - Plausible needs a single address for geolocation, and the rightmost - entry is the one a client cannot forge — the same rule the feedback - rate limiter uses, which is why both now share one resolver. + async def test_reports_the_visitor_not_our_own_infrastructure(self) -> None: + """Analytics wants the LEFTMOST forwarded entry — the actual visitor. + + The opposite of the rate limiter, which takes the rightmost because the + leftmost is client-controlled. Plausible documents that it uses "the + first valid IP address from the list" and that forwarding "a server, + hosting provider, or CDN IP address instead of the actual visitor IP" + makes its bot filtering drop the event — so handing it the rightmost + entry, which is ours, silently loses the data. """ request = MagicMock() request.headers = { @@ -421,7 +425,7 @@ async def test_resolves_a_forwarded_for_chain_to_one_address(self) -> None: track_bot_fetch(request, "/box-basic") await asyncio.sleep(0) - assert mock_client.post.call_args[1]["headers"]["X-Forwarded-For"] == "10.0.0.9" + assert mock_client.post.call_args[1]["headers"]["X-Forwarded-For"] == "203.0.113.7" @pytest.mark.asyncio async def test_sends_nothing_for_a_human(self) -> None: @@ -486,3 +490,86 @@ async def test_a_crawler_fetch_goes_to_the_bot_site(self, user_agent: str, assis assert payload["domain"] == BOT_DOMAIN assert payload["props"]["assistant"] == assistant assert payload["props"]["kind"] == kind + + +class TestBotEventsUseANeutralAgent: + """Plausible drops events whose UA it identifies as a bot — verified live.""" + + @staticmethod + def _request(user_agent: str) -> MagicMock: + request = MagicMock() + request.headers = {"user-agent": user_agent} + request.client.host = "203.0.113.7" + return request + + @pytest.mark.asyncio + async def test_bot_fetch_does_not_forward_the_crawler_agent(self) -> None: + """Forwarding it means the bot site records nothing at all.""" + with patch("api.analytics.httpx.AsyncClient") as mock_client_class: + mock_client = AsyncMock() + mock_client_class.return_value.__aenter__.return_value = mock_client + + track_bot_fetch(self._request("Mozilla/5.0 (compatible; Claude-User/1.0)"), "/box-basic") + await asyncio.sleep(0) + + call = mock_client.post.call_args[1] + assert call["headers"]["User-Agent"] == BOT_SENDER_UA + # the identity is not lost — it travels in the props + assert call["json"]["props"]["assistant"] == "claude" + + @pytest.mark.asyncio + async def test_machine_side_og_image_also_uses_it(self) -> None: + with patch("api.analytics.httpx.AsyncClient") as mock_client_class: + mock_client = AsyncMock() + mock_client_class.return_value.__aenter__.return_value = mock_client + + track_og_image( + self._request("Mozilla/5.0 (compatible; Googlebot/2.1)"), page="spec_detail", spec="box-basic" + ) + await asyncio.sleep(0) + + call = mock_client.post.call_args[1] + assert call["json"]["domain"] == BOT_DOMAIN + assert call["headers"]["User-Agent"] == BOT_SENDER_UA + + @pytest.mark.asyncio + async def test_the_main_site_still_sees_the_real_agent(self) -> None: + """A shared link is human behaviour and its platform detection matters.""" + with patch("api.analytics.httpx.AsyncClient") as mock_client_class: + mock_client = AsyncMock() + mock_client_class.return_value.__aenter__.return_value = mock_client + + track_og_image(self._request("Twitterbot/1.0"), page="spec_detail", spec="box-basic") + await asyncio.sleep(0) + + call = mock_client.post.call_args[1] + assert call["json"]["domain"] == DOMAIN + assert call["headers"]["User-Agent"] == "Twitterbot/1.0" + + +class TestVisitorIpValidation: + """Plausible uses "the first VALID IP address from the list" — so must we.""" + + @staticmethod + def _request(headers: dict[str, str]) -> MagicMock: + request = MagicMock() + request.headers = headers + request.client.host = "127.0.0.1" + return request + + @pytest.mark.parametrize( + ("headers", "expected"), + [ + # `unknown` is the classic proxy filler; skip it, do not forward it + ({"x-forwarded-for": "unknown, 84.75.12.9"}, "84.75.12.9"), + ({"cf-connecting-ip": "garbage", "x-forwarded-for": "84.75.12.9"}, "84.75.12.9"), + ({"x-forwarded-for": "2a02:1210::1, 10.0.0.1"}, "2a02:1210::1"), + # nothing usable anywhere: fall back to the socket peer + ({"x-forwarded-for": "nonsense"}, "127.0.0.1"), + ({}, "127.0.0.1"), + ], + ) + def test_skips_tokens_that_are_not_addresses(self, headers: dict[str, str], expected: str) -> None: + from api.request_context import visitor_ip + + assert visitor_ip(self._request(headers)) == expected