diff --git a/test/CMS/DownloadZipSecurityTests.cs b/test/CMS/DownloadZipSecurityTests.cs new file mode 100644 index 000000000..93bd48b35 --- /dev/null +++ b/test/CMS/DownloadZipSecurityTests.cs @@ -0,0 +1,344 @@ +using CmsData = Viper.Areas.CMS.Services.CmsFilePathSafety; + +namespace Viper.test.CMS; + +/// +/// Security tests for the helpers that back CMS.DownloadZip. +/// Covers the filename sanitizer used for the Content-Disposition header +/// and the per-request temp-archive path builder (VPR-138). +/// +public sealed class DownloadZipSecurityTests +{ + #region SanitizeDownloadName + + [Theory] + [InlineData(@"\..\..\evil.zip")] + [InlineData("../../evil.zip")] + [InlineData(@"C:\Windows\Temp\evil.zip")] + [InlineData("../evil")] + public void SanitizeDownloadName_StripsPathSeparatorsAndTraversal(string input) + { + var result = CmsData.SanitizeDownloadName(input); + + Assert.DoesNotContain('\\', result); + Assert.DoesNotContain('/', result); + Assert.DoesNotContain("..", result); + Assert.EndsWith(".zip", result, StringComparison.OrdinalIgnoreCase); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("!!!")] + [InlineData("///")] + [InlineData(@"\\\")] + [InlineData(".")] + [InlineData("..")] + [InlineData("...")] + [InlineData(". .")] + public void SanitizeDownloadName_NullEmptyOrJunk_ReturnsDefault(string? input) + { + var result = CmsData.SanitizeDownloadName(input); + + Assert.Equal("FileDownload.zip", result); + } + + [Fact] + public void SanitizeDownloadName_NameWithoutExtension_AppendsZip() + { + var result = CmsData.SanitizeDownloadName("export"); + + Assert.Equal("export.zip", result); + } + + [Fact] + public void SanitizeDownloadName_NonZipExtension_ForcesZipSuffix() + { + // Documented choice: preserve the original name and append .zip so the + // response MIME (application/zip) always matches the filename. + var result = CmsData.SanitizeDownloadName("export.exe"); + + Assert.EndsWith(".zip", result, StringComparison.OrdinalIgnoreCase); + Assert.Equal("export.exe.zip", result); + } + + [Fact] + public void SanitizeDownloadName_AlreadyZip_IsPreserved() + { + var result = CmsData.SanitizeDownloadName("monthly-report.zip"); + + Assert.Equal("monthly-report.zip", result); + } + + [Fact] + public void SanitizeDownloadName_CaseInsensitiveZipExtension_IsPreserved() + { + var result = CmsData.SanitizeDownloadName("REPORT.ZIP"); + + Assert.Equal("REPORT.ZIP", result); + } + + [Theory] + [InlineData("CON")] + [InlineData("PRN")] + [InlineData("AUX")] + [InlineData("NUL")] + [InlineData("COM1")] + [InlineData("LPT1")] + [InlineData("con")] + [InlineData("CON.txt")] + public void SanitizeDownloadName_ReservedWindowsDeviceNames_ReturnsDefault(string input) + { + var result = CmsData.SanitizeDownloadName(input); + + Assert.Equal("FileDownload.zip", result); + } + + [Fact] + public void SanitizeDownloadName_AllowsSafeCharacters() + { + var result = CmsData.SanitizeDownloadName("My_File-01 v2.zip"); + + Assert.Equal("My_File-01 v2.zip", result); + } + + #endregion + + #region LooksLikeTraversalAttempt + + [Theory] + [InlineData(@"\..\..\evil.zip")] + [InlineData("../../evil.zip")] + [InlineData(@"C:\Windows\Temp\evil.zip")] + [InlineData("../evil")] + [InlineData("..")] + [InlineData("nested/file.zip")] + [InlineData(@"nested\file.zip")] + public void LooksLikeTraversalAttempt_PathShapedInput_IsFlagged(string input) + { + Assert.True(CmsData.LooksLikeTraversalAttempt(input)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("export")] + [InlineData("monthly-report.zip")] + [InlineData("My_File-01 v2.zip")] + [InlineData("report.final.zip")] + [InlineData("report..final.zip")] + [InlineData("a..b")] + public void LooksLikeTraversalAttempt_PlainName_IsNotFlagged(string? input) + { + Assert.False(CmsData.LooksLikeTraversalAttempt(input)); + } + + #endregion + + #region Regression guard (VPR-138) + + // Before the fix, DownloadZip built the on-disk temp archive path by + // concatenating GetRootFileFolder() + ticks + user-supplied fileName. + // A traversal payload in fileName (e.g. \..\..\evil.zip) escaped the + // CMS root. The fix splits the flow: user input feeds only the + // Content-Disposition name (SanitizeDownloadName); the on-disk path + // is generated from a server-side GUID under a dedicated temp root + // (BuildTempArchivePath). + // + // This test exercises both helpers as DownloadZip wires them and + // asserts the traversal payload cannot influence the on-disk path, + // even if a future refactor reintroduces the old concatenation. + [Theory] + [InlineData(@"\..\..\evil.zip")] + [InlineData("../../evil.zip")] + [InlineData(@"C:\Windows\System32\evil.zip")] + [InlineData(@"..\..\..\..\Windows\evil.zip")] + [InlineData("../../../../etc/passwd")] + public void Regression_Vpr138_TraversalPayload_CannotEscapeTempRoot(string attackPayload) + { + var tempRoot = CreateIsolatedTempRoot(); + try + { + var responseName = CmsData.SanitizeDownloadName(attackPayload); + var tempPath = CmsData.BuildTempArchivePath(tempRoot); + + Assert.DoesNotContain('\\', responseName); + Assert.DoesNotContain('/', responseName); + Assert.DoesNotContain("..", responseName); + Assert.EndsWith(".zip", responseName, StringComparison.OrdinalIgnoreCase); + + var resolvedRoot = Path.GetFullPath(tempRoot); + var resolvedPath = Path.GetFullPath(tempPath); + Assert.StartsWith( + resolvedRoot + Path.DirectorySeparatorChar, + resolvedPath, + StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("..", resolvedPath); + } + finally + { + Cleanup(tempRoot); + } + } + + #endregion + + #region SanitizeZipEntryName + + [Theory] + [InlineData(@"..\..\evil.txt", "evil.txt")] + [InlineData("../../evil.txt", "evil.txt")] + [InlineData(@"nested\folder\file.pdf", "file.pdf")] + [InlineData("nested/folder/file.pdf", "file.pdf")] + [InlineData("Annual Report 2024.pdf", "Annual Report 2024.pdf")] + public void SanitizeZipEntryName_StripsPathComponents(string friendlyName, string expected) + { + var result = CmsData.SanitizeZipEntryName(friendlyName, fallback: "fallback.bin"); + + Assert.Equal(expected, result); + Assert.DoesNotContain('\\', result); + Assert.DoesNotContain('/', result); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void SanitizeZipEntryName_EmptyFriendlyName_FallsBackToFilePath(string? friendlyName) + { + var result = CmsData.SanitizeZipEntryName(friendlyName, fallback: @"C:\storage\real-file.bin"); + + Assert.Equal("real-file.bin", result); + } + + [Theory] + [InlineData("..")] + [InlineData(".")] + [InlineData("dir/..")] + [InlineData(@"nested\.")] + [InlineData(". .")] + public void SanitizeZipEntryName_DotsOnly_FallsBackToFilePath(string friendlyName) + { + var result = CmsData.SanitizeZipEntryName(friendlyName, fallback: @"C:\storage\real-file.bin"); + + Assert.Equal("real-file.bin", result); + } + + #endregion + + #region BuildTempArchivePath + + [Fact] + public void BuildTempArchivePath_ReturnsPathUnderTempRoot() + { + var tempRoot = CreateIsolatedTempRoot(); + try + { + var path = CmsData.BuildTempArchivePath(tempRoot); + + var resolved = Path.GetFullPath(path); + var normalizedRoot = Path.GetFullPath(tempRoot); + + Assert.StartsWith( + normalizedRoot + Path.DirectorySeparatorChar, + resolved, + StringComparison.OrdinalIgnoreCase); + Assert.EndsWith(".zip", resolved, StringComparison.OrdinalIgnoreCase); + } + finally + { + Cleanup(tempRoot); + } + } + + [Fact] + public void BuildTempArchivePath_AcceptsRootWithTrailingSeparator() + { + var tempRoot = CreateIsolatedTempRoot(); + var withTrailing = tempRoot + Path.DirectorySeparatorChar; + try + { + var path = CmsData.BuildTempArchivePath(withTrailing); + + var resolved = Path.GetFullPath(path); + var normalizedRoot = Path.TrimEndingDirectorySeparator(Path.GetFullPath(withTrailing)); + + Assert.StartsWith( + normalizedRoot + Path.DirectorySeparatorChar, + resolved, + StringComparison.OrdinalIgnoreCase); + } + finally + { + Cleanup(tempRoot); + } + } + + [Fact] + public void BuildTempArchivePath_AcceptsFilesystemRoot() + { + // A filesystem root (e.g. "C:\" or "/") keeps its trailing separator after + // TrimEndingDirectorySeparator; the containment check must not double the + // separator and wrongly reject a path that is genuinely inside the root. + var root = Path.GetPathRoot(Path.GetTempPath())!; + + var path = CmsData.BuildTempArchivePath(root); + + Assert.StartsWith(Path.GetFullPath(root), Path.GetFullPath(path), StringComparison.OrdinalIgnoreCase); + Assert.EndsWith(".zip", path, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void BuildTempArchivePath_GeneratesUniquePaths_OnSuccessiveCalls() + { + var tempRoot = CreateIsolatedTempRoot(); + try + { + var a = CmsData.BuildTempArchivePath(tempRoot); + var b = CmsData.BuildTempArchivePath(tempRoot); + + Assert.NotEqual(a, b); + } + finally + { + Cleanup(tempRoot); + } + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void BuildTempArchivePath_RejectsEmptyRoot(string tempRoot) + { + Assert.Throws(() => CmsData.BuildTempArchivePath(tempRoot)); + } + + [Fact] + public void BuildTempArchivePath_RejectsNullRoot() + { + Assert.Throws(() => CmsData.BuildTempArchivePath(null!)); + } + + #endregion + + #region Helpers + + private static string CreateIsolatedTempRoot() + { + var root = Path.Join(Path.GetTempPath(), "Viper-CMS-Test-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(root); + return root; + } + + private static void Cleanup(string root) + { + if (Directory.Exists(root)) + { + Directory.Delete(root, recursive: true); + } + } + + #endregion +} diff --git a/web/Areas/CMS/Data/CMS.cs b/web/Areas/CMS/Data/CMS.cs index 0c639c3ac..79f3bbcf0 100644 --- a/web/Areas/CMS/Data/CMS.cs +++ b/web/Areas/CMS/Data/CMS.cs @@ -5,7 +5,9 @@ using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; +using Microsoft.Net.Http.Headers; using Viper.Areas.CMS.Models; +using Viper.Areas.CMS.Services; using Viper.Classes.SQLContext; using Viper.Classes.Utilities; using Viper.Models.AAUD; @@ -402,18 +404,25 @@ public bool CheckFilePermission(CMSFile file) #region public IActionResult DownloadZip(Controller controller, string[] fileGUIDs, string fileName = "FileDownload.zip") public IActionResult DownloadZip(Controller controller, string[] fileGUIDs, string fileName = "FileDownload.zip") { - if (fileGUIDs.Length == 0 && fileName.Length == 0) + // Treat an empty array or one of only blank segments (e.g. "ids=,,") as a + // missing parameter, and drop blanks so the loop skips no-op GetFile lookups. + var guids = fileGUIDs.Where(g => !string.IsNullOrWhiteSpace(g)).ToArray(); + if (guids.Length == 0) { - ArgumentNullException argumentNullException = new(nameof(fileGUIDs), "Missing fileGUIDs and file name parameters"); - throw argumentNullException; + return controller.BadRequest("Missing fileGUIDs parameter."); } - //only allow good filename characters - fileName = fileName.Replace(@"[^a-zA-Z0-9\.\-_ ]", ""); + var safeDownloadName = CmsFilePathSafety.SanitizeDownloadName(fileName); + List files = new(); AaudUser? currentUser = UserHelper.GetCurrentUser(); - foreach (var guid in fileGUIDs) + if (CmsFilePathSafety.LooksLikeTraversalAttempt(fileName)) + { + LogSuspiciousDownloadName(controller, fileName, safeDownloadName, currentUser); + } + + foreach (var guid in guids) { CMSFile? file = GetFile(guid, null, null, null, null); @@ -429,45 +438,59 @@ public IActionResult DownloadZip(Controller controller, string[] fileGUIDs, stri } } - // create a temp Zip file and populate it with the files - string tempFileName = GetRootFileFolder() + @"\" + DateTime.Now.Ticks + fileName; + if (files.Count == 0) + { + return controller.NotFound(); + } - using (FileStream fs = System.IO.File.Open(tempFileName, FileMode.OpenOrCreate)) + var zipTempFolder = CmsFilePathSafety.GetZipTempFolder(); + // Create on demand so a download survives the OS or an admin sweeping %TEMP% + // mid-run; otherwise File.Open below throws DirectoryNotFoundException. + System.IO.Directory.CreateDirectory(zipTempFolder); + string tempFileName = CmsFilePathSafety.BuildTempArchivePath(zipTempFolder); + + try { - using ZipArchive archive = new(fs, ZipArchiveMode.Update); - foreach (var file in files) + using (FileStream fs = System.IO.File.Open(tempFileName, FileMode.Create)) + using (ZipArchive archive = new(fs, ZipArchiveMode.Create)) { - if (file.Encrypted && !string.IsNullOrEmpty(file.Key)) + foreach (var file in files) { - ZipArchiveEntry fileEntry = archive.CreateEntry(file.FriendlyName); - using StreamWriter writer = new(fileEntry.Open()); - byte[] filebytes = System.IO.File.ReadAllBytes(file.FilePath); - filebytes = DecryptFile(filebytes, file.Key); + var entryName = CmsFilePathSafety.SanitizeZipEntryName(file.FriendlyName, file.FilePath); + + if (file.Encrypted && !string.IsNullOrEmpty(file.Key)) + { + ZipArchiveEntry fileEntry = archive.CreateEntry(entryName); + using var entryStream = fileEntry.Open(); + byte[] filebytes = System.IO.File.ReadAllBytes(file.FilePath); + filebytes = DecryptFile(filebytes, file.Key); - if (filebytes != null) + entryStream.Write(filebytes, 0, filebytes.Length); + } + else { - writer.BaseStream.Write(filebytes, 0, filebytes.Length); + archive.CreateEntryFromFile(file.FilePath, entryName); } } - else + } + + byte[] bytes = System.IO.File.ReadAllBytes(tempFileName); + return controller.File(bytes, MimeTypes["zip"], safeDownloadName); + } + finally + { + // Best-effort cleanup: swallow filesystem exceptions so we don't + // mask an earlier exception from archive creation or the response. + try + { + if (System.IO.File.Exists(tempFileName)) { - archive.CreateEntryFromFile(file.FilePath, file.FriendlyName); + System.IO.File.Delete(tempFileName); } - } + catch (IOException) { /* ignored */ } + catch (UnauthorizedAccessException) { /* ignored */ } } - - // read the temp zip file then delete it - byte[] bytes = System.IO.File.ReadAllBytes(tempFileName); - if (bytes == null) - return controller.NotFound(); - - System.IO.File.Delete(tempFileName); - - string extension = "zip"; - - return controller.File(bytes, MimeTypes[extension.ToLower()], fileName); - } #endregion @@ -526,9 +549,22 @@ public IActionResult ProvideFile(Controller controller, string id, string friend } string extension = file.FilePath[(file.FilePath.LastIndexOf('.') + 1)..]; - controller.Response.Headers["Content-Disposition"] = "inline; filename=" + friendlyName; + if (!MimeTypes.TryGetValue(extension.ToLowerInvariant(), out var contentType)) + { + // Unknown/missing extension: fall back to a generic binary type so an + // unlisted file type downloads instead of throwing (was a 500 via the indexer). + contentType = "application/octet-stream"; + } + + // Build Content-Disposition from the stored record's name, not the user-supplied + // friendlyName param, and let the framework encode it so a hostile fn cannot shape + // the header. Mirrors the DownloadZip download-name hardening. + var downloadName = CmsFilePathSafety.SanitizeZipEntryName(file.FriendlyName, file.FilePath); + var contentDisposition = new ContentDispositionHeaderValue("inline"); + contentDisposition.SetHttpFileName(downloadName); + controller.Response.Headers.ContentDisposition = contentDisposition.ToString(); - return controller.File(bytes, MimeTypes[extension.ToLower()], true); + return controller.File(bytes, contentType, true); } #endregion @@ -556,6 +592,30 @@ private void LogFileNotFound(Controller controller, string id, string friendlyNa LogSanitizer.SanitizeString(request.HttpContext.Connection.RemoteIpAddress?.ToString() ?? string.Empty)); } + // VPR-138: emits a warning when a DownloadZip request supplies a fileName that + // carries path structure (separators or ".." segments). The name only ever feeds + // the Content-Disposition header - the served files come from DB GUIDs and the + // temp path from a server-generated GUID - so this is detection signal for probing, + // not a block. Grep the tag to see who is poking at the download surface. + private void LogSuspiciousDownloadName(Controller controller, string rawFileName, string sanitizedName, AaudUser? currentUser) + { + if (_logger is null) + { + return; + } + + var request = controller.Request; + _logger.LogWarning( + "[CMS-DOWNLOAD-NAME] traversal-shaped download name loginId={LoginId} rawFileName={RawFileName} " + + "servedAs={SanitizedName} userAgent={UserAgent} referer={Referer} remoteIp={RemoteIp}", + LogSanitizer.SanitizeString(currentUser?.LoginId ?? string.Empty), + LogSanitizer.SanitizeString(rawFileName), + LogSanitizer.SanitizeString(sanitizedName), + LogSanitizer.SanitizeString(request.Headers.UserAgent.ToString()), + LogSanitizer.SanitizeString(request.Headers.Referer.ToString()), + LogSanitizer.SanitizeString(request.HttpContext.Connection.RemoteIpAddress?.ToString() ?? string.Empty)); + } + #region public static void AuditFileAccess(VIPERContext viperContext, CMSFile file, AaudUser user, string action, string detail) public static void AuditFileAccess(VIPERContext viperContext, CMSFile file, AaudUser user, string action, string detail) { diff --git a/web/Areas/CMS/Services/CmsFilePathSafety.cs b/web/Areas/CMS/Services/CmsFilePathSafety.cs new file mode 100644 index 000000000..448745d7c --- /dev/null +++ b/web/Areas/CMS/Services/CmsFilePathSafety.cs @@ -0,0 +1,187 @@ +using System.Text.RegularExpressions; + +namespace Viper.Areas.CMS.Services +{ + /// + /// Path-safety primitives for CMS file download flows (VPR-138). + /// + /// Two surfaces: the static class is used by the legacy + /// CMS.DownloadZip call site (no DI plumbing); the + /// interface is the contract the + /// PLAN-CMS migration's IFileStorageService / new download + /// controller depends on. See PLAN-CMS.md §11.7. + /// + public static class CmsFilePathSafety + { + private const string DefaultDownloadName = "FileDownload.zip"; + + private static readonly HashSet ReservedWindowsNames = new(StringComparer.OrdinalIgnoreCase) + { + "CON", "PRN", "AUX", "NUL", + "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", + "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9" + }; + + private static readonly Regex DisallowedFileNameChars = new(@"[^a-zA-Z0-9._\- ]", RegexOptions.Compiled); + + /// + /// Returns a filename safe to use in a Content-Disposition response header. + /// Strips path components, applies an allow-list, rejects reserved Windows + /// device names, and guarantees a .zip suffix so the name matches + /// the application/zip MIME type. + /// + public static string SanitizeDownloadName(string? userInput) + { + if (string.IsNullOrWhiteSpace(userInput)) + { + return DefaultDownloadName; + } + + var fileNamePart = StripPathComponents(userInput); + var filtered = DisallowedFileNameChars.Replace(fileNamePart, string.Empty).Trim(); + + // Reject names that collapse to only dots/spaces: ".", ".." etc. would + // become "..zip" after the suffix step, which is traversal-shaped. + if (filtered.Trim('.', ' ').Length == 0) + { + return DefaultDownloadName; + } + + var stem = filtered; + var dotIndex = stem.IndexOf('.'); + if (dotIndex >= 0) + { + stem = stem[..dotIndex]; + } + if (ReservedWindowsNames.Contains(stem)) + { + return DefaultDownloadName; + } + + if (!filtered.EndsWith(".zip", StringComparison.OrdinalIgnoreCase)) + { + filtered += ".zip"; + } + + return filtered; + } + + /// + /// True when carried path structure: a path + /// separator, or ".." as a distinct segment. Used purely to log a + /// traversal-shaped download name; it never affects what is served (the file + /// set comes from DB GUIDs and the temp path from a server-generated GUID, so a + /// hostile name cannot reach the filesystem). A ".." inside a longer name + /// (e.g. "report..final.zip") is not treated as traversal. + /// + public static bool LooksLikeTraversalAttempt(string? userInput) + { + if (string.IsNullOrWhiteSpace(userInput)) + { + return false; + } + + // A download name should be a single segment: any separator, or a bare ".." + // segment, is traversal-shaped. ".." inside a name (e.g. "report..final") is not. + var normalized = userInput.Replace('\\', '/'); + return normalized.Contains('/') || normalized == ".."; + } + + /// + /// Builds a per-request temp archive path under + /// using a server-generated GUID, and asserts the resolved path stays + /// inside that root. + /// + public static string BuildTempArchivePath(string tempRoot) + { + if (string.IsNullOrWhiteSpace(tempRoot)) + { + throw new ArgumentException("Temp root must be provided.", nameof(tempRoot)); + } + + var resolvedRoot = Path.TrimEndingDirectorySeparator(Path.GetFullPath(tempRoot)); + var candidate = Path.Join(resolvedRoot, Guid.NewGuid().ToString("N") + ".zip"); + var resolved = Path.GetFullPath(candidate); + + // TrimEndingDirectorySeparator leaves a filesystem root (e.g. "C:\" or "/") + // with its trailing separator, so append one only when it isn't already there + // to avoid a doubled separator that would break the containment check. + var rootPrefix = Path.EndsInDirectorySeparator(resolvedRoot) + ? resolvedRoot + : resolvedRoot + Path.DirectorySeparatorChar; + + if (!resolved.StartsWith(rootPrefix, StringComparison.OrdinalIgnoreCase)) + { + throw new InvalidOperationException("Generated temp path escaped the configured root."); + } + + return resolved; + } + + /// + /// Dedicated temp directory for ZIP archives. Kept separate from the + /// CMS content root so transient archives never mix with managed + /// content, and never get served through content URLs. + /// + public static string GetZipTempFolder() + { + return Path.Join(Path.GetTempPath(), "Viper-CMS"); + } + + /// + /// Returns a safe ZIP entry name from a stored file's friendly name. + /// Defense in depth against ZIP-slip when the archive is extracted. + /// + public static string SanitizeZipEntryName(string? friendlyName, string fallback) + { + if (!string.IsNullOrWhiteSpace(friendlyName)) + { + var entry = StripPathComponents(friendlyName); + if (!string.IsNullOrWhiteSpace(entry) && entry.Trim('.', ' ').Length > 0) + { + return entry; + } + } + + return StripPathComponents(fallback); + } + + // Path.GetFileName only honors the host OS separator, so "\..\evil" + // leaks through unchanged on Linux runners. Normalize first. + private static string StripPathComponents(string input) + => Path.GetFileName(input.Replace('\\', '/')); + } + + /// + /// DI-injectable contract for . New code + /// (see PLAN-CMS.md §11.7) depends on this interface so the underlying + /// implementation can evolve without touching call sites. + /// + public interface ICmsFilePathSafetyService + { + string SanitizeDownloadName(string? userInput); + bool LooksLikeTraversalAttempt(string? userInput); + string BuildTempArchivePath(string tempRoot); + string GetZipTempFolder(); + string SanitizeZipEntryName(string? friendlyName, string fallback); + } + + /// + public sealed class CmsFilePathSafetyService : ICmsFilePathSafetyService + { + public string SanitizeDownloadName(string? userInput) => + CmsFilePathSafety.SanitizeDownloadName(userInput); + + public bool LooksLikeTraversalAttempt(string? userInput) => + CmsFilePathSafety.LooksLikeTraversalAttempt(userInput); + + public string BuildTempArchivePath(string tempRoot) => + CmsFilePathSafety.BuildTempArchivePath(tempRoot); + + public string GetZipTempFolder() => + CmsFilePathSafety.GetZipTempFolder(); + + public string SanitizeZipEntryName(string? friendlyName, string fallback) => + CmsFilePathSafety.SanitizeZipEntryName(friendlyName, fallback); + } +} diff --git a/web/Program.cs b/web/Program.cs index 2b4e35807..6b65c380f 100644 --- a/web/Program.cs +++ b/web/Program.cs @@ -258,7 +258,8 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db "Viper.Areas.ClinicalScheduler.Validators", "Viper.Areas.Students.Services", "Viper.Areas.Curriculum.Services", - "Viper.Areas.Effort.Services" + "Viper.Areas.Effort.Services", + "Viper.Areas.CMS.Services" ) .Where(type => type.Name.EndsWith("Service") || type.Name.EndsWith("Validator"))) .UsingRegistrationStrategy(RegistrationStrategy.Skip) @@ -404,44 +405,6 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db // when our auth filter denies — gets the same Razor error view as the rest of the app. app.UseStatusCodePagesWithReExecute("/Error/{0}"); - // In development, set up Vite proxy BEFORE rewrite rules so it can handle .ts/.js files - if (app.Environment.IsDevelopment()) - { - // Development: Proxy Vue.js assets to Vite dev server for hot module replacement (HMR) - // This middleware intercepts requests for Vue assets and forwards them to the Vite dev server - app.Use(async (context, next) => - { - if (ViteProxyHelpers.ShouldProxyToVite(context, VueAppNames)) - { - try - { - // Use the registered HttpClient from dependency injection - var httpClientFactory = context.RequestServices.GetRequiredService(); - var httpClient = httpClientFactory.CreateClient("ViteProxy"); - - // Build the Vite server URL and try to proxy directly - var viteUrl = ViteProxyHelpers.BuildViteUrl(context.Request.Path, context.Request.QueryString, VueAppNames); - var requestMessage = ViteProxyHelpers.CreateProxyRequest(context, viteUrl); - using var response = await httpClient.SendAsync(requestMessage, HttpCompletionOption.ResponseHeadersRead, context.RequestAborted); - - // Copy the response back to the client - await ViteProxyHelpers.CopyProxyResponse(context, response); - return; // Successfully proxied, don't continue to static files - } - catch (Exception ex) - { - var logger = context.RequestServices.GetRequiredService>(); - logger.LogDebug(ex, "Vite server not available, falling back to static files for {Path}", - Uri.EscapeDataString(context.Request.Path.Value ?? "unknown")); - // Fall through to static file serving - } - } - - // Continue to static file serving (either Vite not needed or not available) - await next(); - }); - } - var rewriteOptions = new RewriteOptions(); // Add redirects and rewrites for each SPA using centralized app names @@ -458,36 +421,77 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db rewriteOptions.AddRewrite($@"(?i)^{escapedAppName}", $"/2/vue/src/{lowerAppName}/index.html", true); } - app.UseRewriter(rewriteOptions); - - //for the vue src files, use directories in the url but serve index.html + // Default-file convention for /vue (legacy path). app.UseDefaultFiles(new DefaultFilesOptions { DefaultFileNames = new List { "index.html" }, FileProvider = new PhysicalFileProvider( - Path.Join(builder.Environment.ContentRootPath, "wwwroot/vue")), + Path.Join(builder.Environment.WebRootPath, "vue")), RequestPath = "/vue", RedirectToAppendTrailingSlash = true }); - // Static file serving configuration - // Serve built Vue files - in development proxy middleware runs first, - // in production these files are served directly - app.UseStaticFiles(new StaticFileOptions - { - FileProvider = new PhysicalFileProvider( - Path.Join(builder.Environment.ContentRootPath, "wwwroot/vue")), - RequestPath = "/2/vue" - }); - - // Serve other static files + // General static files (favicon, /css, /js, /images, etc.). app.UseStaticFiles(); - // Add sitemap middleware after static file handling app.UseSitemapMiddleware(); - // apply settings define earlier + // Routing first so subsequent middleware can defer to a matched MVC endpoint. app.UseRouting(); + + // SPA shell serving for Vue app prefixes like /CMS, /Effort, etc. Runs before + // auth/session so static Vue assets skip that per-request overhead. + // Only runs when no MVC controller endpoint claimed the path, so attribute-routed + // legacy endpoints (e.g. /CMS/Files → CMSController.Files) reach the controller + // instead of being rewritten to the SPA shell. + app.UseWhen( + ctx => ctx.GetEndpoint() is null, + branch => + { + if (app.Environment.IsDevelopment()) + { + // Dev: proxy Vue assets and SPA routes to the Vite dev server (HMR). + branch.Use(async (context, next) => + { + if (ViteProxyHelpers.ShouldProxyToVite(context, VueAppNames)) + { + try + { + var httpClientFactory = context.RequestServices.GetRequiredService(); + var httpClient = httpClientFactory.CreateClient("ViteProxy"); + + var viteUrl = ViteProxyHelpers.BuildViteUrl(context.Request.Path, context.Request.QueryString, VueAppNames); + var requestMessage = ViteProxyHelpers.CreateProxyRequest(context, viteUrl); + using var response = await httpClient.SendAsync(requestMessage, HttpCompletionOption.ResponseHeadersRead, context.RequestAborted); + + await ViteProxyHelpers.CopyProxyResponse(context, response); + return; + } + catch (Exception ex) when (ex is HttpRequestException or TaskCanceledException) + { + var viteLogger = context.RequestServices.GetRequiredService>(); + viteLogger.LogDebug(ex, "Vite server not available, falling back to static files for {Path}", + Uri.EscapeDataString(context.Request.Path.Value ?? "unknown")); + } + } + + await next(); + }); + } + + // Prod (and dev fallback): rewrite SPA routes to the built SPA shell, + // then serve the static file from wwwroot/vue. + branch.UseRewriter(rewriteOptions); + branch.UseStaticFiles(new StaticFileOptions + { + FileProvider = new PhysicalFileProvider( + Path.Join(builder.Environment.WebRootPath, "vue")), + RequestPath = "/2/vue" + }); + }); + + // Auth/session run after the SPA shell block so built Vue assets skip them, but + // still before health checks, Hangfire, and controllers, which need an authenticated user. app.UseAuthentication(); app.UseAuthorization(); app.UseCookiePolicy(); diff --git a/web/Views/Home/StatusCode.cshtml b/web/Views/Home/StatusCode.cshtml index dc6d8955a..8cdff3cda 100644 --- a/web/Views/Home/StatusCode.cshtml +++ b/web/Views/Home/StatusCode.cshtml @@ -4,4 +4,4 @@ ViewData["Title"] = "Error"; ViewData["AllowAnonymousContent"] = true; } -

Status Code: @((int)Model); @(Enum.GetName(typeof(HttpStatusCode), Model.ToString()))

\ No newline at end of file +

Status Code: @((int)Model); @Model

\ No newline at end of file