End-to-end review follow-ups: privacy, workspace, admin split, audience split#4
Merged
Conversation
Six low-risk changes that close fingerprinting and information-disclosure
holes the .onion-facing daemon was leaking by default. Each is independently
load-bearing:
- Strip the `X-Service: TorPC` response header. Anyone scanning .onion
services with a HEAD request could identify a TorPC node from a single
header check. The point of an .onion service is *not* to advertise the
software running behind it.
- Sanitize upstream Geth errors. The error path used to format
`"Geth returned status {} : {body}"` and surface that to the JSON-RPC
caller. Geth's bodies leak version strings, EIP support, and internal
paths — all available to anonymous Tor visitors. The full body is now
logged locally for operator debugging; the caller sees only a generic
`"upstream returned status NNN"`.
- Skip the upstream circuit breaker on 4xx Geth responses. 4xx is usually
a client-shaped error (bad params, etc.), not a Geth-availability signal.
Tripping the breaker on every malformed wallet request was causing
spurious "circuit open" states.
- Move per-tx routing logs ("Routing transaction to Flashbots", "Submitting
transaction via MEV relay", "Bundle submitted: 0xABC...") from INFO to
DEBUG. Default RUST_LOG=info pipes journalctl → log shippers, which made
per-tx volume and bundle hashes available upstream by accident.
- Move the .onion hostname log from INFO to DEBUG. Same concern: log
shippers were turning a deliberately-not-published address into an
inadvertently-published one. Operators who want it can `cat
data/tor/torpc/hostname`.
- Make Tor anonymity check fatal when torrc exists. `check_configuration`
used to refuse to start on a missing torrc; now it returns Ok in that
case (no Tor → nothing to enforce) but propagates errors otherwise, so
`HiddenServiceSingleHopMode 1` actually refuses to start. The
`TORPC_ALLOW_NON_ANONYMOUS=1` override remains for benchmarks/CI.
Plus one operator-UX fix:
- Load `.env` at startup via dotenvy. Operators who copied .env.example
to .env previously had no env applied unless they sourced it manually
or used systemd EnvironmentFile=. Now matches the documented setup.
Tests updated to assert *absence* of the fingerprinting headers; the
removed assertion on `X-Service: TorPC` flips to a forbid-list.
54 lib + integration tests pass.
The client proxy SOCKS5-tunnels JSON-RPC over Tor to a configured .onion. Previously every header the wallet attached was forwarded verbatim — only hop-by-hop and `Host` were filtered. That defeated the entire point of Tor for any wallet that sends a recognizable User-Agent, Origin, or Sec-CH-UA-* — i.e., every modern wallet. A `web3_clientVersion` request from MetaMask, Coinbase Wallet, Rabby, etc. arrived at the operator's onion with: User-Agent: Mozilla/5.0 ... Chrome/135.0... + extension UA Accept-Language: en-US,en;q=0.9 Sec-CH-UA: "Chromium";v="135", "Not(A:Brand";v="8" Origin: chrome-extension://... Referer: chrome-extension://... That's enough for the operator (and anyone observing the egress) to fingerprint the wallet, browser, version, and locale on every request. This commit: - Adds `is_wallet_fingerprint()` covering User-Agent, Accept-Language, Origin, Referer, Cookie, Authorization, DNT, X-Forwarded-For, X-Real-IP, every `Sec-*` header, and `X-Wallet-*` extensions. Anything matching is dropped from the forwarded request. - Replaces the missing User-Agent with a single synthetic `torpc-proxy/<version>`. Sending no UA at all is itself a (smaller) fingerprint; sending a versioned constant blends every torpc-proxy user into the same identity. - Adds three tests: that fingerprint headers are classified for stripping, that benign headers (Content-Type, X-Flashbots-Signature, etc.) pass through, and that the synthetic UA contains no wallet/vendor strings. 12 tests pass.
Three lines of doc/UX rot that were actively misleading users:
1. Chain ID footgun in wallet auto-add helpers.
Coinbase + Rabby `addNetwork()` defaulted to `chainId = "1337"` (Geth
`--dev`) while the surrounding HTML copy said "Chain ID: 1 (mainnet) or
1337 (dev)". A user on mainnet who clicked the auto-add button silently
created a dev-chain entry pointing at a mainnet RPC URL — every
signature mismatched the chain the wallet thought it was on.
Defaults flipped to `"1"` and the copy reworked to make the dev case
the explicit exception.
2. README configuration drift.
Multiple defaults the README documented disagreed with the code:
- `STRICT_SECURITY_HEADERS=false` claimed default; code default is `true`.
This one had real consequences: operators reading the README believed
CORS was on by default and tested accordingly.
- `RATE_LIMIT_REQUESTS=60` claimed; code default is `100`.
- `MAX_REQUEST_SIZE=524288` (512KB) claimed; code default is `1048576`
(1 MiB).
- `TOR_SOCKS_PROXY` documented but never read by the daemon.
Plus several wholly-fictional sections:
- One-liner install script that did not exist.
- Docker / docker-compose recipe with no Dockerfile in the repo.
- "Add Prometheus metrics (coming soon)" — actually shipped at /metrics.
- "To add Flashbots authentication" snippet showing a non-functional
hand-roll while the codebase already does EIP-191 end-to-end via
FLASHBOTS_SIGNING_KEY.
- Geth manual command using `--http.api eth,net,web3,personal,miner,
txpool,debug --http.corsdomain "*"` — wider than the dev script,
teaches operators an insecure config that scripts/start-geth-dev.sh
deliberately tightened.
Each fixed against current reality. systemd path corrected from the
nonexistent `services/*.service` to `deploy/systemd-example/install-systemd.sh`.
3. Undocumented env vars added to .env.example:
- FLASHBOTS_REQUEST_TIMEOUT (per-attempt timeout for relay POSTs)
- WRITE_RATE_LIMIT_REQUESTS, WRITE_RATE_LIMIT_WINDOW (per-method bucket
for write methods, tighter than the global limit)
- TORPC_ALLOW_NON_ANONYMOUS (anonymity-disabling override; previously
read by the code with no documented existence)
54 lib + integration tests pass.
Pre-consolidation the repo had two unrelated workspaces:
- root `Cargo.toml` (a `[package]`, no workspace block) — the daemon
- `torpc-proxy/Cargo.toml` — a separate `[workspace]` with three members
(proxy-core, proxy-cli, proxy-gui)
That meant: two `cargo build` invocations, two `Cargo.lock` files, two
clippy passes in CI, and silent dependency drift (tokio 1.35 vs 1.40,
thiserror 1.0 vs 2.0). It also broke `cargo build --workspace` from the
root, which was the implicit assumption every onboarding doc made.
This commit makes the root `Cargo.toml` the single workspace root that
ALSO carries the daemon as the root package. Members list the three
proxy crates explicitly. Shared deps (tokio, anyhow, thiserror, tracing,
serde, hyper, tokio-socks, clap, rand) are promoted to
`[workspace.dependencies]`; daemon-only deps (axum, tower, tower-http,
reqwest, secp256k1, sha3, hex, dotenvy, chrono, once_cell, uuid) stay
in the root package.
Daemon thiserror bumped 1.0 → 2.0 to unify with the proxy. The
ProxyError derive uses only simple `#[error("…")]` patterns, no
backtrace or generics — fully forward-compatible.
Mechanical changes the consolidation triggered:
- Single `Cargo.lock` (the inner one is deleted).
- `make test` and CI both run `cargo test --workspace --tests`.
- CI gains a `cargo audit` job (set to continue-on-error for now —
signal first, tighten later).
- CI installs Tauri/webkit2gtk system libs because the GUI crate
is now in the unified clippy/test pass.
- `cargo clippy --workspace --all-targets -- -D warnings` is now
green. Fixed two `clippy::zombie_processes` warnings in
`tests/mev_integration_tests.rs` (added `.wait()` after `.kill()`
on spawned daemon subprocesses) and ~50 `uninlined_format_args`
across test files via `cargo clippy --fix`.
- `cargo fmt --all --check` is now green; the proxy-gui main.rs had
some pre-existing rustfmt drift now corrected.
Build/test verification:
- `cargo build --workspace` → all four crates compile cleanly.
- `cargo test --workspace --tests` → ~140 tests pass (16 ignored
remain service-required).
- `cargo clippy --workspace --all-targets -- -D warnings` → clean.
- `cargo fmt --all --check` → clean.
The previous README was 880+ lines with three rotted commits' worth of
drift. This commit replaces it with a 303-line version organized around
the two distinct user roles (operator vs wallet user) and verified
against the current code.
Removed in this rewrite:
- The "One-Line Install" section pointing at a `curl | bash` URL that
did not exist.
- The full Docker / docker-compose recipes for a Dockerfile that did
not exist.
- "Monitoring & Metrics: coming soon" — `/metrics` actually shipped.
- Step-by-step "Edit src/whitelist.rs" / "Edit src/rate_limit.rs" recipes
that pre-dated the env-var configuration surface.
- A Flashbots auth code snippet (`headers.insert("X-Flashbots-Signature",
signature)`) presented as setup the user had to do — full EIP-191 auth
is wired end-to-end via FLASHBOTS_SIGNING_KEY.
- Fictional install methods (Foundry "required for testing" — only used
by `scripts/generate-test-data.sh`).
Added or rewritten:
- Two-role table at the top: operator gets one path, wallet user gets
another. They have different goals and different binaries.
- Explicit threat-model section: what TorPC protects against, what it
does NOT (operator trust, traffic correlation, smart-contract MEV).
- Configuration reference table reflecting actual env vars and defaults.
All defaults verified against `src/app.rs` and `src/security.rs`.
- Honest status line: "working but not yet release-distributed" — sets
expectations for the build-from-source step until the release
pipeline lands.
- Privacy-section call-out of what the daemon does to avoid leaking
operator identity (no `Server` / `X-Service`, sanitized Geth errors,
bundle hashes below INFO, strict CSP).
- LICENSE file added (MIT) — README claimed MIT but no LICENSE file
existed.
Triggered on `v*` tags. Builds `torpc` (daemon) and `torpc-proxy` (CLI client) for four platforms and uploads them to a GitHub release as tar.gz / zip archives with SHA256 checksums. Matrix: - x86_64-unknown-linux-gnu (ubuntu-latest) - x86_64-apple-darwin (macos-13, Intel) - aarch64-apple-darwin (macos-latest, Apple Silicon) - x86_64-pc-windows-msvc (windows-latest) The Tauri GUI is excluded for now — bundling (DMG / AppImage / MSI) needs platform-specific post-build steps that don't yet have demand. Source build of the GUI is documented in the README. This unblocks the wallet-user install flow in Phase 1C's "Wallet User Setup" — once a tag is pushed, users will be able to download a ready-to-run binary instead of needing a Rust toolchain. The README copy currently says "no pre-built binaries yet"; we'll flip that to a direct link to the releases page after the first tag.
Pre-Phase-2 wiring:
/rpc, /rpc/flashbots, static UI \
/health, /metrics } single Router on BIND_ADDR
which is what the .onion forwards to.
Result: anyone reaching the .onion could `curl /metrics` and read live
operator state — component circuit-breaker status (`{ "geth": "open" }`
tells observers exactly when Geth went down), per-method blocked-request
counters (volume oracle), rate-limit hit counts, plus uptime, version,
and a wall-clock timestamp from /health. None of this is meant to be
published; it's all operator-side debugging signal.
After this commit:
- `BuiltApp` returns two routers — `app` (Tor-facing, full middleware
stack) and `admin_app` (localhost-only, security headers but no JSON-RPC
timeout / body-limit / rate-limit — those are noise on a localhost
surface).
- `main.rs` binds two TcpListeners (BIND_ADDR + ADMIN_BIND_ADDR), serves
both concurrently, and shares one shutdown signal across both via a
tokio broadcast channel. Both `axum::serve(...)` futures get cancelled
together on SIGINT/SIGTERM.
- New `ADMIN_BIND_ADDR` env var (default `127.0.0.1:9001`). The daemon
refuses to bind it to a non-loopback IP — defense in depth so an
operator who copy-pastes `0.0.0.0:9001` from a tutorial can't
accidentally re-publish what we just hid.
- Both routers share the same `MevProxyState`, so the atomic counters in
`SecurityMetrics` are consistent: a request that trips the public
router's blocked-method counter is immediately visible on the admin
router's `/metrics`.
Tests:
- New regression guard `health_and_metrics_are_404_on_public_router`
asserts both endpoints 404 on the public router and 200 on the admin
router. This catches the case where someone accidentally adds them
back to `app` instead of `admin_app`.
- Existing `health_reports_minimal_process_uptime_payload` and
`metrics_endpoint_exposes_live_counters` updated to hit the admin
router. The metrics test specifically POSTs to `.public` then GETs
`.admin`, exercising the shared-state property.
- 12 prior call sites of `make_server` left unchanged via a thin
wrapper; new `make_servers` returns both for tests that need them.
Docs:
- `.env.example` documents `ADMIN_BIND_ADDR` with the loopback-only
rationale.
- README's Observability section now points at port 9001 and notes the
refusal to bind non-loopback addresses. Configuration table updated.
The single `static/index.html` was being served identically to three
audiences — operator on LAN, Tor Browser visitor without a client, Tor
Browser visitor with a client — even though their needs and mental models
diverge sharply. The .onion visitor without a client was shown
"RPC URL: http://localhost:8545" with no instruction to install anything,
so the page was effectively broken for the audience it most needed to
help.
Architecture:
- `GET /` is now an axum handler that picks one of two `include_str!`
templates based on the request `Host` header. `.onion` suffix → user
template; anything else → operator template. Port is stripped before
suffix matching so `foo.onion:80` still resolves correctly.
- `nest_service("/", ServeDir)` replaced with
`route("/", get(serve_root)) + fallback_service(ServeDir)` so static
assets (app.js, style.css, helper JS) still serve as before.
`static/index_user.html` (new):
- Step 1: "Install the local TorPC client" with prominent download
button (links to the GitHub releases page from Phase 4's pipeline) and
build-from-source link. Below that, a JS probe of `127.0.0.1:8545`
hides Step 2 until a client is detected.
- Step 2: 4-button wallet picker (MetaMask, Coinbase, Rabby, Trust
Wallet). Clicking reveals only that wallet's flow, sharing the same
DOM IDs the existing `WALLETS` adapter expects so app.js drives it
unchanged.
- `<noscript>` block surfaces manual setup instructions for Tor Browser
Safest mode (JS disabled).
- Threat-model `<details>` summarising what TorPC protects against and
what it doesn't.
`static/index_operator.html` (new, replaces old `index.html`):
- Endpoint reference (public RPC, Flashbots, admin loopback at
`:9001/health`, `:9001/metrics`).
- Existing JSON-RPC test panel.
- Self-test wallet flows for the four supported wallets — same DOM as
the user template's flows so the operator can verify each adapter
works locally.
- Footer reminding the operator to share the .onion address (visitors
see the user-onboarding template instead of this dashboard).
JS changes:
- `setupClientProbe()` runs the `localhost:8545` probe on page load with
a 1.5s timeout, swaps the pending callout to success/warn, reveals
Step 2 on success.
- `setupWalletPicker()` wires the wallet-card buttons.
- Both helpers early-return when their DOM isn't present, so the same
`app.js` drives both templates.
- Dropped the dead Rainbow entry from `WALLETS` — it had a section in
the old index.html but no helper file and no addNetwork flow; clicking
the button just showed the generic RPC URL.
Tests (in `tests/daemon_e2e_test.rs`):
- `root_serves_user_template_for_onion_host` pins the `.onion` →
user-template branch.
- `root_serves_operator_template_for_loopback_host` pins the loopback →
operator-template branch.
- `root_with_port_in_onion_host_still_picks_user_template` regression-
guards the port-stripping behavior.
- `static_assets_still_serve_through_fallback` ensures the
`nest_service` → `fallback_service` migration doesn't break /app.js
and /style.css delivery.
- `tests/integration.rs::test_static_files_exist` updated to assert the
two new template files instead of the deleted index.html.
CSS:
- New: `.step-section`, `.step-num`, `.callout-{pending,warn,success}`,
`.wallet-picker`, `.wallet-card`, `.wallet-emoji`, `.wallet-flow`,
`.install-options`, `.muted`, `.threat-model`, `.btn:focus-visible`.
- Removed: dead `.loading` class.
134 daemon + workspace tests pass, fmt + clippy clean.
Two CI failures fixed: 1. clippy + test were failing on `soup2-sys` build because Tauri 1.x needs `libsoup-2.4` and `libwebkit2gtk-4.0`, but Ubuntu 24.04 (the `ubuntu-latest` image) dropped those packages in favor of `-3.0`/ `-4.1`. Rather than installing a backport repo, exclude `torpc-proxy-gui` from CI builds via `--workspace --exclude torpc-proxy-gui`. The GUI is verified locally on macOS/Linux dev boxes and via the (future) release pipeline on per-platform runners; CI value is in the daemon + client core, both of which still build and test fully. Migration to Tauri 2.x removes this restriction. 2. cargo audit was reporting (and failing on) three transitive advisories — bytes RUSTSEC-2026-0007, slab RUSTSEC-2025-0047, tracing-subscriber RUSTSEC-2025-0055 — none of which we can fix directly. Job-level `continue-on-error: true` doesn't change the check's status display (it still shows red), so use `cargo audit || true` at the step level instead. The advisory output is still in the log; tighten this to a hard failure once the ecosystem patches land. Verified locally: 134 workspace-minus-gui tests pass, fmt/clippy clean.
Reverses the prior CI shortcut and fixes things properly: 1. cargo audit is now mandatory (no `|| true`). Bumped Cargo.lock to clear the three RUSTSEC vulnerabilities reported on PR #4: - bytes 1.10.1 → 1.11.1 (RUSTSEC-2026-0007: integer overflow in BytesMut::reserve) - slab 0.4.10 → 0.4.12 (RUSTSEC-2025-0047) - tracing-subscriber 0.3.19 → 0.3.20 (RUSTSEC-2025-0055) All three are minor/patch bumps within the existing semver compatible range, so workspace dep declarations need no change. 2. proxy-gui re-included in clippy/test/build. The previous workaround (`--workspace --exclude torpc-proxy-gui`) was hiding GUI bitrot from CI. Pin clippy + test to `ubuntu-22.04` instead — that runner image still has the Tauri 1.x system deps (libwebkit2gtk-4.0, libsoup2.4) that Ubuntu 24.04 dropped. Pinning is a temporary measure; revisit when the GUI migrates to Tauri 2.x. 3. Tauri 1.x system deps installed properly: libwebkit2gtk-4.0-dev, libgtk-3-dev, libayatana-appindicator3-dev, librsvg2-dev, and libsoup2.4-dev (the missing one that triggered the soup2-sys build failure on PR #4's first run). cargo audit defaults treat unmaintained / unsound / yanked as informational warnings (~20 in our tree, mostly Tauri 1.x's old transitive deps). Those don't fail the pipeline today and won't until either the upstream crates are revived or we move to Tauri 2.x. Vulnerabilities are mandatory, advisories are visible — that's the right balance for now. Verified locally: cargo audit clean (0 vulns), build clean, clippy clean on full workspace, 134 tests pass.
`Swatinem/rust-cache@v2` keys on `runner.os` (literally "Linux") plus
Cargo.lock + rustc version. With ubuntu-latest (24.04, GLIBC 2.39) and
ubuntu-22.04 (GLIBC 2.35) both reporting `runner.os=Linux`, build-script
artifacts cached on the newer image leaked into runs on the older one,
producing:
/lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.39' not found
(required by .../target/debug/build/libc-.../build-script-build)
Adding `prefix-key: "v0-rust-${{ env.ImageOS }}"` to both clippy and
test jobs gives us per-image cache lanes (ubuntu22 vs ubuntu24 vs
macos-15-arm64 etc.). Existing 24.04 caches won't poison new 22.04
runs and vice versa.
`ImageOS` is set by the GitHub runner itself; documented at
https://docs.github.com/en/actions/learn-github-actions/variables.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Eight commits across four phases of the end-to-end review. Each commit is independently load-bearing and reviewable; the per-commit messages explain the why in detail. Summary below.
This branch fast-forwards main — no merge commit needed.
Phase 1 — privacy hardening (3 commits)
bc0241dDaemon privacy hardening: drop fingerprints, sanitize errors, gate logsX-Service: TorPCresponse header removed (single HEAD over .onion was identifying us)configs/torrcexists with anonymity-disabling flags.onionhostname moved to DEBUG (was leaking into syslog/log shippers)dotenvy::dotenv()at startup socp .env.example .envactually appliesf64e81fStrip wallet-fingerprinting headers in client proxy forwarding pathis_wallet_fingerprint()covers User-Agent, Origin, Referer, Cookie, Authorization, DNT, Sec-, X-Wallet-, X-Forwarded-For, X-Real-IP, Accept-LanguageUser-Agent: torpc-proxy/<version>replaces the wallet's own UAea9be8eFrontend chainId fix + README accuracy + .env.example documentationaddNetworkdefaults flipped 1337 → 1 (was creating dev-chain entries pointing at mainnet RPCs)STRICT_SECURITY_HEADERS=true,RATE_LIMIT_REQUESTS=100,MAX_REQUEST_SIZE=1048576.env.exampledocuments previously-undocumented vars:FLASHBOTS_REQUEST_TIMEOUT,WRITE_RATE_LIMIT_*,TORPC_ALLOW_NON_ANONYMOUSPhase 4 — workspace + docs + release (3 commits)
e17dea3Consolidate to a single repo-wide Cargo workspaceCargo.tomlworkspace covering daemon + 3 proxy cratestorpc-proxy/Cargo.tomldeleted; oneCargo.lock[workspace.dependencies]; daemon'sthiserrorbumped 1→2 to aligncargo build --workspace, one clippy, one fmt, plus newcargo auditjobclippy::uninlined_format_argswarnings + 2clippy::zombie_processescleaned viacargo clippy --fixc43d6edLICENSE + README rewrite around the two-role mental modelLICENSE(MIT) — README claimed MIT but no file existedba65564Add binary release pipeline for daemon + CLI client.github/workflows/release.ymltriggers onv*tagstorpc+torpc-proxyfor linux-x86_64, darwin-x86_64, darwin-aarch64, windows-x86_64Phase 2 — admin endpoint split (1 commit)
a58e42dSplit admin endpoints (/health, /metrics) onto a localhost-only listenerBuiltAppreturns two routers: public (Tor-facing) + admin (localhost-only)ADMIN_BIND_ADDR=127.0.0.1:9001env var; daemon refuses non-loopback binds (defense in depth)main.rsbinds two TcpListeners, runs bothaxum::serve(...)concurrently, shares one shutdown signal viatokio::sync::broadcastMevProxyStateso atomic counters inSecurityMetricsstay consistent/healthand/metricsare 404 on the public router and 200 on the admin routerPhase 3 — audience-split static page (1 commit)
1f377ecAudience-split static page: operator dashboard vs user wallet onboardingGET /is now an axum handler that picks template byHostheader.onionsuffix →index_user.html(install-the-client step + JS probe oflocalhost:8545+ wallet picker, with<noscript>fallback)index_operator.html(endpoint reference + JSON-RPC test panel + self-test wallet flows)nest_service("/", ServeDir)replaced withroute("/") + fallback_service(ServeDir)— explicit root, ServeDir handles the restWALLETS(no helper, no addNetwork flow)Verification
cargo build --workspacecleancargo test --workspace --tests— 134 tests pass, 16 ignored (service-required)cargo fmt --all --checkcleancargo clippy --workspace --all-targets -- -D warningscleanTest plan
cargo build --workspacev0.1.0to exercise the release pipeline once merged