feat(codeql): add CodeQL enablement check as a dedicated extension#876
feat(codeql): add CodeQL enablement check as a dedicated extension#876akiioto wants to merge 27 commits into
Conversation
Introduces a new CodeQL extension that checks whether GitHub Advanced Security CodeQL is enabled for each source artefact referenced by an OCM component. Changes: - odg/model.py: add Datatype.CODEQL_FINDING, Datasource.CODEQL, CodeqlStatus, CodeqlFinding, RescoreCodeqlFinding - odg/extensions_cfg.py: add Services.CODEQL and CodeqlConfig (BacklogItemMixins) - odg/extensions_cfg.yaml: add codeql default (enabled: False) - odg/findings_cfg.yaml: add finding/codeql block with BLOCKER categorisation; external (github.com) components are auto-rescored to NONE - codeql.py: new extension — queries GitHub code-scanning analyses API (tool_name=CodeQL), emits CodeqlFinding(NOT_ENABLED) when CodeQL is absent
Per discussion with Philipp Heil: findings should contain enough information to be meaningful in the dashboard and issue view. repo_url points directly to the GitHub repository where CodeQL needs to be enabled.
Required for categorise_finding() to match on codeql_status values in the findings_cfg.yaml selector configuration.
Adds the codeql backlog item creation block analogous to sast, so the artefact-enumerator creates codeql backlog items for source artefacts when the codeql extension is enabled.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- settings_url: direct link to repo/settings/security_analysis so engineers can enable CodeQL in one click from the dashboard/issue - language: primary repo language from GitHub API; enables auto-rescoring for repos in languages CodeQL does not support - fix: key now includes repo_url to avoid collisions across artefacts - refactor: fetch_repo_info() combines /repos and /code-scanning/analyses into one function so both calls share the same auth token lookup
languages: [Go, Python, Java] in extensions_cfg limits CodeQL checks to repos whose primary language matches. Empty list (default) checks all.
Instead of a single enabled/disabled check per repo, the extension now:
- fetches active CodeQL languages from code-scanning/analyses (environment.language)
- emits a separate CodeqlFinding per language configured in codeql_config.languages
that is NOT actively scanned
- key includes language: not-enabled|{repo_url}|{language}
When languages is empty (default), falls back to single finding if CodeQL
is not enabled at all for the repo.
8R0WNI3
left a comment
8R0WNI3
left a comment
There was a problem hiding this comment.
This already looks very promising, thank you for your contribution! :-)
Addresses review comment: Services, Datatype, Datasource.datasource(), and ExtensionsConfiguration fields are now in alphabetical order.
- CodeqlFinding.repo_url and language are now str (not optional) — findings are never created without both values - Remove settings_url from CodeqlFinding — it is derivable as repo_url + '/settings/security_analysis' and does not need to be stored - Remove /repos API call (was only needed for settings_url) - Add /languages API call to skip findings for languages not present in repo - languages config is now required — empty list logs warning and skips - RescoreCodeqlFinding key updated to match CodeqlFinding key
Each finding type has a corresponding _validate_* method in findings.py that checks categorisation selectors are correctly typed. Added _validate_codeql() using CodeqlFindingSelector, registered in _validate() match block alphabetically.
- Remove hostname fallback to 'github.com' — return None with warning instead
- Remove access.branch (field does not exist on GithubAccess) — use access.ref
- Return early with warning when ref is None
- Normalize ref to refs/heads/{ref} if not already prefixed with refs/
process_backlog_items() already creates and injects secret_factory into the callback — no need to create it manually and pass via functools.partial. Also removes the now-unused ctx_util import.
Extensions should not depend on each other. Moved github_api_request, github_api_request_paginated, find_token_for_repo_url and find_token_for_api_url from ghas.py to github_util.py. codeql.py now imports from github_util directly. ghas.py retains its public API via thin wrappers delegating to github_util.
The intent is that all repos get a finding if CodeQL is not enabled — there are no automatic exemptions. Users can manually rescore if needed. No CodeqlRescoringRule implementation exists anyway.
OCM GithubAccess.repoUrl may omit the https:// scheme. Add it when missing before parsing hostname/path.
…api_url
github.com API URLs have format /repos/{org}/{repo}/... (org at index 1)
while GitHub Enterprise URLs have /api/v3/repos/{org}/{repo}/... (org at index 3).
Previously only Enterprise format was handled, causing token lookup to fail
for public github.com repositories.
There was a problem hiding this comment.
Due to the move of functions to github_util.py, I think there are now quite some unused imports:
import requests
import requests.adapters
import urllib3.util.retry
import secret_mgmt.github
There was a problem hiding this comment.
Fixed in efedbc4. Removed requests, requests.adapters, urllib3.util.retry and secret_mgmt.github from ghas.py — none of them are used directly anymore since the HTTP helpers were moved to github_util.
| @@ -52,37 +53,20 @@ def find_token_for_repo_url( | |||
There was a problem hiding this comment.
We don't have an established public Python API of this ghas module, so you might just drop these extra wrappers.
There was a problem hiding this comment.
Fixed in 3d21b35. Dropped all four wrapper functions (find_token_for_repo_url, find_token_for_api_url, github_api_request, github_api_request_paginated) from ghas.py and updated the four internal call sites to reference github_util directly. Also removed the now-unused functools import.
| value: 0 | ||
| rescoring: manual | ||
| allowed_processing_time: ~ | ||
| - id: BLOCKER |
There was a problem hiding this comment.
nit: These severities originated from vulnerabilities, but you are free to choose a more expressive name for CodeQL here, e.g. not-enabled or something like that :-)
There was a problem hiding this comment.
Fixed in 5572baf. Replaced BLOCKER/NONE with not-enabled/accepted — names that describe the CodeQL state rather than a generic vulnerability severity level.
| ) | ||
| return | ||
|
|
||
| if not codeql_config.languages: |
There was a problem hiding this comment.
This does not match the behaviour described in the docstring:
If empty or omitted, all repositories are checked regardless of language.
There was a problem hiding this comment.
Fixed in 9b0d2d8. Updated the languages docstring in CodeqlConfig to reflect that an empty list causes all artefacts to be skipped, not checked unconditionally.
| default branch (from code-scanning/analyses environment field). | ||
| """ | ||
| access = source_node.source.access | ||
| if not isinstance(access, ocm.GithubAccess): |
There was a problem hiding this comment.
To be consistent and already exit more early, you should rather move this check in the is_supported function and define a list of supported_access_types.
There was a problem hiding this comment.
Fixed in 8ed7d79. Added supported_access_types = (ocm.GithubAccess,) and an access parameter to CodeqlConfig.is_supported. The isinstance guard is now handled there alongside the existing artefact_kind check and removed from fetch_repo_info.
|
|
||
| org, repo, api_base = coords | ||
|
|
||
| if not access.ref: |
There was a problem hiding this comment.
Since the ref is not a required property in the spec, I would suggest to rather fallback to the default branch in that case instead of not reporting any findings. Wdyt?
There was a problem hiding this comment.
Fixed in ccf557a. When access.ref is None, the extension now fetches default_branch from GET /repos/{org}/odg-core (a call we already make for other data) and uses that as the ref. Only falls back to skipping if the default branch cannot be determined.
| continue | ||
| env = analysis.get('environment', {}) | ||
| if isinstance(env, str): | ||
| import json as _json |
There was a problem hiding this comment.
Since this is (according to the response schema) the expected case, I don't see a benefit of this late import and would rather suggest to move it as a module level import instead. Also, I think there is no conflict when defining no alias for it.
There was a problem hiding this comment.
Fixed in 3daea00. Moved import json to module level and removed the _json alias.
| try: | ||
| env = _json.loads(env) | ||
| except Exception: | ||
| continue |
There was a problem hiding this comment.
I think we should at least log the exception in this case, wdyt?
There was a problem hiding this comment.
Fixed in 5673ae9. Added logger.warning in the except block with the raw env value and the exception message so malformed analysis environment payloads are visible in the logs.
| ), | ||
| ) | ||
|
|
||
| delivery_service_client.update_metadata(data=all_metadata) |
There was a problem hiding this comment.
What is not yet covered by the extension is the case when a finding has been created because of missing CodeQL scans, but at a later point in time the scans were enabled. In that case, we would either have to delete the finding from the database again (not ideal because the audit trail is not given) or create an automatic rescoring for it.
There was a problem hiding this comment.
Fixed in f9fab7d. On each scan, the extension now queries existing CODEQL_FINDING entries for the artefact from the DB. Any finding whose key is absent from the newly generated findings (i.e. CodeQL is now enabled for that language) gets an automatic rescoring to accepted with a comment. Pattern follows the GHAS extension stale-finding handling.
There was a problem hiding this comment.
Please also add the respective documentation (for the extension and the new finding type) to the charts/bootstrapping/values.documentation.yaml file :-)
Also, what is still missing as well is the Helm chart for the new extension, see documentation.
requests, requests.adapters, urllib3.util.retry and secret_mgmt.github are no longer used directly in ghas.py since the HTTP helper functions were moved to github_util. Added tests to verify the cleanup.
akiioto
force-pushed
the
feat/sast-codeql-check
branch
from
0852afc to
efedbc4
Compare
Since ghas.py has no established public Python API, there is no need to keep wrapper functions that solely delegate to github_util. Removed find_token_for_repo_url, find_token_for_api_url, github_api_request and github_api_request_paginated wrappers; internal call sites now reference github_util directly. Also removed the now-unused functools import.
Replace generic BLOCKER/NONE with CodeQL-specific ids: - not-enabled: CodeQL is not scanning for the configured language - accepted: finding has been manually acknowledged Addresses review feedback that BLOCKER/NONE originate from vulnerability findings and are not expressive for a CodeQL enablement check.
…iour The docstring incorrectly stated that an empty languages list would check all repositories. In practice, an empty list causes the extension to skip all artefacts with a warning. Updated the docstring to reflect this.
Added supported_access_types = (ocm.GithubAccess,) to CodeqlConfig.is_supported and an access parameter so the check is done consistently alongside the artefact_kind check. Removed the isinstance guard from fetch_repo_info.
…g list The generator returned by github_api_request_paginated is only consumed once, so wrapping it in list() is unnecessary. Iterating directly avoids loading all pages into memory before processing.
Per the OCM spec, ref is not a required field on GithubAccess. When ref is absent, the extension now fetches the repository default_branch from the GitHub API and uses that instead of skipping the artefact. Only skips when the default branch cannot be determined.
The import json statement was inside fetch_repo_info, causing it to be re-evaluated on every call. Moved to module level and removed the _json alias.
Silent except made it impossible to diagnose malformed analysis environment payloads. Now logs a warning with the raw value and the exception message before continuing to the next analysis.
When a finding exists in the DB for a language that is now actively scanned by CodeQL, the extension creates an automatic rescoring with severity 'accepted' and a comment explaining why. This ensures stale findings are surfaced rather than silently remaining open. Pattern follows the existing GHAS extension stale-finding handling.
- charts/extensions/charts/codeql/: new Deployment-based Helm chart following the SAST pattern (scaled by backlog-controller, mounts github, github-app, extensions-cfg, findings-cfg, ocm-repo-mappings) - charts/extensions/Chart.yaml: register codeql as subchart - charts/bootstrapping/values.documentation.yaml: document extensions_cfg.codeql (enabled, delivery_service_url, interval, languages, on_unsupported) and add finding/codeql to finding type option lists
akiioto
force-pushed
the
feat/sast-codeql-check
branch
from
0d0662d to
129305b
Compare
2 participants
Summary
codeqlextension that checks whether GitHub Advanced Security CodeQL is enabled for each source artefact referenced by an OCM componentsastextensionChanges
src/codeql.pytool_name=CodeQL), emitsCodeqlFinding(NOT_ENABLED)when CodeQL is absentsrc/odg/model.pyDatatype.CODEQL_FINDING,Datasource.CODEQL,CodeqlStatus,CodeqlFinding,RescoreCodeqlFindingsrc/odg/extensions_cfg.pyServices.CODEQLandCodeqlConfig(BacklogItemMixins, same asSASTConfig)src/odg/extensions_cfg.yamlcodeql: enabled: Falsedefaultsrc/odg/findings_cfg.yamlfinding/codeqlblock with BLOCKER categorisation; external (github.com) components auto-rescored to NONEsrc/odg/findings.pyCodeqlFindingSelectorforcategorise_finding()matching oncodeql_statussrc/artefact_enumerator.pyCODEQLservice so backlog items are created for source artefactsDesign decisions
ghas.github_api_request()— same GitHub API authentication mechanism as the existing GHAS extensionrepo_urlstored in finding — allows direct navigation to the repository from the dashboard/issue viewgithub.comcomponents exempt — external open-source components are not subject to SAP product standardsPrerequisites for full E2E test
github.tools.sap) needssecurity_events: readpermission addedTest plan
codeql.pyloads and starts polling backlog itemsartefact-enumeratorcreatescodeqlbacklog items for source artefactssecurity_events: readpermission is granted to GitHub AppCodeqlFinding(NOT_ENABLED)is stored in DB for repos without CodeQLgithub.comcomponents are auto-rescored to NONE