Make sure we don't use .tmcproject.yml from submissions

Author: nygrenh

crates/tmc-langs-framework/src/plugin.rs

@@ -217,6 +217,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,
@@ -230,15 +231,6 @@ pub trait LanguagePlugin {
         let project_dir = Self::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()?;
@@ -1111,4 +1103,28 @@ 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());
+    }
 }

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)?;
     }
 
@@ -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();