From d372af53ce3f586e0d9ee01889601842f3e9afe5 Mon Sep 17 00:00:00 2001 From: Kaspar Lyngsie Date: Wed, 1 May 2024 13:39:35 +0200 Subject: [PATCH 1/4] fix: giving user-friendly error when trying to scan unpublishable projects --- lib/nuget-parser/cli/dotnet.ts | 10 +++++++++ lib/nuget-parser/index.ts | 9 ++++++++ .../dotnet_8_unpublishable.csproj | 6 ++++++ .../dotnet_8_unpublishable/program.cs | 8 +++++++ test/parsers/csproj.spec.ts | 2 ++ test/parsers/parse-core-v2.spec.ts | 21 ++++++++++++++++++- 6 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/dotnetcore/dotnet_8_unpublishable/dotnet_8_unpublishable.csproj create mode 100644 test/fixtures/dotnetcore/dotnet_8_unpublishable/program.cs diff --git a/lib/nuget-parser/cli/dotnet.ts b/lib/nuget-parser/cli/dotnet.ts index 3988ba50..776d94e1 100644 --- a/lib/nuget-parser/cli/dotnet.ts +++ b/lib/nuget-parser/cli/dotnet.ts @@ -50,6 +50,16 @@ export async function validate() { } } +export async function getProperty( + name: string, + projectPath: string, +): Promise { + const command = 'dotnet'; + const args = ['msbuild', `-getProperty:${name}`, projectPath]; + const result = await handle('getProperty', command, args); + return result.stdout.trim(); +} + export async function restore(projectPath: string): Promise { const command = 'dotnet'; const args = [ diff --git a/lib/nuget-parser/index.ts b/lib/nuget-parser/index.ts index 6b2d7600..77813af5 100644 --- a/lib/nuget-parser/index.ts +++ b/lib/nuget-parser/index.ts @@ -189,7 +189,16 @@ Will attempt to build dependency graph anyway, but the operation might fail.`); // Loop through all TargetFrameworks supplied and generate a dependency graph for each. const results: DotnetCoreV2Results = []; + let property = ''; for (const decidedTargetFramework of decidedTargetFrameworks) { + // Ensure the project can be published. If not, we cannot scan published dependencies, thus cannot read the dependency graph + property = await dotnet.getProperty('IsPublishable', safeRoot); + if (property === 'false') { + throw new CliCommandError( + `unable to scan project in ${safeRoot} as the project has the MSBuild property IsPublishable set to false. Snyk must be able to publish the solution in order to accurately scan dependencies.`, + ); + } + // Run `dotnet publish` to create a self-contained publishable binary with included .dlls for assembly version inspection. const publishDir = await dotnet.publish(safeRoot, decidedTargetFramework); diff --git a/test/fixtures/dotnetcore/dotnet_8_unpublishable/dotnet_8_unpublishable.csproj b/test/fixtures/dotnetcore/dotnet_8_unpublishable/dotnet_8_unpublishable.csproj new file mode 100644 index 00000000..e22d7278 --- /dev/null +++ b/test/fixtures/dotnetcore/dotnet_8_unpublishable/dotnet_8_unpublishable.csproj @@ -0,0 +1,6 @@ + + + net8.0 + false + + diff --git a/test/fixtures/dotnetcore/dotnet_8_unpublishable/program.cs b/test/fixtures/dotnetcore/dotnet_8_unpublishable/program.cs new file mode 100644 index 00000000..0be1a5ef --- /dev/null +++ b/test/fixtures/dotnetcore/dotnet_8_unpublishable/program.cs @@ -0,0 +1,8 @@ +using System; +class TestFixture { + static public void Main(String[] args) + { + var client = new System.Net.Http.HttpClient(); + Console.WriteLine("Hello, World!"); + } +} diff --git a/test/parsers/csproj.spec.ts b/test/parsers/csproj.spec.ts index 96fdb4bb..38258761 100644 --- a/test/parsers/csproj.spec.ts +++ b/test/parsers/csproj.spec.ts @@ -1,6 +1,7 @@ import { getTargetFrameworksFromProjFile } from '../../lib/nuget-parser/parsers/csproj-parser'; import { describe, expect, it } from '@jest/globals'; import * as plugin from '../../lib'; +import * as dotnet from '../../lib/nuget-parser/cli/dotnet'; import { legacyPlugin as pluginApi } from '@snyk/cli-interface'; describe('parse .csproj', () => { @@ -74,6 +75,7 @@ describe('parse .csproj', () => { it('parse dotnet with no deps', async () => { const noDeps = './test/fixtures/target-framework/no-dependencies/'; + await dotnet.restore(noDeps); const result = await plugin.inspect(noDeps, 'obj/project.assets.json'); diff --git a/test/parsers/parse-core-v2.spec.ts b/test/parsers/parse-core-v2.spec.ts index 6c70f437..e6b86220 100644 --- a/test/parsers/parse-core-v2.spec.ts +++ b/test/parsers/parse-core-v2.spec.ts @@ -155,18 +155,37 @@ describe('when generating depGraphs and runtime assemblies using the v2 parser', description: 'net472 - with package.assets.json', projectPath: './test/fixtures/target-framework/no-dependencies/', manifestFile: 'obj/project.assets.json', + requiresRestore: true, expectedErrorMessage: /not able to find any supported TargetFrameworks/, }, { description: 'net461 - no package.assets.json', projectPath: './test/fixtures/packages-config/repositories-config/', manifestFile: 'project.json', + requiresRestore: false, expectedErrorMessage: /runtime resolution flag is currently only supported/, }, + { + description: 'dotnet core project - unpublishable', + projectPath: './test/fixtures/dotnetcore/dotnet_8_unpublishable/', + manifestFile: 'obj/project.assets.json', + requiresRestore: true, + expectedErrorMessage: + /as the project has the MSBuild property IsPublishable set to false/, + }, ])( 'does not allow the runtime option to be set on unsupported projects: $description', - async ({ projectPath, manifestFile, expectedErrorMessage }) => { + async ({ + projectPath, + manifestFile, + requiresRestore, + expectedErrorMessage, + }) => { + if (requiresRestore) { + await dotnet.restore(projectPath); + } + await expect( async () => await plugin.inspect(projectPath, manifestFile, { From 6379cfa42be01b1aad22661b075abb104ee79ce3 Mon Sep 17 00:00:00 2001 From: Kaspar Lyngsie Date: Wed, 1 May 2024 14:32:03 +0200 Subject: [PATCH 2/4] fix: removing already covered test --- test/parsers/parse-packages-config.spec.ts | 36 ---------------------- 1 file changed, 36 deletions(-) diff --git a/test/parsers/parse-packages-config.spec.ts b/test/parsers/parse-packages-config.spec.ts index b12bf410..783d443d 100644 --- a/test/parsers/parse-packages-config.spec.ts +++ b/test/parsers/parse-packages-config.spec.ts @@ -154,42 +154,6 @@ describe('when parsing packages.config', () => { packagesFolder: projectPath + './_packages', }); }); - - it.each([ - { - projectPath: 'test/fixtures/target-framework/no-csproj', - manifestFile: 'obj/project.assets.json', - defaultName: 'no-csproj', - }, - { - projectPath: 'test/fixtures/packages-config/config-only', - manifestFile: 'packages.config', - defaultName: 'config-only', - }, - ])( - 'should parse project with and without name prefix', - async ({ projectPath, manifestFile, defaultName }) => { - // With prefix - let result = await plugin.inspect(projectPath, manifestFile, { - 'project-name-prefix': 'custom-prefix/', - }); - - if (pluginApi.isMultiResult(result) || !result?.package) { - throw new Error('received invalid depTree'); - } - - expect(result.package.name).toBe(`custom-prefix/${defaultName}`); - - // Without - result = await plugin.inspect(projectPath, manifestFile, {}); - - if (pluginApi.isMultiResult(result) || !result?.package) { - throw new Error('received invalid depTree'); - } - - expect(result.package.name).toBe(defaultName); - }, - ); }); describe('when calling getMinimumTargetFramework', () => { From 4ee1b20e85a6849e7baa495799701e558e56472e Mon Sep 17 00:00:00 2001 From: Kaspar Lyngsie Date: Wed, 1 May 2024 14:42:02 +0200 Subject: [PATCH 3/4] fix: removing one more --- .../no-dependencies/obj/project.assets.json | 45 ------------------- test/parsers/csproj.spec.ts | 2 +- 2 files changed, 1 insertion(+), 46 deletions(-) delete mode 100644 test/fixtures/target-framework/no-dependencies/obj/project.assets.json diff --git a/test/fixtures/target-framework/no-dependencies/obj/project.assets.json b/test/fixtures/target-framework/no-dependencies/obj/project.assets.json deleted file mode 100644 index 834a493f..00000000 --- a/test/fixtures/target-framework/no-dependencies/obj/project.assets.json +++ /dev/null @@ -1,45 +0,0 @@ -{ - "version": 3, - "targets": { - ".NETFramework,Version=v4.7.2": {} - }, - "libraries": {}, - "projectFileDependencyGroups": { - ".NETFramework,Version=v4.7.2": [] - }, - "packageFolders": { - "C:\\Program Files\\dotnet\\sdk\\NuGetFallbackFolder": {} - }, - "project": { - "version": "1.0.0", - "restore": { - "projectUniqueName": "C:\\Users\\ma\\csproj_no_deps.csproj", - "projectName": "csproj_no_deps", - "projectPath": "C:\\Users\\ma\\csproj_no_deps.csproj", - "packagesPath": "C:\\Users\\ma\\.nuget\\packages\\", - "outputPath": "C:\\Users\\ma\\obj", - "projectStyle": "PackageReference", - "fallbackFolders": [ - "C:\\Program Files\\dotnet\\sdk\\NuGetFallbackFolder" - ], - "originalTargetFrameworks": [ - "net472" - ], - "sources": { - "C:\\Program Files (x86)\\Microsoft SDKs\\NuGetPackages\\": {}, - "https://api.nuget.org/v3/index.json": {} - }, - "frameworks": { - "net472": { - "projectReferences": {} - } - }, - "warningProperties": { - "warnAsError": ["NU1605"] - } - }, - "frameworks": { - "net472": {} - } - } -} diff --git a/test/parsers/csproj.spec.ts b/test/parsers/csproj.spec.ts index 38258761..dd71dd2f 100644 --- a/test/parsers/csproj.spec.ts +++ b/test/parsers/csproj.spec.ts @@ -83,7 +83,7 @@ describe('parse .csproj', () => { throw new Error('received invalid depTree'); } - expect(Object.keys(result.package.dependencies).length).toBe(0); + expect(Object.keys(result.package.dependencies).length).toBe(1); // Just the TF's own dependency }); it('parse dotnet with no valid framework defined', async () => { From ebff808fc5fc9c3f27f22954d0dcf473bd5b9a92 Mon Sep 17 00:00:00 2001 From: Kaspar Lyngsie Date: Wed, 1 May 2024 16:55:00 +0200 Subject: [PATCH 4/4] fix: dropping already tested test --- test/parsers/csproj.spec.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/test/parsers/csproj.spec.ts b/test/parsers/csproj.spec.ts index dd71dd2f..cc9a99b3 100644 --- a/test/parsers/csproj.spec.ts +++ b/test/parsers/csproj.spec.ts @@ -1,7 +1,6 @@ import { getTargetFrameworksFromProjFile } from '../../lib/nuget-parser/parsers/csproj-parser'; import { describe, expect, it } from '@jest/globals'; import * as plugin from '../../lib'; -import * as dotnet from '../../lib/nuget-parser/cli/dotnet'; import { legacyPlugin as pluginApi } from '@snyk/cli-interface'; describe('parse .csproj', () => { @@ -73,19 +72,6 @@ describe('parse .csproj', () => { expect(result.plugin.targetRuntime).toBe('netcoreapp2.0'); }); - it('parse dotnet with no deps', async () => { - const noDeps = './test/fixtures/target-framework/no-dependencies/'; - await dotnet.restore(noDeps); - - const result = await plugin.inspect(noDeps, 'obj/project.assets.json'); - - if (pluginApi.isMultiResult(result) || !result?.package?.dependencies) { - throw new Error('received invalid depTree'); - } - - expect(Object.keys(result.package.dependencies).length).toBe(1); // Just the TF's own dependency - }); - it('parse dotnet with no valid framework defined', async () => { const noValidFrameworksPath = './test/fixtures/target-framework/no-target-valid-framework';