BUG: tarball extraction fails with "file exists" error when archive contains duplicate directory entries

[juanis2112]

Describe the bug
When running Scorecard against certain repositories, the tool exits early with an internal error due to a mkdir: file exists error during tarball extraction. This causes the repository to be skipped entirely.

Error message:

Skipping https://github.com/alterm4nn/ChronoZoom: run: RepoClient.LocalPath: error during tarballHandler.setup: internal error: error during os.Mkdir: mkdir /var/folders/h5/8mnm7jy57ync65g1j_5knyk40000gn/T/repo3602233882/Source/Chronozoom.UI/dumps: file exists

Reproduction steps
Steps to reproduce the behavior:

1. Run scorecards with this repo: https://github.com/alterm4nn/ChronoZoom

Expected behavior
If tarballs contain file entries that share a parent directory, there shouldn't be an error returned.

Additional context
In clients/githubrepo/tarball.go, the extractTarball() function uses os.Mkdir to create directories while iterating over tarball entries. Some tarballs contain multiple file entries that share a parent directory. When the second entry is processed, the parent directory already exists and os.Mkdir returns a "file exists" error, causing the whole process to abort.

Suggested fix
Replacing os.Mkdir with os.MkdirAll in both locations inside extractTarball() (both for GitHub and GitLab) fixes this since os.MkdirAll doesn't return an error when the target exists.

I already tested the fix locally, let me know if this fix sounds okay and if so, I'm happy to submit a PR. :)

P.D: I am running Scorecard across a large number of repositories as part of a security analysis for the University of California system and have encountered some other bugs along the way, so I'm planning to open more issues and I'm also happy to contribute PRs to fix them. Just wanted to give you a heads up, let me know if that's ok! 😊

[spencerschrock]

If tarballs contain file entries that share a parent directory

Any idea what's special about the project to make this an issue? Either way it should be fine to make the change, I'm more just confused how this is happening

[juanis2112]

Thanks @spencerschrock. I looked more into it and the reason this happens, is because in ChronoZoom, Source/Chronozoom.UI/ contains two different paths that differ only by case (Dumps and dumps). I'm using Mac which is case-insensitive so the second path resolves to the same directory. I tested it in Linux and the error doesn't happen.

I opened a PR with the changes proposed. By using MkdirAll we will skip the error while trying to create another directory with the same name and instead the contents of both directories will be merged. I think this is acceptable, but let me know if you can think of a better fix.

[spencerschrock]

By using MkdirAll we will skip the error while trying to create another directory with the same name and instead the contents of both directories will be merged.

Do you know what the native tar binary would do on your case insensitive system? Does it also merge/overwrite?