mirror of
https://github.com/Twigpine/node.git
synced 2026-11-08 17:55:53 -10:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
923d47d491 | ||
|
|
f9942ee710 |
No files matched your search
@@ -188,12 +188,7 @@ export GITLAWB_NODE=http://localhost:7545
|
||||
git clone gitlawb://did:key:z6Mk.../my-repo
|
||||
```
|
||||
|
||||
For public-network use, make sure `GITLAWB_NODE` points to the node you want, over
|
||||
`https://`. The helper defaults to localhost for local development, and plaintext
|
||||
`http://` is allowed only to this machine: requests are signed but not encrypted, so
|
||||
a cleartext hop to a remote node exposes the pack contents and the `Signature`
|
||||
header. A remote `http://` node is refused, and `GITLAWB_ALLOW_INSECURE_HTTP=1`
|
||||
overrides that for a trusted private network.
|
||||
For public-network use, make sure `GITLAWB_NODE` points to the node you want. The helper defaults to localhost for local development.
|
||||
|
||||
### Full lifecycle against an iCaptcha-enforcing node
|
||||
|
||||
@@ -398,7 +393,7 @@ Important node settings:
|
||||
| `GITLAWB_P2P_BOOTSTRAP` | Comma-separated libp2p multiaddrs. |
|
||||
| `GITLAWB_BOOTSTRAP_DISABLE_SEEDS` | Disable embedded seed peers for isolated dev/test networks. |
|
||||
| `GITLAWB_REQUIRE_SIGNED_PEER_WRITES` | Require signed peer announce/sync writes. Defaults to `false` during the staged rollout below. |
|
||||
| `GITLAWB_ENFORCE_OWNER_PUSH` | Require the authenticated pusher to be the repo owner on `git-receive-pack`. **Defaults to `true`.** A `did:key` signature is authentication, not authorization (anyone can mint a key and sign), so with this off every signed caller may push to every repository, private ones included. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so they cannot push while this is on. Set `false` only for a rolling upgrade; see [`docs/RUN-A-NODE.md`](docs/RUN-A-NODE.md). |
|
||||
| `GITLAWB_ENFORCE_OWNER_PUSH` | Require the authenticated pusher to be the repo owner on `git-receive-pack`. **Defaults to `true`.** A `did:key` signature is authentication, not authorization — anyone can mint a key and sign — so with this off every signed caller may push to every repository, private ones included. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so they cannot push while this is on. Set `false` only for a rolling upgrade; see [`docs/RUN-A-NODE.md`](docs/RUN-A-NODE.md). |
|
||||
| `GITLAWB_AUTO_SYNC` | Enable automatic sync from known peers. |
|
||||
| `GITLAWB_MAX_PACK_BYTES` | Max git pack body size for smart-HTTP routes. |
|
||||
| `GITLAWB_GIT_SERVICE_TIMEOUT_SECS` | Max seconds a served git upload-pack, receive-pack, or `info/refs` advertisement may run before it is aborted (504). Default 600. Also bounds the withheld-blob classification walk (on both the upload-pack serve and receive-pack replication paths) and the push-side pin-candidate discovery (`rev-list` / `cat-file`), each reaped via process-group teardown at the deadline. On the path-scoped upload-pack path the classification walk and the pack serve share ONE deadline, so this value bounds their combined duration rather than granting each stage a full budget: a walk that consumes it leaves the serve nothing and the clone gets a 504. Serving large path-scoped repos may therefore need a higher value than they did when each stage was budgeted separately. Accepted range is 1 to 3153600000 (100 years), since the node derives deadlines from this value and a larger one cannot be represented. |
|
||||
|
||||
@@ -66,10 +66,6 @@ fn main() -> Result<()> {
|
||||
// v0.1: default to localhost. Override with GITLAWB_NODE env var.
|
||||
let node_base =
|
||||
std::env::var("GITLAWB_NODE").unwrap_or_else(|_| "http://127.0.0.1:7545".to_string());
|
||||
check_transport_security(
|
||||
&node_base,
|
||||
std::env::var_os("GITLAWB_ALLOW_INSECURE_HTTP").is_some(),
|
||||
)?;
|
||||
let repo_base = format!("{}/{}/{}", node_base, short_owner, repo_name);
|
||||
tracing::debug!("repo_base: {repo_base}");
|
||||
|
||||
@@ -82,76 +78,6 @@ fn main() -> Result<()> {
|
||||
run_helper(&repo_base, keypair.as_ref())
|
||||
}
|
||||
|
||||
/// Whether `url` is plaintext http, regardless of where it points.
|
||||
fn is_http(url: &str) -> bool {
|
||||
reqwest::Url::parse(url.trim())
|
||||
.map(|u| u.scheme() == "http")
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Whether `url` would send git data off this machine in cleartext.
|
||||
///
|
||||
/// True only for `http://` to a non-loopback host. RFC 9421 signs the request
|
||||
/// but does not encrypt it, so on a plaintext hop the pack contents and the
|
||||
/// `Signature` header are both readable, and a captured signature is replayable
|
||||
/// for its freshness window against any host.
|
||||
///
|
||||
/// Loopback is decided from the parsed address rather than a string match, so
|
||||
/// `127.0.0.2`, `[::1]` and an IPv4-mapped `[::ffff:127.0.0.1]` are all
|
||||
/// recognised as this machine. A value that does not parse, or that is not
|
||||
/// http(s), is not this guard's business and returns false: it fails later with
|
||||
/// its own error, and naming it a TLS problem would misdirect the reader.
|
||||
fn is_insecure_remote(url: &str) -> bool {
|
||||
let Ok(parsed) = reqwest::Url::parse(url.trim()) else {
|
||||
return false;
|
||||
};
|
||||
if parsed.scheme() != "http" {
|
||||
return false;
|
||||
}
|
||||
let Some(host) = parsed.host_str() else {
|
||||
return false;
|
||||
};
|
||||
let host = host.trim_end_matches('.').to_ascii_lowercase();
|
||||
if host == "localhost" {
|
||||
return false;
|
||||
}
|
||||
// host_str() keeps the brackets on an IPv6 literal.
|
||||
let bare = host.trim_start_matches('[').trim_end_matches(']');
|
||||
if let Ok(ip) = bare.parse::<std::net::IpAddr>() {
|
||||
if ip.is_loopback() {
|
||||
return false;
|
||||
}
|
||||
// An IPv4-mapped IPv6 literal hides a v4 loopback from is_loopback().
|
||||
if let std::net::IpAddr::V6(v6) = ip {
|
||||
if let Some(v4) = v6.to_ipv4_mapped() {
|
||||
if v4.is_loopback() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Refuse a cleartext hop off this machine unless the operator has opted in.
|
||||
///
|
||||
/// Fail closed: the alternative is signing a request and then handing it, and
|
||||
/// the pack it carries, to anyone on the path. `GITLAWB_ALLOW_INSECURE_HTTP`
|
||||
/// exists for a private LAN where the operator has decided that is acceptable.
|
||||
fn check_transport_security(node_base: &str, allow_insecure: bool) -> Result<()> {
|
||||
if allow_insecure || !is_insecure_remote(node_base) {
|
||||
return Ok(());
|
||||
}
|
||||
bail!(
|
||||
"refusing to send git data to {node_base} over plaintext http.\n\
|
||||
Requests are signed but not encrypted, so the pack contents and the \
|
||||
Signature header are readable by anyone on the path, and a captured \
|
||||
signature can be replayed.\n\
|
||||
Use https://, or set GITLAWB_ALLOW_INSECURE_HTTP=1 to accept the risk \
|
||||
(for a trusted private network only)."
|
||||
)
|
||||
}
|
||||
|
||||
// ── CLI argument handling ──────────────────────────────────────────────────────
|
||||
|
||||
/// How the binary was invoked, derived from its CLI arguments.
|
||||
@@ -199,7 +125,6 @@ fn help_text() -> String {
|
||||
ENVIRONMENT:\n\
|
||||
\x20 GITLAWB_NODE Node base URL (default: http://127.0.0.1:7545)\n\
|
||||
\x20 GITLAWB_KEY Identity PEM path for signed fetch/push (default: ~/.gitlawb/identity.pem)\n\
|
||||
\x20 GITLAWB_ALLOW_INSECURE_HTTP Permit plaintext http:// to a non-loopback node (unset by default)\n\
|
||||
\x20 GITLAWB_LOG Log filter (default: warn)\n\
|
||||
\n\
|
||||
FLAGS:\n\
|
||||
@@ -274,8 +199,7 @@ fn handle_connect<R: Read>(
|
||||
other => bail!("unsupported git service: {other}"),
|
||||
}
|
||||
|
||||
// A loopback node must never be proxied; see build_http_client.
|
||||
let client = build_http_client(!is_insecure_remote(repo_base) && is_http(repo_base))?;
|
||||
let client = build_http_client()?;
|
||||
|
||||
// ── Phase 1: ref advertisement (GET /info/refs?service=<service>) ─────────
|
||||
//
|
||||
@@ -411,22 +335,8 @@ const HTTP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300);
|
||||
/// node named, and on a 307/308 the pack body went with them. Scope the follow to the
|
||||
/// origin that issued the redirect AND to an identical request-target, which is the
|
||||
/// same predicate `gl` uses.
|
||||
/// `bypass_proxy` must be true whenever the node is on this machine. reqwest's
|
||||
/// default client honours `HTTP_PROXY` / `ALL_PROXY` for a loopback URL too, so a
|
||||
/// proxy variable pointing off-machine turns an allowed local request into a
|
||||
/// cleartext hop carrying the `Signature` header, straight past
|
||||
/// `check_transport_security`. Verified: with `HTTP_PROXY` set, reqwest logged
|
||||
/// `proxy(...) intercepts 'http://127.0.0.1:7545/'` and dialled the proxy. A
|
||||
/// local node is never legitimately reached through a proxy, so this refuses one
|
||||
/// rather than trying to reimplement `NO_PROXY` parsing.
|
||||
fn build_http_client(bypass_proxy: bool) -> Result<reqwest::blocking::Client> {
|
||||
let builder = reqwest::blocking::Client::builder();
|
||||
let builder = if bypass_proxy {
|
||||
builder.no_proxy()
|
||||
} else {
|
||||
builder
|
||||
};
|
||||
Ok(builder
|
||||
fn build_http_client() -> Result<reqwest::blocking::Client> {
|
||||
Ok(reqwest::blocking::Client::builder()
|
||||
.timeout(HTTP_TIMEOUT)
|
||||
.redirect(reqwest::redirect::Policy::custom(same_origin_redirect))
|
||||
.build()?)
|
||||
@@ -919,148 +829,6 @@ fn resolve_key_path() -> std::path::PathBuf {
|
||||
|
||||
// ── Tests ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
#[cfg(test)]
|
||||
mod insecure_transport_tests {
|
||||
use super::*;
|
||||
|
||||
/// Loopback in every form the transport can legitimately be pointed at.
|
||||
/// Plaintext to this machine is the documented local-alpha default and must
|
||||
/// keep working, so a false positive here breaks every stock install.
|
||||
#[test]
|
||||
fn loopback_http_is_allowed() {
|
||||
for url in [
|
||||
"http://127.0.0.1:7545",
|
||||
"http://localhost:7545",
|
||||
"http://[::1]:7545",
|
||||
"http://127.0.0.2:7545",
|
||||
"http://[::ffff:127.0.0.1]:7545",
|
||||
"http://LocalHost:7545",
|
||||
] {
|
||||
assert!(
|
||||
!is_insecure_remote(url),
|
||||
"{url} is this machine; plaintext to it must stay allowed"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The case the guard exists for: cleartext git data leaving the machine.
|
||||
#[test]
|
||||
fn remote_http_is_refused() {
|
||||
for url in [
|
||||
"http://node.example.com:7545",
|
||||
"http://10.0.0.36:7777",
|
||||
"http://192.168.1.10",
|
||||
"http://[2001:db8::1]:7545",
|
||||
"http://8.8.8.8",
|
||||
] {
|
||||
assert!(
|
||||
is_insecure_remote(url),
|
||||
"{url} sends git data off-machine in cleartext and must be refused"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// TLS is always fine, loopback or not, so the guard keys on the scheme and
|
||||
/// not merely on the host being remote.
|
||||
#[test]
|
||||
fn https_is_always_allowed() {
|
||||
for url in [
|
||||
"https://node.gitlawb.com",
|
||||
"https://10.0.0.36:7777",
|
||||
"https://127.0.0.1:7545",
|
||||
] {
|
||||
assert!(!is_insecure_remote(url), "{url} is TLS and must be allowed");
|
||||
}
|
||||
}
|
||||
|
||||
/// Spellings that defeat a naive loopback check. `Url` normalizes the scheme
|
||||
/// and the host at parse time (lowercasing, and the WHATWG numeric forms), so
|
||||
/// these must all still read as this machine.
|
||||
#[test]
|
||||
fn loopback_spellings_are_normalized_not_string_matched() {
|
||||
for url in [
|
||||
"HTTP://127.0.0.1:7545",
|
||||
"Http://LOCALHOST:7545",
|
||||
"http://localhost.:7545",
|
||||
"http://user:[email protected]:7545",
|
||||
"http://2130706433:7545",
|
||||
"http://0x7f000001:7545",
|
||||
"http://[::ffff:7f00:1]:7545",
|
||||
] {
|
||||
assert!(
|
||||
!is_insecure_remote(url),
|
||||
"{url} resolves to this machine; plaintext to it must stay allowed"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The mirror of the above: an uppercase scheme must not become an escape
|
||||
/// from the guard, and a remote host in any spelling is still remote.
|
||||
#[test]
|
||||
fn uppercase_scheme_does_not_escape_the_guard() {
|
||||
for url in [
|
||||
"HTTP://node.example.com:7545",
|
||||
"Http://8.8.8.8",
|
||||
"http://NODE.EXAMPLE.COM",
|
||||
"http://user:[email protected]",
|
||||
] {
|
||||
assert!(
|
||||
is_insecure_remote(url),
|
||||
"{url} is a remote cleartext hop and must be refused"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The gate itself, both directions, including the opt-in escape hatch.
|
||||
#[test]
|
||||
fn gate_refuses_remote_plaintext_unless_opted_in() {
|
||||
let err = check_transport_security("http://node.example.com:7545", false)
|
||||
.expect_err("remote plaintext must be refused by default");
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("plaintext http"),
|
||||
"the refusal must say why: {msg}"
|
||||
);
|
||||
assert!(
|
||||
msg.contains("GITLAWB_ALLOW_INSECURE_HTTP"),
|
||||
"the refusal must name the escape hatch: {msg}"
|
||||
);
|
||||
|
||||
check_transport_security("http://node.example.com:7545", true)
|
||||
.expect("the opt-in must permit the same URL");
|
||||
check_transport_security("http://127.0.0.1:7545", false)
|
||||
.expect("the local default must keep working with no opt-in");
|
||||
check_transport_security("https://node.gitlawb.com", false)
|
||||
.expect("TLS must need no opt-in");
|
||||
}
|
||||
|
||||
/// The documented knob must appear in --help. A gate the operator cannot
|
||||
/// discover reads as a broken transport rather than a deliberate refusal.
|
||||
#[test]
|
||||
fn help_documents_the_opt_in() {
|
||||
assert!(help_text().contains("GITLAWB_ALLOW_INSECURE_HTTP"));
|
||||
}
|
||||
|
||||
/// An unparseable or non-http value is not classified as insecure here: it
|
||||
/// fails later with its own error, and reporting it as a TLS problem would
|
||||
/// send the reader after the wrong thing.
|
||||
#[test]
|
||||
fn unparseable_or_other_scheme_is_not_this_guards_problem() {
|
||||
for url in [
|
||||
"",
|
||||
" ",
|
||||
"not a url",
|
||||
"ftp://node.example.com",
|
||||
"file:///tmp/x",
|
||||
] {
|
||||
assert!(
|
||||
!is_insecure_remote(url),
|
||||
"{url:?} is not a cleartext-http-to-remote case"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -1161,7 +929,7 @@ mod tests {
|
||||
#[test]
|
||||
fn signed_requests_do_not_follow_a_redirect_off_the_node_origin() {
|
||||
let kp = Keypair::generate();
|
||||
let client = build_http_client(false).unwrap();
|
||||
let client = build_http_client().unwrap();
|
||||
|
||||
let mut elsewhere = mockito::Server::new();
|
||||
let never = elsewhere
|
||||
@@ -1263,7 +1031,7 @@ mod tests {
|
||||
#[test]
|
||||
fn a_same_origin_path_changing_redirect_is_refused() {
|
||||
let kp = Keypair::generate();
|
||||
let client = build_http_client(false).unwrap();
|
||||
let client = build_http_client().unwrap();
|
||||
|
||||
let mut node = mockito::Server::new();
|
||||
let bounce = node
|
||||
@@ -1314,7 +1082,7 @@ mod tests {
|
||||
#[test]
|
||||
fn an_identical_target_redirect_is_still_followed_up_to_the_chain_bound() {
|
||||
let kp = Keypair::generate();
|
||||
let client = build_http_client(false).unwrap();
|
||||
let client = build_http_client().unwrap();
|
||||
|
||||
let mut node = mockito::Server::new();
|
||||
let loop_route = node
|
||||
@@ -1475,7 +1243,7 @@ mod tests {
|
||||
fn a_rewritten_target_never_receives_the_signature() {
|
||||
let kp = Keypair::generate();
|
||||
let expected_did = kp.did().to_string();
|
||||
let client = build_http_client(false).unwrap();
|
||||
let client = build_http_client().unwrap();
|
||||
let slot = std::sync::Arc::new(std::sync::Mutex::new(None::<Verdict>));
|
||||
|
||||
let mut node = mockito::Server::new();
|
||||
@@ -1537,7 +1305,7 @@ mod tests {
|
||||
fn a_direct_signed_advertisement_verifies_under_the_node_verifier() {
|
||||
let kp = Keypair::generate();
|
||||
let expected_did = kp.did().to_string();
|
||||
let client = build_http_client(false).unwrap();
|
||||
let client = build_http_client().unwrap();
|
||||
let slot = std::sync::Arc::new(std::sync::Mutex::new(None::<Verdict>));
|
||||
|
||||
let mut node = mockito::Server::new();
|
||||
|
||||
@@ -267,15 +267,6 @@ fn write_response(mut stream: TcpStream, status: &str, content_type: &str, body:
|
||||
/// Run `git fetch` in `clone` through the helper, with a hard timeout so a
|
||||
/// regression to the deadlock fails fast instead of hanging the suite.
|
||||
fn fetch_with_helper(clone: &Path, node_url: &str) -> (bool, std::process::Output) {
|
||||
fetch_with_helper_env(clone, node_url, &[])
|
||||
}
|
||||
|
||||
/// [`fetch_with_helper`] with extra environment for the helper process.
|
||||
fn fetch_with_helper_env(
|
||||
clone: &Path,
|
||||
node_url: &str,
|
||||
extra_env: &[(&str, &str)],
|
||||
) -> (bool, std::process::Output) {
|
||||
let helper_bin = PathBuf::from(env!("CARGO_BIN_EXE_git-remote-gitlawb"));
|
||||
let helper_dir = helper_bin.parent().unwrap().to_path_buf();
|
||||
let path_env = match std::env::var_os("PATH") {
|
||||
@@ -298,14 +289,7 @@ fn fetch_with_helper_env(
|
||||
// machine/CI with git-l10n installed and LANG set to a translated locale.
|
||||
.env("LC_ALL", "C")
|
||||
.env("GITLAWB_NODE", node_url)
|
||||
.env("GITLAWB_KEY", "/nonexistent-key-for-anon-fetch")
|
||||
// The helper inherits this process's environment, so an operator or CI
|
||||
// that exports the insecure-HTTP override would decide the transport
|
||||
// policy for every test below. Clear it and let each test opt in.
|
||||
.env_remove("GITLAWB_ALLOW_INSECURE_HTTP");
|
||||
for (k, v) in extra_env {
|
||||
cmd.env(k, v);
|
||||
}
|
||||
.env("GITLAWB_KEY", "/nonexistent-key-for-anon-fetch");
|
||||
run_bounded(cmd, Duration::from_secs(30))
|
||||
}
|
||||
|
||||
@@ -1258,71 +1242,3 @@ fn run_bounded_bounds_join_when_leader_exits_leaving_a_pipe_holder() {
|
||||
held pipes do not stall the joins)"
|
||||
);
|
||||
}
|
||||
|
||||
/// The plaintext-transport gate, proven at its CALL SITE rather than in
|
||||
/// isolation. `main.rs`'s unit tests cover `is_insecure_remote` and
|
||||
/// `check_transport_security` as functions; neither notices if the call in
|
||||
/// `main()` is deleted, which is the regression that would silently restore the
|
||||
/// cleartext hop. Only driving the built binary binds the wiring.
|
||||
///
|
||||
/// `192.0.2.1` is TEST-NET-1 (RFC 5737): non-loopback, reserved for
|
||||
/// documentation, and never routable, so the gate is what stops this and no
|
||||
/// connection is attempted even if it regressed.
|
||||
#[test]
|
||||
fn real_git_fetch_refuses_a_remote_plaintext_node() {
|
||||
// A shallow fixture is enough: the gate fires before any negotiation.
|
||||
let repos = build_divergent_repos(1);
|
||||
let (ok, out) = fetch_with_helper(&repos.clone, "http://192.0.2.1:7545");
|
||||
let stderr = String::from_utf8_lossy(&out.stderr);
|
||||
|
||||
assert!(
|
||||
!ok || !out.status.success(),
|
||||
"the fetch must not succeed: {stderr}"
|
||||
);
|
||||
assert!(
|
||||
stderr.contains("plaintext http"),
|
||||
"the helper must refuse the cleartext hop and say so; stderr was: {stderr}"
|
||||
);
|
||||
assert!(
|
||||
stderr.contains("GITLAWB_ALLOW_INSECURE_HTTP"),
|
||||
"the refusal must name the escape hatch; stderr was: {stderr}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A proxy variable must not divert a LOOPBACK node off this machine.
|
||||
///
|
||||
/// reqwest's default client honours `HTTP_PROXY` for a loopback URL, so before
|
||||
/// the fix a proxy pointing off-machine carried the signed plaintext request to
|
||||
/// it, past the transport guard, which only ever inspected the URL. Observed
|
||||
/// directly: `proxy(http://...:9999/) intercepts 'http://127.0.0.1:7545/'`.
|
||||
///
|
||||
/// The proxy here is a black hole on TEST-NET-1, so if the bypass regresses this
|
||||
/// fetch cannot succeed: it either hangs to the harness timeout or fails to
|
||||
/// connect. Success is only possible when the request went straight to the shim.
|
||||
#[test]
|
||||
fn real_git_fetch_ignores_a_proxy_for_a_loopback_node() {
|
||||
let repos = build_divergent_repos(1);
|
||||
let (server, clone) = (repos.server.clone(), repos.clone.clone());
|
||||
let shim = start_shim(server.clone(), ShimMode::Normal);
|
||||
|
||||
let (completed, out) = fetch_with_helper_env(
|
||||
&clone,
|
||||
&shim.base_url,
|
||||
&[
|
||||
("HTTP_PROXY", "http://192.0.2.1:9999"),
|
||||
("http_proxy", "http://192.0.2.1:9999"),
|
||||
("ALL_PROXY", "http://192.0.2.1:9999"),
|
||||
],
|
||||
);
|
||||
|
||||
assert!(
|
||||
completed,
|
||||
"the fetch did not finish; a proxied loopback request would stall here. stderr:\n{}",
|
||||
String::from_utf8_lossy(&out.stderr)
|
||||
);
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"a loopback fetch must ignore the proxy variables. stderr:\n{}",
|
||||
String::from_utf8_lossy(&out.stderr)
|
||||
);
|
||||
}
|
||||
@@ -3153,6 +3153,12 @@ struct RefUpdate {
|
||||
ref_name: String,
|
||||
}
|
||||
|
||||
/// A git object id is 40 hex chars; length alone is not enough, since a peer
|
||||
/// can send 40 arbitrary bytes (#398).
|
||||
fn is_hex_sha(s: &str) -> bool {
|
||||
s.len() == 40 && s.bytes().all(|b| b.is_ascii_hexdigit())
|
||||
}
|
||||
|
||||
/// Parse git receive-pack pkt-line ref updates from the request body.
|
||||
/// Format per line: `<40-hex-old> <40-hex-new> <refname>[NUL capabilities]\n`
|
||||
fn parse_ref_updates(body: &[u8]) -> Vec<RefUpdate> {
|
||||
@@ -3194,7 +3200,7 @@ fn parse_ref_updates(body: &[u8]) -> Vec<RefUpdate> {
|
||||
.trim_end_matches('\n');
|
||||
|
||||
let parts: Vec<&str> = line.splitn(3, ' ').collect();
|
||||
if parts.len() == 3 && parts[0].len() == 40 && parts[1].len() == 40 {
|
||||
if parts.len() == 3 && is_hex_sha(parts[0]) && is_hex_sha(parts[1]) {
|
||||
updates.push(RefUpdate {
|
||||
old_sha: parts[0].to_string(),
|
||||
new_sha: parts[1].to_string(),
|
||||
@@ -3488,6 +3494,45 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// #398: a peer-supplied pkt-line whose 40-byte "SHA" fields carry non-hex
|
||||
// content must be dropped, not forwarded into RefUpdate events, branch_cid
|
||||
// rows, and sync-notify JSON as a canonical-looking identifier.
|
||||
#[test]
|
||||
fn parse_ref_updates_requires_hex_shas() {
|
||||
let sha_a = "a".repeat(40);
|
||||
let sha_b = "b".repeat(40);
|
||||
let pkt = |old: &str, new: &str, refname: &str| {
|
||||
let line = format!("{old} {new} {refname}");
|
||||
format!("{:04x}{}0000", line.len() + 4, line).into_bytes()
|
||||
};
|
||||
|
||||
// Valid 40-hex old + new -> accepted.
|
||||
let updates = parse_ref_updates(&pkt(&sha_a, &sha_b, "refs/heads/main"));
|
||||
assert_eq!(updates.len(), 1);
|
||||
assert_eq!(updates[0].old_sha, sha_a);
|
||||
assert_eq!(updates[0].new_sha, sha_b);
|
||||
assert_eq!(updates[0].ref_name, "refs/heads/main");
|
||||
|
||||
// 40-byte non-hex fields -> dropped.
|
||||
let junk = "Z".repeat(40);
|
||||
assert!(parse_ref_updates(&pkt(&junk, &sha_b, "refs/heads/main")).is_empty());
|
||||
assert!(parse_ref_updates(&pkt(&sha_a, &junk, "refs/heads/main")).is_empty());
|
||||
|
||||
// Wrong lengths -> dropped.
|
||||
assert!(parse_ref_updates(&pkt(&"a".repeat(39), &sha_b, "refs/heads/main")).is_empty());
|
||||
assert!(parse_ref_updates(&pkt(&sha_a, &"b".repeat(41), "refs/heads/main")).is_empty());
|
||||
|
||||
// A bad line among good ones drops only the bad line.
|
||||
let sha_c = "c".repeat(40);
|
||||
let bad = pkt(&junk, &sha_b, "refs/heads/bad");
|
||||
let good = pkt(&sha_a, &sha_c, "refs/heads/good");
|
||||
let mut mixed = bad[..bad.len() - 4].to_vec(); // strip trailing flush
|
||||
mixed.extend_from_slice(&good);
|
||||
let updates = parse_ref_updates(&mixed);
|
||||
assert_eq!(updates.len(), 1);
|
||||
assert_eq!(updates[0].ref_name, "refs/heads/good");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_service_app_error_classifies_timeout_bad_request_and_git() {
|
||||
// GitServiceTimeout carried through anyhow -> 504 Timeout.
|
||||
|
||||
Reference in new issue
Block a user