Two inherited defects: --fingerprint always 400'd, and the shipped credential check was dead - #25
Merged
Merged
Conversation
credential check was dead
Found while auditing tokopedia-scraper against the family notes, then
checked here before being patched. Neither is Farfetch-specific.
1. --fingerprint FAILED ON EVERY INVOCATION. The engine's --fp-tags
default was "Windows,Chrome,Desktop", and the fingerprint API rejects
it with HTTP 400 ("Request parameters are invalid") — while
fingerprint_client.py's own --tags help has always said ONE OS-family
tag, not a list, and that Chrome/Desktop/Mobile are each rejected.
Measured against the live API on 2026-09-10:
Windows OK
Windows,Chrome,Desktop 400
Chrome 400
Desktop 400
Present in all four repos in this family; each is fixed on its own
branch.
2. THE SHIPPED CREDENTIAL CHECK WAS DEAD, AND THE LIVE ONE HAD A HOLE.
`.github/ci_checks.py` was in this repo and invoked by NOTHING — not
CI, not the offline suite. `tests.yml` carried an inline grep doing a
narrower version of the same job with its own allowlist, and the two
disagreed in a way that mattered:
* the inline version matched only `ws://` and `wss://`, so an
`http://user:pass@` credential would have sailed past CI;
* the shipped version, which DOES match `http://`, failed on this
repo's own main — the documentation placeholder
`http://login:password@host:port` in proxy_pool.py and four masking
fixtures in smoke_test.py were missing from its allowlist.
A check that fails on its own repository is a check nobody can read; a
check nothing runs is not a check. Now the allowlist covers this repo's
real placeholders, `tests.yml` calls `ci_checks.py --secret-check`, and
the offline suite runs `--all` so a failure shows up before a push.
Both are pinned by checks, including that no second inline grep creeps
back. All smoke tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0193w6TZpMqmbfERNZAGdtv3
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
2 participants
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.
Found by auditing
tokopedia-scraperagainst the family notes, then verified here before patching. Neither is Farfetch-specific.--fingerprintfailed on every invocation. The engine's--fp-tagsdefault wasWindows,Chrome,Desktop, which the fingerprint API rejects with HTTP 400 — whilefingerprint_client.py's own--tagshelp has always said ONE OS-family tag. Measured against the live API:Windowssucceeds;Windows,Chrome,Desktop,ChromeandDesktopeach 400. Present in all four repos in this family.The shipped credential check was dead, and the live one had a hole.
.github/ci_checks.pywas invoked by nothing — not CI, not the suite — whiletests.ymlcarried an inline grep with its own allowlist. The inline one matched onlyws:///wss://, so anhttp://user:pass@credential would have sailed past CI; the shipped one, which does matchhttp://, failed on this repo's own main because five legitimate placeholders were missing from its allowlist. Now: allowlist fixed, CI calls the shipped script, and the suite runs--allso a failure is local.Both pinned by checks, including one that fails if a second inline grep creeps back. All smoke tests pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_0193w6TZpMqmbfERNZAGdtv3