Consensys / truffle-security

MythX smart contract security verification plugin for Truffle Framework
https://mythx.io
124 stars 28 forks source link

Add CI option #106

Closed thec00n closed 5 years ago

thec00n commented 5 years ago

It be great to be able run truffle-security in a Continuous Integration setup such as CircleCI. A user could run a full analysis as a separate CI step, e.g.

truffle run verify --ci --fail-on=High

In the above example truffle-security runs the full analysis and checks if there are any High severity findings present in the analysis report and if so exits with 1 and prints the issues that failed the step to the console. truffle-security should also allow a user to set the --fail-on with the severity ratings Medium and Low.

rocky commented 5 years ago

It [would] be great to be able run truffle-security in a Continuous Integration setup such as CircleCI

Yes, it would.

Having one or many MythX-enabled CIs has long been in the plan. We may even have gitcoin bounties for this as part of the upcoming Ethereal Summit. That would seem to encompass this.

But any MythX-enabled CI really isn't about truffle per se. it would be handle any Solidity file and collection grouping that the file may be a part of.

If the thing CI is working on happens to be in truffle and we can capture that, all the better. But what if, instead, the thing CI is given is part of a VSCode project which provides a different way to organize solidity files? Would we add the same kind of options adapted from the vscode-solidity project there as well?

Or MythX-enabled CI could be in a mercurial or git project (of varieties github, gitlab, bitbucket) and that is neither a VSCode nor a truffle project.

It so happens that whichever kind of format the file or project is in, this CI thing (especially if I am involved in) will likely be based off of this code which uses truffle libraries . We use truffle libraries more because they do a bit of what is needed to be done, than because truffle was intended to be used by the smart contract developer.

So the plan is then to extract what is here, extend as necessary, and put this in an independent library - or as some folks prefer to call it: an SDK.

But sure, until such a project and library is written, we can keep this open here until we have a better place to note this in a public fashion.

And I guess it serves to remind: if this is triggered from a git-commit hook which is tracking file changes, the CI solution might consider whether this is part a truffle project and adjust accordingliy.

As for the suggested design: I dunno. I think we need to take a step back and see what all we want to do, and from that how best to do it. Of course, this might be one possibility.

tagomaru commented 5 years ago

truffle run verify --ci --fail-on=High

My idea to implement it quickly is like below.

  1. --fail-on option should be independent from CI. Maybe the value is either warning or error now.
  2. Output some files (ex. json) regarding 'Internal MythX errors encountered:' and problems (means security issues). CI can check them after running truffle-security to find some error like timeout and security issues.

Do you think it makes sense?

rocky commented 5 years ago

For CI kinds of things it makes more sense to have things directed through a configuration file similar to eslint's .eslintrc, or a circleci.yaml that controls how CI works and what kinds of warnings to give or terminate early on. [Issue #116 was created to address this aspect.]

With command-line options we have to live within what truffle provides and that is limiting in of itself.

And perhaps this configuration then can be used whether or not CI was used. For example the configuration may be used to indicate that you don't want to hear about "floating pragmas" as well as these threshhold kinds of things.

More generally speaking we need to have a broader discussion of how to do CI in general. In this specific situation, I am not in favor of let's make this hack here and see where it gets us.

Here is a little analogy that I offer for amusement. If you are pressed for time or don't want to read a story, you can skip the rest of and just go with the above.

In sailing there are two kinds of navigation, one with a chart (map) and navigation tools and one called "line of site". Charts indicate things like where sandbars are and where the channel is and how strong the current is,. This information assists you in figuring out how to get from here to there. Generally you can't go not the straight path, but have to vary a little bit. And based on the wind, you may tack back and forth. So you are not going the most direct route, but that's okay because you have an overall plan with information to assist you.

In line-of-site navigation, it may seem similar because you are also not going directly, but it is different. Because you don't have these important navigation details and facts you just say: hey there's another island over there and that seems closer so let's get to that. And so on. In this approach you may find out eventually that there is a sand bar between where you wind up and where you need to get to. Or maybe there a strong current and you are on the wrong direction of that, but if you had gone a different longer route maybe you'd be on the right side of the current.

What we are doing here feels like line-of-site navigation. What I'd prefer is having a map which lays out all of the issues.

tagomaru commented 5 years ago

@rocky Thx for your comment. ok i understand your thoughts. Keep discussing about how we can improve with CI.

rocky commented 5 years ago

This is covered by #35 and #118