HarperFast / HarperFast/prerender-plugin

Crawler-invented malformed URLs become immortal Targets: no validation at discovery + 403 exempt from suppression

Offen
#81 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
bug P2
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

Beitragsleitfaden öffnen

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

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.