Zhuyifei1999 has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/405830 )
Change subject: crontab: Remove -u parameter, and make uid < 500 always edit local ...................................................................... crontab: Remove -u parameter, and make uid < 500 always edit local Consensus seems that root should always edit local crontab, and for remotes they can always sudo first. `-u` should not be used, and, to support puppet's use of that parameter, the uid check is moved to before argparse so as not to raise an unrecognized argument error. Bug: T179386 Change-Id: Ifb19c556868416889d2856633650d24205f4a71a --- M debian/changelog M misctools/oge-crontab 2 files changed, 19 insertions(+), 25 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/labs/toollabs refs/changes/30/405830/1 diff --git a/debian/changelog b/debian/changelog index e5d4707..ee0206c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +toollabs (1.29) unstable; urgency=medium + + * crontab: Remove -u parameter, and make uid < 500 always edit local + + -- YiFei Zhu <[email protected]> Tue, 23 Jan 2018 02:10:57 +0000 + toollabs (1.28) unstable; urgency=medium * crontab: Make the input file is binary when it is sys.stdin diff --git a/misctools/oge-crontab b/misctools/oge-crontab index b8859db..0ccbc2c 100644 --- a/misctools/oge-crontab +++ b/misctools/oge-crontab @@ -74,15 +74,12 @@ class Crontab(object): - def __init__(self, pw, cron_host): - self.pw = pw + def __init__(self, cron_host): self.cron_host = cron_host def _remote(self, *args, stdin=None): """Execute remote crontab command and return stdout.""" cmd = ['/usr/bin/ssh', self.cron_host, '/usr/bin/crontab'] + list(args) - if self.pw.pw_uid != os.getuid(): - cmd += ['-u', self.pw.pw_name] ssh = subprocess.Popen( cmd, @@ -228,8 +225,18 @@ def main(): + # This part has to come first. Puppet calls crontab with `-u` which + # would be an unrecognized parameter. T179386 + pw = pwd.getpwuid(os.getuid()) + if pw.pw_uid < 500: + # If the target user is not managed in LDAP and thus likely + # a system user, invoke the original crontab instead. + os.execv('/usr/bin/crontab', sys.argv) + elif pw.pw_uid < 40000: + err('only tools are allowed crontabs') + sys.exit(1) + parser = argparse.ArgumentParser() - parser.add_argument('-u', dest='user', help=argparse.SUPPRESS) group = parser.add_mutually_exclusive_group() group.add_argument( 'file', default=sys.stdin.buffer, nargs='?', @@ -258,30 +265,11 @@ if args.i and args.operation != 'r': parser.error('argument -i: only applicable with -r') - if args.user: - if os.getuid(): - parser.error('argument -u: must be privileged') - try: - pw = pwd.getpwnam(args.user) - except KeyError: - parser.error('argument -u: unknown user "{}"'.format(args.user)) - else: - pw = pwd.getpwuid(os.getuid()) - - if pw.pw_uid < 500: - # If the target user is not managed in LDAP and thus likely - # a system user, invoke the original crontab instead. - os.execv('/usr/bin/crontab', sys.argv) - - if pw.pw_uid < 40000 and os.getuid(): - err('only tools are allowed crontabs') - sys.exit(1) - with open('/etc/toollabs-cronhost', 'r') as f: cron_host = f.read().strip() try: - crontab = Crontab(pw, cron_host) + crontab = Crontab(cron_host) if args.operation is None: # Replace -- To view, visit https://gerrit.wikimedia.org/r/405830 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ifb19c556868416889d2856633650d24205f4a71a Gerrit-PatchSet: 1 Gerrit-Project: labs/toollabs Gerrit-Branch: master Gerrit-Owner: Zhuyifei1999 <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
