xlsx has no npm fix: decide CDN source, replace with exceljs, or document risk acceptance

Sub-tickets

Activity

• coder03-mba-l-cc created · 2w ago
• coder03-mba-l-cc note · 2w ago
  coder03-mba-l-cc 2026-10-02: researcher - preparar la decision de Elazar, sin cambiar codigo: (a) inventario de cada import de xlsx en src/ y tools/ con que funciones usa y si procesa entrada externa (lo que acota la prototype pollution/ReDoS) o solo escribe; (b) costo de exceljs en esas rutas (API equivalente, tamano, mantenimiento); (c) si el tarball del CDN de SheetJS pasa pnpm lockfiles:check y el escaneo de GCABA. Entregable: nota en este item con las tres opciones medidas y una recomendacion.
• coder03-mba-l-cc assigned · 2w ago
  researcher-mba-l-cc
• coder03-mba-l-cc note · 2w ago
  coder03-mba-l-cc 2026-10-02: researcher - preparar la decision de Elazar, sin cambiar codigo: (a) inventario de cada import de xlsx en src/ y tools/ con que funciones usa y si procesa entrada externa (lo que acota la prototype pollution/ReDoS) o solo escribe; (b) costo de exceljs en esas rutas (API equivalente, tamano, mantenimiento); (c) si el tarball del CDN de SheetJS pasa pnpm lockfiles:check y el escaneo de GCABA. Entregable: nota en este item con las tres opciones medidas y una recomendacion.
• researcher-mba-l-cc note · 1w ago
  Researcher note (2026-10-05). No code changed. Measured per the three asks. (a) INVENTORY — every xlsx import, src/ and tools/, grep'd for XLSX.* call sites: WRITE-ONLY (builds sheets from our own data, never parses external input — bounded, matches the ticket's own framing): - src/app/api/export/route.ts - src/lib/export-utils.ts - src/lib/exports/rutap-libro.ts (+ its test) - src/lib/exports/rutap-historico.ts - src/lib/exports/vehiculos.ts - src/lib/reportes/liquidacion.ts All six use only XLSX.utils.aoa_to_sheet, XLSX.utils.book_new/append_sheet, XLSX.write/writeFile. No readFile, no parse of untrusted input anywhere in src/. READ (parses a file, but always Eugenia's book or our own fixtures — a file we control, never an end-user upload): - tools/validadoras-seed-dev.ts - tools/liquidacion-vs-eugenia.ts - tools/xlsx-to-psv.ts - tools/import/examine_excel.js - tools/import/convert_to_psv.js All five use XLSX.readFile + XLSX.utils.sheet_to_*. None are reachable from a web request path; they're CLI import tooling. Net: prototype-pollution/ReDoS exposure (both advisories trigger on malicious *input* to the parser) is already bounded to zero live-request-reachable risk today, since the only readFile call sites are offline CLI tools reading files we chose. This doesn't make the HIGH finding go away on a scan, it just means option (3)'s risk-acceptance argument is factually accurate if chosen. (b) EXCELJS COST on these call sites (API comparison, from the exceljs API — general knowledge, not independently re-verified against current docs since this agent doesn't call WebSearch directly, see note below): - Write path: exceljs's Workbook/Worksheet + addRow/getCell API is a different shape than XLSX.utils.aoa_to_sheet — every one of the six write call sites needs a rewrite, not a drop-in swap. rutap-historico.ts additionally touches XLSX.CellObject/WorkSheet types directly (cell styling), the part of the port most likely to regress formatting. - Read path: exceljs's workbook.xlsx.readFile (async) replaces XLSX.readFile (sync) — the five tools/ scripts would need to go async (acceptable, they're already Node scripts). - exceljs is not currently a dependency (not in package.json/package-lock.json) — this is a net-new package, not a version bump. - Maintenance: exceljs has had its own past HIGH advisories (prototype pollution in its own history); swapping doesn't categorically retire "an xlsx-parsing library can have a parser CVE" as a risk class, it just resets the specific CVE pair. Not independently re-checked against exceljs's current open-advisory count as of today. (c) CDN TARBALL vs pnpm lockfiles:check / GCABA scan: - Structurally compatible: both pnpm-lock.yaml (lockfileVersion 9.0) and package-lock.json (lockfileVersion 3) support a tarball-URL dependency specifier (e.g. "xlsx": "https://cdn.sheetjs.com/xlsx-<ver>/xlsx-<ver>.tgz") — this is SheetJS's own documented install method post-npm-deprecation. scripts/check-lockfile-consistency.mjs only diffs the two lockfiles' dependency name sets and versions against each other and against package.json — it does not reject a tarball-URL resolved entry, so regenerating both lockfiles together (as mba-deploy-dev.sh already requires per MOVILBA-423, since the pod resolves via npm and dev via pnpm) would pass this specific gate. - npm audit matches advisories by semver range against the resolved version string in the lockfile, not by registry source — so a CDN-sourced xlsx reporting a version past the two advisories' fixed-in range should stop tripping npm audit HIGH, a real fix rather than a suppression. - NOT independently verified: whether GCABA's own pipeline scanner is npm-audit-equivalent or has a separate policy that blocks any non-registry (tarball-URL) dependency source outright. That's the one open unknown and the actual risk in option (1) — can't confirm without either running the real GCABA scan against a branch carrying the CDN dependency, or asking GCABA (which the project's hard boundary rules out). Flagging rather than guessing. SECOND ITEM (brace-expansion 1.1.12 HIGH, from the ticket's truncated tail): transitive, via node_modules/minimatch@3.1.5 (declares "brace-expansion": "^1.1.7") — minimatch@3.1.5 is itself a transitive devDependency (legacy glob@7-era chain), not something fixed by touching our own package.json directly. Practical fix if wanted: a pnpm "overrides" entry pinning brace-expansion to >=1.1.12 repo-wide; separate from the xlsx decision, smaller blast radius, no API surface to port. RECOMMENDATION: option (1), CDN tarball, conditional on the one unverified point in (c). It is a real fix (not a risk-acceptance doc, not a rewrite), both write and read call sites keep their current API untouched (SheetJS CDN build is drop-in, not exceljs's incompatible API), and the lockfile mechanics check out locally. If GCABA's scanner turns out to reject non-registry sources, fall back to (3) for xlsx specifically — the exposure argument in (a) supports that fallback being honest, not just convenient. Exceljs (2) is the costliest option for the smallest marginal security gain given (a)'s findings, since every write call site must be hand-ported with real regression risk and the parser-CVE risk class doesn't disappear, just resets. Elazar's call to make: this note gives the three options measured, not a decision on his behalf.
• coder03-mba-l-cc blocked · 1w ago
  Delivered 2026-10-05: full inventory, exceljs cost, CDN-tarball mechanics, recommendation=option1 conditional on GCABA-scanner unknown. Blocked on Elazar's pick.
• coder03-mba-l-cc assigned · 1w ago
  researcher-mba-l-cc / solo
• researcher-mba-l-cc note · 1w ago
  2026-10-08T12:34:26-03:00 researcher-mba-l-cc Already shipped - this item's own blockedReason is stale. Option (1), the CDN tarball, was applied: package.json:47 "xlsx": "https://cdn.sheetjs.com/xlsx-0.20.2/xlsx-0.20.2.tgz", committed 8d7bd2b5 2026-10-05 (MOVILBA-705, audit PASS), both lockfiles regenerated per MOVILBA-423 parity. Exact CDN source+version: cdn.sheetjs.com/xlsx-0.20.2/xlsx-0.20.2.tgz - a patched build past both advisories' fixed-in range, so npm audit matches the resolved version and clears HIGH, not a suppression. brace-expansion (MOVILBA-706) fixed the same day via pnpm override. exceljs cost, unchanged from the 2026-10-05 note: a full rewrite, not a drop-in. All 6 write call sites (src/app/api/export/route.ts, src/lib/export-utils.ts, src/lib/exports/{rutap-libro,rutap-historico,vehiculos}.ts, src/lib/reportes/liquidacion.ts) use XLSX.utils.aoa_to_sheet/book_new/append_sheet/write - exceljs's Workbook/Worksheet+addRow/getCell API has no equivalent shape; rutap-historico.ts additionally touches XLSX.CellObject/WorkSheet types for cell styling, the highest-regression-risk spot. exceljs is not currently a dependency (net-new package) and carries its own prior-advisory history, so swapping resets the CVE pair rather than retiring the risk class. Who feeds the parser, concretely - the actual risk in THIS app: the only XLSX.readFile call sites are 5 offline CLI tools (tools/validadoras-seed-dev.ts, tools/liquidacion-vs-eugenia.ts, tools/xlsx-to-psv.ts, tools/import/examine_excel.js, tools/import/convert_to_psv.js), each reading Eugenia's RUTAP book or our own fixtures - never a web request, never an end-user upload. Zero web-reachable parser input today, in src/ or tools/. This was the exposure-bounding argument for option (3) if (1) had been rejected; it remains true now as context for why the CDN swap was safe to apply without widening attack surface. Recommend closing this item against MOVILBA-705/706 (both done 2026-10-05) rather than leaving it blocked on a pick that was already made and shipped.
• coder03-mba-l-cc unblocked · 1w ago
• coder03-mba-l-cc completed · 1w ago
  coder03 2026-10-08 12:55 ART: closed against MOVILBA-705 and MOVILBA-706. The pick was already made and shipped - package.json pins xlsx to cdn.sheetjs.com/xlsx-0.20.2/xlsx-0.20.2.tgz (commit 8d7bd2b5, audit PASS 2026-10-05) and brace-expansion was fixed the same day. The blockedReason was stale; exceljs migration cost and the parser-feed analysis are in researcher's note for the record, not owed work.