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>
39 lines
6.6 KiB
Markdown
39 lines
6.6 KiB
Markdown
## Why
|
|
|
|
The Host Agent local console (`apps/device-host-agent/host_agent/web/app.py`) currently renders all of its pages with f-strings plus a hand-written `escape()` helper called on every interpolated value. Five pages are already in place (login, status dashboard, devices, account, history) — including a dashboard page with an inline `<script>` polling block that has to double-brace every JS `{`/`}` to survive f-string parsing. The pending `task-execution-progress-visibility` change will add three more pages (task list, task detail, timeline) that render externally-influenced text (task summaries, scene/element descriptions) inside loops, exactly the shape f-string rendering handles worst.
|
|
|
|
The original `host-agent-local-console` design decision to skip a templating engine was made when the assumed page count was four and the rendered fields were mostly internal enums. Both assumptions are now stale: every operator-controlled field (`name`, `driver_type`, `connection_info`, upcoming task summaries) is one missed `escape()` away from an XSS sink, and the per-page f-string HTML has grown verbose enough that the `_chrome()` shell, the per-page body functions, and the inline `_CSS` constant are reproducing a templating engine by hand. Jinja2 with `autoescape=select_autoescape(["html"])` removes the "did I remember to escape this one?" class of bug by mechanism rather than by discipline, and lets loops/conditionals/inheritance be expressed natively.
|
|
|
|
## What Changes
|
|
|
|
- Add `jinja2` as an explicit direct dependency of `device-host-agent` in `apps/device-host-agent/pyproject.toml` (single pure-Python wheel, no C extension).
|
|
- Introduce a module-level Jinja2 `Environment` in `host_agent/web/app.py` configured with `autoescape=select_autoescape(["html"])` and a `FileSystemLoader` rooted at a new `host_agent/web/templates/` directory.
|
|
- Replace the hand-written `_chrome()` shell, the `_CSS` constant, and the five `_login_page`/`_dashboard_body`/`_devices_body`/`_account_body`/`_history_body` f-string builders with a `base.html` template (header/nav/CSS, `{% block body %}`) plus one template file per page.
|
|
- Migrate the inline dashboard `<script>` block into its template using `{% raw %}...{% endraw %}` so JS braces are no longer f-string-escaped; behavior of the 5-second `/api/status` polling is preserved exactly.
|
|
- Render every route handler through `TemplateResponse` instead of `HTMLResponse(_chrome(...))`. Route paths, auth/session/CSRF semantics, form fields, redirects, and the `/api/status` JSON endpoint are unchanged.
|
|
- Remove the module-level `escape()` helper and `_chrome()` from `host_agent/web/app.py` (Jinja2's autoescape subsumes both).
|
|
- Add a `tests/host_agent/web/` test module covering: each template renders without errors for representative contexts; an XSS probe (a model field set to `<script>alert(1)</script>`) appears HTML-escaped in the rendered output; the dashboard polling script body survives the `{% raw %}` migration byte-for-byte against the current page.
|
|
|
|
Non-Goals:
|
|
|
|
- No new pages, no new routes, no new visual design — this is a pure rendering-engine swap.
|
|
- No change to the `console/` SPA (Runtime API UI) or `cloud-console/` SPA, both of which keep their own Vue3 + Vite toolchains.
|
|
- No change to `host_agent/web/auth.py` (session/CSRF) or to the `host-agent-local-console` capability's auth/CSRF/session/config requirements.
|
|
- No introduction of a static-asset pipeline (no `starlette.staticfiles`, no JS/CSS bundling) — the existing inline `<style>` becomes a `{% block styles %}` in `base.html`; future work can extract it later.
|
|
|
|
## Capabilities
|
|
|
|
### New Capabilities
|
|
- `host-agent-console-template-rendering`: Host Agent local console pages SHALL be rendered through a template engine with automatic HTML escaping enabled, so that every operator- or externally-influenced field interpolated into a page is escaped by mechanism rather than by per-call discipline. Decouples the autoescape security property from any single page's author and applies uniformly to every current and future page rendered by `host_agent/web/app.py` (login, dashboard, devices, account, history, and any later additions such as the task pages planned by `task-execution-progress-visibility`).
|
|
|
|
### Modified Capabilities
|
|
(none — the existing `host-agent-local-console` capability is not yet archived into `openspec/specs/`; rather than write a MODIFIED delta against an unarchived pending spec, this change captures the autoescape requirement as a new sibling capability. The `host-agent-local-console` change's design.md decision titled "Reuse FastAPI + Starlette's `HTMLResponse`, not a new micro-framework, not Jinja2" is hereby superseded for the rendering-mechanism question only; that change's auth/session/CSRF/config/lifecycle decisions remain in force. The supersession is recorded as an Open Question in this change's design.md to be reconciled when `host-agent-local-console` is archived.)
|
|
|
|
## Impact
|
|
|
|
- Affected code: `apps/device-host-agent/pyproject.toml` (new `jinja2` direct dependency), `apps/device-host-agent/host_agent/web/app.py` (rewritten to use Jinja2 `TemplateResponse`), new `apps/device-host-agent/host_agent/web/templates/` directory (`base.html`, `login.html`, `dashboard.html`, `devices.html`, `account.html`, `history.html`), new `apps/device-host-agent/tests/host_agent/web/` test module.
|
|
- Dependency graph: `jinja2` enters the resolved `uv.lock` as a direct dependency of `device-host-agent`. It is not currently a transitive dependency of fastapi/starlette (their `Jinja2Templates` is an optional extra), so this is a genuine new dependency, not surfacing an already-present one.
|
|
- Not affected: `cloud.*`, `apps/cloud-api`, `cloud-console/`, `console/`, `host-agent-protocol` outbound behavior, `host_agent/web/auth.py`, `HostAgentConfig`, all store modules, all lifecycle/startup wiring.
|
|
- Coordination point: `openspec/changes/task-execution-progress-visibility` (in flight) currently commits in its design.md D3 to "reuse `api/console.py`'s query patterns and f-string + `html.escape()` convention" for its new Host-Agent-side task pages. After this change lands, those new pages SHOULD be written directly in Jinja2 instead; the two changes do not conflict at the code level (different files inside `host_agent/web/`), but D3's wording will need a follow-up edit when that change is next updated. This is recorded as an Open Question in design.md.
|
|
- Operational impact: none at runtime from an operator's perspective — same URLs, same login, same pages, same polling; first render of each template pays a one-time compile cost (sub-millisecond per template, cached on the module-level `Environment`).
|