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

fix(server): forward GITHUB_APP_PRIVATE_KEY via worker_command

Browse source 
The previous commit added GITHUB_APP_PRIVATE_KEY to the worker env
allowlist, but the secret is in server.env / ServerSecrets, not in the
server's process env, so the allowlist couldn't see it.

Forward the value explicitly from ServerSecrets at spawn time, mirroring
how FABRO_WORKER_TOKEN is already passed. Keeps the allowlist narrow as
a fail-closed barrier against ambient env leakage and keeps ServerSecrets
as the single read site for server.env secrets.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-29 08:09:02 -04:00
parent 85c763d511
commit f40bcd1215
No known key found for this signature in database
2 changed files with 59 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -3907,6 +3907,9 @@ fn worker_command(
cmd.env(EnvVars::FABRO_LOG_DESTINATION, value);
cmd.env_remove(EnvVars::FABRO_WORKER_TOKEN);
cmd.env(EnvVars::FABRO_WORKER_TOKEN, worker_token);
if let Some(pem) = state.server_secret(EnvVars::GITHUB_APP_PRIVATE_KEY) {
cmd.env(EnvVars::GITHUB_APP_PRIVATE_KEY, pem);
}
#[cfg(unix)]
fabro_proc::pre_exec_setpgid(cmd.as_std_mut());
@ -9172,6 +9175,52 @@ provider = "invalid-provider"
assert_eq!(dev_claims.run_id, dev_token_run_id.to_string());
}
#[cfg(unix)]
#[test]
fn worker_command_forwards_github_app_private_key_from_server_secrets() {
let storage_dir = tempfile::tempdir().unwrap();
let state = worker_command_test_state_with_extra_config_and_env_lookup(
storage_dir.path(),
&["dev-token"],
Some(TEST_DEV_TOKEN),
"",
&[(EnvVars::GITHUB_APP_PRIVATE_KEY, "test-private-key")],
|_| None,
);
let cmd = worker_command(
state.as_ref(),
RunId::new(),
RunExecutionMode::Start,
storage_dir.path(),
)
.unwrap();
assert_eq!(
command_env_value(&cmd, EnvVars::GITHUB_APP_PRIVATE_KEY),
EnvOverride::Set("test-private-key".to_string())
);
}
#[cfg(unix)]
#[test]
fn worker_command_omits_github_app_private_key_when_unset() {
let storage_dir = tempfile::tempdir().unwrap();
let state =
worker_command_test_state(storage_dir.path(), &["dev-token"], Some(TEST_DEV_TOKEN));
let cmd = worker_command(
state.as_ref(),
RunId::new(),
RunExecutionMode::Start,
storage_dir.path(),
)
.unwrap();
assert_eq!(
command_env_value(&cmd, EnvVars::GITHUB_APP_PRIVATE_KEY),
EnvOverride::Unchanged
);
}
#[cfg(unix)]
#[test]
fn worker_command_sets_fabro_log_from_server_logging_config() {
@ -9242,6 +9291,7 @@ destination = "stdout"
[server.logging]
destination = "file"
"#,
&[],
|name| (name == EnvVars::FABRO_LOG_DESTINATION).then(|| "stdout".to_string()),
);
let run_id = RunId::new();
@ -9272,6 +9322,7 @@ destination = "file"
[server.logging]
destination = "file"
"#,
&[],
|name| (name == EnvVars::FABRO_LOG_DESTINATION).then(|| "stdot".to_string()),
);
let run_id = RunId::new();
@ -9347,6 +9398,7 @@ methods = ["dev-token"]
methods,
dev_token,
extra_config,
&[],
|_| None,
)
}
@ -9356,6 +9408,7 @@ methods = ["dev-token"]
methods: &[&str],
dev_token: Option<&str>,
extra_config: &str,
extra_server_secrets: &[(&str, &str)],
env_lookup: impl Fn(&str) -> Option<String> + Send + Sync + 'static,
) -> Arc<AppState> {
let dev_token = dev_token.map(str::to_owned);
@ -9390,9 +9443,12 @@ allowed_usernames = ["octocat"]
.write(&runtime_directory)
.unwrap();
let server_secret_env = dev_token
let mut server_secret_env: HashMap<String, String> = dev_token
.map(|token| HashMap::from([("FABRO_DEV_TOKEN".to_string(), token)]))
.unwrap_or_default();
for (key, value) in extra_server_secrets {
server_secret_env.insert((*key).to_string(), (*value).to_string());
}
create_app_state_with_env_lookup_and_server_secret_env(
server_settings_from_toml(&source),
manifest_run_defaults_from_toml(&source),

11
lib/crates/fabro-server/src/spawn_env.rs
View file

@ -13,7 +13,6 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[
EnvVars::FABRO_LOG,
EnvVars::FABRO_HOME,
EnvVars::FABRO_STORAGE_ROOT,
EnvVars::GITHUB_APP_PRIVATE_KEY,
];
const RENDER_GRAPH_ENV_ALLOWLIST: &[&str] = &[EnvVars::PATH, EnvVars::HOME, EnvVars::TMPDIR];
@ -87,10 +86,7 @@ mod tests {
("SESSION_SECRET".to_string(), "leak".to_string()),
("FABRO_JWT_PRIVATE_KEY".to_string(), "leak".to_string()),
("FABRO_JWT_PUBLIC_KEY".to_string(), "leak".to_string()),
(
"GITHUB_APP_PRIVATE_KEY".to_string(),
"private-key".to_string(),
),
("GITHUB_APP_PRIVATE_KEY".to_string(), "leak".to_string()),
("GITHUB_APP_CLIENT_SECRET".to_string(), "leak".to_string()),
("GITHUB_APP_WEBHOOK_SECRET".to_string(), "leak".to_string()),
("FABRO_DEV_TOKEN".to_string(), "garbage".to_string()),
@ -118,10 +114,7 @@ mod tests {
assert!(!actual.contains_key("SESSION_SECRET"));
assert!(!actual.contains_key("FABRO_JWT_PRIVATE_KEY"));
assert!(!actual.contains_key("FABRO_JWT_PUBLIC_KEY"));
assert_eq!(
actual.get("GITHUB_APP_PRIVATE_KEY").map(String::as_str),
Some("private-key")
);
assert!(!actual.contains_key("GITHUB_APP_PRIVATE_KEY"));
assert!(!actual.contains_key("GITHUB_APP_CLIENT_SECRET"));
assert!(!actual.contains_key("GITHUB_APP_WEBHOOK_SECRET"));
assert!(!actual.contains_key("MY_API_KEY"));