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#3709jawwad-ali wants to merge 3 commits into
Conversation
…ndows
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>
There was a problem hiding this comment.
Pull request overview
Resolves Azure CLI authentication on Windows by locating the executable shim before invocation.
Changes:
- Resolves
azthroughshutil.which, retaining the existing fallback. - Adds tests for resolved and fallback executable paths.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/authentication/azure_devops.py |
Resolves the Azure CLI executable before token acquisition. |
tests/test_authentication.py |
Tests executable resolution and fallback behavior. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Medium
…ookup
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>
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback and resolve test & lint errors
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>
|
Good catch — that was the macOS CI failure, fixed in 0dd35a7. The hardcoded The path is now built with the host's rules — |
Problem
AzureDevOpsAuth._acquire_via_az_cli()runs:with a bare
"az". On Windows the Azure CLI is installed asaz.cmd, andsubprocess.runcallsCreateProcess, which — unlike a shell — does not consultPATHEXT. So a bare"az"fails withWinError 2 (FileNotFoundError)even whenazworks fine in the user's terminal afteraz login. Theexcept OSErrorswallows it, soauth: azure-clisilently returns no token on Windows.(Verified on a Windows box:
shutil.which("az")→...\wbin\az.CMD, i.e. the runnable target is the.CMDshim, which the bare name never reaches.)Fix
Resolve the executable with
shutil.which("az")(which honorsPATHEXT) before invoking it — exactly the fix the maintainer already applied inintegrations/base.pyfor the same CreateProcess/.cmdproblem:or "az"preserves the previous behaviour (and the existing not-installedOSErrorpath) whenazis absent. On POSIXshutil.whichreturns the same executable, so behaviour is unchanged there.Verification
test_resolve_token_azure_cli_resolves_executable: patchesshutil.which→ a fakeaz.CMDpath and asserts the resolved path isargv[0]. Fails before (noshutilimport to resolve with), passes after.test_resolve_token_azure_cli_falls_back_to_bare_az:shutil.which→Nonestill yieldsargv[0] == "az"(fallback preserved).ruffclean.AI-assisted: authored with Claude Code. I confirmed on Windows that
shutil.which("az")resolves toaz.CMDand mirrored the maintainer's existingintegrations/base.pyresolution.