Skip to content

Safer output files, a diff that sees article changes, stricter flags, a canary that cannot skip green - #1

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 (base 41048f4, unchanged since the audit).

Problems and resulting behaviour

  • Non-atomic writes. write_json/write_csv/write_meta truncated in place; an interrupted write left 10 bytes of JSON and a 1-byte sidecar. Now: temp file in the target's directory, flush + fsync, os.replace, temp removed on error; new file 0o666 & ~umask, existing file keeps its mode.
  • CSV formula injection. User-written titles/summaries/authors went into CSV raw. Now string cells starting with = + - @ TAB CR LF get a leading ' (CSV only, numbers untouched, JSON unchanged); csv_cells_escaped in the sidecar, merged under the caller's extra keys.
  • PERPLEXITY_URL hijacked typed flags. With it in env/.env, --discover top or --urls-file silently ran one article with exit 0. Now the variable fills --url only when neither is typed; .env.example ships it commented out.
  • diff_runs.py could never report a change. It compared only price/currency, which are always null here. It now reports changed (title, author, summary, dates, section/word/source counts, slug, image), which --fail-on-change acts on, and counters_changed (views/likes/forks, informational). Tracked names are validated against Product at import.
  • Engine parity. Selenium/pyppeteer accepted --retries -1 (zero navigations, then exit 5) and negative delays and --max-solves. All three engines now share page_flow.nonnegative_int/float for --retries, --retry-delay, --delay-between-pages, --max-solves and --proxy-block-retries (exit 2, with an error naming the flag; 0 stays legal).
  • Credential scan. It now prints and floors its file count (MIN_FILES_SCANNED) and no longer exempts smoke_test.py; that file's credential-shaped test values are built from pieces.
  • env_config report. It now prints a secret's length only, not its first and last characters.
  • Private citations. Every pointer to a maintainer-only document or its section numbers is gone from code comments, check labels, .gitignore, the workflows and the CHANGELOG, including the released section (citations only; nothing else there changed). The Claude review prompt now states the invariants inline instead of pointing at a file CI never has. A smoke check fails if a citation comes back; its tokens are assembled from pieces.
  • Other sites' material. Comments that described other sites instead of this one (in captcha_solver.py, scraper_api_client.py, output_writer.py, selenium_scraper.py, smoke_test.py) and a person's name are rewritten or removed. Selenium's timeouts are now stated as shared with the other engines and not separately measured.
  • Retracted claim. The Scraping Browser 401 message no longer says endpoints "expire after about a day".
  • Canary. All 13 scheduled runs (2026-10-02 to 2026-10-14) skipped green because the PERPLEXITY_CDP_ENDPOINT secret does not exist. A missing secret now fails the job with ::error::. The existing assertions are unchanged: exit 0, status: complete, row floor, and non-null sku/title/section_count/sources_json/published_at.

Evidence

  • Core-only venv: smoke_test.py 66/66, test_regressions 12 OK, ci_checks.py passed (43 files), --sample-check OK.
  • One venv per engine (playwright, pyppeteer, selenium; no browser download): driver imported, and the suite and the regression tests passed in each.
  • <engine>_scraper.py --discover top --retries -1 exits with argument --retries: must be >= 0 in all three engines.
  • Ten planted-fault controls, each run with PYTHONDONTWRITEBYTECODE=1 and __pycache__ cleared. Each one went red on the named check, and the suite went green again after restoring:
    • in-place JSON write → atomic check;
    • flat 0o644 mode → atomic check;
    • CSV escape disabled → CSV check;
    • housekeeping merged over the caller's extra → CSV check;
    • env URL hijack restored → env check;
    • Selenium --retries type=int → negative-flag check;
    • diff content comparison blinded → diff check;
    • a planted citation → citation check;
    • file-count floor raised → ci_checks "read only N files";
    • a real-shaped credentialed URL in smoke_test.py → ci_checks failed.

Not changed

  • No version bump, tag or release.
  • --allow-empty writing over good output on a blocked run (F13), and inert flags such as --max-solves/--min-score (F10): product decisions.
  • Docker test material in earlier layers (F11): low; no Docker daemon here to verify a multi-stage build.
  • Discover goto status not passed to classification (F15): low; it needs a live run to avoid misreading a clearing challenge.
  • The hCaptcha code comments in captcha_solver.py: owner decision, left as they are.
  • Old commit subject 462e5f8: history is not rewritten.

🤖 Generated with Claude Code

- Output, CSV and sidecar writes are atomic (temp file + fsync + rename),
  keeping an existing file's mode and giving a new one 0o666 & ~umask.
- CSV formula cells get a leading apostrophe; csv_cells_escaped in the
  sidecar, merged under the caller's extra keys.
- PERPLEXITY_URL no longer overrides a typed --discover / --urls-file;
  .env.example ships it commented out.
- diff_runs compares article content (changed) and counters
  (counters_changed) instead of always-null price columns.
- Selenium and pyppeteer reject negative --retries, --retry-delay,
  --delay-between-pages, --max-solves, --proxy-block-retries.
- Credential scan reports and floors its file count and now scans
  smoke_test.py; env_config's report prints only a secret's length.
- Comments, check labels, the CI review prompt and the changelog state
  rules in this repo's own words; a smoke check keeps private citations out.
- Canary fails with ::error:: when PERPLEXITY_CDP_ENDPOINT is missing.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@jehrr
jehrr marked this pull request as ready for review October 15, 2026 21:15
@jehrr
jehrr merged commit a39e9f1 into main Oct 15, 2026
13 of 14 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