Skip to content

[Review] Daily Deep Review — 2026-04-30: Repo stale 18 days, CI fully broken, 4 new issues filed #242

Description

@evan-zhang11

Repository Status

Metric Status
Last commit 2026-04-12 (18 days stale)
Open PRs 8 (all blocked by CI)
Nightly CI ❌ Failing (cargo fmt)
Security CI ❌ Failing (cargo audit + npm audit)
Open Issues 34+

Recent Commits (since last review)

All by @sharkAndshark:

  1. 9f95d3d (Apr 12) — Merge PR feat(icons): add PNG/SVG icon upload and management #201: icon upload/management
  2. 56c4146 (Apr 11) — Fix SVG dimension precision loss
  3. 518ab3a (Apr 9) — Extract shared storage path helpers
  4. edd0fae (Apr 5) — Icon upload, management, preview feature
  5. 1bf1cc5 (Apr 4) — Fix glyph paths, handle corrupted PBF

New Issues Filed This Review

# Title Severity
#238 cargo fmt fails on font_handlers.rs + icon_handlers.rs (blocks all PRs) 🔴 Critical
#239 cargo audit cannot parse Cargo.lock (num-rational) 🔴 High
#240 4 npm vulnerabilities (Vite path traversal, picomatch ReDoS, etc.) 🟡 Medium
#241 import.rs grown to 1584 lines — needs module split 🟢 Low

Open PR Summary

PR Status Notes
#232 ⚠️ Blocked Cargo minor-patch, has test failures
#233 ✅ Ready sha2 0.10→0.11 (major), blocked only by main CI
#234 ✅ Ready resvg 0.44→0.47 (major), blocked only by main CI
#216 ✅ Ready npm minor-patch, blocked only by main CI
#215 ✅ Ready Rust 1.94→1.95, blocked only by main CI
#214 ✅ Ready action-gh-release v2→v3, blocked only by main CI
#173 ❌ Failing Server file import, merge conflicts + CI failures
#65 ❌ Failing tower-sessions 0.14→0.15

Priority Actions

  1. Fix cargo fmt (ci: cargo fmt fails on font_handlers.rs and icon_handlers.rs (rustfmt line-length enforcement) #238) — one-command fix, unblocks all PRs
  2. Fix Cargo.lock (ci: cargo audit fails — Cargo.lock has unresolvable num-rational dependency #239) — regenerate lockfile
  3. npm audit fix (security: 4 npm vulnerabilities — Vite path traversal, picomatch ReDoS, postcss XSS, proto pollution #240) — cd frontend && npm audit fix
  4. After CI is green, merge ready PRs: chore(deps): bump the npm-minor-patch group across 1 directory with 10 updates #216, chore(deps): bump rust from 1.94-slim-bookworm to 1.95-slim-bookworm #215, chore(deps): bump softprops/action-gh-release from 2 to 3 #214

Code Quality Observations

  • SVG XSS (security: SVG icon served inline enables stored XSS #237, filed yesterday): get_icon_file serves SVG files with image/svg+xml content type without sanitization — enables stored XSS. Should use Content-Disposition: attachment or sanitize SVG content.
  • Large files: handlers.rs (1626), workspace_handlers.rs (1259), import.rs (1584), App.jsx (2111) — all need splitting
  • SQL safety: import.rs uses format!() for SQL extensively but mitigates via escape_sql_string() and column name normalization. The safe_table_name comes from server-generated IDs, not user input. Acceptable but worth documenting.
  • Test coverage: icon_handlers has only serialization tests (test: no automated tests for icon upload/management handlers #236), no integration tests for upload/delete/get endpoints

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions