stacked-git / stgit

Stacked Git
https://stacked-git.github.io/
GNU General Public License v2.0
518 stars 60 forks source link

2.4.2: `sudo -u nobody stg` operations override system umask #413

Closed lkraav closed 7 months ago

lkraav commented 7 months ago

After something like sudo -u nobody stg pop, some or maybe all written git metadata files get written with 600 permissions. This started I think after replatforming on Rust, everything worked fine for years on Python builds. Now others in nobody group are not able to execute any git commands in this repo, because fatal: your current branch appears to be broken.

$ ls -l .git/refs/heads/
total 4
-rw------- 1 nobody nobody 40  2. veebr 13:39 crdy

sudo doesn't seem to configured in any wrong way, touch writes files with 644 permissions as it should:

$ sudo -u nobody touch moo
$ ls -l moo 
-rw-r--r-- 1 nobody nobody 0  2. veebr 13:45 moo

Feels like some kind of Rust lib thing. Your thoughts?

jpgrayson commented 7 months ago

Thanks for posting this issue, @lkraav.

StGit uses gix::Repository::edit_references() to update references such as refs/heads/main. So permissions of refs is going to depend on gix behavior. @Byron might have some immediate thoughts on this.

Next step for StGit will be to add a test or two that reproduce this behavior.

lkraav commented 7 months ago

Next step for StGit will be to add a test or two that reproduce this behavior.

Thanks. Are you able to reproduce this behavior manually?

Byron commented 7 months ago

Thanks for reeling me in!

I researched this real quick and tracked it down to this call in Git which allows to create tempfiles with a given mode. This in turn calls a function that creates said tempfile and possibly adjusts permissions in a separate step.

gix-lock, the crate implement Git-style lock files, definitely doesn't support that yet and ends up creating owner-writable files by merit of lock files being temporary files in disguise.

I do remember that I never managed to create lock-files the way I wanted to, while leveraging the tempfile crate, and just left it at that. However, the current implementation definitely is a problem and it at the very least it should chmod the created files afterwards until a more efficient solution is found (i.e. one that creates the tempfile with the correct mode in the first place). I will try to take a look tomorrow.

Byron commented 7 months ago

This issue is on track to be resolved. It might take a little longer to complete though as it's depending on another PR in the upstream tempfile project.

Byron commented 7 months ago

I have released new versions of gix-tempfile and gix-lock, and once up-to-date in StackedGit, this issue should be no more as all files are created with 0o666 (probably 0o644 after umask).

You are probably able to try it out after running cargo update && cargo build --release in this repository.

lkraav commented 7 months ago

Top notch service, sirs, much appreciated!

Byron commented 7 months ago

You are welcome! Does that mean it works :)?