-
Notifications
You must be signed in to change notification settings - Fork 5
Http server and Session manager #58
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
Conversation
Greptile OverviewGreptile SummaryReplaced WebSocket-based agent architecture with HTTP/SSE server using Hono, dramatically simplifying the codebase by removing ~4,400 lines of complex session management, protocol handling, and agent factory code. Key changes:
Issues:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Client as HTTP Client
participant Server as HttpServer (Hono)
participant SM as SessionManager
participant Agent as GeminiAgent
participant Adapter as VercelAIContentGenerator
participant MCP as MCP Server
participant LLM as LLM Provider
Client->>Server: POST /chat (conversationId, message, provider, model)
Server->>Server: validateRequest(ChatRequestSchema)
Server->>SM: getOrCreate(config)
alt Session exists
SM-->>Server: Return existing GeminiAgent
else New session
SM->>Agent: GeminiAgent.create(config)
Agent->>Adapter: new VercelAIContentGenerator(config)
Adapter->>Adapter: createProvider(config)
Agent->>Agent: initialize GeminiConfig with MCP
SM->>SM: sessions.set(conversationId, agent)
SM-->>Server: Return new GeminiAgent
end
Server->>Agent: execute(message, honoStream)
Agent->>Adapter: setHonoStream(honoStream)
loop Multi-turn agentic loop (max 100 turns)
Agent->>Agent: sendMessageStream(parts)
Agent->>Adapter: generateContentStream()
Adapter->>LLM: streamText(messages, tools)
LLM-->>Adapter: Stream chunks (text-delta, tool-call, finish)
Adapter->>Server: honoStream.write(SSE data)
Server-->>Client: Stream SSE response
alt Tool calls requested
Agent->>Agent: Collect tool call requests
loop For each tool call
Agent->>MCP: executeToolCall(requestInfo)
MCP-->>Agent: Tool response
end
Agent->>Agent: Prepare tool response parts for next turn
else No tool calls
Agent->>Agent: Break loop (execution complete)
end
end
Server-->>Client: End stream
Note over Client,Server: DELETE /chat/:conversationId
Client->>Server: DELETE /chat/:conversationId
Server->>SM: delete(conversationId)
SM->>SM: sessions.delete(conversationId)
Server-->>Client: {success: true, sessionCount}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
33 files reviewed, no comments
felarof99
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| const app = new Hono<{ Variables: AppVariables }>(); | ||
| const sessionManager = new SessionManager(); | ||
|
|
||
| app.use( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@greptileai review the security vulnerability aspect and suggest any fixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the security aspects of this HTTP server implementation, I can identify several potential vulnerabilities and areas for improvement:
Security Vulnerabilities Identified:
- Overly Permissive CORS Configuration: The CORS setup accepts any origins from config without validation
- Missing Rate Limiting: No protection against abuse or DoS attacks
- Sensitive Data Exposure: API keys and credentials are logged and passed around without proper sanitization
- No Authentication/Authorization: The
/chatendpoint is completely open - Missing Input Validation: Beyond schema validation, no additional security checks
- Error Information Disclosure: Stack traces and detailed errors could leak sensitive information
- Missing Security Headers: No security-related HTTP headers
Suggested Fixes:
For the CORS configuration specifically (since that's the line your comment is on), here's a more secure approach:
| app.use( | |
| app.use( | |
| '/*', | |
| cors({ | |
| origin: (origin) => { | |
| // Validate origins against a whitelist | |
| const allowedOrigins = validatedConfig.corsOrigins || []; | |
| if (!origin) return true; // Allow requests with no origin (e.g., mobile apps) | |
| return allowedOrigins.some(allowed => { | |
| if (allowed === '*') return true; | |
| if (typeof allowed === 'string') return allowed === origin; | |
| if (allowed instanceof RegExp) return allowed.test(origin); | |
| return false; | |
| }); | |
| }, | |
| allowMethods: ['GET', 'POST', 'DELETE', 'OPTIONS'], | |
| allowHeaders: ['Content-Type', 'Authorization'], | |
| credentials: false, // Explicitly disable credentials unless needed | |
| maxAge: 86400, // Cache preflight requests for 24 hours | |
| }), | |
| ); |
Additional Security Recommendations:
- Add Rate Limiting: Implement rate limiting middleware to prevent abuse
- Sanitize Logging: Remove sensitive data from logs (API keys, credentials)
- Add Authentication: Consider implementing API key authentication or JWT tokens
- Security Headers: Add security headers like
X-Frame-Options,X-Content-Type-Options, etc. - Input Sanitization: Add additional validation beyond schema parsing
- Error Handling: Sanitize error responses to prevent information disclosure
Would you like me to provide implementations for any of these additional security measures?
No description provided.