Skip to content

fix(mint): reject cross-org installation mismatch #1332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ralphbean wants to merge 1 commit into
base: main
Choose a base branch
from
+179 −0
Open

fix(mint): reject cross-org installation mismatch #1332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 15 additions & 0 deletions internal/dispatch/gcf/mintsrc/main.go.embed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,10 @@ func (h *Handler) mintToken(ctx context.Context, org, role string, repos []strin
// findInstallation looks up the app's installation ID via the repo-based API.
// Using GET /repos/{owner}/{repo}/installation instead of /orgs/{org}/installation
// enables per-repo app installations in the future.
//
// The returned installation's account is verified against the expected org to
// prevent cross-org token leakage when the same GitHub App is installed on
// multiple orgs (see issue #1321).
func (h *Handler) findInstallation(ctx context.Context, jwt, org, repo string) (int64, error) {
reqURL := fmt.Sprintf("%s/repos/%s/%s/installation", h.githubBaseURL, org, repo)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
Expand Down Expand Up @@ -741,6 +745,17 @@ func (h *Handler) findInstallation(ctx context.Context, jwt, org, repo string) (
return 0, fmt.Errorf("no installation found for %s/%s", org, repo)
}

// Verify the installation belongs to the expected org. When the same
// GitHub App is installed on multiple orgs, the API could return an
// installation for the wrong org. Accepting it would mint a token
// scoped to a different org's repos.
if !strings.EqualFold(inst.Account.Login, org) {
log.Printf("installation org mismatch: expected %s, got %s (installation %d for %s/%s)",
org, inst.Account.Login, inst.ID, org, repo)
return 0, fmt.Errorf("installation for %s/%s belongs to %s, not %s",
org, repo, inst.Account.Login, org)
}

return inst.ID, nil
}

Expand Down
15 changes: 15 additions & 0 deletions internal/mint/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,10 @@ func (h *Handler) mintToken(ctx context.Context, org, role string, repos []strin
// findInstallation looks up the app's installation ID via the repo-based API.
// Using GET /repos/{owner}/{repo}/installation instead of /orgs/{org}/installation
// enables per-repo app installations in the future.
//
// The returned installation's account is verified against the expected org to
// prevent cross-org token leakage when the same GitHub App is installed on
// multiple orgs (see issue #1321).
func (h *Handler) findInstallation(ctx context.Context, jwt, org, repo string) (int64, error) {
reqURL := fmt.Sprintf("%s/repos/%s/%s/installation", h.githubBaseURL, org, repo)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
Expand Down Expand Up @@ -741,6 +745,17 @@ func (h *Handler) findInstallation(ctx context.Context, jwt, org, repo string) (
return 0, fmt.Errorf("no installation found for %s/%s", org, repo)
}

// Verify the installation belongs to the expected org. When the same
// GitHub App is installed on multiple orgs, the API could return an
// installation for the wrong org. Accepting it would mint a token
// scoped to a different org's repos.
Comment on lines +748 to +751
Copy link
Copy Markdown
Contributor

@waynesun09 waynesun09 May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — Threat model description overstates the risk

The comment says "the API could return an installation for the wrong org," but GET /repos/{owner}/{repo}/installation constructs the URL using org from the OIDC-validated repository_owner claim, so GitHub should only return the installation for that specific org (or 404). The scenario where Account.Login differs from {owner} is not documented GitHub behavior — realistic vectors are more like repo transfers or org renames, not routine cross-org installation leakage.

The defense-in-depth check is still valuable and worth keeping, but consider clarifying the comment to say this guards against hypothetical API inconsistencies rather than a known, routinely exploitable path.

6/10 review agents flagged this

if !strings.EqualFold(inst.Account.Login, org) {
log.Printf("installation org mismatch: expected %s, got %s (installation %d for %s/%s)",
Copy link
Copy Markdown
Contributor

@waynesun09 waynesun09 May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — No structured alerting for this security-relevant event

A cross-org installation mismatch is a security event (misconfigured App or potential exploitation attempt), but it's only logged via log.Printf — easily lost in Cloud Functions log volume. This is the sole detection layer for this attack path.

Consider tagging the log with a severity prefix (e.g., "SECURITY: installation org mismatch") that Cloud Logging alerting policies can match on, or incrementing a custom metric (mint_cross_org_mismatch_total) so a threshold-based alert fires.

2/10 review agents flagged this

org, inst.Account.Login, inst.ID, org, repo)
return 0, fmt.Errorf("installation for %s/%s belongs to %s, not %s",
org, repo, inst.Account.Login, org)
}

return inst.ID, nil
}

Expand Down
149 changes: 149 additions & 0 deletions internal/mint/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,155 @@ func TestHandler_MultiOrg_WrongOrg(t *testing.T) {
}
}

// TestHandler_CrossOrgInstallationMismatch reproduces the bug from issue #1321:
// an agent running in org-a (agentshed) gets a token that can write to repos
// in org-b (fullsend-ai). This happens when the same GitHub App is installed
// on both orgs and findInstallation returns an installation belonging to a
// different org than the OIDC-authenticated org. The mint must reject this.
func TestHandler_CrossOrgInstallationMismatch(t *testing.T) {
t.Setenv("ALLOWED_ORGS", "org-a,org-b")
t.Setenv("GCP_PROJECT_NUMBER", "123456")
t.Setenv("WIF_POOL_NAME", "test-pool")
t.Setenv("WIF_PROVIDER_NAME", "github-oidc")
t.Setenv("OIDC_AUDIENCE", "fullsend-mint")
// Same app ID for both orgs (shared GitHub App).
t.Setenv("ROLE_APP_IDS", `{"org-a/retro":"999","org-b/retro":"999"}`)
t.Setenv("ALLOWED_WORKFLOW_FILES", "*")

pemData, err := generateTestRSAKey()
if err != nil {
t.Fatalf("generating test key: %v", err)
}

// OIDC token is from org-a (the requesting org).
oidcToken := makeTestOIDCToken(
"https://token.actions.githubusercontent.com",
"fullsend-mint",
"org-a/.fullsend",
"org-a",
"org-a/.fullsend/.github/workflows/retro.yml@refs/heads/main",
time.Now().Add(10*time.Minute).Unix(),
)

// Simulate GitHub returning an installation belonging to org-b instead
// of org-a. This could happen if the App isn't installed on org-a's copy
// of the repo, or if GitHub resolves the installation differently.
github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch {
case r.URL.Path == "/repos/org-a/seshi/installation" && r.Method == http.MethodGet:
// GitHub returns an installation, but the account is org-b.
json.NewEncoder(w).Encode(installationResponse{
ID: 77777, Account: struct {
Login string `json:"login"`
}{Login: "org-b"},
})
case strings.HasPrefix(r.URL.Path, "/app/installations/77777/access_tokens") && r.Method == http.MethodPost:
// If we get here, the mint failed to catch the org mismatch.
w.WriteHeader(http.StatusCreated)
json.NewEncoder(w).Encode(installationTokenResponse{
Token: "ghs_CROSS_ORG_TOKEN",
ExpiresAt: "2026-05-07T12:00:00Z",
})
default:
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
w.WriteHeader(http.StatusNotFound)
}
}))
defer github.Close()

h := NewHandler(&fakePEMAccessor{
pems: map[string][]byte{"org-a/retro": pemData},
}, &fakeTokenValidator{})
h.githubBaseURL = github.URL

body := `{"role":"retro","repos":["seshi"]}`
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodPost, "/v1/token", strings.NewReader(body))
req.Header.Set("Authorization", "Bearer "+oidcToken)
h.ServeHTTP(rec, req)

// The mint MUST reject this — the installation belongs to org-b but the
// OIDC token authenticated org-a. A 502 (bad gateway) is appropriate
// since the upstream GitHub API returned an unexpected installation.
if rec.Code == http.StatusOK {
var resp mintResponse
json.NewDecoder(rec.Body).Decode(&resp)
t.Fatalf("mint should reject cross-org installation mismatch, but returned 200 with token=%s", resp.Token)
}
if rec.Code != http.StatusBadGateway {
t.Fatalf("expected 502 for cross-org installation mismatch, got %d: %s", rec.Code, rec.Body.String())
}
}

// TestHandler_CrossOrgInstallation_SameOrgPasses verifies the positive case:
// when the installation account matches the OIDC org, minting succeeds.
func TestHandler_CrossOrgInstallation_SameOrgPasses(t *testing.T) {
t.Setenv("ALLOWED_ORGS", "org-a,org-b")
t.Setenv("GCP_PROJECT_NUMBER", "123456")
t.Setenv("WIF_POOL_NAME", "test-pool")
t.Setenv("WIF_PROVIDER_NAME", "github-oidc")
t.Setenv("OIDC_AUDIENCE", "fullsend-mint")
t.Setenv("ROLE_APP_IDS", `{"org-a/retro":"999","org-b/retro":"999"}`)
t.Setenv("ALLOWED_WORKFLOW_FILES", "*")

pemData, err := generateTestRSAKey()
if err != nil {
t.Fatalf("generating test key: %v", err)
}

oidcToken := makeTestOIDCToken(
"https://token.actions.githubusercontent.com",
"fullsend-mint",
"org-a/.fullsend",
"org-a",
"org-a/.fullsend/.github/workflows/retro.yml@refs/heads/main",
time.Now().Add(10*time.Minute).Unix(),
)

github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch {
case r.URL.Path == "/repos/org-a/seshi/installation" && r.Method == http.MethodGet:
// Installation correctly belongs to org-a.
json.NewEncoder(w).Encode(installationResponse{
ID: 88888, Account: struct {
Login string `json:"login"`
}{Login: "org-a"},
})
case strings.HasPrefix(r.URL.Path, "/app/installations/88888/access_tokens") && r.Method == http.MethodPost:
w.WriteHeader(http.StatusCreated)
json.NewEncoder(w).Encode(installationTokenResponse{
Token: "ghs_correct_org_token",
ExpiresAt: "2026-05-07T12:00:00Z",
Copy link
Copy Markdown
Contributor

@waynesun09 waynesun09 May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — Missing test for case-mismatch between org and Account.Login

The code uses strings.EqualFold but tests only use exact-case matches ("org-a" vs "org-b", "org-a" vs "org-a"). GitHub returns canonical-cased logins (e.g., "Org-A") while org is lowercased at line 508. Adding a test where this mock returns Login: "Org-A" (mixed case) and verifying minting still succeeds (200) would document the EqualFold intent and guard against a future regression if someone changes it to ==.

4/10 review agents flagged this

})
default:
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
w.WriteHeader(http.StatusNotFound)
}
}))
defer github.Close()

h := NewHandler(&fakePEMAccessor{
pems: map[string][]byte{"org-a/retro": pemData},
}, &fakeTokenValidator{})
h.githubBaseURL = github.URL

body := `{"role":"retro","repos":["seshi"]}`
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodPost, "/v1/token", strings.NewReader(body))
req.Header.Set("Authorization", "Bearer "+oidcToken)
h.ServeHTTP(rec, req)

if rec.Code != http.StatusOK {
t.Fatalf("expected 200 when installation matches OIDC org, got %d: %s", rec.Code, rec.Body.String())
}

var resp mintResponse
json.NewDecoder(rec.Body).Decode(&resp)
if resp.Token != "ghs_correct_org_token" {
t.Fatalf("expected ghs_correct_org_token, got %s", resp.Token)
}
}

func TestResolveWIFProvider(t *testing.T) {
h := &Handler{
defaultWIFProvider: "github-oidc",
Expand Down
Loading