Re: Git Hooks

[Jeff King]

On Fri, Dec 15, 2017 at 12:48:07PM -0800, Satyakiran Duggina wrote:

> To give the code pullers a chance to review, can we not have a
> `trusted-hooks: default` and `trusted-SHA: ` field in .git/.
> I'm assuming githooks/ are source tracked here.
> 
> When developer tries to execute `git commit`, git can ask developer to
> change `trusted-hooks` field to true or false. Let's say developer
> sets it to true, git can record the SHA. If any latest pull has the
> hooks changed, git can revert the `trusted-hook` to default.
> 
> This way there is not much hassle for developers to manually copy
> hooks all the time. And at the same time, they are not running scripts
> that they haven't reviewed.

We've talked about doing something like this (though more for config
than for hooks). But what the discussion always come down to is that
carrying a script like "import-hooks.sh" in the repository ends up being
the exact same amount of work for the developer as any git-blessed "OK,
trust these hooks" command.

And it's a lot more flexible. The writer of that script can touch hooks,
config, etc. They can base decisions about what values to use based on
data that Git otherwise wouldn't care about (e.g., uname). And they only
have to handle cases that the project cares about, whereas anything Git
does has to work everywhere.

-Peff

Re: Git Hooks

[Satyakiran Duggina]

Thanks, Bryan.

To give the code pullers a chance to review, can we not have a
`trusted-hooks: default` and `trusted-SHA: ` field in .git/.
I'm assuming githooks/ are source tracked here.

When developer tries to execute `git commit`, git can ask developer to
change `trusted-hooks` field to true or false. Let's say developer
sets it to true, git can record the SHA. If any latest pull has the
hooks changed, git can revert the `trusted-hook` to default.

This way there is not much hassle for developers to manually copy
hooks all the time. And at the same time, they are not running scripts
that they haven't reviewed.

Will this work?



On Fri, Dec 15, 2017 at 11:23 AM, Bryan Turner  wrote:
> On Fri, Dec 15, 2017 at 11:12 AM, Satyakiran Duggina
>  wrote:
>> I see that `git init` creates a .git directory and hooks are to be
>> placed in that directory and these hooks are not tracked by version
>> control. To achieve tracked hooks, either each developer has to copy
>> the hooks or use tools like overcommit, pre-commit, husky etc.
>>
>> I'm wondering why hooks are not made external like .gitignore. I guess
>> it would be better to have two git configuration directories in a
>> repo, one hosting all the metadata managed by git and the other with
>> user configured data (hooks, ignore/exclude, repo config etc).
>
> Hooks are not external because they're not trusted. It essentially
> amounts to allowing someone to download an arbitrary script or program
> onto your computer which you then execute. It's extremely unsafe, and
> is intentionally not possible. To get hooks in your instance, you have
> to _manually_ install them. This gives you a chance to _review_ them
> before they start executing on your system. Any other approach and the
> hooks become an attack vector.
>
>>
>> Kindly let me know why the current design choice is made and if the
>> proposed change would introduce unseen issues.
>>
>>
>> Thanks,
>> Satya
>
> Hope this helps!
> Bryan Turner



-- 
Regards & Thanks
Satya Kiran Duggina

Re: Git Hooks

[Bryan Turner]

On Fri, Dec 15, 2017 at 11:12 AM, Satyakiran Duggina
 wrote:
> I see that `git init` creates a .git directory and hooks are to be
> placed in that directory and these hooks are not tracked by version
> control. To achieve tracked hooks, either each developer has to copy
> the hooks or use tools like overcommit, pre-commit, husky etc.
>
> I'm wondering why hooks are not made external like .gitignore. I guess
> it would be better to have two git configuration directories in a
> repo, one hosting all the metadata managed by git and the other with
> user configured data (hooks, ignore/exclude, repo config etc).

Hooks are not external because they're not trusted. It essentially
amounts to allowing someone to download an arbitrary script or program
onto your computer which you then execute. It's extremely unsafe, and
is intentionally not possible. To get hooks in your instance, you have
to _manually_ install them. This gives you a chance to _review_ them
before they start executing on your system. Any other approach and the
hooks become an attack vector.

>
> Kindly let me know why the current design choice is made and if the
> proposed change would introduce unseen issues.
>
>
> Thanks,
> Satya

Hope this helps!
Bryan Turner

Git Hooks

[Satyakiran Duggina]

I see that `git init` creates a .git directory and hooks are to be
placed in that directory and these hooks are not tracked by version
control. To achieve tracked hooks, either each developer has to copy
the hooks or use tools like overcommit, pre-commit, husky etc.

I'm wondering why hooks are not made external like .gitignore. I guess
it would be better to have two git configuration directories in a
repo, one hosting all the metadata managed by git and the other with
user configured data (hooks, ignore/exclude, repo config etc).

Kindly let me know why the current design choice is made and if the
proposed change would introduce unseen issues.


Thanks,
Satya

Re: Security of .git/config and .git/hooks

[Stefan Beller]

So once upon a time we compared Gits security model with a
web browser. A web browser lets you execute 3rd party code
(e.g. javascript) and it is supposedly safe to look at malicious sites.

Currently Git only promises to have the clone/fetch operation safe,
not the "here is a zip of my whole repo" case, which sounds more
like the web browser experience ("here is a site with js, even zipped
in transfer"). Tightening the security model of Git towards this seems
like a good idea to me.

>>  1. Introduce a (configurable) list of "safe" configuration items that
>> can be set in .git/config and don't respect any others.
>
> A whitelist is obviously safer than a blacklist. Though I also feel like
> some of the options may give an unexpectedly wide attack surface. I.e.,
> I wouldn't be surprised if some innocent-looking option ends up being
> used in a tricky way to gain more access. E.g., submodule config
> pointing to paths outside of the repository.
>
> Do you plan to run in safe-mode all the time? What if safe-mode was a
> lot more extreme, and simply avoided reading repo-level config at all
> (except for check_repository_format(), which should be pretty innocent).
>
> I have a feeling there are some features (like submodules) that would
> simply be broken in safe-mode.

I would think that the essential submodule things would be "safe"
to look at. But e.g. submodule..update = "!rm -rf /" would be
not ok, hence the .update configuration would be in the unsafe space.

Any unsafe config option would need to be set outside the actual
repository (~/.config/git//config ?)

>
>>  2. But what if I want to set a different pager per-repository?
>> I think we could do this using configuration "profiles".
>> My ~/.config/git/profiles/ directory would contain git-style
>> config files for repositories to include.  Repositories could
>> then contain
>>
>>   [include]
>>   path = ~/.config/git/profiles/fancy-log-pager
>>
>> to make use of those settings.  The facility (1) would
>> special-case this directory to allow it to set "unsafe" settings
>> since files there are assumed not to be under the control of an
>> attacker.
>
> You can do something quite similar already:
>
>   git config --global \
> include.gitdir:/path/to/specific/repo.path
> .gitconfig-fancy-log-pager
>
> The main difference is that the profile inclusion is done by path rather
> than riding along with the repository directory if it gets moved. In
> practice I doubt that matters much, and I think the security model for
> include.gitdir is a lot simpler.

I am not sure if this works so well for the submodule..update
config (that we want to deprecate anyway, but still)

>> For backward compatibility, this facility would be controlled by a
>> global configuration setting.  If that setting is not enabled, then
>> the current, less safe behavior would remain.
>
> Are config and symlinks everything we need to care about? I can't think
> of anything else, but Git really has quite a large attack surface when
> accessing a local repo. Right now the safest thing you can do is "git
> clone --no-local" an untrusted repo and then look only at the clone. Of
> course nobody _actually_ does that, so any "safe" mode seems like it
> would be an improvement. But would claiming to have a "safe" mode
> encourage people to use it to look at risky repositories, exacerbating
> any holes (e.g., exploiting a bug in the index format)? I don't know.

Good point. Though we only care about the case of breaking out and
executing untrusted code; most of the index exploits would rather
trigger a segfault or infinite lop (in my imagination at least).

>
> -Peff

Re: Security of .git/config and .git/hooks

[Jeff King]

On Mon, Oct 02, 2017 at 04:45:17PM -0700, Jonathan Nieder wrote:

> This topic has been mentioned on this mailing list before but I had
> trouble finding a relevant reference.  Links welcome.

There were discussions long ago related to the upload-pack hook. One of
the proposed fixes was checking the owner of the hook against the
running user, but in the end we just removed it.

Some of the discussion I have in my mail archive is off-list, but it was
brought up on-list later, too:

  
https://public-inbox.org/git/6f8b45101001141001q40d8b746v8385bc6ae37a6...@mail.gmail.com/

The owner-match thing is intriguing, but I think it only helps with
multi-user systems. For your zipfile case, there's literally no
information on the disk about whether the repo is trusted or not. You'd
have to put Git into an "untrusted" mode.

There's also some prior art in the uploadpack.packObjectsHook, which is
ignored totally in repo-level config.

> Suppose that I add the following to .git/config in a repository on a
> shared computer:
> 
>   [pager]
>   log = rm -fr /
>   fsck = rm -fr /
> 
> ("rm -fr /" is of course a placeholder here.)
> 
> I then tell a sysadmin that git commands are producing strange output
> and I am having trouble understanding what is going on.  They may run
> "git fsck" or "git log"; in either case, the output is passed to the
> pager I configured, allowing me to run an arbitrary command using the
> sysadmin's credentials.

I know you probably didn't mean this to be an exhaustive list, but there
are really a ton of config options that can result in executing
arbitrary commands. External diff, textconv, ssh commands, and so on.

I don't think that changes your point any, but it's something to keep in
mind when evaluating solutions:

  - if individual options need to be annotated as unsafe, there's a high
risk of missing an option (or introducing a new one incorrectly)

  - any schemes which reduce functionality (e.g., by disallowing certain
options in repo config by default) are going to affect a lot of
people

> Proposed fix: because of case (1), I would like a way to tell Git to
> stop trusting any files in .git.  That is:
> 
>  1. Introduce a (configurable) list of "safe" configuration items that
> can be set in .git/config and don't respect any others.

A whitelist is obviously safer than a blacklist. Though I also feel like
some of the options may give an unexpectedly wide attack surface. I.e.,
I wouldn't be surprised if some innocent-looking option ends up being
used in a tricky way to gain more access. E.g., submodule config
pointing to paths outside of the repository.

Do you plan to run in safe-mode all the time? What if safe-mode was a
lot more extreme, and simply avoided reading repo-level config at all
(except for check_repository_format(), which should be pretty innocent).

I have a feeling there are some features (like submodules) that would
simply be broken in safe-mode.

>  2. But what if I want to set a different pager per-repository?
> I think we could do this using configuration "profiles".
> My ~/.config/git/profiles/ directory would contain git-style
> config files for repositories to include.  Repositories could
> then contain
> 
>   [include]
>   path = ~/.config/git/profiles/fancy-log-pager
> 
> to make use of those settings.  The facility (1) would
> special-case this directory to allow it to set "unsafe" settings
> since files there are assumed not to be under the control of an
> attacker.

You can do something quite similar already:

  git config --global \
include.gitdir:/path/to/specific/repo.path
.gitconfig-fancy-log-pager

The main difference is that the profile inclusion is done by path rather
than riding along with the repository directory if it gets moved. In
practice I doubt that matters much, and I think the security model for
include.gitdir is a lot simpler.

>  3. Likewise for hooks: my ~/.config/git/hooks/ directory would
> contain hooks for repositories to make use of.  Repositories could
> symlink to hook files from there to make use of them.
> 
> That would allow the configured hooks in ~/.config/git/hooks/ to
> be easy to find and to upgrade in one place.
> 
> To help users migrate, when a hook is present and executable in
> .git/hooks/, Git would print instructions for moving it to
> ~/.config/git/hooks/ and replacing it with a symlink after
> inspecting it.

I kind of wonder if the path-limited includes from above plus
core.hooksPath would be simpler. The source of authority then is always
outside of the repository, rather than trying to vet some values inside
the repository.

Reading more, I think I just reinvented you

Re: Security of .git/config and .git/hooks

[Christian Couder]

Hi,

On Tue, Oct 3, 2017 at 1:45 AM, Jonathan Nieder  wrote:

> Proposed fix: because of case (1), I would like a way to tell Git to
> stop trusting any files in .git.  That is:
>
>  1. Introduce a (configurable) list of "safe" configuration items that
> can be set in .git/config and don't respect any others.

Maybe we can already add a --list-security or --check-security or
--unsafe to `git config` to list the unsafe options and their values
as well as the active hooks, so that admins/users can already easily
take a quick look at the config before they start playing with a
potentially unsafe repo.

Re: Security of .git/config and .git/hooks

[Junio C Hamano]

Jonathan Nieder <jrnie...@gmail.com> writes:

> Proposed fix: because of case (1), I would like a way to tell Git to
> stop trusting any files in .git.  That is:
>
>  1. Introduce a (configurable) list of "safe" configuration items that
> can be set in .git/config and don't respect any others.

The list of "safe" things are configurable by having something in
~/.gitconfig, perhaps?

How would this work, from the end-user's point of view, with "git
config --global" and "git config --local"?

>  2. But what if I want to set a different pager per-repository?
> I think we could do this using configuration "profiles".
> My ~/.config/git/profiles/ directory would contain git-style
> config files for repositories to include.  Repositories could
> then contain
>
>   [include]
>   path = ~/.config/git/profiles/fancy-log-pager
>
> to make use of those settings.  The facility (1) would
> special-case this directory to allow it to set "unsafe" settings
> since files there are assumed not to be under the control of an
> attacker.

Meaning, "include" is not in "safe" category, but a value that
begins with "~/.config/git/" are excempt???

>  3. Likewise for hooks: my ~/.config/git/hooks/ directory would
> contain hooks for repositories to make use of.  Repositories could
> symlink to hook files from there to make use of them.

I am not sure what this means.  .git/hooks/pre-commit being a
symbolic link to "~/.config/git/hooks/pre-commit-fancy"
(i.e. readlink gives the path with tilde unexpanded), so that the
attacked sysadmin will not run it from ~attacker/.config/git/hooks?  

And the code that finds a hook to run sees .git/hooks/pre-commit,
resolves the symlink manually and makes sure it leads to somewhere
inside ~/.config/...  (otherwise it rejects) and then uses the
pointed-at copy?

At that point, we are not taking any advantage of symbolic-link-ness
of the entity, so .git/hooks/pre-commit being a text file that has a
single like, e.g.

# safe-hook: pre-commit-fancy

may be sufficient (and we do not have to worry about systems without
symbolic links)?  The machinery that used to manually resolved symlink
instead reads it, finds "pre-commit-fancy" in ~/.config/git/hooks/
and runs it, and you get the same behaviour, no?

> One downside of (3) is its reliance on symlinks.  Some alternatives:
>
>  3b. Use core.hooksPath configuration instead.  Rely on (2).
>  3c. Introduce new hook.* configuration to be used instead of hook
>  scripts.  Rely on (2).

I guess I invented 3d. without reading ahead X-<.  None of the 3x
variants other than 3 proper will not work for scripts and existing
code that sees that .git/hooks/pre-commit is an executable and runs
it, and 3 proper will not work without symbolic links, so this means
we'd need "git locate-hook pre-commit" (and underlying locate_hook()
helper API) that returns "/home/me/.git/config/hook/pre-commit-fancy"
or fails when we do this transition.  In an unconverted repository,
it may return $PWD/.git/hooks/pre-commit, or failure if we are
running under the paranoid mode.

Sounds workable.

Security of .git/config and .git/hooks

[Jonathan Nieder]

Hi,

This topic has been mentioned on this mailing list before but I had
trouble finding a relevant reference.  Links welcome.

Suppose that I add the following to .git/config in a repository on a
shared computer:

[pager]
log = rm -fr /
fsck = rm -fr /

("rm -fr /" is of course a placeholder here.)

I then tell a sysadmin that git commands are producing strange output
and I am having trouble understanding what is going on.  They may run
"git fsck" or "git log"; in either case, the output is passed to the
pager I configured, allowing me to run an arbitrary command using the
sysadmin's credentials.

You might say that this is the sysadmin's fault, that they should have
read through .git/config before running any Git commands.  But I don't
find it so easy to blame them.

A few related cases that might not seem so dated:

 1. I put my repository in a zip file and ask a Git expert to help me
recover data from it, or

 2. My repository is in a shared directory on NFS.  Unless the
administrator setting that up is very careful, it is likely that
the least privileged user with write access to .git/config or
.git/hooks/ may be someone that I don't want to be able to run
arbitrary commands on behalf of the most privileged user working
in that repository.

A similar case to compare to is Linux's "perf" tool, which used to
respect a .perfconfig file from the current working directory.
Fortunately, nowadays "perf" only respects ~/.perfconfig and
/etc/perfconfig.

Proposed fix: because of case (1), I would like a way to tell Git to
stop trusting any files in .git.  That is:

 1. Introduce a (configurable) list of "safe" configuration items that
can be set in .git/config and don't respect any others.

 2. But what if I want to set a different pager per-repository?
I think we could do this using configuration "profiles".
My ~/.config/git/profiles/ directory would contain git-style
config files for repositories to include.  Repositories could
then contain

[include]
path = ~/.config/git/profiles/fancy-log-pager

to make use of those settings.  The facility (1) would
special-case this directory to allow it to set "unsafe" settings
since files there are assumed not to be under the control of an
attacker.

 3. Likewise for hooks: my ~/.config/git/hooks/ directory would
contain hooks for repositories to make use of.  Repositories could
symlink to hook files from there to make use of them.

That would allow the configured hooks in ~/.config/git/hooks/ to
be easy to find and to upgrade in one place.

    To help users migrate, when a hook is present and executable in
    .git/hooks/, Git would print instructions for moving it to
~/.config/git/hooks/ and replacing it with a symlink after
inspecting it.

For backward compatibility, this facility would be controlled by a
global configuration setting.  If that setting is not enabled, then
the current, less safe behavior would remain.

One downside of (3) is its reliance on symlinks.  Some alternatives:

 3b. Use core.hooksPath configuration instead.  Rely on (2).
 3c. Introduce new hook.* configuration to be used instead of hook
 scripts.  Rely on (2).

Thoughts?
Jonathan

Re: [PATCH] respect core.hooksPath, falling back to .git/hooks

[Johannes Schindelin]

Hi Philipp,

On Fri, 2 Jun 2017, Philipp Gortan wrote:

> Signed-off-by: Philipp Gortan <phil...@gortan.org>

I just saw this. I made sure that the thread to which I just replied did
not have any news from you, but you simply started a new thread ;-)

This commit message needs a little bit of love. Something along the lines:

Since v2.9.0, Git knows about the config variable core.hookspath
that allows overriding the path to the directory containing the
    Git hooks.

Since v2.10.0, the `--git-path` option respects that config
variable, too, so we may just as well use that command.

For Git versions older than v2.5.0 (which was the first version to
support the `--git-path` option for the `rev-parse` command), we
simply fall back to the previous code.

(This assumes that you'll go with the approach I outlined in the other
thread, comparing the Git version to 2.5.0 and going with --git-path if
available.)

> ---
> 
> The following patch tries to fix git-gui to respect the core.hooksPath config
> variable, falling back to the old behavior.

That would also have been a decent commit message, if a bit short. But you
need to put this text before the `---` line, even before the
`Signed-off-by:` footer.

> diff --git a/git-gui.sh b/git-gui.sh
> index 5bc21b8..a5335b1 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -624,7 +624,10 @@ proc git_write {args} {
>  }
>  
>  proc githook_read {hook_name args} {
> - set pchook [gitdir hooks $hook_name]
> + if {[catch {set hooksdir [git config core.hooksPath]}]} {

Did you not mean [get_config core.hookspath] here, i.e. get_config and the
key all lower-case?

> + set hooksdir [gitdir hooks]
> + }
> + set pchook [file join $hooksdir $hook_name]
>   lappend args 2>@1
>  

The problem I see with that is, as I mentioned in the other thread, that
it duplicates the logic in config.c that may change at any stage.

Even worse: it is inconsistent with the way Git handles core.hooksPath, if
the installed `git` executable predates v2.9.0. Git GUI explicitly allows
for being used with a large range of Git versions.

In short: I think it would be better to go with the approach I outlined in
the other thread. I'll reproduce the patch (completely untested) here:

-- snipsnap --
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 37c1c5d227b..3067a3b000a 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -624,7 +624,11 @@ proc git_write {args} {
 }
 
 proc githook_read {hook_name args} {
-   set pchook [gitdir hooks $hook_name]
+   if {[package vcompare $::_git_version 2.5.0] >= 0} {
+   set pchook [git rev-parse --git-path "hooks/$hook_name"]
+   } else {
+   set pchook [gitdir hooks $hook_name]
+   }
lappend args 2>@1
 
# On Windows [file executable] might lie so we need to ask

Re: [PATCH] respect core.hooksPath, falling back to .git/hooks

[Philipp Gortan]

Dear Philip,

the previous mail contains a patch against the master of
http://repo.or.cz/git-gui.git

Could you please review it? I am not a TCL developer, so please take
extra care!

Thanks,
Philipp



signature.asc
Description: OpenPGP digital signature

[PATCH] respect core.hooksPath, falling back to .git/hooks

[Philipp Gortan]

Signed-off-by: Philipp Gortan 
---

The following patch tries to fix git-gui to respect the core.hooksPath config
variable, falling back to the old behavior.

git-gui.sh | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/git-gui.sh b/git-gui.sh
index 5bc21b8..a5335b1 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -624,7 +624,10 @@ proc git_write {args} {
 }
 
 proc githook_read {hook_name args} {
-   set pchook [gitdir hooks $hook_name]
+   if {[catch {set hooksdir [git config core.hooksPath]}]} {
+   set hooksdir [gitdir hooks]
+   }
+   set pchook [file join $hooksdir $hook_name]
lappend args 2>@1
 
# On Windows [file executable] might lie so we need to ask
-- 
2.13.0

Re: Additional git hooks

[Jeff King]

On Thu, Dec 15, 2016 at 09:14:30AM -0500, Jeff King wrote:

> On Thu, Dec 15, 2016 at 02:57:18PM +0100, Chiel ten Brinke wrote:
> 
> > Would patches introducing new git hooks, e.g. for post-fetch, be
> > eligible for acceptance?
> 
> The general guidelines for adding hooks is laid out here:
> 
>   http://public-inbox.org/git/7vbq7ibxhh@gitster.siamese.dyndns.org/

One interesting follow-up, though (which seems sensible to me):

  http://public-inbox.org/git/7vr5fraxbf@alter.siamese.dyndns.org/

-Peff

Re: Additional git hooks

[Jeff King]

On Thu, Dec 15, 2016 at 02:57:18PM +0100, Chiel ten Brinke wrote:

> Would patches introducing new git hooks, e.g. for post-fetch, be
> eligible for acceptance?

The general guidelines for adding hooks is laid out here:

  http://public-inbox.org/git/7vbq7ibxhh@gitster.siamese.dyndns.org/

-Peff

Additional git hooks

[Chiel ten Brinke]

Would patches introducing new git hooks, e.g. for post-fetch, be
eligible for acceptance?
If a more detailed explanation about a specific use case is required,
I'd be happy to provide it.


Btw, the link in the README
http://news.gmane.org/gmane.comp.version-control.git/ is dead.


Chiel

Re: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> I think it's fair enough to say that if we had this facility this
> would be good enough:
>
>  * Your hooks are executed in glob() order, local .git first, then 
> /etc/git/...
>
>  * If it's a hook like pre-commit that can reject something the first
> hook to fail short-circuits. I.e. none of the rest get executed.
>
>  * If it's not a hook like that e.g. post-commit all of the hooks will
> get executed.
>
>  * If you need anything more complex you can just wrap your hooks in
> your own shellscript.
>
> I.e. it takes care of the common case where:
>
>  * You just want to execute N hooks and don't want to write a wrapper.
>
>  * For pre-* hooks the common case is it doesn't matter /what/
> rejected things, just that it gets rejected, e.g. for pre-receive.
> Also if you care about performance you can order them in
> cheapest-first order.

Stop using the word "common" to describe what is not demonstratably
"common".

The above only covers a very limited subset of the use cases, which
is the two bullet points above (one of them i.e. "I do not bother to
write a wrapper" is not even a valid use case).  That may be a good
starting point, but it is so simple that can be done with a wrapper
with several lines at most.  So I am not sympathetic to that line of
reasoning at all.

I can buy "It is too cumbersome to require everybody to reinvent and
script the cascading logic, and the core side should help by giving
a standard interface that is flexible enough to suit people's need",
though.

And I have to say that a sequential execution that always
short-circuits at the first failure is below that threshold.

One reason I care about allowing the users to specify "do not
shortcut" is that I anticipate that people would want to have a
logging of the result at the end of the chain.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Marc Branchaud]

On 2016-04-26 12:09 PM, Ævar Arnfjörð Bjarmason wrote:


Basically you can look at patches to a project on a spectrum of:

  1. You hacked something up locally
  2. It's in someone's *.git repo as a POC
  3. It's a third-party patch series used by a bunch of people
  4. In an official release but documented as experimental
  5. In an official release as a first-rate feature

Something like an experimental.WHATEVER=bool flag provides #4.


But the git project already does #4 without needing a special 
configuration tree.  In fact, you ignored the git project's "pu" branch 
entirely.  Once a feature gets onto the "next" branch, it's already much 
less experimental.  If it needs to put the word "experimental" in its 
config settings, then maybe shouldn't leave the "pu" branch in the first 
place.



I think aside from this feature just leaving these things undocumented
really provides the worst of both worlds.


Yes, I apologize.  I did not mean that things should remain 
undocumented, only that if you're afraid of users harming themselves 
then you're better off not documenting settings than labelling them as 
experimental.



Now you have some feature that's undocumented *because* you're not
sure about it, but you can't ever be sure about it unless people
actually use it, and if it's not documented at all effectively it's as
visible as some third-party patch series. I.e. only people really
involved in the project will ever use it.


Slapping "experimental" on the configuration only serves to muddy the 
waters.  Either the feature is good enough to be tried by normal users, 
or it isn't.  If it isn't ready for normal users, let it cook in pu (or 
next) for a while longer.


Git has got on just fine without having some special designation for 
not-ready-for-prime-time features, mostly because the development 
process lends itself naturally to gradually exposing things as they 
mature: topics move from the list to pu to next to master.  (The string 
"experiment" only appears 16 times in all the release notes, which I 
think is something the git project can be proud of.)


To me, designating part of the config tree as "experimental" enables 
sloppier practices, where a feature can be released with a bit less 
effort than might otherwise be acceptable, because it's clearly marked 
as experimental, and so anyone trying it out surely has the requisite 
bulletproof footwear.  (I don't mean to imply that you or any other git 
contributor might slack off on any work you do for the project. It's 
more that the ability to easily designate something as experimental 
lowers the bar a bit, and makes it more tempting to release 
not-quite-ready features.)


Far better to instead work on the feature until it's as ready as can be, 
and release it properly.


In this particular case, for example, I think your proposed approach is 
perfectly fine and does not need to be designated as "experimental" in 
any way.  With a reasonable "hooks.something" config variable to turn on 
directory support, what you've described sounds like a great feature. 
Sure, it may have bugs, it may have unintended consequences, it may not 
fulfill some odd corner cases.  But that's true for almost everything. 
As with every other git feature, it'll be developed to the best of the 
project's abilities and released.  Future patches are welcome.


M.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Christian Couder]

On Tue, Apr 26, 2016 at 6:09 PM, Ævar Arnfjörð Bjarmason
 wrote:
> On Tue, Apr 26, 2016 at 3:40 PM, Marc Branchaud  wrote:
>> On 2016-04-26 06:58 AM, Ævar Arnfjörð Bjarmason wrote:
>>>
>>> Makes sense to have an experimental.* config tree for git for stuff like 
>>> this.
>>
>> I disagree.
>>
>> * If the point is to express some kind of warning to users, I think the
>> community has been much better served by leaving experimental settings
>> undocumented (or documented only in unmerged topic branches).

There are features considered experimental that are documented, like
--split-index in `git update-index` and `git interpret-trailers`.

As far as I know the possible reasons they are considered experimental are:

- in the release notes they were described as "experimental",
- they are not considered complete, because of missing features,
- they might not be useful as is,
- they might be buggy in the real world.

>> It feels like
>> an experimental.* tree is a doorway to putting experimental features in
>> official releases, which seems odd considering that (IMHO) git has so far
>> done very well with the carefully-planned-out integration of all sorts of
>> features.

Some people have been happily experimenting with or even using some of
the experimental features, like the ones I am talking about above.

>> * Part of the experiment is coming up with appropriate configuration knobs,
>> including where those knobs should live.

I agree that it can be difficult to find the right knobs for any new feature.

>> Often such considerations lead to a
>> better implementation for the feature.  Dumping things into an experimental.*
>> tree would merely postpone that part of the feature's design.

I am not so sure about this. For example `git update-index
--untracked-cache` used to be considered experimental, but it
definitely had knobs that were not right, so its UI has been reworked
recently.

Maybe if it had first been created with an UI in an experimental.*
config tree, rather than only options to `git update-index` it would
have been an easier transition.

>> * Such a tree creates a flag day when the experimental feature eventually
>> becomes a "real" feature. That'll annoy any early adopters. Sure, they
>> *should* be prepared to deal with config tree bike-shedding, but still that
>> extra churn seems unnecessary.

Not sure about that. New config options outside the experimental.*
config tree could always take over config options in the
experimental.* config tree, as if they appear, it means that the user
is aware that they are the new way to configure the feature. Then the
cost of supporting both the old options in the experimental.* config
tree and the new ones outside should be very small, especially if
there are not many changes between the two set of options.

If there are a lot of changes between these two sets, it means that we
were probably right to have the old ones in the experimental.* config
tree. And in the worst case, which should hardly ever happen, we can
probably afford to just emit warnings saying "old 'experimental.'
config option is not supported anymore, please use  config option
instead" and just ignore the 'experimental.' config option.

> By "stuff like this", yeah I did mean, and I assume Junio means,
> putting "experimental" features in official releases.
>
> E.g. perl does this, if you type "perldoc experimental" on a Linux box
> you'll get the documentation.
>
> Basically you can look at patches to a project on a spectrum of:
>
>  1. You hacked something up locally
>  2. It's in someone's *.git repo as a POC
>  3. It's a third-party patch series used by a bunch of people
>  4. In an official release but documented as experimental
>  5. In an official release as a first-rate feature

Yeah, and it seems to me that Git also has:

4.5. In an official release, documented, but scarcely documented as experimental

and in fact more stuff under 4.5. than under 4.

And in Git there is also the notion of porcelain vs plumbing, where
porcelain output can more easily be changed a bit than plumbing
output.

> Something like an experimental.WHATEVER=bool flag provides #4.
>
> I think aside from this feature just leaving these things undocumented
> really provides the worst of both worlds.

Yeah I agree. Undocumented stuff in Git is already for stuff that we
don't want people to use.

> Now you have some feature that's undocumented *because* you're not
> sure about it, but you can't ever be sure about it unless people
> actually use it, and if it's not documented at all effectively it's as
> visible as some third-party patch series. I.e. only people really
> involved in the project will ever use it.
>
> Which is why perl has the "experimental" subsystem, it allows for
> playing around with features the maintainers aren't quite sure about
> in official releases, and the users know they're opting in to trying
> something unstable that may go away or 

Re: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Ævar Arnfjörð Bjarmason]

On Tue, Apr 26, 2016 at 3:40 PM, Marc Branchaud  wrote:
> On 2016-04-26 06:58 AM, Ævar Arnfjörð Bjarmason wrote:
>>
>> Makes sense to have an experimental.* config tree for git for stuff like 
>> this.
>
> I disagree.
>
> * If the point is to express some kind of warning to users, I think the
> community has been much better served by leaving experimental settings
> undocumented (or documented only in unmerged topic branches).  It feels like
> an experimental.* tree is a doorway to putting experimental features in
> official releases, which seems odd considering that (IMHO) git has so far
> done very well with the carefully-planned-out integration of all sorts of
> features.
>
> * Part of the experiment is coming up with appropriate configuration knobs,
> including where those knobs should live.  Often such considerations lead to a
> better implementation for the feature.  Dumping things into an experimental.*
> tree would merely postpone that part of the feature's design.
>
> * Such a tree creates a flag day when the experimental feature eventually
> becomes a "real" feature. That'll annoy any early adopters. Sure, they
> *should* be prepared to deal with config tree bike-shedding, but still that
> extra churn seems unnecessary.

By "stuff like this", yeah I did mean, and I assume Junio means,
putting "experimental" features in official releases.

E.g. perl does this, if you type "perldoc experimental" on a Linux box
you'll get the documentation.

Basically you can look at patches to a project on a spectrum of:

 1. You hacked something up locally
 2. It's in someone's *.git repo as a POC
 3. It's a third-party patch series used by a bunch of people
 4. In an official release but documented as experimental
 5. In an official release as a first-rate feature

Something like an experimental.WHATEVER=bool flag provides #4.

I think aside from this feature just leaving these things undocumented
really provides the worst of both worlds.

Now you have some feature that's undocumented *because* you're not
sure about it, but you can't ever be sure about it unless people
actually use it, and if it's not documented at all effectively it's as
visible as some third-party patch series. I.e. only people really
involved in the project will ever use it.

Which is why perl has the "experimental" subsystem, it allows for
playing around with features the maintainers aren't quite sure about
in official releases, and the users know they're opting in to trying
something unstable that may go away or have its semantics changed from
under them.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Marc Branchaud]

On 2016-04-26 06:58 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> Makes sense to have an experimental.* config tree for git for stuff like this.

I disagree.

* If the point is to express some kind of warning to users, I think the
community has been much better served by leaving experimental settings
undocumented (or documented only in unmerged topic branches).  It feels like
an experimental.* tree is a doorway to putting experimental features in
official releases, which seems odd considering that (IMHO) git has so far
done very well with the carefully-planned-out integration of all sorts of
features.

* Part of the experiment is coming up with appropriate configuration knobs,
including where those knobs should live.  Often such considerations lead to a
better implementation for the feature.  Dumping things into an experimental.*
tree would merely postpone that part of the feature's design.

* Such a tree creates a flag day when the experimental feature eventually
becomes a "real" feature. That'll annoy any early adopters. Sure, they
*should* be prepared to deal with config tree bike-shedding, but still that
extra churn seems unnecessary.

M.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 25, 2016 at 7:45 PM, Junio C Hamano  wrote:
> Ævar Arnfjörð Bjarmason  writes:
>
>> The reason for supporting the *.d directories was that I spotted a lot
>> of hooks people had hacked up at work using the pee(1) command[1] to
>> run sequences of other unrelated hook commands.
>
> IIRC, we wanted to do this several years ago but after discussion
> decided that we didn't want to have this in the core, because we
> didn't want to hardcode the policy on interaction among multiple
> hooks.

Ah, would be interesting to see that discussion if someone knows
enough to dig it up, didn't find it with some brief searching.

> You can easily resolve the ordering of hooks--just declare that they
> are executed sequentially in strcmp() order of filenames and users
> will know to prefix them with fixed-number-of-digits to force their
> desired ordering without complaining.

In principle you're describing glob() order here:
http://pubs.opengroup.org/onlinepubs/7908799/xsh/glob.html

We don't set LC_COLLATE in git so this'll be C order, which I think
will just fall back to strcmp().

If it doesn't and there's a functional difference, I'm not sure, it's
probably less confusing to use glob() order, since that's what you'll
get with shell expansion.

I.e. it would be confusing if you expand the hooks in the shell, and
git executes them in a slightly different order.

> What is harder and the core part cannot unilaterally dictate is what
> should happen after seeing a failure/rejection from a hook.  Some
> hooks among the remainder would not want to be even called.  Some
> others do want to be called but want to learn that the previous
> hooks already have decided to fail/reject the operation.  There may
> even be some others that cannot be moved to earlier part of the hook
> chain for other external constraints (e.g. side effect of some
> previous hook is part of its input), but would want to override the
> previous decision to reject and let the operation pass.

I think it's fair enough to say that if we had this facility this
would be good enough:

 * Your hooks are executed in glob() order, local .git first, then /etc/git/...

 * If it's a hook like pre-commit that can reject something the first
hook to fail short-circuits. I.e. none of the rest get executed.

 * If it's not a hook like that e.g. post-commit all of the hooks will
get executed.

 * If you need anything more complex you can just wrap your hooks in
your own shellscript.

I.e. it takes care of the common case where:

 * You just want to execute N hooks and don't want to write a wrapper.

 * For pre-* hooks the common case is it doesn't matter /what/
rejected things, just that it gets rejected, e.g. for pre-receive.
Also if you care about performance you can order them in
cheapest-first order.

> I am happy to see that the idea brought back alive again, but I
> think we prefer this start its life clearly marked as "highly
> experimental and subject to change", then invite interested and
> brave users who tolerate backward incompatible changes to
> experiment, in order to allow us to gauge what the right semantics
> and flexibility the users would want.  One way to do so may be an
> opt-in configuration variable e.g. "experimental.multiHooks";
> another may be to implement the logic as a pair of scripts (one for
> the command line argument variant, the other for stdin variant) and
> ship them in contrib/.

Makes sense to have an experimental.* config tree for git for stuff like this.

> The latter approach (i.e. scripting) might be easier for people to
> experiment and tweak, and in the olden days that would certainly be
> the approach would would have taken, but I am not too afraid of
> appearing uninviting to casual scripters anymore these days, so...

Yeah, actually one thing I didn't think of is that the core.hooksPath
patch I submitted makes this rather trivial to implement as a
collection of scripts...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> The reason for supporting the *.d directories was that I spotted a lot
> of hooks people had hacked up at work using the pee(1) command[1] to
> run sequences of other unrelated hook commands.

IIRC, we wanted to do this several years ago but after discussion
decided that we didn't want to have this in the core, because we
didn't want to hardcode the policy on interaction among multiple
hooks.

You can easily resolve the ordering of hooks--just declare that they
are executed sequentially in strcmp() order of filenames and users
will know to prefix them with fixed-number-of-digits to force their
desired ordering without complaining.

What is harder and the core part cannot unilaterally dictate is what
should happen after seeing a failure/rejection from a hook.  Some
hooks among the remainder would not want to be even called.  Some
others do want to be called but want to learn that the previous
hooks already have decided to fail/reject the operation.  There may
even be some others that cannot be moved to earlier part of the hook
chain for other external constraints (e.g. side effect of some
previous hook is part of its input), but would want to override the
previous decision to reject and let the operation pass.

I am happy to see that the idea brought back alive again, but I
think we prefer this start its life clearly marked as "highly
experimental and subject to change", then invite interested and
brave users who tolerate backward incompatible changes to
experiment, in order to allow us to gauge what the right semantics
and flexibility the users would want.  One way to do so may be an
opt-in configuration variable e.g. "experimental.multiHooks";
another may be to implement the logic as a pair of scripts (one for
the command line argument variant, the other for stdin variant) and
ship them in contrib/.

The latter approach (i.e. scripting) might be easier for people to
experiment and tweak, and in the olden days that would certainly be
the approach would would have taken, but I am not too afraid of
appearing uninviting to casual scripters anymore these days, so...

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Ævar Arnfjörð Bjarmason]

On Sat, Apr 23, 2016 at 1:33 AM, Ævar Arnfjörð Bjarmason
<ava...@gmail.com> wrote:
> Change the hardcoded lookup for .git/hooks/* to optionally lookup in
> $(git config core.hooksDirectory)/* instead if that config key is set.

I think this'll do for my use-case, but I started with a rather more
ambitious patch that I could forsee not finishing today.

I wanted to support executing e.g. the pre-commit hook, in order, from any of:

    .git/hooks/pre-commit
    .git/hooks/pre-commit.d/*
 [We could add some ~-wide path here I guess...]
     /etc/git/hooks/pre-commit
     /etc/git/hooks/pre-commit.d/*

Where the * would be resolved in glob() order.

The motivation was solving the use-case I'm solving with
core.hooksDirectory, perhaps with a config variable to set whether you
wanted to skip per-repo or system-wide hooks, but also having
something more general.

The reason for supporting the *.d directories was that I spotted a lot
of hooks people had hacked up at work using the pee(1) command[1] to
run sequences of other unrelated hook commands.

Just symlinking stuff is simpler and more portable if we do the work
in git.git once. We'd run  The pee(1) command also doesn't quit on the
first command that returns nonzero, which would make sense e.g. for
pre-commit hooks.

I have it working for the hooks that use the simple run_hook_ve()
interface, but the ones that have to e.g. pass input on stdin just
find_hook() directly & do a custom run_command(), so all of those
callsites would have to be patched and/or I'd have to hack up some
custom callback mechanism.

1. http://manpages.ubuntu.com/manpages/xenial/en/man1/pee.1.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [bug] .git/hooks/commit-msg.sample test is reversed

[Matthieu Moy]

Federico Fissore  writes:

> Hello everyone
>
> In file commit-msg.sample, grep test is reversed. It greps
> '^Signed-off-by: '
> while it should grep
> 'Signed-off-by: '
>
> If you run the sample against attached file, it won't complain. Remove
> the ^ and it will work as expected

I think the test is correct.

In grep, ^ can have two meanings:

'^foo' means "foo at the start of a line"
'[^abc]' means "one character but not a, b or c"

The Signed-off-by: trailer is meant to be at the start of a line, hence
the ^.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[bug] .git/hooks/commit-msg.sample test is reversed

[Federico Fissore]

Hello everyone

In file commit-msg.sample, grep test is reversed. It greps
'^Signed-off-by: '
while it should grep
'Signed-off-by: '

If you run the sample against attached file, it won't complain. Remove 
the ^ and it will work as expected


Regards

Federico
Commit message

Signed-off-by: Me 
Signed-off-by: Me 

git hooks and environment variables

[Jacob Keller]

I've noticed that not all hooks have all of the environment variables
set when they are run, and it is not clear from the manual pages which
hooks have which variables set on the command line. Specifically, they
don't all have GIT_DIR set, I haven't taken the time to verify exactly
which variables are set and which are not yet, but..

Which would be preferred? documentation of the current behaviors, or a
fixup that ensures that all the relevant variables get set?

I intend to have complete details as to which hooks get one settings,
but I had this at work and don't have it here, so I can update this
thread with the information later.

Regards,
Jake
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Feature Request: provide cmdline args to git hooks

[Junio C Hamano]

Chris O'Kelly ch...@mapcreative.com.au writes:

 To reiterate and clarify I'm not saying the undesirable behavior
 is a built in part of git, just a feature of our hosting
 platform's Git deployment mechanisms, although if what you mean is
 that the post-receive hook on the receiving end shouldn't be
 getting passed pushed tag refs that the local git believed to be
 already up to date on the remote (as of most recent fetch), then
 yeah... that is weird because it seems to be the behavior in this
 case.

I just checked.

$ rm -fr new  mkdir new  cd new
$ git init src  git init --bare dst
$ cd src
$ echo (date;cat) pushlog .git/hooks/pre-push
$ chmod +x .git/hooks/pre-push
$ git commit -m 'initial' --allow-empty
$ git tag -m 'initial' initial

Push only the branch:

$ GIT_TRACE_PACKET=1 git push ../dst master 21 | grep 'push'
11:07:26 packet: push ... 66ba... refs/heads/master\0report-st...
11:07:26 packet: push 
$ cat pushlog
Wed Apr 15 11:07:26 PDT 2015
refs/heads/master 66ba... refs/heads/master ...

In the packet trace, we can see that we told the remote to update 'master',
and the pre-push logger also records the same.

Then push with --follow-tags:

$ GIT_TRACE_PACKET=1 git push --follow-tags ../dst master 21 | grep 
'push'
11:09:53 packet: push ... 30fa... refs/tags/initial\0report-st...
11:09:53 packet: push 
$ cat pushlog
Wed Apr 15 11:09:53 PDT 2015
refs/tags/initial 30fa... refs/tags/initial ...

We can see that we told the remote to store the tag, which matches
what the pre-push saw.

And then an empty push:

$ GIT_TRACE_PACKET=1 git push --follow-tags ../dst master 21 | grep 
'push'
11:11:23 packet: push 
$ cat pushlog
Wed Apr 15 11:11:23 PDT 2015

We tell them to do nothing, and pre-push saw nothing.

For a good measure, let's advance the branch and push it out:

$ git commit --allow-empty -m second
$ GIT_TRACE_PACKET=1 git push --follow-tags ../dst master 21 | grep 
'push'
11:13:43 packet: push 66ba... e711... refs/heads/master\0report-st...
11:13:43 packet: push 
$ cat pushlog
Wed Apr 15 11:13:43 PDT 2015
refs/heads/master e711... refs/heads/master 66ba...

We notice that the tag is up to date and do not tell them to do
anything to it, and pre-push did not see the tag either.

As far as I can see so far, the behaviour of the underlying push
(i.e. what we decide to tell the remote to update) is sensible,
and what pre-push is told we are doing by the command is consistent
with what is really pushed.

So

 Anyway it sounds like the answer here is that it really isn't a
 feasible task in a client side hook, and we should stick with the
 current solution of Don't use the GUI to push to Live for now, which
 is fine; I should probably stop trying to script around every single
 problem anyway (https://xkcd.com/1319/).

It appears that your GUI is a broken implementation of Git, that
tells the other side to update what it did not even send, and that
is what is causing the trouble, perhaps?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Feature Request: provide cmdline args to git hooks

[Junio C Hamano]

Chris O'Kelly ch...@mapcreative.com.au writes:

 A brief background of my use case:
 I am wanting to write a pre-push hook to prevent tags being pushed to
 our production servers. The production servers in our case are --bare
 endpoints, and when we push a tag at them, they always checkout the
 commit that tag is attached to via some post-receive magic (WPEngine,
 FWIW). This behavior is even present when the tag was already pushed
 to the repo previously, if for instance a normal push is made with the
 --tags argument.

Do you mean that you want to forbid some people from pushing tags
into that repository while allowing others (i.e. those who manage
production deployment)?  If that is the goal, it sounds like that
the right place to do this is at the receiving end via pre-receive,
not at the sending end via pre-push.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Feature Request: provide cmdline args to git hooks

[Junio C Hamano]

Chris O'Kelly ch...@mapcreative.com.au writes:

[administrivia: people read from top to bottom; please do not top
post]

 On Wed, Apr 15, 2015 at 1:08 AM, Junio C Hamano gits...@pobox.com wrote:
 Chris O'Kelly ch...@mapcreative.com.au writes:

 A brief background of my use case:
 I am wanting to write a pre-push hook to prevent tags being pushed to
 our production servers. The production servers in our case are --bare
 endpoints, and when we push a tag at them, they always checkout the
 commit that tag is attached to via some post-receive magic (WPEngine,
 FWIW). This behavior is even present when the tag was already pushed
 to the repo previously, if for instance a normal push is made with the
 --tags argument.

 Do you mean that you want to forbid some people from pushing tags
 into that repository while allowing others (i.e. those who manage
 production deployment)?  If that is the goal, it sounds like that
 the right place to do this is at the receiving end via pre-receive,
 not at the sending end via pre-push.


 Unfortunately in this case we don't have control over the hooks at the
 receiving end - we want to prevent tags from being pushed by all users
 to these repo's.

Well, if you refuse to or are unable to use the mechanism that was
specifically designed to solve your problem, then there is no way we
can offer you a good solution.

Let's set the baseline of the discussion first.

We agree that any client-side mechanism (like the hook) is *not* a
good way to enforce policy or ensure security. It is merely a way to
avoid mistakes---prevent the users to avoid doing something they are
allowed to do (at the mechanical level) that is not desirable to the
project.  After all, the user's hook can be misconfigured (or
disabled) by mistake, or the user's workstation may have an older
version of Git that does not know about the hook, thereby bypassing
whategver you try to do on the client side.  The user can even say
git push --no-verify.  That is why a true policy-enforcement and
security must be done on the receiving end.

So our conversation must start from the shared understanding that
you are seeking a client-side convention that helps users avoid
mistakes.  You do not have anything more than convention to force
certain behaviour on users.  Are we in agreement?

Now, if a convention, is an acceptaible solution, you do not even
necessarily need a hook. You can give git mypush to the users,
tell them to use it instead of git push and do whatever check in
git mypush and you are done.  If your users can be told not to run
git push with --no-verify, they can certainly taught not to use
git push but to use git mypush. Cf. 5 valid reasons to have hook
(http://thread.gmane.org/gmane.comp.version-control.git/71069).

The reason why we added pre-push is primarily because parsing the
command line is too brittle a way to guess what will be pushed.
When the user says git push and your hook sees ah, there is no
argument, that does not mean the user did not try to push any
tags.  The user may have remote.origin.push refspec to always push
things without typing from the command line, for example.  The
approach to inspect the command line, whether it is done in hook
or git mypush, is unworkable, and that is why pre-push is told
what git push decided to update on the remote end.

Having said all that, I am not sure if ec9f (push: Add support
for pre-push hooks, 2013-01-13) implemented the pre-push hook
correctly.

I do not offhand know (I am on a bus with terrible connection so I
won't bother checking the source now) if we send this ref has to
point at that object even for STATUS_UPTODATE cases, to cause your
remote to trigger the receive hook in the frist place, but if that
is the case, then the code in run_pre_push_hook() that omits such
refs from the hook input looks just *wrong*.  On the other hand, I
do not offhand think of a valid reason for the push codepath (with
or without the pre-push hook) to send this ref has to point at that
object when we know the ref is already pointing at that object, so
I am not sure if your claim (i.e. when ... was already pushed
previously) is true for a correctly built receiving side of Git.

On the other side, if we are telling the other end I know your
refs/tags/v1.0 is pointing at this object, but I am telling you to
point at it again anyway, then we should be feeding pre-push that,
too.  The code here in transport.c omits UPTODATE ones, and it may
need to be disabled to be consistent.

for (r = remote_refs; r; r = r-next) {
if (!r-peer_ref) continue;
if (r-status == REF_STATUS_REJECT_NONFASTFORWARD) continue;
if (r-status == REF_STATUS_REJECT_STALE) continue;
if (r-status == REF_STATUS_UPTODATE) continue;

strbuf_reset(buf);
strbuf_addf( buf, %s %s %s %s\n,
 r-peer_ref-name, sha1_to_hex(r-new_sha1),
 r-name, 

Re: Feature Request: provide cmdline args to git hooks

[Chris O'Kelly]

 I do not offhand know (I am on a bus with terrible connection so I
 won't bother checking the source now) if we send this ref has to
 point at that object even for STATUS_UPTODATE cases, to cause your
 remote to trigger the receive hook in the frist place, but if that
 is the case, then the code in run_pre_push_hook() that omits such
 refs from the hook input looks just *wrong*.  On the other hand, I
 do not offhand think of a valid reason for the push codepath (with
 or without the pre-push hook) to send this ref has to point at that
 object when we know the ref is already pointing at that object, so
 I am not sure if your claim (i.e. when ... was already pushed
 previously) is true for a correctly built receiving side of Git.

No I totally understand that the pre-receive is the ideal place to do
it, and I can see that it's not feasible to rework how pre-push was
designed if it was specifically made not to handle this kind of
situation. In a perfect world I would just remove/modify the
post-receive hook causing the undesirable behavior, but I work within
the restrictions of my environment. To reiterate and clarify I'm not
saying the undesirable behavior is a built in part of git, just a
feature of our hosting platform's Git deployment mechanisms, although
if what you mean is that the post-receive hook on the receiving end
shouldn't be getting passed pushed tag refs that the local git
believed to be already up to date on the remote (as of most recent
fetch), then yeah... that is weird because it seems to be the behavior
in this case.

Anyway it sounds like the answer here is that it really isn't a
feasible task in a client side hook, and we should stick with the
current solution of Don't use the GUI to push to Live for now, which
is fine; I should probably stop trying to script around every single
problem anyway (https://xkcd.com/1319/).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Feature Request: provide cmdline args to git hooks

[Chris O'Kelly]

Unfortunately in this case we don't have control over the hooks at the
receiving end - we want to prevent tags from being pushed by all users
to these repo's.

On Wed, Apr 15, 2015 at 1:08 AM, Junio C Hamano gits...@pobox.com wrote:
 Chris O'Kelly ch...@mapcreative.com.au writes:

 A brief background of my use case:
 I am wanting to write a pre-push hook to prevent tags being pushed to
 our production servers. The production servers in our case are --bare
 endpoints, and when we push a tag at them, they always checkout the
 commit that tag is attached to via some post-receive magic (WPEngine,
 FWIW). This behavior is even present when the tag was already pushed
 to the repo previously, if for instance a normal push is made with the
 --tags argument.

 Do you mean that you want to forbid some people from pushing tags
 into that repository while allowing others (i.e. those who manage
 production deployment)?  If that is the goal, it sounds like that
 the right place to do this is at the receiving end via pre-receive,
 not at the sending end via pre-push.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Feature Request: provide cmdline args to git hooks

[Chris O'Kelly]

Hello,
Just a brief note about a feature I would find incredibly useful, were
it available.

A brief background of my use case:
I am wanting to write a pre-push hook to prevent tags being pushed to
our production servers. The production servers in our case are --bare
endpoints, and when we push a tag at them, they always checkout the
commit that tag is attached to via some post-receive magic (WPEngine,
FWIW). This behavior is even present when the tag was already pushed
to the repo previously, if for instance a normal push is made with the
--tags argument. In the past we have had problems when a developer is
using a GUI like SourceTree and accidentally leaves the 'push all
tags' option checked, pushing 100s of tags to the server, which then
dutifully begins checking out each in turn.

Currently I check for tag refs being pushed with:
#...SNIP...
while read local_ref local_sha remote_ref remote_sha
do
local_type=$(echo $local_ref | awk -F/ '{print $2}')
remote_type=$(echo $remote_ref | awk -F/ '{print $2}')
if [[ $no_tags -eq 1  ($local_type = tags || $remote_type = tags)]]
then
echo Detected attempt to push tags to a no_tags repo! Exiting without push...
exit 1
fi
#...SNIP...

which works fine the first time tags are pushed, but when they are
already up to date as of the last fetch, they are not passed to the
stdin for pre-push, so I cannot detect them.

in a linux environment we can inspect /proc/$PPID/cmdline or ps
-ocommand= -p $PPID to find the --tags argument (or any manually
specified tag refs, etc), however commonly developers are using
Windows with SourceTree, and the pseudo-nix environment it provides
lacks a /proc directory and uses the cut down cygwin version of ps. I
have considered going down the parsing route with general ps output,
reading line by line to find the appropriate ppid, then echoing the
output, but varying output and column order of ps between SourceTree
and various linux versions looks to make that infeasible as a portable
solution.

If we could access the original git push commandline inside the hook,
either through a parameter passed directly to the script or possibly a
GIT_* environment variable, it would make this possible. My specific
use case may not be incredibly common, but this could also be used to
check if, for example, a push is being forced in a portable fashion -
something I can see being useful for a pre-push hook in a variety of
circumstances.

Alternatively - am I missing the super easy (and probably super
obvious) way to do this with the existing tools?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Adding git hooks

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Sat, Apr 26, 2014 at 10:24:50AM -0700, Junio C Hamano wrote:

 Suvorov Ivan sv...@inbox.ru writes:
 
  I want to extend the functionality of git due to the possibility of
  separation of the user repository into 2 parts - one part will be
  stored as usual, under version control git, and the second part will
  be stored in another location such as an FTP-server.
 
 Sounds like you are looking for git-annex, perhaps?

 I agree that is the right approach, but git-annex and git-media work
 _above_ the object layer, and taint the history by storing symlinks in
 git instead of the real sha1s. I'd love to see a solution that does the
 same thing, but lives at the pack/loose object layer. Basically:

   1. Teach sha1-file.c to look for missing objects by hitting an
  external script, like:

 git config odb.command curl https://example.com/%s;

  and place them in an alternates-like separate object database.

   2. Teach the git protocol a new extension to say don't bother sending
  blobs over size X. You'd have to coordinate that X with the source
  from your odb.command.

Yes, I'd love to see something along that line in the longer term,
showing all the objects as just regular objects under the hood, with
implementation details hidden in the object layer (just like there
is no distinction between packed and loose objects from the point of
view of read_sha1_file() users), as a real solution to address
issues in larger trees.

Also see http://thread.gmane.org/gmane.comp.version-control.git/241940
where Shawn had an interesting experiment.

 You'd probably want to wrap up the odb.command in a more fancy helper.
 For example, for performance, we'd probably want to be able to query it
 for which objects do you have, as well as fetch this object. And it
 would be nice if it could auto-query the X for step 2, and manage
 pruning local objects (e.g., when they become deep in history).

 We'd probably also want to teach a few places in git to treat external
 objects specially. For example, they should probably be auto-treated as
 binary, so that a log -p does not try to fetch all of them. And
 likewise, things like log -S should probably ignore them by default.

 I have a messy sketch of step 1 that I did quite a while ago, but
 haven't proceeded further on it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Adding git hooks

[Jeff King]

On Mon, Apr 28, 2014 at 09:43:10AM -0700, Junio C Hamano wrote:

 Yes, I'd love to see something along that line in the longer term,
 showing all the objects as just regular objects under the hood, with
 implementation details hidden in the object layer (just like there
 is no distinction between packed and loose objects from the point of
 view of read_sha1_file() users), as a real solution to address
 issues in larger trees.
 
 Also see http://thread.gmane.org/gmane.comp.version-control.git/241940
 where Shawn had an interesting experiment.

Yeah, I think it's pretty clear that a naive high-latency object store
is unusably slow. You mentioned in that thread trying to do pre-fetching
based on commits/trees, and I recall that Shawn's Cassandra experiments
did that (and maybe the BigTable-backed Google Code does, too?).

There's also a question of deltas. You don't want to get trees or text
blobs individually without deltas, because your total size ends up way
bigger.

But I think for large object support, we can side-step the issue. The
objects will all be blobs (so they cannot refer to anything else), they
will typically not delta well, and the connection setup and latency will
be dwarfed by actual transfer time. My plan was to have all clones fetch
all commits and trees (and small blobs, too), and then download and
cache the large blobs as-needed.

That doesn't help with repositories where the actual commit history or
tree size is a problem. But we already have shallow clones to help with
the former. And for the latter, I think we would want a narrow clone
that behaves differently than what I described above. You'd probably
want a specific widen operation that would fetch all of the objects
for the newly-widened part of the tree in one go (including deltas), and
you wouldn't want it to happen on an as-needed basis.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Adding git hooks

[Suvorov Ivan]

Hello.
I want to extend the functionality of git due to the possibility of separation 
of the user repository into 2 parts - one part will be stored as usual, under 
version control git, and the second part will be stored in another location 
such as an FTP-server.

This will be done in order to be able to separate the user repository binary 
data from the source code and binary data can stored separately.

For example, now on github prohibited to upload files larger than 100 MB, but 
some large files still would like to keep under version control.

And it will be possible to make due to the proposed division of the repository. 
It is assumed that this functionality will be developed for the most part 
separately from the program git, using a mechanism git hook. But in the current 
program git not enough hooks to implement this functionality. For example, it 
would be nice to have existed on the hook command git status, so that the 
output of this command can be supplemented by a list of binaries that are not 
under version control in main repository, but are under version control, for 
example in FTP server.

As a community Git consider the idea of a extend mechanism the functionality 
git hook?

Re: Adding git hooks

[Junio C Hamano]

Suvorov Ivan sv...@inbox.ru writes:

 I want to extend the functionality of git due to the possibility of
 separation of the user repository into 2 parts - one part will be
 stored as usual, under version control git, and the second part will
 be stored in another location such as an FTP-server.

Sounds like you are looking for git-annex, perhaps?

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Adding git hooks

[Jeff King]

On Sat, Apr 26, 2014 at 10:24:50AM -0700, Junio C Hamano wrote:

 Suvorov Ivan sv...@inbox.ru writes:
 
  I want to extend the functionality of git due to the possibility of
  separation of the user repository into 2 parts - one part will be
  stored as usual, under version control git, and the second part will
  be stored in another location such as an FTP-server.
 
 Sounds like you are looking for git-annex, perhaps?

I agree that is the right approach, but git-annex and git-media work
_above_ the object layer, and taint the history by storing symlinks in
git instead of the real sha1s. I'd love to see a solution that does the
same thing, but lives at the pack/loose object layer. Basically:

  1. Teach sha1-file.c to look for missing objects by hitting an
 external script, like:

git config odb.command curl https://example.com/%s;

 and place them in an alternates-like separate object database.

  2. Teach the git protocol a new extension to say don't bother sending
 blobs over size X. You'd have to coordinate that X with the source
 from your odb.command.

You'd probably want to wrap up the odb.command in a more fancy helper.
For example, for performance, we'd probably want to be able to query it
for which objects do you have, as well as fetch this object. And it
would be nice if it could auto-query the X for step 2, and manage
pruning local objects (e.g., when they become deep in history).

We'd probably also want to teach a few places in git to treat external
objects specially. For example, they should probably be auto-treated as
binary, so that a log -p does not try to fetch all of them. And
likewise, things like log -S should probably ignore them by default.

I have a messy sketch of step 1 that I did quite a while ago, but
haven't proceeded further on it.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git-hooks pre-push script does not receive input on stdin

[Torsten Bögershausen]

On 2014-03-23 01.44, David Cowden wrote:
 http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin
 
 Is this a bug in git?  Or am I just a shell noob?
 
 Thanks in advance,
 
 David

I assume that you have the right version of Git ?
You can look into the source of Git, to be more exact the test suite:
t/t5571-pre-push-hook.sh

When I play a little but with the script,
(Replace the diff with test_cmp, add a line:
diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
index 6f9916a..01db2fb 100755
--- a/t/t5571-pre-push-hook.sh
+++ b/t/t5571-pre-push-hook.sh
@@ -42,6 +42,7 @@ export COMMIT2
 write_script $HOOK 'EOF'
 echo $1 actual
 echo $2 actual
+echo hello actual
 cat actual
 EOF
-
and finally run it like this:

debug=t verbose=t ./t5571-pre-push-hook.sh  21 | less
I get something like this:

--- expected2014-03-23 07:15:58.0 +
+++ actual  2014-03-23 07:15:58.0 +
@@ -1,3 +1,4 @@
 parent1
 repo1
+hello
 refs/heads/master 139b20d8e6c5b496de61f033f642d0e3dbff528d refs/heads/foreign 
d79ce1670bdcb76e6d1da2ae095e890ccb326ae9
not ok 4 - push with hook
#   
#   git push parent1 master:foreign 
#   test_cmp expected actual
#   

So the question is, if your push simply doesn't have anything to push,
because everything is up to date?
And in this case there is nothing on STDIN?

If the problem still exists, feel free to post a script how to reproduce it
here to the list, t5571 may be a source of inspiration.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git-hooks pre-push script does not receive input on stdin

[David Cowden]

You are correct, it was a misunderstanding on my part.  I have
suggested a patch to clarify the pre-push documentation in a separate
thread.

Thanks for pointing that out!

On Sun, Mar 23, 2014 at 12:19 AM, Torsten Bögershausen tbo...@web.de wrote:
 On 2014-03-23 01.44, David Cowden wrote:
 http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin

 Is this a bug in git?  Or am I just a shell noob?

 Thanks in advance,

 David

 I assume that you have the right version of Git ?
 You can look into the source of Git, to be more exact the test suite:
 t/t5571-pre-push-hook.sh

 When I play a little but with the script,
 (Replace the diff with test_cmp, add a line:
 diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
 index 6f9916a..01db2fb 100755
 --- a/t/t5571-pre-push-hook.sh
 +++ b/t/t5571-pre-push-hook.sh
 @@ -42,6 +42,7 @@ export COMMIT2
  write_script $HOOK 'EOF'
  echo $1 actual
  echo $2 actual
 +echo hello actual
  cat actual
  EOF
 -
 and finally run it like this:

 debug=t verbose=t ./t5571-pre-push-hook.sh  21 | less
 I get something like this:

 --- expected2014-03-23 07:15:58.0 +
 +++ actual  2014-03-23 07:15:58.0 +
 @@ -1,3 +1,4 @@
  parent1
  repo1
 +hello
  refs/heads/master 139b20d8e6c5b496de61f033f642d0e3dbff528d 
 refs/heads/foreign d79ce1670bdcb76e6d1da2ae095e890ccb326ae9
 not ok 4 - push with hook
 #
 #   git push parent1 master:foreign 
 #   test_cmp expected actual
 #
 
 So the question is, if your push simply doesn't have anything to push,
 because everything is up to date?
 And in this case there is nothing on STDIN?

 If the problem still exists, feel free to post a script how to reproduce it
 here to the list, t5571 may be a source of inspiration.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Git-hooks pre-push script does not receive input on stdin

[David Cowden]

http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin

Is this a bug in git?  Or am I just a shell noob?

Thanks in advance,

David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: GIT Hooks and security

[Olivier Revollat]

Very helpful :) thanks !

2013/10/26 Bryan Turner btur...@atlassian.com:
 No, the .git/hooks directory in your clone is created from your local
 templates, installed with your Git distribution, not the remote hooks.
 On Linux distributions, these templates are often in someplace like
 /usr/share/git-core/templates (for normal packages), and on Windows
 with msysgit they are in share\git-core\templates under your
 installation directory. If you look in this directory you will see a
 hooks directory containing the sample hooks.

 Hooks from a remote repository are never cloned. As far as I'm aware,
 nothing from the .git directory (aside from refs and packs, of course)
 is cloned, including configuration. Your .git directory after a clone
 is completely new, assembled from scratch. There's nothing in the Git
 wire protocol (currently) for moving other data like configuration or
 hooks, and this sort of malicious code injection is one of the reasons
 I've seen discussed on the list for why that's the case.

 Hope this helps,
 Bryan Turner


 On 26 October 2013 09:25, Olivier Revollat revol...@gmail.com wrote:

 But when someone do a clone he don't have .git/hooks directory
 downloaded to his local computer ? I thought so ...

 2013/10/26 Junio C Hamano gits...@pobox.com:
  Olivier Revollat revol...@gmail.com writes:
 
  I was wondering : What if I had a malicious GIT repository who can
  inject code  via git hooks mechanism : someone clone my repo and
  some malicious code is executed when a certain GIT hook is triggered
  (for example on commit (prepare-commit-msg' hook))
 
  In that somebody else's clone, you will not have _your_ malicious
  hook installed, unless that cloner explicitly does something stupid,
  like copying that malicious hook.



 --
 Mathematics is made of 50 percent formulas, 50 percent proofs, and 50
 percent imagination.
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Mathematics is made of 50 percent formulas, 50 percent proofs, and 50
percent imagination.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: GIT Hooks and security

[Ondřej Bílka]

 2013/10/26 Bryan Turner btur...@atlassian.com:
  No, the .git/hooks directory in your clone is created from your local
  templates, installed with your Git distribution, not the remote hooks.
  On Linux distributions, these templates are often in someplace like
  /usr/share/git-core/templates (for normal packages), and on Windows
  with msysgit they are in share\git-core\templates under your
  installation directory. If you look in this directory you will see a
  hooks directory containing the sample hooks.
 
  Hooks from a remote repository are never cloned. As far as I'm aware,
  nothing from the .git directory (aside from refs and packs, of course)
  is cloned, including configuration. Your .git directory after a clone
  is completely new, assembled from scratch. There's nothing in the Git
  wire protocol (currently) for moving other data like configuration or
  hooks, and this sort of malicious code injection is one of the reasons
  I've seen discussed on the list for why that's the case.
 
  Hope this helps,
  Bryan Turner
 
 
  On 26 October 2013 09:25, Olivier Revollat revol...@gmail.com wrote:
 
  But when someone do a clone he don't have .git/hooks directory
  downloaded to his local computer ? I thought so ...
 
  2013/10/26 Junio C Hamano gits...@pobox.com:
   Olivier Revollat revol...@gmail.com writes:
  
   I was wondering : What if I had a malicious GIT repository who can
   inject code  via git hooks mechanism : someone clone my repo and
   some malicious code is executed when a certain GIT hook is triggered
   (for example on commit (prepare-commit-msg' hook))
  
   In that somebody else's clone, you will not have _your_ malicious
   hook installed, unless that cloner explicitly does something stupid,
   like copying that malicious hook.
 
Also copying hooks is relatively low risk, real hackers hide exploits in
1MB configure scripts.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

GIT Hooks and security

[Olivier Revollat]

I was wondering : What if I had a malicious GIT repository who can
inject code  via git hooks mechanism : someone clone my repo and
some malicious code is executed when a certain GIT hook is triggered
(for example on commit (prepare-commit-msg' hook)) ? What if I email
/etc/passwd for exemple ?

Does GIT's hooks security is assured by the GIT user privileges ? but
git user can still read /etc/passwd and make something fun with it :)

Is it by the trust relationship ? I mean, If I clone a repo, I
certainly knew the source and I trusted it ... isn't it ?
But if I have a website with file injection vulnerability and I can
replace the git hook script with another (malicious) content ...

I'm maybe paranoid :) but I'm just asking the question ... just for
my curiosity's sake :)

Thanks for your comments and explanations :)

-- 

Mathematics is made of 50 percent formulas, 50 percent proofs, and 50
percent imagination.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: GIT Hooks and security

[Junio C Hamano]

Olivier Revollat revol...@gmail.com writes:

 I was wondering : What if I had a malicious GIT repository who can
 inject code  via git hooks mechanism : someone clone my repo and
 some malicious code is executed when a certain GIT hook is triggered
 (for example on commit (prepare-commit-msg' hook))

In that somebody else's clone, you will not have _your_ malicious
hook installed, unless that cloner explicitly does something stupid,
like copying that malicious hook.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: GIT Hooks and security

[Bryan Turner]

No, the .git/hooks directory in your clone is created from your local
templates, installed with your Git distribution, not the remote hooks.
On Linux distributions, these templates are often in someplace like
/usr/share/git-core/templates (for normal packages), and on Windows
with msysgit they are in share\git-core\templates under your
installation directory. If you look in this directory you will see a
hooks directory containing the sample hooks.

Hooks from a remote repository are never cloned. As far as I'm aware,
nothing from the .git directory (aside from refs and packs, of course)
is cloned, including configuration. Your .git directory after a clone
is completely new, assembled from scratch. There's nothing in the Git
wire protocol (currently) for moving other data like configuration or
hooks, and this sort of malicious code injection is one of the reasons
I've seen discussed on the list for why that's the case.

Hope this helps,
Bryan Turner


On 26 October 2013 09:25, Olivier Revollat revol...@gmail.com wrote:

 But when someone do a clone he don't have .git/hooks directory
 downloaded to his local computer ? I thought so ...

 2013/10/26 Junio C Hamano gits...@pobox.com:
  Olivier Revollat revol...@gmail.com writes:
 
  I was wondering : What if I had a malicious GIT repository who can
  inject code  via git hooks mechanism : someone clone my repo and
  some malicious code is executed when a certain GIT hook is triggered
  (for example on commit (prepare-commit-msg' hook))
 
  In that somebody else's clone, you will not have _your_ malicious
  hook installed, unless that cloner explicitly does something stupid,
  like copying that malicious hook.



 --
 Mathematics is made of 50 percent formulas, 50 percent proofs, and 50
 percent imagination.
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html