Skip to content

Atomic output, CSV formula guard, one paid solve per page, canary names a dead proxy - #21

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, plus a canary that says why it is red. The base is 05824fe, which is the audited SHA.

The canary (live issue)

  • Before: 36 of the last 38 runs failed. The only successes were the two dispatches on 2026-09-09, the day MEDIAMARKT_PROXY was set. Every scheduled run since 2026-09-10 shows the same thing in the job log: Using proxy exit ***eu.proxy.2captcha.com:2334, then Timeout loading … (attempt 1/3), 2/3, Gave up. That surfaced as exit 4 in September and as exit 5 since then, and exit 5 is printed as "Unexpected exit code". The 09-09 run used the same proxy host and fetched 3 pages and 36 rows in about 17 s. A timeout with no 403 and no proxy error, starting the day after it last worked, is what an expired or out-of-traffic proxy account looks like. The secret most likely needs replacing.
  • After:
    • A missing secret fails the job with ::error::Missing secret MEDIAMARKT_PROXY. It no longer skips green.
    • A new step makes an informational request to the site from the bare runner with no proxy. It reports 403 (datacentre refusal, so the site is up), 200, or no answer.
    • A new preflight (.github/canary_preflight.py) asks the proxy directly, with short timeouts, before the browser run. It names the cause: gateway unreachable, credentials rejected (407), tunnel refused, or a connection that delivers nothing. It also warns when the login asks for a region other than de. The secret is never printed, only host:port and the region code.
    • Exit 3, 5 and 0 rows all stay red.
    • Verified locally against fake proxies: a closed port, a black hole, a 407 server and a 502 server each produce their own ::error::, and the password never appears in the output.
  • The 5) branch of the Interpret step is covered by Canary knows exit 5, refusal names the country, dumps are 0600 #19 and was not touched here.

Fixes

  • Atomic writes: JSON, CSV and the sidecar go through _atomic: a temp file in the target's directory, fsync, os.replace, and the temp file removed on error. A new file gets 0o666 & ~umask; an existing file keeps its mode.
  • CSV formula guard: applied to text cells only, after list joining, and in the CSV only. csv_cells_escaped is merged under the caller's extra.
  • Solve budget: page_flow.SolveBudget, with SOLVES_PER_PAGE = 1, is created once per page before the block-retry loop. It is checked before solve_recaptcha in all three engines and is not reset on rotation.
  • HTTP status: Playwright and pyppeteer pass goto()'s status to classify, and clear it after a solve. Selenium passes an explicit None, because it has no status, and says so in the code.
  • reCAPTCHA variant: a challenge frame now means v2, and v2-invisible when size=invisible. An Enterprise widget is flagged, and solve_recaptcha raises before any request.
  • CLI validation: --pages/--concurrency must be ≥1, and --proxy-block-retries/--delay/--retry-delay must be ≥0, in all engines (exit 2, naming the flag). In scraper_api_client, --retries/--retry-delay must be ≥0.
  • Credential scan: exclusions are judged relative to the repo. Scanning 0 files is now a failure. .json files and .env.* backups (but not .env) are scanned.
  • .gitignore: adds *.page[0-9]*, /*.html, live/ out/ output/ dumps/ scratch/.
  • Status: --allow-empty on a complete run with 0 rows now gives status: complete.
  • Wording: "ten" country sites, no stale check count in the README, and the Scraper API --wait-element example uses this site's selector.
  • Private citations: removed from code, the suite and CHANGELOG, including the released 0.1.8 section (citations only). A new check, test_no_private_citations, uses assembled tokens. The v0.1.8 release notes were edited in the same way.

Evidence

  • Suite: core venv 379 PASS; +playwright, +pyppeteer and +selenium each OK. Each engine group ran a behavioural check: three challenged attempts with one budget buy exactly 1 solve. Playwright also has a stub check that a 403 from goto() gives blocked even when the body holds a product link. ci_checks.py --all passes.
  • 20 planted-fault controls, run with PYTHONDONTWRITEBYTECODE=1 and __pycache__ cleared. Each went red on the predicted named check, and the restored tree is green. They cover:
    • atomic JSON, temp-file cleanup, the umask rule, existing-mode keeping;
    • CSV escape, escaping numbers, extra merge order, allow-empty status;
    • Playwright budget (a dead guard and a removed guard), the Selenium handler without the page budget, the pyppeteer budget not created, take() off-by-one;
    • status dropped (Playwright, pyppeteer);
    • the frame→v3 rung, the enterprise guard;
    • --pages back to int, the delay validator, a re-added citation.
  • Credential scan: from a copy under venv/, .venv/ and build/out/ it scans 36 files. A planted key in .env.bak or dump.json is caught. The old absolute-path line under venv/ → "0 files scanned" FAILED.
  • CLI: --pages 0 and --delay -1 → argparse error naming the flag in pyppeteer and Selenium.

Not changed

  • Covered by Canary knows exit 5, refusal names the country, dumps are 0600 #19 (owner, open):

    • the canary 5) branch and README exit-5 row;
    • 0600 debug dumps;
    • the country hint in refusals.

    Canary knows exit 5, refusal names the country, dumps are 0600 #19 conflicts with main on CHANGELOG already, and will also conflict with this PR in CHANGELOG.

  • Owner decisions or out of scope:

    • F14: refusing --concurrency in pyppeteer/Selenium, and flag parity;
    • F9: pyppeteer proxy auth / UA / CDP connect, which needs live credentials;
    • F11: unread should_solve/counts_as_blocked;
    • F15: the vendor-scan gating and markers, which needs CDP captures;
    • shared-loop refactor, version, tag, release.
  • No paid calls, no proxies, no credentials used.

🤖 Generated with Claude Code

…a dead proxy

- Output JSON/CSV/sidecar are written atomically (temp file, fsync, rename),
  new files keep the umask's mode and existing files keep their own.
- CSV cells starting with = + - @ TAB CR LF are neutralised (text only, CSV
  only); csv_cells_escaped is recorded in the sidecar under the caller's extra.
- Every engine checks a per-page SolveBudget before buying a solve; retries
  and rotations no longer buy another one.
- Playwright and pyppeteer pass goto()'s HTTP status to the classifier.
- reCAPTCHA: a challenge frame means v2; an Enterprise widget is not solved
  with an ordinary task.
- --pages/--concurrency/--proxy-block-retries/--delay/--retry-delay reject
  nonsense values with a usage error in all engines; the Scraper API client
  rejects a negative --retries/--retry-delay.
- Credential scan judges exclusions relative to the repo, fails on 0 files,
  and reads .json and renamed .env copies. .gitignore covers more dumps.
- --allow-empty complete runs are status complete.
- Canary fails when MEDIAMARKT_PROXY is missing, checks the proxy first and
  names why it cannot deliver a page, and probes the site from the bare runner.
- Private-note citations removed; a suite check keeps them out.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jehrr
jehrr marked this pull request as ready for review October 15, 2026 21:16
@jehrr
jehrr merged commit d86112a 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