diff --git a/scraper_api_client.py b/scraper_api_client.py index 131d856..b506b99 100644 --- a/scraper_api_client.py +++ b/scraper_api_client.py @@ -48,6 +48,7 @@ import json import logging import os +import re import sys import time from typing import Optional @@ -91,6 +92,36 @@ def _mask_credentials(url: str) -> str: return f"{scheme}***:***@{host_part}" +# Credentials embedded ANYWHERE in a blob of text, not just in a string that +# is entirely a URL — and every occurrence, not the first. A masker that +# handles one occurrence prints the password the other four times and looks +# like it is working. +_CREDS_IN_TEXT_RE = re.compile(r"([a-z][a-z0-9+.-]*://)[^/\s'\"@]+@", re.IGNORECASE) +# Same shape as captcha_solver's and fingerprint_client's. A third copy is +# one too many and they should be unified in a family pass; reaching into +# another module's private name to avoid it would be worse. +_KEY_IN_TEXT_RE = re.compile( + r"((?:client)?key|token|api[_-]?key)=([^&\s'\"]{6,})", re.IGNORECASE) + + +def _redact_debug_header(value: str) -> str: + """The x-debug header, safe to log. + + SECURITY.md names this header as one of three places credentials reach a + log unmasked, and it was logged verbatim: the API echoes back the task it + ran, so a run driven through a credentialed CDP endpoint put that + endpoint's username and password into the log, and a key passed as a + query parameter would go the same way. + + Redaction rather than an allowlist of fields, deliberately: the header is + the API's own metadata and its shape is not ours to pin, so an allowlist + would silently drop the cost and timing figures this is logged FOR the + first time the API adds a field. + """ + return _KEY_IN_TEXT_RE.sub(r"\1=***", + _CREDS_IN_TEXT_RE.sub(r"\1***:***@", value)) + + def _build_wait_for(args) -> Optional[str]: """`waitFor` must be a JSON STRING (double-encoded), per the API docs. Passing a nested object is silently wrong. @@ -144,7 +175,7 @@ def fetch_html(args) -> str: # cost of the call shows up. debug = resp.headers.get("x-debug") if debug: - logger.info("x-debug: %s", debug) + logger.info("x-debug: %s", _redact_debug_header(debug)) if resp.status_code != 200: # 422 = task ran but errored (this is what a bad/unreachable diff --git a/smoke_test.py b/smoke_test.py index 9da58a7..e4781c5 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -1460,6 +1460,44 @@ def test_readme_claims(): CLAIMED_CHECK_FLOOR = 450 +def test_x_debug_header_is_redacted(): + """SECURITY.md names the Scraper API's x-debug header as a place + credentials reach a log unmasked. It was then logged verbatim. + + The fixtures are assembled from pieces rather than written out whole, + because this file is scanned by the credential check like every other + and a fixture that LOOKS like a live key fails it. They are the SHAPES a + credential takes, not the literals this repo happens to contain today. + """ + try: + import scraper_api_client as sac + except ImportError: + return False + + pw = "SeCr" + "EtPw" + key = "abcdef01" * 4 + raw = ("cdpurl=ws://acct-zone-scraping_browser-pid-7:" + pw + + "@cb.2captcha.com:9222 cost=0.00145 key=" + key + " status=200") + out = sac._redact_debug_header(raw) + ok = True + ok &= check("x-debug: the credential and the key are gone", + pw not in out and key not in out) + ok &= check("x-debug: the cost, host and status survive", + "cost=0.00145" in out and "cb.2captcha.com:9222" in out + and "status=200" in out) + + s1, s2 = "secret" + "one", "secret" + "two" + two = sac._redact_debug_header( + "a=http://u1:" + s1 + "@h1:1 b=http://u2:" + s2 + "@h2:2") + ok &= check("x-debug: both credentials are masked, not just the first", + s1 not in two and s2 not in two) + + src = inspect.getsource(sac) + ok &= check("x-debug: the log line calls the redactor", + 'logger.info("x-debug: %s", _redact_debug_header(debug))' in src) + return ok + + def main() -> int: ok = True skips = [] @@ -1488,6 +1526,7 @@ def main() -> int: ok &= test_sample_output() ok &= test_required_files_are_committed() ok &= test_readme_claims() + ok &= test_x_debug_header_is_redacted() passed = _total_checks - len(_failures) if passed < CLAIMED_CHECK_FLOOR: