Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,6 @@ PublishScripts/
!**/[Pp]ackages/build/

# MSBuild Guard scoped trust stores (solution/project level)
**/.msbuildguard/trust.json
**/.msbuildguard/trust.json.audit.jsonl
# Uncomment if necessary however generally it will be regenerated when needed
#!**/[Pp]ackages/repositories.config
# NuGet v3's project.json files produces more ignorable files
Expand Down Expand Up @@ -373,3 +371,8 @@ FodyWeavers.xsd
/.msbuildguard
/plans
/MSBuildGuard.VSCode/*.vsix

.msbuildguard
/.msbuildguard/
**/.msbuildguard/trust.json
**/.msbuildguard/trust.json.audit.jsonl
32 changes: 32 additions & 0 deletions MSBuildGuard.Core.Tests/Baseline/BaselineOnboardingServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,5 +131,37 @@ public async Task GenerateSuggestionsAsync_ShouldSetIsAlreadyTrusted_ForAlreadyT
trustStoreService.RemoveDecisionsBySubject(userTrustPath, packageHash, "Clean up", "TestUser");
}
}

/// <summary>
/// Verifies that the recommendation reason for a Signer trust suggestion is formatted correctly.
/// </summary>
[Test]
[Explicit("Requires internet access and local NuGet package cache.")]
public async Task GenerateSuggestionsAsync_ShouldFormatRecommendationReasonForSigner()
{
var service = new BaselineOnboardingService();
var report = new ScanReport();

report.Findings.Add(new Finding
{
Id = "MBG001",
Fingerprint = "fp-1",
PackageId = "System.Text.Json",
PackageVersion = "10.0.7",
FilePath = "somepath"
});

var result = await service.GenerateSuggestionsAsync(report, CancellationToken.None);

result.ShouldNotBeNull();

var signerSuggestion = result.FirstOrDefault(item => item.Scope == TrustSuggestionScope.Signer);

signerSuggestion.ShouldNotBeNull();
signerSuggestion.RecommendationReason.ShouldStartWith("Signer:");
signerSuggestion.RecommendationReason.ShouldContain("System.Text.Json");
signerSuggestion.RecommendationReason.ShouldContain("System.Text.Json.dll");
signerSuggestion.RecommendationReason.ShouldContain(signerSuggestion.DisplayName);
}
}
}
50 changes: 36 additions & 14 deletions MSBuildGuard.Core/Baseline/BaselineOnboardingService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,13 @@ public async Task<List<TrustSuggestion>> GenerateSuggestionsAsync(ScanReport rep
processedPackages.Add(packageKey);

var assemblyPath = AssemblySignatureService.ResolveAssemblyFilePathFromPackageId(packageId, packageVersion);
var assemblyName = !string.IsNullOrEmpty(assemblyPath) ? Path.GetFileName(assemblyPath) : string.Empty;
var signatureInfo = signatureService.ReadSignature(assemblyPath);
Comment thread
Hefaistos68 marked this conversation as resolved.
var reputationInfo = await this.reputationService.GetReputationAsync(packageId, cancellationToken).ConfigureAwait(false);

if (signatureInfo != null && signatureInfo.HasEmbeddedSignature && signatureInfo.IsSignatureValid)
{
var isMicrosoft = signatureInfo.Signer.IndexOf("Microsoft", StringComparison.OrdinalIgnoreCase) >= 0 ||
signatureInfo.Subject.IndexOf("Microsoft", StringComparison.OrdinalIgnoreCase) >= 0;
var isMicrosoft = IsTrustedMicrosoftSigner(signatureInfo);

if (isMicrosoft)
{
Expand All @@ -161,10 +161,10 @@ public async Task<List<TrustSuggestion>> GenerateSuggestionsAsync(ScanReport rep
IsSelected = true,
Scope = TrustSuggestionScope.Signer,
Subject = signatureInfo.Thumbprint,
DisplayName = signatureInfo.Signer,
RecommendationReason = "This package is signed by Microsoft Corporation with a valid Authenticode signature.",
DisplayName = $"{packageId} ({signatureInfo.Signer})", // signatureInfo.Signer,
RecommendationReason = $"Trusted signer: {signatureInfo.Signer}",
ReputationSourceDescription = "Verified Publisher (Microsoft)"
};
};

suggestion.Metadata["SignerThumbprint"] = signatureInfo.Thumbprint;
suggestion.Metadata["SignerSubject"] = signatureInfo.Subject;
Expand All @@ -186,11 +186,11 @@ public async Task<List<TrustSuggestion>> GenerateSuggestionsAsync(ScanReport rep
{
var suggestion = new TrustSuggestion
{
IsSelected = true,
Scope = TrustSuggestionScope.Package,
Subject = packageHash,
DisplayName = $"{packageId} v{packageVersion}",
RecommendationReason = $"Verified package on NuGet.org with very high download volume ({reputationInfo.TotalDownloads:N0} downloads).",
IsSelected = true,
Scope = TrustSuggestionScope.Package,
Subject = packageHash,
DisplayName = $"{packageId} v{packageVersion}",
RecommendationReason = $"Verified package on NuGet.org with very high download volume ({reputationInfo.TotalDownloads:N0} downloads).",
ReputationSourceDescription = "Verified NuGet.org Publisher"
};

Expand All @@ -214,10 +214,10 @@ public async Task<List<TrustSuggestion>> GenerateSuggestionsAsync(ScanReport rep

var suggestion = new TrustSuggestion
{
IsSelected = true,
Scope = TrustSuggestionScope.Signer,
Subject = signatureInfo.Thumbprint,
DisplayName = signatureInfo.Signer,
IsSelected = true,
Scope = TrustSuggestionScope.Signer,
Subject = signatureInfo.Thumbprint,
DisplayName = signatureInfo.Signer,
RecommendationReason = $"Signed by a valid certificate signer: '{signatureInfo.Signer}'.",
ReputationSourceDescription = "Valid Authenticode Signer"
};
Expand Down Expand Up @@ -302,5 +302,27 @@ private static bool IsSuggestionAlreadyTrusted(

return false;
}

/// <summary>
/// Verifies that the signature belongs to a trusted Microsoft signer.
/// </summary>
/// <param name="signatureInfo">The signature info to check.</param>
/// <returns><see langword="true"/> if the signature belongs to Microsoft; otherwise <see langword="false"/>.</returns>
private static bool IsTrustedMicrosoftSigner(AssemblySignatureInfo signatureInfo)
{
if (signatureInfo == null || !signatureInfo.IsSignatureValid)
{
return false;
}

var subject = signatureInfo.Subject;

return subject.IndexOf("O=Microsoft Corporation", StringComparison.OrdinalIgnoreCase) >= 0 ||
subject.IndexOf("O=\"Microsoft Corporation\"", StringComparison.OrdinalIgnoreCase) >= 0 ||
subject.IndexOf("OU=Microsoft Corporation", StringComparison.OrdinalIgnoreCase) >= 0 ||
subject.IndexOf("OU=\"Microsoft Corporation\"", StringComparison.OrdinalIgnoreCase) >= 0 ||
subject.IndexOf("CN=Microsoft Corporation", StringComparison.OrdinalIgnoreCase) >= 0 ||
subject.IndexOf("CN=\"Microsoft Corporation\"", StringComparison.OrdinalIgnoreCase) >= 0;
}
}
}
4 changes: 2 additions & 2 deletions MSBuildGuard.VSCode/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion MSBuildGuard.VSCode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"displayName": "MSBuild Guard",
"description": "Cross-platform security analysis extension that scans MSBuild project files before execution and prevents malicious code delivery.",
"icon": "resources/shield-icon.png",
"version": "0.1.0",
"version": "0.1.1",
"preview": true,
"publisher": "Hefaistos68",
"author": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
using System;
using System.Collections.Generic;
using System.IO;
using MSBuildGuard.Core;
using MSBuildGuard.Core.Trust;
using MSBuildGuard.VisualStudio.ToolWindows;
using NUnit.Framework;
using Shouldly;

namespace MSBuildGuard.VisualStudio.ToolWindows.Tests
{
/// <summary>
/// Unit tests for the <see cref="BuildBlockDialogViewModel"/> class.
/// </summary>
[TestFixture]
public sealed class BuildBlockDialogViewModelTests
{
private string tempDir = string.Empty;

/// <summary>
/// Sets up the test environment.
/// </summary>
[SetUp]
public void SetUp()
{
this.tempDir = Path.Combine(Path.GetTempPath(), "MSBuildGuardTests", Guid.NewGuid().ToString("N"));

Directory.CreateDirectory(this.tempDir);
}

/// <summary>
/// Tears down the test environment.
/// </summary>
[TearDown]
public void TearDown()
{
if (Directory.Exists(this.tempDir))
{
try
{
Directory.Delete(this.tempDir, true);
}
catch
{
// Ignore clean up errors.
}
}
}

/// <summary>
/// Verifies that risk score calculation correctly ignores trusted findings.
/// </summary>
[Test]
public void Constructor_WithTrustedFindings_CalculatesCorrectRiskScore()
{
var report = new ScanReport();

report.Target.TargetPath = Path.Combine(this.tempDir, "TestSolution.sln");
report.Target.TargetKind = TargetKind.Solution;

var finding = new Finding
{
Id = "MBG001",
Title = "Test Finding",
Severity = FindingSeverity.Medium,
FilePath = Path.Combine(this.tempDir, "TestProj.csproj"),
Fingerprint = "fingerprint-1"
};

report.Findings.Add(finding);

var fileRecord = new MsBuildFileRecord
{
Path = finding.FilePath,
NormalizedSha256 = "sha256-hash-value"
};

report.FilesScanned.Add(fileRecord);

var userTrustPath = new TrustStoreService().GetDefaultUserTrustPath();
var model = new BuildBlockDialogViewModel(report, this.tempDir);

// Initially, the finding is not trusted.
model.RiskScore.ShouldBe(20);

// Now we write a trust entry for it.
var trustStoreService = new TrustStoreService();
var userTrustStore = trustStoreService.Load(userTrustPath);
var originalDecisions = new List<TrustDecisionEntry>(userTrustStore.Decisions);
Comment thread
Hefaistos68 marked this conversation as resolved.

try
{
trustStoreService.AddDecision(userTrustPath, new TrustDecisionEntry
{
DecisionId = Guid.NewGuid().ToString("N"),
Scope = "Finding",
SubjectHash = "fingerprint-1",
Decision = "Trust",
Reason = "Test trust",
UserSid = "TestSid",
CreatedAtUtc = DateTimeOffset.UtcNow
});

var model2 = new BuildBlockDialogViewModel(report, this.tempDir);

model2.RiskScore.ShouldBe(0);
model2.RecommendedAction.ShouldBe(RecommendedAction.Allow.ToString());
}
finally
{
// Restore the original user trust store to not pollute the host.
userTrustStore.Decisions.Clear();

foreach (var d in originalDecisions)
{
userTrustStore.Decisions.Add(d);
}

trustStoreService.Save(userTrustPath, userTrustStore);
}
}
}
}
2 changes: 1 addition & 1 deletion MSBuildGuard.VisualStudio/MSBuildGuard.VisualStudio.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
<RootNamespace>MSBuildGuard.VisualStudio</RootNamespace>
<Product>MSBuildGuard</Product>
<Title>MSBuildGuard</Title>
<Version>0.3.0</Version>
<Version>0.3.1</Version>
<SignAssembly>False</SignAssembly>
<Authors>Hefaistos68</Authors>
<Company>Hefaistos68.dev</Company>
Expand Down
25 changes: 22 additions & 3 deletions MSBuildGuard.VisualStudio/MSBuildGuardPackage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@
/// </summary>
private Core.ScanReport? latestScanReport;

/// <summary>
/// Stores the scan report for which the effective risk was calculated.
/// </summary>
private Core.ScanReport? calculatedRiskReport;

private bool isLatestReportGreen;
private int latestReportEffectiveRiskScore;

Expand Down Expand Up @@ -203,6 +208,9 @@
{
await this.UiFeedbackService.WriteLineAsync("Solution unloaded. Clearing scan and review state.", CancellationToken.None);
this.latestScanReport = null;
this.calculatedRiskReport = null;
this.latestReportEffectiveRiskScore = 0;
this.isLatestReportGreen = false;
this.reviewSelectionService.SolutionReviewTargetPath = null;

await JoinableTaskFactory.SwitchToMainThreadAsync(DisposalToken);
Expand Down Expand Up @@ -377,7 +385,7 @@

int? effectiveRiskScore = null;

if (this.latestScanReport != null)
if (this.latestScanReport != null && this.latestScanReport == this.calculatedRiskReport)
{
effectiveRiskScore = this.latestReportEffectiveRiskScore;
}
Expand Down Expand Up @@ -857,6 +865,7 @@
{
this.isLatestReportGreen = false;
this.latestReportEffectiveRiskScore = 0;
this.calculatedRiskReport = null;

return;
}
Expand All @@ -870,12 +879,22 @@

var buildBlockViewModel = new ToolWindows.BuildBlockDialogViewModel(report, solutionPath);

this.isLatestReportGreen = string.Equals(buildBlockViewModel.RecommendedAction, Core.RecommendedAction.Allow.ToString(), StringComparison.OrdinalIgnoreCase);
this.latestReportEffectiveRiskScore = buildBlockViewModel.RiskScore;
var isGreen = string.Equals(buildBlockViewModel.RecommendedAction, Core.RecommendedAction.Allow.ToString(), StringComparison.OrdinalIgnoreCase);

var riskScore = buildBlockViewModel.RiskScore;

// Switch back to the UI thread to update controls and trigger VS menu updates
await JoinableTaskFactory.SwitchToMainThreadAsync(DisposalToken);

if (report != this.latestScanReport)
{
return;
}

this.isLatestReportGreen = isGreen;
this.latestReportEffectiveRiskScore = riskScore;
this.calculatedRiskReport = report;

await this.RefreshStatusBarShieldAsync().ConfigureAwait(false);

if (await this.GetServiceAsync(typeof(SVsUIShell)) is IVsUIShell uiShell)
Expand Down Expand Up @@ -1627,7 +1646,7 @@

if (!string.IsNullOrWhiteSpace(val))
{
var pathPart = val.Split('|')[0];

Check warning on line 1649 in MSBuildGuard.VisualStudio/MSBuildGuardPackage.cs

View workflow job for this annotation

GitHub Actions / Build VSIX

Dereference of a possibly null reference.

if (File.Exists(pathPart))
{
Expand Down
Loading
Loading