[VM] Fix #33979: az vm user update: Fix password parsing so ')' in --password does not break Windows VM password reset - #33980
Conversation
…' in --password does not break Windows VM password reset * Initial plan * [VM] az vm user update: fix ) in password breaking cmd.exe via az.bat GOTO refactor Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
azure-client-tools-agent
Bot
added
the
Azure Client Tools Agent Requested
microsoft-github-policy-service
Bot
added
Compute
Live test results —
|
azure-client-tools-agent
Bot
changed the title
az vm user update: Fix password parsing so ')' in --password does not break Windows VM password resetaz vm user update: Fix password parsing so ')' in --password does not break Windows VM password reset
|
VM |
![azure-client-tools-agent[bot]](https://avatars.githubusercontent.com/in/4205534?s=60&v=4){kind=small}
There was a problem hiding this comment.
Upstream CI
All 64 checks passed; no failures to report.
Test validation
- Live test: Passed (
azdev testagainst the changed VM test file, live run). - Regression coverage: Not applicable — the production change is in
src/azure-cli/az.bat, which is outside theazdev/module regression-coverage scope.
Review-skill findings
test-strength — src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_custom_vm_commands.py (new test test_reset_windows_admin_special_chars_in_password, lines 170-198)
The actual bug fix in this PR is entirely in src/azure-cli/az.bat: the previous IF EXIST (...) ELSE (...) block expanded %* inside a parenthesized block, so any argument containing ) (e.g. --password "Test)123") truncated/corrupted the batch script's command line before Python ever ran. The fix replaces the parenthesized IF/ELSE with a GOTO-based branch, which is the correct fix for this class of cmd.exe parsing bug.
However, the new test only exercises _reset_windows_admin in Python and asserts that protected_settings['Password'] is passed verbatim. That code path never went through az.bat and was not affected by the bug — this test would pass identically whether or not the az.bat fix is present or is reverted. It therefore provides no regression protection for the actual defect being fixed, and would not fail if the az.bat change were rolled back or broken again in the future.
Remediation: Since az.bat argument parsing can't be exercised through azdev/pytest, add a narrow platform-specific regression check instead (or explicitly note in the PR description why none is added), for example a Windows CI step that invokes az.bat --version (or a lightweight command) with a )-containing dummy argument via subprocess and asserts the process starts without a batch parsing error. Alternatively, at minimum call out in the PR description that the fix is verified manually on Windows, since the added Python unit test is not evidence the az.bat fix works or continues to work.
Verification: After adding a batch-level check, rerun it locally on Windows (cmd /c az.bat vm user update ... --password "Test)123") to confirm the script no longer breaks on ).
Risk assessment
0/100 · Low · Low confidence
The Low rating reflects the detected change scope with no elevated security, reliability, customer, operational, dependency, sovereign-cloud, generated-output, or cross-component signal.
- Change scope: 2 changed files, 41 changed lines (
+36/-5), including 0 production files. - Affected components: No production component was identified.
- Risk drivers: No elevated risk signal was detected.
- Regression evidence: No production-code regression-test signal applies.
- Confidence: Low because no production changed-line evidence was available.
- Required review: No additional owning-squad review signal was detected.
azure-client-tools-agent
Bot
added
azure-client-tools-agent
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Fixes a Windows cmd.exe parsing failure where az vm user update --password "Test)123" could break inside az.bat before Python starts, by removing the parenthesized IF (...) ELSE (...) block that embedded %*.
Changes:
- Update
src/azure-cli/az.batto use aGOTO-based flow so)in arguments can’t terminate a parenthesized block duringcmd.exeparsing. - Add a VM unit test that mocks
VMExtensionCreateand verifies passwords containing),(,&,|, and^are passed through toprotected_settingsunchanged.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_custom_vm_commands.py | Adds a regression unit test asserting special-character passwords reach the Windows access extension call path verbatim. |
| src/azure-cli/az.bat | Replaces the parenthesized IF/ELSE block with GOTO labels to avoid cmd.exe block-parsing issues with ) in %*. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@azure-client-tools-agent This is a correct fix but it needs a new live test to evalute the work. Also, it needs align the changes from this az.bat file with the other executable methods of az inclusive of https://github.com/Azure/azure-cli/blob/dev/build_scripts/windows/scripts/az_msi.cmd and https://github.com/Azure/azure-cli/blob/dev/build_scripts/windows/scripts/az_zip.cmd. Check historically, why was this issue not reported before if our file was incorrect all this time? |
Aditya Pujara (a0x1ab)
added
Azure Client Tools Agent Requested
Live test results —
|
azure-client-tools-agent
Bot
added
the
azure-client-tools-agent
There was a problem hiding this comment.
Automated review summary
CI: ✅ All 50/50 checks passed (breaking-change test, full test suite).
Live test: ✅ Reused prior dispatch — azdev test run for the changed test file completed successfully.
Regression coverage: ✅ Not applicable / no gap — the change touches az.bat (batch launcher) and adds a corresponding unit test (test_reset_windows_admin_special_chars_in_password) in the same PR.
Scope: The diff is limited to src/azure-cli/az.bat (replacing the IF EXIST (...) ELSE (...) parenthesis block with a GOTO-based pattern so ) and other cmd.exe metacharacters in --password don't break argument parsing) and a new unit test validating that special-character passwords reach protected_settings verbatim. Both changes match the stated title/description and issue #33919 (az vm user update password parsing fix).
Review-skill findings: none confirmed.
No further action needed from automation; ready for human merge decision.
azure-client-tools-agent
Bot
removed
the
Azure Client Tools Agent Requested
|
🔔 Routing this PR to @Azure/act-observability-squad. |
🤖 PR Validation — ️✔️ All clear
Description
Fixes #33979.
On Windows,
az vm user update --password 'Test)123'fails with123 was unexpected at this time.before Python starts. The)in the password closes theIF EXIST (...) ... ELSE (...)parenthesis block inaz.bat, breaking cmd.exe parsing.Related command
az vm user updateDescription
src/azure-cli/az.bat: Replace theIF EXIST (...) ELSE (...)block—which embeds%*inside parentheses—with a GOTO-based pattern so argument values containing)(or other cmd.exe metacharacters) never terminate a grouping block:test_custom_vm_commands.py: Addtest_reset_windows_admin_special_chars_in_password— mocksVMExtensionCreateand asserts passwords containing),(,&,|, and^reachprotected_settingsverbatim, confirming the Python-level extension call path does not corrupt special characters.Testing Guide
On Windows with cmd.exe or PowerShell, the following should now succeed rather than erroring before any Azure API call:
az vm user update -g myRg -n myWinVm --username AzureUser --password "Test)123"Unit test (offline, no Azure subscription needed):
History Notes
[VM]
az vm user update: Fix)and other shell metacharacters in--passwordbreaking Windows VM password reset on cmd.exe