nginx_conf resource: Fix include paths with quotes (#2726)

* nginx_conf resource: Fix include paths with quotes
* Move quote removal to `NginxParser`
* Add parsers/tests for quotes in quotes

Signed-off-by: Jerry Aldrich <jerryaldrichiii@gmail.com>

lib/utils/find_files.rb

@@ -17,17 +17,20 @@ module FindFiles
 
   # ignores errors
   def find_files(path, opts = {})
-    find_files_or_error(path, opts) || []
+    find_files_or_warn(path, opts) || []
   end
 
-  def find_files_or_error(path, opts = {})
+  def find_files_or_warn(path, opts = {})
     depth = opts[:depth]
     type = TYPES[opts[:type].to_sym] if opts[:type]
 
-    cmd = "sh -c \'find #{path}"
+    # If `path` contains a `'` we must modify how we quote the `sh -c` argument
+    quote = path.include?("'") ? '"' : '\''
+
+    cmd = "sh -c #{quote}find #{path}"
     cmd += " -type #{type}" unless type.nil?
     cmd += " -maxdepth #{depth.to_i}" if depth.to_i > 0
-    cmd += "\'"
+    cmd += quote
 
     result = inspec.command(cmd)
     exit_status = result.exit_status

lib/utils/nginx_parser.rb

@@ -33,12 +33,32 @@ class NginxParser < Parslet::Parser
     standard_identifier | quoted_identifier
   }
 
-  rule(:value) {
-    ((match('[#;{]').absent? >> any) >> (
+  rule(:standard_value) {
+    ((match(/[#;{'"]/).absent? >> any) >> (
       str('\\') >> any | match('[#;{]|\s').absent? >> any
     ).repeat).as(:value) >> space.repeat
   }
 
+  rule(:single_quoted_value) {
+    str("'") >> (
+      str('\\') >> any | str("'").absent? >> any
+    ).repeat.as(:value) >> str("'") >> space.repeat
+  }
+
+  rule(:double_quoted_value) {
+    str('"') >> (
+      str('\\') >> any | str('"').absent? >> any
+    ).repeat.as(:value) >> str('"') >> space.repeat
+  }
+
+  rule(:quoted_value) {
+    single_quoted_value | double_quoted_value
+  }
+
+  rule(:value) {
+    standard_value | quoted_value
+  }
+
   rule(:values) {
     value.repeat >> space.maybe
   }

test/helper.rb

@@ -157,6 +157,7 @@ class MockLoader
       '/etc/nginx/conf/mime.types' => mockfile.call('nginx_mime.types'),
       '/etc/nginx/conf.d/foobar.conf' => mockfile.call('nginx_confd_foobar.conf'),
       '/etc/nginx/conf.d/multiple.conf' => mockfile.call('nginx_confd_multiple.conf'),
+      '/etc/nginx/quotes.d/example.conf' => mockfile.call('nginx_quotesd_example.conf'),
       '/etc/xinetd.conf' => mockfile.call('xinetd.conf'),
       '/etc/xinetd.d' => mockfile.call('xinetd.d'),
       '/etc/xinetd.d/chargen-stream' => mockfile.call('xinetd.d_chargen-stream'),
@@ -360,6 +361,7 @@ class MockLoader
       "sh -c 'find /etc/nginx/conf/mime.types'" => cmd.call('find-nginx-mime-types'),
       "sh -c 'find /etc/nginx/proxy.conf'" => cmd.call('find-nginx-proxy-conf'),
       "sh -c 'find /etc/nginx/conf.d/*.conf'" => cmd.call('find-nginx-confd-multiple-conf'),
+      "sh -c 'find /etc/nginx/quotes.d/*.conf'" => cmd.call('find-nginx-quotesd-example-conf'),
       # mount
       "mount | grep -- ' on /'" => cmd.call("mount"),
       "mount | grep -- ' on /mnt/iso-disk'" => cmd.call("mount-multiple"),

test/unit/mock/cmd/find-nginx-quotesd-example-conf

@@ -0,0 +1 @@
+/etc/nginx/quotes.d/example.conf

test/unit/mock/files/nginx.conf

@@ -9,6 +9,7 @@ http {
   include    conf/mime.types;                 # relative path
   include    /etc/nginx/proxy.conf;           # absolute path
   include    /etc/nginx/conf.d/*.conf;        # wildcard path
+  include    "/etc/nginx/quotes.d/*.conf";    # path with quotes
 
   index    index.html index.htm index.php;
   default_type application/octet-stream;      # parameter with '/'

test/unit/mock/files/nginx_quotesd_example.conf

@@ -0,0 +1,4 @@
+server {
+  listen       8085;
+  server_name  quotes.com www.quotes.com;
+}

test/unit/resources/nginx_conf_test.rb

@@ -31,6 +31,7 @@ describe 'Inspec::Resources::NginxConf' do
       /etc/nginx/proxy.conf
       /etc/nginx/conf.d/foobar.conf
       /etc/nginx/conf.d/multiple.conf
+      /etc/nginx/quotes.d/example.conf
     )
 
     # verify user
@@ -46,7 +47,7 @@ describe 'Inspec::Resources::NginxConf' do
     _(nginx_conf.params['http'].length).must_equal 1
 
     # verify server count
-    _(nginx_conf.params['http'][0]['server'].length).must_equal 5
+    _(nginx_conf.params['http'][0]['server'].length).must_equal 6
 
     # verify index
     _(nginx_conf.params['http'][0]['index']).must_equal [['index.html', 'index.htm', 'index.php']]
@@ -92,6 +93,10 @@ describe 'Inspec::Resources::NginxConf' do
     _(nginx_conf.params['http'][0]['server'][4]['server_name']).must_equal [['example2.com', 'www.example2.com']]
     _(nginx_conf.params['http'][0]['server'][4]['location'][0]['_']).must_equal ['~', '^/media/']
     _(nginx_conf.params['http'][0]['server'][4]['location'][0]['root']).must_equal [['/var/www/virtual/www.example2.com/htdocs']]
+
+    # verify a server in conf.d_quotes (quotes in path test)
+    _(nginx_conf.params['http'][0]['server'][5]['listen']).must_equal [['8085']]
+    _(nginx_conf.params['http'][0]['server'][5]['server_name']).must_equal [['quotes.com', 'www.quotes.com']]
   end
 
   it 'skips the resource if it cannot parse the config' do
@@ -122,7 +127,7 @@ describe 'Inspec::Resources::NginxConf' do
 
     it 'provides aggregated access to all servers' do
       _(http.servers).must_be_kind_of Array
-      _(http.servers.length).must_equal 5
+      _(http.servers.length).must_equal 6
       http.servers.each do |server|
         _(server).must_be_kind_of Inspec::Resources::NginxConfServer
       end
@@ -153,7 +158,7 @@ describe 'Inspec::Resources::NginxConf' do
 
     it 'provides aggregated access to all servers' do
       _(entry.servers).must_be_kind_of Array
-      _(entry.servers.length).must_equal 5
+      _(entry.servers.length).must_equal 6
       _(entry.servers[0]).must_be_kind_of Inspec::Resources::NginxConfServer
       entry.servers.each do |server|
         _(server).must_be_kind_of Inspec::Resources::NginxConfServer

test/unit/utils/find_files_test.rb

@@ -4,7 +4,7 @@
 require 'helper'
 
 describe FindFiles do
-  let (:findfiles) do
+  let (:helper) do
     class FindFilesTest
       include FindFiles
       def inspec
@@ -14,10 +14,46 @@ describe FindFiles do
     FindFilesTest.new
   end
 
+  let(:inspec) { mock }
+  let(:result) { mock }
+
   describe '#find_files' do
-    it 'returns an array (of findings)' do
-      files = findfiles.find_files('/no/such/mock', type: 'file', depth: 1)
-      files.must_equal([])
+    it 'returns an empty array when no files are found' do
+      helper.expects(:warn)
+      helper.find_files('/no/such/mock', type: 'file', depth: 1).must_equal([])
+    end
+  end
+
+  describe '#find_files_or_warn' do
+    before do
+      helper.expects(:inspec).returns(inspec)
+      result.stubs(:exit_status).returns(0)
+      result.stubs(:stdout).returns('mock')
+    end
+
+    it 'constructs the correct command' do
+      inspec.expects(:command).with("sh -c 'find /a/b/'").returns(result)
+      helper.find_files('/a/b/')
+    end
+
+    it 'builds the correct command when a single quote is used' do
+      inspec.expects(:command).with('sh -c "find /a/\'b/"').returns(result)
+      helper.find_files("/a/'b/")
+    end
+
+    it 'constructs the correct command when a double quote is in the path' do
+      inspec.expects(:command).with("sh -c 'find /a/\"b/'").returns(result)
+      helper.find_files('/a/"b/')
+    end
+
+    it 'builds the correct command when an escaped single quote is used' do
+      inspec.expects(:command).with('sh -c "find /a/\\\'b/"').returns(result)
+      helper.find_files('/a/\\\'b/')
+    end
+
+    it 'builds the correct command when an escaped double quote is used' do
+      inspec.expects(:command).with("sh -c 'find /a/\\\"b/'").returns(result)
+      helper.find_files('/a/\"b/')
     end
   end
 end

test/unit/utils/nginx_parser_test.rb

@@ -30,6 +30,42 @@ describe NginxParser do
     _(parsestr("assignment a;")).must_equal "[{:assignment=>{:identifier=>\"assignment\"@0, :args=>[{:value=>\"a\"@11}]}}]"
   end
 
+  it 'parses an assignment with a single quoted value' do
+    result = parse("include '/a/b/c/*.conf';")
+    result[0][:assignment][:identifier].must_equal 'include'
+    result[0][:assignment][:args][0][:value].must_equal '/a/b/c/*.conf'
+  end
+
+  it 'parses an assignment with a double quoted value' do
+    result = parse('include "/a/b/c/*.conf";')
+    result[0][:assignment][:identifier].must_equal 'include'
+    result[0][:assignment][:args][0][:value].must_equal '/a/b/c/*.conf'
+  end
+
+  it 'parses an assignemnt with single quote in a double quoted value' do
+    result = parse('include "/a/\'b/*.conf";')
+    result[0][:assignment][:identifier].must_equal 'include'
+    result[0][:assignment][:args][0][:value].must_equal '/a/\'b/*.conf'
+  end
+
+  it 'parses an assignemnt with double quote in a single quoted value' do
+    result = parse("include '/a/\"b/*.conf';")
+    result[0][:assignment][:identifier].must_equal 'include'
+    result[0][:assignment][:args][0][:value].must_equal "/a/\"b/*.conf"
+  end
+
+  it 'parses an assignemnt with single quote in a single quoted value' do
+    result = parse("include '/a/\\\'b/*.conf';")
+    result[0][:assignment][:identifier].must_equal 'include'
+    result[0][:assignment][:args][0][:value].must_equal "/a/\\\'b/*.conf"
+  end
+
+  it 'parses an assignemnt with double quote in a double quoted value' do
+    result = parse('include "/a/\"b/*.conf";')
+    result[0][:assignment][:identifier].must_equal 'include'
+    result[0][:assignment][:args][0][:value].must_equal '/a/\"b/*.conf'
+  end
+
   it 'parses an empty group' do
     _(parsestr("group {}")).must_equal "[{:section=>{:identifier=>\"group\"@0}, :args=>\"\", :expressions=>[]}]"
   end