diff --git a/Cargo.lock b/Cargo.lock index d2f53dfa9..27ebec10e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2669,6 +2669,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "url", ] [[package]] diff --git a/lib/components/fabro-github/Cargo.toml b/lib/components/fabro-github/Cargo.toml index 9db73130e..d77b75922 100644 --- a/lib/components/fabro-github/Cargo.toml +++ b/lib/components/fabro-github/Cargo.toml @@ -26,6 +26,7 @@ tracing.workspace = true tokio = { workspace = true } base64.workspace = true thiserror.workspace = true +url.workspace = true [dev-dependencies] fabro-macros = { path = "../../foundation/fabro-macros" } diff --git a/lib/components/fabro-github/src/lib.rs b/lib/components/fabro-github/src/lib.rs index fd4b98936..3e31e9c55 100644 --- a/lib/components/fabro-github/src/lib.rs +++ b/lib/components/fabro-github/src/lib.rs @@ -4,11 +4,15 @@ use base64::engine::general_purpose::STANDARD; use chrono::{DateTime, Utc}; use fabro_redact::DisplaySafeUrl; use fabro_static::EnvVars; -use fabro_types::PullRequestGithubDetail; use fabro_types::settings::run::MergeStrategy; +use fabro_types::{GitHubRepositorySlug, PullRequestGithubDetail}; use serde::Deserialize; use tokio::process::Command; +mod repository_reader; + +pub use repository_reader::{GitHubRepositoryReader, RepositoryReadError}; + pub const GITHUB_API_BASE_URL: &str = "https://api.github.com"; /// Returns the GitHub API base URL, allowing override via `GITHUB_BASE_URL` env @@ -991,61 +995,24 @@ fn normalize_https_host_path(url: &str) -> String { /// Return the commit SHA at the head of a GitHub branch. /// -/// Returns `None` when the branch does not exist. +/// Returns `None` when GitHub responds with 404. GitHub also uses 404 to hide +/// some private resources from credentials that cannot access them, so `None` +/// means the branch was not observable rather than proving it does not exist. pub async fn branch_head_sha( ctx: &GitHubContext<'_>, owner: &str, repo: &str, branch: &str, ) -> anyhow::Result> { - let client = ctx.http_client()?; - branch_head_sha_with_client(&client, ctx, owner, repo, branch).await -} - -async fn branch_head_sha_with_client( - client: &impl HttpClient, - ctx: &GitHubContext<'_>, - owner: &str, - repo: &str, - branch: &str, -) -> anyhow::Result> { - #[derive(Deserialize)] - struct BranchResponse { - commit: BranchCommit, - } - - #[derive(Deserialize)] - struct BranchCommit { - sha: String, - } - - let token = ctx - .creds - .resolve_bearer_token( - client, - owner, - repo, - ctx.base_url, - serde_json::json!({ "contents": "read" }), - ) - .await?; - - let url = format!("{}/repos/{owner}/{repo}/branches/{branch}", ctx.base_url); - let auth = format!("Bearer {token}"); - let resp = client - .request(HttpMethod::Get, &url, &github_headers(&auth), None) + let repository = GitHubRepositorySlug::try_new(&format!("{owner}/{repo}")) + .ok_or_else(|| anyhow!("Invalid GitHub repository coordinate"))?; + let reader = GitHubRepositoryReader::open(ctx, &repository) .await .context("Failed to read remote branch head")?; - - match resp.status { - 200 => { - let branch: BranchResponse = resp - .json() - .context("Failed to parse remote branch response")?; - Ok(Some(branch.commit.sha)) - } - 404 => Ok(None), - status => bail!("Unexpected status {status} reading branch '{branch}'"), + match reader.resolve_commit(&format!("heads/{branch}")).await { + Ok(sha) => Ok(Some(sha)), + Err(RepositoryReadError::RevisionNotFound) => Ok(None), + Err(error) => Err(anyhow::Error::new(error).context("Failed to read remote branch head")), } } @@ -2091,126 +2058,6 @@ mod tests { assert_eq!(token, "ghs_pr_token"); } - // ----------------------------------------------------------------------- - // branch_head_sha - // ----------------------------------------------------------------------- - - fn app_creds() -> GitHubCredentials { - GitHubCredentials::App(GitHubAppCredentials { - app_id: "test".to_string(), - private_key_pem: test_rsa_key().to_string(), - slug: None, - }) - } - - /// Mock the installation-token exchange every App-credentialed call makes - /// before it reaches the endpoint under test. - fn mock_with_installation_token() -> MockHttpClient { - MockHttpClient::new() - .on( - HttpMethod::Get, - "/repos/owner/repo/installation", - 200, - r#"{"id": 1}"#, - ) - .on( - HttpMethod::Post, - "/app/installations/1/access_tokens", - 201, - r#"{"token": "ghs_test", "expires_at": "2099-01-01T00:00:00Z"}"#, - ) - } - - #[tokio::test] - async fn branch_head_sha_returns_commit_on_200() { - let mock = mock_with_installation_token().on( - HttpMethod::Get, - "/repos/owner/repo/branches/my-branch", - 200, - r#"{"name": "my-branch", "commit": {"sha": "abc123"}}"#, - ); - - let creds = app_creds(); - let result = branch_head_sha_with_client( - &mock, - &GitHubContext::new(&creds, ""), - "owner", - "repo", - "my-branch", - ) - .await; - - assert_eq!(result.unwrap(), Some("abc123".to_string())); - } - - #[tokio::test] - async fn branch_head_sha_returns_none_on_404() { - let mock = mock_with_installation_token().on( - HttpMethod::Get, - "/repos/owner/repo/branches/no-such-branch", - 404, - "", - ); - - let creds = app_creds(); - let result = branch_head_sha_with_client( - &mock, - &GitHubContext::new(&creds, ""), - "owner", - "repo", - "no-such-branch", - ) - .await; - - assert_eq!(result.unwrap(), None); - } - - #[tokio::test] - async fn branch_head_sha_returns_error_on_500() { - let mock = mock_with_installation_token().on( - HttpMethod::Get, - "/repos/owner/repo/branches/broken", - 500, - "", - ); - - let creds = app_creds(); - let result = branch_head_sha_with_client( - &mock, - &GitHubContext::new(&creds, ""), - "owner", - "repo", - "broken", - ) - .await; - - assert!(result.is_err()); - } - - #[tokio::test] - async fn branch_head_sha_with_token_uses_direct_bearer_token() { - let mock = MockHttpClient::new() - .on( - HttpMethod::Get, - "/repos/owner/repo/branches/my-branch", - 200, - r#"{"name": "my-branch", "commit": {"sha": "abc123"}}"#, - ) - .with_req_header("Authorization", "Bearer ghu_test"); - - let creds = GitHubCredentials::Pat("ghu_test".to_string()); - let result = branch_head_sha_with_client( - &mock, - &GitHubContext::new(&creds, ""), - "owner", - "repo", - "my-branch", - ) - .await; - - assert_eq!(result.unwrap(), Some("abc123".to_string())); - } - // ----------------------------------------------------------------------- // check_app_installed // ----------------------------------------------------------------------- diff --git a/lib/components/fabro-github/src/repository_reader.rs b/lib/components/fabro-github/src/repository_reader.rs new file mode 100644 index 000000000..2f6d82ffc --- /dev/null +++ b/lib/components/fabro-github/src/repository_reader.rs @@ -0,0 +1,725 @@ +#![expect( + clippy::disallowed_types, + reason = "Validated URL values are used only for request construction and are never logged or included in errors." +)] + +use fabro_http::header::{ACCEPT, CONTENT_TYPE, RETRY_AFTER, USER_AGENT}; +use fabro_http::{HeaderMap, HttpClient, Response, StatusCode, Url}; +use fabro_types::{GitHubRepositorySlug, repository}; + +use crate::GitHubContext; + +const SHA_MEDIA_TYPE: &str = "application/vnd.github.sha"; +const RAW_CONTENT_MEDIA_TYPE: &str = "application/vnd.github.raw+json"; +const MAX_SHA_RESPONSE_BYTES: usize = 128; + +/// Failures while opening or using a repository-scoped GitHub reader. +#[derive(Debug, thiserror::Error)] +pub enum RepositoryReadError { + #[error("invalid GitHub API base URL ({reason})")] + InvalidApiBaseUrl { + reason: &'static str, + #[source] + source: Option, + }, + #[error("invalid GitHub ref selector")] + InvalidRefSelector, + #[error("invalid Git commit SHA")] + InvalidCommitSha, + #[error("invalid repository path ({reason})")] + InvalidRepositoryPath { reason: &'static str }, + #[error("failed to resolve GitHub repository credentials")] + CredentialResolution { + #[source] + source: anyhow::Error, + }, + #[error("GitHub repository request failed")] + RequestTransport { + #[source] + source: anyhow::Error, + }, + #[error("GitHub authentication was rejected")] + AuthenticationRejected, + #[error("GitHub repository permission was denied")] + PermissionDenied, + #[error("GitHub rate limit reached (status {status})")] + RateLimited { status: u16 }, + /// GitHub returned 404 while resolving a revision. GitHub may also use + /// 404 to hide a private resource that these credentials cannot access. + #[error("GitHub revision was not observable")] + RevisionNotFound, + /// GitHub returned 404 while reading content. GitHub may also use 404 to + /// hide a private resource that these credentials cannot access. + #[error("GitHub repository content was not observable")] + ContentNotFound, + #[error("GitHub repository content is not a file")] + ContentNotFile, + #[error("GitHub revision is unavailable (status {status})")] + RevisionUnavailable { status: u16 }, + #[error("GitHub repository content is unavailable (status {status})")] + ContentUnavailable { status: u16 }, + #[error("GitHub {operation} service is unavailable (status {status})")] + UpstreamUnavailable { + operation: &'static str, + status: u16, + }, + #[error("unexpected GitHub {operation} status {status}")] + UnexpectedStatus { + operation: &'static str, + status: u16, + }, + #[error("GitHub response exceeded the {max_bytes}-byte limit")] + BodyTooLarge { max_bytes: usize }, + #[error("GitHub returned a malformed commit SHA")] + MalformedCommitSha, + #[error("GitHub repository content is not valid UTF-8")] + InvalidUtf8 { + #[source] + source: std::str::Utf8Error, + }, +} + +/// An authenticated read session scoped to one GitHub repository. +pub struct GitHubRepositoryReader { + client: HttpClient, + api_base: Url, + repository: GitHubRepositorySlug, + bearer_token: String, +} + +#[derive(Clone, Copy)] +enum Operation { + Revision, + Content, +} + +impl Operation { + const fn name(self) -> &'static str { + match self { + Self::Revision => "revision lookup", + Self::Content => "content read", + } + } +} + +impl GitHubRepositoryReader { + /// Opens an authenticated read session for one repository. + pub async fn open( + ctx: &GitHubContext<'_>, + repository: &GitHubRepositorySlug, + ) -> Result { + let client = ctx + .http_client() + .map_err(|source| RepositoryReadError::RequestTransport { source })?; + let (api_base, normalized_base) = parse_api_base(ctx.base_url)?; + let bearer_token = ctx + .creds + .resolve_bearer_token( + &client, + repository.owner(), + repository.repo(), + &normalized_base, + serde_json::json!({ "contents": "read" }), + ) + .await + .map_err(|source| RepositoryReadError::CredentialResolution { source })?; + + Ok(Self { + client, + api_base, + repository: repository.clone(), + bearer_token, + }) + } + + /// Resolves a validated ref selector to an exact lowercase commit SHA. + pub async fn resolve_commit(&self, selector: &str) -> Result { + if !repository::is_valid_github_ref_selector(selector) { + return Err(RepositoryReadError::InvalidRefSelector); + } + + let url = commit_url(&self.api_base, &self.repository, selector)?; + let response = self + .client + .get(url) + .bearer_auth(&self.bearer_token) + .header(USER_AGENT, "fabro") + .header(ACCEPT, SHA_MEDIA_TYPE) + .send() + .await + .map_err(|source| RepositoryReadError::RequestTransport { + source: anyhow::Error::new(source.without_url()), + })?; + + classify_status(response.status(), response.headers(), Operation::Revision)?; + let bytes = collect_bounded(response, MAX_SHA_RESPONSE_BYTES).await?; + parse_resolved_commit_sha(bytes) + } + + /// Reads one UTF-8 file at an explicitly supplied exact commit SHA. + pub async fn read_utf8_file_at( + &self, + commit_sha: &str, + canonical_repo_path: &str, + max_bytes: usize, + ) -> Result { + if !is_exact_commit_sha(commit_sha.as_bytes()) { + return Err(RepositoryReadError::InvalidCommitSha); + } + validate_repository_path(canonical_repo_path)?; + + let url = content_url( + &self.api_base, + &self.repository, + commit_sha, + canonical_repo_path, + )?; + let response = self + .client + .get(url) + .bearer_auth(&self.bearer_token) + .header(USER_AGENT, "fabro") + .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) + .send() + .await + .map_err(|source| RepositoryReadError::RequestTransport { + source: anyhow::Error::new(source.without_url()), + })?; + + classify_status(response.status(), response.headers(), Operation::Content)?; + if !has_raw_content_media_type(response.headers()) { + return Err(RepositoryReadError::ContentNotFile); + } + let bytes = collect_bounded(response, max_bytes).await?; + String::from_utf8(bytes).map_err(|source| RepositoryReadError::InvalidUtf8 { + source: source.utf8_error(), + }) + } +} + +fn parse_api_base(base_url: &str) -> Result<(Url, String), RepositoryReadError> { + let mut url = + Url::parse(base_url).map_err(|source| RepositoryReadError::InvalidApiBaseUrl { + reason: "parse", + source: Some(source), + })?; + if url.cannot_be_a_base() { + return Err(invalid_base("cannot be a base")); + } + if url.host_str().is_none() { + return Err(invalid_base("host")); + } + if !matches!(url.scheme(), "http" | "https") { + return Err(invalid_base("scheme")); + } + if !url.username().is_empty() || url.password().is_some() { + return Err(invalid_base("credentials")); + } + if url.query().is_some() { + return Err(invalid_base("query")); + } + if url.fragment().is_some() { + return Err(invalid_base("fragment")); + } + + let mut segments = url + .path_segments_mut() + .map_err(|()| invalid_base("cannot be a base"))?; + segments.pop_if_empty(); + drop(segments); + + let normalized = url.as_str().trim_end_matches('/').to_string(); + Ok((url, normalized)) +} + +const fn invalid_base(reason: &'static str) -> RepositoryReadError { + RepositoryReadError::InvalidApiBaseUrl { + reason, + source: None, + } +} + +fn commit_url( + base: &Url, + repository: &GitHubRepositorySlug, + selector: &str, +) -> Result { + let mut url = base.clone(); + let mut segments = url + .path_segments_mut() + .map_err(|()| invalid_base("cannot be a base"))?; + segments + .pop_if_empty() + .push("repos") + .push(repository.owner()) + .push(repository.repo()) + .push("commits") + .push(selector); + drop(segments); + Ok(url) +} + +fn content_url( + base: &Url, + repository: &GitHubRepositorySlug, + commit_sha: &str, + path: &str, +) -> Result { + let mut url = base.clone(); + let mut segments = url + .path_segments_mut() + .map_err(|()| invalid_base("cannot be a base"))?; + segments + .pop_if_empty() + .push("repos") + .push(repository.owner()) + .push(repository.repo()) + .push("contents"); + for component in path.split('/') { + segments.push(component); + } + drop(segments); + url.query_pairs_mut().append_pair("ref", commit_sha); + Ok(url) +} + +fn is_exact_commit_sha(bytes: &[u8]) -> bool { + bytes.len() == 40 && bytes.iter().all(u8::is_ascii_hexdigit) +} + +fn parse_resolved_commit_sha(bytes: Vec) -> Result { + if !is_exact_commit_sha(&bytes) { + return Err(RepositoryReadError::MalformedCommitSha); + } + Ok(bytes + .into_iter() + .map(|byte| char::from(byte.to_ascii_lowercase())) + .collect()) +} + +fn validate_repository_path(path: &str) -> Result<(), RepositoryReadError> { + if path.is_empty() { + return Err(invalid_path("empty")); + } + if path.len() > 4096 { + return Err(invalid_path("too long")); + } + if path.split('/').count() > 256 { + return Err(invalid_path("too many components")); + } + let bytes = path.as_bytes(); + let windows_drive = bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':'; + if path.starts_with(['/', '~']) || windows_drive { + return Err(invalid_path("absolute")); + } + if path.contains('\\') { + return Err(invalid_path("backslash")); + } + if path.chars().any(char::is_control) { + return Err(invalid_path("control character")); + } + if path.ends_with('/') || path.contains("//") { + return Err(invalid_path("empty component")); + } + if path + .split('/') + .any(|component| matches!(component, "." | "..")) + { + return Err(invalid_path("dot segment")); + } + Ok(()) +} + +const fn invalid_path(reason: &'static str) -> RepositoryReadError { + RepositoryReadError::InvalidRepositoryPath { reason } +} + +fn classify_status( + status: StatusCode, + headers: &HeaderMap, + operation: Operation, +) -> Result<(), RepositoryReadError> { + if status == StatusCode::OK { + return Ok(()); + } + + let code = status.as_u16(); + match status { + StatusCode::UNAUTHORIZED => Err(RepositoryReadError::AuthenticationRejected), + StatusCode::FORBIDDEN if is_rate_limited(headers) => { + Err(RepositoryReadError::RateLimited { status: code }) + } + StatusCode::FORBIDDEN => Err(RepositoryReadError::PermissionDenied), + StatusCode::TOO_MANY_REQUESTS => Err(RepositoryReadError::RateLimited { status: code }), + StatusCode::NOT_FOUND => match operation { + Operation::Revision => Err(RepositoryReadError::RevisionNotFound), + Operation::Content => Err(RepositoryReadError::ContentNotFound), + }, + StatusCode::CONFLICT | StatusCode::UNPROCESSABLE_ENTITY => match operation { + Operation::Revision => Err(RepositoryReadError::RevisionUnavailable { status: code }), + Operation::Content => Err(RepositoryReadError::ContentUnavailable { status: code }), + }, + status if status.is_server_error() => Err(RepositoryReadError::UpstreamUnavailable { + operation: operation.name(), + status: code, + }), + _ => Err(RepositoryReadError::UnexpectedStatus { + operation: operation.name(), + status: code, + }), + } +} + +fn is_rate_limited(headers: &HeaderMap) -> bool { + headers + .get("x-ratelimit-remaining") + .is_some_and(|value| value.as_bytes() == b"0") + || headers.contains_key(RETRY_AFTER) +} + +fn has_raw_content_media_type(headers: &HeaderMap) -> bool { + headers + .get(CONTENT_TYPE) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.split(';').next()) + .is_some_and(|value| value.trim().eq_ignore_ascii_case(RAW_CONTENT_MEDIA_TYPE)) +} + +fn checked_body_len( + current: usize, + chunk: usize, + max_bytes: usize, +) -> Result { + current + .checked_add(chunk) + .filter(|length| *length <= max_bytes) + .ok_or(RepositoryReadError::BodyTooLarge { max_bytes }) +} + +async fn collect_bounded( + mut response: Response, + max_bytes: usize, +) -> Result, RepositoryReadError> { + if response + .content_length() + .is_some_and(|length| length > max_bytes as u64) + { + return Err(RepositoryReadError::BodyTooLarge { max_bytes }); + } + + let mut bytes = Vec::new(); + while let Some(chunk) = + response + .chunk() + .await + .map_err(|source| RepositoryReadError::RequestTransport { + source: anyhow::Error::new(source.without_url()), + })? + { + checked_body_len(bytes.len(), chunk.len(), max_bytes)?; + bytes.extend_from_slice(&chunk); + } + Ok(bytes) +} + +#[cfg(test)] +mod tests { + use std::error::Error as _; + + use super::*; + + const SHA: &str = "0123456789abcdef0123456789abcdef01234567"; + + fn repository() -> GitHubRepositorySlug { + GitHubRepositorySlug::try_new("owner/repo").unwrap() + } + + #[test] + fn api_base_preserves_prefix_and_normalizes_trailing_slash() { + let (base, normalized) = parse_api_base("https://ghe.example/api/v3/").unwrap(); + assert_eq!(base.as_str(), "https://ghe.example/api/v3"); + assert_eq!(normalized, "https://ghe.example/api/v3"); + + let (root, normalized) = parse_api_base("https://api.github.com/").unwrap(); + assert_eq!(root.as_str(), "https://api.github.com/"); + assert_eq!(normalized, "https://api.github.com"); + } + + #[test] + fn api_base_rejects_unsafe_shapes_without_echoing_input() { + let cases = [ + ("ftp://example.test/api", "scheme"), + ("mailto:test@example.test", "cannot be a base"), + ("file:///api", "host"), + ("https://user:password@example.test/api", "credentials"), + ("https://example.test/api?token=sentinel", "query"), + ("https://example.test/api#sentinel", "fragment"), + ]; + for (input, reason) in cases { + let error = parse_api_base(input).unwrap_err(); + assert!(matches!( + error, + RepositoryReadError::InvalidApiBaseUrl { + reason: actual, + source: None, + } if actual == reason + )); + assert!(!format!("{error}").contains(input)); + assert!(!format!("{error:?}").contains(input)); + assert!(error.source().is_none()); + } + } + + #[test] + fn syntactically_invalid_api_base_preserves_parse_source() { + let error = parse_api_base("not an absolute URL").unwrap_err(); + assert!(matches!(error, RepositoryReadError::InvalidApiBaseUrl { + reason: "parse", + source: Some(_), + })); + assert!(error.source().is_some()); + } + + #[test] + fn url_builders_encode_selectors_and_paths_once() { + let (base, _) = parse_api_base("https://ghe.example/api/v3").unwrap(); + let commit = commit_url(&base, &repository(), "heads/fabro/run/123").unwrap(); + assert_eq!( + commit.as_str(), + "https://ghe.example/api/v3/repos/owner/repo/commits/heads%2Ffabro%2Frun%2F123" + ); + + let content = content_url(&base, &repository(), SHA, "dir/a b/%2F/#?é.toml").unwrap(); + assert_eq!( + content.as_str(), + "https://ghe.example/api/v3/repos/owner/repo/contents/dir/a%20b/%252F/%23%3F%C3%A9.toml?ref=0123456789abcdef0123456789abcdef01234567" + ); + } + + #[tokio::test] + async fn method_inputs_are_rejected_before_network_access() { + let reader = GitHubRepositoryReader { + client: fabro_http::test_http_client().unwrap(), + api_base: Url::parse("http://127.0.0.1:1").unwrap(), + repository: repository(), + bearer_token: "sentinel-token".to_string(), + }; + + for invalid in ["", " main", "heads//main", "tags/v1.lock"] { + assert!(matches!( + reader.resolve_commit(invalid).await, + Err(RepositoryReadError::InvalidRefSelector) + )); + } + assert!(matches!( + reader.read_utf8_file_at("not-a-sha", "../bad", 1).await, + Err(RepositoryReadError::InvalidCommitSha) + )); + } + + #[test] + fn canonical_ref_selectors_reach_url_construction() { + let (base, _) = parse_api_base("https://api.github.com").unwrap(); + for selector in ["main", "heads/fabro/run/123", "tags/v1.0.0", SHA] { + assert!(repository::is_valid_github_ref_selector(selector)); + assert!(commit_url(&base, &repository(), selector).is_ok()); + } + } + + #[test] + fn repository_path_validation_enforces_boundaries() { + assert!(validate_repository_path("a b/%/é.toml").is_ok()); + assert!(validate_repository_path(&"a".repeat(4096)).is_ok()); + assert!(validate_repository_path(&vec!["a"; 256].join("/")).is_ok()); + + let too_many = vec!["a"; 257].join("/"); + let cases = [ + ("", "empty"), + ("/a", "absolute"), + ("~a", "absolute"), + ("C:/a", "absolute"), + ("a\\b", "backslash"), + ("a\nb", "control character"), + ("a/", "empty component"), + ("a//b", "empty component"), + ("a/./b", "dot segment"), + ("a/../b", "dot segment"), + (&too_many, "too many components"), + ]; + for (path, reason) in cases { + assert!(matches!( + validate_repository_path(path), + Err(RepositoryReadError::InvalidRepositoryPath { reason: actual }) + if actual == reason + )); + } + assert!(matches!( + validate_repository_path(&"a".repeat(4097)), + Err(RepositoryReadError::InvalidRepositoryPath { reason: "too long" }) + )); + } + + #[test] + fn commit_sha_validation_is_exact() { + assert!(is_exact_commit_sha(SHA.as_bytes())); + assert!(is_exact_commit_sha(SHA.to_ascii_uppercase().as_bytes())); + for invalid in [ + &SHA[..39], + "0123456789abcdef0123456789abcdef012345678", + "g123456789abcdef0123456789abcdef01234567", + " 123456789abcdef0123456789abcdef01234567", + ] { + assert!(!is_exact_commit_sha(invalid.as_bytes())); + } + } + + #[test] + fn resolved_commit_sha_is_exact_and_normalized() { + assert_eq!( + parse_resolved_commit_sha(SHA.as_bytes().to_vec()).unwrap(), + SHA + ); + assert_eq!( + parse_resolved_commit_sha(SHA.to_ascii_uppercase().into_bytes()).unwrap(), + SHA + ); + let mut newline = SHA.as_bytes().to_vec(); + newline.push(b'\n'); + for invalid in [ + SHA.as_bytes()[..39].to_vec(), + b"0123456789abcdef0123456789abcdef012345678".to_vec(), + b"g123456789abcdef0123456789abcdef01234567".to_vec(), + b" 123456789abcdef0123456789abcdef01234567".to_vec(), + format!("\"{SHA}\"").into_bytes(), + newline, + format!("{SHA}suffix").into_bytes(), + ] { + assert!(matches!( + parse_resolved_commit_sha(invalid), + Err(RepositoryReadError::MalformedCommitSha) + )); + } + } + + #[test] + fn status_classification_keeps_operations_and_rate_limits_distinct() { + let empty = HeaderMap::new(); + assert!(matches!( + classify_status(StatusCode::UNAUTHORIZED, &empty, Operation::Revision), + Err(RepositoryReadError::AuthenticationRejected) + )); + assert!(matches!( + classify_status(StatusCode::NOT_FOUND, &empty, Operation::Revision), + Err(RepositoryReadError::RevisionNotFound) + )); + assert!(matches!( + classify_status(StatusCode::NOT_FOUND, &empty, Operation::Content), + Err(RepositoryReadError::ContentNotFound) + )); + assert!(matches!( + classify_status(StatusCode::FORBIDDEN, &empty, Operation::Content), + Err(RepositoryReadError::PermissionDenied) + )); + + let mut remaining = HeaderMap::new(); + remaining.insert("x-ratelimit-remaining", "0".parse().unwrap()); + assert!(matches!( + classify_status(StatusCode::FORBIDDEN, &remaining, Operation::Content), + Err(RepositoryReadError::RateLimited { status: 403 }) + )); + + let mut retry_after = HeaderMap::new(); + retry_after.insert(RETRY_AFTER, "60".parse().unwrap()); + assert!(matches!( + classify_status(StatusCode::FORBIDDEN, &retry_after, Operation::Content), + Err(RepositoryReadError::RateLimited { status: 403 }) + )); + assert!(matches!( + classify_status(StatusCode::TOO_MANY_REQUESTS, &empty, Operation::Content), + Err(RepositoryReadError::RateLimited { status: 429 }) + )); + for operation in [Operation::Revision, Operation::Content] { + let conflict = classify_status(StatusCode::CONFLICT, &empty, operation); + let unprocessable = + classify_status(StatusCode::UNPROCESSABLE_ENTITY, &empty, operation); + match operation { + Operation::Revision => { + assert!(matches!( + conflict, + Err(RepositoryReadError::RevisionUnavailable { status: 409 }) + )); + assert!(matches!( + unprocessable, + Err(RepositoryReadError::RevisionUnavailable { status: 422 }) + )); + } + Operation::Content => { + assert!(matches!( + conflict, + Err(RepositoryReadError::ContentUnavailable { status: 409 }) + )); + assert!(matches!( + unprocessable, + Err(RepositoryReadError::ContentUnavailable { status: 422 }) + )); + } + } + } + assert!(matches!( + classify_status(StatusCode::BAD_GATEWAY, &empty, Operation::Content), + Err(RepositoryReadError::UpstreamUnavailable { + operation: "content read", + status: 502, + }) + )); + assert!(matches!( + classify_status(StatusCode::IM_A_TEAPOT, &empty, Operation::Revision), + Err(RepositoryReadError::UnexpectedStatus { + operation: "revision lookup", + status: 418, + }) + )); + } + + #[test] + fn raw_content_media_type_ignores_case_and_parameters() { + let mut headers = HeaderMap::new(); + headers.insert( + CONTENT_TYPE, + "Application/Vnd.Github.Raw+Json; charset=utf-8" + .parse() + .unwrap(), + ); + assert!(has_raw_content_media_type(&headers)); + headers.insert(CONTENT_TYPE, "application/json".parse().unwrap()); + assert!(!has_raw_content_media_type(&headers)); + } + + #[test] + fn body_length_check_is_inclusive_and_overflow_safe() { + assert_eq!(checked_body_len(2, 3, 5).unwrap(), 5); + assert!(matches!( + checked_body_len(2, 4, 5), + Err(RepositoryReadError::BodyTooLarge { max_bytes: 5 }) + )); + assert!(matches!( + checked_body_len(usize::MAX, 1, usize::MAX), + Err(RepositoryReadError::BodyTooLarge { + max_bytes: usize::MAX, + }) + )); + } + + #[test] + fn invalid_utf8_source_does_not_own_repository_bytes() { + let sentinel = b"private-file-contents"; + let mut bytes = sentinel.to_vec(); + bytes.push(0xff); + let source = String::from_utf8(bytes).unwrap_err().utf8_error(); + let error = RepositoryReadError::InvalidUtf8 { source }; + assert!(error.source().is_some()); + let rendered = format!("{error:?}"); + assert!(!rendered.contains("private-file-contents")); + assert!(!format!("{:?}", error.source()).contains("private-file-contents")); + } +} diff --git a/lib/components/fabro-github/tests/integration.rs b/lib/components/fabro-github/tests/integration.rs index 5dc0ec20a..f98d88f1d 100644 --- a/lib/components/fabro-github/tests/integration.rs +++ b/lib/components/fabro-github/tests/integration.rs @@ -1,12 +1,19 @@ +use std::error::Error as _; + use fabro_github::{ - GitHubAppCredentials, GitHubContext, GitHubCredentials, close_pull_request, + GitHubAppCredentials, GitHubContext, GitHubCredentials, GitHubRepositoryReader, + InstallationToken, RepositoryReadError, branch_head_sha, close_pull_request, create_installation_access_token_for_pr, create_pull_request, enable_auto_merge, get_pull_request, merge_pull_request, resolve_authenticated_url, sign_app_jwt, }; use fabro_test::{GitHubAppOptions, GitHubAppState, TwinGitHub}; +use fabro_types::GitHubRepositorySlug; use fabro_types::settings::run::MergeStrategy; const TEST_RSA_KEY: &str = include_str!("../src/testdata/rsa_private.pem"); +const HEAD_SHA: &str = "1111111111111111111111111111111111111111"; +const TAG_SHA: &str = "2222222222222222222222222222222222222222"; +const UPPER_SHA: &str = "ABCDEFABCDEFABCDEFABCDEFABCDEFABCDEFABCD"; fn github_credentials() -> GitHubCredentials { GitHubCredentials::App(GitHubAppCredentials { @@ -33,9 +40,48 @@ fn standard_app_state() -> GitHubAppState { vec!["main".into(), "feature".into()], false, ); + state.set_repository_ref("acme", "widgets", "heads/main", HEAD_SHA); + state.set_repository_ref("acme", "widgets", "heads/release", HEAD_SHA); + state.set_repository_ref("acme", "widgets", "tags/release", TAG_SHA); + state.set_repository_ref("acme", "widgets", "heads/fabro/run/123", HEAD_SHA); + state.set_repository_ref("acme", "widgets", "heads/uppercase", UPPER_SHA); + state.add_repository_file( + "acme", + "widgets", + HEAD_SHA, + ".fabro/workflows/build/workflow.toml", + b"name = \"build\"\n".to_vec(), + ); + state.add_repository_file( + "acme", + "widgets", + HEAD_SHA, + "dir/hello world#.txt", + b"hello\n".to_vec(), + ); + state.add_repository_file("acme", "widgets", HEAD_SHA, "invalid.txt", vec![0xff, 0xfe]); + state.add_repository_file("acme", "widgets", HEAD_SHA, "empty.txt", Vec::new()); + state.add_repository_file( + "acme", + "widgets", + HEAD_SHA, + "five-bytes.txt", + b"12345".to_vec(), + ); + state.add_repository_file( + "acme", + "widgets", + HEAD_SHA, + "nested/a b/%/é.toml", + "unicode: 🦀\n".as_bytes().to_vec(), + ); state } +fn repository() -> GitHubRepositorySlug { + GitHubRepositorySlug::try_new("acme/widgets").expect("test repository slug should be valid") +} + #[fabro_macros::e2e_test(twin)] async fn create_and_get_pull_request() { let twin = TwinGitHub::start(standard_app_state()).await; @@ -226,3 +272,310 @@ async fn resolve_authenticated_url_errors_on_non_github_url() { twin.shutdown().await; } + +#[fabro_macros::e2e_test(twin)] +async fn repository_reader_reuses_one_app_token_for_ref_and_file_reads() { + let twin = TwinGitHub::start(standard_app_state()).await; + assert_eq!(twin.active_token_count().await, 0); + let creds = github_credentials(); + let base_url = format!("{}/", twin.base_url); + let reader = GitHubRepositoryReader::open( + &GitHubContext::with_http_client(&creds, &base_url, fabro_test::test_http_client()), + &repository(), + ) + .await + .unwrap(); + assert_eq!(twin.active_token_count().await, 1); + + assert_eq!(reader.resolve_commit("heads/main").await.unwrap(), HEAD_SHA); + assert_eq!( + reader + .read_utf8_file_at(HEAD_SHA, ".fabro/workflows/build/workflow.toml", 1024) + .await + .unwrap(), + "name = \"build\"\n" + ); + assert_eq!( + reader + .read_utf8_file_at(HEAD_SHA, "dir/hello world#.txt", 1024) + .await + .unwrap(), + "hello\n" + ); + let unicode_contents = "unicode: 🦀\n"; + assert_eq!( + reader + .read_utf8_file_at(HEAD_SHA, "nested/a b/%/é.toml", unicode_contents.len(),) + .await + .unwrap(), + unicode_contents + ); + assert!(matches!( + reader + .read_utf8_file_at(HEAD_SHA, "nested/a b/%/é.toml", unicode_contents.len() - 1,) + .await, + Err(RepositoryReadError::BodyTooLarge { .. }) + )); + assert_eq!(twin.active_token_count().await, 1); + + twin.shutdown().await; +} + +#[fabro_macros::e2e_test(twin)] +async fn repository_reader_preserves_exact_ref_namespaces() { + let twin = TwinGitHub::start(standard_app_state()).await; + let creds = github_credentials(); + let reader = GitHubRepositoryReader::open( + &GitHubContext::with_http_client(&creds, &twin.base_url, fabro_test::test_http_client()), + &repository(), + ) + .await + .unwrap(); + + assert_eq!( + reader.resolve_commit("heads/release").await.unwrap(), + HEAD_SHA + ); + assert_eq!( + reader.resolve_commit("tags/release").await.unwrap(), + TAG_SHA + ); + assert_eq!( + reader.resolve_commit("heads/fabro/run/123").await.unwrap(), + HEAD_SHA + ); + assert_eq!(reader.resolve_commit(HEAD_SHA).await.unwrap(), HEAD_SHA); + assert_eq!( + reader.resolve_commit("heads/uppercase").await.unwrap(), + UPPER_SHA.to_ascii_lowercase() + ); + assert!(matches!( + reader.resolve_commit("heads/missing").await, + Err(RepositoryReadError::RevisionNotFound) + )); + + twin.shutdown().await; +} + +#[fabro_macros::e2e_test(twin)] +async fn repository_reader_classifies_file_failures_without_retaining_bytes() { + let twin = TwinGitHub::start(standard_app_state()).await; + let creds = github_credentials(); + let reader = GitHubRepositoryReader::open( + &GitHubContext::with_http_client(&creds, &twin.base_url, fabro_test::test_http_client()), + &repository(), + ) + .await + .unwrap(); + + assert!(matches!( + reader + .read_utf8_file_at(HEAD_SHA, ".fabro/workflows/build/workflow.toml", 4) + .await, + Err(RepositoryReadError::BodyTooLarge { max_bytes: 4 }) + )); + let error = reader + .read_utf8_file_at(HEAD_SHA, "invalid.txt", 16) + .await + .unwrap_err(); + assert!(matches!(error, RepositoryReadError::InvalidUtf8 { .. })); + assert!( + error + .source() + .and_then(|source| source.downcast_ref::()) + .is_some() + ); + assert!(!format!("{error:?}").contains("255")); + let directory_error = reader + .read_utf8_file_at(HEAD_SHA, "dir", 1024) + .await + .unwrap_err(); + assert!(matches!( + directory_error, + RepositoryReadError::ContentNotFile + )); + assert!(!format!("{directory_error:?}").contains("[]")); + assert!(matches!( + reader + .read_utf8_file_at(HEAD_SHA, "missing.txt", 1024) + .await, + Err(RepositoryReadError::ContentNotFound) + )); + assert_eq!( + reader + .read_utf8_file_at(HEAD_SHA, "five-bytes.txt", 5) + .await + .unwrap(), + "12345" + ); + assert!(matches!( + reader + .read_utf8_file_at(HEAD_SHA, "five-bytes.txt", 4) + .await, + Err(RepositoryReadError::BodyTooLarge { max_bytes: 4 }) + )); + assert_eq!( + reader + .read_utf8_file_at(HEAD_SHA, "empty.txt", 0) + .await + .unwrap(), + "" + ); + + twin.shutdown().await; +} + +#[fabro_macros::e2e_test(twin)] +async fn static_repository_credentials_do_not_mint_tokens() { + let mut state = standard_app_state(); + let static_token = state.generate_access_token( + "42", + 1, + vec!["widgets".to_string()], + serde_json::json!({ "contents": "read" }), + ); + let twin = TwinGitHub::start(state).await; + let initial_tokens = twin.active_token_count().await; + + for credentials in [ + GitHubCredentials::Pat(static_token.clone()), + GitHubCredentials::Installation(InstallationToken { + token: static_token.clone(), + expires_at: chrono::Utc::now() + chrono::Duration::hours(1), + }), + ] { + let reader = GitHubRepositoryReader::open( + &GitHubContext::with_http_client( + &credentials, + &twin.base_url, + fabro_test::test_http_client(), + ), + &repository(), + ) + .await + .unwrap(); + assert_eq!(reader.resolve_commit("heads/main").await.unwrap(), HEAD_SHA); + assert_eq!( + reader + .read_utf8_file_at(HEAD_SHA, "five-bytes.txt", 5) + .await + .unwrap(), + "12345" + ); + } + + assert_eq!(twin.active_token_count().await, initial_tokens); + twin.shutdown().await; +} + +#[fabro_macros::e2e_test(twin)] +async fn expired_installation_token_keeps_credential_error_source() { + let twin = TwinGitHub::start(standard_app_state()).await; + let credentials = GitHubCredentials::Installation(InstallationToken { + token: "ghs_expired".to_string(), + expires_at: chrono::Utc::now() - chrono::Duration::minutes(1), + }); + + let error = GitHubRepositoryReader::open( + &GitHubContext::with_http_client( + &credentials, + &twin.base_url, + fabro_test::test_http_client(), + ), + &repository(), + ) + .await + .err() + .expect("expired installation token should fail"); + let RepositoryReadError::CredentialResolution { source } = error else { + panic!("expected credential resolution error"); + }; + assert!(source.to_string().contains("expired")); + + twin.shutdown().await; +} + +#[fabro_macros::e2e_test(twin)] +async fn branch_head_compatibility_wrapper_uses_repository_reader() { + let twin = TwinGitHub::start(standard_app_state()).await; + let credentials = github_credentials(); + let context = GitHubContext::with_http_client( + &credentials, + &twin.base_url, + fabro_test::test_http_client(), + ); + + assert_eq!( + branch_head_sha(&context, "acme", "widgets", "release") + .await + .unwrap(), + Some(HEAD_SHA.to_string()) + ); + assert_eq!( + branch_head_sha(&context, "acme", "widgets", "missing") + .await + .unwrap(), + None + ); + + twin.shutdown().await; +} + +#[fabro_macros::e2e_test(twin)] +async fn repository_reader_keeps_auth_and_malformed_sha_failures_distinct() { + let mut state = standard_app_state(); + state.set_repository_ref("acme", "widgets", "heads/malformed", "short-sha"); + let denied_token = + state.generate_access_token("42", 1, vec!["widgets".to_string()], serde_json::json!({})); + let twin = TwinGitHub::start(state).await; + + let invalid_credentials = GitHubCredentials::Pat("invalid-token".to_string()); + let invalid_reader = GitHubRepositoryReader::open( + &GitHubContext::with_http_client( + &invalid_credentials, + &twin.base_url, + fabro_test::test_http_client(), + ), + &repository(), + ) + .await + .unwrap(); + assert!(matches!( + invalid_reader.resolve_commit("heads/main").await, + Err(RepositoryReadError::AuthenticationRejected) + )); + + let denied_credentials = GitHubCredentials::Pat(denied_token); + let denied_reader = GitHubRepositoryReader::open( + &GitHubContext::with_http_client( + &denied_credentials, + &twin.base_url, + fabro_test::test_http_client(), + ), + &repository(), + ) + .await + .unwrap(); + assert!(matches!( + denied_reader.resolve_commit("heads/main").await, + Err(RepositoryReadError::PermissionDenied) + )); + + let app_credentials = github_credentials(); + let app_reader = GitHubRepositoryReader::open( + &GitHubContext::with_http_client( + &app_credentials, + &twin.base_url, + fabro_test::test_http_client(), + ), + &repository(), + ) + .await + .unwrap(); + assert!(matches!( + app_reader.resolve_commit("heads/malformed").await, + Err(RepositoryReadError::MalformedCommitSha) + )); + + twin.shutdown().await; +} diff --git a/lib/components/fabro-workflow/src/pipeline/pull_request.rs b/lib/components/fabro-workflow/src/pipeline/pull_request.rs index 1ab76a24b..03b78b309 100644 --- a/lib/components/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/components/fabro-workflow/src/pipeline/pull_request.rs @@ -539,9 +539,9 @@ async fn enable_auto_merge_if_requested( /// How many times to read the remote branch head before giving up. /// -/// `GET /repos/{owner}/{repo}/branches/{branch}` is replica-served, so shortly -/// after the push that publish just made it can still report the previous -/// commit — or 404 for a branch that is new on the remote. +/// Remote ref visibility can lag shortly after the push that publish just +/// made, so the exact-ref lookup can still report the previous commit — or +/// 404 for a branch that is new on the remote. const BRANCH_HEAD_ATTEMPTS: u32 = 3; const BRANCH_HEAD_RETRY_DELAY: Duration = Duration::from_millis(500); @@ -702,6 +702,9 @@ mod tests { use tokio::sync::RwLock as AsyncRwLock; use super::*; + + const FINAL_SHA: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const STALE_SHA: &str = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; use crate::event::{Event, append_event}; use crate::records::StageSummary; @@ -1488,14 +1491,14 @@ mod tests { #[tokio::test] async fn stale_remote_branch_is_rejected_before_pull_request_creation() { let payload = pr_content_json("Fix bug", "Narrative."); - let harness = setup_fallback_test_harness_with_branch_sha(&payload, "stale-sha").await; + let harness = setup_fallback_test_harness_with_branch_sha(&payload, STALE_SHA).await; let github_base_url = harness.github_server.url(""); let error = open_pull_request(OpenPullRequestRequest { github: fabro_github::GitHubContext::new(&harness.creds, &github_base_url), origin_url: "https://github.com/owner/repo.git", base_branch: "main", head_branch: "fabro/run/123", - expected_head_sha: "final-sha", + expected_head_sha: FINAL_SHA, goal: "Fix bug", diff: "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n", model: "claude-sonnet-4-20250514", @@ -1510,8 +1513,8 @@ mod tests { .await .expect_err("stale remote branch must prevent PR creation"); - assert!(error.contains("stale-sha")); - assert!(error.contains("final-sha")); + assert!(error.contains(STALE_SHA)); + assert!(error.contains(FINAL_SHA)); // The branch is re-read to ride out replica lag... httpmock::Mock::new(harness.branch_mock_id, &harness.github_server) .assert_calls_async(BRANCH_HEAD_ATTEMPTS as usize) @@ -1709,7 +1712,7 @@ mod tests { /// credential source, and a run store seeded with a non-empty /// `final_patch`. async fn setup_fallback_test_harness(openai_payload_text: &str) -> FallbackHarness { - setup_fallback_test_harness_with_branch_sha(openai_payload_text, "final-sha").await + setup_fallback_test_harness_with_branch_sha(openai_payload_text, FINAL_SHA).await } async fn setup_fallback_test_harness_with_branch_sha( @@ -1742,13 +1745,12 @@ mod tests { let branch_mock = github_server .mock_async(move |when, then| { when.method(GET) - .path("/repos/owner/repo/branches/fabro/run/123") - .header("authorization", "Bearer test-token"); + .path("/repos/owner/repo/commits/heads%2Ffabro%2Frun%2F123") + .header("authorization", "Bearer test-token") + .header("accept", "application/vnd.github.sha"); then.status(200) - .header("content-type", "application/json") - .json_body(serde_json::json!({ - "commit": { "sha": branch_sha } - })); + .header("content-type", "application/vnd.github.sha") + .body(branch_sha); }) .await; let github_mock = github_server @@ -1892,13 +1894,13 @@ mod tests { let payload = pr_content_json("Unused", "Unused."); let harness = setup_fallback_test_harness_with( &payload, - "final-sha", + FINAL_SHA, serde_json::json!([{ "html_url": "https://github.com/owner/repo/pull/7", "number": 7, "node_id": "PR_existing", "title": "Reconciled title", - "head": {"sha": "final-sha"} + "head": {"sha": FINAL_SHA} }]), ) .await; @@ -1911,7 +1913,7 @@ mod tests { origin_url: "https://github.com/owner/repo.git", base_branch: "main", head_branch: "fabro/run/123", - expected_head_sha: "final-sha", + expected_head_sha: FINAL_SHA, goal: "Fix telemetry leak", diff: "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n", model: "gpt-5.4", @@ -1959,7 +1961,7 @@ mod tests { origin_url: "https://github.com/owner/repo.git", base_branch: "main", head_branch: "fabro/run/123", - expected_head_sha: "final-sha", + expected_head_sha: FINAL_SHA, goal: "Fix telemetry leak\n\ndetails...", diff: "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n", model: "gpt-5.4", @@ -1996,7 +1998,7 @@ mod tests { origin_url: "https://github.com/owner/repo.git", base_branch: "main", head_branch: "fabro/run/123", - expected_head_sha: "final-sha", + expected_head_sha: FINAL_SHA, goal: &goal, diff: "diff --git a/src/lib.rs b/src/lib.rs\n+fn x() {}\n", model: "gpt-5.4", diff --git a/lib/foundation/fabro-test/src/lib.rs b/lib/foundation/fabro-test/src/lib.rs index 74a5f6727..b59d05f7c 100644 --- a/lib/foundation/fabro-test/src/lib.rs +++ b/lib/foundation/fabro-test/src/lib.rs @@ -2122,6 +2122,10 @@ impl TwinGitHub { pub async fn shutdown(self) { self.server.shutdown().await; } + + pub async fn active_token_count(&self) -> usize { + self.server.active_token_count().await + } } impl TwinOpenAi { diff --git a/test/twin/github/src/fixtures.rs b/test/twin/github/src/fixtures.rs index 183294b00..abc9b2160 100644 --- a/test/twin/github/src/fixtures.rs +++ b/test/twin/github/src/fixtures.rs @@ -213,15 +213,28 @@ impl FixtureState { state.repositories = self .repositories .into_iter() - .map(|repository| Repository { - owner: repository.owner, - name: repository.name, - branches: repository.branches, - default_branch: repository - .default_branch - .unwrap_or_else(|| "main".to_string()), - private: repository.private, - git_dir: None, + .map(|repository| { + let refs = repository + .branches + .into_iter() + .map(|branch| { + ( + format!("heads/{branch}"), + crate::state::DEFAULT_REPOSITORY_SHA.to_string(), + ) + }) + .collect(); + Repository { + owner: repository.owner, + name: repository.name, + refs, + files: HashMap::new(), + default_branch: repository + .default_branch + .unwrap_or_else(|| "main".to_string()), + private: repository.private, + git_dir: None, + } }) .collect(); diff --git a/test/twin/github/src/handlers/branches.rs b/test/twin/github/src/handlers/branches.rs index 1e0646d5f..36d4e1c48 100644 --- a/test/twin/github/src/handlers/branches.rs +++ b/test/twin/github/src/handlers/branches.rs @@ -55,16 +55,16 @@ pub async fn get_branch( }; } - // Find repository and check branch + // Find repository and check branch. for repo_data in &state.repositories { if repo_data.owner == owner && repo_data.name == repo { - if repo_data.branches.contains(&branch) { + if let Some(sha) = repo_data.refs.get(&format!("heads/{branch}")) { return ( StatusCode::OK, Json(serde_json::json!({ "name": branch, "commit": { - "sha": "abc123def456", + "sha": sha, }, "protected": false, })), @@ -92,7 +92,7 @@ pub async fn get_branch( #[cfg(test)] mod tests { use crate::server::TestServer; - use crate::state::{AppOptions, AppState}; + use crate::state::{AppOptions, AppState, DEFAULT_REPOSITORY_SHA}; use crate::test_support::{sign_test_jwt, test_http_client, test_rsa_private_key}; async fn get_installation_token( @@ -168,6 +168,7 @@ mod tests { assert_eq!(resp.status(), 200); let body: serde_json::Value = resp.json().await.unwrap(); assert_eq!(body["name"], "feature"); + assert_eq!(body["commit"]["sha"], DEFAULT_REPOSITORY_SHA); server.shutdown().await; } @@ -213,6 +214,7 @@ mod tests { assert_eq!(resp.status(), 200); let body: serde_json::Value = resp.json().await.unwrap(); assert_eq!(body["name"], "fabro/run/123"); + assert_eq!(body["commit"]["sha"], DEFAULT_REPOSITORY_SHA); server.shutdown().await; } diff --git a/test/twin/github/src/handlers/commits.rs b/test/twin/github/src/handlers/commits.rs new file mode 100644 index 000000000..2d092c920 --- /dev/null +++ b/test/twin/github/src/handlers/commits.rs @@ -0,0 +1,233 @@ +use axum::Json; +use axum::extract::{Path, State}; +use axum::http::header::{ACCEPT, CONTENT_TYPE}; +use axum::http::{HeaderMap, StatusCode}; +use axum::response::{IntoResponse, Response}; + +use crate::auth::{ + BearerTokenError, InstallationTokenAccessError, authorize_installation_token, + ensure_repo_permission, +}; +use crate::server::SharedState; +use crate::state::{AppState, PermissionLevel, TokenPermission}; + +const SHA_MEDIA_TYPE: &str = "application/vnd.github.sha"; + +/// GET /repos/{owner}/{repo}/commits/{ref} +pub async fn get_commit( + State(state): State, + Path((owner, repo, selector)): Path<(String, String, String)>, + headers: HeaderMap, +) -> Response { + let state = state.read().await; + if !accepts(&headers, SHA_MEDIA_TYPE) { + return message(StatusCode::NOT_ACCEPTABLE, "Not Acceptable"); + } + if let Err(error) = authorize(&headers, &state, &repo) { + return authorization_error(error); + } + + let Some(repository) = state + .repositories + .iter() + .find(|repository| repository.owner == owner && repository.name == repo) + else { + return message(StatusCode::NOT_FOUND, "Not Found"); + }; + + let sha = repository.refs.get(&selector).cloned().or_else(|| { + is_exact_commit_sha(&selector) + .then(|| selector.clone()) + .filter(|sha| { + repository.refs.values().any(|known| known == sha) + || repository.files.keys().any(|(known, _)| known == sha) + }) + }); + let Some(sha) = sha else { + return message(StatusCode::NOT_FOUND, "Not Found"); + }; + + (StatusCode::OK, [(CONTENT_TYPE, SHA_MEDIA_TYPE)], sha).into_response() +} + +#[derive(Clone, Copy)] +enum AuthorizationError { + MissingCredentials, + InvalidCredentials, + RepoNotAccessible, + PermissionDenied, +} + +fn authorize(headers: &HeaderMap, state: &AppState, repo: &str) -> Result<(), AuthorizationError> { + let token = authorize_installation_token(headers, state).map_err(|error| match error { + BearerTokenError::Missing => AuthorizationError::MissingCredentials, + BearerTokenError::Invalid => AuthorizationError::InvalidCredentials, + })?; + ensure_repo_permission( + &token, + repo, + TokenPermission::Contents, + PermissionLevel::Read, + ) + .map_err(|error| match error { + InstallationTokenAccessError::RepoNotAccessible => AuthorizationError::RepoNotAccessible, + InstallationTokenAccessError::PermissionDenied => AuthorizationError::PermissionDenied, + }) +} + +fn authorization_error(error: AuthorizationError) -> Response { + match error { + AuthorizationError::MissingCredentials => message(StatusCode::UNAUTHORIZED, "Unauthorized"), + AuthorizationError::InvalidCredentials => { + message(StatusCode::UNAUTHORIZED, "Bad credentials") + } + AuthorizationError::RepoNotAccessible => message(StatusCode::NOT_FOUND, "Not Found"), + AuthorizationError::PermissionDenied => message( + StatusCode::FORBIDDEN, + "Resource not accessible by integration", + ), + } +} + +fn accepts(headers: &HeaderMap, media_type: &str) -> bool { + headers.get_all(ACCEPT).iter().any(|value| { + value.to_str().is_ok_and(|value| { + value + .split(',') + .any(|candidate| candidate.trim() == media_type) + }) + }) +} + +fn is_exact_commit_sha(value: &str) -> bool { + value.len() == 40 && value.bytes().all(|byte| byte.is_ascii_hexdigit()) +} + +fn message(status: StatusCode, body: &'static str) -> Response { + (status, Json(serde_json::json!({ "message": body }))).into_response() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::server::TestServer; + use crate::state::AppState; + use crate::test_support::test_http_client; + + const HEAD_SHA: &str = "1111111111111111111111111111111111111111"; + const TAG_SHA: &str = "2222222222222222222222222222222222222222"; + + fn state_with_repository() -> (AppState, String) { + let mut state = AppState::new(); + state.add_repository("owner", "repo", vec!["main".to_string()], true); + state.set_repository_ref("owner", "repo", "heads/main", HEAD_SHA); + state.set_repository_ref("owner", "repo", "tags/v1.0.0", TAG_SHA); + let token = state.generate_access_token( + "app", + 1, + vec!["repo".to_string()], + serde_json::json!({ "contents": "read" }), + ); + (state, token) + } + + #[tokio::test] + async fn resolves_branch_tag_and_exact_sha_as_raw_bodies() { + let (state, token) = state_with_repository(); + let server = TestServer::start(state).await; + let client = test_http_client(); + + for (selector, expected) in [ + ("heads%2Fmain", HEAD_SHA), + ("tags%2Fv1.0.0", TAG_SHA), + (HEAD_SHA, HEAD_SHA), + ] { + let response = client + .get(format!( + "{}/repos/owner/repo/commits/{selector}", + server.url() + )) + .bearer_auth(&token) + .header(ACCEPT, SHA_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.headers().get(CONTENT_TYPE).unwrap(), + SHA_MEDIA_TYPE + ); + assert_eq!(response.text().await.unwrap(), expected); + } + + let response = client + .get(format!( + "{}/repos/owner/repo/commits/heads%2Fmissing", + server.url() + )) + .bearer_auth(&token) + .header(ACCEPT, SHA_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::NOT_FOUND); + + server.shutdown().await; + } + + #[tokio::test] + async fn enforces_auth_repository_permission_and_media_type() { + let (mut state, token) = state_with_repository(); + let other_repo_token = state.generate_access_token( + "app", + 1, + vec!["other".to_string()], + serde_json::json!({ "contents": "read" }), + ); + let denied_token = + state.generate_access_token("app", 1, vec!["repo".to_string()], serde_json::json!({})); + let server = TestServer::start(state).await; + let client = test_http_client(); + let url = format!("{}/repos/owner/repo/commits/heads%2Fmain", server.url()); + + let missing = client + .get(&url) + .header(ACCEPT, SHA_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(missing.status(), StatusCode::UNAUTHORIZED); + + let invalid = client + .get(&url) + .bearer_auth("invalid") + .header(ACCEPT, SHA_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(invalid.status(), StatusCode::UNAUTHORIZED); + + let hidden = client + .get(&url) + .bearer_auth(other_repo_token) + .header(ACCEPT, SHA_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(hidden.status(), StatusCode::NOT_FOUND); + + let denied = client + .get(&url) + .bearer_auth(denied_token) + .header(ACCEPT, SHA_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(denied.status(), StatusCode::FORBIDDEN); + + let unacceptable = client.get(&url).bearer_auth(token).send().await.unwrap(); + assert_eq!(unacceptable.status(), StatusCode::NOT_ACCEPTABLE); + + server.shutdown().await; + } +} diff --git a/test/twin/github/src/handlers/contents.rs b/test/twin/github/src/handlers/contents.rs new file mode 100644 index 000000000..d4a718817 --- /dev/null +++ b/test/twin/github/src/handlers/contents.rs @@ -0,0 +1,282 @@ +use axum::Json; +use axum::body::Body; +use axum::extract::{Path, Query, State}; +use axum::http::header::{ACCEPT, CONTENT_TYPE}; +use axum::http::{HeaderMap, StatusCode}; +use axum::response::{IntoResponse, Response}; +use serde::Deserialize; + +use crate::auth::{ + BearerTokenError, InstallationTokenAccessError, authorize_installation_token, + ensure_repo_permission, +}; +use crate::server::SharedState; +use crate::state::{AppState, PermissionLevel, TokenPermission}; + +const RAW_CONTENT_MEDIA_TYPE: &str = "application/vnd.github.raw+json"; + +#[derive(Deserialize)] +pub struct ContentQuery { + #[serde(rename = "ref")] + revision: String, +} + +/// GET /repos/{owner}/{repo}/contents/{path}?ref={sha} +pub async fn get_content( + State(state): State, + Path((owner, repo, path)): Path<(String, String, String)>, + Query(query): Query, + headers: HeaderMap, +) -> Response { + let state = state.read().await; + if !accepts(&headers, RAW_CONTENT_MEDIA_TYPE) { + return message(StatusCode::NOT_ACCEPTABLE, "Not Acceptable"); + } + if let Err(error) = authorize(&headers, &state, &repo) { + return authorization_error(error); + } + if !is_exact_commit_sha(&query.revision) { + return message(StatusCode::NOT_FOUND, "Not Found"); + } + + let Some(repository) = state + .repositories + .iter() + .find(|repository| repository.owner == owner && repository.name == repo) + else { + return message(StatusCode::NOT_FOUND, "Not Found"); + }; + + if let Some(contents) = repository + .files + .get(&(query.revision.clone(), path.clone())) + { + return ( + StatusCode::OK, + [(CONTENT_TYPE, RAW_CONTENT_MEDIA_TYPE)], + Body::from(contents.clone()), + ) + .into_response(); + } + + let directory_prefix = format!("{path}/"); + if repository.files.keys().any(|(sha, stored_path)| { + sha == &query.revision && stored_path.starts_with(&directory_prefix) + }) { + return (StatusCode::OK, Json(serde_json::json!([]))).into_response(); + } + + message(StatusCode::NOT_FOUND, "Not Found") +} + +#[derive(Clone, Copy)] +enum AuthorizationError { + MissingCredentials, + InvalidCredentials, + RepoNotAccessible, + PermissionDenied, +} + +fn authorize(headers: &HeaderMap, state: &AppState, repo: &str) -> Result<(), AuthorizationError> { + let token = authorize_installation_token(headers, state).map_err(|error| match error { + BearerTokenError::Missing => AuthorizationError::MissingCredentials, + BearerTokenError::Invalid => AuthorizationError::InvalidCredentials, + })?; + ensure_repo_permission( + &token, + repo, + TokenPermission::Contents, + PermissionLevel::Read, + ) + .map_err(|error| match error { + InstallationTokenAccessError::RepoNotAccessible => AuthorizationError::RepoNotAccessible, + InstallationTokenAccessError::PermissionDenied => AuthorizationError::PermissionDenied, + }) +} + +fn authorization_error(error: AuthorizationError) -> Response { + match error { + AuthorizationError::MissingCredentials => message(StatusCode::UNAUTHORIZED, "Unauthorized"), + AuthorizationError::InvalidCredentials => { + message(StatusCode::UNAUTHORIZED, "Bad credentials") + } + AuthorizationError::RepoNotAccessible => message(StatusCode::NOT_FOUND, "Not Found"), + AuthorizationError::PermissionDenied => message( + StatusCode::FORBIDDEN, + "Resource not accessible by integration", + ), + } +} + +fn accepts(headers: &HeaderMap, media_type: &str) -> bool { + headers.get_all(ACCEPT).iter().any(|value| { + value.to_str().is_ok_and(|value| { + value + .split(',') + .any(|candidate| candidate.trim() == media_type) + }) + }) +} + +fn is_exact_commit_sha(value: &str) -> bool { + value.len() == 40 && value.bytes().all(|byte| byte.is_ascii_hexdigit()) +} + +fn message(status: StatusCode, body: &'static str) -> Response { + (status, Json(serde_json::json!({ "message": body }))).into_response() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::server::TestServer; + use crate::state::AppState; + use crate::test_support::test_http_client; + + const SHA: &str = "1111111111111111111111111111111111111111"; + + fn state_with_file() -> (AppState, String) { + let mut state = AppState::new(); + state.add_repository("owner", "repo", vec!["main".to_string()], true); + state.set_repository_ref("owner", "repo", "heads/main", SHA); + state.add_repository_file("owner", "repo", SHA, "dir/file.bin", vec![0, 0xff, 1]); + let token = state.generate_access_token( + "app", + 1, + vec!["repo".to_string()], + serde_json::json!({ "contents": "read" }), + ); + (state, token) + } + + #[tokio::test] + async fn serves_only_exact_sha_files_and_marks_directories_as_json() { + let (state, token) = state_with_file(); + let server = TestServer::start(state).await; + let client = test_http_client(); + + let response = client + .get(format!( + "{}/repos/owner/repo/contents/dir/file.bin?ref={SHA}", + server.url() + )) + .bearer_auth(&token) + .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.headers().get(CONTENT_TYPE).unwrap(), + RAW_CONTENT_MEDIA_TYPE + ); + assert_eq!(response.bytes().await.unwrap().as_ref(), &[0, 0xff, 1]); + + let directory = client + .get(format!( + "{}/repos/owner/repo/contents/dir?ref={SHA}", + server.url() + )) + .bearer_auth(&token) + .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(directory.status(), StatusCode::OK); + assert_eq!( + directory.headers().get(CONTENT_TYPE).unwrap(), + "application/json" + ); + + for suffix in [ + "dir/file.bin?ref=heads%2Fmain", + "dir/file.bin?ref=2222222222222222222222222222222222222222", + &format!("missing.bin?ref={SHA}"), + ] { + let response = client + .get(format!( + "{}/repos/owner/repo/contents/{suffix}", + server.url() + )) + .bearer_auth(&token) + .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + + let missing_ref = client + .get(format!( + "{}/repos/owner/repo/contents/dir/file.bin", + server.url() + )) + .bearer_auth(&token) + .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(missing_ref.status(), StatusCode::BAD_REQUEST); + + server.shutdown().await; + } + + #[tokio::test] + async fn enforces_auth_repository_permission_and_media_type() { + let (mut state, token) = state_with_file(); + let other_repo_token = state.generate_access_token( + "app", + 1, + vec!["other".to_string()], + serde_json::json!({ "contents": "read" }), + ); + let denied_token = + state.generate_access_token("app", 1, vec!["repo".to_string()], serde_json::json!({})); + let server = TestServer::start(state).await; + let client = test_http_client(); + let url = format!( + "{}/repos/owner/repo/contents/dir/file.bin?ref={SHA}", + server.url() + ); + + let missing = client + .get(&url) + .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(missing.status(), StatusCode::UNAUTHORIZED); + + let invalid = client + .get(&url) + .bearer_auth("invalid") + .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(invalid.status(), StatusCode::UNAUTHORIZED); + + let hidden = client + .get(&url) + .bearer_auth(other_repo_token) + .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(hidden.status(), StatusCode::NOT_FOUND); + + let denied = client + .get(&url) + .bearer_auth(denied_token) + .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) + .send() + .await + .unwrap(); + assert_eq!(denied.status(), StatusCode::FORBIDDEN); + + let unacceptable = client.get(&url).bearer_auth(token).send().await.unwrap(); + assert_eq!(unacceptable.status(), StatusCode::NOT_ACCEPTABLE); + + server.shutdown().await; + } +} diff --git a/test/twin/github/src/handlers/mod.rs b/test/twin/github/src/handlers/mod.rs index 30912c764..8ad15f34d 100644 --- a/test/twin/github/src/handlers/mod.rs +++ b/test/twin/github/src/handlers/mod.rs @@ -1,5 +1,7 @@ pub mod app; pub mod branches; +pub mod commits; +pub mod contents; pub mod git; pub mod graphql; pub mod installations; @@ -36,6 +38,16 @@ pub fn build_router(state: SharedState) -> Router { "/repos/{owner}/{repo}/branches/{*branch}", get(branches::get_branch), ) + // Repository-reader endpoints. Refs and paths may both contain `/`, + // so each is captured as the remainder of its route. + .route( + "/repos/{owner}/{repo}/commits/{*selector}", + get(commits::get_commit), + ) + .route( + "/repos/{owner}/{repo}/contents/{*path}", + get(contents::get_content), + ) // Pull request endpoints .route( "/repos/{owner}/{repo}/pulls", diff --git a/test/twin/github/src/server.rs b/test/twin/github/src/server.rs index a4a2b36ff..c6cf410c1 100644 --- a/test/twin/github/src/server.rs +++ b/test/twin/github/src/server.rs @@ -13,6 +13,7 @@ pub type SharedState = Arc>; /// A running test server instance. pub struct TestServer { url: String, + state: SharedState, shutdown_tx: Option>, handle: Option>, _git_root: TempDir, // Kept alive for the server's lifetime; cleaned up on drop @@ -25,7 +26,7 @@ impl TestServer { init_git_repos(&mut state, git_root.path()).expect("failed to initialize git repos"); let shared_state: SharedState = Arc::new(RwLock::new(state)); - let router = build_router(shared_state); + let router = build_router(shared_state.clone()); let listener = TcpListener::bind("127.0.0.1:0") .await @@ -49,6 +50,7 @@ impl TestServer { Self { url, + state: shared_state, shutdown_tx: Some(shutdown_tx), handle: Some(handle), _git_root: git_root, @@ -59,6 +61,10 @@ impl TestServer { &self.url } + pub async fn active_token_count(&self) -> usize { + self.state.read().await.active_tokens.len() + } + pub async fn shutdown(mut self) { if let Some(tx) = self.shutdown_tx.take() { let _ = tx.send(()); diff --git a/test/twin/github/src/state.rs b/test/twin/github/src/state.rs index c75c9bfaf..3a0fe92ce 100644 --- a/test/twin/github/src/state.rs +++ b/test/twin/github/src/state.rs @@ -40,12 +40,15 @@ pub struct Installation { pub struct Repository { pub owner: String, pub name: String, - pub branches: Vec, + pub refs: HashMap, + pub files: HashMap<(String, String), Vec>, pub default_branch: String, pub private: bool, pub git_dir: Option, } +pub const DEFAULT_REPOSITORY_SHA: &str = "0123456789abcdef0123456789abcdef01234567"; + /// A pull request. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PullRequest { @@ -390,16 +393,55 @@ impl AppState { branches: Vec, private: bool, ) { + let refs = branches + .into_iter() + .map(|branch| { + ( + format!("heads/{branch}"), + DEFAULT_REPOSITORY_SHA.to_string(), + ) + }) + .collect(); self.repositories.push(Repository { owner: owner.to_string(), name: name.to_string(), - branches, + refs, + files: HashMap::new(), default_branch: "main".to_string(), private, git_dir: None, }); } + pub fn set_repository_ref(&mut self, owner: &str, name: &str, selector: &str, sha: &str) { + let repository = self + .repositories + .iter_mut() + .find(|repository| repository.owner == owner && repository.name == name) + .expect("repository fixture should exist before adding a ref"); + repository + .refs + .insert(selector.to_string(), sha.to_string()); + } + + pub fn add_repository_file( + &mut self, + owner: &str, + name: &str, + sha: &str, + path: &str, + contents: impl Into>, + ) { + let repository = self + .repositories + .iter_mut() + .find(|repository| repository.owner == owner && repository.name == name) + .expect("repository fixture should exist before adding a file"); + repository + .files + .insert((sha.to_string(), path.to_string()), contents.into()); + } + pub fn find_installation(&self, owner: &str, repo: &str) -> Option<&Installation> { self.installations .iter()