Skip to content

Commit

Permalink
Merge pull request #27 from feloy/feat-19/json-output-details
Browse files Browse the repository at this point in the history 
Returns more information (name, desc, status, severity)
  • Loading branch information
@feloy
feloy authored Nov 21, 2023
2 parents 04cfc71 + 94069fe commit 3052c3c
Show file tree
Hide file tree
Showing 7 changed files with 171 additions and 71 deletions.
14 changes: 5 additions & 9 deletions pkg/cli/analyze.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,17 @@ To find out more, run 'doa %s --help'
`, command, command)
}

func PrintPrettifyJsonOutput(errs []error) {
rowErrs := make([]string, len(errs))
for i, err := range errs {
rowErrs[i] = err.Error()
}
func PrintPrettifyJsonOutput(results []analyzer.Result) {
var bytes []byte
var err error
if bytes, err = json.MarshalIndent(rowErrs, "", " "); err != nil {
if bytes, err = json.MarshalIndent(results, "", " "); err != nil {
fmt.Println("error while converting output to json. Please try again without the output (--o) flag")
}
fmt.Println(string(bytes))
}

func PrintPrettifyOutput(errs []error) {
for i, sug := range errs {
fmt.Printf("%d - %s\n\n", i+1, sug)
func PrintPrettifyOutput(results []analyzer.Result) {
for i, sug := range results {
fmt.Printf("%d - %s (%s): %s\n\n", i+1, sug.Name, sug.Severity, sug.Description)
}
}
82 changes: 69 additions & 13 deletions pkg/command/analyzer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,45 @@ package command

import (
"fmt"
"github.com/redhat-developer/docker-openshift-analyzer/pkg/decompiler"
"os"
"path/filepath"
"strings"

"github.com/redhat-developer/docker-openshift-analyzer/pkg/decompiler"

"github.com/moby/buildkit/frontend/dockerfile/parser"
"github.com/redhat-developer/docker-openshift-analyzer/pkg/utils"
)

type ResultStatus string

const (
StatusFailed ResultStatus = "failed"
StatusPass ResultStatus = "success"
)

type ResultSeverity string

const (
SeverityCritical ResultSeverity = "critical"
SeverityHigh ResultSeverity = "high"
SeverityMedium ResultSeverity = "medium"
SeverityLow ResultSeverity = "low"
)

type Result struct {
Name string `json:"name"`
Status ResultStatus `json:"status"`
Severity ResultSeverity `json:"severity"`
Description string `json:"description"`
}

type Line struct {
Start int
End int
}
type Command interface {
Analyze(*parser.Node, utils.Source, Line) []error
Analyze(*parser.Node, utils.Source, Line) []Result
}

var commandHandlers = map[string]Command{
Expand All @@ -36,10 +60,17 @@ var commandHandlers = map[string]Command{
utils.USER_INSTRUCTION: User{},
}

func AnalyzePath(path string) []error {
func AnalyzePath(path string) []Result {
fileInfo, err := os.Stat(path)
if err != nil {
return []error{fmt.Errorf("unable to analyze %s - error %s", path, err)}
return []Result{
{
Name: "Analyze error",
Status: StatusFailed,
Severity: SeverityCritical,
Description: fmt.Sprintf("unable to analyze %s - error %s", path, err),
},
}
}

if fileInfo.IsDir() {
Expand All @@ -51,29 +82,48 @@ func AnalyzePath(path string) []error {

file, err := os.Open(path)
if err != nil {
return []error{fmt.Errorf("unable to open %s - error %s", path, err)}
return []Result{
{
Name: "File not found",
Status: StatusFailed,
Severity: SeverityCritical,
Description: fmt.Sprintf("unable to open %s - error %s", path, err),
},
}
}
defer file.Close()

return AnalyzeFile(file)
}

func AnalyzeImage(image string) []error {
func AnalyzeImage(image string) []Result {
node, err := decompiler.Decompile(image)
if err != nil {
return []error{fmt.Errorf("unable to analyze %s - error %s", image, err)}
return []Result{
{
Name: "Analyze error",
Status: StatusFailed,
Severity: SeverityCritical,
Description: fmt.Sprintf("unable to analyze %s - error %s", image, err),
},
}
}
return AnalyzeNodeFromSource(node, utils.Source{
Name: "",
Type: utils.Image,
})
}

func AnalyzeFile(file *os.File) []error {
func AnalyzeFile(file *os.File) []Result {
res, err := parser.Parse(file)
if err != nil {
return []error{
fmt.Errorf("unable to analyze the Containerfile. Error when parsing %s : %s", file.Name(), err.Error()),
return []Result{
{
Name: "Parse error",
Status: StatusFailed,
Severity: SeverityCritical,
Description: fmt.Sprintf("unable to analyze the Containerfile. Error when parsing %s : %s", file.Name(), err.Error()),
},
}
}

Expand All @@ -83,8 +133,8 @@ func AnalyzeFile(file *os.File) []error {
})
}

func AnalyzeNodeFromSource(node *parser.Node, source utils.Source) []error {
suggestions := []error{}
func AnalyzeNodeFromSource(node *parser.Node, source utils.Source) []Result {
suggestions := []Result{}
commands := []string{}
for _, child := range node.Children {
commands = append(commands, child.Original) // TODO to be used if we need to check previous rows to make sugestions
Expand All @@ -96,7 +146,13 @@ func AnalyzeNodeFromSource(node *parser.Node, source utils.Source) []error {
if handler != nil {
for n := child.Next; n != nil; n = n.Next {
if n.Value == "" {
suggestions = append(suggestions, fmt.Errorf("%s %s has an empty value", child.Value, GenerateErrorLocation(source, line)))
suggestions = append(suggestions, Result{
Name: "Wrong value",
Status: StatusFailed,
Severity: SeverityMedium,
Description: fmt.Sprintf("%s %s has an empty value", child.Value, GenerateErrorLocation(source, line)),
})

} else {
suggestions = append(suggestions, handler.Analyze(n, source, line)...)
}
Expand Down
20 changes: 15 additions & 5 deletions pkg/command/expose.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,29 @@ import (
type Expose struct {
}

func (e Expose) Analyze(node *parser.Node, source utils.Source, line Line) []error {
errs := []error{}
func (e Expose) Analyze(node *parser.Node, source utils.Source, line Line) []Result {
results := []Result{}
str := node.Value
index := strings.IndexByte(node.Value, '/')
if index >= 0 {
str = node.Value[0:index]
}
port, err := strconv.Atoi(str)
if err != nil {
errs = append(errs, err)
results = append(results, Result{
Name: "Wrong port value",
Status: StatusFailed,
Severity: SeverityCritical,
Description: err.Error(),
})
}
if port < 1024 {
errs = append(errs, fmt.Errorf(`port %d exposed %s could be wrong. TCP/IP port numbers below 1024 are privileged port numbers`, port, GenerateErrorLocation(source, line)))
results = append(results, Result{
Name: "Privileged port exposed",
Status: StatusFailed,
Severity: SeverityHigh,
Description: fmt.Sprintf(`port %d exposed %s could be wrong. TCP/IP port numbers below 1024 are privileged port numbers`, port, GenerateErrorLocation(source, line)),
})
}
return errs
return results
}
18 changes: 12 additions & 6 deletions pkg/command/from.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package command

import (
"fmt"

"github.com/moby/buildkit/frontend/dockerfile/parser"
"github.com/redhat-developer/docker-openshift-analyzer/pkg/decompiler"
"github.com/redhat-developer/docker-openshift-analyzer/pkg/utils"
Expand All @@ -22,23 +23,28 @@ type From struct {

const SCRATCH_IMAGE_NAME = "scratch"

func (f From) Analyze(node *parser.Node, source utils.Source, line Line) []error {
func (f From) Analyze(node *parser.Node, source utils.Source, line Line) []Result {
if node.Value == SCRATCH_IMAGE_NAME {
return nil
}
errs := []error{}
results := []Result{}
decompiledNode, err := decompiler.Decompile(node.Value)
if err != nil {
// unable to decompile base image
errs = append(errs, fmt.Errorf("unable to analyze the base image %s", node.Value))
return errs
results = append(results, Result{
Name: "Analyze error",
Status: StatusFailed,
Severity: SeverityLow,
Description: fmt.Sprintf("unable to analyze the base image %s", node.Value),
})
return results
}
errsFromBaseImage := AnalyzeNodeFromSource(decompiledNode, utils.Source{
Name: node.Value,
Type: utils.Parent,
})
if len(errsFromBaseImage) > 0 {
errs = append(errs, errsFromBaseImage...)
results = append(results, errsFromBaseImage...)
}
return errs
return results
}
65 changes: 46 additions & 19 deletions pkg/command/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,45 +21,50 @@ import (

type Run struct{}

func (r Run) Analyze(node *parser.Node, source utils.Source, line Line) []error {
errs := []error{}
func (r Run) Analyze(node *parser.Node, source utils.Source, line Line) []Result {
results := []Result{}

// let's split the run command by &&. E.g chmod 070 /app && chmod 070 /app/routes && chmod 070 /app/bin
splittedCommands := strings.Split(node.Value, "&&")
for _, command := range splittedCommands {
if r.isChmodCommand(command) {
err := r.analyzeChmodCommand(command, source, line)
if err != nil {
errs = append(errs, err)
results = append(results, *err)
}
} else if r.isChownCommand(command) {
err := r.analyzeChownCommand(command, source, line)
if err != nil {
errs = append(errs, err)
results = append(results, *err)
}
} else if r.isSudoOrSuCommand(command) {
err := r.analyzeSudoAndSuCommand(command, source, line)
if err != nil {
errs = append(errs, err)
results = append(results, *err)
}
}
}

return errs
return results
}

func (r Run) isSudoOrSuCommand(s string) bool {
return IsCommand(s, "sudo") || IsCommand(s, "su")
}

func (r Run) analyzeSudoAndSuCommand(s string, source utils.Source, line Line) error {
func (r Run) analyzeSudoAndSuCommand(s string, source utils.Source, line Line) *Result {
re := regexp.MustCompile(`(\s+|^)(sudo|su)\s+`)

match := re.FindStringSubmatch(s)
if len(match) > 0 {
return fmt.Errorf(`sudo/su command used in '%s' %s could cause an unexpected behavior.
return &Result{
Name: "Use of sudo/su command",
Status: StatusFailed,
Severity: SeverityMedium,
Description: fmt.Sprintf(`sudo/su command used in '%s' %s could cause an unexpected behavior.
In OpenShift, containers are run using arbitrarily assigned user ID and elevating privileges could lead
to an unexpected behavior`, s, GenerateErrorLocation(source, line))
to an unexpected behavior`, s, GenerateErrorLocation(source, line)),
}
}
return nil
}
Expand All @@ -68,7 +73,9 @@ func (r Run) isChownCommand(s string) bool {
return IsCommand(s, "chown")
}

/* to be tested on
/*
to be tested on
chown -R node:node /app
chown --recursive=node:node
chown +x test
Expand All @@ -77,7 +84,7 @@ chown -R 1000:1000 /app
chown 1001 /deployments/run-java.sh
chown -h 501:20 './AirRun Updates'
*/
func (r Run) analyzeChownCommand(s string, source utils.Source, line Line) error {
func (r Run) analyzeChownCommand(s string, source utils.Source, line Line) *Result {
re := regexp.MustCompile(`(\$*\w+)*:(\$*\w+)`)

match := re.FindStringSubmatch(s)
Expand All @@ -86,8 +93,13 @@ func (r Run) analyzeChownCommand(s string, source utils.Source, line Line) error
}
group := match[len(match)-1]
if strings.ToLower(group) != "root" && group != "0" {
return fmt.Errorf(`owner set on %s %s could cause an unexpected behavior.
In OpenShift the group ID must always be set to the root group (0)`, s, GenerateErrorLocation(source, line))
return &Result{
Name: "Owner set",
Status: StatusFailed,
Severity: SeverityMedium,
Description: fmt.Sprintf(`owner set on %s %s could cause an unexpected behavior.
In OpenShift the group ID must always be set to the root group (0)`, s, GenerateErrorLocation(source, line)),
}
}
return nil
}
Expand All @@ -96,28 +108,43 @@ func (r Run) isChmodCommand(s string) bool {
return IsCommand(s, "chmod")
}

func (r Run) analyzeChmodCommand(s string, source utils.Source, line Line) error {
func (r Run) analyzeChmodCommand(s string, source utils.Source, line Line) *Result {
re := regexp.MustCompile(`chmod\s+(\d+)\s+(.*)`)
match := re.FindStringSubmatch(s)
if len(match) == 0 {
return nil
}
if len(match) != 3 {
return fmt.Errorf("unable to fetch args of chmod command %s. Is it correct?", GenerateErrorLocation(source, line))
return &Result{
Name: "Syntax error",
Status: StatusFailed,
Severity: SeverityCritical,
Description: fmt.Sprintf("unable to fetch args of chmod command %s. Is it correct?", GenerateErrorLocation(source, line)),
}
}
permission := match[1]
if len(permission) != 3 {
return fmt.Errorf("unable to fetch args of chmod command %s. Is it correct?", GenerateErrorLocation(source, line))
return &Result{
Name: "Syntax error",
Status: StatusFailed,
Severity: SeverityCritical,
Description: fmt.Sprintf("unable to fetch args of chmod command %s. Is it correct?", GenerateErrorLocation(source, line)),
}
}
groupPermission := permission[1:2]
if groupPermission != "7" {
proposal := fmt.Sprintf("Is it an executable file? Try updating permissions to %s7%s", permission[0:1], permission[2:3])
if groupPermission != "6" {
proposal += fmt.Sprintf(" otherwise set it to %s6%s", permission[0:1], permission[2:3])
}
return fmt.Errorf("permission set on %s %s could cause an unexpected behavior. %s\n"+
"Explanation - in Openshift, directories and files need to be read/writable by the root group and "+
"files that must be executed should have group execute permissions", s, GenerateErrorLocation(source, line), proposal)
return &Result{
Name: "Permission set",
Status: StatusFailed,
Severity: SeverityMedium,
Description: fmt.Sprintf("permission set on %s %s could cause an unexpected behavior. %s\n"+
"Explanation - in Openshift, directories and files need to be read/writable by the root group and "+
"files that must be executed should have group execute permissions", s, GenerateErrorLocation(source, line), proposal),
}
}

return nil
Expand Down
Loading

0 comments on commit 3052c3c

Please sign in to comment.