From 50ae6ad70019d28fcc556c2ccc26051226d50c85 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 7 Oct 2024 13:20:52 -0700 Subject: [PATCH 1/5] fh apply: support requesting restricted tokens --- src/cli/cmd/apply/mod.rs | 94 +++++++++++++++++++++++++++++++++++----- src/cli/cmd/convert.rs | 2 +- src/cli/cmd/mod.rs | 14 ++++-- src/cli/cmd/resolve.rs | 5 ++- 4 files changed, 100 insertions(+), 15 deletions(-) diff --git a/src/cli/cmd/apply/mod.rs b/src/cli/cmd/apply/mod.rs index ed38695a..b4bc75a2 100644 --- a/src/cli/cmd/apply/mod.rs +++ b/src/cli/cmd/apply/mod.rs @@ -11,6 +11,7 @@ use std::{ use clap::{Parser, Subcommand}; use color_eyre::eyre::Context; use tempfile::{tempdir, TempDir}; +use tokio::io::AsyncWriteExt as _; use crate::cli::{cmd::nix_command, error::FhError}; @@ -24,11 +25,20 @@ pub(crate) struct ApplySubcommand { #[clap(subcommand)] system: System, + #[clap(long)] + profile_path: Option, + + #[clap(long, default_value_t = false)] + request_restricted_token: bool, + #[clap(from_global)] api_addr: url::Url, #[clap(from_global)] frontend_addr: url::Url, + + #[clap(from_global)] + cache_addr: url::Url, } #[derive(Subcommand)] @@ -77,7 +87,12 @@ impl CommandExecute for ApplySubcommand { tracing::info!("Resolving {}", output_ref); - let resolved_path = FlakeHubClient::resolve(self.api_addr.as_ref(), &output_ref).await?; + let resolved_path = FlakeHubClient::resolve( + self.api_addr.as_ref(), + &output_ref, + self.request_restricted_token, + ) + .await?; tracing::debug!( "Successfully resolved reference {} to path {}", @@ -85,8 +100,64 @@ impl CommandExecute for ApplySubcommand { &resolved_path.store_path ); + let profile_path = match &self.profile_path { + Some(path) => Some(path.to_owned()), + None => applyer.profile_path().map(ToOwned::to_owned), + }; + + match resolved_path.token { + Some(token) if self.request_restricted_token => { + let mut nix_args = vec![ + "copy".to_string(), + "--from".to_string(), + self.cache_addr.to_string(), + resolved_path.store_path.clone(), + ]; + + let dir = tempdir()?; + let temp_netrc_path = dir.path().join("netrc"); + + let mut f = tokio::fs::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .mode(0o600) + .open(&temp_netrc_path) + .await?; + + let cache_netrc_contents = format!( + "machine {} login flakehub password {}\n", + self.cache_addr.host_str().expect("valid host"), + token + ); + f.write_all(cache_netrc_contents.as_bytes()) + .await + .wrap_err("writing restricted netrc file")?; + + let display = temp_netrc_path.display().to_string(); + nix_args.extend_from_slice(&["--netrc-file".to_string(), display]); + + nix_command(&nix_args, false) + .await + .wrap_err("failed to copy resolved store path with Nix")?; + + dir.close()?; + } + None if self.request_restricted_token => { + return Err(color_eyre::eyre::eyre!( + "FlakeHub did not return a restricted token!" + )); + } + Some(_) if !self.request_restricted_token => { + tracing::warn!( + "Received a restricted token from FlakeHub, but we didn't request one! Ignoring." + ); + } + _ => (), + } + let (profile_path, _tempdir) = apply_path_to_profile( - applyer.profile_path(), + profile_path.as_deref(), &resolved_path.store_path, applyer.requires_root(), ) @@ -212,18 +283,21 @@ async fn apply_path_to_profile( nix_command( &[ - "build", + "build".to_string(), // Don't create a result symlink in the current directory for the profile being installed. // This is verified to not introduce a race condition against an eager garbage collection. - "--no-link", - "--print-build-logs", + "--no-link".to_string(), + "--print-build-logs".to_string(), // `--max-jobs 0` ensures that `nix build` doesn't really *build* anything // and acts more as a fetch operation - "--max-jobs", - "0", - "--profile", - profile_path.to_str().ok_or(FhError::InvalidProfile)?, - store_path, + "--max-jobs".to_string(), + "0".to_string(), + "--profile".to_string(), + profile_path + .to_str() + .ok_or(FhError::InvalidProfile)? + .to_string(), + store_path.to_string(), ], sudo_if_necessary, ) diff --git a/src/cli/cmd/convert.rs b/src/cli/cmd/convert.rs index 5d043a5e..1aa63bca 100644 --- a/src/cli/cmd/convert.rs +++ b/src/cli/cmd/convert.rs @@ -85,7 +85,7 @@ impl CommandExecute for ConvertSubcommand { tracing::debug!("Running: nix flake lock"); - nix_command(&["flake", "lock"], false) + nix_command(&["flake".to_string(), "lock".to_string()], false) .await .wrap_err("failed to create missing lock file entries")?; } diff --git a/src/cli/cmd/mod.rs b/src/cli/cmd/mod.rs index 2f3d6e7b..88055792 100644 --- a/src/cli/cmd/mod.rs +++ b/src/cli/cmd/mod.rs @@ -157,7 +157,11 @@ impl FlakeHubClient { Ok(res) } - async fn resolve(api_addr: &str, output_ref: &FlakeOutputRef) -> Result { + async fn resolve( + api_addr: &str, + output_ref: &FlakeOutputRef, + include_token: bool, + ) -> Result { let FlakeOutputRef { ref org, project: ref flake, @@ -165,7 +169,7 @@ impl FlakeHubClient { ref attr_path, } = output_ref; - let url = flakehub_url!( + let mut url = flakehub_url!( api_addr, "f", org, @@ -175,6 +179,10 @@ impl FlakeHubClient { attr_path ); + if include_token { + url.set_query(Some("include_token=true")); + } + get(url, true).await } @@ -386,7 +394,7 @@ fn is_root_user() -> bool { nix::unistd::getuid().is_root() } -async fn nix_command(args: &[&str], sudo_if_necessary: bool) -> Result<(), FhError> { +async fn nix_command(args: &[String], sudo_if_necessary: bool) -> Result<(), FhError> { command_exists("nix")?; let use_sudo = sudo_if_necessary && !is_root_user(); diff --git a/src/cli/cmd/resolve.rs b/src/cli/cmd/resolve.rs index 88853238..a4f55f96 100644 --- a/src/cli/cmd/resolve.rs +++ b/src/cli/cmd/resolve.rs @@ -29,6 +29,8 @@ pub(crate) struct ResolvedPath { attribute_path: String, // The resolved store path pub(crate) store_path: String, + // A JWT that can only substitute the closure of this store path + pub(crate) token: Option, } #[async_trait::async_trait] @@ -37,7 +39,8 @@ impl CommandExecute for ResolveSubcommand { async fn execute(self) -> color_eyre::Result { let output_ref = parse_flake_output_ref(&self.frontend_addr, &self.flake_ref)?; - let resolved_path = FlakeHubClient::resolve(self.api_addr.as_ref(), &output_ref).await?; + let resolved_path = + FlakeHubClient::resolve(self.api_addr.as_ref(), &output_ref, false).await?; tracing::debug!( "Successfully resolved reference {} to path {}", From 169001bf0f0a8e1d476d875b0068ec0eb5d2a6b2 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Tue, 8 Oct 2024 07:41:00 -0700 Subject: [PATCH 2/5] fixup: remove `--profile-path` flag In the future we may have an `fh apply profile` command where this could live. --- src/cli/cmd/apply/mod.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/cli/cmd/apply/mod.rs b/src/cli/cmd/apply/mod.rs index b4bc75a2..61316489 100644 --- a/src/cli/cmd/apply/mod.rs +++ b/src/cli/cmd/apply/mod.rs @@ -25,9 +25,6 @@ pub(crate) struct ApplySubcommand { #[clap(subcommand)] system: System, - #[clap(long)] - profile_path: Option, - #[clap(long, default_value_t = false)] request_restricted_token: bool, @@ -100,10 +97,7 @@ impl CommandExecute for ApplySubcommand { &resolved_path.store_path ); - let profile_path = match &self.profile_path { - Some(path) => Some(path.to_owned()), - None => applyer.profile_path().map(ToOwned::to_owned), - }; + let profile_path = applyer.profile_path(); match resolved_path.token { Some(token) if self.request_restricted_token => { @@ -157,7 +151,7 @@ impl CommandExecute for ApplySubcommand { } let (profile_path, _tempdir) = apply_path_to_profile( - profile_path.as_deref(), + profile_path, &resolved_path.store_path, applyer.requires_root(), ) From 94f01ebed9cf12008bdaf51a50457c123b6bdb00 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Tue, 8 Oct 2024 07:46:16 -0700 Subject: [PATCH 3/5] fixup: rename flag to `--use-scoped-token`, default to true --- src/cli/cmd/apply/mod.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/cli/cmd/apply/mod.rs b/src/cli/cmd/apply/mod.rs index 61316489..d5b78f90 100644 --- a/src/cli/cmd/apply/mod.rs +++ b/src/cli/cmd/apply/mod.rs @@ -25,8 +25,9 @@ pub(crate) struct ApplySubcommand { #[clap(subcommand)] system: System, - #[clap(long, default_value_t = false)] - request_restricted_token: bool, + /// Use a scoped token generated by FlakeHub that allows substituting the given output _only_. + #[clap(long, default_value_t = true)] + use_scoped_token: bool, #[clap(from_global)] api_addr: url::Url, @@ -84,12 +85,9 @@ impl CommandExecute for ApplySubcommand { tracing::info!("Resolving {}", output_ref); - let resolved_path = FlakeHubClient::resolve( - self.api_addr.as_ref(), - &output_ref, - self.request_restricted_token, - ) - .await?; + let resolved_path = + FlakeHubClient::resolve(self.api_addr.as_ref(), &output_ref, self.use_scoped_token) + .await?; tracing::debug!( "Successfully resolved reference {} to path {}", @@ -100,7 +98,7 @@ impl CommandExecute for ApplySubcommand { let profile_path = applyer.profile_path(); match resolved_path.token { - Some(token) if self.request_restricted_token => { + Some(token) if self.use_scoped_token => { let mut nix_args = vec![ "copy".to_string(), "--from".to_string(), @@ -137,12 +135,12 @@ impl CommandExecute for ApplySubcommand { dir.close()?; } - None if self.request_restricted_token => { + None if self.use_scoped_token => { return Err(color_eyre::eyre::eyre!( "FlakeHub did not return a restricted token!" )); } - Some(_) if !self.request_restricted_token => { + Some(_) if !self.use_scoped_token => { tracing::warn!( "Received a restricted token from FlakeHub, but we didn't request one! Ignoring." ); From dda44da8f64d8d27befa6cfece276aad6749bc4d Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Tue, 8 Oct 2024 08:32:19 -0700 Subject: [PATCH 4/5] fixup: add comment calling out the possibility of GC race --- src/cli/cmd/apply/mod.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/cli/cmd/apply/mod.rs b/src/cli/cmd/apply/mod.rs index d5b78f90..c47f3360 100644 --- a/src/cli/cmd/apply/mod.rs +++ b/src/cli/cmd/apply/mod.rs @@ -129,6 +129,29 @@ impl CommandExecute for ApplySubcommand { let display = temp_netrc_path.display().to_string(); nix_args.extend_from_slice(&["--netrc-file".to_string(), display]); + // NOTE(cole-h): Theoretically, this could be garbage collected immediately after we + // copy it. There's no good way to prevent this at this point in time because: + // + // 0. We want to be able to use the scoped token to talk to FlakeHub Cache, which we + // do via `--netrc-file`, and we want to be able to run this on any user -- trusted + // or otherwise + // + // 1. `nix copy` substitutes on the client, so `--netrc-file` works just fine (it + // won't be sent to the daemon, which will say "no" if you're not a trusted user), + // but it doesn't have a `--profile` or `--out-link` argument, so we can't GC + // root it that way + // + // 2. `nix build --max-jobs 0` does have `--profile` and `--out-link`, but passing + // `--netrc-file` will send it to the daemon which doesn't work if you're not a + // trusted user + // + // 3. Manually making a symlink somewhere doesn't work because adding that symlink + // to gcroots/auto requires root, stashing it in a process's environment is so ugly + // I will not entertain it, and holding a handle to it requires it to exist in the + // first place (so there's still a small window of time where it can be GC'd) + // + // This will be resolved when https://github.com/NixOS/nix/pull/11657 makes it into + // a Nix release. nix_command(&nix_args, false) .await .wrap_err("failed to copy resolved store path with Nix")?; From 5059db4822feccbf835ce8792209f18875f7fbd7 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Tue, 8 Oct 2024 09:00:52 -0700 Subject: [PATCH 5/5] fixup: move guard into match body --- src/cli/cmd/apply/mod.rs | 140 ++++++++++++++++++++------------------- 1 file changed, 71 insertions(+), 69 deletions(-) diff --git a/src/cli/cmd/apply/mod.rs b/src/cli/cmd/apply/mod.rs index c47f3360..e55c1144 100644 --- a/src/cli/cmd/apply/mod.rs +++ b/src/cli/cmd/apply/mod.rs @@ -98,77 +98,79 @@ impl CommandExecute for ApplySubcommand { let profile_path = applyer.profile_path(); match resolved_path.token { - Some(token) if self.use_scoped_token => { - let mut nix_args = vec![ - "copy".to_string(), - "--from".to_string(), - self.cache_addr.to_string(), - resolved_path.store_path.clone(), - ]; - - let dir = tempdir()?; - let temp_netrc_path = dir.path().join("netrc"); - - let mut f = tokio::fs::OpenOptions::new() - .create(true) - .truncate(true) - .write(true) - .mode(0o600) - .open(&temp_netrc_path) - .await?; - - let cache_netrc_contents = format!( - "machine {} login flakehub password {}\n", - self.cache_addr.host_str().expect("valid host"), - token - ); - f.write_all(cache_netrc_contents.as_bytes()) - .await - .wrap_err("writing restricted netrc file")?; - - let display = temp_netrc_path.display().to_string(); - nix_args.extend_from_slice(&["--netrc-file".to_string(), display]); - - // NOTE(cole-h): Theoretically, this could be garbage collected immediately after we - // copy it. There's no good way to prevent this at this point in time because: - // - // 0. We want to be able to use the scoped token to talk to FlakeHub Cache, which we - // do via `--netrc-file`, and we want to be able to run this on any user -- trusted - // or otherwise - // - // 1. `nix copy` substitutes on the client, so `--netrc-file` works just fine (it - // won't be sent to the daemon, which will say "no" if you're not a trusted user), - // but it doesn't have a `--profile` or `--out-link` argument, so we can't GC - // root it that way - // - // 2. `nix build --max-jobs 0` does have `--profile` and `--out-link`, but passing - // `--netrc-file` will send it to the daemon which doesn't work if you're not a - // trusted user - // - // 3. Manually making a symlink somewhere doesn't work because adding that symlink - // to gcroots/auto requires root, stashing it in a process's environment is so ugly - // I will not entertain it, and holding a handle to it requires it to exist in the - // first place (so there's still a small window of time where it can be GC'd) - // - // This will be resolved when https://github.com/NixOS/nix/pull/11657 makes it into - // a Nix release. - nix_command(&nix_args, false) - .await - .wrap_err("failed to copy resolved store path with Nix")?; - - dir.close()?; - } - None if self.use_scoped_token => { - return Err(color_eyre::eyre::eyre!( - "FlakeHub did not return a restricted token!" - )); + Some(token) => { + if self.use_scoped_token { + let mut nix_args = vec![ + "copy".to_string(), + "--from".to_string(), + self.cache_addr.to_string(), + resolved_path.store_path.clone(), + ]; + + let dir = tempdir()?; + let temp_netrc_path = dir.path().join("netrc"); + + let mut f = tokio::fs::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .mode(0o600) + .open(&temp_netrc_path) + .await?; + + let cache_netrc_contents = format!( + "machine {} login flakehub password {}\n", + self.cache_addr.host_str().expect("valid host"), + token + ); + f.write_all(cache_netrc_contents.as_bytes()) + .await + .wrap_err("writing restricted netrc file")?; + + let display = temp_netrc_path.display().to_string(); + nix_args.extend_from_slice(&["--netrc-file".to_string(), display]); + + // NOTE(cole-h): Theoretically, this could be garbage collected immediately after we + // copy it. There's no good way to prevent this at this point in time because: + // + // 0. We want to be able to use the scoped token to talk to FlakeHub Cache, which we + // do via `--netrc-file`, and we want to be able to run this on any user -- trusted + // or otherwise + // + // 1. `nix copy` substitutes on the client, so `--netrc-file` works just fine (it + // won't be sent to the daemon, which will say "no" if you're not a trusted user), + // but it doesn't have a `--profile` or `--out-link` argument, so we can't GC + // root it that way + // + // 2. `nix build --max-jobs 0` does have `--profile` and `--out-link`, but passing + // `--netrc-file` will send it to the daemon which doesn't work if you're not a + // trusted user + // + // 3. Manually making a symlink somewhere doesn't work because adding that symlink + // to gcroots/auto requires root, stashing it in a process's environment is so ugly + // I will not entertain it, and holding a handle to it requires it to exist in the + // first place (so there's still a small window of time where it can be GC'd) + // + // This will be resolved when https://github.com/NixOS/nix/pull/11657 makes it into + // a Nix release. + nix_command(&nix_args, false) + .await + .wrap_err("failed to copy resolved store path with Nix")?; + + dir.close()?; + } else { + tracing::warn!( + "Received a scoped token from FlakeHub, but we didn't request one! Ignoring." + ); + } } - Some(_) if !self.use_scoped_token => { - tracing::warn!( - "Received a restricted token from FlakeHub, but we didn't request one! Ignoring." - ); + None => { + if self.use_scoped_token { + return Err(color_eyre::eyre::eyre!( + "FlakeHub did not return a restricted token!" + )); + } } - _ => (), } let (profile_path, _tempdir) = apply_path_to_profile(