diff --git a/CHANGELOG.md b/CHANGELOG.md index fcace41e409..24a20629369 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,9 @@ Note: Can be used with `oxsecurity/megalinter@beta` in your GitHub Action mega-l - **REPOSITORY_CHECKOV** and **REPOSITORY_BETTERLEAKS** only analyze the Pull Request changes when asked to - Set `BITBUCKET_PR_ID` in your pipeline (Bitbucket provides it on Pull Request builds) to benefit from it - Fixed JSON config schema for Betterleaks + - A run where all linters pass does not **exit with an error** anymore when MegaLinter can not list the files updated by the linters ([#8649](https://github.com/oxsecurity/megalinter/issues/8649)) + - Happens on a **read-only workspace** whose repository uses **git-lfs**: the required LFS filter has nowhere to write its temporary files, so the `git diff` used to detect updated files exits 128 + - MegaLinter now logs a **warning** naming the workspace and the failed command, reports no updated source file, and completes the run. The `UPDATED_SOURCES_REPORTER: false` workaround is not needed anymore - Reporters - Linters reporting in **SARIF** format no longer show **No output available** in Pull Request comments and summaries: the details section now names the SARIF report to open and links the **MegaLinter artifacts** ([#8730](https://github.com/oxsecurity/megalinter/issues/8730)) diff --git a/docs/reporters/UpdatedSourcesReporter.md b/docs/reporters/UpdatedSourcesReporter.md index 1b72ec6c1ab..955b8407560 100644 --- a/docs/reporters/UpdatedSourcesReporter.md +++ b/docs/reporters/UpdatedSourcesReporter.md @@ -34,6 +34,12 @@ If you aren't using GitHub Actions, you can: - use [Email Reporter](EmailReporter.md): Updated source folder will be in the email attachment reports zip - publish folder `/report/updated_sources` as artifact with your CI tool +### Empty updated sources folder + +The folder is empty when MegaLinter can not list the files updated by the linters. This happens on a **read-only workspace** whose repository uses **git-lfs**: the LFS filter has nowhere to write its temporary files, so the `git diff` used to detect updated files fails. + +MegaLinter logs a warning naming the workspace and the failed command, then completes the run. Mount `.git` as writable to get the updated sources back. + ## Configuration | Variable | Description | Default value | diff --git a/megalinter/tests/test_megalinter/utils_test.py b/megalinter/tests/test_megalinter/utils_test.py index bd12702bb52..7e152691dde 100644 --- a/megalinter/tests/test_megalinter/utils_test.py +++ b/megalinter/tests/test_megalinter/utils_test.py @@ -4,14 +4,23 @@ """ +import os import re +import subprocess +import tempfile import unittest import uuid import warnings +from unittest.mock import MagicMock, patch +import git from megalinter import config from megalinter.logger import fetch_betterleaks_regexes, sanitize_string -from megalinter.utils import fix_regex_pattern, get_excluded_directories +from megalinter.utils import ( + fix_regex_pattern, + get_excluded_directories, + list_updated_files, +) class utils_test(unittest.TestCase): @@ -70,3 +79,116 @@ def test_betterleaks_regexes_compile_without_warnings(self): warnings.simplefilter("error", FutureWarning) for pattern in regexes: re.compile(pattern) + + def test_list_updated_files_lists_modified_files(self): + # Nominal case: a file modified after the commit is reported. + # The git configuration is neutralized for the whole test, not only for + # the setup commands: list_updated_files runs git in this very process, + # so the caller's global config (a content filter bound by + # core.attributesFile, for instance) would otherwise apply to the diff + git_env = { + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_CONFIG_SYSTEM": os.devnull, + "GIT_AUTHOR_NAME": "megalinter-test", + "GIT_AUTHOR_EMAIL": "megalinter-test@invalid", + "GIT_COMMITTER_NAME": "megalinter-test", + "GIT_COMMITTER_EMAIL": "megalinter-test@invalid", + } + with patch.dict(os.environ, git_env), tempfile.TemporaryDirectory() as tmp_dir: + + def run(*args): + subprocess.run( + ["git", *args], + cwd=tmp_dir, + check=True, + timeout=60, + capture_output=True, + ) + + run("init", "-q") + with open(os.path.join(tmp_dir, "README.md"), "w", encoding="utf-8") as f: + f.write("initial\n") + run("add", "README.md") + run("commit", "-q", "-m", "init") + with open(os.path.join(tmp_dir, "README.md"), "w", encoding="utf-8") as f: + f.write("updated\n") + + self.assertEqual(list_updated_files(tmp_dir), ["README.md"]) + + def mock_repo_with_failing_diff(self, workspace, diff_err): + # git_dir must stay inside the workspace, otherwise list_updated_files + # returns on the working-copy-root check before reaching the diff + mock_repo = MagicMock() + mock_repo.git_dir = os.path.join(workspace, ".git") + mock_repo.index.diff.side_effect = diff_err + return patch("megalinter.utils.git.Repo", return_value=mock_repo) + + def test_list_updated_files_git_command_error_is_not_fatal(self): + # Regression test for issue #8649: on a read-only workspace, a required + # git content filter (git-lfs) can not write its temporary files, so the + # diff exits 128. Listing updated files is best effort: the failure must + # degrade to an empty list instead of crashing the whole MegaLinter run + # from UpdatedSourcesReporter.produce_report() + diff_err = git.GitCommandError( + ["git", "diff", "--abbrev=40", "--full-index", "-M", "--raw", "-z"], + 128, + # GitPython drains stderr before raising, so git's own message + # ("clean filter 'lfs' failed") never reaches the exception + b"", + ) + with tempfile.TemporaryDirectory() as tmp_dir: + with self.mock_repo_with_failing_diff(tmp_dir, diff_err): + with self.assertLogs(level="WARNING") as log: + updated_files = list_updated_files(tmp_dir) + + self.assertEqual(updated_files, []) + # The empty stderr makes the warning itself the only diagnosis material: + # it must carry the workspace and the failed command + message = "\n".join(log.output) + self.assertIn(tmp_dir, message) + self.assertIn("git diff", message) + # Disabling UPDATED_SOURCES_REPORTER is not a remedy: Linter.py calls + # list_updated_files whatever that variable is set to + self.assertNotIn("UPDATED_SOURCES_REPORTER", message) + + def test_list_updated_files_git_error_warns_once_per_workspace(self): + # Linter.update_files_lint_results calls list_updated_files once per + # linted file in `file` lint mode, so a persistent git failure must not + # repeat the same warning for every file of the run + diff_err = git.GitCommandError(["git", "diff"], 128, b"") + with tempfile.TemporaryDirectory() as tmp_dir: + with self.mock_repo_with_failing_diff(tmp_dir, diff_err): + with self.assertLogs(level="DEBUG") as log: + list_updated_files(tmp_dir) + list_updated_files(tmp_dir) + + warnings_logged = [line for line in log.output if line.startswith("WARNING")] + self.assertEqual(len(warnings_logged), 1) + + def test_list_updated_files_without_git_repository(self): + # Neither the workspace nor the default repo home is a git working copy + with tempfile.TemporaryDirectory() as tmp_dir: + with patch( + "megalinter.utils.git.Repo", + side_effect=git.InvalidGitRepositoryError(tmp_dir), + ): + with self.assertLogs(level="WARNING") as log: + updated_files = list_updated_files(tmp_dir) + + self.assertEqual(updated_files, []) + self.assertIn("Unable to find git repository", "\n".join(log.output)) + + def test_list_updated_files_when_workspace_is_not_the_working_copy_root(self): + # The resolved git dir lies outside the workspace (e.g. the workspace is + # inside a submodule), so the diff would not describe it + with tempfile.TemporaryDirectory() as tmp_dir: + mock_repo = MagicMock() + mock_repo.git_dir = os.path.join(tmp_dir, "..", ".git") + + with patch("megalinter.utils.git.Repo", return_value=mock_repo): + with self.assertLogs(level="WARNING") as log: + updated_files = list_updated_files(tmp_dir) + + self.assertEqual(updated_files, []) + self.assertIn("not a Git working copy root", "\n".join(log.output)) + mock_repo.index.diff.assert_not_called() diff --git a/megalinter/utils.py b/megalinter/utils.py index 489fc862307..80fc2b7a4c0 100644 --- a/megalinter/utils.py +++ b/megalinter/utils.py @@ -29,6 +29,10 @@ ANSI_ESCAPE_REGEX = re.compile(r"(\x9B|\x1B\[)[0-?]*[ -\/]*[@-~]") +# Workspaces whose "git diff" already failed: list_updated_files runs once per +# linted file, and a workspace that fails once keeps failing for the whole run +UPDATED_FILES_FAILED_WORKSPACES: set[str] = set() + # Replacements for temp folder in case of MegaLinter server LIST_OF_REPLACEMENTS_REGEX = [] if os.environ.get("MEGALINTER_SERVER", "") == "true": @@ -509,7 +513,27 @@ def list_updated_files(repo_home): "Your workspace is not a Git working copy root (e.g., the workspace is inside a submodule)" ) return [] - changed_files = [item.a_path for item in repo.index.diff(None)] + try: + changed_files = [item.a_path for item in repo.index.diff(None)] + except git.GitCommandError as git_err: + # Listing updated files is best effort: a git failure must not end a + # run whose linters all passed. Known case (issue #8649): on a + # read-only workspace, a required content filter such as git-lfs + # can not write its temporary files and the diff exits 128. + # GitPython drains stderr before raising, so git's own message is + # usually lost: name the workspace and the command instead. + message = ( + f"Unable to list updated files in {repo_home}: {str(git_err)}\n" + "If your workspace is mounted read-only, a required git filter " + "(e.g. git-lfs) can not write its temporary files: mount .git " + "as writable to let MegaLinter detect the files fixed by linters" + ) + if repo_home in UPDATED_FILES_FAILED_WORKSPACES: + logging.debug(message) + else: + UPDATED_FILES_FAILED_WORKSPACES.add(repo_home) + logging.warning(message) + return [] return changed_files