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

GDPR Privacy issues with Moq, starting from version 4.20 for any contributor to the SDK #38111

Closed
danielmarbach opened this issue Aug 9, 2023 · 13 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. EngSys This issue is impacting the engineering system. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@danielmarbach
Copy link
Contributor

danielmarbach commented Aug 9, 2023

You might want to consider locking Moq to < 4.20 (and prevent any dependency updates) or replacing it

https://github.com/Azure/azure-sdk-for-net/blob/main/eng/Packages.Data.props#L269

as long as https://github.com/moq/moq/pull/1373 is not merged

More context in https://github.com/moq/moq/issues/1372

@github-actions github-actions bot added customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Aug 9, 2023
@Joe4evr
Copy link

Joe4evr commented Aug 9, 2023

Of note: https://github.com/moq/moq/pull/1375 reverted the SponsorLink change, but only because it broke builds on macOS and Linux. We can't be certain it won't be back later.

@jsquire jsquire self-assigned this Aug 9, 2023
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. labels Aug 9, 2023
@github-actions github-actions bot removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 9, 2023
@jsquire
Copy link
Member

jsquire commented Aug 9, 2023

Hi @danielmarbach. Thanks for the heads-up. Like the rest of the community, we're very concerned with the privacy implications of this change and have no plans on upgrading to a version of Moq with this behavior.

We're currently locked to v4.18.2 and will stay there in the short-term. For now, we're going to monitor the conversation and the Moq project's response. Given the community reaction to the change, we're hopeful that Moq will reconsider and remove the SponsorLink functionality and any scraping of local data along with it. If we see indications that Moq intends to continue to include the functionality going forward, we'll begin exploring a move to another mocking framework without the privacy concerns.

@jsquire jsquire added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Aug 9, 2023
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

Hi @danielmarbach. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

@tonyqus
Copy link

tonyqus commented Aug 9, 2023

Is it possible that Azure team/Microsoft sponsor Moq project? I think the major problem for Moq team is that they are lacking of enough sponsorship. If there is enough sponsorship, SponsorLink project is not needed at all.

@jsquire
Copy link
Member

jsquire commented Aug 9, 2023

Is it possible that Azure team/Microsoft sponsor Moq project? I think the major problem for Moq team is that they are lacking of enough sponsorship. If there is enough sponsorship, SponsorLink project is not needed at all.

While I love the suggestion, unfortunately, that decision is outside of my area of influence. It looks like the best path to make that inquiry would be via one of the contact methods listed on the Microsoft GitHub organization page. Tweeting @OpenAtMicrosoft would be a great way to start a public conversation and help them gauge community support.

@r-pankevicius
Copy link

I'd suggest Microsoft to prepare "Mingration from Moq to " guide. something-else could be NSubstitute or something else.

I've never used NSubstitute, this was just another similar library I've heard about today. But I've spent last 2 hours removing Moq and commenting out test affected classes. That reduced number of tests from circa 1100 to 700. Tomorrow we'll decide next steps.

@r-pankevicius
Copy link

I also found AutoFixture.AutoMoq package is dependent on Moq. There could be more of those. So I'd suggest "nuget ban" in the repo to prevent such situation. I.e. if "nuget ban" file contains a fact that Moq is banned then NuGet doesn't install a package that depends on it.

@danielmarbach
Copy link
Contributor Author

danielmarbach commented Aug 9, 2023

@r-pankevicius I was only aware of the direct Moq reference. The thing is with the direct package reference that will overrule the transitive references that are coming from AutoFixture.AutoMoq as far as I understand since the Moq reference is in the centrally reference props file (or at least that is my limited outside view of the problem)

@jsquire
Copy link
Member

jsquire commented Aug 9, 2023

@r-pankevicius: To my knowledge, there is no use of Autofixture.AutoMoq in this repository. Can you help me understand if you're just highlighting the risk of transitive dependencies in general or whether you're seeing a reference that we've overlooked?

@r-pankevicius
Copy link

@jsquire No worries then, I found it in our team repo. Just wanted to spread the insight.

@Joe4evr
Copy link

Joe4evr commented Aug 10, 2023

@r-pankevicius You'll probably want this: @basvd in one of Moq's own threads posted this MSBuild Target which will block SponsorLink from accidentally appearing anywhere in your transitive dependency chain:

<Project>
    <Target Name="CheckBlockedPackages" AfterTargets="ResolvePackageDependenciesForBuild">
        <Error Code="420" Text="Blocked package dependency detected: %(PackageDependencies.Identity)" Condition="'%(PackageDependencies.Identity)' == 'Devlooped.SponsorLink'" />
    </Target>
</Project>

@Atulin
Copy link

Atulin commented Aug 10, 2023

Worth noting, 21 packages use SponsorLink package which is the core of the issue. I'd say checking the transitive dependencies would be prudent.

@github-actions
Copy link

Hi @danielmarbach, since you haven’t asked that we /unresolve the issue, we’ll close this out. If you believe further discussion is needed, please add a comment /unresolve to reopen the issue.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. EngSys This issue is impacting the engineering system. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

6 participants