diff --git a/Cargo.lock b/Cargo.lock index d5dafe8bafa..7308eca487e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2485,7 +2485,7 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tmc-langs" -version = "0.39.2" +version = "0.39.3" dependencies = [ "base64 0.22.1", "blake3", @@ -2528,7 +2528,7 @@ dependencies = [ [[package]] name = "tmc-langs-cli" -version = "0.39.2" +version = "0.39.3" dependencies = [ "anyhow", "base64 0.22.1", @@ -2555,7 +2555,7 @@ dependencies = [ [[package]] name = "tmc-langs-csharp" -version = "0.39.2" +version = "0.39.3" dependencies = [ "dirs", "log", @@ -2573,7 +2573,7 @@ dependencies = [ [[package]] name = "tmc-langs-framework" -version = "0.39.2" +version = "0.39.3" dependencies = [ "blake3", "fd-lock", @@ -2600,7 +2600,7 @@ dependencies = [ [[package]] name = "tmc-langs-java" -version = "0.39.2" +version = "0.39.3" dependencies = [ "dirs", "flate2", @@ -2621,7 +2621,7 @@ dependencies = [ [[package]] name = "tmc-langs-make" -version = "0.39.2" +version = "0.39.3" dependencies = [ "log", "once_cell", @@ -2640,7 +2640,7 @@ dependencies = [ [[package]] name = "tmc-langs-node" -version = "0.39.2" +version = "0.39.3" dependencies = [ "base64 0.22.1", "env_logger", @@ -2658,7 +2658,7 @@ dependencies = [ [[package]] name = "tmc-langs-notests" -version = "0.39.2" +version = "0.39.3" dependencies = [ "log", "simple_logger", @@ -2670,7 +2670,7 @@ dependencies = [ [[package]] name = "tmc-langs-plugins" -version = "0.39.2" +version = "0.39.3" dependencies = [ "blake3", "log", @@ -2693,7 +2693,7 @@ dependencies = [ [[package]] name = "tmc-langs-python3" -version = "0.39.2" +version = "0.39.3" dependencies = [ "dunce", "hex", @@ -2715,7 +2715,7 @@ dependencies = [ [[package]] name = "tmc-langs-r" -version = "0.39.2" +version = "0.39.3" dependencies = [ "log", "serde", @@ -2731,7 +2731,7 @@ dependencies = [ [[package]] name = "tmc-langs-util" -version = "0.39.2" +version = "0.39.3" dependencies = [ "dunce", "fd-lock", @@ -2756,7 +2756,7 @@ dependencies = [ [[package]] name = "tmc-mooc-client" -version = "0.39.2" +version = "0.39.3" dependencies = [ "bytes", "chrono", @@ -2779,7 +2779,7 @@ dependencies = [ [[package]] name = "tmc-server-mock" -version = "0.39.2" +version = "0.39.3" dependencies = [ "mockito", "serde_json", @@ -2787,7 +2787,7 @@ dependencies = [ [[package]] name = "tmc-testmycode-client" -version = "0.39.2" +version = "0.39.3" dependencies = [ "chrono", "dirs", diff --git a/Cargo.toml b/Cargo.toml index 8db5f38b5a9..ece08bcb596 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,7 @@ authors = [ edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.85.0" -version = "0.39.2" +version = "0.39.3" [workspace.dependencies] mooc-langs-api = { git = "https://github.com/rage/secret-project-331.git", rev = "24179d597e5f4120649be50b903a9a4e544ea77c" } diff --git a/crates/tmc-langs-cli/src/app.rs b/crates/tmc-langs-cli/src/app.rs index 3f8e6e425ae..a589980c95f 100644 --- a/crates/tmc-langs-cli/src/app.rs +++ b/crates/tmc-langs-cli/src/app.rs @@ -182,8 +182,7 @@ pub enum Command { #[clap(long)] output_path: PathBuf, /// If given, the tests will be copied from this stub instead, effectively ignoring hidden tests. - // alias for backwards compatibility - #[clap(long, alias = "stub-zip-path")] + #[clap(long)] stub_archive_path: Option, /// Compression algorithm used for the stub archive. #[clap(long, default_value_t = Compression::Zip)] @@ -836,7 +835,7 @@ mod base_test { "tar", "--output-path", "path", - "--stub-zip-path", + "--stub-archive-path", "path", "--submission-path", "path", diff --git a/crates/tmc-langs-framework/src/lib.rs b/crates/tmc-langs-framework/src/lib.rs index c74c287c9d7..f6f910aa0ce 100644 --- a/crates/tmc-langs-framework/src/lib.rs +++ b/crates/tmc-langs-framework/src/lib.rs @@ -11,6 +11,9 @@ mod plugin; mod policy; mod tmc_project_yml; +#[cfg(test)] +mod test_helpers; + pub use self::{ archive::{Archive, ArchiveBuilder, Compression}, command::{ExitStatus, Output, TmcCommand}, diff --git a/crates/tmc-langs-framework/src/plugin.rs b/crates/tmc-langs-framework/src/plugin.rs index d2edc4bddff..a3f32c62caa 100644 --- a/crates/tmc-langs-framework/src/plugin.rs +++ b/crates/tmc-langs-framework/src/plugin.rs @@ -14,6 +14,7 @@ use nom::{IResult, Parser, branch, bytes, character, combinator, multi, sequence use nom_language::error::VerboseError; use std::{ collections::HashSet, + ffi::OsStr, io::{Read, Seek}, ops::ControlFlow::{Break, Continue}, path::{Path, PathBuf}, @@ -217,6 +218,7 @@ pub trait LanguagePlugin { /// Extracts student files from the compressed project. /// It finds the project dir from the zip and extracts the student files from there. /// Overwrites all files. + /// Important: does not extract .tmcproject.yml from the students' submission as they control that file and they could use it to modify the test files. fn extract_student_files( compressed_project: impl Read + Seek, compression: Compression, @@ -227,18 +229,9 @@ pub trait LanguagePlugin { let mut archive = Archive::new(compressed_project, compression)?; // find the exercise root directory inside the archive - let project_dir = Self::find_project_dir_in_archive(&mut archive)?; + let project_dir = Self::safe_find_project_dir_in_archive(&mut archive); log::debug!("Project directory in archive: {}", project_dir.display()); - // extract config file if any - let tmc_project_yml_path = project_dir.join(".tmcproject.yml"); - let tmc_project_yml_path = tmc_project_yml_path - .to_str() - .ok_or_else(|| TmcError::ProjectDirInvalidUtf8(project_dir.clone()))?; - if let Ok(mut file) = archive.by_path(tmc_project_yml_path) { - let target_path = target_location.join(".tmcproject.yml"); - file_util::read_to_file(&mut file, target_path)?; - } let policy = Self::StudentFilePolicy::new(target_location)?; let mut iter = archive.iter()?; @@ -289,6 +282,92 @@ pub trait LanguagePlugin { archive: &mut Archive, ) -> Result; + /// A safer variant of `find_project_dir_in_archive` used by default extraction helpers. + /// + /// Fallback order: + /// 1) Delegate to `find_project_dir_in_archive` implemented by the language plugin + /// 2) First directory containing a `.tmcproject.yml` + /// 3) If archive root has only one folder, use that folder + /// 4) Archive root (empty path) + fn safe_find_project_dir_in_archive(archive: &mut Archive) -> PathBuf { + // 1) Try plugin-specific project dir detection first + if let Ok(dir) = Self::find_project_dir_in_archive(archive) { + return dir; + } + + // 2) Try to find the first directory that contains a .tmcproject.yml + if let Ok(mut iter) = archive.iter() { + loop { + let next = iter.with_next(|file| { + let file_path = file.path()?; + if file.is_file() + && file_path + .file_name() + .map(|name| name == OsStr::new(".tmcproject.yml")) + .unwrap_or(false) + { + let parent = file_path + .parent() + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from("")); + return Ok(Break(Some(parent))); + } + Ok(Continue(())) + }); + match next { + Ok(Continue(_)) => continue, + Ok(Break(Some(dir))) => return dir, + Ok(Break(None)) => break, + Err(_) => break, + } + } + } + + // 3) Check if archive root has only one folder. This is the format tmc-langs-cli sends submissions, so all official clients should use this format. + if let Ok(mut iter) = archive.iter() { + let mut folders = Vec::new(); + let mut root_file_count: usize = 0; + loop { + let next = iter.with_next::<(), _>(|file| { + let file_path = file.path()?; + // Normalize the path to handle Windows backslashes correctly + let normalized_path = file_path.to_string_lossy().replace('\\', "/"); + let normalized_path = Path::new(&normalized_path); + // Only consider entries at the archive root + if normalized_path.components().count() == 1 { + if file.is_dir() { + folders.push(file_path.to_path_buf()); + } else if file.is_file() { + root_file_count += 1; + } + } + Ok(Continue(())) + }); + match next { + Ok(Continue(_)) => continue, + Ok(Break(_)) => break, + Err(_) => break, + } + } + + // If there's exactly one folder at the root and no files, skip over it + // Special case: don't skip over certain folders + let excluded_folders = ["src"]; + if folders.len() == 1 + && root_file_count == 0 + && !folders[0] + .file_name() + .map(|name| excluded_folders.contains(&name.to_string_lossy().as_ref())) + .unwrap_or(false) + { + return folders[0].clone(); + } + } + + // 4) Default to archive root + PathBuf::from("") + } + /// Tells if there's a valid exercise in this archive. /// Unlike `is_exercise_type_correct`, searches the entire archive. fn is_archive_type_correct(archive: &mut Archive) -> bool { @@ -450,10 +529,8 @@ enum Parse { #[allow(clippy::unwrap_used)] mod test { use super::*; - use crate::TmcProjectYml; - use nom::character; + use crate::test_helpers::{MockPlugin, SimpleMockPlugin}; use std::io::Write; - use tmc_langs_util::path_util; use zip::{ZipWriter, write::SimpleFileOptions}; fn init() { @@ -513,131 +590,6 @@ mod test { target } - struct MockPlugin {} - - struct MockPolicy { - project_config: TmcProjectYml, - } - - impl StudentFilePolicy for MockPolicy { - fn new_with_project_config(project_config: TmcProjectYml) -> Self - where - Self: Sized, - { - Self { project_config } - } - fn get_project_config(&self) -> &TmcProjectYml { - &self.project_config - } - fn is_non_extra_student_file(&self, path: &Path) -> bool { - path.starts_with("src") - } - } - - impl LanguagePlugin for MockPlugin { - const PLUGIN_NAME: &'static str = "mock_plugin"; - const DEFAULT_SANDBOX_IMAGE: &'static str = "mock_image"; - const LINE_COMMENT: &'static str = "//"; - const BLOCK_COMMENT: Option<(&'static str, &'static str)> = Some(("/*", "*/")); - type StudentFilePolicy = MockPolicy; - - fn scan_exercise( - &self, - _path: &Path, - _exercise_name: String, - ) -> Result { - unimplemented!() - } - - fn run_tests_with_timeout( - &self, - _path: &Path, - _timeout: Option, - ) -> Result { - Ok(RunResult { - status: RunStatus::Passed, - test_results: vec![], - logs: std::collections::HashMap::new(), - }) - } - - fn find_project_dir_in_archive( - archive: &mut Archive, - ) -> Result { - let mut iter = archive.iter()?; - let project_dir = loop { - let next = iter.with_next(|file| { - let file_path = file.path()?; - - if let Some(parent) = - path_util::get_parent_of_component_in_path(&file_path, "src") - { - return Ok(Break(Some(parent))); - } - Ok(Continue(())) - }); - match next? { - Continue(_) => continue, - Break(project_dir) => break project_dir, - } - }; - - match project_dir { - Some(project_dir) => Ok(project_dir), - None => Err(TmcError::NoProjectDirInArchive), - } - } - - fn is_exercise_type_correct(_path: &Path) -> bool { - unimplemented!() - } - - fn clean(&self, _path: &Path) -> Result<(), TmcError> { - Ok(()) - } - - fn points_parser(i: &str) -> IResult<&str, Vec<&str>, VerboseError<&str>> { - combinator::map( - sequence::delimited( - ( - bytes::complete::tag("@"), - character::complete::multispace0, - bytes::complete::tag_no_case("points"), - character::complete::multispace0, - character::complete::char('('), - character::complete::multispace0, - ), - branch::alt(( - sequence::delimited( - character::complete::char('"'), - bytes::complete::is_not("\""), - character::complete::char('"'), - ), - sequence::delimited( - character::complete::char('\''), - bytes::complete::is_not("'"), - character::complete::char('\''), - ), - )), - ( - character::complete::multispace0, - character::complete::char(')'), - ), - ), - |s: &str| vec![s.trim()], - ) - .parse(i) - } - - fn get_default_student_file_paths() -> Vec { - vec![PathBuf::from("src")] - } - - fn get_default_exercise_file_paths() -> Vec { - vec![PathBuf::from("test")] - } - } - #[test] fn gets_exercise_packaging_configuration() { init(); @@ -1111,4 +1063,166 @@ force_update: MockPlugin::extract_student_files(buf, Compression::Zip, temp.path()).unwrap(); assert!(temp.path().join("src/file").exists()); } + + #[test] + fn extract_student_files_does_not_extract_tmcproject_yml() { + init(); + + let temp = tempfile::tempdir().unwrap(); + // create a project with a .tmcproject.yml in the project root and a student file under src + file_to(&temp, "dir/src/student_file", ""); + file_to(&temp, "dir/.tmcproject.yml", "some: yaml"); + let zip = dir_to_zip(&temp); + + // extract student files + MockPlugin::extract_student_files( + std::io::Cursor::new(zip), + Compression::Zip, + &temp.path().join("extracted"), + ) + .unwrap(); + + // ensure student files are extracted + assert!(temp.path().join("extracted/src/student_file").exists()); + // ensure .tmcproject.yml is NOT extracted + assert!(!temp.path().join("extracted/.tmcproject.yml").exists()); + } + + #[test] + fn safe_find_project_dir_fallback_to_tmcproject_yml() { + init(); + + let temp = tempfile::tempdir().unwrap(); + // create an archive without src directory (which would normally fail) + // but with a .tmcproject.yml in a subdirectory + file_to(&temp, "some/deep/path/.tmcproject.yml", "some: yaml"); + file_to(&temp, "some/deep/path/src/student_file", "content"); + let zip = dir_to_zip(&temp); + + // extract student files - should use fallback to .tmcproject.yml parent + MockPlugin::extract_student_files( + std::io::Cursor::new(zip), + Compression::Zip, + &temp.path().join("extracted"), + ) + .unwrap(); + + // ensure student files are extracted from the fallback directory + assert!(temp.path().join("extracted/src/student_file").exists()); + let content = + std::fs::read_to_string(temp.path().join("extracted/src/student_file")).unwrap(); + assert_eq!(content, "content"); + } + + #[test] + /** Matches the format tmc-langs-cli sends submissions. This makes sure submissions created by official clients are supported. */ + fn safe_find_project_dir_fallback_to_single_folder() { + init(); + + let temp = tempfile::tempdir().unwrap(); + // create an archive with only one folder at root level + file_to(&temp, "project_folder/src/student_file", "content"); + let zip = dir_to_zip(&temp); + + // extract student files - should use fallback to the single folder + MockPlugin::extract_student_files( + std::io::Cursor::new(zip), + Compression::Zip, + &temp.path().join("extracted"), + ) + .unwrap(); + + // ensure student files are extracted from the single folder + assert!(temp.path().join("extracted/src/student_file").exists()); + let content = + std::fs::read_to_string(temp.path().join("extracted/src/student_file")).unwrap(); + assert_eq!(content, "content"); + } + + #[test] + fn safe_find_project_dir_fallback_to_root() { + init(); + + let temp = tempfile::tempdir().unwrap(); + // create an archive without src directory and without .tmcproject.yml + // should fallback to root + file_to(&temp, "src/student_file", "content"); + let zip = dir_to_zip(&temp); + + // extract student files - should use fallback to root + MockPlugin::extract_student_files( + std::io::Cursor::new(zip), + Compression::Zip, + &temp.path().join("extracted"), + ) + .unwrap(); + + // ensure student files are extracted from the root + assert!(temp.path().join("extracted/src/student_file").exists()); + let content = + std::fs::read_to_string(temp.path().join("extracted/src/student_file")).unwrap(); + assert_eq!(content, "content"); + } + + #[test] + #[cfg(not(target_os = "windows"))] + fn safe_find_project_dir_single_folder_not_used_when_root_has_files() { + init(); + + let temp = tempfile::tempdir().unwrap(); + // root has a single folder and also a file at root + // SimpleMockPlugin will never find project directory, so fallback logic will be used + file_to(&temp, "project_folder/src/student_file", "content"); + file_to(&temp, "root_file.txt", "x"); + let zip = dir_to_zip(&temp); + + // extract student files - should NOT use the single-folder fallback because there's a root file + // so it should fallback to root, which extracts project_folder/src/student_file + SimpleMockPlugin::extract_student_files( + std::io::Cursor::new(zip), + Compression::Zip, + &temp.path().join("extracted"), + ) + .unwrap(); + + // The file should be extracted as project_folder/src/student_file (preserving full path from root) + // This test verifies that the single folder fallback is not used when there's a root file + assert!( + temp.path() + .join("extracted/project_folder/src/student_file") + .exists() + ); + let content = std::fs::read_to_string( + temp.path() + .join("extracted/project_folder/src/student_file"), + ) + .unwrap(); + assert_eq!(content, "content"); + } + + #[test] + fn safe_find_project_dir_does_not_skip_over_src_folder() { + init(); + + let temp = tempfile::tempdir().unwrap(); + // root has only a single "src" folder, no root files + // SimpleMockPlugin will never find project directory, so fallback logic will be used + file_to(&temp, "src/student_file", "content"); + let zip = dir_to_zip(&temp); + + // extract student files - should use the "src" folder as project root + SimpleMockPlugin::extract_student_files( + std::io::Cursor::new(zip), + Compression::Zip, + &temp.path().join("extracted"), + ) + .unwrap(); + + // The file should be extracted as student_file (using the src folder as project root) + // This test verifies that the "src" folder is used as the project directory + assert!(temp.path().join("extracted/src/student_file").exists()); + let content = + std::fs::read_to_string(temp.path().join("extracted/src/student_file")).unwrap(); + assert_eq!(content, "content"); + } } diff --git a/crates/tmc-langs-framework/src/test_helpers.rs b/crates/tmc-langs-framework/src/test_helpers.rs new file mode 100644 index 00000000000..02ae90546cd --- /dev/null +++ b/crates/tmc-langs-framework/src/test_helpers.rs @@ -0,0 +1,248 @@ +//! Test helpers for plugin tests + +use crate::plugin::LanguagePlugin; +use crate::{ + Archive, TmcProjectYml, + domain::{ExerciseDesc, RunResult, RunStatus}, + error::TmcError, + policy::StudentFilePolicy, +}; +use nom::{IResult, Parser, branch, bytes, character, combinator, sequence}; +use nom_language::error::VerboseError; +use std::{ + io::{Read, Seek}, + ops::ControlFlow::{Break, Continue}, + path::{Path, PathBuf}, + time::Duration, +}; + +pub struct MockPolicy { + project_config: TmcProjectYml, +} + +impl StudentFilePolicy for MockPolicy { + fn new_with_project_config(project_config: TmcProjectYml) -> Self + where + Self: Sized, + { + Self { project_config } + } + fn get_project_config(&self) -> &TmcProjectYml { + &self.project_config + } + fn is_non_extra_student_file(&self, path: &Path) -> bool { + path.starts_with("src") + } +} + +pub struct SimpleMockPolicy { + project_config: TmcProjectYml, +} + +impl StudentFilePolicy for SimpleMockPolicy { + fn new_with_project_config(project_config: TmcProjectYml) -> Self + where + Self: Sized, + { + Self { project_config } + } + fn get_project_config(&self) -> &TmcProjectYml { + &self.project_config + } + fn is_non_extra_student_file(&self, path: &Path) -> bool { + // Consider files under "src" as student files, even if they're in subdirectories + path.to_string_lossy().contains("/src/") || path.starts_with("src") + } +} + +pub struct MockPlugin {} + +impl LanguagePlugin for MockPlugin { + const PLUGIN_NAME: &'static str = "mock_plugin"; + const DEFAULT_SANDBOX_IMAGE: &'static str = "mock_image"; + const LINE_COMMENT: &'static str = "//"; + const BLOCK_COMMENT: Option<(&'static str, &'static str)> = Some(("/*", "*/")); + type StudentFilePolicy = MockPolicy; + + fn scan_exercise( + &self, + _path: &Path, + _exercise_name: String, + ) -> Result { + unimplemented!() + } + + fn run_tests_with_timeout( + &self, + _path: &Path, + _timeout: Option, + ) -> Result { + Ok(RunResult { + status: RunStatus::Passed, + test_results: vec![], + logs: std::collections::HashMap::new(), + }) + } + + fn find_project_dir_in_archive( + archive: &mut Archive, + ) -> Result { + let mut iter = archive.iter()?; + let project_dir = loop { + let next = iter.with_next(|file| { + let file_path = file.path()?; + + if let Some(parent) = + tmc_langs_util::path_util::get_parent_of_component_in_path(&file_path, "src") + { + return Ok(Break(Some(parent))); + } + Ok(Continue(())) + }); + match next? { + Continue(_) => continue, + Break(project_dir) => break project_dir, + } + }; + + match project_dir { + Some(project_dir) => Ok(project_dir), + None => Err(TmcError::NoProjectDirInArchive), + } + } + + fn is_exercise_type_correct(_path: &Path) -> bool { + unimplemented!() + } + + fn clean(&self, _path: &Path) -> Result<(), TmcError> { + Ok(()) + } + + fn points_parser(i: &str) -> IResult<&str, Vec<&str>, VerboseError<&str>> { + combinator::map( + sequence::delimited( + ( + bytes::complete::tag("@"), + character::complete::multispace0, + bytes::complete::tag_no_case("points"), + character::complete::multispace0, + character::complete::char('('), + character::complete::multispace0, + ), + branch::alt(( + sequence::delimited( + character::complete::char('"'), + bytes::complete::is_not("\""), + character::complete::char('"'), + ), + sequence::delimited( + character::complete::char('\''), + bytes::complete::is_not("'"), + character::complete::char('\''), + ), + )), + ( + character::complete::multispace0, + character::complete::char(')'), + ), + ), + |s: &str| vec![s.trim()], + ) + .parse(i) + } + + fn get_default_student_file_paths() -> Vec { + vec![PathBuf::from("src")] + } + + fn get_default_exercise_file_paths() -> Vec { + vec![PathBuf::from("test")] + } +} + +pub struct SimpleMockPlugin {} + +impl LanguagePlugin for SimpleMockPlugin { + const PLUGIN_NAME: &'static str = "simple_mock_plugin"; + const DEFAULT_SANDBOX_IMAGE: &'static str = "mock_image"; + const LINE_COMMENT: &'static str = "//"; + const BLOCK_COMMENT: Option<(&'static str, &'static str)> = Some(("/*", "*/")); + type StudentFilePolicy = SimpleMockPolicy; + + fn scan_exercise( + &self, + _path: &Path, + _exercise_name: String, + ) -> Result { + unimplemented!() + } + + fn run_tests_with_timeout( + &self, + _path: &Path, + _timeout: Option, + ) -> Result { + Ok(RunResult { + status: RunStatus::Passed, + test_results: vec![], + logs: std::collections::HashMap::new(), + }) + } + + fn find_project_dir_in_archive( + _archive: &mut Archive, + ) -> Result { + // Always fail to find project directory to test fallback logic + Err(TmcError::NoProjectDirInArchive) + } + + fn is_exercise_type_correct(_path: &Path) -> bool { + unimplemented!() + } + + fn clean(&self, _path: &Path) -> Result<(), TmcError> { + Ok(()) + } + + fn points_parser(i: &str) -> IResult<&str, Vec<&str>, VerboseError<&str>> { + combinator::map( + sequence::delimited( + ( + bytes::complete::tag("@"), + character::complete::multispace0, + bytes::complete::tag_no_case("points"), + character::complete::multispace0, + character::complete::char('('), + character::complete::multispace0, + ), + branch::alt(( + sequence::delimited( + character::complete::char('"'), + bytes::complete::is_not("\""), + character::complete::char('"'), + ), + sequence::delimited( + character::complete::char('\''), + bytes::complete::is_not("'"), + character::complete::char('\''), + ), + )), + ( + character::complete::multispace0, + character::complete::char(')'), + ), + ), + |s: &str| vec![s.trim()], + ) + .parse(i) + } + + fn get_default_student_file_paths() -> Vec { + vec![PathBuf::from("src")] + } + + fn get_default_exercise_file_paths() -> Vec { + vec![PathBuf::from("test")] + } +} diff --git a/crates/tmc-langs-plugins/src/lib.rs b/crates/tmc-langs-plugins/src/lib.rs index 0d0b1211c46..7014c184163 100644 --- a/crates/tmc-langs-plugins/src/lib.rs +++ b/crates/tmc-langs-plugins/src/lib.rs @@ -306,6 +306,16 @@ impl PluginType { delegate_plugin_type!(self, find_project_dir_in_archive(archive)) } + pub fn safe_find_project_dir_in_archive( + self, + archive: &mut Archive, + ) -> Result { + Ok(delegate_plugin_type!( + self, + safe_find_project_dir_in_archive(archive) + )) + } + pub fn get_available_points(self, exercise_path: &Path) -> Result, TmcError> { delegate_plugin_type!(self, get_available_points(exercise_path)) } diff --git a/crates/tmc-langs/src/submission_packaging.rs b/crates/tmc-langs/src/submission_packaging.rs index bbf48bdebe3..226713f711b 100644 --- a/crates/tmc-langs/src/submission_packaging.rs +++ b/crates/tmc-langs/src/submission_packaging.rs @@ -53,7 +53,7 @@ pub fn prepare_submission( file_util::LOCK_FILE_NAME, ]; if let Some((stub_zip, compression)) = stub_archive { - // if defined, extract and use as the base + // This branch is used when a student downloads their old submission, and we take the files from the stub (the exercise template) instead of the clone path. This makes sure they cannot see the hidden tests in the downloaded file. extract_with_filter( plugin, stub_zip, @@ -70,7 +70,7 @@ pub fn prepare_submission( false, )?; } else { - // else, copy clone path + // This code branch is used when we package a submission for the sandbox. We use the clone path because it contains the hidden tests, and we want the sandbox to run them. for entry in WalkDir::new(stub_clone_path).min_depth(1) { let entry = entry?; @@ -101,8 +101,25 @@ pub fn prepare_submission( log::debug!("extracting student files"); let file = file_util::open_file(submission.archive)?; if submission.extract_naively { + // If the stub contains a .tmcproject.yml, preserve it so the student's cannot override it + let tmcproject_path = extract_dest_path.join(".tmcproject.yml"); + let preserved_tmcproject = if tmcproject_path.exists() { + Some( + std::fs::read(&tmcproject_path) + .map_err(|e| FileError::FileRead(tmcproject_path.clone(), e))?, + ) + } else { + None + }; + extract_project_overwrite(file, &extract_dest_path, submission.compression)?; + + if let Some(bytes) = preserved_tmcproject { + // restore the stub's .tmcproject.yml + file_util::write_to_file(&bytes, &tmcproject_path)?; + } } else { + // This code branch is used when we package a submission for the sandbox. This extraction method makes sure we don't allow the student to update the files they are not allowed to edit. plugin.extract_student_files(file, submission.compression, &extract_dest_path)?; } @@ -297,10 +314,10 @@ fn extract_with_filter bool>( let file = file_util::open_file(archive)?; let mut zip = Archive::new(file, compression)?; let project_dir_in_archive = if naive { - PathBuf::new() + Ok(PathBuf::new()) } else { - plugin.find_project_dir_in_archive(&mut zip)? - }; + plugin.safe_find_project_dir_in_archive(&mut zip) + }?; let mut iter = zip.iter()?; loop { @@ -645,6 +662,89 @@ mod test { ); } + #[test] + fn stub_tmcproject_yml_overrides_student_in_naive_mode() { + init(); + + // Copy an existing Python exercise fixture to a temp clone path + let temp = tempfile::tempdir().unwrap(); + let clone_root = temp.path().join("some_course"); + file_util::create_dir_all(&clone_root).unwrap(); + let src_clone = Path::new(PYTHON_CLONE); + file_util::copy(src_clone, &clone_root).unwrap(); + + let stub_ex_dir = clone_root.join("PythonExercise"); + // Ensure stub has its own .tmcproject.yml + file_util::write_to_file(b"key: stub", stub_ex_dir.join(".tmcproject.yml")).unwrap(); + + // Create a submission zip that attempts to include its own .tmcproject.yml + let sub_zip_path = temp.path().join("submission.zip"); + let sub_zip_file = file_util::create_file(&sub_zip_path).unwrap(); + let mut zw = zip::ZipWriter::new(sub_zip_file); + let opts = SimpleFileOptions::default(); + zw.add_directory("PythonExercise", opts).unwrap(); + zw.add_directory("PythonExercise/src", opts).unwrap(); + zw.start_file("PythonExercise/src/__init__.py", opts) + .unwrap(); + std::io::Write::write_all(&mut zw, b"print('student')\n").unwrap(); + zw.start_file("PythonExercise/.tmcproject.yml", opts) + .unwrap(); + std::io::Write::write_all(&mut zw, b"key: student\n").unwrap(); + zw.finish().unwrap(); + + let output_arch = temp.path().join("out.tar"); + prepare_submission( + PrepareSubmission { + archive: &sub_zip_path, + compression: Compression::Zip, + extract_naively: true, + }, + &output_arch, + false, + TmcParams::new(), + &stub_ex_dir, + None, + Compression::Tar, + ) + .unwrap(); + assert!(output_arch.exists()); + + // Unpack and verify that .tmcproject.yml content is from stub, not student + let output_file = file_util::open_file(&output_arch).unwrap(); + let mut archive = tar::Archive::new(output_file); + let output_extracted = temp.path().join("output"); + archive.unpack(&output_extracted).unwrap(); + + // Verify .tmcproject.yml content is from stub, not student + let yml = + fs::read_to_string(output_extracted.join("some_course/PythonExercise/.tmcproject.yml")) + .unwrap(); + assert!(yml.contains("key: stub")); + assert!(!yml.contains("key: student")); + + // Verify other expected files are present + assert!( + output_extracted + .join("some_course/PythonExercise/src/__init__.py") + .exists() + ); + assert!( + output_extracted + .join("some_course/PythonExercise/src/__main__.py") + .exists() + ); + assert!( + output_extracted + .join("some_course/PythonExercise/test/test_greeter.py") + .exists() + ); + assert!( + output_extracted + .join("some_course/PythonExercise/__init__.py") + .exists() + ); + } + #[test] fn prepare_make_submission() { init(); @@ -683,4 +783,83 @@ mod test { .exists() ); } + + #[test] + fn includes_files_in_root_dir_from_exercise() { + init(); + + let temp = tempfile::tempdir().unwrap(); + let clone_root = temp.path().join("some_course"); + file_util::create_dir_all(&clone_root).unwrap(); + + // Copy the Maven exercise to our temp directory + let src_clone = Path::new(MAVEN_CLONE); + file_util::copy(src_clone, &clone_root).unwrap(); + + let exercise_dir = clone_root.join("MavenExercise"); + + // Create a file in the root directory of the exercise (simulating repo file) + let repo_file_path = exercise_dir.join("foo.txt"); + file_util::write_to_file(b"repohello", &repo_file_path).unwrap(); + + // Create a submission zip that also has foo.txt but with different content + // We need to create a proper Maven project structure that the plugin can understand + let sub_zip_path = temp.path().join("submission.zip"); + let sub_zip_file = file_util::create_file(&sub_zip_path).unwrap(); + let mut zw = zip::ZipWriter::new(sub_zip_file); + let opts = SimpleFileOptions::default(); + + // Create the Maven project structure + zw.add_directory("MavenExercise", opts).unwrap(); + zw.add_directory("MavenExercise/src", opts).unwrap(); + zw.add_directory("MavenExercise/src/main", opts).unwrap(); + zw.add_directory("MavenExercise/src/main/java", opts) + .unwrap(); + + // Add the student's modified file + zw.start_file("MavenExercise/foo.txt", opts).unwrap(); + std::io::Write::write_all(&mut zw, b"submissionhello").unwrap(); + + // Add a source file + zw.start_file("MavenExercise/src/main/java/SimpleStuff.java", opts) + .unwrap(); + std::io::Write::write_all(&mut zw, b"public class SimpleStuff { }").unwrap(); + + zw.finish().unwrap(); + + let output_arch = temp.path().join("out.tar"); + prepare_submission( + PrepareSubmission { + archive: &sub_zip_path, + compression: Compression::Zip, + extract_naively: false, + }, + &output_arch, + false, + TmcParams::new(), + &exercise_dir, + None, + Compression::Tar, + ) + .unwrap(); + assert!(output_arch.exists()); + + // Unpack and verify that foo.txt content is from the repo, not submission + let output_file = file_util::open_file(&output_arch).unwrap(); + let mut archive = tar::Archive::new(output_file); + let output_extracted = temp.path().join("output"); + archive.unpack(&output_extracted).unwrap(); + + // Verify foo.txt exists and has content from repo, not submission + let foo_file = output_extracted.join("some_course/MavenExercise/foo.txt"); + assert!( + foo_file.exists(), + "foo.txt should be included in the archive" + ); + let content = fs::read_to_string(foo_file).unwrap(); + assert_eq!( + content, "repohello", + "Should use repo content, not submission content" + ); + } }