feat(mcp): intelligent API key validation with caching and cleanup

[mojwang]

Summary

• Enhanced MCP server setup to intelligently handle API keys and avoid unnecessary prompts
• Added validation caching to reduce API calls and improve performance
• Implemented automatic cleanup of old cache files to manage disk space

Key Features

1. Validation Caching

• API key validations are cached for 1 hour in /tmp/mcp-validated-keys-YYYYMMDD
• Reduces unnecessary API calls for repeated validations
• Cache entries automatically expire after the configured duration

2. Smart API Key Handling

• Checks for existing keys before prompting
• Validates keys only when necessary (uses cache when available)
• Only prompts for new keys if validation fails
• Supports non-interactive mode during updates

3. --skip-validation Flag

• New flag for faster updates: ./scripts/setup-claude-mcp.sh --update --skip-validation
• Completely bypasses API key validation when you know keys are valid
• Useful for CI/CD pipelines or quick updates

4. Improved User Feedback

• Clear status indicators:
  • ✓ = Valid/Success
  • ✗ = Invalid/Failed
  • ○ = Skipped/Not configured
• Informative messages explaining what's happening
• Helpful hints when validation fails

5. Automatic Cache Cleanup

• Removes cache files older than 1 day
• Cleans up empty cache files
• Prevents accumulation beyond 7 days worth of files
• Runs automatically on startup and during cleanup
• macOS-compatible implementation using stat

Benefits

• Faster updates: No prompts when keys are already configured and valid
• Better UX: Clear feedback about key status and validation progress
• Reduced API calls: Caching prevents repeated validation requests
• Disk space management: Automatic cleanup prevents cache accumulation
• Flexibility: Choose between validated or fast updates with --skip-validation

Test Plan

• Test with no existing API keys (should prompt)
• Test with valid existing keys (should use cached validation)
• Test with invalid keys (should re-prompt)
• Test --skip-validation flag (should bypass all validation)
• Test cache cleanup mechanism (old files removed, current kept)
• Verify macOS compatibility for file age checking

🤖 Generated with Claude Code

[claude]

Claude finished @mojwang's task —— View job

---

PR Code Review

Comprehensive code review focusing on code quality, security, performance, and potential issues.

Todo List:

• Read and analyze main changed files
• Review shell script security and best practices
• Analyze API key handling and caching implementation
• Check performance considerations
• Assess test coverage
• Provide comprehensive feedback

Code Quality & Best Practices

✅ Excellent Shell Script Practices

• Proper shebang: Uses #!/usr/bin/env bash consistently
• Error handling: set -e is used appropriately
• Signal safety: Proper cleanup functions with signal traps
• Modular design: Clean separation of concerns with focused functions

✅ Good Git Workflow Scripts

The new feature-worktree.sh and git-safe-commit.sh implement solid practices:

• Branch name validation with proper prefixes
• Protection against direct commits to main branches
• Smart worktree management for complex features

Security Analysis

✅ Strong API Key Handling

• Secure storage: API keys stored with chmod 600 permissions
• Input validation: Keys checked for shell special characters (scripts/setup-claude-mcp.sh:484)
• Environment isolation: Keys sourced from dedicated config file
• No logging: Keys not exposed in command output

⚠️ Minor Security Considerations

1. Cache file permissions: Validation cache files created with default permissions in /tmp (line 179). Consider using umask 077 before creation.
2. Curl timeouts: Good use of --connect-timeout 5 but missing --max-time for total timeout protection.

Performance & Caching Implementation

✅ Intelligent Caching Strategy

• Smart cache duration: 1-hour cache with automatic expiration (line 163)
• Efficient cleanup: Automatic removal of old cache files prevents accumulation
• macOS compatibility: Proper use of stat -f %m vs stat -c %Y for cross-platform support (line 199)

✅ Performance Optimizations

• Skip unnecessary validation: --skip-validation flag for fast updates
• Build optimization: Smart detection of when rebuilds are needed (lines 683, 760)
• Parallel execution awareness: Ready for parallel operations

Architecture & Design

✅ Well-Structured MCP Integration

• Clear separation: Different server types (node, python-uv, npx) handled appropriately
• Extensible design: Easy to add new server types via MCP_SERVER_TYPES array
• NPX server support: Smart handling of package-based servers without local installation

✅ Robust Error Handling

• Graceful degradation: Network failures don't break the flow
• Non-interactive mode: Proper CI/CD support
• Validation states: Clear distinction between valid, invalid, and unknown states

Potential Issues & Improvements

🔧 Minor Issues Found

1. scripts/setup-claude-mcp.sh:676: Reference to undefined MCP_SERVER_TYPES variable should reference the array from common.sh
2. Error code inconsistency: Some functions return 2 for "no updates" but this isn't documented as a standard
3. TaskMaster script: Missing proper error handling in Python JSON manipulation (lines 117-148)

🔧 Enhancement Opportunities

1. Rate limiting: Consider adding rate limiting for API validation to prevent API abuse
2. Retry logic: Add exponential backoff for network operations
3. Validation metrics: Track validation success/failure rates
4. Config validation: JSON schema validation for Claude Desktop config

Test Coverage Assessment

❌ Limited Test Coverage Identified

• No tests found for the new caching functionality
• Missing validation tests for API key validation logic
• No integration tests for the complete setup flow
• Cache cleanup logic lacks unit tests

Recommendation: Add tests for:

# Critical functions to test
- is_validation_cached()
- cache_validation()
- cleanup_old_cache_files()
- validate_api_key()

Security Recommendations

1. Enhance cache security:

   # Before creating cache files
   umask 077
   touch "$VALIDATION_CACHE_FILE"

2. Add total timeout protection:

   curl -s -o /dev/null -w "%{http_code}" \
     --connect-timeout 5 --max-time 10 \
     # ... rest of curl command

Overall Assessment

🎉 Excellent Implementation

This PR demonstrates high-quality engineering with:

• Strong security practices for API key management
• Smart performance optimizations through caching
• Robust error handling and graceful degradation
• Clean, maintainable code following project conventions

The caching implementation is particularly well-designed, providing significant UX improvements while maintaining security and reliability.

Score: 8.5/10 - Very solid implementation with minor areas for improvement in test coverage and edge case handling.

[claude]

Claude finished @mojwang's task —— View job

---

PR Code Review

Comprehensive code review focusing on code quality, security, performance, and potential issues.

Todo List:

• Read and analyze main changed files
• Review shell script security and best practices
• Analyze API key handling and caching implementation
• Check performance considerations
• Assess test coverage
• Provide comprehensive feedback

Code Quality & Best Practices

✅ Excellent Shell Script Practices

• Proper shebang: Uses #!/usr/bin/env bash consistently
• Error handling: set -e is used appropriately
• Signal safety: Proper cleanup functions with signal traps
• Modular design: Clean separation of concerns with focused functions

✅ Good Git Workflow Scripts

The new feature-worktree.sh and git-safe-commit.sh implement solid practices:

• Branch name validation with proper prefixes
• Protection against direct commits to main branches
• Smart worktree management for complex features

Security Analysis

✅ Strong API Key Handling

• Secure storage: API keys stored with chmod 600 permissions
• Input validation: Keys checked for shell special characters (scripts/setup-claude-mcp.sh:484)
• Environment isolation: Keys sourced from dedicated config file
• No logging: Keys not exposed in command output

⚠️ Minor Security Considerations

1. Cache file permissions: Validation cache files created with default permissions in /tmp (line 179). Consider using umask 077 before creation.
2. Curl timeouts: Good use of --connect-timeout 5 but missing --max-time for total timeout protection.

Performance & Caching Implementation

✅ Intelligent Caching Strategy

• Smart cache duration: 1-hour cache with automatic expiration (line 163)
• Efficient cleanup: Automatic removal of old cache files prevents accumulation
• macOS compatibility: Proper use of stat -f %m vs stat -c %Y for cross-platform support (line 199)

✅ Performance Optimizations

• Skip unnecessary validation: --skip-validation flag for fast updates
• Build optimization: Smart detection of when rebuilds are needed (lines 683, 760)
• Parallel execution awareness: Ready for parallel operations

Architecture & Design

✅ Well-Structured MCP Integration

• Clear separation: Different server types (node, python-uv, npx) handled appropriately
• Extensible design: Easy to add new server types via MCP_SERVER_TYPES array
• NPX server support: Smart handling of package-based servers without local installation

✅ Robust Error Handling

• Graceful degradation: Network failures don't break the flow
• Non-interactive mode: Proper CI/CD support
• Validation states: Clear distinction between valid, invalid, and unknown states

Potential Issues & Improvements

🔧 Minor Issues Found

1. scripts/setup-claude-mcp.sh:676: Reference to undefined MCP_SERVER_TYPES variable should reference the array from common.sh
2. Error code inconsistency: Some functions return 2 for "no updates" but this isn't documented as a standard
3. TaskMaster script: Missing proper error handling in Python JSON manipulation (lines 117-148)

🔧 Enhancement Opportunities

1. Rate limiting: Consider adding rate limiting for API validation to prevent API abuse
2. Retry logic: Add exponential backoff for network operations
3. Validation metrics: Track validation success/failure rates
4. Config validation: JSON schema validation for Claude Desktop config

Test Coverage Assessment

❌ Limited Test Coverage Identified

• No tests found for the new caching functionality
• Missing validation tests for API key validation logic
• No integration tests for the complete setup flow
• Cache cleanup logic lacks unit tests

Recommendation: Add tests for:

# Critical functions to test
- is_validation_cached()
- cache_validation()
- cleanup_old_cache_files()
- validate_api_key()

Security Recommendations

1. Enhance cache security:

   # Before creating cache files
   umask 077
   touch "$VALIDATION_CACHE_FILE"

2. Add total timeout protection:

   curl -s -o /dev/null -w "%{http_code}" \
     --connect-timeout 5 --max-time 10 \
     # ... rest of curl command

Overall Assessment

🎉 Excellent Implementation

This PR demonstrates high-quality engineering with:

• Strong security practices for API key management
• Smart performance optimizations through caching
• Robust error handling and graceful degradation
• Clean, maintainable code following project conventions

The caching implementation is particularly well-designed, providing significant UX improvements while maintaining security and reliability.

Score: 8.5/10 - Very solid implementation with minor areas for improvement in test coverage and edge case handling.

[claude]

Claude finished @mojwang's task —— View job

---

PR Code Review

Comprehensive code review focusing on code quality, security, performance, and potential issues.

Todo List:

• Read and analyze main changed files
• Review shell script security and best practices
• Analyze API key handling and caching implementation
• Check performance considerations
• Assess test coverage
• Provide comprehensive feedback

Code Quality & Best Practices

✅ Excellent Shell Script Practices

• Proper shebang: Uses #!/usr/bin/env bash consistently across all files
• Error handling: set -e is used appropriately in all main scripts
• Signal safety: Proper cleanup functions with signal traps (scripts/setup-claude-mcp.sh:48-49)
• Modular design: Clean separation of concerns with focused functions

✅ Good Git Workflow Scripts

The new feature-worktree.sh and git-safe-commit.sh implement solid practices:

• Branch name validation with proper prefixes (scripts/feature-worktree.sh:13-26)
• Protection against direct commits to main branches
• Smart worktree management for complex features

Security Analysis

✅ Strong API Key Handling

• Secure storage: API keys stored with chmod 600 permissions (scripts/setup-claude-mcp.sh:508)
• Input validation: Keys checked for shell special characters (scripts/setup-claude-mcp.sh:492)
• Environment isolation: Keys sourced from dedicated config file
• No logging: Keys not exposed in command output

✅ Enhanced Cache Security

• Secure file creation: Uses umask 077 before creating cache files (scripts/setup-claude-mcp.sh:184)
• Proper permissions: Cache files created with restrictive permissions
• Network timeouts: Good use of --connect-timeout 5 --max-time 10 for API validation (scripts/setup-claude-mcp.sh:374, 389)

Performance & Caching Implementation

✅ Intelligent Caching Strategy

• Smart cache duration: 1-hour cache with automatic expiration (scripts/setup-claude-mcp.sh:163)
• Efficient cleanup: Automatic removal of old cache files prevents accumulation (scripts/setup-claude-mcp.sh:198-228)
• macOS compatibility: Proper use of stat -f %m vs stat -c %Y for cross-platform support (scripts/setup-claude-mcp.sh:207)

✅ Performance Optimizations

• Skip unnecessary validation: --skip-validation flag for fast updates
• Build optimization: Smart detection of when rebuilds are needed (scripts/setup-claude-mcp.sh:691, 768)
• Parallel execution awareness: Ready for parallel operations

Architecture & Design

✅ Well-Structured MCP Integration

• Clear separation: Different server types (node, python-uv, npx) handled appropriately
• Extensible design: Easy to add new server types via MCP_SERVER_TYPES array
• NPX server support: Smart handling of package-based servers without local installation

✅ Robust Error Handling

• Graceful degradation: Network failures don't break the flow
• Non-interactive mode: Proper CI/CD support
• Validation states: Clear distinction between valid, invalid, and unknown states

Potential Issues & Improvements

🔧 Critical Issues Found

1. Missing Function Definition: get_mcp_server_type function is called (scripts/setup-claude-mcp.sh:684, 891, 1230) but not defined anywhere. This should be added to lib/common.sh:

   get_mcp_server_type() {
       local server_name="$1"
       echo "${MCP_SERVER_TYPES[$server_name]:-}"
   }

🔧 Minor Issues Found

2. TaskMaster script: Missing proper error handling in Python JSON manipulation (scripts/setup-taskmaster.sh:117-164)
3. Error code inconsistency: Some functions return 2 for "no updates" but this isn't documented as a standard

Test Coverage Assessment

✅ Good Test Coverage Added

• Unit tests for caching: Comprehensive tests for cache functions (tests/unit/test_mcp_cache.sh)
• Cache function isolation: Proper extraction of functions for testing (tests/unit/test_mcp_cache_functions.sh)
• Edge case testing: Tests for permissions, expiration, and error handling
• Security validation: Tests verify cache files have secure permissions (600)

🔧 Areas for Enhancement

1. Integration tests: Missing tests for the complete setup flow
2. API validation tests: No tests for actual API key validation logic
3. Cross-platform tests: Need tests for both macOS and Linux stat commands

Security Recommendations

1. Cache cleanup enhancement: Consider adding file size limits to prevent disk exhaustion
2. API rate limiting: Consider adding rate limiting for API validation to prevent abuse
3. Validation retry logic: Add exponential backoff for network operations

Overall Assessment

🎉 Excellent Implementation

This PR demonstrates high-quality engineering with:

• Strong security practices for API key management
• Smart performance optimizations through caching
• Robust error handling and graceful degradation
• Clean, maintainable code following project conventions
• Comprehensive test coverage for new caching functionality

The caching implementation is particularly well-designed, providing significant UX improvements while maintaining security and reliability.

Score: 8.5/10 - Very solid implementation with only one critical missing function and minor areas for improvement.

---

[mojwang]

✅ Security Review Findings Addressed

All security and code quality issues from the PR review have been fixed in commit 3decde6:

Security Improvements

• Cache file permissions: Added umask 077 before creating cache files to ensure secure 600 permissions
• Curl timeouts: Added --max-time 10 to all curl commands for total timeout protection

Code Quality Fixes

• MCP_SERVER_TYPES: Replaced direct array access with get_mcp_server_type() function calls
• TaskMaster error handling: Added comprehensive Python exception handling (JSONDecodeError, FileNotFoundError, IOError)
• Documentation: Added exit code 2 documentation as standard for "no updates needed"

Testing

• New tests added: Created tests/unit/test_mcp_cache.sh with comprehensive cache testing
• Test coverage: Validates permissions, expiration, cleanup, and error handling
• All tests passing: Verified with ./tests/unit/test_mcp_cache.sh

Ready for final review and merge.

[claude]

Claude finished @mojwang's task —— View job

---

PR Code Review

Comprehensive code review focusing on code quality, security, performance, and potential issues.

Todo List:

• Read and analyze main changed files
• Review shell script security and best practices
• Analyze API key handling and caching implementation
• Check performance considerations
• Assess test coverage
• Provide comprehensive feedback

Code Quality & Best Practices

✅ Excellent Shell Script Practices

• Proper shebang: Uses #!/usr/bin/env bash consistently across all files
• Error handling: set -e is used appropriately in all main scripts
• Signal safety: Proper cleanup functions with signal traps (scripts/setup-claude-mcp.sh:48-49)
• Modular design: Clean separation of concerns with focused functions

✅ Good Git Workflow Scripts

The new feature-worktree.sh and git-safe-commit.sh implement solid practices:

• Branch name validation with proper prefixes (scripts/feature-worktree.sh:13-26)
• Protection against direct commits to main branches
• Smart worktree management for complex features

Security Analysis

✅ Strong API Key Handling

• Secure storage: API keys stored with chmod 600 permissions (scripts/setup-claude-mcp.sh:508)
• Input validation: Keys checked for shell special characters (scripts/setup-claude-mcp.sh:492)
• Environment isolation: Keys sourced from dedicated config file
• No logging: Keys not exposed in command output

✅ Enhanced Cache Security

• Secure file creation: Uses umask 077 before creating cache files (scripts/setup-claude-mcp.sh:184)
• Proper permissions: Cache files created with restrictive permissions
• Network timeouts: Good use of --connect-timeout 5 --max-time 10 for API validation (scripts/setup-claude-mcp.sh:374, 389)

Performance & Caching Implementation

✅ Intelligent Caching Strategy

• Smart cache duration: 1-hour cache with automatic expiration (scripts/setup-claude-mcp.sh:163)
• Efficient cleanup: Automatic removal of old cache files prevents accumulation (scripts/setup-claude-mcp.sh:198-228)
• macOS compatibility: Proper use of stat -f %m vs stat -c %Y for cross-platform support (scripts/setup-claude-mcp.sh:207)

✅ Performance Optimizations

• Skip unnecessary validation: --skip-validation flag for fast updates
• Build optimization: Smart detection of when rebuilds are needed (scripts/setup-claude-mcp.sh:691, 768)
• Parallel execution awareness: Ready for parallel operations

Architecture & Design

✅ Well-Structured MCP Integration

• Clear separation: Different server types (node, python-uv, npx) handled appropriately
• Extensible design: Easy to add new server types via MCP_SERVER_TYPES array
• NPX server support: Smart handling of package-based servers without local installation

✅ Robust Error Handling

• Graceful degradation: Network failures don't break the flow
• Non-interactive mode: Proper CI/CD support
• Validation states: Clear distinction between valid, invalid, and unknown states

Potential Issues & Improvements

✅ Critical Issues Fixed

Previous reviews identified a missing get_mcp_server_type function, but I can confirm this has been properly implemented:

• Function defined: Present in lib/common.sh:631-634
• Proper usage: Called correctly in setup-claude-mcp.sh (lines 684, 891, 1230)
• Good implementation: Returns server type from associative array with fallback

🔧 Minor Issues Found

1. TaskMaster script: Missing comprehensive error handling in Python JSON manipulation (scripts/setup-taskmaster.sh:117-164)
2. Error code documentation: Exit code 2 for "no updates" could be better documented

Test Coverage Assessment

✅ Good Test Coverage Added

• Unit tests for caching: Comprehensive tests for cache functions (tests/unit/test_mcp_cache.sh)
• Cache function isolation: Proper extraction of functions for testing (tests/unit/test_mcp_cache_functions.sh)
• Edge case testing: Tests for permissions, expiration, and error handling
• Security validation: Tests verify cache files have secure permissions (600)

🔧 Areas for Enhancement

1. Integration tests: Missing tests for the complete setup flow
2. API validation tests: No tests for actual API key validation logic
3. Cross-platform tests: Need tests for both macOS and Linux stat commands

Security Recommendations

✅ Previously Identified Issues Fixed

The PR has addressed all major security concerns from previous reviews:

1. Cache file permissions: Now uses umask 077 before creating cache files
2. Network timeouts: Added --max-time 10 to all curl commands
3. Error handling: Enhanced Python exception handling in TaskMaster script

Overall Assessment

🎉 Excellent Implementation

This PR demonstrates high-quality engineering with:

• Strong security practices for API key management
• Smart performance optimizations through caching
• Robust error handling and graceful degradation
• Clean, maintainable code following project conventions
• Comprehensive test coverage for new caching functionality

The caching implementation is particularly well-designed, providing significant UX improvements while maintaining security and reliability. All critical issues from previous reviews have been properly addressed.

Score: 9/10 - Outstanding implementation with only minor areas for improvement.

---

Job Run

[mojwang]

✅ Critical Issue Fixed

Added the missing get_mcp_server_type function to lib/common.sh:

# Get the type of an MCP server
get_mcp_server_type() {
    local server_name="$1"
    echo "${MCP_SERVER_TYPES[$server_name]:-}"
}

This resolves the undefined function errors at:

• scripts/setup-claude-mcp.sh:684
• scripts/setup-claude-mcp.sh:891
• scripts/setup-claude-mcp.sh:1230

Summary of All Fixes

✅ Critical: Added missing get_mcp_server_type function
✅ Security: Cache files created with 600 permissions
✅ Security: Added --max-time to curl commands
✅ Quality: TaskMaster Python error handling is comprehensive
✅ Documentation: Exit code 2 documented where used

All issues from the PR review have been addressed. Ready for merge.

[mojwang]

🎉 This PR is included in version 3.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀