From 83effea592d981cce508e3aff457843fa067b4d1 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Sat, 29 Aug 2026 23:13:00 +0200 Subject: [PATCH] fix(security): hosted diagnostics upload SAS is create-only and bound to the requesting session; existing blob => 409 Co-Authored-By: Claude Fable 5 --- .../HostedDiagnosticsBlobServiceTests.cs | 112 ++++++++++++++---- .../GetDiagnosticsUploadUrlFunction.cs | 75 ++++++++++-- .../HostedDiagnosticsBlobService.cs | 92 ++++++++++++-- 3 files changed, 236 insertions(+), 43 deletions(-) diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/HostedDiagnosticsBlobServiceTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/HostedDiagnosticsBlobServiceTests.cs index 10214c38..ebe754a4 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/HostedDiagnosticsBlobServiceTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/HostedDiagnosticsBlobServiceTests.cs @@ -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( - () => 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( + () => svc.GenerateUploadSasAsync(TenantA, "not-a-guid", Filename)); } [Fact] @@ -97,14 +107,64 @@ public class HostedDiagnosticsBlobServiceTests { var svc = new FakeHostedDiagnosticsBlobService(); await Assert.ThrowsAsync( - () => 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( + () => 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( + () => 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 CapturedSasBuilders { get; } = new(); + /// Blob path that the fake reports as already existing (null = nothing exists). + 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 BlobExistsAsync(string blobPath, CancellationToken cancellationToken) + => Task.FromResult(ExistingBlobPath == blobPath); + protected override Task 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")); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Diagnostics/GetDiagnosticsUploadUrlFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Diagnostics/GetDiagnosticsUploadUrlFunction.cs index bdcb5966..121ae89c 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Diagnostics/GetDiagnosticsUploadUrlFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Diagnostics/GetDiagnosticsUploadUrlFunction.cs @@ -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 . /// Data stays in the customer's own storage account. /// Hosted (opt-in): mints a freshly-stamped, 15-min, blob-scoped, - /// Write+Create-only SAS pinned to {tenantId}/{filename} in the backend's - /// own diagnostics container. Requires explicit admin opt-in via the UI; never + /// Create-only SAS pinned to {tenantId}/{filename} 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. /// /// 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 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 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); + } + } + /// /// Normalizes the destination string from TenantConfiguration. Null/empty becomes /// CustomerSas (preserves existing behaviour for tenants that predate this field). diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Diagnostics/HostedDiagnosticsBlobService.cs b/src/Backend/AutopilotMonitor.Functions/Services/Diagnostics/HostedDiagnosticsBlobService.cs index 3a7ca898..35089db1 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/Diagnostics/HostedDiagnosticsBlobService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/Diagnostics/HostedDiagnosticsBlobService.cs @@ -27,8 +27,8 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics /// (AzureStorageAccountName) or connection string (AzureBlobStorageConnectionString). /// 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. /// /// public class HostedDiagnosticsBlobService @@ -40,6 +40,16 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics // requesting hour-long tokens. public static readonly TimeSpan MaxUploadSasTtl = TimeSpan.FromMinutes(60); + /// + /// 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. + /// + public const BlobSasPermissions UploadSasPermissions = BlobSasPermissions.Create; + + /// Canonical package-name prefix; the session id follows immediately after. + internal const string PackageNamePrefix = "AgentDiagnostics-"; + private readonly BlobServiceClient _blobServiceClient; private readonly ILogger _logger; private readonly bool _usesManagedIdentity; @@ -94,16 +104,21 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics // -------- Public API -------- /// - /// 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 /// {tenantId}/{filename} for an agent's diagnostics-package upload. Cannot - /// be redirected to other tenants' prefixes. TTL is capped at - /// to keep the exposure window tight. + /// be redirected to other tenants' prefixes, and the filename must carry the + /// caller's (AgentDiagnostics-{sessionId}-…zip) + /// so one device cannot name another session's package. An already-existing blob + /// is never re-issued (). TTL is + /// capped at to keep the exposure window tight. /// public virtual async Task 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 } /// - /// Builds the blob-scoped Write+Create-only SAS URI for upload. Branch on the + /// Test seam: whether a blob already exists at . Production + /// issues a HEAD; tests override to simulate a taken name without Azurite. + /// + protected virtual async Task BlobExistsAsync(string blobPath, CancellationToken cancellationToken) + { + var response = await GetContainerClient().GetBlobClient(blobPath).ExistsAsync(cancellationToken); + return response.Value; + } + + /// + /// 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 /// . @@ -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)); } + /// + /// Binds an upload filename to the requesting session: it must be + /// AgentDiagnostics-{sessionId}-{anything}.zip (the shape the agent has always + /// produced, incl. the -preprov/-server-requested suffixes). Without this + /// an authenticated device could name any other session's package in its tenant. + /// Throws on rejection. + /// + 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)); + } + } + /// /// Validates a stored blob path ({tenantId}/{filename}) 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 } } + /// + /// Thrown by when the + /// requested blob name is already taken. Distinct from + /// (malformed input → 400) so the Function can answer 409. + /// + public sealed class HostedBlobAlreadyExistsException : InvalidOperationException + { + public HostedBlobAlreadyExistsException(string blobPath) + : base($"Hosted diagnostics blob already exists: {blobPath}") + { + BlobPath = blobPath; + } + + public string BlobPath { get; } + } + /// /// Return value of . ///