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

Guide on Code Security Scanning #148

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

Guide on Code Security Scanning #148

wants to merge 10 commits into from

Conversation

ingyhere
Copy link
Contributor

@ingyhere ingyhere commented Mar 19, 2024

Purpose

  • Add new security scanning guide focused on the NASA SCRUB tool

Proposed Changes

  • [ADD] README contents

Issues

Testing

@ingyhere ingyhere changed the title Addition of SCRUB guide DRAFT Guide on Code Security Scanning Mar 19, 2024
@riverma riverma added the software lifecycle Process improvements involving developing, testing, integrating, deploying software label Mar 26, 2024
@riverma
Copy link
Collaborator

riverma commented Mar 26, 2024

@ingyhere - just a note, adding labels for the type of SLIM best practice category each PR applies to (i.e. governance, software lifecycle, information sharing) helps to make future release notes more readable. See information about categories here.

Also - adding the SLIM Project Board in the PR right hand menu, and tagging the status as well as the iteration helps people understand the time line for the PR.

@ingyhere ingyhere changed the title DRAFT Guide on Code Security Scanning [DRAFT] Guide on Code Security Scanning Mar 26, 2024
Copy link
Collaborator

@riverma riverma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great progress on this! Added some comments to consider. One thing additional to note, just remember to add a registry entry so that the guide appears properly in our search. See directions here.

First cut at filling in the details on security scans
Remove TOC
pip3 install --upgrade --user nasa-scrub
```

2. **Configuration**
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we ship this guide with some working scrub.cfg file examples the reader can just drop in and start using?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm planning to rename it as scrub.yml and, yes, I'll try to add some examples for scrub.cfg.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - you were thinking of renaming what to scrub.yml?

I'll try to add some examples for scrub.cfg

Awesome! If we can include those files not just as snippets but stand-alone files within /security-scanning, that would allow our infusion automation to easily pick up and propose a solution via pull requests. Now that I think of it: if I wanted to say propose the recommendations you all have put down here via a pull request to a given project repository, which files would I need to propose? A scrub.cfg and a GitHub Action workflow file? Again - having not just a snippet but a file version of the two would make infusion automation much easier to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, to clarify,

Workflow file (example in slim-starterkit-python):
codeql.yml -> scrub.yml

The config file would be the same name...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it @ingyhere - that makes sense!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, not resolved yet. Needs a little more testing.

matrix:
# CodeQL supports ['cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby']
# Learn more about CodeQL language support at https://git.io/codeql-language-support
language: ['python']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more of a question: what happens if I enable all languages? Does CodeQL get confused or is it smart enough to ignore the irrelevant languages? If the latter - can we just list all languages by default so readers don't need to change anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases it should automatically build correctly. But there is an additional step that can be implemented (see comments) to manually build. This is a bit more involved, see this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After re-reviewing, the comments, ordering and documentation here need a bit of work.

@ingyhere
Copy link
Contributor Author

@ingyhere - just a note, adding labels for the type of SLIM best practice category each PR applies ...

Also - adding the SLIM Project Board in the PR right hand menu, and tagging the status ...

Done

@nutjob4life
Copy link

nutjob4life commented May 2, 2024

@jpl-jengelke, quick favor: when this moves out of "draft" state, could you ping me @nutjob4life? I tend to mute draft PRS both logically and mentally 😇 Nevermind, I saw it go out of draft "live" during the tag-up meeting on 2024-05-02

@riverma riverma marked this pull request as ready for review May 2, 2024 16:25
@riverma riverma changed the title [DRAFT] Guide on Code Security Scanning Guide on Code Security Scanning May 2, 2024
Copy link

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superbly written guide with a great cadence and feel as well as utility. Should make SCRUB a much easier pill to swallow. Bravo! 🎉

@ingyhere
Copy link
Contributor Author

ingyhere commented Jun 6, 2024

Superbly written guide with a great cadence and feel as well as utility. Should make SCRUB a much easier pill to swallow. Bravo! 🎉

Unfortunately as things go ... I see some areas for improvement. But I will make changes and ask for re-review.

…r indentation, add registry entry. More to come ...
… there will be a semi-minor rewrite to clarify build requirements in the context of multiple languages. ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
software lifecycle Process improvements involving developing, testing, integrating, deploying software
Projects
Status: 🏗 In Progress
Development

Successfully merging this pull request may close these issues.

None yet

5 participants