Skip to content

Fix the correctness findings of the 2026-09-18 external audit - #1

Closed
2ione wants to merge 1 commit into
mainfrom
fix/audit-2026-09-18
Closed

2ione wants to merge 1 commit into
mainfrom
fix/audit-2026-09-18

Conversation

@2ione

@2ione 2ione commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

No description provided.

Each was reproduced against v1.0.1 before being touched, per CLAUDE.md §14,
and each has a regression check in smoke_test.py (596 checks, up from 578).

diff_runs buried material changes (the worst of them, and mine)
  A row whose price AND price_source both moved went wholesale into
  `source_changed`, taking any other field with it. Reproduced: price
  10->11, price_source tile-text->jsonld, availability InStock->OutOfStock,
  title Old->New produced `changed=[]` and --fail-on-change saw nothing. A
  product going out of stock is never an artefact of how its price was read.
  Only `price`/`currency` are routed now; everything else stays material.

--retries 0 performed zero fetches
  `range(1, args.retries + 1)` is empty at 0, so the run reported an empty
  page it had never requested (the audit got exit 4 on a 39-byte document
  with no navigation). The loop floors at one attempt in all three engines,
  and argparse now rejects a sub-1 value instead of accepting a confusing one.
  --pages and --concurrency get the same validator.

A blocked run left a sidecar claiming yesterday's success
  `meta.json` still said status=complete, products=1 from the previous run,
  so a consumer reading files rather than exit codes read stale data as
  fresh. §9's rule stands -- a failed run must not write a "failed" sidecar
  next to good data -- so this adds `<out>.attempt.json`, written on EVERY
  run, carrying attempt_status, blocked and data_updated. meta.json answers
  "what is this data?", attempt.json answers "what happened just now?".

The README put a secret in argv while telling readers not to
  Line 53 showed `--key "$TWOCAPTCHA_KEY"`; line 322 said credentials never
  reach argv. The example is gone and the client now warns when --key is
  used anyway.

--dump-html could be committed
  A dump named anything but *_debug.html matched no ignore rule, and the
  secret scanner skipped .html entirely -- while SECURITY.md warns that raw
  dumps carry session state. .gitignore now covers *.html and artifacts/,
  and the scanner reads .html/.json/.csv.

Output files were not written atomically
  A crash or a full disk could truncate the previous good file, which is the
  one thing `save()` exists to protect. JSON, CSV and both sidecars now go
  through a temp file in the same directory, fsync, then os.replace.

Transfermarkt vocabulary survived in shipped files
  My earlier sweep was too narrow: it missed `spieler`, `verein`,
  `hauptlink`, `squad`, `ranking`, and did not look at the Dockerfile at all.
  CONTRIBUTING.md described another site's URL patterns, the issue template
  offered `player` as a mode, the Docker quickstart ran `--mode
  market-values`. All rewritten for this site, plus a new
  test_no_sibling_site_vocabulary() so it cannot come back.

NOT changed, deliberately:
  * No direct HTTP transport and no adaptive fallback. The audit's strongest
    practical point, and it deserves a decision rather than a reflex: it
    changes the repo's architecture and the family's. Its evidence is also
    one sample -- a browser blocked and an HTTP 200 on one day from one
    address; this repo measured the opposite on 2026-09-18, with a local
    browser working from three addresses while the paid exit failed.
  * No catalog runner with a persistent queue. Real gap, real work, and the
    README documents the shell loop honestly meanwhile.
  * No variant-level mode, no src/ layout, no console scripts, no SHA-pinned
    actions. The last three are family-wide and belong in the template, not
    in one repo.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@2ione 2ione closed this Sep 22, 2026
@2ione
2ione deleted the fix/audit-2026-09-18 branch September 22, 2026 08:20
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