From 13a67a355bb5e86d44332af46478570d26ec28a1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 17:23:44 +0000 Subject: [PATCH] Redact the Scraper API's x-debug header before logging it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SECURITY.md names this header as one of three places credentials reach a log unmasked, and the client then logged it 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. A rule stated in one file and broken in another is invisible to both. Both patterns are global: a masker that handles the first occurrence prints the password the other four times and looks like it is working. Redaction rather than an allowlist of fields, because x-debug is the remote API's own metadata and its shape is not ours to pin — an allowlist would silently drop the cost and timing figures the line is logged FOR the first time the API adds a field. The guard uses the SHAPES a credential takes, not the literals this repo contains today, and asserts both halves: the secret is gone and the cost/host/status survive. Its fixtures are assembled from pieces rather than written out whole, because this file is scanned by the repo's own credential check; an allowlist entry would be a hole a real credential could hide in. Controlled by pointing the log line back at the raw value: the suite goes red and names this check. Verified that way rather than by "the suite is still green", which a guard that never executes also satisfies. Co-Authored-By: Claude Opus 5 (1M context) --- scraper_api_client.py | 33 ++++++++++++++++++++++++++++++++- smoke_test.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/scraper_api_client.py b/scraper_api_client.py index fa2a130..7230ebe 100644 --- a/scraper_api_client.py +++ b/scraper_api_client.py @@ -46,6 +46,7 @@ import json import logging import os +import re import sys import time from typing import Optional @@ -89,6 +90,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. @@ -142,7 +173,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 dfda18c..9da057f 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -2879,6 +2879,44 @@ def test_log_format_strings_match_their_args(): return ok +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 # Checks that could not run because an optional engine library is absent. @@ -2918,6 +2956,7 @@ def main() -> int: ok &= test_sample_output() ok &= test_public_names_have_consumers() ok &= test_log_format_strings_match_their_args() + ok &= test_x_debug_header_is_redacted() print() if _failures: