From c8995b76885307abfc5e9aa6bcafe159bc706f39 Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Tue, 16 Jun 2026 19:23:54 -0700 Subject: [PATCH 1/2] refactor(codeql): decompose complex boolean conditions Extract named sub-conditions / guard clauses to clear cs/complex-condition findings, behavior-preserving: - CMS.CheckFilePermission: early-return guard clauses; resolve the user's permissions once into an OrdinalIgnoreCase HashSet instead of re-querying per file permission (removes an O(n^2) scan). - ViteProxyHelpers.CreateProxyRequest: methodSupportsBody / hasReadableBody named bools, comparing the request method with OrdinalIgnoreCase rather than allocating an uppercased copy. --- web/Areas/CMS/Data/CMS.cs | 29 +++++++++++++++++++++++++---- web/ViteProxyHelpers.cs | 10 ++++++---- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/web/Areas/CMS/Data/CMS.cs b/web/Areas/CMS/Data/CMS.cs index 79f3bbcf..1cf19d99 100644 --- a/web/Areas/CMS/Data/CMS.cs +++ b/web/Areas/CMS/Data/CMS.cs @@ -383,10 +383,31 @@ public bool CheckFilePermission(CMSFile file) if (_rapsContext! != null && currentUser != null) { - if (file.AllowPublicAccess || - (file.FileToPermissions.Count == 0 && UserHelper.HasPermission(_rapsContext, currentUser, "SVMSecure")) || - (file.FileToPermissions.Count > 0 && file.FileToPermissions.Any(fp => UserHelper.GetAllPermissions(_rapsContext, currentUser).Any(p => string.Compare(fp.Permission, p.Permission, true) == 0))) || - (file.FileToPeople.Count > 0 && file.FileToPeople.Any(fp => fp.IamId == currentUser.IamId))) + // Each access path returns early so later (more expensive) checks are + // skipped once access is granted, preserving the original short-circuit. + if (file.AllowPublicAccess) + { + return true; + } + // No explicit permissions: any authenticated SVMSecure user may access. + if (file.FileToPermissions.Count == 0 && UserHelper.HasPermission(_rapsContext, currentUser, "SVMSecure")) + { + return true; + } + // Explicit permissions: the user must hold a matching one. Resolve the user's + // permissions once into a set rather than re-querying for each file permission. + if (file.FileToPermissions.Count > 0) + { + var userPermissions = UserHelper.GetAllPermissions(_rapsContext, currentUser) + .Select(p => p.Permission) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + if (file.FileToPermissions.Any(fp => userPermissions.Contains(fp.Permission))) + { + return true; + } + } + // Explicit people: the user must be listed. + if (file.FileToPeople.Count > 0 && file.FileToPeople.Any(fp => fp.IamId == currentUser.IamId)) { return true; } diff --git a/web/ViteProxyHelpers.cs b/web/ViteProxyHelpers.cs index eae280cc..d7596b6e 100644 --- a/web/ViteProxyHelpers.cs +++ b/web/ViteProxyHelpers.cs @@ -216,10 +216,12 @@ public static HttpRequestMessage CreateProxyRequest(HttpContext context, string var requestMessage = new HttpRequestMessage(new HttpMethod(context.Request.Method), targetUrl); // Only copy request body for methods that typically support it - var method = context.Request.Method.ToUpperInvariant(); - if (method != "GET" && method != "HEAD" - && ((context.Request.ContentLength.HasValue && context.Request.ContentLength > 0) || - (!context.Request.ContentLength.HasValue && context.Request.Body.CanRead))) + var requestMethod = context.Request.Method; + bool methodSupportsBody = !string.Equals(requestMethod, "GET", StringComparison.OrdinalIgnoreCase) + && !string.Equals(requestMethod, "HEAD", StringComparison.OrdinalIgnoreCase); + bool hasReadableBody = (context.Request.ContentLength.HasValue && context.Request.ContentLength > 0) + || (!context.Request.ContentLength.HasValue && context.Request.Body.CanRead); + if (methodSupportsBody && hasReadableBody) { requestMessage.Content = new StreamContent(context.Request.Body); From 3ceffd8110badaaffb5885fa6c663c7df415ba36 Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Tue, 16 Jun 2026 19:27:47 -0700 Subject: [PATCH 2/2] fix(security): respect directory boundary in Vite web-root check Append a trailing separator to the resolved web root before the StartsWith containment check so a sibling directory (e.g. wwwroot-secret) cannot satisfy the prefix check against wwwroot and bypass the directory-traversal guard when serving Vite static files. --- web/ViteProxyHelpers.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/web/ViteProxyHelpers.cs b/web/ViteProxyHelpers.cs index d7596b6e..5e02786a 100644 --- a/web/ViteProxyHelpers.cs +++ b/web/ViteProxyHelpers.cs @@ -350,10 +350,16 @@ public static async Task HandleProxyError(HttpContext context, Exception ex, ILo var physicalPath = Path.Join(context.RequestServices.GetRequiredService().WebRootPath, staticPath.TrimStart('/').Replace('/', Path.DirectorySeparatorChar)); - // Prevent directory traversal: ensure the resolved physical path is within WebRootPath + // Prevent directory traversal: ensure the resolved physical path is within WebRootPath. + // A trailing separator makes the StartsWith check respect the directory boundary so a + // sibling like "wwwroot-secret" cannot satisfy the prefix check against "wwwroot". var webRoot = context.RequestServices.GetRequiredService().WebRootPath; var resolvedPhysical = Path.GetFullPath(physicalPath); var resolvedWebRoot = Path.GetFullPath(webRoot); + if (!Path.EndsInDirectorySeparator(resolvedWebRoot)) + { + resolvedWebRoot += Path.DirectorySeparatorChar; + } if (!resolvedPhysical.StartsWith(resolvedWebRoot, StringComparison.OrdinalIgnoreCase)) {