[MERGE] OSX's mkdtemp assigns wrong group by default

Vincent Ladeuil v.ladeuil+lp at free.fr
Sun Aug 31 20:03:05 BST 2008

>>>>> "Guillermo" == Guillermo Gonzalez <guillo.gonzo at gmail.com> writes:

    Guillermo> Hi,
    Guillermo> On Sun, Aug 31, 2008 at 3:22 PM, John Arbash Meinel
    Guillermo> <john at arbash-meinel.com> wrote:
    >> ...
    john> dir = mkdtemp()
    john> if sys.platform == 'darwin':
    john> # OS X creates temp dirs with the 'wheel' group, which users are not likely
    john> # to be in, and this prevents us from setting the sgid bit
    john> os.chown(dir, os.getuid(), os.getgid())
    >>> Conditional logic in tests ? >-/
    john> I would certainly be more comfortable modifying the one
    john> test that needs group, rather than everytime someone
    john> creates a temp dir.
    >>> But then we risk writing tests suffering from the same problem.
    >> And then we fix specific tests that need it,

Ok. Only two tests are concerned, I had already made the
modification when I realized I was introducing conditional logic,
so I'll commit that and will split the tests (which are really
huge) so that the the chown become closer to the group bit use.

    >> but I honestly think that actively chowning mkdtemp would
    >> lead to security holes which is worse than having a test
    >> have a bit of platform specific logic.  If we have other
    >> tests crop up, then we can revisit it. But a single test
    >> needing this does not necessitate doing it everywhere.

    Guillermo> I wasn't aware of possible security holes related
    Guillermo> to this, at the time I added it.

    Guillermo> I'm thinking that a possible solution, to avoid
    Guillermo> conditionals in the tests, could be adding an
    Guillermo> argument to osutils.mkdtemp, i.e:
    Guillermo> osutils.mkdtemp(override_ug_id=False)

    Guillermo> This would allow a specific test to override
    Guillermo> uid/guid, and also keeping the current behaviour
    Guillermo> in all other test.

    Guillermo> any thoughts?

OSX devs got a strange idea there let's not make life harder for
everyone else. Anyway on our case the mkdtemp call is far away
from the tests and there is no way to make that additional
parameter travel that far.


More information about the bazaar mailing list