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

securityscorecards: Fix "Branch-Protection" check #7201

Closed
jcfr opened this issue Aug 29, 2023 · 3 comments · Fixed by #7207
Closed

securityscorecards: Fix "Branch-Protection" check #7201

jcfr opened this issue Aug 29, 2023 · 3 comments · Fixed by #7207
Labels
Type: Bug Something isn't working correctly

Comments

@jcfr
Copy link
Member

jcfr commented Aug 29, 2023

Summary

Following the integration of #7197, a list of checks are performed with the intent of given a "security" score.

The "Branch-Protection" check is currently failing with the following message:

internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration

Following the recommendation described in the following issue should help:

Steps to reproduce

See https://securityscorecards.dev/viewer/?uri=github.com/Slicer/Slicer

Expected behavior

No internal error

@jcfr jcfr added the Type: Bug Something isn't working correctly label Aug 29, 2023
@jcfr
Copy link
Member Author

jcfr commented Aug 29, 2023

Specific instruction are documented here:

@jcfr
Copy link
Member Author

jcfr commented Aug 29, 2023

Given the open-source nature of the Slicer project, I don't see an issue granting the scorecard.yml workflow read access to the administration scope1.

@jamesobutler @pieper @sjh26 @lassoan Do you have any concern ?

Read access associated with the Administration scope

GET /repos/{owner}/{repo}/actions/permissions
GET /repos/{owner}/{repo}/actions/permissions/access
GET /repos/{owner}/{repo}/actions/permissions/selected-actions
GET /repos/{owner}/{repo}/actions/permissions/workflow
GET /repos/{owner}/{repo}/actions/runners
GET /repos/{owner}/{repo}/actions/runners/downloads
GET /repos/{owner}/{repo}/actions/runners/{runner_id}
GET /repos/{owner}/{repo}/actions/runners/{runner_id}/labels
GET /repos/{owner}/{repo}/autolinks
GET /repos/{owner}/{repo}/autolinks/{autolink_id}
GET /repos/{owner}/{repo}/automated-security-fixes
GET /repos/{owner}/{repo}/branches/{branch}/protection
GET /repos/{owner}/{repo}/branches/{branch}/protection/enforce_admins
GET /repos/{owner}/{repo}/branches/{branch}/protection/required_pull_request_reviews
GET /repos/{owner}/{repo}/branches/{branch}/protection/required_signatures
GET /repos/{owner}/{repo}/branches/{branch}/protection/required_status_checks
GET /repos/{owner}/{repo}/branches/{branch}/protection/required_status_checks/contexts
GET /repos/{owner}/{repo}/branches/{branch}/protection/restrictions
GET /repos/{owner}/{repo}/branches/{branch}/protection/restrictions/apps
GET /repos/{owner}/{repo}/branches/{branch}/protection/restrictions/teams
GET /repos/{owner}/{repo}/branches/{branch}/protection/restrictions/users
GET /repos/{owner}/{repo}/interaction-limits
GET /repos/{owner}/{repo}/invitations
GET /repos/{owner}/{repo}/keys
GET /repos/{owner}/{repo}/keys/{key_id}
GET /repos/{owner}/{repo}/tags/protection
GET /repos/{owner}/{repo}/teams
GET /repos/{owner}/{repo}/traffic/clones
GET /repos/{owner}/{repo}/traffic/popular/paths
GET /repos/{owner}/{repo}/traffic/popular/referrers
GET /repos/{owner}/{repo}/traffic/views
GET /repos/{owner}/{repo}/vulnerability-alerts
GET /user/repository_invitations

Footnotes

  1. https://docs.github.com/en/rest/overview/permissions-required-for-fine-grained-personal-access-tokens?apiVersion=2022-11-28#repository-permissions-for-administration

@jamesobutler
Copy link
Contributor

Yes I see no problem adding a token for read access to the administration information. @jcfr following the instructions at https://github.com/ossf/scorecard-action#authentication-with-fine-grained-pat-optional makes sense. Since you are a main Slicer developer, re-upping the token whenever it expires seems reasonable and a low effort future task to enable the scorecard action to have access to what it needs for the additional checks.

jcfr added a commit to jcfr/Slicer that referenced this issue Aug 29, 2023
jcfr referenced this issue Sep 27, 2023
Qt lupdate threw warnings about unconsumed metadata for lines that had translator's comments (that is exported to the language translation file to provide additonal context for translators) in the same line as the translatable text. For example:

    this->SupportedReadFileTypes->InsertNextValue(vtkMRMLTr("vtkMRMLColorTableStorageNode", "MRML Color Table") + " (.ctbl)");  //: file format name

The issue with this is that Qt expects translator comment to be in the previous line like this:

    //: File format name
    this->SupportedReadFileTypes->InsertNextValue(vtkMRMLTr("vtkMRMLColorTableStorageNode", "MRML Color Table") + " (.ctbl)");
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working correctly
Development

Successfully merging a pull request may close this issue.

2 participants