Re: [PATCH 2/2] add: refuse to add paths beyond repository boundaries

[Junio C Hamano <gitster@pobox.com>]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Currently, git add has the logic for refusing to add gitlinks using
> treat_path(), which in turn calls check_path_for_gitlink().  However,
> this only checks for an in-index submodule (or gitlink cache_entry).
> A path inside a git repository in the worktree still adds fine, and
> this is a bug.  The logic for denying it is very similar to denying
> adding paths beyond symbolic links: die_if_path_beyond_symlink().
> Follow its example and write a die_if_path_beyond_gitrepo() to fix
> this bug.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---

> @@ -166,6 +166,7 @@ static const char **validate_pathspec(const char **argv, const char *prefix)
>  		const char **p;
>  		for (p = pathspec; *p; p++) {
>  			die_if_path_beyond_symlink(*p, prefix);
> +			die_if_path_beyond_gitrepo(*p, prefix);
>  		}
>  	}
> diff --git a/cache.h b/cache.h
> index e1e8ce8..987d7f3 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -962,6 +962,8 @@ struct cache_def {
>  
>  extern int has_symlink_leading_path(const char *name, int len);
> +extern int has_gitrepo_leading_path(const char *name, int len);

I looked at the output from "grep has_symlink_leading_path" and also
for "die_if_path_beyond"; all of these places are checking "I have
this multi-level path; I want to know if the path does not (should
not) be part of the current project", I think.  Certainly the one in
the "update-index" is about the same operation as "git add" you are
patching.

Isn't it a better approach to _rename_ the existing function not to
single out "symlink"-ness of the path first ?  A symlink in the
middle of such a multi-level path that leads to a place outside the
project is _not_ the only way to step out of our project boundary.  A
directory in the middle of a multi-level path that is the top-level
of the working tree of a foreign project is another way to step out
of our project boundary.  Perhaps

	die_if_path_outside_our_project()
        path_outside_our_project()

And then update the implementation of path_outside_our_project(),
which only took "symlink in the middle" into account so far, and
teach it that such a "top-level of the working tree of a foreign
project" is also stepping out of our project?

That way, you do not have to settle on fixing the bug only in "git
add" and keep the bug in "git update-index", I think.

I think the hit in builtin/apply.c deals with the same "beyond
symlink is outside our project" check and can be updated like so.  I
didn't look at the ones in diff-lib.c and dir.c so you may want to
double check on what they use it for.