diff --git a/Cargo.lock b/Cargo.lock index 8524dee..83a5e83 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -44,6 +44,12 @@ dependencies = [ "once_cell", ] +[[package]] +name = "anstyle" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "038dfcf04a5feb68e9c60b21c9625a54c2c0616e79b72b0fd87075a056ae1d1b" + [[package]] name = "anyhow" version = "1.0.75" @@ -354,12 +360,6 @@ dependencies = [ "syn 1.0.109", ] -[[package]] -name = "difflib" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6184e33543162437515c2e2b48714794e37845ec9851711914eec9d308f6ebe8" - [[package]] name = "digest" version = "0.10.7" @@ -448,15 +448,6 @@ dependencies = [ "miniz_oxide", ] -[[package]] -name = "float-cmp" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "98de4bbd547a563b716d8dfa9aad1cb19bfab00f4fa09a6a4ed21dbcf44ce9c4" -dependencies = [ - "num-traits", -] - [[package]] name = "fnv" version = "1.0.7" @@ -772,15 +763,6 @@ version = "2.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3" -[[package]] -name = "itertools" -version = "0.10.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" -dependencies = [ - "either", -] - [[package]] name = "itoa" version = "1.0.10" @@ -892,9 +874,9 @@ dependencies = [ [[package]] name = "mockall" -version = "0.11.4" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c84490118f2ee2d74570d114f3d0493cbf02790df303d2707606c3e14e07c96" +checksum = "43766c2b5203b10de348ffe19f7e54564b64f3d6018ff7648d1e2d6d3a0f0a48" dependencies = [ "cfg-if", "downcast", @@ -907,14 +889,14 @@ dependencies = [ [[package]] name = "mockall_derive" -version = "0.11.4" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22ce75669015c4f47b289fd4d4f56e894e4c96003ffdf3ac51313126f94c6cbb" +checksum = "af7cbce79ec385a1d4f54baa90a76401eb15d9cab93685f62e7e9f942aa00ae2" dependencies = [ "cfg-if", "proc-macro2", "quote", - "syn 1.0.109", + "syn 2.0.41", ] [[package]] @@ -936,12 +918,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "normalize-line-endings" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" - [[package]] name = "num-traits" version = "0.2.17" @@ -1062,16 +1038,12 @@ checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" [[package]] name = "predicates" -version = "2.1.5" +version = "3.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59230a63c37f3e18569bdb90e4a89cbf5bf8b06fea0b84e65ea10cc4df47addd" +checksum = "68b87bfd4605926cdfefc1c3b5f8fe560e3feca9d5552cf68c466d3d8236c7e8" dependencies = [ - "difflib", - "float-cmp", - "itertools", - "normalize-line-endings", + "anstyle", "predicates-core", - "regex", ] [[package]] diff --git a/library/Cargo.toml b/library/Cargo.toml index 8d03195..537fd45 100644 --- a/library/Cargo.toml +++ b/library/Cargo.toml @@ -63,7 +63,7 @@ log-panics = { version = "2", features = ["with-backtrace"] } oslog = "0.2.0" [dev-dependencies] -mockall = "0.11.4" +mockall = "0.12.1" mockito = "1.2.0" # Gives #[serial] attribute for locking all of our shorebird_init # tests to a single thread so they don't conflict with each other. diff --git a/library/src/c_api/mod.rs b/library/src/c_api/mod.rs index c369dda..c014477 100644 --- a/library/src/c_api/mod.rs +++ b/library/src/c_api/mod.rs @@ -480,6 +480,7 @@ mod test { number: 1, hash: hash.to_owned(), download_url: "ignored".to_owned(), + signature: None, }), }) }, @@ -582,6 +583,7 @@ mod test { number: 1, hash: "ignored".to_owned(), download_url: "ignored".to_owned(), + signature: None, }), }) }, diff --git a/library/src/cache/patch_manager.rs b/library/src/cache/patch_manager.rs index adf6087..cf10c5e 100644 --- a/library/src/cache/patch_manager.rs +++ b/library/src/cache/patch_manager.rs @@ -28,6 +28,9 @@ struct PatchMetadata { /// The hash of the patch artifact on disk. hash: String, + + /// The signature of `hash`. + signature: Option, } /// What gets serialized to disk @@ -53,7 +56,17 @@ struct PatchesState { pub trait ManagePatches { /// Copies the patch file at file_path to the manager's directory structure sets /// this patch as the next patch to boot. - fn add_patch(&mut self, number: usize, file_path: &Path, hash: &str) -> Result<()>; + /// + /// The explicit lifetime is required for automock to work with Options. + /// See https://github.com/asomers/mockall/issues/61. + #[allow(clippy::needless_lifetimes)] + fn add_patch<'a>( + &mut self, + number: usize, + file_path: &Path, + hash: &str, + signature: Option<&'a str>, + ) -> Result<()>; /// Returns the patch we most recently successfully booted from (usually the currently running patch), /// or None if no patch is installed. @@ -314,7 +327,16 @@ impl PatchManager { } impl ManagePatches for PatchManager { - fn add_patch(&mut self, patch_number: usize, file_path: &Path, hash: &str) -> Result<()> { + // The explicit lifetime is required for automock to work with Options. + // See https://github.com/asomers/mockall/issues/61. + #[allow(clippy::needless_lifetimes)] + fn add_patch<'a>( + &mut self, + patch_number: usize, + file_path: &Path, + hash: &str, + signature: Option<&'a str>, + ) -> Result<()> { if !file_path.exists() { bail!("Patch file {} does not exist", file_path.display()); } @@ -330,6 +352,7 @@ impl ManagePatches for PatchManager { number: patch_number, size: std::fs::metadata(&patch_path)?.len(), hash: hash.to_owned(), + signature: signature.map(|s| s.to_owned()), }; // If a patch was never booted (next_boot_patch != last_booted_patch), we should delete @@ -456,7 +479,7 @@ impl PatchManager { .path() .join(format!("patch{}.vmcode", patch_number)); std::fs::write(file_path, patch_number.to_string().repeat(patch_number)).unwrap(); - self.add_patch(patch_number, file_path, "hash") + self.add_patch(patch_number, file_path, "hash", None) } } @@ -497,7 +520,12 @@ mod add_patch_tests { fn errs_if_file_path_does_not_exist() { let mut manager = PatchManager::manager_for_test(&TempDir::new("patch_manager").unwrap()); assert!(manager - .add_patch(1, Path::new("/path/to/file/that/does/not/exist"), "hash") + .add_patch( + 1, + Path::new("/path/to/file/that/does/not/exist"), + "hash", + None, + ) .is_err()); } @@ -512,7 +540,12 @@ mod add_patch_tests { std::fs::write(file_path, patch_file_contents).unwrap(); assert!(manager - .add_patch(patch_number, Path::new(file_path), "hash") + .add_patch( + patch_number, + Path::new(file_path), + "hash", + Some("my_signature") + ) .is_ok()); assert_eq!( @@ -521,6 +554,7 @@ mod add_patch_tests { number: patch_number, size: patch_file_contents.len() as u64, hash: "hash".to_string(), + signature: Some("my_signature".to_owned()) }) ); assert!(!file_path.exists()); @@ -538,19 +572,19 @@ mod add_patch_tests { // Add patch 1 let file_path = &temp_dir.path().join("patch.vmcode"); std::fs::write(file_path, patch_file_contents)?; - assert!(manager.add_patch(1, file_path, "hash").is_ok()); + assert!(manager.add_patch(1, file_path, "hash", None).is_ok()); assert_eq!(manager.highest_seen_patch_number(), Some(1)); // Add patch 4, expect 4 to be the highest patch number we've seen let file_path = &temp_dir.path().join("patch.vmcode"); std::fs::write(file_path, patch_file_contents)?; - assert!(manager.add_patch(4, file_path, "hash").is_ok()); + assert!(manager.add_patch(4, file_path, "hash", None).is_ok()); assert_eq!(manager.highest_seen_patch_number(), Some(4)); // Add patch 3, expect 4 to still be the highest patch number we've seen let file_path = &temp_dir.path().join("patch.vmcode"); std::fs::write(file_path, patch_file_contents)?; - assert!(manager.add_patch(3, file_path, "hash").is_ok()); + assert!(manager.add_patch(3, file_path, "hash", None).is_ok()); assert_eq!(manager.highest_seen_patch_number(), Some(4)); Ok(()) @@ -631,7 +665,7 @@ mod next_boot_patch_tests { let mut manager = PatchManager::with_root_dir(temp_dir.path().to_owned()); let file_path = &temp_dir.path().join("patch1.vmcode"); std::fs::write(file_path, patch_file_contents)?; - assert!(manager.add_patch(1, file_path, "hash").is_ok()); + assert!(manager.add_patch(1, file_path, "hash", None).is_ok()); // Write junk to the artifact, this should render the patch unbootable in the eyes // of the PatchManager. @@ -659,14 +693,14 @@ mod next_boot_patch_tests { std::fs::write(file_path, patch_file_contents)?; // Add patch 1, pretend it booted successfully. - assert!(manager.add_patch(1, file_path, "hash").is_ok()); + assert!(manager.add_patch(1, file_path, "hash", None).is_ok()); assert!(manager.record_boot_start_for_patch(1).is_ok()); assert!(manager.record_boot_success().is_ok()); // Add patch 2, pretend it failed to boot. let file_path = &temp_dir.path().join("patch2.vmcode"); std::fs::write(file_path, patch_file_contents)?; - assert!(manager.add_patch(2, file_path, "hash").is_ok()); + assert!(manager.add_patch(2, file_path, "hash", None).is_ok()); assert!(manager.record_boot_start_for_patch(2).is_ok()); assert!(manager.record_boot_failure_for_patch(2).is_ok()); @@ -685,14 +719,14 @@ mod next_boot_patch_tests { std::fs::write(file_path, patch_file_contents)?; // Add patch 1, pretend it booted successfully. - assert!(manager.add_patch(1, file_path, "hash").is_ok()); + assert!(manager.add_patch(1, file_path, "hash", None).is_ok()); assert!(manager.record_boot_start_for_patch(1).is_ok()); assert!(manager.record_boot_success().is_ok()); // Add patch 2, pretend it failed to boot. let file_path = &temp_dir.path().join("patch2.vmcode"); std::fs::write(file_path, patch_file_contents)?; - assert!(manager.add_patch(2, file_path, "hash").is_ok()); + assert!(manager.add_patch(2, file_path, "hash", None).is_ok()); assert!(manager.record_boot_start_for_patch(2).is_ok()); assert!(manager.record_boot_failure_for_patch(2).is_ok()); @@ -809,6 +843,7 @@ mod fall_back_tests { number: 1, size: 1, hash: "hash".to_string(), + signature: Some("signature".to_owned()), }); manager.try_fall_back_from_patch(1); @@ -942,7 +977,9 @@ mod record_boot_success_for_patch_tests { let mut manager = PatchManager::with_root_dir(temp_dir.path().to_owned()); let file_path = &temp_dir.path().join("patch1.vmcode"); std::fs::write(file_path, patch_file_contents)?; - assert!(manager.add_patch(patch_number, file_path, "hash").is_ok()); + assert!(manager + .add_patch(patch_number, file_path, "hash", None) + .is_ok()); assert!(manager.record_boot_success().is_err()); Ok(()) @@ -956,7 +993,9 @@ mod record_boot_success_for_patch_tests { let mut manager = PatchManager::with_root_dir(temp_dir.path().to_owned()); let file_path = &temp_dir.path().join("patch1.vmcode"); std::fs::write(file_path, patch_file_contents)?; - assert!(manager.add_patch(patch_number, file_path, "hash").is_ok()); + assert!(manager + .add_patch(patch_number, file_path, "hash", None) + .is_ok()); assert!(manager.record_boot_start_for_patch(1).is_ok()); assert!(manager.record_boot_success().is_ok()); @@ -974,7 +1013,9 @@ mod record_boot_success_for_patch_tests { std::fs::write(file_path, patch_file_contents)?; // Add the patch, make sure it has an artifact. - assert!(manager.add_patch(patch_number, file_path, "hash").is_ok()); + assert!(manager + .add_patch(patch_number, file_path, "hash", None) + .is_ok()); let patch_artifact_path = manager.patch_artifact_path(patch_number); assert!(patch_artifact_path.exists()); diff --git a/library/src/cache/updater_state.rs b/library/src/cache/updater_state.rs index 0cd1524..e9bd9b2 100644 --- a/library/src/cache/updater_state.rs +++ b/library/src/cache/updater_state.rs @@ -172,9 +172,14 @@ impl UpdaterState { /// Copies the patch file at file_path to the manager's directory structure sets /// this patch as the next patch to boot. - pub fn install_patch(&mut self, patch: &PatchInfo, hash: &str) -> anyhow::Result<()> { + pub fn install_patch( + &mut self, + patch: &PatchInfo, + hash: &str, + signature: Option<&str>, + ) -> anyhow::Result<()> { self.patch_manager - .add_patch(patch.number, &patch.path, hash) + .add_patch(patch.number, &patch.path, hash, signature) } /// Returns highest patch number that has been installed for this release. @@ -250,7 +255,7 @@ mod tests { let mut patch_manager = PatchManager::with_root_dir(tmp_dir.path().to_path_buf()); let file_path = &tmp_dir.path().join("patch1.vmcode"); std::fs::write(file_path, "patch file contents").unwrap(); - assert!(patch_manager.add_patch(1, file_path, "hash").is_ok()); + assert!(patch_manager.add_patch(1, file_path, "hash", None).is_ok()); let state = test_state(&tmp_dir, patch_manager); let release_version = state.serialized_state.release_version.clone(); @@ -356,13 +361,21 @@ mod tests { let tmp_dir = TempDir::new("example").unwrap(); let patch = fake_patch(&tmp_dir, patch_number); let mut mock_manage_patches = MockManagePatches::new(); + let cloned_patch = patch.clone(); mock_manage_patches .expect_add_patch() - .with(eq(patch.number), eq(patch.path.clone()), eq("hash")) - .returning(|_, __, ___| Ok(())); + .withf(move |number, path, hash, signature| { + number == &cloned_patch.number + && path == cloned_patch.path + && hash == "hash" + && signature == &Some("signature") + }) + .returning(|_, __, ___, ____| Ok(())); let mut state = test_state(&tmp_dir, mock_manage_patches); - assert!(state.install_patch(&patch, "hash").is_ok()); + assert!(state + .install_patch(&patch, "hash", Some("signature")) + .is_ok()); } #[test] diff --git a/library/src/network.rs b/library/src/network.rs index ab4b067..fab02da 100644 --- a/library/src/network.rs +++ b/library/src/network.rs @@ -146,6 +146,9 @@ pub struct Patch { pub hash: String, /// The URL to download the patch file from. pub download_url: String, + /// The signature of `hash`, if this patch is signed. None otherwise. + #[serde(default)] + pub signature: Option, } /// Any edits to this struct should be made carefully and in accordance diff --git a/library/src/updater.rs b/library/src/updater.rs index ad4f45e..4bbb3fc 100644 --- a/library/src/updater.rs +++ b/library/src/updater.rs @@ -321,7 +321,7 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result { let mut state = UpdaterState::load_or_new_on_error(&config.storage_dir, &config.release_version); // Move/state update should be "atomic" (it isn't today). - state.install_patch(&patch_info, &patch.hash)?; + state.install_patch(&patch_info, &patch.hash, patch.signature.as_deref())?; info!("Patch {} successfully installed.", patch.number); // Should set some state to say the status is "update required" and that // we now have a different "next" version of the app from the current @@ -591,6 +591,7 @@ mod tests { number: 1, }, "hash", + None, ) .expect("move failed"); state.save().expect("save failed"); @@ -711,6 +712,7 @@ mod tests { number: patch_number, }, "hash", + None, ) .expect("move failed"); state.save().expect("save failed"); @@ -755,6 +757,7 @@ mod tests { number: 1, }, "hash", + None, ) .expect("move failed"); state.save().expect("save failed");