DRAFT "Stuck in validation" fix

src/NuGet.Services.Contracts/Validation/ValidationStatus.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 namespace NuGet.Services.Validation
@@ -29,5 +29,12 @@ public enum ValidationStatus
         /// out, or the logic of the validation has discovered an issue with the entity that is being validated.
         /// </summary>
         Failed = 3,
+
+        /// <summary>
+        /// The validation step has identified the package as malicious. Unlike <see cref="Failed"/>, this status
+        /// does not trigger the standard validation failure workflow and is not counted as a stuck validation
+        /// by monitoring. The package remains in the validating state pending further review.
+        /// </summary>
+        Malicious = 4,
     }
-}
+}

src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -148,11 +148,13 @@ await ScheduleCheckIfNotTimedOut(
             }
             else
             {
+                // Malicious packages are deliberately held in the validating state — do not send
+                // "taking too long" notifications or count them as stuck in validation.
                 await ScheduleCheckIfNotTimedOut(
                     validationSet,
                     validatingEntity,
                     scheduleNextCheck,
-                    tooLongNotificationAllowed: true);
+                    tooLongNotificationAllowed: !HasMaliciousValidation(validationSet));
             }
         }
 
@@ -205,7 +207,8 @@ private bool AreOptionalValidationsRunning(PackageValidationSet packageValidatio
         {
             return packageValidationSet
                 .PackageValidations
-                .Any(pv => pv.ValidationStatus == ValidationStatus.Incomplete
+                .Any(pv => (pv.ValidationStatus == ValidationStatus.Incomplete
+                        || pv.ValidationStatus == ValidationStatus.Malicious)
                     && GetValidationConfigurationItemByName(pv.Type)?.FailureBehavior == ValidationFailureBehavior.AllowedToFail);
         }
 
@@ -228,6 +231,7 @@ bool IsPackageValidationTimedOut(PackageValidation validation)
                 return duration > config?.TrackAfter;
             }
 
+            // Malicious is intentionally excluded — it is not counted as stuck in validation.
             return packageValidationSet
                 .PackageValidations
                 .Where(v => v.ValidationStatus == ValidationStatus.Incomplete)
@@ -321,8 +325,25 @@ private async Task ScheduleCheckIfNotTimedOut(
                     validationSet.PackageNormalizedVersion,
                     validationSetDuration,
                     _validationConfiguration.TimeoutValidationSetAfter);
-                _telemetryService.TrackValidationSetTimeout(validationSet.PackageId, validationSet.PackageNormalizedVersion, validationSet.ValidationTrackingId);
+
+                // Do not fire the timeout metric for malicious packages — they are intentionally
+                // held in the validating state and must not be counted as stuck in validation.
+                if (!HasMaliciousValidation(validationSet))
+                {
+                    _telemetryService.TrackValidationSetTimeout(validationSet.PackageId, validationSet.PackageNormalizedVersion, validationSet.ValidationTrackingId);
+                }
             }
         }
+
+        /// <summary>
+        /// Returns <c>true</c> if any validation in the set has the
+        /// <see cref="ValidationStatus.Malicious"/> status.
+        /// </summary>
+        private bool HasMaliciousValidation(PackageValidationSet packageValidationSet)
+        {
+            return packageValidationSet
+                .PackageValidations
+                .Any(pv => pv.ValidationStatus == ValidationStatus.Malicious);
+        }
     }
 }

src/NuGet.Services.Validation.Orchestrator/ValidationSetProcessor.cs

@@ -100,7 +100,8 @@ public async Task<ValidationSetProcessorResult> ForceFailValidationSetAsync(Pack
 
         private async Task ProcessIncompleteValidations(PackageValidationSet validationSet, ValidationSetProcessorResult processorStats)
         {
-            foreach (var packageValidation in validationSet.PackageValidations.Where(v => v.ValidationStatus == ValidationStatus.Incomplete))
+            foreach (var packageValidation in validationSet.PackageValidations.Where(v => v.ValidationStatus == ValidationStatus.Incomplete
+                || v.ValidationStatus == ValidationStatus.Malicious))
             {
                 using (_logger.BeginScope("Incomplete {ValidationType} Key {ValidationId}", packageValidation.Type, packageValidation.Key))
                 {
@@ -121,7 +122,8 @@ private async Task ProcessIncompleteValidations(PackageValidationSet validationS
                     var validationRequest = await CreateNuGetValidationRequest(packageValidation.PackageValidationSet, packageValidation);
                     var validationResponse = await validator.GetResponseAsync(validationRequest);
 
-                    if (validationResponse.Status != ValidationStatus.Incomplete)
+                    if (validationResponse.Status != ValidationStatus.Incomplete
+                        && validationResponse.Status != ValidationStatus.Malicious)
                     {
                         _logger.LogInformation(
                             "New status for validation {ValidationType} for {PackageId} {PackageVersion} is " +
@@ -151,6 +153,12 @@ private async Task ProcessIncompleteValidations(PackageValidationSet validationS
                         case ValidationStatus.Incomplete:
                             break;
 
+                        case ValidationStatus.Malicious:
+                            // Persist the malicious status so the DB reflects it, but do not clean up or
+                            // count this as a success — the package remains in the validating state.
+                            await _validationStorageService.UpdateValidationStatusAsync(packageValidation, validationResponse);
+                            break;
+
                         case ValidationStatus.Failed:
                             await _validationStorageService.UpdateValidationStatusAsync(packageValidation, validationResponse);
                             await validator.CleanUpAsync(validationRequest);

src/NuGet.Services.Validation.Orchestrator/ValidationStorageService.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -149,7 +149,8 @@ private async Task SetValidationStatusAsync(
             INuGetValidationResponse validationResponse,
             DateTime now)
         {
-            if (validationResponse.Status != ValidationStatus.Incomplete)
+            if (validationResponse.Status != ValidationStatus.Incomplete
+                && validationResponse.Status != ValidationStatus.Malicious)
             {
                 AddValidationIssues(packageValidation, validationResponse.Issues);
             }

src/NuGetGallery/Services/ValidationService.cs

@@ -81,6 +81,19 @@ public bool IsValidatingTooLong(Package package)
         {
             if (package.PackageStatusKey == PackageStatus.Validating)
             {
+                // Malicious packages are deliberately held in the validating state — do not
+                // count them as validating too long.
+                var hasMaliciousValidation = _validationSets?
+                    .GetAll()
+                    .Any(s => s.PackageKey == package.Key
+                        && s.ValidatingType == ValidatingType.Package
+                        && s.PackageValidations.Any(v => v.ValidationStatus == ValidationStatus.Malicious));
+
+                if (hasMaliciousValidation == true)
+                {
+                    return false;
+                }
+
                 return ((DateTime.UtcNow - package.Created) >= _appConfiguration.ValidationExpectedTime);
             }
 

src/Validation.Common.Job/Validation/NuGetValidationResponse.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -28,6 +28,13 @@ public class NuGetValidationResponse : INuGetValidationResponse
         /// </summary>
         public static INuGetValidationResponse Failed { get; } = new NuGetValidationResponse(ValidationStatus.Failed);
 
+        /// <summary>
+        /// Represents a validation step that has identified the package as malicious and is still in
+        /// progress. Treated the same as <see cref="Incomplete"/> for monitoring purposes — the package
+        /// is not counted as stuck in validation.
+        /// </summary>
+        public static INuGetValidationResponse Malicious { get; } = new NuGetValidationResponse(ValidationStatus.Malicious);
+
         /// <summary>
         /// Create a new validation step response with the given status.
         /// </summary>
@@ -127,4 +134,4 @@ public static NuGetValidationResponse FailedWithIssues(params IValidationIssue[]
             return new NuGetValidationResponse(ValidationStatus.Failed, (IValidationIssue[])issues.Clone());
         }
     }
-}
+}

tests/CatalogTests/Helpers/UtilsTests.cs

@@ -150,7 +150,7 @@ public void GetNupkgMetadata_WhenNuspecAtRootAndInSubdirectory_UsesRootNuspec()
                     var subdirEntry = zipArchive.CreateEntry("subdir\\malicious.nuspec");
                     using (var writer = new StreamWriter(subdirEntry.Open()))
                     {
-                        writer.Write("<?xml version=\"1.0\"?><package xmlns=\"http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd\"><metadata><id>MaliciousPackage</id><version>2.0.0</version><authors>MaliciousAuthor</authors><description>Malicious Description</description></metadata></package>");
+                        writer.Write("<?xml version=\"1.0\"?><package xmlns=\"http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd\"><metadata><id>Malicious</id><version>2.0.0</version><authors>MaliciousAuthor</authors><description>Malicious Description</description></metadata></package>");
                     }
 
                     var rootEntry = zipArchive.CreateEntry("package.nuspec");

tests/NuGet.Services.Contracts.Tests/Validation/ValidationStatusFacts.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -16,10 +16,11 @@ public class ValidationStatusFacts
         [InlineData(1, ValidationStatus.Incomplete)]
         [InlineData(2, ValidationStatus.Succeeded)]
         [InlineData(3, ValidationStatus.Failed)]
+        [InlineData(4, ValidationStatus.Malicious)]
         public void HasUnchangingValues(int expected, ValidationStatus input)
         {
             Assert.Equal(expected, (int)input);
-            Assert.Equal(4, Enum.GetValues(typeof(ValidationStatus)).Length);
+            Assert.Equal(5, Enum.GetValues(typeof(ValidationStatus)).Length);
         }
     }
 }

tests/NuGet.Services.Validation.Orchestrator.Tests/ValidationOutcomeProcessorFacts.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -566,7 +566,9 @@ public async Task SendsTooLongNotificationOnlyWhenItConcernsRequiredValidation(
             ValidationStatus optionalValidationState,
             bool requiredValidationSucceeded)
         {
-            bool expectedNotification = requiredValidationState == ValidationStatus.Incomplete || requiredValidationState == ValidationStatus.NotStarted;
+            bool expectedNotification = (requiredValidationState == ValidationStatus.Incomplete || requiredValidationState == ValidationStatus.NotStarted)
+    && requiredValidationState != ValidationStatus.Malicious    
+    && optionalValidationState != ValidationStatus.Malicious;
 
             AddValidation("requiredValidation", requiredValidationState, ValidationFailureBehavior.MustSucceed);
             AddValidation("optionalValidaiton", optionalValidationState, ValidationFailureBehavior.AllowedToFail);