mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-09 03:20:56 +00:00
parent
540241d6d7
commit
3b4eee96ed
5 changed files with 1871 additions and 30 deletions
538
run.json
538
run.json
File diff suppressed because one or more lines are too long
451
stages/006-simplify_opus@1/diff.patch
Normal file
451
stages/006-simplify_opus@1/diff.patch
Normal file
|
|
@ -0,0 +1,451 @@
|
|||
diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs
|
||||
index 6dcb670b0..2f21e6fb9 100644
|
||||
--- a/lib/crates/fabro-cli/src/commands/install.rs
|
||||
+++ b/lib/crates/fabro-cli/src/commands/install.rs
|
||||
@@ -28,10 +28,11 @@ use fabro_config::daemon::ServerDaemon;
|
||||
use fabro_config::user::{SETTINGS_CONFIG_FILENAME, default_storage_dir};
|
||||
use fabro_config::{Storage, UserSettingsBuilder, envfile};
|
||||
use fabro_install::{
|
||||
- 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,
|
||||
+ GITHUB_APP_VAULT_KEYS, GITHUB_INSTALL_SECRET_KEYS, 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};
|
||||
@@ -71,6 +72,7 @@ use crate::{local_server, server_client, user_config};
|
||||
|
||||
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;
|
||||
+#[cfg(test)]
|
||||
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;
|
||||
|
||||
@@ -1618,28 +1620,14 @@ async fn run_install_github_inner(
|
||||
secret_type: VaultSecretType::Token,
|
||||
description: None,
|
||||
});
|
||||
- server_env_remove.extend([
|
||||
- GITHUB_TOKEN_SECRET_KEY,
|
||||
- GITHUB_APP_PRIVATE_KEY_KEY,
|
||||
- GITHUB_APP_CLIENT_SECRET_KEY,
|
||||
- GITHUB_APP_WEBHOOK_SECRET_KEY,
|
||||
- ]);
|
||||
- vault_remove.extend([
|
||||
- GITHUB_APP_PRIVATE_KEY_KEY,
|
||||
- GITHUB_APP_CLIENT_SECRET_KEY,
|
||||
- GITHUB_APP_WEBHOOK_SECRET_KEY,
|
||||
- ]);
|
||||
+ server_env_remove.extend(GITHUB_INSTALL_SECRET_KEYS.iter().copied());
|
||||
+ vault_remove.extend(GITHUB_APP_VAULT_KEYS.iter().copied());
|
||||
}
|
||||
GitHubInstallSelection::App { owner, username } => {
|
||||
let allowed_username = username.clone().context(
|
||||
"GitHub App install requires an authenticated GitHub username; run `gh auth login` and rerun `fabro install github`",
|
||||
)?;
|
||||
- server_env_remove.extend([
|
||||
- GITHUB_TOKEN_SECRET_KEY,
|
||||
- GITHUB_APP_PRIVATE_KEY_KEY,
|
||||
- GITHUB_APP_CLIENT_SECRET_KEY,
|
||||
- GITHUB_APP_WEBHOOK_SECRET_KEY,
|
||||
- ]);
|
||||
+ server_env_remove.extend(GITHUB_INSTALL_SECRET_KEYS.iter().copied());
|
||||
let registration = setup_github_app(
|
||||
&s,
|
||||
&web_url,
|
||||
@@ -1843,17 +1831,8 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
|
||||
type_: ApiSecretType::Token,
|
||||
description: None,
|
||||
});
|
||||
- server_env_remove.extend([
|
||||
- GITHUB_TOKEN_SECRET_KEY,
|
||||
- GITHUB_APP_PRIVATE_KEY_KEY,
|
||||
- GITHUB_APP_CLIENT_SECRET_KEY,
|
||||
- GITHUB_APP_WEBHOOK_SECRET_KEY,
|
||||
- ]);
|
||||
- vault_remove.extend([
|
||||
- GITHUB_APP_PRIVATE_KEY_KEY,
|
||||
- GITHUB_APP_CLIENT_SECRET_KEY,
|
||||
- GITHUB_APP_WEBHOOK_SECRET_KEY,
|
||||
- ]);
|
||||
+ server_env_remove.extend(GITHUB_INSTALL_SECRET_KEYS.iter().copied());
|
||||
+ vault_remove.extend(GITHUB_APP_VAULT_KEYS.iter().copied());
|
||||
Some(PendingGitHubSettings::Token)
|
||||
}
|
||||
GitHubInstallSelection::App { owner, username } => {
|
||||
@@ -1890,12 +1869,7 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
|
||||
.into_iter()
|
||||
.map(|(key, value)| github_app_secret_request(key, value)),
|
||||
);
|
||||
- server_env_remove.extend([
|
||||
- GITHUB_TOKEN_SECRET_KEY,
|
||||
- GITHUB_APP_PRIVATE_KEY_KEY,
|
||||
- GITHUB_APP_CLIENT_SECRET_KEY,
|
||||
- GITHUB_APP_WEBHOOK_SECRET_KEY,
|
||||
- ]);
|
||||
+ server_env_remove.extend(GITHUB_INSTALL_SECRET_KEYS.iter().copied());
|
||||
vault_remove.push(GITHUB_TOKEN_SECRET_KEY);
|
||||
if !webhook_configured {
|
||||
vault_remove.push(GITHUB_APP_WEBHOOK_SECRET_KEY);
|
||||
diff --git a/lib/crates/fabro-cli/tests/it/cmd/worker_auth.rs b/lib/crates/fabro-cli/tests/it/cmd/worker_auth.rs
|
||||
index 9f9d43921..eb9ff0290 100644
|
||||
--- a/lib/crates/fabro-cli/tests/it/cmd/worker_auth.rs
|
||||
+++ b/lib/crates/fabro-cli/tests/it/cmd/worker_auth.rs
|
||||
@@ -21,6 +21,7 @@ use fabro_client::{AuthEntry, AuthStore, OAuthEntry, ServerTarget, StoredSubject
|
||||
use fabro_config::{Storage, envfile};
|
||||
use fabro_store::EventEnvelope;
|
||||
use fabro_test::{apply_test_isolation, expect_reqwest_json, isolated_storage_dir, test_context};
|
||||
+use fabro_vault::{SecretType, Vault};
|
||||
|
||||
use super::support::{find_run_dir, output_stderr, output_stdout};
|
||||
use crate::support::{
|
||||
@@ -74,12 +75,19 @@ client_id = "github-client-id"
|
||||
.unwrap();
|
||||
envfile::merge_env_file(
|
||||
&Storage::new(&storage_dir).runtime_directory().env_path(),
|
||||
- [
|
||||
- ("SESSION_SECRET", TEST_SESSION_SECRET),
|
||||
- ("GITHUB_APP_CLIENT_SECRET", TEST_GITHUB_CLIENT_SECRET),
|
||||
- ],
|
||||
+ [("SESSION_SECRET", TEST_SESSION_SECRET)],
|
||||
)
|
||||
.unwrap();
|
||||
+ let mut vault =
|
||||
+ Vault::load(Storage::new(&storage_dir).secrets_path()).expect("test vault should load");
|
||||
+ vault
|
||||
+ .set(
|
||||
+ "GITHUB_APP_CLIENT_SECRET",
|
||||
+ TEST_GITHUB_CLIENT_SECRET,
|
||||
+ SecretType::Token,
|
||||
+ None,
|
||||
+ )
|
||||
+ .expect("GitHub client secret should store in test vault");
|
||||
|
||||
let mut cmd = Command::new(env!("CARGO_BIN_EXE_fabro"));
|
||||
apply_test_isolation(&mut cmd, home_root.path());
|
||||
diff --git a/lib/crates/fabro-cli/tests/it/support/auth_harness.rs b/lib/crates/fabro-cli/tests/it/support/auth_harness.rs
|
||||
index e8ec21ac6..e018ff988 100644
|
||||
--- a/lib/crates/fabro-cli/tests/it/support/auth_harness.rs
|
||||
+++ b/lib/crates/fabro-cli/tests/it/support/auth_harness.rs
|
||||
@@ -24,7 +24,7 @@ use fabro_server::auth::GithubEndpoints;
|
||||
use fabro_server::ip_allowlist::IpAllowlistConfig;
|
||||
use fabro_server::jwt_auth::resolve_auth_mode_with_lookup;
|
||||
use fabro_server::server::{RouterOptions, build_router_with_options};
|
||||
-use fabro_server::test_support::test_app_state_with_runtime_settings_and_env_lookup_and_server_secret_env;
|
||||
+use fabro_server::test_support::TestAppStateBuilder;
|
||||
use fabro_test::{GitHubAppState, TestContext, apply_test_isolation};
|
||||
use serde_json::Value;
|
||||
use tokio::net::TcpListener;
|
||||
@@ -77,26 +77,20 @@ impl RealAuthHarness {
|
||||
_ => None,
|
||||
})
|
||||
.expect("auth mode should resolve");
|
||||
- let mut secrets = std::collections::HashMap::from([
|
||||
- (
|
||||
- "SESSION_SECRET".to_string(),
|
||||
- TEST_SESSION_SECRET.to_string(),
|
||||
- ),
|
||||
- (
|
||||
- "GITHUB_APP_CLIENT_SECRET".to_string(),
|
||||
- github_client_secret.clone(),
|
||||
- ),
|
||||
- ]);
|
||||
+ let mut secrets = std::collections::HashMap::from([(
|
||||
+ "SESSION_SECRET".to_string(),
|
||||
+ TEST_SESSION_SECRET.to_string(),
|
||||
+ )]);
|
||||
if let Some(token) = dev_token.clone() {
|
||||
secrets.insert("FABRO_DEV_TOKEN".to_string(), token);
|
||||
}
|
||||
- let state = test_app_state_with_runtime_settings_and_env_lookup_and_server_secret_env(
|
||||
- settings,
|
||||
- RunLayer::default(),
|
||||
- 5,
|
||||
- |_| None,
|
||||
- &secrets,
|
||||
- );
|
||||
+ let state = TestAppStateBuilder::new()
|
||||
+ .runtime_settings(settings, RunLayer::default())
|
||||
+ .max_concurrent_runs(5)
|
||||
+ .env_lookup(|_| None)
|
||||
+ .server_secret_env(secrets)
|
||||
+ .vault_entries([("GITHUB_APP_CLIENT_SECRET", github_client_secret.as_str())])
|
||||
+ .build();
|
||||
let github_base = github_base_url(&twin.base_url);
|
||||
let router = build_router_with_options(
|
||||
state,
|
||||
diff --git a/lib/crates/fabro-install/src/lib.rs b/lib/crates/fabro-install/src/lib.rs
|
||||
index 64e191fe0..933c161cc 100644
|
||||
--- a/lib/crates/fabro-install/src/lib.rs
|
||||
+++ b/lib/crates/fabro-install/src/lib.rs
|
||||
@@ -34,6 +34,24 @@ pub const OBJECT_STORE_MANAGED_COMMENT: &str = "managed by fabro-install: object
|
||||
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;
|
||||
|
||||
+/// Every GitHub-install secret name. Used to drop stale entries from
|
||||
+/// `server.env` whenever an install runs so a switch between Token and App
|
||||
+/// strategies leaves no residue behind.
|
||||
+pub const GITHUB_INSTALL_SECRET_KEYS: &[&str] = &[
|
||||
+ EnvVars::GITHUB_TOKEN,
|
||||
+ EnvVars::GITHUB_APP_PRIVATE_KEY,
|
||||
+ EnvVars::GITHUB_APP_CLIENT_SECRET,
|
||||
+ EnvVars::GITHUB_APP_WEBHOOK_SECRET,
|
||||
+];
|
||||
+
|
||||
+/// GitHub App vault secret names cleared when switching back to the Token
|
||||
+/// strategy.
|
||||
+pub const GITHUB_APP_VAULT_KEYS: &[&str] = &[
|
||||
+ EnvVars::GITHUB_APP_PRIVATE_KEY,
|
||||
+ EnvVars::GITHUB_APP_CLIENT_SECRET,
|
||||
+ EnvVars::GITHUB_APP_WEBHOOK_SECRET,
|
||||
+];
|
||||
+
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct VaultSecretWrite {
|
||||
pub name: String,
|
||||
diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs
|
||||
index ed26a8823..914012c8c 100644
|
||||
--- a/lib/crates/fabro-server/src/install.rs
|
||||
+++ b/lib/crates/fabro-server/src/install.rs
|
||||
@@ -17,11 +17,11 @@ use fabro_config::Storage;
|
||||
use fabro_config::bind::{Bind, BindRequest};
|
||||
use fabro_config::envfile::{EnvFileRemoval, EnvFileUpdate};
|
||||
use fabro_install::{
|
||||
- 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,
|
||||
+ 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,
|
||||
+ 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};
|
||||
@@ -1580,17 +1580,12 @@ async fn post_install_finish(
|
||||
secret_type: VaultSecretType::Token,
|
||||
description: None,
|
||||
});
|
||||
- vault_removals.extend([
|
||||
- EnvVars::GITHUB_APP_PRIVATE_KEY.to_string(),
|
||||
- EnvVars::GITHUB_APP_CLIENT_SECRET.to_string(),
|
||||
- EnvVars::GITHUB_APP_WEBHOOK_SECRET.to_string(),
|
||||
- ]);
|
||||
- server_env_removals.extend([
|
||||
- make_env_removal(EnvVars::GITHUB_TOKEN),
|
||||
- make_env_removal(EnvVars::GITHUB_APP_PRIVATE_KEY),
|
||||
- make_env_removal(EnvVars::GITHUB_APP_CLIENT_SECRET),
|
||||
- make_env_removal(EnvVars::GITHUB_APP_WEBHOOK_SECRET),
|
||||
- ]);
|
||||
+ vault_removals.extend(GITHUB_APP_VAULT_KEYS.iter().map(|k| (*k).to_string()));
|
||||
+ server_env_removals.extend(
|
||||
+ GITHUB_INSTALL_SECRET_KEYS
|
||||
+ .iter()
|
||||
+ .map(|k| make_env_removal(k)),
|
||||
+ );
|
||||
let dev_token_path = Storage::new(state.storage_dir.as_ref())
|
||||
.runtime_directory()
|
||||
.dev_token_path();
|
||||
@@ -1639,12 +1634,11 @@ async fn post_install_finish(
|
||||
vault_removals.push(EnvVars::GITHUB_APP_WEBHOOK_SECRET.to_string());
|
||||
}
|
||||
vault_removals.push(EnvVars::GITHUB_TOKEN.to_string());
|
||||
- server_env_removals.extend([
|
||||
- make_env_removal(EnvVars::GITHUB_TOKEN),
|
||||
- make_env_removal(EnvVars::GITHUB_APP_PRIVATE_KEY),
|
||||
- make_env_removal(EnvVars::GITHUB_APP_CLIENT_SECRET),
|
||||
- make_env_removal(EnvVars::GITHUB_APP_WEBHOOK_SECRET),
|
||||
- ]);
|
||||
+ server_env_removals.extend(
|
||||
+ GITHUB_INSTALL_SECRET_KEYS
|
||||
+ .iter()
|
||||
+ .map(|k| make_env_removal(k)),
|
||||
+ );
|
||||
}
|
||||
}
|
||||
|
||||
diff --git a/lib/crates/fabro-server/src/test_support.rs b/lib/crates/fabro-server/src/test_support.rs
|
||||
index 9187c4dc1..b61f01df8 100644
|
||||
--- a/lib/crates/fabro-server/src/test_support.rs
|
||||
+++ b/lib/crates/fabro-server/src/test_support.rs
|
||||
@@ -21,6 +21,7 @@ use fabro_store::{ArtifactStore, Database};
|
||||
use fabro_types::settings::ServerAuthMethod;
|
||||
use fabro_types::{AuthMethod, IdpIdentity, ServerSettings};
|
||||
use fabro_util::error::SharedError;
|
||||
+use fabro_vault::{SecretType, Vault};
|
||||
use fabro_workflow::handler::HandlerRegistry;
|
||||
use object_store::memory::InMemory as MemoryObjectStore;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
@@ -62,6 +63,7 @@ pub struct TestAppStateBuilder {
|
||||
registry_factory_override: Option<Box<RegistryFactoryOverride>>,
|
||||
store_bundle: Option<(Arc<Database>, ArtifactStore)>,
|
||||
vault_path: Option<PathBuf>,
|
||||
+ vault_entries: Vec<(String, String)>,
|
||||
server_env_path: Option<PathBuf>,
|
||||
active_config_path: Option<PathBuf>,
|
||||
server_secret_env: HashMap<String, String>,
|
||||
@@ -78,6 +80,7 @@ impl Default for TestAppStateBuilder {
|
||||
registry_factory_override: None,
|
||||
store_bundle: None,
|
||||
vault_path: None,
|
||||
+ vault_entries: Vec::new(),
|
||||
server_env_path: None,
|
||||
active_config_path: None,
|
||||
server_secret_env: HashMap::new(),
|
||||
@@ -170,9 +173,30 @@ impl TestAppStateBuilder {
|
||||
self
|
||||
}
|
||||
|
||||
+ /// Pre-populate the vault file with optional integration secrets (token
|
||||
+ /// type) before [`build_app_state`] opens it.
|
||||
+ pub fn vault_entries<I, K, V>(mut self, entries: I) -> Self
|
||||
+ where
|
||||
+ I: IntoIterator<Item = (K, V)>,
|
||||
+ K: Into<String>,
|
||||
+ V: Into<String>,
|
||||
+ {
|
||||
+ self.vault_entries
|
||||
+ .extend(entries.into_iter().map(|(k, v)| (k.into(), v.into())));
|
||||
+ self
|
||||
+ }
|
||||
+
|
||||
pub fn build(self) -> Arc<AppState> {
|
||||
let (store, artifact_store) = self.store_bundle.unwrap_or_else(test_store_bundle);
|
||||
let vault_path = self.vault_path.unwrap_or_else(test_secret_store_path);
|
||||
+ if !self.vault_entries.is_empty() {
|
||||
+ let mut vault = Vault::load(vault_path.clone()).expect("test vault should load");
|
||||
+ for (name, value) in &self.vault_entries {
|
||||
+ vault
|
||||
+ .set(name, value, SecretType::Token, None)
|
||||
+ .expect("test vault entry should persist");
|
||||
+ }
|
||||
+ }
|
||||
let server_env_path = self
|
||||
.server_env_path
|
||||
.unwrap_or_else(|| vault_path.with_file_name("server.env"));
|
||||
@@ -357,21 +381,6 @@ pub fn test_app_state_with_env_lookup(
|
||||
.build()
|
||||
}
|
||||
|
||||
-pub fn test_app_state_with_env_lookup_and_server_secret_env(
|
||||
- server_settings: ServerSettings,
|
||||
- manifest_run_defaults: RunLayer,
|
||||
- max_concurrent_runs: usize,
|
||||
- env_lookup: impl Fn(&str) -> Option<String> + Send + Sync + 'static,
|
||||
- server_secret_env: &HashMap<String, String>,
|
||||
-) -> Arc<AppState> {
|
||||
- TestAppStateBuilder::new()
|
||||
- .runtime_settings(server_settings, manifest_run_defaults)
|
||||
- .max_concurrent_runs(max_concurrent_runs)
|
||||
- .env_lookup(env_lookup)
|
||||
- .server_secret_env(server_secret_env.clone())
|
||||
- .build()
|
||||
-}
|
||||
-
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "test helper writes a fixture server.env with sync std::fs::write"
|
||||
diff --git a/lib/crates/fabro-server/tests/it/helpers.rs b/lib/crates/fabro-server/tests/it/helpers.rs
|
||||
index 6c81eb180..569f7e7d0 100644
|
||||
--- a/lib/crates/fabro-server/tests/it/helpers.rs
|
||||
+++ b/lib/crates/fabro-server/tests/it/helpers.rs
|
||||
@@ -129,10 +129,7 @@ pub(crate) fn test_app_with_mock_anthropic(mock_base_url: &str) -> axum::Router
|
||||
"anthropic",
|
||||
mock_base_url,
|
||||
))
|
||||
- .env_lookup(|name| match name {
|
||||
- "ANTHROPIC_API_KEY" => Some("test-key".to_string()),
|
||||
- _ => None,
|
||||
- })
|
||||
+ .vault_entries([("ANTHROPIC_API_KEY", "test-key")])
|
||||
.build();
|
||||
build_test_router(state)
|
||||
}
|
||||
diff --git a/lib/crates/fabro-server/tests/it/openapi_conformance.rs b/lib/crates/fabro-server/tests/it/openapi_conformance.rs
|
||||
index a3cf16982..fdfa8d894 100644
|
||||
--- a/lib/crates/fabro-server/tests/it/openapi_conformance.rs
|
||||
+++ b/lib/crates/fabro-server/tests/it/openapi_conformance.rs
|
||||
@@ -11,7 +11,7 @@
|
||||
use axum::body::Body;
|
||||
use axum::http::{Method, Request, StatusCode};
|
||||
use fabro_server::install::{InstallAppState, build_install_router};
|
||||
-use fabro_server::test_support::test_app_state_with_runtime_settings_and_env_lookup_and_server_secret_env;
|
||||
+use fabro_server::test_support::TestAppStateBuilder;
|
||||
use serde_yaml::Value;
|
||||
use tower::ServiceExt;
|
||||
|
||||
@@ -143,16 +143,15 @@ fn github_webhook_spec_and_sdk_describe_a_json_body() {
|
||||
|
||||
#[tokio::test]
|
||||
async fn github_webhook_spec_route_is_routable_when_webhook_secret_is_present() {
|
||||
- let secret = "test-webhook-secret".to_string();
|
||||
+ let secret = "test-webhook-secret";
|
||||
let settings = test_settings();
|
||||
let app = fabro_server::test_support::build_test_router(
|
||||
- test_app_state_with_runtime_settings_and_env_lookup_and_server_secret_env(
|
||||
- settings.server_settings,
|
||||
- settings.manifest_run_defaults,
|
||||
- 5,
|
||||
- |_| None,
|
||||
- &std::collections::HashMap::from([("GITHUB_APP_WEBHOOK_SECRET".to_string(), secret)]),
|
||||
- ),
|
||||
+ TestAppStateBuilder::new()
|
||||
+ .runtime_settings(settings.server_settings, settings.manifest_run_defaults)
|
||||
+ .max_concurrent_runs(5)
|
||||
+ .env_lookup(|_| None)
|
||||
+ .vault_entries([("GITHUB_APP_WEBHOOK_SECRET", secret)])
|
||||
+ .build(),
|
||||
);
|
||||
|
||||
let response = app
|
||||
diff --git a/lib/crates/fabro-static/src/lib.rs b/lib/crates/fabro-static/src/lib.rs
|
||||
index f5d94f18e..2abbd588e 100644
|
||||
--- a/lib/crates/fabro-static/src/lib.rs
|
||||
+++ b/lib/crates/fabro-static/src/lib.rs
|
||||
@@ -7,6 +7,4 @@ mod env_vars;
|
||||
mod secret_registry;
|
||||
|
||||
pub use env_vars::EnvVars;
|
||||
-pub use secret_registry::{
|
||||
- SecretScope, is_bootstrap_secret, is_optional_vault_secret, secret_scope,
|
||||
-};
|
||||
+pub use secret_registry::is_bootstrap_secret;
|
||||
diff --git a/lib/crates/fabro-static/src/secret_registry.rs b/lib/crates/fabro-static/src/secret_registry.rs
|
||||
index 86fe1873e..534bac3aa 100644
|
||||
--- a/lib/crates/fabro-static/src/secret_registry.rs
|
||||
+++ b/lib/crates/fabro-static/src/secret_registry.rs
|
||||
@@ -1,7 +1,7 @@
|
||||
use crate::EnvVars;
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
-pub enum SecretScope {
|
||||
+enum SecretScope {
|
||||
Bootstrap,
|
||||
OptionalVault,
|
||||
}
|
||||
@@ -32,7 +32,7 @@ const OPTIONAL_VAULT_SECRETS: &[&str] = &[
|
||||
EnvVars::DAYTONA_API_KEY,
|
||||
];
|
||||
|
||||
-pub fn secret_scope(name: &str) -> Option<SecretScope> {
|
||||
+fn secret_scope(name: &str) -> Option<SecretScope> {
|
||||
if BOOTSTRAP_SECRETS.contains(&name) {
|
||||
Some(SecretScope::Bootstrap)
|
||||
} else if OPTIONAL_VAULT_SECRETS.contains(&name) {
|
||||
@@ -46,7 +46,8 @@ pub fn is_bootstrap_secret(name: &str) -> bool {
|
||||
secret_scope(name) == Some(SecretScope::Bootstrap)
|
||||
}
|
||||
|
||||
-pub fn is_optional_vault_secret(name: &str) -> bool {
|
||||
+#[cfg(test)]
|
||||
+fn is_optional_vault_secret(name: &str) -> bool {
|
||||
secret_scope(name) == Some(SecretScope::OptionalVault)
|
||||
}
|
||||
|
||||
6
stages/006-simplify_opus@1/status.json
Normal file
6
stages/006-simplify_opus@1/status.json
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"outcome": "succeeded",
|
||||
"notes": "Stage completed: simplify_opus",
|
||||
"failure_reason": null,
|
||||
"timestamp": "2026-05-25T18:42:52.644647Z"
|
||||
}
|
||||
901
stages/007-simplify_gpt@1/prompt.md
Normal file
901
stages/007-simplify_gpt@1/prompt.md
Normal file
|
|
@ -0,0 +1,901 @@
|
|||
Goal: # Secrets Rationalization Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Make Fabro server secret resolution simple and predictable: bootstrap secrets come only from process env or `server.env`; optional integration secrets come only from the vault.
|
||||
|
||||
**Architecture:** Introduce a small shared registry that classifies well-known secret names as bootstrap or optional. Keep `ServerSecrets` focused on bootstrap startup/runtime requirements, add vault-only helpers for optional integrations, and wire install mode so it can write optional secrets before enabling features such as GitHub auth.
|
||||
|
||||
**Tech Stack:** Rust workspace crates (`fabro-static`, `fabro-auth`, `fabro-agent`, `fabro-server`, `fabro-cli`), existing vault storage, existing install persistence, existing docs under `docs/internal` and `docs/public`.
|
||||
|
||||
---
|
||||
|
||||
## Decisions Locked In
|
||||
|
||||
- `server.env` is only for bootstrap/runtime secrets that the server may need before the vault can be used.
|
||||
- Process env is also bootstrap-only for server runtime. Optional server integrations do not read process env.
|
||||
- Vault is the only server/runtime source for optional integration secrets.
|
||||
- Optional secrets may be workflow-visible for now.
|
||||
- No compatibility shims or fallback aliases are required. Remove server-runtime `GH_TOKEN` fallback and optional `server.env` fallbacks.
|
||||
- Non-secret configuration, such as base URLs, org IDs, or catalog settings, remains configuration. This plan only governs secrets.
|
||||
- CLI/library code may still use env-backed credential sources where explicitly selected outside server runtime. The server runtime must not use that path for optional integrations.
|
||||
|
||||
## Secret Model
|
||||
|
||||
Bootstrap secrets:
|
||||
|
||||
- `SESSION_SECRET`
|
||||
- `FABRO_DEV_TOKEN`
|
||||
- object-store/storage credentials resolved by current server object-store builders, including manual AWS credentials such as `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, and `AWS_SESSION_TOKEN`
|
||||
|
||||
Optional vault secrets:
|
||||
|
||||
- LLM provider API keys and OAuth credential records
|
||||
- `FABRO_SLACK_BOT_TOKEN`
|
||||
- `FABRO_SLACK_APP_TOKEN`
|
||||
- `DAYTONA_API_KEY`
|
||||
- `BRAVE_SEARCH_API_KEY`
|
||||
- `GITHUB_TOKEN`
|
||||
- `GITHUB_APP_PRIVATE_KEY`
|
||||
- `GITHUB_APP_CLIENT_SECRET`
|
||||
- `GITHUB_APP_WEBHOOK_SECRET`
|
||||
|
||||
## Change Map
|
||||
|
||||
- Create `lib/crates/fabro-static/src/secret_registry.rs` for shared classification.
|
||||
- Modify `lib/crates/fabro-static/src/lib.rs` to export the registry.
|
||||
- Modify `lib/crates/fabro-server/src/server_secrets.rs` to make the bootstrap boundary explicit in tests and naming.
|
||||
- Modify `lib/crates/fabro-server/src/server.rs`, `diagnostics.rs`, `run_files.rs`, `run_manifest.rs`, `web_auth.rs`, `github_webhooks.rs`, and server handlers under `server/handler/` to use vault-only optional lookup.
|
||||
- Modify `lib/crates/fabro-server/src/startup.rs`, `serve.rs`, and `jwt_auth.rs` so GitHub auth validates `GITHUB_APP_CLIENT_SECRET` from vault while session/dev-token auth still validates bootstrap secrets.
|
||||
- Modify `lib/crates/fabro-auth/src/vault_source.rs` to expose an explicit vault-only credential source constructor for server runtime.
|
||||
- Modify `lib/crates/fabro-agent/src/config.rs` and `tools.rs` so `web_search` receives `BRAVE_SEARCH_API_KEY` explicitly instead of reading process env inside the tool.
|
||||
- Modify install flows in `lib/crates/fabro-server/src/install.rs` and `lib/crates/fabro-cli/src/commands/install.rs` so GitHub App secrets are written to vault, not `server.env`.
|
||||
- Modify `lib/crates/fabro-server/src/server/handler/secrets.rs` so bootstrap secrets cannot be written through the vault API.
|
||||
- Update public and internal docs after behavior is implemented.
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Add The Shared Secret Registry
|
||||
|
||||
**Files:**
|
||||
|
||||
- Create: `lib/crates/fabro-static/src/secret_registry.rs`
|
||||
- Modify: `lib/crates/fabro-static/src/lib.rs`
|
||||
- Test: unit tests in `secret_registry.rs`
|
||||
|
||||
- [ ] **Step 1: Define the registry types and classified names**
|
||||
|
||||
Create `secret_registry.rs` with this shape:
|
||||
|
||||
```rust
|
||||
use crate::EnvVars;
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum SecretScope {
|
||||
Bootstrap,
|
||||
OptionalVault,
|
||||
}
|
||||
|
||||
const BOOTSTRAP_SECRETS: &[&str] = &[
|
||||
EnvVars::SESSION_SECRET,
|
||||
EnvVars::FABRO_DEV_TOKEN,
|
||||
EnvVars::AWS_ACCESS_KEY_ID,
|
||||
EnvVars::AWS_SECRET_ACCESS_KEY,
|
||||
EnvVars::AWS_SESSION_TOKEN,
|
||||
];
|
||||
|
||||
const OPTIONAL_VAULT_SECRETS: &[&str] = &[
|
||||
EnvVars::ANTHROPIC_API_KEY,
|
||||
EnvVars::BRAVE_SEARCH_API_KEY,
|
||||
EnvVars::FABRO_SLACK_APP_TOKEN,
|
||||
EnvVars::FABRO_SLACK_BOT_TOKEN,
|
||||
EnvVars::GEMINI_API_KEY,
|
||||
EnvVars::GITHUB_APP_CLIENT_SECRET,
|
||||
EnvVars::GITHUB_APP_PRIVATE_KEY,
|
||||
EnvVars::GITHUB_APP_WEBHOOK_SECRET,
|
||||
EnvVars::GITHUB_TOKEN,
|
||||
EnvVars::INCEPTION_API_KEY,
|
||||
EnvVars::KIMI_API_KEY,
|
||||
EnvVars::MINIMAX_API_KEY,
|
||||
EnvVars::OPENAI_API_KEY,
|
||||
EnvVars::ZAI_API_KEY,
|
||||
EnvVars::DAYTONA_API_KEY,
|
||||
];
|
||||
|
||||
pub fn secret_scope(name: &str) -> Option<SecretScope> {
|
||||
if BOOTSTRAP_SECRETS.contains(&name) {
|
||||
Some(SecretScope::Bootstrap)
|
||||
} else if OPTIONAL_VAULT_SECRETS.contains(&name) {
|
||||
Some(SecretScope::OptionalVault)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
pub fn is_bootstrap_secret(name: &str) -> bool {
|
||||
secret_scope(name) == Some(SecretScope::Bootstrap)
|
||||
}
|
||||
|
||||
pub fn is_optional_vault_secret(name: &str) -> bool {
|
||||
secret_scope(name) == Some(SecretScope::OptionalVault)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Export the registry**
|
||||
|
||||
Modify `lib.rs`:
|
||||
|
||||
```rust
|
||||
mod env_vars;
|
||||
mod secret_registry;
|
||||
|
||||
pub use env_vars::EnvVars;
|
||||
pub use secret_registry::{
|
||||
SecretScope, is_bootstrap_secret, is_optional_vault_secret, secret_scope,
|
||||
};
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Add registry tests**
|
||||
|
||||
Add tests proving:
|
||||
|
||||
- `SESSION_SECRET` and `FABRO_DEV_TOKEN` are bootstrap.
|
||||
- `GITHUB_APP_CLIENT_SECRET`, `GITHUB_APP_PRIVATE_KEY`, `GITHUB_TOKEN`, Slack tokens, Daytona, Brave, and common LLM API keys are optional vault secrets.
|
||||
- `GH_TOKEN` is not classified.
|
||||
- non-secret config names such as `GITHUB_BASE_URL`, `SLACK_BASE_URL`, and `DAYTONA_API_URL` are not classified.
|
||||
|
||||
- [ ] **Step 4: Run the focused tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-static
|
||||
```
|
||||
|
||||
Expected: all `fabro-static` tests pass.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add lib/crates/fabro-static/src/lib.rs lib/crates/fabro-static/src/secret_registry.rs
|
||||
git commit -m "refactor: classify server secret scopes"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Enforce Bootstrap Boundaries In The Secret API
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lib/crates/fabro-server/src/server/handler/secrets.rs`
|
||||
- Test: existing server handler tests or new focused tests near secret handler coverage
|
||||
|
||||
- [ ] **Step 1: Reject bootstrap secrets in `create_secret`**
|
||||
|
||||
Before OAuth parsing or Daytona validation, reject `SecretScope::Bootstrap`:
|
||||
|
||||
```rust
|
||||
if fabro_static::is_bootstrap_secret(&name) {
|
||||
return ApiError::bad_request(format!(
|
||||
"{name} is a bootstrap secret; configure it with process env or server.env"
|
||||
))
|
||||
.into_response();
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Keep optional and unknown secret writes allowed**
|
||||
|
||||
Do not reject unknown names. Workflows may define arbitrary vault-visible secrets.
|
||||
|
||||
- [ ] **Step 3: Add API tests**
|
||||
|
||||
Add tests proving:
|
||||
|
||||
- `POST /secrets` with `SESSION_SECRET` returns `400`.
|
||||
- `POST /secrets` with `FABRO_DEV_TOKEN` returns `400`.
|
||||
- `POST /secrets` with `GITHUB_APP_CLIENT_SECRET` succeeds.
|
||||
- `POST /secrets` with a custom name such as `CUSTOM_WORKFLOW_TOKEN` succeeds.
|
||||
|
||||
- [ ] **Step 4: Run focused tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-server secret
|
||||
```
|
||||
|
||||
Expected: secret handler tests pass.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add lib/crates/fabro-server/src/server/handler/secrets.rs lib/crates/fabro-server
|
||||
git commit -m "fix: reject bootstrap secrets in vault API"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Replace Server Optional Lookups With Vault-Only Lookups
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lib/crates/fabro-server/src/server.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/diagnostics.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/run_files.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/run_manifest.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/server/handler/sandbox.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/server/handler/sessions.rs`
|
||||
- Test: `lib/crates/fabro-server/src/server/tests.rs` and existing handler tests
|
||||
|
||||
- [ ] **Step 1: Add explicit helpers on `AppState`**
|
||||
|
||||
Replace `vault_or_env` and `vault_or_env_pub` with:
|
||||
|
||||
```rust
|
||||
pub(crate) fn vault_secret(&self, name: &str) -> Option<String> {
|
||||
self.vault
|
||||
.try_read()
|
||||
.ok()
|
||||
.and_then(|vault| vault.get(name).map(str::to_string))
|
||||
}
|
||||
|
||||
fn config_env_lookup(&self, name: &str) -> Option<String> {
|
||||
(self.env_lookup)(name)
|
||||
}
|
||||
```
|
||||
|
||||
Keep `server_secret` for bootstrap secrets only.
|
||||
|
||||
- [ ] **Step 2: Change Daytona secret reads**
|
||||
|
||||
Use `state.vault_secret(EnvVars::DAYTONA_API_KEY)` wherever a Daytona API key is required. Keep non-secret Daytona URL and organization settings on `env_lookup` or settings-derived configuration, not vault lookup.
|
||||
|
||||
- [ ] **Step 3: Change GitHub token reads**
|
||||
|
||||
In `github_credentials`, resolve token strategy only through:
|
||||
|
||||
```rust
|
||||
let token = self
|
||||
.vault_secret(EnvVars::GITHUB_TOKEN)
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|token| !token.is_empty())
|
||||
.map(str::to_string);
|
||||
```
|
||||
|
||||
Remove the `GH_TOKEN` fallback and update the error message to say:
|
||||
|
||||
```text
|
||||
GITHUB_TOKEN not configured -- run fabro install or run fabro secret set GITHUB_TOKEN
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Change diagnostics and handlers**
|
||||
|
||||
Replace `state.vault_or_env(...)` and `state.vault_or_env_pub(...)` with `state.vault_secret(...)` for optional secret checks. Update diagnostics remediation text to use `fabro secret set`.
|
||||
|
||||
- [ ] **Step 5: Add regression tests**
|
||||
|
||||
Add tests proving:
|
||||
|
||||
- `DAYTONA_API_KEY` in process env but absent from vault is treated as missing by server diagnostics.
|
||||
- `GITHUB_TOKEN` in process env but absent from vault is treated as missing by token strategy.
|
||||
- `GH_TOKEN` in vault or process env is ignored by server runtime.
|
||||
- `DAYTONA_API_KEY` in vault is accepted.
|
||||
|
||||
- [ ] **Step 6: Run focused tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-server daytona
|
||||
cargo nextest run -p fabro-server github
|
||||
```
|
||||
|
||||
Expected: relevant server tests pass.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add lib/crates/fabro-server
|
||||
git commit -m "refactor: resolve optional server secrets from vault only"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Make Server LLM Credentials Vault-Only
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lib/crates/fabro-auth/src/vault_source.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/server.rs`
|
||||
- Test: `lib/crates/fabro-auth/src/vault_source.rs`, `lib/crates/fabro-server/src/server/tests.rs`
|
||||
|
||||
- [ ] **Step 1: Add an explicit vault-only constructor**
|
||||
|
||||
Add this constructor to `VaultCredentialSource`:
|
||||
|
||||
```rust
|
||||
#[must_use]
|
||||
pub fn vault_only(vault: Arc<AsyncRwLock<Vault>>) -> Self {
|
||||
Self::with_env_lookup(vault, |_| None)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Use vault-only source in server app state**
|
||||
|
||||
In `build_app_state`, change the LLM source construction from `with_env_lookup(... env_lookup ...)` to:
|
||||
|
||||
```rust
|
||||
let llm_source: Arc<dyn CredentialSource> =
|
||||
Arc::new(VaultCredentialSource::vault_only(Arc::clone(&vault)));
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Keep CLI/library env sources explicit**
|
||||
|
||||
Do not remove `EnvCredentialSource`. Do not change direct CLI/library flows that intentionally choose env-backed credentials outside server runtime.
|
||||
|
||||
- [ ] **Step 4: Add tests**
|
||||
|
||||
Add tests proving:
|
||||
|
||||
- `VaultCredentialSource::vault_only` does not resolve process env values.
|
||||
- Server readiness sees a provider configured only when the matching vault secret exists.
|
||||
|
||||
- [ ] **Step 5: Run focused tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-auth
|
||||
cargo nextest run -p fabro-server llm
|
||||
```
|
||||
|
||||
Expected: auth and server LLM tests pass.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add lib/crates/fabro-auth/src/vault_source.rs lib/crates/fabro-server/src/server.rs lib/crates/fabro-server/src/server/tests.rs
|
||||
git commit -m "fix: use vault-only llm credentials in server runtime"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Move GitHub App Runtime Secrets To Vault
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lib/crates/fabro-server/src/server.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/web_auth.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/github_webhooks.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/diagnostics.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/startup.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/serve.rs`
|
||||
- Modify: `lib/crates/fabro-server/src/jwt_auth.rs`
|
||||
- Test: server startup, auth, webhook, diagnostics, worker command tests
|
||||
|
||||
- [ ] **Step 1: Read GitHub App private key from vault**
|
||||
|
||||
In `AppState::github_credentials`, replace:
|
||||
|
||||
```rust
|
||||
let raw = self.server_secret(EnvVars::GITHUB_APP_PRIVATE_KEY);
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```rust
|
||||
let raw = self.vault_secret(EnvVars::GITHUB_APP_PRIVATE_KEY);
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Read OAuth client secret from vault**
|
||||
|
||||
In the GitHub OAuth callback handler, replace `state.server_secret(EnvVars::GITHUB_APP_CLIENT_SECRET)` with `state.vault_secret(EnvVars::GITHUB_APP_CLIENT_SECRET)`.
|
||||
|
||||
- [ ] **Step 3: Read webhook secret from vault**
|
||||
|
||||
Replace webhook secret resolution from `server_secret(WEBHOOK_SECRET_ENV)` to `vault_secret(WEBHOOK_SECRET_ENV)`. Startup logging should report webhook presence based on the vault value after the vault is loaded.
|
||||
|
||||
- [ ] **Step 4: Validate GitHub auth after vault load**
|
||||
|
||||
Change startup validation so `SESSION_SECRET` and `FABRO_DEV_TOKEN` are read from `ServerSecrets`, while `GITHUB_APP_CLIENT_SECRET` is read from the vault.
|
||||
|
||||
Use a composite lookup for auth validation:
|
||||
|
||||
```rust
|
||||
let auth_secret_lookup = |name: &str| match name {
|
||||
EnvVars::GITHUB_APP_CLIENT_SECRET => vault.get(name).map(str::to_string),
|
||||
_ => server_secrets.get(name),
|
||||
};
|
||||
```
|
||||
|
||||
Load the vault before calling GitHub-auth validation. Pass the loaded vault into `build_app_state` or factor vault loading so the server does not perform inconsistent duplicate loads.
|
||||
|
||||
- [ ] **Step 5: Update worker GitHub App key injection**
|
||||
|
||||
Where the worker command currently forwards `GITHUB_APP_PRIVATE_KEY` from `server_secret`, forward it from `vault_secret`. Keep the explicit worker injection because worker environments remain scrubbed by default.
|
||||
|
||||
- [ ] **Step 6: Update tests**
|
||||
|
||||
Add or change tests proving:
|
||||
|
||||
- GitHub auth enabled with `GITHUB_APP_CLIENT_SECRET` only in `server.env` fails startup.
|
||||
- GitHub auth enabled with `GITHUB_APP_CLIENT_SECRET` in vault passes startup.
|
||||
- OAuth callback reads the vault client secret.
|
||||
- Webhook verification reads the vault webhook secret.
|
||||
- Worker command forwards the GitHub App private key from vault.
|
||||
- Diagnostics reports missing GitHub App secrets based on vault, not `server.env`.
|
||||
|
||||
- [ ] **Step 7: Run focused tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-server github
|
||||
cargo nextest run -p fabro-server worker_auth
|
||||
cargo nextest run -p fabro-cli server_start
|
||||
```
|
||||
|
||||
Expected: GitHub auth, webhook, worker, and startup tests pass.
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add lib/crates/fabro-server lib/crates/fabro-cli/tests
|
||||
git commit -m "fix: store github app runtime secrets in vault"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: Move Slack To Vault-Only Credentials
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lib/crates/fabro-server/src/server.rs`
|
||||
- Modify: `lib/crates/fabro-slack/src/config.rs` only if names or status messages need adjustment
|
||||
- Test: Slack credential resolution and server app-state tests
|
||||
|
||||
- [ ] **Step 1: Resolve Slack tokens from vault**
|
||||
|
||||
In `build_app_state`, replace:
|
||||
|
||||
```rust
|
||||
resolve_slack_credentials_status_with_lookup(|name| server_secrets.get(name))
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```rust
|
||||
resolve_slack_credentials_status_with_lookup(|name| {
|
||||
vault.try_read().ok().and_then(|vault| vault.get(name).map(str::to_string))
|
||||
})
|
||||
```
|
||||
|
||||
If the implementation already holds a synchronous vault read guard in this block, reuse that guard rather than calling `try_read` repeatedly.
|
||||
|
||||
- [ ] **Step 2: Keep Slack base URL out of the secret model**
|
||||
|
||||
Leave `SLACK_BASE_URL` as a non-secret test/development override. Do not add it to the secret registry.
|
||||
|
||||
- [ ] **Step 3: Update Slack tests**
|
||||
|
||||
Add tests proving:
|
||||
|
||||
- Slack tokens in vault enable Slack service.
|
||||
- Slack tokens in `server.env` but absent from vault do not enable Slack service.
|
||||
- Missing vault tokens produce the existing disabled status with missing token names.
|
||||
|
||||
- [ ] **Step 4: Run focused tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-slack
|
||||
cargo nextest run -p fabro-server slack
|
||||
```
|
||||
|
||||
Expected: Slack tests pass.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add lib/crates/fabro-server/src/server.rs lib/crates/fabro-slack/src/config.rs
|
||||
git commit -m "fix: resolve slack credentials from vault"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 7: Make Brave Search Tool Use Server-Provided Secrets
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lib/crates/fabro-agent/src/config.rs`
|
||||
- Modify: `lib/crates/fabro-agent/src/tools.rs`
|
||||
- Modify: server session/profile construction where `SessionOptions` are built
|
||||
- Test: `lib/crates/fabro-agent/src/tools.rs`, server diagnostics/tool integration tests
|
||||
|
||||
- [ ] **Step 1: Add tool secret configuration**
|
||||
|
||||
Add to `config.rs`:
|
||||
|
||||
```rust
|
||||
#[derive(Clone, Debug, Default, PartialEq, Eq)]
|
||||
pub struct ToolSecrets {
|
||||
pub brave_search_api_key: Option<String>,
|
||||
}
|
||||
```
|
||||
|
||||
Add this field to `SessionOptions`:
|
||||
|
||||
```rust
|
||||
pub tool_secrets: ToolSecrets,
|
||||
```
|
||||
|
||||
Default it to `ToolSecrets::default()` and include it in the debug impl without printing secret values. Use a boolean:
|
||||
|
||||
```rust
|
||||
.field(
|
||||
"brave_search_configured",
|
||||
&self.tool_secrets.brave_search_api_key.is_some(),
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Pass the Brave key into tool registration**
|
||||
|
||||
Change `register_core_tools`:
|
||||
|
||||
```rust
|
||||
registry.register(make_web_search_tool_with_api_key(
|
||||
config.tool_secrets.brave_search_api_key.clone(),
|
||||
));
|
||||
```
|
||||
|
||||
Remove the process-env-reading `make_web_search_tool` wrapper.
|
||||
|
||||
- [ ] **Step 3: Improve the missing-key message**
|
||||
|
||||
Change the error text from “environment variable is not set” to:
|
||||
|
||||
```text
|
||||
BRAVE_SEARCH_API_KEY is not configured
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Populate `SessionOptions.tool_secrets` in server runtime**
|
||||
|
||||
Where the server constructs agent sessions, set:
|
||||
|
||||
```rust
|
||||
tool_secrets: ToolSecrets {
|
||||
brave_search_api_key: state.vault_secret(EnvVars::BRAVE_SEARCH_API_KEY),
|
||||
},
|
||||
```
|
||||
|
||||
Keep direct CLI agent behavior explicit: if the CLI should support local env-backed Brave Search, set this field at the CLI boundary from `std::env::var(EnvVars::BRAVE_SEARCH_API_KEY).ok()`. Do not let the tool read process env internally.
|
||||
|
||||
- [ ] **Step 5: Update tests**
|
||||
|
||||
Add tests proving:
|
||||
|
||||
- `make_web_search_tool_with_api_key(None)` returns `BRAVE_SEARCH_API_KEY is not configured`.
|
||||
- `register_core_tools` wires `SessionOptions.tool_secrets.brave_search_api_key` into the web search tool.
|
||||
- Server diagnostics and server-created sessions both use the same vault-backed key.
|
||||
|
||||
- [ ] **Step 6: Run focused tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-agent web_search
|
||||
cargo nextest run -p fabro-server brave
|
||||
```
|
||||
|
||||
Expected: Brave tests pass.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add lib/crates/fabro-agent lib/crates/fabro-server
|
||||
git commit -m "fix: pass brave search credentials through server configuration"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 8: Update Install Mode Persistence
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lib/crates/fabro-server/src/install.rs`
|
||||
- Modify: `lib/crates/fabro-cli/src/commands/install.rs`
|
||||
- Modify: install tests in `lib/crates/fabro-server/tests/it/api/install.rs` and `lib/crates/fabro-cli/tests/it/cmd/install.rs`
|
||||
|
||||
- [ ] **Step 1: Server install writes GitHub App secrets to vault**
|
||||
|
||||
In `post_install_finish`, replace GitHub App `server_env_writes.push(...)` calls with `vault_secrets.push(...)` calls:
|
||||
|
||||
```rust
|
||||
vault_secrets.push(VaultSecretWrite {
|
||||
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 {
|
||||
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 {
|
||||
name: EnvVars::GITHUB_APP_WEBHOOK_SECRET.to_string(),
|
||||
value: secret,
|
||||
secret_type: VaultSecretType::Token,
|
||||
description: None,
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Remove stale GitHub App keys from `server.env` during install persistence**
|
||||
|
||||
Add removals for:
|
||||
|
||||
- `GITHUB_APP_PRIVATE_KEY`
|
||||
- `GITHUB_APP_CLIENT_SECRET`
|
||||
- `GITHUB_APP_WEBHOOK_SECRET`
|
||||
|
||||
This keeps greenfield installs clean and removes stale optional secrets if a developer has run earlier local install attempts.
|
||||
|
||||
- [ ] **Step 3: Align CLI GitHub install helper**
|
||||
|
||||
Update CLI install persistence so switching to GitHub App writes app secrets to vault and removes the app keys from `server.env`. Switching to token continues to write `GITHUB_TOKEN` to vault and remove app vault secrets.
|
||||
|
||||
- [ ] **Step 4: Update install tests**
|
||||
|
||||
Add or change tests proving:
|
||||
|
||||
- Browser install with GitHub App writes private key, client secret, and webhook secret to vault.
|
||||
- Browser install with GitHub App does not write those keys to `server.env`.
|
||||
- CLI GitHub App install writes app secrets to vault.
|
||||
- Switching strategies removes stale secrets from the other strategy.
|
||||
- `SESSION_SECRET`, `FABRO_DEV_TOKEN`, and object-store credentials remain in `server.env`.
|
||||
|
||||
- [ ] **Step 5: Run focused tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-server install
|
||||
cargo nextest run -p fabro-cli install
|
||||
```
|
||||
|
||||
Expected: install tests pass.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add lib/crates/fabro-server/src/install.rs lib/crates/fabro-server/tests/it/api/install.rs lib/crates/fabro-cli/src/commands/install.rs lib/crates/fabro-cli/tests/it/cmd/install.rs
|
||||
git commit -m "fix: persist optional install secrets in vault"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 9: Update Strategy And Public Docs
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `docs/internal/server-secrets-strategy.md`
|
||||
- Modify: `docs/public/administration/server-configuration.mdx`
|
||||
- Modify: `docs/public/administration/self-host-docker.mdx`
|
||||
- Modify: `docs/public/administration/deploy-railway.mdx`
|
||||
- Modify: `docs/public/integrations/github.mdx`
|
||||
- Modify: `docs/public/integrations/slack.mdx`
|
||||
- Modify: `docs/public/integrations/brave-search.mdx`
|
||||
- Modify: `docs/public/integrations/daytona.mdx`
|
||||
- Modify: LLM provider docs that still say server runtime reads provider keys from process env
|
||||
|
||||
- [ ] **Step 1: Rewrite internal strategy**
|
||||
|
||||
Update `server-secrets-strategy.md` to state:
|
||||
|
||||
- `ServerSecrets` means bootstrap secrets only.
|
||||
- Bootstrap source precedence is process env then `server.env`.
|
||||
- Optional integration secrets are vault-only.
|
||||
- Requiredness is independent from source: GitHub auth may require a vault secret at startup.
|
||||
- Server worker env remains scrubbed and receives only explicit injected values.
|
||||
|
||||
- [ ] **Step 2: Update administration docs**
|
||||
|
||||
State that `server.env` is for:
|
||||
|
||||
- `SESSION_SECRET`
|
||||
- `FABRO_DEV_TOKEN`
|
||||
- storage/object-store bootstrap credentials
|
||||
|
||||
State that `server.env` is not used for:
|
||||
|
||||
- Slack
|
||||
- Daytona
|
||||
- Brave Search
|
||||
- LLM provider keys
|
||||
- GitHub token
|
||||
- GitHub App private key/client secret/webhook secret
|
||||
|
||||
- [ ] **Step 3: Update integration docs**
|
||||
|
||||
For each optional integration, show `fabro secret set` as the server-runtime configuration path:
|
||||
|
||||
```bash
|
||||
fabro secret set BRAVE_SEARCH_API_KEY <key>
|
||||
fabro secret set DAYTONA_API_KEY <key>
|
||||
fabro secret set FABRO_SLACK_BOT_TOKEN <xoxb-token>
|
||||
fabro secret set FABRO_SLACK_APP_TOKEN <xapp-token>
|
||||
fabro secret set GITHUB_TOKEN <token>
|
||||
```
|
||||
|
||||
For GitHub App, document that install mode stores app secrets in vault.
|
||||
|
||||
- [ ] **Step 4: Remove stale `server.env` guidance**
|
||||
|
||||
Search:
|
||||
|
||||
```bash
|
||||
rg -n "server.env|process env -> server.env|GITHUB_APP_CLIENT_SECRET|FABRO_SLACK" docs/public docs/internal
|
||||
```
|
||||
|
||||
Each result should either describe bootstrap secrets or explicitly say the secret is vault-only.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/internal docs/public
|
||||
git commit -m "docs: document vault-only optional secrets"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 10: Final Verification And Cleanup
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify any touched tests or docs needed by final verification.
|
||||
|
||||
- [ ] **Step 1: Search for removed server-runtime patterns**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
rg -n "vault_or_env|vault_or_env_pub|GH_TOKEN|GITHUB_APP_CLIENT_SECRET.*server_secret|FABRO_SLACK_.*server_secret|BRAVE_SEARCH_API_KEY.*std::env|DAYTONA_API_KEY.*process_env" lib/crates docs
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- no server-runtime `vault_or_env` helper remains;
|
||||
- no server-runtime `GH_TOKEN` fallback remains;
|
||||
- no optional secret docs claim `server.env` is a valid server-runtime source;
|
||||
- live-test annotations and explicit CLI/library env usage may remain.
|
||||
|
||||
- [ ] **Step 2: Run focused crate tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-static
|
||||
cargo nextest run -p fabro-auth
|
||||
cargo nextest run -p fabro-agent web_search
|
||||
cargo nextest run -p fabro-slack
|
||||
cargo nextest run -p fabro-server
|
||||
cargo nextest run -p fabro-cli install
|
||||
```
|
||||
|
||||
Expected: all listed tests pass.
|
||||
|
||||
- [ ] **Step 3: Run workspace quality checks**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo +nightly-2026-04-14 fmt --check --all
|
||||
cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings
|
||||
```
|
||||
|
||||
Expected: formatting and clippy pass.
|
||||
|
||||
- [ ] **Step 4: Run one end-to-end install smoke if credentials are available**
|
||||
|
||||
Run a non-live smoke that exercises install persistence and restart validation through existing install tests. If local live credentials are available, run the relevant ignored/live tests for GitHub App, Daytona, and Brave Search.
|
||||
|
||||
- [ ] **Step 5: Final commit**
|
||||
|
||||
If Task 10 required cleanup changes, commit them:
|
||||
|
||||
```bash
|
||||
git add .
|
||||
git commit -m "test: verify rationalized server secrets"
|
||||
```
|
||||
|
||||
If Task 10 produced no file changes, do not create an empty commit.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- `server.env` is no longer a provider for Slack, Daytona, Brave Search, LLM provider credentials, GitHub token, or GitHub App secrets.
|
||||
- Process env is no longer a server-runtime provider for optional integration secrets.
|
||||
- Install mode can enable GitHub auth in one pass by writing GitHub App secrets to vault before normal startup.
|
||||
- Startup fails clearly when GitHub auth is enabled and `GITHUB_APP_CLIENT_SECRET` is missing from vault.
|
||||
- `fabro secret set` rejects bootstrap secrets and accepts optional integration secrets.
|
||||
- Brave Search diagnostics and the actual `web_search` tool agree on whether the key is configured.
|
||||
- Worker subprocess env remains scrubbed, with only explicit internal injections preserved.
|
||||
|
||||
|
||||
## Completed stages
|
||||
- **toolchain**: succeeded
|
||||
- Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`
|
||||
- Output:
|
||||
```
|
||||
cargo 1.95.0 (f2d3ce0bd 2026-03-21)
|
||||
```
|
||||
- **preflight_compile**: succeeded
|
||||
- Script: `cargo check -q --workspace 2>&1`
|
||||
- Output: (empty)
|
||||
- **preflight_lint**: succeeded
|
||||
- Script: `cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1`
|
||||
- Output: (empty)
|
||||
- **implement**: succeeded
|
||||
- Model: gpt-5.5, 5.5m tokens in / 25.6k out
|
||||
- Files: /home/daytona/workspace/fabro/lib/crates/fabro-static/src/secret_registry.rs
|
||||
- **simplify_opus**: succeeded
|
||||
- Model: claude-opus-4-7, 154.4k tokens in / 47.8k out
|
||||
- Files: /home/daytona/workspace/fabro/lib/crates/fabro-cli/src/commands/install.rs, /home/daytona/workspace/fabro/lib/crates/fabro-cli/tests/it/cmd/worker_auth.rs, /home/daytona/workspace/fabro/lib/crates/fabro-cli/tests/it/support/auth_harness.rs, /home/daytona/workspace/fabro/lib/crates/fabro-install/src/lib.rs, /home/daytona/workspace/fabro/lib/crates/fabro-server/src/install.rs, /home/daytona/workspace/fabro/lib/crates/fabro-server/src/test_support.rs, /home/daytona/workspace/fabro/lib/crates/fabro-server/tests/it/helpers.rs, /home/daytona/workspace/fabro/lib/crates/fabro-server/tests/it/openapi_conformance.rs, /home/daytona/workspace/fabro/lib/crates/fabro-static/src/lib.rs, /home/daytona/workspace/fabro/lib/crates/fabro-static/src/secret_registry.rs
|
||||
|
||||
|
||||
# Simplify: Code Review and Cleanup
|
||||
|
||||
Review changes vs. origin for reuse, quality, and efficiency. Fix any issues found.
|
||||
|
||||
## Phase 1: Identify Changes
|
||||
|
||||
Run git diff (or git diff HEAD if there are staged changes) to see what changed. If there are no git changes, review the most recently modified files that the user mentioned or that you edited earlier in this conversation.
|
||||
|
||||
## Phase 2: Launch Three Review Agents in Parallel
|
||||
|
||||
Use the Agent tool to launch all three agents concurrently in a single message. Pass each agent the full diff so it has the complete context.
|
||||
|
||||
### Agent 1: Code Reuse Review
|
||||
|
||||
For each change:
|
||||
|
||||
1. Search for existing utilities and helpers that could replace newly written code. Use Grep to find similar patterns elsewhere in the codebase — common locations are utility directories, shared modules, and files adjacent to the changed ones.
|
||||
2. Flag any new function that duplicates existing functionality. Suggest the existing function to use instead.
|
||||
3. Flag any inline logic that could use an existing utility — hand-rolled string manipulation, manual path handling, custom environment checks, ad-hoc type guards, and similar patterns are common candidates.
|
||||
|
||||
Note: This is a greenfield app, so focus on maximizing simplicity and don't worry about changing things to achieve it.
|
||||
|
||||
### Agent 2: Code Quality Review
|
||||
|
||||
Review the same changes for hacky patterns:
|
||||
|
||||
1. Redundant state: state that duplicates existing state, cached values that could be derived, observers/effects that could be direct calls
|
||||
2. Parameter sprawl: adding new parameters to a function instead of generalizing or restructuring existing ones
|
||||
3. Copy-paste with slight variation: near-duplicate code blocks that should be unified with a shared abstraction
|
||||
4. Leaky abstractions: exposing internal details that should be encapsulated, or breaking existing abstraction boundaries
|
||||
5. Stringly-typed code: using raw strings where constants, enums (string unions), or branded types already exist in the codebase
|
||||
|
||||
Note: This is a greenfield app, so be aggressive in optimizing quality.
|
||||
|
||||
### Agent 3: Efficiency Review
|
||||
|
||||
Review the same changes for efficiency:
|
||||
|
||||
1. Unnecessary work: redundant computations, repeated file reads, duplicate network/API calls, N+1 patterns
|
||||
2. Missed concurrency: independent operations run sequentially when they could run in parallel
|
||||
3. Hot-path bloat: new blocking work added to startup or per-request/per-render hot paths
|
||||
4. Unnecessary existence checks: pre-checking file/resource existence before operating (TOCTOU anti-pattern) — operate directly and handle the error
|
||||
5. Memory: unbounded data structures, missing cleanup, event listener leaks
|
||||
6. Overly broad operations: reading entire files when only a portion is needed, loading all items when filtering for one
|
||||
|
||||
## Phase 3: Fix Issues
|
||||
|
||||
Wait for all three agents to complete. Aggregate their findings and fix each issue directly. If a finding is a false positive or not worth addressing, note it and move on — do not argue with the finding, just skip it.
|
||||
|
||||
When done, briefly summarize what was fixed (or confirm the code was already clean).
|
||||
5
stages/007-simplify_gpt@1/provider_used.json
Normal file
5
stages/007-simplify_gpt@1/provider_used.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"mode": "agent",
|
||||
"provider": "openai",
|
||||
"model": "gpt-5.5"
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue