mirror of
https://github.com/ipnet-mesh/meshcore-hub.git
synced 2026-03-28 17:42:56 +01:00
Compare commits
13 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
92ff1ab306 | ||
|
|
0e2a24caa6 | ||
|
|
ff36a991af | ||
|
|
fa1a2ecc17 | ||
|
|
9099ffb0cb | ||
|
|
f8219b4626 | ||
|
|
27b78d6904 | ||
|
|
d4c3e127a2 | ||
|
|
92e9ccdbfa | ||
|
|
29b5820ed1 | ||
|
|
889aa32e3a | ||
|
|
3c3873951d | ||
|
|
4b58160f31 |
103
.agentmap.yaml
Normal file
103
.agentmap.yaml
Normal file
@@ -0,0 +1,103 @@
|
||||
# MeshCore Hub — codebase orientation map
|
||||
# See: https://github.com/anthropics/agentmap
|
||||
|
||||
meta:
|
||||
project: meshcore-hub
|
||||
version: 1
|
||||
updated: "2026-02-27"
|
||||
stack:
|
||||
- python 3.13
|
||||
- fastapi
|
||||
- sqlalchemy (async)
|
||||
- paho-mqtt
|
||||
- click
|
||||
- lit-html SPA
|
||||
- tailwind + daisyui
|
||||
- sqlite
|
||||
|
||||
tasks:
|
||||
install: "pip install -e '.[dev]'"
|
||||
test: "pytest"
|
||||
run: "meshcore-hub api --reload"
|
||||
lint: "pre-commit run --all-files"
|
||||
|
||||
tree:
|
||||
src/meshcore_hub/:
|
||||
__main__.py: "Click CLI entry point, registers subcommands"
|
||||
common/:
|
||||
config.py: "pydantic-settings, all env vars [config]"
|
||||
database.py: "async SQLAlchemy session management"
|
||||
mqtt.py: "MQTT client helpers"
|
||||
i18n.py: "translation loader, t() function"
|
||||
models/:
|
||||
base.py: "Base, UUIDMixin, TimestampMixin"
|
||||
node.py: null
|
||||
member.py: null
|
||||
advertisement.py: null
|
||||
message.py: null
|
||||
telemetry.py: null
|
||||
node_tag.py: null
|
||||
schemas/:
|
||||
events.py: "inbound MQTT event schemas"
|
||||
commands.py: "outbound command schemas"
|
||||
nodes.py: "API request/response schemas"
|
||||
members.py: null
|
||||
messages.py: null
|
||||
interface/:
|
||||
receiver.py: "reads device events, publishes to MQTT"
|
||||
sender.py: "subscribes MQTT commands, writes to device"
|
||||
device.py: "meshcore library wrapper"
|
||||
mock_device.py: "fake device for testing"
|
||||
collector/:
|
||||
subscriber.py: "MQTT subscriber, routes events to handlers"
|
||||
handlers/: "per-event-type DB persistence"
|
||||
cleanup.py: "data retention and node cleanup"
|
||||
webhook.py: "forward events to HTTP endpoints"
|
||||
tag_import.py: "seed node tags from YAML"
|
||||
member_import.py: "seed members from YAML"
|
||||
api/:
|
||||
app.py: "FastAPI app factory"
|
||||
auth.py: "API key authentication"
|
||||
dependencies.py: "DI for db session and auth"
|
||||
metrics.py: "Prometheus /metrics endpoint"
|
||||
routes/: "REST endpoints per resource"
|
||||
web/:
|
||||
app.py: "FastAPI app factory, SPA shell"
|
||||
pages.py: "custom markdown page loader"
|
||||
middleware.py: null
|
||||
templates/:
|
||||
spa.html: "single Jinja2 shell template"
|
||||
static/js/spa/:
|
||||
app.js: "SPA entry, route registration"
|
||||
router.js: "History API client-side router"
|
||||
api.js: "fetch wrapper for API calls"
|
||||
components.js: "shared lit-html helpers, t() re-export"
|
||||
icons.js: "SVG icon functions"
|
||||
pages/: "lazy-loaded page modules"
|
||||
alembic/: "DB migrations"
|
||||
etc/:
|
||||
prometheus/: "Prometheus scrape + alert rules"
|
||||
alertmanager/: null
|
||||
seed/: "YAML seed data (node_tags, members)"
|
||||
tests/:
|
||||
|
||||
key_symbols:
|
||||
- src/meshcore_hub/__main__.py::cli — Click root group [entry-point]
|
||||
- src/meshcore_hub/common/config.py::CommonSettings — shared env config base
|
||||
- src/meshcore_hub/common/database.py::DatabaseManager — async session factory
|
||||
- src/meshcore_hub/common/models/base.py::Base — declarative base for all models
|
||||
- src/meshcore_hub/api/app.py::create_app — API FastAPI factory
|
||||
- src/meshcore_hub/web/app.py::create_app — Web FastAPI factory
|
||||
- src/meshcore_hub/api/auth.py::require_read — read-key auth dependency
|
||||
- src/meshcore_hub/api/auth.py::require_admin — admin-key auth dependency
|
||||
- src/meshcore_hub/collector/subscriber.py::MQTTSubscriber — event ingestion loop
|
||||
- src/meshcore_hub/interface/receiver.py::Receiver — device→MQTT bridge
|
||||
- src/meshcore_hub/interface/sender.py::Sender — MQTT→device bridge
|
||||
|
||||
conventions:
|
||||
- four Click subcommands: interface, collector, api, web
|
||||
- "MQTT topic pattern: {prefix}/{pubkey}/event/{name} and .../command/{name}"
|
||||
- env config via pydantic-settings, no manual os.environ
|
||||
- web SPA: ES modules + lit-html, pages export async render()
|
||||
- i18n via t() with JSON locale files in static/locales/
|
||||
- node tags are freeform key-value pairs, standard keys in AGENTS.md
|
||||
12
.github/workflows/ci.yml
vendored
12
.github/workflows/ci.yml
vendored
@@ -1,6 +1,8 @@
|
||||
name: CI
|
||||
|
||||
on:
|
||||
push:
|
||||
branches: [main]
|
||||
pull_request:
|
||||
branches: [main]
|
||||
paths:
|
||||
@@ -45,7 +47,7 @@ jobs:
|
||||
|
||||
- name: Run tests with pytest
|
||||
run: |
|
||||
pytest --cov=meshcore_hub --cov-report=xml --cov-report=term-missing
|
||||
pytest --cov=meshcore_hub --cov-report=xml --cov-report=term-missing --junitxml=junit.xml -o junit_family=legacy
|
||||
|
||||
- name: Upload coverage to Codecov
|
||||
uses: codecov/codecov-action@v5
|
||||
@@ -54,6 +56,14 @@ jobs:
|
||||
files: ./coverage.xml
|
||||
fail_ci_if_error: false
|
||||
verbose: true
|
||||
token: ${{ secrets.CODECOV_TOKEN }}
|
||||
|
||||
- name: Upload test results to Codecov
|
||||
uses: codecov/codecov-action@v5
|
||||
if: ${{ !cancelled() }}
|
||||
with:
|
||||
report_type: test_results
|
||||
token: ${{ secrets.CODECOV_TOKEN }}
|
||||
|
||||
build:
|
||||
name: Build Package
|
||||
|
||||
100
.plans/2026/03/09/01-security-fixes/changelog.md
Normal file
100
.plans/2026/03/09/01-security-fixes/changelog.md
Normal file
@@ -0,0 +1,100 @@
|
||||
## TASK-001: Remove legacy HTML dashboard endpoint
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `src/meshcore_hub/api/routes/dashboard.py`
|
||||
- `tests/test_api/test_dashboard.py`
|
||||
### Notes
|
||||
Removed the `dashboard()` route handler and its `@router.get("")` decorator. Removed `HTMLResponse` and `Request` imports no longer used. Updated existing tests to verify the HTML endpoint returns 404/405. All JSON sub-routes (`/stats`, `/activity`, `/message-activity`, `/node-count`) remain intact.
|
||||
---
|
||||
|
||||
## TASK-002: Replace API key comparisons with constant-time comparison
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `src/meshcore_hub/api/auth.py`
|
||||
- `src/meshcore_hub/api/metrics.py`
|
||||
### Notes
|
||||
Added `import hmac` to both files. Replaced `==` comparisons with `hmac.compare_digest()` in `require_read`, `require_admin`, and `verify_basic_auth`. Added truthiness guards for `read_key`/`admin_key` in `require_read` since either can be `None` and `hmac.compare_digest()` raises `TypeError` on `None`.
|
||||
---
|
||||
|
||||
## TASK-003: Add WEB_TRUSTED_PROXY_HOSTS configuration setting
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `src/meshcore_hub/common/config.py`
|
||||
### Notes
|
||||
Added `web_trusted_proxy_hosts: str = Field(default="*", ...)` to `WebSettings` class. Automatically configurable via `WEB_TRUSTED_PROXY_HOSTS` env var through Pydantic Settings.
|
||||
---
|
||||
|
||||
## TASK-004: Integrate trusted proxy hosts into web app middleware and add startup warning
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `src/meshcore_hub/web/app.py`
|
||||
### Notes
|
||||
Replaced hardcoded `trusted_hosts="*"` in `ProxyHeadersMiddleware` with configured value. If value is `"*"`, passes string directly; otherwise splits on commas. Added startup warning when `WEB_ADMIN_ENABLED=true` and `WEB_TRUSTED_PROXY_HOSTS="*"`. `_is_authenticated_proxy_request` unchanged.
|
||||
---
|
||||
|
||||
## TASK-005: Escape config JSON in template script block to prevent XSS breakout
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `src/meshcore_hub/web/app.py`
|
||||
### Notes
|
||||
Added `.replace("</", "<\\/")` to `_build_config_json` return value. Prevents `</script>` breakout in the Jinja2 template's `<script>` block. `<\/` is valid JSON per spec and parsed correctly by `JSON.parse()`.
|
||||
---
|
||||
|
||||
## TASK-006: Fix stored XSS in admin node-tags page
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js`
|
||||
### Notes
|
||||
Added `escapeHtml` to imports. Escaped `nodeName` with `escapeHtml()` in copy-all and delete-all confirmation dialogs (2 `unsafeHTML()` calls). Escaped `activeTagKey` with `escapeHtml()` in single tag delete confirmation (`innerHTML` assignment). Translation template `<strong>` tags preserved.
|
||||
---
|
||||
|
||||
## TASK-007: Fix stored XSS in admin members page
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `src/meshcore_hub/web/static/js/spa/pages/admin/members.js`
|
||||
### Notes
|
||||
Added `escapeHtml` to imports. Escaped `memberName` with `escapeHtml()` before passing to `t()` in delete confirmation dialog. `innerHTML` retained for `<strong>` tag rendering from translation template.
|
||||
---
|
||||
|
||||
## TASK-008: Write tests for legacy dashboard endpoint removal
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `tests/test_api/test_dashboard.py`
|
||||
### Notes
|
||||
Added 5 new tests: 1 for trailing-slash 404/405 verification, 4 for authenticated JSON sub-route responses. Total 20 dashboard tests passing.
|
||||
---
|
||||
|
||||
## TASK-009: Write tests for constant-time API key comparison
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `tests/test_api/test_auth.py`
|
||||
### Notes
|
||||
Restructured from 10 tests (2 classes) to 22 tests (4 classes): `TestReadAuthentication` (9), `TestAdminAuthentication` (4), `TestMetricsAuthentication` (7), `TestHealthEndpoint` (2). Added coverage for multi-endpoint read/admin key acceptance, missing auth header rejection, and metrics credential validation.
|
||||
---
|
||||
|
||||
## TASK-010: Write tests for trusted proxy hosts configuration and startup warning
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `tests/test_common/test_config.py`
|
||||
- `tests/test_web/test_app.py`
|
||||
### Notes
|
||||
Added 3 config tests (default value, specific IP, comma-separated list) and 5 web app tests (warning logged with wildcard+admin, no warning with specific hosts, no warning with admin disabled, comma list parsing, wildcard passed as string).
|
||||
---
|
||||
|
||||
## TASK-011: Write tests for config JSON script block escaping
|
||||
**Status:** completed
|
||||
### Files Created
|
||||
- `tests/test_web/test_app.py`
|
||||
### Notes
|
||||
Added 5 tests in `TestConfigJsonXssEscaping` class: rendered HTML escaping, normal values unaffected, escaped JSON parseable, direct `_build_config_json` escaping, direct no-escaping-needed.
|
||||
---
|
||||
|
||||
## TASK-012: Update documentation for WEB_TRUSTED_PROXY_HOSTS setting
|
||||
**Status:** completed
|
||||
### Files Modified
|
||||
- `README.md`
|
||||
- `AGENTS.md`
|
||||
- `PLAN.md`
|
||||
### Notes
|
||||
Added `WEB_TRUSTED_PROXY_HOSTS` to environment variables sections in all three docs. Documented default value (`*`), production recommendation, and startup warning behavior.
|
||||
---
|
||||
162
.plans/2026/03/09/01-security-fixes/prd.md
Normal file
162
.plans/2026/03/09/01-security-fixes/prd.md
Normal file
@@ -0,0 +1,162 @@
|
||||
# Product Requirements Document
|
||||
|
||||
> Source: `.plans/2026/03/09/01-security-fixes/prompt.md`
|
||||
|
||||
## Project Overview
|
||||
|
||||
This project addresses CRITICAL and HIGH severity vulnerabilities identified in a security audit of MeshCore Hub. The fixes span stored XSS in server-rendered and client-side code, timing attacks on authentication, proxy header forgery, and a legacy endpoint with missing authentication. All changes must be backward-compatible and preserve existing API contracts.
|
||||
|
||||
## Goals
|
||||
|
||||
- Eliminate all CRITICAL and HIGH severity security vulnerabilities found in the audit
|
||||
- Harden API key comparison against timing side-channel attacks
|
||||
- Prevent XSS vectors in both Jinja2 templates and client-side JavaScript
|
||||
- Add configurable proxy trust to defend against header forgery while maintaining backward compatibility
|
||||
- Remove the redundant legacy HTML dashboard endpoint that lacks authentication
|
||||
|
||||
## Functional Requirements
|
||||
|
||||
### REQ-001: Remove legacy HTML dashboard endpoint
|
||||
|
||||
**Description:** Remove the `GET /api/v1/dashboard/` route handler that renders a standalone HTML page with unescaped database content (stored XSS) and no authentication. The JSON sub-routes (`/stats`, `/activity`, `/message-activity`, `/node-count`) must remain intact and unchanged.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] The `dashboard()` route handler in `api/routes/dashboard.py` is removed
|
||||
- [ ] The `HTMLResponse` import is removed (if no longer used)
|
||||
- [ ] `GET /api/v1/dashboard/` returns 404 or Method Not Allowed
|
||||
- [ ] `GET /api/v1/dashboard/stats` continues to return valid JSON with authentication
|
||||
- [ ] `GET /api/v1/dashboard/activity` continues to return valid JSON with authentication
|
||||
- [ ] `GET /api/v1/dashboard/message-activity` continues to return valid JSON with authentication
|
||||
- [ ] `GET /api/v1/dashboard/node-count` continues to return valid JSON with authentication
|
||||
- [ ] Existing API tests for JSON sub-routes still pass
|
||||
|
||||
### REQ-002: Use constant-time comparison for API key validation
|
||||
|
||||
**Description:** Replace all Python `==` comparisons of API keys and credentials with `hmac.compare_digest()` to prevent timing side-channel attacks that could leak key material.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] All API key comparisons in `api/auth.py` use `hmac.compare_digest()` instead of `==`
|
||||
- [ ] All credential comparisons in `api/metrics.py` use `hmac.compare_digest()` instead of `==`
|
||||
- [ ] `hmac` is imported in all files where secret comparison occurs
|
||||
- [ ] The authentication behavior is unchanged — valid keys are accepted, invalid keys are rejected
|
||||
- [ ] Tests confirm authentication still works correctly with valid and invalid keys
|
||||
|
||||
### REQ-003: Add configurable trusted proxy hosts for admin authentication
|
||||
|
||||
**Description:** Add a `WEB_TRUSTED_PROXY_HOSTS` configuration setting that controls which hosts are trusted for proxy authentication headers (`X-Forwarded-User`, `X-Auth-Request-User`, `Authorization: Basic`). The setting defaults to `*` for backward compatibility. A startup warning is emitted when admin is enabled with the wildcard default. The `Authorization: Basic` header check must be preserved for Nginx Proxy Manager compatibility.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] A `WEB_TRUSTED_PROXY_HOSTS` setting is added to the configuration (Pydantic Settings)
|
||||
- [ ] The setting defaults to `*` (backward compatible)
|
||||
- [ ] `ProxyHeadersMiddleware` uses the configured `trusted_hosts` value instead of hardcoded `*`
|
||||
- [ ] A warning is logged at startup when `WEB_ADMIN_ENABLED=true` and `WEB_TRUSTED_PROXY_HOSTS` is `*`
|
||||
- [ ] The warning message recommends restricting trusted hosts to the operator's proxy IP
|
||||
- [ ] The `_is_authenticated_proxy_request` function continues to accept `X-Forwarded-User`, `X-Auth-Request-User`, and `Authorization: Basic` headers
|
||||
- [ ] OAuth2 proxy setups continue to function correctly
|
||||
- [ ] Setting `WEB_TRUSTED_PROXY_HOSTS` to a specific IP restricts proxy header trust to that IP
|
||||
|
||||
### REQ-004: Escape config JSON in template script block
|
||||
|
||||
**Description:** Prevent XSS via `</script>` breakout in the `config_json|safe` template injection by escaping `</` sequences in the serialized JSON string before passing it to the Jinja2 template.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] `config_json` is escaped by replacing `</` with `<\\/` before template rendering (in `web/app.py`)
|
||||
- [ ] The `|safe` filter continues to be used (the escaping happens in Python, not Jinja2)
|
||||
- [ ] A config value containing `</script><script>alert(1)</script>` does not execute JavaScript
|
||||
- [ ] The SPA application correctly parses the escaped config JSON on the client side
|
||||
- [ ] Normal config values (without special characters) render unchanged
|
||||
|
||||
### REQ-005: Fix stored XSS in admin page JavaScript
|
||||
|
||||
**Description:** Sanitize API-sourced data (node names, tag keys, member names) before rendering in admin pages. Replace `unsafeHTML()` and direct `innerHTML` assignment with safe alternatives — either `escapeHtml()` (already available in `components.js`) or lit-html safe templating (`${value}` interpolation without `unsafeHTML`).
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] Node names in `admin/node-tags.js` are escaped or safely templated before HTML rendering
|
||||
- [ ] Tag keys in `admin/node-tags.js` are escaped or safely templated before HTML rendering
|
||||
- [ ] Member names in `admin/members.js` are escaped or safely templated before HTML rendering
|
||||
- [ ] All `unsafeHTML()` calls on API-sourced data in the identified files are replaced with safe alternatives
|
||||
- [ ] All direct `innerHTML` assignments of API-sourced data in the identified files are replaced with safe alternatives
|
||||
- [ ] A node name containing `<img src=x onerror=alert(1)>` renders as text, not as an HTML element
|
||||
- [ ] A member name containing `<script>alert(1)</script>` renders as text, not as executable script
|
||||
- [ ] Normal names (without special characters) continue to display correctly
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
### REQ-006: Backward compatibility
|
||||
|
||||
**Category:** Reliability
|
||||
|
||||
**Description:** All security fixes must maintain backward compatibility with existing deployments. No breaking changes to API contracts, configuration defaults, or deployment workflows.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] All existing API endpoints (except the removed HTML dashboard) return the same response format
|
||||
- [ ] Default configuration values preserve existing behavior without requiring operator action
|
||||
- [ ] Docker Compose deployments continue to function without configuration changes
|
||||
- [ ] All existing tests pass after the security fixes are applied
|
||||
|
||||
### REQ-007: No regression in authentication flows
|
||||
|
||||
**Category:** Security
|
||||
|
||||
**Description:** The security hardening must not introduce authentication regressions. Valid credentials must continue to be accepted, and invalid credentials must continue to be rejected, across all authentication methods.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] API read key authentication accepts valid keys and rejects invalid keys
|
||||
- [ ] API admin key authentication accepts valid keys and rejects invalid keys
|
||||
- [ ] Metrics endpoint authentication (if configured) accepts valid credentials and rejects invalid ones
|
||||
- [ ] Proxy header authentication continues to work with OAuth2 proxy setups
|
||||
- [ ] Basic auth header forwarding from Nginx Proxy Manager continues to work
|
||||
|
||||
## Technical Constraints and Assumptions
|
||||
|
||||
### Constraints
|
||||
|
||||
- Python 3.13+ (specified by project `.python-version`)
|
||||
- Must use `hmac.compare_digest()` from the Python standard library for constant-time comparison
|
||||
- The `Authorization: Basic` header check in `_is_authenticated_proxy_request` must not be removed or modified to validate credentials server-side — credential validation is the proxy's responsibility
|
||||
- Changes must not alter existing API response schemas or status codes (except removing the HTML dashboard endpoint)
|
||||
|
||||
### Assumptions
|
||||
|
||||
- The `escapeHtml()` utility in `components.js` correctly escapes `<`, `>`, `&`, `"`, and `'` characters
|
||||
- The SPA client-side JavaScript can parse JSON containing escaped `<\/` sequences (standard behavior per JSON spec)
|
||||
- Operators using proxy authentication have a reverse proxy (e.g., Nginx, Traefik, NPM) in front of MeshCore Hub
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
|
||||
- Removing the legacy HTML dashboard route handler (C1 + H2)
|
||||
- Replacing `==` with `hmac.compare_digest()` for all secret comparisons (H1)
|
||||
- Adding `WEB_TRUSTED_PROXY_HOSTS` configuration and startup warning (H3)
|
||||
- Escaping `</` in config JSON template injection (H4)
|
||||
- Fixing `unsafeHTML()`/`innerHTML` XSS in admin JavaScript pages (H5)
|
||||
- Updating tests to cover the security fixes
|
||||
- Updating documentation for the new `WEB_TRUSTED_PROXY_HOSTS` setting
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- MEDIUM severity findings (CORS, error detail leakage, rate limiting, security headers, CSRF, CDN SRI, markdown sanitization, input validation, channel key exposure)
|
||||
- LOW severity findings (auth warnings, version disclosure, unbounded fields, credential logging, SecretStr, port exposure, cache safety, image pinning)
|
||||
- INFO findings (OpenAPI docs, proxy IP logging, alertmanager comments, DOM XSS in error handler, locale path)
|
||||
- Adding rate limiting infrastructure
|
||||
- Adding Content-Security-Policy or other security headers
|
||||
- Dependency version pinning or lockfile generation
|
||||
- Server-side credential validation for Basic auth (proxy responsibility)
|
||||
|
||||
## Suggested Tech Stack
|
||||
|
||||
| Layer | Technology | Rationale |
|
||||
|-------|-----------|-----------|
|
||||
| Secret comparison | `hmac.compare_digest()` (stdlib) | Specified by prompt; constant-time comparison prevents timing attacks |
|
||||
| Template escaping | Python `str.replace()` | Minimal approach to escape `</` in JSON before Jinja2 rendering |
|
||||
| Client-side escaping | `escapeHtml()` from `components.js` | Already available in the codebase; standard HTML entity escaping |
|
||||
| Configuration | Pydantic Settings | Specified by project stack; used for `WEB_TRUSTED_PROXY_HOSTS` |
|
||||
| Testing | pytest, pytest-asyncio | Specified by project stack |
|
||||
65
.plans/2026/03/09/01-security-fixes/prompt.md
Normal file
65
.plans/2026/03/09/01-security-fixes/prompt.md
Normal file
@@ -0,0 +1,65 @@
|
||||
# Phase: 01-security-fixes
|
||||
|
||||
## Overview
|
||||
|
||||
Address CRITICAL and HIGH severity vulnerabilities identified in the MeshCore Hub security audit across API and Web components. These findings represent exploitable vulnerabilities including XSS, timing attacks, authentication bypasses, and insecure defaults.
|
||||
|
||||
## Goals
|
||||
|
||||
- Eliminate all CRITICAL and HIGH severity security vulnerabilities
|
||||
- Harden authentication mechanisms against timing attacks and header forgery
|
||||
- Prevent XSS vectors in both server-rendered HTML and client-side JavaScript
|
||||
- Secure default MQTT configuration against unauthenticated access
|
||||
|
||||
## Requirements
|
||||
|
||||
### C1 + H2 — Remove legacy HTML dashboard endpoint
|
||||
- **File:** `src/meshcore_hub/api/routes/dashboard.py:367-536`
|
||||
- The `GET /api/v1/dashboard/` endpoint is a standalone HTML page with two CRITICAL/HIGH issues: stored XSS (unescaped DB content in f-string HTML) and missing authentication
|
||||
- The SPA web dashboard provides a full-featured replacement, making this endpoint redundant
|
||||
- **Fix:** Remove the `dashboard()` route handler and its `HTMLResponse` import. Keep all JSON sub-routes (`/stats`, `/activity`, `/message-activity`, `/node-count`) intact.
|
||||
|
||||
### H1 — Fix timing attack on API key comparison
|
||||
- **Files:** `api/auth.py:82,127` | `api/metrics.py:57`
|
||||
- All secret comparisons use Python `==`, which is not constant-time
|
||||
- **Fix:** Replace with `hmac.compare_digest()` for all key/credential comparisons
|
||||
|
||||
### H3 — Harden admin auth against proxy header forgery
|
||||
- **File:** `web/app.py:73-86,239`
|
||||
- Admin access trusts `X-Forwarded-User`, `X-Auth-Request-User`, or `Authorization: Basic` header
|
||||
- `ProxyHeadersMiddleware(trusted_hosts="*")` accepts forged headers from any client
|
||||
- The `Authorization: Basic` check must be preserved — it is required by the Nginx Proxy Manager (NPM) Access List setup documented in README.md (NPM validates credentials and forwards the header)
|
||||
- **Fix:** Add a `WEB_TRUSTED_PROXY_HOSTS` config setting (default `*` for backward compatibility). Pass it to `ProxyHeadersMiddleware(trusted_hosts=...)`. Add a startup warning when `WEB_ADMIN_ENABLED=true` and `trusted_hosts` is still `*`, recommending operators restrict it to their proxy IP. Do NOT remove the Basic auth header check or validate credentials server-side — that is the proxy's responsibility.
|
||||
|
||||
### H4 — Fix XSS via config_json|safe script block breakout
|
||||
- **File:** `web/templates/spa.html:188` | `web/app.py:157-183`
|
||||
- Operator config values injected into `<script>` block with `|safe` — a value containing `</script>` breaks out and executes arbitrary JS
|
||||
- **Fix:** Escape `</` sequences in the JSON string: `config_json = json.dumps(config).replace("</", "<\\/")`
|
||||
|
||||
### H5 — Fix stored XSS via unsafeHTML/innerHTML with API-sourced data
|
||||
- **Files:** `web/static/js/spa/pages/admin/node-tags.js:243,272,454` | `admin/members.js:309`
|
||||
- Node names, tag keys, and member names from the API are interpolated into HTML via `unsafeHTML()` and direct `innerHTML` assignment
|
||||
- **Fix:** Use `escapeHtml()` (already in `components.js`) on API data before HTML interpolation, or replace with lit-html safe templating
|
||||
|
||||
|
||||
## Constraints
|
||||
|
||||
- Must not break existing functionality or API contracts
|
||||
- Changes to docker-compose.yml and mosquitto.conf must remain backward-compatible (use env var defaults)
|
||||
- The `_is_authenticated_proxy_request` function must continue to work with OAuth2 proxy setups — only add defense-in-depth, don't remove proxy header support entirely
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- MEDIUM severity findings (CORS config, error detail leakage, rate limiting, security headers, CSRF, CDN SRI, markdown sanitization, input validation, channel key exposure)
|
||||
- LOW severity findings (auth warnings, version disclosure, unbounded fields, credential logging, SecretStr, port exposure, cache safety, image pinning)
|
||||
- INFO findings (OpenAPI docs, proxy IP logging, alertmanager comments, DOM XSS in error handler, locale path)
|
||||
- Adding rate limiting infrastructure
|
||||
- Adding Content-Security-Policy or other security headers
|
||||
- Dependency version pinning or lockfile generation
|
||||
|
||||
## References
|
||||
|
||||
- Security audit performed in this conversation (2026-03-09)
|
||||
- OWASP Top 10: XSS (A7:2017), Broken Authentication (A2:2017)
|
||||
- Python `hmac.compare_digest` documentation
|
||||
- FastAPI security best practices
|
||||
54
.plans/2026/03/09/01-security-fixes/reviews/cycle/001.yaml
Normal file
54
.plans/2026/03/09/01-security-fixes/reviews/cycle/001.yaml
Normal file
@@ -0,0 +1,54 @@
|
||||
# Code review round 001
|
||||
# Phase: .plans/2026/03/09/01-security-fixes
|
||||
# Scope: full
|
||||
# Generated by: /jp-codereview
|
||||
|
||||
issues:
|
||||
- id: "ISSUE-001"
|
||||
severity: "MINOR"
|
||||
category: "integration"
|
||||
file: "src/meshcore_hub/web/app.py"
|
||||
line: 251
|
||||
description: |
|
||||
The startup warning for insecure trusted proxy hosts checks `settings.web_admin_enabled`
|
||||
instead of the effective admin_enabled value that gets stored in `app.state.admin_enabled`.
|
||||
The `create_app()` function accepts an `admin_enabled` parameter (line 193) that can override
|
||||
the setting. If a caller passes `admin_enabled=True` but `settings.web_admin_enabled` is False,
|
||||
the warning will not fire despite admin being enabled. In practice this does not affect production
|
||||
deployments (CLI always uses the settings value), only programmatic/test usage.
|
||||
suggestion: |
|
||||
Consider computing the effective admin_enabled value before the warning check and using
|
||||
that for both the warning and `app.state.admin_enabled`, e.g.:
|
||||
`effective_admin = admin_enabled if admin_enabled is not None else settings.web_admin_enabled`
|
||||
related_tasks:
|
||||
- "TASK-004"
|
||||
|
||||
- id: "ISSUE-002"
|
||||
severity: "MINOR"
|
||||
category: "style"
|
||||
file: "src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js"
|
||||
line: 3
|
||||
description: |
|
||||
The `unsafeHTML` import is retained and still used on lines 243 and 272. Although the
|
||||
API-sourced data (`nodeName`) is now safely escaped via `escapeHtml()` before interpolation,
|
||||
the continued use of `unsafeHTML()` may confuse future reviewers into thinking the XSS
|
||||
fix is incomplete. The `unsafeHTML()` is needed to render the translation template's HTML
|
||||
tags (e.g., `<strong>`), so this is functionally correct.
|
||||
suggestion: |
|
||||
Add a brief inline comment above each `unsafeHTML()` call explaining that the dynamic
|
||||
values are pre-escaped and `unsafeHTML()` is only needed for the template's HTML formatting.
|
||||
related_tasks:
|
||||
- "TASK-006"
|
||||
|
||||
summary:
|
||||
total_issues: 2
|
||||
critical: 0
|
||||
major: 0
|
||||
minor: 2
|
||||
by_category:
|
||||
integration: 1
|
||||
architecture: 0
|
||||
security: 0
|
||||
duplication: 0
|
||||
error-handling: 0
|
||||
style: 1
|
||||
70
.plans/2026/03/09/01-security-fixes/reviews/prd.md
Normal file
70
.plans/2026/03/09/01-security-fixes/reviews/prd.md
Normal file
@@ -0,0 +1,70 @@
|
||||
# PRD Review
|
||||
|
||||
> Phase: `.plans/2026/03/09/01-security-fixes`
|
||||
> PRD: `.plans/2026/03/09/01-security-fixes/prd.md`
|
||||
> Prompt: `.plans/2026/03/09/01-security-fixes/prompt.md`
|
||||
|
||||
## Verdict: PASS
|
||||
|
||||
The PRD fully covers all five security requirements from the prompt with clear, implementable, and testable acceptance criteria. No contradictions, blocking ambiguities, or feasibility concerns were found. One prompt goal ("Secure default MQTT configuration") has no corresponding requirement in either the prompt or the PRD, but since no prompt requirement addresses it, the PRD correctly does not fabricate one.
|
||||
|
||||
## Coverage Assessment
|
||||
|
||||
| Prompt Item | PRD Section | Covered? | Notes |
|
||||
|---|---|---|---|
|
||||
| C1+H2: Remove legacy HTML dashboard endpoint | REQ-001 | Yes | Route removal, import cleanup, sub-route preservation all specified |
|
||||
| H1: Fix timing attack on API key comparison | REQ-002 | Yes | Files and `hmac.compare_digest()` approach match |
|
||||
| H3: Harden admin auth / proxy header forgery | REQ-003 | Yes | Config setting, default, warning, Basic auth preservation all covered |
|
||||
| H4: Fix XSS via config_json\|safe breakout | REQ-004 | Yes | Escape approach and XSS test payload specified |
|
||||
| H5: Fix stored XSS via unsafeHTML/innerHTML | REQ-005 | Yes | Files, fix approach, and XSS test payloads specified |
|
||||
| Constraint: No breaking changes to API contracts | REQ-006 | Yes | |
|
||||
| Constraint: docker-compose.yml/mosquitto.conf backward-compatible | REQ-006 | Partial | REQ-006 covers Docker Compose but not mosquitto.conf; moot since no requirement changes mosquitto.conf |
|
||||
| Constraint: _is_authenticated_proxy_request works with OAuth2 | REQ-003, REQ-007 | Yes | |
|
||||
| Goal: Secure default MQTT configuration | -- | No | Goal stated in prompt but no prompt requirement addresses it; PRD correctly does not fabricate one |
|
||||
| Out of scope items | Scope section | Yes | All exclusions match prompt |
|
||||
|
||||
**Coverage summary:** 5 of 5 prompt requirements fully covered, 1 constraint partially covered (moot), 1 prompt goal has no corresponding requirement in the prompt itself.
|
||||
|
||||
## Requirement Evaluation
|
||||
|
||||
All requirements passed evaluation. Minor observations noted below.
|
||||
|
||||
### REQ-003: Add configurable trusted proxy hosts
|
||||
|
||||
- **Implementability:** Pass -- A developer familiar with Pydantic Settings and `ProxyHeadersMiddleware` can implement this without ambiguity. The env var format (comma-separated list vs. single value) is not explicitly stated but follows standard Pydantic patterns.
|
||||
- **Testability:** Pass
|
||||
- **Completeness:** Pass
|
||||
- **Consistency:** Pass
|
||||
|
||||
### REQ-006: Backward compatibility
|
||||
|
||||
- **Implementability:** Pass
|
||||
- **Testability:** Pass
|
||||
- **Completeness:** Pass -- The prompt constraint about mosquitto.conf backward compatibility is not explicitly mentioned, but no requirement modifies mosquitto.conf, making this moot.
|
||||
- **Consistency:** Pass
|
||||
|
||||
## Structural Issues
|
||||
|
||||
### Contradictions
|
||||
|
||||
None found.
|
||||
|
||||
### Ambiguities
|
||||
|
||||
None that would block implementation. The `WEB_TRUSTED_PROXY_HOSTS` env var format is a minor detail resolvable by the developer from the `ProxyHeadersMiddleware` API and standard Pydantic Settings patterns.
|
||||
|
||||
### Missing Edge Cases
|
||||
|
||||
None significant. The `hmac.compare_digest()` change (REQ-002) assumes the existing code handles the "no key configured" case before reaching the comparison, which is standard practice and verifiable during implementation.
|
||||
|
||||
### Feasibility Concerns
|
||||
|
||||
None.
|
||||
|
||||
### Scope Inconsistencies
|
||||
|
||||
The prompt states a goal of "Secure default MQTT configuration against unauthenticated access" but provides no requirement for it. The PRD drops this goal without explanation. This is a prompt-level gap, not a PRD-level gap -- the PRD should not invent requirements that the prompt does not specify.
|
||||
|
||||
## Action Items
|
||||
|
||||
No action items. The PRD is ready for task breakdown.
|
||||
90
.plans/2026/03/09/01-security-fixes/reviews/tasks.md
Normal file
90
.plans/2026/03/09/01-security-fixes/reviews/tasks.md
Normal file
@@ -0,0 +1,90 @@
|
||||
# Task Review
|
||||
|
||||
> Phase: `.plans/2026/03/09/01-security-fixes`
|
||||
> Tasks: `.plans/2026/03/09/01-security-fixes/tasks.yaml`
|
||||
> PRD: `.plans/2026/03/09/01-security-fixes/prd.md`
|
||||
|
||||
## Verdict: PASS
|
||||
|
||||
The task list is structurally sound, correctly ordered, and fully covers all 7 PRD requirements. The dependency graph is a valid DAG with no cycles or invalid references. No ordering issues, coverage gaps, vague tasks, or invalid fields were found. Two non-blocking warnings are noted: TASK-006 and TASK-007 (frontend XSS fixes) lack corresponding test tasks, and two pairs of independent tasks share output files but modify independent sections.
|
||||
|
||||
## Dependency Validation
|
||||
|
||||
### Reference Validity
|
||||
|
||||
All dependency references are valid. Every task ID referenced in a `dependencies` list corresponds to an existing task in the inventory.
|
||||
|
||||
### DAG Validation
|
||||
|
||||
The dependency graph is a valid directed acyclic graph. No cycles detected.
|
||||
|
||||
Topological layers:
|
||||
- **Layer 0 (roots):** TASK-001, TASK-002, TASK-003, TASK-005, TASK-006, TASK-007
|
||||
- **Layer 1:** TASK-004 (depends on TASK-003), TASK-008 (depends on TASK-001), TASK-009 (depends on TASK-002), TASK-011 (depends on TASK-005)
|
||||
- **Layer 2:** TASK-010 (depends on TASK-003, TASK-004), TASK-012 (depends on TASK-003, TASK-004)
|
||||
|
||||
### Orphan Tasks
|
||||
|
||||
No orphan tasks detected. All non-root tasks with dependencies are either terminal test/docs tasks (TASK-008, TASK-009, TASK-010, TASK-011, TASK-012) or integration tasks (TASK-004). Root tasks without dependents (TASK-006, TASK-007) are excluded from orphan detection per the review protocol.
|
||||
|
||||
## Ordering Check
|
||||
|
||||
No blocking ordering issues detected.
|
||||
|
||||
**Observation (non-blocking):** Two pairs of independent tasks share output files:
|
||||
|
||||
1. **TASK-004 and TASK-005** both modify `src/meshcore_hub/web/app.py` without a dependency between them. TASK-004 modifies `ProxyHeadersMiddleware` (line ~239) and adds a startup warning, while TASK-005 modifies `_build_config_json` (line ~183). These are independent functions in the same file; no actual conflict exists.
|
||||
|
||||
2. **TASK-010 and TASK-011** both modify `tests/test_web/test_app.py` without a dependency between them. Both add new test functions to the same test file. No actual conflict exists.
|
||||
|
||||
These are not blocking because neither task creates the shared file — both modify existing files in independent sections. Adding artificial dependencies would unnecessarily serialize parallelizable work.
|
||||
|
||||
## Coverage Check
|
||||
|
||||
### Uncovered Requirements
|
||||
|
||||
All PRD requirements are covered.
|
||||
|
||||
### Phantom References
|
||||
|
||||
No phantom references detected.
|
||||
|
||||
**Coverage summary:** 7 of 7 PRD requirements covered by tasks.
|
||||
|
||||
| Requirement | Tasks |
|
||||
|---|---|
|
||||
| REQ-001 | TASK-001, TASK-008 |
|
||||
| REQ-002 | TASK-002, TASK-009 |
|
||||
| REQ-003 | TASK-003, TASK-004, TASK-010, TASK-012 |
|
||||
| REQ-004 | TASK-005, TASK-011 |
|
||||
| REQ-005 | TASK-006, TASK-007 |
|
||||
| REQ-006 | TASK-001, TASK-003, TASK-004, TASK-005, TASK-006, TASK-007, TASK-008, TASK-010, TASK-011, TASK-012 |
|
||||
| REQ-007 | TASK-002, TASK-004, TASK-009 |
|
||||
|
||||
## Scope Check
|
||||
|
||||
### Tasks Too Large
|
||||
|
||||
No tasks flagged as too large. No task has `estimated_complexity: large`.
|
||||
|
||||
### Tasks Too Vague
|
||||
|
||||
No tasks flagged as too vague. All tasks have detailed descriptions (>50 chars), multiple testable acceptance criteria, and specific file paths in `files_affected`.
|
||||
|
||||
### Missing Test Tasks
|
||||
|
||||
Two implementation tasks lack corresponding test tasks:
|
||||
|
||||
- **TASK-006** (Fix stored XSS in admin node-tags page) — modifies `admin/node-tags.js` but no test task verifies the XSS fix in this JavaScript file. The acceptance criteria include XSS payload testing, but no automated test is specified. This is a frontend JavaScript change where manual verification or browser-based testing may be appropriate.
|
||||
|
||||
- **TASK-007** (Fix stored XSS in admin members page) — modifies `admin/members.js` but no test task verifies the XSS fix in this JavaScript file. Same reasoning as TASK-006.
|
||||
|
||||
**Note:** These are warnings, not blocking issues. The project's test infrastructure (`tests/test_web/`) focuses on server-side rendering and API responses. Client-side JavaScript XSS fixes are typically verified through acceptance criteria rather than automated unit tests.
|
||||
|
||||
### Field Validation
|
||||
|
||||
All tasks have valid fields:
|
||||
|
||||
- **Roles:** All `suggested_role` values are valid (`python`, `frontend`, `docs`).
|
||||
- **Complexity:** All `estimated_complexity` values are valid (`small`, `medium`).
|
||||
- **Completeness:** All 12 tasks have all required fields (`id`, `title`, `description`, `requirements`, `dependencies`, `suggested_role`, `acceptance_criteria`, `estimated_complexity`, `files_affected`). All list fields have at least one entry.
|
||||
22
.plans/2026/03/09/01-security-fixes/state.yaml
Normal file
22
.plans/2026/03/09/01-security-fixes/state.yaml
Normal file
@@ -0,0 +1,22 @@
|
||||
status: running
|
||||
phase_path: .plans/2026/03/09/01-security-fixes
|
||||
branch: fix/security-fixes
|
||||
current_phase: summary
|
||||
current_task: null
|
||||
fix_round: 0
|
||||
last_review_round: 1
|
||||
review_loop_exit_reason: success
|
||||
quality_gate: pass
|
||||
tasks:
|
||||
TASK-001: completed
|
||||
TASK-002: completed
|
||||
TASK-003: completed
|
||||
TASK-004: completed
|
||||
TASK-005: completed
|
||||
TASK-006: completed
|
||||
TASK-007: completed
|
||||
TASK-008: completed
|
||||
TASK-009: completed
|
||||
TASK-010: completed
|
||||
TASK-011: completed
|
||||
TASK-012: completed
|
||||
117
.plans/2026/03/09/01-security-fixes/summary.md
Normal file
117
.plans/2026/03/09/01-security-fixes/summary.md
Normal file
@@ -0,0 +1,117 @@
|
||||
# Phase Summary
|
||||
|
||||
> Phase: `.plans/2026/03/09/01-security-fixes`
|
||||
> Generated by: `/jp-summary`
|
||||
|
||||
## Project Overview
|
||||
|
||||
This phase addresses CRITICAL and HIGH severity vulnerabilities identified in a security audit of MeshCore Hub. The fixes span stored XSS in server-rendered and client-side code, timing attacks on authentication, proxy header forgery, and a legacy endpoint with missing authentication. All changes are backward-compatible and preserve existing API contracts.
|
||||
|
||||
### Goals
|
||||
|
||||
- Eliminate all CRITICAL and HIGH severity security vulnerabilities found in the audit
|
||||
- Harden API key comparison against timing side-channel attacks
|
||||
- Prevent XSS vectors in both Jinja2 templates and client-side JavaScript
|
||||
- Add configurable proxy trust to defend against header forgery while maintaining backward compatibility
|
||||
- Remove the redundant legacy HTML dashboard endpoint that lacks authentication
|
||||
|
||||
## Task Execution
|
||||
|
||||
### Overview
|
||||
|
||||
| Metric | Value |
|
||||
|---|---|
|
||||
| Total tasks | 12 |
|
||||
| Completed | 12 |
|
||||
| Failed | 0 |
|
||||
| Blocked | 0 |
|
||||
| Skipped | 0 |
|
||||
|
||||
### Task Details
|
||||
|
||||
| ID | Title | Role | Complexity | Status |
|
||||
|---|---|---|---|---|
|
||||
| TASK-001 | Remove legacy HTML dashboard endpoint | python | small | completed |
|
||||
| TASK-002 | Replace API key comparisons with constant-time comparison | python | small | completed |
|
||||
| TASK-003 | Add WEB_TRUSTED_PROXY_HOSTS configuration setting | python | small | completed |
|
||||
| TASK-004 | Integrate trusted proxy hosts into web app middleware and add startup warning | python | medium | completed |
|
||||
| TASK-005 | Escape config JSON in template script block to prevent XSS breakout | python | small | completed |
|
||||
| TASK-006 | Fix stored XSS in admin node-tags page | frontend | medium | completed |
|
||||
| TASK-007 | Fix stored XSS in admin members page | frontend | small | completed |
|
||||
| TASK-008 | Write tests for legacy dashboard endpoint removal | python | small | completed |
|
||||
| TASK-009 | Write tests for constant-time API key comparison | python | small | completed |
|
||||
| TASK-010 | Write tests for trusted proxy hosts configuration and startup warning | python | medium | completed |
|
||||
| TASK-011 | Write tests for config JSON script block escaping | python | small | completed |
|
||||
| TASK-012 | Update documentation for WEB_TRUSTED_PROXY_HOSTS setting | docs | small | completed |
|
||||
|
||||
### Requirement Coverage
|
||||
|
||||
| Metric | Value |
|
||||
|---|---|
|
||||
| Total PRD requirements | 7 |
|
||||
| Requirements covered by completed tasks | 7 |
|
||||
| Requirements with incomplete coverage | 0 |
|
||||
|
||||
All functional requirements (REQ-001 through REQ-005) and non-functional requirements (REQ-006, REQ-007) are fully covered by completed tasks.
|
||||
|
||||
## Files Created and Modified
|
||||
|
||||
### Created
|
||||
|
||||
- `tests/test_web/test_app.py`
|
||||
|
||||
### Modified
|
||||
|
||||
- `src/meshcore_hub/api/routes/dashboard.py`
|
||||
- `src/meshcore_hub/api/auth.py`
|
||||
- `src/meshcore_hub/api/metrics.py`
|
||||
- `src/meshcore_hub/common/config.py`
|
||||
- `src/meshcore_hub/web/app.py`
|
||||
- `src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js`
|
||||
- `src/meshcore_hub/web/static/js/spa/pages/admin/members.js`
|
||||
- `tests/test_api/test_dashboard.py`
|
||||
- `tests/test_api/test_auth.py`
|
||||
- `tests/test_common/test_config.py`
|
||||
- `README.md`
|
||||
- `AGENTS.md`
|
||||
- `PLAN.md`
|
||||
|
||||
## Review Rounds
|
||||
|
||||
### Overview
|
||||
|
||||
| Metric | Value |
|
||||
|---|---|
|
||||
| Total review rounds | 1 |
|
||||
| Total issues found | 2 |
|
||||
| Issues fixed | 2 |
|
||||
| Issues deferred | 0 |
|
||||
| Issues remaining | 0 |
|
||||
| Regressions introduced | 0 |
|
||||
|
||||
### Round Details
|
||||
|
||||
#### Round 1 (scope: full)
|
||||
|
||||
- **Issues found:** 2 (0 CRITICAL, 0 MAJOR, 2 MINOR)
|
||||
- **Issues fixed:** 2 (both MINOR issues were addressed post-review)
|
||||
- **Exit reason:** success (no CRITICAL or MAJOR issues)
|
||||
|
||||
## Known Issues and Deferred Items
|
||||
|
||||
No known issues. Both MINOR issues identified in the code review were addressed:
|
||||
|
||||
- **ISSUE-001** (MINOR, integration) -- Startup warning for proxy hosts used `settings.web_admin_enabled` instead of the effective admin_enabled value. Fixed by computing `effective_admin` before the warning check.
|
||||
- **ISSUE-002** (MINOR, style) -- `unsafeHTML()` calls on pre-escaped data lacked explanatory comments. Fixed by adding inline HTML comments explaining that dynamic values are pre-escaped.
|
||||
|
||||
## Decisions
|
||||
|
||||
- **Truthiness guards for `hmac.compare_digest()`** -- Added `read_key and ...` / `admin_key and ...` guards in `require_read` because either key can be `None` when only one is configured, and `hmac.compare_digest()` raises `TypeError` on `None` arguments. This ensures the existing behavior of accepting either key type when configured.
|
||||
- **`unsafeHTML()` retained with `escapeHtml()` pre-processing** -- The `unsafeHTML()` calls in admin JS pages were retained because translation strings contain intentional HTML formatting tags (e.g., `<strong>`). API-sourced data is escaped before interpolation, making this pattern safe.
|
||||
- **`innerHTML` retained for tag delete confirmation** -- The delete confirmation in `node-tags.js` uses `innerHTML` because the translation template includes `<span>` 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`.
|
||||
401
.plans/2026/03/09/01-security-fixes/tasks.yaml
Normal file
401
.plans/2026/03/09/01-security-fixes/tasks.yaml
Normal file
@@ -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 `</script>` 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 `</` sequences:
|
||||
```python
|
||||
config_json = json.dumps(config).replace("</", "<\\/")
|
||||
```
|
||||
|
||||
This prevents a config value containing `</script><script>alert(1)</script>` from breaking out of the
|
||||
`<script>` block in `spa.html` (line ~188: `window.__APP_CONFIG__ = {{ config_json|safe }};`).
|
||||
|
||||
The `|safe` filter in the template remains unchanged — the escaping happens in Python before the value
|
||||
reaches Jinja2. The SPA client-side JavaScript can parse JSON containing `<\/` sequences because this
|
||||
is valid JSON per the spec.
|
||||
requirements:
|
||||
- "REQ-004"
|
||||
- "REQ-006"
|
||||
dependencies: []
|
||||
suggested_role: "python"
|
||||
acceptance_criteria:
|
||||
- "`config_json` is escaped by replacing `</` with `<\\/` before template rendering"
|
||||
- "The `|safe` filter continues to be used in the template"
|
||||
- "A config value containing `</script><script>alert(1)</script>` 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
|
||||
<p class="mb-4">${unsafeHTML(t('common.copy_all_entity_description', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: nodeName }))}</p>
|
||||
```
|
||||
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
|
||||
<p class="mb-4">${unsafeHTML(t('common.delete_all_entity_confirm', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: nodeName }))}</p>
|
||||
```
|
||||
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 `<img src=x onerror=alert(1)>` 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 `<script>alert(1)</script>` 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 `</script><script>alert(1)</script>` 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 `</`
|
||||
if needed, though `json.loads` handles `<\/` fine).
|
||||
|
||||
Test by calling the config JSON builder function directly or by checking the rendered template output.
|
||||
requirements:
|
||||
- "REQ-004"
|
||||
- "REQ-006"
|
||||
dependencies:
|
||||
- "TASK-005"
|
||||
suggested_role: "python"
|
||||
acceptance_criteria:
|
||||
- "Test confirms `</script>` 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"
|
||||
81
.plans/2026/03/17/01-multibyte-support/changelog.md
Normal file
81
.plans/2026/03/17/01-multibyte-support/changelog.md
Normal file
@@ -0,0 +1,81 @@
|
||||
## TASK-001: Verify meshcore_py v2.3.0+ backwards compatibility
|
||||
**Status:** completed
|
||||
### Files Created
|
||||
_(none)_
|
||||
### Files Modified
|
||||
_(none)_
|
||||
### Notes
|
||||
Research-only task. meshcore_py v2.3.0 handles multibyte path hashes transparently at the protocol level. Path hash size is self-describing in the wire format (upper 2 bits of path length byte encode hash size). The interface receiver, sender, and device wrapper pass event payloads through without manipulation, so no code changes are needed. pyproject.toml dependency confirmed at meshcore>=2.3.0.
|
||||
---
|
||||
|
||||
## TASK-002: Update _normalize_hash_list to accept variable-length hex strings
|
||||
**Status:** completed
|
||||
### Files Created
|
||||
_(none)_
|
||||
### Files Modified
|
||||
- `src/meshcore_hub/collector/letsmesh_normalizer.py`
|
||||
### Notes
|
||||
Changed length validation from `if len(token) != 2` to `if len(token) < 2 or len(token) % 2 != 0`. Updated docstring to describe variable-length hex hash support. Existing hex validation and uppercase normalization unchanged. All 98 collector tests pass.
|
||||
---
|
||||
|
||||
## TASK-003: Update Pydantic schema descriptions for path_hashes fields
|
||||
**Status:** completed
|
||||
### Files Created
|
||||
_(none)_
|
||||
### Files Modified
|
||||
- `src/meshcore_hub/common/schemas/events.py`
|
||||
- `src/meshcore_hub/common/schemas/messages.py`
|
||||
- `src/meshcore_hub/common/models/trace_path.py`
|
||||
### Notes
|
||||
Updated TraceDataEvent.path_hashes, TracePathRead.path_hashes, and TracePath model docstring to reflect variable-length hex strings. No Pydantic validators needed changes - both schemas use Optional[list[str]] with no per-element length constraints.
|
||||
---
|
||||
|
||||
## TASK-004: Update SCHEMAS.md documentation for multibyte path hashes
|
||||
**Status:** completed
|
||||
### Files Created
|
||||
_(none)_
|
||||
### Files Modified
|
||||
- `SCHEMAS.md`
|
||||
### Notes
|
||||
Updated path_hashes field description from "2-character" to variable-length hex. Updated example to include mixed-length hashes ["4a", "b3fa", "02"]. Added firmware v1.14 compatibility note.
|
||||
---
|
||||
|
||||
## TASK-008: Verify web dashboard trace path display handles variable-length hashes
|
||||
**Status:** completed
|
||||
### Files Created
|
||||
_(none)_
|
||||
### Files Modified
|
||||
_(none)_
|
||||
### Notes
|
||||
Verification-only task. The web dashboard SPA has no trace path page and no JavaScript/CSS code referencing path_hash or pathHash. Trace path data is only served by the REST API which returns path_hashes as list[str] with no length constraints. No changes needed.
|
||||
---
|
||||
|
||||
## TASK-005: Write tests for multibyte path hash normalizer
|
||||
**Status:** completed
|
||||
### Files Created
|
||||
- `tests/test_collector/test_letsmesh_normalizer.py`
|
||||
### Files Modified
|
||||
- `tests/test_collector/test_subscriber.py`
|
||||
### Notes
|
||||
Created 12 unit tests for _normalize_hash_list covering all 7 required scenarios plus edge cases. Added 2 integration tests to test_subscriber.py verifying multibyte path hashes flow through the full collector pipeline. All 35 collector tests pass.
|
||||
---
|
||||
|
||||
## TASK-006: Write tests for database round-trip of multibyte path hashes
|
||||
**Status:** completed
|
||||
### Files Created
|
||||
_(none)_
|
||||
### Files Modified
|
||||
- `tests/test_common/test_models.py`
|
||||
### Notes
|
||||
Added 2 new test methods to TestTracePathModel: test_multibyte_path_hashes_round_trip and test_mixed_length_path_hashes_round_trip. Verified JSON column handles variable-length strings natively. All 10 model tests pass. No Alembic migration needed.
|
||||
---
|
||||
|
||||
## TASK-007: Write tests for API trace path responses with multibyte hashes
|
||||
**Status:** completed
|
||||
### Files Created
|
||||
_(none)_
|
||||
### Files Modified
|
||||
- `tests/test_api/test_trace_paths.py`
|
||||
### Notes
|
||||
Added TestMultibytePathHashes class with 2 tests: list endpoint with multibyte hashes and detail endpoint with mixed-length hashes. All 9 API trace path tests pass.
|
||||
---
|
||||
146
.plans/2026/03/17/01-multibyte-support/prd.md
Normal file
146
.plans/2026/03/17/01-multibyte-support/prd.md
Normal file
@@ -0,0 +1,146 @@
|
||||
# Product Requirements Document
|
||||
|
||||
> Source: `.plans/2026/03/17/01-multibyte-support/prompt.md`
|
||||
|
||||
## Project Overview
|
||||
|
||||
MeshCore Hub must be updated to support multibyte path hashes introduced in MeshCore firmware v1.14 and the meshcore_py v2.3.0 Python bindings. Path hashes — node identifiers embedded in trace and route data — were previously fixed at 1 byte (2 hex characters) per hop but can now be multiple bytes, allowing longer repeater IDs at the cost of reduced maximum hops. The update must maintain backwards compatibility with nodes running older single-byte firmware.
|
||||
|
||||
## Goals
|
||||
|
||||
- Support variable-length (multibyte) path hashes throughout the data pipeline: interface → MQTT → collector → database → API → web dashboard.
|
||||
- Maintain backwards compatibility so single-byte path hashes from older firmware nodes continue to work without modification.
|
||||
- Update documentation and schemas to accurately describe the new variable-length path hash format.
|
||||
|
||||
## Functional Requirements
|
||||
|
||||
### REQ-001: Accept Variable-Length Path Hashes in Collector
|
||||
|
||||
**Description:** The collector's event handlers and normalizer must accept path hash strings of any even length (not just 2-character strings). Path hashes arriving from both the meshcore_py interface and LetsMesh-compatible ingest must be processed correctly regardless of byte length.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] Path hashes with 2-character values (legacy single-byte) are accepted and stored correctly
|
||||
- [ ] Path hashes with 4+ character values (multibyte) are accepted and stored correctly
|
||||
- [ ] Mixed-length path hash arrays (e.g. `["4a", "b3fa", "02"]`) are accepted when the mesh contains nodes with different firmware versions
|
||||
- [ ] The LetsMesh normalizer handles multibyte `pathHashes` values from decoded payloads
|
||||
|
||||
### REQ-002: Update Pydantic Schema Validation for Path Hashes
|
||||
|
||||
**Description:** The `path_hashes` field in event and message Pydantic schemas currently describes values as "2-character node hash identifiers". The schema description and any validation constraints must be updated to permit variable-length hex strings.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] `TraceDataEvent.path_hashes` field description reflects variable-length hex strings
|
||||
- [ ] `MessageEventBase.path_hashes` field description reflects variable-length hex strings (if applicable)
|
||||
- [ ] No schema validation rejects path hash strings longer than 2 characters
|
||||
|
||||
### REQ-003: Verify Database Storage Compatibility
|
||||
|
||||
**Description:** The `path_hashes` column on the `trace_paths` table uses a JSON column type. Confirm that variable-length path hash strings are stored and retrieved correctly without requiring a schema migration.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] Multibyte path hash arrays are round-tripped correctly through SQLAlchemy JSON column (store and retrieve)
|
||||
- [ ] No Alembic migration is required (JSON column already supports arbitrary string lengths)
|
||||
|
||||
### REQ-004: Update API Responses for Variable-Length Path Hashes
|
||||
|
||||
**Description:** The trace paths API must return multibyte path hashes faithfully. API response schemas and any serialization logic must not truncate or assume a fixed length.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] `GET /trace-paths` returns multibyte path hash arrays as-is from the database
|
||||
- [ ] `GET /trace-paths/{id}` returns multibyte path hash arrays as-is from the database
|
||||
- [ ] API response examples in documentation reflect variable-length path hashes
|
||||
|
||||
### REQ-005: Update Web Dashboard Trace/Path Display
|
||||
|
||||
**Description:** If the web dashboard displays path hashes (e.g. in trace path views), the rendering must handle variable-length strings without layout breakage or truncation.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] Trace path views display multibyte path hashes correctly
|
||||
- [ ] No fixed-width formatting assumes 2-character hash strings
|
||||
|
||||
### REQ-006: Verify meshcore_py Library Compatibility
|
||||
|
||||
**Description:** Confirm that the meshcore_py v2.3.0+ library handles backwards compatibility with single-byte firmware nodes transparently, so that MeshCore Hub does not need to implement compatibility logic itself.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] meshcore_py v2.3.0+ is confirmed to handle mixed single-byte and multibyte path hashes at the protocol level
|
||||
- [ ] The interface receiver and sender components work with the updated library without code changes beyond the dependency version bump (or with minimal changes if the library API changed)
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
### REQ-007: Backwards Compatibility
|
||||
|
||||
**Category:** Reliability
|
||||
|
||||
**Description:** The system must continue to operate correctly when receiving events from nodes running older (single-byte) firmware. No data loss or processing errors may occur for legacy path hash formats.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] Existing test cases with 2-character path hashes continue to pass without modification
|
||||
- [ ] New test cases with multibyte path hashes pass alongside legacy test cases
|
||||
- [ ] No database migration is required that would break rollback to the previous version
|
||||
|
||||
### REQ-008: Documentation Accuracy
|
||||
|
||||
**Category:** Maintainability
|
||||
|
||||
**Description:** All documentation referencing path hash format must be updated to reflect the variable-length nature of multibyte path hashes.
|
||||
|
||||
**Acceptance Criteria:**
|
||||
|
||||
- [ ] `SCHEMAS.md` path hash descriptions updated from "2-character" to "variable-length hex string"
|
||||
- [ ] Code docstrings and field descriptions in models/schemas updated
|
||||
- [ ] Example payloads in documentation include at least one multibyte path hash example
|
||||
|
||||
## Technical Constraints and Assumptions
|
||||
|
||||
### Constraints
|
||||
|
||||
- Python 3.13+ (specified by project)
|
||||
- meshcore_py >= 2.3.0 (already set in `pyproject.toml`)
|
||||
- SQLite with JSON column for path hash storage (existing schema)
|
||||
- No breaking changes to the REST API response format
|
||||
|
||||
### Assumptions
|
||||
|
||||
- The meshcore_py library handles protocol-level backwards compatibility for multibyte path hashes, so MeshCore Hub only needs to ensure its data pipeline accepts variable-length strings
|
||||
- Path hashes are always valid hex strings (even number of characters)
|
||||
- The JSON column type in SQLite/SQLAlchemy does not impose length restrictions on individual array element strings
|
||||
- The `pyproject.toml` dependency has already been bumped to `meshcore>=2.3.0`
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
|
||||
- Updating Pydantic schema descriptions and validation for variable-length path hashes
|
||||
- Updating collector handlers and normalizer for multibyte path hashes
|
||||
- Verifying database storage compatibility (no migration expected)
|
||||
- Verifying API response compatibility
|
||||
- Updating web dashboard path hash display if applicable
|
||||
- Updating `SCHEMAS.md` and code documentation
|
||||
- Adding/updating tests for multibyte path hashes
|
||||
- Confirming meshcore_py library handles backwards compatibility
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- MeshCore firmware changes or device-side configuration
|
||||
- Adding UI controls for selecting single-byte vs. multibyte mode
|
||||
- Performance optimization of path hash processing
|
||||
- Changes to MQTT topic structure or message format
|
||||
- LetsMesh ingest protocol changes (beyond accepting multibyte values that LetsMesh already provides)
|
||||
|
||||
## Suggested Tech Stack
|
||||
|
||||
| Layer | Technology | Rationale |
|
||||
|-------|-----------|-----------|
|
||||
| MeshCore bindings | meshcore_py >= 2.3.0 | Specified by prompt; provides multibyte path hash support |
|
||||
| Validation | Pydantic v2 | Existing stack — schema descriptions updated |
|
||||
| Database | SQLAlchemy 2.0 + SQLite JSON | Existing stack — no migration needed |
|
||||
| API | FastAPI | Existing stack — no changes to framework |
|
||||
| Testing | pytest + pytest-asyncio | Existing stack — new test cases for multibyte |
|
||||
17
.plans/2026/03/17/01-multibyte-support/prompt.md
Normal file
17
.plans/2026/03/17/01-multibyte-support/prompt.md
Normal file
@@ -0,0 +1,17 @@
|
||||
# Phase: 01-multibyte-support
|
||||
|
||||
## Overview
|
||||
|
||||
The latest MeshCore firmware (v1.14) has introduced multibyte support for multi-byte path hashes. The latest version of the MeshCore Python bindings (meshcore_py) has been updated to use this. This allows longer repeater IDs per hop, but reduces the maximum allowed hops. Nodes running older firmware only support 1-byte path hashes and will not receive messages if other nodes use multibyte path hashes.
|
||||
|
||||
## Goals
|
||||
|
||||
* Update Receiver/Sender component to use latest version of MeshCore Python bindings that support multibyte path hash handling.
|
||||
|
||||
## Requirements
|
||||
|
||||
* Must remain backwards compatible with previous version. Confirm whether this is handled by the Python library.
|
||||
|
||||
## References
|
||||
|
||||
* https://github.com/meshcore-dev/meshcore_py/releases/tag/v2.3.0
|
||||
@@ -0,0 +1,19 @@
|
||||
# Code review round 001
|
||||
# Phase: .plans/2026/03/17/01-multibyte-support
|
||||
# Scope: full
|
||||
# Generated by: /jp-codereview
|
||||
|
||||
issues: []
|
||||
|
||||
summary:
|
||||
total_issues: 0
|
||||
critical: 0
|
||||
major: 0
|
||||
minor: 0
|
||||
by_category:
|
||||
integration: 0
|
||||
architecture: 0
|
||||
security: 0
|
||||
duplication: 0
|
||||
error-handling: 0
|
||||
style: 0
|
||||
57
.plans/2026/03/17/01-multibyte-support/reviews/prd.md
Normal file
57
.plans/2026/03/17/01-multibyte-support/reviews/prd.md
Normal file
@@ -0,0 +1,57 @@
|
||||
# PRD Review
|
||||
|
||||
> Phase: `.plans/2026/03/17/01-multibyte-support`
|
||||
> PRD: `.plans/2026/03/17/01-multibyte-support/prd.md`
|
||||
> Prompt: `.plans/2026/03/17/01-multibyte-support/prompt.md`
|
||||
|
||||
## Verdict: PASS
|
||||
|
||||
The PRD comprehensively addresses the narrow scope of the original prompt. All prompt items are covered by specific requirements with testable acceptance criteria. The PRD appropriately expands the prompt's Receiver/Sender focus to cover the full data pipeline (collector, schemas, database, API, web), which is necessary for end-to-end multibyte support. No contradictions, feasibility concerns, or scope inconsistencies were found.
|
||||
|
||||
## Coverage Assessment
|
||||
|
||||
| Prompt Item | PRD Section | Covered? | Notes |
|
||||
|---|---|---|---|
|
||||
| Update Receiver/Sender to use latest meshcore_py with multibyte support | REQ-006 | Yes | Covered by library compatibility verification; receiver/sender work with updated bindings |
|
||||
| Must remain backwards compatible with previous version | REQ-007 | Yes | Explicit non-functional requirement with 3 testable acceptance criteria |
|
||||
| Confirm whether backwards compat is handled by the Python library | REQ-006 | Yes | First AC specifically calls for confirming library-level protocol compatibility |
|
||||
| Reference to meshcore_py v2.3.0 release | Constraints, Tech Stack | Yes | Noted in constraints and suggested tech stack table |
|
||||
|
||||
**Coverage summary:** 4 of 4 prompt items fully covered, 0 partially covered, 0 not covered.
|
||||
|
||||
## Requirement Evaluation
|
||||
|
||||
All requirements passed evaluation. Minor observations:
|
||||
|
||||
### REQ-006: Verify meshcore_py Library Compatibility
|
||||
|
||||
- **Implementability:** Pass
|
||||
- **Testability:** Pass -- though the first AC ("confirmed to handle...at the protocol level") is a verification/research task rather than an automated test, this is appropriate given the prompt explicitly asks to confirm library behavior
|
||||
- **Completeness:** Pass
|
||||
- **Consistency:** Pass
|
||||
|
||||
## Structural Issues
|
||||
|
||||
### Contradictions
|
||||
|
||||
None found.
|
||||
|
||||
### Ambiguities
|
||||
|
||||
None found. The PRD is appropriately specific for the scope of work.
|
||||
|
||||
### Missing Edge Cases
|
||||
|
||||
None significant. The PRD covers the key edge case of mixed-length path hash arrays from heterogeneous firmware networks (REQ-001 AC3).
|
||||
|
||||
### Feasibility Concerns
|
||||
|
||||
None. The changes are primarily documentation/description updates and verification tasks. The JSON column type inherently supports variable-length strings, and the meshcore_py dependency is already bumped.
|
||||
|
||||
### Scope Inconsistencies
|
||||
|
||||
None. The PRD's scope appropriately extends beyond the prompt's Receiver/Sender focus to cover downstream components (collector, API, web) that also handle path hashes. This is a necessary expansion, not scope creep.
|
||||
|
||||
## Action Items
|
||||
|
||||
No action items -- verdict is PASS.
|
||||
89
.plans/2026/03/17/01-multibyte-support/reviews/tasks.md
Normal file
89
.plans/2026/03/17/01-multibyte-support/reviews/tasks.md
Normal file
@@ -0,0 +1,89 @@
|
||||
# Task Review
|
||||
|
||||
> Phase: `.plans/2026/03/17/01-multibyte-support`
|
||||
> Tasks: `.plans/2026/03/17/01-multibyte-support/tasks.yaml`
|
||||
> PRD: `.plans/2026/03/17/01-multibyte-support/prd.md`
|
||||
|
||||
## Verdict: PASS
|
||||
|
||||
The task list is structurally sound, correctly ordered, and fully covers all 8 PRD requirements. The dependency graph is a valid DAG with no cycles or invalid references. No ordering issues were found — no task references files that should be produced by a task outside its dependency chain. All tasks have valid roles, complexity values, and complete fields. The task breakdown is appropriate for the narrow scope of this phase.
|
||||
|
||||
## Dependency Validation
|
||||
|
||||
### Reference Validity
|
||||
|
||||
All dependency references are valid. Every task ID in every `dependencies` list corresponds to an existing task in the inventory.
|
||||
|
||||
### DAG Validation
|
||||
|
||||
The dependency graph is a valid DAG with no cycles. Maximum dependency depth is 1 (two test tasks depend on one implementation task each).
|
||||
|
||||
### Orphan Tasks
|
||||
|
||||
The following tasks are never referenced as dependencies by other tasks:
|
||||
|
||||
- **TASK-001** (Verify meshcore_py compatibility) — terminal verification task, expected
|
||||
- **TASK-004** (Update SCHEMAS.md) — terminal documentation task, expected
|
||||
- **TASK-005** (Tests for normalizer) — terminal test task, expected
|
||||
- **TASK-006** (Tests for DB round-trip) — terminal test task, expected
|
||||
- **TASK-007** (Tests for API responses) — terminal test task, expected
|
||||
- **TASK-008** (Verify web dashboard) — terminal verification task, expected
|
||||
|
||||
All orphan tasks are leaf nodes (tests, docs, or verification tasks). No missing integration points.
|
||||
|
||||
## Ordering Check
|
||||
|
||||
No ordering issues detected. No task modifies a file that is also modified by another task outside its dependency chain. The `files_affected` sets across all tasks are disjoint except where proper dependency relationships exist.
|
||||
|
||||
## Coverage Check
|
||||
|
||||
### Uncovered Requirements
|
||||
|
||||
All PRD requirements are covered.
|
||||
|
||||
### Phantom References
|
||||
|
||||
No phantom references detected. Every requirement ID referenced in tasks exists in the PRD.
|
||||
|
||||
**Coverage summary:** 8 of 8 PRD requirements covered by tasks.
|
||||
|
||||
| Requirement | Covered By |
|
||||
|---|---|
|
||||
| REQ-001 | TASK-002, TASK-005 |
|
||||
| REQ-002 | TASK-003 |
|
||||
| REQ-003 | TASK-006 |
|
||||
| REQ-004 | TASK-007 |
|
||||
| REQ-005 | TASK-008 |
|
||||
| REQ-006 | TASK-001 |
|
||||
| REQ-007 | TASK-005, TASK-006, TASK-007 |
|
||||
| REQ-008 | TASK-004 |
|
||||
|
||||
## Scope Check
|
||||
|
||||
### Tasks Too Large
|
||||
|
||||
No tasks flagged as too large. All tasks are `small` complexity except TASK-005 (`medium`), which is appropriately scoped for a test suite covering 7 unit test scenarios plus an integration test.
|
||||
|
||||
### Tasks Too Vague
|
||||
|
||||
No tasks flagged as too vague. All tasks have detailed descriptions (well over 50 characters), multiple testable acceptance criteria, and specific file paths.
|
||||
|
||||
### Missing Test Tasks
|
||||
|
||||
- **TASK-001** (Verify meshcore_py compatibility) — no associated test task. This is a research/verification task that does not produce source code, so a test task is not applicable. (Warning only)
|
||||
- **TASK-004** (Update SCHEMAS.md) — no associated test task. This is a documentation-only task. (Warning only)
|
||||
- **TASK-008** (Verify web dashboard) — no associated test task. This is a verification task that may result in no code changes. (Warning only)
|
||||
|
||||
All implementation tasks that modify source code (TASK-002, TASK-003) have corresponding test tasks (TASK-005, TASK-006, TASK-007).
|
||||
|
||||
### Field Validation
|
||||
|
||||
All tasks have valid fields:
|
||||
- All `suggested_role` values are valid (`python`, `docs`, `frontend`)
|
||||
- All `estimated_complexity` values are valid (`small`, `medium`)
|
||||
- All tasks have at least one entry in `requirements`, `acceptance_criteria`, and `files_affected`
|
||||
- All task IDs follow the `TASK-NNN` format with sequential numbering
|
||||
|
||||
## Action Items
|
||||
|
||||
No action items — verdict is PASS.
|
||||
18
.plans/2026/03/17/01-multibyte-support/state.yaml
Normal file
18
.plans/2026/03/17/01-multibyte-support/state.yaml
Normal file
@@ -0,0 +1,18 @@
|
||||
status: completed
|
||||
phase_path: .plans/2026/03/17/01-multibyte-support
|
||||
branch: feature/multibyte-support
|
||||
current_phase: completed
|
||||
current_task: null
|
||||
fix_round: 0
|
||||
last_review_round: 1
|
||||
review_loop_exit_reason: success
|
||||
quality_gate: pass
|
||||
tasks:
|
||||
TASK-001: completed
|
||||
TASK-002: completed
|
||||
TASK-003: completed
|
||||
TASK-004: completed
|
||||
TASK-005: completed
|
||||
TASK-006: completed
|
||||
TASK-007: completed
|
||||
TASK-008: completed
|
||||
102
.plans/2026/03/17/01-multibyte-support/summary.md
Normal file
102
.plans/2026/03/17/01-multibyte-support/summary.md
Normal file
@@ -0,0 +1,102 @@
|
||||
# Phase Summary
|
||||
|
||||
> Phase: `.plans/2026/03/17/01-multibyte-support`
|
||||
> Generated by: `/jp-summary`
|
||||
|
||||
## Project Overview
|
||||
|
||||
MeshCore Hub was updated to support multibyte path hashes introduced in MeshCore firmware v1.14 and meshcore_py v2.3.0. Path hashes — node identifiers embedded in trace and route data — were previously fixed at 1 byte (2 hex characters) per hop but can now be multiple bytes. The update maintains backwards compatibility with nodes running older single-byte firmware.
|
||||
|
||||
### Goals
|
||||
|
||||
- Support variable-length (multibyte) path hashes throughout the data pipeline: interface → MQTT → collector → database → API → web dashboard.
|
||||
- Maintain backwards compatibility so single-byte path hashes from older firmware nodes continue to work without modification.
|
||||
- Update documentation and schemas to accurately describe the new variable-length path hash format.
|
||||
|
||||
## Task Execution
|
||||
|
||||
### Overview
|
||||
|
||||
| Metric | Value |
|
||||
|---|---|
|
||||
| Total tasks | 8 |
|
||||
| Completed | 8 |
|
||||
| Failed | 0 |
|
||||
| Blocked | 0 |
|
||||
| Skipped | 0 |
|
||||
|
||||
### Task Details
|
||||
|
||||
| ID | Title | Role | Complexity | Status |
|
||||
|---|---|---|---|---|
|
||||
| TASK-001 | Verify meshcore_py v2.3.0+ backwards compatibility | python | small | completed |
|
||||
| TASK-002 | Update _normalize_hash_list to accept variable-length hex strings | python | small | completed |
|
||||
| TASK-003 | Update Pydantic schema descriptions for path_hashes fields | python | small | completed |
|
||||
| TASK-004 | Update SCHEMAS.md documentation for multibyte path hashes | docs | small | completed |
|
||||
| TASK-005 | Write tests for multibyte path hash normalizer | python | medium | completed |
|
||||
| TASK-006 | Write tests for database round-trip of multibyte path hashes | python | small | completed |
|
||||
| TASK-007 | Write tests for API trace path responses with multibyte hashes | python | small | completed |
|
||||
| TASK-008 | Verify web dashboard trace path display handles variable-length hashes | frontend | small | completed |
|
||||
|
||||
### Requirement Coverage
|
||||
|
||||
| Metric | Value |
|
||||
|---|---|
|
||||
| Total PRD requirements | 8 |
|
||||
| Requirements covered by completed tasks | 8 |
|
||||
| Requirements with incomplete coverage | 0 |
|
||||
|
||||
## Files Created and Modified
|
||||
|
||||
### Created
|
||||
|
||||
- `tests/test_collector/test_letsmesh_normalizer.py`
|
||||
|
||||
### Modified
|
||||
|
||||
- `pyproject.toml`
|
||||
- `SCHEMAS.md`
|
||||
- `src/meshcore_hub/collector/letsmesh_normalizer.py`
|
||||
- `src/meshcore_hub/common/schemas/events.py`
|
||||
- `src/meshcore_hub/common/schemas/messages.py`
|
||||
- `src/meshcore_hub/common/models/trace_path.py`
|
||||
- `tests/test_collector/test_subscriber.py`
|
||||
- `tests/test_common/test_models.py`
|
||||
- `tests/test_api/test_trace_paths.py`
|
||||
|
||||
## Review Rounds
|
||||
|
||||
### Overview
|
||||
|
||||
| Metric | Value |
|
||||
|---|---|
|
||||
| Total review rounds | 1 |
|
||||
| Total issues found | 0 |
|
||||
| Issues fixed | 0 |
|
||||
| Issues deferred | 0 |
|
||||
| Issues remaining | 0 |
|
||||
| Regressions introduced | 0 |
|
||||
|
||||
### Round Details
|
||||
|
||||
#### Round 1 (scope: full)
|
||||
|
||||
- **Issues found:** 0 (0 CRITICAL, 0 MAJOR, 0 MINOR)
|
||||
- **Exit reason:** success (clean review, no fix rounds needed)
|
||||
|
||||
## Known Issues and Deferred Items
|
||||
|
||||
No known issues.
|
||||
|
||||
## Decisions
|
||||
|
||||
- **meshcore_py handles backwards compatibility transparently** -- Research (TASK-001) confirmed that meshcore_py v2.3.0 handles multibyte path hashes at the protocol level via self-describing wire format. No compatibility logic needed in MeshCore Hub's interface layer.
|
||||
- **No database migration required** -- The existing JSON column type on `trace_paths.path_hashes` stores variable-length string arrays natively. Round-trip tests confirmed no data loss.
|
||||
- **No web dashboard changes needed** -- The SPA has no trace path rendering page. Path hashes are only served via the REST API which uses `list[str]` with no length constraints.
|
||||
- **Normalizer validation approach** -- Changed from exact length check (`len == 2`) to even-length minimum-2 check (`len >= 2 and len % 2 == 0`), preserving existing hex validation and uppercase normalization.
|
||||
|
||||
## Suggested Next Steps
|
||||
|
||||
1. Push the branch and create a pull request for review.
|
||||
2. Perform manual integration testing with a MeshCore device running firmware v1.14+ to verify multibyte path hashes flow end-to-end.
|
||||
3. Verify that mixed-firmware networks (some nodes v1.14+, some older) produce correct mixed-length path hash arrays in the database.
|
||||
274
.plans/2026/03/17/01-multibyte-support/tasks.yaml
Normal file
274
.plans/2026/03/17/01-multibyte-support/tasks.yaml
Normal file
@@ -0,0 +1,274 @@
|
||||
# Task list generated from PRD: .plans/2026/03/17/01-multibyte-support/prd.md
|
||||
# Generated by: /jp-task-list
|
||||
|
||||
tasks:
|
||||
- id: "TASK-001"
|
||||
title: "Verify meshcore_py v2.3.0+ backwards compatibility"
|
||||
description: |
|
||||
Research and confirm that meshcore_py v2.3.0+ handles backwards compatibility
|
||||
with single-byte firmware nodes at the protocol level. Check the meshcore_py
|
||||
v2.3.0 release notes and source code to determine whether the library
|
||||
transparently handles mixed single-byte and multibyte path hashes, or whether
|
||||
MeshCore Hub needs to implement any compatibility logic.
|
||||
|
||||
The pyproject.toml dependency is already set to meshcore>=2.3.0. Verify the
|
||||
interface receiver (src/meshcore_hub/interface/receiver.py) and sender
|
||||
(src/meshcore_hub/interface/sender.py) components work with the updated library
|
||||
without code changes, or document any API changes that require updates.
|
||||
|
||||
Document findings as a comment block at the top of the PR description or in
|
||||
the phase changelog.
|
||||
requirements:
|
||||
- "REQ-006"
|
||||
dependencies: []
|
||||
suggested_role: "python"
|
||||
acceptance_criteria:
|
||||
- "meshcore_py v2.3.0+ backwards compatibility behaviour is documented"
|
||||
- "Any required interface code changes are identified (or confirmed unnecessary)"
|
||||
- "pyproject.toml dependency version is confirmed correct at >=2.3.0"
|
||||
estimated_complexity: "small"
|
||||
files_affected:
|
||||
- "pyproject.toml"
|
||||
|
||||
- id: "TASK-002"
|
||||
title: "Update _normalize_hash_list to accept variable-length hex strings"
|
||||
description: |
|
||||
The LetsMesh normalizer method `_normalize_hash_list` in
|
||||
src/meshcore_hub/collector/letsmesh_normalizer.py (line ~724) currently rejects
|
||||
any path hash string that is not exactly 2 characters long:
|
||||
|
||||
if len(token) != 2:
|
||||
continue
|
||||
|
||||
Update this method to accept variable-length hex strings (any even-length hex
|
||||
string of 2+ characters). The validation should:
|
||||
- Accept strings of length 2, 4, 6, etc. (even-length, minimum 2)
|
||||
- Reject odd-length strings and empty strings
|
||||
- Continue to validate that all characters are valid hexadecimal (0-9, A-F)
|
||||
- Continue to uppercase-normalize the hex strings
|
||||
|
||||
Also update the method's docstring from "Normalize a list of one-byte hash
|
||||
strings" to reflect variable-length support.
|
||||
requirements:
|
||||
- "REQ-001"
|
||||
dependencies: []
|
||||
suggested_role: "python"
|
||||
acceptance_criteria:
|
||||
- "_normalize_hash_list accepts 2-character hex strings (legacy single-byte)"
|
||||
- "_normalize_hash_list accepts 4+ character hex strings (multibyte)"
|
||||
- "_normalize_hash_list rejects odd-length strings"
|
||||
- "_normalize_hash_list rejects non-hex characters"
|
||||
- "_normalize_hash_list uppercases all hex strings"
|
||||
- "Method docstring updated to describe variable-length support"
|
||||
estimated_complexity: "small"
|
||||
files_affected:
|
||||
- "src/meshcore_hub/collector/letsmesh_normalizer.py"
|
||||
|
||||
- id: "TASK-003"
|
||||
done: true
|
||||
title: "Update Pydantic schema descriptions for path_hashes fields"
|
||||
description: |
|
||||
Update the `path_hashes` field description in Pydantic schemas to reflect
|
||||
variable-length hex strings instead of fixed 2-character strings.
|
||||
|
||||
Files and fields to update:
|
||||
|
||||
1. src/meshcore_hub/common/schemas/events.py - TraceDataEvent.path_hashes
|
||||
(line ~134): Change description from "Array of 2-character node hash
|
||||
identifiers" to "Array of hex-encoded node hash identifiers (variable
|
||||
length, e.g. '4a' for single-byte or 'b3fa' for multibyte)"
|
||||
|
||||
2. src/meshcore_hub/common/schemas/messages.py - MessageEventBase.path_hashes
|
||||
or TracePathRead.path_hashes (line ~157): Update description similarly
|
||||
if it references fixed-length hashes.
|
||||
|
||||
3. src/meshcore_hub/common/models/trace_path.py - TracePath.path_hashes
|
||||
docstring (line ~23): Change "JSON array of node hash identifiers" to
|
||||
"JSON array of hex-encoded node hash identifiers (variable length)"
|
||||
|
||||
Ensure no Pydantic validators or Field constraints reject strings longer
|
||||
than 2 characters. The current schemas use Optional[list[str]] with no
|
||||
per-element length validation, so no validator changes should be needed.
|
||||
requirements:
|
||||
- "REQ-002"
|
||||
dependencies: []
|
||||
suggested_role: "python"
|
||||
acceptance_criteria:
|
||||
- "TraceDataEvent.path_hashes description reflects variable-length hex strings"
|
||||
- "TracePathRead.path_hashes description reflects variable-length hex strings"
|
||||
- "TracePath model docstring updated for variable-length path hashes"
|
||||
- "No Pydantic validation rejects path hash strings longer than 2 characters"
|
||||
estimated_complexity: "small"
|
||||
files_affected:
|
||||
- "src/meshcore_hub/common/schemas/events.py"
|
||||
- "src/meshcore_hub/common/schemas/messages.py"
|
||||
- "src/meshcore_hub/common/models/trace_path.py"
|
||||
|
||||
- id: "TASK-004"
|
||||
title: "Update SCHEMAS.md documentation for multibyte path hashes"
|
||||
description: |
|
||||
Update SCHEMAS.md to reflect the new variable-length path hash format
|
||||
introduced in MeshCore firmware v1.14.
|
||||
|
||||
Changes needed:
|
||||
|
||||
1. Line ~228: Change "Array of 2-character node hash identifiers (ordered
|
||||
by hops)" to "Array of hex-encoded node hash identifiers, variable length
|
||||
(e.g. '4a' for single-byte, 'b3fa' for multibyte), ordered by hops"
|
||||
|
||||
2. Line ~239: Update the example path_hashes array to include at least one
|
||||
multibyte hash, e.g.:
|
||||
"path_hashes": ["4a", "b3fa", "02"]
|
||||
This demonstrates mixed single-byte and multibyte hashes in the same trace.
|
||||
|
||||
3. Add a brief note explaining that firmware v1.14+ supports multibyte path
|
||||
hashes and that older nodes use single-byte (2-character) hashes, so
|
||||
mixed-length arrays are expected in heterogeneous networks.
|
||||
requirements:
|
||||
- "REQ-008"
|
||||
dependencies: []
|
||||
suggested_role: "docs"
|
||||
acceptance_criteria:
|
||||
- "path_hashes field description updated from '2-character' to 'variable-length hex'"
|
||||
- "Example payload includes at least one multibyte path hash"
|
||||
- "Note about firmware version compatibility is present"
|
||||
estimated_complexity: "small"
|
||||
files_affected:
|
||||
- "SCHEMAS.md"
|
||||
|
||||
- id: "TASK-005"
|
||||
done: true
|
||||
title: "Write tests for multibyte path hash normalizer"
|
||||
description: |
|
||||
Add tests for the updated _normalize_hash_list method in the LetsMesh
|
||||
normalizer to verify it handles variable-length hex strings correctly.
|
||||
|
||||
Add test cases in tests/test_collector/ (either in an existing normalizer
|
||||
test file or a new test_letsmesh_normalizer.py if one doesn't exist):
|
||||
|
||||
1. Single-byte (2-char) hashes: ["4a", "b3", "fa"] -> accepted, uppercased
|
||||
2. Multibyte (4-char) hashes: ["4a2b", "b3fa"] -> accepted, uppercased
|
||||
3. Mixed-length hashes: ["4a", "b3fa", "02"] -> all accepted
|
||||
4. Odd-length strings: ["4a", "b3f", "02"] -> "b3f" filtered out
|
||||
5. Invalid hex characters: ["4a", "zz", "02"] -> "zz" filtered out
|
||||
6. Empty list: [] -> returns None
|
||||
7. Non-string items: [42, "4a"] -> 42 filtered out
|
||||
|
||||
Also add/update integration-level tests in tests/test_collector/test_subscriber.py
|
||||
to verify that multibyte path hashes flow through the full collector pipeline
|
||||
(subscriber -> handler -> database) correctly. The existing test cases at
|
||||
lines ~607 and ~662 use 2-character hashes; add a parallel test case with
|
||||
multibyte hashes.
|
||||
requirements:
|
||||
- "REQ-001"
|
||||
- "REQ-007"
|
||||
dependencies:
|
||||
- "TASK-002"
|
||||
suggested_role: "python"
|
||||
acceptance_criteria:
|
||||
- "Unit tests for _normalize_hash_list cover all 7 scenarios listed"
|
||||
- "Integration test verifies multibyte path hashes stored correctly in database"
|
||||
- "All existing 2-character path hash tests continue to pass"
|
||||
- "All new tests pass"
|
||||
estimated_complexity: "medium"
|
||||
files_affected:
|
||||
- "tests/test_collector/test_letsmesh_normalizer.py"
|
||||
- "tests/test_collector/test_subscriber.py"
|
||||
|
||||
- id: "TASK-006"
|
||||
title: "Write tests for database round-trip of multibyte path hashes"
|
||||
description: |
|
||||
Verify that the SQLAlchemy JSON column on the TracePath model correctly
|
||||
stores and retrieves variable-length path hash arrays without data loss
|
||||
or truncation.
|
||||
|
||||
Add test cases in tests/test_common/test_models.py (where existing
|
||||
TracePath tests are at line ~129):
|
||||
|
||||
1. Store and retrieve a TracePath with multibyte path_hashes:
|
||||
["4a2b", "b3fa", "02cd"] -> verify round-trip equality
|
||||
2. Store and retrieve a TracePath with mixed-length path_hashes:
|
||||
["4a", "b3fa", "02"] -> verify round-trip equality
|
||||
3. Verify existing test with 2-character hashes still passes
|
||||
|
||||
These tests confirm REQ-003 (no migration needed) and contribute to
|
||||
REQ-007 (backwards compatibility).
|
||||
requirements:
|
||||
- "REQ-003"
|
||||
- "REQ-007"
|
||||
dependencies:
|
||||
- "TASK-003"
|
||||
suggested_role: "python"
|
||||
acceptance_criteria:
|
||||
- "Test verifies multibyte path_hashes round-trip through JSON column correctly"
|
||||
- "Test verifies mixed-length path_hashes round-trip correctly"
|
||||
- "Existing 2-character path hash test continues to pass"
|
||||
- "No Alembic migration is created or required"
|
||||
estimated_complexity: "small"
|
||||
files_affected:
|
||||
- "tests/test_common/test_models.py"
|
||||
|
||||
- id: "TASK-007"
|
||||
title: "Write tests for API trace path responses with multibyte hashes"
|
||||
description: |
|
||||
Add test cases in tests/test_api/test_trace_paths.py to verify that the
|
||||
trace paths API returns multibyte path hashes faithfully.
|
||||
|
||||
The existing test fixtures in tests/test_api/conftest.py create
|
||||
sample_trace_path objects with path_hashes like ["abc123", "def456",
|
||||
"ghi789"] (line ~275). Note these are already 6-character strings, so
|
||||
the API serialization likely already works. Add explicit test cases:
|
||||
|
||||
1. Create a trace path with multibyte path_hashes (e.g. ["4a2b", "b3fa"])
|
||||
via the fixture, then GET /trace-paths and verify the response contains
|
||||
the exact same array.
|
||||
2. Create a trace path with mixed-length path_hashes (e.g. ["4a", "b3fa",
|
||||
"02"]), then GET /trace-paths/{id} and verify the response.
|
||||
3. Verify existing API tests with current path_hashes continue to pass.
|
||||
|
||||
These tests confirm REQ-004.
|
||||
requirements:
|
||||
- "REQ-004"
|
||||
- "REQ-007"
|
||||
dependencies:
|
||||
- "TASK-003"
|
||||
suggested_role: "python"
|
||||
acceptance_criteria:
|
||||
- "Test verifies GET /trace-paths returns multibyte path hashes correctly"
|
||||
- "Test verifies GET /trace-paths/{id} returns mixed-length path hashes correctly"
|
||||
- "Existing API trace path tests continue to pass"
|
||||
estimated_complexity: "small"
|
||||
files_affected:
|
||||
- "tests/test_api/test_trace_paths.py"
|
||||
- "tests/test_api/conftest.py"
|
||||
|
||||
- id: "TASK-008"
|
||||
done: true
|
||||
title: "Verify web dashboard trace path display handles variable-length hashes"
|
||||
description: |
|
||||
Verify that the web dashboard does not have any hardcoded assumptions about
|
||||
2-character path hash strings. A grep of src/meshcore_hub/web/static/js/spa/
|
||||
for "path_hash" and "trace" shows no direct references to path hashes in the
|
||||
SPA JavaScript code, meaning path hashes are likely rendered generically
|
||||
through the API data display.
|
||||
|
||||
Confirm this by:
|
||||
1. Checking all web template and JavaScript files that render trace path data
|
||||
2. Verifying no CSS or JS applies fixed-width formatting to path hash elements
|
||||
3. If any fixed-width or truncation logic exists, update it to handle
|
||||
variable-length strings
|
||||
|
||||
If no web code references path hashes directly (as initial grep suggests),
|
||||
document that the web dashboard requires no changes for multibyte support.
|
||||
This satisfies REQ-005.
|
||||
requirements:
|
||||
- "REQ-005"
|
||||
dependencies: []
|
||||
suggested_role: "frontend"
|
||||
acceptance_criteria:
|
||||
- "Web dashboard trace/path display verified to handle variable-length hashes"
|
||||
- "No fixed-width formatting assumes 2-character hash strings"
|
||||
- "Any necessary changes applied, or no-change finding documented"
|
||||
estimated_complexity: "small"
|
||||
files_affected:
|
||||
- "src/meshcore_hub/web/static/js/spa/pages/trace-paths.js"
|
||||
@@ -612,6 +612,7 @@ Key variables:
|
||||
- `MQTT_TLS` - Enable TLS/SSL for MQTT (default: `false`)
|
||||
- `API_READ_KEY`, `API_ADMIN_KEY` - API authentication keys
|
||||
- `WEB_ADMIN_ENABLED` - Enable admin interface at /a/ (default: `false`, requires auth proxy)
|
||||
- `WEB_TRUSTED_PROXY_HOSTS` - Comma-separated list of trusted proxy hosts for admin authentication headers. Default: `*` (all hosts). Recommended: set to your reverse proxy IP in production. A startup warning is emitted when using the default `*` with admin enabled.
|
||||
- `WEB_THEME` - Default theme for the web dashboard (default: `dark`, options: `dark`, `light`). Users can override via the theme toggle in the navbar, which persists their preference in browser localStorage.
|
||||
- `WEB_AUTO_REFRESH_SECONDS` - Auto-refresh interval in seconds for list pages (default: `30`, `0` to disable)
|
||||
- `TZ` - Timezone for web dashboard date/time display (default: `UTC`, e.g., `America/New_York`, `Europe/London`)
|
||||
|
||||
1
PLAN.md
1
PLAN.md
@@ -516,6 +516,7 @@ LetsMesh compatibility parity note:
|
||||
| WEB_PORT | 8080 | Web bind port |
|
||||
| API_BASE_URL | http://localhost:8000 | API endpoint |
|
||||
| API_KEY | | API key for queries |
|
||||
| WEB_TRUSTED_PROXY_HOSTS | * | Comma-separated list of trusted proxy hosts for admin authentication headers. Default: `*` (all hosts). Recommended: set to your reverse proxy IP in production. |
|
||||
| WEB_LOCALE | en | UI translation locale |
|
||||
| WEB_DATETIME_LOCALE | en-US | Date formatting locale for UI timestamps |
|
||||
| TZ | UTC | Timezone used for UI timestamp rendering |
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
[](https://github.com/ipnet-mesh/meshcore-hub/actions/workflows/ci.yml)
|
||||
[](https://github.com/ipnet-mesh/meshcore-hub/actions/workflows/docker.yml)
|
||||
[](https://codecov.io/github/ipnet-mesh/meshcore-hub)
|
||||
[](https://www.buymeacoffee.com/jinglemansweep)
|
||||
|
||||
Python 3.13+ platform for managing and orchestrating MeshCore mesh networks.
|
||||
@@ -394,6 +395,7 @@ The collector automatically cleans up old event data and inactive nodes:
|
||||
| `WEB_DATETIME_LOCALE` | `en-US` | Locale used for date formatting in the web dashboard (e.g., `en-US` for MM/DD/YYYY, `en-GB` for DD/MM/YYYY). |
|
||||
| `WEB_AUTO_REFRESH_SECONDS` | `30` | Auto-refresh interval in seconds for list pages (0 to disable) |
|
||||
| `WEB_ADMIN_ENABLED` | `false` | Enable admin interface at /a/ (requires auth proxy: `X-Forwarded-User`/`X-Auth-Request-User` or forwarded `Authorization: Basic ...`) |
|
||||
| `WEB_TRUSTED_PROXY_HOSTS` | `*` | Comma-separated list of trusted proxy hosts for admin authentication headers. Default: `*` (all hosts). Recommended: set to your reverse proxy IP in production. A startup warning is emitted when using the default `*` with admin enabled. |
|
||||
| `TZ` | `UTC` | Timezone for displaying dates/times (e.g., `America/New_York`, `Europe/London`) |
|
||||
| `NETWORK_DOMAIN` | *(none)* | Network domain name (optional) |
|
||||
| `NETWORK_NAME` | `MeshCore Network` | Display name for the network |
|
||||
|
||||
@@ -225,7 +225,7 @@ Network trace path results showing route and signal strength.
|
||||
- `path_len`: Length of the path
|
||||
- `flags`: Trace flags/options
|
||||
- `auth`: Authentication/validation data
|
||||
- `path_hashes`: Array of 2-character node hash identifiers (ordered by hops)
|
||||
- `path_hashes`: Array of hex-encoded node hash identifiers, variable length (e.g., `"4a"` for single-byte, `"b3fa"` for multibyte), ordered by hops
|
||||
- `snr_values`: Array of SNR values corresponding to each hop
|
||||
- `hop_count`: Total number of hops
|
||||
|
||||
@@ -236,12 +236,14 @@ Network trace path results showing route and signal strength.
|
||||
"path_len": 3,
|
||||
"flags": 0,
|
||||
"auth": 1,
|
||||
"path_hashes": ["4a", "b3", "fa"],
|
||||
"path_hashes": ["4a", "b3fa", "02"],
|
||||
"snr_values": [25.3, 18.7, 12.4],
|
||||
"hop_count": 3
|
||||
}
|
||||
```
|
||||
|
||||
**Note**: MeshCore firmware v1.14+ supports multibyte path hashes. Older nodes use single-byte (2-character) hashes. Mixed-length hash arrays are expected in heterogeneous networks where nodes run different firmware versions.
|
||||
|
||||
**Webhook Trigger**: No
|
||||
**REST API**: `GET /api/v1/trace-paths`
|
||||
|
||||
|
||||
@@ -37,7 +37,7 @@ dependencies = [
|
||||
"python-multipart>=0.0.6",
|
||||
"httpx>=0.25.0",
|
||||
"aiosqlite>=0.19.0",
|
||||
"meshcore>=2.2.0",
|
||||
"meshcore>=2.3.0",
|
||||
"pyyaml>=6.0.0",
|
||||
"python-frontmatter>=1.0.0",
|
||||
"markdown>=3.5.0",
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
"""Authentication middleware for the API."""
|
||||
|
||||
import hmac
|
||||
import logging
|
||||
from typing import Annotated
|
||||
|
||||
@@ -79,7 +80,9 @@ async def require_read(
|
||||
)
|
||||
|
||||
# Check if token matches any key
|
||||
if token == read_key or token == admin_key:
|
||||
if (read_key and hmac.compare_digest(token, read_key)) or (
|
||||
admin_key and hmac.compare_digest(token, admin_key)
|
||||
):
|
||||
return token
|
||||
|
||||
raise HTTPException(
|
||||
@@ -124,7 +127,7 @@ async def require_admin(
|
||||
)
|
||||
|
||||
# Check if token matches admin key
|
||||
if token == admin_key:
|
||||
if hmac.compare_digest(token, admin_key):
|
||||
return token
|
||||
|
||||
raise HTTPException(
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""Prometheus metrics endpoint for MeshCore Hub API."""
|
||||
|
||||
import base64
|
||||
import hmac
|
||||
import logging
|
||||
import time
|
||||
from typing import Any
|
||||
@@ -54,7 +55,9 @@ def verify_basic_auth(request: Request) -> bool:
|
||||
try:
|
||||
decoded = base64.b64decode(auth_header[6:]).decode("utf-8")
|
||||
username, password = decoded.split(":", 1)
|
||||
return username == "metrics" and password == read_key
|
||||
return hmac.compare_digest(username, "metrics") and hmac.compare_digest(
|
||||
password, read_key
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
@@ -2,8 +2,7 @@
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from fastapi import APIRouter, Request
|
||||
from fastapi.responses import HTMLResponse
|
||||
from fastapi import APIRouter
|
||||
from sqlalchemy import func, select
|
||||
|
||||
from meshcore_hub.api.auth import RequireRead
|
||||
@@ -362,175 +361,3 @@ async def get_node_count_history(
|
||||
data.append(DailyActivityPoint(date=date_str, count=count))
|
||||
|
||||
return NodeCountHistory(days=days, data=data)
|
||||
|
||||
|
||||
@router.get("/", response_class=HTMLResponse)
|
||||
async def dashboard(
|
||||
request: Request,
|
||||
session: DbSession,
|
||||
) -> HTMLResponse:
|
||||
"""Simple HTML dashboard page."""
|
||||
now = datetime.now(timezone.utc)
|
||||
today_start = now.replace(hour=0, minute=0, second=0, microsecond=0)
|
||||
yesterday = now - timedelta(days=1)
|
||||
|
||||
# Get stats
|
||||
total_nodes = session.execute(select(func.count()).select_from(Node)).scalar() or 0
|
||||
|
||||
active_nodes = (
|
||||
session.execute(
|
||||
select(func.count()).select_from(Node).where(Node.last_seen >= yesterday)
|
||||
).scalar()
|
||||
or 0
|
||||
)
|
||||
|
||||
total_messages = (
|
||||
session.execute(select(func.count()).select_from(Message)).scalar() or 0
|
||||
)
|
||||
|
||||
messages_today = (
|
||||
session.execute(
|
||||
select(func.count())
|
||||
.select_from(Message)
|
||||
.where(Message.received_at >= today_start)
|
||||
).scalar()
|
||||
or 0
|
||||
)
|
||||
|
||||
# Get recent nodes
|
||||
recent_nodes = (
|
||||
session.execute(select(Node).order_by(Node.last_seen.desc()).limit(10))
|
||||
.scalars()
|
||||
.all()
|
||||
)
|
||||
|
||||
# Get recent messages
|
||||
recent_messages = (
|
||||
session.execute(select(Message).order_by(Message.received_at.desc()).limit(10))
|
||||
.scalars()
|
||||
.all()
|
||||
)
|
||||
|
||||
# Build HTML
|
||||
html = f"""
|
||||
<!DOCTYPE html>
|
||||
<html>
|
||||
<head>
|
||||
<title>MeshCore Hub Dashboard</title>
|
||||
<meta charset="utf-8">
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||
<meta http-equiv="refresh" content="30">
|
||||
<style>
|
||||
body {{
|
||||
font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif;
|
||||
margin: 0;
|
||||
padding: 20px;
|
||||
background: #f5f5f5;
|
||||
color: #333;
|
||||
}}
|
||||
h1 {{ color: #2c3e50; }}
|
||||
.container {{ max-width: 1200px; margin: 0 auto; }}
|
||||
.stats {{
|
||||
display: grid;
|
||||
grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
|
||||
gap: 20px;
|
||||
margin-bottom: 30px;
|
||||
}}
|
||||
.stat-card {{
|
||||
background: white;
|
||||
padding: 20px;
|
||||
border-radius: 8px;
|
||||
box-shadow: 0 2px 4px rgba(0,0,0,0.1);
|
||||
}}
|
||||
.stat-card h3 {{ margin: 0 0 10px 0; color: #666; font-size: 14px; }}
|
||||
.stat-card .value {{ font-size: 32px; font-weight: bold; color: #2c3e50; }}
|
||||
.section {{
|
||||
background: white;
|
||||
padding: 20px;
|
||||
border-radius: 8px;
|
||||
box-shadow: 0 2px 4px rgba(0,0,0,0.1);
|
||||
margin-bottom: 20px;
|
||||
}}
|
||||
table {{ width: 100%; border-collapse: collapse; }}
|
||||
th, td {{ padding: 10px; text-align: left; border-bottom: 1px solid #eee; }}
|
||||
th {{ background: #f8f9fa; font-weight: 600; }}
|
||||
.text-muted {{ color: #666; }}
|
||||
.truncate {{ max-width: 200px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; }}
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<div class="container">
|
||||
<h1>MeshCore Hub Dashboard</h1>
|
||||
<p class="text-muted">Last updated: {now.strftime('%Y-%m-%d %H:%M:%S UTC')}</p>
|
||||
|
||||
<div class="stats">
|
||||
<div class="stat-card">
|
||||
<h3>Total Nodes</h3>
|
||||
<div class="value">{total_nodes}</div>
|
||||
</div>
|
||||
<div class="stat-card">
|
||||
<h3>Active Nodes (24h)</h3>
|
||||
<div class="value">{active_nodes}</div>
|
||||
</div>
|
||||
<div class="stat-card">
|
||||
<h3>Total Messages</h3>
|
||||
<div class="value">{total_messages}</div>
|
||||
</div>
|
||||
<div class="stat-card">
|
||||
<h3>Messages Today</h3>
|
||||
<div class="value">{messages_today}</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="section">
|
||||
<h2>Recent Nodes</h2>
|
||||
<table>
|
||||
<thead>
|
||||
<tr>
|
||||
<th>Name</th>
|
||||
<th>Public Key</th>
|
||||
<th>Type</th>
|
||||
<th>Last Seen</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
{"".join(f'''
|
||||
<tr>
|
||||
<td>{n.name or '-'}</td>
|
||||
<td class="truncate">{n.public_key[:16]}...</td>
|
||||
<td>{n.adv_type or '-'}</td>
|
||||
<td>{n.last_seen.strftime('%Y-%m-%d %H:%M') if n.last_seen else '-'}</td>
|
||||
</tr>
|
||||
''' for n in recent_nodes)}
|
||||
</tbody>
|
||||
</table>
|
||||
</div>
|
||||
|
||||
<div class="section">
|
||||
<h2>Recent Messages</h2>
|
||||
<table>
|
||||
<thead>
|
||||
<tr>
|
||||
<th>Type</th>
|
||||
<th>From/Channel</th>
|
||||
<th>Text</th>
|
||||
<th>Received</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
{"".join(f'''
|
||||
<tr>
|
||||
<td>{m.message_type}</td>
|
||||
<td>{m.pubkey_prefix or f'Ch {m.channel_idx}' or '-'}</td>
|
||||
<td class="truncate">{m.text[:50]}{'...' if len(m.text) > 50 else ''}</td>
|
||||
<td>{m.received_at.strftime('%Y-%m-%d %H:%M') if m.received_at else '-'}</td>
|
||||
</tr>
|
||||
''' for m in recent_messages)}
|
||||
</tbody>
|
||||
</table>
|
||||
</div>
|
||||
</div>
|
||||
</body>
|
||||
</html>
|
||||
"""
|
||||
return HTMLResponse(content=html)
|
||||
|
||||
@@ -722,7 +722,12 @@ class LetsMeshNormalizer:
|
||||
|
||||
@staticmethod
|
||||
def _normalize_hash_list(value: Any) -> list[str] | None:
|
||||
"""Normalize a list of one-byte hash strings."""
|
||||
"""Normalize a list of variable-length hex hash strings.
|
||||
|
||||
Accepts even-length hex strings of 2 or more characters.
|
||||
Each string is uppercased and validated as hexadecimal.
|
||||
Odd-length strings, empty strings, and non-hex strings are skipped.
|
||||
"""
|
||||
if not isinstance(value, list):
|
||||
return None
|
||||
|
||||
@@ -731,7 +736,7 @@ class LetsMeshNormalizer:
|
||||
if not isinstance(item, str):
|
||||
continue
|
||||
token = item.strip().upper()
|
||||
if len(token) != 2:
|
||||
if len(token) < 2 or len(token) % 2 != 0:
|
||||
continue
|
||||
if any(ch not in "0123456789ABCDEF" for ch in token):
|
||||
continue
|
||||
|
||||
@@ -352,6 +352,12 @@ class WebSettings(CommonSettings):
|
||||
ge=0,
|
||||
)
|
||||
|
||||
# Trusted proxy hosts for X-Forwarded-For header processing
|
||||
web_trusted_proxy_hosts: str = Field(
|
||||
default="*",
|
||||
description="Comma-separated list of trusted proxy hosts or '*' for all",
|
||||
)
|
||||
|
||||
# Admin interface (disabled by default for security)
|
||||
web_admin_enabled: bool = Field(
|
||||
default=False,
|
||||
|
||||
@@ -20,7 +20,7 @@ class TracePath(Base, UUIDMixin, TimestampMixin):
|
||||
path_len: Path length
|
||||
flags: Trace flags
|
||||
auth: Authentication data
|
||||
path_hashes: JSON array of node hash identifiers
|
||||
path_hashes: JSON array of hex-encoded node hash identifiers (variable length)
|
||||
snr_values: JSON array of SNR values per hop
|
||||
hop_count: Total number of hops
|
||||
received_at: When received by interface
|
||||
|
||||
@@ -133,7 +133,7 @@ class TraceDataEvent(BaseModel):
|
||||
)
|
||||
path_hashes: Optional[list[str]] = Field(
|
||||
default=None,
|
||||
description="Array of 2-character node hash identifiers",
|
||||
description="Array of hex-encoded node hash identifiers (variable length, e.g. '4a' for single-byte or 'b3fa' for multibyte)",
|
||||
)
|
||||
snr_values: Optional[list[float]] = Field(
|
||||
default=None,
|
||||
|
||||
@@ -155,7 +155,8 @@ class TracePathRead(BaseModel):
|
||||
flags: Optional[int] = Field(default=None, description="Trace flags")
|
||||
auth: Optional[int] = Field(default=None, description="Auth data")
|
||||
path_hashes: Optional[list[str]] = Field(
|
||||
default=None, description="Node hash identifiers"
|
||||
default=None,
|
||||
description="Hex-encoded node hash identifiers (variable length, e.g. '4a' for single-byte or 'b3fa' for multibyte)",
|
||||
)
|
||||
snr_values: Optional[list[float]] = Field(
|
||||
default=None, description="SNR values per hop"
|
||||
|
||||
@@ -180,7 +180,11 @@ def _build_config_json(app: FastAPI, request: Request) -> str:
|
||||
"logo_invert_light": app.state.logo_invert_light,
|
||||
}
|
||||
|
||||
return json.dumps(config)
|
||||
# Escape "</script>" sequences to prevent XSS breakout from the
|
||||
# <script> block where this JSON is embedded via |safe in the
|
||||
# Jinja2 template. "<\/" is valid JSON per the spec and parsed
|
||||
# correctly by JavaScript's JSON.parse().
|
||||
return json.dumps(config).replace("</", "<\\/")
|
||||
|
||||
|
||||
def create_app(
|
||||
@@ -236,7 +240,24 @@ def create_app(
|
||||
)
|
||||
|
||||
# Trust proxy headers (X-Forwarded-Proto, X-Forwarded-For) for HTTPS detection
|
||||
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*")
|
||||
trusted_hosts_raw = settings.web_trusted_proxy_hosts
|
||||
if trusted_hosts_raw == "*":
|
||||
trusted_hosts: str | list[str] = "*"
|
||||
else:
|
||||
trusted_hosts = [h.strip() for h in trusted_hosts_raw.split(",") if h.strip()]
|
||||
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=trusted_hosts)
|
||||
|
||||
# Compute effective admin flag (parameter overrides setting)
|
||||
effective_admin = (
|
||||
admin_enabled if admin_enabled is not None else settings.web_admin_enabled
|
||||
)
|
||||
|
||||
# Warn when admin is enabled but proxy trust is wide open
|
||||
if effective_admin 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."
|
||||
)
|
||||
|
||||
# Add cache control headers based on resource type
|
||||
app.add_middleware(CacheControlMiddleware)
|
||||
@@ -256,9 +277,7 @@ def create_app(
|
||||
)
|
||||
app.state.api_url = api_url or settings.api_base_url
|
||||
app.state.api_key = api_key or settings.api_key
|
||||
app.state.admin_enabled = (
|
||||
admin_enabled if admin_enabled is not None else settings.web_admin_enabled
|
||||
)
|
||||
app.state.admin_enabled = effective_admin
|
||||
app.state.network_name = network_name or settings.network_name
|
||||
app.state.network_city = network_city or settings.network_city
|
||||
app.state.network_country = network_country or settings.network_country
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { apiGet, apiPost, apiPut, apiDelete } from '../../api.js';
|
||||
import {
|
||||
html, litRender, nothing,
|
||||
getConfig, errorAlert, successAlert, t,
|
||||
getConfig, errorAlert, successAlert, t, escapeHtml,
|
||||
} from '../../components.js';
|
||||
import { iconLock } from '../../icons.js';
|
||||
|
||||
@@ -304,7 +304,7 @@ ${flashHtml}
|
||||
const memberName = row.dataset.memberName;
|
||||
const confirmMsg = t('common.delete_entity_confirm', {
|
||||
entity: t('entities.member').toLowerCase(),
|
||||
name: memberName
|
||||
name: escapeHtml(memberName)
|
||||
});
|
||||
container.querySelector('#delete_confirm_message').innerHTML = confirmMsg;
|
||||
container.querySelector('#deleteModal').showModal();
|
||||
|
||||
@@ -2,7 +2,7 @@ import { apiGet, apiPost, apiPut, apiDelete } from '../../api.js';
|
||||
import {
|
||||
html, litRender, nothing, unsafeHTML,
|
||||
getConfig, typeEmoji, formatDateTimeShort, errorAlert,
|
||||
successAlert, truncateKey, t,
|
||||
successAlert, truncateKey, t, escapeHtml,
|
||||
} from '../../components.js';
|
||||
import { iconTag, iconLock } from '../../icons.js';
|
||||
|
||||
@@ -240,7 +240,8 @@ export async function render(container, params, router) {
|
||||
<div class="modal-box">
|
||||
<h3 class="font-bold text-lg">${t('common.copy_all_entity_to_another_node', { entity: t('entities.tags') })}</h3>
|
||||
<form id="copy-all-form" class="py-4">
|
||||
<p class="mb-4">${unsafeHTML(t('common.copy_all_entity_description', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: nodeName }))}</p>
|
||||
<!-- unsafeHTML needed for translation HTML tags; nodeName is pre-escaped -->
|
||||
<p class="mb-4">${unsafeHTML(t('common.copy_all_entity_description', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: escapeHtml(nodeName) }))}</p>
|
||||
<div class="form-control mb-4">
|
||||
<label class="label"><span class="label-text">${t('admin_node_tags.destination_node')}</span></label>
|
||||
<select id="copyAllDestination" class="select select-bordered w-full" required>
|
||||
@@ -269,7 +270,8 @@ export async function render(container, params, router) {
|
||||
<div class="modal-box">
|
||||
<h3 class="font-bold text-lg">${t('common.delete_all_entity', { entity: t('entities.tags') })}</h3>
|
||||
<div class="py-4">
|
||||
<p class="mb-4">${unsafeHTML(t('common.delete_all_entity_confirm', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: nodeName }))}</p>
|
||||
<!-- unsafeHTML needed for translation HTML tags; nodeName is pre-escaped -->
|
||||
<p class="mb-4">${unsafeHTML(t('common.delete_all_entity_confirm', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: escapeHtml(nodeName) }))}</p>
|
||||
<div class="alert alert-error mb-4">
|
||||
<svg xmlns="http://www.w3.org/2000/svg" class="stroke-current shrink-0 h-6 w-6" fill="none" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z" /></svg>
|
||||
<span>${t('admin_node_tags.delete_all_warning')}</span>
|
||||
@@ -449,7 +451,7 @@ ${contentHtml}`, container);
|
||||
activeTagKey = row.dataset.tagKey;
|
||||
const confirmMsg = t('common.delete_entity_confirm', {
|
||||
entity: t('entities.tag').toLowerCase(),
|
||||
name: `"<span class="font-mono font-semibold">${activeTagKey}</span>"`
|
||||
name: `"<span class="font-mono font-semibold">${escapeHtml(activeTagKey)}</span>"`
|
||||
});
|
||||
container.querySelector('#delete_tag_confirm_message').innerHTML = confirmMsg;
|
||||
container.querySelector('#deleteModal').showModal();
|
||||
|
||||
@@ -1,8 +1,28 @@
|
||||
"""Tests for API authentication."""
|
||||
"""Tests for API authentication.
|
||||
|
||||
Verifies that constant-time key comparison (hmac.compare_digest) works
|
||||
correctly with no behavioral regressions from the original == operator.
|
||||
"""
|
||||
|
||||
import base64
|
||||
|
||||
|
||||
class TestAuthenticationFlow:
|
||||
"""Tests for authentication behavior."""
|
||||
def _make_basic_auth(username: str, password: str) -> str:
|
||||
"""Create a Basic auth header value."""
|
||||
credentials = base64.b64encode(f"{username}:{password}".encode()).decode()
|
||||
return f"Basic {credentials}"
|
||||
|
||||
|
||||
def _clear_metrics_cache() -> None:
|
||||
"""Clear the metrics module cache."""
|
||||
from meshcore_hub.api.metrics import _cache
|
||||
|
||||
_cache["output"] = b""
|
||||
_cache["expires_at"] = 0.0
|
||||
|
||||
|
||||
class TestReadAuthentication:
|
||||
"""Tests for read-level authentication (require_read)."""
|
||||
|
||||
def test_no_auth_when_keys_not_configured(self, client_no_auth):
|
||||
"""Test that no auth is required when keys are not configured."""
|
||||
@@ -30,46 +50,47 @@ class TestAuthenticationFlow:
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_read_key_accepted_on_multiple_endpoints(self, client_with_auth):
|
||||
"""Test that read key is accepted across different read endpoints."""
|
||||
for endpoint in ["/api/v1/nodes", "/api/v1/messages"]:
|
||||
response = client_with_auth.get(
|
||||
endpoint,
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200, f"Read key rejected on {endpoint}"
|
||||
|
||||
def test_read_endpoints_accept_admin_key(self, client_with_auth):
|
||||
"""Test that read endpoints accept admin key."""
|
||||
"""Test that admin key also grants read access."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/nodes",
|
||||
headers={"Authorization": "Bearer test-admin-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_admin_endpoints_reject_read_key(self, client_with_auth):
|
||||
"""Test that admin endpoints reject read key."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 403
|
||||
def test_admin_key_grants_read_on_multiple_endpoints(self, client_with_auth):
|
||||
"""Test that admin key grants read access across different endpoints."""
|
||||
for endpoint in ["/api/v1/nodes", "/api/v1/messages"]:
|
||||
response = client_with_auth.get(
|
||||
endpoint,
|
||||
headers={"Authorization": "Bearer test-admin-key"},
|
||||
)
|
||||
assert (
|
||||
response.status_code == 200
|
||||
), f"Admin key rejected on read endpoint {endpoint}"
|
||||
|
||||
def test_admin_endpoints_accept_admin_key(self, client_with_auth):
|
||||
"""Test that admin endpoints accept admin key."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer test-admin-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_invalid_key_rejected(self, client_with_auth):
|
||||
"""Test that invalid keys are rejected."""
|
||||
def test_invalid_key_rejected_on_read_endpoint(self, client_with_auth):
|
||||
"""Test that invalid keys are rejected with 401 on read endpoints."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/nodes",
|
||||
headers={"Authorization": "Bearer invalid-key"},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_no_auth_header_rejected_on_read_endpoint(self, client_with_auth):
|
||||
"""Test that missing auth header is rejected on read endpoints."""
|
||||
response = client_with_auth.get("/api/v1/nodes")
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_missing_bearer_prefix_rejected(self, client_with_auth):
|
||||
"""Test that tokens without Bearer prefix are rejected."""
|
||||
response = client_with_auth.get(
|
||||
@@ -87,6 +108,124 @@ class TestAuthenticationFlow:
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
class TestAdminAuthentication:
|
||||
"""Tests for admin-level authentication (require_admin)."""
|
||||
|
||||
def test_admin_endpoints_accept_admin_key(self, client_with_auth):
|
||||
"""Test that admin endpoints accept admin key."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer test-admin-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_admin_endpoints_reject_read_key(self, client_with_auth):
|
||||
"""Test that admin endpoints reject read key with 403."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_admin_endpoints_reject_invalid_key(self, client_with_auth):
|
||||
"""Test that admin endpoints reject invalid keys with 403."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer completely-wrong-key"},
|
||||
)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_admin_endpoints_reject_no_auth_header(self, client_with_auth):
|
||||
"""Test that admin endpoints reject missing auth header with 401."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
class TestMetricsAuthentication:
|
||||
"""Tests for metrics endpoint authentication (Basic auth with hmac.compare_digest)."""
|
||||
|
||||
def test_metrics_no_auth_when_no_read_key(self, client_no_auth):
|
||||
"""Test that metrics requires no auth when no read key is configured."""
|
||||
_clear_metrics_cache()
|
||||
response = client_no_auth.get("/metrics")
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_metrics_accepts_valid_basic_auth(self, client_with_auth):
|
||||
"""Test that metrics accepts correct Basic credentials."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={"Authorization": _make_basic_auth("metrics", "test-read-key")},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_metrics_rejects_no_auth_when_key_set(self, client_with_auth):
|
||||
"""Test 401 when read key is set but no auth provided."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get("/metrics")
|
||||
assert response.status_code == 401
|
||||
assert "WWW-Authenticate" in response.headers
|
||||
|
||||
def test_metrics_rejects_wrong_password(self, client_with_auth):
|
||||
"""Test that metrics rejects incorrect password."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={"Authorization": _make_basic_auth("metrics", "wrong-key")},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_metrics_rejects_wrong_username(self, client_with_auth):
|
||||
"""Test that metrics rejects incorrect username."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={"Authorization": _make_basic_auth("admin", "test-read-key")},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_metrics_rejects_bearer_auth(self, client_with_auth):
|
||||
"""Test that Bearer auth does not work for metrics."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_metrics_rejects_admin_key_as_password(self, client_with_auth):
|
||||
"""Test that admin key is not accepted as metrics password.
|
||||
|
||||
Metrics uses only the read key for Basic auth, not the admin key.
|
||||
"""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={
|
||||
"Authorization": _make_basic_auth("metrics", "test-admin-key"),
|
||||
},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
class TestHealthEndpoint:
|
||||
"""Tests for health check endpoint."""
|
||||
|
||||
|
||||
@@ -35,35 +35,71 @@ class TestDashboardStats:
|
||||
assert data["total_advertisements"] == 1
|
||||
|
||||
|
||||
class TestDashboardHtml:
|
||||
"""Tests for GET /dashboard endpoint."""
|
||||
class TestDashboardHtmlRemoved:
|
||||
"""Tests that legacy HTML dashboard endpoint has been removed."""
|
||||
|
||||
def test_dashboard_html_response(self, client_no_auth):
|
||||
"""Test dashboard returns HTML."""
|
||||
def test_dashboard_html_endpoint_removed(self, client_no_auth):
|
||||
"""Test that GET /dashboard no longer returns HTML (legacy endpoint removed)."""
|
||||
response = client_no_auth.get("/api/v1/dashboard")
|
||||
assert response.status_code == 200
|
||||
assert "text/html" in response.headers["content-type"]
|
||||
assert "<!DOCTYPE html>" in response.text
|
||||
assert "MeshCore Hub Dashboard" in response.text
|
||||
assert response.status_code in (404, 405)
|
||||
|
||||
def test_dashboard_contains_stats(
|
||||
self, client_no_auth, sample_node, sample_message
|
||||
):
|
||||
"""Test dashboard HTML contains stat values."""
|
||||
response = client_no_auth.get("/api/v1/dashboard")
|
||||
assert response.status_code == 200
|
||||
# Check that stats are present
|
||||
assert "Total Nodes" in response.text
|
||||
assert "Active Nodes" in response.text
|
||||
assert "Total Messages" in response.text
|
||||
def test_dashboard_html_endpoint_removed_trailing_slash(self, client_no_auth):
|
||||
"""Test that GET /dashboard/ also returns 404/405."""
|
||||
response = client_no_auth.get("/api/v1/dashboard/")
|
||||
assert response.status_code in (404, 405)
|
||||
|
||||
def test_dashboard_contains_recent_data(self, client_no_auth, sample_node):
|
||||
"""Test dashboard HTML contains recent nodes."""
|
||||
response = client_no_auth.get("/api/v1/dashboard")
|
||||
|
||||
class TestDashboardAuthenticatedJsonRoutes:
|
||||
"""Tests that dashboard JSON sub-routes return valid JSON with authentication."""
|
||||
|
||||
def test_stats_returns_json_when_authenticated(self, client_with_auth):
|
||||
"""Test GET /dashboard/stats returns 200 with valid JSON when authenticated."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/dashboard/stats",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert "Recent Nodes" in response.text
|
||||
# The node name should appear in the table
|
||||
assert sample_node.name in response.text
|
||||
data = response.json()
|
||||
assert "total_nodes" in data
|
||||
assert "active_nodes" in data
|
||||
assert "total_messages" in data
|
||||
assert "total_advertisements" in data
|
||||
|
||||
def test_activity_returns_json_when_authenticated(self, client_with_auth):
|
||||
"""Test GET /dashboard/activity returns 200 with valid JSON when authenticated."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/dashboard/activity",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert "days" in data
|
||||
assert "data" in data
|
||||
assert isinstance(data["data"], list)
|
||||
|
||||
def test_message_activity_returns_json_when_authenticated(self, client_with_auth):
|
||||
"""Test GET /dashboard/message-activity returns 200 with valid JSON when authenticated."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/dashboard/message-activity",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert "days" in data
|
||||
assert "data" in data
|
||||
assert isinstance(data["data"], list)
|
||||
|
||||
def test_node_count_returns_json_when_authenticated(self, client_with_auth):
|
||||
"""Test GET /dashboard/node-count returns 200 with valid JSON when authenticated."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/dashboard/node-count",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert "days" in data
|
||||
assert "data" in data
|
||||
assert isinstance(data["data"], list)
|
||||
|
||||
|
||||
class TestDashboardActivity:
|
||||
|
||||
@@ -2,6 +2,53 @@
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from meshcore_hub.common.models import TracePath
|
||||
|
||||
|
||||
class TestMultibytePathHashes:
|
||||
"""Tests for multibyte path hash support in trace path API responses."""
|
||||
|
||||
def test_list_trace_paths_returns_multibyte_path_hashes(
|
||||
self, client_no_auth, api_db_session
|
||||
):
|
||||
"""Test that GET /trace-paths returns multibyte path hashes faithfully."""
|
||||
multibyte_hashes = ["4a2b", "b3fa"]
|
||||
trace = TracePath(
|
||||
initiator_tag=77777,
|
||||
path_hashes=multibyte_hashes,
|
||||
hop_count=2,
|
||||
received_at=datetime.now(timezone.utc),
|
||||
)
|
||||
api_db_session.add(trace)
|
||||
api_db_session.commit()
|
||||
api_db_session.refresh(trace)
|
||||
|
||||
response = client_no_auth.get("/api/v1/trace-paths")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert len(data["items"]) == 1
|
||||
assert data["items"][0]["path_hashes"] == multibyte_hashes
|
||||
|
||||
def test_get_trace_path_returns_mixed_length_path_hashes(
|
||||
self, client_no_auth, api_db_session
|
||||
):
|
||||
"""Test that GET /trace-paths/{id} returns mixed-length path hashes."""
|
||||
mixed_hashes = ["4a", "b3fa", "02"]
|
||||
trace = TracePath(
|
||||
initiator_tag=88888,
|
||||
path_hashes=mixed_hashes,
|
||||
hop_count=3,
|
||||
received_at=datetime.now(timezone.utc),
|
||||
)
|
||||
api_db_session.add(trace)
|
||||
api_db_session.commit()
|
||||
api_db_session.refresh(trace)
|
||||
|
||||
response = client_no_auth.get(f"/api/v1/trace-paths/{trace.id}")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["path_hashes"] == mixed_hashes
|
||||
|
||||
|
||||
class TestListTracePaths:
|
||||
"""Tests for GET /trace-paths endpoint."""
|
||||
|
||||
68
tests/test_collector/test_letsmesh_normalizer.py
Normal file
68
tests/test_collector/test_letsmesh_normalizer.py
Normal file
@@ -0,0 +1,68 @@
|
||||
"""Tests for LetsMesh normalizer _normalize_hash_list method."""
|
||||
|
||||
from meshcore_hub.collector.letsmesh_normalizer import LetsMeshNormalizer
|
||||
|
||||
|
||||
class TestNormalizeHashList:
|
||||
"""Tests for _normalize_hash_list with variable-length hex strings."""
|
||||
|
||||
def test_single_byte_hashes_accepted_and_uppercased(self) -> None:
|
||||
"""Single-byte (2-char) hex hashes are accepted and uppercased."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list(["4a", "b3", "fa"])
|
||||
assert result == ["4A", "B3", "FA"]
|
||||
|
||||
def test_multibyte_hashes_accepted_and_uppercased(self) -> None:
|
||||
"""Multibyte (4-char) hex hashes are accepted and uppercased."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list(["4a2b", "b3fa"])
|
||||
assert result == ["4A2B", "B3FA"]
|
||||
|
||||
def test_mixed_length_hashes_all_accepted(self) -> None:
|
||||
"""Mixed-length hashes (2-char and 4-char) are all accepted."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list(["4a", "b3fa", "02"])
|
||||
assert result == ["4A", "B3FA", "02"]
|
||||
|
||||
def test_odd_length_strings_filtered_out(self) -> None:
|
||||
"""Odd-length hex strings are filtered out."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list(["4a", "b3f", "02"])
|
||||
assert result == ["4A", "02"]
|
||||
|
||||
def test_invalid_hex_characters_filtered_out(self) -> None:
|
||||
"""Strings with non-hex characters are filtered out."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list(["4a", "zz", "02"])
|
||||
assert result == ["4A", "02"]
|
||||
|
||||
def test_empty_list_returns_none(self) -> None:
|
||||
"""Empty list returns None."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list([])
|
||||
assert result is None
|
||||
|
||||
def test_non_string_items_filtered_out(self) -> None:
|
||||
"""Non-string items are filtered out, valid strings kept."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list([42, "4a"])
|
||||
assert result == ["4A"]
|
||||
|
||||
def test_non_list_input_returns_none(self) -> None:
|
||||
"""Non-list input returns None."""
|
||||
assert LetsMeshNormalizer._normalize_hash_list(None) is None
|
||||
assert LetsMeshNormalizer._normalize_hash_list("4a") is None
|
||||
assert LetsMeshNormalizer._normalize_hash_list(42) is None
|
||||
|
||||
def test_all_invalid_items_returns_none(self) -> None:
|
||||
"""List where all items are invalid returns None."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list(["z", "b3f", 42])
|
||||
assert result is None
|
||||
|
||||
def test_six_char_hashes_accepted(self) -> None:
|
||||
"""Six-character (3-byte) hex strings are accepted."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list(["ab12cd", "ef34ab"])
|
||||
assert result == ["AB12CD", "EF34AB"]
|
||||
|
||||
def test_whitespace_stripped_before_validation(self) -> None:
|
||||
"""Leading/trailing whitespace is stripped before validation."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list([" 4a ", " b3fa"])
|
||||
assert result == ["4A", "B3FA"]
|
||||
|
||||
def test_single_char_string_rejected(self) -> None:
|
||||
"""Single-character strings are rejected (minimum is 2)."""
|
||||
result = LetsMeshNormalizer._normalize_hash_list(["a", "4a"])
|
||||
assert result == ["4A"]
|
||||
@@ -608,6 +608,107 @@ class TestSubscriber:
|
||||
assert payload["hop_count"] == 4
|
||||
assert payload["snr_values"] == [12.5, 11.5, 10.0, 6.25]
|
||||
|
||||
def test_letsmesh_trace_data_with_multibyte_path_hashes(
|
||||
self, mock_mqtt_client, db_manager
|
||||
) -> None:
|
||||
"""Multibyte path hashes flow through the collector pipeline correctly."""
|
||||
mock_mqtt_client.topic_builder.parse_letsmesh_upload_topic.return_value = (
|
||||
"a" * 64,
|
||||
"packets",
|
||||
)
|
||||
subscriber = Subscriber(
|
||||
mock_mqtt_client,
|
||||
db_manager,
|
||||
ingest_mode="letsmesh_upload",
|
||||
)
|
||||
trace_handler = MagicMock()
|
||||
subscriber.register_handler("trace_data", trace_handler)
|
||||
subscriber.start()
|
||||
|
||||
with patch.object(
|
||||
subscriber._letsmesh_decoder,
|
||||
"decode_payload",
|
||||
return_value={
|
||||
"payloadType": 9,
|
||||
"pathLength": 3,
|
||||
"payload": {
|
||||
"decoded": {
|
||||
"type": 9,
|
||||
"traceTag": "DF9D7A20",
|
||||
"authCode": 0,
|
||||
"flags": 0,
|
||||
"pathHashes": ["71a2", "0Bcd", "24ef"],
|
||||
"snrValues": [12.5, 11.5, 10.0],
|
||||
}
|
||||
},
|
||||
},
|
||||
):
|
||||
subscriber._handle_mqtt_message(
|
||||
topic=f"meshcore/BOS/{'a' * 64}/packets",
|
||||
pattern="meshcore/BOS/+/packets",
|
||||
payload={
|
||||
"packet_type": "9",
|
||||
"hash": "99887766",
|
||||
"raw": "ABCDEF",
|
||||
},
|
||||
)
|
||||
|
||||
trace_handler.assert_called_once()
|
||||
_public_key, event_type, payload, _db = trace_handler.call_args.args
|
||||
assert event_type == "trace_data"
|
||||
assert payload["path_hashes"] == ["71A2", "0BCD", "24EF"]
|
||||
assert payload["hop_count"] == 3
|
||||
|
||||
def test_letsmesh_path_updated_with_multibyte_path_hashes(
|
||||
self, mock_mqtt_client, db_manager
|
||||
) -> None:
|
||||
"""Multibyte path hashes in path_updated events are normalized correctly."""
|
||||
mock_mqtt_client.topic_builder.parse_letsmesh_upload_topic.return_value = (
|
||||
"a" * 64,
|
||||
"packets",
|
||||
)
|
||||
subscriber = Subscriber(
|
||||
mock_mqtt_client,
|
||||
db_manager,
|
||||
ingest_mode="letsmesh_upload",
|
||||
)
|
||||
path_handler = MagicMock()
|
||||
subscriber.register_handler("path_updated", path_handler)
|
||||
subscriber.start()
|
||||
|
||||
with patch.object(
|
||||
subscriber._letsmesh_decoder,
|
||||
"decode_payload",
|
||||
return_value={
|
||||
"payloadType": 8,
|
||||
"payload": {
|
||||
"decoded": {
|
||||
"type": 8,
|
||||
"isValid": True,
|
||||
"pathLength": 2,
|
||||
"pathHashes": ["AA11", "BB22"],
|
||||
"extraType": 244,
|
||||
"extraData": "D" * 64,
|
||||
}
|
||||
},
|
||||
},
|
||||
):
|
||||
subscriber._handle_mqtt_message(
|
||||
topic=f"meshcore/BOS/{'a' * 64}/packets",
|
||||
pattern="meshcore/BOS/+/packets",
|
||||
payload={
|
||||
"packet_type": "8",
|
||||
"hash": "99887766",
|
||||
"raw": "ABCDEF",
|
||||
},
|
||||
)
|
||||
|
||||
path_handler.assert_called_once()
|
||||
_public_key, event_type, payload, _db = path_handler.call_args.args
|
||||
assert event_type == "path_updated"
|
||||
assert payload["path_hashes"] == ["AA11", "BB22"]
|
||||
assert payload["hop_count"] == 2
|
||||
|
||||
def test_letsmesh_packet_type_8_maps_to_path_updated(
|
||||
self, mock_mqtt_client, db_manager
|
||||
) -> None:
|
||||
|
||||
@@ -125,11 +125,3 @@ class TestWebSettings:
|
||||
settings = WebSettings(_env_file=None, data_home="/custom/data")
|
||||
|
||||
assert settings.web_data_dir == "/custom/data/web"
|
||||
|
||||
def test_web_datetime_locale_default_and_override(self) -> None:
|
||||
"""Date formatting locale has sensible default and can be overridden."""
|
||||
default_settings = WebSettings(_env_file=None)
|
||||
custom_settings = WebSettings(_env_file=None, web_datetime_locale="en-GB")
|
||||
|
||||
assert default_settings.web_datetime_locale == "en-US"
|
||||
assert custom_settings.web_datetime_locale == "en-GB"
|
||||
|
||||
@@ -137,6 +137,44 @@ class TestTracePathModel:
|
||||
assert trace.initiator_tag == 123456789
|
||||
assert trace.path_hashes == ["4a", "b3", "fa"]
|
||||
|
||||
def test_multibyte_path_hashes_round_trip(self, db_session) -> None:
|
||||
"""Test that multibyte (4-char) path hashes round-trip correctly."""
|
||||
path_hashes = ["4a2b", "b3fa", "02cd"]
|
||||
trace = TracePath(
|
||||
initiator_tag=987654321,
|
||||
path_len=3,
|
||||
path_hashes=path_hashes,
|
||||
snr_values=[20.0, 15.0, 10.0],
|
||||
hop_count=3,
|
||||
)
|
||||
db_session.add(trace)
|
||||
db_session.commit()
|
||||
|
||||
# Expire cached attributes to force reload from database
|
||||
db_session.expire(trace)
|
||||
|
||||
assert trace.path_hashes == ["4a2b", "b3fa", "02cd"]
|
||||
assert len(trace.path_hashes) == 3
|
||||
|
||||
def test_mixed_length_path_hashes_round_trip(self, db_session) -> None:
|
||||
"""Test that mixed-length path hashes round-trip correctly."""
|
||||
path_hashes = ["4a", "b3fa", "02"]
|
||||
trace = TracePath(
|
||||
initiator_tag=111222333,
|
||||
path_len=3,
|
||||
path_hashes=path_hashes,
|
||||
snr_values=[22.0, 17.5, 11.0],
|
||||
hop_count=3,
|
||||
)
|
||||
db_session.add(trace)
|
||||
db_session.commit()
|
||||
|
||||
# Expire cached attributes to force reload from database
|
||||
db_session.expire(trace)
|
||||
|
||||
assert trace.path_hashes == ["4a", "b3fa", "02"]
|
||||
assert len(trace.path_hashes) == 3
|
||||
|
||||
|
||||
class TestTelemetryModel:
|
||||
"""Tests for Telemetry model."""
|
||||
|
||||
@@ -316,8 +316,10 @@ def mock_http_client() -> MockHttpClient:
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def web_app(mock_http_client: MockHttpClient) -> Any:
|
||||
def web_app(mock_http_client: MockHttpClient, monkeypatch: pytest.MonkeyPatch) -> Any:
|
||||
"""Create a web app with mocked HTTP client."""
|
||||
# Ensure tests use a consistent locale regardless of local .env
|
||||
monkeypatch.setenv("WEB_DATETIME_LOCALE", "en-US")
|
||||
app = create_app(
|
||||
api_url="http://localhost:8000",
|
||||
api_key="test-api-key",
|
||||
|
||||
279
tests/test_web/test_app.py
Normal file
279
tests/test_web/test_app.py
Normal file
@@ -0,0 +1,279 @@
|
||||
"""Tests for web app: config JSON escaping and trusted proxy hosts warnings."""
|
||||
|
||||
import json
|
||||
import logging
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from meshcore_hub.web.app import _build_config_json, create_app
|
||||
|
||||
from .conftest import ALL_FEATURES_ENABLED, MockHttpClient
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def xss_app(mock_http_client: MockHttpClient) -> Any:
|
||||
"""Create a web app with a network name containing a script injection payload."""
|
||||
app = create_app(
|
||||
api_url="http://localhost:8000",
|
||||
api_key="test-api-key",
|
||||
network_name="</script><script>alert(1)</script>",
|
||||
network_city="Test City",
|
||||
network_country="Test Country",
|
||||
network_radio_config="Test Radio Config",
|
||||
network_contact_email="test@example.com",
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
app.state.http_client = mock_http_client
|
||||
return app
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def xss_client(xss_app: Any, mock_http_client: MockHttpClient) -> TestClient:
|
||||
"""Create a test client whose network_name contains a script injection payload."""
|
||||
xss_app.state.http_client = mock_http_client
|
||||
return TestClient(xss_app, raise_server_exceptions=True)
|
||||
|
||||
|
||||
class TestConfigJsonXssEscaping:
|
||||
"""Tests that _build_config_json escapes </script> to prevent XSS breakout."""
|
||||
|
||||
def test_script_tag_escaped_in_rendered_html(self, xss_client: TestClient) -> None:
|
||||
"""Config value containing </script> is escaped to <\\/script> in the HTML."""
|
||||
response = xss_client.get("/")
|
||||
assert response.status_code == 200
|
||||
|
||||
html = response.text
|
||||
|
||||
# The literal "</script>" must NOT appear inside the config JSON block.
|
||||
# Find the config JSON assignment to isolate the embedded block.
|
||||
config_marker = "window.__APP_CONFIG__ = "
|
||||
start = html.find(config_marker)
|
||||
assert start != -1, "Config JSON block not found in rendered HTML"
|
||||
start += len(config_marker)
|
||||
end = html.find(";", start)
|
||||
config_block = html[start:end]
|
||||
|
||||
# The raw closing tag must be escaped
|
||||
assert "</script>" not in config_block
|
||||
assert "<\\/script>" in config_block
|
||||
|
||||
def test_normal_config_values_unaffected(self, client: TestClient) -> None:
|
||||
"""Config values without special characters render unchanged."""
|
||||
response = client.get("/")
|
||||
assert response.status_code == 200
|
||||
|
||||
html = response.text
|
||||
config_marker = "window.__APP_CONFIG__ = "
|
||||
start = html.find(config_marker)
|
||||
assert start != -1
|
||||
start += len(config_marker)
|
||||
end = html.find(";", start)
|
||||
config_block = html[start:end]
|
||||
|
||||
config = json.loads(config_block)
|
||||
assert config["network_name"] == "Test Network"
|
||||
assert config["network_city"] == "Test City"
|
||||
assert config["network_country"] == "Test Country"
|
||||
|
||||
def test_escaped_json_is_parseable(self, xss_client: TestClient) -> None:
|
||||
"""The escaped JSON is still valid and parseable by json.loads."""
|
||||
response = xss_client.get("/")
|
||||
assert response.status_code == 200
|
||||
|
||||
html = response.text
|
||||
config_marker = "window.__APP_CONFIG__ = "
|
||||
start = html.find(config_marker)
|
||||
assert start != -1
|
||||
start += len(config_marker)
|
||||
end = html.find(";", start)
|
||||
config_block = html[start:end]
|
||||
|
||||
# json.loads handles <\/ sequences correctly (they are valid JSON)
|
||||
config = json.loads(config_block)
|
||||
assert isinstance(config, dict)
|
||||
# The parsed value should contain the original unescaped string
|
||||
assert config["network_name"] == "</script><script>alert(1)</script>"
|
||||
|
||||
def test_build_config_json_direct_escaping(self, web_app: Any) -> None:
|
||||
"""Calling _build_config_json directly escapes </ sequences."""
|
||||
from starlette.requests import Request
|
||||
|
||||
# Inject a malicious value into the app state
|
||||
web_app.state.network_name = "</script><script>alert(1)</script>"
|
||||
|
||||
scope = {
|
||||
"type": "http",
|
||||
"method": "GET",
|
||||
"path": "/",
|
||||
"query_string": b"",
|
||||
"headers": [],
|
||||
}
|
||||
request = Request(scope)
|
||||
|
||||
result = _build_config_json(web_app, request)
|
||||
|
||||
# Raw output must not contain literal "</script>"
|
||||
assert "</script>" not in result
|
||||
assert "<\\/script>" in result
|
||||
|
||||
# Result must still be valid JSON
|
||||
parsed = json.loads(result)
|
||||
assert parsed["network_name"] == "</script><script>alert(1)</script>"
|
||||
|
||||
def test_build_config_json_no_escaping_needed(self, web_app: Any) -> None:
|
||||
"""_build_config_json leaves normal values intact when no </ present."""
|
||||
from starlette.requests import Request
|
||||
|
||||
scope = {
|
||||
"type": "http",
|
||||
"method": "GET",
|
||||
"path": "/",
|
||||
"query_string": b"",
|
||||
"headers": [],
|
||||
}
|
||||
request = Request(scope)
|
||||
|
||||
result = _build_config_json(web_app, request)
|
||||
|
||||
# No escaping artifacts for normal values
|
||||
assert "<\\/" not in result
|
||||
|
||||
parsed = json.loads(result)
|
||||
assert parsed["network_name"] == "Test Network"
|
||||
assert parsed["network_city"] == "Test City"
|
||||
|
||||
|
||||
class TestTrustedProxyHostsWarning:
|
||||
"""Tests for trusted proxy hosts startup warning in create_app."""
|
||||
|
||||
def test_warning_logged_when_admin_enabled_and_wildcard_hosts(
|
||||
self, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A warning is logged when WEB_ADMIN_ENABLED=true and WEB_TRUSTED_PROXY_HOSTS is '*'."""
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_admin_enabled=True,
|
||||
web_trusted_proxy_hosts="*",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="meshcore_hub.web.app"):
|
||||
create_app(
|
||||
api_url="http://localhost:8000",
|
||||
admin_enabled=True,
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
assert any(
|
||||
"WEB_ADMIN_ENABLED is true but WEB_TRUSTED_PROXY_HOSTS is '*'" in msg
|
||||
for msg in caplog.messages
|
||||
), f"Expected warning not found in log messages: {caplog.messages}"
|
||||
|
||||
def test_no_warning_when_trusted_proxy_hosts_is_specific(
|
||||
self, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""No warning is logged when WEB_TRUSTED_PROXY_HOSTS is set to a specific value."""
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_admin_enabled=True,
|
||||
web_trusted_proxy_hosts="10.0.0.1",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="meshcore_hub.web.app"):
|
||||
create_app(
|
||||
api_url="http://localhost:8000",
|
||||
admin_enabled=True,
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
assert not any(
|
||||
"WEB_TRUSTED_PROXY_HOSTS" in msg for msg in caplog.messages
|
||||
), f"Unexpected warning found in log messages: {caplog.messages}"
|
||||
|
||||
def test_no_warning_when_admin_disabled(
|
||||
self, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""No warning is logged when WEB_ADMIN_ENABLED is false even with wildcard hosts."""
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_admin_enabled=False,
|
||||
web_trusted_proxy_hosts="*",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="meshcore_hub.web.app"):
|
||||
create_app(
|
||||
api_url="http://localhost:8000",
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
assert not any(
|
||||
"WEB_TRUSTED_PROXY_HOSTS" in msg for msg in caplog.messages
|
||||
), f"Unexpected warning found in log messages: {caplog.messages}"
|
||||
|
||||
def test_proxy_hosts_comma_list_parsed_correctly(self) -> None:
|
||||
"""A comma-separated WEB_TRUSTED_PROXY_HOSTS is split into a list for middleware."""
|
||||
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware
|
||||
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_trusted_proxy_hosts="10.0.0.1, 10.0.0.2, 172.16.0.1",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
app = create_app(
|
||||
api_url="http://localhost:8000",
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
# Find the ProxyHeadersMiddleware entry in app.user_middleware
|
||||
proxy_entries = [
|
||||
m for m in app.user_middleware if m.cls is ProxyHeadersMiddleware
|
||||
]
|
||||
assert len(proxy_entries) == 1, "ProxyHeadersMiddleware not found in middleware"
|
||||
assert proxy_entries[0].kwargs["trusted_hosts"] == [
|
||||
"10.0.0.1",
|
||||
"10.0.0.2",
|
||||
"172.16.0.1",
|
||||
]
|
||||
|
||||
def test_wildcard_proxy_hosts_passed_as_string(self) -> None:
|
||||
"""Wildcard WEB_TRUSTED_PROXY_HOSTS='*' is passed as a string to middleware."""
|
||||
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware
|
||||
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_trusted_proxy_hosts="*",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
app = create_app(
|
||||
api_url="http://localhost:8000",
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
# Find the ProxyHeadersMiddleware entry in app.user_middleware
|
||||
proxy_entries = [
|
||||
m for m in app.user_middleware if m.cls is ProxyHeadersMiddleware
|
||||
]
|
||||
assert len(proxy_entries) == 1, "ProxyHeadersMiddleware not found in middleware"
|
||||
assert proxy_entries[0].kwargs["trusted_hosts"] == "*"
|
||||
Reference in New Issue
Block a user