{"thread":{"id":"2573","subject":"[PATCH 1/5] Library code for user-relative paths, take three.","startedAt":"2005-11-17T19:37:14Z","lastAt":"2005-11-18T23:23:47Z","messageCount":7,"participants":["Andreas Ericsson","Junio C Hamano","H. Peter Anvin"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"12134","messageId":"20051117193714.2B8995BF93@nox.op5.se","threadId":"2573","inReplyTo":null,"subject":"[PATCH 1/5] Library code for user-relative paths, take three.","fromName":"Andreas Ericsson","fromEmail":"exon@op5.se","sentAt":"2005-11-17T19:37:14Z","receivedAt":"2005-11-17T19:37:14Z","isPatch":true,"sender":{"key":"exon@op5.se","avatar":"https://gravatar.com/avatar/b948c4f759e868f8e721e545e37afe2cf89cfa8e2ee8b70a432f0d76aee39891?d=mp&s=160"},"body":"\nSee the threads \"User-relative paths\", \"[RFC] GIT paths\" and\n\"[PATCH 0/4] User-relative paths, take two\" for previous discussions\non this topic.\n\nThis patch provides the work-horse of the user-relative paths feature,\nusing Linus' idea of a blind chdir() and getcwd() which makes it\nremarkably simple.\n\nSigned-off-by: Andreas Ericsson \n\n---\n\n cache.h | 1 +\n path.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 73 insertions(+), 0 deletions(-)\n\napplies-to: 8ff699dffc817e92fb2101f538f84c38d5ed0a0f\n416ee0a4f47244471b52b9dc8aca3e984b20445f\ndiff --git a/cache.h b/cache.h\nindex 99afa2c..d8be06b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -192,6 +192,7 @@ extern int diff_rename_limit_default;\n \n /* Return a statically allocated filename matching the sha1 signature */\n extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));\n+extern char *enter_repo(char *path, int strict);\n extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));\n extern char *sha1_file_name(const unsigned char *sha1);\n extern char *sha1_pack_name(const unsigned char *sha1);\ndiff --git a/path.c b/path.c\nindex 495d17c..5b61709 100644\n--- a/path.c\n+++ b/path.c\n@@ -11,6 +11,7 @@\n * which is what it's designed for.\n */\n #include \"cache.h\"\n+#include \n \n static char pathname[PATH_MAX];\n static char bad_path[] = \"/bad-path/\";\n@@ -89,3 +90,74 @@ char *safe_strncpy(char *dest, const cha\n \n \treturn dest;\n }\n+\n+static char *current_dir()\n+{\n+\treturn getcwd(pathname, sizeof(pathname));\n+}\n+\n+/* Take a raw path from is_git_repo() and canonicalize it using Linus'\n+ * idea of a blind chdir() and getcwd(). */\n+static const char *canonical_path(char *path, int strict)\n+{\n+\tchar *dir = path;\n+\n+\tif(strict && *dir != '/')\n+\t\treturn NULL;\n+\n+\tif(*dir == '~') {\t\t/* user-relative path */\n+\t\tstruct passwd *pw;\n+\t\tchar *slash = strchr(dir, '/');\n+\n+\t\tdir++;\n+\t\t/* '~/' and '~' (no slash) means users own home-dir */\n+\t\tif(!*dir || *dir == '/')\n+\t\t\tpw = getpwuid(getuid());\n+\t\telse {\n+\t\t\tif (slash) {\n+\t\t\t\t*slash = '\\0';\n+\t\t\t\tpw = getpwnam(dir);\n+\t\t\t\t*slash = '/';\n+\t\t\t}\n+\t\t\telse\n+\t\t\t\tpw = getpwnam(dir);\n+\t\t}\n+\n+\t\t/* make sure we got something back that we can chdir() to */\n+\t\tif(!pw || chdir(pw->pw_dir) < 0)\n+\t\t\treturn NULL;\n+\n+\t\tif(!slash || !slash[1]) /* no path following username */\n+\t\t\treturn current_dir();\n+\n+\t\tdir = slash + 1;\n+\t}\n+\n+\t/* ~foo/path/to/repo is now path/to/repo and we're in foo's homedir */\n+\tif(chdir(dir) < 0)\n+\t\treturn NULL;\n+\n+\treturn current_dir();\n+}\n+\n+char *enter_repo(char *path, int strict)\n+{\n+\tif(!path)\n+\t\treturn NULL;\n+\n+\tif(!canonical_path(path, strict)) {\n+\t\tif(strict || !canonical_path(mkpath(\"%s.git\", path), strict))\n+\t\t\treturn NULL;\n+\t}\n+\n+\t/* This is perfectly safe, and people tend to think of the directory\n+\t * where they ran git-init-db as their repository, so humour them. */\n+\t(void)chdir(\".git\");\n+\n+\tif(access(\"objects\", X_OK) == 0 && access(\"refs\", X_OK) == 0) {\n+\t\tputenv(\"GIT_DIR=.\");\n+\t\treturn current_dir();\n+\t}\n+\n+\treturn NULL;\n+}\n---\n0.99.9.GIT\n"},{"id":"12169","messageId":"7v8xvmsu9o.fsf@assigned-by-dhcp.cox.net","threadId":"2573","inReplyTo":"20051117193714.2B8995BF93@nox.op5.se","subject":"Re: [PATCH 1/5] Library code for user-relative paths, take three.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-17T23:56:51Z","receivedAt":"2005-11-17T23:56:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"exon@op5.se (Andreas Ericsson) writes:\n\n> +\tif(strict && *dir != '/')\n\n(style everywhere)\n\n\tif (strict ...\n\n> +\t/* This is perfectly safe, and people tend to think of the directory\n> +\t * where they ran git-init-db as their repository, so humour them. */\n> +\t(void)chdir(\".git\");\n\nIt might be safe, but I think it changes the behaviour of\nupload-pack with strict case. My gut reaction is we would want\n\"if (!strict)\" in front. Thoughts?\n"},{"id":"12208","messageId":"437DA828.6020207@op5.se","threadId":"2573","inReplyTo":"7v8xvmsu9o.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/5] Library code for user-relative paths, take three.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-18T10:08:40Z","receivedAt":"2005-11-18T10:08:40Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> \n>>+\t/* This is perfectly safe, and people tend to think of the directory\n>>+\t * where they ran git-init-db as their repository, so humour them. */\n>>+\t(void)chdir(\".git\");\n> \n> \n> It might be safe, but I think it changes the behaviour of\n> upload-pack with strict case. My gut reaction is we would want\n> \"if (!strict)\" in front. Thoughts?\n> \n\nAs it says in the comment; People tend to think of the directory where \nthey ran \"git init-db\" as their repository, so humour them. It's nice \nfor sharing files between devs in the office, and it *is* safe. Do as \nyou please though. It's the generality of the\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"12209","messageId":"437DA97E.20203@op5.se","threadId":"2573","inReplyTo":"437DA828.6020207@op5.se","subject":"Re: [PATCH 1/5] Library code for user-relative paths, take three.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-18T10:14:22Z","receivedAt":"2005-11-18T10:14:22Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Andreas Ericsson wrote:\n> Junio C Hamano wrote:\n> \n>>\n>>> + /* This is perfectly safe, and people tend to think of the \n>>> directory\n>>> + * where they ran git-init-db as their repository, so humour \n>>> them. */\n>>> + (void)chdir(\".git\");\n>>\n>>\n>>\n>> It might be safe, but I think it changes the behaviour of\n>> upload-pack with strict case. My gut reaction is we would want\n>> \"if (!strict)\" in front. Thoughts?\n>>\n> \n> As it says in the comment; People tend to think of the directory where \n> they ran \"git init-db\" as their repository, so humour them. It's nice \n> for sharing files between devs in the office, and it *is* safe. Do as \n> you please though. It's the generality of the\n> \n\nButter-fingers be me. Sorry about that.\n\nWhat I meant to say was that:\n\n\"it's the general idea of the patchset I'm after\".\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"12246","messageId":"437E3A9B.1070801@zytor.com","threadId":"2573","inReplyTo":"437DA828.6020207@op5.se","subject":"Re: [PATCH 1/5] Library code for user-relative paths, take three.","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-11-18T20:33:31Z","receivedAt":"2005-11-18T20:33:31Z","isPatch":true,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Andreas Ericsson wrote:\n> Junio C Hamano wrote:\n> \n>>\n>>> + /* This is perfectly safe, and people tend to think of the \n>>> directory\n>>> + * where they ran git-init-db as their repository, so humour \n>>> them. */\n>>> + (void)chdir(\".git\");\n>>\n>>\n>> It might be safe, but I think it changes the behaviour of\n>> upload-pack with strict case. My gut reaction is we would want\n>> \"if (!strict)\" in front. Thoughts?\n> \n> As it says in the comment; People tend to think of the directory where \n> they ran \"git init-db\" as their repository, so humour them. It's nice \n> for sharing files between devs in the office, and it *is* safe.\n\nNo, it's not.\n\nThe whole point with --strict is that it shouldn't DWIM. DWIMming is \n*NOT* safe if the data has previously passed through a security screen.\n\nDon't DWIM in strict mode, ever. If you do, you create security holes. \n If not immediately, then later.\n\n\t-hpa\n"},{"id":"12290","messageId":"437E5A90.3070405@op5.se","threadId":"2573","inReplyTo":"437E3A9B.1070801@zytor.com","subject":"Re: [PATCH 1/5] Library code for user-relative paths, take three.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-18T22:49:52Z","receivedAt":"2005-11-18T22:49:52Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"H. Peter Anvin wrote:\n> The whole point with --strict is that it shouldn't DWIM. DWIMming is \n> *NOT* safe if the data has previously passed through a security screen.\n> \n\nBut it hasn't at this point. The security scan is done afterwards, when \nthe canonical path is compared against the whitelist which, in strict \nmode, only matches if it matches exactly.\n\nBut anyways, how about doing\n\n\tenter_repo(path, 2)\n\nfrom the daemon to make enter_repo() do the chdir(\".git\")?\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"12285","messageId":"437E6283.8060002@op5.se","threadId":"2573","inReplyTo":"437E5A90.3070405@op5.se","subject":"Re: [PATCH 1/5] Library code for user-relative paths, take three.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-18T23:23:47Z","receivedAt":"2005-11-18T23:23:47Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Andreas Ericsson wrote:\n> H. Peter Anvin wrote:\n> \n>> The whole point with --strict is that it shouldn't DWIM. DWIMming is \n>> *NOT* safe if the data has previously passed through a security screen.\n>>\n> \n> But it hasn't at this point. The security scan is done afterwards, when \n> the canonical path is compared against the whitelist which, in strict \n> mode, only matches if it matches exactly.\n> \n> But anyways, how about doing\n> \n> enter_repo(path, 2)\n> \n> from the daemon to make enter_repo() do the chdir(\".git\")?\n> \n\n... while preventing the later call from git-upload-pack from doing so.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"}]}