~/.bazaar created owned by root (when run under sudo)

This is caused by bzr not really supporting to be run under sudo.
In config.py:ensure_config_dir_exists it creates $HOME/.bazaar without dropping rights to the $SUDO_USER. Unfortunately the same could be said for all other config-writing functions. I'm not exactly sure where (and how) it would be best to fix that.

Adding a bzr task to this bug to drag bzr developers into the discussion. The fix for this one could also be discussed at the bzr/etckeeper session at UDS (server track).

On Fri, 2009-05-15 at 09:20 +0000, Thierry Carrez wrote:
> This is caused by bzr not really supporting to be run under sudo.
> In config.py:ensure_config_dir_exists it creates $HOME/.bazaar without dropping rights to the $SUDO_USER. Unfortunately the same could be said for all other config-writing functions. I'm not exactly sure where (and how) it would be best to fix that.
>
> Adding a bzr task to this bug to drag bzr developers into the
> discussion. The fix for this one could also be discussed at the
> bzr/etckeeper session at UDS (server track).

Patches appreciated :) certainly sounds like a valid use case. Its a
shame that we'd need to special case stuff.

-Rob

I agree there's some kind of bug here, but this behavior is very
common in other programs too, and it does seem tragic they'll all need
to specially handle running under sudo. Maybe sudo -H should be the
default.

--
Martin <http://launchpad.net/~mbp/>

Would using pwd.getpwuid(os.geteuid()).pw_dir rather than os.environ['HOME'] cause more problems than it solves?

On 17 February 2010 00:48, Soren Hansen <email address hidden> wrote:
> Would using pwd.getpwuid(os.geteuid()).pw_dir rather than
> os.environ['HOME'] cause more problems than it solves?

I think so.

The right place to solve this is imo under sudo, by using the set_home
config option or -H option. This is not particularly specific to bzr.

--
Martin <http://launchpad.net/~mbp/>

We won't make -H the default in sudo; it would break other use cases which rely on having the original $HOME, and general backwards compatibliity.

That leaves two options:
1/ special-casing sudo in configuration file management in bzr
2/ workaround in etckeeper (override HOME in bzr calls in etckeeper ? Pre-create $HOME/.bazaar and $HOME/.bzr.log correctly ?)

(2) is certainly simpler, however it will fail to catch the case where the etckeeper user runs "sudo bzr diff" by hand. that said, if soren implements the "etckeeper diff" command, that might be considered a corner case.

Please note that this is not a problem with Git / when using Git with etckeeper:
Apparently Git does not create the .git dir automatically - which may also be an option for Bazaar.

Apart from that, Git behaves the same:
$ sudo git config --global user.name "Daniel Hahler"
$ ls -l ~/.gitconfig
-rw-r--r-- 1 root root 29 2010-02-17 12:29 /home/daniel/.gitconfig

Should there be a Git task then, too?

Thierry's comment in #1 is interesting:

>> This is caused by bzr not really supporting to be run under sudo. In config.py:ensure_config_dir_exists it creates $HOME/.bazaar without dropping rights to the $SUDO_USER. Unfortunately the same could be said for all other config-writing functions. I'm not exactly sure where (and how) it would be best to fix that.

This would be possible. However, it also seems likely to break in the case that you do "sudo -H bzr" and actually _want_ bzr to use root's configuration.

It may be that we create ~/.bazaar even when we don't need to, and trimming that would make it similar to #12. However the problem may still occur with ~/.bzr.log.

Another option would be to say we always attempt to chown the .bazaar (and .bzr.log) to be the same as the containing directory. That is perhaps not quite unixy, but it seems correct for the main cases: with HOME=/root it's owned by root, with HOME=/home/mbp by me. It's a bit like a setgid directory.

I plan to look into this issue.

I was thinking of going with the approach Martin suggested:

> Another option would be to say we always attempt to chown
> the .bazaar (and .bzr.log) to be the same as the containing
> directory.

Any other thoughts/suggestions welcome.

I think Parth's patch in https://code.edge.launchpad.net/~parthm/bzr/2.0_376388_dot_bazaar_ownership/+merge/19593 is reasonable and useful for bzr in its own right (review welcome)

but probably also etckeeper should run bzr and other programs with HOME set to ~root to avoid the problem altogether.

How about just adding HOME=/root (or something slightly more dynamic) to our etckeeper.conf ? it's sourced everywhere and should ensure bzr is always called with the right env ?

That doesn't solve the case where people run "sudo bzr diff" or others, but that can probably wait for the fix to land in bzr..

(I meant HOME=~root)

On 19 February 2010 19:06, Thierry Carrez <email address hidden> wrote:
> How about just adding HOME=/root (or something slightly more dynamic) to
> our etckeeper.conf ? it's sourced everywhere and should ensure bzr is
> always called with the right env ?

That makes sense to me.

--
Martin <http://launchpad.net/~mbp/>

This bug was fixed in the package etckeeper - 0.41ubuntu3

---------------
etckeeper (0.41ubuntu3) lucid; urgency=low

  * etckeeper: Set HOME=~root if VCS=bzr, so that bzr doesn't create root-owned
    .bazaar and .bzr.log in home directories when run under sudo (LP: #376388)
 -- Thierry Carrez <email address hidden> Tue, 23 Feb 2010 12:05:55 +0100

> Another option would be to say we always attempt to chown the .bazaar (and .bzr.log) to be the same
> as the containing directory.

Until parthm nudged me enough to reveal that chown isn't working for the average user, I had always thought
*any* user can freely use chown as long as he had write access to the containing directory.

I was wrong.

chown(2) unambiguously says:

        Only a privileged process (Linux: one with the CAP_CHOWN capability)
       may change the owner of a file. The owner of a file may change the
       group of the file to any group of which that owner is a member. A
       privileged process (Linux: with CAP_CHOWN) may change the group arbi‐
       trarily.

On 11 March 2010 00:18, Vincent Ladeuil <email address hidden> wrote:
>> Another option would be to say we always attempt to chown the .bazaar (and .bzr.log) to be the same
>> as the containing directory.
>
> Until parthm nudged me enough to reveal that chown isn't working for the average user, I had always thought
> *any* user can freely use chown as long as he had write access to the containing directory.

Sure, I meant to say "always try to", but we must not be surprised if
we get EPERM.

>
> I was wrong.
>
> chown(2) unambiguously says:
>
>        Only a privileged process (Linux: one with  the  CAP_CHOWN  capability)
>       may  change  the  owner  of a file.  The owner of a file may change the
>       group of the file to any group of which that  owner  is  a  member.   A
>       privileged  process  (Linux: with CAP_CHOWN) may change the group arbi‐
>       trarily.

btw I think this may vary between unixes.

--
Martin <http://launchpad.net/~mbp/>

<snip/>

    > btw I think this may vary between unixes.

I did test with FreeBSD and OSX and got the same results, I
mentioned the explanation for Linux because that was the only one
I cared to find but I've seen some mentions about a sysctl
variable for them that control this precise aspect and the
default value also forbids chown for non-root users.

An alternative solution may be to recognize we are working as
root and just use setuid for the relevant operations or something ?

This bug was fixed in the package bzr - 2.2.0-1

---------------
bzr (2.2.0-1) unstable; urgency=low

  * New upstream release.
   + Adds support for setting timestamps to originating revisions.
     Closes: #473450
   + Removes remaining string exception. Closes: #585193, LP: #586926
   + Add C extension to work around Python issue 1628205. LP: #583941,
     Closes: #577110
   + Avoids showing progress bars when --quiet is used. Closes: #542105,
     LP: #320035
   + No longer creates ~/.bazaar as root when run under sudo. LP: #376388
   + 'bzr commit' now supports -p as alternative for --show-diff. LP: #571467
   + 'bzr add' no longer adds .THIS/.BASE/.THEIRS files unless
     explicitly requested. LP: #322767
   + When parsing patch files, Bazaar now supports diff lines before each
     patch. LP: #502076
   + WorkingTrees now no longer requires using signal.signal, so can
     be used in a threaded environment. LP: #521989
   + An assertion error is no longer triggered when pushing to a pre-1.6
     Bazaar server. LP: #528041
  * Bump standards version to 3.9.1.

bzr (2.2.0~b4-1) experimental; urgency=low

  * New upstream beta.
  * Bump standards version to 3.9.0.
  * Drop build dependency on zlib.

bzr (2.2.0~b2-1) experimental; urgency=low

  * New upstream release.
  * Recommend python-launchpadlib. Closes: #568937

bzr (2.1.2-1.1) unstable; urgency=low

  * Non-maintainer upload.
  * Fix access via http_proxy. Closes: #588430
 -- Jelmer Vernooij <email address hidden> Sat, 07 Aug 2010 00:54:52 +0200