mirror of
https://github.com/ipnet-mesh/meshcore-hub.git
synced 2026-08-09 18:32:44 +02:00
Remove stale .claude/ and .plans/ directories
This commit is contained in:
@@ -1,60 +0,0 @@
|
||||
---
|
||||
allowed-tools: Bash(gh label list:*),Bash(gh issue view:*),Bash(gh issue edit:*),Bash(gh search:*)
|
||||
description: Apply labels to GitHub issues
|
||||
---
|
||||
|
||||
You're an issue triage assistant for GitHub issues. Your task is to analyze the issue and select appropriate labels from the provided list.
|
||||
|
||||
IMPORTANT: Don't post any comments or messages to the issue. Your only action should be to apply labels.
|
||||
|
||||
Issue Information:
|
||||
|
||||
- REPO: ${{ github.repository }}
|
||||
- ISSUE_NUMBER: ${{ github.event.issue.number }}
|
||||
|
||||
TASK OVERVIEW:
|
||||
|
||||
1. First, fetch the list of labels available in this repository by running: `gh label list`. Run exactly this command with nothing else.
|
||||
|
||||
2. Next, use gh commands to get context about the issue:
|
||||
|
||||
- Use `gh issue view ${{ github.event.issue.number }}` to retrieve the current issue's details
|
||||
- Use `gh search issues` to find similar issues that might provide context for proper categorization
|
||||
- You have access to these Bash commands:
|
||||
- Bash(gh label list:\*) - to get available labels
|
||||
- Bash(gh issue view:\*) - to view issue details
|
||||
- Bash(gh issue edit:\*) - to apply labels to the issue
|
||||
- Bash(gh search:\*) - to search for similar issues
|
||||
|
||||
3. Analyze the issue content, considering:
|
||||
|
||||
- The issue title and description
|
||||
- The type of issue (bug report, feature request, question, etc.)
|
||||
- Technical areas mentioned
|
||||
- Severity or priority indicators
|
||||
- User impact
|
||||
- Components affected
|
||||
|
||||
4. Select appropriate labels from the available labels list provided above:
|
||||
|
||||
- Choose labels that accurately reflect the issue's nature
|
||||
- Be specific but comprehensive
|
||||
- IMPORTANT: Add a priority label (P1, P2, or P3) based on the label descriptions from gh label list
|
||||
- Consider platform labels (android, ios) if applicable
|
||||
- If you find similar issues using gh search, consider using a "duplicate" label if appropriate. Only do so if the issue is a duplicate of another OPEN issue.
|
||||
|
||||
5. Apply the selected labels:
|
||||
- Use `gh issue edit` to apply your selected labels
|
||||
- DO NOT post any comments explaining your decision
|
||||
- DO NOT communicate directly with users
|
||||
- If no labels are clearly applicable, do not apply any labels
|
||||
|
||||
IMPORTANT GUIDELINES:
|
||||
|
||||
- Be thorough in your analysis
|
||||
- Only select labels from the provided list above
|
||||
- DO NOT post any comments to the issue
|
||||
- Your ONLY action should be to apply labels using gh issue edit
|
||||
- It's okay to not add any labels if none are clearly applicable
|
||||
|
||||
---
|
||||
@@ -1,44 +0,0 @@
|
||||
---
|
||||
name: documentation
|
||||
description: Audit and update project documentation to accurately reflect the current codebase. Use when documentation may be outdated, after significant code changes, or when the user asks to review or update docs.
|
||||
---
|
||||
|
||||
# Documentation Audit
|
||||
|
||||
Audit and update all project documentation so it accurately reflects the current state of the codebase. Documentation must only describe features, options, configurations, and functionality that actually exist in the code.
|
||||
|
||||
## Files to Review
|
||||
|
||||
- **README.md** - Project overview, setup instructions, usage examples
|
||||
- **AGENTS.md** - AI coding assistant guidelines, project structure, conventions
|
||||
- **.env.example** - Example environment variables
|
||||
|
||||
Also check for substantial comments or inline instructions within the codebase that may be outdated.
|
||||
|
||||
## Process
|
||||
|
||||
1. **Read all documentation files** listed above in full.
|
||||
|
||||
2. **Cross-reference against the codebase.** For every documented item (features, env vars, CLI commands, routes, models, directory paths, conventions), search the code to verify:
|
||||
- It actually exists.
|
||||
- Its described behavior matches the implementation.
|
||||
- File paths and directory structures are accurate.
|
||||
|
||||
3. **Identify and fix discrepancies:**
|
||||
- **Version updates** — ensure documentation reflects any new/updated/removed versions. Check .python-version, pyproject.toml, etc.
|
||||
- **Stale/legacy content** — documented but no longer in the code. Remove it.
|
||||
- **Missing content** — exists in the code but not documented. Add it.
|
||||
- **Inaccurate descriptions** — documented behavior doesn't match implementation. Correct it.
|
||||
|
||||
4. **Apply updates** to each file. Preserve existing style and structure.
|
||||
|
||||
5. **Verify consistency** across all documentation files — they must not contradict each other.
|
||||
|
||||
## Rules
|
||||
|
||||
- Do NOT invent features or options that don't exist in the code.
|
||||
- Do NOT remove documentation for features that DO exist.
|
||||
- Do NOT change the fundamental structure or style of the docs.
|
||||
- Do NOT modify CLAUDE.md.
|
||||
- Focus on accuracy, not cosmetic changes.
|
||||
- When in doubt, check the source code.
|
||||
@@ -1,49 +0,0 @@
|
||||
---
|
||||
name: git-branch
|
||||
description: Create a new branch from latest main with the project's naming convention (feat/fix/chore). Use when starting new work on a feature, bug fix, or chore.
|
||||
---
|
||||
|
||||
# Git Branch
|
||||
|
||||
Create a new branch from the latest `main` branch using the project's naming convention.
|
||||
|
||||
## Arguments
|
||||
|
||||
The user may provide arguments in the format: `<type>/<description>`
|
||||
|
||||
- `type` — one of `feat`, `fix`, or `chore`
|
||||
- `description` — short kebab-case description (e.g., `add-map-clustering`)
|
||||
|
||||
If not provided, ask the user for the branch type and description.
|
||||
|
||||
## Process
|
||||
|
||||
1. **Fetch latest main:**
|
||||
|
||||
```bash
|
||||
git fetch origin main
|
||||
```
|
||||
|
||||
2. **Determine branch name:**
|
||||
|
||||
- If the user provided arguments (e.g., `/git-branch feat/add-map-clustering`), use them directly.
|
||||
- Otherwise, ask the user for:
|
||||
- **Branch type**: `feat`, `fix`, or `chore`
|
||||
- **Short description**: a brief kebab-case slug describing the work
|
||||
- Construct the branch name as `{type}/{slug}` (e.g., `feat/add-map-clustering`).
|
||||
|
||||
3. **Create and switch to the new branch:**
|
||||
|
||||
```bash
|
||||
git checkout -b {branch_name} origin/main
|
||||
```
|
||||
|
||||
4. **Confirm** by reporting the new branch name to the user.
|
||||
|
||||
## Rules
|
||||
|
||||
- Branch names MUST follow the `{type}/{slug}` convention.
|
||||
- Valid types are `feat`, `fix`, and `chore` only.
|
||||
- The slug MUST be kebab-case (lowercase, hyphens, no spaces or underscores).
|
||||
- Always branch from `origin/main`, never from the current branch.
|
||||
- Do NOT push the branch — just create it locally.
|
||||
@@ -1,94 +0,0 @@
|
||||
---
|
||||
name: git-pr
|
||||
description: Create a pull request to main from the current branch. Runs quality checks, commits changes, pushes, and opens a PR via gh CLI. Use when ready to submit work for review.
|
||||
---
|
||||
|
||||
# Git PR
|
||||
|
||||
Create a pull request to `main` from the current feature branch.
|
||||
|
||||
## Process
|
||||
|
||||
### Phase 1: Pre-flight Checks
|
||||
|
||||
1. **Verify branch:**
|
||||
|
||||
```bash
|
||||
git branch --show-current
|
||||
```
|
||||
|
||||
- The current branch must NOT be `main`. If on `main`, tell the user to create a feature branch first (e.g., `/git-branch`).
|
||||
|
||||
2. **Check for uncommitted changes:**
|
||||
|
||||
```bash
|
||||
git status
|
||||
```
|
||||
|
||||
- If there are uncommitted changes, ask the user for a commit message and commit them using the `/git-commit` skill conventions (no Claude authoring details).
|
||||
|
||||
### Phase 2: Quality Checks
|
||||
|
||||
1. **Determine changed components** by comparing against `main`:
|
||||
|
||||
```bash
|
||||
git diff --name-only main...HEAD
|
||||
```
|
||||
|
||||
2. **Run targeted tests** based on changed files:
|
||||
- `tests/test_web/` for web-only changes (templates, static JS, web routes)
|
||||
- `tests/test_api/` for API changes
|
||||
- `tests/test_collector/` for collector changes
|
||||
- `tests/test_interface/` for interface/sender/receiver changes
|
||||
- `tests/test_common/` for common models/schemas/config changes
|
||||
- Run the full `pytest` if changes span multiple components
|
||||
|
||||
3. **Run pre-commit checks:**
|
||||
|
||||
```bash
|
||||
pre-commit run --all-files
|
||||
```
|
||||
|
||||
- If checks fail and auto-fix files, commit the fixes and re-run until clean.
|
||||
|
||||
4. If tests or checks fail and cannot be auto-fixed, report the issues to the user and stop.
|
||||
|
||||
### Phase 3: Push and Create PR
|
||||
|
||||
1. **Push the branch to origin:**
|
||||
|
||||
```bash
|
||||
git push -u origin HEAD
|
||||
```
|
||||
|
||||
2. **Generate PR content:**
|
||||
- **Title**: Derive from the branch name. Convert `feat/add-map-clustering` to `Add map clustering`, `fix/login-error` to `Fix login error`, etc. Keep under 70 characters.
|
||||
- **Body**: Generate a summary from the commit history:
|
||||
|
||||
```bash
|
||||
git log main..HEAD --oneline
|
||||
```
|
||||
|
||||
3. **Create the PR:**
|
||||
|
||||
```bash
|
||||
gh pr create --title "{title}" --body "$(cat <<'EOF'
|
||||
## Summary
|
||||
{bullet points summarizing the changes}
|
||||
|
||||
## Test plan
|
||||
{checklist of testing steps}
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
4. **Return the PR URL** to the user.
|
||||
|
||||
## Rules
|
||||
|
||||
- Do NOT create a PR from `main`.
|
||||
- Do NOT skip quality checks — tests and pre-commit must pass.
|
||||
- Do NOT force-push.
|
||||
- Always target `main` as the base branch.
|
||||
- Keep the PR title concise (under 70 characters).
|
||||
- If quality checks fail, fix issues or report to the user — do NOT create the PR with failing checks.
|
||||
@@ -1,66 +0,0 @@
|
||||
---
|
||||
name: quality
|
||||
description: Run the full test suite, pre-commit checks, and re-run tests to ensure code quality. Fixes any issues found. Use after code changes, before commits, or when the user asks to check quality.
|
||||
---
|
||||
|
||||
# Quality Check
|
||||
|
||||
Run the full quality pipeline: tests, pre-commit checks, and a verification test run. Fix any issues discovered at each stage.
|
||||
|
||||
## Prerequisites
|
||||
|
||||
Before running checks, ensure the environment is ready:
|
||||
|
||||
1. Check for `.venv` directory — create with `python -m venv .venv` if missing.
|
||||
2. Activate the virtual environment: `source .venv/bin/activate`
|
||||
3. Install dependencies: `pip install -e ".[dev]"`
|
||||
|
||||
## Process
|
||||
|
||||
### Phase 1: Initial Test Run
|
||||
|
||||
Run the full test suite to establish a baseline:
|
||||
|
||||
```bash
|
||||
pytest
|
||||
```
|
||||
|
||||
- If tests **pass**, proceed to Phase 2.
|
||||
- If tests **fail**, investigate and fix the failures before continuing. Re-run the failing tests to confirm fixes. Then proceed to Phase 2.
|
||||
|
||||
### Phase 2: Pre-commit Checks
|
||||
|
||||
Run all pre-commit hooks against the entire codebase:
|
||||
|
||||
```bash
|
||||
pre-commit run --all-files
|
||||
```
|
||||
|
||||
- If all checks **pass**, proceed to Phase 3.
|
||||
- If checks **fail**:
|
||||
- Many hooks (black, trailing whitespace, end-of-file) auto-fix issues. Re-run `pre-commit run --all-files` to confirm auto-fixes resolved the issues.
|
||||
- For remaining failures (flake8, mypy, etc.), investigate and fix manually.
|
||||
- Re-run `pre-commit run --all-files` until all checks pass.
|
||||
- Then proceed to Phase 3.
|
||||
|
||||
### Phase 3: Verification Test Run
|
||||
|
||||
Run the full test suite again to ensure pre-commit fixes (formatting, import sorting, etc.) haven't broken any functionality:
|
||||
|
||||
```bash
|
||||
pytest
|
||||
```
|
||||
|
||||
- If tests **pass**, the quality check is complete.
|
||||
- If tests **fail**, the pre-commit fixes introduced a regression. Investigate and fix, then re-run both `pre-commit run --all-files` and `pytest` until both pass cleanly.
|
||||
|
||||
## Rules
|
||||
|
||||
- Always run the FULL test suite (`pytest`), not targeted tests.
|
||||
- Always run pre-commit against ALL files (`--all-files`).
|
||||
- Do NOT skip or ignore failing tests — investigate and fix them.
|
||||
- Do NOT skip or ignore pre-commit failures — investigate and fix them.
|
||||
- Do NOT modify test assertions to make tests pass unless the test is genuinely wrong.
|
||||
- Do NOT disable pre-commit hooks or add noqa/type:ignore unless truly justified.
|
||||
- Fix root causes, not symptoms.
|
||||
- If a fix requires changes outside the scope of a simple quality fix (e.g., a design change), report it to the user rather than making the change silently.
|
||||
@@ -1,114 +0,0 @@
|
||||
---
|
||||
name: release
|
||||
description: Full release workflow — quality gate, semantic version tag, push, and GitHub release. Use when ready to cut a new release from main.
|
||||
---
|
||||
|
||||
# Release
|
||||
|
||||
Run the full release workflow: quality checks, version tagging, push, and GitHub release creation.
|
||||
|
||||
## Arguments
|
||||
|
||||
The user may optionally provide a version number (e.g., `/release 1.2.0`). If not provided, one will be suggested based on commit history.
|
||||
|
||||
## Process
|
||||
|
||||
### Phase 1: Pre-flight Checks
|
||||
|
||||
1. **Verify on `main` branch:**
|
||||
|
||||
```bash
|
||||
git branch --show-current
|
||||
```
|
||||
|
||||
- Must be on `main`. If not, tell the user to switch to `main` first.
|
||||
|
||||
2. **Verify working tree is clean:**
|
||||
|
||||
```bash
|
||||
git status --porcelain
|
||||
```
|
||||
|
||||
- If there are uncommitted changes, tell the user to commit or stash them first.
|
||||
|
||||
3. **Pull latest:**
|
||||
|
||||
```bash
|
||||
git pull origin main
|
||||
```
|
||||
|
||||
### Phase 2: Quality Gate
|
||||
|
||||
1. **Run full test suite:**
|
||||
|
||||
```bash
|
||||
pytest
|
||||
```
|
||||
|
||||
2. **Run pre-commit checks:**
|
||||
|
||||
```bash
|
||||
pre-commit run --all-files
|
||||
```
|
||||
|
||||
3. If either fails, report the issues and stop. Do NOT proceed with a release that has failing checks.
|
||||
|
||||
### Phase 3: Determine Version
|
||||
|
||||
1. **Get the latest tag:**
|
||||
|
||||
```bash
|
||||
git describe --tags --abbrev=0 2>/dev/null || echo "none"
|
||||
```
|
||||
|
||||
2. **List commits since last tag:**
|
||||
|
||||
```bash
|
||||
git log {last_tag}..HEAD --oneline
|
||||
```
|
||||
|
||||
If no previous tag exists, list the last 20 commits:
|
||||
|
||||
```bash
|
||||
git log --oneline -20
|
||||
```
|
||||
|
||||
3. **Determine next version:**
|
||||
- If the user provided a version, use it.
|
||||
- Otherwise, suggest a version based on commit prefixes:
|
||||
- Any commit starting with `feat` or `Add` → **minor** bump
|
||||
- Only `fix` or `Fix` commits → **patch** bump
|
||||
- If no previous tag, suggest `0.1.0`
|
||||
- Present the suggestion and ask the user to confirm or provide a different version.
|
||||
|
||||
### Phase 4: Tag and Release
|
||||
|
||||
1. **Create annotated tag:**
|
||||
|
||||
```bash
|
||||
git tag -a v{version} -m "Release v{version}"
|
||||
```
|
||||
|
||||
2. **Push tag to origin:**
|
||||
|
||||
```bash
|
||||
git push origin v{version}
|
||||
```
|
||||
|
||||
3. **Create GitHub release:**
|
||||
|
||||
```bash
|
||||
gh release create v{version} --title "v{version}" --generate-notes
|
||||
```
|
||||
|
||||
4. **Report** the release URL to the user.
|
||||
|
||||
## Rules
|
||||
|
||||
- MUST be on `main` branch with a clean working tree.
|
||||
- MUST pass all quality checks before tagging.
|
||||
- Tags MUST follow the `v{major}.{minor}.{patch}` format (e.g., `v1.2.0`).
|
||||
- Always create an annotated tag, not a lightweight tag.
|
||||
- Always confirm the version with the user before tagging.
|
||||
- Do NOT skip quality checks under any circumstances.
|
||||
- Do NOT force-push tags.
|
||||
@@ -1,100 +0,0 @@
|
||||
## 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.
|
||||
---
|
||||
@@ -1,162 +0,0 @@
|
||||
# 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 |
|
||||
@@ -1,65 +0,0 @@
|
||||
# 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
|
||||
@@ -1,54 +0,0 @@
|
||||
# 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
|
||||
@@ -1,70 +0,0 @@
|
||||
# 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.
|
||||
@@ -1,90 +0,0 @@
|
||||
# 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.
|
||||
@@ -1,22 +0,0 @@
|
||||
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
|
||||
@@ -1,117 +0,0 @@
|
||||
# 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`.
|
||||
@@ -1,401 +0,0 @@
|
||||
# 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"
|
||||
@@ -1,81 +0,0 @@
|
||||
## 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.
|
||||
---
|
||||
@@ -1,146 +0,0 @@
|
||||
# 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 |
|
||||
@@ -1,17 +0,0 @@
|
||||
# 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
|
||||
@@ -1,19 +0,0 @@
|
||||
# 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
|
||||
@@ -1,57 +0,0 @@
|
||||
# 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.
|
||||
@@ -1,89 +0,0 @@
|
||||
# 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.
|
||||
@@ -1,18 +0,0 @@
|
||||
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
|
||||
@@ -1,102 +0,0 @@
|
||||
# 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.
|
||||
@@ -1,274 +0,0 @@
|
||||
# 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"
|
||||
Reference in New Issue
Block a user