From 2de58faa38e74cec19a40c581d115954013c24d6 Mon Sep 17 00:00:00 2001 From: Bastien Chanot Date: Fri, 17 Jul 2026 11:28:00 +0200 Subject: [PATCH] =?UTF-8?q?feat(seo-data):=20C1b=20=E2=80=94=20sitemap=20v?= =?UTF-8?q?erb,=20the=20denominator=20COVERAGE=20never=20had?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I5 made a COVERAGE line mandatory in STEP 9 and told the agent to "count the URLs in sitemap.xml" without giving it a command. STEP 4 only ever did `curl … | head -50` — a preview, not a count. This closes that. fetch.sh sitemap --url … → {count, urls[], index, dropped}. Stdlib only (urllib + xml.etree + gzip): no auth, no Google, no venv, so it runs wherever the mock/degrade paths run. Follows one level, dedupes, strips whitespace, handles .xml.gz. Every cap REPORTS what it cut (children_skipped, truncated) rather than truncating silently — same rule as COVERAGE itself. PLAN CORRECTION: the proposal said the verb would "validate each URL via the H1 guard". Wrong. urllib fetches these, so nothing here reaches a shell and there is no injection surface to guard. The guard belongs at the point of use, where seo-analyzer interpolates a URL into curl — which is the contract the sameAs check already established. A second copy of url-guard here would only drift from the first. The module carries a garbage filter, named as such. SECURITY: the security-guidance hook asked for defusedxml. Taken seriously, not obeyed — it would drag a venv into a module whose whole point is being stdlib-only. Split the threat instead: xml.etree does NOT expand external entities (XXE is not the vector), but it IS billion-laughs-vulnerable, and the 20 MB read ceiling bounds the input, not the expansion. A sitemap NEVER has a DTD — sitemaps.org is then — so any doctype/entity is refused BEFORE parsing, with its own reason (unsafe_xml_dtd, distinct from parse_failed: it is a finding, not a glitch). Refusing the construct beats depending on parser internals. Fixture is a real billion-laughs payload. Verified against the live target, not just fixtures: zenquality's sitemap returns count=86, dropped=0, matching `grep -c ''` on the raw XML exactly. Dead URL → {"status":"degraded","reason":"fetch_failed"}, exit 0. seo-data 95 -> 110 pass, 0 fail; full suite green; shellcheck + py_compile clean. Note: no config-edit sentinel was needed after all — config-protection guards lib/tests, not lib/seo-data. I posted one, found it uncommitted-and-unconsumed afterwards, and removed it rather than leave an open one-shot gate lying around. Worth knowing: seo-data.test.sh is 110 assertions and is NOT covered by that hook, while lib/tests/*.test.sh is. --- agents/seo-analyzer.md | 38 +++- lib/seo-data/README.md | 26 +++ lib/seo-data/fetch.sh | 5 +- lib/seo-data/fixtures-sitemap-dtd/sitemap.xml | 10 ++ .../fixtures-sitemap-index/sitemap.xml | 5 + .../fixtures-sitemap-index/sitemap_child.xml | 5 + lib/seo-data/fixtures/sitemap.xml | 15 ++ lib/seo-data/seo-data.test.sh | 30 ++++ lib/seo-data/sitemap.py | 163 ++++++++++++++++++ 9 files changed, 290 insertions(+), 7 deletions(-) create mode 100644 lib/seo-data/fixtures-sitemap-dtd/sitemap.xml create mode 100644 lib/seo-data/fixtures-sitemap-index/sitemap.xml create mode 100644 lib/seo-data/fixtures-sitemap-index/sitemap_child.xml create mode 100644 lib/seo-data/fixtures/sitemap.xml create mode 100644 lib/seo-data/sitemap.py diff --git a/agents/seo-analyzer.md b/agents/seo-analyzer.md index 9de8ffe..80759ce 100644 --- a/agents/seo-analyzer.md +++ b/agents/seo-analyzer.md @@ -444,12 +444,38 @@ Fetch rendered HTML. Extract and analyze: ## STEP 5 — ON-PAGE AUDIT `[both]` **Record the denominator BEFORE sampling.** This step samples; the report -says "audit". Count the URLs in `sitemap.xml` (fetch it in full — the -`head -50` in STEP 4 is a preview, not a count). That count is the coverage -denominator, and it feeds the mandatory COVERAGE line in STEP 9. No sitemap -→ denominator unknown: say so, never let silence imply full coverage. On a -500-page site a 12-page sample is 2.4% — the On-page score is an -extrapolation from it, and the reader cannot know that unless you print it. +says "audit". On a 500-page site a 12-page sample is 2.4% — the On-page score +is an extrapolation from it, and the reader cannot know unless you print it. + +```bash +bash ~/.claude/lib/seo-data/fetch.sh sitemap --url "https://$DOMAIN/sitemap.xml" +``` + +Returns `{count, urls[], index, dropped, ...}` — the coverage denominator and +your sampling frame. It follows a `` one level, dedupes, strips +whitespace, and handles `.xml.gz`. No auth, no venv, no Google. + +Read it honestly: +- `count` → the denominator for the STEP 9 COVERAGE line. +- `dropped > 0` → entries that were not usable URLs. Worth a §14 line: a + sitemap emitting junk is a tooling finding. +- `children_failed > 0` or `children_skipped` → the frame is incomplete. Say + so; do NOT present a partial denominator as the total. +- `status: degraded` → denominator UNKNOWN. Print that, never let silence + imply full coverage. `reason: unsafe_xml_dtd` is not a glitch — a sitemap + carrying a DTD is broken tooling or a billion-laughs aimed at the auditor. + Report it as a finding. + +**Guard every URL before it reaches curl.** These come from the target's own +server, not from the operator — the one place in this audit where a remote +file's bytes flow into a shell: + +```bash +U="$(bash ~/.claude/lib/url-guard.sh url "$RAW_FROM_SITEMAP")" || continue +``` + +The verb applies a garbage filter, not that guard; the guard belongs at the +point of use (same contract as the sameAs check in geo-analyzer). ### Meta tags per page (sample 5-15 key pages) diff --git a/lib/seo-data/README.md b/lib/seo-data/README.md index c21b097..faf1792 100644 --- a/lib/seo-data/README.md +++ b/lib/seo-data/README.md @@ -98,6 +98,32 @@ fetch.sh inspect --account client-a --property … --url https://ex.com/page • errors/warnings count issue INSTANCES; issues[] is deduped — the same issueMessage repeats across every affected item. +fetch.sh sitemap --url https://ex.com/sitemap.xml + → {"status":"ok","source":"sitemap","index":false,"count":86,"dropped":0, + "urls":["https://ex.com/", …]} + → {"status":"ok","index":true,"children_total":4,"children_read":4, + "children_failed":0,"count":312,…} # , one level deep + → {"status":"degraded","reason":"fetch_failed"|"parse_failed"|"no_urls" + |"unsafe_xml_dtd"} + + No auth, no Google, no venv: stdlib only (urllib + xml.etree + gzip). + Gives STEP 9's COVERAGE line the denominator it was told to print and never + had, and STEP 5 a real sampling frame. Dedupes, strips whitespace, handles + .xml.gz. Caps: 50 children of an index, 50k URLs, 20 MB read — each cut is + REPORTED (children_skipped / truncated), never silent. + + • NOT a security boundary. urllib fetches these, so nothing here reaches a + shell. The CONSUMER interpolates them into curl, so seo-analyzer runs + lib/url-guard.sh at the point of use — same contract as the sameAs check. + A second copy of the guard here would only drift. + • `unsafe_xml_dtd`: a sitemap NEVER has a DTD (sitemaps.org is then + ). Any doctype/entity is refused BEFORE parsing. xml.etree + does not expand external entities, but it IS billion-laughs-vulnerable — + 1 KB expands to gigabytes, and the 20 MB read ceiling bounds the input, + not the expansion. Refusing the construct beats depending on parser + internals AND keeps this stdlib-only; defusedxml would drag in a venv for + a document type that has no legitimate DTD. + fetch.sh forget --label client-a → {"status":"ok","removed":true|false} # false = label wasn't in the store diff --git a/lib/seo-data/fetch.sh b/lib/seo-data/fetch.sh index ede0ee8..8ca859d 100644 --- a/lib/seo-data/fetch.sh +++ b/lib/seo-data/fetch.sh @@ -29,6 +29,9 @@ case "$cmd" in accounts) exec "$PY" "$HERE/tokenstore.py" list --file "$STORE" ;; crux|queries|inspect) exec "$PY" "$HERE/google_seo.py" "$cmd" --store "$STORE" "$@" ;; + # No auth, no Google: stdlib-only, runs even without the venv. + sitemap) + exec "$PY" "$HERE/sitemap.py" --store "$STORE" "$@" ;; forget) # forget --label