From 3f008d52e8f0545e3b768de2f8a577e96fc5d164 Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 15:58:58 -0800
Subject: [PATCH 01/14] Test parsing PAM to get shell envs

---
 pkg/shellexec/shellexec.go         |  33 ++++++++-
 pkg/util/pamparse/pamparse.go      | 108 +++++++++++++++++++++++++++++
 pkg/util/pamparse/pamparse_test.go |  91 ++++++++++++++++++++++++
 3 files changed, 231 insertions(+), 1 deletion(-)
 create mode 100644 pkg/util/pamparse/pamparse.go
 create mode 100644 pkg/util/pamparse/pamparse_test.go

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index 35a9401ad6..d7aa110179 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -22,6 +22,7 @@ import (
 	"github.com/wavetermdev/waveterm/pkg/panichandler"
 	"github.com/wavetermdev/waveterm/pkg/remote"
 	"github.com/wavetermdev/waveterm/pkg/remote/conncontroller"
+	"github.com/wavetermdev/waveterm/pkg/util/pamparse"
 	"github.com/wavetermdev/waveterm/pkg/util/shellutil"
 	"github.com/wavetermdev/waveterm/pkg/util/utilfn"
 	"github.com/wavetermdev/waveterm/pkg/wavebase"
@@ -443,10 +444,14 @@ func StartShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOpt
 			}
 		}
 		ecmd = exec.Command(shellPath, shellOpts...)
-		ecmd.Env = os.Environ()
+		ecmd.Env = make([]string, 0)
 		if isZshShell(shellPath) {
 			shellutil.UpdateCmdEnv(ecmd, map[string]string{"ZDOTDIR": shellutil.GetZshZDotDir()})
 		}
+		pamEnvs := tryGetPamEnvVars()
+		if len(pamEnvs) > 0 {
+			shellutil.UpdateCmdEnv(ecmd, pamEnvs)
+		}
 	} else {
 		shellOpts = append(shellOpts, "-c", cmdStr)
 		ecmd = exec.Command(shellPath, shellOpts...)
@@ -512,3 +517,29 @@ func RunSimpleCmdInPty(ecmd *exec.Cmd, termSize waveobj.TermSize) ([]byte, error
 	<-ioDone
 	return outputBuf.Bytes(), nil
 }
+
+const etcEnvironmentPath = "/etc/environment"
+const etcSecurityPath = "/etc/security/pam_env.conf"
+const userEnvironmentPath = "~/.pam_environment"
+
+func tryGetPamEnvVars() map[string]string {
+	envVars, err := pamparse.ParseEnvironmentFile(etcEnvironmentPath)
+	if err != nil {
+		log.Printf("error parsing %s: %v", etcEnvironmentPath, err)
+	}
+	envVars2, err := pamparse.ParseEnvironmentConfFile(etcSecurityPath)
+	if err != nil {
+		log.Printf("error parsing %s: %v", etcSecurityPath, err)
+	}
+	envVars3, err := pamparse.ParseEnvironmentConfFile(userEnvironmentPath)
+	if err != nil {
+		log.Printf("error parsing %s: %v", userEnvironmentPath, err)
+	}
+	for k, v := range envVars2 {
+		envVars[k] = v
+	}
+	for k, v := range envVars3 {
+		envVars[k] = v
+	}
+	return envVars
+}
diff --git a/pkg/util/pamparse/pamparse.go b/pkg/util/pamparse/pamparse.go
new file mode 100644
index 0000000000..04745c2d4d
--- /dev/null
+++ b/pkg/util/pamparse/pamparse.go
@@ -0,0 +1,108 @@
+// Copyright 2025, Command Line Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+// Package pamparse provides functions for parsing environment files in the format of /etc/environment, /etc/security/pam_env.conf, and ~/.pam_environment.
+package pamparse
+
+import (
+	"bufio"
+	"os"
+	"regexp"
+	"strings"
+)
+
+// Parses a file in the format of /etc/environment. Accepts a path to the file and returns a map of environment variables.
+func ParseEnvironmentFile(path string) (map[string]string, error) {
+	rtn := make(map[string]string)
+	file, err := os.OpenFile(path, os.O_RDONLY, 0)
+	if err != nil {
+		return nil, err
+	}
+	defer file.Close()
+	scanner := bufio.NewScanner(file)
+	for scanner.Scan() {
+		line := scanner.Text()
+		key, val := parseEnvironmentLine(line)
+		if key == "" {
+			continue
+		}
+		rtn[key] = val
+	}
+	return rtn, nil
+}
+
+// Parses a file in the format of /etc/security/pam_env.conf or ~/.pam_environment. Accepts a path to the file and returns a map of environment variables.
+func ParseEnvironmentConfFile(path string) (map[string]string, error) {
+	rtn := make(map[string]string)
+	file, err := os.OpenFile(path, os.O_RDONLY, 0)
+	if err != nil {
+		return nil, err
+	}
+	defer file.Close()
+	scanner := bufio.NewScanner(file)
+	for scanner.Scan() {
+		line := scanner.Text()
+		key, val := parseEnvironmentConfLine(line)
+
+		// Fall back to ParseEnvironmentLine if ParseEnvironmentConfLine fails
+		if key == "" {
+			key, val = parseEnvironmentLine(line)
+			if key == "" {
+				continue
+			}
+		}
+		rtn[key] = val
+	}
+	return rtn, nil
+}
+
+var envFileLineRe = regexp.MustCompile(`^(?:export\s+)?([A-Z0-9_]+[A-Za-z0-9]*)=(.*)$`)
+
+func parseEnvironmentLine(line string) (string, string) {
+	m := envFileLineRe.FindStringSubmatch(line)
+	if m == nil {
+		return "", ""
+	}
+	return m[1], sanitizeEnvVarValue(m[2])
+}
+
+var confFileLineRe = regexp.MustCompile(`^([A-Z0-9_]+[A-Za-z0-9]*)\s+(?:(?:DEFAULT=)([^\s]+(?: \w+)*))\s*(?:(?:OVERRIDE=)([^\s]+(?: \w+)*))?\s*$`)
+
+func parseEnvironmentConfLine(line string) (string, string) {
+	m := confFileLineRe.FindStringSubmatch(line)
+	if m == nil {
+		return "", ""
+	}
+	var vals []string
+	if len(m) > 3 && m[3] != "" {
+		vals = []string{sanitizeEnvVarValue(m[3]), sanitizeEnvVarValue(m[2])}
+	} else {
+		vals = []string{sanitizeEnvVarValue(m[2])}
+	}
+	return m[1], strings.Join(vals, ":")
+}
+
+// Sanitizes an environment variable value by stripping comments and trimming quotes.
+func sanitizeEnvVarValue(val string) string {
+	return stripComments(trimQuotes(val))
+}
+
+// Trims quotes as defined by https://unix.stackexchange.com/questions/748790/where-is-the-syntax-for-etc-environment-documented
+func trimQuotes(val string) string {
+	if strings.HasPrefix(val, "\"") || strings.HasPrefix(val, "'") {
+		val = val[1:]
+		if strings.HasSuffix(val, "\"") || strings.HasSuffix(val, "'") {
+			val = val[0 : len(val)-1]
+		}
+	}
+	return val
+}
+
+// Strips comments as defined by https://unix.stackexchange.com/questions/748790/where-is-the-syntax-for-etc-environment-documented
+func stripComments(val string) string {
+	commentIdx := strings.Index(val, "#")
+	if commentIdx == -1 {
+		return val
+	}
+	return val[0:commentIdx]
+}
diff --git a/pkg/util/pamparse/pamparse_test.go b/pkg/util/pamparse/pamparse_test.go
new file mode 100644
index 0000000000..acfaa8d876
--- /dev/null
+++ b/pkg/util/pamparse/pamparse_test.go
@@ -0,0 +1,91 @@
+package pamparse_test
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/wavetermdev/waveterm/pkg/util/pamparse"
+)
+
+// Tests influenced by https://unix.stackexchange.com/questions/748790/where-is-the-syntax-for-etc-environment-documented
+func TestParseEnvironmentFile(t *testing.T) {
+	const fileContent = `
+FOO1=bar
+FOO2="bar"
+FOO3="bar
+FOO4=bar"
+FOO5='bar'
+FOO6='bar"
+export FOO7=bar
+FOO8=bar bar bar
+#FOO9=bar
+FOO10=$PATH
+FOO11="foo#bar"
+	`
+
+	// create a temporary file with the content
+	tempFile := filepath.Join(t.TempDir(), "pam_env")
+	if err := os.WriteFile(tempFile, []byte(fileContent), 0644); err != nil {
+		t.Fatalf("failed to write file: %v", err)
+	}
+
+	// parse the file
+	env, err := pamparse.ParseEnvironmentFile(tempFile)
+	if err != nil {
+		t.Fatalf("failed to parse pam environment file: %v", err)
+	}
+	if len(env) != 10 {
+		t.Fatalf("expected 10 environment variables, got %d", len(env))
+	}
+	for k, v := range map[string]string{
+		"FOO1":  "bar",
+		"FOO2":  "bar",
+		"FOO3":  "bar",
+		"FOO4":  "bar\"",
+		"FOO5":  "bar",
+		"FOO6":  "bar",
+		"FOO7":  "bar",
+		"FOO8":  "bar bar bar",
+		"FOO10": "$PATH",
+		"FOO11": "foo",
+	} {
+		if env[k] != v {
+			t.Errorf("expected %q to be %q, got %q", k, v, env[k])
+		}
+	}
+}
+
+func TestParseEnvironmentConfFile(t *testing.T) {
+	const fileContent = `
+TEST   DEFAULT=@{HOME}/.config\ state   OVERRIDE=./config\ s
+FOO   DEFAULT=@{HOME}/.config\ s
+STRING   DEFAULT="string"
+STRINGOVERRIDE   DEFAULT="string"   OVERRIDE="string2"
+	`
+
+	// create a temporary file with the content
+	tempFile := filepath.Join(t.TempDir(), "pam_env_conf")
+	if err := os.WriteFile(tempFile, []byte(fileContent), 0644); err != nil {
+		t.Fatalf("failed to write file: %v", err)
+	}
+
+	// parse the file
+	env, err := pamparse.ParseEnvironmentConfFile(tempFile)
+	if err != nil {
+		t.Fatalf("failed to parse pam environment conf file: %v", err)
+	}
+	if len(env) != 4 {
+		t.Fatalf("expected 4 environment variables, got %d", len(env))
+	}
+	for k, v := range map[string]string{
+		"TEST":           "./config\\ s:@{HOME}/.config\\ state",
+		"FOO":            "@{HOME}/.config\\ s",
+		"STRING":         "string",
+		"STRINGOVERRIDE": "string2:string",
+	} {
+		if env[k] != v {
+			t.Errorf("expected %q to be %q, got %q", k, v, env[k])
+		}
+	}
+}

From 525f33c79784d8ee9e95314300284e8aa6229bdc Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 16:24:33 -0800
Subject: [PATCH 02/14] move code

---
 pkg/shellexec/shellexec.go | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index d7aa110179..0b4f2794cf 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -444,19 +444,25 @@ func StartShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOpt
 			}
 		}
 		ecmd = exec.Command(shellPath, shellOpts...)
-		ecmd.Env = make([]string, 0)
+		ecmd.Env = os.Environ()
 		if isZshShell(shellPath) {
 			shellutil.UpdateCmdEnv(ecmd, map[string]string{"ZDOTDIR": shellutil.GetZshZDotDir()})
 		}
-		pamEnvs := tryGetPamEnvVars()
-		if len(pamEnvs) > 0 {
-			shellutil.UpdateCmdEnv(ecmd, pamEnvs)
-		}
 	} else {
 		shellOpts = append(shellOpts, "-c", cmdStr)
 		ecmd = exec.Command(shellPath, shellOpts...)
 		ecmd.Env = os.Environ()
 	}
+
+	// For Snap installations, we need to unset XDG environment variables as Snap overrides them to point to snap directories
+	if os.Getenv("SNAP") != "" {
+		shellutil.UpdateCmdEnv(ecmd, map[string]string{"XDG_CONFIG_HOME": "", "XDG_DATA_HOME": "", "XDG_CACHE_HOME": "", "XDG_RUNTIME_DIR": "", "XDG_CONFIG_DIRS": "", "XDG_DATA_DIRS": ""})
+		pamEnvs := tryGetPamEnvVars()
+		if len(pamEnvs) > 0 {
+			shellutil.UpdateCmdEnv(ecmd, pamEnvs)
+		}
+	}
+
 	if cmdOpts.Cwd != "" {
 		ecmd.Dir = cmdOpts.Cwd
 	}

From 4f2138aad326db81bdf5f32c8149d711a4b4fca4 Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 16:36:20 -0800
Subject: [PATCH 03/14] update tests

---
 pkg/util/pamparse/pamparse_test.go | 40 ++++++++++++++++++------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/pkg/util/pamparse/pamparse_test.go b/pkg/util/pamparse/pamparse_test.go
index acfaa8d876..027ed4b2b7 100644
--- a/pkg/util/pamparse/pamparse_test.go
+++ b/pkg/util/pamparse/pamparse_test.go
@@ -31,14 +31,12 @@ FOO11="foo#bar"
 	}
 
 	// parse the file
-	env, err := pamparse.ParseEnvironmentFile(tempFile)
+	got, err := pamparse.ParseEnvironmentFile(tempFile)
 	if err != nil {
 		t.Fatalf("failed to parse pam environment file: %v", err)
 	}
-	if len(env) != 10 {
-		t.Fatalf("expected 10 environment variables, got %d", len(env))
-	}
-	for k, v := range map[string]string{
+
+	want := map[string]string{
 		"FOO1":  "bar",
 		"FOO2":  "bar",
 		"FOO3":  "bar",
@@ -49,9 +47,14 @@ FOO11="foo#bar"
 		"FOO8":  "bar bar bar",
 		"FOO10": "$PATH",
 		"FOO11": "foo",
-	} {
-		if env[k] != v {
-			t.Errorf("expected %q to be %q, got %q", k, v, env[k])
+	}
+
+	if len(got) != len(want) {
+		t.Fatalf("expected %d environment variables, got %d", len(want), len(got))
+	}
+	for k, v := range want {
+		if got[k] != v {
+			t.Errorf("expected %q to be %q, got %q", k, v, got[k])
 		}
 	}
 }
@@ -62,6 +65,7 @@ TEST   DEFAULT=@{HOME}/.config\ state   OVERRIDE=./config\ s
 FOO   DEFAULT=@{HOME}/.config\ s
 STRING   DEFAULT="string"
 STRINGOVERRIDE   DEFAULT="string"   OVERRIDE="string2"
+FOO11="foo#bar"
 	`
 
 	// create a temporary file with the content
@@ -71,21 +75,25 @@ STRINGOVERRIDE   DEFAULT="string"   OVERRIDE="string2"
 	}
 
 	// parse the file
-	env, err := pamparse.ParseEnvironmentConfFile(tempFile)
+	got, err := pamparse.ParseEnvironmentConfFile(tempFile)
 	if err != nil {
 		t.Fatalf("failed to parse pam environment conf file: %v", err)
 	}
-	if len(env) != 4 {
-		t.Fatalf("expected 4 environment variables, got %d", len(env))
-	}
-	for k, v := range map[string]string{
+
+	want := map[string]string{
 		"TEST":           "./config\\ s:@{HOME}/.config\\ state",
 		"FOO":            "@{HOME}/.config\\ s",
 		"STRING":         "string",
 		"STRINGOVERRIDE": "string2:string",
-	} {
-		if env[k] != v {
-			t.Errorf("expected %q to be %q, got %q", k, v, env[k])
+		"FOO11":          "foo",
+	}
+
+	if len(got) != len(want) {
+		t.Fatalf("expected %d environment variables, got %d", len(want), len(got))
+	}
+	for k, v := range want {
+		if got[k] != v {
+			t.Errorf("expected %q to be %q, got %q", k, v, got[k])
 		}
 	}
 }

From 7e06dc95b37241cff04d2213f97190f79e5da608 Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 16:39:30 -0800
Subject: [PATCH 04/14] update comments for regex

---
 pkg/util/pamparse/pamparse.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pkg/util/pamparse/pamparse.go b/pkg/util/pamparse/pamparse.go
index 04745c2d4d..ecf59580b9 100644
--- a/pkg/util/pamparse/pamparse.go
+++ b/pkg/util/pamparse/pamparse.go
@@ -56,6 +56,7 @@ func ParseEnvironmentConfFile(path string) (map[string]string, error) {
 	return rtn, nil
 }
 
+// Regex to parse a line from /etc/environment. Follows the guidance from https://wiki.archlinux.org/title/Environment_variables
 var envFileLineRe = regexp.MustCompile(`^(?:export\s+)?([A-Z0-9_]+[A-Za-z0-9]*)=(.*)$`)
 
 func parseEnvironmentLine(line string) (string, string) {
@@ -66,6 +67,7 @@ func parseEnvironmentLine(line string) (string, string) {
 	return m[1], sanitizeEnvVarValue(m[2])
 }
 
+// Regex to parse a line from /etc/security/pam_env.conf or ~/.pam_environment. Follows the guidance from https://wiki.archlinux.org/title/Environment_variables
 var confFileLineRe = regexp.MustCompile(`^([A-Z0-9_]+[A-Za-z0-9]*)\s+(?:(?:DEFAULT=)([^\s]+(?: \w+)*))\s*(?:(?:OVERRIDE=)([^\s]+(?: \w+)*))?\s*$`)
 
 func parseEnvironmentConfLine(line string) (string, string) {

From 89d3869bae103b5c84596950c4ffaf0793ca92bf Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 16:41:42 -0800
Subject: [PATCH 05/14] add license identifier to test

---
 pkg/util/pamparse/pamparse_test.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pkg/util/pamparse/pamparse_test.go b/pkg/util/pamparse/pamparse_test.go
index 027ed4b2b7..223e5bdae3 100644
--- a/pkg/util/pamparse/pamparse_test.go
+++ b/pkg/util/pamparse/pamparse_test.go
@@ -1,3 +1,6 @@
+// Copyright 2025, Command Line Inc.
+// SPDX-License-Identifier: Apache-2.0
+
 package pamparse_test
 
 import (

From 1d1000b31cea7b6ae9ab28443961472dc52ac50b Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 16:49:54 -0800
Subject: [PATCH 06/14] only set the XDG vars from pam, ignore others

---
 pkg/shellexec/shellexec.go | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index 0b4f2794cf..b79744ba99 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -454,11 +454,18 @@ func StartShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOpt
 		ecmd.Env = os.Environ()
 	}
 
-	// For Snap installations, we need to unset XDG environment variables as Snap overrides them to point to snap directories
+	// For Snap installations, we need to unset XDG environment variables as Snap overrides them to point to snap directories. We then need to apply the PAM environment variables in case the true values are set there.
 	if os.Getenv("SNAP") != "" {
-		shellutil.UpdateCmdEnv(ecmd, map[string]string{"XDG_CONFIG_HOME": "", "XDG_DATA_HOME": "", "XDG_CACHE_HOME": "", "XDG_RUNTIME_DIR": "", "XDG_CONFIG_DIRS": "", "XDG_DATA_DIRS": ""})
+		varsToReplace := map[string]string{"XDG_CONFIG_HOME": "", "XDG_DATA_HOME": "", "XDG_CACHE_HOME": "", "XDG_RUNTIME_DIR": "", "XDG_CONFIG_DIRS": "", "XDG_DATA_DIRS": ""}
+		shellutil.UpdateCmdEnv(ecmd, varsToReplace)
 		pamEnvs := tryGetPamEnvVars()
 		if len(pamEnvs) > 0 {
+			// We only want to set the XDG variables from the PAM environment, all others should already be correct or may have been overridden by something else out of our control
+			for k := range pamEnvs {
+				if _, ok := varsToReplace[k]; !ok {
+					delete(pamEnvs, k)
+				}
+			}
 			shellutil.UpdateCmdEnv(ecmd, pamEnvs)
 		}
 	}

From 903365cccdd1c958c0b1b4c4e7b63e7ad68e09d6 Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 16:52:01 -0800
Subject: [PATCH 07/14] fix so we aren't doing more work than we need to

---
 pkg/shellexec/shellexec.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index b79744ba99..67178d4092 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -454,19 +454,19 @@ func StartShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOpt
 		ecmd.Env = os.Environ()
 	}
 
-	// For Snap installations, we need to unset XDG environment variables as Snap overrides them to point to snap directories. We then need to apply the PAM environment variables in case the true values are set there.
+	// For Snap installations, we need to correct the XDG environment variables as Snap overrides them to point to snap directories. We will get the correct values, if set, from the PAM environment. If the XDG variables are set in
+	// profile or in an RC file, it will be overridden when the shell initializes.
 	if os.Getenv("SNAP") != "" {
 		varsToReplace := map[string]string{"XDG_CONFIG_HOME": "", "XDG_DATA_HOME": "", "XDG_CACHE_HOME": "", "XDG_RUNTIME_DIR": "", "XDG_CONFIG_DIRS": "", "XDG_DATA_DIRS": ""}
-		shellutil.UpdateCmdEnv(ecmd, varsToReplace)
 		pamEnvs := tryGetPamEnvVars()
 		if len(pamEnvs) > 0 {
 			// We only want to set the XDG variables from the PAM environment, all others should already be correct or may have been overridden by something else out of our control
 			for k := range pamEnvs {
-				if _, ok := varsToReplace[k]; !ok {
-					delete(pamEnvs, k)
+				if _, ok := varsToReplace[k]; ok {
+					varsToReplace[k] = pamEnvs[k]
 				}
 			}
-			shellutil.UpdateCmdEnv(ecmd, pamEnvs)
+			shellutil.UpdateCmdEnv(ecmd, varsToReplace)
 		}
 	}
 

From 797cc0d52caaf8217f747c047de0f7977190cfe7 Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 16:52:15 -0800
Subject: [PATCH 08/14] one more fix

---
 pkg/shellexec/shellexec.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index 67178d4092..9d2b67df92 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -466,8 +466,8 @@ func StartShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOpt
 					varsToReplace[k] = pamEnvs[k]
 				}
 			}
-			shellutil.UpdateCmdEnv(ecmd, varsToReplace)
 		}
+		shellutil.UpdateCmdEnv(ecmd, varsToReplace)
 	}
 
 	if cmdOpts.Cwd != "" {

From 93257a576d28fa219ed0e7a78f93c652713f6496 Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 17:14:52 -0800
Subject: [PATCH 09/14] add logs

---
 pkg/shellexec/shellexec.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index 9d2b67df92..6a2bcefadd 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -459,14 +459,17 @@ func StartShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOpt
 	if os.Getenv("SNAP") != "" {
 		varsToReplace := map[string]string{"XDG_CONFIG_HOME": "", "XDG_DATA_HOME": "", "XDG_CACHE_HOME": "", "XDG_RUNTIME_DIR": "", "XDG_CONFIG_DIRS": "", "XDG_DATA_DIRS": ""}
 		pamEnvs := tryGetPamEnvVars()
+		log.Printf("PAM environment: %v", pamEnvs)
 		if len(pamEnvs) > 0 {
 			// We only want to set the XDG variables from the PAM environment, all others should already be correct or may have been overridden by something else out of our control
 			for k := range pamEnvs {
 				if _, ok := varsToReplace[k]; ok {
+					log.Printf("Setting %s to %s", k, pamEnvs[k])
 					varsToReplace[k] = pamEnvs[k]
 				}
 			}
 		}
+		log.Printf("Replacing XDG environment variables: %v", varsToReplace)
 		shellutil.UpdateCmdEnv(ecmd, varsToReplace)
 	}
 

From a1425051d5ccd052775cc446d7994c0448296f0d Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 17:29:53 -0800
Subject: [PATCH 10/14] replace home and shell with vals from /etc/passwd

---
 pkg/shellexec/shellexec.go    |  2 +-
 pkg/util/pamparse/pamparse.go | 37 ++++++++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index 6a2bcefadd..8b51ac27b0 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -547,7 +547,7 @@ func tryGetPamEnvVars() map[string]string {
 	if err != nil {
 		log.Printf("error parsing %s: %v", etcSecurityPath, err)
 	}
-	envVars3, err := pamparse.ParseEnvironmentConfFile(userEnvironmentPath)
+	envVars3, err := pamparse.ParseEnvironmentConfFile(wavebase.ExpandHomeDirSafe(userEnvironmentPath))
 	if err != nil {
 		log.Printf("error parsing %s: %v", userEnvironmentPath, err)
 	}
diff --git a/pkg/util/pamparse/pamparse.go b/pkg/util/pamparse/pamparse.go
index ecf59580b9..2f7ae51d2b 100644
--- a/pkg/util/pamparse/pamparse.go
+++ b/pkg/util/pamparse/pamparse.go
@@ -6,6 +6,7 @@ package pamparse
 
 import (
 	"bufio"
+	"fmt"
 	"os"
 	"regexp"
 	"strings"
@@ -39,6 +40,10 @@ func ParseEnvironmentConfFile(path string) (map[string]string, error) {
 		return nil, err
 	}
 	defer file.Close()
+	home, shell, err := parsePasswd()
+	if err != nil {
+		return nil, err
+	}
 	scanner := bufio.NewScanner(file)
 	for scanner.Scan() {
 		line := scanner.Text()
@@ -51,12 +56,38 @@ func ParseEnvironmentConfFile(path string) (map[string]string, error) {
 				continue
 			}
 		}
-		rtn[key] = val
+		rtn[key] = replaceHomeAndShell(val, home, shell)
 	}
 	return rtn, nil
 }
 
-// Regex to parse a line from /etc/environment. Follows the guidance from https://wiki.archlinux.org/title/Environment_variables
+// Gets the home directory and shell from /etc/passwd for the current user.
+func parsePasswd() (string, string, error) {
+	file, err := os.OpenFile("/etc/passwd", os.O_RDONLY, 0)
+	if err != nil {
+		return "", "", err
+	}
+	defer file.Close()
+	userPrefix := fmt.Sprintf("%s:", os.Getenv("USER"))
+	scanner := bufio.NewScanner(file)
+	for scanner.Scan() {
+		line := scanner.Text()
+		if strings.HasPrefix(line, userPrefix) {
+			parts := strings.Split(line, ":")
+			return parts[5], parts[6], nil
+		}
+	}
+	return "", "", nil
+}
+
+// Replaces @{HOME} and @{SHELL} placeholders in a string with the provided values. Follows guidance from https://wiki.archlinux.org/title/Environment_variables#Using_pam_env
+func replaceHomeAndShell(val string, home string, shell string) string {
+	val = strings.ReplaceAll(val, "@{HOME}", home)
+	val = strings.ReplaceAll(val, "@{SHELL}", shell)
+	return val
+}
+
+// Regex to parse a line from /etc/environment. Follows the guidance from https://wiki.archlinux.org/title/Environment_variables#Using_pam_env
 var envFileLineRe = regexp.MustCompile(`^(?:export\s+)?([A-Z0-9_]+[A-Za-z0-9]*)=(.*)$`)
 
 func parseEnvironmentLine(line string) (string, string) {
@@ -67,7 +98,7 @@ func parseEnvironmentLine(line string) (string, string) {
 	return m[1], sanitizeEnvVarValue(m[2])
 }
 
-// Regex to parse a line from /etc/security/pam_env.conf or ~/.pam_environment. Follows the guidance from https://wiki.archlinux.org/title/Environment_variables
+// Regex to parse a line from /etc/security/pam_env.conf or ~/.pam_environment. Follows the guidance from https://wiki.archlinux.org/title/Environment_variables#Using_pam_env
 var confFileLineRe = regexp.MustCompile(`^([A-Z0-9_]+[A-Za-z0-9]*)\s+(?:(?:DEFAULT=)([^\s]+(?: \w+)*))\s*(?:(?:OVERRIDE=)([^\s]+(?: \w+)*))?\s*$`)
 
 func parseEnvironmentConfLine(line string) (string, string) {

From 5bc4a34a79fdb30b136b168499e4bf798404375d Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 17:36:40 -0800
Subject: [PATCH 11/14] apply coderabbit suggestion

---
 pkg/shellexec/shellexec.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index 8b51ac27b0..261f8386bc 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -538,6 +538,11 @@ const etcEnvironmentPath = "/etc/environment"
 const etcSecurityPath = "/etc/security/pam_env.conf"
 const userEnvironmentPath = "~/.pam_environment"
 
+// tryGetPamEnvVars tries to get the environment variables from /etc/environment, /etc/security/pam_env.conf, and ~/.pam_environment.
+// It then returns a map of the environment variables, overriding duplicates with the following order of precedence:
+// 1. /etc/environment
+// 2. /etc/security/pam_env.conf
+// 3. ~/.pam_environment
 func tryGetPamEnvVars() map[string]string {
 	envVars, err := pamparse.ParseEnvironmentFile(etcEnvironmentPath)
 	if err != nil {

From 7fe0499f5e2d0ead494b208470b5f2396ee5984b Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 17:50:31 -0800
Subject: [PATCH 12/14] Update pkg/util/pamparse/pamparse.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
---
 pkg/util/pamparse/pamparse.go | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/pkg/util/pamparse/pamparse.go b/pkg/util/pamparse/pamparse.go
index 2f7ae51d2b..e79531518a 100644
--- a/pkg/util/pamparse/pamparse.go
+++ b/pkg/util/pamparse/pamparse.go
@@ -74,9 +74,15 @@ func parsePasswd() (string, string, error) {
 		line := scanner.Text()
 		if strings.HasPrefix(line, userPrefix) {
 			parts := strings.Split(line, ":")
+			if len(parts) < 7 {
+				return "", "", fmt.Errorf("invalid passwd entry: insufficient fields")
+			}
 			return parts[5], parts[6], nil
 		}
 	}
+	if err := scanner.Err(); err != nil {
+		return "", "", fmt.Errorf("error reading passwd file: %w", err)
+	}
 	return "", "", nil
 }
 

From 534958100265704b1d9da65fc69d203f27b78b7d Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 17:55:19 -0800
Subject: [PATCH 13/14] apply some coderabbit suggestions

---
 pkg/shellexec/shellexec.go    | 23 ++++++++++++++++-------
 pkg/util/pamparse/pamparse.go |  6 +++---
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index 261f8386bc..bfd19fb768 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -454,8 +454,12 @@ func StartShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOpt
 		ecmd.Env = os.Environ()
 	}
 
-	// For Snap installations, we need to correct the XDG environment variables as Snap overrides them to point to snap directories. We will get the correct values, if set, from the PAM environment. If the XDG variables are set in
-	// profile or in an RC file, it will be overridden when the shell initializes.
+	/*
+	 * For Snap installations, we need to correct the XDG environment variables as Snap
+	 * overrides them to point to snap directories. We will get the correct values, if
+	 * set, from the PAM environment. If the XDG variables are set in profile or in an
+	 * RC file, it will be overridden when the shell initializes.
+	 */
 	if os.Getenv("SNAP") != "" {
 		varsToReplace := map[string]string{"XDG_CONFIG_HOME": "", "XDG_DATA_HOME": "", "XDG_CACHE_HOME": "", "XDG_RUNTIME_DIR": "", "XDG_CONFIG_DIRS": "", "XDG_DATA_DIRS": ""}
 		pamEnvs := tryGetPamEnvVars()
@@ -538,11 +542,16 @@ const etcEnvironmentPath = "/etc/environment"
 const etcSecurityPath = "/etc/security/pam_env.conf"
 const userEnvironmentPath = "~/.pam_environment"
 
-// tryGetPamEnvVars tries to get the environment variables from /etc/environment, /etc/security/pam_env.conf, and ~/.pam_environment.
-// It then returns a map of the environment variables, overriding duplicates with the following order of precedence:
-// 1. /etc/environment
-// 2. /etc/security/pam_env.conf
-// 3. ~/.pam_environment
+/*
+ * tryGetPamEnvVars tries to get the environment variables from /etc/environment,
+ * /etc/security/pam_env.conf, and ~/.pam_environment.
+ *
+ * It then returns a map of the environment variables, overriding duplicates with
+ * the following order of precedence:
+ * 1. /etc/environment
+ * 2. /etc/security/pam_env.conf
+ * 3. ~/.pam_environment
+ */
 func tryGetPamEnvVars() map[string]string {
 	envVars, err := pamparse.ParseEnvironmentFile(etcEnvironmentPath)
 	if err != nil {
diff --git a/pkg/util/pamparse/pamparse.go b/pkg/util/pamparse/pamparse.go
index e79531518a..950b6dbe30 100644
--- a/pkg/util/pamparse/pamparse.go
+++ b/pkg/util/pamparse/pamparse.go
@@ -15,7 +15,7 @@ import (
 // Parses a file in the format of /etc/environment. Accepts a path to the file and returns a map of environment variables.
 func ParseEnvironmentFile(path string) (map[string]string, error) {
 	rtn := make(map[string]string)
-	file, err := os.OpenFile(path, os.O_RDONLY, 0)
+	file, err := os.Open(path)
 	if err != nil {
 		return nil, err
 	}
@@ -35,7 +35,7 @@ func ParseEnvironmentFile(path string) (map[string]string, error) {
 // Parses a file in the format of /etc/security/pam_env.conf or ~/.pam_environment. Accepts a path to the file and returns a map of environment variables.
 func ParseEnvironmentConfFile(path string) (map[string]string, error) {
 	rtn := make(map[string]string)
-	file, err := os.OpenFile(path, os.O_RDONLY, 0)
+	file, err := os.Open(path)
 	if err != nil {
 		return nil, err
 	}
@@ -63,7 +63,7 @@ func ParseEnvironmentConfFile(path string) (map[string]string, error) {
 
 // Gets the home directory and shell from /etc/passwd for the current user.
 func parsePasswd() (string, string, error) {
-	file, err := os.OpenFile("/etc/passwd", os.O_RDONLY, 0)
+	file, err := os.Open("/etc/passwd")
 	if err != nil {
 		return "", "", err
 	}

From dbaa77239cbe47e200cc962795f9a9ff0118bdd5 Mon Sep 17 00:00:00 2001
From: Evan Simkowitz <esimkowitz@users.noreply.github.com>
Date: Thu, 9 Jan 2025 17:58:37 -0800
Subject: [PATCH 14/14] remove unnecessary *

---
 pkg/shellexec/shellexec.go | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go
index bfd19fb768..2f374f814c 100644
--- a/pkg/shellexec/shellexec.go
+++ b/pkg/shellexec/shellexec.go
@@ -455,11 +455,11 @@ func StartShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOpt
 	}
 
 	/*
-	 * For Snap installations, we need to correct the XDG environment variables as Snap
-	 * overrides them to point to snap directories. We will get the correct values, if
-	 * set, from the PAM environment. If the XDG variables are set in profile or in an
-	 * RC file, it will be overridden when the shell initializes.
-	 */
+	  For Snap installations, we need to correct the XDG environment variables as Snap
+	  overrides them to point to snap directories. We will get the correct values, if
+	  set, from the PAM environment. If the XDG variables are set in profile or in an
+	  RC file, it will be overridden when the shell initializes.
+	*/
 	if os.Getenv("SNAP") != "" {
 		varsToReplace := map[string]string{"XDG_CONFIG_HOME": "", "XDG_DATA_HOME": "", "XDG_CACHE_HOME": "", "XDG_RUNTIME_DIR": "", "XDG_CONFIG_DIRS": "", "XDG_DATA_DIRS": ""}
 		pamEnvs := tryGetPamEnvVars()
@@ -543,15 +543,15 @@ const etcSecurityPath = "/etc/security/pam_env.conf"
 const userEnvironmentPath = "~/.pam_environment"
 
 /*
- * tryGetPamEnvVars tries to get the environment variables from /etc/environment,
- * /etc/security/pam_env.conf, and ~/.pam_environment.
- *
- * It then returns a map of the environment variables, overriding duplicates with
- * the following order of precedence:
- * 1. /etc/environment
- * 2. /etc/security/pam_env.conf
- * 3. ~/.pam_environment
- */
+tryGetPamEnvVars tries to get the environment variables from /etc/environment,
+/etc/security/pam_env.conf, and ~/.pam_environment.
+
+It then returns a map of the environment variables, overriding duplicates with
+the following order of precedence:
+1. /etc/environment
+2. /etc/security/pam_env.conf
+3. ~/.pam_environment
+*/
 func tryGetPamEnvVars() map[string]string {
 	envVars, err := pamparse.ParseEnvironmentFile(etcEnvironmentPath)
 	if err != nil {