Redact the Scraper API's x-debug header before logging it - #37
Merged
Merged
Conversation
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) <noreply@anthropic.com>
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SECURITY.mdin this repo names the Scraper API'sx-debugresponseheader as one of three places credentials reach a log unmasked. 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.
Measured across the family on 2026-09-21: the header was logged raw in 27
repos. This is that fix, applied here.
What it does
_redact_debug_header()masks two shapes, globally — a masker thathandles the first occurrence prints the password the other four times and
looks like it is working:
scheme://user:pass@hostanywhere in the text, not only in a string thatis entirely a URL;
key=/clientKey=/token=/api_key=values.Redaction rather than an allowlist of fields, deliberately:
x-debugis theremote API's own metadata and its shape is not ours to pin, so 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
The fixtures are the SHAPES a credential takes — a Scraping Browser
endpoint, an authenticated proxy, a key as a query parameter — not the
literals this repo happens to contain today, and they assert both halves:
the secret is gone and the cost/host/status worth logging survives.
They are assembled from pieces rather than written out whole, because this
file is scanned by the repo's own credential check and a fixture that LOOKS
like a live key fails it. An allowlist entry would have been a hole a real
credential could later hide in.
Controlled. With the fix in, the suite is green; with the log line
pointed back at the raw value, the suite goes RED and names this check.
That control is what this change was verified by, rather than by "the suite
is still green" — a guard that is defined and never called satisfies that
perfectly, and one repo's guard turned out to be vacuous for exactly that
reason until the control found it.
🤖 Generated with Claude Code