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.

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

Feature: add line number to Pinned-Dependencies detail reporting #725

Closed
laurentsimon opened this issue Jul 21, 2021 · 8 comments · Fixed by #1413
Closed

Feature: add line number to Pinned-Dependencies detail reporting #725

laurentsimon opened this issue Jul 21, 2021 · 8 comments · Fixed by #1413
Assignees
Labels
kind/enhancement New feature or request
Milestone

Comments

@laurentsimon
Copy link
Contributor

laurentsimon commented Jul 21, 2021

add a line number to the Pinned-Dependencies, Token-Permissions checks.
Today the details contain the line content only, e.g.:
Warn: insecure (unpinned) download detected in .github/workflows/pylint-presubmit.yml: 'python -m pip install --upgrade pip'

We may want to add a line number as well.

@laurentsimon laurentsimon added the kind/enhancement New feature or request label Jul 21, 2021
@laurentsimon laurentsimon changed the title Feature Feature: add line number to Frozen-Deps detail reporting Jul 21, 2021
@ristomcgehee
Copy link
Contributor

I'll take this issue since I have some time now to work on scorecard now that I've settled in from my move. I'd like to have an easier issue to get back into the swing of things.

@laurentsimon
Copy link
Contributor Author

Thank you!

FYI I'm working on adding SARIF support for scorecard, and for this I need to make some code update to support more structured data in details (line, filename, etc).
ETA for the required update is ~10 days. You can start working on the PR for sure in the meantime, you'll just need to change the call to Warn when I land my update.

@laurentsimon
Copy link
Contributor Author

@chrismcgehee assigned issue to you, thanks again for taking this issue!

@laurentsimon laurentsimon changed the title Feature: add line number to Frozen-Deps detail reporting Feature: add line number to Pinned-Dependencies detail reporting Aug 23, 2021
@laurentsimon laurentsimon added this to the milestone-q3 milestone Aug 23, 2021
@laurentsimon laurentsimon removed this from the milestone-q3 milestone Oct 20, 2021
@laurentsimon
Copy link
Contributor Author

laurentsimon commented Oct 20, 2021

@chrismcgehee this issue is important for the v4 (~EOY) release. Do you have cycles this quarter?
I've finished the SARIF support and you can try it using #1074 (comment)

@laurentsimon
Copy link
Contributor Author

laurentsimon commented Dec 3, 2021

@chrismcgehee what's left to be done for this? Sorry, I've not kept up with all the commits :-)
It'd be awesome to have this ready for v4, just let us know if help is needed. v4 is a planned for release in Jan - flexible on the dates
Do you know if other checks need updates as well?

@ristomcgehee
Copy link
Contributor

@chrismcgehee what's left to be done for this?

I'm just about ready to submit a PR that will add numbers for more places. The line numbers for anything in shell code is going to be tricky because of the way we pass shell code throughout the program. I'd be open to having help for that part.

Do you know if other checks need updates as well?

I believe Pinned Dependencies and Token Permissions are all that need updating. Asra already added proper line numbering for Dangerous Workflow.

@laurentsimon
Copy link
Contributor Author

@chrismcgehee what's left to be done for this?

I'm just about ready to submit a PR that will add numbers for more places. The line numbers for anything in shell code is going to be tricky because of the way we pass shell code throughout the program. I'd be open to having help for that part.

sg. Let us know how we should help. I think you can start by adding the line numbers to entry functions and try to propagate them to callees? Then ask for help when things get tricky? Or would you like to proceed differently?

Do you know if other checks need updates as well?

I believe Pinned Dependencies and Token Permissions are all that need updating. Asra already added proper line numbering for Dangerous Workflow.

cc @asraa

@ristomcgehee
Copy link
Contributor

LGTM. Nice work, Laurent!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants