263 lines
11 KiB
Markdown
263 lines
11 KiB
Markdown
# XSS Sink Remediation + CSP 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:** Eliminate the `innerHTML` XSS sink in `utils.showAlert` and add a nonce-based Content-Security-Policy header to the frontend route.
|
|
|
|
**Architecture:** Frontend `showAlert` is rewritten to build alert DOM via `document.createElement` / `textContent` (no `innerHTML`). Backend `read_root()` generates a per-request nonce, injects it into the inline `<script>` tag, and sets a strict CSP header on the HTML response.
|
|
|
|
**Tech Stack:** FastAPI (Python 3.14), vanilla JS frontend, pytest + fastapi.testclient.TestClient.
|
|
|
|
**Spec:** `docs/superpowers/specs/2026-07-08-xss-sink-csp-design.md`
|
|
|
|
## Global Constraints
|
|
|
|
- Run all commands from repo root (relative path `templates/index.html` in `main.py:329`).
|
|
- Python 3.14 (`.python-version`, Dockerfile `python:3.14-alpine`).
|
|
- No lint/typecheck/formatter/CI — pytest is the only verification step.
|
|
- Test command: `python -m pytest tests/ --cov=. --cov-config=.coveragerc -v` (or `./run_tests.sh`).
|
|
- `@lru_cache` OIDC helpers (`main.py:68-129`) — call `func.cache_clear()` when patching `requests`. Not relevant to this plan but good to know.
|
|
- `CLIENT_ID` has no default (`main.py:28`) — the test env sets it via `run_tests.sh`; the CSP test does not depend on it.
|
|
|
|
---
|
|
|
|
### Task 1: Add CSP header + nonce injection to `read_root()`
|
|
|
|
**Files:**
|
|
- Modify: `main.py:1-10` (add `import secrets`)
|
|
- Modify: `main.py:326-331` (`read_root` function)
|
|
- Test: `tests/test_main.py` (add new test)
|
|
|
|
**Interfaces:**
|
|
- Consumes: none
|
|
- Produces: `GET /` now returns a `Content-Security-Policy` response header containing a fresh per-request nonce, and the HTML body's inline `<script>` tag carries a matching `nonce="..."` attribute.
|
|
|
|
- [ ] **Step 1: Write the failing test**
|
|
|
|
Add this test to `tests/test_main.py` (after `test_get_client_config`, around line 33):
|
|
|
|
```python
|
|
def test_root_serves_csp_with_nonce():
|
|
"""Test that GET / returns a CSP header with a nonce and the inline script carries it."""
|
|
response = client.get("/")
|
|
assert response.status_code == 200
|
|
csp = response.headers.get("content-security-policy", "")
|
|
assert "nonce-" in csp
|
|
assert "script-src" in csp
|
|
assert "cdn.jsdelivr.net" in csp
|
|
assert "connect-src" in csp
|
|
assert "login.infomaniak.com" in csp
|
|
assert "api.coingecko.com" in csp
|
|
assert "object-src 'none'" in csp
|
|
body = response.text
|
|
assert "<script nonce=" in body
|
|
import re
|
|
match = re.search(r"nonce-([A-Za-z0-9_-]+)", csp)
|
|
assert match is not None
|
|
nonce = match.group(1)
|
|
assert f'nonce="{nonce}"' in body
|
|
```
|
|
|
|
- [ ] **Step 2: Run test to verify it fails**
|
|
|
|
Run: `python -m pytest tests/test_main.py::test_root_serves_csp_with_nonce -v`
|
|
Expected: FAIL — `assert "nonce-" in csp` fails because no CSP header is set yet (empty string).
|
|
|
|
- [ ] **Step 3: Add `import secrets` to `main.py`**
|
|
|
|
In `main.py`, add `import secrets` after `import os` (line 9), keeping it in the stdlib group:
|
|
|
|
```python
|
|
import os
|
|
import secrets
|
|
from functools import lru_cache
|
|
```
|
|
|
|
(Line 10's `from functools import lru_cache` stays where it is — just insert `import secrets` between lines 9 and 10.)
|
|
|
|
- [ ] **Step 4: Rewrite `read_root()` with nonce injection + CSP header**
|
|
|
|
Replace the body of `read_root()` at `main.py:326-331` with:
|
|
|
|
```python
|
|
@app.get("/", response_class=HTMLResponse)
|
|
async def read_root():
|
|
"""Serve the frontend HTML file"""
|
|
nonce = secrets.token_urlsafe(16)
|
|
with open("templates/index.html", "r") as file:
|
|
html_content = file.read()
|
|
html_content = html_content.replace(
|
|
'<script>', f'<script nonce="{nonce}">', 1
|
|
)
|
|
csp = (
|
|
"default-src 'self'; "
|
|
f"script-src 'self' 'nonce-{nonce}' https://cdn.jsdelivr.net; "
|
|
"style-src 'self' https://cdn.jsdelivr.net; "
|
|
"connect-src 'self' https://login.infomaniak.com https://api.coingecko.com; "
|
|
"img-src 'self'; "
|
|
"object-src 'none'; "
|
|
"base-uri 'self'; "
|
|
"frame-ancestors 'none'"
|
|
)
|
|
return HTMLResponse(
|
|
content=html_content,
|
|
status_code=200,
|
|
headers={"Content-Security-Policy": csp},
|
|
)
|
|
```
|
|
|
|
The `replace('<script>', ..., 1)` targets only the bare inline tag at `templates/index.html:62`. The Bootstrap tag at line 8 is `<script src=...>` (does not match the literal `<script>`) and is authorized by the `https://cdn.jsdelivr.net` entry in `script-src`.
|
|
|
|
- [ ] **Step 5: Run test to verify it passes**
|
|
|
|
Run: `python -m pytest tests/test_main.py::test_root_serves_csp_with_nonce -v`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 6: Run full suite to confirm no regressions**
|
|
|
|
Run: `./run_tests.sh`
|
|
Expected: All tests pass (including the new one). Coverage report builds in `htmlcov/`.
|
|
|
|
- [ ] **Step 7: Commit**
|
|
|
|
```bash
|
|
git add main.py tests/test_main.py
|
|
git commit -m "fix(security): add nonce-based CSP header to frontend route"
|
|
```
|
|
|
|
---
|
|
|
|
### Task 2: Remove the `innerHTML` XSS sink in `showAlert`
|
|
|
|
**Files:**
|
|
- Modify: `templates/index.html:162-164` (`showAlert` method)
|
|
- Modify: `templates/index.html:468` (caller — cached secret)
|
|
- Modify: `templates/index.html:584` (caller — server secret phrase)
|
|
- Modify: `templates/index.html:619-623` (caller — BTC price)
|
|
|
|
**Interfaces:**
|
|
- Consumes: none
|
|
- Produces: `utils.showAlert(element, message, type)` now accepts either a plain `string` (rendered as `textContent`) or a structured `{title, body}` object (title rendered in `<strong>` as `textContent`, body rendered as text). The `innerHTML` sink is gone.
|
|
|
|
**Note on testing:** This repo has no JavaScript test framework. This task is verified by (a) the Python test from Task 1 confirming the page still serves correctly, and (b) manual browser verification steps at the end. The CSP from Task 1 provides defense-in-depth: even if a future caller passes attacker-controlled data, inline script execution is blocked.
|
|
|
|
- [ ] **Step 1: Rewrite `showAlert` to build DOM instead of using `innerHTML`**
|
|
|
|
In `templates/index.html`, replace the `showAlert` method (lines 162-164):
|
|
|
|
Current:
|
|
```js
|
|
showAlert(element, message, type = 'info') {
|
|
element.innerHTML = `<div class="alert alert-${type}" role="alert">${message}</div>`;
|
|
},
|
|
```
|
|
|
|
New:
|
|
```js
|
|
showAlert(element, message, type = 'info') {
|
|
element.replaceChildren();
|
|
const alert = document.createElement('div');
|
|
alert.className = `alert alert-${type}`;
|
|
alert.setAttribute('role', 'alert');
|
|
if (typeof message === 'string') {
|
|
alert.textContent = message;
|
|
} else if (message && typeof message === 'object') {
|
|
if (message.title) {
|
|
const strong = document.createElement('strong');
|
|
strong.textContent = message.title;
|
|
alert.appendChild(strong);
|
|
}
|
|
if (message.body) {
|
|
if (message.title) {
|
|
alert.appendChild(document.createTextNode(' '));
|
|
}
|
|
alert.appendChild(document.createTextNode(message.body));
|
|
}
|
|
}
|
|
element.appendChild(alert);
|
|
},
|
|
```
|
|
|
|
- [ ] **Step 2: Update caller at line 468 (cached secret phrase)**
|
|
|
|
Current (`templates/index.html:468`):
|
|
```js
|
|
utils.showAlert(elements.secretResult, `<strong>Secret Phrase:</strong> ${cachedSecret}`, 'success');
|
|
```
|
|
|
|
New:
|
|
```js
|
|
utils.showAlert(elements.secretResult, { title: 'Secret Phrase:', body: cachedSecret }, 'success');
|
|
```
|
|
|
|
- [ ] **Step 3: Update caller at line 584 (server secret phrase)**
|
|
|
|
Current (`templates/index.html:584`):
|
|
```js
|
|
utils.showAlert(elements.secretResult, `<strong>Secret Phrase:</strong> ${data.secret_phrase}`, 'success');
|
|
```
|
|
|
|
New:
|
|
```js
|
|
utils.showAlert(elements.secretResult, { title: 'Secret Phrase:', body: data.secret_phrase }, 'success');
|
|
```
|
|
|
|
- [ ] **Step 4: Update caller at lines 619-623 (BTC price)**
|
|
|
|
Current (`templates/index.html:619-623`):
|
|
```js
|
|
utils.showAlert(
|
|
elements.btcPriceResult,
|
|
`<strong>1 Bitcoin (BTC) = CHF ${price.toLocaleString(undefined, {minimumFractionDigits: 2, maximumFractionDigits: 2})}</strong>`,
|
|
'success'
|
|
);
|
|
```
|
|
|
|
New:
|
|
```js
|
|
utils.showAlert(
|
|
elements.btcPriceResult,
|
|
{ title: `1 Bitcoin (BTC) = CHF ${price.toLocaleString(undefined, {minimumFractionDigits: 2, maximumFractionDigits: 2})}` },
|
|
'success'
|
|
);
|
|
```
|
|
|
|
- [ ] **Step 5: Verify no `innerHTML` remains in `showAlert` or its callers**
|
|
|
|
Run: `grep -n 'innerHTML' templates/index.html`
|
|
Expected: no matches in the `showAlert` method or its callers. (If other unrelated `innerHTML` uses exist elsewhere in the file, they are out of scope for this task — leave them.)
|
|
|
|
- [ ] **Step 6: Run full Python suite to confirm the page still serves**
|
|
|
|
Run: `./run_tests.sh`
|
|
Expected: All tests pass, including `test_root_serves_csp_with_nonce` from Task 1.
|
|
|
|
- [ ] **Step 7: Manual browser verification**
|
|
|
|
Start the dev server: `uvicorn main:app --host 0.0.0.0 --port 8000 --reload`
|
|
|
|
Open `http://localhost:8000` and verify:
|
|
- The page loads with no browser console errors about CSP violations.
|
|
- The Bootstrap styling still renders (CSS from cdn.jsdelivr.net loads).
|
|
- Click "Fetch Bitcoin Price" — the success alert shows **1 Bitcoin (BTC) = CHF ...** in bold.
|
|
- (If you have OIDC credentials) Log in, click "Refresh" on the Secret Phrase card — the success alert shows **Secret Phrase:** in bold followed by the phrase text.
|
|
- Trigger an error (e.g., click "Validate Token" with no token) — the error alert renders the message as plain text.
|
|
|
|
- [ ] **Step 8: Commit**
|
|
|
|
```bash
|
|
git add templates/index.html
|
|
git commit -m "fix(security): remove innerHTML XSS sink in showAlert"
|
|
```
|
|
|
|
---
|
|
|
|
## Summary
|
|
|
|
| Task | What | Verification |
|
|
|------|------|--------------|
|
|
| 1 | CSP header + nonce injection in `read_root()` | pytest (automated) |
|
|
| 2 | `showAlert` DOM rewrite + 3 caller updates | grep + manual browser check |
|
|
|
|
After both tasks, the finding is resolved: the `innerHTML` sink is structurally gone, and a strict nonce-based CSP blocks inline script execution as defense-in-depth.
|