Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛Binary-Artifacts: no longer complain about ".bin" files #1288

Merged
merged 1 commit into from
Nov 17, 2021
Merged

🐛Binary-Artifacts: no longer complain about ".bin" files #1288

merged 1 commit into from
Nov 17, 2021

Conversation

evverx
Copy link
Contributor

@evverx evverx commented Nov 17, 2021 •
edited
Loading

Those files most likely contain binary data used by tests for
example. It should be safe to remove this because executables
disguised as ".bin" files will still be caught and flagged by scorecard
before it even has a chance to look at extensions.

It should address #1256

@evverx
Binary-Artifacts: no longer complain about ".bin" files
f7f5227 
Those files most likely contain binary data used by tests for
example. It should be safe to remove this because executables
disguised as ".bin" files will still be caught and flagged by scorecard
before it even have a chance to look at extensions.

It should address #1256
@evverx evverx mentioned this pull request Nov 17, 2021
Closed
@evverx evverx changed the title Binary-Artifacts: no longer complain about ".bin" files 🐛Binary-Artifacts: no longer complain about ".bin" files Nov 17, 2021
@laurentsimon laurentsimon self-requested a review November 17, 2021 00:54
laurentsimon
laurentsimon approved these changes Nov 17, 2021
View reviewed changes
Copy link
Contributor

@laurentsimon laurentsimon left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@laurentsimon
Copy link
Contributor

laurentsimon commented Nov 17, 2021 •
edited
Loading

You also suggested using IsArchive() which seemed like a reasonable approach to me. Could you dump here your arguments in favor of it from a previous issue/PR? Maybe send another PR with it, so we can merge this one.

@naveensrinivasan write this part of the code, let see what he thinks.

@laurentsimon laurentsimon enabled auto-merge (squash) November 17, 2021 00:57
@laurentsimon laurentsimon merged commit 0bd5756 into ossf:main Nov 17, 2021
@evverx
Copy link
Contributor Author

evverx commented Nov 17, 2021

I think scorecard can switch to IsArchive once the library starts to recognize archives deterministically. As far as tell, if for example a file is a .deb package and an .ar archive at the same time filetype can return either .deb or .ar depending on the implementation details. Effectively that means that to catch deb packages scorecard still have to fall back to extensions.

@evverx evverx deleted the remove-bin branch November 17, 2021 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurentsimon laurentsimon laurentsimon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@evverx @laurentsimon