Skip to content

Conversation

@bolinfest
Copy link
Contributor

Currently, shellenv.sh is frequently run as part of a user's ~/.zprofile,
which includes the following line:

HOMEBREW_SHELL_NAME="$(/bin/ps -p "${PPID}" -c -o comm=)"

Indeed, if I run the following from zsh, this reports zsh (or -zsh), as expected:

$ bash -c '/bin/ps -p "${PPID}" -c -o comm='
-zsh

Though /bin/ps is a setuid binary:

$ stat -f "%Sp" /bin/ps
-rwsr-xr-x

Which means that it cannot be run under macOS Seatbelt unless explicit support
is added to the Seatbelt policy to run it outside of the sandbox:

(allow process-exec (path "/bin/ps") (with no-sandbox))

Though this is somewhat undesirable because /bin/ps makes it possible to
dump environment variables from processes, which often includes sensitive
information such as API keys.

By comparison, /usr/sbin/lsof is not setuid:

$ stat -f "%Sp" /usr/sbin/lsof
-rwxr-xr-x

And the proposed change yields the same information (without the leading hyphen):

$ bash -c '/usr/sbin/lsof -p "$PPID" -a -d cwd -Fc | /usr/bin/sed -n "s/^c//p"'
zsh

Overall, this would make it possible to invoke zsh -lc under Seatbelt for
users who run shellenv.sh in their ~/.zprofile without having to use a less
restrictive Seatbelt policy.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Currently, `shellenv.sh` is frequently run as part of a user's `~/.zprofile`,
which includes the following line:

```shell
HOMEBREW_SHELL_NAME="$(/bin/ps -p "${PPID}" -c -o comm=)"
```

Indeed, if I run the following from `zsh`, this reports `zsh` (or `-zsh`), as expected:

```shell
$ bash -c '/bin/ps -p "${PPID}" -c -o comm='
-zsh
```

Though `/bin/ps` is a setuid binary:

```shell
$ stat -f "%Sp" /bin/ps
-rwsr-xr-x
```

Which means that it cannot be run under macOS Seatbelt unless explicit support
is added to the Seatbelt policy to run it outside of the sandbox:

```
(allow process-exec (path "/bin/ps") (with no-sandbox))
```

Though this is somewhat undesirable because `/bin/ps` makes it possible to
dump environment variables from processes, which often includes sensitive
information such as API keys.

By comparison, `/usr/sbin/lsof` is _not_ setuid:

```shell
$ stat -f "%Sp" /usr/sbin/lsof
-rwxr-xr-x
```

And the proposed change yields the same information (without the leading hyphen):

```shell
$ bash -c '/usr/sbin/lsof -p "$PPID" -a -d cwd -Fc | /usr/bin/sed -n "s/^c//p"'
zsh
```

Overall, this would make it possible to invoke `zsh -lc` under Seatbelt for
users who run `shellenv.sh` in their `~/.zprofile` without having to use a less
restrictive Seatbelt policy.
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MikeMcQuaid MikeMcQuaid enabled auto-merge December 2, 2025 08:27
@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Dec 2, 2025
Merged via the queue into Homebrew:main with commit 7cbdff2 Dec 2, 2025
48 of 57 checks passed
bolinfest added a commit to openai/codex that referenced this pull request Dec 7, 2025
…ixes (#7680)

As noted in the code comment, we introduced a key fix for `brew` in
Homebrew/brew#21157 that Codex needs, but it has
not hit stable yet, so we update our CI job to use latest `brew` from
`origin/main`.

This is necessary for the new integration tests introduced in
#7617.
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.

2 participants