{"thread":{"id":"66313","subject":"[PATCH] blame: default to ignoring revisions in .git-blame-ignore-revs","startedAt":"2026-09-11T23:29:50Z","lastAt":"2026-10-05T21:12:13Z","messageCount":4,"participants":["Ravi Mistry via GitGitGadget","Ravi Mistry","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"552614","messageId":"pull.2224.git.1789169384240.gitgitgadget@gmail.com","threadId":"66313","inReplyTo":null,"subject":"[PATCH] blame: default to ignoring revisions in .git-blame-ignore-revs","fromName":"Ravi Mistry via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-11T23:29:44Z","receivedAt":"2026-09-11T23:29:50Z","isPatch":true,"body":"From: Ravi Mistry <rmistry@google.com>\n\ngit-blame(1) can ignore a list of commits specified via\n--ignore-revs-file or the blame.ignoreRevsFile configuration option.\nThis is useful for skipping uninteresting revisions such as tree-wide\nformatting changes, large-scale refactors, and code modernizations that\nwould otherwise obscure genuine historical authorship.\n\nWhen revision-ignoring was introduced in commit ae3f36dea1 (\"blame: add\nblame.ignoreRevsFile config option\", 2019-10-18), it intentionally\navoided adopting a default ignore file. At the time, the capability was\nnew and unproven, so avoiding unrequested filesystem I/O or unexpected\nattribution shifts took priority over a project-wide default.\nRequiring explicit opt-in per clone was therefore the prudent design.\n\nSince then, maintaining a .git-blame-ignore-revs file in the repository\nroot has become the de facto standard across the Git ecosystem, adopted\nby major hosting platforms (GitHub, GitLab, Gerrit) and prominent open\nsource projects (such as Chromium and LLVM). As a consequence,\ndevelopers frequently encounter a jarring mismatch: web interfaces\nseamlessly ignore formatting commits, but local git-blame(1) and\ngit-annotate(1) runs do not, unless each user manually configures\nblame.ignoreRevsFile for every local checkout.\n\nTeach git-blame(1) and git-annotate(1) to automatically check for a\nregular .git-blame-ignore-revs file at the root of the working tree when\noperating in a non-bare repository.\n\nTo ensure consistent precedence, security, and override semantics:\n- Loading the default file occurs before reading configuration and CLI\n  options, preserving user and repository config overrides.\n- Path resolution is anchored to repo_get_work_tree() and verified via\n  lstat() to ensure it is a regular file. Symbolic links, directories,\n  FIFOs, and sockets are safely skipped, preventing local information\n  disclosure and denial-of-service hangs.\n- In build_ignorelist(), ignore-rev files are parsed starting after the\n  last empty string entry. This ensures setting blame.ignoreRevsFile to\n  \"\" or passing --ignore-revs-file \"\" or --no-ignore-revs-file cleanly\n  discards the default file without attempting to open or parse it,\n  allowing users to bypass corrupted default files.\n- Duplicate parsing is prevented by tracking seen files in a strset.\n\nUpdate documentation in blame-options.adoc and config/blame.adoc, and\nadd comprehensive test coverage in t8013 for the default file lookup,\nsubdirectory invocations, CLI and config overrides, symlink rejection,\ncomments and whitespace handling, and bare repositories.\n\nBased-on-patch-by: Abhijeetsingh Meena <abhijeet040403@gmail.com>\nHelped-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\nHelped-by: Phillip Wood <phillip.wood@dunelm.org.uk>\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Ravi Mistry <rmistry@google.com>\n---\n    blame: default to ignoring revisions in .git-blame-ignore-revs\n    \n    This series restarts the conversation from the stalled attempt in PR\n    https://github.com/gitgitgadget/git/pull/1809 and addresses feature\n    request https://github.com/gitgitgadget/git/issues/1494\n    \n    See the previous discussion in\n    https://lore.kernel.org/git/pull.1809.v2.git.1728707867.gitgitgadget@gmail.com/\n    \n    In addition to resolving the questions raised by reviewers during the\n    previous discussion, this updated iteration introduces important\n    security hardening, bug fixes, and test improvements:\n    \n     1. Commit message rationale: The commit message now details why\n        revision ignoring originally avoided a default file when the feature\n        was first added:\n        https://github.com/git/git/commit/ae3f36dea16e51041c56ba9ed6b38380c8421816\n        It explains why ecosystem standardization across GitHub, GitLab,\n        Gerrit, Chromium, and LLVM makes a default file desirable today,\n        addressing the previous feedback from Phillip Wood. Trailers\n        acknowledge earlier patch authorship and reviewer contributions.\n    \n     2. Security and path resolution: Path lookup is anchored to\n        repo_get_work_tree(). Using lstat ensures that symbolic links,\n        directories, FIFOs, and sockets are skipped safely.\n    \n     3. Configuration and option override semantics: The build_ignorelist()\n        function begins processing after the last empty string entry.\n        Setting blame.ignoreRevsFile to an empty string or providing an\n        empty filename option on the command line allows users to bypass a\n        corrupted default file without encountering a fatal error.\n    \n     4. Documentation updates: Documentation clarifies that configured files\n        are processed after the default file. It explains how providing an\n        empty filename disables the default file and notes that bare\n        repositories do not search for the file. Documentation checks pass\n        without warning.\n    \n     5. Test harness improvements: The test suite removes the destructive\n        repository reset in t8013 and adds test coverage for comments,\n        whitespace, zero byte files, symlink rejection, corrupted file\n        overrides, and git annotate parity. Test lint checks pass without\n        error.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2224%2Frmistry%2Fblame-default-ignore-revs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2224/rmistry/blame-default-ignore-revs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2224\n\n Documentation/blame-options.adoc |   6 +-\n Documentation/config/blame.adoc  |   9 +-\n builtin/blame.c                  |  42 +++++++--\n t/t8013-blame-ignore-revs.sh     | 151 +++++++++++++++++++++++++++++++\n 4 files changed, 196 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc\nindex 1ae1222b6b..17f5734d61 100644\n--- a/Documentation/blame-options.adoc\n+++ b/Documentation/blame-options.adoc\n@@ -133,8 +133,10 @@ take effect.\n \tIgnore revisions listed in _<file>_, which must be in the same format as an\n \t`fsck.skipList`.  This option may be repeated, and these files will be\n \tprocessed after any files specified with the `blame.ignoreRevsFile` config\n-\toption.  An empty file name, `\"\"`, will clear the list of revs from\n-\tpreviously processed files.\n+\toption or the default `.git-blame-ignore-revs` file.  An empty file name,\n+\t`\"\"`, will clear the list of revs from previously processed files.\n+\t`--no-ignore-revs-file` will clear all previously specified ignore revs\n+\tfiles, including the default `.git-blame-ignore-revs` file.\n \n `--color-lines`::\n \tColor line annotations in the default format differently if they come from\ndiff --git a/Documentation/config/blame.adoc b/Documentation/config/blame.adoc\nindex 4d047c1790..6f9be627e1 100644\n--- a/Documentation/config/blame.adoc\n+++ b/Documentation/config/blame.adoc\n@@ -23,9 +23,12 @@ blame.showRoot::\n blame.ignoreRevsFile::\n \tIgnore revisions listed in the file, one unabbreviated object name per\n \tline, in linkgit:git-blame[1].  Whitespace and comments beginning with\n-\t`#` are ignored.  This option may be repeated multiple times.  Empty\n-\tfile names will reset the list of ignored revisions.  This option will\n-\tbe handled before the command line option `--ignore-revs-file`.\n+\t`#` are ignored.  If `.git-blame-ignore-revs` exists at the root of the\n+\tworking tree in a non-bare repository, it is used by default.  This option\n+\tmay be repeated multiple times; files specified here are processed after\n+\tthe default file.  An empty file name will reset the list of ignored\n+\trevisions from previously processed files and disable the default file.\n+\tThis option is handled before the command-line option `--ignore-revs-file`.\n \n blame.markUnblamableLines::\n \tMark lines that were changed by an ignored revision that we could not\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 48d5251c6d..0935d864ad 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -15,9 +15,11 @@\n #include \"hex.h\"\n #include \"commit.h\"\n #include \"diff.h\"\n+#include \"path.h\"\n #include \"revision.h\"\n #include \"quote.h\"\n #include \"string-list.h\"\n+#include \"strmap.h\"\n #include \"mailmap.h\"\n #include \"parse-options.h\"\n #include \"prio-queue.h\"\n@@ -768,8 +770,12 @@ static int git_blame_config(const char *var, const char *value,\n \t\tret = git_config_pathname(&str, var, value);\n \t\tif (ret)\n \t\t\treturn ret;\n-\t\tif (str)\n-\t\t\tstring_list_insert(&ignore_revs_file_list, str);\n+\t\tif (str) {\n+\t\t\tif (!*str)\n+\t\t\t\tstring_list_clear(&ignore_revs_file_list, 0);\n+\t\t\telse\n+\t\t\t\tstring_list_append(&ignore_revs_file_list, str);\n+\t\t}\n \t\tfree(str);\n \t\treturn 0;\n \t}\n@@ -936,16 +942,24 @@ static void build_ignorelist(struct blame_scoreboard *sb,\n {\n \tstruct string_list_item *i;\n \tstruct object_id oid;\n+\tstruct strset seen_files = STRSET_INIT;\n+\tsize_t start_idx = 0, idx;\n+\n+\tfor (idx = 0; idx < ignore_revs_file_list->nr; idx++) {\n+\t\tif (!*ignore_revs_file_list->items[idx].string)\n+\t\t\tstart_idx = idx + 1;\n+\t}\n \n \toidset_init(&sb->ignore_list, 0);\n-\tfor_each_string_list_item(i, ignore_revs_file_list) {\n-\t\tif (!strcmp(i->string, \"\"))\n-\t\t\toidset_clear(&sb->ignore_list);\n-\t\telse\n-\t\t\toidset_parse_file_carefully(&sb->ignore_list, i->string,\n+\tfor (idx = start_idx; idx < ignore_revs_file_list->nr; idx++) {\n+\t\tconst char *path = ignore_revs_file_list->items[idx].string;\n+\n+\t\tif (strset_add(&seen_files, path))\n+\t\t\toidset_parse_file_carefully(&sb->ignore_list, path,\n \t\t\t\t\t\t    the_repository->hash_algo,\n \t\t\t\t\t\t    peel_to_commit_oid, sb);\n \t}\n+\tstrset_clear(&seen_files);\n \tfor_each_string_list_item(i, ignore_rev_list) {\n \t\tif (repo_get_oid_committish(the_repository, i->string, &oid) ||\n \t\t    peel_to_commit_oid(&oid, sb))\n@@ -1020,6 +1034,20 @@ int cmd_blame(int argc,\n \tconst char *const *opt_usage = cmd_is_annotate ? annotate_opt_usage : blame_opt_usage;\n \n \tsetup_default_color_by_age();\n+\t{\n+\t\tconst char *work_tree = repo_get_work_tree(the_repository);\n+\n+\t\tif (work_tree) {\n+\t\t\tchar *default_file = mkpathdup(\"%s/%s\", work_tree,\n+\t\t\t\t\t\t       \".git-blame-ignore-revs\");\n+\t\t\tstruct stat st;\n+\n+\t\t\tif (!lstat(default_file, &st) && S_ISREG(st.st_mode) &&\n+\t\t\t    !access(default_file, R_OK))\n+\t\t\t\tstring_list_append(&ignore_revs_file_list, default_file);\n+\t\t\tfree(default_file);\n+\t\t}\n+\t}\n \trepo_config(the_repository, git_blame_config, &output_option);\n \trepo_init_revisions(the_repository, &revs, NULL);\n \trevs.date_mode = blame_date_mode;\ndiff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\nindex cace00ae8d..a43654a7ba 100755\n--- a/t/t8013-blame-ignore-revs.sh\n+++ b/t/t8013-blame-ignore-revs.sh\n@@ -327,4 +327,155 @@ test_expect_success ignore_merge '\n \ttest_cmp expect actual\n '\n \n+# Tests for default .git-blame-ignore-revs file\n+test_expect_success 'setup default .git-blame-ignore-revs' '\n+\tgit checkout -b default-file-branch &&\n+\ttest_write_lines line1 line2 >def-file &&\n+\tgit add def-file &&\n+\ttest_tick &&\n+\tgit commit -m \"default base\" &&\n+\tgit tag DEF_A &&\n+\n+\ttest_write_lines line1-modified line2-modified >def-file &&\n+\tgit add def-file &&\n+\ttest_tick &&\n+\tgit commit -m \"default mod\" &&\n+\tgit tag DEF_B &&\n+\n+\tgit rev-parse DEF_B >.git-blame-ignore-revs\n+'\n+\n+test_expect_success 'default .git-blame-ignore-revs is used by default' '\n+\tgit blame --line-porcelain def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_A >expect &&\n+\ttest_cmp expect actual &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p\" blame_raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'default .git-blame-ignore-revs respected by git annotate' '\n+\tgit rev-parse --short DEF_A >expect_sha &&\n+\tgit annotate def-file >actual &&\n+\ttest_grep \"^$(cat expect_sha)\" actual\n+'\n+\n+test_expect_success 'default .git-blame-ignore-revs works from subdirectory' '\n+\tmkdir -p sub &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit blame --line-porcelain ../def-file >blame_raw &&\n+\t\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\t\tgit rev-parse DEF_A >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'disable default .git-blame-ignore-revs with --no-ignore-revs-file' '\n+\tgit blame --line-porcelain --no-ignore-revs-file def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_B >expect &&\n+\ttest_cmp expect actual &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p\" blame_raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'disable default .git-blame-ignore-revs with --ignore-revs-file \"\"' '\n+\tgit blame --line-porcelain --ignore-revs-file \"\" def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_B >expect &&\n+\ttest_cmp expect actual &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p\" blame_raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'disable default .git-blame-ignore-revs with blame.ignoreRevsFile=\"\"' '\n+\ttest_config blame.ignoreRevsFile \"\" &&\n+\tgit blame --line-porcelain def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_B >expect &&\n+\ttest_cmp expect actual &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p\" blame_raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'default .git-blame-ignore-revs handles comments and whitespace' '\n+\ttest_when_finished \"git rev-parse DEF_B >.git-blame-ignore-revs\" &&\n+\t{\n+\t\techo \"# Leading comment\" &&\n+\t\techo \"\" &&\n+\t\techo \"   $(git rev-parse DEF_B)   \" &&\n+\t\techo \"# Trailing comment\"\n+\t} >.git-blame-ignore-revs &&\n+\tgit blame --line-porcelain def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_A >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'empty default .git-blame-ignore-revs is harmless' '\n+\ttest_when_finished \"git rev-parse DEF_B >.git-blame-ignore-revs\" &&\n+\t: >.git-blame-ignore-revs &&\n+\tgit blame def-file\n+'\n+\n+test_expect_success SYMLINKS 'symlink .git-blame-ignore-revs is ignored' '\n+\ttest_when_finished \"rm -f target_file .git-blame-ignore-revs && git rev-parse DEF_B >.git-blame-ignore-revs\" &&\n+\tgit rev-parse DEF_B >target_file &&\n+\tln -sf target_file .git-blame-ignore-revs &&\n+\tgit blame --line-porcelain def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_B >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'malformed default .git-blame-ignore-revs fails but can be bypassed' '\n+\ttest_when_finished \"git rev-parse DEF_B >.git-blame-ignore-revs\" &&\n+\techo \"invalid-oid-value\" >.git-blame-ignore-revs &&\n+\ttest_must_fail git blame def-file &&\n+\tgit blame --no-ignore-revs-file def-file &&\n+\tgit blame --ignore-revs-file \"\" def-file\n+'\n+\n+test_expect_success 'default .git-blame-ignore-revs deduplicated when also set in config' '\n+\ttest_config blame.ignoreRevsFile .git-blame-ignore-revs &&\n+\tgit blame --line-porcelain def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_A >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'default .git-blame-ignore-revs combined with config blame.ignoreRevsFile' '\n+\ttest_write_lines line1-modified line2-c >def-file &&\n+\tgit add def-file &&\n+\ttest_tick &&\n+\tgit commit -m C &&\n+\tgit tag DEF_C &&\n+\tgit rev-parse DEF_C >custom_ignore &&\n+\ttest_config blame.ignoreRevsFile custom_ignore &&\n+\tgit blame --line-porcelain def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_A >expect &&\n+\ttest_cmp expect actual &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_A >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'default .git-blame-ignore-revs ignored in bare repo' '\n+\tgit clone --bare . bare.git &&\n+\tgit -C bare.git blame --line-porcelain def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_C >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'blame works when .git-blame-ignore-revs does not exist' '\n+\trm -f .git-blame-ignore-revs &&\n+\tgit blame --line-porcelain def-file >blame_raw &&\n+\tsed -ne \"/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p\" blame_raw >actual &&\n+\tgit rev-parse DEF_B >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n\nbase-commit: fa7f9290efe2bd22dd736689597b474b93798e11\n-- \ngitgitgadget\n"},{"id":"553715","messageId":"20260930145952.1840998-1-rmistry@google.com","threadId":"66313","inReplyTo":"pull.2224.git.1789169384240.gitgitgadget@gmail.com","subject":"Re: [PATCH] blame: default to ignoring revisions in .git-blame-ignore-revs","fromName":"Ravi Mistry","fromEmail":"rmistry@google.com","sentAt":"2026-09-30T14:59:52Z","receivedAt":"2026-09-30T14:59:58Z","isPatch":true,"body":"Hi all,\n\nGentle ping on this patch. Please let me know if you have any feedback or questions on this approach, or if there are other reviewers I should loop in.\n\nTIA!\n"},{"id":"554188","messageId":"xmqqse2kma4o.fsf@gitster.g","threadId":"66313","inReplyTo":"pull.2224.git.1789169384240.gitgitgadget@gmail.com","subject":"Re: [PATCH] blame: default to ignoring revisions in .git-blame-ignore-revs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-05T15:37:27Z","receivedAt":"2026-10-05T15:37:27Z","isPatch":true,"body":"\"Ravi Mistry via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>  blame.ignoreRevsFile::\n>  \tIgnore revisions listed in the file, one unabbreviated object name per\n>  \tline, in linkgit:git-blame[1].  Whitespace and comments beginning with\n> -\t`#` are ignored.  This option may be repeated multiple times.  Empty\n> -\tfile names will reset the list of ignored revisions.  This option will\n> -\tbe handled before the command line option `--ignore-revs-file`.\n> +\t`#` are ignored.  If `.git-blame-ignore-revs` exists at the root of the\n> +\tworking tree in a non-bare repository, it is used by default.  This option\n> +\tmay be repeated multiple times; files specified here are processed after\n> +\tthe default file.  An empty file name will reset the list of ignored\n> +\trevisions from previously processed files and disable the default file.\n> +\tThis option is handled before the command-line option `--ignore-revs-file`.\n\nThe proposed log message explains that '.git-blame-ignore-revs' is\nused as the default for blame.ignoreRevsFile even when the user does\nnot ask to do so in order to match what hosting sites do, as it\nwould be confusing if the local repository behaved differently.\nWhile wanting consistency is reasonable, the description above does\nnot quite match that goal.  If an untracked '.git-blame-ignore-revs'\nfile exists at the root of the working tree, or if a tracked one has\nlocal changes relative to HEAD, the local repository behaves\ndifferently from hosting sites that operate on the\n'HEAD:.git-blame-ignore-revs' blob.  It may make more sense to say:\n\"If the 'HEAD:.git-blame-ignore-revs' blob exists, it is added as\nthe initial element in the list of ignore-revs files.  Other files\nlisted in the configuration are also used, but an empty element\nmakes all elements that appeared before in the list forgotten.\"\nThis rule should apply whether the repository is bare or not.\n\nThe proposed log message also talks about taking only a regular file\nand ignoring everything else for \"security\" [*], but there is\nanother important thing we need to worry about security-wise.\nSomebody has to audit the parser for these files (one unabbreviated\nobject name per line, ignoring whitespace and lines starting with\n'#') and ensure that the implementation is truly secure.\n\nThis is a new threat vector introduced by this change.  Without this\npatch, blame.ignoreRevsFile comes only from the configuration, which\ncannot point to an attacker-controlled file under our threat model.\nNow, however, the parser must read upstream-controlled content at a\nknown path, and it must be prepared to cope with attempts to use it\nas an attack vector.\n\n[Footnote]\n\n * By the way, \"we do not read anything from the working tree.\n   'HEAD:.git-blame-ignore-revs' is the only thing that is added\n   to the picture\" would make it unnecessary to lstat() and ignore\n   non-regular files.\n\n"},{"id":"554224","messageId":"20261005211213.1896012-1-rmistry@google.com","threadId":"66313","inReplyTo":"xmqqse2kma4o.fsf@gitster.g","subject":"Re: [PATCH] blame: default to ignoring revisions in .git-blame-ignore-revs","fromName":"Ravi Mistry","fromEmail":"rmistry@google.com","sentAt":"2026-10-05T21:12:13Z","receivedAt":"2026-10-05T21:12:13Z","isPatch":true,"body":"\"Junio C Hamano\" <gitster@pobox.com> writes:\n\n> While wanting consistency is reasonable, the description above does\n> not quite match that goal.  If an untracked '.git-blame-ignore-revs'\n> file exists at the root of the working tree, or if a tracked one has\n> local changes relative to HEAD, the local repository behaves\n> differently from hosting sites that operate on the\n> 'HEAD:.git-blame-ignore-revs' blob.  It may make more sense to say:\n> \"If the 'HEAD:.git-blame-ignore-revs' blob exists, it is added as\n> the initial element in the list of ignore-revs files.  Other files\n> listed in the configuration are also used, but an empty element\n> makes all elements that appeared before in the list forgotten.\"\n> This rule should apply whether the repository is bare or not.\n\nThank you very much for the detailed feedback, Junio! Reading the\ncommitted blob from HEAD instead of the working tree totally makes\nsense.\n\n> Somebody has to audit the parser for these files (one unabbreviated\n> object name per line, ignoring whitespace and lines starting with\n> '#') and ensure that the implementation is truly secure.\n\nI looked through the parser in oidset.c (which we can share for\nboth the HEAD blob and configured files) and peel_to_commit_oid in\nbuiltin/blame.c. Mostly looks good, IMHO, but there may be two edge\ncases we can tighten up:\n\n1. Rejecting lines with embedded NUL bytes via memchr in oidset.c\n   (where strchr and the check after parse_oid_hex_algop currently\n   stop at the first NUL byte and ignore trailing bytes on the\n   line).\n\n2. Passing OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK in\n   peel_to_commit_oid and peeling tags step by step so missing OIDs\n   or tag targets do not trigger lazy promisor fetches in partial\n   clones.\n\nDoes this plan sound good to you for v2?\n\nThanks,\nRavi\n\n"}]}