Respect explicitly set CI environment variable (#336)

* fix(proxy): do not override explicitly-set CI env var

pmg forces CI=true for non-interactive (non-PTY) proxy runs so package
managers behave non-interactively. This clobbered a CI value the user set
explicitly (e.g. CI=false on a build server), changing downstream tool
behavior unexpectedly.

Only inject CI=true when CI is not already present in the environment,
preserving the user's intent. mergeEnv override semantics are left intact
since other overrides (HTTP_PROXY, etc.) must clobber.

Fixes #335

* test(proxy): snapshot/restore CI env explicitly in override test

Address review feedback: make the unset-CI subtest's intent explicit by
snapshotting the original CI value, unsetting it for the test, and
restoring it in t.Cleanup instead of relying on t.Setenv cleanup.

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Abhisek Datta
2026-06-13 09:56:43 +05:30
committed by GitHub
co-authored by Claude
parent 788a031003
commit 9a02e8de87
2 changed files with 40 additions and 1 deletions
+11 -1
View File
@@ -207,7 +207,7 @@ func (f *proxyFlow) Run(ctx context.Context, args []string, parsedCmd *packagema
DryRun: cfg.DryRun,
Mode: runner.ExecutionModeAuto,
EnvOverrides: f.setupEnvForProxy(proxyAddr, caCertPath),
DirectEnvOverrides: []string{"CI=true"},
DirectEnvOverrides: ciEnvOverride(),
BeforeDirectRun: func() error {
log.Debugf("Executing proxy for non interactive TTY")
@@ -412,6 +412,16 @@ func (f *proxyFlow) createAndStartProxyServer(
return proxyServer, proxyAddr, nil
}
// ciEnvOverride forces CI=true for non-interactive runs so package managers
// behave non-interactively. It respects an explicitly set CI value (including
// CI=false) so we don't clobber the user's intent. See issue #335.
func ciEnvOverride() []string {
if _, ok := os.LookupEnv("CI"); ok {
return nil
}
return []string{"CI=true"}
}
func (f *proxyFlow) setupEnvForProxy(proxyAddr, caCertPath string) []string {
proxyURL := fmt.Sprintf("http://%s", proxyAddr)
+29
View File
@@ -1,10 +1,12 @@
package flows
import (
"os"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func envToMap(env []string) map[string]string {
@@ -38,3 +40,30 @@ func TestSetupEnvForProxyConfiguresYarn(t *testing.T) {
assert.Equal(t, caCertPath, env["YARN_HTTPS_CA_FILE_PATH"],
"yarn ignores NODE_EXTRA_CA_CERTS; YARN_HTTPS_CA_FILE_PATH is required to trust the MITM CA")
}
// TestCIEnvOverride proves the fix for #335: pmg forces CI=true for
// non-interactive runs but must not clobber a CI value the user set
// explicitly (e.g. CI=false on a build server).
func TestCIEnvOverride(t *testing.T) {
t.Run("sets CI=true when unset", func(t *testing.T) {
original, hadCI := os.LookupEnv("CI")
require.NoError(t, os.Unsetenv("CI"))
t.Cleanup(func() {
if hadCI {
require.NoError(t, os.Setenv("CI", original))
}
})
assert.Equal(t, []string{"CI=true"}, ciEnvOverride())
})
t.Run("does not override explicitly set CI", func(t *testing.T) {
t.Setenv("CI", "false")
assert.Nil(t, ciEnvOverride())
})
t.Run("does not override CI set to empty", func(t *testing.T) {
t.Setenv("CI", "")
assert.Nil(t, ciEnvOverride())
})
}