From 4a80c4d26167d0de9ae90dc7f4ac7d8ff541f847 Mon Sep 17 00:00:00 2001
From: Cian Johnston <cian@coder.com>
Date: Mon, 17 Jun 2024 14:29:49 +0100
Subject: [PATCH 1/3] fix: fix unsetOptionsEnv, add integration test

---
 envbuilder.go                   | 14 +++++---------
 integration/integration_test.go | 29 +++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/envbuilder.go b/envbuilder.go
index 467320fe..de3243b6 100644
--- a/envbuilder.go
+++ b/envbuilder.go
@@ -18,7 +18,6 @@ import (
 	"os/exec"
 	"os/user"
 	"path/filepath"
-	"reflect"
 	"sort"
 	"strconv"
 	"strings"
@@ -1064,16 +1063,13 @@ func createPostStartScript(path string, postStartCommand devcontainer.LifecycleS
 // unsetOptionsEnv unsets all environment variables that are used
 // to configure the options.
 func unsetOptionsEnv() {
-	val := reflect.ValueOf(&Options{}).Elem()
-	typ := val.Type()
-
-	for i := 0; i < val.NumField(); i++ {
-		fieldTyp := typ.Field(i)
-		env := fieldTyp.Tag.Get("env")
-		if env == "" {
+	var o Options
+	for _, opt := range o.CLI() {
+		if opt.Env == "" {
 			continue
 		}
-		os.Unsetenv(env)
+		os.Unsetenv(opt.Env)
+		os.Unsetenv(strings.TrimPrefix(opt.Env, envPrefix))
 	}
 }
 
diff --git a/integration/integration_test.go b/integration/integration_test.go
index 05aced57..2a20f2f1 100644
--- a/integration/integration_test.go
+++ b/integration/integration_test.go
@@ -695,6 +695,35 @@ PATH=/usr/local/bin:/bin:/go/bin:/opt
 REMOTE_BAR=bar`)
 }
 
+func TestUnsetOptionsEnv(t *testing.T) {
+	t.Parallel()
+
+	// Ensures that a Git repository with a devcontainer.json is cloned and built.
+	srv := createGitServer(t, gitServerOptions{
+		files: map[string]string{
+			".devcontainer/devcontainer.json": `{
+				"name": "Test",
+				"build": {
+					"dockerfile": "Dockerfile"
+				},
+			}`,
+			".devcontainer/Dockerfile": "FROM " + testImageAlpine + "\nENV FROM_DOCKERFILE=foo",
+		},
+	})
+	ctr, err := runEnvbuilder(t, options{env: []string{
+		envbuilderEnv("GIT_URL", srv.URL),
+		envbuilderEnv("INIT_SCRIPT", "env > /root/env.txt && sleep infinity"),
+	}})
+	require.NoError(t, err)
+
+	output := execContainer(t, ctr, "cat /root/env.txt")
+	for _, s := range strings.Split(strings.TrimSpace(output), "\n") {
+		if strings.HasPrefix(s, envbuilder.WithEnvPrefix("")) {
+			assert.Fail(t, "environment variable should be stripped when running init script", s)
+		}
+	}
+}
+
 func TestLifecycleScripts(t *testing.T) {
 	t.Parallel()
 

From b73d2cf39333011ea01487990f9fad4f092c301e Mon Sep 17 00:00:00 2001
From: Cian Johnston <cian@coder.com>
Date: Mon, 17 Jun 2024 14:38:19 +0100
Subject: [PATCH 2/3] fixup! fix: fix unsetOptionsEnv, add integration test

---
 envbuilder.go                   |  5 +++++
 integration/integration_test.go | 15 +++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/envbuilder.go b/envbuilder.go
index de3243b6..bc90e555 100644
--- a/envbuilder.go
+++ b/envbuilder.go
@@ -1068,6 +1068,11 @@ func unsetOptionsEnv() {
 		if opt.Env == "" {
 			continue
 		}
+		// Do not strip options that do not have the magic prefix!
+		if !strings.HasPrefix(opt.Env, envPrefix) {
+			continue
+		}
+		// Strip both with and without prefix.
 		os.Unsetenv(opt.Env)
 		os.Unsetenv(strings.TrimPrefix(opt.Env, envPrefix))
 	}
diff --git a/integration/integration_test.go b/integration/integration_test.go
index 2a20f2f1..caacad56 100644
--- a/integration/integration_test.go
+++ b/integration/integration_test.go
@@ -712,14 +712,25 @@ func TestUnsetOptionsEnv(t *testing.T) {
 	})
 	ctr, err := runEnvbuilder(t, options{env: []string{
 		envbuilderEnv("GIT_URL", srv.URL),
+		"GIT_URL", srv.URL,
+		envbuilderEnv("GIT_PASSWORD", "supersecret"),
+		"GIT_PASSWORD", "supersecret",
 		envbuilderEnv("INIT_SCRIPT", "env > /root/env.txt && sleep infinity"),
+		"INIT_SCRIPT", "env > /root/env.txt && sleep infinity",
 	}})
 	require.NoError(t, err)
 
 	output := execContainer(t, ctr, "cat /root/env.txt")
+	var os envbuilder.Options
 	for _, s := range strings.Split(strings.TrimSpace(output), "\n") {
-		if strings.HasPrefix(s, envbuilder.WithEnvPrefix("")) {
-			assert.Fail(t, "environment variable should be stripped when running init script", s)
+		for _, o := range os.CLI() {
+			if strings.HasPrefix(s, o.Env) {
+				assert.Fail(t, "environment variable should be stripped when running init script", s)
+			}
+			optWithoutPrefix := strings.TrimPrefix(o.Env, envbuilder.WithEnvPrefix(""))
+			if strings.HasPrefix(s, optWithoutPrefix) {
+				assert.Fail(t, "environment variable should be stripped when running init script", s)
+			}
 		}
 	}
 }

From a63f895326034f7315d478950aef1d331178c348 Mon Sep 17 00:00:00 2001
From: Cian Johnston <cian@coder.com>
Date: Mon, 17 Jun 2024 14:40:45 +0100
Subject: [PATCH 3/3] add clarifying comment

---
 envbuilder.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/envbuilder.go b/envbuilder.go
index bc90e555..6fbc4f04 100644
--- a/envbuilder.go
+++ b/envbuilder.go
@@ -1069,6 +1069,7 @@ func unsetOptionsEnv() {
 			continue
 		}
 		// Do not strip options that do not have the magic prefix!
+		// For example, CODER_AGENT_URL, CODER_AGENT_TOKEN, CODER_AGENT_SUBSYSTEM.
 		if !strings.HasPrefix(opt.Env, envPrefix) {
 			continue
 		}