diff --git a/.github/workflows/vulnetix.yml b/.github/workflows/vulnetix.yml index 20a47f2e5..c7c45b5a1 100644 --- a/.github/workflows/vulnetix.yml +++ b/.github/workflows/vulnetix.yml @@ -259,6 +259,7 @@ jobs: org-id: ${{ secrets.VULNETIX_ORG_ID }} api-key: ${{ secrets.VULNETIX_API_KEY }} fail-on-findings: false + github-token: "" - uses: Vulnetix/cli/github-advisories@v4.24.30 with: org-id: ${{ secrets.VULNETIX_ORG_ID }} @@ -277,6 +278,7 @@ jobs: org-id: ${{ secrets.VULNETIX_ORG_ID }} api-key: ${{ secrets.VULNETIX_API_KEY }} fail-on-findings: false + github-token: "" - uses: Vulnetix/cli/bom-validate@v4.24.30 with: org-id: ${{ secrets.VULNETIX_ORG_ID }} @@ -391,6 +393,7 @@ jobs: org-id: ${{ secrets.VULNETIX_ORG_ID }} api-key: ${{ secrets.VULNETIX_API_KEY }} fail-on-findings: false + github-token: "" - uses: Vulnetix/cli/upload@v4.24.30 with: org-id: ${{ secrets.VULNETIX_ORG_ID }} diff --git a/AGENTS.md b/AGENTS.md index cc38152d8..dc2d792ba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -384,6 +384,29 @@ than HTTP 404 still marks the repo indefinite, so the same misses are stored. `VULNETIX_GH_PARITY=1 go test ./internal/license -run TestGHAPIDirectMatchesGH` compares the two against the live API; run it when adding an expression. +Those lookups spend the GitHub API quota that, inside GitHub Actions, the whole +repository shares. On the push that merged #341, the scan jobs' lookups spent +4,913 requests in under three and a half minutes, and SCA's SARIF upload and +analyze's forge pass found the quota gone. So: + +- **Validate the name first.** `validOwnerRepo` drops a name GitHub cannot + hold before any request: `$/sbom` from `uses: $/sbom`, a glob, ` job each, and the largest job carries a rest step so the jobs together cover what the high-level sast action covers; three or more use the high-level action. Package managers work the same way with sca/. +The job token's GitHub API quota belongs to the whole repository, so the +workflow spends it only where it publishes. Scanners get the token off pull +requests only. A scanner run as an extra's prior step gets none, because it +only writes the file the extra reads. + Refs are written as @v4. Run gh actions-lock afterwards to narrow them to exact tags and lock them; --check ignores refs, so a locked workflow stays current. diff --git a/cmd/license.go b/cmd/license.go index 6fb7ee819..1cd672bb4 100644 --- a/cmd/license.go +++ b/cmd/license.go @@ -218,7 +218,15 @@ var licenseCmd = &cobra.Command{ Manifests are discovered by walking the directory tree, parsed locally, and each package's license is resolved from manifest fields or an embedded SPDX license database. When authenticated, license findings are submitted to -Vulnetix so the scan is recorded; unauthenticated scans stay entirely local. +Vulnetix so the scan is recorded; unauthenticated scans submit nothing. + +A license those cannot name is looked up in the Vulnetix store (when +authenticated), the package's registry and deps.dev, and then, with a GitHub +token (GITHUB_TOKEN, GH_TOKEN or the gh CLI's), on GitHub. The GitHub lookups +stop while part of the token's hourly rate limit is left: half, or 80% inside +GitHub Actions, where the job token's quota belongs to the whole repository. +They also stop when GitHub answers with a rate limit. A license they did not +check stays UNKNOWN, and a warning says how many repositories went unchecked. License findings are stored in the CycloneDX SBOM at .vulnetix/sbom.cdx.json alongside vulnerability findings (neither overwrites the other). diff --git a/docs/github-actions.md b/docs/github-actions.md index 255b6c2ee..02ac098ef 100644 --- a/docs/github-actions.md +++ b/docs/github-actions.md @@ -75,6 +75,27 @@ report declares what it did not fetch. The workflow `vulnetix gha init` writes p run (`workflow_dispatch`) reads the full `--window-days` window from GitHub. The git history window stays at 365 days in every case. +The scanners that resolve licences (`sca`, `license`, `sbom`, `containers` and `scan`) ask +GitHub for a repository's licence when the Vulnetix store, the package registry and deps.dev +cannot name it, with the same job token. Those lookups spend from the same quota: + +- **A name GitHub cannot hold is never asked about.** That includes a workflow's `uses: $/sbom`, + a glob and a template placeholder. +- **The lookup chain is short.** The repository answers first. A 404 there ends the chain; + otherwise the next steps are GitHub's own licence detection, then the licence files that + the root listing names, so a repository costs one to four requests. +- **Inside GitHub Actions the lookups keep 80% of the token's hourly limit for everything + else.** They stop at 4,000 left of 5,000, above the half that analyze's forge pass keeps. + Every job of a run sees the same remaining count, so the floor holds for the whole run. +- **They also stop as soon as GitHub answers with a rate limit.** A licence they did not + check stays `UNKNOWN`, and a warning names how many repositories went unchecked. +- **Each job logs what it spent.** In Actions, every batch of lookups prints a line such as + `GitHub licence lookups: 31 requests for 26 repositories (4880 of 5000 left)`. + +`vulnetix gha init` passes the token to a scanner only off pull requests. It passes no token +to a scanner that runs as an extra's `prior` step, because that step only writes the file the +extra reads. + They are generated, not hand-written: `tools/actiongen/spec.go` is the table, and `go generate ./tools/actiongen` writes every `/action.yml`, every `docs/github-actions/.md` and `action-lib/commands.sh`. `TestGeneratedFilesAreCurrent` diff --git a/internal/analyze/forge/budget.go b/internal/analyze/forge/budget.go index 13f61d13e..c1282b08f 100644 --- a/internal/analyze/forge/budget.go +++ b/internal/analyze/forge/budget.go @@ -1,134 +1,14 @@ package forge -// The request budget. -// -// GitHub's rate limit is the single most common way a tool like this produces a wrong -// answer. There are three ways to get it wrong and all three are in the prior art: -// -// - issue-metrics catches the rate-limit exception and calls sys.exit(1). The whole run is -// lost, including the work already done. -// - Several tools sleep until the reset, which for a core-quota exhaustion is up to an -// hour. A CI job does not have an hour. -// - The worst option, and the tempting one: catch the error, return what you have, and say -// nothing. The report then quietly understates every count it touched, and looks exactly -// like a report from a repository that simply has less going on. -// -// So: the budget stops the collector *before* the quota runs out, and what was not fetched -// is declared. A metric that ran out of budget is `truncated` with an `omittedCount` and a -// reason. It is not a failure, and it is not a silence. +// The request budget lives in internal/ghquota, which the licence lookups share: both +// spend the same token, and inside GitHub Actions the same repository-wide quota. -import ( - "errors" - "fmt" - "sync" +import "github.com/vulnetix/cli/v4/internal/ghquota" - "github.com/google/go-github/v66/github" -) +type budget = ghquota.Budget -// reserve is the least number of requests we refuse to spend. Leaving headroom matters -// because this token is usually shared — with other steps in the same CI job, and in -// vdb-manager's case with other processors — and draining it to zero breaks them, not us. -const reserve = 100 +// newBudget keeps half the token's hourly limit for the other users of the token. +func newBudget() *budget { return ghquota.NewBudget() } -// reserveShare is the fraction of the hourly limit we refuse to spend, when it is more than -// reserve. A fixed 100 was not enough headroom: inside GitHub Actions the job token's quota -// (1,000 an hour) belongs to the whole repository, and a forge pass on every push to the -// default branch spent it down to 100. CodeQL's SARIF upload and the release dispatch then -// failed with "API rate limit exceeded for installation". Keeping half the limit bounds -// what one run can take, whatever else runs beside it. -const reserveShare = 0.5 - -// floor is the remaining quota at which the collector stops, for a token with this hourly -// limit. -func floor(limit int) int { - return max(reserve, int(float64(limit)*reserveShare)) -} - -type budget struct { - mu sync.Mutex - - remaining int - limit int - known bool - - // spent counts requests we made. Used when GitHub does not tell us the remaining quota - // (enterprise instances sometimes do not) so we can still bound the work. - spent int - - exhausted bool -} - -func newBudget() *budget { - return &budget{} -} - -// Observe records the rate-limit state GitHub reported on a response. Every call the -// collector makes should pass its response through here. -func (b *budget) Observe(resp *github.Response) { - if resp == nil { - return - } - b.mu.Lock() - defer b.mu.Unlock() - - b.spent++ - if resp.Rate.Limit > 0 { - b.remaining = resp.Rate.Remaining - b.limit = resp.Rate.Limit - b.known = true - if b.remaining <= floor(b.limit) { - b.exhausted = true - } - } -} - -// Exhausted reports whether the collector should stop. A caller that ignores this and keeps -// going will get a 403 and lose the run; a caller that respects it keeps everything it has -// already gathered. -func (b *budget) Exhausted() bool { - b.mu.Lock() - defer b.mu.Unlock() - - return b.exhausted -} - -// Remaining reports the quota left, and whether GitHub actually told us. -func (b *budget) Remaining() (n int, known bool) { - b.mu.Lock() - defer b.mu.Unlock() - - return b.remaining, b.known -} - -// Spent reports how many requests were made. -func (b *budget) Spent() int { - b.mu.Lock() - defer b.mu.Unlock() - - return b.spent -} - -// Reason explains, in the words that will end up in the report's truncationReason, why the -// collector stopped early. -func (b *budget) Reason() string { - b.mu.Lock() - defer b.mu.Unlock() - - if !b.exhausted { - return "" - } - - return fmt.Sprintf("Stopped at this run's share of the GitHub API rate limit (%d of %d requests left, "+ - "kept for other jobs that share the token); the remaining items were not fetched. "+ - "Re-run when the quota resets, or narrow --forge-window-days.", b.remaining, b.limit) -} - -// IsRateLimit reports whether an error is GitHub telling us to stop. Distinguishing it from -// a real failure matters: one means "come back later", the other means something is broken, -// and treating them the same makes both harder to diagnose. -func IsRateLimit(err error) bool { - var rl *github.RateLimitError - var abuse *github.AbuseRateLimitError - - return errors.As(err, &rl) || errors.As(err, &abuse) -} +// IsRateLimit reports whether an error is GitHub telling us to stop. +func IsRateLimit(err error) bool { return ghquota.IsRateLimit(err) } diff --git a/internal/ghainit/render.go b/internal/ghainit/render.go index 76ff02340..b71552b9f 100644 --- a/internal/ghainit/render.go +++ b/internal/ghainit/render.go @@ -304,6 +304,17 @@ func actionRef(action, ref string) string { return "Vulnetix/cli/" + action + "@" + ref } +// priorScanners are the scanners a prior step can run. A prior step only writes +// the file its extra reads and publishes nothing, so the one thing the job token +// bought it was licence lookups against the repository's GitHub API quota; the +// action's own default token is withheld from it on every event. Two such steps +// (sbom before bom-validate and before upload) looked up the same licences as +// the sbom job on every push. +var priorScanners = map[string]bool{ + "scan": true, "sca": true, "sast": true, "secrets": true, "iac": true, "containers": true, + "license": true, "sbom": true, "cbom": true, "aibom": true, +} + func (r *renderer) extraJob(e Extra) job { id := e.ID if id == "" { @@ -315,6 +326,9 @@ func (r *renderer) extraJob(e Extra) job { if a != "setup" { // setup takes none of these p.with = [][2]string{{"org-id", "${{ secrets.VULNETIX_ORG_ID }}"}, {"api-key", "${{ secrets.VULNETIX_API_KEY }}"}, {"fail-on-findings", "false"}} + if priorScanners[strings.SplitN(a, "/", 2)[0]] { + p.with = append(p.with, [2]string{"github-token", ""}) + } } j.steps = append(j.steps, p) } diff --git a/internal/ghainit/render_test.go b/internal/ghainit/render_test.go index 4d95b92fa..3593bdea7 100644 --- a/internal/ghainit/render_test.go +++ b/internal/ghainit/render_test.go @@ -164,6 +164,50 @@ func TestScannersGetNoTokenOnPullRequests(t *testing.T) { } } +// A prior step writes the file its extra reads and publishes nothing, so a scanner run +// there gets no token, on any event: with the action's default token it looked up +// licences against the repository's GitHub API quota. +func TestPriorScannerStepsGetNoToken(t *testing.T) { + p := DefaultPolicy() + p.Extras = []Extra{ + {Action: "bom-validate", Prior: []string{"sbom"}}, + {Action: "github-advisories", Prior: []string{"sast/go"}}, + {Action: "report", Prior: []string{"gha-status"}}, + } + out := Render(p, &Signals{}, Options{}) + var wf struct { + Jobs map[string]struct { + Steps []struct { + Uses string `yaml:"uses"` + With map[string]*string `yaml:"with"` + } `yaml:"steps"` + } `yaml:"jobs"` + } + if err := yaml.Unmarshal([]byte(out), &wf); err != nil { + t.Fatalf("rendered workflow is not YAML: %v\n%s", err, out) + } + for id, prior := range map[string]string{"bom-validate": "sbom", "github-advisories": "sast/go", "report": "gha-status"} { + steps := wf.Jobs[id].Steps + i := 0 + for i < len(steps) && !strings.HasPrefix(steps[i].Uses, "Vulnetix/cli/") { + i++ + } + if i >= len(steps)-1 || !strings.HasPrefix(steps[i].Uses, "Vulnetix/cli/"+prior+"@") { + t.Fatalf("%s: prior step missing:\n%s", id, out) + } + tok, set := steps[i].With["github-token"] + if prior == "gha-status" { + if set { + t.Errorf("%s: a prior that is not a scanner keeps the action's default token", id) + } + continue + } + if !set || tok == nil || *tok != "" { + t.Errorf("%s: prior %s github-token = %v, want an empty string\n%s", id, prior, tok, out) + } + } +} + func TestSetupExtraTakesNoInputs(t *testing.T) { p := DefaultPolicy() p.Extras = []Extra{{Action: "setup", Then: []string{"vulnetix version"}}} diff --git a/internal/ghquota/budget.go b/internal/ghquota/budget.go new file mode 100644 index 000000000..ce00c98d3 --- /dev/null +++ b/internal/ghquota/budget.go @@ -0,0 +1,232 @@ +// Package ghquota is the GitHub API request budget every caller of the API in this CLI +// shares the design of: analyze's forge pass and the licence lookups. +// +// GitHub's rate limit is the single most common way a tool like this produces a wrong +// answer. There are three ways to get it wrong and all three are in the prior art: +// +// - issue-metrics catches the rate-limit exception and calls sys.exit(1). The whole run is +// lost, including the work already done. +// - Several tools sleep until the reset, which for a core-quota exhaustion is up to an +// hour. A CI job does not have an hour. +// - The worst option, and the tempting one: catch the error, return what you have, and say +// nothing. The report then quietly understates every count it touched, and looks exactly +// like a report from a repository that simply has less going on. +// +// So: the budget stops the caller *before* the quota runs out, and what was not fetched is +// declared. A metric that ran out of budget is `truncated` with an `omittedCount` and a +// reason. It is not a failure, and it is not a silence. +// +// Inside GitHub Actions the job token's quota belongs to the whole repository, so the +// remaining count a response reports is the same number in every job of the run. A floor on +// it is therefore a floor for the run, not just for this process: every job that stops at +// it stops at the same level, however many run at once. +package ghquota + +import ( + "errors" + "fmt" + "net/http" + "strconv" + "sync" + + "github.com/google/go-github/v66/github" +) + +// reserve is the least number of requests we refuse to spend. Leaving headroom matters +// because this token is usually shared — with other steps in the same CI job, and in +// vdb-manager's case with other processors — and draining it to zero breaks them, not us. +const reserve = 100 + +// DefaultShare is the fraction of the hourly limit a budget refuses to spend, when it is +// more than reserve. A fixed 100 was not enough headroom: inside GitHub Actions the job +// token's quota (1,000 an hour, 5,000 for an organisation's installation) belongs to the +// whole repository, and a forge pass on every push to the default branch spent it down to +// 100. CodeQL's SARIF upload and the release dispatch then failed with "API rate limit +// exceeded for installation". Keeping half the limit bounds what one run can take, whatever +// else runs beside it. +const DefaultShare = 0.5 + +// DefaultHint is the advice Reason ends with unless the caller gives its own. +const DefaultHint = "Re-run when the quota resets, or narrow --forge-window-days." + +// Budget bounds a caller's GitHub API requests. It is safe for concurrent use. +type Budget struct { + mu sync.Mutex + + share float64 + hint string + + remaining int + limit int + known bool + + // spent counts requests we made. Used when GitHub does not tell us the remaining quota + // (enterprise instances sometimes do not) so we can still bound the work. + spent int + + exhausted bool + // refused is set when GitHub itself answered with a rate limit, as opposed to the + // budget stopping at its floor. + refused bool +} + +// Option configures a Budget. +type Option func(*Budget) + +// Keep sets the fraction of the hourly limit the budget refuses to spend. A caller whose +// requests matter less than the other users of the token keeps more. +func Keep(share float64) Option { return func(b *Budget) { b.share = share } } + +// Hint sets the advice Reason ends with. +func Hint(h string) Option { return func(b *Budget) { b.hint = h } } + +// NewBudget returns a budget that keeps DefaultShare of the limit. +func NewBudget(opts ...Option) *Budget { + b := &Budget{share: DefaultShare, hint: DefaultHint} + for _, o := range opts { + o(b) + } + return b +} + +// floor is the remaining quota at which the budget stops, for a token with this hourly +// limit. +func (b *Budget) floor(limit int) int { + return max(reserve, int(float64(limit)*b.share)) +} + +// Observe records the rate-limit state GitHub reported on a response. Every call the +// caller makes should pass its response through here. +func (b *Budget) Observe(resp *github.Response) { + if resp == nil { + return + } + b.mu.Lock() + defer b.mu.Unlock() + + b.spent++ + if resp.Rate.Limit > 0 { + b.setRate(resp.Rate.Limit, resp.Rate.Remaining) + } +} + +// ObserveHTTP is Observe for a response read with net/http rather than go-github. A +// response that is GitHub refusing the request for its rate limit, primary or secondary, +// exhausts the budget whatever the headers say. +func (b *Budget) ObserveHTTP(r *http.Response) { + if r == nil { + return + } + rate := github.Rate{} + rate.Limit, _ = strconv.Atoi(r.Header.Get("X-RateLimit-Limit")) + rate.Remaining, _ = strconv.Atoi(r.Header.Get("X-RateLimit-Remaining")) + b.Observe(&github.Response{Response: r, Rate: rate}) + switch r.StatusCode { + case http.StatusTooManyRequests: + b.Refused() + case http.StatusForbidden: + if IsRateLimit(github.CheckResponse(r)) { + b.Refused() + } + } +} + +// Spend counts a request whose response carried no rate-limit state the caller could read +// (one made through the gh CLI, say). It still counts toward Spent. +func (b *Budget) Spend() { + b.mu.Lock() + defer b.mu.Unlock() + b.spent++ +} + +// Seed records the quota GitHub reported before any request was spent (GET /rate_limit, +// which does not count against it). A caller that starts below the floor spends nothing. +func (b *Budget) Seed(limit, remaining int) { + if limit <= 0 { + return + } + b.mu.Lock() + defer b.mu.Unlock() + b.setRate(limit, remaining) +} + +func (b *Budget) setRate(limit, remaining int) { + b.remaining = remaining + b.limit = limit + b.known = true + if b.remaining <= b.floor(b.limit) { + b.exhausted = true + } +} + +// Refused records that GitHub answered with a rate limit. Nothing more is asked: every +// further request would be refused the same way. +func (b *Budget) Refused() { + b.mu.Lock() + defer b.mu.Unlock() + b.exhausted = true + b.refused = true +} + +// Exhausted reports whether the caller should stop. A caller that ignores this and keeps +// going will get a 403 and lose the run; a caller that respects it keeps everything it has +// already gathered. +func (b *Budget) Exhausted() bool { + b.mu.Lock() + defer b.mu.Unlock() + + return b.exhausted +} + +// Remaining reports the quota left, and whether GitHub actually told us. +func (b *Budget) Remaining() (n int, known bool) { + b.mu.Lock() + defer b.mu.Unlock() + + return b.remaining, b.known +} + +// Limit reports the hourly limit, and whether GitHub actually told us. +func (b *Budget) Limit() (n int, known bool) { + b.mu.Lock() + defer b.mu.Unlock() + + return b.limit, b.known +} + +// Spent reports how many requests were made. +func (b *Budget) Spent() int { + b.mu.Lock() + defer b.mu.Unlock() + + return b.spent +} + +// Reason explains, in the words that will end up in the report's truncationReason, why the +// caller stopped early. +func (b *Budget) Reason() string { + b.mu.Lock() + defer b.mu.Unlock() + + if !b.exhausted { + return "" + } + if b.refused && (!b.known || b.remaining > b.floor(b.limit)) { + return "Stopped because GitHub answered that the API rate limit for this token is exhausted; " + + "the remaining items were not fetched. " + b.hint + } + + return fmt.Sprintf("Stopped at this run's share of the GitHub API rate limit (%d of %d requests left, "+ + "kept for other jobs that share the token); the remaining items were not fetched. %s", + b.remaining, b.limit, b.hint) +} + +// IsRateLimit reports whether an error is GitHub telling us to stop. Distinguishing it from +// a real failure matters: one means "come back later", the other means something is broken, +// and treating them the same makes both harder to diagnose. +func IsRateLimit(err error) bool { + var rl *github.RateLimitError + var abuse *github.AbuseRateLimitError + + return errors.As(err, &rl) || errors.As(err, &abuse) +} diff --git a/internal/ghquota/budget_test.go b/internal/ghquota/budget_test.go new file mode 100644 index 000000000..1572b86cc --- /dev/null +++ b/internal/ghquota/budget_test.go @@ -0,0 +1,96 @@ +package ghquota + +import ( + "io" + "net/http" + "strconv" + "strings" + "testing" +) + +func response(status, limit, remaining int, body string) *http.Response { + h := http.Header{} + if limit > 0 { + h.Set("X-RateLimit-Limit", strconv.Itoa(limit)) + h.Set("X-RateLimit-Remaining", strconv.Itoa(remaining)) + } + req, _ := http.NewRequest(http.MethodGet, "https://api.github.com/repos/o/r", nil) + return &http.Response{StatusCode: status, Header: h, Body: io.NopCloser(strings.NewReader(body)), Request: req} +} + +// A caller that reads GitHub with net/http spends from the same budget as one that uses +// go-github, from the same headers. +func TestObserveHTTPReadsTheRateHeaders(t *testing.T) { + b := NewBudget() + b.ObserveHTTP(response(http.StatusOK, 5000, 2501, `{}`)) + if b.Exhausted() { + t.Fatal("2,501 of 5,000 is above half") + } + b.ObserveHTTP(response(http.StatusOK, 5000, 2500, `{}`)) + if !b.Exhausted() || b.Spent() != 2 { + t.Fatalf("exhausted=%v spent=%d, want the floor reached after 2 requests", b.Exhausted(), b.Spent()) + } +} + +// Keep raises the floor for a caller whose requests matter less than the token's other +// users: the licence lookups keep 80% of a job token's quota for everything else. +func TestKeepRaisesTheFloor(t *testing.T) { + b := NewBudget(Keep(0.8), Hint("hint.")) + b.ObserveHTTP(response(http.StatusOK, 5000, 4001, `{}`)) + if b.Exhausted() { + t.Fatal("4,001 of 5,000 is above 80%") + } + b.ObserveHTTP(response(http.StatusOK, 5000, 4000, `{}`)) + if !b.Exhausted() { + t.Fatal("4,000 of 5,000 is the floor") + } + if r := b.Reason(); !strings.Contains(r, "4000 of 5000 requests left") || !strings.HasSuffix(r, "hint.") { + t.Fatalf("reason %q", r) + } +} + +// A secondary rate limit answers 403 with quota still left in the headers; it stops the +// caller all the same, and says GitHub refused rather than that a floor was reached. +func TestObserveHTTPStopsOnARateLimitAnswer(t *testing.T) { + b := NewBudget() + b.ObserveHTTP(response(http.StatusForbidden, 5000, 4000, + `{"message":"You have exceeded a secondary rate limit. Please wait a few minutes before you try again.",`+ + `"documentation_url":"https://docs.github.com/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits"}`)) + if !b.Exhausted() { + t.Fatal("a secondary rate limit did not stop the budget") + } + if !strings.Contains(b.Reason(), "GitHub answered") { + t.Fatalf("reason %q", b.Reason()) + } + + // A 403 that is not a rate limit is a real failure, not a reason to stop everything. + b = NewBudget() + b.ObserveHTTP(response(http.StatusForbidden, 5000, 4000, `{"message":"Resource not accessible by integration"}`)) + if b.Exhausted() { + t.Fatal("a permission 403 stopped the budget") + } +} + +// Seed takes the quota from GET /rate_limit, which costs nothing: a caller that starts +// below its floor stops before its first request. +func TestSeedBelowTheFloorSpendsNothing(t *testing.T) { + b := NewBudget(Keep(0.8)) + b.Seed(5000, 3000) + if !b.Exhausted() || b.Spent() != 0 { + t.Fatalf("exhausted=%v spent=%d", b.Exhausted(), b.Spent()) + } + b = NewBudget() + b.Seed(0, 0) // no answer: nothing known, nothing stopped + if b.Exhausted() { + t.Fatal("an unknown quota stopped the budget") + } +} + +func TestSpendCountsWithoutRate(t *testing.T) { + b := NewBudget() + b.Spend() + b.Spend() + if b.Spent() != 2 || b.Exhausted() { + t.Fatalf("spent=%d exhausted=%v", b.Spent(), b.Exhausted()) + } +} diff --git a/internal/license/ghapi.go b/internal/license/ghapi.go index 062a8a3ad..d6dd6c4c3 100644 --- a/internal/license/ghapi.go +++ b/internal/license/ghapi.go @@ -12,6 +12,8 @@ import ( "strings" "sync" "time" + + "github.com/vulnetix/cli/v4/internal/ghquota" ) // The licence lookups asked GitHub through `gh api`, one process per request: @@ -30,6 +32,19 @@ var ( ghToken string ) +// lookupToken is the token the in-process lookups send: gh's (which is +// GITHUB_TOKEN or GH_TOKEN when either is set, since gh reads them first), else, +// with no gh, GITHUB_TOKEN or GH_TOKEN themselves. +func lookupToken() string { + if t := ghAuthToken(); t != "" { + return t + } + if isGHCLIAvailable() { + return "" // gh answers for itself (a non-default GH_HOST) + } + return findGitHubPAT() +} + // ghAuthToken is the token gh sends to github.com, or "" when gh has none. func ghAuthToken() string { ghTokenOnce.Do(func() { @@ -79,17 +94,10 @@ func ghAPIDirect(path, expr string) (out []byte, ok bool, err error) { if !plainAPIPath.MatchString(path) { return nil, false, nil } - token := ghAuthToken() - if token == "" { - return nil, false, nil - } - req, rerr := http.NewRequest(http.MethodGet, ghAPIBase+path, nil) - if rerr != nil { + if lookupToken() == "" { return nil, false, nil } - req.Header.Set("Authorization", "token "+token) - req.Header.Set("Accept", "application/vnd.github+json") - resp, derr := ghHTTP.Do(req) + resp, derr := githubGet(ghAPIBase + path) if derr != nil { return nil, true, derr } @@ -105,6 +113,158 @@ func ghAPIDirect(path, expr string) (out []byte, ok bool, err error) { return out, true, err } +// ── The request budget ─────────────────────────────────────────────────── +// +// Every GitHub request this package makes goes through githubGet or ghAPICall, +// and both spend from one budget per process (internal/ghquota, the one +// analyze's forge pass uses). Inside GitHub Actions the job token's quota +// belongs to the whole repository: on the push that merged Vulnetix/cli#341, +// the scan jobs' licence lookups took it from 4,913 requests to none in under +// three and a half minutes, so the analyze job's forge pass, and SCA's own +// SARIF upload, found it spent. Licences are the least important use of that +// quota (the Vulnetix store, the registries and deps.dev answer most of +// them), so in Actions the lookups keep ciLicenseShare of the limit for +// everything else. The remaining count is the repository's, the same number +// in every job, so the floor holds for the run however many jobs look up +// licences at once. + +// ciLicenseShare is the fraction of the limit the lookups keep inside GitHub +// Actions: with a 5,000 limit they stop at 4,000 left, above the 2,500 the +// forge pass keeps for CodeQL and the release. +const ciLicenseShare = 0.8 + +const licenseBudgetHint = "Their licences stay UNKNOWN in this run; a later run, or one with a token of its own, resolves them." + +// errGitHubBudget is a request the budget did not make. +var errGitHubBudget = errors.New("GitHub licence lookups stopped at this run's share of the API rate limit") + +var ( + ghBudgetMu sync.Mutex + ghBudget *ghquota.Budget + ghSeedOnce sync.Once + ghNoticeOnce sync.Once + // noticeOut is where the lookups say they stopped; a var for tests. + noticeOut io.Writer = os.Stderr +) + +// githubBudget is this process's licence-lookup budget. +func githubBudget() *ghquota.Budget { + ghBudgetMu.Lock() + defer ghBudgetMu.Unlock() + if ghBudget == nil { + share := ghquota.DefaultShare + if inGitHubActions() { + share = ciLicenseShare + } + ghBudget = ghquota.NewBudget(ghquota.Keep(share), ghquota.Hint(licenseBudgetHint)) + } + return ghBudget +} + +// resetGitHubBudget starts a new budget; for tests. +func resetGitHubBudget() { + ghBudgetMu.Lock() + ghBudget = nil + ghBudgetMu.Unlock() + ghSeedOnce = sync.Once{} + ghNoticeOnce = sync.Once{} +} + +func inGitHubActions() bool { return os.Getenv("GITHUB_ACTIONS") == "true" } + +// seedGitHubBudget asks GitHub for the quota left before the first lookup. +// GET /rate_limit does not count against it, so a run that starts below the +// floor spends nothing at all. +func seedGitHubBudget(token string) { + ghSeedOnce.Do(func() { + req, err := http.NewRequest(http.MethodGet, ghAPIBase+"rate_limit", nil) + if err != nil { + return + } + setGitHubHeaders(req, token) + resp, err := ghHTTP.Do(req) + if err != nil { + return + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return + } + var rl struct { + Resources struct { + Core struct { + Limit int `json:"limit"` + Remaining int `json:"remaining"` + } `json:"core"` + } `json:"resources"` + } + if json.NewDecoder(resp.Body).Decode(&rl) == nil { + githubBudget().Seed(rl.Resources.Core.Limit, rl.Resources.Core.Remaining) + } + }) +} + +// primeGitHubBudget seeds the budget before the first lookup, when there is a +// token to ask with. +func primeGitHubBudget() { + if token := lookupToken(); token != "" { + seedGitHubBudget(token) + } +} + +func setGitHubHeaders(req *http.Request, token string) { + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + if token != "" { + req.Header.Set("Authorization", "token "+token) + } +} + +// githubGet is a GET to the GitHub API with the lookup token, spent from the +// budget. The caller closes the body. It returns errGitHubBudget, without a +// request, once the budget has run out. +func githubGet(u string) (*http.Response, error) { + token := lookupToken() + primeGitHubBudget() + b := githubBudget() + if b.Exhausted() { + return nil, errGitHubBudget + } + req, err := http.NewRequest(http.MethodGet, u, nil) + if err != nil { + return nil, err + } + setGitHubHeaders(req, token) + resp, err := ghHTTP.Do(req) + if err != nil { + return nil, err + } + b.ObserveHTTP(resp) + return resp, nil +} + +// reportGitHubLookups says what a batch of lookups cost. Inside GitHub Actions +// every batch that made a request says so, which is how a run's logs show what +// each job spent; anywhere, a batch the budget cut short says what it left +// unchecked, once per process. +func reportGitHubLookups(repos, spent, notChecked int) { + b := githubBudget() + if inGitHubActions() && spent > 0 { + left := "quota not reported" + if n, known := b.Remaining(); known { + l, _ := b.Limit() + left = fmt.Sprintf("%d of %d left", n, l) + } + fmt.Fprintf(noticeOut, "GitHub licence lookups: %d requests for %d repositories (%s)\n", spent, repos, left) + } + if b.Exhausted() && notChecked > 0 { + ghNoticeOnce.Do(func() { + fmt.Fprintf(noticeOut, "warning: GitHub licence lookups: %d of %d repositories not checked. %s\n", + notChecked, repos, b.Reason()) + }) + } +} + // renderGHJQ is what gh prints for expr over body (see jqLine), each result // on its own line. func renderGHJQ(body []byte, expr string) ([]byte, error) { diff --git a/internal/license/github.go b/internal/license/github.go index 95a9b8d69..a204a7c7b 100644 --- a/internal/license/github.go +++ b/internal/license/github.go @@ -3,14 +3,17 @@ package license import ( "bytes" "encoding/base64" - "encoding/json" + "errors" "fmt" "io" "net/http" "os" "os/exec" + "regexp" + "sort" "strings" "sync" + "sync/atomic" ) // ghCLIAvailable caches whether the `gh` CLI is installed and authenticated. @@ -75,131 +78,118 @@ func resolveOwnerRepo(name, ecosystem string) (string, string) { return "", "" } -// FetchLicenseFromGitHub resolves a license for a GitHub-hosted package. -// Uses a multi-strategy approach: -// 1. GitHub repo API .license.spdx_id field (via gh CLI or REST API) -// 2. Dedicated /repos/{owner}/{repo}/license endpoint -// 3. Fetch LICENSE/COPYING file content and classify text -// 4. Discover license files via directory listing -func FetchLicenseFromGitHub(name, ecosystem string) string { - owner, repo := resolveOwnerRepo(name, ecosystem) - if owner == "" || repo == "" { +// validOwner and validRepo are the names GitHub allows. A name that is not one +// cannot be a repository, and asking GitHub about it costs requests that can +// only 404. A scan of this repository asked about 46 such names on every run +// (`$/sbom` from a workflow's `uses: $/sbom`, a glob, " true -// ghAPI runs `gh api args...` for owner/repo and returns its stdout. +// errNoGitHub is a request that could not be made: no token and no gh. +var errNoGitHub = errors.New("no GitHub credentials for licence lookups") + +// ghAPI runs `gh api args...` for owner/repo and returns its stdout. A 404 is +// returned as ghHTTPError{404}, whichever way the request went. func ghAPI(owner, repo string, args ...string) ([]byte, error) { + out, err := ghAPICall(args...) + if err != nil && !isNotFound(err) { + ghIndefinite.Store(owner+"/"+repo, true) + } + return out, err +} + +func ghAPICall(args ...string) ([]byte, error) { + b := githubBudget() + if b.Exhausted() { + return nil, errGitHubBudget + } if len(args) == 3 && args[1] == "--jq" { if out, ok, err := ghAPIDirect(args[0], args[2]); ok { - if err != nil && !strings.Contains(err.Error(), "HTTP 404") { - ghIndefinite.Store(owner+"/"+repo, true) - } return out, err } } + if !isGHCLIAvailable() { + return nil, errNoGitHub + } cmd := exec.Command("gh", append([]string{"api"}, args...)...) var stderr bytes.Buffer cmd.Stderr = &stderr out, err := cmd.Output() - if err != nil && !strings.Contains(stderr.String(), "HTTP 404") { - ghIndefinite.Store(owner+"/"+repo, true) + b.Spend() + if err != nil { + msg := stderr.String() + switch { + case strings.Contains(msg, "HTTP 404"): + return out, ghHTTPError{http.StatusNotFound} + case strings.Contains(strings.ToLower(msg), "rate limit"): + b.Refused() + } } return out, err } -// resolveRepoLicenseChecked is resolveRepoLicense that also reports whether a -// miss is definitive: every lookup went through gh and each one either answered -// or said 404. A miss through the PAT path, or with gh unavailable, is not. +// isNotFound reports a 404: the thing asked about does not exist for this token. +func isNotFound(err error) bool { + var h ghHTTPError + return errors.As(err, &h) && h.status == http.StatusNotFound +} + +// resolveRepoLicenseChecked is resolveRepoLicense with no store, that also +// reports whether a miss is definitive: every request either answered or said +// 404. func resolveRepoLicenseChecked(owner, repo string) (lic string, definitiveMiss bool) { key := owner + "/" + repo ghIndefinite.Delete(key) @@ -537,5 +423,5 @@ func resolveRepoLicenseChecked(owner, repo string) (lic string, definitiveMiss b return lic, false } _, indefinite := ghIndefinite.Load(key) - return "", isGHCLIAvailable() && findGitHubPAT() == "" && !indefinite + return "", !indefinite } diff --git a/internal/license/github_quota_test.go b/internal/license/github_quota_test.go new file mode 100644 index 000000000..eb2316f91 --- /dev/null +++ b/internal/license/github_quota_test.go @@ -0,0 +1,298 @@ +package license + +import ( + "bytes" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "sync/atomic" + "testing" +) + +// fakeGitHub is the GitHub API for the licence lookups: repos maps +// "owner/repo" to how that repository answers, and every response carries the +// quota as GitHub's headers do, one request less each time. +type fakeGitHub struct { + t *testing.T + repos map[string]fakeRepo + limit int + remaining atomic.Int64 + // rateLimited answers every lookup with GitHub's primary rate-limit 403. + rateLimited bool + + mu sync.Mutex + requests []string // lookups, not /rate_limit +} + +type fakeRepo struct { + spdx string // license.spdx_id on the repository; "" is null + license string // the text /license serves; "" is a 404 + files map[string]string // root files and their text +} + +func (f *fakeGitHub) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "token test-token" { + f.t.Errorf("%s sent without the lookup token", r.URL.Path) + } + path := strings.TrimPrefix(r.URL.Path, "/") + if path == "rate_limit" { + _ = json.NewEncoder(w).Encode(map[string]any{"resources": map[string]any{ + "core": map[string]int{"limit": f.limit, "remaining": int(f.remaining.Load())}}}) + return + } + f.mu.Lock() + f.requests = append(f.requests, path) + f.mu.Unlock() + left := f.remaining.Add(-1) + if f.rateLimited { + left = 0 + } + w.Header().Set("X-RateLimit-Limit", fmt.Sprint(f.limit)) + w.Header().Set("X-RateLimit-Remaining", fmt.Sprint(left)) + if f.rateLimited { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"API rate limit exceeded for installation ID 1."}`)) + return + } + + parts := strings.SplitN(strings.TrimPrefix(path, "repos/"), "/", 3) + repo, ok := f.repos[parts[0]+"/"+parts[1]] + if !ok { + http.NotFound(w, r) + return + } + content := func(text string) map[string]any { + return map[string]any{"content": base64.StdEncoding.EncodeToString([]byte(text))} + } + switch { + case len(parts) == 2: + var lic any + if repo.spdx != "" { + lic = map[string]string{"spdx_id": repo.spdx} + } + _ = json.NewEncoder(w).Encode(map[string]any{"license": lic}) + case parts[2] == "license" && repo.license != "": + body := content(repo.license) + body["license"] = map[string]string{"spdx_id": "NOASSERTION"} + _ = json.NewEncoder(w).Encode(body) + case parts[2] == "contents/": + list := []map[string]string{} + for name := range repo.files { + list = append(list, map[string]string{"name": name}) + } + _ = json.NewEncoder(w).Encode(list) + case strings.HasPrefix(parts[2], "contents/"): + text, ok := repo.files[strings.TrimPrefix(parts[2], "contents/")] + if !ok { + http.NotFound(w, r) + return + } + _ = json.NewEncoder(w).Encode(content(text)) + default: + http.NotFound(w, r) + } +} + +func (f *fakeGitHub) lookups() []string { + f.mu.Lock() + defer f.mu.Unlock() + return append([]string(nil), f.requests...) +} + +// withFakeGitHub points the lookups at f with a token and no gh, as a runner +// without gh would, and a fresh budget, store and notice writer. +func withFakeGitHub(t *testing.T, f *fakeGitHub, actions bool) *bytes.Buffer { + t.Helper() + f.t = t + srv := httptest.NewServer(f) + t.Cleanup(srv.Close) + + oldBase, oldStore, oldOut := ghAPIBase, licenseStoreDir, noticeOut + ghAPIBase = srv.URL + "/" + licenseStoreDir = t.TempDir() + notices := &bytes.Buffer{} + noticeOut = notices + t.Cleanup(func() { ghAPIBase, licenseStoreDir, noticeOut = oldBase, oldStore, oldOut }) + + t.Setenv("PATH", t.TempDir()) // no gh + t.Setenv("GITHUB_TOKEN", "test-token") + t.Setenv("GH_TOKEN", "") + t.Setenv("GITHUB_ACTIONS", map[bool]string{true: "true", false: ""}[actions]) + resetLookupState() + t.Cleanup(resetLookupState) + return notices +} + +func resetLookupState() { + ghCLIOnce = sync.Once{} + ghTokenOnce = sync.Once{} + ghToken = "" + ghIndefinite = sync.Map{} + resetGitHubBudget() +} + +func unknown(names ...string) []PackageLicense { + var out []PackageLicense + for _, n := range names { + out = append(out, PackageLicense{PackageName: n, Ecosystem: "github-actions", LicenseSpdxID: "UNKNOWN"}) + } + return out +} + +const mitLicenseText = `MIT License + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.` + +// A scan of this repository asked GitHub about 46 names that cannot be +// repositories ($/sbom from `uses: $/sbom`, a glob, " 10+7 { + t.Fatalf("made %d requests; the budget should stop at the floor (10 requests away)", got) + } + resolved := 0 + for _, p := range pkgs { + if p.LicenseSpdxID == "MIT" { + resolved++ + } + } + if resolved != got { + t.Errorf("%d requests resolved %d licences", got, resolved) + } + out := notices.String() + if !strings.Contains(out, fmt.Sprintf("GitHub licence lookups: %d requests for 40 repositories", got)) { + t.Errorf("no usage line for the job's log:\n%s", out) + } + if !strings.Contains(out, fmt.Sprintf("%d of 40 repositories not checked", 40-got)) || + !strings.Contains(out, "requests left") { + t.Errorf("the repositories left unchecked were not declared:\n%s", out) + } +} + +// A run that starts with the quota already below the floor spends nothing: +// /rate_limit, which does not count against it, says so first. +func TestLookupsSpendNothingBelowTheFloor(t *testing.T) { + f := &fakeGitHub{limit: 5000, repos: map[string]fakeRepo{"a/b": {spdx: "MIT"}}} + f.remaining.Store(3000) + notices := withFakeGitHub(t, f, true) + + BatchFetchGitHubLicenses(unknown("a/b", "c/d"), nil) + if got := f.lookups(); len(got) != 0 { + t.Fatalf("spent %d requests below the floor: %v", len(got), got) + } + if !strings.Contains(notices.String(), "2 of 2 repositories not checked") { + t.Errorf("notice: %q", notices.String()) + } +} + +// GitHub answering with its rate limit stops every lookup after it, and a miss +// it caused is never stored as the repository having no licence. +func TestRateLimitAnswerStopsLookups(t *testing.T) { + f := &fakeGitHub{limit: 5000, rateLimited: true, repos: map[string]fakeRepo{}} + var names []string + for i := range 30 { + names = append(names, fmt.Sprintf("o/r%d", i)) + } + f.remaining.Store(5000) + notices := withFakeGitHub(t, f, false) + + BatchFetchGitHubLicenses(unknown(names...), nil) + if got := len(f.lookups()); got > 8 { + t.Fatalf("kept asking after GitHub said the quota was spent: %d requests", got) + } + if !strings.Contains(notices.String(), "repositories not checked") { + t.Errorf("notice: %q", notices.String()) + } + if loadStoredMiss("github", "o/r0") { + t.Error("a rate-limited lookup was stored as a definitive miss") + } +} diff --git a/internal/license/registries.go b/internal/license/registries.go index a019cb89b..1d95a46d5 100644 --- a/internal/license/registries.go +++ b/internal/license/registries.go @@ -15,24 +15,6 @@ import ( var registryCache sync.Map // "ecosystem:name:version" → string (license) -// getAuthenticatedGitHubRequest creates an HTTP GET request for the GitHub API, -// adding the Authorization header if a token is available. -func getAuthenticatedGitHubRequest(u string) (*http.Request, error) { - req, err := http.NewRequest("GET", u, nil) - if err != nil { - return nil, err - } - req.Header.Set("Accept", "application/vnd.github+json") - req.Header.Set("X-GitHub-Api-Version", "2022-11-28") - for _, key := range []string{"GITHUB_TOKEN", "GH_TOKEN"} { - if tok := os.Getenv(key); tok != "" { - req.Header.Set("Authorization", "Bearer "+tok) - break - } - } - return req, nil -} - // FetchFromEcosystemRegistry attempts to resolve a license from the package's // native ecosystem registry for ecosystems not covered by deps.dev. func FetchFromEcosystemRegistry(pkg PackageLicense) string { @@ -589,14 +571,10 @@ func fetchCocoaPodsLicense(name string) string { // from the CocoaPods/Specs GitHub directory listing and fetches its spec. func fetchCocoaPodsSpecLatestFromGitHub(name string) string { hash := fmt.Sprintf("%x", md5.Sum([]byte(name))) - dirURL := fmt.Sprintf("https://api.github.com/repos/CocoaPods/Specs/contents/Specs/%s/%s/%s/%s", + dirURL := fmt.Sprintf(ghAPIBase+"repos/CocoaPods/Specs/contents/Specs/%s/%s/%s/%s", string(hash[0]), string(hash[1]), string(hash[2]), url.PathEscape(name)) - req, err := getAuthenticatedGitHubRequest(dirURL) - if err != nil { - return "" - } - resp, err := getHTTPClient().Do(req) + resp, err := githubGet(dirURL) if err != nil || resp.StatusCode != http.StatusOK { if resp != nil { resp.Body.Close() @@ -892,20 +870,10 @@ func fetchOpamLicense(name, version string) string { // an opam package and fetches the latest opam file. func fetchOpamLicenseViaGitHubSearch(name string) string { // List subdirectories of packages/{name} to find all available versions. - apiURL := fmt.Sprintf( - "https://api.github.com/repos/ocaml/opam-repository/contents/packages/%s", + apiURL := fmt.Sprintf(ghAPIBase+"repos/ocaml/opam-repository/contents/packages/%s", url.PathEscape(name)) - req, err := http.NewRequest("GET", apiURL, nil) - if err != nil { - return "" - } - req.Header.Set("Accept", "application/vnd.github+json") - if pat := findGitHubPAT(); pat != "" { - req.Header.Set("Authorization", "token "+pat) - } - - resp, err := getHTTPClient().Do(req) + resp, err := githubGet(apiURL) if err != nil || resp.StatusCode != http.StatusOK { if resp != nil { resp.Body.Close()