scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-07 03:00:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

refactor(install): share persistence pipeline (#332)

Browse source 
## Summary

Unifies installer persistence so CLI and web install paths share the
same file/env/vault primitives, while preserving the CLI's server-API
secret persistence and auth bootstrap behavior.

## What Changed

- Added shared `fabro-install` config writers for installer-owned tagged
enum tables, replacing `server.listen` and `cli.target` atomically so
stale variant fields cannot survive.
- Added `InstallPersistencePlan` for disk-backed settings, server env,
and vault writes/removals with the existing rollback semantics for
settings and vault failures.
- Refactored `fabro install`, `fabro install github`, and
`/install/finish` to use the shared persistence plan where their disk
behavior overlaps.
- Preserved full-install ordering: settings/env first, workflow-visible
secrets through the server API second, and CLI `auth.json` only after
API secret persistence succeeds.
- Preserved web installer failure response fields for leftover and
removed env keys, plus the post-success finish hook/shutdown behavior.

## Test Plan

- `cargo nextest run -p fabro-install`
- `cargo nextest run -p fabro-cli commands::install::tests`
- `cargo nextest run -p fabro-cli --test it cmd::install`
- `cargo nextest run -p fabro-server --features test-support --test it
api::install`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `git diff --check`

---

[![Compound
Engineering](https://img.shields.io/badge/Compound_Engineering-6366f1)](https://github.com/EveryInc/compound-engineering-plugin)
🤖 Generated with GPT-5 via [Codex](https://openai.com/codex)
This commit is contained in:
Bryan Helmkamp 2026-05-21 08:04:04 -04:00 • committed by GitHub
parent 49449f7a07
commit 9837a08929
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 1405 additions and 239 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -2002,6 +2002,7 @@ dependencies = [
"fabro-config",
"fabro-static",
"fabro-types",
"fabro-util",
"fabro-vault",
"ring",
"tempfile",

381
lib/crates/fabro-cli/src/commands/install.rs
View file

@ -26,10 +26,12 @@ use fabro_client::{AuthEntry, AuthStore, DevTokenEntry, ServerTarget};
use fabro_config::bind::Bind;
use fabro_config::daemon::ServerDaemon;
use fabro_config::user::{SETTINGS_CONFIG_FILENAME, default_storage_dir};
use fabro_config::{Storage, envfile};
use fabro_config::{Storage, UserSettingsBuilder, envfile};
use fabro_install::{
InstallListenConfig, PendingSettingsWrite, merge_server_settings as merge_server_settings_impl,
persist_install_outputs_direct, write_github_app_settings, write_token_settings,
InstallListenConfig, InstallPersistencePlan, PendingDevTokenWrite, PendingSettingsWrite,
VaultSecretWrite, merge_server_settings as merge_server_settings_impl,
prepare_dev_token_write_for_install, restore_optional_file, rollback_dev_token_write,
write_github_app_settings, write_token_settings,
};
use fabro_model::catalog::CatalogProvider;
use fabro_model::{Catalog, CredentialRef, ProviderId};
@ -42,7 +44,7 @@ use fabro_util::printer::Printer;
use fabro_util::terminal::Styles;
use fabro_util::version::FABRO_VERSION;
use fabro_util::{browser, dev_token, path, session_secret};
use fabro_vault::{SecretType as VaultSecretType, Vault};
use fabro_vault::SecretType as VaultSecretType;
use futures::future::BoxFuture;
use rand::Rng;
use tokio::net::TcpListener;
@ -65,12 +67,12 @@ use crate::shared::provider_auth::{
ApiKeySource, authenticate_provider, authenticate_provider_with_api_key_source,
authenticate_provider_with_method, prompt_confirm, prompt_password, provider_display_name,
};
use crate::{local_server, server_client};
use crate::{local_server, server_client, user_config};
const GITHUB_TOKEN_SECRET_KEY: &str = "GITHUB_TOKEN";
const GITHUB_APP_PRIVATE_KEY_KEY: &str = "GITHUB_APP_PRIVATE_KEY";
const GITHUB_APP_CLIENT_SECRET_KEY: &str = "GITHUB_APP_CLIENT_SECRET";
const GITHUB_APP_WEBHOOK_SECRET_KEY: &str = "GITHUB_APP_WEBHOOK_SECRET";
const GITHUB_TOKEN_SECRET_KEY: &str = fabro_static::EnvVars::GITHUB_TOKEN;
const GITHUB_APP_PRIVATE_KEY_KEY: &str = fabro_static::EnvVars::GITHUB_APP_PRIVATE_KEY;
const GITHUB_APP_CLIENT_SECRET_KEY: &str = fabro_static::EnvVars::GITHUB_APP_CLIENT_SECRET;
const GITHUB_APP_WEBHOOK_SECRET_KEY: &str = fabro_static::EnvVars::GITHUB_APP_WEBHOOK_SECRET;
static INSTALL_CATALOG: LazyLock<Catalog> = LazyLock::new(|| {
Catalog::from_builtin().expect("embedded install model catalog should be valid")
@ -1261,15 +1263,32 @@ async fn persist_install_outputs(
server_env_secrets: &[(String, String)],
vault_secrets: &[CreateSecretRequest],
settings_write: Option<PendingSettingsWrite<'_>>,
dev_token_write: Option<PendingDevTokenWrite>,
server_was_running: bool,
bootstrap_dev_token: Option<&str>,
) -> Result<()> {
persist_install_outputs_with_settings(
let bootstrap_dev_token = if server_was_running {
None
} else {
bootstrap_dev_token.map(str::to_string)
};
persist_cli_install_outputs_with(
storage_dir,
server_env_secrets,
server_env_updates(server_env_secrets),
Vec::new(),
vault_secrets,
settings_write,
dev_token_write,
server_was_running,
|path| Box::pin(server_client::connect_server(path)),
move |path| {
let bootstrap_dev_token = bootstrap_dev_token.clone();
Box::pin(async move {
match bootstrap_dev_token {
Some(token) => server_client::connect_server_with_dev_token(path, &token).await,
None => server_client::connect_server(path).await,
}
})
},
|path, timeout| {
Box::pin(async move { stop::stop_server(path, timeout).await.unwrap_or(false) })
},
@ -1285,76 +1304,48 @@ struct PendingGitHubInstallWrite<'a> {
vault_remove: Vec<&'static str>,
}
fn restore_optional_file(path: &Path, previous_contents: Option<&str>) -> Result<()> {
match previous_contents {
Some(contents) => {
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent)
.with_context(|| format!("creating directory {}", parent.display()))?;
}
std::fs::write(path, contents)
.with_context(|| format!("restoring {}", path.display()))?;
}
None => match std::fs::remove_file(path) {
Ok(()) => {}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => {
return Err(anyhow::Error::new(err).context(format!("removing {}", path.display())));
}
},
}
Ok(())
}
fn persist_github_install_changes(
storage_dir: &Path,
writes: &PendingGitHubInstallWrite<'_>,
) -> Result<()> {
let storage = Storage::new(storage_dir);
let server_env_path = storage.runtime_directory().env_path();
let vault_path = storage.secrets_path();
let server_env_path = Storage::new(storage_dir).runtime_directory().env_path();
let previous_server_env = std::fs::read_to_string(&server_env_path).ok();
let previous_vault = std::fs::read_to_string(&vault_path).ok();
let result = (|| -> Result<()> {
let server_env_writes = server_env_updates(&writes.server_env_set);
let server_env_removals = server_env_removals(&writes.server_env_remove);
persist_install_outputs_direct(
storage_dir,
&server_env_writes,
&server_env_removals,
&[],
Some(&writes.settings_write),
)?;
let mut vault = Vault::load(vault_path.clone()).map_err(anyhow::Error::from)?;
for key in &writes.vault_remove {
match vault.remove(key) {
Ok(()) | Err(fabro_vault::Error::NotFound(_)) => {}
Err(err) => return Err(err.into()),
match (InstallPersistencePlan {
storage_dir,
settings_write: Some(writes.settings_write),
server_env_writes: server_env_updates(&writes.server_env_set),
server_env_removals: server_env_removals(&writes.server_env_remove),
dev_token_write: None,
vault_writes: writes
.vault_set
.iter()
.map(|(key, value)| VaultSecretWrite {
name: key.clone(),
value: value.clone(),
secret_type: VaultSecretType::Token,
description: None,
})
.collect(),
vault_removals: writes
.vault_remove
.iter()
.map(|key| (*key).to_string())
.collect(),
}
.persist_direct())
{
Ok(()) => Ok(()),
Err(err) => {
let err = anyhow::Error::from(err);
match restore_optional_file(&server_env_path, previous_server_env.as_deref()) {
Ok(()) => Err(err),
Err(restore_err) => {
Err(err.context(format!("server env rollback failure: {restore_err}")))
}
}
}
for (key, value) in &writes.vault_set {
vault
.set(key, value, VaultSecretType::Token, None)
.map_err(anyhow::Error::from)?;
}
Ok(())
})();
if let Err(err) = result {
restore_optional_file(
writes.settings_write.path,
writes.settings_write.previous_contents,
)?;
restore_optional_file(&server_env_path, previous_server_env.as_deref())?;
restore_optional_file(&vault_path, previous_vault.as_deref())?;
return Err(err);
}
Ok(())
}
async fn write_artifact_store_metadata(
@ -1367,25 +1358,30 @@ async fn write_artifact_store_metadata(
Ok(())
}
async fn persist_install_outputs_with_settings(
async fn persist_cli_install_outputs_with(
storage_dir: &Path,
server_env_secrets: &[(String, String)],
server_env_writes: Vec<envfile::EnvFileUpdate>,
server_env_removals: Vec<envfile::EnvFileRemoval>,
vault_secrets: &[CreateSecretRequest],
settings_write: Option<PendingSettingsWrite<'_>>,
dev_token_write: Option<PendingDevTokenWrite>,
server_was_running: bool,
connect_server: impl for<'a> Fn(&'a Path) -> BoxFuture<'a, Result<server_client::Client>>,
stop_server: impl for<'a> Fn(&'a Path, Duration) -> BoxFuture<'a, bool>,
) -> Result<()> {
let server_env_path = Storage::new(storage_dir).runtime_directory().env_path();
let previous_server_env = std::fs::read_to_string(&server_env_path).ok();
let settings_write_ref = settings_write.as_ref();
persist_install_outputs_direct(
let dev_token_write_for_rollback = dev_token_write.clone();
InstallPersistencePlan {
storage_dir,
&server_env_updates(server_env_secrets),
&[],
&[],
settings_write_ref,
)?;
settings_write,
server_env_writes,
server_env_removals,
dev_token_write,
vault_writes: Vec::new(),
vault_removals: Vec::new(),
}
.persist_direct()?;
let persist_result = persist_vault_secrets_with(
storage_dir,
@ -1397,17 +1393,29 @@ async fn persist_install_outputs_with_settings(
.await;
if let Err(err) = persist_result {
restore_optional_file(&server_env_path, previous_server_env.as_deref())?;
let mut rollback_failures = Vec::new();
if let Err(restore_err) =
restore_optional_file(&server_env_path, previous_server_env.as_deref())
{
rollback_failures.push(restore_err.to_string());
}
if let Some(write) = settings_write {
match write.previous_contents {
Some(previous) => std::fs::write(write.path, previous)
.with_context(|| format!("restoring settings file {}", write.path.display()))?,
None if write.path.exists() => std::fs::remove_file(write.path)
.with_context(|| format!("removing settings file {}", write.path.display()))?,
None => {}
if let Err(restore_err) = restore_optional_file(write.path, write.previous_contents) {
rollback_failures.push(restore_err.to_string());
}
}
return Err(err);
if let Some(write) = dev_token_write_for_rollback.as_ref() {
if let Err(restore_err) = rollback_dev_token_write(write) {
rollback_failures.push(restore_err.to_string());
}
}
if rollback_failures.is_empty() {
return Err(err);
}
return Err(err.context(format!(
"rollback failures: {}",
rollback_failures.join("; ")
)));
}
Ok(())
@ -1870,6 +1878,7 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
// Secrets and auth material
let mut dev_token_for_auth_store = None;
let mut dev_token_write = None;
{
let session_secret = session_secret::generate_session_secret();
fabro_util::printerr!(
@ -1887,7 +1896,9 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
let dev_token_path = Storage::new(&storage_dir)
.runtime_directory()
.dev_token_path();
let token = dev_token::read_or_mint_dev_token_for_install(&dev_token_path)?;
let prepared = prepare_dev_token_write_for_install(&dev_token_path)?;
let token = prepared.token;
dev_token_write = prepared.write;
dev_token_for_auth_store = Some(token.clone());
fabro_util::printerr!(
printer,
@ -1899,9 +1910,13 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
None
};
let mut generated_server_env_pairs = vec![("SESSION_SECRET".to_string(), session_secret)];
let mut generated_server_env_pairs = vec![(
fabro_static::EnvVars::SESSION_SECRET.to_string(),
session_secret,
)];
if let Some(token) = dev_token {
generated_server_env_pairs.push(("FABRO_DEV_TOKEN".to_string(), token));
generated_server_env_pairs
.push((fabro_static::EnvVars::FABRO_DEV_TOKEN.to_string(), token));
}
server_env_pairs.extend(generated_server_env_pairs);
}
@ -1915,11 +1930,20 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
contents: settings_toml.as_str(),
previous_contents: existing_config_contents.as_deref(),
}),
dev_token_write,
server_was_running,
dev_token_for_auth_store.as_deref(),
)
.await?;
if let Some(token) = dev_token_for_auth_store {
let target = ServerTarget::http_url(&web_url)?;
let user_settings = UserSettingsBuilder::from_toml(&settings_toml)?;
let target = match user_config::resolve_nondefault_server_target(
&ServerTargetArgs::default(),
&user_settings,
)? {
Some(target) => target,
None => ServerTarget::http_url(&web_url)?,
};
if let Err(err) = AuthStore::default().put(
&target,
AuthEntry::DevToken(DevTokenEntry {
@ -2048,6 +2072,7 @@ mod tests {
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use fabro_vault::Vault;
use httpmock::Method::POST;
use httpmock::MockServer;
@ -2829,7 +2854,7 @@ client_id = "client-id"
}
#[tokio::test]
async fn persist_install_outputs_with_settings_rolls_back_new_files_on_secret_failure() {
async fn persist_cli_install_outputs_rolls_back_new_files_on_secret_failure() {
let dir = tempfile::tempdir().unwrap();
let server_env_pairs = [("SESSION_SECRET".to_string(), "session".to_string())];
let vault_secrets = [CreateSecretRequest {
@ -2841,15 +2866,17 @@ client_id = "client-id"
let settings_path = dir.path().join(SETTINGS_CONFIG_FILENAME);
let stop_called = Arc::new(AtomicBool::new(false));
let result = persist_install_outputs_with_settings(
let result = persist_cli_install_outputs_with(
dir.path(),
&server_env_pairs,
server_env_updates(&server_env_pairs),
Vec::new(),
&vault_secrets,
Some(PendingSettingsWrite {
path: &settings_path,
contents: "_version = 1\n",
previous_contents: None,
}),
None,
false,
|_| Box::pin(async move { Err(anyhow::anyhow!("boom")) }),
{
@ -2877,7 +2904,99 @@ client_id = "client-id"
}
#[tokio::test]
async fn persist_install_outputs_with_settings_restores_previous_contents_on_secret_failure() {
async fn persist_cli_install_outputs_rolls_back_staged_dev_token_on_secret_failure() {
let dir = tempfile::tempdir().unwrap();
let storage = Storage::new(dir.path());
let dev_token_path = storage.runtime_directory().dev_token_path();
let prepared = fabro_install::prepare_dev_token_write_for_install(&dev_token_path).unwrap();
let server_env_pairs = [
("SESSION_SECRET".to_string(), "session".to_string()),
("FABRO_DEV_TOKEN".to_string(), prepared.token.clone()),
];
let vault_secrets = [CreateSecretRequest {
name: "GITHUB_CLI_TOKEN".to_string(),
value: "gh-token".to_string(),
type_: ApiSecretType::Token,
description: None,
}];
let settings_path = dir.path().join(SETTINGS_CONFIG_FILENAME);
let result = persist_cli_install_outputs_with(
dir.path(),
server_env_updates(&server_env_pairs),
Vec::new(),
&vault_secrets,
Some(PendingSettingsWrite {
path: &settings_path,
contents: "_version = 1\n",
previous_contents: None,
}),
prepared.write,
false,
|_| Box::pin(async move { Err(anyhow::anyhow!("boom")) }),
|_, _| Box::pin(async move { true }),
)
.await;
assert!(result.is_err());
assert!(!settings_path.exists());
assert!(!storage.runtime_directory().env_path().exists());
assert!(
!dev_token_path.exists(),
"failed install should not leave a staged dev token"
);
}
#[tokio::test]
async fn persist_cli_install_outputs_preserves_existing_dev_token_on_secret_failure() {
let dir = tempfile::tempdir().unwrap();
let storage = Storage::new(dir.path());
let dev_token_path = storage.runtime_directory().dev_token_path();
let token = fabro_util::dev_token::generate_dev_token();
fabro_util::dev_token::write_dev_token(&dev_token_path, &token).unwrap();
let prepared = fabro_install::prepare_dev_token_write_for_install(&dev_token_path).unwrap();
assert_eq!(prepared.token, token);
assert!(prepared.write.is_none());
let server_env_pairs = [
("SESSION_SECRET".to_string(), "session".to_string()),
("FABRO_DEV_TOKEN".to_string(), token.clone()),
];
let vault_secrets = [CreateSecretRequest {
name: "GITHUB_CLI_TOKEN".to_string(),
value: "gh-token".to_string(),
type_: ApiSecretType::Token,
description: None,
}];
let settings_path = dir.path().join(SETTINGS_CONFIG_FILENAME);
let result = persist_cli_install_outputs_with(
dir.path(),
server_env_updates(&server_env_pairs),
Vec::new(),
&vault_secrets,
Some(PendingSettingsWrite {
path: &settings_path,
contents: "_version = 1\n",
previous_contents: None,
}),
prepared.write,
false,
|_| Box::pin(async move { Err(anyhow::anyhow!("boom")) }),
|_, _| Box::pin(async move { true }),
)
.await;
assert!(result.is_err());
assert_eq!(
fabro_util::dev_token::read_dev_token_file(&dev_token_path).as_deref(),
Some(token.as_str())
);
assert!(!settings_path.exists());
assert!(!storage.runtime_directory().env_path().exists());
}
#[tokio::test]
async fn persist_cli_install_outputs_restores_previous_contents_on_secret_failure() {
let dir = tempfile::tempdir().unwrap();
let server_env_pairs = [("SESSION_SECRET".to_string(), "session".to_string())];
let vault_secrets = [CreateSecretRequest {
@ -2889,15 +3008,17 @@ client_id = "client-id"
let settings_path = dir.path().join(SETTINGS_CONFIG_FILENAME);
std::fs::write(&settings_path, "_version = 1\n[server]\n").unwrap();
let result = persist_install_outputs_with_settings(
let result = persist_cli_install_outputs_with(
dir.path(),
&server_env_pairs,
server_env_updates(&server_env_pairs),
Vec::new(),
&vault_secrets,
Some(PendingSettingsWrite {
path: &settings_path,
contents: "_version = 1\n[server]\nfoo = \"bar\"\n",
previous_contents: Some("_version = 1\n[server]\n"),
}),
None,
false,
|_| Box::pin(async move { Err(anyhow::anyhow!("boom")) }),
|_, _| Box::pin(async move { true }),
@ -3044,6 +3165,66 @@ client_id = "client-id"
assert_eq!(std::fs::read_to_string(&settings_path).unwrap(), "after");
}
#[test]
fn persist_github_install_changes_restores_server_env_on_vault_failure() {
let dir = tempfile::tempdir().unwrap();
let storage = Storage::new(dir.path());
let server_env_path = storage.runtime_directory().env_path();
envfile::write_env_file(
&server_env_path,
&std::collections::HashMap::from([
(
GITHUB_APP_PRIVATE_KEY_KEY.to_string(),
"private".to_string(),
),
(
GITHUB_APP_CLIENT_SECRET_KEY.to_string(),
"client".to_string(),
),
("KEEP_ME".to_string(), "1".to_string()),
]),
)
.unwrap();
let settings_path = dir.path().join(SETTINGS_CONFIG_FILENAME);
std::fs::write(&settings_path, "before").unwrap();
let result = persist_github_install_changes(dir.path(), &PendingGitHubInstallWrite {
settings_write: PendingSettingsWrite {
path: &settings_path,
contents: "after",
previous_contents: Some("before"),
},
server_env_set: Vec::new(),
server_env_remove: vec![GITHUB_APP_PRIVATE_KEY_KEY, GITHUB_APP_CLIENT_SECRET_KEY],
vault_set: vec![("bad-secret-name".to_string(), "token".to_string())],
vault_remove: Vec::new(),
});
assert!(result.is_err());
let server_env = envfile::read_env_file(&server_env_path).unwrap();
assert_eq!(
server_env
.get(GITHUB_APP_PRIVATE_KEY_KEY)
.map(String::as_str),
Some("private")
);
assert_eq!(
server_env
.get(GITHUB_APP_CLIENT_SECRET_KEY)
.map(String::as_str),
Some("client")
);
assert_eq!(server_env.get("KEEP_ME").map(String::as_str), Some("1"));
assert_eq!(std::fs::read_to_string(&settings_path).unwrap(), "before");
assert_eq!(
Vault::load(storage.secrets_path())
.unwrap()
.get("bad-secret-name"),
None
);
}
#[tokio::test]
async fn write_artifact_store_metadata_creates_marker_in_resolved_store() {
let dir = tempfile::tempdir().unwrap();

30
lib/crates/fabro-cli/src/server_client.rs
View file

@ -36,6 +36,18 @@ pub(crate) async fn connect_server(storage_dir: &Path) -> Result<Client> {
connect_local_api_client_bundle(storage_dir, &user_config::active_settings_path(None)).await
}
pub(crate) async fn connect_server_with_dev_token(
storage_dir: &Path,
dev_token: &str,
) -> Result<Client> {
connect_local_api_client_bundle_with_dev_token(
storage_dir,
&user_config::active_settings_path(None),
Some(dev_token),
)
.await
}
pub(crate) async fn connect_server_target(target: &ServerTarget) -> Result<Client> {
connect_target_api_client_bundle(target).await
}
@ -121,6 +133,14 @@ async fn connect_managed_unix_socket_api_client_bundle(
async fn connect_local_api_client_bundle(
storage_dir: &Path,
active_config_path: &Path,
) -> Result<Client> {
connect_local_api_client_bundle_with_dev_token(storage_dir, active_config_path, None).await
}
async fn connect_local_api_client_bundle_with_dev_token(
storage_dir: &Path,
active_config_path: &Path,
bootstrap_dev_token: Option<&str>,
) -> Result<Client> {
let bind = start::ensure_server_running_for_storage(storage_dir, active_config_path)
.await
@ -130,7 +150,10 @@ async fn connect_local_api_client_bundle(
let runtime_token_path = Storage::new(storage_dir)
.runtime_directory()
.dev_token_path();
let token = wait_for_runtime_dev_token(&runtime_token_path).await?;
let token = match bootstrap_dev_token {
Some(token) => token.to_string(),
None => wait_for_runtime_dev_token(&runtime_token_path).await?,
};
let http_client = connect_unix_socket_http_client(&path, Some(&token)).await?;
Ok(Client::builder()
.transport("http://fabro", http_client)
@ -140,7 +163,10 @@ async fn connect_local_api_client_bundle(
}
Bind::Tcp(addr) => {
let target = ServerTarget::http_url(format!("http://{addr}"))?;
let credential = resolve_target_credential(&target)?;
let credential = match bootstrap_dev_token {
Some(token) => Some(Credential::DevToken(token.to_string())),
None => resolve_target_credential(&target)?,
};
let oauth_session = refreshable_oauth(&target, credential.as_ref());
build_client(target, credential, oauth_session, None).await
}

482
lib/crates/fabro-cli/tests/it/cmd/install.rs
View file

@ -1,12 +1,18 @@
#![expect(
clippy::disallowed_methods,
reason = "integration tests stage fixtures and subprocess env with sync test infrastructure"
clippy::disallowed_types,
reason = "integration tests stage fixtures, reserve ports, and subprocess env with sync test infrastructure"
)]
use std::net::TcpListener;
use std::time::Duration;
use fabro_config::{Storage, envfile};
use fabro_test::{EnvVars, fabro_snapshot, test_context};
use fabro_vault::{SecretType, Vault};
const INSTALL_COMMAND_TIMEOUT: Duration = Duration::from_secs(30);
#[test]
fn help() {
let context = test_context!();
@ -252,6 +258,312 @@ fn skip_llm_conflicts_with_llm_credential_flags() {
);
}
#[test]
fn non_interactive_token_install_bootstraps_server_auth_for_secret_persistence() {
let mut context = test_context!();
std::fs::remove_file(context.home_dir.join(".fabro/settings.toml")).unwrap();
let storage_dir = context.temp_dir.join("install-storage");
context.manage_storage_dir(&storage_dir);
let path = fake_gh_path(&context, "ghp_install_bootstrap");
let web_url = unused_loopback_web_url();
let output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.env(EnvVars::PATH, &path)
.args([
"install",
"--storage-dir",
storage_dir.to_str().unwrap(),
"--web-url",
&web_url,
"--non-interactive",
"--skip-llm",
"--github-strategy",
"token",
"--github-username",
"octocat",
"--overwrite-settings",
])
.output()
.expect("install command should run");
assert!(
output.status.success(),
"install should bootstrap auth before persisting secrets\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
!stderr.contains("Authentication required"),
"install should not require a separate auth login while bootstrapping: {stderr}"
);
let list_output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.args(["--json", "secret", "list"])
.output()
.expect("secret list command should run");
assert!(
list_output.status.success(),
"CLI auth saved by install should authenticate follow-up secret commands\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&list_output.stdout),
String::from_utf8_lossy(&list_output.stderr)
);
let secrets: serde_json::Value =
serde_json::from_slice(&list_output.stdout).expect("secret list JSON should parse");
assert!(
secrets
.as_array()
.expect("secret list should return an array")
.iter()
.any(|secret| secret["name"] == "GITHUB_TOKEN"),
"installed GitHub token should be persisted as a server-owned secret: {secrets}"
);
}
#[test]
fn keep_existing_settings_persists_secrets_without_rewriting_server_target() {
let mut context = test_context!();
let storage_dir = context.temp_dir.join("install-storage");
context.manage_storage_dir(&storage_dir);
let existing_web_url = unused_loopback_web_url();
let requested_web_url = unused_loopback_web_url();
write_http_install_settings(&context, &storage_dir, &existing_web_url, "keep-me");
login_with_storage_dev_token(&context, &storage_dir, &existing_web_url);
let path = fake_gh_path(&context, "ghp_keep_existing");
let output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.env(EnvVars::PATH, path)
.args([
"install",
"--storage-dir",
storage_dir.to_str().unwrap(),
"--web-url",
&requested_web_url,
"--non-interactive",
"--skip-llm",
"--github-strategy",
"token",
"--keep-existing-settings",
])
.output()
.expect("install command should run");
assert!(
output.status.success(),
"install should keep existing server settings while persisting secrets\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let settings = read_home_settings(&context);
let parsed: toml::Value = toml::from_str(&settings).unwrap();
assert_eq!(
parsed
.get("server")
.and_then(toml::Value::as_table)
.and_then(|server| server.get("web"))
.and_then(toml::Value::as_table)
.and_then(|web| web.get("url"))
.and_then(toml::Value::as_str),
Some(existing_web_url.as_str())
);
assert_eq!(
parsed
.get("cli")
.and_then(toml::Value::as_table)
.and_then(|cli| cli.get("target"))
.and_then(toml::Value::as_table)
.and_then(|target| target.get("url"))
.and_then(toml::Value::as_str),
Some(existing_web_url.as_str())
);
assert!(
!settings.contains(&requested_web_url),
"--keep-existing-settings should not rewrite settings to the requested web URL"
);
assert_secret_list_contains(&context, &["GITHUB_TOKEN"]);
}
#[test]
fn install_against_running_authenticated_server_persists_secrets_and_leaves_server_running() {
let mut context = test_context!();
let storage_dir = context.temp_dir.join("install-storage");
context.manage_storage_dir(&storage_dir);
let web_url = unused_loopback_web_url();
write_http_install_settings(&context, &storage_dir, &web_url, "running-server");
login_with_storage_dev_token(&context, &storage_dir, &web_url);
let start_output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.args([
"server",
"start",
"--storage-dir",
storage_dir.to_str().unwrap(),
])
.output()
.expect("server start command should run");
assert!(
start_output.status.success(),
"server start should succeed before install\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&start_output.stdout),
String::from_utf8_lossy(&start_output.stderr)
);
let path = fake_gh_path(&context, "ghp_running_server");
let output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.env(EnvVars::PATH, path)
.args([
"install",
"--storage-dir",
storage_dir.to_str().unwrap(),
"--web-url",
&web_url,
"--non-interactive",
"--skip-llm",
"--github-strategy",
"token",
"--github-username",
"octocat",
"--overwrite-settings",
])
.output()
.expect("install command should run");
assert!(
output.status.success(),
"install should persist secrets through the already-running server\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let status_output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.args([
"server",
"status",
"--json",
"--storage-dir",
storage_dir.to_str().unwrap(),
])
.output()
.expect("server status command should run");
assert!(
status_output.status.success(),
"server should still be running after install\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&status_output.stdout),
String::from_utf8_lossy(&status_output.stderr)
);
let status: serde_json::Value =
serde_json::from_slice(&status_output.stdout).expect("server status JSON should parse");
assert_eq!(status["status"].as_str(), Some("running"));
assert_secret_list_contains(&context, &["GITHUB_TOKEN"]);
}
#[test]
fn install_json_non_interactive_success_emits_complete_event() {
let mut context = test_context!();
std::fs::remove_file(context.home_dir.join(".fabro/settings.toml")).unwrap();
let storage_dir = context.temp_dir.join("install-storage");
context.manage_storage_dir(&storage_dir);
let web_url = unused_loopback_web_url();
login_with_storage_dev_token(&context, &storage_dir, &web_url);
let path = fake_gh_path(&context, "ghp_json_success");
let output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.env(EnvVars::PATH, path)
.args([
"--json",
"install",
"--storage-dir",
storage_dir.to_str().unwrap(),
"--web-url",
&web_url,
"--non-interactive",
"--skip-llm",
"--github-strategy",
"token",
"--github-username",
"octocat",
"--overwrite-settings",
])
.output()
.expect("install command should run");
assert!(
output.status.success(),
"JSON install should succeed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let stdout = String::from_utf8(output.stdout).unwrap();
let events = stdout
.lines()
.map(|line| serde_json::from_str::<serde_json::Value>(line).unwrap())
.collect::<Vec<_>>();
assert_eq!(events, vec![serde_json::json!({
"event": "install_complete",
"status": "success"
})]);
}
#[fabro_macros::e2e_test(live("ANTHROPIC_API_KEY"))]
fn install_with_anthropic_api_key_persists_llm_and_github_secrets() {
let mut context = test_context!();
std::fs::remove_file(context.home_dir.join(".fabro/settings.toml")).unwrap();
let storage_dir = context.temp_dir.join("install-storage");
context.manage_storage_dir(&storage_dir);
let web_url = unused_loopback_web_url();
login_with_storage_dev_token(&context, &storage_dir, &web_url);
let path = fake_gh_path(&context, "ghp_anthropic");
let output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.env(EnvVars::PATH, path)
.args([
"install",
"--storage-dir",
storage_dir.to_str().unwrap(),
"--web-url",
&web_url,
"--non-interactive",
"--llm-provider",
"anthropic",
"--llm-api-key-env",
"ANTHROPIC_API_KEY",
"--github-strategy",
"token",
"--github-username",
"octocat",
"--overwrite-settings",
])
.output()
.expect("install command should run");
assert!(
output.status.success(),
"install should validate and persist the scripted LLM API key\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert_secret_list_contains(&context, &["GITHUB_TOKEN", "ANTHROPIC_API_KEY"]);
}
#[test]
fn github_requires_prior_install() {
let context = test_context!();
@ -350,26 +662,7 @@ mode = "keep-me"
)
.unwrap();
let fake_bin = context.temp_dir.join("fake-bin");
std::fs::create_dir_all(&fake_bin).unwrap();
let fake_gh = fake_bin.join("gh");
std::fs::write(
&fake_gh,
"#!/bin/sh\nif [ \"$1\" = \"auth\" ] && [ \"$2\" = \"token\" ]; then\n printf 'token-from-gh\\n'\n exit 0\nfi\nexit 1\n",
)
.unwrap();
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
std::fs::set_permissions(&fake_gh, std::fs::Permissions::from_mode(0o755)).unwrap();
}
let path = format!(
"{}:{}",
fake_bin.display(),
std::env::var(EnvVars::PATH).unwrap()
);
let path = fake_gh_path(&context, "token-from-gh");
let output = context
.command()
.env(EnvVars::PATH, path)
@ -454,3 +747,150 @@ mode = "keep-me"
Some(SecretType::Token)
);
}
fn unused_loopback_web_url() -> String {
let listener = TcpListener::bind("127.0.0.1:0").expect("bind unused loopback port");
let port = listener.local_addr().expect("read loopback addr").port();
drop(listener);
format!("http://127.0.0.1:{port}")
}
fn fake_gh_path(context: &fabro_test::TestContext, token: &str) -> String {
let fake_bin = context.temp_dir.join(format!("fake-bin-{token}"));
std::fs::create_dir_all(&fake_bin).expect("fake gh bin directory should be created");
let fake_gh = fake_bin.join("gh");
std::fs::write(
&fake_gh,
format!("#!/bin/sh\nif [ \"$1\" = \"auth\" ] && [ \"$2\" = \"token\" ]; then\n printf '{token}\\n'\n exit 0\nfi\nexit 1\n"),
)
.expect("fake gh script should be written");
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
std::fs::set_permissions(&fake_gh, std::fs::Permissions::from_mode(0o755))
.expect("fake gh script should be executable");
}
format!(
"{}:{}",
fake_bin.display(),
std::env::var(EnvVars::PATH).expect("PATH should be set for install tests")
)
}
fn write_raw_home_settings(context: &fabro_test::TestContext, settings: &str) {
let settings_path = context.home_dir.join(".fabro/settings.toml");
std::fs::create_dir_all(
settings_path
.parent()
.expect("settings path should have a parent directory"),
)
.expect("settings directory should be created");
std::fs::write(settings_path, settings).expect("settings file should be written");
}
fn read_home_settings(context: &fabro_test::TestContext) -> String {
std::fs::read_to_string(context.home_dir.join(".fabro/settings.toml"))
.expect("settings file should be readable")
}
fn write_http_install_settings(
context: &fabro_test::TestContext,
storage_dir: &std::path::Path,
web_url: &str,
metadata_mode: &str,
) {
let address = web_url
.strip_prefix("http://")
.expect("test web URL should be an http URL");
write_raw_home_settings(
context,
&format!(
r#"
_version = 1
[server.storage]
root = "{}"
[server.api]
url = "{}/api/v1"
[server.web]
enabled = true
url = "{}"
[server.auth]
methods = ["dev-token"]
[server.listen]
type = "tcp"
address = "{}"
[cli.target]
type = "http"
url = "{}"
[project.metadata]
mode = "{}"
"#,
storage_dir.display(),
web_url,
web_url,
address,
web_url,
metadata_mode
),
);
}
fn login_with_storage_dev_token(
context: &fabro_test::TestContext,
storage_dir: &std::path::Path,
web_url: &str,
) {
let token = fabro_util::dev_token::read_dev_token_file(
&Storage::new(storage_dir)
.runtime_directory()
.dev_token_path(),
)
.expect("storage dev token should be valid");
let output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.args(["auth", "login", "--server", web_url, "--dev-token", &token])
.output()
.expect("auth login command should run");
assert!(
output.status.success(),
"auth login should seed CLI auth for {web_url}\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
}
fn assert_secret_list_contains(context: &fabro_test::TestContext, expected_names: &[&str]) {
let list_output = context
.command()
.timeout(INSTALL_COMMAND_TIMEOUT)
.args(["--json", "secret", "list"])
.output()
.expect("secret list command should run");
assert!(
list_output.status.success(),
"secret list should succeed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&list_output.stdout),
String::from_utf8_lossy(&list_output.stderr)
);
let secrets: serde_json::Value =
serde_json::from_slice(&list_output.stdout).expect("secret list JSON should parse");
let array = secrets
.as_array()
.expect("secret list should return an array");
for expected_name in expected_names {
assert!(
array.iter().any(|secret| secret["name"] == *expected_name),
"secret list should include {expected_name}: {secrets}"
);
}
}

1
lib/crates/fabro-install/Cargo.toml
View file

@ -17,6 +17,7 @@ toml.workspace = true
fabro-config = { path = "../fabro-config" }
fabro-static.workspace = true
fabro-types = { path = "../fabro-types" }
fabro-util = { path = "../fabro-util" }
fabro-vault = { path = "../fabro-vault" }
[dev-dependencies]

639
lib/crates/fabro-install/src/lib.rs
View file

@ -8,14 +8,28 @@ use std::path::{Path, PathBuf};
use anyhow::{Context, Result};
use fabro_config::{Storage, envfile};
use fabro_static::EnvVars;
use fabro_util::dev_token;
use fabro_vault::{SecretType as VaultSecretType, Vault};
#[derive(Debug, Clone, Copy)]
pub struct PendingSettingsWrite<'a> {
pub path: &'a Path,
pub contents: &'a str,
pub previous_contents: Option<&'a str>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct PendingDevTokenWrite {
path: PathBuf,
token: String,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct PreparedInstallDevToken {
pub token: String,
pub write: Option<PendingDevTokenWrite>,
}
pub const OBJECT_STORE_MANAGED_COMMENT: &str = "managed by fabro-install: object-store";
pub const OBJECT_STORE_ACCESS_KEY_ID_ENV: &str = EnvVars::AWS_ACCESS_KEY_ID;
pub const OBJECT_STORE_SECRET_ACCESS_KEY_ENV: &str = EnvVars::AWS_SECRET_ACCESS_KEY;
@ -28,6 +42,16 @@ pub struct VaultSecretWrite {
pub description: Option<String>,
}
pub struct InstallPersistencePlan<'a> {
pub storage_dir: &'a Path,
pub settings_write: Option<PendingSettingsWrite<'a>>,
pub server_env_writes: Vec<envfile::EnvFileUpdate>,
pub server_env_removals: Vec<envfile::EnvFileRemoval>,
pub dev_token_write: Option<PendingDevTokenWrite>,
pub vault_writes: Vec<VaultSecretWrite>,
pub vault_removals: Vec<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum InstallListenConfig {
Tcp(String),
@ -99,6 +123,21 @@ pub fn default_web_url() -> String {
"http://127.0.0.1:32276".to_string()
}
pub fn prepare_dev_token_write_for_install(path: &Path) -> Result<PreparedInstallDevToken> {
if let Some(token) = dev_token::read_dev_token_for_install(path)? {
return Ok(PreparedInstallDevToken { token, write: None });
}
let token = dev_token::generate_dev_token();
Ok(PreparedInstallDevToken {
token: token.clone(),
write: Some(PendingDevTokenWrite {
path: path.to_path_buf(),
token,
}),
})
}
fn root_table_mut(doc: &mut toml::Value) -> Result<&mut toml::Table> {
doc.as_table_mut()
.context("settings.toml root is not a table")
@ -136,28 +175,14 @@ fn github_integration_table(doc: &mut toml::Value) -> Result<&mut toml::Table> {
.context("settings.toml [server.integrations.github] is not a table")
}
pub fn merge_server_settings(
doc: &mut toml::Value,
web_url: &str,
listen_config: &InstallListenConfig,
) -> Result<()> {
fn set_server_listen(doc: &mut toml::Value, listen_config: &InstallListenConfig) -> Result<()> {
let root = root_table_mut(doc)?;
root.insert("_version".to_string(), toml::Value::Integer(1));
let server = ensure_table(root, "server")?;
let api = ensure_table(server, "api")?;
api.insert(
"url".to_string(),
toml::Value::String(format!("{web_url}/api/v1")),
);
let listen = ensure_table(server, "listen")?;
let mut listen = toml::Table::default();
match listen_config {
InstallListenConfig::Tcp(address) => {
listen.insert("type".to_string(), toml::Value::String("tcp".to_string()));
listen.insert("address".to_string(), toml::Value::String(address.clone()));
listen.remove("path");
}
InstallListenConfig::Unix(path) => {
listen.insert("type".to_string(), toml::Value::String("unix".to_string()));
@ -165,9 +190,44 @@ pub fn merge_server_settings(
"path".to_string(),
toml::Value::String(path.display().to_string()),
);
listen.remove("address");
}
}
server.insert("listen".to_string(), toml::Value::Table(listen));
Ok(())
}
fn set_cli_target_http(doc: &mut toml::Value, web_url: &str) -> Result<()> {
let root = root_table_mut(doc)?;
let cli = ensure_table(root, "cli")?;
let mut target = toml::Table::default();
target.insert("type".to_string(), toml::Value::String("http".to_string()));
target.insert("url".to_string(), toml::Value::String(web_url.to_string()));
cli.insert("target".to_string(), toml::Value::Table(target));
Ok(())
}
pub fn merge_server_settings(
doc: &mut toml::Value,
web_url: &str,
listen_config: &InstallListenConfig,
) -> Result<()> {
{
let root = root_table_mut(doc)?;
root.insert("_version".to_string(), toml::Value::Integer(1));
let server = ensure_table(root, "server")?;
let api = ensure_table(server, "api")?;
api.insert(
"url".to_string(),
toml::Value::String(format!("{web_url}/api/v1")),
);
}
set_server_listen(doc, listen_config)?;
let root = root_table_mut(doc)?;
let server = ensure_table(root, "server")?;
let web = ensure_table(server, "web")?;
web.insert("enabled".to_string(), toml::Value::Boolean(true));
@ -179,10 +239,7 @@ pub fn merge_server_settings(
toml::Value::Array(vec![toml::Value::String("dev-token".to_string())]),
);
let cli = ensure_table(root, "cli")?;
let target = ensure_table(cli, "target")?;
target.insert("type".to_string(), toml::Value::String("http".to_string()));
target.insert("url".to_string(), toml::Value::String(web_url.to_string()));
set_cli_target_http(doc, web_url)?;
Ok(())
}
@ -403,7 +460,7 @@ pub fn write_sandbox_settings(
Ok(())
}
fn restore_optional_file(path: &Path, previous_contents: Option<&str>) -> Result<()> {
pub fn restore_optional_file(path: &Path, previous_contents: Option<&str>) -> Result<()> {
match previous_contents {
Some(contents) => {
if let Some(parent) = path.parent() {
@ -425,6 +482,24 @@ fn restore_optional_file(path: &Path, previous_contents: Option<&str>) -> Result
Ok(())
}
pub fn rollback_dev_token_write(write: &PendingDevTokenWrite) -> Result<()> {
match std::fs::remove_file(&write.path) {
Ok(()) => {}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => {
return Err(anyhow::Error::new(err)
.context(format!("removing dev token {}", write.path.display())));
}
}
Ok(())
}
fn write_pending_dev_token(write: &PendingDevTokenWrite) -> Result<()> {
dev_token::write_dev_token(&write.path, &write.token)
.with_context(|| format!("writing dev token {}", write.path.display()))
}
fn persist_server_env_secrets(
storage_dir: &Path,
writes: &[envfile::EnvFileUpdate],
@ -446,13 +521,23 @@ fn persist_server_env_secrets(
.with_context(|| format!("updating server env file {}", env_path.display()))
}
fn persist_vault_secrets_direct(storage_dir: &Path, secrets: &[VaultSecretWrite]) -> Result<()> {
if secrets.is_empty() {
fn persist_vault_secrets_direct(
storage_dir: &Path,
secrets: &[VaultSecretWrite],
removals: &[String],
) -> Result<()> {
if secrets.is_empty() && removals.is_empty() {
return Ok(());
}
let vault_path = Storage::new(storage_dir).secrets_path();
let mut vault = Vault::load(vault_path).map_err(anyhow::Error::from)?;
for name in removals {
match vault.remove(name) {
Ok(()) | Err(fabro_vault::Error::NotFound(_)) => {}
Err(err) => return Err(err.into()),
}
}
for secret in secrets {
vault
.set(
@ -466,6 +551,108 @@ fn persist_vault_secrets_direct(storage_dir: &Path, secrets: &[VaultSecretWrite]
Ok(())
}
fn direct_persistence_error(err: anyhow::Error, rollback_failures: &[String]) -> anyhow::Error {
if rollback_failures.is_empty() {
err.context("persisting install outputs directly")
} else {
err.context(format!(
"persisting install outputs directly; rollback failures: {}",
rollback_failures.join("; ")
))
}
}
fn rollback_direct_persistence(
settings_write: Option<&PendingSettingsWrite<'_>>,
vault_path: &Path,
previous_vault: Option<&str>,
dev_token_write: Option<&PendingDevTokenWrite>,
) -> Vec<String> {
let mut failures = Vec::new();
if let Some(write) = settings_write {
if let Err(err) = restore_optional_file(write.path, write.previous_contents) {
failures.push(err.to_string());
}
}
if let Err(err) = restore_optional_file(vault_path, previous_vault) {
failures.push(err.to_string());
}
if let Some(write) = dev_token_write {
if let Err(err) = rollback_dev_token_write(write) {
failures.push(err.to_string());
}
}
failures
}
impl InstallPersistencePlan<'_> {
pub fn persist_direct(&self) -> std::result::Result<(), PersistInstallOutputsError> {
let server_env_report = persist_server_env_secrets(
self.storage_dir,
&self.server_env_writes,
&self.server_env_removals,
)
.map_err(|err| PersistInstallOutputsError::new(err, false, Vec::new()))?;
let removed_env_keys = server_env_report.removed_keys;
if let Some(write) = self.settings_write.as_ref() {
if let Some(parent) = write.path.parent() {
std::fs::create_dir_all(parent)
.with_context(|| format!("creating settings directory {}", parent.display()))
.map_err(|err| {
PersistInstallOutputsError::new(err, true, removed_env_keys.clone())
})?;
}
std::fs::write(write.path, write.contents)
.with_context(|| format!("writing settings file {}", write.path.display()))
.map_err(|err| {
PersistInstallOutputsError::new(err, true, removed_env_keys.clone())
})?;
}
let vault_path = Storage::new(self.storage_dir).secrets_path();
let previous_vault = std::fs::read_to_string(&vault_path).ok();
if let Err(err) =
persist_vault_secrets_direct(self.storage_dir, &self.vault_writes, &self.vault_removals)
{
let rollback_failures = rollback_direct_persistence(
self.settings_write.as_ref(),
&vault_path,
previous_vault.as_deref(),
self.dev_token_write.as_ref(),
);
let error = direct_persistence_error(err, &rollback_failures);
return Err(PersistInstallOutputsError::new(
error,
true,
removed_env_keys,
));
}
if let Some(write) = self.dev_token_write.as_ref() {
if let Err(err) = write_pending_dev_token(write) {
let rollback_failures = rollback_direct_persistence(
self.settings_write.as_ref(),
&vault_path,
previous_vault.as_deref(),
Some(write),
);
let error = direct_persistence_error(err, &rollback_failures);
return Err(PersistInstallOutputsError::new(
error,
true,
removed_env_keys,
));
}
}
Ok(())
}
}
pub fn persist_install_outputs_direct(
storage_dir: &Path,
server_env_writes: &[envfile::EnvFileUpdate],
@ -473,65 +660,35 @@ pub fn persist_install_outputs_direct(
vault_secrets: &[VaultSecretWrite],
settings_write: Option<&PendingSettingsWrite<'_>>,
) -> std::result::Result<(), PersistInstallOutputsError> {
let server_env_report =
persist_server_env_secrets(storage_dir, server_env_writes, server_env_removals)
.map_err(|err| PersistInstallOutputsError::new(err, false, Vec::new()))?;
let removed_env_keys = server_env_report.removed_keys;
if let Some(write) = settings_write {
if let Some(parent) = write.path.parent() {
std::fs::create_dir_all(parent)
.with_context(|| format!("creating settings directory {}", parent.display()))
.map_err(|err| {
PersistInstallOutputsError::new(err, true, removed_env_keys.clone())
})?;
}
std::fs::write(write.path, write.contents)
.with_context(|| format!("writing settings file {}", write.path.display()))
.map_err(|err| PersistInstallOutputsError::new(err, true, removed_env_keys.clone()))?;
InstallPersistencePlan {
storage_dir,
settings_write: settings_write.copied(),
server_env_writes: server_env_writes.to_vec(),
server_env_removals: server_env_removals.to_vec(),
dev_token_write: None,
vault_writes: vault_secrets.to_vec(),
vault_removals: Vec::new(),
}
let vault_path = Storage::new(storage_dir).secrets_path();
let previous_vault = std::fs::read_to_string(&vault_path).ok();
if let Err(err) = persist_vault_secrets_direct(storage_dir, vault_secrets) {
let mut rollback_failures = Vec::new();
if let Some(write) = settings_write {
if let Err(restore_err) = restore_optional_file(write.path, write.previous_contents) {
rollback_failures.push(restore_err.to_string());
}
}
if let Err(restore_err) = restore_optional_file(&vault_path, previous_vault.as_deref()) {
rollback_failures.push(restore_err.to_string());
}
let error = if rollback_failures.is_empty() {
err.context("persisting install outputs directly")
} else {
err.context(format!(
"persisting install outputs directly; rollback failures: {}",
rollback_failures.join("; ")
))
};
return Err(PersistInstallOutputsError::new(
error,
true,
removed_env_keys,
));
}
Ok(())
.persist_direct()
}
#[cfg(test)]
mod tests {
use fabro_config::{ServerSettingsBuilder, Storage, envfile};
use std::path::PathBuf;
use fabro_config::{ServerSettingsBuilder, Storage, UserSettingsBuilder, envfile};
use fabro_types::settings::cli::CliTargetSettings;
use fabro_util::dev_token::{
generate_dev_token, read_dev_token_file, validate_dev_token_format, write_dev_token,
};
use fabro_vault::{SecretType as VaultSecretType, Vault};
use super::{
InstallListenConfig, InstallObjectStoreCredentialMode, InstallObjectStoreSelection,
InstallSandboxSelection, OBJECT_STORE_ACCESS_KEY_ID_ENV, OBJECT_STORE_MANAGED_COMMENT,
OBJECT_STORE_SECRET_ACCESS_KEY_ENV, PendingSettingsWrite, VaultSecretWrite,
default_web_url, merge_server_settings, persist_install_outputs_direct,
InstallPersistencePlan, InstallSandboxSelection, OBJECT_STORE_ACCESS_KEY_ID_ENV,
OBJECT_STORE_MANAGED_COMMENT, OBJECT_STORE_SECRET_ACCESS_KEY_ENV, PendingSettingsWrite,
VaultSecretWrite, default_web_url, merge_server_settings, persist_install_outputs_direct,
prepare_dev_token_write_for_install, set_cli_target_http, set_server_listen,
write_github_app_settings, write_object_store_settings, write_sandbox_settings,
};
@ -598,6 +755,141 @@ name = "custom"
);
}
#[test]
fn merge_server_settings_replaces_stale_unix_cli_target_fields() {
let mut doc: toml::Value = toml::from_str(
r#"
_version = 1
[cli.target]
type = "unix"
path = "/tmp/fabro.sock"
"#,
)
.unwrap();
merge_server_settings(
&mut doc,
&default_web_url(),
&InstallListenConfig::Tcp("127.0.0.1:32276".to_string()),
)
.unwrap();
let target = doc
.get("cli")
.and_then(toml::Value::as_table)
.and_then(|cli| cli.get("target"))
.and_then(toml::Value::as_table)
.expect("cli.target should be a table");
assert_eq!(
target.get("type").and_then(toml::Value::as_str),
Some("http")
);
assert_eq!(
target.get("url").and_then(toml::Value::as_str),
Some(default_web_url().as_str())
);
assert!(!target.contains_key("path"));
let toml_str = toml::to_string_pretty(&doc).expect("settings should serialize");
let settings = UserSettingsBuilder::from_toml(&toml_str).expect("settings should resolve");
assert!(matches!(
settings.cli.target,
Some(CliTargetSettings::Http { .. })
));
}
#[test]
fn set_cli_target_http_replaces_the_full_tagged_enum_table() {
let mut doc: toml::Value = toml::from_str(
r#"
_version = 1
[cli.target]
type = "unix"
path = "/tmp/fabro.sock"
stale = "remove-me"
"#,
)
.unwrap();
set_cli_target_http(&mut doc, &default_web_url()).unwrap();
let target = doc
.get("cli")
.and_then(toml::Value::as_table)
.and_then(|cli| cli.get("target"))
.and_then(toml::Value::as_table)
.expect("cli.target should be a table");
assert_eq!(target.len(), 2);
assert_eq!(
target.get("type").and_then(toml::Value::as_str),
Some("http")
);
assert_eq!(
target.get("url").and_then(toml::Value::as_str),
Some(default_web_url().as_str())
);
}
#[test]
fn set_server_listen_replaces_stale_variant_fields_in_both_directions() {
let mut doc: toml::Value = toml::from_str(
r#"
_version = 1
[server.listen]
type = "tcp"
address = "127.0.0.1:32276"
path = "/tmp/stale.sock"
stale = "remove-me"
"#,
)
.unwrap();
set_server_listen(
&mut doc,
&InstallListenConfig::Unix(PathBuf::from("/tmp/fabro.sock")),
)
.unwrap();
let listen = doc
.get("server")
.and_then(toml::Value::as_table)
.and_then(|server| server.get("listen"))
.and_then(toml::Value::as_table)
.expect("server.listen should be a table");
assert_eq!(listen.len(), 2);
assert_eq!(
listen.get("type").and_then(toml::Value::as_str),
Some("unix")
);
assert_eq!(
listen.get("path").and_then(toml::Value::as_str),
Some("/tmp/fabro.sock")
);
set_server_listen(
&mut doc,
&InstallListenConfig::Tcp("0.0.0.0:32276".to_string()),
)
.unwrap();
let listen = doc
.get("server")
.and_then(toml::Value::as_table)
.and_then(|server| server.get("listen"))
.and_then(toml::Value::as_table)
.expect("server.listen should be a table");
assert_eq!(listen.len(), 2);
assert_eq!(
listen.get("type").and_then(toml::Value::as_str),
Some("tcp")
);
assert_eq!(
listen.get("address").and_then(toml::Value::as_str),
Some("0.0.0.0:32276")
);
}
#[test]
fn write_github_app_settings_uses_server_integrations_github() {
let mut doc = toml::Value::Table(toml::Table::default());
@ -706,6 +998,205 @@ name = "custom"
);
}
#[test]
fn install_persistence_plan_direct_writes_and_removes_vault_secrets() {
let dir = tempfile::tempdir().unwrap();
let storage = Storage::new(dir.path());
let mut vault = Vault::load(storage.secrets_path()).unwrap();
vault
.set("REMOVE_ME", "old", VaultSecretType::Token, None)
.unwrap();
vault
.set("KEEP_ME", "keep", VaultSecretType::Token, None)
.unwrap();
InstallPersistencePlan {
storage_dir: dir.path(),
settings_write: None,
server_env_writes: Vec::new(),
server_env_removals: Vec::new(),
dev_token_write: None,
vault_writes: vec![VaultSecretWrite {
name: "NEW_SECRET".to_string(),
value: "new".to_string(),
secret_type: VaultSecretType::Token,
description: None,
}],
vault_removals: vec!["REMOVE_ME".to_string()],
}
.persist_direct()
.unwrap();
let vault = Vault::load(storage.secrets_path()).unwrap();
assert_eq!(vault.get("REMOVE_ME"), None);
assert_eq!(vault.get("KEEP_ME"), Some("keep"));
assert_eq!(vault.get("NEW_SECRET"), Some("new"));
}
#[test]
fn install_persistence_plan_direct_restores_settings_and_vault_on_secret_failure() {
let dir = tempfile::tempdir().unwrap();
let storage = Storage::new(dir.path());
let settings_path = dir.path().join("settings.toml");
std::fs::write(&settings_path, "_version = 1\n[server]\n").unwrap();
let vault_path = storage.secrets_path();
let mut vault = Vault::load(vault_path.clone()).unwrap();
vault
.set("REMOVE_ME", "old", VaultSecretType::Token, None)
.unwrap();
let result = InstallPersistencePlan {
storage_dir: dir.path(),
settings_write: Some(PendingSettingsWrite {
path: &settings_path,
contents: "_version = 1\n[server]\nfoo = \"bar\"\n",
previous_contents: Some("_version = 1\n[server]\n"),
}),
server_env_writes: vec![envfile::EnvFileUpdate {
key: "SESSION_SECRET".to_string(),
value: "session".to_string(),
comment: None,
}],
server_env_removals: Vec::new(),
dev_token_write: None,
vault_writes: vec![VaultSecretWrite {
name: "bad-secret-name".to_string(),
value: "boom".to_string(),
secret_type: VaultSecretType::Token,
description: None,
}],
vault_removals: vec!["REMOVE_ME".to_string()],
}
.persist_direct();
assert!(result.is_err());
assert_eq!(
std::fs::read_to_string(&settings_path).unwrap(),
"_version = 1\n[server]\n"
);
let vault = Vault::load(vault_path).unwrap();
assert_eq!(vault.get("REMOVE_ME"), Some("old"));
assert_eq!(vault.get("bad-secret-name"), None);
let server_env = envfile::read_env_file(&storage.runtime_directory().env_path()).unwrap();
assert_eq!(
server_env.get("SESSION_SECRET").map(String::as_str),
Some("session")
);
}
#[test]
fn prepare_dev_token_write_for_install_missing_file_stages_without_writing() {
let dir = tempfile::tempdir().unwrap();
let path = Storage::new(dir.path())
.runtime_directory()
.dev_token_path();
let prepared = prepare_dev_token_write_for_install(&path).unwrap();
assert!(validate_dev_token_format(&prepared.token));
assert!(
prepared.write.is_some(),
"missing token file should stage a write"
);
assert!(
!path.exists(),
"preparing a token must not create the token file"
);
}
#[test]
fn prepare_dev_token_write_for_install_reuses_existing_file() {
let dir = tempfile::tempdir().unwrap();
let path = Storage::new(dir.path())
.runtime_directory()
.dev_token_path();
let token = generate_dev_token();
write_dev_token(&path, &token).unwrap();
let prepared = prepare_dev_token_write_for_install(&path).unwrap();
assert_eq!(prepared.token, token);
assert!(
prepared.write.is_none(),
"existing valid token should not be rewritten"
);
}
#[test]
fn prepare_dev_token_write_for_install_rejects_invalid_existing_file() {
let dir = tempfile::tempdir().unwrap();
let path = Storage::new(dir.path())
.runtime_directory()
.dev_token_path();
std::fs::create_dir_all(path.parent().unwrap()).unwrap();
std::fs::write(&path, "not-a-valid-token").unwrap();
let err = prepare_dev_token_write_for_install(&path).unwrap_err();
assert!(err.to_string().contains("invalid dev token format"));
}
#[test]
fn install_persistence_plan_direct_writes_staged_dev_token_on_success() {
let dir = tempfile::tempdir().unwrap();
let storage = Storage::new(dir.path());
let path = storage.runtime_directory().dev_token_path();
let prepared = prepare_dev_token_write_for_install(&path).unwrap();
let token = prepared.token.clone();
InstallPersistencePlan {
storage_dir: dir.path(),
settings_write: None,
server_env_writes: Vec::new(),
server_env_removals: Vec::new(),
dev_token_write: prepared.write,
vault_writes: Vec::new(),
vault_removals: Vec::new(),
}
.persist_direct()
.unwrap();
assert_eq!(read_dev_token_file(&path).as_deref(), Some(token.as_str()));
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt as _;
let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777;
assert_eq!(mode, 0o600);
}
}
#[test]
fn install_persistence_plan_direct_does_not_leave_staged_dev_token_on_vault_failure() {
let dir = tempfile::tempdir().unwrap();
let storage = Storage::new(dir.path());
let path = storage.runtime_directory().dev_token_path();
let prepared = prepare_dev_token_write_for_install(&path).unwrap();
let result = InstallPersistencePlan {
storage_dir: dir.path(),
settings_write: None,
server_env_writes: Vec::new(),
server_env_removals: Vec::new(),
dev_token_write: prepared.write,
vault_writes: vec![VaultSecretWrite {
name: "bad-secret-name".to_string(),
value: "boom".to_string(),
secret_type: VaultSecretType::Token,
description: None,
}],
vault_removals: Vec::new(),
}
.persist_direct();
assert!(result.is_err());
assert!(
!path.exists(),
"failed persistence should not leave a newly staged dev token file"
);
}
#[test]
fn merge_server_settings_keeps_tcp_bind_separate_from_public_web_url() {
use fabro_types::settings::server::ServerListenSettings;

37
lib/crates/fabro-server/src/install.rs
View file

@ -17,10 +17,11 @@ use fabro_config::Storage;
use fabro_config::bind::{Bind, BindRequest};
use fabro_config::envfile::EnvFileUpdate;
use fabro_install::{
InstallListenConfig, InstallSandboxSelection, OBJECT_STORE_ACCESS_KEY_ID_ENV,
OBJECT_STORE_SECRET_ACCESS_KEY_ENV, PendingSettingsWrite, VaultSecretWrite,
merge_server_settings, persist_install_outputs_direct, write_github_app_settings,
write_object_store_settings, write_sandbox_settings, write_token_settings,
InstallListenConfig, InstallPersistencePlan, InstallSandboxSelection,
OBJECT_STORE_ACCESS_KEY_ID_ENV, OBJECT_STORE_SECRET_ACCESS_KEY_ENV, PendingSettingsWrite,
VaultSecretWrite, merge_server_settings, prepare_dev_token_write_for_install,
write_github_app_settings, write_object_store_settings, write_sandbox_settings,
write_token_settings,
};
use fabro_llm::client::Client as LlmClient;
use fabro_llm::generate::{GenerateParams, generate};
@ -34,7 +35,7 @@ use fabro_types::settings::interp::InterpString;
use fabro_types::settings::server::ObjectStoreSettings;
use fabro_types::settings::{is_wildcard_host, validate_public_url_with_label};
use fabro_util::version::FABRO_VERSION;
use fabro_util::{Home, dev_token, session_secret};
use fabro_util::{Home, session_secret};
use fabro_vault::SecretType as VaultSecretType;
use object_store::aws::resolve_bucket_region;
use object_store::path::Path as ObjectStorePath;
@ -1562,6 +1563,7 @@ async fn post_install_finish(
let mut server_env_writes = object_store_env_plan.writes;
let server_env_removals = object_store_env_plan.removals;
let mut dev_token: Option<String> = None;
let mut dev_token_write = None;
match github {
GithubInstallState::Token(github) => {
if let Err(err) = write_token_settings(&mut settings_doc) {
@ -1576,7 +1578,7 @@ async fn post_install_finish(
let dev_token_path = Storage::new(state.storage_dir.as_ref())
.runtime_directory()
.dev_token_path();
let token = match dev_token::read_or_mint_dev_token_for_install(&dev_token_path) {
let prepared = match prepare_dev_token_write_for_install(&dev_token_path) {
Ok(value) => value,
Err(err) => {
return install_error_response(
@ -1585,7 +1587,8 @@ async fn post_install_finish(
);
}
};
dev_token = Some(token);
dev_token_write = prepared.write;
dev_token = Some(prepared.token);
}
GithubInstallState::App(github) => {
if let Err(err) = write_github_app_settings(
@ -1631,23 +1634,27 @@ async fn post_install_finish(
)]
let previous_settings = std::fs::read_to_string(state.config_path.as_ref()).ok();
if let Err(err) = persist_install_outputs_direct(
state.storage_dir.as_ref(),
&server_env_writes,
&server_env_removals,
&vault_secrets,
Some(&PendingSettingsWrite {
let persistence_plan = InstallPersistencePlan {
storage_dir: state.storage_dir.as_ref(),
settings_write: Some(PendingSettingsWrite {
path: state.config_path.as_ref(),
contents: &settings_toml,
previous_contents: previous_settings.as_deref(),
}),
) {
server_env_writes,
server_env_removals,
dev_token_write,
vault_writes: vault_secrets,
vault_removals: Vec::new(),
};
if let Err(err) = persistence_plan.persist_direct() {
error!(error = %err, "install persistence failed");
let status = StatusCode::INTERNAL_SERVER_ERROR;
let detail = err.to_string();
let title = status.canonical_reason().unwrap_or("Unknown").to_string();
let leftover_env_keys: Vec<String> = if err.server_env_applied {
server_env_writes
persistence_plan
.server_env_writes
.iter()
.map(|write| write.key.clone())
.collect()

52
lib/crates/fabro-server/tests/it/api/install.rs
View file

@ -11,13 +11,13 @@ use std::time::Duration;
use axum::body::Body;
use axum::http::{Request, StatusCode};
use fabro_config::{ServerSettingsBuilder, Storage};
use fabro_config::{ServerSettingsBuilder, Storage, envfile};
use fabro_install::OBJECT_STORE_MANAGED_COMMENT;
use fabro_model::ProviderId;
use fabro_server::install::{
InstallAppState, InstallFinishHook, InstallFinishInfo, build_install_router,
};
use fabro_util::{Home, dev_token};
use fabro_util::Home;
use fabro_vault::Vault;
use httpmock::Method::GET;
use httpmock::MockServer;
@ -929,18 +929,27 @@ async fn token_install_finish_persists_settings_env_and_vault() {
"127.0.0.1:32276"
);
let server_env = std::fs::read_to_string(
fabro_config::Storage::new(temp_dir.path())
.runtime_directory()
.env_path(),
)
.unwrap();
assert!(server_env.contains("SESSION_SECRET="));
assert!(server_env.contains("FABRO_DEV_TOKEN="));
assert!(!server_env.contains("AWS_ACCESS_KEY_ID="));
assert!(!server_env.contains("AWS_SECRET_ACCESS_KEY="));
let storage = fabro_config::Storage::new(temp_dir.path());
let server_env = envfile::read_env_file(&storage.runtime_directory().env_path()).unwrap();
assert!(server_env.contains_key(fabro_static::EnvVars::SESSION_SECRET));
assert!(server_env.contains_key(fabro_static::EnvVars::FABRO_DEV_TOKEN));
assert!(!server_env.contains_key(fabro_static::EnvVars::AWS_ACCESS_KEY_ID));
assert!(!server_env.contains_key(fabro_static::EnvVars::AWS_SECRET_ACCESS_KEY));
let finish_dev_token = finish_body["dev_token"]
.as_str()
.expect("token install should return a dev token");
let storage_dev_token =
fabro_util::dev_token::read_dev_token_file(&storage.runtime_directory().dev_token_path())
.expect("token install should write the storage dev token");
assert_eq!(storage_dev_token, finish_dev_token);
assert_eq!(
server_env
.get(fabro_static::EnvVars::FABRO_DEV_TOKEN)
.map(String::as_str),
Some(finish_dev_token)
);
let vault = Vault::load(fabro_config::Storage::new(temp_dir.path()).secrets_path()).unwrap();
let vault = Vault::load(storage.secrets_path()).unwrap();
assert!(vault.get("ANTHROPIC_API_KEY").is_some());
assert_eq!(vault.get("GITHUB_TOKEN"), Some("ghp_test_token"));
}
@ -2278,7 +2287,7 @@ async fn install_finish_failure_reports_only_env_keys_actually_removed() {
}
#[tokio::test]
async fn install_finish_failure_does_not_create_home_dev_token() {
async fn install_finish_failure_does_not_create_dev_token_files() {
let temp_dir = tempfile::tempdir().unwrap();
let home_root = tempfile::tempdir().unwrap();
let home = Home::new(home_root.path().join(".fabro"));
@ -2319,12 +2328,17 @@ async fn install_finish_failure_does_not_create_home_dev_token() {
!home.root().join("dev-token").exists(),
"home dev token file should not be created"
);
let storage_dev_token =
dev_token::read_dev_token_file(&storage.runtime_directory().dev_token_path())
.expect("storage dev token should exist");
assert!(
!storage.runtime_directory().dev_token_path().exists(),
"storage dev token file should not be created when persistence fails"
);
let server_env = std::fs::read_to_string(storage.runtime_directory().env_path()).unwrap();
assert!(server_env.contains(&format!("FABRO_DEV_TOKEN={storage_dev_token}")));
let server_env = envfile::read_env_file(&storage.runtime_directory().env_path()).unwrap();
assert!(
server_env
.get(fabro_static::EnvVars::FABRO_DEV_TOKEN)
.is_some_and(|value| !value.is_empty())
);
}
#[tokio::test]

21
lib/crates/fabro-util/src/dev_token.rs
View file

@ -63,20 +63,25 @@ pub fn read_dev_token_or_err(path: &Path) -> Result<String> {
}
}
pub fn read_or_mint_dev_token_for_install(path: &Path) -> Result<String> {
pub fn read_dev_token_for_install(path: &Path) -> Result<Option<String>> {
match fs::read_to_string(path) {
Ok(contents) => {
let token = contents.trim().to_string();
if validate_dev_token_format(&token) {
return Ok(token);
Ok(Some(token))
} else {
Err(anyhow!("invalid dev token format in {}", path.display()))
}
return Err(anyhow!("invalid dev token format in {}", path.display()));
}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => {
return Err(anyhow::Error::from(err))
.with_context(|| format!("read dev token {}", path.display()));
}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None),
Err(err) => Err(anyhow::Error::from(err))
.with_context(|| format!("read dev token {}", path.display())),
}
}
pub fn read_or_mint_dev_token_for_install(path: &Path) -> Result<String> {
if let Some(token) = read_dev_token_for_install(path)? {
return Ok(token);
}
let token = generate_dev_token();