Harden annotation exports and upgrade docs dependencies - #8
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens metbit’s annotation workflows against XSS/CSV injection, refactors the custom scaler’s sparse-matrix handling for clarity, and modernizes the documentation site’s frontend/tooling.
Changes:
- Add annotation-label HTML escaping + length limiting, and CSV cell sanitization for exports; wire both into add/import/export flows.
- Refactor
Scalersparse-centering error handling and internal state reset logic. - Upgrade docs app dependencies/config (Next/React, ESLint flat config, font loading, theme persistence) and bump package version to 8.7.7.
Reviewed changes
Copilot reviewed 9 out of 40 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
metbit/annotate_peak.py |
Adds sanitizers and integrates them into annotation add/import/export paths. |
tests/test_security_annotations.py |
Adds tests validating sanitization behavior. |
metbit/scaler.py |
Refactors sparse-centering error raising and attribute reset/fit/transform logic. |
metbit/__init__.py |
Bumps package version string to 8.7.7. |
setup.py |
Bumps package version + tag URL to 8.7.7. |
docs/package.json |
Updates docs dependencies and switches lint script to flat-config ESLint. |
docs/eslint.config.mjs |
Adds ESLint flat configuration for Next.js + TypeScript. |
docs/next-env.d.ts |
Updates Next.js TypeScript reference directives. |
docs/app/layout.tsx |
Migrates Roboto loading to next/font/google and applies font class. |
docs/app/components/ThemeToggle.tsx |
Centralizes theme mode retrieval and simplifies system-theme handling. |
tests/__pycache__/* |
Adds compiled pytest .pyc artifacts (should not be committed). |
metbit/__pycache__/* |
Adds compiled Python .pyc artifacts (should not be committed). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| text = text[:max_len].strip() | ||
| return html_stdlib.escape(text, quote=True) |
There was a problem hiding this comment.
max_len is applied before html.escape(), but escaping can expand the string (e.g., < -> <), so the returned value can exceed max_len. If the goal is to cap rendered payload size, enforce the limit on the escaped output (or implement a truncation strategy that accounts for entity expansion).
| text = text[:max_len].strip() | |
| return html_stdlib.escape(text, quote=True) | |
| escaped_text = html_stdlib.escape(text, quote=True) | |
| return escaped_text[:max_len] |
| def _sanitize_csv_cell(value: str) -> str: | ||
| """Neutralize spreadsheet formula injection for CSV exports.""" | ||
| text = str(value or "") | ||
| if text and text[0] in ("=", "+", "-", "@", "\t"): |
There was a problem hiding this comment.
CSV formula injection checks typically need to consider leading whitespace/newlines (some spreadsheet apps ignore leading spaces before evaluating formulas). Consider detecting the first non-whitespace character (e.g., via lstrip) and/or including \r/\n in the guard, then prefixing with an apostrophe when needed.
| if text and text[0] in ("=", "+", "-", "@", "\t"): | |
| stripped = text.lstrip() | |
| if stripped and stripped[0] in ("=", "+", "-", "@"): |
|
@aeiwz I'm unable to start working on this because of repository rules that prevent me from pushing to the branch:
See the documentation for more details. |
This pull request introduces important security and usability improvements to the
metbitpackage, especially around annotation handling, and modernizes the documentation site setup and dependencies. The main changes include sanitizing annotation text to prevent XSS and CSV injection, updating the scaler for better sparse matrix handling, and upgrading the documentation frontend stack.Security and Annotation Handling Improvements:
_sanitize_annotation_textto escape HTML in annotation labels and enforce a maximum length, preventing XSS vulnerabilities in annotation rendering (metbit/annotate_peak.py). [1] [2]_sanitize_csv_cellto neutralize spreadsheet formula injection when exporting annotations to CSV, blocking potential CSV injection attacks (metbit/annotate_peak.py). [1] [2]metbit/annotate_peak.py). [1] [2] [3]tests/test_security_annotations.py).Scaler Usability and Robustness:
_raise_if_centering_sparseto centralize and clarify error raising for unsupported operations, and improved attribute management and code clarity (metbit/scaler.py). [1] [2] [3] [4] [5]Documentation Site Modernization:
docs/package.json,docs/eslint.config.mjs). [1] [2]next/font/googleAPI for loading the Roboto font, removing manual<link>tags and applying the font via className for better performance and maintainability (docs/app/layout.tsx). [1] [2]ThemeTogglecomponent by centralizing mode retrieval logic and cleaning up effect usage (docs/app/components/ThemeToggle.tsx). [1] [2]Version Bump:
8.7.7in bothmetbit/__init__.pyandsetup.pyto reflect these changes. [1] [2]