Skip to content

Safer output files, honest exit codes and a canary that must test the site - #16

Merged
jehrr merged 1 commit into
mainfrom
fix/audit-2026-10-14
Oct 15, 2026
Merged

jehrr merged 1 commit 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 0895e83, plus the canary rule: a canary that did not test real rows must not be green.

What changes for a user

  • Atomic output (F1). write_json, write_csv and write_run_meta write a temp file in the target's directory, then flush, fsync and os.replace. A failed write leaves the previous file intact and no temp file behind. A new file gets 0o666 & ~umask and an existing file keeps its mode. The implementation is the one already used in the sibling repos.
  • CSV formula guard (F2). String cells, including joined list cells, that start with = + - @ TAB CR LF get a leading ', in CSV only. Numbers and JSON are untouched. The sidecar gets csv_cells_escaped, merged under the caller's extra.
  • Exit codes (F5, F6, F11). Playwright and pyppeteer pass the navigation's HTTP status to classify, so a marker-less 401/403/429/5xx shell is blocked (exit 3) instead of an exit-4 "empty listing". The status is dropped after a solve replaces the document. Selenium has no status to pass, and passes an explicit None with a comment saying so. A page served WITH tiles that parses to 0 rows is now parser_found_nothing, which gives exit 5 with no rows and exit 6 after earlier pages. All three engines get it through one page_flow.failure_stop_reason. A complete empty --allow-empty run writes status: complete. The dead single_page_mode reason is removed (F14).
  • CLI validation. --pages, --concurrency (≥1), --delay, --retry-delay and --proxy-block-retries (≥0) are validated in all three engines, with an argparse error that names the flag. In scraper_api_client.py, --retries and --retry-delay must be ≥0 (0 retries is legal there because that flag counts extra attempts).
  • Credential scan (F9). Exclusions are tested on path.relative_to(REPO).parts. Zero files scanned is now a failure. Untracked .json and .env.* backups are scanned. .env itself is not scanned, because it holds the user's own key.
  • pyppeteer parity (F12). A plain load failure no longer relaunches and re-spends --retries for every block retry. Only an unusable exit with a pool rotates, which matches Playwright and Selenium.
  • Wording (F4, F16, F3). The "profile lives about a day" claim is replaced in README, TROUBLESHOOTING, .env.example and canary.yml. The Turnstile log no longer calls a UA mismatch the likeliest cause of a refused token. About 148 internal section-number citations and one reference to a private instruction file are removed from tracked files, including released CHANGELOG sections (citations only). test_no_private_citations assembles its tokens from pieces so it does not match itself. test_wording now scans .github/, the Dockerfile, .env.example and the suite itself, again with assembled phrases (F15). The release notes v0.1.0, v0.2.0 and v0.2.1 contained no citations, so they were not edited.
  • .gitignore (F10). Adds *.page[0-9]*, /*.html, live/, out/, output/, dumps/ and scratch/. A suite check confirms that dumps are ignored and shipped files are not.
  • Canary. With neither FOODPANDA_PROXY nor FOODPANDA_CDP_ENDPOINT set, the job now FAILS with ::error:: naming both secrets; it used to skip green. Exit 5 is red. Exit 6 runs the data floors on the rows it gathered. Exit 3 and exit 4 were already red. It stays dispatch-only: a datacentre runner cannot reach the site without a paid exit, so a schedule only makes sense once a secret exists. This repo has no secrets configured, so a dispatch is expected to be red until one is added.

Evidence

  • Offline suite, run once per venv: core-only, 606 checks OK. Core + playwright (no browser): 625 checks OK, which includes the behavioural _fetch_one_page stubs (a 403 shell is blocked, a 200 shell is parsed, a li→div fixture gives parser_found_nothing). Core + pyppeteer: 608 OK. Core + selenium: 608 OK. Each engine's own groups ran in its venv.
  • CLI: --pages 0, --delay -1, --concurrency 0, --proxy-block-retries -1 and --retries 0 → exit 2 with error: argument --<flag> in pyppeteer and Selenium.
  • Credential-scan probe: clones under venv/, .venv/ and build/ each scan 37 files. A planted 32-hex value in .env.bak and in live_dump.json is caught; one planted in .env is not read.
  • Planted-fault controls: 18 in total, run with PYTHONDONTWRITEBYTECODE=1 and the cache cleared each time. Each turned the suite red, and the restored tree is green. 17 went red on the predicted check. One (adding parser_found_nothing to the COMPLETE reasons) went red on "parser_found_nothing is not a COMPLETE stop reason" instead of the exit-5 check, because that test's pages_failed=[1] already makes the run incomplete. The faults: non-atomic write_json, flat 0o644 mode, removed CSV escape, reversed extra merge, old allow_empty status, the COMPLETE-reasons change above, absolute-path exclusions, .json dropped from the scan, --pages type=int in Selenium, the validator minimum removed, Selenium classify without None, pyppeteer classify without status, the old pyppeteer load-failure branch, parser_failed removed in Selenium, parser_found_nothing removed from the stop-reason helper, a planted section citation, *.page[0-9]* removed from .gitignore, and a banned phrase planted in smoke_test.py.

Not changed

  • F7, the --fingerprint README claim. Testing it needs a key and a paid re-measurement, so it is left for the owner.
  • F8, the version/tag mismatch (0.2.2 in pyproject without a tag). No version bump, tag or release here.
  • F13, --proxy/--concurrency with --cdp-endpoint, which are still warnings. Making them usage errors is the owner's decision.
  • Adding a canary schedule. That decision belongs with adding a secret.
  • The shared fetch loop refactor, a schema and sinks are out of scope.
  • Sibling repo names in code comments are kept as provenance.
  • The inline heredocs in tests.yml are left in place (low).
  • .env.example's FOODPANDA_URL is kept. This repo has no URL-builder flags for it to hijack (--category is only a run label), and a typed --url still wins.

🤖 Generated with Claude Code

… site

- JSON, CSV and the sidecar are replaced atomically and keep umask/existing
  modes; CSV formula cells are neutralised and counted in the sidecar.
- A served grid that parses to nothing ends as parser_found_nothing (exit
  5/6), and Playwright/pyppeteer pass the HTTP status to the classifier.
- Counting and delay flags reject impossible values in every engine.
- Credential scan judges exclusions inside the repo, fails on 0 files, and
  reads untracked .json dumps and .env.* backups.
- pyppeteer load-failure retries match the other engines.
- The canary fails when no paid exit is configured or nothing was fetched.
- Corrected the Scraping Browser lifetime claim, removed internal section
  citations, widened .gitignore for dump shapes.

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 564da4a into main Oct 15, 2026
7 of 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