fix(deps): replace xlsx with read-excel-file to fix high severity vulnerability #6357
Conversation
|
⏩ No test execution environment matched (3971fc5) View output ↗ View check history
|
![promptfoo-scanner[bot]](https://avatars.githubusercontent.com/in/2172945?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR which replaces the xlsx library with read-excel-file to address a dependency vulnerability. The changes are focused on Excel file parsing functionality with no LLM-related code modifications.
Minimum severity threshold for this scan: Medium
|
👍 All Clear No LLM security vulnerabilities were found in this PR. |
JustinBeckwith
left a comment
JustinBeckwith
left a comment
There was a problem hiding this comment.
Thank you. I tried doing this before I started, but wasn't quite confident enough in the codebase yet.
📝 WalkthroughWalkthroughThis pull request replaces the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
examples/simple-csv/promptfooconfig.yaml (1)
8-9: Excel alternative comment now correctly referencesread-excel-fileThe updated comment is aligned with the new Excel reader and the rest of the config (schema line, field order,
file://usage, short description) still looks good. When you next touch this example, consider bumping the provider model to one of the current recommended IDs (e.g.,openai:o3-mini/openai:gpt-4o-mini) to match the examples guidelines, but that’s not blocking this PR.package.json (1)
169-170: read-excel-file wiring matches the new XLSX code; clarify sharp’s role and avoid duplicationAdding
read-excel-fileto devDependencies and optionalDependencies is consistent with treating Excel support as optional while keeping it available for local development.
sharpnow appears in bothdevDependenciesandoptionalDependencies, which is redundant. It’s worth deciding whethersharpis truly a runtime-optional feature (keep only inoptionalDependencies) or purely a dev/test tool (keep only indevDependencies), and removing the extra entry accordingly.Also applies to: 229-230
src/util/xlsx.ts (1)
15-32: Dynamic import + sheet/data validation look solid; a couple of tiny cleanups possibleThe new
parseXlsxFileflow is coherent:
- Validates file existence up front.
- Dynamically imports
read-excel-file/nodewith a clear “install this optional dependency” message.- Handles
file.xlsx#SheetNameandfile.xlsx#1selection correctly againstreadSheetNames, with good range/name errors.- Maps
[row][col]intoCsvRowobjects with string values and''defaults, then enforces non-empty headers, at least one data row, and at least one non-empty cell anywhere.Two small nits you might consider (non‑blocking):
- The
knownErrorsentry'contains only empty data'is redundant, since all of your own messages for that case start withSheet "…and are already caught by the'Sheet "'prefix; you can safely drop it or switch the check toincludesfor that one entry.- The “read-excel-file is not installed…” string is duplicated in the import
catchand in the bottomcatch; extracting it to a constant would make future edits less error‑prone.Functionally this implementation matches the updated tests.
Also applies to: 37-101, 112-132
test/util/testCaseReader.test.ts (1)
567-600: Integration XLSX test will usually skip due to mockedfs.existsSyncBecause this file globally mocks
fs(withexistsSync: jest.fn()), the integration test’sconst fs = require('fs'); if (fs.existsSync(exampleFile)) { … }will almost always hit the “Skipping integration test – example Excel file not found” branch unless you explicitly overrideexistsSyncbeforehand.If you want this test to actually exercise the real example XLSX file under normal dev runs, consider switching just this test to use
jest.requireActual('fs')(or temporarily un-mockingfs) for the existence check, while still letting the rest of the file use the mockedfs.test/util/xlsx.test.ts (1)
3-15: Comprehensive coverage of the newparseXlsxFilebehaviorThese tests do a nice job exercising the new
read-excel-file–based implementation:
- The module-level mock for
read-excel-file/nodeplus per-test stubbing gives you precise control overreadSheetNamesand row data.- You cover success, missing-module, generic read failures, empty/no-sheet conditions, sheet selection by name and index (including out-of-range and invalid), and detailed data validation (no headers, only empty data, mixed empty/valid rows, nulls, numerics), all matching the logic in
src/util/xlsx.ts.If you ever see mock state leaking between tests, you could switch the
afterEachto also calljest.resetAllMocks(), but the currentrestoreAllMocks+ per-test stubbing appears sufficient here.Also applies to: 20-245
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
examples/simple-csv/promptfooconfig.yaml(1 hunks)package.json(2 hunks)src/util/xlsx.ts(3 hunks)test/util/testCaseReader.test.ts(3 hunks)test/util/xlsx.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Always use both--coverageand--randomizeflags when running Jest tests:npm test -- --coverage --randomize
Never increase test timeouts - fix the slow test instead
Never use.only()or.skip()in committed code
Always include mock cleanup in test files usingafterEach(() => { jest.resetAllMocks(); })to prevent test pollution
Test entire objects using.toEqual()rather than testing individual fields with separate assertions
Mock minimally - only mock external dependencies (APIs, databases), not code under test
Mirror thesrc/directory structure in test files:test/providers/mirrorssrc/providers/,test/redteam/mirrorssrc/redteam/, etc.
Use@jest/globalsimports in Jest test files
Files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/gh-cli-workflow.mdc)
**/*.{ts,tsx}: Ensure TypeScript compilation passes by runningnpm run buildornpx tscfrom the root to verify TypeScript is valid
Prefer not to introduce new TypeScript types; use existing interfaces whenever possibleUse TypeScript with strict type checking
Files:
test/util/xlsx.test.tstest/util/testCaseReader.test.tssrc/util/xlsx.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/gh-cli-workflow.mdc)
Avoid disabling or skipping tests unless absolutely necessary and documented
**/*.test.{ts,tsx,js,jsx}: Follow Jest best practices with describe/it blocks
Test both success and error cases for all functionality
Files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Follow consistent import order (Biome will handle import sorting)
Use consistent curly braces for all control statements
Prefer const over let; avoid var
Use object shorthand syntax whenever possible
Use async/await for asynchronous code
Use consistent error handling with proper type checks
**/*.{ts,tsx,js,jsx}: Follow consistent import order (Biome will handle import sorting)
Use consistent curly braces for all control statements
Prefer const over let; avoid var
Use object shorthand syntax whenever possible
Use async/await for asynchronous code
Use consistent error handling with proper type checks
Always sanitize sensitive data before logging to prevent exposing secrets, API keys, passwords, and other credentials in logs
Files:
test/util/xlsx.test.tstest/util/testCaseReader.test.tssrc/util/xlsx.ts
**/test/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow Jest best practices with describe/it blocks
Files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
examples/**/promptfooconfig.yaml
📄 CodeRabbit inference engine (examples/CLAUDE.md)
examples/**/promptfooconfig.yaml: promptfooconfig.yaml must include schema reference and follow strict field order: description, env (optional), prompts, providers, defaultTest (optional), scenarios (optional), tests
Use latest AI models in examples: 'openai:gpt-5', 'anthropic:claude-sonnet-4-5-20250929'
Use 'file://' prefix for external file references in example configurations
Keep description field in promptfooconfig.yaml SHORT (3-10 words)
examples/**/promptfooconfig.yaml: Include a workingpromptfooconfig.yaml(or equivalent) file in each example
Always include the YAML schema reference at the top of configuration files:# yaml-language-server: $schema=https://promptfoo.dev/config-schema.json
Follow specific field order in configuration files: description, env, prompts, providers, defaultTest, scenarios, tests
When referencing external files in configuration, always use thefile://prefix
For trivial test cases, make them quirky and fun to increase engagement
Always use the latest model versions available in 2025 (e.g., openai:o3-mini, openai:gpt-4o-mini, anthropic:claude-3-7-sonnet-20250219)
Include a mix of providers when comparing model performance
When demonstrating specialized capabilities (vision, audio, etc.), use models that support those features
Always use the latest available model versions for that provider in provider examples
Update model versions when new ones become available
Files:
examples/simple-csv/promptfooconfig.yaml
examples/**/*
📄 CodeRabbit inference engine (.cursor/rules/examples.mdc)
Each example should have its own directory with a clear, descriptive name
Files:
examples/simple-csv/promptfooconfig.yaml
examples/**/*.yaml
📄 CodeRabbit inference engine (.cursor/rules/examples.mdc)
examples/**/*.yaml: Ensure all configuration files pass YAML lint validation
Include comments to explain non-obvious parts in configuration and code files
Use descriptive variable and function names
Format configuration files consistently
Files:
examples/simple-csv/promptfooconfig.yaml
examples/**
📄 CodeRabbit inference engine (.cursor/rules/gh-cli-workflow.mdc)
When modifying examples, update existing files instead of adding new ones. Replace outdated model IDs rather than introducing brand new example files
Files:
examples/simple-csv/promptfooconfig.yaml
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Drizzle ORM for database operations
Files:
src/util/xlsx.ts
package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use CommonJS modules (type: 'commonjs' in package.json)
Use CommonJS modules (type: "commonjs" in package.json)
Files:
package.json
{.nvmrc,package.json}
📄 CodeRabbit inference engine (CLAUDE.md)
Node.js version requirement (>=20.0.0). Use
nvm useto align with .nvmrc (currently v24.7.0)
Files:
package.json
🧠 Learnings (39)
📚 Learning: 2025-11-24T18:15:41.142Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:15:41.142Z
Learning: Applies to test/**/*.test.ts : Mock minimally - only mock external dependencies (APIs, databases), not code under test
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Use Jest's mocking utilities (jest.mock, jest.Mocked) rather than complex custom mocks
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Mock as few functions as possible to keep tests realistic
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Prefer shallow mocking over deep mocking in Jest tests
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Mock API responses to avoid external dependencies in Jest tests
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Reset mocks between tests to prevent test pollution
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Mock external dependencies but not the code being tested in Jest tests
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:14:47.318Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/app/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:14:47.318Z
Learning: Applies to src/app/**/*.test.{ts,tsx} : Import test utilities from 'vitest', not 'jest/globals'. Use: `import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'`
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.tspackage.json
📚 Learning: 2025-11-24T18:15:41.142Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:15:41.142Z
Learning: Applies to test/**/*.test.ts : Use `jest/globals` imports in Jest test files
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:14:47.318Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/app/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:14:47.318Z
Learning: Applies to src/app/**/*.test.{ts,tsx} : Mock API calls in tests using `vi.mock()` to prevent real network requests
Applied to files:
test/util/xlsx.test.ts
📚 Learning: 2025-11-24T18:15:41.142Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:15:41.142Z
Learning: Applies to test/**/*.test.ts : Always include mock cleanup in test files using `afterEach(() => { jest.resetAllMocks(); })` to prevent test pollution
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T19:12:15.259Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T19:12:15.259Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Test both success and error cases for all functionality
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:15:14.923Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/providers/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:15:14.923Z
Learning: Applies to src/providers/test/providers/**/*.test.ts : Every provider must have tests including mock API responses, success and error cases, rate limits, timeouts, and invalid config handling, runnable with `npx jest providers/my-provider --coverage --randomize`
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Test error handling and edge cases including rate limits and timeouts for providers
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:15:41.142Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:15:41.142Z
Learning: Applies to test/providers/**/*.test.ts : Every provider test must include: success case (normal API response), error cases (4xx, 5xx, rate limits), configuration validation, and token usage tracking
Applied to files:
test/util/xlsx.test.tstest/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:13:58.059Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: examples/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:13:58.059Z
Learning: Applies to examples/**/promptfooconfig.yaml : Keep description field in promptfooconfig.yaml SHORT (3-10 words)
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-07-18T17:25:38.445Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-07-18T17:25:38.445Z
Learning: Applies to examples/*/promptfooconfig*.yaml : Format configuration files consistently
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-07-18T17:25:38.444Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-07-18T17:25:38.444Z
Learning: Applies to examples/*/promptfooconfig*.yaml : For trivial test cases in configuration, make them quirky and fun to increase engagement
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-07-18T17:25:38.444Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-07-18T17:25:38.444Z
Learning: Applies to examples/*/promptfooconfig*.yaml : Follow the specific field order in all configuration files: description, env (optional), prompts, providers, defaultTest (optional), scenarios (optional), tests
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-11-24T18:13:58.059Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: examples/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:13:58.059Z
Learning: Applies to examples/**/promptfooconfig.yaml : promptfooconfig.yaml must include schema reference and follow strict field order: description, env (optional), prompts, providers, defaultTest (optional), scenarios (optional), tests
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-11-24T18:16:46.293Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-11-24T18:16:46.293Z
Learning: Applies to examples/**/promptfooconfig.yaml : Follow specific field order in configuration files: description, env, prompts, providers, defaultTest, scenarios, tests
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-11-24T18:16:46.293Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-11-24T18:16:46.293Z
Learning: Applies to examples/**/promptfooconfig.yaml : For trivial test cases, make them quirky and fun to increase engagement
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-07-18T17:25:38.445Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-07-18T17:25:38.445Z
Learning: Applies to examples/*/promptfooconfig*.yaml : Update model versions when new ones become available in configuration files
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-07-18T17:25:38.444Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-07-18T17:25:38.444Z
Learning: Applies to examples/*/promptfooconfig*.yaml : Include a working promptfooconfig.yaml (or equivalent) file in each example
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-11-24T18:16:46.293Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-11-24T18:16:46.293Z
Learning: Applies to examples/**/promptfooconfig.yaml : Update model versions when new ones become available
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-07-18T17:25:38.444Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-07-18T17:25:38.444Z
Learning: Applies to examples/*/promptfooconfig*.yaml : For OpenAI, prefer models like 'openai:o3-mini' and 'openai:gpt-4o-mini' in configuration files
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-11-24T18:13:58.059Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: examples/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:13:58.059Z
Learning: Applies to examples/**/promptfooconfig.yaml : Use latest AI models in examples: 'openai:gpt-5', 'anthropic:claude-sonnet-4-5-20250929'
Applied to files:
examples/simple-csv/promptfooconfig.yaml
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Ensure test failures are deterministic and do not depend on external state
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Clean up after tests by calling `jest.resetAllMocks()` in an `afterEach` block to prevent side effects
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Clean up any test data or mocks after each test to prevent side effects
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:44.371Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T18:17:44.371Z
Learning: Applies to **/test/**/*.{ts,tsx,js,jsx} : Follow Jest best practices with describe/it blocks
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:13:58.059Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: examples/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:13:58.059Z
Learning: Always test examples with local build using 'npm run local -- eval -c examples/my-example/promptfooconfig.yaml', not with published versions
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-07-18T17:25:38.445Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/examples.mdc:0-0
Timestamp: 2025-07-18T17:25:38.445Z
Learning: When developing or testing examples locally, use 'npm run local' commands instead of 'npx promptfoolatest' to ensure local changes are tested
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:17:17.843Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/jest.mdc:0-0
Timestamp: 2025-11-24T18:17:17.843Z
Learning: Applies to test/**/*.test.ts,test/**/*.spec.ts : Always run `nvm use` first to ensure you're using the correct Node.js version before running Jest tests
Applied to files:
test/util/testCaseReader.test.tspackage.json
📚 Learning: 2025-11-24T18:17:01.480Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/gh-cli-workflow.mdc:0-0
Timestamp: 2025-11-24T18:17:01.480Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid disabling or skipping tests unless absolutely necessary and documented
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-07-18T17:25:57.700Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/gh-cli-workflow.mdc:0-0
Timestamp: 2025-07-18T17:25:57.700Z
Learning: Applies to **/*.{test,spec}.{js,ts,jsx,tsx} : Avoid disabling or skipping tests unless absolutely necessary and documented
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:15:41.142Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:15:41.142Z
Learning: Applies to test/**/*.test.ts : Never use `.only()` or `.skip()` in committed code
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:13:48.317Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: drizzle/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:13:48.317Z
Learning: Applies to drizzle/test/**/*.{ts,tsx,js} : Use in-memory SQLite database in test files for migration testing (see `test/` directory for patterns)
Applied to files:
test/util/testCaseReader.test.ts
📚 Learning: 2025-11-24T18:14:47.318Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/app/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:14:47.318Z
Learning: Applies to src/app/**/*.test.{ts,tsx} : Use Vitest for testing (not Jest). Vitest is configured in vite.config.ts with jsdom environment and globals enabled
Applied to files:
package.json
🧬 Code graph analysis (3)
test/util/xlsx.test.ts (1)
src/util/xlsx.ts (1)
parseXlsxFile(4-144)
test/util/testCaseReader.test.ts (1)
src/util/xlsx.ts (1)
parseXlsxFile(4-144)
src/util/xlsx.ts (1)
src/types/index.ts (1)
CsvRow(85-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: Test on Node 22.x and windows-latest
- GitHub Check: Test on Node 24.x and windows-latest
- GitHub Check: Test on Node 22.x and ubuntu-latest
- GitHub Check: Test on Node 22.x and macOS-latest
- GitHub Check: Test on Node 24.x and ubuntu-latest
- GitHub Check: Test on Node 20.x and macOS-latest
- GitHub Check: Test on Node 20.x and windows-latest
- GitHub Check: Test on Node 20.x and ubuntu-latest
- GitHub Check: Redteam (Production API)
- GitHub Check: Share Test
- GitHub Check: Build on Node 24.x
- GitHub Check: Build on Node 22.x
- GitHub Check: webui tests
- GitHub Check: Redteam (Staging API)
- GitHub Check: Build on Node 20.x
- GitHub Check: Build Docs
- GitHub Check: security-scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
test/util/testCaseReader.test.ts (1)
454-504: XLSX tests now correctly mirror theread-excel-filebehavior and error pathsThe updated tests around XLSX support look good:
jest.doMock('read-excel-file/node', …)+jest.resetModules()+ fresh import ofreadStandaloneTestsFileensures the dynamic import insrc/util/xlsx.tssees the mocked module.- The happy-path test uses the correct “array-of-arrays with headers in row 0” shape and aligns with the new CsvRow mapping.
- The “module not installed” test throws from the mock factory and validates the user-facing guidance string, exercising the dynamic import failure path.
- The “no sheets” case stubs
readSheetNamesto[]and checks the exact"Excel file has no sheets"message, matching the implementation.This provides good coverage of both success and primary error scenarios after the library swap.
Also applies to: 506-565
…nerability The xlsx package (SheetJS) version 0.18.5 on npm has a high severity prototype pollution vulnerability (CVE-2023-30533). The fix exists in version 0.19.3+, but SheetJS stopped publishing to npm. This replaces xlsx with read-excel-file, which: - Has no known vulnerabilities - Is actively maintained (last update Sep 2025) - Is 7x smaller (1.0 MB vs 7.5 MB) - Provides all needed functionality (read xlsx, select sheets) Changes: - Replace xlsx with read-excel-file in package.json - Rewrite src/util/xlsx.ts to use read-excel-file API - Update tests in test/util/xlsx.test.ts - Update tests in test/util/testCaseReader.test.ts - Update example comment in examples/simple-csv/promptfooconfig.yaml npm audit results: 22 vulnerabilities → 21 (high severity removed)
mldangelo
force-pushed
the
fix/replace-xlsx-with-read-excel-file
branch
from
6fd8433 to
ecc9e1d
Compare
Co-authored-by: James Hiester <james@promptfoo.dev> Co-authored-by: mldangelo <michael.l.dangelo@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
Summary
xlsx(SheetJS) withread-excel-fileto eliminate CVE-2023-30533 (high severity prototype pollution)read-excel-fileis actively maintained, has no vulnerabilities, and is 7x smaller (1MB vs 7.5MB)Changes
src/util/xlsx.tsto use the new library's APInpm audit results
Test plan
npm run buildpassesnpm run lintpassestest/util/xlsx.test.ts)test/util/testCaseReader.test.ts)examples/simple-csv/tests.xlsx)