From 5093faf5658b9963993932719212f935620d9fea Mon Sep 17 00:00:00 2001 From: "K. Andrew Parker" Date: Tue, 7 Jul 2026 12:20:07 -0400 Subject: [PATCH] Determine unarchive source course ID from archive contents, not filename unarchiveCourse() derived the source course ID from the caller-supplied oldCourseID, which the admin controller computes from the archive's *filename*. Archive::Tar->extract(), however, always writes files to the directory name stored *inside* the archive (the course's name at archive time). These agree only until someone renames the .tar.gz. When they diverge (e.g. an archive renamed before import), the course files extract under the archived name while the database-dump lookup, the conflicting-course guard (_unarchiveCourse_move_away), and the post-restore rename all use the filename-derived name. The result is a course whose files restore correctly but whose database is not restored -- reported only as a warning while the unarchive otherwise "succeeds": course 'X' has no database dump in its data directory (checked for .../X/DATA/mysqldump). database tables will not be restored. Fix at the single chokepoint in unarchiveCourse() so every caller is covered: open the archive first and take the source course ID from its sole top-level directory, dying if the archive does not contain exactly one. The caller's newCourseID still controls the target name, so renaming on import continues to work via the "New course ID" field rather than by renaming the file. The oldCourseID option is now unused, so drop it from the sole caller (do_unarchive_course) and from the POD. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 1 - lib/WeBWorK/Utils/CourseManagement.pm | 56 +++++++++++++-------- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index 9f611899c9..e876986b36 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -1202,7 +1202,6 @@ sub do_unarchive_course ($c) { unarchiveCourse( newCourseID => $new_courseID, - oldCourseID => $unarchive_courseID =~ s/\.tar\.gz$//r, archivePath => "$ce->{webworkDirs}{courses}/$ce->{admin_course_id}/archives/$unarchive_courseID", ce => $ce, ); diff --git a/lib/WeBWorK/Utils/CourseManagement.pm b/lib/WeBWorK/Utils/CourseManagement.pm index 5c695658e0..16e69ff6a3 100644 --- a/lib/WeBWorK/Utils/CourseManagement.pm +++ b/lib/WeBWorK/Utils/CourseManagement.pm @@ -920,7 +920,6 @@ sub _archiveCourse_remove_dump_dir { %options must contain: - oldCourseID => $oldCourseID, archivePath => $archivePath, ce => $ce, @@ -928,12 +927,13 @@ sub _archiveCourse_remove_dump_dir { newCourseID => $newCourseID, -Restores course $oldCourseID from a gzipped tar archive (.tar.gz) located at -$archivePath. After unarchiving, the course database is restored from a -subdirectory of the course's DATA directory. +Restores the course contained in the gzipped tar archive (.tar.gz) located at +$archivePath. The ID of the course being restored is taken from the archive's +top-level directory, not from the file name. After unarchiving, the course +database is restored from a subdirectory of the course's DATA directory. -If $newCourseID is defined and differs from $oldCourseID, the course is renamed -after unarchiving. +If $newCourseID is defined and differs from the archived course ID, the course +is renamed after unarchiving. $ce is a WeBWorK::CourseEnvironment object that describes the some course's environment. (Usually this would be the admin course.) This is used to access @@ -946,10 +946,9 @@ If an error occurs, an exception is thrown. sub unarchiveCourse { my (%options) = @_; - my $newCourseID = $options{newCourseID}; - my $currCourseID = $options{oldCourseID}; - my $archivePath = $options{archivePath}; - my $ce = $options{ce}; + my $newCourseID = $options{newCourseID}; + my $archivePath = $options{archivePath}; + my $ce = $options{ce}; my $coursesDir = $ce->{webworkDirs}{courses}; @@ -962,17 +961,32 @@ sub unarchiveCourse { croak "New course ID cannot exceed " . $ce->{maxCourseIdLength} . " characters." if (length($newCourseID) > $ce->{maxCourseIdLength}); - ##### step 1: move a conflicting course away ##### - - # if this function returns undef, it means there was no course in the way - my $restoreCourseData = _unarchiveCourse_move_away($ce, $currCourseID); - - ##### step 2: crack open the tarball ##### + ##### step 1: open the tarball and determine the archived course ID ##### my $arch = Archive::Tar->new($archivePath); die "The tar file $archivePath is not valid." unless $arch; $arch->setcwd($coursesDir); + # Archive::Tar extracts to the directory name stored in the archive, so the + # source course ID must come from there and not from the caller-supplied name + # -- otherwise a renamed .tar.gz restores files under one name while the + # database dump is sought under another. + my %top_level; + for my $file ($arch->get_files) { + (my $first = $file->full_path) =~ s{/.*}{}s; + $top_level{$first} = 1 if length $first; + } + die "The archive $archivePath does not contain a single top-level course directory.\n" + unless keys %top_level == 1; + my ($currCourseID) = keys %top_level; + + ##### step 2: move a conflicting course away ##### + + # if this function returns undef, it means there was no course in the way + my $restoreCourseData = _unarchiveCourse_move_away($ce, $currCourseID); + + ##### step 3: crack open the tarball ##### + # Secure extract mode refuses symbolic/hard links whose targets leave the # course directory (CVE-2026-42496/-42497), which the standard template links # do. Extract the files under secure mode, then recreate the symbolic links. @@ -992,7 +1006,7 @@ sub unarchiveCourse { symlink($symlink->linkname, $link_path) unless -e $link_path; } - ##### step 3: read the course environment for this course ##### + ##### step 4: read the course environment for this course ##### my $ce2 = WeBWorK::CourseEnvironment->new({ get_SeedCE($ce), courseName => $currCourseID }); @@ -1001,7 +1015,7 @@ sub unarchiveCourse { my $data_dir = $ce2->{courseDirs}{DATA}; my $dump_dir = "$data_dir/mysqldump"; - ##### step 4: restore the database tables ##### + ##### step 5: restore the database tables ##### my $no_database; my $restore_db_result = 1; @@ -1018,7 +1032,7 @@ sub unarchiveCourse { warn "database restore of course '$currCourseID' failed: the course will probably not be usable.\n"; } - ##### step 5: delete dump_dir ##### + ##### step 6: delete dump_dir ##### _archiveCourse_remove_dump_dir($ce, $dump_dir) if -e $dump_dir; @@ -1049,7 +1063,7 @@ sub unarchiveCourse { } } - ##### step 6: rename course ##### + ##### step 7: rename course ##### if (defined $newCourseID && $newCourseID ne $currCourseID) { renameCourse( @@ -1060,7 +1074,7 @@ sub unarchiveCourse { ); } - ##### step 7: return conflicting course to its rightful place ##### + ##### step 8: return conflicting course to its rightful place ##### _unarchiveCourse_move_back($restoreCourseData); }