Skip to content

No-proxy crashes, one paid solve per page, scope-aware diffs, stricter credential scan - #13

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 of main at fe2a381.

Problems and resulting behaviour

Finding Before After
Selenium/pyppeteer with no --proxy a refused or empty page (and in pyppeteer a navigation that never loaded) hit pool.current/pool.advance on None: AttributeError, exit 1 re-fetch through the same access path, as Playwright does; exit 3/5 as appropriate
Captcha budget the pre-classification call was unbudgeted: 2 purchases with no pool, 4–5 with a pool of four, limit 1 (stubbed solver) one page_flow.SolveBudget per page, checked immediately before purchase at both call sites, not reset by retry or rotation (budget is a required keyword, so a new call site cannot skip it)
HTTP status goto()'s response discarded; a branded 403/429/503 would read as "unpainted" and be waited on Playwright and pyppeteer pass the status to classify; reset to None after a solve reloads the page. Selenium has no status. No not_found state added
Numeric flags --pages 0, --concurrency 0, negative --delay/--retry-delay/--proxy-block-retries accepted argparse error naming the flag, exit 2, in all three engines; scraper_api_client.py rejects negative --retries/--retry-delay (0 stays legal there: it counts extra attempts)
diff_runs.py scope newyork price band vs losangeles diffed as added/removed, exit 0 refused unless --force when sidecar start_urls differ in host, path or query (order-free, fragment ignored); a warning when a run hit the result ceiling
Credential scan exclusions on absolute path.parts: a clone under venv/ or .venv/ scanned 0 files and passed; .env.bak never read exclusions on the path inside the repo, fails on 0 files scanned, reads .env.* copies (not .env itself) and .json/.csv/.html; venvs are still skipped by pyvenv.cfg
Private citations 4 code files and the CHANGELOG cited a private working file or its sections removed and the rule stated in the repo's own words; a new suite check scans every shipped text file (tokens assembled from pieces)
Docs README said exit 5 = remote API error, --concurrency >1 "refused", a "653 checks" count, nothing about --locale; the Scraper API help and challenge message described another site corrected; canary exit-5 message too
.gitignore live/, results/, foo.page3, root x.html not ignored *.page[0-9]*, /*.html, scratch dirs by kind; .github/, tests/, samples and .env.example still tracked (checked with git check-ignore)

Also fixed: pyppeteer's "grid still had not painted" log used found <= need, so it fired on a page that had painted (log only).

Evidence

  • New suite check test_fetch_loop_without_a_pool_and_within_the_solve_budget runs each engine's real _fetch_one_page in a subprocess with every driver module stubbed. It runs in every venv, the bare one included. Against old main it reproduced the audit: Playwright 2/4/5 solves, and Selenium and pyppeteer AttributeError. On this branch every case raises nothing and buys ≤1 solve.
  • Offline suite: core-only venv exit 0, and one venv per engine (requirements + that engine, no browser download) exit 0, each skipping only the two absent engines. ci_checks.py --all green (36 files scanned).
  • Planted-fault controls (run with PYTHONDONTWRITEBYTECODE=1, __pycache__ cleared). Each one turned the suite red on the check it was meant to trip, and the restored tree is green:
    • Selenium no-pool guard
    • pyppeteer load-failure guard
    • solve budget
    • status to the classifier
    • scan on relative parts
    • scan fails on 0 files
    • scan reads .env copies
    • private citation in README
    • diff scope refusal
    • numeric flag validator
  • No requests to craigslist.org, no paid calls, no keys, no proxies.

Canary (owner request: a canary must actually test the site)

  • Before: ungated daily run, 3-batch walk plus a 1-batch served-list run. Exits 3, 4, 5 and 6 of the walking run were already red. But a non-zero exit of the served-list run was a ::warning:: with exit 0, so the served list and its JSON-LD alignment could go untested on a green badge. The walking run's 90% price-share floor went red on 4 of 32 scheduled runs (79–90%), and the artefacts show free and "make offer" adverts behind it.
  • After:
    • a non-zero served-run exit is ::error::;
    • every row must carry sku, url and title;
    • the price-share floor is 60%, which a broken price read still falls under (checked against the 10-09 artefact: 79% passes, and the same artefact with prices nulled and one title removed fails on both);
    • the served-batch row floor is 150, not 250. The first dispatch on this branch read 243 rows, every one parsed, and failed the 250 floor: a time-of-day total, not a parser check.
  • Secrets: none required. The README and every scheduled run show the site serving a bare GitHub runner, and CRAIGSLIST_PROXY stays optional.
  • Dispatch on this branch:

Not changed, and why

  • Scraper API client sidecar / --allow-empty (L3), sidecar total_results/source (L4): owner decision.
  • Refusing rather than warning for --proxy/--fingerprint with --cdp-endpoint and for --concurrency > 1 (L6): owner decision. The docs now describe the warning.
  • Version, tag and release (L1): out of scope. Changes are under [Unreleased], with behaviour changes in leading blockquotes.
  • Shared fetch loop (T10): a refactor that needs live runs of all three engines.
  • CRAIGSLIST_URL in .env.example: kept. This repo has no URL-builder flags for it to hijack, and the audit's round-trip found it fine.
  • Release notes v0.1.0–v0.1.2 contain no private citations, so none were edited. Repo metadata: no finding for this repo.
  • Historical commit subjects: not rewritten.

🤖 Generated with Claude Code

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

- Selenium and pyppeteer read the proxy pool on a retry with no pool
  configured and exited 1 on a refused, empty or unloadable page; they now
  re-fetch through the same access path, as Playwright does.
- Both captcha call sites per attempt go through one per-page allowance
  checked before any purchase (was 2 solves with no pool, up to 5 with a
  pool of four, against a limit of 1).
- Playwright and pyppeteer pass the navigation's HTTP status to the
  classifier.
- --pages/--concurrency 0 and negative --delay/--retry-delay/
  --proxy-block-retries are usage errors; scraper_api_client rejects a
  negative --retries/--retry-delay.
- diff_runs refuses runs of different listings (area, category, filters)
  unless --force, and warns when a run hit the result ceiling.
- The credential scan matches exclusions inside the repo, fails on 0 files
  scanned, and reads .env copies and JSON/CSV/HTML.
- README, Scraper API help and canary messages corrected; .gitignore covers
  page dumps and scratch directories by kind; private citations removed
  with a suite guard.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… parser

The one-batch run's non-zero exit was a warning on a green job; it is now
an error. Core identity columns must be on every row, and the price-share
floor moves from 90% (red on 4 of 32 scheduled runs from unpriced adverts)
to 60%, which a broken price read still falls under.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A dispatch on this branch read 243 rows from one served response, every
one parsed, and failed a floor of 250.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jehrr
jehrr marked this pull request as ready for review October 15, 2026 13:00
@jehrr
jehrr merged commit 9cb875e into main Oct 15, 2026
9 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