937720 - Re-implement repo-group for hg local disk infra

Status: RESOLVED FIXED

(Developer Services :: General, task)

Description

[Ben Kero [:bkero]]

The repo-group script created in bug 670781 will not work with the upcoming hg local disk servers since mirroring repo ownership web heads will had unncessary complexity to the system, and is generally not required.

As such, this script will need to be written.

Example usage of this script is:

$ curl http://hg.mozilla.org/repo-group?repo=/mozilla-central
scm_level_3

My suggested implementation method is to create a pash.py command similar to the pushlog retrieval mechanism of:

$ ssh [e-mail] repo-group /mozilla-central
scm_level_3

and then using a CGI script to forward this onto the user at the existing URL.

Comment 1

[Ben Kero [:bkero]]

I've created a patch for this and tested it out on the staging host.

$ ssh hgssh.stage.dmz.scl3.mozilla.com -l root repo-group /mozilla-central
scm_level_3

$ ssh hgssh.stage.dmz.scl3.mozilla.com -l root repo-group /mozilla-centrale
/mozilla-centrale is not an hg repository

$ ssh hgssh.stage.dmz.scl3.mozilla.com -l root repo-group /mozilla-central/../../usr
Only alpha-numeric characters, ".", and "-" are allowed in the repository names.
Please try again with only those characters.

Comment 2

[Ben Kero [:bkero]]

--- a/hg_helper.py.old
+++ b/hg_helper.py.new
@@ -6,6 +6,7 @@ from sh_helper import run_command, prompt_user
 from cgi import escape
 from subprocess import Popen, PIPE, STDOUT
 import shlex
+import repo_group
 
 doc_root = {'hg.mozilla.org': '/repo/hg/mozilla',
             'hgpvt.mozilla.org': '/repo/hg_pvt/mozilla',
@@ -324,6 +325,10 @@ def serve (cname):
             fh = open("/repo/hg/mozilla/%s/.hg/pushlog2.db" % (args[0]))
             print(fh.read())
             fh.close()
+    elif ssh_command.startswith ('repo-group') and (cname != 'hg.ecmascript.org'):
+        args = ssh_command.replace ('repo-group', '').split()
+        if check_repo_name (args[0]):
+            print(repo_group.repo_owner(args[0]))
     else:
         sys.stderr.write ('No interactive commands allowed here!
') 
         sys.exit (1)



--- a/repo_group.py.old
+++ b/repo_group.py.new
@@ -1,52 +1,46 @@
 #!/usr/bin/env python
 
-import cgi
-import cgitb
-from hg_helper import check_repo_name
+#import cgi
+#import cgitb
 from grp import getgrgid
+import hg_helper
 import os
 import sys
 
-#cgitb.enable()
-cgitb.enable(display=0)
-
-repo_root = "/repo/hg/mozilla"
-
-print "Content-Type: text/plain"
-print
-
-form = cgi.FieldStorage()
-if not form or not form.has_key("repo"):
-    print "Need a repository to check"
-    sys.exit(1)
-
-repo = form.getfirst("repo", "")
-if not check_repo_name(repo):
-    print "You've included some illegal characters in your repo name"
-    sys.stderr.write("Warning: illegal characters in repo name
")
-    sys.exit(1)
-# ensure that the repo is within repo_root
-if repo.find('/../') != -1:
-    print "That's not allowed"
-    sys.stderr.write("Warning: /../ found in a repo name.
")
-    sys.exit(1)
-
-dir = "%s/%s" % (repo_root, repo)
-
-if not (os.path.isdir(dir) and os.path.isdir(dir + "/.hg")):
-    print "%s is not an hg repository" % repo
-    sys.exit(1)
-
-try:
-    fdata = os.stat(dir)
-except:
-    sys.stderr.write("Warning: Couldn't stat %s" % dir)
-    print "Could not read %s" % repo
-    sys.exit(1)
-
-gid = fdata.st_gid
-group = getgrgid(gid)[0]
-print group
+def repo_owner(repo):
+    repo_root = "/repo/hg/mozilla"
+
+    if not repo:
+        print "Need a repository to check"
+        sys.exit(1)
+
+    if not hg_helper.check_repo_name(repo):
+        print "You've included some illegal characters in your repo name"
+        sys.stderr.write("Warning: illegal characters in repo name
")
+        sys.exit(1)
+
+    # ensure that the repo is within repo_root
+    if repo.find('/../') != -1:
+        print "That's not allowed"
+        sys.stderr.write("Warning: /../ found in a repo name.
")
+        sys.exit(1)
+
+    dir = "%s/%s" % (repo_root, repo)
+
+    if not (os.path.isdir(dir) and os.path.isdir(dir + "/.hg")):
+        print "%s is not an hg repository" % repo
+        sys.exit(1)
+
+    try:
+        fdata = os.stat(dir)
+    except:
+        sys.stderr.write("Warning: Couldn't stat %s" % dir)
+        print "Could not read %s" % repo
+        sys.exit(1)
+
+    gid = fdata.st_gid
+    group = getgrgid(gid)[0]
+    return group
 
 # Local variables:
 # mode: python

Comment 3

[Justin Wood (:Callek)]

(In reply to Ben Kero [:bkero] from comment #2)
> --- a/hg_helper.py.old
> +++ b/hg_helper.py.new
> +    elif ssh_command.startswith ('repo-group') and (cname !=
> 'hg.ecmascript.org'):
> +        args = ssh_command.replace ('repo-group', '').split()
> +        if check_repo_name (args[0]):

Python Style Nit: No space before open paren of function arg list

> --- a/repo_group.py.old
> +++ b/repo_group.py.new
> +#import cgi
> +#import cgitb

Nit: do away with commented out lines in final patch

> +    # ensure that the repo is within repo_root
> +    if repo.find('/../') != -1:
> +        print "That's not allowed"
> +        sys.stderr.write("Warning: /../ found in a repo name.
> ")
> +        sys.exit(1)

nit: tweak messages to say something more obvious, e.g. "Error: relative paths in repo name not allowed" -- especially something better than "That's not allowed" (since user has no clue what "That" is)

r+ with those nits and the following notes

------

Two remaining things of note:
* This patch will break the current `curl http://hg.mozilla.org/repo-group?repo=/mozilla-central` support, so if http (anonymous) checking of repo group is something we want to still support this can't deploy as is.
* I was told in IRC that the http:// format is intended to stay, with an upcoming patch from bkero, and be done via an ssh call from the webheads, this is something I feel will/should have a sec review both on this patch and the upcoming one if it was to happen - since anonymous users, based purely on a webhead load, can then be causing ssh connections to happen.
** I have no concerns on the sec nature of this beyond making sure the sec team gets a chance to voice their concerns [if any], so if they don't find any concerns I don't have a single reason to block.

Comment 4

[Ben Kero [:bkero]]

This was talked about (on IRC) and landed earlier this week.