threads / patch / 2573

patch, 5 partsLibrary code for user-relative paths, take three.

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

## tl;dr

7 messages between Nov 17, 2005 and Nov 18, 2005. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Andreas Ericsson· Nov 17, 2005, 19:37 UTC · lore

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

Show changes to 2 files +73 −0

cache.h, path.c

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
Junio C Hamano· Nov 17, 2005, 23:56 UTC · re: Andreas Ericsson · lore

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

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?

Andreas Ericsson· Nov 18, 2005, 10:08 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 10 quoted lines
> 
>>+	/* 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
Andreas Ericsson· Nov 18, 2005, 10:14 UTC · re: Andreas Ericsson · lore

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

Andreas Ericsson wrote:
Show 21 quoted lines
> 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
H. Peter Anvin· Nov 18, 2005, 20:33 UTC · re: Andreas Ericsson · lore

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

Andreas Ericsson wrote:
Show 17 quoted lines
> 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
Andreas Ericsson· Nov 18, 2005, 22:49 UTC · re: H. Peter Anvin · lore

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

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
Andreas Ericsson· Nov 18, 2005, 23:23 UTC · re: Andreas Ericsson · lore

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

Andreas Ericsson wrote:
Show 16 quoted lines
> 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

← back to recent threads