Skip to content

Commit aa7cd10

Browse files
authored
Preserve Git-quoted filenames when running all hooks (sds#924)
With Git path quoting enabled, `overcommit --run` treats escaped `git ls-files` output as filenames. Unicode, quoted, and control-character paths fail include filters, allowing violations to pass unchecked. Request NUL-delimited output for both tracked and deleted files and split on NUL, preserving the actual filenames and the existing deletion filtering. Fixes sds#908. Reproduced locally before the fix: five CLI cases (Unicode, nested Unicode, quotes, tabs, and newlines) incorrectly exited 0 despite trailing whitespace. Regression coverage also checks spaces and surrounding whitespace, deleted Unicode and control-character paths, and a tracked broken symlink. Filesystem-invalid Windows names are tested on Unix; Windows runs the Unicode cases and existing enumeration controls. Validation: full local RSpec suite (1,728 examples); RuboCop on changed Ruby files. The PR runs the full Linux/Windows CI matrix.
1 parent 3bf9d27 commit aa7cd10

3 files changed

Lines changed: 63 additions & 3 deletions

File tree

‎lib/overcommit/git_repo.rb‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ def tracked?(path)
136136
#
137137
# @return [Array<String>] list of absolute file paths
138138
def all_files
139-
deleted_files, tracked_files = [%w[git ls-files --deleted], %w[git ls-files]].map do |command|
139+
deleted_files, tracked_files = [%w[git ls-files -z --deleted],
140+
%w[git ls-files -z]].map do |command|
140141
result = Overcommit::Utils.execute(command)
141142
unless result.success?
142143
raise Overcommit::Exceptions::Error,
@@ -145,7 +146,7 @@ def all_files
145146
"STDERR: #{result.stderr}."
146147
end
147148

148-
result.stdout.split(/\n/)
149+
result.stdout.split("\0")
149150
end
150151

151152
(tracked_files - deleted_files).

‎spec/integration/run_flag_spec.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,20 @@
4646
(subject.stdout + subject.stderr).should include('Error listing files')
4747
subject.stdout.should_not include('All pre-commit hooks passed')
4848
end
49+
50+
['café.rb', 'répertoire/test.rb', 'quote".rb', "tab\t.rb", "line\n.rb"].each do |path|
51+
it "checks trailing whitespace in #{path.inspect} with Git path quoting enabled" do
52+
if Overcommit::OS.windows? && path.match?(/["\t\n]/)
53+
skip 'Windows filenames cannot contain quotes or control characters'
54+
end
55+
56+
shell(%w[git config core.quotePath true]).status.should == 0
57+
FileUtils.mkdir_p(File.dirname(path))
58+
echo('puts :fixture ', path)
59+
shell(['git', 'add', '--', path]).status.should == 0
60+
subject.status.should == 65
61+
end
62+
end
4963
end
5064

5165
context 'when using an existing pre-commit hook script' do

‎spec/overcommit/git_repo_spec.rb‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@
317317
before do
318318
result = Overcommit::Subprocess::Result.new(128, '', 'fatal')
319319
allow(Overcommit::Utils).to receive(:execute).and_call_original
320-
allow(Overcommit::Utils).to receive(:execute).with(%w[git ls-files]).and_return(result)
320+
allow(Overcommit::Utils).to receive(:execute).with(%w[git ls-files -z]).and_return(result)
321321
end
322322

323323
it 'raises with the Git error' do
@@ -334,6 +334,51 @@
334334
it { should_not include File.expand_path('tracked') }
335335
it { should include File.expand_path('staged') }
336336
end
337+
338+
context 'when Git quotes tracked filenames' do
339+
before { shell(%w[git config core.quotePath true]).status.should == 0 }
340+
341+
def stage_files(paths)
342+
paths.each do |path|
343+
FileUtils.mkdir_p(File.dirname(path))
344+
touch(path)
345+
end
346+
shell(%w[git add --] + paths).status.should == 0
347+
end
348+
349+
it 'preserves Unicode, nested Unicode, and space-containing paths' do
350+
paths = ['café.rb', 'répertoire/test.rb', 'with spaces.rb']
351+
stage_files(paths)
352+
subject.should include(*paths.map { |path| File.expand_path(path) })
353+
end
354+
355+
it 'preserves quotes, tabs, newlines, and surrounding whitespace' do
356+
skip 'Windows filenames cannot contain quotes or control characters' if Overcommit::OS.windows?
357+
paths = ['quote".rb', "tab\t.rb", "line\n.rb", ' padded.rb ']
358+
stage_files(paths)
359+
subject.should include(*paths.map { |path| File.expand_path(path) })
360+
end
361+
362+
it 'excludes a deleted Unicode path' do
363+
stage_files(['café.rb'])
364+
FileUtils.rm('café.rb')
365+
subject.should_not include(File.expand_path('café.rb'))
366+
end
367+
368+
it 'excludes deleted paths containing quotes or control characters' do
369+
skip 'Windows filenames cannot contain quotes or control characters' if Overcommit::OS.windows?
370+
paths = ['quote".rb', "tab\t.rb", "line\n.rb"]
371+
stage_files(paths)
372+
FileUtils.rm(paths)
373+
subject.should_not include(*paths.map { |path| File.expand_path(path) })
374+
end
375+
376+
it 'retains a tracked broken symlink with a Unicode name' do
377+
symlink('missing-target', 'café.rb')
378+
shell(['git', 'add', '--', 'café.rb']).status.should == 0
379+
subject.should include(File.expand_path('café.rb'))
380+
end
381+
end
337382
end
338383

339384
describe '.initial_commit?' do

0 commit comments

Comments
 (0)