-
Notifications
You must be signed in to change notification settings - Fork 0
Migrate from unmaintained xlsx to exceljs to resolve Prototype Pollution vulnerability #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
Co-authored-by: granatonatalia <[email protected]>
Co-authored-by: granatonatalia <[email protected]>
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR resolves a Prototype Pollution security vulnerability by migrating from the unmaintained xlsx package (v0.18.5) to the actively maintained exceljs library (v4.4.0). The migration updates the Excel export functionality to use ExcelJS's API while maintaining the same export behavior.
Key changes:
- Replaced vulnerable
xlsxdependency withexceljs@^4.4.0 - Converted
exportToExcel()method to async with enhanced error handling and column auto-sizing - Maintained identical export behavior and filename for end users
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| package.json | Removed xlsx@^0.18.5 and added exceljs@^4.4.0 |
| src/app/pages/mapping/mapping.component.ts | Updated import statement, converted exportToExcel() to use ExcelJS API with DOM-based table extraction, added column auto-sizing constants and logic, converted method to async |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| worksheet.addRow(rowData); | ||
| }); | ||
|
|
||
| // Auto-fit columns (optional, improves readability) |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states "Auto-fit columns (optional, improves readability)" but this code is not optional - it's being executed unconditionally. The comment should be updated to reflect that this is the actual behavior, not an optional feature. Consider changing to "Auto-fit columns to improve readability" or removing the "(optional, improves readability)" portion.
| // Auto-fit columns (optional, improves readability) | |
| // Auto-fit columns to improve readability |
User description
The xlsx package (v0.18.5) is unmaintained and has a known Prototype Pollution vulnerability (CVE) with no patch available. While the app only exports (not imports) Excel files, removing the vulnerable dependency is necessary.
Changes
Dependencies
[email protected][email protected](actively maintained, 2M+ weekly downloads)Code (
src/app/pages/mapping/mapping.component.ts)XLSX→ExcelJSexportToExcel()to asyncXLSX.utils.table_to_sheet()with DOM-based table extractionBefore:
After:
Security Impact
Resolves Dependabot alert #3. CodeQL scan shows 0 new alerts.
Original prompt
Security Issue: Prototype Pollution in xlsx Package
Problem
Dependabot alert #3 identifies a Prototype Pollution vulnerability in the
xlsx(SheetJS) package. All versions through 0.19.2 are vulnerable, and the npm package is no longer maintained.Current version:
[email protected]Status: No patched version available on npm
Current Usage Analysis
The
xlsxpackage is currently used in only one location:src/app/pages/mapping/mapping.component.tsexportToExcel()(lines 160-167)Note: Since the application only exports data and doesn't import/read Excel files, it is NOT currently vulnerable to the Prototype Pollution attack. However, migration is necessary because:
Solution: Migrate to ExcelJS
Replace
xlsxwithexceljs, an actively maintained alternative with similar functionality.Changes Required
1. Update package.json
"xlsx": "^0.18.5""exceljs": "^4.4.0"2. Update src/app/pages/mapping/mapping.component.ts
Current code (lines 1-6, 160-167):
Replace with ExcelJS equivalent:
3. Update template (if needed)
If the button/trigger for export uses
(click)="exportToExcel()", it should remain unchanged. The function signature changes from sync to async, but Angular handles this automatically.Acceptance Criteria
xlsxpackage removed frompackage.jsonexceljspackage added topackage.json(version 4.4.0 or higher)package-lock.jsonupdatedimport * as XLSX from 'xlsx'toimport * as ExcelJS from 'exceljs'exportToExcel()method updated to use ExcelJS APInpm run build)Testing
After applying changes:
Manual testing:
DSOMM - Activities.xlsxdownloadsAdditional Notes
This pull request was created from Copilot chat.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
PR Type
Bug fix, Enhancement
Description
Migrate from unmaintained xlsx to actively maintained exceljs library
Resolve Prototype Pollution vulnerability (CVE) in xlsx package
Refactor exportToExcel() to async with DOM-based table extraction
Add column auto-sizing with configurable width constraints
Diagram Walkthrough
File Walkthrough
package.json
Replace xlsx with exceljs dependencypackage.json
[email protected]dependency[email protected]as replacementmapping.component.ts
Refactor Excel export to use ExcelJS librarysrc/app/pages/mapping/mapping.component.ts
XLSXtoExcelJSexportToExcel()method to asyncquerySelectorAll
(DEFAULT_COLUMN_WIDTH, COLUMN_PADDING, MAX_COLUMN_WIDTH)
XLSX.writeFile