diff --git a/Cargo.lock b/Cargo.lock index 1ade11382..c55fc0c41 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2558,6 +2558,7 @@ dependencies = [ "sandbox-driver-host", "serde", "serde_json", + "smol_str", "sqlx", "tempfile", "thiserror 2.0.18", @@ -5163,7 +5164,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "petri-attractor-steps" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "async-trait", "globset", @@ -5194,7 +5195,7 @@ dependencies = [ [[package]] name = "petri-driver" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "async-trait", "getrandom 0.3.4", @@ -5214,7 +5215,7 @@ dependencies = [ [[package]] name = "petri-engine" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "petri-ir", "serde", @@ -5226,7 +5227,7 @@ dependencies = [ [[package]] name = "petri-execution" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "async-trait", "petri-driver", @@ -5250,7 +5251,7 @@ dependencies = [ [[package]] name = "petri-executor" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "async-trait", "libc", @@ -5265,7 +5266,7 @@ dependencies = [ [[package]] name = "petri-executor-sandbox" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "async-trait", "petri-executor", @@ -5287,7 +5288,7 @@ dependencies = [ [[package]] name = "petri-frontend" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "marked-yaml", "petri-ir", @@ -5301,7 +5302,7 @@ dependencies = [ [[package]] name = "petri-frontend-attractor" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "minijinja", "petri-frontend", @@ -5318,7 +5319,7 @@ dependencies = [ [[package]] name = "petri-frontend-fabro" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "petri-frontend", "petri-frontend-attractor", @@ -5334,7 +5335,7 @@ dependencies = [ [[package]] name = "petri-frontend-native" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "petri-frontend", "petri-ir", @@ -5345,7 +5346,7 @@ dependencies = [ [[package]] name = "petri-ir" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "regex", "serde", @@ -5358,7 +5359,7 @@ dependencies = [ [[package]] name = "petri-runtime" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "async-trait", "petri-driver", @@ -5379,7 +5380,7 @@ dependencies = [ [[package]] name = "petri-steps" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "async-trait", "petri-executor", @@ -5395,7 +5396,7 @@ dependencies = [ [[package]] name = "petri-store" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "async-trait", "getrandom 0.3.4", @@ -5410,7 +5411,7 @@ dependencies = [ [[package]] name = "petri-testkit" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/petri.git?branch=main#11465f23feafe303a18e255b5c51ca40e0d008f7" +source = "git+https://github.com/lithoscomputer/petri.git?branch=main#e46845bd0139dd04e9e795d3201b5b9b4b8b1026" dependencies = [ "async-trait", "petri-driver", @@ -5426,6 +5427,7 @@ dependencies = [ "serde_json", "smol_str", "tokio", + "tokio-util", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 8604c92c0..aac456640 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,6 +70,7 @@ jsonwebtoken = { version = "10", features = ["aws_lc_rs"] } hkdf = "0.12" hmac = "0.12" sha2 = "0.10" +smol_str = "0.3" hex = "0.4" insta = "1" fabro-test = { path = "lib/foundation/fabro-test" } diff --git a/docs/public/integrations/github.mdx b/docs/public/integrations/github.mdx index 9f2d9eac0..602e7f782 100644 --- a/docs/public/integrations/github.mdx +++ b/docs/public/integrations/github.mdx @@ -262,7 +262,7 @@ pull_requests = "write" Only the listed permissions are requested. If no GitHub credentials are configured, Fabro does not inject a managed token. If configured credentials cannot resolve a token, the execution scope fails to initialize. -GitHub-target runs also receive Git read access to their origin without declaring an integration permission map. In App mode this default token requests only `contents = "read"`, and it is used by Git's credential helper. Declaring integration permissions additionally supplies `GITHUB_TOKEN` to command and agent processes. Explicit workflow or ACP environment values take precedence over the managed `GITHUB_TOKEN`. +GitHub-target runs also receive Git read access to their origin without declaring an integration permission map. In App mode this default token requests only `contents = "read"`, and it is used by Git's credential helper. Declaring integration permissions additionally supplies `GITHUB_TOKEN` to command and agent processes. The managed token carries exactly the access the run declares, so it replaces any `GITHUB_TOKEN` set in the workflow environment, an ACP agent's environment or the sandbox's own environment. To give a stage different GitHub access, change the declared permissions or repositories instead. One-shot containers a stage runs (such as `docker://` steps) receive the managed `GITHUB_TOKEN` but not the Git credential helper, which reads a store inside the stage's sandbox. In App mode, the token covers only the run's origin repository unless the run declares [additional repositories](#additional-repositories). Injecting `GITHUB_TOKEN` alone does not make other private repositories reachable. diff --git a/lib/apps/fabro-cli/tests/it/cmd/attach.rs b/lib/apps/fabro-cli/tests/it/cmd/attach.rs index c8978361e..ff4b4bb42 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/attach.rs @@ -1103,7 +1103,7 @@ fn attach_json_errors_without_prompting_for_human_input() { "recorded_at": "[EPOCH_MS]", "body": { "event": "run.started", - "format_version": 7, + "format_version": 8, "key": "[ULID]", "root": 0, "middleware_chain": [ diff --git a/lib/components/fabro-petri/Cargo.toml b/lib/components/fabro-petri/Cargo.toml index 9cb0d049b..4034e8a0f 100644 --- a/lib/components/fabro-petri/Cargo.toml +++ b/lib/components/fabro-petri/Cargo.toml @@ -21,6 +21,7 @@ test-support = ["dep:petri_testkit"] [dependencies] fabro-github = { path = "../fabro-github" } percent-encoding.workspace = true +smol_str.workspace = true fabro-api = { path = "../../foundation/fabro-api" } fabro-client = { path = "../../foundation/fabro-client" } fabro-db = { path = "../../foundation/fabro-db" } diff --git a/lib/components/fabro-petri/src/projection/engine.rs b/lib/components/fabro-petri/src/projection/engine.rs index 12fae7891..a6c82b292 100644 --- a/lib/components/fabro-petri/src/projection/engine.rs +++ b/lib/components/fabro-petri/src/projection/engine.rs @@ -13,7 +13,7 @@ use fabro_types::{ use petri_execution::events::{Derived, RunEvent, Subject, ViewEvent, WaitState}; use petri_execution::{ExecutionId, InvocationId}; use petri_runtime::engine::{Admission, Event}; -use petri_runtime::ir::{Metrics, Status}; +use petri_runtime::ir::{Metrics, Status, UnderlyingFailure}; use serde_json::Value; use tracing::debug; @@ -382,9 +382,12 @@ pub(super) fn failure_message(status: &Status) -> Option { match status { Status::Failure(info) | Status::PartialSuccess { - underlying: Some(info), + underlying: Some(UnderlyingFailure::Failure(info)), } => Some(info.message.clone()), - Status::TimedOut => Some("the step timed out".to_string()), + Status::TimedOut + | Status::PartialSuccess { + underlying: Some(UnderlyingFailure::TimedOut), + } => Some("the step timed out".to_string()), Status::Cancelled => Some("the step was cancelled".to_string()), Status::Success | Status::PartialSuccess { underlying: None } | Status::Skipped => None, } diff --git a/lib/components/fabro-petri/src/stage_credentials.rs b/lib/components/fabro-petri/src/stage_credentials.rs index 35101db3d..7d8db5f99 100644 --- a/lib/components/fabro-petri/src/stage_credentials.rs +++ b/lib/components/fabro-petri/src/stage_credentials.rs @@ -2,10 +2,11 @@ //! //! Git reads a private, renewable store outside the workspace. Processes get //! the helper configuration and, when requested, a fresh `GITHUB_TOKEN` at -//! spawn. A long-lived agent's Git commands read the renewed store. +//! spawn. A long-lived agent's Git commands read the renewed store. A +//! one-shot container gets the token alone: the store lives in the scope's +//! sandbox, which the container does not share. -use std::collections::HashMap; -use std::path::Path; +use std::collections::{BTreeMap, HashMap}; use std::sync::Arc; use std::time::Duration; @@ -17,10 +18,11 @@ use fabro_types::{GitHubRepositorySlug, RunSpec, RunTarget}; use fabro_util::shell; use percent_encoding::{NON_ALPHANUMERIC, utf8_percent_encode}; use petri_runtime::executor::{ - AcquireContext, DirectoryEntry, EnvError, EnvHandle, ExecEnv, Executor, Masker, PreviewUrl, - ProcessHandle, ProcessSpec, ReleaseReport, ScopeOutcome, ScopeSpec, + AcquireContext, EnvError, EnvHandle, ExecEnv, Executor, Masker, ProcessSpec, ReleaseReport, + ScopeOutcome, ScopeSpec, SpawnEnv, SpawnTarget, }; use petri_runtime::ir::LogStream; +use smol_str::SmolStr; use tokio::sync::Mutex; use tokio::task::JoinHandle; use tokio::time; @@ -184,7 +186,7 @@ impl Executor for CredentialExecutor { env: env.clone(), task, }); - Ok(handle.with_exec(env)) + Ok(handle.with_spawn_env(env)) } async fn release(&self, handle: EnvHandle, outcome: ScopeOutcome) -> ReleaseReport { @@ -248,9 +250,8 @@ impl CredentialEnv { .map(|_| ()) } - fn git_env(&self, spec: &mut ProcessSpec) -> Result<(), EnvError> { - let count = spec - .env + fn git_env(&self, env: &mut BTreeMap) -> Result<(), EnvError> { + let count = env .get("GIT_CONFIG_COUNT") .map(ToString::to_string) .or_else(|| self.inner.ambient_env("GIT_CONFIG_COUNT")) @@ -288,98 +289,60 @@ impl CredentialEnv { for index in 0..count { for prefix in ["GIT_CONFIG_KEY_", "GIT_CONFIG_VALUE_"] { let key = format!("{prefix}{index}"); - if !spec.env.contains_key(key.as_str()) { + if !env.contains_key(key.as_str()) { if let Some(value) = self.inner.ambient_env(&key) { - spec.env.insert(key.into(), value.into()); + env.insert(key.into(), value.into()); } } } } for (offset, (key, value)) in entries.iter().enumerate() { - spec.env.insert( + env.insert( format!("GIT_CONFIG_KEY_{}", count + offset).into(), (*key).into(), ); - spec.env.insert( + env.insert( format!("GIT_CONFIG_VALUE_{}", count + offset).into(), value.as_str().into(), ); } - spec.env.insert( + env.insert( "GIT_CONFIG_COUNT".into(), (count + entries.len()).to_string().into(), ); - spec.env - .entry("GIT_TERMINAL_PROMPT".into()) + env.entry("GIT_TERMINAL_PROMPT".into()) .or_insert_with(|| "0".into()); Ok(()) } } #[async_trait] -impl ExecEnv for CredentialEnv { - async fn spawn(&self, mut spec: ProcessSpec) -> Result, EnvError> { - self.refresh().await?; - if let Some(tokens) = &self.credentials.api_tokens { - if !spec.env.contains_key("GITHUB_TOKEN") - && self.inner.ambient_env("GITHUB_TOKEN").is_none() - { - let resolved = time::timeout(CREDENTIAL_TIMEOUT, tokens.resolve()) - .await - .map_err(|_| { - EnvError::backend("github", "resolve", "token resolution timed out") - })? - .map_err(|_| { - EnvError::backend("github", "resolve", "token resolution failed") - })?; - self.masker.register_explicit(resolved.token.expose()); - spec.env - .insert("GITHUB_TOKEN".into(), resolved.token.expose().into()); - } +impl SpawnEnv for CredentialEnv { + async fn apply( + &self, + target: SpawnTarget, + env: &mut BTreeMap, + ) -> Result<(), EnvError> { + // The store and the helper configuration that names it live in the + // scope's sandbox; a one-shot container cannot read either. + if target == SpawnTarget::Process { + self.refresh().await?; } - self.git_env(&mut spec)?; - self.inner.spawn(spec).await - } - fn workspace_path(&self) -> &str { - self.inner.workspace_path() - } - async fn read_file(&self, path: &Path) -> Result>, EnvError> { - self.inner.read_file(path).await - } - async fn read_file_limited( - &self, - path: &Path, - limit: usize, - ) -> Result>, EnvError> { - self.inner.read_file_limited(path, limit).await - } - async fn write_file(&self, path: &Path, contents: &[u8]) -> Result<(), EnvError> { - self.inner.write_file(path, contents).await - } - async fn list_directory( - &self, - path: &Path, - depth: usize, - ) -> Result, EnvError> { - self.inner.list_directory(path, depth).await - } - fn grace(&self) -> Duration { - self.inner.grace() - } - fn host_address(&self) -> Result<&str, EnvError> { - self.inner.host_address() - } - fn ambient_env(&self, name: &str) -> Option { - self.inner.ambient_env(name) - } - fn shares_host_filesystem(&self) -> bool { - self.inner.shares_host_filesystem() - } - async fn preview_url(&self, port: u16) -> Result, EnvError> { - self.inner.preview_url(port).await - } - async fn release_preview_url(&self, port: u16) -> Result<(), EnvError> { - self.inner.release_preview_url(port).await + // The managed token carries exactly the access the run declared, so + // it replaces any `GITHUB_TOKEN` the process would otherwise see, as + // Fabro's stage environment did before Petri. + if let Some(tokens) = &self.credentials.api_tokens { + let resolved = time::timeout(CREDENTIAL_TIMEOUT, tokens.resolve()) + .await + .map_err(|_| EnvError::backend("github", "resolve", "token resolution timed out"))? + .map_err(|_| EnvError::backend("github", "resolve", "token resolution failed"))?; + self.masker.register_explicit(resolved.token.expose()); + env.insert("GITHUB_TOKEN".into(), resolved.token.expose().into()); + } + if target == SpawnTarget::Process { + self.git_env(env)?; + } + Ok(()) } } @@ -421,6 +384,7 @@ async fn command( mod tests { use std::fs; use std::os::unix::fs::PermissionsExt as _; + use std::path::Path; use std::sync::atomic::{AtomicUsize, Ordering}; use chrono::Utc; @@ -580,7 +544,59 @@ mod tests { } #[tokio::test] - async fn declared_api_tokens_reach_processes_and_explicit_environment_wins() { + async fn a_container_gets_the_managed_token_but_no_store_configuration() { + let dir = tempfile::tempdir().expect("directory"); + let runtime = providers::standard_runtime(&SandboxProviderConfig::default()); + let router = runtime.sandbox_router_for(dir.path()).expect("router"); + let handle = router + .acquire( + &ScopeSpec::new(ScopeId::new(0), "container-credentials"), + &AcquireContext::bare(), + ) + .await + .expect("acquire"); + let store = dir.path().join("store-directory"); + let secrets = MapSecrets::empty(); + let env = CredentialEnv { + inner: handle.exec(), + credentials: credentials(true), + masker: secrets.masker(), + directory: store.display().to_string(), + update: Mutex::new(()), + }; + let mut container = BTreeMap::from([("GITHUB_TOKEN".into(), "explicit".into())]); + env.apply(SpawnTarget::Container, &mut container) + .await + .expect("apply"); + assert!( + container["GITHUB_TOKEN"].starts_with("scripted-token-generation-"), + "the managed token replaces an explicit one in a container too" + ); + assert!( + !container.keys().any(|key| key.starts_with("GIT_CONFIG")), + "no helper configuration names the scope's store" + ); + assert!(!store.exists(), "a container spawn writes no store"); + let mut process = BTreeMap::new(); + fs::create_dir(&store).expect("store directory"); + env.apply(SpawnTarget::Process, &mut process) + .await + .expect("apply"); + assert!(process.contains_key("GIT_CONFIG_COUNT")); + assert!( + store.join("store").exists(), + "a process spawn refreshes the store" + ); + assert!( + router + .release(handle, ScopeOutcome::Succeeded) + .await + .is_clean() + ); + } + + #[tokio::test] + async fn declared_api_tokens_reach_processes_and_replace_explicit_values() { let dir = tempfile::tempdir().expect("directory"); let runtime = providers::standard_runtime(&SandboxProviderConfig::default()); let router = runtime.sandbox_router_for(dir.path()).expect("router"); @@ -603,14 +619,14 @@ mod tests { .expect("the integration token reaches the child"); command( &env, - "test \"$GITHUB_TOKEN\" = command-token-override", + "case $GITHUB_TOKEN in scripted-token-generation-*) exit 0;; *) exit 1;; esac", vec![( "GITHUB_TOKEN".to_string(), "command-token-override".to_string(), )], ) .await - .expect("explicit command environment wins"); + .expect("the managed token replaces an explicit one"); assert!( secrets .masker()