Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
3 changes: 3 additions & 0 deletions .jules/palette.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@
## 2024-07-13 - CLI Debugging Stack Traces
**Learning:** Adding a `FAST_MLSIRM_DEBUG` bypass to user-friendly `try/except` blocks is crucial for DX. Otherwise, unexpected runtime errors during development will be swallowed into generic stderr messages, hiding the stack trace needed to actually fix the bug.
**Action:** When adding `try-except` blocks to Python CLI subcommands to improve Developer Experience (DX) by preventing raw tracebacks for users, include a debug bypass (e.g., `if os.environ.get("FAST_MLSIRM_DEBUG"): raise`) in *all* catch blocks (including `RuntimeError` and `Exception`) to ensure tracebacks aren't swallowed during local development and debugging.
## 2024-05-15 - Accessible Data Tables
Comment thread
seonghobae marked this conversation as resolved.
**Learning:** Using <th scope="row"> for the first column improves screen reader experience, but requires careful CSS updates to prevent visual regressions when replacing <td> tags.
Comment thread
seonghobae marked this conversation as resolved.
**Action:** Always update table CSS rules (like borders and fonts) to include tbody th when enhancing accessibility.
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ Explicitly defining `allow_pickle=False` is a robust defense-in-depth practice.
**Vulnerability:** MD5 hashing in `fast_mlsirm/report.py` triggered a high severity warning by Bandit, because by default it is assumed to be used for security purposes which is unsafe due to weak hashing.
**Learning:** For non-security purposes like generating unique dom ids, `hashlib.md5()` triggers a vulnerability warning unless `usedforsecurity=False` is passed. This allows bypassing FIPS compliance limitations as well as suppressing false positive warnings.
**Prevention:** Always add `usedforsecurity=False` parameter to `hashlib.md5` and other weak hashing functions unless they are genuinely used for secure cryptography (which they shouldn't be).
## 2024-05-15 - Strix Availability Remediation (vuln-0001)
**Vulnerability:** Diagnostics HTML renderer previously materialized entire attacker-sized JSON collections without limits, leading to excessive memory (CWE-400, CVSS 5.5).
**Learning:** Rendering components that accept JSON input must bound the size of inputs and collection iterations to prevent DoS.
**Prevention:** Apply a strict 16 MiB boundary check on source JSON read and bound column iterations directly in row materialization (`_rows_from_columnar`).
69 changes: 48 additions & 21 deletions python/fast_mlsirm/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ def render_diagnostics_report(
"""Render saved diagnostics JSON as a standalone HTML report."""

source = Path(diagnostics_path)
if source.stat().st_size > 16 * 1024 * 1024:
raise ValueError("diagnostics JSON exceeds 16 MiB limit")
payload = json.loads(source.read_text(encoding="utf-8"))
Comment thread
seonghobae marked this conversation as resolved.
if not isinstance(payload, dict):
raise ValueError("diagnostics JSON must contain an object")
Expand Down Expand Up @@ -154,7 +156,12 @@ def _render_dimensionality_report(payload: dict[str, Any]) -> list[str]:
if not isinstance(candidates, list):
raise ValueError("candidates must be a list")

rows = [row for row in candidates if isinstance(row, dict)]
rows = []
for row in candidates:
if isinstance(row, dict):
rows.append(row)
if len(rows) >= 100:
break
sections = []
no_metric_sections = []
best_section = _metric_section("Best Candidate", best)
Expand All @@ -180,7 +187,7 @@ def _render_dimensionality_report(payload: dict[str, Any]) -> list[str]:

def _metric_section(heading: str, metrics: dict[str, Any]) -> str | None:
cards = []
for key, value in metrics.items():
for key, value in list(metrics.items())[:24]:
Comment thread
seonghobae marked this conversation as resolved.
cards.append(
Comment thread
seonghobae marked this conversation as resolved.
"\n".join(
[
Expand Down Expand Up @@ -269,11 +276,16 @@ def _bar_chart(rows: list[dict[str, Any]], value_key: str | None) -> str:
return ""
if not rows:
return ""
numeric_rows = [
(index, row, float(row[value_key]))
for index, row in enumerate(rows)
if _is_number(row.get(value_key))
]
numeric_rows = []
for index, row in enumerate(rows):
val = row.get(value_key)
if _is_number(val):
try:
numeric_rows.append((index, row, float(val)))
except (OverflowError, ValueError, TypeError):
pass
if len(numeric_rows) >= 12:
break
values = [value for _, _, value in numeric_rows]
if not values:
return ""
Expand Down Expand Up @@ -318,8 +330,10 @@ def _table(rows: list[dict[str, Any]], *, label: str, limit: int = 12) -> str:
body_rows = []
for row in rows[:limit]:
cells = "".join(
f"<td>{escape(_format_value(row.get(column, '')))}</td>"
for column in columns
f'<th scope="row">{escape(_format_value(row.get(column, "")))}</th>'
if i == 0
else f"<td>{escape(_format_value(row.get(column, '')))}</td>"
for i, column in enumerate(columns)
)
body_rows.append(f"<tr>{cells}</tr>")

Expand Down Expand Up @@ -356,14 +370,14 @@ def _rows_from_columnar(section: Any) -> list[dict[str, Any]]:
if not isinstance(section, dict) or not section:
return []

columns = list(section)
columns = list(section)[:20]
Comment thread
seonghobae marked this conversation as resolved.
lengths = [_value_length(section[column]) for column in columns]
row_count = max(lengths) if lengths else 0
if row_count == 0:
return []

rows = []
for index in range(row_count):
for index in range(min(row_count, 100)):
row = {}
Comment thread
seonghobae marked this conversation as resolved.
for column in columns:
row[column] = _index_value(section[column], index)
Expand All @@ -377,6 +391,8 @@ def _columns(rows: list[dict[str, Any]]) -> list[str]:
for key in row:
if key not in ordered:
ordered.append(key)
if len(ordered) >= 20:
return ordered
return ordered


Expand Down Expand Up @@ -416,14 +432,17 @@ def _format_value(value: Any) -> str:
if isinstance(value, bool):
return "true" if value else "false"
if isinstance(value, int):
return str(value)
s = str(value)
return s if len(s) <= 100 else s[:100] + "..."
if isinstance(value, float):
if math.isnan(value) or math.isinf(value):
return str(value)
return f"{value:.4g}"
if value is None:
return ""
return str(value)
s = str(value)
return s if len(s) <= 100 else s[:100] + "..."



def _format_label_value(value: Any) -> str:
Expand All @@ -433,11 +452,12 @@ def _format_label_value(value: Any) -> str:


def _is_number(value: Any) -> bool:
return (
isinstance(value, (int, float))
and not isinstance(value, bool)
and math.isfinite(float(value))
)
if not isinstance(value, (int, float)) or isinstance(value, bool):
return False
try:
return math.isfinite(float(value))
except (OverflowError, ValueError, TypeError):
return False


def _content_security_policy() -> str:
Expand Down Expand Up @@ -679,21 +699,28 @@ def _css() -> str:
border: 0;
}

th,
thead th,
tbody th,
td {
padding: 10px 12px;
text-align: left;
border-bottom: 1px solid var(--line);
white-space: nowrap;
font-variant-numeric: tabular-nums;
}

th {
thead th {
background: #f1f4ef;
color: #2f3437;
font-size: 0.8rem;
}

tr:last-child td {
tbody th {
font-weight: normal;
}

tr:last-child td,
tr:last-child tbody th {
border-bottom: 0;
Comment thread
seonghobae marked this conversation as resolved.
}
Comment thread
seonghobae marked this conversation as resolved.

Expand Down
Loading