From b111e0c0cb3639b8ef7c367265ddf4e4bdfb67de Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 28 Sep 2026 19:54:47 +0000 Subject: [PATCH] fix(ads): refuse stale ad clicks outright instead of redirecting them #264 marked a click that trails its impression past the device ceiling as invalid, but still wrote a row and still 302'd to the advertiser. That is the reward the feed-link harvester keeps coming back for: it fetches feeds with a Chrome UA, keeps the /a/ links, and replays them from fresh IPs days to weeks later. Last 7 days on prod: 13,178 of 13,513 clicks were exactly that, 0 of them valid. A stale click now gets a 410 with no link, no-store and noindex/nofollow, and nothing is recorded or charged. Feed-reader impressions keep no ceiling, so a subscriber opening an old item is untouched; no browser click in 30 days has trailed its impression past 6h. Every other invalid click (duplicate, cooldown, forged) is still recorded and redirected as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/a/[id]/route.ts | 4 +- app/api/ads/click/route.ts | 4 +- lib/ads/blocked-click.ts | 22 ++++++ lib/ads/serve.ts | 18 ++++- .../contract/ads-stale-click-blocked.test.ts | 68 +++++++++++++++++++ 5 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 lib/ads/blocked-click.ts create mode 100644 tests/contract/ads-stale-click-blocked.test.ts diff --git a/app/a/[id]/route.ts b/app/a/[id]/route.ts index 797a0bd5..6f509f70 100644 --- a/app/a/[id]/route.ts +++ b/app/a/[id]/route.ts @@ -10,7 +10,8 @@ // impression row) redirect to the site rather than dead-ending. import { NextRequest, NextResponse } from "next/server"; -import { resolveClick } from "@/lib/ads/serve"; +import { BLOCKED_CLICK, resolveClick } from "@/lib/ads/serve"; +import { blockedClickResponse } from "@/lib/ads/blocked-click"; import { serviceClient } from "@/lib/supabase/service"; import { lookupGeo } from "@/lib/tracker/geo"; import { adClickIp } from "@/lib/ads/client-ip"; @@ -107,6 +108,7 @@ export async function GET(request: NextRequest, ctx: { params: Promise<{ id: str }, }); + if (dest === BLOCKED_CLICK) return blockedClickResponse(); if (!dest) return NextResponse.redirect(fallback, { status: 302 }); if (crawler) return NextResponse.redirect(dest, { status: 302, headers: { "cache-control": "no-store", "x-robots-tag": "noindex, nofollow" } }); diff --git a/app/api/ads/click/route.ts b/app/api/ads/click/route.ts index 402dffef..29f75aaa 100644 --- a/app/api/ads/click/route.ts +++ b/app/api/ads/click/route.ts @@ -3,7 +3,8 @@ // still redirect somewhere safe rather than dead-ending the user. import { NextRequest, NextResponse } from "next/server"; -import { resolveClick } from "@/lib/ads/serve"; +import { BLOCKED_CLICK, resolveClick } from "@/lib/ads/serve"; +import { blockedClickResponse } from "@/lib/ads/blocked-click"; import { lookupGeo } from "@/lib/tracker/geo"; import { adClickIp } from "@/lib/ads/client-ip"; import { parseDevice } from "@/lib/tracker/device"; @@ -39,6 +40,7 @@ export async function GET(request: NextRequest) { ctx: { visitorId, ip, country: geo?.countryCode ?? null, device }, }); + if (dest === BLOCKED_CLICK) return blockedClickResponse(); return NextResponse.redirect(dest ?? fallback, { status: 302 }); } catch { return NextResponse.redirect(fallback, { status: 302 }); diff --git a/lib/ads/blocked-click.ts b/lib/ads/blocked-click.ts new file mode 100644 index 00000000..4799dcc1 --- /dev/null +++ b/lib/ads/blocked-click.ts @@ -0,0 +1,22 @@ +// What a refused click gets back (see BLOCKED_CLICK in ./serve). +// +// 410 rather than a redirect to the homepage: a redirect is a reward, and the +// harvesting crawler would simply follow it and crawl us instead. The body +// carries no link to the advertiser for the same reason. noindex/nofollow and +// no-store so nothing between us and it keeps the answer or the URL. + +import { NextResponse } from "next/server"; + +export function blockedClickResponse(): NextResponse { + return new NextResponse( + "Link expired

This ad link has expired.

", + { + status: 410, + headers: { + "content-type": "text/html; charset=utf-8", + "cache-control": "no-store", + "x-robots-tag": "noindex, nofollow", + }, + }, + ); +} diff --git a/lib/ads/serve.ts b/lib/ads/serve.ts index f94f615b..2b75cfaf 100644 --- a/lib/ads/serve.ts +++ b/lib/ads/serve.ts @@ -717,15 +717,28 @@ async function resolveClickVisitor( } } +/** + * resolveClick's answer for a click that is refused outright: no row, no + * redirect. Today that is exactly one case, a click trailing an impression + * past its device's ceiling (see maxClickAgeMs). Those were recorded as + * invalid and still sent to the advertiser, which is what kept the harvesting + * crawler coming back — ~37k a month, every one of them replaying a feed-item + * link days to weeks after it was fetched. Nobody real is refused by it: no + * browser click in 30 days trailed its impression past 6h, and feed-reader + * impressions have no ceiling at all. + */ +export const BLOCKED_CLICK = Symbol("blocked-click"); + // Resolve a click: record it, return the destination URL (with ?ref=) to -// redirect to. Returns null if the campaign/creative can't be resolved. +// redirect to. Returns null if the campaign/creative can't be resolved, and +// BLOCKED_CLICK if the click is refused. export async function resolveClick(input: { impressionId?: string | null; slotId?: string | null; campaignId?: string | null; creativeId?: string | null; ctx?: ServeContext; -}): Promise { +}): Promise { const sb = serviceClient(); if (!input.campaignId) return null; @@ -753,6 +766,7 @@ export async function resolveClick(input: { ipHashes: rotatingIpHashCandidates(input.ctx?.ip ?? null, CLICK_DEDUPE_WINDOW_MS), device: input.ctx?.device, }); + if (validity.reason === "stale_impression") return BLOCKED_CLICK; if (validity.valid) { const admission = await claimClickCooldown({ visitorId, ip: input.ctx?.ip }); if (!admission.allowed) validity = { valid: false, reason: admission.reason }; diff --git a/tests/contract/ads-stale-click-blocked.test.ts b/tests/contract/ads-stale-click-blocked.test.ts new file mode 100644 index 00000000..cb98386c --- /dev/null +++ b/tests/contract/ads-stale-click-blocked.test.ts @@ -0,0 +1,68 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// A click trailing its impression past the device ceiling is refused outright: +// no ad_clicks row, no redirect to the advertiser. Recording it as invalid and +// still redirecting is what kept the feed-link harvester coming back. + +const state = vi.hoisted(() => ({ + validity: { valid: true } as { valid: boolean; reason?: string }, + inserts: [] as Record[], + rpc: vi.fn(), +})); +vi.mock("@/lib/supabase/service", () => ({ serviceClient: () => ({ + rpc: state.rpc, + from(table: string) { + const q = { + select: () => q, eq: () => q, + insert(row: Record) { state.inserts.push(row); return q; }, + maybeSingle: async () => ({ data: table === "ad_campaigns" + ? { id: "campaign", destination_url: "https://example.com/offer", ref_slug: "ad-test", bid_credits: 4 } + : { visitor_id: "visitor" } }), + }; + return q; + }, +}) })); +vi.mock("@/lib/ads/fraud", async (original) => ({ + ...await original(), assessClickValidity: async () => state.validity, +})); +vi.mock("@/lib/ads/click-cooldown", () => ({ claimClickCooldown: async () => ({ allowed: true }) })); +vi.mock("@/lib/ads/promos", () => ({ promoForCampaign: async () => null })); +vi.mock("@/lib/ads/bids", () => ({ paperCharge: vi.fn() })); +import { BLOCKED_CLICK, resolveClick } from "@/lib/ads/serve"; +import { blockedClickResponse } from "@/lib/ads/blocked-click"; + +beforeEach(() => { + state.validity = { valid: true }; + state.inserts = []; + state.rpc.mockReset(); + state.rpc.mockResolvedValue({ data: [{ click_id: "click", valid: true, charged_cents: 20 }] }); +}); + +const click = () => resolveClick({ + campaignId: "campaign", slotId: "slot", impressionId: "imp", + ctx: { ip: "8.8.8.8", visitorId: "visitor", device: "desktop" }, +}); + +describe("stale ad clicks", () => { + it("are refused: no row, no charge, no destination", async () => { + state.validity = { valid: false, reason: "stale_impression" }; + expect(await click()).toBe(BLOCKED_CLICK); + expect(state.inserts).toHaveLength(0); + expect(state.rpc).not.toHaveBeenCalled(); + }); + + it("leave every other invalid click recorded and redirected", async () => { + state.validity = { valid: false, reason: "duplicate" }; + expect(await click()).toBe("https://example.com/offer?ref=ad-test"); + expect(state.inserts).toHaveLength(1); + }); + + it("get a 410 that links nowhere and is kept by nothing", async () => { + const res = blockedClickResponse(); + expect(res.status).toBe(410); + expect(res.headers.get("location")).toBeNull(); + expect(res.headers.get("cache-control")).toBe("no-store"); + expect(res.headers.get("x-robots-tag")).toContain("nofollow"); + expect(await res.text()).not.toContain("example.com"); + }); +});