docs: add security hardening design spec

docs/superpowers/specs/2026-04-20-security-hardening-design.md

@@ -0,0 +1,169 @@
+# http2pic Security Hardening & Bug Fixes
+
+**Date:** 2026-04-20
+**Status:** Approved
+
+## Summary
+
+Harden http2pic against abuse and fix two correctness bugs. All changes target `web/index.php`, `src/helpers.php`, `docker/start.sh`, and `docker-compose.yml`. No new dependencies.
+
+---
+
+## 1. Config & Environment
+
+Two new optional env vars. Both default to off so existing deployments are unaffected.
+
+| Var | Default | Effect |
+|-----|---------|--------|
+| `API_KEY` | `""` | If non-empty, all `/api` requests must supply it |
+| `BLOCK_PRIVATE_IPS` | `false` | If `true`, rejects URLs that resolve to private/loopback/metadata IPs |
+
+`docker/start.sh` writes both into `src/config.inc.php` alongside the existing `URL` define:
+
+```php
+define('API_KEY', '${API_KEY:-}');
+define('BLOCK_PRIVATE_IPS', ${BLOCK_PRIVATE_IPS:-false});
+```
+
+`docker-compose.yml` gets commented-out examples:
+
+```yaml
+environment:
+  - URL=http://localhost:8080
+  # - API_KEY=your-secret-key       # if set, all /api requests must provide it
+  # - BLOCK_PRIVATE_IPS=true        # block LAN/loopback/metadata IPs (safe default for public hosting)
+```
+
+---
+
+## 2. API Key Authentication
+
+Checked at the top of `case 'api':`, before any Chrome work.
+
+**Logic:** if `API_KEY` is defined and non-empty, require the key. Accept it from either:
+- HTTP header: `X-API-Key: <key>`
+- Query param: `?key=<key>`
+
+Header takes priority. Returns `401 Unauthorized` if missing or wrong.
+
+```php
+if (defined('API_KEY') && API_KEY !== '') {
+    $provided = $_SERVER['HTTP_X_API_KEY'] ?? $_REQUEST['key'] ?? '';
+    if ($provided !== API_KEY) {
+        header('HTTP/1.0 401 Unauthorized');
+        echo 'Invalid or missing API key';
+        exit;
+    }
+}
+```
+
+**Usage examples (to add to README):**
+
+```bash
+# via header (preferred — not logged in access logs)
+curl -H "X-API-Key: secret" "http://host/api?url=https://example.com"
+
+# via query param (simpler for browser use)
+curl "http://host/api?key=secret&url=https://example.com"
+```
+
+---
+
+## 3. SSRF Protection (opt-in)
+
+Active only when `BLOCK_PRIVATE_IPS=true`. Runs after URL format validation, before spawning Chrome.
+
+**Flow:**
+1. Extract hostname from target URL via `parse_url()`
+2. Resolve to IP via `gethostbyname()`
+3. If resolution fails (returns same string) or IP is private → `403 Forbidden`
+
+**Blocked ranges:** `127.0.0.0/8`, `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`, `169.254.0.0/16` (AWS/cloud metadata).
+
+Implementation uses `ip2long()` + bitmask checks — no regex.
+
+A new `isPrivateIP(string $ip): bool` helper lives in `src/helpers.php`.
+
+Returns `403 Forbidden` with body `URL not allowed` (no detail about why, to avoid enumeration).
+
+---
+
+## 4. Bug Fixes
+
+### 4a. RemoteWebDriver request timeout (60ms → 60s)
+
+```php
+// before
+RemoteWebDriver::create($serverUrl, $capabilities, 30000, 60);
+// after
+RemoteWebDriver::create($serverUrl, $capabilities, 30000, 60000);
+```
+
+4th param is milliseconds. `60` = 60ms, effectively zero. Fix to `60000`.
+
+### 4b. Viewport set before page load
+
+Move `window()->setSize()` to before `driver->get()` so the page loads at the correct viewport from the start. This ensures responsive layouts and media queries fire at the intended dimensions.
+
+---
+
+## 5. Security Hardening (always applied)
+
+### 5a. Viewport dimension cap
+
+After format validation, reject dimensions exceeding 3840×2160:
+
+```php
+$parts = explode('x', $viewport);
+if ((int)$parts[0] > 3840 || (int)$parts[1] > 2160) {
+    header('HTTP/1.0 400 Bad Request');
+    echo 'Viewport exceeds maximum (3840x2160)';
+    exit;
+}
+```
+
+Prevents memory exhaustion via `99999x99999` requests.
+
+### 5b. Generic error responses
+
+Catch block logs full exception message but returns only:
+
+```
+Screenshot failed
+```
+
+Stops ChromeDriver internals (internal hostnames, paths, stack traces) leaking to clients.
+
+### 5c. Log injection fix
+
+`addToLog()` strips `\n`, `\r`, `\t` from data before writing:
+
+```php
+$data = str_replace(["\n", "\r", "\t"], ' ', $data);
+```
+
+Prevents crafted URLs/IPs from injecting fake log entries.
+
+### 5d. getUserIP() fix
+
+Drop `HTTP_CLIENT_IP` (client-controlled, untrustworthy). Take only the first IP from `X-Forwarded-For` (Caddy appends subsequent hops; first entry is the real client):
+
+```php
+function getUserIP() {
+    if (!empty($_SERVER['HTTP_X_FORWARDED_FOR']))
+        return trim(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR'])[0]);
+    return $_SERVER['REMOTE_ADDR'];
+}
+```
+
+---
+
+## Files Changed
+
+| File | Changes |
+|------|---------|
+| `web/index.php` | API key check, SSRF check, viewport cap, viewport-before-get, timeout fix, generic errors |
+| `src/helpers.php` | `isPrivateIP()` helper, log injection fix, `getUserIP()` fix |
+| `docker/start.sh` | Write `API_KEY` and `BLOCK_PRIVATE_IPS` into config |
+| `docker-compose.yml` | Commented env var examples |
+| `src/config.inc.php` | (generated) gets two new defines |