fix: improve input validation, error logging, and env var handling
All checks were successful
Build and Push Monarch MCP Docker Image / build (push) Successful in 30s
All checks were successful
Build and Push Monarch MCP Docker Image / build (push) Successful in 30s
- Add validate_account_id() for get_account_holdings input validation - Fix double logging bug in retry_on_auth_error decorator - Remove emojis from log messages for cleaner log parsing - Make PORT and LOG_LEVEL environment variables functional - Delete redundant requirements.txt (pyproject.toml is authoritative) - Clarify MONARCH_PORT is for Docker Compose only in .env.example
This commit is contained in:
@@ -13,5 +13,5 @@ MONARCH_MFA_SECRET=
|
|||||||
|
|
||||||
# Server Configuration
|
# Server Configuration
|
||||||
PORT=8000
|
PORT=8000
|
||||||
MONARCH_PORT=8070
|
MONARCH_PORT=8070 # Docker Compose host port mapping only
|
||||||
LOG_LEVEL=INFO
|
LOG_LEVEL=INFO
|
||||||
|
|||||||
@@ -1,8 +0,0 @@
|
|||||||
fastmcp>=0.4.1
|
|
||||||
monarchmoney>=0.1.15
|
|
||||||
gql>=3.4,<4.0
|
|
||||||
python-dotenv>=1.0.0
|
|
||||||
pydantic>=2.0.0
|
|
||||||
starlette>=0.35.0
|
|
||||||
uvicorn>=0.27.0
|
|
||||||
pyotp==2.9.0
|
|
||||||
@@ -23,7 +23,7 @@ def load_token() -> Optional[str]:
|
|||||||
# 1. Check environment variable (Best for Docker)
|
# 1. Check environment variable (Best for Docker)
|
||||||
token = os.getenv("MONARCH_TOKEN")
|
token = os.getenv("MONARCH_TOKEN")
|
||||||
if token:
|
if token:
|
||||||
logger.info("✅ Token loaded from MONARCH_TOKEN environment variable")
|
logger.info("Token loaded from MONARCH_TOKEN environment variable")
|
||||||
return token
|
return token
|
||||||
|
|
||||||
return None
|
return None
|
||||||
@@ -37,9 +37,9 @@ def save_token(token: str) -> None:
|
|||||||
KEYRING_SERVICE = "com.mcp.monarch-mcp-server"
|
KEYRING_SERVICE = "com.mcp.monarch-mcp-server"
|
||||||
KEYRING_USERNAME = "monarch-token"
|
KEYRING_USERNAME = "monarch-token"
|
||||||
keyring.set_password(KEYRING_SERVICE, KEYRING_USERNAME, token)
|
keyring.set_password(KEYRING_SERVICE, KEYRING_USERNAME, token)
|
||||||
logger.info("✅ Token saved securely to keyring")
|
logger.info("Token saved securely to keyring")
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.warning(f"⚠️ Failed to save token to keyring (non-fatal): {e}")
|
logger.warning(f"Failed to save token to keyring (non-fatal): {e}")
|
||||||
|
|
||||||
|
|
||||||
async def get_authenticated_client() -> MonarchMoney:
|
async def get_authenticated_client() -> MonarchMoney:
|
||||||
@@ -58,11 +58,11 @@ async def get_authenticated_client() -> MonarchMoney:
|
|||||||
_client_instance = MonarchMoney(token=token)
|
_client_instance = MonarchMoney(token=token)
|
||||||
return _client_instance
|
return _client_instance
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"❌ Failed to initialize MonarchMoney with token: {e}")
|
logger.error(f"Failed to initialize MonarchMoney with token: {e}")
|
||||||
raise
|
raise
|
||||||
|
|
||||||
raise RuntimeError(
|
raise RuntimeError(
|
||||||
"🔐 Authentication required. Please provide MONARCH_TOKEN or run login_setup.py"
|
"Authentication required. Please provide MONARCH_TOKEN or run login_setup.py"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -86,11 +86,11 @@ async def refresh_authentication() -> MonarchMoney:
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
await mm.login(email, password, mfa_secret_key=mfa_secret, save_session=False)
|
await mm.login(email, password, mfa_secret_key=mfa_secret, save_session=False)
|
||||||
logger.info("✅ Re-authentication successful")
|
logger.info("Re-authentication successful")
|
||||||
_client_instance = mm
|
_client_instance = mm
|
||||||
return mm
|
return mm
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"❌ Re-authentication failed: {e}")
|
logger.error(f"Re-authentication failed: {e}")
|
||||||
raise
|
raise
|
||||||
|
|
||||||
|
|
||||||
@@ -127,11 +127,15 @@ def retry_on_auth_error(max_retries: int = 1):
|
|||||||
|
|
||||||
if is_auth_error and attempt < max_retries:
|
if is_auth_error and attempt < max_retries:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
f"⚠️ Authentication failure detected (attempt {attempt + 1}/{max_retries + 1}), re-authenticating..."
|
f"Authentication failed in {func.__name__}, refreshing token... "
|
||||||
|
f"(attempt {attempt + 1}/{max_retries + 1})"
|
||||||
)
|
)
|
||||||
await refresh_authentication()
|
await refresh_authentication()
|
||||||
continue
|
continue
|
||||||
else:
|
|
||||||
|
# Only log error for non-auth errors (auth errors were already logged as warnings)
|
||||||
|
if not is_auth_error:
|
||||||
|
logger.error(f"Request failed in {func.__name__}: {e}")
|
||||||
raise
|
raise
|
||||||
|
|
||||||
return wrapper
|
return wrapper
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ Monarch Money MCP Server - Custom SSE Implementation.
|
|||||||
|
|
||||||
import logging
|
import logging
|
||||||
import json
|
import json
|
||||||
|
import os
|
||||||
from typing import Optional, Any
|
from typing import Optional, Any
|
||||||
|
|
||||||
from dotenv import load_dotenv
|
from dotenv import load_dotenv
|
||||||
@@ -17,9 +18,11 @@ from monarch_mcp_custom.auth import get_authenticated_client, retry_on_auth_erro
|
|||||||
# Load environment variables
|
# Load environment variables
|
||||||
load_dotenv()
|
load_dotenv()
|
||||||
|
|
||||||
# Configure logging
|
# Configure logging with LOG_LEVEL from environment
|
||||||
|
log_level = os.getenv("LOG_LEVEL", "INFO").upper()
|
||||||
logging.basicConfig(
|
logging.basicConfig(
|
||||||
level=logging.INFO, format="%(asctime)s - %(name)s - %(levelname)s - %(message)s"
|
level=getattr(logging, log_level, logging.INFO),
|
||||||
|
format="%(asctime)s - %(name)s - %(levelname)s - %(message)s",
|
||||||
)
|
)
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -124,13 +127,23 @@ async def get_budgets(reason: Optional[str] = None) -> str:
|
|||||||
return serialize_json(budget_list)
|
return serialize_json(budget_list)
|
||||||
|
|
||||||
|
|
||||||
|
def validate_account_id(account_id: str) -> int:
|
||||||
|
"""Validate and convert account_id to integer."""
|
||||||
|
if not account_id or not account_id.strip():
|
||||||
|
raise ValueError("account_id must be provided and cannot be empty")
|
||||||
|
try:
|
||||||
|
return int(account_id.strip())
|
||||||
|
except (ValueError, TypeError):
|
||||||
|
raise ValueError("account_id must be a valid integer")
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
@retry_on_auth_error()
|
@retry_on_auth_error()
|
||||||
async def get_account_holdings(account_id: str, reason: Optional[str] = None) -> str:
|
async def get_account_holdings(account_id: str, reason: Optional[str] = None) -> str:
|
||||||
"""Get investment holdings for a specific account."""
|
"""Get investment holdings for a specific account."""
|
||||||
|
validated_id = validate_account_id(account_id)
|
||||||
client = await get_authenticated_client()
|
client = await get_authenticated_client()
|
||||||
# The library expects an int for account_id
|
holdings = await client.get_account_holdings(validated_id)
|
||||||
holdings = await client.get_account_holdings(int(account_id))
|
|
||||||
return serialize_json(holdings)
|
return serialize_json(holdings)
|
||||||
|
|
||||||
|
|
||||||
@@ -176,4 +189,5 @@ app = create_app()
|
|||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
import uvicorn
|
import uvicorn
|
||||||
|
|
||||||
uvicorn.run(app, host="0.0.0.0", port=8000)
|
port = int(os.getenv("PORT", "8000"))
|
||||||
|
uvicorn.run(app, host="0.0.0.0", port=port)
|
||||||
|
|||||||
Reference in New Issue
Block a user