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

refactor(query): containers_run_with_low_uid rewrite #3430

Merged

Conversation

rogeriopeixotocx
Copy link
Contributor

I submit this contribution under the Apache-2.0 license.

@rogeriopeixotocx rogeriopeixotocx added query New query feature kubernetes Kubernetes query labels May 25, 2021
@rogeriopeixotocx rogeriopeixotocx added this to the Queries Support milestone May 25, 2021
@rogeriopeixotocx rogeriopeixotocx requested a review from a team May 25, 2021 09:15
@rogeriopeixotocx rogeriopeixotocx self-assigned this May 25, 2021
@kicsbot
Copy link
Contributor

kicsbot commented May 25, 2021

Scan submitted to Checkmarx

@kicsbot
Copy link
Contributor

kicsbot commented May 25, 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

Copy link
Collaborator

@joaoReigota1 joaoReigota1 left a comment

Choose a reason for hiding this comment

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

Other than a comment LGTM

"documentId": id,
"searchKey": sprintf("metadata.name={{%s}}.%sspec.securityContext.runAsUser", [metadata.name, path]),
"documentId": doc.id,
"searchKey": sprintf("%s.securityContext.runAsUser=%d", [common_lib.concat_path(path), securityContext.runAsUser]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since in line 11 we use to_number, I assume security context can be something other than an integer, maybe use to_number in the search key as well so it doesn't conflict with %d

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"searchKey": sprintf("%s.securityContext.runAsUser=%d", [common_lib.concat_path(path), securityContext.runAsUser]),
"searchKey": sprintf("%s.securityContext.runAsUser=%d", [common_lib.concat_path(path), to_number(securityContext.runAsUser)]),

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to add to_number here, since it will only execute this line if runAsUser is a number, otherwise, it will fail on the first to_number, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think @felipe-avelar is right @joaoReigota1

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.

LGTM

@rogeriopeixotocx rogeriopeixotocx merged commit a97af93 into master May 25, 2021
@rogeriopeixotocx rogeriopeixotocx deleted the feature/refactor-containers-run-with-low-uid branch May 25, 2021 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kubernetes Kubernetes query query New query feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants