diff --git a/test/Areas/Directory/VMACSServiceTest.cs b/test/Areas/Directory/VMACSServiceTest.cs index 68ccee68a..7039de4f0 100644 --- a/test/Areas/Directory/VMACSServiceTest.cs +++ b/test/Areas/Directory/VMACSServiceTest.cs @@ -61,7 +61,7 @@ public void Deserialize_ReturnsItem_WhenPayloadMatches() Assert.NotNull(query); Assert.NotNull(query.item); - Assert.Equal("Doe, John", query.item!.Name?.Single()); + Assert.Equal("Doe, John", query.item.Name?.Single()); } [Fact] diff --git a/test/CMS/CMSContentControllerTests.cs b/test/CMS/CMSContentControllerTests.cs index badcb4553..f24c807de 100644 --- a/test/CMS/CMSContentControllerTests.cs +++ b/test/CMS/CMSContentControllerTests.cs @@ -150,7 +150,7 @@ public async Task GetContentBlock_ReturnsBlock_WhenFound() var result = await _controller.GetContentBlock(5, TestContext.Current.CancellationToken); Assert.NotNull(result.Value); - Assert.Equal(5, result.Value!.ContentBlockId); + Assert.Equal(5, result.Value.ContentBlockId); } [Fact] @@ -273,7 +273,7 @@ public async Task CreateContentBlock_ReturnsBlock_OnSuccess() var result = await _controller.CreateContentBlock(request, TestContext.Current.CancellationToken); Assert.NotNull(result.Value); - Assert.Equal(7, result.Value!.ContentBlockId); + Assert.Equal(7, result.Value.ContentBlockId); } [Fact] @@ -286,7 +286,7 @@ public async Task CreateContentBlock_ReturnsValidationProblem_OnArgumentExceptio var result = await _controller.CreateContentBlock(request, TestContext.Current.CancellationToken); Assert.IsType(result.Result); - var problem = (ObjectResult)result.Result!; + var problem = (ObjectResult)result.Result; Assert.IsType(problem.Value); } @@ -313,7 +313,7 @@ public async Task UpdateContentBlock_ReturnsBlock_OnSuccess() var result = await _controller.UpdateContentBlock(3, request, TestContext.Current.CancellationToken); Assert.NotNull(result.Value); - Assert.Equal(3, result.Value!.ContentBlockId); + Assert.Equal(3, result.Value.ContentBlockId); } [Fact] diff --git a/test/CMS/CMSFilesControllerTests.cs b/test/CMS/CMSFilesControllerTests.cs index dade1b68e..3974e8c04 100644 --- a/test/CMS/CMSFilesControllerTests.cs +++ b/test/CMS/CMSFilesControllerTests.cs @@ -160,7 +160,7 @@ public async Task GetFile_ReturnsFile_WhenFound() var result = await _controller.GetFile(guid, TestContext.Current.CancellationToken); Assert.NotNull(result.Value); - Assert.Equal(guid, result.Value!.FileGuid); + Assert.Equal(guid, result.Value.FileGuid); } [Fact] diff --git a/test/CMS/CMSLeftNavControllerTests.cs b/test/CMS/CMSLeftNavControllerTests.cs index 5fa6cb2d4..e0c4fc245 100644 --- a/test/CMS/CMSLeftNavControllerTests.cs +++ b/test/CMS/CMSLeftNavControllerTests.cs @@ -56,7 +56,7 @@ public async Task GetMenu_ReturnsMenu_WhenFound() var result = await _controller.GetMenu(5, TestContext.Current.CancellationToken); Assert.NotNull(result.Value); - Assert.Equal(5, result.Value!.LeftNavMenuId); + Assert.Equal(5, result.Value.LeftNavMenuId); } [Fact] @@ -96,7 +96,7 @@ public async Task UpdateMenu_ReturnsMenu_OnSuccess() var result = await _controller.UpdateMenu(5, request, TestContext.Current.CancellationToken); Assert.NotNull(result.Value); - Assert.Equal(5, result.Value!.LeftNavMenuId); + Assert.Equal(5, result.Value.LeftNavMenuId); } [Fact] diff --git a/test/CMS/CMSOptionsControllerTests.cs b/test/CMS/CMSOptionsControllerTests.cs index 44ae5b1f0..6877b6dfd 100644 --- a/test/CMS/CMSOptionsControllerTests.cs +++ b/test/CMS/CMSOptionsControllerTests.cs @@ -70,7 +70,7 @@ public async Task SearchPeople_ReturnsEmpty_WhenSearchBelowMinimumLength(string? var result = await _controller.SearchPeople(search!, TestContext.Current.CancellationToken); Assert.NotNull(result.Value); - Assert.Empty(result.Value!); + Assert.Empty(result.Value); } [Fact] diff --git a/test/ClinicalScheduler/PermissionsControllerTest.cs b/test/ClinicalScheduler/PermissionsControllerTest.cs index 4d4b1b39e..7280cbff3 100644 --- a/test/ClinicalScheduler/PermissionsControllerTest.cs +++ b/test/ClinicalScheduler/PermissionsControllerTest.cs @@ -128,10 +128,10 @@ public async Task GetUserPermissions_WithValidUser_ReturnsOkWithPermissions() var okResult = Assert.IsType(result.Result); dynamic? response = okResult.Value; Assert.Equal(TestUserMothraId, response!.user.mothraId); - Assert.Equal(TestUserDisplayName, response!.user.displayName); - Assert.False(response!.permissions.hasManagePermission); - Assert.Equal(1, response!.permissions.editableServiceCount); - Assert.Single(response!.editableServices); + Assert.Equal(TestUserDisplayName, response.user.displayName); + Assert.False(response.permissions.hasManagePermission); + Assert.Equal(1, response.permissions.editableServiceCount); + Assert.Single(response.editableServices); } [Fact] diff --git a/test/Effort/HarvestTimeParserTests.cs b/test/Effort/HarvestTimeParserTests.cs index 02df15732..0898d1ec0 100644 --- a/test/Effort/HarvestTimeParserTests.cs +++ b/test/Effort/HarvestTimeParserTests.cs @@ -37,8 +37,7 @@ public void ParseTimeString_ValidTimeFormats_ParsesCorrectly(string input, int e { var result = HarvestTimeParser.ParseTimeString(input); - Assert.NotNull(result); - var value = result!.Value; + var value = Assert.IsType(result); Assert.Equal(expectedHour, value.Hours); Assert.Equal(expectedMinute, value.Minutes); } diff --git a/test/HealthChecks/HealthCheckCollectorTokenHandlerTests.cs b/test/HealthChecks/HealthCheckCollectorTokenHandlerTests.cs index 20fd159e9..6e7e201e2 100644 --- a/test/HealthChecks/HealthCheckCollectorTokenHandlerTests.cs +++ b/test/HealthChecks/HealthCheckCollectorTokenHandlerTests.cs @@ -16,7 +16,7 @@ public async Task SendAsync_StampsTokenHeader() Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(recorder.LastRequest); - var values = recorder.LastRequest!.Headers.GetValues(HealthCheckCollectorAuth.HeaderName).ToList(); + var values = recorder.LastRequest.Headers.GetValues(HealthCheckCollectorAuth.HeaderName).ToList(); Assert.Single(values); Assert.Equal(HealthCheckCollectorAuth.Token, values[0]); } diff --git a/test/Scheduler/ScheduledJobRunnerTests.cs b/test/Scheduler/ScheduledJobRunnerTests.cs index b850d9c44..a35caac40 100644 --- a/test/Scheduler/ScheduledJobRunnerTests.cs +++ b/test/Scheduler/ScheduledJobRunnerTests.cs @@ -48,7 +48,7 @@ public async Task RunAsync_ResolvesJobByIdAndStampsSchedulerActor() Assert.True(capturing.Ran); Assert.NotNull(capturing.CapturedContext); - Assert.Equal(ScheduledJobContext.SchedulerActor, capturing.CapturedContext!.ModBy); + Assert.Equal(ScheduledJobContext.SchedulerActor, capturing.CapturedContext.ModBy); } [Fact] diff --git a/test/Students/EmergencyContactControllerTests.cs b/test/Students/EmergencyContactControllerTests.cs index 2efc6b194..ac8fd9fe6 100644 --- a/test/Students/EmergencyContactControllerTests.cs +++ b/test/Students/EmergencyContactControllerTests.cs @@ -214,7 +214,7 @@ public async Task GetStudentContactDetail_StudentCannotViewOtherRecord_ReturnsFo var result = await _controller.GetStudentContactDetail(999); Assert.IsType(result.Result); - var objectResult = (ObjectResult)result.Result!; + var objectResult = (ObjectResult)result.Result; Assert.Equal(403, objectResult.StatusCode); } @@ -662,7 +662,7 @@ public void GetStudentContactDetail_HasAuthorizeAttributeButNoPermissionGate() var method = typeof(EmergencyContactController).GetMethod(nameof(EmergencyContactController.GetStudentContactDetail)); Assert.NotNull(method); - var permissionAttrs = method!.GetCustomAttributes(typeof(PermissionAttribute), false); + var permissionAttrs = method.GetCustomAttributes(typeof(PermissionAttribute), false); Assert.Empty(permissionAttrs); var authorizeAttrs = method.GetCustomAttributes(typeof(AuthorizeAttribute), false); @@ -676,7 +676,7 @@ public void CanEdit_HasAuthorizeAttributeButNoPermissionGate() var method = typeof(EmergencyContactController).GetMethod(nameof(EmergencyContactController.CanEdit)); Assert.NotNull(method); - var permissionAttrs = method!.GetCustomAttributes(typeof(PermissionAttribute), false); + var permissionAttrs = method.GetCustomAttributes(typeof(PermissionAttribute), false); Assert.Empty(permissionAttrs); var authorizeAttrs = method.GetCustomAttributes(typeof(AuthorizeAttribute), false); diff --git a/test/Students/EmergencyContactServiceTests.cs b/test/Students/EmergencyContactServiceTests.cs index 407f387ee..d21fd43bd 100644 --- a/test/Students/EmergencyContactServiceTests.cs +++ b/test/Students/EmergencyContactServiceTests.cs @@ -1387,7 +1387,7 @@ public async Task GetStudentContactReportAsync_IncludesContactDetails() Assert.Equal("95616", student.Zip); Assert.True(student.ContactPermanent); Assert.NotNull(student.EmergencyContact); - Assert.Equal("Jane Doe", student.EmergencyContact!.Name); + Assert.Equal("Jane Doe", student.EmergencyContact.Name); } } diff --git a/web/Areas/CMS/Services/CmsFilePathSafety.cs b/web/Areas/CMS/Services/CmsFilePathSafety.cs index a5d5f4e9e..702b8ff43 100644 --- a/web/Areas/CMS/Services/CmsFilePathSafety.cs +++ b/web/Areas/CMS/Services/CmsFilePathSafety.cs @@ -30,7 +30,9 @@ public static class CmsFilePathSafety "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9" }; - private static readonly Regex DisallowedFileNameChars = new(@"[^a-zA-Z0-9._\- ]", RegexOptions.Compiled); + // Bounds matching so a pathological filename cannot pin a request thread. + private static readonly Regex DisallowedFileNameChars = + new(@"[^a-zA-Z0-9._\- ]", RegexOptions.Compiled, TimeSpan.FromSeconds(1)); /// /// Returns a filename safe to use in a Content-Disposition response header. @@ -46,7 +48,17 @@ public static string SanitizeDownloadName(string? userInput) } var fileNamePart = StripPathComponents(userInput); - var filtered = DisallowedFileNameChars.Replace(fileNamePart, string.Empty).Trim(); + + string filtered; + try + { + filtered = DisallowedFileNameChars.Replace(fileNamePart, string.Empty).Trim(); + } + catch (RegexMatchTimeoutException) + { + // Fail safe: an unfiltered name must never reach the response header. + return DefaultDownloadName; + } // Reject names that collapse to only dots/spaces: ".", ".." etc. would // become "..zip" after the suffix step, which is traversal-shaped. diff --git a/web/Areas/CTS/Controllers/BundleCompetencyController.cs b/web/Areas/CTS/Controllers/BundleCompetencyController.cs index a3df4a6dc..f7c7ecfc3 100644 --- a/web/Areas/CTS/Controllers/BundleCompetencyController.cs +++ b/web/Areas/CTS/Controllers/BundleCompetencyController.cs @@ -22,12 +22,12 @@ public BundleCompetencyController(VIPERContext context) private bool BundleExists(int bundleId) { - return context.Bundles.Where(b => b.BundleId == bundleId).Any(); + return context.Bundles.AsNoTracking().Any(b => b.BundleId == bundleId); } private bool CompetencyExists(int competencyId) { - return context.Competencies.Where(b => b.CompetencyId == competencyId).Any(); + return context.Competencies.AsNoTracking().Any(b => b.CompetencyId == competencyId); } [HttpGet] diff --git a/web/Areas/CTS/Controllers/BundleCompetencyGroupController.cs b/web/Areas/CTS/Controllers/BundleCompetencyGroupController.cs index a9ed6330b..2801e4c88 100644 --- a/web/Areas/CTS/Controllers/BundleCompetencyGroupController.cs +++ b/web/Areas/CTS/Controllers/BundleCompetencyGroupController.cs @@ -22,16 +22,16 @@ public BundleCompetencyGroupController(VIPERContext context) private bool BundleExists(int bundleId) { - return context.Bundles.Where(b => b.BundleId == bundleId).Any(); + return context.Bundles.AsNoTracking().Any(b => b.BundleId == bundleId); } private bool SameNameExists(int bundleId, string name, int? bundleCompetencyGroupId = null) { return context.BundleCompetencyGroups - .Where(g => g.BundleId == bundleId + .AsNoTracking() + .Any(g => g.BundleId == bundleId && g.Name == name - && (bundleCompetencyGroupId == null || bundleCompetencyGroupId != g.BundleCompetencyGroupId)) - .Any(); + && (bundleCompetencyGroupId == null || bundleCompetencyGroupId != g.BundleCompetencyGroupId)); } [HttpGet] diff --git a/web/Areas/CTS/Models/CompetencyBundleAssociationDto.cs b/web/Areas/CTS/Models/CompetencyBundleAssociationDto.cs index a4bcfa387..e2e77832f 100644 --- a/web/Areas/CTS/Models/CompetencyBundleAssociationDto.cs +++ b/web/Areas/CTS/Models/CompetencyBundleAssociationDto.cs @@ -49,7 +49,7 @@ public CompetencyBundleAssociationDto(Competency c) .Where(bc => bc.Bundle != null) .Select(bc => new BundleInfoDto { - BundleId = bc.Bundle!.BundleId, + BundleId = bc.Bundle.BundleId, Name = bc.Bundle.Name, Clinical = bc.Bundle.Clinical, Assessment = bc.Bundle.Assessment, diff --git a/web/Areas/ClinicalScheduler/Controllers/CliniciansController.cs b/web/Areas/ClinicalScheduler/Controllers/CliniciansController.cs index 69be2975f..b0badf8f1 100644 --- a/web/Areas/ClinicalScheduler/Controllers/CliniciansController.cs +++ b/web/Areas/ClinicalScheduler/Controllers/CliniciansController.cs @@ -289,7 +289,7 @@ public async Task GetClinicianSchedule(string mothraId, [FromQuer isPrimaryEvaluator = schedule.Evaluator }; }) - .OrderBy(r => r!.name) // Sort rotations alphabetically + .OrderBy(r => r.name) // Sort rotations alphabetically .Cast() .ToArray() : Array.Empty(); diff --git a/web/Areas/ClinicalScheduler/Controllers/InstructorScheduleController.cs b/web/Areas/ClinicalScheduler/Controllers/InstructorScheduleController.cs index f38d36146..1c969242d 100644 --- a/web/Areas/ClinicalScheduler/Controllers/InstructorScheduleController.cs +++ b/web/Areas/ClinicalScheduler/Controllers/InstructorScheduleController.cs @@ -76,24 +76,36 @@ public async Task AddInstructor( correlationId)); } + // ValidateRequestAsync above already guarantees both are present. Capturing them + // as non-null locals makes every later use provable rather than asserted. + if (request.RotationId is not { } rotationId || request.GradYear is not { } gradYear) + { + return BadRequest(new ErrorResponse( + ErrorCodes.ValidationError, + "Please check your input and try again.", + correlationId)); + } + // Step 2: Check permissions - include own schedule check - if (!await CheckPermissionsForAddAsync(request.RotationId!.Value, request.MothraId!, correlationId, cancellationToken)) + if (!await CheckPermissionsForAddAsync(rotationId, request.MothraId, correlationId, cancellationToken)) { return Forbid(); } // Step 3: Check for conflicts and build warning message var warningMessage = await BuildConflictWarningAsync( - request.MothraId!, + request.MothraId, request.WeekIds, - request.GradYear!.Value, - request.RotationId!.Value, + gradYear, + rotationId, correlationId, cancellationToken); // Step 4: Add instructor through service layer var response = await ProcessAddInstructorAsync( request, + rotationId, + gradYear, warningMessage, correlationId, cancellationToken); @@ -176,16 +188,18 @@ private async Task CheckPermissionsForAddAsync( private async Task ProcessAddInstructorAsync( AddInstructorRequest request, + int rotationId, + int gradYear, string? warningMessage, string correlationId, CancellationToken cancellationToken) { // Add instructor to schedule var createdSchedules = await _scheduleEditService.AddInstructorAsync( - request.MothraId!, - request.RotationId!.Value, + request.MothraId, + rotationId, request.WeekIds, - request.GradYear!.Value, + gradYear, request.IsPrimaryEvaluator, cancellationToken); @@ -198,7 +212,7 @@ private async Task ProcessAddInstructorAsync( } _logger.LogInformation("Successfully added instructor to rotation {RotationId} for {WeekCount} weeks (CorrelationId: {CorrelationId})", - request.RotationId!.Value, request.WeekIds.Length, correlationId); + rotationId, request.WeekIds.Length, correlationId); return new AddInstructorResponse { @@ -388,8 +402,18 @@ public async Task SetPrimaryEvaluator( correlationId)); } + // [Required] plus the ModelState check above already guarantee this. Capturing it + // as a non-null local makes every later use provable rather than asserted. + if (request.IsPrimary is not { } isPrimary) + { + return BadRequest(new ErrorResponse( + ErrorCodes.ValidationError, + "Please check your input and try again.", + correlationId)); + } + var (success, previousPrimaryName) = await _scheduleEditService.SetPrimaryEvaluatorAsync( - instructorScheduleId, request.IsPrimary!.Value, cancellationToken, request.RequiresPrimaryEvaluator); + instructorScheduleId, isPrimary, cancellationToken, request.RequiresPrimaryEvaluator); if (!success) { @@ -399,14 +423,14 @@ public async Task SetPrimaryEvaluator( correlationId)); } - var action = request.IsPrimary!.Value ? "set as" : "removed as"; + var action = isPrimary ? "set as" : "removed as"; _logger.LogInformation("Successfully {Action} primary evaluator for instructor schedule {ScheduleId} (CorrelationId: {CorrelationId})", action, instructorScheduleId, correlationId); return Ok(new { message = $"Instructor successfully {action} primary evaluator", - isPrimaryEvaluator = request.IsPrimary!.Value, + isPrimaryEvaluator = isPrimary, previousPrimaryName }); } diff --git a/web/Areas/Computing/Services/BiorenderStudentLookup.cs b/web/Areas/Computing/Services/BiorenderStudentLookup.cs index 4eb6b5821..8aa4d111b 100644 --- a/web/Areas/Computing/Services/BiorenderStudentLookup.cs +++ b/web/Areas/Computing/Services/BiorenderStudentLookup.cs @@ -56,8 +56,10 @@ public async Task> GetBiorenderStudentInfo(List e ); } + // GetSingleStudent returns a constructed BiorenderStudent on every path, so there is + // nothing to filter out here. var taskResults = await Task.WhenAll(resultList); - return taskResults.Where(t => t != null).ToList()!; + return taskResults.ToList(); } /// diff --git a/web/Areas/Effort/Controllers/ClinicalImportController.cs b/web/Areas/Effort/Controllers/ClinicalImportController.cs index cfbdd89fe..7acc85be3 100644 --- a/web/Areas/Effort/Controllers/ClinicalImportController.cs +++ b/web/Areas/Effort/Controllers/ClinicalImportController.cs @@ -131,8 +131,10 @@ public async Task StreamImport( // Create a channel for progress events var channel = Channel.CreateUnbounded(); - // Start the import in a background task. - // Don't pass ct to Task.Run — cancellation is handled cooperatively inside the lambda. + // Start the import in a background task. CancellationToken.None is deliberate: a token + // here only stops the task ever starting, which would skip the lambda's own catch and + // leave the channel un-completed, hanging the SSE reader below. Cancellation is handled + // cooperatively via ct inside the lambda instead. var importTask = Task.Run(async () => { try @@ -160,7 +162,7 @@ public async Task StreamImport( { channel.Writer.TryComplete(); } - }); + }, CancellationToken.None); try { diff --git a/web/Areas/Effort/Controllers/PercentRolloverController.cs b/web/Areas/Effort/Controllers/PercentRolloverController.cs index a411f42c3..a3d91af88 100644 --- a/web/Areas/Effort/Controllers/PercentRolloverController.cs +++ b/web/Areas/Effort/Controllers/PercentRolloverController.cs @@ -91,8 +91,10 @@ public async Task StreamRollover(int year, CancellationToken ct) // Create a channel for progress events var channel = Channel.CreateUnbounded(); - // Start the rollover in a background task. - // Don't pass ct to Task.Run — cancellation is handled cooperatively inside the lambda. + // Start the rollover in a background task. CancellationToken.None is deliberate: a token + // here only stops the task ever starting, which would skip the lambda's own catch and + // leave the channel un-completed, hanging the SSE reader below. Cancellation is handled + // cooperatively via ct inside the lambda instead. var rolloverTask = Task.Run(async () => { try @@ -120,7 +122,7 @@ public async Task StreamRollover(int year, CancellationToken ct) { channel.Writer.TryComplete(); } - }); + }, CancellationToken.None); try { diff --git a/web/Areas/Effort/Services/ClinicalImportService.cs b/web/Areas/Effort/Services/ClinicalImportService.cs index a4dc4b301..10bd7eb54 100644 --- a/web/Areas/Effort/Services/ClinicalImportService.cs +++ b/web/Areas/Effort/Services/ClinicalImportService.cs @@ -128,8 +128,8 @@ public async Task ValidateImportableInstructorsAsync( // (emeritus/recall appointments are excluded from harvest). var titleCodeByMothraId = aaudImportInfo .Where(a => !string.IsNullOrEmpty(a.TitleCode?.Trim()) - && _titleLookup.ContainsKey(a.TitleCode!.Trim()) - && !excludedTitleCodes.Contains(a.TitleCode!.Trim())) + && _titleLookup.ContainsKey(a.TitleCode.Trim()) + && !excludedTitleCodes.Contains(a.TitleCode.Trim())) .GroupBy(a => a.MothraId, StringComparer.OrdinalIgnoreCase) .ToDictionary(g => g.Key, g => g.First().TitleCode!.Trim(), StringComparer.OrdinalIgnoreCase); @@ -265,9 +265,11 @@ public async Task ExecuteImportAsync(int termCode, Clin await _context.SaveChangesAsync(ct); - if (ownsTransaction) + // Null-check the transaction rather than ownsTransaction: identical by construction + // above, but this is the form the compiler can actually prove. + if (transaction != null) { - await transaction!.CommitAsync(ct); + await transaction.CommitAsync(ct); } return result; @@ -341,9 +343,11 @@ public async Task ExecuteImportWithProgressAsync( await _context.SaveChangesAsync(ct); - if (ownsTransaction) + // Null-check the transaction rather than ownsTransaction: identical by construction + // above, but this is the form the compiler can actually prove. + if (transaction != null) { - await transaction!.CommitAsync(ct); + await transaction.CommitAsync(ct); } _logger.LogInformation( diff --git a/web/Areas/Effort/Services/ClinicalScheduleService.cs b/web/Areas/Effort/Services/ClinicalScheduleService.cs index 4b56245cb..e6959ec46 100644 --- a/web/Areas/Effort/Services/ClinicalScheduleService.cs +++ b/web/Areas/Effort/Services/ClinicalScheduleService.cs @@ -150,7 +150,11 @@ private async Task> QueryClinicalScheduleAsync( paramNames.Add(paramName); command.Parameters.AddWithValue(paramName, clinicalJobCodes[i]); } + // The interpolation injects only the "@jc0, @jc1, ..." placeholder names generated + // above; every job code value is bound through AddWithValue, never concatenated. +#pragma warning disable S2077 // Formatting SQL queries is security-sensitive command.CommandText = $"SELECT DISTINCT emplid FROM pps.dbo.vw_personJobPosition WHERE jobcode IN ({string.Join(", ", paramNames)})"; +#pragma warning restore S2077 await using var reader = await command.ExecuteReaderAsync(ct); while (await reader.ReadAsync(ct)) diff --git a/web/Areas/Effort/Services/Harvest/ClinicalHarvestPhase.cs b/web/Areas/Effort/Services/Harvest/ClinicalHarvestPhase.cs index fbba63a8a..151a21d3a 100644 --- a/web/Areas/Effort/Services/Harvest/ClinicalHarvestPhase.cs +++ b/web/Areas/Effort/Services/Harvest/ClinicalHarvestPhase.cs @@ -236,7 +236,7 @@ private async Task BuildClinicalInstructorPreviewsAsync( { // Only build previews for importable instructors (those with valid AAUD data + title code) var clinicalMothraIds = clinicalPersonData - .Where(c => !string.IsNullOrEmpty(c.MothraId) && importableMothraIds.Contains(c.MothraId!)) + .Where(c => !string.IsNullOrEmpty(c.MothraId) && importableMothraIds.Contains(c.MothraId)) .Select(c => c.MothraId!) .Distinct() .ToList(); diff --git a/web/Areas/Effort/Services/Harvest/CrestHarvestPhase.cs b/web/Areas/Effort/Services/Harvest/CrestHarvestPhase.cs index f9279c21b..26b4b880f 100644 --- a/web/Areas/Effort/Services/Harvest/CrestHarvestPhase.cs +++ b/web/Areas/Effort/Services/Harvest/CrestHarvestPhase.cs @@ -272,7 +272,7 @@ private static async Task> GetCrestInstructorsAsync( .Select(i => new { i.IdsMothraid, i.IdsPKey }) .ToListAsync(ct); - var pKeys = aaudIds.Where(i => !string.IsNullOrEmpty(i.IdsPKey)).Select(i => i.IdsPKey!).Distinct().ToList(); + var pKeys = aaudIds.Where(i => !string.IsNullOrEmpty(i.IdsPKey)).Select(i => i.IdsPKey).Distinct().ToList(); var employees = await context.AaudContext.Employees .AsNoTracking() @@ -311,7 +311,7 @@ private static async Task> GetCrestInstructorsAsync( var mothraIdToPKey = aaudIds .Where(i => !string.IsNullOrEmpty(i.IdsMothraid) && !string.IsNullOrEmpty(i.IdsPKey)) .GroupBy(i => i.IdsMothraid!, StringComparer.OrdinalIgnoreCase) - .ToDictionary(g => g.Key, g => g.Select(i => i.IdsPKey!).ToList(), StringComparer.OrdinalIgnoreCase); + .ToDictionary(g => g.Key, g => g.Select(i => i.IdsPKey).ToList(), StringComparer.OrdinalIgnoreCase); // Step 4: Build instructor details var instructorDetails = new List(); diff --git a/web/Areas/Effort/Services/InstructorService.cs b/web/Areas/Effort/Services/InstructorService.cs index 301c41333..28c6120e6 100644 --- a/web/Areas/Effort/Services/InstructorService.cs +++ b/web/Areas/Effort/Services/InstructorService.cs @@ -802,7 +802,7 @@ public async Task> BatchResolveDepartmentsAsync( var mothraIdToPKey = idsRecords .Where(i => !string.IsNullOrEmpty(i.IdsMothraid) && !string.IsNullOrEmpty(i.IdsPKey)) .GroupBy(i => i.IdsMothraid!, StringComparer.OrdinalIgnoreCase) - .ToDictionary(g => g.Key, g => g.First().IdsPKey!, StringComparer.OrdinalIgnoreCase); + .ToDictionary(g => g.Key, g => g.First().IdsPKey, StringComparer.OrdinalIgnoreCase); var pKeys = mothraIdToPKey.Values.Distinct().ToList(); @@ -1357,7 +1357,7 @@ public async Task> GetInstructorEffortRecordsAsy .Where(cr => cr.ChildCourse != null) .Select(cr => new ChildCourseDto { - Id = cr.ChildCourse!.Id, + Id = cr.ChildCourse.Id, SubjCode = cr.ChildCourse.SubjCode, CrseNumb = cr.ChildCourse.CrseNumb, SeqNumb = cr.ChildCourse.SeqNumb, diff --git a/web/Areas/RAPS/Services/OuGroupService.cs b/web/Areas/RAPS/Services/OuGroupService.cs index ea5ab6292..e2e10006d 100644 --- a/web/Areas/RAPS/Services/OuGroupService.cs +++ b/web/Areas/RAPS/Services/OuGroupService.cs @@ -251,7 +251,7 @@ private async Task CompareToGroup(string groupName, List members) //Create a lookup of loginids of members that should be in the group Dictionary membersInRoles = members .Where(m => m.LoginId != null) - .Select(m => m.LoginId!) + .Select(m => m.LoginId) .Distinct() .ToDictionary(id => id, _ => true); diff --git a/web/Areas/RAPS/Services/UinformService.cs b/web/Areas/RAPS/Services/UinformService.cs index b99d4cf04..03bf17213 100644 --- a/web/Areas/RAPS/Services/UinformService.cs +++ b/web/Areas/RAPS/Services/UinformService.cs @@ -250,9 +250,9 @@ private static string GetAuthSignature(HttpMethod method, string publicKey, int { string toSign = method.Method.ToUpper() + ":" + epochTime + ":" + publicKey; // Legacy API requires HMACSHA1 - third-party system constraint -#pragma warning disable CA5350 // Do Not Use Weak Cryptographic Algorithms +#pragma warning disable CA5350, S4790 // Do Not Use Weak Cryptographic Algorithms using var sha1 = new HMACSHA1(Encoding.ASCII.GetBytes(privateKey)); -#pragma warning restore CA5350 +#pragma warning restore CA5350, S4790 byte[] hashed = sha1.ComputeHash(Encoding.ASCII.GetBytes(toSign)); return Convert.ToBase64String(hashed); } diff --git a/web/Areas/Students/Services/StudentList.cs b/web/Areas/Students/Services/StudentList.cs index e320fef73..32228eeeb 100644 --- a/web/Areas/Students/Services/StudentList.cs +++ b/web/Areas/Students/Services/StudentList.cs @@ -57,11 +57,16 @@ public async Task> GetStudents(string? classLevel = null, int? cla //include all class years, as long as at least one of them is the given year else if (classYear != null) { - //get all students that have a class year entry in this year - q = q.Where(q => q.Student != null && _context.StudentClassYears - .Where(anyClassYear => anyClassYear.ClassYear == classYear && anyClassYear.PersonId == q.Student.PersonId) - .Any() - ); + //get all students that have a class year entry in this year. Pre-loading the ids + //keeps this a single IN list instead of a subquery correlated to every outer row. + var personIdsInClassYear = await _context.StudentClassYears + .AsNoTracking() + .Where(cy => cy.ClassYear == classYear) + .Select(cy => cy.PersonId) + .Distinct() + .ToListAsync(); + q = q.Where(studentClassYear => studentClassYear.Student != null + && EF.Parameter(personIdsInClassYear).Contains(studentClassYear.Student.PersonId)); } if (personId != null) diff --git a/web/Classes/SitemapMiddleware.cs b/web/Classes/SitemapMiddleware.cs index 6c5f00556..0ae70bbb1 100644 --- a/web/Classes/SitemapMiddleware.cs +++ b/web/Classes/SitemapMiddleware.cs @@ -80,11 +80,18 @@ public async Task Invoke(HttpContext context) using (var memoryStream = new MemoryStream()) { var bytes = Encoding.UTF8.GetBytes(sitemapContent.ToString()); - await memoryStream.WriteAsync(bytes.AsMemory()); + await memoryStream.WriteAsync(bytes.AsMemory(), context.RequestAborted); memoryStream.Seek(0, SeekOrigin.Begin); - await memoryStream.CopyToAsync(stream, bytes.Length); + await memoryStream.CopyToAsync(stream, bytes.Length, context.RequestAborted); } } + // A disconnected client is not a generation failure. The response has + // already started by this point, so end the request instead of running + // the rest of the pipeline against it. + catch (OperationCanceledException) when (context.RequestAborted.IsCancellationRequested) + { + // Intentionally no fall-through to _next: the request is over. + } // Middleware boundary: any sitemap-generation failure (DB, IO, // reflection, etc.) must fall through to the pipeline rather than // break the request. diff --git a/web/Controllers/HomeController.cs b/web/Controllers/HomeController.cs index 65769ae8e..1f7d613b7 100644 --- a/web/Controllers/HomeController.cs +++ b/web/Controllers/HomeController.cs @@ -29,7 +29,11 @@ public class HomeController : AreaController private readonly AAUDContext _aAUDContext; private readonly RAPSContext _rapsContext; private readonly VIPERContext _viperContext; + // An XML namespace identifier, not a network endpoint. The scheme is part of the + // literal CAS responses are namespaced with; changing it stops the elements matching. +#pragma warning disable S5332 // Using http protocol is insecure private readonly XNamespace _ns = "http://www.yale.edu/tp/cas"; +#pragma warning restore S5332 private readonly IHttpClientFactory _clientFactory; private readonly CasSettings _settings; private readonly List _casAttributesToCapture = new() { "authenticationDate", "credentialType" }; @@ -210,7 +214,12 @@ public IActionResult ClearCache() [Route("/[action]")] [Route("/[action]/{statusCode:int}")] [AllowAnonymous] + // Anti-forgery is irrelevant here: the error page is anonymous, binds one int? route + // value, and mutates no state. Requiring a token would break the 404/500 handler, + // which is re-executed on requests that never carried one. +#pragma warning disable S4502 // Disabling CSRF protections is security-sensitive [IgnoreAntiforgeryToken] +#pragma warning restore S4502 [ResponseCache(Duration = 0, Location = ResponseCacheLocation.None, NoStore = true)] [SearchExclude] #pragma warning disable S6967 // Error handler uses simple route parameter, not form data requiring validation diff --git a/web/ViteProxyHelpers.cs b/web/ViteProxyHelpers.cs index a1f8a00d7..81058dce1 100644 --- a/web/ViteProxyHelpers.cs +++ b/web/ViteProxyHelpers.cs @@ -319,7 +319,17 @@ public static async Task CopyProxyResponse(HttpContext context, HttpResponseMess } } - await response.Content.CopyToAsync(context.Response.Body); + try + { + await response.Content.CopyToAsync(context.Response.Body, context.RequestAborted); + } + catch (OperationCanceledException) when (context.RequestAborted.IsCancellationRequested) + { + // The client disconnected mid-stream. Swallow rather than rethrow: the caller + // catches TaskCanceledException as "Vite dev server unavailable" and falls + // through to the static-file branch, which would both log the wrong cause and + // write to a response that has already started. + } } /// @@ -392,7 +402,7 @@ public static async Task HandleProxyError(HttpContext context, Exception ex, ILo var contentType = GetContentType(Path.GetExtension(resolvedPhysical)); // Set content type if not already started context.Response.ContentType = contentType ?? "application/octet-stream"; - await context.Response.SendFileAsync(resolvedPhysical); + await context.Response.SendFileAsync(resolvedPhysical, context.RequestAborted); return; } } @@ -403,7 +413,7 @@ public static async Task HandleProxyError(HttpContext context, Exception ex, ILo } context.Response.StatusCode = 502; - await context.Response.WriteAsync("Vite dev server not running. Please start the frontend development server or build static files as appropriate."); + await context.Response.WriteAsync("Vite dev server not running. Please start the frontend development server or build static files as appropriate.", context.RequestAborted); } }