In both PRs, we tell the user to merge, and the formatting is inconsistent.

One thing that comes to mind is branch protection. It would be cool to see people enable required reviews and commit signatures. I'm not sure how difficult that would be to implement in the learning lab.

From #81

You’re done this course! But a good thing to do now is uninstall this app from your repository.

• ✨ @jasonetco

When considering the security of your repository, it is important to know what applications are installed on it. As much as it pains me to leave you, I want you to uninstall me, I won't be able to congratulate you on achieving this task, but just know I'm excited about your progress.

• ✨ @beardofedu

Text

• "I've opened another issue with the next steps." not formatting properly
• After user comments with version, include "I can't check what the most recent version is, so I'll take your word for it."

Got stuck because I think the course can't move on because I changed the template repo, and that requires some database magic. @JasonEtco maybe can help out?

After event where user edits .gitignore and we pass the gate, the bot needs to say it's OK to merge.

"Is your repository public? If so, skip below to step 4. These features are enabled by default on public repositories. If this is a private repository, continue with the instructions here." isn't right. the users who have public will still need to do steps 1 and 4, not only 4.

Updating the package-lock.json directly will be difficult, and also isn't good JavaScript best practice.

Would it be better for us to introduce a pull request with the changes, and have them review?

Instead of introducing a .env file, let's already have one, and have the user add it to the .gitignore and also change the title of the .env to .env-example.

I tried to join, and it said there was an error. I tried again and it said I was already registered. Then, I refreshed, and it said I might need to install and the repository was being created. When I did that, I couldn't, because it's already installed. The repository is created but the landing page is not right on LL.

cc @JasonEtco (ps do we have a @githubtraining/learning-engineering) team?

Currently we have two vulnerable dependencies, and it will make instructions simpler if we only use 1. Neither are integral to the code's functionality.

@githubtraining/trainers When thinking of a possible project, we need something that:

• Will be able to show an end product on GitHub Pages so we don't spend too much time discussing deployment, and also don't have them build something they can't see
• Has dependencies

Based on @JasonEtco's ✨ work to create the octocat game, and @ppremk's initial idea to use it for this project, I've created this repository with the code that fits, and it can be played here.

We may need to tweak a few things if we decide to go forward with this as the project, namely:

• The instructions currently have them only change the package.json file, but they would also need to change the package-lock.json. Can this be done without running npm install locally?
• We currently have instructions that would introduce a branch with a completely new .gitignore file, but for the repository to not include the node_modules directory, that already needs to exist. How can/should we change that activity so it still focuses on adding a .env file, but already has an open branch? Do we need to have an open PR for them?

• Update first event action to be enabling GH pages, not closing the issue
• Have bot close issue as part of reaction

Great work. Because of the available API endpoints, I can't check to see if you've completed the first few steps. But, I can see that you've closed this issue.
Make sure to complete the first few steps in your settings, and you should be able to see your published project site on GitHub Pages. I've opened another issue with the next steps.

Great work. Make sure to complete the first few steps in your settings, and you should be able to see your published project site on GitHub Pages. (Because of the available API endpoints, I can't check to see if you've completed the first few steps, but I can see that you've closed this issue.)

<hr>
<h3 align="center">I've opened another issue with the next steps.</h3>

Issue 1

• Are "step 1" type things consistent?
• Enabling GitHub Pages should either be a step or it shouldn't be.
• Numbering is broken
• Instructions have us to go user settings, not repository settings
• Step 1 should say "enable security features and close issue"
• All of the "selecting" information is too much. we should leave it out or link to docs.

Comment 1

• Remove the "sometimes I respond too fast" because it doesn't make sense there

Issue 2

• Remove at least 1 full paragraph of text, it's too much
• Add a step after 1 and before 2 to click "dependencies" on the left hand side bar
• Remove node-mime as a dependency in general in template

Comment 2

• TOO LONG
  • Note: You might notice that this project has a package.json file, but no package-lock.json file. We are doing all parts of this activity on GitHub.com, instead of working with anything locally. If you work with a Node project locally, you might notice some slight differences, but the main concepts discussed here will still completely apply.

If the user is supposed to complete another action (like enable pages) but they instead close the issue, should the bot do anything?

Options:

• Bot does nothing
• Bot reopens issue and comments with instructions again

How are we handling this in other courses?

• Edit 03_found-vulnerability.md so it doesn't duplicate information already given

• Since the bot is waiting for GH pages to be deployed, it could take some time, and we should note that like we do in the pages course.

Turning on GitHub Pages creates a deployment of your repository. I may take up to a minute to respond as I await the deployment.

Currently, the user can make any commit on the add-gitignore branch and the tests will pass. 😦

Based on @gregose's feedback in #81:

Another item would be to have someone commit an access token and have it revoked with the credential scanning feature... but thats a bit tricky to implement in a safe way. I'm not sure what we could do there. Maybe a dummy token we could scan for? @ptoomey3 (not part of this org yet) may have some ideas.

This is something we started with, but took out. The possible benefits would be:

• Raise awareness of token scanning as a feature
• Let people know they shouldn't be committing tokens

The reasons we decided to take them out are:

• For the flow to happen as a step, we would actually ask them to commit a token, which ultimately isn't a behavior we want users to do. In general, if a user shouldn't do it in every day life, we don't want them to practice it during a course
• When the token is added to a repository, the user who added the token isn't notified at all. The notifications (from what I understand) all take place on the other end, so that the service provider who owns that token will be the one notified, and the token can be revoked. To a user, it would be anticlimactic and we wouldn't see anything happen in reaction to that commit immediately

This is definitely still not a perfect case, because I think we could do a better job of talking about token scanning and tokens in general. Maybe some different verbage around the .gitignore? Or, if I've misunderstood how the alerts would be sent when a token is committed, maybe we should rethink this completely? I still worry about instructing a user to commit a dummy token, but we could have the bot do it, and have the user fix it.

"Finding Vulnerabilities" is just a lot of steps in the action phase. Is it too much? How could we break it up?

For adding a .gitignore, we have the bot create a branch. Should/could we do the same thing here, and have the bot create a branch, point out the part of the file that should be edited, and then post the instructions in the PR?

What if user's instruction was to check the dependencies, comment the proper version, and then the bot opened the the PR and the instructions were for the user to edit the file?

Currently, the bot will only approve the PR if the version is updated to a number, like "2.6.9", but other valid versioning sytnax doesn't pass, like "^2.6.9". This should be changed.

Valid syntax can include ^, >= , >, or ~ prefixes. (At least for this use case, based on stack overflow).

It doesn't look like there's an API endpoint we can access (yet) regarding vulnerability alerts, but we should at least be receiving information about them via the repository_vulnerability_alert event. However, even when creating new vulnerabilities, and fixing those vulnerabilities, I'm not seeing that event come through in smee. The only issue I can think of is enabling previews -- but maybe I'm missing something else?

@JasonEtco could you help me understand what's going on?

Do we want to use project site on GitHub Pages when the site is a game as opposed to a project site? I just through I was going to see something different based on the wording used.

Thanks to @jonico for pointing out that the link does not exist yet. We will need to do this as soon as the written instructions are finalized. 👍

cc @a-a-ron because I believe you've done this in the past

We don't add a .env file.

• After 1st event, issue didn't close, but the next issue did open.

✅ Wording

• First step can't actually validate. Should we put a note in the comment that says "Great work. Because of the available API endpoints, I can't check if you've completed the first few steps, but I can see that you've closed this issue to tell me you're ready to move on. Make sure to complete the first few steps in your settings before moving on." or something 7b1f313
• Update template README.md githubtraining/security-template@ecacba4
• Add note: you'll see some reminders about the dependency during the GitHub Flow, that's OK, you're fixing it.
• Comment in PR where dependency is updated "Move on to the next thing here now!" is unclear. Maybe "Let's learn about .gitignore files in the next pull request." 970ff99
• .gitignore PR still has messed up italicized note ba2d299
• Comment response in .gitignore PR "This looks great @brianamarie, thanks for adding a .env file to the .gitignore and renaming the .env file. Feel free to add any other file or potential sensitive data to the .gitignore file, and then proceed to merge this pull request." we need to remove the reference to renaming the .env file. Make merge this pull request bold or even better, call it out like an activity. 3ed2fc3 9727d7a
• "Great, you've done it @brianamarie!." has double punctuation 0096912
• Final comment "this documentation" should be more descriptive, "security GitHub Apps" needs reworded 4f64ac3
• Feel free to check out our "other courses?" final comment. I thought that was fixed? 5a6acc4

Flow

• We don't mention actually seeing the project anywhere. If we have them enable pages, we should point them to their project. c4791c2
• Issue "Find repository vulnerabilities" never gets closed c7a0382
• "Finding Vulnerabilities" is just a lot of steps in the action phase. Is it too much? How could we break it up? #34

Error in building-secure-repositories-on-github:enable-security-settings:respond[3]

HttpError: Empty value for parameter 'number': undefined
    at values.forEach (/app/node_modules/@octokit/rest/lib/plugins/endpoint-methods/validate.js:80:15)
    at Array.forEach (<anonymous>)
    at Object.keys.forEach.parameterName (/app/node_modules/@octokit/rest/lib/plugins/endpoint-methods/validate.js:54:12)
    at Array.forEach (<anonymous>)
    at validate (/app/node_modules/@octokit/rest/lib/plugins/endpoint-methods/validate.js:32:31)
    at <anonymous>

There have been changes made on master, but it's still not appearing in staging.

@githubtraining/learning-engineering What's the magic trigger for the new changes to appear?

Once https://github.com/github/learning-lab/issues/655 is closed, we should make the template repo private.

Test 1

• Installed in a private repository

Building Secure Repositories

• Activity: Enable Vulnerability alerts & GitHub Pages
  1. Enabled GitHub Pages without enabling Data services, nothing happened
  2. Enabled Allow GitHub to perform read-only analysis of this repository, nothing happened
  3. Enabled Dependency graph, nothing happened
  4. Enabled Vulnerability alerts, nothing happened
  5. Disabled and Enabled GitHub Pages, nothing happened

Test 2

• Installed in a public repository

Building Secure Repositories

• Activity: Enable Vulnerability alerts & GitHub Pages
  1. Enabled GitHub Pages , data services enabled by default (one option), nothing happened

@brianamarie , lets chat in the morning about testing, since I seem to be getting blocked on the first activity

In the PR where we edit the package.json file, if the user does something wrong (like removes the dependency instead of updating it) the bot should comment again letting them know that something doesn't look right, ex: they should make sure they kept the dependency and updated it to a valid version.

"Securing your workflows" is the title that will be used in the Keynote and in marketing collateral, so we should make sure it matches. 👍

Just a suggestion, feel free to close:

Can we not create the add-gitignore branch until it's relevant for the user? (Step 2 of the course I believe?)

Now people start the course with a big yellow banner that draws everyone's attention. In my it's-still-monday way of thinking I clicked on it and was slightly confused. Only after that I went to the issues tab to find out I had to follow those instructions. (Even though I followed learning lab courses before)

Make sure branch protection doesn't kick in too early, in case they accidentally close and need to try again

Great work y'all!

When I was reviewing the course I noticed some small copy/markdown issues.

courses? -> courses.

Italic _ symbol not closed

Security vulnerability -> Security vulnerabilities. (as we do in https://github.com/githubtraining/security/blob/476c7c66604442626861e977cc61f1375f698187/responses/01a_class-introduction-issue.md)

@ppremk has found a way for a react app to also use GitHub Pages 🎉 @ppremk Is this also the octocat memory game? It would be ideal if we could have the memory game also be the react app, so they have something fun when they deploy, either locally or on GH.

• Update flow so that LL comments with recommended version correctly, and then user opens a PR, then bot checks for that correct version.
• Update all responses
• Update config, get as finished as possible
• Update instructions to tell user how to deploy locally
• Make sure to address all open issues in this repo
• Finalise security-template repo

@ppremk What else do you think we need to add to this list as our current status, and the things we need to do before deploying to testing?

Test 1

• Installed in a private repository

Building Secure Repositories

• Activity: Enable Vulnerability alerts & GitHub Pages

  • Enabled GitHub Pages, bot responded with the next activity even though Data services weren't enabled

• Activity: Enable Vulnerability alerts & GitHub Pages - Attempt 2

  • Enabled Allow GitHub to perform read-only analysis of this repository, nothing happened
  • Enabled Dependency graph, nothing happened
  • Enabled Vulnerability alerts, nothing happened
  • Enabled GitHub Pages, bot responded with next activity and closed issue 👍

• Activity: Find repository vulnerabilities

  • Entered im a problem in the comment, bot responded with success and closed the issue

🚧 The Activity for this doesn't have a title 🚧

• Activity: Update the vulnerable dependency

  • Entered 2.6.8 (the vulnerable dependency #), bot responded with success

• Activity: Add a .gitignore file

  • Didn't enter .env on the first commit, got an error message 👍
  • Entered * .env on the second commit, got an approval message

Hi @githubtraining/trainers 👋

I've created a test repository that definitely has vulnerable dependencies listed. But, I haven't actually gotten any security alerts. I also can't see the dependency graph. I was looking in the wrong place

Do we know for sure that once we enable these features, they'll be functional even for existing package vulnerabilities, or will GitHub only send notifications for new vulnerabilities as they're found?

Edit: crosslinking in our Universe Learning Lab issue for higher visibility across GitHub

It would be good to talk about GitHub Marketplace, both the security apps and the dependency management apps. Let's add this to a response, maybe at the end?

"Once you open a pull request, I'll continue looking at your work there." should say something more standard, because bot will comment again in this issue.

This issue is to document the initial approach and ideas which we had experimented before deciding on the final structure of the security-template repo for the learning lab.

💡 Ideas

1️⃣The idea to have this course extensible with other features

• The idea is not implemented because the goal is to have learning lab courses to be stand alone implementations in order to highlight key features being taught

2️⃣@brianamarie and I had the idea of introducing a vulnerability in a projects' dependency via a user activity to highlight and educate the user of the new security feature.

• For this we decided to experiment by building a react application where introducing a dependency in the package.json file would trigger the security feature

3️⃣I created a template repo (security-template) using the create-react-app to experiment how far we could go with this approach. Here are some feedback from the attempt

• I tried to recreate the Octocat Memory Game as a react app by referencing the following link shared by @JasonEtco in slack
• To get this to work some extra work is needed to convert the existing js into react ready code
• For the users to have a working copy of the react app on their local machines, they would need to spin up the cloned repo with yarn start or npm start to see the final result of the game in their browser
• Stumbled upon a bug in the create-react-app if you have a mixed case in the path name. yarn start will throw an error cannot resolve path. More on it in this issue in fb\cra repo I do not know if this is fixed since there is some open threads after the issue is closed

🔔 Conclusion 🔔

Based on the progress made, it is not a good option to proceed with the idea of having a react app to finalise this security-template repo for the learning labs. Why? Because:

• There is no value add having an working react app which the users would not be deploying
• There is no option for us to keep track of terminal interactions of the users at this moment
• Depending on a third party app to spin up a working web app could lead to having unnecessary bugs that might have not been directly related to learning labs. We would end up fire fighting issues that are not directly related to us

☝️ the conclusion above are also values and points both @brianamarie and I share and agree upon. We also learned a good deal by keeping the feedback loop in small iterations and failing fast.

🎉 Next Steps 🎉

• Moving Forward, we plan to keep the security-template repo to be simple and easy to work with. Thus we have decided to use the github-games repo to achieve the goals of the security learning lab courses because it already has some gemlock vulnerabilities that sits well in our outline of responses flow
• @brianamarie and I will continue to sync up on the responses and template repo related tasks
• we will reach out to bounce ideas or if need help

🗣CC @hectorsector, @hollenberry, @a-a-ron, @beardofedu and @crichID

There have been changes in the template repository that aren't showing up when going through the course. Do we need to do something extra?

I had to enable the dependency graph before I saw any vulnerability alerts. Should we tell users to do the same?

From @hectorsector:

When asking them to update the package.json should we tell them that this is specific to a Node projects? It's been my experience that even advanced developers don't know as much about their non-primary languages.

• Set branch protection status while creating .gitignore PR so that users can't merge without bot validation

The template repository is working fine, but commits are currently done by @brianamarie instead of @githubteacher, and include commits with merge conflicts.

Currently, users get the same set of instructions even though their actions will be different based on if they have a private or public repository.

@githubtraining/learning-engineering Can we check programmatically to see if they have a public or private repository, and give a different response based on that information?

Text

• Something like "new to the GitHub flow? Try our introduction course. "
• "Taking advantage of the .gitignore file for security" has a note that's not properly italicized #28 (review)
• In opened PR for gitignore, "Return to this issue for my next comment" but it's a PR not an issue
• Renaming a file is going to be really hard from this screen, so maybe we should remove that step.

Function

• Link "Move on to the next thing here now" is broken #25
• After I commit the .env file to the .gitignore, there's no feedback at all. So I just merged? Nothing happened. Course appears stuck. #32
• I did close the issue in step 2, but it shows as unclosed "Enable security settings" on LL even when I am on step 5.

cc @JasonEtco

update package.json activity doesn't have a title