HarperFast / HarperFast/prerender-plugin
Crawler-invented malformed URLs become immortal Targets: no validation at discovery + 403 exempt from suppression
- Vorherrschende Sprache
- JavaScript
- Sterne
- 0
- Forks
- 0
- Ø Merge
- 9 Std. 39 Min.
- Gemergte PRs (30 T.)
- 39
Beschreibung
## Summary
Crawlers with broken link extractors invent malformed URLs from our rendered HTML. Discovery creates a
`Target` for each with **no URL validation**, the origin's WAF rejects them with **403**, and 401/403 is
deliberately exempt from suppression — so they can never be retired. They accumulate permanently and
retry forever.
Measured on a production deployment over 24h: **1,264 distinct URLs**, producing **~2,950 renders/day**
that can never succeed. All of them: `sitemapUrl` null (never submitted), `strikes` at max, and **no
`PrerenderedPage` for any device cacheKey — i.e. not one has ever rendered successfully.**
## What has changed since filing — updated 2026-09-16
**The dominant funnel now has a switch, but neither fix in this issue shipped.**
#127 (`prerender-v0.54.0`) added the discovery gate — `ingress.discoveryBots` (a creation-time bot
allowlist) and per-route `ingress.routes[].discoverTargets` — plus
`POST /prerender_admin/discovery-purge` to remove what was already minted. Where a deployment sets
them, the broken-extractor crawlers that invent these URLs (AhrefsBot alone was 81.5% of the junk
misses; DotBot and friends the rest) are exactly the ones left off the list, and this class stops
arriving. #84, which asked for that gate, is closed.
**That is a deployment-configurable mitigation, not a fix, and it does not cover the whole class:**
- An **allowlisted** crawler follows invented links too. Googlebot crawls links wherever it finds them,
including the mangled ones a third party published, so a curated allowlist still admits this shape.
- It does nothing for what has already accumulated — `discovery-purge` deletes by URL prefix and
`sitemapUrl`-null, which does not select *malformed* URLs specifically.
- A deployment that leaves `discoveryBots` at its `['*']` default — the default, chosen so an upgrade
changes no behaviour — is exactly where it was.
Both fixes below are still absent from the tree:
- `maybeSchedule` (`src/http_handlers/bot_request.js`) gates on the route flag and the bot allowlist and
**nothing about the URL itself**.
- `authShaped = (statusCode) => statusCode === 401 || statusCode === 403` in `src/resources/RenderQueue.js`
still suppresses nothing, unconditionally.
## Why it happens
Representative captured URLs (product ids redacted):
```
/product/prd-XXXXXXX/target=
/product/prd-XXXXXXX/url(%22https:/fonts.googleapis.com/css2
/product/prd-XXXXXXX/....jsp'%20defer='defer
/product/prd-XXXXXXX/name@example-vendor.com
/product/prd-XXXXXXX (bare id, slug truncated)
```
These are artefacts of third-party crawlers mis-parsing our own output:
- `url(%22https:/fonts…` is a CSS `url("https://fonts…")` declaration resolved as a **relative page
URL** — note `https://` collapsed to `https:/` by the crawler's normalizer. A single sampled snapshot
contained **200 `url(` occurrences**.
- `target=` is the `target` attribute concatenated onto the href.
- `' defer='defer` is script attributes concatenated.
**Verified NOT our bug:** the same sampled snapshot had 162 `href`s, **0** containing suspicious tokens
and **0** unquoted attributes. Our emitted HTML is well-formed; the mis-parsing is crawler-side.
Worth noting that prerendering enlarges the attack surface: a rendered page is ~2.5x the size of the
origin SSR document with substantially more markup and CSS, so bad parsers have more to mangle than
they would against the origin.
Then the chain closes:
1. crawler requests the malformed URL
2. cache miss → `handlePageScheduling` creates a `Target` — **no URL validation**
3. render → the CDN/WAF answers **403** (these read as injection payloads: quotes, `url(`, `=`
fragments), not 404
4. `RenderQueue` classifies 401/403 as auth-shaped and **deliberately does not suppress** — correct in
general, because an auth failure is usually a broken renderer credential and striking would
mass-delete healthy targets during an outage
5. `failureRetry` retries each ~2x/day, forever
Each decision is individually right. Together they make an immortal class of targets.
## Impact
Compute cost is trivial — ~0.2% of render throughput. The real damage is diagnostic:
`RenderQueue` logs, per occurrence:
> `Prerender got 403 for — auth-shaped, NOT suppressing. If these are widespread, check the
> renderer's origin-bypass credential and the CDN/origin access rules.`
That is exactly the alarm you need for a genuine bypass-token break or a CDN rule change — and it
fires ~2,950 times/day as steady-state noise, so a real incident has to be spotted against that floor.
This matters more as crawl volume grows: the junk population scales with crawl traffic.
## Proposed fix (two parts, complementary)
### 1. Validate URLs at discovery
Reject non-canonical shapes in `handlePageScheduling` **before** creating a `Target`. This prevents the
row, the schedule entries, and every downstream render — the cheapest possible point to stop it, and it
is independent of *who* is asking, which is what makes it cover the allowlisted-crawler case the gate
cannot.
Prefer a positive rule over a blocklist: for a known route, require the path to match that route's
canonical shape (e.g. product slugs restricted to a safe charset). A blocklist of `url(`, quotes,
whitespace, `=`-in-segment etc. would work today but invites an endless game of catch-up.
Should be config-driven and default-on, with a counter for rejected discoveries so the rule's
selectivity is observable (and a too-strict rule is visible rather than silent). The `discovery_gated`
series added in v0.54.0 is the natural home — it already splits by which gate refused.
Note the related finding from #84's measurement: the origin answers a `%252B`-style double-encoded
mutation with a 200 whose canonical is a **newly fabricated** junk URL derived from the mutation. So the
origin manufactures fresh junk URLs from junk input, which crawlers then follow — an argument for
rejecting `%25xx` escapes at discovery specifically, and against ever adopting a page's canonical as a
new Target.
### 2. Let 403 suppress under a never-successful predicate
Keep the mass-outage protection, but narrow it. Suppress on 401/403 only when **all** hold:
- `sitemapUrl` is null (never submitted), **and**
- `strikes >= maxStrikes`, **and**
- no `PrerenderedPage` exists for any device cacheKey (never rendered successfully in its lifetime)
A target meeting all three has never produced a page and was never submitted, so retiring it cannot
lose anything — and any submitted URL keeps the current unconditional protection, which is the case the
exemption was written for. Deletion is self-healing regardless: a later request proxies to origin and
rediscovers the URL.
Part 1 stops new arrivals; part 2 retires what slips past (and anything already accumulated) —
including on deployments running the default `discoveryBots: ['*']`.
## Interim action taken
1,106 of these targets were deleted via `Target.delete()` (which also drops the `RenderSchedule` rows
and `PrerenderedPage` rows — a raw table delete would orphan the schedule row, and `claim` builds jobs
from the schedule alone without checking the target still exists). Every deletion was gated on the
three predicates above; 0 were in a sitemap, 0 had a cached page, 0 errors.
That is a cleanup, not a fix — without part 1 they simply come back on the next crawl.
Beitragsleitfaden
Rechercherichtung
Start by tracing handlePageScheduling and maybeSchedule in src/http_handlers/bot_request.js to see how discovery creates Targets, then inspect authShaped in src/resources/RenderQueue.js. Define how route-aware URL validation and its rejection counter fit the existing discovery gates. Done means malformed discoveries create no Target, and 401/403 suppression applies only to never-submitted, never-successful targets at max strikes without weakening outage protection.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- javascript
- Bereich
- backend
- Issue-Typ
- Bug
- Schwierigkeit
- 5/5
- Geschätzter Aufwand
- Über eine Woche
- Aktivitätsstatus
- Ruhig
- Klarheit
- Größtenteils klar
- Anfängerfreundlichkeit
- 38/100