Skip to content
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

Replace snowflake server with more complete version #337

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

isaacwasserman
Copy link
Contributor

@isaacwasserman isaacwasserman commented Dec 13, 2024

Description

I've replaced the Snowflake server listed due to security concerns with the one listed.

Motivation and Context

  • The Snowflake server listed simply implements a single tool execute_query.
    • This can execute arbitrary code (including write operations and administrative operations like ALTER).
    • Sanitization (as is done in the reference SQLite and PostgreSQL servers by recognizing the statement keyword and segregating read and write ops) is especially important when dealing with data services like Snowflake as they are often very large and security critical data warehouses.
  • The Snowflake server added implements these missing security features as well as making the LLM's ability to perform write operations opt-in with an --allow-write configuration flag. The new server additionally provides a read/write resource similar to the SQLite reference server.

How Has This Been Tested?

I've verified that all features work in the Claude Desktop client as well as the LangChain MCP Client.

Breaking Changes

No.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options

Additional context

I notice in #284 a second MySQL server was added for a similar reason to the above. I'd also be happy to add this new listing in addition to the existing Snowflake server, rather than replacing it outright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant