Skip to content

Commit da8a190

Browse files
YunchuWangCopilot
andcommitted
Make blob payload delete path token-v2 aware to prevent orphaned-payload data loss
The auto-purge delete path in BlobPayloadStore was v1-only. Once self-describing `blob:v2:<fullBlobUrl>` tokens ship (PR #766), a v2 token would hit the v1-only DecodeToken, throw ArgumentException, and DeleteExternalBlobActivity would classify it as a poison token -> Discarded -> the backend acks/clears the tombstone while the blob is never deleted -> orphaned payload (leak). Converge the shared token code with #766 verbatim (trivial future merge) and make the delete path v2-aware, mirroring DownloadAsync resolution: - Recognize both blob:v1: and blob:v2: prefixes; DecodeToken returns a 5-tuple and parses v2 URLs via BlobUriBuilder (DNS and Azurite path-style); v1 stays read-compatible. - UploadAsync now emits v2 tokens (EncodeToken(blob.Uri)). - DeleteAsync: v1 container-match check preserved; v2 same-account fast-path via IsConfiguredContainer; v2 cross-account with an identity credential deletes directly; v2 cross-account WITHOUT identity throws PayloadStorageException. DeleteIfExistsAsync keeps deletes idempotent. Poison classification is unchanged and verified: PayloadStorageException derives from InvalidOperationException (not ArgumentException), so a cross-account-without-identity delete falls through DeleteExternalBlobActivity's generic catch -> Retry (stays tombstoned until credentials are fixed), never Discarded. Tests: mirror #766 v2 encode/round-trip, DecodeToken v2 (DNS + Azurite) and v1 back-compat, and IsKnownPayloadToken v1+v2; add a v2 same-account happy-path delete and a v2 cross-account-without-identity DeleteAsync -> PayloadStorageException guard (throws before any network call); add activity tests asserting ArgumentException -> Discarded, PayloadStorageException -> Retry, and success -> Deleted. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 44c27836-c49c-45fd-ae2b-3309f9c3f0f0
1 parent e74f633 commit da8a190

3 files changed

Lines changed: 315 additions & 47 deletions

File tree

src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs

Lines changed: 154 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,22 @@ namespace Microsoft.DurableTask;
1313

1414
/// <summary>
1515
/// Azure Blob Storage implementation of <see cref="PayloadStore"/>.
16-
/// Stores payloads as blobs and returns opaque tokens in the form "blob:v1:&lt;container&gt;:&lt;blobName&gt;".
16+
/// Stores payloads as blobs and returns self-describing opaque tokens in the form
17+
/// "blob:v2:&lt;fullBlobUrl&gt;", where the URL is the blob's absolute URI including the storage account.
18+
/// Legacy "blob:v1:&lt;container&gt;:&lt;blobName&gt;" tokens are still recognized for read back-compatibility.
1719
/// </summary>
1820
public sealed class BlobPayloadStore : PayloadStore
1921
{
20-
const string TokenPrefix = "blob:v1:";
22+
const string TokenPrefixV1 = "blob:v1:";
23+
const string TokenPrefixV2 = "blob:v2:";
2124
const string ContentEncodingGzip = "gzip";
2225
const int MaxRetryAttempts = 8;
2326
const int BaseDelayMs = 250;
2427
const int MaxDelayMs = 10_000;
2528
const int NetworkTimeoutMinutes = 2;
2629
readonly BlobContainerClient containerClient;
2730
readonly LargePayloadStorageOptions options;
31+
readonly BlobClientOptions clientOptions;
2832

2933
/// <summary>
3034
/// Initializes a new instance of the <see cref="BlobPayloadStore"/> class.
@@ -48,7 +52,7 @@ public BlobPayloadStore(LargePayloadStorageOptions options)
4852
nameof(options));
4953
}
5054

51-
BlobClientOptions clientOptions = new()
55+
this.clientOptions = new BlobClientOptions
5256
{
5357
Retry =
5458
{
@@ -61,8 +65,8 @@ public BlobPayloadStore(LargePayloadStorageOptions options)
6165
};
6266

6367
BlobServiceClient serviceClient = hasIdentityAuth
64-
? new BlobServiceClient(options.AccountUri, options.Credential, clientOptions)
65-
: new BlobServiceClient(options.ConnectionString, clientOptions);
68+
? new BlobServiceClient(options.AccountUri, options.Credential, this.clientOptions)
69+
: new BlobServiceClient(options.ConnectionString, this.clientOptions);
6670

6771
this.containerClient = serviceClient.GetBlobContainerClient(options.ContainerName);
6872
}
@@ -117,56 +121,85 @@ public override async Task<string> UploadAsync(string payLoad, CancellationToken
117121
await blobStream.FlushAsync(cancellationToken);
118122
}
119123

120-
return EncodeToken(this.containerClient.Name, blobName);
124+
return EncodeToken(blob.Uri);
121125
}
122126

123127
/// <inheritdoc/>
124128
public override async Task<string> DownloadAsync(string token, CancellationToken cancellationToken)
125129
{
126-
(string container, string name) = DecodeToken(token);
127-
if (!string.Equals(container, this.containerClient.Name, StringComparison.Ordinal))
128-
{
129-
throw new ArgumentException("Token container does not match configured container.", nameof(token));
130-
}
131-
132-
BlobClient blob = this.containerClient.GetBlobClient(name);
130+
(bool isV2, string container, string name, Uri? blobUri, Uri? containerUri) = DecodeToken(token);
133131

134-
try
132+
if (!isV2)
135133
{
136-
using BlobDownloadStreamingResult result = await blob.DownloadStreamingAsync(cancellationToken: cancellationToken);
137-
Stream contentStream = result.Content;
138-
bool isGzip = string.Equals(
139-
result.Details.ContentEncoding, ContentEncodingGzip, StringComparison.OrdinalIgnoreCase);
140-
141-
if (isGzip)
134+
// v1 tokens do not carry the account, so the payload is assumed to live in the configured container.
135+
if (!string.Equals(container, this.containerClient.Name, StringComparison.Ordinal))
142136
{
143-
using GZipStream decompressed = new(contentStream, CompressionMode.Decompress);
144-
using StreamReader reader = new(decompressed, Encoding.UTF8);
145-
return await ReadToEndAsync(reader, cancellationToken);
137+
throw new ArgumentException("Token container does not match configured container.", nameof(token));
146138
}
147139

148-
using StreamReader uncompressedReader = new(contentStream, Encoding.UTF8);
149-
return await ReadToEndAsync(uncompressedReader, cancellationToken);
140+
return await DownloadFromBlobAsync(this.containerClient.GetBlobClient(name), cancellationToken);
150141
}
151-
catch (RequestFailedException ex) when (ex.Status == (int)HttpStatusCode.NotFound)
142+
143+
// v2 tokens are self-describing: honor the account and container encoded in the token.
144+
BlobClient blob;
145+
if (this.IsConfiguredContainer(containerUri!))
146+
{
147+
// Same account and container as the configured store: reuse it (works with any auth mode).
148+
blob = this.containerClient.GetBlobClient(name);
149+
}
150+
else if (this.options.Credential != null)
151+
{
152+
// The payload lives in a different account (e.g. the store was repointed). Identity auth can still
153+
// read it as long as the credential has RBAC access to that account.
154+
blob = new BlobClient(blobUri, this.options.Credential, this.clientOptions);
155+
}
156+
else
152157
{
153158
throw new PayloadStorageException(
154-
$"The blob '{name}' was not found in container '{container}'. " +
155-
"The payload may have been deleted or the container was never created.",
156-
ex);
159+
$"The externalized payload lives in a different storage account ('{containerUri}') than the " +
160+
$"currently-configured payload store ('{this.containerClient.Uri}'). Cross-account payload reads " +
161+
"require identity (AAD) authentication with access to both accounts; connection-string / " +
162+
"account-key credentials are account-specific and cannot read another account.");
157163
}
164+
165+
return await DownloadFromBlobAsync(blob, cancellationToken);
158166
}
159167

160168
/// <inheritdoc/>
161169
public override async Task DeleteAsync(string token, CancellationToken cancellationToken)
162170
{
163-
(string container, string name) = DecodeToken(token);
164-
if (!string.Equals(container, this.containerClient.Name, StringComparison.Ordinal))
171+
(bool isV2, string container, string name, Uri? blobUri, Uri? containerUri) = DecodeToken(token);
172+
173+
BlobClient blob;
174+
if (!isV2)
165175
{
166-
throw new ArgumentException("Token container does not match configured container.", nameof(token));
167-
}
176+
// v1 tokens do not carry the account, so the payload is assumed to live in the configured container.
177+
if (!string.Equals(container, this.containerClient.Name, StringComparison.Ordinal))
178+
{
179+
throw new ArgumentException("Token container does not match configured container.", nameof(token));
180+
}
168181

169-
BlobClient blob = this.containerClient.GetBlobClient(name);
182+
blob = this.containerClient.GetBlobClient(name);
183+
}
184+
else if (this.IsConfiguredContainer(containerUri!))
185+
{
186+
// Same account and container as the configured store: reuse it (works with any auth mode).
187+
blob = this.containerClient.GetBlobClient(name);
188+
}
189+
else if (this.options.Credential != null)
190+
{
191+
// The payload lives in a different account (e.g. the store was repointed). Identity auth can still
192+
// delete it as long as the credential has RBAC access to that account.
193+
blob = new BlobClient(blobUri, this.options.Credential, this.clientOptions);
194+
}
195+
else
196+
{
197+
throw new PayloadStorageException(
198+
$"The externalized payload lives in a different storage account ('{containerUri}') than the " +
199+
$"currently-configured payload store ('{this.containerClient.Uri}'). Cross-account payload deletes " +
200+
"require identity (AAD) authentication with access to both accounts; connection-string / " +
201+
"account-key credentials are account-specific and cannot delete from another account.");
202+
}
170203

171204
// Idempotent by design: DeleteIfExistsAsync returns false (rather than throwing) when the blob is
172205
// already gone, so re-delivered tombstones and concurrent purges from multiple worker replicas are safe.
@@ -184,7 +217,60 @@ public override bool IsKnownPayloadToken(string value)
184217
return false;
185218
}
186219

187-
return value.StartsWith(TokenPrefix, StringComparison.Ordinal);
220+
return value.StartsWith(TokenPrefixV1, StringComparison.Ordinal)
221+
|| value.StartsWith(TokenPrefixV2, StringComparison.Ordinal);
222+
}
223+
224+
/// <summary>
225+
/// Encodes a self-describing v2 payload token from the blob's absolute URI. The token carries the full blob
226+
/// URL (including the storage account) so readers can locate the payload without relying on the currently
227+
/// configured store. <c>BlobClient.Uri</c> contains no SAS or account key and is safe to persist.
228+
/// </summary>
229+
/// <param name="blobUri">The absolute URI of the blob holding the payload.</param>
230+
/// <returns>An opaque payload token in the form "blob:v2:&lt;fullBlobUrl&gt;".</returns>
231+
internal static string EncodeToken(Uri blobUri) => $"{TokenPrefixV2}{blobUri}";
232+
233+
/// <summary>
234+
/// Decodes a payload token. Supports self-describing v2 tokens ("blob:v2:&lt;fullBlobUrl&gt;") and legacy v1
235+
/// tokens ("blob:v1:&lt;container&gt;:&lt;blobName&gt;"), the latter for read back-compatibility.
236+
/// </summary>
237+
/// <param name="token">The payload token to decode.</param>
238+
/// <returns>
239+
/// A tuple describing the token: whether it is v2, the container and blob names, and (for v2 only) the
240+
/// absolute blob URI and its container-level URI.
241+
/// </returns>
242+
internal static (bool IsV2, string Container, string Name, Uri? BlobUri, Uri? ContainerUri) DecodeToken(
243+
string token)
244+
{
245+
if (token.StartsWith(TokenPrefixV2, StringComparison.Ordinal))
246+
{
247+
string rest = token.Substring(TokenPrefixV2.Length);
248+
if (!Uri.TryCreate(rest, UriKind.Absolute, out Uri? blobUri))
249+
{
250+
throw new ArgumentException("Invalid external payload token format.", nameof(token));
251+
}
252+
253+
BlobUriBuilder builder = new(blobUri);
254+
string container = builder.BlobContainerName;
255+
string name = builder.BlobName;
256+
builder.BlobName = string.Empty;
257+
Uri containerUri = builder.ToUri();
258+
return (true, container, name, blobUri, containerUri);
259+
}
260+
261+
if (token.StartsWith(TokenPrefixV1, StringComparison.Ordinal))
262+
{
263+
string rest = token.Substring(TokenPrefixV1.Length);
264+
int sep = rest.IndexOf(':');
265+
if (sep <= 0 || sep >= rest.Length - 1)
266+
{
267+
throw new ArgumentException("Invalid external payload token format.", nameof(token));
268+
}
269+
270+
return (false, rest.Substring(0, sep), rest.Substring(sep + 1), null, null);
271+
}
272+
273+
throw new ArgumentException("Invalid external payload token.", nameof(token));
188274
}
189275

190276
static async Task WritePayloadAsync(byte[] payloadBuffer, Stream target, CancellationToken cancellationToken)
@@ -208,22 +294,43 @@ static async Task<string> ReadToEndAsync(StreamReader reader, CancellationToken
208294
#endif
209295
}
210296

211-
static string EncodeToken(string container, string name) => $"blob:v1:{container}:{name}";
212-
213-
static (string Container, string Name) DecodeToken(string token)
297+
static async Task<string> DownloadFromBlobAsync(BlobClient blob, CancellationToken cancellationToken)
214298
{
215-
if (!token.StartsWith(TokenPrefix, StringComparison.Ordinal))
299+
try
216300
{
217-
throw new ArgumentException("Invalid external payload token.", nameof(token));
218-
}
301+
using BlobDownloadStreamingResult result = await blob.DownloadStreamingAsync(cancellationToken: cancellationToken);
302+
Stream contentStream = result.Content;
303+
bool isGzip = string.Equals(
304+
result.Details.ContentEncoding, ContentEncodingGzip, StringComparison.OrdinalIgnoreCase);
305+
306+
if (isGzip)
307+
{
308+
using GZipStream decompressed = new(contentStream, CompressionMode.Decompress);
309+
using StreamReader reader = new(decompressed, Encoding.UTF8);
310+
return await ReadToEndAsync(reader, cancellationToken);
311+
}
219312

220-
string rest = token.Substring(TokenPrefix.Length);
221-
int sep = rest.IndexOf(':');
222-
if (sep <= 0 || sep >= rest.Length - 1)
313+
using StreamReader uncompressedReader = new(contentStream, Encoding.UTF8);
314+
return await ReadToEndAsync(uncompressedReader, cancellationToken);
315+
}
316+
catch (RequestFailedException ex) when (ex.Status == (int)HttpStatusCode.NotFound)
223317
{
224-
throw new ArgumentException("Invalid external payload token format.", nameof(token));
318+
throw new PayloadStorageException(
319+
$"The blob '{blob.Name}' was not found in container '{blob.BlobContainerName}'. " +
320+
"The payload may have been deleted or the container was never created.",
321+
ex);
225322
}
323+
}
226324

227-
return (rest.Substring(0, sep), rest.Substring(sep + 1));
325+
bool IsConfiguredContainer(Uri tokenContainerUri)
326+
{
327+
Uri configured = this.containerClient.Uri;
328+
return string.Equals(tokenContainerUri.Scheme, configured.Scheme, StringComparison.OrdinalIgnoreCase)
329+
&& string.Equals(tokenContainerUri.Host, configured.Host, StringComparison.OrdinalIgnoreCase)
330+
&& tokenContainerUri.Port == configured.Port
331+
&& string.Equals(
332+
tokenContainerUri.AbsolutePath.TrimEnd('/'),
333+
configured.AbsolutePath.TrimEnd('/'),
334+
StringComparison.Ordinal);
228335
}
229-
}
336+
}

test/AzureBlobPayloads.Tests/AutoPurge/DeleteExternalBlobActivityTests.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,53 @@ public async Task RunAsync_WhenDeleteThrowsRequestFailedNon400_LeavesTombstonedF
3838
result.Should().Be(BlobDeleteResult.Retry);
3939
}
4040

41+
[Fact]
42+
public async Task RunAsync_WhenDeleteThrowsArgumentException_DiscardsPoisonToken()
43+
{
44+
// Arrange - a malformed / unknown-prefix token makes the store throw ArgumentException at decode time,
45+
// before any network call. It can never succeed, so it is discarded (acked) as poison.
46+
StubPayloadStore store = new(new ArgumentException("Invalid external payload token."));
47+
DeleteExternalBlobActivity activity = new(store, new TestLogger<DeleteExternalBlobActivity>());
48+
49+
// Act
50+
BlobDeleteResult result = await activity.RunAsync(null!, "not-a-valid-token");
51+
52+
// Assert - discarded so the backend clears the row instead of re-streaming poison forever.
53+
result.Should().Be(BlobDeleteResult.Discarded);
54+
}
55+
56+
[Fact]
57+
public async Task RunAsync_WhenDeleteThrowsPayloadStorageException_LeavesTombstonedForRetry()
58+
{
59+
// Arrange - a v2 cross-account token without an identity credential makes the store throw
60+
// PayloadStorageException. That is a permission/capability gap, not a poison token: dropping the blob
61+
// would orphan it, so it must stay tombstoned for retry - the very data loss this path guards against.
62+
StubPayloadStore store = new(new PayloadStorageException("cross-account delete requires identity"));
63+
DeleteExternalBlobActivity activity = new(store, new TestLogger<DeleteExternalBlobActivity>());
64+
65+
// Act
66+
BlobDeleteResult result = await activity.RunAsync(
67+
null!, "blob:v2:https://otheraccount.blob.core.windows.net/c/abc123");
68+
69+
// Assert - retried (not discarded) so the tombstone survives until the credential is fixed.
70+
result.Should().Be(BlobDeleteResult.Retry);
71+
}
72+
73+
[Fact]
74+
public async Task RunAsync_WhenDeleteSucceeds_AcksAsDeleted()
75+
{
76+
// Arrange - a v2 token that deletes cleanly (same-account, or cross-account with identity access).
77+
StubPayloadStore store = new(deleteError: null);
78+
DeleteExternalBlobActivity activity = new(store, new TestLogger<DeleteExternalBlobActivity>());
79+
80+
// Act
81+
BlobDeleteResult result = await activity.RunAsync(
82+
null!, "blob:v2:https://acct.blob.core.windows.net/c/abc123");
83+
84+
// Assert
85+
result.Should().Be(BlobDeleteResult.Deleted);
86+
}
87+
4188
sealed class StubPayloadStore : PayloadStore
4289
{
4390
readonly Exception? deleteError;

0 commit comments

Comments
 (0)