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

Salus Orb #39

Closed
jborrey opened this issue Feb 23, 2019 · 19 comments

Comments

@jborrey
Copy link
Contributor

@jborrey jborrey commented Feb 23, 2019

From #10, @raphdev brought up the following:

A small improvement we've made is simplifying for CircleCI through an orb. This allows teams to integrate Salus and any configuration, simply by including the orb. The orb would have the correct docker image/version, and remote configuration locations managed by the security team. One benefit of this is that we no longer require updating several pipelines for updates that aren't in the container itself, such as handling of reporting, since these are reused from the orb.
Does something like this sound like a valuable contribution?

Thanks for bringing this to my attention! I need to keep up with the CircleCI blog more.

Yeah I certainly think that a Salus orb could be useful. Not sure if the Coinbase team wants to own the creation of that or @raphdev, are you considering making your orb general purpose to work outside of your org?

@raphdev

This comment has been minimized.

Copy link
Contributor

@raphdev raphdev commented Feb 25, 2019

My intention is to make the orb general purpose. Since production CircleCI orbs are public, it makes sense to make our Salus orb useful outside our org. We'd be more than happy to contribute this!

@jborrey

This comment has been minimized.

Copy link
Contributor Author

@jborrey jborrey commented Feb 25, 2019

Sounds great! Thank you.

Orb source code seems public so if other orgs want to have their own version or modify it that should be easy for them to do.

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented Apr 20, 2019

Is this orb publicly available? Not seeing it in the registry at https://circleci.com/orbs/registry/

Thanks!

@nishils

This comment has been minimized.

Copy link
Collaborator

@nishils nishils commented Apr 20, 2019

As far as I know, not yet. If you know how to build an orb, we could use help on that front. @raphdev are you still able to make the Salus orb public?

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented Apr 29, 2019

Never have before, but gave it a shot. It's currently published as a development version.

federacy/salus@dev:0.0.1

https://github.com/federacy/salus-orb

@raphdev

This comment has been minimized.

Copy link
Contributor

@raphdev raphdev commented Apr 30, 2019

We've been running a dev version in my org, but it's not public just yet. I can work on a prod release this week since I'm continuing Salus-related work for the next few weeks.

@jsulinski Nice job! I like how you parameterized some of the Salus local config options. The orb I have doesn't expose these options, only command line flags.

Given there are two orbs in development, do we want to coordinate efforts? If so, how do we proceed?

@jborrey

This comment has been minimized.

Copy link
Contributor Author

@jborrey jborrey commented Apr 30, 2019

Yeah nice job @jsulinski - and I appreciate the fact that you kept the default params quite strong (the README mislead me, but in the orb.yml is see it's different).

@raphdev I see pros and cons for both options. If we have two implementations and they are significantly different (not sure how different orbs can be ...) then it's probably worth having both. If you think they will be very similar, I'd lean towards collaboration provided that @jsulinski is happy to facilitate code reviews/merging.

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented Apr 30, 2019

Thanks! I'm happy to facilitate code reviews/merging. Will update the docs with the defaults and other examples soon, just wanted to make sure I'm on the right path.

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented Apr 30, 2019

@raphdev We should collaborate as I doubt there will be significant disparity in functionality and it's MIT licensed by nature of being an Orb.

By the way, I think even dev Orbs are public, just not published to the visible registry. I tested using the one at federacy/salus@dev:0.0.1 from a different org and it was accessible.

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented May 4, 2019

@raphdev Any update here? I've added some additional documentation and am otherwise ready to publish the orb.

@raphdev

This comment has been minimized.

Copy link
Contributor

@raphdev raphdev commented May 5, 2019

@jsulinski You are right, dev orbs are also world-readable. I was under the impression you needed to be in the org.

Feel free to try wework/salus@dev:salus-testing. Since the code for the orb is still private, you can process a config to resolve the orb:

version: 2.1
orbs:
  salus: wework/salus@dev:salus-testing
workflows:
  security_scan:
    jobs:
      - salus/scan

Then run circleci config process config.yml > expanded_config.yml and you should see the salus/scan job resolved in expanded_config.yml. You can test running it locally by copying to a repo and running circleci build --job salus/scan -c expanded_config.yml.

The orb is similar in many ways, some noteworthy differences:

  • I didn't paremeterize the config options. Instead we expose config that maps to the --config command line option, and use the local/remote configurations to control it. This may be valuable to control multiple repos using the orb:
salus/scan:
    - config: "http://myconfig.internal/salus.yml"
  • This orb maintains CircleCI default directories (e.g., /home/repo) for its working directory. I don't think this matters too much, unless we want compatibility with Circle convenience images for any reason. I found it convenient for overriding the least path settings. You can change this through a source_dir option, which maps to --repo_path.
  • The job executor is configurable. I thought this was important for testing custom Salus builds, and I expect people may want to be able to define and pass their own images to run Salus without needing a new orb:
version: 2.1
orbs:
  salus: wework/salus@dev:salus-testing
executors:
  salus_latest:
    docker:
      - image: coinbase/salus:latest
workflows:
  security_scan:
    jobs:
      - salus/scan
          salus_container:
            name: salus_latest

This also works with private images.

I think the configuration options are worthwhile to consolidate to support a variety of scenarios. Likewise, the executor is helpful for custom, experimental, or otherwise company specific Salus scanners. Thoughts?

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented May 6, 2019

Awesome, thanks! I agree with you and I'll work on merging them.

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented May 9, 2019

@raphdev I tested and merged in the executor branch.

I also implemented your other suggestions, let me know if you agree with the approach: https://github.com/federacy/salus-orb/tree/raph-suggestions.

These changes are currently available here: federacy/salus@dev:0.0.3.

Some challenges I ran into:

  • There are relative paths in report_node_modules and report_python_modules, ie: https://github.com/coinbase/salus/blob/master/lib/salus/scanners/report_node_modules.rb#L47 which required setting the working_directory parameter.
  • --config is nested below --repo-path, so I couldn't use full paths
  • You can override --configuration with an environment variable, but then it is rather ambiguous to the user which is being used. Worked around this with a new parameter and some shell logic, which clearly tells the user if the configuration file they specified is not present. The environment variable can still be used to utilize a configuration file over HTTPS.

If you agree with the approach, I'll go ahead and publish a non-dev version of the Orb.

@raphdev

This comment has been minimized.

Copy link
Contributor

@raphdev raphdev commented May 12, 2019

Awesome! I went ahead and PR'd the changes so I could comment there.

There are relative paths in report_node_modules and report_python_modules, ie: https://github.com/coinbase/salus/blob/master/lib/salus/scanners/report_node_modules.rb#L47 which required setting the working_directory parameter.

I confirmed this breaks for me too. Does your comment about --config and --repo-path affect these utils as well, preventing us from using full paths for those?

I agree with the approach - the workarounds are well thought out.

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented May 13, 2019

Correct, in addition to report_uri. For example:

https://github.com/coinbase/salus/blob/master/lib/salus/processor.rb#L92-L94
https://github.com/coinbase/salus/blob/master/lib/salus/processor.rb#L39

I fixed a few typos and pegged the tag to 2.4.2. Also tested a handful of different configurations. I'll merge the PR and Publish soon unless you have any objections!

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented May 13, 2019

The reason for the relative paths is to put any generated configuration and the default report outside of the repo directory so that we don't taint it.

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented May 19, 2019

Orb has been published. Thanks so much for the help @raphdev!

https://circleci.com/orbs/registry/orb/federacy/salus

@jsulinski

This comment has been minimized.

Copy link
Contributor

@jsulinski jsulinski commented May 23, 2019

Added a usage example here: #54

@jborrey

This comment has been minimized.

Copy link
Contributor Author

@jborrey jborrey commented May 23, 2019

Awesome work y'all. Thanks again for helping grow this project.

@jborrey jborrey closed this May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.