Re: [Patch v2] daemon: Set ownership of kept build directories to the calling user.

[Ludovic Courtès]

Hartmut Goebel <address@hidden> skribis:

> Fixes <http://bugs.gnu.org/15890>.
>
> * nix/libstore/globals.hh (Settings) Add clientUid and clientGid.
> * nix/nix-daemon/nix-daemon.cc (pdaemonLoop] Store UID and GID of the
>   caller in settings.
> * nix/libstore/build.cc (_chown): New function.
>   (DerivationGoal::deleteTmpDir): Use it, change ownership of build
>   directory if it is kept.

[...]

> +static void _chown(const Path & path, uid_t uid, gid_t gid)
> +{
> +    checkInterrupt();
> +
> +    printMsg(lvlVomit, format("%1%") % path);
> +
> +    if (chown(path.c_str(), uid, gid) == -1) {

I think this should use ‘lchown’.

> --- a/nix/libstore/globals.hh
> +++ b/nix/libstore/globals.hh
> @@ -70,6 +70,12 @@ struct Settings {
>         subgoal of the same goal) fails. */
>      bool keepGoing;
>  
> +    /* User and groud id of the client issuing the buld request.  Used to set
> +       the owner and group of the keept temporary directories of failed
> +       builds. */
> +    uid_t clientUid;
> +    gid_t clientGid;

I don’t like the idea of passing those via the big ‘Settings’
singleton.

Could we instead pass them via the ‘LocalStore’ constructor, with their
default values taken from ‘getuid’ and ‘getgid’ (rather than 0)?  WDYT?

Thank you!

Ludo’.