Add report summary and HTML web-UI output#6
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new data sources and expands output formatting options for the breach scraper, adding interactive HTML dashboards and text reports, along with corresponding CLI options, documentation, and unit tests. The review feedback highlights a correctness bug in _html_affected_count where a count of 0 is incorrectly treated as falsy, potential XSS vulnerabilities in HTML link generation due to unvalidated URL schemes, and an opportunity to reduce code duplication by reusing the affected count helper in the text report generator.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _html_affected_count(row: Row) -> str: | ||
| return str( | ||
| row.get("persons_affected", "") | ||
| or row.get("total_persons_affected", "") | ||
| or row.get("number_affected", "") | ||
| or row.get("individuals_affected", "") | ||
| or "" | ||
| ) |
There was a problem hiding this comment.
The current implementation of _html_affected_count uses an or chain to find the first non-empty value. However, in Python, the integer 0 is falsy. If any of these fields contain the integer 0 (which is a valid count of affected individuals), the or chain will skip it and evaluate the subsequent fields, or default to "". This can lead to incorrect data representation and statistics.
Using a loop to check for explicit non-None and non-empty string values resolves this correctness issue.
def _html_affected_count(row: Row) -> str:
for key in ("persons_affected", "total_persons_affected", "number_affected", "individuals_affected"):
val = row.get(key)
if val is not None and val != "":
return str(val)
return ""There was a problem hiding this comment.
Fixed. _html_affected_count now iterates the candidate keys and returns the first value that is not None and not empty, so a 0 count is preserved. Covered by test_zero_is_not_skipped.
| def _html_notice_link(row: Row) -> str: | ||
| notice_url = str(row.get("notice_url", "") or row.get("organization_name_url", "") or "") | ||
| if not notice_url: | ||
| return "" | ||
| return ( | ||
| f'<a href="{html.escape(notice_url, quote=True)}" target="_blank" rel="noopener noreferrer">' | ||
| "View notice" | ||
| "</a>" | ||
| ) |
There was a problem hiding this comment.
The notice_url is retrieved from scraped data and rendered directly into an href attribute. If a scraped source contains a malicious URL (such as a javascript: URI), this can lead to a Cross-Site Scripting (XSS) vulnerability when a user clicks the link in the generated HTML report.
To mitigate this, we should validate that the URL uses a safe scheme (e.g., http or https) using urlparse before rendering it as a link.
def _is_safe_url(url: str) -> bool:
try:
parsed = urlparse(url)
return parsed.scheme in ("http", "https")
except Exception:
return False
def _html_notice_link(row: Row) -> str:
notice_url = str(row.get("notice_url", "") or row.get("organization_name_url", "") or "").strip()
if not _is_safe_url(notice_url):
return ""
return (
f'<a href="{html.escape(notice_url, quote=True)}" target="_blank" rel="noopener noreferrer">'
"View notice"
"</a>"
)There was a problem hiding this comment.
Fixed. Added _is_safe_url (http/https only, via urlparse) and a _safe_notice_url helper; _html_notice_link renders no link for unsafe schemes, and the data-sort-notice attribute now uses the sanitized value too. Verified that a javascript: notice_url produces no and that 'javascript:' no longer appears anywhere in the HTML output. Covered by test_notice_link_rejects_javascript_scheme.
| def _html_organization_link(row: Row) -> str: | ||
| organization_name = str(row.get("organization_name", "") or "Unknown organization") | ||
| organization_url = str(row.get("organization_name_url", "") or "") | ||
| safe_name = html.escape(organization_name) | ||
| if not organization_url: | ||
| return safe_name | ||
| return ( | ||
| f'<a href="{html.escape(organization_url, quote=True)}" target="_blank" rel="noopener noreferrer">' | ||
| f"{safe_name}" | ||
| "</a>" | ||
| ) |
There was a problem hiding this comment.
Similar to _html_notice_link, the organization_url is retrieved from scraped data and rendered directly into an href attribute. To prevent potential Cross-Site Scripting (XSS) attacks via malicious javascript: URIs, we should validate the URL scheme using the _is_safe_url helper.
| def _html_organization_link(row: Row) -> str: | |
| organization_name = str(row.get("organization_name", "") or "Unknown organization") | |
| organization_url = str(row.get("organization_name_url", "") or "") | |
| safe_name = html.escape(organization_name) | |
| if not organization_url: | |
| return safe_name | |
| return ( | |
| f'<a href="{html.escape(organization_url, quote=True)}" target="_blank" rel="noopener noreferrer">' | |
| f"{safe_name}" | |
| "</a>" | |
| ) | |
| def _html_organization_link(row: Row) -> str: | |
| organization_name = str(row.get("organization_name", "") or "Unknown organization") | |
| organization_url = str(row.get("organization_name_url", "") or "").strip() | |
| safe_name = html.escape(organization_name) | |
| if not _is_safe_url(organization_url): | |
| return safe_name | |
| return ( | |
| f'<a href="{html.escape(organization_url, quote=True)}" target="_blank" rel="noopener noreferrer">' | |
| f"{safe_name}" | |
| "</a>" | |
| ) |
There was a problem hiding this comment.
Fixed. _html_organization_link now uses _is_safe_url and falls back to the escaped name for unsafe schemes. Covered by test_organization_link_rejects_javascript_scheme.
| lines.append( | ||
| "- Persons affected: " | ||
| f"{row.get('persons_affected', '') or row.get('total_persons_affected', '') or row.get('number_affected', '') or row.get('individuals_affected', '') or 'Unknown'}" | ||
| ) |
There was a problem hiding this comment.
Instead of duplicating the long or chain to extract the affected count (which also suffers from the falsy 0 bug), we should reuse the _html_affected_count helper function. This improves maintainability and ensures correctness.
affected = _html_affected_count(row) or "Unknown"
lines.append(f"- Persons affected: {affected}")There was a problem hiding this comment.
Fixed. to_report now reuses _html_affected_count for the persons-affected line, removing the duplicated or-chain (and its falsy-0 bug).
1 participant
Stage 3 (final) of the multi-source integration: the
reportsummary and the sortable HTML web UI, stdlib-only.Summary
output.pynow provides two more formats:report(text summary with totals + per-notice detail) andhtml(a self-contained, sortable dashboard with accessible sort buttons; no external assets).reportandhtmlinto--output.↕etc.), JS usesString.fromCharCode(...)-- so both the source and the generated HTML are pure ASCII.lineterminator="\n"fix and switchedoutput.pytocollections.abcimports; added a per-fileE501ignore for the embedded web-UI template.Test Plan
breach-scraper --source wa_atg --input-html tests/fixtures/wa_atg_sample.html --output html --start-date 2024-01-01 --end-date 2024-12-31renders a sortable table; output is ASCII--output reportprints a summary with totals