scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Clean up SQLite secrets migration and fix CLI env credential regression

Browse source 
Review pass over the secrets-to-SQLite migration:

- Add SecretStore::open() consolidating the connect/migrate/import-legacy
  sequence repeated at five call sites; fabro-agent and fabro-cli drop
  their fabro-db dependency
- Restore process-env LLM credential lookup in the standalone CLI/agent
  sources via SqlVaultCredentialSource::new (regression: vault_only
  dropped the env fallback that VaultCredentialSource::new provided)
- Fix five install tests that still asserted against the legacy
  secrets.json, which the importer renames to .bak
- Make AppStateConfig.preloaded_vault required, deleting the fallback
  that re-read the already-renamed legacy file; drop the now-unused
  vault_path field and demote load_startup_vault to test-only
- Skip the snapshot clones and CAS retry in resolve() when the vault
  holds no OAuth secrets (per-request hot path)
- Remove dead persist_with_secret_store, the VaultSecretWrite alias,
  the secret_type_string one-liner (now SecretType::as_str), the
  impossible RowCountOverflow error, and duplicated row parsing
- Run check_crypto concurrently with the other diagnostics checks

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-07-11 13:41:31 -04:00 • committed by Scott Werner
parent 6860852c9c
commit f2cbc016ee
24 changed files with 167 additions and 192 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
Cargo.lock generated
View file

@ -2267,7 +2267,6 @@ dependencies = [
"dirs",
"fabro-auth",
"fabro-config",
"fabro-db",
"fabro-http",
"fabro-llm",
"fabro-macros",
@ -2410,7 +2409,6 @@ dependencies = [
"fabro-checkpoint",
"fabro-client",
"fabro-config",
"fabro-db",
"fabro-dump",
"fabro-environment",
"fabro-github",
@ -3300,6 +3298,7 @@ dependencies = [
name = "fabro-vault"
version = "0.302.0-nightly.1"
dependencies = [
"anyhow",
"chrono",
"fabro-db",
"fabro-static",

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

@ -25,7 +25,6 @@ workspace = true
clap.workspace = true
anyhow.workspace = true
fabro-auth = { path = "../fabro-auth" }
fabro-db = { path = "../fabro-db" }
fabro-config = { path = "../fabro-config", features = ["clap"] }
fabro-types = { path = "../fabro-types", features = ["clap"] }
fabro-llm = { path = "../fabro-llm" }

20
lib/crates/fabro-agent/src/cli.rs
View file

@ -12,7 +12,6 @@ use clap::{Args, Parser};
use fabro_auth::{CredentialSource, SqlVaultCredentialSource};
use fabro_config::Storage;
use fabro_config::user::default_storage_dir;
use fabro_db::Database;
use fabro_llm::Error as LlmError;
use fabro_llm::client::Client;
use fabro_llm::middleware::{Middleware, NextFn, NextStreamFn};
@ -24,7 +23,7 @@ use fabro_model::catalog::LlmCatalogSettings;
use fabro_model::{AgentProfileKind, Catalog, ModelHandle, ProviderId};
use fabro_static::EnvVars;
use fabro_util::terminal::Styles;
use fabro_vault::{SecretStore, import_legacy_json_once};
use fabro_vault::SecretStore;
use tokio::io::{AsyncWriteExt, stdout};
use tokio::signal;
use tokio::sync::Mutex as AsyncMutex;
@ -275,20 +274,11 @@ fn resolve_provider_id(catalog: &Catalog, args: &AgentArgs) -> anyhow::Result<Pr
}
async fn standalone_llm_source() -> anyhow::Result<Arc<dyn CredentialSource>> {
let storage_dir = default_storage_dir();
let storage = Storage::new(storage_dir);
let database = Database::connect(storage.sqlite_path())
let storage = Storage::new(default_storage_dir());
let store = SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await
.context("opening the Fabro database for secrets")?;
database
.migrate()
.await
.context("migrating the Fabro database for secrets")?;
import_legacy_json_once(database.pool(), storage.secrets_path())
.await
.context("importing legacy secrets into SQLite")?;
let store = Arc::new(SecretStore::new(database.clone_pool()));
Ok(Arc::new(SqlVaultCredentialSource::vault_only(store)))
.context("opening the Fabro secret store")?;
Ok(Arc::new(SqlVaultCredentialSource::new(Arc::new(store))))
}
fn profile_kind_for_provider(

19
lib/crates/fabro-auth/src/sql_vault_source.rs
View file

@ -17,6 +17,16 @@ pub struct SqlVaultCredentialSource {
}
impl SqlVaultCredentialSource {
#[must_use]
#[expect(
clippy::disallowed_methods,
reason = "SqlVaultCredentialSource::new owns the process-env fallback used after vault \
lookup."
)]
pub fn new(store: Arc<SecretStore>) -> Self {
Self::with_env_lookup(store, |name| std::env::var(name).ok())
}
#[must_use]
pub fn vault_only(store: Arc<SecretStore>) -> Self {
Self::with_env_lookup(store, |_| None)
@ -87,6 +97,15 @@ impl CredentialSource for SqlVaultCredentialSource {
async fn resolve(&self, catalog: &Catalog) -> anyhow::Result<ResolvedCredentials> {
for _ in 0..2 {
let before = self.store.snapshot().await?;
let has_oauth = before
.entries()
.values()
.any(|entry| entry.secret_type == SecretType::Oauth);
if !has_oauth {
// Only OAuth resolution can write back (token refresh); with no
// OAuth secrets, skip the snapshot clones and CAS machinery.
return self.source_for_snapshot(before).resolve(catalog).await;
}
let source = self.source_for_snapshot(before.clone());
let resolved = source.resolve(catalog).await?;
let after = source.snapshot().await;

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

@ -19,7 +19,6 @@ workspace = true
[dependencies]
fabro-auth = { path = "../fabro-auth" }
fabro-db = { path = "../fabro-db" }
fabro-config = { path = "../fabro-config" }
fabro-environment = { path = "../fabro-environment" }
fabro-llm = { path = "../fabro-llm" }

17
lib/crates/fabro-cli/src/command_context.rs
View file

@ -4,14 +4,13 @@ use std::sync::{Arc, OnceLock};
use anyhow::{Context as _, Result, bail};
use fabro_auth::{CredentialSource, SqlVaultCredentialSource};
use fabro_config::{CliLayer, Storage, load_llm_catalog_settings};
use fabro_db::Database;
use fabro_model::Catalog;
use fabro_types::UserSettings;
use fabro_types::settings::RunNamespace;
use fabro_types::settings::cli::{OutputFormat, OutputVerbosity};
use fabro_util::error::SharedError;
use fabro_util::printer::Printer;
use fabro_vault::{SecretStore, import_legacy_json_once};
use fabro_vault::SecretStore;
use tokio::sync::OnceCell;
use crate::args::{
@ -171,19 +170,11 @@ impl CommandContext {
.llm_source
.get_or_try_init(|| async move {
let storage = Storage::new(&storage_dir);
let database = Database::connect(storage.sqlite_path())
let store = SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await
.context("opening the Fabro database for secrets")?;
database
.migrate()
.await
.context("migrating the Fabro database for secrets")?;
import_legacy_json_once(database.pool(), storage.secrets_path())
.await
.context("importing legacy secrets into SQLite")?;
let store = Arc::new(SecretStore::new(database.clone_pool()));
.context("opening the Fabro secret store")?;
let source: Arc<dyn CredentialSource> =
Arc::new(SqlVaultCredentialSource::vault_only(store));
Arc::new(SqlVaultCredentialSource::new(Arc::new(store)));
Ok::<Arc<dyn CredentialSource>, anyhow::Error>(source)
})
.await?;

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

@ -29,7 +29,7 @@ use fabro_config::user::{SETTINGS_CONFIG_FILENAME, default_storage_dir};
use fabro_config::{Storage, UserSettingsBuilder, envfile};
use fabro_install::{
GITHUB_APP_VAULT_KEYS, GITHUB_INSTALL_SECRET_KEYS, InstallListenConfig, InstallPersistencePlan,
PendingDevTokenWrite, PendingSettingsWrite, VaultSecretWrite,
PendingDevTokenWrite, PendingSettingsWrite, SecretStoreWrite,
merge_server_settings as merge_server_settings_impl, prepare_dev_token_write_for_install,
restore_optional_file, rollback_dev_token_write, seed_environments_in_storage,
write_github_app_settings, write_token_settings,
@ -1349,7 +1349,7 @@ struct PendingGitHubInstallWrite<'a> {
settings_write: PendingSettingsWrite<'a>,
server_env_set: Vec<(String, String)>,
server_env_remove: Vec<&'static str>,
vault_set: Vec<VaultSecretWrite>,
vault_set: Vec<SecretStoreWrite>,
vault_remove: Vec<&'static str>,
}
@ -1630,7 +1630,7 @@ async fn run_install_github_inner(
match selection {
GitHubInstallSelection::Token { token } => {
write_token_settings(&mut doc)?;
vault_set.push(VaultSecretWrite {
vault_set.push(SecretStoreWrite {
name: GITHUB_TOKEN_SECRET_KEY.to_string(),
value: token,
secret_type: VaultSecretType::Token,
@ -1668,7 +1668,7 @@ async fn run_install_github_inner(
} else {
VaultSecretType::Token
};
vault_set.push(VaultSecretWrite {
vault_set.push(SecretStoreWrite {
name: key,
value,
secret_type,
@ -2174,11 +2174,9 @@ mod tests {
use super::*;
async fn load_secret_snapshot(storage: &Storage) -> Vault {
let database = fabro_db::Database::connect(storage.sqlite_path())
fabro_vault::SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await
.unwrap();
database.migrate().await.unwrap();
fabro_vault::SecretStore::new(database.clone_pool())
.unwrap()
.snapshot()
.await
.unwrap()
@ -3282,7 +3280,7 @@ client_id = "client-id"
GITHUB_APP_CLIENT_SECRET_KEY,
GITHUB_APP_WEBHOOK_SECRET_KEY,
],
vault_set: vec![VaultSecretWrite {
vault_set: vec![SecretStoreWrite {
name: GITHUB_TOKEN_SECRET_KEY.to_string(),
value: "token".to_string(),
secret_type: VaultSecretType::Token,
@ -3348,19 +3346,19 @@ client_id = "client-id"
GITHUB_APP_WEBHOOK_SECRET_KEY,
],
vault_set: vec![
VaultSecretWrite {
SecretStoreWrite {
name: GITHUB_APP_PRIVATE_KEY_KEY.to_string(),
value: "private".to_string(),
secret_type: VaultSecretType::File,
description: None,
},
VaultSecretWrite {
SecretStoreWrite {
name: GITHUB_APP_CLIENT_SECRET_KEY.to_string(),
value: "client".to_string(),
secret_type: VaultSecretType::Token,
description: None,
},
VaultSecretWrite {
SecretStoreWrite {
name: GITHUB_APP_WEBHOOK_SECRET_KEY.to_string(),
value: "webhook".to_string(),
secret_type: VaultSecretType::Token,
@ -3436,7 +3434,7 @@ client_id = "client-id"
},
server_env_set: Vec::new(),
server_env_remove: vec![GITHUB_APP_PRIVATE_KEY_KEY, GITHUB_APP_CLIENT_SECRET_KEY],
vault_set: vec![VaultSecretWrite {
vault_set: vec![SecretStoreWrite {
name: "bad-secret-name".to_string(),
value: "token".to_string(),
secret_type: VaultSecretType::Token,

20
lib/crates/fabro-cli/src/commands/run/runner.rs
View file

@ -9,7 +9,6 @@ use fabro_api::types::RunManifest;
use fabro_client::ServerTarget;
use fabro_config::user::active_settings_path;
use fabro_config::{ServerSettingsBuilder, Storage, load_llm_catalog_settings};
use fabro_db::Database;
use fabro_interview::{
AnswerSubmission, ControlInterviewer, WORKER_CONTROL_INVALID_CURSOR_REASON,
WORKER_CONTROL_PONG_TIMEOUT_REASON, WORKER_CONTROL_WS_LIVENESS_TIMEOUT,
@ -25,7 +24,7 @@ use fabro_types::{
ArtifactUpload, EventBody, FailureReason, Principal, RunBlobId, RunEvent, RunId,
WorkflowSettings,
};
use fabro_vault::{SecretStore, Vault, import_legacy_json_once};
use fabro_vault::{SecretStore, Vault};
use fabro_workflow::artifact_upload::{ArtifactSink, StageArtifactUploader};
use fabro_workflow::event::{Emitter, RunEventSink};
use fabro_workflow::operations::{self, StartServices};
@ -279,17 +278,14 @@ async fn load_worker_vault(storage_dir: Option<&Path>) -> Result<Option<Arc<Asyn
};
let storage = Storage::new(storage_dir);
let database = Database::connect(storage.sqlite_path())
let vault = SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await
.with_context(|| format!("failed to open database from {}", storage.root().display()))?;
database
.migrate()
.await
.context("migrating worker database")?;
import_legacy_json_once(database.pool(), storage.secrets_path())
.await
.context("importing legacy worker secrets into SQLite")?;
let vault = SecretStore::new(database.clone_pool())
.with_context(|| {
format!(
"failed to open worker secret store from {}",
storage.root().display()
)
})?
.snapshot()
.await
.context("loading worker secrets snapshot")?

15
lib/crates/fabro-cli/src/commands/server/start.rs
View file

@ -11,7 +11,6 @@ use fabro_config::bind::{Bind, BindRequest};
use fabro_config::daemon::ServerDaemon;
use fabro_config::user::default_settings_path;
use fabro_config::{RuntimeDirectory, Storage};
use fabro_db::Database;
use fabro_server::jwt_auth::auth_method_name;
use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs, resolve_runtime_server_settings_for_start};
use fabro_server::{
@ -21,7 +20,7 @@ use fabro_static::EnvVars;
use fabro_types::settings::{LogDestination, ServerAuthMethod};
use fabro_util::printer::Printer;
use fabro_util::terminal::Styles;
use fabro_vault::{SecretStore, import_legacy_json_once};
use fabro_vault::SecretStore;
use tokio::process::Command as TokioCommand;
use tokio::task::spawn_blocking;
use tokio::time;
@ -293,17 +292,9 @@ async fn execute_daemon(
validate_startup_configuration(&resolved_settings)?;
let storage = Storage::new(storage_dir);
migrate_startup_vault(storage.secrets_path());
let database = Database::connect(storage.sqlite_path())
let startup_vault = SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await
.context("opening the Fabro database for startup validation")?;
database
.migrate()
.await
.context("migrating the Fabro database for startup validation")?;
import_legacy_json_once(database.pool(), storage.secrets_path())
.await
.context("importing legacy secrets into SQLite")?;
let startup_vault = SecretStore::new(database.clone_pool())
.context("opening the Fabro secret store for startup validation")?
.snapshot()
.await
.context("loading secrets for startup validation")?;

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

@ -13,6 +13,16 @@ use fabro_vault::{SecretType, Vault};
const INSTALL_COMMAND_TIMEOUT: Duration = Duration::from_secs(30);
async fn load_secret_snapshot(storage: &Storage) -> Vault {
fabro_vault::SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await
.expect("test secret store should open")
.snapshot()
.await
.expect("test secret snapshot should load")
.into_vault()
}
#[test]
fn help() {
let context = test_context!();
@ -611,8 +621,8 @@ fn github_non_interactive_requires_strategy() {
assert!(stderr.contains("install github --non-interactive requires --strategy"));
}
#[test]
fn github_non_interactive_token_reconfigures_existing_app_install() {
#[tokio::test]
async fn github_non_interactive_token_reconfigures_existing_app_install() {
let mut context = test_context!();
let storage_dir = context.home_dir.join("install-storage");
context.manage_storage_dir(&storage_dir);
@ -758,7 +768,7 @@ mode = "keep-me"
assert!(!server_env.contains_key("GITHUB_APP_WEBHOOK_SECRET"));
assert_eq!(server_env.get("KEEP_ME").map(String::as_str), Some("1"));
let vault = Vault::load(Storage::new(&storage_dir).secrets_path()).unwrap();
let vault = load_secret_snapshot(&Storage::new(&storage_dir)).await;
assert_eq!(vault.get("GITHUB_TOKEN"), Some("token-from-gh"));
assert_eq!(vault.get("GITHUB_APP_PRIVATE_KEY"), None);
assert_eq!(vault.get("GITHUB_APP_CLIENT_SECRET"), None);

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

@ -10,8 +10,8 @@ use fabro_config::{Storage, envfile};
use fabro_static::EnvVars;
use fabro_types::settings::run::EnvironmentProvider;
use fabro_util::dev_token;
pub use fabro_vault::SecretStoreWrite as VaultSecretWrite;
use fabro_vault::{SecretStore, import_legacy_json_once};
use fabro_vault::SecretStore;
pub use fabro_vault::SecretStoreWrite;
#[derive(Debug, Clone, Copy)]
pub struct PendingSettingsWrite<'a> {
@ -60,7 +60,7 @@ pub struct InstallPersistencePlan<'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_writes: Vec<SecretStoreWrite>,
pub vault_removals: Vec<String>,
}
@ -577,7 +577,7 @@ fn persist_server_env_secrets(
async fn persist_vault_secrets_direct(
storage_dir: &Path,
secrets: &[VaultSecretWrite],
secrets: &[SecretStoreWrite],
removals: &[String],
) -> Result<()> {
if secrets.is_empty() && removals.is_empty() {
@ -585,9 +585,8 @@ async fn persist_vault_secrets_direct(
}
let storage = Storage::new(storage_dir);
let database = open_migrated_database(storage_dir).await?;
import_legacy_json_once(database.pool(), storage.secrets_path()).await?;
SecretStore::new(database.clone_pool())
SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await?
.apply(removals, secrets)
.await?;
Ok(())
@ -690,28 +689,13 @@ impl InstallPersistencePlan<'_> {
Ok(())
}
pub async fn persist_with_secret_store(
&self,
store: &SecretStore,
) -> std::result::Result<(), PersistInstallOutputsError> {
let removed_env_keys = self.persist_files()?;
if let Err(err) = store
.apply(&self.vault_removals, &self.vault_writes)
.await
.map_err(anyhow::Error::new)
{
return Err(self.secret_persistence_error(err, removed_env_keys));
}
Ok(())
}
}
pub async fn persist_install_outputs_direct(
storage_dir: &Path,
server_env_writes: &[envfile::EnvFileUpdate],
server_env_removals: &[envfile::EnvFileRemoval],
vault_secrets: &[VaultSecretWrite],
vault_secrets: &[SecretStoreWrite],
settings_write: Option<&PendingSettingsWrite<'_>>,
) -> std::result::Result<(), PersistInstallOutputsError> {
InstallPersistencePlan {
@ -739,11 +723,9 @@ mod tests {
use fabro_vault::{SecretType as VaultSecretType, Vault};
async fn load_secret_snapshot(storage: &Storage) -> Vault {
let database = fabro_db::Database::connect(storage.sqlite_path())
SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await
.unwrap();
database.migrate().await.unwrap();
SecretStore::new(database.clone_pool())
.unwrap()
.snapshot()
.await
.unwrap()
@ -754,7 +736,7 @@ mod tests {
InstallListenConfig, InstallObjectStoreCredentialMode, InstallObjectStoreSelection,
InstallPersistencePlan, InstallSandboxSelection, OBJECT_STORE_ACCESS_KEY_ID_ENV,
OBJECT_STORE_MANAGED_COMMENT, OBJECT_STORE_SECRET_ACCESS_KEY_ENV, PendingSettingsWrite,
SecretStore, VaultSecretWrite, default_web_url, merge_server_settings,
SecretStore, SecretStoreWrite, 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,
@ -1036,7 +1018,7 @@ stale = "remove-me"
comment: None,
}],
&[],
&[VaultSecretWrite {
&[SecretStoreWrite {
name: "bad-secret-name".to_string(),
value: "boom".to_string(),
secret_type: VaultSecretType::Token,
@ -1085,7 +1067,7 @@ stale = "remove-me"
server_env_writes: Vec::new(),
server_env_removals: Vec::new(),
dev_token_write: None,
vault_writes: vec![VaultSecretWrite {
vault_writes: vec![SecretStoreWrite {
name: "NEW_SECRET".to_string(),
value: "new".to_string(),
secret_type: VaultSecretType::Token,
@ -1129,7 +1111,7 @@ stale = "remove-me"
}],
server_env_removals: Vec::new(),
dev_token_write: None,
vault_writes: vec![VaultSecretWrite {
vault_writes: vec![SecretStoreWrite {
name: "bad-secret-name".to_string(),
value: "boom".to_string(),
secret_type: VaultSecretType::Token,
@ -1252,7 +1234,7 @@ stale = "remove-me"
server_env_writes: Vec::new(),
server_env_removals: Vec::new(),
dev_token_write: prepared.write,
vault_writes: vec![VaultSecretWrite {
vault_writes: vec![SecretStoreWrite {
name: "bad-secret-name".to_string(),
value: "boom".to_string(),
secret_type: VaultSecretType::Token,

4
lib/crates/fabro-server/src/diagnostics.rs
View file

@ -89,14 +89,14 @@ fn validate_session_secret(value: &str) -> Result<(), String> {
}
pub async fn run_all(state: &AppState) -> DiagnosticsReport {
let (llm, github, docker_sandbox, cloud_sandbox, brave) = tokio::join!(
let (llm, github, docker_sandbox, cloud_sandbox, brave, crypto) = tokio::join!(
check_llm_providers(state),
check_github_app(state),
check_docker_sandbox(state),
check_cloud_sandbox(state),
check_brave_search(state),
check_crypto(state),
);
let crypto = check_crypto(state).await;
DiagnosticsReport {
version: FABRO_VERSION.to_string(),

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

@ -19,7 +19,7 @@ use fabro_config::envfile::{EnvFileRemoval, EnvFileUpdate};
use fabro_install::{
GITHUB_APP_VAULT_KEYS, GITHUB_INSTALL_SECRET_KEYS, InstallListenConfig, InstallPersistencePlan,
InstallSandboxSelection, OBJECT_STORE_ACCESS_KEY_ID_ENV, OBJECT_STORE_SECRET_ACCESS_KEY_ENV,
PendingSettingsWrite, VaultSecretWrite, merge_server_settings,
PendingSettingsWrite, SecretStoreWrite, merge_server_settings,
prepare_dev_token_write_for_install, seed_default_environment_in_storage,
write_github_app_settings, write_object_store_settings, write_sandbox_settings,
write_token_settings,
@ -1573,7 +1573,7 @@ async fn post_install_finish(
}
let mut vault_secrets = Vec::new();
if let InstallSandboxProviderState::Daytona { api_key } = &sandbox.provider {
vault_secrets.push(VaultSecretWrite {
vault_secrets.push(SecretStoreWrite {
name: EnvVars::DAYTONA_API_KEY.to_string(),
value: api_key.expose_secret().to_string(),
secret_type: VaultSecretType::Token,
@ -1585,7 +1585,7 @@ async fn post_install_finish(
Ok(name) => name,
Err(err) => return install_error_response(StatusCode::UNPROCESSABLE_ENTITY, err),
};
vault_secrets.push(VaultSecretWrite {
vault_secrets.push(SecretStoreWrite {
name,
value: provider.api_key,
secret_type: VaultSecretType::Token,
@ -1612,7 +1612,7 @@ async fn post_install_finish(
if let Err(err) = write_token_settings(&mut settings_doc) {
return install_error_response(StatusCode::INTERNAL_SERVER_ERROR, err.to_string());
}
vault_secrets.push(VaultSecretWrite {
vault_secrets.push(SecretStoreWrite {
name: EnvVars::GITHUB_TOKEN.to_string(),
value: github.token,
secret_type: VaultSecretType::Token,
@ -1649,20 +1649,20 @@ async fn post_install_finish(
) {
return install_error_response(StatusCode::INTERNAL_SERVER_ERROR, err.to_string());
}
vault_secrets.push(VaultSecretWrite {
vault_secrets.push(SecretStoreWrite {
name: EnvVars::GITHUB_APP_PRIVATE_KEY.to_string(),
value: BASE64_STANDARD.encode(github.pem.as_bytes()),
secret_type: VaultSecretType::File,
description: None,
});
vault_secrets.push(VaultSecretWrite {
vault_secrets.push(SecretStoreWrite {
name: EnvVars::GITHUB_APP_CLIENT_SECRET.to_string(),
value: github.client_secret,
secret_type: VaultSecretType::Token,
description: None,
});
if let Some(secret) = github.webhook_secret {
vault_secrets.push(VaultSecretWrite {
vault_secrets.push(SecretStoreWrite {
name: EnvVars::GITHUB_APP_WEBHOOK_SECRET.to_string(),
value: secret,
secret_type: VaultSecretType::Token,
@ -2502,10 +2502,9 @@ mod tests {
assert!(!server_env.contains_key(EnvVars::GITHUB_APP_CLIENT_SECRET));
assert!(!server_env.contains_key(EnvVars::GITHUB_APP_WEBHOOK_SECRET));
let database = fabro_db::Database::connect(storage.sqlite_path())
let vault = fabro_vault::SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await
.unwrap();
let vault = fabro_vault::SecretStore::new(database.clone_pool())
.unwrap()
.snapshot()
.await
.unwrap();

4
lib/crates/fabro-server/src/lib.rs
View file

@ -57,6 +57,4 @@ mod worker_token;
pub use error::{ApiError, Error, Result};
pub use run_manifest::workflow_bundle_from_manifest;
pub use server_secrets::process_env_snapshot;
pub use startup::{
load_startup_vault, migrate_startup_vault, validate_startup, validate_startup_configuration,
};
pub use startup::{migrate_startup_vault, validate_startup, validate_startup_configuration};

3
lib/crates/fabro-server/src/serve.rs
View file

@ -790,9 +790,8 @@ where
max_concurrent_runs,
store,
artifact_store,
vault_path,
db_pool,
preloaded_vault: Some(startup_vault.into_vault()),
preloaded_vault: startup_vault.into_vault(),
server_secrets,
env_lookup,
github_api_base_url: None,

12
lib/crates/fabro-server/src/server.rs
View file

@ -163,7 +163,6 @@ use crate::request_id::{self, RequestId};
use crate::run_files::{FilesInFlight, new_files_in_flight};
use crate::server_secrets::{LlmClientResult, ServerSecrets};
use crate::spawn_env::apply_render_graph_env;
use crate::startup::load_startup_vault;
use crate::worker_control::{LocalWorkerControlBus, WorkerControlBus, WorkerControlBusError};
use crate::worker_runtime::{
LocalWorkerRuntime, WorkerExit, WorkerLaunchSpec, WorkerRef, WorkerRuntime,
@ -1251,9 +1250,8 @@ pub(crate) struct AppStateConfig {
pub(crate) max_concurrent_runs: usize,
pub(crate) store: Arc<Database>,
pub(crate) artifact_store: ArtifactStore,
pub(crate) vault_path: PathBuf,
pub(crate) db_pool: DbPool,
pub(crate) preloaded_vault: Option<Vault>,
pub(crate) preloaded_vault: Vault,
pub(crate) server_secrets: ServerSecrets,
pub(crate) env_lookup: EnvLookup,
pub(crate) github_api_base_url: Option<String>,
@ -2354,7 +2352,6 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result<Arc<AppS
max_concurrent_runs,
store,
artifact_store,
vault_path,
db_pool,
preloaded_vault,
server_secrets,
@ -2397,10 +2394,7 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result<Arc<AppS
);
let variables = Arc::new(VariableStore::new(db_pool.clone()));
let secret_store = Arc::new(SecretStore::new(db_pool));
let vault = match preloaded_vault {
Some(vault) => vault,
None => load_startup_vault(&vault_path)?,
};
let vault = preloaded_vault;
// Read vault secrets needed for synchronous setup before we wrap the vault in
// an async lock for the rest of AppState.
let daytona_api_key = vault.get(EnvVars::DAYTONA_API_KEY).map(str::to_string);
@ -2533,6 +2527,8 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result<Arc<AppS
registry_factory_override,
slack_service,
slack_started: AtomicBool::new(false),
// Startup snapshot for the sync router build; rotating the webhook
// secret requires a server restart.
github_webhook_secret: vault.get(WEBHOOK_SECRET_ENV).map(str::to_string),
}))
}

26
lib/crates/fabro-server/src/server/tests.rs
View file

@ -2052,8 +2052,7 @@ fn slack_app_state_with_settings_and_secret_sources(
store,
artifact_store,
db_pool: test_db_pool_for_vault_path(&vault_path).expect("test db pool should build"),
vault_path,
preloaded_vault: Some(vault),
preloaded_vault: vault,
server_secrets: load_test_server_secrets(server_env_path, server_secret_env),
env_lookup: default_env_lookup(),
github_api_base_url: None,
@ -2211,8 +2210,7 @@ fn slack_service_respects_disabled_server_config_even_with_vault_tokens() {
store,
artifact_store,
db_pool: test_db_pool_for_vault_path(&vault_path).expect("test db pool should build"),
vault_path,
preloaded_vault: Some(vault),
preloaded_vault: vault,
server_secrets: load_test_server_secrets(
tempfile::tempdir().unwrap().path().join("server.env"),
HashMap::new(),
@ -2555,6 +2553,9 @@ methods = ["dev-token"]
let (store, artifact_store) = test_store_bundle();
let vault_path = test_secret_store_path();
let server_env_path = vault_path.with_file_name("server.env");
let db_pool = test_db_pool_for_vault_path(&vault_path).expect("test db pool should build");
let preloaded_vault = crate::test_support::test_secret_snapshot(db_pool.clone())
.expect("test secret snapshot should build");
let Err(err) = build_app_state(AppStateConfig {
resolved_settings: resolved_runtime_settings_for_tests(
server_settings,
@ -2565,9 +2566,8 @@ methods = ["dev-token"]
max_concurrent_runs: 5,
store,
artifact_store,
db_pool: test_db_pool_for_vault_path(&vault_path).expect("test db pool should build"),
vault_path,
preloaded_vault: None,
db_pool,
preloaded_vault,
server_secrets: ServerSecrets::load(server_env_path, HashMap::new()).unwrap(),
env_lookup: default_env_lookup(),
github_api_base_url: None,
@ -2679,6 +2679,8 @@ async fn build_app_state_migrates_legacy_vault_file_on_boot() {
fn build_test_app_state_with_vault_path(vault_path: &Path) -> anyhow::Result<Arc<AppState>> {
let (store, artifact_store) = test_store_bundle();
let db_pool = test_db_pool_for_vault_path(vault_path)?;
let preloaded_vault = crate::test_support::test_secret_snapshot(db_pool.clone())?;
build_app_state(AppStateConfig {
resolved_settings: resolved_runtime_settings_for_tests(
default_test_server_settings(),
@ -2689,9 +2691,8 @@ fn build_test_app_state_with_vault_path(vault_path: &Path) -> anyhow::Result<Arc
max_concurrent_runs: 5,
store,
artifact_store,
db_pool: test_db_pool_for_vault_path(vault_path)?,
vault_path: vault_path.to_path_buf(),
preloaded_vault: None,
db_pool,
preloaded_vault,
server_secrets: load_test_server_secrets(
vault_path.with_file_name("server.env"),
HashMap::new(),
@ -6059,6 +6060,8 @@ fn create_github_token_app_state_with_env_lookup_and_llm_catalog_settings(
.expect("test github token should be writable");
}
let db_pool = test_db_pool_for_vault_path(&vault_path).expect("test db pool should build");
let preloaded_vault = crate::test_support::test_secret_snapshot(db_pool.clone())
.expect("test secret snapshot should build");
let config = AppStateConfig {
resolved_settings: resolved_runtime_settings_for_tests(
github_token_settings(),
@ -6070,8 +6073,7 @@ fn create_github_token_app_state_with_env_lookup_and_llm_catalog_settings(
store,
artifact_store,
db_pool,
vault_path,
preloaded_vault: None,
preloaded_vault,
server_secrets: load_test_server_secrets(server_env_path, HashMap::new()),
env_lookup: Arc::new(env_lookup),
github_api_base_url,

4
lib/crates/fabro-server/src/startup.rs
View file

@ -1,6 +1,7 @@
use std::collections::HashMap;
use std::path::Path;
#[cfg(test)]
use anyhow::Context as _;
use fabro_static::EnvVars;
use fabro_types::settings::ServerNamespace;
@ -53,7 +54,8 @@ pub fn migrate_startup_vault(vault_path: impl AsRef<Path>) {
}
}
pub fn load_startup_vault(vault_path: impl AsRef<Path>) -> anyhow::Result<Vault> {
#[cfg(test)]
fn load_startup_vault(vault_path: impl AsRef<Path>) -> anyhow::Result<Vault> {
let vault_path = vault_path.as_ref();
migrate_startup_vault(vault_path);
Vault::load(vault_path.to_path_buf())

5
lib/crates/fabro-server/src/test_support.rs
View file

@ -264,8 +264,7 @@ impl TestAppStateBuilder {
store,
artifact_store,
db_pool,
vault_path,
preloaded_vault: Some(preloaded_vault),
preloaded_vault,
server_secrets: load_test_server_secrets(server_env_path, self.server_secret_env),
env_lookup: self.env_lookup,
github_api_base_url: None,
@ -288,7 +287,7 @@ impl TestAppStateBuilder {
clippy::disallowed_methods,
reason = "sync test builders may run inside Tokio; a dedicated thread avoids a nested runtime"
)]
fn test_secret_snapshot(pool: DbPool) -> anyhow::Result<Vault> {
pub(crate) fn test_secret_snapshot(pool: DbPool) -> anyhow::Result<Vault> {
std::thread::spawn(move || {
let runtime = TokioRuntimeBuilder::new_current_thread()
.enable_all()

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

@ -35,6 +35,17 @@ fn spa_fixture_root() -> PathBuf {
PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/spa")
}
async fn load_secret_snapshot(storage_dir: &std::path::Path) -> Vault {
let storage = Storage::new(storage_dir);
fabro_vault::SecretStore::open(storage.sqlite_path(), storage.secrets_path())
.await
.expect("test secret store should open")
.snapshot()
.await
.expect("test secret snapshot should load")
.into_vault()
}
fn assert_sandbox_provider_policy(
settings: &str,
local_enabled: bool,
@ -994,7 +1005,7 @@ async fn token_install_finish_persists_settings_env_and_vault() {
Some(finish_dev_token)
);
let vault = Vault::load(storage.secrets_path()).unwrap();
let vault = load_secret_snapshot(temp_dir.path()).await;
assert!(vault.get("ANTHROPIC_API_KEY").is_some());
assert_eq!(vault.get("GITHUB_TOKEN"), Some("ghp_test_token"));
}
@ -1080,7 +1091,7 @@ async fn browser_install_finish_with_skipped_llm_persists_no_llm_credentials() {
assert!(server_env.contains("SESSION_SECRET="));
assert!(server_env.contains("FABRO_DEV_TOKEN="));
let vault = Vault::load(fabro_config::Storage::new(temp_dir.path()).secrets_path()).unwrap();
let vault = load_secret_snapshot(temp_dir.path()).await;
assert!(
vault.get("OPENAI_API_KEY").is_none() && vault.get("OPENAI_CODEX").is_none(),
"skipped LLM install should not write any OpenAI vault entries"
@ -2551,7 +2562,7 @@ async fn sandbox_daytona_resave_without_api_key_preserves_saved_key() {
)
.await;
let vault = Vault::load(Storage::new(temp_dir.path()).secrets_path()).unwrap();
let vault = load_secret_snapshot(temp_dir.path()).await;
assert_eq!(vault.get("DAYTONA_API_KEY"), Some(api_key));
}
@ -2604,7 +2615,7 @@ async fn sandbox_switching_from_daytona_to_docker_drops_saved_key() {
assert_no_legacy_environment_dir(&temp_dir);
let default_environment = seeded_default_environment(&temp_dir).await;
assert_eq!(default_environment.settings.provider.to_string(), "docker");
let vault = Vault::load(Storage::new(temp_dir.path()).secrets_path()).unwrap();
let vault = load_secret_snapshot(temp_dir.path()).await;
assert_eq!(vault.get("DAYTONA_API_KEY"), None);
}
@ -2730,7 +2741,7 @@ async fn daytona_install_finish_writes_settings_and_vault_secret() {
if content.contains("buildpack-deps:noble")
));
let vault = Vault::load(Storage::new(temp_dir.path()).secrets_path()).unwrap();
let vault = load_secret_snapshot(temp_dir.path()).await;
assert_eq!(vault.get("DAYTONA_API_KEY"), Some(api_key));
}

7
lib/crates/fabro-types/src/secret.rs
View file

@ -27,6 +27,13 @@ pub enum SecretType {
File,
}
impl SecretType {
#[must_use]
pub fn as_str(self) -> &'static str {
self.into()
}
}
/// JSON shape stored when [`SecretType::Oauth`] is used.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct OAuthCredential {

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

@ -13,6 +13,7 @@ doctest = false
workspace = true
[dependencies]
anyhow.workspace = true
chrono.workspace = true
fabro-db = { path = "../fabro-db" }
fabro-static = { path = "../fabro-static" }

2
lib/crates/fabro-vault/src/lib.rs
View file

@ -110,6 +110,8 @@ impl Vault {
})
}
/// Builds a detached in-memory vault with no backing file: mutations
/// update memory only and are never persisted to disk.
#[must_use]
pub fn from_entries(entries: HashMap<String, SecretEntry>) -> Self {
Self {

69
lib/crates/fabro-vault/src/store.rs
View file

@ -4,7 +4,7 @@ use std::path::{Path, PathBuf};
use std::str::FromStr as _;
use chrono::{DateTime, Utc};
use fabro_db::DbPool;
use fabro_db::{Database, DbPool};
use fabro_types::{OAuthCredential, SecretMetadata, SecretType};
use sqlx::sqlite::SqliteRow;
use sqlx::{Row as _, Sqlite, Transaction};
@ -86,17 +86,14 @@ pub enum SecretStoreError {
#[source]
source: std::io::Error,
},
#[error("secret row count {count} exceeds SQLite integer range")]
RowCountOverflow { count: usize },
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ImportReport {
pub source_path: PathBuf,
pub backup_path: PathBuf,
pub imported_rows: i64,
pub skipped_rows: i64,
pub imported_rows: usize,
pub skipped_rows: usize,
pub secret_names: Vec<String>,
}
@ -160,6 +157,18 @@ impl SecretStore {
Self { pool }
}
/// Opens the Fabro database at `sqlite_path`, runs migrations, imports any
/// legacy secrets JSON at `legacy_secrets_path`, and returns the store.
pub async fn open(
sqlite_path: impl AsRef<Path>,
legacy_secrets_path: impl AsRef<Path>,
) -> anyhow::Result<Self> {
let database = Database::connect(sqlite_path).await?;
database.migrate().await?;
import_legacy_json_once(database.pool(), legacy_secrets_path).await?;
Ok(Self::new(database.clone_pool()))
}
pub async fn get(&self, name: &str) -> Result<Option<SecretEntry>, SecretStoreError> {
let row = sqlx::query(
"SELECT name, secret_type, value, description, revision, created_at, updated_at \
@ -270,7 +279,7 @@ impl SecretStore {
RETURNING name, secret_type, value, description, revision, created_at, updated_at
",
)
.bind(secret_type_string(secret_type))
.bind(secret_type.as_str())
.bind(value)
.bind(now)
.bind(name)
@ -329,7 +338,7 @@ async fn upsert_secret(
",
)
.bind(name)
.bind(secret_type_string(secret_type))
.bind(secret_type.as_str())
.bind(value)
.bind(description)
.bind(now)
@ -391,8 +400,8 @@ pub async fn import_legacy_json_once(
let report = ImportReport {
source_path: source_path.to_path_buf(),
backup_path,
imported_rows: row_count(imported_names.len())?,
skipped_rows: row_count(skipped_rows)?,
imported_rows: imported_names.len(),
skipped_rows,
secret_names: imported_names,
};
info!(
@ -420,7 +429,7 @@ async fn insert_legacy_entry(
",
)
.bind(name)
.bind(secret_type_string(entry.secret_type))
.bind(entry.secret_type.as_str())
.bind(&entry.value)
.bind(entry.description.as_deref())
.bind(entry.created_at.to_rfc3339())
@ -431,26 +440,10 @@ async fn insert_legacy_entry(
}
fn entry_from_row(row: &SqliteRow) -> Result<(String, SecretEntry), SecretStoreError> {
let name = row.try_get::<String, _>("name")?;
let type_value = row.try_get::<String, _>("secret_type")?;
let secret_type =
SecretType::from_str(&type_value).map_err(|_| SecretStoreError::StoredType {
name: name.clone(),
value: type_value,
})?;
validate_stored_name(&name, secret_type)?;
let created_at = parse_timestamp(
&name,
"created_at",
&row.try_get::<String, _>("created_at")?,
)?;
let updated_at = parse_timestamp(
&name,
"updated_at",
&row.try_get::<String, _>("updated_at")?,
)?;
let metadata = metadata_from_row(row)?;
let name = metadata.name;
let value = row.try_get::<String, _>("value")?;
if secret_type == SecretType::Oauth {
if metadata.secret_type == SecretType::Oauth {
validate_oauth_json(&value).map_err(|source| SecretStoreError::StoredOauth {
name: name.clone(),
source,
@ -462,10 +455,10 @@ fn entry_from_row(row: &SqliteRow) -> Result<(String, SecretEntry), SecretStoreE
}
let entry = SecretEntry {
value,
secret_type,
description: row.try_get("description")?,
created_at,
updated_at,
secret_type: metadata.secret_type,
description: metadata.description,
created_at: metadata.created_at,
updated_at: metadata.updated_at,
revision,
};
Ok((name, entry))
@ -497,10 +490,6 @@ fn metadata_from_row(row: &SqliteRow) -> Result<SecretMetadata, SecretStoreError
})
}
fn secret_type_string(secret_type: SecretType) -> &'static str {
secret_type.into()
}
fn parse_timestamp(
name: &str,
column: &'static str,
@ -561,7 +550,3 @@ fn legacy_backup_path(source_path: &Path, imported_at: DateTime<Utc>) -> PathBuf
file_name.push(format!(".imported-{timestamp}.bak"));
source_path.with_file_name(file_name)
}
fn row_count(count: usize) -> Result<i64, SecretStoreError> {
i64::try_from(count).map_err(|_| SecretStoreError::RowCountOverflow { count })
}