Fix SbomValidator.ValidateSbomAsync false positive when outputPath is directory

src/Microsoft.Sbom.Api/Output/Telemetry/IRecorder.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Microsoft.Sbom.Api.Entities;
+using Microsoft.Sbom.Api.Entities.Output;
 using Microsoft.Sbom.Extensions.Entities;
 
 namespace Microsoft.Sbom.Api.Output.Telemetry;
@@ -100,4 +101,9 @@ public interface IRecorder
     /// Record telemetry for an AggregationSource.
     /// </summary>
     public void RecordAggregationSource(string identifier, int packageCount, int relationshipCount);
+
+    /// <summary>
+    /// Gets the result status of the recorder execution.
+    /// </summary>
+    public Result Result { get; }
 }

src/Microsoft.Sbom.Api/Output/Telemetry/TelemetryRecorder.cs

@@ -122,6 +122,8 @@ public async Task LogException(Exception exception)
 
     public IList<FileValidationResult> Errors => errors;
 
+    public Result Result => result;
+
     /// <summary>
     /// Start recording the duration of exeuction of the given event.
     /// </summary>

src/Microsoft.Sbom.Api/SbomValidator.cs

@@ -9,6 +9,7 @@
 using System.Threading.Tasks;
 using Microsoft.Sbom.Api.Config;
 using Microsoft.Sbom.Api.Config.Extensions;
+using Microsoft.Sbom.Api.Entities.Output;
 using Microsoft.Sbom.Api.Output.Telemetry;
 using Microsoft.Sbom.Api.Workflows;
 using Microsoft.Sbom.Common;
@@ -104,7 +105,7 @@ public async Task<SbomValidationResult> ValidateSbomAsync(
         await recorder.FinalizeAndLogTelemetryAsync();
 
         var errors = recorder.Errors.Select(error => error.ToEntityError()).ToList();
-        return new SbomValidationResult(!errors.Any(), errors);
+        return new SbomValidationResult(recorder.Result == Result.Success, errors);
     }
 
     private InputConfiguration ValidateConfig(InputConfiguration config)

test/Microsoft.Sbom.Api.Tests/Output/Telemetry/TelemetryRecorderTests.cs

@@ -91,4 +91,36 @@ public void RecordAggregationSource_Duplicate_Throws()
             telemetryRecorder.RecordAggregationSource(testKey, testPackageCount, testRelationshipCount);
         });
     }
+
+    [TestMethod]
+    public void TelemetryRecorder_RecordException_StoresExceptionsCorrectly()
+    {
+        var telemetryRecorder = new TelemetryRecorder(fileSystemUtilsMock.Object, configMock.Object, loggerMock.Object);
+        var testException1 = new InvalidOperationException("Test exception 1");
+        var testException2 = new ArgumentException("Test exception 2");
+
+        // Use reflection to access the private exceptions field
+        var exceptionsField = typeof(TelemetryRecorder).GetField("exceptions", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
+        var exceptions = (IList<Exception>)exceptionsField.GetValue(telemetryRecorder);
+
+        Assert.AreEqual(0, exceptions.Count);
+
+        telemetryRecorder.RecordException(testException1);
+        telemetryRecorder.RecordException(testException2);
+
+        Assert.AreEqual(2, exceptions.Count);
+        Assert.AreEqual(testException1, exceptions[0]);
+        Assert.AreEqual(testException2, exceptions[1]);
+    }
+
+    [TestMethod]
+    public void TelemetryRecorder_RecordException_NullException_Throws()
+    {
+        var telemetryRecorder = new TelemetryRecorder(fileSystemUtilsMock.Object, configMock.Object, loggerMock.Object);
+
+        Assert.ThrowsException<ArgumentNullException>(() =>
+        {
+            telemetryRecorder.RecordException(null);
+        });
+    }
 }

test/Microsoft.Sbom.Api.Tests/SbomValidatorTests.cs

@@ -0,0 +1,167 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.Sbom.Api.Entities;
+using Microsoft.Sbom.Api.Entities.Output;
+using Microsoft.Sbom.Api.Output.Telemetry;
+using Microsoft.Sbom.Api.Workflows;
+using Microsoft.Sbom.Common;
+using Microsoft.Sbom.Common.Config;
+using Microsoft.Sbom.Common.Config.Validators;
+using Microsoft.Sbom.Contracts;
+using Microsoft.Sbom.Extensions;
+using Microsoft.Sbom.Extensions.Entities;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
+using Constants = Microsoft.Sbom.Api.Utils.Constants;
+
+namespace Microsoft.Sbom.Api.Tests;
+
+[TestClass]
+public class SbomValidatorTests
+{
+    private Mock<IWorkflow<SbomParserBasedValidationWorkflow>> workflowMock;
+    private Mock<IRecorder> recorderMock;
+    private Mock<IEnumerable<ConfigValidator>> configValidatorsMock;
+    private Mock<IConfiguration> configurationMock;
+    private Mock<ISbomConfigProvider> sbomConfigProviderMock;
+    private Mock<IFileSystemUtils> fileSystemUtilsMock;
+    private Mock<ISbomConfig> sbomConfigMock;
+    private SbomValidator sbomValidator;
+
+    // Common test data
+    private readonly string buildDropPath = "/test/drop";
+    private readonly string outputPathFile = "/test/output.json";
+    private readonly string outputPathDirectory = "/test/output";
+    private readonly List<SbomSpecification> specifications = new List<SbomSpecification> { new SbomSpecification("SPDX", "2.2") };
+    private readonly string manifestDirPath = "/test/manifest";
+    private readonly ManifestInfo manifestInfo = Constants.TestManifestInfo;
+    private readonly string manifestJsonPath = "/test/manifest/manifest.json";
+
+    [TestInitialize]
+    public void Init()
+    {
+        workflowMock = new Mock<IWorkflow<SbomParserBasedValidationWorkflow>>(MockBehavior.Strict);
+        recorderMock = new Mock<IRecorder>(MockBehavior.Strict);
+        configValidatorsMock = new Mock<IEnumerable<ConfigValidator>>(MockBehavior.Strict);
+        configurationMock = new Mock<IConfiguration>(MockBehavior.Strict);
+        sbomConfigProviderMock = new Mock<ISbomConfigProvider>(MockBehavior.Strict);
+        fileSystemUtilsMock = new Mock<IFileSystemUtils>(MockBehavior.Strict);
+        sbomConfigMock = new Mock<ISbomConfig>(MockBehavior.Strict);
+
+        sbomValidator = new SbomValidator(
+            workflowMock.Object,
+            recorderMock.Object,
+            configValidatorsMock.Object,
+            configurationMock.Object,
+            sbomConfigProviderMock.Object,
+            fileSystemUtilsMock.Object);
+    }
+
+    [TestCleanup]
+    public void AfterEachTest()
+    {
+        workflowMock.VerifyAll();
+        recorderMock.VerifyAll();
+        configValidatorsMock.VerifyAll();
+        configurationMock.VerifyAll();
+        sbomConfigProviderMock.VerifyAll();
+        fileSystemUtilsMock.VerifyAll();
+        sbomConfigMock.VerifyAll();
+    }
+
+    private void SetupMocksForValidation(List<FileValidationResult> errors, List<Exception> exceptions)
+    {
+        configValidatorsMock.Setup(cv => cv.GetEnumerator()).Returns(new List<ConfigValidator>().GetEnumerator());
+
+        configurationMock.Setup(c => c.ManifestInfo).Returns(new ConfigurationSetting<IList<ManifestInfo>>
+        {
+            Value = new List<ManifestInfo> { manifestInfo }
+        });
+
+        sbomConfigProviderMock.Setup(scp => scp.Get(manifestInfo)).Returns(sbomConfigMock.Object);
+        sbomConfigMock.Setup(sc => sc.ManifestJsonFilePath).Returns(manifestJsonPath);
+
+        fileSystemUtilsMock.Setup(fs => fs.FileExists(manifestJsonPath)).Returns(true);
+        workflowMock.Setup(w => w.RunAsync()).ReturnsAsync(true);
+
+        recorderMock.Setup(r => r.FinalizeAndLogTelemetryAsync()).Returns(Task.CompletedTask);
+        recorderMock.Setup(r => r.Errors).Returns(errors);
+
+        // Determine the result based on whether there are errors or exceptions
+        var result = (errors.Any() || exceptions.Any()) ? Result.Failure : Result.Success;
+        recorderMock.Setup(r => r.Result).Returns(result);
+    }
+
+    [TestMethod]
+    public async Task ValidateSbomAsync_WithNoErrorsAndNoExceptions_ReturnsTrue()
+    {
+        var errors = new List<FileValidationResult>();
+        var exceptions = new List<Exception>();
+
+        SetupMocksForValidation(errors, exceptions);
+
+        var result = await sbomValidator.ValidateSbomAsync(buildDropPath, outputPathFile, specifications, manifestDirPath);
+
+        Assert.IsTrue(result.IsSuccess);
+        Assert.AreEqual(0, result.Errors.Count);
+    }
+
+    [TestMethod]
+    public async Task ValidateSbomAsync_WithErrorsButNoExceptions_ReturnsFalse()
+    {
+        var errors = new List<FileValidationResult>
+        {
+            new FileValidationResult { ErrorType = ErrorType.MissingFile, Path = "/test/missing.txt" }
+        };
+        var exceptions = new List<Exception>();
+
+        SetupMocksForValidation(errors, exceptions);
+
+        var result = await sbomValidator.ValidateSbomAsync(buildDropPath, outputPathFile, specifications, manifestDirPath);
+
+        Assert.IsFalse(result.IsSuccess);
+        Assert.AreEqual(1, result.Errors.Count);
+    }
+
+    [TestMethod]
+    public async Task ValidateSbomAsync_WithNoErrorsButWithExceptions_ReturnsFalse()
+    {
+        var errors = new List<FileValidationResult>();
+        var exceptions = new List<Exception>
+        {
+            new InvalidOperationException("Cannot write to directory path")
+        };
+
+        SetupMocksForValidation(errors, exceptions);
+
+        var result = await sbomValidator.ValidateSbomAsync(buildDropPath, outputPathDirectory, specifications, manifestDirPath);
+
+        Assert.IsFalse(result.IsSuccess);
+        Assert.AreEqual(0, result.Errors.Count); // No validation errors, but should still fail due to exception
+    }
+
+    [TestMethod]
+    public async Task ValidateSbomAsync_WithBothErrorsAndExceptions_ReturnsFalse()
+    {
+        var errors = new List<FileValidationResult>
+        {
+            new FileValidationResult { ErrorType = ErrorType.MissingFile, Path = "/test/missing.txt" }
+        };
+        var exceptions = new List<Exception>
+        {
+            new InvalidOperationException("Cannot write to directory path")
+        };
+
+        SetupMocksForValidation(errors, exceptions);
+
+        var result = await sbomValidator.ValidateSbomAsync(buildDropPath, outputPathDirectory, specifications, manifestDirPath);
+
+        Assert.IsFalse(result.IsSuccess);
+        Assert.AreEqual(1, result.Errors.Count);
+    }
+}

test/Microsoft.Sbom.Api.Tests/VersionSpecificPins/Version_4_0/InterfaceConcretionTests.cs

@@ -15,6 +15,7 @@
 using Microsoft.Sbom.Api.Config.Args;
 using Microsoft.Sbom.Api.Convertors;
 using Microsoft.Sbom.Api.Entities;
+using Microsoft.Sbom.Api.Entities.Output;
 using Microsoft.Sbom.Api.Executors;
 using Microsoft.Sbom.Api.Filters;
 using Microsoft.Sbom.Api.FormatValidator;
@@ -128,6 +129,8 @@ private class PinnedIRecorder : IRecorder
     {
         public IList<FileValidationResult> Errors => throw new NotImplementedException();
 
+        public Result Result => throw new NotImplementedException();
+
         public void AddResult(string propertyName, string value) => throw new NotImplementedException();
         public void AddToTotalCountOfLicenses(int count) => throw new NotImplementedException();
         public void AddToTotalNumberOfPackageDetailsEntries(int count) => throw new NotImplementedException();