All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.