2560905be2cdf9e18fdf33d979ba2100b1ce222c
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Bastien Chanot
|
b00e8ef442 |
feat(seo-data): safe_fetch — resolve-then-pin, close DNS-rebinding + SSRF
By-principle hardening. H1's url-guard validates the NAME; urlopen then resolved AND connected — two DNS lookups with a window a hostile authority uses to answer PUBLIC to validation and PRIVATE (169.254.169.254 metadata, 127.0.0.1, the LAN) to the connect. A name-level guard cannot see that rebind. safe_fetch collapses the two lookups into one: resolve ONCE, validate every IP (ipaddress, dual-stack v4+v6), refuse if ANY is non-public (the multi-A vector), connect to the exact validated IP with Host+SNI+cert for the real host — no second resolution to poison. Redirects re-validate each hop (urlopen followed them blind). One seam: sitemap._fetch, which linkgraph/render_check/drift all call, so every network verb inherits it. The load-bearing property (confirmed by the security review): classification is on the OS-resolved address (sockaddr[0]), never the URL text — so octal/hex/ decimal literals, IPv4-mapped IPv6, NAT64, 6to4 are all defeated structurally, not by enumeration. Better than the source idea (claude-seo url_safety.py, MIT): dual-stack (theirs IPv4-only), no global monkeypatch so thread-safe by construction (theirs locks a patched getaddrinfo), stdlib-only (no requests). Proven end-to-end before writing: pinned connect keeps SNI+cert for the real host. NOT covered, stated not silent: shell `curl` in the agent specs (separate process, unpinnable here). Smaller surface; `curl --resolve` is a separate change. REVIEW-SURFACED (fresh security-auditor, adversarial, VERDICT PASS) — two real holes it found while attacking the diff, both fixed here: - billion-laughs REOPENED in C1b: _refuse_dtd scanned only raw[:4096], so a >4KB leading comment pushed <!DOCTYPE past the window while ET parsed AND EXPANDED the entities. Proven (&lol2; → "lollollollollol"), now a full-doc case-insensitive scan. This is a genuine fix to already-merged C1b, not this feature — fixed here rather than filed, per root-cause discipline. - 192.88.99.0/24 (6to4-relay anycast) passed is_global as public — added to an extra special-use deny list. Verified: rebind-to-metadata refused BEFORE any connect (injected resolver), multi-A public+private refused, classifier fuzzed dual-stack incl. CGNAT/6to4, non-http scheme refused, both review fixes proven with no false positive; real fetch still works (zenquality 86 loc, lavageangels 24) through the pinned path; all 4 verbs work end-to-end via fetch.sh; seo-data 210 → 221 pass, 0 fail; full suite green; shellcheck + py_compile clean. |
||
|
Bastien Chanot
|
dca977bb27 |
fix(seo-data,seo): backtest on a second, native site — two real bugs
Everything on this branch was grounded on ONE Astro repo. A native PHP site
(lavageangels356.fr) broke two things that looked fine there.
BUG 1 — sitemap counted images as pages. _locs matched
`el.tag.endswith("}loc")`, and <image:loc> from Google's image-sitemap
namespace ALSO ends with '}loc'. Astro's sitemap has no image extension, so
this was invisible. The native site's does: 24 <url> + 3 <image:loc> came back
as count=27. The COVERAGE denominator was 12.5% too high and img/logo.png was
about to be sampled and audited as a page.
Fixed with two locks: walk the DIRECT children of each <url>/<sitemap> instead
of root.iter() (which alone excludes <image:image><image:loc>), and test the
sitemaps.org namespace explicitly. Regression fixture carries the image
extension; the old endswith code returns 9 URLs against it, the new one 7 with
zero images.
Verified both sites: native 27 -> 24, zero images; Astro unchanged at 86.
BUG 2 — the C1c family heuristic was tuned to one URL layout. "First path
segment" works for NESTED city pages (/creation-site-internet/essonne-91/ →
25 pages, 1 family) and FAILS for FLAT ones (/lavage-auto-pomponne,
/lavage-auto-torcy → 8 pages, 8 singletons). Consequence: C1c's rule "sample
>=3 from the largest family" would have targeted /services (5) and missed the
8 city pages entirely — the exact doorway-page risk the 30/70 rule exists to
catch.
Family is now "shared parent path OR shared slug prefix (>=3 URLs sharing 2+
hyphen tokens)", with both real layouts as the worked examples, plus a
sanity-check: a sitemap yielding almost as many families as URLs has defeated
the heuristic, not proved the site has no templates. Fixed in seo-analyzer and
in the geo pointer that referenced it.
Backtest results on the native site for everything else: url-guard accepts the
domain; source-scope excludes only .git (no dist/build/out exists — the
exclusions are correctly no-ops, and cache/ holds only .htaccess+.gitignore so
it is rightly untouched); the sameAs check runs and finds zero (a real GEO gap
for that site, not a tool bug); links are present in the served HTML (PHP is
SSR), so C3 is feasible there.
Verified: seo-data 119 -> 122 pass, 0 fail; full suite green; py_compile clean.
|
||
|
Bastien Chanot
|
2de58faa38 |
feat(seo-data): C1b — sitemap verb, the denominator COVERAGE never had
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 <sitemapindex> 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 <?xml?> then <urlset xmlns=> — 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 '<loc>'` 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.
|