On Wed, 9 Nov 2022, Diogo Sant'Anna wrote:
Hi,
I think your last response didn't reach the curl-library list as you weren't
subscribed yet. I will therefore include more of your response in the quotes
below so that readers can still see what was written.
Well I'm happy you tried on Scorecards, I appreciate the trust! It's
unfortunate that the action was not helpful for you, but your
detailed feedbacks are already of huge value for us and I'll get all of
them to the OpenSSF team.
I want to be clear and say that I'm not dissing the tool nor the idea, I just
don't think it is very helpful *to us*.
I would like to use this reply to complement our discussion about
pinned-dependencies and get some details to try to understand why some
checks did not work properly on your repo, so that we can possibly work to
enhance the tool.
1 - "Code-Review High"
I think this check did not get a good result because the last commits were
not merged through PRs, as I could see in this example
<https://github.com/curl/curl/pull/9691>, and the Scorecards is yet not able
to verify reviews like this. In order to give a more complete feedback and
reason to add this feature, could you explain to me how the review process
works (or send me a link)?
We never click the merge button in the GitHub UI, we always merge
pull-requests "manually" using git. This allows us to make sure the commit
messages are exactly the way we want them and is a much easier/faster way to
squash/cleanup commits etc before merge.
We only fairly infrequently add a "Reviewed-by:" line in the commit message,
most commits are actually done without recording any review details. Most (but
far from all) commits are still reviewed by someone else than its author. I
think primarily my commits are at times not reviewed by anyone else.
2 - "Token-Permissions High"
I think there was a misunderstanding because of the name of the check. The
check is not verifying if you set up a token for the job, it looks for the
permissions given to the GITHUB_TOKEN that's automatically generated for
your workflows, so those permissions turns out to be called
"token-permissions" >< . Please forgive me if I understood incorrectly, but
this confusion makes perfect sense to me and I'm already sending this as
feedback to the Scorecards team.
In any case, the "token-permissions" check verifies if the top-level
permissions are set to read-only, following the principle of least
privilege. Those permissions are related to our discussion about
pinned-dependencies, so I'll join these subjects on the 4th point.
Thanks, I went back and read the descriptions again and the linked-to github
documentation and I think I'm starting to understand what you are talking
about.
I now understand that you point out that the github workflows as they are
setup right now for curl allow the jobs to write contents back into the git
repo as hosted on github?
If so, then we really need to stop that. It was however not immediately clear
to me exactly what we want the permissions to be set to. Is 'read-all' too
restrictive ?
3 - "Dependency-Update-Tool High"
Could you tell which dependency-update-tool the project is currently using?
I can raise an issue to ask for its support.
We don't have depdencies like that so there is no such tool. You build curl to
use the dependencies that are installed in the machine where curl is
built/run. We cannot scan for those on GitHub.
4 - "Pinned-Dependencies Medium"
So, the thing is that, depending on the workflow permissions, an external
GitHub Action actually could modify your code. Take a look at this action
<https://github.com/marketplace/actions/add-commit>, for example; it's an
action created for the purpose of committing into the code of whoever calls
it. Of course this one is intended to be used for good and is trustworthy,
but if a potential malicious user compromises the spell-checker, for
example, he could try similar things and modify your code.
Against this kind of threat, two solutions can be applied (ideally both of
them):
1. Limit the privileges of the workflows, setting them as read-only or
giving only the permissions they need. If no privilege is explicitly set on
the workflow, the default one is used. If you have not changed the default
privilege on your GitHub settings (I can not evaluate that, as this setting
is not public), the default is write-all, which can be dangerous. Adding
these explicit workflow permissions correspond to the "Token-Permissions"
check on Scorecards.
2. Pin the dependencies to specific and unmodifiable versions of the job.
This would mean hash-pinning the dependencies, as even when referring by
version (as @vX.Y.Z), the version can be modified to point to different
code. The downside of this solution is that the job would not be updated
automatically, but some dependency-updates tools(such as dependabot and
renovatebot) are already able to suggest updates on this scenario.
Is read-all what we want?
BTW, in the settings/actions section for the curl/curl repo under the
"Workflow persmissions" title I have selected "Read respository contents
persmission" which is described as: "Workflows have read permissions in the
repository for the contents scope only."
As opposed to the other alternative which says "Workflows have read and write
permissions in the repository for all scopes"
*read permissions* vs *read and write permissions*. If that doesn't restrict
the workflows from writing content, then what does this option mean?
--
/ daniel.haxx.se
| Commercial curl support up to 24x7 is available!
| Private help, bug fixes, support, ports, new features
| https://curl.se/support.html
--
Unsubscribe: https://lists.haxx.se/listinfo/curl-library
Etiquette: https://curl.se/mail/etiquette.html
