Request: make the internal/packageclient package not internal

[jeffmendoza]

Is your feature request related to a problem? Please describe. In Allstar, we call individual checks directly using the Check Scruct and CheckFn (type CheckFn func(*CheckRequest) CheckResult). To create a CheckRequest we create the individual clients directly, and setup the logger. This is due to already having GitHub auth among other reasons. We don't use GetClients() directly.

To call the "Signed-Releases" check we need to set a ProjectClient packageclient.ProjectPackageClient in the CheckRequest, however the constructor is in internal/packageclient which is internal.

Describe the solution you'd like Make internal/packageclient not internal.

Describe alternatives you've considered Could call GetClients() and use the projectClient and discard the clients that we need to create separately.

[spencerschrock]

Describe alternatives you've considered Could call GetClients() and use the projectClient and discard the clients that we need to create separately.

Ah shoot, GetClients() may have been something else we should have ripped out before we released v5.

Have you considered the new scorecard.Run entrypoint as a way of calling individual checks? If that's not feasible, we could expose packageclient.

[jeffmendoza]

I had looked at checker.Runner and decided it wasn't needed. I'll take a look at pkg/scorecard and see if it will work and get back to you.

[jeffmendoza]

Ok, this is what I've tried for running with an authenticated transport. It all seems to be working well for me:

package main

import (
    "context"
    "fmt"
    "log"
    "net/http"
    "os"

    "github.com/bradleyfalzon/ghinstallation/v2"
    "github.com/google/go-github/v63/github"
    "github.com/ossf/scorecard/v5/clients/githubrepo"
    "github.com/ossf/scorecard/v5/pkg/scorecard"
)

const appID = 112922
const keyFile = "/path/to/private-key.pem"
const orgName = "orgname"
const repoName = "orgname/repo"
const check = "Token-Permissions"

func main() {
    if err := do(); err != nil {
        log.Fatal(err)
    }
    log.Print("success")
}

func do() error {
    dt := http.DefaultTransport
    ctx := context.Background()

    // Read in GitHub App private key
    key, err := os.ReadFile(keyFile)
    if err != nil {
        return err
    }

    // Create authenticated transport for App
    at, err := ghinstallation.NewAppsTransport(dt, appID, key)
    if err != nil {
        return err
    }

    // Query GitHub to find installation ID of App on org/owner we want to scan
    ghc := github.NewClient(&http.Client{Transport: at})
    inst, _, err := ghc.Apps.FindOrganizationInstallation(ctx, orgName)
    if err != nil {
        return err
    }

    // Create a new authenticated transport for App installation that will be
    // used for Scorecard.
    it := ghinstallation.NewFromAppsTransport(at, inst.GetID())

    // Create Scorecard RepoClient using authenticated transport
    rc := githubrepo.CreateGithubRepoClientWithTransport(ctx, it)

    // Create Scorecard Repo object for repo name we want to scan
    repo, err := githubrepo.MakeGithubRepo(repoName)
    if err != nil {
        return err
    }

    // Run scorecard with Repo, RepoClient, and list of Checks
    res, err := scorecard.Run(ctx, repo,
        scorecard.WithRepoClient(rc),
        scorecard.WithChecks([]string{check}),
    )
    if err != nil {
        return err
    }

    // Interpret check results
    if len(res.Checks) != 1 {
        return fmt.Errorf("got unexpected number of check results: %d", len(res.Checks))
    }
    if res.Checks[0].Name != check {
        return fmt.Errorf("got unexpected check result name: %q", res.Checks[0].Name)
    }
    fmt.Printf("%q score: %d\n", check, res.Checks[0].Score)
    fmt.Printf("Reason: %s\n", res.Checks[0].Reason)
    return nil
}

Do you see any issues?

[spencerschrock]

Do you see any issues?

I skimmed everything starting before the call to scorecard.Run, as it likely matches what Allstar already does to setup Scorecard clients in https://github.com/ossf/allstar/blob/38d990dae8dcbc3eaeb4f853432e00350facea56/pkg/scorecard/scorecard.go.

With regard to the scorecard.Run, it seems like a sensible list of arguments based on what you want to do:

• The check names are available as constants in the github.com/ossf/scorecard/v5/checks if you prefer that over writing it yourself.
• There's some optimizations with tradeoffs you could take if desired. Reusing the same OSS-Fuzz client could save you a network call and a 1.3MB download per "Fuzzing" check, at the expense of using cached data.
• Result parsing seems fine, slightly clunkier than what you're doing now but is the interface.

[jeffmendoza]

Thanks, I'll test creating and saving a Fuzz client between runs as well. Closing this as completed.