From c1c19c4f7ada8dada5cbdc212f7b49b7dfde9888 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 24 Feb 2026 22:18:21 +0100 Subject: [PATCH] fix diagnostics upload 401 and enrich upload event data - Add /api/diagnostics/upload-url to anonymous routes in AuthenticationMiddleware so the new device-cert-authenticated endpoint is not blocked by the JWT middleware - Add DiagnosticsUploadResult value object to DiagnosticsPackageService carrying BlobName, SasUrlPrefix (truncated, safe to log), and ErrorCode - Change CreateAndUploadAsync return type to DiagnosticsUploadResult for richer error context - Add BuildSasUrlPrefix helper to produce a safe truncated URL for event data - Change UploadToBlobStorageAsync to return (bool success, string errorCode) tuple - Update MonitoringService to populate sasUrl and errorCode in diagnostics_upload_failed event data dict for visible timeline diagnostics Co-Authored-By: Claude Sonnet 4.6 --- .../Core/DiagnosticsPackageService.cs | 77 +++++++++++++++---- .../Monitoring/Core/MonitoringService.cs | 18 +++-- .../Middleware/AuthenticationMiddleware.cs | 3 +- 3 files changed, 77 insertions(+), 21 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Core/DiagnosticsPackageService.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Core/DiagnosticsPackageService.cs index 6c467ead..8b480e7c 100644 --- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Core/DiagnosticsPackageService.cs +++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Core/DiagnosticsPackageService.cs @@ -11,6 +11,27 @@ using AutopilotMonitor.Shared; namespace AutopilotMonitor.Agent.Core.Monitoring.Core { + /// + /// Result of a diagnostics package upload attempt. + /// + public class DiagnosticsUploadResult + { + /// Blob name on success, null on failure or skip. + public string BlobName { get; set; } + + /// + /// First part of the SAS URL (up to and including the first query param) with the token truncated, + /// so the event shows the target container without leaking the full signature. + /// Example: "https://account.blob.core.windows.net/diagnostics?sp=(truncated)" + /// + public string SasUrlPrefix { get; set; } + + /// Human-readable error code/message when upload failed. Null on success or skip. + public string ErrorCode { get; set; } + + public bool Success => BlobName != null; + } + /// /// Creates and uploads a diagnostics ZIP package (agent logs + IME logs + session info) /// to the tenant's Azure Blob Storage container via a short-lived SAS URL. @@ -39,10 +60,11 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Core /// /// Creates a diagnostics ZIP and uploads it to the configured Blob Storage container. - /// Returns the blob name on success, or null if skipped or failed. + /// Returns a DiagnosticsUploadResult with BlobName set on success, or ErrorCode set on failure. + /// Returns null if the upload was skipped (mode=Off, not configured, or OnFailure+succeeded). /// This method is non-fatal: all exceptions are caught and logged. /// - public async Task CreateAndUploadAsync(bool enrollmentSucceeded) + public async Task CreateAndUploadAsync(bool enrollmentSucceeded) { try { @@ -102,24 +124,50 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Core if (uploadUrlResponse == null || !uploadUrlResponse.Success || string.IsNullOrEmpty(uploadUrlResponse.UploadUrl)) { - _logger.Warning("Failed to get diagnostics upload URL from backend — skipping upload"); - return null; + var errorCode = uploadUrlResponse?.Message ?? "Failed to get diagnostics upload URL from backend"; + _logger.Warning($"Failed to get diagnostics upload URL from backend — skipping upload: {errorCode}"); + return new DiagnosticsUploadResult { ErrorCode = errorCode }; } + var sasUrlPrefix = BuildSasUrlPrefix(uploadUrlResponse.UploadUrl); + // Upload to Blob Storage using the freshly obtained URL - var uploaded = await UploadToBlobStorageAsync(zipFileName, zipBytes, uploadUrlResponse.UploadUrl); + var (uploaded, uploadErrorCode) = await UploadToBlobStorageAsync(zipFileName, zipBytes, uploadUrlResponse.UploadUrl); if (uploaded) { _logger.Info($"Diagnostics package uploaded successfully: {zipFileName}"); - return zipFileName; + return new DiagnosticsUploadResult { BlobName = zipFileName, SasUrlPrefix = sasUrlPrefix }; } - return null; + return new DiagnosticsUploadResult { SasUrlPrefix = sasUrlPrefix, ErrorCode = uploadErrorCode }; } catch (Exception ex) { _logger.Warning($"Diagnostics package creation/upload failed (non-fatal): {ex.Message}"); - return null; + return new DiagnosticsUploadResult { ErrorCode = ex.Message }; + } + } + + /// + /// Returns a safe, truncated SAS URL prefix for logging — shows account/container but not the token. + /// Example: "https://account.blob.core.windows.net/diagnostics?sp=(truncated)" + /// + private static string BuildSasUrlPrefix(string sasUrl) + { + try + { + var qIndex = sasUrl.IndexOf('?'); + if (qIndex < 0) return sasUrl; + + // Keep only the first query param name to show which container/permissions are set + var basePath = sasUrl.Substring(0, qIndex); + var query = sasUrl.Substring(qIndex + 1); + var firstParam = query.Split('&')[0].Split('=')[0]; + return $"{basePath}?{firstParam}=(truncated)"; + } + catch + { + return "(url unavailable)"; } } @@ -191,9 +239,9 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Core /// Uploads the ZIP bytes to Azure Blob Storage via PUT with the provided container SAS URL. /// Container SAS URL format: https://account.blob.core.windows.net/container?sv=...&sig=... /// Blob URL is constructed by inserting the blob name before the query string. - /// The URL is passed in directly (fetched on-demand just before upload, never from config). + /// Returns (success, errorCode) — errorCode is null on success, otherwise the last HTTP error. /// - private async Task UploadToBlobStorageAsync(string blobName, byte[] data, string containerSasUrl) + private async Task<(bool success, string errorCode)> UploadToBlobStorageAsync(string blobName, byte[] data, string containerSasUrl) { // Build blob URL: split at '?' to insert blob name var questionMarkIndex = containerSasUrl.IndexOf('?'); @@ -211,6 +259,7 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Core } const int maxRetries = 3; + string lastErrorCode = null; for (int attempt = 1; attempt <= maxRetries; attempt++) { try @@ -225,14 +274,16 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Core if (response.IsSuccessStatusCode) { - return true; + return (true, null); } + lastErrorCode = $"HTTP {(int)response.StatusCode} {response.ReasonPhrase}"; var responseBody = await response.Content.ReadAsStringAsync(); - _logger.Warning($"Blob upload attempt {attempt} failed: HTTP {(int)response.StatusCode} - {responseBody}"); + _logger.Warning($"Blob upload attempt {attempt} failed: {lastErrorCode} - {responseBody}"); } catch (Exception ex) { + lastErrorCode = ex.Message; _logger.Warning($"Blob upload attempt {attempt} failed: {ex.Message}"); } @@ -245,7 +296,7 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Core } _logger.Warning($"Diagnostics package upload failed after {maxRetries} attempts"); - return false; + return (false, lastErrorCode); } } } diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Core/MonitoringService.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Core/MonitoringService.cs index edb1b0e3..f3cf7237 100644 --- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Core/MonitoringService.cs +++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Core/MonitoringService.cs @@ -766,17 +766,21 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Core }); await UploadEventsAsync(); - // Perform the actual ZIP + upload (returns blob name on success, null on failure) - var blobName = await _diagnosticsService.CreateAndUploadAsync(enrollmentSucceeded); - var success = blobName != null; + // Perform the actual ZIP + upload + var uploadResult = await _diagnosticsService.CreateAndUploadAsync(enrollmentSucceeded); + var success = uploadResult?.Success == true; // Emit result event (includes blobName so backend can store it on the session) var resultData = new Dictionary { { "uploadMode", mode } }; - if (blobName != null) - resultData["blobName"] = blobName; + if (uploadResult?.BlobName != null) + resultData["blobName"] = uploadResult.BlobName; + if (uploadResult?.SasUrlPrefix != null) + resultData["sasUrl"] = uploadResult.SasUrlPrefix; + if (uploadResult?.ErrorCode != null) + resultData["errorCode"] = uploadResult.ErrorCode; EmitEvent(new EnrollmentEvent { @@ -788,8 +792,8 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Core Source = "MonitoringService", Phase = EnrollmentPhase.Complete, Message = success - ? $"Diagnostics package uploaded: {blobName}" - : "Diagnostics package upload failed", + ? $"Diagnostics package uploaded: {uploadResult.BlobName}" + : $"Diagnostics package upload failed{(uploadResult?.ErrorCode != null ? $": {uploadResult.ErrorCode}" : "")}", Data = resultData }); await UploadEventsAsync(); diff --git a/src/Backend/AutopilotMonitor.Functions/Middleware/AuthenticationMiddleware.cs b/src/Backend/AutopilotMonitor.Functions/Middleware/AuthenticationMiddleware.cs index e88497b1..98383a91 100644 --- a/src/Backend/AutopilotMonitor.Functions/Middleware/AuthenticationMiddleware.cs +++ b/src/Backend/AutopilotMonitor.Functions/Middleware/AuthenticationMiddleware.cs @@ -44,7 +44,8 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware "/api/platform-stats", "/api/sessions/register", "/api/events/ingest", - "/api/agent/config" + "/api/agent/config", + "/api/diagnostics/upload-url" }; public AuthenticationMiddleware(