mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 04:32:32 +00:00
fix(security): hosted diagnostics upload SAS is create-only and bound to the requesting session; existing blob => 409
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -26,7 +26,9 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
{
|
||||
private const string TenantA = "11111111-1111-1111-1111-111111111111";
|
||||
private const string TenantB = "22222222-2222-2222-2222-222222222222";
|
||||
private const string Filename = "AgentDiagnostics-aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-20260519T120000.zip";
|
||||
private const string SessionA = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa";
|
||||
private const string SessionB = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb";
|
||||
private const string Filename = "AgentDiagnostics-" + SessionA + "-20260519T120000.zip";
|
||||
|
||||
// ── Path build + validation ────────────────────────────────────────────────────
|
||||
|
||||
@@ -89,7 +91,15 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await Assert.ThrowsAsync<ArgumentException>(
|
||||
() => svc.GenerateUploadSasAsync("not-a-guid", Filename));
|
||||
() => svc.GenerateUploadSasAsync("not-a-guid", SessionA, Filename));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_RejectsNonGuidSessionId()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await Assert.ThrowsAsync<ArgumentException>(
|
||||
() => svc.GenerateUploadSasAsync(TenantA, "not-a-guid", Filename));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -97,14 +107,64 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await Assert.ThrowsAsync<ArgumentException>(
|
||||
() => svc.GenerateUploadSasAsync(TenantA, "../other-tenant/escape.zip"));
|
||||
() => svc.GenerateUploadSasAsync(TenantA, SessionA, "../other-tenant/escape.zip"));
|
||||
}
|
||||
|
||||
// ── Session binding — one device must not be able to name another session's package ──
|
||||
|
||||
[Theory]
|
||||
[InlineData("AgentDiagnostics-" + SessionB + "-20260519T120000.zip")] // foreign session
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "20260519T120000.zip")] // missing separator
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-")] // nothing after the session
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-20260519T120000.txt")] // not a zip
|
||||
[InlineData("x-AgentDiagnostics-" + SessionA + "-20260519T120000.zip")] // wrong prefix
|
||||
[InlineData("other.zip")]
|
||||
public async Task GenerateUploadSas_RejectsFilenameNotBoundToSession(string filename)
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await Assert.ThrowsAsync<ArgumentException>(
|
||||
() => svc.GenerateUploadSasAsync(TenantA, SessionA, filename));
|
||||
Assert.Empty(svc.CapturedSasBuilders);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-20260519T120000.zip")]
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-20260519T120000-preprov.zip")]
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-20260724170902-server-requested.zip")]
|
||||
[InlineData("agentdiagnostics-" + SessionA + "-20260519T120000.ZIP")] // case-insensitive
|
||||
public async Task GenerateUploadSas_AcceptsEveryAgentProducedShapeForOwnSession(string filename)
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, filename);
|
||||
Assert.Equal($"{TenantA}/{filename}", result.BlobPath);
|
||||
}
|
||||
|
||||
// ── Create-only — an existing blob is never re-issued ─────────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_ExistingBlob_ThrowsAlreadyExists_AndMintsNothing()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService { ExistingBlobPath = $"{TenantA}/{Filename}" };
|
||||
var ex = await Assert.ThrowsAsync<HostedBlobAlreadyExistsException>(
|
||||
() => svc.GenerateUploadSasAsync(TenantA, SessionA, Filename));
|
||||
Assert.Equal($"{TenantA}/{Filename}", ex.BlobPath);
|
||||
Assert.Empty(svc.CapturedSasBuilders);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_ExistingBlobInOtherTenant_DoesNotBlockSameName()
|
||||
{
|
||||
// Existence is checked on the tenant-prefixed path, not on the bare filename.
|
||||
var svc = new FakeHostedDiagnosticsBlobService { ExistingBlobPath = $"{TenantB}/{Filename}" };
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
Assert.Equal($"{TenantA}/{Filename}", result.BlobPath);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_ReturnsTenantPrefixedBlobPath()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
Assert.Equal($"{TenantA}/{Filename}", result.BlobPath);
|
||||
}
|
||||
|
||||
@@ -112,9 +172,9 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
public async Task GenerateUploadSas_EnsuresContainerOnce()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantB, "other.zip");
|
||||
await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantB, SessionA, Filename);
|
||||
Assert.Equal(1, svc.EnsureContainerCallCount);
|
||||
}
|
||||
|
||||
@@ -124,7 +184,7 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var requested = TimeSpan.FromHours(24);
|
||||
var before = DateTime.UtcNow;
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, Filename, requested);
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename, requested);
|
||||
var maxAllowed = before.Add(HostedDiagnosticsBlobService.MaxUploadSasTtl).AddMinutes(1); // tolerance
|
||||
Assert.True(result.ExpiresAt <= maxAllowed,
|
||||
$"ExpiresAt {result.ExpiresAt:O} exceeded MaxUploadSasTtl-bounded {maxAllowed:O}");
|
||||
@@ -135,16 +195,16 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var before = DateTime.UtcNow;
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
var delta = result.ExpiresAt - before;
|
||||
Assert.InRange(delta.TotalMinutes, 14, 16);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_PassesBlobScopedResourceAndWriteCreateOnlyPermsToBuilder()
|
||||
public async Task GenerateUploadSas_PassesBlobScopedResourceAndCreateOnlyPermsToBuilder()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
|
||||
Assert.Single(svc.CapturedSasBuilders);
|
||||
var builder = svc.CapturedSasBuilders.Single();
|
||||
@@ -152,24 +212,26 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
Assert.Equal($"{TenantA}/{Filename}", builder.BlobName);
|
||||
Assert.Equal("diagnostics", builder.BlobContainerName);
|
||||
|
||||
// Reconstruct the permissions string to ensure ONLY Write + Create are set —
|
||||
// ONLY Create — no Write (would allow replacing an existing same-tenant blob),
|
||||
// no Read (would expose other-tenant blobs in cross-tenant scenarios),
|
||||
// no Delete (cleanup is service-side via DeleteIfExistsAsync),
|
||||
// no List (agent has no enumeration need).
|
||||
var permsString = builder.Permissions ?? string.Empty;
|
||||
Assert.Contains("w", permsString);
|
||||
Assert.Contains("c", permsString);
|
||||
Assert.DoesNotContain("r", permsString);
|
||||
Assert.DoesNotContain("d", permsString);
|
||||
Assert.DoesNotContain("l", permsString);
|
||||
Assert.Equal("c", builder.Permissions);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UploadSasPermissions_IsCreateOnly()
|
||||
{
|
||||
// Pinned: Write must never come back — it is what turns a create into an overwrite.
|
||||
Assert.Equal(BlobSasPermissions.Create, HostedDiagnosticsBlobService.UploadSasPermissions);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_DifferentTenants_GetDifferentBlobPaths()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var a = await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
var b = await svc.GenerateUploadSasAsync(TenantB, Filename);
|
||||
var a = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
var b = await svc.GenerateUploadSasAsync(TenantB, SessionA, Filename);
|
||||
Assert.NotEqual(a.BlobPath, b.BlobPath);
|
||||
Assert.StartsWith(TenantA + "/", a.BlobPath);
|
||||
Assert.StartsWith(TenantB + "/", b.BlobPath);
|
||||
@@ -177,8 +239,6 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
|
||||
// ── DeleteBySessionPrefixAsync — cascade sweep for multi-package sessions ──────
|
||||
|
||||
private const string SessionA = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa";
|
||||
|
||||
[Theory]
|
||||
[InlineData("not-a-guid", SessionA)]
|
||||
[InlineData(TenantA, "not-a-guid")]
|
||||
@@ -310,6 +370,9 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
public int EnsureContainerCallCount { get; private set; }
|
||||
public List<BlobSasBuilder> CapturedSasBuilders { get; } = new();
|
||||
|
||||
/// <summary>Blob path that the fake reports as already existing (null = nothing exists).</summary>
|
||||
public string? ExistingBlobPath { get; set; }
|
||||
|
||||
public FakeHostedDiagnosticsBlobService()
|
||||
: base(
|
||||
new BlobServiceClient("UseDevelopmentStorage=true"),
|
||||
@@ -324,6 +387,9 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
protected override Task<bool> BlobExistsAsync(string blobPath, CancellationToken cancellationToken)
|
||||
=> Task.FromResult(ExistingBlobPath == blobPath);
|
||||
|
||||
protected override Task<Uri> BuildUploadSasUriAsync(
|
||||
string blobPath, DateTimeOffset expiresOn, CancellationToken cancellationToken)
|
||||
{
|
||||
@@ -338,7 +404,7 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
StartsOn = DateTimeOffset.UtcNow.AddMinutes(-5),
|
||||
ExpiresOn = expiresOn,
|
||||
};
|
||||
builder.SetPermissions(BlobSasPermissions.Write | BlobSasPermissions.Create);
|
||||
builder.SetPermissions(HostedDiagnosticsBlobService.UploadSasPermissions);
|
||||
CapturedSasBuilders.Add(builder);
|
||||
|
||||
return Task.FromResult(new Uri($"https://fake.blob.core.windows.net/diagnostics/{blobPath}?fake=sas"));
|
||||
|
||||
+66
-9
@@ -2,6 +2,7 @@
|
||||
using System.Web;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Functions.Services.Deletion;
|
||||
using AutopilotMonitor.Functions.Services.Diagnostics;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using Microsoft.Azure.Functions.Worker;
|
||||
@@ -19,8 +20,10 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
/// container SAS URL stored in <see cref="TenantConfiguration.DiagnosticsBlobSasUrl"/>.
|
||||
/// Data stays in the customer's own storage account.</item>
|
||||
/// <item><c>Hosted</c> (opt-in): mints a freshly-stamped, 15-min, blob-scoped,
|
||||
/// Write+Create-only SAS pinned to <c>{tenantId}/{filename}</c> in the backend's
|
||||
/// own diagnostics container. Requires explicit admin opt-in via the UI; never
|
||||
/// Create-only SAS pinned to <c>{tenantId}/{filename}</c> in the backend's
|
||||
/// own diagnostics container. The filename must carry the requesting session id
|
||||
/// and the blob must not exist yet, so one device can neither name nor replace
|
||||
/// another session's package. Requires explicit admin opt-in via the UI; never
|
||||
/// silent.</item>
|
||||
/// </list>
|
||||
/// Called by the agent just before uploading — never at startup, never cached.
|
||||
@@ -56,6 +59,8 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
private readonly IntuneDeviceBindingValidator _intuneDeviceBindingValidator;
|
||||
private readonly BootstrapSessionService _bootstrapSessionService;
|
||||
private readonly HostedDiagnosticsBlobService _hostedDiagnostics;
|
||||
private readonly ISessionDeletionInventoryReader _sessionRowReader;
|
||||
private readonly SessionOwnerBindingObserver _ownerBinding;
|
||||
|
||||
public GetDiagnosticsUploadUrlFunction(
|
||||
ILogger<GetDiagnosticsUploadUrlFunction> logger,
|
||||
@@ -68,8 +73,12 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
CloudPcDeviceValidator cloudPcDeviceValidator,
|
||||
IntuneDeviceBindingValidator intuneDeviceBindingValidator,
|
||||
BootstrapSessionService bootstrapSessionService,
|
||||
HostedDiagnosticsBlobService hostedDiagnostics)
|
||||
HostedDiagnosticsBlobService hostedDiagnostics,
|
||||
ISessionDeletionInventoryReader sessionRowReader,
|
||||
SessionOwnerBindingObserver ownerBinding)
|
||||
{
|
||||
_sessionRowReader = sessionRowReader;
|
||||
_ownerBinding = ownerBinding;
|
||||
_logger = logger;
|
||||
_configService = configService;
|
||||
_adminConfigService = adminConfigService;
|
||||
@@ -145,7 +154,7 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
|
||||
if (destination == DestinationHosted)
|
||||
{
|
||||
return await IssueHostedSasAsync(req, requestBody);
|
||||
return await IssueHostedSasAsync(req, requestBody, validation);
|
||||
}
|
||||
|
||||
if (destination == DestinationCustomerSas)
|
||||
@@ -237,7 +246,7 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
}
|
||||
|
||||
private async Task<HttpResponseData> IssueHostedSasAsync(
|
||||
HttpRequestData req, GetDiagnosticsUploadUrlRequest requestBody)
|
||||
HttpRequestData req, GetDiagnosticsUploadUrlRequest requestBody, SecurityValidationResult validation)
|
||||
{
|
||||
if (string.IsNullOrEmpty(requestBody.FileName))
|
||||
{
|
||||
@@ -250,18 +259,36 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
return badRequest;
|
||||
}
|
||||
|
||||
// The blob name is bound to the session below, so the session id has to be a real
|
||||
// one — and it is the coordinate for the owner-binding observation.
|
||||
if (!SecurityValidator.IsValidGuid(requestBody.SessionId))
|
||||
{
|
||||
var badRequest = req.CreateResponse(HttpStatusCode.BadRequest);
|
||||
await badRequest.WriteAsJsonAsync(new GetDiagnosticsUploadUrlResponse
|
||||
{
|
||||
Success = false,
|
||||
Message = "sessionId is required for hosted destination"
|
||||
});
|
||||
return badRequest;
|
||||
}
|
||||
|
||||
// SESSION-OWNER-BINDING-SHADOW: same observation as the session-scoped writes, so
|
||||
// stage-2 enforcement covers this endpoint with the same data. Never rejects here.
|
||||
await ObserveSessionOwnerAsync(req, requestBody.TenantId, requestBody.SessionId, validation);
|
||||
|
||||
HostedUploadSasResult sasResult;
|
||||
try
|
||||
{
|
||||
sasResult = await _hostedDiagnostics.GenerateUploadSasAsync(
|
||||
requestBody.TenantId, requestBody.FileName);
|
||||
requestBody.TenantId, requestBody.SessionId, requestBody.FileName);
|
||||
}
|
||||
catch (ArgumentException ex)
|
||||
{
|
||||
// Filename validation failed (path separator, traversal, oversize, etc.)
|
||||
// Filename validation failed (path separator, traversal, oversize, not bound
|
||||
// to the requesting session, etc.)
|
||||
_logger.LogWarning(
|
||||
"GetDiagnosticsUploadUrl: rejecting hosted SAS for tenant {TenantId}, file {FileName}: {Reason}",
|
||||
requestBody.TenantId, requestBody.FileName, ex.Message);
|
||||
"GetDiagnosticsUploadUrl: rejecting hosted SAS for tenant {TenantId}, session {SessionId}, file {FileName}: {Reason}",
|
||||
requestBody.TenantId, requestBody.SessionId, requestBody.FileName, ex.Message);
|
||||
var badRequest = req.CreateResponse(HttpStatusCode.BadRequest);
|
||||
await badRequest.WriteAsJsonAsync(new GetDiagnosticsUploadUrlResponse
|
||||
{
|
||||
@@ -270,6 +297,21 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
});
|
||||
return badRequest;
|
||||
}
|
||||
catch (HostedBlobAlreadyExistsException ex)
|
||||
{
|
||||
// A legitimate agent never re-requests an existing name (every package is
|
||||
// freshly timestamped); this is either a replay or an overwrite attempt.
|
||||
_logger.LogWarning(
|
||||
"GetDiagnosticsUploadUrl: rejecting hosted SAS for tenant {TenantId}, session {SessionId}: blob {BlobPath} already exists",
|
||||
requestBody.TenantId, requestBody.SessionId, ex.BlobPath);
|
||||
var conflict = req.CreateResponse(HttpStatusCode.Conflict);
|
||||
await conflict.WriteAsJsonAsync(new GetDiagnosticsUploadUrlResponse
|
||||
{
|
||||
Success = false,
|
||||
Message = "A diagnostics package with this name already exists"
|
||||
});
|
||||
return conflict;
|
||||
}
|
||||
|
||||
_logger.LogInformation(
|
||||
"GetDiagnosticsUploadUrl: Issuing Hosted upload URL for tenant {TenantId}, session {SessionId}, blob {BlobPath}, SAS expires {ExpiresAt}",
|
||||
@@ -288,6 +330,21 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
return response;
|
||||
}
|
||||
|
||||
private async Task ObserveSessionOwnerAsync(
|
||||
HttpRequestData req, string tenantId, string sessionId, SecurityValidationResult validation)
|
||||
{
|
||||
try
|
||||
{
|
||||
var row = await _sessionRowReader.GetSessionRowAsync(tenantId, sessionId);
|
||||
_ownerBinding.Observe(req, tenantId, sessionId, row, validation, "agent/upload-url");
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogWarning(ex,
|
||||
"GetDiagnosticsUploadUrl: session-owner observation skipped for session {SessionId}", sessionId);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Normalizes the destination string from TenantConfiguration. Null/empty becomes
|
||||
/// CustomerSas (preserves existing behaviour for tenants that predate this field).
|
||||
|
||||
+81
-11
@@ -27,8 +27,8 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
/// (<c>AzureStorageAccountName</c>) or connection string (<c>AzureBlobStorageConnectionString</c>).
|
||||
/// SAS issuance for the agent upload-url endpoint differs by auth path — User
|
||||
/// Delegation SAS under MI, account-key SAS under connection-string — both produce
|
||||
/// a blob-scoped, Write+Create-only, 15-min token that cannot reach other tenants'
|
||||
/// prefixes.
|
||||
/// a blob-scoped, Create-only, 15-min token that cannot reach other tenants'
|
||||
/// prefixes and cannot replace an existing blob.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public class HostedDiagnosticsBlobService
|
||||
@@ -40,6 +40,16 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
// requesting hour-long tokens.
|
||||
public static readonly TimeSpan MaxUploadSasTtl = TimeSpan.FromMinutes(60);
|
||||
|
||||
/// <summary>
|
||||
/// Permissions on the upload SAS. Create-only by design: the agent PUTs one new block
|
||||
/// blob per package (fresh timestamped name), so Write — which would allow replacing an
|
||||
/// existing blob under the same tenant prefix — is never needed.
|
||||
/// </summary>
|
||||
public const BlobSasPermissions UploadSasPermissions = BlobSasPermissions.Create;
|
||||
|
||||
/// <summary>Canonical package-name prefix; the session id follows immediately after.</summary>
|
||||
internal const string PackageNamePrefix = "AgentDiagnostics-";
|
||||
|
||||
private readonly BlobServiceClient _blobServiceClient;
|
||||
private readonly ILogger<HostedDiagnosticsBlobService> _logger;
|
||||
private readonly bool _usesManagedIdentity;
|
||||
@@ -94,16 +104,21 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
// -------- Public API --------
|
||||
|
||||
/// <summary>
|
||||
/// Mints a blob-scoped, Write+Create-only SAS URI pinned to the EXACT path
|
||||
/// Mints a blob-scoped, Create-only SAS URI pinned to the EXACT path
|
||||
/// <c>{tenantId}/{filename}</c> for an agent's diagnostics-package upload. Cannot
|
||||
/// be redirected to other tenants' prefixes. TTL is capped at
|
||||
/// <see cref="MaxUploadSasTtl"/> to keep the exposure window tight.
|
||||
/// be redirected to other tenants' prefixes, and the filename must carry the
|
||||
/// caller's <paramref name="sessionId"/> (<c>AgentDiagnostics-{sessionId}-…zip</c>)
|
||||
/// so one device cannot name another session's package. An already-existing blob
|
||||
/// is never re-issued (<see cref="HostedBlobAlreadyExistsException"/>). TTL is
|
||||
/// capped at <see cref="MaxUploadSasTtl"/> to keep the exposure window tight.
|
||||
/// </summary>
|
||||
public virtual async Task<HostedUploadSasResult> GenerateUploadSasAsync(
|
||||
string tenantId, string filename, TimeSpan? ttl = null, CancellationToken cancellationToken = default)
|
||||
string tenantId, string sessionId, string filename, TimeSpan? ttl = null, CancellationToken cancellationToken = default)
|
||||
{
|
||||
SecurityValidator.EnsureValidGuid(tenantId, nameof(tenantId));
|
||||
SecurityValidator.EnsureValidGuid(sessionId, nameof(sessionId));
|
||||
ValidateFilename(filename);
|
||||
EnsureFilenameBoundToSession(filename, sessionId);
|
||||
|
||||
var effectiveTtl = ttl ?? TimeSpan.FromMinutes(15);
|
||||
if (effectiveTtl > MaxUploadSasTtl)
|
||||
@@ -116,6 +131,14 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
|
||||
await EnsureContainerAsync(cancellationToken);
|
||||
|
||||
// Create-only semantics, enforced twice: the SAS carries no Write permission (an
|
||||
// existing blob cannot be replaced with it), and issuance itself refuses a name
|
||||
// that is already taken so the conflict surfaces here as a 409 instead of as an
|
||||
// opaque 403 from storage. Every package gets a fresh timestamped name, so a
|
||||
// legitimate agent never asks for an existing one.
|
||||
if (await BlobExistsAsync(blobPath, cancellationToken))
|
||||
throw new HostedBlobAlreadyExistsException(blobPath);
|
||||
|
||||
var sasUri = await BuildUploadSasUriAsync(blobPath, expiresOn, cancellationToken);
|
||||
|
||||
return new HostedUploadSasResult
|
||||
@@ -185,7 +208,7 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
|
||||
// Trailing dash bounds the match to this exact session id (GUIDs are fixed-length,
|
||||
// but the explicit separator keeps the invariant obvious and layout-proof).
|
||||
var prefix = $"{tenantId}/AgentDiagnostics-{sessionId}-";
|
||||
var prefix = $"{tenantId}/{PackageNamePrefix}{sessionId}-";
|
||||
var deleted = 0;
|
||||
await foreach (var item in containerClient.GetBlobsAsync(
|
||||
BlobTraits.None, BlobStates.None, prefix: prefix, cancellationToken: cancellationToken))
|
||||
@@ -272,7 +295,17 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the blob-scoped Write+Create-only SAS URI for upload. Branch on the
|
||||
/// Test seam: whether a blob already exists at <paramref name="blobPath"/>. Production
|
||||
/// issues a HEAD; tests override to simulate a taken name without Azurite.
|
||||
/// </summary>
|
||||
protected virtual async Task<bool> BlobExistsAsync(string blobPath, CancellationToken cancellationToken)
|
||||
{
|
||||
var response = await GetContainerClient().GetBlobClient(blobPath).ExistsAsync(cancellationToken);
|
||||
return response.Value;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the blob-scoped Create-only SAS URI for upload. Branch on the
|
||||
/// auth mode: MI uses User Delegation SAS (no account key needed); connection
|
||||
/// string uses the underlying StorageSharedKeyCredential via
|
||||
/// <see cref="BlobClient.GenerateSasUri(BlobSasBuilder)"/>.
|
||||
@@ -290,9 +323,10 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
StartsOn = DateTimeOffset.UtcNow.AddMinutes(-5), // clock-skew tolerance
|
||||
ExpiresOn = expiresOn,
|
||||
};
|
||||
// Write + Create only — agent must not be able to Read other tenants' blobs,
|
||||
// Delete anything, or List the container.
|
||||
sasBuilder.SetPermissions(BlobSasPermissions.Write | BlobSasPermissions.Create);
|
||||
// Create only — the agent does a single Put Blob of a brand-new name. No Write
|
||||
// (would let the holder replace an existing blob of the same tenant), no Read,
|
||||
// no Delete, no List.
|
||||
sasBuilder.SetPermissions(UploadSasPermissions);
|
||||
|
||||
if (UsesManagedIdentity)
|
||||
{
|
||||
@@ -337,6 +371,26 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
throw new ArgumentException("filename must not contain null bytes", nameof(filename));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Binds an upload filename to the requesting session: it must be
|
||||
/// <c>AgentDiagnostics-{sessionId}-{anything}.zip</c> (the shape the agent has always
|
||||
/// produced, incl. the <c>-preprov</c>/<c>-server-requested</c> suffixes). Without this
|
||||
/// an authenticated device could name any other session's package in its tenant.
|
||||
/// Throws <see cref="ArgumentException"/> on rejection.
|
||||
/// </summary>
|
||||
internal static void EnsureFilenameBoundToSession(string filename, string sessionId)
|
||||
{
|
||||
var required = $"{PackageNamePrefix}{sessionId}-";
|
||||
if (!filename.StartsWith(required, StringComparison.OrdinalIgnoreCase)
|
||||
|| filename.Length <= required.Length
|
||||
|| !filename.EndsWith(".zip", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
throw new ArgumentException(
|
||||
"filename must be of the form 'AgentDiagnostics-{sessionId}-{timestamp}.zip' for the requesting session",
|
||||
nameof(filename));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Validates a stored blob path (<c>{tenantId}/{filename}</c>) on the read paths.
|
||||
/// Defends the download proxy against path-traversal even if a row was tampered with.
|
||||
@@ -358,6 +412,22 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Thrown by <see cref="HostedDiagnosticsBlobService.GenerateUploadSasAsync"/> when the
|
||||
/// requested blob name is already taken. Distinct from <see cref="ArgumentException"/>
|
||||
/// (malformed input → 400) so the Function can answer 409.
|
||||
/// </summary>
|
||||
public sealed class HostedBlobAlreadyExistsException : InvalidOperationException
|
||||
{
|
||||
public HostedBlobAlreadyExistsException(string blobPath)
|
||||
: base($"Hosted diagnostics blob already exists: {blobPath}")
|
||||
{
|
||||
BlobPath = blobPath;
|
||||
}
|
||||
|
||||
public string BlobPath { get; }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Return value of <see cref="HostedDiagnosticsBlobService.GenerateUploadSasAsync"/>.
|
||||
/// </summary>
|
||||
|
||||
Reference in New Issue
Block a user