chore: add CLI startup for gateway env validation

[buremba]

Summary

• wrap the gateway startup script in a Commander CLI that loads an optional env file and surfaces missing required environment variables with clear errors
• relax the make dev target so it proceeds without a .env file, logging whether overrides will be loaded

Testing

• make dev (fails: Docker CLI is unavailable in this environment)
• ./slack-qa-bot.js "Hello bot" --timeout 10 (fails: dotenv dependency is not installed in this environment)
• make logs (fails: Docker CLI is unavailable in this environment)
• bun run typecheck (fails: repository lacks the ambient typings and dependencies required for a clean TypeScript build in this environment)

---

https://chatgpt.com/codex/tasks/task_e_68ebe18db0488325825950d5f3bbb8c9

[claude]

Claude finished @buremba's task —— View job

---

PR Review: CLI startup for gateway env validation

I've completed a comprehensive review of this PR. The changes introduce CLI wrapper functionality for gateway startup with environment validation and lay foundations for MCP (Model Context Protocol) integration. Here's my detailed feedback:

Review Checklist:

• Analyze repository structure and guidelines
• Review changed files for code quality and best practices
• Check for potential bugs and security issues
• Assess performance considerations
• Evaluate test coverage and documentation

✅ Code Quality & Best Practices

Strong Points:

• Good use of Commander.js for CLI interface in packages/gateway/src/index.ts:285-298
• Proper error handling with custom MissingRequiredEnvError class (packages/gateway/src/index.ts:99-107)
• Clean separation of concerns with dedicated MCP service classes
• Consistent use of structured logging throughout
• TypeScript types are well-defined and comprehensive

Improvements Needed:

• Environment variable validation could be more robust: Current validation only checks if variables exist, not their format/validity
• Missing JSDoc comments: Complex functions like normalizeConfig() in packages/gateway/src/mcp/config-service.ts:198-221 would benefit from documentation
• Magic numbers: Consider extracting constants like port 8080 and timeout values

⚠️ Potential Bugs & Issues

1. Critical: Unsafe JSON parsing in packages/gateway/src/mcp/credential-store.ts:29

   return JSON.parse(value) as McpCredentialRecord;

   This could throw if Redis contains malformed JSON. Should use safe parsing with try-catch.

2. Memory leak risk in packages/gateway/src/mcp/proxy.ts:129-141

   while (true) {
     const { done, value } = await reader.read();
     // No timeout or error handling for infinite loop
   }

   The streaming loop has no timeout or size limits, could cause memory issues with large responses.

3. URL construction vulnerability in packages/gateway/src/mcp/proxy.ts:144-167

   const remainder = (req.params as any)[0] ? `/${(req.params as any)[0]}` : "";

   No input validation on URL path parameters could allow path traversal attacks.

4. Race condition potential in packages/gateway/src/gateway/index.ts:73-78

   const isRegistered = await this.jobRouter.isWorkerRegistered(deploymentName);
   if (!isRegistered) {
     await this.jobRouter.registerWorker(deploymentName);
   }

   Multiple workers could register simultaneously without proper locking.

🔒 Security Concerns

1. Credential exposure in logs: packages/gateway/src/index.ts:133-137 logs truncated tokens but should avoid logging credentials entirely in production
2. Missing CORS configuration: No CORS headers configured for the proxy endpoints
3. No rate limiting: MCP proxy endpoints lack rate limiting which could enable DoS attacks
4. Token validation: Worker token verification should include expiration checks

🚀 Performance Considerations

Good:

• Efficient credential caching in McpConfigService
• Connection reuse in Worker Gateway
• Proper cleanup in shutdown handlers

Areas for improvement:

• File watching: Config service loads from file/URL on every request - should implement file watching or caching with TTL
• Redis connection pooling: Each credential store operation creates new connections
• Stream buffering: Proxy doesn't implement proper backpressure handling

📋 Test Coverage & Documentation

Missing test coverage for:

• MCP proxy authentication flows
• Error handling in credential store operations
• Environment variable validation edge cases
• Worker gateway connection management

Documentation gaps:

• The new docs/mcp_integration_plan.md is comprehensive but implementation doesn't match all planned features
• Missing API documentation for MCP endpoints
• No migration guide for the new CLI interface

🔧 Specific Recommendations

1. Fix JSON parsing safety:

   // In packages/gateway/src/mcp/credential-store.ts:29
   try {
     return JSON.parse(value) as McpCredentialRecord;
   } catch (error) {
     logger.error("Failed to parse credential JSON", { error, key });
     return null;
   }

2. Add stream timeout:

   // In packages/gateway/src/mcp/proxy.ts
   const controller = new AbortController();
   const timeoutId = setTimeout(() => controller.abort(), 30000);

3. Validate URL paths:

   // Add path validation to prevent traversal
   const safePath = remainder.replace(/\.\./g, '').replace(/\/+/g, '/');

4. Add environment variable format validation:

   // Validate URL format, numeric ranges, etc.
   const validateDatabaseUrl = (url: string) => {
     return url.match(/^postgres(ql)?:\/\/.+/);
   };

📊 Overall Assessment

This PR makes significant progress toward MCP integration and improves the development experience with better environment validation. The code structure is solid, but there are several security and reliability concerns that should be addressed before merging.

Priority Issues to Fix:

1. JSON parsing safety (High)
2. Stream timeout handling (Medium)
3. URL path validation (High)
4. Add basic tests for new functionality (Medium)

The Makefile changes appropriately relax the .env requirement while maintaining clear messaging, which aligns well with the CLI improvements.

---

[claude]

Claude finished @buremba's task —— View job

---

PR Review: CLI startup for gateway env validation

Review Checklist:

• Read project guidelines and understand codebase structure
• Analyze changed files for code quality and best practices
• Check for potential bugs and security issues
• Assess performance considerations
• Evaluate test coverage and documentation
• Provide final review summary

---

✅ Code Quality & Best Practices

Strong Points:

• Excellent CLI implementation using Commander.js in packages/gateway/src/index.ts:285-298 with proper error handling
• Well-structured MCP services with clear separation of concerns across config, credential store, and proxy components
• Robust environment validation with custom MissingRequiredEnvError class (packages/gateway/src/index.ts:99-107)
• Consistent logging throughout all components using the shared logger
• TypeScript types are comprehensive and well-defined

Minor Improvements:

• Consider extracting magic constants like port 8080 and timeout values
• Some complex functions like normalizeConfig() in packages/gateway/src/mcp/config-service.ts:198-221 could benefit from JSDoc documentation

⚠️ Critical Security & Bug Issues

1. CRITICAL: Unsafe JSON parsing in packages/gateway/src/mcp/credential-store.ts:29

   return JSON.parse(value) as McpCredentialRecord;

   This will throw if Redis contains malformed JSON. Should use safe parsing with try-catch.

2. HIGH: Memory leak risk in packages/gateway/src/mcp/proxy.ts:129-141

   while (true) {
     const { done, value } = await reader.read();
     // No timeout or error handling for infinite loop
   }

   The streaming loop lacks timeout or size limits, could cause memory issues with large responses.

3. HIGH: URL path traversal vulnerability in packages/gateway/src/mcp/proxy.ts:144-167

   const remainder = (req.params as any)[0] ? `/${(req.params as any)[0]}` : "";

   No input validation on URL path parameters could allow path traversal attacks.

4. MEDIUM: Race condition in packages/gateway/src/gateway/index.ts:73-78

   const isRegistered = await this.jobRouter.isWorkerRegistered(deploymentName);
   if (!isRegistered) {
     await this.jobRouter.registerWorker(deploymentName);
   }

   Multiple workers could register simultaneously without proper locking.

5. MEDIUM: Credential exposure in logs - packages/gateway/src/index.ts:133-137 logs truncated tokens but should avoid logging credentials in production entirely.

🚀 Performance Considerations

Good:

• Efficient credential caching in McpConfigService
• Connection reuse in Worker Gateway
• Proper cleanup in shutdown handlers

Areas for improvement:

• Config reloading: Service loads from file/URL on every request - should implement file watching or caching with TTL
• Redis connection pooling: Each credential store operation could benefit from connection pooling
• Stream buffering: Proxy doesn't implement proper backpressure handling

📋 Test Coverage & Documentation

Missing test coverage for:

• MCP proxy authentication flows
• Error handling in credential store operations
• Environment variable validation edge cases
• Worker gateway connection management

Documentation gaps:

• Missing API documentation for new MCP endpoints
• No migration guide for the new CLI interface
• MCP integration workflow needs more detail

🔧 Specific Fix Recommendations

1. Fix JSON parsing safety:

   // In packages/gateway/src/mcp/credential-store.ts:29
   try {
     return JSON.parse(value) as McpCredentialRecord;
   } catch (error) {
     logger.error("Failed to parse credential JSON", { error, key });
     return null;
   }

2. Add stream timeout and size limits:

   // In packages/gateway/src/mcp/proxy.ts
   const controller = new AbortController();
   const timeoutId = setTimeout(() => controller.abort(), 30000);
   let bytesRead = 0;
   const MAX_SIZE = 50 * 1024 * 1024; // 50MB limit

3. Validate URL paths:

   // Add path validation to prevent traversal
   const safePath = remainder.replace(/\.\./g, '').replace(/\/+/g, '/');

4. Add environment variable format validation:

   // Validate URL format, numeric ranges, etc.
   const validateDatabaseUrl = (url: string) => {
     return url.match(/^postgres(ql)?:\/\/.+/);
   };

✅ Positive Changes

The Makefile changes are well-done - they appropriately relax the .env requirement while maintaining clear messaging, which aligns perfectly with the CLI improvements. The new CLI startup process provides excellent error visibility.

The MCP integration foundation is solid and follows good architectural patterns.

---

Priority Issues to Address Before Merging:

1. JSON parsing safety (Critical)
2. Stream timeout handling (High)
3. URL path validation (High)
4. Add basic tests for new functionality (Medium)

Overall this PR makes significant progress toward MCP integration and improves the development experience. The security and reliability issues should be addressed, but the foundation is well-architected.

---

[claude]

Claude finished @buremba's task —— View job

---

PR Review: CLI startup for gateway env validation

Review Checklist:

• Read project guidelines and understand codebase structure
• Analyze changed files for code quality and best practices
• Check for potential bugs and security issues
• Assess performance considerations
• Evaluate test coverage and documentation
• Provide final review summary

---

✅ Code Quality & Best Practices

Strong Points:

• Excellent CLI implementation using Commander.js in packages/gateway/src/index.ts:290-296 with proper error handling
• Well-structured MCP services with clear separation of concerns across config, credential store, and proxy components
• Robust environment validation with custom MissingRequiredEnvError class (packages/gateway/src/index.ts:99-107)
• Consistent logging throughout all components using the shared logger
• TypeScript types are comprehensive and well-defined

Minor Improvements:

• Consider extracting magic constants like port 8080 and timeout values
• Some complex functions like normalizeConfig() in packages/gateway/src/mcp/config-service.ts:198-221 could benefit from JSDoc documentation

⚠️ Critical Security & Bug Issues

1. CRITICAL: Unsafe JSON parsing in packages/gateway/src/mcp/credential-store.ts:29

   return JSON.parse(value) as McpCredentialRecord;

   This will throw if Redis contains malformed JSON. Should use safe parsing with try-catch.

2. HIGH: Memory leak risk in packages/gateway/src/mcp/proxy.ts:129-141

   while (true) {
     const { done, value } = await reader.read();
     // No timeout or error handling for infinite loop
   }

   The streaming loop lacks timeout or size limits, could cause memory issues with large responses.

3. HIGH: URL path traversal vulnerability in packages/gateway/src/mcp/proxy.ts:144-167

   const remainder = (req.params as any)[0] ? `/${(req.params as any)[0]}` : "";

   No input validation on URL path parameters could allow path traversal attacks.

4. MEDIUM: Race condition in packages/gateway/src/gateway/index.ts:73-78

   const isRegistered = await this.jobRouter.isWorkerRegistered(deploymentName);
   if (!isRegistered) {
     await this.jobRouter.registerWorker(deploymentName);
   }

   Multiple workers could register simultaneously without proper locking.

5. MEDIUM: Credential exposure in logs - packages/gateway/src/index.ts:133-137 logs truncated tokens but should avoid logging credentials in production entirely.

🚀 Performance Considerations

Good:

• Efficient credential caching in McpConfigService
• Connection reuse in Worker Gateway
• Proper cleanup in shutdown handlers

Areas for improvement:

• Config reloading: Service loads from file/URL on every request - should implement file watching or caching with TTL
• Redis connection pooling: Each credential store operation could benefit from connection pooling
• Stream buffering: Proxy doesn't implement proper backpressure handling

📋 Test Coverage & Documentation

Missing test coverage for:

• MCP proxy authentication flows
• Error handling in credential store operations
• Environment variable validation edge cases
• Worker gateway connection management

Documentation gaps:

• Missing API documentation for new MCP endpoints
• No migration guide for the new CLI interface
• MCP integration workflow needs more detail

🔧 Specific Fix Recommendations

1. Fix JSON parsing safety:

   // In packages/gateway/src/mcp/credential-store.ts:29
   try {
     return JSON.parse(value) as McpCredentialRecord;
   } catch (error) {
     logger.error("Failed to parse credential JSON", { error, key });
     return null;
   }

2. Add stream timeout and size limits:

   // In packages/gateway/src/mcp/proxy.ts
   const controller = new AbortController();
   const timeoutId = setTimeout(() => controller.abort(), 30000);
   let bytesRead = 0;
   const MAX_SIZE = 50 * 1024 * 1024; // 50MB limit

3. Validate URL paths:

   // Add path validation to prevent traversal
   const safePath = remainder.replace(/\.\./g, '').replace(/\/+/g, '/');

4. Add environment variable format validation:

   // Validate URL format, numeric ranges, etc.
   const validateDatabaseUrl = (url: string) => {
     return url.match(/^postgres(ql)?:\/\/.+/);
   };

✅ Positive Changes

The Makefile changes are well-done - they appropriately relax the .env requirement while maintaining clear messaging, which aligns perfectly with the CLI improvements. The new CLI startup process provides excellent error visibility.

The MCP integration foundation is solid and follows good architectural patterns.

---

Priority Issues to Address Before Merging:

1. JSON parsing safety (Critical)
2. Stream timeout handling (High)
3. URL path validation (High)
4. Add basic tests for new functionality (Medium)

Overall this PR makes significant progress toward MCP integration and improves the development experience. The security and reliability issues should be addressed, but the foundation is well-architected.

---