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

make policheck nearly clean #4992

Merged
merged 3 commits into from
Jul 22, 2024
Merged

Conversation

danmoseley
Copy link
Member

@danmoseley danmoseley commented Jul 20, 2024

  • plotly is full of geopolitical terms that are assumed fine in context and it's not something we control anyway
  • country and capital are fine in context so removed them where they were arbitrary strings
  • tested locally with policheck.exe.

this makes us 98% clean which makes it easy to eyeball violations.

Microsoft Reviewers: Open in CodeFlow

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Jul 20, 2024
@danmoseley danmoseley requested a review from joperezr July 20, 2024 04:59
@danmoseley danmoseley added area-engineering-systems and removed area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication labels Jul 20, 2024
@danmoseley
Copy link
Member Author

@julioct also a different scanner detected the secret added in your change. I am excluding it as hopefully it is not in fact a secret...

@danmoseley
Copy link
Member Author

Latest plotly is actually 2.34 .. maybe we need a readme next to it to say "update policheck xml when you update plotly version"

@julioct
Copy link
Contributor

julioct commented Jul 20, 2024

@julioct also a different scanner detected the secret added in your change. I am excluding it as hopefully it is not in fact a secret...

It was a password for a test user used in the Keycloak playground. It was already removed in this commit

This reverts commit 4907e57.
@danmoseley
Copy link
Member Author

Great, reverted that commit

@danmoseley danmoseley merged commit 935adbb into dotnet:main Jul 22, 2024
9 checks passed
@danmoseley danmoseley deleted the cleanpolicheck branch July 22, 2024 19:12
@danmoseley
Copy link
Member Author

@JamesNK @adamint note this file needs an update when getting a new plotly.

@JamesNK
Copy link
Member

JamesNK commented Jul 23, 2024

Latest plotly is actually 2.34 .. maybe we need a readme next to it to say "update policheck xml when you update plotly version"

Are wildcards supported? e.g.

<Exclusion Type="FileName">plotly-*.js</Exclusion>

@danmoseley
Copy link
Member Author

No mention in the doc linked, but I should have confirmed by trying it..

@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants