git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: permissions

From
Thomas Rast <trast@student.ethz.ch>
Date
Jun 9, 2010, 12:06 UTC
Message-ID
<201006091406.50955.trast@student.ethz.ch>
In-Reply-To
<4C0F6CF0.3020603@wpursell.net>
William Pursell wrote:
Show 28 quoted lines
> Alex Riesen wrote:
> > On Wed, Jun 9, 2010 at 00:27, William Pursell <bill.pursell@gmail.com> wrote:
> >> Junio C Hamano wrote:
> >>> Alex Riesen <raa.lkml@gmail.com> writes:
> >>>
> >>>> The patch moves the function is_git_directory at the level of user
> >>>> interface where it wasn't before: it now complains and die.
> >>>> Not all callers of the function call it only to die if it fails.
> >>> Thanks for shooting it down before I had to look at it ;-)
> >> The point of the patch is that it now complains and dies.
> > 
> > At wrong point. Points, actually. There are many callers of the
> > function you modified. You should have looked at them all.
> 
> I did look at all 4 calls, and it seemed to me
> that localizing the change in one location is a better
> design than adding logic to 4 different locations.
> 
> >> Perhaps I'm being obtuse, but can you describe a situation
> >> in which this causes git to terminate inappropriately?
> > 
> > Maybe. BTW, can you? (if you try, I mean).
> 
> No, I can't.  As far as I can tell, the patch adds
> exactly the functionality that I want it to add.  You
> do make good points about its problems below, however,
> and you are right that I did miss the point of
> your criticism.  Thank you for clarifying.

Maybe I'm missing something, but I think that also apart from any meta-criticism the patch is wrong. From the use of setup_git_directory_gently() in cmd_apply() [for example; there are other commands that are supposed to work both in- and outside of repos], I conclude that the invocation of is_git_directory() must not die() because it is *okay* if the directory is, after all, not a git repo.

And I think the same goes for your new patch
Show 12 quoted lines
> @@ -407,6 +413,11 @@ const char *setup_git_directory_gently(int *nongit_ok)
>                 }
>                 if (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
>                         break;
> +               if (access(DEFAULT_GIT_DIR_ENVIRONMENT, X_OK)
> +                                && errno != ENOENT )
> +                       die_errno("Unable to access %s/%s",
> +                                cwd, DEFAULT_GIT_DIR_ENVIRONMENT);
> +
>                 if (is_git_directory(".")) {
>                         inside_git_dir = 1;
>                         if (!work_tree_env)
[DEFAULT_GIT_DIR_ENVIRONMENT is ".git"]

Unless I'm missing something, this effectively prevents git-apply and friends from working outside any repos if your BOFH sysadmin thinks it funny to place an unreadable .git somewhere on the way up to /.

Or maybe we don't care about BOFH ideas?
-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Previous: William PursellNext: Alex Riesen
Message 14 of 17 in “permissions”
  1. William PursellJun 5, 2010
  2. Andreas SchwabJun 5, 2010
  3. William PursellJun 5, 2010
  4. Alex RiesenJun 6, 2010
  5. William PursellJun 6, 2010
  6. Alex RiesenJun 6, 2010
  7. Junio C HamanoJun 6, 2010
  8. William PursellJun 8, 2010
  9. Alex RiesenJun 8, 2010
  10. Junio C HamanoJun 8, 2010
  11. William PursellJun 8, 2010
  12. Alex RiesenJun 9, 2010
  13. William PursellJun 9, 2010
  14. Thomas RastJun 9, 2010
  15. Alex RiesenJun 9, 2010
  16. Alex RiesenJun 9, 2010
  17. Steven MichalskeJun 9, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.