Skip to content

One paid solve per page, parser failures reported, atomic CSV-safe output - #3

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

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

Conversation

@jehrr

@jehrr jehrr commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes the findings of the 2026-10-14 audit (base 3c6665d).

Problems and resulting behaviour

Finding Before After
H1 paid-solve budget SOLVES_PER_PAGE applied per settle() call; a re-fetch after a refusal started a new allowance: 2 solves/page with no pool, 4 with a 4-exit pool one SolveBudget per page, threaded through every navigation, landing and the reviews language check; checked before each purchase; not reset by retry or rotation. 1 solve in every scenario
H2 parser returns 0 on a page that states rows page 1 → exit 4 ("empty"); page 2+ → exit 0, complete, end_of_listing stop_reason: parser_found_nothing, page saved as debug HTML; exit 5 (page 1) / 6 (later), with no new exit code; canary fails on it by name. An author list past its end (404) and a search with no results are still exit 4; a shelf's constant page count and a reviews filter total of 0 are not treated as stated rows
M1 atomic writes open(path,"w"): a failed dump left 10 bytes of a 539-byte file _atomic (temp in target dir, flush+fsync, os.replace, unlink on error, newline="" for CSV) for JSON, CSV, sidecar; new file 0o666 & ~umask, existing file keeps its mode
M2 CSV formulas =HYPERLINK(...) written verbatim string cells (after list join) starting = + - @ \t \r \n get '; numbers untouched; JSON untouched; csv_cells_escaped in the sidecar, merged UNDER the caller's extra
M3 private citations ~109 citations of private notes in 14 tracked files, one in a user-facing --concurrency error removed, each rule restated in place; new suite guard (tokens assembled from pieces)
M4 donor prose captcha_solver docstring/closing section about another site, shop_rating, "announcement codes", unmeasured "451" rewritten for goodreads / removed
CLI floors (incl. L5) negative --delay, --retry-delay, --proxy-block-retries; --pages/--concurrency/--max-books 0; negative --retries in scraper_api_client.py silently clamped page_flow.bounded type in all three engines and the API client → exit 2 naming the flag
Retracted claim "a profile's credentials last about a day" in the 401 hint and .env.example the HTTP 401 is named as the signal, no age claimed

CHANGELOG [Unreleased] leads with a blockquote listing the behaviour changes. Released sections untouched (they held no citations). Release notes v0.1.0/v0.1.1: 0 citations, not edited.

Evidence

  • Offline suite, core-only venv (Python 3.13): 425 passed, 0 failed, 5 engine groups skipped (drivers absent).
  • Per-engine venvs (requirements + that engine, no browser download): playwright 430 passed / 2 skipped, selenium 427 / 4, puppeteer 427 / 4. In each, --help works and --delay -1 / --proxy-block-retries -2 give error: argument --X: --X must be at least 0.
  • ci_checks.py --sample-check --secret-check OK (47 files).
  • The canary's Assert block was extracted and run against a sidecar with parser_found_nothing: it fails naming it.
  • Planted-fault controls, each RED on the named check and green once restored (PYTHONDONTWRITEBYTECODE=1, __pycache__ cleared): budget passed as None per call (2/2/4 solves) · budget reset on relaunch · parse_failed not set (4 / 0-complete) · truncating JSON writer · no chmod · flat 0644 under umask 002 · no CSV escape · escape before list join · stats merged over extra · plain float --delay in selenium · plain int --retries in scraper_api_client · a planted citation.

Canary (second commit)

The canary already ran ungated every day and failed on any non-zero exit; the last 10 scheduled runs were green with real rows. The Assert step now also fails when any row has no value in a core column (list: sku/title/author; book: also work_id; reviews: sku/reviewer/created_at), and it names parser_found_nothing. No secret is required. Dispatched on this branch: https://github.com/2scraper/goodreads-scraper/actions/runs/38694402188, success (300 list rows, 5 books, 90 reviews).

Not changed

  • L1 refusing --proxy/--fingerprint/--concurrency with --cdp-endpoint (behaviour change; owner decision).
  • L2 documenting --category as intentionally absent (owner decision).
  • L3 a paid-but-empty document reading as exit 5 (candidate only; not reproduced against a real solve).
  • Version bump / tag / release (L4/T13); schema and sinks (T11).
  • H2 is not live-measured per listing kind; the rule only fires when the page's own title/header total or pagination says rows exist, and shelves/series rely on a stated total only.
  • Repo description is 29 words (informational in the org audit); left alone.

🤖 Generated with Claude Code

jehrr and others added 2 commits October 15, 2026 06:42
…tput

- A page buys at most one CAPTCHA solve across every attempt, relaunch and
  proxy rotation (was 2 with no pool, one per exit with a pool). The suite's
  `solves <= 2` pinned the old behaviour; it now asserts 1.
- A served page whose own total says it holds rows, parsed to none, is
  `parser_found_nothing`: exit 5 on page 1 (was 4), exit 6 later (was 0,
  complete). The canary names it.
- JSON, CSV and sidecar writes are atomic, with the mode open() would give.
- CSV cells starting with = + - @ TAB CR LF get a leading apostrophe;
  `csv_cells_escaped` in the sidecar, merged under the caller's extra.
- Counted and timed flags refuse values below their floor in every entry
  point (exit 2 naming the flag).
- Rules are stated in place instead of pointing at private notes; a suite
  guard keeps it that way. Captcha module notes describe this site.
- The expired-endpoint hint no longer claims profiles last about a day.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A row count alone passes when the parser fills rows with nulls. sku, title
and author (list, book; plus work_id on books), and sku, reviewer and
created_at (reviews), are filled on every row of every capture, so one
null fails the canary.

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