digitalcoyote / NuGetDefense

An MSBuildTask that checks for known vulnerabilities. Inspired by OWASP SafeNuGet.
MIT License
96 stars 19 forks source link

Build fails with dotnet publish after upgrading from NuGetDefense 2.1 to 3.0.7 #95

Open natan-abolafya opened 2 years ago

natan-abolafya commented 2 years ago

Describe the bug It happens only on our build agent as I can't reproduce locally. Feels like a string escaping problem but I know too little about NuGetDefense to troubleshoot it further.

The command

dotnet publish Windows.sln --configuration ReleaseProxy /p:platform="Any CPU" --output proxy\

Output:

[21:47:26] [solution] Microsoft (R) Build Engine version 17.2.0+41abc5629 for .NET
[21:47:26] [solution] Copyright (C) Microsoft Corporation. All rights reserved.
[21:47:26] [solution] 
[21:47:27] [solution]   Determining projects to restore...
[21:47:29] [solution]   Restored C:\BuildAgent\work\cabf95aa4bca1f9d\src\client\Windows\Client\Client.csproj (in 1.13 sec).
[21:47:29] [solution]   1 of 2 projects are up-to-date for restore.
[21:47:30] [solution]   Shared -> C:\BuildAgent\work\cabf95aa4bca1f9d\src\client\Shared\bin\Release\netstandard2.1\Shared.dll
[21:47:31] [solution]   Shared -> C:\BuildAgent\work\cabf95aa4bca1f9d\src\client\Windows\build-artifacts\proxy\
[21:47:32] [solution]   Client -> C:\BuildAgent\work\cabf95aa4bca1f9d\src\client\Windows\Client\bin\ReleaseProxy\Appgate SDP Service.dll
[21:47:32] [solution]   '-p' is not recognized as an internal or external command,
[21:47:32] [solution]   operable program or batch file.
[21:47:32] [solution] C:\WINDOWS\system32\config\systemprofile\.nuget\packages\nugetdefense\3.0.7\build\nugetdefense.targets(11,9): error MSB3073: The command " -p "C:\BuildAgent\work\cabf95aa4bca1f9d\src\client\Windows\Client\Client.csproj" --tfm net6.0" exited with code 9009. [C:\BuildAgent\work\cabf95aa4bca1f9d\src\client\Windows\Client\Client.csproj]

One possible relevant issue is that the build runs as SYSTEM. The Client.csproj is a .net6 project. Though same issue were present with .net5 too.

Sorry, I can't think of anything else that might be relevant right now.

Expected behavior Build not to fail.

Tools (please complete the following information):

Thanks for the help.

digitalcoyote commented 2 years ago

That does sound like a strong escape issue. I'll have to double-check, but I believe the -p comes from the msbuild call to run NuGetDefense targeting the current project.

I'll see what I can do to reproduce it.

digitalcoyote commented 2 years ago

I'm going to be unavailable until 7/28. If anyone else can reproduce this or get more details before then, it would be much appreciated. I'll try to put a priority on this when I get back.

digitalcoyote commented 2 years ago

@natan-abolafya I'm probably going to have to test this on an actual project to be able to reproduce it. If you need an immediate work around, since it looks like this is using CI, the dotnet tool can be used. I'm running this locally on Linux, by my CI setup includes a few windows builds (but no publishes) so I'll need to set that up and run it.

natan-abolafya commented 2 years ago

hey @digitalcoyote, have you had the chance to set up a project?

digitalcoyote commented 2 years ago

Unfortunately not yet. I've been working extra hours with the assumption that we'd eventually not be rushing to meet a deadline at work. I'm going to have to carve out some time specifically to look into this soon. I'll see if I can look into it tonight after work.

natan-abolafya commented 2 years ago

that sounds rough, hope it ends soon.

There is no rush really. I just wanted to check in case it slipped your mind.

digitalcoyote commented 2 years ago

sigh I forgot tonight was Halloween. Apparently I'll be doing this tomorrow night.

digitalcoyote commented 2 years ago

Did not reproduce it with the projects I tried. I did not have the ability to run it in a CI for those though. If I remember correctly, Ms build has some special functions when it detects it's being built in a CI environment. I'll do some reading and maybe setup TeamCity with a build agent on windows and try it again.

digitalcoyote commented 2 years ago

I have a CI scenario where something similar with this has started happening with msdeploy. I'm still investigating those, but I don;t have permission to run any tests in that environment for open source projects. If I figure out what is causing that I'll try to recreate it using nugetdefense so I can hopefully resolve this.

digitalcoyote commented 9 months ago

I believe this has been resolved in 4.0.0.1 If anyone is still running into this, reopen this issue or feel free to submit a new one.

natan-abolafya commented 8 months ago

Hi @digitalcoyote thanks. But unfortunately 4.0.2 did not solve the problem. It's still the exact same error message. I don't see any button to reopen the issue, I guess I don't have the relevant rights?

digitalcoyote commented 8 months ago

I'll take another look tonight. I used the global tool for debugging. It's possible the issue only occurs when running. From the build task.

digitalcoyote commented 8 months ago

Srry about more delays... We got mandatory overtime yesterday and again today (minimum 12 hour shifts until it's done). I'm hoping to get my project merged in time to work on this tonight, but if we find issues it could be pushed back to tomorrow or even this weekend.

Should be a simple fix once I reproduce it though (usually is in a project this size).

natan-abolafya commented 8 months ago

No worries, 2.1 is still kicking so there is no rush :). Thank you so much for trying to prioritize it though. But yeah, work comes first. Good luck!

digitalcoyote commented 8 months ago

I've found 2 possible issues, but my gut tells me they aren't the core issue here. I'll be publishing those in a few minutes, but I'll keep trying to reproduce this tonight. Assuming work doesn't have me working late again, the next day I'll have time to look into this will be Tuesday.

digitalcoyote commented 8 months ago

v4.0.3.0 and v3.2.3.0 are released and you are welcome to try them as they do deal with parsing issues, but based on the error message I don't think that's the only issue. I'll keep testing/debugging after I eat, but I expect this will roll into Tuesday.

natan-abolafya commented 8 months ago

Hey, thanks. I've tested it and it did not help as you expected.

digitalcoyote commented 8 months ago

I found an issue we only encountered on a publish, but it ended up not being related to this one at all. I'm starting to think the library I use for command line option parsing may be an issue but it's the standard System.CommandLine. I may try an alternative package and see if that will solve the problem but I'm going to research that route first to see how they are parsing the options.