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

chore(general): Add a job to ensure evaluated keys are added to new resource checks #6958

Merged
merged 5 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions .github/workflows/pr-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -494,3 +494,61 @@ jobs:
run: |
pipenv run pytest
working-directory: ${{ env.WORKING_DIRECTORY }}

eval-keys-test:
runs-on: ubuntu-latest
steps:
- name: Check out repository
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v3

- name: Get changed Python files
id: changed-files
uses: tj-actions/changed-files@6b2903bdce6310cfbddd87c418f253cf29b2dec9 # v44
with:
files: checkov/**/*.py

- name: Validate 'BaseResourceCheck' use contains eval keys
if: steps.changed-files.outputs.any_changed == 'true'
run: |
# Define an array of exceptions (files to skip)
EXCEPTIONS=(
"base_resource_check.py"
"VPCDefaultNetwork.py"
"IAMUserNotUsedForAccess.py" # Whole Resource type check
)

echo "Changed files:"
echo "${{ steps.changed-files.outputs.all_changed_files }}"

EXIT_CODE=0
IFS=$'\n' # Change Internal Field Separator to handle spaces in filenames too
for file in $(echo "${{ steps.changed-files.outputs.all_changed_files }}" | tr ',' '\n'); do
# Check if the file is in the list of exceptions
SKIP_FILE="false"
for exception in "${EXCEPTIONS[@]}"; do
# If the file ends with one of the exception file names, skip it
if [[ "$file" == *"$exception" ]]; then
echo "Skipping $file (allowed exception)"
SKIP_FILE="true"
break
fi
done

# Only run checks if not in exceptions list
if [[ "$SKIP_FILE" == "false" ]]; then
# If file contains 'BaseResourceCheck', check for 'get_inspected_key' or 'evaluated_keys'
Copy link
Contributor

Choose a reason for hiding this comment

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

this is only a partial validation, as BaseResourceCheck doesn't exist in all of the code.
for example, if we inherited from it a class called BaseCloudformationResourceCheck, any check which inerits from this one will not catch this.
I don't mind adding this check for now, but a better solution would be a python linter (flake8-plugin for example) which can catch this issue

if grep -q "BaseResourceCheck" "$file"; then
if ! grep -q "get_inspected_key" "$file" && ! grep -q "evaluated_keys" "$file"; then
echo "ERROR: $file has BaseResourceCheck but does NOT contain 'get_inspected_key' or 'evaluated_keys'"
EXIT_CODE=1
fi
fi
fi
done
unset IFS # Restore IFS to default

# Fail the job if any file violated the rule
if [ "$EXIT_CODE" -ne 0 ]; then
echo "One or more files did not satisfy the requirement."
exit 1
fi
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Dict, Any
from typing import Dict, Any, List

from checkov.common.models.enums import CheckCategories, CheckResult
from checkov.arm.base_resource_check import BaseResourceCheck
Expand Down Expand Up @@ -29,5 +29,8 @@ def scan_resource_conf(self, conf: Dict[str, Any]) -> CheckResult:
return CheckResult.PASSED
return CheckResult.FAILED

def get_evaluated_keys(self) -> List[str]:
return ["properties", "properties/[0]/managedNetwork", "properties/[0]/managedNetwork/[0]/outboundRules"]


check = AzureMLWorkspacePrivateEndpoint()
Loading