Files
sbroenne__mcp-server-excel/SECURITY.md
T
Stefan Broenner df928e95f4 Refactor (#188)
* feat: align CLI with MCP Server session API pattern

MAJOR CHANGE: CLI now uses MCP-aligned session terminology
- Old: batch-begin, batch-commit, batch-list
- New: open, save, list (primary commands)
- Legacy batch commands still work but map to new session commands

IMPLEMENTATION:
✓ BatchCommands: Renamed Begin() → Open(), Commit() → Save()
✓ Program.cs: Updated routing for new command names
✓ Program.cs: Added backward compatibility for old batch commands
✓ Program.cs: Updated help text to show session lifecycle
✓ Program.cs: Banner now says 'MCP-aligned Session API'
✓ CommandHelper: Updated --batch-id → --session-id parameter detection
✓ Error messages: Updated terminology from 'batch' to 'session'
✓ All file operations: Updated comments and output messages

BACKWARD COMPATIBILITY:
✓ Old batch-begin/batch-commit/batch-list commands still work
✓ --batch-id flag still accepted (mapped to --session-id internally)
✓ Both terminologies coexist during transition

TESTING:
✓ CLI builds: 0 warnings, 0 errors
✓ MCP smoke test passed
✓ Help text displays new session commands and deprecation notice

DOCUMENTATION:
✓ CLI README already updated in previous commit with session API examples
✓ Help output now clearly shows primary commands and deprecation path

This aligns the CLI completely with the MCP Server's session-based API,
making both interfaces consistent and simplifying user experience.

* feat: split save/close commands - save keeps session open, close discards changes

* refactor: remove backward compatibility layer - legacy GetBatch method and usage

* fix: improve Range command error messages - use RangeHelpers specific errors

- All Range operations now use RangeHelpers.ResolveRange() with specificError out parameter
- Instead of cryptic '0x800A03EC' COM error, users now get helpful messages like:
  * 'Sheet \\'Validation_Summary\\' not found. Available sheets: Sheet1, Sheet2'
  * 'Named range \\'Sales\\' not found. Available named ranges: Parameters, Region'
- Includes suggestions to use excel_worksheet or excel_namedrange tools
- Affects: Get/Set Values/Formulas, Copy, Clear, Insert, Delete, Find, Replace, Sort, Hyperlinks, NumberFormat operations

* fix: simplify COM error messages - remove LLM guidance

Removed verbose step-by-step guidance from error messages. LLMs don't need to be told what to do, just what failed.

Changed from:
  'This usually means: (1) Sheet doesn't exist, (2) Range invalid, or (3) Session not open.
   Use excel_worksheet(action: list) to verify...'

To:
  'Cannot read range X on sheet Y: Insufficient memory to continue the execution of the program'

Files changed:
- RangeCommands.Values.cs: Simplified GetValuesAsync and SetValuesAsync COM error messages
- RangeCommands.Formulas.cs: Simplified GetFormulasAsync and SetFormulasAsync COM error messages
- ExcelToolsBase.cs: Simplified session validation error - just state facts, no instructions

* docs: add error message style guidance - facts not instructions

Added Best Practice #9 and Error Message Style section to mcp-server-guide.

Key principle: Error messages should state facts (what failed, why), not prescribe solutions.
LLMs are intelligent agents that determine next steps - they don't need step-by-step guidance.

Example:
  ❌ Wrong: 'This usually means: (1) Sheet doesn't exist... Use excel_worksheet(list)...'
  ✅ Correct: 'Cannot read range X on sheet Y: Insufficient memory'

This matches the simplifications in commit 21d4c58.

* fix: return JSON errors from OpenSessionAsync instead of throwing

Changed OpenSessionAsync to catch exceptions from CreateSessionAsync and return JSON with success=false instead of letting them bubble up as unhandled exceptions.

Fixes the generic 'An error occurred invoking excel_file' message shown when file was already open or other session creation errors occurred.

Files changed:
- ExcelFileTool.cs: Wrapped CreateSessionAsync in try-catch, return JSON errors

* refactor: Replace McpException with standard .NET exceptions

CRITICAL INSIGHT (Phase 8): Discovered that McpException usage was unnecessary complexity.
Since we catch ALL exceptions before the MCP SDK sees them and return JSON responses,
the SDK never encounters McpException - it gets our JSON error response instead.

CHANGES:
- Removed McpException catch blocks from all 11 tool files
- Updated ExcelToolsBase to throw ArgumentException (validation) and InvalidOperationException (state)
- Replaced McpException throws in:
  * ExcelFileTool (5 throws)
  * ExcelConnectionTool (17 throws)
  * All 9 'unknown action' switch branches
  * ExcelWorksheetTool invalid visibility

ARCHITECTURAL BENEFIT:
- Removed unnecessary coupling to ModelContextProtocol.McpException
- Single catch (Exception ex) block now handles all cases
- Uses standard .NET exception types for clearer semantics:
  * ArgumentException for parameter validation
  * InvalidOperationException for invalid state
- Simpler, more idiomatic C# code
- Same error handling quality (descriptive JSON responses)

REMAINING:
- ~86 parameter validation throws still using McpException (will be caught as Exception)
- These still work correctly but should be migrated to ArgumentException in future PRs
- Build: 0 warnings, 0 errors

RATIONALE:
The MCP SDK wraps our tool invocations with try-catch. Our code catches exceptions
BEFORE the SDK sees them and returns JSON with isError=true. Therefore, whether we
throw McpException, ArgumentException, or any other Exception is irrelevant to the
SDK - it receives JSON in all cases. Using standard .NET exceptions is cleaner,
doesn't add SDK coupling, and makes the code more maintainable.

* refactor: Complete migration to ArgumentException for ALL parameter validations

Complete removal of McpException pattern across all 11 tool files:
- ExcelDataModelTool: 23 throws (all measure/table/relationship parameters)
- ExcelWorksheetTool: 16 throws (all sheet/color/visibility parameters)
- ExcelVbaTool: 10 throws (all module/path parameters)
- ExcelTableTool: 5 throws (all table/column/format parameters)
- ExcelQueryTableTool: 10 throws (all query table parameters)
- ExcelRangeTool: 2 throws (shift direction validation)
- ExcelPivotTableTool: 4 throws (aggregation/sort direction validation)
- ExcelPowerQueryTool: 12 throws (completed in prior batch)
- ExcelNamedRangeTool: 9 throws (completed in prior batch)
- ExcelConnectionTool: 17 throws (completed in prior batch)
- ExcelFileTool: 5 throws (completed in prior batch)

TOTAL MIGRATION: 117/~117 parameter validation throws replaced

Architecture Insight:
- Standard .NET exceptions (ArgumentException for invalid input)
- Single catch (Exception ex) block handles all cases
- No SDK coupling - clean separation of concerns
- Build: 0 warnings, 0 errors (maintained throughout)

Breaking Simplification:
- Removed unnecessary McpException abstraction layer
- SDK-side exception handling wraps all tool invocations
- No behavior change to callers (still returns JSON with isError=true)
- Cleaner, more maintainable code with standard .NET patterns

* refactor: Simplify async command invocation by removing unnecessary lambda expressions

* fix: update .gitignore to exclude Jekyll generated files

* Fix MCP Server tests: tool discovery and enum completeness

- Fixed ToolDiscoveryTests: Removed non-existent 'excel_batch' from expected tools list
- Fixed ActionEnumCompletenessTests: Added FileAction.Open/Save/Close mappings to ActionExtensions
- Fixed tool naming: Changed 'excel_conditional_format' to 'excel_conditionalformat' (matches naming convention)
- Enum cleanup: Removed unused BatchAction enum and its ToActionString() mapping

Test results: 60 passing (was 58), 9 failing (was 11)
Remaining failures are pre-existing issues (Power Query COM errors, invalid session handling)

* Update documentation: 12 tools with 168 operations

- Added excel_conditionalformat tool (2 actions: add-rule, clear-rules)
- Removed excel_batch from tool lists (doesn't exist)
- Updated operation counts: 166 → 168
- Updated tool counts: 11 → 12
- Added conditional formatting examples to CLI README
- Updated all READMEs: main, MCP Server, CLI, VS Code extension, GitHub pages
- Updated range action counts: 45 → 43 (moved conditional formatting to dedicated tool)
- Added excel_querytable (8 actions) to complete tool list
- Updated excel_file action count: 3 → 6 (open, save, close, create-empty, close-workbook, test)

* Delete incorrect MCP Server tests expecting exceptions

Removed tests that expected McpException for invalid sessions/missing parameters.
Per CRITICAL-RULES.md Rule 17, MCP tools should return JSON with success=false
for business errors, NOT throw exceptions.

Tests deleted:
- DetailedErrorMessageTests.cs (entire file - 8 tests)
- ExcelMcpServerTests.ExcelWorksheet_InvalidSession_ShouldThrowError

Test results: 59 passing (was 60), 2 failing (was 9)
Remaining 2 failures are legitimate Power Query COM errors (RPC failures)

* Fix ExcelBatch async disposal and timeout handling

- Replace synchronous Task.Wait() with async DisposeAsync pattern
- Add proper CancellationTokenSource disposal in finally blocks
- Use ConfigureAwait(false) for library code
- Fix timeout handling to properly cancel operations
- Add comprehensive disposal tests with timeout scenarios

* Remove ExportAsync methods - consolidate with ViewAsync

All three command types (PowerQuery, VBA, Connection) now use ViewAsync
instead of ExportAsync to return code/data directly rather than writing
to files. This simplifies the API by eliminating duplicate functionality.

Changes:
- Removed ExportAsync from IPowerQueryCommands, IVbaCommands, IConnectionCommands interfaces
- Removed ExportAsync implementations from all three command files
- Updated CLI commands to use ViewAsync instead (return code directly)
- Updated MCP Server tools to use ViewAsync instead (return JSON with code)
- Updated VBA test to use ViewAsync instead of ExportAsync
- Updated POWERQUERY-FUTURE-STATE-SPEC.md to remove ExportAsync references

Tests now verify code is returned in ViewAsync result rather than written to file.

* WIP: CLI async-to-sync conversion attempt - reverting to clean state

* Save WIP

* Align MCP tooling with updated actions

* - Refactored tools to reduce cognizant load on LLM
- Fix stability issues when working on multiple files at the same time

* Increase STA thread cleanup timeout from 3 to 10 seconds to prevent potential deadlocks during disposal

* Refactor QueryTable handling to use unified PowerQueryHelpers

- Updated QueryTableCommand to utilize PowerQueryHelpers for creating QueryTables.
- Introduced ConnectionHelpers for retrieving connection strings and command texts.
- Consolidated QueryTable creation options into PowerQueryHelpers.QueryTableCreateOptions.
- Removed redundant methods and classes related to QueryTable options.
- Enhanced error handling and validation for connection strings.
- Updated tests to reflect changes in QueryTable creation and options application.

* Revert "Refactor QueryTable handling to use unified PowerQueryHelpers"

This reverts commit 3603afcb71.

* Switch PowerQuery flows to inline M code

* Fix formula string formatting in JSON deserialization tests

* Fix test failures: formula syntax, PowerQuery Update, and QueryTable API usage

- Fixed JSON syntax in formula tests (removed extra quotes in array)
- Fixed raw string literal quote handling in Excel formulas
- Fixed PowerQuery Update to preserve worksheet data:
  * Capture QueryTable destination and ResultRange before deletion
  * Clear only ResultRange (not entire sheet)
  * Recreate at same location with clearEntireSheet: false
  * Eliminates legacy clearEntireSheet anti-pattern
- Fixed QueryTable tests to pass M code directly instead of file paths:
  * Removed file I/O operations (7 occurrences)
  * Aligned with PowerQueryCommands.Create() API signature

All 9 originally failing tests now pass (2 formula + 7 QueryTable/PowerQuery).

* Include mcp.json in version control

- Added .vscode/mcp.json to .gitignore exceptions
- Tracks MCP server configuration for project

* Add sheet movement and cross-workbook operations: Move, CopyToWorkbook, MoveToWorkbook
2025-11-17 09:27:59 +00:00

6.7 KiB

Security Policy

Supported Versions

We actively support the following versions of ExcelMcp with security updates:

Version Supported Status
1.0.x ✅ Active
< 1.0 ❌ Unsupported

Security Features

ExcelMcp includes several security measures:

Input Validation

  • Path Traversal Protection: All file paths are validated with Path.GetFullPath()
  • File Size Limits: 1GB maximum file size to prevent DoS attacks
  • Extension Validation: Only .xlsx and .xlsm files are accepted
  • Path Length Validation: Maximum 32,767 characters (Windows limit)

Code Analysis

  • Enhanced Security Rules: CA2100, CA3003, CA3006, CA5389, CA5390, CA5394 enforced as errors
  • Treat Warnings as Errors: All code quality issues must be resolved
  • CodeQL Scanning: Automated security scanning on every push

COM Security

  • Controlled Excel Automation: Excel.Application runs with Visible=false and DisplayAlerts=false
  • Resource Cleanup: Comprehensive COM object disposal and garbage collection
  • No Remote Connections: Only local Excel automation supported

Dependency Management

  • Dependabot: Automated dependency updates and security patches
  • Dependency Review: Pull request scanning for vulnerable dependencies
  • Central Package Management: Consistent versioning across all projects

Reporting a Vulnerability

We take security vulnerabilities seriously. If you discover a security issue, please follow these steps:

1. DO NOT Create a Public Issue

Please do not create a public GitHub issue for security vulnerabilities. This could put all users at risk.

2. Report Privately

Report security vulnerabilities using one of these methods:

Preferred Method: GitHub Security Advisories

  1. Go to https://github.com/sbroenne/mcp-server-excel/security/advisories
  2. Click "Report a vulnerability"
  3. Fill out the advisory form with detailed information

Alternative: Email Send an email to: [maintainer email - to be added]

Subject: [SECURITY] ExcelMcp Vulnerability Report

3. Information to Include

Please provide as much information as possible:

  • Description: Clear description of the vulnerability
  • Impact: What could an attacker do with this vulnerability?
  • Affected Versions: Which versions are affected?
  • Proof of Concept: Steps to reproduce (if possible)
  • Suggested Fix: If you have a fix or mitigation (optional)

Example:

Vulnerability: Path traversal in file operations
Impact: Attacker could read/write files outside intended directory
Affected Versions: 1.0.0 - 1.0.2
PoC: ExcelMcp.exe pq-export "../../../etc/passwd" "query"
Suggested Fix: Validate resolved paths are within allowed directories

4. What to Expect

  • Acknowledgment: Within 48 hours
  • Initial Assessment: Within 5 business days
  • Status Updates: Regular updates on progress
  • Fix Timeline:
    • Critical: 7 days
    • High: 30 days
    • Medium: 90 days
    • Low: Best effort

5. Coordinated Disclosure

We follow responsible disclosure practices:

  1. Private Fix: We'll develop a fix privately
  2. Security Advisory: Create GitHub Security Advisory
  3. CVE Assignment: Request CVE if applicable
  4. Public Release: Release patch with security notes
  5. Credit: We'll credit you in the release notes (if desired)

Security Best Practices for Users

MCP Server Security

  • Validate AI Requests: Review Excel operations requested by AI assistants
  • File Path Restrictions: Only allow MCP Server access to specific directories
  • Audit Logs: Monitor MCP Server operations in logs
  • Trust Configuration: Only enable VBA trust when necessary

CLI Security

  • Script Validation: Review automation scripts before execution
  • File Permissions: Ensure Excel files have appropriate permissions
  • Isolated Environment: Run in sandboxed environment when processing untrusted files
  • Excel Security Settings: Maintain appropriate Excel macro security settings

Development Security

  • Code Review: All changes require review before merge
  • Branch Protection: Main branch protected with required checks
  • Signed Commits: Consider using signed commits (recommended)
  • Least Privilege: Run with minimal required permissions

Known Security Considerations

Excel COM Automation

  • Local Only: ExcelMcp only supports local Excel automation
  • Windows Only: Requires Windows with Excel installed
  • Excel Process: Creates Excel.Application COM objects
  • Macro Security: VBA operations require user consent via setup-vba-trust

File System Access

  • Full Path Resolution: All paths resolved to absolute paths
  • No Network Paths: UNC paths and network drives not supported
  • Current User Context: Operations run with current user permissions

AI Integration (MCP Server)

  • Trusted AI Assistants: Only use with trusted AI platforms
  • Request Validation: Review operations before Excel executes them
  • Sensitive Data: Avoid exposing workbooks with sensitive data to AI assistants
  • Audit Trail: MCP Server logs all operations

Security Updates

Security updates are published through:

Subscribe to repository notifications to receive security alerts.

Vulnerability Disclosure Policy

Our Commitment

  • We will acknowledge receipt of vulnerability reports within 48 hours
  • We will keep reporters informed of progress
  • We will credit researchers in security advisories (if desired)
  • We will not take legal action against researchers following responsible disclosure

Researcher Guidelines

  • Responsible Disclosure: Give us time to fix before public disclosure
  • No Harm: Do not access, modify, or delete other users' data
  • Good Faith: Act in good faith to help improve security
  • Legal: Follow all applicable laws

Security Contacts

Additional Resources

Version History

Version Date Security Changes
1.0.0 2024 Initial security implementation with input validation

Last Updated: 2024-10-19

Thank you for helping keep ExcelMcp and its users safe!