Sitaram Chamarty <sitaramc@xxxxxxxxx> writes: > I created a new project called gitosis-lite, which combines > the essential pieces of gitosis with the per-branch > permissions stuff in the example in the howto directory of > git.git. As for the name: gitness, gitamine, gitrify,... ;-) > The config file is different, (there's an annotated example > you can look at). > > The "why" and the "what" are all at > http://github.com/sitaramc/gitosis-lite A few comments about the code, taking gl-auth-command as example. > #!/usr/bin/perl -w > > use strict; Wouldn't it be better to use "use warnings" instead of 'perl -w'? > # === auth-command === > # the command that GL users actually run > > # part of the gitosis-lite (GL) suite > > # how run: via sshd, being listed in "command=" in ssh authkeys > # when: every login by a GL user > # input: $1 is GL username, plus $SSH_ORIGINAL_COMMAND > # output: > # security: > # - currently, we just make some basic checks, copied from gitosis > > # robustness: > > # other notes: It would be, I think, better if you have used POD for such documentation. One would be able to generate manpage using pod2man, and it is no less readable in source code. See e.g. perl/Git.pm or contrib/hooks/update-paranoid. > our $GL_ADMINDIR; > our $GL_CONF; > our $GL_KEYDIR; > our $GL_CONF_COMPILED; > our $REPO_BASE; > our %repos; Why is the reason behind using 'our' instead of 'my' here? > # first, fix the biggest gripe I have with gitosis, a 1-line change > my $user=$ENV{GL_USER}=shift; # there; now that's available everywhere! Eh? This is standalone script, isn't it? Shouldn't it be my $user = $ENV{GL_USER} = $ARGV[0]; # there; now that's available everywhere! > my $perm = 'W'; $perm = 'R' if $verb =~ $R_COMMANDS; Either split it into two lines, or use ?: confitional operator: my $perm = ($verb =~ $R_COMMANDS ? 'R' : 'W'); > open(LOG, ">>", "$GL_ADMINDIR/log"); > print LOG "\n", scalar(localtime), " $ENV{SSH_ORIGINAL_COMMAND} $user\n"; > close(LOG); It is better practice to use lexical variables instead of barewords for filehandles: if (open my $logfh, ">>", "$GL_ADMINDIR/log") { print $logfh "\n", scalar(localtime), " $ENV{SSH_ORIGINAL_COMMAND} $user\n"; close $logfh; } Don't forget to check for error. > $repo = "'$REPO_BASE/$repo.git'"; > exec("git", "shell", "-c", "$verb $repo"); That's not enough. You have to shell-quote $repo, like in gitweb or using String::ShellQuote module, or somehow use list form to pass arguments to git-shell. You protect here againts spaces in filename, but not againts "'" (single quote) and for show shells "!" (exclamation mark). -- Jakub Narebski Poland ShadeHawk on #git -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html