CycloneDX / cyclonedx-dotnet

Creates CycloneDX Software Bill of Materials (SBOM) from .NET Projects
https://cyclonedx.org/
Apache License 2.0
185 stars 89 forks source link

developmentDependency not excluded from SBOM #665

Closed mieliespoor closed 1 year ago

mieliespoor commented 1 year ago

In quite a number of repositories, we have a Directory.Build.props file looking like this:

<!--EXTERNAL_PROPERTIES: IsTestProject-->
<Project>

  <PropertyGroup>
    <CodeAnalysisRuleSet>..\sonarqube.api.ruleset</CodeAnalysisRuleSet>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="SonarAnalyzer.CSharp" Version="8.53.0.62665" developmentDependency="true" Condition="'$(IsTestProject)' != 'true'">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>

  <Target Name="DisableAnalyzersForTestProjects" BeforeTargets="CoreCompile" Condition="'$(IsTestProject)' == 'true'">
    <ItemGroup>
      <Analyzer Remove="@(Analyzer)"/>
    </ItemGroup>
  </Target>

</Project>

Note that we mark the SonarAnalyzer.CSharp reference as developmentDependency="true".

when we generate the SBOM, we run the command like:

dotnet CycloneDX -o $PublishPath $solutionPath -j -t -d -dpr -f $projectName.sbom.json -st $Classifier -sn $projectName -sv $PackageVersion 

We expected that because we specify the -d option, we won't see SonarAnalyzer.CSharp in the SBOM's anymore, but it still pops up. In addition to that, the nuspec file for SonarAnalyzer.CSharp also states that it is a development Dependency.

Why is this dependency still showing up in our SBOM's if we explicitly mark it to be excluded?

mieliespoor commented 1 year ago

related to #445

mieliespoor commented 1 year ago

I think the issue is on around here: https://github.com/CycloneDX/cyclonedx-dotnet/blob/8ad0962a770f993f64608f1d6a8e1f0b93d173be/CycloneDX/Program.cs#L339

if we do something like this somewhere inside that if statement, it appears to fix the problem

if (excludeDev && package.Scope.HasValue && package.Scope.Value.Equals(Component.ComponentScope.Excluded))
{
    continue;
}
mtsfoni commented 1 year ago

@mieliespoor, @Bertk Did PR #693 solve this issue?

Bertk commented 1 year ago

@mieliespoor yes, the development dependencies like SonarAnalyzer.CSharp are ignored with option -d

mieliespoor commented 1 year ago

Yes this is still not working. I'm not sure this will ever get fixed though. I attempted the fix in #694 but it was said that #693 might fix it, which it didn't.

Getting anything merged or released in this repo is not worth the effort. I'll leave this open and hope that it will be done some day.

Bertk commented 1 year ago

Please check the unit test for this topic . This was working in my environment but maybe you have some special configurations which are not considered.

@mieliespoor Could you please share your PackageReference statement for SonarAnalyzer.CSharp. This is the information from nuget.org and the typical way to declare a development dependency.

<PackageReference Include="SonarAnalyzer.CSharp" Version="9.9.0.77355">
  <PrivateAssets>all</PrivateAssets>
  <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>
mieliespoor commented 1 year ago

It's a very simple case of

<PackageReference Include="SonarAnalyzer.CSharp"  developmentDependency="true" Version="9.9.0.77355"/>

I will make sure that the example in the first post of this issue is working, but the above seems to have it included.

Bertk commented 1 year ago

@mieliespoor Thank you for sharing this line of code.

PackageReference does not support developmentDependency setting. The attribute developmentDependency is significant for nuget package creation.

Please notice the specification from Microsoft DevelopmentDependency support for PackageReference.

This document has also a issue reference which has a discussion why developmentDependency cannot be used for identifying developments references.

<PackageReference Include="SonarAnalyzer.CSharp" Version="8.53.0.62665" developmentDependency="true" Condition="'$(IsTestProject)' != 'true'">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>

The attribute developmentDependency="true" and the Condition="'$(IsTestProject)' != 'true'" are unnecessary.

mtsfoni commented 1 year ago

I tried this: Make new solution, with Project A. Add SonarAnalyzer and Ninject via Nuget, so I get this:

<ItemGroup>
  <PackageReference Include="Ninject" Version="4.0.0-beta.1" />
  <PackageReference Include="SonarAnalyzer.CSharp" Version="9.10.0.77988">
    <PrivateAssets>all</PrivateAssets>
    <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
  </PackageReference>
</ItemGroup>

I understand the <PrivateAssets>-node tells me the analyser is a development dependency.

I added a Project B that has a reference to Project A and started to build Boms with the -d paramter:

Is it possible that the -d flag only works for transitive dependencies but not direct dependencies in the project one creates the bom for?

Bertk commented 1 year ago

I understand the -node tells me the analyser is a development dependency.

I do not understand “-node”. Please explain.

Please also read documentation suppress-parent

Tag Description Default Value
PrivateAssets These assets will be consumed but won't flow to the parent project contentfiles;analyzers;build

<PrivateAssets>all</PrivateAssets> is the behavior for the 3rd bullet.

Bertk commented 1 year ago

I attached a ZIP file which has 2 BOM files.

The file with development dependencies has a library component for SonarAnalyzer.CSharp and the other does not.

    <component type="library" bom-ref="pkg:nuget/SonarAnalyzer.CSharp@9.9.0.77355">
      <author>SonarSource</author>
      <name>SonarAnalyzer.CSharp</name>
      <version>9.9.0.77355</version>
      <description>Roslyn analyzers that spot Bugs, Vulnerabilities and Code Smells in your code. For an even better overall experience, you can use SonarLint for Visual Studio or Rider, which is a free extension that can be used standalone or with SonarQube and/or SonarCloud.</description>
      <scope>required</scope>
      <hashes>
        <hash alg="SHA-512">BB682ED60436DC3A82FDEE73CA886066E4F751EA3EACB4BC08AA5A98A65780DC6567C055587FC697C89E82369EEDF36246DA091779EFF54BBB5834D20EDC855B</hash>
      </hashes>
      <licenses>
        <license>
          <id>LGPL-3.0-only</id>
        </license>
      </licenses>
      <copyright>Copyright © 2015-2023 SonarSource SA</copyright>
      <purl>pkg:nuget/SonarAnalyzer.CSharp@9.9.0.77355</purl>
      <externalReferences>
        <reference type="website">
          <url>https://redirect.sonarsource.com/doc/sonar-visualstudio.html</url>
        </reference>
        <reference type="vcs">
          <url>https://github.com/SonarSource/sonar-dotnet</url>
        </reference>
      </externalReferences>
    </component>

CycloneDX-V2.8.1.bom.zip

Bertk commented 1 year ago

Please confirm and close the issue.

mtsfoni commented 1 year ago

Wow.... I'm in utter disbelief, but it appears to me, that the call dotnet cyclonedx ... does not forward the -d parameter to the actual application. While calling the .exe in the traditional style works fine. I could reproduce this behaviour on a 2nd device.

That was the reason that my boms included the library despite the -d option. I can only assume that dotnet reserves -d for something and doesn't forward it. I'd like if somebody else could confirm my observation (I almost lost my sanity figuring this out).

@mieliespoor if you call cyclonedx via dotnet as dotnet cyclonedx -d, try dotnet cyclonedx --exclude-dev instead, please. That might solve the problem.

We probably should change the parameter shortname or add a second shortname to stay downwards compatible.

mtsfoni commented 1 year ago

On another note, the dependency tree still includes the development dependencies, even though the component is not added:

<bom xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" serialNumber="urn:uuid:9ca106bf-681b-4c80-9925-5b6f28dfad8d" version="1" xmlns="http://cyclonedx.org/schema/bom/1.4">
  <metadata>
    <tools>
      <tool>
        <vendor>CycloneDX</vendor>
        <name>CycloneDX module for .NET</name>
        <version>2.1.8</version>
      </tool>
    </tools>
    <component type="application" bom-ref="ClassLibrary1@0.0.0">
      <name>ClassLibrary1</name>
      <version>0.0.0</version>
    </component>
  </metadata>
  <components />
  <dependencies>
    <dependency ref="ClassLibrary1@0.0.0">
      <dependency ref="pkg:nuget/SonarAnalyzer.CSharp@9.10.0.77988" /><!--This shouldn't be here in my oppinion-->
    </dependency>
    <dependency ref="pkg:nuget/SonarAnalyzer.CSharp@9.10.0.77988" /><!--This shouldn't be here in my oppinion-->
  </dependencies>
</bom>
Bertk commented 1 year ago

@CodeTigerCloud Please create a new issue for this dangling reference. This should be clarified and documented.

<dependency ref="pkg:nuget/SonarAnalyzer.CSharp@9.10.0.77988" />

image

https://cyclonedx.org/docs/1.4/json/#dependencies_items_ref

mieliespoor commented 1 year ago

I will do some tests tomorrow, but it seems that the -d flag is being used to enable diagnostic output by dotnet.

See: https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet#options-for-running-a-command

Bertk commented 1 year ago

@mieliespoor Please share your test results and hopefully this was successful. This issue should be closed :relaxed:

Please create another issue for the command line options conflict. This confusing option should be fixed ASAP.

-d|--exclude-dev Exclude development dependencies from the BOM

mtsfoni commented 1 year ago

I created issue #760 and #761 for the discussed problems.

Since this issue is basically done with the merged PR, I close this.

@mieliespoor if you have further troubled, please open a fresh issue and try to provide us additional information.