[Change Request] Add script/cron job for checking git repo perms

Todd Zullinger tmz at pobox.com
Thu Aug 20 23:54:24 UTC 2009


The git::check-perms class includes a script for checking that the
permissions of git repositories are generally proper for shared
repositories.  It also runs this script each day via a cron job.

This is included on the hosted1 node.
---

The intent of this script is to check that git repos on hosted don't
end up with permissions that will cause problems when folks try to
push to them.  This shouldn't happen too often anymore since we fixed
a git bug and have better scripts for creating the repositories, but
it can still crop up.

This script won't catch something like the a repo having the wrong
group, unless we want to standardize on group naming and fix up
existing repositories that don't follow that convention.  (Which
doesn't seem worth the effort.)

I just picked the time for the cron job at random.  If there is a
better time for it to run, I can change it before pushing this.

The change should be very low risk and easy to fix should it cause any
problems.  If I weren't likely to forget about it, it could wait until
after the freeze it over. :)

 manifests/nodes/hosted1.fedoraproject.org.pp |    4 +
 modules/git/README                           |    4 +
 modules/git/files/check-perms.py             |  148 ++++++++++++++++++++++++++
 modules/git/manifests/init.pp                |   31 ++++++
 4 files changed, 187 insertions(+), 0 deletions(-)
 create mode 100755 modules/git/files/check-perms.py

diff --git a/manifests/nodes/hosted1.fedoraproject.org.pp b/manifests/nodes/hosted1.fedoraproject.org.pp
index e94c151..a6c86c0 100644
--- a/manifests/nodes/hosted1.fedoraproject.org.pp
+++ b/manifests/nodes/hosted1.fedoraproject.org.pp
@@ -5,6 +5,10 @@ node hosted1 {
     include openvpn::client
     include spamassassin::server
 
+    $git_check_perms_gitroot = "/git"
+    $git_check_perms_mailto = "sysadmin-hosted-members at fedoraproject.org"
+    include git::check-perms
+
     $mailman_default_url_proto = "https"
     $mailman_default_url_host = "fedorahosted.org"
     $mailman_default_email_host = "lists.fedorahosted.org"
diff --git a/modules/git/README b/modules/git/README
index e9a5e99..100a560 100644
--- a/modules/git/README
+++ b/modules/git/README
@@ -14,6 +14,10 @@ The git rpm installs the core tools with minimal dependencies.  To
 install all git packages, including tools for integrating with other
 SCMs, install the git-all meta-package.
 
+The git::check-perms class includes a script for checking that the
+permissions of git repositories are generally proper for shared
+repositories.  It also runs this script each day via a cron job.
+
 The git::mail-hooks class installs some convenient tools for use as
 post-receive hooks, courtesy of the gnome.org sysadmins.
 
diff --git a/modules/git/files/check-perms.py b/modules/git/files/check-perms.py
new file mode 100755
index 0000000..88d7bff
--- /dev/null
+++ b/modules/git/files/check-perms.py
@@ -0,0 +1,148 @@
+#!/usr/bin/python -tt
+"""Check permissions of a tree of git repositories, optionally fixing any
+problems found.
+"""
+
+import os
+import re
+import sys
+import optparse
+from stat import *
+from subprocess import call, PIPE, Popen
+
+usage = '%prog [options] [gitroot]'
+parser = optparse.OptionParser(usage=usage)
+parser.add_option('-f', '--fix', dest='fix',
+                  action='store_true', default=False,
+                  help='Correct any problems [%default]')
+opts, args = parser.parse_args()
+
+if args:
+    gitroot = args[0]
+else:
+    gitroot = '/git'
+
+object_re = re.compile('[0-9a-z]{40}')
+
+def is_object(path):
+    """Check if a path is a git object."""
+    parts = path.split(os.path.sep)
+    if 'objects' in parts and len(parts) > 2 and \
+            object_re.match(''.join(path.split(os.path.sep)[-2:])):
+        return True
+    return False
+
+def is_shared_repo(gitdir):
+    """Check if a git repository is shared."""
+    cmd = ['git', '--git-dir', gitdir, 'config', 'core.sharedRepository']
+    p = Popen(cmd, stdout=PIPE, stderr=PIPE)
+    shared, error = p.communicate()
+    sharedmodes = ['1', 'group', 'true', '2', 'all', 'world', 'everybody']
+    if shared.rstrip() not in sharedmodes or p.returncode:
+        return False
+    return True
+
+def set_shared_repo(gitdir, value='group'):
+    """Set core.sharedRepository for a git repository."""
+    mode_re = re.compile('06[0-7]{2}')
+    if value in [0, 'false', 'umask']:
+        value = 'umask'
+    elif value in [1, 'true', 'group']:
+        value = 'group'
+    elif value in [2, 'all', 'world', 'everybody']:
+        value = 'all'
+    elif mode_re.match(value):
+        pass
+    else:
+        raise SystemExit('Bogus core.sharedRepository value "%s"' % value)
+    cmd = ['git', '--git-dir', gitdir, 'config', 'core.sharedRepository',
+            value]
+    ret = call(cmd)
+    if ret:
+        return False
+    return True
+
+def check_git_perms(path, fix=False):
+    """Check if permissions on a git repo are correct.
+
+    If fix is true, problems found are corrected.
+    """
+    object_mode = S_IRUSR | S_IRGRP | S_IROTH
+    oldmode = mode = S_IMODE(os.lstat(path)[ST_MODE])
+    errors = []
+    if os.path.isdir(path):
+        newmode = mode | S_ISGID
+        if mode != newmode:
+            msg = 'Not SETGID (should be "%s")' % oct(newmode)
+            errors.append(msg)
+            mode = newmode
+    elif is_object(path) and mode ^ object_mode:
+        msg = 'Wrong object mode "%s" (should be "%s")' % (
+                oct(mode), oct(object_mode))
+        errors.append(msg)
+        mode = object_mode
+    if mode & S_IWUSR and not is_object(path):
+        newmode = mode | S_IWGRP
+        if mode != newmode:
+            msg = 'Not group writable (should be "%s")' % oct(newmode)
+            errors.append(msg)
+            mode = newmode
+    if mode != oldmode and not os.path.islink(path):
+        print >> sys.stderr, '%s:' % path,
+        print >> sys.stderr, ', '.join(['%s' % e for e in errors])
+        if not fix:
+            return False
+        try:
+            os.chmod(path, mode)
+            return True
+        except Exception, e:
+            error = hasattr(e, 'strerror') and e.strerror or e
+            mode = oct(mode)
+            print >> sys.stderr, 'Error setting "%s" mode on %s: %s' % (
+                    mode, path, error)
+            return False
+    return True
+
+def main():
+    if not os.path.isdir(gitroot):
+        raise SystemExit('%s does not exist or is not a directory' % gitroot)
+
+    gitdirs = []
+    for path, dirs, files in os.walk(gitroot):
+        if path in gitdirs:
+            continue
+        if 'description' in os.listdir(path):
+            gitdirs.append(path)
+
+    problems = []
+    for gitdir in sorted(gitdirs):
+        if not is_shared_repo(gitdir):
+            print >> sys.stderr, '%s: core.sharedRepository not set' % gitdir
+            if not opts.fix or not set_shared_repo(gitdir):
+                problems.append(gitdir)
+                continue
+        paths = []
+        for path, dirs, files in os.walk(gitdir):
+            for d in dirs:
+                d = os.path.join(path, d)
+                if d not in paths:
+                    paths.append(d)
+            for f in files:
+                f = os.path.join(path, f)
+                if f not in paths:
+                    paths.append(f)
+        for path in paths:
+            if not check_git_perms(path, fix=opts.fix):
+                if path not in problems:
+                    problems.append(path)
+
+    if problems:
+        raise SystemExit('%d paths remain unfixed' % len(problems))
+
+    raise SystemExit()
+
+if __name__ == '__main__':
+    try:
+        main()
+    except KeyboardInterrupt:
+        raise SystemExit('\nExiting on user cancel (Ctrl-C)')
diff --git a/modules/git/manifests/init.pp b/modules/git/manifests/init.pp
index 87282b5..ab1abec 100644
--- a/modules/git/manifests/init.pp
+++ b/modules/git/manifests/init.pp
@@ -35,3 +35,34 @@ class git::mail-hooks {
             require => [File["$mailhooks/git.py"], File["$mailhooks/util.py"]];
     }
 }
+
+class git::check-perms {
+    include git::package
+
+    file { '/usr/local/bin/git-check-perms':
+        owner   => 'root',
+        group   => 'root',
+        mode    => 0755,
+        source  => 'puppet:///git/check-perms.py',
+        require => Package['git'],
+    }
+
+    $gitroot = $git_check_perms_gitroot ? {
+        ''      => '/git',
+        default => $git_check_perms_gitroot,
+    }
+
+    $mailto = $git_check_perms_mailto ? {
+        ''      => 'root',
+        default => $git_check_perms_mailto,
+    }
+
+    cron { 'git-check-perms':
+        command     => "git check-perms $gitroot",
+        user        => 'nobody',
+        hour        => 0,
+        minute      => 10,
+        environment => ["MAILTO=$mailto", 'PATH=/usr/bin:/usr/local/bin'],
+        require     => File['/usr/local/bin/git-check-perms'],
+    }
+}
-- 
1.6.4

-- 
Todd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
If Stupidity got us into this mess, then why can't it get us out?
    -- Will Rogers (1879-1935)




More information about the Fedora-infrastructure-list mailing list