mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-07 08:27:12 +00:00
Merge pull request #844 from fabro-sh/remove/expired-secret-migrations
Remove expired startup secret migrations
This commit is contained in:
commit
5d290a609b
13 changed files with 19 additions and 878 deletions
|
|
@ -20,7 +20,7 @@ The crate-local `src/migrations.rs` module is the registry. It imports numbered
|
|||
Examples:
|
||||
|
||||
- `fabro-config` owns settings-file migrations.
|
||||
- `fabro-server` owns server startup migrations for `server.env` and vault files.
|
||||
- `fabro-server` owns server startup activation migrations for SQLite blob storage and run history.
|
||||
|
||||
Keep migration APIs `pub(crate)` unless another crate genuinely orchestrates the migration.
|
||||
|
||||
|
|
@ -125,7 +125,7 @@ If a migration removes entries from a file after writing another store, write th
|
|||
|
||||
Choose the error policy deliberately.
|
||||
|
||||
Use warn-and-continue only when the normal path may still succeed and compatibility is best-effort. The legacy vault-entry migration does this because an unreadable legacy shape should not block loading an otherwise usable vault file.
|
||||
Use warn-and-continue only when the normal path may still succeed and compatibility is best-effort.
|
||||
|
||||
Return an error when the migration found data it must move or rewrite and cannot do so safely. This gives operators a precise migration failure instead of a later, misleading startup error.
|
||||
|
||||
|
|
|
|||
|
|
@ -119,7 +119,7 @@ Bootstrap secrets come from one of two sources:
|
|||
|
||||
Optional integration secrets are provisioned into the vault, usually with `fabro secret set` or `fabro install`.
|
||||
|
||||
There is no startup-time secret generation. A temporary startup migration moves recognized legacy optional secrets from process env or `server.env` into the vault, removes matching `server.env` entries after writing a backup, and logs conflicts by key name only. Runtime lookup remains vault-only after that migration step. See [migrations-strategy.md](migrations-strategy.md) for the migration pattern.
|
||||
There is no startup-time secret generation or import of optional integration secrets from process env or `server.env`. The compatibility migrations for those sources and pre-token/OAuth vault entries have been removed. The separate one-time import of current-format `secrets.json` entries into SQLite remains supported.
|
||||
|
||||
## Subprocess Boundaries
|
||||
|
||||
|
|
|
|||
|
|
@ -371,7 +371,7 @@ Fabro splits server-runtime secrets into two scopes:
|
|||
|
||||
`server.env` is not used for Slack, Daytona, Brave Search, Venice Search, LLM provider keys, `GITHUB_TOKEN`, or GitHub App private key/client secret/webhook secret. Configure those optional integrations with `fabro secret set`, `fabro provider login`, or `fabro install`.
|
||||
|
||||
During startup, Fabro temporarily migrates recognized legacy optional integration secrets from process env or `server.env` into the vault. When a matching `server.env` entry can be safely removed, Fabro writes a hidden backup beside `server.env` first. Process env values cannot be cleaned up automatically, so remove those from your deployment environment after the vault contains the secret.
|
||||
Startup does not import optional integration secrets from process env or `server.env`, or rewrite old `credential` / `environment` vault entries. Provision these secrets with the commands above before upgrading an installation that still uses those retired sources or formats.
|
||||
|
||||
Fabro no longer auto-loads `.env` files. Provider API keys are required for the models you want to use; everything else is optional.
|
||||
|
||||
|
|
|
|||
|
|
@ -13,9 +13,7 @@ use fabro_config::user::default_settings_path;
|
|||
use fabro_config::{RuntimeDirectory, Storage};
|
||||
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::{
|
||||
migrate_startup_vault, process_env_snapshot, validate_startup, validate_startup_configuration,
|
||||
};
|
||||
use fabro_server::{process_env_snapshot, validate_startup, validate_startup_configuration};
|
||||
use fabro_static::EnvVars;
|
||||
use fabro_types::settings::{LogDestination, ServerAuthMethod};
|
||||
use fabro_util::printer::Printer;
|
||||
|
|
@ -291,7 +289,6 @@ async fn execute_daemon(
|
|||
}
|
||||
validate_startup_configuration(&resolved_settings)?;
|
||||
let storage = Storage::new(storage_dir);
|
||||
migrate_startup_vault(storage.secrets_path());
|
||||
let startup_vault = SecretStore::open_snapshot(storage.sqlite_path(), storage.secrets_path())
|
||||
.await
|
||||
.context("loading secrets for startup validation")?;
|
||||
|
|
|
|||
|
|
@ -292,6 +292,15 @@ methods = []
|
|||
server_env: &[("SESSION_SECRET", TEST_SESSION_SECRET)],
|
||||
expected_error: "Fabro server refuses to start: github auth is enabled but GITHUB_APP_CLIENT_SECRET is not configured in the vault.",
|
||||
},
|
||||
StartupFailureCase {
|
||||
name: "github-client-secret-only-in-server-env",
|
||||
settings: GITHUB_SETTINGS,
|
||||
server_env: &[
|
||||
("SESSION_SECRET", TEST_SESSION_SECRET),
|
||||
("GITHUB_APP_CLIENT_SECRET", "unprovisioned-client-secret"),
|
||||
],
|
||||
expected_error: "Fabro server refuses to start: github auth is enabled but GITHUB_APP_CLIENT_SECRET is not configured in the vault.",
|
||||
},
|
||||
StartupFailureCase {
|
||||
name: "empty-auth-methods",
|
||||
settings: EMPTY_AUTH_METHODS_SETTINGS,
|
||||
|
|
|
|||
|
|
@ -1,275 +0,0 @@
|
|||
//! Temporary compatibility shim for pre-token/oauth vault files.
|
||||
//!
|
||||
//! Delete this module after 2026-08-18, once supported installs have had a
|
||||
//! release window to rewrite `credential` / `environment` entries to the
|
||||
//! `oauth` / `token` schemas.
|
||||
|
||||
#![expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "Temporary startup migration uses synchronous vault file I/O before serving requests."
|
||||
)]
|
||||
|
||||
use std::collections::HashSet;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use anyhow::{Context, bail};
|
||||
use fabro_auth::{OAuthConfig, OAuthCredential, OAuthTokens};
|
||||
use serde::Deserialize;
|
||||
use serde_json::{Map, Value};
|
||||
|
||||
pub(crate) const REMOVAL_DEADLINE: &str = "2026-08-18";
|
||||
|
||||
#[derive(Debug, Default, PartialEq, Eq)]
|
||||
pub(crate) struct LegacyVaultMigrationReport {
|
||||
pub(crate) migrated_entries: usize,
|
||||
pub(crate) skipped_entries: usize,
|
||||
pub(crate) backup_path: Option<PathBuf>,
|
||||
}
|
||||
|
||||
impl LegacyVaultMigrationReport {
|
||||
pub(crate) fn changed(&self) -> bool {
|
||||
self.migrated_entries > 0 || self.skipped_entries > 0
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct LegacyAuthCredential {
|
||||
provider: String,
|
||||
#[serde(flatten)]
|
||||
details: LegacyAuthDetails,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
#[serde(tag = "type", rename_all = "snake_case")]
|
||||
enum LegacyAuthDetails {
|
||||
ApiKey {
|
||||
key: String,
|
||||
},
|
||||
CodexOauth {
|
||||
tokens: OAuthTokens,
|
||||
config: OAuthConfig,
|
||||
#[serde(default)]
|
||||
account_id: Option<String>,
|
||||
},
|
||||
}
|
||||
|
||||
pub(crate) fn migrate_legacy_vault_file(path: &Path) -> anyhow::Result<LegacyVaultMigrationReport> {
|
||||
let contents = match std::fs::read_to_string(path) {
|
||||
Ok(contents) => contents,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
|
||||
return Ok(LegacyVaultMigrationReport::default());
|
||||
}
|
||||
Err(err) => return Err(err).with_context(|| format!("read vault {}", path.display())),
|
||||
};
|
||||
let entries = parse_vault_entries(&contents)?;
|
||||
let (next_entries, migrated_entries, skipped_entries) = rewrite_entries(entries);
|
||||
let mut report = LegacyVaultMigrationReport {
|
||||
migrated_entries,
|
||||
skipped_entries,
|
||||
backup_path: None,
|
||||
};
|
||||
if !report.changed() {
|
||||
return Ok(report);
|
||||
}
|
||||
|
||||
let backup_path = backup_vault_file(path)?;
|
||||
write_vault_entries(path, &next_entries)?;
|
||||
report.backup_path = Some(backup_path);
|
||||
Ok(report)
|
||||
}
|
||||
|
||||
fn parse_vault_entries(contents: &str) -> anyhow::Result<Map<String, Value>> {
|
||||
let value: Value = serde_json::from_str(contents).context("parse vault JSON")?;
|
||||
match value {
|
||||
Value::Object(entries) => Ok(entries),
|
||||
_ => bail!("vault JSON root must be an object"),
|
||||
}
|
||||
}
|
||||
|
||||
fn rewrite_entries(entries: Map<String, Value>) -> (Map<String, Value>, usize, usize) {
|
||||
let mut next_entries = Map::new();
|
||||
let mut occupied = HashSet::new();
|
||||
for (name, entry) in &entries {
|
||||
if matches!(
|
||||
entry.get("type").and_then(Value::as_str),
|
||||
Some("token" | "oauth" | "file")
|
||||
) {
|
||||
next_entries.insert(name.clone(), entry.clone());
|
||||
occupied.insert(name.clone());
|
||||
}
|
||||
}
|
||||
|
||||
let mut migrated_entries = 0;
|
||||
let mut skipped_entries = 0;
|
||||
for (name, entry) in entries {
|
||||
match entry.get("type").and_then(Value::as_str) {
|
||||
Some("token" | "oauth" | "file") => {}
|
||||
Some("environment") => {
|
||||
if insert_rewritten_entry(
|
||||
&mut next_entries,
|
||||
&mut occupied,
|
||||
name,
|
||||
rewrite_entry(entry, "token", None),
|
||||
) {
|
||||
migrated_entries += 1;
|
||||
} else {
|
||||
skipped_entries += 1;
|
||||
}
|
||||
}
|
||||
Some("credential") => match legacy_credential_entry(&name, &entry) {
|
||||
Some((target_name, rewritten)) => {
|
||||
if insert_rewritten_entry(
|
||||
&mut next_entries,
|
||||
&mut occupied,
|
||||
target_name,
|
||||
rewritten,
|
||||
) {
|
||||
migrated_entries += 1;
|
||||
} else {
|
||||
skipped_entries += 1;
|
||||
}
|
||||
}
|
||||
None => skipped_entries += 1,
|
||||
},
|
||||
_ => skipped_entries += 1,
|
||||
}
|
||||
}
|
||||
|
||||
(next_entries, migrated_entries, skipped_entries)
|
||||
}
|
||||
|
||||
fn insert_rewritten_entry(
|
||||
entries: &mut Map<String, Value>,
|
||||
occupied: &mut HashSet<String>,
|
||||
name: String,
|
||||
entry: Value,
|
||||
) -> bool {
|
||||
if !occupied.insert(name.clone()) {
|
||||
return false;
|
||||
}
|
||||
entries.insert(name, entry);
|
||||
true
|
||||
}
|
||||
|
||||
fn legacy_credential_entry(name: &str, entry: &Value) -> Option<(String, Value)> {
|
||||
let value = entry.get("value").and_then(Value::as_str)?;
|
||||
let credential: LegacyAuthCredential = serde_json::from_str(value).ok()?;
|
||||
match credential.details {
|
||||
LegacyAuthDetails::ApiKey { key } => {
|
||||
let target_name = api_key_secret_name(&credential.provider)?;
|
||||
Some((
|
||||
target_name,
|
||||
rewrite_entry(entry.clone(), "token", Some(key)),
|
||||
))
|
||||
}
|
||||
LegacyAuthDetails::CodexOauth {
|
||||
tokens,
|
||||
config,
|
||||
account_id,
|
||||
} if credential.provider == "openai" && name == "openai_codex" => {
|
||||
let credential = OAuthCredential {
|
||||
tokens,
|
||||
config,
|
||||
account_id,
|
||||
};
|
||||
let value = serde_json::to_string(&credential).ok()?;
|
||||
Some((
|
||||
"OPENAI_CODEX".to_string(),
|
||||
rewrite_entry(entry.clone(), "oauth", Some(value)),
|
||||
))
|
||||
}
|
||||
LegacyAuthDetails::CodexOauth { .. } => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn rewrite_entry(mut entry: Value, secret_type: &str, value: Option<String>) -> Value {
|
||||
if let Value::Object(fields) = &mut entry {
|
||||
fields.insert("type".to_string(), Value::String(secret_type.to_string()));
|
||||
if let Some(value) = value {
|
||||
fields.insert("value".to_string(), Value::String(value));
|
||||
}
|
||||
}
|
||||
entry
|
||||
}
|
||||
|
||||
fn api_key_secret_name(provider: &str) -> Option<String> {
|
||||
let mut name = String::new();
|
||||
for ch in provider.chars() {
|
||||
if ch.is_ascii_alphanumeric() {
|
||||
name.push(ch.to_ascii_uppercase());
|
||||
} else if !name.ends_with('_') {
|
||||
name.push('_');
|
||||
}
|
||||
}
|
||||
while name.ends_with('_') {
|
||||
name.pop();
|
||||
}
|
||||
if name.is_empty() {
|
||||
return None;
|
||||
}
|
||||
if !name.ends_with("_API_KEY") {
|
||||
name.push_str("_API_KEY");
|
||||
}
|
||||
Some(name)
|
||||
}
|
||||
|
||||
fn backup_vault_file(path: &Path) -> anyhow::Result<PathBuf> {
|
||||
let parent = path.parent().unwrap_or_else(|| Path::new("."));
|
||||
let file_name = path
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.unwrap_or("secrets.json");
|
||||
let backup_path = parent.join(format!(
|
||||
".{file_name}.legacy-vault-migration-{}.bak",
|
||||
ulid::Ulid::new()
|
||||
));
|
||||
std::fs::copy(path, &backup_path).with_context(|| {
|
||||
format!(
|
||||
"copy vault {} to backup {}",
|
||||
path.display(),
|
||||
backup_path.display()
|
||||
)
|
||||
})?;
|
||||
set_private_permissions(&backup_path)?;
|
||||
Ok(backup_path)
|
||||
}
|
||||
|
||||
fn write_vault_entries(path: &Path, entries: &Map<String, Value>) -> anyhow::Result<()> {
|
||||
let parent = path.parent().unwrap_or_else(|| Path::new("."));
|
||||
std::fs::create_dir_all(parent)
|
||||
.with_context(|| format!("create vault directory {}", parent.display()))?;
|
||||
let file_name = path
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.unwrap_or("secrets.json");
|
||||
let tmp_path = parent.join(format!(
|
||||
".{file_name}.legacy-vault-migration-tmp-{}",
|
||||
ulid::Ulid::new()
|
||||
));
|
||||
let json = serde_json::to_vec_pretty(entries).context("serialize migrated vault JSON")?;
|
||||
std::fs::write(&tmp_path, json)
|
||||
.with_context(|| format!("write migrated vault temp file {}", tmp_path.display()))?;
|
||||
set_private_permissions(&tmp_path)?;
|
||||
std::fs::rename(&tmp_path, path).with_context(|| {
|
||||
format!(
|
||||
"rename migrated vault temp file {} to {}",
|
||||
tmp_path.display(),
|
||||
path.display()
|
||||
)
|
||||
})?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn set_private_permissions(path: &Path) -> anyhow::Result<()> {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))
|
||||
.with_context(|| format!("set private permissions on {}", path.display()))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
fn set_private_permissions(_path: &Path) -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -1,172 +0,0 @@
|
|||
//! Temporary compatibility shim for optional secrets that used to live in
|
||||
//! `server.env`.
|
||||
//!
|
||||
//! Delete this migration after 2026-08-18, once supported installs have had a
|
||||
//! release window to move optional integration secrets into the vault.
|
||||
|
||||
#![expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "Temporary startup migration uses synchronous file I/O before serving requests."
|
||||
)]
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use anyhow::Context as _;
|
||||
use fabro_config::envfile::{self, EnvFileRemoval};
|
||||
use fabro_static::{EnvVars, optional_vault_secrets};
|
||||
use fabro_vault::{SecretStore, SecretStoreWrite, SecretType};
|
||||
|
||||
pub(crate) const REMOVAL_DEADLINE: &str = "2026-08-18";
|
||||
|
||||
#[derive(Debug, Default, PartialEq, Eq)]
|
||||
pub(crate) struct OptionalServerEnvSecretsMigrationReport {
|
||||
pub(crate) migrated_secrets: usize,
|
||||
pub(crate) removed_env_entries: usize,
|
||||
pub(crate) preserved_env_entries: usize,
|
||||
pub(crate) backup_path: Option<PathBuf>,
|
||||
pub(crate) warnings: Vec<String>,
|
||||
}
|
||||
|
||||
impl OptionalServerEnvSecretsMigrationReport {
|
||||
pub(crate) fn changed(&self) -> bool {
|
||||
self.migrated_secrets > 0 || self.removed_env_entries > 0
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn migrate_to_store(
|
||||
store: &SecretStore,
|
||||
server_env_path: &Path,
|
||||
env_entries: &HashMap<String, String>,
|
||||
) -> anyhow::Result<OptionalServerEnvSecretsMigrationReport> {
|
||||
let server_env_entries = envfile::read_env_file(server_env_path)
|
||||
.with_context(|| format!("read server env file {}", server_env_path.display()))?;
|
||||
let stored = store.snapshot().await?;
|
||||
let mut writes = Vec::new();
|
||||
let mut env_removals = Vec::new();
|
||||
let mut warnings = Vec::new();
|
||||
let mut preserved_env_entries = 0;
|
||||
|
||||
for &name in optional_vault_secrets() {
|
||||
let process_value = env_entries.get(name);
|
||||
let file_value = server_env_entries.get(name);
|
||||
if let Some(entry) = stored.get_entry(name) {
|
||||
if let Some(file_value) = file_value {
|
||||
if file_value == &entry.value {
|
||||
env_removals.push(env_removal(name));
|
||||
} else {
|
||||
preserved_env_entries += 1;
|
||||
warnings.push(format!(
|
||||
"Preserved {name} in server.env because the secret store already contains a different value"
|
||||
));
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
let value = match (process_value, file_value) {
|
||||
(Some(value), Some(file_value)) => {
|
||||
if value == file_value {
|
||||
env_removals.push(env_removal(name));
|
||||
} else {
|
||||
preserved_env_entries += 1;
|
||||
warnings.push(format!(
|
||||
"Preserved {name} in server.env because process env takes precedence and the file value differs"
|
||||
));
|
||||
}
|
||||
Some(value)
|
||||
}
|
||||
(Some(value), None) => Some(value),
|
||||
(None, Some(value)) => {
|
||||
env_removals.push(env_removal(name));
|
||||
Some(value)
|
||||
}
|
||||
(None, None) => None,
|
||||
};
|
||||
if let Some(value) = value {
|
||||
writes.push(SecretStoreWrite {
|
||||
name: name.to_string(),
|
||||
value: value.clone(),
|
||||
secret_type: secret_type_for(name),
|
||||
description: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let mut report = OptionalServerEnvSecretsMigrationReport {
|
||||
migrated_secrets: writes.len(),
|
||||
removed_env_entries: 0,
|
||||
preserved_env_entries,
|
||||
backup_path: None,
|
||||
warnings,
|
||||
};
|
||||
if writes.is_empty() && env_removals.is_empty() {
|
||||
return Ok(report);
|
||||
}
|
||||
|
||||
store.apply(&[], &writes).await?;
|
||||
if !env_removals.is_empty() {
|
||||
let backup_path = backup_server_env_file(server_env_path)?;
|
||||
let update_report =
|
||||
envfile::update_env_file_with_report(server_env_path, env_removals, Vec::new())
|
||||
.with_context(|| {
|
||||
format!(
|
||||
"remove migrated optional secrets from {}",
|
||||
server_env_path.display()
|
||||
)
|
||||
})?;
|
||||
report.removed_env_entries = update_report.removed_keys.len();
|
||||
report.backup_path = Some(backup_path);
|
||||
}
|
||||
Ok(report)
|
||||
}
|
||||
|
||||
fn secret_type_for(name: &str) -> SecretType {
|
||||
if name == EnvVars::GITHUB_APP_PRIVATE_KEY {
|
||||
SecretType::File
|
||||
} else {
|
||||
SecretType::Token
|
||||
}
|
||||
}
|
||||
|
||||
fn env_removal(name: &str) -> EnvFileRemoval {
|
||||
EnvFileRemoval {
|
||||
key: name.to_string(),
|
||||
comment: None,
|
||||
}
|
||||
}
|
||||
|
||||
fn backup_server_env_file(path: &Path) -> anyhow::Result<PathBuf> {
|
||||
let parent = path.parent().unwrap_or_else(|| Path::new("."));
|
||||
let file_name = path
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.unwrap_or("server.env");
|
||||
let backup_path = parent.join(format!(
|
||||
".{file_name}.optional-server-env-secrets-to-vault-migration-{}.bak",
|
||||
ulid::Ulid::new()
|
||||
));
|
||||
std::fs::copy(path, &backup_path).with_context(|| {
|
||||
format!(
|
||||
"copy server env {} to backup {}",
|
||||
path.display(),
|
||||
backup_path.display()
|
||||
)
|
||||
})?;
|
||||
set_private_permissions(&backup_path)?;
|
||||
Ok(backup_path)
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn set_private_permissions(path: &Path) -> anyhow::Result<()> {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))
|
||||
.with_context(|| format!("set permissions on {}", path.display()))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
fn set_private_permissions(_path: &Path) -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -59,4 +59,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::{migrate_startup_vault, validate_startup, validate_startup_configuration};
|
||||
pub use startup::{validate_startup, validate_startup_configuration};
|
||||
|
|
|
|||
|
|
@ -1,12 +1,3 @@
|
|||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
|
||||
use fabro_vault::SecretStore;
|
||||
|
||||
#[path = "../migrations/2026051801_legacy_vault_entries.rs"]
|
||||
mod legacy_vault_entries;
|
||||
#[path = "../migrations/2026052501_optional_server_env_secrets_to_vault.rs"]
|
||||
mod optional_server_env_secrets_to_vault;
|
||||
#[path = "../migrations/sqlite_activation_backup.rs"]
|
||||
mod sqlite_activation_backup;
|
||||
#[path = "../migrations/2026082301_sqlite_blob_activation.rs"]
|
||||
|
|
@ -14,24 +5,5 @@ mod sqlite_blob_activation;
|
|||
#[path = "../migrations/2026082801_sqlite_run_history_activation.rs"]
|
||||
mod sqlite_run_history_activation;
|
||||
|
||||
pub(crate) use legacy_vault_entries::REMOVAL_DEADLINE as LEGACY_VAULT_REMOVAL_DEADLINE;
|
||||
pub(crate) use optional_server_env_secrets_to_vault::REMOVAL_DEADLINE as OPTIONAL_SERVER_ENV_SECRETS_REMOVAL_DEADLINE;
|
||||
pub(crate) use sqlite_blob_activation::activate_blob_storage;
|
||||
pub(crate) use sqlite_run_history_activation::activate_run_history;
|
||||
|
||||
pub(crate) type LegacyVaultMigrationReport = legacy_vault_entries::LegacyVaultMigrationReport;
|
||||
pub(crate) type OptionalServerEnvSecretsMigrationReport =
|
||||
optional_server_env_secrets_to_vault::OptionalServerEnvSecretsMigrationReport;
|
||||
|
||||
pub(crate) fn migrate_legacy_vault_file(path: &Path) -> anyhow::Result<LegacyVaultMigrationReport> {
|
||||
legacy_vault_entries::migrate_legacy_vault_file(path)
|
||||
}
|
||||
|
||||
pub(crate) async fn migrate_optional_server_env_secrets_to_store(
|
||||
store: &SecretStore,
|
||||
server_env_path: &Path,
|
||||
env_entries: &HashMap<String, String>,
|
||||
) -> anyhow::Result<OptionalServerEnvSecretsMigrationReport> {
|
||||
optional_server_env_secrets_to_vault::migrate_to_store(store, server_env_path, env_entries)
|
||||
.await
|
||||
}
|
||||
|
|
|
|||
|
|
@ -39,7 +39,7 @@ use crate::server::{
|
|||
spawn_automation_scheduler, spawn_pull_request_creation_supervisor, spawn_scheduler,
|
||||
};
|
||||
use crate::server_secrets::{ServerSecrets, process_env_snapshot};
|
||||
use crate::startup::{migrate_startup_vault, resolve_startup, validate_startup_configuration};
|
||||
use crate::startup::{resolve_startup, validate_startup_configuration};
|
||||
use crate::{migrations, static_files};
|
||||
|
||||
pub const DEFAULT_TCP_PORT: u16 = 32276;
|
||||
|
|
@ -666,7 +666,6 @@ where
|
|||
let resolved_server_settings = resolved_app_settings.server_settings.server.clone();
|
||||
validate_startup_configuration(&resolved_server_settings)?;
|
||||
let env_entries = process_env_snapshot();
|
||||
migrate_startup_vault(&vault_path);
|
||||
let bind_request = resolve_bind_request_from_server_settings(
|
||||
&resolved_app_settings.server_settings,
|
||||
args.bind.as_deref(),
|
||||
|
|
@ -680,30 +679,6 @@ where
|
|||
.await
|
||||
.with_context(|| format!("importing legacy secrets file {}", vault_path.display()))?;
|
||||
let secret_store = fabro_vault::SecretStore::new(database.clone_pool());
|
||||
let optional_report = migrations::migrate_optional_server_env_secrets_to_store(
|
||||
&secret_store,
|
||||
&server_env_path,
|
||||
&env_entries,
|
||||
)
|
||||
.await
|
||||
.context("migrate optional server env secrets into SQLite")?;
|
||||
for warning in &optional_report.warnings {
|
||||
warn!(
|
||||
warning = %warning,
|
||||
removal_deadline = migrations::OPTIONAL_SERVER_ENV_SECRETS_REMOVAL_DEADLINE,
|
||||
"Optional server env secrets migration warning"
|
||||
);
|
||||
}
|
||||
if optional_report.changed() {
|
||||
warn!(
|
||||
migrated_secrets = optional_report.migrated_secrets,
|
||||
removed_env_entries = optional_report.removed_env_entries,
|
||||
preserved_env_entries = optional_report.preserved_env_entries,
|
||||
backup_path = ?optional_report.backup_path,
|
||||
removal_deadline = migrations::OPTIONAL_SERVER_ENV_SECRETS_REMOVAL_DEADLINE,
|
||||
"Migrated optional server env secrets into SQLite"
|
||||
);
|
||||
}
|
||||
let startup_vault = secret_store
|
||||
.snapshot()
|
||||
.await
|
||||
|
|
|
|||
|
|
@ -2622,128 +2622,6 @@ methods = ["dev-token"]
|
|||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_app_state_migrates_legacy_vault_file_on_boot() {
|
||||
let vault_path = test_secret_store_path();
|
||||
let timestamp = "2026-05-18T12:00:00Z";
|
||||
let legacy_api_key = json!({
|
||||
"provider": "anthropic",
|
||||
"type": "api_key",
|
||||
"key": "sk-ant-legacy",
|
||||
});
|
||||
let legacy_oauth = json!({
|
||||
"provider": "openai",
|
||||
"type": "codex_oauth",
|
||||
"tokens": {
|
||||
"access_token": "codex-access",
|
||||
"refresh_token": "codex-refresh",
|
||||
"expires_at": "2026-05-18T13:00:00Z",
|
||||
},
|
||||
"config": {
|
||||
"auth_url": "https://auth.openai.com",
|
||||
"token_url": "https://auth.openai.com/oauth/token",
|
||||
"client_id": "client",
|
||||
"scopes": ["openid", "offline_access"],
|
||||
"redirect_uri": "https://auth.openai.com/deviceauth/callback",
|
||||
"use_pkce": false,
|
||||
},
|
||||
"account_id": "acct_legacy",
|
||||
});
|
||||
let legacy_vault = json!({
|
||||
"anthropic": {
|
||||
"value": legacy_api_key.to_string(),
|
||||
"type": "credential",
|
||||
"created_at": timestamp,
|
||||
"updated_at": timestamp,
|
||||
},
|
||||
"openai_codex": {
|
||||
"value": legacy_oauth.to_string(),
|
||||
"type": "credential",
|
||||
"created_at": timestamp,
|
||||
"updated_at": timestamp,
|
||||
},
|
||||
"GITHUB_TOKEN": {
|
||||
"value": "ghp_legacy",
|
||||
"type": "environment",
|
||||
"created_at": timestamp,
|
||||
"updated_at": timestamp,
|
||||
},
|
||||
"/tmp/github.pem": {
|
||||
"value": "/tmp/github.pem",
|
||||
"type": "file",
|
||||
"created_at": timestamp,
|
||||
"updated_at": timestamp,
|
||||
},
|
||||
});
|
||||
std::fs::write(
|
||||
&vault_path,
|
||||
serde_json::to_vec_pretty(&legacy_vault).unwrap(),
|
||||
)
|
||||
.expect("legacy vault should be writable");
|
||||
|
||||
let state = build_test_app_state_with_vault_path(&vault_path)
|
||||
.expect("legacy vault should not prevent server boot");
|
||||
|
||||
let vault = state.stores.vault.snapshot().await.unwrap();
|
||||
let api_key_entry = vault
|
||||
.get_entry("ANTHROPIC_API_KEY")
|
||||
.expect("legacy provider credential should be migrated to token name");
|
||||
assert_eq!(api_key_entry.secret_type, SecretType::Token);
|
||||
assert_eq!(api_key_entry.value, "sk-ant-legacy");
|
||||
assert!(vault.get_entry("anthropic").is_none());
|
||||
|
||||
let oauth_entry = vault
|
||||
.get_entry("OPENAI_CODEX")
|
||||
.expect("legacy Codex credential should be migrated to canonical OAuth name");
|
||||
assert_eq!(oauth_entry.secret_type, SecretType::Oauth);
|
||||
let oauth: fabro_auth::OAuthCredential =
|
||||
serde_json::from_str(&oauth_entry.value).expect("migrated OAuth JSON should parse");
|
||||
assert_eq!(oauth.tokens.access_token, "codex-access");
|
||||
assert_eq!(oauth.account_id.as_deref(), Some("acct_legacy"));
|
||||
assert!(vault.get_entry("openai_codex").is_none());
|
||||
|
||||
assert_eq!(
|
||||
vault.get_entry("GITHUB_TOKEN").unwrap().secret_type,
|
||||
SecretType::Token
|
||||
);
|
||||
assert_eq!(
|
||||
vault.get_entry("/tmp/github.pem").unwrap().secret_type,
|
||||
SecretType::File
|
||||
);
|
||||
}
|
||||
|
||||
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(),
|
||||
RunLayer::default(),
|
||||
LlmCatalogSettings::default(),
|
||||
),
|
||||
registry_factory_override: None,
|
||||
max_concurrent_runs: 5,
|
||||
store,
|
||||
artifact_store,
|
||||
db_pool,
|
||||
preloaded_vault,
|
||||
server_secrets: load_test_server_secrets(
|
||||
vault_path.with_file_name("server.env"),
|
||||
HashMap::new(),
|
||||
),
|
||||
env_lookup: default_env_lookup(),
|
||||
github_api_base_url: None,
|
||||
active_config_path: tempfile::tempdir().unwrap().path().join("settings.toml"),
|
||||
http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")),
|
||||
sandbox_provider_registry: None,
|
||||
shutdown: tokio_util::sync::CancellationToken::new(),
|
||||
worker_control_bus: None,
|
||||
worker_runtime: None,
|
||||
automation_materializer_override: None,
|
||||
})
|
||||
}
|
||||
|
||||
fn test_worker_ref(pid: u32) -> WorkerRef {
|
||||
WorkerRef::Local { pid }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,10 +4,8 @@ use std::path::Path;
|
|||
use fabro_static::EnvVars;
|
||||
use fabro_types::settings::ServerNamespace;
|
||||
use fabro_vault::Vault;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::jwt_auth::{AuthMode, resolve_auth_mode_with_lookup, validate_auth_configuration};
|
||||
use crate::migrations;
|
||||
use crate::server_secrets::ServerSecrets;
|
||||
|
||||
pub(crate) fn resolve_startup(
|
||||
|
|
@ -25,33 +23,6 @@ pub(crate) fn resolve_startup(
|
|||
Ok((auth_mode, server_secrets))
|
||||
}
|
||||
|
||||
pub fn migrate_startup_vault(vault_path: impl AsRef<Path>) {
|
||||
let vault_path = vault_path.as_ref();
|
||||
match migrations::migrate_legacy_vault_file(vault_path) {
|
||||
Ok(report) if report.changed() => {
|
||||
let backup_path = report
|
||||
.backup_path
|
||||
.as_ref()
|
||||
.map_or_else(|| "<none>".to_string(), |path| path.display().to_string());
|
||||
warn!(
|
||||
migrated_entries = report.migrated_entries,
|
||||
skipped_entries = report.skipped_entries,
|
||||
backup_path = %backup_path,
|
||||
removal_deadline = migrations::LEGACY_VAULT_REMOVAL_DEADLINE,
|
||||
"Migrated legacy vault file"
|
||||
);
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(err) => {
|
||||
warn!(
|
||||
error = %err,
|
||||
removal_deadline = migrations::LEGACY_VAULT_REMOVAL_DEADLINE,
|
||||
"Legacy vault migration failed; continuing with normal vault load"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn validate_startup(
|
||||
env_path: &Path,
|
||||
env_entries: HashMap<String, String>,
|
||||
|
|
@ -68,15 +39,13 @@ pub fn validate_startup_configuration(settings: &ServerNamespace) -> anyhow::Res
|
|||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use fabro_config::{ServerSettingsBuilder, envfile};
|
||||
use fabro_config::ServerSettingsBuilder;
|
||||
use fabro_static::EnvVars;
|
||||
use fabro_types::settings::ServerNamespace;
|
||||
use fabro_vault::{SecretStore, SecretType, Vault};
|
||||
use fabro_vault::{SecretType, Vault};
|
||||
|
||||
use super::validate_startup;
|
||||
use crate::migrations;
|
||||
|
||||
fn resolved_settings(auth_methods: &[&str]) -> ServerNamespace {
|
||||
ServerSettingsBuilder::from_toml(&format!(
|
||||
|
|
@ -106,36 +75,6 @@ client_id = "Iv1.test"
|
|||
Vault::load(dir.path().join("secrets.json")).unwrap()
|
||||
}
|
||||
|
||||
fn env_path(dir: &tempfile::TempDir) -> PathBuf {
|
||||
dir.path().join("server.env")
|
||||
}
|
||||
|
||||
async fn test_secret_store(dir: &tempfile::TempDir) -> SecretStore {
|
||||
let database = fabro_db::Database::connect(dir.path().join("fabro.db"))
|
||||
.await
|
||||
.unwrap();
|
||||
database.migrate().await.unwrap();
|
||||
SecretStore::new(database.clone_pool())
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "test helper scans a temporary directory after startup migration completes"
|
||||
)]
|
||||
fn migration_backups(dir: &Path) -> Vec<PathBuf> {
|
||||
std::fs::read_dir(dir)
|
||||
.unwrap()
|
||||
.map(|entry| entry.unwrap().path())
|
||||
.filter(|path| {
|
||||
path.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.is_some_and(|name| {
|
||||
name.contains("optional-server-env-secrets-to-vault-migration")
|
||||
})
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_startup_accepts_configured_secrets() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
@ -234,185 +173,4 @@ client_id = "Iv1.test"
|
|||
)
|
||||
.expect("github client secret in vault should satisfy startup");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn migrate_optional_secrets_moves_server_env_secrets_to_store() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let server_env_path = env_path(&dir);
|
||||
envfile::write_env_file(
|
||||
&server_env_path,
|
||||
&HashMap::from([
|
||||
(
|
||||
EnvVars::SESSION_SECRET.to_string(),
|
||||
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef".to_string(),
|
||||
),
|
||||
(
|
||||
EnvVars::GITHUB_APP_CLIENT_SECRET.to_string(),
|
||||
"legacy-client-secret".to_string(),
|
||||
),
|
||||
(
|
||||
EnvVars::GITHUB_APP_PRIVATE_KEY.to_string(),
|
||||
"legacy-private-key".to_string(),
|
||||
),
|
||||
(EnvVars::OPENAI_API_KEY.to_string(), "sk-legacy".to_string()),
|
||||
]),
|
||||
)
|
||||
.unwrap();
|
||||
let store = test_secret_store(&dir).await;
|
||||
|
||||
migrations::migrate_optional_server_env_secrets_to_store(
|
||||
&store,
|
||||
&server_env_path,
|
||||
&HashMap::new(),
|
||||
)
|
||||
.await
|
||||
.expect("legacy optional secrets should migrate");
|
||||
|
||||
let client_secret = store
|
||||
.get(EnvVars::GITHUB_APP_CLIENT_SECRET)
|
||||
.await
|
||||
.unwrap()
|
||||
.expect("client secret should be stored");
|
||||
assert_eq!(client_secret.value, "legacy-client-secret");
|
||||
assert_eq!(client_secret.secret_type, SecretType::Token);
|
||||
let private_key = store
|
||||
.get(EnvVars::GITHUB_APP_PRIVATE_KEY)
|
||||
.await
|
||||
.unwrap()
|
||||
.expect("private key should be stored");
|
||||
assert_eq!(private_key.value, "legacy-private-key");
|
||||
assert_eq!(private_key.secret_type, SecretType::File);
|
||||
let openai_key = store
|
||||
.get(EnvVars::OPENAI_API_KEY)
|
||||
.await
|
||||
.unwrap()
|
||||
.expect("openai key should be stored");
|
||||
assert_eq!(openai_key.value, "sk-legacy");
|
||||
|
||||
let server_env = envfile::read_env_file(&server_env_path).unwrap();
|
||||
assert!(server_env.contains_key(EnvVars::SESSION_SECRET));
|
||||
assert!(!server_env.contains_key(EnvVars::GITHUB_APP_CLIENT_SECRET));
|
||||
assert!(!server_env.contains_key(EnvVars::GITHUB_APP_PRIVATE_KEY));
|
||||
assert!(!server_env.contains_key(EnvVars::OPENAI_API_KEY));
|
||||
assert_eq!(migration_backups(dir.path()).len(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn migrate_optional_secrets_prefers_process_env_and_preserves_conflicting_server_env() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let server_env_path = env_path(&dir);
|
||||
envfile::write_env_file(
|
||||
&server_env_path,
|
||||
&HashMap::from([(
|
||||
EnvVars::GITHUB_APP_CLIENT_SECRET.to_string(),
|
||||
"file-client-secret".to_string(),
|
||||
)]),
|
||||
)
|
||||
.unwrap();
|
||||
let env_entries = HashMap::from([(
|
||||
EnvVars::GITHUB_APP_CLIENT_SECRET.to_string(),
|
||||
"process-client-secret".to_string(),
|
||||
)]);
|
||||
let store = test_secret_store(&dir).await;
|
||||
|
||||
migrations::migrate_optional_server_env_secrets_to_store(
|
||||
&store,
|
||||
&server_env_path,
|
||||
&env_entries,
|
||||
)
|
||||
.await
|
||||
.expect("process env secret should migrate");
|
||||
|
||||
let client_secret = store
|
||||
.get(EnvVars::GITHUB_APP_CLIENT_SECRET)
|
||||
.await
|
||||
.unwrap()
|
||||
.expect("client secret should be stored");
|
||||
assert_eq!(client_secret.value, "process-client-secret");
|
||||
let server_env = envfile::read_env_file(&server_env_path).unwrap();
|
||||
assert_eq!(
|
||||
server_env
|
||||
.get(EnvVars::GITHUB_APP_CLIENT_SECRET)
|
||||
.map(String::as_str),
|
||||
Some("file-client-secret")
|
||||
);
|
||||
assert!(migration_backups(dir.path()).is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn migrate_optional_secrets_keeps_existing_stored_secret_and_removes_matching_server_env()
|
||||
{
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let server_env_path = env_path(&dir);
|
||||
envfile::write_env_file(
|
||||
&server_env_path,
|
||||
&HashMap::from([(
|
||||
EnvVars::GITHUB_APP_CLIENT_SECRET.to_string(),
|
||||
"vault-client-secret".to_string(),
|
||||
)]),
|
||||
)
|
||||
.unwrap();
|
||||
let store = test_secret_store(&dir).await;
|
||||
store
|
||||
.set(
|
||||
EnvVars::GITHUB_APP_CLIENT_SECRET,
|
||||
"vault-client-secret",
|
||||
SecretType::Token,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
migrations::migrate_optional_server_env_secrets_to_store(
|
||||
&store,
|
||||
&server_env_path,
|
||||
&HashMap::new(),
|
||||
)
|
||||
.await
|
||||
.expect("redundant server env secret should be cleaned up");
|
||||
|
||||
let client_secret = store
|
||||
.get(EnvVars::GITHUB_APP_CLIENT_SECRET)
|
||||
.await
|
||||
.unwrap()
|
||||
.expect("client secret should be stored");
|
||||
assert_eq!(client_secret.value, "vault-client-secret");
|
||||
let server_env = envfile::read_env_file(&server_env_path).unwrap();
|
||||
assert!(!server_env.contains_key(EnvVars::GITHUB_APP_CLIENT_SECRET));
|
||||
assert_eq!(migration_backups(dir.path()).len(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn migrate_optional_secrets_migrated_github_client_secret_satisfies_startup() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let server_env_path = env_path(&dir);
|
||||
envfile::write_env_file(
|
||||
&server_env_path,
|
||||
&HashMap::from([
|
||||
(
|
||||
EnvVars::SESSION_SECRET.to_string(),
|
||||
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef".to_string(),
|
||||
),
|
||||
(
|
||||
EnvVars::GITHUB_APP_CLIENT_SECRET.to_string(),
|
||||
"legacy-client-secret".to_string(),
|
||||
),
|
||||
]),
|
||||
)
|
||||
.unwrap();
|
||||
let settings = resolved_settings(&["github"]);
|
||||
let store = test_secret_store(&dir).await;
|
||||
|
||||
migrations::migrate_optional_server_env_secrets_to_store(
|
||||
&store,
|
||||
&server_env_path,
|
||||
&HashMap::new(),
|
||||
)
|
||||
.await
|
||||
.expect("legacy github client secret should migrate");
|
||||
|
||||
let vault = store.snapshot().await.unwrap().into_vault();
|
||||
validate_startup(&server_env_path, HashMap::new(), &settings, &vault)
|
||||
.expect("migrated github client secret should satisfy startup");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ use tokio::runtime::Builder as TokioRuntimeBuilder;
|
|||
use tokio_util::sync::CancellationToken;
|
||||
use ulid::Ulid;
|
||||
|
||||
use crate::auth;
|
||||
pub use crate::automation_materializer::TestAutomationRunMaterializer;
|
||||
use crate::interp::process_env_var;
|
||||
use crate::jwt_auth::{AuthMode, ConfiguredAuth};
|
||||
|
|
@ -45,7 +46,6 @@ use crate::server::{
|
|||
use crate::server_secrets::ServerSecrets;
|
||||
#[cfg(test)]
|
||||
use crate::worker_runtime::WorkerRuntime;
|
||||
use crate::{auth, migrations};
|
||||
|
||||
pub const TEST_DEV_TOKEN: &str =
|
||||
"fabro_dev_abababababababababababababababababababababababababababababababab";
|
||||
|
|
@ -600,7 +600,6 @@ fn test_db_pool(
|
|||
default_environment_provider: Option<EnvironmentProvider>,
|
||||
) -> anyhow::Result<DbPool> {
|
||||
std::thread::spawn(move || {
|
||||
migrations::migrate_legacy_vault_file(&vault_path)?;
|
||||
let runtime = TokioRuntimeBuilder::new_current_thread()
|
||||
.enable_all()
|
||||
.build()?;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue