diff --git a/.superpowers/sdd/evidence-task-2-report.md b/.superpowers/sdd/evidence-task-2-report.md index a803032f..583ffda0 100644 --- a/.superpowers/sdd/evidence-task-2-report.md +++ b/.superpowers/sdd/evidence-task-2-report.md @@ -54,3 +54,12 @@ Four review findings were closed in a separate follow-up commit: conditional headers; a 304 reuses only previously verified cached bytes and identity. The LRU content cache has an explicit byte bound (`max_cache_bytes`). Validators are not forwarded across redirect origins. + +### Conditional cache binding correction + +The conditional cache now binds bytes and validators to both the canonical provenance key and the +exact final effective representation URL. Redirect traversal recomputes request headers per hop: +validators are sent only when that exact URL matches the cached final URL, never merely because a +redirect retains an origin. A same-origin path change therefore downloads and replaces the body. +The adapter accepts 304 only when the exact request carried a bound ETag or Last-Modified validator; +unsolicited and cross-origin 304 responses are permanent protocol errors. diff --git a/harness/tests/test_http_evidence_source.py b/harness/tests/test_http_evidence_source.py index d668e9b8..38883dea 100644 --- a/harness/tests/test_http_evidence_source.py +++ b/harness/tests/test_http_evidence_source.py @@ -11,6 +11,9 @@ from tht.ports.evidence import EvidenceSourceError class Handler(BaseHTTPRequestHandler): etag_requests = 0 etag_body_responses = 0 + redirect_target = "/redirected-v1" + redirect_request_validators = [] + final_request_validators = [] def do_GET(self): if self.path.startswith("/etag"): @@ -46,6 +49,24 @@ class Handler(BaseHTTPRequestHandler): "Location", f"http://user:password@127.0.0.1:{self.server.server_port}/etag" ) self.end_headers() + elif self.path == "/stable-redirect": + type(self).redirect_request_validators.append(self.headers.get("If-None-Match")) + self.send_response(302) + self.send_header("Location", type(self).redirect_target) + self.end_headers() + elif self.path in {"/redirected-v1", "/redirected-v2"}: + type(self).final_request_validators.append( + (self.path, self.headers.get("If-None-Match")) + ) + etag = '"v1"' if self.path.endswith("v1") else '"v2"' + if self.headers.get("If-None-Match") == etag: + self.send_response(304) + self.end_headers() + return + self.send_response(200) + self.send_header("ETag", etag) + self.end_headers() + self.wfile.write(self.path.encode()) else: self.send_response(200) self.send_header("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT") @@ -60,6 +81,9 @@ class Handler(BaseHTTPRequestHandler): def server_url(): Handler.etag_requests = 0 Handler.etag_body_responses = 0 + Handler.redirect_target = "/redirected-v1" + Handler.redirect_request_validators = [] + Handler.final_request_validators = [] server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() @@ -246,3 +270,62 @@ def test_http_closes_response_when_streaming_fails(): with pytest.raises(EvidenceSourceError): list(source.discover()) assert response.closed + + +def test_http_binds_validators_to_exact_final_redirect_url(server_url): + source = HttpManifestEvidenceSource( + [server_url + "/stable-redirect"], allow_private_hosts=True + ) + first = next(iter(source.discover())) + second = next(iter(source.discover())) + + assert first == second + assert Handler.redirect_request_validators == [None, None] + assert Handler.final_request_validators == [ + ("/redirected-v1", None), + ("/redirected-v1", '"v1"'), + ] + + +def test_http_redirect_path_change_fetches_and_replaces_body(server_url): + source = HttpManifestEvidenceSource( + [server_url + "/stable-redirect"], allow_private_hosts=True + ) + first = next(iter(source.discover())) + assert source.acquire(first).content == b"/redirected-v1" + + Handler.redirect_target = "/redirected-v2" + with pytest.raises(EvidenceSourceError): + source.acquire(first) + current = next(iter(source.discover())) + + assert source.acquire(current).content == b"/redirected-v2" + assert ("/redirected-v2", None) in Handler.final_request_validators + + +def test_http_rejects_unsolicited_304_without_bound_validator(monkeypatch): + monkeypatch.setattr(socket, "getaddrinfo", lambda *args, **kwargs: [ + (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("93.184.216.34", 443)), + ]) + source = HttpManifestEvidenceSource(["https://example.test/doc"]) + response = FakeResponse(peer="93.184.216.34") + response.status_code = 304 + source._session = FakeSession(response) + + with pytest.raises(EvidenceSourceError) as caught: + list(source.discover()) + assert not caught.value.retryable + assert response.closed + + +def test_http_rejects_cross_origin_304_for_cached_provenance(server_url): + source = HttpManifestEvidenceSource([server_url + "/etag"], allow_private_hosts=True) + item = next(iter(source.discover())) + response = FakeResponse() + response.status_code = 304 + source._session = FakeSession(response) + + with pytest.raises(EvidenceSourceError) as caught: + source._download("https://other.example/doc", item.uri) + assert not caught.value.retryable + assert response.closed diff --git a/harness/tht/adapters/evidence/http.py b/harness/tht/adapters/evidence/http.py index 0d9c0e03..9c895693 100644 --- a/harness/tht/adapters/evidence/http.py +++ b/harness/tht/adapters/evidence/http.py @@ -57,7 +57,8 @@ class HttpManifestEvidenceSource: self._session = requests.Session() self._session.trust_env = False self._cache: OrderedDict[str, AcquiredDocument] = OrderedDict() - self._validators: dict[str, tuple[str | None, str | None]] = {} + # provenance -> (exact final effective URL, ETag, Last-Modified) + self._validators: dict[str, tuple[str, str | None, str | None]] = {} self._cache_bytes = 0 def __repr__(self) -> str: @@ -137,12 +138,14 @@ class HttpManifestEvidenceSource: return EvidenceSourceErrorCategory.TRANSIENT return EvidenceSourceErrorCategory.PERMANENT - def _conditional_headers(self, provenance: str) -> dict[str, str]: + def _conditional_headers(self, provenance: str, request_url: str) -> dict[str, str]: cached = self._cache.get(self._source_id(provenance)) validators = self._validators.get(provenance) if cached is None or validators is None: return {} - etag, last_modified = validators + final_url, etag, last_modified = validators + if request_url != final_url: + return {} headers = {} if etag: headers["If-None-Match"] = etag @@ -153,6 +156,7 @@ class HttpManifestEvidenceSource: def _remember( self, provenance: str, + final_url: str, document: AcquiredDocument, validators: tuple[str | None, str | None], ) -> None: @@ -162,7 +166,7 @@ class HttpManifestEvidenceSource: self._cache_bytes -= len(old.content) self._cache[source_id] = document self._cache_bytes += len(document.content) - self._validators[provenance] = validators + self._validators[provenance] = (final_url, *validators) while self._cache and self._cache_bytes > self.max_cache_bytes: evicted_id, evicted = self._cache.popitem(last=False) self._cache_bytes -= len(evicted.content) @@ -172,9 +176,9 @@ class HttpManifestEvidenceSource: def _download(self, transport_url: str, provenance: str) -> AcquiredDocument: current = transport_url - headers = self._conditional_headers(provenance) try: for redirect_count in range(self.max_redirects + 1): + headers = self._conditional_headers(provenance, current) allowed = self._resolve_allowed(current) response = None try: @@ -194,25 +198,18 @@ class HttpManifestEvidenceSource: self._validate_url_shape(destination) except ValueError as error: raise self._safe_error("redirect") from error - current_origin = urlsplit(current) - destination_origin = urlsplit(destination) - if ( - current_origin.scheme, - current_origin.hostname, - current_origin.port, - ) != ( - destination_origin.scheme, - destination_origin.hostname, - destination_origin.port, - ): - headers = {} - # Resolve every hop independently. Validators are never forwarded across - # origins, where even an opaque ETag would become cross-origin state. + # The next iteration binds validators to the exact destination URL. current = destination continue if response.status_code == 304: cached = self._cache.get(self._source_id(provenance)) - if cached is None: + binding = self._validators.get(provenance) + if ( + cached is None + or not headers + or binding is None + or binding[0] != current + ): raise self._safe_error("conditional_response") self._cache.move_to_end(cached.source.source_id) return cached @@ -273,7 +270,7 @@ class HttpManifestEvidenceSource: media_type=media_type, acquired_at=datetime.now(UTC), ) - self._remember(provenance, document, (etag, last_modified)) + self._remember(provenance, current, document, (etag, last_modified)) return document def discover(self):