scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-11 22:53:00 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

refactor(install): consolidate shared primitives in fabro-install crate

Browse source 
The `fabro-install` crate was introduced for the web wizard but the CLI
kept its own copies of the same JWT keypair generation, TOML merging,
and GitHub auth settings helpers. Delete the duplicates and route the
CLI through `fabro_install::*`. The CLI keeps a thin
`merge_server_settings` wrapper because it only ever binds TCP and
derives the authority from `--web-url`.

Also tighten `persist_install_outputs_direct` to take its
`PendingSettingsWrite` argument by reference (satisfies
`needless_pass_by_value`) and pull the remaining absolute paths in the
crate's test module into `use` statements, clearing the nightly clippy
warnings that this branch was carrying.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-19 13:54:47 -04:00
parent acb6b3f9d6
commit c6ff36d9fb
No known key found for this signature in database
5 changed files with 24 additions and 204 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -1590,6 +1590,7 @@ dependencies = [
"fabro-graphviz",
"fabro-hooks",
"fabro-http",
"fabro-install",
"fabro-interview",
"fabro-llm",
"fabro-macros",

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

@ -27,6 +27,7 @@ fabro-github = { path = "../fabro-github" }
fabro-agent = { path = "../fabro-agent" }
fabro-devcontainer = { path = "../fabro-devcontainer" }
fabro-hooks = { path = "../fabro-hooks" }
fabro-install = { path = "../fabro-install" }
fabro-interview = { path = "../fabro-interview" }
fabro-mcp = { path = "../fabro-mcp" }
fabro-proc = { path = "../fabro-proc" }

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

@ -18,6 +18,10 @@ use fabro_api::types::{CreateSecretRequest, SecretType as ApiSecretType};
use fabro_auth::{AuthCredential, AuthMethod, codex_oauth_config, credential_id_for};
use fabro_config::user::{SETTINGS_CONFIG_FILENAME, legacy_default_storage_root};
use fabro_config::{ResolveError, Storage, envfile, legacy_env};
use fabro_install::{
InstallListenConfig, generate_jwt_keypair, merge_server_settings as merge_server_settings_impl,
write_github_app_settings, write_token_settings,
};
use fabro_model::Provider;
use fabro_server::bind::Bind;
use fabro_server::serve;
@ -32,8 +36,6 @@ use fabro_util::{dev_token, path, session_secret};
use fabro_vault::{SecretType as VaultSecretType, Vault};
use futures::future::BoxFuture;
use rand::Rng;
use ring::rand::SystemRandom;
use ring::signature::{Ed25519KeyPair, KeyPair as _};
use tokio::net::TcpListener;
use tokio::process::Command as TokioCommand;
use tokio::sync::oneshot;
@ -52,59 +54,11 @@ use crate::shared::provider_auth::{
};
use crate::{server_client, user_config};
// ---------------------------------------------------------------------------
// JWT keypair generation
// ---------------------------------------------------------------------------
const ED25519_SPKI_PREFIX: [u8; 12] = [
0x30, 0x2A, 0x30, 0x05, 0x06, 0x03, 0x2B, 0x65, 0x70, 0x03, 0x21, 0x00,
];
const ED25519_PUBLIC_KEY_LEN: usize = 32;
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";
fn pem_encode(label: &str, bytes: &[u8]) -> String {
let body = BASE64_STANDARD.encode(bytes);
let mut pem = String::new();
pem.push_str("-----BEGIN ");
pem.push_str(label);
pem.push_str("-----\n");
for chunk in body.as_bytes().chunks(64) {
pem.push_str(std::str::from_utf8(chunk).expect("base64 output should be valid UTF-8"));
pem.push('\n');
}
pem.push_str("-----END ");
pem.push_str(label);
pem.push_str("-----\n");
pem
}
fn ed25519_public_key_spki(public_key: &[u8]) -> Result<Vec<u8>> {
if public_key.len() != ED25519_PUBLIC_KEY_LEN {
bail!("generated Ed25519 public key had unexpected length");
}
let mut spki = Vec::with_capacity(ED25519_SPKI_PREFIX.len() + public_key.len());
spki.extend_from_slice(&ED25519_SPKI_PREFIX);
spki.extend_from_slice(public_key);
Ok(spki)
}
fn generate_jwt_keypair() -> Result<(String, String)> {
let pkcs8 = Ed25519KeyPair::generate_pkcs8(&SystemRandom::new())
.map_err(|_| anyhow!("failed to generate Ed25519 keypair"))?;
let keypair = Ed25519KeyPair::from_pkcs8(pkcs8.as_ref())
.map_err(|_| anyhow!("failed to parse generated Ed25519 keypair"))?;
let public_der = ed25519_public_key_spki(keypair.public_key().as_ref())?;
Ok((
pem_encode("PRIVATE KEY", pkcs8.as_ref()),
pem_encode("PUBLIC KEY", &public_der),
))
}
// ---------------------------------------------------------------------------
// Auth status display
// ---------------------------------------------------------------------------
@ -143,19 +97,6 @@ fn print_auth_status(
// Config TOML generation
// ---------------------------------------------------------------------------
fn root_table_mut(doc: &mut toml::Value) -> Result<&mut toml::Table> {
doc.as_table_mut()
.context("settings.toml root is not a table")
}
fn ensure_table<'a>(table: &'a mut toml::Table, key: &str) -> Result<&'a mut toml::Table> {
table
.entry(key.to_string())
.or_insert_with(|| toml::Value::Table(toml::Table::default()))
.as_table_mut()
.with_context(|| format!("settings.toml [{key}] is not a table"))
}
/// Default web URL used by `fabro install` when `--web-url` is omitted.
pub(crate) fn default_web_url() -> String {
format!("http://127.0.0.1:{}", serve::DEFAULT_TCP_PORT)
@ -170,132 +111,11 @@ fn merge_server_settings(doc: &mut toml::Value, web_url: &str) -> Result<()> {
.split('/')
.next()
.unwrap_or(web_url);
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")?;
listen.insert("type".to_string(), toml::Value::String("tcp".to_string()));
listen.insert(
"address".to_string(),
toml::Value::String(authority.to_string()),
);
let web = ensure_table(server, "web")?;
web.insert("enabled".to_string(), toml::Value::Boolean(true));
web.insert("url".to_string(), toml::Value::String(web_url.to_string()));
let auth = ensure_table(server, "auth")?;
auth.insert(
"methods".to_string(),
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()));
Ok(())
}
fn github_integration_table(doc: &mut toml::Value) -> Result<&mut toml::Table> {
let root = doc
.as_table_mut()
.context("settings.toml root is not a table")?;
let server = root
.entry("server")
.or_insert(toml::Value::Table(toml::Table::default()));
let server_table = server
.as_table_mut()
.context("settings.toml [server] is not a table")?;
let integrations = server_table
.entry("integrations")
.or_insert(toml::Value::Table(toml::Table::default()));
let integrations_table = integrations
.as_table_mut()
.context("settings.toml [server.integrations] is not a table")?;
let github = integrations_table
.entry("github")
.or_insert(toml::Value::Table(toml::Table::default()));
github
.as_table_mut()
.context("settings.toml [server.integrations.github] is not a table")
}
fn write_token_settings(doc: &mut toml::Value) -> Result<()> {
if let Some(server) = doc.get_mut("server").and_then(toml::Value::as_table_mut) {
if let Some(auth) = server.get_mut("auth").and_then(toml::Value::as_table_mut) {
if let Some(methods) = auth.get_mut("methods").and_then(toml::Value::as_array_mut) {
methods.retain(|value| value.as_str() != Some("github"));
if methods.is_empty() {
methods.push(toml::Value::String("dev-token".to_string()));
}
}
auth.remove("github");
}
}
let github = github_integration_table(doc)?;
github.insert("strategy".into(), toml::Value::String("token".to_string()));
github.remove("app_id");
github.remove("slug");
github.remove("client_id");
Ok(())
}
fn write_github_app_settings(
doc: &mut toml::Value,
app_id: &str,
slug: &str,
client_id: &str,
allowed_usernames: &[String],
) -> Result<()> {
anyhow::ensure!(
!allowed_usernames.is_empty(),
"GitHub App install requires at least one allowed GitHub username"
);
let root = root_table_mut(doc)?;
let server = ensure_table(root, "server")?;
let auth = ensure_table(server, "auth")?;
let methods = auth
.entry("methods".to_string())
.or_insert_with(|| toml::Value::Array(Vec::new()))
.as_array_mut()
.context("settings.toml [server.auth].methods is not an array")?;
if !methods.iter().any(|value| value.as_str() == Some("github")) {
methods.push(toml::Value::String("github".to_string()));
}
let github_auth = ensure_table(auth, "github")?;
github_auth.insert(
"allowed_usernames".to_string(),
toml::Value::Array(
allowed_usernames
.iter()
.cloned()
.map(toml::Value::String)
.collect(),
),
);
let github = github_integration_table(doc)?;
github.insert("strategy".into(), toml::Value::String("app".to_string()));
github.insert("app_id".into(), toml::Value::String(app_id.to_string()));
github.insert("slug".into(), toml::Value::String(slug.to_string()));
github.insert(
"client_id".into(),
toml::Value::String(client_id.to_string()),
);
Ok(())
merge_server_settings_impl(
doc,
web_url,
&InstallListenConfig::Tcp(authority.to_string()),
)
}
#[cfg(test)]

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

@ -290,11 +290,11 @@ pub fn persist_install_outputs_direct(
storage_dir: &Path,
server_env_secrets: &[(String, String)],
vault_secrets: &[VaultSecretWrite],
settings_write: Option<PendingSettingsWrite<'_>>,
settings_write: Option<&PendingSettingsWrite<'_>>,
) -> Result<()> {
persist_server_env_secrets(storage_dir, server_env_secrets)?;
if let Some(ref write) = settings_write {
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()))?;
@ -308,7 +308,7 @@ pub fn persist_install_outputs_direct(
if let Err(err) = persist_vault_secrets_direct(storage_dir, vault_secrets) {
let mut rollback_failures = Vec::new();
if let Some(ref write) = settings_write {
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());
}
@ -332,7 +332,7 @@ pub fn persist_install_outputs_direct(
#[cfg(test)]
mod tests {
use fabro_config::Storage;
use fabro_config::{Storage, envfile};
use fabro_vault::{SecretType as VaultSecretType, Vault};
use super::{
@ -353,7 +353,7 @@ mod tests {
#[test]
fn config_toml_has_auth_strategies() {
use fabro_types::settings::SettingsLayer;
use fabro_types::settings::{ServerAuthMethod, SettingsLayer};
let toml_str = format_config_toml();
let cfg: SettingsLayer = fabro_config::parse_settings_layer(&toml_str).unwrap();
@ -362,10 +362,7 @@ mod tests {
.as_ref()
.and_then(|s| s.auth.as_ref())
.expect("server.auth should be set");
assert_eq!(
auth.methods,
Some(vec![fabro_types::settings::ServerAuthMethod::DevToken])
);
assert_eq!(auth.methods, Some(vec![ServerAuthMethod::DevToken]));
}
#[test]
@ -464,7 +461,7 @@ name = "custom"
secret_type: VaultSecretType::Environment,
description: None,
}],
Some(PendingSettingsWrite {
Some(&PendingSettingsWrite {
path: &settings_path,
contents: "_version = 1\n[server]\nfoo = \"bar\"\n",
previous_contents: Some("_version = 1\n[server]\n"),
@ -481,8 +478,7 @@ name = "custom"
assert_eq!(restored.get("EXISTING_SECRET"), Some("keep"));
assert_eq!(restored.get("bad-secret-name"), None);
let server_env =
fabro_config::envfile::read_env_file(&storage.server_state().env_path()).unwrap();
let server_env = envfile::read_env_file(&storage.server_state().env_path()).unwrap();
assert_eq!(
server_env.get("SESSION_SECRET").map(String::as_str),
Some("session")
@ -491,6 +487,8 @@ name = "custom"
#[test]
fn merge_server_settings_keeps_tcp_bind_separate_from_public_web_url() {
use fabro_types::settings::server::ServerListenSettings;
let mut doc = toml::Value::Table(toml::Table::default());
merge_server_settings(
&mut doc,
@ -506,10 +504,10 @@ name = "custom"
let resolved =
fabro_config::resolve_server_from_file(&settings).expect("settings should resolve");
match resolved.listen {
fabro_types::settings::server::ServerListenSettings::Tcp { address, .. } => {
ServerListenSettings::Tcp { address, .. } => {
assert_eq!(address.to_string(), "0.0.0.0:32276");
}
fabro_types::settings::server::ServerListenSettings::Unix { .. } => {
ServerListenSettings::Unix { .. } => {
panic!("expected tcp listen settings");
}
}

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

@ -903,7 +903,7 @@ async fn post_install_finish(
state.storage_dir.as_ref(),
&server_env_secrets,
&vault_secrets,
Some(PendingSettingsWrite {
Some(&PendingSettingsWrite {
path: state.config_path.as_ref(),
contents: &settings_toml,
previous_contents: previous_settings.as_deref(),