Skip to content

overcommit --run skips Git-quoted filenames with include filters #908

Open

Description

@Osraka

Reproduced on current main 50f3e866bef75e3c3be7f92b47b1c45b6e7436ba (0.74.0), macOS 15.6.1, Ruby 4.0.7, Apple Git 2.50.1.

With Git's default core.quotePath=true, GitRepo.all_files returns the quoted/escaped output of git ls-files rather than actual paths. When a hook has an include pattern such as **/*.rb, those paths no longer match, and overcommit --run can report success without checking a tracked file.

Reproduction

Use a scratch repository, not an existing project:

mkdir overcommit-path-repro
cd overcommit-path-repro
git init
git config --local user.name 'Local Fixture'
git config --local user.email 'fixture@example.invalid'
git config --local commit.gpgsign false
git config --local core.hooksPath /dev/null
git config --local core.quotePath true
git commit --allow-empty -m 'Initial fixture'

cat > .overcommit.yml <<'YAML'
PreCommit:
  ALL:
    enabled: false
  TrailingWhitespace:
    enabled: true
    on_warn: fail
    include:
      - '**/*.rb'
YAML

overcommit --sign
printf 'puts "fixture" \n' > normal.rb
git add -- normal.rb
overcommit --run
# TrailingWhitespace FAILED; exit 65.

git mv -- normal.rb 'café.rb'
overcommit --run
# All pre-commit hooks passed; exit 0. The content is unchanged.

git config --local core.quotePath false
overcommit --run
# TrailingWhitespace FAILED again; exit 65.

The same false-success result occurs for filenames containing double quotes, tabs, or newlines. core.quotePath=false is a control for the Unicode case, not a general workaround for those other characters.

Scope

GitRepo.all_files in lib/overcommit/git_repo.rb:138-143 splits both git ls-files and git ls-files --deleted on newlines. Using NUL-delimited output for both lists would avoid interpreting Git's C-style filename quoting while retaining the deleted-file filtering added in #905.

This behavior predates #905; I am not reporting it as a regression from that PR. The older #220/#222 discussion concerned a repository root containing spaces and list_files. I reproduced this with the repository root itself containing a space, and ordinary space-containing basenames still work.

The regression cases should include live Unicode/quoted/control-character paths, a nested Unicode path, unstaged deletions, an empty index, untracked files, and a tracked broken symlink. The broken symlink must remain visible to the BrokenSymlinks hook.

Would a small all_files parsing fix with these tests be welcome? I would leave other Git helpers, command execution, and index-error handling out of this change.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions