Re: [PATCH] blame: default to ignoring revisions in .git-blame-ignore-revs
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 5, 2026, 15:37 UTC
- Message-ID
- <xmqqse2kma4o.fsf@gitster.g>
- In-Reply-To
- <pull.2224.git.1789169384240.gitgitgadget@gmail.com>
"Ravi Mistry via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 12 quoted lines
> blame.ignoreRevsFile:: > Ignore revisions listed in the file, one unabbreviated object name per > line, in linkgit:git-blame[1]. Whitespace and comments beginning with > - `#` are ignored. This option may be repeated multiple times. Empty > - file names will reset the list of ignored revisions. This option will > - be handled before the command line option `--ignore-revs-file`. > + `#` are ignored. If `.git-blame-ignore-revs` exists at the root of the > + working tree in a non-bare repository, it is used by default. This option > + may be repeated multiple times; files specified here are processed after > + the default file. An empty file name will reset the list of ignored > + revisions from previously processed files and disable the default file. > + This option is handled before the command-line option `--ignore-revs-file`.
The proposed log message explains that '.git-blame-ignore-revs' is used as the default for blame.ignoreRevsFile even when the user does not ask to do so in order to match what hosting sites do, as it would be confusing if the local repository behaved differently. While wanting consistency is reasonable, the description above does not quite match that goal. If an untracked '.git-blame-ignore-revs' file exists at the root of the working tree, or if a tracked one has local changes relative to HEAD, the local repository behaves differently from hosting sites that operate on the 'HEAD:.git-blame-ignore-revs' blob. It may make more sense to say: "If the 'HEAD:.git-blame-ignore-revs' blob exists, it is added as the initial element in the list of ignore-revs files. Other files listed in the configuration are also used, but an empty element makes all elements that appeared before in the list forgotten." This rule should apply whether the repository is bare or not.
The proposed log message also talks about taking only a regular file and ignoring everything else for "security" [*], but there is another important thing we need to worry about security-wise. Somebody has to audit the parser for these files (one unabbreviated object name per line, ignoring whitespace and lines starting with '#') and ensure that the implementation is truly secure.
This is a new threat vector introduced by this change. Without this patch, blame.ignoreRevsFile comes only from the configuration, which cannot point to an attacker-controlled file under our threat model. Now, however, the parser must read upstream-controlled content at a known path, and it must be prepared to cope with attempts to use it as an attack vector.
[Footnote]
* By the way, "we do not read anything from the working tree. 'HEAD:.git-blame-ignore-revs' is the only thing that is added to the picture" would make it unnecessary to lstat() and ignore non-regular files.