Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog.d/999-report-title-callback-safety.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Harden generic diagnostics report title callback boundary

## Fixed

- Reject caller-defined `str` subclasses at the public generic diagnostics-report title boundary before truth-value or HTML-escaping callbacks can run, while preserving `None` and an empty exact built-in string as requests for the report-type default title.
31 changes: 15 additions & 16 deletions python/fast_mlsirm/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ def render_diagnostics_report(
raise ValueError("diagnostics JSON must contain an object")

report_type = _diagnostics_type(payload)
resolved_title = title or _default_title(report_type)
if title is not None and type(title) is not str:
raise ValueError("title must be a string or None")
resolved_title = title if title else _default_title(report_type)

out = Path(output_path)
if out.suffix.lower() != ".html":
Expand Down Expand Up @@ -217,9 +219,7 @@ def _render_dimensionality_report(payload: dict[str, Any]) -> list[str]:
def _metric_section(heading: str, metrics: dict[str, Any]) -> str | None:
"""Render a labelled metric-card grid, or ``None`` when there are no metrics."""
if len(metrics) > MAX_REPORT_METRICS:
raise ValueError(
f"report metrics exceed the {MAX_REPORT_METRICS}-entry limit"
)
raise ValueError(f"report metrics exceed the {MAX_REPORT_METRICS}-entry limit")

cards = []
for key, value in metrics.items():
Expand Down Expand Up @@ -443,13 +443,9 @@ def _rows_from_columnar(section: Any) -> list[dict[str, Any]]:
if row_count == 0:
return []
if row_count > MAX_REPORT_ROWS:
raise ValueError(
f"report table rows exceed the {MAX_REPORT_ROWS}-row limit"
)
raise ValueError(f"report table rows exceed the {MAX_REPORT_ROWS}-row limit")
if row_count * len(columns) > MAX_REPORT_CELLS:
raise ValueError(
f"report table cells exceed the {MAX_REPORT_CELLS}-cell limit"
)
raise ValueError(f"report table cells exceed the {MAX_REPORT_CELLS}-cell limit")

rows = []
for index in range(row_count):
Expand All @@ -463,9 +459,7 @@ def _rows_from_columnar(section: Any) -> list[dict[str, Any]]:
def _validate_report_rows(rows: list[dict[str, Any]]) -> None:
"""Reject row-oriented diagnostics that exceed report rendering budgets."""
if len(rows) > MAX_REPORT_ROWS:
raise ValueError(
f"report table rows exceed the {MAX_REPORT_ROWS}-row limit"
)
raise ValueError(f"report table rows exceed the {MAX_REPORT_ROWS}-row limit")

columns: set[str] = set()
cell_count = 0
Expand Down Expand Up @@ -646,7 +640,7 @@ def _css() -> str:
padding: 32px 20px 48px;
}

main:focus {
main:focus:not(:focus-visible) {
outline: none;
}

Expand Down Expand Up @@ -836,7 +830,7 @@ def _css() -> str:
border-radius: 8px;
}

.table-wrap:focus {
.table-wrap:focus:not(:focus-visible) {
outline: none;
}

Expand Down Expand Up @@ -925,6 +919,11 @@ def _css() -> str:
color: var(--teal);
}

.exact-values > summary:focus:not(:focus-visible),
.export-block > summary:focus:not(:focus-visible) {
outline: none;
}

.exact-values > summary:focus-visible,
.export-block > summary:focus-visible {
outline: 3px solid var(--teal);
Expand All @@ -951,7 +950,7 @@ def _css() -> str:
white-space: pre;
}

.export-block pre:focus {
.export-block pre:focus:not(:focus-visible) {
outline: none;
}

Expand Down
14 changes: 13 additions & 1 deletion tests/test_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@


def test_render_fit_diagnostics_report_has_sections(tmp_path):
"""Render populated fit diagnostics with the expected report sections."""
source = tmp_path / "fit_diagnostics.json"
out = tmp_path / "report.html"
source.write_text(
Expand Down Expand Up @@ -48,6 +49,7 @@ def test_render_fit_diagnostics_report_has_sections(tmp_path):


def test_render_dimensionality_report_has_best_candidate(tmp_path):
"""Render dimensionality diagnostics with the selected candidate."""
source = tmp_path / "dimension_diagnostics.json"
out = tmp_path / "dimensions.html"
source.write_text(
Expand All @@ -74,6 +76,7 @@ def test_render_dimensionality_report_has_best_candidate(tmp_path):


def test_render_dimensionality_report_summarizes_empty_candidates(tmp_path):
"""Summarize dimensionality diagnostics when no candidates are available."""
source = tmp_path / "dimension_diagnostics.json"
out = tmp_path / "dimensions.html"
source.write_text(
Expand All @@ -97,6 +100,7 @@ def test_render_dimensionality_report_summarizes_empty_candidates(tmp_path):


def test_render_report_summarizes_empty_metric_sections(tmp_path):
"""Summarize an empty metric section without rendering an empty panel."""
source = tmp_path / "fit_diagnostics.json"
out = tmp_path / "report.html"
source.write_text(
Expand All @@ -123,6 +127,7 @@ def test_render_report_summarizes_empty_metric_sections(tmp_path):


def test_render_dimensionality_report_summarizes_empty_best_candidate(tmp_path):
"""Summarize a dimensionality report whose best candidate has no metrics."""
source = tmp_path / "dimension_diagnostics.json"
out = tmp_path / "dimensions.html"
source.write_text(
Expand Down Expand Up @@ -150,6 +155,7 @@ def test_render_dimensionality_report_summarizes_empty_best_candidate(tmp_path):


def test_render_coverage_omits_empty_rendered_tables_column(tmp_path):
"""Omit the rendered-table coverage column when no table is present."""
source = tmp_path / "dimension_diagnostics.json"
out = tmp_path / "dimensions.html"
source.write_text(
Expand All @@ -173,6 +179,7 @@ def test_render_coverage_omits_empty_rendered_tables_column(tmp_path):


def test_render_table_section_omits_empty_chart_placeholder(tmp_path):
"""Avoid an empty chart placeholder when rows have no numeric values."""
source = tmp_path / "fit_diagnostics.json"
out = tmp_path / "report.html"
source.write_text(
Expand All @@ -194,6 +201,7 @@ def test_render_table_section_omits_empty_chart_placeholder(tmp_path):


def test_render_table_section_charts_later_numeric_rows(tmp_path):
"""Include a later numeric row when earlier rows are not chartable."""
source = tmp_path / "fit_diagnostics.json"
out = tmp_path / "report.html"
item_ids = list(range(13))
Expand All @@ -217,6 +225,7 @@ def test_render_table_section_charts_later_numeric_rows(tmp_path):


def test_render_report_rejects_unknown_payload(tmp_path):
"""Reject diagnostics JSON that has no supported report payload."""
source = tmp_path / "unknown.json"
out = tmp_path / "report.html"
source.write_text(json.dumps({"status": "ok"}), encoding="utf-8")
Expand All @@ -226,6 +235,7 @@ def test_render_report_rejects_unknown_payload(tmp_path):


def test_render_report_rejects_excessive_json_nesting(tmp_path):
"""Reject JSON that exceeds the configured nesting safety limit."""
source = tmp_path / "deep.json"
out = tmp_path / "report.html"
depth = MAX_JSON_NESTING_DEPTH + 1
Expand All @@ -238,6 +248,7 @@ def test_render_report_rejects_excessive_json_nesting(tmp_path):


def test_render_report_requires_html_output(tmp_path):
"""Reject output paths that do not use the HTML extension."""
source = tmp_path / "fit_diagnostics.json"
out = tmp_path / "report.txt"
source.write_text(json.dumps({"model_fit": {}}), encoding="utf-8")
Expand All @@ -247,6 +258,7 @@ def test_render_report_requires_html_output(tmp_path):


def test_render_table_region_has_keyboard_focus_style(tmp_path):
"""Keep keyboard focus styling while suppressing mouse-click outlines."""
source = tmp_path / "dimension_diagnostics.json"
out = tmp_path / "dimensions.html"
source.write_text(
Expand All @@ -265,7 +277,7 @@ def test_render_table_region_has_keyboard_focus_style(tmp_path):
assert 'aria-label="Candidate Comparison diagnostics table"' in html
assert 'tabindex="0"' in html
assert ".table-wrap:focus-visible" in html
assert ".table-wrap:focus {" in html
assert ".table-wrap:focus:not(:focus-visible) {" in html
assert "tbody tr:hover" in html
assert '<div class="bar-chart" aria-hidden="true">' in html
assert '<div class="bar-track" aria-hidden="true">' in html
8 changes: 5 additions & 3 deletions tests/test_report_focus_contrast.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ def test_hover_does_not_dim_unrelated_chart_or_table_content(tmp_path: Path) ->


def test_focus_containers_suppress_mouse_click_outlines(tmp_path: Path) -> None:
"""Semantic focus containers must suppress mouse click outlines."""
"""Pointer-focus suppression must preserve keyboard-visible focus fallback."""
html = _render_report(tmp_path)
assert ".table-wrap:focus {\n outline: none;\n}" in html
assert ".export-block pre:focus {\n outline: none;\n}" in html
assert ".table-wrap:focus:not(:focus-visible) {\n outline: none;\n}" in html
assert ".export-block pre:focus:not(:focus-visible) {\n outline: none;\n}" in html
assert ".table-wrap:focus {\n outline: none;\n}" not in html
assert ".export-block pre:focus {\n outline: none;\n}" not in html
32 changes: 32 additions & 0 deletions tests/test_report_numeric_motion_css.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,35 @@ def test_rendered_report_uses_tabular_numerals_without_opacity_transitions(tmp_p
reduced_motion = style.split("@media (prefers-reduced-motion: reduce)", 1)[1]
reduced_motion = reduced_motion.split("@media (max-width: 720px)", 1)[0]
assert "transition-duration: 0.01ms !important;" in reduced_motion


def test_rendered_report_preserves_keyboard_focus_on_main(tmp_path) -> None:
"""Pointer focus may be quiet, but keyboard focus on main must stay visible."""
source = tmp_path / "fit_diagnostics.json"
output = tmp_path / "report.html"
source.write_text(
json.dumps(
{
"model_fit": {"loglik": -3.2, "deviance": 6.4},
"itemfit": {
"item_id": ["A", "B"],
"outfit_mnsq": [1.0, 1.2],
"observed_count": [4, 4],
},
}
),
encoding="utf-8",
)

render_diagnostics_report(source, output)

html = output.read_text(encoding="utf-8")
style = html.split("<style>", 1)[1].split("</style>", 1)[0]

pointer_rule = _rule_body(style, "main:focus:not(:focus-visible)")
assert "outline: none;" in pointer_rule
keyboard_rule = _rule_body(style, "main:focus-visible")
assert "outline: 3px solid var(--teal);" in keyboard_rule
assert "outline-offset: 3px;" in keyboard_rule
assert "outline: none;" not in keyboard_rule
assert "\nmain:focus {" not in style
64 changes: 64 additions & 0 deletions tests/test_report_summary_focus.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
"""Pointer-versus-keyboard focus regressions for diagnostics disclosures."""

from __future__ import annotations

import json
from pathlib import Path

from fast_mlsirm.report import render_diagnostics_report


def _render_report(tmp_path: Path) -> str:
"""Render one report containing exact-value and export disclosures."""
source = tmp_path / "diagnostics.json"
output = tmp_path / "diagnostics.html"
source.write_text(
json.dumps(
{
"model_fit": {"loglik": -3.2},
"itemfit": {
"item_id": ["item_alpha"],
"outfit_mnsq": [1.0],
"observed_count": [120],
},
}
),
encoding="utf-8",
)
render_diagnostics_report(source, output, title="Disclosure Focus Review")
return output.read_text(encoding="utf-8")


def test_disclosure_summary_suppresses_pointer_focus_without_blanket_focus_rule(
tmp_path: Path,
) -> None:
"""Pointer focus is suppressed without erasing generic keyboard focus."""
html = _render_report(tmp_path)
selector = (
".exact-values > summary:focus:not(:focus-visible),\n"
".export-block > summary:focus:not(:focus-visible) {"
)

assert html.count(selector) == 1
assert ".exact-values > summary:focus,\n.export-block > summary:focus {" not in html
rule = html.split(selector, maxsplit=1)[1].split("}", maxsplit=1)[0]
assert "outline: none;" in rule


def test_disclosure_summary_retains_keyboard_focus_indicator(tmp_path: Path) -> None:
"""The pointer rule must not erase the explicit keyboard focus treatment."""
html = _render_report(tmp_path)
pointer_selector = (
".exact-values > summary:focus:not(:focus-visible),\n"
".export-block > summary:focus:not(:focus-visible) {"
)
keyboard_selector = (
".exact-values > summary:focus-visible,\n"
".export-block > summary:focus-visible {"
)

assert html.count(keyboard_selector) == 1
assert html.index(pointer_selector) < html.index(keyboard_selector)
rule = html.split(keyboard_selector, maxsplit=1)[1].split("}", maxsplit=1)[0]
assert "outline: 3px solid var(--teal);" in rule
assert "outline-offset: 2px;" in rule
71 changes: 71 additions & 0 deletions tests/test_report_title_callback_safety.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
"""Regression tests for the generic diagnostics-report title trust boundary."""

from __future__ import annotations

import json

import pytest

from fast_mlsirm.report import render_diagnostics_report


class _HostileTitle(str):
"""String subclass whose title callbacks must never be dispatched."""

calls = 0

def __bool__(self) -> bool:
"""Record forbidden truth-value dispatch during title resolution."""
type(self).calls += 1
raise RuntimeError("private title truth callback")

def replace(self, *args: object, **kwargs: object) -> str:
"""Record forbidden replacement dispatch during HTML escaping."""
type(self).calls += 1
raise RuntimeError("private title replacement callback")


def _write_fit_payload(path) -> None:
"""Write one minimal valid fit-diagnostics payload."""
path.write_text(json.dumps({"model_fit": {"loglik": -1.0}}), encoding="utf-8")


def test_generic_report_rejects_title_subclass_without_callbacks(tmp_path) -> None:
"""A caller string subclass cannot execute while a report title is admitted."""
source = tmp_path / "fit.json"
output = tmp_path / "fit.html"
_write_fit_payload(source)
_HostileTitle.calls = 0

with pytest.raises(ValueError, match="title must be a string or None"):
render_diagnostics_report(source, output, title=_HostileTitle("Private title"))

assert _HostileTitle.calls == 0
assert not output.exists()


def test_generic_report_empty_builtin_title_keeps_default(tmp_path) -> None:
"""An empty exact built-in title preserves the existing default-title contract."""
source = tmp_path / "fit.json"
output = tmp_path / "fit.html"
_write_fit_payload(source)

render_diagnostics_report(source, output, title="")

html = output.read_text(encoding="utf-8")
assert "<title>Fit Diagnostics Report</title>" in html
assert '<h1 id="hero-heading">Fit Diagnostics Report</h1>' in html


def test_generic_report_builtin_title_remains_html_escaped(tmp_path) -> None:
"""An admitted exact built-in title remains escaped in every HTML title surface."""
source = tmp_path / "fit.json"
output = tmp_path / "fit.html"
_write_fit_payload(source)

render_diagnostics_report(source, output, title='<Admin & "Ops">')

html = output.read_text(encoding="utf-8")
assert "<title>&lt;Admin &amp; &quot;Ops&quot;&gt;</title>" in html
assert '<h1 id="hero-heading">&lt;Admin &amp; &quot;Ops&quot;&gt;</h1>' in html
assert '<Admin & "Ops">' not in html
Loading