Compare commits

..
Author SHA1 Message Date
beardthelion 6143c2ebb0 test(node): derive the first-OID expectation from every reachable object
sweep_persists_last_attempted_not_planned_tail_on_mid_batch_narrow sorted only
HEAD's commit, tree and three blobs to find the first object the sweep
dispatches. The fixture makes three commits, so the two earlier commits and
trees are reachable too, and an earlier commit id changes with its timestamp.
When one sorted below the other five the expected first OID was wrong and the
test failed.

Take the expectation from git rev-list --objects --all (nine objects). On an
unpatched main build the test failed 3 of 45 serial runs; with this change it
failed 0 of 45.
2026-11-13 09:51:03 -06:00
17 changed files with 45 additions and 9209 deletions

No files matched your search

Generated
+6 -6
View File
@@ -2208,7 +2208,7 @@ version = "3.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34"
dependencies = [
"windows-sys 0.48.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -2701,7 +2701,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7ab67060fc6b8ef687992d439ca0fa36e7ed17e9a0b16b25b601e8757df720de"
dependencies = [
"data-encoding",
"syn 1.0.109",
"syn 2.0.117",
]
[[package]]
@@ -5779,16 +5779,16 @@ dependencies = [
[[package]]
name = "quinn-udp"
version = "0.5.16"
version = "0.5.14"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "af66907df18639dcf4db56ca65490cabc4b27a97dbadd96f2926cca73298f016"
checksum = "addec6a0dcad8a8d96a771f815f0eaf55f9d1805756410b39f5fa81332574cbd"
dependencies = [
"cfg_aliases",
"libc",
"once_cell",
"socket2",
"tracing",
"windows-sys 0.61.2",
"windows-sys 0.52.0",
]
[[package]]
@@ -7959,7 +7959,7 @@ version = "0.1.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22"
dependencies = [
"windows-sys 0.48.0",
"windows-sys 0.61.2",
]
[[package]]
File diff suppressed because it is too large. Load diff
-29
View File
@@ -22,7 +22,6 @@ pub mod replicas;
pub mod repos;
pub mod resolve;
pub mod stars;
pub mod status;
pub mod task_cursor;
pub mod tasks;
pub mod visibility;
@@ -69,12 +68,6 @@ pub(crate) async fn authorize_repo_read(
/// representation only within `did:key`; never let a bare id match across methods —
/// `did:web` / `did:gitlawb` share the base58 space with `did:key`, so a
/// trailing-segment compare would treat `did:key:X` and `did:gitlawb:X` as equal.
///
/// The collapse itself lives in exactly one place, [`crate::db::normalize_owner_key`],
/// which is also the function the stored `owner_did` / `authorizing_did` columns
/// are normalized through and which `OWNER_KEY_CASE_SQL` mirrors byte for byte.
/// Two identities match when they normalize to the same key; a second copy of the
/// rule here is how the Rust gate and the SQL filters would drift apart.
pub(crate) fn did_matches(a: &str, b: &str) -> bool {
// One implementation, shared with `gl` and `git-remote-gitlawb` through core:
// every boundary that compares a DID applies the same collapse rule.
@@ -212,7 +205,6 @@ mod authz_guard {
let events = include_str!("events.rs");
let tasks = include_str!("tasks.rs");
let stars = include_str!("stars.rs");
let status = include_str!("status/mod.rs");
let protect = include_str!("protect.rs");
let visibility = include_str!("visibility.rs");
let profiles = include_str!("profiles.rs");
@@ -238,13 +230,6 @@ mod authz_guard {
// (existence hiding) and require_repo_owner guards the owner half.
(webhooks, "list_webhooks", "authorize_repo_read("),
(webhooks, "list_webhooks", "require_repo_owner("),
// Same two-half shape as list_webhooks: authorize_repo_read runs
// first (a quarantined or unreadable repo gets the missing-repo
// not-found, never a 403 that would confirm existence), then
// require_repo_owner 403s a non-owner of a readable repo. Both halves
// are pinned, because dropping either changes what a stranger learns.
(status, "create_status", "authorize_repo_read("),
(status, "create_status", "require_repo_owner("),
(labels, "add_label", "authorize_repo_read("),
(labels, "add_label", "require_repo_owner("),
(labels, "remove_label", "authorize_repo_read("),
@@ -276,19 +261,6 @@ mod authz_guard {
(protect, "list_protected_branches", "authorize_repo_read("),
(labels, "list_labels", "authorize_repo_read("),
(events, "list_repo_events", "authorize_repo_read("),
// The catch-up poll surface gates before any push-event row is read,
// so a stranger cannot use it to learn that a private repo exists or
// is being pushed to.
(events, "list_repo_push_events", "authorize_repo_read("),
// The status read gates before any claim data is loaded, so its deny
// is the repo's own not-found rather than an existence oracle over
// which commits were reported on.
(status, "commit_status", "authorize_repo_read("),
// The rollup gates on the same helper before the pull request row is
// loaded, so a caller who cannot read the repo cannot learn which
// pull request numbers exist on it — and the fallback's branch
// resolve and head persist sit behind that same gate.
(status, "pull_request_status", "authorize_repo_read("),
// Bucket C — signer-self: the acting DID is matched/bound to auth.0
(tasks, "create_task", "did_matches("),
// #496: create_task is signer-self AND owner-when-repo-scoped — a
@@ -1199,7 +1171,6 @@ mod tests { async fn t() { let _ = did_matches(x, y); } }
(include_str!("replicas.rs"), "replicas.rs"),
(include_str!("repos.rs"), "repos.rs"),
(include_str!("stars.rs"), "stars.rs"),
(include_str!("status/mod.rs"), "status/mod.rs"),
(include_str!("visibility.rs"), "visibility.rs"),
(include_str!("webhooks.rs"), "webhooks.rs"),
];
+2 -283
View File
@@ -76,7 +76,6 @@ pub async fn create_pr(
status: "open".to_string(),
merged_by_did: None,
merged_at: None,
head_commit: None,
created_at: now.clone(),
updated_at: now,
};
@@ -227,15 +226,6 @@ pub async fn merge_pr(
let disk_path = guard.path().to_path_buf();
let merger_did = auth.0;
// The source head this merge is about to consume, read under the write lock.
// The PR row above was loaded before the lock, so a push landing in between
// already moved the branch and the row's own `head_commit` is stale; the
// value frozen on the merged PR has to be what was merged. `None` when the
// ref cannot be resolved, which leaves whatever the push path last stored.
let merged_source_head =
store::read_ref(&disk_path, &store::local_branch_ref(&pr.source_branch))
.ok()
.flatten();
// Snapshot the target ref's pre-merge OID so a refused durable upload can
// rewind the local merge, mirroring the close_issue / create_issue
// compensating pattern. A read failure or an absent ref (a first-commit
@@ -284,10 +274,7 @@ pub async fn merge_pr(
let merge_sha = merge_result.map_err(|e| AppError::Git(e.to_string()))?;
state
.db
.merge_pr(&pr.id, &merger_did, merged_source_head.as_deref())
.await?;
state.db.merge_pr(&pr.id, &merger_did).await?;
let _ = state.db.touch_repo(&record.id).await;
webhooks::fire_event(
@@ -337,36 +324,7 @@ pub async fn close_pr(
));
}
// Same refusal merge_pr gives, before the head is resolved: a repeated close
// must not store the branch's current tip on a historical pull request.
if pr.status != "open" {
return Err(AppError::BadRequest(format!("PR is already {}", pr.status)));
}
// Freeze the head this close decided against, the same snapshot merge_pr
// takes on its path: a closed pull request's rollup serves only the stored
// head, so a PR closed before its first status read would report
// head_resolved=false forever. The recorded push for the branch is the
// canonical answer; the disk read covers a push whose event write failed.
// A failed lookup fails the close, leaving the pull request open to retry:
// it must not read as an absent head. An unresolvable head keeps whatever
// the push path last stored.
let branch_ref = store::local_branch_ref(&pr.source_branch);
let closed_head = if pr.head_commit.is_some() {
None
} else {
state
.db
.latest_push_sha_for_ref(&record.id, &branch_ref)
.await?
.or_else(|| {
store::read_ref(std::path::Path::new(&record.disk_path), &branch_ref)
.ok()
.flatten()
})
.map(|sha| sha.to_ascii_lowercase())
};
state.db.close_pr(&pr.id, closed_head.as_deref()).await?;
state.db.close_pr(&pr.id).await?;
webhooks::fire_event(
std::sync::Arc::clone(&state.db),
@@ -512,243 +470,6 @@ pub async fn list_comments(
Ok(Json(serde_json::json!({ "comments": comments })))
}
#[cfg(test)]
mod tests {
use super::*;
use axum::body::Body;
use axum::http::{Method, StatusCode};
use axum::routing::post;
use axum::Router;
use std::process::Command;
use tower::ServiceExt;
const OWNER_DID: &str = "did:key:z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH";
const STALE_SHA: &str = "9999999999999999999999999999999999999999";
fn git(dir: &std::path::Path, args: &[&str]) {
let out = Command::new("git")
.args(args)
.current_dir(dir)
.env("GIT_AUTHOR_NAME", "t")
.env("GIT_AUTHOR_EMAIL", "[email protected]")
.env("GIT_COMMITTER_NAME", "t")
.env("GIT_COMMITTER_EMAIL", "[email protected]")
.output()
.expect("git runs");
assert!(
out.status.success(),
"git {args:?} failed: {}",
String::from_utf8_lossy(&out.stderr)
);
}
/// A bare repo with `main` and a `feature` branch one commit ahead.
/// Returns the bare path and the real `feature` tip.
fn bare_repo_with_feature(
root: &std::path::Path,
owner_slug: &str,
) -> (std::path::PathBuf, String) {
let work = root.join("work");
std::fs::create_dir_all(&work).unwrap();
git(&work, &["init", "-q", "-b", "main", "."]);
std::fs::write(work.join("a.txt"), "one").unwrap();
git(&work, &["add", "."]);
git(&work, &["commit", "-qm", "one"]);
git(&work, &["checkout", "-qb", "feature"]);
std::fs::write(work.join("b.txt"), "two").unwrap();
git(&work, &["add", "."]);
git(&work, &["commit", "-qm", "two"]);
git(&work, &["checkout", "-q", "main"]);
let bare_dir = root.join(owner_slug);
std::fs::create_dir_all(&bare_dir).unwrap();
let bare = bare_dir.join("demo.git");
git(
root,
&[
"clone",
"-q",
"--bare",
work.to_str().unwrap(),
bare.to_str().unwrap(),
],
);
let tip = Command::new("git")
.args(["rev-parse", "refs/heads/feature"])
.current_dir(&bare)
.output()
.unwrap();
let tip = String::from_utf8_lossy(&tip.stdout).trim().to_string();
assert_eq!(tip.len(), 40, "feature tip must resolve");
(bare, tip)
}
/// Merging freezes the stored head at the source commit the merge actually
/// consumed. The PR row is loaded before the write lock is taken, so a push
/// that lands in between leaves the row's own `head_commit` stale; seeding a
/// value that matches nothing on disk stands in for that race, and the
/// merged PR must still come out pointing at the real `feature` tip.
#[sqlx::test]
async fn merge_stamps_the_source_head_it_merged_over_a_racing_value(pool: sqlx::PgPool) {
let tmp = tempfile::tempdir().unwrap();
let owner_slug = OWNER_DID.replace([':', '/'], "_");
let (bare, feature_tip) = bare_repo_with_feature(tmp.path(), &owner_slug);
let mut state = crate::test_support::test_state(pool.clone()).await;
state.repo_store =
crate::git::repo_store::RepoStore::for_testing(tmp.path().to_path_buf(), pool);
let now = Utc::now();
let repo = crate::db::RepoRecord {
id: uuid::Uuid::new_v4().to_string(),
name: "demo".into(),
owner_did: OWNER_DID.into(),
description: None,
is_public: true,
default_branch: "main".into(),
created_at: now,
updated_at: now,
disk_path: bare.to_string_lossy().into_owned(),
forked_from: None,
machine_id: None,
};
state.db.create_repo(&repo).await.unwrap();
let pr = PullRequest {
id: Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
title: "add b".into(),
body: None,
author_did: OWNER_DID.into(),
source_branch: "feature".into(),
target_branch: "main".into(),
status: "open".into(),
merged_by_did: None,
merged_at: None,
head_commit: None,
created_at: now.to_rfc3339(),
updated_at: now.to_rfc3339(),
};
state.db.create_pr(&pr).await.unwrap();
// The stale value the racing push scenario leaves on the row.
state
.db
.set_open_pr_heads(&repo.id, "feature", STALE_SHA)
.await
.unwrap();
let db = state.db.clone();
let app = Router::new()
.route(
"/api/v1/repos/{owner}/{repo}/pulls/{number}/merge",
post(merge_pr),
)
.with_state(state);
let response = app
.oneshot(crate::test_support::signed_request_as(
OWNER_DID,
Method::POST,
"/api/v1/repos/z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH/demo/pulls/1/merge",
Body::empty(),
))
.await
.unwrap();
assert_eq!(response.status(), StatusCode::OK);
let merged = db.get_pr(&repo.id, 1).await.unwrap().unwrap();
assert_eq!(merged.status, "merged");
assert_eq!(
merged.head_commit,
Some(feature_tip),
"the merge must stamp the source head it consumed, not the racing value"
);
}
/// A failed head lookup is not an absent head. With the push-event table
/// unreadable, the close must fail while the pull request is still open, so
/// the caller can retry. Closing with a NULL head would leave the rollup
/// unresolved for good, since a closed pull request serves only the stored
/// head. The branch exists on disk, so a swallowed error would also close
/// successfully through the disk fallback: that is the case this pins.
#[sqlx::test]
async fn close_fails_open_when_the_head_lookup_errors(pool: sqlx::PgPool) {
let tmp = tempfile::tempdir().unwrap();
let owner_slug = OWNER_DID.replace([':', '/'], "_");
let (bare, _tip) = bare_repo_with_feature(tmp.path(), &owner_slug);
let mut state = crate::test_support::test_state(pool.clone()).await;
state.repo_store =
crate::git::repo_store::RepoStore::for_testing(tmp.path().to_path_buf(), pool.clone());
let now = Utc::now();
let repo = crate::db::RepoRecord {
id: uuid::Uuid::new_v4().to_string(),
name: "demo".into(),
owner_did: OWNER_DID.into(),
description: None,
is_public: true,
default_branch: "main".into(),
created_at: now,
updated_at: now,
disk_path: bare.to_string_lossy().into_owned(),
forked_from: None,
machine_id: None,
};
state.db.create_repo(&repo).await.unwrap();
let pr = PullRequest {
id: Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
title: "add b".into(),
body: None,
author_did: OWNER_DID.into(),
source_branch: "feature".into(),
target_branch: "main".into(),
status: "open".into(),
merged_by_did: None,
merged_at: None,
head_commit: None,
created_at: now.to_rfc3339(),
updated_at: now.to_rfc3339(),
};
state.db.create_pr(&pr).await.unwrap();
sqlx::query("ALTER TABLE repo_push_events RENAME TO repo_push_events_gone")
.execute(&pool)
.await
.unwrap();
let db = state.db.clone();
let app = Router::new()
.route(
"/api/v1/repos/{owner}/{repo}/pulls/{number}/close",
post(close_pr),
)
.with_state(state);
let response = app
.oneshot(crate::test_support::signed_request_as(
OWNER_DID,
Method::POST,
"/api/v1/repos/z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH/demo/pulls/1/close",
Body::empty(),
))
.await
.unwrap();
assert!(
response.status().is_server_error(),
"a failed head lookup must fail the close, got {}",
response.status()
);
let after = db.get_pr(&repo.id, 1).await.unwrap().unwrap();
assert_eq!(after.status, "open", "the pull request must stay open");
assert_eq!(after.head_commit, None);
}
}
#[cfg(test)]
mod merge_rollback_tests {
use super::*;
@@ -852,7 +573,6 @@ mod merge_rollback_tests {
status: "open".to_string(),
merged_by_did: None,
merged_at: None,
head_commit: None,
created_at: now.clone(),
updated_at: now,
})
@@ -923,7 +643,6 @@ mod lock_pool_shed_tests {
state
.db
.create_pr(&PullRequest {
head_commit: None,
id: Uuid::new_v4().to_string(),
repo_id: repo_id.clone(),
number: 1,
File diff suppressed because it is too large. Load diff
-665
View File
@@ -1,665 +0,0 @@
//! Commit status claims: the write path and the combined read.
//!
//! POST /api/v1/repos/:owner/:repo/statuses/:sha — append one claim (owner only)
//! GET /api/v1/repos/:owner/:repo/commits/:sha/status — the combined projection
//!
//! Claims are append-only: a producer reporting twice for the same context
//! leaves both rows and the visible status is a projection over the history.
//! Reporting twice means two REQUESTS, though. An exact repeat of one already
//! accepted is answered with 200 and the row it wrote, not a second row: the
//! projection elects the highest `seq`, so a replayed claim would not duplicate
//! a verdict but overturn the one that superseded it.
use axum::extract::{Extension, Path, State};
use axum::http::StatusCode;
use axum::Json;
use chrono::Utc;
use serde::{Deserialize, Serialize};
use uuid::Uuid;
use crate::auth::AuthenticatedDid;
use crate::db::StatusClaim;
use crate::error::{AppError, Result};
use crate::state::AppState;
/// Claims for one (repo, commit, producer, context). Generous for a real
/// producer; a producer past it is misbehaving.
const MAX_CLAIMS_PER_TUPLE: i64 = 100;
/// Distinct contexts for one (repo, commit). The context string is caller-chosen
/// and free-form, so the tuple cap alone bounds nothing.
const MAX_CONTEXTS_PER_COMMIT: i64 = 50;
/// Claim rows for one repo within the trailing window the db layer counts over.
/// The commit SHA is caller-chosen and never existence-checked, so a writer at
/// the two caps above can still fan out over fresh 40-hex strings without this
/// one. It bounds the RATE of that fan-out rather than a lifetime total, because
/// nothing prunes the table: a lifetime bound would close the surface on a repo
/// permanently while answering with a status every client retries.
const MAX_CLAIMS_PER_REPO_PER_WINDOW: i64 = 10_000;
#[derive(Deserialize)]
#[serde(deny_unknown_fields)]
pub struct CreateStatusRequest {
/// Payload format version. The signature covers these bytes, so the
/// field set is the signed format: `deny_unknown_fields` keeps a future
/// producer's claim from silently reading as this one, and `v` names the
/// version explicitly so a later revision has somewhere to live. Absent
/// means 1.
pub v: Option<u32>,
pub state: String,
pub context: String,
pub target_url: Option<String>,
pub description: Option<String>,
}
/// The wire state set (KTD-1): GitHub's four commit-status states, so absence
/// stays distinguishable without a fifth value no client understands.
const CLAIM_STATES: [&str; 4] = ["error", "failure", "pending", "success"];
/// Bounds on the signature material this write path persists verbatim. Every one
/// of the four is caller-influenced: the headers come off the request, the
/// signing string grows with the component list the caller's Signature-Input
/// chose, and the body is the caller's. Without a bound the claim log's row size
/// is set by whoever writes to it, so these are explicit and generous rather than
/// implicit and absent. A conforming `sign_request` produces roughly 100, 200 and
/// 400 bytes for the first three.
const MAX_SIGNATURE_CHARS: usize = 512;
const MAX_SIGNATURE_INPUT_CHARS: usize = 1024;
const MAX_SIGNING_STRING_CHARS: usize = 4096;
/// The body is a `CreateStatusRequest`, whose own fields are already capped at
/// roughly 3.3 KB in total by the three limits below; this leaves room for JSON
/// framing and nothing more.
///
/// Enforced twice, at two different moments, because the handler's check alone
/// is not the bound it advertises: `require_signature` buffers the whole body
/// before any handler runs, so by the time [`create_status`] measures it the
/// allocation has already happened. `build_router` applies the same number as a
/// `RequestBodyLimitLayer` at the transport, outside the auth layers, which is
/// what makes it a bound on what is ALLOCATED and not only on what is STORED.
/// The handler keeps its own check regardless — it is the one that holds for a
/// handler reached by any other route table, and it is what the material size
/// tests drive.
pub(crate) const MAX_REQUEST_BODY_BYTES: usize = 8192;
const MAX_CONTEXT_CHARS: usize = 255;
const MAX_TARGET_URL_CHARS: usize = 2048;
const MAX_DESCRIPTION_CHARS: usize = 1024;
/// The 201 body for an accepted claim: what the client needs to identify the row
/// it just wrote, and nothing else.
///
/// Deliberately not the stored [`StatusClaim`]. That struct carries the signature
/// material, and serializing it echoed the signature, the signing string and the
/// whole request body back on every write — the body as a JSON array of integers.
/// Write-time provenance belongs in the row, not on the wire, which is the same
/// line [`StatusEntry`] draws on the read side.
#[derive(Serialize)]
pub struct CreatedStatus {
pub id: String,
pub repo_id: String,
pub commit_sha: String,
pub state: String,
pub context: String,
pub target_url: Option<String>,
pub description: Option<String>,
pub producer_did: String,
pub created_at: String,
}
impl CreatedStatus {
fn from_claim(claim: StatusClaim) -> Self {
Self {
id: claim.id,
repo_id: claim.repo_id,
commit_sha: claim.commit_sha,
state: claim.state,
context: claim.context,
target_url: claim.target_url,
description: claim.description,
producer_did: claim.producer_did,
created_at: claim.created_at,
}
}
}
/// POST /api/v1/repos/:owner/:repo/statuses/:sha
pub async fn create_status(
State(state): State<AppState>,
Extension(auth): Extension<AuthenticatedDid>,
Path((owner, name, sha)): Path<(String, String, String)>,
material: Option<Extension<crate::auth::SignatureMaterial>>,
Json(req): Json<CreateStatusRequest>,
) -> Result<(StatusCode, Json<CreatedStatus>)> {
// Read-visibility first, then owner, and the order is the security property.
// authorize_repo_read denies a quarantined repo before the visibility gate and
// answers with the repo's own not-found, byte-identical to a missing repo, so
// a caller who cannot read the repo cannot learn it exists. Loading the repo
// and comparing the owner would answer 403 there, turning this endpoint into
// an existence oracle. require_repo_owner then 403s a non-owner of a repo the
// caller can read, where existence is not secret.
let (record, _rules) =
crate::api::authorize_repo_read(&state, &owner, &name, Some(&auth.0), "/").await?;
crate::api::require_repo_owner(&record, &auth.0)?;
let commit_sha = normalize_sha(&sha)?;
if req.v.is_some_and(|v| v != 1) {
return Err(AppError::BadRequest(
"unsupported status payload version".into(),
));
}
let claim_state = validate_state(&req.state)?;
let context = validate_context(&req.context)?;
let target_url = validate_target_url(req.target_url.as_deref())?;
let description = validate_description(req.description.as_deref())?;
// KTD-5: the signature material is captured here because the request carries
// the only copy, and `signing_string` is the exact byte sequence the
// middleware verified. Its absence means this handler was reached without
// `require_signature` — a server misconfiguration (a route group that lost
// its auth layer, a handler mounted somewhere new), not anything the client
// did, so it is a 500 and no row is written. Storing an empty payload instead
// would leave every claim from that moment on unverifiable with nothing going
// red, which is exactly the absence-renders-as-success shape.
let Some(Extension(material)) = material else {
return Err(AppError::Internal(anyhow::anyhow!(
"status claim write reached without verified signature material — \
the route is not behind require_signature"
)));
};
// The body is carried only behind the `PersistsSignedBody` marker, which
// this route's group applies outside the auth layers. Its absence means the
// marker did not reach the middleware, because the layer was dropped or
// reordered. That is the same class of misconfiguration as the branch above
// and gets the same answer. Storing an empty body instead would write claims
// that carry a signature over a digest of bytes nobody kept, which is
// exactly the case `stored_claim_re_verifies_and_a_tampered_row_does_not`
// exists to make impossible.
let Some(body) = material.body.clone() else {
return Err(AppError::Internal(anyhow::anyhow!(
"status claim write reached with signature material carrying no request body: \
the route lost its persist-body marker or applied it inside the auth layers"
)));
};
// Bounded before anything is written. The material is verified, which says
// the caller holds the key, not that what they signed is a reasonable size.
bound("signature", material.signature.len(), MAX_SIGNATURE_CHARS)?;
bound(
"signature_input",
material.signature_input.len(),
MAX_SIGNATURE_INPUT_CHARS,
)?;
bound(
"signing_string",
material.signing_string.len(),
MAX_SIGNING_STRING_CHARS,
)?;
bound("request body", body.len(), MAX_REQUEST_BODY_BYTES)?;
// After the bounds, so the scan below walks a signing string of known size.
require_body_bound_signature(&material, &body)?;
let digest = request_digest(&material, &body);
let (signature, signature_input, signing_string, request_body) = (
material.signature,
material.signature_input,
material.signing_string,
body.to_vec(),
);
let claim = StatusClaim {
id: Uuid::new_v4().to_string(),
// Ignored on insert: the database assigns the ordering key (KTD-3).
seq: 0,
repo_id: record.id.clone(),
commit_sha,
state: claim_state,
context,
target_url,
description,
// Both are the owner identity today (KTD-5); they split when delegated
// capabilities land.
producer_did: auth.0.clone(),
authorizing_did: auth.0,
signature,
signature_input,
signing_string,
request_body,
request_digest: digest,
created_at: Utc::now().to_rfc3339(),
};
let caps = crate::db::ClaimCaps {
per_tuple: MAX_CLAIMS_PER_TUPLE,
contexts_per_commit: MAX_CONTEXTS_PER_COMMIT,
per_repo_window: MAX_CLAIMS_PER_REPO_PER_WINDOW,
};
match state.db.insert_status_claim_capped(&claim, &caps).await? {
crate::db::ClaimInsert::Inserted(_) => {
Ok((StatusCode::CREATED, Json(CreatedStatus::from_claim(claim))))
}
// Already recorded, so 200 and the original row rather than 201 and a
// second one. The claim the caller gets back is the stored one, id
// included, which is what makes a retry safe to treat as success.
crate::db::ClaimInsert::AlreadyRecorded(existing) => {
Ok((StatusCode::OK, Json(CreatedStatus::from_claim(*existing))))
}
crate::db::ClaimInsert::CapExceeded(which) => Err(AppError::TooManyRequests(format!(
"claim limit reached for {which}"
))),
}
}
/// Refuse a claim whose signature does not cover the body about to be stored.
///
/// A claim IS provenance, so the signature stored beside it has to bind the
/// bytes stored beside it. It does not do that directly: `signing_string`
/// reaches the body only through a `content-digest` line. And a caller can sign
/// a content-digest the request then never carries. `require_signature` rebuilds
/// the covered values from the request, so an ABSENT `Content-Digest` header
/// makes the covered digest the empty string on the signing side and the
/// verifying side alike; the Ed25519 check passes over method, path and an empty
/// digest, and any body at all rides along. The row that write produces asserts
/// a provenance that cannot be re-verified: the signing string covers a digest
/// of bytes that are not the ones in `request_body`.
///
/// The predicate here is the write-time form of the one `re_verify` runs against
/// a stored row in the tests — recompute the digest of the bytes about to be
/// persisted and require the signing string to carry exactly that line — so what
/// this route accepts and what a third party can later re-verify are the same
/// question, asked at both ends.
///
/// This is a route property, checked where the claim becomes history, and it is
/// deliberately not the only line of defence: requiring the header in
/// `require_signature` closes the same hole one layer earlier and for every
/// signed route, and that change belongs there rather than here because no other
/// route's correctness should depend on the status handler. The two agree and
/// neither substitutes for the other — without the middleware's requirement the
/// empty digest is still accepted everywhere else, and without this one a later
/// relaxation of the middleware would silently resume writing unverifiable
/// history.
///
/// A 400, not a 500: the material is exactly what the client signed, and what
/// they signed does not cover what they sent.
fn require_body_bound_signature(
material: &crate::auth::SignatureMaterial,
body: &[u8],
) -> Result<()> {
let covered = format!(
"\"content-digest\": {}",
gitlawb_core::http_sig::compute_content_digest(body)
);
// Line-exact rather than a substring search: `build_signing_string` emits one
// component per line, and a `contains` would also be satisfied by the digest
// appearing INSIDE some other covered component's value, which proves
// nothing about what the signature covered.
if material.signing_string.lines().any(|line| line == covered) {
return Ok(());
}
Err(AppError::BadRequest(
"the signature does not cover this request body: send a Content-Digest header \
computed over the body and name \"content-digest\" among the signature's covered \
components"
.into(),
))
}
/// The identity of one signed write, as a hex sha-256.
///
/// All four inputs together, because a replay is the same bytes arriving twice:
/// the signature, the input that names what it covers, the canonical string it
/// was verified over, and the body that string covers through a content-digest.
/// The signature alone would nearly do (it covers the other three transitively),
/// but hashing what is actually stored means the digest describes the row rather
/// than a claim about it.
///
/// Each field is length-prefixed so no two different requests can concatenate to
/// the same bytes: without it, a signature ending in some prefix of the next
/// field would collide with the shorter signature that absorbed it.
///
/// This is not a nonce, and deliberately so. A nonce table needs pruning and a
/// pruning window is a second replay window; the claim row IS the record of what
/// was accepted, so uniqueness on it is self-maintaining.
fn request_digest(material: &crate::auth::SignatureMaterial, body: &[u8]) -> String {
use sha2::{Digest, Sha256};
let mut h = Sha256::new();
for part in [
material.signature.as_bytes(),
material.signature_input.as_bytes(),
material.signing_string.as_bytes(),
body,
] {
h.update((part.len() as u64).to_be_bytes());
h.update(part);
}
format!("{:x}", h.finalize())
}
/// One reported context in the combined response. Signature material is
/// deliberately absent: it is write-time provenance, not read-surface data.
#[derive(Serialize)]
pub struct StatusEntry {
pub state: String,
pub context: String,
pub target_url: Option<String>,
pub description: Option<String>,
pub producer_did: String,
pub created_at: String,
}
/// The combined commit status. `state` never leaves the four-value set (KTD-1),
/// so absence is carried by `total_count` 0 with the pending state rather than a
/// fifth value, and `reported_only` (R19) says out loud that the state covers the
/// contexts that reported, not every check a caller expected.
#[derive(Serialize)]
pub struct CombinedStatus {
pub state: String,
pub sha: String,
pub total_count: usize,
pub statuses: Vec<StatusEntry>,
pub reported_only: bool,
}
/// GET /api/v1/repos/:owner/:repo/commits/:sha/status
///
/// The auth extension is optional and MUST be last in the extractor list: the
/// route group carries `optional_signature`, so an unsigned caller reaches here
/// with no `AuthenticatedDid` at all and a public repo still answers.
pub async fn commit_status(
State(state): State<AppState>,
Path((owner, name, sha)): Path<(String, String, String)>,
auth: Option<Extension<AuthenticatedDid>>,
) -> Result<Json<CombinedStatus>> {
let caller = auth.as_ref().map(|Extension(a)| a.0.as_str());
// The gate runs first and on the requested path, before any claim data is
// touched. Its deny is the repo's own not-found, byte-identical to a missing
// repo, so the status surface cannot answer "does this repo exist".
let (record, _rules) =
crate::api::authorize_repo_read(&state, &owner, &name, caller, "/").await?;
let commit_sha = normalize_sha(&sha)?;
let statuses = project_claims(&state, &record, &commit_sha).await?;
Ok(Json(CombinedStatus {
state: combined_state(&statuses).to_string(),
sha: commit_sha,
total_count: statuses.len(),
statuses,
reported_only: true,
}))
}
/// The projection, in one place. Both read surfaces call it, so R3's "the rollup
/// is derived by the same projection as the commit read" holds by construction
/// rather than by two implementations happening to agree.
///
/// KTD-5: current authorization, not write-time authorization. An ownership
/// transfer drops the prior owner's claims from the projection while the
/// append-only history keeps them.
async fn project_claims(
state: &AppState,
record: &crate::db::RepoRecord,
commit_sha: &str,
) -> Result<Vec<StatusEntry>> {
let claims = state
.db
.latest_status_claims(&record.id, commit_sha, &record.owner_did)
.await?;
Ok(claims
.into_iter()
.map(|c| StatusEntry {
state: c.state,
context: c.context,
target_url: c.target_url,
description: c.description,
producer_did: c.producer_did,
created_at: c.created_at,
})
.collect())
}
/// The pull request head rollup (R11). `state` stays inside the four wire values
/// whatever happened to the head (KTD-1): an unresolvable head is carried by
/// `head_resolved`, alongside the pull request's own state, so a client can tell
/// "the head could not be resolved" from "the head resolved and nothing reported"
/// without a fifth state value no client understands.
#[derive(Serialize)]
pub struct PullRequestStatus {
pub number: i64,
/// The pull request's own state: open, closed, or merged.
pub pull_request_state: String,
pub head_resolved: bool,
/// The target commit, absent exactly when `head_resolved` is false.
pub sha: Option<String>,
pub state: String,
pub total_count: usize,
pub statuses: Vec<StatusEntry>,
pub reported_only: bool,
}
/// GET /api/v1/repos/:owner/:repo/pulls/:number/status
///
/// Same optional-auth shape as the commit read, and the auth extension MUST stay
/// last in the extractor list.
pub async fn pull_request_status(
State(state): State<AppState>,
Path((owner, name, number)): Path<(String, String, i64)>,
auth: Option<Extension<AuthenticatedDid>>,
) -> Result<Json<PullRequestStatus>> {
let caller = auth.as_ref().map(|Extension(a)| a.0.as_str());
// The gate runs first, on the requested path, before the pull request row is
// loaded. Its deny is the repo's own not-found, byte-identical to a missing
// repo. Once it passes, a missing pull request number is the plain not-found:
// existence is no longer secret.
let (record, _rules) =
crate::api::authorize_repo_read(&state, &owner, &name, caller, "/").await?;
let pr = state
.db
.get_pr(&record.id, number)
.await?
.ok_or_else(|| AppError::NotFound(format!("PR #{number} not found")))?;
let head = rollup_head(&state, &record, &pr).await?;
let statuses = match &head {
Some(sha) => project_claims(&state, &record, sha).await?,
None => Vec::new(),
};
Ok(Json(PullRequestStatus {
number: pr.number,
pull_request_state: pr.status,
head_resolved: head.is_some(),
sha: head,
state: combined_state(&statuses).to_string(),
total_count: statuses.len(),
statuses,
reported_only: true,
}))
}
/// Branch-head lookups performed by the rollup fallback, counted so a test can
/// assert WORK DONE rather than only the answer returned. A fallback that
/// re-resolved on every read and then discarded the answer would be invisible in
/// the response and visible here.
#[cfg(test)]
pub(crate) static BRANCH_RESOLVES: std::sync::atomic::AtomicUsize =
std::sync::atomic::AtomicUsize::new(0);
/// The rollup's target commit (KTD-4).
///
/// The stored head wins whenever it is set. It is frozen at close or merge, so a
/// closed or merged pull request keeps the commit the decision was made against,
/// and only an OPEN pull request with no stored head falls back to the branch.
///
/// The fallback is one database read: the latest push this node recorded for the
/// pull request's source branch, from `repo_push_events`. It deliberately does
/// NOT go through `git::store` ref listing, which needs an acquired repository
/// path — on a cold node that downloads the whole repository from object storage,
/// and this read group carries no rate limiter, so an anonymous caller could
/// drive repeated downloads with a URL.
///
/// `repo_push_events` is the source rather than `branch_cids` because
/// `record_push_events` writes it for every ref update unconditionally, whereas
/// the sole writer of `branch_cids` sits behind a pin CID. A node with no object
/// pinning configured never writes that table, so a resolve keyed on it could
/// never answer there and every open pull request without a stored head reported
/// `head_resolved: false` forever. The residual limit is that only pushes taken
/// after this shipped have rows, so a branch last pushed before then still does
/// not resolve.
///
/// The resolved head is persisted, which makes an unauthenticated GET write. That
/// is only acceptable because the write is self-limiting: it fires exactly when
/// `head_commit` is absent and it fills it, and the fill is conditioned on that
/// same absence in SQL, so the PERSIST happens at most once per pull request.
/// The resolve attempt itself is not once-only — a pull request whose branch does
/// not resolve stores nothing, so there is nothing to short-circuit on and the
/// lookup runs again on every read until it succeeds.
async fn rollup_head(
state: &AppState,
record: &crate::db::RepoRecord,
pr: &crate::db::PullRequest,
) -> Result<Option<String>> {
if let Some(stored) = pr.head_commit.clone() {
return Ok(Some(stored));
}
if pr.status != "open" {
return Ok(None);
}
// The push path records the full ref while a pull request stores the bare
// branch name, the same mismatch `crate::api::repos::branch_from_ref`
// reconciles on the write side; this is that mapping run backwards. The empty
// branch is excluded for the same reason it is there: `refs/heads/` is not a
// branch and nothing can have written it.
if pr.source_branch.is_empty() {
return Ok(None);
}
let ref_name = crate::git::store::local_branch_ref(&pr.source_branch);
#[cfg(test)]
BRANCH_RESOLVES.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
let resolved = state
.db
.latest_push_sha_for_ref(&record.id, &ref_name)
.await?
// A recorded head that is not a commit SHA is no target at all, and
// storing it would leave the rollup pointing at nothing forever.
.and_then(|sha| normalize_sha(&sha).ok());
let Some(sha) = resolved else {
return Ok(None);
};
// Best effort, and deliberately not `?`. The head is already resolved and the
// response does not depend on this write landing, so a transient database
// failure here must not turn a served read into a 500; the next read resolves
// again and re-attempts the fill. Same catch-and-log the sibling writes on the
// push path use (`update_open_pr_heads`, `record_push_events`).
if let Err(e) = state.db.set_pr_head_if_absent(&pr.id, &sha).await {
tracing::warn!(
err = %e,
pr_id = %pr.id,
"failed to persist a resolved pull request head; serving the resolved head anyway"
);
}
Ok(Some(sha))
}
/// KTD-1's fold: any error or failure yields failure, else any pending yields
/// pending, else success. An empty set is pending, never success — total absence
/// of a verdict is its own state and must not read as a pass (R10).
fn combined_state(statuses: &[StatusEntry]) -> &'static str {
if statuses.is_empty() {
return "pending";
}
if statuses
.iter()
.any(|s| s.state == "error" || s.state == "failure")
{
"failure"
} else if statuses.iter().any(|s| s.state == "pending") {
"pending"
} else if statuses.iter().all(|s| s.state == "success") {
"success"
} else {
// A stored state outside the vocabulary is not a pass: an
// unrecognized verdict folds toward failure, never silently to success.
"failure"
}
}
/// 40 hex characters, lowercased. The SHA is never existence-checked (KTD-7):
/// a claim describes a commit object, and the object may not have arrived yet.
fn normalize_sha(sha: &str) -> Result<String> {
if crate::api::repos::is_hex_sha(sha) {
Ok(sha.to_ascii_lowercase())
} else {
Err(AppError::BadRequest(
"commit sha must be exactly 40 hexadecimal characters".into(),
))
}
}
/// One size bound on persisted signature material, measured in bytes.
fn bound(what: &str, len: usize, max: usize) -> Result<()> {
if len > max {
return Err(AppError::BadRequest(format!(
"{what} must be at most {max} bytes"
)));
}
Ok(())
}
fn validate_state(state: &str) -> Result<String> {
if CLAIM_STATES.contains(&state) {
Ok(state.to_string())
} else {
Err(AppError::BadRequest(format!(
"state must be one of {}",
CLAIM_STATES.join(", ")
)))
}
}
/// The context is the projection key, so it is trimmed, bounded, and required to
/// be free of control characters.
fn validate_context(context: &str) -> Result<String> {
let trimmed = context.trim();
if trimmed.is_empty() || trimmed.chars().count() > MAX_CONTEXT_CHARS {
return Err(AppError::BadRequest(format!(
"context must be 1 to {MAX_CONTEXT_CHARS} characters"
)));
}
if trimmed.chars().any(char::is_control) {
return Err(AppError::BadRequest(
"context must not contain control characters".into(),
));
}
Ok(trimmed.to_string())
}
fn validate_target_url(url: Option<&str>) -> Result<Option<String>> {
let Some(url) = url else { return Ok(None) };
if url.chars().count() > MAX_TARGET_URL_CHARS {
return Err(AppError::BadRequest(format!(
"target_url must be at most {MAX_TARGET_URL_CHARS} characters"
)));
}
let lower = url.to_ascii_lowercase();
if lower.starts_with("http://") || lower.starts_with("https://") {
Ok(Some(url.to_string()))
} else {
Err(AppError::BadRequest(
"target_url must be an http or https URL".into(),
))
}
}
fn validate_description(description: Option<&str>) -> Result<Option<String>> {
match description {
Some(d) if d.chars().count() > MAX_DESCRIPTION_CHARS => Err(AppError::BadRequest(format!(
"description must be at most {MAX_DESCRIPTION_CHARS} characters"
))),
other => Ok(other.map(str::to_string)),
}
}
#[cfg(test)]
mod tests;
File diff suppressed because it is too large. Load diff
-168
View File
@@ -17,53 +17,6 @@ use crate::state::AppState;
#[derive(Clone, Debug)]
pub struct AuthenticatedDid(pub String);
/// The RFC 9421 material this request was actually verified against, injected
/// alongside [`AuthenticatedDid`] by `require_signature`.
///
/// A handler that records a signed claim as history has to store it: the header
/// values and the canonical signing string are gone once the request ends, and a
/// stored claim nobody can re-verify is not history. `signing_string` is the exact
/// byte sequence the Ed25519 verification succeeded over, not a rebuild — a
/// handler reconstructing it from headers would be verifying a different string
/// than the middleware did.
#[derive(Clone, Debug)]
pub struct SignatureMaterial {
/// The `Signature` header value.
pub signature: String,
/// The `Signature-Input` header value.
pub signature_input: String,
/// The canonical bytes the signature covered.
pub signing_string: String,
/// The buffered request body, on the routes that persist it. The signing
/// string covers it only through the content-digest, so without these bytes
/// a stored claim can be shown to carry *a* valid signature over *some*
/// digest and nothing more. Carried here because the body is consumed
/// downstream by the extractor and cannot be recovered afterwards.
///
/// `Option`, and populated only behind [`PersistsSignedBody`], because a
/// route that never stores the body has no use for a second handle to it.
/// The unlike-sized field is this one: the three above are header-derived
/// and bounded by the server's header limit, while a receive-pack POST
/// carries up to GITLAWB_MAX_PACK_BYTES (2 GB by default). `Bytes`, not
/// `Vec<u8>`, so the handle the marked routes do take is a refcount bump
/// over the buffer the middleware already holds rather than a copy.
pub body: Option<axum::body::Bytes>,
}
/// Marks a route group whose handler persists the signed request body.
///
/// Applied as an extension layer OUTSIDE the auth layers, so it is on the
/// request before [`require_signature`] decides whether to carry the body. A
/// marker applied inside them is never seen, and the failure is silent: the
/// handler still compiles, tests that inject material by hand still pass, and
/// production stores nothing. The status write handler refuses an absent body
/// for that reason.
///
/// A marker rather than a path check because `require_signature` must not learn
/// which URL persists claims; that knowledge belongs at the route table.
#[derive(Clone, Copy, Debug)]
pub struct PersistsSignedBody;
/// A UCAN that passed full chain validation, with the root issuer the chain
/// rests on. Inserted into request extensions by [`require_ucan_chain`] when
/// `X-Ucan` is present; absent when the header is.
@@ -422,28 +375,10 @@ async fn verify_signature(limit: usize, request: Request, next: Next) -> Respons
tracing::info!(did = %sig.key_id, "✓ authenticated request");
// The body travels only where it is stored. On every other signed route the
// middleware's handle would keep the buffer alive for the rest of the layer
// chain, until the handler's own extractor consumes the request.
let body = parts
.extensions
.get::<PersistsSignedBody>()
.map(|_| body_bytes.clone());
let material = SignatureMaterial {
signature: sig_header,
signature_input: sig_input,
signing_string,
body,
};
let mut request = Request::from_parts(parts, Body::from(body_bytes));
request
.extensions_mut()
.insert(AuthenticatedDid(sig.key_id.to_string()));
// Carry the verified material forward for handlers that persist a signed
// claim; none of it can be recovered once the request is gone.
request.extensions_mut().insert(material);
next.run(request).await
}
@@ -863,109 +798,6 @@ mod tests {
}
}
/// Reports what the signature middleware put in `SignatureMaterial.body`,
/// so a test can tell "captured" from "deliberately not captured" apart
/// from "the middleware never ran".
async fn report_body(material: Option<axum::Extension<SignatureMaterial>>) -> String {
match material {
Some(axum::Extension(m)) => match m.body {
Some(bytes) => format!("captured:{}", bytes.len()),
None => "absent".to_string(),
},
None => "no-material".to_string(),
}
}
fn signed_post(path: &str, body: &[u8]) -> Request {
let kp = Keypair::generate();
let signed = gitlawb_core::http_sig::sign_request(&kp, "POST", path, body);
Request::builder()
.method("POST")
.uri(path)
.header("content-digest", signed.content_digest)
.header("signature-input", signed.signature_input)
.header("signature", signed.signature)
.body(axum::body::Body::from(body.to_vec()))
.unwrap()
}
/// The body handle travels only on a route that asked for it.
///
/// Both arms drive a genuinely signed request through `require_signature`;
/// the only difference is the [`PersistsSignedBody`] marker layer. A
/// middleware that captured unconditionally would make both arms read
/// `captured`, and one that never read the marker would make both read
/// `absent`, so the pair pins the decision rather than the mechanism.
#[tokio::test]
async fn the_body_is_carried_only_when_the_route_marks_itself_as_persisting_it() {
let body = br#"{"state":"success"}"#;
// The marker layer is added AFTER the middleware layer, which is what
// makes it the outer of the two and therefore the one that runs first.
// This is the same relative position `build_router` uses on the status
// write group, where the marker sits outside `add_auth_layers`.
let marked = Router::new()
.route("/", axum::routing::post(report_body))
.layer(middleware::from_fn_with_state(
crate::auth::SignatureBodyLimit(65536),
require_signature,
))
.layer(axum::Extension(PersistsSignedBody));
let resp = marked.oneshot(signed_post("/", body)).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let seen = axum::body::to_bytes(resp.into_body(), 2048).await.unwrap();
assert_eq!(
String::from_utf8(seen.to_vec()).unwrap(),
format!("captured:{}", body.len()),
"a route carrying the persist marker must receive the buffered body"
);
let unmarked = Router::new()
.route("/", axum::routing::post(report_body))
.layer(middleware::from_fn_with_state(
crate::auth::SignatureBodyLimit(65536),
require_signature,
));
let resp = unmarked.oneshot(signed_post("/", body)).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let seen = axum::body::to_bytes(resp.into_body(), 2048).await.unwrap();
assert_eq!(
String::from_utf8(seen.to_vec()).unwrap(),
"absent",
"a signed route that never persists the body must not be handed a handle to it"
);
}
/// The marker is read from the request the middleware was given, and the
/// request it hands downstream still carries it. Both halves matter: the
/// first is the capture decision, the second is that rebuilding the request
/// from its parts does not drop extensions a later layer may want.
#[tokio::test]
async fn the_marker_survives_the_request_rebuild() {
async fn report_marker(marker: Option<axum::Extension<PersistsSignedBody>>) -> String {
match marker {
Some(_) => "marked".to_string(),
None => "unmarked".to_string(),
}
}
let app = Router::new()
.route("/", axum::routing::post(report_marker))
.layer(middleware::from_fn_with_state(
crate::auth::SignatureBodyLimit(65536),
require_signature,
))
.layer(axum::Extension(PersistsSignedBody));
let resp = app.oneshot(signed_post("/", b"{}")).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let seen = axum::body::to_bytes(resp.into_body(), 2048).await.unwrap();
assert_eq!(
String::from_utf8(seen.to_vec()).unwrap(),
"marked",
"the marker must reach the handler, so the middleware saw it too"
);
}
#[tokio::test]
async fn require_ucan_chain_no_header_passes_through() {
let state = make_test_state(Keypair::generate().did());
File diff suppressed because it is too large. Load diff
-32
View File
@@ -1285,38 +1285,6 @@ async fn run_effect_bundle(
}
}
// 7a. Commit-status surfaces: the stored pull-request heads and the
// catch-up poll feed are durable accounting like everything else
// in this bundle, so they ride the same proven-landing authority —
// derived from the accepted transition rows, never from a live
// parse, and a landing reconcile proves only later still emits
// them here on the drain pass. Head writes check each branch's
// current tip before moving it: this executor runs after the repo
// write lock is released, so a newer push may already have moved
// the branch and only the still-current landing may win the head.
let landed: Vec<crate::api::repos::RefUpdate> = accepted_children
.iter()
.map(|c| crate::api::repos::RefUpdate {
old_sha: c.old_sha.clone(),
new_sha: c.new_sha.clone(),
ref_name: c.ref_name.clone(),
})
.collect();
let heads =
crate::api::repos::heads_still_at_tip(std::path::Path::new(&repo.disk_path), &landed)?;
if let Err(e) = crate::api::repos::update_open_pr_heads(&state.db, &req.repo_id, &heads).await {
return Ok(EffectsOutcome::Retry {
last_error: format!("stored pr heads: {e}"),
});
}
if let Err(e) =
crate::api::repos::record_push_events(&state.db, &req.id, accepted_children).await
{
return Ok(EffectsOutcome::Retry {
last_error: format!("push events: {e}"),
});
}
// 8. Per-ref certs and anchor jobs. Each accepted child gets one
// of each. Failures are accumulated; the first one is
// returned as the Retry reason. Anchor identity is by occurrence
-10
View File
@@ -379,16 +379,6 @@ mod tests {
);
}
/// A 429 does not advertise a retry interval: the status-claim caps behind
/// it are per-commit and append-only, so a client honoring a 1s hint would
/// retry a request that can never succeed.
#[test]
fn too_many_requests_carries_no_retry_after() {
let resp = AppError::TooManyRequests("x".into()).into_response();
assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS);
assert!(resp.headers().get("retry-after").is_none());
}
/// #226: raw sqlx/DB detail must never appear in the Internal 500 body.
#[tokio::test]
async fn internal_error_body_is_opaque() {
+6 -8
View File
@@ -683,14 +683,12 @@ pub fn list_refs(repo_path: &Path) -> Result<Vec<(String, String)>> {
/// a 40-char SHA-1 hex string. The reconcile compares two hex
/// strings.
///
/// `show-ref --verify` matches the exact ref name — unlike `rev-parse`,
/// which peels expressions, so it is the right primitive when the name is
/// caller-adjacent. Most production callers use [`read_ref_bounded`]; this
/// unbounded form also serves test harnesses that stage markers without a
/// timeout budget.
/// Production callers use [`read_ref_bounded`]; this unbounded form is kept
/// for test harnesses that stage markers without a timeout budget.
#[allow(dead_code)]
pub fn read_ref(repo_path: &Path, ref_name: &str) -> Result<Option<String>> {
// allow-unbounded-git: exact-name resolution; callers that need a timeout
// budget use read_ref_bounded.
// allow-unbounded-git: #[allow(dead_code)] harness form; production
// callers use read_ref_bounded.
let output = Command::new("git")
.args(["show-ref", "--verify", "--hash", ref_name])
.current_dir(repo_path)
@@ -1579,7 +1577,7 @@ fn guard_refs(target_branch: &str, source_branch: &str) -> Result<()> {
/// Stored short branch names are passed to git as explicit local-branch refs so a
/// syntax-valid name cannot be reinterpreted as a revision shorthand at the sink.
pub(crate) fn local_branch_ref(short_name: &str) -> String {
fn local_branch_ref(short_name: &str) -> String {
format!("refs/heads/{short_name}")
}
+1 -69
View File
@@ -15,7 +15,7 @@ use tracing::Level;
use crate::api::{
agents, arweave, bounties, certs, changelog, events, ipfs, issues, labels, peers, profiles,
protect, pulls, register, replicas, repos, resolve, stars, status, tasks, visibility, webhooks,
protect, pulls, register, replicas, repos, resolve, stars, tasks, visibility, webhooks,
};
use crate::auth;
use crate::rate_limit;
@@ -317,68 +317,6 @@ pub fn build_router(state: AppState) -> Router {
)
.write_braked(&write_ip_limiter);
// ── Status claim writes — signature, plus the per-DID throttle AND the
// per-IP flood brake `creation_routes` carries. Not in `write_routes`, which
// has neither: every call here appends a row, so an unthrottled writer grows
// the claim log on demand and the three per-repo caps would be the only bound.
let status_write_routes = add_auth_layers(
Router::new()
.route(
"/api/v1/repos/{owner}/{repo}/statuses/{sha}",
post(status::create_status),
)
.layer(middleware::from_fn(rate_limit::rate_limit_by_did))
.layer(axum::Extension(state.rate_limiter.clone())),
state.clone(),
SIGNED_BODY_LIMIT,
)
// The route's advertised body bound, applied at the transport. It sits
// OUTSIDE the auth layers because `require_signature` collects and hashes
// the whole body before any handler runs: the handler's own check measures
// an allocation that has already happened, so without this layer the 8 KiB
// figure bounded what gets stored and not what gets buffered, and
// concurrent requests multiplied exactly the cost it was written to
// prevent. It sits INSIDE the rate limiters so an oversized request is
// still counted by the per-IP brake before it is refused.
//
// A declared Content-Length over the limit is answered 413 without reading
// a byte; a body that declares nothing (or understates) is truncated at the
// limit and fails where it is read, which `require_signature` reports as an
// unreadable body.
.layer(RequestBodyLimitLayer::new(status::MAX_REQUEST_BODY_BYTES))
// The one route that persists the signed request body, so the one route
// whose signature material carries it. Applied OUTSIDE `add_auth_layers`
// (outermost = runs first) because `require_signature` reads this marker to
// decide whether to carry the body; a layer added inside the auth pair runs
// after the middleware and is never seen. `create_status` refuses an absent
// body rather than storing an empty column, so getting this order wrong
// fails loudly instead of quietly writing unverifiable claims.
.layer(axum::Extension(auth::PersistsSignedBody))
.layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
.layer(axum::Extension(rate_limit::IpRateLimiter {
limiter: state.create_ip_rate_limiter.clone(),
trust: state.push_limiter_trust,
}));
// ── Status claim reads — open, behind `optional_signature` so the handler
// sees the caller when one signs and still answers a public repo for an
// anonymous reader. Without the layer the handler's optional auth extension
// is None for EVERY caller, including the owner, and every private repo
// reads as not-found.
let status_read_routes = Router::new()
.route(
"/api/v1/repos/{owner}/{repo}/commits/{sha}/status",
get(status::commit_status),
)
.route(
"/api/v1/repos/{owner}/{repo}/pulls/{number}/status",
get(status::pull_request_status),
)
.layer(middleware::from_fn_with_state(
auth::SignatureBodyLimit(SIGNED_BODY_LIMIT),
auth::optional_signature,
));
// Body limit is raised to GITLAWB_MAX_PACK_BYTES (default 2 GB) for git
// routes only — all other API routes keep axum's default 2 MB cap.
// HTTP Signature is enforced on receive-pack (push) — the git-remote-gitlawb
@@ -640,10 +578,6 @@ pub fn build_router(state: AppState) -> Router {
"/api/v1/repos/{owner}/{repo}/events",
get(events::list_repo_events),
)
.route(
"/api/v1/repos/{owner}/{repo}/push-events",
get(events::list_repo_push_events),
)
.route("/api/v1/agents", get(agents::list_agents))
.route("/api/v1/agents/{did}", get(agents::show_agent))
.route("/api/v1/agents/{did}/trust", get(agents::get_trust))
@@ -749,8 +683,6 @@ pub fn build_router(state: AppState) -> Router {
.merge(profile_write_routes)
.merge(creation_routes)
.merge(write_routes)
.merge(status_write_routes)
.merge(status_read_routes)
.merge(git_write_routes)
.merge(git_read_routes)
.merge(issue_write_routes)
-1
View File
@@ -398,7 +398,6 @@ impl TestNode {
status: "open".to_string(),
merged_by_did: None,
merged_at: None,
head_commit: None,
created_at: now.clone(),
updated_at: now,
};
+1 -223
View File
@@ -184,153 +184,6 @@ pub(crate) fn signed_request_as(did: &str, method: Method, uri: &str, body: Body
.expect("request builder")
}
/// Slice a region out of a module's own source (obtained with `include_str!`)
/// and drop full-line `//` comments from it.
///
/// Several guards in `api/` assert that a piece of PRODUCTION code does or does
/// not contain something: that the receive-pack handler really calls the
/// push-event recorder, that the status module never acquires the repository.
/// Each guard carried its own copy of this parse, and three copies of a parser
/// is three places for the slicing to drift apart while every guard still reads
/// green.
///
/// `start` and `end` are literal anchors. The region runs from the first
/// occurrence of `start` (inclusive; the top of the file when `None`) to the
/// next occurrence of `end` (exclusive; end of file when `None`).
///
/// Returns `None` when an anchor is missing, rather than silently falling back
/// to a larger or empty region. Each caller supplies its own message for that
/// case and keeps its own assertion that the region really covers what it claims
/// to, which is what stops a bug in this one helper from turning every guard
/// that uses it into a vacuous pass.
///
/// Comment lines are stripped so that a doc comment naming the very call a guard
/// searches for cannot stand in for the call itself.
pub(crate) fn scrape_source_region(
src: &str,
start: Option<&str>,
end: Option<&str>,
) -> Option<String> {
let from = match start {
Some(anchor) => src.find(anchor)?,
None => 0,
};
let rest = &src[from..];
let to = match end {
// Search past the first CHARACTER, so an `end` anchor that also matches
// at the very start of the region cannot collapse it to nothing. A fixed
// one-byte skip does the same job on ASCII and lands mid-character on a
// multibyte first character, where the slice fails and the helper answers
// `None`, which is indistinguishable, to every caller, from a missing
// anchor.
Some(anchor) => {
let skip = rest.chars().next().map_or(0, char::len_utf8);
rest.get(skip..)?.find(anchor)? + skip
}
None => rest.len(),
};
Some(
rest[..to]
.lines()
.filter(|l| !l.trim_start().starts_with("//"))
.collect::<Vec<_>>()
.join("\n"),
)
}
#[cfg(test)]
mod scrape_source_region_tests {
use super::scrape_source_region;
const SRC: &str = "prelude line\nfn target() {\n // calls_the_thing()\n calls_the_thing();\n}\nfn later() {\n calls_the_thing();\n}\n";
#[test]
fn stops_at_the_end_anchor_so_a_later_function_cannot_satisfy_a_guard() {
let body = scrape_source_region(SRC, Some("fn target()"), Some("\n}")).unwrap();
assert!(body.contains("calls_the_thing();"));
assert!(
!body.contains("fn later()"),
"the region must stop at the first column-zero brace, got: {body}"
);
}
#[test]
fn strips_full_line_comments_so_a_doc_comment_cannot_stand_in_for_code() {
let commented = "fn target() {\n // calls_the_thing();\n}\n";
let body = scrape_source_region(commented, Some("fn target()"), Some("\n}")).unwrap();
assert!(
!body.contains("calls_the_thing"),
"a commented-out call must not survive the scrape, got: {body}"
);
}
#[test]
fn no_start_anchor_reads_from_the_top_of_the_file() {
let body = scrape_source_region(SRC, None, Some("fn later()")).unwrap();
assert!(body.starts_with("prelude line"));
assert!(!body.contains("fn later()"));
}
#[test]
fn no_end_anchor_reads_to_the_end_of_the_file() {
let body = scrape_source_region(SRC, Some("fn later()"), None).unwrap();
assert!(body.trim_end().ends_with('}'));
}
/// A missing anchor is None, never a silently wider or empty region: that is
/// what lets each caller keep its own "the thing I meant to scan is gone"
/// message instead of asserting against whatever the fallback produced.
#[test]
fn a_missing_anchor_is_none_rather_than_a_fallback_region() {
assert!(scrape_source_region(SRC, Some("fn absent()"), Some("\n}")).is_none());
assert!(scrape_source_region(SRC, Some("fn target()"), Some("~absent~")).is_none());
}
/// A region whose first character is multibyte is scraped, not reported
/// missing.
///
/// The end-anchor search skips the region's first CHARACTER. Skipping a
/// fixed byte instead lands mid-character here, the slice fails, and the
/// helper answers `None`, which every caller reads as "the anchor is gone"
/// and reports as a missing region. A guard that says "not found"
/// for input that is present is worse than one that fails loudly, and the
/// five callers sharing this helper all inherit it.
#[test]
fn a_multibyte_first_character_does_not_read_as_a_missing_anchor() {
let src = "é prelude\nfn target() {\n body();\n}\nfn later() {}\n";
let body = scrape_source_region(src, None, Some("\n}"))
.expect("a region starting on a multibyte character must be scraped");
assert!(
body.starts_with("é prelude") && body.contains("body();"),
"the region must cover the multibyte start through the end anchor, got: {body}"
);
assert!(
!body.contains("fn later()"),
"the region must still stop at the end anchor, got: {body}"
);
// The same through a multibyte START anchor, so the skip is measured
// from the region rather than from the top of the file.
let body = scrape_source_region(src, Some("é"), Some("\n}"))
.expect("a multibyte start anchor must be scraped");
assert!(body.starts_with('é'), "got: {body}");
}
/// The end anchor is searched past the region's first character, so a
/// region whose own start also matches the end anchor is not collapsed to
/// nothing.
#[test]
fn an_end_anchor_matching_at_the_start_does_not_collapse_the_region() {
let src = "\n}\nfn target() {\n body();\n}\n";
let body = scrape_source_region(src, None, Some("\n}")).unwrap();
assert!(
body.contains("fn target()") && body.contains("body();"),
"got: {body}"
);
}
}
/// A local endpoint whose TCP accept succeeds instantly but that never writes an
/// HTTP response, so any request against it stalls deterministically until the
/// caller's own timeout. (A non-routable address hangs only if the network
@@ -338,6 +191,7 @@ mod scrape_source_region_tests {
/// test pass for the wrong reason.) The accepted sockets are parked in the
/// spawned task, which dies with the test's runtime, so the peer never sees a
/// close mid-test.
#[allow(dead_code)]
pub(crate) async fn silent_http_endpoint() -> String {
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let endpoint = format!("http://{}", listener.local_addr().unwrap());
@@ -350,71 +204,6 @@ pub(crate) async fn silent_http_endpoint() -> String {
endpoint
}
/// The response body decoded as JSON.
#[allow(dead_code)] // shared response helper for api test mods
pub(crate) async fn body_json(resp: axum::response::Response) -> serde_json::Value {
let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
.await
.expect("body bytes");
serde_json::from_slice(&bytes).expect("json body")
}
/// Status plus the full response body, for the byte-identical deny comparison.
#[allow(dead_code)] // shared response helper for api test mods
pub(crate) async fn status_and_bytes(
resp: axum::response::Response,
) -> (axum::http::StatusCode, Vec<u8>) {
let status = resp.status();
let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
.await
.unwrap()
.to_vec();
(status, bytes)
}
/// A `PendingRefTransition` test row in `applied` state: the fields the
/// commit-status effect writers read (`ref_name`, `new_sha`, `ordinal`,
/// `repo_id`) are the parameters, the rest is inert filler matching the
/// shape `insert_pending_ref_transitions` writes. `applied_at` stays None
/// so the recorded event falls back to the per-push timestamp the writer
/// computes once for the whole batch — the shape a real push produces.
#[allow(dead_code)] // commit-status surface tests across api modules
pub(crate) fn pending_child(
request_id: &str,
repo_id: &str,
ref_name: &str,
old_sha: &str,
new_sha: &str,
ordinal: i32,
) -> crate::db::PendingRefTransition {
crate::db::PendingRefTransition {
id: crate::db::deterministic_id(&[
"pending_ref_transition",
request_id,
repo_id,
ref_name,
old_sha,
new_sha,
]),
request_id: request_id.to_string(),
repo_id: repo_id.to_string(),
ref_name: ref_name.to_string(),
old_sha: old_sha.to_string(),
new_sha: new_sha.to_string(),
pusher_did: "did:key:z6pusher".to_string(),
node_did: "did:key:z6node".to_string(),
signature_header: String::new(),
signature_input: String::new(),
content_digest: String::new(),
state: crate::db::pending_state::APPLIED.to_string(),
created_at: "2026-01-01T00:00:00Z".to_string(),
applied_at: None,
cancelled_at: None,
ordinal,
git_target_kind: Some("update".to_string()),
}
}
/// Body of the `for-each-ref` arm of a fake-git fixture in the
/// COLUMN SHAPE `blob_paths` phase 2 parses. The caller is
/// responsible for wrapping this in a full `case "$1" in ... esac`
@@ -2493,7 +2282,6 @@ mod tests {
// Insert the poisoned row DIRECTLY, as a pre-fix row or an ungated writer would.
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -2852,7 +2640,6 @@ mod tests {
state.db.create_repo(&repo).await.expect("seed repo");
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -2971,7 +2758,6 @@ mod tests {
state.db.create_repo(&repo).await.expect("seed repo");
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -3096,7 +2882,6 @@ mod tests {
state.db.create_repo(&repo).await.expect("seed repo");
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -3213,7 +2998,6 @@ mod tests {
state.db.create_repo(&repo).await.expect("seed repo");
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -3398,7 +3182,6 @@ mod tests {
state.db.create_repo(&repo).await.expect("seed repo");
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -3525,7 +3308,6 @@ mod tests {
state.db.create_repo(&repo).await.expect("seed repo");
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -3694,7 +3476,6 @@ mod tests {
state.db.create_repo(&repo).await.expect("seed repo");
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -3859,7 +3640,6 @@ mod tests {
state.db.create_repo(&repo).await.expect("seed repo");
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -4050,7 +3830,6 @@ mod tests {
state.db.create_repo(&repo).await.expect("seed repo");
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -4203,7 +3982,6 @@ mod tests {
let _ = std::fs::remove_file(&pwn);
let pr = crate::db::PullRequest {
head_commit: None,
id: uuid::Uuid::new_v4().to_string(),
repo_id: repo.id.clone(),
number: 1,
@@ -915,7 +915,6 @@ fn fill(path: &str, fixture: &Fixture, repo: &str, id_source: IdSource) -> Strin
};
path.replace("{owner}", &fixture.owner_did)
.replace("{repo}", repo)
.replace("{sha}", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
.replace("{number}", "1")
.replace("{id}", id)
.replace("{label}", "bug")
@@ -234,23 +234,6 @@ pub fn deny_bearing_routes() -> &'static [Row] {
id_source: IdSource::Fixed,
anon_read: AnonRead::Deny404,
},
// create_status: signed non-owner on a public repo passes the read
// gate and hits `require_repo_owner` — a plain owner-gate 403. The
// body is a minimal valid claim (the request is body-bound signed, so
// the probe's RFC-9421 content-digest coverage is load-bearing: a
// signer that did not cover the body would 400 before the gate).
Row {
method: "POST",
path: "/api/v1/repos/{owner}/{repo}/statuses/{sha}",
gate: GateClass::OwnerGate,
handler: "status::create_status",
body: Some(r#"{"state":"pending","context":"ci/build"}"#),
needs: NO_ENTITY,
reach: Reach::None,
principals: NO_PRINCIPAL,
id_source: IdSource::Fixed,
anon_read: AnonRead::Deny404,
},
// #195 (F1): close_pr / close_issue are owner-OR-author gates (pulls.rs:276,
// issues.rs:255). A plain OwnerGate tests only the owner arm, so reverting
// the author arm is invisible. MultiPrincipalGate drives BOTH arms.
@@ -801,50 +784,6 @@ pub fn deny_bearing_routes() -> &'static [Row] {
id_source: IdSource::Fixed,
anon_read: AnonRead::Deny404,
},
// pull_request_status: the seeded PRIVATE PR #1 is the reach target, so
// the owner twin gets a 200 rollup, not the absent-entity 404 the gate's
// own existence-hiding would make indistinguishable.
Row {
method: "GET",
path: "/api/v1/repos/{owner}/{repo}/pulls/{number}/status",
gate: GateClass::ReadGate,
handler: "status::pull_request_status",
body: None,
needs: &["priv_pr"],
reach: Reach::ReaderReads,
principals: NO_PRINCIPAL,
id_source: IdSource::Fixed,
anon_read: AnonRead::Deny404,
},
// commit_status / list_repo_push_events: repo-scoped optional-auth reads
// whose deny is the repo's own existence-hiding 404. Neither needs a
// seeded entity — a status rollup over zero claims and an empty
// push-event page are both valid 2xx responses, so the owner twin
// proving the gate is the repo lookup, not the data.
Row {
method: "GET",
path: "/api/v1/repos/{owner}/{repo}/commits/{sha}/status",
gate: GateClass::ReadGate,
handler: "status::commit_status",
body: None,
needs: NO_ENTITY,
reach: Reach::ReaderReads,
principals: NO_PRINCIPAL,
id_source: IdSource::Fixed,
anon_read: AnonRead::Deny404,
},
Row {
method: "GET",
path: "/api/v1/repos/{owner}/{repo}/push-events",
gate: GateClass::ReadGate,
handler: "events::list_repo_push_events",
body: None,
needs: NO_ENTITY,
reach: Reach::ReaderReads,
principals: NO_PRINCIPAL,
id_source: IdSource::Fixed,
anon_read: AnonRead::Deny404,
},
// get_cert: id-keyed by a real ref-certificate issued by an owner push to
// the private repo. No cert-content marker is seeded: get_cert read-gates
// the repo on "/" and 404s a non-reader BEFORE fetching the cert, so cert