polymarket_arbitrage/SECURITY_REVIEW.md

Security Review: Polymarket Arbitrage Bot

Date: 2026-07-22 Scope: Full codebase (27 Python files, Docker config, web UI, live executor) Mode: Paper mode by default; Live mode requires ARB_MODE=live

---

Summary

Severity	Count
HIGH	3
MEDIUM	4
LOW	3
INFO	3

The codebase follows strong security hygiene in several areas (SecretStr config, parameterized SQL, no hardcoded secrets, proper .gitignore). The critical gaps are around the web UI (no auth), the live executor (dry_run=False in scan mode), and the Docker deployment (exposed dashboard with kill/unkill controls).

---

HIGH Severity

H1: Live executor runs with dry_run=False in arb scan — no final confirmation

File: arbitrage/cli.py:87-89

executor = (
    LiveExecutor(books=books, dry_run=False)   # ← no dry_run flag in LiveExecutor!
    if settings.mode == Mode.LIVE
    else PaperExecutor(books=books)
)

The LiveExecutor.__init__ accepts dry_run but defaults to True. However, the CLI hardcodes dry_run=False when mode is live. This means ARB_MODE=live alone is sufficient to submit real orders. A misconfigured .env (or accidental env var bleed from another process) could trigger real trades.

Recommendation: Add an explicit CLI flag (--confirm-live) or a one-time confirmation prompt. The current single-env-var guard is acceptable for a CLI tool but should be documented prominently.

---

H2: Web UI /kill and /unkill have zero authentication

File: arbitrage/web/app.py:77-88

@app.post("/kill", response_class=HTMLResponse)
async def kill(request: Request) -> HTMLResponse:
    settings.kill_switch_file.parent.mkdir(parents=True, exist_ok=True)
    settings.kill_switch_file.touch(exist_ok=True)
    return await pnl(request)

@app.post("/unkill", response_class=HTMLResponse)
async def unkill(request: Request) -> HTMLResponse:
    path = settings.kill_switch_file
    if path.exists():
        path.unlink()
    return await pnl(request)

In Docker, the web service binds to 0.0.0.0 and exposes port 8000 (docker-compose.yml:47-51). Anyone on the network can POST to /kill to halt trading, or POST to /unkill to bypass a kill switch. No CSRF token, no auth header, no rate limiting.

Recommendation:

• Option A: Add a simple API key header check (e.g., X-Admin-Key) for /kill and /unkill
• Option B: Bind web to 127.0.0.1 in production and use a reverse proxy with auth
• Option C: Add HTTP Basic Auth via uvicorn middleware

---

H3: Docker web dashboard exposed on 0.0.0.0:8000 with no access control

File: docker-compose.yml:42-52

web:
  environment:
    - ARB_WEB_HOST=0.0.0.0
    - ARB_WEB_PORT=8000
  ports:
    - "8000:8000"

Combined with H2, the entire dashboard (including PnL data, basket details, kill/unkill controls) is accessible to any network participant. The dashboard shows event titles, basket counts, realized PnL, and opportunity details — operational intelligence that should not be public.

Recommendation:

• Change ARB_WEB_HOST to 127.0.0.1 and remove the port mapping, or
• Add a reverse proxy (nginx/Caddy) with auth before the dashboard
• If remote access is needed, use SSH tunneling or WireGuard

---

MEDIUM Severity

M1: _unwind sells at price=0.0 — unprotected market sell

File: arbitrage/engine/live_executor.py:261-272

args = OrderArgs(
    token_id=fill.token_id,
    price=0.0,  # market — no minimum price protection
    size=float(fill.size),
    side="SELL",
)

When unwinding a partial-fill basket, the executor submits SELL orders with price=0.0, which Polymarket interprets as a market order. In a fast-moving market, this could execute at an extremely unfavorable price (e.g., if the token price has dropped to near zero during the unwind).

Recommendation: Add a min_price parameter (configurable via settings) that sets a floor on the unwind sell price. If the current bid is below the floor, skip the unwind and log it for manual intervention.

---

M2: py-clob-client dependency is unpinned — supply chain risk

File: pyproject.toml:10

"py-clob-client>=0.20.0",

The docs (docs/api/order-signing.md:5-7) explicitly recommend pinning to 0.34.6. The unpinned >= constraint means pip install could pull in any version ≥0.20.0, including versions with breaking API changes, security regressions, or supply chain compromises.

Recommendation: Pin to py-clob-client==0.34.6 and add a CI step to verify the pinned version.

---

M3: Kill switch file is trivially bypassed (delete + unkill)

File: arbitrage/engine/live_executor.py:57-58, arbitrage/web/app.py:83-88

if limits.kill_switch_file.exists():
    raise RiskDenied(f"kill switch present: {limits.kill_switch_file}")

The kill switch is a simple file existence check (./KILL by default). An attacker (or operator) can:

1. Delete the file to bypass it (rm ./KILL)
2. Call /unkill to remove it remotely

There's no write-lock, no integrity check, no audit log of kill switch state changes.

Recommendation: If the kill switch is intended as a hard safety mechanism, use a more robust approach:

• A file with write-once semantics (chmod 444 after creation)
• An in-memory flag + periodic file check (so deletion alone doesn't resume)
• Audit logging of all kill/unkill events

---

M4: TOCTOU race in risk_gate basket count check

File: arbitrage/engine/live_executor.py:65-95

The risk_gate function checks the current number of open baskets, then execute() proceeds to submit orders. Between the check and the actual submission, another instance/process could open additional baskets, exceeding the configured limits.

In single-process mode (default), this is low risk. In multi-container deployments (docker-compose has separate scan and discover services), the risk is higher if multiple scan instances run.

Recommendation: Use SQLite's BEGIN IMMEDIATE or a row-level advisory lock around the risk check + order submission. For production multi-instance deployments, use a Redis-based distributed lock.

---

LOW Severity

L1: No rate limiting on web fragment endpoints

File: arbitrage/web/app.py:43-74

The /fragments/* endpoints are polled every 2-3 seconds via HTMX. There's no rate limiting or DoS protection. An attacker could consume excessive DB connections or CPU by spamming requests.

Recommendation: Add a simple request counter or slowapi middleware. Low priority since the web is intended for local use.

---

L2: JSHTMX loads from unpinned CDN

File: arbitrage/web/templates/index.html:6

<script src="https://unpkg.com/htmx.org@2.0.3"></script>

While unpinned (@2.0.3 is specified), unpkg doesn't pin the sub-version. If 2.0.3 is yanked or compromised, a different version could be served. For a self-hosted dashboard this is low risk, but worth noting.

Recommendation: Pin the full version hash or self-host the HTMX bundle.

---

L3: raw_json in DB could contain sensitive event metadata

File: arbitrage/clients/polymarket_rest.py:204

raw_json = json.dumps(raw_by_id.get(ev.id))

Full raw Gamma event JSON is persisted to the SQLite raw_json column. While this is Polymarket's public data, it's worth considering if any sensitive operational fields leak in (e.g., internal IDs, timestamps that reveal monitoring cadence).

Recommendation: Review the Gamma API response shape and selectively store only needed fields. Or accept this as low-risk since the data is public.

---

INFO (Good Practices Observed)

I1: Secrets management via SecretStr ✅

config.py:32-36 uses pydantic.SecretStr for all sensitive fields (private key, API key, secret, passphrase). SecretStr.get_secret_value() is the only way to access plaintext, which prevents accidental logging.

I2: Parameterized SQL everywhere ✅

All database queries use ? placeholders with parameter tuples. No string interpolation, no f-strings in SQL. Zero SQL injection risk.

I3: .env properly excluded ✅

.env is in .gitignore (line 6), .env.example contains only defaults. No secrets in source.

I4: Logging configured to avoid backtraces ✅

logging_setup.py:13-14 sets backtrace=False, diagnose=False — prevents stack traces from leaking sensitive local variable values in log files.

I5: Mode guard on LiveExecutor ✅

live_executor.py:117-119 raises RuntimeError if instantiated when ARB_MODE != live. This is a good belt-and-suspenders check.

I6: dry_run defaults to True in LiveExecutor ✅

live_executor.py:116: dry_run: bool = True — the constructor is safe by default. Only the CLI overrides it.

---

Risk Matrix (Live Mode Only)

Threat	Impact	Likelihood	Mitigation Status
Accidental live trade from misconfigured .env	Financial loss	Medium	Partial (mode check)
Remote kill/unkill via web UI	Operational	High	None
Unprotected market sell during unwind	Financial loss	Low	None
Supply chain via unpinned py-clob-client	Unknown	Low	None
TOCTOU race on basket limits	Over-exposure	Low-Medium	None
XSS in dashboard	Data leak	Very Low	Jinja2 autoescape
SQL injection	Data compromise	None	Parameterized queries

---

Priority Recommendations

1. 🔴 Add auth to web /kill and /unkill — or remove the web server from production Docker
2. 🔴 Add --confirm-live flag to arb scan — prevent accidental live execution
3. 🟡 Pin py-clob-client==0.34.6 in pyproject.toml
4. 🟡 Add min-price protection to _unwind SELL orders
5. 🟡 Change Docker web to 127.0.0.1 or add reverse proxy auth
6. 🟢 Add rate limiting to web endpoints
7. 🟢 Self-host or pin HTMX script hash