# 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`

```python
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`

```python
@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`

```yaml
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`

```python
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`

```toml
"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`

```python
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`

```html
<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`

```python
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**