From 79d523efa86dfcfa04528d3b4f291579593d7793 Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Wed, 3 Jun 2026 07:44:36 +0900 Subject: [PATCH 1/2] Warn early on repos whose plan does not support branch rules setup and status crash with a raw GitHub 403 ('Upgrade to GitHub Pro or make this repository public') when run against a private repo on a free plan. The 403 comes from reading branch rulesets and is surfaced as an opaque wrapped error. Detect that 403 with a typed check (errors.As to *github.ErrorResponse, StatusCode 403) rather than string matching, convert it to a new ErrUnsupportedRepoPlan sentinel, and read the controls up front in the onboard flow so the warning is printed before any mutation. setup and status now print an actionable message telling the user to make the repo public or upgrade the plan. Fixes #326 Signed-off-by: Arpit Jain --- internal/cmd/setup.go | 7 ++ internal/cmd/status.go | 19 ++++ pkg/sourcetool/backends/vcs/github/github.go | 31 +++++++ .../backends/vcs/github/github_test.go | 86 +++++++++++++++++++ pkg/sourcetool/models/models.go | 6 ++ pkg/sourcetool/tool.go | 10 +++ pkg/sourcetool/tool_test.go | 29 +++++++ 7 files changed, 188 insertions(+) create mode 100644 pkg/sourcetool/backends/vcs/github/github_test.go diff --git a/internal/cmd/setup.go b/internal/cmd/setup.go index f966c6a1..3dcd0c7a 100644 --- a/internal/cmd/setup.go +++ b/internal/cmd/setup.go @@ -220,6 +220,9 @@ sourcetool is about to perform the following actions on your behalf: cmd.Context(), opts.GetBranch().Repository, []*models.Branch{opts.GetBranch()}, ) if err != nil { + if errors.Is(err, models.ErrUnsupportedRepoPlan) { + return unsupportedRepoPlanError(opts.GetRepository().Path) + } return fmt.Errorf("onboarding repo: %w", err) } @@ -446,6 +449,10 @@ a fork of the repository you want to protect. cmd.Context(), opts.GetBranch().Repository, []*models.Branch{opts.GetBranch()}, cs, ) if err != nil { + if errors.Is(err, models.ErrUnsupportedRepoPlan) { + return unsupportedRepoPlanError(opts.GetRepository().Path) + } + // if strings.Contains(err.Error(), models.ErrProtectionAlreadyInPlace.Error()) { if errors.Is(err, models.ErrProtectionAlreadyInPlace) { fmt.Printf("\n ℹ️ Controls already enabled on %s\n\n", opts.GetRepository().Path) diff --git a/internal/cmd/status.go b/internal/cmd/status.go index ff080ae1..62992634 100644 --- a/internal/cmd/status.go +++ b/internal/cmd/status.go @@ -16,8 +16,24 @@ import ( "github.com/slsa-framework/source-tool/pkg/policy" "github.com/slsa-framework/source-tool/pkg/slsa" "github.com/slsa-framework/source-tool/pkg/sourcetool" + "github.com/slsa-framework/source-tool/pkg/sourcetool/models" ) +// unsupportedRepoPlanError builds a user facing error explaining that the +// repository plan does not support reading branch rules. This is the friendly +// message the setup and status subcommands print instead of the raw GitHub 403 +// "Upgrade to GitHub Pro or make this repository public" response. +func unsupportedRepoPlanError(repoPath string) error { + return fmt.Errorf( + "%s is a private repository on a GitHub plan that does not expose branch "+ + "rules to the API.\n"+ + "sourcetool cannot read or configure SLSA source controls on it yet.\n\n"+ + "To continue, either make the repository public or upgrade its account "+ + "to a plan that includes repository rules (GitHub Pro or higher).", + repoPath, + ) +} + var ( w = color.New(color.FgHiWhite, color.BgBlack).SprintFunc() w2 = color.New(color.Faint, color.FgWhite, color.BgBlack).SprintFunc() @@ -112,6 +128,9 @@ sourcetool status myorg/myrepo@mybranch // Get the active repository controls controls, err := srctool.GetBranchControls(cmd.Context(), opts.GetBranch()) if err != nil { + if errors.Is(err, models.ErrUnsupportedRepoPlan) { + return unsupportedRepoPlanError(opts.GetRepository().Path) + } return fmt.Errorf("fetching active controls: %w", err) } diff --git a/pkg/sourcetool/backends/vcs/github/github.go b/pkg/sourcetool/backends/vcs/github/github.go index 215cc8ce..d43405b5 100644 --- a/pkg/sourcetool/backends/vcs/github/github.go +++ b/pkg/sourcetool/backends/vcs/github/github.go @@ -8,8 +8,11 @@ import ( "errors" "fmt" "log" + "net/http" "time" + "github.com/google/go-github/v69/github" + "github.com/slsa-framework/source-tool/pkg/attest" "github.com/slsa-framework/source-tool/pkg/auth" "github.com/slsa-framework/source-tool/pkg/ghcontrol" @@ -17,6 +20,28 @@ import ( "github.com/slsa-framework/source-tool/pkg/sourcetool/models" ) +// asUnsupportedPlanError inspects an error returned from the GitHub API and, +// when it is a 403 raised because the repository's plan does not include the +// requested feature (for example reading branch rulesets on a private repo on a +// free plan), returns models.ErrUnsupportedRepoPlan wrapping the original +// error. It returns nil if err is not that case so callers can fall through to +// their normal handling. The check is done on the typed *github.ErrorResponse +// rather than on the error string so it does not break if GitHub rewords the +// message. +func asUnsupportedPlanError(err error) error { + if err == nil { + return nil + } + var ghErr *github.ErrorResponse + if !errors.As(err, &ghErr) { + return nil + } + if ghErr.Response == nil || ghErr.Response.StatusCode != http.StatusForbidden { + return nil + } + return fmt.Errorf("%w: %w", models.ErrUnsupportedRepoPlan, err) +} + // InherentControls are the controls that are always true because we are // in git and/org GitHub. var InherentControls = slsa.ControlNameSet{ @@ -146,6 +171,12 @@ func (b *Backend) GetBranchControlsAtCommit(ctx context.Context, branch *models. // legacy checks sourcetool did (continuity, review, RequiredChecks, tag hygiene) activeControls, err := ghc.GetBranchControls(ctx, branch.FullRef()) if err != nil { + // Reading branch rules 403s on private repos that are on a free plan. + // Surface a typed, actionable error before anything else happens so the + // caller can warn the user instead of leaking the raw API message. + if planErr := asUnsupportedPlanError(err); planErr != nil { + return nil, planErr + } return nil, fmt.Errorf("checking status: %w", err) } diff --git a/pkg/sourcetool/backends/vcs/github/github_test.go b/pkg/sourcetool/backends/vcs/github/github_test.go new file mode 100644 index 00000000..d09cdc85 --- /dev/null +++ b/pkg/sourcetool/backends/vcs/github/github_test.go @@ -0,0 +1,86 @@ +// SPDX-FileCopyrightText: Copyright 2025 The SLSA Authors +// SPDX-License-Identifier: Apache-2.0 + +package github + +import ( + "errors" + "fmt" + "net/http" + "testing" + + "github.com/google/go-github/v69/github" + "github.com/stretchr/testify/require" + + "github.com/slsa-framework/source-tool/pkg/sourcetool/models" +) + +func TestAsUnsupportedPlanError(t *testing.T) { + t.Parallel() + + // This is the shape GitHub returns when reading branch rules on a private + // repo that is on a free plan (see slsa-framework/source-tool#326). The + // detection must key off the typed response and status code, not the + // message text, so the test uses the real go-github error type. + forbidden := &github.ErrorResponse{ + Response: &http.Response{StatusCode: http.StatusForbidden}, + Message: "Upgrade to GitHub Pro or make this repository public to enable this feature.", + } + + for _, tc := range []struct { + name string + err error + expectPlan bool + }{ + { + name: "nil", + err: nil, + expectPlan: false, + }, + { + name: "plain-403", + err: forbidden, + expectPlan: true, + }, + { + name: "wrapped-403", + err: fmt.Errorf("checking status: %w", forbidden), + expectPlan: true, + }, + { + name: "404-not-plan", + err: &github.ErrorResponse{ + Response: &http.Response{StatusCode: http.StatusNotFound}, + Message: "Not Found", + }, + expectPlan: false, + }, + { + name: "non-github-error", + err: errors.New("some other failure"), + expectPlan: false, + }, + { + name: "403-without-response", + err: &github.ErrorResponse{ + Message: "Forbidden but no response attached", + }, + expectPlan: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := asUnsupportedPlanError(tc.err) + if !tc.expectPlan { + require.Nil(t, got) + return + } + require.Error(t, got) + // The actionable sentinel must be detectable with errors.Is so the + // CLI can switch on it... + require.ErrorIs(t, got, models.ErrUnsupportedRepoPlan) + // ...and the original API error must still be reachable for debugging. + require.ErrorIs(t, got, tc.err) + }) + } +} diff --git a/pkg/sourcetool/models/models.go b/pkg/sourcetool/models/models.go index 06b92468..de496ce5 100644 --- a/pkg/sourcetool/models/models.go +++ b/pkg/sourcetool/models/models.go @@ -21,6 +21,12 @@ import ( var ( ErrProtectionAlreadyInPlace = errors.New("controls already in place in the repository") ErrRepositoryAccessDenied = errors.New("access to repository denied") + // ErrUnsupportedRepoPlan is returned when a control check or configuration + // hits a GitHub feature that the repository's plan does not include. This + // happens, for example, when reading branch rulesets on a private repo that + // is on a free plan: GitHub answers with a 403 asking to upgrade or make the + // repo public. + ErrUnsupportedRepoPlan = errors.New("repository plan does not support this feature (private repositories require GitHub Pro or a public repository to read branch rules)") ) // AttestationStorageReader abstracts an attestation storage system where diff --git a/pkg/sourcetool/tool.go b/pkg/sourcetool/tool.go index 039b12c3..3cf7e6dc 100644 --- a/pkg/sourcetool/tool.go +++ b/pkg/sourcetool/tool.go @@ -131,6 +131,16 @@ func (t *Tool) OnboardRepository(ctx context.Context, repo *models.Repository, b return fmt.Errorf("verifying options: %w", err) } + // Read the current controls before changing anything. This is a read-only + // call that fails fast with models.ErrUnsupportedRepoPlan when the repo is + // private on a free plan, letting us warn the user before we mutate the + // repository. + for _, branch := range branches { + if _, err := t.impl.GetBranchControls(ctx, t.backend, branch); err != nil { + return fmt.Errorf("checking repository controls: %w", err) + } + } + if err := t.backend.ConfigureControls( repo, branches, []models.ControlConfiguration{ models.CONFIG_BRANCH_RULES, models.CONFIG_GEN_PROVENANCE, models.CONFIG_TAG_RULES, diff --git a/pkg/sourcetool/tool_test.go b/pkg/sourcetool/tool_test.go index ee73c22f..f224444e 100644 --- a/pkg/sourcetool/tool_test.go +++ b/pkg/sourcetool/tool_test.go @@ -344,3 +344,32 @@ func TestOnboardRepository(t *testing.T) { }) } } + +// TestOnboardRepositoryUnsupportedPlanWarnsBeforeMutation reproduces +// slsa-framework/source-tool#326: when the repository plan does not support +// reading branch rules (a private repo on a free plan returns a 403), the +// onboard flow must surface models.ErrUnsupportedRepoPlan and must NOT perform +// any mutating action (it must not call the backend ConfigureControls). +func TestOnboardRepositoryUnsupportedPlanWarnsBeforeMutation(t *testing.T) { + t.Parallel() + + timp := &sourcetoolfakes.FakeToolImplementation{} + timp.VerifyOptionsForFullOnboardReturns(nil) + // The pre-mutation read trips the plan limitation. + timp.GetBranchControlsReturns(nil, models.ErrUnsupportedRepoPlan) + + bend := &modelsfakes.FakeVcsBackend{} + + tool := &Tool{impl: timp, backend: bend} + + err := tool.OnboardRepository( + t.Context(), &models.Repository{Path: "example/repo"}, []*models.Branch{{Name: "main"}}, + ) + + require.Error(t, err) + require.ErrorIs(t, err, models.ErrUnsupportedRepoPlan) + + // The mutation must never run: ConfigureControls is the destructive call and + // it has to stay at zero invocations. + require.Equal(t, 0, bend.ConfigureControlsCallCount(), "must not mutate the repository after an unsupported-plan warning") +} From d1600440b5e1cd1628f4ddb8a4102183df74332a Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Fri, 5 Jun 2026 10:07:52 +0900 Subject: [PATCH 2/2] Fix golangci-lint nits in unsupported-plan warning Address the two lint findings flagged on this branch: - status.go: drop the trailing period from the unsupportedRepoPlanError message so it satisfies staticcheck ST1005 (error strings should not end with punctuation). - github_test.go: use require.NoError instead of require.Nil for the nil-error assertion, per testifylint error-nil. Signed-off-by: Arpit Jain --- internal/cmd/status.go | 2 +- pkg/sourcetool/backends/vcs/github/github_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/cmd/status.go b/internal/cmd/status.go index 62992634..93119397 100644 --- a/internal/cmd/status.go +++ b/internal/cmd/status.go @@ -29,7 +29,7 @@ func unsupportedRepoPlanError(repoPath string) error { "rules to the API.\n"+ "sourcetool cannot read or configure SLSA source controls on it yet.\n\n"+ "To continue, either make the repository public or upgrade its account "+ - "to a plan that includes repository rules (GitHub Pro or higher).", + "to a plan that includes repository rules (GitHub Pro or higher)", repoPath, ) } diff --git a/pkg/sourcetool/backends/vcs/github/github_test.go b/pkg/sourcetool/backends/vcs/github/github_test.go index d09cdc85..8c0b0a7c 100644 --- a/pkg/sourcetool/backends/vcs/github/github_test.go +++ b/pkg/sourcetool/backends/vcs/github/github_test.go @@ -72,7 +72,7 @@ func TestAsUnsupportedPlanError(t *testing.T) { t.Parallel() got := asUnsupportedPlanError(tc.err) if !tc.expectPlan { - require.Nil(t, got) + require.NoError(t, got) return } require.Error(t, got)