* Re: inotify to minimize stat() calls
From: Duy Nguyen @ 2013-02-11 3:21 UTC (permalink / raw)
To: Robert Zeh; +Cc: Junio C Hamano, Ramkumar Ramachandra, Git List
In-Reply-To: <CAKXa9=qQwJqxZLxhAS35QeF1+dwH+ukod0NfFggVCuUZHz-USg@mail.gmail.com>
On Mon, Feb 11, 2013 at 2:03 AM, Robert Zeh <robert.allan.zeh@gmail.com> wrote:
> On Sat, Feb 9, 2013 at 1:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>>
>>> This is much better than Junio's suggestion to study possible
>>> implementations on all platforms and designing a generic daemon/
>>> communication channel. That's no weekend project.
>>
>> It appears that you misunderstood what I wrote. That was not "here
>> is a design; I want it in my system. Go implemment it".
>>
>> It was "If somebody wants to discuss it but does not know where to
>> begin, doing a small experiment like this and reporting how well it
>> worked here may be one way to do so.", nothing more.
>
> What if instead of communicating over a socket, the daemon
> dumped a file containing all of the lstat information after git
> wrote a file? By definition the daemon should know about file writes.
>
> There would be no network communication, which I think would make
> things more secure. It would simplify the rendezvous by insisting on
> well known locations in $GIT_DIR.
We need some sort of interactive communication to the daemon anyway,
to validate that the information is uptodate. Assume that a user makes
some changes to his worktree before starting the daemon, git needs to
know that what the daemon provides does not represent a complete
file-change picture and it better refreshes the index the old way
once, then trust the daemon.
I think we could solve that by storing a "session id", provided by the
daemon, in .git/index. If the session id is not present (or does not
match what the current daemon gives), refresh the old way. After
refreshing, it may ask the daemon for new session id and store it.
Next time if the session id is still valid, trust the daemon's data.
This session id should be different every time the daemon restarts for
this to work.
--
Duy
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Junio C Hamano @ 2013-02-11 3:44 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211012016.GA13243@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> How about this?
>
> A patch on top could change the default "git-shell-commands is not
> present" message if that seems worthwhile.
Hmph.
I wonder if rewording the message when git-shell-commmands directory
is not there may be a better first step (which actually could be the
last step)?
That is, showing something like this,
> +printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"
> +printf '%s\n' "provide interactive shell access."
but rephrased with a reference to "git help shell" for people
preparing their own server when ~/git-shell-commands/ in good order?
Something like
diff --git a/shell.c b/shell.c
index 84b237f..71ff04f 100644
--- a/shell.c
+++ b/shell.c
@@ -162,9 +162,11 @@ int main(int argc, char **argv)
/* Allow the user to run an interactive shell */
cd_to_homedir();
if (access(COMMAND_DIR, R_OK | X_OK) == -1) {
- die("Interactive git shell is not enabled.\n"
+ die("The user has been recognized as '%s' but "
+ "interactive git shell is not enabled.\n"
"hint: ~/" COMMAND_DIR " should exist "
- "and have read and execute access.");
+ "and have read and execute access.",
+ get_user_name());
}
run_shell();
exit(0);
^ permalink raw reply related
* Re: inotify to minimize stat() calls
From: Duy Nguyen @ 2013-02-11 3:53 UTC (permalink / raw)
To: kusmabite
Cc: Ramkumar Ramachandra, Robert Zeh, Junio C Hamano, Git List,
finnag, Karsten Blees
In-Reply-To: <CABPQNSZ282Lre=sy-+ZQdJA9JnGqQguq2bQDOwvjb0fP+1-w8Q@mail.gmail.com>
On Sun, Feb 10, 2013 at 11:58 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> Karsten Blees has done something similar-ish on Windows, and he posted
> the results here:
>
> https://groups.google.com/forum/#!topic/msysgit/fL_jykUmUNE/discussion
>
> I also seem to remember he doing a ReadDirectoryChangesW version, but
> I don't remember what happened with that.
Thanks. I came across that but did not remember. For one thing, we
know the inotify alternative for Windows: ReadDirectoryChangesW.
But the meat of the patch is not about that function. In fact it's
dropped in fscache-v3 [1]. It seems that doing
FindFirstFile/FindNextFile for an entire directory, cache the results
and use it to simulate lstat() is faster on Windows. Sounds similar to
preload-index. And because directory listing is cached anyway,
opendir/readdir is replaced to read from cache instead of opening the
directory again.
So it is orthogonal with using ReadDirectoryChangesW/inotify to
further reduce the system calls.
I copy "git status"'s (impressive) numbers from fscache-v0 for those
who are interested in:
preload | -u | normal | cached | gain
--------+-----+--------+--------+------
false | all | 25.144 | 3.055 | 8.2
false | no | 22.822 | 1.748 | 12.8
true | all | 9.234 | 2.179 | 4.2
true | no | 6.833 | 0.955 | 7.2
[1] https://github.com/kblees/git/commit/35f319609aa046d2350db32d3afa1fa44920e880
--
Duy
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jeff King @ 2013-02-11 3:59 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sitaram Chamarty, Junio C Hamano, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211012016.GA13243@elie.Belkin>
On Sun, Feb 10, 2013 at 05:20:16PM -0800, Jonathan Nieder wrote:
> > This of course now means that the ~/git-shell-commands should not be
> > empty, since that is where this default command also will be present.
>
> How about this?
I like the general direction this is going, but:
> +When run interactively (with no arguments), 'git-shell' will
> +automatically run `~/git-shell-commands/help` on startup, provided it
> +exists. If the 'help' command fails then the interactive shell is
> +aborted.
Doesn't that mean that people who currently do allow interactive access
and have a ~/git-shell-commands/help (that returns zero) will get
spammed by its as a motd each time they connect?
To be honest, I am not really clear on what interactive git-shell is
used for. AFAIK, it does nothing unless you have set up custom commands,
and I have never actually seen them in the wild. So maybe it is not a
big deal.
If I understand correctly, calling it "check-interactive", "greeting",
or something instead of "help" would be sufficient, and then you
wouldn't have to worry about backwards compatibility.
> diff --git a/shell.c b/shell.c
> index 84b237fe..3abc2b84 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -63,10 +63,16 @@ static void cd_to_homedir(void)
>
> static void run_shell(void)
> {
> - int done = 0;
> + int done = 0, status;
> static const char *help_argv[] = { HELP_COMMAND, NULL };
> /* Print help if enabled */
> - run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> + status = run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> + if (!status)
> + ; /* success */
> + else if (status == -1 && errno == ENOENT)
> + ; /* help disabled */
> + else
> + exit(status);
This kicks in only when there is no command given, right? So if I ran
"ssh example.com", it would give me the help message rather than (or in
addition) giving me interactive access.
What about "ssh example.com foo"? Do we want to allow a custom message
there, too (it might be different there; e.g., an allowed list of
commands might make more sense)?
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 4:14 UTC (permalink / raw)
To: Jeff King
Cc: Sitaram Chamarty, Junio C Hamano, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211035908.GA4543@sigill.intra.peff.net>
Jeff King wrote:
> On Sun, Feb 10, 2013 at 05:20:16PM -0800, Jonathan Nieder wrote:
>> +When run interactively (with no arguments), 'git-shell' will
>> +automatically run `~/git-shell-commands/help` on startup, provided it
>> +exists. If the 'help' command fails then the interactive shell is
>> +aborted.
>
> Doesn't that mean that people who currently do allow interactive access
> and have a ~/git-shell-commands/help (that returns zero) will get
> spammed by its as a motd each time they connect?
Only interactive connections. That's the existing behavior.
[...]
> What about "ssh example.com foo"? Do we want to allow a custom message
> there, too (it might be different there; e.g., an allowed list of
> commands might make more sense)?
I wouldn't mind, but it's definitely not my itch.
Hoping that clarifies,
Jonathan
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 4:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7v7gmfqzt1.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> How about this?
>>
>> A patch on top could change the default "git-shell-commands is not
>> present" message if that seems worthwhile.
>
> Hmph.
>
> I wonder if rewording the message when git-shell-commmands directory
> is not there may be a better first step (which actually could be the
> last step)?
Maybe, but it's not a step that I'm interested in. I don't think it
changes the desirability of the patch I sent. They are independent.
[...]
> --- a/shell.c
> +++ b/shell.c
> @@ -162,9 +162,11 @@ int main(int argc, char **argv)
> /* Allow the user to run an interactive shell */
> cd_to_homedir();
> if (access(COMMAND_DIR, R_OK | X_OK) == -1) {
> - die("Interactive git shell is not enabled.\n"
> + die("The user has been recognized as '%s' but "
> + "interactive git shell is not enabled.\n"
> "hint: ~/" COMMAND_DIR " should exist "
> - "and have read and execute access.");
> + "and have read and execute access.",
> + get_user_name());
Personally I don't think the hint should be here at all (it should be
obvious that git-shell(1) is the place to read about the login
behavior of an account with shell set to git-shell), but I don't mind
as long as it's possible to override the message.
Thanks,
Jonathan
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jeff King @ 2013-02-11 4:17 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sitaram Chamarty, Junio C Hamano, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211041404.GA15329@elie.Belkin>
On Sun, Feb 10, 2013 at 08:14:04PM -0800, Jonathan Nieder wrote:
> Jeff King wrote:
> > On Sun, Feb 10, 2013 at 05:20:16PM -0800, Jonathan Nieder wrote:
>
> >> +When run interactively (with no arguments), 'git-shell' will
> >> +automatically run `~/git-shell-commands/help` on startup, provided it
> >> +exists. If the 'help' command fails then the interactive shell is
> >> +aborted.
> >
> > Doesn't that mean that people who currently do allow interactive access
> > and have a ~/git-shell-commands/help (that returns zero) will get
> > spammed by its as a motd each time they connect?
>
> Only interactive connections. That's the existing behavior.
Ah, sorry. I misread the patch. I see now that we already run help, and
this is just making the exit value significant. In that case, yeah, I
think it's fine.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 4:26 UTC (permalink / raw)
To: Jeff King
Cc: Sitaram Chamarty, Junio C Hamano, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211041714.GA12281@sigill.intra.peff.net>
Jeff King wrote:
> On Sun, Feb 10, 2013 at 08:14:04PM -0800, Jonathan Nieder wrote:
>> Only interactive connections. That's the existing behavior.
>
> Ah, sorry. I misread the patch. I see now that we already run help, and
> this is just making the exit value significant. In that case, yeah, I
> think it's fine.
No problem --- the description was unclear. Would retitling the patch
to "shell: pay attention to exit status from 'help' command" work?
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Junio C Hamano @ 2013-02-11 4:30 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211041706.GB15329@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> How about this?
>>>
>>> A patch on top could change the default "git-shell-commands is not
>>> present" message if that seems worthwhile.
>>
>> Hmph.
>>
>> I wonder if rewording the message when git-shell-commmands directory
>> is not there may be a better first step (which actually could be the
>> last step)?
>
> Maybe, but it's not a step that I'm interested in. I don't think it
> changes the desirability of the patch I sent. They are independent.
What I thought I read in the log message was that you wanted to give
a better message telling the users that the site does _not_ allow an
interactive shell access. I do not see how that is independent from
a message given from this codepath, where the side has forbidden
shell access by not having ~/git-shell-commands directory in the
first place. Are you shooting for customizability?
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 4:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7vwqufpj50.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Are you shooting for customizability?
Yes, and the ability to generate the message dynamically.
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jeff King @ 2013-02-11 4:33 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sitaram Chamarty, Junio C Hamano, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211042609.GC15329@elie.Belkin>
On Sun, Feb 10, 2013 at 08:26:09PM -0800, Jonathan Nieder wrote:
> Jeff King wrote:
> > On Sun, Feb 10, 2013 at 08:14:04PM -0800, Jonathan Nieder wrote:
>
> >> Only interactive connections. That's the existing behavior.
> >
> > Ah, sorry. I misread the patch. I see now that we already run help, and
> > this is just making the exit value significant. In that case, yeah, I
> > think it's fine.
>
> No problem --- the description was unclear. Would retitling the patch
> to "shell: pay attention to exit status from 'help' command" work?
I think what threw me off was reading the documentation part of the
patch, which adds a note that we run "help" on startup, and then
elaborates on the exit value. I didn't realize that the first half was
documenting what already happened.
Tweaking the third paragraph of the commit message to:
An appropriate greeting might even include more complex information,
like a list of repositories the user has access to. If the
git-shell-commands directory exists and contains a "help" script, we
already run it when the shell is run without any commands, giving the
server a chance to provide a custom message. Unfortunately, the
presence of the git-shell-commands directory means we also enter an
interactive mode, prompting and accepting commands (of which there may
be none) from the user, which many servers would not want. To solve
this, we abort the interactive shell on a non-zero exit code from the
"help" script. This lets the server say whatever it likes, and then
hangup.
makes it more clear to me. But once you explained it, I realize that I
also could have just read the C code part of the patch more carefully. :)
So I'm fine with or without that change.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jeff King @ 2013-02-11 4:36 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Sitaram Chamarty, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211043247.GD15329@elie.Belkin>
On Sun, Feb 10, 2013 at 08:32:47PM -0800, Jonathan Nieder wrote:
> Junio C Hamano wrote:
>
> > Are you shooting for customizability?
>
> Yes, and the ability to generate the message dynamically.
As far as the default goes, I think the current one is OK, provided
there is an option to customize it (e.g., like your patch). Right now it
is just nonsensical to random users ("What? What in the world is
~/git-shell-commands?"). But once it is customizable, the main consumer
of the message is admins who say "What? Why isn't the git-shell I just
set up working?". The current message helps them diagnose the problem,
and when they are ready to accept connections from random users, they'll
want something customizable anyway.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jeff King @ 2013-02-11 4:45 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sitaram Chamarty, Junio C Hamano, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211012016.GA13243@elie.Belkin>
On Sun, Feb 10, 2013 at 05:20:16PM -0800, Jonathan Nieder wrote:
> diff --git a/shell.c b/shell.c
> index 84b237fe..3abc2b84 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -63,10 +63,16 @@ static void cd_to_homedir(void)
>
> static void run_shell(void)
> {
> - int done = 0;
> + int done = 0, status;
> static const char *help_argv[] = { HELP_COMMAND, NULL };
> /* Print help if enabled */
> - run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> + status = run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> + if (!status)
> + ; /* success */
> + else if (status == -1 && errno == ENOENT)
> + ; /* help disabled */
> + else
> + exit(status);
One final comment on this. I believe we convert an exit code of 127 from
the child into ENOENT. So something like:
#!/bin/sh
echo >&2 "Sorry, no interactive shells allowed."
exti 1
would actually go into the "help disabled" code path and accidentally
run an interactive shell. I wondered if this is something that might
happen accidentally (since the old semantics of "help" were that exit
code did not matter), and if there might be security implications to
entering an interactive shell. But I think we are OK for two reasons:
1. An old script would not be trying to exit with failure and
expecting to abort the interactive session; that is a new feature
you are adding. So even if we accidentally exit 127 (because the
old script relied on a missing command), it is not changing the
semantics.
2. Even if we accidentally do enter the interactive prompt, it should
not be a security issue. It is not like you can then run arbitrary
commands; unless you have put something else into
~/git-shell-commands, the user can only run "help" over and over.
Maybe obvious, but I wanted to note it as part of the review. I think we
need to be extra careful with thinking through git-shell security
implications, since it is a major potential attack surface for many git
setups.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Junio C Hamano @ 2013-02-11 5:22 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211043247.GD15329@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>
>> Are you shooting for customizability?
>
> Yes, and the ability to generate the message dynamically.
Hmph, if that is the case, wouldn't it be a better direction to give
a better help for majority of the case where git-shell is used as
the login shell to allow push and fetch but not for interactive
access at all?
The first step in that direction may be to give a better canned
message, followed by a mechanism (perhaps a hook) that lets a
message customized for the site's needs, no? Why should a site
administrator create an otherwise empty directory for each and every
user and add an executable in there that shows an error message,
only to improve the default message because it is not friendly
enough?
I may be being slower than usual, but I am still not convinced...
^ permalink raw reply
* [PATCH 0/2 v2] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 5:56 UTC (permalink / raw)
To: Jeff King
Cc: Sitaram Chamarty, Junio C Hamano, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211043322.GA12735@sigill.intra.peff.net>
Jeff King wrote:
> I think what threw me off was reading the documentation part of the
> patch, which adds a note that we run "help" on startup, and then
> elaborates on the exit value. I didn't realize that the first half was
> documenting what already happened.
>
> Tweaking the third paragraph of the commit message to:
Very nice. How about this version?
Jonathan Nieder (2):
shell doc: emphasize purpose and security guarantees
shell: pay attention to exit status from 'help' command
Documentation/git-shell.txt | 86 +++++++++++++++++++++++++++++++++++++--------
shell.c | 10 ++++--
2 files changed, 79 insertions(+), 17 deletions(-)
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Ethan Reesor @ 2013-02-11 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Sitaram Chamarty, Jeff King, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7vpq07pgpy.fsf@alter.siamese.dyndns.org>
On Mon, Feb 11, 2013 at 12:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Hmph, if that is the case, wouldn't it be a better direction to give
> a better help for majority of the case where git-shell is used as
> the login shell to allow push and fetch but not for interactive
> access at all?
>
> The first step in that direction may be to give a better canned
> message, followed by a mechanism (perhaps a hook) that lets a
> message customized for the site's needs, no? Why should a site
> administrator create an otherwise empty directory for each and every
> user and add an executable in there that shows an error message,
> only to improve the default message because it is not friendly
> enough?
Jonathan made the following comment on the thread I started that lead
to this RFC:
> You can disable interactive logins by removing the
> ~/git-shell-commands/ directory. Unfortunately that doesn't let you
> customize the message. Perhaps it would make sense to teach shell.c
> to look for a
>
> [shell]
> greeting = 'Hi %(username)! You've successfully authenticated, but I do not provide interactive shell access.'
>
> setting in git's config file.
How is this for an alternative? Have shell.c look for
[shell]
missing_commands_directory = "Stuff is broke."
setting. If the setting is missing, then it prints the default message
(the current message). That way, there's a default setting, there can
be a system-wide message, there can be a user specific message, and
those messages can be set via `git-commit`.
--
Ethan Reesor
^ permalink raw reply
* [PATCH 1/2] shell doc: emphasize purpose and security model
From: Jonathan Nieder @ 2013-02-11 5:57 UTC (permalink / raw)
To: Jeff King
Cc: Sitaram Chamarty, Junio C Hamano, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211055604.GE15329@elie.Belkin>
The original git-shell(1) manpage emphasized that the shell
supports only git transport commands, and as the shell gained
features that emphasis and focus in the manual has been lost.
Bring it back by splitting the manpage into a few short sections
and fleshing out each:
- SYNOPSIS, describing how the shell gets used in practice
- DESCRIPTION, which gives an overview of the purpose and
guarantees provided by this restricted shell
- COMMANDS, listing supported commands and restrictions on the
arguments they accept
- INTERACTIVE USE, describing the interactive mode
Also add a "see also" section with some relevant related reading.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
New text. Split off from patch 2 --- this is just documenting
existing behavior.
Documentation/git-shell.txt | 66 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 51 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
index 9b925060..4fe93203 100644
--- a/Documentation/git-shell.txt
+++ b/Documentation/git-shell.txt
@@ -9,25 +9,61 @@ git-shell - Restricted login shell for Git-only SSH access
SYNOPSIS
--------
[verse]
-'git shell' [-c <command> <argument>]
+'chsh' -s $(which git-shell) git
+'git clone' `git@localhost:/path/to/repo.git`
+'ssh' `git@localhost`
DESCRIPTION
-----------
-A login shell for SSH accounts to provide restricted Git access. When
-'-c' is given, the program executes <command> non-interactively;
-<command> can be one of 'git receive-pack', 'git upload-pack', 'git
-upload-archive', 'cvs server', or a command in COMMAND_DIR. The shell
-is started in interactive mode when no arguments are given; in this
-case, COMMAND_DIR must exist, and any of the executables in it can be
-invoked.
-
-'cvs server' is a special command which executes git-cvsserver.
-
-COMMAND_DIR is the path "$HOME/git-shell-commands". The user must have
-read and execute permissions to the directory in order to execute the
-programs in it. The programs are executed with a cwd of $HOME, and
-<argument> is parsed as a command-line string.
+This is a login shell for SSH accounts to provide restricted Git access.
+It permits execution only of server-side Git commands implementing the
+pull/push functionality, plus custom commands present in a subdirectory
+named `git-shell-commands` in the user's home directory.
+
+COMMANDS
+--------
+
+'git shell' accepts the following commands after the '-c' option:
+
+'git receive-pack <argument>'::
+'git upload-pack <argument>'::
+'git upload-archive <argument>'::
+ Call the corresponding server-side command to support
+ the client's 'git push', 'git fetch', or 'git archive --remote'
+ request.
+'cvs server'::
+ Imitate a CVS server. See linkgit:git-cvsserver[1].
+
+If a `~/git-shell-commands` directory is present, 'git shell' will
+also handle other, custom commands by running
+"`git-shell-commands/<command> <arguments>`" from the user's home
+directory.
+
+INTERACTIVE USE
+---------------
+
+By default, the commands above can be executed only with the '-c'
+option; the shell is not interactive.
+
+If a `~/git-shell-commands` directory is present, 'git shell'
+can also be run interactively (with no arguments). If a `help`
+command is present in the `git-shell-commands` directory, it is
+run to provide the user with an overview of allowed actions. Then a
+"`git> `" prompt is presented at which one can enter any of the
+commands from the `git-shell-commands` directory, or `exit` to close
+the connection.
+
+Generally this mode is used as an administrative interface to allow
+users to list repositories they have access to, create, delete, or
+rename repositories, or change repository descriptions and
+permissions.
+
+SEE ALSO
+--------
+ssh(1),
+linkgit:git-daemon[1],
+contrib/git-shell-commands/README
GIT
---
--
1.8.1.3
^ permalink raw reply related
* [PATCH 2/2] shell: pay attention to exit status from 'help' command
From: Jonathan Nieder @ 2013-02-11 5:58 UTC (permalink / raw)
To: Jeff King
Cc: Sitaram Chamarty, Junio C Hamano, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211055604.GE15329@elie.Belkin>
If I disable git-shell's interactive mode by removing the
~/git-shell-commands directory, then attempts to use 'ssh' with the
git account interactively produce an error message intended for the
administrator:
$ ssh git@myserver
fatal: Interactive git shell is not enabled.
hint: ~/git-shell-commands should exist and have read and execute access.
$
That is helpful for the new admin who is wondering "What? Why isn't
the git-shell I just set up working?", but once the site setup is
finished, it is better to give the user a friendly hint that she is on
the right track, like GitHub does:
Hi <username>! You've successfully authenticated, but
GitHub does not provide shell access.
An appropriate greeting might even include more complex information,
like a list of repositories the user has access to. If the
git-shell-commands directory exists and contains a "help" script, we
already run it when the shell is run without any commands, giving the
server a chance to provide a custom message. Unfortunately, the
presence of the git-shell-commands directory means we also enter an
interactive mode, prompting and accepting commands (of which there may
be none) from the user, which many servers would not want. To solve
this, we abort the interactive shell on a non-zero exit code from the
"help" script. This lets the server say whatever it likes, and then
hang up.
Downside: this will prevent interactive git-shell logins in existing
setups where the "help" script exits with nonzero status by mistake.
Hopefully those are rare enough to not cause much trouble in practice.
Reported-by: Ethan Reesor <firelizzard@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Improved-by: Jeff King <peff@peff.net>
---
Documentation/git-shell.txt | 20 ++++++++++++++++++++
shell.c | 10 ++++++++--
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
index 4fe93203..60051e63 100644
--- a/Documentation/git-shell.txt
+++ b/Documentation/git-shell.txt
@@ -59,6 +59,26 @@ users to list repositories they have access to, create, delete, or
rename repositories, or change repository descriptions and
permissions.
+If the `help` command exists and exits with nonzero status, the
+interactive shell is aborted.
+
+EXAMPLE
+-------
+
+To disable interactive logins, displaying a greeting instead:
++
+----------------
+$ chsh -s /usr/bin/git-shell
+$ mkdir $HOME/git-shell-commands
+$ cat >$HOME/git-shell-commands/help <<\EOF
+#!/bin/sh
+printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"
+printf '%s\n' "provide interactive shell access."
+exit 128
+EOF
+$ chmod +x $HOME/git-shell-commands/help
+----------------
+
SEE ALSO
--------
ssh(1),
diff --git a/shell.c b/shell.c
index 84b237fe..3abc2b84 100644
--- a/shell.c
+++ b/shell.c
@@ -63,10 +63,16 @@ static void cd_to_homedir(void)
static void run_shell(void)
{
- int done = 0;
+ int done = 0, status;
static const char *help_argv[] = { HELP_COMMAND, NULL };
/* Print help if enabled */
- run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
+ status = run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
+ if (!status)
+ ; /* success */
+ else if (status == -1 && errno == ENOENT)
+ ; /* help disabled */
+ else
+ exit(status);
do {
struct strbuf line = STRBUF_INIT;
--
1.8.1.3
^ permalink raw reply related
* Re: [PATCH 2/2] shell: pay attention to exit status from 'help' command
From: Ethan Reesor @ 2013-02-11 6:06 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jeff King, Sitaram Chamarty, Junio C Hamano, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211055847.GG15329@elie.Belkin>
I feel like the suggestion I posted in response to Junio C Hamano
<gitster@pobox.com>'s complaint on the RFC for this patch provides a
more elegant solution to the problem of administrators wanting to
prevent interactive sessions for users with their login shell set to
git-prompt. The suggestion was as follows:
> How is this for an alternative? Have shell.c look for a
> [shell]
> missing_commands_directory = "Stuff is broke."
> setting. If the setting is missing, then it prints the default message
> (the current message). That way, there's a default setting, there can
> be a system-wide message, there can be a user specific message, and
> those messages can be set via `git-config`.
On Mon, Feb 11, 2013 at 12:58 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> If I disable git-shell's interactive mode by removing the
> ~/git-shell-commands directory, then attempts to use 'ssh' with the
> git account interactively produce an error message intended for the
> administrator:
>
> $ ssh git@myserver
> fatal: Interactive git shell is not enabled.
> hint: ~/git-shell-commands should exist and have read and execute access.
> $
>
> That is helpful for the new admin who is wondering "What? Why isn't
> the git-shell I just set up working?", but once the site setup is
> finished, it is better to give the user a friendly hint that she is on
> the right track, like GitHub does:
>
> Hi <username>! You've successfully authenticated, but
> GitHub does not provide shell access.
>
> An appropriate greeting might even include more complex information,
> like a list of repositories the user has access to. If the
> git-shell-commands directory exists and contains a "help" script, we
> already run it when the shell is run without any commands, giving the
> server a chance to provide a custom message. Unfortunately, the
> presence of the git-shell-commands directory means we also enter an
> interactive mode, prompting and accepting commands (of which there may
> be none) from the user, which many servers would not want. To solve
> this, we abort the interactive shell on a non-zero exit code from the
> "help" script. This lets the server say whatever it likes, and then
> hang up.
>
> Downside: this will prevent interactive git-shell logins in existing
> setups where the "help" script exits with nonzero status by mistake.
> Hopefully those are rare enough to not cause much trouble in practice.
>
> Reported-by: Ethan Reesor <firelizzard@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Improved-by: Jeff King <peff@peff.net>
> ---
> Documentation/git-shell.txt | 20 ++++++++++++++++++++
> shell.c | 10 ++++++++--
> 2 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
> index 4fe93203..60051e63 100644
> --- a/Documentation/git-shell.txt
> +++ b/Documentation/git-shell.txt
> @@ -59,6 +59,26 @@ users to list repositories they have access to, create, delete, or
> rename repositories, or change repository descriptions and
> permissions.
>
> +If the `help` command exists and exits with nonzero status, the
> +interactive shell is aborted.
> +
> +EXAMPLE
> +-------
> +
> +To disable interactive logins, displaying a greeting instead:
> ++
> +----------------
> +$ chsh -s /usr/bin/git-shell
> +$ mkdir $HOME/git-shell-commands
> +$ cat >$HOME/git-shell-commands/help <<\EOF
> +#!/bin/sh
> +printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"
> +printf '%s\n' "provide interactive shell access."
> +exit 128
> +EOF
> +$ chmod +x $HOME/git-shell-commands/help
> +----------------
> +
> SEE ALSO
> --------
> ssh(1),
> diff --git a/shell.c b/shell.c
> index 84b237fe..3abc2b84 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -63,10 +63,16 @@ static void cd_to_homedir(void)
>
> static void run_shell(void)
> {
> - int done = 0;
> + int done = 0, status;
> static const char *help_argv[] = { HELP_COMMAND, NULL };
> /* Print help if enabled */
> - run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> + status = run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> + if (!status)
> + ; /* success */
> + else if (status == -1 && errno == ENOENT)
> + ; /* help disabled */
> + else
> + exit(status);
>
> do {
> struct strbuf line = STRBUF_INIT;
> --
> 1.8.1.3
>
--
Ethan Reesor (Gmail)
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Ethan Reesor @ 2013-02-11 6:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Sitaram Chamarty, Jeff King, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <CAE_TNim2wrL3SWxy_2ugyGmEFDngBJ8+z04y2tJFzMo4N8mUug@mail.gmail.com>
I noticed a typo I made. I meant `git-config` rather than
`git-commit`. Sorry for my mistake.
On Mon, Feb 11, 2013 at 12:57 AM, Ethan Reesor <firelizzard@gmail.com> wrote:
> On Mon, Feb 11, 2013 at 12:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Hmph, if that is the case, wouldn't it be a better direction to give
>> a better help for majority of the case where git-shell is used as
>> the login shell to allow push and fetch but not for interactive
>> access at all?
>>
>> The first step in that direction may be to give a better canned
>> message, followed by a mechanism (perhaps a hook) that lets a
>> message customized for the site's needs, no? Why should a site
>> administrator create an otherwise empty directory for each and every
>> user and add an executable in there that shows an error message,
>> only to improve the default message because it is not friendly
>> enough?
>
> Jonathan made the following comment on the thread I started that lead
> to this RFC:
>> You can disable interactive logins by removing the
>> ~/git-shell-commands/ directory. Unfortunately that doesn't let you
>> customize the message. Perhaps it would make sense to teach shell.c
>> to look for a
>>
>> [shell]
>> greeting = 'Hi %(username)! You've successfully authenticated, but I do not provide interactive shell access.'
>>
>> setting in git's config file.
>
> How is this for an alternative? Have shell.c look for
> [shell]
> missing_commands_directory = "Stuff is broke."
> setting. If the setting is missing, then it prints the default message
> (the current message). That way, there's a default setting, there can
> be a system-wide message, there can be a user specific message, and
> those messages can be set via `git-commit`.
>
> --
> Ethan Reesor
--
Ethan Reesor (Gmail)
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 6:09 UTC (permalink / raw)
To: Ethan Reesor
Cc: Junio C Hamano, Sitaram Chamarty, Jeff King, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <CAE_TNim2wrL3SWxy_2ugyGmEFDngBJ8+z04y2tJFzMo4N8mUug@mail.gmail.com>
Ethan Reesor wrote:
> That way, there's a default setting, there can
> be a system-wide message, there can be a user specific message, and
> those messages can be set via `git-commit`.
That won't let me imitate gitolite's behavior without a lot of
config file churn:
$ ssh git@localhost
Hello, jrn. This is git@elie running git-shell 1.8.1.3.
R W path/to/one/repo
R path/to/another/repo
$
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Ethan Reesor @ 2013-02-11 6:11 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Sitaram Chamarty, Jeff King, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130211060911.GH15329@elie.Belkin>
Why not have both? That way there is a way to get a customizable
response that avoids Junio's complaints and there is a way to do what
you are trying to achieve.
On Mon, Feb 11, 2013 at 1:09 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ethan Reesor wrote:
>
>> That way, there's a default setting, there can
>> be a system-wide message, there can be a user specific message, and
>> those messages can be set via `git-commit`.
>
> That won't let me imitate gitolite's behavior without a lot of
> config file churn:
>
> $ ssh git@localhost
> Hello, jrn. This is git@elie running git-shell 1.8.1.3.
>
> R W path/to/one/repo
> R path/to/another/repo
> $
--
Ethan Reesor (Gmail)
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 6:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sitaram Chamarty, Jeff King, Ethan Reesor, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <7vpq07pgpy.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> Junio C Hamano wrote:
>>> Are you shooting for customizability?
>>
>> Yes, and the ability to generate the message dynamically.
>
> Hmph, if that is the case, wouldn't it be a better direction to give
> a better help for majority of the case where git-shell is used as
> the login shell to allow push and fetch but not for interactive
> access at all?
>
> The first step in that direction may be to give a better canned
> message, followed by a mechanism (perhaps a hook) that lets a
> message customized for the site's needs, no?
The trouble is that I can't imagine a canned message that everyone
will like. (For example, I quite dislike the current one.) That's
exactly the situation in which some configurability is helpful.
Some configurability is nice for other situations, anyway. For
example, sites serving a multilingual audience may want the message to
vary based on the user's language (or even source IP). The message
can include a list of available repositories or extra information that
changes over time. And so on.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
From: Jonathan Nieder @ 2013-02-11 6:15 UTC (permalink / raw)
To: Ethan Reesor
Cc: Junio C Hamano, Sitaram Chamarty, Jeff King, git,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <CAE_TNi=fN66+9WfMn86H6J_BVAjFP=xiE8m3JHe_4ANHB2V5wA@mail.gmail.com>
[administrivia: please don't top-post]
Ethan Reesor wrote:
> Why not have both? That way there is a way to get a customizable
> response that avoids Junio's complaints and there is a way to do what
> you are trying to achieve.
What was Junio's complaint?
Jonathan
^ permalink raw reply
* Re: [RFC/PATCH] Introduce branch.<name>.pushremote
From: Blind @ 2013-02-11 6:16 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <1360307982-20027-1-git-send-email-artagnon@gmail.com>
If I understand correctly,
in your scenario the branches with branch.<name>.pushremote
will be still included in the $git push <remote> --all?
Are you considering some way to exclude a branch from "push --all"
(branch.<name>.push = always, explicit, never... for example)?
Or maybe, if the branch is already marked as special
with branch.<name>.pushremote,
then it could be logical to push it only when is explicitly specified
on the command line (excluded from --all)?
Thanks,
Blind.
2013/2/8 Ramkumar Ramachandra <artagnon@gmail.com>:
> This new configuration variable overrides the remote in
> `branch.<name>.remote` for pushes. It is useful in the typical
> scenario, where the remote I'm pulling from is not the remote I'm
> pushing to. Although `remote.<name>.pushurl` is similar, it does not
> serve the purpose as the URL would lack corresponding remote tracking
> branches.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> This is a first cut. There's code duplication at the moment, but I'm
> currently trying to figure out which other remote_get() calls to
> replace with pushremote_get(). Comments are welcome.
>
> I will leave it to future patches to do the following things:
> 1. Fix the status output to be more meaningful when pushremote is
> set. At the moment, I'm thinking statuses like [pull: 4 behind,
> push: 3 ahead] will make sense.
> 2. Introduce a remote.pushDefault (peff)
> 3. Introduce a remote.default (peff)
>
> Documentation/config.txt | 6 ++++++
> builtin/push.c | 2 +-
> remote.c | 41 +++++++++++++++++++++++++++++++++++++++++
> remote.h | 2 ++
> 4 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 9b11597..0b3b1f8 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -727,6 +727,12 @@ branch.<name>.remote::
> remote to fetch from/push to. It defaults to `origin` if no remote is
> configured. `origin` is also used if you are not on any branch.
>
> +branch.<name>.pushremote::
> + When in branch <name>, it tells 'git push' which remote to
> + push to. It falls back to `branch.<name>.remote`, and
> + defaults to `origin` if no remote is configured. `origin` is
> + also used if you are not on any branch.
> +
> branch.<name>.merge::
> Defines, together with branch.<name>.remote, the upstream branch
> for the given branch. It tells 'git fetch'/'git pull'/'git rebase' which
> diff --git a/builtin/push.c b/builtin/push.c
> index 42b129d..d447a80 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags)
> static int do_push(const char *repo, int flags)
> {
> int i, errs;
> - struct remote *remote = remote_get(repo);
> + struct remote *remote = pushremote_get(repo);
> const char **url;
> int url_nr;
>
> diff --git a/remote.c b/remote.c
> index e53a6eb..d6fcfc0 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -48,6 +48,7 @@ static int branches_nr;
>
> static struct branch *current_branch;
> static const char *default_remote_name;
> +static const char *pushremote_name;
> static int explicit_default_remote_name;
>
> static struct rewrites rewrites;
> @@ -363,6 +364,12 @@ static int handle_config(const char *key, const char *value, void *cb)
> default_remote_name = branch->remote_name;
> explicit_default_remote_name = 1;
> }
> + } else if (!strcmp(subkey, ".pushremote")) {
> + if (!value)
> + return config_error_nonbool(key);
> + branch->pushremote_name = xstrdup(value);
> + if (branch == current_branch)
> + pushremote_name = branch->pushremote_name;
> } else if (!strcmp(subkey, ".merge")) {
> if (!value)
> return config_error_nonbool(key);
> @@ -700,6 +707,40 @@ struct remote *remote_get(const char *name)
> return ret;
> }
>
> +struct remote *pushremote_get(const char *name)
> +{
> + struct remote *ret;
> + int name_given = 0;
> +
> + read_config();
> + if (name)
> + name_given = 1;
> + else {
> + if (pushremote_name) {
> + name = pushremote_name;
> + name_given = 1;
> + } else {
> + name = default_remote_name;
> + name_given = explicit_default_remote_name;
> + }
> + }
> +
> + ret = make_remote(name, 0);
> + if (valid_remote_nick(name)) {
> + if (!valid_remote(ret))
> + read_remotes_file(ret);
> + if (!valid_remote(ret))
> + read_branches_file(ret);
> + }
> + if (name_given && !valid_remote(ret))
> + add_url_alias(ret, name);
> + if (!valid_remote(ret))
> + return NULL;
> + ret->fetch = parse_fetch_refspec(ret->fetch_refspec_nr, ret->fetch_refspec);
> + ret->push = parse_push_refspec(ret->push_refspec_nr, ret->push_refspec);
> + return ret;
> +}
> +
> int remote_is_configured(const char *name)
> {
> int i;
> diff --git a/remote.h b/remote.h
> index 251d8fd..aa42ff5 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -51,6 +51,7 @@ struct remote {
> };
>
> struct remote *remote_get(const char *name);
> +struct remote *pushremote_get(const char *name);
> int remote_is_configured(const char *name);
>
> typedef int each_remote_fn(struct remote *remote, void *priv);
> @@ -130,6 +131,7 @@ struct branch {
> const char *refname;
>
> const char *remote_name;
> + const char *pushremote_name;
> struct remote *remote;
>
> const char **merge_name;
> --
> 1.8.1.2.545.g2f19ada.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox