Skip to content

Fix Windows subscription ID quoting issue in Azure CLI authentication#4368

Merged
EronWright merged 2 commits into
masterfrom
eronwright/fix-windows-subscription-quoting
Oct 22, 2025
Merged

Fix Windows subscription ID quoting issue in Azure CLI authentication#4368
EronWright merged 2 commits into
masterfrom
eronwright/fix-windows-subscription-quoting

Conversation

@EronWright

Copy link
Copy Markdown
Contributor

Summary

Fixes #4319 - Windows users encountered "Subscription '"UUID"' not found" errors when using Azure CLI authentication in v3.8.0.

Root Cause

The issue had two causes:

  1. Provider's az account show command used shell-based execution (cmd.exe /c) with string interpolation, causing Windows to interpret quotes literally in the subscription ID parameter.

  2. Provider always passed subscription IDs to Azure SDK's AzureCLICredential, which has the same shell quoting bug in its az account get-access-token command.

Solution

This PR implements the fix suggested by @stooj in the issue comments, with an additional optimization:

1. Fixed provider's az account show (pkg/provider/azure_cli.go)

  • Replaced shell execution with direct argument arrays: exec.Command("az", args...)
  • Eliminates quote escaping issues entirely across all platforms
  • Simpler, more maintainable code

2. Intelligent subscription handling (pkg/provider/auth_azidentity.go)

  • Query subscription info before creating CLI credential
  • Check the IsDefault field from az account show output
  • Only pass subscription ID to Azure SDK when it's NOT the default
  • Restores pre-3.8 behavior for default subscription users (majority)
  • Works around Azure SDK's shell quoting bug without requiring upstream changes

3. Updated tests (pkg/provider/auth_azidentity_test.go)

  • Added test coverage for default vs non-default subscription handling
  • All provider unit tests pass

Benefits

Fixes Windows CLI authentication - Eliminates quote escaping bug in provider code
Restores pre-3.8 behavior - Default subscription users avoid Azure SDK bug
No upstream dependency - Works around Azure SDK issue without waiting for fix
Respects explicit configuration - Non-default subscriptions still work correctly
Cross-platform compatible - Cleaner code works on all platforms
All tests pass - Full provider test suite passes

Test Plan

  • All provider unit tests pass
  • Added test coverage for default subscription handling
  • Manual verification on Windows would be helpful from affected users

Related Analysis

See issue comments for detailed analysis:

🤖 Generated with Claude Code

This change fixes GitHub issue #4319 where Windows users encountered
"Subscription '"UUID"' not found" errors when using Azure CLI authentication
in v3.8.0.

The issue had two root causes:

1. Provider's `az account show` command used shell-based execution with
   string interpolation, causing Windows to interpret quotes literally:
   `cmd.exe /c "az account show --subscription \"UUID\""` resulted in
   the subscription ID including literal quote characters.

2. The provider always passed subscription IDs to Azure SDK's
   AzureCLICredential, which has the same shell quoting bug.

Changes:

- pkg/provider/azure_cli.go: Replaced shell execution with direct
  argument arrays (exec.Command("az", args...)) to eliminate quote
  escaping issues entirely across all platforms.

- pkg/provider/auth_azidentity.go: Query subscription info before
  creating CLI credential, and only pass subscription ID to Azure SDK
  when it's not the default. This restores pre-3.8 behavior for most
  users while working around the Azure SDK's shell quoting bug.

- pkg/provider/auth_azidentity_test.go: Updated tests to accommodate
  new function signature and added test coverage for default vs
  non-default subscription handling.

Fixes #4319

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@EronWright EronWright requested review from a team and Graham-Pedersen October 22, 2025 22:18
@codecov

codecov Bot commented Oct 22, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.66%. Comparing base (5bec27c) to head (99ef735).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
provider/pkg/provider/auth_azidentity.go 72.72% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4368      +/-   ##
==========================================
- Coverage   59.68%   59.66%   -0.02%     
==========================================
  Files          90       90              
  Lines       14146    14142       -4     
==========================================
- Hits         8443     8438       -5     
  Misses       5045     5045              
- Partials      658      659       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

cliCmd = exec.CommandContext(ctx, "/bin/sh", "-c", commandLine)
cliCmd.Dir = "/bin"
}
cliCmd := exec.CommandContext(ctx, "az", args...)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ahhh much cleaner!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As described in overview, I cribbed the original code from Azure SDK. Why, I wonder, did they use a shell?

@EronWright EronWright enabled auto-merge (squash) October 22, 2025 22:41
@EronWright EronWright disabled auto-merge October 22, 2025 22:41
@EronWright EronWright enabled auto-merge (squash) October 22, 2025 23:08
@EronWright EronWright merged commit d6fb1f5 into master Oct 22, 2025
23 checks passed
@EronWright EronWright deleted the eronwright/fix-windows-subscription-quoting branch October 22, 2025 23:23
@pulumi-bot

Copy link
Copy Markdown
Contributor

This PR has been shipped in release v3.10.0.

3 similar comments
@pulumi-bot

Copy link
Copy Markdown
Contributor

This PR has been shipped in release v3.10.0.

@pulumi-bot

Copy link
Copy Markdown
Contributor

This PR has been shipped in release v3.10.0.

@pulumi-bot

Copy link
Copy Markdown
Contributor

This PR has been shipped in release v3.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subscription not found when creating a resourceGroup

3 participants