Summary

Access remote git repositories (ssh scheme url) without a raw file url.

Review Request #4910 — Created Nov. 6, 2013 and discarded July 11, 2014, 5:47 p.m.

Information

Owner

jop

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Access remote git repositories (ssh scheme url) without a raw file url.

Git-shell, as usually configured on ssh git servers, allows executing
arbitrary commands. This patch uses this feature to avoid having to setup
an http server for raw files.

To use this feature:

1. Set both path and mirror path to the same ssh git url, without the .git
sufix.

2. Leave raw file url mask empty.

3. Add the following script in ~/git-shell-commans/rb-cat-file:

------------
#!/usr/bin/env sh
git --git-dir="$1/.git" cat-file $2 $3
------------

This assumes that ssh authentication is working, by having setup the
proper ssh authorization at the server.

Testing Done

This has been tested in release 1.7.16 (current Fedora rpm installation) with a remote git repo on a different server. It works for submitting and updating review requests from rbt and the web interface.

Tests with invalid setups are as follows:

Missing chsh to git-shell:

$ rbt post -g
ERROR:root:Error uploading diff

One or more fields had errors (HTTP 400, API Error 105)

path: bash: rb-cat-file: command not found

Your review request still exists, but the diff is not attached.

Not chmod-ing the script 755:

$ rbt post -g
ERROR:root:Error uploading diff

One or more fields had errors (HTTP 400, API Error 105)

path: fatal: unrecognized command 'rb-cat-file /path/to/repo -t pha7AhquZa7icohBeoPh2AjuuthaiNg4faiMi9oo'

Your review request still exists, but the diff is not attached.

Not having the script at all:

$ rbt post -g
ERROR:root:Error uploading diff

One or more fields had errors (HTTP 400, API Error 105)

path: fatal: unrecognized command 'rb-cat-file /path/to/repo -t pha7AhquZa7icohBeoPh2AjuuthaiNg4faiMi9oo'

Your review request still exists, but the diff is not attached.

Any of the above from the web interface: the same error (i.e. the "path:" line) appears in red by the diff selector.

Corrupting the commit id fed to the script (this can only be done by changing the source code):

$ rbt post -g
ERROR:root:Error uploading diff

The file was not found in the repository (HTTP 400, API Error 207)

Your review request still exists, but the diff is not attached.

Corrupting the commit id fed to the script in the web interface (this can only be done by changing the source code):

"The file 'README' (rd702173) could not be found in the repository" appears in red by the diff selector.

This is definitely an interesting approach, but how sure are you that all installs allow arbitrary commands?

There's no way we can require placing a script on there. Can this not be done without a custom script?

jop Nov. 7, 2013, 2:33 a.m.

AFAIK this cannot be done without the script. This git limitation is actually the reason for having the "Raw file URL mask" workaround, right?

But note that no requirement is actually being added. Every setup that has previously worked, either with an HTTP server or a local repo mirror, will work unchanged. That would still be the preferred setup with publicly available repos that already have cgit/gitweb access.

This patch only adds another option. And when you have a typical private git repo with ssh access, it seems to me easier to add the script than either (1) setting up an http server with cgit/gitweb (including a different authentication mechanism...); or (2) having a local repository mirror. But it certainly depends on each setup.

Arbitrary scripts are a standard feature of git-shell (http://git-scm.com/docs/git-shell.html), which is the recommended setup for remote ssh access to git repos.

chipx86 Nov. 7, 2013, 3:09 a.m.

Sure. At this point, it's more that we have yet another method to document and support, but I can see the value in it. We'll have to discuss what we want to do here.

Can you go through the change and make sure it's consistent with the rest of our code? Right now, it's missing things like spaces around operators, and I'd like to see it a bit closer before doing an initial review.

Change Summary:

Spacing and indenting have been changed and should now be consistent with the rest of the code.

Summary:

-	Access remote git repositories (ssh scheme url) without a raw file url.
+	Fixed space and indenting.

Description:

		Access remote git repositories (ssh scheme url) without a raw file url.

		Git-shell, as usually configured on ssh git servers, allows executing
		arbitrary commands. This patch uses this feature to avoid having to setup
		an http server for raw files.

		To use this feature:

~		Set both path and mirror path to the same ssh git url, without the .git sufix.
~		Leave raw file url mask empty.
	~	1. Set both path and mirror path to the same ssh git url, without the .git
	~	sufix.
-		Add the following script in ~/git-shell-commans/rb-cat-file:

~
	~	2. Leave raw file url mask empty.

~		#!/usr/bin/env sh
	~	3. Add the following script in ~/git-shell-commans/rb-cat-file:
-		git --git-dir="$1/.git" cat-file $2 $3

~
	~	------------
	+	#!/usr/bin/env sh
	+	git --git-dir="$1/.git" cat-file $2 $3
	+	------------

		This assumes that ssh authentication is working, by having setup the
		proper ssh authorization at the server.

Diff:

Revision 2 (+22 -4)

Show changes

reviewboard/scmtools/git.py

You seem to have renamed the summary.

Can you also provide the Testing Done section? Main things I want to see:

Successful tests with the script installed.
Tests with invalid data passed to the script.
The script not installed (or not installed correctly -- wrong chmod, for instance), and how that's handled.

Change Summary:

Added information about testing.

Summary:

-	Fixed space and indenting.
+	Access remote git repositories (ssh scheme url) without a raw file url.

Testing Done:

	+	This has been tested in release 1.7.16 (current Fedora rpm installation) with a remote git repo on a different server. It works for submitting and updating review requests from rbt and the web interface.
	+
	+	Tests with invalid setups are as follows:
	+
	+	Missing chsh to git-shell: $ rbt post -g ERROR:root:Error uploading diff
	+
	+
	+
	+	One or more fields had errors (HTTP 400, API Error 105)
	+
	+	`path: bash: rb-cat-file: command not found`
	+
	+
	+
	+
	+	Your review request still exists, but the diff is not attached.
	+
	+	Not chmod-ing the script 755: $ rbt post -g ERROR:root:Error uploading diff
	+
	+
	+
	+	One or more fields had errors (HTTP 400, API Error 105)
	+
	+	`path: fatal: unrecognized command 'rb-cat-file /path/to/repo -t pha7AhquZa7icohBeoPh2AjuuthaiNg4faiMi9oo'`
	+
	+
	+
	+
	+	Your review request still exists, but the diff is not attached.
	+
	+	Not having the script at all: $ rbt post -g ERROR:root:Error uploading diff
	+
	+
	+
	+	One or more fields had errors (HTTP 400, API Error 105)
	+
	+	`path: fatal: unrecognized command 'rb-cat-file /path/to/repo -t pha7AhquZa7icohBeoPh2AjuuthaiNg4faiMi9oo'`
	+
	+
	+
	+
	+	Your review request still exists, but the diff is not attached.
	+
	+	Any of the above from the web interface: the same error (i.e. the "path:" line) appears in red by the diff selector.
	+
	+
	+
	+	Corrupting the commit id fed to the script (this can only be done by changing the source code): $ rbt post -g ERROR:root:Error uploading diff
	+
	+
	+
	+	The file was not found in the repository (HTTP 400, API Error 207)
	+
	+	Your review request still exists, but the diff is not attached.
	+
	+	Corrupting the commit id fed to the script in the web interface (this can only be done by changing the source code):
	+
	+	"The file 'README' (rd702173) could not be found in the repository" appears in red by the diff selector.

Review Board 7.0 alpha 0 (dev)

Access remote git repositories (ssh scheme url) without a raw file url.

Information

Reviewers

Missing chsh to git-shell:

Your review request still exists, but the diff is not attached.

Not chmod-ing the script 755:

Your review request still exists, but the diff is not attached.

Not having the script at all:

Your review request still exists, but the diff is not attached.

Corrupting the commit id fed to the script (this can only be done by changing the source code):

Your review request still exists, but the diff is not attached.

Change Summary:

Summary:

Description:

Diff:

Change Summary:

Summary:

Testing Done:

Missing chsh to git-shell:

Your review request still exists, but the diff is not attached.

Not chmod-ing the script 755:

Your review request still exists, but the diff is not attached.

Not having the script at all:

Your review request still exists, but the diff is not attached.

Corrupting the commit id fed to the script (this can only be done by changing the source code):

Your review request still exists, but the diff is not attached.

Status: Discarded