git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Sitaram Chamarty <sitaramc@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Ethan Reesor <firelizzard@gmail.com>,
	git@vger.kernel.org, Ramkumar Ramachandra <artagnon@gmail.com>,
	Greg Brockman <gdb@mit.edu>
Subject: [RFC/PATCH] shell: allow 'help' command to disable interactive shell
Date: Sun, 10 Feb 2013 17:20:16 -0800	[thread overview]
Message-ID: <20130211012016.GA13243@elie.Belkin> (raw)
In-Reply-To: <CAMK1S_jFUXiHM6teVwoxO9gv77B1KBQoSi-B32dwVKemXnDx9w@mail.gmail.com>

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.
	$

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.  A
git-shell-commands directory with only a "help" script can get us most
of the way there, but it unfortunately it produces a "git>" prompt
where the user can do nothing but ask for more help or exit.  So allow
the "help" script to abort the shell by exiting with nonzero status.

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>
---
Sitaram Chamarty wrote:

> Indeed!  In gitolite, I borrowed that idea added to it by making it
> print a list of repos you have access to, along with what permissions
> (R or RW) you have :-)
>
> I'm not suggesting git should do that, but instead of a fixed string,
> a default command to be executed would be better.

Good call.

[...]
> 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?

A patch on top could change the default "git-shell-commands is not
present" message if that seems worthwhile.

 Documentation/git-shell.txt | 26 ++++++++++++++++++++++++++
 shell.c                     | 10 ++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
index 9b925060..758083ff 100644
--- a/Documentation/git-shell.txt
+++ b/Documentation/git-shell.txt
@@ -29,6 +29,32 @@ 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.
 
+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.
+
+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
+--------
+contrib/git-shell-commands/README
+
 GIT
 ---
 Part of the linkgit:git[1] suite
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

  reply	other threads:[~2013-02-11  1:20 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-10 21:05 Git prompt Ethan Reesor
2013-02-10 21:25 ` Jonathan Nieder
2013-02-10 21:54   ` Ethan Reesor
2013-02-10 22:43   ` Jeff King
2013-02-10 22:54     ` Junio C Hamano
2013-02-11  0:43       ` Sitaram Chamarty
2013-02-11  1:20         ` Jonathan Nieder [this message]
2013-02-11  3:44           ` [RFC/PATCH] shell: allow 'help' command to disable interactive shell Junio C Hamano
2013-02-11  4:17             ` Jonathan Nieder
2013-02-11  4:30               ` Junio C Hamano
2013-02-11  4:32                 ` Jonathan Nieder
2013-02-11  4:36                   ` Jeff King
2013-02-11  5:22                   ` Junio C Hamano
2013-02-11  5:57                     ` Ethan Reesor
2013-02-11  6:07                       ` Ethan Reesor
2013-02-11  6:09                       ` Jonathan Nieder
2013-02-11  6:11                         ` Ethan Reesor
2013-02-11  6:15                           ` Jonathan Nieder
2013-02-11  6:22                             ` Ethan Reesor
2013-02-11  6:14                     ` Jonathan Nieder
2013-02-11  7:01                       ` Junio C Hamano
2013-02-11  7:12                         ` Jonathan Nieder
2013-02-11  7:17                           ` Junio C Hamano
2013-02-11  7:21                             ` Jonathan Nieder
2013-02-11  7:44                               ` Junio C Hamano
2013-02-11  8:13                                 ` Jonathan Nieder
2013-02-11 16:17                                   ` Junio C Hamano
2013-02-11 16:00                             ` Jeff King
2013-02-11 17:18                               ` Junio C Hamano
2013-02-11 17:27                                 ` Jeff King
2013-02-11  7:18                           ` Ethan Reesor
2013-02-11  7:15                         ` Ethan Reesor
2013-02-11  7:22                           ` Junio C Hamano
2013-02-11  7:26                             ` Ethan Reesor
2013-02-11  7:28                               ` Junio C Hamano
2013-02-11  3:59           ` Jeff King
2013-02-11  4:14             ` Jonathan Nieder
2013-02-11  4:17               ` Jeff King
2013-02-11  4:26                 ` Jonathan Nieder
2013-02-11  4:33                   ` Jeff King
2013-02-11  5:56                     ` [PATCH 0/2 v2] " Jonathan Nieder
2013-02-11  5:57                       ` [PATCH 1/2] shell doc: emphasize purpose and security model Jonathan Nieder
2013-02-11  7:10                         ` Junio C Hamano
2013-02-11  7:13                           ` Jonathan Nieder
2013-02-11 18:32                         ` Junio C Hamano
2013-02-11  5:58                       ` [PATCH 2/2] shell: pay attention to exit status from 'help' command Jonathan Nieder
2013-02-11  6:06                         ` Ethan Reesor
2013-02-11  7:15                         ` Junio C Hamano
2013-02-11  7:52                           ` Jonathan Nieder
2013-02-11 16:28                             ` Junio C Hamano
2013-02-11  4:45           ` [RFC/PATCH] shell: allow 'help' command to disable interactive shell Jeff King
2013-03-09 21:52 ` [PATCH v3 0/2] shell: allow 'no-interactive-login' " Jonathan Nieder
2013-03-09 21:55   ` [PATCH 1/2] shell doc: emphasize purpose and security model Jonathan Nieder
2013-03-09 22:00   ` [PATCH 2/2] shell: new no-interactive-login command to print a custom message Jonathan Nieder
2013-03-10  5:04     ` Junio C Hamano
2013-03-10  5:21       ` Jonathan Nieder
2013-03-10 10:49     ` Ramkumar Ramachandra
2013-03-11 22:48       ` Jonathan Nieder
2013-03-12 10:47   ` [PATCH v3 0/2] shell: allow 'no-interactive-login' command to disable interactive shell Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130211012016.GA13243@elie.Belkin \
    --to=jrnieder@gmail.com \
    --cc=artagnon@gmail.com \
    --cc=firelizzard@gmail.com \
    --cc=gdb@mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sitaramc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).