From 730660766a5cbcf17c575875cbd1166f5533e871 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 14 Apr 2026 01:58:49 +0000 Subject: [PATCH 01/10] fix: respect customized OIDC subject claims in azd pipeline config When GitHub organizations customize their OIDC subject claim format (e.g. using repository_owner_id and repository_id instead of the default repo:owner/name format), azd pipeline config now queries the GitHub OIDC customization API to determine the actual subject format before creating federated identity credentials. Changes: - Add GetOIDCSubjectConfig() and GetRepoInfo() methods to github.Cli - Add BuildOIDCSubject() helper to construct correct subject strings - Update credentialOptions() to query OIDC config and build subjects - Add user confirmation prompt with option for manual subject override - Graceful fallback to default format when OIDC API is unavailable Based on initial work in PR #7551 by @charris-msft, with fixes for: - Correct repo vs org fallback (only on 404) - No type assertions to concrete struct - Single repo info fetch (no duplicate API calls) - Error on unknown/empty claim keys - Full unit test coverage Fixes #7374 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/pipeline/github_provider.go | 236 ++++++++++++++- cli/azd/pkg/pipeline/github_provider_test.go | 177 +++++++++++ .../pkg/pipeline/pipeline_coverage3_test.go | 18 +- cli/azd/pkg/pipeline/pipeline_helpers_test.go | 49 ++- cli/azd/pkg/tools/github/oidc.go | 187 ++++++++++++ cli/azd/pkg/tools/github/oidc_test.go | 282 ++++++++++++++++++ 6 files changed, 927 insertions(+), 22 deletions(-) create mode 100644 cli/azd/pkg/tools/github/oidc.go create mode 100644 cli/azd/pkg/tools/github/oidc_test.go diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go index b0c3f0cd91e..0d8a5f503fe 100644 --- a/cli/azd/pkg/pipeline/github_provider.go +++ b/cli/azd/pkg/pipeline/github_provider.go @@ -387,29 +387,49 @@ func (p *GitHubCiProvider) credentialOptions( } repoSlug := repoDetails.owner + "/" + repoDetails.repoName - credentialSafeName := credentialNameSanitizer.ReplaceAllString(repoSlug, "-") + credentialSafeName := credentialNameSanitizer.ReplaceAllString( + repoSlug, "-", + ) + + // Query OIDC subject claim customization and build subjects + subjects, err := p.resolveOIDCSubjects( + ctx, repoSlug, branches, + ) + if err != nil { + return nil, fmt.Errorf( + "failed to resolve OIDC subjects: %w", err, + ) + } federatedCredentials := []*graphsdk.FederatedIdentityCredential{ { - Name: fmt.Sprintf("%s-pull_request", credentialSafeName), - Issuer: federatedIdentityIssuer, - Subject: fmt.Sprintf("repo:%s:pull_request", repoSlug), - Description: new("Created by Azure Developer CLI"), - Audiences: []string{federatedIdentityAudience}, + Name: fmt.Sprintf("%s-pull_request", credentialSafeName), + Issuer: federatedIdentityIssuer, + Subject: subjects.pullRequest, + Description: new( + "Created by Azure Developer CLI", + ), + Audiences: []string{federatedIdentityAudience}, }, } for _, branch := range branches { - safeBranchName := credentialNameSanitizer.ReplaceAllString(branch, "-") + safeBranchName := credentialNameSanitizer.ReplaceAllString( + branch, "-", + ) branchCredentials := &graphsdk.FederatedIdentityCredential{ - Name: fmt.Sprintf("%s-%s", credentialSafeName, safeBranchName), - Issuer: federatedIdentityIssuer, - Subject: fmt.Sprintf("repo:%s:ref:refs/heads/%s", repoSlug, branch), - Description: new("Created by Azure Developer CLI"), - Audiences: []string{federatedIdentityAudience}, + Name: fmt.Sprintf("%s-%s", credentialSafeName, safeBranchName), + Issuer: federatedIdentityIssuer, + Subject: subjects.branches[branch], + Description: new( + "Created by Azure Developer CLI", + ), + Audiences: []string{federatedIdentityAudience}, } - federatedCredentials = append(federatedCredentials, branchCredentials) + federatedCredentials = append( + federatedCredentials, branchCredentials, + ) } return &CredentialOptions{ @@ -426,6 +446,196 @@ func (p *GitHubCiProvider) credentialOptions( // *** ciProvider implementation ****** +// oidcSubjects holds the resolved OIDC subject strings for federated credentials. +type oidcSubjects struct { + pullRequest string + branches map[string]string +} + +// resolveOIDCSubjects queries the GitHub OIDC customization API, builds the +// auto-detected subject strings, and optionally prompts the user to confirm +// or override them. +func (p *GitHubCiProvider) resolveOIDCSubjects( + ctx context.Context, repoSlug string, branches []string, +) (*oidcSubjects, error) { + oidcConfig, repoInfo, err := p.detectOIDCConfig(ctx, repoSlug) + if err != nil { + return nil, err + } + + // Build auto-detected subjects + subjects, err := buildAllSubjects( + repoSlug, repoInfo, oidcConfig, branches, + ) + if err != nil { + return nil, err + } + + // In non-interactive mode, use detected subjects without prompting + if p.console.IsNoPromptMode() { + return subjects, nil + } + + return p.promptForSubjects(ctx, repoSlug, oidcConfig, subjects) +} + +// detectOIDCConfig queries the GitHub OIDC customization API and fetches +// repo info if needed. Falls back to default format on API errors. +func (p *GitHubCiProvider) detectOIDCConfig( + ctx context.Context, repoSlug string, +) (*github.OIDCSubjectConfig, *github.RepoInfo, error) { + oidcConfig, err := p.ghCli.GetOIDCSubjectConfig(ctx, repoSlug) + if err != nil { + p.console.Message( + ctx, + fmt.Sprintf( + "Warning: unable to query OIDC subject claim config,"+ + " using default format: %v", err, + ), + ) + return &github.OIDCSubjectConfig{UseDefault: true}, nil, nil + } + + var repoInfo *github.RepoInfo + if !oidcConfig.UseDefault { + repoInfo, err = p.ghCli.GetRepoInfo(ctx, repoSlug) + if err != nil { + return nil, nil, fmt.Errorf( + "failed to get repository info for OIDC"+ + " subject construction: %w", err, + ) + } + } + return oidcConfig, repoInfo, nil +} + +// buildAllSubjects constructs all OIDC subject strings from config. +func buildAllSubjects( + repoSlug string, + repoInfo *github.RepoInfo, + oidcConfig *github.OIDCSubjectConfig, + branches []string, +) (*oidcSubjects, error) { + prSubject, err := github.BuildOIDCSubject( + repoSlug, repoInfo, oidcConfig, "pull_request", + ) + if err != nil { + return nil, fmt.Errorf( + "failed to build OIDC subject for pull requests: %w", err, + ) + } + + branchSubjects := make(map[string]string, len(branches)) + for _, branch := range branches { + subject, err := github.BuildOIDCSubject( + repoSlug, repoInfo, oidcConfig, + fmt.Sprintf("ref:refs/heads/%s", branch), + ) + if err != nil { + return nil, fmt.Errorf( + "failed to build OIDC subject for branch %s: %w", + branch, err, + ) + } + branchSubjects[branch] = subject + } + + return &oidcSubjects{ + pullRequest: prSubject, + branches: branchSubjects, + }, nil +} + +const ( + oidcOptionUseDetected = "Use detected subjects (Recommended)" + oidcOptionCustom = "Enter custom subject manually" +) + +// promptForSubjects shows the detected OIDC subjects and lets the user +// confirm, override, or skip. +func (p *GitHubCiProvider) promptForSubjects( + ctx context.Context, + repoSlug string, + oidcConfig *github.OIDCSubjectConfig, + detected *oidcSubjects, +) (*oidcSubjects, error) { + // Display detected subjects + p.console.Message(ctx, "") + p.console.Message( + ctx, + "Detected OIDC subject format for federated credentials:", + ) + for branch, subject := range detected.branches { + p.console.Message( + ctx, + fmt.Sprintf(" • Branch %s: %s", branch, subject), + ) + } + p.console.Message( + ctx, + fmt.Sprintf(" • Pull request: %s", detected.pullRequest), + ) + p.console.Message(ctx, "") + + options := []string{oidcOptionUseDetected, oidcOptionCustom} + selection, err := p.console.Select(ctx, input.ConsoleOptions{ + Message: "How would you like to configure federated" + + " identity credential subjects?", + Options: options, + }) + if err != nil { + return nil, fmt.Errorf("prompting for OIDC subject choice: %w", err) + } + + switch options[selection] { + case oidcOptionUseDetected: + return detected, nil + case oidcOptionCustom: + return p.promptCustomSubjects(ctx, detected) + } + + return detected, nil +} + +// promptCustomSubjects prompts the user to enter custom OIDC subjects, +// pre-filling with the auto-detected values as defaults. +func (p *GitHubCiProvider) promptCustomSubjects( + ctx context.Context, defaults *oidcSubjects, +) (*oidcSubjects, error) { + result := &oidcSubjects{ + branches: make(map[string]string, len(defaults.branches)), + } + + for branch, defaultSubject := range defaults.branches { + subject, err := p.console.Prompt(ctx, input.ConsoleOptions{ + Message: fmt.Sprintf( + "Enter the OIDC subject for the '%s' branch credential:", + branch, + ), + DefaultValue: defaultSubject, + }) + if err != nil { + return nil, fmt.Errorf( + "prompting for branch %s OIDC subject: %w", branch, err, + ) + } + result.branches[branch] = subject + } + + prSubject, err := p.console.Prompt(ctx, input.ConsoleOptions{ + Message: "Enter the OIDC subject for the pull request credential:", + DefaultValue: defaults.pullRequest, + }) + if err != nil { + return nil, fmt.Errorf( + "prompting for pull request OIDC subject: %w", err, + ) + } + result.pullRequest = prSubject + + return result, nil +} + // configureConnection set up GitHub account with Azure Credentials for // GitHub actions to use a service principal account to log in to Azure // and make changes on behalf of a user. diff --git a/cli/azd/pkg/pipeline/github_provider_test.go b/cli/azd/pkg/pipeline/github_provider_test.go index d8242bb0de5..8b3f3ae9dec 100644 --- a/cli/azd/pkg/pipeline/github_provider_test.go +++ b/cli/azd/pkg/pipeline/github_provider_test.go @@ -138,3 +138,180 @@ func Test_credentialNameSanitizer(t *testing.T) { }) } } + +func Test_credentialOptions_withOIDCCustomSubject(t *testing.T) { + repoDetails := &gitRepositoryDetails{ + owner: "Azure-Samples", + repoName: "my-repo", + branch: "main", + } + repoSlug := "Azure-Samples/my-repo" + + // Helper to set up mock context with no-prompt mode + // (tests run non-interactively so prompts are skipped) + setupMock := func(t *testing.T) *mocks.MockContext { + t.Helper() + mc := mocks.NewMockContext(context.Background()) + setupGithubCliMocks(mc) + mc.Console.SetNoPromptMode(true) + return mc + } + + t.Run("default OIDC subjects when API returns use_default", func(t *testing.T) { + mockContext := setupMock(t) + + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug+ + "/actions/oidc/customization/sub") + }).Respond(exec.NewRunResult( + 0, + `{"use_default": true, "include_claim_keys": []}`, + "", + )) + + provider := createGitHubCiProvider(t, mockContext).(*GitHubCiProvider) + opts, err := provider.credentialOptions( + t.Context(), + repoDetails, + provisioning.Options{}, + AuthTypeFederated, + nil, + ) + require.NoError(t, err) + require.True(t, opts.EnableFederatedCredentials) + require.Len(t, opts.FederatedCredentialOptions, 2) + + prCred := opts.FederatedCredentialOptions[0] + require.Equal(t, + "repo:Azure-Samples/my-repo:pull_request", + prCred.Subject, + ) + + mainCred := opts.FederatedCredentialOptions[1] + require.Equal(t, + "repo:Azure-Samples/my-repo:ref:refs/heads/main", + mainCred.Subject, + ) + }) + + t.Run("custom OIDC subjects with ID-based claims", func(t *testing.T) { + mockContext := setupMock(t) + + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug+ + "/actions/oidc/customization/sub") + }).Respond(exec.NewRunResult( + 0, + `{"use_default": false, "include_claim_keys": `+ + `["repository_owner_id", "repository_id"]}`, + "", + )) + + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug) && + !strings.Contains(cmd, "oidc") + }).Respond(exec.NewRunResult( + 0, + `{"id": 599293758, "owner": {"id": 1844662}}`, + "", + )) + + provider := createGitHubCiProvider(t, mockContext).(*GitHubCiProvider) + opts, err := provider.credentialOptions( + t.Context(), + repoDetails, + provisioning.Options{}, + AuthTypeFederated, + nil, + ) + require.NoError(t, err) + require.True(t, opts.EnableFederatedCredentials) + + prCred := opts.FederatedCredentialOptions[0] + require.Equal(t, + "repository_owner_id:1844662:"+ + "repository_id:599293758:pull_request", + prCred.Subject, + ) + + mainCred := opts.FederatedCredentialOptions[1] + require.Equal(t, + "repository_owner_id:1844662:"+ + "repository_id:599293758:ref:refs/heads/main", + mainCred.Subject, + ) + }) + + t.Run("graceful fallback on OIDC API error", func(t *testing.T) { + mockContext := setupMock(t) + + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "oidc/customization/sub") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 403: Forbidden"), + fmt.Errorf("HTTP 403: Forbidden") + }) + + provider := createGitHubCiProvider(t, mockContext).(*GitHubCiProvider) + opts, err := provider.credentialOptions( + t.Context(), + repoDetails, + provisioning.Options{}, + AuthTypeFederated, + nil, + ) + require.NoError(t, err) + require.True(t, opts.EnableFederatedCredentials) + + prCred := opts.FederatedCredentialOptions[0] + require.Equal(t, + "repo:Azure-Samples/my-repo:pull_request", + prCred.Subject, + ) + }) + + t.Run("multiple branches with custom OIDC", func(t *testing.T) { + mockContext := setupMock(t) + + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/Azure-Samples/my-repo"+ + "/actions/oidc/customization/sub") + }).Respond(exec.NewRunResult( + 0, + `{"use_default": false, "include_claim_keys": `+ + `["repository_owner_id", "repository_id"]}`, + "", + )) + + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/Azure-Samples/my-repo") && + !strings.Contains(cmd, "oidc") + }).Respond(exec.NewRunResult( + 0, + `{"id": 599293758, "owner": {"id": 1844662}}`, + "", + )) + + devDetails := &gitRepositoryDetails{ + owner: "Azure-Samples", + repoName: "my-repo", + branch: "develop", + } + + provider := createGitHubCiProvider(t, mockContext).(*GitHubCiProvider) + opts, err := provider.credentialOptions( + t.Context(), + devDetails, + provisioning.Options{}, + AuthTypeFederated, + nil, + ) + require.NoError(t, err) + // PR + develop + main = 3 credentials + require.Len(t, opts.FederatedCredentialOptions, 3) + + for _, cred := range opts.FederatedCredentialOptions { + require.Contains(t, cred.Subject, "repository_owner_id:1844662") + } + }) +} diff --git a/cli/azd/pkg/pipeline/pipeline_coverage3_test.go b/cli/azd/pkg/pipeline/pipeline_coverage3_test.go index 0c523a5c57f..52e8d3b3d7d 100644 --- a/cli/azd/pkg/pipeline/pipeline_coverage3_test.go +++ b/cli/azd/pkg/pipeline/pipeline_coverage3_test.go @@ -1974,7 +1974,23 @@ func Test_parseAzDoRemote_nonStandardHost(t *testing.T) { func Test_GitHubCiProvider_credentialOptions_branchSpecialChars(t *testing.T) { t.Parallel() - provider := &GitHubCiProvider{} + mockContext := mocks.NewMockContext(t.Context()) + mockContext.Console.SetNoPromptMode(true) + mockContext.CommandRunner.When( + func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "oidc/customization/sub") + }, + ).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 404: Not Found"), + fmt.Errorf("HTTP 404: Not Found") + }) + + provider := &GitHubCiProvider{ + ghCli: github.NewGitHubCli( + mockContext.Console, mockContext.CommandRunner, + ), + console: mockContext.Console, + } opts, err := provider.credentialOptions(t.Context(), &gitRepositoryDetails{ diff --git a/cli/azd/pkg/pipeline/pipeline_helpers_test.go b/cli/azd/pkg/pipeline/pipeline_helpers_test.go index 087a5de4f6f..3654ef6ab78 100644 --- a/cli/azd/pkg/pipeline/pipeline_helpers_test.go +++ b/cli/azd/pkg/pipeline/pipeline_helpers_test.go @@ -5,14 +5,19 @@ package pipeline import ( "context" + "fmt" "os" "path/filepath" + "strings" "testing" "github.com/azure/azure-dev/cli/azd/pkg/config" "github.com/azure/azure-dev/cli/azd/pkg/entraid" + "github.com/azure/azure-dev/cli/azd/pkg/exec" "github.com/azure/azure-dev/cli/azd/pkg/graphsdk" "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" + "github.com/azure/azure-dev/cli/azd/pkg/tools/github" + "github.com/azure/azure-dev/cli/azd/test/mocks" "github.com/microsoft/azure-devops-go-api/azuredevops/v7/build" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -801,11 +806,34 @@ func Test_servicePrincipal(t *testing.T) { // ------------------------------------------------------------------ func Test_GitHubCiProvider_credentialOptions(t *testing.T) { - ctx := t.Context() - provider := &GitHubCiProvider{} + // Helper to create a provider with mocked OIDC API returning default + newProvider := func(t *testing.T) *GitHubCiProvider { + t.Helper() + mockContext := mocks.NewMockContext(t.Context()) + mockContext.Console.SetNoPromptMode(true) + + // Mock OIDC API to return default config for any repo + mockContext.CommandRunner.When( + func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "oidc/customization/sub") + }, + ).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 404: Not Found"), + fmt.Errorf("HTTP 404: Not Found") + }) + + return &GitHubCiProvider{ + ghCli: github.NewGitHubCli( + mockContext.Console, + mockContext.CommandRunner, + ), + console: mockContext.Console, + } + } t.Run("client-credentials auth", func(t *testing.T) { - opts, err := provider.credentialOptions(ctx, + provider := newProvider(t) + opts, err := provider.credentialOptions(t.Context(), &gitRepositoryDetails{ owner: "Azure", repoName: "azure-dev", @@ -820,7 +848,8 @@ func Test_GitHubCiProvider_credentialOptions(t *testing.T) { }) t.Run("federated auth creates credentials", func(t *testing.T) { - opts, err := provider.credentialOptions(ctx, + provider := newProvider(t) + opts, err := provider.credentialOptions(t.Context(), &gitRepositoryDetails{ owner: "Azure", repoName: "azure-dev", @@ -843,7 +872,8 @@ func Test_GitHubCiProvider_credentialOptions(t *testing.T) { t.Run( "federated on main branch - no duplicate", func(t *testing.T) { - opts, err := provider.credentialOptions(ctx, + provider := newProvider(t) + opts, err := provider.credentialOptions(t.Context(), &gitRepositoryDetails{ owner: "Azure", repoName: "azure-dev", @@ -859,7 +889,8 @@ func Test_GitHubCiProvider_credentialOptions(t *testing.T) { t.Run("empty auth type defaults to federated", func(t *testing.T) { - opts, err := provider.credentialOptions(ctx, + provider := newProvider(t) + opts, err := provider.credentialOptions(t.Context(), &gitRepositoryDetails{ owner: "Azure", repoName: "azure-dev", @@ -876,7 +907,8 @@ func Test_GitHubCiProvider_credentialOptions(t *testing.T) { t.Run( "unknown auth type returns empty options", func(t *testing.T) { - opts, err := provider.credentialOptions(ctx, + provider := newProvider(t) + opts, err := provider.credentialOptions(t.Context(), &gitRepositoryDetails{ owner: "Azure", repoName: "azure-dev", @@ -893,7 +925,8 @@ func Test_GitHubCiProvider_credentialOptions(t *testing.T) { t.Run( "federated credential names sanitized", func(t *testing.T) { - opts, err := provider.credentialOptions(ctx, + provider := newProvider(t) + opts, err := provider.credentialOptions(t.Context(), &gitRepositoryDetails{ owner: "my.org", repoName: "my.repo", diff --git a/cli/azd/pkg/tools/github/oidc.go b/cli/azd/pkg/tools/github/oidc.go new file mode 100644 index 00000000000..89bb59c13c9 --- /dev/null +++ b/cli/azd/pkg/tools/github/oidc.go @@ -0,0 +1,187 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package github + +import ( + "context" + "encoding/json" + "fmt" + "strings" +) + +// OIDCSubjectConfig represents the OIDC subject claim customization +// returned by the GitHub Actions OIDC customization API. +type OIDCSubjectConfig struct { + UseDefault bool `json:"use_default"` + IncludeClaimKeys []string `json:"include_claim_keys"` +} + +// RepoInfo holds GitHub API repository metadata needed for OIDC subject construction. +type RepoInfo struct { + ID int `json:"id"` + Owner struct { + ID int `json:"id"` + } `json:"owner"` +} + +// isGitHubNotFoundError returns true if the error indicates a GitHub 404/not-found response. +func isGitHubNotFoundError(err error) bool { + if err == nil { + return false + } + + errText := strings.ToLower(err.Error()) + return strings.Contains(errText, "404") || strings.Contains(errText, "not found") +} + +// GetOIDCSubjectConfig queries the GitHub OIDC customization API for a repository. +// It first checks the repo-level customization. If the repo returns a valid response +// (even with UseDefault=true), it is returned as-is — this handles the case where a repo +// explicitly sets use_default=true to override an org-level customization. +// Only when the repo-level endpoint returns 404 does it fall back to the org-level endpoint. +// If both return 404, it returns a config with UseDefault=true (the default format). +func (cli *Cli) GetOIDCSubjectConfig( + ctx context.Context, repoSlug string, +) (*OIDCSubjectConfig, error) { + // Try repo-level first. + runArgs := cli.newRunArgs( + "api", "/repos/"+repoSlug+"/actions/oidc/customization/sub", + ) + res, err := cli.run(ctx, runArgs) + if err == nil { + var config OIDCSubjectConfig + if jsonErr := json.Unmarshal([]byte(res.Stdout), &config); jsonErr != nil { + return nil, fmt.Errorf( + "failed to parse OIDC config for %s: %w", repoSlug, jsonErr, + ) + } + return &config, nil + } + + if !isGitHubNotFoundError(err) { + return nil, fmt.Errorf( + "failed to query repo-level OIDC config for %s: %w", repoSlug, err, + ) + } + + // Fall back to org-level only when the repo-level endpoint returns 404. + parts := strings.SplitN(repoSlug, "/", 2) + if len(parts) == 2 { + orgRunArgs := cli.newRunArgs( + "api", + "/orgs/"+parts[0]+"/actions/oidc/customization/sub", + ) + orgRes, orgErr := cli.run(ctx, orgRunArgs) + if orgErr == nil { + var config OIDCSubjectConfig + if jsonErr := json.Unmarshal( + []byte(orgRes.Stdout), &config, + ); jsonErr != nil { + return nil, fmt.Errorf( + "failed to parse org OIDC config for %s: %w", + parts[0], jsonErr, + ) + } + return &config, nil + } + + if !isGitHubNotFoundError(orgErr) { + return nil, fmt.Errorf( + "failed to query org-level OIDC config for %s: %w", + parts[0], orgErr, + ) + } + } + + // Default: no customization. + return &OIDCSubjectConfig{UseDefault: true}, nil +} + +// GetRepoInfo queries the GitHub API for repository metadata (IDs) needed to +// construct OIDC subjects with custom claim keys like repository_owner_id. +func (cli *Cli) GetRepoInfo( + ctx context.Context, repoSlug string, +) (*RepoInfo, error) { + runArgs := cli.newRunArgs( + "api", "/repos/"+repoSlug, + "--jq", "{id: .id, owner: {id: .owner.id}}", + ) + res, err := cli.run(ctx, runArgs) + if err != nil { + return nil, fmt.Errorf( + "failed to get repository info for %s: %w", repoSlug, err, + ) + } + + var info RepoInfo + if err := json.Unmarshal([]byte(res.Stdout), &info); err != nil { + return nil, fmt.Errorf( + "failed to parse repository info for %s: %w", repoSlug, err, + ) + } + return &info, nil +} + +// BuildOIDCSubject constructs the correct OIDC subject claim string for a +// federated identity credential based on the OIDC customization config. +// +// This is a pure function — all needed data (repo info, config) must be +// pre-fetched. The suffix is the trailing part of the subject, e.g. +// "ref:refs/heads/main" or "pull_request". +// +// If oidcConfig is nil or UseDefault is true, the default GitHub format is used: +// +// repo:{owner}/{repo}:{suffix} +// +// For custom configs, claim keys are mapped to values and joined with the +// suffix. Unknown claim keys cause an error so the user knows azd needs +// updating. +func BuildOIDCSubject( + repoSlug string, + repoInfo *RepoInfo, + oidcConfig *OIDCSubjectConfig, + suffix string, +) (string, error) { + if oidcConfig == nil || oidcConfig.UseDefault { + return fmt.Sprintf("repo:%s:%s", repoSlug, suffix), nil + } + + if len(oidcConfig.IncludeClaimKeys) == 0 { + return "", fmt.Errorf( + "OIDC config for %s has use_default=false but no"+ + " claim keys specified", repoSlug, + ) + } + + var parts []string + for _, key := range oidcConfig.IncludeClaimKeys { + switch key { + case "repository_owner_id": + parts = append(parts, + fmt.Sprintf("repository_owner_id:%d", repoInfo.Owner.ID), + ) + case "repository_id": + parts = append(parts, + fmt.Sprintf("repository_id:%d", repoInfo.ID), + ) + case "repository_owner": + owner := strings.SplitN(repoSlug, "/", 2) + parts = append(parts, + fmt.Sprintf("repository_owner:%s", owner[0]), + ) + case "repository": + parts = append(parts, + fmt.Sprintf("repository:%s", repoSlug), + ) + default: + return "", fmt.Errorf( + "unsupported OIDC claim key %q in subject"+ + " template for %s — azd may need to be updated", + key, repoSlug, + ) + } + } + parts = append(parts, suffix) + return strings.Join(parts, ":"), nil +} diff --git a/cli/azd/pkg/tools/github/oidc_test.go b/cli/azd/pkg/tools/github/oidc_test.go new file mode 100644 index 00000000000..9f9a73aed6c --- /dev/null +++ b/cli/azd/pkg/tools/github/oidc_test.go @@ -0,0 +1,282 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package github + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/azure/azure-dev/cli/azd/pkg/exec" + "github.com/azure/azure-dev/cli/azd/test/mocks" + "github.com/stretchr/testify/require" +) + +func TestBuildOIDCSubject(t *testing.T) { + repoSlug := "Azure-Samples/my-repo" + repoInfo := &RepoInfo{ + ID: 599293758, + } + repoInfo.Owner.ID = 1844662 + + tests := []struct { + name string + repoSlug string + repoInfo *RepoInfo + oidcConfig *OIDCSubjectConfig + suffix string + want string + wantErr string + }{ + { + name: "nil config uses default format", + repoSlug: repoSlug, + repoInfo: repoInfo, + oidcConfig: nil, + suffix: "ref:refs/heads/main", + want: "repo:Azure-Samples/my-repo:ref:refs/heads/main", + }, + { + name: "use_default true uses default format", + repoSlug: repoSlug, + repoInfo: repoInfo, + oidcConfig: &OIDCSubjectConfig{UseDefault: true}, + suffix: "pull_request", + want: "repo:Azure-Samples/my-repo:pull_request", + }, + { + name: "custom owner_id and repo_id claims", + repoSlug: repoSlug, + repoInfo: repoInfo, + oidcConfig: &OIDCSubjectConfig{ + UseDefault: false, + IncludeClaimKeys: []string{ + "repository_owner_id", + "repository_id", + }, + }, + suffix: "ref:refs/heads/main", + want: "repository_owner_id:1844662:" + + "repository_id:599293758:ref:refs/heads/main", + }, + { + name: "custom owner_id and repo_id for pull_request", + repoSlug: repoSlug, + repoInfo: repoInfo, + oidcConfig: &OIDCSubjectConfig{ + UseDefault: false, + IncludeClaimKeys: []string{ + "repository_owner_id", + "repository_id", + }, + }, + suffix: "pull_request", + want: "repository_owner_id:1844662:" + + "repository_id:599293758:pull_request", + }, + { + name: "custom with repository_owner and repository", + repoSlug: repoSlug, + repoInfo: repoInfo, + oidcConfig: &OIDCSubjectConfig{ + UseDefault: false, + IncludeClaimKeys: []string{ + "repository_owner", + "repository", + }, + }, + suffix: "ref:refs/heads/main", + want: "repository_owner:Azure-Samples:" + + "repository:Azure-Samples/my-repo:" + + "ref:refs/heads/main", + }, + { + name: "empty claim keys with use_default false errors", + repoSlug: repoSlug, + repoInfo: repoInfo, + oidcConfig: &OIDCSubjectConfig{ + UseDefault: false, + IncludeClaimKeys: []string{}, + }, + suffix: "ref:refs/heads/main", + wantErr: "no claim keys specified", + }, + { + name: "unknown claim key errors", + repoSlug: repoSlug, + repoInfo: repoInfo, + oidcConfig: &OIDCSubjectConfig{ + UseDefault: false, + IncludeClaimKeys: []string{ + "repository_owner_id", + "some_future_key", + }, + }, + suffix: "ref:refs/heads/main", + wantErr: "unsupported OIDC claim key", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := BuildOIDCSubject( + tt.repoSlug, tt.repoInfo, tt.oidcConfig, tt.suffix, + ) + if tt.wantErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} + +func TestGetOIDCSubjectConfig(t *testing.T) { + repoSlug := "Azure-Samples/my-repo" + orgName := "Azure-Samples" + + customConfig := OIDCSubjectConfig{ + UseDefault: false, + IncludeClaimKeys: []string{ + "repository_owner_id", "repository_id", + }, + } + customJSON, _ := json.Marshal(customConfig) + + defaultConfig := OIDCSubjectConfig{UseDefault: true} + defaultJSON, _ := json.Marshal(defaultConfig) + + tests := []struct { + name string + setup func(mockContext *mocks.MockContext) + wantConf *OIDCSubjectConfig + wantErr string + }{ + { + name: "repo-level returns custom config", + setup: func(mc *mocks.MockContext) { + mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug+ + "/actions/oidc/customization/sub") + }).Respond(exec.NewRunResult( + 0, string(customJSON), "", + )) + }, + wantConf: &customConfig, + }, + { + name: "repo-level use_default true is returned as-is", + setup: func(mc *mocks.MockContext) { + mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug+ + "/actions/oidc/customization/sub") + }).Respond(exec.NewRunResult( + 0, string(defaultJSON), "", + )) + }, + wantConf: &defaultConfig, + }, + { + name: "repo 404 falls back to org custom config", + setup: func(mc *mocks.MockContext) { + mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug+ + "/actions/oidc/customization/sub") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 404: Not Found"), + fmt.Errorf("HTTP 404: Not Found") + }) + mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/orgs/"+orgName+ + "/actions/oidc/customization/sub") + }).Respond(exec.NewRunResult( + 0, string(customJSON), "", + )) + }, + wantConf: &customConfig, + }, + { + name: "both 404 returns default", + setup: func(mc *mocks.MockContext) { + mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug+ + "/actions/oidc/customization/sub") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 404: Not Found"), + fmt.Errorf("HTTP 404: Not Found") + }) + mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/orgs/"+orgName+ + "/actions/oidc/customization/sub") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 404: Not Found"), + fmt.Errorf("HTTP 404: Not Found") + }) + }, + wantConf: &OIDCSubjectConfig{UseDefault: true}, + }, + { + name: "repo non-404 error is returned", + setup: func(mc *mocks.MockContext) { + mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug+ + "/actions/oidc/customization/sub") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 403: Forbidden"), + fmt.Errorf("HTTP 403: Forbidden") + }) + }, + wantErr: "failed to query repo-level OIDC config", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + tt.setup(mockContext) + + cli := NewGitHubCli( + mockContext.Console, mockContext.CommandRunner, + ) + // Set path so newRunArgs works + cli.path = "gh" + + config, err := cli.GetOIDCSubjectConfig( + t.Context(), repoSlug, + ) + if tt.wantErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + require.Equal(t, tt.wantConf, config) + }) + } +} + +func TestGetRepoInfo(t *testing.T) { + repoSlug := "Azure-Samples/my-repo" + + mockContext := mocks.NewMockContext(context.Background()) + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug) + }).Respond(exec.NewRunResult( + 0, `{"id": 599293758, "owner": {"id": 1844662}}`, "", + )) + + cli := NewGitHubCli( + mockContext.Console, mockContext.CommandRunner, + ) + cli.path = "gh" + + info, err := cli.GetRepoInfo(t.Context(), repoSlug) + require.NoError(t, err) + require.Equal(t, 599293758, info.ID) + require.Equal(t, 1844662, info.Owner.ID) +} From 44bec3a3add109de8d988226849691629d120381 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 14 Apr 2026 02:17:25 +0000 Subject: [PATCH 02/10] fix: address Copilot review feedback (iteration 1) - Remove unused params from promptForSubjects - Sort branch names for deterministic display/prompt ordering - Use int64 for RepoInfo IDs to avoid overflow - Add nil-check for repoInfo in BuildOIDCSubject - Fix misleading comment about skip option - Use t.Context() instead of context.Background() in tests - Fix import ordering in pipeline_helpers_test.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/pipeline/github_provider.go | 16 ++++++++-------- cli/azd/pkg/pipeline/pipeline_helpers_test.go | 7 ++++--- cli/azd/pkg/tools/github/oidc.go | 18 ++++++++++++++++-- cli/azd/pkg/tools/github/oidc_test.go | 9 ++++----- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go index 0d8a5f503fe..5b6e1be67fc 100644 --- a/cli/azd/pkg/pipeline/github_provider.go +++ b/cli/azd/pkg/pipeline/github_provider.go @@ -476,7 +476,7 @@ func (p *GitHubCiProvider) resolveOIDCSubjects( return subjects, nil } - return p.promptForSubjects(ctx, repoSlug, oidcConfig, subjects) + return p.promptForSubjects(ctx, subjects) } // detectOIDCConfig queries the GitHub OIDC customization API and fetches @@ -552,11 +552,9 @@ const ( ) // promptForSubjects shows the detected OIDC subjects and lets the user -// confirm, override, or skip. +// use them or override them. func (p *GitHubCiProvider) promptForSubjects( ctx context.Context, - repoSlug string, - oidcConfig *github.OIDCSubjectConfig, detected *oidcSubjects, ) (*oidcSubjects, error) { // Display detected subjects @@ -565,10 +563,11 @@ func (p *GitHubCiProvider) promptForSubjects( ctx, "Detected OIDC subject format for federated credentials:", ) - for branch, subject := range detected.branches { + branchNames := slices.Sorted(maps.Keys(detected.branches)) + for _, branch := range branchNames { p.console.Message( ctx, - fmt.Sprintf(" • Branch %s: %s", branch, subject), + fmt.Sprintf(" • Branch %s: %s", branch, detected.branches[branch]), ) } p.console.Message( @@ -606,13 +605,14 @@ func (p *GitHubCiProvider) promptCustomSubjects( branches: make(map[string]string, len(defaults.branches)), } - for branch, defaultSubject := range defaults.branches { + branchNames := slices.Sorted(maps.Keys(defaults.branches)) + for _, branch := range branchNames { subject, err := p.console.Prompt(ctx, input.ConsoleOptions{ Message: fmt.Sprintf( "Enter the OIDC subject for the '%s' branch credential:", branch, ), - DefaultValue: defaultSubject, + DefaultValue: defaults.branches[branch], }) if err != nil { return nil, fmt.Errorf( diff --git a/cli/azd/pkg/pipeline/pipeline_helpers_test.go b/cli/azd/pkg/pipeline/pipeline_helpers_test.go index 3654ef6ab78..f963e90245c 100644 --- a/cli/azd/pkg/pipeline/pipeline_helpers_test.go +++ b/cli/azd/pkg/pipeline/pipeline_helpers_test.go @@ -11,6 +11,10 @@ import ( "strings" "testing" + "github.com/microsoft/azure-devops-go-api/azuredevops/v7/build" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/azure/azure-dev/cli/azd/pkg/config" "github.com/azure/azure-dev/cli/azd/pkg/entraid" "github.com/azure/azure-dev/cli/azd/pkg/exec" @@ -18,9 +22,6 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" "github.com/azure/azure-dev/cli/azd/pkg/tools/github" "github.com/azure/azure-dev/cli/azd/test/mocks" - "github.com/microsoft/azure-devops-go-api/azuredevops/v7/build" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // ------------------------------------------------------------------ diff --git a/cli/azd/pkg/tools/github/oidc.go b/cli/azd/pkg/tools/github/oidc.go index 89bb59c13c9..9bf58ff0051 100644 --- a/cli/azd/pkg/tools/github/oidc.go +++ b/cli/azd/pkg/tools/github/oidc.go @@ -19,9 +19,9 @@ type OIDCSubjectConfig struct { // RepoInfo holds GitHub API repository metadata needed for OIDC subject construction. type RepoInfo struct { - ID int `json:"id"` + ID int64 `json:"id"` Owner struct { - ID int `json:"id"` + ID int64 `json:"id"` } `json:"owner"` } @@ -158,10 +158,24 @@ func BuildOIDCSubject( for _, key := range oidcConfig.IncludeClaimKeys { switch key { case "repository_owner_id": + if repoInfo == nil { + return "", fmt.Errorf( + "OIDC config for %s includes claim key %q"+ + " but repository metadata is required", + repoSlug, key, + ) + } parts = append(parts, fmt.Sprintf("repository_owner_id:%d", repoInfo.Owner.ID), ) case "repository_id": + if repoInfo == nil { + return "", fmt.Errorf( + "OIDC config for %s includes claim key %q"+ + " but repository metadata is required", + repoSlug, key, + ) + } parts = append(parts, fmt.Sprintf("repository_id:%d", repoInfo.ID), ) diff --git a/cli/azd/pkg/tools/github/oidc_test.go b/cli/azd/pkg/tools/github/oidc_test.go index 9f9a73aed6c..3e7e7d3e335 100644 --- a/cli/azd/pkg/tools/github/oidc_test.go +++ b/cli/azd/pkg/tools/github/oidc_test.go @@ -4,7 +4,6 @@ package github import ( - "context" "encoding/json" "fmt" "strings" @@ -237,7 +236,7 @@ func TestGetOIDCSubjectConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mockContext := mocks.NewMockContext(context.Background()) + mockContext := mocks.NewMockContext(t.Context()) tt.setup(mockContext) cli := NewGitHubCli( @@ -263,7 +262,7 @@ func TestGetOIDCSubjectConfig(t *testing.T) { func TestGetRepoInfo(t *testing.T) { repoSlug := "Azure-Samples/my-repo" - mockContext := mocks.NewMockContext(context.Background()) + mockContext := mocks.NewMockContext(t.Context()) mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { return strings.Contains(cmd, "/repos/"+repoSlug) }).Respond(exec.NewRunResult( @@ -277,6 +276,6 @@ func TestGetRepoInfo(t *testing.T) { info, err := cli.GetRepoInfo(t.Context(), repoSlug) require.NoError(t, err) - require.Equal(t, 599293758, info.ID) - require.Equal(t, 1844662, info.Owner.ID) + require.Equal(t, int64(599293758), info.ID) + require.Equal(t, int64(1844662), info.Owner.ID) } From bc5406e5ceab289f942b9ed5e11fa25e97e7ac2d Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 14 Apr 2026 02:31:51 +0000 Subject: [PATCH 03/10] fix: address Copilot review feedback (iteration 2) - Tighten isGitHubNotFoundError to match only HTTP 404 signals - Use MessageUxItem with ux.WarningMessage for OIDC fallback warning - Use t.Context() instead of context.Background() in test helpers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/pipeline/github_provider.go | 12 ++++++------ cli/azd/pkg/pipeline/github_provider_test.go | 2 +- cli/azd/pkg/tools/github/oidc.go | 9 +++++++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go index 5b6e1be67fc..fbe5d68138b 100644 --- a/cli/azd/pkg/pipeline/github_provider.go +++ b/cli/azd/pkg/pipeline/github_provider.go @@ -486,13 +486,13 @@ func (p *GitHubCiProvider) detectOIDCConfig( ) (*github.OIDCSubjectConfig, *github.RepoInfo, error) { oidcConfig, err := p.ghCli.GetOIDCSubjectConfig(ctx, repoSlug) if err != nil { - p.console.Message( - ctx, - fmt.Sprintf( - "Warning: unable to query OIDC subject claim config,"+ - " using default format: %v", err, + p.console.MessageUxItem(ctx, &ux.WarningMessage{ + Description: fmt.Sprintf( + "Unable to query OIDC subject claim config;"+ + " using default format. %v", + err, ), - ) + }) return &github.OIDCSubjectConfig{UseDefault: true}, nil, nil } diff --git a/cli/azd/pkg/pipeline/github_provider_test.go b/cli/azd/pkg/pipeline/github_provider_test.go index 8b3f3ae9dec..bfe87773305 100644 --- a/cli/azd/pkg/pipeline/github_provider_test.go +++ b/cli/azd/pkg/pipeline/github_provider_test.go @@ -151,7 +151,7 @@ func Test_credentialOptions_withOIDCCustomSubject(t *testing.T) { // (tests run non-interactively so prompts are skipped) setupMock := func(t *testing.T) *mocks.MockContext { t.Helper() - mc := mocks.NewMockContext(context.Background()) + mc := mocks.NewMockContext(t.Context()) setupGithubCliMocks(mc) mc.Console.SetNoPromptMode(true) return mc diff --git a/cli/azd/pkg/tools/github/oidc.go b/cli/azd/pkg/tools/github/oidc.go index 9bf58ff0051..b5ef9d04f65 100644 --- a/cli/azd/pkg/tools/github/oidc.go +++ b/cli/azd/pkg/tools/github/oidc.go @@ -25,14 +25,19 @@ type RepoInfo struct { } `json:"owner"` } -// isGitHubNotFoundError returns true if the error indicates a GitHub 404/not-found response. +// isGitHubNotFoundError returns true if the error indicates a GitHub HTTP 404 response. func isGitHubNotFoundError(err error) bool { if err == nil { return false } errText := strings.ToLower(err.Error()) - return strings.Contains(errText, "404") || strings.Contains(errText, "not found") + + // Only treat explicit HTTP 404 signals as GitHub "not found" responses. + // Avoid matching generic "not found" text, which can appear in unrelated + // failures such as missing executables, network issues, or permission errors. + return strings.Contains(errText, "http 404") || + strings.Contains(errText, "404 not found") } // GetOIDCSubjectConfig queries the GitHub OIDC customization API for a repository. From 2817626c647a6948e0831d751cfc1c1b295b2c43 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 14 Apr 2026 02:45:41 +0000 Subject: [PATCH 04/10] fix: address Copilot review feedback (iteration 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix import ordering in oidc_test.go (stdlib → external → azd internal) - Only call GetRepoInfo when ID-based claim keys are present Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/pipeline/github_provider.go | 9 ++++++++- cli/azd/pkg/tools/github/oidc_test.go | 3 ++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go index fbe5d68138b..aaece55f4ba 100644 --- a/cli/azd/pkg/pipeline/github_provider.go +++ b/cli/azd/pkg/pipeline/github_provider.go @@ -497,7 +497,7 @@ func (p *GitHubCiProvider) detectOIDCConfig( } var repoInfo *github.RepoInfo - if !oidcConfig.UseDefault { + if !oidcConfig.UseDefault && needsRepoInfo(oidcConfig) { repoInfo, err = p.ghCli.GetRepoInfo(ctx, repoSlug) if err != nil { return nil, nil, fmt.Errorf( @@ -509,6 +509,13 @@ func (p *GitHubCiProvider) detectOIDCConfig( return oidcConfig, repoInfo, nil } +// needsRepoInfo returns true if the OIDC config contains claim keys that +// require numeric repository/owner IDs from the GitHub API. +func needsRepoInfo(config *github.OIDCSubjectConfig) bool { + return slices.Contains(config.IncludeClaimKeys, "repository_owner_id") || + slices.Contains(config.IncludeClaimKeys, "repository_id") +} + // buildAllSubjects constructs all OIDC subject strings from config. func buildAllSubjects( repoSlug string, diff --git a/cli/azd/pkg/tools/github/oidc_test.go b/cli/azd/pkg/tools/github/oidc_test.go index 3e7e7d3e335..a5a1a480880 100644 --- a/cli/azd/pkg/tools/github/oidc_test.go +++ b/cli/azd/pkg/tools/github/oidc_test.go @@ -9,9 +9,10 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" + "github.com/azure/azure-dev/cli/azd/pkg/exec" "github.com/azure/azure-dev/cli/azd/test/mocks" - "github.com/stretchr/testify/require" ) func TestBuildOIDCSubject(t *testing.T) { From 7a4301fe1ef80ca3259a068dd4f440d06b982068 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 14 Apr 2026 02:58:58 +0000 Subject: [PATCH 05/10] fix: address Copilot review feedback (iteration 4) - Validate custom subject inputs: trim whitespace, reject empty values Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/pipeline/github_provider.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go index aaece55f4ba..a42b169e44b 100644 --- a/cli/azd/pkg/pipeline/github_provider.go +++ b/cli/azd/pkg/pipeline/github_provider.go @@ -626,6 +626,12 @@ func (p *GitHubCiProvider) promptCustomSubjects( "prompting for branch %s OIDC subject: %w", branch, err, ) } + subject = strings.TrimSpace(subject) + if subject == "" { + return nil, fmt.Errorf( + "OIDC subject for branch %s cannot be empty", branch, + ) + } result.branches[branch] = subject } @@ -638,6 +644,12 @@ func (p *GitHubCiProvider) promptCustomSubjects( "prompting for pull request OIDC subject: %w", err, ) } + prSubject = strings.TrimSpace(prSubject) + if prSubject == "" { + return nil, fmt.Errorf( + "OIDC subject for pull request cannot be empty", + ) + } result.pullRequest = prSubject return result, nil From 4501dfee7b4562bd1f90e6015ddb1066adfabc4c Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 14 Apr 2026 22:54:14 +0000 Subject: [PATCH 06/10] test: add missing OIDC test coverage per review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add test: repo 404 → org 403 error path in GetOIDCSubjectConfig - Add tests: GetRepoInfo API error and malformed JSON paths - Add code comment noting repoSlug format assumption Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/tools/github/oidc.go | 2 + cli/azd/pkg/tools/github/oidc_test.go | 87 ++++++++++++++++++++++----- 2 files changed, 75 insertions(+), 14 deletions(-) diff --git a/cli/azd/pkg/tools/github/oidc.go b/cli/azd/pkg/tools/github/oidc.go index b5ef9d04f65..98db65b907b 100644 --- a/cli/azd/pkg/tools/github/oidc.go +++ b/cli/azd/pkg/tools/github/oidc.go @@ -185,6 +185,8 @@ func BuildOIDCSubject( fmt.Sprintf("repository_id:%d", repoInfo.ID), ) case "repository_owner": + // repoSlug is always "owner/repo" — constructed by the caller + // from gitRepositoryDetails.owner + "/" + repoDetails.repoName. owner := strings.SplitN(repoSlug, "/", 2) parts = append(parts, fmt.Sprintf("repository_owner:%s", owner[0]), diff --git a/cli/azd/pkg/tools/github/oidc_test.go b/cli/azd/pkg/tools/github/oidc_test.go index a5a1a480880..67f38e58a55 100644 --- a/cli/azd/pkg/tools/github/oidc_test.go +++ b/cli/azd/pkg/tools/github/oidc_test.go @@ -233,6 +233,26 @@ func TestGetOIDCSubjectConfig(t *testing.T) { }, wantErr: "failed to query repo-level OIDC config", }, + { + name: "repo 404 then org non-404 error is returned", + setup: func(mc *mocks.MockContext) { + mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug+ + "/actions/oidc/customization/sub") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 404: Not Found"), + fmt.Errorf("HTTP 404: Not Found") + }) + mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/orgs/"+orgName+ + "/actions/oidc/customization/sub") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 403: Forbidden"), + fmt.Errorf("HTTP 403: Forbidden") + }) + }, + wantErr: "failed to query org-level OIDC config", + }, } for _, tt := range tests { @@ -263,20 +283,59 @@ func TestGetOIDCSubjectConfig(t *testing.T) { func TestGetRepoInfo(t *testing.T) { repoSlug := "Azure-Samples/my-repo" - mockContext := mocks.NewMockContext(t.Context()) - mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { - return strings.Contains(cmd, "/repos/"+repoSlug) - }).Respond(exec.NewRunResult( - 0, `{"id": 599293758, "owner": {"id": 1844662}}`, "", - )) + t.Run("success", func(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug) + }).Respond(exec.NewRunResult( + 0, `{"id": 599293758, "owner": {"id": 1844662}}`, "", + )) + + cli := NewGitHubCli( + mockContext.Console, mockContext.CommandRunner, + ) + cli.path = "gh" + + info, err := cli.GetRepoInfo(t.Context(), repoSlug) + require.NoError(t, err) + require.Equal(t, int64(599293758), info.ID) + require.Equal(t, int64(1844662), info.Owner.ID) + }) + + t.Run("API error", func(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug) + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + return exec.NewRunResult(1, "", "HTTP 403: Forbidden"), + fmt.Errorf("HTTP 403: Forbidden") + }) + + cli := NewGitHubCli( + mockContext.Console, mockContext.CommandRunner, + ) + cli.path = "gh" + + _, err := cli.GetRepoInfo(t.Context(), repoSlug) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to get repository info") + }) + + t.Run("malformed JSON", func(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { + return strings.Contains(cmd, "/repos/"+repoSlug) + }).Respond(exec.NewRunResult( + 0, `{not valid json`, "", + )) - cli := NewGitHubCli( - mockContext.Console, mockContext.CommandRunner, - ) - cli.path = "gh" + cli := NewGitHubCli( + mockContext.Console, mockContext.CommandRunner, + ) + cli.path = "gh" - info, err := cli.GetRepoInfo(t.Context(), repoSlug) - require.NoError(t, err) - require.Equal(t, int64(599293758), info.ID) - require.Equal(t, int64(1844662), info.Owner.ID) + _, err := cli.GetRepoInfo(t.Context(), repoSlug) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to parse repository info") + }) } From dfbf41f60370403663ccc255cc4928f88533997d Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Mon, 27 Apr 2026 20:02:40 +0000 Subject: [PATCH 07/10] fix: remove org-level OIDC fallback per GitHub docs Per GitHub's OIDC subject claim docs, a repo-level 404 means the repository has not opted in to custom subjects. Repos that haven't opted in receive default-format tokens regardless of any org-level template. Applying the org template would create federated credentials with subjects that don't match the actual tokens, causing AADSTS700213. Remove the org fallback: repo 404 now returns default format directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/tools/github/oidc.go | 44 +++++------------------ cli/azd/pkg/tools/github/oidc_test.go | 50 +-------------------------- 2 files changed, 9 insertions(+), 85 deletions(-) diff --git a/cli/azd/pkg/tools/github/oidc.go b/cli/azd/pkg/tools/github/oidc.go index 98db65b907b..233a6e84771 100644 --- a/cli/azd/pkg/tools/github/oidc.go +++ b/cli/azd/pkg/tools/github/oidc.go @@ -41,15 +41,16 @@ func isGitHubNotFoundError(err error) bool { } // GetOIDCSubjectConfig queries the GitHub OIDC customization API for a repository. -// It first checks the repo-level customization. If the repo returns a valid response -// (even with UseDefault=true), it is returned as-is — this handles the case where a repo -// explicitly sets use_default=true to override an org-level customization. -// Only when the repo-level endpoint returns 404 does it fall back to the org-level endpoint. -// If both return 404, it returns a config with UseDefault=true (the default format). +// It checks the repo-level customization endpoint. If the repo returns a valid response +// (even with UseDefault=true), it is returned as-is. +// +// A repo-level 404 means the repository has not opted in to custom OIDC subjects. +// Per GitHub's docs, repos that haven't opted in receive default-format tokens +// regardless of any org-level template, so we return the default config directly +// without checking the org endpoint. func (cli *Cli) GetOIDCSubjectConfig( ctx context.Context, repoSlug string, ) (*OIDCSubjectConfig, error) { - // Try repo-level first. runArgs := cli.newRunArgs( "api", "/repos/"+repoSlug+"/actions/oidc/customization/sub", ) @@ -70,36 +71,7 @@ func (cli *Cli) GetOIDCSubjectConfig( ) } - // Fall back to org-level only when the repo-level endpoint returns 404. - parts := strings.SplitN(repoSlug, "/", 2) - if len(parts) == 2 { - orgRunArgs := cli.newRunArgs( - "api", - "/orgs/"+parts[0]+"/actions/oidc/customization/sub", - ) - orgRes, orgErr := cli.run(ctx, orgRunArgs) - if orgErr == nil { - var config OIDCSubjectConfig - if jsonErr := json.Unmarshal( - []byte(orgRes.Stdout), &config, - ); jsonErr != nil { - return nil, fmt.Errorf( - "failed to parse org OIDC config for %s: %w", - parts[0], jsonErr, - ) - } - return &config, nil - } - - if !isGitHubNotFoundError(orgErr) { - return nil, fmt.Errorf( - "failed to query org-level OIDC config for %s: %w", - parts[0], orgErr, - ) - } - } - - // Default: no customization. + // Repo 404 = not opted in → default format. return &OIDCSubjectConfig{UseDefault: true}, nil } diff --git a/cli/azd/pkg/tools/github/oidc_test.go b/cli/azd/pkg/tools/github/oidc_test.go index 67f38e58a55..aba287817b1 100644 --- a/cli/azd/pkg/tools/github/oidc_test.go +++ b/cli/azd/pkg/tools/github/oidc_test.go @@ -138,8 +138,6 @@ func TestBuildOIDCSubject(t *testing.T) { func TestGetOIDCSubjectConfig(t *testing.T) { repoSlug := "Azure-Samples/my-repo" - orgName := "Azure-Samples" - customConfig := OIDCSubjectConfig{ UseDefault: false, IncludeClaimKeys: []string{ @@ -182,7 +180,7 @@ func TestGetOIDCSubjectConfig(t *testing.T) { wantConf: &defaultConfig, }, { - name: "repo 404 falls back to org custom config", + name: "repo 404 returns default (no org fallback)", setup: func(mc *mocks.MockContext) { mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { return strings.Contains(cmd, "/repos/"+repoSlug+ @@ -191,32 +189,6 @@ func TestGetOIDCSubjectConfig(t *testing.T) { return exec.NewRunResult(1, "", "HTTP 404: Not Found"), fmt.Errorf("HTTP 404: Not Found") }) - mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { - return strings.Contains(cmd, "/orgs/"+orgName+ - "/actions/oidc/customization/sub") - }).Respond(exec.NewRunResult( - 0, string(customJSON), "", - )) - }, - wantConf: &customConfig, - }, - { - name: "both 404 returns default", - setup: func(mc *mocks.MockContext) { - mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { - return strings.Contains(cmd, "/repos/"+repoSlug+ - "/actions/oidc/customization/sub") - }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { - return exec.NewRunResult(1, "", "HTTP 404: Not Found"), - fmt.Errorf("HTTP 404: Not Found") - }) - mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { - return strings.Contains(cmd, "/orgs/"+orgName+ - "/actions/oidc/customization/sub") - }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { - return exec.NewRunResult(1, "", "HTTP 404: Not Found"), - fmt.Errorf("HTTP 404: Not Found") - }) }, wantConf: &OIDCSubjectConfig{UseDefault: true}, }, @@ -233,26 +205,6 @@ func TestGetOIDCSubjectConfig(t *testing.T) { }, wantErr: "failed to query repo-level OIDC config", }, - { - name: "repo 404 then org non-404 error is returned", - setup: func(mc *mocks.MockContext) { - mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { - return strings.Contains(cmd, "/repos/"+repoSlug+ - "/actions/oidc/customization/sub") - }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { - return exec.NewRunResult(1, "", "HTTP 404: Not Found"), - fmt.Errorf("HTTP 404: Not Found") - }) - mc.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { - return strings.Contains(cmd, "/orgs/"+orgName+ - "/actions/oidc/customization/sub") - }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { - return exec.NewRunResult(1, "", "HTTP 403: Forbidden"), - fmt.Errorf("HTTP 403: Forbidden") - }) - }, - wantErr: "failed to query org-level OIDC config", - }, } for _, tt := range tests { From c2c594a54c27af593c199380f64985e06f9990b5 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 28 Apr 2026 17:04:20 +0000 Subject: [PATCH 08/10] fix: propagate OIDC API errors instead of silent fallback Non-404 errors from GetOIDCSubjectConfig (e.g. 403, network) should propagate rather than silently falling back to default subjects, which would create mismatched credentials. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/pipeline/github_provider.go | 11 +++-------- cli/azd/pkg/pipeline/github_provider_test.go | 14 ++++---------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go index a42b169e44b..d4704a4219e 100644 --- a/cli/azd/pkg/pipeline/github_provider.go +++ b/cli/azd/pkg/pipeline/github_provider.go @@ -486,14 +486,9 @@ func (p *GitHubCiProvider) detectOIDCConfig( ) (*github.OIDCSubjectConfig, *github.RepoInfo, error) { oidcConfig, err := p.ghCli.GetOIDCSubjectConfig(ctx, repoSlug) if err != nil { - p.console.MessageUxItem(ctx, &ux.WarningMessage{ - Description: fmt.Sprintf( - "Unable to query OIDC subject claim config;"+ - " using default format. %v", - err, - ), - }) - return &github.OIDCSubjectConfig{UseDefault: true}, nil, nil + return nil, nil, fmt.Errorf( + "failed to query OIDC config for %s: %w", repoSlug, err, + ) } var repoInfo *github.RepoInfo diff --git a/cli/azd/pkg/pipeline/github_provider_test.go b/cli/azd/pkg/pipeline/github_provider_test.go index bfe87773305..0e549a183e0 100644 --- a/cli/azd/pkg/pipeline/github_provider_test.go +++ b/cli/azd/pkg/pipeline/github_provider_test.go @@ -242,7 +242,7 @@ func Test_credentialOptions_withOIDCCustomSubject(t *testing.T) { ) }) - t.Run("graceful fallback on OIDC API error", func(t *testing.T) { + t.Run("error on OIDC API failure", func(t *testing.T) { mockContext := setupMock(t) mockContext.CommandRunner.When(func(args exec.RunArgs, cmd string) bool { @@ -253,21 +253,15 @@ func Test_credentialOptions_withOIDCCustomSubject(t *testing.T) { }) provider := createGitHubCiProvider(t, mockContext).(*GitHubCiProvider) - opts, err := provider.credentialOptions( + _, err := provider.credentialOptions( t.Context(), repoDetails, provisioning.Options{}, AuthTypeFederated, nil, ) - require.NoError(t, err) - require.True(t, opts.EnableFederatedCredentials) - - prCred := opts.FederatedCredentialOptions[0] - require.Equal(t, - "repo:Azure-Samples/my-repo:pull_request", - prCred.Subject, - ) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to query") }) t.Run("multiple branches with custom OIDC", func(t *testing.T) { From 49492b538c0231828624e1fca348534167ef38d5 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 28 Apr 2026 17:05:19 +0000 Subject: [PATCH 09/10] fix: validate repoSlug format in BuildOIDCSubject Add defensive validation for the repository_owner claim key to ensure repoSlug contains a valid owner/repo format before splitting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/tools/github/oidc.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/cli/azd/pkg/tools/github/oidc.go b/cli/azd/pkg/tools/github/oidc.go index 233a6e84771..8c1f4c8ca2c 100644 --- a/cli/azd/pkg/tools/github/oidc.go +++ b/cli/azd/pkg/tools/github/oidc.go @@ -157,11 +157,15 @@ func BuildOIDCSubject( fmt.Sprintf("repository_id:%d", repoInfo.ID), ) case "repository_owner": - // repoSlug is always "owner/repo" — constructed by the caller - // from gitRepositoryDetails.owner + "/" + repoDetails.repoName. - owner := strings.SplitN(repoSlug, "/", 2) + ownerParts := strings.SplitN(repoSlug, "/", 2) + if len(ownerParts) != 2 || ownerParts[0] == "" || ownerParts[1] == "" { + return "", fmt.Errorf( + "invalid repoSlug format: expected 'owner/repo', got %q", + repoSlug, + ) + } parts = append(parts, - fmt.Sprintf("repository_owner:%s", owner[0]), + fmt.Sprintf("repository_owner:%s", ownerParts[0]), ) case "repository": parts = append(parts, From d777e17fa9b79411f05f4ca7173004d12bd47197 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 28 Apr 2026 17:08:50 +0000 Subject: [PATCH 10/10] fix: warn and fall through to manual prompt on unsupported claim keys In interactive mode, if buildAllSubjects fails (e.g. unsupported claim key), warn the user and fall through to the manual prompt with default-format pre-fills. In --no-prompt mode, fail hard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/pipeline/github_provider.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go index d4704a4219e..5a644bde03c 100644 --- a/cli/azd/pkg/pipeline/github_provider.go +++ b/cli/azd/pkg/pipeline/github_provider.go @@ -468,7 +468,27 @@ func (p *GitHubCiProvider) resolveOIDCSubjects( repoSlug, repoInfo, oidcConfig, branches, ) if err != nil { - return nil, err + // In non-interactive mode, fail hard on unsupported claim keys + if p.console.IsNoPromptMode() { + return nil, err + } + + // In interactive mode, warn and fall through to manual prompt + // with default-format pre-fills so the user can enter correct subjects + p.console.MessageUxItem(ctx, &ux.WarningMessage{ + Description: fmt.Sprintf( + "Unable to build OIDC subjects from detected config: %v."+ + " Falling back to default format for manual entry.", + err, + ), + }) + defaultConfig := &github.OIDCSubjectConfig{UseDefault: true} + subjects, err = buildAllSubjects( + repoSlug, nil, defaultConfig, branches, + ) + if err != nil { + return nil, err + } } // In non-interactive mode, use detected subjects without prompting