From eeab2cac812b490592faa529058b1ce456778ac4 Mon Sep 17 00:00:00 2001 From: Manfred Endres Date: Fri, 6 Mar 2026 16:59:53 +0100 Subject: [PATCH] Fix panic on empty integrity string from live platform API ## Description Newer Unity versions return an empty string for the `integrity` field instead of `null`. `ssri::Integrity::from_str("")` silently produces an `Integrity` with zero hashes, causing `i.check()` to panic with an index-out-of-bounds error during checksum verification. ## Changes * ![FIX] Treat empty `integrity` string as `None` in `deserialize_sri` via `Some(s) if s.is_empty()` guard * ![ADD] Unit tests for `deserialize_sri` covering empty string, null, missing field, and valid SRI --- .../.openspec.yaml | 2 + .../design.md | 25 +++++++++++++ .../proposal.md | 20 ++++++++++ .../specs/integrity-deserialization/spec.md | 20 ++++++++++ .../tasks.md | 16 ++++++++ .../specs/integrity-deserialization/spec.md | 20 ++++++++++ uvm_live_platform/src/model/release.rs | 37 +++++++++++++++++++ 7 files changed, 140 insertions(+) create mode 100644 openspec/changes/archive/2026-03-06-fix-integrity-empty-string/.openspec.yaml create mode 100644 openspec/changes/archive/2026-03-06-fix-integrity-empty-string/design.md create mode 100644 openspec/changes/archive/2026-03-06-fix-integrity-empty-string/proposal.md create mode 100644 openspec/changes/archive/2026-03-06-fix-integrity-empty-string/specs/integrity-deserialization/spec.md create mode 100644 openspec/changes/archive/2026-03-06-fix-integrity-empty-string/tasks.md create mode 100644 openspec/specs/integrity-deserialization/spec.md diff --git a/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/.openspec.yaml b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/.openspec.yaml new file mode 100644 index 00000000..3184e5ab --- /dev/null +++ b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-03-06 diff --git a/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/design.md b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/design.md new file mode 100644 index 00000000..2bae12dd --- /dev/null +++ b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/design.md @@ -0,0 +1,25 @@ +## Context + +`ssri::Integrity::from_str("")` does not return an error — it silently produces an `Integrity` value with an empty hash list. The existing `deserialize_sri` function only guards against `None` and parse errors, so an empty string passes through as `Some(Integrity{hashes: []})`. When `verify_checksum` calls `i.check(...)` on that value, `ssri` panics with an index-out-of-bounds. + +The affected code path: +1. Live platform API returns `"integrity": ""` for newer Unity versions +2. `deserialize_sri` calls `Integrity::from_str("")` → succeeds, zero hashes +3. `verify_checksum` receives `Some(empty_integrity)`, calls `i.check(&bytes)` → panic + +## Goals / Non-Goals + +**Goals:** +- Treat `""` as absent integrity (same as `null`) in `deserialize_sri` + +**Non-Goals:** +- Changing checksum verification logic +- Handling other malformed integrity values differently than today + +## Decisions + +### Guard empty string before `from_str` + +Add `if s.is_empty() { return Ok(None); }` in `deserialize_sri` before the `Integrity::from_str` call. This is the earliest, safest point to intercept the bad value — it requires no changes to `verify_checksum` or callers, and keeps the existing `Err(_) => Ok(None)` fallback for other malformed values. + +**Alternative considered**: Validate the parsed `Integrity` after construction (e.g., check `hashes.is_empty()`). Rejected — requires knowledge of `ssri` internals and is more fragile than rejecting the empty input directly. diff --git a/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/proposal.md b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/proposal.md new file mode 100644 index 00000000..3ad62eec --- /dev/null +++ b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/proposal.md @@ -0,0 +1,20 @@ +## Why + +Newer Unity versions return an empty string (`""`) for the `integrity` field in the live platform API instead of `null`. `ssri::Integrity::from_str("")` parses successfully and produces an `Integrity` with zero hashes. When `verify_checksum` later calls `i.check(...)` on that value, `ssri` panics with an index-out-of-bounds error because it assumes at least one hash is present. This crash prevents installation of any Unity version that returns an empty integrity string. + +## What Changes + +- Treat an empty `integrity` string as absent (`None`) in `deserialize_sri`, so that `verify_checksum` receives `None` and returns `CheckSumResult::NoCheckSum` instead of panicking + +## Capabilities + +### New Capabilities + + +### Modified Capabilities +- `payload-format-detection`: the deserialization of the `integrity` field must now handle empty-string values in addition to `null` and valid SRI strings + +## Impact + +- **`uvm_live_platform/src/model/release.rs`**: one-line guard in `deserialize_sri` — skip `from_str` and return `Ok(None)` when the string is empty +- No behaviour change for `null` or valid integrity values; installations without a checksum continue to be skipped gracefully diff --git a/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/specs/integrity-deserialization/spec.md b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/specs/integrity-deserialization/spec.md new file mode 100644 index 00000000..fdf7afe0 --- /dev/null +++ b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/specs/integrity-deserialization/spec.md @@ -0,0 +1,20 @@ +## ADDED Requirements + +### Requirement: Empty integrity string is treated as absent +The `integrity` field deserializer SHALL treat an empty string (`""`) as equivalent to `null`, producing `None` rather than a parsed `Integrity` value with zero hashes. + +#### Scenario: Empty string produces None +- **WHEN** the live platform API returns `"integrity": ""` +- **THEN** the deserialized `integrity` field SHALL be `None` + +#### Scenario: Checksum verification is skipped for empty integrity +- **WHEN** the `integrity` field deserializes to `None` due to an empty string +- **THEN** `verify_checksum` SHALL return `CheckSumResult::NoCheckSum` without panicking + +#### Scenario: Null integrity still produces None +- **WHEN** the live platform API returns `"integrity": null` +- **THEN** the deserialized `integrity` field SHALL be `None` (existing behaviour preserved) + +#### Scenario: Valid integrity string still parses correctly +- **WHEN** the live platform API returns a valid SRI string (e.g. `"sha256-abc123..."`) +- **THEN** the deserialized `integrity` field SHALL be `Some(Integrity)` (existing behaviour preserved) diff --git a/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/tasks.md b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/tasks.md new file mode 100644 index 00000000..9a5245e7 --- /dev/null +++ b/openspec/changes/archive/2026-03-06-fix-integrity-empty-string/tasks.md @@ -0,0 +1,16 @@ +## 1. Fix + +- [x] 1.1 In `uvm_live_platform/src/model/release.rs`, add an early return in `deserialize_sri`: if the deserialized string is empty, return `Ok(None)` before calling `Integrity::from_str` + +## 2. Tests + +- [x] 2.1 Add a `#[cfg(test)]` module to `uvm_live_platform/src/model/release.rs` with unit tests for `deserialize_sri`: + - `empty_string_integrity_deserializes_to_none` — deserialize `{"integrity": ""}` into `UnityReleaseFile`, assert `integrity` is `None` + - `null_integrity_deserializes_to_none` — deserialize `{"integrity": null}`, assert `integrity` is `None` + - `missing_integrity_field_deserializes_to_none` — deserialize `{}` (field absent), assert `integrity` is `None` + - `valid_integrity_deserializes_to_some` — deserialize a valid SRI string, assert `integrity` is `Some` + +## 3. Verify + +- [x] 3.1 Run `cargo test -p uvm_live_platform` and confirm all tests pass +- [x] 3.2 Run `cargo clippy -p uvm_live_platform` and confirm no new warnings diff --git a/openspec/specs/integrity-deserialization/spec.md b/openspec/specs/integrity-deserialization/spec.md new file mode 100644 index 00000000..a04fde3f --- /dev/null +++ b/openspec/specs/integrity-deserialization/spec.md @@ -0,0 +1,20 @@ +## Requirements + +### Requirement: Empty integrity string is treated as absent +The `integrity` field deserializer SHALL treat an empty string (`""`) as equivalent to `null`, producing `None` rather than a parsed `Integrity` value with zero hashes. + +#### Scenario: Empty string produces None +- **WHEN** the live platform API returns `"integrity": ""` +- **THEN** the deserialized `integrity` field SHALL be `None` + +#### Scenario: Checksum verification is skipped for empty integrity +- **WHEN** the `integrity` field deserializes to `None` due to an empty string +- **THEN** `verify_checksum` SHALL return `CheckSumResult::NoCheckSum` without panicking + +#### Scenario: Null integrity still produces None +- **WHEN** the live platform API returns `"integrity": null` +- **THEN** the deserialized `integrity` field SHALL be `None` (existing behaviour preserved) + +#### Scenario: Valid integrity string still parses correctly +- **WHEN** the live platform API returns a valid SRI string (e.g. `"sha256-abc123..."`) +- **THEN** the deserialized `integrity` field SHALL be `Some(Integrity)` (existing behaviour preserved) diff --git a/uvm_live_platform/src/model/release.rs b/uvm_live_platform/src/model/release.rs index 9f4e8b09..a70186fe 100644 --- a/uvm_live_platform/src/model/release.rs +++ b/uvm_live_platform/src/model/release.rs @@ -127,6 +127,7 @@ where let sri_str: Option = Option::deserialize(deserializer)?; match sri_str { + Some(s) if s.is_empty() => Ok(None), Some(s) => match Integrity::from_str(&s) { Ok(integrity) => Ok(Some(integrity)), Err(_) => Ok(None), // If parsing fails (e.g., MD5 hash), ignore it @@ -165,3 +166,39 @@ impl<'a> Iterator for ModuleIterator<'a> { Some(next) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn deserialize_integrity(json: &str) -> Option { + let file: UnityReleaseFile = serde_json::from_str(json).unwrap(); + file.integrity + } + + #[test] + fn empty_string_integrity_deserializes_to_none() { + let result = deserialize_integrity(r#"{"integrity": ""}"#); + assert!(result.is_none()); + } + + #[test] + fn null_integrity_deserializes_to_none() { + let result = deserialize_integrity(r#"{"integrity": null}"#); + assert!(result.is_none()); + } + + #[test] + fn missing_integrity_field_deserializes_to_none() { + let result = deserialize_integrity(r#"{}"#); + assert!(result.is_none()); + } + + #[test] + fn valid_integrity_deserializes_to_some() { + let result = deserialize_integrity( + r#"{"integrity": "sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU="}"#, + ); + assert!(result.is_some()); + } +}