diff --git a/Cargo.lock b/Cargo.lock index e4dc6ac5..508c2711 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3709,6 +3709,7 @@ dependencies = [ "sha2", "snafu", "tar", + "tempfile", "testutils", "tmpdir", "tokio", diff --git a/crates/tower-cmd/src/output.rs b/crates/tower-cmd/src/output.rs index 1fa99fc4..d2bb5e38 100644 --- a/crates/tower-cmd/src/output.rs +++ b/crates/tower-cmd/src/output.rs @@ -177,6 +177,9 @@ pub fn package_error(err: tower_package::Error) { format!("Missing required app field `{}` in Towerfile", field) } tower_package::Error::Io { source } => format!("IO error: {}", source), + tower_package::Error::MissingScript { script } => { + format!("Script '{}' not found. Check that the 'script' field in your Towerfile points to a file that exists in your project.", script) + } }; let line = format!("{} {}\n", "Package error:".red(), msg); diff --git a/crates/tower-package/Cargo.toml b/crates/tower-package/Cargo.toml index 709358be..ce0ddb3f 100644 --- a/crates/tower-package/Cargo.toml +++ b/crates/tower-package/Cargo.toml @@ -48,3 +48,4 @@ wasm-bindgen = { version = "0.2", optional = true } [dev-dependencies] testutils = { workspace = true } +tempfile = { workspace = true } diff --git a/crates/tower-package/src/error.rs b/crates/tower-package/src/error.rs index e57afe64..1f53bd26 100644 --- a/crates/tower-package/src/error.rs +++ b/crates/tower-package/src/error.rs @@ -24,6 +24,9 @@ pub enum Error { #[snafu(display("Missing required app field `{field}` in Towerfile"))] MissingRequiredAppField { field: String }, + #[snafu(display("Script '{script}' not found. Check that the 'script' field in your Towerfile points to a file that exists in your project."))] + MissingScript { script: String }, + #[snafu(display("IO error: {source}"))] Io { source: std::io::Error }, } diff --git a/crates/tower-package/src/native.rs b/crates/tower-package/src/native.rs index 0db79b38..6227db0b 100644 --- a/crates/tower-package/src/native.rs +++ b/crates/tower-package/src/native.rs @@ -100,6 +100,8 @@ impl Package { file.read_to_string(&mut contents).await?; let manifest = Manifest::from_json(&contents)?; + Self::validate_invoke_script(&manifest, &path)?; + Ok(Self { tmp_dir: None, package_file_path: None, @@ -108,6 +110,41 @@ impl Package { }) } + /// Validates that the manifest's invoke script path is safe and exists on disk. + /// Returns an error if the path contains traversal sequences, is absolute, or + /// the target file does not exist. + fn validate_invoke_script(manifest: &Manifest, package_path: &Path) -> Result<(), Error> { + if manifest.invoke.is_empty() { + return Ok(()); + } + + if manifest.invoke.contains("..") + || Path::new(&manifest.invoke).is_absolute() + || manifest.invoke.starts_with('/') + { + return Err(Error::InvalidTowerfile { + message: format!( + "Invalid script path '{}': must be a relative path within the package", + manifest.invoke + ), + }); + } + + let working_dir = if manifest.version == Some(1) { + package_path.to_path_buf() + } else { + package_path.join(&manifest.app_dir_name) + }; + let invoke_path = working_dir.join(&manifest.invoke); + if !invoke_path.exists() { + return Err(Error::MissingScript { + script: manifest.invoke.clone(), + }); + } + + Ok(()) + } + // build creates a new package from a PackageSpec. PackageSpec is typically composed of fields // copied from the Towerfile. The most important thing to know is that the collection of file // globs to include in the package. @@ -464,6 +501,7 @@ pub async fn compute_sha256_file(file_path: &PathBuf) -> Result { mod test { use super::*; use std::path::PathBuf; + use tempfile::TempDir; #[test] fn test_should_ignore_pyc_files() { @@ -478,4 +516,67 @@ mod test { // A .py file should not be ignored assert!(!resolver.should_ignore(&PathBuf::from("/project/module.py"))); } + + fn make_manifest(invoke: &str, version: Option) -> Manifest { + Manifest { + version, + invoke: invoke.to_string(), + parameters: vec![], + schedule: None, + import_paths: vec![], + app_dir_name: "app".to_string(), + modules_dir_name: "modules".to_string(), + checksum: String::new(), + } + } + + #[test] + fn test_validate_invoke_script_rejects_path_traversal() { + let dir = TempDir::new().unwrap(); + let manifest = make_manifest("../../etc/passwd", None); + let err = Package::validate_invoke_script(&manifest, dir.path()).unwrap_err(); + assert!(matches!(err, Error::InvalidTowerfile { .. })); + } + + #[test] + fn test_validate_invoke_script_rejects_absolute_path() { + let dir = TempDir::new().unwrap(); + let manifest = make_manifest("/usr/bin/python3", None); + let err = Package::validate_invoke_script(&manifest, dir.path()).unwrap_err(); + assert!(matches!(err, Error::InvalidTowerfile { .. })); + } + + #[test] + fn test_validate_invoke_script_rejects_missing_script() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir_all(dir.path().join("app")).unwrap(); + let manifest = make_manifest("./nonexistent.py", None); + let err = Package::validate_invoke_script(&manifest, dir.path()).unwrap_err(); + assert!(matches!(err, Error::MissingScript { .. })); + } + + #[test] + fn test_validate_invoke_script_accepts_valid_script() { + let dir = TempDir::new().unwrap(); + let app_dir = dir.path().join("app"); + std::fs::create_dir_all(&app_dir).unwrap(); + std::fs::write(app_dir.join("main.py"), "print('hello')").unwrap(); + let manifest = make_manifest("./main.py", None); + assert!(Package::validate_invoke_script(&manifest, dir.path()).is_ok()); + } + + #[test] + fn test_validate_invoke_script_accepts_empty_invoke() { + let dir = TempDir::new().unwrap(); + let manifest = make_manifest("", None); + assert!(Package::validate_invoke_script(&manifest, dir.path()).is_ok()); + } + + #[test] + fn test_validate_invoke_script_v1_uses_root_as_working_dir() { + let dir = TempDir::new().unwrap(); + std::fs::write(dir.path().join("task.py"), "print('hello')").unwrap(); + let manifest = make_manifest("./task.py", Some(1)); + assert!(Package::validate_invoke_script(&manifest, dir.path()).is_ok()); + } }