[Aditya Pujara (a0x1ab)]

🤖 PR Validation — ️✔️ All clear

Breaking Changes	Tests
️✔️ None	️✔️ 130/130

Related command

az aks get-credentials

Description

Fixes #34091.

Implemented by X Engineering Agent using Azure AI Foundry. This is an AI-generated change, published by the configured operator.

Testing Guide

• Run az aks get-credentials.
• Focused regression coverage: src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py.
• Repository CI and the authorized live-test workflow validate the published revision.

History Notes

[AKS] az aks get-credentials: Fix kubelogin conversion for explicit kubeconfig paths

---

This checklist is used to make sure that common guidelines for a pull request are followed.

• The PR title and description has followed the guideline in Submitting Pull Requests.

• I adhere to the Command Guidelines.

• I adhere to the Error Handling Guidelines.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Copilot review overview

🟡 Changes recommended

Resolve the critical risk of launching an unintended kubelogin executable instead of the resolved PATH executable.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Use resolved kubelogin path to prevent unintended executable execution · New

What changed in this PR

Fixes az aks get-credentials kubelogin conversion when an explicit kubeconfig path is used.

Changes:

• Passes the selected kubeconfig path to kubelogin.
• Adds regression coverage for conversion, path handling, failures, and security cases.

File	Summary
src/​azure-cli/​azure/​cli/​command_modules/​acs/​tests/​latest/​test_custom.py	Adds focused regression coverage.
src/​azure-cli/​azure/​cli/​command_modules/​acs/​custom.py	Updates kubelogin conversion invocation.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[FumingZhang]

Aditya Pujara (@a0x1ab) Please take a look at this unresolved executable-resolution issue. The duplicate thread has been removed; please retain the resolved which("kubelogin") result, normalize it with os.path.abspath(), and use that absolute path as argv[0].

[Aditya Pujara (a0x1ab)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).

[Aditya Pujara (a0x1ab)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).

[FumingZhang]

There's still merge conflict Aditya Pujara (@a0x1ab)

[Yong Zhang (yonzhan)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).

[x-engineering-agent]

Live test results — azdev test --live --series (changed test files only)

✅ PASS

Selectors: acs.AksGetCredentialsTest (module)
PR head ref: agent-assist/azure-azure-cli-issue-34091-63d5031c5731
PR head sha: 63ce4617c2d7b65bdaf31d0633bbd077b17c665c
PR base ref: dev
New test files in PR: false

Changed test files run

src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py

Workflow run: https://github.com/Azure/issue-sentinel/actions/runs/38500187536

Last 80 lines of azdev output

=============
| Run Tests |
=============


test index found: /home/runner/.azdev/env_config/home/runner/work/issue-sentinel/issue-sentinel/.venv/test_index/latest.json

Test on modules: acs.AksGetCredentialsTest

WARNING: RUNNING TESTS LIVE
The tests are set to run against current profile "latest"
============================= test session starts ==============================
platform linux -- Python 3.12.15, pytest-9.1.1, pluggy-1.6.0 -- /home/runner/work/issue-sentinel/issue-sentinel/.venv/bin/python
cachedir: .pytest_cache
rootdir: /home/runner/work/issue-sentinel/issue-sentinel/azure-cli/src/azure-cli
configfile: pyproject.toml
plugins: platformdirs-4.13.0, forked-1.7.5, xdist-3.8.0
collecting ... collected 9 items

azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_conversion_failure_warns PASSED [ 11%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_conversion_paths PASSED [ 22%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_converts_selected_kubeconfig PASSED [ 33%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_missing_kubelogin_warns PASSED [ 44%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_nonzero_kubelogin_exit PASSED [ 55%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_preserves_overwrite_context_and_admin PASSED [ 66%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_rejects_symlink_before_conversion PASSED [ 77%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_stdout_skips_conversion PASSED [ 88%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_without_devicecode_skips_conversion PASSED [100%]

- generated xml file: /home/runner/work/issue-sentinel/issue-sentinel/test-output/results.xml -
============================== 9 passed in 1.13s ===============================

Posted by the X Engineering Agent live-test workflow.

[FumingZhang]

Azure/azure-cli#34099 and Azure/azure-cli#34104 implement the same runtime fix; only comments and tests differ. I suggest consolidating into #34099 for its broader test coverage, carrying over useful cases from #34104. Before merging, please also address the executable-resolution feedback: invoke the absolute path returned by which (normalized with os.path.abspath) instead of bare kubelogin, while keeping --kubeconfig path and no cwd override.

[x-engineering-agent]

Aditya Pujara (@a0x1ab)

Review

Upstream CI passed (52 checks), and the Agent live test passed for head 63ce4617c2d7b65bdaf31d0633bbd077b17c665c. One confirmed security finding remains.

Confirmed code finding

Executable selection (high): src/azure-cli/azure/cli/command_modules/acs/custom.py:2069 still passes the bare kubelogin name as argv[0] after discarding which("kubelogin"). On Windows, CreateProcess can select a current-directory executable before the executable found on PATH. Removing cwd also changes that search directory to the caller's working directory. Save the lookup result, normalize it with os.path.abspath, and pass that absolute executable as argv[0]. This addresses the outstanding human feedback.

Update src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py to assert the resolved executable path, including a relative PATH result and a Windows current-directory shadowing case. Existing subprocess-mock assertions require the bare name, and the real-executable tests skip Windows, so they do not catch this risk. Rerun the focused AksGetCredentialsTest cases through the approved validation workflow.

Test validation

• Live test: Passed. Workflow run
• Regression coverage: For acs: 1 focused test file(s) changed. Scenario coverage is unverified; review the issue's required conditions, test setup, and assertions before approval. No linked issue was found in the PR timeline; inspect the source issue and its required scenario manually.

The source issue #34091 requires conversion of the selected kubeconfig from devicecode to azurecli mode. The changed tests cover relative/absolute/spaced paths, KUBECONFIG selection, stdout bypass, missing converter and subprocess failure; their POSIX subprocess fixture asserts the converted file and unrelated-file preservation. No updated recording or actual Windows executable-selection coverage is included; the live workflow result does not establish that missing Windows scenario.

Risk assessment

65/100 - High - High confidence

• Evidence: The changed subprocess call uses an unresolved executable name on a credential-handling path; Windows coverage is absent. CI and POSIX conversion coverage reduce uncertainty for the path-selection fix, not the executable-selection issue.
• Scope: Two files in acs, one production file, with focused tests.
• Required review: Owning AKS squad and security review before merge; no approval is requested until executable resolution and its regression coverage are addressed.

Posted by x-engineering-agent (Reviewer)

[x-engineering-agent]

X Engineering Agent Report

Handed to maintainers

Aditya Pujara (@a0x1ab)

I could not verify this PR's durable Foundry publication and managed head. I cannot safely apply the feedback at #34104 (comment); leaving this PR for manual follow-up.

Posted by x-engineering-agent (Reviewer)

Review — Comments

Aditya Pujara (@a0x1ab)

Upstream CI succeeded, but the executable-resolution finding remains actionable and regression coverage for the issue's real command scenario is not verified.

Finding

Use the resolved kubelogin executable — custom.py:2068-2073 discards which("kubelogin") and launches the bare name. On Windows, a working-directory executable can shadow the PATH-resolved binary and access the selected kubeconfig. Retain the resolved path, preserve the missing-executable warning, apply os.path.abspath(), and use it as argv[0], matching the existing kubectl handling. Copilot feedback 4053329051 and human feedback 4278531502 describe this same unresolved concern.

Focused verification: test_custom.py:181-190 mocks /bin/kubelogin but asserts bare kubelogin. Correct the assertion, cover absolute and relative lookup results, and add a Windows regression verifying a distinct working-directory executable cannot replace the PATH-resolved executable.

Regression coverage

Changed test: src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py; no recording files were supplied. The tests mock the service client and most subprocess calls; two execute a POSIX Python stand-in, not genuine kubelogin. The Agent workflow's nine reported outcomes are these focused tests; they do not establish real AKS authentication or Windows executable behavior. The related-test names in the supplied validation summary below likewise do not prove the issue scenario.

Issue #34091 was retrieved safely despite the timeline lookup below finding no linked issue. Before merge, verify its macOS Apple Silicon/AAD-enabled AKS scenario with genuine kubelogin v0.2.19: run az aks get-credentials with a bare relative --file, subscription selection and --overwrite-existing; verify the selected file converts to azurecli, unrelated kubeconfigs remain unchanged, no empty-path error occurs, and a subsequent kubectl request authenticates without a device-code prompt. Verify the missing-executable warning too. After the fix, rerun AksGetCredentialsTest and retain the focused real-service/executable scenario output through the live-test workflow. Coverage remains gap=false, scenario_status=unknown.

Test validation

• Live test: Passed. 9 of 9 test(s) passed on 6d7db861b10b. Workflow run
• Related command live tests: Ran test_aks_availability_zones_msi, test_aks_availability_zones, test_aks_nodepool_delete_machines, test_aks_nodepool_scale_down_mode for az aks get-credentials, az aks check-acr, which use the changed shared code.
• Regression coverage: For acs: 1 focused test file(s) changed. Scenario coverage is unverified; review the issue's required conditions, test setup, and assertions before approval. No linked issue was found in the PR timeline; inspect the source issue and its required scenario manually.

Risk assessment — 16/100 · Low

The Low rating is driven by public CLI behavior.

• Change scope: 2 changed files, 246 changed lines (+242 / -4), including 1 production file.
• Affected components: acs
• Risk drivers: public CLI behavior (+18)
• Regression evidence: Changed regression tests are included, reducing risk.
• Confidence: High because changed-line patches were available for every production file.
• Required review: Owning-squad review is recommended for acs before merge.

Posted by x-engineering-agent (Reviewer)

Live test — Passed (9/9)

PASS

Selectors: acs.AksGetCredentialsTest (module)
PR head ref: agent-assist/azure-azure-cli-issue-34091-63d5031c5731
PR head sha: 6d7db861b10b3fc64ce74d71f8c9692c1b6dbeaa
PR base ref: dev
New test files in PR: false

Changed test files run

src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py

Workflow run: https://github.com/Azure/issue-sentinel/actions/runs/39805390277

Last 80 lines of azdev output

=============
| Run Tests |
=============


test index found: /home/runner/.azdev/env_config/home/runner/work/issue-sentinel/issue-sentinel/.venv/test_index/latest.json

Test on modules: acs.AksGetCredentialsTest

WARNING: RUNNING TESTS LIVE
The tests are set to run against current profile "latest"
============================= test session starts ==============================
platform linux -- Python 3.12.15, pytest-9.1.1, pluggy-1.7.0 -- /home/runner/work/issue-sentinel/issue-sentinel/.venv/bin/python
cachedir: .pytest_cache
rootdir: /home/runner/work/issue-sentinel/issue-sentinel/azure-cli/src/azure-cli
configfile: pyproject.toml
plugins: forked-1.7.5, xdist-3.8.0, platformdirs-4.13.5
collecting ... collected 9 items

azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_conversion_failure_warns PASSED [ 11%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_conversion_paths PASSED [ 22%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_converts_selected_kubeconfig PASSED [ 33%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_missing_kubelogin_warns PASSED [ 44%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_nonzero_kubelogin_exit PASSED [ 55%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_preserves_overwrite_context_and_admin PASSED [ 66%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_rejects_symlink_before_conversion PASSED [ 77%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_stdout_skips_conversion PASSED [ 88%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AksGetCredentialsTest::test_get_credentials_without_devicecode_skips_conversion PASSED [100%]

- generated xml file: /home/runner/work/issue-sentinel/issue-sentinel/test-output/results.xml -
============================== 9 passed in 1.30s ===============================

Related command live tests
The PR changes shared code (src/azure-cli/azure/cli/command_modules/acs/custom.py) used by az aks get-credentials, az aks check-acr.

• AzureKubernetesServiceScenarioTest.test_aks_availability_zones_msi for az aks get-credentials has no result
• AzureKubernetesServiceScenarioTest.test_aks_availability_zones for az aks get-credentials has no result
• AzureKubernetesServiceScenarioTest.test_aks_nodepool_delete_machines for az aks get-credentials has no result
• AzureKubernetesServiceScenarioTest.test_aks_nodepool_scale_down_mode for az aks get-credentials has no result

Workflow run

Posted by x-engineering-agent (Tester)

[x-engineering-agent]

Automated processing is paused because the local security scan detected sensitive information in this pull request.

• Categories: credential
• Locations: source file src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py line 1264, source file src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py line 1406

Please remove the sensitive values from the PR description, discussion, review comments, and commits. Rewrite the affected commit history if needed, then rotate or revoke any exposed credentials. Automated processing will resume after the PR is clean.

For safety, the detected values are not reproduced here.