From 043897993b123a02da07a5551ae021a8b7a769a5 Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Wed, 8 Jul 2026 13:22:20 -0400 Subject: [PATCH 1/4] fix(wif): preserve GitHub canonical case in attributeCondition and IAM principals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WIF provisioner lowercased org/repo names before substituting them into CEL attributeCondition literals and IAM principalSet paths. Since GitHub's OIDC token preserves canonical case and Google STS compares claims byte-for-byte, any org/repo with uppercase letters permanently failed authentication with "unauthorized_client". Separate case-insensitive dedup (for internal bookkeeping, provider ID generation) from the values written into security-critical CEL conditions and IAM principal paths — those now preserve the original case. Fixes: fullsend-ai/fullsend#3678 Assisted-by: Claude Signed-off-by: Wayne Sun --- internal/dispatch/gcf/provisioner.go | 76 ++++++++++------------- internal/dispatch/gcf/provisioner_test.go | 65 ++++++++++++++++--- 2 files changed, 91 insertions(+), 50 deletions(-) diff --git a/internal/dispatch/gcf/provisioner.go b/internal/dispatch/gcf/provisioner.go index 7c325847f0..5669b6bbb6 100644 --- a/internal/dispatch/gcf/provisioner.go +++ b/internal/dispatch/gcf/provisioner.go @@ -574,7 +574,7 @@ func (p *Provisioner) Provision(ctx context.Context) (map[string]string, error) return nil, fmt.Errorf("at least one GitHub org is required") } seen := make(map[string]bool) - for i, org := range p.cfg.GitHubOrgs { + for _, org := range p.cfg.GitHubOrgs { if !mintcore.GitHubOrgPattern.MatchString(org) || strings.Contains(org, "--") { return nil, fmt.Errorf("invalid GitHub org name: %q", org) } @@ -583,7 +583,6 @@ func (p *Provisioner) Provision(ctx context.Context) (map[string]string, error) return nil, fmt.Errorf("duplicate GitHub org after normalization: %q", org) } seen[lower] = true - p.cfg.GitHubOrgs[i] = lower } for role := range p.cfg.AgentPEMs { if !mintcore.RolePattern.MatchString(role) { @@ -1143,10 +1142,10 @@ func parseConditionOrgs(condition string) []string { if strings.HasSuffix(part, fullsendRepoSuffix) { org := strings.TrimSuffix(part, fullsendRepoSuffix) if mintcore.GitHubOrgPattern.MatchString(org) { - orgs = append(orgs, strings.ToLower(org)) + orgs = append(orgs, org) } } else if mintcore.GitHubOrgPattern.MatchString(part) { - orgs = append(orgs, strings.ToLower(part)) + orgs = append(orgs, part) } } return orgs @@ -1182,9 +1181,7 @@ func (p *Provisioner) ensureWIFPoolAndProvider(ctx context.Context, installingOr attrCondition = buildPublicAttributeCondition() } else { allOrgs = make([]string, len(installingOrgs)) - for i, org := range installingOrgs { - allOrgs[i] = strings.ToLower(org) - } + copy(allOrgs, installingOrgs) } existingProvider, getErr := p.gcpAPI.GetWIFProvider(ctx, projectNumber, p.cfg.WIFPoolName, p.cfg.WIFProvider) if getErr != nil { @@ -1197,19 +1194,21 @@ func (p *Provisioner) ensureWIFPoolAndProvider(ctx context.Context, installingOr if !p.cfg.PublicMint { if existingProvider != nil { existingOrgs := parseConditionOrgs(existingProvider.AttributeCondition) - merged := make(map[string]bool) - for _, org := range allOrgs { + // Case-insensitive dedup: use lowered key, preserve canonical case. + // Installing orgs take precedence over existing ones when casing differs. + merged := make(map[string]string) + for _, org := range existingOrgs { if org != PlaceholderOrg { - merged[org] = true + merged[strings.ToLower(org)] = org } } - for _, org := range existingOrgs { - if org != PlaceholderOrg && !merged[org] { - merged[org] = true + for _, org := range allOrgs { + if org != PlaceholderOrg { + merged[strings.ToLower(org)] = org } } allOrgs = make([]string, 0, len(merged)) - for org := range merged { + for _, org := range merged { allOrgs = append(allOrgs, org) } if len(allOrgs) == 0 { @@ -1234,8 +1233,6 @@ func (p *Provisioner) ensureWIFPoolAndProvider(ctx context.Context, installingOr // GrantOrgVertexAIAccess grants roles/aiplatform.user to an org's .fullsend // repo principal so that enrolled org workflows can call Agent Platform. func (p *Provisioner) GrantOrgVertexAIAccess(ctx context.Context, org string) error { - org = strings.ToLower(org) - projectNumber, err := p.gcpAPI.GetProjectNumber(ctx, p.cfg.ProjectID) if err != nil { return fmt.Errorf("getting project number: %w", err) @@ -1245,7 +1242,6 @@ func (p *Provisioner) GrantOrgVertexAIAccess(ctx context.Context, org string) er } func (p *Provisioner) grantOrgVertexAIAccessWithNumber(ctx context.Context, projectNumber, org string) error { - org = strings.ToLower(org) principal := fmt.Sprintf("principalSet://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s/attribute.repository/%s/.fullsend", projectNumber, p.cfg.WIFPoolName, org) if err := p.gcpAPI.SetProjectIAMBinding(ctx, p.cfg.ProjectID, principal, "roles/aiplatform.user"); err != nil { @@ -1255,7 +1251,6 @@ func (p *Provisioner) grantOrgVertexAIAccessWithNumber(ctx context.Context, proj } func (p *Provisioner) grantRepoVertexAIAccessWithNumber(ctx context.Context, projectNumber, repo string) error { - repo = strings.ToLower(repo) principal := fmt.Sprintf("principalSet://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s/attribute.repository/%s", projectNumber, p.cfg.WIFPoolName, repo) if err := p.gcpAPI.SetProjectIAMBinding(ctx, p.cfg.ProjectID, principal, "roles/aiplatform.user"); err != nil { @@ -1269,8 +1264,6 @@ func (p *Provisioner) grantRepoVertexAIAccessWithNumber(ctx context.Context, pro // Strips the deploy-time placeholder (PlaceholderOrg) if present. // WARNING: read-modify-write without locking — concurrent calls may race. func (p *Provisioner) EnsureOrgInWIFCondition(ctx context.Context, org string) error { - org = strings.ToLower(org) - projectNumber, err := p.gcpAPI.GetProjectNumber(ctx, p.cfg.ProjectID) if err != nil { return fmt.Errorf("getting project number: %w", err) @@ -1285,16 +1278,16 @@ func (p *Provisioner) EnsureOrgInWIFCondition(ctx context.Context, org string) e } existingOrgs := parseConditionOrgs(existing.AttributeCondition) - merged := make(map[string]bool) + merged := make(map[string]string) for _, o := range existingOrgs { if o != PlaceholderOrg { - merged[o] = true + merged[strings.ToLower(o)] = o } } - merged[org] = true + merged[strings.ToLower(org)] = org allOrgs := make([]string, 0, len(merged)) - for o := range merged { + for _, o := range merged { allOrgs = append(allOrgs, o) } sort.Strings(allOrgs) @@ -1315,8 +1308,6 @@ func (p *Provisioner) EnsureOrgInWIFCondition(ctx context.Context, org string) e // attribute condition. // WARNING: read-modify-write without locking — concurrent calls may race. func (p *Provisioner) RemoveOrgFromWIFCondition(ctx context.Context, org string) error { - org = strings.ToLower(org) - projectNumber, err := p.gcpAPI.GetProjectNumber(ctx, p.cfg.ProjectID) if err != nil { return fmt.Errorf("getting project number: %w", err) @@ -1333,7 +1324,7 @@ func (p *Provisioner) RemoveOrgFromWIFCondition(ctx context.Context, org string) existingOrgs := parseConditionOrgs(existing.AttributeCondition) var filtered []string for _, o := range existingOrgs { - if o != org { + if !strings.EqualFold(o, org) { filtered = append(filtered, o) } } @@ -1429,32 +1420,31 @@ func (p *Provisioner) ProvisionWIF(ctx context.Context) (wifProvider string, err return "", fmt.Errorf("duplicate GitHub org after normalization: %q", org) } seen[lower] = true - orgs[i] = lower + orgs[i] = org } var projectNumber string providerID := p.cfg.WIFProvider - repo := strings.ToLower(p.cfg.Repo) if p.cfg.Repo != "" { // Repo-scoped: dedicated provider per repo, no org merge. // Each repo gets a unique provider ID (via BuildRepoProviderID), // so no risk of clobbering another repo's WIF condition. - parts := strings.SplitN(repo, "/", 2) - origParts := strings.SplitN(p.cfg.Repo, "/", 2) + parts := strings.SplitN(p.cfg.Repo, "/", 2) if len(parts) != 2 || parts[0] == "" || parts[1] == "" { return "", fmt.Errorf("repo must be in owner/repo format, got %q", p.cfg.Repo) } - if !mintcore.GitHubOrgPattern.MatchString(parts[0]) || strings.Contains(parts[0], "--") { - return "", fmt.Errorf("invalid repo owner %q: must be a valid GitHub org/user name", origParts[0]) + partsLower := [2]string{strings.ToLower(parts[0]), strings.ToLower(parts[1])} + if !mintcore.GitHubOrgPattern.MatchString(partsLower[0]) || strings.Contains(partsLower[0], "--") { + return "", fmt.Errorf("invalid repo owner %q: must be a valid GitHub org/user name", parts[0]) } - if !githubRepoSlugPattern.MatchString(parts[1]) { - return "", fmt.Errorf("invalid repo name %q: must contain only alphanumeric, hyphens, dots, or underscores", origParts[1]) + if !githubRepoSlugPattern.MatchString(partsLower[1]) { + return "", fmt.Errorf("invalid repo name %q: must contain only alphanumeric, hyphens, dots, or underscores", parts[1]) } - if parts[1] == "." || parts[1] == ".." { - return "", fmt.Errorf("invalid repo name %q: cannot be \".\" or \"..\"", origParts[1]) + if partsLower[1] == "." || partsLower[1] == ".." { + return "", fmt.Errorf("invalid repo name %q: cannot be \".\" or \"..\"", parts[1]) } - if strings.HasSuffix(parts[1], ".git") { - return "", fmt.Errorf("invalid repo name %q: cannot end with \".git\"", origParts[1]) + if strings.HasSuffix(partsLower[1], ".git") { + return "", fmt.Errorf("invalid repo name %q: cannot end with \".git\"", parts[1]) } var err error projectNumber, err = p.gcpAPI.GetProjectNumber(ctx, p.cfg.ProjectID) @@ -1464,8 +1454,8 @@ func (p *Provisioner) ProvisionWIF(ctx context.Context) (wifProvider string, err if err := p.gcpAPI.CreateWIFPool(ctx, projectNumber, p.cfg.WIFPoolName, "Fullsend GitHub OIDC Pool"); err != nil { return "", fmt.Errorf("creating WIF pool: %w", err) } - providerID = mintcore.BuildRepoProviderID(parts[0], parts[1]) - attrCondition := fmt.Sprintf("assertion.repository == '%s'", repo) + providerID = mintcore.BuildRepoProviderID(partsLower[0], partsLower[1]) + attrCondition := fmt.Sprintf("assertion.repository == '%s'", p.cfg.Repo) audiences := []string{oidcAudience, iamAudience(projectNumber, p.cfg.WIFPoolName, providerID)} if err := p.gcpAPI.CreateWIFProvider(ctx, projectNumber, p.cfg.WIFPoolName, providerID, OIDCProviderConfig{ IssuerURI: oidcIssuer, @@ -1484,10 +1474,10 @@ func (p *Provisioner) ProvisionWIF(ctx context.Context) (wifProvider string, err } if p.cfg.Repo != "" { - if err := p.grantRepoVertexAIAccessWithNumber(ctx, projectNumber, repo); err != nil { + if err := p.grantRepoVertexAIAccessWithNumber(ctx, projectNumber, p.cfg.Repo); err != nil { return "", err } - log.Printf("granted roles/aiplatform.user to %s (propagation may take several minutes)", repo) + log.Printf("granted roles/aiplatform.user to %s (propagation may take several minutes)", p.cfg.Repo) } else { for _, org := range orgs { if err := p.grantOrgVertexAIAccessWithNumber(ctx, projectNumber, org); err != nil { diff --git a/internal/dispatch/gcf/provisioner_test.go b/internal/dispatch/gcf/provisioner_test.go index 0cb36d7256..32363125fd 100644 --- a/internal/dispatch/gcf/provisioner_test.go +++ b/internal/dispatch/gcf/provisioner_test.go @@ -1826,7 +1826,7 @@ func TestProvisionWIF_InvalidProjectID(t *testing.T) { assert.Contains(t, err.Error(), "invalid GCP project ID") } -func TestProvisionWIF_NormalizesOrgCase(t *testing.T) { +func TestProvisionWIF_PreservesOrgCase(t *testing.T) { fake := newFakeGCFClient() p := NewProvisioner(Config{ ProjectID: "my-project", @@ -1835,7 +1835,7 @@ func TestProvisionWIF_NormalizesOrgCase(t *testing.T) { _, err := p.ProvisionWIF(context.Background()) require.NoError(t, err) - assert.Equal(t, "assertion.repository_owner == 'acme'", fake.lastWIFProviderConfig.AttributeCondition) + assert.Equal(t, "assertion.repository_owner == 'ACME'", fake.lastWIFProviderConfig.AttributeCondition) } func TestProvisionWIF_RepoScoped(t *testing.T) { @@ -1859,7 +1859,7 @@ func TestProvisionWIF_RepoScoped(t *testing.T) { assert.NotContains(t, fake.calls, "GetWIFProvider") } -func TestProvisionWIF_RepoScoped_LowercasesRepo(t *testing.T) { +func TestProvisionWIF_RepoScoped_PreservesRepoCase(t *testing.T) { fake := newFakeGCFClient() p := NewProvisioner(Config{ ProjectID: "my-project", @@ -1870,8 +1870,9 @@ func TestProvisionWIF_RepoScoped_LowercasesRepo(t *testing.T) { _, err := p.ProvisionWIF(context.Background()) require.NoError(t, err) - assert.Equal(t, "assertion.repository == 'acme/widget'", fake.lastWIFProviderConfig.AttributeCondition) - assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/acme/widget") + assert.Equal(t, "assertion.repository == 'Acme/Widget'", fake.lastWIFProviderConfig.AttributeCondition) + assert.Equal(t, "gh-acme-widget", fake.lastWIFProviderID, "provider ID should still be lowercased") + assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/Acme/Widget") assert.Equal(t, "Acme/Widget", p.cfg.Repo, "ProvisionWIF should not mutate p.cfg.Repo") } @@ -1992,6 +1993,38 @@ func TestProvisionWIF_OrgScoped_MergesExistingOrgs(t *testing.T) { assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/acme/.fullsend") } +func TestProvisionWIF_OrgScoped_PreservesOrgCase(t *testing.T) { + fake := newFakeGCFClient() + p := NewProvisioner(Config{ + ProjectID: "my-project", + GitHubOrgs: []string{"AcmeCorp"}, + }, fake) + + _, err := p.ProvisionWIF(context.Background()) + require.NoError(t, err) + + assert.Equal(t, "assertion.repository_owner == 'AcmeCorp'", fake.lastWIFProviderConfig.AttributeCondition) + assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/AcmeCorp/.fullsend") +} + +func TestProvisionWIF_OrgScoped_MergeDedupsCase(t *testing.T) { + fake := newFakeGCFClient() + fake.wifProvider = &WIFProviderInfo{ + AttributeCondition: "assertion.repository_owner == 'AcmeCorp'", + } + p := NewProvisioner(Config{ + ProjectID: "my-project", + GitHubOrgs: []string{"acmecorp"}, + }, fake) + + _, err := p.ProvisionWIF(context.Background()) + require.NoError(t, err) + + // Installing org's case wins over existing condition's case. + assert.Equal(t, "assertion.repository_owner == 'acmecorp'", + fake.lastWIFProviderConfig.AttributeCondition) +} + func TestProvisionWIF_OrgScoped_GetProviderError_FailsToPreventClobber(t *testing.T) { fake := newFakeGCFClient() fake.errs["GetWIFProvider"] = fmt.Errorf("transient error") @@ -2014,7 +2047,7 @@ func TestParseConditionOrgs(t *testing.T) { {"single org", "assertion.repository_owner == 'acme'", []string{"acme"}}, {"multiple orgs", "assertion.repository_owner in ['alpha', 'beta', 'gamma']", []string{"alpha", "beta", "gamma"}}, {"legacy repo-scoped", "assertion.repository == 'acme/.fullsend'", []string{"acme"}}, - {"mixed case normalized", "assertion.repository_owner in ['AcMe', 'BETA']", []string{"acme", "beta"}}, + {"mixed case preserved", "assertion.repository_owner in ['AcMe', 'BETA']", []string{"AcMe", "BETA"}}, {"empty condition", "", nil}, {"no quoted orgs", "assertion.repository_owner == true", nil}, } @@ -3384,7 +3417,7 @@ func TestEnsureOrgInWIFCondition_AddsOrgAndStripsPlaceholder(t *testing.T) { err := p.EnsureOrgInWIFCondition(context.Background(), "Acme") require.NoError(t, err) assert.Contains(t, fake.(*fakeGCFClient).calls, "UpdateWIFProvider") - assert.Contains(t, fake.(*fakeGCFClient).lastWIFProviderConfig.AttributeCondition, "'acme'") + assert.Contains(t, fake.(*fakeGCFClient).lastWIFProviderConfig.AttributeCondition, "'Acme'") assert.NotContains(t, fake.(*fakeGCFClient).lastWIFProviderConfig.AttributeCondition, PlaceholderOrg) } @@ -3421,6 +3454,24 @@ func TestRemoveOrgFromWIFCondition_RemovesOrgAndAddsPlaceholder(t *testing.T) { assert.NotContains(t, fake.(*fakeGCFClient).lastWIFProviderConfig.AttributeCondition, "'acme'") } +func TestRemoveOrgFromWIFCondition_CaseInsensitiveMatch(t *testing.T) { + fake := NewFakeGCFClient(WithFakeWIFProvider(&WIFProviderInfo{ + AttributeCondition: "assertion.repository_owner in ['AcmeCorp', 'other']", + })) + p := NewProvisioner(Config{ + ProjectID: "proj1", + Region: "us-central1", + WIFPoolName: "fullsend-pool", + WIFProvider: "github-oidc", + }, fake) + + err := p.RemoveOrgFromWIFCondition(context.Background(), "acmecorp") + require.NoError(t, err) + assert.Contains(t, fake.(*fakeGCFClient).calls, "UpdateWIFProvider") + assert.Contains(t, fake.(*fakeGCFClient).lastWIFProviderConfig.AttributeCondition, "'other'") + assert.NotContains(t, fake.(*fakeGCFClient).lastWIFProviderConfig.AttributeCondition, "AcmeCorp") +} + func TestRemoveOrgFromWIFCondition_NoOpWhenOrgAbsent(t *testing.T) { fake := NewFakeGCFClient(WithFakeWIFProvider(&WIFProviderInfo{ AttributeCondition: "assertion.repository_owner in ['other']", From 9a935bdc0aa3df905778e6b999f93271bc44277e Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Wed, 8 Jul 2026 15:49:48 -0400 Subject: [PATCH 2/4] test(gcf): document installing-org-wins precedence on re-enrollment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add TestEnsureOrgInWIFCondition_ReEnrollmentInstallingCaseWins covering the case where an org re-enrolls with different casing than the existing WIF condition — verifies the installing org's case replaces the existing one, per review feedback on #3692. Signed-off-by: Wayne Sun --- internal/dispatch/gcf/provisioner_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/internal/dispatch/gcf/provisioner_test.go b/internal/dispatch/gcf/provisioner_test.go index 32363125fd..cf11b59d1b 100644 --- a/internal/dispatch/gcf/provisioner_test.go +++ b/internal/dispatch/gcf/provisioner_test.go @@ -3436,6 +3436,24 @@ func TestEnsureOrgInWIFCondition_NoOpWhenAlreadyPresent(t *testing.T) { assert.NotContains(t, fake.(*fakeGCFClient).calls, "UpdateWIFProvider") } +func TestEnsureOrgInWIFCondition_ReEnrollmentInstallingCaseWins(t *testing.T) { + fake := NewFakeGCFClient(WithFakeWIFProvider(&WIFProviderInfo{ + AttributeCondition: "assertion.repository_owner == 'acme'", + })) + p := NewProvisioner(Config{ + ProjectID: "proj1", + Region: "us-central1", + WIFPoolName: "fullsend-pool", + WIFProvider: "github-oidc", + }, fake) + + err := p.EnsureOrgInWIFCondition(context.Background(), "ACME") + require.NoError(t, err) + assert.Contains(t, fake.(*fakeGCFClient).calls, "UpdateWIFProvider") + assert.Equal(t, "assertion.repository_owner == 'ACME'", + fake.(*fakeGCFClient).lastWIFProviderConfig.AttributeCondition) +} + func TestRemoveOrgFromWIFCondition_RemovesOrgAndAddsPlaceholder(t *testing.T) { fake := NewFakeGCFClient(WithFakeWIFProvider(&WIFProviderInfo{ AttributeCondition: "assertion.repository_owner in ['acme', 'other']", From e4eab89c5ca168dad311fa616ebbbef89df4a396 Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Thu, 9 Jul 2026 09:33:49 -0400 Subject: [PATCH 3/4] fix(mint): preserve org case for WIF condition on CLI enroll/unenroll runMintEnrollOrg and runMintUnenrollOrg lowercased the org name before calling EnsureOrgInWIFCondition/RemoveOrgFromWIFCondition, silently discarding the canonical case those functions were fixed to preserve. `fullsend mint enroll AcmeCorp` still wrote `acmecorp` into the attributeCondition, which fails GitHub OIDC's byte-for-byte claim match for mixed-case orgs. Keep the lowercased org for ALLOWED_ORGS env var bookkeeping, but pass the original-case org to the WIF condition calls. Assisted-by: Claude Signed-off-by: Wayne Sun --- internal/cli/mint.go | 6 ++++-- internal/cli/mint_test.go | 13 +++++++++++++ internal/dispatch/gcf/fakeclient.go | 4 ++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/internal/cli/mint.go b/internal/cli/mint.go index 7328b39a38..6b6e03fc06 100644 --- a/internal/cli/mint.go +++ b/internal/cli/mint.go @@ -654,6 +654,7 @@ func verifyEnrollment(ctx context.Context, printer *ui.Printer, provisioner enro } func runMintEnrollOrg(ctx context.Context, printer *ui.Printer, org, project, region string, dryRun bool) error { + originalCaseOrg := org org = strings.ToLower(org) if err := validateOrgName(org); err != nil { return err @@ -721,7 +722,7 @@ func runMintEnrollOrg(ctx context.Context, printer *ui.Printer, org, project, re verifyEnrollment(ctx, printer, provisioner, org, project) printer.StepStart("Updating WIF provider condition") - if err := provisioner.EnsureOrgInWIFCondition(ctx, org); err != nil { + if err := provisioner.EnsureOrgInWIFCondition(ctx, originalCaseOrg); err != nil { printer.StepFail("Failed to update WIF condition") return fmt.Errorf("updating WIF condition: %w", err) } @@ -935,6 +936,7 @@ func confirmUnenroll(printer *ui.Printer, target string, reader *bufio.Reader, i } func runMintUnenrollOrg(ctx context.Context, printer *ui.Printer, org, project, region string, dryRun, yolo bool, stdin *os.File) error { + originalCaseOrg := org org = strings.ToLower(org) if err := validateOrgName(org); err != nil { return err @@ -1005,7 +1007,7 @@ func runMintUnenrollOrg(ctx context.Context, printer *ui.Printer, org, project, // Step 3: Remove org from WIF provider condition. printer.StepStart("Updating WIF provider condition") - if err := provisioner.RemoveOrgFromWIFCondition(ctx, org); err != nil { + if err := provisioner.RemoveOrgFromWIFCondition(ctx, originalCaseOrg); err != nil { printer.StepFail("Failed to update WIF condition") return fmt.Errorf("updating WIF condition: %w", err) } diff --git a/internal/cli/mint_test.go b/internal/cli/mint_test.go index d7c5c6f73c..32284a6700 100644 --- a/internal/cli/mint_test.go +++ b/internal/cli/mint_test.go @@ -834,6 +834,19 @@ func TestRunMintEnrollOrg_Success(t *testing.T) { require.NoError(t, err) } +func TestRunMintEnrollOrg_PreservesCaseInWIFCondition(t *testing.T) { + client := mintDiscoveryClient() + withMintGCFClient(t, client) + printer := ui.New(&strings.Builder{}) + err := runMintEnrollOrg(context.Background(), printer, "AcmeCorp", "my-project", "us-central1", false) + require.NoError(t, err) + + updated, err := client.GetWIFProvider(context.Background(), "123456789", "", "") + require.NoError(t, err) + assert.Contains(t, updated.AttributeCondition, "AcmeCorp") + assert.NotContains(t, updated.AttributeCondition, "acmecorp") +} + func TestRunMintEnrollOrg_PublicMode(t *testing.T) { withMintGCFClient(t, publicMintDiscoveryClient()) out := &strings.Builder{} diff --git a/internal/dispatch/gcf/fakeclient.go b/internal/dispatch/gcf/fakeclient.go index b7c6a83a61..ece2f0012b 100644 --- a/internal/dispatch/gcf/fakeclient.go +++ b/internal/dispatch/gcf/fakeclient.go @@ -98,6 +98,10 @@ func (f *fakeGCFClient) GetWIFProvider(_ context.Context, _, _, _ string) (*WIFP } func (f *fakeGCFClient) UpdateWIFProvider(_ context.Context, _, _, _ string, cfg OIDCProviderConfig) error { f.lastWIFProviderConfig = cfg + f.wifProvider = &WIFProviderInfo{ + AttributeCondition: cfg.AttributeCondition, + AllowedAudiences: cfg.AllowedAudiences, + } return f.record("UpdateWIFProvider") } func (f *fakeGCFClient) GetSecret(_ context.Context, _ string, sid string) error { From 27398bb3717bffe7d56950c413eff3cbe8675294 Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Thu, 9 Jul 2026 10:09:53 -0400 Subject: [PATCH 4/4] fix(mint): preserve case in repo-enroll WIF condition and dry-run previews runMintEnrollRepo lowercased repoFullName before it reached ProvisionWIF, so the per-repo attributeCondition and IAM principalSet still got a lowercased owner/repo, reproducing the byte-for-byte STS mismatch this PR fixes -- just via the repo-enroll CLI path instead of the org path. Keep the lowered value for PER_REPO_WIF_REPOS bookkeeping, but pass the original-case repo to ProvisionWIF via gcf.Config.Repo. Also fix two dry-run previews (mint enroll --dry-run, inference provision --dry-run) that printed a lowercased condition even though the real provisioning call now preserves case, which would mislead an operator reviewing the preview before approving a security-sensitive WIF change. Add gcf.LastWIFProviderCondition test helper and regression tests covering the repo-enroll case-preservation and dry-run preview text. Assisted-by: Claude, Gemini Signed-off-by: Wayne Sun --- internal/cli/inference.go | 4 ++-- internal/cli/mint.go | 5 +++-- internal/cli/mint_test.go | 26 ++++++++++++++++++++++---- internal/dispatch/gcf/fakeclient.go | 16 ++++++++++++---- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/internal/cli/inference.go b/internal/cli/inference.go index 38d4d69ba9..f5b57d5114 100644 --- a/internal/cli/inference.go +++ b/internal/cli/inference.go @@ -138,13 +138,13 @@ func runInferenceProvisionDryRun(cmd *cobra.Command, printer *ui.Printer, org, r parts := strings.SplitN(repo, "/", 2) providerID := mintcore.BuildRepoProviderID(parts[0], parts[1]) printer.StepInfo(fmt.Sprintf("WIF provider: %s (repo-scoped)", providerID)) - printer.StepInfo(fmt.Sprintf("Condition: assertion.repository == '%s'", strings.ToLower(repo))) + printer.StepInfo(fmt.Sprintf("Condition: assertion.repository == '%s'", repo)) } else { printer.Header("Dry run: provision WIF for org-scoped inference") printer.Blank() printer.StepInfo(fmt.Sprintf("Organization: %s", org)) printer.StepInfo(fmt.Sprintf("WIF provider: %s (org-scoped)", provider)) - printer.StepInfo(fmt.Sprintf("Condition: assertion.repository_owner == '%s'", strings.ToLower(org))) + printer.StepInfo(fmt.Sprintf("Condition: assertion.repository_owner == '%s'", org)) } printer.Blank() diff --git a/internal/cli/mint.go b/internal/cli/mint.go index 6b6e03fc06..dbffc2b758 100644 --- a/internal/cli/mint.go +++ b/internal/cli/mint.go @@ -706,7 +706,7 @@ func runMintEnrollOrg(ctx context.Context, printer *ui.Printer, org, project, re printer.StepInfo("Dry run — no changes will be made") printer.Blank() printer.StepInfo(fmt.Sprintf(" Would add %s to ALLOWED_ORGS", org)) - printer.StepInfo(fmt.Sprintf(" Would add %s to WIF provider condition", org)) + printer.StepInfo(fmt.Sprintf(" Would add %s to WIF provider condition", originalCaseOrg)) printer.Blank() printer.StepInfo("To grant Agent Platform access, run 'fullsend inference provision' separately") return nil @@ -740,6 +740,7 @@ func runMintEnrollOrg(ctx context.Context, printer *ui.Printer, org, project, re } func runMintEnrollRepo(ctx context.Context, printer *ui.Printer, repoFullName, project, region string, dryRun bool) error { + originalCaseRepo := repoFullName repoFullName = strings.ToLower(repoFullName) parts := strings.SplitN(repoFullName, "/", 2) if len(parts) != 2 || parts[0] == "" || parts[1] == "" { @@ -764,7 +765,7 @@ func runMintEnrollRepo(ctx context.Context, printer *ui.Printer, repoFullName, p ProjectID: project, Region: region, GitHubOrgs: []string{owner}, - Repo: repoFullName, + Repo: originalCaseRepo, }, gcpClient) // Step 1: Discover existing mint. diff --git a/internal/cli/mint_test.go b/internal/cli/mint_test.go index 32284a6700..2d013ab590 100644 --- a/internal/cli/mint_test.go +++ b/internal/cli/mint_test.go @@ -807,6 +807,15 @@ func TestRunMintEnrollOrg_DryRun(t *testing.T) { require.NoError(t, err) } +func TestRunMintEnrollOrg_DryRunPreservesCaseInPreview(t *testing.T) { + withMintGCFClient(t, mintDiscoveryClient()) + out := &strings.Builder{} + printer := ui.New(out) + err := runMintEnrollOrg(context.Background(), printer, "AcmeCorp", "my-project", "us-central1", true) + require.NoError(t, err) + assert.Contains(t, out.String(), "Would add AcmeCorp to WIF provider condition") +} + func TestRunMintEnrollOrg_NoRoleAppIDs(t *testing.T) { withMintGCFClient(t, gcf.NewFakeGCFClient( gcf.WithFakeFunctionInfo(&gcf.FunctionInfo{ @@ -841,10 +850,9 @@ func TestRunMintEnrollOrg_PreservesCaseInWIFCondition(t *testing.T) { err := runMintEnrollOrg(context.Background(), printer, "AcmeCorp", "my-project", "us-central1", false) require.NoError(t, err) - updated, err := client.GetWIFProvider(context.Background(), "123456789", "", "") - require.NoError(t, err) - assert.Contains(t, updated.AttributeCondition, "AcmeCorp") - assert.NotContains(t, updated.AttributeCondition, "acmecorp") + condition := gcf.LastWIFProviderCondition(client) + assert.Contains(t, condition, "AcmeCorp") + assert.NotContains(t, condition, "acmecorp") } func TestRunMintEnrollOrg_PublicMode(t *testing.T) { @@ -1122,6 +1130,16 @@ func TestRunMintEnrollRepo_Success(t *testing.T) { require.NoError(t, err) } +func TestRunMintEnrollRepo_PreservesCaseInWIFCondition(t *testing.T) { + client := mintDiscoveryClient() + withMintGCFClient(t, client) + printer := ui.New(&strings.Builder{}) + err := runMintEnrollRepo(context.Background(), printer, "Acme/Widget", "my-project", "us-central1", false) + require.NoError(t, err) + + assert.Equal(t, "assertion.repository == 'Acme/Widget'", gcf.LastWIFProviderCondition(client)) +} + func TestRunMintEnrollRepo_PublicMode(t *testing.T) { withMintGCFClient(t, publicMintDiscoveryClient()) out := &strings.Builder{} diff --git a/internal/dispatch/gcf/fakeclient.go b/internal/dispatch/gcf/fakeclient.go index ece2f0012b..541dfb4021 100644 --- a/internal/dispatch/gcf/fakeclient.go +++ b/internal/dispatch/gcf/fakeclient.go @@ -98,10 +98,6 @@ func (f *fakeGCFClient) GetWIFProvider(_ context.Context, _, _, _ string) (*WIFP } func (f *fakeGCFClient) UpdateWIFProvider(_ context.Context, _, _, _ string, cfg OIDCProviderConfig) error { f.lastWIFProviderConfig = cfg - f.wifProvider = &WIFProviderInfo{ - AttributeCondition: cfg.AttributeCondition, - AllowedAudiences: cfg.AllowedAudiences, - } return f.record("UpdateWIFProvider") } func (f *fakeGCFClient) GetSecret(_ context.Context, _ string, sid string) error { @@ -300,3 +296,15 @@ func WithFakeErrors(errs map[string]error) FakeGCFOption { func WithFakeWIFProvider(p *WIFProviderInfo) FakeGCFOption { return func(f *fakeGCFClient) { f.wifProvider = p } } + +// LastWIFProviderCondition returns the AttributeCondition passed to the most +// recent CreateWIFProvider or UpdateWIFProvider call on a fake client, for +// cross-package test assertions. Returns "" if client isn't a fake or no +// call was made yet. +func LastWIFProviderCondition(client GCFClient) string { + f, ok := client.(*fakeGCFClient) + if !ok { + return "" + } + return f.lastWIFProviderConfig.AttributeCondition +}