From 8d6c690140ff5e54011f2cf01f65b1d6f7b06a7c Mon Sep 17 00:00:00 2001 From: unseensnick <84652498+unseensnick@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:04:40 +0200 Subject: [PATCH 1/7] docs: state the contributor standard where contributors look An outside pull request failed the commit check because nothing a contributor reads said what the standard is. The rules now live in CONTRIBUTING.md and the pull request template, in plain words. - CONTRIBUTING.md covers setup with uv, the test command, the two-engine rule, /v1 compatibility, every commit rule CI enforces, and CHANGELOG scope - The pull request template carries the same checklist - The README links both, and its "From source" steps use uv and Python 3.14 instead of pip and 3.9 - The README's em dashes are gone, so it passes the rule it points to - Adds a short code of conduct --- .github/pull_request_template.md | 18 +++++++ CODE_OF_CONDUCT.md | 11 +++++ CONTRIBUTING.md | 85 ++++++++++++++++++++++++++++++++ README.md | 39 ++++++++------- 4 files changed, 136 insertions(+), 17 deletions(-) create mode 100644 .github/pull_request_template.md create mode 100644 CODE_OF_CONDUCT.md create mode 100644 CONTRIBUTING.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..a3c3a5b --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,18 @@ +## Summary + + + +## How it was tested + + + +## Checklist + +See [CONTRIBUTING.md](../CONTRIBUTING.md) for what each item means. + +- [ ] The browser-free suite passes: `PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src` +- [ ] Commit messages follow `type(scope): summary`, at most 72 characters, with no em dash, no bare `#N`, and no AI attribution +- [ ] No names of target sites and no scraping vocabulary in commits, docs, or code comments +- [ ] A change a client can notice lands for both engines, or the pull request names the capability one engine does not have +- [ ] The `/v1` request and response shape is unchanged, apart from new optional fields +- [ ] `CHANGELOG.md` has an `[Unreleased]` entry if someone running Solverr could notice the change, and none otherwise diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..41e4356 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,11 @@ +# Code of Conduct + +Solverr is a small personal project. The expectations are simple: + +- Be respectful in issues and pull requests, and assume good faith. +- No harassment, hate speech, personal attacks, or sharing anyone's private information. +- Keep it on-topic and constructive. + +The maintainer may edit, remove, or lock any comment, and may block accounts that don't follow this, at their discretion. + +To report a problem, contact [@unseensnick](https://github.com/unseensnick). If it's sensitive, reach out privately rather than opening a public issue. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..ff2282a --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,85 @@ +# Contributing to Solverr + +Solverr is a personal fork, maintained in spare time, so a pull request may sit for a while or come back with changes. Bug reports are always useful. For anything bigger than a small fix, open an issue first so the approach can be agreed on before you put in the work. + +Everyone taking part is expected to follow the [Code of Conduct](CODE_OF_CONDUCT.md). + +## Reporting a bug + +Use the [bug report form](https://github.com/unseensnick/Solverr/issues/new?template=bug_report.yml). Say which image tag you run, which engine was involved, and whether you use a proxy, and attach a log taken with `LOG_LEVEL=debug`. Check the README's Troubleshooting section first: a site that blocks your IP address fails on every solver, and a residential proxy fixes that where no code change can. + +## Setting up + +You need [uv](https://docs.astral.sh/uv/) and Git. Solverr does not use a system Python; everything runs through uv. + +```bash +git clone https://github.com/unseensnick/Solverr.git +cd Solverr +git config core.hooksPath .githooks +uv venv --python 3.14 +uv pip install -r requirements.txt -r test-requirements.txt +``` + +The `core.hooksPath` line turns on the same commit checks CI runs, so a problem shows up when you commit instead of after you push. To run the full service with both browsers, use Docker: `docker compose up -d --build`. + +## Running the tests + +```bash +PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src +``` + +This is the browser-free suite. It takes seconds, and CI runs it on every pull request. It cannot tell you whether a page still clears a real challenge, so if your change touches solving, say in the pull request what you tried against a live site. The maintainer runs a live check before anything touching solving is released. + +## The two engines + +Every request is served by one of two engines: `chrome` (Selenium with undetected-chromedriver, from FlareSolverr) or `stealth` (Camoufox, from Byparr). The rule that keeps them from drifting apart: + +- **A change a client can notice lands for both engines in the same pull request.** That covers the `/v1` response, request parameters, and what a configuration variable does. The only exception is a browser-automation capability one engine genuinely does not have; name it in the pull request. +- **Put a shared rule in the shared code, not in one engine.** `src/assembly.py`, `src/pipeline.py`, `src/budget.py` and `src/sessions.py` hold what both engines must do the same way. +- **Test it once for both.** A behaviour both engines must share gets a test in `src/test_engine_conformance.py`, which runs it against each engine. + +The full rule, with its reasoning, is [.claude/rules/engine-layer.md](.claude/rules/engine-layer.md). + +## Keeping the API compatible + +Solverr is a drop-in replacement for FlareSolverr, so clients built for FlareSolverr must keep working: + +- Add optional fields only. Never rename, retype, or remove a field in the request or the response. +- Keep the `"FlareSolverr is ready!"` banner exactly as it is. Clients detect session support by it. +- Only `http://` and `https://` URLs may reach a browser. + +## Commit messages + +CI checks every commit in a pull request with [.githooks/commit-msg](.githooks/commit-msg), so these are hard requirements, not style advice: + +- **The subject is `type(scope): summary`.** The type is one of `feat`, `fix`, `docs`, `chore`, `refactor`, `test`, `perf`, `build`, `ci`, `style`, `revert`. The scope is optional and names the area (`chrome`, `stealth`, `sessions`, `api`, `docker`). Write the summary in the imperative, in lower case, with no trailing period. +- **The subject is at most 72 characters.** +- **No em dash anywhere in the message.** Use commas, parentheses, periods, or colons. +- **No bare `#123`.** It silently links to an issue in this repository. Write `owner/repo#123` instead, for example `FlareSolverr/FlareSolverr#1626`. +- **No AI attribution.** No `Co-authored-by` trailer naming an AI tool and no "Generated with" footer. A `Co-authored-by` trailer for a person is fine. +- **No names of the sites you point Solverr at, and no scraping vocabulary.** Write `example-site.tld` or "a Cloudflare-gated site" instead. +- **A change that is more than a one-liner gets a body.** Lead with one or two plain sentences on what changed and why it matters, then bullets. + +For example, `Fixed the cookie bug.` is rejected, and `fix(stealth): accept a cookie without a domain` passes. + +If a commit is rejected, reword it with `git commit --amend`, or `git rebase -i` for an older one. If you would rather not, say so in the pull request: the maintainer can reword it when merging, and your name stays on the commit. + +## The CHANGELOG + +Add a bullet to `CHANGELOG.md` under `## [Unreleased]` only when someone running Solverr, or calling its API, could notice the change. Put it under `Additions`, `Changes`, or `Fixes`, and lead with a bold headline that says what the user gets and ends in a period. The bold headline is the entire release note, so anything a deployer must act on (a new variable, a changed default) goes inside it. Tests, CI, documentation and tooling changes get no entry. If you are unsure, leave it out and the maintainer will add it. + +## Upstream code + +Solverr still takes changes from both of its upstreams, [FlareSolverr](https://github.com/FlareSolverr/FlareSolverr) and [Byparr](https://github.com/ThePhaseless/Byparr). [docs/dev/upstream-sync.md](docs/dev/upstream-sync.md) records what has been taken and what is deliberately different; read it before porting something or calling a difference a bug. Some files are kept byte-identical to FlareSolverr so they stay mergeable, and should not be edited: `src/undetected_chromedriver/`, `src/tests.py`, `src/tests_sites.py`, `src/bottle_plugins/`, and `html_samples/`. + +## How pull requests are merged + +The maintainer merges with a merge commit, so your commits land on `main` as you wrote them, under your name. + +## Claude Code configuration + +`CLAUDE.md` and `.claude/` configure [Claude Code](https://claude.com/claude-code) for this repository. You do not need them to contribute; they hold the same rules as this file, in more detail. + +## License + +Solverr is licensed under the [GNU General Public License v3.0](LICENSE), and contributions are accepted under the same license. diff --git a/README.md b/README.md index 06e6b74..2ada38e 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,8 @@ Solverr is a proxy server to bypass Cloudflare and DDoS-GUARD protection. It fuses the two best open-source solvers into one service and switches between them automatically, so you get reliable solving **and** coverage of the newer challenge tiers. -- **Chrome engine** (default) — the original [FlareSolverr](https://github.com/FlareSolverr/FlareSolverr) approach: [Selenium](https://www.selenium.dev) + [undetected-chromedriver](https://github.com/ultrafunkamsterdam/undetected-chromedriver) driving a real Chromium. Fast, session-capable, and clears most sites. -- **Stealth engine** — [Byparr](https://github.com/ThePhaseless/Byparr)'s stack: [Camoufox](https://github.com/daijro/camoufox) (an anti-detect Firefox that patches its fingerprint in compiled code) + [playwright-captcha](https://github.com/techinz/playwright-captcha). Clears the newer Cloudflare **Turnstile / Managed Challenges** that headless Chromium gives up on. +- **Chrome engine** (default): the original [FlareSolverr](https://github.com/FlareSolverr/FlareSolverr) approach: [Selenium](https://www.selenium.dev) + [undetected-chromedriver](https://github.com/ultrafunkamsterdam/undetected-chromedriver) driving a real Chromium. Fast, session-capable, and clears most sites. +- **Stealth engine**: [Byparr](https://github.com/ThePhaseless/Byparr)'s stack: [Camoufox](https://github.com/daijro/camoufox) (an anti-detect Firefox that patches its fingerprint in compiled code) + [playwright-captcha](https://github.com/techinz/playwright-captcha). Clears the newer Cloudflare **Turnstile / Managed Challenges** that headless Chromium gives up on. It speaks the exact FlareSolverr `/v1` API on port `8191`, so it is a drop-in replacement: existing clients (the *arr stack, manga/novel readers, etc.) work unchanged. @@ -13,9 +13,9 @@ Beyond the two engines, it keeps **[sessions](#sessions--automatic-cleanup)** wa ## Contents -- **Getting started** — [How it works](#how-it-works) · [Quick start](#quick-start) · [Installation](#installation) -- **Using it** — [Engines & fallback](#engines--fallback) · [Sessions & cleanup](#sessions--automatic-cleanup) · [API usage](#api-usage) · [Passthrough proxy](#passthrough-proxy) -- **Reference** — [Configuration](#configuration) · [Proxy & reliability](#proxy--reliability) · [Prometheus exporter](#prometheus-exporter) · [Troubleshooting](#troubleshooting) +- **Getting started**: [How it works](#how-it-works) · [Quick start](#quick-start) · [Installation](#installation) +- **Using it**: [Engines & fallback](#engines--fallback) · [Sessions & cleanup](#sessions--automatic-cleanup) · [API usage](#api-usage) · [Passthrough proxy](#passthrough-proxy) +- **Reference**: [Configuration](#configuration) · [Proxy & reliability](#proxy--reliability) · [Prometheus exporter](#prometheus-exporter) · [Troubleshooting](#troubleshooting) ## How it works @@ -85,18 +85,19 @@ On a Debian **host**, make sure `libseccomp2` is 2.5.x (`sudo apt-cache policy l ### From source -For development or unsupported architectures. Requires Python 3.9+ (3.11+ recommended for the vendored undetected-chromedriver; the Docker image uses 3.14), and both browsers if you want both engines: +For development or unsupported architectures. Needs [uv](https://docs.astral.sh/uv/) and Python 3.14 (the version the image runs), plus both browsers if you want both engines: ```bash -# install Python deps (pip, or `uv pip`) -pip install -r requirements.txt +# create the environment and install Python deps +uv venv --python 3.14 +uv pip install -r requirements.txt # Chrome engine: install Chrome or Chromium (+ Xvfb on Linux) # Stealth engine: install Firefox libraries and fetch Camoufox -playwright install-deps firefox -python -m invisible_playwright fetch +uv run --no-project playwright install-deps firefox +uv run --no-project python -m invisible_playwright fetch -python src/flaresolverr.py +uv run --no-project python src/flaresolverr.py ``` Set `STEALTH_ENGINE=false` to run Chrome-only and skip the Camoufox/Firefox setup entirely. @@ -126,7 +127,7 @@ Clients often create a session and never destroy it (a mobile app can be killed - closes any session idle longer than `SESSION_TTL_MINUTES` (default 30). Every request bumps the session's last-used time, so an in-use session is never reaped. - evicts the oldest-idle session once an engine exceeds `SESSION_MAX` (default 20). -So `sessions.destroy` is good practice but optional — cleanup happens automatically. +So `sessions.destroy` is good practice but optional: cleanup happens automatically. ## API usage @@ -232,7 +233,7 @@ Like `request.get`, plus `postData`. ## Passthrough proxy -Some clients don't consume the solved HTML that `/v1` returns. Instead they take the `cf_clearance` cookie and **re-fetch the URL themselves** with their own HTTP client. Cloudflare fingerprints that second request (different TLS/JA4, HTTP/2 settings, headers) than the browser that solved the challenge, decides it doesn't match, and re-challenges — so the client fails even though the solve worked. Indexer managers that drive Cloudflare-protected sites are the common case. +Some clients don't consume the solved HTML that `/v1` returns. Instead they take the `cf_clearance` cookie and **re-fetch the URL themselves** with their own HTTP client. Cloudflare fingerprints that second request (different TLS/JA4, HTTP/2 settings, headers) than the browser that solved the challenge, decides it doesn't match, and re-challenges, so the client fails even though the solve worked. Indexer managers that drive Cloudflare-protected sites are the common case. The passthrough removes the replay step. Point the client at Solverr's passthrough port instead of the site; Solverr solves in-process (reusing engine fallback, sessions, and per-host memory) and returns the solved body as a clean `200`. The client never sees a challenge, so it never re-fetches. @@ -276,14 +277,14 @@ You don't need a bundled indexer file. Take the site's existing definition from - **Prowlarr**: `/config/Definitions/Custom/`. The `Custom` subfolder often doesn't exist yet, and Prowlarr **ignores** YAMLs placed directly in `Definitions/`, so create `Custom/` and put the file there. - **Jackett**: its custom-definitions folder, which Jackett prints in its startup log (commonly `/config/Jackett/Indexers/custom/` on the linuxserver image); create it if missing. -Then add the indexer in the manager, pick a mirror as the **Base URL**, and **do not attach a FlareSolverr/proxy tag** — the passthrough already does the solving, and a proxy tag would route around it. Everything else in the definition (search paths, selectors, categories) stays untouched. +Then add the indexer in the manager, pick a mirror as the **Base URL**, and **do not attach a FlareSolverr/proxy tag**: the passthrough already does the solving, and a proxy tag would route around it. Everything else in the definition (search paths, selectors, categories) stays untouched. > **Grab the definition as a file, not via copy-paste.** A few definitions contain non-printable characters in their filters (a rare title-cleanup step); pasting through a chat or some editors silently strips them and breaks parsing ("No title provided" on every result). Download the raw file so the bytes stay intact. Notes and limits: - **`GET`/`HEAD` only**; request bodies aren't forwarded. Most indexer definitions are `GET`. -- Encode the mirror as a **bare host** (`example-site.tld`), not `https://…` — clients that normalise `//` in a path would otherwise corrupt an embedded scheme. +- Encode the mirror as a **bare host** (`example-site.tld`), not `https://…`, because clients that normalise `//` in a path would otherwise corrupt an embedded scheme. - Successful bodies are cached for `PASSTHROUGH_CACHE_TTL`; challenge pages and non-2xx responses are not, so a transient block retries rather than sticking. - The cache holds at most `PASSTHROUGH_CACHE_MAX_BYTES` in total. The TTL alone bounded how long a body was kept but not how much was kept, so a client walking many pages inside one TTL window could hold all of them at once. - It's still bound by IP reputation like any solve (see [Proxy & reliability](#proxy--reliability)). If a site blocks your IP, a residential `PROXY_URL` applies to passthrough solves too. @@ -404,7 +405,7 @@ If the exit IP can't be reached, Solverr falls back to the container's `TZ` for ## Proxy & reliability -No solver beats Cloudflare by fingerprint alone — **IP reputation dominates**. A datacenter/VPS IP fails far more challenges than a residential one. If a site keeps failing on **both** engines, the single most effective fix is a residential proxy: set `PROXY_URL` (and credentials), or pass `proxy` per request/session. +No solver beats Cloudflare by fingerprint alone: **IP reputation dominates**. A datacenter/VPS IP fails far more challenges than a residential one. If a site keeps failing on **both** engines, the single most effective fix is a residential proxy: set `PROXY_URL` (and credentials), or pass `proxy` per request/session. Rough guide to expected latency: Chrome solves take a few seconds; Camoufox solves take ~10–20 s (the price of clearing challenges Chromium can't). Session reuse brings follow-ups on the same host down to ~1–3 s. @@ -416,7 +417,7 @@ The domain label is capped at 100 distinct hosts; every host after that is repor ## Troubleshooting -**A source shows no results but the log says `Challenge not detected!` with a 200.** An engine loaded the page but couldn't recognise a newer managed/Turnstile challenge and returned it as if solved. Solverr's auto-fallback is designed to catch this and retry on the other engine; make sure `ENGINE_FALLBACK` is on and the stealth engine is enabled. If it still fails, the site is likely gating on your IP — add a residential proxy. +**A source shows no results but the log says `Challenge not detected!` with a 200.** An engine loaded the page but couldn't recognise a newer managed/Turnstile challenge and returned it as if solved. Solverr's auto-fallback is designed to catch this and retry on the other engine; make sure `ENGINE_FALLBACK` is on and the stealth engine is enabled. If it still fails, the site is likely gating on your IP, so add a residential proxy. **Out-of-memory / browser launch errors (Proxmox LXC, low-RAM hosts).** Give the container more shared memory: `shm_size: 512mb` in `docker-compose.yml` (or `--shm-size=512m`). Reduce `SESSION_MAX` and keep `SESSION_TTL_MINUTES` modest so idle browsers are freed sooner. @@ -424,6 +425,10 @@ The domain label is capped at 100 distinct hosts; every host after that is repor **Cloudflare has blocked this request / IP banned.** Your IP is flagged for that site. Try a (residential) proxy, or open the site in a normal browser from the same network to confirm. +## Contributing + +Bug reports and pull requests are welcome. Read [CONTRIBUTING.md](CONTRIBUTING.md) before opening a pull request: it covers the setup, the tests, and the commit message standard that CI checks every commit against. Everyone taking part is expected to follow the [Code of Conduct](CODE_OF_CONDUCT.md). + ## License Solverr is licensed under the **GNU General Public License v3.0** (see [LICENSE](LICENSE)). It began as a fork of [FlareSolverr](https://github.com/FlareSolverr/FlareSolverr) (MIT) and its stealth engine derives from [Byparr](https://github.com/ThePhaseless/Byparr) (GPL-3.0); because Byparr is copyleft, the combined work is GPL-3.0. Upstream copyright notices are preserved in [NOTICE](NOTICE). From 1e76b6a451a3afdaa47290de59dc9a0bed564e39 Mon Sep 17 00:00:00 2001 From: unseensnick <84652498+unseensnick@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:09:29 +0200 Subject: [PATCH 2/7] ci: run the tests on every pull request and test the hooks Nothing ran the test suite on a pull request, and nothing proved the commit hooks still reject what they claim to. Both now run in CI, and a person can be credited as a co-author. - A Tests workflow runs the browser-free suite on pull requests and pushes to main, on Python 3.14 through uv - .githooks/tests/run.sh asserts a failing and a passing case for every hook rule, and the Standards job runs it before trusting the hooks - commit-msg rejects only AI co-author trailers, so a human one passes - pre-commit also lints CONTRIBUTING.md and CLAUDE.md, both of which the naming rule already covers - Every action is pinned to a commit, and Renovate keeps the pins current - .gitattributes stores and checks out text as LF, since the hooks break under bash with CRLF; .editorconfig matches it --- .editorconfig | 15 ++++ .gitattributes | 14 ++++ .githooks/commit-msg | 7 +- .githooks/pre-commit | 4 +- .githooks/tests/run.sh | 103 +++++++++++++++++++++++++++ .github/workflows/release-docker.yml | 12 ++-- .github/workflows/release.yml | 6 +- .github/workflows/standards.yml | 7 +- .github/workflows/tests.yml | 32 +++++++++ renovate.json | 2 +- 10 files changed, 186 insertions(+), 16 deletions(-) create mode 100644 .editorconfig create mode 100644 .gitattributes create mode 100644 .githooks/tests/run.sh create mode 100644 .github/workflows/tests.yml diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..20d668b --- /dev/null +++ b/.editorconfig @@ -0,0 +1,15 @@ +root = true + +[*] +charset = utf-8 +end_of_line = lf +indent_size = 2 +indent_style = space +insert_final_newline = true +trim_trailing_whitespace = true + +[*.py] +indent_size = 4 + +[*.md] +trim_trailing_whitespace = false diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..3aaf683 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,14 @@ +# Text is stored and checked out with LF on every platform. The git hooks and the +# .claude/hooks scripts run under bash, which breaks on CRLF, and both upstreams are LF, so a +# CRLF working copy made every line differ when diffing against them. +* text=auto eol=lf + +*.png binary +*.jpg binary +*.gif binary +*.ico binary +*.webp binary +*.pyc binary +*.gz binary +*.zip binary +*.mmdb binary diff --git a/.githooks/commit-msg b/.githooks/commit-msg index fca42b2..db966b3 100644 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -22,9 +22,10 @@ echo "$subject" | grep -qE '^(feat|fix|docs|chore|refactor|test|perf|build|ci|st # 3) No em dash anywhere in the message echo "$msg" | grep -q '—' && errors+=("contains an em dash; use commas, parentheses, periods, or colons") -# 4) No AI watermark -echo "$msg" | grep -qiE 'co-authored-by|generated with|claude\.ai/code|🤖' \ - && errors+=("contains an AI watermark (Co-Authored-By / Generated with / robot emoji)") +# 4) No AI watermark. A Co-authored-by trailer naming a person is credit and passes; one +# naming an AI tool is a watermark. +echo "$msg" | grep -qiE 'co-authored-by:.*(claude|anthropic|openai|chatgpt|copilot|gemini|cursoragent)|generated with|claude\.ai/code|🤖' \ + && errors+=("contains an AI watermark (an AI Co-authored-by trailer / Generated with / robot emoji)") # 5) No BARE '#' reference: it links to an issue in THIS repo, which is rarely what was meant. # Strip explicit owner/repo#N first (FlareSolverr/FlareSolverr#1626), then flag what remains. diff --git a/.githooks/pre-commit b/.githooks/pre-commit index cf5df43..81dc0ee 100644 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -1,6 +1,6 @@ #!/usr/bin/env bash # Solverr docs enforcement (see .claude/rules/workflow.md: "CHANGELOG" and "Public-facing naming"). -# Lints staged CHANGELOG.md and README.md: +# Lints staged CHANGELOG.md, README.md, CONTRIBUTING.md and CLAUDE.md: # both - no target-site names or scraping vocabulary in newly added lines, no em dash. # CHANGELOG - every new [Unreleased] entry under Additions / Changes / Fixes leads with a # self-contained bold headline ending in . ! or ? (Other is exempt). @@ -29,7 +29,7 @@ name_patterns='(^|[^A-Za-z0-9./@-])[a-z0-9][a-z0-9-]{1,}\.(to|st|se|nl|cc|ws|eu| local_deny="$(dirname "$0")/deny-names.local" [ -f "$local_deny" ] && name_patterns="$name_patterns|$(paste -sd'|' "$local_deny")" -for f in CHANGELOG.md README.md; do +for f in CHANGELOG.md README.md CONTRIBUTING.md CLAUDE.md; do staged "$f" || continue new=$(added "$f") [ -n "$new" ] || continue diff --git a/.githooks/tests/run.sh b/.githooks/tests/run.sh new file mode 100644 index 0000000..11692b9 --- /dev/null +++ b/.githooks/tests/run.sh @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +# Checks that the git hooks reject what they claim to reject, and pass what they must pass. +# +# Both hooks are regexes, and a regex that silently stops matching looks exactly like a clean +# commit. Every rule below therefore has a case that must fail as well as one that must pass. +# CI runs this before it runs the hooks themselves (.github/workflows/standards.yml). +# +# Run it after touching either hook: bash .githooks/tests/run.sh +set -u + +hooks="$(cd "$(dirname "$0")/.." && pwd)" +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT +pass=0 +broke=0 +em=$(printf '\xe2\x80\x94') + +# check +check() { + local name="$1" want="$2" + shift 2 + local got=0 + "$@" > /dev/null 2>&1 || got=1 + if [ "$got" = "$want" ]; then + echo " ok $name" + pass=$((pass + 1)) + else + echo " BROKE $name (wanted exit $want, got $got)" + broke=$((broke + 1)) + fi +} + +# msg +msg() { + printf '%b' "$3" > "$work/msg" + check "$2" "$1" bash "$hooks/commit-msg" "$work/msg" +} + +echo "commit-msg" +msg 0 "passes a conventional subject" 'fix(stealth): accept a cookie without a domain\n' +msg 0 "passes a subject with no scope" 'docs: explain the passthrough timeout\n' +msg 1 "rejects a subject with no type" 'Fixed the cookie bug.\n' +msg 1 "rejects an unknown type" 'update(api): tweak things\n' +msg 1 "rejects a subject over 72 characters" 'fix(api): this subject keeps going well past the seventy-two character limit\n' +msg 1 "rejects an em dash in the body" "fix(api): accept headers\n\nIt was refused ${em} now it is not.\n" +msg 1 "rejects an AI co-author trailer" 'fix(api): accept headers\n\nCo-Authored-By: Claude \n' +msg 1 "rejects a Copilot co-author trailer" 'fix(api): accept headers\n\nCo-authored-by: Copilot \n' +msg 1 "rejects a generated-with footer" 'fix(api): accept headers\n\nGenerated with a tool\n' +msg 0 "passes a human co-author trailer" 'fix(api): accept headers\n\nCo-authored-by: Jane Doe \n' +msg 1 "rejects a bare issue number" 'fix(api): accept headers\n\nSee #12.\n' +msg 0 "passes the owner/repo issue form" 'fix(chrome): port the focus fix\n\nFrom FlareSolverr/FlareSolverr#1626.\n' +msg 1 "rejects a squash-merge (#N) suffix" 'fix(api): accept headers (#11)\n' +msg 1 "rejects a domain-shaped site name" 'fix: clears example.to again\n' +msg 1 "rejects scraping vocabulary" 'feat: add a scraper mode\n' +msg 0 "lets a merge commit through" 'Merge pull request #11 from someone/branch\n\nfix(api): accept headers\n' + +# The pre-commit hook reads the index (or a commit range), so each case runs in a scratch repo. +repo="$work/repo" +g() { git -C "$repo" -c core.hooksPath=/nonexistent -c user.name=t -c user.email=t@example.com -c core.autocrlf=false "$@"; } +reset_repo() { + rm -rf "$repo" + mkdir -p "$repo" + g init -q + printf '# Changelog\n\n## [Unreleased]\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md" + printf '# Readme\n' > "$repo/README.md" + printf '# Contributing\n' > "$repo/CONTRIBUTING.md" + g add -A + g commit -q -m "chore: baseline" +} +# stage +stage() { + printf '%b' "$2" >> "$repo/$1" + g add "$1" +} +pre() { (cd "$repo" && bash "$hooks/pre-commit" "$@"); } + +echo "pre-commit" +reset_repo; stage README.md 'A plain new line.\n' +check "passes a clean README line" 0 pre +reset_repo; stage README.md "A line ${em} with an em dash.\n" +check "rejects an em dash in the README" 1 pre +reset_repo; stage CONTRIBUTING.md "A line ${em} with an em dash.\n" +check "rejects an em dash in CONTRIBUTING" 1 pre +reset_repo; stage README.md 'Point it at example.to for testing.\n' +check "rejects a site name in the README" 1 pre +reset_repo +printf '# Changelog\n\n## [Unreleased]\n\n### Fixes\n\n- **A real headline.** Detail.\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md"; g add CHANGELOG.md +check "passes a bold CHANGELOG headline" 0 pre +reset_repo +printf '# Changelog\n\n## [Unreleased]\n\n### Fixes\n\n- No bold headline here.\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md"; g add CHANGELOG.md +check "rejects an entry with no headline" 1 pre +reset_repo +printf '# Changelog\n\n## [Unreleased]\n\n### Fixes\n\n- **A headline with no stop**\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md"; g add CHANGELOG.md +check "rejects a headline with no full stop" 1 pre +reset_repo +printf '# Changelog\n\n## [Unreleased]\n\n### Other\n\n- Bumped a dependency.\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md"; g add CHANGELOG.md +check "exempts the Other section" 0 pre +reset_repo; stage README.md "A line ${em} in a commit.\n"; g commit -q -m "docs: add a line" +check "range mode rejects a committed em dash" 1 pre HEAD~1..HEAD + +echo "" +echo "$pass passed, $broke broke" +[ "$broke" -eq 0 ] diff --git a/.github/workflows/release-docker.yml b/.github/workflows/release-docker.yml index ec48f36..223f21b 100644 --- a/.github/workflows/release-docker.yml +++ b/.github/workflows/release-docker.yml @@ -21,11 +21,11 @@ jobs: contents: read packages: write steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Docker metadata id: meta - uses: docker/metadata-action@v6 + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 with: # metadata-action lowercases the image name (ghcr requires lowercase). images: ghcr.io/${{ github.repository }} @@ -36,18 +36,18 @@ jobs: # Move :latest only when building a version tag. flavor: latest=${{ github.ref_type == 'tag' }} - - uses: docker/setup-qemu-action@v4 - - uses: docker/setup-buildx-action@v4 + - uses: docker/setup-qemu-action@1f40c72289eff860ee54a304f1438e3cff362e0a # v4.3.0 + - uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0 - name: Log in to ghcr.io - uses: docker/login-action@v4 + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build and push - uses: docker/build-push-action@v7 + uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 with: context: . # arm64 is built under QEMU emulation and is slow for this large diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5de08df..3324206 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -33,7 +33,7 @@ jobs: name: Create release runs-on: ubuntu-latest steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 @@ -47,7 +47,7 @@ jobs: fi - name: Install parse-changelog - uses: taiki-e/install-action@parse-changelog + uses: taiki-e/install-action@ffe3fd350e607ed6df4dd1765629bcc24853038a # parse-changelog - name: Prepare release body env: @@ -79,7 +79,7 @@ jobs: } > release_body.md - name: Create release - uses: softprops/action-gh-release@v3 + uses: softprops/action-gh-release@efb35369e0ad2afab669f228072c1b0d510eae64 # v3.0.3 with: tag_name: ${{ steps.vars.outputs.tag }} name: Solverr ${{ steps.vars.outputs.tag }} diff --git a/.github/workflows/standards.yml b/.github/workflows/standards.yml index c302942..c928752 100644 --- a/.github/workflows/standards.yml +++ b/.github/workflows/standards.yml @@ -18,7 +18,7 @@ jobs: name: Commit messages and docs runs-on: ubuntu-latest steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 @@ -49,6 +49,11 @@ jobs: echo "head=$head" >> "$GITHUB_OUTPUT" echo "Checking ${base}..${head}" + # The hooks are regexes, and one that silently stops matching looks exactly like a clean + # commit, so prove they still reject what they should before trusting a pass from them. + - name: Hook self-test + run: bash .githooks/tests/run.sh + - name: Commit messages env: BASE: ${{ steps.range.outputs.base }} diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..d9ca6f2 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,32 @@ +# Runs the browser-free suite on every pull request and every push to main. It needs no browser +# and takes seconds. Whether a page still clears a real challenge is outside what it can see; that +# is the live check's job before a release. +name: Tests + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +permissions: + contents: read + +jobs: + tests: + name: Browser-free suite + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 + with: + python-version: "3.14" + + - name: Install dependencies + run: | + uv venv + uv pip install -r requirements.txt -r test-requirements.txt + + - name: Run the suite + run: PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src diff --git a/renovate.json b/renovate.json index 5aab414..5450289 100644 --- a/renovate.json +++ b/renovate.json @@ -1,6 +1,6 @@ { "$schema": "https://docs.renovatebot.com/renovate-schema.json", - "extends": ["config:recommended"], + "extends": ["config:recommended", "helpers:pinGitHubActionDigests"], "schedule": ["before 6am on monday"], "labels": ["dependencies"], "dependencyDashboard": false, From 7584d797cdb09ba33a1b8bcce3de4fec0fdd3128 Mon Sep 17 00:00:00 2001 From: unseensnick <84652498+unseensnick@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:20:30 +0200 Subject: [PATCH 3/7] chore(claude): harden the command guard and fit the agents to Solverr Claude could merge a pull request and read secret files through the shell, and the review agents still described a web app. The guard now covers both, and the agents review the code this repository actually has. - The command guard blocks gh pr merge and API merges, since a person merges every PR, and blocks shell reads of .env, keys, secrets/ and the local deny list, which the Read deny rule never covered - A safe /dev/null redirect no longer trips the raw-device check, and no longer hides a dangerous redirect later in the same command - A bare git push is judged by the session's own branch, so a push from a loop worktree is not read against the main tree - Eight new hook cases cover each change, all seen failing first - Settings narrow gh pr to reads plus draft PRs, deny history-destroying git commands, ask before force-with-lease pushes, history rewrites, public comments, release writes and write-method API calls, allow read-only git in PowerShell, and wire notifications to notify.sh, which reaches Windows - The four review agents are rewritten for Solverr's stack, rules and untrusted-input boundaries - Removes two hooks that were never wired and gave advice for another stack, and an example file of template text --- .claude/agents/code-reviewer.md | 77 +++++---- .claude/agents/doc-reviewer.md | 69 ++++---- .claude/agents/performance-reviewer.md | 80 ++++------ .claude/agents/security-reviewer.md | 86 ++++------ .claude/hooks/auto-test.sh | 148 ------------------ .claude/hooks/block-dangerous-commands.sh | 37 ++++- .claude/hooks/context-recovery.sh | 111 ------------- .../31-block-gh-pr-merge.json | 6 + .../32-block-gh-api-merge.json | 6 + .../33-allow-gh-pr-view.json | 5 + .../34-block-cat-env.json | 6 + .../35-block-ps-read-deny-names.json | 6 + .../36-allow-reader-words-in-prose.json | 5 + .../37-allow-devnull-then-semicolon.json | 5 + ...-block-raw-device-after-safe-redirect.json | 6 + .claude/settings.json | 55 ++++++- .dockerignore | 1 - CLAUDE.local.md.example | 26 --- 18 files changed, 272 insertions(+), 463 deletions(-) delete mode 100644 .claude/hooks/auto-test.sh delete mode 100644 .claude/hooks/context-recovery.sh create mode 100644 .claude/hooks/tests/fixtures/block-dangerous-commands/31-block-gh-pr-merge.json create mode 100644 .claude/hooks/tests/fixtures/block-dangerous-commands/32-block-gh-api-merge.json create mode 100644 .claude/hooks/tests/fixtures/block-dangerous-commands/33-allow-gh-pr-view.json create mode 100644 .claude/hooks/tests/fixtures/block-dangerous-commands/34-block-cat-env.json create mode 100644 .claude/hooks/tests/fixtures/block-dangerous-commands/35-block-ps-read-deny-names.json create mode 100644 .claude/hooks/tests/fixtures/block-dangerous-commands/36-allow-reader-words-in-prose.json create mode 100644 .claude/hooks/tests/fixtures/block-dangerous-commands/37-allow-devnull-then-semicolon.json create mode 100644 .claude/hooks/tests/fixtures/block-dangerous-commands/38-block-raw-device-after-safe-redirect.json delete mode 100644 CLAUDE.local.md.example diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index c316c6a..405a52f 100644 --- a/.claude/agents/code-reviewer.md +++ b/.claude/agents/code-reviewer.md @@ -1,6 +1,6 @@ --- name: code-reviewer -description: Reviews code for quality, correctness, and maintainability. Use for diff review, PR review, or post-change verification. +description: Reviews Solverr's Python changes for correctness, stealth event-loop safety, the engine-layer law, and /v1 compatibility. Use for diff review, PR review, or post-change verification. tools: - Read - Grep @@ -8,63 +8,74 @@ tools: - Bash --- -You are a thorough code reviewer focused on catching real issues, not style nitpicks. +You review Python changes in Solverr, a FlareSolverr fork: Python 3.14, `bottle` + `waitress` (synchronous WSGI), the FlareSolverr `/v1` API on 8191 and an optional passthrough proxy on 8888. Two engines sit behind `src/engines/base.py`: `chrome` (Selenium + the vendored undetected_chromedriver) and `stealth` (Camoufox via invisible_playwright, run on ONE background asyncio loop thread in `src/async_runtime.py`). The shared spine is `src/assembly.py`, `src/pipeline.py`, `src/budget.py` and `src/sessions.py`; the controller is `src/flaresolverr_service.py`. Catch real issues, not style nitpicks. + +The repo's own rules are the baseline, so read the relevant one before flagging against it: `CLAUDE.md`, `.claude/rules/engine-layer.md` (the law for anything touching an engine), `code-quality.md`, `error-handling.md`, `testing.md`, and `docs/dev/upstream-sync.md` for what is deliberately different. ## Operating principles - State assumptions explicitly. If multiple readings of the code are possible, surface them. Don't pick silently. -- Surgical scope. Only flag lines that changed or directly relate. Ignore pre-existing issues outside. +- Surgical scope. Only flag lines that changed or directly relate. Ignore pre-existing issues outside, including upstream-inherited code the diff didn't touch. - Verify before flagging. Cite file:line. If you can't verify, say so. - Confidence threshold. Only ship findings you're at least 80% sure are real. Drop the rest. ## How to review -Run `git diff --name-only` for changed files. Read each, grep for related patterns. Report only concrete problems with evidence. +Run `git diff --name-only` for changed files. Read each, grep for related patterns. When the diff touches one engine, open the other engine's file too: the usual defect here is the half that did not land. Report only concrete problems with evidence. -## Correctness +## Python correctness -**Off-by-one**: `array[array.length]` vs `array.length - 1`. `i <= n` vs `i < n`. Inclusive vs exclusive ranges. Fence-post errors (n items need n-1 separators). +- **Mutable defaults**: `def f(x=[])` or `={}` shared across calls. The `None` class attributes on `V1RequestBase` are fine. +- **bool is an int**: `isinstance(True, int)` holds, so an int check that does not exclude `bool` lets `true` through as 1. `validate_request_types` (`src/dtos.py`) and `_validate_max_timeout` show the shape. +- **Truthiness on request input**: `"false"` is truthy, and `0` can be a real value. `validate_request_types` types declared `/v1` fields once; code reading anything it skips (`_TYPE_OVERRIDES`) checks the type itself. +- **Clocks**: `budget.solve_deadline` takes `time.monotonic()` on a request thread and the loop's clock on the stealth engine. Mixing the two, or a deadline on `datetime.now()`, is a finding. +- **Shared module state** written from waitress threads without a lock. `_DOMAIN_ENGINE` is guarded by `_DOMAIN_LOCK`; follow that. -**Null/undefined**: properties on possibly-null values, missing optional chaining, array methods on possibly-undefined arrays, destructuring from possibly-null objects. +## The stealth event loop -**Logic**: inverted conditions, short-circuit skipping side effects, `==` vs `===` (JS/TS), mutation of shared references, missing `break` in switch (unless intentional and commented). +- A coroutine called without `await`, or `asyncio.create_task` / `ensure_future` whose result nobody holds or awaits. `error-handling.md` requires every coroutine on the loop to be awaited or scheduled through `async_runtime`; a floating task drops its exception. +- `AsyncRuntime.run` called from code already on the loop thread. It blocks on `future.result()`, so the loop waits on itself until the timeout. +- A `StealthContext` browser, context or page touched from a request thread. Its docstring says it is only ever touched from the loop. +- A blocking call inside a coroutine (`time.sleep`, a sync network lookup, file I/O). Move it off the loop the way `StealthContext.start` sends `geo.browser_identity` through `asyncio.to_thread`. +- A bare `except:` or `except BaseException` in a coroutine. It swallows `CancelledError`, which is how `asyncio.wait_for` and `AsyncRuntime.run`'s timeout stop a solve. -**Race conditions**: shared mutable state in async callbacks, read-then-write without atomicity, awaits depending on the same mutable variable, event handlers registered without cleanup. +## The engine-layer law -## Error handling +`.claude/rules/engine-layer.md` binds every engine change. Flag: -- Swallowed errors: `catch (e) {}` or `catch (e) { return null }`. -- Missing `.catch()` on promise chains. -- Wrapped errors that lose context: `throw new Error("failed")` discards the original. -- Try/catch too broad, catching errors from unrelated code. -- Missing cases: 404? File not found? Parse error? +- A client-observable change that lands for one engine only. The only exit is a named browser-automation mechanism the other engine cannot provide, cited in the commit and recorded in `docs/dev/upstream-sync.md`. "Structured differently" and "needs a rewrite first" are not exits. +- A per-engine branch, nullable field, or boolean-flag combination inside the shared spine. Divergence is a typed capability slot. +- A capability that silently does nothing on one engine instead of being routed or refused by name (`tabs_till_verify` on stealth is the rule's own example). +- Shared storage that each engine interprets its own way; spine code reimplementing a clearing core; a re-proposed `SessionRef`. +- A second per-engine test where one test in `src/test_engine_conformance.py` (driven by `HARNESSES` in `src/engine_fakes.py`) would pin both. A new test whose PR does not say it was seen red with its production clause deleted has not been verified by mutation. -## Naming +## /v1 compatibility -- Names that lie: `isValid` returning a string, `getUser` that creates. -- Generic where a specific name exists: `data`, `result`, `temp`, `item`. -- Booleans missing `is` / `has` / `should` prefix. -- Abbreviations that obscure: `usr`, `mgr`, `ctx`. +- A removed, renamed or retyped request or response field. Only additive optional fields are allowed (`workflow.md`, "Fork compatibility"). +- An optional response field now emitted as `null` instead of omitted (`_to_challenge_resolution`). +- A changed error message clients or the fallback match on (`"Error solving the challenge. ..."`, `"session ... not found"`), or an error leaving the FlareSolverr shape (`status: "error"`, HTTP 500). +- Refusing unknown parameters: `validate_request_types` logs and keeps them on purpose. +- Any change to the `"FlareSolverr is ready!"` banner (`index_endpoint`). Clients detect session support by it. -## Complexity +## Error handling -- Functions over ~30 lines. -- Nesting deeper than 3 levels (early returns flatten). -- More than 3 parameters (use options object). -- God functions doing read, validate, transform, persist, and notify. +- `except Exception: pass` or a silent `return None` on a teardown or reaper path. `error-handling.md` requires `logging.debug(..., exc_info=True)` there, as `SessionStore._teardown` does. +- A raw traceback reaching the response body. +- Failure handled by silencing an engine instead of at the controller's fallback in `_resolve_challenge`. +- A browser, context or temp dir that leaks when a launch fails partway. Cleanup belongs in a `finally`, as in `StealthEngine.solve` and `utils.get_webdriver`. ## Tests -- Changed behavior without a corresponding test change. -- Tests asserting implementation (mock call counts) instead of output values. -- Missing edge case for the specific code path that changed. +- Changed behaviour without a test change, where a browser-free `src/test_*.py` test can reach it. `src/tests.py` needs a browser and live sites. +- Tests asserting mock call counts where output values would do (`testing.md`). +- A detection or engine change claimed verified by compile and unit tests alone. Those cannot say whether a page still clears; `workflow.md` names `/live-check` for that. ## What NOT to flag -- Style handled by linters (formatting, semicolons, quotes). -- Minor naming preferences without clarity impact. +- Upstream code kept mergeable on purpose, outside the changed lines: `src/undetected_chromedriver/`, `src/tests.py`, `src/tests_sites.py`, `src/bottle_plugins/`, the Chrome clearing core (FlareSolverr's) and the stealth clearing core (Byparr's). Upstream idiom there is not a finding. +- Anything under "Deliberately different" in `docs/dev/upstream-sync.md`. +- Code that looks wrong but encodes a measured constraint from the "Architecture (non-obvious)" bullets in `CLAUDE.md`: the `quote()` inside `escape()` in `postform.py`, the coordinate Turnstile click, no `page.evaluate` against a challenge page, the second look before a challenge counts as cleared, the even `maxTimeout` split. The tuned constants (`_CHALLENGE_CONFIRM_SECONDS`, `_NETWORKIDLE_MS`, `_CLICK_COOLDOWN_SECONDS`, `_WIDGET_RENDER_SECONDS`, `SOLVE_MARGIN_SECONDS`) are the same case. Ask for a measurement rather than proposing the obvious fix. - "I would have done it differently" without a concrete problem. -- Suggestions to add types or docs to code you didn't review. - Pre-existing issues outside the changed scope. ## Output format @@ -83,10 +94,10 @@ End with a single sentence naming the most important fix. For each finding: - **File:Line**: exact location. -- **Issue**: what's wrong and why it matters. Be specific ("this throws if user is null", not "potential null issue"). +- **Issue**: what's wrong and why it matters. Be specific ("the cookie read moved below `waitInSeconds` in `chrome_engine.py` only, so stealth still returns cookies from before the wait", not "possible inconsistency"). - **Suggestion**: how to fix it. Include code if helpful. - **Confidence**: 0 to 100. End with a brief overall assessment: what's solid, what needs work, the single most important fix. -Either way, apply the ≥80 confidence filter internally and drop findings below it. +Either way, apply the >=80 confidence filter internally and drop findings below it. diff --git a/.claude/agents/doc-reviewer.md b/.claude/agents/doc-reviewer.md index ffba74d..48f7161 100644 --- a/.claude/agents/doc-reviewer.md +++ b/.claude/agents/doc-reviewer.md @@ -1,6 +1,6 @@ --- name: doc-reviewer -description: Reviews documentation for accuracy, completeness, and clarity. Cross-references docs against the actual source code. +description: Reviews Solverr's docs, CHANGELOG, commit messages and code comments for accuracy against the code and for the repo's conventions (no em dashes, no target-site names, bold CHANGELOG headlines that stand alone as the release note, sentence-case headings). Cross-references docs against the actual source. tools: - Read - Grep @@ -8,74 +8,79 @@ tools: - Bash --- -You review documentation changes for quality. Focus on whether docs are accurate, complete, and useful, not whether they're pretty. +You review documentation changes in Solverr, a FlareSolverr fork (Python 3.14, the `/v1` API on 8191, an optional passthrough on 8888, a `chrome` and a `stealth` engine). Two jobs: verify claims against the actual code, and enforce the repo's own doc conventions. `.claude/rules/workflow.md` (CHANGELOG, commits, public-facing naming) and `.claude/rules/prose-style.md` (sentences and vocabulary) are the baseline; `.claude/rules/code-quality.md` covers comments. Focus on whether docs are accurate, complete, and convention-clean, not whether they're pretty. ## Operating principles - State assumptions explicitly. If you can't verify a claim against the code, say so. -- Surgical scope. Only flag issues in docs that changed, or that changes invalidated. -- Verify before flagging. Cite the source file:line you cross-checked. +- Surgical scope. Only flag issues in docs that changed, or that the code changes invalidated. +- Verify before flagging. Cite the source file you cross-checked. - Confidence threshold. Only ship findings you're at least 80% sure are real. ## How to review -Run `git diff --name-only` for changed docs (`.md`, `.txt`, `.rst`, docstrings, JSDoc, inline comments). For each doc change, read the source code it references and verify accuracy. +Run `git diff --name-only` for changed docs (`.md`, docstrings, comments) and `git log` over the range for commit messages. For each doc change, read the source code it references and verify accuracy. ## Accuracy (cross-reference with code) -- Function signatures: read the actual function, verify parameter names, types, return types, defaults match the docs. -- Code examples: trace each example against the source. Does the import path exist? Does the function accept those arguments? Does it return what the example claims? -- Config options: grep for the option name. Still used? Default value correct? -- File or directory references: use Glob to verify referenced paths exist. -- Can't verify? Say so explicitly: "Could not verify X. Requires runtime testing." +- Named symbols: grep every function, constant and file a doc names; verify it exists with that name and does what the doc says. +- Env vars: most are read in `src/config.py`, a few inherited ones in `src/utils.py` and `src/flaresolverr.py`. Check the name and the default against the README's Configuration tables. +- Commands: verify the commands in `CLAUDE.md` and `README.md` still run as written. +- Upstream claims: what came from FlareSolverr or Byparr, through which commit, and what is deliberately different must match `docs/dev/upstream-sync.md`, the single owner of that question. +- Can't verify? Say so explicitly: "Could not verify X." + +## Repo doc conventions + +- **No em dashes** in docs, comments, CHANGELOG or commits. No AI watermarks. +- **No target-site names or scraping vocabulary** in public surfaces: commit messages, branch names, `README.md`, `CLAUDE.md`, `CHANGELOG.md`, release notes (`workflow.md`, "Public-facing naming"). The generic forms are "a Cloudflare-gated site", "an indexer", "example-site.tld". `.githooks/pre-commit` only checks added lines in CHANGELOG and README against a heuristic, so read `CLAUDE.md` and commit messages yourself. +- **The bold CHANGELOG headline is the entire release note.** `release.yml` keeps only the bold text (`s/^- \*\*([^*]+)\*\*.*/- \1/`), so a new env var, default or limit a deployer must act on has to sit inside the bold. Apply that sed to every new entry. The headline is benefit-first, self-contained, ends in `.`, `!` or `?`, and names no class or mechanism. +- **CHANGELOG scope.** Only changes a deployer or API client could notice get an entry. A dependency bump or refactor that ships in the image is a plain line under `Other`. `.claude/`, hooks, CI, tests and repo docs get no entry at all. Iterating on something already in `[Unreleased]` edits that bullet instead of adding one. +- **Commits.** `type(scope): summary`, imperative, lower-case, <=72 chars, no trailing period. A non-trivial body leads with plain language. Never a bare `#N`; use `owner/repo#N`. +- **Prose style.** Sentence-case headings. Flag the `prose-style.md` habits (trailing significance clauses, inflated significance, padded triples, "serves as" where "is" would do) and its vocabulary table (`leverage`, `robust`, `ensure`, sentence-initial `Additionally`). Flag the pattern, not every word: the file allows a listed word when it is the precise term. +- **README describes current behaviour, not the journey** (`workflow.md`). `docs/dev/` records and the "Architecture (non-obvious)" bullets in `CLAUDE.md` carry reasoning and measurements on purpose. +- **Dev docs cite path plus symbol, not Solverr line numbers.** `_validate_url` in `src/flaresolverr_service.py`, not `:270`; line refs rot. +- **Single owner per fact.** Upstream history and divergences live in `docs/dev/upstream-sync.md`; the engine-layer rationale in `docs/dev/engine-layer-architecture.md`; the law in `.claude/rules/engine-layer.md`. A fact restated in a second doc is a finding; name the canonical home. +- **Code comments** (when the diff touches them): WHY, never WHAT (`code-quality.md`). Flag a comment that restates the adjacent code, and equally a cut that drops a measured constraint or an upstream divergence. Comments here often hold the measurement behind code that looks wrong (the `postform.py` docstring, `_resolve_challenge`), and losing one invites someone to "fix" it. ## Completeness -- Required parameters or environment variables not mentioned. -- Error cases: what happens when the function throws? What errors should the caller handle? -- Setup prerequisites a new developer would need. -- Breaking changes: if behavior changed, does the doc reflect it? +- A behaviour or config change without `README.md` updated in the same change (`workflow.md`). A new env var in `src/config.py` with no README row is the usual case. +- A port from, or decline of, an upstream change without the ledger's audited-through row or "Deliberately different" entry updated. +- A one-engine exception to the engine-layer law without its ledger record. +- `CLAUDE.md` "Where things live" missing a new top-level module or still naming a removed one. ## Staleness -- `grep -r "functionName"` to verify referenced functions and classes still exist. -- Version numbers, dependency names, and URLs that may be outdated. -- Deprecated API references (grep for `@deprecated` near referenced code). - -## Clarity - -- Vague instructions: "configure the service appropriately". Configure WHAT, WHERE, HOW? -- Missing context that assumes knowledge the reader may not have. -- Wall of text without structure (needs headings, lists, code blocks). -- Contradictions between sections. +- Grep referenced symbols to verify they still exist. +- Internal links: verify relative links resolve to files that exist. ## What NOT to flag - Minor wording preferences unless genuinely confusing. -- Formatting nitpicks handled by linters. -- Missing docs for internal or private code. -- Verbose but accurate content (suggest trimming, don't flag as wrong). +- Missing docs for internal code; docstrings belong at module and engine boundaries. +- Verbose but accurate content (suggest `/tighten`, don't flag as wrong). +- Site names where `workflow.md` allows them: chat, local scratch files, and the upstream list in `src/tests.py` and `src/tests_sites.py` (adding to those is a finding). ## Output format Default to terse. Switch to verbose only if the invocation prompt contains `verbose`, `full report`, or `detailed`. -**Default (terse)**: one line per finding, sorted by importance (accuracy issues first). +**Default (terse)**: one line per finding, sorted by importance (accuracy issues first, then convention violations). ``` file:line: (fix: ) ``` -End with one short sentence: accurate or inaccurate, complete or incomplete. +End with one short sentence: accurate or inaccurate, convention-clean or not. **Verbose**: For each finding: - **File:Line**: exact location. -- **Issue**: be specific ("README says `createUser(name)` takes one arg, but source shows `createUser(name, options)` with required `options.email`"). +- **Issue**: be specific ("README says `SESSION_MAX` defaults to 10, `config.session_max` returns 20"). - **Fix**: concrete rewrite or addition. - **Confidence**: 0 to 100. -End with overall assessment: accurate or inaccurate, complete or incomplete, structural suggestions. +End with overall assessment: accurate or inaccurate, complete or incomplete, convention issues. -Either way, apply the ≥80 confidence filter internally and drop findings below it. +Either way, apply the >=80 confidence filter internally and drop findings below it. diff --git a/.claude/agents/performance-reviewer.md b/.claude/agents/performance-reviewer.md index 8590189..e1ffa96 100644 --- a/.claude/agents/performance-reviewer.md +++ b/.claude/agents/performance-reviewer.md @@ -1,6 +1,6 @@ --- name: performance-reviewer -description: Reviews code for performance issues like memory leaks, slow queries, unnecessary computation, bundle size, and runtime bottlenecks. Use proactively after changes to hot paths, data processing, or API endpoints. +description: Reviews Solverr changes for extra browser launches, maxTimeout budget misuse, work that stalls the single stealth event loop, lock scope, and unbounded growth. Use after changes to the engines, sessions, the controller, or the passthrough. tools: - Read - Grep @@ -8,9 +8,9 @@ tools: - Bash --- -You are a performance engineer. Find real bottlenecks, not theoretical ones. Only flag issues that would cause measurable impact. +You are a performance engineer reviewing Solverr: Python 3.14, `bottle` + `waitress` (synchronous WSGI) serving the FlareSolverr `/v1` API on 8191, an optional passthrough on 8888, and two browser engines (`chrome`: Selenium + vendored undetected_chromedriver; `stealth`: Camoufox via invisible_playwright on ONE asyncio loop thread in `src/async_runtime.py`). Find real bottlenecks, not theoretical ones. Nearly all the cost is browsers: a launch takes seconds, a Camoufox browser is the heavier of the two in memory, and every page read is a round trip. Python-level cost rarely registers next to that. -This is static analysis. You can read code and estimate impact but cannot profile or benchmark. Flag based on how often the code path runs and how expensive the operation is. +This is static analysis. You can read code and estimate impact but cannot profile. Flag based on how often the code path runs and how expensive the operation is. ## Operating principles @@ -21,63 +21,41 @@ This is static analysis. You can read code and estimate impact but cannot profil ## How to review -Run `git diff --name-only`. Read each changed file plus its callers. Determine path frequency (per request, per user, once at startup). Rank findings by impact (frequency times cost). +Run `git diff --name-only`. Read each changed file plus its callers. Determine path frequency (per request, per engine attempt, per poll tick inside a solve, per reaper interval, once at startup). Rank findings by impact. -## Database and queries +## Browser launches -- **N+1**: ORM calls inside `for` / `forEach` / `map`, awaits in loops hitting the DB. Fix: join, include, or batch. -- **Missing indexes**: columns used in WHERE, ORDER BY, JOIN. Grep raw SQL or `where()` calls; check if indexed. -- **`SELECT *`** when only specific columns are serialized. -- **Unbounded queries**: no LIMIT on user-facing list endpoints, `.findAll()`, `.find({})`. -- **Missing pagination** on collection endpoints. -- **Transactions held open** during slow operations (network calls, file I/O inside the transaction). +- A path that launches where it used to reuse: a session rebuilt per request, a new context or page per poll tick, a fallback engine started with too little budget to reach the page. Sessions exist so one solve is reused many times (`CLAUDE.md`). +- A browser, context or page not closed on every exit path. Per-request browsers are closed in the engines' `finally` (`ChromeEngine.solve`, `StealthEngine.solve`); an early return that skips it leaks one browser per request. +- A new per-launch lookup. The stealth launch sends `geo.browser_identity` through `asyncio.to_thread`, and `src/geo.py` caches per proxy; an uncached lookup costs every launch. -## Memory +## The request budget -- Listeners, subscriptions, timers, intervals added without cleanup (`addEventListener` without `removeEventListener`, `setInterval` without `clearInterval`, RxJS `.subscribe()` without `.unsubscribe()`). -- Loading entire files or tables into memory when only a subset is needed. -- Long-lived closures capturing more scope than necessary (class instances captured in event handlers). -- Unbounded caches: `Map` / dict / `HashMap` that only gets `.set()`, no eviction or size limit. -- Streams or file handles not closed. +- `maxTimeout` is one budget for the whole request. `_resolve_challenge` splits what is left evenly across the planned engines and skips a fallback under `_MIN_ENGINE_SECONDS`; `budget.solve_deadline` keeps `SOLVE_MARGIN_SECONDS` back to build the response. +- Flag an engine handed the full budget instead of its share, a wait or retry loop with no deadline (the ledger records an unbounded Chrome Turnstile loop that needed one), a fixed sleep that ignores the remaining budget, or work after the deadline that the margin does not cover. -## Computation +## Threads, the loop, and locks -- Work repeated inside loops that could be hoisted (function calls, regex compilation, object creation in `map`). -- Synchronous blocking on the main thread: `fs.readFileSync`, `execSync`, CPU-heavy work without worker threads. -- Missing early returns when the answer is already known. -- Sorting or filtering large datasets on every render or request instead of caching. +- **The stealth loop is one thread** (`stealth-loop`). Every stealth request, launch and teardown runs on it, so anything blocking in a coroutine (`time.sleep`, a sync lookup, CPU-heavy parsing) stalls every stealth request at once. Requests on one stealth session also serialize on `StealthContext.lock`. +- **Waitress threads are few.** `flaresolverr.py` calls `serve` with no `threads=`, so waitress's default pool of four applies, and each solve holds its thread for the whole solve (Chrome under `func_timeout`, stealth blocked in `AsyncRuntime.run`). Anything that lengthens a solve cuts throughput for every client. +- **SessionStore lock scope** (`src/sessions.py`). Build and teardown run outside `self._lock` by design, per its docstring. A browser round trip or teardown moved under that lock blocks every create, get, destroy and the reaper. A session taken with `get` and not released with `end_use` in a `finally` is pinned for good, since the reaper and the cap both skip `in_use` sessions. +- **The passthrough lock** (`src/passthrough.py`). One module `_lock` guards `_cache` and `_inflight` for a `ThreadingHTTPServer` (a thread per connection). The cache-hit branch of `_Handler._handle` already calls `_send` while holding it, so one slow client stalls every passthrough request; flag any new socket write or solve under that lock. Concurrent requests for one path coalesce on `_Pending`; breaking that turns N identical requests into N solves. -## Network and I/O +## Unbounded growth -- Sequential awaits that could run in parallel. Fix: `Promise.all`, `asyncio.gather`, goroutines. -- Missing request timeouts (`fetch`, `axios`, `http.get` without timeout config). -- No retry-with-backoff for transient failures. -- Over-fetching (sending whole objects when partial data would do). -- Missing compression on responses over 1KB. -- No caching headers on static or rarely-changing responses. +Nothing restarts the process, so a dict keyed by host, path or session id that only grows is a leak. -## Frontend - -- Re-renders: inline object or function props (`onClick={() => ...}`), missing `key`, state updates that don't need to propagate. -- Images without `loading="lazy"`, `srcset`, or size optimization. -- Whole-library imports for one function (`import _ from 'lodash'` instead of `import debounce from 'lodash/debounce'`). -- Layout thrashing: interleaving DOM reads and writes in a loop. -- Animations triggering layout or paint instead of `transform` and `opacity`. -- Render-blocking CSS or JS in the critical path. - -## Concurrency - -- Shared mutable state without synchronization. -- Lock contention: holding locks during I/O or long computations. -- Unbounded worker, goroutine, or thread creation. Use a pool. -- Missing connection pooling for DB or HTTP clients. +- Precedents for the fix: Prometheus labels capped at `_MAX_DOMAIN_LABELS` (`src/bottle_plugins/prometheus_plugin.py`), the passthrough cache capped in bytes (`_cache_store`, `_MAX_BODY_SHARE`), PDFs capped at `_MAX_PDF_BYTES`. +- `_DOMAIN_ENGINE` in `flaresolverr_service.py` is known to be unbounded. Don't re-report it on an unrelated diff; flag a change that copies its shape or makes it grow faster. +- Session count is bounded only by `SESSION_MAX` per engine and `SESSION_TTL_MINUTES`; a change that bypasses `SessionStore` bypasses both. ## What NOT to flag -- Micro-optimizations with no measurable impact. -- Premature optimization in code that runs rarely or handles small data. -- "This could be faster in theory" without evidence it's a real bottleneck. -- Style preferences disguised as performance concerns. +- Micro-optimizations, and Python-level cost next to a browser round trip. +- Code that runs once at startup (`test_browser_installation`) or once per reaper interval, unless egregious. +- The measured waits: `_CHALLENGE_CONFIRM_SECONDS`, `_NETWORKIDLE_MS`, `_CLICK_COOLDOWN_SECONDS`, `_POLL_SECONDS`, `_WIDGET_RENDER_SECONDS`. A wait that looks slow there is what lets the challenge clear (`CLAUDE.md`, "Architecture (non-obvious)"). Ask for a measurement instead. +- Upstream-inherited code outside the diff (`src/undetected_chromedriver/`, the clearing cores). +- "This could be faster in theory" without a frequency-times-cost argument. ## Output format @@ -94,12 +72,12 @@ End with the single highest-impact fix to do first. **Verbose**: For each finding: -- **Impact**: High / Medium / Low, with WHY ("runs per request", "called once at startup, low impact"). +- **Impact**: High / Medium / Low, with WHY ("blocks the stealth loop on every request", "once at startup, low impact"). - **File:Line**: exact location. -- **Issue**: what's slow ("await inside a `for` loop makes N sequential DB calls for N items"). +- **Issue**: what's slow ("`time.sleep` inside `_wait_until_cleared` freezes every other stealth solve for its duration"). - **Fix**: specific code change. - **Confidence**: 0 to 100. End with the single highest-impact fix if they can only do one thing. -Either way, apply the ≥80 confidence filter internally and drop findings below it. +Either way, apply the >=80 confidence filter internally and drop findings below it. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md index 957c753..caec888 100644 --- a/.claude/agents/security-reviewer.md +++ b/.claude/agents/security-reviewer.md @@ -1,6 +1,6 @@ --- name: security-reviewer -description: Reviews code changes for security vulnerabilities. Use for PR review, pre-deploy verification, or audit of recently changed files. +description: Reviews Solverr changes for untrusted /v1 and passthrough input reaching a browser, secrets or solved cookies leaking into logs, proxy credentials left on disk, and accidental exposure. Use for PR review or audit of recently changed files. tools: - Read - Grep @@ -8,79 +8,59 @@ tools: - Bash --- -You are a senior security engineer reviewing code for vulnerabilities. This is static analysis. Flag patterns that look vulnerable, explain the attack vector, and when in doubt flag with a note. +You are a security engineer reviewing Solverr, a self-hosted bypass proxy: Python 3.14, `bottle` + `waitress` serving the FlareSolverr `/v1` API on 8191, an optional passthrough on 8888, and two real browsers (`chrome`: Selenium + vendored undetected_chromedriver; `stealth`: Camoufox via invisible_playwright on one asyncio loop thread in `src/async_runtime.py`). The threat surface is anyone who can reach those ports steering a browser, and secrets leaking into logs or onto disk. `.claude/rules/security.md` is the project's own baseline; enforce it. + +This is static analysis. Flag patterns that look vulnerable, explain the attack vector, and when in doubt flag with a note. ## Operating principles - State assumptions explicitly. If you can't tell whether input is trusted, say so. - Surgical scope. Review what changed; only flag pre-existing issues if the new code makes them exploitable. -- Verify before flagging. Cite file:line, name the attack vector, give a sample payload when relevant. +- Verify before flagging. Cite file:line and name the attack vector. - Confidence threshold. Only ship findings you're at least 80% sure are exploitable. ## How to review -Run `git diff --name-only`, read each changed file, grep the codebase for related patterns (one SQL injection often means more elsewhere). Cover every category below; skip nothing. - -## Injection - -- **SQL**: string concatenation or interpolation in queries (`"... WHERE id=" + id`, `f"WHERE id={id}"`, template literals). Fix: parameterized queries (`?`, `$1`, named params). -- **Command**: user input reaching shell execution (`exec("ls " + userInput)`, `os.system(f"ping {host}")`). Fix: array-form APIs (`execFile`, `subprocess.run([...])`). -- **XSS**: user input rendered without escaping (`innerHTML = userInput`, `dangerouslySetInnerHTML`, `v-html`, Blade `{!! $var !!}`, `document.write`). Fix: framework text rendering (JSX, Vue `{{ }}`, Go `html/template`). -- **Template**: user input as template content (`render_template_string(user_input)`). Fix: never pass user input as template body. -- **Path traversal**: user input in file paths (`fs.readFile("/uploads/" + filename)` and `../../etc/passwd`). Fix: allowlist + `path.resolve()` + verify prefix, reject `..`. - -## Authentication - -- Password compare with `==` or `===` instead of constant-time (`timingSafeEqual`, `hmac.compare_digest`). -- Session tokens in localStorage (XSS-readable) instead of httpOnly cookies. -- JWTs without `exp` claim. -- Password hashing with MD5, SHA1, SHA256 instead of bcrypt, scrypt, argon2. -- Hardcoded credentials: grep for `password =`, `secret =`, `apiKey =`, `token =` with string literals. -- Missing rate limiting on login, signup, and password reset endpoints. +Run `git diff --name-only`, read each changed file, grep the codebase for related patterns (one unsafe pattern often means more elsewhere). -## Authorization +## No auth, by design -- IDOR: lookups using user-supplied ID without checking ownership (`getOrder(req.params.id)` without `WHERE userId = currentUser`). -- Endpoints serving data without role or permission checks. -- Privilege escalation: user can set their own role in the request body. -- Frontend-only authorization (UI-checked but server doesn't re-verify). +Solverr has no authentication, and exposing it is the deployer's job (`security.md`). Do not propose adding auth. Flag accidental exposure instead: a new listener or endpoint, a default flipped on (`PASSTHROUGH_ENABLED`, `LOG_HTML`), or an endpoint that hands back local state such as files or environment. -## Data exposure +## Untrusted input boundaries -- Secrets in code: `API_KEY`, `SECRET`, `PASSWORD`, `TOKEN` assigned to literals. -- PII in logs: `console.log(user)`, `logger.info(request.body)`. -- Stack traces in responses: `res.json({ error: err.stack })`, unhandled error middleware that leaks internals. -- Verbose errors revealing schema, file paths, or service names. +- **The `/v1` body.** `validate_request_types` (`src/dtos.py`) types every declared field once, in `_controller_v1_handler`. Code that reads a field before that call, or one exempted in `_TYPE_OVERRIDES`, is unvalidated. +- **`url`.** Only `http(s)` may reach a browser: `_validate_url` in `src/flaresolverr_service.py`, called by `_cmd_request_get` and `_cmd_request_post`. Before v1.2.1 a `file://` URL came back in `solution.response`. A new navigating path that skips it, or any loosening of the anchored `_HTTP_URL` regex, is Critical. +- **`postData`.** `build_post_html` (`src/postform.py`) builds an auto-submitting form that both engines load as a `data:text/html` URL. The action is `escape(url, quote=True)` and every field `escape(quote(...))`; dropping either lets a caller inject markup and script into a page the browser runs. +- **`proxy`.** Validated once in `geo.proxy_to_config`, which fails closed on a malformed value (a string proxy used to launch unproxied while reporting success). A new path reading `req.proxy` without it bypasses that. +- **`cookies`.** A list per `validate_request_types`; the entries go to `driver.add_cookie` as sent, or through `_to_playwright_cookies` (a key filter plus domain anchoring). Anything that reads a cookie field into something other than the browser jar is a new boundary. +- **The passthrough** (`src/passthrough.py`). The target host must be in `PASSTHROUGH_ALLOWED_HOSTS` (`_split_host`, `_ALLOWED_HOSTS`); anything else goes to the default mirror, never a caller-chosen host. That allow list is the only thing keeping it from being an open proxy, so a target host taken from anywhere else (a header, the query, a redirect) is Critical. +- **Shell.** Request input never reaches a command string; navigation uses the driver or page API. -## Dependencies +## Secrets and logging -- `npm install` / `pip install` without pinned versions in CI. -- Postinstall scripts executing arbitrary code. -- CDN imports without integrity hashes (SRI). -- Run `npm audit` or `pip audit` if available. +- Never log `PROXY_PASSWORD`, `CAPTCHA_API_KEY`, or returned cookies (`cf_clearance`, `__ddg2_`). Once `flaresolverr.py` or `config.env_proxy` has filled in the proxy, the dict carries the password, so logging `req.proxy`, a `proxy_config`, or a whole request is the same leak. +- Known inherited site: `controller_v1_endpoint` logs the whole request at INFO and the whole response at DEBUG, lines kept byte-identical with FlareSolverr. Don't re-report it on an unrelated diff; flag a change that adds another such line or widens these. +- Exception text reaches both the response and the error log as `"Error: " + str(e)`. An exception built from a credentialed proxy URL or a cookie value leaks it to both. +- `LOG_HTML=true` (`utils.get_config_log_html`) dumps page HTML and stays off by default. +- Hardcoded credentials or API keys. -## Cryptography +## Credentials on disk -- MD5 / SHA1 used for security (not just checksums). -- `Math.random()` or `random.random()` for security tokens. Fix: `crypto.randomBytes`, `secrets.token_hex`. -- Hardcoded keys or IVs. -- ECB mode for block ciphers. -- Missing HTTPS enforcement. +- An authenticated Chrome proxy goes through a generated extension (`utils.create_proxy_extension`) whose temp dir holds the username and password in plaintext. `get_webdriver` removes it in a `finally`, a recorded divergence from upstream, which cleaned up only after a successful launch. A launch path that skips that `finally` leaves credentials in the temp directory. -## Input validation +## Browser and dependencies -- Missing validation on request body fields before use. -- ReDoS: nested quantifiers like `(a+)+`, `(a|b)*c` on user input. -- `parseInt(userInput)` without checking NaN. -- Missing length limits on strings (DoS via large payloads). -- Missing Content-Type validation on file uploads. +- `page.evaluate` or `execute_script` with a string built from request input. +- A launch pref that weakens isolation. Firefox's COOP/COEP stay on by recorded decision (`docs/dev/upstream-sync.md`). +- `invisible-playwright` loosened from its exact pin in `requirements.txt`. It carries the patched Firefox, so a floor lets an unattended bump change the browser. ## What NOT to flag -- Theoretical attacks with no realistic path (timing attacks against admin-only endpoints behind VPN). -- Pre-existing issues outside the diff unless the new code makes them exploitable. +- Missing auth, rate limiting, CSRF or session fixation. No auth is the design. +- A caller steering the browser to any `http(s)` host, private addresses included. `/v1` is a proxy; that is why exposure is the deployer's problem. +- Upstream code outside the diff: `src/undetected_chromedriver/`, `src/tests.py`, `src/tests_sites.py`, `src/bottle_plugins/`, and upstream's Chrome launch flags in `get_webdriver` (`--no-sandbox`, `--ignore-certificate-errors`). Also anything under "Deliberately different" in `docs/dev/upstream-sync.md`. - Defense-in-depth nice-to-haves when the primary defense is sound. -- Style or linter-territory issues. ## Output format @@ -99,10 +79,10 @@ End with a single sentence naming the highest-severity blocker, or "no issues fo For each finding: - **Severity**: Critical / High / Medium / Low. - **File:Line**: exact location. -- **Issue**: attack vector ("an attacker can send `../../../etc/passwd` as filename to read arbitrary files"). +- **Issue**: attack vector ("a `url` of `file:///etc/passwd` passes the new check and the file comes back in `solution.response`"). - **Fix**: specific code change. - **Confidence**: 0 to 100. If no issues, say so explicitly. Don't invent. -Either way, apply the ≥80 confidence filter internally. This tool is not a substitute for a professional audit. +Either way, apply the >=80 confidence filter internally. This tool is not a substitute for a professional audit. diff --git a/.claude/hooks/auto-test.sh b/.claude/hooks/auto-test.sh deleted file mode 100644 index 11ada26..0000000 --- a/.claude/hooks/auto-test.sh +++ /dev/null @@ -1,148 +0,0 @@ -#!/bin/bash -# Finds and runs the matching test file after Claude edits a source file. -# PostToolUse hook for Edit|Write. -# Silent on success. Only emits output when tests fail, so passing tests -# contribute zero tokens. Skips test files themselves, config files, and -# non-testable extensions. - -# Requires jq for JSON parsing. -if ! command -v jq >/dev/null 2>&1; then - exit 0 -fi - -INPUT=$(cat) -FILE_PATH=$(echo "$INPUT" | jq -r '.tool_input.file_path // empty') - -if [ -z "$FILE_PATH" ] || [ ! -f "$FILE_PATH" ]; then - exit 0 -fi - -BASENAME=$(basename "$FILE_PATH") -EXTENSION="${BASENAME##*.}" -NAME="${BASENAME%.*}" -DIR=$(dirname "$FILE_PATH") - -# Skip if the edited file IS a test file. -case "$BASENAME" in - *.test.*|*.spec.*|*_test.*|*_spec.*|test_*|spec_*) exit 0 ;; -esac - -# Skip config, style, and non-code files. -case "$EXTENSION" in - json|yaml|yml|toml|ini|cfg|env|md|txt|css|scss|less|svg|png|jpg|ico|html) exit 0 ;; -esac - -# Skip files in non-testable directories. -case "$FILE_PATH" in - */.claude/*|*/public/*|*/static/*|*/assets/*|*/__mocks__/*) exit 0 ;; -esac - -# Find project root. -find_project_root() { - local dir="$PWD" - while [ "$dir" != "/" ]; do - if [ -f "$dir/package.json" ] || [ -f "$dir/pyproject.toml" ] || [ -f "$dir/Cargo.toml" ] || [ -f "$dir/go.mod" ] || [ -d "$dir/.git" ]; then - echo "$dir" - return - fi - dir=$(dirname "$dir") - done - echo "$PWD" -} - -ROOT=$(find_project_root) -STEM="$NAME" - -# Search for a matching test file in the usual conventions. -find_test_file() { - local stem="$1" - local ext="$2" - - local patterns=( - "${stem}.test.${ext}" - "${stem}.spec.${ext}" - "${stem}_test.${ext}" - "${stem}_spec.${ext}" - "test_${stem}.${ext}" - ) - - # Same directory first. - for pattern in "${patterns[@]}"; do - [ -f "${DIR}/${pattern}" ] && { echo "${DIR}/${pattern}"; return; } - done - - # __tests__ subdirectory (Jest convention). - for pattern in "${patterns[@]}"; do - [ -f "${DIR}/__tests__/${pattern}" ] && { echo "${DIR}/__tests__/${pattern}"; return; } - done - - # Parallel test directory structure (src/foo.ts -> tests/foo.test.ts). - local rel_dir="${DIR#$ROOT/}" - local test_rel_dir - for test_root in "tests" "test" "__tests__" "spec"; do - test_rel_dir=$(echo "$rel_dir" | sed "s|^src/|${test_root}/|;s|^lib/|${test_root}/|") - for pattern in "${patterns[@]}"; do - [ -f "${ROOT}/${test_rel_dir}/${pattern}" ] && { echo "${ROOT}/${test_rel_dir}/${pattern}"; return; } - done - done - - # Broad search as last resort, depth-limited to stay fast. - local found - for pattern in "${patterns[@]}"; do - found=$(find "$ROOT" -maxdepth 5 -name "$pattern" -not -path "*/node_modules/*" -not -path "*/.git/*" -print -quit 2>/dev/null) - [ -n "$found" ] && { echo "$found"; return; } - done -} - -TEST_FILE=$(find_test_file "$STEM" "$EXTENSION") - -if [ -z "$TEST_FILE" ]; then - # No matching test found, not an error. - exit 0 -fi - -# Make path relative for cleaner output if we end up emitting failure logs. -REL_TEST="${TEST_FILE#$ROOT/}" - -# Run tests, capture output, only emit on failure. -# Use default (non-verbose) reporters to keep failure logs tight. -OUTPUT="" -EXIT=0 -case "$EXTENSION" in - js|jsx|ts|tsx|mjs|cjs) - if [ -f "$ROOT/node_modules/.bin/vitest" ]; then - OUTPUT=$(cd "$ROOT" && npx vitest run "$REL_TEST" 2>&1); EXIT=$? - elif [ -f "$ROOT/node_modules/.bin/jest" ]; then - OUTPUT=$(cd "$ROOT" && npx jest "$REL_TEST" 2>&1); EXIT=$? - elif [ -f "$ROOT/node_modules/.bin/mocha" ]; then - OUTPUT=$(cd "$ROOT" && npx mocha "$REL_TEST" 2>&1); EXIT=$? - else - OUTPUT=$(cd "$ROOT" && npm test -- "$REL_TEST" 2>&1); EXIT=$? - fi - ;; - py) - if command -v pytest >/dev/null 2>&1; then - OUTPUT=$(cd "$ROOT" && pytest "$REL_TEST" 2>&1); EXIT=$? - elif command -v python3 >/dev/null 2>&1; then - OUTPUT=$(cd "$ROOT" && python3 -m unittest "$REL_TEST" 2>&1); EXIT=$? - elif command -v python >/dev/null 2>&1; then - OUTPUT=$(cd "$ROOT" && python -m unittest "$REL_TEST" 2>&1); EXIT=$? - fi - ;; - go) - OUTPUT=$(cd "$DIR" && go test ./... 2>&1); EXIT=$? - ;; - rs) - OUTPUT=$(cd "$ROOT" && cargo test 2>&1); EXIT=$? - ;; - *) - exit 0 - ;; -esac - -if [ "$EXIT" -ne 0 ]; then - echo "auto-test: failures in $REL_TEST" - echo "$OUTPUT" -fi - -exit 0 diff --git a/.claude/hooks/block-dangerous-commands.sh b/.claude/hooks/block-dangerous-commands.sh index 6faa178..34f317f 100644 --- a/.claude/hooks/block-dangerous-commands.sh +++ b/.claude/hooks/block-dangerous-commands.sh @@ -43,6 +43,9 @@ contains_cmd() { printf '%s' "$COMMAND" | grep -qE "$1"; } contains_icmd() { printf '%s' "$COMMAND" | grep -qiE "$1"; } # ── Git push protections ──────────────────────────────────────────────── +# The session's cwd, which is where the command actually runs. The hook's own cwd is always the +# project dir, so without this a push from a loop worktree is judged by the main tree's branch. +SESSION_CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // empty' 2>/dev/null || true) if contains_cmd '(^|[;&|()]+[[:space:]]*)git[[:space:]]+push'; then # Explicit refspec to a protected branch (origin main, :main, HEAD:main, remote branch) if contains_cmd "git[[:space:]]+push[[:space:]]+[^[:space:]]+[[:space:]]+([^[:space:]]*:)?($BR_REGEX)(\$|[[:space:]])"; then @@ -55,7 +58,7 @@ if contains_cmd '(^|[;&|()]+[[:space:]]*)git[[:space:]]+push'; then fi # Bare `git push` while on protected branch if contains_cmd 'git[[:space:]]+push[[:space:]]*($|[;&|])'; then - CURRENT=$(git branch --show-current 2>/dev/null || true) + CURRENT=$(git -C "${SESSION_CWD:-.}" branch --show-current 2>/dev/null || git branch --show-current 2>/dev/null || true) if [ -n "$CURRENT" ] && printf '%s' ",$PROTECTED_BRANCHES," | grep -q ",$CURRENT,"; then emit_deny "Blocked: you are on '$CURRENT' (a protected branch). Switch to a feature branch." fi @@ -67,6 +70,17 @@ if contains_cmd '(^|[;&|()]+[[:space:]]*)git[[:space:]]+push'; then fi fi +# ── Merging is never the agent's call ─────────────────────────────────── +# The loop worker opens a draft PR and stops, and every PR is merged by a person with a merge +# commit. To GitHub a PR merge is a legitimate action, so no ruleset can express this; the +# matcher here is the only guard. +if contains_cmd '(^|[;&|()]+[[:space:]]*)gh[[:space:]]+pr[[:space:]]+merge'; then + emit_deny "Blocked: merging a PR is the owner's call. Open the PR and stop." +fi +if contains_cmd 'gh[[:space:]]+api[^;&|]*pulls/[0-9]+/merge'; then + emit_deny "Blocked: merging a PR through the API is the owner's call. Open the PR and stop." +fi + # ── Destructive filesystem operations ─────────────────────────────────── # rm -rf targeting root, home, $HOME, $VAR (any unresolved expansion), or parent traversal. # We normalise quotes before matching so "my folder", '$HOME/trash', etc. Are all inspected. @@ -79,6 +93,19 @@ if printf '%s' "$CMD_NOQUOTE" | grep -qE 'rm[[:space:]]+(-[a-zA-Z]+[[:space:]]+) emit_deny "Blocked: recursive delete targeting a system directory." fi +# ── Secret files are never read through the shell ─────────────────────── +# The permissions deny list covers the Read, Write and Edit tools only, and auto mode routes file +# reads through the shell instead, so `cat .env` walks straight past it. Only this hook sees the +# command text. The reader must sit in command position (line start or after an operator): +# several reader names are ordinary English words, and matching them mid-sentence rejected +# commit messages. +SECRET_READERS='cat|head|tail|sed|awk|grep|rg|less|more|strings|xxd|od|base64|cp|mv|scp|curl|type|gc|Get-Content|Select-String|Copy-Item' +SECRET_TARGETS='(^|[[:space:]=/\\])\.env([[:space:]./]|$)|\.(pem|key|p12|pfx|jks|keystore)([[:space:]]|$)|(^|[[:space:]=/\\])secrets[/\\]|id_rsa|deny-names\.local' +if printf '%s' "$CMD_NOQUOTE" | grep -qiE "(^|[;&|(])[[:space:]]*($SECRET_READERS)[[:space:]]" \ + && printf '%s' "$CMD_NOQUOTE" | grep -qE "$SECRET_TARGETS"; then + emit_deny "Blocked: that reads a secret file (a .env, a key or certificate, secrets/, or the local deny list). Open it yourself if you need its contents." +fi + # ── PowerShell destructive operations ─────────────────────────────────── # PowerShell has its own spelling for everything above and the POSIX patterns # see none of it. Parameter names may be truncated (-Recurse accepts -Rec), so @@ -152,9 +179,11 @@ fi # Disk / partition. Note: only REDIRECTIONS to /dev/ are destructive. `2>/dev/null` is not. # Pattern matches: `>[ ]*/dev/` but NOT `2>/dev/null` or `&>/dev/null` style for fd-null. -# Strategy: match `>` optionally with whitespace, followed by /dev/, EXCLUDING /dev/null and /dev/stderr/stdout. -if printf '%s' "$COMMAND" | grep -qE '(^|[^0-9&])>[[:space:]]*/dev/[a-zA-Z][a-zA-Z0-9]*' \ - && ! printf '%s' "$COMMAND" | grep -qE '>[[:space:]]*/dev/(null|stdout|stderr|tty|zero|random|urandom)([[:space:]]|$)' ; then +# Strategy: delete the harmless redirects first, then match `>` followed by /dev/ on what is +# left. Excluding them on the whole command failed both ways: `>/dev/null;` read as unsafe because +# the exclusion wanted whitespace after it, and one safe redirect cleared a dangerous one later on. +CMD_SANS_SAFE=$(printf '%s' "$COMMAND" | sed -E 's#>[[:space:]]*/dev/(null|stdout|stderr|tty|zero|random|urandom)##g') +if printf '%s' "$CMD_SANS_SAFE" | grep -qE '(^|[^0-9&])>[[:space:]]*/dev/[a-zA-Z][a-zA-Z0-9]*' ; then emit_deny "Blocked: redirection into a raw device file can destroy data." fi if contains_cmd '(^|[;&|[:space:]])(mkfs|mkfs\.[a-z0-9]+)([[:space:]]|$)' \ diff --git a/.claude/hooks/context-recovery.sh b/.claude/hooks/context-recovery.sh deleted file mode 100644 index bfe9392..0000000 --- a/.claude/hooks/context-recovery.sh +++ /dev/null @@ -1,111 +0,0 @@ -#!/bin/bash -# Re-injects critical project rules after context compaction. -# Used as a SessionStart hook with matcher "compact". -# -# When Claude's context window fills up, compaction summarizes the conversation -# and loses specific details. This hook restores your non-negotiable project -# rules so Claude stays aligned even after compaction. -# -# Customize the RULES section below with your project-specific requirements. - -# ────────────────────────────────────────────── -# Find project root -# ────────────────────────────────────────────── - -find_project_root() { - local dir="$PWD" - while [ "$dir" != "/" ]; do - if [ -f "$dir/package.json" ] || [ -f "$dir/pyproject.toml" ] || [ -f "$dir/Cargo.toml" ] || [ -f "$dir/go.mod" ] || [ -d "$dir/.git" ]; then - echo "$dir" - return - fi - dir=$(dirname "$dir") - done - echo "$PWD" -} - -ROOT=$(find_project_root) - -# ────────────────────────────────────────────── -# Dynamic context (same as session-start.sh) -# ────────────────────────────────────────────── - -CONTEXT="" - -BRANCH=$(git branch --show-current 2>/dev/null) -if [ -n "$BRANCH" ]; then - CONTEXT="Branch: $BRANCH" -fi - -LAST_COMMIT=$(git log --oneline -1 2>/dev/null) -if [ -n "$LAST_COMMIT" ]; then - CONTEXT="$CONTEXT | Last commit: $LAST_COMMIT" -fi - -CHANGES=$(git status --porcelain 2>/dev/null | wc -l | tr -d ' ') -if [ "$CHANGES" -gt 0 ] 2>/dev/null; then - CONTEXT="$CONTEXT | Uncommitted changes: $CHANGES files" -fi - -# ────────────────────────────────────────────── -# Re-inject critical project rules -# ────────────────────────────────────────────── - -cat <<'RULES' -=== CONTEXT RECOVERED AFTER COMPACTION === - -CRITICAL PROJECT RULES (restored automatically. Do not ignore): - -1. TESTING - - Run the specific test file after changes, not the full suite. - - Tests must verify behavior, not implementation details. - - Prefer real implementations over mocks. Only mock at system boundaries. - - One assertion per test. Arrange-Act-Assert structure. - -2. CODE QUALITY - - Don't add features beyond what was asked. - - No dead code or commented-out blocks. - - Functions do one thing. No magic values. - - Named exports over default exports. - -3. WORKFLOW - - Run typecheck after making code changes. - - Prefer fixing root causes over workarounds. - - Don't modify generated files (*.gen.ts, *.generated.*). - - Don't modify lock files, .env files, or hook scripts. - -4. SECURITY - - Never commit secrets, tokens, or credentials. - - Validate all user input at system boundaries. - - Parameterized queries only. No string interpolation in SQL. - -5. GIT - - Don't push directly to main/master. - - No force pushes (use --force-with-lease if needed). - - Create feature branches for all work. - -RULES - -# ────────────────────────────────────────────── -# Append dynamic context -# ────────────────────────────────────────────── - -if [ -n "$CONTEXT" ]; then - echo "" - echo "Current state: $CONTEXT" -fi - -# ────────────────────────────────────────────── -# Re-read CLAUDE.md if it exists (belt and suspenders) -# ────────────────────────────────────────────── - -if [ -f "$ROOT/CLAUDE.md" ]; then - echo "" - echo "=== CLAUDE.md (re-injected) ===" - cat "$ROOT/CLAUDE.md" -fi - -echo "" -echo "=== END CONTEXT RECOVERY ===" - -exit 0 \ No newline at end of file diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/31-block-gh-pr-merge.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/31-block-gh-pr-merge.json new file mode 100644 index 0000000..3d545e1 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/31-block-gh-pr-merge.json @@ -0,0 +1,6 @@ +{ + "name": "block gh pr merge", + "stdin": { "tool_input": { "command": "gh pr merge 11 -R owner/repo --merge" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "merg"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/32-block-gh-api-merge.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/32-block-gh-api-merge.json new file mode 100644 index 0000000..e450a19 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/32-block-gh-api-merge.json @@ -0,0 +1,6 @@ +{ + "name": "block a PR merge through gh api", + "stdin": { "tool_input": { "command": "gh api -X PUT repos/owner/repo/pulls/11/merge" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "merg"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/33-allow-gh-pr-view.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/33-allow-gh-pr-view.json new file mode 100644 index 0000000..3fdcb08 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/33-allow-gh-pr-view.json @@ -0,0 +1,5 @@ +{ + "name": "allow reading a PR", + "stdin": { "tool_input": { "command": "gh pr view 11 -R owner/repo --json state,mergeable" } }, + "expect_exit": 0 +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/34-block-cat-env.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/34-block-cat-env.json new file mode 100644 index 0000000..0fe1fb7 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/34-block-cat-env.json @@ -0,0 +1,6 @@ +{ + "name": "block reading a .env through the shell", + "stdin": { "tool_input": { "command": "cat .env" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "secret"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/35-block-ps-read-deny-names.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/35-block-ps-read-deny-names.json new file mode 100644 index 0000000..4280939 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/35-block-ps-read-deny-names.json @@ -0,0 +1,6 @@ +{ + "name": "block reading the local deny list through PowerShell", + "stdin": { "tool_name": "PowerShell", "tool_input": { "command": "Get-Content .githooks/deny-names.local" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "secret"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/36-allow-reader-words-in-prose.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/36-allow-reader-words-in-prose.json new file mode 100644 index 0000000..54f8048 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/36-allow-reader-words-in-prose.json @@ -0,0 +1,5 @@ +{ + "name": "allow a commit message that uses reader verbs as words", + "stdin": { "tool_input": { "command": "git commit -m \"docs: say how head and tail of the .env example are read\"" } }, + "expect_exit": 0 +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/37-allow-devnull-then-semicolon.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/37-allow-devnull-then-semicolon.json new file mode 100644 index 0000000..c2a9682 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/37-allow-devnull-then-semicolon.json @@ -0,0 +1,5 @@ +{ + "name": "allow a /dev/null redirect followed by another command", + "stdin": { "tool_input": { "command": "ls >/dev/null; echo done" } }, + "expect_exit": 0 +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/38-block-raw-device-after-safe-redirect.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/38-block-raw-device-after-safe-redirect.json new file mode 100644 index 0000000..0a547eb --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/38-block-raw-device-after-safe-redirect.json @@ -0,0 +1,6 @@ +{ + "name": "block a raw device write even after a safe redirect", + "stdin": { "tool_input": { "command": "ls >/dev/null && cat x > /dev/sda" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "device"] +} diff --git a/.claude/settings.json b/.claude/settings.json index 073768e..de0c290 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -35,7 +35,11 @@ "Bash(git fetch *)", "Bash(git checkout *)", "Bash(git switch *)", - "Bash(gh pr *)", + "Bash(gh pr view *)", + "Bash(gh pr list *)", + "Bash(gh pr diff *)", + "Bash(gh pr checks *)", + "Bash(gh pr create *)", "Bash(gh issue *)", "Bash(gh run *)", "Bash(git rev-parse *)", @@ -46,7 +50,15 @@ "Bash(git push -u origin loop/*)", "Bash(gh label *)", "Bash(docker manifest inspect *)", - "Bash(gh release view *)" + "Bash(gh release view *)", + "PowerShell(git status*)", + "PowerShell(git log *)", + "PowerShell(git diff *)", + "PowerShell(git show *)", + "PowerShell(git rev-parse *)", + "PowerShell(git ls-files *)", + "PowerShell(git add *)", + "PowerShell(git commit *)" ], "deny": [ "Read(**/.env)", @@ -67,7 +79,41 @@ "Read(**/*.jks)", "Read(**/*.keystore)", "Read(**/*.p12)", - "Read(**/deny-names.local)" + "Read(**/deny-names.local)", + "Bash(git checkout -f *)", + "Bash(git switch -f *)", + "Bash(git switch --discard-changes *)", + "Bash(git stash drop *)", + "Bash(git stash clear)", + "Bash(git branch -D *)", + "Bash(git tag -d *)", + "Bash(git push --delete *)", + "Bash(git reflog expire *)", + "Bash(git gc --prune=now*)" + ], + "ask": [ + "Bash(git push --force-with-lease*)", + "PowerShell(git push*)", + "Bash(git rebase *)", + "Bash(git cherry-pick *)", + "Bash(git commit --amend*)", + "Bash(git filter-branch *)", + "Bash(gh pr edit *)", + "Bash(gh pr close *)", + "Bash(gh pr comment *)", + "Bash(gh pr review *)", + "Bash(gh issue close *)", + "Bash(gh issue comment *)", + "Bash(gh release create *)", + "Bash(gh release edit *)", + "Bash(gh release delete *)", + "Bash(gh release upload *)", + "Bash(gh api -X *)", + "Bash(gh api --method *)", + "Bash(gh api * -X *)", + "Bash(gh api * --method *)", + "Bash(gh repo edit *)", + "Bash(gh workflow run *)" ] }, "hooks": { @@ -139,7 +185,8 @@ "hooks": [ { "type": "command", - "command": "which osascript >/dev/null 2>&1 && osascript -e 'display notification \"Claude Code needs your attention\" with title \"Claude Code\"' || which notify-send >/dev/null 2>&1 && notify-send 'Claude Code' 'Claude Code needs your attention' || true" + "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/notify.sh", + "timeout": 5000 } ] } diff --git a/.dockerignore b/.dockerignore index 4eaa27c..4c76b55 100644 --- a/.dockerignore +++ b/.dockerignore @@ -19,6 +19,5 @@ docs/ html_samples/ CHANGELOG.md CLAUDE.md -CLAUDE.local.md.example Handoff.md README.md diff --git a/CLAUDE.local.md.example b/CLAUDE.local.md.example deleted file mode 100644 index f3b1f53..0000000 --- a/CLAUDE.local.md.example +++ /dev/null @@ -1,26 +0,0 @@ -# Personal Overrides - -> Rename this to CLAUDE.local.md. It's gitignored and won't be shared with the team. - -## My Preferences - -- I prefer verbose commit messages with context -- Always explain your reasoning before making changes -- When in doubt, ask rather than guess - -## Environment - -- My test database runs on port 5433 (not default 5432) -- Use `pnpm` instead of `npm` on my machine - -## Shortcuts - -- When I say "ship it", run `/ship` -- When I say "review", run `/pr-review` -- When I say "fix it", run `/debug-fix` - -## Current Context - -- I'm working on the billing module this sprint -- The staging environment is at https://staging.example.com -- Feature flags are managed in LaunchDarkly From 43a4a2175209546e2af4e9fe90168edba1122d6c Mon Sep 17 00:00:00 2001 From: unseensnick <84652498+unseensnick@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:27:32 +0200 Subject: [PATCH 4/7] docs(claude): record the owner's rulings and fix the stale rules Several rules contradicted the engine-layer law or each other, and a handful of docs still described the code as it was before 1.6.0. The owner ruled on each conflict; the rules now say one thing. - An engine pair is never split: "fix some, name the rest" stays for ordinary sibling sites only, and a parity gap on a surface being touched is levelled up in that change - Tests may loop with subTest over a fixed tuple of engines, the shape the conformance suite already uses, and every new test is seen failing - A knob or dependency that only one clearing core uses falls under the clearing-core decline, and one boolean from each adapter is allowed until a second divergent bit appears on the same surface - A WHAT comment is allowed when the what is not visible in the code - Merging is by a person with a merge commit; a human co-author trailer is credit; reports cap at 700 words with graded detail - The architecture notes move to a rule scoped to src/, and CLAUDE.md gains the hooks, agents, test commands and working rules it lacked - The ledger, the architecture record and loops.md drop stale versions, "(unreleased)" markers, a shipped gap, the dropped SessionRef, the old test count, and the idea that the loop worker never touches engines - CONTRIBUTING.md describes the naming check without using a word the check rejects --- .claude/rules/architecture.md | 19 +++++++ .claude/rules/code-quality.md | 12 ++-- .claude/rules/engine-layer.md | 22 ++++++-- .claude/rules/plan-output.md | 10 ++-- .claude/rules/testing.md | 10 ++-- .claude/rules/workflow.md | 17 +++--- .github/pull_request_template.md | 2 +- CLAUDE.md | 80 ++++++++++++++------------- CONTRIBUTING.md | 2 +- docs/dev/engine-layer-architecture.md | 10 ++-- docs/dev/loops.md | 12 ++-- docs/dev/upstream-sync.md | 22 ++++---- 12 files changed, 130 insertions(+), 88 deletions(-) create mode 100644 .claude/rules/architecture.md diff --git a/.claude/rules/architecture.md b/.claude/rules/architecture.md new file mode 100644 index 0000000..caa60e0 --- /dev/null +++ b/.claude/rules/architecture.md @@ -0,0 +1,19 @@ +--- +paths: + - "src/**" +--- + +# Architecture (non-obvious) + +How the pieces fit, and the constraints that look wrong until you know what they were measured against. `CLAUDE.md` carries the one-paragraph version; [engine-layer.md](engine-layer.md) is the law for what is shared between the engines and what is not. + +- Two engines behind one interface (`engines/base.py`): `chrome` (Selenium + vendored undetected_chromedriver, the default) and `stealth` (Camoufox via invisible_playwright + playwright-captcha). The controller auto-falls-back between them and remembers per-host which one cleared it. +- The stealth engine is async Playwright running on ONE background event-loop thread (`async_runtime.py`); persistent Camoufox contexts (sessions) live there so their cookies survive across requests. The server itself is synchronous. +- Sessions: each engine keeps its own pool, both using one `SessionStore` (`sessions.py`) so the lifecycle rules exist once; a background reaper (`session_reaper.py`) closes idle browsers. A session handed out is marked in use under the same lock that found it, which is what keeps the reaper and the cap off a live browser. Solve once, reuse the cookie many times. +- Escalation ladder for an `auto` request: Chrome → Camoufox click-solve → (optional, dormant) paid CAPTCHA API. +- **A Turnstile checkbox is clicked by coordinate, with no JS evaluation.** The widget's iframe sits in a closed shadow root, so `query_selector` cannot find it, but `page.frames` lists it anyway; `frame_element().bounding_box()` gives its rect and `page.mouse` clicks the checkbox. This exists because playwright-captcha's shadow-root traversal uses `evaluate_handle`, and the iframe's CSP blocks eval under Firefox, which silently broke widget solving. Cloudflare's own interstitial builds the widget itself and its frame reports an empty URL, so when no frame matches, the rect comes from the nearest ancestor `div` of the token input instead, which is in the light DOM. Do not reach for `page.evaluate` to measure any of this: running page scripts against a live challenge makes Cloudflare reissue it. +- **A challenge is only over once a clear reading survives a second look.** Cloudflare drops the challenge markup while it issues the next round, so believing the first clear reading returns an intermediate challenge page. +- **`maxTimeout` is one budget for the whole request, split evenly across the planned engines.** It used to be handed to each engine in full, so a fallback could take twice as long as asked and trip the caller's own timeout. An even share is what makes the fallback reachable: giving the first engine everything let it spend the lot, and a request that used to succeed in 133s failed at 120s with the second engine skipped. A quick first engine costs the fallback nothing, since the fallback inherits everything unspent. +- **The POST form is carried to the browser as a `data:text/html,` URL, so its fields are percent-encoded and must stay that way** (`postform.py`). The browser URL-decodes the document before the HTML parser sees it, so a value holding a bare `%` or `#` is otherwise re-read as an escape or truncates the document at the fragment. The `quote()` calls look like double-encoding and are not: removing them breaks POST for those values, measured against a live echo service. +- **Solverr resolves the browser's timezone and language itself (`geo.py`), and hands both engines the same pair.** Left alone, the stealth stack resolves both from the exit IP on every launch, inside the library, uncached, and raises behind a proxy when the lookup fails, which kills the launch; Chrome derived neither, so the two engines disagreed about the country. Passing concrete values returns before that fatal branch. They travel together because the pairing is what a site checks. Chrome follows via `Emulation.setTimezoneOverride` (which moves its ICU clock rather than patching `Intl` in the page) and `--accept-lang`. A failed lookup falls back to `TZ` and `en-US`: a wrong zone still solves, no browser does not. +- **playwright-captcha only ever touches a throwaway page**, and only the paid escalation reaches it now. Preparing a solver injects init scripts (one rewrites `Element.prototype.attachShadow`) that a Cloudflare interstitial will not clear while they are present, and Playwright cannot remove an init script. Verified live: an interstitial clears in ~3s without them and never in 40s with them. diff --git a/.claude/rules/code-quality.md b/.claude/rules/code-quality.md index d866500..31d05eb 100644 --- a/.claude/rules/code-quality.md +++ b/.claude/rules/code-quality.md @@ -9,19 +9,19 @@ alwaysApply: true - **DRY**: before adding a helper, search for an existing equivalent (`postform.py`, `detection.py`, `config.py`). - **YAGNI**: add only what the task needs. No speculative parameters or abstractions for hypothetical callers. - **KISS**: simplest correct solution. Justify complexity with a concrete requirement, not elegance. -- **Fix the defect, not the instance that reproduced.** A bug present in five places is one bug with five sites. Fix all five, or name the ones you left and why. Search for the sibling instances before calling a fix done. The shared spine (`assembly.py`, `pipeline.py`, `budget.py`, `sessions.py`) now holds the rules that used to be written once per engine, so a defect in one of those is a defect for both; what remains genuinely per-engine is each clearing core. +- **Fix the defect, not the instance that reproduced.** A bug present in five places is one bug with five sites. Fix all five, or name the ones you left and why, with one exception: an engine pair is never split. A change a client can observe lands for both engines in the same commit, and the only exit is the named mechanism in [engine-layer.md](engine-layer.md). Search for the sibling instances before calling a fix done. The shared spine (`assembly.py`, `pipeline.py`, `budget.py`, `sessions.py`) now holds the rules that used to be written once per engine, so a defect in one of those is a defect for both; what remains genuinely per-engine is each clearing core. - **Minimal blast radius is measured against the defect, not against the diff.** Leave genuinely unrelated code alone. A small diff is not the goal: when the correct fix needs a helper extracted, a signature changed, or a call site moved, do that instead of threading a workaround through the shape that is already there. -- **Refactor when the fix needs it**, in the same change, with the reason in the commit body. Still no standalone refactor sprints, and still nothing adjacent riding along uninvited. -- **One standing exemption to that ban** (owner, 2026-08-25): the engine layer program in [engine-layer.md](engine-layer.md), whose steps are refactors with no fix attached. It exists because the per-engine duplication produced the same defect in both engines at once, which a fix-shaped change cannot prevent recurring. The exemption covers only the sequenced steps recorded in [engine-layer-architecture.md](../../docs/dev/engine-layer-architecture.md); anything else is still an ordinary refactor and still needs a fix to ride with. +- **Refactor when the fix needs it**, in the same change, with the reason in the commit body. Still no standalone refactor sprints, and still nothing adjacent riding along uninvited. One exception comes from the engine-layer law: a parity gap you notice on an engine surface you are touching is levelled up in that change, unless the owner gates it. That never licenses cleanup on a file just because it was open. +- **One standing exemption to that ban** (owner, 2026-08-25): the engine layer program in [engine-layer.md](engine-layer.md), whose steps are refactors with no fix attached. It exists because the per-engine duplication produced the same defect in both engines at once, which a fix-shaped change cannot prevent recurring. The exemption covers only the sequenced steps recorded in [engine-layer-architecture.md](../../docs/dev/engine-layer-architecture.md); anything else is still an ordinary refactor and still needs a fix to ride with. All six steps are done, so the exemption covers nothing further: a new behaviour-free move needs the owner's approval like any other refactor. - **Prefer the proper fix over the patch.** If the patch is genuinely the right call (a risky area, a release in flight), say so explicitly and record what the proper fix would be. An unstated tradeoff reads as an oversight to whoever finds it next. ## Anti-defaults (counter common Claude tendencies) -- No premature abstractions. Three similar lines beat a helper used once. -- Don't add features beyond what was asked. Refactoring is the different case: do it when the correct fix requires it, not as a separate pass and not as adjacent cleanup. +- No premature abstractions. Three similar lines beat a helper used once. A rule that must hold for both engines is never "used once": it has two callers by definition, so it belongs in the spine. +- Don't add features beyond what was asked. The second engine is not beyond what was asked (write-once, above). Refactoring is the different case: do it when the correct fix requires it, not as a separate pass and not as adjacent cleanup. - Don't stop at the first site that made the bug visible. "The reported case now passes" is not the same as "the bug is fixed". - No dead code or commented-out blocks. Git has history. -- WHY comments, never WHAT. If code needs a "what" comment, rename instead. Docstrings at module/engine boundaries, not every internal function. +- Comments say WHY: why this approach, why not the obvious one, what breaks otherwise. A WHAT comment is allowed when the what is not visible in the code at hand: an invariant, how an upstream or library dependency behaves, what a magic value means, how this piece couples to a distant one. A comment that restates the adjacent code is dead weight; rename instead. Docstrings at module/engine boundaries, not every internal function. - No em dashes in code, comments, or docs. Use commas, parentheses, periods, or colons. - No AI watermarks: no "Co-Authored-By: Claude", no "Generated with Claude Code", no robot-emoji footers. diff --git a/.claude/rules/engine-layer.md b/.claude/rules/engine-layer.md index 0cffadf..360817d 100644 --- a/.claude/rules/engine-layer.md +++ b/.claude/rules/engine-layer.md @@ -24,9 +24,10 @@ Solverr has two upstreams and owes both a mergeable diff. The ledger - **The stealth clearing core is Byparr's.** It shares Byparr's algorithm and its widget constants by name and role (`ANCESTOR_DEPTHS`, `MIN_WIDTH`, `MIN_HEIGHT`, `MAX_HEIGHT`, `COOLDOWN`), and the ledger's Taken section records four separate ports into it. -- **Everything wrapped around those cores is ours, and it is written twice.** Request-option - handling, session lifecycle and result assembly are each implemented once per engine. That is - where the duplication lives, and it is ordinary duplication with no exemption. +- **Everything wrapped around those cores is ours.** Request-option handling, session lifecycle and + result assembly used to be implemented once per engine, which is where the same defect kept + landing twice. They now live once in the spine (the seam-depth table below); what stays per + engine is each adapter, and duplication there is ordinary duplication with no exemption. **The line runs inside each engine, not between them.** The two cores are two mechanisms, not two implementations of one rule, so collapsing them would fork both engines from their upstream and buy @@ -57,13 +58,22 @@ nothing. recorded in the ledger. "The engines are structured differently", "the other side needs a rewrite first" and "no caller needs it yet" are not exits, they are the work. If the second half cannot ship in the same commit, the change goes back to planning as one item covering both. +- **What a clearing core owns alone falls under the clearing-core decline, not write-once.** A + tuning knob that only parameterizes one core's own loop (`BROWSER_WAIT_TIMEOUT`, Chrome's + per-attempt wait) has nothing to tune on the other engine, and a dependency only one engine uses + (Playwright and invisible-playwright for stealth, Selenium for Chrome) moves with a live check + rather than a second-engine half. Both are documented as engine-specific and recorded in the + ledger, never silent. The moment either changes what a client can observe, write-once applies. - **Sharing the implementation is a means, not the rule.** Declining a code collapse stays allowed on cited mechanism grounds (the two clearing cores are the standing example), and it never licenses a behaviour fork. Two implementations that must behave identically are pinned by one conformance test. - **Divergent bits are typed capability slots.** Never a nullable field, never a boolean-flag combination, never a per-engine branch inside shared code. A capability an engine cannot support - is routed to one that can, or refused by name. Never a silent no-op: `tabs_till_verify` quietly + is routed to one that can, or refused by name. One boolean the spine takes from each adapter is + not a combination and is allowed while it is the only divergent bit on its surface + (`turnstile_is_a_challenge` on `pipeline.verdict`); a second one on the same surface turns both + into one typed capability. Never a silent no-op: `tabs_till_verify` quietly doing nothing on the stealth engine is the defect this rule exists to stop. - **A shared component either derives a piece of state or does not own it.** Sharing the storage while each engine interprets it its own way is a fork wearing shared-code clothing, and nobody @@ -74,7 +84,9 @@ nothing. conformance rung is `src/test_engine_conformance.py`, driven by `src/engine_fakes.py`; add to it rather than writing a second per-engine test, and delete the per-engine test it supersedes. - **Parity is the default; a gap needs a ruling to stay open.** A gap you notice on a surface you are - touching is levelled up in that change unless the owner gates it. + touching is levelled up in that change unless the owner gates it. A gate is the owner's ruling + and is never self-issued by whoever is doing the work. A gap that predates this rule is paid + when its surface is next touched, never in a sweep of its own. - **A decline expires with its evidence.** Record the premise with the decline and treat the decline as void once that premise changes. - **Verify by mutation.** A new test is not done until the production clause it names has been diff --git a/.claude/rules/plan-output.md b/.claude/rules/plan-output.md index abc0be3..93264c1 100644 --- a/.claude/rules/plan-output.md +++ b/.claude/rules/plan-output.md @@ -4,7 +4,7 @@ alwaysApply: true # Plan and findings output format -How a research report or implementation plan is written, whether it comes from `/scout`, `/upstream-audit`, or a plan given directly in conversation. The goal is density: keep every technical claim and every `file:line`, cut the words around them. +How a research report or implementation plan is written, whether it comes from `/scout`, `/code-research`, `/upstream-audit`, or a plan given directly in conversation. The goal is density: keep every technical claim and every `file:line`, cut the words around them. This file governs the structure. [prose-style.md](prose-style.md) governs the sentences inside it. @@ -20,7 +20,7 @@ The single binding constraint in two or three sentences: what actually drives th Grouped **High / Medium / Low**. Each finding is a **bolded one-line claim**, then the shortest prose that carries the evidence, with inline `file:line` references. -Prose, not bullet fragments, and a few tight sentences rather than a paragraph. The claim line states the conclusion; the prose exists only to make it checkable. Mark a finding **verified** when re-read directly, **reported** when it came from a subagent and was not re-read. +Detail is tiered by grade. **High** findings get the evidence prose: a few tight sentences, not a paragraph; the claim line states the conclusion and the prose exists only to make it checkable. **Medium and Low** findings are one line each: the claim plus its citation, no supporting prose. If a Medium finding cannot be stated in one line with a citation, it is either High or it is two findings. Mark a finding **verified** when re-read directly, **reported** when it came from a subagent and was not re-read. ### 3. Stale docs @@ -34,14 +34,14 @@ Omit this section for a pure audit with no implementation to propose. ### 5. Open questions -Last section, numbered, each marked **blocking** or **non-blocking**. Give the concrete options, a recommendation, and the reasoning behind it. These get answered before implementation starts, so a question with no options attached is not finished. +Last section, numbered, each marked **blocking** or **non-blocking**. At most three sentences per question: the question, the options as a short phrase each, the recommendation with a one-clause reason. The full tradeoff discussion happens in conversation only if the owner asks for it. A question with no options attached is not finished. ## Rules - **Density over length.** Every sentence carries a fact the reader does not already have. Cut restatement, throat-clearing, and transitions that only announce what is coming. - **Never drop a `file:line` to save space.** References are the payload, prose is the wrapper. Trim the wrapper. -- **No progress narration in the artifact.** "Now let me check", "Terrain mapped", "Six good returns" are working-log material and never appear in the report. +- **No progress narration, in the artifact or around it.** "Now let me check", "Terrain mapped", "Six good returns" are working-log material. In the report they never appear; in the conversation, one sentence when the fan-out starts, then silence until the report. - **Bullets enumerate options; prose carries findings.** Do not fragment a finding into bullets to look shorter. - **Cite it or drop it.** A claim without a `file:line` from code actually read belongs in Open questions, not Findings. - **No em dashes.** Commas, parentheses, periods, colons. -- Cap the artifact around 1500 words. Longer means the question needed splitting, not that the report needed more room. +- Cap the artifact around 700 words, and treat that as a ceiling, not a target. Longer means the question needed splitting, not that the report needed more room. diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 5ebf81b..73f967e 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -5,9 +5,11 @@ alwaysApply: true # Testing - Verify behavior, not implementation. Don't assert mock call counts when output values would do. -- Run the specific test file after changes, not the full suite. Faster feedback, fewer tokens. +- **A rule that must hold for both engines is pinned once, not twice.** Prefer a test over the shared spine both engines call; where the engines are genuinely separate, write one case in `src/test_engine_conformance.py`, which drives each engine through `src/engine_fakes.py`, instead of a hand-maintained pair. A pair drifts. Full rule: [engine-layer.md](engine-layer.md). +- Run the specific test module after a change (`PYTHONPATH=src uv run --no-project python -m unittest test_request_validation`), then the whole browser-free suite before calling it done: `PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src`. Never a hand-picked list of modules as the final check: it skips the conformance suite. - Flaky test? Fix it or delete it. Never retry to make it pass. -- Prefer real implementations. Mock only at system boundaries (network, filesystem, clock, randomness). -- One assertion per test. Test names describe behavior. Arrange-Act-Assert. No `if` or loops in tests. +- Prefer real implementations. Mock only at system boundaries (network, filesystem, clock, randomness). Patch a polling sleep with a plain function, never a `MagicMock`: the mock records every call and a polling wait turns into unbounded memory that looks like a hang. +- One assertion per test. Test names describe behavior. Arrange-Act-Assert. No `if` or loops in tests; parameterize instead. Here that means `self.subTest` over a fixed tuple of cases or engines, the shape the conformance suite uses, and the one sanctioned loop. - Never assert only that a mock was called without verifying arguments. -- This project uses `unittest` + `webtest` (`src/tests.py`); the full suite launches a real browser and hits live sites, so it's slow and network-dependent. For fast feedback on non-solving changes, prefer `uv run --no-project python -m py_compile ...` and small targeted `unittest` runs over the whole suite. +- **A new test is not done until it has failed.** Delete the production clause it names, see it red, restore the clause (engine-layer.md, "Verify by mutation"). +- `src/tests.py` is upstream's suite: it launches a real browser and hits live sites, so it is slow and network-dependent, and it cannot run in CI. The `src/test_*.py` modules are the browser-free suite CI runs on every pull request. Neither can tell you whether a page still clears a real challenge; that is `/live-check`. diff --git a/.claude/rules/workflow.md b/.claude/rules/workflow.md index 59288bd..fc661f1 100644 --- a/.claude/rules/workflow.md +++ b/.claude/rules/workflow.md @@ -21,9 +21,9 @@ After a code change with any user-facing effect, add a bullet under `## [Unrelea ## Cutting a release (user-initiated) -1. Rename `## [Unreleased]` to `## []`. +1. Rename `## [Unreleased]` to `## []`, and collapse any entries in it that state the same fact: a fix to something added in the same release folds into that addition's entry. 2. Add a fresh empty `## [Unreleased]` above it. -3. Bump `version` in `package.json` to ``, and commit. +3. Bump `version` in `package.json` and the version in the README's response example to ``, and commit. 4. Tag and push: `git tag v && git push origin v`. The tag triggers `release-docker.yml` (builds + pushes the ghcr image) and `release.yml` (creates the GitHub Release from the `[]` section). `release.yml` can also be run manually from the Actions tab (workflow_dispatch) with the version and an optional note. Don't bump the version mid-cycle; only at release-cut. @@ -34,9 +34,11 @@ Create a commit after a change (do not push unless asked). - Subject `type(scope): summary`: a real conventional type (`feat`, `fix`, `docs`, `chore`, `refactor`, `test`, `perf`), imperative, lower-case, no trailing period, `<=72` chars. Scope optional (`chrome`, `stealth`, `sessions`, `docker`). - Non-trivial commits get a body: lead with 1-2 plain-language sentences (what changed and why it matters), then benefit-first bullets. A trivial commit is just the subject. -- No em dashes. No AI watermarks (no `Co-Authored-By: Claude`, no generated-by footer, no robot emoji). +- No em dashes. No AI watermarks (no `Co-Authored-By: Claude`, no generated-by footer, no robot emoji). A `Co-authored-by` trailer for a person is credit and passes the hook; one naming an AI tool is a watermark and is rejected. - **Never a bare `#N`** in the subject or body: it silently links to an issue in this repo. Use the explicit `owner/repo#N` form (`FlareSolverr/FlareSolverr#1626`, `ThePhaseless/Byparr#377`). +**Merging a pull request.** A person merges every pull request, with a merge commit, never squash or rebase. A squash subject ends in ` (#N)`, which the bare-`#N` check rejects on `main`, and a merge commit keeps an outside contributor's commits under their name. The standard for contributors is written out in `CONTRIBUTING.md` and the pull request template; keep both in step with this file. When a contributor's commit message breaks the standard, reword it with `git commit --amend` (which keeps them as the author), push it to their branch with `--force-with-lease`, then merge. Never recommit their change under your own name. + ### Pre-commit checklist Run these against the message before committing. The first four are also enforced by `.githooks/commit-msg`; the rest are on you. @@ -50,7 +52,7 @@ Run these against the message before committing. The first four are also enforce ## Public-facing naming -**Keep the names of the sites Solverr is pointed at out of every public surface**: commit messages, branch names, `README.md`, `CLAUDE.md`, `CHANGELOG.md`, release notes, and the repo description and topics. Solverr is a general-purpose bypass proxy; naming targets makes it read as tooling for one specific site. +**Keep the names of the sites Solverr is pointed at out of every public surface**: commit messages, branch names, `README.md`, `CLAUDE.md`, `CHANGELOG.md`, `CONTRIBUTING.md`, the pull request template, release notes, and the repo description and topics. Solverr is a general-purpose bypass proxy; naming targets makes it read as tooling for one specific site. Use generic wording instead: "a Cloudflare-gated site", "an indexer", "the default mirror", "example-site.tld" in docs and examples. Site names are fine in local test scratch files, in chat, and in a private indexer definition that lives outside this repo. @@ -64,10 +66,11 @@ Inherited exception: `src/tests_sites.py` and `src/tests.py` carry a site list f git config core.hooksPath .githooks ``` -- `commit-msg` enforces the message standard above. -- `pre-commit` lints staged `CHANGELOG.md` and `README.md` for the naming rule, em dashes, and the benefit-first headline format. +- `commit-msg` rejects: a subject that is not `type(scope): summary`; a subject over 72 characters; an em dash anywhere; an AI watermark (an AI `Co-authored-by` trailer, "Generated with", the robot emoji); a bare `#N`; a domain-shaped site name or scraping vocabulary. Merge, revert, fixup and squash commits pass untouched. +- `pre-commit` lints the lines a commit adds to `CHANGELOG.md`, `README.md`, `CONTRIBUTING.md` and `CLAUDE.md` for the naming rule and em dashes, and every `[Unreleased]` entry outside `Other` for a bold headline ending in `.`, `!` or `?`. +- `.githooks/tests/run.sh` proves each rule above still rejects a real violation and passes a clean case. Run it after touching either hook. -Never bypass with `--no-verify`. If a hook fires on something legitimate, fix the hook in the same change. +CI runs all of it: the Standards workflow runs the hook self-test, then `commit-msg` on every non-merge commit and `pre-commit` over the pushed range, and the Tests workflow runs the browser-free suite on every pull request. Never bypass with `--no-verify`. If a hook fires on something legitimate, fix the hook and its self-test in the same change. ## Approach diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index a3c3a5b..b845cbc 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -12,7 +12,7 @@ See [CONTRIBUTING.md](../CONTRIBUTING.md) for what each item means. - [ ] The browser-free suite passes: `PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src` - [ ] Commit messages follow `type(scope): summary`, at most 72 characters, with no em dash, no bare `#N`, and no AI attribution -- [ ] No names of target sites and no scraping vocabulary in commits, docs, or code comments +- [ ] No names of target sites in commits, docs, or code comments (see CONTRIBUTING.md for the words the check also rejects) - [ ] A change a client can notice lands for both engines, or the pull request names the capability one engine does not have - [ ] The `/v1` request and response shape is unchanged, apart from new optional fields - [ ] `CHANGELOG.md` has an `[Unreleased]` entry if someone running Solverr could notice the change, and none otherwise diff --git a/CLAUDE.md b/CLAUDE.md index 7871a63..9fc2004 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,58 +7,64 @@ FlareSolverr fork with two solving engines and automatic fallback. Cloudflare/DD ```bash docker compose up -d --build # build + run (image bundles both browsers, ~2.3 GB) docker logs -f solverr # logs (set LOG_LEVEL=debug for more) +PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src # browser-free suite, seconds; CI runs it +bash .githooks/tests/run.sh # the git hooks still reject what they claim to +bash .claude/hooks/tests/run-all.sh # the Claude Code guard hooks, against their fixtures uv run --no-project python -m py_compile src/*.py src/engines/*.py # quick compile check -uv run python -m unittest src.tests # test suite (unittest + webtest; needs a browser) ``` -## Architecture (non-obvious) +Python only through uv; there is no system Python. `src/tests.py` is upstream's suite: it needs a real browser and live sites. Whether a page still clears a real challenge is `/live-check`, never the unit tests. -- Two engines behind one interface (`engines/base.py`): `chrome` (Selenium + vendored undetected_chromedriver, the default) and `stealth` (Camoufox via invisible_playwright + playwright-captcha). The controller auto-falls-back between them and remembers per-host which one cleared it. -- The stealth engine is async Playwright running on ONE background event-loop thread (`async_runtime.py`); persistent Camoufox contexts (sessions) live there so their cookies survive across requests. The server itself is synchronous. -- Sessions: each engine keeps its own pool, both using one `SessionStore` (`sessions.py`) so the lifecycle rules exist once; a background reaper (`session_reaper.py`) closes idle browsers. A session handed out is marked in use under the same lock that found it, which is what keeps the reaper and the cap off a live browser. Solve once, reuse the cookie many times. -- Escalation ladder for an `auto` request: Chrome → Camoufox click-solve → (optional, dormant) paid CAPTCHA API. -- **A Turnstile checkbox is clicked by coordinate, with no JS evaluation.** The widget's iframe sits in a closed shadow root, so `query_selector` cannot find it, but `page.frames` lists it anyway; `frame_element().bounding_box()` gives its rect and `page.mouse` clicks the checkbox. This exists because playwright-captcha's shadow-root traversal uses `evaluate_handle`, and the iframe's CSP blocks eval under Firefox, which silently broke widget solving. Cloudflare's own interstitial builds the widget itself and its frame reports an empty URL, so when no frame matches, the rect comes from the nearest ancestor `div` of the token input instead, which is in the light DOM. Do not reach for `page.evaluate` to measure any of this: running page scripts against a live challenge makes Cloudflare reissue it. -- **A challenge is only over once a clear reading survives a second look.** Cloudflare drops the challenge markup while it issues the next round, so believing the first clear reading returns an intermediate challenge page. -- **`maxTimeout` is one budget for the whole request, split evenly across the planned engines.** It used to be handed to each engine in full, so a fallback could take twice as long as asked and trip the caller's own timeout. An even share is what makes the fallback reachable: giving the first engine everything let it spend the lot, and a request that used to succeed in 133s failed at 120s with the second engine skipped. A quick first engine costs the fallback nothing, since the fallback inherits everything unspent. -- **The POST form is carried to the browser as a `data:text/html,` URL, so its fields are percent-encoded and must stay that way** (`postform.py`). The browser URL-decodes the document before the HTML parser sees it, so a value holding a bare `%` or `#` is otherwise re-read as an escape or truncates the document at the fragment. The `quote()` calls look like double-encoding and are not: removing them breaks POST for those values, measured against a live echo service. -- **Solverr resolves the browser's timezone and language itself (`geo.py`), and hands both engines the same pair.** Left alone, the stealth stack resolves both from the exit IP on every launch, inside the library, uncached, and raises behind a proxy when the lookup fails, which kills the launch; Chrome derived neither, so the two engines disagreed about the country. Passing concrete values returns before that fatal branch. They travel together because the pairing is what a site checks. Chrome follows via `Emulation.setTimezoneOverride` (which moves its ICU clock rather than patching `Intl` in the page) and `--accept-lang`. A failed lookup falls back to `TZ` and `en-US`: a wrong zone still solves, no browser does not. -- **playwright-captcha only ever touches a throwaway page**, and only the paid escalation reaches it now. Preparing a solver injects init scripts (one rewrites `Element.prototype.attachShadow`) that a Cloudflare interstitial will not clear while they are present, and Playwright cannot remove an init script. Verified live: an interstitial clears in ~3s without them and never in 40s with them. +## Working approach + +- **Memory and `Handoff.md` are hypotheses, not facts.** A memory that names a function, file or flag is true only if it still exists in current code. When one turns out stale, surface it for pruning instead of acting on it. +- **Plan steps carry their check inline**, as `1. -> verify: `, so a step nothing can check is visible before it is built. +- **Reply length.** Default replies are a few sentences: the answer or outcome, the detail that matters, done. A full report is for when the owner asks for one, or for a `/scout` or `/code-research` deliverable, which has its own cap in [.claude/rules/plan-output.md](.claude/rules/plan-output.md). + +## Architecture in brief + +Two engines behind one interface: `chrome` (Selenium + vendored undetected_chromedriver, the default) and `stealth` (Camoufox via invisible_playwright, on one background event-loop thread). The controller falls back between them and remembers per host which one cleared it. What both engines must do the same way lives once in the shared spine (`assembly.py`, `pipeline.py`, `budget.py`, `sessions.py`); each engine is an adapter over a clearing core derived from its upstream. Several constraints look wrong until you know what they were measured against (the coordinate Turnstile click, no `page.evaluate` against a challenge page, the second look before a challenge counts as cleared, the even `maxTimeout` split, the `quote()` calls in `postform.py`): read [.claude/rules/architecture.md](.claude/rules/architecture.md) before touching any of them. It loads on its own when you work in `src/`. ## Key decisions (WHY) -- **Fork on FlareSolverr, not Byparr.** FlareSolverr's Chrome engine already clears the target sites and has sessions; Python 3.11 + a vendored undetected_chromedriver let the Camoufox/Playwright stack coexist. Byparr pins Python 3.14, too new for undetected_chromedriver. +- **Fork on FlareSolverr, not Byparr.** FlareSolverr's Chrome engine already clears the target sites and has sessions, and its vendored undetected_chromedriver lets the Camoufox/Playwright stack run beside it in one Python 3.14 image. - **Reliability is dominated by IP reputation, not the tool.** A residential proxy (`PROXY_URL`) is the biggest lever; warm-session cookie reuse is the second. - **The consuming client keeps one shared session and never destroys it**, so the server-side reaper is what prevents leaked browsers (especially the heavier Camoufox ones). ## Where things live -- `src/flaresolverr.py` — entrypoint: logging setup (note the `force=True`), server, reaper start. -- `src/flaresolverr_service.py` — controller: `/v1` commands, engine selection + fallback, per-host memory, session commands. -- `src/assembly.py`, `src/pipeline.py`, `src/budget.py` — the shared spine: what a response contains and in what order, the page verdict and the navigate-cookies-reload order, the solve deadline. The first two are sans-io generators (they yield what to read, the engine supplies how) because one engine is synchronous and the other asynchronous; see `.claude/rules/engine-layer.md` before reshaping them. -- `src/engines/` — `base.py` (Engine + SolveResult), `chrome_engine.py`, `stealth_engine.py`. Each is an adapter over an upstream-derived clearing core, which is the one thing the spine never takes over. -- `src/async_runtime.py`, `src/session_reaper.py`, `src/sessions.py` — stealth event loop, idle reaper, and the `SessionStore` both engines use (each holds its own instance; the lifecycle rules live once). -- `src/detection.py` (shared challenge/title/selector lists), `src/geo.py` (browser timezone for both engines), `src/config.py` (env, including `env_proxy`), `src/postform.py`, `src/dtos.py` (request DTOs plus the type validation that makes their annotations binding). -- `src/engine_fakes.py` — drives either engine browser-free from one neutral `World`, for `test_engine_conformance.py`. Imported, not collected. -- `.claude/rules/engine-layer.md` — **the law for anything touching an engine**: write-once and its one exit, which code is upstream's and which is ours, capability slots, the pin-once ladder, and how deep the seam goes per surface. Loads every session. -- `.claude/rules/workflow.md` — CHANGELOG + commit rules, release-cut, public-facing naming, git hooks. `code-quality.md` — coding principles. `security.md` / `error-handling.md` — path-scoped to `src/`. `plan-output.md` — how a findings report or plan is structured. `prose-style.md` — sentence-level writing for every output. -- `docs/dev/engine-layer-architecture.md` — the rationale behind that law: the divergence measurements against both upstreams, the target seam, the sequencing, and every ruling with the evidence it rests on. Read it before designing anything forward-looking. -- `docs/dev/upstream-sync.md` — what has been taken from FlareSolverr and Byparr, through which commit, and every deliberate divergence with its reasoning. Read it before calling something drift. -- `docs/dev/loops.md` — the port loop's contract: what the manager and worker each own, what they may not do, the three verification gates, and the eligibility rules that keep the worker away from the engines. -- `.githooks/` — tracked commit-msg and pre-commit hooks. Activate with `git config core.hooksPath .githooks`. +- `src/flaresolverr.py`: entrypoint. Logging setup (note the `force=True`), server, reaper start. +- `src/flaresolverr_service.py`: controller. `/v1` commands, engine selection and fallback, per-host memory, session commands. +- `src/assembly.py`, `src/pipeline.py`, `src/budget.py`: the shared spine. What a response contains and in what order, the page verdict and the navigate-cookies-reload order, the solve deadline. The first two are sans-io generators (they yield what to read, the engine supplies how) because one engine is synchronous and the other asynchronous; see `.claude/rules/engine-layer.md` before reshaping them. +- `src/engines/`: `base.py` (Engine + SolveResult), `chrome_engine.py`, `stealth_engine.py`. Each is an adapter over an upstream-derived clearing core, which is the one thing the spine never takes over. +- `src/async_runtime.py`, `src/session_reaper.py`, `src/sessions.py`: stealth event loop, idle reaper, and the `SessionStore` both engines use (each holds its own instance; the lifecycle rules live once). +- `src/detection.py` (shared challenge/title/selector lists), `src/geo.py` (browser timezone and language for both engines), `src/config.py` (env, including `env_proxy`), `src/postform.py`, `src/dtos.py` (request DTOs plus the type validation that makes their annotations binding). +- `src/engine_fakes.py`: drives either engine browser-free from one neutral `World`, for `test_engine_conformance.py`. Imported, not collected. +- `.claude/rules/engine-layer.md`: **the law for anything touching an engine**. Write-once and its one exit, which code is upstream's and which is ours, capability slots, the pin-once ladder, and how deep the seam goes per surface. Loads every session. +- `.claude/rules/architecture.md`: the non-obvious architecture and its measured constraints, path-scoped to `src/`. +- `.claude/rules/workflow.md`: CHANGELOG and commit rules, merging, release-cut, public-facing naming, the git hooks and every check they run. `code-quality.md`: coding principles. `testing.md`: test rules and commands. `security.md` / `error-handling.md`: path-scoped to `src/`. `plan-output.md`: how a findings report or plan is structured. `prose-style.md`: sentence-level writing for every output. +- `CONTRIBUTING.md` and `.github/pull_request_template.md`: the same standard, written for outside contributors. Keep them in step with `workflow.md`. +- `docs/dev/engine-layer-architecture.md`: the rationale behind the law. The divergence measurements against both upstreams, the target seam, the sequencing, and every ruling with the evidence it rests on. Read it before designing anything forward-looking. +- `docs/dev/upstream-sync.md`: what has been taken from FlareSolverr and Byparr, through which commit, and every deliberate divergence with its reasoning. Read it before calling something drift. +- `docs/dev/loops.md`: the loops' contract. What the managers and the worker each own, what they may not do, the three verification gates, and the eligibility rules. +- `.githooks/`: tracked `commit-msg` and `pre-commit` hooks, plus `tests/run.sh`, which proves each rule still rejects a real violation. Activate with `git config core.hooksPath .githooks`. CI runs the same checks (the Standards and Tests workflows). +- `.claude/hooks/`: the guards that screen tool calls before they run, so an unexplained `Blocked:` message comes from here. `block-dangerous-commands.sh` covers **both Bash and PowerShell** (matching only one lets a command through the other tool) and refuses a push to `main`, a bare force push (`--force-with-lease` is allowed), merging a PR (`gh pr merge` or through `gh api`), reading secret files through the shell, and the usual destructive deletes. Merging is always the owner's call. +- `.claude/agents/`: the four review subagents to spawn with the `Agent` tool: `code-reviewer`, `doc-reviewer`, `performance-reviewer`, `security-reviewer`. `/pr-review` runs all four in parallel. ## Skills -- `/scout` — investigate one non-trivial task, then produce its plan, grounded in `file:line` citations. Use before porting from an upstream or touching the engines, sessions, or the controller. -- `/upstream-audit` — compare against FlareSolverr and Byparr, classify every difference as covered, missing, or deliberate, and check `/v1` compatibility. Updates the sync ledger. -- `/port-scan` — manager for the upstream port loop. Triages new Byparr and FlareSolverr commits into labeled issues. No file-writing tools by design. `--dry-run` files nothing. -- `/audit-scan` — manager for the audit and bug-fix loop. Audits one dimension per run and files only findings that survived an attempt to refute them. Same containment. `--dry-run` files nothing. -- `/loop-work` — the worker both managers feed. Takes one `loop:ready` issue, works it in its own worktree and branch, fixes every site the issue lists, proves it with three gates, opens a draft PR. Never merges. `--dry-run` mutates nothing. -- `/live-check` — verify a change against live challenges through an isolated container. The unit tests cannot tell you whether a page still clears; this can. -- `/release` — cut a version end to end: decide the bump, preflight, tag, then verify the workflows and the published image digests. -- `/session-handoff` — rewrite `Handoff.md` from verified state, then bring the CHANGELOG, dependent docs, and memory store in line with it. -- `/pr-review` — review changes via the four specialist agents in parallel. -- `/tighten` — trim verbose docs and WHAT comments without losing vital info. Always plans first. -- `/context-budget` — what this `.claude/` config costs per turn. +- `/scout`: investigate one non-trivial task, then produce its plan, grounded in `file:line` citations. Use before porting from an upstream or touching the engines, sessions, or the controller. +- `/code-research`: fan-out research for a broad question spanning many files. `/scout` is for one concrete task. +- `/upstream-audit`: compare against FlareSolverr and Byparr, classify every difference as covered, missing, or deliberate, and check `/v1` compatibility and the dependency pins. Proposes the ledger update. +- `/port-scan`: manager for the upstream port loop. Triages new Byparr and FlareSolverr commits into labeled issues. Writes no files, by rule. `--dry-run` files nothing. +- `/audit-scan`: manager for the audit and bug-fix loop. Audits one dimension per run and files only findings that survived independent attempts to refute them. Same containment. `--dry-run` files nothing. +- `/loop-work`: the worker both managers feed. Takes one `loop:ready` issue, works it in its own worktree and branch, fixes every site the issue lists, proves it with three gates, opens a draft PR. It may change the spine and the adapters; either clearing core always goes to a person. Never merges. `--dry-run` mutates nothing; `--resume ` carries review feedback back into an open loop PR. +- `/live-check`: verify a change against live challenges through an isolated container. The unit tests cannot tell you whether a page still clears; this can. +- `/release`: cut a version end to end: decide the bump, preflight, tag, then verify the workflows and the published image digests. +- `/session-handoff`: rewrite `Handoff.md` from verified state, then bring the CHANGELOG, dependent docs, and memory store in line with it. +- `/pr-review`: review changes via the four specialist agents in parallel. +- `/tighten`: trim verbose docs and WHAT comments without losing vital info. Always plans first. +- `/context-budget`: what this `.claude/` config costs per turn. ## Don'ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ff2282a..a60d365 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -57,7 +57,7 @@ CI checks every commit in a pull request with [.githooks/commit-msg](.githooks/c - **No em dash anywhere in the message.** Use commas, parentheses, periods, or colons. - **No bare `#123`.** It silently links to an issue in this repository. Write `owner/repo#123` instead, for example `FlareSolverr/FlareSolverr#1626`. - **No AI attribution.** No `Co-authored-by` trailer naming an AI tool and no "Generated with" footer. A `Co-authored-by` trailer for a person is fine. -- **No names of the sites you point Solverr at, and no scraping vocabulary.** Write `example-site.tld` or "a Cloudflare-gated site" instead. +- **No names of the sites you point Solverr at.** Write `example-site.tld` or "a Cloudflare-gated site" instead. The check also rejects a few words that would make Solverr read as a tool built for harvesting one site; `.githooks/commit-msg` lists them. - **A change that is more than a one-liner gets a body.** Lead with one or two plain sentences on what changed and why it matters, then bullets. For example, `Fixed the cookie bug.` is rejected, and `fix(stealth): accept a cookie without a domain` passes. diff --git a/docs/dev/engine-layer-architecture.md b/docs/dev/engine-layer-architecture.md index 185020d..e908db2 100644 --- a/docs/dev/engine-layer-architecture.md +++ b/docs/dev/engine-layer-architecture.md @@ -246,11 +246,11 @@ copy cannot drift from the law. `Reactor` that ships alongside it is deliberately not used, since it starts a polling thread per driver. Three caveats bound the work: the capability is set at driver creation so it is a config decision rather than a per-request one, the log must be drained per request or it grows on a - long-lived session, and it needs a fingerprint A/B before shipping because it is on by default. -- **Identity is a sealed `SessionRef`, not a bare id string.** The two pools can hold the same id and - `_cmd_sessions_list` already deduplicates them at runtime, which is the symptom. A bare string - cannot express which engine owns a session, so the wrong-pool lookup stays constructible until the - type says otherwise. + long-lived session, and it needs a fingerprint A/B before it could ever be on by default. It + shipped off by default for that reason. +- **Identity stays a bare id string.** A sealed `SessionRef` was proposed because the two pools can + hold the same id, and dropped once measured: the `/v1` contract has clients send a bare id, and + resolving which engine holds one is the controller's job. Step 5 above records the full reason. - **The conformance suite is the pin, and the spine is the kernel.** Both rungs of the ladder are available here because the codebase is small enough, so an unpinned twin should not exist at all. - **Sequencing puts characterisation second, not last.** `/live-check` is user-invoked and takes tens diff --git a/docs/dev/loops.md b/docs/dev/loops.md index b69ec18..44f6f6b 100644 --- a/docs/dev/loops.md +++ b/docs/dev/loops.md @@ -58,9 +58,9 @@ Accountability sits with the person operating this. Every merge is a human decis The first draft of these rules made `loop:ready` mean "at most one file under `src/`". That is exactly the rule that produces patchwork: it rewards fixing the one call site that made the bug visible and leaving its siblings alone, and it makes a well-understood four-file change ineligible while an unexamined one-file change sails through. -Eligibility is about **how well the scope is known**, never how small it is. Every issue carries a checklist of every affected site with `file:line`, plus the search that produced it, and the worker re-runs that search before calling the code done. Three outcomes are acceptable: fix every site, fix some and list the rest in the PR body with the reason, or escalate. Silence about a site is not one of them. +Eligibility is about **how well the scope is known**, never how small it is. Every issue carries a checklist of every affected site with `file:line`, plus the search that produced it, and the worker re-runs that search before calling the code done. Three outcomes are acceptable: fix every site, fix some and list the rest in the PR body with the reason, or escalate. Silence about a site is not one of them. An engine pair is the exception to the middle option: a change a client can observe lands for both engines in the same commit, or cites the mechanism one engine genuinely lacks (`.claude/rules/engine-layer.md`), or escalates. -Solverr has two engines written against each other, so a defect in one usually has a twin in the other. That is the single most common way a fix here ends up half done. +The rules both engines must share now live once in the spine (`src/assembly.py`, `src/pipeline.py`, `src/budget.py`, `src/sessions.py`), so a fix there reaches both from one site. What stays per engine is each adapter and each clearing core, and a fix that lands in one of those and not the other is still the most common way work here ends up half done. Refactoring is in scope when the correct fix needs it, in the same change, with the reason in the commit body. What stays out is adjacent cleanup nothing in the issue motivates. See `.claude/rules/code-quality.md`. @@ -68,7 +68,7 @@ Refactoring is in scope when the correct fix needs it, in the same change, with Cheapest first, so a run fails fast. -**Gate A, the browser-free suite.** 138 tests, no browser, seconds. Pass or fail, no interpretation. +**Gate A, the browser-free suite.** The whole suite (`unittest discover`, never a hand-picked module list), no browser, seconds. Pass or fail, no interpretation. **Gate B, the live solve tally.** The one gate that cannot be boolean. Cloudflare's behavior varies with IP reputation, time of day, and how hard a host was hit five minutes ago, so a single result carries no information either way. The worker builds a baseline container off `origin/main` and runs it interleaved with the change, trial for trial, in the same window. Within noise is a pass, clearly worse is a fail, and an ambiguous window opens the PR labeled `needs-live-recheck` with the raw numbers. Asking a person is a valid outcome; a confident verdict off one sample is not. @@ -99,13 +99,13 @@ Dedupe on the `source:` labels, never on the `loop:` ones. Repeated `--label` fl ## Tripwire zones -`loop:needs-human`, always, whichever manager finds it: the widget measuring and click path in `stealth_engine.py`, the shared `maxTimeout` budget split, the `quote()` calls in `postform.py`, session and reaper lifecycle, `geo.py`, and any dependency pin for the browser stack. `Handoff.md`'s "What failed" section is the list of conclusions a confident agent reaches and gets wrong, so it is also the list of things the worker may not reason about alone. +`loop:needs-human`, always, whichever manager finds it: either clearing core (the Chrome challenge wait and Turnstile path in `chrome_engine.py`, the stealth clearing loop and the widget measuring and click path in `stealth_engine.py`; `/loop-work` names the functions), the shared `maxTimeout` budget split, the `quote()` calls in `postform.py`, session and reaper lifecycle, `geo.py`, and any dependency pin for the browser stack. `Handoff.md`'s "What failed" section is the list of conclusions a confident agent reaches and gets wrong, so it is also the list of things the worker may not reason about alone. ## Guards The worker can write code and push a branch, so the guards around it are worth stating. -`.claude/hooks/block-dangerous-commands.sh` blocks pushes to protected branches, force pushes, and destructive operations. It matches **both** the Bash and the PowerShell tool: matching only Bash left every guard bypassable by rewriting the same command in PowerShell, which is a different tool with a different name and its own spelling for every destructive operation. Fixtures under `.claude/hooks/tests/fixtures/` cover both syntaxes; run them with `bash .claude/hooks/tests/run-all.sh`. +`.claude/hooks/block-dangerous-commands.sh` blocks pushes to protected branches, force pushes, merging a PR, shell reads of secret files, and destructive operations. It matches **both** the Bash and the PowerShell tool: matching only Bash left every guard bypassable by rewriting the same command in PowerShell, which is a different tool with a different name and its own spelling for every destructive operation. Fixtures under `.claude/hooks/tests/fixtures/` cover both syntaxes; run them with `bash .claude/hooks/tests/run-all.sh`. The commit-msg and pre-commit hooks are never bypassed. `--no-verify` is not an option the worker has. @@ -135,4 +135,4 @@ Both run locally and only while the machine is on. Gates B and C need Docker and ## What these loops do not do -They do not merge, release, or decide that a divergence should end. They do not touch the engines without a person in the path. They do not run the paid CAPTCHA escalation, which needs an API key, and they do not cover a Cloudflare-gated PDF, because no such URL has been found. Those stay in the "not covered" section of every PR body the worker writes. +They do not merge, release, or decide that a divergence should end. They do not touch either clearing core without a person in the path; the spine and the engine adapters are in scope for the worker, subject to the tripwire zones above. They do not run the paid CAPTCHA escalation, which needs an API key, and they do not cover a Cloudflare-gated PDF, because no such URL has been found. Those stay in the "not covered" section of every PR body the worker writes. diff --git a/docs/dev/upstream-sync.md b/docs/dev/upstream-sync.md index 0419c57..330a72f 100644 --- a/docs/dev/upstream-sync.md +++ b/docs/dev/upstream-sync.md @@ -37,7 +37,7 @@ Cite one of these instead of re-arguing it. Change one only when the owner asks. - **`LANG` rather than a new `BROWSER_LOCALE`.** Byparr added `BROWSER_LOCALE` (`8cb5770`) because it had no language variable. Solverr inherited `LANG` from FlareSolverr, wired to Chrome's `--accept-lang` (`src/utils.py`), so a second variable would have meant one knob per engine. `LANG` now feeds both through `config.browser_locale()`. It is also normalized rather than forwarded: `invisible_core/prefs.py` only maps `_` to `-` and appends the base subtag, so a raw `LANG=en_US.UTF-8` would set `navigator.languages = ["en-US.UTF-8", "en"]` and a matching `Accept-Language`, which is a more distinctive fingerprint than leaving it unset. Values that are not language tags are dropped with a warning. - **The browser language comes from the same lookup as the timezone.** Byparr resolves a locale from the exit country and Chrome has never resolved one at all, so with nothing configured the two engines answered in different languages (measured 2026-08-13: Chrome `en-US,en`, Camoufox `nb-NO,nb` from the same Norwegian exit), and `ENGINE_FALLBACK` made it change mid-session. Neither value was wrong; disagreeing was. `src/geo.py` now takes the exit IP out of `prepare_session_geo` and feeds it to `resolve_session_locale`, so both are derived from one address and cannot name different countries. That is one round trip behind a proxy and two on a direct connection, where the library reports no `egress_ip` (the field exists for the WebRTC override) and the locale resolver looks the address up again; the result is cached per proxy either way, so it is per process rather than per launch. That pairing is the point: the library's own `_warn_locale_fallback` comment says a locale falling back to `en-US` while the timezone resolves is "a cross-field inconsistency of exactly the kind the timezone trap exists to prevent". Chrome also gets the `tag, base` pair rather than a bare tag, since `--accept-lang` is passed through verbatim and produced a one-entry `navigator.languages` no desktop browser sends. - **Solverr resolves the browser timezone itself, for both engines.** Byparr leaves it to `invisible_playwright`, which resolves from the exit IP on every launch inside the library. Measured on 2026-08-13 against the pinned version: an address lookup under a 15 second budget plus a 53 MB geoip download, 8 seconds cold, no caching anywhere in `invisible_core/_geo.py`, and behind a proxy `prepare_session_geo` raises on a failed lookup and takes the launch with it. Solverr resolves in `src/geo.py`, once per proxy and cached, and passes a concrete `timezone=`, which returns from `prepare_session_geo` before that fatal branch. Behind a proxy the library still makes one lookup of its own for the WebRTC override, but non-fatally. Chrome gets the same zone through `Emulation.setTimezoneOverride`, chosen over a JS patch because it moves the browser's own ICU clock and leaves nothing in the document looking rewritten, and over the `TZ` environment variable because `uc.Chrome()` exposes no per-process environment and a process-global would race concurrent launches on different proxies. FlareSolverr has no timezone handling at all, so nothing to reconcile there. -- **`STEALTHFOX_GEOIP_MMDB` is set in the image, and it is not a public API.** The geoip database is baked at build time and pinned with this variable, because `ensure_geoip_mmdb` re-checks for a newer build on every call and would download over the baked copy. The variable belongs to `invisible_core` 18.13.0, held by the exact `invisible-playwright==0.6.1` pin, so re-check the name whenever that pin moves. It is the only thing here depending on a dependency's internals. +- **`STEALTHFOX_GEOIP_MMDB` is set in the image, and it is not a public API.** The geoip database is baked at build time and pinned with this variable, because `ensure_geoip_mmdb` re-checks for a newer build on every call and would download over the baked copy. The variable belongs to `invisible_core` (20.15.0, held by the exact `invisible-playwright==0.7.2` pin), so re-check the name whenever that pin moves. It is the only thing here depending on a dependency's internals. - **`BROWSER_GEO` reads the system tzdata table, with four overrides.** Country to timezone comes from `/usr/share/zoneinfo/zone1970.tab` rather than a list kept in the repo, since a hand-kept list is how Camoufox ended up returning wrong zones (`daijro/camoufox#589`). Two things measured against the real file on 2026-08-13 that its header does not prepare you for: a row lists every country sharing a zone and only the first is the one it speaks for (crediting them all put Germany in `Europe/Zurich`, because the Swiss row sorts first), and the promise to put "the most populous timezones first" is overridden by geography for 4 of the 24 multi-zone countries, which tzdata orders east to west. `_POPULATION_ZONES` in `src/geo.py` corrects Brazil, Russia, Australia and Canada, and an override applies only when the system confirms the zone exists. - **`BROWSER_WAIT_TIMEOUT` lives in `config.py` and falls back instead of raising.** Upstream put `get_config_browser_wait_timeout` in `utils.py` and parses with a bare `int()`, which raises on a value that is not a number. Two reasons not to copy that. It is read inside `_evil_logic`, so upstream's raise happens mid-solve, where the engine wraps it as "Error solving the challenge", the controller falls back to the other engine, and nobody learns the variable is malformed; `config._int_env` defaults to 1 instead, matching every other integer knob here. Upstream also reads it a second time at startup purely to make a bad value fatal there, and that line is unused, which this fork does not carry. The cost is that `utils.py` now diverges from upstream by one absent function on top of the `LANG` normalization, so expect that hunk when diffing. - **The Chrome Turnstile path waits for the widget and bounds its retry loop.** Upstream's `_resolve_turnstile_captcha` (`src/flaresolverr_service.py`, unchanged through `3ae649a`) reads `TURNSTILE_SELECTORS` the instant `driver.get()` returns and then retries in a `while True` with no deadline. Both halves are wrong together, and each one hid the other: `driver.get()` returns at `readyState complete`, so a widget injected by Cloudflare's api.js is not there yet, and a request that missed it answered "Challenge not detected!" with an empty token. Measured 2026-08-25 with a bare-browser probe in the container, over four samples across two demo pages: the token input appeared 0.01s to 0.92s after `get()` returned, and never on a page with no widget. Solverr waits `_WIDGET_RENDER_SECONDS` (5, the same grace the stealth engine gives through its networkidle settle), which is what makes the unbounded loop reachable often enough to matter, so the loop now takes a deadline of `maxTimeout` minus a margin and returns `None` rather than raising. Two smaller repairs ride along because they are the same twenty lines: the token is resolved after the cookie reload rather than before it (a reload replaces the document the token described), and the input is re-located on every read rather than held across passes (a re-render raised `StaleElementReferenceException` out of the whole request). Not offered upstream, by the owner's call. @@ -48,14 +48,14 @@ Cite one of these instead of re-arguing it. Change one only when the owner asks. ## Inherited from FlareSolverr, byte-identical -These need no review until upstream changes them. Verified identical on 2026-07-25: +These need no review until upstream changes them. Verified identical on 2026-09-11: - `src/undetected_chromedriver/` (the whole vendored package) - `src/tests.py`, `src/tests_sites.py` (they carry upstream's site list; leave as-is so the files stay mergeable, and do not add to them) - `html_samples/*.html` -- `src/bottle_plugins/` +- `src/bottle_plugins/`, except `prometheus_plugin.py`, which caps the domain label (`_MAX_DOMAIN_LABELS`) so an unbounded set of hosts cannot grow the Prometheus registry without limit -`src/utils.py` was on this list until 2026-08-13 and now carries two divergences: the `LANG` normalization below (`get_webdriver`, four comment lines and one call), and the absence of upstream's `get_config_browser_wait_timeout`, which lives in `config.py` here for the reason given below. Everything else in it is still upstream's, so diff it with `--strip-trailing-cr` before assuming otherwise: the working copy is CRLF and FlareSolverr's is LF, so a plain `diff` reports every line as changed. +`src/utils.py` was on this list until 2026-08-13 and now carries two divergences: the `LANG` normalization below (`get_webdriver`, four comment lines and one call), and the absence of upstream's `get_config_browser_wait_timeout`, which lives in `config.py` here for the reason given below. Everything else in it is still upstream's. Diff it with `--strip-trailing-cr` before assuming otherwise: `.gitattributes` now checks text out as LF, like FlareSolverr, but a checkout made before it may still be CRLF, and then a plain `diff` reports every line as changed. ## Taken @@ -67,17 +67,17 @@ These need no review until upstream changes them. Verified identical on 2026-07- - **The stealth browser from PyPI, pinned exactly.** Byparr moved `invisible-playwright` off a git dependency to a PyPI floor of `>=0.6.1` (`a0c4b1d`, `e298eb8`) and its lock now resolves 0.7.2. Solverr followed to `==0.7.2` (v1.4.0), which pins `invisible-core==20.15.0` outright; `STEALTHFOX_GEOIP_MMDB`, `prepare_session_geo`, `resolve_session_locale`, `ensure_geoip_mmdb` and the `locale`/`timezone` launch arguments were all re-checked against it and are unchanged, as is the `_` to `-` locale normalization that `config._language_tag` exists to get ahead of. Solverr pins exactly rather than to a floor, for the reason the original commit pin existed: this package carries the patched Firefox, so an unattended bump changes solving behavior while the build stays green. A floor is not a pin, which is why moving it is a release with a live check attached rather than a dependency bump. That commit pin was never as reproducible as it looked either: it fixed `invisible-playwright` at 0.3.0 but left `invisible-core` to float. - **Reading the Turnstile token off the value property** (v1.4.0). Byparr reads it with `input_value` (`challenge.py`), which is `element.value`; Solverr read it with Playwright's `get_attribute`, which is `element.getAttribute` and so returns the `value` content attribute. Taken, but for a narrower reason than it first looked: the expectation was that a token assigned to `input.value` never reaches the attribute, and measured against the live widget on 2026-08-18 both reads returned the same token, so the old read was not broken for Cloudflare's own widget. It is broken for a token the paid escalation injects, which playwright-captcha assigns to the property alone (`appliers/applyCloudflareTurnstile.js`). Parity with Selenium's `get_attribute`, which returns the property, is the rest of the reason. **No user-visible effect on the free path**, so no CHANGELOG entry. -- **Cookies read after `waitInSeconds`, on both engines** (unreleased). FlareSolverr moved its cookie read below the wait and the screenshot (`3ae649a`) so a page that sets cookies from its own JavaScript during that wait is not read too early. Solverr had the same defect twice, once per engine (`chrome_engine.py`, `stealth_engine.py`), because the two were written against each other. Fixed in both. `returnOnlyCookies` never waits, so it is unaffected either way, and the read stays outside that branch so it still returns cookies. `test_response_shape.py` covers the Chrome side with a fake driver whose jar changes while the wait runs; the stealth side is the same edit against an async context and rests on the live check. -- **A single reusable focus helper in the Chrome turnstile loop** (unreleased). `_get_turnstile_token` prepended a fresh `