Skip to content

MAINT: Use DefaultAzureCredential directly for blob storage auth#1946

Merged
romanlutz merged 2 commits into
microsoft:mainfrom
adrian-gavrila:adrian-gavrila/blob-storage-default-credential
Jun 30, 2026
Merged

MAINT: Use DefaultAzureCredential directly for blob storage auth#1946
romanlutz merged 2 commits into
microsoft:mainfrom
adrian-gavrila:adrian-gavrila/blob-storage-default-credential

Conversation

@adrian-gavrila

@adrian-gavrila adrian-gavrila commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Description

Swaps the Azure user-delegation SAS flow in AzureBlobStorageIO and AzureBlobStorageTarget for direct DefaultAzureCredential data-plane auth. The explicit SAS-token path is untouched in both classes; only the no-token path changes.

This is a simplification, not an RBAC reduction. Storage Blob Data Contributor already includes the generateUserDelegationKey action, so the required built-in role is unchanged. What it drops is the per-call delegation-key round-trip and SAS minting, so the container client just authenticates with the caller's Entra identity directly. That removes the dependency on generateUserDelegationKey (relevant for minimal custom roles) and one Entra round-trip per client.

Both classes now dispose the credential alongside their container client via a _close_client_async helper. For AzureBlobStorageTarget this also closes the pre-existing per-send container-client leak (the client was never closed before). Container-URL parsing in both classes now raises a clear ValueError for a URL missing a container name instead of an opaque IndexError.

Supersedes #1615 (closed, went stale vs main) and additionally covers AzureBlobStorageTarget, which answers the parity question raised on that PR.

Note: This branch was rebuilt on top of current main after the storage I/O classes were relocated from pyrit/models/storage_io.py to pyrit/memory/storage/storage.py (now a deprecation shim). The auth change is applied at the new location; no logic differs from the original branch.

Tests and Documentation

  • Updated/added unit tests in tests/unit/memory/storage/test_storage.py and tests/unit/prompt_target/target/test_prompt_target_azure_blob_storage.py covering both auth paths, credential/client cleanup, and the container-URL validation guard. All pass, pre-commit clean.
  • Live-verified a real write -> path_exists -> is_file -> read round-trip through AzureBlobStorageIO against a storage account using DefaultAzureCredential.
  • No doc changes, internal auth only.

@adrian-gavrila adrian-gavrila force-pushed the adrian-gavrila/blob-storage-default-credential branch from df1755a to 9aaa2ef Compare June 5, 2026 17:23
Replace user-delegation SAS auth with direct DefaultAzureCredential in AzureBlobStorageIO and AzureBlobStorageTarget, add a dispose path that closes both the client and credential, and harden container-URL parsing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@adrian-gavrila adrian-gavrila force-pushed the adrian-gavrila/blob-storage-default-credential branch from 9aaa2ef to c6959bc Compare June 11, 2026 20:41
@adrian-gavrila adrian-gavrila requested a review from Copilot June 11, 2026 20:52

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.

Pull request overview

This PR simplifies Azure Blob Storage authentication by using DefaultAzureCredential directly (when no SAS token is provided) in both AzureBlobStorageIO and AzureBlobStorageTarget, while preserving the explicit SAS-token path. It also adds explicit validation for container URLs missing a container name and ensures clients/credentials are properly disposed to avoid leaks.

Changes:

  • Replace the user-delegation SAS minting flow with direct DefaultAzureCredential data-plane auth for the no-token path.
  • Add _close_client_async to reliably close both the container client and credential, and call it from upload/read/write operations.
  • Add/adjust unit tests for both auth paths, cleanup behavior, and container URL validation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
pyrit/prompt_target/azure_blob_storage_target.py Switch no-token auth to DefaultAzureCredential, add client/credential cleanup, and improve URL validation/errors.
pyrit/memory/storage/storage.py Switch AzureBlobStorageIO no-token auth to DefaultAzureCredential and centralize cleanup via _close_client_async.
tests/unit/prompt_target/target/test_prompt_target_azure_blob_storage.py Add tests for SAS vs DefaultAzureCredential path, URL validation, and cleanup behavior.
tests/unit/memory/storage/test_storage.py Update/add tests for auth path selection, URL validation, and cleanup behavior.

Comment thread pyrit/prompt_target/azure_blob_storage_target.py
Comment thread pyrit/memory/storage/storage.py
Comment thread tests/unit/memory/storage/test_storage.py
Copilot AI added a commit that referenced this pull request Jun 20, 2026
@romanlutz romanlutz enabled auto-merge June 30, 2026 22:09
@romanlutz romanlutz added this pull request to the merge queue Jun 30, 2026
Merged via the queue into microsoft:main with commit f2d4976 Jun 30, 2026
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants