From 652c4dfa230acf38cb77d94893c3001772822e8c Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Sat, 21 Jan 2023 17:15:15 -0500 Subject: [PATCH] Trust check & installation improvements (#38) * Split trust prompting from trust check * Split trust check from install when installing multiple tools * Implement --skip-untrusted flag for `aftman install` * Also let installation install all trusted tools before bailing * Update README --- Cargo.lock | 5 ++-- Cargo.toml | 1 + README.md | 4 +++- src/cli/mod.rs | 5 +++- src/tool_storage.rs | 57 ++++++++++++++++++++++++++++++++++----------- src/trust.rs | 6 +++++ 6 files changed, 60 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 480a7ca..14db2bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,6 +21,7 @@ dependencies = [ "env_logger", "fs-err", "insta", + "itertools", "log", "once_cell", "reqwest", @@ -579,9 +580,9 @@ checksum = "879d54834c8c76457ef4293a689b2a8c59b076067ad77b15efafbb05f92a592b" [[package]] name = "itertools" -version = "0.10.3" +version = "0.10.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9a9d19fa1e79b6215ff29b9d6880b706147f16e9b1dbb1e4e5947b5b02bc5e3" +checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" dependencies = [ "either", ] diff --git a/Cargo.toml b/Cargo.toml index 2013701..4457439 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ dialoguer = "0.9.0" dirs = "3.0.2" env_logger = "0.9.0" fs-err = "2.6.0" +itertools = "0.10.5" log = "0.4.14" once_cell = "1.9.0" reqwest = { version = "0.11.4", features = ["blocking"] } diff --git a/README.md b/README.md index 1740f0a..0797034 100644 --- a/README.md +++ b/README.md @@ -123,13 +123,15 @@ aftman add rojo-rbx/rojo@6.2.0 rojo6 Usage: ```bash -aftman install [--no-trust-check] +aftman install [--no-trust-check] [--skip-untrusted] ``` Install all tools listed in `aftman.toml` files based on your current directory. If `--no-trust-check` is given, all tools will be installed, regardless of whether they are known. This should generally only be used in CI environments. To trust a specific tool before running `aftman install`, use `aftman trust ` instead. +If `--skip-untrusted` is given, only already trusted tools will be installed, others will be skipped and not emit any errors. + ### `aftman self-install` Usage: diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 3430b56..ff2bafa 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -155,6 +155,9 @@ pub struct InstallSubcommand { /// recommended to only run this on CI machines. #[clap(long)] pub no_trust_check: bool, + /// Skip / don't error if a tool was not trusted during install. + #[clap(long)] + pub skip_untrusted: bool, } impl InstallSubcommand { @@ -165,7 +168,7 @@ impl InstallSubcommand { TrustMode::Check }; - tools.install_all(trust) + tools.install_all(trust, self.skip_untrusted) } } diff --git a/src/tool_storage.rs b/src/tool_storage.rs index cf5ec27..32ed2ce 100644 --- a/src/tool_storage.rs +++ b/src/tool_storage.rs @@ -9,6 +9,7 @@ use std::path::{Path, PathBuf}; use anyhow::{bail, Context}; use fs_err::File; +use itertools::{Either, Itertools}; use once_cell::unsync::OnceCell; use crate::auth::AuthManifest; @@ -19,7 +20,7 @@ use crate::tool_id::ToolId; use crate::tool_name::ToolName; use crate::tool_source::{Asset, GitHubSource, Release}; use crate::tool_spec::ToolSpec; -use crate::trust::{TrustCache, TrustMode}; +use crate::trust::{TrustCache, TrustMode, TrustStatus}; pub struct ToolStorage { pub storage_dir: PathBuf, @@ -135,15 +136,35 @@ impl ToolStorage { } /// Install all tools from all reachable manifest files. - pub fn install_all(&self, trust: TrustMode) -> anyhow::Result<()> { + pub fn install_all(&self, trust: TrustMode, skip_untrusted: bool) -> anyhow::Result<()> { let current_dir = current_dir().context("Failed to get current working directory")?; let manifests = Manifest::discover(&self.home, ¤t_dir)?; - for manifest in manifests { - for (alias, tool_id) in manifest.tools { - self.install_exact(&tool_id, trust)?; - self.link(&alias)?; - } + // Installing all tools is split into multiple steps: + // 1. Trust check, which may prompt the user and yield if untrusted + // 2. Installation of trusted tools, which will be in parallel in the future + // 3. Reporting of installation trust errors, unless trust errors are skipped + + let (trusted_tools, trust_errors): (Vec<_>, Vec<_>) = manifests + .iter() + .flat_map(|manifest| &manifest.tools) + .partition_map( + |(alias, tool_id)| match self.trust_check(tool_id.name(), trust) { + Ok(_) => Either::Left((alias, tool_id)), + Err(e) => Either::Right(e), + }, + ); + + for (alias, tool_id) in &trusted_tools { + self.install_exact(tool_id, trust)?; + self.link(alias)?; + } + + if !trust_errors.is_empty() && !skip_untrusted { + bail!( + "Installation trust check failed for the following tools:\n{}", + trust_errors.iter().map(|e| format!(" {e}")).join("\n") + ) } Ok(()) @@ -320,10 +341,9 @@ impl ToolStorage { } fn trust_check(&self, name: &ToolName, mode: TrustMode) -> anyhow::Result<()> { - let trusted = TrustCache::read(&self.home)?; - let is_trusted = trusted.tools.contains(name); + let status = self.trust_status(name)?; - if !is_trusted { + if status == TrustStatus::NotTrusted { if mode == TrustMode::Check { // If the terminal isn't interactive, tell the user that they // need to open an interactive terminal to trust this tool. @@ -344,11 +364,10 @@ impl ToolStorage { .interact_opt()?; if let Some(false) | None = proceed { - eprintln!( - "Skipping installation of {} and exiting with an error.", - name + bail!( + "Tool {name} is not trusted. \ + Run `aftman trust {name}` in your terminal to trust it.", ); - std::process::exit(1); } } @@ -358,6 +377,16 @@ impl ToolStorage { Ok(()) } + fn trust_status(&self, name: &ToolName) -> anyhow::Result { + let trusted = TrustCache::read(&self.home)?; + let is_trusted = trusted.tools.contains(name); + if is_trusted { + Ok(TrustStatus::Trusted) + } else { + Ok(TrustStatus::NotTrusted) + } + } + fn install_executable(&self, id: &ToolId, mut contents: impl Read) -> anyhow::Result<()> { let output_path = self.exe_path(id); diff --git a/src/trust.rs b/src/trust.rs index 3ec0a86..a670933 100644 --- a/src/trust.rs +++ b/src/trust.rs @@ -13,6 +13,12 @@ pub enum TrustMode { NoCheck, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TrustStatus { + Trusted, + NotTrusted, +} + #[derive(Debug)] pub struct TrustCache { pub tools: BTreeSet,