[ClemRz]

🤔 What

Step 6 of the nightly bot-surface pipeline: an audit that reads the published pages back from the storage adapter and checks what is actually online. It also delivers the t_bot_surface_state criterion of #1888 (the table exists and the pipeline and the withdrawal worker honour it).

• Audit (BotAuditService): checks every page uploaded since the start of the run, plus a random sample of others (1,000 by default), for coordinates, sensitive or unfit records and a leaked crawler variant.
• On a failure the page is deleted through the adapter, its manifest row is removed, and a distinct bot-surface-audit failure <path>: <reason> error line is logged.
• More than BOT_AUDIT_FAILURE_LIMIT (10) failures in one run set t_bot_surface_state.publishing = false and log bot-surface-audit publishing stopped.
• t_bot_surface_state: one row, fails closed (a missing row or an unreadable table reads as publishing = false, logged). With it off, the generator, the sitemaps and the worker upload nothing and still delete.
• Adapters get get(key, { maxBytes }) (local and Azure).
• docs/bot-surface.md has a new "Audit and the publishing stop" section: how to resume (SQL), the failure reasons, the alert queries. chore(bot-surface): launch plan, staged enablement per entity type and runbook #1888 links to it for the runbook line.

Closes #1887

🤷‍♂️ Why

Every protection so far sits on the input side (the view's gates, the field whitelist, the text scan, the withdrawal worker) and is tested on fixtures. Nothing looked at what is online: a template that interpolates the wrong variable would pass every fixture test and still leak coordinates. The audit checks the output, independently of how the pages were built, and the switch lets it stop uploads until someone investigates.

🔍 How

• Reading back. adapter.get returns a Buffer (or null), and rejects with E_BOT_PAGE_TOO_LARGE before reading the body when the page is over the cap (2 MiB; such a page fails closed). The audit reads at most 4 pages at a time and keeps only the ids a batch refers to.
• Coordinates. The detector is unchanged. api/utils/botAuditPage.js shapes its input: markup stripped, attribute values and comments kept, <style> dropped, script contents kept, entities decoded, and each JSON-LD block checked both as written and flattened to key: value lines. No false positive over the snapshot pages or over 200 generated pages from the real renderer.
• Records. Links and JSON-LD @ids (/ui/caves/<id> etc.) and the page's own record are looked up by id in batches. A sensitive entrance or massif, a merged, deleted or missing record, and a cave with no public entrance are failures. A redirect page is about a merged record by definition: only its target is checked.
• Variant. For BOT_AUDIT_VARIANT_SAMPLE (20, 0 disables) published addresses, the public SPA URL is requested with a browser User-Agent (2 at a time, 500 ms apart, 10 s timeout, injectable fetch). The generator marker in the answer is a failure. A network error or a non-2xx answer is counted as variantUnreachable and never deletes a page.
• Bounds. Batches by keyset (tonight) or ORDER BY random() LIMIT (sample), a 30-minute time budget (BOT_AUDIT_TIME_BUDGET_MS) after which truncated: true is recorded instead of failing the run. The audit fails the run only on its own errors (adapter or database unreadable, page lock held for 5 minutes).
• Deleting a failing page takes the page lock, so the withdrawal worker cannot leave a page online without a manifest row.
• Publishing off, shared by the three writers (BotPublishService.publishOrWithhold): an unchanged page stays; a page whose hash differs is deleted instead of uploaded (a changed page may be the reduced version of a withdrawal); a new page is not uploaded. 404.html carries no record data, so a changed one is left as it is. A sitemap shard or index that would change is not produced and is deleted (index first), so the sitemaps never name a deleted page.
• Config. BOT_AUDIT_SAMPLE_SIZE, BOT_AUDIT_FAILURE_LIMIT, BOT_AUDIT_VARIANT_SAMPLE, BOT_AUDIT_TIME_BUDGET_MS, validated at startup. config/env/test.js sets the variant sample to 0 so no test reaches the network.
• The run record gets counts.audit (runAt, pagesChecked, uploadedChecked, sampled, missing, failures, failuresByReason, variantChecked, variantUnreachable, truncated, publishingStopped, publishing, durationMs), and the final log line shows publishing=on|OFF.

🧪 Testing

• npm run lint
• The whole suite on one thread from a freshly seeded database (npm run test:snapshot:clean, then mocha over test/integration/**): 6157 passing, 0 failing
• The new and changed bot tests together with Organization/find-all.test.js in one mocha process
• Each failure kind with a seeded page (coordinate in the body, in the JSON-LD, in a meta tag, sensitive entrance, sensitive massif, deleted, merged and missing records, cave with no public entrance, page too large), the deletion from the adapter and the manifest, the variant check (marker, plain shell, unreachable, timeout, spacing), the threshold (exactly the limit does not stop, limit + 1 does), the bounds, get on both adapters (Azure with a Sinon stub), step 6 in the real orchestrator (green run, run with a template that leaks, run that stops publishing)
• Publishing off end to end: nothing uploaded, a changed page deleted, an unchanged one kept, deletes still run, fail closed on a missing row, for the generator, the sitemaps and the worker
• Mutation checks on 20 changes to the code (each applied, each caught by a test)
• Not verified: a real Azure read, the variant check against the real site, the duration of the first full audit (about 12,000 pages)

Deployment

Production does not run migrations on deploy. Apply sql/9_40_2026_11_09_bot_surface_state.sql by hand before the API that contains this change starts running the pipeline: without the table or its row, publishing reads as off. It is idempotent (INSERT ... ON CONFLICT DO NOTHING).

[tristan0x]

Solid step: the audit reads what is actually online, fails closed on the switch, and the publishing-off behaviour of the three writers is consistent and well tested. One blocking issue: the publishing stop is only evaluated at the very end of run(), so any error raised after failures were found (page-lock timeout, adapter or database error in a later batch) skips it and leaves publishing on. The other comments are suggestions about false positives in the record references, the CodeQL thread on SCRIPT_RE, and how a variant leak is handled.

[tristan0x]

The threshold only runs when everything before it succeeded. If deleteFailed throws E_BOT_AUDIT_LOCK_TIMEOUT (the test fails on its own error when the lock stays held shows the pages then stay online), or if badRecords/adapter.get throws in a later batch, run() rejects before it reaches this line. A run that has already logged, for example, 40 coordinates-body failures then leaves publishing = true and the leaking pages online, and the next night uploads them again. That is the worst case for the switch: it fails open exactly when the audit found the most.

Suggested fix: check the limit as soon as counts.failures crosses it (inside fail(), or right after each auditBatch), and also wrap steps 1–3 in try { … } finally { if (counts.failures > failureLimit && !counts.publishingStopped) await stopPublishing(…) } so the stop is recorded before the error propagates. Then add a test: more than failureLimit failing pages, lock held for the whole retry, and assert that the run rejects and t_bot_surface_state.publishing is false.

[ClemRz]

Fixed, and a bit more: the limit is now checked (1) right after the failures of a batch are found, before deleteFailed, so a lock that never frees finds the switch already off, and (2) again when the run ends on an error of its own, so an error in a later batch (adapter.get, the database) cannot skip it. The second check logs and keeps the original error if stopping itself fails. Tests: 12 failing pages with a limit of 10 and the lock held for the whole retry: the run rejects with E_BOT_AUDIT_LOCK_TIMEOUT and publishing is false with the reason; the second batch cannot be read after the first crossed the limit: rejects and publishing is false; a run that fails below the limit leaves publishing on. Honest note on the mutants: each of the two checks alone is enough for these scenarios, so removing only one still passes; removing both fails four tests.

[tristan0x]

REF_RE matches /caves/<n> anywhere in the decoded HTML, including the description text that contributors write, and URLs on other hosts (https://some-club.fr/caves/12, an old grottocenter.org/ui/caves/<merged id> pasted into a description). Such a page fails as merged-record/missing-record/sensitive-entrance, is deleted, is regenerated the next night (its manifest row is gone, so it is "new"), and fails again, every night. A dozen such descriptions are enough to stop publishing on the first run. The snapshot pages and the 200 generated pages say nothing about the ~12,000 real descriptions.

I suggest only reading references the generator emits: href values and JSON-LD @id values that start with the configured publicBaseUrl (or with the relative /ui/), and leaving free text out. Before turning the stop on in production, a dry count over t_description/t_name with ~ '/(caves|entrances|massifs|organizations)/[0-9]' would show how many published records could hit this.

[ClemRz]

Fixed as you suggest. recordRefs(html, baseUrl) now reads only the href values and the JSON-LD @id values that start with publicBaseUrl or with its path alone (/ui/...), and the address must be exactly <base>/<directory>/<id> (a trailing /, ? or # is allowed). The text of a description, a JSON-LD name or description, another host and /api/caves/... are left out. Tests: a description with https://grottocenter.org/ui/caves/<merged>, /caves/<merged> and a club link does not fail the page; a real href to a merged cave does (merged-record); relative addresses, single quotes, unquoted and entity-encoded hrefs are read; a mutant that also reads free text fails two tests. The dry count you asked for, read-only on production: 45 non-deleted descriptions and 5 names contain such a path, so with the old rule those pages would have failed and come back every night.

[tristan0x]

About the open CodeQL thread (</script\t\n bar> is not matched): here it does not hide anything, because an unmatched <script> is not removed: its contents fall through to body after TAG_RE drops the tag, and the detector still runs over them. It would be worth stating that in the thread before dismissing the alert, or simply closing the gap with <\/script[^>]*> (same for STYLE_RE), plus a test with an unusual end tag that shows the coordinates are still detected. Note that <style> is the riskier one: an unmatched </style foo> keeps the CSS in the body (false positives at worst), which is harmless.

[ClemRz]

Same fix as the CodeQL thread: </script[^>]*> and </style[^>]*>, with the test on </script\t\n bar> showing the coordinates are still detected.

[tristan0x]

A variant-leak means a cache or rewrite layer serves the crawler page to browsers; the blob itself is fine. Deleting it takes the page off the site for crawlers too, does not purge what the cache already holds, and the next night's step 4 uploads it again (no manifest row), so it leaks again until the cache rule is fixed. Is that intended? Since a variant leak is a configuration problem affecting every page, not just the sampled ones, it might be better to stop publishing on the first variant-leak (independently of failureLimit) rather than delete the sampled pages. At minimum, the runbook in docs/bot-surface.md should say that this reason is fixed in the CDN/cache configuration, not in the data.

[ClemRz]

Not what I intended, and your reading is better: a variant-leak now stops publishing at the first one, whatever BOT_AUDIT_FAILURE_LIMIT, and deletes nothing. It is still counted as a failure and logged (bot-surface-audit failure <path>: variant-leak, then bot-surface-audit variant leak, publishing stopped: ... with the sampled addresses and 'fix the cache or CDN rule'). The pages stay online, since the blob is fine and deleting it would fix nothing. The docs say this reason is fixed in the CDN or cache configuration, not in the data, then publishing is re-enabled with the SQL line. An unreachable address still stops nothing. Tests cover the stop, the reason on the state row, the page left in the adapter and the manifest, and the unreachable case.

[tristan0x]

response.text() buffers the whole body before .slice(0, VARIANT_BODY_LIMIT), so the limit bounds nothing. The SPA shell is small, so this is minor, but if the cap matters, read response.body chunk by chunk and stop (and controller.abort()) once VARIANT_BODY_LIMIT bytes have been read, as the Azure adapter's get does with maxBytes.

[ClemRz]

Fixed: the answer is read with response.body.getReader(), chunk by chunk, and the request is aborted and the reader cancelled once VARIANT_BODY_LIMIT (2 MiB) bytes were read; a response without a stream falls back to text(). Tests use an endless ReadableStream: the audit pulls fewer than 16 chunks and the stream is cancelled; and a marker at the start of a long body is found without reading the rest.

[tristan0x]

This iteration addresses every previous thread. The failure limit is now checked before the deletes and again when the run ends on an error. A variant leak stops publishing without deleting anything. Record references are read only from generated href and JSON-LD @id values, the script and style end tags close as a browser closes them, and the variant body is read with a byte cap. Each fix has a test that fails without it. Two non-blocking suggestions remain: the new reference rule fails open when a page was rendered with a different base URL, and a test name claims an early exit on the marker that the code does not have.

[tristan0x]

Matching only ^(?:<base>|<path>)/<dir>/<id> fixes the false positives from description text. It also makes the reference check fail open whenever a page was rendered with a base URL other than the one the audit uses now. That happens after a change of BOT_PAGES_PUBLIC_BASE_URL, between http/https or www/apex, or when the random sample reaches pages published before such a change. In each case, every href and @id of that page goes unmatched, so a link to a sensitive entrance or massif is no longer checked. Only the page's own record (from the manifest path) still is.

The page then passes silently. The old REF_RE would have caught it.

One way to keep the benefit without the blind spot: resolve each href/@id with new URL(value, base) and match on its pathname against the SPA path (/ui/<dir>/<id>), whatever the host. Count references on a foreign origin that still match the SPA path in a foreignRefs counter (or log them). That way a base URL mismatch shows up in the run record instead of disappearing. A club site that happens to use /ui/caves/<n> is unlikely enough to accept.

A smaller point on the same rule: organization.ejs renders the contributor-entered model.website as an href. A website set to a Grottocenter cave address would still fail the page every night. This is rare, but it could be excluded by skipping rel="nofollow ugc" links.

[ClemRz]

Good catch: the first version failed open exactly when the base URL had changed. recordRefs now resolves each href/@id with new URL(value, base) and matches on the path (/ui/<dir>/<id>), whatever the host, so a page rendered under another origin (a changed setting, http/https, www/apex, pages from before such a change reached by the sample) still has its links checked. The references on another origin than the current base are counted: recordRefs returns foreign, counts.audit.foreignRefs carries the total, and the audit logs one warning (...point at the SPA path on another origin than <base>; the base URL was probably changed...). A path that is not the SPA's (/caves/79 on a club site, /api/caves/80) is still not a reference, and mailto:/javascript: are ignored. On your smaller point: the links marked rel="nofollow ugc" are now skipped (the attributes of each <a> are parsed, ugc in any order), so the website of an organization can be a Grottocenter address without failing the page every night. Tests: the same-path/other-host cases with the exact counts, the sensitive-entrance link on an old host still fails the page and counts 1 foreign, one warning when the origin moved and none when it did not, ugc links skipped and non-ugc ones read. Mutants (host required again, foreign not counted, ugc links read) each fail tests.

[tristan0x]

readCapped always reads up to VARIANT_BODY_LIMIT. It does not stop once the marker has been seen. So the test finds the marker at the start of a long body without reading it all passes only because of the 2 MiB cap: about 4 pulls of 512 KiB, under 16. It does not test an early exit. Either rename the test (for example, finds the marker at the start of a body larger than the limit) or make the behaviour match the name by testing the marker on the accumulated text inside the loop:

text += decoder.decode(value, { stream: true });
if (GENERATOR_MARKER.test(text) || bytes >= limit) {
  controller.abort();
  await reader.cancel().catch(() => {});
  break;
}

Then assert pulled is 1 or 2 in that test.

[ClemRz]

Right, the old test only passed because of the 2 MiB cap. The loop now tests the marker on the accumulated text as well as the limit (GENERATOR_MARKER.test(text) || bytes >= limit), so it aborts and cancels the reader as soon as the marker is seen, including when it straddles two chunks. Tests: a new one splits the marker across two chunks and asserts at most 3 pulls (two plus one of read-ahead); the old one is renamed finds the marker at the start of a body larger than the limit; the endless-stream test still covers the cap. Removing the early exit fails the straddling test.

[tristan0x]

This iteration addresses the two remaining threads: references are now matched on their SPA path whatever the host, with foreignRefs counted and logged once, contributor-typed rel="nofollow ugc" links are skipped, and the variant reader exits as soon as the marker is seen, with a test for a marker that straddles two chunks. One non-blocking suggestion remains, about how the new tag regex finds href attributes when an earlier attribute value contains a raw >.

[tristan0x]

TAG_WITH_HREF_RE stops each tag at the first >, so the href must come before any > in the tag. If an earlier attribute value contains a raw > (for example <a title="a > b" href="/ui/entrances/12">), the match fails and that link is never checked, so the check fails open. The previous HREF_RE matched \shref= anywhere and would have caught it. EJS's <%= %> escapes >, so the current templates are safe. But the audit is meant to check the output independently of how the templates were written, and a <%- or a hand-built attribute would hide a reference to a sensitive entrance here.

You can let the tag pattern skip over quoted values:

const TAG_WITH_HREF_RE = /<[a-z](?:[^>"']|"[^"]*"|'[^']*')*>/gi;

and keep the href === undefined check in hrefs() to drop tags without one. A test with <a title="x > y" href="/ui/caves/5"> would pin this down.

[ClemRz]

Fixed with your pattern: a tag is now matched with its quoted values skipped (<[a-z](?:[^>"']|"[^"]*"|'[^']*')*>), so a raw > inside an earlier attribute value no longer hides the href after it; hrefs() still drops the tags that have none. Tests: <a title="a > b" href="/ui/caves/5">, the same with single quotes and with an apostrophe inside double quotes, and a check that the attribute of another tag (title, src) is not taken for an href. Going back to [^>]* fails three tests. The audit stays independent of the templates, as you say: it does not rely on EJS having escaped the >.

[tristan0x]

This iteration is a rebase onto develop (only test/customSQL.js and test/seed-database.js context changed) plus one commit: OPENING_TAG_RE now skips quoted attribute values, so a raw > in an earlier attribute no longer hides the href after it. That closes the last open thread, and every earlier thread is addressed in the code. One suggestion remains: the new pattern fails open in a different way when a tag holds an unbalanced quote.

[tristan0x]

The quoted-value pattern fixes title="a > b", but it fails open in a different case: a tag with an unbalanced quote. An unquoted value can hold an apostrophe, and browsers accept it (it is only a parse error): <a title=don't href="/ui/entrances/12">.

• If no other ' follows anywhere in the page, '[^']*' cannot close, so the match fails at that <. The whole tag is dropped, and its href is never checked. The old [^>]* pattern read it.
• If a later ' exists, the "tag" swallows everything up to that quote and the next >, across other <a> tags. attributesOf then keeps only the last href of that span (found[name] = … overwrites), so the earlier links in the span are lost.

As in the previous thread, the templates escape ' today, so this only matters for a <%- or a hand-built attribute. But the audit is meant not to rely on that.

A cheap fix that fails closed in both cases: collect the hrefs from both scans and take the union. Duplicate ids are already removed by the Set in recordRefs.

const SIMPLE_TAG_RE = /<[a-z][^>]*>/gi;
const hrefs = (html) => {
  const found = [];
  [OPENING_TAG_RE, SIMPLE_TAG_RE].forEach((re) => {
    (html.match(re) || []).forEach((tag) => { /* same body */ });
  });
  return found;
};

The union can add a false positive (<div title="a href=/ui/caves/8 > b">), but that deletes the page; it does not let one through. Also keep the first occurrence of an attribute in attributesOf (if (!(name in found))), since that is the one browsers use. Add a test with <a title=don't href="/ui/caves/5"> and one with a stray ' before two <a href> tags.

[ClemRz]

Done with the union you describe: hrefs() scans the tags twice, with OPENING_TAG_RE (quoted values skipped) and with the simple <[a-z][^>]*>, and returns every href of both; the references are deduplicated before they are matched, so a link read by both scans is counted once (the foreignRefs count would otherwise have doubled). attributesOf keeps the first occurrence of an attribute. Tests: <a title=don't href="/ui/caves/5"> alone; the same followed by two more <a href> and a later ' (all three references found, which a single scan loses); two href attributes on one tag, the first wins; and <div title="a href=/ui/caves/8 > b"><a href="/ui/caves/9"> lists both 8 and 9, pinning the false positive you mention as the accepted direction. Using only the quoted scan, only the simple scan, the last occurrence, or no dedupe each fails tests. The docs say the references are unioned and why.

[tristan0x]

This iteration adds one commit for the last open thread: hrefs() now unions two tag scans (quoted values skipped, and the simple <[a-z][^>]*>), attributesOf keeps the first occurrence of an attribute, references are deduplicated before matching, and the docs explain why. Each case asked for has a test, and every earlier thread is addressed in the code. One non-blocking suggestion remains: a tag that has both an unbalanced quote and a raw > inside a later quoted value still escapes both scans.

[tristan0x]

The union closes each case on its own, but it still fails open when one tag has both problems: an unbalanced quote in an unquoted value and a raw > inside a later quoted value. A browser reads the href here:

<a title=don't data-x="a > b" href="/ui/entrances/12">x</a>

• SIMPLE_TAG_RE stops at <a title=don't data-x="a >, so it finds no href.
• OPENING_TAG_RE returns null when no later ' appears in the page. When a later ' does appear, it swallows the following tags up to that quote. If one of those tags is rel="nofollow ugc", attributesOf returns href=/ui/entrances/12 together with rel=nofollow ugc, and hrefs() skips the whole span.

I checked both cases against the two patterns copied as they are. As before, the templates escape ' and >, so this needs a <%- or a hand-built attribute. It is also more contrived than the previous cases. Still, the audit is meant not to depend on the templates.

A browser treats a quote as a value delimiter only right after =. A third pattern that follows that rule reads this tag correctly:

const VALUE_TAG_RE = /<[a-z](?:=\s*"[^"]*"|=\s*'[^']*'|[^>])*>/gi;

It returns <a title=don't data-x="a > b" href="/ui/entrances/12">. It could be added as a third scan of the union (the Set already removes duplicates), or it could replace OPENING_TAG_RE. Please add a test with the tag above, alone and followed by a rel="nofollow ugc" link and a later '. If you would rather accept this residue, a line in the comment above these patterns saying so would be enough.

[ClemRz]

Added as a third scan of the union, with one change: the pattern as suggested (...|=\s*'[^']*'|[^>])*>) has overlapping alternatives (a = can be consumed by [^>] or by the quoted branch), and it backtracks exponentially when no > closes the tag. I measured it: <a followed by 28 repetitions of ="a" took 12 seconds, and a page of a few hundred of them would hang the audit. The version in the code keeps the browser rule (a quote opens a value only right after =) with disjoint alternatives: [^>=] for any other character, and a = only when no closed quote follows it (=(?!\s*(?:"[^"]*"|'[^']*'))). It reads <a title=don't data-x="a > b" href="/ui/entrances/12">, alone and followed by a rel="nofollow ugc" link and a later ' (the entrance is found, the typed link is still skipped). A test runs the pattern on runs of 20,000 repetitions of five hostile units in a child process with a 10 s timeout, because an exponential regex never returns and would hang the test run instead of failing it; with the overlapping alternatives put back it fails. Taking the third scan out fails the new test. The docs describe the three scans.

[tristan0x]

This iteration closes the last open thread. VALUE_TAG_RE is added as a third scan of the union and follows the browser rule (a quote opens a value only right after =). It reads <a title=don't data-x="a > b" href="/ui/entrances/12"> on its own and when a later rel="nofollow ugc" link and a stray ' follow it. Its alternatives are disjoint, so the exponential case is gone, and every earlier thread is addressed in the code. One non-blocking suggestion remains: all three tag scans are still quadratic when a page has many < with no closing >, and the new test only covers a single <, even though its name claims linear time.

[tristan0x]

The disjoint alternatives do remove the exponential case. All three scans are still quadratic, though, when the page holds many tag starts and no > follows them. String.prototype.match with /g retries from every <[a-z], and each attempt runs to the end of the page before it fails. I checked this with the three patterns copied as they are (Node 24). The page was '<a ="a"'.repeat(n):

n (≈ size)	OPENING_TAG_RE	SIMPLE_TAG_RE	VALUE_TAG_RE
5,000 (35 KB)	0.24 s	0.11 s	0.29 s
10,000 (70 KB)	0.95 s	0.44 s	1.13 s
20,000 (140 KB)	3.8 s	1.8 s	4.5 s

Scaled to the 2 MiB cap, each scan would take about a quarter of an hour, and hrefs() runs three of them. The work is synchronous, so BOT_AUDIT_TIME_BUDGET_MS cannot interrupt it, and it blocks the event loop of the process that runs the pipeline. As with the previous threads, the templates escape <, so a page like this needs a <%- or a hand-built fragment. But the audit is meant not to depend on the templates, and this failure mode is an availability problem, not fail-open.

The new test (... is read in linear time) does not cover this: it puts a single <a in front of the hostile run, so there is only one start position. Adding one case with the unit repeated including its <a (for example '<a ="a"'.repeat(20000), with a tighter timeout) would show the problem.

Possible fixes, cheapest first:

• Bound the tag length in all three patterns, for example (?:…){0,8192}>. That makes each attempt O(8192), so the whole scan is linear. A real generated tag is far below that. To keep the check fail-closed, report the page as failing (for example unreadable-markup) when a <[a-z] start is not matched by SIMPLE_TAG_RE but no > appears within the bound.
• Or replace the three regexes with one small hand-written scanner that applies the browser rule in a single pass. That is linear by construction and also removes the need for the union.

[ClemRz]

You are right, and the table understates it: the same quadratic cost is in TAG_RE, SCRIPT_RE, STYLE_RE and COMMENT_RE of detectionText (many <script> with no end tag does the same), and each synchronous regex is out of reach of BOT_AUDIT_TIME_BUDGET_MS. Bounding the length of a tag in three patterns would have left those, and this is the eighth round of the same kind, so I took your second option: the regular expressions that read markup are gone. scan(html) in api/utils/botAuditPage.js is one hand-written pass that follows the HTML tokenizer: a quote opens a value only right after =, a > in a quoted value does not end the tag, an apostrophe in an unquoted value is part of it, the first occurrence of an attribute counts, <script> and <style> hold raw text up to their end tag (found case-insensitively, once, from where they start), and an end tag is read like a start tag. detectionText and recordRefs both read from it, and the union of three scans is gone. It is linear: every character is looked at a bounded number of times. A tag, a comment or a script still open at the end of the page, or a bogus comment (<!x>, <?x?>, </ >, which a browser drops up to the next >, possibly the start of a real tag), makes the page fail closed with a new reason, unreadable-markup, as you suggested; the generator never writes either. Measured: 15 hostile units (many <a ="a", <script>, <!--, <, unclosed quotes) repeated 40,000 times, about 300 KB each, take 0.4 s in total, and 4x the input takes 2.2x the time. The test runs this in a child process with a timeout and a limit of 2.5 s, and includes the <a in the repeated unit, as you point out the old one did not; a mutant that walks to the end of the page at each tag start fails it. Other tests: tags, comments, scripts and styles left open and bogus comments are unreadable, well-formed ones are not (a < in text, </SCRIPT\t\n bar>, </style foo>); a script's content is text, so a link inside a script is not a link; and the service deletes a page with open markup as unreadable-markup. One behaviour changes on purpose: <div title="a href=/ui/caves/8 > b"> no longer yields cave 8, since a browser does not read an href there (the union made it a false positive). On the spec point, I did not add a dependency: a parser such as parse5 would do the same job, but the scanner is about 150 lines and needs none.

[tristan0x]

This iteration replaces the regex tag scans with one hand-written, linear scan() that follows the HTML tokenizer, adds the fail-closed unreadable-markup reason, and closes the quadratic-time thread with a test whose repeated unit now includes the <a. Every earlier thread is addressed in the code. Four non-blocking suggestions remain, all inline: abruptly closed comments, the order of reasons in the docs, a triple scan per page, and a wall-clock bound in the linear-time test.

[tristan0x]

Starting the search for --> at lt + 4 misses the two abruptly closed comments the tokenizer accepts: <!--> and <!---> are complete, empty comments in a browser (the --> begins at lt + 2 or lt + 3). Here they open a comment that runs to the next --> further down the page, so the tags in between are read as comment text. Their hrefs are never collected, which fails open for the reference check. Coordinates are still caught because comment text stays in body. If no later --> exists, the page is marked unreadable, so the problem only appears when a real comment follows.

As in the earlier threads, the templates do not emit this, so it needs a <%- or a hand-built fragment. But the fix is one character and follows the spec:

const end = html.indexOf('-->', lt + 2);
text.push(`\n${html.slice(lt + 4, end === -1 ? n : Math.max(lt + 4, end))}\n`);

Add a test with <!--><a href="/ui/caves/5">x</a><!-- c -->, which should list cave 5 and not be unreadable.

[tristan0x]

This line no longer matches the code. readPages now returns unreadable-markup before coordinateFailure runs, so a page with an unclosed tag that also leaks coordinates is reported as unreadable-markup and not as coordinates-body. Anyone using the alert queries or failuresByReason to look for leaks would undercount them. Either say so here, e.g. "(unreadable markup, then coordinates, then the most severe record reason)", or check coordinates first in readPages (the body and jsonLd text is still produced for unreadable markup) and fall back to unreadable-markup only when they are clean. The second option keeps the most informative reason in the run record.

[tristan0x]

Each page is now scanned three times: here, in detectionText (through coordinateFailure), and in recordRefs (through refsOfPage). Each scan is linear, but on a 2 MiB page that is three full passes and three sets of text, script and href arrays for a result that is the same every time. A small refactor would let the two helpers take a scanned page, for example detectionText(html, page = scan(html)) and recordRefs(html, baseUrl, page = scan(html)). Then readPages scans once and passes the result to both. The public signatures would stay compatible with the existing tests.

[tristan0x]

An absolute wall-clock bound (below(2500)) on about 4.5 MB of input measures the machine as much as the algorithm. AGENTS.md already notes that the parallel suite is flaky under shard contention, and a slow CI runner or a busy shard could cross 2.5 s with a linear scan. A ratio is more robust and still catches a quadratic mutant: time each unit at n and 4n repetitions in the child process and assert that the larger run takes less than about 8x the smaller one (quadratic would give about 16x). Keep the spawnSync timeout as the hard backstop.