harden(winrm): strict, namespace-correct XML rejection (Codex round)

crates/ironposh-client-tokio/tests/e2e/command_matrix.rs

@@ -100,6 +100,23 @@ fn real_server_command_matrix_all_auths() {
                 "C3 retry marker missing for auth={auth}. stdout={stdout} stderr={stderr}"
             );
         }
+
+        // C4: a non-existent command. PowerShell reports CommandNotFound as an
+        // error record (not a process-fatal failure), so the client prints it via
+        // `render_concise` to stdout and still exits 0. The message echoes the
+        // command name, which is what we assert on (locale-independent).
+        {
+            let bogus = "Get-IronPoshDefinitelyNotARealCommand";
+            let (ok, stdout, stderr) = run_noninteractive(auth, bogus);
+            assert!(
+                ok,
+                "C4 process should exit 0 (error record is non-fatal) for auth={auth}. stdout={stdout} stderr={stderr}"
+            );
+            assert!(
+                stdout.contains(bogus),
+                "C4 command-not-found error missing the command name for auth={auth}. stdout={stdout} stderr={stderr}"
+            );
+        }
     }
     let _ = Instant::now(); // keep `Instant` import used without adding behavior
 }

crates/ironposh-macros/src/lib.rs

@@ -1364,6 +1364,7 @@ fn impl_from_xml(input: &DeriveInput) -> Result<TokenStream2, syn::Error> {
                 node: ironposh_xml::parser::Node<'a, 'a>,
             ) -> Result<Self, ironposh_xml::XmlError> {
                 use ironposh_xml::mapping::NodeExt;
+                ironposh_xml::mapping::reject_mixed_content(node)?;
                 #(#inits)*
                 for child in node.children() {
                     if !child.is_element() {

crates/ironposh-winrm/Cargo.toml

@@ -13,7 +13,7 @@ ironposh-xml = { path = "../ironposh-xml" }
 ironposh-macros = { path = "../ironposh-macros" }
 byteorder = "1.5.0"
 base64 = "0.22.1"
-paste = "1.0.15"
+paste = "1.0"
 uuid = { version = "1.0", features = ["v4"] }
 
 [dev-dependencies]

crates/ironposh-winrm/src/cores/attribute.rs

@@ -19,20 +19,33 @@ macro_rules! define_attributes {
         impl<'a> Attribute<'a> {
             /// Convert an attribute name to the corresponding enum variant type
             /// This is automatically generated to match all enum variants
-            pub fn from_name_and_value(name: &str, value: &'a str) -> Result<Option<Self>, ironposh_xml::XmlError> {
-                match name {
-                    $(
-                        $attr_name => {
-                            match $parser(value) {
+            pub fn from_name_and_value(namespace: Option<&str>, name: &str, value: &'a str) -> Result<Option<Self>, ironposh_xml::XmlError> {
+                // The reserved `xml:` prefix is modeled literally (e.g. "xml:lang",
+                // no declared namespace); fold roxmltree's expanded
+                // (xml-namespace, local) form back to that spelling so it matches.
+                let xml_prefixed;
+                let (namespace, name) = if namespace == Some("http://www.w3.org/XML/1998/namespace") {
+                    xml_prefixed = format!("xml:{name}");
+                    (None, xml_prefixed.as_str())
+                } else {
+                    (namespace, name)
+                };
+                // Identity is the (namespace-URI, local-name) pair, like elements:
+                // a known attribute in the wrong namespace is not that attribute.
+                $(
+                    {
+                        let expected_ns: Option<crate::cores::namespace::Namespace> = $namespace;
+                        if name == $attr_name && namespace == expected_ns.map(|ns| ns.uri()) {
+                            return match $parser(value) {
                                 Ok(val) => Ok(Some(Attribute::$variant(val))),
                                 Err(e) => Err(ironposh_xml::XmlError::InvalidXml(
                                     format!("Invalid value for {}: {}", $attr_name, e)
                                 )),
-                            }
+                            };
                         }
-                    )*
-                    _ => Ok(None), // Unknown attribute, ignore
-                }
+                    }
+                )*
+                Ok(None) // Unknown attribute, ignore
             }
 
             /// Get the attribute name for this enum variant
@@ -82,16 +95,27 @@ macro_rules! define_attributes {
     };
 }
 
+/// XSD boolean: `true`/`false` and their `1`/`0` aliases (MS-WSMV/SOAP send both).
+fn parse_xml_bool(v: &str) -> Result<bool, String> {
+    match v {
+        "true" | "1" => Ok(true),
+        "false" | "0" => Ok(false),
+        other => Err(format!(
+            "expected xs:boolean (true/false/1/0), got {other:?}"
+        )),
+    }
+}
+
 // Define all attributes here - adding a new one automatically updates ALL related code
 define_attributes!(
     MustUnderstand(bool) => (Some(crate::cores::namespace::Namespace::SoapEnvelope2003), "mustUnderstand"),
-        |v: &str| v.parse::<bool>().map_err(|e| e.to_string()),
+        parse_xml_bool,
         |v: bool| v.to_string(),
     Name(Cow<'a, str>) => (None, "Name"),
         |v: &str| -> Result<Cow<'a, str>, String> { Ok(Cow::Owned(v.to_string())) },
         |v: Cow<'a, str>| v.into_owned(),
     MustComply(bool) => (None, "MustComply"),
-        |v: &str| v.parse::<bool>().map_err(|e| e.to_string()),
+        parse_xml_bool,
         |v: bool| v.to_string(),
     ShellId(Cow<'a, str>) => (None, "ShellId"),
         |v: &str| -> Result<Cow<'a, str>, String> { Ok(Cow::Owned(v.to_string())) },
@@ -114,13 +138,13 @@ define_attributes!(
         |v: &str| -> Result<Cow<'a, str>, String> { Ok(Cow::Owned(v.to_string())) },
         |v: Cow<'a, str>| v.into_owned(),
     End(bool) => (None, "End"),
-        |v: &str| v.parse::<bool>().map_err(|e| e.to_string()),
+        parse_xml_bool,
         |v: bool| v.to_string(),
     Unit(Cow<'a, str>) => (None, "Unit"),
         |v: &str| -> Result<Cow<'a, str>, String> { Ok(Cow::Owned(v.to_string())) },
         |v: Cow<'a, str>| v.into_owned(),
     EndUnit(bool) => (None, "EndUnit"),
-        |v: &str| v.parse::<bool>().map_err(|e| e.to_string()),
+        parse_xml_bool,
         |v: bool| v.to_string(),
     SequenceID(u64) => (None, "SequenceID"),
         |v: &str| v.parse::<u64>().map_err(|e| e.to_string()),
@@ -151,13 +175,18 @@ mod tests {
     #[test]
     fn test_parsing_round_trip() {
         let test_cases = [
-            ("mustUnderstand", "true"),
-            ("Name", "test-name"),
-            ("MustComply", "false"),
+            (
+                Some(crate::cores::namespace::Namespace::SoapEnvelope2003.uri()),
+                "mustUnderstand",
+                "true",
+            ),
+            (None, "Name", "test-name"),
+            (None, "MustComply", "false"),
         ];
 
-        for (attr_name, attr_value) in test_cases {
-            if let Some(parsed) = Attribute::from_name_and_value(attr_name, attr_value).unwrap() {
+        for (ns, attr_name, attr_value) in test_cases {
+            if let Some(parsed) = Attribute::from_name_and_value(ns, attr_name, attr_value).unwrap()
+            {
                 // Test that we can get the name back
                 assert_eq!(parsed.attribute_name(), attr_name);
 
@@ -166,4 +195,47 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn known_attribute_in_wrong_namespace_is_unmatched() {
+        // `mustUnderstand` lives in the SOAP namespace; unqualified it is unknown.
+        assert!(
+            Attribute::from_name_and_value(None, "mustUnderstand", "true")
+                .unwrap()
+                .is_none()
+        );
+    }
+
+    #[test]
+    fn known_attribute_with_bad_value_propagates_error() {
+        assert!(Attribute::from_name_and_value(None, "MustComply", "notabool").is_err());
+    }
+
+    #[test]
+    fn xsd_boolean_accepts_one_and_zero() {
+        assert!(matches!(
+            Attribute::from_name_and_value(None, "MustComply", "1").unwrap(),
+            Some(Attribute::MustComply(true))
+        ));
+        assert!(matches!(
+            Attribute::from_name_and_value(
+                Some(crate::cores::namespace::Namespace::SoapEnvelope2003.uri()),
+                "mustUnderstand",
+                "0"
+            )
+            .unwrap(),
+            Some(Attribute::MustUnderstand(false))
+        ));
+    }
+
+    #[test]
+    fn xml_lang_matches_via_expanded_namespace() {
+        let parsed = Attribute::from_name_and_value(
+            Some("http://www.w3.org/XML/1998/namespace"),
+            "lang",
+            "en-US",
+        )
+        .unwrap();
+        assert!(matches!(parsed, Some(Attribute::XmlLang(_))));
+    }
 }

crates/ironposh-winrm/src/cores/tag.rs

@@ -174,17 +174,20 @@ where
             node
         } else {
             // Wrapper case (`Tag<Tag<..>, _>`): exactly one child element, which
-            // must be N. Reject zero (wrong wrapper), >1 (malformed), or a single
-            // child of the wrong name rather than silently picking one.
+            // must be N. Reject zero (wrong wrapper), >1 (malformed), a single
+            // child of the wrong name, or stray text rather than silently
+            // picking one.
+            ironposh_xml::mapping::reject_mixed_content(node)?;
             let mut elements = node
                 .children()
                 .filter(ironposh_xml::parser::Node::is_element);
-            let only = elements
-                .next()
-                .ok_or_else(|| ironposh_xml::XmlError::XmlInvalidTag {
-                    expected: N::TAG_NAME.to_string(),
-                    found: node.tag_name().name().to_string(),
-                })?;
+            let only = elements.next().ok_or_else(|| {
+                ironposh_xml::XmlError::InvalidXml(format!(
+                    "expected a <{}> child in <{}>, found none",
+                    N::TAG_NAME,
+                    node.tag_name().name()
+                ))
+            })?;
             if elements.next().is_some() {
                 return Err(ironposh_xml::XmlError::InvalidXml(format!(
                     "expected exactly one child element in <{}>",
@@ -201,14 +204,16 @@ where
         };
 
         let value = V::from_xml(element)?;
-        let attributes = element
-            .attributes()
-            .filter_map(|attr| {
-                Attribute::from_name_and_value(attr.name(), attr.value())
-                    .ok()
-                    .flatten()
-            })
-            .collect();
+        // Identity is (namespace-URI, local-name); a parse error on a *known*
+        // attribute is propagated, while a truly unknown attribute is ignored.
+        let mut attributes = Vec::new();
+        for attr in element.attributes() {
+            if let Some(parsed) =
+                Attribute::from_name_and_value(attr.namespace(), attr.name(), attr.value())?
+            {
+                attributes.push(parsed);
+            }
+        }
         let namespaces_declaration = NamespaceDeclaration::from_xml(element)?;
 
         Ok(Tag {
@@ -298,4 +303,32 @@ mod tests {
             .expect("nested CommandResponse/CommandId should parse");
         assert_eq!(tag.value.value.0.to_string().to_uppercase(), uuid);
     }
+
+    /// A wrapper that yields no usable child must be rejected, not defaulted.
+    #[test]
+    fn nested_tag_rejects_no_child() {
+        let xml = format!(r#"<rsp:CommandResponse xmlns:rsp="{RSP}"/>"#);
+        let doc = parse(&xml).unwrap();
+        assert!(CommandResponse::from_xml(doc.root_element()).is_err());
+    }
+
+    /// More than one child element is ambiguous; the descend branch must reject
+    /// it rather than silently picking the first.
+    #[test]
+    fn nested_tag_rejects_multiple_children() {
+        let uuid = "2D6534D0-6B12-40E3-B773-CBA26459CFA8";
+        let xml = format!(
+            r#"<rsp:CommandResponse xmlns:rsp="{RSP}"><rsp:CommandId>{uuid}</rsp:CommandId><rsp:CommandId>{uuid}</rsp:CommandId></rsp:CommandResponse>"#
+        );
+        let doc = parse(&xml).unwrap();
+        assert!(CommandResponse::from_xml(doc.root_element()).is_err());
+    }
+
+    #[test]
+    fn nested_tag_rejects_wrong_child_name() {
+        let xml =
+            format!(r#"<rsp:CommandResponse xmlns:rsp="{RSP}"><rsp:Wrong/></rsp:CommandResponse>"#);
+        let doc = parse(&xml).unwrap();
+        assert!(CommandResponse::from_xml(doc.root_element()).is_err());
+    }
 }

crates/ironposh-winrm/src/cores/tag_name.rs

@@ -43,14 +43,14 @@ tag!(DesiredStream = Text<'a> => WsmanShell);
 tag!(Stream = Text<'a> => WsmanShell);
 tag!(ExitCode = I32 => WsmanShell);
 tag!(CommandId = WsUuid => WsmanShell);
-tag!(CommandResponse = CommandId<'a> => WsmanShell); // wraps a single CommandId child
+tag!(CommandResponse = CommandId<'a> => WsmanShell);
 tag!(Command = Text<'a> => WsmanShell);
 tag!(Arguments = Text<'a> => WsmanShell);
 tag!(DisconnectResponse = Empty => WsmanShell);
 tag!(Reconnect = Empty => WsmanShell);
 tag!(ReconnectResponse = Empty => WsmanShell);
 tag!(SignalCode = "Code": Text<'a> => WsmanShell);
-tag!(Signal = SignalCode<'a> => WsmanShell); // wraps a single Code child
+tag!(Signal = SignalCode<'a> => WsmanShell);
 tag!(SignalResponse = Empty => WsmanShell);
 
 tag!(CreationXml = "creationXml": Text<'a> => PowerShellRemoting);