Skip to content

Commit

Permalink
sobelow:chore - improve tests and code cleaning (#898)
Browse files Browse the repository at this point in the history 
This commit add some new asserts on successful parsing Sobelow
results, to verify that all fields of Vulnerability was filled.

Some code organization was also made, and the entities packages was
removed and the Sobelow schema output was moved to sobelow package.

Updates #718

Signed-off-by: Matheus Alcantara <[email protected]>
  • Loading branch information
@matheusalcantarazup
matheusalcantarazup authored Dec 27, 2021
1 parent c597528 commit 657531d
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 80 deletions.
59 changes: 28 additions & 31 deletions internal/services/formatters/elixir/sobelow/formatter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,32 @@ import (
"github.com/ZupIT/horusec-devkit/pkg/enums/tools"
"github.com/ZupIT/horusec-devkit/pkg/utils/logger"

dockerEntities "github.com/ZupIT/horusec/internal/entities/docker"
"github.com/ZupIT/horusec/internal/entities/docker"
"github.com/ZupIT/horusec/internal/enums/images"
"github.com/ZupIT/horusec/internal/helpers/messages"
"github.com/ZupIT/horusec/internal/services/formatters"
"github.com/ZupIT/horusec/internal/services/formatters/elixir/sobelow/entities"
vulnhash "github.com/ZupIT/horusec/internal/utils/vuln_hash"
)

const (
replaceDefaultMessage = "Checking Sobelow version..."

// nolint: lll
notAPhoenixApplication = "project not appear to be a Phoenix application. If this is an Umbrella application, each application should be scanned separately"
)

var ErrorNotAPhoenixApplication = errors.New(notAPhoenixApplication)

type Formatter struct {
formatters.IService
}

func NewFormatter(service formatters.IService) formatters.IFormatter {
func NewFormatter(service formatters.IService) *Formatter {
return &Formatter{
service,
}
}

const NotAPhoenixApplication = "this does not appear to be a Phoenix application. if this is an Umbrella application," +
" each application should be scanned separately"

var ErrorNotAPhoenixApplication = errors.New(NotAPhoenixApplication)

func (f *Formatter) StartAnalysis(projectSubPath string) {
if f.ToolIsToIgnore(tools.Sobelow) || f.IsDockerDisabled() {
logger.LogDebugWithLevel(messages.MsgDebugToolIgnored + tools.Sobelow.ToString())
Expand All @@ -66,16 +69,16 @@ func (f *Formatter) startSobelow(projectSubPath string) (string, error) {
return output, err
}

if strings.Contains(output, NotAPhoenixApplication) {
if strings.Contains(output, notAPhoenixApplication) {
return output, ErrorNotAPhoenixApplication
}

f.parseOutput(output, projectSubPath)
return output, nil
}

func (f *Formatter) getConfigData(projectSubPath string) *dockerEntities.AnalysisData {
analysisData := &dockerEntities.AnalysisData{
func (f *Formatter) getConfigData(projectSubPath string) *docker.AnalysisData {
analysisData := &docker.AnalysisData{
CMD: f.GetConfigCMDByFileExtension(projectSubPath, CMD, "mix.lock", tools.Sobelow),
Language: languages.Elixir,
}
Expand All @@ -84,21 +87,20 @@ func (f *Formatter) getConfigData(projectSubPath string) *dockerEntities.Analysi
}

func (f *Formatter) parseOutput(output, projectSubPath string) {
const replaceDefaultMessage = "Checking Sobelow version..."
output = strings.ReplaceAll(strings.ReplaceAll(output, replaceDefaultMessage, ""), "\r", "")

for _, value := range strings.Split(output, "\n") {
if value == "" {
continue
}

if data := f.setOutputData(value); data != nil {
f.AddNewVulnerabilityIntoAnalysis(f.setVulnerabilityData(data, projectSubPath))
if data := f.newOutput(value); data != nil {
f.AddNewVulnerabilityIntoAnalysis(f.newVulnerability(data, projectSubPath))
}
}
}

func (f *Formatter) setOutputData(output string) *entities.Output {
func (f *Formatter) newOutput(output string) *sobelowOutput {
indexFirstColon := strings.Index(output, ":")
indexLastColon := strings.LastIndex(output, ":")
indexTrace := strings.LastIndex(output, "-")
Expand All @@ -107,26 +109,21 @@ func (f *Formatter) setOutputData(output string) *entities.Output {
return nil
}

return &entities.Output{
return &sobelowOutput{
Title: strings.TrimSpace(output[indexFirstColon+1 : indexTrace]),
File: strings.TrimSpace(output[indexTrace+1 : indexLastColon]),
Line: strings.TrimSpace(output[indexLastColon+1:]),
}
}

func (f *Formatter) setVulnerabilityData(output *entities.Output, projectSubPath string) *vulnerability.Vulnerability {
vuln := f.getDefaultVulnerabilitySeverity()
vuln.Details = output.Title
vuln.File = f.GetFilepathFromFilename(output.File, projectSubPath)
vuln.Line = output.Line
vuln = vulnhash.Bind(vuln)
return f.SetCommitAuthor(vuln)
}

func (f *Formatter) getDefaultVulnerabilitySeverity() *vulnerability.Vulnerability {
vulnerabilitySeverity := &vulnerability.Vulnerability{}
vulnerabilitySeverity.SecurityTool = tools.Sobelow
vulnerabilitySeverity.Language = languages.Elixir
vulnerabilitySeverity.Severity = severities.Unknown
return vulnerabilitySeverity
func (f *Formatter) newVulnerability(output *sobelowOutput, projectSubPath string) *vulnerability.Vulnerability {
vuln := &vulnerability.Vulnerability{
SecurityTool: tools.Sobelow,
Language: languages.Elixir,
Severity: severities.Unknown,
Details: output.Title,
File: f.GetFilepathFromFilename(output.File, projectSubPath),
Line: output.Line,
}
return f.SetCommitAuthor(vulnhash.Bind(vuln))
}
95 changes: 48 additions & 47 deletions internal/services/formatters/elixir/sobelow/formatter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,91 +16,79 @@ package sobelow

import (
"errors"
"path/filepath"
"testing"

"github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
"github.com/ZupIT/horusec-devkit/pkg/enums/languages"
"github.com/ZupIT/horusec-devkit/pkg/enums/tools"
"github.com/stretchr/testify/assert"

cliConfig "github.com/ZupIT/horusec/config"
"github.com/ZupIT/horusec/config"
"github.com/ZupIT/horusec/internal/entities/toolsconfig"
"github.com/ZupIT/horusec/internal/entities/workdir"
"github.com/ZupIT/horusec/internal/services/formatters"
"github.com/ZupIT/horusec/internal/utils/testutil"
)

func getOutputString() string {
return `
[+] Config.CSP: Missing Content-Security-Policy - lib/built_with_elixir_web/router.ex:9
[+] Config.Secrets: Hardcoded Secret - config/travis.exs:24
[+] Config.HTTPS: HTTPS Not Enabled - config/prod.exs:0
[+] XSS.Raw: XSS - lib/built_with_elixir_web/templates/layout/app.html.eex:17
test
`
}

func TestStartCFlawfinder(t *testing.T) {
t.Run("should success execute container and process output", func(t *testing.T) {
func TestSobelowStartAnalysis(t *testing.T) {
t.Run("should add 4 vulnerabilities on analysis with no errors", func(t *testing.T) {
dockerAPIControllerMock := testutil.NewDockerMock()
entity := &analysis.Analysis{}
config := &cliConfig.Config{}
config.WorkDir = &workdir.WorkDir{}

output := getOutputString()

dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return(output, nil)

service := formatters.NewFormatterService(entity, dockerAPIControllerMock, config)
entity := new(analysis.Analysis)

service := formatters.NewFormatterService(entity, dockerAPIControllerMock, newTestConfig(t, entity))
formatter := NewFormatter(service)

formatter.StartAnalysis("")

assert.NotEmpty(t, entity)
assert.Len(t, entity.AnalysisVulnerabilities, 4)

for _, v := range entity.AnalysisVulnerabilities {
vuln := v.Vulnerability

assert.Equal(t, tools.Sobelow, vuln.SecurityTool)
assert.Equal(t, languages.Elixir, vuln.Language)
assert.NotEmpty(t, vuln.Details, "Expected not empty details")
assert.NotEmpty(t, vuln.File, "Expected not empty file name")
assert.NotEmpty(t, vuln.Line, "Expected not empty line")
assert.NotEmpty(t, vuln.Severity, "Expected not empty severity")

}
})

t.Run("should return error when invalid output", func(t *testing.T) {
t.Run("should not add error on analysis when empty output", func(t *testing.T) {
dockerAPIControllerMock := testutil.NewDockerMock()
analysis := &analysis.Analysis{}
config := &cliConfig.Config{}
config.WorkDir = &workdir.WorkDir{}
dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return("", nil)

output := ""
analysis := new(analysis.Analysis)

dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return(output, nil)

service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config)
service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, newTestConfig(t, analysis))
formatter := NewFormatter(service)
formatter.StartAnalysis("")

assert.NotPanics(t, func() {
formatter.StartAnalysis("")
})
assert.False(t, analysis.HasErrors(), "Expected no errors on analysis")
})

t.Run("should return error when executing container", func(t *testing.T) {
dockerAPIControllerMock := testutil.NewDockerMock()
entity := &analysis.Analysis{}
config := &cliConfig.Config{}
config.WorkDir = &workdir.WorkDir{}

dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return("", errors.New("test"))

service := formatters.NewFormatterService(entity, dockerAPIControllerMock, config)
entity := new(analysis.Analysis)

service := formatters.NewFormatterService(entity, dockerAPIControllerMock, newTestConfig(t, entity))
formatter := NewFormatter(service)
formatter.StartAnalysis("")

assert.NotPanics(t, func() {
formatter.StartAnalysis("")
})
assert.True(t, entity.HasErrors(), "Expected errors on analysis")
})

t.Run("should not execute tool because it's ignored", func(t *testing.T) {
entity := &analysis.Analysis{}
entity := new(analysis.Analysis)

dockerAPIControllerMock := testutil.NewDockerMock()
config := &cliConfig.Config{}
config.WorkDir = &workdir.WorkDir{}

config := config.New()
config.ToolsConfig = toolsconfig.ToolsConfig{
tools.Sobelow: toolsconfig.Config{
IsToIgnore: true,
Expand All @@ -113,3 +101,16 @@ func TestStartCFlawfinder(t *testing.T) {
formatter.StartAnalysis("")
})
}

func newTestConfig(t *testing.T, analysiss *analysis.Analysis) *config.Config {
cfg := config.New()
cfg.ProjectPath = testutil.CreateHorusecAnalysisDirectory(t, analysiss, testutil.ElixirExample)
return cfg
}

var output = `
[+] Config.CSP: Missing Content-Security-Policy - ` + filepath.Join("lib", "built_with_elixir_web", "router") + `.ex:9
[+] Config.Secrets: Hardcoded Secret - ` + filepath.Join("config", "prod") + `.exs:1
[+] Config.HTTPS: HTTPS Not Enabled - ` + filepath.Join("config", "dev") + `.exs:1
[+] XSS.Raw: XSS - ` + filepath.Join("lib", "built_with_elixir_web", "templates", "layout", "app.html") + `.eex:17
`
4 changes: 2 additions & 2 deletions ...matters/elixir/sobelow/entities/output.go → ...vices/formatters/elixir/sobelow/output.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package entities
package sobelow

type Output struct {
type sobelowOutput struct {
Title string
File string
Line string
Expand Down

0 comments on commit 657531d

Please sign in to comment.