diff --git a/bin/pungi-koji b/bin/pungi-koji index 1044ec1a..a0eba0e1 100755 --- a/bin/pungi-koji +++ b/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) diff --git a/pungi/checks.py b/pungi/checks.py index 6895b2ab..f0d8b5bc 100644 --- a/pungi/checks.py +++ b/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: diff --git a/pungi/util.py b/pungi/util.py index 857174ea..f4f5e335 100644 --- a/pungi/util.py +++ b/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): diff --git a/pungi/wrappers/pungi.py b/pungi/wrappers/pungi.py index 4d686bb1..ccd5eb6f 100644 --- a/pungi/wrappers/pungi.py +++ b/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[^\)]+)\))?: (?:file://)?(?P/?[^ ]+)$"), @@ -43,11 +44,7 @@ class PungiWrapper(object): ks_path = os.path.abspath(ks_path) ks_dir = os.path.dirname(ks_path) - try: - os.makedirs(ks_dir) - except OSError as ex: - if ex.errno != errno.EEXIST: - raise + util.makedirs(ks_dir) kickstart = open(ks_path, "w") diff --git a/tests/test_checks.py b/tests/test_checks.py index a0a49e00..0e2a9e2b 100755 --- a/tests/test_checks.py +++ b/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() diff --git a/tests/test_util.py b/tests/test_util.py index d730ba4c..97892b71 100755 --- a/tests/test_util.py +++ b/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()