mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
feat(workflow): bridge git and gh to the shared token
Carry the resolved GitHub integration (permissions plus declared additional repositories) as one value from run materialization into workflow startup, and make the sandbox environment reach every declared repository through the single managed GITHUB_TOKEN. - `StartServices.github_permissions` becomes `github_integration: ResolvedGithubIntegration`; CLI and server workers build it with `resolve_integration()` after interpolation and pass it through `SandboxEnvSpec` as one unit. - `build_sandbox_env` constructs the validated `GitHubRepositoryAccess` and scopes the App token source to the whole effective set. Missing credentials or a missing origin are hard initialization errors when additional repositories are declared; legacy permissions-only configuration keeps its best-effort behavior. - When additional repositories are declared, initialization eagerly resolves each repository's App installation (naming any repository the App cannot see) and the token itself, so an inaccessible declared repository fails before the first workflow stage. - A new `git_bridge` module injects secret-free `GIT_CONFIG_*` entries into the stage environment: a github.com credential helper that reads `$GITHUB_TOKEN` at invocation time, per-repository SSH-to-HTTPS `insteadOf` rewrites, and `GIT_TERMINAL_PROMPT=0`. Entries append after a valid user-provided Git config overlay and fail clearly on a malformed one. Contract tests drive the installed git binary against local fixtures for the rewrite, credential, prefix-collision, and overlay-preservation behaviors. - The long-running ACP notice now says all declared repository access expires together. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
7bfed23153
commit
d8edd410f3
10 changed files with 785 additions and 56 deletions
|
|
@ -161,13 +161,13 @@ pub(crate) async fn execute(
|
|||
artifact_sink,
|
||||
run_control: Some(run_control),
|
||||
github_app,
|
||||
github_permissions: run_spec
|
||||
github_integration: run_spec
|
||||
.settings
|
||||
.run
|
||||
.integrations
|
||||
.github
|
||||
.resolve_permissions()
|
||||
.context("failed to resolve github permissions")?,
|
||||
.resolve_integration()
|
||||
.context("failed to resolve github integration")?,
|
||||
vault,
|
||||
catalog,
|
||||
on_node: None,
|
||||
|
|
|
|||
|
|
@ -4081,15 +4081,15 @@ async fn execute_run_in_process(state: Arc<AppState>, run_id: RunId) {
|
|||
return;
|
||||
}
|
||||
};
|
||||
let github_permissions = match persisted
|
||||
let github_integration = match persisted
|
||||
.run_spec()
|
||||
.settings
|
||||
.run
|
||||
.integrations
|
||||
.github
|
||||
.resolve_permissions()
|
||||
.resolve_integration()
|
||||
{
|
||||
Ok(permissions) => permissions,
|
||||
Ok(integration) => integration,
|
||||
Err(err) => {
|
||||
tracing::error!(
|
||||
run_id = %run_id,
|
||||
|
|
@ -4131,7 +4131,7 @@ async fn execute_run_in_process(state: Arc<AppState>, run_id: RunId) {
|
|||
artifact_sink: Some(ArtifactSink::Store(state.artifact_store.clone())),
|
||||
run_control: None,
|
||||
github_app,
|
||||
github_permissions,
|
||||
github_integration,
|
||||
vault: Arc::new(AsyncRwLock::new(vault.into_vault())),
|
||||
catalog: state.catalog(),
|
||||
on_node: None,
|
||||
|
|
|
|||
|
|
@ -140,6 +140,19 @@ impl GitHubRepositoryAccess {
|
|||
!self.additional.is_empty()
|
||||
}
|
||||
|
||||
/// [`Self::resolve_shared_installation`] against the production GitHub
|
||||
/// API with a fresh HTTP client.
|
||||
pub async fn resolve_shared_installation_via_api(
|
||||
&self,
|
||||
creds: &GitHubAppCredentials,
|
||||
) -> anyhow::Result<u64> {
|
||||
let client = fabro_http::http_client()
|
||||
.map_err(anyhow::Error::new)
|
||||
.context("building HTTP client for installation resolution")?;
|
||||
self.resolve_shared_installation(creds, &client, &crate::github_api_base_url())
|
||||
.await
|
||||
}
|
||||
|
||||
/// Resolve every target's App installation and require one shared
|
||||
/// installation ID, so a repository the App cannot see — or one that
|
||||
/// resolves to a different installation — is named before any token is
|
||||
|
|
|
|||
448
lib/components/fabro-workflow/src/git_bridge.rs
Normal file
448
lib/components/fabro-workflow/src/git_bridge.rs
Normal file
|
|
@ -0,0 +1,448 @@
|
|||
//! Secret-free Git bridging environment for additional-repository access.
|
||||
//!
|
||||
//! When a run declares additional GitHub repositories, every resolved
|
||||
//! command/tool/ACP environment receives `GIT_CONFIG_COUNT` /
|
||||
//! `GIT_CONFIG_KEY_n` / `GIT_CONFIG_VALUE_n` entries that make plain Git
|
||||
//! commands work against the declared set through the managed
|
||||
//! `GITHUB_TOKEN`:
|
||||
//!
|
||||
//! - a credential helper for `https://github.com` that reads `$GITHUB_TOKEN`
|
||||
//! from the invoking Git process's environment at invocation time, so token
|
||||
//! refresh flows through per-stage environment resolution with no bridging
|
||||
//! update;
|
||||
//! - per-repository `url.<https>.insteadOf` rewrites for the
|
||||
//! `git@github.com:owner/repo[.git]` and
|
||||
//! `ssh://git@github.com/owner/repo[.git]` SSH spellings of each effective
|
||||
//! repository.
|
||||
//!
|
||||
//! None of the values contain a secret; the token lives only in
|
||||
//! `GITHUB_TOKEN`.
|
||||
//!
|
||||
//! The credential helper is host-scoped to `https://github.com`, not
|
||||
//! path-scoped. This is safe because the token is scoped server-side to the
|
||||
//! declared repository set and is only ever offered to github.com. It does
|
||||
//! change one failure mode for *undeclared* repositories: public HTTPS
|
||||
//! clones are unaffected (Git tries unauthenticated first), while private
|
||||
//! undeclared HTTPS repositories fail with a GitHub authorization error
|
||||
//! instead of a missing-credential error. Both fail; only the diagnostic
|
||||
//! differs.
|
||||
//!
|
||||
//! `insteadOf` matches by string prefix, not exactly: a rule for
|
||||
//! `owner/repo` also matches `owner/repo-other`. An undeclared repository
|
||||
//! that shares a declared prefix is therefore rewritten to HTTPS; the scoped
|
||||
//! token is invalid for it at GitHub, so authority is unchanged, but its Git
|
||||
//! transport changes from SSH to HTTPS.
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
use fabro_types::GitHubRepositorySlug;
|
||||
|
||||
use crate::error::Error;
|
||||
|
||||
/// Section base for the effective repositories' HTTPS routes.
|
||||
const GITHUB_HTTPS_BASE: &str = "https://github.com/";
|
||||
|
||||
const CREDENTIAL_HELPER_KEY: &str = "credential.https://github.com.helper";
|
||||
/// Reads the invoking process's `$GITHUB_TOKEN` at invocation time; contains
|
||||
/// no secret itself. Non-`get` operations (`store`, `erase`) are ignored.
|
||||
const CREDENTIAL_HELPER: &str = r#"!f() { if [ "$1" = get ]; then echo username=x-access-token; echo "password=$GITHUB_TOKEN"; fi; }; f"#;
|
||||
|
||||
/// Merge the bridging entries into `env` for the effective repository set
|
||||
/// (primary first). Appends after any valid user-provided `GIT_CONFIG_COUNT`
|
||||
/// overlay without overwriting it, and fails with a configuration error when
|
||||
/// the user overlay is malformed rather than silently replacing it.
|
||||
pub(crate) fn merge_git_bridge_env(
|
||||
env: &mut HashMap<String, String>,
|
||||
targets: &[&GitHubRepositorySlug],
|
||||
) -> Result<(), Error> {
|
||||
let start = user_git_config_count(env)?;
|
||||
for (offset, (key, value)) in bridge_entries(targets, GITHUB_HTTPS_BASE)
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
{
|
||||
let index = start + offset;
|
||||
env.insert(format!("GIT_CONFIG_KEY_{index}"), key);
|
||||
env.insert(format!("GIT_CONFIG_VALUE_{index}"), value);
|
||||
}
|
||||
let total = start + bridge_entry_count(targets);
|
||||
env.insert("GIT_CONFIG_COUNT".to_string(), total.to_string());
|
||||
// Fail instead of hanging when access is missing or invalid; a user who
|
||||
// explicitly configured prompting keeps their value.
|
||||
env.entry("GIT_TERMINAL_PROMPT".to_string())
|
||||
.or_insert_with(|| "0".to_string());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn bridge_entry_count(targets: &[&GitHubRepositorySlug]) -> usize {
|
||||
1 + targets.len() * 2
|
||||
}
|
||||
|
||||
/// The bridge's Git config entries in order: the credential helper, then two
|
||||
/// SSH-to-HTTPS rewrites per repository. `https_base` is
|
||||
/// [`GITHUB_HTTPS_BASE`] in production; contract tests substitute a local
|
||||
/// `file://` root to prove real Git applies the generated entries without
|
||||
/// touching the network.
|
||||
fn bridge_entries(targets: &[&GitHubRepositorySlug], https_base: &str) -> Vec<(String, String)> {
|
||||
let mut entries = Vec::with_capacity(bridge_entry_count(targets));
|
||||
entries.push((
|
||||
CREDENTIAL_HELPER_KEY.to_string(),
|
||||
CREDENTIAL_HELPER.to_string(),
|
||||
));
|
||||
for slug in targets {
|
||||
let owner = slug.owner();
|
||||
let repo = slug.repo();
|
||||
let https = format!("{https_base}{owner}/{repo}");
|
||||
// One prefix rule per SSH spelling covers both the bare and `.git`
|
||||
// suffixed forms.
|
||||
entries.push((
|
||||
format!("url.{https}.insteadOf"),
|
||||
format!("git@github.com:{owner}/{repo}"),
|
||||
));
|
||||
entries.push((
|
||||
format!("url.{https}.insteadOf"),
|
||||
format!("ssh://git@github.com/{owner}/{repo}"),
|
||||
));
|
||||
}
|
||||
entries
|
||||
}
|
||||
|
||||
/// Validate and measure a user-provided `GIT_CONFIG_COUNT` overlay so the
|
||||
/// bridge appends after it. Orphaned `GIT_CONFIG_KEY_n` entries without a
|
||||
/// count are inert to Git and are treated as absent.
|
||||
fn user_git_config_count(env: &HashMap<String, String>) -> Result<usize, Error> {
|
||||
let Some(raw) = env.get("GIT_CONFIG_COUNT") else {
|
||||
return Ok(0);
|
||||
};
|
||||
let count: usize = raw.trim().parse().map_err(|_| {
|
||||
Error::Precondition(format!(
|
||||
"environment variable GIT_CONFIG_COUNT must be a non-negative integer to combine \
|
||||
with Fabro's Git bridging entries, got `{raw}`"
|
||||
))
|
||||
})?;
|
||||
for index in 0..count {
|
||||
let key = format!("GIT_CONFIG_KEY_{index}");
|
||||
let value = format!("GIT_CONFIG_VALUE_{index}");
|
||||
if !env.contains_key(&key) || !env.contains_key(&value) {
|
||||
return Err(Error::Precondition(format!(
|
||||
"GIT_CONFIG_COUNT is {count} but {key} or {value} is missing; fix the indexed \
|
||||
Git config overlay so Fabro can append its bridging entries after it"
|
||||
)));
|
||||
}
|
||||
}
|
||||
Ok(count)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
clippy::disallowed_types,
|
||||
reason = "contract tests drive the installed git binary synchronously in non-async tests"
|
||||
)]
|
||||
mod tests {
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
|
||||
use super::*;
|
||||
|
||||
fn slug(value: &str) -> GitHubRepositorySlug {
|
||||
value.parse().expect("test slug should parse")
|
||||
}
|
||||
|
||||
fn bridged_env(
|
||||
base_env: HashMap<String, String>,
|
||||
targets: &[&GitHubRepositorySlug],
|
||||
) -> HashMap<String, String> {
|
||||
let mut env = base_env;
|
||||
merge_git_bridge_env(&mut env, targets).expect("bridge entries should merge");
|
||||
env
|
||||
}
|
||||
|
||||
/// Run `git` with ONLY the bridge-relevant environment: the inherited
|
||||
/// user/system/global Git config is disabled so assertions observe just
|
||||
/// the generated entries.
|
||||
fn git(args: &[&str], env: &HashMap<String, String>, cwd: &Path) -> std::process::Output {
|
||||
let mut command = Command::new("git");
|
||||
command
|
||||
.args(args)
|
||||
.current_dir(cwd)
|
||||
.env("GIT_CONFIG_NOSYSTEM", "1")
|
||||
.env("GIT_CONFIG_GLOBAL", "/dev/null")
|
||||
.env("GIT_TERMINAL_PROMPT", "0")
|
||||
.env("GIT_ASKPASS", "true");
|
||||
for (key, value) in env {
|
||||
command.env(key, value);
|
||||
}
|
||||
command.output().expect("git should run")
|
||||
}
|
||||
|
||||
/// Create a bare fixture answering both the bare and `.git`-suffixed
|
||||
/// routes, the way GitHub serves both HTTPS spellings.
|
||||
fn init_bare_fixture(root: &Path, owner_repo: &str) -> String {
|
||||
let fixture = root.join(format!("{owner_repo}.git"));
|
||||
std::fs::create_dir_all(&fixture).unwrap();
|
||||
let init = Command::new("git")
|
||||
.args(["init", "--bare", "--initial-branch=main"])
|
||||
.arg(&fixture)
|
||||
.output()
|
||||
.expect("git init should run");
|
||||
assert!(init.status.success(), "{init:?}");
|
||||
#[cfg(unix)]
|
||||
std::os::unix::fs::symlink(&fixture, root.join(owner_repo)).unwrap();
|
||||
format!("file://{}/", root.display())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_targets_means_no_bridge_call_and_empty_env_stays_empty() {
|
||||
// The caller only bridges when the additional set is non-empty; the
|
||||
// pure entry builder is still total for the primary-only case.
|
||||
assert_eq!(bridge_entry_count(&[]), 1);
|
||||
let env: HashMap<String, String> = HashMap::new();
|
||||
assert!(!env.contains_key("GIT_CONFIG_COUNT"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merges_helper_rewrites_count_and_terminal_prompt() {
|
||||
let keystone = slug("fabro-sh/keystone");
|
||||
let fabro = slug("fabro-sh/fabro");
|
||||
let env = bridged_env(HashMap::new(), &[&fabro, &keystone]);
|
||||
|
||||
assert_eq!(env.get("GIT_CONFIG_COUNT").map(String::as_str), Some("5"));
|
||||
assert_eq!(
|
||||
env.get("GIT_CONFIG_KEY_0").map(String::as_str),
|
||||
Some("credential.https://github.com.helper")
|
||||
);
|
||||
assert_eq!(
|
||||
env.get("GIT_CONFIG_KEY_1").map(String::as_str),
|
||||
Some("url.https://github.com/fabro-sh/fabro.insteadOf")
|
||||
);
|
||||
assert_eq!(
|
||||
env.get("GIT_CONFIG_VALUE_1").map(String::as_str),
|
||||
Some("git@github.com:fabro-sh/fabro")
|
||||
);
|
||||
assert_eq!(
|
||||
env.get("GIT_CONFIG_VALUE_2").map(String::as_str),
|
||||
Some("ssh://git@github.com/fabro-sh/fabro")
|
||||
);
|
||||
assert_eq!(
|
||||
env.get("GIT_TERMINAL_PROMPT").map(String::as_str),
|
||||
Some("0")
|
||||
);
|
||||
|
||||
// No secrets anywhere in the generated values.
|
||||
for (key, value) in &env {
|
||||
assert!(!value.contains("ghs_"), "{key}={value}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn respects_an_explicit_user_terminal_prompt() {
|
||||
let keystone = slug("fabro-sh/keystone");
|
||||
let env = bridged_env(
|
||||
HashMap::from([("GIT_TERMINAL_PROMPT".to_string(), "1".to_string())]),
|
||||
&[&keystone],
|
||||
);
|
||||
assert_eq!(
|
||||
env.get("GIT_TERMINAL_PROMPT").map(String::as_str),
|
||||
Some("1")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn appends_after_a_valid_user_git_config_overlay() {
|
||||
let keystone = slug("fabro-sh/keystone");
|
||||
let env = bridged_env(
|
||||
HashMap::from([
|
||||
("GIT_CONFIG_COUNT".to_string(), "1".to_string()),
|
||||
("GIT_CONFIG_KEY_0".to_string(), "user.name".to_string()),
|
||||
("GIT_CONFIG_VALUE_0".to_string(), "Overlay User".to_string()),
|
||||
]),
|
||||
&[&keystone],
|
||||
);
|
||||
|
||||
assert_eq!(env.get("GIT_CONFIG_COUNT").map(String::as_str), Some("4"));
|
||||
assert_eq!(
|
||||
env.get("GIT_CONFIG_KEY_0").map(String::as_str),
|
||||
Some("user.name"),
|
||||
"user entry must survive at its original index"
|
||||
);
|
||||
assert_eq!(
|
||||
env.get("GIT_CONFIG_KEY_1").map(String::as_str),
|
||||
Some("credential.https://github.com.helper")
|
||||
);
|
||||
|
||||
// Real Git sees both the user's entry and the appended bridge entry.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = git(&["config", "--list"], &env, dir.path());
|
||||
assert!(output.status.success(), "{output:?}");
|
||||
let listed = String::from_utf8_lossy(&output.stdout);
|
||||
assert!(listed.contains("user.name=Overlay User"), "{listed}");
|
||||
assert!(
|
||||
listed.contains("credential.https://github.com.helper"),
|
||||
"{listed}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_a_malformed_user_git_config_overlay() {
|
||||
let keystone = slug("fabro-sh/keystone");
|
||||
|
||||
let mut non_numeric = HashMap::from([("GIT_CONFIG_COUNT".to_string(), "two".to_string())]);
|
||||
let err = merge_git_bridge_env(&mut non_numeric, &[&keystone]).unwrap_err();
|
||||
assert!(err.to_string().contains("GIT_CONFIG_COUNT"), "{err}");
|
||||
|
||||
let mut missing_index = HashMap::from([
|
||||
("GIT_CONFIG_COUNT".to_string(), "2".to_string()),
|
||||
("GIT_CONFIG_KEY_0".to_string(), "user.name".to_string()),
|
||||
("GIT_CONFIG_VALUE_0".to_string(), "Overlay".to_string()),
|
||||
]);
|
||||
let err = merge_git_bridge_env(&mut missing_index, &[&keystone]).unwrap_err();
|
||||
assert!(err.to_string().contains("GIT_CONFIG_KEY_1"), "{err}");
|
||||
}
|
||||
|
||||
/// With the bridge active, `git credential fill` for github.com resolves
|
||||
/// through the generated helper and reads `$GITHUB_TOKEN` from the
|
||||
/// invoking process environment at invocation time.
|
||||
#[test]
|
||||
fn credential_helper_reads_github_token_at_invocation_time() {
|
||||
use std::io::Write as _;
|
||||
|
||||
let keystone = slug("fabro-sh/keystone");
|
||||
let mut env = bridged_env(HashMap::new(), &[&keystone]);
|
||||
env.insert("GITHUB_TOKEN".to_string(), "test-token-value".to_string());
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut command = Command::new("git");
|
||||
command
|
||||
.args(["credential", "fill"])
|
||||
.current_dir(dir.path())
|
||||
.env("GIT_CONFIG_NOSYSTEM", "1")
|
||||
.env("GIT_CONFIG_GLOBAL", "/dev/null")
|
||||
.stdin(std::process::Stdio::piped())
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.stderr(std::process::Stdio::piped());
|
||||
for (key, value) in &env {
|
||||
command.env(key, value);
|
||||
}
|
||||
let mut child = command.spawn().expect("git credential fill should spawn");
|
||||
child
|
||||
.stdin
|
||||
.as_mut()
|
||||
.unwrap()
|
||||
.write_all(b"protocol=https\nhost=github.com\npath=fabro-sh/keystone\n\n")
|
||||
.unwrap();
|
||||
let output = child.wait_with_output().unwrap();
|
||||
|
||||
assert!(output.status.success(), "{output:?}");
|
||||
let filled = String::from_utf8_lossy(&output.stdout);
|
||||
assert!(filled.contains("username=x-access-token"), "{filled}");
|
||||
assert!(filled.contains("password=test-token-value"), "{filled}");
|
||||
}
|
||||
|
||||
/// Real Git applies the generated `insteadOf` rewrites: the exact SSH
|
||||
/// spellings of a declared repository resolve to their HTTPS-analog
|
||||
/// route (a local `file://` fixture here, so no network is involved),
|
||||
/// while `GIT_SSH_COMMAND=false` proves SSH is never attempted.
|
||||
#[test]
|
||||
fn declared_ssh_urls_rewrite_to_the_https_route() {
|
||||
let root = tempfile::tempdir().unwrap();
|
||||
let base = init_bare_fixture(root.path(), "fabro-sh/keystone");
|
||||
let keystone = slug("fabro-sh/keystone");
|
||||
|
||||
let mut env: HashMap<String, String> = HashMap::new();
|
||||
for (offset, (key, value)) in bridge_entries(&[&keystone], &base).into_iter().enumerate() {
|
||||
env.insert(format!("GIT_CONFIG_KEY_{offset}"), key);
|
||||
env.insert(format!("GIT_CONFIG_VALUE_{offset}"), value);
|
||||
}
|
||||
env.insert("GIT_CONFIG_COUNT".to_string(), "3".to_string());
|
||||
env.insert("GIT_SSH_COMMAND".to_string(), "false".to_string());
|
||||
|
||||
for url in [
|
||||
"ssh://git@github.com/fabro-sh/keystone.git",
|
||||
"ssh://git@github.com/fabro-sh/keystone",
|
||||
"git@github.com:fabro-sh/keystone.git",
|
||||
"git@github.com:fabro-sh/keystone",
|
||||
] {
|
||||
let output = git(&["ls-remote", url], &env, root.path());
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"{url} should rewrite to the fixture route: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// An undeclared SSH URL that shares no declared prefix is not
|
||||
/// rewritten: Git still routes it to SSH, where the scripted
|
||||
/// `GIT_SSH_COMMAND=false` fails immediately without network access.
|
||||
#[test]
|
||||
fn undeclared_ssh_urls_are_not_rewritten() {
|
||||
let root = tempfile::tempdir().unwrap();
|
||||
let base = init_bare_fixture(root.path(), "fabro-sh/keystone");
|
||||
let keystone = slug("fabro-sh/keystone");
|
||||
|
||||
let mut env: HashMap<String, String> = HashMap::new();
|
||||
for (offset, (key, value)) in bridge_entries(&[&keystone], &base).into_iter().enumerate() {
|
||||
env.insert(format!("GIT_CONFIG_KEY_{offset}"), key);
|
||||
env.insert(format!("GIT_CONFIG_VALUE_{offset}"), value);
|
||||
}
|
||||
env.insert("GIT_CONFIG_COUNT".to_string(), "3".to_string());
|
||||
env.insert("GIT_SSH_COMMAND".to_string(), "false".to_string());
|
||||
env.insert("GITHUB_TOKEN".to_string(), "test-token-value".to_string());
|
||||
|
||||
let output = git(
|
||||
&["ls-remote", "git@github.com:fabro-sh/undeclared"],
|
||||
&env,
|
||||
root.path(),
|
||||
);
|
||||
assert!(!output.status.success(), "{output:?}");
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
// Not rewritten: the failure never mentions the local HTTPS-analog
|
||||
// fixture route, so Git still chose the SSH transport.
|
||||
assert!(
|
||||
!stderr.contains(&root.path().display().to_string()),
|
||||
"undeclared URL must not be rewritten to the fixture route: {stderr}"
|
||||
);
|
||||
assert!(!stderr.contains("test-token-value"), "{stderr}");
|
||||
}
|
||||
|
||||
/// Prefix collision: with `fabro-sh/keystone` declared, both SSH
|
||||
/// spellings of `fabro-sh/keystone-other` are rewritten to the HTTPS
|
||||
/// route (prefix match), where access fails — at GitHub this is an
|
||||
/// authorization error for the scoped token — and no token leaks into
|
||||
/// the output.
|
||||
#[test]
|
||||
fn prefix_colliding_undeclared_repositories_rewrite_and_fail_without_token_leak() {
|
||||
let root = tempfile::tempdir().unwrap();
|
||||
let base = init_bare_fixture(root.path(), "fabro-sh/keystone");
|
||||
let keystone = slug("fabro-sh/keystone");
|
||||
|
||||
let mut env: HashMap<String, String> = HashMap::new();
|
||||
for (offset, (key, value)) in bridge_entries(&[&keystone], &base).into_iter().enumerate() {
|
||||
env.insert(format!("GIT_CONFIG_KEY_{offset}"), key);
|
||||
env.insert(format!("GIT_CONFIG_VALUE_{offset}"), value);
|
||||
}
|
||||
env.insert("GIT_CONFIG_COUNT".to_string(), "3".to_string());
|
||||
env.insert("GIT_SSH_COMMAND".to_string(), "false".to_string());
|
||||
env.insert("GITHUB_TOKEN".to_string(), "test-token-value".to_string());
|
||||
|
||||
for url in [
|
||||
"git@github.com:fabro-sh/keystone-other",
|
||||
"ssh://git@github.com/fabro-sh/keystone-other.git",
|
||||
] {
|
||||
let output = git(&["ls-remote", url], &env, root.path());
|
||||
assert!(!output.status.success(), "{url}: {output:?}");
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
// The failure names the (missing) HTTPS-analog fixture route,
|
||||
// proving the prefix rule rewrote the URL away from SSH.
|
||||
assert!(
|
||||
stderr.contains("keystone-other"),
|
||||
"{url} must be rewritten away from SSH, got: {stderr}"
|
||||
);
|
||||
assert!(
|
||||
stderr.contains(&root.path().display().to_string()),
|
||||
"{url} must land on the rewritten route, got: {stderr}"
|
||||
);
|
||||
assert!(!stderr.contains("test-token-value"), "{stderr}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -473,8 +473,9 @@ impl AgentAcpBackend {
|
|||
emitter.notice(
|
||||
RunNoticeLevel::Info,
|
||||
RunNoticeCode::GithubTokenRefreshLimited,
|
||||
"ACP agent stages receive workflow env at process launch; stages running beyond \
|
||||
token expiry may need to be retried.",
|
||||
"ACP agent stages receive workflow env at process launch; GITHUB_TOKEN access to \
|
||||
every declared repository expires together, so stages running beyond token \
|
||||
expiry may need to be retried.",
|
||||
);
|
||||
}
|
||||
provider
|
||||
|
|
|
|||
|
|
@ -293,6 +293,7 @@ pub mod error;
|
|||
pub mod event;
|
||||
pub mod file_resolver;
|
||||
pub mod git;
|
||||
pub(crate) mod git_bridge;
|
||||
pub(crate) mod graph;
|
||||
pub mod handler;
|
||||
mod hook_context;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
use std::collections::{HashMap, HashSet};
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::time::{Duration, Instant};
|
||||
|
|
@ -17,7 +17,7 @@ use fabro_sandbox::{DockerSandboxOptions, SandboxSpec};
|
|||
use fabro_static::EnvVars;
|
||||
use fabro_types::settings::run::{
|
||||
ApprovalMode, McpServerSettings as ResolvedMcpServerSettings, PullRequestSettings,
|
||||
ResolvedMcpEntry, RunMode, RunNamespace as ResolvedRunSettings,
|
||||
ResolvedGithubIntegration, ResolvedMcpEntry, RunMode, RunNamespace as ResolvedRunSettings,
|
||||
RunPrepareSettings as ResolvedRunPrepareSettings,
|
||||
};
|
||||
use fabro_types::{ManifestPath, RunId, RunRunnableSource, SandboxProviderKind};
|
||||
|
|
@ -104,9 +104,10 @@ pub struct StartServices {
|
|||
pub artifact_sink: Option<ArtifactSink>,
|
||||
pub run_control: Option<Arc<RunControlState>>,
|
||||
pub github_app: Option<fabro_github::GitHubCredentials>,
|
||||
/// Server-resolved GitHub integration permissions to inject into the
|
||||
/// sandbox env. Empty when github integration has no permissions.
|
||||
pub github_permissions: HashMap<String, String>,
|
||||
/// The resolved GitHub integration request (interpolated permissions
|
||||
/// plus declared additional repositories) to inject into the sandbox
|
||||
/// env. Empty when the github integration requests no token.
|
||||
pub github_integration: ResolvedGithubIntegration,
|
||||
pub vault: Arc<AsyncRwLock<Vault>>,
|
||||
pub catalog: Arc<Catalog>,
|
||||
pub on_node: crate::OnNodeCallback,
|
||||
|
|
@ -452,11 +453,13 @@ impl RunSession {
|
|||
.environment
|
||||
.resolve_env(secret_lookup)
|
||||
.map_err(|err| Error::engine_with_source("failed to resolve run environment", err))?;
|
||||
let github_permissions: Option<HashMap<String, String>> =
|
||||
(!services.github_permissions.is_empty()).then(|| services.github_permissions.clone());
|
||||
let github_integration = services
|
||||
.github_integration
|
||||
.is_token_requested()
|
||||
.then(|| services.github_integration.clone());
|
||||
let sandbox_env = SandboxEnvSpec {
|
||||
toml_env,
|
||||
github_permissions,
|
||||
github_integration,
|
||||
origin_url: record.repo_origin_url().map(str::to_string),
|
||||
};
|
||||
|
||||
|
|
@ -1725,7 +1728,7 @@ reasoning = false
|
|||
artifact_sink: None,
|
||||
run_control: None,
|
||||
github_app: None,
|
||||
github_permissions: HashMap::new(),
|
||||
github_integration: ResolvedGithubIntegration::default(),
|
||||
vault: Arc::new(AsyncRwLock::new(start_vault(&[]))),
|
||||
catalog: test_catalog(),
|
||||
on_node: None,
|
||||
|
|
|
|||
|
|
@ -287,7 +287,7 @@ async fn execute_test_run_with_options(
|
|||
hooks: HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
github_integration: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: auth_test_support::empty_vault(),
|
||||
|
|
@ -349,7 +349,7 @@ async fn execute_runs_start_to_exit_and_returns_final_context() {
|
|||
hooks: HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
github_integration: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: auth_test_support::empty_vault(),
|
||||
|
|
@ -488,7 +488,7 @@ async fn resumed_in_flight_node_starts_a_new_stage_execution() {
|
|||
hooks: HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
github_integration: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: auth_test_support::empty_vault(),
|
||||
|
|
@ -599,7 +599,7 @@ async fn run_with_lifecycle(
|
|||
hooks: HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
github_integration: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: auth_test_support::empty_vault(),
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ use super::types::{InitOptions, Initialized, LlmSpec, Persisted, SandboxEnvSpec}
|
|||
use crate::error::Error;
|
||||
use crate::event::{Event, RunNoticeCode, RunNoticeLevel};
|
||||
use crate::git::GitAuthor;
|
||||
use crate::git_bridge;
|
||||
use crate::handler::llm::{AgentAcpBackend, AgentApiBackend, BackendRouter, routing};
|
||||
use crate::handler::{HandlerRegistry, default_registry};
|
||||
#[cfg(test)]
|
||||
|
|
@ -37,10 +38,14 @@ use crate::services::{
|
|||
use crate::stage_execution::{StageExecutionSeed, StageExecutionTracker};
|
||||
use crate::steering_hub::SteeringHub;
|
||||
|
||||
type BuiltSandboxEnv = (
|
||||
HashMap<String, String>,
|
||||
Option<Arc<InstallationTokenSource>>,
|
||||
);
|
||||
struct BuiltSandboxEnv {
|
||||
env: HashMap<String, String>,
|
||||
github_token: Option<Arc<InstallationTokenSource>>,
|
||||
/// The validated effective repository set behind `github_token`.
|
||||
/// Present only in App mode or when additional repositories are
|
||||
/// declared; drives the eager access validation at initialization.
|
||||
github_access: Option<fabro_github::GitHubRepositoryAccess>,
|
||||
}
|
||||
|
||||
async fn run_hooks(
|
||||
hook_runner: Option<&HookRunner>,
|
||||
|
|
@ -91,41 +96,120 @@ fn build_sandbox_env(
|
|||
spec: &SandboxEnvSpec,
|
||||
github_app: Option<&fabro_github::GitHubCredentials>,
|
||||
) -> Result<BuiltSandboxEnv, Error> {
|
||||
let env = spec.toml_env.clone();
|
||||
let mut env = spec.toml_env.clone();
|
||||
|
||||
let Some(permissions) = spec.github_permissions.as_ref().filter(|p| !p.is_empty()) else {
|
||||
return Ok((env, None));
|
||||
let no_token = |env| BuiltSandboxEnv {
|
||||
env,
|
||||
github_token: None,
|
||||
github_access: None,
|
||||
};
|
||||
let Some(integration) = spec
|
||||
.github_integration
|
||||
.as_ref()
|
||||
.filter(|integration| integration.is_token_requested())
|
||||
else {
|
||||
return Ok(no_token(env));
|
||||
};
|
||||
let declares_additional = integration.has_additional_repositories();
|
||||
let Some(creds) = github_app else {
|
||||
return Ok((env, None));
|
||||
if declares_additional {
|
||||
// Legacy permissions-only configuration stays best-effort, but a
|
||||
// declared additional set is an explicit access requirement.
|
||||
return Err(Error::Precondition(
|
||||
"run.integrations.github.additional_repositories requires GitHub credentials, \
|
||||
but none are configured"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
return Ok(no_token(env));
|
||||
};
|
||||
|
||||
let source = match creds {
|
||||
// Validate the effective repository set whenever it matters: App mode
|
||||
// scopes the mint to it, and any declared additional set must hold its
|
||||
// invariants regardless of credential kind. Legacy PAT/static
|
||||
// permissions-only runs skip it to preserve their origin-agnostic
|
||||
// behavior.
|
||||
let github_access =
|
||||
if declares_additional || matches!(creds, fabro_github::GitHubCredentials::App(_)) {
|
||||
fabro_github::GitHubRepositoryAccess::new(
|
||||
spec.origin_url.as_deref(),
|
||||
&integration.additional_repositories,
|
||||
integration.permissions.clone(),
|
||||
)
|
||||
.map_err(|err| {
|
||||
Error::engine_with_anyhow("Failed to validate GitHub repository access", err)
|
||||
})?
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let github_token = match creds {
|
||||
fabro_github::GitHubCredentials::Pat(token) => {
|
||||
Some(InstallationTokenSource::pat(token.clone()))
|
||||
}
|
||||
fabro_github::GitHubCredentials::Installation(token) => {
|
||||
Some(InstallationTokenSource::installation(token.clone()))
|
||||
}
|
||||
fabro_github::GitHubCredentials::App(_) => {
|
||||
let Some(origin_url) = spec.origin_url.as_deref() else {
|
||||
return Ok((env, None));
|
||||
};
|
||||
let https_url = fabro_github::ssh_url_to_https(origin_url);
|
||||
let (owner, repo) = fabro_github::parse_github_owner_repo(&https_url)
|
||||
.map_err(|err| Error::engine_with_anyhow("Failed to parse GitHub origin", err))?;
|
||||
let permissions = serde_json::to_value(permissions).map_err(|err| {
|
||||
Error::engine_with_source("Failed to serialize GitHub permissions", err)
|
||||
})?;
|
||||
Some(
|
||||
InstallationTokenSource::for_repository(creds, owner, repo, permissions).map_err(
|
||||
|err| Error::engine_with_anyhow("Failed to build GitHub token source", err),
|
||||
)?,
|
||||
)
|
||||
}
|
||||
fabro_github::GitHubCredentials::App(_) => match github_access.as_ref() {
|
||||
Some(access) => Some(InstallationTokenSource::for_access(creds, access).map_err(
|
||||
|err| Error::engine_with_anyhow("Failed to build GitHub token source", err),
|
||||
)?),
|
||||
// No origin URL and nothing declared: keep the legacy
|
||||
// best-effort skip.
|
||||
None => None,
|
||||
},
|
||||
};
|
||||
|
||||
Ok((env, source))
|
||||
if declares_additional {
|
||||
let access = github_access
|
||||
.as_ref()
|
||||
.expect("access is always constructed when additional repositories are declared");
|
||||
git_bridge::merge_git_bridge_env(&mut env, &access.targets())?;
|
||||
}
|
||||
|
||||
Ok(BuiltSandboxEnv {
|
||||
env,
|
||||
github_token,
|
||||
github_access,
|
||||
})
|
||||
}
|
||||
|
||||
/// When additional repositories are declared, prove the whole effective set
|
||||
/// is reachable before the first workflow stage: resolve every repository's
|
||||
/// App installation (naming any repository the App cannot see), then resolve
|
||||
/// the token once eagerly. Legacy permissions-only runs skip this and keep
|
||||
/// their best-effort behavior.
|
||||
async fn validate_declared_repository_access(
|
||||
built: &BuiltSandboxEnv,
|
||||
github_app: Option<&fabro_github::GitHubCredentials>,
|
||||
) -> Result<(), Error> {
|
||||
let Some(access) = built
|
||||
.github_access
|
||||
.as_ref()
|
||||
.filter(|access| access.has_additional_repositories())
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
if let Some(fabro_github::GitHubCredentials::App(app)) = github_app {
|
||||
access
|
||||
.resolve_shared_installation_via_api(app)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
Error::engine_with_anyhow(
|
||||
"Declared additional GitHub repository is not accessible",
|
||||
err,
|
||||
)
|
||||
})?;
|
||||
}
|
||||
if let Some(source) = built.github_token.as_ref() {
|
||||
source.resolve().await.map_err(|err| {
|
||||
Error::engine_with_anyhow(
|
||||
"Failed to resolve GitHub access for the declared repository set",
|
||||
err,
|
||||
)
|
||||
})?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn build_registry(
|
||||
|
|
@ -443,10 +527,17 @@ pub async fn initialize(
|
|||
});
|
||||
}
|
||||
|
||||
let (base_env, github_token) = build_sandbox_env(
|
||||
let built_env = build_sandbox_env(
|
||||
&options.sandbox_env,
|
||||
options.run_options.github_app.as_ref(),
|
||||
)?;
|
||||
validate_declared_repository_access(&built_env, options.run_options.github_app.as_ref())
|
||||
.await?;
|
||||
let BuiltSandboxEnv {
|
||||
env: base_env,
|
||||
github_token,
|
||||
github_access: _,
|
||||
} = built_env;
|
||||
let tool_env_provider = Arc::new(WorkflowToolEnvProvider {
|
||||
base_env: base_env.clone(),
|
||||
github_token: github_token.clone(),
|
||||
|
|
@ -818,7 +909,7 @@ mod tests {
|
|||
hooks: fabro_hooks::HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
github_integration: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: auth_test_support::empty_vault(),
|
||||
|
|
@ -948,7 +1039,7 @@ mod tests {
|
|||
let initialized = initialize(persisted, InitOptions {
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::from([("TEST_KEY".to_string(), "value".to_string())]),
|
||||
github_permissions: None,
|
||||
github_integration: None,
|
||||
origin_url: None,
|
||||
},
|
||||
..test_init_options(
|
||||
|
|
@ -1271,7 +1362,7 @@ mod tests {
|
|||
hooks: fabro_hooks::HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
github_integration: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault,
|
||||
|
|
@ -1366,7 +1457,7 @@ mod tests {
|
|||
hooks: fabro_hooks::HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
github_integration: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: auth_test_support::empty_vault(),
|
||||
|
|
@ -1508,7 +1599,7 @@ mod tests {
|
|||
hooks: fabro_hooks::HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
github_integration: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: auth_test_support::empty_vault(),
|
||||
|
|
@ -1524,4 +1615,171 @@ mod tests {
|
|||
|
||||
assert!(matches!(result, Err(Error::Cancelled)));
|
||||
}
|
||||
|
||||
mod github_integration_env {
|
||||
//! Focused tests for `build_sandbox_env` /
|
||||
//! `validate_declared_repository_access` around declared additional
|
||||
//! repositories. Installation-resolution failure naming is covered
|
||||
//! by `fabro_github::access` tests; these prove the initialization
|
||||
//! wiring: hard errors for declared sets, best-effort behavior for
|
||||
//! legacy permissions-only configuration.
|
||||
|
||||
use fabro_github::test_support::{InstallationTokenMinter, installation_token_source};
|
||||
use fabro_github::{GitHubAppCredentials, GitHubCredentials, InstallationToken};
|
||||
use fabro_types::settings::run::ResolvedGithubIntegration;
|
||||
|
||||
use super::*;
|
||||
|
||||
fn integration(additional: &[&str]) -> ResolvedGithubIntegration {
|
||||
ResolvedGithubIntegration {
|
||||
permissions: HashMap::from([(
|
||||
"contents".to_string(),
|
||||
"read".to_string(),
|
||||
)]),
|
||||
additional_repositories: additional
|
||||
.iter()
|
||||
.map(|value| value.parse().expect("test slug should parse"))
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
fn spec(
|
||||
origin: Option<&str>,
|
||||
github_integration: Option<ResolvedGithubIntegration>,
|
||||
) -> SandboxEnvSpec {
|
||||
SandboxEnvSpec {
|
||||
toml_env: HashMap::new(),
|
||||
github_integration,
|
||||
origin_url: origin.map(str::to_string),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn declared_additional_repositories_require_credentials() {
|
||||
let spec = spec(
|
||||
Some("https://github.com/fabro-sh/fabro"),
|
||||
Some(integration(&["fabro-sh/keystone"])),
|
||||
);
|
||||
let Err(err) = build_sandbox_env(&spec, None) else {
|
||||
panic!("declared additional repositories without credentials must fail");
|
||||
};
|
||||
assert!(
|
||||
err.to_string().contains("requires GitHub credentials"),
|
||||
"{err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn declared_additional_repositories_require_an_origin() {
|
||||
let spec = spec(None, Some(integration(&["fabro-sh/keystone"])));
|
||||
let creds = GitHubCredentials::Pat("ghp_x".to_string());
|
||||
let Err(err) = build_sandbox_env(&spec, Some(&creds)) else {
|
||||
panic!("declared additional repositories without an origin must fail");
|
||||
};
|
||||
assert!(
|
||||
err.to_string().contains("GitHub repository access"),
|
||||
"{err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn declared_repositories_inject_bridge_entries_and_keep_the_pat_source() {
|
||||
let spec = spec(
|
||||
Some("https://github.com/fabro-sh/fabro"),
|
||||
Some(integration(&["fabro-sh/keystone"])),
|
||||
);
|
||||
let creds = GitHubCredentials::Pat("ghp_x".to_string());
|
||||
let built = build_sandbox_env(&spec, Some(&creds)).unwrap();
|
||||
|
||||
assert!(built.github_token.is_some());
|
||||
let access = built.github_access.expect("access should be constructed");
|
||||
assert!(access.has_additional_repositories());
|
||||
// Helper entry plus two SSH rewrites for each of the two
|
||||
// effective repositories (origin + declared additional).
|
||||
assert_eq!(
|
||||
built.env.get("GIT_CONFIG_COUNT").map(String::as_str),
|
||||
Some("5")
|
||||
);
|
||||
assert_eq!(
|
||||
built.env.get("GIT_CONFIG_KEY_0").map(String::as_str),
|
||||
Some("credential.https://github.com.helper")
|
||||
);
|
||||
assert_eq!(
|
||||
built.env.get("GIT_TERMINAL_PROMPT").map(String::as_str),
|
||||
Some("0")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_permissions_only_configuration_stays_best_effort() {
|
||||
// No credentials: no error, no token source, no bridge entries.
|
||||
let no_creds = spec(
|
||||
Some("https://github.com/fabro-sh/fabro"),
|
||||
Some(integration(&[])),
|
||||
);
|
||||
let built = build_sandbox_env(&no_creds, None).unwrap();
|
||||
assert!(built.github_token.is_none());
|
||||
assert!(!built.env.contains_key("GIT_CONFIG_COUNT"));
|
||||
|
||||
// App credentials without an origin: legacy best-effort skip.
|
||||
let creds = GitHubCredentials::App(GitHubAppCredentials {
|
||||
app_id: "1".to_string(),
|
||||
private_key_pem: "unused".to_string(),
|
||||
slug: None,
|
||||
});
|
||||
let no_origin = spec(None, Some(integration(&[])));
|
||||
let built = build_sandbox_env(&no_origin, Some(&creds)).unwrap();
|
||||
assert!(built.github_token.is_none());
|
||||
assert!(built.github_access.is_none());
|
||||
}
|
||||
|
||||
struct FailingMinter;
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl InstallationTokenMinter for FailingMinter {
|
||||
async fn mint(&self) -> anyhow::Result<InstallationToken> {
|
||||
Err(anyhow::anyhow!("scripted mint failure"))
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn eager_validation_fails_when_the_declared_token_cannot_resolve() {
|
||||
let access = fabro_github::GitHubRepositoryAccess::new(
|
||||
Some("https://github.com/fabro-sh/fabro"),
|
||||
&["fabro-sh/keystone".parse().unwrap()].into_iter().collect(),
|
||||
HashMap::from([("contents".to_string(), "read".to_string())]),
|
||||
)
|
||||
.unwrap();
|
||||
let built = BuiltSandboxEnv {
|
||||
env: HashMap::new(),
|
||||
github_token: Some(installation_token_source(
|
||||
"fabro-sh/fabro (+1 additional)",
|
||||
Arc::new(FailingMinter),
|
||||
)),
|
||||
github_access: access,
|
||||
};
|
||||
|
||||
let err = validate_declared_repository_access(&built, None)
|
||||
.await
|
||||
.unwrap_err();
|
||||
let message = err.to_string();
|
||||
assert!(message.contains("declared repository set"), "{message}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn eager_validation_skips_legacy_permissions_only_runs() {
|
||||
let built = BuiltSandboxEnv {
|
||||
env: HashMap::new(),
|
||||
github_token: Some(installation_token_source(
|
||||
"fabro-sh/fabro",
|
||||
Arc::new(FailingMinter),
|
||||
)),
|
||||
github_access: None,
|
||||
};
|
||||
|
||||
validate_declared_repository_access(&built, None)
|
||||
.await
|
||||
.expect("legacy permissions-only runs must not resolve eagerly");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,7 +8,9 @@ use fabro_mcp::config::McpServerSettings;
|
|||
use fabro_model::{Catalog, ProviderId};
|
||||
use fabro_sandbox::SandboxSpec;
|
||||
use fabro_template::TemplateContext;
|
||||
use fabro_types::settings::run::{PullRequestSettings, RunModelControls};
|
||||
use fabro_types::settings::run::{
|
||||
PullRequestSettings, ResolvedGithubIntegration, RunModelControls,
|
||||
};
|
||||
use fabro_types::{ManifestPath, RunId, RunProjection};
|
||||
use fabro_validate::{Diagnostic, Severity};
|
||||
use fabro_vault::Vault;
|
||||
|
|
@ -246,7 +248,10 @@ pub struct LlmSpec {
|
|||
#[derive(Clone)]
|
||||
pub struct SandboxEnvSpec {
|
||||
pub toml_env: HashMap<String, String>,
|
||||
pub github_permissions: Option<HashMap<String, String>>,
|
||||
/// The resolved GitHub integration request (interpolated permissions
|
||||
/// plus declared additional repositories). `None` when the run requests
|
||||
/// no `GITHUB_TOKEN`.
|
||||
pub github_integration: Option<ResolvedGithubIntegration>,
|
||||
pub origin_url: Option<String>,
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue