q792602257andClaude Opus 4.6
8381b3068a
Tests / Test failed: 4, passed: 744
Replace hand-written f-string + html.escape() rendering in the Host Agent
local console with a module-level Jinja2 Environment configured with
select_autoescape(["html","xml"]). XSS safety now holds by mechanism
rather than per-call discipline — every operator-controlled field
(device name, connection_info, task summary, etc.) is escaped by the
engine uniformly.
Eight templates under host_agent/web/templates/ replace the former
_chrome(), _CSS, escape(), and per-page _xxx_body() helpers: base.html
(header/nav/CSS + {% block body %}), login, dashboard (with the polling
<script> preserved byte-identically inside {% raw %}), devices, account,
history, tasks_list, and task_detail. The task-list and task-detail
templates — added by the just-landed task-execution-progress-visibility
change — were also migrated here rather than left in f-string form,
since this change removes the shared helpers they depended on.
URLs, auth/session/CSRF semantics, redirects, and /api/status JSON are
unchanged. 15 new template tests cover render-smoke, XSS probing, script
byte-identity, and no-autoescape-bypass guards. Tasks 8.1-8.6 (manual
browser verification) remain.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
72 lines
10 KiB
Markdown
72 lines
10 KiB
Markdown
## 1. Dependency and lockfile
|
|
|
|
- [x] 1.1 Add `jinja2>=3.1` to the `dependencies` list in `apps/device-host-agent/pyproject.toml`
|
|
- [x] 1.2 Run `uv lock` (or `uv lock --package device-host-agent`) and confirm `jinja2` plus its sole transitive runtime dependency `markupsafe` resolve; commit the updated `uv.lock`
|
|
- [x] 1.3 Run `uv sync --locked --all-packages` and confirm no package fails to install
|
|
|
|
## 2. Capture pre-migration reference
|
|
|
|
- [x] 2.1 From a clean working copy of the current `host_agent/web/app.py`, render each of the five pages (login, dashboard, devices, account, history) in-process with a representative context (mock `SessionState`, mock `HostIdentityState`, two sample devices, one history entry) and save the HTML to `apps/device-host-agent/tests/host_agent/web/__baseline__/` as `<page>.html` (one file per page). The dashboard reference must include the rendered inline `<script>` block. These files become the regression oracle for tasks 6.3 and 6.4
|
|
- [x] 2.2 Note the exact bytes of the dashboard inline `<script>` block (from `function render(data) {` through the closing `})();`) into a separate `dashboard_script.txt` reference under the same baseline directory, for the byte-identical assertion in 6.4
|
|
|
|
## 3. Templates
|
|
|
|
- [x] 3.1 Create `apps/device-host-agent/host_agent/web/templates/base.html` per design D4: `<!doctype html>`, `<head>` with `<meta charset="utf-8">`, `<title>{{ title }}</title>`, a `<style>` block holding the current `_CSS` content, a `<header>` containing `<strong>Host Agent Console</strong>` and a `{% block nav %}` that renders the same nav (Status / Devices / Account / History / Logout form) when `session` is truthy and empty otherwise, and a `<main>` containing `{% block body %}{% endblock %}`
|
|
- [x] 3.2 Create `templates/login.html` extending `base.html`, overriding `{% block nav %}` to empty and `{% block body %}` with the current `_login_page` body markup, including the "no local account exists" branch (use `{% if not account %}`) and the optional error paragraph (`{% if error %}`)
|
|
- [x] 3.3 Create `templates/dashboard.html` extending `base.html`, porting the current `_dashboard_body` body markup (enrollment/heartbeat/policy/current-assignment sections, devices table with a `{% for device in devices %}` loop) and the inline `<script>` block wrapped in `{% raw %}...{% endraw %}` so JS braces are not interpreted by Jinja2; the script body must be byte-identical to the reference captured in 2.2
|
|
- [x] 3.4 Create `templates/devices.html` extending `base.html`, porting the current `_devices_body` markup including the device list table (loop with `{% for device in devices %}`), the optional error paragraph, and the add/edit form (use `{% if edit_record %}` to switch the form heading and pre-fill values)
|
|
- [x] 3.5 Create `templates/account.html` extending `base.html`, porting the current `_account_body` markup including the optional message/error paragraphs and the change-password form
|
|
- [x] 3.6 Create `templates/history.html` extending `base.html`, porting the current `_history_body` markup including the history table with a `{% for entry in entries %}` loop
|
|
|
|
## 4. Engine and route handlers
|
|
|
|
- [x] 4.1 In `host_agent/web/app.py`, construct a module-level `_ENV = jinja2.Environment(loader=jinja2.FileSystemLoader(Path(__file__).parent / "templates"), autoescape=jinja2.select_autoescape(["html", "xml"]))`; import `jinja2` and `pathlib.Path` at module top
|
|
- [x] 4.2 Rewrite the `/login` GET handler to render `login.html` via `_ENV.get_template("login.html").render(account=account)` and return `HTMLResponse(...)`; keep the current `asyncio.to_thread(local_account_store.load)` call shape
|
|
- [x] 4.3 Rewrite the `/` dashboard GET handler to render `dashboard.html` with `identity`, `snapshot`, `devices`, `config`, `session` in the context; ensure the inline `<script>` survives `{% raw %}` migration byte-identically (visually diff against the 2.2 reference)
|
|
- [x] 4.4 Rewrite the `/devices` GET handler to render `devices.html` with `devices`, `csrf_token`, `edit_record`, `error=None`; keep the existing `config_store.list`/`config_store.get` calls
|
|
- [x] 4.5 Rewrite the `/devices/save` POST handler's error-path branch to render `devices.html` (same context as 4.4 with `error=<message>`); keep the success-path `RedirectResponse(url="/devices", status_code=303)` unchanged
|
|
- [x] 4.6 Rewrite the `/account` GET and POST handlers to render `account.html` with `csrf_token`, `message`, `error` in the context; preserve the password-change success/error branches
|
|
- [x] 4.7 Rewrite the `/history` GET handler to render `history.html` with `entries` in the context
|
|
- [x] 4.8 Confirm `/api/status` (JSON) and all POST handlers that issue `RedirectResponse` (`/login`, `/logout`, `/devices/save` success, `/devices/remove`) are unchanged in behavior — only the GET and error-render paths swap from f-string to template render
|
|
|
|
## 5. Cleanup of dead code
|
|
|
|
- [x] 5.1 Remove the `escape()` helper from `host_agent/web/app.py`
|
|
- [x] 5.2 Remove `_chrome()` from `host_agent/web/app.py`
|
|
- [x] 5.3 Remove the `_CSS` constant from `host_agent/web/app.py`
|
|
- [x] 5.4 Remove the `_login_page`, `_dashboard_body`, `_devices_body`, `_account_body`, `_history_body` functions from `host_agent/web/app.py`
|
|
- [x] 5.5 Remove the now-unused `from html import escape as _escape` import
|
|
- [x] 5.6 Grep `apps/device-host-agent/` for any remaining `escape(`, `_chrome(`, `_CSS`, `_dashboard_body`, `_devices_body`, `_account_body`, `_history_body`, `_login_page` references and remove any stragglers; only `_ENV` / `get_template` / `render` references should remain in `host_agent/web/app.py`
|
|
|
|
## 6. Tests
|
|
|
|
- [x] 6.1 Create `apps/device-host-agent/tests/host_agent/web/__init__.py` if not present (empty)
|
|
- [x] 6.2 Create `apps/device-host-agent/tests/host_agent/web/conftest.py` with fixtures exposing the module-level `_ENV` from `host_agent.web.app` and helper factories for representative contexts (one valid session, one no-account state, two sample devices with one containing the XSS probe, one history entry, one edit_record)
|
|
- [x] 6.3 Create `apps/device-host-agent/tests/host_agent/web/test_templates.py` with one render-smoke test per template (login, dashboard, devices, account, history) asserting the rendered HTML contains a stable selector from each section (e.g. `id="last-heartbeat"` for dashboard, the `<form method="post" action="/devices/save">` for devices) and that rendering does not raise for the representative context
|
|
- [x] 6.4 Add an XSS-probe test that, for each of `dashboard.html`, `devices.html`, `history.html`, renders the template with a context in which every operator-influenced field is set to `<script>alert(1)</script>` (device name, device driver_type, connection_info JSON string, history summary) and asserts the substring `<script>alert(1)</script>` does not appear in the output while `<script>alert(1)</script>` does
|
|
- [x] 6.5 Add a byte-identity test that loads the dashboard template, renders it with a representative context, and asserts the inline `<script>` block body extracted from the output equals the contents of `tests/host_agent/web/__baseline__/dashboard_script.txt` captured in 2.2 (normalize trailing whitespace)
|
|
- [x] 6.6 Add a no-autoescape-bypass test that greps the templates directory for the literal substrings `| safe`, `{% autoescape false %}`, and `{% endautoescape %}`, asserting zero matches across all current templates (guards against a future change silently opting out)
|
|
- [x] 6.7 Add a test asserting `_ENV.autoescape` is configured for `.html` templates (e.g. by rendering a template whose name ends in `.html` with a probe value and confirming the output is escaped — concrete and future-proof against Environment construction changes)
|
|
|
|
## 7. Local validation
|
|
|
|
- [x] 7.1 Run `uv run --package device-host-agent pytest` and confirm the full package suite (existing tests plus the new section-6 tests) passes
|
|
- [x] 7.2 Run the root non-integration suite (`uv run --all-packages pytest -m "not integration"`) and confirm no regressions versus the pre-change baseline
|
|
- [x] 7.3 Run `ruff check --fix` and `ruff format` over `apps/device-host-agent/host_agent/web/`, `apps/device-host-agent/tests/host_agent/web/`, and `apps/device-host-agent/pyproject.toml`; resolve any findings
|
|
- [x] 7.4 Run `python -m compileall apps/device-host-agent/host_agent/web apps/device-host-agent/tests/host_agent/web` and confirm no syntax errors
|
|
- [x] 7.5 Run `git diff --check` to catch whitespace errors before commit
|
|
- [x] 7.6 Run `openspec validate host-agent-console-jinja2-templates --strict` and confirm it passes
|
|
|
|
## 8. Manual browser verification
|
|
|
|
- [ ] 8.1 Start a Host Agent locally (`uv run --package device-host-agent device-host-agent` or the existing run command documented in `docs/CLOUD_DEPLOYMENT.md`); open `http://127.0.0.1:8765/login` in a browser and log in with the existing local account
|
|
- [ ] 8.2 On the status dashboard, confirm the auto-refresh polling still works (watch the "Last heartbeat" line update at the 5-second cadence) and the inline `<script>` executes without console errors
|
|
- [ ] 8.3 Visit `/devices`, add a device whose name is `<script>alert(1)</script>` (and a valid driver type/connection info); confirm the device appears in the list with the script visible as text rather than executing, then remove it
|
|
- [ ] 8.4 Visit `/account`, change the password (current → new → confirm) end-to-end, log out, and log back in with the new password
|
|
- [ ] 8.5 Visit `/history` and confirm recent entries render
|
|
- [ ] 8.6 Compare each page's visual layout to the pre-change version and confirm no styling regression (the `_CSS` content must be byte-identical inside `base.html`'s `<style>` block)
|
|
|
|
## 9. Coordination follow-up (not blocking archive of this change)
|
|
|
|
- [x] 9.1 After this change lands on `master`, open a follow-up note (issue or PR comment) on `openspec/changes/task-execution-progress-visibility` advising that its design.md D3 ("reuse `api/console.py`'s query patterns and f-string + `html.escape()` convention") is superseded: the task list/detail/timeline pages planned there SHOULD be written directly against the Jinja2 templates introduced here. This task does not block archive of either change; it only prevents the next author from re-growing the f-string pattern.
|