From 7789ca9455c9ce91cdc6c731e6a306f563789588 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 3 Jun 2026 14:14:45 -0700 Subject: [PATCH 1/8] Add Antivirus Scanner --- Directory.Packages.props | 4 + OrchardCore.slnx | 2 + mkdocs.yml | 1 + src/OrchardCore.AspireHost/ClamAV.cs | 53 +++++ .../OrchardCore.AspireHost.csproj | 19 ++ src/OrchardCore.AspireHost/Program.cs | 20 ++ .../Properties/launchSettings.json | 29 +++ .../appsettings.Development.json | 9 + src/OrchardCore.AspireHost/appsettings.json | 8 + src/OrchardCore.AspireHost/aspire.config.json | 5 + .../ClamAVOptions.cs | 14 ++ .../OrchardCore.Antivirus.ClamAV/Manifest.cs | 15 ++ .../OrchardCore.Antivirus.ClamAV.csproj | 21 ++ .../Services/ClamAVAntivirusScanner.cs | 159 ++++++++++++++ .../OrchardCore.Antivirus.ClamAV/Startup.cs | 25 +++ .../ImportRemoteInstanceController.cs | 70 ++++++- .../OrchardCore.Deployment.Remote.csproj | 1 + .../OrchardCore.Deployment.Remote/Startup.cs | 3 + .../Controllers/ImportController.cs | 70 ++++++- .../OrchardCore.Deployment.csproj | 1 + .../OrchardCore.Deployment/Startup.cs | 3 + .../OrchardCore.Media.AmazonS3/Startup.cs | 2 + .../OrchardCore.Media.Azure/Startup.cs | 3 +- .../OrchardCore.Media/Startup.cs | 4 +- ...rdCore.Application.Cms.Core.Targets.csproj | 1 + .../AntivirusResult.cs | 21 ++ .../AntivirusScanContext.cs | 19 ++ .../AntivirusScanningException.cs | 14 ++ .../IAntivirusScanner.cs | 6 + .../NullAntivirusScanner.cs | 7 + .../DefaultMediaFileStore.cs | 89 +++++++- .../modules/Antivirus.ClamAV/README.md | 50 +++++ src/docs/releases/3.0.0.md | 4 +- .../AssetUrlShortcodeTests.cs | 1 + .../ClamAVAntivirusScannerTests.cs | 198 ++++++++++++++++++ .../DefaultMediaFileStoreTests.cs | 167 +++++++++++++++ .../OrchardCore.Media/ImageShortcodeTests.cs | 1 + .../MediaOrchardHelperExtensionsTests.cs | 1 + .../OrchardCore.Tests.csproj | 2 + 39 files changed, 1113 insertions(+), 9 deletions(-) create mode 100644 src/OrchardCore.AspireHost/ClamAV.cs create mode 100644 src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj create mode 100644 src/OrchardCore.AspireHost/Program.cs create mode 100644 src/OrchardCore.AspireHost/Properties/launchSettings.json create mode 100644 src/OrchardCore.AspireHost/appsettings.Development.json create mode 100644 src/OrchardCore.AspireHost/appsettings.json create mode 100644 src/OrchardCore.AspireHost/aspire.config.json create mode 100644 src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Manifest.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/OrchardCore.Antivirus.ClamAV.csproj create mode 100644 src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Services/ClamAVAntivirusScanner.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Startup.cs create mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusResult.cs create mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanContext.cs create mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanningException.cs create mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/IAntivirusScanner.cs create mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/NullAntivirusScanner.cs create mode 100644 src/docs/reference/modules/Antivirus.ClamAV/README.md create mode 100644 test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVAntivirusScannerTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index e06af86a673..44f7093142e 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -110,4 +110,8 @@ + + + + diff --git a/OrchardCore.slnx b/OrchardCore.slnx index 8ed8126a7e8..7001db16050 100644 --- a/OrchardCore.slnx +++ b/OrchardCore.slnx @@ -65,6 +65,7 @@ + @@ -116,6 +117,7 @@ + diff --git a/mkdocs.yml b/mkdocs.yml index 456ab77a348..68772d75311 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -209,6 +209,7 @@ nav: - Media: - Media: reference/modules/Media/README.md - Media Slugify: reference/modules/Media.Slugify/README.md + - ClamAV Antivirus Scanner: reference/modules/Antivirus.ClamAV/README.md - Media Amazon S3: reference/modules/Media.AmazonS3/README.md - Media Azure: reference/modules/Media.Azure/README.md - ReCaptcha: reference/modules/ReCaptcha/README.md diff --git a/src/OrchardCore.AspireHost/ClamAV.cs b/src/OrchardCore.AspireHost/ClamAV.cs new file mode 100644 index 00000000000..f8f94131051 --- /dev/null +++ b/src/OrchardCore.AspireHost/ClamAV.cs @@ -0,0 +1,53 @@ +using Aspire.Hosting; +using Aspire.Hosting.ApplicationModel; + +namespace OrchardCore.AspireHost; + +public sealed class ClamAVResource : ContainerResource, IResourceWithConnectionString +{ + internal const string PrimaryEndpointName = "tcp"; + + private EndpointReference _primaryEndpoint; + + public ClamAVResource(string name) + : base(name) + { + } + + public EndpointReference PrimaryEndpoint + => _primaryEndpoint ??= new EndpointReference(this, PrimaryEndpointName); + + public ReferenceExpression ConnectionStringExpression + => ReferenceExpression.Create( + $"tcp://{PrimaryEndpoint.Property(EndpointProperty.Host)}:{PrimaryEndpoint.Property(EndpointProperty.Port)}"); +} + +internal static class ClamAVResourceBuilderExtensions +{ + public static IResourceBuilder AddClamAV( + this IDistributedApplicationBuilder builder, + string name, + int? port = null) + { + var resource = new ClamAVResource(name); + + return builder.AddResource(resource) + .WithImage("clamav/clamav") + .WithImageRegistry("docker.io") + .WithImageTag("latest") + .WithEnvironment("CLAMAV_NO_FRESHCLAMD", "true") + .WithEndpoint(port: port, name: ClamAVResource.PrimaryEndpointName, targetPort: 3310); + } + + public static IResourceBuilder WithDataVolume( + this IResourceBuilder builder, + string name, + bool isReadOnly = false) => + builder.WithVolume(name, "/var/lib/clamav", isReadOnly); + + public static IResourceBuilder WithDataBindMount( + this IResourceBuilder builder, + string source, + bool isReadOnly = false) => + builder.WithBindMount(source, "/var/lib/clamav", isReadOnly); +} diff --git a/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj b/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj new file mode 100644 index 00000000000..19730ee4406 --- /dev/null +++ b/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj @@ -0,0 +1,19 @@ + + + + net10.0 + Exe + true + false + 53f65258-c4bc-45d0-ab79-c8309ff77904 + + + + + + + + + + + diff --git a/src/OrchardCore.AspireHost/Program.cs b/src/OrchardCore.AspireHost/Program.cs new file mode 100644 index 00000000000..2789ab5058d --- /dev/null +++ b/src/OrchardCore.AspireHost/Program.cs @@ -0,0 +1,20 @@ +using Aspire.Hosting; +using Aspire.Hosting.ApplicationModel; +using OrchardCore.AspireHost; + +var builder = DistributedApplication.CreateBuilder(args); + +var clamAv = builder.AddClamAV("antivirus") + .WithDataVolume("clamavdb"); + +builder.AddProject("OrchardCoreCms") + .WithExternalHttpEndpoints() + .WithReference(clamAv) + .WithEnvironment("OrchardCore__Antivirus_ClamAV__Host", clamAv.Resource.PrimaryEndpoint.Property(EndpointProperty.Host)) + .WithEnvironment("OrchardCore__Antivirus_ClamAV__Port", clamAv.Resource.PrimaryEndpoint.Property(EndpointProperty.Port)) + .WithEnvironment("OrchardCore__Antivirus_ClamAV__ConnectTimeoutSeconds", "5") + .WithEnvironment("OrchardCore__Antivirus_ClamAV__TransferTimeoutSeconds", "30"); + +var app = builder.Build(); + +await app.RunAsync(); diff --git a/src/OrchardCore.AspireHost/Properties/launchSettings.json b/src/OrchardCore.AspireHost/Properties/launchSettings.json new file mode 100644 index 00000000000..81cd4e5f970 --- /dev/null +++ b/src/OrchardCore.AspireHost/Properties/launchSettings.json @@ -0,0 +1,29 @@ +{ + "$schema": "https://json.schemastore.org/launchsettings.json", + "profiles": { + "https": { + "commandName": "Project", + "dotnetRunMessages": true, + "launchBrowser": true, + "applicationUrl": "https://localhost:17262;http://localhost:15209", + "environmentVariables": { + "ASPNETCORE_ENVIRONMENT": "Development", + "DOTNET_ENVIRONMENT": "Development", + "DOTNET_DASHBOARD_OTLP_ENDPOINT_URL": "https://localhost:21196", + "DOTNET_RESOURCE_SERVICE_ENDPOINT_URL": "https://localhost:22006" + } + }, + "http": { + "commandName": "Project", + "dotnetRunMessages": true, + "launchBrowser": true, + "applicationUrl": "http://localhost:15209", + "environmentVariables": { + "ASPNETCORE_ENVIRONMENT": "Development", + "DOTNET_ENVIRONMENT": "Development", + "DOTNET_DASHBOARD_OTLP_ENDPOINT_URL": "http://localhost:19032", + "DOTNET_RESOURCE_SERVICE_ENDPOINT_URL": "http://localhost:20030" + } + } + } +} diff --git a/src/OrchardCore.AspireHost/appsettings.Development.json b/src/OrchardCore.AspireHost/appsettings.Development.json new file mode 100644 index 00000000000..31c092aa450 --- /dev/null +++ b/src/OrchardCore.AspireHost/appsettings.Development.json @@ -0,0 +1,9 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Information", + "Microsoft.AspNetCore": "Warning", + "Aspire.Hosting.Dcp": "Warning" + } + } +} diff --git a/src/OrchardCore.AspireHost/appsettings.json b/src/OrchardCore.AspireHost/appsettings.json new file mode 100644 index 00000000000..0c208ae9181 --- /dev/null +++ b/src/OrchardCore.AspireHost/appsettings.json @@ -0,0 +1,8 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Information", + "Microsoft.AspNetCore": "Warning" + } + } +} diff --git a/src/OrchardCore.AspireHost/aspire.config.json b/src/OrchardCore.AspireHost/aspire.config.json new file mode 100644 index 00000000000..da58ac9e4c9 --- /dev/null +++ b/src/OrchardCore.AspireHost/aspire.config.json @@ -0,0 +1,5 @@ +{ + "appHost": { + "path": "OrchardCore.AspireHost.csproj" + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs new file mode 100644 index 00000000000..49c8351597f --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs @@ -0,0 +1,14 @@ +namespace OrchardCore.Antivirus.ClamAV; + +public sealed class ClamAvOptions +{ + public const string ConfigSection = "OrchardCore_Antivirus_ClamAV"; + + public string Host { get; set; } + + public int Port { get; set; } = 3310; + + public int ConnectTimeoutSeconds { get; set; } = 5; + + public int TransferTimeoutSeconds { get; set; } = 30; +} diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Manifest.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Manifest.cs new file mode 100644 index 00000000000..e4ff3a66f4e --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Manifest.cs @@ -0,0 +1,15 @@ +using OrchardCore.Modules.Manifest; + +[assembly: Module( + Name = "ClamAV Antivirus Scanner", + Author = ManifestConstants.OrchardCoreTeam, + Website = ManifestConstants.OrchardCoreWebsite, + Version = ManifestConstants.OrchardCoreVersion +)] + +[assembly: Feature( + Id = "OrchardCore.Antivirus.ClamAV", + Name = "ClamAV Antivirus Scanner", + Description = "Scans files with ClamAV before Orchard Core stores or imports them.", + Category = "Security" +)] diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/OrchardCore.Antivirus.ClamAV.csproj b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/OrchardCore.Antivirus.ClamAV.csproj new file mode 100644 index 00000000000..df290d1277a --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/OrchardCore.Antivirus.ClamAV.csproj @@ -0,0 +1,21 @@ + + + + true + OrchardCore Antivirus ClamAV + $(OCCMSDescription) + + Provides a ClamAV-backed antivirus scanner for Orchard Core uploads. + $(PackageTags) OrchardCoreCMS ContentManagement Security + + + + + + + + + + + + diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Services/ClamAVAntivirusScanner.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Services/ClamAVAntivirusScanner.cs new file mode 100644 index 00000000000..45d34429ecb --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Services/ClamAVAntivirusScanner.cs @@ -0,0 +1,159 @@ +using System.Buffers.Binary; +using System.Net.Sockets; +using System.Text; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using OrchardCore.FileStorage; + +namespace OrchardCore.Antivirus.ClamAV.Services; + +public sealed class ClamAVAntivirusScanner : IAntivirusScanner +{ + private const int BufferSize = 81920; + private static readonly byte[] _scanCommand = "nINSTREAM\n"u8.ToArray(); + private readonly ClamAvOptions _options; + private readonly ILogger _logger; + + public ClamAVAntivirusScanner( + IOptions options, + ILogger logger) + { + _options = options.Value; + _logger = logger; + } + + public async Task ScanAsync(AntivirusScanContext context, Stream stream) + { + ValidateOptions(); + + try + { + using var client = new TcpClient(); + using var connectCancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(_options.ConnectTimeoutSeconds)); + + await client.ConnectAsync(_options.Host, _options.Port, connectCancellationTokenSource.Token); + + client.SendTimeout = (int)TimeSpan.FromSeconds(_options.TransferTimeoutSeconds).TotalMilliseconds; + client.ReceiveTimeout = (int)TimeSpan.FromSeconds(_options.TransferTimeoutSeconds).TotalMilliseconds; + + await using var networkStream = client.GetStream(); + using var transferCancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(_options.TransferTimeoutSeconds)); + + await networkStream.WriteAsync(_scanCommand, transferCancellationTokenSource.Token); + + var buffer = new byte[BufferSize]; + + int bytesRead; + + while ((bytesRead = await stream.ReadAsync(buffer.AsMemory(0, buffer.Length), transferCancellationTokenSource.Token)) > 0) + { + var prefix = new byte[sizeof(int)]; + BinaryPrimitives.WriteInt32BigEndian(prefix, bytesRead); + + await networkStream.WriteAsync(prefix, transferCancellationTokenSource.Token); + await networkStream.WriteAsync(buffer.AsMemory(0, bytesRead), transferCancellationTokenSource.Token); + } + + await networkStream.WriteAsync(new byte[sizeof(int)], transferCancellationTokenSource.Token); + await networkStream.FlushAsync(transferCancellationTokenSource.Token); + + var response = await ReadResponseAsync(networkStream, transferCancellationTokenSource.Token); + + return CreateResult(context, response); + } + catch (OperationCanceledException exception) + { + _logger.LogError(exception, "ClamAV timed out while scanning '{FileName}'.", context.FileName); + + throw new AntivirusScanningException($"The ClamAV antivirus scanner timed out while scanning '{context.FileName}'.", exception); + } + catch (SocketException exception) + { + _logger.LogError(exception, "ClamAV could not be reached while scanning '{FileName}'.", context.FileName); + + throw new AntivirusScanningException($"The ClamAV antivirus scanner could not be reached while scanning '{context.FileName}'.", exception); + } + catch (IOException exception) + { + _logger.LogError(exception, "ClamAV failed while scanning '{FileName}'.", context.FileName); + + throw new AntivirusScanningException($"The ClamAV antivirus scanner failed while scanning '{context.FileName}'.", exception); + } + } + + private static AntivirusResult CreateResult(AntivirusScanContext context, string response) + { + if (string.Equals(response, "stream: OK", StringComparison.Ordinal)) + { + return AntivirusResult.Clean; + } + + if (response.EndsWith(" FOUND", StringComparison.Ordinal)) + { + var signature = response; + var separatorIndex = signature.IndexOf(": ", StringComparison.Ordinal); + + if (separatorIndex >= 0) + { + signature = signature[(separatorIndex + 2)..]; + } + + signature = signature[..^" FOUND".Length]; + + return AntivirusResult.Unsafe( + $"The uploaded file '{context.FileName}' was rejected because ClamAV detected '{signature}'.", + signature); + } + + throw new AntivirusScanningException( + $"The ClamAV antivirus scanner returned an unexpected response while scanning '{context.FileName}': {response}"); + } + + private static async Task ReadResponseAsync(NetworkStream stream, CancellationToken cancellationToken) + { + using var responseStream = new MemoryStream(); + var buffer = new byte[1]; + + while (true) + { + var bytesRead = await stream.ReadAsync(buffer.AsMemory(0, 1), cancellationToken); + + if (bytesRead == 0) + { + break; + } + + if (buffer[0] == '\n') + { + break; + } + + responseStream.WriteByte(buffer[0]); + } + + return Encoding.ASCII.GetString(responseStream.ToArray()); + } + + private void ValidateOptions() + { + if (string.IsNullOrWhiteSpace(_options.Host)) + { + throw new AntivirusScanningException("The ClamAV antivirus scanner is enabled but the host setting is missing."); + } + + if (_options.Port is < 1 or > 65535) + { + throw new AntivirusScanningException("The ClamAV antivirus scanner is enabled but the port setting is invalid."); + } + + if (_options.ConnectTimeoutSeconds <= 0) + { + throw new AntivirusScanningException("The ClamAV antivirus scanner is enabled but the connection timeout must be greater than zero."); + } + + if (_options.TransferTimeoutSeconds <= 0) + { + throw new AntivirusScanningException("The ClamAV antivirus scanner is enabled but the transfer timeout must be greater than zero."); + } + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Startup.cs new file mode 100644 index 00000000000..22875d3b8b3 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Startup.cs @@ -0,0 +1,25 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using OrchardCore.Environment.Shell.Configuration; +using OrchardCore.FileStorage; +using OrchardCore.Antivirus.ClamAV.Services; +using OrchardCore.Modules; + +namespace OrchardCore.Antivirus.ClamAV; + +public sealed class Startup : StartupBase +{ + private readonly IShellConfiguration _configuration; + + public Startup(IShellConfiguration configuration) + { + _configuration = configuration; + } + + public override void ConfigureServices(IServiceCollection services) + { + services.Configure(_configuration.GetSection(ClamAvOptions.ConfigSection)); + + services.Replace(ServiceDescriptor.Singleton()); + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs index e9f1e35401e..c2060b51bee 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Localization; using Microsoft.Extensions.Logging; +using OrchardCore.FileStorage; using OrchardCore.Deployment.Remote.Services; using OrchardCore.Deployment.Remote.ViewModels; using OrchardCore.Deployment.Services; @@ -22,6 +23,7 @@ public sealed class ImportRemoteInstanceController : Controller private readonly INotifier _notifier; private readonly ILogger _logger; private readonly IDataProtector _dataProtector; + private readonly IAntivirusScanner _fileUploadScanner; internal readonly IHtmlLocalizer H; internal readonly IStringLocalizer S; @@ -30,12 +32,14 @@ public ImportRemoteInstanceController( IDataProtectionProvider dataProtectionProvider, RemoteClientService remoteClientService, IDeploymentManager deploymentManager, + IAntivirusScanner fileUploadScanner, INotifier notifier, IHtmlLocalizer htmlLocalizer, IStringLocalizer stringLocalizer, ILogger logger) { _deploymentManager = deploymentManager; + _fileUploadScanner = fileUploadScanner; _notifier = notifier; _logger = logger; _remoteClientService = remoteClientService; @@ -76,9 +80,12 @@ public async Task Import(ImportViewModel model) try { + await using var uploadedStream = model.Content.OpenReadStream(); + await using var scannedStream = await CreateScannedStreamAsync(model.Content.FileName, model.Content.ContentType, uploadedStream); + using (var fs = System.IO.File.Create(tempArchiveName)) { - await model.Content.CopyToAsync(fs); + await scannedStream.CopyToAsync(fs); } ZipFile.ExtractToDirectory(tempArchiveName, tempArchiveFolder); @@ -112,4 +119,65 @@ public async Task Import(ImportViewModel model) return Ok(); } + + private async Task CreateScannedStreamAsync(string fileName, string contentType, Stream stream) + { + if (_fileUploadScanner is NullAntivirusScanner) + { + return stream; + } + + var scanStream = stream; + + if (scanStream.CanSeek) + { + scanStream.Position = 0; + } + else + { + scanStream = await CreateTemporarySeekableStreamAsync(scanStream); + } + + try + { + var result = await _fileUploadScanner.ScanAsync( + new AntivirusScanContext(fileName, scanStream.Length, contentType), + scanStream); + + if (!result.IsClean) + { + throw new AntivirusScanningException(result.Message ?? $"The uploaded file '{fileName}' was rejected by the anti-virus scanner."); + } + + scanStream.Position = 0; + + return scanStream; + } + catch + { + if (scanStream != stream) + { + scanStream.Dispose(); + } + + throw; + } + } + + private static async Task CreateTemporarySeekableStreamAsync(Stream stream) + { + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + var tempStream = new FileStream( + tempFilePath, + FileMode.CreateNew, + FileAccess.ReadWrite, + FileShare.None, + 81920, + FileOptions.Asynchronous | FileOptions.DeleteOnClose); + + await stream.CopyToAsync(tempStream); + tempStream.Position = 0; + + return tempStream; + } } diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/OrchardCore.Deployment.Remote.csproj b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/OrchardCore.Deployment.Remote.csproj index 8c1a77018ce..ac939193b4f 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/OrchardCore.Deployment.Remote.csproj +++ b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/OrchardCore.Deployment.Remote.csproj @@ -19,6 +19,7 @@ + diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs index b4eb3c0152d..87b52cef58b 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs @@ -1,6 +1,8 @@ using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using OrchardCore.Deployment.Remote; using OrchardCore.Deployment.Remote.Services; +using OrchardCore.FileStorage; using OrchardCore.Modules; using OrchardCore.Navigation; using OrchardCore.Security.Permissions; @@ -12,6 +14,7 @@ public sealed class Startup : StartupBase public override void ConfigureServices(IServiceCollection services) { services.AddHttpClient(); + services.TryAddSingleton(); services.AddNavigationProvider(); services.AddScoped(); diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs b/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs index 14c4560fa5f..7b90152e977 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Localization; using Microsoft.Extensions.Logging; +using OrchardCore.FileStorage; using OrchardCore.Admin; using OrchardCore.Deployment.Services; using OrchardCore.Deployment.ViewModels; @@ -23,6 +24,7 @@ public sealed class ImportController : Controller private readonly IAuthorizationService _authorizationService; private readonly INotifier _notifier; private readonly ILogger _logger; + private readonly IAntivirusScanner _fileUploadScanner; internal readonly IHtmlLocalizer H; internal readonly IStringLocalizer S; @@ -30,6 +32,7 @@ public sealed class ImportController : Controller public ImportController( IDeploymentManager deploymentManager, IAuthorizationService authorizationService, + IAntivirusScanner fileUploadScanner, INotifier notifier, ILogger logger, IHtmlLocalizer htmlLocalizer, @@ -38,6 +41,7 @@ IStringLocalizer stringLocalizer { _deploymentManager = deploymentManager; _authorizationService = authorizationService; + _fileUploadScanner = fileUploadScanner; _notifier = notifier; _logger = logger; H = htmlLocalizer; @@ -69,9 +73,12 @@ public async Task Import(IFormFile importedPackage) try { + await using var uploadedStream = importedPackage.OpenReadStream(); + await using var scannedStream = await CreateScannedStreamAsync(importedPackage.FileName, importedPackage.ContentType, uploadedStream); + using (var stream = new FileStream(tempArchiveName, FileMode.Create)) { - await importedPackage.CopyToAsync(stream); + await scannedStream.CopyToAsync(stream); } if (importedPackage.FileName.EndsWith(".zip")) @@ -186,4 +193,65 @@ public async Task Json(ImportJsonViewModel model) return View(model); } + + private async Task CreateScannedStreamAsync(string fileName, string contentType, Stream stream) + { + if (_fileUploadScanner is NullAntivirusScanner) + { + return stream; + } + + var scanStream = stream; + + if (scanStream.CanSeek) + { + scanStream.Position = 0; + } + else + { + scanStream = await CreateTemporarySeekableStreamAsync(scanStream); + } + + try + { + var result = await _fileUploadScanner.ScanAsync( + new AntivirusScanContext(fileName, scanStream.Length, contentType), + scanStream); + + if (!result.IsClean) + { + throw new AntivirusScanningException(result.Message ?? $"The uploaded file '{fileName}' was rejected by the anti-virus scanner."); + } + + scanStream.Position = 0; + + return scanStream; + } + catch + { + if (scanStream != stream) + { + scanStream.Dispose(); + } + + throw; + } + } + + private static async Task CreateTemporarySeekableStreamAsync(Stream stream) + { + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + var tempStream = new FileStream( + tempFilePath, + FileMode.CreateNew, + FileAccess.ReadWrite, + FileShare.None, + 81920, + FileOptions.Asynchronous | FileOptions.DeleteOnClose); + + await stream.CopyToAsync(tempStream); + tempStream.Position = 0; + + return tempStream; + } } diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment/OrchardCore.Deployment.csproj b/src/OrchardCore.Modules/OrchardCore.Deployment/OrchardCore.Deployment.csproj index 6fcc560bfc0..7baacaaf38b 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment/OrchardCore.Deployment.csproj +++ b/src/OrchardCore.Modules/OrchardCore.Deployment/OrchardCore.Deployment.csproj @@ -20,6 +20,7 @@ + diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs index 7911c0cb69c..b95342eb2c2 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs @@ -1,5 +1,6 @@ using System.Text.Json.Serialization; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using OrchardCore.Data; using OrchardCore.Data.Migration; using OrchardCore.Deployment.Core; @@ -9,6 +10,7 @@ using OrchardCore.Deployment.Recipes; using OrchardCore.Deployment.Steps; using OrchardCore.DisplayManagement.Handlers; +using OrchardCore.FileStorage; using OrchardCore.Modules; using OrchardCore.Navigation; using OrchardCore.Recipes; @@ -20,6 +22,7 @@ public sealed class Startup : StartupBase { public override void ConfigureServices(IServiceCollection services) { + services.TryAddSingleton(); services.AddDeploymentServices(); services.AddNavigationProvider(); diff --git a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs index 57bc1aa610e..13d651724d7 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs @@ -103,6 +103,7 @@ public override void ConfigureServices(IServiceCollection services) var mediaOptions = serviceProvider.GetRequiredService>().Value; var mediaEventHandlers = serviceProvider.GetServices(); var mediaCreatingEventHandlers = serviceProvider.GetServices(); + var antivirusScanner = serviceProvider.GetRequiredService(); var clock = serviceProvider.GetRequiredService(); var logger = serviceProvider.GetRequiredService>(); var amazonS3Client = serviceProvider.GetService(); @@ -126,6 +127,7 @@ public override void ConfigureServices(IServiceCollection services) mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, + antivirusScanner, logger); })); diff --git a/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs index 903d45f22f3..c649a383d2e 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs @@ -95,6 +95,7 @@ public override void ConfigureServices(IServiceCollection services) var contentTypeProvider = serviceProvider.GetRequiredService(); var mediaEventHandlers = serviceProvider.GetServices(); var mediaCreatingEventHandlers = serviceProvider.GetServices(); + var fileUploadScanner = serviceProvider.GetRequiredService(); var logger = serviceProvider.GetRequiredService>(); var fileStore = new BlobFileStore(blobStorageOptions, clock, contentTypeProvider); @@ -109,7 +110,7 @@ public override void ConfigureServices(IServiceCollection services) mediaUrlBase = fileStore.Combine(originalPathBase.Value, mediaUrlBase); } - return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, logger); + return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileUploadScanner, logger); })); services.AddSingleton(); diff --git a/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs index bc51125e5f4..e153e0c8ebc 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs @@ -87,6 +87,7 @@ public override void ConfigureServices(IServiceCollection services) services.AddResourceConfiguration(); services.AddTransient, MediaOptionsConfiguration>(); + services.TryAddSingleton(); services.AddSingleton(serviceProvider => { @@ -115,6 +116,7 @@ public override void ConfigureServices(IServiceCollection services) var mediaOptions = serviceProvider.GetRequiredService>().Value; var mediaEventHandlers = serviceProvider.GetServices(); var mediaCreatingEventHandlers = serviceProvider.GetServices(); + var fileUploadScanner = serviceProvider.GetRequiredService(); var fileSystemStoreLogger = serviceProvider.GetRequiredService>(); var defaultMediaFileStoreLogger = serviceProvider.GetRequiredService>(); @@ -132,7 +134,7 @@ public override void ConfigureServices(IServiceCollection services) mediaUrlBase = fileStore.Combine(originalPathBase.Value, mediaUrlBase); } - return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, defaultMediaFileStoreLogger); + return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileUploadScanner, defaultMediaFileStoreLogger); }); services.AddPermissionProvider(); diff --git a/src/OrchardCore/OrchardCore.Application.Cms.Core.Targets/OrchardCore.Application.Cms.Core.Targets.csproj b/src/OrchardCore/OrchardCore.Application.Cms.Core.Targets/OrchardCore.Application.Cms.Core.Targets.csproj index 4b7a20af08a..28f81b24bef 100644 --- a/src/OrchardCore/OrchardCore.Application.Cms.Core.Targets/OrchardCore.Application.Cms.Core.Targets.csproj +++ b/src/OrchardCore/OrchardCore.Application.Cms.Core.Targets/OrchardCore.Application.Cms.Core.Targets.csproj @@ -41,6 +41,7 @@ + diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusResult.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusResult.cs new file mode 100644 index 00000000000..5bf27e47128 --- /dev/null +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusResult.cs @@ -0,0 +1,21 @@ +namespace OrchardCore.FileStorage; + +public sealed class AntivirusResult +{ + private AntivirusResult(bool isClean, string message, string threatName) + { + IsClean = isClean; + Message = message; + ThreatName = threatName; + } + + public bool IsClean { get; } + + public string Message { get; } + + public string ThreatName { get; } + + public static AntivirusResult Clean { get; } = new(true, null, null); + + public static AntivirusResult Unsafe(string message, string threatName = null) => new(false, message, threatName); +} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanContext.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanContext.cs new file mode 100644 index 00000000000..e48acd6c626 --- /dev/null +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanContext.cs @@ -0,0 +1,19 @@ +namespace OrchardCore.FileStorage; + +public sealed class AntivirusScanContext +{ + public AntivirusScanContext(string path, long? length = null, string contentType = null) + { + Path = path; + Length = length; + ContentType = contentType; + } + + public string Path { get; } + + public string FileName => System.IO.Path.GetFileName(Path); + + public long? Length { get; } + + public string ContentType { get; } +} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanningException.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanningException.cs new file mode 100644 index 00000000000..c48e43168d6 --- /dev/null +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanningException.cs @@ -0,0 +1,14 @@ +namespace OrchardCore.FileStorage; + +public class AntivirusScanningException : FileStoreException +{ + public AntivirusScanningException(string message) + : base(message) + { + } + + public AntivirusScanningException(string message, Exception innerException) + : base(message, innerException) + { + } +} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/IAntivirusScanner.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/IAntivirusScanner.cs new file mode 100644 index 00000000000..b2ffb86fc12 --- /dev/null +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/IAntivirusScanner.cs @@ -0,0 +1,6 @@ +namespace OrchardCore.FileStorage; + +public interface IAntivirusScanner +{ + Task ScanAsync(AntivirusScanContext context, Stream stream); +} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/NullAntivirusScanner.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/NullAntivirusScanner.cs new file mode 100644 index 00000000000..378d6c79f84 --- /dev/null +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/NullAntivirusScanner.cs @@ -0,0 +1,7 @@ +namespace OrchardCore.FileStorage; + +public sealed class NullAntivirusScanner : IAntivirusScanner +{ + public Task ScanAsync(AntivirusScanContext context, Stream stream) + => Task.FromResult(AntivirusResult.Clean); +} diff --git a/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs b/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs index 48b01b34ded..54c9d581fdc 100644 --- a/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs +++ b/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs @@ -17,6 +17,7 @@ public class DefaultMediaFileStore : IMediaFileStore private readonly string _cdnBaseUrl; private readonly IEnumerable _mediaEventHandlers; private readonly IEnumerable _mediaCreatingEventHandlers; + private readonly IAntivirusScanner _antivirusScanner; private readonly ILogger _logger; private bool _requestBasePathValidated; @@ -27,6 +28,7 @@ public DefaultMediaFileStore( string cdnBaseUrl, IEnumerable mediaEventHandlers, IEnumerable mediaCreatingEventHandlers, + IAntivirusScanner antivirusScanner, ILogger logger) { _fileStore = fileStore; @@ -39,6 +41,7 @@ public DefaultMediaFileStore( _mediaEventHandlers = mediaEventHandlers; _mediaCreatingEventHandlers = mediaCreatingEventHandlers; + _antivirusScanner = antivirusScanner; _logger = logger; } @@ -192,6 +195,15 @@ public virtual async Task CreateFileFromStreamAsync(string path, Stream } } + var scannedStream = await ScanAsync(context.Path, outputStream); + + if (scannedStream != outputStream && outputStream != inputStream) + { + outputStream.Dispose(); + } + + outputStream = scannedStream; + await ValidateAvailableStorageAsync(outputStream.Length); return await _fileStore.CreateFileFromStreamAsync(context.Path, outputStream, overwrite); @@ -204,9 +216,21 @@ public virtual async Task CreateFileFromStreamAsync(string path, Stream } else { - await ValidateAvailableStorageAsync(inputStream.Length); + var scannedStream = await ScanAsync(path, inputStream); - return await _fileStore.CreateFileFromStreamAsync(path, inputStream, overwrite); + try + { + await ValidateAvailableStorageAsync(scannedStream.Length); + + return await _fileStore.CreateFileFromStreamAsync(path, scannedStream, overwrite); + } + finally + { + if (scannedStream != inputStream) + { + scannedStream.Dispose(); + } + } } } @@ -262,4 +286,65 @@ private async Task ValidateAvailableStorageAsync(long requiredStorageSpace) $"a file that fits the available space, or delete some unnecessary files."); } } + + private async Task ScanAsync(string path, Stream stream) + { + if (_antivirusScanner is NullAntivirusScanner) + { + return stream; + } + + var scanStream = stream; + + if (scanStream.CanSeek) + { + scanStream.Position = 0; + } + else + { + scanStream = await CreateTemporarySeekableStreamAsync(scanStream); + } + + try + { + var result = await _antivirusScanner.ScanAsync( + new AntivirusScanContext(path, scanStream.Length), + scanStream); + + if (!result.IsClean) + { + throw new AntivirusScanningException(result.Message ?? $"The uploaded file '{Path.GetFileName(path)}' was rejected by the anti-virus scanner."); + } + + scanStream.Position = 0; + + return scanStream; + } + catch + { + if (scanStream != stream) + { + scanStream.Dispose(); + } + + throw; + } + } + + private static async Task CreateTemporarySeekableStreamAsync(Stream stream) + { + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + var tempStream = new FileStream( + tempFilePath, + FileMode.CreateNew, + FileAccess.ReadWrite, + FileShare.None, + 81920, + FileOptions.Asynchronous | FileOptions.DeleteOnClose); + + await stream.CopyToAsync(tempStream); + tempStream.Position = 0; + + return tempStream; + } } diff --git a/src/docs/reference/modules/Antivirus.ClamAV/README.md b/src/docs/reference/modules/Antivirus.ClamAV/README.md new file mode 100644 index 00000000000..fa99c423837 --- /dev/null +++ b/src/docs/reference/modules/Antivirus.ClamAV/README.md @@ -0,0 +1,50 @@ +# ClamAV Antivirus Scanner (`OrchardCore.Antivirus.ClamAV`) + +The ClamAV Antivirus Scanner module scans files with a `clamd` service before Orchard Core stores or imports them. + +When the feature is enabled, uploads fail closed: + +- Malware detections are rejected before storage. +- Scanner connectivity, timeout, or protocol failures also reject the upload. + +The scanner is wired through the reusable `IAntivirusScanner` abstraction, so custom modules can replace it with another implementation when they need a different anti-virus provider. + +## Configuration + +Configure the ClamAV connection in application configuration: + +```json +{ + "OrchardCore": { + "OrchardCore_Antivirus_ClamAV": { + "Host": "localhost", + "Port": 3310, + "ConnectTimeoutSeconds": 5, + "TransferTimeoutSeconds": 30 + } + } +} +``` + +The same settings can be provided with environment variables: + +```text +OrchardCore__Antivirus_ClamAV__Host=localhost +OrchardCore__Antivirus_ClamAV__Port=3310 +OrchardCore__Antivirus_ClamAV__ConnectTimeoutSeconds=5 +OrchardCore__Antivirus_ClamAV__TransferTimeoutSeconds=30 +``` + +## Usage + +1. Configure the ClamAV settings. +2. Enable the `ClamAV Antivirus Scanner` feature. +3. Ensure a reachable `clamd` instance is running. + +If the feature is enabled without a valid ClamAV connection, uploads are rejected until the scanner can verify them. + +## Notes + +- The currently audited upload surfaces covered by this change are media uploads plus deployment package imports, both local and remote. +- Media uploads are scanned before storage because they flow through `DefaultMediaFileStore`. +- Deployment package zip/json uploads are scanned before Orchard Core writes the uploaded file to a temporary archive location for import. diff --git a/src/docs/releases/3.0.0.md b/src/docs/releases/3.0.0.md index b63732b9650..ed808f42282 100644 --- a/src/docs/releases/3.0.0.md +++ b/src/docs/releases/3.0.0.md @@ -283,9 +283,7 @@ This is a source and binary breaking change for custom SMS services and provider The `INotificationService.SendAsync()` method now returns a `NotificationSendResult`, which provides the aggregate notification delivery outcome, including successful and failed sends across notification methods. -The `INotificationMethodProvider.TrySendAsync()` method was renamed to `SendAsync()`. - -The `INotificationService.SendAsync()` and `INotificationMethodProvider.SendAsync()` methods now also accept a `CancellationToken`. +The `INotificationService.SendAsync()` and `INotificationMethodProvider.TrySendAsync()` methods now also accept a `CancellationToken`. The notification event handler callbacks on `INotificationEvents`, including overrides based on `NotificationEventsHandler`, also now accept a `CancellationToken`. diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs index c497dca55f1..4aa61d6cc62 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs @@ -35,6 +35,7 @@ public async Task ShouldProcess(string cdnBaseUrl, string text, string expected) cdnBaseUrl, [], [], + new NullAntivirusScanner(), Mock.Of>()); var defaultHttpContext = new DefaultHttpContext(); diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVAntivirusScannerTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVAntivirusScannerTests.cs new file mode 100644 index 00000000000..989dd783a47 --- /dev/null +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVAntivirusScannerTests.cs @@ -0,0 +1,198 @@ +using System.Buffers.Binary; +using System.Net.Sockets; +using OrchardCore.Antivirus.ClamAV; +using OrchardCore.Antivirus.ClamAV.Services; +using OrchardCore.FileStorage; + +namespace OrchardCore.Tests.Modules.OrchardCore.Media; + +public class ClamAVAntivirusScannerTests +{ + [Fact] + public async Task ScanAsync_ReturnsCleanResult_WhenClamAvReturnsOk() + { + await using var server = await FakeClamAvServer.StartAsync("stream: OK\n"); + var scanner = CreateScanner(server.Port); + + var result = await scanner.ScanAsync( + new AntivirusScanContext("folder/test.txt"), + new MemoryStream("hello world"u8.ToArray())); + + var request = await server.Completion; + + Assert.True(result.IsClean); + Assert.Equal("nINSTREAM\n", request.Command); + Assert.Equal("hello world"u8.ToArray(), request.Payload); + } + + [Fact] + public async Task ScanAsync_ReturnsUnsafeResult_WhenClamAvFindsMalware() + { + await using var server = await FakeClamAvServer.StartAsync("stream: Eicar-Test-Signature FOUND\n"); + var scanner = CreateScanner(server.Port); + + var result = await scanner.ScanAsync( + new AntivirusScanContext("folder/test.txt"), + new MemoryStream("hello world"u8.ToArray())); + + Assert.False(result.IsClean); + Assert.Equal("Eicar-Test-Signature", result.ThreatName); + Assert.Equal("The uploaded file 'test.txt' was rejected because ClamAV detected 'Eicar-Test-Signature'.", result.Message); + } + + [Fact] + public async Task ScanAsync_Throws_WhenClamAvReturnsError() + { + await using var server = await FakeClamAvServer.StartAsync("INSTREAM size limit exceeded. ERROR\n"); + var scanner = CreateScanner(server.Port); + + var exception = await Assert.ThrowsAsync(() => + scanner.ScanAsync( + new AntivirusScanContext("folder/test.txt"), + new MemoryStream("hello world"u8.ToArray()))); + + Assert.Equal("The ClamAV antivirus scanner returned an unexpected response while scanning 'test.txt': INSTREAM size limit exceeded. ERROR", exception.Message); + } + + [Fact] + public async Task ScanAsync_Throws_WhenHostIsMissing() + { + var scanner = new ClamAVAntivirusScanner( + Options.Create(new ClamAvOptions + { + Host = "", + }), + Mock.Of>()); + + var exception = await Assert.ThrowsAsync(() => + scanner.ScanAsync( + new AntivirusScanContext("folder/test.txt"), + new MemoryStream("hello world"u8.ToArray()))); + + Assert.Equal("The ClamAV antivirus scanner is enabled but the host setting is missing.", exception.Message); + } + + private static ClamAVAntivirusScanner CreateScanner(int port) => + new( + Options.Create(new ClamAvOptions + { + Host = IPAddress.Loopback.ToString(), + Port = port, + ConnectTimeoutSeconds = 5, + TransferTimeoutSeconds = 5, + }), + Mock.Of>()); + + private sealed class FakeClamAvServer : IAsyncDisposable + { + private readonly TcpListener _listener; + + private FakeClamAvServer(TcpListener listener, Task completion) + { + _listener = listener; + Completion = completion; + } + + public int Port => ((IPEndPoint)_listener.LocalEndpoint).Port; + + public Task Completion { get; } + + public static Task StartAsync(string response) + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + + var completion = Task.Run(async () => + { + using var client = await listener.AcceptTcpClientAsync(); + await using var networkStream = client.GetStream(); + + var command = await ReadLineAsync(networkStream); + var payload = await ReadPayloadAsync(networkStream); + var responseBytes = Encoding.ASCII.GetBytes(response); + + await networkStream.WriteAsync(responseBytes); + await networkStream.FlushAsync(); + + return new ClamAvRequest(command, payload); + }); + + return Task.FromResult(new FakeClamAvServer(listener, completion)); + } + + public ValueTask DisposeAsync() + { + _listener.Stop(); + + return ValueTask.CompletedTask; + } + + private static async Task ReadLineAsync(NetworkStream stream) + { + using var commandStream = new MemoryStream(); + var buffer = new byte[1]; + + while (true) + { + var bytesRead = await stream.ReadAsync(buffer.AsMemory(0, 1)); + + if (bytesRead == 0) + { + break; + } + + commandStream.WriteByte(buffer[0]); + + if (buffer[0] == '\n') + { + break; + } + } + + return Encoding.ASCII.GetString(commandStream.ToArray()); + } + + private static async Task ReadPayloadAsync(Stream stream) + { + using var payloadStream = new MemoryStream(); + var lengthBuffer = new byte[sizeof(int)]; + + while (true) + { + await ReadExactlyAsync(stream, lengthBuffer); + + var chunkLength = BinaryPrimitives.ReadInt32BigEndian(lengthBuffer); + + if (chunkLength == 0) + { + break; + } + + var chunk = new byte[chunkLength]; + await ReadExactlyAsync(stream, chunk); + await payloadStream.WriteAsync(chunk); + } + + return payloadStream.ToArray(); + } + + private static async Task ReadExactlyAsync(Stream stream, byte[] buffer) + { + var offset = 0; + + while (offset < buffer.Length) + { + var bytesRead = await stream.ReadAsync(buffer.AsMemory(offset, buffer.Length - offset)); + + if (bytesRead == 0) + { + throw new EndOfStreamException(); + } + + offset += bytesRead; + } + } + } + + private sealed record ClamAvRequest(string Command, byte[] Payload); +} diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs index f1315b6e6e5..fadfe7edbc1 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs @@ -21,6 +21,7 @@ public void MapPathToPublicUrl_ReturnsUrlEncodedPath(string path, string expecte "", [], [], + new NullAntivirusScanner(), Mock.Of>()); var result = store.MapPathToPublicUrl(path); @@ -39,6 +40,7 @@ public void MapPathToPublicUrl_WithCdnBaseUrl_ReturnsUrlEncodedPath(string cdnBa cdnBaseUrl, [], [], + new NullAntivirusScanner(), Mock.Of>()); var result = store.MapPathToPublicUrl(path); @@ -63,6 +65,7 @@ public async Task CreateFileFromStreamAsync_NoHandlers_CallsFileStoreDirectly() "", new List(), new List(), + new NullAntivirusScanner(), loggerMock.Object); // Act @@ -95,6 +98,7 @@ public async Task CreateFileFromStreamAsync_HandlerReturnsInputStream_PassesInpu "", new List(), new[] { handlerMock.Object }, + new NullAntivirusScanner(), loggerMock.Object); // Act @@ -135,6 +139,7 @@ public async Task CreateFileFromStreamAsync_HandlersCreateNewStreams_PassesCorre "", new List(), new[] { handler1.Object, handler2.Object }, + new NullAntivirusScanner(), loggerMock.Object); // Act @@ -175,6 +180,7 @@ public async Task CreateFileFromStreamAsync_ValidateAvailableStorageAsync() "", [handler.Object], [], + new NullAntivirusScanner(), loggerMock.Object); // Act @@ -188,4 +194,165 @@ public async Task CreateFileFromStreamAsync_ValidateAvailableStorageAsync() "uploading a file that fits the available space, or delete some unnecessary files."; Assert.Equal(expectedMessage, exception.Message); } + + [Fact] + public async Task CreateFileFromStreamAsync_ScannerResetsStreamBeforeSaving() + { + var fileStoreMock = new Mock(); + var loggerMock = new Mock>(); + var scannerMock = new Mock(); + var inputStream = new MemoryStream("hello world"u8.ToArray()); + + scannerMock + .Setup(x => x.ScanAsync(It.IsAny(), inputStream)) + .ReturnsAsync(AntivirusResult.Clean) + .Callback((_, stream) => + { + while (stream.ReadByte() != -1) + { + } + }); + + fileStoreMock + .Setup(x => x.CreateFileFromStreamAsync("test.txt", It.IsAny(), false)) + .ReturnsAsync("result") + .Callback((_, stream, _) => + { + using var copy = new MemoryStream(); + stream.CopyTo(copy); + Assert.Equal("hello world"u8.ToArray(), copy.ToArray()); + }); + + var store = new DefaultMediaFileStore( + fileStoreMock.Object, + "", + "", + [], + [], + scannerMock.Object, + loggerMock.Object); + + await store.CreateFileFromStreamAsync("test.txt", inputStream); + } + + [Fact] + public async Task CreateFileFromStreamAsync_ScannerRejectsUnsafeFile() + { + var fileStoreMock = new Mock(MockBehavior.Strict); + var loggerMock = new Mock>(); + var scannerMock = new Mock(); + var inputStream = new MemoryStream("hello world"u8.ToArray()); + + scannerMock + .Setup(x => x.ScanAsync(It.IsAny(), inputStream)) + .ReturnsAsync(AntivirusResult.Unsafe("The uploaded file was rejected by the anti-virus scanner.", "EICAR-Test-File")); + + var store = new DefaultMediaFileStore( + fileStoreMock.Object, + "", + "", + [], + [], + scannerMock.Object, + loggerMock.Object); + + var exception = await Assert.ThrowsAsync(() => + store.CreateFileFromStreamAsync("test.txt", inputStream)); + + Assert.Equal("The uploaded file was rejected by the anti-virus scanner.", exception.Message); + } + + [Fact] + public async Task CreateFileFromStreamAsync_ScannerBuffersNonSeekableStreams() + { + var fileStoreMock = new Mock(); + var loggerMock = new Mock>(); + var scannerMock = new Mock(); + using var baseStream = new MemoryStream("hello world"u8.ToArray()); + using var inputStream = new NonSeekableReadStream(baseStream); + + scannerMock + .Setup(x => x.ScanAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(AntivirusResult.Clean) + .Callback((_, stream) => + { + Assert.True(stream.CanSeek); + while (stream.ReadByte() != -1) + { + } + }); + + fileStoreMock + .Setup(x => x.CreateFileFromStreamAsync("test.txt", It.IsAny(), false)) + .ReturnsAsync("result") + .Callback((_, stream, _) => + { + Assert.True(stream.CanSeek); + + using var copy = new MemoryStream(); + stream.CopyTo(copy); + Assert.Equal("hello world"u8.ToArray(), copy.ToArray()); + }); + + var store = new DefaultMediaFileStore( + fileStoreMock.Object, + "", + "", + [], + [], + scannerMock.Object, + loggerMock.Object); + + await store.CreateFileFromStreamAsync("test.txt", inputStream); + } + + private sealed class NonSeekableReadStream : Stream + { + private readonly Stream _innerStream; + + public NonSeekableReadStream(Stream innerStream) + { + _innerStream = innerStream; + } + + public override bool CanRead => _innerStream.CanRead; + + public override bool CanSeek => false; + + public override bool CanWrite => false; + + public override long Length => throw new NotSupportedException(); + + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public override void Flush() => _innerStream.Flush(); + + public override int Read(byte[] buffer, int offset, int count) => _innerStream.Read(buffer, offset, count); + + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + public override void SetLength(long value) => throw new NotSupportedException(); + + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + + public override async ValueTask DisposeAsync() + { + await _innerStream.DisposeAsync(); + await base.DisposeAsync(); + } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + _innerStream.Dispose(); + } + + base.Dispose(disposing); + } + } } diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs index 46817307e8e..e22ec10b9f1 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs @@ -46,6 +46,7 @@ public async Task ShouldProcess(string cdnBaseUrl, string text, string expected) cdnBaseUrl, [], [], + new NullAntivirusScanner(), Mock.Of>()); var fileVersionProvider = Mock.Of(); diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs index 030eff3781a..f4be3797a8c 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs @@ -43,6 +43,7 @@ private static TestOrchardHelper CreateOrchardHelper() "", [], [], + new NullAntivirusScanner(), Mock.Of>()); var mediaProfileServiceMock = new Mock(); diff --git a/test/OrchardCore.Tests/OrchardCore.Tests.csproj b/test/OrchardCore.Tests/OrchardCore.Tests.csproj index 125dac70836..0b898c0598e 100644 --- a/test/OrchardCore.Tests/OrchardCore.Tests.csproj +++ b/test/OrchardCore.Tests/OrchardCore.Tests.csproj @@ -52,9 +52,11 @@ + + From 7798de64dcc0870a9693021367082d4ef7d19cfd Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 3 Jun 2026 14:43:32 -0700 Subject: [PATCH 2/8] fix aspire --- src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj | 3 ++- src/OrchardCore.AspireHost/Properties/launchSettings.json | 4 ++-- src/OrchardCore.Cms.Web/OrchardCore.Cms.Web.csproj | 2 +- .../OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj b/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj index 19730ee4406..b6eccde5154 100644 --- a/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj +++ b/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj @@ -1,7 +1,8 @@  - net10.0 + net10.0 + $(CommonTargetFrameworks) Exe true false diff --git a/src/OrchardCore.AspireHost/Properties/launchSettings.json b/src/OrchardCore.AspireHost/Properties/launchSettings.json index 81cd4e5f970..bf2f5b9ee03 100644 --- a/src/OrchardCore.AspireHost/Properties/launchSettings.json +++ b/src/OrchardCore.AspireHost/Properties/launchSettings.json @@ -4,7 +4,7 @@ "https": { "commandName": "Project", "dotnetRunMessages": true, - "launchBrowser": true, + "launchBrowser": false, "applicationUrl": "https://localhost:17262;http://localhost:15209", "environmentVariables": { "ASPNETCORE_ENVIRONMENT": "Development", @@ -16,7 +16,7 @@ "http": { "commandName": "Project", "dotnetRunMessages": true, - "launchBrowser": true, + "launchBrowser": false, "applicationUrl": "http://localhost:15209", "environmentVariables": { "ASPNETCORE_ENVIRONMENT": "Development", diff --git a/src/OrchardCore.Cms.Web/OrchardCore.Cms.Web.csproj b/src/OrchardCore.Cms.Web/OrchardCore.Cms.Web.csproj index fc449c0160b..9a6cd034486 100644 --- a/src/OrchardCore.Cms.Web/OrchardCore.Cms.Web.csproj +++ b/src/OrchardCore.Cms.Web/OrchardCore.Cms.Web.csproj @@ -1,4 +1,4 @@ - + diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs index 49c8351597f..fdd3e9d7686 100644 --- a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs @@ -2,7 +2,7 @@ namespace OrchardCore.Antivirus.ClamAV; public sealed class ClamAvOptions { - public const string ConfigSection = "OrchardCore_Antivirus_ClamAV"; + public const string ConfigSection = "Antivirus_ClamAV"; public string Host { get; set; } From 2490233ad065c89a9fa6ec6ac604b241d5b4e189 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 3 Jun 2026 15:12:33 -0700 Subject: [PATCH 3/8] fix build --- src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj b/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj index b6eccde5154..7c8993859c1 100644 --- a/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj +++ b/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj @@ -1,8 +1,7 @@  - net10.0 - $(CommonTargetFrameworks) + $(DefaultTargetFramework) Exe true false From d8e0271ccda60da400b96a6ddb422771f769a896 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 3 Jun 2026 15:16:09 -0700 Subject: [PATCH 4/8] undo changes --- src/docs/releases/3.0.0.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/docs/releases/3.0.0.md b/src/docs/releases/3.0.0.md index ed808f42282..b63732b9650 100644 --- a/src/docs/releases/3.0.0.md +++ b/src/docs/releases/3.0.0.md @@ -283,7 +283,9 @@ This is a source and binary breaking change for custom SMS services and provider The `INotificationService.SendAsync()` method now returns a `NotificationSendResult`, which provides the aggregate notification delivery outcome, including successful and failed sends across notification methods. -The `INotificationService.SendAsync()` and `INotificationMethodProvider.TrySendAsync()` methods now also accept a `CancellationToken`. +The `INotificationMethodProvider.TrySendAsync()` method was renamed to `SendAsync()`. + +The `INotificationService.SendAsync()` and `INotificationMethodProvider.SendAsync()` methods now also accept a `CancellationToken`. The notification event handler callbacks on `INotificationEvents`, including overrides based on `NotificationEventsHandler`, also now accept a `CancellationToken`. From 6c5c0f4c7ba7e19eb7e751ec19b554effd5f1617 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 3 Jun 2026 15:25:44 -0700 Subject: [PATCH 5/8] fix Visual Studio Errors --- src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj b/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj index 7c8993859c1..0e2144694d9 100644 --- a/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj +++ b/src/OrchardCore.AspireHost/OrchardCore.AspireHost.csproj @@ -13,7 +13,10 @@ - + From 2ae174f371689d38e9a5195a10b1ce8a6c8dceab Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 4 Jun 2026 22:38:02 -0700 Subject: [PATCH 6/8] Migrate to event file handlers --- OrchardCore.slnx | 2 +- .../ClamAV}/ClamAVOptions.cs | 0 .../ClamAV/ClamAvConnection.cs | 132 ++++++++ .../ClamAV/ClamAvConnectionFactory.cs | 64 ++++ .../ClamAV/ClamAvFileEventHandler.cs} | 152 +++++---- .../Manifest.cs | 2 +- .../OrchardCore.Antivirus.csproj} | 0 .../Startup.cs | 18 +- .../ImportRemoteInstanceController.cs | 83 +---- .../OrchardCore.Deployment.Remote/Startup.cs | 2 +- .../Controllers/ImportController.cs | 85 +---- .../OrchardCore.Deployment/Startup.cs | 2 +- .../OrchardCore.Media.AmazonS3/Startup.cs | 4 +- .../OrchardCore.Media.Azure/Startup.cs | 4 +- .../OrchardCore.Media/Startup.cs | 6 +- ...rdCore.Application.Cms.Core.Targets.csproj | 2 +- .../AntivirusResult.cs | 21 -- .../AntivirusScanContext.cs | 19 -- .../FileCreatingContext.cs | 19 ++ .../FileCreatingResult.cs | 55 ++++ .../FileEventService.cs | 85 +++++ .../IAntivirusScanner.cs | 6 - .../IFileEventHandler.cs | 8 + .../NullAntivirusScanner.cs | 7 - ...rchardCore.FileStorage.Abstractions.csproj | 4 + .../Result.cs | 27 +- .../ResultOfT.cs | 14 +- .../DefaultMediaFileStore.cs | 95 +----- src/docs/reference/README.md | 1 + .../reference/core/file-upload-security.md | 113 +++++++ .../{Antivirus.ClamAV => ClamAV}/README.md | 15 +- src/docs/releases/3.0.0.md | 13 + .../AssetUrlShortcodeTests.cs | 2 +- .../ClamAVAntivirusScannerTests.cs | 198 ------------ .../ClamAVFileEventHandlerTests.cs | 301 ++++++++++++++++++ .../DefaultMediaFileStoreTests.cs | 157 ++++----- .../FileEventServiceTests.cs | 81 +++++ .../OrchardCore.Media/ImageShortcodeTests.cs | 2 +- .../MediaOrchardHelperExtensionsTests.cs | 2 +- .../OrchardCore.Tests.csproj | 2 +- 40 files changed, 1128 insertions(+), 677 deletions(-) rename src/OrchardCore.Modules/{OrchardCore.Antivirus.ClamAV => OrchardCore.Antivirus/ClamAV}/ClamAVOptions.cs (100%) create mode 100644 src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvConnection.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvConnectionFactory.cs rename src/OrchardCore.Modules/{OrchardCore.Antivirus.ClamAV/Services/ClamAVAntivirusScanner.cs => OrchardCore.Antivirus/ClamAV/ClamAvFileEventHandler.cs} (50%) rename src/OrchardCore.Modules/{OrchardCore.Antivirus.ClamAV => OrchardCore.Antivirus}/Manifest.cs (94%) rename src/OrchardCore.Modules/{OrchardCore.Antivirus.ClamAV/OrchardCore.Antivirus.ClamAV.csproj => OrchardCore.Antivirus/OrchardCore.Antivirus.csproj} (100%) rename src/OrchardCore.Modules/{OrchardCore.Antivirus.ClamAV => OrchardCore.Antivirus}/Startup.cs (53%) delete mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusResult.cs delete mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanContext.cs create mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreatingContext.cs create mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreatingResult.cs create mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileEventService.cs delete mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/IAntivirusScanner.cs create mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/IFileEventHandler.cs delete mode 100644 src/OrchardCore/OrchardCore.FileStorage.Abstractions/NullAntivirusScanner.cs create mode 100644 src/docs/reference/core/file-upload-security.md rename src/docs/reference/modules/{Antivirus.ClamAV => ClamAV}/README.md (51%) delete mode 100644 test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVAntivirusScannerTests.cs create mode 100644 test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVFileEventHandlerTests.cs create mode 100644 test/OrchardCore.Tests/Modules/OrchardCore.Media/FileEventServiceTests.cs diff --git a/OrchardCore.slnx b/OrchardCore.slnx index 7001db16050..de1a7ea611c 100644 --- a/OrchardCore.slnx +++ b/OrchardCore.slnx @@ -117,7 +117,7 @@ - + diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAVOptions.cs similarity index 100% rename from src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/ClamAVOptions.cs rename to src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAVOptions.cs diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvConnection.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvConnection.cs new file mode 100644 index 00000000000..0c5e24926a6 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvConnection.cs @@ -0,0 +1,132 @@ +using System.Net.Sockets; +using System.Text; +using Microsoft.Extensions.Logging; + +namespace OrchardCore.Antivirus.ClamAV; + +public sealed class ClamAvConnection : IDisposable +{ + private const int BufferSize = 81920; + private static readonly byte[] _scanCommand = "nINSTREAM\n"u8.ToArray(); + + private readonly ClamAvOptions _options; + private readonly ILogger _logger; + private readonly SemaphoreSlim _lock = new(1, 1); + + private TcpClient _client; + private NetworkStream _networkStream; + + public ClamAvConnection(ClamAvOptions options, ILogger logger) + { + _options = options; + _logger = logger; + } + + public async Task ScanAsync(Stream stream, CancellationToken cancellationToken) + { + await _lock.WaitAsync(cancellationToken); + + try + { + try + { + await EnsureConnectedAsync(cancellationToken); + + using var transferCancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + transferCancellationTokenSource.CancelAfter(TimeSpan.FromSeconds(_options.TransferTimeoutSeconds)); + + await _networkStream.WriteAsync(_scanCommand, transferCancellationTokenSource.Token); + + var buffer = new byte[BufferSize]; + int bytesRead; + + while ((bytesRead = await stream.ReadAsync(buffer.AsMemory(0, buffer.Length), transferCancellationTokenSource.Token)) > 0) + { + var prefix = BitConverter.GetBytes(System.Net.IPAddress.HostToNetworkOrder(bytesRead)); + + await _networkStream.WriteAsync(prefix, transferCancellationTokenSource.Token); + await _networkStream.WriteAsync(buffer.AsMemory(0, bytesRead), transferCancellationTokenSource.Token); + } + + await _networkStream.WriteAsync(new byte[sizeof(int)], transferCancellationTokenSource.Token); + await _networkStream.FlushAsync(transferCancellationTokenSource.Token); + + return await ReadResponseAsync(_networkStream, transferCancellationTokenSource.Token); + } + catch (Exception exception) when ( + exception is IOException or + SocketException or + OperationCanceledException or + ObjectDisposedException) + { + ResetConnection(); + throw; + } + } + finally + { + _lock.Release(); + } + } + + public void Dispose() + { + ResetConnection(); + _lock.Dispose(); + } + + private async Task EnsureConnectedAsync(CancellationToken cancellationToken) + { + if (_client?.Connected == true && _networkStream is not null) + { + return; + } + + ResetConnection(); + + var client = new TcpClient(); + using var connectCancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + connectCancellationTokenSource.CancelAfter(TimeSpan.FromSeconds(_options.ConnectTimeoutSeconds)); + + await client.ConnectAsync(_options.Host, _options.Port, connectCancellationTokenSource.Token); + + client.SendTimeout = (int)TimeSpan.FromSeconds(_options.TransferTimeoutSeconds).TotalMilliseconds; + client.ReceiveTimeout = (int)TimeSpan.FromSeconds(_options.TransferTimeoutSeconds).TotalMilliseconds; + + _client = client; + _networkStream = client.GetStream(); + + if (_logger.IsEnabled(LogLevel.Debug)) + { + _logger.LogDebug("Created a shared ClamAV TCP connection for '{Host}:{Port}'.", _options.Host, _options.Port); + } + } + + private void ResetConnection() + { + _networkStream?.Dispose(); + _networkStream = null; + _client?.Dispose(); + _client = null; + } + + private static async Task ReadResponseAsync(NetworkStream stream, CancellationToken cancellationToken) + { + using var responseStream = new MemoryStream(); + var buffer = new byte[1]; + + while (true) + { + var bytesRead = await stream.ReadAsync(buffer.AsMemory(0, 1), cancellationToken); + + if (bytesRead == 0 || buffer[0] == '\n') + { + break; + } + + responseStream.WriteByte(buffer[0]); + } + + return Encoding.ASCII.GetString(responseStream.ToArray()); + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvConnectionFactory.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvConnectionFactory.cs new file mode 100644 index 00000000000..3505b687318 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvConnectionFactory.cs @@ -0,0 +1,64 @@ +using System.Collections.Concurrent; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +namespace OrchardCore.Antivirus.ClamAV; + +public sealed class ClamAvConnectionFactory : IDisposable +{ + private static readonly ConcurrentDictionary> _connections = new(); + private static volatile int _registered; + private static volatile int _refCount; + + private readonly IHostApplicationLifetime _lifetime; + private readonly ILoggerFactory _loggerFactory; + + public ClamAvConnectionFactory( + IHostApplicationLifetime lifetime, + ILoggerFactory loggerFactory) + { + Interlocked.Increment(ref _refCount); + + _lifetime = lifetime; + _loggerFactory = loggerFactory; + + if (Interlocked.CompareExchange(ref _registered, 1, 0) == 0) + { + _lifetime.ApplicationStopped.Register(Release); + } + } + + public ClamAvConnection Create(ClamAvOptions options) + { + var key = $"{options.Host}:{options.Port}:{options.ConnectTimeoutSeconds}:{options.TransferTimeoutSeconds}"; + + return _connections.GetOrAdd(key, _ => new Lazy(() => + new ClamAvConnection(options, _loggerFactory.CreateLogger()))).Value; + } + + public void Dispose() + { + if (Interlocked.Decrement(ref _refCount) == 0 && _lifetime.ApplicationStopped.IsCancellationRequested) + { + Release(); + } + } + + internal static void Release() + { + if (Interlocked.CompareExchange(ref _refCount, 0, 0) == 0) + { + var connections = _connections.Values.ToArray(); + + _connections.Clear(); + + foreach (var connection in connections) + { + if (connection.IsValueCreated) + { + connection.Value.Dispose(); + } + } + } + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Services/ClamAVAntivirusScanner.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvFileEventHandler.cs similarity index 50% rename from src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Services/ClamAVAntivirusScanner.cs rename to src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvFileEventHandler.cs index 45d34429ecb..a59bf11504b 100644 --- a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Services/ClamAVAntivirusScanner.cs +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus/ClamAV/ClamAvFileEventHandler.cs @@ -1,91 +1,133 @@ -using System.Buffers.Binary; using System.Net.Sockets; -using System.Text; +using Microsoft.Extensions.Localization; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using OrchardCore.FileStorage; +using OrchardCore.Infrastructure; -namespace OrchardCore.Antivirus.ClamAV.Services; +namespace OrchardCore.Antivirus.ClamAV; -public sealed class ClamAVAntivirusScanner : IAntivirusScanner +public sealed class ClamAvFileEventHandler : IFileEventHandler { - private const int BufferSize = 81920; - private static readonly byte[] _scanCommand = "nINSTREAM\n"u8.ToArray(); private readonly ClamAvOptions _options; + private readonly ClamAvConnectionFactory _connectionFactory; private readonly ILogger _logger; - public ClamAVAntivirusScanner( + public ClamAvFileEventHandler( IOptions options, - ILogger logger) + ClamAvConnectionFactory connectionFactory, + ILogger logger) { _options = options.Value; + _connectionFactory = connectionFactory; _logger = logger; } - public async Task ScanAsync(AntivirusScanContext context, Stream stream) + public async Task CreatingAsync(FileCreatingContext context, Stream stream, CancellationToken cancellationToken = default) { ValidateOptions(); - try - { - using var client = new TcpClient(); - using var connectCancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(_options.ConnectTimeoutSeconds)); - - await client.ConnectAsync(_options.Host, _options.Port, connectCancellationTokenSource.Token); - - client.SendTimeout = (int)TimeSpan.FromSeconds(_options.TransferTimeoutSeconds).TotalMilliseconds; - client.ReceiveTimeout = (int)TimeSpan.FromSeconds(_options.TransferTimeoutSeconds).TotalMilliseconds; + var scanStream = stream; - await using var networkStream = client.GetStream(); - using var transferCancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(_options.TransferTimeoutSeconds)); - - await networkStream.WriteAsync(_scanCommand, transferCancellationTokenSource.Token); - - var buffer = new byte[BufferSize]; + if (scanStream.CanSeek) + { + scanStream.Position = 0; + } + else + { + scanStream = await CreateSeekableStreamAsync(scanStream, cancellationToken); + } - int bytesRead; + try + { + var response = await _connectionFactory.Create(_options).ScanAsync(scanStream, cancellationToken); - while ((bytesRead = await stream.ReadAsync(buffer.AsMemory(0, buffer.Length), transferCancellationTokenSource.Token)) > 0) + if (TryCreateFailureResult(context, scanStream, response) is { } failureResult) { - var prefix = new byte[sizeof(int)]; - BinaryPrimitives.WriteInt32BigEndian(prefix, bytesRead); - - await networkStream.WriteAsync(prefix, transferCancellationTokenSource.Token); - await networkStream.WriteAsync(buffer.AsMemory(0, bytesRead), transferCancellationTokenSource.Token); + return failureResult; } - await networkStream.WriteAsync(new byte[sizeof(int)], transferCancellationTokenSource.Token); - await networkStream.FlushAsync(transferCancellationTokenSource.Token); - - var response = await ReadResponseAsync(networkStream, transferCancellationTokenSource.Token); + scanStream.Position = 0; - return CreateResult(context, response); + return FileCreatingResult.Success(scanStream); } catch (OperationCanceledException exception) { + if (scanStream != stream) + { + await scanStream.DisposeAsync(); + } + _logger.LogError(exception, "ClamAV timed out while scanning '{FileName}'.", context.FileName); throw new AntivirusScanningException($"The ClamAV antivirus scanner timed out while scanning '{context.FileName}'.", exception); } catch (SocketException exception) { + if (scanStream != stream) + { + await scanStream.DisposeAsync(); + } + _logger.LogError(exception, "ClamAV could not be reached while scanning '{FileName}'.", context.FileName); throw new AntivirusScanningException($"The ClamAV antivirus scanner could not be reached while scanning '{context.FileName}'.", exception); } catch (IOException exception) { + if (scanStream != stream) + { + await scanStream.DisposeAsync(); + } + _logger.LogError(exception, "ClamAV failed while scanning '{FileName}'.", context.FileName); throw new AntivirusScanningException($"The ClamAV antivirus scanner failed while scanning '{context.FileName}'.", exception); } + catch + { + if (scanStream != stream) + { + await scanStream.DisposeAsync(); + } + + throw; + } } - private static AntivirusResult CreateResult(AntivirusScanContext context, string response) + public Task CreatedAsync(IFileStoreEntry fileInfo, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + private static async Task CreateSeekableStreamAsync(Stream stream, CancellationToken cancellationToken) + { + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + var tempStream = new FileStream( + tempFilePath, + FileMode.CreateNew, + FileAccess.ReadWrite, + FileShare.None, + 81920, + FileOptions.Asynchronous | FileOptions.DeleteOnClose); + + try + { + await stream.CopyToAsync(tempStream, cancellationToken); + tempStream.Position = 0; + + return tempStream; + } + catch + { + await tempStream.DisposeAsync(); + throw; + } + } + + private static FileCreatingResult TryCreateFailureResult(FileCreatingContext context, Stream stream, string response) { if (string.Equals(response, "stream: OK", StringComparison.Ordinal)) { - return AntivirusResult.Clean; + return null; } if (response.EndsWith(" FOUND", StringComparison.Ordinal)) @@ -100,38 +142,16 @@ private static AntivirusResult CreateResult(AntivirusScanContext context, string signature = signature[..^" FOUND".Length]; - return AntivirusResult.Unsafe( - $"The uploaded file '{context.FileName}' was rejected because ClamAV detected '{signature}'.", - signature); - } + stream.Position = 0; - throw new AntivirusScanningException( - $"The ClamAV antivirus scanner returned an unexpected response while scanning '{context.FileName}': {response}"); - } - - private static async Task ReadResponseAsync(NetworkStream stream, CancellationToken cancellationToken) - { - using var responseStream = new MemoryStream(); - var buffer = new byte[1]; - - while (true) - { - var bytesRead = await stream.ReadAsync(buffer.AsMemory(0, 1), cancellationToken); - - if (bytesRead == 0) + return FileCreatingResult.Failed(stream, new ResultError { - break; - } - - if (buffer[0] == '\n') - { - break; - } - - responseStream.WriteByte(buffer[0]); + Message = new LocalizedString(nameof(ClamAvFileEventHandler), $"The uploaded file '{context.FileName}' was rejected because ClamAV detected '{signature}'."), + }); } - return Encoding.ASCII.GetString(responseStream.ToArray()); + throw new AntivirusScanningException( + $"The ClamAV antivirus scanner returned an unexpected response while scanning '{context.FileName}': {response}"); } private void ValidateOptions() diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Manifest.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus/Manifest.cs similarity index 94% rename from src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Manifest.cs rename to src/OrchardCore.Modules/OrchardCore.Antivirus/Manifest.cs index e4ff3a66f4e..76b0a84570f 100644 --- a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Manifest.cs +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus/Manifest.cs @@ -10,6 +10,6 @@ [assembly: Feature( Id = "OrchardCore.Antivirus.ClamAV", Name = "ClamAV Antivirus Scanner", - Description = "Scans files with ClamAV before Orchard Core stores or imports them.", + Description = "Scans files with ClamAV before Orchard Core stores them.", Category = "Security" )] diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/OrchardCore.Antivirus.ClamAV.csproj b/src/OrchardCore.Modules/OrchardCore.Antivirus/OrchardCore.Antivirus.csproj similarity index 100% rename from src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/OrchardCore.Antivirus.ClamAV.csproj rename to src/OrchardCore.Modules/OrchardCore.Antivirus/OrchardCore.Antivirus.csproj diff --git a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Antivirus/Startup.cs similarity index 53% rename from src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Startup.cs rename to src/OrchardCore.Modules/OrchardCore.Antivirus/Startup.cs index 22875d3b8b3..2becf57d075 100644 --- a/src/OrchardCore.Modules/OrchardCore.Antivirus.ClamAV/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Antivirus/Startup.cs @@ -1,17 +1,25 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; +using OrchardCore.Antivirus.ClamAV; using OrchardCore.Environment.Shell.Configuration; using OrchardCore.FileStorage; -using OrchardCore.Antivirus.ClamAV.Services; using OrchardCore.Modules; -namespace OrchardCore.Antivirus.ClamAV; +namespace OrchardCore.Antivirus; public sealed class Startup : StartupBase +{ + public override void ConfigureServices(IServiceCollection services) + { + } +} + +[Feature("OrchardCore.Antivirus.ClamAV")] +public sealed class ClamAVStartup : StartupBase { private readonly IShellConfiguration _configuration; - public Startup(IShellConfiguration configuration) + public ClamAVStartup(IShellConfiguration configuration) { _configuration = configuration; } @@ -19,7 +27,7 @@ public Startup(IShellConfiguration configuration) public override void ConfigureServices(IServiceCollection services) { services.Configure(_configuration.GetSection(ClamAvOptions.ConfigSection)); - - services.Replace(ServiceDescriptor.Singleton()); + services.TryAddSingleton(); + services.TryAddEnumerable(ServiceDescriptor.Singleton()); } } diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs index c2060b51bee..d08e4c07c31 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs @@ -7,11 +7,11 @@ using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Localization; using Microsoft.Extensions.Logging; -using OrchardCore.FileStorage; using OrchardCore.Deployment.Remote.Services; using OrchardCore.Deployment.Remote.ViewModels; using OrchardCore.Deployment.Services; using OrchardCore.DisplayManagement.Notify; +using OrchardCore.FileStorage; using OrchardCore.Recipes.Models; namespace OrchardCore.Deployment.Remote.Controllers; @@ -23,7 +23,7 @@ public sealed class ImportRemoteInstanceController : Controller private readonly INotifier _notifier; private readonly ILogger _logger; private readonly IDataProtector _dataProtector; - private readonly IAntivirusScanner _fileUploadScanner; + private readonly FileEventService _fileEventService; internal readonly IHtmlLocalizer H; internal readonly IStringLocalizer S; @@ -32,14 +32,14 @@ public ImportRemoteInstanceController( IDataProtectionProvider dataProtectionProvider, RemoteClientService remoteClientService, IDeploymentManager deploymentManager, - IAntivirusScanner fileUploadScanner, + FileEventService fileEventService, INotifier notifier, IHtmlLocalizer htmlLocalizer, IStringLocalizer stringLocalizer, ILogger logger) { _deploymentManager = deploymentManager; - _fileUploadScanner = fileUploadScanner; + _fileEventService = fileEventService; _notifier = notifier; _logger = logger; _remoteClientService = remoteClientService; @@ -81,11 +81,19 @@ public async Task Import(ImportViewModel model) try { await using var uploadedStream = model.Content.OpenReadStream(); - await using var scannedStream = await CreateScannedStreamAsync(model.Content.FileName, model.Content.ContentType, uploadedStream); + await using var fileCreatingResult = await _fileEventService.ProcessAsync( + new FileCreatingContext(model.Content.FileName, model.Content.Length, model.Content.ContentType), + uploadedStream, + HttpContext.RequestAborted); + + if (!fileCreatingResult.Succeeded) + { + return StatusCode((int)HttpStatusCode.BadRequest, fileCreatingResult.ErrorMessage ?? $"The uploaded file '{model.Content.FileName}' was rejected."); + } - using (var fs = System.IO.File.Create(tempArchiveName)) + await using (var fs = System.IO.File.Create(tempArchiveName)) { - await scannedStream.CopyToAsync(fs); + await fileCreatingResult.Stream.CopyToAsync(fs, HttpContext.RequestAborted); } ZipFile.ExtractToDirectory(tempArchiveName, tempArchiveFolder); @@ -119,65 +127,4 @@ public async Task Import(ImportViewModel model) return Ok(); } - - private async Task CreateScannedStreamAsync(string fileName, string contentType, Stream stream) - { - if (_fileUploadScanner is NullAntivirusScanner) - { - return stream; - } - - var scanStream = stream; - - if (scanStream.CanSeek) - { - scanStream.Position = 0; - } - else - { - scanStream = await CreateTemporarySeekableStreamAsync(scanStream); - } - - try - { - var result = await _fileUploadScanner.ScanAsync( - new AntivirusScanContext(fileName, scanStream.Length, contentType), - scanStream); - - if (!result.IsClean) - { - throw new AntivirusScanningException(result.Message ?? $"The uploaded file '{fileName}' was rejected by the anti-virus scanner."); - } - - scanStream.Position = 0; - - return scanStream; - } - catch - { - if (scanStream != stream) - { - scanStream.Dispose(); - } - - throw; - } - } - - private static async Task CreateTemporarySeekableStreamAsync(Stream stream) - { - var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - var tempStream = new FileStream( - tempFilePath, - FileMode.CreateNew, - FileAccess.ReadWrite, - FileShare.None, - 81920, - FileOptions.Asynchronous | FileOptions.DeleteOnClose); - - await stream.CopyToAsync(tempStream); - tempStream.Position = 0; - - return tempStream; - } } diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs index 87b52cef58b..2b24a917870 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs @@ -14,7 +14,7 @@ public sealed class Startup : StartupBase public override void ConfigureServices(IServiceCollection services) { services.AddHttpClient(); - services.TryAddSingleton(); + services.TryAddSingleton(); services.AddNavigationProvider(); services.AddScoped(); diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs b/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs index 7b90152e977..51327fbf9e6 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs @@ -7,11 +7,11 @@ using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Localization; using Microsoft.Extensions.Logging; -using OrchardCore.FileStorage; using OrchardCore.Admin; using OrchardCore.Deployment.Services; using OrchardCore.Deployment.ViewModels; using OrchardCore.DisplayManagement.Notify; +using OrchardCore.FileStorage; using OrchardCore.Mvc.Utilities; using OrchardCore.Recipes.Models; @@ -24,7 +24,7 @@ public sealed class ImportController : Controller private readonly IAuthorizationService _authorizationService; private readonly INotifier _notifier; private readonly ILogger _logger; - private readonly IAntivirusScanner _fileUploadScanner; + private readonly FileEventService _fileEventService; internal readonly IHtmlLocalizer H; internal readonly IStringLocalizer S; @@ -32,7 +32,7 @@ public sealed class ImportController : Controller public ImportController( IDeploymentManager deploymentManager, IAuthorizationService authorizationService, - IAntivirusScanner fileUploadScanner, + FileEventService fileEventService, INotifier notifier, ILogger logger, IHtmlLocalizer htmlLocalizer, @@ -41,7 +41,7 @@ IStringLocalizer stringLocalizer { _deploymentManager = deploymentManager; _authorizationService = authorizationService; - _fileUploadScanner = fileUploadScanner; + _fileEventService = fileEventService; _notifier = notifier; _logger = logger; H = htmlLocalizer; @@ -74,11 +74,21 @@ public async Task Import(IFormFile importedPackage) try { await using var uploadedStream = importedPackage.OpenReadStream(); - await using var scannedStream = await CreateScannedStreamAsync(importedPackage.FileName, importedPackage.ContentType, uploadedStream); + await using var fileCreatingResult = await _fileEventService.ProcessAsync( + new FileCreatingContext(importedPackage.FileName, importedPackage.Length, importedPackage.ContentType), + uploadedStream, + HttpContext.RequestAborted); + + if (!fileCreatingResult.Succeeded) + { + await _notifier.ErrorAsync(H[fileCreatingResult.ErrorMessage ?? $"The uploaded file '{importedPackage.FileName}' was rejected."]); + + return RedirectToAction(nameof(Index)); + } - using (var stream = new FileStream(tempArchiveName, FileMode.Create)) + await using (var stream = new FileStream(tempArchiveName, FileMode.Create)) { - await scannedStream.CopyToAsync(stream); + await fileCreatingResult.Stream.CopyToAsync(stream, HttpContext.RequestAborted); } if (importedPackage.FileName.EndsWith(".zip")) @@ -193,65 +203,4 @@ public async Task Json(ImportJsonViewModel model) return View(model); } - - private async Task CreateScannedStreamAsync(string fileName, string contentType, Stream stream) - { - if (_fileUploadScanner is NullAntivirusScanner) - { - return stream; - } - - var scanStream = stream; - - if (scanStream.CanSeek) - { - scanStream.Position = 0; - } - else - { - scanStream = await CreateTemporarySeekableStreamAsync(scanStream); - } - - try - { - var result = await _fileUploadScanner.ScanAsync( - new AntivirusScanContext(fileName, scanStream.Length, contentType), - scanStream); - - if (!result.IsClean) - { - throw new AntivirusScanningException(result.Message ?? $"The uploaded file '{fileName}' was rejected by the anti-virus scanner."); - } - - scanStream.Position = 0; - - return scanStream; - } - catch - { - if (scanStream != stream) - { - scanStream.Dispose(); - } - - throw; - } - } - - private static async Task CreateTemporarySeekableStreamAsync(Stream stream) - { - var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - var tempStream = new FileStream( - tempFilePath, - FileMode.CreateNew, - FileAccess.ReadWrite, - FileShare.None, - 81920, - FileOptions.Asynchronous | FileOptions.DeleteOnClose); - - await stream.CopyToAsync(tempStream); - tempStream.Position = 0; - - return tempStream; - } } diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs index b95342eb2c2..24164c201b2 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs @@ -22,7 +22,7 @@ public sealed class Startup : StartupBase { public override void ConfigureServices(IServiceCollection services) { - services.TryAddSingleton(); + services.TryAddSingleton(); services.AddDeploymentServices(); services.AddNavigationProvider(); diff --git a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs index 13d651724d7..87e7b5da82a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs @@ -103,7 +103,7 @@ public override void ConfigureServices(IServiceCollection services) var mediaOptions = serviceProvider.GetRequiredService>().Value; var mediaEventHandlers = serviceProvider.GetServices(); var mediaCreatingEventHandlers = serviceProvider.GetServices(); - var antivirusScanner = serviceProvider.GetRequiredService(); + var fileEventService = serviceProvider.GetRequiredService(); var clock = serviceProvider.GetRequiredService(); var logger = serviceProvider.GetRequiredService>(); var amazonS3Client = serviceProvider.GetService(); @@ -127,7 +127,7 @@ public override void ConfigureServices(IServiceCollection services) mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, - antivirusScanner, + fileEventService, logger); })); diff --git a/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs index c649a383d2e..8ace11ab081 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs @@ -95,7 +95,7 @@ public override void ConfigureServices(IServiceCollection services) var contentTypeProvider = serviceProvider.GetRequiredService(); var mediaEventHandlers = serviceProvider.GetServices(); var mediaCreatingEventHandlers = serviceProvider.GetServices(); - var fileUploadScanner = serviceProvider.GetRequiredService(); + var fileEventService = serviceProvider.GetRequiredService(); var logger = serviceProvider.GetRequiredService>(); var fileStore = new BlobFileStore(blobStorageOptions, clock, contentTypeProvider); @@ -110,7 +110,7 @@ public override void ConfigureServices(IServiceCollection services) mediaUrlBase = fileStore.Combine(originalPathBase.Value, mediaUrlBase); } - return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileUploadScanner, logger); + return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileEventService, logger); })); services.AddSingleton(); diff --git a/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs index e153e0c8ebc..2d3501cadb8 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs @@ -87,7 +87,7 @@ public override void ConfigureServices(IServiceCollection services) services.AddResourceConfiguration(); services.AddTransient, MediaOptionsConfiguration>(); - services.TryAddSingleton(); + services.TryAddSingleton(); services.AddSingleton(serviceProvider => { @@ -116,7 +116,7 @@ public override void ConfigureServices(IServiceCollection services) var mediaOptions = serviceProvider.GetRequiredService>().Value; var mediaEventHandlers = serviceProvider.GetServices(); var mediaCreatingEventHandlers = serviceProvider.GetServices(); - var fileUploadScanner = serviceProvider.GetRequiredService(); + var fileEventService = serviceProvider.GetRequiredService(); var fileSystemStoreLogger = serviceProvider.GetRequiredService>(); var defaultMediaFileStoreLogger = serviceProvider.GetRequiredService>(); @@ -134,7 +134,7 @@ public override void ConfigureServices(IServiceCollection services) mediaUrlBase = fileStore.Combine(originalPathBase.Value, mediaUrlBase); } - return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileUploadScanner, defaultMediaFileStoreLogger); + return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileEventService, defaultMediaFileStoreLogger); }); services.AddPermissionProvider(); diff --git a/src/OrchardCore/OrchardCore.Application.Cms.Core.Targets/OrchardCore.Application.Cms.Core.Targets.csproj b/src/OrchardCore/OrchardCore.Application.Cms.Core.Targets/OrchardCore.Application.Cms.Core.Targets.csproj index 28f81b24bef..f3c19af713e 100644 --- a/src/OrchardCore/OrchardCore.Application.Cms.Core.Targets/OrchardCore.Application.Cms.Core.Targets.csproj +++ b/src/OrchardCore/OrchardCore.Application.Cms.Core.Targets/OrchardCore.Application.Cms.Core.Targets.csproj @@ -41,7 +41,7 @@ - + diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusResult.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusResult.cs deleted file mode 100644 index 5bf27e47128..00000000000 --- a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusResult.cs +++ /dev/null @@ -1,21 +0,0 @@ -namespace OrchardCore.FileStorage; - -public sealed class AntivirusResult -{ - private AntivirusResult(bool isClean, string message, string threatName) - { - IsClean = isClean; - Message = message; - ThreatName = threatName; - } - - public bool IsClean { get; } - - public string Message { get; } - - public string ThreatName { get; } - - public static AntivirusResult Clean { get; } = new(true, null, null); - - public static AntivirusResult Unsafe(string message, string threatName = null) => new(false, message, threatName); -} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanContext.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanContext.cs deleted file mode 100644 index e48acd6c626..00000000000 --- a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/AntivirusScanContext.cs +++ /dev/null @@ -1,19 +0,0 @@ -namespace OrchardCore.FileStorage; - -public sealed class AntivirusScanContext -{ - public AntivirusScanContext(string path, long? length = null, string contentType = null) - { - Path = path; - Length = length; - ContentType = contentType; - } - - public string Path { get; } - - public string FileName => System.IO.Path.GetFileName(Path); - - public long? Length { get; } - - public string ContentType { get; } -} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreatingContext.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreatingContext.cs new file mode 100644 index 00000000000..a2f073c5f33 --- /dev/null +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreatingContext.cs @@ -0,0 +1,19 @@ +namespace OrchardCore.FileStorage; + +public sealed class FileCreatingContext +{ + public FileCreatingContext(string path, long? length = null, string contentType = null) + { + Path = path; + Length = length; + ContentType = contentType; + } + + public string Path { get; set; } + + public string FileName => System.IO.Path.GetFileName(Path); + + public long? Length { get; set; } + + public string ContentType { get; set; } +} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreatingResult.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreatingResult.cs new file mode 100644 index 00000000000..98c28dbb7e7 --- /dev/null +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreatingResult.cs @@ -0,0 +1,55 @@ +using OrchardCore.Infrastructure; + +namespace OrchardCore.FileStorage; + +/// +/// Represents the outcome of processing a file before it is stored. +/// +public sealed class FileCreatingResult : Result, IAsyncDisposable +{ + private readonly bool _ownsStream; + + private FileCreatingResult(Stream stream, bool ownsStream) + : base(true) + { + Stream = stream; + _ownsStream = ownsStream; + } + + private FileCreatingResult(Stream stream, bool ownsStream, IEnumerable errors) + : base(errors) + { + Stream = stream; + _ownsStream = ownsStream; + } + + /// + /// Gets the stream that should continue through the upload pipeline. + /// + public Stream Stream { get; } + + public string ErrorMessage + => Errors + .Select(error => error.Message?.Value) + .FirstOrDefault(message => !string.IsNullOrWhiteSpace(message)); + + public static FileCreatingResult Success(Stream stream) + => new(stream, false); + + public static FileCreatingResult Failed(Stream stream = null, params ResultError[] errors) + => new(stream, false, errors); + + internal static FileCreatingResult Create(Stream stream, bool ownsStream, IEnumerable errors) + => new(stream, ownsStream, errors); + + internal static FileCreatingResult Create(Stream stream, bool ownsStream) + => new(stream, ownsStream); + + public async ValueTask DisposeAsync() + { + if (_ownsStream && Stream is not null) + { + await Stream.DisposeAsync(); + } + } +} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileEventService.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileEventService.cs new file mode 100644 index 00000000000..2958d1477a0 --- /dev/null +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileEventService.cs @@ -0,0 +1,85 @@ +namespace OrchardCore.FileStorage; + +/// +/// Coordinates instances for user-uploaded files. +/// +public sealed class FileEventService +{ + private readonly IEnumerable _handlers; + + public FileEventService(IEnumerable handlers) + { + _handlers = handlers; + } + + /// + /// Runs the pre-create pipeline and returns the stream that should be stored. + /// The caller owns the original and should dispose the returned + /// to clean up any replacement stream created by handlers. + /// + public async Task ProcessAsync( + FileCreatingContext context, + Stream stream, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(context); + ArgumentNullException.ThrowIfNull(stream); + + var currentStream = stream; + + try + { + foreach (var handler in _handlers) + { + var creatingStream = currentStream; + var result = await handler.CreatingAsync(context, creatingStream, cancellationToken); + + if (result is null) + { + throw new InvalidOperationException($"{handler.GetType().Name} returned a null {nameof(FileCreatingResult)}."); + } + + currentStream = result.Stream ?? creatingStream; + + if (creatingStream != currentStream && creatingStream != stream) + { + await creatingStream.DisposeAsync(); + } + + if (!result.Succeeded) + { + return FileCreatingResult.Create(currentStream, currentStream != stream, result.Errors.ToList()); + } + } + + return FileCreatingResult.Create(currentStream, currentStream != stream); + } + catch + { + if (currentStream != stream) + { + await currentStream.DisposeAsync(); + } + + throw; + } + } + + /// + /// Runs the post-create pipeline after the file was stored successfully. + /// + public async Task CreatedAsync(IFileStoreEntry fileInfo, CancellationToken cancellationToken = default) + { + if (!_handlers.Any()) + { + return; + } + + ArgumentNullException.ThrowIfNull(fileInfo); + + foreach (var handler in _handlers) + { + await handler.CreatedAsync(fileInfo, cancellationToken); + } + } +} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/IAntivirusScanner.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/IAntivirusScanner.cs deleted file mode 100644 index b2ffb86fc12..00000000000 --- a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/IAntivirusScanner.cs +++ /dev/null @@ -1,6 +0,0 @@ -namespace OrchardCore.FileStorage; - -public interface IAntivirusScanner -{ - Task ScanAsync(AntivirusScanContext context, Stream stream); -} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/IFileEventHandler.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/IFileEventHandler.cs new file mode 100644 index 00000000000..78c4be8ed52 --- /dev/null +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/IFileEventHandler.cs @@ -0,0 +1,8 @@ +namespace OrchardCore.FileStorage; + +public interface IFileEventHandler +{ + Task CreatingAsync(FileCreatingContext context, Stream stream, CancellationToken cancellationToken = default); + + Task CreatedAsync(IFileStoreEntry fileInfo, CancellationToken cancellationToken = default); +} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/NullAntivirusScanner.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/NullAntivirusScanner.cs deleted file mode 100644 index 378d6c79f84..00000000000 --- a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/NullAntivirusScanner.cs +++ /dev/null @@ -1,7 +0,0 @@ -namespace OrchardCore.FileStorage; - -public sealed class NullAntivirusScanner : IAntivirusScanner -{ - public Task ScanAsync(AntivirusScanContext context, Stream stream) - => Task.FromResult(AntivirusResult.Clean); -} diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/OrchardCore.FileStorage.Abstractions.csproj b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/OrchardCore.FileStorage.Abstractions.csproj index 5a8ad8eae76..90b9bbc7aa0 100644 --- a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/OrchardCore.FileStorage.Abstractions.csproj +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/OrchardCore.FileStorage.Abstractions.csproj @@ -15,4 +15,8 @@ + + + + diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Result.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Result.cs index 837ab69caed..3cf44afc2eb 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Result.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Result.cs @@ -7,22 +7,26 @@ namespace OrchardCore.Infrastructure; /// public class Result { + private static readonly ResultError[] _emptyErrors = []; private static readonly Result _success = new Result { Succeeded = true }; - private readonly List _errors = []; + private IEnumerable _errors = _emptyErrors; - private Result() + protected Result() { } + protected Result(bool succeeded) + { + Succeeded = succeeded; + } + /// /// Initializes a new instance of the class with the specified success status and errors. /// - /// Indicates whether the operation succeeded. /// The errors that occurred during the operation. - protected Result(bool succeeded, List errors) + protected Result(IEnumerable errors) { - Succeeded = succeeded; - _errors = errors; + _errors = errors ?? _emptyErrors; } /// @@ -48,7 +52,7 @@ protected Result(bool succeeded, List errors) /// The value returned by the operation. /// A successful result instance with the specified value. public static Result Success(TValue value) - => new Result(value, true, []); + => new Result(value); /// /// Returns a failed result instance with the specified errors. @@ -57,12 +61,7 @@ public static Result Success(TValue value) /// A failed result instance with the specified errors. public static Result Failed(params IEnumerable errors) { - var result = new Result(); - - if (errors is not null) - { - result._errors.AddRange(errors); - } + var result = new Result(errors); return result; } @@ -91,5 +90,5 @@ public static Result Failed(LocalizedString error) => Failed(new ResultError /// The errors that occurred during the operation. /// A failed result instance with the specified error message. public static Result Failed(params ResultError[] errors) - => new Result(default, false, errors.ToList()); + => new Result(default, errors); } diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/ResultOfT.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/ResultOfT.cs index ef521ec0a38..19262b67bec 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/ResultOfT.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/ResultOfT.cs @@ -15,8 +15,16 @@ public class Result : Result /// Initializes a new instance of the class. /// /// The value returned by the operation. - /// Indicates whether the operation succeeded. /// The errors that occurred during the operation. - protected internal Result(TValue value, bool succeeded, List errors) - : base(succeeded, errors) => Value = value; + protected internal Result(TValue value, IEnumerable errors) + : base(errors) + { + Value = value; + } + + protected internal Result(TValue value) + : base(true) + { + Value = value; + } } diff --git a/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs b/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs index 54c9d581fdc..bb932000da3 100644 --- a/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs +++ b/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs @@ -17,7 +17,7 @@ public class DefaultMediaFileStore : IMediaFileStore private readonly string _cdnBaseUrl; private readonly IEnumerable _mediaEventHandlers; private readonly IEnumerable _mediaCreatingEventHandlers; - private readonly IAntivirusScanner _antivirusScanner; + private readonly FileEventService _fileEventService; private readonly ILogger _logger; private bool _requestBasePathValidated; @@ -28,7 +28,7 @@ public DefaultMediaFileStore( string cdnBaseUrl, IEnumerable mediaEventHandlers, IEnumerable mediaCreatingEventHandlers, - IAntivirusScanner antivirusScanner, + FileEventService fileEventService, ILogger logger) { _fileStore = fileStore; @@ -41,7 +41,7 @@ public DefaultMediaFileStore( _mediaEventHandlers = mediaEventHandlers; _mediaCreatingEventHandlers = mediaCreatingEventHandlers; - _antivirusScanner = antivirusScanner; + _fileEventService = fileEventService; _logger = logger; } @@ -195,18 +195,7 @@ public virtual async Task CreateFileFromStreamAsync(string path, Stream } } - var scannedStream = await ScanAsync(context.Path, outputStream); - - if (scannedStream != outputStream && outputStream != inputStream) - { - outputStream.Dispose(); - } - - outputStream = scannedStream; - - await ValidateAvailableStorageAsync(outputStream.Length); - - return await _fileStore.CreateFileFromStreamAsync(context.Path, outputStream, overwrite); + return await CreateFileAsync(new FileCreatingContext(context.Path), outputStream, overwrite); } finally { @@ -216,21 +205,7 @@ public virtual async Task CreateFileFromStreamAsync(string path, Stream } else { - var scannedStream = await ScanAsync(path, inputStream); - - try - { - await ValidateAvailableStorageAsync(scannedStream.Length); - - return await _fileStore.CreateFileFromStreamAsync(path, scannedStream, overwrite); - } - finally - { - if (scannedStream != inputStream) - { - scannedStream.Dispose(); - } - } + return await CreateFileAsync(new FileCreatingContext(path), inputStream, overwrite); } } @@ -287,64 +262,22 @@ private async Task ValidateAvailableStorageAsync(long requiredStorageSpace) } } - private async Task ScanAsync(string path, Stream stream) + private async Task CreateFileAsync(FileCreatingContext fileCreatingContext, Stream stream, bool overwrite) { - if (_antivirusScanner is NullAntivirusScanner) - { - return stream; - } - - var scanStream = stream; + await using var fileCreatingResult = await _fileEventService.ProcessAsync(fileCreatingContext, stream); - if (scanStream.CanSeek) - { - scanStream.Position = 0; - } - else + if (!fileCreatingResult.Succeeded) { - scanStream = await CreateTemporarySeekableStreamAsync(scanStream); + throw new FileStoreException(fileCreatingResult.ErrorMessage ?? $"The file '{fileCreatingContext.FileName}' was rejected."); } - try - { - var result = await _antivirusScanner.ScanAsync( - new AntivirusScanContext(path, scanStream.Length), - scanStream); + await ValidateAvailableStorageAsync(fileCreatingResult.Stream.Length); - if (!result.IsClean) - { - throw new AntivirusScanningException(result.Message ?? $"The uploaded file '{Path.GetFileName(path)}' was rejected by the anti-virus scanner."); - } + var createdPath = await _fileStore.CreateFileFromStreamAsync(fileCreatingContext.Path, fileCreatingResult.Stream, overwrite); + var fileInfo = await _fileStore.GetFileInfoAsync(createdPath); - scanStream.Position = 0; + await _fileEventService.CreatedAsync(fileInfo); - return scanStream; - } - catch - { - if (scanStream != stream) - { - scanStream.Dispose(); - } - - throw; - } - } - - private static async Task CreateTemporarySeekableStreamAsync(Stream stream) - { - var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - var tempStream = new FileStream( - tempFilePath, - FileMode.CreateNew, - FileAccess.ReadWrite, - FileShare.None, - 81920, - FileOptions.Asynchronous | FileOptions.DeleteOnClose); - - await stream.CopyToAsync(tempStream); - tempStream.Position = 0; - - return tempStream; + return createdPath; } } diff --git a/src/docs/reference/README.md b/src/docs/reference/README.md index bead5c3a123..f6994f1770a 100644 --- a/src/docs/reference/README.md +++ b/src/docs/reference/README.md @@ -91,6 +91,7 @@ Here's a categorized overview of all built-in Orchard Core features at a glance. ### Extensibility +- [File Upload Security](core/file-upload-security.md) - [Auto Setup](modules/AutoSetup/README.md) - [GraphQL](modules/Apis.GraphQL/README.md) - [GraphQL queries](modules/Apis.GraphQL.Abstractions/README.md) diff --git a/src/docs/reference/core/file-upload-security.md b/src/docs/reference/core/file-upload-security.md new file mode 100644 index 00000000000..5a2910c9760 --- /dev/null +++ b/src/docs/reference/core/file-upload-security.md @@ -0,0 +1,113 @@ +# File Upload Security + +When your feature accepts a user-uploaded file and stores it outside Orchard Core's built-in media flow, use `FileEventService` to run the shared pre-storage security pipeline. + +This ensures every `IFileEventHandler` can inspect or replace the stream before the file is written permanently. If any handler returns a failed `FileCreatingResult`, the upload must be aborted and the file must not be stored. + +## When to use `FileEventService` + +Use `FileEventService` in custom controllers, admin endpoints, APIs, recipe importers, or background flows that: + +- accept a file from a user; +- write that file to disk, cloud storage, or another permanent store; and +- do **not** already go through `IMediaFileStore.CreateFileFromStreamAsync()`. + +!!! note + `DefaultMediaFileStore` already uses `FileEventService` internally. If your code uploads through `IMediaFileStore`, do not call the service a second time. + +## Core types + +- `FileEventService`: runs `CreatingAsync()` before storage and `CreatedAsync()` after storage succeeds. +- `FileCreatingContext`: describes the file being processed. +- `FileCreatingResult`: returns the stream that should continue through the pipeline and whether creation should proceed. +- `IFileEventHandler`: participates in the upload pipeline. + +## Ownership and cleanup + +- The original upload stream stays owned by the caller. +- If a handler replaces the stream, `FileEventService` disposes the superseded intermediate stream. +- The `FileCreatingResult` returned from `ProcessAsync()` owns the final replacement stream when one was created, so callers should use `await using` and keep that result alive for as long as they need the processed stream. +- If `ProcessAsync()` returns a failed result, dispose that result the same way and abort the upload without storing the file. +- After the file has been written permanently, disposing `FileCreatingResult` cleans up any temporary replacement stream used during the upload pipeline. + +Because cleanup is modeled through stream disposal, handlers normally do **not** need a separate `FailedAsync()` callback. A handler that creates a temporary stream should return that stream and let the service/result lifecycle clean it up. + +## Using `FileEventService` + +Call `ProcessAsync()` before writing the file. If the returned result did not succeed, abort the request. Only call `CreatedAsync()` after the file was stored successfully. + +```csharp +using Microsoft.AspNetCore.Http; +using OrchardCore.FileStorage; + +public sealed class CustomUploadService +{ + private readonly FileEventService _fileEventService; + private readonly IFileStore _fileStore; + + public CustomUploadService( + FileEventService fileEventService, + IFileStore fileStore) + { + _fileEventService = fileEventService; + _fileStore = fileStore; + } + + public async Task UploadAsync(IFormFile file, CancellationToken cancellationToken) + { + await using var uploadedStream = file.OpenReadStream(); + await using var fileCreatingResult = await _fileEventService.ProcessAsync( + new FileCreatingContext(file.FileName, file.Length, file.ContentType), + uploadedStream, + cancellationToken); + + if (!fileCreatingResult.Succeeded) + { + throw new FileStoreException(fileCreatingResult.ErrorMessage ?? $"The uploaded file '{file.FileName}' was rejected before it could be stored."); + } + + // Use the processed stream while the result is still in scope. + var path = await _fileStore.CreateFileFromStreamAsync(file.FileName, fileCreatingResult.Stream); + var fileInfo = await _fileStore.GetFileInfoAsync(path); + + await _fileEventService.CreatedAsync(fileInfo, cancellationToken); + + return path; + } +} +``` + +## Implementing a handler + +Return `FileCreatingResult.Failed(...)` from `IFileEventHandler.CreatingAsync()` to stop the upload before it is stored. This is where a module would perform checks such as antivirus scanning, content inspection, or file-type validation. + +```csharp +using Microsoft.Extensions.Localization; +using OrchardCore.FileStorage; +using OrchardCore.Infrastructure; + +namespace MyModule.Services; + +public sealed class RejectExecutableFileEventHandler : IFileEventHandler +{ + public Task CreatingAsync(FileCreatingContext context, Stream stream, CancellationToken cancellationToken = default) + { + if (string.Equals(Path.GetExtension(context.FileName), ".exe", StringComparison.OrdinalIgnoreCase)) + { + return Task.FromResult(FileCreatingResult.Failed(stream, new ResultError + { + Message = new LocalizedString(nameof(RejectExecutableFileEventHandler), "Executable files are not allowed."), + })); + } + + return Task.FromResult(FileCreatingResult.Success(stream)); + } + + public Task CreatedAsync(IFileStoreEntry fileInfo, CancellationToken cancellationToken = default) + => Task.CompletedTask; +} +``` + +## Security guidance + +Always run `FileEventService` before any permanent write. Do not save the uploaded file first and scan it afterward, since a failed scan must abort the upload before the file is persisted. diff --git a/src/docs/reference/modules/Antivirus.ClamAV/README.md b/src/docs/reference/modules/ClamAV/README.md similarity index 51% rename from src/docs/reference/modules/Antivirus.ClamAV/README.md rename to src/docs/reference/modules/ClamAV/README.md index fa99c423837..69a197f2451 100644 --- a/src/docs/reference/modules/Antivirus.ClamAV/README.md +++ b/src/docs/reference/modules/ClamAV/README.md @@ -1,17 +1,18 @@ -# ClamAV Antivirus Scanner (`OrchardCore.Antivirus.ClamAV`) +# ClamAV (`OrchardCore.Antivirus.ClamAV`) -The ClamAV Antivirus Scanner module scans files with a `clamd` service before Orchard Core stores or imports them. +The ClamAV module scans files with a `clamd` service before Orchard Core stores or imports them. When the feature is enabled, uploads fail closed: - Malware detections are rejected before storage. - Scanner connectivity, timeout, or protocol failures also reject the upload. +- The ClamAV connection is reused per configuration to avoid creating a new TCP client for every scan. -The scanner is wired through the reusable `IAntivirusScanner` abstraction, so custom modules can replace it with another implementation when they need a different anti-virus provider. +The scanner is wired through Orchard Core's file event handling abstractions, so uploads can be validated before storage without coupling media and deployment flows to a scanner-specific interface. ClamAV participates in that flow as an `IFileEventHandler`, and `FileEventService` aborts the upload when ClamAV returns a failed `FileCreatingResult`. ## Configuration -Configure the ClamAV connection in application configuration: +Configure the ClamAV connection in application configuration. The settings key remains `OrchardCore_Antivirus_ClamAV` for compatibility: ```json { @@ -38,11 +39,15 @@ OrchardCore__Antivirus_ClamAV__TransferTimeoutSeconds=30 ## Usage 1. Configure the ClamAV settings. -2. Enable the `ClamAV Antivirus Scanner` feature. +2. Enable the `ClamAV Antivirus Scanner` feature (`OrchardCore.Antivirus.ClamAV`). 3. Ensure a reachable `clamd` instance is running. If the feature is enabled without a valid ClamAV connection, uploads are rejected until the scanner can verify them. +This feature integrates with the shared file upload security pipeline through `IFileEventHandler`, so uploads can be rejected before Orchard Core stores them permanently. + +See [File Upload Security](../../core/file-upload-security.md) for the canonical guidance on invoking `FileEventService` in custom upload flows and aborting rejected files before they are stored. + ## Notes - The currently audited upload surfaces covered by this change are media uploads plus deployment package imports, both local and remote. diff --git a/src/docs/releases/3.0.0.md b/src/docs/releases/3.0.0.md index b63732b9650..9a9af354c9b 100644 --- a/src/docs/releases/3.0.0.md +++ b/src/docs/releases/3.0.0.md @@ -87,6 +87,19 @@ The email event handler callbacks on `IEmailServiceEvents`, including overrides This is a source and binary breaking change for custom email services, providers, and event handlers. Update your implementations and overrides to include the new `cancellationToken` parameter. +### File Upload Security + +Orchard Core now provides a file upload event pipeline for securing user-uploaded files before they are stored: + +- `IFileEventHandler` +- `FileCreatingContext` +- `FileCreatingResult` +- `FileEventService` + +Use `FileEventService.ProcessAsync()` anywhere your code accepts a user-uploaded file and stores it directly. If the returned `FileCreatingResult.Succeeded` value is `false`, you must abort the upload and avoid persisting the file. Only call `FileEventService.CreatedAsync()` after the file was stored successfully. + +The ClamAV integration now uses this event pipeline through the `OrchardCore.Antivirus.ClamAV` feature. + ### Admin Theme — Removal of `TheAdminThemeOptions` and CSS Helper Extensions The `TheAdminThemeOptions` class and the `CssOrchardHelperExtensions` (from `OrchardCore.DisplayManagement`) have been removed. Admin editor views now use static CSS classes prefixed with `ocat-` (Orchard Core Admin Theme) instead of C# helper methods that injected CSS classes at runtime. diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs index 4aa61d6cc62..b759287db7a 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs @@ -35,7 +35,7 @@ public async Task ShouldProcess(string cdnBaseUrl, string text, string expected) cdnBaseUrl, [], [], - new NullAntivirusScanner(), + new FileEventService([]), Mock.Of>()); var defaultHttpContext = new DefaultHttpContext(); diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVAntivirusScannerTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVAntivirusScannerTests.cs deleted file mode 100644 index 989dd783a47..00000000000 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVAntivirusScannerTests.cs +++ /dev/null @@ -1,198 +0,0 @@ -using System.Buffers.Binary; -using System.Net.Sockets; -using OrchardCore.Antivirus.ClamAV; -using OrchardCore.Antivirus.ClamAV.Services; -using OrchardCore.FileStorage; - -namespace OrchardCore.Tests.Modules.OrchardCore.Media; - -public class ClamAVAntivirusScannerTests -{ - [Fact] - public async Task ScanAsync_ReturnsCleanResult_WhenClamAvReturnsOk() - { - await using var server = await FakeClamAvServer.StartAsync("stream: OK\n"); - var scanner = CreateScanner(server.Port); - - var result = await scanner.ScanAsync( - new AntivirusScanContext("folder/test.txt"), - new MemoryStream("hello world"u8.ToArray())); - - var request = await server.Completion; - - Assert.True(result.IsClean); - Assert.Equal("nINSTREAM\n", request.Command); - Assert.Equal("hello world"u8.ToArray(), request.Payload); - } - - [Fact] - public async Task ScanAsync_ReturnsUnsafeResult_WhenClamAvFindsMalware() - { - await using var server = await FakeClamAvServer.StartAsync("stream: Eicar-Test-Signature FOUND\n"); - var scanner = CreateScanner(server.Port); - - var result = await scanner.ScanAsync( - new AntivirusScanContext("folder/test.txt"), - new MemoryStream("hello world"u8.ToArray())); - - Assert.False(result.IsClean); - Assert.Equal("Eicar-Test-Signature", result.ThreatName); - Assert.Equal("The uploaded file 'test.txt' was rejected because ClamAV detected 'Eicar-Test-Signature'.", result.Message); - } - - [Fact] - public async Task ScanAsync_Throws_WhenClamAvReturnsError() - { - await using var server = await FakeClamAvServer.StartAsync("INSTREAM size limit exceeded. ERROR\n"); - var scanner = CreateScanner(server.Port); - - var exception = await Assert.ThrowsAsync(() => - scanner.ScanAsync( - new AntivirusScanContext("folder/test.txt"), - new MemoryStream("hello world"u8.ToArray()))); - - Assert.Equal("The ClamAV antivirus scanner returned an unexpected response while scanning 'test.txt': INSTREAM size limit exceeded. ERROR", exception.Message); - } - - [Fact] - public async Task ScanAsync_Throws_WhenHostIsMissing() - { - var scanner = new ClamAVAntivirusScanner( - Options.Create(new ClamAvOptions - { - Host = "", - }), - Mock.Of>()); - - var exception = await Assert.ThrowsAsync(() => - scanner.ScanAsync( - new AntivirusScanContext("folder/test.txt"), - new MemoryStream("hello world"u8.ToArray()))); - - Assert.Equal("The ClamAV antivirus scanner is enabled but the host setting is missing.", exception.Message); - } - - private static ClamAVAntivirusScanner CreateScanner(int port) => - new( - Options.Create(new ClamAvOptions - { - Host = IPAddress.Loopback.ToString(), - Port = port, - ConnectTimeoutSeconds = 5, - TransferTimeoutSeconds = 5, - }), - Mock.Of>()); - - private sealed class FakeClamAvServer : IAsyncDisposable - { - private readonly TcpListener _listener; - - private FakeClamAvServer(TcpListener listener, Task completion) - { - _listener = listener; - Completion = completion; - } - - public int Port => ((IPEndPoint)_listener.LocalEndpoint).Port; - - public Task Completion { get; } - - public static Task StartAsync(string response) - { - var listener = new TcpListener(IPAddress.Loopback, 0); - listener.Start(); - - var completion = Task.Run(async () => - { - using var client = await listener.AcceptTcpClientAsync(); - await using var networkStream = client.GetStream(); - - var command = await ReadLineAsync(networkStream); - var payload = await ReadPayloadAsync(networkStream); - var responseBytes = Encoding.ASCII.GetBytes(response); - - await networkStream.WriteAsync(responseBytes); - await networkStream.FlushAsync(); - - return new ClamAvRequest(command, payload); - }); - - return Task.FromResult(new FakeClamAvServer(listener, completion)); - } - - public ValueTask DisposeAsync() - { - _listener.Stop(); - - return ValueTask.CompletedTask; - } - - private static async Task ReadLineAsync(NetworkStream stream) - { - using var commandStream = new MemoryStream(); - var buffer = new byte[1]; - - while (true) - { - var bytesRead = await stream.ReadAsync(buffer.AsMemory(0, 1)); - - if (bytesRead == 0) - { - break; - } - - commandStream.WriteByte(buffer[0]); - - if (buffer[0] == '\n') - { - break; - } - } - - return Encoding.ASCII.GetString(commandStream.ToArray()); - } - - private static async Task ReadPayloadAsync(Stream stream) - { - using var payloadStream = new MemoryStream(); - var lengthBuffer = new byte[sizeof(int)]; - - while (true) - { - await ReadExactlyAsync(stream, lengthBuffer); - - var chunkLength = BinaryPrimitives.ReadInt32BigEndian(lengthBuffer); - - if (chunkLength == 0) - { - break; - } - - var chunk = new byte[chunkLength]; - await ReadExactlyAsync(stream, chunk); - await payloadStream.WriteAsync(chunk); - } - - return payloadStream.ToArray(); - } - - private static async Task ReadExactlyAsync(Stream stream, byte[] buffer) - { - var offset = 0; - - while (offset < buffer.Length) - { - var bytesRead = await stream.ReadAsync(buffer.AsMemory(offset, buffer.Length - offset)); - - if (bytesRead == 0) - { - throw new EndOfStreamException(); - } - - offset += bytesRead; - } - } - } - - private sealed record ClamAvRequest(string Command, byte[] Payload); -} diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVFileEventHandlerTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVFileEventHandlerTests.cs new file mode 100644 index 00000000000..26089194653 --- /dev/null +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ClamAVFileEventHandlerTests.cs @@ -0,0 +1,301 @@ +using System.Buffers.Binary; +using System.Net.Sockets; +using OrchardCore.Antivirus.ClamAV; +using OrchardCore.FileStorage; + +namespace OrchardCore.Tests.Modules.OrchardCore.Media; + +public class ClamAvFileEventHandlerTests +{ + [Fact] + public async Task CreatingAsync_ReturnsSeekableStream_WhenClamAvReturnsOk() + { + await using var server = await FakeClamAvServer.StartAsync("stream: OK\n"); + using var factory = CreateFactory(); + var handler = CreateHandler(server.Port, factory); + + var inputStream = new MemoryStream("hello world"u8.ToArray()); + var result = await handler.CreatingAsync(new FileCreatingContext("folder/test.txt"), inputStream, TestContext.Current.CancellationToken); + var request = (await server.Completion).Single(); + + Assert.True(result.Succeeded); + Assert.Same(inputStream, result.Stream); + Assert.Equal("nINSTREAM\n", request.Command); + Assert.Equal("hello world"u8.ToArray(), request.Payload); + } + + [Fact] + public async Task CreatingAsync_BuffersNonSeekableStreams() + { + await using var server = await FakeClamAvServer.StartAsync("stream: OK\n"); + using var factory = CreateFactory(); + var handler = CreateHandler(server.Port, factory); + await using var baseStream = new MemoryStream("hello world"u8.ToArray()); + await using var inputStream = new NonSeekableReadStream(baseStream); + + var result = await handler.CreatingAsync(new FileCreatingContext("folder/test.txt"), inputStream); + + Assert.True(result.Succeeded); + Assert.NotSame(inputStream, result.Stream); + Assert.True(result.Stream.CanSeek); + + using var copy = new MemoryStream(); + await result.Stream.CopyToAsync(copy); + Assert.Equal("hello world"u8.ToArray(), copy.ToArray()); + } + + [Fact] + public async Task CreatingAsync_ReturnsFailedResult_WhenClamAvFindsMalware() + { + await using var server = await FakeClamAvServer.StartAsync("stream: Eicar-Test-Signature FOUND\n"); + using var factory = CreateFactory(); + var handler = CreateHandler(server.Port, factory); + + var result = await handler.CreatingAsync(new FileCreatingContext("folder/test.txt"), new MemoryStream("hello world"u8.ToArray())); + + Assert.False(result.Succeeded); + Assert.Equal("The uploaded file 'test.txt' was rejected because ClamAV detected 'Eicar-Test-Signature'.", result.ErrorMessage); + } + + [Fact] + public async Task CreatingAsync_Throws_WhenClamAvReturnsError() + { + await using var server = await FakeClamAvServer.StartAsync("INSTREAM size limit exceeded. ERROR\n"); + using var factory = CreateFactory(); + var handler = CreateHandler(server.Port, factory); + + var exception = await Assert.ThrowsAsync(() => + handler.CreatingAsync(new FileCreatingContext("folder/test.txt"), new MemoryStream("hello world"u8.ToArray()))); + + Assert.Equal("The ClamAV antivirus scanner returned an unexpected response while scanning 'test.txt': INSTREAM size limit exceeded. ERROR", exception.Message); + } + + [Fact] + public async Task CreatingAsync_Throws_WhenHostIsMissing() + { + using var factory = CreateFactory(); + var handler = new ClamAvFileEventHandler( + Options.Create(new ClamAvOptions + { + Host = "", + }), + factory, + NullLogger.Instance); + + var exception = await Assert.ThrowsAsync(() => + handler.CreatingAsync(new FileCreatingContext("folder/test.txt"), new MemoryStream("hello world"u8.ToArray()))); + + Assert.Equal("The ClamAV antivirus scanner is enabled but the host setting is missing.", exception.Message); + } + + [Fact] + public async Task CreatingAsync_ReusesTheSameTcpConnection() + { + await using var server = await FakeClamAvServer.StartAsync("stream: OK\n", "stream: OK\n"); + using var factory = CreateFactory(); + var handler = CreateHandler(server.Port, factory); + + var firstResult = await handler.CreatingAsync(new FileCreatingContext("folder/first.txt"), new MemoryStream("first"u8.ToArray())); + var secondResult = await handler.CreatingAsync(new FileCreatingContext("folder/second.txt"), new MemoryStream("second"u8.ToArray())); + var requests = await server.Completion; + + Assert.True(firstResult.Succeeded); + Assert.True(secondResult.Succeeded); + Assert.Equal(1, server.ConnectionCount); + Assert.Equal("first"u8.ToArray(), requests[0].Payload); + Assert.Equal("second"u8.ToArray(), requests[1].Payload); + } + + private static ClamAvFileEventHandler CreateHandler(int port, ClamAvConnectionFactory factory) + => new( + Options.Create(new ClamAvOptions + { + Host = IPAddress.Loopback.ToString(), + Port = port, + ConnectTimeoutSeconds = 5, + TransferTimeoutSeconds = 5, + }), + factory, + NullLogger.Instance); + + private static ClamAvConnectionFactory CreateFactory() + => new(Mock.Of(l => l.ApplicationStopped == CancellationToken.None), NullLoggerFactory.Instance); + + private sealed class FakeClamAvServer : IAsyncDisposable + { + private readonly TcpListener _listener; + + private FakeClamAvServer(TcpListener listener, Task completion, Func connectionCount) + { + _listener = listener; + Completion = completion; + _connectionCount = connectionCount; + } + + private readonly Func _connectionCount; + + public int Port => ((IPEndPoint)_listener.LocalEndpoint).Port; + + public int ConnectionCount => _connectionCount(); + + public Task Completion { get; } + + public static Task StartAsync(params string[] responses) + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + + var connectionCount = 0; + var completion = Task.Run(async () => + { + var requests = new List(); + + using var client = await listener.AcceptTcpClientAsync(); + Interlocked.Increment(ref connectionCount); + + await using var networkStream = client.GetStream(); + + foreach (var response in responses) + { + var command = await ReadLineAsync(networkStream); + var payload = await ReadPayloadAsync(networkStream); + var responseBytes = Encoding.ASCII.GetBytes(response); + + requests.Add(new ClamAvRequest(command, payload)); + + await networkStream.WriteAsync(responseBytes); + await networkStream.FlushAsync(); + } + + return requests.ToArray(); + }); + + return Task.FromResult(new FakeClamAvServer(listener, completion, () => connectionCount)); + } + + public ValueTask DisposeAsync() + { + _listener.Stop(); + + return ValueTask.CompletedTask; + } + + private static async Task ReadLineAsync(NetworkStream stream) + { + using var commandStream = new MemoryStream(); + var buffer = new byte[1]; + + while (true) + { + var bytesRead = await stream.ReadAsync(buffer.AsMemory(0, 1)); + + if (bytesRead == 0) + { + break; + } + + commandStream.WriteByte(buffer[0]); + + if (buffer[0] == '\n') + { + break; + } + } + + return Encoding.ASCII.GetString(commandStream.ToArray()); + } + + private static async Task ReadPayloadAsync(Stream stream) + { + using var payloadStream = new MemoryStream(); + var lengthBuffer = new byte[sizeof(int)]; + + while (true) + { + await ReadExactlyAsync(stream, lengthBuffer); + + var chunkLength = BinaryPrimitives.ReadInt32BigEndian(lengthBuffer); + + if (chunkLength == 0) + { + break; + } + + var chunk = new byte[chunkLength]; + await ReadExactlyAsync(stream, chunk); + await payloadStream.WriteAsync(chunk); + } + + return payloadStream.ToArray(); + } + + private static async Task ReadExactlyAsync(Stream stream, byte[] buffer) + { + var offset = 0; + + while (offset < buffer.Length) + { + var bytesRead = await stream.ReadAsync(buffer.AsMemory(offset, buffer.Length - offset)); + + if (bytesRead == 0) + { + throw new EndOfStreamException(); + } + + offset += bytesRead; + } + } + } + + private sealed record ClamAvRequest(string Command, byte[] Payload); + + private sealed class NonSeekableReadStream : Stream + { + private readonly Stream _innerStream; + + public NonSeekableReadStream(Stream innerStream) + { + _innerStream = innerStream; + } + + public override bool CanRead => _innerStream.CanRead; + + public override bool CanSeek => false; + + public override bool CanWrite => false; + + public override long Length => throw new NotSupportedException(); + + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public override void Flush() => _innerStream.Flush(); + + public override int Read(byte[] buffer, int offset, int count) => _innerStream.Read(buffer, offset, count); + + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + public override void SetLength(long value) => throw new NotSupportedException(); + + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + + public override async ValueTask DisposeAsync() + { + await _innerStream.DisposeAsync(); + await base.DisposeAsync(); + } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + _innerStream.Dispose(); + } + + base.Dispose(disposing); + } + } +} diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs index fadfe7edbc1..9effc5712db 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.Localization; using OrchardCore.FileStorage; +using OrchardCore.Infrastructure; using OrchardCore.Media.Core; using OrchardCore.Media.Events; @@ -21,7 +23,7 @@ public void MapPathToPublicUrl_ReturnsUrlEncodedPath(string path, string expecte "", [], [], - new NullAntivirusScanner(), + CreateFileEventService(), Mock.Of>()); var result = store.MapPathToPublicUrl(path); @@ -40,7 +42,7 @@ public void MapPathToPublicUrl_WithCdnBaseUrl_ReturnsUrlEncodedPath(string cdnBa cdnBaseUrl, [], [], - new NullAntivirusScanner(), + CreateFileEventService(), Mock.Of>()); var result = store.MapPathToPublicUrl(path); @@ -59,13 +61,17 @@ public async Task CreateFileFromStreamAsync_NoHandlers_CallsFileStoreDirectly() .Setup(x => x.CreateFileFromStreamAsync("test.txt", inputStream, false)) .ReturnsAsync("result"); + fileStoreMock + .Setup(x => x.GetFileInfoAsync("result")) + .ReturnsAsync(Mock.Of()); + var store = new DefaultMediaFileStore( fileStoreMock.Object, "", "", new List(), new List(), - new NullAntivirusScanner(), + CreateFileEventService(), loggerMock.Object); // Act @@ -98,7 +104,7 @@ public async Task CreateFileFromStreamAsync_HandlerReturnsInputStream_PassesInpu "", new List(), new[] { handlerMock.Object }, - new NullAntivirusScanner(), + CreateFileEventService(), loggerMock.Object); // Act @@ -133,13 +139,17 @@ public async Task CreateFileFromStreamAsync_HandlersCreateNewStreams_PassesCorre .Setup(x => x.CreateFileFromStreamAsync("test.txt", stream2, false)) .ReturnsAsync("result"); + fileStoreMock + .Setup(x => x.GetFileInfoAsync("result")) + .ReturnsAsync(Mock.Of()); + var store = new DefaultMediaFileStore( fileStoreMock.Object, "", "", new List(), new[] { handler1.Object, handler2.Object }, - new NullAntivirusScanner(), + CreateFileEventService(), loggerMock.Object); // Act @@ -180,7 +190,7 @@ public async Task CreateFileFromStreamAsync_ValidateAvailableStorageAsync() "", [handler.Object], [], - new NullAntivirusScanner(), + CreateFileEventService(), loggerMock.Object); // Act @@ -196,22 +206,17 @@ public async Task CreateFileFromStreamAsync_ValidateAvailableStorageAsync() } [Fact] - public async Task CreateFileFromStreamAsync_ScannerResetsStreamBeforeSaving() + public async Task CreateFileFromStreamAsync_FileEventHandlerCanReplaceStreamBeforeSaving() { var fileStoreMock = new Mock(); var loggerMock = new Mock>(); - var scannerMock = new Mock(); - var inputStream = new MemoryStream("hello world"u8.ToArray()); + var inputStream = new MemoryStream("ignored"u8.ToArray()); + var replacementStream = new MemoryStream("hello world"u8.ToArray()); + var handlerMock = new Mock(); - scannerMock - .Setup(x => x.ScanAsync(It.IsAny(), inputStream)) - .ReturnsAsync(AntivirusResult.Clean) - .Callback((_, stream) => - { - while (stream.ReadByte() != -1) - { - } - }); + handlerMock + .Setup(x => x.CreatingAsync(It.IsAny(), inputStream, It.IsAny())) + .ReturnsAsync(FileCreatingResult.Success(replacementStream)); fileStoreMock .Setup(x => x.CreateFileFromStreamAsync("test.txt", It.IsAny(), false)) @@ -223,29 +228,37 @@ public async Task CreateFileFromStreamAsync_ScannerResetsStreamBeforeSaving() Assert.Equal("hello world"u8.ToArray(), copy.ToArray()); }); + fileStoreMock + .Setup(x => x.GetFileInfoAsync("result")) + .ReturnsAsync(Mock.Of()); + var store = new DefaultMediaFileStore( fileStoreMock.Object, "", "", [], [], - scannerMock.Object, + CreateFileEventService(handlerMock.Object), loggerMock.Object); await store.CreateFileFromStreamAsync("test.txt", inputStream); } [Fact] - public async Task CreateFileFromStreamAsync_ScannerRejectsUnsafeFile() + public async Task CreateFileFromStreamAsync_FileEventHandlerCanRejectFile() { var fileStoreMock = new Mock(MockBehavior.Strict); var loggerMock = new Mock>(); - var scannerMock = new Mock(); var inputStream = new MemoryStream("hello world"u8.ToArray()); + var handlerMock = new Mock(); - scannerMock - .Setup(x => x.ScanAsync(It.IsAny(), inputStream)) - .ReturnsAsync(AntivirusResult.Unsafe("The uploaded file was rejected by the anti-virus scanner.", "EICAR-Test-File")); + handlerMock + .Setup(x => x.CreatingAsync(It.IsAny(), inputStream, It.IsAny())) + .ReturnsAsync(FileCreatingResult.Failed(stream: null, + new ResultError + { + Message = new LocalizedString("Rejected", "The uploaded file was rejected by the anti-virus scanner."), + })); var store = new DefaultMediaFileStore( fileStoreMock.Object, @@ -253,46 +266,35 @@ public async Task CreateFileFromStreamAsync_ScannerRejectsUnsafeFile() "", [], [], - scannerMock.Object, + CreateFileEventService(handlerMock.Object), loggerMock.Object); - var exception = await Assert.ThrowsAsync(() => + var exception = await Assert.ThrowsAsync(() => store.CreateFileFromStreamAsync("test.txt", inputStream)); Assert.Equal("The uploaded file was rejected by the anti-virus scanner.", exception.Message); } [Fact] - public async Task CreateFileFromStreamAsync_ScannerBuffersNonSeekableStreams() + public async Task CreateFileFromStreamAsync_FileEventHandlersRunCreatedAfterTheFileIsStored() { var fileStoreMock = new Mock(); var loggerMock = new Mock>(); - var scannerMock = new Mock(); - using var baseStream = new MemoryStream("hello world"u8.ToArray()); - using var inputStream = new NonSeekableReadStream(baseStream); - - scannerMock - .Setup(x => x.ScanAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync(AntivirusResult.Clean) - .Callback((_, stream) => - { - Assert.True(stream.CanSeek); - while (stream.ReadByte() != -1) - { - } - }); + var inputStream = new MemoryStream("hello world"u8.ToArray()); + var fileInfoMock = new Mock(); + var handlerMock = new Mock(); + + handlerMock + .Setup(x => x.CreatingAsync(It.IsAny(), inputStream, It.IsAny())) + .ReturnsAsync(FileCreatingResult.Success(inputStream)); fileStoreMock - .Setup(x => x.CreateFileFromStreamAsync("test.txt", It.IsAny(), false)) - .ReturnsAsync("result") - .Callback((_, stream, _) => - { - Assert.True(stream.CanSeek); + .Setup(x => x.CreateFileFromStreamAsync("test.txt", inputStream, false)) + .ReturnsAsync("result"); - using var copy = new MemoryStream(); - stream.CopyTo(copy); - Assert.Equal("hello world"u8.ToArray(), copy.ToArray()); - }); + fileStoreMock + .Setup(x => x.GetFileInfoAsync("result")) + .ReturnsAsync(fileInfoMock.Object); var store = new DefaultMediaFileStore( fileStoreMock.Object, @@ -300,59 +302,14 @@ public async Task CreateFileFromStreamAsync_ScannerBuffersNonSeekableStreams() "", [], [], - scannerMock.Object, + CreateFileEventService(handlerMock.Object), loggerMock.Object); await store.CreateFileFromStreamAsync("test.txt", inputStream); - } - - private sealed class NonSeekableReadStream : Stream - { - private readonly Stream _innerStream; - - public NonSeekableReadStream(Stream innerStream) - { - _innerStream = innerStream; - } - - public override bool CanRead => _innerStream.CanRead; - - public override bool CanSeek => false; - public override bool CanWrite => false; - - public override long Length => throw new NotSupportedException(); - - public override long Position - { - get => throw new NotSupportedException(); - set => throw new NotSupportedException(); - } - - public override void Flush() => _innerStream.Flush(); - - public override int Read(byte[] buffer, int offset, int count) => _innerStream.Read(buffer, offset, count); - - public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); - - public override void SetLength(long value) => throw new NotSupportedException(); - - public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); - - public override async ValueTask DisposeAsync() - { - await _innerStream.DisposeAsync(); - await base.DisposeAsync(); - } - - protected override void Dispose(bool disposing) - { - if (disposing) - { - _innerStream.Dispose(); - } - - base.Dispose(disposing); - } + handlerMock.Verify(x => x.CreatedAsync(fileInfoMock.Object, It.IsAny()), Times.Once); } + + private static FileEventService CreateFileEventService(params IFileEventHandler[] handlers) + => new(handlers); } diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/FileEventServiceTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/FileEventServiceTests.cs new file mode 100644 index 00000000000..50d4acbb8ab --- /dev/null +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/FileEventServiceTests.cs @@ -0,0 +1,81 @@ +using OrchardCore.FileStorage; +using OrchardCore.Infrastructure; + +namespace OrchardCore.Tests.Modules.OrchardCore.Media; + +public class FileEventServiceTests +{ + [Fact] + public async Task ProcessAsync_DisposesReplacementStream_WhenLaterHandlerThrows() + { + var originalStream = new TrackingStream(); + var replacementStream = new TrackingStream(); + + var firstHandler = new Mock(); + firstHandler + .Setup(x => x.CreatingAsync(It.IsAny(), originalStream, It.IsAny())) + .ReturnsAsync(FileCreatingResult.Success(replacementStream)); + + var secondHandler = new Mock(); + secondHandler + .Setup(x => x.CreatingAsync(It.IsAny(), replacementStream, It.IsAny())) + .ThrowsAsync(new InvalidOperationException("Handler failure.")); + + var service = new FileEventService([firstHandler.Object, secondHandler.Object]); + + await Assert.ThrowsAsync(() => + service.ProcessAsync(new FileCreatingContext("file.txt"), originalStream)); + + Assert.True(replacementStream.IsDisposed); + Assert.False(originalStream.IsDisposed); + } + + [Fact] + public async Task ProcessAsync_ReturnedFailedResult_DisposesReplacementStream() + { + var originalStream = new TrackingStream(); + var replacementStream = new TrackingStream(); + + var firstHandler = new Mock(); + firstHandler + .Setup(x => x.CreatingAsync(It.IsAny(), originalStream, It.IsAny())) + .ReturnsAsync(FileCreatingResult.Success(replacementStream)); + + var secondHandler = new Mock(); + secondHandler + .Setup(x => x.CreatingAsync(It.IsAny(), replacementStream, It.IsAny())) + .ReturnsAsync(FileCreatingResult.Failed(replacementStream, new ResultError + { + Message = new LocalizedString("Rejected", "The file was rejected."), + })); + + var service = new FileEventService([firstHandler.Object, secondHandler.Object]); + + await using (var result = await service.ProcessAsync(new FileCreatingContext("file.txt"), originalStream, TestContext.Current.CancellationToken)) + { + Assert.False(result.Succeeded); + Assert.Same(replacementStream, result.Stream); + Assert.False(replacementStream.IsDisposed); + } + + Assert.True(replacementStream.IsDisposed); + Assert.False(originalStream.IsDisposed); + } + + private sealed class TrackingStream : MemoryStream + { + public bool IsDisposed { get; private set; } + + protected override void Dispose(bool disposing) + { + IsDisposed = true; + base.Dispose(disposing); + } + + public override async ValueTask DisposeAsync() + { + IsDisposed = true; + await base.DisposeAsync(); + } + } +} diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs index e22ec10b9f1..6f0fb52d9ca 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs @@ -46,7 +46,7 @@ public async Task ShouldProcess(string cdnBaseUrl, string text, string expected) cdnBaseUrl, [], [], - new NullAntivirusScanner(), + new FileEventService([]), Mock.Of>()); var fileVersionProvider = Mock.Of(); diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs index f4be3797a8c..fdcc30e0584 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs @@ -43,7 +43,7 @@ private static TestOrchardHelper CreateOrchardHelper() "", [], [], - new NullAntivirusScanner(), + new FileEventService([]), Mock.Of>()); var mediaProfileServiceMock = new Mock(); diff --git a/test/OrchardCore.Tests/OrchardCore.Tests.csproj b/test/OrchardCore.Tests/OrchardCore.Tests.csproj index 0b898c0598e..d1dc44fcfc2 100644 --- a/test/OrchardCore.Tests/OrchardCore.Tests.csproj +++ b/test/OrchardCore.Tests/OrchardCore.Tests.csproj @@ -52,7 +52,7 @@ - + From 7816779d0fc7222d11790fe17cc5a1b7c2d37345 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 4 Jun 2026 23:00:44 -0700 Subject: [PATCH 7/8] rename service and fix CI --- mkdocs.yml | 3 +- .../ImportRemoteInstanceController.cs | 8 ++--- .../OrchardCore.Deployment.Remote/Startup.cs | 2 +- .../Controllers/ImportController.cs | 8 ++--- .../OrchardCore.Deployment/Startup.cs | 2 +- .../OrchardCore.Media.AmazonS3/Startup.cs | 4 +-- .../OrchardCore.Media.Azure/Startup.cs | 4 +-- .../OrchardCore.Media/Startup.cs | 6 ++-- ...EventService.cs => FileCreationService.cs} | 6 ++-- .../DefaultMediaFileStore.cs | 10 +++--- src/docs/reference/README.md | 1 + .../reference/core/file-upload-security.md | 32 +++++++++---------- .../modules/{ClamAV => Antivirus}/README.md | 18 +++++++---- src/docs/releases/3.0.0.md | 4 +-- .../AssetUrlShortcodeTests.cs | 2 +- .../DefaultMediaFileStoreTests.cs | 20 ++++++------ ...ceTests.cs => FileCreationServiceTests.cs} | 14 ++++---- .../OrchardCore.Media/ImageShortcodeTests.cs | 2 +- .../MediaOrchardHelperExtensionsTests.cs | 2 +- 19 files changed, 77 insertions(+), 71 deletions(-) rename src/OrchardCore/OrchardCore.FileStorage.Abstractions/{FileEventService.cs => FileCreationService.cs} (93%) rename src/docs/reference/modules/{ClamAV => Antivirus}/README.md (77%) rename test/OrchardCore.Tests/Modules/OrchardCore.Media/{FileEventServiceTests.cs => FileCreationServiceTests.cs} (79%) diff --git a/mkdocs.yml b/mkdocs.yml index 68772d75311..073938aa3d8 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -209,7 +209,7 @@ nav: - Media: - Media: reference/modules/Media/README.md - Media Slugify: reference/modules/Media.Slugify/README.md - - ClamAV Antivirus Scanner: reference/modules/Antivirus.ClamAV/README.md + - Antivirus: reference/modules/Antivirus/README.md - Media Amazon S3: reference/modules/Media.AmazonS3/README.md - Media Azure: reference/modules/Media.Azure/README.md - ReCaptcha: reference/modules/ReCaptcha/README.md @@ -253,6 +253,7 @@ nav: - Audit Trail: reference/modules/AuditTrail/README.md - Auto Setup: reference/modules/AutoSetup/README.md - Features: reference/modules/Features/README.md + - File Upload Security: reference/core/file-upload-security.md - Contents: reference/modules/Contents/README.md - Configuration: reference/modules/Configuration/README.md - Cors: reference/modules/Cors/README.md diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs index d08e4c07c31..db05a20ff89 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Controllers/ImportRemoteInstanceController.cs @@ -23,7 +23,7 @@ public sealed class ImportRemoteInstanceController : Controller private readonly INotifier _notifier; private readonly ILogger _logger; private readonly IDataProtector _dataProtector; - private readonly FileEventService _fileEventService; + private readonly FileCreationService _fileCreationService; internal readonly IHtmlLocalizer H; internal readonly IStringLocalizer S; @@ -32,14 +32,14 @@ public ImportRemoteInstanceController( IDataProtectionProvider dataProtectionProvider, RemoteClientService remoteClientService, IDeploymentManager deploymentManager, - FileEventService fileEventService, + FileCreationService fileCreationService, INotifier notifier, IHtmlLocalizer htmlLocalizer, IStringLocalizer stringLocalizer, ILogger logger) { _deploymentManager = deploymentManager; - _fileEventService = fileEventService; + _fileCreationService = fileCreationService; _notifier = notifier; _logger = logger; _remoteClientService = remoteClientService; @@ -81,7 +81,7 @@ public async Task Import(ImportViewModel model) try { await using var uploadedStream = model.Content.OpenReadStream(); - await using var fileCreatingResult = await _fileEventService.ProcessAsync( + await using var fileCreatingResult = await _fileCreationService.CreateAsync( new FileCreatingContext(model.Content.FileName, model.Content.Length, model.Content.ContentType), uploadedStream, HttpContext.RequestAborted); diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs index 2b24a917870..5eb35ca29de 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment.Remote/Startup.cs @@ -14,7 +14,7 @@ public sealed class Startup : StartupBase public override void ConfigureServices(IServiceCollection services) { services.AddHttpClient(); - services.TryAddSingleton(); + services.TryAddSingleton(); services.AddNavigationProvider(); services.AddScoped(); diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs b/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs index 51327fbf9e6..314a8909c47 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment/Controllers/ImportController.cs @@ -24,7 +24,7 @@ public sealed class ImportController : Controller private readonly IAuthorizationService _authorizationService; private readonly INotifier _notifier; private readonly ILogger _logger; - private readonly FileEventService _fileEventService; + private readonly FileCreationService _fileCreationService; internal readonly IHtmlLocalizer H; internal readonly IStringLocalizer S; @@ -32,7 +32,7 @@ public sealed class ImportController : Controller public ImportController( IDeploymentManager deploymentManager, IAuthorizationService authorizationService, - FileEventService fileEventService, + FileCreationService fileCreationService, INotifier notifier, ILogger logger, IHtmlLocalizer htmlLocalizer, @@ -41,7 +41,7 @@ IStringLocalizer stringLocalizer { _deploymentManager = deploymentManager; _authorizationService = authorizationService; - _fileEventService = fileEventService; + _fileCreationService = fileCreationService; _notifier = notifier; _logger = logger; H = htmlLocalizer; @@ -74,7 +74,7 @@ public async Task Import(IFormFile importedPackage) try { await using var uploadedStream = importedPackage.OpenReadStream(); - await using var fileCreatingResult = await _fileEventService.ProcessAsync( + await using var fileCreatingResult = await _fileCreationService.CreateAsync( new FileCreatingContext(importedPackage.FileName, importedPackage.Length, importedPackage.ContentType), uploadedStream, HttpContext.RequestAborted); diff --git a/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs index 24164c201b2..29c0dbf1ee6 100644 --- a/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Deployment/Startup.cs @@ -22,7 +22,7 @@ public sealed class Startup : StartupBase { public override void ConfigureServices(IServiceCollection services) { - services.TryAddSingleton(); + services.TryAddSingleton(); services.AddDeploymentServices(); services.AddNavigationProvider(); diff --git a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs index 87e7b5da82a..9e9fa81adac 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs @@ -103,7 +103,7 @@ public override void ConfigureServices(IServiceCollection services) var mediaOptions = serviceProvider.GetRequiredService>().Value; var mediaEventHandlers = serviceProvider.GetServices(); var mediaCreatingEventHandlers = serviceProvider.GetServices(); - var fileEventService = serviceProvider.GetRequiredService(); + var fileCreationService = serviceProvider.GetRequiredService(); var clock = serviceProvider.GetRequiredService(); var logger = serviceProvider.GetRequiredService>(); var amazonS3Client = serviceProvider.GetService(); @@ -127,7 +127,7 @@ public override void ConfigureServices(IServiceCollection services) mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, - fileEventService, + fileCreationService, logger); })); diff --git a/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs index 8ace11ab081..26b209dc41a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs @@ -95,7 +95,7 @@ public override void ConfigureServices(IServiceCollection services) var contentTypeProvider = serviceProvider.GetRequiredService(); var mediaEventHandlers = serviceProvider.GetServices(); var mediaCreatingEventHandlers = serviceProvider.GetServices(); - var fileEventService = serviceProvider.GetRequiredService(); + var fileCreationService = serviceProvider.GetRequiredService(); var logger = serviceProvider.GetRequiredService>(); var fileStore = new BlobFileStore(blobStorageOptions, clock, contentTypeProvider); @@ -110,7 +110,7 @@ public override void ConfigureServices(IServiceCollection services) mediaUrlBase = fileStore.Combine(originalPathBase.Value, mediaUrlBase); } - return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileEventService, logger); + return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileCreationService, logger); })); services.AddSingleton(); diff --git a/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs index 2d3501cadb8..77ca434566a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media/Startup.cs @@ -87,7 +87,7 @@ public override void ConfigureServices(IServiceCollection services) services.AddResourceConfiguration(); services.AddTransient, MediaOptionsConfiguration>(); - services.TryAddSingleton(); + services.TryAddSingleton(); services.AddSingleton(serviceProvider => { @@ -116,7 +116,7 @@ public override void ConfigureServices(IServiceCollection services) var mediaOptions = serviceProvider.GetRequiredService>().Value; var mediaEventHandlers = serviceProvider.GetServices(); var mediaCreatingEventHandlers = serviceProvider.GetServices(); - var fileEventService = serviceProvider.GetRequiredService(); + var fileCreationService = serviceProvider.GetRequiredService(); var fileSystemStoreLogger = serviceProvider.GetRequiredService>(); var defaultMediaFileStoreLogger = serviceProvider.GetRequiredService>(); @@ -134,7 +134,7 @@ public override void ConfigureServices(IServiceCollection services) mediaUrlBase = fileStore.Combine(originalPathBase.Value, mediaUrlBase); } - return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileEventService, defaultMediaFileStoreLogger); + return new DefaultMediaFileStore(fileStore, mediaUrlBase, mediaOptions.CdnBaseUrl, mediaEventHandlers, mediaCreatingEventHandlers, fileCreationService, defaultMediaFileStoreLogger); }); services.AddPermissionProvider(); diff --git a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileEventService.cs b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreationService.cs similarity index 93% rename from src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileEventService.cs rename to src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreationService.cs index 2958d1477a0..e1ada6d0e38 100644 --- a/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileEventService.cs +++ b/src/OrchardCore/OrchardCore.FileStorage.Abstractions/FileCreationService.cs @@ -3,11 +3,11 @@ namespace OrchardCore.FileStorage; /// /// Coordinates instances for user-uploaded files. /// -public sealed class FileEventService +public sealed class FileCreationService { private readonly IEnumerable _handlers; - public FileEventService(IEnumerable handlers) + public FileCreationService(IEnumerable handlers) { _handlers = handlers; } @@ -17,7 +17,7 @@ public FileEventService(IEnumerable handlers) /// The caller owns the original and should dispose the returned /// to clean up any replacement stream created by handlers. /// - public async Task ProcessAsync( + public async Task CreateAsync( FileCreatingContext context, Stream stream, CancellationToken cancellationToken = default) diff --git a/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs b/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs index bb932000da3..f607320b0be 100644 --- a/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs +++ b/src/OrchardCore/OrchardCore.Media.Core/DefaultMediaFileStore.cs @@ -17,7 +17,7 @@ public class DefaultMediaFileStore : IMediaFileStore private readonly string _cdnBaseUrl; private readonly IEnumerable _mediaEventHandlers; private readonly IEnumerable _mediaCreatingEventHandlers; - private readonly FileEventService _fileEventService; + private readonly FileCreationService _fileCreationService; private readonly ILogger _logger; private bool _requestBasePathValidated; @@ -28,7 +28,7 @@ public DefaultMediaFileStore( string cdnBaseUrl, IEnumerable mediaEventHandlers, IEnumerable mediaCreatingEventHandlers, - FileEventService fileEventService, + FileCreationService fileCreationService, ILogger logger) { _fileStore = fileStore; @@ -41,7 +41,7 @@ public DefaultMediaFileStore( _mediaEventHandlers = mediaEventHandlers; _mediaCreatingEventHandlers = mediaCreatingEventHandlers; - _fileEventService = fileEventService; + _fileCreationService = fileCreationService; _logger = logger; } @@ -264,7 +264,7 @@ private async Task ValidateAvailableStorageAsync(long requiredStorageSpace) private async Task CreateFileAsync(FileCreatingContext fileCreatingContext, Stream stream, bool overwrite) { - await using var fileCreatingResult = await _fileEventService.ProcessAsync(fileCreatingContext, stream); + await using var fileCreatingResult = await _fileCreationService.CreateAsync(fileCreatingContext, stream); if (!fileCreatingResult.Succeeded) { @@ -276,7 +276,7 @@ private async Task CreateFileAsync(FileCreatingContext fileCreatingConte var createdPath = await _fileStore.CreateFileFromStreamAsync(fileCreatingContext.Path, fileCreatingResult.Stream, overwrite); var fileInfo = await _fileStore.GetFileInfoAsync(createdPath); - await _fileEventService.CreatedAsync(fileInfo); + await _fileCreationService.CreatedAsync(fileInfo); return createdPath; } diff --git a/src/docs/reference/README.md b/src/docs/reference/README.md index f6994f1770a..bdfecaa43a0 100644 --- a/src/docs/reference/README.md +++ b/src/docs/reference/README.md @@ -70,6 +70,7 @@ Here's a categorized overview of all built-in Orchard Core features at a glance. - Media: - [Media](modules/Media/README.md) - [Media Slugify](modules/Media.Slugify/README.md) + - [Antivirus](modules/Antivirus/README.md) - [Media Amazon S3](modules/Media.AmazonS3/README.md) - [Media Azure](modules/Media.Azure/README.md) - [XML-RPC](modules/XmlRpc/README.md) diff --git a/src/docs/reference/core/file-upload-security.md b/src/docs/reference/core/file-upload-security.md index 5a2910c9760..38e4c9b53c5 100644 --- a/src/docs/reference/core/file-upload-security.md +++ b/src/docs/reference/core/file-upload-security.md @@ -1,23 +1,23 @@ # File Upload Security -When your feature accepts a user-uploaded file and stores it outside Orchard Core's built-in media flow, use `FileEventService` to run the shared pre-storage security pipeline. +When your feature accepts a user-uploaded file and stores it outside Orchard Core's built-in media flow, use `FileCreationService` to run the shared pre-storage security pipeline. This ensures every `IFileEventHandler` can inspect or replace the stream before the file is written permanently. If any handler returns a failed `FileCreatingResult`, the upload must be aborted and the file must not be stored. -## When to use `FileEventService` +## When to use `FileCreationService` -Use `FileEventService` in custom controllers, admin endpoints, APIs, recipe importers, or background flows that: +Use `FileCreationService` in custom controllers, admin endpoints, APIs, recipe importers, or background flows that: - accept a file from a user; - write that file to disk, cloud storage, or another permanent store; and - do **not** already go through `IMediaFileStore.CreateFileFromStreamAsync()`. !!! note - `DefaultMediaFileStore` already uses `FileEventService` internally. If your code uploads through `IMediaFileStore`, do not call the service a second time. + `DefaultMediaFileStore` already uses `FileCreationService` internally. If your code uploads through `IMediaFileStore`, do not call the service a second time. ## Core types -- `FileEventService`: runs `CreatingAsync()` before storage and `CreatedAsync()` after storage succeeds. +- `FileCreationService`: runs `CreatingAsync()` before storage and `CreatedAsync()` after storage succeeds. - `FileCreatingContext`: describes the file being processed. - `FileCreatingResult`: returns the stream that should continue through the pipeline and whether creation should proceed. - `IFileEventHandler`: participates in the upload pipeline. @@ -25,16 +25,16 @@ Use `FileEventService` in custom controllers, admin endpoints, APIs, recipe impo ## Ownership and cleanup - The original upload stream stays owned by the caller. -- If a handler replaces the stream, `FileEventService` disposes the superseded intermediate stream. -- The `FileCreatingResult` returned from `ProcessAsync()` owns the final replacement stream when one was created, so callers should use `await using` and keep that result alive for as long as they need the processed stream. -- If `ProcessAsync()` returns a failed result, dispose that result the same way and abort the upload without storing the file. +- If a handler replaces the stream, `FileCreationService` disposes the superseded intermediate stream. +- The `FileCreatingResult` returned from `CreateAsync()` owns the final replacement stream when one was created, so callers should use `await using` and keep that result alive for as long as they need the processed stream. +- If `CreateAsync()` returns a failed result, dispose that result the same way and abort the upload without storing the file. - After the file has been written permanently, disposing `FileCreatingResult` cleans up any temporary replacement stream used during the upload pipeline. Because cleanup is modeled through stream disposal, handlers normally do **not** need a separate `FailedAsync()` callback. A handler that creates a temporary stream should return that stream and let the service/result lifecycle clean it up. -## Using `FileEventService` +## Using `FileCreationService` -Call `ProcessAsync()` before writing the file. If the returned result did not succeed, abort the request. Only call `CreatedAsync()` after the file was stored successfully. +Call `CreateAsync()` before writing the file. If the returned result did not succeed, abort the request. Only call `CreatedAsync()` after the file was stored successfully. ```csharp using Microsoft.AspNetCore.Http; @@ -42,21 +42,21 @@ using OrchardCore.FileStorage; public sealed class CustomUploadService { - private readonly FileEventService _fileEventService; + private readonly FileCreationService _fileCreationService; private readonly IFileStore _fileStore; public CustomUploadService( - FileEventService fileEventService, + FileCreationService fileCreationService, IFileStore fileStore) { - _fileEventService = fileEventService; + _fileCreationService = fileCreationService; _fileStore = fileStore; } public async Task UploadAsync(IFormFile file, CancellationToken cancellationToken) { await using var uploadedStream = file.OpenReadStream(); - await using var fileCreatingResult = await _fileEventService.ProcessAsync( + await using var fileCreatingResult = await _fileCreationService.CreateAsync( new FileCreatingContext(file.FileName, file.Length, file.ContentType), uploadedStream, cancellationToken); @@ -70,7 +70,7 @@ public sealed class CustomUploadService var path = await _fileStore.CreateFileFromStreamAsync(file.FileName, fileCreatingResult.Stream); var fileInfo = await _fileStore.GetFileInfoAsync(path); - await _fileEventService.CreatedAsync(fileInfo, cancellationToken); + await _fileCreationService.CreatedAsync(fileInfo, cancellationToken); return path; } @@ -110,4 +110,4 @@ public sealed class RejectExecutableFileEventHandler : IFileEventHandler ## Security guidance -Always run `FileEventService` before any permanent write. Do not save the uploaded file first and scan it afterward, since a failed scan must abort the upload before the file is persisted. +Always run `FileCreationService` before any permanent write. Do not save the uploaded file first and scan it afterward, since a failed scan must abort the upload before the file is persisted. diff --git a/src/docs/reference/modules/ClamAV/README.md b/src/docs/reference/modules/Antivirus/README.md similarity index 77% rename from src/docs/reference/modules/ClamAV/README.md rename to src/docs/reference/modules/Antivirus/README.md index 69a197f2451..1a5056fffa1 100644 --- a/src/docs/reference/modules/ClamAV/README.md +++ b/src/docs/reference/modules/Antivirus/README.md @@ -1,6 +1,10 @@ -# ClamAV (`OrchardCore.Antivirus.ClamAV`) +# Antivirus (`OrchardCore.Antivirus`) -The ClamAV module scans files with a `clamd` service before Orchard Core stores or imports them. +The Antivirus module provides features that inspect uploaded files before Orchard Core stores or imports them permanently. + +## ClamAV feature (`OrchardCore.Antivirus.ClamAV`) + +The ClamAV feature scans files with a `clamd` service before Orchard Core stores or imports them. When the feature is enabled, uploads fail closed: @@ -8,9 +12,9 @@ When the feature is enabled, uploads fail closed: - Scanner connectivity, timeout, or protocol failures also reject the upload. - The ClamAV connection is reused per configuration to avoid creating a new TCP client for every scan. -The scanner is wired through Orchard Core's file event handling abstractions, so uploads can be validated before storage without coupling media and deployment flows to a scanner-specific interface. ClamAV participates in that flow as an `IFileEventHandler`, and `FileEventService` aborts the upload when ClamAV returns a failed `FileCreatingResult`. +The scanner is wired through Orchard Core's file event handling abstractions, so uploads can be validated before storage without coupling media and deployment flows to a scanner-specific interface. ClamAV participates in that flow as an `IFileEventHandler`, and `FileCreationService` aborts the upload when ClamAV returns a failed `FileCreatingResult`. -## Configuration +### Configuration Configure the ClamAV connection in application configuration. The settings key remains `OrchardCore_Antivirus_ClamAV` for compatibility: @@ -36,7 +40,7 @@ OrchardCore__Antivirus_ClamAV__ConnectTimeoutSeconds=5 OrchardCore__Antivirus_ClamAV__TransferTimeoutSeconds=30 ``` -## Usage +### Usage 1. Configure the ClamAV settings. 2. Enable the `ClamAV Antivirus Scanner` feature (`OrchardCore.Antivirus.ClamAV`). @@ -46,9 +50,9 @@ If the feature is enabled without a valid ClamAV connection, uploads are rejecte This feature integrates with the shared file upload security pipeline through `IFileEventHandler`, so uploads can be rejected before Orchard Core stores them permanently. -See [File Upload Security](../../core/file-upload-security.md) for the canonical guidance on invoking `FileEventService` in custom upload flows and aborting rejected files before they are stored. +See [File Upload Security](../../core/file-upload-security.md) for the canonical guidance on invoking `FileCreationService` in custom upload flows and aborting rejected files before they are stored. -## Notes +### Notes - The currently audited upload surfaces covered by this change are media uploads plus deployment package imports, both local and remote. - Media uploads are scanned before storage because they flow through `DefaultMediaFileStore`. diff --git a/src/docs/releases/3.0.0.md b/src/docs/releases/3.0.0.md index 9a9af354c9b..5a46c06afbd 100644 --- a/src/docs/releases/3.0.0.md +++ b/src/docs/releases/3.0.0.md @@ -94,9 +94,9 @@ Orchard Core now provides a file upload event pipeline for securing user-uploade - `IFileEventHandler` - `FileCreatingContext` - `FileCreatingResult` -- `FileEventService` +- `FileCreationService` -Use `FileEventService.ProcessAsync()` anywhere your code accepts a user-uploaded file and stores it directly. If the returned `FileCreatingResult.Succeeded` value is `false`, you must abort the upload and avoid persisting the file. Only call `FileEventService.CreatedAsync()` after the file was stored successfully. +Use `FileCreationService.CreateAsync()` anywhere your code accepts a user-uploaded file and stores it directly. If the returned `FileCreatingResult.Succeeded` value is `false`, you must abort the upload and avoid persisting the file. Only call `FileCreationService.CreatedAsync()` after the file was stored successfully. The ClamAV integration now uses this event pipeline through the `OrchardCore.Antivirus.ClamAV` feature. diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs index b759287db7a..b03d98f31e4 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/AssetUrlShortcodeTests.cs @@ -35,7 +35,7 @@ public async Task ShouldProcess(string cdnBaseUrl, string text, string expected) cdnBaseUrl, [], [], - new FileEventService([]), + new FileCreationService([]), Mock.Of>()); var defaultHttpContext = new DefaultHttpContext(); diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs index 9effc5712db..f992e7ac42e 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/DefaultMediaFileStoreTests.cs @@ -23,7 +23,7 @@ public void MapPathToPublicUrl_ReturnsUrlEncodedPath(string path, string expecte "", [], [], - CreateFileEventService(), + CreateFileCreationService(), Mock.Of>()); var result = store.MapPathToPublicUrl(path); @@ -42,7 +42,7 @@ public void MapPathToPublicUrl_WithCdnBaseUrl_ReturnsUrlEncodedPath(string cdnBa cdnBaseUrl, [], [], - CreateFileEventService(), + CreateFileCreationService(), Mock.Of>()); var result = store.MapPathToPublicUrl(path); @@ -71,7 +71,7 @@ public async Task CreateFileFromStreamAsync_NoHandlers_CallsFileStoreDirectly() "", new List(), new List(), - CreateFileEventService(), + CreateFileCreationService(), loggerMock.Object); // Act @@ -104,7 +104,7 @@ public async Task CreateFileFromStreamAsync_HandlerReturnsInputStream_PassesInpu "", new List(), new[] { handlerMock.Object }, - CreateFileEventService(), + CreateFileCreationService(), loggerMock.Object); // Act @@ -149,7 +149,7 @@ public async Task CreateFileFromStreamAsync_HandlersCreateNewStreams_PassesCorre "", new List(), new[] { handler1.Object, handler2.Object }, - CreateFileEventService(), + CreateFileCreationService(), loggerMock.Object); // Act @@ -190,7 +190,7 @@ public async Task CreateFileFromStreamAsync_ValidateAvailableStorageAsync() "", [handler.Object], [], - CreateFileEventService(), + CreateFileCreationService(), loggerMock.Object); // Act @@ -238,7 +238,7 @@ public async Task CreateFileFromStreamAsync_FileEventHandlerCanReplaceStreamBefo "", [], [], - CreateFileEventService(handlerMock.Object), + CreateFileCreationService(handlerMock.Object), loggerMock.Object); await store.CreateFileFromStreamAsync("test.txt", inputStream); @@ -266,7 +266,7 @@ public async Task CreateFileFromStreamAsync_FileEventHandlerCanRejectFile() "", [], [], - CreateFileEventService(handlerMock.Object), + CreateFileCreationService(handlerMock.Object), loggerMock.Object); var exception = await Assert.ThrowsAsync(() => @@ -302,7 +302,7 @@ public async Task CreateFileFromStreamAsync_FileEventHandlersRunCreatedAfterTheF "", [], [], - CreateFileEventService(handlerMock.Object), + CreateFileCreationService(handlerMock.Object), loggerMock.Object); await store.CreateFileFromStreamAsync("test.txt", inputStream); @@ -310,6 +310,6 @@ public async Task CreateFileFromStreamAsync_FileEventHandlersRunCreatedAfterTheF handlerMock.Verify(x => x.CreatedAsync(fileInfoMock.Object, It.IsAny()), Times.Once); } - private static FileEventService CreateFileEventService(params IFileEventHandler[] handlers) + private static FileCreationService CreateFileCreationService(params IFileEventHandler[] handlers) => new(handlers); } diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/FileEventServiceTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/FileCreationServiceTests.cs similarity index 79% rename from test/OrchardCore.Tests/Modules/OrchardCore.Media/FileEventServiceTests.cs rename to test/OrchardCore.Tests/Modules/OrchardCore.Media/FileCreationServiceTests.cs index 50d4acbb8ab..5502af85310 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/FileEventServiceTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/FileCreationServiceTests.cs @@ -3,10 +3,10 @@ namespace OrchardCore.Tests.Modules.OrchardCore.Media; -public class FileEventServiceTests +public class FileCreationServiceTests { [Fact] - public async Task ProcessAsync_DisposesReplacementStream_WhenLaterHandlerThrows() + public async Task CreateAsync_DisposesReplacementStream_WhenLaterHandlerThrows() { var originalStream = new TrackingStream(); var replacementStream = new TrackingStream(); @@ -21,17 +21,17 @@ public async Task ProcessAsync_DisposesReplacementStream_WhenLaterHandlerThrows( .Setup(x => x.CreatingAsync(It.IsAny(), replacementStream, It.IsAny())) .ThrowsAsync(new InvalidOperationException("Handler failure.")); - var service = new FileEventService([firstHandler.Object, secondHandler.Object]); + var service = new FileCreationService([firstHandler.Object, secondHandler.Object]); await Assert.ThrowsAsync(() => - service.ProcessAsync(new FileCreatingContext("file.txt"), originalStream)); + service.CreateAsync(new FileCreatingContext("file.txt"), originalStream)); Assert.True(replacementStream.IsDisposed); Assert.False(originalStream.IsDisposed); } [Fact] - public async Task ProcessAsync_ReturnedFailedResult_DisposesReplacementStream() + public async Task CreateAsync_ReturnedFailedResult_DisposesReplacementStream() { var originalStream = new TrackingStream(); var replacementStream = new TrackingStream(); @@ -49,9 +49,9 @@ public async Task ProcessAsync_ReturnedFailedResult_DisposesReplacementStream() Message = new LocalizedString("Rejected", "The file was rejected."), })); - var service = new FileEventService([firstHandler.Object, secondHandler.Object]); + var service = new FileCreationService([firstHandler.Object, secondHandler.Object]); - await using (var result = await service.ProcessAsync(new FileCreatingContext("file.txt"), originalStream, TestContext.Current.CancellationToken)) + await using (var result = await service.CreateAsync(new FileCreatingContext("file.txt"), originalStream, TestContext.Current.CancellationToken)) { Assert.False(result.Succeeded); Assert.Same(replacementStream, result.Stream); diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs index 6f0fb52d9ca..09d7488a639 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/ImageShortcodeTests.cs @@ -46,7 +46,7 @@ public async Task ShouldProcess(string cdnBaseUrl, string text, string expected) cdnBaseUrl, [], [], - new FileEventService([]), + new FileCreationService([]), Mock.Of>()); var fileVersionProvider = Mock.Of(); diff --git a/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs b/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs index fdcc30e0584..7c774cfce9f 100644 --- a/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs +++ b/test/OrchardCore.Tests/Modules/OrchardCore.Media/MediaOrchardHelperExtensionsTests.cs @@ -43,7 +43,7 @@ private static TestOrchardHelper CreateOrchardHelper() "", [], [], - new FileEventService([]), + new FileCreationService([]), Mock.Of>()); var mediaProfileServiceMock = new Mock(); From 1f68e25da8da4dcfceae9b442b6261c515e7ed5e Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 4 Jun 2026 23:07:03 -0700 Subject: [PATCH 8/8] cleanup --- src/docs/reference/core/file-upload-security.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/docs/reference/core/file-upload-security.md b/src/docs/reference/core/file-upload-security.md index 38e4c9b53c5..80a86f7acee 100644 --- a/src/docs/reference/core/file-upload-security.md +++ b/src/docs/reference/core/file-upload-security.md @@ -30,8 +30,6 @@ Use `FileCreationService` in custom controllers, admin endpoints, APIs, recipe i - If `CreateAsync()` returns a failed result, dispose that result the same way and abort the upload without storing the file. - After the file has been written permanently, disposing `FileCreatingResult` cleans up any temporary replacement stream used during the upload pipeline. -Because cleanup is modeled through stream disposal, handlers normally do **not** need a separate `FailedAsync()` callback. A handler that creates a temporary stream should return that stream and let the service/result lifecycle clean it up. - ## Using `FileCreationService` Call `CreateAsync()` before writing the file. If the returned result did not succeed, abort the request. Only call `CreatedAsync()` after the file was stored successfully.