Files
proxy-god/docs/AUDIT_FINDINGS.md
Dr Jones 792b320257
Some checks failed
CI / Test Python 3.10 (push) Has been cancelled
CI / Test Python 3.11 (push) Has been cancelled
CI / Test Python 3.12 (push) Has been cancelled
fix: address P0/P1/P2 audit findings — security, reliability, CI, docs
P0 - Critical:
- config.py: add _mask() credential-redaction helper, SETTINGS_SCHEMA_VERSION,
  plaintext-storage warning; corrupt settings.json backed up before defaults
- gost_util.py: pin SHA256 for gost_3.2.6_windows_amd64.zip; verify before
  extraction; _add_defender_exclusion now logs warning on failure
- firewall.py: add emergency_disengage() for atexit/signal use
- service.py: register fw_emergency_disengage via atexit + SIGTERM/SIGINT;
  stop() calls emergency_disengage if thread hangs past 15s timeout
- app.py: wrap main() in top-level except with CTk error dialog + log
- LICENSE: add MIT license file

P1 - Important:
- app.py: switch to RotatingFileHandler (5 MB / 3 backups)
- config.py: settings_version + migrate(); save_settings() writes .bak before
  overwrite; _is_safe_https_url() strips RFC-1918 sources/ip_check_url
- service.py: threading.Lock on _settings; _signal_handler; graceful stop
- requirements.txt: pin exact versions; add cryptography==48.0.0
- .gitignore: add settings.json, credential JSON files, screenshot noise
- tray.py, dns_leak.py, gost_util.py: replace bare except:pass with logging
- CHANGELOG.md: document all session changes

P2 - Nice to have:
- .github/workflows/test.yml: CI on Python 3.10/3.11/3.12 windows-latest
- run.py: --version / -V flag
- docs/OPERATOR_RUNBOOK.md: emergency disengage, proxy leak, GOST, settings

Tests: 47/47 passed (python -m unittest discover -s tests -v)
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-21 23:49:02 -07:00

194 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Proxy God — Audit Findings
**Audit date:** 2026-05-21
**Scope:** Production-readiness gaps, README cross-check, automated tests
**Tests run:** `python -m unittest discover -s tests -v`**47 passed**, 0 failed (~25s)
---
## Executive Summary
| Category | P0 | P1 | P2 | Total |
|----------|----|----|-----|-------|
| **Production readiness gaps** (this pass) | 15 | 46 | 16 | **77** |
| Critical bugs (separate bug-hunt pass) | — | — | — | *pending / parallel* |
**Test status:** Core pure-logic modules are covered (config, leak_detect, validator timing, chain_map, browser persona). No automated tests touch `service.py`, `firewall.py`, `gost_util.py`, `sysproxy.py`, or `app.py`.
**Top 5 P0 production blockers**
1. **No CI/CD** — nothing runs tests or build on push/tag (`requirements.txt:1-4`, no `.github/workflows` / `.gitea/workflows`).
2. **No code signing** — release `.exe` is unsigned (`ProxyChainManager.spec:70-71`).
3. **Secrets at rest in plaintext** — proxy credentials and signup passwords in `%LOCALAPPDATA%\ProxyChainManager\` JSON (`config.py:364-368`, `signup_prep.py:295-296`).
4. **GOST binary integrity not verified** — runtime download with zip magic check only, no SHA256/signature pin (`gost_util.py:17-65`).
5. **Kill-switch recovery gap** — outbound BLOCK can persist if process dies mid-run; no standalone recovery tool documented (`firewall.py:142-156`, `service.py:136-143`).
---
## Production Readiness Gaps
### Infrastructure
| # | What's missing | Why it matters for prod | Suggested fix | Priority |
|---|----------------|-------------------------|---------------|----------|
| I-1 | **No CI/CD pipeline** (no GitHub/Gitea workflow YAML in repo) | Regressions ship silently; no reproducible build gate | Add workflow: `pip install -r requirements.txt`, `unittest discover`, optional `pyinstaller` on tag | **P0** |
| I-2 | **No automated test gate on PR/merge** | CONTRIBUTING.md says “run tests” but nothing enforces it (`CONTRIBUTING.md:9`) | Fail CI if `python -m unittest discover -s tests` fails | **P0** |
| I-3 | **No dependency lockfile** (`requirements.txt` uses `>=` only) | Non-reproducible builds across machines/time | Generate `requirements.lock` or pin exact versions in release branch | **P1** |
| I-4 | **PyInstaller not pinned** — installed ad hoc in `scripts/setup_and_build.ps1:39` | Release binaries differ per builder Python/PyInstaller version | Pin `pyinstaller==x.y.z` in dev requirements; record in release notes | **P1** |
| I-5 | **No release automation**`docs/RELEASE_PROCESS.md` is manual checklist only | SHA256/commit attachment steps are skipped in practice | Script: build → hash → attach to Gitea release with commit SHA | **P1** |
| I-6 | **No pre-commit / secret scanning** | Risk of committing proxy creds or signup JSON from dev machines | `gitleaks` or similar in CI + pre-commit | **P1** |
| I-7 | **No documented required env vars**`MAINTENANCE.md:22` asks for them but none exist | Operators cannot configure corporate proxy, custom data dir, etc. | Document optional `LOCALAPPDATA` override pattern or explicit env contract | **P2** |
| I-8 | **Stewardship checklists are templates only** — all items unchecked in `docs/SECURITY_REVIEW.md`, `docs/PROVENANCE_CHECKLIST.md` | Release gate in docs is never enforced | Complete checklists per release or automate in CI | **P1** |
### Reliability
| # | What's missing | Why it matters for prod | Suggested fix | Priority |
|---|----------------|-------------------------|---------------|----------|
| R-1 | **Kill-switch may remain engaged after abnormal exit**`fw_disengage()` only on `stop()` / `_close()` (`service.py:136-143`, `app.py:2877-2886`) | Crashed/killed process leaves `netsh` outbound BLOCK → machine appears “offline” | `atexit` + signal handlers; ship `scripts/disengage_kill_switch.ps1`; detect orphan rules at startup | **P0** |
| R-2 | **GOST download without checksum/signature** (`gost_util.py:49-61`) | Supply-chain compromise or CDN swap replaces `gost.exe` | Pin release URL + publish SHA256; verify before extract; optional minisign | **P0** |
| R-3 | **No settings schema version / migration**`load_settings()` merges unknown keys silently (`config.py:329-361`) | Upgrades break or drop fields; no upgrade path for operators | Add `settings_version`; migration functions per version | **P1** |
| R-4 | **Corrupt `settings.json` silently reverts to defaults** (`config.py:335-336`) | Operator loses pinned chain/credentials without warning | On parse failure: backup corrupt file, surface UI error, offer restore | **P1** |
| R-5 | **`update_settings()` while service thread running** — no lock around `_settings` (`service.py:112-114`, `349+`) | Race: health interval / pool refresh reads half-written settings | `threading.Lock` on settings read/write or restart service on save | **P1** |
| R-6 | **No offline / degraded mode** — empty pool sleeps 60s and retries (`service.py:402` area) | Air-gapped or CDN-down hosts spin with no cached pool | Persist last-good pool snapshot; UI banner “offline, using cache” | **P1** |
| R-7 | **ChainService thread is daemon** (`service.py:131-133`) | Process exit can abort mid-teardown (proxy/firewall left dirty) | Non-daemon thread + bounded shutdown protocol | **P1** |
| R-8 | **`stop()` join timeout 15s only** (`service.py:140-141`) | Hung GOST/async loop leaves firewall engaged | Force-kill GOST after timeout; always call `fw_disengage()` in `finally` | **P0** |
| R-9 | **Global `ThreadPoolExecutor`s never shut down** (`service.py:64`, `pool_ops.py:15`) | Interpreter shutdown warnings; resource leaks in long-running tray sessions | `shutdown(wait=False)` on app exit | **P2** |
| R-10 | **No GOST process watchdog** — if GOST dies, behavior depends on loop branch | Silent proxy outage until next health check | Detect `proc.poll()`; restart or rotate immediately | **P1** |
| R-11 | **Logon scheduled task runs LIMITED** (`windows_task.py:47-48`) | Auto-start cannot engage kill-switch / MAC / HKLM fixes that need Admin | Document “task ≠ full enforcement”; optional elevated task with warning | **P1** |
| R-12 | **No backup/restore for operator data** | Reinstall or profile wipe loses chains and signup vault | Export/import ZIP of `%LOCALAPPDATA%\ProxyChainManager\` | **P2** |
| R-13 | **No rollback to prior release** | Bad build bricks operators | Keep previous `ProxyChainManager.exe` + settings backup; versioned releases | **P1** |
### Security
| # | What's missing | Why it matters for prod | Suggested fix | Priority |
|---|----------------|-------------------------|---------------|----------|
| S-1 | **Plaintext `settings.json`** with embedded proxy auth (`config.py:364-368`, `asdict` stores URLs) | Disk theft = credential leak | DPAPI-encrypt sensitive fields or Windows credential locker | **P0** |
| S-2 | **Plaintext signup vault**`signup_draft.json`, `signup_accounts.json` (`signup_prep.py:252-296`) | Passwords/emails readable by any local user/process | Encrypt at rest; redact in logs (partially done for proxy URLs only) | **P0** |
| S-3 | **No TLS/certificate pinning for feed URLs** (`fetcher.py:11-18`) | MITM on jsdivr/GitHub could inject malicious proxy list | Pin certs or use signed offline bundle for sources | **P2** |
| S-4 | **Broad Defender exclusion** on entire GOST folder (`gost_util.py:23-39`) | AV blind spot if folder is poisoned | Exclude only `gost.exe`; verify hash before exclusion | **P1** |
| S-5 | **`ip_check_url` and custom `sources` not URL-validated** (`config.py:338-340`) | Hand-edited JSON could point health checks at internal/metadata endpoints | Allowlist HTTPS hosts; block RFC1918/link-local in validator | **P1** |
| S-6 | **Kill-switch default ON** (`config.py:186`) without mandatory recovery docs in README | Bricked network for non-technical operators | README “emergency disengage” + startup orphan detection | **P0** |
| S-7 | **Always-elevated PyInstaller build** (`ProxyChainManager.spec:72` `uac_admin=True`) | Every launch prompts UAC; users may habit-click; not least-privilege | Split “Operator” vs “Enforcer” binaries or manifest optional elevation | **P1** |
| S-8 | **Signup autofill extension** writes config into Firefox profile (`signup_prep.py:334+`) | Local profile theft exposes generated credentials | Document threat model; optional OS-level profile encryption | **P1** |
| S-9 | **No rate limiting on outbound validation fan-out** (default concurrency 64, `config.py:179`) | Can trigger abuse flags on ipify/jsdelivr/proxy sources | Cap per-host concurrency; exponential backoff on 429 | **P1** |
| S-10 | **No secrets rotation story** | Long-lived proxy passwords in JSON never expire | Export reminder; credential field rotation UX | **P2** |
### Observability
| # | What's missing | Why it matters for prod | Suggested fix | Priority |
|---|----------------|-------------------------|---------------|----------|
| O-1 | **`proxy_chain_manager.log` has no rotation** (`app.py:91-98`) — unlike `gost.log` (`gost_util.py:98-105`) | Long sessions fill disk | `RotatingFileHandler` or size cap like GOST | **P1** |
| O-2 | **No crash reporting** (WER hook, Sentry, minidump) | Field failures are invisible to maintainer | Opt-in crash telemetry with redaction | **P1** |
| O-3 | **Unstructured text logs only** | Hard to query in SIEM | Optional JSON log format for file handler | **P2** |
| O-4 | **No metrics** (Prometheus/windows perf counters) | Cannot alert on pool size, rotation rate, leak count | Export counters via named pipe or local HTTP if needed | **P2** |
| O-5 | **No external health endpoint** | Expected for server apps; desktop app has tray only — gap if headless automation desired | Document N/A for GUI; optional `127.0.0.1:status` for scripting | **P2** |
| O-6 | **Broad `except Exception: pass` in Defender helper** (`gost_util.py:38-39`) | Silent failure to exclude → GOST quarantined with no UI clue | Log warning + surface in Live tab | **P1** |
| O-7 | **UI event queue unbounded** (`app.py:200`) | Log flood can grow memory | `queue.Queue(maxsize=N)` with drop-oldest policy | **P2** |
### Distribution
| # | What's missing | Why it matters for prod | Suggested fix | Priority |
|---|----------------|-------------------------|---------------|----------|
| D-1 | **No Authenticode signing** (`ProxyChainManager.spec:70-71` `codesign_identity=None`) | SmartScreen blocks; enterprise deployment blocked | Sign with cert; document thumbprint in release | **P0** |
| D-2 | **No SHA256 publish step in build script** — required by `docs/RELEASE_PROCESS.md:10` but `setup_and_build.ps1` only copies exe | Users cannot verify download integrity | Emit `ProxyChainManager.exe.sha256` in `dist/` | **P1** |
| D-3 | **UPX compression enabled** (`ProxyChainManager.spec:66-67`) | AV false positives on packed binaries | Disable UPX for release builds | **P1** |
| D-4 | **GOST not shipped in installer** — downloaded on first run (`gost_util.py:42-65`) | First-run requires internet; air-gap fails | Bundle pinned `gost.exe` in installer or optional offline pack | **P1** |
| D-5 | **No MSI/Inno installer** — only raw exe + Desktop copy (`setup_and_build.ps1:55-58`) | No uninstaller, no Start Menu integration, no upgrade path | Wrap in signed installer with upgrade code | **P2** |
| D-6 | **No in-app auto-update channel** | Security patches require manual git pull / rebuild | Check release API + signed delta or full exe | **P1** |
| D-7 | **Version `1.0.0` static** (`__init__.py:3`) — not embedded in Windows file properties | Support cannot map exe → commit | PyInstaller `version` resource from git tag | **P1** |
| D-8 | **No reproducible build manifest** (SBOM) | Supply-chain audits fail | `pip freeze` + GOST hash in release notes | **P1** |
### Documentation
| # | What's missing | Why it matters for prod | Suggested fix | Priority |
|---|----------------|-------------------------|---------------|----------|
| DOC-1 | **No LICENSE file**`LICENSE_STATUS.md:3-9` says “all rights reserved” | Cannot legally redistribute or accept external contributions | Choose SPDX license; add `LICENSE` | **P0** |
| DOC-2 | **README omits production realities** — no mention of: unsigned exe, first-run GOST download, Defender exclusion, kill-switch recovery (`README.md:69-114`) | Operators misconfigure or panic when network dies | Add “Production deployment” section | **P1** |
| DOC-3 | **README test claim understates gaps** — “Covers config… smoke paths” (`README.md:124`) but no service/firewall/GUI tests | False confidence in release quality | Align README with actual test matrix | **P1** |
| DOC-4 | **`CHANGELOG.md` Unreleased empty** (`CHANGELOG.md:5-7`) | No user-visible record between tags | Require changelog entry in PR template | **P1** |
| DOC-5 | **No `THIRD_PARTY_NOTICES.md`** for GOST, CustomTkinter, Pystray, Proxifly data | License compliance for binary distribution | Generate notices from `pip licenses` + GOST MIT | **P1** |
| DOC-6 | **No operator runbook** (kill-switch stuck, proxy leak, pool empty) | MTTR high in field | `docs/RUNBOOK.md` with netsh recovery steps | **P0** |
| DOC-7 | **“Mission-grade” / “fail closed” marketing without SLOs** (`README.md:5-12`) | Enterprise buyers expect defined behavior | Add limitations: public proxy quality, VPN requirement, admin scope | **P2** |
| DOC-8 | **`docs/MAINTENANCE.md` rate-limit note** (`MAINTENANCE.md:23`) not reflected in product | Release gate incomplete | Implement S-9 or remove claim | **P1** |
### Testing
| # | What's missing | Why it matters for prod | Suggested fix | Priority |
|---|----------------|-------------------------|---------------|----------|
| T-1 | **No CI executing test suite** | 47 tests only run manually | Wire I-1 to `unittest discover` | **P0** |
| T-2 | **Zero tests for `service.py`, `firewall.py`, `gost_util.py`, `sysproxy.py`** | Core runtime unverified | Mock subprocess/netsh; test engage/disengage, settings races | **P0** |
| T-3 | **No tests for `load_settings` / `save_settings` round-trip** | Regression on migration/sanitize | Add `TestSettingsPersistence` with temp dir | **P1** |
| T-4 | **No frozen-bundle path tests**`_MEIPASS` branches in `signup_prep.py:21-22`, `chain_map.py:50-51` | PyInstaller regressions break map/signup | Test with `sys.frozen` monkeypatch | **P1** |
| T-5 | **No GUI smoke tests** (`app.py` ~2900 lines) | UI regressions ship every release | Minimal `pytest-qt` or screenshot smoke on CI Windows runner | **P1** |
| T-6 | **No `compileall` / lint in pipeline** | Syntax errors in rarely-imported modules | Add `python -m compileall proxy_chain_manager` to CI | **P1** |
| T-7 | **`TestFetcher.test_fetch_smoke` network-dependent** (`tests/test_proxy_god.py`) | CI flakiness offline | Mark `@unittest.skipUnless` or mock httpx | **P2** |
| T-8 | **No coverage threshold** | Unknown untested lines | `coverage run` + fail under 60% on core modules | **P2** |
### Compliance / Legal
| # | What's missing | Why it matters for prod | Suggested fix | Priority |
|---|----------------|-------------------------|---------------|----------|
| C-1 | **No SPDX license on repository** (`LICENSE_STATUS.md`) | Blocks open-source and commercial redistribution | Legal review → add `LICENSE` | **P0** |
| C-2 | **GOST upstream license not bundled in release artifact** | Binary distribution obligation | Include go-gost LICENSE in installer | **P1** |
| C-3 | **Signup presets target third-party ToS** (`signup_prep.py:37-164`) | Automation/autofill may violate site policies | Disclaimer in UI; operator assumes liability | **P1** |
| C-4 | **Provenance checklist incomplete** (`docs/PROVENANCE_CHECKLIST.md` all `[ ]`) | `docs/ROADMAP.md:14` blocks “first verified release” | Complete before v1.0 tag | **P1** |
| C-5 | **No privacy notice for local JSON stores** | GDPR-style transparency if EU operators | Short privacy section: what is stored where | **P1** |
### Operator Experience
| # | What's missing | Why it matters for prod | Suggested fix | Priority |
|---|----------------|-------------------------|---------------|----------|
| OP-1 | **No emergency kill-switch disengage UX** — must know netsh or restart app elevated (`firewall.py:149-156`) | #1 support incident when chain crashes | Tray item “Disengage firewall”; standalone script | **P0** |
| OP-2 | **“Run as Admin” relaunch closes app** (`app.py:347-350`) | Loses unsaved UI state; confusing flow | Relaunch with `--elevate` handoff file | **P1** |
| OP-3 | **Kill-switch skipped silently when non-admin** (`service.py:328-330`) | User believes they are “fail closed” but are not | Prominent banner when not admin + kill-switch ON | **P1** |
| OP-4 | **No settings export/import** | Machine migration painful | JSON export with optional encryption | **P2** |
| OP-5 | **No first-run wizard** (GOST download, VPN check, admin recommendation) | High abandonment on first failure | Step-through: VPN → Admin → Test pool | **P1** |
| OP-6 | **Close button minimizes to tray only** (`app.py:2888`) — good for daemon, bad if user expects quit | Document; tray “Exit” must disengage FW | Already in tray; add tooltip on X | **P2** |
| OP-7 | **README says “Accept UAC” but spec forces `uac_admin`** (`README.md:131`, `ProxyChainManager.spec:72`) | Confusing for non-admin workflows | Document two launch modes | **P1** |
| OP-8 | **No visible “version / build commit” in UI** | Support tickets lack context | Show `__version__` + git SHA in About | **P1** |
| OP-9 | **Pool refresh hammers external CDNs** (default 1800s, 3 sources) | Operator IP banned from jsdivr | Backoff; custom mirror field | **P1** |
| OP-10 | **Ban tester / leak audit results not persisted** | Cannot trend ban rate over sessions | Optional CSV export | **P2** |
---
## README Claims vs Implementation
| README claim | Status | Evidence |
|--------------|--------|----------|
| “Self-healing… rotate on failure, repeat forever” | **Partial** | Rotation exists (`service.py` main loop); no GOST watchdog (R-10) |
| “Fail closed” kill-switch | **Partial** | Requires Admin (`service.py:328-330`); orphan rules possible (R-1) |
| “Python not required” for exe | **OK** | PyInstaller one-file build (`ProxyChainManager.spec`) |
| “Works with or without VPN” | **OK** | `leak_detect.py` VPN-aware modes; tested |
| “Validation / tests” coverage | **Overstated** | 47 unit tests; no service/firewall/GUI (T-2, DOC-3) |
| Default kill-switch ON | **OK** | `config.py:186` |
| VM-safe loopback `127.0.0.1` | **OK** | `config.py:22-24`, `normalize_listen_host` |
| Build via `setup_and_build.ps1` | **OK** | Script exists; no signing/checksum (D-1, D-2) |
| Tray red/yellow/green | **OK** | `tray.py`, `app.py` status handlers |
| QUIC disabled where needed | **Verify manually** | Browser/sysproxy paths — not covered by unit tests |
---
## Test Run Notes (2026-05-21)
```
Ran 47 tests in ~25s — OK
```
**Modules with tests:** `config`, `leak_detect`, `validator`, `fetcher` (network smoke), `leak_audit`, `privacy_lan`, `browser_identity`, `chain_map`, `exit_intel`.
**Modules without tests:** `app`, `service`, `gost_util`, `firewall`, `sysproxy`, `tray`, `ban_tester`, `dns_leak`, `webrtc_check`, `fingerprint`, `artifact_wipe`, `windows_task`, `browser_launcher`, `signup_prep` (integration).
---
## Recommended Fix Priority (production)
| Priority | Focus |
|----------|--------|
| **P0** | CI + tests (I-1, T-1), code signing (D-1), secrets at rest (S-1, S-2), GOST integrity (R-2), kill-switch recovery (R-1, R-8, OP-1, DOC-6), LICENSE (C-1, DOC-1) |
| **P1** | Lockfiles, migrations, runbooks, README honesty, integration tests, rate limits, log rotation, third-party notices |
| **P2** | Metrics, installer, backup/restore, coverage gates, TLS pinning |
---
*Bug-hunt findings (critical bugs, edge cases, UI issues) may be appended by a separate audit pass under sections 28 above.*