Test failures due to core.sharedRepository and

Test failures due to core.sharedRepository and

[Julien Moutinho]

sandboxing
Reply-To:

Hi!

While updating from 1.8.0 to 1.9.0, I've stumbled upon
that failure in t/lei-q-kw.t (but other tests fail for the same reason):
> ok 52 - lei import -F eml t/x-unknown-alpine.eml
> not ok 53 - no errors importing previous external-only message
> #   Failed test 'no errors importing previous external-only message'
> #   at t/lei-q-kw.t line 181.
> #          got: 'error: unable to create temporary file: Operation not permitted
> # fatal: failed to write object
> # '
> #     expected: ''

strace -f prove -bvw t/lei-q-kw.t
revealed this EPERM:
> chmod("/build/tmp/pi-lei-q-kw-1976-HlW5/lei-daemon/.local/share/lei/store/local/0.git/objects/pack", 02700) = -1 EPERM (Operation not permitted)

Turns out this is another consequence of running inside nix's sandbox:
> ; Disallow creating setuid/setgid binaries, since that
> ; would allow breaking build user isolation.
> (deny file-write-setugid)
https://github.com/NixOS/nix/blob/b3d2a05c59266688aa904d5fb326394cbb7e9e90/src/libstore/sandbox-defaults.sb#L5-L7
https://github.com/NixOS/nix/blob/b3d2a05c59266688aa904d5fb326394cbb7e9e90/src/libstore/build/local-derivation-goal.cc#L1555-L1568

That SGID bit in 2700 is due to git's FORCE_DIR_SET_GID:
> if (S_ISDIR(old_mode)) {
>   /* Copy read bits to execute bits */
>   new_mode |= (new_mode & 0444) >> 2;
>   new_mode |= FORCE_DIR_SET_GID;
> }
https://github.com/git/git/blob/1fc3c0ad407008c2f71dd9ae1241d8b75f8ef886/path.c#L901-L905

which is enabled when public-inbox sets core.sharedRepository:
> $self->git->qx(qw(config core.sharedRepository 0600));
https://public-inbox.org/public-inbox.git/tree/lib/PublicInbox/ExtSearchIdx.pm?id=0881010d123914be5e47544229e2b03412a6a691#n1231

Eric, do you think something can be done to accomodate nix's sandbox?
otherwise I can disable those failing tests.

Cheers,

Re: Test failures due to core.sharedRepository and

[Eric Wong]

Julien Moutinho <julm+public-inbox@sourcephile.fr> wrote:
> sandboxing
> Reply-To:
> 
> Hi!
> 
> While updating from 1.8.0 to 1.9.0, I've stumbled upon
> that failure in t/lei-q-kw.t (but other tests fail for the same reason):
> > ok 52 - lei import -F eml t/x-unknown-alpine.eml
> > not ok 53 - no errors importing previous external-only message
> > #   Failed test 'no errors importing previous external-only message'
> > #   at t/lei-q-kw.t line 181.
> > #          got: 'error: unable to create temporary file: Operation not permitted
> > # fatal: failed to write object
> > # '
> > #     expected: ''
> 
> strace -f prove -bvw t/lei-q-kw.t
> revealed this EPERM:
> > chmod("/build/tmp/pi-lei-q-kw-1976-HlW5/lei-daemon/.local/share/lei/store/local/0.git/objects/pack", 02700) = -1 EPERM (Operation not permitted)
> 
> Turns out this is another consequence of running inside nix's sandbox:
> > ; Disallow creating setuid/setgid binaries, since that
> > ; would allow breaking build user isolation.
> > (deny file-write-setugid)
> https://github.com/NixOS/nix/blob/b3d2a05c59266688aa904d5fb326394cbb7e9e90/src/libstore/sandbox-defaults.sb#L5-L7
> https://github.com/NixOS/nix/blob/b3d2a05c59266688aa904d5fb326394cbb7e9e90/src/libstore/build/local-derivation-goal.cc#L1555-L1568
> 
> That SGID bit in 2700 is due to git's FORCE_DIR_SET_GID:
> > if (S_ISDIR(old_mode)) {
> >   /* Copy read bits to execute bits */
> >   new_mode |= (new_mode & 0444) >> 2;
> >   new_mode |= FORCE_DIR_SET_GID;
> > }
> https://github.com/git/git/blob/1fc3c0ad407008c2f71dd9ae1241d8b75f8ef886/path.c#L901-L905

OK, That descends from a very old git commit:
https://public-inbox.org/git/457f06d68e427bbf4f1a921877441a622a05e5c4/s/
https://public-inbox.org/git/Pine.LNX.4.63.0512222313070.12044@wbgn013.biozentrum.uni-wuerzburg.de/

My brain is very tired atm, but I'm wondering if git should
strip S_ISGID and retry if it hits EPERM...

> which is enabled when public-inbox sets core.sharedRepository:
> > $self->git->qx(qw(config core.sharedRepository 0600));
> https://public-inbox.org/public-inbox.git/tree/lib/PublicInbox/ExtSearchIdx.pm?id=0881010d123914be5e47544229e2b03412a6a691#n1231
> 
> Eric, do you think something can be done to accomodate nix's sandbox?
> otherwise I can disable those failing tests.

On a traditional Unix-like system, the objective of 0600 is to
ensure only the user running lei can read their own email.
AFAIK, that's standard behavior for MUAs creating local files
(I only know it's mutt behavior off the top-of-my-head).

That said, I don't know how NixOS's sandbox is different than a
traditional system.

Is setting any value of core.sharedRepository even worthwhile on NixOS?

Thanks.

Re: Test failures due to core.sharedRepository and sandboxing

[Julien Moutinho]

Le mar. 25 oct. 2022 10h17 +0000, Eric Wong a écrit :
> My brain is very tired atm, but I'm wondering if git should
> strip S_ISGID and retry if it hits EPERM...
I've asked on git@ here
https://public-inbox.org/git/20221025163024.uutqv7w24yi4eo5i@sourcephile.fr/T/#u

> On a traditional Unix-like system, the objective of 0600 is to
> ensure only the user running lei can read their own email.
> AFAIK, that's standard behavior for MUAs creating local files
> (I only know it's mutt behavior off the top-of-my-head).
> 
> That said, I don't know how NixOS's sandbox is different than a
> traditional system.
nix's build sandbox denies g+s, but that sandbox only applies
when building a package (which includes running its tests).
Once the package is built, that sandbox is no longer used,
leaving the package free to use g+s when run by users.

> Is setting any value of core.sharedRepository even worthwhile on NixOS?
AFAIK NixOS would be like any other Linux-based OS on that matter.

Cheers,