Skip to content

preserve inherited umask on autodaemonize#384

Open
tomfitzhenry wants to merge 1 commit into
shell-pool:masterfrom
tomfitzhenry:umask
Open

preserve inherited umask on autodaemonize#384
tomfitzhenry wants to merge 1 commit into
shell-pool:masterfrom
tomfitzhenry:umask

Conversation

@tomfitzhenry

Copy link
Copy Markdown

AI Policy Ack

Please ack that you have read the AI Policy
and explain your use of AI to generate this PR.

Ack. An LLM diagnosed this issue when I prompted it to find why the umask was 027 only in shpool. It found it, and generated a fix. Its initial attempt used unsafe, but when prompted, it switched to using the nix crate. I read daemonize's docs re default mode. I tidied up the code (removed verbose comments, improved variable name).

This PR was:

  • mostly or completely vibe coded
  • mostly or completely meat coded
  • bit of both

Description

When shpool autodaemonizes, it uses the daemonize crate to detach the daemon process. By default, the daemonize crate overrides the process umask to 0o027.

This causes the daemon (and all shells it subsequently spawns) to run with umask 0027, even if the user launched shpool attach with another umask (e.g. 0022). I noticed this when I saw some tools/workflows break only in shpool.

This change fixes the issue by inherting the umask. The parent is the correct place to decide on umask.

Steps to reproduce:

$ umask
0022
$ killall shpool; shpool --socket $(mktemp) attach --cmd "bash -c 'umask; sleep 1s'" foo
0027

With this commit:

$ umask
0022
$ cargo run -- --socket $(mktemp) attach --cmd "bash -c 'umask; sleep 1s'" foo
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
     Running `target/debug/shpool --socket /tmp/tmp.POFOUoQDt2 attach --cmd 'bash -c '\''umask; sleep 1s'\''' foo`
0022

When shpool autodaemonizes, it uses the `daemonize` crate to detach
the daemon process. By default, the `daemonize` crate overrides the
process umask to `0o027`.

This causes the daemon (and all shells it subsequently spawns) to run
with umask `0027`, even if the user launched `shpool attach` with
another umask (e.g. `0022`). I noticed this when I saw some
tools/workflows break only in shpool.

This change fixes the issue by inherting the umask. The parent is
the correct place to decide on umask.

Steps to reproduce:

```
$ umask
0022
$ killall shpool; shpool --socket $(mktemp) attach --cmd "bash -c 'umask; sleep 1s'" foo
0027
```

With this commit:

```
$ umask
0022
$ cargo run -- --socket $(mktemp) attach --cmd "bash -c 'umask; sleep 1s'" foo
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
     Running `target/debug/shpool --socket /tmp/tmp.POFOUoQDt2 attach --cmd 'bash -c '\''umask; sleep 1s'\''' foo`
0022
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant