Merge #242 Fix wrong file permissions

bin/pungi-koji

@@ -172,6 +172,7 @@ def main():
     import pungi.checks
     if not pungi.checks.check(conf):
         sys.exit(1)
+    pungi.checks.check_umask(logger)
 
     if opts.target_dir:
         compose_dir = Compose.get_compose_dir(opts.target_dir, conf, compose_type=compose_type, compose_label=opts.label)

pungi/checks.py

@@ -102,6 +102,16 @@ def check(conf):
     return not fail
 
 
+def check_umask(logger):
+    """Make sure umask is set to something reasonable. If not, log a warning."""
+    mask = os.umask(0)
+    os.umask(mask)
+
+    if mask > 0o022:
+        logger.warning('Unusually strict umask detected (0%03o), '
+                       'expect files with broken permissions.', mask)
+
+
 def validate_options(conf, valid_options):
     errors = []
     for i in valid_options:

pungi/util.py

@@ -138,13 +138,11 @@ def _doCheckSum(path, hash, logger):
 
 
 def makedirs(path, mode=0o775):
-    mask = os.umask(0)
     try:
         os.makedirs(path, mode=mode)
     except OSError as ex:
         if ex.errno != errno.EEXIST:
             raise
-    os.umask(mask)
 
 
 def rmtree(path, ignore_errors=False, onerror=None):

pungi/wrappers/pungi.py

@@ -15,10 +15,11 @@
 # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 
 
-import errno
 import os
 import re
 
+from .. import util
+
 
 PACKAGES_RE = {
     "rpm": re.compile(r"^RPM(\((?P<flags>[^\)]+)\))?: (?:file://)?(?P<path>/?[^ ]+)$"),
@@ -43,11 +44,7 @@ class PungiWrapper(object):
         ks_path = os.path.abspath(ks_path)
 
         ks_dir = os.path.dirname(ks_path)
-        try:
+        util.makedirs(ks_dir)
-            os.makedirs(ks_dir)
-        except OSError as ex:
-            if ex.errno != errno.EEXIST:
-                raise
 
         kickstart = open(ks_path, "w")
 

tests/test_checks.py

@@ -143,5 +143,36 @@ class CheckDependenciesTestCase(unittest.TestCase):
         self.assertFalse(result)
 
 
+class TestUmask(unittest.TestCase):
+    def setUp(self):
+        self.orig_umask = os.umask(0)
+        os.umask(self.orig_umask)
+
+    def tearDown(self):
+        os.umask(self.orig_umask)
+
+    def test_no_warning_with_0022(self):
+        os.umask(0o022)
+        logger = mock.Mock()
+        checks.check_umask(logger)
+        self.assertItemsEqual(logger.mock_calls, [])
+
+    def test_no_warning_with_0000(self):
+        os.umask(0o000)
+        logger = mock.Mock()
+        checks.check_umask(logger)
+        self.assertItemsEqual(logger.mock_calls, [])
+
+    def test_warning_with_0044(self):
+        os.umask(0o044)
+        logger = mock.Mock()
+        checks.check_umask(logger)
+        self.assertItemsEqual(
+            logger.mock_calls,
+            [mock.call.warning('Unusually strict umask detected (0%03o), '
+                               'expect files with broken permissions.', 0o044)]
+        )
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_util.py

@@ -13,7 +13,7 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 from pungi import compose
 from pungi import util
 
-from tests.helpers import touch
+from tests.helpers import touch, PungiTestCase
 
 
 class TestGitRefResolver(unittest.TestCase):
@@ -193,7 +193,7 @@ class TestFindOldCompose(unittest.TestCase):
         self.assertEqual(old, self.tmp_dir + '/Fedora-Rawhide-Base-1-20160229.0')
 
 
-class TestHelpers(unittest.TestCase):
+class TestHelpers(PungiTestCase):
     def test_process_args(self):
         self.assertEqual(util.process_args('--opt={}', None), [])
         self.assertEqual(util.process_args('--opt={}', []), [])
@@ -201,6 +201,17 @@ class TestHelpers(unittest.TestCase):
                          ['--opt=foo', '--opt=bar'])
         self.assertEqual(util.process_args('--opt={}', 'foo'), ['--opt=foo'])
 
+    def test_makedirs(self):
+        util.makedirs(self.topdir + '/foo/bar/baz')
+        self.assertTrue(os.path.isdir(self.topdir + '/foo/bar/baz'))
+
+    def test_makedirs_on_existing(self):
+        os.makedirs(self.topdir + '/foo/bar/baz')
+        try:
+            util.makedirs(self.topdir + '/foo/bar/baz')
+        except OSError:
+            self.fail('makedirs raised exception on existing directory')
+
 
 if __name__ == "__main__":
     unittest.main()