Validation - #38
Merged
Merged
Conversation
Replace Pydantic models with dataclasses and custom validation throughout the codebase. This eliminates all external dependencies from the core package while maintaining full type safety, validation, and security. Changes: - Add shannot/validation.py with custom ValidationError and validation helpers - Convert all Pydantic BaseModel classes to dataclasses in config.py and tools.py - Add from_dict() class methods for validation (replaces .model_validate()) - Add __post_init__() to SSHExecutorConfig for path expansion - Update MCP server to use ValidationError from shannot.validation - Update all tests to use custom validation instead of Pydantic - Remove pydantic from all dependencies in pyproject.toml - Remove pydantic-ai optional dependency entirely - Update documentation to reflect dataclass-based models Benefits: - Zero external dependencies for core package - Maintained all validation logic and security features - Preserved type safety with full type hints - Better performance without Pydantic overhead - All 90 tests passing Validation maintained: - Command injection prevention via list validation - Path expansion for SSH keys and known_hosts - Type checking for all configuration fields - Port range validation (1-65535) - Boolean validation for security flags - Required field validation
This commit addresses all critical and high-priority validation issues
identified in the comprehensive validation audit, including a security
vulnerability and several validation bypasses.
Critical Security Fixes:
- Add validate_safe_path() helper to prevent path traversal attacks
- Protect FileReadInput and DirectoryListInput from '../' exploits
- Add command validation to SandboxManager.run() and run_async()
Validation Infrastructure Improvements:
- Replace cast() with explicit validation in CLI handlers (15 locations)
* cast() is only a type hint - provides NO runtime validation
* Now using validate_type() and getattr() for proper validation
- Standardize on ValidationError for validation failures in config.py
- Add validate_command() calls to core sandbox execution paths
Files Modified:
- shannot/validation.py: Added validate_safe_path() security helper
- shannot/cli.py: Replaced all cast() calls with proper validation
- shannot/config.py: Changed ValueError to ValidationError (2 locations)
- shannot/sandbox.py: Added command validation to run() and run_async()
- shannot/tools.py: Added path traversal protection (2 classes)
- shannot/executors/ssh.py: Enhanced validation in constructor and run_command
- shannot/executors/local.py: Enhanced validation in constructor and run_command
- shannot/process.py: Added ProcessResult.__post_init__ validation
- tests/test_config.py: Updated test to expect ValidationError
- tests/test_sandbox.py: Updated tests to expect ValidationError
Security Impact:
BEFORE: FileReadInput.from_dict({"path": "../../../etc/passwd"})
→ Would read /etc/passwd!
AFTER: → ValidationError: path: contains path traversal pattern '..'
Test Results: ✅ 110 passed, 59 skipped, 0 failed
Co-authored-by: Comprehensive Validation Audit
Related-to: VALIDATION_AUDIT_SUMMARY.md
Remove unused imports that are no longer needed after switching to validate_safe_path() and other validation improvements: - shannot/sandbox.py: Remove unused validate_bool, validate_type - shannot/tools.py: Remove unused validate_type - tests/test_sandbox.py: Remove unused SandboxError These imports were left over from refactoring validation code and are no longer referenced anywhere in the files.
|
❌ CHANGELOG.md is out of date Please update the changelog by running: make changelogOr manually: git-cliff --config cliff.toml -o CHANGELOG.mdThen commit the updated |
- Resolved conflicts in pyproject.toml (kept tomli dependencies from main) - Resolved conflicts in shannot/cli.py (kept enhanced MCP install from main) - Resolved conflicts in shannot/tools.py (kept validate_safe_path security checks from validation) - Resolved conflicts in shannot/validation.py (kept enhanced validation functions) - Resolved conflicts in tests/test_mcp_server.py (kept MCP availability check from main) - Regenerated uv.lock All tests passing (130 passed, 47 skipped)
TOML support is only needed for: - MCP install to TOML-based clients (codex) - Remote target configuration Moved tomli/tomli-w from core dependencies to: - mcp extras (for MCP installation) - remote extras (for config management) - all extras (includes both) This keeps the core package lightweight - users who only use local sandbox execution don't need TOML libraries.
validate_safe_path is used instead for path validation, making validate_type import unnecessary.
MCP Server Test ResultsMCP Server Test SummaryGenerated: /__w/shannot/shannot Test ExecutionTest results from mcp-probe: {
"metadata": {
"generated_at": "2025-11-07T07:21:01.702394908+00:00",
"mcp_probe_version": "0.3.0",
"total_duration_ms": 1201,
"transport_type": "stdio"
},
"protocol_info": {
"available_protocols": [
{
"endpoints": [
"/mcp"
],
"session_management": "Mcp-Session-Id header",
"spec_date": "2025-03-26",
"supported_methods": [
"initialize",
"initialized",
"tools/list",
"tools/call",
"resources/list",
"resources/read",
"prompts/list",
"prompts/get",
"logging/setLevel",
"notifications/*"
],
"version": "Modern Streamable HTTP"
},
{
"endpoints": [
"/sse",
"/events"
],
"session_management": "sessionId query parameter",
"spec_date": "2024-11-05",
"supported_methods": [
"initialize",
"initialized",
"tools/list",
"tools/call",
"resources/list",
"resources/read",
"prompts/list",
"prompts/get",
"logging/setLevel"
],
"version": "Legacy HTTP+SSE"
},
{
"endpoints": [
"stdio"
],
"session_management": "N/A (stdio)",
"spec_date": "2025-03-26",
"supported_methods": [
"initialize",
"initialized",
"tools/list",
"tools/call",
"resources/list",
"resources/read",
"prompts/list",
"prompts/get",
"logging/setLevel",
"notifications/*"
],
"version": "Standard Transport"
}
],
"detected_protocol": {
"endpoints": [
"stdio"
],
"session_management": "N/A (stdio)",
"spec_date": "2025-03-26",
"supported_methods": [
"initialize",
"initialized",
"tools/list",
"tools/call",
"resources/list",
"resources/read",
"prompts/list",
"prompts/get",
"logging/setLevel",
"notifications/*"
],
"version": "Standard Transport"
}
},
"results": [
{
"details": null,
"duration_ms": 0,
"message": "Successfully created MCP client",
"name": "Connection",
"status": "PASS"
},
{
"details": {
"name": "shannot-sandbox",
"protocol_version": "2025-03-26",
"version": "0.2.1"
},
"duration_ms": 395,
"message": "Connected to shannot-sandbox v0.2.1",
"name": "Initialization",
"status": "PASS"
},
{
"details": null,
"duration_ms": 1,
"message": "Server capabilities discovered successfully",
"name": "Capability Discovery",
"status": "PASS"
},
{
"details": [
{
"description": "Execute read-only commands in 'minimal' sandbox on local sandbox. Allowed commands include: ls, /usr/bin/ls, /bin/ls, cat, /usr/bin/cat, ... (11 total). network isolated. Provide arguments as {\"command\": [\"ls\", \"/\"]}.",
"input_schema": {
"properties": {
"command": {
"description": "Command and arguments to execute",
"items": {
"type": "string"
},
"type": "array"
}
},
"required": [
"command"
],
"type": "object"
},
"name": "sandbox_minimal"
},
{
"description": "Execute read-only commands in 'diagnostics' sandbox on local sandbox. Allowed commands include: ls, /usr/bin/ls, /bin/ls, cat, /usr/bin/cat, ... (31 total). network isolated. Provide arguments as {\"command\": [\"ls\", \"/\"]}.",
"input_schema": {
"properties": {
"command": {
"description": "Command and arguments to execute",
"items": {
"type": "string"
},
"type": "array"
}
},
"required": [
"command"
],
"type": "object"
},
"name": "sandbox_diagnostics"
},
{
"description": "Execute read-only commands in 'readonly' sandbox on local sandbox. Allowed commands include: ls, /usr/bin/ls, cat, /usr/bin/cat, head, ... (13 total). network isolated. Provide arguments as {\"command\": [\"ls\", \"/\"]}.",
"input_schema": {
"properties": {
"command": {
"description": "Command and arguments to execute",
"items": {
"type": "string"
},
"type": "array"
}
},
"required": [
"command"
],
"type": "object"
},
"name": "sandbox_readonly"
}
],
"duration_ms": 1,
"message": "Successfully listed 3 tools",
"name": "Tools Listing",
"status": "PASS"
},
{
"details": [
{
"description": "Configuration for minimal sandbox profile",
"name": "Sandbox Profile: minimal",
"uri": "sandbox://profiles/minimal"
},
{
"description": "Configuration for diagnostics sandbox profile",
"name": "Sandbox Profile: diagnostics",
"uri": "sandbox://profiles/diagnostics"
},
{
"description": "Configuration for readonly sandbox profile",
"name": "Sandbox Profile: readonly",
"uri": "sandbox://profiles/readonly"
}
],
"duration_ms": 1,
"message": "Successfully listed 3 resources",
"name": "Resources Listing",
"status": "PASS"
},
{
"details": [
{
"arguments": [
{
"description": "Target environment for context (e.g., 'production', 'staging')",
"name": "target",
"required": false
}
],
"description": "Perform a comprehensive system health assessment checking disk space, memory, CPU, processes, and uptime",
"name": "system-health-check"
},
{
"arguments": [
{
"description": "Observed symptom (e.g., 'slow response', 'high load', 'memory exhaustion')",
"name": "symptom",
"required": false
}
],
"description": "Diagnose performance issues by checking resource consumption and identifying bottlenecks",
"name": "investigate-performance"
},
{
"arguments": [
{
"description": "Specific log file path to analyze (e.g., '/var/log/syslog')",
"name": "log_path",
"required": false
},
{
"description": "Time period to analyze (e.g., 'last hour', 'today')",
"name": "timeframe",
"required": false
}
],
"description": "Search and analyze system logs to identify errors, warnings, and anomalies",
"name": "analyze-logs"
},
{
"arguments": [
{
"description": "Alert if disk usage exceeds this percentage (e.g., '80')",
"name": "threshold",
"required": false
}
],
"description": "Audit disk space usage to identify what's consuming storage and potential cleanup targets",
"name": "disk-usage-audit"
},
{
"arguments": [
{
"description": "Sort criteria: 'cpu', 'memory', or 'time'",
"name": "sort_by",
"required": false
}
],
"description": "Monitor and analyze running processes to identify resource-intensive or unusual activity",
"name": "monitor-processes"
},
{
"arguments": [
{
"description": "Name of the service to check (e.g., 'nginx', 'postgresql')",
"name": "service_name",
"required": true
}
],
"description": "Check the status and health of a specific system service",
"name": "check-service-status"
}
],
"duration_ms": 0,
"message": "Successfully listed 6 prompts",
"name": "Prompts Listing",
"status": "PASS"
},
{
"details": {
"code": -32602,
"data": "",
"message": "Invalid request parameters"
},
"duration_ms": 0,
"message": "Unexpected error code: -32602",
"name": "Error Handling",
"status": "WARN"
}
],
"summary": {
"failed": 0,
"passed": 6,
"skipped": 0,
"total_tests": 7,
"warnings": 1
}
}Artifacts
|
The config.get_executor_config() method now raises ValidationError instead of ValueError for consistency with the validation refactoring. Updated test to match the new behavior.
MCP Server Test ResultsMCP Server Test SummaryGenerated: /__w/shannot/shannot Test ExecutionTest results from mcp-probe: {
"metadata": {
"generated_at": "2025-11-07T08:26:34.247529099+00:00",
"mcp_probe_version": "0.3.0",
"total_duration_ms": 1213,
"transport_type": "stdio"
},
"protocol_info": {
"available_protocols": [
{
"endpoints": [
"/mcp"
],
"session_management": "Mcp-Session-Id header",
"spec_date": "2025-03-26",
"supported_methods": [
"initialize",
"initialized",
"tools/list",
"tools/call",
"resources/list",
"resources/read",
"prompts/list",
"prompts/get",
"logging/setLevel",
"notifications/*"
],
"version": "Modern Streamable HTTP"
},
{
"endpoints": [
"/sse",
"/events"
],
"session_management": "sessionId query parameter",
"spec_date": "2024-11-05",
"supported_methods": [
"initialize",
"initialized",
"tools/list",
"tools/call",
"resources/list",
"resources/read",
"prompts/list",
"prompts/get",
"logging/setLevel"
],
"version": "Legacy HTTP+SSE"
},
{
"endpoints": [
"stdio"
],
"session_management": "N/A (stdio)",
"spec_date": "2025-03-26",
"supported_methods": [
"initialize",
"initialized",
"tools/list",
"tools/call",
"resources/list",
"resources/read",
"prompts/list",
"prompts/get",
"logging/setLevel",
"notifications/*"
],
"version": "Standard Transport"
}
],
"detected_protocol": {
"endpoints": [
"stdio"
],
"session_management": "N/A (stdio)",
"spec_date": "2025-03-26",
"supported_methods": [
"initialize",
"initialized",
"tools/list",
"tools/call",
"resources/list",
"resources/read",
"prompts/list",
"prompts/get",
"logging/setLevel",
"notifications/*"
],
"version": "Standard Transport"
}
},
"results": [
{
"details": null,
"duration_ms": 0,
"message": "Successfully created MCP client",
"name": "Connection",
"status": "PASS"
},
{
"details": {
"name": "shannot-sandbox",
"protocol_version": "2025-03-26",
"version": "0.2.1"
},
"duration_ms": 407,
"message": "Connected to shannot-sandbox v0.2.1",
"name": "Initialization",
"status": "PASS"
},
{
"details": null,
"duration_ms": 1,
"message": "Server capabilities discovered successfully",
"name": "Capability Discovery",
"status": "PASS"
},
{
"details": [
{
"description": "Execute read-only commands in 'minimal' sandbox on local sandbox. Allowed commands include: ls, /usr/bin/ls, /bin/ls, cat, /usr/bin/cat, ... (11 total). network isolated. Provide arguments as {\"command\": [\"ls\", \"/\"]}.",
"input_schema": {
"properties": {
"command": {
"description": "Command and arguments to execute",
"items": {
"type": "string"
},
"type": "array"
}
},
"required": [
"command"
],
"type": "object"
},
"name": "sandbox_minimal"
},
{
"description": "Execute read-only commands in 'diagnostics' sandbox on local sandbox. Allowed commands include: ls, /usr/bin/ls, /bin/ls, cat, /usr/bin/cat, ... (31 total). network isolated. Provide arguments as {\"command\": [\"ls\", \"/\"]}.",
"input_schema": {
"properties": {
"command": {
"description": "Command and arguments to execute",
"items": {
"type": "string"
},
"type": "array"
}
},
"required": [
"command"
],
"type": "object"
},
"name": "sandbox_diagnostics"
},
{
"description": "Execute read-only commands in 'readonly' sandbox on local sandbox. Allowed commands include: ls, /usr/bin/ls, cat, /usr/bin/cat, head, ... (13 total). network isolated. Provide arguments as {\"command\": [\"ls\", \"/\"]}.",
"input_schema": {
"properties": {
"command": {
"description": "Command and arguments to execute",
"items": {
"type": "string"
},
"type": "array"
}
},
"required": [
"command"
],
"type": "object"
},
"name": "sandbox_readonly"
}
],
"duration_ms": 1,
"message": "Successfully listed 3 tools",
"name": "Tools Listing",
"status": "PASS"
},
{
"details": [
{
"description": "Configuration for minimal sandbox profile",
"name": "Sandbox Profile: minimal",
"uri": "sandbox://profiles/minimal"
},
{
"description": "Configuration for diagnostics sandbox profile",
"name": "Sandbox Profile: diagnostics",
"uri": "sandbox://profiles/diagnostics"
},
{
"description": "Configuration for readonly sandbox profile",
"name": "Sandbox Profile: readonly",
"uri": "sandbox://profiles/readonly"
}
],
"duration_ms": 1,
"message": "Successfully listed 3 resources",
"name": "Resources Listing",
"status": "PASS"
},
{
"details": [
{
"arguments": [
{
"description": "Target environment for context (e.g., 'production', 'staging')",
"name": "target",
"required": false
}
],
"description": "Perform a comprehensive system health assessment checking disk space, memory, CPU, processes, and uptime",
"name": "system-health-check"
},
{
"arguments": [
{
"description": "Observed symptom (e.g., 'slow response', 'high load', 'memory exhaustion')",
"name": "symptom",
"required": false
}
],
"description": "Diagnose performance issues by checking resource consumption and identifying bottlenecks",
"name": "investigate-performance"
},
{
"arguments": [
{
"description": "Specific log file path to analyze (e.g., '/var/log/syslog')",
"name": "log_path",
"required": false
},
{
"description": "Time period to analyze (e.g., 'last hour', 'today')",
"name": "timeframe",
"required": false
}
],
"description": "Search and analyze system logs to identify errors, warnings, and anomalies",
"name": "analyze-logs"
},
{
"arguments": [
{
"description": "Alert if disk usage exceeds this percentage (e.g., '80')",
"name": "threshold",
"required": false
}
],
"description": "Audit disk space usage to identify what's consuming storage and potential cleanup targets",
"name": "disk-usage-audit"
},
{
"arguments": [
{
"description": "Sort criteria: 'cpu', 'memory', or 'time'",
"name": "sort_by",
"required": false
}
],
"description": "Monitor and analyze running processes to identify resource-intensive or unusual activity",
"name": "monitor-processes"
},
{
"arguments": [
{
"description": "Name of the service to check (e.g., 'nginx', 'postgresql')",
"name": "service_name",
"required": true
}
],
"description": "Check the status and health of a specific system service",
"name": "check-service-status"
}
],
"duration_ms": 1,
"message": "Successfully listed 6 prompts",
"name": "Prompts Listing",
"status": "PASS"
},
{
"details": {
"code": -32602,
"data": "",
"message": "Invalid request parameters"
},
"duration_ms": 0,
"message": "Unexpected error code: -32602",
"name": "Error Handling",
"status": "WARN"
}
],
"summary": {
"failed": 0,
"passed": 6,
"skipped": 0,
"total_tests": 7,
"warnings": 1
}
}Artifacts
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve validation throughout codebase