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

Handle Pod Security Context #2630

Merged
merged 6 commits into from Jan 25, 2023

Conversation

ben-elttam
Copy link
Contributor

  • Original rule only considered security context at container level, but security context can be applied at pod level as well.
  • Updated the rules to handle both levels. This reduces false positives.
  • Kubernetes gives precedence to container security context, test file includes positive/negative cases where both pod and container security context exist.

- Original rule only considered security context at container level,
  but security context can be applied at pod level as well.
- Updated the rules to handle both levels. This reduces false positives.
- Kubernetes gives precedence to container security context, test file
  includes positive/negative cases where both pod and container security
  context exist.
@ben-elttam
Copy link
Contributor Author

The pre-commit hook yaml check fails because of multi-document yaml in the tests.
I feel this is best way to test different cases to ensure the rule is working correctly (accurate). Any suggestions on how to handle this?

@mjambon mjambon requested a review from a team January 7, 2023 03:08
ben-elttam added a commit to elttam/semgrep-rules that referenced this pull request Jan 13, 2023
- This file used to be excluded from check-yaml in pre-commit.
- It's actually multi-document, hence the duplicate keys, but missing the `---` delinator.
- Add missing `---`
- Future updates will be checked against check-yaml with allow
  multi-documents.
@ben-elttam
Copy link
Contributor Author

I've updated .pre-commit-config.yaml to allow multi-document in yaml/kubernetes tests. I've also included checking semgrep-github-action-push-without-branches.test.yml which used to be excluded (would have no yaml checks). And fixed the missing --- document separators in it. pre-commit is now happy, so GitHub Actions are now happy.

@ben-elttam
Copy link
Contributor Author

I fixed the pre-commit exclude/include regex, it needed the full path.
I've also added a name for the second yaml check to distinguish it (multi-documents).

pre-commit run --all
Check for case conflicts.................................................Passed
Check for added large files..............................................Passed
Check that executables have shebangs.....................................Passed
Check for merge conflicts................................................Passed
Check for broken symlinks............................(no files to check)Skipped
Check Yaml...............................................................Passed
Check Yaml (Multi-documents).............................................Passed

@kurt-r2c kurt-r2c merged commit 895f70a into semgrep:develop Jan 25, 2023
@ben-elttam ben-elttam deleted the f-kubernetes-pod-run-as-non-root branch January 30, 2023 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants