Skip to content

Report lost pages, dead exits and bad flags honestly; harden the key scan - #18

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 the findings of a rules audit of main at 7fac23d. The live canary failure is diagnosed and fixed separately in #17.

Problem → resulting behaviour

Finding Before Now
Concurrent run loses a page A worker raising on page 3, or worker browsers failing to launch, ended complete, exit 0. The in-flight page appeared in no list. The raising page goes into pages_failed. Queued pages no worker reached go into the new pages_unattempted sidecar field with stop_reason: pages_unattempted. Either way the run is partial, exit 6.
Navigation timeout or dead proxy land() returned blocked, so the run exited 3 with blocked_by='no-response' and advised buying an exit. New states unreachable and proxy_failed, with stop_reason page_load_timeout or proxy_failed, give exit 5. The store-lookup paths do the same. block_advice says "no document was received" when the HTML is empty. A real Akamai denial is still exit 3.
Numeric flags --pages 0/-2 fetched one page and reported complete. --delay -1 crashed after page 1 with its rows unsaved. --concurrency 0 and negative retry counts were accepted. All of these are argparse errors (exit 2) naming the flag, in all three engines, through shared page_flow validators.
.env.example URL A copied .env made --postcode 3000 a search-listing run. WOOLWORTHS_URL is commented out. Engines capture typed_url before .env is applied and decide the store shortcut on it.
diff_runs.py on store files 0 added, 0 removed, 0 changed, exit 0, with address, phone and latitude changed. Refused with exit 2, also under --force, because the tool compares product prices.
Credential scan It never read .json or .csv files or an untracked .env.bak. A clone under a parent named venv scanned 0 files and passed. The suite never ran the scan. It reads those files and .env* (except .env), and matches JSON-escaped key fields. Exclusions are relative to the repo. A scan of 0 files fails. A context word only excuses a 32-hex run when it sits near the match on long lines. The suite runs secret_check().
pyppeteer engine page.authenticate crashed on current Chromium. The user agent was a hardcoded Windows string. connect(ignoreHTTPSErrors=True) was sent to the managed browser. Falls back to CDP Fetch.authRequired and answers only Proxy challenges. Keeps the browser's own UA and only strips "Headless". connect() drops ignoreHTTPSErrors.
Private citations About 280 section-sign references and mentions of the private notes, including released CHANGELOG sections. All removed or restated in the repo's own words. A new suite guard, test_no_private_citations_in_shipped_files, uses assembled tokens. Release notes v0.2.1 and v0.3.0 were edited: only the citation sentence changed, verified byte-for-byte.
Low items .gitignore missed X.pageN dumps, live/ and top-level *.html. Docs said "credentials expire in about a day". Donor (Medium) prose remained in docstrings. The banned stem appeared in a docstring. CI had a duplicate inline sample check, and its 3.9/3.12 matrix did not match the 3.9–3.13 badge. All fixed. The banned-wording list now includes the bare stem. CI runs 3.9, 3.12 and 3.13.
Canary Messages assumed a proxy for exit 3, and exit 5 was unhandled. Clearer 3 and 5 messages. New floors: every row must have a title and a URL. It stays ungated and requires exit 0 and real rows. No secret is required: none is set, and the README promises none is needed.

Evidence

Offline suite, run in four venvs with PYTHONDONTWRITEBYTECODE=1 and __pycache__ cleared:

  • core-only: 128/128, three engine groups skipped
  • +playwright: 128/128
  • +selenium: 128/128
  • +pyppeteer: 128/128

ci_checks.py --all and --history-check are clean (142 blobs).

Controls. Each one plants the fault, confirms the suite goes red on the named check, then restores it and confirms green:

  • playwright_scraper.py from main: red on test_a_worker_that_raises_neither_hangs_the_run_nor_loses_its_siblings (rewritten: it used to assert the raising page was lost), test_a_concurrent_run_that_loses_a_page_is_partial_not_complete and test_workers_that_never_launch_leave_a_partial_run.
  • land() returning blocked again: red on test_the_shared_loop_runs_end_to_end_against_a_fake_driver, new cases 7 and 8.
  • diff_runs.py from main: red on test_diff_runs_refuses_store_lookups_instead_of_reporting_no_change.
  • --delay back to type=float: tried in each engine separately; each one is red on test_numeric_flags_are_validated_in_every_engine.
  • Selenium store branch keyed on args.url: red on test_a_url_from_env_does_not_turn_a_store_lookup_into_a_listing.
  • Credential scan, planted in throwaway copies: a 32-hex key in sample_output.json, TWOCAPTCHA_KEY=<hex> in an untracked .env.bak, a JSON-escaped clientKey inside the served-page fixture, and a key in notes.py in a clone under …/venv/ws. Each one fails ci_checks.py --secret-check by file:line and fails test_the_credential_scan_runs_here_and_finds_nothing. A clean clone under …/venv/ws scans 42 files and passes.
  • Citation guard: planting a private-file name in README, a section sign in the Dockerfile, "section 19" in env_config.py or a rules-directory name in .gitignore turns test_no_private_citations_in_shipped_files red. A use_<stem> in captcha_solver.py turns test_banned_wording red.

Behavioural probes:

  • With a copied .env, --postcode 3000 now parses as mode=stores in all three engines.
  • Every bad numeric value exits 2 with error: argument --X: --X must be at least N.
  • pyppeteer, live and headful, through a local proxy that requires Basic auth (no paid exit, no credential) with --chromium-path set to Chromium from the Playwright cache: the proxy logged 407 challenges answered and 87 authenticated tunnels, and the run returned 37 rows, exit 0. The same command on main dies with Protocol error (Network.setRequestInterception), exit 1.
  • .gitignore: git check-ignore ignores live/x.html, x.page3, canary_dump.json.page3, dump.html and sub/dump.page2, and keeps the samples, fixtures, .env.example, README and tests.

Not changed

  • Canary skip/notice policy beyond the above, versions, tags and releases (--retries 0 and these exit-code changes sit unreleased under [Unreleased]), schema/sinks, and shared-loop refactors.
  • --allow-empty on a genuinely empty listing writes status: failed. That is an open question for the owner.
  • diff_runs store support. Store files are refused rather than diffed; supporting them would be a feature, and it is the owner's call.
  • The Scraping Browser connect() change was not exercised live, because that needs a paid credential. The pyppeteer UA change was checked from source only.
  • The DOM cross-check race behind the red canary is fixed in Wait for the product grid before the DOM price cross-check #17. Until Wait for the product grid before the DOM price cross-check #17 lands, a canary run of this branch can still go red on that race.

🤖 Generated with Claude Code

…scan

- A concurrent run that lost a page (worker raised, or workers never
  launched) now ends partial (exit 6) with pages_failed /
  pages_unattempted, instead of complete, exit 0.
- A navigation that never returned a document (timeout, failed proxy) now
  exits 5 with page_load_timeout / proxy_failed instead of 3 "blocked".
- --pages/--concurrency below 1 and --delay/--retry-delay/
  --proxy-block-retries below 0 are usage errors in all three engines.
- A WOOLWORTHS_URL from .env no longer turns --postcode into a listing run;
  .env.example ships it commented out.
- diff_runs.py refuses store-lookup files instead of reporting 0 changes.
- pyppeteer answers proxy auth through CDP Fetch on current Chromium, keeps
  the browser's own user agent, and connects without ignoreHTTPSErrors.
- The credential scan reads JSON/CSV/.env backups, sees JSON-escaped key
  fields, judges exclusions relative to the repo, fails on 0 files, and
  the offline suite runs it.
- Private-note citations removed from shipped files, with a guard.
- .gitignore, docs, canary messages and CI matrix brought in line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jehrr
jehrr force-pushed the fix/audit-2026-10-14 branch from 2dc134b to a13868a Compare October 15, 2026 13:55
@jehrr
jehrr marked this pull request as ready for review October 15, 2026 13:56
@jehrr
jehrr merged commit 69d7feb 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