From 6b003186053d585510c10ae070f80b142e971f18 Mon Sep 17 00:00:00 2001
From: Greg Lamberson <greg@lamco.io>
Date: Wed, 27 May 2026 13:09:04 -0500
Subject: [PATCH] refactor(pdu): make the CapabilitySet encoder exhaustive

The inner match in `CapabilitySet::Encode::encode`'s raw-buffer fallback
(`mod.rs:448`) used a `_ => unreachable!()` catch-all. Replace it with an
explicit enumeration of the 17 structured `CapabilitySet` variants already
handled by the outer match's specific arms, so adding a new variant to
`CapabilitySet` is a compile error here until the variant is routed in this
`Encode` impl. The arm body stays `unreachable!()` because it is genuinely
unreachable under the current outer-match structure. PR #1313 (BitmapCacheV3
encoder `unreachable!()` reached on decoder-accepted input) demonstrated why
a runtime catch-all is the wrong shape for this match.

The nested `match guid { ... _ => unreachable!() }` in `CodecProperty` decode
for `GUID_REMOTEFX | GUID_IMAGE_REMOTEFX` (`bitmap_codecs/mod.rs`) keeps its
structure. Per review, `unreachable!()` is the right tool there: `guid` is
already validated by the outer arm, so the `_` branch is a redundant
correctness check that fires under tests and fuzzing if a future change breaks
the invariant. The only change is a message documenting that invariant inline.

This is part of the audit pass on issue #1314. Both sites are class (a)
(provably unreachable in current code), confirmed by `pdu_round_trip` (#1291)
and `message_decoding_invariants` (#1320) exercising `CapabilitySet` and
`ShareControlHeader` for millions of iterations post-#1313 with no panics.

Refs #1314.
---
 .../rdp/capability_sets/bitmap_codecs/mod.rs  |  7 ++++-
 .../src/rdp/capability_sets/mod.rs            | 29 ++++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs/mod.rs b/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs/mod.rs
index 59687dd4f..3143de072 100644
--- a/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs/mod.rs
+++ b/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs/mod.rs
@@ -281,7 +281,12 @@ impl<'de> Decode<'de> for Codec {
                 match guid {
                     GUID_REMOTEFX => CodecProperty::RemoteFx(property),
                     GUID_IMAGE_REMOTEFX => CodecProperty::ImageRemoteFx(property),
-                    _ => unreachable!(),
+                    // `guid` is validated as RemoteFX or ImageRemoteFX by the outer
+                    // match arm, so the `_` branch is genuinely dead. Keep it as a
+                    // redundant correctness check that fires loudly under tests and
+                    // fuzzing if a future change to the outer arm breaks that
+                    // invariant. Not reachable from the wire.
+                    _ => unreachable!("guid validated as RemoteFX or ImageRemoteFX by the outer match"),
                 }
             }
             GUID_IGNORE => CodecProperty::Ignore,
diff --git a/crates/ironrdp-pdu/src/rdp/capability_sets/mod.rs b/crates/ironrdp-pdu/src/rdp/capability_sets/mod.rs
index 555888525..3e894bf03 100644
--- a/crates/ironrdp-pdu/src/rdp/capability_sets/mod.rs
+++ b/crates/ironrdp-pdu/src/rdp/capability_sets/mod.rs
@@ -445,7 +445,34 @@ impl Encode for CapabilitySet {
                     CapabilitySet::Rail(buffer) => (CapabilitySetType::Rail, buffer),
                     CapabilitySet::WindowList(buffer) => (CapabilitySetType::WindowList, buffer),
                     CapabilitySet::BitmapCacheV3(buffer) => (CapabilitySetType::BitmapCacheV3CodecID, buffer),
-                    _ => unreachable!(),
+                    // Structured variants are routed through the outer match's
+                    // specific arms above this block and cannot reach this
+                    // inner match. Listing them explicitly (instead of using
+                    // `_ =>`) makes a future addition to `CapabilitySet` a
+                    // compile error here until the new variant is routed in
+                    // this `Encode` impl. PR #1313 (BitmapCacheV3 encoder
+                    // `unreachable!()` reached on decoder-accepted input)
+                    // demonstrated why a runtime catch-all is the wrong shape
+                    // for this match.
+                    CapabilitySet::General(_)
+                    | CapabilitySet::Bitmap(_)
+                    | CapabilitySet::Order(_)
+                    | CapabilitySet::BitmapCache(_)
+                    | CapabilitySet::BitmapCacheRev2(_)
+                    | CapabilitySet::Pointer(_)
+                    | CapabilitySet::Sound(_)
+                    | CapabilitySet::Input(_)
+                    | CapabilitySet::Brush(_)
+                    | CapabilitySet::GlyphCache(_)
+                    | CapabilitySet::OffscreenBitmapCache(_)
+                    | CapabilitySet::VirtualChannel(_)
+                    | CapabilitySet::MultiFragmentUpdate(_)
+                    | CapabilitySet::LargePointer(_)
+                    | CapabilitySet::SurfaceCommands(_)
+                    | CapabilitySet::BitmapCodecs(_)
+                    | CapabilitySet::FrameAcknowledge(_) => {
+                        unreachable!("structured variant routed to raw-buffer encoder arm")
+                    }
                 };
 
                 dst.write_u16(capability_set_type.as_u16());