forked from bchanot/claude
Merge feature/dns-rebinding-guard into develop
This commit is contained in:
Bastien Chanot
@@ -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).
|
||||
|
||||
@@ -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 `<!DOCTYPE` past the window while
|
||||
`ET.fromstring` still parsed AND EXPANDED the entities (`&lol2;` →
|
||||
"lollollollollol", proven). Billion-laughs reopened on my own already-merged
|
||||
code. Found by the security review of the rebinding diff, not by me — fixed
|
||||
there rather than filed (root-cause discipline).
|
||||
- **future**: over ≤20 MB a full `re.search` is microseconds — no perf excuse
|
||||
for a bounded scan. Corollary of [[LRN-133]]: if you refuse a construct,
|
||||
refuse it EVERYWHERE, not just where you look first. A fresh adversarial
|
||||
reviewer attacking diff A routinely surfaces a real hole in already-shipped
|
||||
code B — see [[EVAL-020]].
|
||||
|
||||
@@ -120,6 +120,23 @@ fetch.sh cannibal --account client-a --property … [--days 90] [--rows 1000]
|
||||
Rows gained a `keys` list; `key` stays as keys[0], so the single-dim
|
||||
consumer is untouched.
|
||||
|
||||
safe_fetch.py — NOT a verb; the SSRF/DNS-rebinding-safe fetcher behind
|
||||
sitemap._fetch, so every network verb (sitemap, linkgraph, rendercheck,
|
||||
drift) inherits it. urlopen resolved then connected — two DNS lookups, 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. This resolves
|
||||
ONCE, validates every IP (ipaddress, dual-stack v4+v6), refuses if ANY is
|
||||
non-public (the multi-A vector), and connects to the exact validated IP with
|
||||
Host+SNI+cert for the real host — no second resolution to poison. Redirects
|
||||
are followed with each hop RE-VALIDATED (urlopen followed them blind).
|
||||
• 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 socket.getaddrinfo), stdlib-only (no requests).
|
||||
• Refusal raises UnsafeTarget; callers already degrade → fail-open kept.
|
||||
• NOT covered, and said so: the shell `curl` in the agent specs runs in
|
||||
another process, unpinnable from here. Smaller surface (fixed set vs an
|
||||
operator-confirmed $DOMAIN); `curl --resolve` would close it, separate change.
|
||||
|
||||
fetch.sh sitemap --url https://ex.com/sitemap.xml
|
||||
→ {"status":"ok","source":"sitemap","index":false,"count":86,"dropped":0,
|
||||
"urls":["https://ex.com/", …]}
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
#!/usr/bin/env python3
|
||||
"""SSRF- and DNS-rebinding-safe HTTP(S) fetch. Stdlib only.
|
||||
|
||||
The verbs that fetch remote content (sitemap, linkgraph, render_check, drift)
|
||||
all route through sitemap._fetch, which used urllib.request.urlopen. urlopen
|
||||
resolves the host, then connects — two DNS lookups with a window between them.
|
||||
A hostile authority can answer PUBLIC to the validation lookup and a PRIVATE
|
||||
address (169.254.169.254 cloud metadata, 127.0.0.1, the LAN) to the connect
|
||||
lookup. That is DNS rebinding, and a name-level guard cannot see it.
|
||||
|
||||
This collapses the two lookups into one: resolve ONCE, validate every returned
|
||||
IP, then connect to the exact validated IP while preserving the Host header,
|
||||
TLS SNI, and certificate validation for the real hostname. There is no second
|
||||
resolution to poison.
|
||||
|
||||
Better than the reference implementation this idea came from (claude-seo
|
||||
url_safety.py, MIT) on three axes, all verified before writing:
|
||||
- dual-stack: validates IPv4 AND IPv6 (theirs is IPv4-only);
|
||||
- no global state: each connection pins its own socket, so it is thread-safe
|
||||
by construction (theirs monkeypatches socket.getaddrinfo behind a global
|
||||
lock);
|
||||
- stdlib only: http.client + ssl + ipaddress, no `requests`.
|
||||
|
||||
NOT covered, stated rather than left silent: the shell `curl` calls in the
|
||||
agent specs (seo-analyzer/geo-analyzer STEP 4, the sameAs loop) run in a
|
||||
separate process and cannot be pinned from here. Their surface is smaller
|
||||
(a fixed set against an operator-typed/confirmed $DOMAIN). Closing them needs
|
||||
`curl --resolve` and is a separate change.
|
||||
"""
|
||||
import gzip
|
||||
import http.client
|
||||
import ipaddress
|
||||
import socket
|
||||
import ssl
|
||||
from urllib.parse import urljoin, urlparse
|
||||
|
||||
DEFAULT_TIMEOUT = 20
|
||||
DEFAULT_MAX_BYTES = 20 * 1024 * 1024
|
||||
MAX_REDIRECTS = 5
|
||||
|
||||
|
||||
class UnsafeTarget(Exception):
|
||||
"""A URL resolved to a non-public address, or a redirect did. Raised BEFORE
|
||||
any connection to that address. Callers already wrap _fetch in try/except
|
||||
and degrade, so the fail-open contract is preserved."""
|
||||
|
||||
|
||||
# Special-use ranges that `is_global` reports as public but are not legitimate
|
||||
# fetch targets. 192.88.99.0/24 = RFC 3068 6to4-relay anycast (a security
|
||||
# review flagged it 2026-07-17). Grows if more surface.
|
||||
_EXTRA_DENY = (ipaddress.ip_network("192.88.99.0/24"),)
|
||||
|
||||
|
||||
def _ip_is_public(ip_str):
|
||||
"""A globally routable unicast address, dual-stack. `is_global` is the
|
||||
decisive gate — it alone rejects CGNAT (100.64/10) that the per-flag checks
|
||||
miss — with the explicit flags plus an extra special-use deny list as
|
||||
defence in depth."""
|
||||
ip = ipaddress.ip_address(ip_str)
|
||||
if not ip.is_global:
|
||||
return False
|
||||
if any(ip in net for net in _EXTRA_DENY):
|
||||
return False
|
||||
return not (ip.is_private or ip.is_loopback or ip.is_link_local
|
||||
or ip.is_reserved or ip.is_multicast or ip.is_unspecified)
|
||||
|
||||
|
||||
def _resolve_pinned(host, port, resolver=socket.getaddrinfo):
|
||||
"""Resolve host ONCE and return [(family, ip)] for connecting. Refuse if
|
||||
ANY resolved address is non-public — a name advertising both public and
|
||||
private A records is exactly the multi-answer rebinding vector, and a
|
||||
legitimate public site does not do it. `resolver` is injected in tests to
|
||||
plant a private address and prove the refusal."""
|
||||
try:
|
||||
infos = resolver(host, port, type=socket.SOCK_STREAM)
|
||||
except socket.gaierror as e:
|
||||
raise UnsafeTarget("cannot resolve %r: %s" % (host, e))
|
||||
pinned = []
|
||||
for family, _type, _proto, _canon, sockaddr in infos:
|
||||
ip = sockaddr[0]
|
||||
if not _ip_is_public(ip):
|
||||
raise UnsafeTarget("%s resolves to non-public %s" % (host, ip))
|
||||
pinned.append((family, ip))
|
||||
if not pinned:
|
||||
raise UnsafeTarget("%s resolved to nothing" % host)
|
||||
return pinned
|
||||
|
||||
|
||||
class _PinnedHTTPSConnection(http.client.HTTPSConnection):
|
||||
"""HTTPS to a pinned IP, with SNI + cert validation for the real host."""
|
||||
def __init__(self, host, pinned_ip, family, **kw):
|
||||
super().__init__(host, **kw) # host → Host header + SNI
|
||||
self._pinned_ip = pinned_ip
|
||||
self._family = family
|
||||
|
||||
def connect(self):
|
||||
sock = socket.create_connection((self._pinned_ip, self.port),
|
||||
timeout=self.timeout)
|
||||
# server_hostname = the real host → SNI + hostname check both use it,
|
||||
# never the IP.
|
||||
self.sock = self._context.wrap_socket(sock, server_hostname=self.host)
|
||||
|
||||
|
||||
class _PinnedHTTPConnection(http.client.HTTPConnection):
|
||||
"""Plain HTTP to a pinned IP (Host header stays the real host)."""
|
||||
def __init__(self, host, pinned_ip, family, **kw):
|
||||
super().__init__(host, **kw)
|
||||
self._pinned_ip = pinned_ip
|
||||
self._family = family
|
||||
|
||||
def connect(self):
|
||||
self.sock = socket.create_connection((self._pinned_ip, self.port),
|
||||
timeout=self.timeout)
|
||||
|
||||
|
||||
def _one_request(url, timeout, max_bytes, resolver):
|
||||
"""One hop: resolve+pin the host, connect, return (status, headers, body)."""
|
||||
p = urlparse(url)
|
||||
if p.scheme not in ("http", "https"):
|
||||
raise UnsafeTarget("scheme must be http/https: %r" % url)
|
||||
host = p.hostname
|
||||
if not host:
|
||||
raise UnsafeTarget("no host in %r" % url)
|
||||
port = p.port or (443 if p.scheme == "https" else 80)
|
||||
family, ip = _resolve_pinned(host, port, resolver)[0] # any is public here
|
||||
ctx = ssl.create_default_context() if p.scheme == "https" else None
|
||||
if p.scheme == "https":
|
||||
conn = _PinnedHTTPSConnection(host, ip, family, port=port,
|
||||
timeout=timeout, context=ctx)
|
||||
else:
|
||||
conn = _PinnedHTTPConnection(host, ip, family, port=port,
|
||||
timeout=timeout)
|
||||
try:
|
||||
path = p.path or "/"
|
||||
if p.query:
|
||||
path += "?" + p.query
|
||||
# No Accept-Encoding: keep HTTP bodies un-gzipped; the .xml.gz
|
||||
# content-level case is handled by the caller's magic-byte check.
|
||||
conn.request("GET", path, headers={"Host": host,
|
||||
"User-Agent": "claude-seo-data/1.0"})
|
||||
r = conn.getresponse()
|
||||
body = r.read(max_bytes)
|
||||
return r.status, {k.lower(): v for k, v in r.getheaders()}, body
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def safe_fetch(url, timeout=DEFAULT_TIMEOUT, max_bytes=DEFAULT_MAX_BYTES,
|
||||
max_redirects=MAX_REDIRECTS, resolver=socket.getaddrinfo):
|
||||
"""Fetch url with resolve-then-pin, following redirects and RE-VALIDATING
|
||||
each hop — urlopen followed redirects to whatever address the Location
|
||||
named, re-opening the rebinding window on every hop. Returns the raw body
|
||||
bytes (the caller handles content-level gzip)."""
|
||||
seen = 0
|
||||
current = url
|
||||
while True:
|
||||
status, headers, body = _one_request(current, timeout, max_bytes, resolver)
|
||||
if status in (301, 302, 303, 307, 308) and "location" in headers:
|
||||
seen += 1
|
||||
if seen > max_redirects:
|
||||
raise UnsafeTarget("too many redirects from %r" % url)
|
||||
current = urljoin(current, headers["location"]) # re-validated next loop
|
||||
continue
|
||||
return body
|
||||
@@ -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 <!DOCTYPE past the
|
||||
# old raw[:4096] scan while ET still parsed+expanded it. Now the whole doc is
|
||||
# scanned. Prove a padded DTD is refused and the entity never expands.
|
||||
PADDED="$(python3 -c '
|
||||
import sys; sys.path.insert(0,"'"$SD"'"); import sitemap as sm
|
||||
bomb=("<?xml version=\"1.0\"?>\n<!-- "+("x"*5000)+" -->\n"
|
||||
"<!DOCTYPE d [ <!ENTITY lol \"lol\"> ]>\n<urlset><url><loc>https://x/&lol;</loc></url></urlset>").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" \
|
||||
|
||||
+13
-6
@@ -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"<!DOCTYPE" in head or b"<!ENTITY" in head:
|
||||
# Scan the WHOLE document, not a prefix. A security review (2026-07-17)
|
||||
# showed a >4 KB leading comment pushed <!DOCTYPE past the old raw[:4096]
|
||||
# window while ET.fromstring still parsed and EXPANDED the entities —
|
||||
# billion-laughs reopened. A legitimate sitemap contains neither construct
|
||||
# anywhere, so a full case-insensitive scan is correct; over ≤20 MB it is a
|
||||
# single re.search, microseconds, no 20 MB uppercased copy.
|
||||
import re # stdlib, lazy
|
||||
if re.search(rb"(?i)<!\s*(DOCTYPE|ENTITY)", raw):
|
||||
raise UnsafeXML("DTD in sitemap")
|
||||
|
||||
SITEMAP_NS = "{http://www.sitemaps.org/schemas/sitemap/0.9}"
|
||||
|
||||
+7
-5
@@ -16,11 +16,13 @@
|
||||
# vault and into a request. Allowlist, per CLAUDE.md: explicit allowlist beats
|
||||
# implicit denylist.
|
||||
#
|
||||
# NOT COVERED, deliberately: DNS-level SSRF. A public hostname that RESOLVES to
|
||||
# a private address passes this guard. Closing that needs resolve-then-pin at
|
||||
# the HTTP layer; curl in a shell cannot do it without a TOCTOU window between
|
||||
# the check and the connection. Literal local targets ARE rejected below. The
|
||||
# omission is stated rather than silent — see lib/seo-data/README.md.
|
||||
# DNS-level SSRF (a public hostname that RESOLVES to a private address, or
|
||||
# rebinds between check and connect): this NAME-level guard does not catch it —
|
||||
# closing it needs resolve-then-pin at the HTTP layer. That is now DONE for the
|
||||
# Python egress: lib/seo-data/safe_fetch.py pins every fetch (sitemap, linkgraph,
|
||||
# rendercheck, drift). It is NOT done for shell `curl`, which cannot pin without
|
||||
# `curl --resolve`; those paths keep this literal-local check only. Stated, not
|
||||
# silent — see lib/seo-data/README.md (safe_fetch).
|
||||
set -uo pipefail
|
||||
|
||||
_die() { echo "url-guard: $1" >&2; exit 2; }
|
||||
|
||||
Reference in New Issue
Block a user