Add phased-execution planning structure

Introduce .agents/ (PLAN.md with locked architectural anchors, phase roadmap under phases/todo/) and AGENTS.md rules for agents working in the repo. Queues the pending phases: fix history XSS, simulate fetch commands, and nginx security headers.
2026-09-18 17:23:41 -04:00
parent 010758b3e9
commit a2f331e526
13 changed files with 963 additions and 0 deletions
@@ -0,0 +1,238 @@
+# Security & Code Quality Audit — homepage
+
+**Date:** 2026-09-18
+**Auditor:** pi agent (read-only audit; no fixes applied at audit time)
+**Method:** manual source review (all files) + tooling: `git log`, `git grep` secret scan, `ssh-keygen` key validation, `curl` live-header probe.
+
+## Disposition (2026-09-18, same session as the audit)
+
+- **M-1, M-2, M-4 → scheduled as phases** under the phased-execution protocol:
+  `.agents/phases/todo/01_fix_history_xss/`, `02_simulate_fetch_commands/`,
+  `03_nginx_security_headers/` (see `.agents/PLAN.md` §7). Execute with the
+  `phased-execution` skill (`auto-phase.sh`).
+- **M-3 → DISMISSED (false positive per project owner, 2026-09-18).** The
+  host firewall already restricts access to the container port and the site
+  is fronted by the Caddy TLS edge; the README's `0.0.0.0:8080` binding is
+  accepted as-is.
+- **Code-quality findings (Q-1…Q-12, plus the vim `dd` dead path) → executed
+  directly in-session** (explicit owner instruction to run them outside the
+  phase pipeline): module split (`terminal.js` 1353 → 652 lines +
+  `terminal-commands.js` / `terminal-vim.js` / `terminal-achievements.js`),
+  dead-code removal, expand/collapse helpers, rack-decoration CSS classes,
+  `hidden`-attribute reveal, a11y label, dynamic `date`, vim line-deletion
+  fix. A 44-test Playwright E2E regression suite (`tests/e2e/`, run with
+  `./build.sh && npm test`) was created and is green.
+- Remaining Low items (L-1, L-3, L-5, L-7, L-8) stay unphased; L-1 is folded
+  into Phase 03's verification notes, L-2 into Phase 03 task 01.
+
+## Scope & Stack
+
+- **Stack:** static site (vanilla HTML/CSS/JS, no framework, no build deps) → hashed via `build.sh` (md5 cache-bust + `sed` rewrites) → served by `nginx:alpine` on `:8080` (plain HTTP, TLS expected at an external Caddy edge) → Docker/Podman image pushed to `gitea.reeseapps.com/services/homepage` via Gitea Actions.
+- **Core functionality:** personal portfolio with an interactive fake terminal (easter-egg commands, vim simulator, achievements in `localStorage`).
+- **Data sensitivity:** none at rest. Public PII only (name, email, GPG public keys). No auth, no backend, no forms.
+- **Assumptions:** TLS termination + HSTS happen at the Caddy reverse proxy in front of this container (container only listens on 8080 HTTP). Live site headers could not be verified from this network.
+
+**Overall:** low-risk attack surface (static, no input persistence, no backend). No Critical/High findings. One Medium XSS-pattern, two Medium deployment issues, several Low hardening items, and a number of code-quality debts (dead code, duplication, untested build).
+
+## Findings (severity-ranked)
+
+---
+
+### M-1. XSS sink: terminal command history recalled via `innerHTML`
+- **Severity:** Medium (High if ever served without the CSP below)
+- **Type:** CWE-79 (DOM-based XSS via innerHTML)
+- **Location:** `src/terminal.js`, ArrowUp/ArrowDown handlers in the terminal `keydown` listener (~lines 330–370)
+- **Description:** The Enter handler stores raw user input into `commandHistory` (captured via `textContent`, so typing is safe). But ArrowUp/ArrowDown recall it with:
+  ```js
+  lastLine.innerHTML = '<span class="terminal-prompt">$</span> ' + cmdLine + ' ';
+  ```
+  `cmdLine` is untrusted input injected into `innerHTML`.
+- **PoC:** In the site's terminal type:
+  ```
+  <img src=x onerror=alert(1)>
+  ```
+  press Enter, then press ↑. The payload is injected as HTML.
+  Mitigated *today* by the deployed CSP `script-src 'self'` (inline handlers and inline `<script>` are blocked), so impact on the production nginx container is limited to DOM tampering and click-based `javascript:` links. However: (a) it is fully exploitable if the site is ever served without this CSP (e.g. a Caddy static-file mount or dev server — see M-2/M-3 for how easily that happens), (b) any future CSP relaxation (e.g. adding `'unsafe-inline'` for styles) re-arms it, and (c) a co-browsing victim who is talked into typing a "magic command" gets exploited.
+- **Remediation:** Render recalled history exactly like `updateDisplay()` does — never `innerHTML` with user data:
+  ```js
+  const lastLine = content.lastElementChild;
+  lastLine.textContent = '';
+  const p = document.createElement('span');
+  p.className = 'terminal-prompt';
+  p.textContent = isRoot ? '#' : '$';
+  lastLine.append(p, document.createTextNode(' ' + cmdLine + ' '));
+  const c = document.createElement('span');
+  c.className = 'terminal-cursor';
+  lastLine.appendChild(c);
+  ```
+  Apply to both ArrowUp and ArrowDown branches (3 occurrences).
+
+---
+
+### M-2. Arbitrary client-side `fetch(url)` via `curl`/`wget` commands (SSRF-style primitive + broken in prod)
+- **Severity:** Medium
+- **Type:** CWE-918 (client-side request forgery), CWE-601-adjacent (unvalidated URL)
+- **Location:** `src/terminal.js`, the `curl ` / `wget ` branches of the Enter handler (~lines 884–920)
+- **Description:** `fetch(url)` is called with a visitor-supplied, unvalidated URL, from the visitor's own browser, and the response body is displayed. Consequences:
+  1. **Broken in production:** the CSP is `default-src 'self'` with no `connect-src`, so `connect-src` falls back to `'self'` — every external fetch is silently blocked by the browser. The `curl`/`wget` feature (and the `web_navigator` achievement) only works on a CSP-less dev server. When this is noticed and "fixed" by adding `connect-src *`, the next problem appears:
+  2. **Cookie-attached requests:** `fetch()` defaults to `credentials: 'same-origin'`. Once the site ever has *any* authenticated/cookie endpoint, a visitor can run `curl /internal-path` (or the site origin) and read the response text in-page — a self-service exfiltration/CSRF primitive.
+- **PoC:** In a dev server (no CSP): `curl https://api.example.com/secret` → the browser fetches it and prints the body. After a future `connect-src *`: `curl https://<site>/admin` with the visitor's cookies attached.
+- **Remediation (pick one):**
+  - *Preferred:* this is a fake terminal — make `curl`/`wget` simulated like every other command (canned output). Zero risk, feature "works" everywhere.
+  - *If real fetches are wanted:* validate URL (`new URL(url)`, require `https:`, require hostname in an explicit allowlist), pass `{ credentials: 'omit' }`, and set an explicit `connect-src` in the CSP listing only those origins.
+
+---
+
+### M-3. `podman run -p 0.0.0.0:8080:8080` in README — plain HTTP exposed on all interfaces
+- **Status:** ❌ **DISMISSED — false positive** (project owner, 2026-09-18).
+  The host firewall restricts access to the container port and the site is
+  served through the Caddy TLS edge in production. No change made.
+- ~~Severity: Medium~~ (original assessment kept for the record)
+- **Type:** CWE-319 (cleartext exposure), CWE-614
+- **Location:** `README.md` ("Run the container" section)
+
+---
+
+### M-4. Security headers lost on `/index.html` and all static assets (nginx `add_header` inheritance trap)
+- **Severity:** Medium (defense-in-depth gap; main `/` URL is fine)
+- **Type:** CWE-693 (protection mechanism failure — misconfiguration)
+- **Location:** `nginx.conf`, server block vs. `location = /index.html` and `location ~* \.(css|js|jpeg|...)$`
+- **Description:** nginx only inherits `add_header` directives from an outer level if the current level defines **none**. Both of those locations define their own `add_header` (cache headers), so they serve responses with **no CSP, no X-Frame-Options, no X-Content-Type-Options, no Referrer-Policy**. A direct request to `/index.html` or any `.css`/`.js` asset is served without security headers — including CSP, which is the mitigation for M-1.
+- **PoC:** `curl -sI https://<site>/index.html` → no `Content-Security-Policy` header (vs. `curl -sI https://<site>/` which has it).
+- **Remediation:** put the shared security headers in a snippet, e.g. `headers.inc`:
+  ```
+  add_header X-Frame-Options "SAMEORIGIN" always;
+  add_header X-Content-Type-Options "nosniff" always;
+  add_header Referrer-Policy "strict-origin-when-cross-origin" always;
+  add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self';" always;
+  ```
+  and `include /etc/nginx/headers.inc;` in **every** location block (alongside the cache headers).
+
+---
+
+### L-1. No HSTS emitted by this service (verify at the edge)
+- **Severity:** Low
+- **Type:** CWE-319
+- **Location:** `nginx.conf` (absent), assumed Caddy edge
+- **Description:** No `Strict-Transport-Security` anywhere in this repo. If Caddy isn't sending it, first-visit users are downgrade-attack-exposed.
+- **Remediation:** Verify `curl -sI https://reeseapps.com/ | grep -i strict-transport`. If absent, add at the Caddy edge (`header Strict-Transport-Security "max-age=31536000; includeSubDomains"`). (Don't add it inside the 8080 container — it must be set by the TLS terminator.)
+
+### L-2. `server_tokens` not disabled
+- **Severity:** Low
+- **Type:** CWE-200 (version disclosure)
+- **Location:** `nginx.conf`
+- **Description:** Default nginx advertises its version in the `Server` header.
+- **Remediation:** Add `server_tokens off;` (server or http level — in a conf.d snippet it must go in the `server` block).
+
+### L-3. `style-src 'unsafe-inline'` in CSP
+- **Severity:** Low
+- **Type:** CWE-79 (reduced CSP strength)
+- **Location:** `nginx.conf` CSP
+- **Description:** Required today because `index.html` and the JS use dozens of inline `style="..."` attributes and `el.style.x = ...` (note: JS-sets `el.style` are **not** blocked by CSP; only inline style *attributes* are). `script-src 'self'` is clean — no inline scripts, no inline handlers in HTML, which is the important half.
+- **Remediation (optional):** move inline `style="..."` attributes in `index.html` into classes; then drop `'unsafe-inline'`. Low priority; do together with other CSS cleanup.
+
+### L-4. README documents a nonexistent `homepage.container` and a conflicting `./src` mount
+- **Severity:** Low (doc/ops correctness)
+- **Location:** `README.md` (Deploy with Quadlet, Run the container)
+- **Description:** `cp homepage.container ~/.config/containers/systemd/` references a file that is not in the repo (verified: not tracked, not present). Also `-v ./src:/usr/share/nginx/html` mounts *unhashed* source, contradicting the hashed `dist/` build the image produces — following the README yields a working-but-unhashed deployment that bypasses the cache-busting pipeline.
+- **Remediation:** Either commit a `homepage.container` unit file, or rewrite the README to describe the actual image build (`podman build` + run without the volume mount).
+
+### L-5. Fake terminal shows a syntactically-valid SSH public key
+- **Severity:** Low / Informational
+- **Type:** CWE-540 (inclusion of functionality from untrusted control plane — informational only)
+- **Location:** `src/terminal.js`, `commands['cat ~/.ssh/id_ed25519.pub']`
+- **Description:** `ssh-keygen -l` confirms the displayed key parses as a real ED25519 key (fingerprint `SHA256:A1/bjtrxqkXFPXawKfVD2nHW4LBElmQ37+r55YFCJmo`). Public keys are public, so exposure is not a leak. Two notes: (a) if this is a *real* homelab key, publishing it bootstraps trust — an attacker who can MITM the site (or its GPG verification chain) could substitute their key for a visitor adding it to `known_hosts`; keep the page's TLS chain strong and consider displaying only the fingerprint. (b) If it's fabricated, add a comment or make it obviously fictitious so nobody mistakes it for a trust anchor.
+- **Remediation:** Decide intent; if real, show fingerprint-only; if fake, mark it clearly.
+
+### L-6. Easter eggs wipe the entire page (`document.body.innerHTML = ''`)
+- **Severity:** Low (intended behavior, noted for quality)
+- **Location:** `src/terminal.js` — `exit` (non-root) and `rm -rf /` (root) branches
+- **Description:** Both destroy the whole DOM (nav, styles references, script state). Recovery is a full reload; the `exit` login screen has no way back without refresh. Fine as an easter egg, but fragile to future edits (e.g. any service worker or analytics would also be nuked).
+- **Remediation:** Optional — scope the wipe to a dedicated overlay instead of `document.body`, or add a "reload" hint on the login screen.
+
+### L-7. CI supply-chain hardening (optional for a personal repo)
+- **Severity:** Low / Informational
+- **Location:** `.gitea/workflows/build-push.yml`
+- **Description:** Actions pinned to major versions only (`actions/checkout@v4`, `docker/build-push-action@v6`), image not signed, `push: true` on every push to `main`. Acceptable for a self-hosted personal Gitea; noted for completeness.
+- **Remediation (optional):** pin to full commit SHAs, sign the image with cosign + Sigstore keyless, restrict the workflow to `main` only (drop the `release` trigger or guard it).
+
+### L-8. No tests; `build.sh` is untested and fragile
+- **Severity:** Low (quality)
+- **Location:** `tests/` (empty), `build.sh`
+- **Description:** `build.sh` uses `sed` string-rewrites to hash asset references — it silently no-ops if an asset is referenced in any other form (e.g. `url(...)` in CSS, single quotes, CSS `@import`). Nothing verifies dist integrity.
+- **Remediation:** Add a small test (shell + `grep` is enough): run `build.sh`, then assert (1) every asset file in `src` exists in `dist` with a hash, (2) no unhashed `*.css|*.js|*.jpeg|*.ico|*.svg` references remain in `dist/index.html`, (3) `dist/index.html` differs from `src/index.html`. Wire into the Gitea workflow before the Docker build.
+
+---
+
+## Code Quality Findings (non-security)
+
+| # | Issue | Location |
+|---|-------|----------|
+| Q-1 | `terminal.js` is 1353 lines; `createTerminal()` is ~700 lines with a full vim simulator (~350 lines) nested inside the keydown handler. Split into modules: terminal core / command registry / vim / achievements. | `src/terminal.js` |
+| Q-2 | **Dead code:** an outer `const commands = {...}` (~line 120) is shadowed by a second, near-identical `const commands = {...}` inside the Enter handler (~line 850). The outer map is never used and has already drifted (different `help` text, missing `apt`/`dnf` entries). Delete the outer one; keep a single registry (also fixes the `date` snapshot bug below). | `src/terminal.js` |
+| Q-3 | `let isLoginScreen` is written, never read. | `src/terminal.js` |
+| Q-4 | Copy-paste bug (harmless): `cmdText.startsWith('wget ') \|\| cmdText.startsWith('wget ')` — second condition should probably have been the `curl ` check that the first branch already handles; as written it's redundant. | `src/terminal.js` curl/wget branch |
+| Q-5 | Grown/expanded terminal state is toggled in **4 places** with duplicated logic (terminal `focus`, terminal `blur`, `mobileInput` `blur`, hero `click`). Extract `expandTerminal()` / `collapseTerminal()` helpers. | `src/terminal.js`, `src/script.js` |
+| Q-6 | `const brands = ['Framework']` — random selection from a single-element array. | `src/script.js` `createServerRack()` |
+| Q-7 | Repeated inline `el.style.position/left/top/transform` boilerplate for rack decorations — move to CSS classes (also enables L-3 cleanup). | `src/script.js` |
+| Q-8 | `revealAchievements()` sets `section.style.display = 'block'` but never clears the `hidden` attribute. Works only because inline style beats the UA `[hidden]` rule — fragile. Use `section.hidden = false`. | `src/terminal.js` |
+| Q-9 | **Accessibility:** the 1px invisible `mobileInput` (the real key source) has no `aria-label`/role; the `tabindex="0"` terminal div has no keyboard handler of its own. Screen-reader users cannot operate the terminal. Add a labelled input (visually-hidden pattern) and/or an `aria-hidden` toggle; also `#achievements` nav is injected client-side only. | `src/terminal.js`, `src/index.html` |
+| Q-10 | `date` command is a page-load snapshot (the commands map is built once) — reports the time the page loaded, not "now". Make it a function or compute at dispatch. | `src/terminal.js` |
+| Q-11 | `error_page 404 /index.html` + `try_files ... /index.html` → every unknown path returns the homepage body with a 404 status. Acceptable for a one-pager, but be aware all soft-404s serve full content. | `nginx.conf` |
+| Q-12 | Local `dist/` is stale (contains an older `index.html` with different classes) — untracked, so no repo harm, but rebuild before deploying to avoid confusion. | `dist/` |
+
+## What's already good (verified, not assumed)
+
+- No secrets in the repo or git history (secret-pattern grep across tracked files + `git log`); CI uses Gitea `secrets.*` correctly.
+- No inline `<script>` anywhere; JS loaded from local hashed files with `defer`; CSP `script-src 'self'` is meaningful.
+- All user *typing* paths in the terminal render via `textContent` / `insertAdjacentText`; the vim module correctly uses `escapeHtml()` before its only `innerHTML` sinks. (The history-recall path, M-1, is the exception.)
+- `rel="noopener"` on every `target="_blank"` link.
+- No external CDNs or third-party runtime dependencies — small, auditable supply chain.
+- `.gitignore` correctly excludes `dist/` (verified via `git ls-files`); Dockerfile removes build context after hashing; container has a healthcheck.
+- GPG key block is public material (expected exposure, correctly presented with fingerprints).
+- `style.css`: 0 `!important`, sane z-index usage.
+
+## Remediation Task List (final state, 2026-09-18)
+
+**Phased (run via `phased-execution` skill, in order):**
+1. **[M-1]** → `01_fix_history_xss` — replace the 3 `lastLine.innerHTML = ... + cmdLine` history-recall writes with the safe `updateDisplay()` path + XSS regression test.
+2. **[M-2]** → `02_simulate_fetch_commands` — replace `curl`/`wget` real `fetch(url)` with deterministic simulated output (`simulatedCurlOutput`/`simulatedWgetOutput`) + no-network regression test.
+3. **[M-4] (+L-2)** → `03_nginx_security_headers` — extract shared `add_header` snippet, `include` in every `location`, `server_tokens off;` + container header-check script.
+
+**Dismissed:**
+4. **[M-3]** — false positive per owner (firewall + Caddy edge already cover it). No change.
+
+**Executed directly in-session (owner instruction; not phased):**
+10. **[Q-2, Q-3, Q-4, Q-10]** done — outer dead `commands` map deleted, `isLoginScreen` removed, duplicated wget condition fixed, `date` dynamic.
+11. **[Q-5, Q-7, Q-8]** done — `expandTerminal`/`collapseTerminal` helpers, rack-decoration CSS classes, `section.hidden = false`.
+12. **[Q-1, Q-9]** done — `terminal.js` split into `terminal-commands.js` / `terminal-vim.js` / `terminal-achievements.js` (1353 → 652 lines), `aria-label` on the hidden input.
+15. **[vim `dd`]** done — dead Shift+D key check fixed, last-line deletion guard fixed (buffer keeps one empty line, so insert mode can't index a missing line).
+16. **[tests]** done — 44-test Playwright E2E regression suite (`tests/e2e/`), green.
+
+**Remaining (unphased Low items):**
+5. **[L-1]** Verify (or add at Caddy edge) `Strict-Transport-Security` — folded into Phase 03 verification notes (edge-only, never in the container).
+7. **[L-4]** Fix README quadlet section (add `homepage.container` or remove reference); remove the `./src` volume-mount instruction.
+8. **[L-5]** Decide real/fake for the displayed SSH key; fingerprint-only if real, mark fictitious if not.
+9. **[L-8]** Add a `build.sh` verification test + a "run build" step to the Gitea workflow before image build.
+13. **[L-3]** (optional) Remove inline `style="..."` attributes from `index.html`, then drop `style-src 'unsafe-inline'`.
+14. **[L-7]** (optional) Pin CI actions to SHAs; consider image signing.
+
+## Verification Commands (after remediation)
+
+```sh
+# Rebuild and check dist integrity
+./build.sh && grep -Eno '(href|src)="[a-z]+\.(css|js|jpeg|ico|svg)"' dist/index.html  # expect: no unhashed refs
+
+# Header checks on the running container
+curl -sI http://localhost:8080/            | grep -Ei 'content-security|x-frame|x-content-type|referrer'
+curl -sI http://localhost:8080/index.html  | grep -Ei 'content-security|x-frame|x-content-type|referrer'  # must match after M-4
+curl -sI http://localhost:8080/style.*.css | grep -Ei 'x-content-type'
+
+# Live edge (run from a network that can reach it)
+curl -sI https://reeseapps.com/ | grep -Ei 'strict-transport|x-content-type|content-security'
+
+# Secret re-scan
+git grep -iEn 'password|secret|api[_-]?key|BEGIN (RSA|OPENSSH|PRIVATE)' -- ':!dist'
+
+# M-1 regression: type payload, Enter, Up-arrow in the terminal → no script/DOM injection
+```