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

fix(query): fixed issue containers_running_as_root #3412 #3422

Conversation

mukeshpilaniya
Copy link
Contributor

@mukeshpilaniya mukeshpilaniya commented May 24, 2021

Fixes #3412 Containers_running_as_root query failed to evaluate

Description

Added one more corner case and fixed already existing issue #3412

#if pod runAsNonRoot==true and container runAsNonRoot==true (container not runs as root)
#if pod runAsNonRoot==true and container runAsNonRoot==false 
							#if container runAsUser>0 (container not runs as root)
							#if container runAsUser<=0 (container runs as root) //new added corner case

@kicsbot
Copy link
Contributor

kicsbot commented May 24, 2021

Scan submitted to Checkmarx

@kicsbot
Copy link
Contributor

kicsbot commented May 24, 2021

Logo
Checkmarx SAST - Scan Summary & Details

Cx-SAST Summary

Total of 0 vulnerabilities
High 0 High
Medium 0 Medium
Low 0 Low
Info 0 Info

Violation Summary

No policy violation found

@rogeriopeixotocx rogeriopeixotocx added the community Community contribution label May 24, 2021
@kaplanlior
Copy link
Contributor

Hi @mukeshpilaniya , first welcome to KICS, we're happy to get a PR from you 👍

A bit on the OCD side, but can you fix the typo in the PR/commit? container_ruuiing_as_root -> containers_running_as_root and to close the issue it refers (see this ).

Thanks (:

@rogeriopeixotocx rogeriopeixotocx changed the title fix(query): fixed issue container_ruuiing_as_root#3412 fix(query): fixed issue containers_running_as_root #3412 May 24, 2021
Copy link
Contributor

@felipe-avelar felipe-avelar left a comment

Choose a reason for hiding this comment

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

In general, I think the query changes looks good, I would like just to ask to you to add positive and negative samples covering your new edge case.
If you need some help with this, don't hesitate to ask by @ us at this thread, using gitter or [email protected]

assets/queries/k8s/containers_running_as_root/query.rego Outdated Show resolved Hide resolved
@kicsbot
Copy link
Contributor

kicsbot commented May 25, 2021

Scan not submitted to Checkmarx due to existing Active scan for the same project.

Copy link
Contributor

@rogeriopeixotocx rogeriopeixotocx left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @mukeshpilaniya

We just need to check why integration tests are failing

@rogeriopeixotocx
Copy link
Contributor

This PR should fix the integration tests issue #3430

@rogeriopeixotocx
Copy link
Contributor

Hi @mukeshpilaniya, now that #3430 is merged can you please update your branch to the master so we can run the PR checks again?

@mukeshpilaniya
Copy link
Contributor Author

Hi @mukeshpilaniya, now that #3430 is merged can you please update your branch to the master so we can run the PR checks again?

@rogeriopeixotocx, I updated my branch to the master branch. now you can run the Integration test again.

@joaoReigota1
Copy link
Collaborator

Hi @mukeshpilaniya, now that #3430 is merged can you please update your branch to the master so we can run the PR checks again?

@rogeriopeixotocx, I updated my branch to the master branch. now you can run the Integration test again.

Hi @mukeshpilaniya, I think you forgot to pull the latest changes in the master branch, can you pull and then merge?

Copy link
Contributor

@rogeriopeixotocx rogeriopeixotocx left a comment

Choose a reason for hiding this comment

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

LGTM

@rogeriopeixotocx rogeriopeixotocx merged commit ea67baa into Checkmarx:master May 25, 2021
@rogeriopeixotocx
Copy link
Contributor

@mukeshpilaniya thank you for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working community Community contribution query New query feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Containers_running_as_root query faild to evaluate
6 participants