Slight improvements to validation failures. Shows the failing command if possible and doesn't show anything for sensitive resources to be safe.

Refs https://github.com/chef/chef/issues/4793 and ping @chef/client-core for review.

4 files changed, 38 insertions, 6 deletions

diff --git a/lib/chef/provider/file.rb b/lib/chef/provider/file.rb
index fea77dd7ea..86e7e5a5c4 100644
--- a/lib/chef/provider/file.rb
+++ b/lib/chef/provider/file.rb
@@ -339,7 +339,7 @@ class Chef
         if tempfile
           new_resource.verify.each do |v|
             if ! v.verify(tempfile.path)
-              raise Chef::Exceptions::ValidationFailed.new "Proposed content for #{new_resource.path} failed verification #{v}"
+              raise Chef::Exceptions::ValidationFailed.new "Proposed content for #{new_resource.path} failed verification #{new_resource.sensitive ? '[sensitive]' : v}"
             end
           end
         end
diff --git a/lib/chef/resource/file/verification.rb b/lib/chef/resource/file/verification.rb
index 079e6dc201..cfbdfd5263 100644
--- a/lib/chef/resource/file/verification.rb
+++ b/lib/chef/resource/file/verification.rb
@@ -119,6 +119,16 @@ class Chef
           v = verification_class.new(@parent_resource, @command, @command_opts, &@block)
           v.verify(path, opts)
         end
+
+        def to_s
+          if @block
+            '<Proc>'
+          elsif @command.is_a?(Symbol)
+            "#{@command.inspect} (#{Chef::Resource::File::Verification.lookup(@command).name})"
+          elsif @command.is_a?(String)
+            @command
+          end
+        end
       end
     end
   end
diff --git a/spec/support/shared/unit/provider/file.rb b/spec/support/shared/unit/provider/file.rb
index b58159fcc9..d508e76b24 100644
--- a/spec/support/shared/unit/provider/file.rb
+++ b/spec/support/shared/unit/provider/file.rb
@@ -476,7 +476,15 @@ shared_examples_for Chef::Provider::File do
           allow(File).to receive(:directory?).with("C:\\Windows\\system32/cmd.exe").and_return(false)
           provider.new_resource.verify windows? ? "REM" : "true"
           provider.new_resource.verify windows? ? "cmd.exe /c exit 1" : "false"
-          expect { provider.send(:do_validate_content) }.to raise_error(Chef::Exceptions::ValidationFailed)
+          expect { provider.send(:do_validate_content) }.to raise_error(Chef::Exceptions::ValidationFailed, "Proposed content for #{provider.new_resource.path} failed verification #{windows? ? "cmd.exe /c exit 1" : "false"}")
+        end
+
+        it "does not show verification for sensitive resources" do
+          allow(File).to receive(:directory?).with("C:\\Windows\\system32/cmd.exe").and_return(false)
+          provider.new_resource.verify windows? ? "REM" : "true"
+          provider.new_resource.verify windows? ? "cmd.exe /c exit 1" : "false"
+          provider.new_resource.sensitive true
+          expect { provider.send(:do_validate_content) }.to raise_error(Chef::Exceptions::ValidationFailed, "Proposed content for #{provider.new_resource.path} failed verification [sensitive]")
         end
       end
     end
diff --git a/spec/unit/resource/file/verification_spec.rb b/spec/unit/resource/file/verification_spec.rb
index feacd715a4..8e850395d8 100644
--- a/spec/unit/resource/file/verification_spec.rb
+++ b/spec/unit/resource/file/verification_spec.rb
@@ -66,6 +66,11 @@ describe Chef::Resource::File::Verification do
         v = Chef::Resource::File::Verification.new(parent_resource, nil, {}, &f_block)
         expect(v.verify(temp_path)).to eq(false)
       end
+
+      it "responds to to_s" do
+        v = Chef::Resource::File::Verification.new(parent_resource, nil, {}) { }
+        expect(v.to_s).to eq("<Proc>")
+      end
     end
 
     context "with a verification command(String)" do
@@ -110,23 +115,32 @@ describe Chef::Resource::File::Verification do
         v = Chef::Resource::File::Verification.new(parent_resource, "true", {})
         expect(v.verify(temp_path)).to eq(true)
       end
+
+      it "responds to to_s" do
+        v = Chef::Resource::File::Verification.new(parent_resource, "some command --here", {})
+        expect(v.to_s).to eq("some command --here")
+      end
     end
 
     context "with a named verification(Symbol)" do
+      let(:registered_verification) { double("registered_verification") }
+      subject { described_class.new(parent_resource, :cats, {}) }
       before(:each) do
         class Chef::Resource::File::Verification::Turtle < Chef::Resource::File::Verification
           provides :cats
           def verify(path, opts)
           end
         end
+        allow(Chef::Resource::File::Verification::Turtle).to receive(:new).and_return(registered_verification)
       end
 
       it "delegates to the registered verification" do
-        registered_verification = double()
-        allow(Chef::Resource::File::Verification::Turtle).to receive(:new).and_return(registered_verification)
-        v = Chef::Resource::File::Verification.new(parent_resource, :cats, {})
         expect(registered_verification).to receive(:verify).with(temp_path, {})
-        v.verify(temp_path, {})
+        subject.verify(temp_path, {})
+      end
+
+      it "responds to to_s" do
+        expect(subject.to_s).to eq(":cats (Chef::Resource::File::Verification::Turtle)")
       end
     end
   end