Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixed-dependabot-alerts-permission.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- The sec-check agent's Dependabot alerts call no longer 403s once the GitHub App is granted the new read-only Dependabot alerts permission (documented in the GitHub App setup guide); existing installations must re-accept it once. Only the sec-check role's token requests the grant — other agents' tokens are unchanged.
21 changes: 18 additions & 3 deletions src/crux/agentmode/agentmode.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,23 @@ func TokenTierForRole(m AgentMode, role string) string {
if m == ModeAdvisory && role == ReviewerRole {
return "reviewer"
}
if m == ModeIssuesAndPRs && role == SecCheckRole {
return SecCheckTier
}
return m.TokenTier()
}

// SecCheckRole is the agent role whose ISSUES_AND_PRS token additionally
// carries Dependabot alerts read (#17426). Like the reviewer refinement it
// is a role, not a rung on the mode ladder: a sec-check agent mints the
// SecCheckTier (the contributor set plus vulnerability_alerts:read) so that
// no other contributor-tier agent's token is widened by the grant.
const SecCheckRole = "sec-check"

// SecCheckTier is the scoped-token tier TokenTierForRole yields for an
// ISSUES_AND_PRS agent with SecCheckRole.
const SecCheckTier = "sec-check"

// CanComment reports whether an agent at this mode may post a comment on an
// issue. It sits at the ISSUES_ONLY rung: the newcomer token tier is the first
// with Issues:write, and ADVISORY (advisor, and the reviewer refinement whose
Expand All @@ -137,8 +151,9 @@ func TokenTierForRole(m AgentMode, role string) string {
func (m AgentMode) CanComment() bool { return m >= ModeIssuesOnly }

// ModeForTokenTier is the inverse of TokenTier / TokenTierForRole: the mode a
// scoped-token tier corresponds to. The reviewer tier maps to ADVISORY (it is
// a role refinement of that mode, see TokenTierForRole); "merger" is the
// scoped-token tier corresponds to. The reviewer tier maps to ADVISORY and the
// sec-check tier to ISSUES_AND_PRS (both are role refinements of their mode,
// see TokenTierForRole); "merger" is the
// contribute queue's name for the tier above trusted and shares its mode. An
// unknown tier reports ADVISORY and false, so a caller that gates a write on
// the result fails closed.
Expand All @@ -148,7 +163,7 @@ func ModeForTokenTier(tier string) (AgentMode, bool) {
return ModeAdvisory, true
case "newcomer":
return ModeIssuesOnly, true
case "contributor":
case "contributor", SecCheckTier:
return ModeIssuesAndPRs, true
case "trusted", "merger":
return ModeIssuesPRsMerge, true
Expand Down
30 changes: 30 additions & 0 deletions src/crux/agentmode/reviewer_tier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,33 @@ func TestTokenTierForRole_ReviewerGainsNoModeCapability(t *testing.T) {
t.Error("ADVISORY reports CanMerge — the reviewer's safety asymmetry depends on this staying false")
}
}

// The sec-check role refines ISSUES_AND_PRS into a tier whose App token adds
// Dependabot alerts read (#17426). It is scoped to that one role and mode:
// every other contributor-tier agent, and a sec-check agent at any other mode,
// keeps the plain mode tier.
func TestTokenTierForRole_SecCheckGetsAlertsCapableTier(t *testing.T) {
if got := TokenTierForRole(ModeIssuesAndPRs, SecCheckRole); got != SecCheckTier {
t.Errorf("ISSUES_AND_PRS sec-check minted %q, want %q", got, SecCheckTier)
}
for _, role := range []string{"", "scanner", ReviewerRole} {
if got := TokenTierForRole(ModeIssuesAndPRs, role); got != "contributor" {
t.Errorf("ISSUES_AND_PRS %q minted %q, want contributor — only the sec-check role is refined", role, got)
}
}
for _, tc := range []struct {
mode AgentMode
want string
}{
{ModeAdvisory, "advisor"},
{ModeIssuesOnly, "newcomer"},
{ModeIssuesPRsMerge, "trusted"},
} {
if got := TokenTierForRole(tc.mode, SecCheckRole); got != tc.want {
t.Errorf("%v sec-check minted %q, want %q", tc.mode, got, tc.want)
}
}
if mode, ok := ModeForTokenTier(SecCheckTier); !ok || mode != ModeIssuesAndPRs {
t.Errorf("ModeForTokenTier(%q) = %v, %v; want ISSUES_AND_PRS, true", SecCheckTier, mode, ok)
}
}
1 change: 1 addition & 0 deletions src/docs/github-app-setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ Repository permissions used by the dashboard setup UI are:
| Actions | Read and write (Read-only with `governor.ci.auto_approve_workflows: false`) | Inspect workflow runs; the default-on approval sweep approves gated `pull_request` runs for trusted bots, the App and dashboard-admitted heads with the App token ([#16603](https://github.com/hivecommons/hive/issues/16603)). If the sweep is off or refused and GitHub holds trusted Dependabot/App/clanker runs in `action_required`, Hive posts `/ok-to-test`; the repository-local `hive-ok-to-test` workflow uses its own `GITHUB_TOKEN` with Actions write to approve those runs. With Read and write, the contributor lane also approves held runs directly for a PR author the contributor registry admitted at a `require_tiers` tier (`auto_merge.contributor_lane.approve_held_runs_for_registry_trusted`). Optional: without it the hive marks such a PR `hive:unauthorized` and waits for a maintainer to click **Approve and run** ([#14589](https://github.com/hivecommons/hive/issues/14589)). |
| Administration | Read-only | Read branch-protection rules before the merge-request watcher lands a PR; Hive refuses closed if it cannot verify the base branch is protected. |
| Commit statuses | Read/write | Needed when `governor.labels.automerge` is the default `lgtm` on a repository running Prow's commit-bound `lgtm` (prow-github-actions v3): the queue action sets a `prow/lgtm` success status on the approved head so Prow does not strip the label. Without the write grant the label and approval still land, but every queue action takes the failure branch and posts a PR comment saying the status could not be set. Choose a distinct label such as `hive/auto-merge` if you would rather keep this at Read. |
| Dependabot alerts | Read-only | Required for the sec-check agent's Dependabot scanning (`gh api repos/<org>/<repo>/dependabot/alerts`, [#17426](https://github.com/hivecommons/hive/issues/17426)). Least privilege: only the token minted for an ISSUES_AND_PRS agent with role `sec-check` requests `vulnerability_alerts: read`; every other agent's token is unchanged, even though the App itself must declare the permission. Existing installations must re-accept the updated permission once under Settings → Integrations → the app → Review request. Optional: without it the sec-check mint retries without the grant, the Dependabot call returns 403, and the hive still mints tokens. |

| Workflows | Read/write | Let trusted-tier agents push branches that modify `.github/workflows/`, and let the contributor lane merge an owner-approved PR that edits `.github/workflows/**` (`auto_merge.contributor_lane.owner_approval_releases_workflow_fence`, [#14664](https://github.com/hivecommons/hive/issues/14664)); without the grant such a merge is refused and the hive marks the PR `hive:unauthorized` (`merge-workflow-path`) and parks it at 090 for a maintainer's **Merge** click. Without it GitHub also rejects any GitHub App-token push touching those files server-side ("refusing to allow a GitHub App to create or update workflow … without workflows permission") no matter what the token requests. Maintainer user credentials are separate and can still push workflow-file changes when their account has the normal repository rights; the Hive App cannot until this grant is added **and re-accepted on each installation** (an existing install must approve the new permission under Settings → Integrations → the app → Review request). See [`hive-open-pr`](hive-open-pr.md#workflow-file-pushes-rejected-by-app-tokens) for the agent delivery procedure and [#6985](https://github.com/hivecommons/hive/issues/6985) for the outstanding permission change. |
Organization permission:
Expand Down
41 changes: 41 additions & 0 deletions src/pkg/github/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/golang-jwt/jwt/v5"
gh "github.com/google/go-github/v72/github"
"github.com/hivecommons/hive/crux"
"github.com/hivecommons/hive/crux/agentmode"
"github.com/hivecommons/hive/pkg/config"
)

Expand Down Expand Up @@ -334,6 +335,20 @@ func (a *AppAuth) ScopedTokenForRepos(ctx context.Context, tier string, repos []
PullRequests: gh.Ptr("write"),
Metadata: gh.Ptr("read"),
}
case agentmode.SecCheckTier:
// The sec-check role refinement of the contributor tier (#17426,
// agentmode.TokenTierForRole): the contributor set plus Dependabot
// alerts read so the agent can call dependabot/alerts. Scoped to
// this one role so no other ISSUES_AND_PRS agent's token carries
// it. Optional like Workflows: the mint below retries without it
// when the installation has not accepted the App permission.
perms = &gh.InstallationPermissions{
Issues: gh.Ptr("write"),
Contents: gh.Ptr("write"),
PullRequests: gh.Ptr("write"),
Metadata: gh.Ptr("read"),
VulnerabilityAlerts: gh.Ptr("read"),
}
case "trusted", "merger":
perms = &gh.InstallationPermissions{
Issues: gh.Ptr("write"),
Expand Down Expand Up @@ -425,6 +440,15 @@ func (a *AppAuth) ScopedTokenForRepos(ctx context.Context, tier string, repos []
defer cancel()
jwtClient := newJWTClient(jwtToken, a.apiURL)
installToken, _, err := jwtClient.Apps.CreateInstallationToken(mintCtx, installationID, opts)
if isPermissionNotGrantedMintError(err) && perms.VulnerabilityAlerts != nil && perms.Workflows == nil {
a.logger.Warn("scoped token mint with vulnerability_alerts:read failed; retrying without it — "+
"the sec-check Dependabot alerts call will 403 until the App grants Dependabot alerts read-only and the installation re-accepts it; see docs/github-app-setup.md",
"tier", tier, "error", err)
perms.VulnerabilityAlerts = nil
alertsCtx, alertsCancel := mintContext(ctx)
defer alertsCancel()
installToken, _, err = jwtClient.Apps.CreateInstallationToken(alertsCtx, installationID, opts)
}
if err != nil && perms.Workflows != nil {
// The installation has likely not accepted the Workflows permission
// on the App yet — GitHub refuses to mint a token requesting a
Expand All @@ -448,6 +472,22 @@ func (a *AppAuth) ScopedTokenForRepos(ctx context.Context, tier string, repos []
return installToken.GetToken(), nil
}

// isPermissionNotGrantedMintError reports whether an installation-token mint
// was refused because the request named a permission the installation has
// not accepted (GitHub answers 422 "The permissions requested are not granted
// to this installation."). A timeout, a repo-scope 422 (IsRepoScopeMintError)
// or any other failure is not that case and must not earn a second mint: the
// retry would double the bounded mint window and break the one-mint-per-repo
// accounting the hub relies on to exclude an unmintable repo.
func isPermissionNotGrantedMintError(err error) bool {
var ghErr *gh.ErrorResponse
if !errors.As(err, &ghErr) || ghErr.Response == nil {
return false
}
return ghErr.Response.StatusCode == http.StatusUnprocessableEntity &&
strings.Contains(strings.ToLower(ghErr.Message), "permission")
}

// IsRepoScopeMintError reports whether an installation-token mint failed
// because of the REPOSITORY the token was scoped to, not the tier or the App.
// GitHub answers a token request naming a repository the installation cannot
Expand Down Expand Up @@ -796,6 +836,7 @@ func installationPermissionMap(p *gh.InstallationPermissions) map[string]string
put("metadata", p.GetMetadata())
put("pull_requests", p.GetPullRequests())
put("secrets", p.GetSecrets())
put("vulnerability_alerts", p.GetVulnerabilityAlerts())
put("workflows", p.GetWorkflows())
return out
}
103 changes: 103 additions & 0 deletions src/pkg/github/workflows_permission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import (
"os"
"testing"
"time"

"github.com/hivecommons/hive/crux"
"github.com/hivecommons/hive/crux/agentmode"
)

// These are the regression tests for #4677: agents could not open PRs
Expand Down Expand Up @@ -137,3 +140,103 @@ func TestScopedToken_WorkflowsFallbackWhenInstallationLacksGrant(t *testing.T) {
}
}
}

// Only the sec-check role refinement of the contributor tier requests
// Dependabot alerts read (#17426, least privilege): every other tier's token
// must not carry it.
func TestScopedToken_OnlySecCheckTierRequestsVulnerabilityAlerts(t *testing.T) {
var gotPerms map[string]string
mux := http.NewServeMux()
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
gotPerms = decodePermissions(t, r)
json.NewEncoder(w).Encode(map[string]any{
"token": "scoped-token",
"expires_at": time.Now().Add(time.Hour).Format(time.RFC3339),
})
})
server := httptest.NewServer(mux)
defer server.Close()
auth := newWorkflowsTestAuth(t, server.URL)

for _, tier := range []string{"advisor", "reviewer", "newcomer", "contributor", "trusted", "merger", crux.SharedUIDReadTokenTier} {
if _, err := auth.ScopedToken(context.Background(), tier); err != nil {
t.Fatalf("ScopedToken(%q): %v", tier, err)
}
if got, present := gotPerms["vulnerability_alerts"]; present {
t.Errorf("tier %q requests vulnerability_alerts=%q; only the sec-check tier may", tier, got)
}
}
if _, err := auth.ScopedToken(context.Background(), agentmode.SecCheckTier); err != nil {
t.Fatalf("ScopedToken(sec-check): %v", err)
}
if gotPerms["vulnerability_alerts"] != "read" {
t.Errorf("sec-check tier vulnerability_alerts = %q, want read", gotPerms["vulnerability_alerts"])
}
for _, perm := range []string{"contents", "issues", "pull_requests"} {
if gotPerms[perm] != "write" {
t.Errorf("sec-check tier %s = %q, want write (it is the contributor set plus alerts read)", perm, gotPerms[perm])
}
}
}

// The sec-check tier requests Dependabot alerts read and degrades to a token
// without it when the installation has not accepted the permission.
func TestScopedToken_SecCheckVulnerabilityAlertsWithFallback(t *testing.T) {
var requests []map[string]string
mux := http.NewServeMux()
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
perms := decodePermissions(t, r)
requests = append(requests, perms)
if perms["vulnerability_alerts"] != "" {
w.WriteHeader(http.StatusUnprocessableEntity)
json.NewEncoder(w).Encode(map[string]any{"message": "The permissions requested are not granted to this installation."})
return
}
json.NewEncoder(w).Encode(map[string]any{
"token": "scoped-token-contributor",
"expires_at": time.Now().Add(time.Hour).Format(time.RFC3339),
})
})
server := httptest.NewServer(mux)
defer server.Close()

auth := newWorkflowsTestAuth(t, server.URL)
if _, err := auth.ScopedToken(context.Background(), agentmode.SecCheckTier); err != nil {
t.Fatalf("ScopedToken: %v", err)
}
if len(requests) != 2 {
t.Fatalf("mint requests = %d, want 2", len(requests))
}
if requests[0]["vulnerability_alerts"] != "read" {
t.Errorf("first mint vulnerability_alerts = %q, want read", requests[0]["vulnerability_alerts"])
}
if _, present := requests[1]["vulnerability_alerts"]; present {
t.Errorf("retry still requests vulnerability_alerts: %v", requests[1])
}
if requests[1]["pull_requests"] != "write" {
t.Errorf("retry pull_requests = %q, want write", requests[1]["pull_requests"])
}
}

// A repo-scope 422 (renamed / removed repository, #7869) is not a missing
// permission grant: the contributor mint must fail on the first attempt so
// the hub's one-mint-per-repo fall-through accounting still holds.
func TestScopedToken_SecCheckRepoScopeErrorDoesNotRetry(t *testing.T) {
var requests int
mux := http.NewServeMux()
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
requests++
w.WriteHeader(http.StatusUnprocessableEntity)
json.NewEncoder(w).Encode(map[string]any{"message": "There is at least one repository that does not exist or is not accessible to the parent installation."})
})
server := httptest.NewServer(mux)
defer server.Close()

auth := newWorkflowsTestAuth(t, server.URL)
if _, err := auth.ScopedTokenForRepos(context.Background(), agentmode.SecCheckTier, []string{"gone"}); err == nil {
t.Fatal("ScopedTokenForRepos: want the repo-scope error, got nil")
}
if requests != 1 {
t.Fatalf("mint requests = %d, want 1 (a repo-scope 422 must not trigger the permission fallback)", requests)
}
}
6 changes: 5 additions & 1 deletion src/pkg/mint/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ var tierScopes = map[string][]string{
"advisor": {ScopeIssuesRead},
"newcomer": {ScopeIssuesRead, ScopeIssuesWrite},
"contributor": {ScopeIssuesRead, ScopeIssuesWrite, ScopeContentsWrite, ScopePullsWrite},
"trusted": {ScopeIssuesRead, ScopeIssuesWrite, ScopeContentsWrite, ScopePullsWrite, ScopeMerge},
// The sec-check role refinement of contributor (agentmode.SecCheckTier):
// same hive-side scopes; the extra Dependabot alerts read lives only on
// the GitHub App token.
"sec-check": {ScopeIssuesRead, ScopeIssuesWrite, ScopeContentsWrite, ScopePullsWrite},
"trusted": {ScopeIssuesRead, ScopeIssuesWrite, ScopeContentsWrite, ScopePullsWrite, ScopeMerge},
}

// ScopesForTier returns the scope set for a trust tier. An unrecognized tier
Expand Down
Loading