Feedback and Review

[a-a-ron]

Hey @brianamarie and @hectorsector

For reference: https://lab.github.com/githubtraining/security-strategy-essentials/8.1.0

Above is my up-to-date version for the workshop security course. I'd love some feedback on the logic and making sure this is flowing nicely.

The responses are not complete but hopefully they have enough info to guide you through it. If you need more info, I have notes in the config file on each step.

Also, the step to add dependabot took about 5+ minutes to work. So, I instead took the route of providing instructions to install it for automatic dependency scanning in just an issue that the learner closes when done instead of building a step around it.

Specific items for feedback:

• Step 6: The security policy is added to a new PR. Maybe adding a branch protection is needed here so the learner doesn't commit to master?
• Step 7: This step won't complete. Not sure why... thoughts?
• Step 12: I'm trying to figure out how to complete this step. I need to remove the historical commits that introduced the .env file at the beginning of the history. I looked at this lightning talk resource (https://github.com/github/support-security-ombuds/blob/master/education/lightning-talks/removing-sensitive-data.md) that was helpful. Maybe we validate on the .env file instead of the commit? I'm not sure how to do this one. Looking for ✨ suggestions here!

Thanks!!

[brianamarie]

Hi @a-a-ron! I went through the course (until step 8) and it's AWESOME! 🎉 You're right, it is familiar 😄 A few specific notes:

• Step 2: We can give a URL to the dependency graph, combining steps 1 and 2 and providing a link: {{ repoUrl }}/network/dependencies
• LOVE where dependabot step is, flow makes lots of sense. Is that actually installing another app, or is it opting in to automated security fixes?
• Security policy - do we commit directly to master? I just did 🤷 Master should probably be protected to force a new PR, and I think that branch should probably also be protected to keep rogue people like me from messing it up :)
• Where we ask the user to approve, we should request it through an action
• I got stuck at the step where I submitted a pull request review - I think that's happening because the event is "pull_request_review", but the component type "mergePullRequest" on like 214 may have a problem with that. I tried to change that but it didn't fix it. :(
• The rest of the flow looks really great! It is hard to say 100% without going through it in the UI, but I think this looks good to go once the responses are finished and step 8 works. 🎉