vault: check password file is actually a file

Passing a directory currently fails with a "permission denied" error. I
can think of no use case where you want (or actually even can) execute
a directory, so explicitly check for this and raise a descriptive error.

Also rename that one test to match what it is actually checking. And
add tests to make sure symlinks (still) work when they should.

changelogs/fragments/xxxxx-vault-empty.yml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - passing a directory as vault password file now raises a meaningful error (https://github.com/ansible/ansible/pull/xxxxx).

lib/ansible/parsing/vault/__init__.py

@@ -356,6 +356,8 @@ def get_file_vault_secret(filename=None, vault_id=None, encoding=None, loader=No
     this_path = unfrackpath(filename, follow=False)
     if not os.path.exists(this_path):
         raise AnsibleError("The vault password file %s was not found" % this_path)
+    if not os.path.isfile(this_path):
+        raise AnsibleError("The vault password file %s is not a file" % this_path)
 
     # it is a script?
     if loader.is_executable(this_path):

test/units/parsing/vault/test_vault.py

@@ -375,7 +375,7 @@ class TestGetFileVaultSecret(unittest.TestCase):
 
         self.assertEqual(secret.bytes, to_bytes(password))
 
-    def test_file_not_a_directory(self):
+    def test_file_path_part_not_a_directory(self):
         filename = '/dev/null/foobar'
 
         self.assertRaisesRegex(errors.AnsibleError,
@@ -392,6 +392,44 @@ class TestGetFileVaultSecret(unittest.TestCase):
                                vault.get_file_vault_secret,
                                filename=filename)
 
+    def test_file_not_a_directory(self):
+        filename = '/dev'
+
+        self.assertRaisesRegex(errors.AnsibleError,
+                               '.*The vault password file %s is not a file.*' % filename,
+                               vault.get_file_vault_secret,
+                               filename=filename)
+
+    def test_file_is_symlink_to_file(self):
+        with tempfile.NamedTemporaryFile(suffix='symlink') as tmp_file:
+            symlink_filename = os.path.realpath(tmp_file.name)
+
+        try:
+            with tempfile.NamedTemporaryFile(suffix='target') as tmp_file:
+                target_filename = tmp_file.name
+                os.symlink(target_filename, symlink_filename)
+
+                secret = vault.get_file_vault_secret(filename=symlink_filename, loader=self.fake_loader)
+                self.assertIsInstance(secret, vault.FileVaultSecret)
+                self.assertEqual(secret.filename, symlink_filename)
+        finally:
+            os.remove(symlink_filename)
+
+    def test_file_is_symlink_to_directory(self):
+        with tempfile.NamedTemporaryFile(suffix='symlink') as tmp_file:
+            symlink_filename = os.path.realpath(tmp_file.name)
+
+        try:
+            with tempfile.TemporaryDirectory(suffix='target') as target_filename:
+                os.symlink(target_filename, symlink_filename)
+
+                self.assertRaisesRegex(errors.AnsibleError,
+                                       '.*The vault password file %s is not a file.*' % symlink_filename,
+                                       vault.get_file_vault_secret,
+                                       filename=symlink_filename)
+        finally:
+            os.remove(symlink_filename)
+
 
 class TestVaultIsEncrypted(unittest.TestCase):
     def test_bytes_not_encrypted(self):