fix(auth): resolve az via shutil.which so azure-cli token works on Windows (#3709)

* fix(auth): resolve az via shutil.which so azure-cli token works on Windows

AzureDevOpsAuth._acquire_via_az_cli runs subprocess.run with a bare "az".
On Windows the Azure CLI is installed as az.cmd, and subprocess.run calls
CreateProcess, which does not consult PATHEXT -- so a bare "az" fails with
WinError 2 even after `az login`, and azure-cli token acquisition silently
returns None (the OSError is swallowed).

Resolve the executable with shutil.which("az") (which honors PATHEXT) before
the call, mirroring the maintainer's own fix in integrations/base.py for the
same CreateProcess/.cmd issue. `or "az"` preserves prior behavior (and the
existing not-installed OSError path) when az is absent. POSIX is unaffected.

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): require an absolute az path so the CWD cannot hijack the lookup

Self-review catch on my own change: resolving with a bare
`shutil.which("az") or "az"` widened an execution surface. On Windows
shutil.which prepends the CURRENT DIRECTORY to the search path (unless
NoDefaultCurrentDirectoryInExePath is set) AND honors PATHEXT, so a stray
.\az.cmd / .\az.bat in the working directory resolves ahead of the real Azure
CLI -- for a credential operation. Verified: with the real az scrubbed from
PATH, shutil.which("az") returns '.\az.CMD'.

Accept the resolution only when it is absolute; otherwise fall back to the bare
"az" (which also preserves the existing not-installed OSError path). A
legitimate install always resolves absolutely, so the Windows .cmd fix this PR
exists for is unaffected. The not-installed and PATHEXT tests are extended with
relative-result cases, all of which fail before this commit.

Note: integrations/base.py resolves executables the same way; hardening that
shared path is a separate concern and is left untouched here.

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

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(auth): build the mocked az path with the host's path rules

Fixes the macOS CI failure. The test hardcoded a Windows absolute path, but the
production code calls os.path.isabs() -- on POSIX runners "C:\Program
Files\..." reads as RELATIVE, so the fallback branch ran and argv[0] was "az"
instead of the resolved path.

Construct the path with os.path.join(os.path.abspath(os.sep), ...) so it is
absolute under the host's rules, and assert against that value. The fallback
test's inputs (".\az.CMD", "az.cmd", "./az") are relative under both ntpath
and posixpath, so they were already portable.

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

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ali jawwad
2026-07-29 03:21:42 +05:00
committed by GitHub
parent 52a6514e38
commit 596a31ee0f
2 changed files with 71 additions and 1 deletions

View File

@@ -5,6 +5,7 @@ from __future__ import annotations
import base64
import json as _json
import os
import shutil
import subprocess
from typing import TYPE_CHECKING
@@ -71,9 +72,27 @@ class AzureDevOpsAuth(AuthProvider):
def _acquire_via_az_cli() -> str | None:
"""Run ``az account get-access-token`` and return the access token."""
try:
# Windows: ``subprocess.run`` calls ``CreateProcess``, which does
# not consult ``PATHEXT``, so a bare ``"az"`` (installed as
# ``az.cmd``) fails with ``WinError 2`` even after ``az login``.
# Resolve via ``shutil.which`` (which honors ``PATHEXT``) so the
# ``.cmd`` shim works. On POSIX this is a harmless lookup that
# returns the same executable.
#
# Require an ABSOLUTE result: on Windows ``shutil.which`` prepends
# the current directory to the search path (unless
# ``NoDefaultCurrentDirectoryInExePath`` is set), so a stray
# ``.\az.cmd`` in the working directory would otherwise be resolved
# ahead of the real Azure CLI and run for a credential operation. A
# legitimate install always resolves to an absolute path, so this
# costs nothing; falling back to the bare ``"az"`` preserves the
# prior behavior (and the existing OSError path) when ``az`` is
# absent.
resolved = shutil.which("az")
az = resolved if resolved and os.path.isabs(resolved) else "az"
result = subprocess.run( # noqa: S603, S607
[
"az",
az,
"account",
"get-access-token",
"--resource",

View File

@@ -535,6 +535,57 @@ class TestAzureDevOpsAuth:
with patch("specify_cli.authentication.azure_devops.subprocess.run", return_value=result):
assert AzureDevOpsAuth().resolve_token(entry) is None
def test_resolve_token_azure_cli_resolves_executable(self):
"""The az executable is resolved via shutil.which before invocation, so
the .cmd/.bat shim on Windows (CreateProcess ignores PATHEXT) is used."""
from unittest.mock import patch, MagicMock
entry = AuthConfigEntry(
hosts=("dev.azure.com",), provider="azure-devops", auth="azure-cli",
)
# Build the absolute path with the HOST's rules: the production code
# calls os.path.isabs(), so a hardcoded Windows path would read as
# RELATIVE on POSIX runners and silently exercise the fallback branch
# instead of the one under test.
resolved_path = os.path.join(os.path.abspath(os.sep), "opt", "az", "az.CMD")
assert os.path.isabs(resolved_path)
result = MagicMock()
result.returncode = 0
result.stdout = '{"accessToken": "tok"}'
with patch(
"specify_cli.authentication.azure_devops.shutil.which",
return_value=resolved_path,
), patch(
"specify_cli.authentication.azure_devops.subprocess.run",
return_value=result,
) as run:
assert AzureDevOpsAuth().resolve_token(entry) == "tok"
argv = run.call_args.args[0]
assert argv[0] == resolved_path
assert argv[1:4] == ["account", "get-access-token", "--resource"]
@pytest.mark.parametrize("which_result", [None, r".\az.CMD", "az.cmd", "./az"])
def test_resolve_token_azure_cli_falls_back_to_bare_az(self, which_result):
"""Fall back to the bare "az" when shutil.which finds nothing OR returns a
NON-ABSOLUTE path. On Windows shutil.which searches the current directory
first, so a stray .\\az.cmd must never be executed for a credential
operation; the bare name also preserves the not-installed OSError path."""
from unittest.mock import patch, MagicMock
entry = AuthConfigEntry(
hosts=("dev.azure.com",), provider="azure-devops", auth="azure-cli",
)
result = MagicMock()
result.returncode = 0
result.stdout = '{"accessToken": "tok"}'
with patch(
"specify_cli.authentication.azure_devops.shutil.which",
return_value=which_result,
), patch(
"specify_cli.authentication.azure_devops.subprocess.run",
return_value=result,
) as run:
assert AzureDevOpsAuth().resolve_token(entry) == "tok"
assert run.call_args.args[0][0] == "az", which_result
def test_resolve_token_azure_cli_not_installed_returns_none(self):
"""azure-cli returns None when az is not installed."""
from unittest.mock import patch