flutter/shorebird-updater
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: store patch signature on disk with patch metadata (#170)

Browse Source 
* feat: store patch signature on disk with patch metadata

* add note about explicit lifetime

* fix tests
This commit is contained in:
Bryan Oltman
2024-05-22 10:00:36 -04:00
committed by GitHub
parent 5de07bb48a
commit 4685789e59
7 changed files with 100 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Cargo.lock
Generated
+14 -42
View File
@@ -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]]
library/Cargo.toml
+1 -1
View File
@@ -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.
library/src/c_api/mod.rs
+2
View File
@@ -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,
}),
})
},
library/src/cache/patch_manager.rs
Vendored
+57 -16
View File
@@ -28,6 +28,9 @@ struct PatchMetadata {
/// The hash of the patch artifact on disk.
hash: String,
/// The signature of `hash`.
signature: Option<String>,
}
/// 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());
library/src/cache/updater_state.rs
Vendored
+19 -6
View File
@@ -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]
library/src/network.rs
+3
View File
@@ -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<String>,
}
/// Any edits to this struct should be made carefully and in accordance
library/src/updater.rs
+4 -1
View File
@@ -321,7 +321,7 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result<UpdateStatus> {
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");