feat(registry): ensure all elected versions can be parsed

rs/registry/canister/src/invariants/hostos_version.rs

@@ -5,6 +5,7 @@ use crate::invariants::common::{
 
 use ic_protobuf::registry::hostos_version::v1::HostosVersionRecord;
 use ic_registry_keys::make_hostos_version_key;
+use ic_types::hostos_version::HostosVersion;
 
 /// A predicate on the HostOS version records contained in a registry
 /// snapshot.
@@ -13,6 +14,7 @@ use ic_registry_keys::make_hostos_version_key;
 /// or that is contained in a HostosVersionRecord, the following is checked:
 ///
 /// * The corresponding HostosVersionRecord exists.
+/// * The version ID is well-formed, i.e. it can be parsed as a HostosVersion.
 /// * Each set URL is well-formed.
 /// * Each set hash is a well-formed hex-encoded SHA256 value.
 pub(crate) fn check_hostos_version_invariants(
@@ -31,6 +33,12 @@ pub(crate) fn check_hostos_version_invariants(
     all_versions.dedup();
 
     for version in all_versions {
+        // Enforce that the version ID is well-formed, so that consumers reading
+        // it back out of the Registry can turn it into a HostosVersion.
+        if let Err(err) = HostosVersion::try_from(version.as_str()) {
+            panic!("Registered an invalid HostOS version ID: {err}");
+        }
+
         // Check that every referenced version exists, i.e. we can only set a
         // Node's version to one that has already been added to the registry.
         let r = get_hostos_version_record(snapshot, version);
@@ -60,3 +68,47 @@ fn get_all_hostos_versions_of_nodes(snapshot: &RegistrySnapshot) -> Vec<String>
         .filter_map(|node_record| node_record.hostos_version_id)
         .collect()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use crate::common::test_helpers::invariant_compliant_registry;
+    use ic_registry_transport::{insert, pb::v1::RegistryMutation};
+    use prost::Message;
+
+    // HostOS version IDs are git commit IDs (pointing to the source code used
+    // to build HostOS).
+    const HOSTOS_VERSION_ID: &str = "eb3ab997954f2a91db8a42f84132cf37078d481c";
+
+    fn register_version_mutation(hostos_version_id: &str) -> Vec<RegistryMutation> {
+        vec![insert(
+            make_hostos_version_key(hostos_version_id).as_bytes(),
+            HostosVersionRecord {
+                release_package_urls: vec![],
+                release_package_sha256_hex: "".to_string(),
+                hostos_version_id: hostos_version_id.to_string(),
+            }
+            .encode_to_vec(),
+        )]
+    }
+
+    #[test]
+    fn no_panic_when_registering_valid_version() {
+        let registry = invariant_compliant_registry(0);
+
+        let mutations = register_version_mutation(HOSTOS_VERSION_ID);
+
+        registry.check_global_state_invariants(&mutations);
+    }
+
+    #[test]
+    #[should_panic(expected = "Registered an invalid HostOS version ID")]
+    fn panic_when_registering_version_with_illegal_characters() {
+        let registry = invariant_compliant_registry(0);
+
+        let mutations = register_version_mutation("G@RBAGE");
+
+        registry.check_global_state_invariants(&mutations);
+    }
+}

rs/registry/canister/src/invariants/replica_version.rs

@@ -20,6 +20,7 @@ use ic_registry_keys::{
     make_replica_version_key, make_standard_engine_replica_version_record_key,
     make_subnet_record_key, make_unassigned_nodes_config_record_key,
 };
+use ic_types::ReplicaVersion;
 use prost::Message;
 
 /// A predicate on the replica version records contained in a registry
@@ -30,6 +31,7 @@ use prost::Message;
 /// or that is used by the unassigned nodes, the following is checked:
 ///
 /// * The corresponding ReplicaVersionRecord exists.
+/// * The version ID is well-formed, i.e. it can be parsed as a ReplicaVersion.
 /// * Each URL is well-formed.
 /// * Release package hash is a well-formed hex-encoded SHA256 value.
 ///
@@ -66,6 +68,12 @@ pub(crate) fn check_replica_version_invariants(
     );
 
     for version in elected_set {
+        // Enforce that the version ID is well-formed, so that consumers reading
+        // it back out of the Registry can turn it into a ReplicaVersion.
+        if let Err(err) = ReplicaVersion::try_from(version.as_str()) {
+            panic!("Elected an invalid version ID: {err}");
+        }
+
         let r = get_replica_version_record(snapshot, &version);
 
         // Check whether release package URLs (update image) and corresponding hash are well-formed.
@@ -195,7 +203,6 @@ mod tests {
         GuestLaunchMeasurement, GuestLaunchMeasurementMetadata, GuestLaunchMeasurements,
     };
     use ic_registry_transport::{delete, insert, pb::v1::RegistryMutation, upsert};
-    use ic_types::ReplicaVersion;
     use prost::Message;
 
     const MOCK_HASH: &str = "C0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEED00D";
@@ -244,6 +251,27 @@ mod tests {
         registry.check_global_state_invariants(&mutations);
     }
 
+    #[test]
+    #[should_panic(expected = "Elected an invalid version ID")]
+    fn panic_when_electing_version_with_illegal_characters() {
+        let registry = invariant_compliant_registry(0);
+
+        let mutations = elect_version_mutations(vec!["G@RBAGE".into()]);
+
+        registry.check_global_state_invariants(&mutations);
+    }
+
+    #[test]
+    fn no_panic_when_electing_version_with_test_suffix() {
+        let registry = invariant_compliant_registry(0);
+
+        // Version IDs like this are used by system-tests, so they must remain
+        // acceptable.
+        let mutations = elect_version_mutations(vec![format!("{REPLICA_VERSION_ID_1}-test")]);
+
+        registry.check_global_state_invariants(&mutations);
+    }
+
     #[test]
     #[should_panic(expected = "Using a version that isn't elected.")]
     fn panic_when_using_unelected_version() {

rs/registry/canister/unreleased_changelog.md

@@ -12,6 +12,11 @@ on the process that this file is part of, see
 * Invariant requiring that SEV-enabled subnets may only run a GuestOS version that has
   `guest_launch_measurements`.
 
+* Invariant requiring that every elected GuestOS and HostOS version ID is well-formed,
+  i.e. that it consists only of alphanumeric characters, dots, dashes and underscores.
+  Such IDs are what `ReplicaVersion` and `HostosVersion` accept, so until now, it was
+  possible to elect a version that consumers could not read back out of the Registry.
+
 ## Changed
 
 ## Deprecated