search

DOI-USGS / ale

Abstraction Layer for Ephemerides (ALE)
Other
13 stars 33 forks source link

Administrative Security Review #440

Closed jessemapel closed 2 years ago

jessemapel commented 2 years ago

As part of the USGS Release process the repo needs and administrative code review as per the USGS Software Management website.

jessemapel commented 2 years ago

I've combed through the repository and cannot find any sensitive information.

jessemapel commented 2 years ago

This needs to be acknowledged or redone by another dev

scsides commented 2 years ago

Admin review for ALE:

A search that shows the potential problems (413 including false possitives) is: grep -r -e " /" -e "\"/" | grep -vi -e "}\/" -e " \/ " -e "\/\/" -e http -e "\/*" -e "\\/" -e ".tf:" -e ".ti:" Some files identified with this grep are SPICE and .cub files.

jessemapel commented 2 years ago

I think the notebook directory can go. All of them are out of date and we're pushing newer versions of that documentation over to USGSCSM.

I will put in a PR to remove the util.py path.

jessemapel commented 2 years ago

@scsides Is this good now that #461 is done?

scsides commented 2 years ago

This looks good now

jlaura commented 2 years ago

@jessemapel The paths were removed, but the fix is not complete. For example: https://github.com/USGS-Astrogeology/ale/blob/da2e7b2faadcefe0d33153e3c89b97a436699a5e/ale/util.py

When one goes back through the history on the repo, the absolute paths are maintained. The remediation (which is good practice if we ever have to do this for something more serious than these absolute paths) is to do a history rewrite.

jessemapel commented 2 years ago

I've cleaned out the paths from the history now too. I completely erased the notebooks from the repo.

For the issues in util.py I had to do a bit more in depth modifications. I saved off the clean version then completely wiped them from the repo. Then, I used rebase to re-insert them as a new commit after 0.8. Finally, I had to go back and update all of the tags for 0.8.1 - 0.8.7 to point to the rebased history with the clean util.py file in them.

So, we should still be able to create the old release back to 0.8.1 (including the ones ISIS uses) without issue now. The history is going to look a bit weird and people probably got spammed about me creating a bunch of new releases, but all of that can be ignored. It's just from me re-tagging and re-publishing the old releases.

jessemapel commented 2 years ago

GitHub doesn't allow re-opening PRs even after the branch coming in has been updated. So impacted PRs #432 #443 #444 #462 #463 will all need to be re-done. I'm working to get branches updated when I have permission. If I don't have permission, then I'm pushing them onto new branches on this repo.

jessemapel commented 2 years ago

If anyone has outstanding changes that are committed to their local repo but have not been PR'd yet here's how to get them rebased onto the new master branch. I'm going to show examples from working on #443

Find the last commit on your branch that existed on this repo. Use git log and look for replaced

commit eda3951a326660b9550543ec1accb85ddfb8eb54 (HEAD -> CrismDriver, amy/CrismDriver)
Author: Amy Stamile <74275278+amystamile-usgs@users.noreply.github.com>
Date:   Thu Mar 17 11:08:33 2022 -0700

    Rename FRT00003B73_01_IF156S_TRR2_isis.lbl to FRT00003B73_01_IF156S_TRR2_isis3.lbl

commit e988348648904689dbea33cdba703494573a6a34
Author: Amy Stamile <astamile@contractor.usgs.gov>
Date:   Thu Mar 17 07:59:25 2022 -0700

    Modified KernelSlice and crism_isd.

commit 3ef4eb920ff7d89a06de7e958c7e44470e7ee837
Author: Amy Stamile <astamile@contractor.usgs.gov>
Date:   Mon Mar 14 08:15:24 2022 -0700

    Fixes to_isd errors.

commit bb2b78e31756e409029b5265d2411cbe422e45b6
Author: Amy Stamile <astamile@contractor.usgs.gov>
Date:   Fri Mar 11 10:21:20 2022 -0700

    Updates instrument id and sensor name lookups.

commit 362cbe483ef00d6a605d96ec74b570ed22c1a559
Author: Amy Stamile <astamile@contractor.usgs.gov>
Date:   Thu Mar 10 10:44:28 2022 -0700

    Initial crism driver.

commit 03bf4ec42e509088a7ace0c7cfa50859858ff2bd (replaced, amy/master) <--- This is the commit we want
Author: Jesse Mapel <jmapel@usgs.gov>
Date:   Fri Mar 4 14:15:52 2022 -0700

    Relicense to CC-0 and add meta data for USGS release (#439)

    * Relicensed to CC-0

    * Added code.json file

    * Added disclaimer

    * Updated laborHours estimate

    * Fixed release URLs

In our example this is 03bf4ec42e509088a7ace0c7cfa50859858ff2bd

get the most recent commit on this repo's master, in this case 78253de63ddb386cf57c0279fbaa519ea71ce019

Then rebase your local commit onto this repo's master commit. git rebase --onto <new-master-commit> <old-master-commit>. So for our example it would be git rebase --onto 78253de63ddb386cf57c0279fbaa519ea71ce019 03bf4ec42e509088a7ace0c7cfa50859858ff2bd.

You will potentially hit some merge conflicts here but they should be straight forward to resolve. Anything that you accidentally add back into the repo should show up in your PR as new changes and need to be removed prior to squashing and merging your PR. In our example I got merge conflicts for util.py and test_util.py being added in both places

jmapel$ git rebase --onto 78253de63ddb386cf57c0279fbaa519ea71ce019 03bf4ec42e509088a7ace0c7cfa50859858ff2bd
Auto-merging ale/drivers/mro_drivers.py
Auto-merging ale/util.py
CONFLICT (add/add): Merge conflict in ale/util.py
Auto-merging tests/pytests/test_util.py
CONFLICT (add/add): Merge conflict in tests/pytests/test_util.py
error: could not apply 362cbe4... Initial crism driver.
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 362cbe4... Initial crism driver.
(knoten_cleanup) IGSWZAWGLT00014:ale_cleanup jmapel$ git status
interactive rebase in progress; onto 78253de
Last command done (1 command done):
   pick 362cbe4 Initial crism driver.
Next commands to do (4 remaining commands):
   pick bb2b78e Updates instrument id and sensor name lookups.
   pick 3ef4eb9 Fixes to_isd errors.
  (use "git rebase --edit-todo" to view and edit)
You are currently rebasing branch 'CrismDriver' on '78253de'.
  (fix conflicts and then run "git rebase --continue")
  (use "git rebase --skip" to skip this patch)
  (use "git rebase --abort" to check out the original branch)

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
    modified:   ale/drivers/mro_drivers.py
    new file:   notebooks/write_CassiniIssPds3LabelNaifSpiceDriver.ipynb
    new file:   notebooks/write_MessengerMDISPds3LabelNaifSpiceDriver.ipynb
    new file:   notebooks/write_MexHrscPds3LabelNaifSpiceDriver.ipynb
    new file:   notebooks/write_MroCrismIsisLabelNaifSpiceDriver.ipynb
    new file:   tests/pytests/data/FRT00003B73_01_IF156S_TRR2/FRT00003B73_01_IF156S_TRR2_isis.lbl
    modified:   tests/pytests/test_mro_drivers.py

Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add <file>..." to mark resolution)
    both added:      ale/util.py
    both added:      tests/pytests/test_util.py

I simply need to accept all of the current changes git checkout --ours ale/util.py git checkout --ours tests/pytests/test_util.py and then use git add ale/util.py tests/pytests/test_util.py to flag them as fine and then git rebase --continue runs cleanly.