Merge branch 'jk/open-dotgitx-with-nofollow'
It does not make sense to make ".gitattributes", ".gitignore" and
".mailmap" symlinks, as they are supposed to be usable from the
object store (think: bare repositories where HEAD:.mailmap etc. are
used). When these files are symbolic links, we used to read the
contents of the files pointed by them by mistake, which has been
corrected.
* jk/open-dotgitx-with-nofollow:
mailmap: do not respect symlinks for in-tree .mailmap
exclude: do not respect symlinks for in-tree .gitignore
attr: do not respect symlinks for in-tree .gitattributes
exclude: add flags parameter to add_patterns()
attr: convert "macro_ok" into a flags field
add open_nofollow() helper
diff --git a/attr.c b/attr.c
index 59a8d8a..ac8ec7c 100644
--- a/attr.c
+++ b/attr.c
@@ -278,6 +278,10 @@ struct match_attr {
static const char blank[] = " \t\r\n";
+/* Flags usable in read_attr() and parse_attr_line() family of functions. */
+#define READ_ATTR_MACRO_OK (1<<0)
+#define READ_ATTR_NOFOLLOW (1<<1)
+
/*
* Parse a whitespace-delimited attribute state (i.e., "attr",
* "-attr", "!attr", or "attr=value") from the string starting at src.
@@ -331,7 +335,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
}
static struct match_attr *parse_attr_line(const char *line, const char *src,
- int lineno, int macro_ok)
+ int lineno, unsigned flags)
{
int namelen;
int num_attr, i;
@@ -355,7 +359,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
if (strlen(ATTRIBUTE_MACRO_PREFIX) < namelen &&
starts_with(name, ATTRIBUTE_MACRO_PREFIX)) {
- if (!macro_ok) {
+ if (!(flags & READ_ATTR_MACRO_OK)) {
fprintf_ln(stderr, _("%s not allowed: %s:%d"),
name, src, lineno);
goto fail_return;
@@ -653,11 +657,11 @@ static void handle_attr_line(struct attr_stack *res,
const char *line,
const char *src,
int lineno,
- int macro_ok)
+ unsigned flags)
{
struct match_attr *a;
- a = parse_attr_line(line, src, lineno, macro_ok);
+ a = parse_attr_line(line, src, lineno, flags);
if (!a)
return;
ALLOC_GROW(res->attrs, res->num_matches + 1, res->alloc);
@@ -672,7 +676,8 @@ static struct attr_stack *read_attr_from_array(const char **list)
CALLOC_ARRAY(res, 1);
while ((line = *(list++)) != NULL)
- handle_attr_line(res, line, "[builtin]", ++lineno, 1);
+ handle_attr_line(res, line, "[builtin]", ++lineno,
+ READ_ATTR_MACRO_OK);
return res;
}
@@ -698,21 +703,31 @@ void git_attr_set_direction(enum git_attr_direction new_direction)
direction = new_direction;
}
-static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
+static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)
{
- FILE *fp = fopen_or_warn(path, "r");
+ int fd;
+ FILE *fp;
struct attr_stack *res;
char buf[2048];
int lineno = 0;
- if (!fp)
+ if (flags & READ_ATTR_NOFOLLOW)
+ fd = open_nofollow(path, O_RDONLY);
+ else
+ fd = open(path, O_RDONLY);
+
+ if (fd < 0) {
+ warn_on_fopen_errors(path);
return NULL;
+ }
+ fp = xfdopen(fd, "r");
+
CALLOC_ARRAY(res, 1);
while (fgets(buf, sizeof(buf), fp)) {
char *bufp = buf;
if (!lineno)
skip_utf8_bom(&bufp, strlen(bufp));
- handle_attr_line(res, bufp, path, ++lineno, macro_ok);
+ handle_attr_line(res, bufp, path, ++lineno, flags);
}
fclose(fp);
return res;
@@ -720,7 +735,7 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
static struct attr_stack *read_attr_from_index(const struct index_state *istate,
const char *path,
- int macro_ok)
+ unsigned flags)
{
struct attr_stack *res;
char *buf, *sp;
@@ -741,7 +756,7 @@ static struct attr_stack *read_attr_from_index(const struct index_state *istate,
ep = strchrnul(sp, '\n');
more = (*ep == '\n');
*ep = '\0';
- handle_attr_line(res, sp, path, ++lineno, macro_ok);
+ handle_attr_line(res, sp, path, ++lineno, flags);
sp = ep + more;
}
free(buf);
@@ -749,19 +764,19 @@ static struct attr_stack *read_attr_from_index(const struct index_state *istate,
}
static struct attr_stack *read_attr(const struct index_state *istate,
- const char *path, int macro_ok)
+ const char *path, unsigned flags)
{
struct attr_stack *res = NULL;
if (direction == GIT_ATTR_INDEX) {
- res = read_attr_from_index(istate, path, macro_ok);
+ res = read_attr_from_index(istate, path, flags);
} else if (!is_bare_repository()) {
if (direction == GIT_ATTR_CHECKOUT) {
- res = read_attr_from_index(istate, path, macro_ok);
+ res = read_attr_from_index(istate, path, flags);
if (!res)
- res = read_attr_from_file(path, macro_ok);
+ res = read_attr_from_file(path, flags);
} else if (direction == GIT_ATTR_CHECKIN) {
- res = read_attr_from_file(path, macro_ok);
+ res = read_attr_from_file(path, flags);
if (!res)
/*
* There is no checked out .gitattributes file
@@ -769,7 +784,7 @@ static struct attr_stack *read_attr(const struct index_state *istate,
* We allow operation in a sparsely checked out
* work tree, so read from it.
*/
- res = read_attr_from_index(istate, path, macro_ok);
+ res = read_attr_from_index(istate, path, flags);
}
}
@@ -844,6 +859,7 @@ static void bootstrap_attr_stack(const struct index_state *istate,
struct attr_stack **stack)
{
struct attr_stack *e;
+ unsigned flags = READ_ATTR_MACRO_OK;
if (*stack)
return;
@@ -854,23 +870,23 @@ static void bootstrap_attr_stack(const struct index_state *istate,
/* system-wide frame */
if (git_attr_system()) {
- e = read_attr_from_file(git_etc_gitattributes(), 1);
+ e = read_attr_from_file(git_etc_gitattributes(), flags);
push_stack(stack, e, NULL, 0);
}
/* home directory */
if (get_home_gitattributes()) {
- e = read_attr_from_file(get_home_gitattributes(), 1);
+ e = read_attr_from_file(get_home_gitattributes(), flags);
push_stack(stack, e, NULL, 0);
}
/* root directory */
- e = read_attr(istate, GITATTRIBUTES_FILE, 1);
+ e = read_attr(istate, GITATTRIBUTES_FILE, flags | READ_ATTR_NOFOLLOW);
push_stack(stack, e, xstrdup(""), 0);
/* info frame */
if (startup_info->have_repository)
- e = read_attr_from_file(git_path_info_attributes(), 1);
+ e = read_attr_from_file(git_path_info_attributes(), flags);
else
e = NULL;
if (!e)
@@ -956,7 +972,7 @@ static void prepare_attr_stack(const struct index_state *istate,
strbuf_add(&pathbuf, path + pathbuf.len, (len - pathbuf.len));
strbuf_addf(&pathbuf, "/%s", GITATTRIBUTES_FILE);
- next = read_attr(istate, pathbuf.buf, 0);
+ next = read_attr(istate, pathbuf.buf, READ_ATTR_NOFOLLOW);
/* reset the pathbuf to not include "/.gitattributes" */
strbuf_setlen(&pathbuf, len);
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 2306a9a..d7da50a 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -64,7 +64,7 @@ static int sparse_checkout_list(int argc, const char **argv)
pl.use_cone_patterns = core_sparse_checkout_cone;
sparse_filename = get_sparse_checkout_filename();
- res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL);
+ res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
free(sparse_filename);
if (res < 0) {
@@ -321,7 +321,7 @@ static int sparse_checkout_init(int argc, const char **argv)
memset(&pl, 0, sizeof(pl));
sparse_filename = get_sparse_checkout_filename();
- res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL);
+ res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
/* If we already have a sparse-checkout file, use it. */
if (res >= 0) {
@@ -483,7 +483,7 @@ static void add_patterns_cone_mode(int argc, const char **argv,
existing.use_cone_patterns = core_sparse_checkout_cone;
if (add_patterns_from_file_to_list(sparse_filename, "", 0,
- &existing, NULL))
+ &existing, NULL, 0))
die(_("unable to load existing sparse-checkout patterns"));
free(sparse_filename);
@@ -507,7 +507,7 @@ static void add_patterns_literal(int argc, const char **argv,
{
char *sparse_filename = get_sparse_checkout_filename();
if (add_patterns_from_file_to_list(sparse_filename, "", 0,
- pl, NULL))
+ pl, NULL, 0))
die(_("unable to load existing sparse-checkout patterns"));
free(sparse_filename);
add_patterns_from_input(pl, argc, argv);
diff --git a/dir.c b/dir.c
index 19c2fa2..3474e67 100644
--- a/dir.c
+++ b/dir.c
@@ -1035,6 +1035,9 @@ static int add_patterns_from_buffer(char *buf, size_t size,
const char *base, int baselen,
struct pattern_list *pl);
+/* Flags for add_patterns() */
+#define PATTERN_NOFOLLOW (1<<0)
+
/*
* Given a file with name "fname", read it (either from disk, or from
* an index if 'istate' is non-null), parse it and store the
@@ -1046,7 +1049,7 @@ static int add_patterns_from_buffer(char *buf, size_t size,
*/
static int add_patterns(const char *fname, const char *base, int baselen,
struct pattern_list *pl, struct index_state *istate,
- struct oid_stat *oid_stat)
+ unsigned flags, struct oid_stat *oid_stat)
{
struct stat st;
int r;
@@ -1054,7 +1057,11 @@ static int add_patterns(const char *fname, const char *base, int baselen,
size_t size = 0;
char *buf;
- fd = open(fname, O_RDONLY);
+ if (flags & PATTERN_NOFOLLOW)
+ fd = open_nofollow(fname, O_RDONLY);
+ else
+ fd = open(fname, O_RDONLY);
+
if (fd < 0 || fstat(fd, &st) < 0) {
if (fd < 0)
warn_on_fopen_errors(fname);
@@ -1143,9 +1150,10 @@ static int add_patterns_from_buffer(char *buf, size_t size,
int add_patterns_from_file_to_list(const char *fname, const char *base,
int baselen, struct pattern_list *pl,
- struct index_state *istate)
+ struct index_state *istate,
+ unsigned flags)
{
- return add_patterns(fname, base, baselen, pl, istate, NULL);
+ return add_patterns(fname, base, baselen, pl, istate, flags, NULL);
}
int add_patterns_from_blob_to_list(
@@ -1194,7 +1202,7 @@ static void add_patterns_from_file_1(struct dir_struct *dir, const char *fname,
if (!dir->untracked)
dir->unmanaged_exclude_files++;
pl = add_pattern_list(dir, EXC_FILE, fname);
- if (add_patterns(fname, "", 0, pl, NULL, oid_stat) < 0)
+ if (add_patterns(fname, "", 0, pl, NULL, 0, oid_stat) < 0)
die(_("cannot use %s as an exclude file"), fname);
}
@@ -1558,6 +1566,7 @@ static void prep_exclude(struct dir_struct *dir,
strbuf_addstr(&sb, dir->exclude_per_dir);
pl->src = strbuf_detach(&sb, NULL);
add_patterns(pl->src, pl->src, stk->baselen, pl, istate,
+ PATTERN_NOFOLLOW,
untracked ? &oid_stat : NULL);
}
/*
@@ -3006,7 +3015,7 @@ int get_sparse_checkout_patterns(struct pattern_list *pl)
char *sparse_filename = get_sparse_checkout_filename();
pl->use_cone_patterns = core_sparse_checkout_cone;
- res = add_patterns_from_file_to_list(sparse_filename, "", 0, pl, NULL);
+ res = add_patterns_from_file_to_list(sparse_filename, "", 0, pl, NULL, 0);
free(sparse_filename);
return res;
diff --git a/dir.h b/dir.h
index facfae4..04d886c 100644
--- a/dir.h
+++ b/dir.h
@@ -420,7 +420,8 @@ int hashmap_contains_parent(struct hashmap *map,
struct pattern_list *add_pattern_list(struct dir_struct *dir,
int group_type, const char *src);
int add_patterns_from_file_to_list(const char *fname, const char *base, int baselen,
- struct pattern_list *pl, struct index_state *istate);
+ struct pattern_list *pl, struct index_state *istate,
+ unsigned flags);
void add_patterns_from_file(struct dir_struct *, const char *fname);
int add_patterns_from_blob_to_list(struct object_id *oid,
const char *base, int baselen,
diff --git a/git-compat-util.h b/git-compat-util.h
index dcc786e..9ddf9d7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1242,6 +1242,13 @@ int access_or_die(const char *path, int mode, unsigned flag);
/* Warn on an inaccessible file if errno indicates this is an error */
int warn_on_fopen_errors(const char *path);
+/*
+ * Open with O_NOFOLLOW, or equivalent. Note that the fallback equivalent
+ * may be racy. Do not use this as protection against an attacker who can
+ * simultaneously create paths.
+ */
+int open_nofollow(const char *path, int flags);
+
#if !defined(USE_PARENS_AROUND_GETTEXT_N) && defined(__GNUC__)
#define USE_PARENS_AROUND_GETTEXT_N 1
#endif
diff --git a/mailmap.c b/mailmap.c
index 183a2f6..d1f7c0d 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -157,20 +157,30 @@ static void read_mailmap_line(struct string_list *map, char *buffer)
add_mapping(map, name1, email1, name2, email2);
}
-static int read_mailmap_file(struct string_list *map, const char *filename)
+/* Flags for read_mailmap_file() */
+#define MAILMAP_NOFOLLOW (1<<0)
+
+static int read_mailmap_file(struct string_list *map, const char *filename,
+ unsigned flags)
{
char buffer[1024];
FILE *f;
+ int fd;
if (!filename)
return 0;
- f = fopen(filename, "r");
- if (!f) {
+ if (flags & MAILMAP_NOFOLLOW)
+ fd = open_nofollow(filename, O_RDONLY);
+ else
+ fd = open(filename, O_RDONLY);
+
+ if (fd < 0) {
if (errno == ENOENT)
return 0;
return error_errno("unable to open mailmap at %s", filename);
}
+ f = xfdopen(fd, "r");
while (fgets(buffer, sizeof(buffer), f) != NULL)
read_mailmap_line(map, buffer);
@@ -226,10 +236,12 @@ int read_mailmap(struct string_list *map)
git_mailmap_blob = "HEAD:.mailmap";
if (!startup_info->have_repository || !is_bare_repository())
- err |= read_mailmap_file(map, ".mailmap");
+ err |= read_mailmap_file(map, ".mailmap",
+ startup_info->have_repository ?
+ MAILMAP_NOFOLLOW : 0);
if (startup_info->have_repository)
err |= read_mailmap_blob(map, git_mailmap_blob);
- err |= read_mailmap_file(map, git_mailmap_file);
+ err |= read_mailmap_file(map, git_mailmap_file, 0);
return err;
}
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index b660593..1e4c672 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -4,12 +4,16 @@
. ./test-lib.sh
-attr_check () {
+attr_check_basic () {
path="$1" expect="$2" git_opts="$3" &&
git $git_opts check-attr test -- "$path" >actual 2>err &&
echo "$path: test: $expect" >expect &&
- test_cmp expect actual &&
+ test_cmp expect actual
+}
+
+attr_check () {
+ attr_check_basic "$@" &&
test_must_be_empty err
}
@@ -331,7 +335,6 @@
test_cmp expect actual
'
-
test_expect_success 'query binary macro directly' '
echo "file binary" >.gitattributes &&
echo file: binary: set >expect &&
@@ -339,4 +342,31 @@
test_cmp expect actual
'
+test_expect_success SYMLINKS 'set up symlink tests' '
+ echo "* test" >attr &&
+ rm -f .gitattributes
+'
+
+test_expect_success SYMLINKS 'symlinks respected in core.attributesFile' '
+ test_when_finished "rm symlink" &&
+ ln -s attr symlink &&
+ test_config core.attributesFile "$(pwd)/symlink" &&
+ attr_check file set
+'
+
+test_expect_success SYMLINKS 'symlinks respected in info/attributes' '
+ test_when_finished "rm .git/info/attributes" &&
+ ln -s ../../attr .git/info/attributes &&
+ attr_check file set
+'
+
+test_expect_success SYMLINKS 'symlinks not respected in-tree' '
+ test_when_finished "rm -rf .gitattributes subdir" &&
+ ln -s attr .gitattributes &&
+ mkdir subdir &&
+ ln -s ../attr subdir/.gitattributes &&
+ attr_check_basic subdir/file unspecified &&
+ test_i18ngrep "unable to access.*gitattributes" err
+'
+
test_done
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index f7abde6..a594b4a 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -865,4 +865,38 @@
test_cmp expect actual
'
+test_expect_success SYMLINKS 'set up ignore file for symlink tests' '
+ echo "*" >ignore &&
+ rm -f .gitignore .git/info/exclude
+'
+
+test_expect_success SYMLINKS 'symlinks respected in core.excludesFile' '
+ test_when_finished "rm symlink" &&
+ ln -s ignore symlink &&
+ test_config core.excludesFile "$(pwd)/symlink" &&
+ echo file >expect &&
+ git check-ignore file >actual 2>err &&
+ test_cmp expect actual &&
+ test_must_be_empty err
+'
+
+test_expect_success SYMLINKS 'symlinks respected in info/exclude' '
+ test_when_finished "rm .git/info/exclude" &&
+ ln -s ../../ignore .git/info/exclude &&
+ echo file >expect &&
+ git check-ignore file >actual 2>err &&
+ test_cmp expect actual &&
+ test_must_be_empty err
+'
+
+test_expect_success SYMLINKS 'symlinks not respected in-tree' '
+ test_when_finished "rm .gitignore" &&
+ ln -s ignore .gitignore &&
+ mkdir subdir &&
+ ln -s ignore subdir/.gitignore &&
+ test_must_fail git check-ignore subdir/file >actual 2>err &&
+ test_must_be_empty actual &&
+ test_i18ngrep "unable to access.*gitignore" err
+'
+
test_done
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 93caf9a..d8e7374 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -932,4 +932,35 @@
test_cmp expect actual
'
+test_expect_success SYMLINKS 'set up symlink tests' '
+ git commit --allow-empty -m foo --author="Orig <orig@example.com>" &&
+ echo "New <new@example.com> <orig@example.com>" >map &&
+ rm -f .mailmap
+'
+
+test_expect_success SYMLINKS 'symlinks respected in mailmap.file' '
+ test_when_finished "rm symlink" &&
+ ln -s map symlink &&
+ git -c mailmap.file="$(pwd)/symlink" log -1 --format=%aE >actual &&
+ echo "new@example.com" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success SYMLINKS 'symlinks respected in non-repo shortlog' '
+ git log -1 >input &&
+ test_when_finished "nongit rm .mailmap" &&
+ nongit ln -sf "$TRASH_DIRECTORY/map" .mailmap &&
+ nongit git shortlog -s <input >actual &&
+ echo " 1 New" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success SYMLINKS 'symlinks not respected in-tree' '
+ test_when_finished "rm .mailmap" &&
+ ln -s map .mailmap &&
+ git log -1 --format=%aE >actual &&
+ echo "orig@example.com" >expect&&
+ test_cmp expect actual
+'
+
test_done
diff --git a/wrapper.c b/wrapper.c
index bcda41e..563ad59 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -678,3 +678,19 @@ int is_empty_or_missing_file(const char *filename)
return !st.st_size;
}
+
+int open_nofollow(const char *path, int flags)
+{
+#ifdef O_NOFOLLOW
+ return open(path, flags | O_NOFOLLOW);
+#else
+ struct stat st;
+ if (lstat(path, &st) < 0)
+ return -1;
+ if (S_ISLNK(st.st_mode)) {
+ errno = ELOOP;
+ return -1;
+ }
+ return open(path, flags);
+#endif
+}
