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

Do not output helplink for custom check diags #10923

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JanKrivanek
Copy link
Member

Context

Buildcheck is outputing helplinks as part of its diagnostics - e.g.:

warning BC0201: https://aka.ms/buildcheck/codes#BC0201 - Property: 'GITHUB_TOKEN' was accessed, but it was never initialized.

However the link is missleading for custom checks - as we do not have any help for those

warning DU0201: https://aka.ms/buildcheck/codes#DU0201 - Location: 'C:\Users\jankrivanek\Downloads\secrets-redaction-demo-main\secrets-redaction-demo-main' cannot be fully trusted, place your projects outside of that folder (Project: secrets-redaction-demo.csproj).

Changes Made

The help link is provided only for internaly reported diagnostics

Testing

Existing tests

@baronfel
Copy link
Member

baronfel commented Nov 1, 2024

Alternatively we should allow for custom checks to register a help URL as part of their definitions/reports. We can decide separately to render that or not but the ability to link to external documentation is pretty fundamental to all modern diagnostic reporting experiences (see VS and VSCode problem displays for example).

@JanKrivanek
Copy link
Member Author

Alternatively we should allow for custom checks to register a help URL as part of their definitions/reports. We can decide separately to render that or not but the ability to link to external documentation is pretty fundamental to all modern diagnostic reporting experiences (see VS and VSCode problem displays for example).

I was woried of security concerns. But now I can see there is a prior art for this: https://learn.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.diagnosticdescriptor.helplinkuri

Lm try to imporove here

@JanProvaznik
Copy link
Contributor

JanProvaznik commented Nov 4, 2024

I was woried of security concerns.

What is the concern? That users would click on links that seem trustworthy because they come from MSBuild or something else?

Perhaps we could wrap the link to something like https://aka.ms/CustomBuildCheck?name=<checkName>&helplink=<https://usersBClink.com> and that page would be just saying that this check is not microsoft provided and to proceed if you trust it.

@JanKrivanek
Copy link
Member Author

I was woried of security concerns.

What is the concern? That users would click on links that seem trustworthy because they come from MSBuild or something else?

Perhaps we could wrap the link to something like https://aka.ms/CustomBuildCheck?name=<checkName>&helplink=<https://usersBClink.com> and that page would be just saying that this check is not microsoft provided and to proceed if you trust it.

Yes - exactly this

Let's wait with wrapping till we see some initial usages of cusom buildchecks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants