Skip to content

{Storage} Validate blob copy source endpoint - #34138

Open
YangAn-microsoft wants to merge 6 commits into
Azure:devfrom
YangAn-microsoft:fix/storage-blob-copy-source-endpoint
Open

YangAn-microsoft wants to merge 6 commits into
Azure:devfrom
YangAn-microsoft:fix/storage-blob-copy-source-endpoint

Conversation

@YangAn-microsoft

@YangAn-microsoft YangAn-microsoft commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Description

Improve az storage blob copy start endpoint validation before destination credentials are reused for a source blob. The source and destination URLs must now have the same normalized origin, including scheme, hostname, and effective port.

Structured same-account blob sources validated by Azure CLI preserve the existing credential reuse behavior, including when custom endpoints are configured. Arbitrary source URLs must match the destination origin and have present, matching parsed storage account identities.

The normalization supports hostname case, default ports, trailing DNS separators, IDNA hostnames, IPv4/IPv6 endpoints, sovereign clouds, custom endpoints, and path-style emulator endpoints.

Testing Guide

Offline regression tests

Added coverage for:

  • equivalent public, sovereign, custom, IPv4, and IPv6 origins
  • mismatched schemes, ports, userinfo, and hostname suffixes
  • IDNA normalization edge cases
  • structured same-account source provenance
  • distinct accounts on path-style endpoints
  • BlockBlob, AppendBlob, and PageBlob copy paths
  • preserving credential reuse for equivalent source and destination origins
  • preserving an existing source SAS for custom endpoints whose account identity cannot be parsed

Result: 10 passed, 18 subtests passed.

Live Azure tests

Ran the complete test_storage_blob_copy_scenarios.py module against an Azure subscription with AZURE_TEST_RUN_LIVE=True.

Result: 11 passed, 4 environment/preparer failures in 22m 21s. Of the 11 passes, 6 were resource-backed live scenarios and 5 were the offline security tests in the same module.

Live scenarios that passed:

  • test_storage_blob_copy_batch
  • test_storage_blob_copy_batch_destination_blob_type
  • test_storage_blob_copy_batch_rehydrate_priority
  • test_storage_blob_copy_destination_blob_type
  • test_storage_blob_copy_requires_sync
  • test_storage_blob_show_with_copy_in_progress

The four remaining scenarios did not reach the affected copy operation:

  • test_storage_blob_copy_oauth and test_storage_blob_copy_batch_oauth: the test identity lacked a Storage Blob data-plane role (AuthorizationPermissionMismatch).
  • test_storage_blob_copy_same_account_sas and test_storage_blob_copy_with_sas_and_snapshot: resource preparation requested legacy account kind Storage, which the test subscription rejected (AccountKindNotSupported).

The two scenarios most directly covering this change were then rerun by exact test ID:

  • test_storage_blob_copy_destination_blob_type
  • test_storage_blob_copy_requires_sync

Result: 2 passed in 7m 13s.

YangAn-microsoft and others added 2 commits September 28, 2026 14:45
Require the source and destination blob URLs to share the same normalized origin before reusing destination credentials. Add regression coverage for endpoint normalization and untrusted source authorities.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@yonzhan

Copy link
Copy Markdown
Collaborator

Storage

YangAn-microsoft and others added 2 commits September 28, 2026 18:17
Carry structured same-account source provenance into blob copy while keeping strict origin validation for arbitrary source URLs. Also distinguish accounts on path-style endpoints.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@YangAn-microsoft
YangAn-microsoft marked this pull request as ready for review September 29, 2026 05:48
@YangAn-microsoft
YangAn-microsoft requested a review from a team as a code owner September 29, 2026 05:48
Copilot AI balanced review requested due to automatic review settings September 29, 2026 05:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Same-account provenance can bypass origin validation without a present, validated account identity.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Improves credential-reuse security for az storage blob copy start.

Changes:

  • Normalizes and compares source/destination URL origins.
  • Tracks validated same-account structured sources.
  • Adds regression coverage for endpoint and blob-type variations.
File Description
_validators.py Records same-account source provenance.
_params.py Registers the internal provenance argument.
operations/​blob.py Validates origins before credential reuse.
test_storage_validators.py Tests source provenance validation.
test_storage_blob_copy_scenarios.py Tests normalization and credential reuse.

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

Comment thread src/azure-cli/azure/cli/command_modules/storage/_validators.py Outdated
@x-engineering-agent

Copy link
Copy Markdown
Contributor

Automated sensitive-information remediation ran on this pull request.

  • Detected categories: email address
  • 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: Copilot

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.

@jsntcy Yu Chen (jsntcy) added the Request X Engineering Agent Request X Engineering Agent testing and review label Sep 29, 2026
@microsoft-github-policy-service

Copy link
Copy Markdown
Contributor

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

Require a present valid storage account name before structured source provenance can bypass origin matching.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@x-engineering-agent

Copy link
Copy Markdown
Contributor

Automated sensitive-information remediation ran on this pull request.

  • Detected categories: email address
  • 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: Copilot, YangAn-microsoft

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The endpoint validation is correctly integrated and comprehensively covers the affected credential-reuse paths.

Review effort: Balanced
Findings: None

Resolved since last review (1)

x-engineering-agent[bot]

This comment was marked as outdated.

@x-engineering-agent x-engineering-agent Bot added the X Engineering Agent Reviewed Pull request reviewed by X Engineering Agent label Sep 30, 2026
@microsoft-github-policy-service

Copy link
Copy Markdown
Contributor

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

Comment thread src/azure-cli/azure/cli/command_modules/storage/operations/blob.py Outdated
Comment thread src/azure-cli/azure/cli/command_modules/storage/operations/blob.py Outdated
Require both parsed account names before destination credentials can be reused for an arbitrary source URL, and cover custom endpoints using real SDK clients.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@x-engineering-agent x-engineering-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YangAn-microsoft

Review: CI failures outside PR scope

No source change is requested for these CI failures. No confirmed semantic defect or deterministic repository-policy finding was identified at db5d440747072845b4f928a79bd5c83cfa1c611b.

Summary

  • 46/50 checks passed; 4 failed check entries collapse into 2 diagnostic groups.
  • Failing tasks: 0 PR-related, 3 Not PR-related, 0 Uncertain.

Upstream CI

azdev-style

  • Result: failure
  • Relevance: Not PR-related
  • Evidence: Run Azdev Style: "src/azure-cli/azure/cli/command_modules/acs/custom.py:1064:11: W0718: Catching too general exception Exception (broad-exception-caught)". Remaining diagnostics name ACS, SQL and CLI core, not the five changed storage files; the task exits 12.
  • Suggested fix: Route the quoted diagnostics to the ACS/SQL/CLI-core owners for upstream correction, rather than editing unrelated source in this PR. Retry this job after the validation base is corrected.
  • Verify: Re-run the failed azdev-style job in Actions run 36960211966; confirm Run Azdev Style exits 0. For PR-scoped validation, use azdev style storage.

Azure.azure-cli

  • Result: failure
  • Included checks: Azure.azure-cli (Test Extensions Loading Python314), Azure.azure-cli (Check CLI Style)
  • Failure details:
    • Check CLI Style / Bash (Not PR-related)
      • Evidence: "src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_validators.py:361:19: R0916: Too many boolean expressions in if statement (6/5) (too-many-boolean-expressions)" and "src/azure-cli/azure/cli/command_modules/sql/custom.py:1779:0: W0613: Unused argument 'kwargs' (unused-argument)" precede "ERROR: Pylint: FAILED"; "Flake8: PASSED". Both files are outside this storage-only diff.
      • Next action: Have the ACS/SQL owners address these upstream diagnostics, then retry Check CLI Style on the corrected validation base. No unrelated source change is requested in this PR.
      • Verify: Re-run the Check CLI Style job / Bash task from ADO build 356056; confirm Pylint passes and the task exits 0.
    • Test Extensions Loading Python314 / Load extensions (Not PR-related)
      • Evidence: Log 293:40713-40714 reports "Failed to establish a new connection: [Errno 111] Connection refused" while downloading netappfiles_preview-1.0.0b4 from azcliprod.blob.core.windows.net, then "Failed to load: netappfiles-preview"; the task exits 1. This is an extension-download/network failure outside the changed storage code. The later missing azext_storage_blob_preview vendored-SDK trace is not evidence of a regression in these changes.
      • Next action: Retry the Python314 extension-loading job when the wheel endpoint is reachable. If the download still fails, send log 293:40713-40714 to the CI/network and netappfiles-preview owners; do not change blob-copy source to repair it.
      • Verify: Re-run Test Extensions Loading Python314 / Load extensions from ADO build 356056; confirm the netappfiles-preview wheel downloads and the task exits 0.

Test validation

  • Agent live test: Not run; this is a review-only PR. Check author-provided test evidence and upstream CI separately.
  • Regression coverage: For storage: 2 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.
  • Current-head author evidence: The author's head-linked reply reports 32 passed, 10 skipped, 18 subtests passed. The new real-SDK regression mocks only upload_blob_from_url and asserts that the original source URL/SAS is forwarded unchanged, addressing the earlier custom-endpoint feedback. Execution results are author-reported, not Agent-run.
  • Live-evidence limits: The description separately reports six resource-backed live passes, four authorization/account-preparer failures, and two focused rerun passes—not a fully passing live-module result. No recordings changed.

Risk assessment

46/100 · Medium · High confidence

The Medium rating is driven by security-sensitive behavior, public CLI behavior.

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

Posted by x-engineering-agent (Reviewer)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

act-codegen-extensibility-squad Auto-Assign Auto assign by bot Request X Engineering Agent Request X Engineering Agent testing and review Storage az storage X Engineering Agent Reviewed Pull request reviewed by X Engineering Agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants