From 6bd13fc5d1701fe066ff7b4f634dd8e5201cb4af Mon Sep 17 00:00:00 2001 From: fullsend-code Date: Thu, 14 May 2026 11:54:06 +0000 Subject: [PATCH 1/2] fix(#910): use repository_owner in WIF attribute conditions The WIF attribute condition used assertion.repository scoped to the .fullsend repo, which rejected OIDC tokens from cross-repo dispatched workflows. When a shim workflow in another repo (e.g. org/my-app) calls .fullsend via workflow_call, GitHub sets the repository claim to the caller repo, causing GCP STS to return HTTP 400. Changed buildAttributeCondition to use assertion.repository_owner instead of assertion.repository, scoping to the org level. Also updated the per-repo path in ProvisionWIF to use repository_owner with just the owner portion. This is safe because the mint's prevalidateOIDCToken already validates org membership, allowed workflow files, and workflow ref prefixes. parseConditionOrgs retains backward compatibility with old assertion.repository conditions via the fullsendRepoSuffix check. Added TestBuildAttributeCondition unit test covering single-org and multi-org cases. Note: pre-commit could not run due to Go toolchain permission error in sandbox. All Go tests pass. Closes #910 --- internal/dispatch/gcf/provisioner.go | 14 ++++++----- internal/dispatch/gcf/provisioner_test.go | 30 ++++++++++++++++------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/internal/dispatch/gcf/provisioner.go b/internal/dispatch/gcf/provisioner.go index 188504dd94..327743dcc0 100644 --- a/internal/dispatch/gcf/provisioner.go +++ b/internal/dispatch/gcf/provisioner.go @@ -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" @@ -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) } diff --git a/internal/dispatch/gcf/provisioner_test.go b/internal/dispatch/gcf/provisioner_test.go index 49e960702b..96493aae2c 100644 --- a/internal/dispatch/gcf/provisioner_test.go +++ b/internal/dispatch/gcf/provisioner_test.go @@ -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" @@ -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" @@ -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. @@ -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) { @@ -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") @@ -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) { @@ -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) @@ -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) { @@ -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 From 8325945671dd74adf7d2f64758958326974143f8 Mon Sep 17 00:00:00 2001 From: fullsend-fix Date: Thu, 14 May 2026 12:10:08 +0000 Subject: [PATCH 2/2] fix: keep per-repo WIF condition repo-scoped and fix comment terminology - Revert per-repo ProvisionWIF path to use assertion.repository == '' instead of assertion.repository_owner. Per-repo installs don't use cross-repo dispatch, and widening to org-level would be inconsistent with the repo-scoped IAM principal binding. - Update parseConditionOrgs doc comment to label repository_owner as the current format and assertion.repository as legacy, matching the new org-level default from this PR. Addresses review feedback on #920 Co-Authored-By: Claude Opus 4.6 Signed-off-by: fullsend-fix --- internal/dispatch/gcf/provisioner.go | 6 +++--- internal/dispatch/gcf/provisioner_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/dispatch/gcf/provisioner.go b/internal/dispatch/gcf/provisioner.go index 327743dcc0..098dc07852 100644 --- a/internal/dispatch/gcf/provisioner.go +++ b/internal/dispatch/gcf/provisioner.go @@ -672,8 +672,8 @@ func buildAttributeCondition(orgs []string) string { const fullsendRepoSuffix = "/.fullsend" // parseConditionOrgs extracts GitHub org names from a WIF attribute condition. -// Supports both repo-scoped ("assertion.repository == 'org/.fullsend'") and -// legacy org-scoped ("assertion.repository_owner == 'org'") formats. +// Supports both the current org-scoped ("assertion.repository_owner == 'org'") +// and legacy repo-scoped ("assertion.repository == 'org/.fullsend'") formats. func parseConditionOrgs(condition string) []string { var orgs []string for _, part := range strings.Split(condition, "'") { @@ -783,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_owner == '%s'", parts[0]) + attrCondition = fmt.Sprintf("assertion.repository == '%s'", p.cfg.Repo) } else { attrCondition = buildAttributeCondition(orgs) } diff --git a/internal/dispatch/gcf/provisioner_test.go b/internal/dispatch/gcf/provisioner_test.go index 96493aae2c..9d563472bf 100644 --- a/internal/dispatch/gcf/provisioner_test.go +++ b/internal/dispatch/gcf/provisioner_test.go @@ -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_owner == 'acme'", fake.lastWIFProviderConfig.AttributeCondition) + assert.Equal(t, "assertion.repository == 'acme/widget'", fake.lastWIFProviderConfig.AttributeCondition) assert.Contains(t, wifPath, "gh-acme-widget") require.Len(t, fake.projectIAMBindings, 1) @@ -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_owner == 'acme'", fake.lastWIFProviderConfig.AttributeCondition) + assert.Equal(t, "assertion.repository == 'acme/widget'", fake.lastWIFProviderConfig.AttributeCondition) } func TestProvisionWIF_OrgScoped_Unchanged(t *testing.T) {