Compare commits

...
Author SHA1 Message Date
beardthelion d7da6aa159 fix(node): distinguish missing ref from probe failure in store::log 2026-09-10 21:31:12 -05:00
beardthelion bbf2f876c8 fix: warn or error instead of returning silent empty results
Three sites folded failures into empty results indistinguishable from
genuine emptiness:

- clone.rs encrypted-blobs list fetch: a non-2xx or transport error
  silently returned no recovered paths; now warns via emit_warning,
  matching the per-blob stage in the same function.
- clone.rs arweave fallback: unwrap_or_default() became unwrap_or_else
  that warns, matching the node-recovery arm directly above it.
- changelog handler: store::log failures now return a git error and
  list_prs failures propagate (503 when the pool is unreachable)
  instead of answering 200 with an empty timeline. store::log itself
  only returns empty when the ref genuinely does not resolve: a ref
  that resolves but fails to log is a read failure, not an empty repo.

Closes #400.
2026-09-10 20:17:13 -05:00
Kevin Codex bfc44f926d Merge pull request #381 from Gitlawb/fix/gl-whoami-capped-body-read
fix(gl): read whoami error bodies through CappedBody.text
2026-08-26 13:26:50 +08:00
beardthelion 65f5989a1d test(node): pin owner-push off in receive-pack lock tests
These lock-path proofs use non-owner proof DIDs. With enforce_owner_push
defaulting on, they fail before receive-pack runs.
2026-08-25 10:27:48 -05:00
beardthelion 4725146db2 fix(gl): read whoami error bodies through CappedBody.text
#173 changed read_body_capped to return CappedBody; peer.rs and sync.rs
were updated but whoami.rs was not, leaving main red after the #223 merge.
2026-08-25 10:04:20 -05:00
5 changed files with 375 additions and 8 deletions

No files matched your search

+216 -2
View File
@@ -43,7 +43,10 @@ pub async fn get_changelog(
.await
.map_err(|e| AppError::Git(e.to_string()))?;
let head_ref = store::resolve_head(&disk_path, &record.default_branch);
let commits = store::log(&disk_path, &head_ref, limit).unwrap_or_default();
// A read failure is not an empty history: returning a bare 200 with no
// events makes a degraded repo look identical to a brand-new one (#400).
let commits =
store::log(&disk_path, &head_ref, limit).map_err(|e| AppError::Git(e.to_string()))?;
let mut events: Vec<serde_json::Value> = commits
.into_iter()
@@ -60,7 +63,9 @@ pub async fn get_changelog(
.collect();
// ── Merged PRs ───────────────────────────────────────────────────────
let prs = state.db.list_prs(&record.id).await.unwrap_or_default();
// Same for the DB half: an outage must surface as an error (503 when the
// pool is unreachable), not an empty timeline (#400).
let prs = state.db.list_prs(&record.id).await?;
for pr in prs.iter().filter(|p| p.status == "merged") {
events.push(serde_json::json!({
"type": "pr_merged",
@@ -88,3 +93,212 @@ pub async fn get_changelog(
"count": events.len(),
})))
}
/// #400: endpoint-level proof that a degraded store or DB reaches the caller
/// as an error, not a 200 with an empty timeline.
#[cfg(test)]
mod tests {
use super::*;
use axum::http::Request;
use axum::http::StatusCode;
use axum::Router;
use sqlx::PgPool;
use tempfile::TempDir;
use tower::ServiceExt;
fn seed_repo(owner_did: &str, name: &str) -> crate::db::RepoRecord {
let now = chrono::Utc::now();
crate::db::RepoRecord {
id: uuid::Uuid::new_v4().to_string(),
name: name.to_string(),
owner_did: owner_did.to_string(),
description: None,
is_public: true,
default_branch: "main".to_string(),
created_at: now,
updated_at: now,
disk_path: format!("/tmp/{name}"),
forked_from: None,
machine_id: None,
}
}
/// A state whose repo store roots in `repos_dir` so the test controls the
/// on-disk repo, with the repo record already inserted.
async fn seeded_state(
pool: &PgPool,
repos_dir: &std::path::Path,
owner: &str,
name: &str,
) -> AppState {
let mut state = crate::test_support::test_state(pool.clone()).await;
state.repo_store =
crate::git::repo_store::RepoStore::for_testing(repos_dir.to_path_buf(), pool.clone());
state
.db
.create_repo(&seed_repo(owner, name))
.await
.expect("seed repo");
state
}
fn repo_disk_path(repos_dir: &std::path::Path, owner: &str, name: &str) -> std::path::PathBuf {
repos_dir
.join(owner.replace([':', '/'], "_"))
.join(format!("{name}.git"))
}
fn init_bare(path: &std::path::Path) {
std::fs::create_dir_all(path).unwrap();
let out = std::process::Command::new("git")
.args(["init", "--bare"])
.arg(path)
.output()
.unwrap();
assert!(out.status.success());
}
/// A bare repo with one real commit on HEAD.
fn bare_repo_with_commit(
repos_dir: &std::path::Path,
owner: &str,
name: &str,
) -> std::path::PathBuf {
let scratch = repos_dir.join(format!("scratch-{name}"));
let out = std::process::Command::new("git")
.args(["init"])
.arg(&scratch)
.output()
.unwrap();
assert!(out.status.success());
let out = std::process::Command::new("git")
.args([
"-c",
"user.email=t@t",
"-c",
"user.name=t",
"commit",
"--allow-empty",
"-m",
"initial",
])
.current_dir(&scratch)
.output()
.unwrap();
assert!(out.status.success());
let repo_path = repo_disk_path(repos_dir, owner, name);
std::fs::create_dir_all(repo_path.parent().unwrap()).unwrap();
let out = std::process::Command::new("git")
.args(["clone", "--bare"])
.arg(&scratch)
.arg(&repo_path)
.output()
.unwrap();
assert!(out.status.success());
repo_path
}
/// Delete the object behind HEAD and leave garbage: `git log` fails while
/// `rev-parse` still resolves the ref.
fn corrupt_head_object(repo_path: &std::path::Path) {
let out = std::process::Command::new("git")
.args(["rev-parse", "HEAD"])
.current_dir(repo_path)
.output()
.unwrap();
let oid = String::from_utf8(out.stdout).unwrap().trim().to_string();
let obj = repo_path.join("objects").join(&oid[..2]).join(&oid[2..]);
std::fs::remove_file(&obj).unwrap();
std::fs::write(&obj, b"garbage").unwrap();
}
async fn oneshot_changelog(
state: AppState,
owner: &str,
name: &str,
) -> axum::response::Response {
Router::new()
.route(
"/api/v1/repos/{owner}/{repo}/changelog",
axum::routing::get(get_changelog),
)
.with_state(state)
.oneshot(
Request::builder()
.uri(format!("/api/v1/repos/{owner}/{name}/changelog"))
.body(axum::body::Body::empty())
.unwrap(),
)
.await
.unwrap()
}
/// The git half of the fold: a repo whose HEAD resolves but whose object
/// store is corrupt must be a 500, not a 200 with zero events.
#[sqlx::test]
async fn changelog_on_corrupt_object_store_returns_500_not_empty_200(pool: PgPool) {
let owner = "did:key:zCHANGELOGCORRUPTAAAAAAAAAAAAAAAAAAAA";
let dir = TempDir::new().unwrap();
let state = seeded_state(&pool, dir.path(), owner, "corrupt-log").await;
let repo_path = bare_repo_with_commit(dir.path(), owner, "corrupt-log");
corrupt_head_object(&repo_path);
let resp = oneshot_changelog(state, owner, "corrupt-log").await;
assert_eq!(
resp.status(),
StatusCode::INTERNAL_SERVER_ERROR,
"a resolving ref whose objects cannot be read is a git error"
);
}
/// The DB half of the fold: with the pull_requests table gone, list_prs
/// fails and the endpoint must surface it (503) rather than answering a
/// 200 with only the git-derived events.
#[sqlx::test]
async fn changelog_on_pr_table_failure_returns_error_not_empty_200(pool: PgPool) {
let owner = "did:key:zCHANGELOGDBBBBBBBBBBBBBBBBBBBBBBBBBBB";
let dir = TempDir::new().unwrap();
let state = seeded_state(&pool, dir.path(), owner, "db-fail").await;
init_bare(&repo_disk_path(dir.path(), owner, "db-fail"));
sqlx::query("DROP TABLE pull_requests")
.execute(&pool)
.await
.unwrap();
let resp = oneshot_changelog(state, owner, "db-fail").await;
assert!(
resp.status().is_server_error(),
"a DB failure must not answer a 200 empty timeline; got {}",
resp.status()
);
}
/// Must-not direction: a healthy repo with a commit still returns the
/// event; an empty repo is still a valid empty timeline.
#[sqlx::test]
async fn changelog_still_serves_commit_and_empty_repo(pool: PgPool) {
let owner = "did:key:zCHANGELOGOKCCCCCCCCCCCCCCCCCCCCCCCCCCC";
let dir = TempDir::new().unwrap();
let state = seeded_state(&pool, dir.path(), owner, "with-commit").await;
bare_repo_with_commit(dir.path(), owner, "with-commit");
let resp = oneshot_changelog(state, owner, "with-commit").await;
assert_eq!(resp.status(), StatusCode::OK);
let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
.await
.unwrap();
let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap();
assert_eq!(v["count"], 1, "the seeded commit must appear");
assert_eq!(v["events"][0]["type"], "commit");
let state = seeded_state(&pool, dir.path(), owner, "empty-repo").await;
init_bare(&repo_disk_path(dir.path(), owner, "empty-repo"));
let resp = oneshot_changelog(state, owner, "empty-repo").await;
assert_eq!(resp.status(), StatusCode::OK);
let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
.await
.unwrap();
let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap();
assert_eq!(v["count"], 0, "a genuinely empty repo is still 200/empty");
}
}
+9
View File
@@ -6260,6 +6260,9 @@ mod tests {
// Long enough that the git-service timeout is never what ends this push; the
// disconnect is.
cfg.git_service_timeout_secs = 600;
// Lock-path test, not owner-push authorization: pin off so the default-on gate
// does not reject the proof DID before receive-pack runs.
cfg.enforce_owner_push = false;
state.config = std::sync::Arc::new(cfg);
state
.db
@@ -6430,6 +6433,9 @@ mod tests {
None,
crate::git::repo_store::build_lock_pool(&pool, 4, std::time::Duration::from_secs(5)),
);
let mut cfg = (*state.config).clone();
cfg.enforce_owner_push = false;
state.config = std::sync::Arc::new(cfg);
state
.db
.upsert_mirror_repo(owner, name, "/tmp/z6succ-sc1", None, false)
@@ -6516,6 +6522,9 @@ mod tests {
let owner = "z6lockpool";
let name = "lp1";
let mut state = crate::test_support::test_state(pool.clone()).await;
let mut cfg = (*state.config).clone();
cfg.enforce_owner_push = false;
state.config = std::sync::Arc::new(cfg);
// One lock-pool connection, short checkout timeout so the exhaustion surfaces
// promptly rather than at the handler's own acquire deadline.
state.repo_store = crate::git::repo_store::RepoStore::new(
+84 -1
View File
@@ -141,7 +141,29 @@ pub fn log(repo_path: &Path, refname: &str, limit: usize) -> Result<Vec<CommitIn
.context("failed to run git log")?;
if !output.status.success() {
return Ok(vec![]); // empty repo
// An unresolvable ref means a genuinely empty repo. A ref that
// resolves but fails to log is a read failure (corrupt object store, a
// gc mid-read): report it rather than passing an empty log off as no
// history (#400). rev-parse resolves the name without reading objects.
// allow-unbounded-git: failure-path existence recheck; module
// convention, at most one extra spawn per failed log.
let resolved = Command::new("git")
.args(["rev-parse", "--verify", "--quiet", refname])
.current_dir(repo_path)
.output()
.context("failed to run git rev-parse")?;
// --quiet exits 1 only for "ref does not resolve"; other failures
// (not a repo, broken config) are real errors, not an empty history.
match resolved.status.code() {
Some(1) => return Ok(vec![]),
Some(0) => {}
_ => {
let stderr = String::from_utf8_lossy(&resolved.stderr);
anyhow::bail!("git rev-parse failed for {refname}: {}", stderr.trim());
}
}
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("git log failed for {refname}: {}", stderr.trim());
}
let stdout = String::from_utf8_lossy(&output.stdout);
@@ -2025,4 +2047,65 @@ mod tests {
"a clean `missing` twice on a readable store is a genuine absence; got {res:?}"
);
}
// #400: `log` may fold a failed `git log` into Ok(vec![]) only when the ref
// genuinely does not resolve (an empty repo). A ref that resolves but
// fails to read is an error, not an empty history.
#[test]
fn log_empty_is_empty_and_resolved_ref_read_failure_errors() {
let td = tempfile::TempDir::new().unwrap();
let work: &Path = td.path();
let g = |args: &[&str]| {
assert!(Command::new("git")
.args(args)
.current_dir(work)
.status()
.unwrap()
.success());
};
g(&["init", "-q"]);
g(&["config", "user.email", "t@t"]);
g(&["config", "user.name", "t"]);
// No commits: nothing resolves, so a failed log is a real empty repo.
assert!(super::log(work, "HEAD", 10).unwrap().is_empty());
std::fs::write(work.join("f.txt"), b"x\n").unwrap();
g(&["add", "."]);
g(&["commit", "-qm", "c1"]);
assert_eq!(super::log(work, "HEAD", 10).unwrap().len(), 1);
// Corrupt the object the branch resolves to: `git log` fails but
// `rev-parse --verify` still resolves the name, so the failure must
// surface as an error rather than an empty history.
let oid = {
let o = Command::new("git")
.args(["rev-parse", "HEAD"])
.current_dir(work)
.output()
.unwrap();
String::from_utf8_lossy(&o.stdout).trim().to_string()
};
let obj = work.join(".git/objects").join(&oid[..2]).join(&oid[2..]);
std::fs::remove_file(&obj).unwrap();
std::fs::write(&obj, b"garbage").unwrap();
assert!(super::log(work, "HEAD", 10).is_err());
}
/// The rev-parse recheck inside `log` must distinguish "ref missing"
/// (exit 1 -> empty history) from a real probe failure (exit 128 on a
/// non-repo -> error), not fold both into `Ok(vec![])`.
#[test]
fn log_probe_failure_on_a_non_repo_errors_instead_of_empty() {
let td = tempfile::TempDir::new().unwrap();
// A directory that is not a git repo at all: `git log` fails and the
// `rev-parse` recheck fails with 128, which is not "missing ref".
let not_a_repo = td.path().join("not-a-repo");
std::fs::create_dir_all(&not_a_repo).unwrap();
assert!(
super::log(&not_a_repo, "HEAD", 10).is_err(),
"a rev-parse probe failure must not read as an empty repo"
);
}
}
+65 -4
View File
@@ -54,6 +54,13 @@ fn emit_warning(line: &str) {
let _ = writeln!(warn_sink(), "{line}");
}
/// Warn-and-degrade for the recovery arms in `run`: a failed recovery source
/// is reported and yields no paths, never silently empty (#400).
fn warn_recovery_failed(what: &str, e: &anyhow::Error) -> Vec<String> {
emit_warning(&format!("warning: {what} failed: {e}"));
Vec::new()
}
#[cfg(not(test))]
fn warn_sink() -> impl std::io::Write {
std::io::stderr()
@@ -322,7 +329,19 @@ async fn recover_encrypted_blobs(
.await
{
Ok(r) if r.status().is_success() => r,
_ => return Ok(vec![]),
Ok(r) => {
emit_warning(&format!(
"warning: could not list encrypted blobs for {owner}/{name}: node returned {}; skipping recovery",
r.status()
));
return Ok(vec![]);
}
Err(e) => {
emit_warning(&format!(
"warning: could not list encrypted blobs for {owner}/{name}: {e}; skipping recovery"
));
return Ok(vec![]);
}
};
let body: EncryptedBlobsResponse = resp.json().await.context("parsing encrypted-blobs")?;
if body.blobs.is_empty() {
@@ -836,8 +855,7 @@ pub async fn run(args: CloneArgs) -> Result<()> {
// fallback still runs), but the strict /encrypted-blobs parse now
// fails closed on schema drift, so surface it rather than letting
// `.unwrap_or_default()` silently swallow it into "no paths".
eprintln!("warning: encrypted-blobs recovery failed: {e}");
Vec::new()
warn_recovery_failed("encrypted-blobs recovery", &e)
});
let from_arweave = recover_from_arweave(
&args.arweave_gateway,
@@ -848,7 +866,10 @@ pub async fn run(args: CloneArgs) -> Result<()> {
&keypair,
)
.await
.unwrap_or_default();
.unwrap_or_else(|e| {
// Same rule as the node-recovery arm above: warn, never abort.
warn_recovery_failed("arweave/ipfs gateway recovery", &e)
});
paths.extend(from_arweave);
if !paths.is_empty() {
@@ -929,6 +950,46 @@ mod tests {
WARNINGS.with(|w| w.borrow().clone())
}
// #400: the recovery-arm degrade must warn, not silently become "no
// paths". `warn_recovery_failed` is the write both arms go through.
#[test]
fn warn_recovery_failed_warns_and_returns_empty() {
reset_warnings();
let out =
super::warn_recovery_failed("arweave/ipfs gateway recovery", &anyhow::anyhow!("boom"));
assert!(out.is_empty());
assert!(
warnings().contains("arweave/ipfs gateway recovery failed: boom"),
"a recovery failure must warn, got: {:?}",
warnings()
);
}
// #400: the encrypted-blobs LIST fetch must warn on failure instead of
// silently returning an empty recovery set. The per-blob stage already
// warns; the list fetch was the outlier.
#[tokio::test]
async fn encrypted_blobs_list_failure_warns_and_returns_empty() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/api/v1/repos/o/r/encrypted-blobs")
.with_status(500)
.create_async()
.await;
let kp = gitlawb_core::identity::Keypair::generate();
let dest = TempDir::new().unwrap();
reset_warnings();
let paths = recover_encrypted_blobs(&server.url(), "o", "r", dest.path(), &kp)
.await
.unwrap();
assert!(paths.is_empty());
assert!(
warnings().contains("could not list encrypted blobs"),
"a failed list fetch must warn, got: {:?}",
warnings()
);
}
fn g(args: &[&str], dir: &Path) {
assert!(Command::new("git")
.args(args)
+1 -1
View File
@@ -60,7 +60,7 @@ pub(crate) async fn run_to_writer(args: WhoamiArgs, w: &mut impl std::io::Write)
}
Ok(resp) => {
let status = resp.status();
let raw = read_body_capped(resp, 8 * 1024).await;
let raw = read_body_capped(resp, 8 * 1024).await.text;
let msg = serde_json::from_str::<Value>(&raw)
.ok()
.and_then(|v| {