Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: giving user-friendly error when trying to scan unpublishable project #207

Merged
merged 4 commits into from
May 1, 2024
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
10 changes: 10 additions & 0 deletions lib/nuget-parser/cli/dotnet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ export async function validate() {
}
}

export async function getProperty(
name: string,
projectPath: string,
): Promise<string> {
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<string> {
const command = 'dotnet';
const args = [
Expand Down
9 changes: 9 additions & 0 deletions lib/nuget-parser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<IsPublishable>false</IsPublishable>
</PropertyGroup>
</Project>
8 changes: 8 additions & 0 deletions test/fixtures/dotnetcore/dotnet_8_unpublishable/program.cs
Original file line number Diff line number Diff line change
@@ -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!");
}
}

This file was deleted.

12 changes: 0 additions & 12 deletions test/parsers/csproj.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +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/';

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(0);
});

it('parse dotnet with no valid framework defined', async () => {
const noValidFrameworksPath =
'./test/fixtures/target-framework/no-target-valid-framework';
Expand Down
21 changes: 20 additions & 1 deletion test/parsers/parse-core-v2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down
36 changes: 0 additions & 36 deletions test/parsers/parse-packages-config.spec.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test was already covered.

Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down