# [PATCH 1/5] Library code for user-relative paths, take three. 7 messages from 2005-11-17 to 2005-11-18. Participants: Andreas Ericsson, Junio C Hamano, H. Peter Anvin. Thread: https://gitlist.dev/t/2573 ## Andreas Ericsson, 2005-11-17 19:37 Subject: [PATCH 1/5] Library code for user-relative paths, take three. Message-ID: <20051117193714.2B8995BF93@nox.op5.se> URL: https://gitlist.dev/e/20051117193714.2B8995BF93%40nox.op5.se ``` 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 --- 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 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, 2005-11-17 23:56 Subject: Re: [PATCH 1/5] Library code for user-relative paths, take three. Message-ID: <7v8xvmsu9o.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v8xvmsu9o.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20051117193714.2B8995BF93@nox.op5.se> ``` 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, 2005-11-18 10:08 Subject: Re: [PATCH 1/5] Library code for user-relative paths, take three. Message-ID: <437DA828.6020207@op5.se> URL: https://gitlist.dev/e/437DA828.6020207%40op5.se In-Reply-To: <7v8xvmsu9o.fsf@assigned-by-dhcp.cox.net> ``` 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 ``` ## Andreas Ericsson, 2005-11-18 10:14 Subject: Re: [PATCH 1/5] Library code for user-relative paths, take three. Message-ID: <437DA97E.20203@op5.se> URL: https://gitlist.dev/e/437DA97E.20203%40op5.se In-Reply-To: <437DA828.6020207@op5.se> ``` 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 ``` ## H. Peter Anvin, 2005-11-18 20:33 Subject: Re: [PATCH 1/5] Library code for user-relative paths, take three. Message-ID: <437E3A9B.1070801@zytor.com> URL: https://gitlist.dev/e/437E3A9B.1070801%40zytor.com In-Reply-To: <437DA828.6020207@op5.se> ``` 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 ``` ## Andreas Ericsson, 2005-11-18 22:49 Subject: Re: [PATCH 1/5] Library code for user-relative paths, take three. Message-ID: <437E5A90.3070405@op5.se> URL: https://gitlist.dev/e/437E5A90.3070405%40op5.se In-Reply-To: <437E3A9B.1070801@zytor.com> ``` 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, 2005-11-18 23:23 Subject: Re: [PATCH 1/5] Library code for user-relative paths, take three. Message-ID: <437E6283.8060002@op5.se> URL: https://gitlist.dev/e/437E6283.8060002%40op5.se In-Reply-To: <437E5A90.3070405@op5.se> ``` 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 ```