Files
owlen/crates/owlen-core/tests/file_write.rs
vikingowl 0728262a9e fix(core,mcp,security)!: resolve critical P0/P1 issues
BREAKING CHANGES:
- owlen-core no longer depends on ratatui/crossterm
- RemoteMcpClient constructors are now async
- MCP path validation is stricter (security hardening)

This commit resolves three critical issues identified in project analysis:

## P0-1: Extract TUI dependencies from owlen-core

Create owlen-ui-common crate to hold UI-agnostic color and theme
abstractions, removing architectural boundary violation.

Changes:
- Create new owlen-ui-common crate with abstract Color enum
- Move theme.rs from owlen-core to owlen-ui-common
- Define Color with Rgb and Named variants (no ratatui dependency)
- Create color conversion layer in owlen-tui (color_convert.rs)
- Update 35+ color usages with conversion wrappers
- Remove ratatui/crossterm from owlen-core dependencies

Benefits:
- owlen-core usable in headless/CLI contexts
- Enables future GUI frontends
- Reduces binary size for core library consumers

## P0-2: Fix blocking WebSocket connections

Convert RemoteMcpClient constructors to async, eliminating runtime
blocking that froze TUI for 30+ seconds on slow connections.

Changes:
- Make new_with_runtime(), new_with_config(), new() async
- Remove block_in_place wrappers for I/O operations
- Add 30-second connection timeout with tokio::time::timeout
- Update 15+ call sites across 10 files to await constructors
- Convert 4 test functions to #[tokio::test]

Benefits:
- TUI remains responsive during WebSocket connections
- Proper async I/O follows Rust best practices
- No more indefinite hangs

## P1-1: Secure path traversal vulnerabilities

Implement comprehensive path validation with 7 defense layers to
prevent file access outside workspace boundaries.

Changes:
- Create validate_safe_path() with multi-layer security:
  * URL decoding (prevents %2E%2E bypasses)
  * Absolute path rejection
  * Null byte protection
  * Windows-specific checks (UNC/device paths)
  * Lexical path cleaning (removes .. components)
  * Symlink resolution via canonicalization
  * Boundary verification with starts_with check
- Update 4 MCP resource functions (get/list/write/delete)
- Add 11 comprehensive security tests

Benefits:
- Blocks URL-encoded, absolute, UNC path attacks
- Prevents null byte injection
- Stops symlink escape attempts
- Cross-platform security (Windows/Linux/macOS)

## Test Results

- owlen-core: 109/109 tests pass (100%)
- owlen-tui: 52/53 tests pass (98%, 1 pre-existing failure)
- owlen-providers: 2/2 tests pass (100%)
- Build: cargo build --all succeeds

## Verification

- ✓ cargo tree -p owlen-core shows no TUI dependencies
- ✓ No block_in_place calls remain in MCP I/O code
- ✓ All 11 security tests pass

Fixes: #P0-1, #P0-2, #P1-1

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-29 12:31:20 +01:00

68 lines
2.4 KiB
Rust

use owlen_core::McpToolCall;
use owlen_core::mcp::remote_client::RemoteMcpClient;
use tempfile::tempdir;
#[tokio::test]
async fn remote_write_and_delete() {
// Build the server binary first
let status = std::process::Command::new("cargo")
.args(["build", "-p", "owlen-mcp-server"])
.status()
.expect("failed to build MCP server");
assert!(status.success());
// Use a temp dir as project root
let dir = tempdir().expect("tempdir");
std::env::set_current_dir(dir.path()).expect("set cwd");
let client = RemoteMcpClient::new().await.expect("client init");
// Write a file via MCP
let write_call = McpToolCall {
name: "resources_write".to_string(),
arguments: serde_json::json!({ "path": "test.txt", "content": "hello" }),
};
client.call_tool(write_call).await.expect("write tool");
// Verify content via local read (fallback check)
let content = std::fs::read_to_string(dir.path().join("test.txt")).expect("read back");
assert_eq!(content, "hello");
// Delete the file via MCP
let del_call = McpToolCall {
name: "resources_delete".to_string(),
arguments: serde_json::json!({ "path": "test.txt" }),
};
client.call_tool(del_call).await.expect("delete tool");
assert!(!dir.path().join("test.txt").exists());
}
#[tokio::test]
async fn write_outside_root_is_rejected() {
// Build server (already built in previous test, but ensure it exists)
let status = std::process::Command::new("cargo")
.args(["build", "-p", "owlen-mcp-server"])
.status()
.expect("failed to build MCP server");
assert!(status.success());
// Set cwd to a fresh temp dir
let dir = tempdir().expect("tempdir");
std::env::set_current_dir(dir.path()).expect("set cwd");
let client = RemoteMcpClient::new().await.expect("client init");
// Attempt to write outside the root using "../evil.txt"
let call = McpToolCall {
name: "resources_write".to_string(),
arguments: serde_json::json!({ "path": "../evil.txt", "content": "bad" }),
};
let err = client.call_tool(call).await.unwrap_err();
// The server returns a Network error with path traversal message
let err_str = format!("{err}");
assert!(
err_str.contains("path traversal") || err_str.contains("Path traversal"),
"Expected path traversal error, got: {}",
err_str
);
}