Add HTTP transport and Bearer token auth for Google Cloud Run deployment#11
Add HTTP transport and Bearer token auth for Google Cloud Run deployment#11hemati wants to merge 4 commits into
Conversation
…ort and Bearer Token authentication
|
Warning Rate limit exceeded@hemati has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 20 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds deployment/run files and README Cloud Run instructions; updates packaging/dependencies. server.py gains environment-driven FastMCP/Uvicorn startup, optional Bearer-token Starlette middleware for HTTP transport, transport selection (stdio vs HTTP), defensive Reddit reply handling, and expanded comment typing. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant Uvicorn as Uvicorn/ASGI
participant Middleware as BearerTokenMiddleware
participant Server as FastMCP App (server.py)
participant Reddit as Reddit API (praw)
Client->>Uvicorn: HTTP request (POST /events or stream)
Uvicorn->>Middleware: pass request
alt MCP_BEARER_TOKEN set
Middleware->>Client: validate Authorization: Bearer
Middleware-->>Uvicorn: allow or 401
end
Uvicorn->>Server: dispatch event to handler
Server->>Server: process event, require_write_access checks
Server->>Reddit: create reply / fetch submission
Reddit-->>Server: reply object (or None)
alt reply is None
Server->>Server: raise RuntimeError / log error
else
Server->>Client: return success / event response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server.py (1)
155-208: Consider constant-time comparison for Bearer token validation.The Bearer token authentication middleware is well-structured with proper error responses (401 for missing/invalid format, 403 for wrong token). However, the token comparison at line 192 uses standard string comparison, which may be vulnerable to timing attacks.
🔎 Proposed fix using secrets.compare_digest
Add import at the top of the file:
import secretsThen update the token comparison:
# Extract and validate token token = auth_header[7:] # Remove "Bearer " prefix - if token != self.bearer_token: + if not secrets.compare_digest(token, self.bearer_token): return JSONResponse( status_code=403, content={"error": "Invalid bearer token"} )
secrets.compare_digestperforms constant-time comparison, making timing attacks significantly harder.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ProcfileREADME.mdpyproject.tomlrequirements.txtserver.py
🧰 Additional context used
🪛 Gitleaks (8.30.0)
README.md
[high] 291-296: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
README.md
[grammar] ~262-~262: Ensure spelling is correct
Context: ...} } } } ``` ### Local Testing **Stdio mode (default, for local MCP clients):*...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.14.10)
server.py
140-140: Possible binding to all interfaces
(S104)
1432-1432: Abstract raise to an inner function
(TRY301)
1432-1432: Avoid specifying long messages outside the exception class
(TRY003)
1537-1537: Abstract raise to an inner function
(TRY301)
1537-1537: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (9)
README.md (1)
173-342: Well-documented Cloud Run deployment guide.The deployment instructions are comprehensive and cover:
- Prerequisites and authentication setup
- Basic and secure (Bearer token) deployment options
- MCP client configuration examples
- Local testing for both stdio and HTTP modes
- Monitoring and operational commands
The documentation structure is clear and provides actionable commands.
Procfile (1)
1-1: LGTM! Simple and correct.The Procfile correctly defines the web process for Heroku-style deployments. The server.py main block will handle transport selection based on the
MCP_TRANSPORTenvironment variable.server.py (7)
9-9: Good addition for type checking.Explicitly importing
praw.modelsimproves type annotations and IDE support, especially for the updated_format_commentsignature.
138-152: Environment-driven configuration is well-designed.The FastMCP configuration correctly reads from environment variables:
HOSTdefaults to0.0.0.0(appropriate for Cloud Run containers)PORTdefaults to8080(Cloud Run standard)MCP_STATELESSenables stateless HTTP mode for serverless deploymentsThe static analysis warning about binding to all interfaces (0.0.0.0) is a false positive here—Cloud Run containers require this to accept traffic.
332-332: Improved type annotation for flexibility.Updating the type hint to accept both
praw.models.Commentandpraw.models.Messageimproves type safety and correctly reflects that the function can handle both types of objects.
1431-1432: Good defensive check for API response.Adding validation that
replyis not None prevents potential issues if the Reddit API fails silently. The explicit error message helps with debugging.
1536-1537: Good defensive check for API response.Consistent with the pattern in
reply_to_post, this defensive check ensures the Reddit API returned a valid response.
1906-1908: Good fix for potential UnboundLocalError.Initializing
clean_submission_idbefore the try block ensures it's defined if_extract_reddit_idraises an exception, preventing an UnboundLocalError when the exception handler references it at line 2038.
2402-2434: Well-structured entry point with proper transport selection.The
__main__block correctly handles three modes:
- HTTP with Bearer token authentication (manual uvicorn with middleware)
- HTTP without authentication (standard mcp.run)
- Stdio mode (backward-compatible default)
The conditional logic is clear, and logging messages help with debugging deployment issues.
…erver.py with detailed docstrings for better clarity
hemati
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server.py (1)
184-231: Consider using constant-time comparison for bearer token validation.The bearer token comparison at line 225 uses a standard equality check, which may be vulnerable to timing attacks. For security-sensitive token validation, consider using a constant-time comparison function.
🔎 Proposed fix using secrets.compare_digest
Add import at the top of the file:
+import secrets import functools import loggingThen update the token comparison:
# Extract and validate token token = auth_header[7:] # Remove "Bearer " prefix - if token != self.bearer_token: + if not secrets.compare_digest(token, self.bearer_token): return JSONResponse( status_code=403, content={"error": "Invalid bearer token"} )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pyproject.tomlrequirements.txtserver.py
🚧 Files skipped from review as they are similar to previous changes (1)
- requirements.txt
🧰 Additional context used
🪛 Ruff (0.14.10)
server.py
158-158: Possible binding to all interfaces
(S104)
1465-1465: Abstract raise to an inner function
(TRY301)
1465-1465: Avoid specifying long messages outside the exception class
(TRY003)
1570-1570: Abstract raise to an inner function
(TRY301)
1570-1570: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (7)
server.py (7)
9-9: LGTM!Explicit import of
praw.modelsimproves type checking and makes type annotations clearer.
28-32: LGTM!The docstring additions improve code documentation and follow Python best practices.
Also applies to: 128-140
156-170: LGTM! Configuration appropriate for Cloud Run deployment.The environment-based configuration is well-structured. Binding to
0.0.0.0(flagged by static analysis) is intentional and necessary for Cloud Run deployments to accept external traffic. The stateless HTTP mode is appropriate for serverless environments.
365-365: LGTM!The type annotation improvement makes the function's accepted types explicit and enhances type safety.
1464-1465: LGTM! Good defensive programming.The checks for
Nonereplies add robustness by ensuring Reddit API calls succeeded before proceeding. This prevents silent failures and provides clear error messages.Also applies to: 1569-1570
1939-1941: LGTM! Good fix for potential UnboundLocalError.Initializing
clean_submission_idbefore the try block ensures the variable is always bound when referenced in the exception handler, preventing potential errors.
2436-2467: LGTM! Well-structured transport selection and server startup.The implementation properly handles both HTTP and stdio transports, with backward compatibility maintained through the stdio default. The conditional middleware injection for Bearer token authentication is correctly implemented, and logging provides good visibility into the server's configuration.
|
@Arindam200 this is a larger PR, but it adds an important new feature: using the MCP over HTTP. |
1 participant
This pull request adds support for deploying the Reddit MCP server to Google Cloud Run using HTTP transport, including optional Bearer Token authentication for secure access. The changes also improve local development and testing workflows, update dependencies to support HTTP serving, and enhance error handling and logging throughout the codebase.
Deployment and Authentication Enhancements:
README.mdfor deploying to Google Cloud Run, including environment variable configuration, authentication setup, and log monitoring.MCP_BEARER_TOKENenvironment variable and implemented using Starlette middleware.__main__section inserver.pyto support both stdio and HTTP transports, with conditional middleware injection for authentication and Uvicorn server startup for HTTP mode.Procfileto specify the web server startup command for deployment platforms.Dependency and Configuration Updates:
pyproject.tomlto addfastmcp,uvicorn, and an optionalauthdependency group for Starlette-based authentication.praw.modelsis explicitly imported for type checking and improved type annotations for comment formatting. [1] [2]Robustness and Logging Improvements:
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.