Fix betterleaks PR scan crash on Azure Pipelines, and gather CI platform logic in ci_providers - #8780
Merged
Merged
Conversation
REPOSITORY_BETTERLEAKS_PR_COMMITS_SCAN resolved the Azure Pipelines target branch with a single unguarded GitPython lookup on origin/<branch>. The default Azure checkout is shallow and creates no remote tracking ref, so gitdb.exc.BadName escaped BetterleaksLinter.__init__ and aborted the whole MegaLinter run before any linter started. The GitHub event payload lookup had the same flaw, and GitLab/unknown platforms failed silently. - New megalinter/ci_providers/ package (mirrors api_providers): CiProvider base class, CiProviderAzurePipelines, CiProviderGithubActions and CiProviderGitlab, exposing get_pr_commit_shas() and a platform specific get_pr_commit_shas_hint(). get_pr_ci_provider() falls back to the neutral base provider, so callers never handle a missing provider - Azure target branch is now searched across several ref spellings (origin/<branch>, refs/remotes/origin/<branch>, raw value, bare name) - Every lookup degrades to None with an actionable warning instead of raising, and betterleaks scans the whole repository - BetterleaksLinter keeps only the orchestration - Restore the Pull Request scan setup documentation lost when gitleaks was replaced by betterleaks: per platform checkout depth, Azure Pipelines variables to forward to the container, how to compute the SHAs manually - Add ci_providers_test.py, covering the crash outside Docker Fixes #8732 Fixes #8731
nvuillam
requested review from
Kurt-von-Laven,
bdovaz and
echoix
as code owners
August 22, 2026 21:00
Contributor
✅
|
The CI/CD platform specific logic was spread across utils, utils_reporter, MegaLinter and the reporters. Concentrate it in megalinter/ci_providers/, which now models the platform itself rather than only the Pull Request commit range. - jenkins_ci_vars moves from reporters/ to ci_providers/CiProviderJenkins: it was never a reporter, it is called from MegaLinter.__init__ - New CiProviderBitbucket, and every provider gains is_current(), so ci_providers.get_ci_provider() resolves the platform running the build - CiProvider gains the shared platform surface: get_repo_name(), get_branch_name(), get_job_url(), log_section_start/end(), set_output(), publish_job_summary() and markdown_supports_html_details - utils.get_git_context_info() delegates to the provider instead of its four-platform if/elif chain, keeping the git fallbacks and the GITHUB_JOB_URL / CI_JOB_URL overrides - utils_reporter.log_section_start/end delegate too, moving the GitLab section key sanitizing to CiProviderGitlab - MegaLinter.check_results uses set_output(), MarkdownSummaryReporter uses publish_job_summary() and stops reading os.environ directly - Deduplicates the GitHub run URL (built in 3 places), the Bitbucket step URL (2 places) and the Azure BUILD_BUILDID/BUILD_BUILD_ID fallback Fixes two detection gaps found on the way: is_ci() and is_pr() ignored Bitbucket Pipelines, so the Bitbucket branch of log_section_start was unreachable and the Pull Request optimizations of CheckovLinter and BetterleaksLinter never engaged there. Adds is_bitbucket_pr(). Verified the refactored console folding is byte-identical to the previous implementation on GitLab, and the repo/branch/job URL context is unchanged on GitHub, Azure, Bitbucket and outside any CI.
nvuillam
changed the title
The four comment reporters and GithubStatusReporter each read the platform environment variables themselves, duplicating url construction and auth handling. They now ask their provider for the platform identity and keep only the comment transport and rendering. - CiProviderAzurePipelines gains get_team_project(), get_artifacts_url(), get_pr_number(), get_auth_token(), get_api_headers(), build_git_api_url() and get_repository_id(), which owns the SYSTEM_PULLREQUEST_SOURCEREPO URI parsing, the API lookup and the BUILD_REPOSITORY_ID fallback - CiProviderGithubActions gains get_repo_slug(), get_commit_sha(), get_api_url(), get_pr_number() and the two token accessors - CiProviderGitlab gains get_project_id(), get_api_auth_options() and the merge request iid resolution, including the CI_OPEN_MERGE_REQUESTS parsing and the CI_MERGE_REQUEST_IID retry - CiProviderBitbucket gains get_repo_slug(), get_pr_number() and get_api_headers() Each reporter instantiates its own platform provider directly instead of calling get_ci_provider(). Under Jenkins the running platform is Jenkins, which maps its variables onto the other platforms', so resolving through the factory would have disabled every comment reporter there. GitHub keeps get_auth_token() (GITHUB_TOKEN) and get_user_auth_token() (PAT) separate on purpose: the comment reporter prefers PAT so the comment is attributed to the user, while a commit status needs the statuses:write scope that the documented fine-grained PAT does not carry. Unifying them would have broken status reporting for those users. Verified the Azure REST urls and the Basic auth header are byte-identical to the previous implementation.
Add hexsha (GitPython commit attribute) to the cspell dictionary, and assert the Azure Basic auth header by decoding it instead of hardcoding the base64 literal, whose OnRvaw fragment cspell flagged.
The suite runs inside GitHub Actions, where GITHUB_ACTIONS, GITHUB_REPOSITORY and GITHUB_RUN_ID are really set, so get_ci_provider() returned the GitHub provider and GITHUB_REPOSITORY leaked into a test expecting no repository. - A shared CiProviderTestCase clears every platform variable from the request configuration before each test - The factory tests activate exactly one provider by patching is_current / is_pr_context on all of them, instead of mocking a single utils detector: the detectors read the global configuration, which the request-level cleanup can not reach Verified green with the environment simulating each of GitHub Actions, Azure Pipelines, GitLab, Bitbucket, Jenkins, and no CI at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8732, #8731.
The bug
With
REPOSITORY_BETTERLEAKS_PR_COMMITS_SCAN: trueon Azure Pipelines, MegaLinter crashes before running a single linter:The target branch SHA was resolved with a single unguarded GitPython lookup on
origin/<branch>. The default Azure Pipelines checkout is shallow and creates no remote-tracking ref, soBadNamepropagates out ofBetterleaksLinter.__init__— called fromlinter_factory.build_descriptor_linters— and aborts the whole run, not just betterleaks.The GitHub path had the same flaw (
GITHUB_EVENT_PATHmissing, unreadable, or not a PR payload), and GitLab / unrecognized platforms returned no SHAs with no feedback at all.The fix
The target branch is now searched across several ref spellings (
origin/<branch>,refs/remotes/origin/<branch>, the raw value, the bare name), so a branch present only locally is found too. Every lookup degrades toNonewith an actionable warning instead of raising, and betterleaks scans the whole repository — a superset, so nothing goes undetected:Documentation (#8731): restores the sections dropped from the descriptor when gitleaks was replaced by betterleaks in v10 — per-platform checkout depth (
fetch-depth/fetchDepth/GIT_DEPTH), the Azure Pipelinesdocker run -e SYSTEM_PULLREQUEST_*passthrough, thegit rev-list/git rev-parsesnippets, and the GitLab merged-results-pipelines caveat.New
megalinter/ci_providers/packageThe SHA lookups were CI-platform concerns living on a linter. Rather than move them to yet another place, this PR gathers the platform knowledge that was spread across
utils,utils_reporter,MegaLinterand the reporters, following the existingapi_providerspattern.CiProviderCiProviderAzurePipelinesSYSTEM_PULLREQUEST_SOURCECOMMITID, target resolved from the branch nameCiProviderGithubActionsCiProviderGitlabCiProviderBitbucketCiProviderJenkinsreporters/jenkins_ci_vars.pyjenkins_ci_vars.pywas never a reporter — it is called fromMegalinter.__init__and maps Jenkins variables onto the other platforms' native ones. It now sits with its peers.What each provider exposes:
is_current(),is_pr_context(),get_pr_commit_shas()+get_pr_commit_shas_hint(),get_repo_name(),get_branch_name(),get_job_url(),log_section_start/end(),set_output(),publish_job_summary(),markdown_supports_html_details.Consolidated as a result:
utils.get_git_context_info()— a 95-line four-platformif/elifchain — delegates to the provider, keeping the git fallbacks and theGITHUB_JOB_URL/CI_JOB_URLoverridesutils_reporter.log_section_start/end()delegate too; the GitLab section-key sanitizing moves toCiProviderGitlabMegaLinter.check_results()usesset_output();MarkdownSummaryReporterusespublish_job_summary()and stops readingos.environdirectlyBUILD_BUILDID/BUILD_BUILD_IDfallback (2)Two detection gaps fixed on the way
utils.is_ci()andutils.is_pr()both omitted Bitbucket Pipelines. Consequences:is_bitbucket()branch oflog_section_startwas unreachable (harmless — it returned the same value as the fallthrough)CheckovLinterandBetterleaksLinternever engaged on BitbucketAdded
utils.is_bitbucket_pr()and both missing cases.The reporters now read their context from the providers
The four comment reporters and
GithubStatusReportereach read the platform variables themselves, duplicating url construction and auth handling. They now ask their provider for the platform identity and keep only the comment transport and rendering — 238 lines lighter.CiProviderAzurePipelinesbuild_git_api_url(),get_api_headers(),get_artifacts_url(), andget_repository_id()— theSYSTEM_PULLREQUEST_SOURCEREPOSITORYURIparsing, API lookup andBUILD_REPOSITORY_IDfallbackCiProviderGithubActionsget_repo_slug(),get_commit_sha(),get_api_url(),get_pr_number()(therefs/pull/N/mergeregex)CiProviderGitlabget_project_id(),get_api_auth_options(), merge request iid resolution incl.CI_OPEN_MERGE_REQUESTSparsing and theCI_MERGE_REQUEST_IIDretryCiProviderBitbucketget_repo_slug(),get_pr_number(),get_api_headers()One trap worth knowing about. Each reporter instantiates its own platform provider directly rather than calling
get_ci_provider(). Under Jenkins the running platform is Jenkins, which maps its variables onto the other platforms' — so resolving through the factory (or anisinstancecheck inmanage_activation) would have silently disabled every comment reporter on Jenkins. Reporter activation is deliberately left reading the mapped variables.Deliberately not changed
GithubStatusReporterusesGITHUB_TOKENwhileGithubCommentReporterprefersPAT. This looks like an inconsistency to unify behind oneget_auth_token(), but it is not: MegaLinter documents fine-grained PATs scoped toContents: Read/Writeonly, which lackstatuses: write. PreferringPATfor commit statuses would give those users 403s. The providers therefore exposeget_auth_token()(runner token) andget_user_auth_token()(PAT) as two distinct accessors, and each caller picks the right one.Also left alone, being pure deduplication with no user benefit and real risk: the four
get_comment_marker()implementations. Their output is persisted inside existing PR comments, so any drift would orphan them and post duplicates instead of updating.Testing
megalinter/tests/test_megalinter/ci_providers_test.py— 56 tests, no Docker needed, including a direct regression test for the #8732 traceback:originremote, from a bare branch name, unresolvable ref, non-git workspace, missing branch variable, full shallow-checkout scenario,BUILD_BUILD_IDfallback, URL-encoded project namepull_request,set_output/publish_job_summaryincluding an unwritable pathget_ci_provider()andget_pr_ci_provider()Beyond unit tests, the refactor was checked for behavioral equivalence against the previous implementations: the GitLab console folding output is byte-identical, the Azure REST urls and Basic auth header are byte-identical, and the repo/branch/job URL context is unchanged on GitHub, Azure, Bitbucket and outside any CI.
black,isort(withTEMPLATES/.isort.cfg) andflake8clean. Descriptor validated againstmegalinter-descriptor.jsonschema.json.linter_textfeeds documentation generation only, so no build artifact changes.