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

New output #1320

Merged
merged 67 commits into from Aug 3, 2023
Merged

New output #1320

merged 67 commits into from Aug 3, 2023

Conversation

Daniel-GrunbergerCA
Copy link
Collaborator

@Daniel-GrunbergerCA Daniel-GrunbergerCA commented Aug 2, 2023

Overview

  1. Add scan image output: pretty printer, json and sarif
  2. Add scan workload command and output
  3. Add security view for scan command
  4. Support image scanning for all scans

Additional Information

  1. Pretty printer now shows image scanning results by showing a table with the CVEs, and a scan summary. It suports a verbose flag for showing detailed view.
  2. Workload scanning will scan the workload against a specific set of controls, and display them in a specific order. The images belonging to that workload will be scanned as well, and the image scan summary will be displayed as well
  3. The scan command will be showing a security view when combined with the --view=security flag. This view will show a specific set of controls, and how to continue investigating the resources / control documentation. It will also show the compliance score for the NSA and MITRE frameworks. On top of that, the top 5 most dangerous workloads will be shown in a new section
  4. To scan a workload you can run kubescape scan workload <kind>/<name> --namespace=<namespace>. There are two available flags: --file-path for scanning using a file, and --chart-path for scanning using a helm-chart. The second flag must be combined with the first.
  5. The scan image and scan workload command will be showing security view.
  6. New --scan-images flag to scanning all images on a cluster/repository/file/helm-chart
  7. Add follow-up steps for scans

TODO:

  1. Support different output formats for when running a configuration scanning with image scanning
  2. Support different output formats for image scanning
  3. Documentation for the most vulnerable workloads
  4. Show spinner on image scanning

How to Test

As of the time of opening this PR, you will need to add --env=dev every time you use --view=security flag

Examples/Screenshots

  1. kubescape scan --view=security --env=dev
    image
    image
  2. kubescape scan workload Deployment/argocd-redis --namespace argocd --env=dev
    image
    image
  3. kubescape scan image redis:7.0.11-alpine
    image
  4. kubescape scan image redis:7.0.11-alpine -v
    image
  5. kubescape scan /Users/danielgrunberger/charts/stable --security-view --env=dev
    image
    image
  • [X ] My code follows the style guidelines of this project
  • [ X] I have commented on my code, particularly in hard-to-understand areas
  • [ X] I have performed a self-review of my code
  • [ X] If it is a core feature, I have added thorough tests.
  • [ X] New and existing unit tests pass locally with my changes

Daniel Grunberger and others added 30 commits July 6, 2023 08:54
Signed-off-by: Daniel Grunberger <danielgrunberger@armosec.io>
Signed-off-by: Daniel Grunberger <danielgrunberger@armosec.io>
Add a CLI command that launches an image scan. Does not scan images yet.

Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
Signed-off-by: Daniel Grunberger <danielgrunberger@armosec.io>
Signed-off-by: Daniel Grunberger <danielgrunberger@armosec.io>
Signed-off-by: Daniel Grunberger <danielgrunberger@armosec.io>
Signed-off-by: Daniel Grunberger <danielgrunberger@armosec.io>
Signed-off-by: Daniel Grunberger <danielgrunberger@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
…can-workload

Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
This commit adds a CLI command and an associated package that scan
images for vulnerabilities.

Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>

feat(imagescan): fail on exceeding the severity threshold

Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
…kload

Signed-off-by: Amir Malka <amirm@armosec.io>
This commit adds the dependencies necessary for image scanning.

Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
…kload

Signed-off-by: Amir Malka <amirm@armosec.io>
Signed-off-by: Amir Malka <amirm@armosec.io>
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

PR Analysis

  • 🎯 Main theme: This pull request adds new output features to the codebase, including scan image output, scan workload command and output, and a security view for the scan command.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: No
  • Focused PR: Yes, the title and description clearly explain the purpose of the PR and the changes made.
  • 🔒 Security concerns: No, this PR does not introduce any security concerns.

PR Feedback

  • 💡 General PR suggestions: It would be beneficial to include relevant tests for the new features added in this PR to ensure their correctness and prevent regressions. Additionally, it would be helpful to provide documentation for the new features to guide users on how to use them.

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve: Suggest improvements to the code in the PR.
/ask <QUESTION>: Pose a question about the PR.

To edit any configuration parameter from 'configuration.toml', add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."

@dwertent
Copy link
Contributor

dwertent commented Aug 2, 2023

/improve

@github-actions
Copy link

github-actions bot commented Aug 2, 2023

Preparing review...

cmd/scan/scan.go Outdated
@@ -90,6 +91,8 @@ func GetScanCommand(ks meta.IKubescape) *cobra.Command {
scanCmd.PersistentFlags().BoolVarP(&scanInfo.Submit, "submit", "", false, "Submit the scan results to Kubescape SaaS where you can see the results in a user-friendly UI, choose your preferred compliance framework, check risk results history and trends, manage exceptions, get remediation recommendations and much more. By default the results are not submitted")
scanCmd.PersistentFlags().BoolVarP(&scanInfo.OmitRawResources, "omit-raw-resources", "", false, "Omit raw resources from the output. By default the raw resources are included in the output")
scanCmd.PersistentFlags().BoolVarP(&scanInfo.PrintAttackTree, "print-attack-tree", "", false, "Print attack tree")
scanCmd.PersistentFlags().BoolVarP(&scanInfo.ScanImages, "scan-images", "", false, "Scan resources images")
scanCmd.PersistentFlags().BoolVarP(&scanInfo.IsSecurityView, "security-view", "", false, "Show security view")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use --view instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would this work when using security view with resource view?

cmd/scan/framework.go Show resolved Hide resolved
cmd/scan/image.go Outdated Show resolved Hide resolved
return err
}
logger.L().Success("Image scan completed successfully")

scanInfo.SetScanType(cautils.ScanTypeImage)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to pre-run

cmd/scan/image.go Outdated Show resolved Hide resolved
var workloads []workloadinterface.IMetadata
var err error

if scanInfo.ChartPath != "" && scanInfo.FilePath != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use an interface instead

@@ -98,6 +107,80 @@ func (fileHandler *FileResourceHandler) GetResources(ctx context.Context, sessio
return k8sResources, allResources, ksResources, excludedRulesMap, nil
}

func getWorkloadFromHelmChart(ctx context.Context, helmPath, workloadPath string) (map[string]reporthandling.Source, []workloadinterface.IMetadata, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add unit tests


}

func getWorkloadSourceHelmChart(repoRoot string, source string, gitRepo *cautils.LocalGitRepository, helmChart cautils.Chart) reporthandling.Source {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add unit tests

var workloadSource reporthandling.Source
if clonedRepo != "" {
workloadSource = reporthandling.Source{
Path: "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Path: "",
Path: clonedRepo,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove if/else

pp.mainPrinter = prettyprinter.NewImagePrinter(pp.writer, pp.verboseMode)
case cautils.ScanTypeWorkload:
pp.mainPrinter = prettyprinter.NewWorkloadPrinter(pp.writer)
default:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add FW type

Signed-off-by: David Wertenteil <dwertent@armosec.io>
@dwertent dwertent added the trigger-integration-test Trigger integration test label Aug 2, 2023
Comment on lines 28 to 29
installHelmText = fmt.Sprintf("Install helm for continuos monitoring: %s", linkToHelm)
CICDSetupText = fmt.Sprintf("Add Kubescape to CICD: %s", linkToCICDSetup)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
installHelmText = fmt.Sprintf("Install helm for continuos monitoring: %s", linkToHelm)
CICDSetupText = fmt.Sprintf("Add Kubescape to CICD: %s", linkToCICDSetup)
installHelmText = fmt.Sprintf("Install Kubescape in your cluster for continuous monitoring: %s", linkToHelm)
CICDSetupText = fmt.Sprintf("Add Kubescape to your CI/CD pipeline: %s", linkToCICDSetup)

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Scan results:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: failure

@mrT23
Copy link

mrT23 commented Aug 3, 2023

Preparing review...

2 similar comments
@mrT23
Copy link

mrT23 commented Aug 3, 2023

Preparing review...

@mrT23
Copy link

mrT23 commented Aug 3, 2023

Preparing review...

@craigbox
Copy link
Contributor

craigbox commented Aug 3, 2023

Pitying fools...

Comment on lines +90 to 94
doc, err := models.NewDocument(presenterConfig.Packages, presenterConfig.Context, presenterConfig.Matches, presenterConfig.IgnoredMatches, presenterConfig.MetadataProvider, nil, presenterConfig.DBStatus)
if err != nil {
logger.L().Error(fmt.Sprintf("failed to create document for image: %v", imageScanData[i].Image), helpers.Error(err))
continue
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: It's recommended to handle errors properly. In the function convertToImageScanSummary, when an error occurs while creating a new document, the error is logged and the loop continues. This could lead to unexpected behavior if the error is not handled properly. Consider returning the error and handling it in the calling function.

Suggested change
doc, err := models.NewDocument(presenterConfig.Packages, presenterConfig.Context, presenterConfig.Matches, presenterConfig.IgnoredMatches, presenterConfig.MetadataProvider, nil, presenterConfig.DBStatus)
if err != nil {
logger.L().Error(fmt.Sprintf("failed to create document for image: %v", imageScanData[i].Image), helpers.Error(err))
continue
}
doc, err := models.NewDocument(presenterConfig.Packages, presenterConfig.Context, presenterConfig.Matches, presenterConfig.IgnoredMatches, presenterConfig.MetadataProvider, nil, presenterConfig.DBStatus)
if err != nil {
return nil, fmt.Errorf("failed to create document for image: %v: %w", imageScanData[i].Image, err)
}

Comment on lines +115 to +146
func (pp *PrettyPrinter) ActionPrint(_ context.Context, opaSessionObj *cautils.OPASessionObj, imageScanData []cautils.ImageScanData) {
if opaSessionObj != nil {
fmt.Fprintf(pp.writer, "\n"+getSeparator("^")+"\n")

sortedControlIDs := getSortedControlsIDs(opaSessionObj.Report.SummaryDetails.Controls) // ListControls().All())

switch pp.viewType {
case cautils.ControlViewType:
pp.printResults(&opaSessionObj.Report.SummaryDetails.Controls, opaSessionObj.AllResources, sortedControlIDs)
case cautils.ResourceViewType:
if pp.verboseMode {
pp.resourceTable(opaSessionObj)
}
}

pp.printOverview(opaSessionObj, pp.verboseMode)

pp.mainPrinter.PrintConfigurationsScanning(&opaSessionObj.Report.SummaryDetails, sortedControlIDs)

// When writing to Stdout, we aren’t really writing to an output file,
// so no need to print that we are
if pp.writer.Name() != os.Stdout.Name() {
printer.LogOutputFile(pp.writer.Name())
}

pp.printAttackTracks(opaSessionObj)
}

if len(imageScanData) > 0 {
pp.PrintImageScan(imageScanData)
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The function ActionPrint is doing too many things. It's printing the report, handling image scanning, and printing attack tracks. Consider breaking it down into smaller functions to improve readability and maintainability.

Suggested change
func (pp *PrettyPrinter) ActionPrint(_ context.Context, opaSessionObj *cautils.OPASessionObj, imageScanData []cautils.ImageScanData) {
if opaSessionObj != nil {
fmt.Fprintf(pp.writer, "\n"+getSeparator("^")+"\n")
sortedControlIDs := getSortedControlsIDs(opaSessionObj.Report.SummaryDetails.Controls) // ListControls().All())
switch pp.viewType {
case cautils.ControlViewType:
pp.printResults(&opaSessionObj.Report.SummaryDetails.Controls, opaSessionObj.AllResources, sortedControlIDs)
case cautils.ResourceViewType:
if pp.verboseMode {
pp.resourceTable(opaSessionObj)
}
}
pp.printOverview(opaSessionObj, pp.verboseMode)
pp.mainPrinter.PrintConfigurationsScanning(&opaSessionObj.Report.SummaryDetails, sortedControlIDs)
// When writing to Stdout, we aren’t really writing to an output file,
// so no need to print that we are
if pp.writer.Name() != os.Stdout.Name() {
printer.LogOutputFile(pp.writer.Name())
}
pp.printAttackTracks(opaSessionObj)
}
if len(imageScanData) > 0 {
pp.PrintImageScan(imageScanData)
}
}
func (pp *PrettyPrinter) ActionPrint(_ context.Context, opaSessionObj *cautils.OPASessionObj, imageScanData []cautils.ImageScanData) {
if opaSessionObj != nil {
pp.printReport(opaSessionObj)
}
if len(imageScanData) > 0 {
pp.PrintImageScan(imageScanData)
}
}
func (pp *PrettyPrinter) printReport(opaSessionObj *cautils.OPASessionObj) {
fmt.Fprintf(pp.writer, "\n"+getSeparator("^")+"\n")
sortedControlIDs := getSortedControlsIDs(opaSessionObj.Report.SummaryDetails.Controls)
switch pp.viewType {
case cautils.ControlViewType:
pp.printResults(&opaSessionObj.Report.SummaryDetails.Controls, opaSessionObj.AllResources, sortedControlIDs)
case cautils.ResourceViewType:
if pp.verboseMode {
pp.resourceTable(opaSessionObj)
}
}
pp.printOverview(opaSessionObj, pp.verboseMode)
pp.mainPrinter.PrintConfigurationsScanning(&opaSessionObj.Report.SummaryDetails, sortedControlIDs)
if pp.writer.Name() != os.Stdout.Name() {
printer.LogOutputFile(pp.writer.Name())
}
pp.printAttackTracks(opaSessionObj)
}

@ghost
Copy link

ghost commented Aug 3, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Scan results:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: failure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
trigger-integration-test Trigger integration test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants