fix: harden render command validation

1 files changed, 421 insertions, 17 deletions

diff --git a/crates/fparkan-render/src/lib.rs b/crates/fparkan-render/src/lib.rs
index 5f6d1da..1d8b0e7 100644
--- a/crates/fparkan-render/src/lib.rs
+++ b/crates/fparkan-render/src/lib.rs
@@ -139,6 +139,15 @@ pub struct RenderCommandList {
     pub commands: Vec<RenderCommand>,
 }
 
+/// Optional render command validation limits.
+#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
+pub struct RenderValidationLimits {
+    /// Exclusive upper bound for GPU mesh ids.
+    pub mesh_count: Option<u64>,
+    /// Exclusive upper bound for index ranges.
+    pub index_count: Option<u32>,
+}
+
 /// Frame output.
 #[derive(Clone, Debug, Default, Eq, PartialEq)]
 pub struct FrameOutput;
@@ -148,6 +157,13 @@ pub struct FrameOutput;
 pub enum RenderError {
     /// Invalid range.
     InvalidRange,
+    /// Invalid command stream framing or ordering.
+    InvalidCommandStream {
+        /// Command index.
+        index: usize,
+        /// Contextual error message.
+        message: &'static str,
+    },
     /// Invalid draw range with command-generation context.
     InvalidDrawRange {
         /// Draw id.
@@ -159,6 +175,49 @@ pub enum RenderError {
         /// Range count.
         count: u32,
     },
+    /// Index range arithmetic overflow.
+    IndexRangeOverflow {
+        /// Draw id.
+        draw_id: DrawId,
+        /// Range start.
+        start: u32,
+        /// Range count.
+        count: u32,
+    },
+    /// Index range exceeds validation limits.
+    IndexRangeOutOfBounds {
+        /// Draw id.
+        draw_id: DrawId,
+        /// Exclusive index limit.
+        index_count: u32,
+        /// Range end.
+        end: u32,
+    },
+    /// Mesh id exceeds validation limits.
+    MeshOutOfBounds {
+        /// Draw id.
+        draw_id: DrawId,
+        /// Mesh id.
+        mesh: GpuMeshId,
+        /// Exclusive mesh limit.
+        mesh_count: u64,
+    },
+    /// Draw transform contains a non-finite value.
+    NonFiniteTransform {
+        /// Draw id.
+        draw_id: DrawId,
+        /// Matrix element index.
+        element: usize,
+    },
+    /// Draw commands are not ordered by phase, stable order and draw id.
+    PhaseOrderViolation {
+        /// Draw id.
+        draw_id: DrawId,
+        /// Previous phase.
+        previous: RenderPhase,
+        /// Current phase.
+        current: RenderPhase,
+    },
     /// A batch material index did not resolve through the material table.
     MaterialIndexOutOfBounds {
         /// Draw id.
@@ -174,6 +233,12 @@ impl std::fmt::Display for RenderError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             Self::InvalidRange => write!(f, "render command contains an empty index range"),
+            Self::InvalidCommandStream { index, message } => {
+                write!(
+                    f,
+                    "render command stream is invalid at command {index}: {message}"
+                )
+            }
             Self::InvalidDrawRange {
                 draw_id,
                 stable_order,
@@ -184,6 +249,47 @@ impl std::fmt::Display for RenderError {
                 "draw {} has invalid index range start={} count={} at stable order {}",
                 draw_id.0, start, count, stable_order
             ),
+            Self::IndexRangeOverflow {
+                draw_id,
+                start,
+                count,
+            } => write!(
+                f,
+                "draw {} index range overflows start={} count={}",
+                draw_id.0, start, count
+            ),
+            Self::IndexRangeOutOfBounds {
+                draw_id,
+                index_count,
+                end,
+            } => write!(
+                f,
+                "draw {} index range ends at {} but mesh has {} indices",
+                draw_id.0, end, index_count
+            ),
+            Self::MeshOutOfBounds {
+                draw_id,
+                mesh,
+                mesh_count,
+            } => write!(
+                f,
+                "draw {} references mesh {} but only {} meshes are available",
+                draw_id.0, mesh.0, mesh_count
+            ),
+            Self::NonFiniteTransform { draw_id, element } => write!(
+                f,
+                "draw {} has non-finite transform element {}",
+                draw_id.0, element
+            ),
+            Self::PhaseOrderViolation {
+                draw_id,
+                previous,
+                current,
+            } => write!(
+                f,
+                "draw {} phase order regressed from {:?} to {:?}",
+                draw_id.0, previous, current
+            ),
             Self::MaterialIndexOutOfBounds {
                 draw_id,
                 material_index,
@@ -227,6 +333,8 @@ pub fn build_commands(
                 count: draw.range.count,
             });
         }
+        validate_index_range(draw.id, draw.range)?;
+        validate_transform(draw.id, &draw.transform)?;
         let material = draw
             .material_slots
             .get(usize::from(draw.material_index))
@@ -268,7 +376,7 @@ pub struct NullBackend;
 
 impl RenderBackend for NullBackend {
     fn execute(&mut self, commands: &RenderCommandList) -> Result<FrameOutput, RenderError> {
-        validate_commands(commands)?;
+        validate_command_list(commands)?;
         Ok(FrameOutput)
     }
 }
@@ -312,7 +420,7 @@ impl RenderBackend for RecordingBackend {
 ///
 /// Returns [`RenderError`] when a draw command contains an invalid index range.
 pub fn canonical_capture(commands: &RenderCommandList) -> Result<Vec<u8>, RenderError> {
-    validate_commands(commands)?;
+    validate_command_list(commands)?;
     let mut out = Vec::new();
     for command in &commands.commands {
         match command {
@@ -332,12 +440,132 @@ pub fn canonical_capture(commands: &RenderCommandList) -> Result<Vec<u8>, Render
     Ok(out)
 }
 
-fn validate_commands(commands: &RenderCommandList) -> Result<(), RenderError> {
-    for command in &commands.commands {
-        if let RenderCommand::Draw(draw) = command {
-            if draw.range.count == 0 {
-                return Err(RenderError::InvalidRange);
+/// Validates a render command list without backend-specific resource limits.
+///
+/// # Errors
+///
+/// Returns [`RenderError`] when framing, ordering or draw data is invalid.
+pub fn validate_command_list(commands: &RenderCommandList) -> Result<(), RenderError> {
+    validate_command_list_with_limits(commands, RenderValidationLimits::default())
+}
+
+/// Validates a render command list with optional backend resource limits.
+///
+/// # Errors
+///
+/// Returns [`RenderError`] when framing, ordering, draw data or resource bounds
+/// are invalid.
+pub fn validate_command_list_with_limits(
+    commands: &RenderCommandList,
+    limits: RenderValidationLimits,
+) -> Result<(), RenderError> {
+    let Some(first) = commands.commands.first() else {
+        return Err(RenderError::InvalidCommandStream {
+            index: 0,
+            message: "empty command list",
+        });
+    };
+    if !matches!(first, RenderCommand::BeginFrame) {
+        return Err(RenderError::InvalidCommandStream {
+            index: 0,
+            message: "first command must be BeginFrame",
+        });
+    }
+    if commands.commands.len() < 2 {
+        return Err(RenderError::InvalidCommandStream {
+            index: 0,
+            message: "frame must end with EndFrame",
+        });
+    }
+    let end_index = commands.commands.len() - 1;
+    if !matches!(commands.commands[end_index], RenderCommand::EndFrame) {
+        return Err(RenderError::InvalidCommandStream {
+            index: end_index,
+            message: "last command must be EndFrame",
+        });
+    }
+
+    let mut previous_key: Option<(RenderPhase, u64, DrawId)> = None;
+    for (index, command) in commands.commands.iter().enumerate() {
+        match command {
+            RenderCommand::BeginFrame if index == 0 => {}
+            RenderCommand::BeginFrame => {
+                return Err(RenderError::InvalidCommandStream {
+                    index,
+                    message: "nested BeginFrame is not allowed",
+                });
+            }
+            RenderCommand::EndFrame if index == end_index => {}
+            RenderCommand::EndFrame => {
+                return Err(RenderError::InvalidCommandStream {
+                    index,
+                    message: "EndFrame before final command is not allowed",
+                });
             }
+            RenderCommand::Draw(draw) => {
+                validate_draw_command(draw, limits)?;
+                let key = (draw.phase, draw.stable_order, draw.id);
+                if let Some(previous) = previous_key {
+                    if key < previous {
+                        return Err(RenderError::PhaseOrderViolation {
+                            draw_id: draw.id,
+                            previous: previous.0,
+                            current: draw.phase,
+                        });
+                    }
+                }
+                previous_key = Some(key);
+            }
+        }
+    }
+    Ok(())
+}
+
+fn validate_draw_command(
+    draw: &DrawCommand,
+    limits: RenderValidationLimits,
+) -> Result<(), RenderError> {
+    if draw.range.count == 0 {
+        return Err(RenderError::InvalidRange);
+    }
+    let end = validate_index_range(draw.id, draw.range)?;
+    validate_transform(draw.id, &draw.transform)?;
+    if let Some(mesh_count) = limits.mesh_count {
+        if draw.mesh.0 >= mesh_count {
+            return Err(RenderError::MeshOutOfBounds {
+                draw_id: draw.id,
+                mesh: draw.mesh,
+                mesh_count,
+            });
+        }
+    }
+    if let Some(index_count) = limits.index_count {
+        if end > index_count {
+            return Err(RenderError::IndexRangeOutOfBounds {
+                draw_id: draw.id,
+                index_count,
+                end,
+            });
+        }
+    }
+    Ok(())
+}
+
+fn validate_index_range(draw_id: DrawId, range: IndexRange) -> Result<u32, RenderError> {
+    range
+        .start
+        .checked_add(range.count)
+        .ok_or(RenderError::IndexRangeOverflow {
+            draw_id,
+            start: range.start,
+            count: range.count,
+        })
+}
+
+fn validate_transform(draw_id: DrawId, transform: &[f32; 16]) -> Result<(), RenderError> {
+    for (element, value) in transform.iter().enumerate() {
+        if !value.is_finite() {
+            return Err(RenderError::NonFiniteTransform { draw_id, element });
         }
     }
     Ok(())
@@ -400,16 +628,20 @@ mod tests {
     fn null_backend_validates_without_capture() {
         let mut backend = NullBackend;
         let invalid = RenderCommandList {
-            commands: vec![RenderCommand::Draw(DrawCommand {
-                id: DrawId(1),
-                phase: RenderPhase::Opaque,
-                object_id: None,
-                mesh: GpuMeshId(2),
-                material: GpuMaterialId(3),
-                transform: [0.0; 16],
-                range: IndexRange { start: 0, count: 0 },
-                stable_order: 4,
-            })],
+            commands: vec![
+                RenderCommand::BeginFrame,
+                RenderCommand::Draw(DrawCommand {
+                    id: DrawId(1),
+                    phase: RenderPhase::Opaque,
+                    object_id: None,
+                    mesh: GpuMeshId(2),
+                    material: GpuMaterialId(3),
+                    transform: [0.0; 16],
+                    range: IndexRange { start: 0, count: 0 },
+                    stable_order: 4,
+                }),
+                RenderCommand::EndFrame,
+            ],
         };
 
         assert!(matches!(
@@ -556,6 +788,178 @@ mod tests {
     }
 
     #[test]
+    fn command_validation_rejects_bad_frame_framing() {
+        let missing_begin = RenderCommandList {
+            commands: vec![RenderCommand::EndFrame],
+        };
+        assert!(matches!(
+            validate_command_list(&missing_begin),
+            Err(RenderError::InvalidCommandStream {
+                index: 0,
+                message: "first command must be BeginFrame"
+            })
+        ));
+
+        let nested = RenderCommandList {
+            commands: vec![
+                RenderCommand::BeginFrame,
+                RenderCommand::BeginFrame,
+                RenderCommand::EndFrame,
+            ],
+        };
+        assert!(matches!(
+            validate_command_list(&nested),
+            Err(RenderError::InvalidCommandStream {
+                index: 1,
+                message: "nested BeginFrame is not allowed"
+            })
+        ));
+    }
+
+    #[test]
+    fn command_validation_rejects_nonfinite_transform_and_range_overflow() {
+        let mut draw = snapshot_draw(10, RenderPhase::Opaque, 0, 10);
+        draw.transform[5] = f32::NAN;
+        let nonfinite = build_commands(
+            &RenderSnapshot {
+                camera: CameraSnapshot::default(),
+                draws: vec![draw],
+            },
+            RenderProfile::default(),
+        );
+        assert!(matches!(
+            nonfinite,
+            Err(RenderError::NonFiniteTransform {
+                draw_id: DrawId(10),
+                element: 5
+            })
+        ));
+
+        let list = RenderCommandList {
+            commands: vec![
+                RenderCommand::BeginFrame,
+                RenderCommand::Draw(DrawCommand {
+                    id: DrawId(11),
+                    phase: RenderPhase::Opaque,
+                    object_id: None,
+                    mesh: GpuMeshId(2),
+                    material: GpuMaterialId(3),
+                    transform: identity_transform(),
+                    range: IndexRange {
+                        start: u32::MAX,
+                        count: 1,
+                    },
+                    stable_order: 4,
+                }),
+                RenderCommand::EndFrame,
+            ],
+        };
+        assert!(matches!(
+            validate_command_list(&list),
+            Err(RenderError::IndexRangeOverflow {
+                draw_id: DrawId(11),
+                start: u32::MAX,
+                count: 1
+            })
+        ));
+    }
+
+    #[test]
+    fn command_validation_checks_order_and_resource_bounds() {
+        let ordered = build_commands(
+            &RenderSnapshot {
+                camera: CameraSnapshot::default(),
+                draws: vec![snapshot_draw(1, RenderPhase::Opaque, 0, 10)],
+            },
+            RenderProfile::default(),
+        )
+        .expect("commands");
+        assert!(matches!(
+            validate_command_list_with_limits(
+                &ordered,
+                RenderValidationLimits {
+                    mesh_count: Some(5),
+                    index_count: Some(16)
+                }
+            ),
+            Err(RenderError::MeshOutOfBounds {
+                draw_id: DrawId(1),
+                mesh: GpuMeshId(11),
+                mesh_count: 5
+            })
+        ));
+
+        let out_of_bounds = RenderCommandList {
+            commands: vec![
+                RenderCommand::BeginFrame,
+                RenderCommand::Draw(DrawCommand {
+                    id: DrawId(12),
+                    phase: RenderPhase::Opaque,
+                    object_id: None,
+                    mesh: GpuMeshId(2),
+                    material: GpuMaterialId(3),
+                    transform: identity_transform(),
+                    range: IndexRange {
+                        start: 14,
+                        count: 3,
+                    },
+                    stable_order: 4,
+                }),
+                RenderCommand::EndFrame,
+            ],
+        };
+        assert!(matches!(
+            validate_command_list_with_limits(
+                &out_of_bounds,
+                RenderValidationLimits {
+                    mesh_count: Some(5),
+                    index_count: Some(16)
+                }
+            ),
+            Err(RenderError::IndexRangeOutOfBounds {
+                draw_id: DrawId(12),
+                index_count: 16,
+                end: 17
+            })
+        ));
+
+        let unordered = RenderCommandList {
+            commands: vec![
+                RenderCommand::BeginFrame,
+                RenderCommand::Draw(DrawCommand {
+                    id: DrawId(1),
+                    phase: RenderPhase::Transparent,
+                    object_id: None,
+                    mesh: GpuMeshId(1),
+                    material: GpuMaterialId(1),
+                    transform: identity_transform(),
+                    range: IndexRange { start: 0, count: 3 },
+                    stable_order: 0,
+                }),
+                RenderCommand::Draw(DrawCommand {
+                    id: DrawId(2),
+                    phase: RenderPhase::Opaque,
+                    object_id: None,
+                    mesh: GpuMeshId(1),
+                    material: GpuMaterialId(1),
+                    transform: identity_transform(),
+                    range: IndexRange { start: 0, count: 3 },
+                    stable_order: 0,
+                }),
+                RenderCommand::EndFrame,
+            ],
+        };
+        assert!(matches!(
+            validate_command_list(&unordered),
+            Err(RenderError::PhaseOrderViolation {
+                draw_id: DrawId(2),
+                previous: RenderPhase::Transparent,
+                current: RenderPhase::Opaque
+            })
+        ));
+    }
+
+    #[test]
     fn render_error_display_is_actionable() {
         assert_eq!(
             RenderError::InvalidDrawRange {