diff --git a/.github/workflows/vulnetix.yml b/.github/workflows/vulnetix.yml index eee5bb69e6..5f649483d3 100644 --- a/.github/workflows/vulnetix.yml +++ b/.github/workflows/vulnetix.yml @@ -214,7 +214,7 @@ jobs: artifact-name: analyze fail-on-findings: false github-token: ${{ github.event_name != 'pull_request' && github.token || '' }} - extra-args: ${{ github.event_name == 'pull_request' && '--no-forge' || '' }} + extra-args: ${{ github.event_name == 'pull_request' && '--no-forge' || github.event_name != 'workflow_dispatch' && '--forge-window-days=7' || '' }} sbom: name: SBOM diff --git a/cmd/analyze.go b/cmd/analyze.go index 2cbd40a3b2..9248e8b938 100644 --- a/cmd/analyze.go +++ b/cmd/analyze.go @@ -66,6 +66,11 @@ GitHub authentication than zero, because "we found no unreviewed commits" and "we did not look" are different claims and this report will not conflate them. + The token's hourly quota is usually shared (in Actions, with every other job in the + repository), so the GitHub pass stops while half the quota is still left and reports + what it did not fetch. --forge-window-days reads a shorter window from GitHub than + --window-days reads from git, which costs fewer requests. + A repository whose remote is not GitHub skips the check automatically.`, Args: cobra.MaximumNArgs(1), RunE: runAnalyze, @@ -74,6 +79,7 @@ GitHub authentication func init() { analyzeCmd.Flags().String("path", ".", "Directory to analyze") analyzeCmd.Flags().Int("window-days", 365, "How far back to walk history") + analyzeCmd.Flags().Int("forge-window-days", 0, "How far back to read pull requests, issues and commit reviews from GitHub (0: --window-days). Each pull request costs several API calls") analyzeCmd.Flags().Int("max-commits", 20000, "Cap on commits walked; when hit, the report declares it") analyzeCmd.Flags().Int("complexity-threshold", 15, "Cyclomatic complexity at which a file counts as highly complex") analyzeCmd.Flags().StringP("output", "o", "pretty", "Terminal output format: pretty, json") @@ -98,6 +104,7 @@ func runAnalyze(cmd *cobra.Command, args []string) error { opts.Path = args[0] } opts.WindowDays, _ = cmd.Flags().GetInt("window-days") + opts.ForgeWindowDays, _ = cmd.Flags().GetInt("forge-window-days") opts.MaxCommits, _ = cmd.Flags().GetInt("max-commits") opts.ComplexityThreshold, _ = cmd.Flags().GetInt("complexity-threshold") opts.NoGit, _ = cmd.Flags().GetBool("no-git") diff --git a/docs/command-manifest.json b/docs/command-manifest.json index 01157a9faa..c61bf224a7 100644 --- a/docs/command-manifest.json +++ b/docs/command-manifest.json @@ -525,6 +525,7 @@ "complexity-threshold", "disable-memory", "fail-on-upload-error", + "forge-window-days", "max-commits", "no-analytics", "no-banner", diff --git a/docs/github-actions.md b/docs/github-actions.md index 1891f556a2..255b6c2ee6 100644 --- a/docs/github-actions.md +++ b/docs/github-actions.md @@ -66,6 +66,15 @@ Each CLI command also has its own composite action in a subdirectory of this rep `config-plan`, `config-apply`, `attest`, `jail`, `vex` and `github-publish`. Their inputs, outputs, permissions and examples are in [`github-actions/`](github-actions/README.md). +`analyze` reads pull requests, issues and commit reviews from GitHub with the job token, several +requests per pull request. That token's hourly quota (1,000 requests) belongs to the whole +repository, and every other job shares it, including CodeQL's SARIF upload and a workflow +dispatch. So the GitHub pass stops while half the token's hourly limit is still left, and the +report declares what it did not fetch. The workflow `vulnetix gha init` writes passes +`--no-forge` on a pull request and `--forge-window-days=7` on a push or a release. Only a manual +run (`workflow_dispatch`) reads the full `--window-days` window from GitHub. The git history +window stays at 365 days in every case. + 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/collect_forge.go b/internal/analyze/collect_forge.go index 9ff312ba96..f902a4d40a 100644 --- a/internal/analyze/collect_forge.go +++ b/internal/analyze/collect_forge.go @@ -38,11 +38,12 @@ const maxCommitReviewChecks = 300 func collectForge(b *Builder, client *forge.Client, repo forge.Repo, git *gitStats, opts Options, now time.Time, pr reporter) error { ctx := context.Background() - since := now.AddDate(0, 0, -opts.WindowDays) + days := opts.forgeWindowDays() + since := now.AddDate(0, 0, -days) window := &MetricWindow{ Since: since.UTC().Format(time.RFC3339), Until: now.UTC().Format(time.RFC3339), - Label: fmt.Sprintf("last_%d_days", opts.WindowDays), + Label: fmt.Sprintf("last_%d_days", days), } // This is the slowest collector by a distance: every pull request costs several API calls @@ -67,7 +68,7 @@ func collectForge(b *Builder, client *forge.Client, repo forge.Repo, git *gitSta emitIssues(b, issues, window) pr.Stage("Resolving which commits were reviewed") - emitCommitReview(b, client, repo, git, prRefs, window) + emitCommitReview(b, client, repo, git, prRefs, since, window) // If the budget ran out, every count above is short by an unknown amount — and an unknown // amount is exactly what must not be silently absorbed. Say it. @@ -396,9 +397,28 @@ func emitIssues(b *Builder, issues []forge.Issue, window *MetricWindow) { window, respSecs, respRefs) } +// commitsToReview picks the commits the compliance pass asks the forge about: non-merge +// commits inside the forge window, newest first. A merge commit's review status is the review +// status of the PR it merged, which we already have. The forge window can be shorter than the +// history walk, and every commit asked about costs an API call, so a commit older than the +// window is not asked about. +func commitsToReview(commits []*CommitRecord, since time.Time) []string { + shas := make([]string, 0, len(commits)) + for _, c := range commits { + if c.ParentCount > 1 { + continue + } + if at, err := time.Parse(time.RFC3339, c.CommittedAt); err == nil && at.Before(since) { + continue + } + shas = append(shas, c.SHA) + } + return shas +} + // emitCommitReview is github-metrics-aggregator's compliance metric. func emitCommitReview(b *Builder, client *forge.Client, repo forge.Repo, git *gitStats, - prRefs map[string]EvidenceRef, window *MetricWindow) { + prRefs map[string]EvidenceRef, since time.Time, window *MetricWindow) { if git == nil || len(git.commits) == 0 { b.Unmeasured(Metric{ @@ -410,16 +430,8 @@ func emitCommitReview(b *Builder, client *forge.Client, repo forge.Repo, git *gi return } - // Only non-merge commits, newest first, capped. A merge commit's review status is the - // review status of the PR it merged, which we already have. - shas := make([]string, 0, len(git.commits)) + shas := commitsToReview(git.commits, since) byShaRef := map[string]EvidenceRef{} - for _, c := range git.commits { - if c.ParentCount > 1 { - continue - } - shas = append(shas, c.SHA) - } omitted := 0 if len(shas) > maxCommitReviewChecks { diff --git a/internal/analyze/collect_forge_window_test.go b/internal/analyze/collect_forge_window_test.go new file mode 100644 index 0000000000..93e9215696 --- /dev/null +++ b/internal/analyze/collect_forge_window_test.go @@ -0,0 +1,34 @@ +package analyze + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// The forge window can be shorter than the history walk. The compliance pass costs an API +// call a commit, so it asks only about the commits inside the forge window, and never about +// a merge commit, whose review status is its pull request's. +func TestCommitsToReview_StaysInsideTheForgeWindow(t *testing.T) { + now := time.Date(2026, 11, 14, 0, 0, 0, 0, time.UTC) + at := func(days int) string { return now.AddDate(0, 0, -days).Format(time.RFC3339) } + commits := []*CommitRecord{ + {SHA: "new", CommittedAt: at(1)}, + {SHA: "merge", CommittedAt: at(2), ParentCount: 2}, + {SHA: "edge", CommittedAt: at(7)}, + {SHA: "old", CommittedAt: at(30)}, + {SHA: "undated"}, + } + + require.Equal(t, []string{"new", "edge", "undated"}, commitsToReview(commits, now.AddDate(0, 0, -7)), + "a commit with no parseable date is kept rather than dropped unseen") + require.Equal(t, []string{"new", "edge", "old", "undated"}, commitsToReview(commits, now.AddDate(0, 0, -365))) +} + +func TestForgeWindowDays_FollowsTheHistoryWindowUnlessSet(t *testing.T) { + o := DefaultOptions() + require.Equal(t, 365, o.forgeWindowDays()) + o.ForgeWindowDays = 7 + require.Equal(t, 7, o.forgeWindowDays()) +} diff --git a/internal/analyze/forge/budget.go b/internal/analyze/forge/budget.go index 518edceca5..2b070525aa 100644 --- a/internal/analyze/forge/budget.go +++ b/internal/analyze/forge/budget.go @@ -18,16 +18,31 @@ package forge // reason. It is not a failure, and it is not a silence. import ( + "fmt" "sync" "github.com/google/go-github/v66/github" ) -// reserve is the 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. +// 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 +// 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 @@ -60,7 +75,7 @@ func (b *budget) Observe(resp *github.Response) { b.remaining = resp.Rate.Remaining b.limit = resp.Rate.Limit b.known = true - if b.remaining <= reserve { + if b.remaining <= floor(b.limit) { b.exhausted = true } } @@ -102,8 +117,9 @@ func (b *budget) Reason() string { return "" } - return "GitHub API rate limit reached; the remaining items were not fetched. " + - "Re-run when the quota resets, or narrow --window-days." + 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 diff --git a/internal/analyze/forge/forge_test.go b/internal/analyze/forge/forge_test.go index b69f24942c..92c7cf858e 100644 --- a/internal/analyze/forge/forge_test.go +++ b/internal/analyze/forge/forge_test.go @@ -2,6 +2,7 @@ package forge import ( "context" + "fmt" "os" "testing" "time" @@ -128,6 +129,43 @@ func TestBudget(t *testing.T) { require.Equal(t, 2, b.Spent()) } +// Inside GitHub Actions the job token's 1,000 requests an hour belong to the whole repository. +// A forge pass that spent them down to a fixed 100 starved CodeQL's SARIF upload and the +// release dispatch on the same push, so the collector keeps half the limit for them. +func TestBudget_KeepsHalfTheLimitForOtherJobs(t *testing.T) { + cases := []struct { + limit, remaining int + exhausted bool + }{ + {1000, 501, false}, + {1000, 500, true}, + {1000, 300, true}, // already below the floor when we start: spend nothing more + {5000, 2501, false}, + {5000, 2500, true}, + {150, 101, false}, // a small limit still keeps at least the fixed reserve + {150, 100, true}, + } + for _, c := range cases { + b := newBudget() + b.Observe(&github.Response{Rate: github.Rate{Limit: c.limit, Remaining: c.remaining}}) + require.Equal(t, c.exhausted, b.Exhausted(), "limit %d, remaining %d", c.limit, c.remaining) + if c.exhausted { + require.Contains(t, b.Reason(), fmt.Sprintf("%d of %d requests left", c.remaining, c.limit)) + } + } +} + +// A response that carries no rate headers (some enterprise instances) cannot exhaust the +// budget on its own; it is still counted. +func TestBudget_UnknownLimit(t *testing.T) { + b := newBudget() + b.Observe(&github.Response{}) + require.False(t, b.Exhausted()) + _, known := b.Remaining() + require.False(t, known) + require.Equal(t, 1, b.Spent()) +} + func TestIsRateLimit(t *testing.T) { require.True(t, IsRateLimit(&github.RateLimitError{})) require.True(t, IsRateLimit(&github.AbuseRateLimitError{})) diff --git a/internal/analyze/run.go b/internal/analyze/run.go index 17a4fcdc68..0e3da9a103 100644 --- a/internal/analyze/run.go +++ b/internal/analyze/run.go @@ -31,6 +31,12 @@ type Options struct { WindowDays int MaxCommits int + // ForgeWindowDays bounds the forge pass (pull requests, issues, commit reviews) on its + // own. Every pull request in the window costs several GitHub API calls, so a CI run on + // every push can read a short recent window and leave the full one to a release or a + // manual run. Zero follows WindowDays. + ForgeWindowDays int + ComplexityThreshold int NoGit bool @@ -64,6 +70,15 @@ type Options struct { Context context.Context } +// forgeWindowDays is the window the forge pass reads: ForgeWindowDays when set, else the +// history window. +func (o Options) forgeWindowDays() int { + if o.ForgeWindowDays > 0 { + return o.ForgeWindowDays + } + return o.WindowDays +} + func (o Options) ctx() context.Context { if o.Context != nil { return o.Context diff --git a/internal/ghainit/render.go b/internal/ghainit/render.go index 517a9ac5aa..76ff023406 100644 --- a/internal/ghainit/render.go +++ b/internal/ghainit/render.go @@ -116,12 +116,7 @@ func Render(p *Policy, sig *Signals, opt Options) string { case "analyze": // it reads history and the repository's pull requests and issues j.fullHistory = true j.perms = map[string]string{"pull-requests": "read", "issues": "read"} - // The forge pass reads every pull request and issue in the window and - // spends the job token down to its reserve. That token shares the - // repository's hourly quota with every other job, CodeQL's SARIF - // upload among them, so on a pull request it would fail the run's - // required checks. The push to the default branch measures the forge. - j.steps[0].with = append(j.steps[0].with, [2]string{"extra-args", "${{ github.event_name == 'pull_request' && '--no-forge' || '' }}"}) + j.steps[0].with = append(j.steps[0].with, [2]string{"extra-args", analyzeForgeArgs}) case "aibom": // a commit scan is slow and this runs on every push j.steps[0].with = append(j.steps[0].with, [2]string{"extra-args", "--commit-scan-max\n1"}) } @@ -214,6 +209,19 @@ func (r *renderer) scanStep(action, gate string, with [][2]string, artifact stri // GitHub from these jobs; the push to the default branch keeps the token. const scanToken = "${{ github.event_name != 'pull_request' && github.token || '' }}" +// analyzeForgeArgs bounds the analyze job's forge pass, which reads pull requests, +// issues and commit reviews from GitHub with the job token, several requests a pull +// request. That token's hourly quota belongs to the whole repository and is shared with +// every other job, CodeQL's SARIF upload and the release dispatch among them. A pull +// request skips the pass. A push to the default branch, which happens on every merge, and +// a release, which an auto-versioned repository publishes on every merge too, read only +// the last recentForgeWindowDays days. A manual run (workflow_dispatch) reads the full +// window. The CLI also stops the pass while half the quota is left. +var analyzeForgeArgs = fmt.Sprintf("${{ github.event_name == 'pull_request' && '--no-forge' || github.event_name != 'workflow_dispatch' && '--forge-window-days=%d' || '' }}", recentForgeWindowDays) + +// recentForgeWindowDays is the forge window on a push or a release. +const recentForgeWindowDays = 7 + func (r *renderer) scanJob(id, name, action, gate string, with [][2]string, _ string) job { j := job{id: id, name: name, steps: []step{r.scanStep(action, gate, with, id)}} if r.p.Publish { diff --git a/internal/ghainit/render_test.go b/internal/ghainit/render_test.go index 7c0fa8a023..4d95b92fa3 100644 --- a/internal/ghainit/render_test.go +++ b/internal/ghainit/render_test.go @@ -99,8 +99,9 @@ func TestPublishAndTEAFollowTheLegs(t *testing.T) { } // The forge pass spends the repository's shared hourly token quota, so a pull -// request runs analyze without it and the push to the default branch keeps it. -func TestAnalyzeSkipsTheForgeOnPullRequests(t *testing.T) { +// request runs analyze without it, a push or a release reads a short recent +// window, and only a manual run reads the full one. +func TestAnalyzeBoundsTheForgePass(t *testing.T) { p := DefaultPolicy() p.Analyze = true out := Render(p, &Signals{}, Options{}) @@ -119,7 +120,7 @@ func TestAnalyzeSkipsTheForgeOnPullRequests(t *testing.T) { if !strings.HasPrefix(s.Uses, "Vulnetix/cli/analyze@") { continue } - want := "${{ github.event_name == 'pull_request' && '--no-forge' || '' }}" + want := "${{ github.event_name == 'pull_request' && '--no-forge' || github.event_name != 'workflow_dispatch' && '--forge-window-days=7' || '' }}" if got := s.With["extra-args"]; got != want { t.Errorf("analyze extra-args = %q, want %q", got, want) }