diff --git a/src/ApiService/ApiService/Functions/Download.cs b/src/ApiService/ApiService/Functions/Download.cs index 27202afad..3639b07ae 100644 --- a/src/ApiService/ApiService/Functions/Download.cs +++ b/src/ApiService/ApiService/Functions/Download.cs @@ -1,4 +1,5 @@ -using System.Web; +using System.Net; +using System.Web; using Azure.Storage.Sas; using Microsoft.Azure.Functions.Worker; using Microsoft.Azure.Functions.Worker.Http; @@ -45,6 +46,16 @@ public class Download { BlobSasPermissions.Read, TimeSpan.FromMinutes(5)); + if (sasUri is null) { + // Note that we do not validate the existence of the file, only the container. + return await _context.RequestHandling.NotOk(req, + Error.Create( + ErrorCode.INVALID_CONTAINER, + "container not found"), + "generating download file SAS", + HttpStatusCode.NotFound); + } + return RequestHandling.Redirect(req, sasUri); } } diff --git a/src/ApiService/ApiService/onefuzzlib/Containers.cs b/src/ApiService/ApiService/onefuzzlib/Containers.cs index 2d160af64..5e7fb970a 100644 --- a/src/ApiService/ApiService/onefuzzlib/Containers.cs +++ b/src/ApiService/ApiService/onefuzzlib/Containers.cs @@ -20,7 +20,7 @@ public interface IContainers { public Async.Task FindContainer(Container container, StorageType storageType); - public Async.Task GetFileSasUrl(Container container, string name, StorageType storageType, BlobSasPermissions permissions, TimeSpan? duration = null); + public Async.Task GetFileSasUrl(Container container, string name, StorageType storageType, BlobSasPermissions permissions, TimeSpan? duration = null); public Async.Task SaveBlob(Container container, string name, string data, StorageType storageType); public Async.Task GetInstanceId(); @@ -145,8 +145,12 @@ public class Containers : IContainers { return null; } - public async Async.Task GetFileSasUrl(Container container, string name, StorageType storageType, BlobSasPermissions permissions, TimeSpan? duration = null) { - var client = await FindContainer(container, storageType) ?? throw new Exception($"unable to find container: {container} - {storageType}"); + public async Async.Task GetFileSasUrl(Container container, string name, StorageType storageType, BlobSasPermissions permissions, TimeSpan? duration = null) { + var client = await FindContainer(container, storageType); + if (client is null) { + return null; + } + var blobClient = client.GetBlobClient(name); var timeWindow = SasTimeWindow(duration ?? TimeSpan.FromDays(30)); return _storage.GenerateBlobSasUri(permissions, blobClient, timeWindow); diff --git a/src/ApiService/ApiService/onefuzzlib/Events.cs b/src/ApiService/ApiService/onefuzzlib/Events.cs index 2a8039094..8667355f8 100644 --- a/src/ApiService/ApiService/onefuzzlib/Events.cs +++ b/src/ApiService/ApiService/onefuzzlib/Events.cs @@ -114,9 +114,16 @@ namespace Microsoft.OneFuzz.Service { return eventMessageResult.ErrorV; } - var sasUrl = await _containers.GetFileSasUrl(WellKnownContainers.Events, eventId.ToString(), StorageType.Corpus, BlobSasPermissions.Read); + var sasUrl = await _containers.GetFileSasUrl( + WellKnownContainers.Events, + eventId.ToString(), + StorageType.Corpus, + BlobSasPermissions.Read); + if (sasUrl == null) { - return OneFuzzResult.Error(ErrorCode.UNABLE_TO_FIND, $"Could not find container for event with id {eventId}"); + return OneFuzzResult.Error( + ErrorCode.UNABLE_TO_FIND, + $"Could not find container for event with id {eventId}"); } var eventMessage = eventMessageResult.OkV!; @@ -133,8 +140,19 @@ namespace Microsoft.OneFuzz.Service { } public async Task MakeDownloadable(EventMessage eventMessage) { - await _containers.SaveBlob(WellKnownContainers.Events, eventMessage.EventId.ToString(), JsonSerializer.Serialize(eventMessage, _options), StorageType.Corpus); - var sasUrl = await _containers.GetFileSasUrl(WellKnownContainers.Events, eventMessage.EventId.ToString(), StorageType.Corpus, BlobSasPermissions.Read); + await _containers.SaveBlob( + WellKnownContainers.Events, + eventMessage.EventId.ToString(), + JsonSerializer.Serialize(eventMessage, _options), + StorageType.Corpus); + + var sasUrl = await _containers.GetFileSasUrl( + WellKnownContainers.Events, + eventMessage.EventId.ToString(), + StorageType.Corpus, + BlobSasPermissions.Read) + // events container should always exist + ?? throw new InvalidOperationException("Events container is missing"); return new DownloadableEventMessage( eventMessage.EventId, diff --git a/src/ApiService/ApiService/onefuzzlib/Extension.cs b/src/ApiService/ApiService/onefuzzlib/Extension.cs index b972b7782..7995026ec 100644 --- a/src/ApiService/ApiService/onefuzzlib/Extension.cs +++ b/src/ApiService/ApiService/onefuzzlib/Extension.cs @@ -1,4 +1,5 @@ -using System.Text.Json; +using System.IO; +using System.Text.Json; using System.Text.Json.Serialization; using System.Threading.Tasks; using Azure.Core; @@ -374,7 +375,13 @@ public class Extensions : IExtensions { return extensions.Select(extension => extension.GetAsVirtualMachineScaleSetExtension()).ToList(); } - public async Task> ReproExtensions(AzureLocation region, Os reproOs, Guid reproId, ReproConfig reproConfig, Container? setupContainer) { + public async Task> ReproExtensions( + AzureLocation region, + Os reproOs, + Guid reproId, + ReproConfig reproConfig, + Container? setupContainer) { + // TODO: what about contents of repro.ps1 / repro.sh? var report = await _context.Reports.GetReport(reproConfig.Container, reproConfig.Path); var checkedReport = report.EnsureNotNull($"invalid report: {reproConfig}"); @@ -399,13 +406,13 @@ public class Extensions : IExtensions { reproConfig.Path, StorageType.Corpus, BlobSasPermissions.Read - ), + ) ?? throw new InvalidDataException($"failed to get repro config url: container '{reproConfig.Container}' missing"), await _context.Containers.GetFileSasUrl( inputBlob.Container, inputBlob.Name, StorageType.Corpus, BlobSasPermissions.Read - ) + ) ?? throw new InvalidDataException($"failed to get input blob url: container '{inputBlob.Container}' missing"), }; List reproFiles; @@ -437,21 +444,22 @@ public class Extensions : IExtensions { ); foreach (var reproFile in reproFiles) { - urls.AddRange(new List() - { + urls.Add( await _context.Containers.GetFileSasUrl( WellKnownContainers.ReproScripts, reproFile, StorageType.Config, BlobSasPermissions.Read - ), - await _context.Containers.GetFileSasUrl( + ) // this container should always exist + ?? throw new InvalidOperationException("repro scripts container missing")); + + urls.Add(await _context.Containers.GetFileSasUrl( WellKnownContainers.TaskConfigs, $"{reproId}/{scriptName}", StorageType.Config, BlobSasPermissions.Read - ) - }); + ) // this container should always exist + ?? throw new InvalidOperationException("task configs container missing")); } var baseExtension = await AgentConfig(region, reproOs, AgentMode.Repro, urls: urls, withSas: true); @@ -472,13 +480,17 @@ public class Extensions : IExtensions { WellKnownContainers.ProxyConfigs, $"{region}/{proxyId}/config.json", StorageType.Config, - BlobSasPermissions.Read); + BlobSasPermissions.Read) + // this should always exist + ?? throw new InvalidOperationException("proxy config container missing"); var proxyManager = await _context.Containers.GetFileSasUrl( WellKnownContainers.Tools, $"linux/onefuzz-proxy-manager", StorageType.Config, - BlobSasPermissions.Read); + BlobSasPermissions.Read) + // this should always exist + ?? throw new InvalidOperationException("tools container missing"); var baseExtension = await AgentConfig(region, Os.Linux, AgentMode.Proxy, new List { config, proxyManager }, true); diff --git a/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs b/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs index fc0edae74..92ac38f91 100644 --- a/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs @@ -45,7 +45,19 @@ public class NotificationOperations : Orm, INotificationOperations await foreach (var (task, containers) in GetQueueTasks()) { if (containers.Contains(container)) { _logTracer.LogInformation("queuing input {Container} {Filename} {TaskId}", container, filename, task.TaskId); - var url = await _context.Containers.GetFileSasUrl(container, filename, StorageType.Corpus, BlobSasPermissions.Read | BlobSasPermissions.Delete); + + var url = await _context.Containers.GetFileSasUrl( + container, + filename, + StorageType.Corpus, + BlobSasPermissions.Read | BlobSasPermissions.Delete); + + if (url is null) { + _logTracer.LogError("unable to generate sas url for missing container '{Container}'", container); + // try again later + throw new InvalidOperationException($"container '{container}' is missing"); + } + await _context.Queue.SendMessage(task.TaskId.ToString(), url.ToString(), StorageType.Corpus); } } @@ -93,9 +105,9 @@ public class NotificationOperations : Orm, INotificationOperations public IAsyncEnumerable<(Task, IEnumerable)> GetQueueTasks() { // Nullability mismatch: We filter tuples where the containers are null return _context.TaskOperations.SearchStates(states: TaskStateHelper.AvailableStates) - .Select(task => (task, _context.TaskOperations.GetInputContainerQueues(task.Config))) - .Where(taskTuple => taskTuple.Item2.IsOk && taskTuple.Item2.OkV != null) - .Select(x => (Task: x.Item1, Containers: x.Item2.OkV))!; + .Select(task => (task, containers: _context.TaskOperations.GetInputContainerQueues(task.Config))) + .Where(x => x.containers.IsOk && x.containers.OkV is not null) + .Select(x => (Task: x.task, Containers: x.containers.OkV!)); } public async Async.Task> Create(Container container, NotificationTemplate config, bool replaceExisting) { diff --git a/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs b/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs index 39bbc46b5..ce3c7c8f7 100644 --- a/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs @@ -130,8 +130,16 @@ public class ProxyOperations : StatefulOrm, IPr public async Async.Task SaveProxyConfig(Proxy proxy) { var forwards = await GetForwards(proxy); - var url = (await _context.Containers.GetFileSasUrl(WellKnownContainers.ProxyConfigs, $"{proxy.Region}/{proxy.ProxyId}/config.json", StorageType.Config, BlobSasPermissions.Read)).EnsureNotNull("Can't generate file sas"); - var queueSas = await _context.Queue.GetQueueSas("proxy", StorageType.Config, QueueSasPermissions.Add).EnsureNotNull("can't generate queue sas") ?? throw new Exception("Queue sas is null"); + var url = (await _context.Containers.GetFileSasUrl( + WellKnownContainers.ProxyConfigs, + $"{proxy.Region}/{proxy.ProxyId}/config.json", + StorageType.Config, + BlobSasPermissions.Read)).EnsureNotNull("proxy configs container missing"); + + var queueSas = await _context.Queue.GetQueueSas( + "proxy", + StorageType.Config, + QueueSasPermissions.Add).EnsureNotNull("can't generate queue sas") ?? throw new Exception("Queue sas is null"); var proxyConfig = new ProxyConfig( Url: url, diff --git a/src/ApiService/IntegrationTests/DownloadTests.cs b/src/ApiService/IntegrationTests/DownloadTests.cs index 1912071ca..f0afc9bb6 100644 --- a/src/ApiService/IntegrationTests/DownloadTests.cs +++ b/src/ApiService/IntegrationTests/DownloadTests.cs @@ -55,6 +55,22 @@ public abstract class DownloadTestBase : FunctionTestBase { Assert.Equal(ErrorCode.INVALID_REQUEST.ToString(), err.Title); } + [Fact] + public async Async.Task Container_NotFound_Generates404() { + var req = TestHttpRequestData.Empty("GET"); + // this container won't exist because we haven't explicitly created it + var url = new UriBuilder(req.Url) { Query = "container=xxx&filename=yyy" }.Uri; + req.SetUrl(url); + + var func = new Download(Context); + + var result = await func.Run(req); + Assert.Equal(HttpStatusCode.NotFound, result.StatusCode); + + var err = BodyAs(result); + Assert.Equal(ErrorCode.INVALID_CONTAINER.ToString(), err.Title); + } + [Fact] public async Async.Task Download_RedirectsToResult_WithLocationHeader() { // set up a file to download