mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
Reduce duplication in GitHub repository reads
Share the twin's handler scaffolding, collapse the reader's parallel URL
and error machinery, and resolve branch-head credentials once per verify.
Twin GitHub server:
- Add handlers/support.rs holding the response envelope, installation-token
authorization, Accept matching, and commit-SHA checks. The commits and
contents handlers carried byte-identical copies of all six items, and
pulls.rs had its own copy of the two response mappers.
- Add AppState::find_repository and repository_mut, replacing four
open-coded repository lookups.
- Add head_refs and heads_selector so the heads/{branch} mapping is
spelled once instead of in add_repository, the fixture conversion, and
the branch handler.
- Key repository files by commit SHA then path rather than by a
(String, String) tuple, which drops two allocations and two full-map
scans per content request.
Repository reader:
- Use DisplaySafeUrl, which removes the file-scope disallowed_types
suppression and the direct url dependency. The suppression covered the
whole module and everything later added to it.
- Build {api_base}/repos/{owner}/{repo} once when the session opens, so
the URL builders become infallible methods and three unreachable
cannot-be-a-base error paths disappear.
- Collapse the per-operation NotFound and Unavailable variants into ones
carrying the operation, derive its rendering with strum, and mark the
error non_exhaustive.
- Return the status classification as one Err(match), size the body
buffer from Content-Length, and lowercase the resolved SHA in place.
Pull request pipeline:
- Open one reader before the branch-head retry loop instead of once per
attempt. With App credentials each attempt previously minted a fresh
installation token, costing two extra round trips per retry. Only the
ref lookup is retried now; credential failures surface immediately.
Tests keep their coverage: one helper opens readers across eight call
sites, and the repository file fixtures become a table.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c46817bf26
commit
8dfda7ede0
14 changed files with 443 additions and 550 deletions
2
Cargo.lock
generated
2
Cargo.lock
generated
|
|
@ -2665,11 +2665,11 @@ dependencies = [
|
|||
"jsonwebtoken",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"strum 0.28.0",
|
||||
"thiserror 2.0.18",
|
||||
"tokio",
|
||||
"tracing",
|
||||
"tracing-subscriber",
|
||||
"url",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ tracing.workspace = true
|
|||
tokio = { workspace = true }
|
||||
base64.workspace = true
|
||||
thiserror.workspace = true
|
||||
url.workspace = true
|
||||
strum.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
fabro-macros = { path = "../../foundation/fabro-macros" }
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ use tokio::process::Command;
|
|||
|
||||
mod repository_reader;
|
||||
|
||||
pub use repository_reader::{GitHubRepositoryReader, RepositoryReadError};
|
||||
pub use repository_reader::{GitHubRepositoryReader, Operation, RepositoryReadError};
|
||||
|
||||
pub const GITHUB_API_BASE_URL: &str = "https://api.github.com";
|
||||
|
||||
|
|
@ -1011,7 +1011,7 @@ pub async fn branch_head_sha(
|
|||
.context("Failed to read remote branch head")?;
|
||||
match reader.resolve_commit(&format!("heads/{branch}")).await {
|
||||
Ok(sha) => Ok(Some(sha)),
|
||||
Err(RepositoryReadError::RevisionNotFound) => Ok(None),
|
||||
Err(RepositoryReadError::NotFound { .. }) => Ok(None),
|
||||
Err(error) => Err(anyhow::Error::new(error).context("Failed to read remote branch head")),
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,10 +1,6 @@
|
|||
#![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_http::{HeaderMap, HttpClient, Response, StatusCode};
|
||||
use fabro_redact::{DisplaySafeUrl, DisplaySafeUrlError};
|
||||
use fabro_types::{GitHubRepositorySlug, repository};
|
||||
|
||||
use crate::GitHubContext;
|
||||
|
|
@ -13,14 +9,24 @@ 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;
|
||||
|
||||
/// Which repository read a failure came from. Carried on the errors that can
|
||||
/// arise from either, so one status classification serves both.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, strum::Display)]
|
||||
#[strum(serialize_all = "lowercase")]
|
||||
pub enum Operation {
|
||||
Revision,
|
||||
Content,
|
||||
}
|
||||
|
||||
/// Failures while opening or using a repository-scoped GitHub reader.
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[non_exhaustive]
|
||||
pub enum RepositoryReadError {
|
||||
#[error("invalid GitHub API base URL ({reason})")]
|
||||
InvalidApiBaseUrl {
|
||||
reason: &'static str,
|
||||
#[source]
|
||||
source: Option<url::ParseError>,
|
||||
source: Option<DisplaySafeUrlError>,
|
||||
},
|
||||
#[error("invalid GitHub ref selector")]
|
||||
InvalidRefSelector,
|
||||
|
|
@ -44,28 +50,26 @@ pub enum RepositoryReadError {
|
|||
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,
|
||||
/// GitHub returned 404. GitHub may also use 404 to hide a private resource
|
||||
/// that these credentials cannot access, so this means "not observable"
|
||||
/// rather than "does not exist".
|
||||
#[error("GitHub {operation} was not observable")]
|
||||
NotFound { operation: Operation },
|
||||
#[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} is unavailable (status {status})")]
|
||||
Unavailable {
|
||||
operation: Operation,
|
||||
status: u16,
|
||||
},
|
||||
#[error("GitHub {operation} service is unavailable (status {status})")]
|
||||
UpstreamUnavailable {
|
||||
operation: &'static str,
|
||||
operation: Operation,
|
||||
status: u16,
|
||||
},
|
||||
#[error("unexpected GitHub {operation} status {status}")]
|
||||
UnexpectedStatus {
|
||||
operation: &'static str,
|
||||
operation: Operation,
|
||||
status: u16,
|
||||
},
|
||||
#[error("GitHub response exceeded the {max_bytes}-byte limit")]
|
||||
|
|
@ -80,26 +84,13 @@ pub enum RepositoryReadError {
|
|||
}
|
||||
|
||||
/// An authenticated read session scoped to one GitHub repository.
|
||||
///
|
||||
/// Opening resolves credentials once; every read reuses that token.
|
||||
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",
|
||||
}
|
||||
}
|
||||
client: HttpClient,
|
||||
/// `{api_base}/repos/{owner}/{repo}`, validated and built once at open.
|
||||
repository_base: DisplaySafeUrl,
|
||||
bearer_token: String,
|
||||
}
|
||||
|
||||
impl GitHubRepositoryReader {
|
||||
|
|
@ -111,14 +102,14 @@ impl GitHubRepositoryReader {
|
|||
let client = ctx
|
||||
.http_client()
|
||||
.map_err(|source| RepositoryReadError::RequestTransport { source })?;
|
||||
let (api_base, normalized_base) = parse_api_base(ctx.base_url)?;
|
||||
let api_base = parse_api_base(ctx.base_url)?;
|
||||
let bearer_token = ctx
|
||||
.creds
|
||||
.resolve_bearer_token(
|
||||
&client,
|
||||
repository.owner(),
|
||||
repository.repo(),
|
||||
&normalized_base,
|
||||
api_base.as_str().trim_end_matches('/'),
|
||||
serde_json::json!({ "contents": "read" }),
|
||||
)
|
||||
.await
|
||||
|
|
@ -126,8 +117,7 @@ impl GitHubRepositoryReader {
|
|||
|
||||
Ok(Self {
|
||||
client,
|
||||
api_base,
|
||||
repository: repository.clone(),
|
||||
repository_base: repository_base(&api_base, repository),
|
||||
bearer_token,
|
||||
})
|
||||
}
|
||||
|
|
@ -138,19 +128,9 @@ impl GitHubRepositoryReader {
|
|||
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()),
|
||||
})?;
|
||||
|
||||
.send(&self.commit_url(selector), SHA_MEDIA_TYPE)
|
||||
.await?;
|
||||
classify_status(response.status(), response.headers(), Operation::Revision)?;
|
||||
let bytes = collect_bounded(response, MAX_SHA_RESPONSE_BYTES).await?;
|
||||
parse_resolved_commit_sha(bytes)
|
||||
|
|
@ -168,24 +148,8 @@ impl GitHubRepositoryReader {
|
|||
}
|
||||
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()),
|
||||
})?;
|
||||
|
||||
let url = self.content_url(commit_sha, canonical_repo_path);
|
||||
let response = self.send(&url, RAW_CONTENT_MEDIA_TYPE).await?;
|
||||
classify_status(response.status(), response.headers(), Operation::Content)?;
|
||||
if !has_raw_content_media_type(response.headers()) {
|
||||
return Err(RepositoryReadError::ContentNotFile);
|
||||
|
|
@ -195,14 +159,61 @@ impl GitHubRepositoryReader {
|
|||
source: source.utf8_error(),
|
||||
})
|
||||
}
|
||||
|
||||
/// `{repository_base}/commits/{selector}`, with the selector's slashes
|
||||
/// escaped so a `heads/a/b` ref stays one path segment.
|
||||
fn commit_url(&self, selector: &str) -> DisplaySafeUrl {
|
||||
let mut url = self.repository_base.clone();
|
||||
url.path_segments_mut()
|
||||
.expect("repository base is a hierarchical URL")
|
||||
.push("commits")
|
||||
.push(selector);
|
||||
url
|
||||
}
|
||||
|
||||
/// `{repository_base}/contents/{path}?ref={commit_sha}`, with each path
|
||||
/// component escaped individually so separators survive as separators.
|
||||
fn content_url(&self, commit_sha: &str, path: &str) -> DisplaySafeUrl {
|
||||
let mut url = self.repository_base.clone();
|
||||
{
|
||||
let mut segments = url
|
||||
.path_segments_mut()
|
||||
.expect("repository base is a hierarchical URL");
|
||||
segments.push("contents");
|
||||
for component in path.split('/') {
|
||||
segments.push(component);
|
||||
}
|
||||
}
|
||||
url.query_pairs_mut().append_pair("ref", commit_sha);
|
||||
url
|
||||
}
|
||||
|
||||
/// Issues one authenticated GET asking for exactly `media_type`.
|
||||
async fn send(
|
||||
&self,
|
||||
url: &DisplaySafeUrl,
|
||||
media_type: &str,
|
||||
) -> Result<Response, RepositoryReadError> {
|
||||
self.client
|
||||
.get(url.raw_string())
|
||||
.bearer_auth(&self.bearer_token)
|
||||
.header(USER_AGENT, "fabro")
|
||||
.header(ACCEPT, media_type)
|
||||
.send()
|
||||
.await
|
||||
.map_err(|source| RepositoryReadError::RequestTransport {
|
||||
source: anyhow::Error::new(source.without_url()),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_api_base(base_url: &str) -> Result<(Url, String), RepositoryReadError> {
|
||||
let mut url =
|
||||
Url::parse(base_url).map_err(|source| RepositoryReadError::InvalidApiBaseUrl {
|
||||
fn parse_api_base(base_url: &str) -> Result<DisplaySafeUrl, RepositoryReadError> {
|
||||
let mut url = DisplaySafeUrl::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"));
|
||||
}
|
||||
|
|
@ -222,14 +233,10 @@ fn parse_api_base(base_url: &str) -> Result<(Url, String), RepositoryReadError>
|
|||
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))
|
||||
url.path_segments_mut()
|
||||
.map_err(|()| invalid_base("cannot be a base"))?
|
||||
.pop_if_empty();
|
||||
Ok(url)
|
||||
}
|
||||
|
||||
const fn invalid_base(reason: &'static str) -> RepositoryReadError {
|
||||
|
|
@ -239,62 +246,30 @@ const fn invalid_base(reason: &'static str) -> RepositoryReadError {
|
|||
}
|
||||
}
|
||||
|
||||
fn commit_url(
|
||||
base: &Url,
|
||||
repository: &GitHubRepositorySlug,
|
||||
selector: &str,
|
||||
) -> Result<Url, RepositoryReadError> {
|
||||
let mut url = base.clone();
|
||||
let mut segments = url
|
||||
.path_segments_mut()
|
||||
.map_err(|()| invalid_base("cannot be a base"))?;
|
||||
segments
|
||||
/// `{api_base}/repos/{owner}/{repo}`, the prefix every read extends.
|
||||
///
|
||||
/// Infallible: `parse_api_base` has already rejected cannot-be-a-base URLs.
|
||||
fn repository_base(api_base: &DisplaySafeUrl, repository: &GitHubRepositorySlug) -> DisplaySafeUrl {
|
||||
let mut url = api_base.clone();
|
||||
url.path_segments_mut()
|
||||
.expect("api base is a hierarchical URL")
|
||||
.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<Url, RepositoryReadError> {
|
||||
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)
|
||||
.push(repository.repo());
|
||||
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<u8>) -> Result<String, RepositoryReadError> {
|
||||
fn parse_resolved_commit_sha(mut bytes: Vec<u8>) -> Result<String, RepositoryReadError> {
|
||||
if !is_exact_commit_sha(&bytes) {
|
||||
return Err(RepositoryReadError::MalformedCommitSha);
|
||||
}
|
||||
Ok(bytes
|
||||
.into_iter()
|
||||
.map(|byte| char::from(byte.to_ascii_lowercase()))
|
||||
.collect())
|
||||
bytes.make_ascii_lowercase();
|
||||
String::from_utf8(bytes).map_err(|_| RepositoryReadError::MalformedCommitSha)
|
||||
}
|
||||
|
||||
fn validate_repository_path(path: &str) -> Result<(), RepositoryReadError> {
|
||||
|
|
@ -343,31 +318,32 @@ fn classify_status(
|
|||
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 })
|
||||
let status_code = status.as_u16();
|
||||
Err(match status {
|
||||
StatusCode::UNAUTHORIZED => RepositoryReadError::AuthenticationRejected,
|
||||
StatusCode::FORBIDDEN if is_rate_limited(headers) => RepositoryReadError::RateLimited {
|
||||
status: status_code,
|
||||
},
|
||||
StatusCode::FORBIDDEN => RepositoryReadError::PermissionDenied,
|
||||
StatusCode::TOO_MANY_REQUESTS => RepositoryReadError::RateLimited {
|
||||
status: status_code,
|
||||
},
|
||||
StatusCode::NOT_FOUND => RepositoryReadError::NotFound { operation },
|
||||
StatusCode::CONFLICT | StatusCode::UNPROCESSABLE_ENTITY => {
|
||||
RepositoryReadError::Unavailable {
|
||||
operation,
|
||||
status: 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),
|
||||
status if status.is_server_error() => RepositoryReadError::UpstreamUnavailable {
|
||||
operation,
|
||||
status: status_code,
|
||||
},
|
||||
StatusCode::CONFLICT | StatusCode::UNPROCESSABLE_ENTITY => match operation {
|
||||
Operation::Revision => Err(RepositoryReadError::RevisionUnavailable { status: code }),
|
||||
Operation::Content => Err(RepositoryReadError::ContentUnavailable { status: code }),
|
||||
_ => RepositoryReadError::UnexpectedStatus {
|
||||
operation,
|
||||
status: 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 {
|
||||
|
|
@ -400,14 +376,18 @@ async fn collect_bounded(
|
|||
mut response: Response,
|
||||
max_bytes: usize,
|
||||
) -> Result<Vec<u8>, RepositoryReadError> {
|
||||
if response
|
||||
.content_length()
|
||||
.is_some_and(|length| length > max_bytes as u64)
|
||||
{
|
||||
return Err(RepositoryReadError::BodyTooLarge { max_bytes });
|
||||
}
|
||||
// A declared length over the cap fails before any body is read; otherwise it
|
||||
// sizes the buffer exactly. Chunked responses start empty and grow under the
|
||||
// same bound, enforced per chunk below.
|
||||
let capacity = match response.content_length() {
|
||||
Some(length) => usize::try_from(length)
|
||||
.ok()
|
||||
.filter(|length| *length <= max_bytes)
|
||||
.ok_or(RepositoryReadError::BodyTooLarge { max_bytes })?,
|
||||
None => 0,
|
||||
};
|
||||
|
||||
let mut bytes = Vec::new();
|
||||
let mut bytes = Vec::with_capacity(capacity);
|
||||
while let Some(chunk) =
|
||||
response
|
||||
.chunk()
|
||||
|
|
@ -436,13 +416,19 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn api_base_preserves_prefix_and_normalizes_trailing_slash() {
|
||||
let (base, normalized) = parse_api_base("https://ghe.example/api/v3/").unwrap();
|
||||
let base = 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");
|
||||
assert_eq!(
|
||||
base.as_str().trim_end_matches('/'),
|
||||
"https://ghe.example/api/v3"
|
||||
);
|
||||
|
||||
let (root, normalized) = parse_api_base("https://api.github.com/").unwrap();
|
||||
let root = parse_api_base("https://api.github.com/").unwrap();
|
||||
assert_eq!(root.as_str(), "https://api.github.com/");
|
||||
assert_eq!(normalized, "https://api.github.com");
|
||||
assert_eq!(
|
||||
root.as_str().trim_end_matches('/'),
|
||||
"https://api.github.com"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -480,16 +466,26 @@ mod tests {
|
|||
assert!(error.source().is_some());
|
||||
}
|
||||
|
||||
/// Builds the URL a read would request without issuing it.
|
||||
fn reader_at(base_url: &str) -> GitHubRepositoryReader {
|
||||
let api_base = parse_api_base(base_url).unwrap();
|
||||
GitHubRepositoryReader {
|
||||
client: fabro_http::test_http_client().unwrap(),
|
||||
repository_base: repository_base(&api_base, &repository()),
|
||||
bearer_token: "sentinel-token".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[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();
|
||||
let reader = reader_at("https://ghe.example/api/v3");
|
||||
let commit = reader.commit_url("heads/fabro/run/123");
|
||||
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();
|
||||
let content = reader.content_url(SHA, "dir/a b/%2F/#?é.toml");
|
||||
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"
|
||||
|
|
@ -498,12 +494,7 @@ mod tests {
|
|||
|
||||
#[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(),
|
||||
};
|
||||
let reader = reader_at("http://127.0.0.1:1");
|
||||
|
||||
for invalid in ["", " main", "heads//main", "tags/v1.lock"] {
|
||||
assert!(matches!(
|
||||
|
|
@ -519,10 +510,15 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn canonical_ref_selectors_reach_url_construction() {
|
||||
let (base, _) = parse_api_base("https://api.github.com").unwrap();
|
||||
let reader = reader_at("https://api.github.com");
|
||||
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());
|
||||
assert!(
|
||||
reader
|
||||
.commit_url(selector)
|
||||
.as_str()
|
||||
.starts_with("https://api.github.com/repos/owner/repo/commits/")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -610,11 +606,15 @@ mod tests {
|
|||
));
|
||||
assert!(matches!(
|
||||
classify_status(StatusCode::NOT_FOUND, &empty, Operation::Revision),
|
||||
Err(RepositoryReadError::RevisionNotFound)
|
||||
Err(RepositoryReadError::NotFound {
|
||||
operation: Operation::Revision,
|
||||
})
|
||||
));
|
||||
assert!(matches!(
|
||||
classify_status(StatusCode::NOT_FOUND, &empty, Operation::Content),
|
||||
Err(RepositoryReadError::ContentNotFound)
|
||||
Err(RepositoryReadError::NotFound {
|
||||
operation: Operation::Content,
|
||||
})
|
||||
));
|
||||
assert!(matches!(
|
||||
classify_status(StatusCode::FORBIDDEN, &empty, Operation::Content),
|
||||
|
|
@ -638,44 +638,31 @@ mod tests {
|
|||
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::CONFLICT, &empty, Operation::Revision),
|
||||
Err(RepositoryReadError::Unavailable {
|
||||
operation: Operation::Revision,
|
||||
status: 409,
|
||||
})
|
||||
));
|
||||
assert!(matches!(
|
||||
classify_status(StatusCode::UNPROCESSABLE_ENTITY, &empty, Operation::Content),
|
||||
Err(RepositoryReadError::Unavailable {
|
||||
operation: Operation::Content,
|
||||
status: 422,
|
||||
})
|
||||
));
|
||||
assert!(matches!(
|
||||
classify_status(StatusCode::BAD_GATEWAY, &empty, Operation::Content),
|
||||
Err(RepositoryReadError::UpstreamUnavailable {
|
||||
operation: "content read",
|
||||
operation: Operation::Content,
|
||||
status: 502,
|
||||
})
|
||||
));
|
||||
assert!(matches!(
|
||||
classify_status(StatusCode::IM_A_TEAPOT, &empty, Operation::Revision),
|
||||
Err(RepositoryReadError::UnexpectedStatus {
|
||||
operation: "revision lookup",
|
||||
operation: Operation::Revision,
|
||||
status: 418,
|
||||
})
|
||||
));
|
||||
|
|
|
|||
|
|
@ -45,36 +45,19 @@ fn standard_app_state() -> GitHubAppState {
|
|||
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(),
|
||||
);
|
||||
for (path, contents) in [
|
||||
(
|
||||
".fabro/workflows/build/workflow.toml",
|
||||
b"name = \"build\"\n".to_vec(),
|
||||
),
|
||||
("dir/hello world#.txt", b"hello\n".to_vec()),
|
||||
("invalid.txt", vec![0xff, 0xfe]),
|
||||
("empty.txt", Vec::new()),
|
||||
("five-bytes.txt", b"12345".to_vec()),
|
||||
("nested/a b/%/é.toml", "unicode: 🦀\n".as_bytes().to_vec()),
|
||||
] {
|
||||
state.add_repository_file("acme", "widgets", HEAD_SHA, path, contents);
|
||||
}
|
||||
state
|
||||
}
|
||||
|
||||
|
|
@ -82,6 +65,18 @@ fn repository() -> GitHubRepositorySlug {
|
|||
GitHubRepositorySlug::try_new("acme/widgets").expect("test repository slug should be valid")
|
||||
}
|
||||
|
||||
/// Open a reader for the standard repository against `base_url`.
|
||||
async fn open_reader(
|
||||
base_url: &str,
|
||||
credentials: &GitHubCredentials,
|
||||
) -> Result<GitHubRepositoryReader, RepositoryReadError> {
|
||||
GitHubRepositoryReader::open(
|
||||
&GitHubContext::with_http_client(credentials, base_url, fabro_test::test_http_client()),
|
||||
&repository(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[fabro_macros::e2e_test(twin)]
|
||||
async fn create_and_get_pull_request() {
|
||||
let twin = TwinGitHub::start(standard_app_state()).await;
|
||||
|
|
@ -279,12 +274,7 @@ async fn repository_reader_reuses_one_app_token_for_ref_and_file_reads() {
|
|||
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();
|
||||
let reader = open_reader(&base_url, &creds).await.unwrap();
|
||||
assert_eq!(twin.active_token_count().await, 1);
|
||||
|
||||
assert_eq!(reader.resolve_commit("heads/main").await.unwrap(), HEAD_SHA);
|
||||
|
|
@ -325,12 +315,7 @@ async fn repository_reader_reuses_one_app_token_for_ref_and_file_reads() {
|
|||
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();
|
||||
let reader = open_reader(&twin.base_url, &creds).await.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
reader.resolve_commit("heads/release").await.unwrap(),
|
||||
|
|
@ -351,7 +336,9 @@ async fn repository_reader_preserves_exact_ref_namespaces() {
|
|||
);
|
||||
assert!(matches!(
|
||||
reader.resolve_commit("heads/missing").await,
|
||||
Err(RepositoryReadError::RevisionNotFound)
|
||||
Err(RepositoryReadError::NotFound {
|
||||
operation: fabro_github::Operation::Revision,
|
||||
})
|
||||
));
|
||||
|
||||
twin.shutdown().await;
|
||||
|
|
@ -361,12 +348,7 @@ async fn repository_reader_preserves_exact_ref_namespaces() {
|
|||
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();
|
||||
let reader = open_reader(&twin.base_url, &creds).await.unwrap();
|
||||
|
||||
assert!(matches!(
|
||||
reader
|
||||
|
|
@ -399,7 +381,9 @@ async fn repository_reader_classifies_file_failures_without_retaining_bytes() {
|
|||
reader
|
||||
.read_utf8_file_at(HEAD_SHA, "missing.txt", 1024)
|
||||
.await,
|
||||
Err(RepositoryReadError::ContentNotFound)
|
||||
Err(RepositoryReadError::NotFound {
|
||||
operation: fabro_github::Operation::Content,
|
||||
})
|
||||
));
|
||||
assert_eq!(
|
||||
reader
|
||||
|
|
@ -444,16 +428,7 @@ async fn static_repository_credentials_do_not_mint_tokens() {
|
|||
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();
|
||||
let reader = open_reader(&twin.base_url, &credentials).await.unwrap();
|
||||
assert_eq!(reader.resolve_commit("heads/main").await.unwrap(), HEAD_SHA);
|
||||
assert_eq!(
|
||||
reader
|
||||
|
|
@ -476,17 +451,10 @@ async fn expired_installation_token_keeps_credential_error_source() {
|
|||
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 error = open_reader(&twin.base_url, &credentials)
|
||||
.await
|
||||
.err()
|
||||
.expect("expired installation token should fail");
|
||||
let RepositoryReadError::CredentialResolution { source } = error else {
|
||||
panic!("expected credential resolution error");
|
||||
};
|
||||
|
|
@ -530,48 +498,25 @@ async fn repository_reader_keeps_auth_and_malformed_sha_failures_distinct() {
|
|||
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();
|
||||
let invalid_reader = open_reader(&twin.base_url, &invalid_credentials)
|
||||
.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();
|
||||
let denied_reader = open_reader(&twin.base_url, &denied_credentials)
|
||||
.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();
|
||||
let app_reader = open_reader(&twin.base_url, &app_credentials).await.unwrap();
|
||||
assert!(matches!(
|
||||
app_reader.resolve_commit("heads/malformed").await,
|
||||
Err(RepositoryReadError::MalformedCommitSha)
|
||||
|
|
|
|||
|
|
@ -3,14 +3,16 @@ use std::sync::{Arc, LazyLock};
|
|||
use std::time::Duration;
|
||||
|
||||
use fabro_auth::CredentialSource;
|
||||
use fabro_github::{self as github_app, ssh_url_to_https};
|
||||
use fabro_github::{
|
||||
self as github_app, GitHubRepositoryReader, RepositoryReadError, ssh_url_to_https,
|
||||
};
|
||||
use fabro_graphviz::parser;
|
||||
use fabro_llm::client::Client;
|
||||
use fabro_llm::generate::{GenerateParams, generate_object};
|
||||
use fabro_model::{Catalog, ProviderId};
|
||||
use fabro_store::RunProjection;
|
||||
use fabro_types::PullRequestLink;
|
||||
use fabro_types::settings::run::MergeStrategy;
|
||||
use fabro_types::{GitHubRepositorySlug, PullRequestLink};
|
||||
use fabro_util::text::strip_goal_decoration;
|
||||
use tokio::time::sleep;
|
||||
use tracing::{debug, info, warn};
|
||||
|
|
@ -548,15 +550,29 @@ const BRANCH_HEAD_RETRY_DELAY: Duration = Duration::from_millis(500);
|
|||
/// Confirm the remote branch points at the run's final commit.
|
||||
///
|
||||
/// Publish failures are terminal, so a replica that has not caught up yet must
|
||||
/// not be mistaken for a genuinely stale branch.
|
||||
/// not be mistaken for a genuinely stale branch. Credentials are resolved once
|
||||
/// and reused across attempts; only the ref lookup is retried.
|
||||
async fn verify_remote_head(
|
||||
req: &OpenPullRequestRequest<'_>,
|
||||
owner: &str,
|
||||
repo: &str,
|
||||
) -> Result<(), String> {
|
||||
let repository =
|
||||
GitHubRepositorySlug::try_new(&format!("{owner}/{repo}")).ok_or_else(|| {
|
||||
format!("failed to verify remote branch head: invalid repository {owner}/{repo}")
|
||||
})?;
|
||||
let reader = GitHubRepositoryReader::open(&req.github, &repository)
|
||||
.await
|
||||
.map_err(|err| format!("failed to verify remote branch head: {err:#}"))?;
|
||||
let selector = format!("heads/{}", req.head_branch);
|
||||
|
||||
let mut last_seen = Ok(None);
|
||||
for attempt in 1..=BRANCH_HEAD_ATTEMPTS {
|
||||
last_seen = github_app::branch_head_sha(&req.github, owner, repo, req.head_branch).await;
|
||||
last_seen = match reader.resolve_commit(&selector).await {
|
||||
Ok(sha) => Ok(Some(sha)),
|
||||
Err(RepositoryReadError::NotFound { .. }) => Ok(None),
|
||||
Err(err) => Err(err),
|
||||
};
|
||||
match &last_seen {
|
||||
Ok(Some(head)) if head == req.expected_head_sha => return Ok(()),
|
||||
Ok(head) => debug!(
|
||||
|
|
@ -702,11 +718,11 @@ mod tests {
|
|||
use tokio::sync::RwLock as AsyncRwLock;
|
||||
|
||||
use super::*;
|
||||
use crate::event::{Event, append_event};
|
||||
use crate::records::StageSummary;
|
||||
|
||||
const FINAL_SHA: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
|
||||
const STALE_SHA: &str = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";
|
||||
use crate::event::{Event, append_event};
|
||||
use crate::records::StageSummary;
|
||||
|
||||
struct MockProvider {
|
||||
name: String,
|
||||
|
|
|
|||
|
|
@ -213,28 +213,16 @@ impl FixtureState {
|
|||
state.repositories = self
|
||||
.repositories
|
||||
.into_iter()
|
||||
.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,
|
||||
}
|
||||
.map(|repository| Repository {
|
||||
owner: repository.owner,
|
||||
name: repository.name,
|
||||
refs: crate::state::head_refs(repository.branches),
|
||||
files: HashMap::new(),
|
||||
default_branch: repository
|
||||
.default_branch
|
||||
.unwrap_or_else(|| "main".to_string()),
|
||||
private: repository.private,
|
||||
git_dir: None,
|
||||
})
|
||||
.collect();
|
||||
|
||||
|
|
|
|||
|
|
@ -58,7 +58,7 @@ pub async fn get_branch(
|
|||
// Find repository and check branch.
|
||||
for repo_data in &state.repositories {
|
||||
if repo_data.owner == owner && repo_data.name == repo {
|
||||
if let Some(sha) = repo_data.refs.get(&format!("heads/{branch}")) {
|
||||
if let Some(sha) = repo_data.refs.get(&crate::state::heads_selector(&branch)) {
|
||||
return (
|
||||
StatusCode::OK,
|
||||
Json(serde_json::json!({
|
||||
|
|
|
|||
|
|
@ -1,15 +1,11 @@
|
|||
use axum::Json;
|
||||
use axum::extract::{Path, State};
|
||||
use axum::http::header::{ACCEPT, CONTENT_TYPE};
|
||||
use axum::http::header::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::handlers::support::{accepts, authorize_repo_access, is_exact_commit_sha, message};
|
||||
use crate::server::SharedState;
|
||||
use crate::state::{AppState, PermissionLevel, TokenPermission};
|
||||
use crate::state::{PermissionLevel, TokenPermission};
|
||||
|
||||
const SHA_MEDIA_TYPE: &str = "application/vnd.github.sha";
|
||||
|
||||
|
|
@ -23,25 +19,25 @@ pub async fn get_commit(
|
|||
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);
|
||||
if let Err(response) = authorize_repo_access(
|
||||
&headers,
|
||||
&state,
|
||||
&repo,
|
||||
TokenPermission::Contents,
|
||||
PermissionLevel::Read,
|
||||
) {
|
||||
return *response;
|
||||
}
|
||||
|
||||
let Some(repository) = state
|
||||
.repositories
|
||||
.iter()
|
||||
.find(|repository| repository.owner == owner && repository.name == repo)
|
||||
else {
|
||||
let Some(repository) = state.find_repository(&owner, &repo) else {
|
||||
return message(StatusCode::NOT_FOUND, "Not Found");
|
||||
};
|
||||
|
||||
// A selector is either a known ref, or an exact SHA this repository can
|
||||
// already serve; an unknown SHA must not resolve to itself.
|
||||
let sha = repository.refs.get(&selector).cloned().or_else(|| {
|
||||
is_exact_commit_sha(&selector)
|
||||
(is_exact_commit_sha(&selector) && repository.knows_commit(&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");
|
||||
|
|
@ -50,65 +46,10 @@ pub async fn get_commit(
|
|||
(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 axum::http::header::ACCEPT;
|
||||
|
||||
use super::*;
|
||||
use crate::server::TestServer;
|
||||
use crate::state::AppState;
|
||||
|
|
|
|||
|
|
@ -1,17 +1,14 @@
|
|||
use axum::Json;
|
||||
use axum::body::Body;
|
||||
use axum::extract::{Path, Query, State};
|
||||
use axum::http::header::{ACCEPT, CONTENT_TYPE};
|
||||
use axum::http::header::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::handlers::support::{accepts, authorize_repo_access, is_exact_commit_sha, message};
|
||||
use crate::server::SharedState;
|
||||
use crate::state::{AppState, PermissionLevel, TokenPermission};
|
||||
use crate::state::{PermissionLevel, TokenPermission};
|
||||
|
||||
const RAW_CONTENT_MEDIA_TYPE: &str = "application/vnd.github.raw+json";
|
||||
|
||||
|
|
@ -32,25 +29,24 @@ pub async fn get_content(
|
|||
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 let Err(response) = authorize_repo_access(
|
||||
&headers,
|
||||
&state,
|
||||
&repo,
|
||||
TokenPermission::Contents,
|
||||
PermissionLevel::Read,
|
||||
) {
|
||||
return *response;
|
||||
}
|
||||
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 {
|
||||
let Some(repository) = state.find_repository(&owner, &repo) else {
|
||||
return message(StatusCode::NOT_FOUND, "Not Found");
|
||||
};
|
||||
|
||||
if let Some(contents) = repository
|
||||
.files
|
||||
.get(&(query.revision.clone(), path.clone()))
|
||||
{
|
||||
if let Some(contents) = repository.file_at(&query.revision, &path) {
|
||||
return (
|
||||
StatusCode::OK,
|
||||
[(CONTENT_TYPE, RAW_CONTENT_MEDIA_TYPE)],
|
||||
|
|
@ -59,75 +55,19 @@ pub async fn get_content(
|
|||
.into_response();
|
||||
}
|
||||
|
||||
let directory_prefix = format!("{path}/");
|
||||
if repository.files.keys().any(|(sha, stored_path)| {
|
||||
sha == &query.revision && stored_path.starts_with(&directory_prefix)
|
||||
}) {
|
||||
// GitHub answers a directory path with a JSON listing rather than raw
|
||||
// bytes, which is what lets the reader reject non-file targets.
|
||||
if repository.has_directory_at(&query.revision, &format!("{path}/")) {
|
||||
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 axum::http::header::ACCEPT;
|
||||
|
||||
use super::*;
|
||||
use crate::server::TestServer;
|
||||
use crate::state::AppState;
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ pub mod manifests;
|
|||
pub mod oauth;
|
||||
pub mod pulls;
|
||||
pub mod releases;
|
||||
pub mod support;
|
||||
pub mod users;
|
||||
|
||||
use axum::Router;
|
||||
|
|
|
|||
|
|
@ -3,43 +3,11 @@ use axum::extract::{Path, State};
|
|||
use axum::http::{HeaderMap, StatusCode};
|
||||
use axum::response::IntoResponse;
|
||||
|
||||
use crate::auth::{
|
||||
BearerTokenError, InstallationTokenAccessError, authorize_installation_token,
|
||||
ensure_repo_permission,
|
||||
};
|
||||
use crate::auth::{authorize_installation_token, ensure_repo_permission};
|
||||
use crate::handlers::support::{bearer_token_error_response, repo_permission_error_response};
|
||||
use crate::server::SharedState;
|
||||
use crate::state::{PermissionLevel, PullRequest, TokenPermission};
|
||||
|
||||
fn bearer_token_error_response(error: BearerTokenError) -> axum::response::Response {
|
||||
match error {
|
||||
BearerTokenError::Missing => (
|
||||
StatusCode::UNAUTHORIZED,
|
||||
Json(serde_json::json!({"message": "Unauthorized"})),
|
||||
)
|
||||
.into_response(),
|
||||
BearerTokenError::Invalid => (
|
||||
StatusCode::UNAUTHORIZED,
|
||||
Json(serde_json::json!({"message": "Bad credentials"})),
|
||||
)
|
||||
.into_response(),
|
||||
}
|
||||
}
|
||||
|
||||
fn repo_permission_error_response(error: InstallationTokenAccessError) -> axum::response::Response {
|
||||
match error {
|
||||
InstallationTokenAccessError::RepoNotAccessible => (
|
||||
StatusCode::NOT_FOUND,
|
||||
Json(serde_json::json!({"message": "Not Found"})),
|
||||
)
|
||||
.into_response(),
|
||||
InstallationTokenAccessError::PermissionDenied => (
|
||||
StatusCode::FORBIDDEN,
|
||||
Json(serde_json::json!({"message": "Resource not accessible by integration"})),
|
||||
)
|
||||
.into_response(),
|
||||
}
|
||||
}
|
||||
|
||||
/// POST /repos/{owner}/{repo}/pulls
|
||||
pub async fn create_pull_request(
|
||||
State(state): State<SharedState>,
|
||||
|
|
|
|||
75
test/twin/github/src/handlers/support.rs
Normal file
75
test/twin/github/src/handlers/support.rs
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
//! Response and authorization helpers shared by the twin's handlers.
|
||||
//!
|
||||
//! GitHub answers every rejection with the same `{"message": ...}` envelope, so
|
||||
//! the mapping from an auth failure to a status lives here once rather than in
|
||||
//! each endpoint.
|
||||
|
||||
use axum::Json;
|
||||
use axum::http::header::ACCEPT;
|
||||
use axum::http::{HeaderMap, StatusCode};
|
||||
use axum::response::{IntoResponse, Response};
|
||||
|
||||
use crate::auth::{
|
||||
BearerTokenError, InstallationTokenAccessError, authorize_installation_token,
|
||||
ensure_repo_permission,
|
||||
};
|
||||
use crate::state::{AppState, PermissionLevel, TokenInfo, TokenPermission};
|
||||
|
||||
/// Render GitHub's `{"message": ...}` error envelope.
|
||||
pub fn message(status: StatusCode, body: &'static str) -> Response {
|
||||
(status, Json(serde_json::json!({ "message": body }))).into_response()
|
||||
}
|
||||
|
||||
pub fn bearer_token_error_response(error: BearerTokenError) -> Response {
|
||||
match error {
|
||||
BearerTokenError::Missing => message(StatusCode::UNAUTHORIZED, "Unauthorized"),
|
||||
BearerTokenError::Invalid => message(StatusCode::UNAUTHORIZED, "Bad credentials"),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn repo_permission_error_response(error: InstallationTokenAccessError) -> Response {
|
||||
match error {
|
||||
InstallationTokenAccessError::RepoNotAccessible => {
|
||||
message(StatusCode::NOT_FOUND, "Not Found")
|
||||
}
|
||||
InstallationTokenAccessError::PermissionDenied => message(
|
||||
StatusCode::FORBIDDEN,
|
||||
"Resource not accessible by integration",
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// Authorize an installation token against one repository permission,
|
||||
/// rendering GitHub's response shape for every rejection.
|
||||
///
|
||||
/// The rejection is boxed because an `axum` `Response` is far larger than the
|
||||
/// token it displaces in the `Ok` path.
|
||||
pub fn authorize_repo_access(
|
||||
headers: &HeaderMap,
|
||||
state: &AppState,
|
||||
repo: &str,
|
||||
permission: TokenPermission,
|
||||
level: PermissionLevel,
|
||||
) -> Result<TokenInfo, Box<Response>> {
|
||||
let token = authorize_installation_token(headers, state)
|
||||
.map_err(|error| Box::new(bearer_token_error_response(error)))?;
|
||||
ensure_repo_permission(&token, repo, permission, level)
|
||||
.map_err(|error| Box::new(repo_permission_error_response(error)))?;
|
||||
Ok(token)
|
||||
}
|
||||
|
||||
/// Whether the client explicitly listed `media_type` in its `Accept` header.
|
||||
pub 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)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
/// Whether `value` is an exact 40-character hex commit SHA.
|
||||
pub fn is_exact_commit_sha(value: &str) -> bool {
|
||||
value.len() == 40 && value.bytes().all(|byte| byte.is_ascii_hexdigit())
|
||||
}
|
||||
|
|
@ -40,15 +40,51 @@ pub struct Installation {
|
|||
pub struct Repository {
|
||||
pub owner: String,
|
||||
pub name: String,
|
||||
/// Ref selector (`heads/main`, `tags/v1.0.0`) to the commit SHA it names.
|
||||
pub refs: HashMap<String, String>,
|
||||
pub files: HashMap<(String, String), Vec<u8>>,
|
||||
/// Commit SHA to the repository paths readable at that commit.
|
||||
pub files: HashMap<String, HashMap<String, Vec<u8>>>,
|
||||
pub default_branch: String,
|
||||
pub private: bool,
|
||||
pub git_dir: Option<std::path::PathBuf>,
|
||||
}
|
||||
|
||||
impl Repository {
|
||||
/// Contents of `path` at `sha`, if the fixture seeded one.
|
||||
pub fn file_at(&self, sha: &str, path: &str) -> Option<&Vec<u8>> {
|
||||
self.files.get(sha).and_then(|paths| paths.get(path))
|
||||
}
|
||||
|
||||
/// Whether any path at `sha` sits under `prefix`, i.e. `prefix` names a
|
||||
/// directory rather than a file.
|
||||
pub fn has_directory_at(&self, sha: &str, prefix: &str) -> bool {
|
||||
self.files
|
||||
.get(sha)
|
||||
.is_some_and(|paths| paths.keys().any(|stored| stored.starts_with(prefix)))
|
||||
}
|
||||
|
||||
/// Whether `sha` is a commit this repository can serve content for.
|
||||
pub fn knows_commit(&self, sha: &str) -> bool {
|
||||
self.files.contains_key(sha) || self.refs.values().any(|known| known == sha)
|
||||
}
|
||||
}
|
||||
|
||||
pub const DEFAULT_REPOSITORY_SHA: &str = "0123456789abcdef0123456789abcdef01234567";
|
||||
|
||||
/// The ref selector GitHub uses for a branch.
|
||||
pub fn heads_selector(branch: &str) -> String {
|
||||
format!("heads/{branch}")
|
||||
}
|
||||
|
||||
/// Seed each branch at [`DEFAULT_REPOSITORY_SHA`], the shape every repository
|
||||
/// fixture starts in.
|
||||
pub fn head_refs(branches: Vec<String>) -> HashMap<String, String> {
|
||||
branches
|
||||
.into_iter()
|
||||
.map(|branch| (heads_selector(&branch), DEFAULT_REPOSITORY_SHA.to_string()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// A pull request.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct PullRequest {
|
||||
|
|
@ -393,19 +429,10 @@ impl AppState {
|
|||
branches: Vec<String>,
|
||||
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(),
|
||||
refs,
|
||||
refs: head_refs(branches),
|
||||
files: HashMap::new(),
|
||||
default_branch: "main".to_string(),
|
||||
private,
|
||||
|
|
@ -413,13 +440,21 @@ impl AppState {
|
|||
});
|
||||
}
|
||||
|
||||
pub fn set_repository_ref(&mut self, owner: &str, name: &str, selector: &str, sha: &str) {
|
||||
let repository = self
|
||||
.repositories
|
||||
pub fn find_repository(&self, owner: &str, name: &str) -> Option<&Repository> {
|
||||
self.repositories
|
||||
.iter()
|
||||
.find(|repository| repository.owner == owner && repository.name == name)
|
||||
}
|
||||
|
||||
fn repository_mut(&mut self, owner: &str, name: &str) -> &mut Repository {
|
||||
self.repositories
|
||||
.iter_mut()
|
||||
.find(|repository| repository.owner == owner && repository.name == name)
|
||||
.expect("repository fixture should exist before adding a ref");
|
||||
repository
|
||||
.expect("repository fixture should exist before seeding refs or files")
|
||||
}
|
||||
|
||||
pub fn set_repository_ref(&mut self, owner: &str, name: &str, selector: &str, sha: &str) {
|
||||
self.repository_mut(owner, name)
|
||||
.refs
|
||||
.insert(selector.to_string(), sha.to_string());
|
||||
}
|
||||
|
|
@ -432,14 +467,11 @@ impl AppState {
|
|||
path: &str,
|
||||
contents: impl Into<Vec<u8>>,
|
||||
) {
|
||||
let repository = self
|
||||
.repositories
|
||||
.iter_mut()
|
||||
.find(|repository| repository.owner == owner && repository.name == name)
|
||||
.expect("repository fixture should exist before adding a file");
|
||||
repository
|
||||
self.repository_mut(owner, name)
|
||||
.files
|
||||
.insert((sha.to_string(), path.to_string()), contents.into());
|
||||
.entry(sha.to_string())
|
||||
.or_default()
|
||||
.insert(path.to_string(), contents.into());
|
||||
}
|
||||
|
||||
pub fn find_installation(&self, owner: &str, repo: &str) -> Option<&Installation> {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue