Although this is not a "hole" that allows a normal user to bypass
controls, I still consider this a hole in the sense that I want to
separate "admin push" rights from "shell access on server" rights.
(I realise that most people don't make this distinction, but I do, and
for me and most sites I consult for it is important).
Thanks to drue on #gitolite who pointed it out excitedly, and apologies
for killing what he thought of as a feature!
(manually smoke tested; no test script)
I've been meaning to do this for a while, since someone told me that is
one of gerrit's features they like.
Of course, gitolite can't/won't do the whole "code review" thing (nor
the workflow enforcement that follows).
But voting is simple -- literally 2-3 lines of code in a VREF. (The
rest is inline documentation).
...so there's no need for the VREF.
Ironically, while I was arguing with Eli that I wouldn't do it and why,
the code was *already* there, and had been for over a month! (It must
have been there for much longer for me to have forgotten!)
TODO: convert from using fingerprint compute to actual key strings when
the complaints about speed start appearing.
My own personal speed up loop [1] I guess :)
[1]: http://thedailywtf.com/Articles/Classic-WTF-The-Speedup-Loop.aspx
(but change repo check to allow repoPATT instead of just repoNAME)
This is because there are/will be some situations where access() is
called without those two checks being done (i.e., it is not only from
src/commands/access that it is called).
(1) the backward compat breakage: you can't create empty-valued config
keys anymore. That is, you can't do the eqvt of the following shell
command using gitolite
git config foo.bar ""
(2) fixed a bug where this:
repo foo
config foo.bar =
when queried using
gitolite git-config -r foo .
would return even the empty valued ones, which -- remember! -- are
not supposed to exist anymore.
Fixing this bug allows situations like this to not show the admin
repo in gitweb:
repo [a-z].*
config gitweb.owner = P-h B
repo gitolite-admin
config gitweb.owner =
----
background...
Somewhere in g3 (well actually in 057506b), we lost the ability to
distinguish
config foo.bar = ""
from
config foo.bar =
I decided that conflating them is more intuitive for most people,
because a survey [1] revealed that no one seemed to want the equivalent
of the following shell command:
----
[1] ...of a (small prime greater than 1) number of people on #git
when running under httpd, $ENV{USER} is not set, so we use a (hopefully
informative) default to print.
Thanks to Thomas Hager (duke at sigsegv dot at) for catching this.
Using a g2-style "chained update hook" as a VREF doesn't *quite* work:
- all STDOUT from the hook is lost
- worse, all lines get parsed as a ref followed by a message, and if
the ref doesn't look like a ref it dies
So now we do all this only if the message starts with 'VREF/'. Any
other output is just printed out as is.
thanks to milki for all the efforts!
Details:
- partial-copy fell afoul of BSD not having $RANDOM
- test suite: fix bad GNU sort with good perl sort
- test suite: fix md5sum dependency (which BSD doesn't have or can't
easily have or requires extra options or whatever...), by doing it
in perl. (Requires Digest::MD5, which is probably available
anyway, but since this is only for the test suite, meh!)
I had not remembered that the 'tc' subcommand in tsh adds *text* that
contains the current time, so commit SHAs were changing.
Thanks to milki for catching this, and in fact being the only person who
ever appears to have attempted to run the test suite at all!
The POST_CREATE trigger is called when
* a user creates a new "wild" repo,
* a user uses the "perms" command, and
* a user uses the "fork" command.
The trigger calls 3 programs (see rc file):
post-compile/update-git-configs
post-compile/update-gitweb-access-list
post-compile/update-git-daemon-access-list
(They are also called by the POST_COMPILE trigger, by the way.)
However, the 3 programs shown are a bit wasteful -- they run through
*all* the repos when really only *one* repo has been affected.
This patch
* passes the repo name to the 3 programs (duh!)
* adds the optimisation to the first of the 3 programs listed above
(the one dealing with 'git config').
For the other two programs (gitweb and git-daemon), you have 3 choices:
* if you don't have too many repos, ignore the problem.
* take out the 2nd and 3rd lines from the POST_CREATE list in the rc
file, so they don't run.
Then run 'gitolite trigger POST_COMPILE' from cron at regular
intervals. (Note that is POST_COMPILE not POST_CREATE!) However,
this means that gitweb and daemon permissions won't be current
immediately after someone adds a new repo or sets perms etc.; they
get updated only on the next cron run.
* patch the programs to add this optimisation (and send me the
patches). The optimisation would check if arg-1 ($1 in shell,
$ARGV[0] in perl) is 'POST_CREATE', and if it is, take the *next*
argument as a repo name that may have changed.
gitolite setup fails to check admin pubkey, because $text always
contains 2 or more lines after tsh_try() (the key and -n).
[committer adds:
I wasn't sure if 'printf' would work on cygwin, so I chose what
looked like a safer option, but apparently it wasn't safe enough and
fell afoul of Solaris.
Anyway I managed to check (using a small test program) with someone
who runs gitolite on cygwin, and it works.
If you're wondering why I didn't just use echo followed by chomp(),
that would of course have been the easy way out but I wanted to see
how you'd do it without a post-processing option. It became a
frustrating challenge of sorts because it seems such a trivial thing!
]
The fix is easy enough, but I hate having to code work-arounds for
proprietary OSs when the same code works fine on Linux and BSD.
/me wisely avoids words like posix in his rant ;-)
Thanks to Franck Zoccolo for help in finding what the problem was and
when and why it occurred.
----
Someday there will be some issue that requires a fix with significant
code change (or worse, a change that is incompatible with Linux), and I
will probably refuse. Of course, I will be properly regretful about my
inability to fix it.[1]
I must have blindly converted from some shell-thinking/shell-code for
these to have slipped through!
(found when doing an audit of all system, exec, ``, qx, and tsh_)
(manually tested, no test script)
the whimsically named "D" command deletes repos, and is the opposite of
the "C" permission that enables the user to create one in the first
place. See the usage message for user info, and look in the comments of
the code itself for admin info.
- minor typo fixes, clarifications, etc.
- keep sts.html url consistent, because many people link to
http://sitaramc.github.com/gitolite/sts.html
- create a common migration doc, so the old 'migr.html' does not 404
when g3 docs become "main"
- progit doc done
- add gitosis convert script (FWIW)
- a minor comment fix to Sugar.pm
