Skip to content

Commit 67f6ca3

Browse files
RafaelPoclaude
andcommitted
Security hardening: Sheets token encryption, user-scoped storage, rate limits, input validation, error sanitization
- Encrypt Google tokens at rest in Redis (C1) and key by user_id not "current" (C2) - Add per-user rate limiting on all 5 sheets tools (H3) - Sanitize error messages to avoid leaking response bodies or internal state (H1, L4) - Validate A1 range notation to block path traversal and injection chars (M1) - Sanitize Drive API query to strip non-alphanumeric chars (M6) - Narrow OAuth scope from drive.readonly to drive.metadata.readonly (M3) - Re-raise on token storage failure instead of silently swallowing (M4) - Log only exception type on refresh failure, not full stack trace (M2) - Use server-provided expires_in for token TTL instead of hardcoded constant (L1) - Move sheets import into main() so transport is set before registration (L3) - Mark sheets_write as destructiveHint=True (M5) with audit logging (H4) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 132fa95 commit 67f6ca3

File tree

8 files changed

+328
-37
lines changed

8 files changed

+328
-37
lines changed

everyrow-mcp/src/everyrow_mcp/auth.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ def _supabase_redirect_url(supabase_verifier: str) -> str:
254254
'code_challenge_method': 's256',
255255
'scopes': (
256256
'https://www.googleapis.com/auth/spreadsheets '
257-
'https://www.googleapis.com/auth/drive.readonly'
257+
'https://www.googleapis.com/auth/drive.metadata.readonly'
258258
),
259259
}
260260
)
@@ -467,9 +467,15 @@ async def _issue_token_response(
467467
if google_access_token:
468468
from everyrow_mcp.sheets_client import store_google_token # noqa: PLC0415
469469

470-
await store_google_token(
471-
"current", google_access_token, google_refresh_token or None
472-
)
470+
try:
471+
await store_google_token(
472+
jwt_claims.get("sub", "unknown"),
473+
google_access_token,
474+
google_refresh_token or None,
475+
expires_in=expires_in,
476+
)
477+
except Exception:
478+
logger.warning("Could not store Google token during token issue")
473479

474480
rt_str = secrets.token_urlsafe(32)
475481
rt = EveryRowRefreshToken(

everyrow-mcp/src/everyrow_mcp/config.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,12 @@ class Settings(BaseSettings):
112112
default=False,
113113
description="Enable Google Sheets tools (requires HTTP mode with Google OAuth)",
114114
)
115+
sheets_rate_limit: PositiveInt = Field(
116+
default=60, description="Max sheets ops per user per rate window"
117+
)
118+
sheets_rate_window: PositiveInt = Field(
119+
default=60, description="Sheets rate limit window in seconds"
120+
)
115121
everyrow_api_key: str | None = Field(default=None, repr=False)
116122
google_sheets_credentials_json: str | None = Field(
117123
default=None,

everyrow-mcp/src/everyrow_mcp/server.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,6 @@
2020
)
2121
from everyrow_mcp.uploads import register_upload_tool
2222

23-
# Only register sheets tools when enabled (requires HTTP mode + Google OAuth)
24-
if settings.enable_sheets_tools:
25-
import everyrow_mcp.sheets_tools # noqa: F401
26-
2723

2824
class InputArgs(BaseModel):
2925
http: bool = False
@@ -84,6 +80,10 @@ def main():
8480
settings.transport = transport.value
8581
mcp._mcp_server.instructions = get_instructions(is_http=input_args.http)
8682

83+
# Register sheets tools after transport is set (they require HTTP mode)
84+
if settings.enable_sheets_tools and settings.is_http:
85+
import everyrow_mcp.sheets_tools # noqa: F401, PLC0415
86+
8787
# tools.py registers everyrow_results_stdio by default.
8888
# Override with the HTTP variant when running in HTTP mode.
8989
# ToolManager.add_tool() is a no-op for existing names, so remove first.

everyrow-mcp/src/everyrow_mcp/sheets_client.py

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,44 +9,59 @@
99

1010
import json
1111
import logging
12+
import re
1213
import time
1314
from typing import Any
1415

1516
import httpx
1617

17-
from everyrow_mcp.redis_store import build_key, get_redis_client
18+
from everyrow_mcp.redis_store import (
19+
build_key,
20+
decrypt_value,
21+
encrypt_value,
22+
get_redis_client,
23+
)
1824

1925
logger = logging.getLogger(__name__)
2026

2127
SHEETS_API_BASE = "https://sheets.googleapis.com/v4/spreadsheets"
2228
DRIVE_API_BASE = "https://www.googleapis.com/drive/v3"
2329

2430
# Google token TTL and refresh buffer
25-
GOOGLE_TOKEN_TTL = 3600 # 1 hour
31+
GOOGLE_TOKEN_TTL_DEFAULT = 3600 # 1 hour
2632
GOOGLE_TOKEN_REFRESH_BUFFER = 300 # refresh 5 min before expiry
27-
GOOGLE_TOKEN_REDIS_TTL = 3600 # store for 1 hour in Redis
2833

2934

3035
# ── Token resolution ──────────────────────────────────────────────────
3136

3237

33-
async def get_google_token() -> str:
38+
async def get_google_token(user_id: str | None = None) -> str:
3439
"""Resolve a valid Google access token from Redis.
3540
3641
The token is stored during the OAuth callback when the user logs in
3742
via Google through Supabase. Auto-refreshes if near expiry.
3843
3944
Only available in HTTP mode — sheets tools are removed in stdio mode.
4045
"""
46+
if user_id is None:
47+
from mcp.server.auth.middleware.auth_context import ( # noqa: PLC0415
48+
get_access_token,
49+
)
50+
51+
access_token = get_access_token()
52+
user_id = access_token.client_id if access_token else None
53+
if not user_id:
54+
raise RuntimeError(
55+
"No authenticated user. The user must log in with Google "
56+
"(with Sheets scopes) to use Google Sheets tools."
57+
)
4158

4259
redis = get_redis_client()
4360

44-
# NOTE: single-tenant — "current" key is shared. If multi-tenancy is
45-
# needed, key by session/user ID instead.
46-
token_key = build_key("google_token", "current")
47-
token_data = await redis.get(token_key)
48-
if token_data:
49-
data = json.loads(token_data)
61+
token_key = build_key("google_token", user_id)
62+
raw = await redis.get(token_key)
63+
if raw:
64+
data = json.loads(decrypt_value(raw))
5065
expires_at = data.get("expires_at", 0)
5166
if time.time() < expires_at - GOOGLE_TOKEN_REFRESH_BUFFER:
5267
return data["access_token"]
@@ -55,11 +70,9 @@ async def get_google_token() -> str:
5570
refresh_token = data.get("refresh_token")
5671
if refresh_token:
5772
try:
58-
return await _refresh_google_token_http(refresh_token)
59-
except Exception:
60-
logger.warning(
61-
"Failed to refresh Google token, using existing", exc_info=True
62-
)
73+
return await _refresh_google_token_http(refresh_token, user_id)
74+
except Exception as e:
75+
logger.warning("Failed to refresh Google token: %s", type(e).__name__)
6376
if time.time() < expires_at:
6477
return data["access_token"]
6578

@@ -69,7 +82,7 @@ async def get_google_token() -> str:
6982
)
7083

7184

72-
async def _refresh_google_token_http(refresh_token: str) -> str:
85+
async def _refresh_google_token_http(refresh_token: str, user_id: str) -> str:
7386
"""Refresh a Google access token using the Supabase-stored refresh token."""
7487
from everyrow_mcp.config import settings # noqa: PLC0415
7588

@@ -88,38 +101,46 @@ async def _refresh_google_token_http(refresh_token: str) -> str:
88101

89102
provider_token = data.get("provider_token", "")
90103
provider_refresh_token = data.get("provider_refresh_token", refresh_token)
104+
expires_in = data.get("expires_in")
91105

92106
if not provider_token:
93107
raise RuntimeError("Supabase refresh did not return a Google provider_token")
94108

95-
await store_google_token("current", provider_token, provider_refresh_token)
109+
await store_google_token(
110+
user_id, provider_token, provider_refresh_token, expires_in=expires_in
111+
)
96112
return provider_token
97113

98114

99115
async def store_google_token(
100116
user_id: str,
101117
access_token: str,
102118
refresh_token: str | None = None,
119+
*,
120+
expires_in: int | None = None,
103121
) -> None:
104122
"""Store Google access token in Redis with TTL."""
105123
try:
106124
redis = get_redis_client()
107125
except Exception:
108-
return
126+
logger.error("Failed to obtain Redis client for Google token storage")
127+
raise
128+
ttl = expires_in if expires_in and expires_in > 0 else GOOGLE_TOKEN_TTL_DEFAULT
109129
try:
110-
data = {
130+
data: dict[str, Any] = {
111131
"access_token": access_token,
112-
"expires_at": time.time() + GOOGLE_TOKEN_TTL,
132+
"expires_at": time.time() + ttl,
113133
}
114134
if refresh_token:
115135
data["refresh_token"] = refresh_token
116136
await redis.setex(
117137
build_key("google_token", user_id),
118-
GOOGLE_TOKEN_REDIS_TTL,
119-
json.dumps(data),
138+
ttl,
139+
encrypt_value(json.dumps(data)),
120140
)
121141
except Exception:
122-
logger.warning("Failed to store Google token in Redis for %s", user_id)
142+
logger.error("Failed to store Google token in Redis for %s", user_id)
143+
raise
123144

124145

125146
# ── Sheets API client ─────────────────────────────────────────────────
@@ -224,8 +245,7 @@ async def list_spreadsheets(
224245
"""
225246
q = "mimeType='application/vnd.google-apps.spreadsheet' and trashed=false"
226247
if query:
227-
# Escape single quotes in the user's query
228-
safe_query = query.replace("'", "\\'")
248+
safe_query = re.sub(r"[^a-zA-Z0-9 ]", "", query)
229249
q += f" and name contains '{safe_query}'"
230250

231251
resp = await self._client.get(

everyrow-mcp/src/everyrow_mcp/sheets_models.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
# Matches the 44-char alphanumeric spreadsheet ID in a Google Sheets URL
1111
_SHEETS_URL_RE = re.compile(r"/spreadsheets/d/([a-zA-Z0-9_-]+)")
1212

13+
# A1 notation range validation
14+
_A1_RANGE_RE = re.compile(r"^[A-Za-z0-9_' !:$]+$")
15+
_MAX_RANGE_LENGTH = 200
16+
1317

1418
def _extract_spreadsheet_id(v: str) -> str:
1519
"""Accept a full Google Sheets URL or a bare spreadsheet ID.
@@ -31,6 +35,18 @@ def _extract_spreadsheet_id(v: str) -> str:
3135
)
3236

3337

38+
def _validate_a1_range(v: str) -> str:
39+
"""Validate an A1 notation range string."""
40+
if len(v) > _MAX_RANGE_LENGTH:
41+
raise ValueError(f"Range too long ({len(v)} chars, max {_MAX_RANGE_LENGTH})")
42+
if not _A1_RANGE_RE.fullmatch(v):
43+
raise ValueError(
44+
"Invalid range: contains disallowed characters. "
45+
"Use A1 notation (e.g. 'Sheet1!A1:D10')."
46+
)
47+
return v
48+
49+
3450
class SheetsReadInput(BaseModel):
3551
"""Input for the sheets_read tool."""
3652

@@ -53,6 +69,11 @@ class SheetsReadInput(BaseModel):
5369
def extract_id(cls, v: str) -> str:
5470
return _extract_spreadsheet_id(v)
5571

72+
@field_validator("range")
73+
@classmethod
74+
def validate_range(cls, v: str) -> str:
75+
return _validate_a1_range(v)
76+
5677

5778
class SheetsWriteInput(BaseModel):
5879
"""Input for the sheets_write tool."""
@@ -84,6 +105,11 @@ class SheetsWriteInput(BaseModel):
84105
def extract_id(cls, v: str) -> str:
85106
return _extract_spreadsheet_id(v)
86107

108+
@field_validator("range")
109+
@classmethod
110+
def validate_range(cls, v: str) -> str:
111+
return _validate_a1_range(v)
112+
87113

88114
class SheetsCreateInput(BaseModel):
89115
"""Input for the sheets_create tool."""

0 commit comments

Comments
 (0)