Conversation
…(instances) as column headers
…d created dedicated Encounters UI page
| conn.close() | ||
| current = "X" | ||
| cell_html = f'<td hx-post="/ui/soa/{soa_id}/toggle_cell_instance" hx-vals=\'{{"instance_id": {instance_id}, "activity_id": {activity_id}}}\' hx-swap="outerHTML" class="cell">{current}</td>' | ||
| return HTMLResponse(cell_html) |
Check warning
Code scanning / CodeQL
Reflected server-side cross-site scripting Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
General approach: Any value derived from user input that is interpolated into HTML should be safely escaped for the relevant context (HTML/attribute/JS). In Python web apps, that typically means using html.escape() or a framework-provided escaping function before embedding user data into HTML strings.
Best concrete fix here: Explicitly escape soa_id, instance_id, and activity_id before inserting them into cell_html. Even though they are currently integers, converting them to strings and passing them through html.escape() ensures that, regardless of future type changes, no special characters can break out of HTML attributes or JSON-like structures. We can import html from the standard library and apply html.escape() when building the f-string. This change preserves existing functionality (the visual output is unchanged for numeric IDs) while eliminating the XSS concern and addressing all alert variants, since they all flow through the same cell_html expression.
Specific changes:
- In
src/soa_builder/web/app.py, addimport htmlalongside the other imports near the top of the file (we are allowed to add standard-library imports). - In function
ui_toggle_cell_instance, before buildingcell_html, compute escaped string versions ofsoa_id,instance_id, andactivity_idusinghtml.escape(str(...), quote=True). - Update the
cell_htmlf-string to use these escaped string variables instead of interpolating the raw values directly.
No additional methods or complex definitions are needed—just the import and local escaping.
| @@ -24,6 +24,7 @@ | ||
| import urllib.parse | ||
| import tempfile | ||
| import time | ||
| import html | ||
| from contextlib import asynccontextmanager | ||
| from datetime import datetime, timezone | ||
| from typing import Any, List, Optional | ||
| @@ -2569,7 +2570,14 @@ | ||
| conn.commit() | ||
| conn.close() | ||
| current = "X" | ||
| cell_html = f'<td hx-post="/ui/soa/{soa_id}/toggle_cell_instance" hx-vals=\'{{"instance_id": {instance_id}, "activity_id": {activity_id}}}\' hx-swap="outerHTML" class="cell">{current}</td>' | ||
| escaped_soa_id = html.escape(str(soa_id), quote=True) | ||
| escaped_instance_id = html.escape(str(instance_id), quote=True) | ||
| escaped_activity_id = html.escape(str(activity_id), quote=True) | ||
| cell_html = ( | ||
| f'<td hx-post="/ui/soa/{escaped_soa_id}/toggle_cell_instance" ' | ||
| f'hx-vals=\'{{"instance_id": {escaped_instance_id}, "activity_id": {escaped_activity_id}}}\' ' | ||
| f'hx-swap="outerHTML" class="cell">{current}</td>' | ||
| ) | ||
| return HTMLResponse(cell_html) | ||
|
|
||
|
|
There was a problem hiding this comment.
Pull request overview
This pull request adds a new page for managing "Encounters" (visits) in the UI and updates the data model to transition from visit-based to instance-based matrix cells. The changes support CDISC-compliant encounter metadata including environmental settings, timing, transition rules, and epoch associations.
Changes:
- Added a dedicated encounters management page with full CRUD operations
- Migrated matrix cells from
visit_idtoinstance_idas the primary axis - Integrated CDISC Library API for environmental settings and controlled terminology
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_web_api.py | Removed deprecated test file for old API |
| tests/test_ui_visit_create.py | Removed deprecated UI visit creation tests |
| tests/test_ui_toggle.py | Removed deprecated toggle tests |
| tests/test_ui_set_visit_epoch.py | Removed deprecated epoch assignment tests |
| tests/test_exports.py | Removed deprecated export tests |
| tests/test_deletion.py | Removed deprecated deletion tests |
| tests/test_cell_clear.py | Removed deprecated cell clearing tests |
| tests/test_bulk_import.py | Updated matrix import to use instances instead of visits |
| src/soa_builder/web/utils.py | Added functions for epoch/timing/environmental settings lookups |
| src/soa_builder/web/templates/*.html | Updated templates to use instance-based matrix, added encounters page |
| src/soa_builder/web/static/style.css | Added sidebar navigation styling |
| src/soa_builder/web/schemas.py | Added fields to VisitCreate/Update, renamed MatrixVisit to MatrixInstance |
| src/soa_builder/web/routers/visits.py | Major refactor: added UI endpoints, environmental settings integration |
| src/soa_builder/web/routers/timings.py | Minor whitespace cleanup |
| src/soa_builder/web/routers/instances.py | Added instance_options for UI dropdowns |
| src/soa_builder/web/migrate_database.py | Added migration for matrix_cells.instance_id column |
| src/soa_builder/web/app.py | Updated matrix operations to use instance_id, added toggle_cell_instance endpoint |
| scripts/check_ct_href.py | New utility script for testing CDISC API integration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.