From b00e8ef44222ae407b3e8a07a73ea27f9fdb508b Mon Sep 17 00:00:00 2001 From: Bastien Chanot Date: Fri, 17 Jul 2026 19:58:59 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(seo-data):=20safe=5Ffetch=20=E2=80=94?= =?UTF-8?q?=20resolve-then-pin,=20close=20DNS-rebinding=20+=20SSRF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 max_redirects: + raise UnsafeTarget("too many redirects from %r" % url) + current = urljoin(current, headers["location"]) # re-validated next loop + continue + return body diff --git a/lib/seo-data/seo-data.test.sh b/lib/seo-data/seo-data.test.sh index fe8012a..d13d260 100644 --- a/lib/seo-data/seo-data.test.sh +++ b/lib/seo-data/seo-data.test.sh @@ -99,6 +99,47 @@ check_first() { [ "$1" = "$2" ] && ok "$3" || no "$3" "got[$1]"; } check_first "$CAN_FIRST" "urgence fuite https://ex.com/urgence" "cannibal ranks by impact" has "cannibal reports the cap" "$CAN" '"capped": false' +echo "── safe_fetch (DNS-rebinding / SSRF) ──" +# Inject a hostile resolver: the name is public, the address is internal. This +# is the rebinding vector a name-level guard cannot see — prove it is refused +# BEFORE any connection. Deterministic + offline via the injected resolver. +sfpy() { PYTHONPATH="$SD" python3 -c "$1" 2>&1; } +REBIND="$(sfpy ' +import socket, safe_fetch as sf +def meta(h,p,**k): return [(socket.AF_INET,socket.SOCK_STREAM,6,"",("169.254.169.254",p))] +try: sf.safe_fetch("https://evil.example/", resolver=meta); print("CONNECTED") +except sf.UnsafeTarget as e: print("REFUSED", e)')" +has "rebind to metadata refused" "$REBIND" 'REFUSED' +has "refusal names the ip" "$REBIND" '169.254.169.254' +hasnt "never connected" "$REBIND" 'CONNECTED' +MIXED="$(sfpy ' +import socket, safe_fetch as sf +def mix(h,p,**k): return [(socket.AF_INET,socket.SOCK_STREAM,6,"",("93.184.216.34",p)), + (socket.AF_INET,socket.SOCK_STREAM,6,"",("127.0.0.1",p))] +try: sf.safe_fetch("https://evil.example/", resolver=mix); print("CONNECTED") +except sf.UnsafeTarget as e: print("REFUSED")')" +has "multi-A public+private refused" "$MIXED" 'REFUSED' +# classification, dual-stack — is_global catches CGNAT the per-flags miss +CLS="$(sfpy ' +import safe_fetch as sf +pub=[c for c in ["8.8.8.8","2606:2800:220:1:248:1893:25c8:1946"] if sf._ip_is_public(c)] +bad=[c for c in ["169.254.169.254","127.0.0.1","10.0.0.1","192.168.1.1","100.64.1.1","::1","fe80::1","0.0.0.0"] if sf._ip_is_public(c)] +print("PUB",len(pub),"BADPASS",len(bad))')" +has "public v4+v6 pass" "$CLS" 'PUB 2' +has "no internal ip passes" "$CLS" 'BADPASS 0' +# security review 2026-07-17: 6to4-relay anycast passes is_global — extra deny +SIXTOFOUR="$(sfpy 'import safe_fetch as sf; print("6TO4", sf._ip_is_public("192.88.99.1"))')" +has "6to4 relay anycast refused" "$SIXTOFOUR" '6TO4 False' +# scheme + stdlib +SCHEME="$(sfpy ' +import safe_fetch as sf +try: sf.safe_fetch("file:///etc/passwd"); print("OK") +except sf.UnsafeTarget: print("REFUSED")')" +has "non-http scheme refused" "$SCHEME" 'REFUSED' +IMP="$(/bin/grep -E "^(import|from) " "$SD/safe_fetch.py" | /bin/grep -cvE "gzip|http\.client|ipaddress|socket|ssl|urllib\.parse")" +[ "$IMP" = "0" ] && ok "safe_fetch is stdlib-only" || no "safe_fetch is stdlib-only" "$IMP non-stdlib imports" +hasnt "no requests dependency" "$(cat "$SD/safe_fetch.py")" 'import requests' + echo "── sitemap ──" SM="$(SEO_DATA_MOCK_DIR="$MOCK" python3 "$SD/sitemap.py" --url https://ex.com/sitemap.xml)" has "sitemap ok" "$SM" '"status": "ok"' @@ -134,6 +175,16 @@ DTD="$(SEO_DATA_MOCK_DIR="$SD/fixtures-sitemap-dtd" python3 "$SD/sitemap.py" \ has "billion-laughs refused" "$DTD" '"status": "degraded"' has "dtd reason is distinct" "$DTD" 'unsafe_xml_dtd' hasnt "dtd never parsed" "$DTD" '"count"' +# security review 2026-07-17: a >4KB leading comment pushed \n\n" + " ]>\nhttps://x/&lol;").encode() +try: sm._refuse_dtd(bomb); print("PARSED") +except sm.UnsafeXML: print("REFUSED")')" +has "padded DTD refused (full scan)" "$PADDED" 'REFUSED' echo "── render_check (R2) ──" SPA="$(SEO_DATA_MOCK_DIR="$SD/fixtures-spa" python3 "$SD/render_check.py" \ diff --git a/lib/seo-data/sitemap.py b/lib/seo-data/sitemap.py index 99454ab..5eb3afa 100644 --- a/lib/seo-data/sitemap.py +++ b/lib/seo-data/sitemap.py @@ -29,10 +29,11 @@ def _mock(name): return f.read() def _fetch(url): - from urllib.request import urlopen, Request # stdlib, lazy - req = Request(url, headers={"User-Agent": "claude-seo-data/1.0"}) - with urlopen(req, timeout=TIMEOUT) as r: # nosec: audited target - raw = r.read(20 * 1024 * 1024) # 20 MB ceiling + # SSRF + DNS-rebinding safe: resolve-then-pin, redirects re-validated. + # This is the single seam for ALL network egress — linkgraph/render_check/ + # drift all call sitemap._fetch — so pinning here covers every verb. + import safe_fetch # sibling, lazy + raw = safe_fetch.safe_fetch(url, timeout=TIMEOUT, max_bytes=20 * 1024 * 1024) if raw[:2] == b"\x1f\x8b": # sitemap.xml.gz is common raw = gzip.decompress(raw) return raw @@ -53,8 +54,14 @@ def _refuse_dtd(raw): google_seo.py's mock/degrade paths. A sitemap with a DTD is not a sitemap we want anyway. """ - head = raw[:4096].lstrip()[:2048].upper() - if b"4 KB leading comment pushed &2; exit 2; } From a391be4906c27529b4327dcce6dc315ad56d2aaf Mon Sep 17 00:00:00 2001 From: Bastien Chanot Date: Fri, 17 Jul 2026 20:16:50 +0200 Subject: [PATCH 2/2] =?UTF-8?q?chore(memory):=20LRN-134=20LRN-135=20?= =?UTF-8?q?=E2=80=94=20capitalize?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude/memory/journal.md | 1 + .claude/memory/learnings.md | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/.claude/memory/journal.md b/.claude/memory/journal.md index 3e554b3..0c27d3e 100644 --- a/.claude/memory/journal.md +++ b/.claude/memory/journal.md @@ -396,6 +396,7 @@ rules: - BDR-068 (close-auto-persist) MERGED to develop + pushed. Then cut + pushed **v1.1.0** (minor, that feature). Standard forward bump → sonnet release-executor ran BOTH spans (prep + finish+tag); lineage continued 1.0.0→1.1.0 not 5.x (validates [[BDR-067]]). origin: main=2f8dc6b, develop=21b1e21, tags v1.0.0 + v1.1.0. WATCH-ITEM: a stale local tag `v4.0.0` reappeared during the release — NOT from origin (origin never regained it; `push.followTags` off; its commit unreachable from develop/main). Inert (push targeted main/develop/v1.1.0 explicitly + deleted the local copy; origin verified clean). Mechanism unexplained — if `v4.0.0` resurfaces locally after a `gitflow` op, trace the release lib (gitflow.sh / release-executor) for stray tag re-creation. ## 2026-07-17 +- safe_fetch DNS-rebinding guard shipped by-principle (feature/dns-rebinding-guard): resolve-then-pin in stdlib http.client, closes SSRF+rebinding for the Python egress (4 verbs via sitemap._fetch), better than claude-seo url_safety on 3 axes. Fresh security-auditor VERDICT PASS + surfaced a REAL billion-laughs hole in my own already-merged C1b (prefix-only DTD scan bypassed by >4KB padding, entity expanded — proven, fixed here). LRN-134/135 capitalized. seo-data 210→221. claude-seo question CLOSED: 3 pieces taken (schema_gen/content_quality/safe_fetch), rest killed-at-measure or rejected-on-principle. - content_quality verb shipped via /feat (2nd cherry-pick, stacked on feature/seo-data-cherry-picks): deterministic filler/AI-slop signal (QRG list intact, no LLM), advisory-not-verdict wired into geo STEP 8. GATE 1 CONFORME 10/10 both verbs, seo-data 190→210. Two easy claude-seo picks DONE; url_safety (DNS-rebinding) still deferred pending threat-model. Branch carries 2 feat + 1 journal commit, UNMERGED (human gate). - Gap-revisit claude-seo after the 21-commit build: remaining cherry-pick value narrowed to 2 clean stdlib picks + url_safety (DNS-rebinding, deferred on threat-model). schema_gen verb shipped via /feat (honors [[BDR-070]] adapt-not-copy): generates JSON-LD (Reservation/OrderAction/DiscussionForumPosting/ProfilePage), the system only audited before. GATE 1 CONFORME 10/10, seo-data 167→190 pass. content_quality next (same /feat, stacked — shares fetch.sh/test/README). - seo/geo parity vs github.com/AgriciDaniel/claude-seo (11.5k★, MIT): full 20-point plan built from a 3-subagent inventory, then executed. Verdict cherry-pick-never-install ([[BDR-070]]). 21 commits: Phase 1 (I1-I8 integrity, markdown specs) MERGED to develop (02c7a6f, 8 commits); Phases 2-7 on bugfix/seo-geo-integrity UNMERGED (13 commits, human gate). `fetch.sh` 5→11 verbs (richresults via inspect, sitemap, rendercheck, linkgraph, cannibal, drift, score); seo-data test suite 85→167 pass, 0 fail. Dogfooded on 2 live sites (zenquality Astro + lavageangels356 native PHP) — the second caught 2 bugs Astro hid (image:loc counted as page, flat-URL family heuristic). diff --git a/.claude/memory/learnings.md b/.claude/memory/learnings.md index 628c5b8..66c4b9e 100644 --- a/.claude/memory/learnings.md +++ b/.claude/memory/learnings.md @@ -136,6 +136,8 @@ rules: | LRN-131 | 2026-07-17 | WebSearch is NOT verification for a number — SEO blogs cross-cite into fake consensus; require primary source + `measured:` field | any stat headed for a client report; verifying a metric/claim exists | | LRN-132 | 2026-07-17 | a subagent summary is a CLAIM, not a fact — 7 disproven in one session (incl. 3 I reproduced writing the fixes) | before planning on any relayed finding; verify vs primary source / live test first | | LRN-133 | 2026-07-17 | an omission must stay LEGIBLE, never silent — tool that can't measure says so in its output | designing any audit/measure output; deciding what a cap/refusal/N-A emits | +| LRN-134 | 2026-07-17 | resolve-then-pin in stdlib http.client beats monkeypatching getaddrinfo — dual-stack, thread-safe, no requests; classify the OS-resolved IP not the URL text | closing SSRF/DNS-rebinding on any Python HTTP egress | +| LRN-135 | 2026-07-17 | a prefix-only scan for a dangerous construct is bypassable by padding — scan the WHOLE document | refusing any hostile construct (DTD/directive/marker) before parse | --- @@ -1301,3 +1303,39 @@ rules: - **pattern**: when a tool cannot measure something, it says so IN its output — a caller must never read absence as "fine". - **context**: red thread of 21 commits — NAP with no canonical → finding WITHOUT direction (never pick from source majority); unmeasured backlinks → mandatory §14 line; sample → mandatory COVERAGE ratio; dropped security headers → §14 + "run /harden" pointer; capped crawl → `orphans_withheld` (the cap doesn't degrade the result, it INVALIDATES it — a partial-crawl orphan is a false orphan); SPA → refuse, don't score; N/A ≠ zero in the scorer. - **future**: the system already HAD the invariant (code-ceiling, §14 Annexe) but applied it in spots. Generalised it. A false signal is worse than a declared gap — the 4 features KILLED at measurement (B1/B2/B3/W2) beat 4 false-signal features. See [[LRN-131]]/[[LRN-132]] (same session, the verification discipline that feeds it). + +## LRN-134 — resolve-then-pin in stdlib beats monkeypatching getaddrinfo — 2026-07-17 +- **pattern**: to close SSRF/DNS-rebinding on Python HTTP egress, resolve the + host ONCE, validate every returned IP (`ipaddress`, dual-stack v4+v6), refuse + if ANY is non-public (the multi-A vector), then connect to the exact pinned IP + via an `http.client.HTTPSConnection` subclass whose `connect()` does + `create_connection((pinned_ip, port))` and `wrap_socket(sock, + server_hostname=real_host)` — SNI + cert stay bound to the real host. No + second resolution to poison. `safe_fetch.py`. +- **context**: the load-bearing property — classify the IP the OS RESOLVED + (`sockaddr[0]`), NEVER the URL text. That defeats octal/hex/decimal literals, + IPv4-mapped IPv6, NAT64, 6to4 structurally, not by enumeration (confirmed by + the security review's fuzz). `is_global` is the decisive gate (catches CGNAT + 100.64/10 the per-flags miss); add a small extra-deny for special-use ranges + it passes (192.88.99.0/24 6to4-relay). Redirects: re-validate EACH hop — + urlopen followed them blind. +- **future**: beats claude-seo url_safety.py on 3 axes — dual-stack (theirs + IPv4-only), thread-safe by construction (theirs monkeypatches getaddrinfo + behind a global lock), stdlib-only (theirs `requests`). A name-level guard + (url-guard.sh) cannot see a rebind; this is the layer that can. Shell `curl` + stays unpinnable from here → `curl --resolve`, separate. + +## LRN-135 — a prefix-only scan for a dangerous construct is bypassable by padding — 2026-07-17 +- **pattern**: to refuse a hostile construct (DTD, directive, marker) before + parsing, scan the WHOLE document, never a bounded prefix. +- **context**: `_refuse_dtd` (C1b) scanned only `raw[:4096]` → a sitemap with + >4 KB of leading comment pushed `