fix: add security hardening and documentation for deployment

- Add document validation to prevent NoneType crash when document not configured - Add SQL query validation (SELECT only, no multi-statement) - Add 30-second HTTP request timeout - Fix filter parameter JSON encoding for get_records - Add return type annotation to get_document - Add tests for document lookup and SQL validation - Add comprehensive README with usage instructions
2025-12-29 18:42:36 -05:00
parent f716e5d37e
commit ed612694fe
5 changed files with 247 additions and 7 deletions
--- a/README.md
+++ b/README.md
@@ -0,0 +1,177 @@
+# grist-mcp
+
+MCP server for AI agents to interact with Grist documents.
+
+## Overview
+
+grist-mcp is a [Model Context Protocol (MCP)](https://modelcontextprotocol.io/) server that enables AI agents to read, write, and modify Grist spreadsheets. It provides secure, token-based access control with granular permissions per document.
+
+## Features
+
+- **Discovery**: List accessible documents with permissions
+- **Read Operations**: List tables, describe columns, fetch records, run SQL queries
+- **Write Operations**: Add, update, and delete records
+- **Schema Operations**: Create tables, add/modify/delete columns
+- **Security**: Token-based authentication with per-document permission scopes (read, write, schema)
+- **Multi-tenant**: Support multiple Grist instances and documents
+
+## Requirements
+
+- Python 3.14+
+- Access to one or more Grist documents with API keys
+
+## Installation
+
+```bash
+# Clone the repository
+git clone https://github.com/your-org/grist-mcp.git
+cd grist-mcp
+
+# Install with uv
+uv sync --dev
+```
+
+## Configuration
+
+Create a `config.yaml` file based on the example:
+
+```bash
+cp config.yaml.example config.yaml
+```
+
+### Configuration Structure
+
+```yaml
+# Document definitions
+documents:
+  my-document:
+    url: https://docs.getgrist.com      # Grist instance URL
+    doc_id: abcd1234                     # Document ID from URL
+    api_key: ${GRIST_API_KEY}            # API key (supports env vars)
+
+# Agent tokens with access scopes
+tokens:
+  - token: your-secret-token             # Unique token for this agent
+    name: my-agent                       # Human-readable name
+    scope:
+      - document: my-document
+        permissions: [read, write]       # Allowed: read, write, schema
+```
+
+### Generating Tokens
+
+```bash
+python -c "import secrets; print(secrets.token_urlsafe(32))"
+# or
+openssl rand -base64 32
+```
+
+### Environment Variables
+
+- `CONFIG_PATH`: Path to config file (default: `/app/config.yaml`)
+- `GRIST_MCP_TOKEN`: Agent token for authentication
+- Config file supports `${VAR}` syntax for API keys
+
+## Usage
+
+### Running the Server
+
+```bash
+# Set your agent token
+export GRIST_MCP_TOKEN="your-agent-token"
+
+# Run with custom config path
+CONFIG_PATH=./config.yaml uv run python -m grist_mcp.main
+```
+
+### MCP Client Configuration
+
+Add to your MCP client configuration (e.g., Claude Desktop):
+
+```json
+{
+  "mcpServers": {
+    "grist": {
+      "command": "uv",
+      "args": ["run", "python", "-m", "grist_mcp.main"],
+      "cwd": "/path/to/grist-mcp",
+      "env": {
+        "CONFIG_PATH": "/path/to/config.yaml",
+        "GRIST_MCP_TOKEN": "your-agent-token",
+        "GRIST_API_KEY": "your-grist-api-key"
+      }
+    }
+  }
+}
+```
+
+## Available Tools
+
+### Discovery
+| Tool | Description |
+|------|-------------|
+| `list_documents` | List documents accessible to this agent with their permissions |
+
+### Read Operations (requires `read` permission)
+| Tool | Description |
+|------|-------------|
+| `list_tables` | List all tables in a document |
+| `describe_table` | Get column information (id, type, formula) for a table |
+| `get_records` | Fetch records with optional filter, sort, and limit |
+| `sql_query` | Run a read-only SELECT query against a document |
+
+### Write Operations (requires `write` permission)
+| Tool | Description |
+|------|-------------|
+| `add_records` | Add new records to a table |
+| `update_records` | Update existing records by ID |
+| `delete_records` | Delete records by ID |
+
+### Schema Operations (requires `schema` permission)
+| Tool | Description |
+|------|-------------|
+| `create_table` | Create a new table with specified columns |
+| `add_column` | Add a column to an existing table |
+| `modify_column` | Change a column's type or formula |
+| `delete_column` | Remove a column from a table |
+
+## Security
+
+- **Token-based auth**: Each agent has a unique token with specific document access
+- **Permission scopes**: Granular control with `read`, `write`, and `schema` permissions
+- **SQL validation**: Only SELECT queries allowed, no multi-statement queries
+- **API key isolation**: Each document can use a different Grist API key
+- **No token exposure**: Tokens are validated at startup, not stored in responses
+
+## Development
+
+### Running Tests
+
+```bash
+uv run pytest -v
+```
+
+### Project Structure
+
+```
+grist-mcp/
+├── src/grist_mcp/
+│   ├── __init__.py
+│   ├── main.py          # Entry point
+│   ├── server.py        # MCP server setup and tool registration
+│   ├── config.py        # Configuration loading
+│   ├── auth.py          # Authentication and authorization
+│   ├── grist_client.py  # Grist API client
+│   └── tools/
+│       ├── discovery.py # list_documents
+│       ├── read.py      # Read operations
+│       ├── write.py     # Write operations
+│       └── schema.py    # Schema operations
+├── tests/
+├── config.yaml.example
+└── pyproject.toml
+```
+
+## License
+
+MIT
--- a/src/grist_mcp/auth.py
+++ b/src/grist_mcp/auth.py
@@ -67,6 +67,14 @@ class Authenticator:
            for scope in agent._token_obj.scope
        ]

-    def get_document(self, document_name: str):
-        """Get document config by name."""
-        return self._config.documents.get(document_name)
+    def get_document(self, document_name: str) -> "Document":
+        """Get document config by name.
+
+        Raises:
+            AuthError: If document is not configured.
+        """
+        from grist_mcp.config import Document
+        doc = self._config.documents.get(document_name)
+        if doc is None:
+            raise AuthError(f"Document '{document_name}' not configured")
+        return doc
--- a/src/grist_mcp/grist_client.py
+++ b/src/grist_mcp/grist_client.py
@@ -1,21 +1,27 @@
 """Grist API client."""

+import json
+
 import httpx

 from grist_mcp.config import Document

+# Default timeout for HTTP requests (30 seconds)
+DEFAULT_TIMEOUT = 30.0
+

 class GristClient:
    """Async client for Grist API operations."""

-    def __init__(self, document: Document):
+    def __init__(self, document: Document, timeout: float = DEFAULT_TIMEOUT):
        self._doc = document
        self._base_url = f"{document.url.rstrip('/')}/api/docs/{document.doc_id}"
        self._headers = {"Authorization": f"Bearer {document.api_key}"}
+        self._timeout = timeout

    async def _request(self, method: str, path: str, **kwargs) -> dict:
        """Make an authenticated request to Grist API."""
-        async with httpx.AsyncClient() as client:
+        async with httpx.AsyncClient(timeout=self._timeout) as client:
            response = await client.request(
                method,
                f"{self._base_url}{path}",
@@ -54,7 +60,7 @@ class GristClient:
        """Fetch records from a table."""
        params = {}
        if filter:
-            params["filter"] = filter
+            params["filter"] = json.dumps(filter)
        if sort:
            params["sort"] = sort
        if limit:
@@ -68,10 +74,27 @@ class GristClient:
        ]

    async def sql_query(self, sql: str) -> list[dict]:
-        """Run a read-only SQL query."""
+        """Run a read-only SQL query.
+
+        Raises:
+            ValueError: If query is not a SELECT statement or contains multiple statements.
+        """
+        self._validate_sql_query(sql)
        data = await self._request("GET", "/sql", params={"q": sql})
        return [r["fields"] for r in data.get("records", [])]

+    @staticmethod
+    def _validate_sql_query(sql: str) -> None:
+        """Validate SQL query for safety.
+
+        Only allows SELECT statements and rejects multiple statements.
+        """
+        sql_stripped = sql.strip()
+        if not sql_stripped.upper().startswith("SELECT"):
+            raise ValueError("Only SELECT queries are allowed")
+        if ";" in sql_stripped[:-1]:  # Allow trailing semicolon
+            raise ValueError("Multiple statements not allowed")
+
    # Write operations

    async def add_records(self, table: str, records: list[dict]) -> list[int]:
--- a/tests/test_auth.py
+++ b/tests/test_auth.py
@@ -82,3 +82,18 @@ def test_get_accessible_documents(sample_config):
    assert len(docs) == 2
    assert {"name": "budget", "permissions": ["read", "write"]} in docs
    assert {"name": "expenses", "permissions": ["read"]} in docs
+
+
+def test_get_document_returns_document(sample_config):
+    auth = Authenticator(sample_config)
+
+    doc = auth.get_document("budget")
+
+    assert doc.doc_id == "abc123"
+
+
+def test_get_document_raises_on_unknown(sample_config):
+    auth = Authenticator(sample_config)
+
+    with pytest.raises(AuthError, match="Document 'unknown' not configured"):
+        auth.get_document("unknown")
--- a/tests/test_grist_client.py
+++ b/tests/test_grist_client.py
@@ -179,3 +179,20 @@ async def test_delete_column(client, httpx_mock: HTTPXMock):

    # Should not raise
    await client.delete_column("Table1", "OldCol")
+
+
+# SQL validation tests
+
+def test_sql_validation_rejects_non_select(client):
+    with pytest.raises(ValueError, match="Only SELECT queries are allowed"):
+        client._validate_sql_query("DROP TABLE users")
+
+
+def test_sql_validation_rejects_multiple_statements(client):
+    with pytest.raises(ValueError, match="Multiple statements not allowed"):
+        client._validate_sql_query("SELECT * FROM users; DROP TABLE users")
+
+
+def test_sql_validation_allows_trailing_semicolon(client):
+    # Should not raise
+    client._validate_sql_query("SELECT * FROM users;")