harden(winrm): reject malformed XML in the FromXml layer

crates/ironposh-client-core/src/runspace/win_rs.rs

@@ -1,10 +1,10 @@
 use base64::Engine;
 use ironposh_winrm::{
-    cores::{Attribute, DesiredStream, Receive, Shell, Tag, Text, Time, tag_name},
+    cores::{Attribute, DesiredStreamTag, StreamTag, Tag, Text, Time},
     rsp::{
         commandline::CommandLineValue,
-        receive::{CommandStateValue, ReceiveValue},
-        shell_value::ShellValue,
+        receive::{CommandStateTag, CommandStateValue, ReceiveTag, ReceiveValue},
+        shell_value::{ShellTag, ShellValue},
     },
     soap::{SoapEnvelope, body::SoapBody},
     ws_management::{OptionSetValue, SelectorSetValue, WsAction, WsMan},
@@ -67,7 +67,7 @@ impl WinRunspace {
         option_set: Option<OptionSetValue>,
         open_content: &'a str,
     ) -> impl Into<Element<'a>> {
-        let shell = Tag::from_name(Shell)
+        let shell = Tag::from_name(ShellTag)
             .with_attribute(ironposh_winrm::cores::Attribute::ShellId(
                 self.id.to_string().into(),
             ))
@@ -130,7 +130,7 @@ impl WinRunspace {
                 // Join stream names with spaces as required by Windows Shell schema
                 let combined_streams = stream_names.join(" ");
                 let mut tag =
-                    Tag::from_name(DesiredStream).with_value(Text::from(combined_streams));
+                    Tag::from_name(DesiredStreamTag).with_value(Text::from(combined_streams));
 
                 if let Some(command_id) = command_id {
                     tag = tag.with_attribute(Attribute::CommandId(command_id));
@@ -144,7 +144,7 @@ impl WinRunspace {
             .desired_streams(desired_stream_tags)
             .build();
 
-        let receive_tag = Tag::from_name(Receive)
+        let receive_tag = Tag::from_name(ReceiveTag)
             .with_value(receive)
             .with_declaration(ironposh_winrm::cores::Namespace::WsmanShell);
 
@@ -172,9 +172,12 @@ impl WinRunspace {
 
     /// Build a Disconnect request targeting this shell (MS-WSMV 3.1.4.13).
     pub(crate) fn fire_disconnect<'a>(&'a self, ws_man: &'a WsMan) -> impl Into<Element<'a>> {
-        use ironposh_winrm::{cores::Namespace, rsp::disconnect::DisconnectValue};
+        use ironposh_winrm::{
+            cores::Namespace,
+            rsp::disconnect::{DisconnectTag, DisconnectValue},
+        };
 
-        let disconnect_tag = Tag::from_name(tag_name::Disconnect)
+        let disconnect_tag = Tag::from_name(DisconnectTag)
             .with_declaration(Namespace::WsmanShell)
             .with_value(DisconnectValue::builder().build());
 
@@ -197,13 +200,16 @@ impl WinRunspace {
         option_set: Option<OptionSetValue>,
         connect_payload: &'a str,
     ) -> impl Into<Element<'a>> {
-        use ironposh_winrm::{cores::Namespace, rsp::connect::ConnectValue};
+        use ironposh_winrm::{
+            cores::Namespace,
+            rsp::connect::{ConnectTag, ConnectValue},
+        };
 
         let connect_value = ConnectValue {
             connect_xml: Tag::new(connect_payload).with_declaration(Namespace::PowerShellRemoting),
         };
 
-        let connect_tag = Tag::from_name(tag_name::Connect)
+        let connect_tag = Tag::from_name(ConnectTag)
             .with_declaration(Namespace::WsmanShell)
             .with_value(connect_value);
 
@@ -218,9 +224,9 @@ impl WinRunspace {
 
     /// Build a Reconnect request targeting this shell (MS-WSMV 3.1.4.14).
     pub(crate) fn fire_reconnect<'a>(&'a self, ws_man: &'a WsMan) -> impl Into<Element<'a>> {
-        use ironposh_winrm::cores::{Empty, Namespace};
+        use ironposh_winrm::cores::{Empty, Namespace, ReconnectTag};
 
-        let reconnect_tag = Tag::from_name(tag_name::Reconnect)
+        let reconnect_tag = Tag::from_name(ReconnectTag)
             .with_declaration(Namespace::WsmanShell)
             .with_value(Empty);
 
@@ -390,20 +396,17 @@ impl WinRunspace {
         data: &'a [String],
     ) -> Result<impl Into<Element<'a>>, crate::PwshCoreError> {
         use ironposh_winrm::{
-            cores::{
-                Namespace, Tag,
-                tag_name::{Send, Stream},
-            },
-            rsp::send::SendValue,
+            cores::{Namespace, StreamTag, Tag},
+            rsp::send::{SendTag, SendValue},
             soap::body::SoapBody,
         };
 
         // Create a Stream tag for each fragment
         // Each fragment is a base64-encoded PSRP fragment that goes in its own <rsp:Stream> element
-        let streams: Vec<Tag<Text, Stream>> = data
+        let streams: Vec<Tag<Text, StreamTag>> = data
             .iter()
             .map(|fragment| {
-                Tag::from_name(Stream)
+                Tag::from_name(StreamTag)
                     .with_value(Text::from(fragment.as_str()))
                     .with_attribute(Attribute::Name("stdin".into()))
             })
@@ -413,12 +416,12 @@ impl WinRunspace {
 
         // Add send tag with SendValue containing multiple streams
         let send_tag = if let Some(cmd_id) = command_id {
-            Tag::from_name(Send)
+            Tag::from_name(SendTag)
                 .with_value(send_value)
                 .with_attribute(Attribute::CommandId(cmd_id))
                 .with_declaration(Namespace::WsmanShell)
         } else {
-            Tag::from_name(Send)
+            Tag::from_name(SendTag)
                 .with_value(send_value)
                 .with_declaration(Namespace::WsmanShell)
         };
@@ -460,18 +463,15 @@ impl WinRunspace {
         connection: &'a WsMan,
         id: Uuid,
     ) -> Result<impl Into<Element<'a>>, crate::PwshCoreError> {
-        use ironposh_winrm::cores::{
-            Namespace,
-            tag_name::{Signal, SignalCode},
-        };
+        use ironposh_winrm::cores::{Namespace, SignalCodeTag, SignalTag};
 
         // Build <rsp:Code>http://schemas.microsoft.com/wbem/wsman/1/windows/shell/signal/ctrl_c</rsp:Code>
-        let code = Tag::from_name(SignalCode).with_value(Text::from(
+        let code = Tag::from_name(SignalCodeTag).with_value(Text::from(
             "http://schemas.microsoft.com/wbem/wsman/1/windows/shell/signal/terminate",
         ));
 
         // Build <w:Signal CommandId="...">...</w:Signal>
-        let signal = Tag::from_name(Signal)
+        let signal = Tag::from_name(SignalTag)
             .with_attribute(Attribute::CommandId(id))
             .with_value(code)
             .with_declaration(Namespace::WsmanShell);
@@ -535,10 +535,10 @@ impl Stream {
     }
 }
 
-impl<'a> TryFrom<&Tag<'a, Text<'a>, tag_name::Stream>> for Stream {
+impl<'a> TryFrom<&Tag<'a, Text<'a>, StreamTag>> for Stream {
     type Error = crate::PwshCoreError;
 
-    fn try_from(value: &Tag<'a, Text<'a>, tag_name::Stream>) -> Result<Self, Self::Error> {
+    fn try_from(value: &Tag<'a, Text<'a>, StreamTag>) -> Result<Self, Self::Error> {
         let attributes = &value.attributes;
         let name = attributes
             .iter()
@@ -577,11 +577,11 @@ pub struct CommandState {
     pub exit_code: Option<i32>,
 }
 
-impl<'a> TryFrom<&Tag<'a, CommandStateValue<'a>, tag_name::CommandState>> for CommandState {
+impl<'a> TryFrom<&Tag<'a, CommandStateValue<'a>, CommandStateTag>> for CommandState {
     type Error = crate::PwshCoreError;
 
     fn try_from(
-        value: &Tag<'a, CommandStateValue<'a>, tag_name::CommandState>,
+        value: &Tag<'a, CommandStateValue<'a>, CommandStateTag>,
     ) -> Result<Self, Self::Error> {
         let command_id = value
             .attributes

crates/ironposh-client-core/tests/test_send_roundtrip.rs

@@ -7,8 +7,11 @@ use ironposh_psrp::{
     fragmentation::{DefragmentResult, Defragmenter, Fragmenter},
 };
 use ironposh_winrm::{
-    cores::{Attribute, ReceiveResponse, Send, Tag, Text, tag_name::Stream},
-    rsp::{receive::ReceiveResponseValue, send::SendValue},
+    cores::{Attribute, StreamTag, Tag, Text},
+    rsp::{
+        receive::{ReceiveResponseTag, ReceiveResponseValue},
+        send::{SendTag, SendValue},
+    },
     soap::{SoapEnvelope, body::SoapBody},
 };
 use ironposh_xml::builder::Element;
@@ -42,18 +45,18 @@ fn test_send_receive_roundtrip_with_fragmentation() {
         .collect();
 
     // 4. Create SendValue with multiple Stream elements (NEW CODE PATH)
-    let streams: Vec<Tag<Text, Stream>> = base64_fragments
+    let streams: Vec<Tag<Text, StreamTag>> = base64_fragments
         .iter()
         .map(|fragment| {
-            Tag::from_name(Stream)
+            Tag::from_name(StreamTag)
                 .with_value(Text::from(fragment.as_str()))
                 .with_attribute(Attribute::Name("stdin".into()))
         })
         .collect();
 
     let send_value = SendValue::builder().streams(streams).build();
 
-    let send_tag = Tag::from_name(Send)
+    let send_tag = Tag::from_name(SendTag)
         .with_value(send_value)
         .with_attribute(Attribute::CommandId(command_id))
         .with_declaration(ironposh_winrm::cores::Namespace::WsmanShell);
@@ -80,10 +83,10 @@ fn test_send_receive_roundtrip_with_fragmentation() {
 
     // 6. Simulate receiving the response - create ReceiveResponse with same streams
     // (In real flow, server would echo back or client would receive similar structure)
-    let receive_streams: Vec<Tag<Text, Stream>> = base64_fragments
+    let receive_streams: Vec<Tag<Text, StreamTag>> = base64_fragments
         .iter()
         .map(|fragment| {
-            Tag::from_name(Stream)
+            Tag::from_name(StreamTag)
                 .with_value(Text::from(fragment.as_str()))
                 .with_attribute(Attribute::Name("stdout".into()))
                 .with_attribute(Attribute::CommandId(command_id))
@@ -95,7 +98,7 @@ fn test_send_receive_roundtrip_with_fragmentation() {
         .command_state(None)
         .build();
 
-    let receive_response_tag = Tag::from_name(ReceiveResponse)
+    let receive_response_tag = Tag::from_name(ReceiveResponseTag)
         .with_value(receive_response_value)
         .with_declaration(ironposh_winrm::cores::Namespace::WsmanShell);
 
@@ -183,7 +186,7 @@ fn create_large_session_capability() -> SessionCapability {
 fn test_send_with_no_fragments() {
     let send_value = SendValue::builder().streams(vec![]).build();
 
-    let send_tag = Tag::from_name(Send)
+    let send_tag = Tag::from_name(SendTag)
         .with_value(send_value)
         .with_declaration(ironposh_winrm::cores::Namespace::WsmanShell);
 
@@ -222,13 +225,13 @@ fn test_send_with_single_fragment() {
 
     let base64_fragment = base64::engine::general_purpose::STANDARD.encode(&fragments[0]);
 
-    let stream = Tag::from_name(Stream)
+    let stream = Tag::from_name(StreamTag)
         .with_value(Text::from(base64_fragment.as_str()))
         .with_attribute(Attribute::Name("stdin".into()));
 
     let send_value = SendValue::builder().streams(vec![stream]).build();
 
-    let send_tag = Tag::from_name(Send)
+    let send_tag = Tag::from_name(SendTag)
         .with_value(send_value)
         .with_attribute(Attribute::CommandId(command_id))
         .with_declaration(ironposh_winrm::cores::Namespace::WsmanShell);

crates/ironposh-macros/src/lib.rs

@@ -1260,30 +1260,47 @@ pub fn derive_simple_tag_value(input: TokenStream) -> TokenStream {
 }
 
 /// Derives [`ironposh_xml::mapping::FromXml`] for a WinRM struct whose fields
-/// are `Tag<'a, V, N>` / `Option<Tag<'a, V, N>>`.
+/// are tag types (`Tag<'a, V, N>`, or a `tag!` alias for one, optionally wrapped
+/// in `Option`).
 ///
-/// Generates a direct, namespace-correct `from_xml(node)` — no visitor struct,
-/// no `finish()`. Each child element is matched by its `(namespace-URI,
-/// local-name)` pair, sourced from the field's `N: TagName` (`NAMESPACE` +
-/// `TAG_NAME`); the prefix is never compared. `Option<_>` fields stay `None`
-/// when absent; required fields error. This is the deserialize-side replacement
-/// for the visitor that `SimpleXmlDeserialize` generates.
+/// Generates a direct, namespace-correct `from_xml(node)` — no visitor. Each
+/// child is matched by its `(namespace-URI, local-name)` pair, read from the
+/// field type via `NamedTag` (so it works through type aliases); the prefix is
+/// never compared. `Option<_>` fields stay `None` when absent; required fields
+/// error.
+///
+/// Requirements: the deriving struct must carry a single lifetime parameter `'a`,
+/// and the consumer crate must expose `cores::{NamedTag, TagValue}` — this derive
+/// is winrm-specific.
 #[proc_macro_derive(FromXml)]
 pub fn derive_from_xml(input: TokenStream) -> TokenStream {
     let input = parse_macro_input!(input as DeriveInput);
-    TokenStream::from(impl_from_xml(&input))
+    match impl_from_xml(&input) {
+        Ok(ts) => ts.into(),
+        Err(e) => e.to_compile_error().into(),
+    }
 }
 
-fn impl_from_xml(input: &DeriveInput) -> TokenStream2 {
+fn impl_from_xml(input: &DeriveInput) -> Result<TokenStream2, syn::Error> {
     let name = &input.ident;
     let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
 
     let fields = match &input.data {
         Data::Struct(data) => match &data.fields {
             Fields::Named(fields) => &fields.named,
-            _ => panic!("FromXml can only be derived for structs with named fields"),
+            _ => {
+                return Err(syn::Error::new_spanned(
+                    input,
+                    "FromXml can only be derived for structs with named fields",
+                ));
+            }
         },
-        _ => panic!("FromXml can only be derived for structs"),
+        _ => {
+            return Err(syn::Error::new_spanned(
+                input,
+                "FromXml can only be derived for structs",
+            ));
+        }
     };
 
     let entries: Vec<SimpleFieldEntry> = fields
@@ -1307,6 +1324,7 @@ fn impl_from_xml(input: &DeriveInput) -> TokenStream2 {
 
     // One namespace-correct match per field: identity is (URI, local-name),
     // read from the field's tag type via `NamedTag` so it works through aliases.
+    // Emitted as an `if … else if …` chain so each child binds at most one field.
     let matchers = entries.iter().map(|e| {
         let f = &e.field_name;
         let ty = &e.value_type;
@@ -1315,6 +1333,13 @@ fn impl_from_xml(input: &DeriveInput) -> TokenStream2 {
                 <#ty as crate::cores::NamedTag>::NAMESPACE,
                 <#ty as crate::cores::NamedTag>::TAG_NAME,
             ) {
+                if #f.is_some() {
+                    return Err(ironposh_xml::XmlError::InvalidXml(format!(
+                        "duplicate <{}> in {}",
+                        <#ty as crate::cores::NamedTag>::TAG_NAME,
+                        stringify!(#name),
+                    )));
+                }
                 #f = Some(ironposh_xml::mapping::FromXml::from_xml(child)?);
             }
         }
@@ -1333,7 +1358,7 @@ fn impl_from_xml(input: &DeriveInput) -> TokenStream2 {
         }
     });
 
-    quote! {
+    Ok(quote! {
         impl #impl_generics ironposh_xml::mapping::FromXml<'a> for #name #ty_generics #where_clause {
             fn from_xml(
                 node: ironposh_xml::parser::Node<'a, 'a>,
@@ -1344,12 +1369,12 @@ fn impl_from_xml(input: &DeriveInput) -> TokenStream2 {
                     if !child.is_element() {
                         continue;
                     }
-                    #(#matchers)*
+                    #(#matchers)else*
                 }
                 Ok(#name { #(#construct),* })
             }
         }
-    }
+    })
 }
 
 fn impl_simple_tag_value(input: &DeriveInput) -> TokenStream2 {