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
This commit is contained in:
Junio C Hamano 2021-03-22 14:00:22 -07:00
commit 204333b015
10 changed files with 197 additions and 41 deletions

60
attr.c
View File

@ -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);

View File

@ -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);

21
dir.c
View File

@ -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;

3
dir.h
View File

@ -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,

View File

@ -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

View File

@ -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;
}

View File

@ -4,12 +4,16 @@ test_description=gitattributes
. ./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_expect_success 'binary macro expanded by -a' '
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_expect_success 'query binary macro directly' '
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

View File

@ -865,4 +865,38 @@ test_expect_success 'info/exclude trumps core.excludesfile' '
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

View File

@ -932,4 +932,35 @@ test_expect_success 'find top-level mailmap from subdir' '
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

View File

@ -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
}