[Bugfix:System] File lost when running install clean fix#12158
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12158 +/- ##
============================================
- Coverage 21.67% 21.66% -0.01%
- Complexity 9620 9638 +18
============================================
Files 268 268
Lines 36164 36224 +60
Branches 486 487 +1
============================================
+ Hits 7837 7847 +10
- Misses 27845 27894 +49
- Partials 482 483 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
github-actions
Bot
added
the
Abandoned PR - Needs New Owner
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
hyeenaa
left a comment
hyeenaa
left a comment
There was a problem hiding this comment.
code looks good. Additionally, it works, to my knowledge. The bash script, INSTALL_SUBMITTY_HELPER.sh, (/usr/local/submitty/GIT_CHECKOUT/Submitty/.setup/INSTALL_SUBMITTY_HELPER.sh) successfully, to my knowledge, copies over the preserved files to a temporary directory (/tmp) and puts them back where they originally lived after the system install is completed. The file, /usr/local/submitty/config/preserve_files_list.json does not initially exist but when created the script does pick it up.
An optional change, perhaps a small commit, but in the future it might be helpful for the JSON file to exist without having to be explicitly created, but maybe this is where the submitty png could live, or other persistent, non-changing files that don't need to be constantly reinstalled.
github-project-automation
Bot
moved this from In Review
to Awaiting Maintainer Review
in Submitty Development
github-actions
Bot
removed
the
Abandoned PR - Needs New Owner
Rkoester47
left a comment
Rkoester47
left a comment
There was a problem hiding this comment.
I tested the changes on this PR, and can confirm that they allowed me to preserve a file upon a INSTALL_SUBMITTY.sh clean. I created a new test file in a folder, and added the path of this file to preserve_file_list.json. Afterwards, I ran INSTALL_SUBMITTY.sh clean and then verified that my file was preserved after it finished. The file was in place, and the path was still in the preserve_file_list.json as well.
created documentation for [#12158](Submitty/Submitty#12158) related to issue Submitty/Submitty#9511 --------- Co-authored-by: Barb Cutler <[email protected]>
github-project-automation
Bot
moved this from Awaiting Maintainer Review
to Done
in Submitty Development
* main: (50 commits) [Bugfix:Developer] Fix Broken CI (#12851) [Bugfix:System] Restore TCLAPP (#12850) [Bugfix:Submission] Fix non-VCS repo error (#12608) [Feature:Submission] Jupyter warnings outside notebook (#12523) [Bugfix:InstructorUI] Removed feature flag for Bulk Late Days (#12826) [Bugfix:Forum] Fix stats header rendering (#12698) [Bugfix:Autograding] Fix negative number tolerance (#12569) [Bugfix:HelpQueue] Always use Abbreviated Names in OH (#12820) [Bugfix:Autograding] Fix regrade team detection (#12737) [Dependency] Bump Lichen from v23.09.00 to v26.04.01 (#12823) [Dependency] Bump RainbowGrades from v25.10.00 to v26.04.01 (#12824) [Feature:Developer] Remove misc Java utils and TCLAPP (#12819) [Bugfix:TAGrading] Fix 2-panel open-document select (#12577) [Dependency] Bump AnalysisToolsTS from v23.10.00 to v26.04.00 (#12822) [Bugfix:Developer] Fix bump_repo permissions (#12821) [Bugfix:InstructorUI] Change Colors to match in Docker UI table (#12739) [Bugfix:Forum] Reduce Page Caching (#12339) [Bugfix:System] File lost when running install clean fix (#12158) [Bugfix:Submission] No File Rubric submit all user groups (#12812) [Dependency] Bump twig/markdown-extra from 3.23.0 to 3.24.0 in /site (#12721) ...
5 participants
Why is this Change Important & Necessary?
Fixed #9511
Currently there is no way to preserve certain files when running
INSTALL_SUBMITTY.sh cleanFor example, the institution logo on the SAML login screen at
/usr/local/submitty/site/public/img/<INSTITUTION_LOGO>.png
or any other files the sysadmin may not want to have to replace, are deleted by the script.
What is the New Behavior?
It will allow for a json file with a list of absolute paths to files that should be preserved. When CONFIGURE_SUBMITTY.py is run or when the system is first created, it will write to the location:
CONFIG_DIR/preserve_file_list.json
which is probably /usr/local/submitty/config/preserve_file_list.json
If there was not a configuration previously, the file will be empty. Otherwise, it will be preserved.
Structure the preserve_file_list.json like so:
All of the absolute paths listed in this file will be preserved.
This is done in the INSTALL_SUBMITTY_HELPER.sh script, where all of the filepaths included are copied to a new directory in /tmp/ at the beginning of the script, and then copied out to their previous locations once the script is done. Afterwards the directory in tmp is deleted.
What steps should a reviewer take to reproduce or test the bug or new feature?
Verify initially that the preserve_file_list.json will exist even on new system creation (vagrant)
Try making new files, adding their path to the json and running
INSTALL_SUBMITTY.sh cleanand then verifying that the files are still there and intact at the end.
Try specifying files that don't exist, and seeing if CONFIGURE_SUBMITTY.py overwrites your config when you run it again.
Automated Testing & Documentation
I opened a pull request in github.submitty.io for the sysadmin section of submitty.org detailing how to update preserve_files.json
Other information
I am not totally sure whether this is a valid concern or not, but if sensitive files are included in the preserve list then they may be able to be leaked during the time that they are in the tmp directory if their permissions are misconfigured. That shouldn't be a problem though since I don't think users would have any reason to use it with sensitive files. If someone tells me this is a real concern, I can tweak it.
An attacker could also use this to preserve files that we would otherwise want to be cleaned, if they were to get access to the config json.