Keep a green run green when git cannot list updated files

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))

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 `<WORKSPACE>/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   |

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()

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