devlooped / moq

The most popular and friendly mocking framework for .NET
Other
5.94k stars 802 forks source link

Warnings with latest version from SponsorLink #1370

Closed GeorgDangl closed 1 year ago

GeorgDangl commented 1 year ago

Hey team,

I appreciate the effort, and really like Moq, thank you for creating it!

In one project, I just did a dependency update and noticed that I get MOQ101 warnings when building in Visual Studio, along with a behavior that seems to block the build for a few hundred milliseconds: image

It looks like this was introduced in in https://github.com/moq/moq/releases/tag/v4.20.0 with #1363.

Is there a way to disable the warnings, e.g. via a setting in the *.csproj file of the test projects or similar?

njannink commented 1 year ago

I tried to suppress the warning using

<NoWarn>$(NoWarn);MOQ101</NoWarn>

But then I get the following empty warning.

image

If this SponsorLink is going to be a requirement then Moq will no longer will pass our acceptance criterea. Our company GDPR rules won't allow leaking all developer internal email addresses, meaning we will be forced to switch an alternative mocking framework.

DanielCordell commented 1 year ago

I'm likely in the same boat. I've made an issue on the sponsorlink repo too highlighting some potential issues, https://github.com/devlooped/SponsorLink/issues/16, but I'd hate to have projects basically become unusable in company environments over something like this, which I really do believe is a genuinely noble thing.

Aculeo commented 1 year ago

This is just unacceptable, free or not. It is not even mentioned on the release notes. Suddenly having a proprietary blob DLL added that sends data to a cloud service is big no and we will avoid Moq in the future.

martincostello commented 1 year ago

The MSBuild target in this GitHub gist may work around the issue with 4.20.0 and 4.20.1 (I'm not conclusive of that from my experimentation in Visual Studio, and given caveat emptor) until a solution that's broadly acceptable to the community is reached.

I'm supportive of Moq getting sponsorship for the value it brings, but the implementation added here is disruptive and raises privacy concerns, particularly for non-corporate sponsored usage of Moq in other FOSS.

girlpunk commented 1 year ago

Additionally, the library being used to do this has now licence. As such, Moq will be unusable for most (if not all) business/commercial uses.

Atulin commented 1 year ago

Some deeper discussion in #1372. As it turns out, SponsorLink also gets it's config from a remote location, and hides itself for the first few days...

zarlo commented 1 year ago

Some deeper discussion in #1372. As it turns out, SponsorLink also gets it's config from a remote location, and hides itself for the first few days...

so just like a virus

dmsch commented 1 year ago

Malware in a dependency is in no way acceptable.

kzu commented 1 year ago

Heya folks: added a note to get you at ease with this change: https://github.com/devlooped/SponsorLink/blob/main/readme.md#privacy-considerations

At no point is the email sent to anyone, unless you EXPLICITLY authorize so on GitHub itself.

Would making the SponsorLink analyzer itself opensource change things at all? Also, wouldn't that defeat the purpose of the check if people just saw how it's performed and simply bypass it?

NOTE: there is NOTHING done on CLI / CI, there is no run-time dependency, it's just an IDE feature.

jameshilliard commented 1 year ago

Would making the SponsorLink analyzer itself opensource change things at all?

Dependencies that can't be easily audited are inherently a security risk, especially when they have the ability to perform network communications.

Also, wouldn't that defeat the purpose of the check if people just saw how it's performed and simply bypass it?

Can't it already be bypassed by doing something like #1373?

it's just an IDE feature

Potentially compromising an IDE is still a significant security risk.

Atulin commented 1 year ago

At no point is the email sent to anyone, unless you EXPLICITLY authorize so on GitHub itself.

NOTE: there is NOTHING done on CLI / CI, there is no run-time dependency, it's just an IDE feature.

Source: trust me bro

Aculeo commented 1 year ago

It doesn't really solve the problem though: we can only trust that the DLL is safe and works as advertised.

And all of this just to nag developers to sponsor the library? It is unfortunate that so many companies don't contribute back to open source libraries, that should definitely change, but this is not the solution.

GeorgDangl commented 1 year ago

Heya folks: added a note to get you at ease with this change: https://github.com/devlooped/SponsorLink/blob/main/readme.md#privacy-considerations

At no point is the email sent to anyone, unless you EXPLICITLY authorize so on GitHub itself.

Would making the SponsorLink analyzer itself opensource change things at all? Also, wouldn't that defeat the purpose of the check if people just saw how it's performed and simply bypass it?

NOTE: there is NOTHING done on CLI / CI, there is no run-time dependency, it's just an IDE feature.

Hey @kzu, thank you for the reply!

I was very surprised yesterday when I noticed that new behavior. As I said, I get your reasoning for this, but I think it went way too far. With just doing a package upgrade, there was suddenly code running during build time that analyzed my local computers configuration. Additionally, it seems that there are web requests being sent to a command & control server. This, combined with the fact that SponsorLink is not only closed source but even obfuscated, makes it just look very untrustworthy.

Analyzers are supposed to locally analyze the code, as @sharwell made you aware of in April: https://github.com/devlooped/SponsorLink/issues/10

I can not imagine a scenario where any company would allow such a behavior.

Personally, I don't believe there was malicious intent on your side, but the whole behavior is so close to malware. And, additionally, we're introducing a whole new attack vector by allowing SponsorLink outside communication during build times.

JackGilmore commented 1 year ago

Regardless of the privacy and security concerns that have been raised, I don't believe using the error list is an appropriate method for conveying sponsorship information. The purpose of the error list is to highlight problems or potential issues with the code you're writing, not for advertisement.

I agree that open source maintainers deserve financial support, as I'm sure do many other developers, but this is not the right way to go about it.

jameshilliard commented 1 year ago

I don't believe using the error list is an appropriate method for conveying sponsorship information.

True, if all open source transient dependencies used by a project were to exhibit similar behavior it would cause serious usability issues as everyone would be inundated with spurious warnings/errors.

mikocot commented 1 year ago

Regardless of the privacy and security concerns that have been raised, I don't believe using the error list is an appropriate method for conveying sponsorship information. The purpose of the error list is to highlight problems or potential issues with the code you're writing, not for advertisement.

I agree that open source maintainers deserve financial support, as I'm sure do many other developers, but this is not the right way to go about it.

to be fair he wouldn't be the first one to do so, I've seen it several times with some extensions or libraries, but definitely not a way to go

BTW you can report the library at nuget for its doubtful privacy standards: https://www.nuget.org/packages/Devlooped.SponsorLink/1.0.0/ReportAbuse

saliej commented 1 year ago

Are there any plans to change this behaviour? This is a massive problem for me right now because I've just updated all our packages to conform to our company's new security mandate and I've got to report on progress to the SecOps team tomorrow.

hilari0n commented 1 year ago

The release notes for Moq 4.20.2 seem to suggest, that this version does not contain this dubious mechanism, although it may be temporary, as the reason is that it breaks builds on MacOS, so if they manage to address this for MacOS, then the mechanism may be put back into Moq.

hahyes commented 1 year ago

Are there any plans to change this behaviour? This is a massive problem for me right now because I've just updated all our packages to conform to our company's new security mandate and I've got to report on progress to the SecOps team tomorrow.

@saliej Version 4.20.2 is free of this crap. At least for now. Consider change to NHibernate or other library.

aguimaraes commented 1 year ago

@kzu what is the decision on this? is the removal temporary or permanent?

brenthompson2 commented 1 year ago

Dang, removing Moq is definitely gonna be an expensive refactor of our corporate codebase... better start writing some technical debt items...

Drag13 commented 1 year ago

I know how it feels when you work hard without valuable sign of appreciation. This hurts, really.

But installing any extra package without clear consent is not an option, really. It burns the trust to the package and, what even worse to the Open Software, to all of us who doing Open Source. This hurts even worse :'(

Stay safe and strong, the package is great and very well known across the world. I believe you will find the funding in another way

SeanKilleen commented 1 year ago

In case anyone is interested in my long-form thoughts on this, figured I'd write it up as a blog post rather than fracture it across GitHub threads: https://seankilleen.com/2023/08/on-moq-and-our-part-in-the-oss-sustainability-social-contract/

aguimaraes commented 1 year ago

@kzu I'm sure sure you know this already, but I would like to offer more context. The SponsorLink change removes the decision to use Moq from the technology helm. I must report to my legal team immediately and prioritize the migration away from it because of licensing and privacy concerns. I'd like to have my teams delivering business value instead of migrating away from Moq, so I would appreciate a definitive answer to my prior question: was the decision to use SponsorLink reverted permanently?

kfrancis commented 1 year ago

I've sponsored but also will be making sure to specifically remove this during builds.

mjameson-se commented 1 year ago

Out of curiosity, I hopped over to check out how many new forks popped up since yesterday. At least five new forks were created removing SponsorLink. I'm not a lawyer but I think the license would preclude that. However, that alone isn't going to stop people from doing it. In the future, I might consider avoiding dependencies on OSS libraries whose license does preclude simply forking away from undesired additions. In the meantime, I'm content to pin to 4.18 until this all gets sorted out

IanBUK commented 1 year ago

Inserting compiler warnings almost as an advert?

Game over man, game over.

alexchandel commented 1 year ago

The road to bleak dystopia is paved with good intentions. Pausing our build until we install a binary ad blob is just another stone. Don’t think for one moment that closed-source suppliers won’t adopt this.

Please type “McDonalds” into your IDE to continue your build, or upgrade to premium Json.NET for $9.99/mo.

Say McDonalds

merklegroot commented 1 year ago

Next time, don't collect people's information without making it incredibly clear in advance exactly what you are going to do.

Our org is done with Moq as is every other org that cares about security.

I'll miss using this great library, but I'm going to comb through our codebase and remove every reference to it.

Goodbye.

merklegroot commented 1 year ago

@kzu I'm sure sure you know this already, but I would like to offer more context. The SponsorLink change removes the decision to use Moq from the technology helm. I must report to my legal team immediately and prioritize the migration away from it because of licensing and privacy concerns. I'd like to have my teams delivering business value instead of migrating away from Moq, so I would appreciate a definitive answer to my prior question: was the decision to use SponsorLink reverted permanently?

Does it matter? I have no reason to trust them.

aguimaraes commented 1 year ago

Does it matter? I have no reason to trust them.

I operate with zero trust in this or the possible replacement. If they abort the decision permanently, I can take my time to assess a better replacement; if not, this is an immediate (and costly) change.

It doesn't matter anymore because of the lack of response here and his attitude in other threads already decided for me.

thardy commented 1 year ago

Perfect example of how to completely kill an open source project. Well done.

Open source culture and values:

I'm pretty sure you nailed all three (as in, destroyed). There's a simple way out though - own the mistake, and reverse the mistake. The community can be forgiving. Anything less and you're sunk.

kzu commented 1 year ago

Hello folks. Now that SponsorLink itself is OSS, please head over to the repo and follow/comment on the issue of warnings specifically: https://github.com/devlooped/SponsorLink/issues/32

MrRosendahl commented 6 months ago

Moq = sadly zero trust from now on

DanielCordell commented 6 months ago

Moq = sadly zero trust from now on

@MrRosendahl was this comment, which sent an email to all 27 people attached to this 8 month old closed issue, entirely necessary?

(yes I see the hypocrisy).