Better sanitization of announced validator locations (#1844)

.github/workflows/e2e.yml

@@ -22,6 +22,9 @@ jobs:
   e2e:
     runs-on: ubuntu-latest
     steps:
+      - uses: actions/setup-node@v3
+        with:
+          node-version: 16
       - uses: actions/checkout@v3
         with:
           submodules: recursive

rust/agents/relayer/src/msg/metadata_builder.rs

@@ -12,6 +12,7 @@ use hyperlane_base::{
 use hyperlane_core::{
     HyperlaneChain, HyperlaneMessage, Mailbox, MultisigIsm, ValidatorAnnounce, H160, H256,
 };
+use std::str::FromStr;
 
 use crate::merkle_tree_builder::MerkleTreeBuilder;
 
@@ -138,7 +139,7 @@ impl MetadataBuilder {
         // Only use the most recently announced location for now.
         for (i, validator_storage_locations) in storage_locations.iter().enumerate() {
             for storage_location in validator_storage_locations.iter().rev() {
-                if let Some(conf) = CheckpointSyncerConf::from_storage_location(storage_location) {
+                if let Ok(conf) = CheckpointSyncerConf::from_str(storage_location) {
                     if let Ok(checkpoint_syncer) = conf.build(None) {
                         checkpoint_syncers
                             .insert(H160::from(validators[i]), checkpoint_syncer.into());

rust/hyperlane-base/src/types/checkpoint_syncer.rs

@@ -26,43 +26,54 @@ pub enum CheckpointSyncerConf {
     },
 }
 
-impl CheckpointSyncerConf {
-    /// Create a CheckpointSyncerConf from a storage location string
-    pub fn from_storage_location(storage_location: &str) -> Option<Self> {
-        let s3_prefix = "s3://";
-        let local_prefix = "file://";
-        if let Some(location) = storage_location.strip_prefix(s3_prefix) {
-            let pieces: Vec<&str> = location.split('/').collect();
-            if pieces.len() == 2 {
-                Some(CheckpointSyncerConf::S3 {
-                    bucket: pieces[0].into(),
-                    region: pieces[1].into(),
+/// Error for parsing announced storage locations
+#[derive(Debug, PartialEq, Eq)]
+pub struct ParseStorageLocationError;
+
+impl FromStr for CheckpointSyncerConf {
+    type Err = ParseStorageLocationError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let [prefix, suffix]: [&str; 2] = s
+            .split("://")
+            .collect::<Vec<_>>()
+            .try_into()
+            .map_err(|_| ParseStorageLocationError)?;
+
+        match prefix {
+            "s3" => {
+                let [bucket, region]: [&str; 2] = suffix
+                    .split('/')
+                    .collect::<Vec<_>>()
+                    .try_into()
+                    .map_err(|_| ParseStorageLocationError)?;
+                Ok(CheckpointSyncerConf::S3 {
+                    bucket: bucket.into(),
+                    region: region.into(),
                 })
-            } else {
-                None
             }
-        } else {
-            storage_location
-                .strip_prefix(local_prefix)
-                .map(|path| CheckpointSyncerConf::LocalStorage { path: path.into() })
+            "file" => Ok(CheckpointSyncerConf::LocalStorage {
+                path: suffix.into(),
+            }),
+            _ => Err(ParseStorageLocationError),
+        }
     }
 }
 
+impl CheckpointSyncerConf {
     /// Turn conf info a Checkpoint Syncer
     pub fn build(
         &self,
         latest_index_gauge: Option<IntGauge>,
     ) -> Result<Box<dyn CheckpointSyncer>, Report> {
-        match self {
+        Ok(match self {
             CheckpointSyncerConf::LocalStorage { path } => {
-                Ok(Box::new(LocalStorage::new(path, latest_index_gauge)))
+                Box::new(LocalStorage::new(path, latest_index_gauge))
             }
-            CheckpointSyncerConf::S3 { bucket, region } => Ok(Box::new(S3Storage::new(
-                bucket,
-                region.parse().expect("invalid s3 region"),
-                latest_index_gauge,
-            ))),
+            CheckpointSyncerConf::S3 { bucket, region } => {
+                Box::new(S3Storage::new(bucket, region.parse()?, latest_index_gauge))
             }
+        })
     }
 }
 
@@ -85,7 +96,11 @@ impl MultisigCheckpointSyncerConf {
         for (key, value) in self.checkpointsyncers.iter() {
             let gauge =
                 validator_checkpoint_index.with_label_values(&[origin, &key.to_lowercase()]);
-            checkpoint_syncers.insert(Address::from_str(key)?, value.build(Some(gauge))?.into());
+            if let Ok(conf) = value.build(Some(gauge)) {
+                checkpoint_syncers.insert(Address::from_str(key)?, conf.into());
+            } else {
+                continue;
+            }
         }
         Ok(MultisigCheckpointSyncer::new(checkpoint_syncers))
     }

typescript/infra/config/environments/mainnet2/agent.ts

@@ -30,7 +30,7 @@ export const hyperlane: AgentConfig<MainnetChains> = {
   context: Contexts.Hyperlane,
   docker: {
     repo: 'gcr.io/abacus-labs-dev/hyperlane-agent',
-    tag: '207563b-20230215-165009',
+    tag: '69c49a3-20230220-224405',
   },
   aws: {
     region: 'us-east-1',
@@ -74,7 +74,7 @@ export const releaseCandidate: AgentConfig<MainnetChains> = {
   context: Contexts.ReleaseCandidate,
   docker: {
     repo: 'gcr.io/abacus-labs-dev/hyperlane-agent',
-    tag: '207563b-20230215-165009',
+    tag: '69c49a3-20230220-224405',
   },
   aws: {
     region: 'us-east-1',

typescript/infra/config/environments/testnet3/agent.ts

@@ -30,7 +30,7 @@ export const hyperlane: AgentConfig<TestnetChains> = {
   context: Contexts.Hyperlane,
   docker: {
     repo: 'gcr.io/abacus-labs-dev/hyperlane-agent',
-    tag: '207563b-20230215-165009',
+    tag: '69c49a3-20230220-224405',
   },
   aws: {
     region: 'us-east-1',
@@ -71,7 +71,7 @@ export const releaseCandidate: AgentConfig<TestnetChains> = {
   context: Contexts.ReleaseCandidate,
   docker: {
     repo: 'gcr.io/abacus-labs-dev/hyperlane-agent',
-    tag: '207563b-20230215-165009',
+    tag: '69c49a3-20230220-224405',
   },
   aws: {
     region: 'us-east-1',