Fix vulnerability https://github.com/smartsheet-platform/smar-mcp/security/dependabot/16#84
Fix vulnerability https://github.com/smartsheet-platform/smar-mcp/security/dependabot/16#84smar-imran-khawaja wants to merge 7 commits into
Conversation
…25.0 Fixes https://github.com/smartsheet-platform/smar-mcp/security/dependabot/16 BREAKING CHANGE: Requires Node.js 16+ and zod ^3.25.0 for MCP SDK compatibility
Changed all console.info() calls to console.error() so log messages go to stderr instead of stdout. MCP uses stdout exclusively for JSON-RPC messages, and any non-JSON output breaks the protocol.
There was a problem hiding this comment.
Pull request overview
This PR addresses a Dependabot security vulnerability and modifies logging behavior across the codebase. While the dependency updates appear appropriate for security remediation, the logging changes introduce semantic inconsistencies that may cause operational issues.
Key Changes:
- Updated four dependencies to newer versions, including major version bumps for semantic-release packages
- Replaced all
console.infostatements withconsole.erroracross tool modules, API classes, and the main server file
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updates @modelcontextprotocol/sdk, zod, @semantic-release/npm, and semantic-release to newer versions for security remediation |
| src/index.ts | Changes server startup and configuration logs from console.info to console.error |
| src/apis/smartsheet-api.ts | Changes API request logging from console.info to console.error |
| src/apis/smartsheet-sheet-api.ts | Changes sheet copy result logging from console.info to console.error |
| src/tools/smartsheet-workspace-tools.ts | Changes workspace operation logs from console.info to console.error |
| src/tools/smartsheet-user-tools.ts | Changes user operation logs from console.info to console.error |
| src/tools/smartsheet-update-request-tools.ts | Changes update request logs from console.info to console.error |
| src/tools/smartsheet-sheet-tools.ts | Changes sheet operation logs from console.info to console.error |
| src/tools/smartsheet-search-tools.ts | Changes search operation logs from console.info to console.error |
| src/tools/smartsheet-folder-tools.ts | Changes folder operation logs from console.info to console.error |
| src/tools/smartsheet-discussion-tools.ts | Changes discussion operation logs from console.info to console.error |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| console.info(`API Request: ${method} ${url.toString()}`); | ||
| console.error(`API Request: ${method} ${url.toString()}`); |
There was a problem hiding this comment.
Inconsistent logging approach across the codebase. This file uses console.error for informational messages, while src/apis/smartsheet-sheet-api.ts still uses console.debug for similar operational logs (lines 147, 151, 155). The codebase also uses console.warn in src/tools/smartsheet-sheet-tools.ts (lines 340, 397). This mix of console.error, console.debug, and console.warn makes it difficult to properly filter and monitor logs. Consider establishing a consistent logging strategy.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Replaced all console.log/info/warn/error calls with a centralized logger that uses MCP's sendLoggingMessage API to send logs through the protocol. - Added src/utils/logger.ts with debug/info/warning/error levels - Falls back to stderr before server connection - After connection, logs are sent via MCP notifications - Claude Desktop displays these in its MCP logs
|
@smar-imran-khawaja I've opened a new pull request, #85, to work on those changes. Once the pull request is ready, I'll request review from you. |
- Updated package.json engines to require Node.js ^22.14.0 || >=24.10.0 - Updated npm requirement to >=10.0.0 - Created comprehensive upgrade documentation in docs/SEMANTIC_RELEASE_UPGRADE.md - Updated README.md prerequisites section with new requirements - Verified .releaserc.json configuration is compatible with v25 - Confirmed GitHub Actions workflow already uses Node.js 22 Co-authored-by: smar-imran-khawaja <174051013+smar-imran-khawaja@users.noreply.github.com>
- Clarified that Node.js 22 will use latest 22.x version (>= 22.14.0) - Added note about setup-node@v3 being functional but recommended to update to v4 Co-authored-by: smar-imran-khawaja <174051013+smar-imran-khawaja@users.noreply.github.com>
fix: update Node.js requirements for semantic-release v25 compatibility
|
@smar-imran-khawaja I've opened a new pull request, #86, to work on those changes. Once the pull request is ready, I'll request review from you. |
Fixes https://github.com/smartsheet-platform/smar-mcp/security/dependabot/16 and updates logging to prevent errors shown in Claude and other MCP clients
The most important changes are:
Dependency Updates:
package.json, including@modelcontextprotocol/sdk,zod,@semantic-release/npm, andsemantic-releaseto their latest versions.Logging Changes:
console.infostatements withconsole.errorin the following areas to standardize logging output:src/apis/smartsheet-api.tsandsrc/apis/smartsheet-sheet-api.ts[1] [2]src/index.ts[1] [2]Description
Related Issue
Type of Change
How Has This Been Tested?
Checklist