# How to contribute

This guide is meant to provide guidelines for how to contribute to the Norad docker-images project.
These procedures should work in the majority of cases.  At times, it will be necessary to step
outside of these guidelines and Do What Makes Sense. This is a living document that we can continue
to modify as we define the process that helps us add code quickly and safely.

## Getting Started

* If you haven't already, you'll likely want to set up a [development
  box](https://gitlab.com/norad/dev-box).  This provides a consistent environment for
  building assessments.

* Due to the variance in types of assessments that will be written, we do not enforce any sort of
  style or code quality.  However, if the code you are writing is written in Ruby, you may wish to
  use [rubocop](https://github.com/bbatsov/rubocop).  You can familiarize yourself with the
  [style guide](https://github.com/bbatsov/ruby-style-guide).  You can also install a tool into your
  editor which will help you follow the style guideline. One for vim can be found
  [here](https://github.com/scrooloose/syntastic).

* You may wish to use the scaffold generator provided in this repository. For more information,
  please reference [the README](https://gitlab.com/norad/security-modules#scaffolding).

## Making Changes

Fundamentally, we follow the Github Flow(1) methodology for branching and committing code. The
process essentially breaks down as follows:

1. Assign the feature you are beginning to work on to yourself in JIRA.
2. Create a feature branch off of the master branch. Note that anything in master is always
   deployable, so this branch will always have the latest code. Branch names should be descriptive.
3. Your branch is your branch. Feel free to experiment and commit freely. You may wish to make your
   life easier when it's time to merge into master by learning about [auto-squashing
   commits](https://robots.thoughtbot.com/autosquashing-git-commits).
4. You are free to push the feature branch to the origin as freqently as you wish. This will run the
   CI suite for the following stages: **validate**, **build**, and **test**.  You can also run the
   test suite locally on your dev box (refer to the .gitlab-ci.yml file and the scripts in the
   ```./ci``` directory).  By pushing to the feature branch, you can engage other engineers on the
   team if you need help. They can checkout your branch and examine the code.
5. When your feature is done, open a Merge Request in Gitlab.

At this point in process, there is no requirement of any kind for your commit messages. Before
merging into master, though, you will need a "good commit message" for any commits that aren't being
squashed. Links to further reading on this topic are available in the Additional Resources section.

## Creating a Manifest File

In addition to creating the Dockerfile and wrapper scripts for your image, you
will need to create a manifest.yml file in the directory for your new tool.
This manifest file is used by the CI scripts to create a Merge Request against
the Norad [API](https://gitlab.com/norad/api) project to add the
new assessment to the API database. A sample manifest file is included below:

```yaml
registry: norad-registry.cisco.com:5000
name: arachni-xss
version: 0.0.1
prog_args: '--checks=%{check} %{protocol}://%{target}:%{port}%{url}'
default_config:
  protocol: "https"
  port: '443'
  url: '/'
  check: "xss*"
category: blackbox
```

## Writing Tests

A testing harness is provided for verifying basic functionality of the
assessments included in this repository. In general, the following steps
outline the test-creation process:

1. Create a directory in ```spec/assessments``` by the same name of the image you want to test, e.g. ```spec/assessments/nmap-ssl-dh-param/```
2. Create an rspec file, also named the same as the image under test, in this directory e.g. ```nmap-ssl-dh-param_spec.rb```
3. In this directory, create a ```targets/``` directory
4. Add a ```Dockerfile.secure``` and ```Dockerfile.vulnerable``` to the ```targets/``` directory

The CI scripts will handle building the secure and vulnerable test targets and execute the
assessment against each. It will make the results available to your Rspec script via instance
variables. If your assessment requires authentication on the target, the test harness will determine
this automatically based on the default arguments for the assessment.

### Sample Rspec Script
```ruby
require_relative '../../spec_helper.rb'

class NmapSslDhParam
  extend AssessmentHelpers
end

describe NmapSslDhParam, scan_assessment: true do
  context 'for all targets' do
    it 'should report a single result' do
      expect(@vulnerable_results.size).to eq(1)
      expect(@secure_results.size).to eq(1)
      expect(@base_results.size).to eq(1)
    end
  end
end
```

## Writing a README

The README.md file included with each assessment is used by the CI suite to update the Norad
[docs](https://gitlab.com/norad/docs) application with information about the assessment
it is associated with. The contents of this README are largely up to the author, but should include
some general information such as:

1. A description of what the assessment tests for
2. Configuration options table explaining how to configure the asssessment
3. Whether or not the assessment requires authentication on the target (i.e. SSH)
4. What requirements, if any, are addressed by the assessment

## Submitting Changes

When your feature is ready, or is at a state where you'd like to formally begin engaging the rest of
the team, create a [Merge
Request](https://gitlab.com/norad/security-modules/merge_requests) in Gitlab. Please write
a meaningful title and detailed description that provides an overview of What your feature does and
Why it does it. You can leave it to the code itself to explain the How, but feel free to call
anything out here that you would like the reviewer(s) to pay special attention to.  If you feel that
someone in particular needs to look at your Merge Request, you may assign the MR to them. Mentioning
someone in the Merge Request description is another way to alert someone in particular you'd like
for them to look at your MR, e.g.  ```cc @ravangar```.

## Code Review Process

The Code Review Process is meant to be an open-ended discussion about the feature that is being
committed. It should address whether the changeset meets the requirements of the feature as well as
the quality of the code itself. The reviewer(s) should also make sure that the feature includes an
adequate number of meaningful tests.

Please be polite in both giving and receiving feedback. Remember that it's a review of the code, not
an individual. As a reviewer, do your best to separate your personal opinion of how you would do
something from what is objectively constructive criticism. Please review the Thoughtbot guide for
Code Reviews in the Additional Resources section (3).

Make any changes to your feature branch that you and the reviewer agree should be made, and push
those changes to your feature branch. This will automatically update your Merge Request. Repeat this
process until the Merge Request is approved.

The merge request must receive at least one **+1** before it can be merged. If individuals are
called out to look at something during any point of the review process, they will need to provide a
**+1** as well before it can be merged.

## Testing and Integration Process

We leverage [Gitlab CI](https://about.gitlab.com/gitlab-ci/) to execute our test suite and integrate
the merged assessment content into our documentation and API applications as well as push the newly
created image to our Registry.  The integration is broken down into the following 5 stages:

1. validate (ensure correctness of the manifest file and the existence of a README)
2. build (attempt to build all of the Docker images in the repository)
3. test (execute the test suite for the built Docker images)
4. deploy (push the Docker images to the Norad Registry)*
5. integrate (open pull requests in the [api](https://gitlab.com/norad/api) and
   [docs](https://gitlab.com/norad/docs) using the manifest files and the READMEs,
   respectively)*

*\* only execute on the ```master``` branch*

More details can be found in the
[.gitlab-ci.yml](https://gitlab.com/norad/security-modules/blob/master/.gitlab-ci.yml)
file.

## Merge Process

Your feature got approved! Great! Now it's time to merge into master.

1. Before code can be merged to master, **all tests must be passing**. In other words, you need a
   green check from Gitlab CI.
2. In order to keep the master branch's commit history clean, you may be required to rebase your
   feature branch before it can be merged. You may also consider squashing interim commits into one
   or more block commits. The goal here is to keep the master branch devoid of commits such as
   "fixing typo" or "trying out this method." Either way, you should rebase your commits on top of
   the most recent commit to master.  This helps keep the git history organized and commits related
   to the same feature will be grouped to gether.  To rebase, run the following on your freature
   branch: ```git fetch orgin master && git rebase -i origin/master```.  See the additional
   resources for more information on rebasing.
3. After (force) pushing your rebased feature branch, the Merge Request can be merged **as long as
   all tests still pass**.

# Additional Resources

1. [Github Flow](https://guides.github.com/introduction/flow/)
2. [Code Review Guide](https://github.com/thoughtbot/guides/tree/master/code-review)
3. [Style Guide](https://github.com/bbatsov/ruby-style-guide)
4. [Git Tips](https://github.com/thoughtbot/guides/blob/master/protocol/git)
5. [Git Rebase](https://help.github.com/articles/about-git-rebase/)
6. [Auto-squashing Git Commits](https://robots.thoughtbot.com/autosquashing-git-commits)
7. [Good Commit Messages](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html)
8. [More Good Commit Messages](http://chris.beams.io/posts/git-commit/)
9. [Even More Good Commit Messages](http://www.alexkras.com/19-git-tips-for-everyday-use/#good-commit-message)