Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions .github/ci_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
# into pip's vendored code and flags a 32-hex string in `_elffile.py` as
# key-shaped. Correct about the string, wrong about the file — and it is the
# FIRST thing a new user sees from `python3 smoke_test.py`. A guard people
# have to argue with is one they learn to suppress (CLAUDE.md §22).
# have to argue with is one they learn to suppress.
#
# Deliberately NOT a narrowing of what gets scanned: an untracked file is
# still read, because a key pasted into a scratch file beside the scripts is
Expand Down Expand Up @@ -107,8 +107,12 @@ def scanned_files():
for path in sorted(REPO.rglob("*")):
if not path.is_file() or path.suffix not in SCANNED_SUFFIXES:
continue
# Judged on the path INSIDE the repo. `path.parts` of the absolute
# path includes every ancestor, so a clone that happened to live
# under a directory named `venv` (or `.git`) skipped every file and
# reported "nothing credential-shaped" after scanning 0 of them.
if any(part in {".git", "__pycache__", ".venv", "venv"}
for part in path.parts):
for part in path.relative_to(REPO).parts):
continue
if _is_in_virtualenv(path):
continue
Expand Down Expand Up @@ -200,6 +204,10 @@ def secret_check():
failed.append(f"{rel}:{lineno} contains {match[:6]}… — a "
f"32-char hex string, the shape of a 2captcha key")

# A scan that read nothing has proved nothing, and must not print "ok".
if scanned == 0:
failed.append("0 files scanned — the exclusions matched everything, "
"so this check proved nothing")
if not failed:
print(f"ok {scanned} files scanned, nothing credential-shaped")
return failed
Expand Down
9 changes: 6 additions & 3 deletions .github/workflows/canary.yml
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,12 @@ jobs:
# product, three numbers, two hours. A canary pinned to any of them would
# go red for the catalogue behaving normally.
#
# If it does start being refused from runners, gate its steps on
# AMAZON_PROXY the way this file's header describes and let it SKIP with a
# ::notice:: when the secret is absent, rather than leaving it red.
# If it does start being refused from runners, set the AMAZON_PROXY secret
# (see this file's header). Do NOT make it skip green without one: a
# canary that skips, is blocked (exit 3), never reaches the site (exit 5)
# or parses zero rows must end red, or the badge stops testing the site.
# If a proxy becomes required, the job should fail with an ::error::
# naming AMAZON_PROXY when the secret is absent.
variation-canary:
runs-on: ubuntu-latest
steps:
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/weekly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ name: weekly
# needs the workflow on the DEFAULT branch, or it answers a 404 that reads
# like a filename typo), then read what actually happened per job. Add the
# schedule below for the jobs that pass, and for any that are refused from
# runners either gate them on the AMAZON_PROXY secret with a ::notice:: skip
# — the pattern canary.yml's header describes — or drop them and say why.
# runners either require the AMAZON_PROXY secret (failing with an ::error::
# that names it when it is absent, never a green skip) or drop them and say
# why.
#
# schedule:
# - cron: "41 5 * * 1" # Mondays, off the hour
Expand Down
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ secrets*
# recoverable only because that repo was private at the time. On 2026-09-17
# a sibling had an untracked `.env.bak` holding a 32-hex key in a PUBLIC
# repository, one `git add -A` from publication. Craigslist was fixed then
# and the fix never reached anyone else, which is §16's rule unapplied: when
# you fix one, check the siblings.
# and the fix never reached anyone else: when you fix one, check the
# siblings.
.env*
!.env.example
74 changes: 55 additions & 19 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,54 @@ one, and when it does the release notes say so first.

## [Unreleased]

> **Behaviour changes from the 2026-10-14 audit fixes:**
>
> * **A URL Amazon answers with "page not found" (HTTP 404) now exits 5 with
> `stop_reason: not_found`.** It used to be read as the 503 throttle page
> (both carry the same "automated access" comment), retried, rotated and
> reported as **blocked, exit 3**.
> * **CSV cells that begin with `=`, `+`, `-`, `@`, a tab or a line break get a
> leading apostrophe** (CSV only; numbers and the JSON are unchanged). The
> sidecar's `csv_cells_escaped` says how many.
> * **At most one paid captcha solve per page**, across all retries and proxy
> rotations. With a 3-exit pool one page could buy three.
> * **Newly rejected flag values:** a negative `--proxy-block-retries` in all
> three engines; in `scraper_api_client.py` a negative `--retries` or
> `--retry-delay`, and `--timeout` below 1. Each is now exit 2.
> * Output files are now written atomically and keep the permissions plain
> `open()` gave them: new files follow the umask, an existing file keeps its
> mode.

### Fixed (2026-10-14 audit)

- **Output writers truncated in place.** `write_json`, `write_csv`,
`write_run_meta` and `write_last_attempt` opened their target with `"w"`, so
a failure mid-write left a truncated file (a good 703-byte `out.json` became
5 bytes of invalid JSON). They now write a temporary file in the same
directory, fsync it and rename it over the target; a failed write leaves the
previous file byte-identical and no temp file behind.
- **CSV formula injection.** Third-party text (titles, sellers, bullets, review
bodies) reached the CSV as-is. See the blockquote above.
- **No captcha solve budget.** The solve sat inside the per-exit loop with
nothing counting it. `page_flow.SolveBudget` (one per page) is now asked
before every purchase in all three engines.
- **Amazon's 404 read as a throttle.** The Playwright and pyppeteer engines now
pass the navigation's HTTP status to the page classifier, which has a
`not_found` state; Selenium, which has no status to read, recognises
Amazon's own (English) not-found page.
- **The credential scan skipped everything** when the clone lived under a
directory named `venv` or `.git`, because it tested the absolute path. It now
tests the path inside the repo, and fails if it scanned no files at all.
- **A `{...}` placeholder copied from the docs read as configured** in `.env`
(`ws://{login}-zone-…`). Any value still containing one now reads as unset,
with a warning that names the variable, not the value.
- README: the flags the engines do not share are listed, instead of a claim
that all three take the same flags; the offline suite now checks that list
in both directions.
- Removed references to documents outside this repository from comments and
the changelog, and a stale comment about a `--mode variations` that does not
exist, plus the unused `UNIQUE_BY_SKU_MODES` constant.

> **Two changes here affect an existing user before anything else does.**
>
> **A run that never got its page now exits 5, not 4.** Exit 4 has always
Expand Down Expand Up @@ -99,7 +147,7 @@ one, and when it does the release notes say so first.
sidecar and the most recent attempt both be true. Rewritten on success too,
so it is never a stale relic of an old failure.
- **A check that binds every call into a shared module against the callee's
real signature** (§17's check #1), which this repo did not have. It catches
real signature**, which this repo did not have. It catches
two things nothing else here can: a call whose arguments do not fit the
signature, and a call to a name the shared module does not define at all —
both of which reach a live run as a crash on the first fetch while import,
Expand All @@ -126,9 +174,9 @@ one, and when it does the release notes say so first.
engine logs the status and does not classify on it). It now reads
`http_code`, falling back to `status` only if that is an integer. After the fix, one live call (`--wait-text results` on the README's search URL) answered HTTP 200, upstream 200, 16 rows, status complete.

- **farfetch-scraper leftovers removed from the issue templates, TROUBLESHOOTING,
SECURITY and `diff_runs.py`.** The bug-report tips sent a reader to
Farfetch's `/shopping/kids/items.aspx` hub "which carries no product
- **The issue templates, TROUBLESHOOTING, SECURITY and `diff_runs.py`
describe Amazon.** They had been written for another site: the bug-report
tips sent a reader to Farfetch's `/shopping/kids/items.aspx` hub "which carries no product
JSON-LD", the example wrote `--out girls` and expected "96 products"; the
site-change template said the parser "tries JSON-LD first" and asked for a
JSON-LD dump; TROUBLESHOOTING's 0-rows table named Farfetch's
Expand All @@ -153,18 +201,6 @@ one, and when it does the release notes say so first.

### CI

- **A check that binds every call into a shared module against the callee's
real signature** (§17's check #1), which this repo did not have. It catches
two things nothing else here can: a call whose arguments do not fit the
signature, and a call to a name the shared module does not define at all —
both of which reach a live run as a crash on the first fetch while import,
`--help`, `compileall` and the undefined-name walk all stay green. It skips
calls using `*args`/`**kwargs` rather than guessing, treats a locally-bound
name as shadowing a same-named module, and asserts it found calls to bind
at all so it cannot pass by scanning nothing. Verified by control.

### CI

- **A variation canary**, reading one real multi-variant product page and
asserting all four columns are populated, that the count is not 1 on a
product picked for having many, and that there is one selected value per
Expand Down Expand Up @@ -208,9 +244,9 @@ one, and when it does the release notes say so first.

- **`fingerprint_client.py` could not read the key from `.env`.** `--key`
defaulted to `os.environ.get("TWOCAPTCHA_KEY")` and only that, so a key put
in `.env` — exactly as §3, the README and `.env.example` instruct — worked
in `.env` — exactly as the README and `.env.example` instruct — worked
for every engine and failed HERE with "No API key". A documented mechanism
not applied on one path, which is the shape of half the defects §16 lists.
not applied on one path.

It now reads through `env_config.env_value`, calling `load_env()` itself
because this is a standalone entry point that no engine has necessarily run
Expand All @@ -222,7 +258,7 @@ one, and when it does the release notes say so first.
"still set to the placeholder from .env.example" instead.

Found on a sibling repo's first live `--fingerprint` run, then checked
across the family before patching, per §16: five repos had it and one had
across the family before patching: five repos had it and one had
already fixed it. Pinned by a check verified to fail on the old code —
including that the help string does not interpolate its default, which is
one substring away from printing a live credential to anyone who types
Expand Down
26 changes: 24 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,18 @@ See [`sample_output.json`](sample_output.json) and
[`sample_output.csv`](sample_output.csv) — both cut from a real run, not
written by hand. CI checks their columns against the schema.

**In the CSV only, a text cell that begins with `=`, `+`, `-`, `@`, a tab or
a line break gets a leading apostrophe**, so a spreadsheet shows it as text
instead of running it as a formula. Titles, sellers, bullets and review text
are written by third parties. Numbers are never touched and the JSON keeps
the original text; `csv_cells_escaped` in `<out>.meta.json` says how many
cells were changed.

Output files are replaced atomically (written to a temporary file beside the
target, then renamed), so a run killed mid-write leaves the previous file
intact rather than a truncated one. File permissions follow your umask, and
an existing file keeps its mode.

### The run-metadata sidecar

Every successful run writes `<out>.meta.json` beside its output:
Expand Down Expand Up @@ -225,7 +237,7 @@ way, which is why it is what gets read.
| 2 | Bad usage (including a credentialed `--cdp-endpoint` given to Selenium) |
| 3 | Blocked before parsing: Amazon's image captcha, an unresolved throttle, or a sign-in wall |
| 4 | Ran fine, found nothing |
| 5 | The page was never fetched: a navigation timeout, a dead or unauthenticated proxy, or a Scraper API error. **Not** an empty result — nothing can be concluded about the catalogue |
| 5 | The page was never fetched: a navigation timeout, a dead or unauthenticated proxy, a Scraper API error, or a URL Amazon says does not exist (HTTP 404, `stop_reason: not_found`). **Not** an empty result — nothing can be concluded about the catalogue |
| 6 | Partial: some pages gathered, then the run stopped early |

Exit 3 covers the sign-in wall as well as a captcha, because the family's
Expand Down Expand Up @@ -434,7 +446,17 @@ faster way to get that address scored than to gather data).
All three engines share the page-state policy (`page_flow.py`) and the
status/exit-code mapping (`output_writer.finish_run`), so they cannot drift
apart on what a page means or what a run reports. The offline suite asserts
they take the same flags.
that each engine takes the family's core flags; the flag sets are not
identical:

* `--fingerprint`, `--fp-country` and `--fp-tags` exist in Playwright and
Selenium, not in pyppeteer; `--locale` is Playwright-only.
* pyppeteer adds `--chromium-path`; Selenium adds `--no-sandbox` and
`--disable-dev-shm-usage`.
* `--concurrency` is accepted and ignored by Selenium and pyppeteer (above).

Paid captcha solves are capped at **one per page** in every engine, however
many retries or proxy rotations that page takes.

Limits worth knowing before you pick one:

Expand Down
7 changes: 3 additions & 4 deletions captcha_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,8 @@ def detect_recaptcha_v3(html: str, page_url: str) -> Optional[CaptchaChallenge]:
attributes (data-captcha-type="recaptcha" data-version="v3"
data-sitekey="..." data-action="..."), with the actual execute()
call happening inside a bundled JS file, never appearing as
readable inline script text at all — confirmed live on the sibling
farfetch-scraper repo's target,
sign-up modal via DevTools inspection. A previous version of this
readable inline script text at all — confirmed live on another
site's sign-up modal via DevTools inspection. A previous version of this
function, which only checked for format 1, reported "no captcha"
on this exact page despite one being genuinely present — caught by
manually inspecting the DOM, not by the detector itself, which is
Expand Down Expand Up @@ -170,7 +169,7 @@ def detect_recaptcha_v3(html: str, page_url: str) -> Optional[CaptchaChallenge]:

# ---------------------------------------------------------------------------
# Runtime detection (added 2026-08-24 — the static-HTML detector below no
# longer matched the sibling farfetch-scraper repo's target)
# longer matched the site it was first written for)
# ---------------------------------------------------------------------------
#
# What changed there: the sign-up modal used to render
Expand Down
18 changes: 18 additions & 0 deletions env_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import logging
import os
import re
from pathlib import Path

logger = logging.getLogger(__name__)
Expand All @@ -54,6 +55,15 @@
"",
}

# A `{...}` left anywhere in a value is a placeholder too. The two
# credentialled URLs are documented the way the vendor documents them —
# `ws://{login}-zone-...:{password}@cb.2captcha.com:9222` and
# `http://{user}:{password}@...` — and neither literal is in the set above,
# so a value copied from the docs read as CONFIGURED and reached the
# endpoint with `{login}` as its username, coming back as a 401 a long way
# from its cause. Real credentials and URLs never contain braces.
_BRACED_PLACEHOLDER = re.compile(r"\{[^{}\s]*\}")

_loaded_from = None


Expand Down Expand Up @@ -159,6 +169,14 @@ def env_value(name):
"%s is still set to the placeholder from .env.example — treating it "
"as unset. Put your real value in .env.", name)
return None
if _BRACED_PLACEHOLDER.search(stripped):
# Name the variable, never quote the value: it may be a URL with a
# real password beside the placeholder.
logger.warning(
"%s still contains a {...} placeholder from the documentation — "
"treating it as unset. Replace every {...} with your real value.",
name)
return None
return stripped


Expand Down
2 changes: 1 addition & 1 deletion fingerprint_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ def main() -> int:
# loader. It used to read `os.environ` alone, which meant a key put in
# `.env` exactly as the README and .env.example instruct worked for every
# engine and failed HERE with "No API key" — a documented mechanism not
# applied on one path, which is the shape of half the defects §16 lists.
# applied on one path.
#
# `.env` has to be LOADED before it can be read: `env_value` looks at
# os.environ, and `load_env` is what fills that from the file. Calling it
Expand Down
Loading
Loading