Compare commits

..
Author SHA1 Message Date
beardthelion f3acd33f01 test(git-remote): clear the insecure-HTTP override in the fetch harness
fetch_with_helper passes the test process environment through to the helper,
so an operator or CI exporting GITLAWB_ALLOW_INSECURE_HTTP decided the
transport policy for every test that runs through it. Reported by CodeRabbit
on the review of 12e68e54.

Confirmed by execution: with the variable exported,
real_git_fetch_refuses_a_remote_plaintext_node failed, because the helper
permitted the URL and the refusal never appeared on stderr. With the
env_remove in place the whole target passes with the variable still exported.

Cleared in the shared harness rather than the one test, so a later test
cannot inherit the same ambient policy. A test that wants the opt-in can pass
it through the extra_env argument.
2026-09-08 00:01:42 -05:00
beardthelion d58da1e6d2 fix(git-remote): bypass the proxy for a loopback node
The transport guard inspected the URL, but reqwest's default client decides
the actual destination, and it honours HTTP_PROXY / ALL_PROXY for a loopback
URL too. So a proxy variable pointing off-machine carried the signed
plaintext request to it, past a guard that had just allowed the URL as
local. Reported by Greptile on the review of 12e68e54.

Confirmed against the built binary before the change:

    reqwest::connect: proxy(http://10.0.0.36:9999/) intercepts 'http://127.0.0.1:7545/'
    hyper_util::client::legacy::connect::http: connecting to 10.0.0.36:9999

and after it:

    hyper_util::client::legacy::connect::http: connecting to 127.0.0.1:7545

build_http_client now takes bypass_proxy and calls no_proxy() when the node
is plaintext-and-local. A local node is never legitimately reached through a
proxy, so this refuses one outright rather than reimplementing NO_PROXY
matching, which is the kind of duplicated parsing that goes wrong quietly.
A TLS node keeps its proxy support, and a remote plaintext node is already
refused unless the operator opted in, in which case they accepted this.

The regression test drives a real git fetch against the loopback shim with
HTTP_PROXY, http_proxy and ALL_PROXY all pointed at a TEST-NET-1 black hole,
and requires it to succeed: only an unproxied request can. Verified
load-bearing by building the client with bypass_proxy hardcoded false, which
reddens it. Worth noting it reddens on the "did not finish" assertion rather
than the "must ignore the proxy" one, because a proxied loopback request
stalls to the harness timeout instead of failing fast.

#394 reached the same conclusion independently for the gl doctor probe,
which uses a dedicated no-proxy client for exactly this reason.
2026-09-07 22:53:22 -05:00
beardthelion 12e68e5463 fix(git-remote): refuse a plaintext node off this machine unless opted in
git-remote-gitlawb took GITLAWB_NODE verbatim and built every request URL
from it with no scheme check, so pointing it at a remote node over
http:// sent git data in cleartext. Verified against the built binary on
main: GITLAWB_NODE=http://10.0.0.36:7777 with a keypair loaded produced

    GET http://10.0.0.36:7777/z6Mkq.../probe-repo/info/refs?service=git-upload-pack
    hyper_util::client::legacy::connect::http: connecting to 10.0.0.36:7777

with no warning and no refusal.

RFC 9421 signs a request but does not encrypt it, so on a plaintext hop
the pack contents and the Signature header are both readable, and open
#253 records that a captured signature is replayable for around 600s and
is not bound to a target host. Plaintext is what turns that from a
compromised-endpoint problem into a passive-observer one.

The guard fails closed and keys on the parsed address, not a string
match. Url normalizes the scheme and host at parse time, so an uppercase
HTTP://, a trailing-dot FQDN, userinfo, and the WHATWG numeric spellings
(2130706433, 0x7f000001, [::ffff:7f00:1]) all still read as this machine.
Loopback plaintext is the documented local-alpha default, so a false
positive there would break every stock install.

GITLAWB_ALLOW_INSECURE_HTTP=1 is the escape hatch for a private LAN where
the operator has accepted the risk, and it is documented in --help. This
is a breaking change for anyone already running a remote node over
plaintext: they get an error naming the variable rather than a silent
downgrade.

Nine tests. Eight in main.rs cover the classifier over loopback, remote,
TLS, unparseable and adversarially-spelled inputs, the gate in both
directions including the opt-in, and that --help documents the variable.
The ninth drives a real git fetch through the built binary in
real_git_fetch.rs, because none of the other eight notice if the call in
main() is deleted, which is the regression that would silently restore
the cleartext hop. Three mutations verified load-bearing: neutering the
gate, breaking the localhost allowance, and deleting the call site.

Two things deliberately not in scope. gl reads the same GITLAWB_NODE and
signs against it, confirmed by execution, but its default is https and
the fix belongs in NodeClient::new across roughly forty call sites, so it
is its own change. And the check lives in the helper rather than
gitlawb-core because that crate is held to a dependency-purity job and
keeps `url` behind a feature; when #394 lands and moves resolution into a
shared resolve_transport_node, this folds into it.
2026-09-07 22:24:58 -05:00
7 changed files with 428 additions and 574 deletions

No files matched your search

Generated
+93 -12
View File
@@ -770,6 +770,23 @@ dependencies = [
"zeroize",
]
[[package]]
name = "ark-ff"
version = "0.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f7a806ac6c8307b929df4645776290a50ee2aac754ad09d8bdf73391309e43af"
dependencies = [
"ark-ff-asm 0.6.0",
"ark-ff-macros 0.6.0",
"ark-serialize 0.6.0",
"ark-std 0.6.0",
"digest 0.10.7",
"educe",
"num-bigint",
"num-traits",
"zeroize",
]
[[package]]
name = "ark-ff-asm"
version = "0.3.0"
@@ -800,6 +817,16 @@ dependencies = [
"syn 2.0.117",
]
[[package]]
name = "ark-ff-asm"
version = "0.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1479009684adc073dff49a1025d3a7065b317a9ead25aaaca38cdc70058ba8a2"
dependencies = [
"quote",
"syn 2.0.117",
]
[[package]]
name = "ark-ff-macros"
version = "0.3.0"
@@ -838,6 +865,19 @@ dependencies = [
"syn 2.0.117",
]
[[package]]
name = "ark-ff-macros"
version = "0.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4a0691ed21ef00ef89c1e9bda832eba493dda3ec2f8d892fb25b705f73f06bb8"
dependencies = [
"num-bigint",
"num-traits",
"proc-macro2",
"quote",
"syn 2.0.117",
]
[[package]]
name = "ark-serialize"
version = "0.3.0"
@@ -871,6 +911,30 @@ dependencies = [
"num-bigint",
]
[[package]]
name = "ark-serialize"
version = "0.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a74dd304fd536fb95d0a328e72be759209cc496a9da094c5bc56e5fea4f9e86b"
dependencies = [
"ark-serialize-derive",
"ark-std 0.6.0",
"digest 0.10.7",
"num-bigint",
"serde_with",
]
[[package]]
name = "ark-serialize-derive"
version = "0.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4f153690697a2b91e5e1251ff98411ee5371500a111a0fd317a70e588eb300f9"
dependencies = [
"proc-macro2",
"quote",
"syn 2.0.117",
]
[[package]]
name = "ark-std"
version = "0.3.0"
@@ -901,6 +965,16 @@ dependencies = [
"rand 0.8.6",
]
[[package]]
name = "ark-std"
version = "0.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "367c9c827ed431bff6868b7aa926e05b16eb46603cc8b6e768e4a5553fa1d155"
dependencies = [
"num-traits",
"rand 0.8.6",
]
[[package]]
name = "arrayref"
version = "0.3.9"
@@ -3300,19 +3374,22 @@ dependencies = [
[[package]]
name = "git-remote-gitlawb"
version = "0.5.1"
version = "0.7.1"
dependencies = [
"anyhow",
"gitlawb-core",
"libc",
"mockito",
"reqwest",
"tempfile",
"tracing",
"tracing-subscriber",
"windows-sys 0.61.2",
]
[[package]]
name = "gitlawb-attest"
version = "0.5.1"
version = "0.7.1"
dependencies = [
"base64",
"ed25519-dalek",
@@ -3329,7 +3406,7 @@ dependencies = [
[[package]]
name = "gitlawb-core"
version = "0.5.1"
version = "0.7.1"
dependencies = [
"anyhow",
"base64",
@@ -3350,13 +3427,14 @@ dependencies = [
"sha2",
"thiserror 2.0.18",
"tokio",
"url",
"uuid",
"zeroize",
]
[[package]]
name = "gitlawb-node"
version = "0.5.1"
version = "0.7.1"
dependencies = [
"alloy",
"anyhow",
@@ -3412,10 +3490,11 @@ dependencies = [
[[package]]
name = "gl"
version = "0.5.1"
version = "0.7.1"
dependencies = [
"alloy",
"anyhow",
"base64",
"chrono",
"clap",
"dirs",
@@ -3464,9 +3543,9 @@ dependencies = [
[[package]]
name = "h2"
version = "0.4.13"
version = "0.4.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2f44da3a8150a6703ed5d34e164b875fd14c2cdab9af1252a9a1020bde2bdc54"
checksum = "839c0e8a181239723652be9062bb56ca5bf5f64011f73b623f6f4fc59086a228"
dependencies = [
"atomic-waker",
"bytes",
@@ -3785,7 +3864,7 @@ dependencies = [
"libc",
"percent-encoding",
"pin-project-lite",
"socket2 0.6.3",
"socket2 0.5.10",
"tokio",
"tower-service",
"tracing",
@@ -3821,6 +3900,7 @@ version = "0.4.0"
dependencies = [
"anyhow",
"gitlawb-core",
"mockito",
"reqwest",
"serde",
"serde_json",
@@ -5352,7 +5432,7 @@ dependencies = [
"quinn-udp",
"rustc-hash",
"rustls",
"socket2 0.6.3",
"socket2 0.5.10",
"thiserror 2.0.18",
"tokio",
"tracing",
@@ -5389,7 +5469,7 @@ dependencies = [
"cfg_aliases",
"libc",
"once_cell",
"socket2 0.6.3",
"socket2 0.5.10",
"tracing",
"windows-sys 0.52.0",
]
@@ -5739,14 +5819,15 @@ dependencies = [
[[package]]
name = "ruint"
version = "1.17.2"
version = "1.20.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c141e807189ad38a07276942c6623032d3753c8859c146104ac2e4d68865945a"
checksum = "f5e99bff0393163bb25029a6af25d3d8d202ba5b5438a74d1bd8789f5c822970"
dependencies = [
"alloy-rlp",
"ark-ff 0.3.0",
"ark-ff 0.4.2",
"ark-ff 0.5.0",
"ark-ff 0.6.0",
"bytes",
"fastrlp 0.3.1",
"fastrlp 0.4.0",
+8 -3
View File
@@ -71,7 +71,7 @@ Known limitations:
- Agent lifecycle revocation is not enforced by HTTP Signature authorization; do not rely on removing or revoking an agent record to block a compromised signer.
- Read visibility is not a blanket data-classification boundary: task, IPFS-pin, and Arweave-anchor listings are not repository-gated; withheld path names can be visible to a root reader; and later visibility changes cannot retract content already announced or externally anchored.
- Peer writes are signed by upgraded nodes, but strict signed-peer enforcement is opt-in during rolling upgrades.
- GraphQL mutations require a signed identity (a verified signer bound to the acting DID) but are limited to the agent-task queue; repo writes are not exposed over GraphQL and remain REST-only.
- Current GraphQL mutations require an authenticated signer, but there is no mutation-specific guardrail that prevents a future mutation from omitting that check.
- Pull-request review comments do not yet have threaded line-level anchors, and merges do not enforce approval requirements.
See:
@@ -188,7 +188,12 @@ 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. The helper defaults to localhost for local development.
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.
### Full lifecycle against an iCaptcha-enforcing node
@@ -393,7 +398,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. |
+240 -8
View File
@@ -66,6 +66,10 @@ 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}");
@@ -78,6 +82,76 @@ 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.
@@ -125,6 +199,7 @@ 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\
@@ -199,7 +274,8 @@ fn handle_connect<R: Read>(
other => bail!("unsupported git service: {other}"),
}
let client = build_http_client()?;
// 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))?;
// ── Phase 1: ref advertisement (GET /info/refs?service=<service>) ─────────
//
@@ -335,8 +411,22 @@ 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.
fn build_http_client() -> Result<reqwest::blocking::Client> {
Ok(reqwest::blocking::Client::builder()
/// `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
.timeout(HTTP_TIMEOUT)
.redirect(reqwest::redirect::Policy::custom(same_origin_redirect))
.build()?)
@@ -829,6 +919,148 @@ 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::*;
@@ -929,7 +1161,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().unwrap();
let client = build_http_client(false).unwrap();
let mut elsewhere = mockito::Server::new();
let never = elsewhere
@@ -1031,7 +1263,7 @@ mod tests {
#[test]
fn a_same_origin_path_changing_redirect_is_refused() {
let kp = Keypair::generate();
let client = build_http_client().unwrap();
let client = build_http_client(false).unwrap();
let mut node = mockito::Server::new();
let bounce = node
@@ -1082,7 +1314,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().unwrap();
let client = build_http_client(false).unwrap();
let mut node = mockito::Server::new();
let loop_route = node
@@ -1243,7 +1475,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().unwrap();
let client = build_http_client(false).unwrap();
let slot = std::sync::Arc::new(std::sync::Mutex::new(None::<Verdict>));
let mut node = mockito::Server::new();
@@ -1305,7 +1537,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().unwrap();
let client = build_http_client(false).unwrap();
let slot = std::sync::Arc::new(std::sync::Mutex::new(None::<Verdict>));
let mut node = mockito::Server::new();
@@ -267,6 +267,15 @@ 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") {
@@ -289,7 +298,14 @@ fn fetch_with_helper(clone: &Path, node_url: &str) -> (bool, std::process::Outpu
// 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");
.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);
}
run_bounded(cmd, Duration::from_secs(30))
}
@@ -1242,3 +1258,71 @@ 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)
);
}
-548
View File
@@ -279,554 +279,6 @@ mod authz_guard {
}
}
/// Completeness fence over the GraphQL mutation surface: the mirror of
/// `every_in_scope_mutation_has_its_gate` for `graphql/mutation.rs`. Kept a
/// SEPARATE guard with its own table (never folded into the REST rows above):
/// a guard over a heterogeneous set must classify per member, and these
/// same-named task rows belong to the GraphQL surface, not `api/tasks.rs`.
///
/// COMPLETENESS comes from the engine's own SDL (`schema.sdl()`), parsed as a
/// structured AST (#219), NOT introspection and NOT a text parse of source.
/// This matters: async-graphql OMITS `#[graphql(visible = false)]` fields from
/// introspection while leaving them executable, so an introspection-sourced fence
/// could be defeated by a hidden mutation. SDL includes those fields, so no
/// visibility flag (and, via the AST parse, no brace-in-description quirk) can hide
/// a live mutation. The SDL field set must equal the registered set: a new field with
/// no row fails (a repo-write cannot slip in unlisted); a registered name absent from
/// the schema fails (renamed or removed). The set is also cross-checked against
/// introspection, so any field present in SDL but hidden from introspection fails
/// named as a forbidden hidden mutation.
///
/// The MARKER check is the only source-scraped part, and it is not the
/// completeness guarantee. Three honest limits, so a green check is not misread
/// as full authorization coverage:
/// 1. A present marker is not load-bearingness (the gate may not redden on a
/// hostile probe: the #203 present-but-vacuous case). The adversarial tests
/// in `graphql/mutation.rs` are the load-bearing layer.
/// 2. A present marker is not gate-correctness (it may be the wrong bucket for
/// the operation's resource: a repo-write registered as signer-self would
/// pass, so INV-1 review remains the defense).
/// 3. The marker check scrapes the comment-masked, brace-bounded resolver body;
/// a `{`/`}` or a `did_matches(` inside a STRING literal is not masked, so it
/// could mis-slice that body or satisfy the marker vacuously (the same
/// vacuous-marker class as limit 1). Completeness is unaffected: it comes
/// from the SDL AST, not from this scrape.
#[test]
fn every_graphql_mutation_has_its_gate() {
// (Rust snake name, schema field in camelCase, expected gate marker). The
// four are Bucket C signer-self (`did_matches(`); a repo-write would be
// Bucket A (`require_repo_owner(`). Same-named REST rows live in the guard
// above against `api/tasks.rs`.
let registered: &[(&str, &str, &str)] = &[
("create_task", "createTask", "did_matches("),
("claim_task", "claimTask", "did_matches("),
("complete_task", "completeTask", "did_matches("),
("fail_task", "failTask", "did_matches("),
];
// Completeness + hidden-mutation cross-check against the REAL schema (#219),
// delegated to the generic `assert_mutation_fence` so the SDL sourcing and the
// cross-check are ALSO exercised by a synthetic hidden schema in
// `fence_rejects_a_hidden_mutation` (INV-21). Because the real MutationRoot has no
// hidden field today, that synthetic test — not a hypothetical production canary —
// is what turns reverting the SDL source or deleting the cross-check RED.
{
use crate::graphql::mutation::MutationRoot;
use crate::graphql::query::QueryRoot;
use crate::graphql::subscription::SubscriptionRoot;
let schema =
async_graphql::Schema::build(QueryRoot, MutationRoot, SubscriptionRoot).finish();
let registered_camel: Vec<&str> = registered.iter().map(|(_, c, _)| *c).collect();
assert_mutation_fence(&schema, &registered_camel);
}
// Marker present (source scrape; the vacuity limit, backstopped by the
// adversarial tests): each resolver body carries its bucket marker.
let masked = mask_comments(include_str!("../graphql/mutation.rs"));
for (snake, _, marker) in registered {
let body = resolver_body(&masked, snake);
assert!(
body.contains(marker),
"GraphQL mutation `{snake}` is missing its gate marker `{marker}`: gate removed"
);
}
}
/// Version-safety self-check (#219, INV-21): the fence's completeness guarantee rests
/// on `schema.sdl()` INCLUDING `visible = false` fields while introspection omits
/// them. That is observed async-graphql behavior, not a documented contract, so pin
/// it here: a synthetic schema with a hidden mutation must have that field in its
/// SDL-derived set and NOT in its introspection-derived set. If a future
/// async-graphql upgrade ever filters SDL by visibility, the first assertion flips
/// RED — the assumption is load-bearing, not trusted.
#[test]
fn sdl_source_sees_hidden_mutations_that_introspection_omits() {
use async_graphql::{EmptySubscription, Object, Schema};
struct Q;
#[Object]
impl Q {
async fn ping(&self) -> i32 {
0
}
}
struct M;
#[Object]
impl M {
async fn visible_mut(&self) -> i32 {
1
}
#[graphql(visible = false)]
async fn hidden_mut(&self) -> i32 {
2
}
}
let schema = Schema::build(Q, M, EmptySubscription).finish();
// SDL source (the fix): includes the hidden field.
let sdl_fields = mutation_field_names_from_sdl(&schema.sdl());
assert!(
sdl_fields.iter().any(|f| f == "hiddenMut"),
"SDL-sourced set MUST include the visible=false mutation; if this is RED, \
async-graphql started filtering SDL by visibility and the fence's \
completeness guarantee is broken: {sdl_fields:?}"
);
// Introspection (the fence's own helper): OMITS the hidden field. This is the
// exact gap the fix closes — an introspection-sourced fence never sees `hiddenMut`.
let introspected = introspection_mutation_fields(&schema);
assert!(
!introspected.iter().any(|f| f == "hiddenMut"),
"introspection is expected to OMIT the visible=false mutation (that is the \
gap the SDL source closes); got {introspected:?}"
);
// The exact diff the fence computes: hidden ∈ (sdl \ introspection).
let hidden: Vec<&String> = sdl_fields
.iter()
.filter(|f| !introspected.iter().any(|i| i == *f))
.collect();
assert_eq!(
hidden,
vec!["hiddenMut"],
"the SDL-vs-introspection diff must surface exactly the hidden mutation"
);
}
/// INV-21 wiring guard (#219): drives the generic `assert_mutation_fence` — the SAME
/// helper the real fence calls — against a synthetic schema whose mutation is
/// `visible = false` and IS registered, so completeness passes and ONLY the
/// hidden-from-introspection cross-check can fail. This is what makes the fence's SDL
/// sourcing load-bearing without a production canary: reverting the source to
/// introspection (the hidden field then absent from the SDL set) or deleting the
/// cross-check removes this panic (or changes its message), turning the
/// `should_panic` test RED. The real MutationRoot has no hidden field, so nothing on
/// the real schema could catch that regression.
#[test]
#[should_panic(expected = "hidden from introspection")]
fn fence_rejects_a_hidden_mutation() {
use async_graphql::{EmptySubscription, Object, Schema};
struct Q;
#[Object]
impl Q {
async fn ping(&self) -> i32 {
0
}
}
struct M;
#[Object]
impl M {
async fn visible_mut(&self) -> i32 {
1
}
#[graphql(visible = false)]
async fn hidden_mut(&self) -> i32 {
2
}
}
let schema = Schema::build(Q, M, EmptySubscription).finish();
// Both fields registered, so completeness passes and only the cross-check fires.
assert_mutation_fence(&schema, &["visibleMut", "hiddenMut"]);
}
/// Deprecated-mutation guard (#219): a `#[graphql(deprecation = "...")]` mutation is
/// executable and present in SDL, but GraphQL introspection omits it by default. The
/// fence's introspection helper uses `includeDeprecated: true`, so a deprecated (but
/// visible) mutation is NOT mistaken for a hidden one. With it registered, the fence
/// must NOT panic. Remove `includeDeprecated: true` and this goes RED — the deprecated
/// field would look hidden and false-red as a forbidden visible=false mutation.
#[test]
fn fence_admits_a_deprecated_mutation() {
use async_graphql::{EmptySubscription, Object, Schema};
struct Q;
#[Object]
impl Q {
async fn ping(&self) -> i32 {
0
}
}
struct M;
#[Object]
impl M {
async fn visible_mut(&self) -> i32 {
1
}
#[graphql(deprecation = "use visibleMut")]
async fn deprecated_mut(&self) -> i32 {
2
}
}
let schema = Schema::build(Q, M, EmptySubscription).finish();
// Must not panic: the deprecated field is visible (present in introspection with
// includeDeprecated) and registered, so it is neither hidden nor unclassified.
assert_mutation_fence(&schema, &["visibleMut", "deprecatedMut"]);
}
/// Completeness + hidden-mutation cross-check for a BUILT schema (#219). Generic over
/// the schema type so the real fence AND the synthetic `fence_rejects_a_hidden_mutation`
/// / `fence_admits_a_deprecated_mutation` tests drive the SAME sourcing and cross-check.
/// Completeness is sourced from SDL (which includes `visible = false` fields); the
/// cross-check fails any SDL field hidden from introspection. This genericity is the
/// INV-21 fix: because the real MutationRoot has no hidden field, only a synthetic
/// hidden schema through THIS helper makes reverting the SDL source or removing the
/// cross-check RED, rather than leaving the class latent behind a production canary.
fn assert_mutation_fence<Q, M, S>(
schema: &async_graphql::Schema<Q, M, S>,
registered_camel: &[&str],
) where
Q: async_graphql::ObjectType + 'static,
M: async_graphql::ObjectType + 'static,
S: async_graphql::SubscriptionType + 'static,
{
let sdl_fields = mutation_field_names_from_sdl(&schema.sdl());
let introspected = introspection_mutation_fields(schema);
// No SDL field may be hidden from introspection (a visible=false mutation).
for f in &sdl_fields {
assert!(
introspected.iter().any(|i| i == f),
"GraphQL mutation `{f}` is in the schema but hidden from introspection \
(a `#[graphql(visible = false)]` field): hidden mutations are forbidden \
by this fence — make it visible so its classification is enforceable, or \
remove it"
);
}
// Completeness both directions against the registered (camelCase) set.
for f in &sdl_fields {
assert!(
registered_camel.contains(&f.as_str()),
"unclassified GraphQL mutation `{f}`: add it to \
every_graphql_mutation_has_its_gate with a deliberate gate \
(`require_repo_owner(` for a repo-write, `did_matches(` for a task op)"
);
}
for c in registered_camel {
assert!(
sdl_fields.iter().any(|f| f == c),
"registered GraphQL mutation `{c}` is not in the schema \
(renamed or removed); update the table"
);
}
}
/// The mutation field names via the engine's introspection, run against a passed
/// schema. STRUCTURED (names arrive as data). Uses `fields(includeDeprecated: true)`:
/// GraphQL's default omits `@deprecated` fields, but a `#[graphql(deprecation = "...")]`
/// mutation is still executable and present in SDL, so without this the cross-check
/// above would false-red a deprecated mutation as a hidden (visible=false) one. Only a
/// real visibility filter should trip that arm; deprecation is a lifecycle marker.
fn introspection_mutation_fields<Q, M, S>(
schema: &async_graphql::Schema<Q, M, S>,
) -> Vec<String>
where
Q: async_graphql::ObjectType + 'static,
M: async_graphql::ObjectType + 'static,
S: async_graphql::SubscriptionType + 'static,
{
use async_graphql::Value;
let resp = tokio::runtime::Runtime::new()
.expect("tokio runtime")
.block_on(schema.execute(
"{ __schema { mutationType { fields(includeDeprecated: true) { name } } } }",
));
assert!(
resp.errors.is_empty(),
"schema introspection failed: {:?}",
resp.errors
);
let mut names = Vec::new();
if let Value::Object(root) = &resp.data {
if let Some(Value::Object(sc)) = root.get("__schema") {
if let Some(Value::Object(mt)) = sc.get("mutationType") {
if let Some(Value::List(field_list)) = mt.get("fields") {
for field in field_list {
if let Value::Object(fo) = field {
if let Some(Value::String(n)) = fo.get("name") {
names.push(n.clone());
}
}
}
}
}
}
}
names
}
/// The mutation field names sourced from the built schema's SDL — the completeness
/// source (#219). Builds the same DB-free schema as the introspection helper and runs
/// its `.sdl()` (sync, no runtime needed) through [`mutation_field_names_from_sdl`].
/// SDL includes `#[graphql(visible = false)]` fields, so this source cannot be
/// defeated by hiding a mutation from introspection.
fn graphql_mutation_fields_from_sdl() -> Vec<String> {
use crate::graphql::mutation::MutationRoot;
use crate::graphql::query::QueryRoot;
use crate::graphql::subscription::SubscriptionRoot;
let schema =
async_graphql::Schema::build(QueryRoot, MutationRoot, SubscriptionRoot).finish();
mutation_field_names_from_sdl(&schema.sdl())
}
/// The mutation type's field names extracted from an async-graphql SDL string via
/// the engine's OWN parser AST (`async_graphql::parser::parse_schema`), NOT
/// introspection. This is the completeness source (#219): unlike introspection,
/// which async-graphql filters by `visible` (a `#[graphql(visible = false)]`
/// mutation is executable but ABSENT from introspection), `schema.sdl()` includes
/// every registered field, so a hidden mutation still appears here and must carry a
/// classified row. Parsing the AST (not a brace/line text scan) means a `}` inside a
/// field description cannot mis-slice the type block. camelCase, as async-graphql
/// emits. An empty result on a navigation miss is safe: the set-equality in
/// `every_graphql_mutation_has_its_gate` turns "no fields" into a loud failure for
/// every registered name, never a vacuous pass.
fn mutation_field_names_from_sdl(sdl: &str) -> Vec<String> {
use async_graphql::parser::parse_schema;
use async_graphql::parser::types::{TypeKind, TypeSystemDefinition};
let doc = parse_schema(sdl).expect("async-graphql SDL must parse");
// The mutation root type name: from the `schema { mutation: X }` block if
// present, else the GraphQL default "Mutation".
let mutation_type = doc
.definitions
.iter()
.find_map(|def| match def {
TypeSystemDefinition::Schema(s) => s
.node
.mutation
.as_ref()
.map(|m| m.node.as_str().to_string()),
_ => None,
})
.unwrap_or_else(|| "Mutation".to_string());
// The fields of that object type.
doc.definitions
.iter()
.find_map(|def| match def {
TypeSystemDefinition::Type(t) if t.node.name.node.as_str() == mutation_type => {
match &t.node.kind {
TypeKind::Object(obj) => Some(
obj.fields
.iter()
.map(|f| f.node.name.node.as_str().to_string())
.collect::<Vec<_>>(),
),
_ => None,
}
}
_ => None,
})
.unwrap_or_default()
}
#[test]
fn sdl_extractor_includes_a_hidden_visible_false_field() {
// A field that would be hidden from introspection must still be listed from SDL.
let sdl = "schema { query: QueryRoot mutation: MutationRoot }\n\
type QueryRoot { ping: Int! }\n\
type MutationRoot { createTask: Int! hiddenMut: Int! }\n";
let fields = mutation_field_names_from_sdl(sdl);
assert!(
fields.iter().any(|f| f == "createTask"),
"visible field must be listed: {fields:?}"
);
assert!(
fields.iter().any(|f| f == "hiddenMut"),
"a visible=false field (present in SDL, hidden from introspection) MUST be \
listed by the completeness source: {fields:?}"
);
}
#[test]
fn sdl_extractor_is_not_fooled_by_a_brace_in_a_field_description() {
// A `}` inside a field description must not truncate the type block (the AST
// parse is immune to the text-parse quirk the old design feared).
let sdl = "schema { query: QueryRoot mutation: MutationRoot }\n\
type QueryRoot { ping: Int! }\n\
type MutationRoot {\n\
\"\"\"a description with a } brace and { another\"\"\"\n\
createTask: Int!\n\
claimTask: Int!\n\
}\n";
let fields = mutation_field_names_from_sdl(sdl);
assert!(
fields.iter().any(|f| f == "createTask") && fields.iter().any(|f| f == "claimTask"),
"both fields must be extracted despite a brace in a description: {fields:?}"
);
}
#[test]
fn sdl_extractor_returns_empty_for_a_query_only_schema() {
let sdl = "schema { query: QueryRoot }\ntype QueryRoot { ping: Int! }\n";
assert!(
mutation_field_names_from_sdl(sdl).is_empty(),
"a schema with no mutation root yields no mutation fields (no panic)"
);
}
/// The comment-masked body of `async fn <snake>` in `graphql/mutation.rs`, bounded
/// to the resolver's OWN braces (its `{ .. }`), for the marker check. Brace-matching
/// on the comment-masked source means a `}` in a trailing helper cannot extend the
/// body and a comment marker does not count. A `{`/`}` inside a STRING literal is
/// not masked, a documented marker-check-only residual (limit 3); completeness does
/// not use this.
fn resolver_body(masked_src: &str, snake: &str) -> String {
let needle = format!("async fn {snake}(");
let start = masked_src.find(&needle).unwrap_or_else(|| {
panic!("resolver `async fn {snake}(` not found in graphql/mutation.rs")
});
let open = start
+ masked_src[start..]
.find('{')
.expect("resolver body opening brace");
let mut depth = 0i32;
let mut end = masked_src.len();
for (i, c) in masked_src[open..].char_indices() {
match c {
'{' => depth += 1,
'}' => {
depth -= 1;
if depth == 0 {
end = open + i + 1;
break;
}
}
_ => {}
}
}
masked_src[open..end].to_string()
}
/// `src` with `//` line comments and `/* */` block comments (nested) blanked to
/// spaces, WITHOUT opening a comment inside a `"..."` string (so a `"https://"` does
/// not erase the rest of its line). String CONTENTS are left intact, so a brace or
/// a gate marker inside a string is not masked; for the marker check that is a
/// documented vacuity residual (limit 3), and completeness does not use this at
/// all. Char literals and raw/byte strings are not tracked (absent in resolver
/// bodies); a `'"'` char or a raw string is the remaining narrow residual.
fn mask_comments(src: &str) -> String {
let chars: Vec<char> = src.chars().collect();
let mut out = String::with_capacity(src.len());
let mut i = 0;
let mut block_depth = 0usize;
let mut in_string = false;
while i < chars.len() {
let c = chars[i];
let next = chars.get(i + 1).copied();
if block_depth > 0 {
if c == '/' && next == Some('*') {
block_depth += 1;
out.push_str(" ");
i += 2;
} else if c == '*' && next == Some('/') {
block_depth -= 1;
out.push_str(" ");
i += 2;
} else {
out.push(if c == '\n' { '\n' } else { ' ' });
i += 1;
}
} else if in_string {
out.push(c);
if c == '\\' {
if let Some(n) = next {
out.push(n);
i += 2;
} else {
i += 1;
}
} else {
if c == '"' {
in_string = false;
}
i += 1;
}
} else if c == '"' {
in_string = true;
out.push('"');
i += 1;
} else if c == '/' && next == Some('/') {
while i < chars.len() && chars[i] != '\n' {
out.push(' ');
i += 1;
}
} else if c == '/' && next == Some('*') {
block_depth = 1;
out.push_str(" ");
i += 2;
} else {
out.push(c);
i += 1;
}
}
out
}
/// The SDL field set (the completeness source of truth, #219) is exactly the
/// registered mutations. If async-graphql adds or renames a field, this reddens
/// and forces the `every_graphql_mutation_has_its_gate` table to be updated. Uses
/// SDL, not introspection: introspection omits `visible = false` fields, so an
/// introspection-based pin here would go green while a hidden mutation existed.
#[test]
fn schema_lists_exactly_the_registered_mutations() {
let mut fields = graphql_mutation_fields_from_sdl();
fields.sort();
assert_eq!(
fields,
vec!["claimTask", "completeTask", "createTask", "failTask"],
"the SDL mutation field set is the authoritative discovered set"
);
}
/// The marker check is bounded to a resolver's OWN body and is comment- and
/// string-aware: a `//` inside a `"https://"` string does not erase a real gate on
/// that line (no false-red), and a trailing module helper's `did_matches(` does not
/// rescue a resolver whose own gate was removed (no false-green).
#[test]
fn resolver_body_is_bounded_and_comment_and_string_aware() {
let src = "\
impl MutationRoot {
async fn ok(&self) -> Result<u32> {
let _ = (\"see https://x/y\", did_matches(a, b)); // URL string then gate, one line
Ok(0)
}
async fn last(&self) -> Result<u32> {
// did_matches( only in a comment
Ok(0)
}
}
fn helper() { let _ = did_matches(z, z); }
#[cfg(test)]
mod tests { async fn t() { let _ = did_matches(x, y); } }
";
let masked = mask_comments(src);
assert!(
resolver_body(&masked, "ok").contains("did_matches("),
"a `//` inside a string must not erase the real gate on that line"
);
assert!(
!resolver_body(&masked, "last").contains("did_matches("),
"the marker check must be bounded to the resolver's own body (a trailing \
helper's marker must not rescue a resolver whose gate was removed)"
);
}
/// Proves the comment-stripping that GUARD-1 added: a marker that appears only
/// in a full-line comment (the real `replicas.rs` false-pass shape) must NOT
/// satisfy a row.
+1 -1
View File
@@ -41,7 +41,7 @@ Owner focus: protocol/auth.
- Close default-open repo write authorization and wire trusted UCAN delegation into repository permissions.
- Close the remaining visibility gaps: restrict or document task, pin, and anchor metadata routes, and publish an irreversible-publication policy.
- Add UCAN revocation or blocklisting, with an emergency compromised-key runbook.
- GraphQL mutations require a verified signer (shipped, #87). Keep the surface limited to the agent-task queue; do not expose an owner-gated repo-write mutation over GraphQL without its owner-gate and REST parity. The `every_graphql_mutation_has_its_gate` fence forces any new mutation to be classified in its table (discovered from the schema, so none can slip in unlisted); the owner-gate and REST parity themselves stay the INV-1 review check, not something the fence proves.
- Add mutation-aware GraphQL auth before GraphQL becomes a public write API surface.
- Harden peer registration and outbound peer calls against SSRF and peer-list poisoning.
- After all live peers upgrade, enable signed peer writes by default.
+1 -1
View File
@@ -92,7 +92,7 @@ Fixed or staged in this pass:
Live-network blockers to prioritize:
- GraphQL mutations now require a verified signer bound to the acting DID (#87), and the surface stays limited to the agent-task queue, so there is no public repo-write GraphQL API to gate. Queries remain open by design. (Resolved; kept here as posture, not a live blocker.)
- GraphQL POST is still open for compatibility; GraphQL mutations should get mutation-aware auth before it becomes a public write API surface.
- Push authorization is still not capability-complete. A valid DID signature is authentication, not authorization. Owner checks are now enforced on every branch, protected or not (`GITLAWB_ENFORCE_OWNER_PUSH`, on by default); what remains is that a UCAN `git/push` capability is not yet honored, so a delegated or CI key cannot push.
- UCAN chain validation is incomplete and UCAN revocation/blocklisting is not implemented as an operator feature.
- Private repository reads are not enforced. `is_public` and `GITLAWB_PUBLIC_READ` exist, but per-repository private-read behavior is not wired.