diff --git a/Cargo.toml b/Cargo.toml index 84612ea..a7fb290 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,18 +1,18 @@ [package] name = "path_jail_py" -version = "0.2.0" +version = "0.3.0" edition = "2021" license = "MIT OR Apache-2.0" description = "Python bindings for path_jail - secure filesystem sandbox" repository = "https://github.com/tenuo-ai/path-jail-python" -homepage = "https://tenuo.dev" +homepage = "https://tenuo.ai" [lib] name = "path_jail" crate-type = ["cdylib"] [dependencies] -path_jail = "0.2" +path_jail = "0.5" pyo3 = { version = "0.24", features = ["extension-module", "abi3-py39"] } [package.metadata] -authors = ["Niki A. "] \ No newline at end of file +authors = ["Niki A. "] \ No newline at end of file diff --git a/README.md b/README.md index 4dd3c77..5b26ce6 100644 --- a/README.md +++ b/README.md @@ -29,8 +29,8 @@ path1 = jail.join("2025/report.pdf") path2 = jail.join("data.csv") # These raise ValueError: -jail.join("../../etc/passwd") # Path traversal -jail.join("/etc/passwd") # Absolute path +jail.join("../../etc/passwd") # Path traversal +jail.join("/etc/passwd") # Absolute path ``` ## Why path-jail? @@ -132,6 +132,10 @@ try: except ValueError as e: # Path escapes jail, broken symlink, or invalid path print(f"Rejected: {e}") +except OSError as e: + # An existing component couldn't be inspected (e.g. permission denied). + # Fail closed: treat it as a rejection, not as a missing file. + print(f"Cannot verify: {e}") except TypeError as e: # Invalid type (not str or PathLike) print(f"Bad input: {e}") @@ -141,9 +145,12 @@ Creating a jail can also fail: ```python try: - jail = Jail("/nonexistent") + jail = Jail("/var/uploads") except OSError as e: - # Root doesn't exist or isn't a directory + # Root doesn't exist or can't be read + print(f"Invalid root: {e}") +except ValueError as e: + # Root is a file, or a filesystem root like "/" print(f"Invalid root: {e}") ``` @@ -156,17 +163,18 @@ from path_jail import Jail UPLOAD_DIR = "/var/uploads" jail = Jail(UPLOAD_DIR) + def save_upload(user_id: str, filename: str, data: bytes) -> str: """Safely save an uploaded file.""" # Validate and build path safe_path = jail.join(f"{user_id}/{filename}") - + # Create parent directories Path(safe_path).parent.mkdir(parents=True, exist_ok=True) - + # Write file Path(safe_path).write_bytes(data) - + # Return relative path for database storage return jail.relative(safe_path) ``` @@ -176,19 +184,22 @@ def save_upload(user_id: str, filename: str, data: bytes) -> str: ### FastAPI ```python -from fastapi import FastAPI, UploadFile, HTTPException +from pathlib import Path + +from fastapi import FastAPI, HTTPException, UploadFile from path_jail import Jail app = FastAPI() uploads = Jail("/var/uploads") + @app.post("/upload/{filename:path}") async def upload(filename: str, file: UploadFile): try: safe_path = uploads.join(filename) except ValueError: raise HTTPException(400, "Invalid filename") - + Path(safe_path).parent.mkdir(parents=True, exist_ok=True) Path(safe_path).write_bytes(await file.read()) return {"path": filename} @@ -204,13 +215,14 @@ from path_jail import Jail app = Flask(__name__) uploads = Jail("/var/uploads") + @app.route("/upload/", methods=["POST"]) def upload(filename): try: safe_path = uploads.join(filename) except ValueError: abort(400, "Invalid filename") - + Path(safe_path).parent.mkdir(parents=True, exist_ok=True) request.files["file"].save(safe_path) return {"path": filename} @@ -226,12 +238,13 @@ from path_jail import Jail uploads = Jail(settings.MEDIA_ROOT) + def upload(request, filename): try: safe_path = uploads.join(filename) except ValueError: return HttpResponseBadRequest("Invalid filename") - + Path(safe_path).parent.mkdir(parents=True, exist_ok=True) with open(safe_path, "wb") as f: for chunk in request.FILES["file"].chunks(): @@ -274,12 +287,23 @@ path-jail validates paths at call time. A symlink could be created between valid ```python safe_path = jail.join("file.txt") # Validated # Attacker creates symlink here -open(safe_path) # Escapes! +open(safe_path) # Escapes! ``` **Mitigations:** -- Use `O_NOFOLLOW` when opening files +- Open with `O_NOFOLLOW` so a symlink swapped in at the final component is + refused (intermediate directories are still unprotected): + ```python + import os + + fd = os.open(safe_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, 0o644) + with os.fdopen(fd, "wb") as f: + f.write(data) + ``` - Use container/chroot isolation for strong guarantees +- The Rust crate's `guard` feature makes validate-and-open a single + kernel-checked step on Linux x86_64/aarch64 with kernel 5.6+; it is not + exposed in these bindings yet. #### Windows Reserved Device Names @@ -287,8 +311,8 @@ On Windows, filenames like `CON`, `PRN`, `AUX`, `NUL`, `COM1`-`COM9`, `LPT1`-`LP ```python # If an attacker uploads "CON.txt": -safe_path = jail.join("CON.txt") # Returns "C:\uploads\CON.txt" -open(safe_path) # Opens console device, not file! +safe_path = jail.join("CON.txt") # Returns "C:\uploads\CON.txt" +open(safe_path) # Opens console device, not file! ``` **Impact:** Denial of Service (thread hangs or data vanishes). Not a filesystem escape. @@ -323,8 +347,8 @@ jail.join("SECRET.TXT") # Not in blocklist, but same file! Windows silently strips trailing dots and spaces from filenames: ```python -jail.join("file.txt.") # Becomes "file.txt" -jail.join("file.txt ") # Becomes "file.txt" +jail.join("file.txt.") # Becomes "file.txt" +jail.join("file.txt ") # Becomes "file.txt" # Could bypass extension checks: if not filename.endswith(".exe"): @@ -374,6 +398,7 @@ When comparing paths, always canonicalize your expected values: ```python import os + assert result == os.path.realpath("/var/uploads/file.txt") ``` diff --git a/pyproject.toml b/pyproject.toml index baaf054..be5d5a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "maturin" [project] name = "path-jail" -version = "0.2.0" +version = "0.3.0" description = "Secure filesystem sandbox. Restricts paths to a root directory, preventing traversal attacks." readme = "README.md" license = { text = "MIT OR Apache-2.0" } @@ -27,7 +27,7 @@ classifiers = [ ] [project.urls] -Homepage = "https://tenuo.dev" +Homepage = "https://tenuo.ai" Repository = "https://github.com/tenuo-ai/path-jail-python" Documentation = "https://github.com/tenuo-ai/path-jail-python#readme" diff --git a/python/path_jail/__init__.py b/python/path_jail/__init__.py index c7840d2..33c200c 100644 --- a/python/path_jail/__init__.py +++ b/python/path_jail/__init__.py @@ -2,4 +2,6 @@ from .path_jail import Jail, join __all__ = ["Jail", "join"] -__version__ = "0.1.0" +from importlib.metadata import version as _version + +__version__ = _version("path-jail") diff --git a/python/path_jail/__init__.pyi b/python/path_jail/__init__.pyi index adc8753..f0af071 100644 --- a/python/path_jail/__init__.pyi +++ b/python/path_jail/__init__.pyi @@ -25,7 +25,8 @@ class Jail: root: Path to the jail root directory (must exist) Raises: - OSError: If root does not exist or is not a directory + ValueError: If root is a filesystem root or not a directory + OSError: If root does not exist or cannot be read """ ... @@ -50,6 +51,8 @@ class Jail: Raises: ValueError: If path would escape the jail or is absolute + OSError: If an existing path component cannot be inspected + (e.g. permission denied) """ ... @@ -65,6 +68,7 @@ class Jail: Raises: ValueError: If path is outside the jail or not absolute + OSError: If the path cannot be inspected """ ... @@ -79,6 +83,7 @@ class Jail: Raises: ValueError: If path is outside the jail + OSError: If the path cannot be inspected """ ... @@ -100,8 +105,8 @@ def join(root: _PathLike, path: _PathLike) -> str: prefix for paths exceeding 250 characters. Raises: - ValueError: If path would escape the jail - OSError: If root does not exist + ValueError: If path would escape the jail, or root is not a directory + OSError: If root does not exist, or a path component cannot be inspected Note: On Windows, returns standard paths (e.g. ``C:\\...``) when possible. diff --git a/src/lib.rs b/src/lib.rs index f7c7340..d5af38f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,7 @@ #![allow(clippy::useless_conversion)] use ::path_jail::{Jail as RustJail, JailError}; -use pyo3::exceptions::{PyIOError, PyTypeError, PyValueError}; +use pyo3::exceptions::{PyOSError, PyTypeError, PyValueError}; use pyo3::prelude::*; use pyo3::types::PyString; use std::path::PathBuf; @@ -89,6 +89,18 @@ fn extract_path(obj: &Bound<'_, PyAny>) -> PyResult { Err(PyTypeError::new_err("expected str or os.PathLike object")) } +/// Convert an OS error without discarding its numeric error code. +fn io_error_to_py(err: std::io::Error) -> PyErr { + let message = err.to_string(); + match err.raw_os_error() { + #[cfg(windows)] + Some(code) => PyOSError::new_err((0, message, Option::::None, code)), + #[cfg(not(windows))] + Some(code) => PyOSError::new_err((code, message)), + None => err.into(), + } +} + /// Convert JailError to Python exception fn to_py_err(err: JailError) -> PyErr { match err { @@ -104,7 +116,9 @@ fn to_py_err(err: JailError) -> PyErr { JailError::InvalidPath(reason) => { PyValueError::new_err(format!("invalid path: {}", reason)) } - JailError::Io(err) => PyIOError::new_err(err.to_string()), + // Filesystem root or non-directory. (A missing root surfaces as Io.) + err @ JailError::InvalidRoot { .. } => PyValueError::new_err(err.to_string()), + JailError::Io(err) => io_error_to_py(err), // Handle future error variants from path_jail crate _ => PyValueError::new_err(format!("path_jail error: {}", err)), } @@ -129,7 +143,8 @@ impl Jail { /// root: Path to the jail root directory (must exist) /// /// Raises: - /// IOError: If root does not exist or is not a directory + /// ValueError: If root is a filesystem root or not a directory + /// OSError: If root does not exist or cannot be read #[new] fn new(root: &Bound<'_, PyAny>) -> PyResult { let path = extract_path(root)?; @@ -153,6 +168,8 @@ impl Jail { /// /// Raises: /// ValueError: If path would escape the jail or is absolute + /// OSError: If an existing path component cannot be inspected + /// (e.g. permission denied) fn join(&self, path: &Bound<'_, PyAny>) -> PyResult { let path = extract_path(path)?; self.inner @@ -172,6 +189,7 @@ impl Jail { /// /// Raises: /// ValueError: If path is outside the jail or not absolute + /// OSError: If the path cannot be inspected fn contains(&self, path: &Bound<'_, PyAny>) -> PyResult { let path = extract_path(path)?; self.inner @@ -191,6 +209,7 @@ impl Jail { /// /// Raises: /// ValueError: If path is outside the jail + /// OSError: If the path cannot be inspected fn relative(&self, path: &Bound<'_, PyAny>) -> PyResult { let path = extract_path(path)?; self.inner @@ -227,8 +246,8 @@ impl Jail { /// Absolute path inside the jail /// /// Raises: -/// ValueError: If path would escape the jail -/// IOError: If root does not exist +/// ValueError: If path would escape the jail, or root is not a directory +/// OSError: If root does not exist, or a path component cannot be inspected /// /// Example: /// >>> from path_jail import join diff --git a/tests/test_jail.py b/tests/test_jail.py index 2b2e565..e5a954f 100644 --- a/tests/test_jail.py +++ b/tests/test_jail.py @@ -46,8 +46,9 @@ def test_create_jail_pathlike(self, jail_dir): assert paths_equal(jail.root, os.path.realpath(jail_dir)) def test_create_jail_nonexistent(self): - with pytest.raises(OSError): + with pytest.raises(FileNotFoundError) as exc_info: Jail("/nonexistent/path") + assert exc_info.value.errno is not None def test_join_simple(self, jail_dir): jail = Jail(jail_dir) @@ -99,6 +100,12 @@ def test_contains_outside(self, jail_dir): with pytest.raises(ValueError, match="escapes"): jail.contains(outside_path) + def test_contains_missing_path_is_os_error(self, jail_dir): + jail = Jail(jail_dir) + missing = os.path.join(jail_dir, "missing.txt") + with pytest.raises(FileNotFoundError): + jail.contains(missing) + def test_relative(self, jail_dir): jail = Jail(jail_dir) test_file = os.path.join(jail_dir, "subdir", "file.txt") @@ -109,6 +116,12 @@ def test_relative(self, jail_dir): # On Windows, path separator might differ assert normalize_path(result) == os.path.join("subdir", "file.txt") + def test_relative_missing_path_is_os_error(self, jail_dir): + jail = Jail(jail_dir) + missing = os.path.join(jail_dir, "missing.txt") + with pytest.raises(FileNotFoundError): + jail.relative(missing) + def test_repr(self, jail_dir): jail = Jail(jail_dir) assert "Jail(" in repr(jail) @@ -323,7 +336,7 @@ def test_special_chars(self, jail_dir): result = jail.join(special) # If it works, verify it's inside the jail assert normalize_path(result).startswith(normalize_path(jail.root)) - except ValueError: + except (ValueError, OSError): pass # Also acceptable to reject def test_backslash_on_unix(self, jail_dir): @@ -385,3 +398,38 @@ def test_trailing_slash(self, jail_dir): jail = Jail(jail_dir) result = jail.join("subdir/") assert normalize_path(result).startswith(normalize_path(jail.root)) + + +class TestCoreBehavior: + """Behavior that comes from the path_jail 0.5 core.""" + + def test_non_directory_root_is_value_error(self, jail_dir): + file_root = os.path.join(jail_dir, "file.txt") + with open(file_root, "w"): + pass + with pytest.raises(ValueError, match="not a directory"): + Jail(file_root) + + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX permissions") + @pytest.mark.skipif( + hasattr(os, "geteuid") and os.geteuid() == 0, reason="root bypasses permissions" + ) + def test_unreadable_component_fails_closed(self, jail_dir): + locked = os.path.join(jail_dir, "locked") + os.mkdir(locked) + os.chmod(locked, 0) + try: + jail = Jail(jail_dir) + # 0.2 treated the unreadable component as nonexistent and returned + # a path; 0.5 reports the I/O error instead. + with pytest.raises(PermissionError): + jail.join("locked/file.txt") + finally: + os.chmod(locked, 0o700) + + def test_version_matches_metadata(self): + from importlib.metadata import version + + import path_jail + + assert path_jail.__version__ == version("path-jail")