[PATCH 1/5] Library code for user-relative paths, take three.

[PATCH 1/5] Library code for user-relative paths, take three.

[Andreas Ericsson]

See the threads "User-relative paths", "[RFC] GIT paths" and
"[PATCH 0/4] User-relative paths, take two" for previous discussions
on this topic.

This patch provides the work-horse of the user-relative paths feature,
using Linus' idea of a blind chdir() and getcwd() which makes it
remarkably simple.

Signed-off-by: Andreas Ericsson <ae@op5.se>

---

 cache.h |    1 +
 path.c  |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 0 deletions(-)

applies-to: 8ff699dffc817e92fb2101f538f84c38d5ed0a0f
416ee0a4f47244471b52b9dc8aca3e984b20445f
diff --git a/cache.h b/cache.h
index 99afa2c..d8be06b 100644
--- a/cache.h
+++ b/cache.h
@@ -192,6 +192,7 @@ extern int diff_rename_limit_default;
 
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
+extern char *enter_repo(char *path, int strict);
 extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern char *sha1_file_name(const unsigned char *sha1);
 extern char *sha1_pack_name(const unsigned char *sha1);
diff --git a/path.c b/path.c
index 495d17c..5b61709 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
  * which is what it's designed for.
  */
 #include "cache.h"
+#include <pwd.h>
 
 static char pathname[PATH_MAX];
 static char bad_path[] = "/bad-path/";
@@ -89,3 +90,74 @@ char *safe_strncpy(char *dest, const cha
 
 	return dest;
 }
+
+static char *current_dir()
+{
+	return getcwd(pathname, sizeof(pathname));
+}
+
+/* Take a raw path from is_git_repo() and canonicalize it using Linus'
+ * idea of a blind chdir() and getcwd(). */
+static const char *canonical_path(char *path, int strict)
+{
+	char *dir = path;
+
+	if(strict && *dir != '/')
+		return NULL;
+
+	if(*dir == '~') {		/* user-relative path */
+		struct passwd *pw;
+		char *slash = strchr(dir, '/');
+
+		dir++;
+		/* '~/' and '~' (no slash) means users own home-dir */
+		if(!*dir || *dir == '/')
+			pw = getpwuid(getuid());
+		else {
+			if (slash) {
+				*slash = '\0';
+				pw = getpwnam(dir);
+				*slash = '/';
+			}
+			else
+				pw = getpwnam(dir);
+		}
+
+		/* make sure we got something back that we can chdir() to */
+		if(!pw || chdir(pw->pw_dir) < 0)
+			return NULL;
+
+		if(!slash || !slash[1]) /* no path following username */
+			return current_dir();
+
+		dir = slash + 1;
+	}
+
+	/* ~foo/path/to/repo is now path/to/repo and we're in foo's homedir */
+	if(chdir(dir) < 0)
+		return NULL;
+
+	return current_dir();
+}
+
+char *enter_repo(char *path, int strict)
+{
+	if(!path)
+		return NULL;
+
+	if(!canonical_path(path, strict)) {
+		if(strict || !canonical_path(mkpath("%s.git", path), strict))
+			return NULL;
+	}
+
+	/* This is perfectly safe, and people tend to think of the directory
+	 * where they ran git-init-db as their repository, so humour them. */
+	(void)chdir(".git");
+
+	if(access("objects", X_OK) == 0 && access("refs", X_OK) == 0) {
+		putenv("GIT_DIR=.");
+		return current_dir();
+	}
+
+	return NULL;
+}
---
0.99.9.GIT

Re: [PATCH 1/5] Library code for user-relative paths, take three.

[Junio C Hamano]

exon@op5.se (Andreas Ericsson) writes:

> +	if(strict && *dir != '/')

(style everywhere)

	if (strict ...

> +	/* This is perfectly safe, and people tend to think of the directory
> +	 * where they ran git-init-db as their repository, so humour them. */
> +	(void)chdir(".git");

It might be safe, but I think it changes the behaviour of
upload-pack with strict case.  My gut reaction is we would want
"if (!strict)" in front.  Thoughts?

Re: [PATCH 1/5] Library code for user-relative paths, take three.

[Andreas Ericsson]

Junio C Hamano wrote:
> 
>>+	/* This is perfectly safe, and people tend to think of the directory
>>+	 * where they ran git-init-db as their repository, so humour them. */
>>+	(void)chdir(".git");
> 
> 
> It might be safe, but I think it changes the behaviour of
> upload-pack with strict case.  My gut reaction is we would want
> "if (!strict)" in front.  Thoughts?
> 

As it says in the comment; People tend to think of the directory where 
they ran "git init-db" as their repository, so humour them. It's nice 
for sharing files between devs in the office, and it *is* safe. Do as 
you please though. It's the generality of the

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 1/5] Library code for user-relative paths, take three.

[Andreas Ericsson]

Andreas Ericsson wrote:
> Junio C Hamano wrote:
> 
>>
>>> +    /* This is perfectly safe, and people tend to think of the 
>>> directory
>>> +     * where they ran git-init-db as their repository, so humour 
>>> them. */
>>> +    (void)chdir(".git");
>>
>>
>>
>> It might be safe, but I think it changes the behaviour of
>> upload-pack with strict case.  My gut reaction is we would want
>> "if (!strict)" in front.  Thoughts?
>>
> 
> As it says in the comment; People tend to think of the directory where 
> they ran "git init-db" as their repository, so humour them. It's nice 
> for sharing files between devs in the office, and it *is* safe. Do as 
> you please though. It's the generality of the
> 

Butter-fingers be me. Sorry about that.

What I meant to say was that:

"it's the general idea of the patchset I'm after".

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 1/5] Library code for user-relative paths, take three.

[H. Peter Anvin]

Andreas Ericsson wrote:
> Junio C Hamano wrote:
> 
>>
>>> +    /* This is perfectly safe, and people tend to think of the 
>>> directory
>>> +     * where they ran git-init-db as their repository, so humour 
>>> them. */
>>> +    (void)chdir(".git");
>>
>>
>> It might be safe, but I think it changes the behaviour of
>> upload-pack with strict case.  My gut reaction is we would want
>> "if (!strict)" in front.  Thoughts?
> 
> As it says in the comment; People tend to think of the directory where 
> they ran "git init-db" as their repository, so humour them. It's nice 
> for sharing files between devs in the office, and it *is* safe.

No, it's not.

The whole point with --strict is that it shouldn't DWIM.  DWIMming is 
*NOT* safe if the data has previously passed through a security screen.

Don't DWIM in strict mode, ever.  If you do, you create security holes. 
  If not immediately, then later.

	-hpa

Re: [PATCH 1/5] Library code for user-relative paths, take three.

[Andreas Ericsson]

H. Peter Anvin wrote:
> The whole point with --strict is that it shouldn't DWIM.  DWIMming is 
> *NOT* safe if the data has previously passed through a security screen.
> 

But it hasn't at this point. The security scan is done afterwards, when 
the canonical path is compared against the whitelist which, in strict 
mode, only matches if it matches exactly.

But anyways, how about doing

	enter_repo(path, 2)

from the daemon to make enter_repo() do the chdir(".git")?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 1/5] Library code for user-relative paths, take three.

[Andreas Ericsson]

Andreas Ericsson wrote:
> H. Peter Anvin wrote:
> 
>> The whole point with --strict is that it shouldn't DWIM.  DWIMming is 
>> *NOT* safe if the data has previously passed through a security screen.
>>
> 
> But it hasn't at this point. The security scan is done afterwards, when 
> the canonical path is compared against the whitelist which, in strict 
> mode, only matches if it matches exactly.
> 
> But anyways, how about doing
> 
>     enter_repo(path, 2)
> 
> from the daemon to make enter_repo() do the chdir(".git")?
> 

... while preventing the later call from git-upload-pack from doing so.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231