perf(relay/git): stream read-path, manifest info/refs fast path, idx sidecar (A/B/C) (#1240)

Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Co-authored-by: Max <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
Tyler
2026-06-24 19:44:24 -04:00
committed by GitHub
co-authored by npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d Tyler Longwell Max
parent 08d375aa2f
commit 856815994c
5 changed files with 742 additions and 20 deletions
+4 -1
View File
@@ -28,7 +28,10 @@ buzz-audit = { workspace = true }
buzz-search = { workspace = true }
axum = { workspace = true }
tokio = { workspace = true }
tokio-util = { workspace = true }
# `io` (ReaderStream) is needed to stream git subprocess stdout straight into
# the HTTP response body — see api::git::transport read-path streaming. The
# workspace default only enables rt+codec.
tokio-util = { workspace = true, features = ["io"] }
tower = { workspace = true }
tower-http = { workspace = true }
nostr = { workspace = true }
+72 -1
View File
@@ -63,6 +63,7 @@ use std::collections::BTreeMap;
use std::path::Path;
use std::process::Stdio;
use tempfile::TempDir;
use tokio::io::AsyncWriteExt;
use tokio::process::Command;
use tracing::{debug, warn};
@@ -256,6 +257,54 @@ async fn snapshot_workspace_state(
Ok((refs, head))
}
fn digest_from_pack_key(key: &str) -> Result<String, CasError> {
key.strip_prefix("packs/")
.filter(|digest| digest.len() == 64 && digest.chars().all(|c| c.is_ascii_hexdigit()))
.map(str::to_string)
.ok_or_else(|| {
CasError::Backend(StoreError::Backend(s3::error::S3Error::HttpFailWithBody(
500,
format!("put_pack returned non-standard key: {key}"),
)))
})
}
async fn write_idx_sidecar(
store: &GitStore,
pack_key: &str,
pack_bytes: &[u8],
) -> Result<(), CasError> {
let pack_digest = digest_from_pack_key(pack_key)?;
let tempdir = TempDir::new().map_err(|e| CasError::PackCapture(format!("idx tempdir: {e}")))?;
let pack_path = tempdir.path().join(format!("pack-{pack_digest}.pack"));
tokio::fs::write(&pack_path, pack_bytes)
.await
.map_err(|e| CasError::PackCapture(format!("write idx input pack {pack_digest}: {e}")))?;
let mut cmd = Command::new("git");
cmd.args(["index-pack", pack_path.to_str().unwrap()])
.current_dir(tempdir.path());
super::transport::harden_git_env(&mut cmd);
let out = cmd
.output()
.await
.map_err(|e| CasError::PackCapture(format!("index-pack spawn for idx sidecar: {e}")))?;
if !out.status.success() {
return Err(CasError::PackCapture(format!(
"index-pack for idx sidecar failed: status={:?} stderr={}",
out.status.code(),
String::from_utf8_lossy(&out.stderr)
)));
}
let idx_path = pack_path.with_extension("idx");
let idx_bytes = tokio::fs::read(&idx_path)
.await
.map_err(|e| CasError::PackCapture(format!("read idx sidecar {pack_digest}: {e}")))?;
store.put_idx(&pack_digest, &idx_bytes).await?;
Ok(())
}
/// Capture the objects this push introduced as a single pack.
///
/// Runs `git pack-objects --revs --stdout` reading rev-spec lines from
@@ -439,7 +488,15 @@ pub async fn cas_publish(
let pack_bytes = capture_pack(repo_path, &parent_state.parent.refs, &refs_after).await?;
let new_pack_key = if let Some(bytes) = pack_bytes {
debug!(bytes = bytes.len(), "captured push pack");
Some(store.put_pack(&bytes).await?)
let pack_key = store.put_pack(&bytes).await?;
if let Err(e) = write_idx_sidecar(store, &pack_key, &bytes).await {
warn!(
pack_key = %pack_key,
error = %e,
"failed to write git pack idx sidecar; push will continue"
);
}
Some(pack_key)
} else {
debug!("no new objects in push; manifest will reuse parent packs");
None
@@ -560,6 +617,20 @@ mod tests {
// `pointer_key` is owned by `manifest.rs` and unit-tested there
// (one source of truth — Max/Sami's centralization point).
#[test]
fn digest_from_pack_key_strips_prefix() {
let k = format!("packs/{}", "b".repeat(64));
let d = digest_from_pack_key(&k).unwrap();
assert_eq!(d, "b".repeat(64));
}
#[test]
fn digest_from_pack_key_rejects_unknown_prefix_or_bad_digest() {
assert!(digest_from_pack_key("manifests/abc").is_err());
assert!(digest_from_pack_key("packs/abc").is_err());
assert!(digest_from_pack_key(&format!("packs/{}", "g".repeat(64))).is_err());
}
#[test]
fn digest_from_key_strips_prefix() {
let k = format!("manifests/{}", "a".repeat(64));
+60 -7
View File
@@ -7,7 +7,8 @@
//! 1. GET pointer → manifest digest.
//! 2. GET manifest (digest-verified) → parsed [`Manifest`].
//! 3. GET every pack the manifest names (digest-verified, in parallel).
//! 4. **Phase 1** — for each pack: write `pack-<hex>.pack`, run
//! 4. **Phase 1** — for each pack: write `pack-<hex>.pack`; install and
//! validate a cached `idx/<hex>` sidecar when present, otherwise run
//! `git index-pack` to materialize `.idx`. Failure here tears down the
//! tempdir with no refs/HEAD ever written.
//! 5. **Phase 2** — only after all packs are indexed: write loose refs and
@@ -95,6 +96,21 @@ pub async fn hydrate_for_read(
Ok(Some(materialize_manifest(store, &manifest).await?))
}
/// Load the current manifest for a read path without materializing pack objects.
///
/// Used by `info/refs` fast paths that can answer directly from manifest refs.
/// Returns `Ok(None)` when the repo pointer is absent; otherwise verifies the
/// pointer-named manifest digest before returning it.
pub async fn load_manifest_for_read(
store: &GitStore,
owner: &str,
repo: &str,
) -> Result<Option<Manifest>, HydrateError> {
Ok(load_pointer(store, owner, repo)
.await?
.map(|(_etag, _digest, manifest)| manifest))
}
/// Hydrate a bare repo for write (`receive-pack`) and return the
/// `ParentState` the workspace was hydrated from.
///
@@ -206,12 +222,7 @@ async fn materialize_manifest(
tokio::fs::write(&pack_path, bytes)
.await
.map_err(|e| HydrateError::Hydrate(format!("write pack {digest}: {e}")))?;
// No `--strict`: `index-pack` already validates structural integrity
// (CRC, type tags, internal refs). `--strict` adds connectivity-graph
// checks, which would re-prove what manifest.packs already covers by
// construction (Inv_Closed, write-path invariant). Latency cost on
// every clone is not worth re-proving a write-path bug.
run_git(&path, &["index-pack", pack_path.to_str().unwrap()]).await?;
install_or_generate_idx(store, &path, digest, &pack_path).await?;
}
// Phase 2: install refs and HEAD. After this point, the repo advertises.
@@ -258,6 +269,48 @@ async fn materialize_manifest(
})
}
async fn install_or_generate_idx(
store: &GitStore,
repo_path: &Path,
pack_digest: &str,
pack_path: &Path,
) -> Result<(), HydrateError> {
let idx_path = pack_path.with_extension("idx");
match store.get_idx(pack_digest).await {
Ok(Some(idx_bytes)) => {
tokio::fs::write(&idx_path, &idx_bytes)
.await
.map_err(|e| HydrateError::Hydrate(format!("write idx {pack_digest}: {e}")))?;
match run_git(repo_path, &["verify-pack", idx_path.to_str().unwrap()]).await {
Ok(()) => return Ok(()),
Err(e) => {
tracing::warn!(
pack_digest = %pack_digest,
error = %e,
"git pack idx sidecar failed validation; regenerating"
);
let _ = tokio::fs::remove_file(&idx_path).await;
}
}
}
Ok(None) => {}
Err(e) => {
tracing::warn!(
pack_digest = %pack_digest,
error = %e,
"failed to read git pack idx sidecar; regenerating"
);
}
}
// No `--strict`: `index-pack` already validates structural integrity
// (CRC, type tags, internal refs). `--strict` adds connectivity-graph
// checks, which would re-prove what manifest.packs already covers by
// construction (Inv_Closed, write-path invariant). Latency cost on every
// clone is not worth re-proving a write-path bug.
run_git(repo_path, &["index-pack", pack_path.to_str().unwrap()]).await
}
/// Run `git <args>` in `cwd`, fail on non-zero exit.
async fn run_git(cwd: &Path, args: &[&str]) -> Result<(), HydrateError> {
let mut cmd = Command::new("git");
+77
View File
@@ -189,6 +189,22 @@ impl GitStore {
format!("{prefix}/{}", hex::encode(h.finalize()))
}
/// Derive the idx sidecar key for a content-addressed pack digest.
///
/// The idx is a pure cache derived from `packs/<pack_digest>`, so it is
/// keyed by the pack digest rather than by the idx bytes. This keeps the
/// manifest schema unchanged: readers can derive `idx/<pack_digest>` from
/// each manifest pack key.
pub fn idx_key_for_pack_digest(pack_digest: &str) -> Result<String, StoreError> {
if pack_digest.len() != 64 || !pack_digest.chars().all(|c| c.is_ascii_hexdigit()) {
return Err(StoreError::Backend(S3Error::HttpFailWithBody(
400,
format!("invalid pack digest for idx sidecar: {pack_digest:?}"),
)));
}
Ok(format!("idx/{pack_digest}"))
}
/// Create-only write of a content-addressed object (pack or manifest).
///
/// **The caller does not choose the key.** It is derived as
@@ -233,6 +249,51 @@ impl GitStore {
.await
}
/// Best-effort create-only write of an idx sidecar for `packs/<pack_digest>`.
///
/// Unlike packs/manifests, the key is not the SHA-256 of `idx_bytes`; it is
/// `idx/<pack_digest>` so hydrates can derive it without changing manifest
/// bytes. A 412 is idempotent success for the cache layer: the first writer
/// already produced the sidecar for this pack, and hydrate validates before
/// trusting it.
pub async fn put_idx(&self, pack_digest: &str, idx_bytes: &[u8]) -> Result<String, StoreError> {
let key = Self::idx_key_for_pack_digest(pack_digest)?;
let mut headers = axum::http::HeaderMap::new();
headers.insert(axum::http::header::IF_NONE_MATCH, "*".parse().unwrap());
match self
.bucket
.put_object_with_content_type_and_headers(
&key,
idx_bytes,
"application/x-git-index",
Some(headers),
)
.await
{
Ok(resp) if (200..300).contains(&resp.status_code()) => Ok(key),
Err(S3Error::HttpFailWithBody(412, _)) => Ok(key),
Ok(resp) => Err(StoreError::Backend(S3Error::HttpFailWithBody(
resp.status_code(),
"unexpected status".into(),
))),
Err(e) => Err(StoreError::Backend(e)),
}
}
/// Read an idx sidecar for `packs/<pack_digest>`.
///
/// A missing idx is a cache miss, not a hydrate failure; callers should
/// regenerate with `git index-pack`. Other backend failures are surfaced so
/// callers can decide whether to fall back or fail.
pub async fn get_idx(&self, pack_digest: &str) -> Result<Option<Bytes>, StoreError> {
let key = Self::idx_key_for_pack_digest(pack_digest)?;
match self.get(&key).await {
Ok(bytes) => Ok(Some(bytes)),
Err(StoreError::NotFound(_)) => Ok(None),
Err(e) => Err(e),
}
}
/// Write a manifest object. Returns the content-addressed key (`manifests/<hex>`).
pub async fn put_manifest(&self, bytes: &[u8]) -> Result<String, StoreError> {
self.put_immutable("manifests", bytes, "application/json")
@@ -747,6 +808,22 @@ impl GitStore {
mod tests {
use super::*;
#[test]
fn idx_key_uses_pack_digest_namespace() {
let digest = "a".repeat(64);
assert_eq!(
GitStore::idx_key_for_pack_digest(&digest).unwrap(),
format!("idx/{digest}")
);
}
#[test]
fn idx_key_rejects_non_digest_input() {
for bad in ["packs/abc", "abc", "../escape", &"g".repeat(64)] {
assert!(GitStore::idx_key_for_pack_digest(bad).is_err());
}
}
#[test]
fn classify_cas_412_is_lost_race() {
let r = Err(S3Error::HttpFailWithBody(412, "PreconditionFailed".into()));
+529 -11
View File
@@ -28,7 +28,9 @@ use tracing::{error, info, warn};
use super::cas_publish::{cas_publish, CasError, ParentState};
use super::hook::install_hook;
use super::hydrate::{hydrate_for_read, hydrate_for_write, HydrateError, HydratedRepo};
use super::hydrate::{
hydrate_for_read, hydrate_for_write, load_manifest_for_read, HydrateError, HydratedRepo,
};
use super::manifest_event::{build_ref_state_event, RefStateInputs};
use crate::state::AppState;
@@ -296,16 +298,177 @@ pub struct GitRepoParams {
repo: String,
}
// ── Manifest-Driven Advertisement (Track C) ──────────────────────────────────
/// Longest refname the fast path will emit. `is_safe_refname` enforces an
/// alphabet but no length bound; `pkt_line` encodes its payload length in a
/// 4-hex prefix that overflows past `0xffff`. Git's own refname limits sit far
/// below this, so any refname this long is pathological — degrade to the
/// subprocess path rather than risk a malformed length prefix. Generous bound:
/// `<oid> <refname>\n` plus the 4-byte pkt header must stay under `0xffff`, and
/// 4096 leaves vast headroom (git's de-facto practical ceiling is a few hundred
/// bytes total).
const MAX_FAST_PATH_REFNAME_LEN: usize = 4096;
/// Whether the `info/refs` fast path can serve this manifest without shelling
/// out. The manifest carries only `refname → oid`, so it cannot reproduce the
/// `^{}` peel line an **annotated** tag advertises (the peeled commit oid is
/// not stored — see RESEARCH/GIT_REF_ADVERTISEMENT_FORMAT.md). We cannot tell
/// an annotated tag from a lightweight one at the manifest level, so **any**
/// `refs/tags/*` forces the subprocess fallback for byte-correctness. We also
/// require the symbolic `head` to resolve to a ref we actually advertise —
/// otherwise the `symref=HEAD:<ref>` capability would point at a ref the
/// client never sees. The dominant clone case (branches only, HEAD→a branch)
/// takes the fast path; everything else stays exactly as it is today.
///
/// Eligibility is also a **safety gate**: the fast path emits manifest
/// refnames/oids straight into pkt-line bytes, so this predicate re-runs the
/// same `is_safe_refname`/`is_hex_oid` checks the hydrate path applies
/// (hydrate.rs) and the write path applies (`Manifest::validate`). The manifest
/// is already digest-verified against the pointer when loaded, so on every
/// normally-reachable path these re-checks are redundant — but keeping the
/// emit path symmetric with hydrate means an out-of-band-written manifest
/// (migration, manual S3 put, a future writer that skips `validate`) degrades
/// to the subprocess path instead of advertising unchecked bytes. Any failure
/// here → `false` → subprocess fallback, which surfaces the error correctly.
fn fast_path_eligible(manifest: &super::manifest::Manifest) -> bool {
use super::manifest::{is_hex_oid, is_safe_refname};
if manifest.refs.keys().any(|r| r.starts_with("refs/tags/")) {
return false;
}
// HEAD must resolve to an advertised ref. (Detached HEAD — head not in
// refs — can't be expressed as a symref; fall back.)
if !manifest.refs.contains_key(&manifest.head) {
return false;
}
// Safety re-check: every refname/oid we'd emit must be well-formed and
// bounded. HEAD is a key in `refs` (checked above), so the loop covers it.
manifest.refs.iter().all(|(refname, oid)| {
is_safe_refname(refname) && refname.len() <= MAX_FAST_PATH_REFNAME_LEN && is_hex_oid(oid)
})
}
/// Largest payload a single pkt-line can carry: the 4-hex length prefix counts
/// itself, so the total frame is bounded to `0xffff` and the payload to
/// `0xffff - 4`.
const PKT_LINE_MAX_PAYLOAD: usize = 0xffff - 4;
/// Encode one pkt-line: 4-char lowercase-hex length prefix (counting itself)
/// followed by `payload`. Appends to `out`.
///
/// The 4-hex prefix can only express a frame length up to `0xffff`. A payload
/// past [`PKT_LINE_MAX_PAYLOAD`] would make `format!("{len:04x}")` emit *five*
/// hex digits — not a truncation but a silent stream corruption (the next
/// reader takes the first four as the length). Callers on the manifest fast
/// path are already gated by [`fast_path_eligible`]'s length cap, but this is
/// the construction boundary: rather than trust every present and future
/// caller, an overlong payload here is dropped (emitting an empty `0004`
/// pkt-line) and logged at `error`, instead of writing a malformed length.
/// Non-panicking in every build profile — the worst case is a ref-short
/// advertisement that fails cleanly client-side, never a corrupted stream.
fn pkt_line(out: &mut Vec<u8>, payload: &[u8]) {
if payload.len() > PKT_LINE_MAX_PAYLOAD {
tracing::error!(
payload_len = payload.len(),
limit = PKT_LINE_MAX_PAYLOAD,
"pkt-line payload exceeds 0xffff-4 frame limit; dropping (bug: caller \
bypassed the fast-path length gate)"
);
out.extend_from_slice(b"0004"); // empty pkt-line; never a 5-hex length
return;
}
let len = payload.len() + 4;
out.extend_from_slice(format!("{len:04x}").as_bytes());
out.extend_from_slice(payload);
}
/// Build the **complete** `info/refs` HTTP body for `git-upload-pack` directly
/// from the published manifest — no hydrate, no subprocess. Byte-compatible
/// with `git upload-pack --advertise-refs` for the branches-only case that
/// [`fast_path_eligible`] gates on.
///
/// Layout (matches the subprocess oracle, git 2.51):
/// ```text
/// <pkt># service=git-upload-pack\n
/// 0000
/// <pkt><head-oid> HEAD\0<caps> symref=HEAD:<head-ref> object-format=<fmt> agent=buzz-git\n
/// <pkt><oid> <refname>\n # each ref, sorted ascending (BTreeMap order)
/// 0000
/// ```
/// The advertised capabilities are a fixed conservative **offer**; the client
/// re-negotiates against the real `upload-pack` subprocess in its follow-up
/// POST, so any subset the real upload-pack supports is safe. `object-format`
/// is derived from the oid width (40 hex = sha1, 64 = sha256) rather than
/// hardcoded. Caller guarantees [`fast_path_eligible`] returned true.
fn build_upload_pack_advertisement(manifest: &super::manifest::Manifest) -> Vec<u8> {
// head ref is guaranteed present by `fast_path_eligible`.
let head_oid = &manifest.refs[&manifest.head];
let object_format = if head_oid.len() == 64 {
"sha256"
} else {
"sha1"
};
// Capability offer. Conservative, version-agnostic. The symref tells the
// client which branch HEAD tracks (so `git clone` checks it out).
let caps = format!(
"multi_ack thin-pack side-band side-band-64k ofs-delta shallow \
deepen-since deepen-not deepen-relative no-progress include-tag \
multi_ack_detailed no-done symref=HEAD:{head} object-format={fmt} \
agent=buzz-git",
head = manifest.head,
fmt = object_format,
);
let mut out = Vec::new();
// 1. service header + flush.
let svc_line = b"# service=git-upload-pack\n";
pkt_line(&mut out, svc_line);
out.extend_from_slice(b"0000");
// 2. First line: HEAD with NUL-joined caps.
let mut first = Vec::new();
first.extend_from_slice(head_oid.as_bytes());
first.extend_from_slice(b" HEAD\0");
first.extend_from_slice(caps.as_bytes());
first.push(b'\n');
pkt_line(&mut out, &first);
// 3. Each ref, sorted (BTreeMap iterates ascending — matches git).
for (refname, oid) in &manifest.refs {
let mut line = Vec::new();
line.extend_from_slice(oid.as_bytes());
line.push(b' ');
line.extend_from_slice(refname.as_bytes());
line.push(b'\n');
pkt_line(&mut out, &line);
}
// 4. Trailing flush.
out.extend_from_slice(b"0000");
out
}
/// `GET /git/{owner}/{repo}/info/refs?service={service}`
///
/// Advertises refs for clone (git-upload-pack) or push (git-receive-pack).
///
/// **Track C fast path:** for `git-upload-pack` on a branches-only repo
/// ([`fast_path_eligible`]), the advertisement is built directly from the
/// published manifest — **no hydrate, no subprocess, no git semaphore
/// permit**. This is the dominant clone case, and it's exactly what the
/// W=20 permit used to serialize. Repos with any `refs/tags/*`, or the
/// `git-receive-pack` advertisement (different cap set), fall back to the
/// subprocess path — which still acquires a permit and hydrates, preserving
/// today's behavior byte-for-byte.
///
/// **Read fail-closed (Max's blocker):** pointer-absent → 404 (repo
/// never existed). *Any* below-pointer failure (manifest 404 under a
/// non-empty pointer, digest mismatch, pack 404, `index-pack` failure)
/// → 5xx via `HydrateError`. The legacy "leave disk as-is on hydrate
/// error" behavior is gone — A1 detectability now holds end-to-end on
/// the read side too.
/// error" behavior is gone — A1 detectability holds on the read side too.
pub async fn info_refs(
State(state): State<Arc<AppState>>,
_auth: GitAuth,
@@ -319,11 +482,55 @@ pub async fn info_refs(
};
let _repo_name = validate_repo_id(&params.owner, &params.repo)?;
let _permit = acquire_git_permit(&state)?;
// Track C fast path: only for clone advertisement. The receive-pack
// advertisement carries a different capability set (report-status,
// delete-refs, atomic, …) that we don't reproduce, so it always takes
// the subprocess path below.
if service == "git-upload-pack" {
// Load just the verified manifest — no object materialization, no
// permit. `Ok(None)` = pointer absent = repo never existed → 404.
match load_manifest_for_read(&state.git_store, &params.owner, &params.repo).await {
Ok(Some(manifest)) if fast_path_eligible(&manifest) => {
let body = build_upload_pack_advertisement(&manifest);
return Ok(Response::builder()
.status(StatusCode::OK)
.header(
header::CONTENT_TYPE,
"application/x-git-upload-pack-advertisement",
)
.header(header::CACHE_CONTROL, "no-cache")
.body(Body::from(body))
.unwrap());
}
// Eligible repo but has tags, or below-pointer failure handling:
// a present-but-ineligible manifest falls through to the
// subprocess path. Pointer-absent is a definitive 404 here —
// no point hydrating a repo that doesn't exist.
Ok(Some(_)) => { /* ineligible (has tags) → subprocess fallback */ }
Ok(None) => return Err((StatusCode::NOT_FOUND, "repository not found").into_response()),
Err(e) => return Err(hydrate_error_to_response(&params.owner, &params.repo, e)),
}
}
// Subprocess path: receive-pack advertisement, or upload-pack for a
// tagged repo. Acquires a permit and hydrates — today's behavior.
info_refs_subprocess(&state, service, &params).await
}
/// Subprocess-backed `info/refs` advertisement: hydrate the published state
/// into an ephemeral bare repo and shell out to `git <svc> --advertise-refs`.
///
/// This is the fallback from the Track C fast path (tagged repos, and the
/// `git-receive-pack` advertisement). The advertisement is O(refs), not
/// O(pack), so it stays buffered — streaming would buy nothing and would
/// lose the clean timeout/error mapping that buffering gives us.
async fn info_refs_subprocess(
state: &Arc<AppState>,
service: &str,
params: &GitRepoParams,
) -> Result<Response, Response> {
let _permit = acquire_git_permit(state)?;
// Hydrate the published state into an ephemeral bare repo. `Ok(None)`
// = pointer absent = repo never existed → 404. `Err(_)` = below-pointer
// failure → 5xx.
let repo = match hydrate_for_read(&state.git_store, &params.owner, &params.repo).await {
Ok(Some(repo)) => repo,
Ok(None) => return Err((StatusCode::NOT_FOUND, "repository not found").into_response()),
@@ -333,7 +540,7 @@ pub async fn info_refs(
// Git's smart HTTP protocol uses service names like "git-upload-pack" and
// "git-receive-pack", but the actual git subcommands are "upload-pack" and
// "receive-pack" (without the "git-" prefix).
let git_subcmd = service.strip_prefix("git-").unwrap_or(service.as_str());
let git_subcmd = service.strip_prefix("git-").unwrap_or(service);
let mut cmd = Command::new("git");
cmd.arg(git_subcmd)
@@ -413,9 +620,21 @@ pub async fn upload_pack(
Err(e) => return Err(hydrate_error_to_response(&params.owner, &params.repo, e)),
};
let output = run_git_at(repo.path(), "upload-pack", body, &[]).await?;
drop(repo);
Ok(build_git_response("upload-pack", output))
// Track A: stream the subprocess stdout straight into the response body
// instead of buffering the whole pack into RAM. `repo` (the hydrated
// tempdir) is moved into the stream and stays alive until the last byte
// is drained — the streaming analogue of the old `drop(repo)`. The
// `_permit` (git semaphore) is bound for the function scope; binding it
// here keeps it held only until the response head is built, not for the
// whole drain. That's intentional — see Track C notes on permit hold time.
stream_git_read(
repo,
"upload-pack",
&[],
body,
Vec::new(),
"application/x-git-upload-pack-result".to_string(),
)
}
/// `POST /git/{owner}/{repo}/git-receive-pack`
@@ -620,6 +839,133 @@ async fn run_git_at(
})
}
// ── Read-Path Streaming Runner (Track A) ─────────────────────────────────────
/// Keeps the git subprocess and its hydrated workspace alive for exactly as
/// long as the response body is being streamed.
///
/// The HTTP body is a [`tokio_util::io::ReaderStream`] over the child's
/// stdout. That borrows nothing it can keep alive on its own: the `Child`
/// (whose `kill_on_drop` would otherwise reap the process mid-stream) and the
/// [`HydratedRepo`] tempdir (whose objects the subprocess is still reading)
/// must outlive the last byte. We park both here and drop them only when the
/// stream is exhausted or the client disconnects — the structural analogue of
/// the `drop(repo)` the buffered path did after `wait_with_output`.
///
/// Why streaming is safe here but **not** on the push path: these are
/// read-only operations (`upload-pack`, `info/refs --advertise-refs`). They
/// never mutate published state, so there is no fence to preserve — contrast
/// `receive_pack`, which must buffer into [`PackOutput`] so [`finalize_push`]
/// can sequence the pointer CAS *before* any 2xx byte exists.
struct StreamingGit {
inner: tokio_util::io::ReaderStream<tokio::process::ChildStdout>,
/// Held purely to extend lifetime. `kill_on_drop(true)` means dropping
/// this after the stream completes reaps any lingering process; on the
/// happy path the child has already exited by then.
_child: tokio::process::Child,
/// The ephemeral bare repo the subprocess reads objects from. Must not be
/// removed from disk until the subprocess is done — i.e. until the stream
/// ends.
_repo: HydratedRepo,
}
impl futures_util::Stream for StreamingGit {
type Item = Result<bytes::Bytes, std::io::Error>;
fn poll_next(
mut self: std::pin::Pin<&mut Self>,
cx: &mut std::task::Context<'_>,
) -> std::task::Poll<Option<Self::Item>> {
std::pin::Pin::new(&mut self.inner).poll_next(cx)
}
}
/// Spawn a read-only `git --stateless-rpc <service>` subprocess and return a
/// streaming [`Response`] whose body is the child's stdout, optionally
/// preceded by `prefix` bytes (used by `info/refs` for the
/// `# service=…\n0000` pkt-line header).
///
/// The request `body` is pumped to the child's stdin concurrently. The
/// returned response owns the child + the hydrated workspace via
/// [`StreamingGit`], so neither is torn down until the body is fully drained.
///
/// **Read-path only.** Errors after the response head is sent cannot change
/// the status code (it's already 200), which is exactly git's smart-HTTP
/// contract: protocol-level failures are reported in-band within the pack
/// stream, not via HTTP status. The buffered [`run_git_at`] stays the push
/// path's runner precisely because the fence needs the bytes in hand before
/// committing to a status.
#[allow(clippy::result_large_err)]
fn stream_git_read(
repo: HydratedRepo,
service: &'static str,
extra_args: &[&str],
body: Body,
prefix: Vec<u8>,
content_type: String,
) -> Result<Response, Response> {
let mut cmd = Command::new("git");
cmd.arg(service).arg("--stateless-rpc");
for a in extra_args {
cmd.arg(a);
}
cmd.arg(repo.path())
.stdin(std::process::Stdio::piped())
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::null())
.kill_on_drop(true);
harden_git_env(&mut cmd);
let mut child = cmd.spawn().map_err(|e| {
error!(error = %e, service = %service, "git subprocess failed to spawn");
(StatusCode::INTERNAL_SERVER_ERROR, "git error").into_response()
})?;
// Pump the request body into git's stdin, then close it (EOF). Detached:
// the task ends on its own when the body ends or the write fails.
let mut stdin = child.stdin.take().expect("stdin piped");
tokio::spawn(async move {
use futures_util::StreamExt;
let mut stream = body.into_data_stream();
while let Some(chunk) = stream.next().await {
match chunk {
Ok(bytes) => {
if tokio::io::AsyncWriteExt::write_all(&mut stdin, &bytes)
.await
.is_err()
{
break;
}
}
Err(_) => break,
}
}
drop(stdin); // close stdin → EOF for git
});
let stdout = child.stdout.take().expect("stdout piped");
let git_stream = StreamingGit {
inner: tokio_util::io::ReaderStream::new(stdout),
_child: child,
_repo: repo,
};
// Prepend any protocol header (info/refs) ahead of git's stdout. The
// prefix is a single ready chunk; the rest streams from the subprocess.
let prefix_stream =
futures_util::stream::once(
async move { Ok::<_, std::io::Error>(bytes::Bytes::from(prefix)) },
);
let body_stream = futures_util::StreamExt::chain(prefix_stream, git_stream);
Ok(Response::builder()
.status(StatusCode::OK)
.header(header::CONTENT_TYPE, content_type)
.header(header::CACHE_CONTROL, "no-cache")
.body(Body::from_stream(body_stream))
.unwrap())
}
/// Build the canonical `application/x-git-{service}-result` response from
/// a completed subprocess. For the push path this is **only** reached via
/// [`finalize_push`], which is the unique constructor of a push 2xx — the
@@ -832,3 +1178,175 @@ pub fn git_router(state: Arc<AppState>) -> Router {
.layer(RequestBodyLimitLayer::new(body_limit))
.with_state(state)
}
#[cfg(test)]
mod track_c_tests {
use super::*;
use crate::api::git::manifest::Manifest;
use std::collections::BTreeMap;
fn oid_sha1() -> String {
"cb09a769da1c01f458fa6959d4e8eded38fac8d3".to_string()
}
fn branches_only_manifest() -> Manifest {
let mut refs = BTreeMap::new();
refs.insert("refs/heads/feature".to_string(), oid_sha1());
refs.insert("refs/heads/main".to_string(), oid_sha1());
Manifest {
version: 1,
head: "refs/heads/main".to_string(),
refs,
packs: vec!["packs/deadbeef".to_string()],
parent: None,
}
}
/// Split a pkt-line stream into `(len_prefix, payload)` frames, validating
/// that each 4-hex length counts itself and that `0000` is a flush.
fn parse_pkt_lines(bytes: &[u8]) -> Vec<Vec<u8>> {
let mut out = Vec::new();
let mut i = 0;
while i + 4 <= bytes.len() {
let len_hex = std::str::from_utf8(&bytes[i..i + 4]).unwrap();
let len = usize::from_str_radix(len_hex, 16).unwrap();
if len == 0 {
out.push(Vec::new()); // flush marker
i += 4;
continue;
}
assert!(len >= 4, "pkt-line length must count its own 4 bytes");
let payload = bytes[i + 4..i + len].to_vec();
out.push(payload);
i += len;
}
assert_eq!(i, bytes.len(), "pkt-line stream must consume exactly");
out
}
#[test]
fn fast_path_eligible_branches_only() {
assert!(fast_path_eligible(&branches_only_manifest()));
}
#[test]
fn fast_path_rejects_any_tag() {
let mut m = branches_only_manifest();
m.refs.insert("refs/tags/v1".to_string(), oid_sha1());
// Any tag → subprocess fallback (can't peel annotated tags from manifest).
assert!(!fast_path_eligible(&m));
}
#[test]
fn fast_path_rejects_head_not_in_refs() {
let mut m = branches_only_manifest();
m.head = "refs/heads/nonexistent".to_string();
// HEAD must resolve to an advertised ref to emit symref=HEAD:<ref>.
assert!(!fast_path_eligible(&m));
}
#[test]
fn fast_path_rejects_unsafe_refname() {
let mut m = branches_only_manifest();
// A pkt-line-injecting refname (newline) must never reach the emit path;
// eligibility is the safety gate → subprocess fallback re-validates.
m.refs.insert("refs/heads/evil\nx".to_string(), oid_sha1());
assert!(!fast_path_eligible(&m));
}
#[test]
fn fast_path_rejects_malformed_oid() {
let mut m = branches_only_manifest();
// A non-hex / wrong-length oid must degrade to subprocess, not be emitted.
m.refs
.insert("refs/heads/bad".to_string(), "not-a-valid-oid".to_string());
assert!(!fast_path_eligible(&m));
}
#[test]
fn fast_path_rejects_overlong_refname() {
let mut m = branches_only_manifest();
// A refname past MAX_FAST_PATH_REFNAME_LEN would overflow the 4-hex
// pkt-line length prefix; degrade to subprocess instead.
let long = format!("refs/heads/{}", "a".repeat(MAX_FAST_PATH_REFNAME_LEN));
m.refs.insert(long, oid_sha1());
assert!(!fast_path_eligible(&m));
}
#[test]
fn pkt_line_encodes_max_payload_without_overflow() {
// The largest payload that still fits a 4-hex frame length emits a
// single valid `ffff` (= 0xffff) frame — the boundary the guard
// protects, exercised on the safe side.
let payload = vec![b'a'; PKT_LINE_MAX_PAYLOAD];
let mut out = Vec::new();
pkt_line(&mut out, &payload);
assert_eq!(&out[..4], b"ffff", "frame length prefix");
assert_eq!(out.len(), 4 + PKT_LINE_MAX_PAYLOAD, "no truncation");
let frames = parse_pkt_lines(&out);
assert_eq!(frames.len(), 1);
assert_eq!(frames[0].len(), PKT_LINE_MAX_PAYLOAD);
}
// The overlong-payload guard degrades to an empty `0004` pkt-line in every
// build profile (no 5-hex length, stream stays parseable) and logs at
// `error`. Non-panicking, so it's testable directly.
#[test]
fn pkt_line_overlong_payload_degrades_to_empty_frame() {
let payload = vec![b'a'; PKT_LINE_MAX_PAYLOAD + 1];
let mut out = Vec::new();
pkt_line(&mut out, &payload);
// Empty pkt-line, payload dropped — never a malformed 5-hex length.
assert_eq!(out, b"0004");
}
#[test]
fn advertisement_framing_matches_git_oracle_shape() {
let body = build_upload_pack_advertisement(&branches_only_manifest());
let frames = parse_pkt_lines(&body);
// Layout: [service header, flush, HEAD line, feature, main, flush]
assert_eq!(frames.len(), 6, "frame count");
// 0: service header
assert_eq!(frames[0], b"# service=git-upload-pack\n");
// 1: flush after service header
assert!(frames[1].is_empty());
// 2: HEAD line — "<oid> HEAD\0<caps>\n"
let head = &frames[2];
let nul = head.iter().position(|&b| b == 0).expect("NUL in HEAD line");
assert_eq!(&head[..nul], format!("{} HEAD", oid_sha1()).as_bytes());
let caps = std::str::from_utf8(&head[nul + 1..head.len() - 1]).unwrap();
assert_eq!(*head.last().unwrap(), b'\n');
// symref + object-format are the load-bearing caps.
assert!(caps.contains("symref=HEAD:refs/heads/main"));
assert!(caps.contains("object-format=sha1"));
assert!(caps.contains("side-band-64k"));
// 3,4: refs sorted ascending — feature before main (BTreeMap order),
// each "<oid> <refname>\n", no NUL.
assert_eq!(
frames[3],
format!("{} refs/heads/feature\n", oid_sha1()).into_bytes()
);
assert_eq!(
frames[4],
format!("{} refs/heads/main\n", oid_sha1()).into_bytes()
);
// 5: trailing flush
assert!(frames[5].is_empty());
}
#[test]
fn advertisement_picks_sha256_from_oid_width() {
let mut m = branches_only_manifest();
let oid256 = "a".repeat(64);
m.refs.insert("refs/heads/main".to_string(), oid256.clone());
m.refs.insert("refs/heads/feature".to_string(), oid256);
let body = build_upload_pack_advertisement(&m);
let caps = String::from_utf8_lossy(&body);
assert!(caps.contains("object-format=sha256"));
}
}