Skip to content

Commit ef224ca

Browse files
authored
Azure Identity: AzureCliCredentials and AzureDeveloperCliCredential handle stderr may be None when executing subprocess (#39176)
* Azure Identity: Handle stderr may be None for AzureCliCredential According to the docs CredentialUnavailableError.stderr may be None if no output is captured. ``` Stderr output of the child process if it was captured by run(). Otherwise, None. ``` https://docs.python.org/3/library/subprocess.html#subprocess.CalledProcessError In AzureCliCredential._run_command this is handled when propergating the error but was overlooked in other parts of the code. This pr fixes the code to ensure that the None case is handled before checking for str content. * AzureIdentity: AzureDeveloperCliCredential add a test for error handling when AzureDeveloperCliCredential fails to capture stderr * Azure Identity: Handle stderr may be None for AzureDeveloperCliCredential According to the docs CredentialUnavailableError.stderr may be None if no output is captured. ``` Stderr output of the child process if it was captured by run(). Otherwise, None. ``` https://docs.python.org/3/library/subprocess.html#subprocess.CalledProcessError In _run_command this is handled when propergating the error but was overlooked in other parts of the code. This pr fixes the code to ensure that the None case is handled before checking for str content. * AzureIdentity: AzureCliCredential add a test for error handling when AzureCliCredential execution fails to capture stderr * Add changelog for 39176
1 parent cebf572 commit ef224ca

File tree

5 files changed

+33
-4
lines changed

5 files changed

+33
-4
lines changed

sdk/identity/azure-identity/CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
### Bugs Fixed
1212

13+
- A bug in the error handling for AzureCliCredentials and AzureDeveloperCliCredential which would result in the unexpected error `'NoneType' object has no attribute 'startswith'` has been fixed ([#39176](https://github.com/Azure/azure-sdk-for-python/pull/39176))
14+
1315
### Other Changes
1416

1517
## 1.19.0 (2024-10-08)

sdk/identity/azure-identity/azure/identity/_credentials/azd_cli.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,11 @@ def _run_command(command: str, timeout: int) -> str:
261261
except subprocess.CalledProcessError as ex:
262262
# non-zero return from shell
263263
# Fallback check in case the executable is not found while executing subprocess.
264-
if ex.returncode == 127 or ex.stderr.startswith("'azd' is not recognized"):
264+
if ex.returncode == 127 or (ex.stderr is not None and ex.stderr.startswith("'azd' is not recognized")):
265265
raise CredentialUnavailableError(message=CLI_NOT_FOUND) from ex
266-
if "not logged in, run `azd auth login` to login" in ex.stderr and "AADSTS" not in ex.stderr:
266+
if ex.stderr is not None and (
267+
"not logged in, run `azd auth login` to login" in ex.stderr and "AADSTS" not in ex.stderr
268+
):
267269
raise CredentialUnavailableError(message=NOT_LOGGED_IN) from ex
268270

269271
# return code is from the CLI -> propagate its output

sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,11 @@ def _run_command(command: str, timeout: int) -> str:
252252
except subprocess.CalledProcessError as ex:
253253
# non-zero return from shell
254254
# Fallback check in case the executable is not found while executing subprocess.
255-
if ex.returncode == 127 or ex.stderr.startswith("'az' is not recognized"):
255+
if ex.returncode == 127 or (ex.stderr is not None and ex.stderr.startswith("'az' is not recognized")):
256256
raise CredentialUnavailableError(message=CLI_NOT_FOUND) from ex
257-
if ("az login" in ex.stderr or "az account set" in ex.stderr) and "AADSTS" not in ex.stderr:
257+
if ex.stderr is not None and (
258+
("az login" in ex.stderr or "az account set" in ex.stderr) and "AADSTS" not in ex.stderr
259+
):
258260
raise CredentialUnavailableError(message=NOT_LOGGED_IN) from ex
259261

260262
# return code is from the CLI -> propagate its output

sdk/identity/azure-identity/tests/test_azd_cli_credential.py

+11
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,17 @@ def test_unexpected_error(get_token_method):
147147
getattr(AzureDeveloperCliCredential(), get_token_method)("scope")
148148

149149

150+
@pytest.mark.parametrize("get_token_method", GET_TOKEN_METHODS)
151+
def test_unexpected_error_no_strerr(get_token_method):
152+
"""When the CLI returns an unexpected error with no stderr captured, the credential should raise an error with a str output"""
153+
stderr = None
154+
default_message = "Failed to invoke Azure Developer CLI"
155+
with mock.patch("shutil.which", return_value="azd"):
156+
with mock.patch(CHECK_OUTPUT, raise_called_process_error(42, stderr=stderr)):
157+
with pytest.raises(ClientAuthenticationError, match=default_message):
158+
getattr(AzureDeveloperCliCredential(), get_token_method)("scope")
159+
160+
150161
@pytest.mark.parametrize("output", TEST_ERROR_OUTPUTS)
151162
@pytest.mark.parametrize("get_token_method", GET_TOKEN_METHODS)
152163
def test_parsing_error_does_not_expose_token(output, get_token_method):

sdk/identity/azure-identity/tests/test_cli_credential.py

+12
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,18 @@ def test_unexpected_error(get_token_method):
230230
getattr(AzureCliCredential(), get_token_method)("scope")
231231

232232

233+
@pytest.mark.parametrize("get_token_method", GET_TOKEN_METHODS)
234+
def test_unexpected_error_no_stderr(get_token_method):
235+
"""When the CLI returns an unexpected error with no stderr captured, the credential should raise an error with a str output"""
236+
237+
stderr = None
238+
default_message = "Failed to invoke Azure CLI"
239+
with mock.patch("shutil.which", return_value="az"):
240+
with mock.patch(CHECK_OUTPUT, raise_called_process_error(42, stderr=stderr)):
241+
with pytest.raises(ClientAuthenticationError, match=stderr):
242+
getattr(AzureCliCredential(), get_token_method)("scope")
243+
244+
233245
@pytest.mark.parametrize("output,get_token_method", product(TEST_ERROR_OUTPUTS, GET_TOKEN_METHODS))
234246
def test_parsing_error_does_not_expose_token(output, get_token_method):
235247
"""Errors during CLI output parsing shouldn't expose access tokens in that output"""

0 commit comments

Comments
 (0)