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

Devlooped.SponsorLink must not perform I/O outside of compiler APIs #10

Closed
sharwell opened this issue Apr 5, 2023 · 6 comments
Closed
Labels
wontfix This will not be worked on

Comments

@sharwell
Copy link

sharwell commented Apr 5, 2023

The Roslyn analyzer contract does not allow for analyzer I/O outside of the constructs provided by the APIs (e.g. using AdditionalFiles to pass external data to an analyzer). Analyzers should not side-step these mechanisms, either through direct file I/O or through web invocations.

@sharwell sharwell added the bug Something isn't working label Apr 5, 2023
@kzu kzu added wontfix This will not be worked on and removed bug Something isn't working labels Apr 10, 2023
@kzu
Copy link
Member

kzu commented Apr 10, 2023

SponsorLink is not using any undocumented APIs. Moreover, it makes every effort to only do I/O in the context of a full IDE-only build (so, no CLI or design-time builds). For its purposes, there is just no other way to do what it does.

If there is a particular performance issue you're seeing, I'd be happy to investigate. But as a generic blank request, this isn't actionable on my side.

Thanks for taking a look at this though!

@SimonCropp
Copy link

@kzu u should lock this

@warren-lockhart
Copy link

@kzu Why was this issue closed?

@cmjdiff
Copy link

cmjdiff commented Aug 10, 2023

@kzu Why was this issue closed?

Because he wants to keep running his spyware.

@thomaslevesque
Copy link

@kzu I think you shouldn't ignore @sharwell's remark. Your package could be banned from NuGet for not respecting the analyzer contract.

For its purposes, there is just no other way to do what it does.

I think there is. The SponsorLink Github app could generate a file with details about the sponsorship, and sign the file using a private key. This file would be added to the repo, and included in the project via AdditionalFiles. The analyzer could then access the file via the Roslyn API, validate the signature using the public key (probably embedded in the binary), and validate the sponsorship details.

Pros:

  • No network call, works offline
  • No collection of PII
  • Tamper-proof, because of the signature
  • More reliable than getting the user's email via git config

Cons:

  • The user would need to frequently update the sponsorship details file, so that the analyzer can tell if the sponsorship is still active.
    • To alleviate this issue, the GH app could automatically create a PR to update the file

I don't know enough about this project to figure out all the details, but I think it's an approach worth considering.

To be clear, I'm still not sold on the whole SponsorLink approach, but I think that if it:

there would probably be much less opposition to it (of course, now that the damage is done, it might already be too late to change people's minds...)

@kzu
Copy link
Member

kzu commented Aug 16, 2023

Great suggestions! They are in line with #31. I like the idea of the analyzer getting the file vía AdditionalFiles 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

6 participants