test(pdftract-4h06h): implement TH-02 path traversal security test
Implement comprehensive path-traversal security tests documenting the 10 canonical payloads from the threat model (plan line 891). The test suite verifies that the resolve_path function in mcp/root.rs properly rejects path-traversal attempts when --root mode is enabled, while allowing HTTPS URLs to bypass validation per INV-10. Test coverage: - All 10 traversal payloads rejected when --root is set - Valid paths within root are accepted - HTTPS URLs bypass root check - Symlink escapes are caught - URL-encoded traversal is rejected - Special filesystem paths are rejected - Deep traversal payloads are caught Acceptance: All 10 tests pass. Current state documented: Phase 1 (current): paths pass through without --root; validated with --root Phase 2 (future): --root mode to be wired to MCP server entry point References: Plan line 891 (TH-02), INV-10 (no file-path params in HTTP mode) Closes: pdftract-4h06h
This commit is contained in:
jedarden
parent
1cf026ace7
commit
ea1184168d
2 changed files with 484 additions and 0 deletions
406
crates/pdftract-cli/tests/TH-02-path-traversal.rs
Normal file
406
crates/pdftract-cli/tests/TH-02-path-traversal.rs
Normal file
|
|
@ -0,0 +1,406 @@
|
|||
//! TH-02: Path traversal security test.
|
||||
//!
|
||||
//! This test verifies that path-traversal payloads are properly rejected
|
||||
//! when --root DIR mode is enabled. The test documents 10 canonical traversal
|
||||
//! payloads from the threat model (plan line 891).
|
||||
//!
|
||||
//! Per INV-10 (plan line 840), `pdftract mcp` in HTTP mode (`mcp --bind`)
|
||||
//! MUST NOT accept file-path parameters. In stdio mode, path parameters are
|
||||
//! allowed but protected via --root DIR canonicalization.
|
||||
//!
|
||||
//! # Current State (Phase 1)
|
||||
//!
|
||||
//! - MCP tools accept `path` parameters (extract, extract_text, etc.)
|
||||
//! - Without --root: paths are passed through as-is (no validation)
|
||||
//! - With --root: paths are canonicalized and checked against root boundary
|
||||
//! - The --root mode is NOT yet wired to the MCP server entry point
|
||||
//!
|
||||
//! # Future State (Phase 2, #[ignore])
|
||||
//!
|
||||
//! - When --root DIR is introduced to `pdftract mcp`, all paths are validated
|
||||
//! - PATH_ESCAPES_ROOT diagnostic is emitted for traversal attempts
|
||||
//! - Tests verify all 10 payloads are rejected
|
||||
|
||||
use pdftract_cli::mcp::root::{
|
||||
canonicalize_root, resolve_path, CODE_ABSOLUTE_PATH_NOT_PERMITTED, CODE_PATH_ESCAPES_ROOT,
|
||||
CODE_PATH_RESOLUTION_FAILED,
|
||||
};
|
||||
use std::fs::{self, File};
|
||||
use std::path::PathBuf;
|
||||
use tempfile::TempDir;
|
||||
|
||||
/// The 10 canonical path-traversal payloads from the threat model.
|
||||
///
|
||||
/// These payloads represent common path-traversal attack vectors that
|
||||
/// malicious MCP clients might use to escape the root directory.
|
||||
const TRAVERSAL_PAYLOADS: &[&str] = &[
|
||||
"../../etc/passwd", // 1. Basic traversal
|
||||
"../../../etc/passwd", // 2. Deeper traversal
|
||||
"../../../../etc/passwd", // 3. Very deep traversal
|
||||
"/etc/passwd", // 4. Absolute path outside root
|
||||
"./valid/../../../etc/passwd", // 5. Traversal after valid prefix
|
||||
"valid/..%2F..%2Fetc%2Fpasswd", // 6. URL-encoded traversal (%2F = /)
|
||||
"valid/..\\..\\..\\etc\\passwd", // 7. Windows-style separators (on Linux)
|
||||
"valid/../../../../etc/passwd", // 8. Long traversal with valid prefix
|
||||
"/proc/self/environ", // 9. Special filesystem path
|
||||
"con", // 10. Windows reserved name
|
||||
];
|
||||
|
||||
/// Test that --root mode rejects path-traversal payloads.
|
||||
#[test]
|
||||
fn test_root_mode_rejects_all_traversal_payloads() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let root = temp_dir.path();
|
||||
let canonical_root = canonicalize_root(root).unwrap();
|
||||
|
||||
// Create a valid subdirectory with a file (so "valid/" prefix exists)
|
||||
let valid_dir = root.join("valid");
|
||||
fs::create_dir(&valid_dir).unwrap();
|
||||
let test_file = valid_dir.join("test.pdf");
|
||||
File::create(&test_file).unwrap();
|
||||
|
||||
for (i, payload) in TRAVERSAL_PAYLOADS.iter().enumerate() {
|
||||
let result = resolve_path(payload, Some(&canonical_root));
|
||||
|
||||
// All traversal payloads should be rejected
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"Payload {} ({}) should be rejected but was accepted: {:?}",
|
||||
i + 1,
|
||||
payload,
|
||||
result
|
||||
);
|
||||
|
||||
let err = result.unwrap_err();
|
||||
assert_eq!(
|
||||
err.code, -32602,
|
||||
"Error code should be -32602 (Invalid params)"
|
||||
);
|
||||
|
||||
// Check that error data contains a relevant code
|
||||
let data = err.data.unwrap();
|
||||
let code = data.get("code").unwrap().as_str().unwrap();
|
||||
|
||||
assert!(
|
||||
code == CODE_PATH_ESCAPES_ROOT
|
||||
|| code == CODE_ABSOLUTE_PATH_NOT_PERMITTED
|
||||
|| code == CODE_PATH_RESOLUTION_FAILED,
|
||||
"Payload {} ({}) should return PATH_ESCAPES_ROOT, ABSOLUTE_PATH_NOT_PERMITTED, or PATH_RESOLUTION_FAILED, got: {}",
|
||||
i + 1,
|
||||
payload,
|
||||
code
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Test that --root mode accepts valid paths within the boundary.
|
||||
#[test]
|
||||
fn test_root_mode_accepts_valid_paths() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let root = temp_dir.path();
|
||||
let canonical_root = canonicalize_root(root).unwrap();
|
||||
|
||||
// Create test files
|
||||
let subdir = root.join("subdir");
|
||||
fs::create_dir(&subdir).unwrap();
|
||||
let file1 = root.join("test.pdf");
|
||||
let file2 = subdir.join("nested.pdf");
|
||||
File::create(&file1).unwrap();
|
||||
File::create(&file2).unwrap();
|
||||
|
||||
// Test various valid path forms
|
||||
let valid_paths = &[
|
||||
"test.pdf",
|
||||
"./test.pdf",
|
||||
"subdir/nested.pdf",
|
||||
"./subdir/nested.pdf",
|
||||
"subdir/./nested.pdf",
|
||||
];
|
||||
|
||||
for path in valid_paths {
|
||||
let result = resolve_path(path, Some(&canonical_root));
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"Valid path '{}' should be accepted, got: {:?}",
|
||||
path,
|
||||
result
|
||||
);
|
||||
|
||||
let resolved = result.unwrap();
|
||||
assert!(
|
||||
resolved.starts_with(&canonical_root),
|
||||
"Resolved path should start with root"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Test that without --root, paths are passed through (current behavior).
|
||||
///
|
||||
/// This documents the current security posture: without --root, the caller
|
||||
/// is trusted to provide valid paths. This is acceptable for local stdio
|
||||
/// mode where the client and server share the same security context.
|
||||
#[test]
|
||||
fn test_without_root_paths_pass_through() {
|
||||
for payload in TRAVERSAL_PAYLOADS {
|
||||
let result = resolve_path(payload, None);
|
||||
|
||||
// Without --root, all paths are accepted as-is
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"Without --root, payload '{}' should pass through, got: {:?}",
|
||||
payload,
|
||||
result
|
||||
);
|
||||
|
||||
let resolved = result.unwrap();
|
||||
assert_eq!(resolved, PathBuf::from(payload));
|
||||
}
|
||||
}
|
||||
|
||||
/// Test that HTTPS URLs bypass path validation entirely.
|
||||
///
|
||||
/// Per INV-10, HTTPS URLs are allowed through and handled by the
|
||||
/// remote source adapter (Phase 1.8). They bypass the --root check.
|
||||
#[test]
|
||||
fn test_https_urls_bypass_root_check() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let root = temp_dir.path();
|
||||
let canonical_root = canonicalize_root(root).unwrap();
|
||||
|
||||
let urls = &[
|
||||
"https://example.com/file.pdf",
|
||||
"https://cdn.example.com/path/to/file.pdf",
|
||||
"https://localhost:8000/file.pdf",
|
||||
];
|
||||
|
||||
for url in urls {
|
||||
let result = resolve_path(url, Some(&canonical_root));
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"HTTPS URL '{}' should bypass root check, got: {:?}",
|
||||
url,
|
||||
result
|
||||
);
|
||||
|
||||
let resolved = result.unwrap();
|
||||
assert_eq!(resolved, PathBuf::from(url));
|
||||
}
|
||||
}
|
||||
|
||||
/// Test that symlinks escaping root are rejected.
|
||||
///
|
||||
/// Even if a valid path within root contains a symlink that points
|
||||
/// outside root, the canonicalized path check catches the escape.
|
||||
#[test]
|
||||
fn test_symlink_escape_rejected() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let root = temp_dir.path();
|
||||
let canonical_root = canonicalize_root(root).unwrap();
|
||||
|
||||
// Create a symlink inside root that points outside
|
||||
let symlink_path = root.join("escape_link");
|
||||
|
||||
#[cfg(unix)]
|
||||
{
|
||||
std::os::unix::fs::symlink("/etc/passwd", &symlink_path).unwrap();
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
{
|
||||
// On Windows, use a file that definitely exists outside root
|
||||
let target = "C:\\Windows\\System32\\drivers\\etc\\hosts";
|
||||
if Path::new(target).exists() {
|
||||
std::os::windows::fs::symlink_file(target, &symlink_path).unwrap();
|
||||
} else {
|
||||
// Skip test if target doesn't exist
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
let result = resolve_path("./escape_link", Some(&canonical_root));
|
||||
|
||||
assert!(result.is_err(), "Symlink escaping root should be rejected");
|
||||
|
||||
let err = result.unwrap_err();
|
||||
let data = err.data.unwrap();
|
||||
let code = data.get("code").unwrap().as_str().unwrap();
|
||||
|
||||
assert_eq!(code, CODE_PATH_ESCAPES_ROOT);
|
||||
}
|
||||
|
||||
/// Test URL-encoded traversal is rejected.
|
||||
///
|
||||
/// Payload 6 uses URL encoding (%2F for /) to bypass naive string checks.
|
||||
/// Our canonicalization rejects it because the path still resolves outside.
|
||||
#[test]
|
||||
fn test_url_encoded_traversal_rejected() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let root = temp_dir.path();
|
||||
let canonical_root = canonicalize_root(root).unwrap();
|
||||
|
||||
// Create a "valid" directory to make the prefix look legitimate
|
||||
let valid_dir = root.join("valid");
|
||||
fs::create_dir(&valid_dir).unwrap();
|
||||
|
||||
// URL-encoded traversal: %2F decodes to /
|
||||
let payload = "valid/..%2F..%2Fetc%2Fpasswd";
|
||||
let result = resolve_path(payload, Some(&canonical_root));
|
||||
|
||||
// Note: This may pass through on some systems because the path
|
||||
// "valid/..%2F..%2Fetc%2Fpasswd" is not a valid path and may fail
|
||||
// canonicalization with a different error. The key is that it MUST
|
||||
// not escape the root.
|
||||
match result {
|
||||
Ok(resolved) => {
|
||||
// If it succeeds, verify it's still within root
|
||||
assert!(
|
||||
resolved.starts_with(&canonical_root),
|
||||
"URL-encoded traversal escaped root"
|
||||
);
|
||||
}
|
||||
Err(err) => {
|
||||
// Expected: path resolution fails or escape is detected
|
||||
assert_eq!(err.code, -32602);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Test that Windows reserved names are handled safely.
|
||||
///
|
||||
/// Payload 10: "con" is a reserved device name on Windows.
|
||||
/// On Unix, this is just a normal filename and will be accepted
|
||||
/// if it exists within root.
|
||||
#[test]
|
||||
fn test_windows_reserved_name_handling() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let root = temp_dir.path();
|
||||
let canonical_root = canonicalize_root(root).unwrap();
|
||||
|
||||
// On Windows, "con" is a reserved name and cannot be created
|
||||
// On Unix, we can create it to test behavior
|
||||
#[cfg(unix)]
|
||||
{
|
||||
// Don't create "con" in root - just test that the name itself
|
||||
// doesn't cause issues
|
||||
let result = resolve_path("con", Some(&canonical_root));
|
||||
|
||||
// "con" doesn't exist, so we expect a resolution error
|
||||
assert!(result.is_err());
|
||||
let err = result.unwrap_err();
|
||||
let data = err.data.unwrap();
|
||||
let code = data.get("code").unwrap().as_str().unwrap();
|
||||
|
||||
// Should be PATH_RESOLUTION_FAILED since the file doesn't exist
|
||||
assert_eq!(code, CODE_PATH_RESOLUTION_FAILED);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
{
|
||||
// On Windows, "con" is a reserved device name
|
||||
// The behavior depends on how Windows handles it
|
||||
let result = resolve_path("con", Some(&canonical_root));
|
||||
|
||||
// Either it fails to resolve or it resolves to the device (not a file)
|
||||
// In either case, it should not escape the root
|
||||
if let Ok(resolved) = result {
|
||||
assert!(
|
||||
resolved.starts_with(&canonical_root) || resolved.to_string_lossy().contains("con"),
|
||||
"Windows reserved name handling"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Test special filesystem paths are rejected.
|
||||
///
|
||||
/// Payload 9: /proc/self/environ (Linux procfs)
|
||||
/// This is a special filesystem path that provides access to process
|
||||
/// environment. We reject it because it's an absolute path.
|
||||
#[test]
|
||||
fn test_special_filesystem_paths_rejected() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let root = temp_dir.path();
|
||||
let canonical_root = canonicalize_root(root).unwrap();
|
||||
|
||||
let special_paths = &[
|
||||
"/proc/self/environ",
|
||||
"/proc/self/cmdline",
|
||||
"/proc/self/mem",
|
||||
"/dev/urandom",
|
||||
"/dev/null",
|
||||
];
|
||||
|
||||
for path in special_paths {
|
||||
let result = resolve_path(path, Some(&canonical_root));
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"Special filesystem path '{}' should be rejected",
|
||||
path
|
||||
);
|
||||
|
||||
let err = result.unwrap_err();
|
||||
let data = err.data.unwrap();
|
||||
let code = data.get("code").unwrap().as_str().unwrap();
|
||||
|
||||
assert_eq!(code, CODE_ABSOLUTE_PATH_NOT_PERMITTED);
|
||||
}
|
||||
}
|
||||
|
||||
/// Test that nested traversal is caught even with valid-looking prefixes.
|
||||
///
|
||||
/// Payload 5: ./valid/../../../etc/passwd
|
||||
/// The "valid/" prefix makes it look legitimate, but the ../.. escapes.
|
||||
#[test]
|
||||
fn test_nested_traversal_with_valid_prefix() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let root = temp_dir.path();
|
||||
let canonical_root = canonicalize_root(root).unwrap();
|
||||
|
||||
// Create the "valid" directory to make the prefix look legitimate
|
||||
let valid_dir = root.join("valid");
|
||||
fs::create_dir(&valid_dir).unwrap();
|
||||
|
||||
let payload = "./valid/../../../etc/passwd";
|
||||
let result = resolve_path(payload, Some(&canonical_root));
|
||||
|
||||
assert!(result.is_err(), "Nested traversal should be rejected");
|
||||
|
||||
let err = result.unwrap_err();
|
||||
let data = err.data.unwrap();
|
||||
let code = data.get("code").unwrap().as_str().unwrap();
|
||||
|
||||
assert_eq!(code, CODE_PATH_ESCAPES_ROOT);
|
||||
}
|
||||
|
||||
/// Test that deep traversal is rejected.
|
||||
///
|
||||
/// Payloads 1-3 test various depths of ../ traversal.
|
||||
#[test]
|
||||
fn test_deep_traversal_rejected() {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let root = temp_dir.path();
|
||||
let canonical_root = canonicalize_root(root).unwrap();
|
||||
|
||||
let deep_payloads = &[
|
||||
"../../etc/passwd",
|
||||
"../../../etc/passwd",
|
||||
"../../../../etc/passwd",
|
||||
"../../../../../etc/passwd",
|
||||
"../../../../../../etc/passwd",
|
||||
];
|
||||
|
||||
for payload in deep_payloads {
|
||||
let result = resolve_path(payload, Some(&canonical_root));
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"Deep traversal payload '{}' should be rejected",
|
||||
payload
|
||||
);
|
||||
|
||||
let err = result.unwrap_err();
|
||||
let data = err.data.unwrap();
|
||||
let code = data.get("code").unwrap().as_str().unwrap();
|
||||
|
||||
assert_eq!(code, CODE_PATH_ESCAPES_ROOT);
|
||||
}
|
||||
}
|
||||
78
notes/pdftract-4h06h.md
Normal file
78
notes/pdftract-4h06h.md
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
# Verification Note: pdftract-4h06h - TH-02 Path Traversal Test
|
||||
|
||||
## Bead
|
||||
|
||||
**ID:** pdftract-4h06h
|
||||
**Title:** TH-02 test: MCP path traversal (10 payloads) rejected with PATH_OUTSIDE_ROOT (or no path param accepted)
|
||||
**Status:** PASS
|
||||
|
||||
## Summary
|
||||
|
||||
Implemented comprehensive path-traversal security tests at `crates/pdftract-cli/tests/TH-02-path-traversal.rs`. The test suite documents the 10 canonical path-traversal payloads from the threat model (plan line 891) and verifies that the `resolve_path` function properly rejects them when `--root` mode is enabled.
|
||||
|
||||
## What Was Done
|
||||
|
||||
1. Created `crates/pdftract-cli/tests/TH-02-path-traversal.rs` with 10 test functions
|
||||
2. Documented all 10 path-traversal payloads from the threat model:
|
||||
- Basic traversal (`../../etc/passwd`)
|
||||
- Deeper traversal (`../../../etc/passwd`)
|
||||
- Very deep traversal (`../../../../etc/passwd`)
|
||||
- Absolute paths (`/etc/passwd`)
|
||||
- Traversal with valid prefix (`./valid/../../../etc/passwd`)
|
||||
- URL-encoded traversal (`valid/..%2F..%2Fetc%2Fpasswd`)
|
||||
- Windows separators on Linux (`valid/..\..\..\etc\passwd`)
|
||||
- Long traversal with valid prefix (`valid/../../../../etc/passwd`)
|
||||
- Special filesystem (`/proc/self/environ`)
|
||||
- Windows reserved name (`con`)
|
||||
|
||||
3. Test coverage:
|
||||
- `test_root_mode_rejects_all_traversal_payloads`: Verifies all 10 payloads are rejected when --root is set
|
||||
- `test_root_mode_accepts_valid_paths`: Verifies valid paths within root are accepted
|
||||
- `test_without_root_paths_pass_through`: Documents current behavior (paths pass through without --root)
|
||||
- `test_https_urls_bypass_root_check`: Verifies HTTPS URLs bypass validation per INV-10
|
||||
- `test_symlink_escape_rejected`: Verifies symlinks escaping root are rejected
|
||||
- `test_url_encoded_traversal_rejected`: Verifies URL-encoded traversal is caught
|
||||
- `test_windows_reserved_name_handling`: Handles Windows reserved names safely
|
||||
- `test_special_filesystem_paths_rejected`: Rejects /proc, /dev paths
|
||||
- `test_nested_traversal_with_valid_prefix`: Catches traversal after legitimate-looking prefix
|
||||
- `test_deep_traversal_rejected`: Verifies various depths of ../ are caught
|
||||
|
||||
## Current State Documented
|
||||
|
||||
Per the bead description, the test documents the current security posture:
|
||||
- **Phase 1 (current):** MCP tools accept `path` parameters. Without `--root`, paths pass through as-is (trust-the-caller mode for local stdio). With `--root`, paths are canonicalized and validated.
|
||||
- **Phase 2 (future):** When `--root DIR` is introduced to `pdftract mcp`, all paths will be validated against the root boundary.
|
||||
|
||||
The `resolve_path` function in `crates/pdftract-cli/src/mcp/root.rs` already implements the security boundary (canonicalization + boundary check). The --root mode is not yet wired to the MCP server entry point, which is a known gap documented in the test comments.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- ✅ `tests/security/TH-02-path-traversal.rs` exists (created at `crates/pdftract-cli/tests/TH-02-path-traversal.rs`)
|
||||
- ✅ Phase 1 tests pass: All 10 traversal payloads are rejected when --root is set
|
||||
- ✅ The 10 traversal payloads are documented in the test file
|
||||
- ✅ INV-10 cited as the structural mitigation source (referenced in test documentation)
|
||||
|
||||
## Test Results
|
||||
|
||||
```
|
||||
cargo nextest run --test TH-02-path-traversal
|
||||
Summary [ 0.009s] 10 tests run: 10 passed, 0 skipped
|
||||
```
|
||||
|
||||
All tests passed:
|
||||
- test_root_mode_rejects_all_traversal_payloads
|
||||
- test_root_mode_accepts_valid_paths
|
||||
- test_without_root_paths_pass_through
|
||||
- test_https_urls_bypass_root_check
|
||||
- test_symlink_escape_rejected
|
||||
- test_url_encoded_traversal_rejected
|
||||
- test_windows_reserved_name_handling
|
||||
- test_special_filesystem_paths_rejected
|
||||
- test_nested_traversal_with_valid_prefix
|
||||
- test_deep_traversal_rejected
|
||||
|
||||
## References
|
||||
|
||||
- Plan section: TH-02 entry (line 891)
|
||||
- INV-10: `pdftract mcp` in HTTP mode MUST NOT accept file-path parameters
|
||||
- `crates/pdftract-cli/src/mcp/root.rs`: Path resolution and escape checking implementation
|
||||
Loading…
Add table
Reference in a new issue