harden(winrm): strict, namespace-correct XML rejection (Codex round)#39
Merged
irvingouj@Devolutions (irvingoujAtDevolution) merged 6 commits intoJun 25, 2026
Merged
Conversation
Pin the rejection behavior the hardening round added: mixed-content leaf rejection, Empty non-empty rejection, single-child descend (zero/multiple/ wrong-name), duplicate singleton (Command/CommandState/derived ExitCode), and duplicate selector/option keys.
A non-existent command surfaces as an error record (non-fatal), printed via render_concise to stdout while the process still exits 0. Asserts both, across all auth methods.
Polish from the independent review pass (no behavior change except a clearer error message): remove dead commented-out visitor block in ws_addressing; correct the tag! marker-visibility doc; drop two what/how comments; loosen the paste version pin; clearer 'found none' diagnostic for an empty tag wrapper; restore optional-permutation derive tests and add mixed-content rejection tests for numeric/uuid leaves.
Address the independent Codex pass — reject malformed input uniformly rather
than silently absorbing it:
- attributes matched by (namespace-URI, local-name) with parse errors on known
attributes propagated instead of dropped (cores/attribute.rs, cores/tag.rs)
- <Selector>/<Option> require an unqualified Name attribute (ws_management/header.rs)
- SoapEnvelope::from_xml validates the root really is {SOAP}Envelope before
reading children (soap/mod.rs)
- reject mixed content (non-whitespace text) in element containers, applied in
the derive and every hand-written container (ironposh-xml mapping helper)
- leaf_text tolerates comments/PIs and concatenates text runs so a comment can
neither be rejected nor truncate the value (cores/tag_value.rs)
Adds negative tests for each.
Two issues the strict pass exposed: - xs:boolean attributes (mustUnderstand/MustComply/End/EndUnit) now accept the 1/0 forms, not just true/false, so propagating parse errors can't reject a spec-compliant <s:mustUnderstand="1"> response. - xml:lang is modeled literally as "xml:lang" (the reserved prefix is never declared); fold roxmltree's expanded (xml-namespace, "lang") form back to that spelling so it matches on input instead of being dropped.
SOAP 1.2 permits multiple <Text xml:lang> entries in a fault <Reason>; the derived singleton rejected them as duplicates. Hand-write FromXml to keep the first and ignore the rest.
irvingouj@Devolutions (irvingoujAtDevolution)
changed the base branch from
stack/xml-fromxml-tests
to
master
irvingouj@Devolutions (irvingoujAtDevolution)
deleted the
stack/xml-fromxml-strict
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 7/7. Base:
stack/xml-fromxml-tests(#38).Addresses every finding from the independent Codex review pass — reject malformed/untrusted input uniformly instead of silently absorbing it:
(namespace-URI, local-name); a parse error on a known attribute is propagated, not dropped (cores/attribute.rs,cores/tag.rs).<Selector>/<Option>require an unqualifiedNameattribute; missing → error (ws_management/header.rs).SoapEnvelope::from_xmlvalidates the root really is{SOAP}Envelopebefore reading children — the value parser alone would accept any wrapper carrying aBody(soap/mod.rs).reject_mixed_contenthelper applied in the derive and every hand-written container (ironposh-xml/mapping.rs).leaf_textnow tolerates comments/PIs and concatenates text runs, so a comment can neither be rejected nor silently truncate the value (cores/tag_value.rs).Negative tests added for each. Verified: fmt + clippy
-D warnings+ workspace tests clean; real-server matrix 10/10 and command matrix green (the new strictness does not reject real WinRM responses).FromXml stack (review/merge bottom-up):
tag!macro + aliases (refactor(winrm): alias-based tags via tag! macro #36)