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

Enable security scanning of PR's (WIP, hold for now...) #72

Merged
merged 5 commits into from Nov 21, 2021

Conversation

wokket
Copy link
Collaborator

@wokket wokket commented Oct 9, 2021

Simple change to hopefully ensure we're doing the right things...

Having got a first run it takes a few minutes, and is currently reporting no issues 👍

TODO:

  • Remove new workflow file if we're going to scan in CI build
  • Add a positive security issue in the branch temporarily to ensure check fails the build

Simple change to hopefully ensure we're doing the right things...
@wokket wokket changed the title Enable security scanning of PR's Enable security scanning of PR's (WIP, hold for now...) Oct 9, 2021
For simplicity/consistent scanning...
@erikbra
Copy link
Owner

erikbra commented Oct 9, 2021

Nice initiative! I haven't used these before, maybe you could explain a bit, when you fell that it's ready for talking about?

@wokket
Copy link
Collaborator Author

wokket commented Oct 9, 2021

It's just static code analysis at build time, see https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/about-code-scanning

In theory it should result in a failed CI build if it finds insecure coding practises, but I need to add a positive to check that.

The results can also be viewed in the security menu above (image
), then "Code Scanning Alerts"

@wokket
Copy link
Collaborator Author

wokket commented Oct 9, 2021

I generally enable the 'dependabot' in my repos as well, it will alert you when there's a security update for one of your nuget dependencies, and optionally raise a PR that updates the dependency for you... I don't believe I have permission to configure that on your repo though.

@erikbra
Copy link
Owner

erikbra commented Oct 11, 2021

Btw, @wokket - I've enabled the dependabot for github actions and nuget packages. It seems to be just a file now (don't know what it was earlier): https://github.com/erikbra/grate/blob/main/.github/dependabot.yml

@wokket
Copy link
Collaborator Author

wokket commented Oct 15, 2021

Just a quick update on this one, something has moved with the pre-release .Net6 so the 'autobuild' line now fails in my fork (https://github.com/wokket/grate/tree/security/code-scanning)... I vote we still hold off here until the RC world cleans up a bit.

@erikbra
Copy link
Owner

erikbra commented Oct 16, 2021

Looks like the security build step is using .NET core 3.1:

/usr/share/dotnet/sdk/3.1.302/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(234,5): error NETSDK1005: Assets file '/home/runner/work/grate/grate/grate/obj/project.assets.json' doesn't have a target for '.NETFramework,Version=v6.0'

It seems to be running on mono/.net full framework: https://github.com/wokket/grate/runs/3910208595?check_suite_focus=true#step:4:1230

Or, maybe the problem is with the SDKs that are installed:

https://github.com/wokket/grate/runs/3910208595?check_suite_focus=true#step:4:45

  Running dotnet --info
  .NET SDK (reflecting any global.json):
   Version:   5.0.401
   Commit:    4bef5f3dbf
  
  Runtime Environment:
   OS Name:     ubuntu
   OS Version:  20.04
   OS Platform: Linux
   RID:         ubuntu.20.04-x64
   Base Path:   /usr/share/dotnet/sdk/5.0.401/
  
  Host (useful for support):
    Version: 5.0.10
    Commit:  e1825b4928
  
  .NET SDKs installed:
    2.1.302 [/usr/share/dotnet/sdk]
    2.1.403 [/usr/share/dotnet/sdk]
    2.1.526 [/usr/share/dotnet/sdk]
    2.1.617 [/usr/share/dotnet/sdk]
    2.1.701 [/usr/share/dotnet/sdk]
    2.1.818 [/usr/share/dotnet/sdk]
    3.1.119 [/usr/share/dotnet/sdk]
    3.1.202 [/usr/share/dotnet/sdk]
    3.1.302 [/usr/share/dotnet/sdk]
    3.1.413 [/usr/share/dotnet/sdk]
    5.0.104 [/usr/share/dotnet/sdk]
    5.0.207 [/usr/share/dotnet/sdk]
    5.0.303 [/usr/share/dotnet/sdk]
    5.0.401 [/usr/share/dotnet/sdk]

No .net 6. We can hold off until it goes GA, no probs, it's live in less than a month: https://www.dotnetconf.net/

@wokket
Copy link
Collaborator Author

wokket commented Oct 16, 2021

Unfortunately that doesn't explain why it used to build, per the green ticks all through this PR 🤣

Edit: actually, maybe it never ran? I should know better than to try and review this stuff on my phone.... Either way I agree, housing off until we have an rtm release for this isn't the end of the world

@erikbra
Copy link
Owner

erikbra commented Nov 21, 2021

I think I'll just merge this one, and we can work more on polishing it later :)
I got it to work on .NET 6

@erikbra erikbra merged commit e09241d into main Nov 21, 2021
@erikbra erikbra deleted the security/code-scanning branch November 21, 2021 20:58
@erikbra erikbra added this to the 1.0.0 milestone Nov 21, 2021
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.

None yet

2 participants