fix(security): strip PM_CLI_BRIDGE_PASSWORD from child process env

CHANGELOG.md

@@ -30,6 +30,11 @@ Targeted for the 0.2.6 release.
 - The COPY no-match check is gated on the server advertising UIDPLUS, since COPYUID is
   only guaranteed there; without the guard every successful copy on a non-UIDPLUS
   server would have been reported as a failure (#17).
+- `mail watch --exec` no longer passes `PM_CLI_BRIDGE_PASSWORD` to the command it
+  runs. The child environment was built from `os.Environ()`, so a Bridge password
+  supplied via the environment variable added in this release was inherited by the
+  user-supplied command and by anything it shelled out to. Child environments are now
+  built through `config.ScrubSecrets`.
 
 ### Changed
 - `mail list --unread` now returns up to `--limit` unread messages. Previously the

SECURITY.md

@@ -28,6 +28,24 @@ If you discover a security vulnerability in pm-cli, please report it responsibly
 - Config file permissions are set to `0600` (owner read/write only)
 - Config directory permissions are set to `0700`
 
+#### Environment variable credentials
+
+`PM_CLI_BRIDGE_PASSWORD` supplies the Bridge password directly, for headless
+environments with no secret service available. When set and non-empty it takes
+precedence over the keyring.
+
+This trades some protection for portability, and the tradeoff should be a
+deliberate choice:
+
+- The value is readable by any process running as the same user, and appears in
+  `/proc/<pid>/environ` on Linux.
+- It may be captured by shell history or process listings depending on how it is set.
+- pm-cli removes it from the environment of child processes it spawns (see
+  `config.ScrubSecrets`), so `mail watch --exec` commands do not inherit it.
+
+Prefer the system keyring on interactive machines. Where the variable is required,
+inject it from a secrets manager rather than a shell profile.
+
 ### Network Security
 
 - All connections to Proton Bridge use TLS encryption

internal/cli/mail.go

@@ -1943,7 +1943,11 @@ func (c *MailWatchCmd) executeCommand(ctx *Context, msg imap.MessageSummary) {
 	ctx.Formatter.Verbosef("Executing: %s", cmdStr)
 
 	cmd := exec.Command("sh", "-c", cmdStr)
-	cmd.Env = append(os.Environ(),
+	// Build the child environment from ScrubSecrets, never os.Environ()
+	// directly: a Bridge password supplied via PM_CLI_BRIDGE_PASSWORD would
+	// otherwise be inherited by this user-supplied command and by everything
+	// it shells out to.
+	cmd.Env = append(config.ScrubSecrets(os.Environ()),
 		fmt.Sprintf("PM_MSG_SEQ=%d", msg.SeqNum),
 		fmt.Sprintf("PM_MSG_UID=%d", msg.UID),
 		"PM_MSG_FROM="+safetext.SanitizeHeaderValue(msg.From),

internal/config/config.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/zalando/go-keyring"
@@ -163,6 +164,46 @@ func DeletePassword(email string) error {
 	return keyring.Delete(AppName, email)
 }
 
+// secretEnvVars lists environment variables that carry pm-cli credentials and
+// must never be handed to a child process.
+var secretEnvVars = []string{EnvBridgePassword}
+
+// ScrubSecrets returns env with every pm-cli credential variable removed.
+//
+// Any code spawning a child process must build its environment from this
+// rather than from os.Environ() directly. Without it, a Bridge password
+// supplied via EnvBridgePassword is inherited by user-supplied commands (for
+// example `mail watch --exec`), handing the mail credential to arbitrary
+// third-party scripts.
+//
+// The input slice is not modified.
+func ScrubSecrets(env []string) []string {
+	out := make([]string, 0, len(env))
+	for _, kv := range env {
+		if isSecretEnv(kv) {
+			continue
+		}
+		out = append(out, kv)
+	}
+	return out
+}
+
+// isSecretEnv reports whether a "KEY=VALUE" entry names a credential variable.
+// A bare "KEY" with no "=" is matched too, since Go permits such entries and
+// dropping them is the safe direction.
+func isSecretEnv(kv string) bool {
+	name := kv
+	if i := strings.IndexByte(kv, '='); i >= 0 {
+		name = kv[:i]
+	}
+	for _, secret := range secretEnvVars {
+		if name == secret {
+			return true
+		}
+	}
+	return false
+}
+
 func Exists() bool {
 	path, err := ConfigPath()
 	if err != nil {

internal/config/scrub_secrets_test.go

@@ -0,0 +1,120 @@
+package config
+
+import (
+	"os"
+	"os/exec"
+	"strings"
+	"testing"
+)
+
+func TestScrubSecretsRemovesBridgePassword(t *testing.T) {
+	env := []string{
+		"PATH=/usr/bin",
+		EnvBridgePassword + "=super-secret",
+		"HOME=/home/user",
+	}
+
+	got := ScrubSecrets(env)
+
+	for _, kv := range got {
+		if strings.HasPrefix(kv, EnvBridgePassword+"=") {
+			t.Fatalf("ScrubSecrets left the credential in place: %q", kv)
+		}
+		if strings.Contains(kv, "super-secret") {
+			t.Fatalf("ScrubSecrets leaked the secret value: %q", kv)
+		}
+	}
+	if len(got) != 2 {
+		t.Errorf("expected 2 surviving entries, got %d: %v", len(got), got)
+	}
+}
+
+func TestScrubSecretsKeepsUnrelatedVars(t *testing.T) {
+	env := []string{"PATH=/usr/bin", "PM_MSG_FROM=a@b.test", "LANG=C"}
+
+	got := ScrubSecrets(env)
+
+	if len(got) != len(env) {
+		t.Fatalf("unrelated vars were dropped: %v", got)
+	}
+	for i := range env {
+		if got[i] != env[i] {
+			t.Errorf("entry %d changed: got %q, want %q", i, got[i], env[i])
+		}
+	}
+}
+
+// TestScrubSecretsDoesNotMatchPrefixes guards against dropping variables that
+// merely start with the secret's name.
+func TestScrubSecretsDoesNotMatchPrefixes(t *testing.T) {
+	similar := EnvBridgePassword + "_BACKUP=keep-me"
+	got := ScrubSecrets([]string{similar})
+
+	if len(got) != 1 || got[0] != similar {
+		t.Errorf("a similarly-named variable was dropped: %v", got)
+	}
+}
+
+// TestScrubSecretsBareName covers an entry with no "=", which Go permits.
+func TestScrubSecretsBareName(t *testing.T) {
+	got := ScrubSecrets([]string{EnvBridgePassword, "PATH=/usr/bin"})
+
+	for _, kv := range got {
+		if kv == EnvBridgePassword {
+			t.Error("bare credential name survived scrubbing")
+		}
+	}
+}
+
+func TestScrubSecretsDoesNotMutateInput(t *testing.T) {
+	env := []string{"PATH=/usr/bin", EnvBridgePassword + "=secret"}
+	_ = ScrubSecrets(env)
+
+	if len(env) != 2 || env[1] != EnvBridgePassword+"=secret" {
+		t.Errorf("input slice was modified: %v", env)
+	}
+}
+
+func TestScrubSecretsEmpty(t *testing.T) {
+	if got := ScrubSecrets(nil); len(got) != 0 {
+		t.Errorf("expected empty result, got %v", got)
+	}
+}
+
+// TestScrubbedEnvironNotVisibleToChild is the end-to-end guard for the actual
+// leak: a real child process must not be able to read the Bridge password.
+func TestScrubbedEnvironNotVisibleToChild(t *testing.T) {
+	t.Setenv(EnvBridgePassword, "bridge-secret-value")
+
+	// Mirrors MailWatchCmd.executeCommand's environment construction.
+	cmd := exec.Command("sh", "-c", `printf '%s' "$`+EnvBridgePassword+`"`)
+	cmd.Env = append(ScrubSecrets(os.Environ()), "PM_MSG_SEQ=1")
+
+	out, err := cmd.Output()
+	if err != nil {
+		t.Fatalf("child failed: %v", err)
+	}
+	if len(out) != 0 {
+		t.Errorf("child could read the Bridge password: %q", out)
+	}
+}
+
+// TestUnscrubbedEnvironWouldLeak pins the regression this fix prevents: using
+// os.Environ() directly does expose the credential. If this ever stops
+// leaking, the scrubbing above is no longer load-bearing and the test should
+// be revisited rather than deleted.
+func TestUnscrubbedEnvironWouldLeak(t *testing.T) {
+	t.Setenv(EnvBridgePassword, "bridge-secret-value")
+
+	cmd := exec.Command("sh", "-c", `printf '%s' "$`+EnvBridgePassword+`"`)
+	cmd.Env = os.Environ()
+
+	out, err := cmd.Output()
+	if err != nil {
+		t.Fatalf("child failed: %v", err)
+	}
+	if string(out) != "bridge-secret-value" {
+		t.Skipf("environment did not propagate as expected (got %q); "+
+			"the scrubbed-child test above is the load-bearing assertion", out)
+	}
+}