Add MySQL support to MCP Database Server#5
Conversation
Updated package.json and package-lock.json to include MySQL dependencies. Enhanced README with MySQL usage instructions and configuration details. Modified index.ts to handle MySQL connection parameters and logging. Added MysqlAdapter for database interactions.
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
| */ | ||
| async init(): Promise<void> { | ||
| try { | ||
| console.error(`[INFO] Connecting to MySQL: ${this.host}, Database: ${this.database}`); |
There was a problem hiding this comment.
Warning
Description: Log Injection occurs when untrusted user input is directly written to log files without proper sanitization. This can allow attackers to manipulate log entries, potentially leading to security issues like log forging or cross-site scripting. To prevent this, always sanitize user input before logging by removing or encoding newline characters, using string encoding functions, and leveraging built-in sanitization features of logging libraries when available. Learn more - https://cwe.mitre.org/data/definitions/117.html
Severity: High
There was a problem hiding this comment.
The fix involves using a sanitizeLog function (from an imported sanitize-log package) to sanitize user input before logging, preventing potential log injection vulnerabilities.
| console.error(`[INFO] Connecting to MySQL: ${this.host}, Database: ${this.database}`); | |
| // Import the sanitize-log package for log sanitization | |
| async init(): Promise<void> { | |
| try { | |
| console.error(`[INFO] Connecting to MySQL: ${sanitizeLog(this.host)}, Database: ${sanitizeLog(this.database)}`); | |
| this.connection = await mysql.createConnection(this.config); | |
| console.error(`[INFO] MySQL connection established successfully`); | |
| } catch (err) { | |
| console.error(`[ERROR] MySQL connection error: ${sanitizeLog((err as Error).message)}`); | |
| throw new Error(`Failed to connect to MySQL: ${sanitizeLog((err as Error).message)}`); | |
| } | |
| } |
|
✅ I finished the code review, and left comments with the issues I found. I will now generate code fix suggestions. |
Added validation for the MySQL connection port to ensure it is a number, with error handling for invalid values. Included a debug log statement to output the port being used for the connection.
Bumped version to 1.1.0 in index.ts and package.json. Updated README and connection reference documentation to include MySQL port option and usage examples. Added release notes for new features and improvements related to MySQL support.
2 participants
Updated package.json and package-lock.json to include MySQL dependencies. Enhanced README with MySQL usage instructions and configuration details. Modified index.ts to handle MySQL connection parameters and logging. Added MysqlAdapter for database interactions.