From a6203a520afb6266b952aeab7222f857cc076170 Mon Sep 17 00:00:00 2001 From: Erazem Kokot Date: Thu, 15 Oct 2026 10:00:16 +0200 Subject: [PATCH] chore: General repo cleanup This commit removes Nghia from codeowners, applies linter fixes to most Python files, switches to using requirements.in and updates dependencies. The only functional changes are not printing the contents of .global-checks-tooling/trufflehog.json since those seemingly can contain secrets, and exiting with exit code 1 if trufflehog checks fail. It made sense to me to fail the CI in that case, but let me know if this is not desired and I'll revert it. https://app.devrev.ai/devrev/issue/FORGE-687 --- .circleci/config.yml | 2 +- .github/CODEOWNERS | 2 +- .github/workflows/global-checks.yml | 9 ++-- .github/workflows/patching-sla-check.yml | 8 +-- .github/workflows/validate-description.yml | 6 +-- Makefile | 8 ++- README.md | 6 +++ checks/check_codeowners.py | 7 ++- checks/check_commit_emails.py | 5 +- checks/check_patching_sla.py | 58 ++++++++++++-------- checks/check_pr_description.py | 17 +++--- checks/check_repo_yml.py | 18 ++++--- checks/check_trufflehog.py | 47 +++++++++------- checks/test_checks.py | 62 ++++++++++++++-------- requirements.in | 3 ++ requirements.txt | 25 ++++++++- 16 files changed, 189 insertions(+), 94 deletions(-) create mode 100644 requirements.in diff --git a/.circleci/config.yml b/.circleci/config.yml index a2375a5..a7e2e5e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -15,7 +15,7 @@ jobs: steps: - sidekick/tidy_build_publish: cache_key: r20230619 - cache_files: requirements.txt Makefile Makefile.sidekick + cache_files: requirements.in requirements.txt Makefile Makefile.sidekick publish: << parameters.publish >> snyk_python: true snyk_enabled: true diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index f4085ad..939ff88 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1 @@ -* jo@devrev.ai nn@devrev.ai +* jo@devrev.ai diff --git a/.github/workflows/global-checks.yml b/.github/workflows/global-checks.yml index a2b587c..fdbeab5 100644 --- a/.github/workflows/global-checks.yml +++ b/.github/workflows/global-checks.yml @@ -9,17 +9,17 @@ jobs: runs-on: ubuntu-latest steps: - name: Check out target repository - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: fetch-depth: 0 - name: Check out workflow tooling repository - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: - repository: devrev/jano-gh-workflow + repository: devrev/global-gh-tools path: .global-checks-tooling - name: Load python requirements. run: | - pip install strictyaml + python -m pip install -r .global-checks-tooling/requirements.txt - name: Check that there is a CODEOWNERS file run: | echo "Check whether the repo has a .github/CODEOWNERS file" @@ -42,5 +42,4 @@ jobs: --no-fail \ --no-update \ --json > .global-checks-tooling/trufflehog.json - cat .global-checks-tooling/trufflehog.json python .global-checks-tooling/checks/check_trufflehog.py .global-checks-tooling/trufflehog.json diff --git a/.github/workflows/patching-sla-check.yml b/.github/workflows/patching-sla-check.yml index 4b1269e..e1a63cf 100644 --- a/.github/workflows/patching-sla-check.yml +++ b/.github/workflows/patching-sla-check.yml @@ -13,23 +13,23 @@ jobs: pull-requests: write steps: - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@v4 + uses: aws-actions/configure-aws-credentials@v6 with: role-to-assume: arn:aws:iam::162856926107:role/gh-action-security-tools aws-region: us-east-1 - name: Check out target repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: fetch-depth: 0 - name: Check out workflow tooling repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: repository: devrev/global-gh-tools ref: main path: .global-checks-tooling - name: Load python requirements. run: | - pip install strictyaml boto3 + python -m pip install -r .global-checks-tooling/requirements.txt - name: Run patching SLA check id: patch_check continue-on-error: true diff --git a/.github/workflows/validate-description.yml b/.github/workflows/validate-description.yml index 7c69cab..f2a1d7f 100644 --- a/.github/workflows/validate-description.yml +++ b/.github/workflows/validate-description.yml @@ -15,7 +15,7 @@ jobs: contents: read steps: - name: Check out workflow tooling repository - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: repository: devrev/global-gh-tools path: .global-checks-tooling @@ -40,12 +40,12 @@ jobs: if [ "$pr_body" == "null" ] || [ -z "$pr_body" ]; then echo "❌ ERROR: Failed to fetch PR description" echo "" - + if [ -n "$api_message" ]; then echo "API Message: $api_message" echo "" fi - + echo "API Response:" echo "$response" | jq '.' exit 1 diff --git a/Makefile b/Makefile index e05777c..3f1de09 100644 --- a/Makefile +++ b/Makefile @@ -26,4 +26,10 @@ deps: $(venv)/.deps .PHONY: deps test: $(venv)/.deps - $(py) -m unittest \ No newline at end of file + $(py) -m unittest + +update-requirements: $(py) + $(py) -m uv pip compile --python-version $(PYTHON_VERSION) --universal \ + --upgrade --custom-compile-command "make update-requirements" \ + requirements.in --output-file requirements.txt +.PHONY: update-requirements diff --git a/README.md b/README.md index 085383f..7174fa4 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,12 @@ # global-gh-tools + Globally required workflow to enforce: + - The use of a CODEOWNERS file. - The use of a repo.yml file. - The use of devrev.ai emails in commits. - No credentials getting committed. + +## Python dependencies + +Update Python dependencies with `make update-requirements`. diff --git a/checks/check_codeowners.py b/checks/check_codeowners.py index 85a5192..32fa6a2 100644 --- a/checks/check_codeowners.py +++ b/checks/check_codeowners.py @@ -1,7 +1,9 @@ import os +import sys CODEOWNERS_FILE = ".github/CODEOWNERS" + def check_codeowners(): """ Check if the CODEOWNERS file exists and is not empty. @@ -17,9 +19,10 @@ def check_codeowners(): print(f"{CODEOWNERS_FILE} file exists and is not empty.") return True + if __name__ == "__main__": if not check_codeowners(): print("Please add a CODEOWNERS file to the repository.") - exit(1) + sys.exit(1) else: - print("CODEOWNERS file check passed.") \ No newline at end of file + print("CODEOWNERS file check passed.") diff --git a/checks/check_commit_emails.py b/checks/check_commit_emails.py index 0540c23..244817f 100644 --- a/checks/check_commit_emails.py +++ b/checks/check_commit_emails.py @@ -1,4 +1,5 @@ import os +import sys EMAILS_PATH = ".global-checks-tooling/committers.txt" @@ -6,6 +7,7 @@ "actions@github.com", ] + # Check that the emails in the file are from @devrev.ai. def check_commit_emails(): """ @@ -29,7 +31,8 @@ def check_commit_emails(): print(f"{EMAILS_PATH} file exists and contains valid emails.") return True + if __name__ == "__main__": if not check_commit_emails(): print("Please ensure all committer emails are from @devrev.ai.") - exit(1) \ No newline at end of file + sys.exit(1) diff --git a/checks/check_patching_sla.py b/checks/check_patching_sla.py index b4c6c3e..e7a048a 100644 --- a/checks/check_patching_sla.py +++ b/checks/check_patching_sla.py @@ -1,9 +1,8 @@ -import json -import sys -import os import fnmatch +import os +import sys + import boto3 -from botocore.exceptions import ClientError # The list of allowed files/patterns in the patch. If any file outside this list is modified, the patch will be blocked. # The list includes any file that is necessary for dependency management or build configuration. @@ -27,9 +26,10 @@ "pnpm-lock.yaml", "build.gradle", "pyproject.toml", - ".dockerignore" + ".dockerignore", ] + def is_file_allowed(file_path, allowed_patterns): """ Check if a file basename matches any of the allowed patterns. @@ -49,6 +49,7 @@ def is_file_allowed(file_path, allowed_patterns): return False + def query_dynamodb_vulns(repo_name): """ Query DynamoDB for vulnerability information by repo_name. @@ -65,18 +66,23 @@ def query_dynamodb_vulns(repo_name): region_name = os.environ.get("AWS_DEFAULT_REGION", "us-east-1") table_name = os.environ.get("DYNAMODB_TABLE_NAME", "blocked-repos") - dynamodb = boto3.resource('dynamodb', endpoint_url=endpoint_url, region_name=region_name) + dynamodb = boto3.resource( + "dynamodb", endpoint_url=endpoint_url, region_name=region_name + ) table = dynamodb.Table(table_name) - response = table.get_item(Key={'repo_name': repo_name}) + response = table.get_item(Key={"repo_name": repo_name}) - if 'Item' in response and 'vulns' in response['Item']: - return response['Item']['vulns'] + if "Item" in response and "vulns" in response["Item"]: + return response["Item"]["vulns"] return None + def check_patching_sla(): if len(sys.argv) != 4: - print("Usage: python check_patching_sla.py ") + print( + "Usage: python check_patching_sla.py " + ) sys.exit(1) changed_files_path = sys.argv[1] @@ -90,25 +96,33 @@ def check_patching_sla(): changed_files = f.read().splitlines() for file in changed_files: if not is_file_allowed(file, ALLOWED_FILES): - print(f"Repository {repo_name} is in the blocked repos list. Please see PR comment for details.") - with open(comment_file, 'w') as f: - f.write(f"## ⚠️ Heads-up: This repository will be blocked from any work other than patching.\n") + print( + f"Repository {repo_name} is in the blocked repos list. Please see PR comment for details." + ) + with open(comment_file, "w") as f: + f.write( + "## ⚠️ Heads-up: This repository will be blocked from any work other than patching.\n" + ) f.write(f"File {file} is not allowed to be modified in this patch.\n") f.write("The following vulnerability issues are past SLA:\n") for issue in vulns: - id = issue['id'] - overdue_days = issue['overdue_days'] - severity = issue['severity'] - owner_email = issue['owner_email'] - last_checked = issue['timestamp'] + id = issue["id"] + overdue_days = issue["overdue_days"] + severity = issue["severity"] + owner_email = issue["owner_email"] + last_checked = issue["timestamp"] # Extract issue number from id, it is the last part after the last slash. - issue_number = id.split('/')[-1] - f.write(f"- ISS-{issue_number}, overdue {overdue_days:.1f}d, sev {severity}, owner: {owner_email}, checked: {last_checked}\n") - f.write("\nNote that there is significant latency in updating this list. Please reach out on #antifragile if you are in a hurry or have an emergency.\n") + issue_number = id.split("/")[-1] + f.write( + f"- {issue_number}, overdue {overdue_days:.1f}d, sev {severity}, owner: {owner_email}, checked: {last_checked}\n" + ) + f.write( + "\nNote that there is significant latency in updating this list. Please reach out on #antifragile if you are in a hurry or have an emergency.\n" + ) return False return True if __name__ == "__main__": if not check_patching_sla(): - exit(1) + sys.exit(1) diff --git a/checks/check_pr_description.py b/checks/check_pr_description.py index aab0aa5..871ef3d 100644 --- a/checks/check_pr_description.py +++ b/checks/check_pr_description.py @@ -1,6 +1,7 @@ import re import sys + def check_description(description): """ Checks if the PR description contains a work item link. @@ -22,6 +23,7 @@ def check_description(description): ) return bool(re.search(regex, description, re.IGNORECASE)) + def check_description_cli(description): """ CLI version that prints messages and exits. @@ -29,24 +31,27 @@ def check_description_cli(description): print(f"Checking PR description: {description}") if not check_description(description): print("❌ VALIDATION FAILED") - print("") + print() print("Reason: PR description must include a work item link") - print("") + print() print( "Valid formats: ISS-123, TKT-456, TASK-789, ABC-123, " "https://app.devrev.ai/devrev/works/ISS-123, or " "https://app.devrev.ai/devrev/issue/DES-123" ) - print("Note: Use 'work-item: ISS-123' (space after colon), not 'work-item:ISS-123'") + print( + "Note: Use 'work-item: ISS-123' (space after colon), not 'work-item:ISS-123'" + ) sys.exit(1) - + print("✅ VALIDATION PASSED - PR description contains a valid work item link") sys.exit(0) + if __name__ == "__main__": if len(sys.argv) < 2: - print("Usage: python check_pr_description.py \"\"") + print('Usage: python check_pr_description.py ""') sys.exit(1) - + pr_description = sys.argv[1] check_description_cli(pr_description) diff --git a/checks/check_repo_yml.py b/checks/check_repo_yml.py index 6e4dab6..e16487a 100644 --- a/checks/check_repo_yml.py +++ b/checks/check_repo_yml.py @@ -1,20 +1,25 @@ import os +import sys + import strictyaml ANNOTATION_FILE = ".devrev/repo.yml" -schema = strictyaml.Map({ - "deployable": strictyaml.Bool(), -}) +schema = strictyaml.Map( + { + "deployable": strictyaml.Bool(), + } +) + def check_repo_yml(): if not os.path.exists(ANNOTATION_FILE): print(f"Annotation file {ANNOTATION_FILE} does not exist.") return False - + with open(ANNOTATION_FILE) as f: text = f.read() - + try: # Validate the YAML file against the schema strictyaml.load(text, schema) @@ -23,7 +28,8 @@ def check_repo_yml(): return False return True + if __name__ == "__main__": if not check_repo_yml(): print("Please ensure the .devrev/repo.yml file exists and is valid.") - exit(1) \ No newline at end of file + sys.exit(1) diff --git a/checks/check_trufflehog.py b/checks/check_trufflehog.py index b580543..35ed413 100644 --- a/checks/check_trufflehog.py +++ b/checks/check_trufflehog.py @@ -1,18 +1,24 @@ import json import os -import strictyaml import sys +import strictyaml + CREDS_FILE = ".devrev/creds.yml" -schema = strictyaml.Map({ - "allow": strictyaml.Seq( - strictyaml.Map({ - "path": strictyaml.Str(), - "lines": strictyaml.Str(), - }) - ), -}) +schema = strictyaml.Map( + { + "allow": strictyaml.Seq( + strictyaml.Map( + { + "path": strictyaml.Str(), + "lines": strictyaml.Str(), + } + ) + ), + } +) + def get_creds_overrides(): allowed = {} @@ -21,13 +27,13 @@ def get_creds_overrides(): with open(CREDS_FILE) as f: text = f.read() data = strictyaml.load(text, schema) - for override in data.data["allow"]: - path = override["path"] + for override in data["allow"]: + path = override["path"].text # Decode lines string field. It can contain: # 1. A single line number # 2. A range of line numbers (e.g., "1-5") # 3. A comma-separated list of line number ranaged (e.g., "1,2,3-8") - parts = override["lines"].split(",") + parts = override["lines"].text.split(",") lines = set() for part in parts: if "-" in part: @@ -44,18 +50,18 @@ def get_creds_overrides(): return allowed + def is_overridden(filename, line_num, creds_overrides): lines = creds_overrides.get(filename) if lines is None: return False - if line_num in lines: - return True - return False + return line_num in lines + def check_trufflehog(json_path): cred_overrides = get_creds_overrides() with open(json_path) as f: - for line in f.readlines(): + for line in f: line = line.strip() if len(line) == 0: continue @@ -67,23 +73,26 @@ def check_trufflehog(json_path): # Special handling for github tokens in commit messages: if detector_name == "Github" and fn is None: - print(f"Skipping {detector_name} finding because detection is in commit message") + print( + f"Skipping {detector_name} finding because detection is in commit message" + ) continue if is_overridden(fn, line_num, cred_overrides): print(f"Skipping {fn}:{line_num} because it is in the creds.yml file") continue if fn is None: # Clarify that the finding is in a commit message. - print(f"") + print("") print(f"Found secret in {fn}:{line_num}") return False print("No secrets found") return True + if __name__ == "__main__": if len(sys.argv) < 2: print("Usage: python check_trufflehog.py ") sys.exit(1) json_path = sys.argv[1] - check_trufflehog(json_path) + sys.exit(0 if check_trufflehog(json_path) else 1) diff --git a/checks/test_checks.py b/checks/test_checks.py index 35232d3..8ea317a 100644 --- a/checks/test_checks.py +++ b/checks/test_checks.py @@ -1,13 +1,14 @@ +import os +import unittest +from contextlib import contextmanager +from pathlib import Path + import checks.check_codeowners import checks.check_commit_emails +import checks.check_pr_description import checks.check_repo_yml import checks.check_trufflehog -import checks.check_pr_description -import unittest -import os -from pathlib import Path -from contextlib import contextmanager @contextmanager def change_dir(destination): @@ -18,6 +19,7 @@ def change_dir(destination): finally: os.chdir(prev_dir) # Revert to the original directory + class TestCodeOwners(unittest.TestCase): def test_without_codeowners(self): with change_dir("checks/fixtures/codeowners/without"): @@ -27,6 +29,7 @@ def test_with_codeowners(self): with change_dir("checks/fixtures/codeowners/with"): self.assertTrue(checks.check_codeowners.check_codeowners()) + class TestRepoYaml(unittest.TestCase): def test_good1_file(self): with change_dir("checks/fixtures/repo_yml/good1"): @@ -36,37 +39,44 @@ def test_bad1_file(self): with change_dir("checks/fixtures/repo_yml/bad1"): self.assertFalse(checks.check_repo_yml.check_repo_yml()) + class TestTrufflehog(unittest.TestCase): def test_empty(self): with change_dir("checks/fixtures/trufflehog/empty"): - self.assertTrue(checks.check_trufflehog.check_trufflehog( - "./trufflehog.json")) + self.assertTrue( + checks.check_trufflehog.check_trufflehog("./trufflehog.json") + ) def test_bad(self): with change_dir("checks/fixtures/trufflehog/bad"): - self.assertFalse(checks.check_trufflehog.check_trufflehog( - "./trufflehog.json")) - + self.assertFalse( + checks.check_trufflehog.check_trufflehog("./trufflehog.json") + ) def test_override(self): with change_dir("checks/fixtures/trufflehog/override"): - self.assertTrue(checks.check_trufflehog.check_trufflehog( - "./trufflehog.json")) + self.assertTrue( + checks.check_trufflehog.check_trufflehog("./trufflehog.json") + ) def test_mixed(self): with change_dir("checks/fixtures/trufflehog/mixed"): - self.assertFalse(checks.check_trufflehog.check_trufflehog( - "./trufflehog.json")) + self.assertFalse( + checks.check_trufflehog.check_trufflehog("./trufflehog.json") + ) def test_two_overrides(self): with change_dir("checks/fixtures/trufflehog/two_overrides"): - self.assertTrue(checks.check_trufflehog.check_trufflehog( - "./trufflehog.json")) + self.assertTrue( + checks.check_trufflehog.check_trufflehog("./trufflehog.json") + ) def test_merge_message(self): with change_dir("checks/fixtures/trufflehog/merge_message"): - self.assertTrue(checks.check_trufflehog.check_trufflehog( - "./trufflehog.json")) + self.assertTrue( + checks.check_trufflehog.check_trufflehog("./trufflehog.json") + ) + class TestCommitEmails(unittest.TestCase): def test_good(self): @@ -77,6 +87,7 @@ def test_bad(self): with change_dir("checks/fixtures/emails/bad"): self.assertFalse(checks.check_commit_emails.check_commit_emails()) + class TestPRDescription(unittest.TestCase): def test_valid_iss_link(self): with open("checks/fixtures/pr_description/valid/iss_link.txt", "r") as f: @@ -99,12 +110,16 @@ def test_valid_full_url(self): self.assertTrue(checks.check_pr_description.check_description(description)) def test_valid_generic_display_id(self): - with open("checks/fixtures/pr_description/valid/generic_display_id.txt", "r") as f: + with open( + "checks/fixtures/pr_description/valid/generic_display_id.txt", "r" + ) as f: description = f.read() self.assertTrue(checks.check_pr_description.check_description(description)) def test_valid_issue_url_display_id(self): - with open("checks/fixtures/pr_description/valid/issue_url_display_id.txt", "r") as f: + with open( + "checks/fixtures/pr_description/valid/issue_url_display_id.txt", "r" + ) as f: description = f.read() self.assertTrue(checks.check_pr_description.check_description(description)) @@ -119,9 +134,12 @@ def test_invalid_wrong_format(self): self.assertFalse(checks.check_pr_description.check_description(description)) def test_invalid_issue_url_lowercase_key(self): - with open("checks/fixtures/pr_description/invalid/issue_url_lowercase_key.txt", "r") as f: + with open( + "checks/fixtures/pr_description/invalid/issue_url_lowercase_key.txt", "r" + ) as f: description = f.read() self.assertFalse(checks.check_pr_description.check_description(description)) -if __name__ == '__main__': + +if __name__ == "__main__": unittest.main() diff --git a/requirements.in b/requirements.in new file mode 100644 index 0000000..a364516 --- /dev/null +++ b/requirements.in @@ -0,0 +1,3 @@ +boto3 +strictyaml +urllib3 diff --git a/requirements.txt b/requirements.txt index 16e2845..2adc671 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,26 @@ +# This file was autogenerated by uv via the following command: +# make update-requirements +boto3==1.43.114 + # via -r requirements.in +botocore==1.43.114 + # via + # boto3 + # s3transfer +jmespath==1.1.0 + # via + # boto3 + # botocore +python-dateutil==2.9.0.post0 + # via + # botocore + # strictyaml +s3transfer==0.19.2 + # via boto3 +six==1.17.0 + # via python-dateutil strictyaml==1.7.3 -boto3==1.42.92 + # via -r requirements.in urllib3==2.8.0 + # via + # -r requirements.in + # botocore