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 42 commits into
Conversation
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds CodeQL finding contracts, configuration, GitHub repository scanning, stale-finding rescoring, backlog integration, shared GitHub API helpers, and a conditional Helm deployment. ChangesCodeQL contracts and configuration
Shared GitHub API access
CodeQL metadata scanning
CodeQL extension deployment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ArtefactEnumerator
participant CodeQLWorker
participant GitHubAPI
participant DeliveryService
ArtefactEnumerator->>CodeQLWorker: create CodeQL backlog item
CodeQLWorker->>GitHubAPI: inspect repository and Code Scanning analyses
GitHubAPI-->>CodeQLWorker: repository and active-language data
CodeQLWorker->>DeliveryService: write findings and stale-finding rescoring metadata
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
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! :-)
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.
0d0662d to
129305b
Compare
8R0WNI3
left a comment
8R0WNI3
left a comment
There was a problem hiding this comment.
For testing I ran the extension for our ocm.software/open-delivery-gear/ui component (see GitHub repository). The extension yielded the following information:
repo_languages={'javascript', 'shell', 'dockerfile', 'go template', 'html'}
active_code_ql_languages={'actions', 'python', 'javascript-typescript'}Becuase javascript does not match javascript-typescript, the extension created a finding. How are we supposed to deal with such cases?
Fixed in 50c1c52. CodeQL uses different language identifiers than GitHub's |
0e22b4d to
3a75bd6
Compare
57f10c3 to
27e3a74
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
charts/extensions/charts/codeql/templates/codeql.yaml (2)
42-43: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHarden securityContext and mark ConfigMap mounts as read-only.
The container
securityContextonly setsallowPrivilegeEscalation: false. AddingrunAsNonRoot: true,readOnlyRootFilesystem: true, and dropping all capabilities further reduces privilege. Additionally, theextensions-cfgandfindings-cfgConfigMap mounts should setreadOnly: truefor consistency with theocm-repo-mappingsmount and to prevent accidental writes.🛡️ Suggested security hardening
securityContext: allowPrivilegeEscalation: false + runAsNonRoot: true + readOnlyRootFilesystem: true + capabilities: + drop: + - ALLvolumeMounts: - name: github mountPath: /secrets/github - name: github-app mountPath: /secrets/github-app - name: extensions-cfg mountPath: /extensions_cfg + readOnly: true - name: findings-cfg mountPath: /findings_cfg + readOnly: true - name: ocm-repo-mappings mountPath: /ocm_repo_mappings readOnly: trueAlso applies to: 60-63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/extensions/charts/codeql/templates/codeql.yaml` around lines 42 - 43, Harden the container securityContext by adding runAsNonRoot, readOnlyRootFilesystem, and dropping all capabilities alongside allowPrivilegeEscalation in the chart template. Update the extensions-cfg and findings-cfg ConfigMap volume mounts to set readOnly: true, matching the existing ocm-repo-mappings mount.
110-111: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTighten NetworkPolicy egress rules.
The
egress: - {}rule allows all outbound traffic to any destination and port. CodeQL only needs HTTPS access to GitHub API endpoints (and DNS resolution). Consider restricting egress to specific ports and destinations to reduce the attack surface.🔒️ Suggested egress restriction
egress: - - {} # Allows all egress traffic to any destination and port + - to: [] + ports: + - protocol: UDP + port: 53 # DNS + - protocol: TCP + port: 443 # HTTPS to GitHub API🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/extensions/charts/codeql/templates/codeql.yaml` around lines 110 - 111, Update the NetworkPolicy egress configuration in the CodeQL chart to replace the unrestricted egress entry with rules allowing only DNS resolution and HTTPS traffic to the required GitHub API destinations. Preserve any necessary namespace or pod selectors, explicitly restrict ports to DNS and 443, and remove the `- {}` allow-all rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codeql.py`:
- Line 138: Change the creation_timestamp default in the affected scan-related
definition to be generated per invocation rather than evaluated at import time,
using the callable/default-factory mechanism supported by its declaration.
Ensure scan continues to derive each finding’s ARTEFACT_SCAN_INFO and
discovery_date from the current invocation timestamp when creation_timestamp is
not explicitly supplied.
- Around line 285-323: Update scan() and the fetch_repo_info() result handling
so failed GitHub API lookups remain distinguishable from a successful lookup
with no languages. Only execute the stale-rescoring loop that creates
RescoreCodeqlFinding metadata when repository information was fetched
successfully; otherwise preserve existing findings without rescoring.
In `@src/github_util.py`:
- Around line 90-112: Update find_token_for_repo_url and find_token_for_api_url
so GitHub App installation tokens are not cached indefinitely; either remove
functools.cache from both functions or replace it with expiry-aware caching that
refreshes tokens before their one-hour expiration. Ensure long-lived workers
obtain a fresh token after expiration while preserving the existing token lookup
behavior.
---
Nitpick comments:
In `@charts/extensions/charts/codeql/templates/codeql.yaml`:
- Around line 42-43: Harden the container securityContext by adding
runAsNonRoot, readOnlyRootFilesystem, and dropping all capabilities alongside
allowPrivilegeEscalation in the chart template. Update the extensions-cfg and
findings-cfg ConfigMap volume mounts to set readOnly: true, matching the
existing ocm-repo-mappings mount.
- Around line 110-111: Update the NetworkPolicy egress configuration in the
CodeQL chart to replace the unrestricted egress entry with rules allowing only
DNS resolution and HTTPS traffic to the required GitHub API destinations.
Preserve any necessary namespace or pod selectors, explicitly restrict ports to
DNS and 443, and remove the `- {}` allow-all rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 008977fa-3616-4cf7-9fc9-fec17b231299
📒 Files selected for processing (15)
charts/bootstrapping/values.documentation.yamlcharts/extensions/Chart.yamlcharts/extensions/charts/codeql/Chart.yamlcharts/extensions/charts/codeql/templates/codeql.yamlcharts/extensions/values.yamlsrc/artefact_enumerator.pysrc/codeql.pysrc/ghas.pysrc/github_util.pysrc/odg/extensions_cfg.pysrc/odg/extensions_cfg.yamlsrc/odg/findings.pysrc/odg/findings_cfg.yamlsrc/odg/model.pysrc/odg/profiles.yaml
27e3a74 to
31abf58
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
charts/extensions/charts/codeql/templates/codeql.yaml (1)
55-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMount ConfigMap volumes as read-only.
Consider explicitly setting
readOnly: truefor theextensions-cfgandfindings-cfgmounts to match theocm-repo-mappingsmount. This improves consistency and aligns with the best practice of mounting configuration files immutably.♻️ Proposed refactor
volumeMounts: - name: github mountPath: /secrets/github - name: github-app mountPath: /secrets/github-app - name: extensions-cfg mountPath: /extensions_cfg + readOnly: true - name: findings-cfg mountPath: /findings_cfg + readOnly: true - name: ocm-repo-mappings mountPath: /ocm_repo_mappings readOnly: true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/extensions/charts/codeql/templates/codeql.yaml` around lines 55 - 66, Add readOnly: true to the extensions-cfg and findings-cfg volume mounts in the volumeMounts configuration, matching the existing ocm-repo-mappings mount while leaving the other mounts unchanged.src/codeql.py (1)
212-233: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueDerive
new_keysfrom the sameCodeqlFinding.keypath. The current comprehension repeats the language filter and string format, so the two paths can drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codeql.py` around lines 212 - 233, Update the new_keys construction alongside the existing CodeQL finding generation loop to derive keys from the same CodeqlFinding.key path, reusing the yielded or constructed finding data instead of duplicating language filters and string formatting. Keep the existing inclusion behavior for repository languages that are not active.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codeql.py`:
- Around line 33-36: Update _CODEQL_LANGUAGE_ALIASES to add a java-kotlin entry
mapping to both java and kotlin, so analyses reported under java-kotlin match
repositories configured for either language.
In `@src/odg/extensions_cfg.py`:
- Around line 948-952: Rename the `access` parameter in `is_supported` to
`access_type` to match `BacklogItemMixins` and preserve keyword-based
polymorphic calls. Update callers in `src/codeql.py` to pass `access_type` and
use the enum’s `type` as required by the base-class contract.
---
Nitpick comments:
In `@charts/extensions/charts/codeql/templates/codeql.yaml`:
- Around line 55-66: Add readOnly: true to the extensions-cfg and findings-cfg
volume mounts in the volumeMounts configuration, matching the existing
ocm-repo-mappings mount while leaving the other mounts unchanged.
In `@src/codeql.py`:
- Around line 212-233: Update the new_keys construction alongside the existing
CodeQL finding generation loop to derive keys from the same CodeqlFinding.key
path, reusing the yielded or constructed finding data instead of duplicating
language filters and string formatting. Keep the existing inclusion behavior for
repository languages that are not active.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4d133e4-37a5-482d-833a-5326f1439950
📒 Files selected for processing (15)
charts/bootstrapping/values.documentation.yamlcharts/extensions/Chart.yamlcharts/extensions/charts/codeql/Chart.yamlcharts/extensions/charts/codeql/templates/codeql.yamlcharts/extensions/values.yamlsrc/artefact_enumerator.pysrc/codeql.pysrc/ghas.pysrc/github_util.pysrc/odg/extensions_cfg.pysrc/odg/extensions_cfg.yamlsrc/odg/findings.pysrc/odg/findings_cfg.yamlsrc/odg/model.pysrc/odg/profiles.yaml
🚧 Files skipped from review as they are similar to previous changes (11)
- charts/extensions/charts/codeql/Chart.yaml
- charts/extensions/Chart.yaml
- src/odg/profiles.yaml
- charts/extensions/values.yaml
- src/odg/findings_cfg.yaml
- src/odg/extensions_cfg.yaml
- src/odg/model.py
- src/github_util.py
- charts/bootstrapping/values.documentation.yaml
- src/artefact_enumerator.py
- src/ghas.py
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 Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
Required for categorise_finding() to match on codeql_status values in the findings_cfg.yaml selector configuration. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
- 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 Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
languages: [Go, Python, Java] in extensions_cfg limits CodeQL checks to repos whose primary language matches. Empty list (default) checks all. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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.
Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
Addresses review comment: Services, Datatype, Datasource.datasource(), and ExtensionsConfiguration fields are now in alphabetical order. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
- 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 Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
- 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/
Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
…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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
…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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
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. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
- 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 Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
…uages CodeQL uses different language identifiers than GitHub's /languages API: - 'javascript-typescript' covers both 'javascript' and 'typescript' - 'c-cpp' covers both 'c' and 'c++' Without this mapping, a repo scanned by CodeQL for 'javascript-typescript' would incorrectly receive a finding when the config specifies 'javascript'. Added _CODEQL_LANGUAGE_ALIASES dict and switched from add() to update() with alias expansion when building active_languages. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
util.urlparse is still used in iter_secret_locations and scan. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
An empty languages list yields no findings — making it required forces operators to explicitly configure which languages to check. Updated the default extensions_cfg.yaml with example values. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
Eases local setup by including the new finding type in the default profile alongside the other finding types. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
…tion.yaml Languages must be specified using CodeQL identifiers (e.g. 'javascript-typescript', 'c-cpp') which differ from GitHub language names. Updated example and added link to CodeQL language documentation. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
…entifiers The extension accepts standard GitHub repository language names (e.g. 'javascript', 'c') and internally maps them to CodeQL identifiers via _CODEQL_LANGUAGE_ALIASES. Updated values.documentation.yaml and the CodeqlConfig docstring accordingly. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
- remove unused ocm import from codeql.py - break long log lines - rename ambiguous variable l -> lang - add missing blank line in findings.py - break long warning message in extensions_cfg.py Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
When github_api_request() returns None (e.g. 403, network error), fetch_repo_info() now returns api_success=False. iter_artefact_metadata skips the stale-rescoring loop in that case, preserving existing findings rather than incorrectly rescoring them to 'accepted'. Also moves the stale-rescoring logic from scan() into iter_artefact_metadata so it has direct access to api_success. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
- add java-kotlin -> {java, kotlin} to _CODEQL_LANGUAGE_ALIASES so
repositories analysed under the combined identifier do not receive
false-positive findings when java or kotlin is configured
- rename CodeqlConfig.is_supported parameter from access to access_type
and change supported_access_types to use ocm.AccessType.GITHUB instead
of isinstance check, aligning the signature with BacklogItemMixins and
fixing LSP violation
Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
48484a9 to
20bad69
Compare
github_api_request() returns either a dict or None — a non-dict non-None value cannot occur in practice. For defensive consistency, treat it the same as None rather than silently returning an empty language set with api_success=True. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/odg/extensions_cfg.py (1)
928-956: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit: Fix docstring syntax and use consistent
Nonechecks.A couple of minor polish items:
- The Sphinx docstring parameters for
delivery_service_urlandon_unsupportedare missing a trailing colon, which may cause rendering warnings or omissions in the generated documentation.- The
artefact_kindcheck uses truthiness (if artefact_kind:), while theaccess_typecheck explicitly usesis not Nonea few lines below. Usingis not Noneconsistently is safer and clearer.♻️ Proposed adjustments
- :param str delivery_service_url + :param str delivery_service_url: :param int interval: Time after which an artefact must be re-checked at latest. - :param WarningVerbosities on_unsupported + :param WarningVerbosities on_unsupported: Defines the handling if a backlog item should be processed which contains unsupported properties, e.g. an unsupported artefact kind. :param list[str] languages: Languages to check CodeQL coverage for, using GitHub repository language names (e.g. 'go', 'python', 'javascript'). The extension automatically handles CodeQL-specific identifiers (e.g. 'javascript-typescript' is treated as covering 'javascript' and 'typescript'). A finding is emitted for each configured language that is present in the repository but not actively scanned. """ service: Services = Services.CODEQL delivery_service_url: str interval: int = 60 * 60 * 24 # 24h on_unsupported: WarningVerbosities = WarningVerbosities.WARNING languages: list[str] def is_supported( self, artefact_kind: odg.model.ArtefactKind | None = None, access_type: ocm.AccessType | None = None, ) -> bool: supported_artefact_kinds = (odg.model.ArtefactKind.SOURCE,) supported_access_types = (ocm.AccessType.GITHUB,) - if artefact_kind and artefact_kind not in supported_artefact_kinds: + if artefact_kind is not None and artefact_kind not in supported_artefact_kinds:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/odg/extensions_cfg.py` around lines 928 - 956, Update the surrounding class docstring so the delivery_service_url and on_unsupported parameter entries use Sphinx’s trailing-colon syntax. In is_supported, replace the truthiness check for artefact_kind with an explicit is not None check, matching the existing access_type handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/odg/extensions_cfg.py`:
- Around line 928-956: Update the surrounding class docstring so the
delivery_service_url and on_unsupported parameter entries use Sphinx’s
trailing-colon syntax. In is_supported, replace the truthiness check for
artefact_kind with an explicit is not None check, matching the existing
access_type handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 21cc18ef-6d08-40b8-882f-771333b44a1b
📒 Files selected for processing (15)
charts/bootstrapping/values.documentation.yamlcharts/extensions/Chart.yamlcharts/extensions/charts/codeql/Chart.yamlcharts/extensions/charts/codeql/templates/codeql.yamlcharts/extensions/values.yamlsrc/artefact_enumerator.pysrc/codeql.pysrc/ghas.pysrc/github_util.pysrc/odg/extensions_cfg.pysrc/odg/extensions_cfg.yamlsrc/odg/findings.pysrc/odg/findings_cfg.yamlsrc/odg/model.pysrc/odg/profiles.yaml
🚧 Files skipped from review as they are similar to previous changes (12)
- charts/extensions/charts/codeql/Chart.yaml
- charts/extensions/Chart.yaml
- src/odg/extensions_cfg.yaml
- src/odg/findings_cfg.yaml
- src/odg/profiles.yaml
- src/artefact_enumerator.py
- src/odg/model.py
- charts/extensions/values.yaml
- charts/bootstrapping/values.documentation.yaml
- src/github_util.py
- src/codeql.py
- src/ghas.py
iter_artefact_metadata is only called from scan(), which always passes existing_findings. There is no reason to make it optional. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
Instead of duplicating the key format as a hardcoded f-string, collect keys from the actual CodeqlFinding objects as they are created. This removes the duplication with CodeqlFinding.key and ensures there is a single source of truth for the key definition. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
…t import time datetime.datetime.now(...) as a default argument is evaluated once when the module is loaded. Replace with None and add a per-call guard to ensure each invocation gets the current timestamp. Fixes discovery_date drift for long-lived workers that survive past midnight. Affects codeql.iter_artefact_metadata and three functions in sast.py which had the same pre-existing issue. Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
- add trailing colon to delivery_service_url and on_unsupported param entries in CodeqlConfig docstring (Sphinx syntax) - replace truthiness check with explicit is not None for artefact_kind, consistent with the existing access_type check in the same function Signed-off-by: Patryk Dobrowolski <patryk.dobrowolski@sap.com>
There was a problem hiding this comment.
Please also create documentation in ODG repo.
You can use the skills to generate reference documentation. However, I think a how-to is also helpful here.
The documentation follows diataxis principles
edit: nice, there is a draft PR already - thanks!
| # @param extensions_cfg.codeql worker which checks whether GitHub Advanced Security CodeQL is | ||
| # enabled for each OCM source artefact and creates findings for languages that are configured but | ||
| # not actively scanned |
There was a problem hiding this comment.
and creates findings for languages that are configured but not actively scanned
are there also other cases where the extension creates findings, e.g. if codeql is not enabled at all?
| interval: 86400 # 24h | ||
| # @param extensions_cfg.codeql.languages list of programming languages to check CodeQL coverage | ||
| # for. A finding is emitted for each language that is present in the repository but not actively | ||
| # scanned by CodeQL. Use the language names as returned by the GitHub repository languages API |
There was a problem hiding this comment.
please add a link to github-api docs to make it easier for consumers to check allowed values
There was a problem hiding this comment.
A finding is emitted for each language that is present in the repository but not actively scanned by CodeQL
why do we need an explicit user-managed list of relevant languages at all?
the repository scanning identifies used languages, does CodeQL require language specific configuration?
There was a problem hiding this comment.
The languages config list exists because CodeQL does not support all programming languages. For example, a repository may contain YAML, Dockerfile, or Shell - all of which would appear in the GitHub /languages response - but CodeQL cannot analyse them. Without an explicit list, the extension would have to either hardcode the set of CodeQL-supported languages.
Instead, operators configure the languages their organisation requires SAST coverage for once, e.g.:
languages:
- go
- python
- java
The extension then only creates findings for those languages. The /languages API is used purely as a filter to avoid false positives - if a configured language (e.g. go) is not actually present in the repository, no finding is created.
There was a problem hiding this comment.
from end-user perspective, wouldn't I by default rather want to have all CodeQL findings and optionally exclude languages?
I am afraid that with the current approach someone performs a one time assessment of used languages, configures them for the entire extension, and repository adapting new languages will not be covered.
wdyt?
There was a problem hiding this comment.
hmm, actually excluding those languages make a lot of sense. good catch, i'll adjust the code.
| logger.warning(f'Cannot parse org/repo from {repo_url=}') | ||
| return repo_url, set(), set(), False | ||
|
|
||
| org, repo, api_base = coords |
There was a problem hiding this comment.
nit: rather pass in coordinates directly (-> Dependency Injection), this makes the function more flexible and reusable
| creation_timestamp: datetime.datetime | None = None, | ||
| ) -> odg.model.ArtefactMetadata | None: | ||
| if creation_timestamp is None: | ||
| creation_timestamp = datetime.datetime.now(tz=datetime.timezone.utc) |
@zkdev There is already this PR open-component-model/open-delivery-gear#144 to create the respective documentation :-) |
3 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