[PATCH] dir: fix negative pathspecs in 'git ls-files' and 'git add'
From: Diogo Castro via GitGitGadget <hidden>
Date: 2026-08-28 20:35:50
Subsystem:
the rest · Maintainer:
Linus Torvalds
From: Diogo Castro <redacted> `git ls-files` calls `common_prefix()` / `get_common_prefix_len()` which calculate the length of the common prefix of all *positive* pathspecs, `max_prefix_len`. `max_prefix_len` is then passed to `match_pathspec()` -> `match_pathspec_with_flags()` -> `do_match_pathspec()`, which strips `max_prefix_len` bytes off of *all* paths and `match_pathspec_item()` strips *all* pathspecs (positive or negative). This causes the bug previously reported in [1]. As a result, when we run `git ls-files -- sub/sub/sub/file ':(exclude)nonexistent'`: * The common prefix of the positive pathspecs is `sub/sub/sub`, 11 bytes * 11 bytes get stripped off both pathspecs: * "sub/sub/sub/file" becomes "/file" * "nonexistent" becomes "" * Since the negative pathspec degenerated into "", it matches every file, and thus no results are returned. When the common prefix is longer than the negative pathspec, we read out of bounds. `git add` suffers from the same issue. It uses `fill_directory()`, which returns the common prefix length, but doesn't strip the trailing slash. Using the same pathspecs as in the example above, the common prefix would be `sub/sub/sub/`, 12 bytes. Only `git ls-files` and `git add` are impacted. Other callers pass in `0` as the prefix. Bug introduced in: ef79b1f870 (Support pathspec magic :(exclude) and its short form :!, 2013-12-06). Solution: in `do_match_pathspec()`, only strip the prefix when handling positive pathspecs, not when handling negative pathspecs. [1]: https://lore.kernel.org/git/e2dbe996f6a7285fe0487e34d65eccf712867547.camel@redhat.com (local) Reported-by: Thomas Haller <redacted> Signed-off-by: Diogo Castro <redacted> --- dir: fix negative pathspecs in git ls-files and git add Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2391%2Fdcastro%2Fdiogo.castro%2Ffix-pathspecs-common-prefix-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2391/dcastro/diogo.castro/fix-pathspecs-common-prefix-v1 Pull-Request: https://github.com/git/git/pull/2391 dir.c | 11 ++++++++ t/t6132-pathspec-exclude.sh | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+)
diff --git a/dir.c b/dir.c
index 32430090dc..3fb2764efe 100644
--- a/dir.c
+++ b/dir.c@@ -539,6 +539,17 @@ static int do_match_pathspec(struct index_state *istate, return 0; } + /* + * The `prefix`, calculated by `common_prefix_len()`, only takes + * positive pathspecs into account. Negative pathspecs are not + * considered. + * + * Therefore, the prefix can only be stripped from positive + * pathspecs, not from negative pathspecs. + */ + if (exclude) + prefix = 0; + name += prefix; namelen -= prefix;
diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh
index 9fdafeb1e9..dd54378019 100755
--- a/t/t6132-pathspec-exclude.sh
+++ b/t/t6132-pathspec-exclude.sh@@ -425,4 +425,56 @@ test_expect_success 'stash with all negative' ' test_cmp expect actual ' +# `ls-files` finds the length of the common prefix of the *positive* pathspecs. +# In this example, there's only one positive pathspec, so the common prefix is `aaa/bbb`, with length 7. +# +# Before the bug described in https://lore.kernel.org/git/e2dbe996f6a7285fe0487e34d65eccf712867547.camel@redhat.com +# was patched, as an optimization, we would then strip the first 7 characters from the path, +# the positive pathspec, and (incorrectly) the negative pathspec. +# +# But stripping the negative pathspec would mean that `xxx/yyy/file` becomes `file` +# and we'd wrongly end up excluding `aaa/bbb/file`. +# +# After this bug fix, `aaa/bbb/file` should no longer be excluded by `:(exclude)xxx/yyy/file`. +test_expect_success 'exclude is not matched against the tail of the path' ' + test_when_finished "git rm -q --cached -r aaa xxx && rm -rf aaa xxx" && + mkdir -p aaa/bbb xxx/yyy && + >aaa/bbb/file && + >xxx/yyy/other && + git add aaa xxx && + echo aaa/bbb/file >expect && + git ls-files -- aaa/bbb/file ":(exclude)xxx/yyy/file" >actual && + test_cmp expect actual +' + +# Before the bug described in https://lore.kernel.org/git/e2dbe996f6a7285fe0487e34d65eccf712867547.camel@redhat.com +# was patched, when the negative pathspec had the same length or was +# shorter than the common prefix of the positive pathspecs, +# then stripping the common prefix from the negative pathspec would result in an empty string, +# which would match everything, and thus exclude all files. +# +# In this test, the prefix for "sub/sub/sub/file" is "sub/sub/sub" (11 bytes). +test_expect_success 'ls-files keeps entries when an exclude matches the common prefix length' ' + echo sub/sub/sub/file >expect && + git ls-files -- sub/sub/sub/file ":(exclude)nonexistent" >actual && + test_cmp expect actual +' + +# This test is similar to the above, but tests `git add` instead of `git ls-files`. +# +# `git add` does not exclude the trailing slash, so the common prefix is "sub/sub/sub/" (12 bytes). +test_expect_success 'add keeps entries when an exclude matches the common prefix length' ' + test_when_finished "git reset -q && rm -f sub/sub/sub/untracked" && + >sub/sub/sub/untracked && + git add -- sub/sub/sub/ ":(exclude)no/such/path" && + echo sub/sub/sub/untracked >expect && + git diff --cached --name-only HEAD >actual && + test_cmp expect actual +' + +test_expect_success 'an exclude shorter than the common prefix still excludes' ' + git ls-files -- sub/sub/sub/file ":(exclude)sub" >actual && + test_must_be_empty actual +' + test_done
base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc -- gitgitgadget