Skip to content

One paid solve per page, atomic outputs, and matching exit codes in every engine - #42

Merged
jehrr merged 3 commits into
mainfrom
fix/audit-2026-10-14
Oct 15, 2026
Merged

jehrr merged 3 commits into
mainfrom
fix/audit-2026-10-14

Conversation

@jehrr

@jehrr jehrr commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes from the 2026-10-14 audit, base 97b1d96.

Problems and resulting behaviour

Finding Before After
H1 captcha budget Playwright's handler ran once per block-retry with nothing counting. A 3-exit pool with --solve-captcha always bought 3 solves for one page. A per-page SolveBudget (in captcha_solver.py) is checked before each purchase in all three engines. It is created outside the rotation loop and is not reset. Stub probe: 1 solve.
H2 CDP connect A connect_over_cdp failure escaped as a traceback (exit 1). Its text repeats the endpoint, user:pass included. pyppeteer connect() had no timeout. The failure is caught and every occurrence redacted. A 401 is explained as wrong or expired credentials. profile_locked/500 is retried 3× at 3 s. The run exits 5. pyppeteer connect is bounded at 30 s.
M1 atomic writes open(path,"w"): a failed json.dump turned a good file into a truncated one. JSON, CSV and the sidecar go through _atomic: a temp file in the target's dir, fsync, then os.replace, with cleanup on error. A new file gets 0o666 & ~umask and an existing file keeps its mode. Checked under 022/002/077 and for a 0640 file.
M2 CSV formulas Raw. A string cell starting with = + - @ \t \r \n gets a leading ' (CSV only; numbers untouched; JSON untouched). The sidecar records csv_cells_escaped, merged under existing keys.
M3/M8 mirror exit codes pyppeteer dropped the goto status, so an HTTP 403 with an unknown body exited 4. Neither mirror reported parser drift. README and docstring said drift exits 4. pyppeteer keeps the status. Selenium reads it from PerformanceNavigationTiming.responseStatus. Both mirrors report http_error and parse_drift as Playwright does. Stub probe: pyppeteer 403 now exits 5. README table and docstring corrected.
M4 skip report main() rebound _skips/_skipped_engines locally, so SKIPPED_ENGINES: was always empty and the engine-smoke gate could not fail. The local rebinding is removed. A new check asserts that every absent driver is listed.
M5 credential scan Exclusions were tested on the absolute path, so a clone under venv/ scanned 0 files and reported "clean". JSON and .env.* were never read. The scan uses relative_to(REPO).parts, fails on 0 files, and also reads .json .csv .html .toml .cfg and .env.* backups. The pyvenv.cfg skip is kept.
Currency default (found while fixing) The DOM fallback wrote currency or "USD". It writes the currency the tile states, or null. A suite check forces an unattributed price and asserts None; the planted or "USD" turns it red. Current output is unchanged, because every recognised price shape carries a symbol or ISO code.
L1 .gitignore X.page3, live/, captures/ and a root dump.html were not ignored. All four are ignored. .env.example, the samples, tests/ and .github/ are still tracked (checked with git check-ignore).
L3 env placeholders {login}/{password} values were applied as real credentials, and empty values logged a placeholder warning. A {…} slot reads as unset, and the warning names the slot, not the value. An empty value is unset with no warning.
L2 private citations 28 lines across code, workflows, pyproject and released CHANGELOG sections pointed at private notes. They are restated in the repo's own words. A suite check (tokens assembled from pieces) keeps them out. The v0.4.3 release notes were edited to drop the one citation; the rest is byte-identical.

The CHANGELOG [Unreleased] section opens with a blockquote listing the behaviour changes: mirror exit codes, CSV escaping, and CDP-failure exit 5.

Evidence

  • Offline suite passes in a core-only venv and in one venv per engine (playwright, pyppeteer, selenium; no browser downloads). Each run reported only the other two engines in SKIPPED_ENGINES, and --help works in each. ruff check . is clean.
  • The new checks drive Playwright's _fetch_one_page and pyppeteer's scrape() through stub drivers loaded from disk, so they run in the core-only CI job too.
  • Controls: 12 faults were planted one at a time, run with PYTHONDONTWRITEBYTECODE=1 and __pycache__ cleared. Each turned the suite red on the expected named check, with no crash, and the restored tree was green. The faults:
    • budget check removed;
    • write_json back to open("w");
    • flat 0o644 mode;
    • CSV escape disabled;
    • local _skipped_engines re-added;
    • absolute-path exclusion;
    • empty-scan guard removed;
    • .env.* read removed;
    • a private citation added;
    • braced-placeholder check removed;
    • redaction made identity;
    • pyppeteer status branch removed.

Canary (owner request, 2026-10-15)

Before:

  • Annotations on the last 10 canary runs: every reachability job was refused from the runner (exit 3, reported as a notice).
  • Every production-like job skipped with a notice, because FARFETCH_PROXY is not set. The repo's only secret is CLAUDE_CODE_OAUTH_TOKEN.
  • So all 10 runs were green and none of them reached the site.

After:

  • production-like fails with ::error:: naming FARFETCH_PROXY when the secret is absent.
  • reachability is renamed and documented as "datacentre refusal expected". It is the only job allowed --allow-block, and a suite check pins that.
  • The thresholds are unchanged: ≥120 rows over 3 pages, ≥0.9 priced, ≥0.5 DOM-confirmed, a complete status, and a named fail on parse_drift. These test the parser, not merchandising.

Dispatched on this branch: run 38694719863.

  • production-like: failure, "The FARFETCH_PROXY secret is not set…". This is the expected result until the owner adds the secret.
  • reachability: success, with the notice "Blocked … (exit 3)".

Secret required: FARFETCH_PROXY, a residential proxy URL. It is read from the environment and never put on a command line.

Not changed

  • The Scraper API --retries 0 message (L4), and making --cdp-endpoint combined with --concurrency/--proxy/--fingerprint exit 2 instead of warning (L5). Both are owner decisions.
  • Version, tag and release reconciliation for 0.5.1 (M7): owner decision; there is no version bump here.
  • Adding the FARFETCH_PROXY secret itself (M6): the owner needs to set it.
  • The shared fetch loop (T10) and schema/sinks (T11).
  • pyppeteer page.authenticate and client hints (L6), the challenge-platform marker count (L7) and detail-mode retries/captcha (L8). Each needs a live run.
  • The (§12) commit subject on main: history is not rewritten.
  • Dependabot ci: bump docker/build-push-action from 6 to 7 #41 is untouched.

🤖 Generated with Claude Code

jehrr and others added 3 commits October 15, 2026 06:44
…very engine

- Captcha: a per-page SolveBudget checked before every purchase in all three
  engines, not reset by retries or exit rotation (a 3-exit pool bought 3 solves
  for one page before).
- Output: JSON, CSV and the sidecar are written atomically with umask-honouring
  modes; CSV cells starting with = + - @ tab CR LF get an apostrophe, counted
  as csv_cells_escaped in the sidecar. JSON is unchanged.
- CDP: a refused --cdp-endpoint connect exits 5 with a credential-free message;
  transient 500/profile_locked retried; pyppeteer connect bounded.
- pyppeteer keeps the goto status, Selenium reads it from the browser; both
  report HTTP errors and parser drift as Playwright does (exit 5, not 4).
- Suite: the skip report reads the module-level record again; new checks for
  all of the above, driven through stubbed drivers.
- Credential scan: relative-path exclusions, fails on 0 files, reads data
  files and .env.* backups. .gitignore covers dump/capture shapes.
- env: {...} placeholders read as unset; an empty value is not a warning.
- Docs: README exit table and stop_reason_for docstring corrected; private
  citations removed and guarded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The no-proxy job is refused from GitHub's datacentre addresses (exit 3 on
every one of the last 10 runs) and the proxy job skipped green without its
secret, so the badge stayed green while nothing reached farfetch.com. The
proxy job now fails with an ::error:: naming the missing secret; the no-proxy
job is documented as "datacentre refusal expected" and is the only one that
may excuse a block. The suite check that pinned the old skip is rewritten.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fallback parser wrote `currency or "USD"`, presenting a guess as a fact.
It now writes only what the tile states (ISO code, prefixed or bare symbol)
and null otherwise. A suite check forces an unattributed price and asserts
None; planting the old default turns it red.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jehrr
jehrr marked this pull request as ready for review October 15, 2026 21:14
@jehrr
jehrr merged commit c59da26 into main Oct 15, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant