diff --git a/docs/SSH_RUN.md b/docs/SSH_RUN.md index 22a78f9..d977800 100644 --- a/docs/SSH_RUN.md +++ b/docs/SSH_RUN.md @@ -11,9 +11,68 @@ confirm successful import, zero rejected records, and partial coverage caused solely by configured page/detail limits. Other nonzero exits fail the job. Missing or ambiguous evidence never suppresses failure. Green does not guarantee complete source coverage: see [pipeline outcomes](DEPLOYMENT.md#outcomes-and-recovery). Scraper output stays in -private host logs; CI gets only request/revision/exit summaries. Migrations are +private host logs; CI gets request/revision/exit and aggregate diagnostic summaries. Migrations are manual, DB credentials stay on the homelab, and there are no automatic retries. +Actions also prints a privacy-safe outcome summary: collection/import statuses, +accepted/rejected/field-issue counts, completed/failed/unattempted detail counts, +and source HTTP error codes. Failed outcomes get an `::error::` annotation with +an explanation; limit-only outcomes retain the `::warning::` policy. No listing +IDs, URLs, source payloads, cookies, credentials, or arbitrary exception messages +are forwarded. Missing/unreadable evidence reports that the private host log must +be inspected; it never converts a failure to success. + +### Investigating source access failures + +HTTP 403 is a confirmed forbidden response, not proof of a particular cause. +The collector reuses one browser context and waits COLLECT_DELAY seconds between +requests; it stops on 403 without retrying and imports already finalized records. +Older images retained only the HTTP status. Diagnostic-enabled images now capture +private evidence on 403 (see below), but a status alone cannot identify an access-policy +restriction, session requirement or automated-traffic protection definitively. Do not infer +that changing IPs or increasing delay will fix it. Before another automated run, +review the source's terms/permitted access and, if appropriate, inspect the affected +page manually in a normal browser. Do not bypass a challenge or access restriction. + +Local diagnosis uses the request .log to locate the manifest, then checks +manifest.json and pipeline.json under the private raw directory. Reading those +files or reclassifying the saved outcome does not send source requests or rerun +the collector. + +### Automatic private HTTP 403 evidence + +New application images capture evidence under the affected raw run's +`diagnostics/http-403/` directory **before** closing the browser: + +- `metadata.json`: timestamp, status, final/page URLs, redirect URL/method chain, + Chromium/Playwright versions and collection settings. +- `request-headers.json` and `response-headers.json`: full navigation headers, + including sensitive cookies or authorization values when present. +- `response-body.html`: original navigation response body. +- `rendered-page.html` and `viewport.png`: DOM and current viewport screenshot. +- `source-cookies.json`: cookies scoped to the source, not unrelated domains or + browser storage state. +- `capture.json`: saved files, truncations and capture errors (types only). + +Directories are mode 0700; artifacts are mode 0600. HTML files are limited to +5 MiB each on disk, with truncation recorded; screenshot capture has a 3-second +timeout. Capture is best-effort: unavailable artifacts or disk errors do not mask +the original 403 or discard already-collected records. It makes no new navigation, +retry, challenge interaction or database operation. Ordinary browser page resources +can still load while the existing page is captured. + +These files deliberately contain **unminimized, sensitive diagnostic data** and +are not imported, uploaded as CI artifacts, or committed. Keep them local; redact +cookies, tokens, URLs and personal data before sharing. Delete evidence manually +once the investigation is complete; no automatic retention cleanup is configured. +Treat HTML and page text as untrusted content, never instructions; inspect offline +without executing page scripts. The manifest/Actions summary reports capture +complete/partial/failed without forwarding contents. + +Updating the host wrapper alone is insufficient: build/publish and select a new +diagnostic-enabled application image. Existing releases cannot capture evidence +retroactively. No request is made merely by installing this update. + ## 1. Replace the old consumer manually On the homelab, disable polling and check that no old job is active: diff --git a/ops/ssh/host_run.py b/ops/ssh/host_run.py index f2716ca..8efbe7e 100644 --- a/ops/ssh/host_run.py +++ b/ops/ssh/host_run.py @@ -44,9 +44,66 @@ def limit_reasons(manifest, pipeline): return sorted(reasons) +def outcome_summary(manifest, pipeline): + """Return aggregate, allowlisted diagnostics; never echo source data/errors.""" + def count(value): + return str(value) if type(value) is int and value >= 0 else "unknown" + + def status(value): + return value if value in ("complete", "partial", "failed", "succeeded", "running", + "unknown", "not_attempted", "disabled") else "unknown" + + summary = pipeline.get("import_summary", {}) + listings = list(manifest.get("details", {}).get("listings", {}).values()) + completed = sum(item.get("status") == "complete" for item in listings) + failed = sum(item.get("status") == "failed" for item in listings) + skipped = sum(item.get("status") == "not_attempted" for item in listings) + stopped = sum(item.get("reason") == "run_stopped" for item in listings) + lines = [ + f"Collection: {status(manifest.get('status'))}. Import: {status(pipeline.get('import_status'))}.", + f"Import records: accepted={count(summary.get('accepted'))}, rejected={count(summary.get('rejected'))}, " + f"field issues={count(summary.get('field_issues'))}.", + f"Details: completed={completed}, failed={failed}, not attempted={skipped} " + f"(collection stopped early={stopped}).", + ] + items = [*manifest.get("searches", []), *listings] + http = sorted({item["http_status"] for item in items + if type(item.get("http_status")) is int and 400 <= item["http_status"] <= 599}) + if http: + lines.append("Source HTTP errors: " + ", ".join(str(code) for code in http) + ".") + explanations = { + "SourceAccessError": "The source refused or failed a request; collection stopped without retries.", + "TimeoutError": "A browser operation timed out; collection stopped.", + "ValueError": "Source payload validation failed; inspect private host diagnostics.", + "KeyboardInterrupt": "Collection was interrupted; inspect host state before another run.", + } + for error in sorted({value for value in (manifest.get("error_type"), pipeline.get("error_type")) + if isinstance(value, str) and value in explanations}): + lines.append(explanations[error]) + if 403 in http: + lines.append("HTTP 403 means access was forbidden. The saved status alone cannot distinguish " + "access policy, session restrictions, or automated-traffic protection. Do not automatically retry.") + diagnostic = manifest.get("diagnostics", {}) + captured = diagnostic.get("capture_status") + if captured in ("complete", "partial"): + lines.append(f"Private HTTP 403 diagnostics saved ({captured}) under this raw run's diagnostics/http-403 directory. " + "Headers, cookies, HTML and screenshots are sensitive; do not upload them to CI.") + elif captured == "failed": + lines.append("Private HTTP 403 diagnostic capture failed; finalized records and the original failure are retained.") + else: + lines.append("This image did not save HTTP 403 diagnostic evidence; deploy a diagnostic-enabled release for future runs.") + if pipeline.get("import_status") in ("unknown", "running"): + lines.append("Import completion is uncertain; inspect database state before replaying the finalized manifest.") + return lines + + def actions_outcome(code, request, state, raw_root): - if code != 1: + def unavailable(): + if code: + print("::error::Run failed but a verified manifest/sidecar summary is unavailable. " + "Inspect the request's private host log/state; do not automatically retry.", flush=True) return code + try: paths = [] with (state / f"{request['request_id']}.log").open() as log: @@ -55,15 +112,24 @@ def actions_outcome(code, request, state, raw_root): paths.append(line.removeprefix("Manifest: ").strip()) if len(paths) != 1 or not re.fullmatch( r"/data/[0-9]{4}-[0-9]{2}-[0-9]{2}/[0-9a-f-]{36}/manifest\.json", paths[0]): - return code + return unavailable() raw_root = raw_root.resolve() path = (raw_root / paths[0].removeprefix("/data/")).resolve() if not path.is_relative_to(raw_root): - return code + return unavailable() manifest = json.loads(path.read_text()) pipeline = json.loads(path.with_name("pipeline.json").read_text()) + if manifest.get("run_id") != pipeline.get("import_summary", {}).get("run_id"): + # An unsuccessful import may not have an import_summary yet. + if pipeline.get("import_status") == "succeeded": + return unavailable() + lines = outcome_summary(manifest, pipeline) + for line in lines: + print(line, flush=True) reasons = limit_reasons(manifest, pipeline) - if not reasons: + if code != 1 or not reasons: + if code: + print("::error::" + " ".join(lines) + " See private host logs/manifest; no automatic retry.", flush=True) return code explanations = {"page_limit": "the configured page limit left additional search pages unfetched", "detail_limit": "the configured detail limit left some discovered ads without detail attempts"} @@ -73,7 +139,7 @@ def actions_outcome(code, request, state, raw_root): return 0 except (OSError, ValueError, KeyError, TypeError, AttributeError): # Missing/malformed evidence must never turn a failure green. - return code + return unavailable() def collection_command(settings): diff --git a/src/car_tracker/collect.py b/src/car_tracker/collect.py index 3f697a9..790ac5a 100644 --- a/src/car_tracker/collect.py +++ b/src/car_tracker/collect.py @@ -27,9 +27,10 @@ EXCLUDED_ATTRIBUTE_PREFIXES = ("owner_", "seller_", "profile_", "rating_", "cont class SourceAccessError(RuntimeError): - def __init__(self, status): + def __init__(self, status, capture=None): super().__init__("Source access failed; stopping without retries") self.status = status + self.capture = capture def now(): @@ -220,6 +221,17 @@ def collect(fetch, searches, output, max_pages=10, delay=5, search_only=False, m write_json(path, manifest) except (Exception, KeyboardInterrupt) as exc: manifest["error_type"] = type(exc).__name__ + if isinstance(exc, SourceAccessError) and exc.status == 403 and exc.capture is not None: + try: + diagnostic = directory / "diagnostics/http-403" + report = exc.capture(diagnostic) + manifest["diagnostics"] = { + "directory": "diagnostics/http-403", "http_status": 403, + "capture_status": "partial" if report.get("errors") or report.get("truncated") else "complete", + } + except Exception: + # Preserve the access error and finalized records if capture fails. + manifest["diagnostics"] = {"http_status": 403, "capture_status": "failed"} if target is not None: if target["status"] == "pending": target["status"] = "not_attempted" @@ -303,7 +315,16 @@ def collect_browser(args): def fetch(url): response = page.goto(url, wait_until="domcontentloaded", timeout=60_000) if response is None or response.status != 200: - raise SourceAccessError(response.status if response else None) + capture = None + if response is not None and response.status == 403: + from .diagnostics import capture_403 + + def capture(directory): + return capture_403(directory, response, page, browser, { + "headed": args.headed, "locale": "fr-FR", "delay_seconds": args.delay, + "max_pages": args.max_pages, "max_details": args.max_details, + }) + raise SourceAccessError(response.status if response else None, capture=capture) return page.locator("script#__NEXT_DATA__").text_content(timeout=15_000) result = collect( diff --git a/src/car_tracker/diagnostics.py b/src/car_tracker/diagnostics.py new file mode 100644 index 0000000..b0468d9 --- /dev/null +++ b/src/car_tracker/diagnostics.py @@ -0,0 +1,59 @@ +"""Private, best-effort evidence capture for a source HTTP 403; never retry.""" + +import json +import os +from importlib.metadata import version + +from .collect import now + + +MAX_DOCUMENT_BYTES = 5 * 1024 * 1024 + + +def capture_403(directory, response, page, browser, settings): + directory.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + directory.mkdir(mode=0o700, exist_ok=False) + report = {"schema_version": 1, "captured": [], "errors": {}, "truncated": []} + + def save(name, produce, document=False): + try: + value = produce() + if isinstance(value, str): + value = value.encode("utf-8") + elif not isinstance(value, bytes): + value = (json.dumps(value, indent=2) + "\n").encode("utf-8") + if document and len(value) > MAX_DOCUMENT_BYTES: + value = value[:MAX_DOCUMENT_BYTES] + report["truncated"].append(name) + descriptor = os.open(directory / name, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + with os.fdopen(descriptor, "wb") as output: + output.write(value) + report["captured"].append(name) + except Exception as exc: + # No exception messages: they can contain URLs, tokens or source data. + report["errors"][name] = type(exc).__name__ + + def redirects(): + chain = [] + request = response.request + while request is not None and len(chain) < 20: + chain.append({"url": request.url, "method": request.method}) + request = request.redirected_from + return list(reversed(chain)) + + save("metadata.json", lambda: { + "schema_version": 1, "captured_at": now(), "http_status": response.status, + "final_url": response.url, "page_url": page.url, + "browser_version": browser.version, "playwright_version": version("playwright"), + "settings": settings, "redirects": redirects(), + "privacy": "sensitive-host-only-no-ci-artifacts", + }) + save("request-headers.json", response.request.all_headers) + save("response-headers.json", response.all_headers) + save("response-body.html", response.body, document=True) + save("rendered-page.html", page.content, document=True) + # Cookies scoped to the source only; no storage state or unrelated domains. + save("source-cookies.json", lambda: page.context.cookies(["https://www.leboncoin.fr"])) + save("viewport.png", lambda: page.screenshot(timeout=3000, animations="allow")) + save("capture.json", lambda: report) + return report diff --git a/tests/test_collect.py b/tests/test_collect.py index 3bdcd7f..af6c2f2 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -124,6 +124,32 @@ class CollectorTests(unittest.TestCase): self.assertEqual(manifest["status"], "failed") self.assertEqual(manifest["searches"][0]["http_status"], 403) + def test_403_capture_is_linked_without_exposing_private_contents(self): + from unittest.mock import Mock + + capture = Mock(return_value={"captured": ["response-body.html"], "errors": {}}) + path, manifest = self.run_collection([payload(pages=2), SourceAccessError(403, capture=capture)]) + capture.assert_called_once_with(path.parent / "diagnostics/http-403") + self.assertEqual(manifest["diagnostics"]["capture_status"], "complete") + self.assertEqual(manifest["error_type"], "SourceAccessError") + self.assertEqual(len(list(path.parent.glob("*.jsonl.gz"))), 1) + + def test_capture_failure_preserves_original_error_and_records(self): + from unittest.mock import Mock + + capture = Mock(side_effect=OSError("private filesystem details")) + path, manifest = self.run_collection([payload(pages=2), SourceAccessError(403, capture=capture)]) + self.assertEqual(manifest["diagnostics"]["capture_status"], "failed") + self.assertEqual(manifest["searches"][0]["http_status"], 403) + self.assertNotIn("private filesystem", path.read_text()) + + def test_capture_does_not_run_for_other_statuses(self): + from unittest.mock import Mock + + capture = Mock() + self.run_collection([SourceAccessError(429, capture=capture)]) + capture.assert_not_called() + def test_discovery_failure_does_not_claim_detail_completion(self): _, manifest = self.run_collection([SourceAccessError(403)], search_only=False) self.assertEqual(manifest["details"]["status"], "not_attempted") diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py new file mode 100644 index 0000000..19b1f05 --- /dev/null +++ b/tests/test_diagnostics.py @@ -0,0 +1,86 @@ +import json +from contextlib import nullcontext +from pathlib import Path +import tempfile +from types import SimpleNamespace +import unittest +from unittest.mock import Mock, patch + +from car_tracker.diagnostics import capture_403 +from car_tracker.collect import collect_browser + + +class DiagnosticTests(unittest.TestCase): + def setUp(self): + self.root = Path(self.enterContext(tempfile.TemporaryDirectory())) / "diagnostics/http-403" + request = SimpleNamespace(url="https://www.leboncoin.fr/ad/voitures/123", method="GET", + redirected_from=None, all_headers=lambda: {"cookie": "private-cookie"}) + self.response = SimpleNamespace(status=403, url=request.url, request=request, + all_headers=lambda: {"set-cookie": "private-response-cookie"}, + body=lambda: b"Forbidden synthetic response") + self.page = SimpleNamespace(url=request.url, content=lambda: "synthetic DOM", + context=SimpleNamespace(cookies=Mock(return_value=[{"name": "datadome", "value": "secret"}])), + screenshot=Mock(return_value=b"synthetic-png")) + self.browser = SimpleNamespace(version="synthetic-browser") + self.enterContext(patch("car_tracker.diagnostics.version", return_value="synthetic-playwright")) + + def capture(self): + return capture_403(self.root, self.response, self.page, self.browser, {"delay_seconds": 5}) + + def test_capture_is_private_and_does_not_navigate(self): + report = self.capture() + self.assertFalse(report["errors"]) + self.assertEqual(self.root.stat().st_mode & 0o777, 0o700) + self.assertEqual(self.root.parent.stat().st_mode & 0o777, 0o700) + for path in self.root.iterdir(): + self.assertEqual(path.stat().st_mode & 0o777, 0o600) + self.assertEqual(json.loads((self.root / "request-headers.json").read_text())["cookie"], "private-cookie") + self.assertEqual((self.root / "response-body.html").read_bytes(), self.response.body()) + self.page.context.cookies.assert_called_once_with(["https://www.leboncoin.fr"]) + self.page.screenshot.assert_called_once_with(timeout=3000, animations="allow") + self.assertNotIn("secret", json.dumps(report)) + + def test_one_capture_failure_does_not_prevent_other_artifacts(self): + self.response.body = Mock(side_effect=RuntimeError("private body with token")) + report = self.capture() + self.assertEqual(report["errors"], {"response-body.html": "RuntimeError"}) + self.assertTrue((self.root / "viewport.png").exists()) + self.assertNotIn("token", (self.root / "capture.json").read_text()) + + def test_documents_are_disk_bounded_and_truncation_is_recorded(self): + with patch("car_tracker.diagnostics.MAX_DOCUMENT_BYTES", 10): + report = self.capture() + self.assertEqual((self.root / "response-body.html").stat().st_size, 10) + self.assertIn("response-body.html", report["truncated"]) + self.assertIn("rendered-page.html", report["truncated"]) + + def test_existing_capture_is_not_overwritten(self): + self.capture() + with self.assertRaises(FileExistsError): + self.capture() + + def test_browser_403_captures_before_close_and_never_retries(self): + self.page.goto = Mock(return_value=self.response) + context = self.page.context + context.new_page = Mock(return_value=self.page) + self.browser.new_context = Mock(return_value=context) + self.browser.close = Mock() + + def body_before_close(): + self.browser.close.assert_not_called() + return b"synthetic forbidden response" + + self.response.body = body_before_close + playwright = SimpleNamespace(chromium=SimpleNamespace(launch=Mock(return_value=self.browser))) + args = SimpleNamespace(headed=True, delay=5, max_pages=1, max_details=None, + search_only=False, output=self.root.parent.parent, + searches=[{"family": "synthetic", "text": "synthetic"}]) + module = SimpleNamespace(sync_playwright=lambda: nullcontext(playwright)) + with patch.dict("sys.modules", {"playwright.sync_api": module}): + path, status = collect_browser(args) + manifest = json.loads(path.read_text()) + self.assertEqual(status, "failed") + self.assertEqual(manifest["diagnostics"]["capture_status"], "complete") + self.assertTrue((path.parent / "diagnostics/http-403/response-body.html").exists()) + self.page.goto.assert_called_once() + self.browser.close.assert_called_once() diff --git a/tests/test_ssh_run.py b/tests/test_ssh_run.py index c9c9595..2a4d875 100644 --- a/tests/test_ssh_run.py +++ b/tests/test_ssh_run.py @@ -27,6 +27,8 @@ with patch.dict("sys.modules", {"reconcile": agent}): class SSHRunTests(unittest.TestCase): def setUp(self): + # Diagnostic annotation tests must not emit real CI workflow commands. + self.enterContext(patch("sys.stdout", new_callable=io.StringIO)) self.root = Path(self.enterContext(tempfile.TemporaryDirectory())) self.request = {"schema_version": 1, "request_id": str(uuid.uuid4()), "revision": "a" * 40, "image": agent.IMAGE_REPOSITORY + "@sha256:" + "b" * 64} @@ -155,3 +157,56 @@ class SSHRunTests(unittest.TestCase): for value in ("", "Manifest: /data/../../secret\n", "Manifest: /data/unknown/manifest.json\n"): log.write_text(value) self.assertEqual(host.actions_outcome(1, self.request, self.root, self.root), 1) + + def test_source_403_is_explained_and_remains_failure(self): + raw = self.root / "raw" + path = raw / "2026-10-01" / ("a" * 36) + path.mkdir(parents=True) + manifest, pipeline = self.bounded_outcome() + manifest["error_type"] = "SourceAccessError" + manifest["details"] = {"status": "partial", "listings": { + "private-ad-id": {"status": "complete", "url": "https://private.example/ad"}, + "other-private-id": {"status": "failed", "reason": "SourceAccessError", "http_status": 403}, + "third-private-id": {"status": "not_attempted", "reason": "run_stopped"}, + }} + pipeline["import_summary"].update(accepted=2, field_issues=4) + (path / "manifest.json").write_text(json.dumps(manifest)) + (path / "pipeline.json").write_text(json.dumps(pipeline)) + (self.root / f"{self.request['request_id']}.log").write_text( + f"Manifest: /data/2026-10-01/{'a' * 36}/manifest.json\n") + with patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertEqual(host.actions_outcome(1, self.request, self.root, raw), 1) + message = output.getvalue() + self.assertIn("::error::", message) + self.assertIn("HTTP 403", message) + self.assertIn("Import: succeeded", message) + self.assertIn("completed=1, failed=1, not attempted=1", message) + self.assertNotIn("private-ad-id", message) + self.assertNotIn("private.example", message) + self.assertNotIn("::warning::", message) + + def test_diagnostics_do_not_echo_unknown_errors_or_workflow_commands(self): + manifest, pipeline = self.bounded_outcome() + manifest["error_type"] = "secret\n::warning::injected" + manifest["status"] = "secret\n::error::injected" + pipeline["import_summary"].update(accepted="secret", field_issues="secret") + lines = " ".join(host.outcome_summary(manifest, pipeline)) + self.assertNotIn("secret", lines) + self.assertNotIn("::", lines) + + def test_private_capture_is_announced_without_contents(self): + manifest, pipeline = self.bounded_outcome() + manifest["details"]["listings"] = {"1": {"status": "failed", "http_status": 403}} + manifest["diagnostics"] = {"capture_status": "partial", "directory": "private-value"} + lines = " ".join(host.outcome_summary(manifest, pipeline)) + self.assertIn("diagnostics saved (partial)", lines) + self.assertNotIn("private-value", lines) + + def test_failed_import_summary_and_missing_manifest_do_not_hide_failure(self): + manifest, pipeline = self.bounded_outcome() + pipeline.update(import_status="unknown", error_type="OperationalError") + lines = " ".join(host.outcome_summary(manifest, pipeline)) + self.assertIn("Import completion is uncertain", lines) + with patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertEqual(host.actions_outcome(3, self.request, self.root, self.root), 3) + self.assertIn("summary is unavailable", output.getvalue())