diff --git a/src/NuGet.Services.Contracts/Validation/ValidationStatus.cs b/src/NuGet.Services.Contracts/Validation/ValidationStatus.cs index 6c856fd08c..93a97f542c 100644 --- a/src/NuGet.Services.Contracts/Validation/ValidationStatus.cs +++ b/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. /// Failed = 3, + + /// + /// The validation step has identified the package as malicious. Unlike , 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. + /// + Malicious = 4, } -} \ No newline at end of file +} diff --git a/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs b/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs index 35973a63fb..eff9628290 100644 --- a/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs +++ b/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); + } } } + + /// + /// Returns true if any validation in the set has the + /// status. + /// + private bool HasMaliciousValidation(PackageValidationSet packageValidationSet) + { + return packageValidationSet + .PackageValidations + .Any(pv => pv.ValidationStatus == ValidationStatus.Malicious); + } } } diff --git a/src/NuGet.Services.Validation.Orchestrator/ValidationSetProcessor.cs b/src/NuGet.Services.Validation.Orchestrator/ValidationSetProcessor.cs index 853d6a9e54..f7a012b051 100644 --- a/src/NuGet.Services.Validation.Orchestrator/ValidationSetProcessor.cs +++ b/src/NuGet.Services.Validation.Orchestrator/ValidationSetProcessor.cs @@ -100,7 +100,8 @@ public async Task 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); diff --git a/src/NuGet.Services.Validation.Orchestrator/ValidationStorageService.cs b/src/NuGet.Services.Validation.Orchestrator/ValidationStorageService.cs index 7fb186561f..11e9879a1d 100644 --- a/src/NuGet.Services.Validation.Orchestrator/ValidationStorageService.cs +++ b/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); } diff --git a/src/NuGetGallery/Services/ValidationService.cs b/src/NuGetGallery/Services/ValidationService.cs index df8efd5a9a..b731ee650c 100644 --- a/src/NuGetGallery/Services/ValidationService.cs +++ b/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); } diff --git a/src/Validation.Common.Job/Validation/NuGetValidationResponse.cs b/src/Validation.Common.Job/Validation/NuGetValidationResponse.cs index 326dd0343a..b073a2a8cc 100644 --- a/src/Validation.Common.Job/Validation/NuGetValidationResponse.cs +++ b/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 /// public static INuGetValidationResponse Failed { get; } = new NuGetValidationResponse(ValidationStatus.Failed); + /// + /// Represents a validation step that has identified the package as malicious and is still in + /// progress. Treated the same as for monitoring purposes — the package + /// is not counted as stuck in validation. + /// + public static INuGetValidationResponse Malicious { get; } = new NuGetValidationResponse(ValidationStatus.Malicious); + /// /// Create a new validation step response with the given status. /// @@ -127,4 +134,4 @@ public static NuGetValidationResponse FailedWithIssues(params IValidationIssue[] return new NuGetValidationResponse(ValidationStatus.Failed, (IValidationIssue[])issues.Clone()); } } -} \ No newline at end of file +} diff --git a/tests/CatalogTests/Helpers/UtilsTests.cs b/tests/CatalogTests/Helpers/UtilsTests.cs index c59889bbbd..622762a2da 100644 --- a/tests/CatalogTests/Helpers/UtilsTests.cs +++ b/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("MaliciousPackage2.0.0MaliciousAuthorMalicious Description"); + writer.Write("Malicious2.0.0MaliciousAuthorMalicious Description"); } var rootEntry = zipArchive.CreateEntry("package.nuspec"); diff --git a/tests/NuGet.Services.Contracts.Tests/Validation/ValidationStatusFacts.cs b/tests/NuGet.Services.Contracts.Tests/Validation/ValidationStatusFacts.cs index 1f29c5a06b..dc2ddc9982 100644 --- a/tests/NuGet.Services.Contracts.Tests/Validation/ValidationStatusFacts.cs +++ b/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); } } } diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/ValidationOutcomeProcessorFacts.cs b/tests/NuGet.Services.Validation.Orchestrator.Tests/ValidationOutcomeProcessorFacts.cs index 6f583429ca..df34ea1c81 100644 --- a/tests/NuGet.Services.Validation.Orchestrator.Tests/ValidationOutcomeProcessorFacts.cs +++ b/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);