**Phase 01 — final verification pass (all criteria green)** - Verified M-1 fix in `src/terminal.js`: ArrowUp / ArrowDown(`>0`) / ArrowDown-else now route through `updateDisplay()` (`textContent`); no `innerHTML` sink touches `cmdLine`/`commandHistory`. - `grep -n innerHTML src/terminal.js` → 12 hits, all allowed (static prompt literals, `clear`/prank wipes, `outLine.innerHTML = output` gated on the static canned `neofetch` string). **C1 ✓** - XSS regression test at `tests/e2e/02-terminal.spec.js:156` present + passing; re-proved it is a real guard: re-injected the old `innerHTML` recall into `dist/terminal.*.js` → test failed (`#xss-marker` received 1); rebuild → byte-identical to `src/`. **C2 ✓** - `./build.sh` → `dist/` OK; `npm test` → **45 passed (0 failed)**; `bash .agents/validate.sh` → **exit 0** (suite + repo-hygiene gates). **C3 ✓** - Branch coverage of modified code: all 3 recall arms asserted (ArrowUp ×2 specs, ArrowDown `>0` in the existing history spec, ArrowDown-else → empty prompt in the new spec) → >90% gate met; no JS coverage/lint toolchain exists in the locked stack, so lint = `node --check src/*.js tests/e2e/*.js playwright.config.js` + `bash -n build.sh .agents/validate.sh` — clean. - C4 ✓ terminal/vim/achievements/menu specs all green; only the three recall branches changed. - Docs (step 6, full README/AGENTS review): README — run section now states the image already contains the built `dist/` (mount `dist/`, never `src/`); new “Serving & configuration” section for `nginx.conf`, incl. **empirically measured** (built image on 127.0.0.1:8124) that `/`, `/index.html` and hashed assets currently return *only* cache headers — the `add_header` non-merge gap = finding M-4/Phase 03; TLS/HSTS-at-edge rule; Quadlet section fixed (unit is not in this repo; Quadlet generates `homepage.service`, so `… start homepage.service`, and `systemctl enable` is a no-op for transient Quadlet units → `[Install]` + `daemon-reload` + linger); Development gained the Node 20+ floor, `npx playwright install chromium`, and `.agents/validate.sh`. AGENTS.md — added the validate.sh gate, flagged `./scripts/check-headers.sh` as not yet in the repo (Phase 03), and scoped the M-1 rule with the allowed-`innerHTML` list. - Notable: `.agents/PLAN.md` still says “44 tests” (3 places) and its §7 roadmap omits phases 04/05 — left untouched per the no-PLAN-edit rule; flag for the next PLAN review. No live-infra touched (temp podman image built/removed for the header measurement only). - Next pending phase: `02_simulate_fetch_commands`.
This commit is contained in:
@@ -0,0 +1,38 @@
|
||||
# Phase 01 — Fix terminal history-recall XSS (audit M-1)
|
||||
|
||||
**Story:** n/a
|
||||
**Context:** `.agents/PLAN.md` §4 (terminal modules), `.agents/remediation_plan.md` finding M-1
|
||||
|
||||
## Objective
|
||||
Eliminate the DOM-XSS sink in the fake terminal's command-history recall:
|
||||
ArrowUp/ArrowDown currently rebuild the prompt line with
|
||||
`lastLine.innerHTML = '<span class="terminal-prompt">$</span> ' + cmdLine + ' '`
|
||||
where `cmdLine` is raw user input from `commandHistory`. Replace it with
|
||||
textContent-based construction so terminal input can never be injected as
|
||||
HTML, and lock the fix with a Playwright regression test.
|
||||
|
||||
## Dependencies
|
||||
— (none)
|
||||
|
||||
## Tasks
|
||||
1. `01_fix_history_recall.md` — Rewrite the ArrowUp/ArrowDown recall to build the prompt line with `createElement`/`textContent` (mirror the existing `updateDisplay()` pattern).
|
||||
2. `02_xss_regression_test.md` — Add a Playwright test proving a markup payload typed into the terminal is recalled as literal text, not DOM.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration: the new regression test must cover (a) a payload that
|
||||
would create a DOM node if injected (`<b id="xss-marker">x</b>`), (b) a
|
||||
payload with an inline event handler (`<img src=x onerror=...`) asserting
|
||||
no script execution, and (c) a benign command (`whoami`) still recalled
|
||||
and executable.
|
||||
- Coverage: **>90%** on new/modified code (the two recall branches in
|
||||
`src/terminal.js`).
|
||||
- E2E: full Playwright suite (`npm test`) green — it already exercises
|
||||
history recall with benign commands and must not regress.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `grep -n "innerHTML" src/terminal.js` shows no occurrence combining
|
||||
`innerHTML` with `cmdLine`/history data (static prompt literals and the
|
||||
neofetch canned-output assignment are the only allowed `innerHTML` uses)
|
||||
- [ ] new XSS regression test present in `tests/e2e/02-terminal.spec.js` and passing
|
||||
- [ ] `./build.sh && npm test` fully green (44+ tests)
|
||||
- [ ] no behavior change in completed work (terminal, vim, achievements, menu)
|
||||
@@ -0,0 +1,49 @@
|
||||
# Task 01 — Rewrite history recall with the safe display path
|
||||
|
||||
**Phase:** `01_fix_history_xss` · **Story:** n/a
|
||||
|
||||
## Objective
|
||||
Remove the `innerHTML` sink from the terminal's ArrowUp/ArrowDown history
|
||||
recall by routing recall through the existing `updateDisplay(text)` function,
|
||||
which already builds the prompt line with `textContent`/`createElement`.
|
||||
|
||||
## Work
|
||||
1. `src/terminal.js` — in the terminal `keydown` handler, replace the three
|
||||
`lastLine.innerHTML = '<span class="terminal-prompt">$</span> ' + cmdLine + ' '`
|
||||
constructions with `updateDisplay(...)` calls:
|
||||
|
||||
- **ArrowUp branch** (`if (historyIndex < commandHistory.length - 1)`):
|
||||
delete the `innerHTML` line, the `newCursor` creation, and the
|
||||
`appendChild` — keep `historyIndex++` and `const cmdLine = ...`, then:
|
||||
```js
|
||||
updateDisplay(cmdLine);
|
||||
```
|
||||
- **ArrowDown branch, first case** (`if (historyIndex > 0)`): same
|
||||
replacement — `updateDisplay(cmdLine);`
|
||||
- **ArrowDown branch, else case** (`historyIndex = -1`): replace the
|
||||
`innerHTML`/cursor block with:
|
||||
```js
|
||||
updateDisplay('');
|
||||
```
|
||||
|
||||
`updateDisplay` already: removes any stale `.terminal-cursor`, clears the
|
||||
line via `textContent`, appends a `.terminal-prompt` span (respecting
|
||||
`isRoot` → `#` vs `$`), the text, and a fresh cursor (red when root).
|
||||
|
||||
Note the two intentional, behavior-preserving improvements this brings:
|
||||
(a) recalled prompts match the live typing state (cursor directly after
|
||||
the text, no trailing space); (b) after `sudo su -`, recall shows `#`
|
||||
instead of a stale `$`. Both are consistency fixes, not regressions.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration: no new unit code — the existing E2E test
|
||||
`02-terminal.spec.js › history: ArrowUp/ArrowDown recall previous commands`
|
||||
exercises recall and execution of recalled commands and must stay green.
|
||||
- Coverage: **>90%** on the modified branches (covered by that E2E test plus
|
||||
the task-02 regression test).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `grep -n "innerHTML" src/terminal.js` — no remaining occurrence that
|
||||
involves `cmdLine` or history data
|
||||
- [ ] `./build.sh && npm test` fully green
|
||||
- [ ] no behavior change in completed work (terminal, vim, achievements, menu)
|
||||
@@ -0,0 +1,65 @@
|
||||
# Task 02 — Add the XSS regression test
|
||||
|
||||
**Phase:** `01_fix_history_xss` · **Story:** n/a
|
||||
|
||||
## Objective
|
||||
Add a Playwright E2E test that proves a markup payload typed into the
|
||||
terminal is recalled (ArrowUp) as **literal text**, never as DOM, and never
|
||||
executes. This test fails on the pre-fix (innerHTML) code and passes on the
|
||||
fixed code, so it guards against regressions/reverts.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/02-terminal.spec.js` — add the following test inside the
|
||||
`test.describe('Fake terminal — state and special commands', ...)` block
|
||||
(it reuses `openTerminal`, `typeCommand`, `currentPromptLine` from
|
||||
`./helpers`):
|
||||
|
||||
```js
|
||||
test('history recall renders markup payloads as literal text (no DOM injection)', async ({ page }) => {
|
||||
await openTerminal(page);
|
||||
|
||||
// 1) A payload that would create a DOM node if innerHTML-injected.
|
||||
await typeCommand(page, '<b id="xss-marker">pwned</b>');
|
||||
await page.keyboard.press('ArrowUp');
|
||||
await expect(page.locator('#xss-marker')).toHaveCount(0);
|
||||
await expect(currentPromptLine(page)).toContainText('<b id="xss-marker">pwned</b>');
|
||||
|
||||
// 2) An event-handler payload must not execute. `data:,` is an empty
|
||||
// data URI so the <img> fails to decode deterministically (no
|
||||
// network dependency) and onerror would fire if injected as HTML.
|
||||
await page.evaluate(() => { window.__xssFired = false; });
|
||||
await typeCommand(page, '<img src="data:," onerror="window.__xssFired=true">');
|
||||
await page.keyboard.press('ArrowUp');
|
||||
await page.waitForTimeout(200); // allow onerror to fire if vulnerable
|
||||
expect(await page.evaluate(() => window.__xssFired)).toBe(false);
|
||||
await expect(page.locator('img[onerror]')).toHaveCount(0);
|
||||
await expect(currentPromptLine(page)).toContainText('<img src="data:," onerror="window.__xssFired=true">');
|
||||
|
||||
// 3) Benign recall still works and is executable.
|
||||
await page.keyboard.press('ArrowDown');
|
||||
await page.keyboard.press('ArrowDown');
|
||||
await page.keyboard.press('Enter');
|
||||
await expect(page.locator('.terminal-content > div').nth(-2)).toHaveText('optional');
|
||||
});
|
||||
```
|
||||
|
||||
Notes for the executor:
|
||||
- Step 3 walks back down through history (`<img…>`, then `<b…>`, then the
|
||||
prior real command) and executes it; adjust the final assertion to the
|
||||
benign command that is actually on top of the history at that point if
|
||||
the exact stack differs when you run it. The point is that recall +
|
||||
execute of a benign command still works.
|
||||
- Do **not** add an `onerror`-based assertion that depends on a real
|
||||
network fetch; keep it self-contained with `data:,`.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration: this test **is** the regression coverage for the M-1 fix.
|
||||
It must: fail on innerHTML-based recall, pass on textContent-based recall.
|
||||
- Coverage: **>90%** on the modified recall branches (both payload classes +
|
||||
benign path covered).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] new test present in `tests/e2e/02-terminal.spec.js`
|
||||
- [ ] `./build.sh && npm test` fully green (45+ tests)
|
||||
- [ ] (sanity) temporarily reverting task 01's fix makes this test fail —
|
||||
confirms it is a genuine regression guard, then re-apply the fix
|
||||
Reference in New Issue
Block a user