Brew: Filter all environment variables by default

[MikeMcQuaid]

Homebrew should filter all user environment variables and favour the system defaults by default when doing brew install (or friends) or brew test. This will avoid environment contamination breaking the from-source build.

[zmwangx]

Like env -i?

[MikeMcQuaid]

Somewhat, yeh. We'll need to obviously keep some variables around. This may end up being either a whitelist or sourceing the system defaults.

[zmwangx]

login(1): HOME, SHELL, PATH, TERM, LOGNAME and USER. Everything except PATH is obvious, and PATH set through

eval `/usr/libexec/path_helper -s`

but path_helper is affected by /etc/paths and /etc/paths.d which could be modified by sysadmin.

Anything else?

[MikeMcQuaid]

TMPDIR, PWD (needed for some commands), LANG, LOGNAME, EDITOR and obviously HOMEBREW_* stuff.

[zmwangx]

TMPDIR and friends are already overwritten by brew (#817).
LANG and friends: should we really allow users to set these to arbitrary values during install and test?
EDITOR: why do you ever need it during install and test?
HOMEBREW_*: maybe they can all be handled prior to resetting env? Anyway, shouldn't be hard to whitelist.

[MikeMcQuaid]

Agreed all around, then 👍

[MikeMcQuaid]

I'm more thinking with LANG that things may 💥 with no value set.

[zmwangx]

I'm more thinking with LANG that things may 💥 with no value set.

Yeah, we can probably set LC_ALL to C or en_US.UTF-8.

[apjanke]

:+1:

For LANG et al, setting LANG and clearing the LC_* should be sufficient, and more minimal. LANG must be set.

I suspect we should use the system's default <region>.UTF-8. Like @zmwangx, says, we probably don't want users setting these to arbitrary values; the installation processes might be sniffing them. And from my experience with Oh My Zsh, it's kinda common for users to set these to wacky things.

Setting it to C will break some things, because that invokes undefined/system-dependent behavior, and different languages and libraries deal with it differently.

[zmwangx]

For LANG et al, setting LANG and clearing the LC_* should be sufficient, and more minimal. LANG must be set.

Why? LANG is just a fallback, and when LC_ALL is set you don't need a fallback because everything's covered. See locale(1).

Setting it to C will break some things, because that invokes undefined/system-dependent behavior

Isn't C the well-defined POSIX locale? But sure, I don't do a lot of locale-related stuff so if you're knowledgeable on that front I'll take your word for it.

[MikeMcQuaid]

Isn't C the well-defined POSIX locale? But sure, I don't do a lot of locale-related stuff so if you're knowledgeable on that front I'll take your word for it.

It can also be interpreted as "don't support UTF-8". We now have MacOS.language that we can use to read the system default.

[zmwangx]

It can also be interpreted as "don't support UTF-8".

Good point.

We now have MacOS.language that we can use to read the system default.

👍

[apjanke]

Why? LANG is just a fallback, and when LC_ALL is set you don't need a fallback because everything's covered. See locale(1).

Viewed the other way, LANG is the primary definition, and the others are just overrides, and LC_ALL is an override for the overrides. It doesn't really matter; I just find a single LANG variable feels cleaner.

Isn't C the well-defined POSIX locale?

Yes, C is the well-defined POSIX locale, but not all characters (well, byte sequences, really) are well-defined under that locale. Specifically, only the "Portable Character Set" (basically, the characters for 7-bit ASCII, and their corresponding encodings) is defined under POSIX. Other character/encoding values are technically undefined, and will have system-dependent behavior. Most libraries and applications will treat them as opaque values and assume a single-byte encoding (basically, doing the C char thing or treating it as binary data), but that's not required by POSIX, and some implementations may differ. In particular, when ruby is running under C, it will treat byte values over 127 as invalid encodings and throw an error when encountering them. (I'd guess because Ruby comes from Japan, and most of the rest of Unix comes from America, where non-English encodings are less of a concern.)

The POSIX locale is a minimal locale intended for portability, not one that handles everything in some uniform way.

Yeah, I have done a lot of locale and character-encoding work. Trust me that this is a rabbit hole we could go down forever before getting back to Homebrew's env sanitization. :smile:

[gelraen]

Got bitten by (the lack of) this: TOP var leaked from my shell and broke the build of [email protected] for me: https://gist.githubusercontent.com/anonymous/e0bdd43763bb19d6e1499e00de399842/raw/b2a695e9ad714316add792524d8d3a7aaa615c7d/03.make

[johnhawkinson]

Somehow it seems that PR #1753 never got referenced here, as a draft implementation of this.
So fixing that.
(I was directed here via https://github.com/Homebrew/homebrew-core/issues/10133 but never saw #1753 was active)

It doe sort of look like there are some unresolved subtleties in the discussion here that the #1753 implementor should maybe address.

[akvadrako]

This would be quite nice to have. I recently encountered a formula that broke because I have LIBDIR set in my environment and it was rather confusing to debug.

[ilovezfs]

@akvadrako you can try it out

export HOMEBREW_ENV_FILTERING=1

[MikeMcQuaid]

This will be done by default in Homebrew 1.4.0 (https://github.com/Homebrew/brew/pull/3529) and is done by default right now for anyone on the master branch. It's caused minimal breakage which is nice.