Skip to content

Atomic output, CSV formula guard, one paid solve per page, typed --category wins over SLEEPNUMBER_URL - #7

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

Works through the 2026-10-14 audit of main at 1ded84e (v0.2.0). No open PRs overlapped.

Problems and resulting behaviour

# Problem (reproduced on main) Now
F1 write_json/write_csv/write_run_meta used open(path, "w"). A failure inside json.dump turned a 559-byte out.json into 13 bytes of invalid JSON, and clobbered the sidecar too. _atomic(): temp file in the target's dir, flush + fsync, os.replace, unlink on error, newline="" kept for CSV. New file 0o666 & ~umask; an existing file keeps its mode.
F2 No CSV formula neutralisation. String cells (after list joining) starting with = + - @ TAB CR LF get ' in CSV only. Numbers and JSON are untouched. csv_cells_escaped goes in the sidecar, merged UNDER caller extra. Fires on 0 of 362 string cells in the committed sample and fixtures.
F3 The solver ran once per block attempt: 3 solves for one page with a 3-exit pool (stubbed solver). page_flow.SolveBudget(SOLVES_PER_PAGE=1) is created once per page before the retry/rotation loop and passed to the only solve call site in each of the three browser engines. claim() happens immediately before solve_recaptcha, so detecting nothing or having no key spends nothing. A failed purchase still counts.
F4 The credential scan tested absolute path.parts: a clone under venv/ scanned 0 files and printed the clean line. .env.bak and .json were never read. Exclusions use path.relative_to(REPO).parts, and a scan of 0 files fails. .json .csv .toml .cfg .ini and .env.* (renamed copies) are read; .env itself is not, since it is meant to hold the user's real key. Now 43 files are scanned, all clean.
F5 SLEEPNUMBER_URL silently replaced a typed --category in every engine. typed_url is captured before env_config.apply, then page_flow.resolve_start_url: typed --url > typed --category > env URL > default. --url + --category is a usage error. .env.example ships the variable commented out.
F6 About 21 CLAUDE.md and about 90 § citations in shipped files. All removed, with the rule restated in place. New guard test_no_private_citations_in_shipped_files (tokens assembled from pieces). Release notes for v0.1.0 and v0.2.0 and the released CHANGELOG sections had none, so nothing was edited there.
F7 Canary green on every run because it skipped (no SLEEPNUMBER_PROXY). Without the secret the job fails with ::error:: naming SLEEPNUMBER_PROXY. Exit 3/4/5/6 were already red. The thresholds are row floors plus parser properties; the discount share stays informational.
F8/F9 Comments presenting transfermarkt/tokopedia/medium facts as this site's. .env.example said "the block tracks geography". .env.example and the README said credentials "last about a day". Rewritten to describe this site, or deleted. "Reputation, not geography" now matches the README. The lifetime is stated as unmeasured.
F11 pyppeteer discarded the goto Response. Status and headers are passed to classify, as in Playwright.
F12 Badge says 3.9–3.13, CI tested 3.9/3.12. Matrix is now 3.9, 3.12, 3.13.
F13 The image lacked the recommended http_scraper.py. Copied, with a usage note in the Dockerfile.
item 6 Negative counts and delays were accepted. --pages/--concurrency must be ≥1; --delay/--retry-delay/--proxy-block-retries must be ≥0. This applies to all five CLIs, and argparse names the flag in the error.

Evidence

  • Suite, PYTHONDONTWRITEBYTECODE=1, no browser download:

    • core-only venv: 78/78 passed, 4 groups skipped (the 3 engines plus the behavioural budget probe).
    • playwright venv, selenium venv and puppeteer venv: each 78/78, 2 skipped. The engine's own checks ran, including the behavioural budget probe that drives the real handle_captcha_if_present 3 times against one budget with the detector and solver stubbed. Result: 1 solver call per engine.
  • Real argparse of http, playwright, selenium and puppeteer with SLEEPNUMBER_URL=…/mattresses --category furniture gives …/categories/furniture. --url X --category Y gives exit 2, and --pages 0 gives exit 2.

  • ci_checks.py --help-check --sample-check --secret-check --published-check passes.

  • Controls. 19 planted faults, each applied with an assert that the edit changed the file, with __pycache__ cleared, and then restored. Each turned the suite red on the named check:

    • plain open() in write_json: atomic check;
    • chmod removed: atomic check (umask 002/077);
    • escape disabled: CSV check;
    • extra-merge order reversed: CSV check;
    • claim removed (core source check, and the playwright venv behavioural check);
    • selenium budget moved inside the loop: source check;
    • puppeteer call site without budget=: source check;
    • spent += 1 removed (selenium venv): behavioural check;
    • absolute path.parts: scan check;
    • .env. names dropped: scan check;
    • .json dropped: scan check;
    • env URL winning over category: precedence check;
    • typed_url captured after apply: precedence check;
    • positive_int accepting 0: numeric check;
    • selenium --delay type=float: numeric check;
    • CLAUDE.md in README: citation guard;
    • § in tools/capture.py: citation guard;
    • pyppeteer status dropped: pyppeteer check.

    The tree is green after restore.

  • No paid calls, no proxies, no credentials, no requests to sleepnumber.com.

Not changed, and why

  • page_flow.should_solve / the solve policy column is still unread by the engines (F10). Owner decision: wire it or delete it.
  • scraper_api_client.py --category is a row LABEL there, not a URL builder, so the env-URL precedence change does not apply to it.
  • Zero-file scan guard: covered indirectly. The venv-parent check requires >10 files scanned; there is no separate planted control for the guard alone.
  • No version bump, tag or release. Entries are under [Unreleased] with a behaviour-change blockquote.
  • Shared-loop refactor, schema/sinks, the commit-subject convention (F14): out of scope.
  • Repo metadata: the description is 26 words (informational only), so nothing was edited.
  • The canary will be red on schedule until SLEEPNUMBER_PROXY (a residential exit) is set. This is intended: before, it was green without ever reaching the site.

🤖 Generated with Claude Code

…tegory wins

- JSON, CSV and the sidecar are written to a temp file in the target's
  directory, fsynced and renamed; a failed write no longer truncates the
  previous good file. New files take 0o666 & ~umask, existing ones keep
  their mode.
- CSV string cells starting with = + - @ TAB CR LF get a leading
  apostrophe (CSV only); the sidecar records csv_cells_escaped, merged
  under the caller's extra.
- page_flow.SolveBudget: one paid solve per page across every retry and
  exit rotation, claimed immediately before the purchase, in all three
  browser engines.
- The credential scan tests repo-relative paths, fails on zero files, and
  reads .json/.csv/.toml/.cfg/.ini and renamed .env copies.
- A typed --category wins over SLEEPNUMBER_URL; --url plus --category is a
  usage error. .env.example ships SLEEPNUMBER_URL commented out.
- --pages/--concurrency >= 1, --delay/--retry-delay/--proxy-block-retries
  >= 0 in every entry point.
- pyppeteer passes the goto status and headers to the classifier.
- The canary fails with an error naming SLEEPNUMBER_PROXY instead of
  skipping green.
- Comments describing other sites' measurements as this site's are
  corrected; references to private notes are replaced with the rule in
  the repo's own words, with a suite guard.
- Docker image carries http_scraper.py; CI tests Python 3.13.

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