Cabal 3.14.1.0 invokes test binaries with a corrupt (duplicated) environment variable list #10718

Rufflewind · 2025-01-06T12:48:57Z

Describe the bug

cabal 3.14.1.0 invokes test binaries with with an invalid environment variable list that contains duplicate entries.

As documented by POSIX: https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap08.html

If more than one string in an environment of a process has the same name, the consequences are undefined.

This is causing the directory CI tests to fail: https://github.com/haskell/directory/actions/runs/12595046406

The bug is not found in cabal 3.12.1.0.

To Reproduce

-- Main.hs
module Main where
import Data.List (sort)
import System.Environment (getEnvironment)
main = print =<< fmap sort getEnvironment

-- foo.cabal
cabal-version:      3.0
name:               foo
version:            0.1.0.0
build-type:         Simple

test-suite test
    build-depends:    base
    default-language: Haskell2010
    main-is:          Main.hs
    type:             exitcode-stdio-1.0

$ cabal test

Actual output

[("COLORTERM","truecolor"),("COLORTERM","truecolor"),("DBUS_SESSION_BUS_ADDRESS",...),("DBUS_SESSION_BUS_ADDRESS",...),...

Expected behavior

There should not be any duplicates, i.e.

[("COLORTERM","truecolor"),("DBUS_SESSION_BUS_ADDRESS",...),...

System information

Operating system: Tested on Ubuntu (Github Actions) + WSL Debian
Reproducible on cabal 3.14.1.0. Not reproducible on cabal 3.12.1.0.
Reproducible on several GHC versions.

The text was updated successfully, but these errors were encountered:

haskell/cabal#10718 breaks the tests.

ulysses4ever · 2025-01-06T14:20:33Z

Hey! Thanks for the report! Presumably, this issue doesn't lead to a failure in any case, does it? I'm asking because it'd be good to understand how severe it is. On the surface it looks rather benign to me.

Rufflewind · 2025-01-06T20:50:09Z

It may cause uses of environment variables in tests to behave incorrectly. In the theoretical worst-case scenario, it could lead to security risks, e.g.

The severity of this issue in practice is unclear. The most probable scenario I can think of is that a fragile piece of code that scrubs sensitive data out of environment variables could misbehave due to the duplication, causing credentials to get leaked through, say, logs. A bit of a stretch but not impossible.

mpickering · 2025-03-10T14:16:57Z

I have written a test and you are right that with 3.14, many duplicates are passed.

With 3.12, the p_datadir variable is passed twice.

mpickering · 2025-03-10T15:07:13Z

I see the issue was introduced in ee11ac6

This fixes a bug where environment variables were duplicated when running executables. ``` overrideEnv <- fromMaybe [] <$> getEffectiveEnvironment ([("PATH", Just newPath)] ++ envOverrides) let shellEnv = overrideEnv ++ existingEnv ``` Since getEffectiveEnvironment already calls getEnvironment internally, if any overrides are passed then the result is a complete environment. Appending it to the already existing environment results in duplicated environment variables. The fix: * Added getFullEnvironment function to handle the common pattern correctly * Updated code in Bench, Test/ExeV10, Test/LibV09, and Client/Run to use this function In the future it would be good to generalise `getFullEnvironment` further so it can also handle the `addLibraryPath` case, which modifies an environment variable, rather than merely setting or unsetting it. Fixes #10718