fix(windows): reject malformed process environments (#3500)

* fix(windows): reject malformed process environments

refs #3430

* fix(windows): validate registry environment strings

refs #3430

* fix(windows): preserve hidden environment variables

refs #3430

3 files changed, 405 insertions, 32 deletions

diff --git a/vendor/patches/portable-pty/0003-reject-malformed-windows-environments.patch b/vendor/patches/portable-pty/0003-reject-malformed-windows-environments.patch
new file mode 100644
index 00000000..d2f7c4ae
--- /dev/null
+++ b/vendor/patches/portable-pty/0003-reject-malformed-windows-environments.patch
@@ -0,0 +1,220 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: herdr maintainers <maintainers@herdr.dev>
+Date: Tue, 1 Sep 2026 00:00:00 +0000
+Subject: [PATCH] reject malformed Windows process environments
+
+Windows registry environment keys can contain empty names, non-string types,
+or malformed UTF-16. Decode string values into aligned storage, check Win32
+expansion failures, preserve valid drive variables, and omit entries that
+cannot form a valid environment record before calling CreateProcessW.
+
+Herdr issue: https://github.com/herdrdev/herdr/issues/3430
+Vendored base: portable-pty 0.9.0
+---
+diff --git a/vendor/portable-pty/src/cmdbuilder.rs b/vendor/portable-pty/src/cmdbuilder.rs
+index 385fe81c..b214bb6d 100644
+--- a/vendor/portable-pty/src/cmdbuilder.rs
++++ b/vendor/portable-pty/src/cmdbuilder.rs
+@@ -71,6 +71,70 @@ fn get_shell() -> String {
+     "/bin/sh".into()
+ }
+ 
++#[cfg(windows)]
++fn reg_value_to_string(value: &winreg::RegValue) -> anyhow::Result<OsString> {
++    use std::os::windows::ffi::OsStringExt;
++    use winapi::um::processenv::ExpandEnvironmentStringsW;
++    use winreg::enums::RegType;
++
++    if !matches!(value.vtype, RegType::REG_SZ | RegType::REG_EXPAND_SZ) {
++        anyhow::bail!(
++            "unsupported registry type {:?} for environment variable",
++            value.vtype
++        );
++    }
++
++    let mut bytes = value.bytes.chunks_exact(2);
++    let mut units: Vec<u16> = bytes
++        .by_ref()
++        .map(|pair| u16::from_le_bytes([pair[0], pair[1]]))
++        .collect();
++    anyhow::ensure!(
++        bytes.remainder().is_empty(),
++        "invalid UTF-16 byte length for environment variable"
++    );
++    while let Some(0) = units.last() {
++        units.pop();
++    }
++    anyhow::ensure!(
++        !units.contains(&0),
++        "embedded null in registry environment variable"
++    );
++
++    if value.vtype == RegType::REG_SZ {
++        return Ok(OsString::from_wide(&units));
++    }
++
++    units.push(0);
++    let size = unsafe { ExpandEnvironmentStringsW(units.as_ptr(), std::ptr::null_mut(), 0) };
++    if size == 0 {
++        anyhow::bail!(
++            "ExpandEnvironmentStringsW failed: {}",
++            std::io::Error::last_os_error()
++        );
++    }
++    let mut buf = vec![0u16; size as usize];
++    let written = unsafe {
++        ExpandEnvironmentStringsW(units.as_ptr(), buf.as_mut_ptr(), buf.len() as u32)
++    };
++    if written == 0 {
++        anyhow::bail!(
++            "ExpandEnvironmentStringsW failed: {}",
++            std::io::Error::last_os_error()
++        );
++    }
++    anyhow::ensure!(
++        written as usize <= buf.len(),
++        "environment changed while expanding registry value"
++    );
++
++    buf.truncate(written as usize);
++    while let Some(0) = buf.last() {
++        buf.pop();
++    }
++    Ok(OsString::from_wide(&buf))
++}
++
+ fn get_base_env() -> BTreeMap<OsString, EnvEntry> {
+     let mut env: BTreeMap<OsString, EnvEntry> = std::env::vars_os()
+         .map(|(key, value)| {
+@@ -103,37 +167,8 @@ fn get_base_env() -> BTreeMap<OsString, EnvEntry> {
+ 
+     #[cfg(windows)]
+     {
+-        use std::os::windows::ffi::OsStringExt;
+-        use winapi::um::processenv::ExpandEnvironmentStringsW;
+-        use winreg::enums::{RegType, HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE};
+-        use winreg::types::FromRegValue;
+-        use winreg::{RegKey, RegValue};
+-
+-        fn reg_value_to_string(value: &RegValue) -> anyhow::Result<OsString> {
+-            match value.vtype {
+-                RegType::REG_EXPAND_SZ => {
+-                    let src = unsafe {
+-                        std::slice::from_raw_parts(
+-                            value.bytes.as_ptr() as *const u16,
+-                            value.bytes.len() / 2,
+-                        )
+-                    };
+-                    let size =
+-                        unsafe { ExpandEnvironmentStringsW(src.as_ptr(), std::ptr::null_mut(), 0) };
+-                    let mut buf = vec![0u16; size as usize + 1];
+-                    unsafe {
+-                        ExpandEnvironmentStringsW(src.as_ptr(), buf.as_mut_ptr(), buf.len() as u32)
+-                    };
+-
+-                    let mut buf = buf.as_slice();
+-                    while let Some(0) = buf.last() {
+-                        buf = &buf[0..buf.len() - 1];
+-                    }
+-                    Ok(OsString::from_wide(buf))
+-                }
+-                _ => Ok(OsString::from_reg_value(value)?),
+-            }
+-        }
++        use winreg::enums::{HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE};
++        use winreg::RegKey;
+ 
+         if let Ok(sys_env) = RegKey::predef(HKEY_LOCAL_MACHINE)
+             .open_subkey("System\\CurrentControlSet\\Control\\Session Manager\\Environment")
+@@ -662,12 +697,24 @@ impl CommandBuilder {
+             value,
+         } in self.envs.values()
+         {
+-            block.extend(preferred_key.encode_wide());
++            let key: Vec<u16> = preferred_key.encode_wide().collect();
++            if key.is_empty()
++                || key.contains(&0)
++                || key[1..].contains(&(b'=' as u16))
++                || value.encode_wide().any(|unit| unit == 0)
++            {
++                continue;
++            }
++
++            block.extend(key);
+             block.push(b'=' as u16);
+             block.extend(value.encode_wide());
+             block.push(0);
+         }
+         // and a final terminator for CreateProcessW
++        if block.is_empty() {
++            block.push(0);
++        }
+         block.push(0);
+ 
+         block
+@@ -869,4 +916,65 @@ mod tests {
+         assert!(command.ends_with(r#"/d /c echo "hi""#), "{}", command);
+         assert!(!command.contains(r#"\"hi\""#), "{}", command);
+     }
++
++    #[cfg(windows)]
++    #[test]
++    fn windows_environment_rejects_malformed_entries() {
++        use std::os::windows::ffi::{OsStrExt, OsStringExt};
++        use winreg::enums::RegType;
++        use winreg::RegValue;
++
++        let multi_string = RegValue {
++            bytes: vec![b'a', 0, 0, 0],
++            vtype: RegType::REG_MULTI_SZ,
++        };
++        assert!(reg_value_to_string(&multi_string).is_err());
++        let malformed_expand_string = RegValue {
++            bytes: vec![b'%', 0, b'P'],
++            vtype: RegType::REG_EXPAND_SZ,
++        };
++        assert!(reg_value_to_string(&malformed_expand_string).is_err());
++        let expandable_string = RegValue {
++            bytes: "literal\0"
++                .encode_utf16()
++                .flat_map(u16::to_le_bytes)
++                .collect(),
++            vtype: RegType::REG_EXPAND_SZ,
++        };
++        assert_eq!(
++            reg_value_to_string(&expandable_string).unwrap(),
++            OsString::from("literal")
++        );
++
++        let mut cmd = CommandBuilder::new("dummy");
++        cmd.env_clear();
++        cmd.env("", "ignored");
++        cmd.env(
++            OsString::from_wide(&[b'B' as u16, 0, b'A' as u16, b'D' as u16]),
++            "ignored",
++        );
++        cmd.env(
++            "MULTI",
++            OsString::from_wide(&[b'a' as u16, 0, b'b' as u16]),
++        );
++        cmd.env("BAD=KEY", "ignored");
++        cmd.env("=::", "colon");
++        cmd.env("=C:", r"C:\valid");
++        cmd.env("=ExitCode", "hidden");
++        cmd.env("EMPTY", "");
++        cmd.env("VALID", "ok");
++
++        let expected: Vec<u16> =
++            OsStr::new("=::=colon\0=C:=C:\\valid\0=ExitCode=hidden\0EMPTY=\0VALID=ok\0\0")
++                .encode_wide()
++                .collect();
++        assert_eq!(cmd.environment_block(), expected);
++
++        cmd.env_remove("=::");
++        cmd.env_remove("=C:");
++        cmd.env_remove("=ExitCode");
++        cmd.env_remove("EMPTY");
++        cmd.env_remove("VALID");
++        assert_eq!(cmd.environment_block(), vec![0, 0]);
++    }
+ }
diff --git a/vendor/portable-pty.patches.md b/vendor/portable-pty.patches.md
index 8ed47a65..14eae869 100644
--- a/vendor/portable-pty.patches.md
+++ b/vendor/portable-pty.patches.md
@@ -84,3 +84,48 @@ python3 -m unittest scripts.test_vendor_portable_pty
 ```
 
 On Windows, also run `cargo test raw_arg_appends_unescaped_windows_command_tail`.
+
+## 0003 reject malformed Windows environments
+
+status: active
+
+patch: `vendor/patches/portable-pty/0003-reject-malformed-windows-environments.patch`
+
+herdr issue: https://github.com/herdrdev/herdr/issues/3430
+
+upstream discussions:
+
+- https://github.com/wezterm/wezterm/issues/4364
+- https://github.com/warpdotdev/warp/commit/2992d02e3e38af697c83a3bd6f20003f54ffe066
+
+upstream pr: none
+
+vendored base: `portable-pty 0.9.0`
+
+local files:
+
+- `vendor/portable-pty/src/cmdbuilder.rs`
+
+reason: Windows process environments may contain a registry value with an
+empty name or a non-string type. `winreg 0.10` converts `REG_MULTI_SZ` to an
+`OsString` that can contain embedded nulls. `portable-pty` serialized those
+values unchanged, producing an invalid environment block that makes
+`CreateProcessW` fail with `ERROR_INVALID_PARAMETER` (87). Import only Windows
+environment string types and omit entries that cannot form one complete
+`name=value\0` record.
+
+remove when: upstream `portable-pty` both imports only valid Windows environment
+string types and prevents malformed names or values from corrupting the process
+environment block, or Herdr replaces this launch path.
+
+verification:
+
+```sh
+python3 -m unittest scripts.test_vendor_portable_pty
+```
+
+On Windows, also run:
+
+```sh
+cargo test --manifest-path vendor/portable-pty/Cargo.toml windows_environment_rejects_malformed_entries
+```
diff --git a/vendor/portable-pty/src/cmdbuilder.rs b/vendor/portable-pty/src/cmdbuilder.rs
index 385fe81c..a9395f41 100644
--- a/vendor/portable-pty/src/cmdbuilder.rs
+++ b/vendor/portable-pty/src/cmdbuilder.rs
@@ -71,6 +71,70 @@ fn get_shell() -> String {
     "/bin/sh".into()
 }
 
+#[cfg(windows)]
+fn reg_value_to_string(value: &winreg::RegValue) -> anyhow::Result<OsString> {
+    use std::os::windows::ffi::OsStringExt;
+    use winapi::um::processenv::ExpandEnvironmentStringsW;
+    use winreg::enums::RegType;
+
+    if !matches!(value.vtype, RegType::REG_SZ | RegType::REG_EXPAND_SZ) {
+        anyhow::bail!(
+            "unsupported registry type {:?} for environment variable",
+            value.vtype
+        );
+    }
+
+    let mut bytes = value.bytes.chunks_exact(2);
+    let mut units: Vec<u16> = bytes
+        .by_ref()
+        .map(|pair| u16::from_le_bytes([pair[0], pair[1]]))
+        .collect();
+    anyhow::ensure!(
+        bytes.remainder().is_empty(),
+        "invalid UTF-16 byte length for environment variable"
+    );
+    while let Some(0) = units.last() {
+        units.pop();
+    }
+    anyhow::ensure!(
+        !units.contains(&0),
+        "embedded null in registry environment variable"
+    );
+
+    if value.vtype == RegType::REG_SZ {
+        return Ok(OsString::from_wide(&units));
+    }
+
+    units.push(0);
+    let size = unsafe { ExpandEnvironmentStringsW(units.as_ptr(), std::ptr::null_mut(), 0) };
+    if size == 0 {
+        anyhow::bail!(
+            "ExpandEnvironmentStringsW failed: {}",
+            std::io::Error::last_os_error()
+        );
+    }
+    let mut buf = vec![0u16; size as usize];
+    let written = unsafe {
+        ExpandEnvironmentStringsW(units.as_ptr(), buf.as_mut_ptr(), buf.len() as u32)
+    };
+    if written == 0 {
+        anyhow::bail!(
+            "ExpandEnvironmentStringsW failed: {}",
+            std::io::Error::last_os_error()
+        );
+    }
+    anyhow::ensure!(
+        written as usize <= buf.len(),
+        "environment changed while expanding registry value"
+    );
+
+    buf.truncate(written as usize);
+    while let Some(0) = buf.last() {
+        buf.pop();
+    }
+    Ok(OsString::from_wide(&buf))
+}
+
 fn get_base_env() -> BTreeMap<OsString, EnvEntry> {
     let mut env: BTreeMap<OsString, EnvEntry> = std::env::vars_os()
         .map(|(key, value)| {
@@ -103,37 +167,8 @@ fn get_base_env() -> BTreeMap<OsString, EnvEntry> {
 
     #[cfg(windows)]
     {
-        use std::os::windows::ffi::OsStringExt;
-        use winapi::um::processenv::ExpandEnvironmentStringsW;
-        use winreg::enums::{RegType, HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE};
-        use winreg::types::FromRegValue;
-        use winreg::{RegKey, RegValue};
-
-        fn reg_value_to_string(value: &RegValue) -> anyhow::Result<OsString> {
-            match value.vtype {
-                RegType::REG_EXPAND_SZ => {
-                    let src = unsafe {
-                        std::slice::from_raw_parts(
-                            value.bytes.as_ptr() as *const u16,
-                            value.bytes.len() / 2,
-                        )
-                    };
-                    let size =
-                        unsafe { ExpandEnvironmentStringsW(src.as_ptr(), std::ptr::null_mut(), 0) };
-                    let mut buf = vec![0u16; size as usize + 1];
-                    unsafe {
-                        ExpandEnvironmentStringsW(src.as_ptr(), buf.as_mut_ptr(), buf.len() as u32)
-                    };
-
-                    let mut buf = buf.as_slice();
-                    while let Some(0) = buf.last() {
-                        buf = &buf[0..buf.len() - 1];
-                    }
-                    Ok(OsString::from_wide(buf))
-                }
-                _ => Ok(OsString::from_reg_value(value)?),
-            }
-        }
+        use winreg::enums::{HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE};
+        use winreg::RegKey;
 
         if let Ok(sys_env) = RegKey::predef(HKEY_LOCAL_MACHINE)
             .open_subkey("System\\CurrentControlSet\\Control\\Session Manager\\Environment")
@@ -662,12 +697,24 @@ impl CommandBuilder {
             value,
         } in self.envs.values()
         {
-            block.extend(preferred_key.encode_wide());
+            let key: Vec<u16> = preferred_key.encode_wide().collect();
+            if key.is_empty()
+                || key.contains(&0)
+                || key[1..].contains(&(b'=' as u16))
+                || value.encode_wide().any(|unit| unit == 0)
+            {
+                continue;
+            }
+
+            block.extend(key);
             block.push(b'=' as u16);
             block.extend(value.encode_wide());
             block.push(0);
         }
         // and a final terminator for CreateProcessW
+        if block.is_empty() {
+            block.push(0);
+        }
         block.push(0);
 
         block
@@ -869,4 +916,65 @@ mod tests {
         assert!(command.ends_with(r#"/d /c echo "hi""#), "{}", command);
         assert!(!command.contains(r#"\"hi\""#), "{}", command);
     }
+
+    #[cfg(windows)]
+    #[test]
+    fn windows_environment_rejects_malformed_entries() {
+        use std::os::windows::ffi::{OsStrExt, OsStringExt};
+        use winreg::enums::RegType;
+        use winreg::RegValue;
+
+        let multi_string = RegValue {
+            bytes: vec![b'a', 0, 0, 0],
+            vtype: RegType::REG_MULTI_SZ,
+        };
+        assert!(reg_value_to_string(&multi_string).is_err());
+        let malformed_expand_string = RegValue {
+            bytes: vec![b'%', 0, b'P'],
+            vtype: RegType::REG_EXPAND_SZ,
+        };
+        assert!(reg_value_to_string(&malformed_expand_string).is_err());
+        let expandable_string = RegValue {
+            bytes: "literal\0"
+                .encode_utf16()
+                .flat_map(u16::to_le_bytes)
+                .collect(),
+            vtype: RegType::REG_EXPAND_SZ,
+        };
+        assert_eq!(
+            reg_value_to_string(&expandable_string).unwrap(),
+            OsString::from("literal")
+        );
+
+        let mut cmd = CommandBuilder::new("dummy");
+        cmd.env_clear();
+        cmd.env("", "ignored");
+        cmd.env(
+            OsString::from_wide(&[b'B' as u16, 0, b'A' as u16, b'D' as u16]),
+            "ignored",
+        );
+        cmd.env(
+            "MULTI",
+            OsString::from_wide(&[b'a' as u16, 0, b'b' as u16]),
+        );
+        cmd.env("BAD=KEY", "ignored");
+        cmd.env("=::", "colon");
+        cmd.env("=C:", r"C:\valid");
+        cmd.env("=ExitCode", "hidden");
+        cmd.env("EMPTY", "");
+        cmd.env("VALID", "ok");
+
+        let expected: Vec<u16> =
+            OsStr::new("=::=colon\0=C:=C:\\valid\0=ExitCode=hidden\0EMPTY=\0VALID=ok\0\0")
+                .encode_wide()
+                .collect();
+        assert_eq!(cmd.environment_block(), expected);
+
+        cmd.env_remove("=::");
+        cmd.env_remove("=C:");
+        cmd.env_remove("=ExitCode");
+        cmd.env_remove("EMPTY");
+        cmd.env_remove("VALID");
+        assert_eq!(cmd.environment_block(), vec![0, 0]);
+    }
 }