diff --git a/SPECS/ARCHIVE/FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets/FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets.md b/SPECS/ARCHIVE/FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets/FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets.md new file mode 100644 index 00000000..f23b1877 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets/FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets.md @@ -0,0 +1,85 @@ +# PRD: FU-P12-T1-6 — Uniform HTML escaping in `renderClientWidgets` + +**Created:** 2026-02-19 +**Priority:** P3 +**Branch:** `codex/feature/FU-P12-T1-6-uniform-html-escaping-client-widgets` +**Status:** PLAN + +--- + +## 1. Problem Statement + +`renderClientWidgets` in the dashboard escapes `name` and `version`, but does +not consistently escape all interpolated values (`count`, `lastSeen`) before +injecting HTML. This asymmetric pattern is low risk today but increases review +and audit complexity. + +--- + +## 2. Scope + +### In Scope +- Update `renderClientWidgets` so all interpolated values are passed through + `escapeHtml()`. +- Add or update frontend tests to verify escaping is applied uniformly. + +### Out of Scope +- Dashboard visual redesign. +- Backend metrics payload changes. +- Changes to `formatRelativeAge` behavior. + +--- + +## 3. Deliverables + +1. Uniform escaping in client widget rendering +- `src/mcpbridge_wrapper/webui/static/dashboard.js` +- Ensure `count` and `lastSeen` paths use `escapeHtml()` consistently. + +2. Test coverage +- `tests/unit/webui/static/test_dashboard.js` +- Add/update assertions for escaped rendering values. + +3. Validation artifact +- `SPECS/INPROGRESS/FU-P12-T1-6_Validation_Report.md` + +--- + +## 4. Acceptance Criteria + +- [ ] All interpolated values in `renderClientWidgets` are escaped. +- [ ] No visual/behavioral regression in client widget rendering. +- [ ] `pytest` passes. +- [ ] `ruff check src/` passes. +- [ ] `mypy src/` passes. +- [ ] `pytest --cov` reports coverage >= 90%. + +--- + +## 5. Dependencies + +- FU-P12-T1-3 ✅ + +--- + +## 6. Risks and Mitigations + +- **Risk:** Escaping numeric values may alter formatting unexpectedly. + - **Mitigation:** Keep formatting conversion stable (`String(...)`) and verify + rendered output via tests. + +- **Risk:** Duplicate escaping for derived strings could change displayed text. + - **Mitigation:** Escape at the final interpolation boundary and validate with + existing widget rendering tests. + +--- + +## 7. Validation Plan + +1. Apply uniform escaping in `renderClientWidgets`. +2. Add/update tests for escaped output paths. +3. Run full quality gates and capture results. + +--- +**Archived:** 2026-02-19 +**Verdict:** PASS diff --git a/SPECS/ARCHIVE/FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets/FU-P12-T1-6_Validation_Report.md b/SPECS/ARCHIVE/FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets/FU-P12-T1-6_Validation_Report.md new file mode 100644 index 00000000..1c51c072 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets/FU-P12-T1-6_Validation_Report.md @@ -0,0 +1,39 @@ +# Validation Report — FU-P12-T1-6 + +**Task:** FU-P12-T1-6 — Uniform HTML escaping in `renderClientWidgets` +**Date:** 2026-02-19 +**Verdict:** PASS + +## Scope + +- Applied uniform escaping in `renderClientWidgets` so `count` and `lastSeen` + are escaped before interpolation into `innerHTML`. +- Added a static-asset test verifying `dashboard.js` contains the escaped + interpolation path for these widget values. + +## Files Changed + +- `src/mcpbridge_wrapper/webui/static/dashboard.js` +- `tests/unit/webui/test_server.py` + +## Required Quality Gates + +- `pytest` + - Result: **PASS** (`594 passed, 5 skipped, 2 warnings`) +- `ruff check src/` + - Result: **PASS** (`All checks passed!`) +- `mypy src/` + - Result: **PASS** (`Success: no issues found in 18 source files`) +- `pytest --cov` + - Result: **PASS** (`594 passed, 5 skipped, 2 warnings`; total coverage **92.18%**, threshold 90%) + +## Acceptance Criteria Status + +- [x] All interpolated values in `renderClientWidgets` are passed through `escapeHtml()`. +- [x] No visual regression in client widget rendering. +- [x] `pytest` suite remains green. + +## Notes + +- Existing third-party deprecation warnings from `websockets` / `uvicorn` were + observed during test runs and are unrelated to this task. diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 8d7314f0..16afc9bc 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,6 +1,6 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-02-19 (REVIEW_fu_p12_t1_5_client_identity_retention_archived) +**Last Updated:** 2026-02-19 (REVIEW_fu_p12_t1_6_uniform_client_widget_escaping_archived) ## Archived Tasks @@ -120,6 +120,7 @@ | FU-P12-T1-3 | [FU-P12-T1-3_Show_multi-client_widgets_in_Web_UI_instead_of_single_overwritten_active_client/](FU-P12-T1-3_Show_multi-client_widgets_in_Web_UI_instead_of_single_overwritten_active_client/) | 2026-02-18 | PASS | | FU-P12-T1-4 | [FU-P12-T1-4_Make_IN_FLIGHT_KPI_reflect_real_in_flight_requests_in_shared_metrics_mode/](FU-P12-T1-4_Make_IN_FLIGHT_KPI_reflect_real_in_flight_requests_in_shared_metrics_mode/) | 2026-02-19 | PASS | | FU-P12-T1-5 | [FU-P12-T1-5_Cap_clients_dict_and_prune_client_identities_to_prevent_unbounded_growth/](FU-P12-T1-5_Cap_clients_dict_and_prune_client_identities_to_prevent_unbounded_growth/) | 2026-02-19 | PASS | +| FU-P12-T1-6 | [FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets/](FU-P12-T1-6_Uniform_HTML_escaping_in_renderClientWidgets/) | 2026-02-19 | PASS | | FU-P12-T3-2 | [FU-P12-T3-2_Add_error_code_column_to_audit_CSV_export/](FU-P12-T3-2_Add_error_code_column_to_audit_CSV_export/) | 2026-02-19 | PASS | ## Historical Artifacts @@ -204,6 +205,7 @@ | [REVIEW_FU-P12-T1-3_multi_client_widgets.md](_Historical/REVIEW_FU-P12-T1-3_multi_client_widgets.md) | Review report for FU-P12-T1-3 | | [REVIEW_FU-P12-T1-4_in_flight_shared_metrics.md](_Historical/REVIEW_FU-P12-T1-4_in_flight_shared_metrics.md) | Review report for FU-P12-T1-4 | | [REVIEW_FU-P12-T1-5_client_identity_retention.md](_Historical/REVIEW_FU-P12-T1-5_client_identity_retention.md) | Review report for FU-P12-T1-5 | +| [REVIEW_FU-P12-T1-6_uniform_client_widget_escaping.md](_Historical/REVIEW_FU-P12-T1-6_uniform_client_widget_escaping.md) | Review report for FU-P12-T1-6 | | [REVIEW_FU-P12-T3-2_error_code_csv_export.md](_Historical/REVIEW_FU-P12-T3-2_error_code_csv_export.md) | Review report for FU-P12-T3-2 | ## Archive Log @@ -362,5 +364,7 @@ | 2026-02-19 | FU-P12-T1-4 | Archived REVIEW_FU-P12-T1-4_in_flight_shared_metrics report | | 2026-02-19 | FU-P12-T1-5 | Archived Cap_clients_dict_and_prune_client_identities_to_prevent_unbounded_growth (PASS) | | 2026-02-19 | FU-P12-T1-5 | Archived REVIEW_FU-P12-T1-5_client_identity_retention report | +| 2026-02-19 | FU-P12-T1-6 | Archived Uniform_HTML_escaping_in_renderClientWidgets (PASS) | +| 2026-02-19 | FU-P12-T1-6 | Archived REVIEW_FU-P12-T1-6_uniform_client_widget_escaping report | | 2026-02-19 | FU-P12-T3-2 | Archived Add_error_code_column_to_audit_CSV_export (PASS) | | 2026-02-19 | FU-P12-T3-2 | Archived REVIEW_FU-P12-T3-2_error_code_csv_export report | diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_FU-P12-T1-6_uniform_client_widget_escaping.md b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P12-T1-6_uniform_client_widget_escaping.md new file mode 100644 index 00000000..4c2876f5 --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P12-T1-6_uniform_client_widget_escaping.md @@ -0,0 +1,32 @@ +## REVIEW REPORT — FU-P12-T1-6 uniform client widget escaping + +**Scope:** origin/main..HEAD +**Files:** 9 + +### Summary Verdict +- [x] Approve +- [ ] Approve with comments +- [ ] Request changes +- [ ] Block + +### Critical Issues +- None. + +### Secondary Issues +- None. + +### Architectural Notes +- Frontend rendering now escapes all `renderClientWidgets` interpolated values + uniformly, reducing audit ambiguity for future XSS/security reviews. +- Added a static asset assertion in server tests to lock in the escaping path. + +### Tests +- Quality gates rerun and passing: + - `pytest` (`594 passed, 5 skipped, 2 warnings`) + - `ruff check src/` (`All checks passed!`) + - `mypy src/` (`Success: no issues found in 18 source files`) + - `pytest --cov` (`92.18%`, threshold `>=90%`) + +### Next Steps +- No actionable follow-up items identified. +- FOLLOW-UP step can be skipped for this task. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index b099b699..0cb0aeac 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -2,15 +2,15 @@ ## Recently Archived +- 2026-02-19 — FU-P12-T1-6: Uniform HTML escaping in `renderClientWidgets` (PASS) - 2026-02-19 — FU-P12-T1-5: Cap `_clients` dict and prune `client_identities` to prevent unbounded growth (PASS) - 2026-02-19 — FU-P12-T1-4: Make `IN FLIGHT` KPI reflect real in-flight requests in shared-metrics mode (PASS) - 2026-02-19 — FU-P12-T3-2: Add `error_code` column to audit CSV export (PASS) - 2026-02-18 — FU-P12-T1-3: Show multi-client widgets in Web UI instead of single overwritten active client (PASS) - 2026-02-18 — FU-P12-T1-2: Add code comment clarifying stdin-only client capture in `on_request` (PASS) -- 2026-02-18 — FU-P12-T1-1: Remove or document `MCPInitializeParams` in schemas (PASS) ## Suggested Next Tasks - P13-T5 follow-up — Complete interactive prompt verification in a desktop session (P1) -- FU-P12-T1-6 — Uniform HTML escaping in `renderClientWidgets` (P3) - FU-P12-T3-1 — Document unused `error_message` parameter in `MetricsCollector.record_response` (P3) +- Final scripted follow-up after this: no additional `pick_next_task` items remain diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index 2765e1f7..de6357c5 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -2268,7 +2268,8 @@ Phase 9 Follow-up Backlog --- -#### ⬜️ FU-P12-T1-6: Uniform HTML escaping in `renderClientWidgets` +#### ✅ FU-P12-T1-6: Uniform HTML escaping in `renderClientWidgets` +- **Status:** ✅ Completed (2026-02-19) - **Description:** In `dashboard.js` `renderClientWidgets`, the `count` integer and `lastSeen` string are interpolated into innerHTML without `escapeHtml()`, while `name` and `version` are escaped. Although `count` is always a number and `lastSeen` already passes through `escapeHtml` inside `formatRelativeAge`, the asymmetric pattern makes security auditing harder. Apply `escapeHtml()` uniformly to all interpolated values for consistency. - **Priority:** P3 - **Dependencies:** FU-P12-T1-3 @@ -2276,9 +2277,9 @@ Phase 9 Follow-up Backlog - **Outputs/Artifacts:** - Updated `src/mcpbridge_wrapper/webui/static/dashboard.js` — uniform escaping in `renderClientWidgets` - **Acceptance Criteria:** - - [ ] All interpolated values in `renderClientWidgets` are passed through `escapeHtml()` - - [ ] No visual regression in client widget rendering - - [ ] `pytest` suite remains green + - [x] All interpolated values in `renderClientWidgets` are passed through `escapeHtml()` + - [x] No visual regression in client widget rendering + - [x] `pytest` suite remains green --- diff --git a/src/mcpbridge_wrapper/webui/static/dashboard.js b/src/mcpbridge_wrapper/webui/static/dashboard.js index b8347ba3..dd1823f9 100644 --- a/src/mcpbridge_wrapper/webui/static/dashboard.js +++ b/src/mcpbridge_wrapper/webui/static/dashboard.js @@ -227,10 +227,12 @@ var version = client.version || "unknown"; var count = client.initialize_count || 0; var lastSeen = formatRelativeAge(client.last_seen); + var escapedCount = escapeHtml(String(count)); + var escapedLastSeen = escapeHtml(String(lastSeen)); return "
" + "
" + escapeHtml(name) + " " + escapeHtml(version) + "
" - + "
Initialize calls: " + count + "
" - + "
Last seen: " + escapeHtml(lastSeen) + "
" + + "
Initialize calls: " + escapedCount + "
" + + "
Last seen: " + escapedLastSeen + "
" + "
"; }).join(""); } diff --git a/tests/unit/webui/test_server.py b/tests/unit/webui/test_server.py index b327f053..c4e30e32 100644 --- a/tests/unit/webui/test_server.py +++ b/tests/unit/webui/test_server.py @@ -159,6 +159,15 @@ def test_dashboard_served(self, client): assert "/static/dashboard.css" in response.text assert "/static/dashboard.js" in response.text + def test_dashboard_js_uses_uniform_client_widget_escaping(self, client): + """Client widget interpolations in dashboard.js use escapeHtml uniformly.""" + response = client.get("/static/dashboard.js") + assert response.status_code == 200 + assert 'Initialize calls: " + escapedCount + "' in response.text + assert 'Last seen: " + escapedLastSeen + "' in response.text + assert "var escapedCount = escapeHtml(String(count));" in response.text + assert "var escapedLastSeen = escapeHtml(String(lastSeen));" in response.text + def test_websocket_metrics_update_includes_sessions(self, client, audit): """WebSocket metrics_update message includes sessions key.""" with client.websocket_connect("/ws/metrics") as websocket: