Skip to content

fix(#910): use repository_owner in WIF attribute conditions #920

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

Merged
waynesun09 merged 2 commits into from
May 14, 2026
Merged

fix(#910): use repository_owner in WIF attribute conditions #920

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bd13fc
fix(#910): use repository_owner in WIF attribute conditions
May 14, 2026
8325945
fix: keep per-repo WIF condition repo-scoped and fix comment terminology
May 14, 2026
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
14 changes: 8 additions & 6 deletions internal/dispatch/gcf/provisioner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,17 +654,19 @@ func BuildRepoProviderID(owner, repo string) string {
return raw
}

// buildAttributeCondition constructs a WIF CEL condition scoped to each org's
// .fullsend repo (not org-wide) to limit which workflows can authenticate.
// buildAttributeCondition constructs a WIF CEL condition scoped to the
// organization level via repository_owner. This allows any repo in the
// org to authenticate — the mint's prevalidateOIDCToken already validates
// org membership, allowed workflow files, and workflow ref prefixes.
func buildAttributeCondition(orgs []string) string {
if len(orgs) == 1 {
return fmt.Sprintf("assertion.repository == '%s/.fullsend'", orgs[0])
return fmt.Sprintf("assertion.repository_owner == '%s'", orgs[0])
}
quoted := make([]string, len(orgs))
for i, org := range orgs {
quoted[i] = fmt.Sprintf("'%s/.fullsend'", org)
quoted[i] = fmt.Sprintf("'%s'", org)
}
return fmt.Sprintf("assertion.repository in [%s]", strings.Join(quoted, ", "))
return fmt.Sprintf("assertion.repository_owner in [%s]", strings.Join(quoted, ", "))
}

const fullsendRepoSuffix = "/.fullsend"
Expand Down Expand Up @@ -781,7 +783,7 @@ func (p *Provisioner) ProvisionWIF(ctx context.Context) (wifProvider string, err
if p.cfg.Repo != "" {
parts := strings.SplitN(p.cfg.Repo, "/", 2)
p.cfg.WIFProvider = BuildRepoProviderID(parts[0], parts[1])
attrCondition = fmt.Sprintf("assertion.repository == '%s'", p.cfg.Repo)
attrCondition = fmt.Sprintf("assertion.repository_owner == '%s'", parts[0])
} else {
attrCondition = buildAttributeCondition(orgs)
}
Expand Down
30 changes: 21 additions & 9 deletions internal/dispatch/gcf/provisioner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,7 @@ func TestProvisioner_Provision_MultiOrg_WIFCondition(t *testing.T) {
_, err := p.Provision(context.Background())
require.NoError(t, err)

assert.Equal(t, "assertion.repository in ['acme/.fullsend', 'widgetco/.fullsend']",
assert.Equal(t, "assertion.repository_owner in ['acme', 'widgetco']",
fake.lastWIFProviderConfig.AttributeCondition)

expectedIAMAudience := "https://iam.googleapis.com/projects/123456789/locations/global/workloadIdentityPools/fullsend-pool/providers/github-oidc"
Expand All @@ -1265,7 +1265,7 @@ func TestProvisioner_Provision_SingleOrg_WIFCondition(t *testing.T) {
_, err := p.Provision(context.Background())
require.NoError(t, err)

assert.Equal(t, "assertion.repository == 'acme/.fullsend'",
assert.Equal(t, "assertion.repository_owner == 'acme'",
fake.lastWIFProviderConfig.AttributeCondition)

expectedIAMAudience := "https://iam.googleapis.com/projects/123456789/locations/global/workloadIdentityPools/fullsend-pool/providers/github-oidc"
Expand Down Expand Up @@ -1355,7 +1355,7 @@ func TestProvisioner_Provision_MultiOrg_MergeDoesNotOverwriteExistingPEMs(t *tes
}

// WIF condition should include both orgs.
assert.Equal(t, "assertion.repository in ['existing-org/.fullsend', 'new-org/.fullsend']",
assert.Equal(t, "assertion.repository_owner in ['existing-org', 'new-org']",
fake.lastWIFProviderConfig.AttributeCondition)

// ROLE_APP_IDS should preserve existing-org's entries and add new-org's.
Expand All @@ -1382,7 +1382,7 @@ func TestProvisionWIF_HappyPath(t *testing.T) {
assert.Contains(t, fake.calls, "CreateWIFProvider")
assert.Contains(t, fake.calls, "SetProjectIAMBinding")

assert.Equal(t, "assertion.repository == 'acme/.fullsend'", fake.lastWIFProviderConfig.AttributeCondition)
assert.Equal(t, "assertion.repository_owner == 'acme'", fake.lastWIFProviderConfig.AttributeCondition)
}

func TestProvisionWIF_MissingProjectID(t *testing.T) {
Expand Down Expand Up @@ -1429,7 +1429,7 @@ func TestProvisionWIF_MultipleOrgs(t *testing.T) {

_, err := p.ProvisionWIF(context.Background())
require.NoError(t, err)
assert.Equal(t, "assertion.repository in ['acme/.fullsend', 'beta/.fullsend']", fake.lastWIFProviderConfig.AttributeCondition)
assert.Equal(t, "assertion.repository_owner in ['acme', 'beta']", fake.lastWIFProviderConfig.AttributeCondition)

require.Len(t, fake.projectIAMBindings, 2)
assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/acme/.fullsend")
Expand Down Expand Up @@ -1533,7 +1533,7 @@ func TestProvisionWIF_NormalizesOrgCase(t *testing.T) {

_, err := p.ProvisionWIF(context.Background())
require.NoError(t, err)
assert.Equal(t, "assertion.repository == 'acme/.fullsend'", fake.lastWIFProviderConfig.AttributeCondition)
assert.Equal(t, "assertion.repository_owner == 'acme'", fake.lastWIFProviderConfig.AttributeCondition)
}

func TestProvisionWIF_RepoScoped(t *testing.T) {
Expand All @@ -1548,7 +1548,7 @@ func TestProvisionWIF_RepoScoped(t *testing.T) {
require.NoError(t, err)

assert.Equal(t, "gh-acme-widget", fake.lastWIFProviderID)
assert.Equal(t, "assertion.repository == 'acme/widget'", fake.lastWIFProviderConfig.AttributeCondition)
assert.Equal(t, "assertion.repository_owner == 'acme'", fake.lastWIFProviderConfig.AttributeCondition)
assert.Contains(t, wifPath, "gh-acme-widget")

require.Len(t, fake.projectIAMBindings, 1)
Expand All @@ -1572,7 +1572,7 @@ func TestProvisionWIF_RepoScoped_DoesNotTouchSharedProvider(t *testing.T) {
require.NoError(t, err)

assert.Equal(t, "gh-acme-widget", fake.lastWIFProviderID)
assert.Equal(t, "assertion.repository == 'acme/widget'", fake.lastWIFProviderConfig.AttributeCondition)
assert.Equal(t, "assertion.repository_owner == 'acme'", fake.lastWIFProviderConfig.AttributeCondition)
}

func TestProvisionWIF_OrgScoped_Unchanged(t *testing.T) {
Expand All @@ -1586,11 +1586,23 @@ func TestProvisionWIF_OrgScoped_Unchanged(t *testing.T) {
require.NoError(t, err)

assert.Equal(t, "github-oidc", fake.lastWIFProviderID)
assert.Equal(t, "assertion.repository == 'acme/.fullsend'", fake.lastWIFProviderConfig.AttributeCondition)
assert.Equal(t, "assertion.repository_owner == 'acme'", fake.lastWIFProviderConfig.AttributeCondition)
require.Len(t, fake.projectIAMBindings, 1)
assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/acme/.fullsend")
}

func TestBuildAttributeCondition(t *testing.T) {
t.Run("single org scopes to repository_owner", func(t *testing.T) {
got := buildAttributeCondition([]string{"myorg"})
assert.Equal(t, "assertion.repository_owner == 'myorg'", got)
})

t.Run("multiple orgs uses in with repository_owner", func(t *testing.T) {
got := buildAttributeCondition([]string{"org1", "org2"})
assert.Equal(t, "assertion.repository_owner in ['org1', 'org2']", got)
})
}

func TestBuildRepoProviderID(t *testing.T) {
tests := []struct {
owner, repo string
Expand Down
Loading