scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-06 02:48:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Fix snapshot execution cleanup and GitHub PEM loading

Browse source
This commit is contained in:
Bryan Helmkamp 2026-03-29 23:00:43 -04:00
parent be14f7d904
commit c6ff6c95a2
No known key found for this signature in database
9 changed files with 229 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
lib/crates/fabro-cli/src/commands/pr/mod.rs
View file

@ -18,7 +18,7 @@ use crate::user_config::load_user_settings_with_globals;
pub(crate) async fn dispatch(ns: PrNamespace, globals: &GlobalArgs) -> Result<()> {
let cli_settings = load_user_settings_with_globals(globals)?;
let github_app = build_github_app_credentials(cli_settings.app_id());
let github_app = build_github_app_credentials(cli_settings.app_id())?;
match ns.command {
PrCommand::Create(args) => create::create_command(args, github_app, globals).await,

2
lib/crates/fabro-cli/src/commands/preflight.rs
View file

@ -26,7 +26,7 @@ pub(crate) async fn execute(mut args: PreflightArgs, globals: &GlobalArgs) -> an
let cli_settings: FabroSettings = load_user_settings_with_globals(globals)?;
args.verbose = args.verbose || cli_settings.verbose_enabled();
let github_app = build_github_app_credentials(cli_settings.app_id());
let github_app = build_github_app_credentials(cli_settings.app_id())?;
let cli_args_config = ConfigLayer::try_from(&args)?;
let cwd = std::env::current_dir()?;
let settings = cli_args_config

25
lib/crates/fabro-cli/src/commands/repo/init.rs
View file

@ -173,15 +173,22 @@ async fn check_github_app_installation() {
let slug = cli_settings.slug().map(String::from);
// Build GitHub App credentials
let Some(creds) = build_github_app_credentials(Some(&app_id)) else {
eprintln!(
"\n Set {} to enable GitHub App integration",
console::Style::new()
.cyan()
.bold()
.apply_to("GITHUB_APP_PRIVATE_KEY")
);
return;
let creds = match build_github_app_credentials(Some(&app_id)) {
Ok(Some(creds)) => creds,
Ok(None) => {
eprintln!(
"\n Set {} to enable GitHub App integration",
console::Style::new()
.cyan()
.bold()
.apply_to("GITHUB_APP_PRIVATE_KEY")
);
return;
}
Err(err) => {
eprintln!("\n Warning: invalid GITHUB_APP_PRIVATE_KEY: {err}");
return;
}
};
let jwt = match fabro_github::sign_app_jwt(&creds.app_id, &creds.private_key_pem) {

2
lib/crates/fabro-cli/src/commands/run/detached.rs
View file

@ -31,7 +31,7 @@ pub(crate) async fn execute(run_dir: PathBuf, launcher_path: PathBuf, resume: bo
let store = store::build_store(&run_record.settings.storage_dir())?;
let run_store = open_or_hydrate_run(store.as_ref(), &run_dir).await?;
let github_app = shared::github::build_github_app_credentials(run_record.settings.app_id());
let github_app = shared::github::build_github_app_credentials(run_record.settings.app_id())?;
let runtime_state = RuntimeState::new(&run_dir);
let services = StartServices {

7
lib/crates/fabro-cli/src/shared/github.rs
View file

@ -1,5 +1,8 @@
use anyhow::anyhow;
use fabro_github::GitHubAppCredentials;
pub(crate) fn build_github_app_credentials(app_id: Option<&str>) -> Option<GitHubAppCredentials> {
GitHubAppCredentials::from_env(app_id)
pub(crate) fn build_github_app_credentials(
app_id: Option<&str>,
) -> anyhow::Result<Option<GitHubAppCredentials>> {
GitHubAppCredentials::from_env(app_id).map_err(|err| anyhow!(err))
}

69
lib/crates/fabro-cli/tests/it/cli.rs
View file

@ -1587,6 +1587,75 @@ digraph Test {
);
}
#[test]
fn bug5_detached_uses_snapshotted_app_id_for_github_credentials() {
let storage_dir = tempfile::tempdir().unwrap();
let home = init_cli_home(storage_dir.path());
let run_dir = storage_dir.path().join("runs").join("20260101-test-bug5");
std::fs::create_dir_all(&run_dir).unwrap();
let dot = "\
digraph G {
start [shape=Mdiamond, label=\"Start\"]
exit [shape=Msquare, label=\"Exit\"]
start -> exit
}";
let run_record = serde_json::json!({
"run_id": "test-bug5",
"created_at": "2026-01-01T00:00:00Z",
"settings": {
"dry_run": true,
"auto_approve": true,
"no_retro": true,
"llm": {
"model": "test-model"
},
"sandbox": {
"provider": "local"
},
"git": {
"app_id": "snapshotted-app-id"
}
},
"graph": {
"name": "G",
"nodes": {},
"edges": [],
"attrs": {}
},
"working_directory": run_dir.to_str().unwrap(),
});
std::fs::write(
run_dir.join("run.json"),
serde_json::to_string(&run_record).unwrap(),
)
.unwrap();
std::fs::write(run_dir.join("graph.fabro"), dot).unwrap();
arc()
.env("HOME", home.path())
.env("GITHUB_APP_PRIVATE_KEY", "%%%not-base64%%%")
.args([
"__detached",
"--run-dir",
run_dir.to_str().unwrap(),
"--launcher-path",
storage_dir
.path()
.join("launchers")
.join("test-bug5.json")
.to_str()
.unwrap(),
])
.timeout(std::time::Duration::from_secs(10))
.assert()
.failure()
.stderr(predicate::str::contains(
"GITHUB_APP_PRIVATE_KEY is not valid PEM or base64",
));
}
// Bug 3: attach loop must leave interview_request.json in place until the
// engine consumes interview_response.json, so reattach remains safe.
#[test]

58
lib/crates/fabro-github/src/lib.rs
View file

@ -54,23 +54,37 @@ pub struct GitHubAppCredentials {
}
impl GitHubAppCredentials {
pub fn from_env(app_id: Option<&str>) -> Option<Self> {
let app_id = app_id?;
let raw = std::env::var("GITHUB_APP_PRIVATE_KEY").ok()?;
let private_key_pem = if raw.starts_with("-----") {
raw
} else {
let pem_bytes =
base64::Engine::decode(&base64::engine::general_purpose::STANDARD, &raw).ok()?;
String::from_utf8(pem_bytes).ok()?
pub fn private_key_from_env() -> Result<Option<String>, String> {
let Ok(raw) = std::env::var("GITHUB_APP_PRIVATE_KEY") else {
return Ok(None);
};
Some(Self {
decode_pem_env("GITHUB_APP_PRIVATE_KEY", &raw).map(Some)
}
pub fn from_env(app_id: Option<&str>) -> Result<Option<Self>, String> {
let Some(app_id) = app_id else {
return Ok(None);
};
let Some(private_key_pem) = Self::private_key_from_env()? else {
return Ok(None);
};
Ok(Some(Self {
app_id: app_id.to_string(),
private_key_pem,
})
}))
}
}
fn decode_pem_env(name: &str, raw: &str) -> Result<String, String> {
if raw.starts_with("-----") {
return Ok(raw.to_string());
}
let pem_bytes = base64::Engine::decode(&base64::engine::general_purpose::STANDARD, raw)
.map_err(|err| format!("{name} is not valid PEM or base64: {err}"))?;
String::from_utf8(pem_bytes)
.map_err(|err| format!("{name} base64 decoded to invalid UTF-8: {err}"))
}
/// HTTP method used in GitHub API calls.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum HttpMethod {
@ -960,6 +974,28 @@ pub async fn create_installation_access_token_for_projects(
mod tests {
use super::*;
#[test]
fn decode_pem_env_accepts_raw_pem() {
let pem = "-----BEGIN TEST KEY-----\nabc\n-----END TEST KEY-----";
assert_eq!(decode_pem_env("GITHUB_APP_PRIVATE_KEY", pem).unwrap(), pem);
}
#[test]
fn decode_pem_env_accepts_base64_pem() {
let pem = "-----BEGIN TEST KEY-----\nabc\n-----END TEST KEY-----";
let encoded = base64::Engine::encode(&base64::engine::general_purpose::STANDARD, pem);
assert_eq!(
decode_pem_env("GITHUB_APP_PRIVATE_KEY", &encoded).unwrap(),
pem
);
}
#[test]
fn decode_pem_env_rejects_invalid_base64() {
let err = decode_pem_env("GITHUB_APP_PRIVATE_KEY", "%%%not-base64%%%").unwrap_err();
assert!(err.contains("GITHUB_APP_PRIVATE_KEY is not valid PEM or base64"));
}
// -----------------------------------------------------------------------
// parse_github_owner_repo
// -----------------------------------------------------------------------

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

@ -14,7 +14,7 @@ use clap::Args;
use fabro_config::FabroSettings;
use crate::github_webhooks::WebhookManager;
use crate::jwt_auth::{AuthMode, AuthStrategy, decode_pem_env, resolve_auth_mode};
use crate::jwt_auth::{AuthMode, AuthStrategy, resolve_auth_mode};
use crate::server::{build_router, create_app_state_with_store, spawn_scheduler};
use crate::tls::{ClientAuth, build_rustls_config, serve_tls};
use fabro_llm::client::Client as LlmClient;
@ -182,9 +182,24 @@ pub async fn serve_command(
let webhook_manager = match webhook_app_id {
Some(app_id) => {
let secret = std::env::var("GITHUB_APP_WEBHOOK_SECRET").ok();
let private_key_pem = read_github_private_key();
if let (Some(secret), Some(pem)) = (secret, private_key_pem) {
match WebhookManager::start(secret.into_bytes(), &app_id, &pem).await {
let github_app = match fabro_github::GitHubAppCredentials::from_env(Some(&app_id)) {
Ok(github_app) => github_app,
Err(err) => {
warn!(
error = %err,
"Webhook config present but GITHUB_APP_PRIVATE_KEY is invalid; skipping webhook listener"
);
None
}
};
if let (Some(secret), Some(github_app)) = (secret, github_app) {
match WebhookManager::start(
secret.into_bytes(),
&github_app.app_id,
&github_app.private_key_pem,
)
.await
{
Ok(manager) => Some(manager),
Err(err) => {
error!(error = %err, "Failed to start webhook listener");
@ -268,12 +283,6 @@ pub async fn serve_command(
Ok(())
}
/// Read the GitHub App private key from the environment, decoding base64 if needed.
fn read_github_private_key() -> Option<String> {
let raw = std::env::var("GITHUB_APP_PRIVATE_KEY").ok()?;
Some(decode_pem_env("GITHUB_APP_PRIVATE_KEY", &raw))
}
/// Derive client certificate verification mode from the resolved auth strategies.
fn client_auth_from_mode(auth_mode: &AuthMode) -> ClientAuth {
let strategies = match auth_mode {

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

@ -490,6 +490,13 @@ fn compute_queue_positions(runs: &HashMap<String, ManagedRun>) -> HashMap<String
.collect()
}
fn clear_live_run_state(run: &mut ManagedRun) {
run.interviewer = None;
run.event_tx = None;
run.cancel_tx = None;
run.cancel_token = None;
}
async fn start_run(
_auth: AuthenticatedService,
State(state): State<Arc<AppState>>,
@ -625,6 +632,7 @@ async fn execute_run(state: Arc<AppState>, run_id: String) {
if let Some(managed_run) = runs.get_mut(&run_id) {
if managed_run.status != RunStatus::Starting {
// Was cancelled during setup
clear_live_run_state(managed_run);
state.scheduler_notify.notify_one();
return;
}
@ -642,7 +650,7 @@ async fn execute_run(state: Arc<AppState>, run_id: String) {
if let Some(managed_run) = runs.get_mut(&run_id) {
managed_run.status = RunStatus::Failed;
managed_run.error = Some(format!("Failed to open or hydrate run store: {e}"));
managed_run.event_tx = None;
clear_live_run_state(managed_run);
}
state.scheduler_notify.notify_one();
return;
@ -656,14 +664,28 @@ async fn execute_run(state: Arc<AppState>, run_id: String) {
if let Some(managed_run) = runs.get_mut(&run_id) {
managed_run.status = RunStatus::Failed;
managed_run.error = Some(format!("Failed to load persisted run: {e}"));
managed_run.event_tx = None;
clear_live_run_state(managed_run);
}
state.scheduler_notify.notify_one();
return;
}
};
let github_app = match fabro_github::GitHubAppCredentials::from_env(
persisted.run_record().settings.app_id(),
) {
Ok(github_app) => github_app,
Err(e) => {
tracing::error!(run_id = %run_id, error = %e, "Invalid GitHub App credentials");
let mut runs = state.runs.lock().expect("runs lock poisoned");
if let Some(managed_run) = runs.get_mut(&run_id) {
managed_run.status = RunStatus::Failed;
managed_run.error = Some(format!("Invalid GitHub App credentials: {e}"));
clear_live_run_state(managed_run);
}
state.scheduler_notify.notify_one();
return;
}
};
let github_app =
fabro_github::GitHubAppCredentials::from_env(persisted.run_record().settings.app_id());
let services = operations::StartServices {
cancel_token: Some(Arc::clone(&cancel_token)),
emitter: Arc::clone(&emitter),
@ -757,9 +779,7 @@ async fn execute_run(state: Arc<AppState>, run_id: String) {
}
}
managed_run.run_dir = Some(run_dir);
managed_run.event_tx = None;
managed_run.cancel_tx = None;
managed_run.cancel_token = None;
clear_live_run_state(managed_run);
}
drop(runs);
state.scheduler_notify.notify_one();
@ -2459,6 +2479,49 @@ mod tests {
);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn cancel_before_run_transitions_to_running_closes_event_stream() {
let state = create_app_state_with_registry_factory(test_db().await, |interviewer| {
std::thread::sleep(std::time::Duration::from_millis(200));
fabro_workflows::handler::default_registry(interviewer, || None)
});
let app = build_router(Arc::clone(&state), AuthMode::Disabled);
let req = Request::builder()
.method("POST")
.uri("/runs")
.header("content-type", "application/json")
.body(Body::from(
serde_json::to_string(&serde_json::json!({"dot_source": MINIMAL_DOT})).unwrap(),
))
.unwrap();
let response = app.clone().oneshot(req).await.unwrap();
assert_eq!(response.status(), StatusCode::CREATED);
let body = body_json(response.into_body()).await;
let run_id = body["id"].as_str().unwrap().to_string();
let runner = tokio::spawn(execute_run(Arc::clone(&state), run_id.clone()));
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
let req = Request::builder()
.method("POST")
.uri(format!("/runs/{run_id}/cancel"))
.body(Body::empty())
.unwrap();
let response = app.clone().oneshot(req).await.unwrap();
assert_eq!(response.status(), StatusCode::OK);
runner.await.unwrap();
let req = Request::builder()
.method("GET")
.uri(format!("/runs/{run_id}/events"))
.body(Body::empty())
.unwrap();
let response = app.oneshot(req).await.unwrap();
assert_eq!(response.status(), StatusCode::GONE);
}
#[tokio::test]
async fn queue_position_reported_for_queued_runs() {
let state = create_app_state(test_db().await);