# Security Hardening & Bug Fixes Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Harden http2pic with optional API key auth, opt-in SSRF protection, viewport cap, and fix two correctness bugs (60ms timeout, viewport set after page load).

**Architecture:** All PHP changes stay in `web/index.php` and `src/helpers.php`. Config vars flow from `docker-compose.yml` env → `docker/start.sh` → `src/config.inc.php` defines. No new dependencies.

**Tech Stack:** PHP 8.4, php-webdriver, Caddy, Docker

---

## File Map

| File | What changes |
|------|-------------|
| `tests/test_helpers.php` | New — unit tests for `isPrivateIP()` and `getUserIP()` |
| `src/helpers.php` | Add `isPrivateIP()`, fix `getUserIP()`, fix `addToLog()` log injection |
| `docker/start.sh` | Write `API_KEY` and `BLOCK_PRIVATE_IPS` defines into config |
| `docker-compose.yml` | Add commented env var examples |
| `web/index.php` | API key check, SSRF check, viewport cap, viewport-before-get, timeout fix, generic errors |

---

## Task 1: Write failing tests for helpers

**Files:**
- Create: `tests/test_helpers.php`

- [ ] **Step 1: Create test file**

```php
<?php
define('DS', DIRECTORY_SEPARATOR);
define('ROOT', dirname(__FILE__) . '/..');
require_once ROOT . '/src/helpers.php';

$passed = 0;
$failed = 0;

function check(bool $condition, string $label): void {
    global $passed, $failed;
    if ($condition) { echo "PASS: $label\n"; $passed++; }
    else            { echo "FAIL: $label\n"; $failed++; }
}

// --- isPrivateIP ---
check(isPrivateIP('127.0.0.1'),        'loopback 127.0.0.1');
check(isPrivateIP('127.255.255.255'),   'loopback 127.255.255.255');
check(isPrivateIP('10.0.0.1'),          '10.x private');
check(isPrivateIP('10.255.255.255'),    '10.255.x private');
check(isPrivateIP('172.16.0.1'),        '172.16.x private');
check(isPrivateIP('172.31.255.255'),    '172.31.x private');
check(!isPrivateIP('172.15.255.255'),   '172.15.x is public');
check(!isPrivateIP('172.32.0.0'),       '172.32.x is public');
check(isPrivateIP('192.168.0.1'),       '192.168.x private');
check(isPrivateIP('169.254.169.254'),   'AWS metadata IP');
check(!isPrivateIP('8.8.8.8'),          'Google DNS is public');
check(!isPrivateIP('93.184.216.34'),    'example.com IP is public');
check(isPrivateIP('not-an-ip'),         'unparseable IP blocked');

// --- getUserIP ---
unset($_SERVER['HTTP_X_FORWARDED_FOR']);
$_SERVER['REMOTE_ADDR'] = '5.6.7.8';
check(getUserIP() === '5.6.7.8', 'getUserIP falls back to REMOTE_ADDR');

$_SERVER['HTTP_X_FORWARDED_FOR'] = '1.2.3.4, 10.0.0.1, 172.16.0.5';
check(getUserIP() === '1.2.3.4', 'getUserIP takes first X-Forwarded-For IP');

$_SERVER['HTTP_X_FORWARDED_FOR'] = '  9.9.9.9  ';
check(getUserIP() === '9.9.9.9', 'getUserIP trims whitespace');

echo "\n$passed passed, $failed failed\n";
exit($failed > 0 ? 1 : 0);
```

- [ ] **Step 2: Run tests — expect failures**

```bash
php tests/test_helpers.php
```

Expected: several `FAIL:` lines (functions don't exist yet or have wrong logic). PHP fatal error on `isPrivateIP` is fine.

---

## Task 2: Implement helpers changes

**Files:**
- Modify: `src/helpers.php`

- [ ] **Step 1: Replace `getUserIP()` and fix `addToLog()`, add `isPrivateIP()`**

Replace the entire contents of `src/helpers.php` with:

```php
<?php

function renderTemplate($templatename, $variables = [], $basepath = ROOT . '/src')
{
    ob_start();
    if (is_array($variables))
        extract($variables);
    if (file_exists($basepath . DS . 'templates' . DS . $templatename . '.php'))
        include($basepath . DS . 'templates' . DS . $templatename . '.php');
    else if (file_exists($basepath . DS . 'templates' . DS . $templatename))
        include($basepath . DS . 'templates' . DS . $templatename);
    $rendered = ob_get_contents();
    ob_end_clean();
    return $rendered;
}

function addToLog(string $data): void
{
    $data = str_replace(["\n", "\r", "\t"], ' ', $data);
    $fp = fopen(ROOT . DS . 'logs' . DS . 'app.log', 'a');
    fwrite($fp, date("d.m.y H:i") . "\t" . $data . "\n");
    fclose($fp);
}

function getUserIP(): string
{
    if (!empty($_SERVER['HTTP_X_FORWARDED_FOR']))
        return trim(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR'])[0]);
    return $_SERVER['REMOTE_ADDR'];
}

function isPrivateIP(string $ip): bool
{
    if (filter_var($ip, FILTER_VALIDATE_IP) === false) return true;

    $long = ip2long($ip);
    if ($long === false) return true;

    foreach ([
        [ip2long('127.0.0.0'),   0xFF000000], // 127.0.0.0/8  loopback
        [ip2long('10.0.0.0'),    0xFF000000], // 10.0.0.0/8   RFC1918
        [ip2long('172.16.0.0'),  0xFFF00000], // 172.16.0.0/12 RFC1918
        [ip2long('192.168.0.0'), 0xFFFF0000], // 192.168.0.0/16 RFC1918
        [ip2long('169.254.0.0'), 0xFFFF0000], // 169.254.0.0/16 link-local/metadata
    ] as [$base, $mask]) {
        if (($long & $mask) === ($base & $mask)) return true;
    }
    return false;
}
```

- [ ] **Step 2: Run tests — expect all pass**

```bash
php tests/test_helpers.php
```

Expected output ends with: `N passed, 0 failed`

- [ ] **Step 3: Commit**

```bash
git add src/helpers.php tests/test_helpers.php
git commit -m "feat: add isPrivateIP helper, fix getUserIP and addToLog"
```

---

## Task 3: Update config generation

**Files:**
- Modify: `docker/start.sh`
- Modify: `docker-compose.yml`

- [ ] **Step 1: Update `_buildConfig` in `docker/start.sh`**

Replace the `_buildConfig` function:

```bash
_buildConfig() {
    echo "<?php"
    echo "date_default_timezone_set('Europe/Vienna');"
    echo "define('URL','${URL:-http://localhost:8080}');"
    echo "define('API_KEY','${API_KEY:-}');"
    echo "define('BLOCK_PRIVATE_IPS',${BLOCK_PRIVATE_IPS:-false});"
    echo ""
}
```

- [ ] **Step 2: Add commented env var examples to `docker-compose.yml`**

Replace the `environment:` block:

```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 (recommended for public hosting)
```

- [ ] **Step 3: Verify config generation manually**

```bash
URL=http://localhost:8080 API_KEY=test BLOCK_PRIVATE_IPS=true bash -c '
_buildConfig() {
    echo "<?php"
    echo "date_default_timezone_set('"'"'Europe/Vienna'"'"');"
    echo "define('"'"'URL'"'"','"'"'${URL:-http://localhost:8080}'"'"');"
    echo "define('"'"'API_KEY'"'"','"'"'${API_KEY:-}'"'"');"
    echo "define('"'"'BLOCK_PRIVATE_IPS'"'"',${BLOCK_PRIVATE_IPS:-false});"
    echo ""
}
_buildConfig'
```

Expected output:
```
<?php
date_default_timezone_set('Europe/Vienna');
define('URL','http://localhost:8080');
define('API_KEY','test');
define('BLOCK_PRIVATE_IPS',true);
```

- [ ] **Step 4: Commit**

```bash
git add docker/start.sh docker-compose.yml
git commit -m "feat: add API_KEY and BLOCK_PRIVATE_IPS config vars"
```

---

## Task 4: Fix index.php bugs + viewport cap + generic errors

**Files:**
- Modify: `web/index.php`

This task rewrites the `case 'api':` block. Read the current state of `web/index.php` first, then replace the entire `case 'api':` block (from `case 'api':` through `break;`) with the following.

- [ ] **Step 1: Replace `case 'api':` block**

```php
    case 'api':
        $target = substr($_SERVER['REQUEST_URI'], 5);
        if (!$target || !filter_var($target, FILTER_VALIDATE_URL))
            $target = $_REQUEST['url'];
        if (!filter_var($target, FILTER_VALIDATE_URL)) {
            header('HTTP/1.0 400 Bad Request');
            echo 'Invalid URL';
            exit;
        }
        $ip = getUserIP();

        $viewport = $_REQUEST['viewport'] ?: '1024x768';
        if (!preg_match('/^\d+x\d+$/', $viewport)) {
            header('HTTP/1.0 400 Bad Request');
            echo 'Invalid viewport format. Use WIDTHxHEIGHT (e.g., 1024x768)';
            exit;
        }
        $vpParts = array_map('intval', explode('x', $viewport));
        if ($vpParts[0] > 3840 || $vpParts[1] > 2160) {
            header('HTTP/1.0 400 Bad Request');
            echo 'Viewport exceeds maximum (3840x2160)';
            exit;
        }

        $js = $_REQUEST['js'] == 'false' ? false : true;

        $serverUrl = 'http://localhost:4444';
        $options = new \Facebook\WebDriver\Chrome\ChromeOptions();
        $options->addArguments(['--headless', '--disable-gpu', '--no-sandbox', '--disable-dev-shm-usage']);

        $capabilities = DesiredCapabilities::chrome();
        $capabilities->setCapability(\Facebook\WebDriver\Chrome\ChromeOptions::CAPABILITY, $options);

        if (!$js)
            $capabilities->setCapability('javascriptEnabled', false);

        $driver = null;
        $error = null;
        try {
            $driver = RemoteWebDriver::create($serverUrl, $capabilities, 30000, 60000);
            $driver->manage()->window()->setSize(new \Facebook\WebDriver\WebDriverDimension($vpParts[0], $vpParts[1]));
            $driver->get($target);
            $driver->executeScript('document.body.style.overflow = "hidden";');

            addToLog("$ip\tRequested $target with viewport $viewport and js " . ($js ? 'enabled' : 'disabled'));

            $screenshot = $driver->takeScreenshot();
            header('Content-Type: image/png');
            header('Content-Length: ' . strlen($screenshot));
            echo $screenshot;
        } catch (Exception $e) {
            $error = $e->getMessage();
            addToLog("$ip\tRequested $target but resulted in error:\t" . $error);
        } finally {
            if ($driver instanceof RemoteWebDriver) {
                try { $driver->quit(); } catch (Exception $q) {}
            }
        }
        if ($error !== null) {
            header('HTTP/1.0 500 Internal Server Error');
            echo 'Screenshot failed';
        }

        break;
```

- [ ] **Step 2: Test viewport cap with curl**

```bash
curl -s -o /dev/null -w "%{http_code}" "http://localhost:8080/api?url=https://example.com&viewport=99999x99999"
```

Expected: `400`

```bash
curl -s -o /dev/null -w "%{http_code}" "http://localhost:8080/api?url=https://example.com&viewport=1024x768"
```

Expected: `200`

- [ ] **Step 3: Commit**

```bash
git add web/index.php
git commit -m "fix: viewport before page load, 60ms→60s timeout, viewport cap, generic errors"
```

---

## Task 5: Add API key authentication

**Files:**
- Modify: `web/index.php`

- [ ] **Step 1: Add API key check at top of `case 'api':` block**

Insert immediately after `case 'api':` (before the `$target` line):

```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;
            }
        }
```

- [ ] **Step 2: Test without key (no API_KEY configured)**

```bash
curl -s -o /dev/null -w "%{http_code}" "http://localhost:8080/api?url=https://example.com"
```

Expected: `200` (API_KEY not set in local config → open access)

- [ ] **Step 3: Test with key via header**

Temporarily set `API_KEY` in `src/config.inc.php` to `'testkey'`, then:

```bash
# missing key → 401
curl -s -o /dev/null -w "%{http_code}" "http://localhost:8080/api?url=https://example.com"
# header → 200
curl -s -o /dev/null -w "%{http_code}" -H "X-API-Key: testkey" "http://localhost:8080/api?url=https://example.com"
# query param → 200
curl -s -o /dev/null -w "%{http_code}" "http://localhost:8080/api?key=testkey&url=https://example.com"
# wrong key → 401
curl -s -o /dev/null -w "%{http_code}" -H "X-API-Key: wrongkey" "http://localhost:8080/api?url=https://example.com"
```

Expected: `401`, `200`, `200`, `401`

Revert the manual edit to `src/config.inc.php` after testing (it gets regenerated by `start.sh` in Docker).

- [ ] **Step 4: Document API key usage in CLAUDE.md**

In `CLAUDE.md`, add a new section under the existing content:

```markdown
## API Key

Set `API_KEY` env var in docker-compose to require authentication on all `/api` requests.
Leave unset (default) for open access.

```bash
# via header (preferred — not logged in access logs)
curl -H "X-API-Key: your-secret-key" "http://host/api?url=https://example.com"

# via query param
curl "http://host/api?key=your-secret-key&url=https://example.com"
```

## SSRF Protection

Set `BLOCK_PRIVATE_IPS=true` to reject requests to LAN, loopback, and cloud metadata IPs.
Recommended when hosting publicly. Default is off (allows local/LAN addresses).
```

- [ ] **Step 5: Commit**

```bash
git add web/index.php CLAUDE.md
git commit -m "feat: optional API key auth via X-API-Key header or ?key= param"
```

---

## Task 6: Add SSRF protection

**Files:**
- Modify: `web/index.php`

- [ ] **Step 1: Add SSRF check after viewport validation**

Insert after the `$js = ...` line (before `$serverUrl = ...`):

```php
        if (defined('BLOCK_PRIVATE_IPS') && BLOCK_PRIVATE_IPS) {
            $host = parse_url($target, PHP_URL_HOST);
            if (filter_var($host, FILTER_VALIDATE_IP)) {
                $resolvedIp = $host;
            } else {
                $resolvedIp = gethostbyname($host);
                if ($resolvedIp === $host) {
                    header('HTTP/1.0 403 Forbidden');
                    echo 'URL not allowed';
                    exit;
                }
            }
            if (isPrivateIP($resolvedIp)) {
                header('HTTP/1.0 403 Forbidden');
                echo 'URL not allowed';
                exit;
            }
        }
```

- [ ] **Step 2: Test SSRF protection**

Temporarily set `BLOCK_PRIVATE_IPS` to `true` in `src/config.inc.php`, then:

```bash
# private IP → 403
curl -s -o /dev/null -w "%{http_code}" "http://localhost:8080/api?url=http://192.168.1.1/"
# loopback → 403
curl -s -o /dev/null -w "%{http_code}" "http://localhost:8080/api?url=http://127.0.0.1/"
# metadata → 403
curl -s -o /dev/null -w "%{http_code}" "http://localhost:8080/api?url=http://169.254.169.254/"
# public URL → 200
curl -s -o /dev/null -w "%{http_code}" "http://localhost:8080/api?url=https://example.com"
```

Expected: `403`, `403`, `403`, `200`

Revert the manual edit to `src/config.inc.php` after testing.

- [ ] **Step 3: Commit**

```bash
git add web/index.php
git commit -m "feat: opt-in SSRF protection via BLOCK_PRIVATE_IPS env var"
```

---

## Final state of `web/index.php` `case 'api':` block

For reference, the complete block after all tasks:

```php
    case 'api':
        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;
            }
        }

        $target = substr($_SERVER['REQUEST_URI'], 5);
        if (!$target || !filter_var($target, FILTER_VALIDATE_URL))
            $target = $_REQUEST['url'];
        if (!filter_var($target, FILTER_VALIDATE_URL)) {
            header('HTTP/1.0 400 Bad Request');
            echo 'Invalid URL';
            exit;
        }
        $ip = getUserIP();

        $viewport = $_REQUEST['viewport'] ?: '1024x768';
        if (!preg_match('/^\d+x\d+$/', $viewport)) {
            header('HTTP/1.0 400 Bad Request');
            echo 'Invalid viewport format. Use WIDTHxHEIGHT (e.g., 1024x768)';
            exit;
        }
        $vpParts = array_map('intval', explode('x', $viewport));
        if ($vpParts[0] > 3840 || $vpParts[1] > 2160) {
            header('HTTP/1.0 400 Bad Request');
            echo 'Viewport exceeds maximum (3840x2160)';
            exit;
        }

        $js = $_REQUEST['js'] == 'false' ? false : true;

        if (defined('BLOCK_PRIVATE_IPS') && BLOCK_PRIVATE_IPS) {
            $host = parse_url($target, PHP_URL_HOST);
            if (filter_var($host, FILTER_VALIDATE_IP)) {
                $resolvedIp = $host;
            } else {
                $resolvedIp = gethostbyname($host);
                if ($resolvedIp === $host) {
                    header('HTTP/1.0 403 Forbidden');
                    echo 'URL not allowed';
                    exit;
                }
            }
            if (isPrivateIP($resolvedIp)) {
                header('HTTP/1.0 403 Forbidden');
                echo 'URL not allowed';
                exit;
            }
        }

        $serverUrl = 'http://localhost:4444';
        $options = new \Facebook\WebDriver\Chrome\ChromeOptions();
        $options->addArguments(['--headless', '--disable-gpu', '--no-sandbox', '--disable-dev-shm-usage']);

        $capabilities = DesiredCapabilities::chrome();
        $capabilities->setCapability(\Facebook\WebDriver\Chrome\ChromeOptions::CAPABILITY, $options);

        if (!$js)
            $capabilities->setCapability('javascriptEnabled', false);

        $driver = null;
        $error = null;
        try {
            $driver = RemoteWebDriver::create($serverUrl, $capabilities, 30000, 60000);
            $driver->manage()->window()->setSize(new \Facebook\WebDriver\WebDriverDimension($vpParts[0], $vpParts[1]));
            $driver->get($target);
            $driver->executeScript('document.body.style.overflow = "hidden";');

            addToLog("$ip\tRequested $target with viewport $viewport and js " . ($js ? 'enabled' : 'disabled'));

            $screenshot = $driver->takeScreenshot();
            header('Content-Type: image/png');
            header('Content-Length: ' . strlen($screenshot));
            echo $screenshot;
        } catch (Exception $e) {
            $error = $e->getMessage();
            addToLog("$ip\tRequested $target but resulted in error:\t" . $error);
        } finally {
            if ($driver instanceof RemoteWebDriver) {
                try { $driver->quit(); } catch (Exception $q) {}
            }
        }
        if ($error !== null) {
            header('HTTP/1.0 500 Internal Server Error');
            echo 'Screenshot failed';
        }

        break;
```