Skip to content

Fix #47614 [BUG] AzureCliCredential fails in non-interactive environments du… #47615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hydraxman wants to merge 2 commits into
base: main
Choose a base branch
from
+20 −0
Open

Fix #47614 [BUG] AzureCliCredential fails in non-interactive environments du… #47615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7be66be
Fix [BUG] AzureCliCredential fails in non-interactive environments du…
hydraxman Dec 31, 2025
a87c4f4
Update CHANGELOG.md
hydraxman Dec 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sdk/identity/azure-identity/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

- Fixed `AzureCliCredential` fails in non-interactive environments due to hardcoded `/bin/sh` PATH resolution on MacOS and Linux.

### Other Changes
- Removed unused jetty, redisson, and lettuce-core dependencies.

Expand Down
18 changes: 18 additions & 0 deletions ...ty/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ AccessToken getTokenFromAzureCLIAuthentication(StringBuilder azCommand) {
}

ProcessBuilder builder = new ProcessBuilder(starter, switcher, azCommand.toString());
ensureCliPath(builder);
// Redirects stdin to dev null, helps to avoid messages sent in by the cmd process to upgrade etc.
builder.redirectInput(ProcessBuilder.Redirect.from(IdentityUtil.NULL_FILE));

Expand Down Expand Up @@ -679,6 +680,22 @@ AccessToken getTokenFromAzureCLIAuthentication(StringBuilder azCommand) {
return token;
}

/**
* Ensures that the process environment PATH contains common Azure CLI installation directories
* when invoking CLI commands in non-interactive environments.
* Azure CLI is commonly installed via Homebrew on macOS and Linux, which places the `az` executable
* under locations such as {@code /usr/local/bin} (Intel) or {@code /opt/homebrew/bin} (Apple Silicon).
*/
Comment on lines +684 to +688
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

The JavaDoc comment uses an asterisk (*) at the start of line 684 instead of the standard JavaDoc format with proper indentation and asterisk alignment. The opening /** should be on its own line, and each subsequent line should start with a space, asterisk, and space pattern for consistency with JavaDoc conventions used elsewhere in the codebase.

Suggested change
* Ensures that the process environment PATH contains common Azure CLI installation directories
* when invoking CLI commands in non-interactive environments.
* Azure CLI is commonly installed via Homebrew on macOS and Linux, which places the `az` executable
* under locations such as {@code /usr/local/bin} (Intel) or {@code /opt/homebrew/bin} (Apple Silicon).
*/
* Ensures that the process environment PATH contains common Azure CLI installation directories
* when invoking CLI commands in non-interactive environments.
* Azure CLI is commonly installed via Homebrew on macOS and Linux, which places the `az` executable
* under locations such as {@code /usr/local/bin} (Intel) or {@code /opt/homebrew/bin} (Apple Silicon).
*/

Copilot uses AI. Check for mistakes.
private void ensureCliPath(ProcessBuilder builder) {
if (!isWindowsPlatform()) {
Map<String, String> env = builder.environment();
String path = env.getOrDefault("PATH", "");
if (!path.contains("/usr/local/bin") && !path.contains("/opt/homebrew/bin")) {
env.put("PATH", "/usr/local/bin:/opt/homebrew/bin:" + path);
Comment on lines +692 to +694
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

When constructing the new PATH value, if the original PATH is empty or not set, this will result in PATH being set to /usr/local/bin:/opt/homebrew/bin: with a trailing colon. While this is typically handled gracefully by most systems (treating the empty entry as the current directory), it's not a best practice and could lead to subtle security issues. Consider handling the empty PATH case explicitly to avoid the trailing colon.

Suggested change
String path = env.getOrDefault("PATH", "");
if (!path.contains("/usr/local/bin") && !path.contains("/opt/homebrew/bin")) {
env.put("PATH", "/usr/local/bin:/opt/homebrew/bin:" + path);
String existingPath = env.get("PATH");
String pathToCheck = existingPath == null ? "" : existingPath;
if (!pathToCheck.contains("/usr/local/bin") && !pathToCheck.contains("/opt/homebrew/bin")) {
String additionalPaths = "/usr/local/bin:/opt/homebrew/bin";
if (existingPath == null || existingPath.isEmpty()) {
env.put("PATH", additionalPaths);
} else {
env.put("PATH", additionalPaths + ":" + existingPath);
}

Copilot uses AI. Check for mistakes.
}
Comment on lines +693 to +695
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

The current logic using contains() to check if paths are already in the PATH variable is fragile and can lead to false positives. For example, if PATH contains /home/user/opt/homebrew/bin/tool, the check path.contains("/opt/homebrew/bin") would incorrectly return true even though /opt/homebrew/bin is not actually in the PATH as a separate directory entry. This could prevent the fix from working in such cases.

Consider splitting the PATH by the path separator (: on Unix systems) and checking if the specific directories exist as individual entries in the resulting array.

Suggested change
if (!path.contains("/usr/local/bin") && !path.contains("/opt/homebrew/bin")) {
env.put("PATH", "/usr/local/bin:/opt/homebrew/bin:" + path);
}
boolean hasUsrLocalBin = false;
boolean hasOptHomebrewBin = false;
if (!path.isEmpty()) {
String[] entries = path.split(Pattern.quote(java.io.File.pathSeparator));
for (String entry : entries) {
if ("/usr/local/bin".equals(entry)) {
hasUsrLocalBin = true;
} else if ("/opt/homebrew/bin".equals(entry)) {
hasOptHomebrewBin = true;
}
if (hasUsrLocalBin && hasOptHomebrewBin) {
break;
}
}
}
// Preserve original behavior: only prepend both directories when neither is present.
if (!hasUsrLocalBin && !hasOptHomebrewBin) {
StringBuilder updatedPath = new StringBuilder();
updatedPath.append("/usr/local/bin")
.append(java.io.File.pathSeparator)
.append("/opt/homebrew/bin")
.append(java.io.File.pathSeparator)
.append(path);
env.put("PATH", updatedPath.toString());
}

Copilot uses AI. Check for mistakes.
}
}
Comment on lines +689 to +697
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

The new ensureCliPath method lacks test coverage. There are no tests validating that:

  1. The PATH is correctly modified when the directories are not present
  2. The PATH is not modified when the directories are already present
  3. The method correctly handles edge cases like empty PATH or PATH with trailing colons
  4. The method is correctly invoked for both Azure CLI and Azure Developer CLI commands

Since the existing test file AzureCliCredentialTest.java has comprehensive test coverage for other behaviors, tests should be added for this new functionality to maintain the same quality standard.

Copilot uses AI. Check for mistakes.
Comment on lines +689 to +697
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

The CHANGELOG.md file has not been updated to document this bug fix. According to the contribution guidelines and the checklist in the PR description, the CHANGELOG should be updated for bug fixes. Please add an entry in the "Bugs Fixed" section of the unreleased version (1.19.0-beta.2) describing this fix for AzureCliCredential failing in non-interactive environments.

Copilot uses AI. Check for mistakes.

AccessToken getTokenFromAzureDeveloperCLIAuthentication(StringBuilder azdCommand) {
AccessToken token;
try {
Expand All @@ -693,6 +710,7 @@ AccessToken getTokenFromAzureDeveloperCLIAuthentication(StringBuilder azdCommand
}

ProcessBuilder builder = new ProcessBuilder(starter, switcher, azdCommand.toString());
ensureCliPath(builder);
// Redirects stdin to dev null, helps to avoid messages sent in by the cmd process to upgrade etc.
builder.redirectInput(ProcessBuilder.Redirect.from(IdentityUtil.NULL_FILE));

Expand Down
Loading