Skip to content

Commit c93fbb0

Browse files
gfsCopilot
andauthored
Bumps npm dependencies and DevSkim .NET dependencies, Fix #697 (#698)
* Bumps npm dependencies * Update Changelog.md * Fix #697 Populate the Description or Recommendation fields in the markdown description since that is what ends up rendered when used in github code scanning per report in #697 of inability to customize message with custom rules. * Clean up and changelog update * Add unit tests for SarifWriter help field logic Introduces SarifWriterTests to verify SARIF help field population for rules with various combinations of recommendation, description, and rule info. Tests cover fallback logic, markdown formatting, and edge cases such as empty or whitespace recommendations. * Update DevSkim-DotNet/Microsoft.DevSkim.CLI/Writers/SarifWriter.cs Co-authored-by: Copilot <[email protected]> * Refactor SARIF rule markdown description logic Extracted markdown description building into a new BuildMarkdownDescription method for improved readability and maintainability. The logic for constructing the SARIF rule's Help.Markdown field is now encapsulated in a dedicated helper function. * Refactor SarifWriterTests to use local writers Replaces class-level StringWriter and SarifWriter fields with local variables in each test method. This improves test isolation and resource management by using 'using' statements for disposable objects. * Refactor SARIF output parsing in tests Replaced calls to the ParseSarifOutput helper with direct usage of JObject.Parse in SarifWriterTests. Removed the now-unused ParseSarifOutput method for simplification. * Rename SarifWriter test methods for clarity Test method names in SarifWriterTests.cs were updated to use descriptive, behavior-driven naming. This improves readability and makes test purposes clearer for future maintenance. * Remove unused Patterns property in SarifWriterTests Eliminated the Patterns property from the test case object initialization in SarifWriterTests.cs, as it was not required for the test. * Refactor SARIF rule text description logic Moved the logic for building SARIF rule text descriptions into a dedicated BuildTextDescription method for improved readability and maintainability. * Add test for empty markdown in SARIF rule help Introduces a unit test to verify that when a rule has no recommendation and no rule info, the SARIF 'help.markdown' field is empty or null, while 'help.text' falls back to the rule description. * Update changelog for v1.0.63 with fixes and tests Added details for version 1.0.63 including a fix for Sarif Markdown recommendation value population (#697), new test cases for SarifWriter, and updated dependencies. Fixed some section header levels to improve formatting. * Remove redundant test for SARIF markdown help content Deleted the test 'When_rule_has_recommendation_and_rule_info_then_markdown_includes_both' from SarifWriterTests.cs as it was redundant with `When_rule_has_recommendation_and_rule_info_then_markdown_is_properly_formatted` * Dependencies in checked in package-lock file should use npmjs repository The internal repository is substituted during pipeline build to allow for external contributor use * Refactor SARIF help URI construction and update tests Introduced a CreateHelpUri method in SarifWriter to safely construct help URIs for DevSkim rules, handling null or empty RuleInfo values. Updated related unit tests to use the new baseHelpUri constant for consistency and maintainability. * Update tests to use SarifWriter.CreateHelpUri Replaces references to SarifWriter.baseHelpUri with SarifWriter.CreateHelpUri in SarifWriterTests to ensure help URIs are generated consistently. This improves test accuracy and future-proofs against changes in URI construction. * Rename baseHelpUri to BaseHelpUri in SarifWriter Updated the constant baseHelpUri to use PascalCase (BaseHelpUri) for consistency with naming conventions. Adjusted references to the constant accordingly. * Update SarifWriterTests to use exact string assertions Changed tests to use Assert.AreEqual with expected markdown and help text strings instead of Assert.IsTrue with Contains. This ensures stricter validation of the output format. * Update Changelog.md * Refactor SarifWriterTests to reuse helpUri variable Replaces repeated calls to SarifWriter.CreateHelpUri with a local helpUri variable in test assertions for expectedMarkdown. This improves readability and reduces redundant method calls. * Change BaseHelpUri to private constant Updated the visibility of the BaseHelpUri constant from public to private in SarifWriter.cs to restrict its access within the class. * Change CreateHelpUri to internal access modifier The CreateHelpUri method in SarifWriter was changed from public to internal to restrict its visibility within the assembly. This helps encapsulate implementation details and limits external usage. * Update SarifWriter.cs * Expose internals to test project and update method visibility Added InternalsVisibleTo for Microsoft.DevSkim.Tests in the CLI project file to allow unit testing of internal members. Changed CreateHelpUri from public to internal in SarifWriter to restrict its visibility to within the assembly. * Remove unused variable in AnalyzeTest.cs Deleted the unused 'oneUpPath' variable from the test setup to clean up the code. * Remove unused variable in SuppressionsTest Deleted the unused 'oneUpPath' variable from the test method to clean up the code. * Remove unused exception variable in regex creation Eliminated the unused exception variable in the catch block of the regex creation method. Added a TODO comment noting the need to refactor for logging since the logger is not accessible in the static context. --------- Co-authored-by: Copilot <[email protected]>
1 parent 6afdd9b commit c93fbb0

File tree

9 files changed

+494
-41
lines changed

9 files changed

+494
-41
lines changed

Changelog.md

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,69 +4,79 @@ All notable changes to this project will be documented in this file.
44
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
55
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

7+
## [1.0.63] - 2025-07-29
8+
### Fix
9+
Fixes Sarif Markdown value failing to populate the rule provided recommendation value. #697
10+
11+
### Dependencies
12+
Update dependencies
13+
14+
### Tests
15+
Adds test cases for SarifWriter
16+
717
## [1.0.62] - 2025-07-23
8-
## Pipelines
18+
### Pipelines
919
Pipeline updates
1020

1121
## [1.0.61] - 2025-07-11
12-
## Pipelines
22+
### Pipelines
1323
Pipeline updates
1424

15-
## [1.0.60] - 2025-06-36
16-
## Pipelines
25+
## [1.0.60] - 2025-06-26
26+
### Pipelines
1727
Pipeline updates
1828

1929
## [1.0.59] - 2025-06-10
20-
## Misc (non-code)
30+
### Misc (non-code)
2131
Removes old doc publish workflow.
2232

2333
## [1.0.58] - 2025-06-09
24-
## New Feature
34+
### New Feature
2535
Adds a `vs` output format to leverage the DevSkim CLI as a build task in a csproj.
2636

2737
## [1.0.57] - 2025-05-28
28-
## Fix
38+
### Fix
2939
Fix an issue handling non-ascii paths when launching LSP in VS Code Extension
3040

31-
## Dependencies
41+
### Dependencies
3242
Update Dependencies
3343

3444
## [1.0.56] - 2025-04-14
35-
## Tests
45+
### Tests
3646
Migrate to MTP
3747

3848
## [1.0.55] - 2025-04-14
39-
## Dependencies
49+
### Dependencies
4050
Updates Dependencies
4151

4252
## [1.0.54] - 2025-04-09
43-
## Dependencies
53+
### Dependencies
4454
Updates Dependencies
4555

4656
## [1.0.53] - 2025-04-01
47-
## Documentation
57+
### Documentation
4858
Adds a link to the Microsoft Privacy Statement to the Readme.
4959

5060
## [1.0.52] - 2024-12-09
51-
## Dependencies
61+
### Dependencies
5262
Updates Dependencies
5363

54-
## .Net Targets
64+
### .Net Targets
5565
CLI now targets .NET 8.0 and .NET 9.0, .NET 6.0/7.0 targeting removed. DevSkim Library component retains .Net Standard 2.1 support.
5666

5767
## [1.0.51] - 2024-12-09
58-
## Fix
68+
### Fix
5969
Fix confidence filtering at rule level.
6070

6171
## [1.0.50] - 2024-12-05
62-
## Fix
72+
### Fix
6373
Fixes #664 handling of options from IgnoreRuleMap when using OptionsJson
6474

65-
## New Functionality
75+
### New Functionality
6676
Adds `include-globs` argument to require all scanned files match a specific glob pattern #663.
6777

6878
## [1.0.49] - 2024-12-03
69-
## Rules
79+
### Rules
7080
Fixed false positives and false negatives in outdated/banned SSL/TLS protocols. #649
7181

7282
## [1.0.48] - 2024-11-20

DevSkim-DotNet/Microsoft.DevSkim.CLI/Microsoft.DevSkim.CLI.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
<LangVersion>10.0</LangVersion>
2626
<Nullable>Enable</Nullable>
2727
</PropertyGroup>
28+
29+
<ItemGroup>
30+
<InternalsVisibleTo Include="Microsoft.DevSkim.Tests" />
31+
</ItemGroup>
2832
<ItemGroup>
2933
<None Include="..\Content\LICENSE.txt" Pack="true" PackagePath="" />
3034
<None Include="..\Content\devskim-icon-128.png" Pack="true" PackagePath="" />

DevSkim-DotNet/Microsoft.DevSkim.CLI/Writers/SarifWriter.cs

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Linq;
77
using System.Reflection;
88
using System.Runtime.InteropServices;
9+
using System.Text;
910
using Microsoft.ApplicationInspector.RulesEngine;
1011
using Newtonsoft.Json;
1112
using Newtonsoft.Json.Linq;
@@ -190,23 +191,25 @@ var s when s.HasFlag(Severity.ManualReview) => FailureLevel.Note,
190191

191192
public string? OutputPath { get; }
192193

194+
private const string BaseHelpUri = "https://github.com/Microsoft/DevSkim/blob/main/guidance/";
195+
193196
private void AddRuleToSarifRule(DevSkimRule devskimRule)
194197
{
195198
if (!_rules.ContainsKey(devskimRule.Id))
196199
{
197-
Uri helpUri = new Uri("https://github.com/Microsoft/DevSkim/blob/main/guidance/" + devskimRule.RuleInfo); ;
200+
Uri helpUri = CreateHelpUri(devskimRule.RuleInfo);
198201
ReportingDescriptor sarifRule = new ReportingDescriptor();
199202
sarifRule.Id = devskimRule.Id;
200203
sarifRule.Name = ToSarifFriendlyName(devskimRule.Name);
201204
sarifRule.ShortDescription = new MultiformatMessageString() { Text = devskimRule.Description };
202205
sarifRule.FullDescription = new MultiformatMessageString() { Text = $"{devskimRule.Name}: {devskimRule.Description}" };
206+
203207
sarifRule.Help = new MultiformatMessageString()
204208
{
205-
// If recommendation is present use that, otherwise use description if present, otherwise use the HelpUri
206-
Text = !string.IsNullOrEmpty(devskimRule.Recommendation) ? devskimRule.Recommendation :
207-
(!string.IsNullOrEmpty(devskimRule.Description) ? devskimRule.Description : $"Visit {helpUri} for guidance on this issue."),
208-
Markdown = $"Visit [{helpUri}]({helpUri}) for guidance on this issue."
209+
Text = BuildTextDescription(devskimRule, helpUri),
210+
Markdown = BuildMarkdownDescription(devskimRule, helpUri)
209211
};
212+
210213
sarifRule.HelpUri = helpUri;
211214
sarifRule.DefaultConfiguration = new ReportingConfiguration()
212215
{
@@ -316,5 +319,74 @@ private void MapRuleToResult(Rule rule, ref Result resultItem)
316319
resultItem.Tags.Add(tag);
317320
}
318321
}
322+
323+
/// <summary>
324+
/// Creates a help URI for a DevSkim rule, handling null or empty RuleInfo safely
325+
/// </summary>
326+
/// <param name="ruleInfo">The rule info filename, can be null or empty</param>
327+
/// <returns>A properly formed URI</returns>
328+
internal static Uri CreateHelpUri(string? ruleInfo)
329+
{
330+
var baseUri = new Uri(BaseHelpUri);
331+
332+
if (string.IsNullOrEmpty(ruleInfo))
333+
{
334+
// Return base URI if no specific rule info is provided
335+
return baseUri;
336+
}
337+
338+
// Use Uri constructor to safely combine base URI with relative path
339+
return new Uri(baseUri, ruleInfo);
340+
}
341+
342+
/// <summary>
343+
/// Builds the text description for a SARIF rule based on the DevSkim rule properties
344+
/// </summary>
345+
/// <param name="devskimRule">The DevSkim rule containing recommendation and description</param>
346+
/// <param name="helpUri">The help URI for the rule</param>
347+
/// <returns>The text description string</returns>
348+
private static string BuildTextDescription(DevSkimRule devskimRule, Uri helpUri)
349+
{
350+
// If recommendation is present use that, otherwise use description if present, otherwise use the HelpUri
351+
if (!string.IsNullOrEmpty(devskimRule.Recommendation))
352+
{
353+
return devskimRule.Recommendation;
354+
}
355+
356+
if (!string.IsNullOrEmpty(devskimRule.Description))
357+
{
358+
return devskimRule.Description;
359+
}
360+
361+
return $"Visit {helpUri} for guidance on this issue.";
362+
}
363+
364+
/// <summary>
365+
/// Builds the markdown description for a SARIF rule based on the DevSkim rule properties
366+
/// </summary>
367+
/// <param name="devskimRule">The DevSkim rule containing recommendation and rule info</param>
368+
/// <param name="helpUri">The help URI for the rule</param>
369+
/// <returns>The formatted markdown string</returns>
370+
private static string BuildMarkdownDescription(DevSkimRule devskimRule, Uri helpUri)
371+
{
372+
StringBuilder markdownDescriptionBuilder = new();
373+
374+
if (!string.IsNullOrEmpty(devskimRule.Recommendation))
375+
{
376+
markdownDescriptionBuilder.Append(devskimRule.Recommendation);
377+
}
378+
379+
if (!string.IsNullOrEmpty(devskimRule.RuleInfo))
380+
{
381+
// If a recommendation was set put a space before the rule info string.
382+
if (markdownDescriptionBuilder.Length > 0)
383+
{
384+
markdownDescriptionBuilder.Append(' ');
385+
}
386+
markdownDescriptionBuilder.Append($"Visit [{helpUri}]({helpUri}) for additional guidance on this issue.");
387+
}
388+
389+
return markdownDescriptionBuilder.ToString();
390+
}
319391
}
320392
}

DevSkim-DotNet/Microsoft.DevSkim.Tests/AnalyzeTest.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ public void RelativePathTest()
2020
File.Delete(outFileName);
2121

2222
string basePath = Path.GetTempPath();
23-
string oneUpPath = Directory.GetParent(basePath).FullName;
2423
using FileStream file = File.Open(tempFileName, FileMode.Create);
2524
file.Write(Encoding.UTF8.GetBytes("MD5;\nhttp://contoso.com\n"));
2625
file.Close();

0 commit comments

Comments
 (0)