mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
Tighten automation Git validation types
This commit is contained in:
parent
5af791c812
commit
7a3f58c87e
12 changed files with 303 additions and 172 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -2383,7 +2383,6 @@ dependencies = [
|
|||
"serde_json",
|
||||
"sha2 0.10.9",
|
||||
"sqlx",
|
||||
"strum 0.28.0",
|
||||
"tempfile",
|
||||
"thiserror 2.0.18",
|
||||
"tokio",
|
||||
|
|
|
|||
|
|
@ -2,20 +2,18 @@ use std::path::{Path, PathBuf};
|
|||
use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use fabro_automation::{
|
||||
AutomationGitWorkflowSource, AutomationId, AutomationValidationError, validate_workflow_source,
|
||||
};
|
||||
use fabro_automation::{AutomationGitWorkflowSource, AutomationId};
|
||||
use fabro_manifest::WorkflowVersionCollectError;
|
||||
use fabro_types::{
|
||||
GitHubRepositorySlug, GitRunTarget, ResolvedAutomationGitWorkflowSource, RunId, RunIntent,
|
||||
RunIntentArgs, RunTarget, TargetValidationError, WorkflowVersionId,
|
||||
GitCoordinateValidationError, GitHubRepositorySlug, GitRunTarget,
|
||||
ResolvedAutomationGitWorkflowSource, RunId, RunIntent, RunIntentArgs, RunTarget,
|
||||
WorkflowVersionId,
|
||||
};
|
||||
use fabro_workflow_version::{WorkflowVersionStore, WorkflowVersionStoreError};
|
||||
use tokio::{fs, task};
|
||||
|
||||
use crate::git_checkout::{
|
||||
GitAuthConfig, GitCheckoutError, GitCheckoutSelector, GitRepoCache, WorktreePrepareInput,
|
||||
github_clone_url, resolve_git_read_auth_config,
|
||||
self, GitAuthConfig, GitCheckoutError, GitCheckoutSelector, GitRepoCache, WorktreePrepareInput,
|
||||
};
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
|
|
@ -56,12 +54,12 @@ pub(crate) enum RunMaterializeError {
|
|||
#[error("invalid automation Git target")]
|
||||
InvalidTarget {
|
||||
#[source]
|
||||
source: TargetValidationError,
|
||||
source: GitCoordinateValidationError,
|
||||
},
|
||||
#[error("invalid automation workflow source")]
|
||||
InvalidWorkflowSource {
|
||||
#[source]
|
||||
source: AutomationValidationError,
|
||||
source: GitCoordinateValidationError,
|
||||
},
|
||||
#[error("failed to resolve automation {role} credentials")]
|
||||
Credentials {
|
||||
|
|
@ -153,7 +151,7 @@ struct ServerGitHubRemoteResolver {
|
|||
#[async_trait]
|
||||
impl AutomationGitRemoteResolver for ServerGitHubRemoteResolver {
|
||||
async fn resolve(&self, repo: &GitHubRepositorySlug) -> anyhow::Result<GitRemote> {
|
||||
let auth = resolve_git_read_auth_config(
|
||||
let auth = git_checkout::resolve_git_read_auth_config(
|
||||
self.credentials.as_ref(),
|
||||
repo,
|
||||
&self.api_base_url,
|
||||
|
|
@ -161,7 +159,7 @@ impl AutomationGitRemoteResolver for ServerGitHubRemoteResolver {
|
|||
)
|
||||
.await?;
|
||||
Ok(GitRemote {
|
||||
clone_url: github_clone_url(repo),
|
||||
clone_url: git_checkout::github_clone_url(repo),
|
||||
auth,
|
||||
})
|
||||
}
|
||||
|
|
@ -232,45 +230,24 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer {
|
|||
&self,
|
||||
input: AutomationRunMaterializeInput,
|
||||
) -> Result<AutomationRunMaterialized, RunMaterializeError> {
|
||||
let validated_target = RunTarget::Git(input.target)
|
||||
let validated_target = input
|
||||
.target
|
||||
.validate()
|
||||
.map_err(|source| RunMaterializeError::InvalidTarget { source })?;
|
||||
let RunTarget::Git(mut exact_target) = validated_target.target else {
|
||||
unreachable!("a validated Git target remains Git-backed");
|
||||
};
|
||||
let target_repo: GitHubRepositorySlug =
|
||||
exact_target
|
||||
.repo
|
||||
.parse()
|
||||
.map_err(|_| RunMaterializeError::InvalidTarget {
|
||||
source: TargetValidationError::Repository,
|
||||
})?;
|
||||
let target_repo = validated_target.repository().clone();
|
||||
let mut exact_target = validated_target.into_target();
|
||||
let workflow_source = input
|
||||
.workflow_source
|
||||
.map(validate_workflow_source)
|
||||
.map(GitRunTarget::validate)
|
||||
.transpose()
|
||||
.map_err(|source| RunMaterializeError::InvalidWorkflowSource { source })?;
|
||||
// A workflow source naming the target's exact coordinate shares its
|
||||
// checkout; anything else needs a second worktree.
|
||||
let separate_source = workflow_source
|
||||
.as_ref()
|
||||
.map(|source| {
|
||||
source
|
||||
.repo
|
||||
.parse::<GitHubRepositorySlug>()
|
||||
.map(|repo| (repo, source))
|
||||
.map_err(|_| RunMaterializeError::InvalidWorkflowSource {
|
||||
source: AutomationValidationError::InvalidWorkflowSource {
|
||||
source: TargetValidationError::Repository,
|
||||
},
|
||||
})
|
||||
})
|
||||
.transpose()?
|
||||
.filter(|(repo, source)| {
|
||||
*repo != target_repo
|
||||
|| GitCheckoutSelector::from(*source)
|
||||
!= GitCheckoutSelector::from(&exact_target)
|
||||
});
|
||||
let separate_source = workflow_source.as_ref().filter(|source| {
|
||||
source.repository() != &target_repo
|
||||
|| GitCheckoutSelector::from(source.target())
|
||||
!= GitCheckoutSelector::from(&exact_target)
|
||||
});
|
||||
|
||||
fs::create_dir_all(&input.temp_root)
|
||||
.await
|
||||
|
|
@ -304,20 +281,21 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer {
|
|||
.await?;
|
||||
let (workflow_checkout_dir, workflow_checkout_sha) = match separate_source {
|
||||
None => (target_checkout_dir, checked_out_sha.clone()),
|
||||
Some((repo, source)) => {
|
||||
let remote = if repo == target_repo {
|
||||
Some(source) => {
|
||||
let repo = source.repository();
|
||||
let remote = if repo == &target_repo {
|
||||
target_remote
|
||||
} else {
|
||||
self.resolve_remote(CheckoutRole::WorkflowSource, &repo)
|
||||
self.resolve_remote(CheckoutRole::WorkflowSource, repo)
|
||||
.await?
|
||||
};
|
||||
let source_checkout_dir = temp_dir.path().join("workflow-source");
|
||||
let source_sha = self
|
||||
.prepare_checkout(
|
||||
CheckoutRole::WorkflowSource,
|
||||
&repo,
|
||||
repo,
|
||||
&remote,
|
||||
GitCheckoutSelector::from(source),
|
||||
GitCheckoutSelector::from(source.target()),
|
||||
&source_checkout_dir,
|
||||
)
|
||||
.await?;
|
||||
|
|
@ -327,7 +305,7 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer {
|
|||
exact_target.sha = Some(checked_out_sha);
|
||||
let resolved_workflow_source = workflow_source.map(|source| {
|
||||
Box::new(ResolvedAutomationGitWorkflowSource::from_requested(
|
||||
source,
|
||||
source.into_target(),
|
||||
workflow_checkout_sha,
|
||||
))
|
||||
});
|
||||
|
|
@ -385,10 +363,8 @@ struct TestAutomationRunMaterializerState {
|
|||
#[cfg(any(test, feature = "test-support"))]
|
||||
#[derive(Clone)]
|
||||
enum TestMaterializeFailure {
|
||||
InvalidTarget(TargetValidationError),
|
||||
/// Unit because `AutomationValidationError` is not `Clone`; any variant
|
||||
/// exercises the same handler path.
|
||||
InvalidWorkflowSource,
|
||||
InvalidTarget(GitCoordinateValidationError),
|
||||
InvalidWorkflowSource(GitCoordinateValidationError),
|
||||
}
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
|
|
@ -396,11 +372,9 @@ impl From<TestMaterializeFailure> for RunMaterializeError {
|
|||
fn from(failure: TestMaterializeFailure) -> Self {
|
||||
match failure {
|
||||
TestMaterializeFailure::InvalidTarget(source) => Self::InvalidTarget { source },
|
||||
TestMaterializeFailure::InvalidWorkflowSource => Self::InvalidWorkflowSource {
|
||||
source: AutomationValidationError::InvalidWorkflowSource {
|
||||
source: TargetValidationError::Branch,
|
||||
},
|
||||
},
|
||||
TestMaterializeFailure::InvalidWorkflowSource(source) => {
|
||||
Self::InvalidWorkflowSource { source }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -433,12 +407,14 @@ impl TestAutomationRunMaterializer {
|
|||
|
||||
pub fn fail_invalid_target() -> Self {
|
||||
Self::new(Err(TestMaterializeFailure::InvalidTarget(
|
||||
TargetValidationError::Repository,
|
||||
GitCoordinateValidationError::Repository,
|
||||
)))
|
||||
}
|
||||
|
||||
pub fn fail_invalid_workflow_source() -> Self {
|
||||
Self::new(Err(TestMaterializeFailure::InvalidWorkflowSource))
|
||||
Self::new(Err(TestMaterializeFailure::InvalidWorkflowSource(
|
||||
GitCoordinateValidationError::Branch,
|
||||
)))
|
||||
}
|
||||
|
||||
fn new(response: Result<Box<TestMaterializedWorkflow>, TestMaterializeFailure>) -> Self {
|
||||
|
|
@ -622,10 +598,7 @@ mod tests {
|
|||
.clone();
|
||||
Ok(GitRemote {
|
||||
clone_url,
|
||||
auth: Some(GitAuthConfig::new(
|
||||
Some("x-access-token".to_string()),
|
||||
Some(FAKE_TOKEN.to_string()),
|
||||
)),
|
||||
auth: Some(GitAuthConfig::from_parts("x-access-token", FAKE_TOKEN)),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -817,6 +790,39 @@ mod tests {
|
|||
}))
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn coordinate_validation_errors_have_role_specific_nonduplicated_chains() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
let materializer = production_materializer(
|
||||
temp.path(),
|
||||
test_version_store(),
|
||||
Arc::new(RecordingCredentialResolver::succeeds()),
|
||||
HashMap::new(),
|
||||
);
|
||||
|
||||
let mut invalid_target =
|
||||
input("fabro-sh/target", None, &temp.path().join("invalid-target"));
|
||||
invalid_target.target.branch = "refs/heads/main".to_string();
|
||||
let error = materializer.materialize(invalid_target).await.unwrap_err();
|
||||
assert_eq!(fabro_util::error::collect_chain(&error), [
|
||||
"invalid automation Git target",
|
||||
"branch must be a non-empty branch name, not a ref or commit selector",
|
||||
]);
|
||||
|
||||
let error = materializer
|
||||
.materialize(input(
|
||||
"fabro-sh/target",
|
||||
Some(source("fabro-sh/workflows", "refs/heads/main", None, None)),
|
||||
&temp.path().join("invalid-source"),
|
||||
))
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert_eq!(fabro_util::error::collect_chain(&error), [
|
||||
"invalid automation workflow source",
|
||||
"branch must be a non-empty branch name, not a ref or commit selector",
|
||||
]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn collected_closure_stores_dependency_first_and_idempotently() {
|
||||
let temp = TempDir::new().unwrap();
|
||||
|
|
|
|||
|
|
@ -270,40 +270,36 @@ pub(crate) fn github_clone_url(repo: &GitHubRepositorySlug) -> String {
|
|||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub(crate) struct GitAuthConfig {
|
||||
extraheader: Option<String>,
|
||||
extraheader: String,
|
||||
sensitive_values: Vec<String>,
|
||||
}
|
||||
|
||||
impl GitAuthConfig {
|
||||
pub(crate) fn new(username: Option<String>, password: Option<String>) -> Self {
|
||||
let Some(password) = password.filter(|value| !value.is_empty()) else {
|
||||
return Self {
|
||||
extraheader: None,
|
||||
sensitive_values: Vec::new(),
|
||||
};
|
||||
};
|
||||
let username = username
|
||||
.filter(|value| !value.is_empty())
|
||||
.unwrap_or_else(|| "x-access-token".to_string());
|
||||
pub(crate) fn new(credentials: &fabro_github::GitCloneCredentials) -> Self {
|
||||
Self::from_parts(credentials.username(), credentials.password())
|
||||
}
|
||||
|
||||
pub(crate) fn from_parts(username: &str, password: &str) -> Self {
|
||||
let encoded_credentials = BASE64_STANDARD.encode(format!("{username}:{password}"));
|
||||
let extraheader = basic_auth_header_from_encoded(&encoded_credentials);
|
||||
Self {
|
||||
sensitive_values: vec![password, encoded_credentials, extraheader.clone()],
|
||||
extraheader: Some(extraheader),
|
||||
sensitive_values: vec![
|
||||
password.to_string(),
|
||||
encoded_credentials,
|
||||
extraheader.clone(),
|
||||
],
|
||||
extraheader,
|
||||
}
|
||||
}
|
||||
|
||||
fn git_env(&self, clone_url: &str) -> Vec<(String, String)> {
|
||||
let Some(extraheader) = self.extraheader.as_ref() else {
|
||||
return Vec::new();
|
||||
};
|
||||
vec![
|
||||
("GIT_CONFIG_COUNT".to_string(), "1".to_string()),
|
||||
(
|
||||
"GIT_CONFIG_KEY_0".to_string(),
|
||||
format!("http.{clone_url}.extraheader"),
|
||||
),
|
||||
("GIT_CONFIG_VALUE_0".to_string(), extraheader.clone()),
|
||||
("GIT_CONFIG_VALUE_0".to_string(), self.extraheader.clone()),
|
||||
]
|
||||
}
|
||||
|
||||
|
|
@ -327,10 +323,10 @@ pub(crate) async fn resolve_git_read_auth_config(
|
|||
}
|
||||
None => fabro_github::GitHubContext::new(credentials, github_api_base_url),
|
||||
};
|
||||
let (username, password) =
|
||||
let credentials =
|
||||
fabro_github::resolve_read_only_clone_credentials(&context, repo.owner(), repo.repo())
|
||||
.await?;
|
||||
Ok(Some(GitAuthConfig::new(username, password)))
|
||||
Ok(Some(GitAuthConfig::new(&credentials)))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
@ -748,10 +744,7 @@ mod tests {
|
|||
fn credential_config_env_keeps_clone_url_uncredentialed() {
|
||||
let repo = repository_slug("fabro-sh/fabro");
|
||||
let clone_url = github_clone_url(&repo);
|
||||
let auth = GitAuthConfig::new(
|
||||
Some("x-access-token".to_string()),
|
||||
Some("ghu_secret".to_string()),
|
||||
);
|
||||
let auth = GitAuthConfig::from_parts("x-access-token", "ghu_secret");
|
||||
let plan = build_bare_clone_plan(&clone_url, Path::new("/tmp/fabro-checkout"), Some(&auth));
|
||||
|
||||
assert!(
|
||||
|
|
|
|||
|
|
@ -21,7 +21,6 @@ hex.workspace = true
|
|||
serde.workspace = true
|
||||
sha2.workspace = true
|
||||
sqlx.workspace = true
|
||||
strum.workspace = true
|
||||
thiserror.workspace = true
|
||||
tokio.workspace = true
|
||||
toml.workspace = true
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
use std::path::PathBuf;
|
||||
|
||||
use croner::errors::CronError;
|
||||
use fabro_types::TargetValidationError;
|
||||
use fabro_types::{GitCoordinateValidationError, TargetValidationError};
|
||||
use toml::de::Error as TomlDeError;
|
||||
use toml::ser::Error as TomlSerError;
|
||||
|
||||
|
|
@ -27,7 +27,7 @@ pub enum AutomationValidationError {
|
|||
#[error("automation workflow source is invalid")]
|
||||
InvalidWorkflowSource {
|
||||
#[source]
|
||||
source: TargetValidationError,
|
||||
source: GitCoordinateValidationError,
|
||||
},
|
||||
#[error("workflow selector {value:?} is not safe")]
|
||||
InvalidWorkflowSelector { value: String },
|
||||
|
|
|
|||
|
|
@ -157,17 +157,17 @@ pub type AutomationGitWorkflowSource = GitRunTarget;
|
|||
|
||||
/// Validate and canonicalize a saved workflow source without resolving remote
|
||||
/// repository state.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Returns an error when the repository slug, branch, tag, or exact commit
|
||||
/// does not use the canonical Git-coordinate grammar.
|
||||
pub fn validate_workflow_source(
|
||||
source: AutomationGitWorkflowSource,
|
||||
) -> Result<AutomationGitWorkflowSource, AutomationValidationError> {
|
||||
RunTarget::Git(source)
|
||||
source
|
||||
.validate()
|
||||
.map(|validated| match validated.target {
|
||||
RunTarget::Git(source) => source,
|
||||
RunTarget::None {} | RunTarget::Folder { .. } => {
|
||||
unreachable!("a validated Git workflow source remains Git-backed")
|
||||
}
|
||||
})
|
||||
.map(fabro_types::ValidatedGitRunTarget::into_target)
|
||||
.map_err(|source| AutomationValidationError::InvalidWorkflowSource { source })
|
||||
}
|
||||
|
||||
|
|
@ -470,7 +470,9 @@ fn validate_triggers(triggers: &[AutomationTrigger]) -> Result<(), AutomationVal
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use fabro_types::{GitRunTarget, RunTarget, TargetValidationError};
|
||||
use fabro_types::{
|
||||
GitCoordinateValidationError, GitRunTarget, RunTarget, TargetValidationError,
|
||||
};
|
||||
|
||||
use crate::{
|
||||
ApiTrigger, Automation, AutomationGitWorkflowSource, AutomationId, AutomationReplace,
|
||||
|
|
@ -634,24 +636,24 @@ mod tests {
|
|||
let cases = [
|
||||
(
|
||||
workflow_source("main", None, None),
|
||||
TargetValidationError::Repository,
|
||||
GitCoordinateValidationError::Repository,
|
||||
),
|
||||
(
|
||||
workflow_source("refs/heads/main", None, None),
|
||||
TargetValidationError::Branch,
|
||||
GitCoordinateValidationError::Branch,
|
||||
),
|
||||
(
|
||||
workflow_source("main", Some("tags/v1"), None),
|
||||
TargetValidationError::Tag,
|
||||
GitCoordinateValidationError::Tag,
|
||||
),
|
||||
(
|
||||
workflow_source("main", None, Some("short")),
|
||||
TargetValidationError::Sha,
|
||||
GitCoordinateValidationError::Sha,
|
||||
),
|
||||
];
|
||||
|
||||
for (mut source, expected) in cases {
|
||||
if expected == TargetValidationError::Repository {
|
||||
if expected == GitCoordinateValidationError::Repository {
|
||||
source.repo = "not/a/github/slug".to_string();
|
||||
}
|
||||
let error = Automation::from_replace(
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ use fabro_types::settings::run::MergeStrategy;
|
|||
use serde::Deserialize;
|
||||
use tokio::process::Command;
|
||||
|
||||
use crate::token_source::SecretString;
|
||||
|
||||
pub mod access;
|
||||
pub mod token_source;
|
||||
|
||||
|
|
@ -1266,16 +1268,57 @@ pub async fn update_app_webhook_config(
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Resolve git clone credentials for a GitHub repository.
|
||||
/// Required credentials for an authenticated GitHub HTTPS clone.
|
||||
///
|
||||
/// Returns `(username, password)` for authenticated cloning and pushing.
|
||||
/// Always generates a token regardless of repo visibility, since the token
|
||||
/// is needed for pushing from the sandbox.
|
||||
/// The password is redacted from `Debug` output and should only be exposed at
|
||||
/// the point where it is passed to Git.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct GitCloneCredentials {
|
||||
username: String,
|
||||
password: SecretString,
|
||||
}
|
||||
|
||||
impl GitCloneCredentials {
|
||||
fn from_token(token: String) -> anyhow::Result<Self> {
|
||||
if token.is_empty() {
|
||||
bail!("GitHub clone credential token is empty");
|
||||
}
|
||||
Ok(Self {
|
||||
username: "x-access-token".to_string(),
|
||||
password: SecretString::new(token),
|
||||
})
|
||||
}
|
||||
|
||||
/// The username passed to Git's HTTPS basic authentication.
|
||||
#[must_use]
|
||||
pub fn username(&self) -> &str {
|
||||
&self.username
|
||||
}
|
||||
|
||||
/// The secret passed to Git's HTTPS basic authentication.
|
||||
///
|
||||
/// Callers must not log or persist the returned value.
|
||||
#[must_use]
|
||||
pub fn password(&self) -> &str {
|
||||
self.password.expose()
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve Git clone credentials for a GitHub repository.
|
||||
///
|
||||
/// Always generates credentials regardless of repository visibility because
|
||||
/// the token is needed for pushing from the sandbox.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Returns an error when an installation token is expired, a GitHub App token
|
||||
/// cannot be minted, the HTTP client cannot be created, or the resolved token
|
||||
/// is empty.
|
||||
pub async fn resolve_clone_credentials(
|
||||
ctx: &GitHubContext<'_>,
|
||||
owner: &str,
|
||||
repo: &str,
|
||||
) -> anyhow::Result<(Option<String>, Option<String>)> {
|
||||
) -> anyhow::Result<GitCloneCredentials> {
|
||||
let token = match ctx.creds {
|
||||
GitHubCredentials::Pat(token) => token.clone(),
|
||||
GitHubCredentials::Installation(token) => token.valid_token()?.to_string(),
|
||||
|
|
@ -1291,7 +1334,7 @@ pub async fn resolve_clone_credentials(
|
|||
.await?
|
||||
}
|
||||
};
|
||||
Ok((Some("x-access-token".to_string()), Some(token)))
|
||||
GitCloneCredentials::from_token(token)
|
||||
}
|
||||
|
||||
/// Resolve credentials for fetching repository contents without granting a
|
||||
|
|
@ -1300,11 +1343,17 @@ pub async fn resolve_clone_credentials(
|
|||
/// Static PATs and pre-minted installation tokens retain their configured
|
||||
/// permissions. App credentials mint a repository-scoped token with
|
||||
/// `contents: read`.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Returns an error when an installation token is expired, a GitHub App token
|
||||
/// cannot be minted, the HTTP client cannot be created, or the resolved token
|
||||
/// is empty.
|
||||
pub async fn resolve_read_only_clone_credentials(
|
||||
ctx: &GitHubContext<'_>,
|
||||
owner: &str,
|
||||
repo: &str,
|
||||
) -> anyhow::Result<(Option<String>, Option<String>)> {
|
||||
) -> anyhow::Result<GitCloneCredentials> {
|
||||
let token = match ctx.creds {
|
||||
GitHubCredentials::Pat(token) => token.clone(),
|
||||
GitHubCredentials::Installation(token) => token.valid_token()?.to_string(),
|
||||
|
|
@ -1320,7 +1369,7 @@ pub async fn resolve_read_only_clone_credentials(
|
|||
.await?
|
||||
}
|
||||
};
|
||||
Ok((Some("x-access-token".to_string()), Some(token)))
|
||||
GitCloneCredentials::from_token(token)
|
||||
}
|
||||
|
||||
async fn mint_git_token(
|
||||
|
|
@ -1360,11 +1409,8 @@ pub async fn resolve_authenticated_url(
|
|||
url: &str,
|
||||
) -> anyhow::Result<DisplaySafeUrl> {
|
||||
let (owner, repo) = parse_github_owner_repo(url)?;
|
||||
let (_username, password) = resolve_clone_credentials(ctx, &owner, &repo).await?;
|
||||
match password {
|
||||
Some(token) => embed_token_in_url(url, &token),
|
||||
None => DisplaySafeUrl::parse(url).context("Failed to parse GitHub HTTPS URL"),
|
||||
}
|
||||
let credentials = resolve_clone_credentials(ctx, &owner, &repo).await?;
|
||||
embed_token_in_url(url, credentials.password())
|
||||
}
|
||||
|
||||
/// Fetch detailed information about a pull request.
|
||||
|
|
@ -2568,13 +2614,9 @@ mod tests {
|
|||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
credentials,
|
||||
(
|
||||
Some("x-access-token".to_string()),
|
||||
Some("ghu_test".to_string())
|
||||
)
|
||||
);
|
||||
assert_eq!(credentials.username(), "x-access-token");
|
||||
assert_eq!(credentials.password(), "ghu_test");
|
||||
assert!(!format!("{credentials:?}").contains("ghu_test"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
@ -2586,13 +2628,8 @@ mod tests {
|
|||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
credentials,
|
||||
(
|
||||
Some("x-access-token".to_string()),
|
||||
Some("ghu_test".to_string())
|
||||
)
|
||||
);
|
||||
assert_eq!(credentials.username(), "x-access-token");
|
||||
assert_eq!(credentials.password(), "ghu_test");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
|
|||
|
|
@ -1195,7 +1195,7 @@ async fn daytona_clone_public_repo_gets_credentials() {
|
|||
|
||||
// Directly test resolve_clone_credentials against a repo in an org where the
|
||||
// app is installed
|
||||
let (username, password) = fabro_github::resolve_clone_credentials(
|
||||
let credentials = fabro_github::resolve_clone_credentials(
|
||||
&fabro_github::GitHubContext::new(&creds, &fabro_github::github_api_base_url()),
|
||||
"fabro-sh",
|
||||
"fabro",
|
||||
|
|
@ -1204,12 +1204,12 @@ async fn daytona_clone_public_repo_gets_credentials() {
|
|||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
username.as_deref(),
|
||||
Some("x-access-token"),
|
||||
credentials.username(),
|
||||
"x-access-token",
|
||||
"installed org repo should get credentials for pushing"
|
||||
);
|
||||
assert!(
|
||||
password.is_some(),
|
||||
!credentials.password().is_empty(),
|
||||
"installed org repo should get a token for pushing"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@ use fabro_api::types::{
|
|||
};
|
||||
use fabro_automation::{
|
||||
Automation, AutomationDraft, AutomationGitWorkflowSource, AutomationReplace, AutomationTrigger,
|
||||
validate_workflow_source,
|
||||
};
|
||||
use serde_json::json;
|
||||
|
||||
|
|
@ -160,5 +159,5 @@ fn automation_workflow_source_rejects_unknown_or_incomplete_coordinates() {
|
|||
"sha": "short"
|
||||
}))
|
||||
.unwrap();
|
||||
assert!(validate_workflow_source(invalid_commit).is_err());
|
||||
assert!(fabro_automation::validate_workflow_source(invalid_commit).is_err());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -132,7 +132,8 @@ pub use run_event::{
|
|||
pub use run_failure::RunFailure;
|
||||
pub use run_id::{RunId, fixtures};
|
||||
pub use run_intent::{
|
||||
GitRunTarget, RunIntent, RunIntentArgs, RunTarget, TargetValidationError, ValidatedRunTarget,
|
||||
GitCoordinateValidationError, GitRunTarget, RunIntent, RunIntentArgs, RunTarget,
|
||||
TargetValidationError, ValidatedGitRunTarget, ValidatedRunTarget,
|
||||
};
|
||||
pub use run_projection::{
|
||||
ActivatedSkill, AgentControlState, CheckpointRecord, McpServerProjection, McpServerStatus,
|
||||
|
|
|
|||
|
|
@ -66,6 +66,56 @@ pub struct GitRunTarget {
|
|||
pub sha: Option<String>,
|
||||
}
|
||||
|
||||
impl GitRunTarget {
|
||||
/// Validates and canonicalizes this Git coordinate without resolving remote
|
||||
/// repository state.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Returns an error when the repository slug, branch, tag, or exact commit
|
||||
/// does not use the canonical grammar accepted for Git-backed runs.
|
||||
pub fn validate(self) -> Result<ValidatedGitRunTarget, GitCoordinateValidationError> {
|
||||
let Self {
|
||||
repo,
|
||||
branch,
|
||||
tag,
|
||||
sha,
|
||||
} = self;
|
||||
let repository =
|
||||
GitHubRepositorySlug::try_new(&repo).ok_or(GitCoordinateValidationError::Repository)?;
|
||||
if !repository::is_valid_git_branch_name(&branch) {
|
||||
return Err(GitCoordinateValidationError::Branch);
|
||||
}
|
||||
if tag
|
||||
.as_deref()
|
||||
.is_some_and(|tag| !repository::is_valid_git_tag_name(tag))
|
||||
{
|
||||
return Err(GitCoordinateValidationError::Tag);
|
||||
}
|
||||
let sha = sha
|
||||
.map(|sha| {
|
||||
repository::normalize_git_commit_sha(&sha).ok_or(GitCoordinateValidationError::Sha)
|
||||
})
|
||||
.transpose()?;
|
||||
let git = GitContext {
|
||||
origin_url: repository.https_url(),
|
||||
branch: branch.clone(),
|
||||
sha: sha.clone(),
|
||||
dirty: DirtyStatus::Clean,
|
||||
};
|
||||
Ok(ValidatedGitRunTarget {
|
||||
target: Self {
|
||||
repo,
|
||||
branch,
|
||||
tag,
|
||||
sha,
|
||||
},
|
||||
repository,
|
||||
git,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl RunTarget {
|
||||
/// The wire `kind` discriminator (`git`, `none`, or `folder`), for
|
||||
/// diagnostics.
|
||||
|
|
@ -78,44 +128,18 @@ impl RunTarget {
|
|||
/// Git targets include their derived operational Git projection. Targets
|
||||
/// without a repository return no projection. Folder paths require
|
||||
/// filesystem validation and canonicalization during provider admission.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
/// Returns an error when a Git target's repository slug, branch, tag, or
|
||||
/// exact commit does not use the canonical grammar accepted for runs.
|
||||
pub fn validate(self) -> Result<ValidatedRunTarget, TargetValidationError> {
|
||||
match self {
|
||||
Self::Git(GitRunTarget {
|
||||
repo,
|
||||
branch,
|
||||
tag,
|
||||
sha,
|
||||
}) => {
|
||||
let slug = GitHubRepositorySlug::try_new(&repo)
|
||||
.ok_or(TargetValidationError::Repository)?;
|
||||
if !repository::is_valid_git_branch_name(&branch) {
|
||||
return Err(TargetValidationError::Branch);
|
||||
}
|
||||
if tag
|
||||
.as_deref()
|
||||
.is_some_and(|tag| !repository::is_valid_git_tag_name(tag))
|
||||
{
|
||||
return Err(TargetValidationError::Tag);
|
||||
}
|
||||
let sha = sha
|
||||
.map(|sha| {
|
||||
repository::normalize_git_commit_sha(&sha).ok_or(TargetValidationError::Sha)
|
||||
})
|
||||
.transpose()?;
|
||||
let git = GitContext {
|
||||
origin_url: slug.https_url(),
|
||||
branch: branch.clone(),
|
||||
sha: sha.clone(),
|
||||
dirty: DirtyStatus::Clean,
|
||||
};
|
||||
Self::Git(target) => {
|
||||
let validated = target.validate().map_err(TargetValidationError::from)?;
|
||||
Ok(ValidatedRunTarget {
|
||||
target: Self::Git(GitRunTarget {
|
||||
repo,
|
||||
branch,
|
||||
tag,
|
||||
sha,
|
||||
}),
|
||||
git: Some(git),
|
||||
target: Self::Git(validated.target),
|
||||
git: Some(validated.git),
|
||||
})
|
||||
}
|
||||
Self::None {} => Ok(ValidatedRunTarget {
|
||||
|
|
@ -130,6 +154,34 @@ impl RunTarget {
|
|||
}
|
||||
}
|
||||
|
||||
/// A [`GitRunTarget`] whose local grammar has been validated and canonicalized.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ValidatedGitRunTarget {
|
||||
target: GitRunTarget,
|
||||
repository: GitHubRepositorySlug,
|
||||
git: GitContext,
|
||||
}
|
||||
|
||||
impl ValidatedGitRunTarget {
|
||||
/// The canonical Git target coordinate.
|
||||
#[must_use]
|
||||
pub fn target(&self) -> &GitRunTarget {
|
||||
&self.target
|
||||
}
|
||||
|
||||
/// The parsed GitHub repository named by the target.
|
||||
#[must_use]
|
||||
pub fn repository(&self) -> &GitHubRepositorySlug {
|
||||
&self.repository
|
||||
}
|
||||
|
||||
/// Consume the validation proof and return the canonical Git target.
|
||||
#[must_use]
|
||||
pub fn into_target(self) -> GitRunTarget {
|
||||
self.target
|
||||
}
|
||||
}
|
||||
|
||||
/// A [`RunTarget`] whose grammar has been validated, together with its
|
||||
/// optional operational Git projection.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
|
|
@ -138,6 +190,19 @@ pub struct ValidatedRunTarget {
|
|||
pub git: Option<GitContext>,
|
||||
}
|
||||
|
||||
/// A Git coordinate that failed local grammar validation.
|
||||
#[derive(Debug, thiserror::Error, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum GitCoordinateValidationError {
|
||||
#[error("repository must be a valid GitHub owner/name slug")]
|
||||
Repository,
|
||||
#[error("branch must be a non-empty branch name, not a ref or commit selector")]
|
||||
Branch,
|
||||
#[error("tag must be a non-empty bare tag name, not a ref or commit selector")]
|
||||
Tag,
|
||||
#[error("SHA must be exactly 40 ASCII hexadecimal characters")]
|
||||
Sha,
|
||||
}
|
||||
|
||||
/// A [`RunTarget`] that failed grammar validation.
|
||||
#[derive(Debug, thiserror::Error, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum TargetValidationError {
|
||||
|
|
@ -150,3 +215,14 @@ pub enum TargetValidationError {
|
|||
#[error("target SHA must be exactly 40 ASCII hexadecimal characters")]
|
||||
Sha,
|
||||
}
|
||||
|
||||
impl From<GitCoordinateValidationError> for TargetValidationError {
|
||||
fn from(error: GitCoordinateValidationError) -> Self {
|
||||
match error {
|
||||
GitCoordinateValidationError::Repository => Self::Repository,
|
||||
GitCoordinateValidationError::Branch => Self::Branch,
|
||||
GitCoordinateValidationError::Tag => Self::Tag,
|
||||
GitCoordinateValidationError::Sha => Self::Sha,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -188,6 +188,25 @@ fn target_validation_normalizes_sha_without_network_resolution() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_target_validation_carries_the_parsed_repository_proof() {
|
||||
let validated = GitRunTarget {
|
||||
repo: "Fabro-Sh/Fabro".to_string(),
|
||||
branch: "feature/run-intent".to_string(),
|
||||
tag: None,
|
||||
sha: Some("ABCDEF0123456789ABCDEF0123456789ABCDEF01".to_string()),
|
||||
}
|
||||
.validate()
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(validated.repository().owner(), "Fabro-Sh");
|
||||
assert_eq!(validated.repository().repo(), "Fabro");
|
||||
assert_eq!(
|
||||
validated.target().sha.as_deref(),
|
||||
Some("abcdef0123456789abcdef0123456789abcdef01")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn run_intent_none_target_validates_without_a_git_projection() {
|
||||
let validated = RunTarget::None {}.validate().unwrap();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue