Phil Miller <mille121@xxxxxxxxxxxx> writes:
> The Gitosis single-account Git/ssh hosting system runs git commands
> through git-shell after confirming that the connecting user is
> authorized to access the requested repository. This works well for
> upload-pack and receive-pack, which take a repository argument through
> git-shell. This doesn't work so well for `cvs server', which is passed
> through literally, with no arguments. Allowing arguments risks
> sneaking in `--export-all', so that restriction should be maintained.
>
> Despite that, passing a list of repository roots is necessary for
> per-user access control by the hosting software, and passing a base
> path improves usability without weakening security. Thus,
> git-cvsserver needs to come up with these values at runtime by some
> other means. Since git-shell preserves the environment for other
> purposes, the environment can carry these arguments as well.
>
> Thus, modify git-cvsserver to read $GIT_CVSSERVER_{BASE_PATH,ROOTS} in
> the absence of equivalent command line arguments.
>
> Signed-off-by: Phil Miller <mille121@xxxxxxxxxxxx>
> ---
Thanks.
Any comments from cvsserver users?
> git-cvsserver.perl | 21 ++++++++++++++++++++-
> 1 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index 6dc45f5..9e58d2a 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -104,6 +104,7 @@ $log->info("--------------- STARTING -----------------");
> my $usage =
> "Usage: git cvsserver [options] [pserver|server] [<directory> ...]\n".
> " --base-path <path> : Prepend to requested CVSROOT\n".
> + " Can be read from GIT_CVSSERVER_BASE_PATH\n".
> " --strict-paths : Don't allow recursing into subdirectories\n".
> " --export-all : Don't check for gitcvs.enabled in config\n".
> " --version, -V : Print version information and exit\n".
> @@ -111,7 +112,8 @@ my $usage =
> "\n".
> "<directory> ... is a list of allowed directories. If no directories\n".
> "are given, all are allowed. This is an additional restriction, gitcvs\n".
> - "access still needs to be enabled by the gitcvs.enabled config option.\n";
> + "access still needs to be enabled by the gitcvs.enabled config option.\n".
> + "Alternately, these directories may be specified in
> GIT_CVSSERVER_ROOTS.\n";
When you introduce a single variable holding multiple values, you need to
document how to cram the values into it. Maybe such a detail isn't
necessary in this usage text, but definitely in the documentation.
Documentation/git-cvsserver.txt needs to be updated to describe the
features added by this patch.
By the way, this patch is line-wrapped. Here, and...
> my @opts = ( 'help|h|H', 'version|V',
> 'base-path=s', 'strict-paths', 'export-all' );
> @@ -148,6 +150,23 @@ if ($state->{'export-all'} &&
> !@{$state->{allowed_roots}}) {
... also here.
> die "--export-all can only be used together with an explicit whitelist\n";
> }
>
> +# Environment handling for running under git-shell
> +if ($ENV{GIT_CVSSERVER_BASE_PATH}) {
It probably is more kosher to say
if (exists $ENV{...})
as the base_path _could_ be '0' that evaluates to false. When the path
does not begin with a '/', what will it be relative to? Do we want to
document it (not a rhetorical question)?
> + if ($state->{'base-path'}) {
> + die "Cannot specify base path both ways.\n";
> + }
> + my $base_path = $ENV{GIT_CVSSERVER_BASE_PATH};
> + $state->{'base-path'} = $base_path;
> + $log->debug("Picked up base path '$base_path' from environment.\n");
> +}
> +if ($ENV{GIT_CVSSERVER_ROOTS}) {
> + if (@{$state->{allowed_roots}}) {
> + die "Cannot specify roots both ways: @ARGV\n";
> + }
> + my @allowed_root = split(',', $ENV{GIT_CVSSERVER_ROOTS});
> + $state->{allowed_roots} = [ @allowed_root ];
Even though a comma is probably rare in pathname components, I do not know
if this is good.
How much thought went into choosing comma for this purpose? Is there a
particular reason you chose ',' as the separator? That should be
documented in the commit log message.
Logical alternative choices are "split at whitespace" (mimics the way how
command line argument splitting works) and "colon" (mimics the way how
$PATH is split into component paths).
If a pathname component with whitespaces (Windows and Macs?) is not an
issue, I would imagine "split at whitespace" is much more natural way to
handle this, but probably many people have "My Programs" and such.
Especially because it is hard, if not impossible, to have a pathname
component that contains a colon on Windows, I suspect that a colon is much
rare compared to whitespaces and commas in the name of people's
directories and files. It might be better to split at colon like $PATH is
handled than using a comma, if you are not going to give any escape
mechanism to .
--
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