Repository navigation
{AKS} Fix desktop archive tests for tarfile path normalization - #34172
Open
FumingZhang wants to merge 2 commits into
Open
FumingZhang wants to merge 2 commits into
FumingZhang wants to merge 2 commits into
Conversation
Accept either native rejection or safe normalized extraction for the parent-symlink fixture. Preserve the outside-sentinel check, mandatory compatibility-extractor rejection, and all other unsafe-member checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
microsoft-github-policy-service
Bot
requested review from
Yu Chen (jsntcy),
ZelinWang (wangzelin007) and
Yong Zhang (yonzhan)
October 6, 2026 03:53
FumingZhang
marked this pull request as ready for review
October 6, 2026 04:13
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused test-only change correctly accommodates differing safe tarfile behavior without weakening containment validation.
Review effort: Balanced
Findings: None
What changed in this PR
Updates AKS Desktop archive tests for portable tar path normalization behavior.
Changes:
- Separates the normalization-sensitive symlink case.
- Accepts safe native extraction or rejection while requiring fallback rejection.
- Verifies destination containment and sentinel integrity.
| File | Description |
|---|---|
src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py |
Adds portable containment coverage for parent-symlink archive paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Resolve the overlapping normalization fixture while preserving the upstream compatibility extractor and stricter unsafe-member case. Require the containment error for both filter API states and verify neither inside output nor the outside sentinel is changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related command
az aks install-desktop(tests only).Description
Fix the current Azure Linux 3.0 RPM test failure in build 357861, on both AMD64 and ARM64. Merge current
dev(7e73b31a0150df02975d120ea57ffa585834ad07) and resolve the overlapping archive-test changes.This PR originally addressed the normalization-sensitive test introduced in #34100. Since then:
test_aks_install_desktop_archive_contains_normalized_traversal, which still expects successful extraction when the standard library normalizesdir/foo/../../outside. This contradicts the strict CLI validator and fails on newer Python packages.Update that existing upstream test to match the current implementation:
FileOperationErrorregardless of the availabletarfile.data_filterbehavior.dir/foo/../../../outsidefixture and all other rejection checks.The net diff against current
devis one test file: 3 insertions and 7 deletions. No production code, CLI behavior, dependency, or pipeline changes beyond those inherited fromdev.Testing Guide
From the repository root with the development dependencies installed:
PYTHONPATH=src/azure-cli:src/azure-cli-core:src/azure-cli-testsdk:src/azure-cli-telemetry \ python -m pytest -q \ src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py \ -k 'aks_install_desktop_archive'Verified locally:
Host Python 3.12.3: 22 passed, 79 subtests passed.
Exact image from the current failing build (
mcr.microsoft.com/azurelinux/base/core@sha256:bfd3e44899fe7c17f6fda42a6ef2a322f2178c2dafb88e69fd87675cdcac39ec) with Python3.12.15-1.azl3: ran the actual pytest tests against the current strict CLI implementation, using the installedtarfileand then each earlier RPM's extracted module:python3-libs-3.12.14-1.azl3python3-libs-3.12.14-2.azl3python3-libs-3.12.15-1.azl3(installed)Mutation check: the updated test fails if extraction silently accepts the fixture, in both filter API states.
Python compilation, changed-line pycodestyle, and
git diff --checkpass.The local commit/push hooks reported that no activated
azdevenvironment was configured; their wrappers still allowed the operations. No hooks were bypassed. The explicit checks above completed successfully; the fullazdevhook suites were not run locally.Pipeline validation
The full-test PR pipeline can exercise the modified unit tests on Python 3.12 and 3.14. However, the Azure Linux RPM build/test jobs in azure-pipelines.yml explicitly exclude
Build.Reason=PullRequest. Ordinary PR checks therefore do not reproduce that packaging environment.For end-to-end packaging confirmation, manually run
Azure.azure-cli(definition 32) against this PR revision, including the Azure Linux 3.0 AMD64/ARM64 RPM build and test jobs. The local container validation above reproduces the current upstream failure and verifies the fix in the affected runtime; it is not a claim that the full packaging pipeline has run.History Notes
Internal test-only change; no customer-facing history note.