` formatting. The dynamic tag key is escaped with `escapeHtml()` before interpolation.
+
+## Suggested Next Steps
+
+1. Run full manual testing of admin pages (node-tags, members) with XSS payloads to verify fixes in a browser environment.
+2. Test `WEB_TRUSTED_PROXY_HOSTS` with a real reverse proxy (Traefik/Nginx) to verify proxy header trust restriction works as expected.
+3. Push commits and create a pull request for merge into `main`.
diff --git a/.plans/2026/03/09/01-security-fixes/tasks.yaml b/.plans/2026/03/09/01-security-fixes/tasks.yaml
new file mode 100644
index 0000000..014b0e7
--- /dev/null
+++ b/.plans/2026/03/09/01-security-fixes/tasks.yaml
@@ -0,0 +1,401 @@
+# Task list generated from PRD: .plans/2026/03/09/01-security-fixes/prd.md
+# Generated by: /jp-task-list
+
+tasks:
+ - id: "TASK-001"
+ title: "Remove legacy HTML dashboard endpoint"
+ description: |
+ Remove the `dashboard()` route handler from `src/meshcore_hub/api/routes/dashboard.py` (lines ~367-536).
+ This handler renders a standalone HTML page using f-string HTML with unescaped database content (stored XSS)
+ and has no authentication. The JSON sub-routes (`/stats`, `/activity`, `/message-activity`, `/node-count`)
+ must remain intact and unchanged.
+
+ Specifically:
+ 1. Delete the `dashboard()` async function and its `@router.get("")` decorator (the handler that returns HTMLResponse).
+ 2. Remove the `HTMLResponse` import from `fastapi.responses` if it is no longer used by any remaining route.
+ 3. Verify that `GET /api/v1/dashboard/stats`, `/activity`, `/message-activity`, and `/node-count` still function.
+ requirements:
+ - "REQ-001"
+ - "REQ-006"
+ dependencies: []
+ suggested_role: "python"
+ acceptance_criteria:
+ - "The `dashboard()` route handler is removed from `api/routes/dashboard.py`"
+ - "`HTMLResponse` import is removed if no longer used"
+ - "`GET /api/v1/dashboard/` returns 404 or 405"
+ - "`GET /api/v1/dashboard/stats` returns valid JSON with authentication"
+ - "`GET /api/v1/dashboard/activity` returns valid JSON with authentication"
+ - "`GET /api/v1/dashboard/message-activity` returns valid JSON with authentication"
+ - "`GET /api/v1/dashboard/node-count` returns valid JSON with authentication"
+ estimated_complexity: "small"
+ files_affected:
+ - "src/meshcore_hub/api/routes/dashboard.py"
+
+ - id: "TASK-002"
+ title: "Replace API key comparisons with constant-time comparison"
+ description: |
+ Replace all Python `==` comparisons of API keys and credentials with `hmac.compare_digest()` to prevent
+ timing side-channel attacks.
+
+ In `src/meshcore_hub/api/auth.py`:
+ 1. Add `import hmac` at the top of the file.
+ 2. Line ~82 in `require_read`: replace `if token == read_key or token == admin_key:` with
+ `if hmac.compare_digest(token, read_key) or hmac.compare_digest(token, admin_key):`.
+ 3. Line ~127 in `require_admin`: replace `if token == admin_key:` with
+ `if hmac.compare_digest(token, admin_key):`.
+
+ In `src/meshcore_hub/api/metrics.py`:
+ 1. Add `import hmac` at the top of the file.
+ 2. Line ~57: replace `return username == "metrics" and password == read_key` with
+ `return hmac.compare_digest(username, "metrics") and hmac.compare_digest(password, read_key)`.
+
+ Note: `hmac.compare_digest()` requires both arguments to be strings (or both bytes). The existing code
+ already works with strings, so no type conversion is needed.
+ requirements:
+ - "REQ-002"
+ - "REQ-007"
+ dependencies: []
+ suggested_role: "python"
+ acceptance_criteria:
+ - "All API key comparisons in `api/auth.py` use `hmac.compare_digest()`"
+ - "All credential comparisons in `api/metrics.py` use `hmac.compare_digest()`"
+ - "`hmac` is imported in both files"
+ - "Valid API keys are accepted and invalid keys are rejected (no behavior change)"
+ estimated_complexity: "small"
+ files_affected:
+ - "src/meshcore_hub/api/auth.py"
+ - "src/meshcore_hub/api/metrics.py"
+
+ - id: "TASK-003"
+ title: "Add WEB_TRUSTED_PROXY_HOSTS configuration setting"
+ description: |
+ Add a `web_trusted_proxy_hosts` field to the web settings in `src/meshcore_hub/common/config.py`.
+
+ 1. In the `WebSettings` class (or the relevant settings class containing web config), add:
+ ```python
+ web_trusted_proxy_hosts: str = Field(default="*", description="Comma-separated list of trusted proxy hosts or '*' for all")
+ ```
+ 2. The field should accept a string value. The `ProxyHeadersMiddleware` in uvicorn accepts either `"*"` or a list of strings.
+ If the value is `"*"`, pass it directly. Otherwise, split on commas and strip whitespace to produce a list.
+
+ This task only adds the configuration field. The middleware integration and startup warning are in TASK-004.
+ requirements:
+ - "REQ-003"
+ - "REQ-006"
+ dependencies: []
+ suggested_role: "python"
+ acceptance_criteria:
+ - "A `web_trusted_proxy_hosts` setting exists in the configuration with default value `*`"
+ - "The setting can be configured via the `WEB_TRUSTED_PROXY_HOSTS` environment variable"
+ - "The setting accepts `*` or a comma-separated list of hostnames/IPs"
+ estimated_complexity: "small"
+ files_affected:
+ - "src/meshcore_hub/common/config.py"
+
+ - id: "TASK-004"
+ title: "Integrate trusted proxy hosts into web app middleware and add startup warning"
+ description: |
+ Update `src/meshcore_hub/web/app.py` to use the new `WEB_TRUSTED_PROXY_HOSTS` setting and emit a
+ startup warning when using the insecure default.
+
+ 1. Find the `ProxyHeadersMiddleware` addition (line ~239):
+ ```python
+ app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*")
+ ```
+ Replace the hardcoded `"*"` with the configured value. If the config value is `"*"`, pass `"*"`.
+ Otherwise, split the comma-separated string into a list of strings.
+
+ 2. Add a startup warning (in the app factory or lifespan) when `WEB_ADMIN_ENABLED=true` and
+ `WEB_TRUSTED_PROXY_HOSTS` is `"*"`:
+ ```python
+ import logging
+ logger = logging.getLogger(__name__)
+ if settings.web_admin_enabled and settings.web_trusted_proxy_hosts == "*":
+ logger.warning(
+ "WEB_ADMIN_ENABLED is true but WEB_TRUSTED_PROXY_HOSTS is '*' (trust all). "
+ "Consider restricting to your reverse proxy IP for production deployments."
+ )
+ ```
+
+ 3. Verify that the `_is_authenticated_proxy_request` function still accepts `X-Forwarded-User`,
+ `X-Auth-Request-User`, and `Authorization: Basic` headers — do not modify that function.
+ requirements:
+ - "REQ-003"
+ - "REQ-006"
+ - "REQ-007"
+ dependencies:
+ - "TASK-003"
+ suggested_role: "python"
+ acceptance_criteria:
+ - "`ProxyHeadersMiddleware` uses the configured `trusted_hosts` value instead of hardcoded `*`"
+ - "A warning is logged at startup when admin is enabled and trusted hosts is `*`"
+ - "The warning recommends restricting trusted hosts to the proxy IP"
+ - "`_is_authenticated_proxy_request` still accepts all three header types"
+ - "Setting `WEB_TRUSTED_PROXY_HOSTS` to a specific IP restricts proxy header trust"
+ estimated_complexity: "medium"
+ files_affected:
+ - "src/meshcore_hub/web/app.py"
+
+ - id: "TASK-005"
+ title: "Escape config JSON in template script block to prevent XSS breakout"
+ description: |
+ Prevent XSS via `` breakout in the config JSON template injection in `src/meshcore_hub/web/app.py`.
+
+ In the `_build_config_json` function (or wherever `config_json` is prepared for the template, around
+ line 183), after calling `json.dumps(config)`, escape `` from breaking out of the
+ `` does not execute JavaScript"
+ - "The SPA application correctly parses the escaped config JSON"
+ - "Normal config values without special characters render unchanged"
+ estimated_complexity: "small"
+ files_affected:
+ - "src/meshcore_hub/web/app.py"
+
+ - id: "TASK-006"
+ title: "Fix stored XSS in admin node-tags page"
+ description: |
+ Sanitize API-sourced data in `src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js` to prevent
+ stored XSS.
+
+ Three locations need fixing:
+
+ 1. **Line ~243** — `unsafeHTML()` with nodeName in copy-all confirmation:
+ ```javascript
+ ${unsafeHTML(t('common.copy_all_entity_description', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: nodeName }))}
+ ```
+ Replace `unsafeHTML()` with safe rendering. Either escape `nodeName` with `escapeHtml()` before
+ passing to `t()`, or use `textContent`-based rendering.
+
+ 2. **Line ~272** — `unsafeHTML()` with nodeName in delete-all confirmation:
+ ```javascript
+ ${unsafeHTML(t('common.delete_all_entity_confirm', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: nodeName }))}
+ ```
+ Same fix as above.
+
+ 3. **Line ~454** — `innerHTML` with tag key in delete confirmation:
+ ```javascript
+ container.querySelector('#delete_tag_confirm_message').innerHTML = confirmMsg;
+ ```
+ where `confirmMsg` is built with `activeTagKey` interpolated into an HTML span. Replace `innerHTML`
+ with `textContent`, or escape `activeTagKey` with `escapeHtml()` before interpolation.
+
+ Import `escapeHtml` from `../components.js` if not already imported. The function escapes `<`, `>`,
+ `&`, `"`, and `'` characters using DOM textContent.
+ requirements:
+ - "REQ-005"
+ - "REQ-006"
+ dependencies: []
+ suggested_role: "frontend"
+ acceptance_criteria:
+ - "Node names in node-tags.js are escaped before HTML rendering"
+ - "Tag keys in node-tags.js are escaped before HTML rendering"
+ - "All `unsafeHTML()` calls on API-sourced data are replaced with safe alternatives"
+ - "All `innerHTML` assignments of API-sourced data are replaced with safe alternatives"
+ - "A node name containing `
` renders as text"
+ - "Normal names without special characters display correctly"
+ estimated_complexity: "medium"
+ files_affected:
+ - "src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js"
+
+ - id: "TASK-007"
+ title: "Fix stored XSS in admin members page"
+ description: |
+ Sanitize API-sourced data in `src/meshcore_hub/web/static/js/spa/pages/admin/members.js` to prevent
+ stored XSS.
+
+ **Line ~309** — `innerHTML` with memberName in delete confirmation:
+ ```javascript
+ container.querySelector('#delete_confirm_message').innerHTML = confirmMsg;
+ ```
+ where `confirmMsg` is built from `t('common.delete_entity_confirm', { entity: ..., name: memberName })`.
+ `memberName` comes from `row.dataset.memberName` which is API-sourced data.
+
+ Fix by escaping `memberName` with `escapeHtml()` before passing to `t()`, or replace `innerHTML` with
+ `textContent`.
+
+ Import `escapeHtml` from `../components.js` if not already imported.
+ requirements:
+ - "REQ-005"
+ - "REQ-006"
+ dependencies: []
+ suggested_role: "frontend"
+ acceptance_criteria:
+ - "Member names in members.js are escaped before HTML rendering"
+ - "The `innerHTML` assignment of API-sourced data is replaced with a safe alternative"
+ - "A member name containing `` renders as text"
+ - "Normal member names display correctly"
+ estimated_complexity: "small"
+ files_affected:
+ - "src/meshcore_hub/web/static/js/spa/pages/admin/members.js"
+
+ - id: "TASK-008"
+ title: "Write tests for legacy dashboard endpoint removal"
+ description: |
+ Add or update tests in `tests/test_api/` to verify that the legacy HTML dashboard endpoint is removed
+ while JSON sub-routes remain functional.
+
+ Tests to add/update:
+ 1. `GET /api/v1/dashboard/` returns 404 or 405 (no longer serves HTML).
+ 2. `GET /api/v1/dashboard/stats` returns 200 with valid JSON when authenticated.
+ 3. `GET /api/v1/dashboard/activity` returns 200 with valid JSON when authenticated.
+ 4. `GET /api/v1/dashboard/message-activity` returns 200 with valid JSON when authenticated.
+ 5. `GET /api/v1/dashboard/node-count` returns 200 with valid JSON when authenticated.
+
+ Use the existing test fixtures and patterns from `tests/test_api/`. Check `tests/conftest.py` for
+ available fixtures (test client, db session, auth headers).
+ requirements:
+ - "REQ-001"
+ - "REQ-006"
+ dependencies:
+ - "TASK-001"
+ suggested_role: "python"
+ acceptance_criteria:
+ - "Test confirms `GET /api/v1/dashboard/` returns 404 or 405"
+ - "Tests confirm all four JSON sub-routes return valid JSON with authentication"
+ - "All tests pass"
+ estimated_complexity: "small"
+ files_affected:
+ - "tests/test_api/test_dashboard.py"
+
+ - id: "TASK-009"
+ title: "Write tests for constant-time API key comparison"
+ description: |
+ Add or update tests in `tests/test_api/` to verify that authentication still works correctly after
+ switching to `hmac.compare_digest()`.
+
+ Tests to add/update:
+ 1. Valid read key is accepted by read-protected endpoints.
+ 2. Valid admin key is accepted by admin-protected endpoints.
+ 3. Invalid keys are rejected with 401/403.
+ 4. Valid admin key also grants read access.
+ 5. Metrics endpoint accepts valid credentials and rejects invalid ones (if metrics auth is testable).
+
+ These tests verify no behavioral regression from the `==` to `hmac.compare_digest()` change.
+ Use existing test patterns and fixtures from `tests/test_api/`.
+ requirements:
+ - "REQ-002"
+ - "REQ-007"
+ dependencies:
+ - "TASK-002"
+ suggested_role: "python"
+ acceptance_criteria:
+ - "Tests confirm valid read key is accepted"
+ - "Tests confirm valid admin key is accepted"
+ - "Tests confirm invalid keys are rejected"
+ - "Tests confirm metrics auth works correctly"
+ - "All tests pass"
+ estimated_complexity: "small"
+ files_affected:
+ - "tests/test_api/test_auth.py"
+
+ - id: "TASK-010"
+ title: "Write tests for trusted proxy hosts configuration and startup warning"
+ description: |
+ Add tests to verify the `WEB_TRUSTED_PROXY_HOSTS` configuration setting and the startup warning.
+
+ Tests to add:
+ 1. Default value of `WEB_TRUSTED_PROXY_HOSTS` is `*`.
+ 2. Setting `WEB_TRUSTED_PROXY_HOSTS` to a specific IP is correctly parsed.
+ 3. Setting `WEB_TRUSTED_PROXY_HOSTS` to a comma-separated list is correctly parsed into a list.
+ 4. A warning is logged when `WEB_ADMIN_ENABLED=true` and `WEB_TRUSTED_PROXY_HOSTS` is `*`.
+ 5. No warning is logged when `WEB_TRUSTED_PROXY_HOSTS` is set to a specific value.
+
+ Place config tests in `tests/test_common/` and web app tests in `tests/test_web/`.
+ requirements:
+ - "REQ-003"
+ - "REQ-006"
+ dependencies:
+ - "TASK-003"
+ - "TASK-004"
+ suggested_role: "python"
+ acceptance_criteria:
+ - "Tests confirm default value is `*`"
+ - "Tests confirm specific IP/list parsing works"
+ - "Tests confirm startup warning is emitted with wildcard default"
+ - "Tests confirm no warning when specific hosts are configured"
+ - "All tests pass"
+ estimated_complexity: "medium"
+ files_affected:
+ - "tests/test_common/test_config.py"
+ - "tests/test_web/test_app.py"
+
+ - id: "TASK-011"
+ title: "Write tests for config JSON script block escaping"
+ description: |
+ Add tests in `tests/test_web/` to verify that the config JSON escaping prevents XSS breakout.
+
+ Tests to add:
+ 1. A config value containing `` is escaped to `<\/script>...` in
+ the rendered HTML.
+ 2. A config value without special characters renders unchanged.
+ 3. The escaped JSON is still valid and parseable by `json.loads()` (after un-escaping `<\/` back to `` in config values is escaped to `<\\/script>`"
+ - "Test confirms normal config values are unaffected"
+ - "Test confirms escaped JSON is still valid and parseable"
+ - "All tests pass"
+ estimated_complexity: "small"
+ files_affected:
+ - "tests/test_web/test_app.py"
+
+ - id: "TASK-012"
+ title: "Update documentation for WEB_TRUSTED_PROXY_HOSTS setting"
+ description: |
+ Update project documentation to document the new `WEB_TRUSTED_PROXY_HOSTS` environment variable.
+
+ Files to update:
+
+ 1. **README.md** — Add `WEB_TRUSTED_PROXY_HOSTS` to the environment variables table with description:
+ "Comma-separated list of trusted proxy hosts for admin authentication headers. Default: `*` (all hosts).
+ Recommended: set to your reverse proxy IP in production."
+
+ 2. **AGENTS.md** — Add `WEB_TRUSTED_PROXY_HOSTS` to the Environment Variables section with the same description.
+
+ 3. **PLAN.md** — If there is a configuration section, add the new variable there as well.
+
+ Ensure the documentation notes:
+ - Default is `*` for backward compatibility
+ - A startup warning is emitted when using the default with admin enabled
+ - Operators should set this to their reverse proxy IP in production
+ requirements:
+ - "REQ-003"
+ - "REQ-006"
+ dependencies:
+ - "TASK-003"
+ - "TASK-004"
+ suggested_role: "docs"
+ acceptance_criteria:
+ - "`WEB_TRUSTED_PROXY_HOSTS` is documented in README.md"
+ - "`WEB_TRUSTED_PROXY_HOSTS` is documented in AGENTS.md"
+ - "Documentation notes the default value, startup warning, and production recommendation"
+ estimated_complexity: "small"
+ files_affected:
+ - "README.md"
+ - "AGENTS.md"
+ - "PLAN.md"