[Storage] az storage blob download-batch: Ensure downloaded blobs stay within the destination directory

[Seb C (Sebby37)]

🤖 PR Validation — 🔄 Running

Breaking Changes	Tests
️✔️ None	🔄 130/130

Related command
az storage blob download-batch

Description
az storage blob download-batch builds each local file path by joining --destination with the blob name returned by the service. The resulting path was never checked against the destination, so a blob whose name is an absolute path (for example a drive-qualified path such as C:/... on Windows) or resolves above the destination directory could be written outside the directory the user specified.

This PR adds a containment check before any file is written:

• A new helper, _get_blob_download_path(), only accepts blob names that normalize to a plain relative path (no drive, root, or leading ./.. component) and joins them onto the destination, so the result always stays inside it. This avoids case-insensitive path comparisons, which Windows per-directory case sensitivity could bypass. Existing symlinks/junctions under the destination are still followed. Drive-relative names such as C:file.txt are rejected on Windows.
• All blob names are validated before any download starts, so the command fails cleanly instead of partially downloading the batch. The check also applies to --dryrun.
• On rejection, a FileOperationError names the offending blob and suggests using --pattern to exclude it, or az storage blob download to download it to an explicit file path.

Downloads where every blob resolves inside the destination are unchanged. Existing symlinks/junctions under the destination are still followed; only the blob name itself is constrained.

Testing Guide
Added test_storage_blob_download_batch.py (mock-based, no recordings required), covering:

• Normal, nested, leading-slash and self-normalizing blob names download inside the destination.
• Names that resolve outside the destination are rejected and nothing is written, including when mixed with valid blobs and with --dryrun.
• Symlink (Linux/macOS) and junction (Windows) escapes.
• --overwrite does not replace existing files outside the destination.
• Windows path semantics, simulated with ntpath so the test runs on every platform, plus a native Windows-only test.
• Re-recorded test_storage_blob_batch_download_scenarios live (preparer switched to kind='StorageV2', since Storage accounts can no longer be created).
• test_storage_blob_batch_oauth_scenarios (live-only) also passes live.

python -m pytest src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py

Existing test_storage_batch_operations.py tests pass in playback. Also manually verified on Linux and on Windows (against the released 2.90.0 install) that affected blob names are rejected with nothing written outside the destination, and that --pattern can still be used to download the remaining blobs.

Live test evidence
All scenario tests that run az storage blob download-batch were run live locally against the latest head:

test_storage_batch_operations.py::StorageBatchOperationScenarios::test_storage_blob_batch_download_scenarios PASSED
test_storage_batch_operations.py::StorageBatchOperationScenarios::test_storage_blob_batch_sas_scenarios PASSED
================= 2 passed, 9 deselected in 403.16s (0:06:43) ==================
test_storage_oauth_track2.py::StorageOauthTests::test_storage_blob_batch_oauth_scenarios PASSED
================= 1 passed, 10 deselected in 143.32s (0:02:23) =================

History Notes

---

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-client-tools-bot-prd]

Hi Seb C (@Sebby37),
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

[Copilot]

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

Adds path-containment validation to az storage blob download-batch, preventing downloads from escaping the destination directory.

Changes:

• Validates resolved paths, including symlink and Windows drive escapes.
• Validates all blobs before downloading, including dry runs.
• Adds cross-platform tests for safe and rejected paths.

File	Description
src/​azure-cli/​azure/​cli/​command_modules/​storage/​tests/​latest/​test_storage_blob_download_batch.py	Tests containment, traversal, symlink, and Windows path scenarios.
src/​azure-cli/​azure/​cli/​command_modules/​storage/​operations/​blob.py	Implements secure download-path validation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[microsoft-github-policy-service]

🔔 Routing this PR to @Azure/act-codegen-extensibility-squad.

[microsoft-github-policy-service]

🔔 Routing this PR to @Azure/act-platform-engineering-squad.

[x-engineering-agent]

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

✅ PASS

Selectors: storage.test_storage_blob_download_batch (module)
PR head ref: fix/storage-blob-download-batch-path-traversal
PR head sha: 49e1782309a86f5b8734f5de172fa5e115e3627a
PR base ref: dev
New test files in PR: true

Changed test files run

src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py

New test files

src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py

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

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: storage.test_storage_blob_download_batch

WARNING: RUNNING TESTS LIVE
The tests are set to run against current profile "latest"
============================= test session starts ==============================
platform linux -- Python 3.12.14, 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
plugins: forked-1.7.5, xdist-3.8.0
collecting ... collected 8 items

azure-cli/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py::TestStorageBlobDownloadBatch::test_download_batch_dryrun_rejects_path_traversal_blob_names PASSED [ 12%]
azure-cli/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py::TestStorageBlobDownloadBatch::test_download_batch_rejects_before_downloading_any_blob PASSED [ 25%]
azure-cli/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py::TestStorageBlobDownloadBatch::test_download_batch_rejects_directory_link_escape PASSED [ 37%]
azure-cli/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py::TestStorageBlobDownloadBatch::test_download_batch_rejects_path_traversal_blob_names PASSED [ 50%]
azure-cli/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py::TestStorageBlobDownloadBatch::test_download_batch_rejects_windows_blob_names_with_simulated_windows_paths PASSED [ 62%]
azure-cli/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py::TestStorageBlobDownloadBatch::test_download_batch_rejects_windows_traversal_blob_names SKIPPED [ 75%]
azure-cli/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py::TestStorageBlobDownloadBatch::test_download_batch_with_overwrite_does_not_replace_files_outside_destination PASSED [ 87%]
azure-cli/src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py::TestStorageBlobDownloadBatch::test_download_batch_writes_blobs_inside_destination PASSED [100%]

- generated xml file: /home/runner/work/issue-sentinel/issue-sentinel/test-output/results.xml -
========================= 7 passed, 1 skipped in 0.27s =========================

Posted by the X Engineering Agent live-test workflow.

[x-engineering-agent]

Seb C (@Sebby37)

Review

Reviewed 49e1782309a86f5b8734f5de172fa5e115e3627a. One Windows filesystem-containment issue needs attention; this is a COMMENT, not an approval.

Upstream CI

All 51/51 checks passed at this head. Reported CI/live-test failures: 0 PR-related, 0 unrelated, 0 uncertain.

Confirmed semantic finding

• P2 — Case-sensitive Windows directories can bypass containment. In src/azure-cli/azure/cli/command_modules/storage/operations/blob.py:450–454, both resolved paths are lowercased before comparison. Windows supports per-directory case sensitivity. If a case-sensitive C:\work contains distinct dest and DEST directories, downloading ../DEST/payload.txt into C:\work\dest resolves to the sibling C:\work\DEST\payload.txt, but the lowercased containment comparison accepts it. Line 462 returns the original-case path; lines 508–517 pass it to download_blob, which opens that path at line 749. Consequently both validation passes accept a write outside the selected destination. This conclusion follows from the changed-line/data-flow inspection; no local reproduction was run.
  Remediation: Check resolved ancestor directory identity using filesystem-aware comparisons that preserve case-sensitive distinctions. Account for Windows commonpath being case-insensitive too; removing normcase alone is insufficient.
  Verify: Extend src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py with native Windows case-sensitive-parent fixtures containing both siblings. Cover normal execution, a valid blob preceding the escaping blob, --dryrun, and --overwrite with an existing sibling-file sentinel. Assert FileOperationError, no download calls, and no sibling creation/overwrite; retain successful mixed-case paths on ordinary case-insensitive Windows. Run this focused file on Windows and retain its POSIX checks.

Test validation

• Live test: Passed. Workflow run
• Regression coverage: Present for storage: 1 focused test file(s) changed.

Review scope and recording assessment

All seven skills were accounted for: release artifact and scope match the two-file change; generated ownership and command/help are not applicable; user intent was traced through filtering before preflight and preservation of original service blob names; test strength and domain edge cases were inspected above. No deterministic repository-policy findings were emitted.

The added tests exercise actual local writes with mocked downloads, including traversal rejection, batch preflight, links, dry-run, and overwrite protection. No recordings changed. Service request/response shapes and successful-download result accumulation are unchanged, so these filesystem checks do not require new service recordings. The case-sensitive Windows regression above is still missing; green CI/live tests and the module-level coverage signal do not establish complete platform coverage.

Risk assessment

26/100 · Medium · High confidence

The Medium rating is driven by security-sensitive behavior.

• Change scope: 2 changed files, 163 changed lines (+162 / -1), including 1 production file.
• Affected components: storage
• Risk drivers: security-sensitive behavior (+28)
• 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 required for storage before merge.

[microsoft-github-policy-service]

🔔 Routing this PR to @Azure/act-codegen-extensibility-squad.

[microsoft-github-policy-service]

🔔 Routing this PR to @Azure/act-platform-engineering-squad.

[Yong Zhang (yonzhan)]

Storage

[x-engineering-agent]

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

❌ FAIL (exit 1)

Selectors: storage.test_storage_batch_operations storage.test_storage_blob_download_batch storage.test_storage_oauth_track2 (module)
PR head ref: fix/storage-blob-download-batch-path-traversal
PR head sha: f116f590a4910615264740f32c056655e5840342
PR base ref: dev
New test files in PR: true

Changed test files run

src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_batch_operations.py
src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py
src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_oauth_track2.py

New test files

src/azure-cli/azure/cli/command_modules/storage/tests/latest/test_storage_blob_download_batch.py

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

Last 80 lines of azdev output

           ^^^^^^^^^^^^^^^^^^^^^
.venv/lib/python3.12/site-packages/azure/mgmt/storage/operations/_operations.py:6851: in begin_create
    raw_result = self._create_initial(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <azure.mgmt.storage.operations._operations.StorageAccountsOperations object at 0x7fc9f2face30>
resource_group_name = 'clitest.rgbctpvdvu4bojepe5vexw7vrbevm7qafhyhnkthsn2hbelggde3ii7wpp32kyvrm5p'
account_name = 'clitest5epnop3s57dj7zms4'
parameters = {'sku': {'name': 'Standard_LRS'}, 'kind': 'Storage', 'location': 'westus', 'properties': {'encryption': {'keySource': 'Microsoft.Storage', 'services': {'blob': {}}}, 'supportsHttpsTrafficOnly': True, 'allowBlobPublicAccess': False}}
kwargs = {}

    def _create_initial(
        self,
        resource_group_name: str,
        account_name: str,
        parameters: Union[_models.StorageAccountCreateParameters, JSON, IO[bytes]],
        **kwargs: Any
    ) -> Iterator[bytes]:
        error_map: MutableMapping = {
            401: ClientAuthenticationError,
            404: ResourceNotFoundError,
            409: ResourceExistsError,
            304: ResourceNotModifiedError,
        }
        error_map.update(kwargs.pop("error_map", {}) or {})
    
        _headers = case_insensitive_dict(kwargs.pop("headers", {}) or {})
        _params = kwargs.pop("params", {}) or {}
    
        content_type: Optional[str] = kwargs.pop("content_type", _headers.pop("Content-Type", None))
        cls: ClsType[Iterator[bytes]] = kwargs.pop("cls", None)
    
        content_type = content_type or "application/json"
        _content = None
        if isinstance(parameters, (IOBase, bytes)):
            _content = parameters
        else:
            _content = json.dumps(parameters, cls=SdkJSONEncoder, exclude_readonly=True)  # type: ignore
    
        _request = build_storage_accounts_create_request(
            resource_group_name=resource_group_name,
            account_name=account_name,
            subscription_id=self._config.subscription_id,
            content_type=content_type,
            api_version=self._config.api_version,
            content=_content,
            headers=_headers,
            params=_params,
        )
        path_format_arguments = {
            "endpoint": self._serialize.url("self._config.base_url", self._config.base_url, "str", skip_quote=True),
        }
        _request.url = self._client.format_url(_request.url, **path_format_arguments)
    
        _decompress = kwargs.pop("decompress", True)
        _stream = True
        pipeline_response: PipelineResponse = self._client._pipeline.run(  # pylint: disable=protected-access
            _request, stream=_stream, **kwargs
        )
    
        response = pipeline_response.http_response
    
        if response.status_code not in [200, 202]:
            try:
                response.read()  # Load the body in memory and close the socket
            except (StreamConsumedError, StreamClosedError):
                pass
            map_error(status_code=response.status_code, response=response, error_map=error_map)
            error = _failsafe_deserialize(
                _models.ErrorResponse,
                response,
            )
>           raise HttpResponseError(response=response, model=error, error_format=ARMErrorFormat)
E           azure.core.exceptions.HttpResponseError: (AccountKindNotSupported) Account Kind Storage is not supported.
E           Code: AccountKindNotSupported
E           Message: Account Kind Storage is not supported.

.venv/lib/python3.12/site-packages/azure/mgmt/storage/operations/_operations.py:6700: HttpResponseError
!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!
============================== 1 failed in 6.63s ===============================

Posted by the X Engineering Agent live-test workflow.

[x-engineering-agent]

Automated sensitive-information remediation ran on this pull request.

• Detected categories: payment card
• Replaced with typed [REDACTED:category] placeholders in: no PR metadata fields
• Comment/review owners notified because X Engineering Agent cannot edit another user's text: @x-engineering-agent[bot]

X Engineering Agent does not modify source files. The PR creator must remove or replace each suspected value at the linked line:

• No changed-file findings

If a credential was exposed, rotate or revoke it immediately. Detected values are never copied into this comment.

✅ Confirm the finding · ❌ Dispute the finding

GitHub only supports a fixed reaction set, so 👍 represents ✅ and 👎 represents ❌. The bot-created reactions are only poll choices.