mirror of
https://github.com/Twigpine/node.git
synced 2026-11-09 05:20:05 -10:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
5338a51e12 | ||
|
|
fe155e1ea8 | ||
|
|
f2d172432e | ||
|
|
a7f32fbf01 | ||
|
|
5ed3381a6b | ||
|
|
9b915b09a7 | ||
|
|
77ac1037cb | ||
|
|
db98b3957a | ||
|
|
8c9e4adc2d | ||
|
|
8eedb935c3 | ||
|
|
47c6e6ce22 | ||
|
|
317a1550c2 | ||
|
|
30637193a6 | ||
|
|
0bae56f5ca | ||
|
|
a6d8459002 | ||
|
|
1b7639f0d7 | ||
|
|
5b9e06d501 | ||
|
|
92997b3695 | ||
|
|
ea34d84c3a | ||
|
|
5cc7e7a564 |
No files matched your search
@@ -504,6 +504,23 @@ mod tests {
|
||||
assert!(matches!(err, Error::Did(_)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn verify_rejects_oversized_method_id_before_decoding() {
|
||||
let sk = fresh();
|
||||
let mut att = dummy_attestation(&sk, sample_cert_hash());
|
||||
// A well-formed prefix on an absurdly long id must fail the length
|
||||
// bound, not the decoder: base58 decode cost is quadratic in input
|
||||
// length, so reaching it at all is the defect.
|
||||
att.signer = format!("did:key:z{}", "1".repeat(256));
|
||||
let err = att.verify_signature(sample_cert_hash()).unwrap_err();
|
||||
// Match the bound's own message: any other Did error (a decode or
|
||||
// length mismatch downstream) means the oversized input was decoded.
|
||||
match err {
|
||||
Error::Did(m) => assert!(m.contains("too long"), "unexpected: {m}"),
|
||||
e => panic!("expected Error::Did, got {e:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn verify_rejects_bad_signature_base64() {
|
||||
let sk = fresh();
|
||||
|
||||
@@ -150,23 +150,48 @@ impl Registry {
|
||||
}
|
||||
|
||||
/// Verify a batch, then enforce `RequireAll`.
|
||||
///
|
||||
/// Under [`Policy::RequireAll`] this never short-circuits: an attestation
|
||||
/// that fails to verify — bad signature, cert-hash mismatch, or malformed
|
||||
/// payload, the actual `Err` paths of [`Registry::verify`] — is dropped
|
||||
/// from the result rather than aborting the batch. A valid attestation of
|
||||
/// a type with NO registered verifier is not one of those: under
|
||||
/// `RequireAll` it verifies to `Ok` with `fully_verified: false` and stays
|
||||
/// in the result, preserving the distinction between *untrusted* and
|
||||
/// *invalid* (see `require_all_is_lenient_on_unknown_types_in_the_batch`).
|
||||
/// Any third party can attach an attestation, so a hard error here would
|
||||
/// let an unrelated `attacker/spam/v1` deny-of-service the whole cert (the
|
||||
/// DoS vector the module docstring warns about). The `required_types`
|
||||
/// check below is the real gate, and it only counts `fully_verified`
|
||||
/// entries — so a required type represented solely by a malformed
|
||||
/// attestation still fails with [`Error::RequiredMissing`].
|
||||
///
|
||||
/// Under `AcceptKnown` and `RejectUnknown` the batch stays strict: the
|
||||
/// first verification error is surfaced to the caller.
|
||||
pub fn verify_all(
|
||||
&self,
|
||||
attestations: &[Attestation],
|
||||
expected_cert_hash: [u8; 32],
|
||||
) -> Result<Vec<VerifiedAttestation>> {
|
||||
let mut verified = Vec::with_capacity(attestations.len());
|
||||
for a in attestations {
|
||||
verified.push(self.verify(a, expected_cert_hash)?);
|
||||
if self.policy != Policy::RequireAll {
|
||||
let mut verified = Vec::with_capacity(attestations.len());
|
||||
for a in attestations {
|
||||
verified.push(self.verify(a, expected_cert_hash)?);
|
||||
}
|
||||
return Ok(verified);
|
||||
}
|
||||
if self.policy == Policy::RequireAll {
|
||||
for required in &self.required_types {
|
||||
let present = verified
|
||||
.iter()
|
||||
.any(|v| &v.type_ == required && v.fully_verified);
|
||||
if !present {
|
||||
return Err(Error::RequiredMissing(required.clone()));
|
||||
}
|
||||
|
||||
// RequireAll: drop entries that fail to verify instead of aborting.
|
||||
let verified: Vec<VerifiedAttestation> = attestations
|
||||
.iter()
|
||||
.filter_map(|a| self.verify(a, expected_cert_hash).ok())
|
||||
.collect();
|
||||
for required in &self.required_types {
|
||||
let present = verified
|
||||
.iter()
|
||||
.any(|v| &v.type_ == required && v.fully_verified);
|
||||
if !present {
|
||||
return Err(Error::RequiredMissing(required.clone()));
|
||||
}
|
||||
}
|
||||
Ok(verified)
|
||||
@@ -324,6 +349,38 @@ mod tests {
|
||||
.any(|v| v.type_ == "other/v1" && !v.fully_verified));
|
||||
}
|
||||
|
||||
/// The strict policies must surface a batch member's verification error
|
||||
/// through `verify_all`, not launder it into a partial result. This binds
|
||||
/// the POLICY BOUNDARY itself: only the `RequireAll` arm may drop failing
|
||||
/// entries, and an apparently harmless cleanup that applies its
|
||||
/// `filter_map(Result::ok)` shape to the strict arms would silently turn
|
||||
/// the fail-closed contracts into fail-open — with every per-attestation
|
||||
/// `verify` test still green, because none of them go through the batch
|
||||
/// path. The failing entry is a wrong-cert-hash attestation (signed over a
|
||||
/// different hash than the batch expects), a genuine `Err` under every
|
||||
/// policy.
|
||||
#[test]
|
||||
fn strict_policies_propagate_a_batch_members_error() {
|
||||
let sk = fresh();
|
||||
let cert_hash = sample_hash();
|
||||
let mut wrong_hash = sample_hash();
|
||||
wrong_hash[0] ^= 0xff;
|
||||
|
||||
for policy in [Policy::AcceptKnown, Policy::RejectUnknown] {
|
||||
let mut reg = Registry::new().with_policy(policy);
|
||||
reg.register(DemoVerifier);
|
||||
let good = signed_demo(&sk, cert_hash, "ok");
|
||||
let bad = signed_demo(&sk, wrong_hash, "ok");
|
||||
let err = reg
|
||||
.verify_all(&[good, bad], cert_hash)
|
||||
.expect_err("a strict policy must fail the whole batch");
|
||||
assert!(
|
||||
matches!(err, Error::Signature(_) | Error::CertHashMismatch),
|
||||
"expected the underlying verification error, got {err:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A required type that is present but unverified (no verifier registered
|
||||
/// even though the type appears in `required_types`) must fail
|
||||
/// `verify_all` with `RequiredMissing`. Catches a misconfiguration where
|
||||
@@ -341,6 +398,65 @@ mod tests {
|
||||
assert!(matches!(err, Error::RequiredMissing(t) if t == "demo/v1"));
|
||||
}
|
||||
|
||||
/// A cert-hash that does not match `sample_hash()`, so an attestation
|
||||
/// signed against it fails `verify_signature` when the batch is checked
|
||||
/// against `sample_hash()`.
|
||||
fn other_hash() -> [u8; 32] {
|
||||
let mut h = sample_hash();
|
||||
h[0] ^= 0xff;
|
||||
h
|
||||
}
|
||||
|
||||
/// Under `RequireAll`, a malformed attestation (here: signed against the
|
||||
/// wrong cert hash, so it fails signature verification) attached alongside a
|
||||
/// valid required one must not abort the batch. The junk is dropped and the
|
||||
/// cert still verifies. Before the fix, `verify_all` propagated the error
|
||||
/// via `?` and the whole batch failed — the DoS vector the docstring warns
|
||||
/// about.
|
||||
#[test]
|
||||
fn require_all_drops_a_malformed_extra_and_still_accepts() {
|
||||
let sk = fresh();
|
||||
let cert_hash = sample_hash();
|
||||
let mut reg = Registry::new()
|
||||
.with_policy(Policy::RequireAll)
|
||||
.require_types(["demo/v1"]);
|
||||
reg.register(DemoVerifier);
|
||||
|
||||
// Signed against `other_hash()`, so it will not verify under `cert_hash`.
|
||||
let malformed = signed_demo(&sk, other_hash(), "boom");
|
||||
let real = signed_demo(&sk, cert_hash, "ok");
|
||||
|
||||
let verified = reg
|
||||
.verify_all(&[malformed, real], cert_hash)
|
||||
.expect("a malformed extra must not abort the batch under RequireAll");
|
||||
assert_eq!(verified.len(), 1, "the malformed entry should be dropped");
|
||||
assert!(verified
|
||||
.iter()
|
||||
.any(|v| v.type_ == "demo/v1" && v.fully_verified));
|
||||
}
|
||||
|
||||
/// Leniency must not open a hole: if the *only* attestation of a required
|
||||
/// type is malformed, dropping it leaves the required type absent and
|
||||
/// `verify_all` must still fail with `RequiredMissing`. Guards against a fix
|
||||
/// that skips errors so eagerly it lets an unverified required type pass.
|
||||
#[test]
|
||||
fn require_all_rejects_when_only_a_malformed_required_type_is_present() {
|
||||
let sk = fresh();
|
||||
let cert_hash = sample_hash();
|
||||
let mut reg = Registry::new()
|
||||
.with_policy(Policy::RequireAll)
|
||||
.require_types(["demo/v1"]);
|
||||
reg.register(DemoVerifier);
|
||||
|
||||
// The sole demo/v1 is bound to the wrong cert hash → fails verification.
|
||||
let malformed_required = signed_demo(&sk, other_hash(), "ok");
|
||||
|
||||
let err = reg
|
||||
.verify_all(&[malformed_required], cert_hash)
|
||||
.unwrap_err();
|
||||
assert!(matches!(err, Error::RequiredMissing(t) if t == "demo/v1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn payload_check_failure_rejects() {
|
||||
let sk = fresh();
|
||||
|
||||
@@ -285,11 +285,22 @@ pub async fn submit_bounty(
|
||||
Path(id): Path<String>,
|
||||
Json(req): Json<SubmitBountyRequest>,
|
||||
) -> Result<Json<BountyRecord>> {
|
||||
let bounty = state
|
||||
.db
|
||||
.get_bounty(&id)
|
||||
.await?
|
||||
.ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?;
|
||||
let not_found = || AppError::NotFound(format!("bounty {id} not found"));
|
||||
|
||||
let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?;
|
||||
|
||||
if crate::api::authorize_repo_read(
|
||||
&state,
|
||||
&bounty.repo_owner,
|
||||
&bounty.repo_name,
|
||||
Some(auth.0.as_str()),
|
||||
"/",
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
return Err(not_found());
|
||||
}
|
||||
|
||||
if bounty.status != "claimed" {
|
||||
return Err(AppError::BadRequest(format!(
|
||||
@@ -325,11 +336,22 @@ pub async fn approve_bounty(
|
||||
Path(id): Path<String>,
|
||||
Json(req): Json<ApproveBountyRequest>,
|
||||
) -> Result<Json<BountyRecord>> {
|
||||
let bounty = state
|
||||
.db
|
||||
.get_bounty(&id)
|
||||
.await?
|
||||
.ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?;
|
||||
let not_found = || AppError::NotFound(format!("bounty {id} not found"));
|
||||
|
||||
let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?;
|
||||
|
||||
if crate::api::authorize_repo_read(
|
||||
&state,
|
||||
&bounty.repo_owner,
|
||||
&bounty.repo_name,
|
||||
Some(auth.0.as_str()),
|
||||
"/",
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
return Err(not_found());
|
||||
}
|
||||
|
||||
if bounty.status != "submitted" {
|
||||
return Err(AppError::BadRequest(format!(
|
||||
@@ -372,11 +394,22 @@ pub async fn cancel_bounty(
|
||||
Extension(auth): Extension<AuthenticatedDid>,
|
||||
Path(id): Path<String>,
|
||||
) -> Result<Json<BountyRecord>> {
|
||||
let bounty = state
|
||||
.db
|
||||
.get_bounty(&id)
|
||||
.await?
|
||||
.ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?;
|
||||
let not_found = || AppError::NotFound(format!("bounty {id} not found"));
|
||||
|
||||
let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?;
|
||||
|
||||
if crate::api::authorize_repo_read(
|
||||
&state,
|
||||
&bounty.repo_owner,
|
||||
&bounty.repo_name,
|
||||
Some(auth.0.as_str()),
|
||||
"/",
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
return Err(not_found());
|
||||
}
|
||||
|
||||
if bounty.status != "open" {
|
||||
return Err(AppError::BadRequest(format!(
|
||||
@@ -407,11 +440,22 @@ pub async fn dispute_bounty(
|
||||
Extension(auth): Extension<AuthenticatedDid>,
|
||||
Path(id): Path<String>,
|
||||
) -> Result<Json<BountyRecord>> {
|
||||
let bounty = state
|
||||
.db
|
||||
.get_bounty(&id)
|
||||
.await?
|
||||
.ok_or_else(|| AppError::NotFound(format!("bounty {id} not found")))?;
|
||||
let not_found = || AppError::NotFound(format!("bounty {id} not found"));
|
||||
|
||||
let bounty = state.db.get_bounty(&id).await?.ok_or_else(not_found)?;
|
||||
|
||||
if crate::api::authorize_repo_read(
|
||||
&state,
|
||||
&bounty.repo_owner,
|
||||
&bounty.repo_name,
|
||||
Some(auth.0.as_str()),
|
||||
"/",
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
return Err(not_found());
|
||||
}
|
||||
|
||||
if bounty.status != "claimed" && bounty.status != "submitted" {
|
||||
return Err(AppError::BadRequest(format!(
|
||||
@@ -456,21 +500,68 @@ pub async fn dispute_bounty(
|
||||
Ok(Json(updated))
|
||||
}
|
||||
|
||||
/// Resolve the set of `(owner_did, name)` pairs for repos an anonymous caller
|
||||
/// can list. Mirrors the `stats()` pattern in `server.rs` (#104): batch-load
|
||||
/// all deduped repos, batch-load their visibility rules, keep only those that
|
||||
/// pass `listable_at_root`. Pure I/O after the two DB round-trips — no
|
||||
/// per-repo authorization queries.
|
||||
///
|
||||
/// Both bounty stats handlers mount behind `optional_signature` (see
|
||||
/// `bounty_read_routes` in server.rs), so the caller is always `None` for
|
||||
/// anonymous access. Every caller, signed or not, currently receives the
|
||||
/// anonymous-scoped aggregates (matching the #104 pattern); per-caller
|
||||
/// visibility can be threaded through here when that work lands.
|
||||
///
|
||||
/// A database error is returned to the caller, never collapsed to an empty
|
||||
/// set: an empty set would render as HTTP 200 with zero counts, which a client
|
||||
/// reads as confirmed absence of activity.
|
||||
async fn visible_repo_pairs(state: &AppState) -> Result<Vec<(String, String)>> {
|
||||
let rows = state.db.list_all_repos_deduped().await?;
|
||||
let ids: Vec<String> = rows.iter().map(|r| r.id.clone()).collect();
|
||||
let rules_by_repo = state.db.list_visibility_rules_for_repos(&ids).await?;
|
||||
let pairs = rows
|
||||
.iter()
|
||||
.filter(|r| {
|
||||
let rules = rules_by_repo.get(&r.id).map(Vec::as_slice).unwrap_or(&[]);
|
||||
// mirror-rows-handled: a mirror with a canonical twin is deduped to the
|
||||
// canonical row and its rules; a mirror-only repo counts as public, the
|
||||
// same answer authorize_repo_read and /stats give. Pinned by
|
||||
// bounty_stats_mirror_rows_follow_canonical_twin_or_count_when_alone.
|
||||
crate::visibility::listable_at_root(rules, r.is_public, &r.owner_did, None)
|
||||
})
|
||||
.map(|r| {
|
||||
(
|
||||
crate::db::normalize_owner_key(&r.owner_did).to_string(),
|
||||
r.name.clone(),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
Ok(pairs)
|
||||
}
|
||||
|
||||
/// GET /api/v1/bounties/stats
|
||||
///
|
||||
/// Aggregates are restricted to anonymously-listable repos so private-repo
|
||||
/// bounty activity is not exposed (#477). The visible-repo set is resolved
|
||||
/// once per request via `visible_repo_pairs` (two SQL round-trips), then
|
||||
/// passed into the filtered aggregate queries.
|
||||
pub async fn bounty_stats(State(state): State<AppState>) -> Result<Json<BountyStatsResponse>> {
|
||||
let open = state.db.count_bounties_by_status("open").await.unwrap_or(0);
|
||||
let visible = visible_repo_pairs(&state).await?;
|
||||
|
||||
let open = state
|
||||
.db
|
||||
.count_bounties_by_status_visible("open", &visible)
|
||||
.await?;
|
||||
let claimed = state
|
||||
.db
|
||||
.count_bounties_by_status("claimed")
|
||||
.await
|
||||
.unwrap_or(0);
|
||||
.count_bounties_by_status_visible("claimed", &visible)
|
||||
.await?;
|
||||
let completed = state
|
||||
.db
|
||||
.count_bounties_by_status("completed")
|
||||
.await
|
||||
.unwrap_or(0);
|
||||
.count_bounties_by_status_visible("completed", &visible)
|
||||
.await?;
|
||||
|
||||
let leaders = state.db.bounty_leaderboard(10).await.unwrap_or_default();
|
||||
let leaders = state.db.bounty_leaderboard_visible(10, &visible).await?;
|
||||
let leaderboard = leaders
|
||||
.into_iter()
|
||||
.map(|(did, cnt, total)| AgentBountyEntry {
|
||||
@@ -489,11 +580,14 @@ pub async fn bounty_stats(State(state): State<AppState>) -> Result<Json<BountySt
|
||||
}
|
||||
|
||||
/// GET /api/v1/agents/{did}/bounties
|
||||
///
|
||||
/// Per-agent earnings restricted to anonymously-listable repos (#477).
|
||||
pub async fn agent_bounty_stats(
|
||||
State(state): State<AppState>,
|
||||
Path(did): Path<String>,
|
||||
) -> Result<Json<serde_json::Value>> {
|
||||
let (count, total) = state.db.agent_bounty_stats(&did).await.unwrap_or((0, 0));
|
||||
let visible = visible_repo_pairs(&state).await?;
|
||||
let (count, total) = state.db.agent_bounty_stats_visible(&did, &visible).await?;
|
||||
Ok(Json(serde_json::json!({
|
||||
"did": did,
|
||||
"completed_bounties": count,
|
||||
|
||||
@@ -14,14 +14,13 @@ pub struct LabelRequest {
|
||||
pub label: String,
|
||||
}
|
||||
|
||||
/// POST /api/v1/repos/:owner/:repo/labels
|
||||
pub async fn add_label(
|
||||
State(state): State<AppState>,
|
||||
Extension(auth): Extension<AuthenticatedDid>,
|
||||
Path((owner, name)): Path<(String, String)>,
|
||||
Json(req): Json<LabelRequest>,
|
||||
) -> Result<(StatusCode, Json<serde_json::Value>)> {
|
||||
let label = req.label.trim().to_lowercase();
|
||||
/// Canonicalize and validate a label string. `add_label` and `remove_label`
|
||||
/// must agree on the form stored in the DB: `add_label` already trims and
|
||||
/// lowercases before storing, so `remove_label` has to do the same to find
|
||||
/// the row. Validation is shared so neither handler can drift on charset or
|
||||
/// length (#344).
|
||||
fn canonicalize_label(raw: &str) -> std::result::Result<String, AppError> {
|
||||
let label = raw.trim().to_lowercase();
|
||||
if label.is_empty() || label.len() > 50 {
|
||||
return Err(AppError::BadRequest("label must be 1–50 characters".into()));
|
||||
}
|
||||
@@ -33,6 +32,17 @@ pub async fn add_label(
|
||||
"label must contain only alphanumeric characters, hyphens, and colons".into(),
|
||||
));
|
||||
}
|
||||
Ok(label)
|
||||
}
|
||||
|
||||
/// POST /api/v1/repos/:owner/:repo/labels
|
||||
pub async fn add_label(
|
||||
State(state): State<AppState>,
|
||||
Extension(auth): Extension<AuthenticatedDid>,
|
||||
Path((owner, name)): Path<(String, String)>,
|
||||
Json(req): Json<LabelRequest>,
|
||||
) -> Result<(StatusCode, Json<serde_json::Value>)> {
|
||||
let label = canonicalize_label(&req.label)?;
|
||||
|
||||
let record = state
|
||||
.db
|
||||
@@ -59,6 +69,8 @@ pub async fn remove_label(
|
||||
Extension(auth): Extension<AuthenticatedDid>,
|
||||
Path((owner, name, label)): Path<(String, String, String)>,
|
||||
) -> Result<Json<serde_json::Value>> {
|
||||
let label = canonicalize_label(&label)?;
|
||||
|
||||
let record = state
|
||||
.db
|
||||
.get_repo(&owner, &name)
|
||||
@@ -66,7 +78,10 @@ pub async fn remove_label(
|
||||
.ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?;
|
||||
crate::api::require_repo_owner(&record, &auth.0)?;
|
||||
|
||||
state.db.remove_label(&record.id, &label).await?;
|
||||
let removed = state.db.remove_label(&record.id, &label).await?;
|
||||
if !removed {
|
||||
return Err(AppError::NotFound(format!("label '{label}'")));
|
||||
}
|
||||
Ok(Json(serde_json::json!({ "label": label, "removed": true })))
|
||||
}
|
||||
|
||||
|
||||
@@ -70,6 +70,8 @@ pub struct PeerResponse {
|
||||
pub reachable: bool,
|
||||
}
|
||||
|
||||
pub(crate) const PUBLIC_HTTP_URL_REQUIREMENT: &str = "must be a public http(s) URL (no loopback, private, localhost, .localhost, .internal, or .local hosts)";
|
||||
|
||||
/// Extract an IPv4 address embedded in an IPv6 literal across the transition
|
||||
/// formats that carry one: IPv4-mapped (`::ffff:a.b.c.d`), IPv4-compatible
|
||||
/// (`::a.b.c.d`), 6to4 (`2002:WWXX:YYZZ::/16`), and the NAT64 well-known prefix
|
||||
@@ -104,8 +106,8 @@ fn embedded_ipv4(v6: std::net::Ipv6Addr) -> Option<std::net::Ipv4Addr> {
|
||||
|
||||
/// Whether a peer `http_url` is a public http(s) endpoint safe to register.
|
||||
/// Rejects non-http(s) schemes, loopback/unspecified/private/link-local IPs,
|
||||
/// and `localhost` / `.local` / `.internal` hostnames. Used at announce time
|
||||
/// and by the boot-time prune of already-poisoned rows.
|
||||
/// and `localhost` / `.localhost` / `.local` / `.internal` hostnames. Used at
|
||||
/// announce time and by the boot-time prune of already-poisoned rows.
|
||||
pub fn is_public_http_url(raw: &str) -> bool {
|
||||
let url = match reqwest::Url::parse(raw) {
|
||||
Ok(u) => u,
|
||||
@@ -125,6 +127,7 @@ pub fn is_public_http_url(raw: &str) -> bool {
|
||||
host.truncate(host.trim_end_matches('.').len());
|
||||
if host.is_empty()
|
||||
|| host == "localhost"
|
||||
|| host.ends_with(".localhost")
|
||||
|| host.ends_with(".local")
|
||||
|| host.ends_with(".internal")
|
||||
{
|
||||
@@ -260,9 +263,9 @@ pub async fn announce(
|
||||
// and turn our outbound sync-notify fan-out into an SSRF probe — and bury
|
||||
// the real peers under junk so node-origin repos stop replicating.
|
||||
if !is_public_http_url(&req.http_url) {
|
||||
return Err(AppError::BadRequest(
|
||||
"http_url must be a public http(s) URL (no loopback, private, or .internal/.local hosts)".into(),
|
||||
));
|
||||
return Err(AppError::BadRequest(format!(
|
||||
"http_url {PUBLIC_HTTP_URL_REQUIREMENT}"
|
||||
)));
|
||||
}
|
||||
|
||||
// Reject self-announcements: a peer row whose http_url is our own public
|
||||
@@ -541,6 +544,7 @@ mod tests {
|
||||
fn rejects_loopback_private_and_internal() {
|
||||
for bad in [
|
||||
"http://localhost:7545",
|
||||
"http://node.localhost:7545",
|
||||
"http://127.0.0.1:5432/",
|
||||
"http://localhost:22/",
|
||||
"http://0.0.0.0:7545",
|
||||
@@ -620,11 +624,14 @@ mod tests {
|
||||
#[test]
|
||||
fn rejects_repeated_trailing_root_dots() {
|
||||
// Stripping a single root dot leaves `localhost.`, which matches
|
||||
// neither the equality nor the suffix check. The subdomain form
|
||||
// (`node.localhost..`) is not asserted here: `.localhost` is not yet a
|
||||
// rejected suffix on this base, so that case belongs to whichever
|
||||
// change adds it rather than to this one.
|
||||
for bad in ["http://localhost../", "http://localhost...:7545"] {
|
||||
// neither the equality nor the suffix check. The subdomain form now
|
||||
// exercises the `.localhost` suffix rejection together with the extra
|
||||
// trailing dot.
|
||||
for bad in [
|
||||
"http://localhost../",
|
||||
"http://localhost...:7545",
|
||||
"http://node.localhost../",
|
||||
] {
|
||||
assert!(!is_public_http_url(bad), "{bad:?} must be rejected");
|
||||
}
|
||||
}
|
||||
@@ -1582,6 +1589,31 @@ mod tests {
|
||||
)
|
||||
}
|
||||
|
||||
#[sqlx::test]
|
||||
async fn announce_rejects_localhost_subdomains_with_an_accurate_error(pool: PgPool) {
|
||||
let state = test_state(pool).await;
|
||||
let did = Keypair::generate().did().to_string();
|
||||
let resp = announce_only(state.clone())
|
||||
.oneshot(announce_as(
|
||||
&did,
|
||||
&announce_body(&did, "https://node.localhost:7545"),
|
||||
))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let (status, error, message) = status_and_error(resp).await;
|
||||
assert_eq!(status, StatusCode::BAD_REQUEST);
|
||||
assert_eq!(error, "bad_request");
|
||||
assert_eq!(
|
||||
message,
|
||||
"http_url must be a public http(s) URL (no loopback, private, localhost, .localhost, .internal, or .local hosts)"
|
||||
);
|
||||
assert!(
|
||||
snapshot(&state.db, &did).await.is_none(),
|
||||
"a rejected .localhost announce must leave no row behind"
|
||||
);
|
||||
}
|
||||
|
||||
/// U4 scenario 1, and the first test the keyid branch has ever had: a caller
|
||||
/// who proved control of one DID must not announce another's. Kills
|
||||
/// neutralizing the handler's keyid comparison, and kills demoting this
|
||||
|
||||
@@ -46,9 +46,10 @@ pub async fn create_webhook(
|
||||
// endpoints (SSRF). Delivery runs on the shared no-redirect client
|
||||
// (main.rs), which closes the 3xx-to-internal bounce.
|
||||
if !crate::api::peers::is_public_http_url(&req.url) {
|
||||
return Err(AppError::BadRequest(
|
||||
"webhook URL must be a public http(s) URL (no loopback, private, or .internal/.local hosts)".into(),
|
||||
));
|
||||
return Err(AppError::BadRequest(format!(
|
||||
"webhook URL {}",
|
||||
crate::api::peers::PUBLIC_HTTP_URL_REQUIREMENT
|
||||
)));
|
||||
}
|
||||
|
||||
let events = req.events.unwrap_or_else(|| vec!["*".into()]);
|
||||
@@ -141,7 +142,19 @@ pub async fn delete_webhook(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::api::peers::is_public_http_url;
|
||||
use axum::extract::{Extension, Path, State};
|
||||
use axum::Json;
|
||||
use chrono::Utc;
|
||||
use gitlawb_core::identity::Keypair;
|
||||
use sqlx::PgPool;
|
||||
use uuid::Uuid;
|
||||
|
||||
use super::{create_webhook, CreateWebhookRequest};
|
||||
use crate::api::peers::{is_public_http_url, PUBLIC_HTTP_URL_REQUIREMENT};
|
||||
use crate::auth::AuthenticatedDid;
|
||||
use crate::db::RepoRecord;
|
||||
use crate::error::AppError;
|
||||
use crate::test_support::test_state;
|
||||
|
||||
// create_webhook gates req.url through is_public_http_url. Pin the exact
|
||||
// SSRF targets from issue #81 so the webhook path can never regress to the
|
||||
@@ -152,6 +165,7 @@ mod tests {
|
||||
"http://127.0.0.1:5432/",
|
||||
"http://169.254.169.254/latest/meta-data/",
|
||||
"http://localhost/",
|
||||
"http://node.localhost/",
|
||||
"http://10.0.0.5/",
|
||||
"http://[::1]/",
|
||||
// IPv6 transition encodings smuggling loopback v4 (6to4 / NAT64).
|
||||
@@ -169,4 +183,53 @@ mod tests {
|
||||
assert!(is_public_http_url("https://hooks.example.com/gitlawb"));
|
||||
assert!(is_public_http_url("http://203.0.113.10:7545/"));
|
||||
}
|
||||
|
||||
#[sqlx::test]
|
||||
async fn webhook_localhost_rejection_uses_shared_public_url_contract(pool: PgPool) {
|
||||
let state = test_state(pool).await;
|
||||
let owner = Keypair::generate().did().to_string();
|
||||
let repo_id = Uuid::new_v4().to_string();
|
||||
state
|
||||
.db
|
||||
.create_repo(&RepoRecord {
|
||||
id: repo_id.clone(),
|
||||
name: "webhook-contract".to_string(),
|
||||
owner_did: owner.clone(),
|
||||
description: None,
|
||||
is_public: true,
|
||||
default_branch: "main".to_string(),
|
||||
created_at: Utc::now(),
|
||||
updated_at: Utc::now(),
|
||||
disk_path: "/tmp/webhook-contract.git".to_string(),
|
||||
forked_from: None,
|
||||
machine_id: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let result = create_webhook(
|
||||
State(state.clone()),
|
||||
Extension(AuthenticatedDid(owner.clone())),
|
||||
Path((owner, "webhook-contract".to_string())),
|
||||
Json(CreateWebhookRequest {
|
||||
url: "https://node.localhost/hook".to_string(),
|
||||
secret: None,
|
||||
events: None,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
match result {
|
||||
Err(AppError::BadRequest(message)) => assert_eq!(
|
||||
message,
|
||||
format!("webhook URL {PUBLIC_HTTP_URL_REQUIREMENT}")
|
||||
),
|
||||
Err(error) => panic!("unexpected webhook rejection: {error}"),
|
||||
Ok(_) => panic!("a .localhost webhook must be rejected"),
|
||||
}
|
||||
assert!(
|
||||
state.db.list_webhooks(&repo_id).await.unwrap().is_empty(),
|
||||
"a rejected .localhost webhook must leave no row behind"
|
||||
);
|
||||
}
|
||||
}
|
||||
File diff suppressed because it is too large.
Load diff
@@ -141,19 +141,10 @@ pub fn resolve_issue_id(repo_path: &Path, id_or_prefix: &str) -> Result<Option<S
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
// Try exact match first — fast path for callers passing the full UUID.
|
||||
// List all refs that start with the given string; the glob also matches the exact
|
||||
// ref. The id handed back is always git's own refname, never the request input.
|
||||
let exact_ref = format!("refs/gitlawb/issues/{id_or_prefix}");
|
||||
let check = Command::new("git")
|
||||
.args(["cat-file", "-e", &exact_ref])
|
||||
.current_dir(repo_path)
|
||||
.output()
|
||||
.context("failed to run git cat-file -e")?;
|
||||
if check.status.success() {
|
||||
return Ok(Some(id_or_prefix.to_string()));
|
||||
}
|
||||
|
||||
// Prefix search: list all refs that start with the given string.
|
||||
let prefix_glob = format!("refs/gitlawb/issues/{id_or_prefix}*");
|
||||
let prefix_glob = format!("{exact_ref}*");
|
||||
let list = Command::new("git")
|
||||
.args(["for-each-ref", "--format=%(refname)", &prefix_glob])
|
||||
.current_dir(repo_path)
|
||||
@@ -165,25 +156,44 @@ pub fn resolve_issue_id(repo_path: &Path, id_or_prefix: &str) -> Result<Option<S
|
||||
}
|
||||
|
||||
let output = String::from_utf8_lossy(&list.stdout);
|
||||
let matches: Vec<&str> = output.lines().filter(|l| !l.trim().is_empty()).collect();
|
||||
let matches: Vec<&str> = output
|
||||
.lines()
|
||||
.map(str::trim)
|
||||
.filter(|l| !l.is_empty())
|
||||
.collect();
|
||||
|
||||
match matches.len() {
|
||||
0 => Ok(None),
|
||||
1 => {
|
||||
// Strip the "refs/gitlawb/issues/" prefix to get the bare ID.
|
||||
let full_id = matches[0]
|
||||
.trim()
|
||||
.strip_prefix("refs/gitlawb/issues/")
|
||||
.unwrap_or(matches[0].trim())
|
||||
.to_string();
|
||||
Ok(Some(full_id))
|
||||
// An exact id wins over longer ids that share it as a prefix, but only if its
|
||||
// object exists: for-each-ref also lists a ref whose blob is missing, so a
|
||||
// dangling exact ref falls through to the prefix rules below.
|
||||
let exact = match matches.iter().find(|m| **m == exact_ref) {
|
||||
Some(m) => {
|
||||
let check = Command::new("git")
|
||||
.args(["cat-file", "-e", m])
|
||||
.current_dir(repo_path)
|
||||
.output()
|
||||
.context("failed to run git cat-file -e")?;
|
||||
check.status.success().then_some(*m)
|
||||
}
|
||||
_ => anyhow::bail!(
|
||||
"ambiguous issue prefix '{}': matches {} issues",
|
||||
id_or_prefix,
|
||||
matches.len()
|
||||
),
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
let found = match exact {
|
||||
Some(m) => m,
|
||||
None => match matches.as_slice() {
|
||||
[] => return Ok(None),
|
||||
[m] => *m,
|
||||
_ => anyhow::bail!(
|
||||
"ambiguous issue prefix '{}': matches {} issues",
|
||||
id_or_prefix,
|
||||
matches.len()
|
||||
),
|
||||
},
|
||||
};
|
||||
// Strip the "refs/gitlawb/issues/" prefix to get the bare ID.
|
||||
let full_id = found
|
||||
.strip_prefix("refs/gitlawb/issues/")
|
||||
.unwrap_or(found)
|
||||
.to_string();
|
||||
Ok(Some(full_id))
|
||||
}
|
||||
|
||||
/// Close an issue by updating its status to "closed" in the git ref.
|
||||
@@ -267,6 +277,43 @@ mod tests {
|
||||
assert_eq!(resolved, Some(full_id.to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_exact_id_wins_over_longer_id() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
init_repo(&dir);
|
||||
create_issue(dir.path(), "abc12345", r#"{"status":"open"}"#).unwrap();
|
||||
create_issue(dir.path(), "abc12345-0000", r#"{"status":"open"}"#).unwrap();
|
||||
let resolved = resolve_issue_id(dir.path(), "abc12345").unwrap();
|
||||
assert_eq!(resolved, Some("abc12345".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_dangling_exact_id_does_not_win_over_longer_id() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
init_repo(&dir);
|
||||
create_issue(dir.path(), "abc12345", r#"{"status":"open","n":1}"#).unwrap();
|
||||
create_issue(dir.path(), "abc12345-0000", r#"{"status":"open","n":2}"#).unwrap();
|
||||
// Dangle the exact ref: the ref stays listed by for-each-ref, but its blob is gone.
|
||||
let out = Command::new("git")
|
||||
.args(["rev-parse", "refs/gitlawb/issues/abc12345"])
|
||||
.current_dir(dir.path())
|
||||
.output()
|
||||
.unwrap();
|
||||
let hash = String::from_utf8(out.stdout).unwrap();
|
||||
let hash = hash.trim();
|
||||
std::fs::remove_file(
|
||||
dir.path()
|
||||
.join(".git/objects")
|
||||
.join(&hash[..2])
|
||||
.join(&hash[2..]),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let result = resolve_issue_id(dir.path(), "abc12345");
|
||||
assert!(result.unwrap_err().to_string().contains("ambiguous"));
|
||||
assert!(close_issue(dir.path(), "abc12345").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_prefix_matches_unique() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
|
||||
@@ -314,7 +314,9 @@ pub fn record_sync_processed(status: &str) {
|
||||
}
|
||||
|
||||
/// Record one webhook delivery attempt outcome. `result` ∈
|
||||
/// {ok, http_error, network_error, skipped}.
|
||||
/// {ok, http_error, network_error, skipped, invalid_url}.
|
||||
/// `invalid_url` is a policy skip: the stored hook URL failed the public-URL
|
||||
/// check at delivery time and the POST was never sent.
|
||||
pub fn record_webhook_delivery(result: &str) {
|
||||
if let Some(c) = WEBHOOK_DELIVERIES.get() {
|
||||
c.with_label_values(&[result]).inc();
|
||||
|
||||
File diff suppressed because it is too large.
Load diff
@@ -79,6 +79,19 @@ async fn fire_event_async(
|
||||
let delivery_id = uuid::Uuid::new_v4().to_string();
|
||||
|
||||
tokio::spawn(async move {
|
||||
// Policy skip, not a delivery failure: the stored URL predates the
|
||||
// create-time gate. The "invalid_url" label is documented on
|
||||
// record_webhook_delivery in metrics.rs.
|
||||
if !crate::api::peers::is_public_http_url(&hook.url) {
|
||||
crate::metrics::record_webhook_delivery("invalid_url");
|
||||
tracing::warn!(
|
||||
url = %hook.url,
|
||||
event = %event_name,
|
||||
"webhook delivery skipped: stored URL is not a public http(s) target"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let signature = hook.secret.as_deref().map(|s| sign_payload(s, &bytes));
|
||||
|
||||
let mut req = client
|
||||
|
||||
@@ -88,7 +88,7 @@ async fn cmd_list(node: String, capability: Option<String>) -> Result<()> {
|
||||
let short = did
|
||||
.split(':')
|
||||
.next_back()
|
||||
.map(|s| &s[..s.len().min(16)])
|
||||
.map(|s| crate::text::truncate(s, 16))
|
||||
.unwrap_or("?");
|
||||
let trust = agent["trust_score"].as_f64().unwrap_or(0.0);
|
||||
let caps = agent["capabilities"]
|
||||
|
||||
@@ -275,7 +275,7 @@ async fn cmd_list(
|
||||
let title = b["title"].as_str().unwrap_or("?");
|
||||
let amount = b["amount"].as_i64().unwrap_or(0);
|
||||
let st = b["status"].as_str().unwrap_or("?");
|
||||
let short_id = &id[..8.min(id.len())];
|
||||
let short_id = crate::text::truncate(id, 8);
|
||||
println!("{short_id} {st:<10} {amount:>12} $GITLAWB {title}");
|
||||
}
|
||||
}
|
||||
@@ -403,7 +403,7 @@ async fn cmd_stats(node: String) -> Result<()> {
|
||||
let did = entry["did"].as_str().unwrap_or("?");
|
||||
let count = entry["completed"].as_i64().unwrap_or(0);
|
||||
let earned = entry["total_earned"].as_i64().unwrap_or(0);
|
||||
let short = &did[did.len().saturating_sub(8)..];
|
||||
let short = crate::text::truncate_suffix(did, 8);
|
||||
println!(
|
||||
" {}. ...{short} {count} bounties {earned} $GITLAWB",
|
||||
i + 1
|
||||
|
||||
@@ -117,7 +117,10 @@ async fn cmd_list(repo: String, node: String, dir: Option<PathBuf>) -> Result<()
|
||||
let id = cert["id"].as_str().unwrap_or("?");
|
||||
let ref_name = cert["ref_name"].as_str().unwrap_or("?");
|
||||
let new_sha = cert["new_sha"].as_str().unwrap_or("?");
|
||||
let issued_at = cert["issued_at"].as_str().map(|s| &s[..19]).unwrap_or("?");
|
||||
let issued_at = cert["issued_at"]
|
||||
.as_str()
|
||||
.map(|s| crate::text::truncate(s, 19))
|
||||
.unwrap_or("?");
|
||||
println!(" {id:.8} {issued_at} {ref_name} {new_sha:.12}");
|
||||
}
|
||||
Ok(())
|
||||
|
||||
@@ -75,14 +75,14 @@ pub async fn run(args: ChangelogArgs) -> Result<()> {
|
||||
|
||||
for event in &events {
|
||||
let ts = event["timestamp"].as_str().unwrap_or("?");
|
||||
let date = &ts[..ts.len().min(10)];
|
||||
let date = crate::text::truncate(ts, 10);
|
||||
|
||||
match event["type"].as_str().unwrap_or("") {
|
||||
"commit" => {
|
||||
let sha = event["sha"].as_str().unwrap_or("?");
|
||||
let msg = event["message"].as_str().unwrap_or("?");
|
||||
let first_line = msg.lines().next().unwrap_or(msg);
|
||||
let short_sha = &sha[..sha.len().min(8)];
|
||||
let short_sha = crate::text::truncate(sha, 8);
|
||||
println!(" {date} commit {short_sha} {first_line}");
|
||||
}
|
||||
"pr_merged" => {
|
||||
|
||||
@@ -131,11 +131,7 @@ async fn cmd_list(node: String, dir: Option<PathBuf>) -> Result<()> {
|
||||
let sha = pin["sha256_hex"].as_str().unwrap_or("?");
|
||||
let pinned_at = pin["pinned_at"].as_str().unwrap_or("?");
|
||||
// Trim pinned_at to date+time without subseconds
|
||||
let ts = if pinned_at.len() >= 19 {
|
||||
&pinned_at[..19]
|
||||
} else {
|
||||
pinned_at
|
||||
};
|
||||
let ts = crate::text::truncate(pinned_at, 19);
|
||||
println!(" {cid}");
|
||||
println!(" sha256: {sha}");
|
||||
println!(" pinned: {ts}");
|
||||
|
||||
+34
-3
@@ -251,7 +251,7 @@ async fn cmd_list(repo: String, node: String, dir: Option<PathBuf>) -> Result<()
|
||||
let status = issue["status"].as_str().unwrap_or("?");
|
||||
let created = issue["created_at"]
|
||||
.as_str()
|
||||
.map(|s| &s[..10])
|
||||
.map(|s| crate::text::truncate(s, 10))
|
||||
.unwrap_or("?");
|
||||
let icon = match status {
|
||||
"open" => "○",
|
||||
@@ -365,10 +365,13 @@ async fn cmd_issue_comments(
|
||||
let author_short = author
|
||||
.split(':')
|
||||
.next_back()
|
||||
.map(|s| &s[..s.len().min(8)])
|
||||
.map(|s| crate::text::truncate(s, 8))
|
||||
.unwrap_or("?");
|
||||
let cbody = c["body"].as_str().unwrap_or("");
|
||||
let created = c["created_at"].as_str().map(|s| &s[..10]).unwrap_or("?");
|
||||
let created = c["created_at"]
|
||||
.as_str()
|
||||
.map(|s| crate::text::truncate(s, 10))
|
||||
.unwrap_or("?");
|
||||
println!(" · {author_short} ({created})");
|
||||
println!(" {cbody}");
|
||||
println!();
|
||||
@@ -439,6 +442,34 @@ mod tests {
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_cmd_list_survives_malformed_timestamps() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
write_identity(&dir);
|
||||
|
||||
let mut server = mockito::Server::new_async().await;
|
||||
// A short timestamp used to panic on `&s[..10]`; a timestamp whose
|
||||
// byte 10 is mid-char panics even with a len() guard.
|
||||
let _m = server
|
||||
.mock(
|
||||
"GET",
|
||||
mockito::Matcher::Regex(r"^/api/v1/repos/[^/]+/myrepo/issues$".to_string()),
|
||||
)
|
||||
.with_status(200)
|
||||
.with_header("content-type", "application/json")
|
||||
.with_body(r#"{"issues":[{"id":"a","title":"short ts","status":"open","created_at":"2026"},{"id":"b","title":"mb ts","status":"open","created_at":"2026-08-1é5T00:00:00Z"},{"id":"c","title":"empty ts","status":"open","created_at":""}]}"#)
|
||||
.create_async()
|
||||
.await;
|
||||
|
||||
cmd_list(
|
||||
"myrepo".to_string(),
|
||||
server.url(),
|
||||
Some(dir.path().to_path_buf()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_cmd_create_success() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
|
||||
@@ -30,6 +30,7 @@ mod star;
|
||||
mod status;
|
||||
mod sync;
|
||||
mod task;
|
||||
mod text;
|
||||
mod ucan_cmd;
|
||||
mod visibility;
|
||||
mod webhook;
|
||||
|
||||
@@ -413,7 +413,7 @@ async fn cmd_status(node: String, dir: Option<PathBuf>) -> Result<()> {
|
||||
let ref_name = ev["ref"].as_str().unwrap_or("?");
|
||||
let ts = ev["timestamp"]
|
||||
.as_str()
|
||||
.map(|s| &s[..10.min(s.len())])
|
||||
.map(|s| crate::text::truncate(s, 10))
|
||||
.unwrap_or("?");
|
||||
println!(" {ts} {repo} {ref_name}");
|
||||
}
|
||||
|
||||
@@ -97,7 +97,7 @@ async fn cmd_list(node: String) -> Result<()> {
|
||||
let reachable = peer["reachable"].as_bool().unwrap_or(false);
|
||||
let last_seen = peer["last_seen"]
|
||||
.as_str()
|
||||
.map(|s| &s[..10])
|
||||
.map(|s| crate::text::truncate(s, 10))
|
||||
.unwrap_or("never");
|
||||
let status = if reachable { "✓" } else { "✗" };
|
||||
println!(" {status} {url}");
|
||||
|
||||
+13
-7
@@ -304,7 +304,7 @@ async fn cmd_list(repo: String, node: String, dir: Option<PathBuf>) -> Result<()
|
||||
let author_short = author
|
||||
.split(':')
|
||||
.next_back()
|
||||
.map(|s| &s[..s.len().min(8)])
|
||||
.map(|s| crate::text::truncate(s, 8))
|
||||
.unwrap_or("?");
|
||||
let status_icon = match status {
|
||||
"open" => "○",
|
||||
@@ -374,7 +374,7 @@ async fn cmd_view(repo: String, number: u64, node: String, dir: Option<PathBuf>)
|
||||
let reviewer_short = reviewer
|
||||
.split(':')
|
||||
.next_back()
|
||||
.map(|s| &s[..s.len().min(8)])
|
||||
.map(|s| crate::text::truncate(s, 8))
|
||||
.unwrap_or("?");
|
||||
let rstatus = r["status"].as_str().unwrap_or("?");
|
||||
let rbody = r["body"].as_str().unwrap_or("");
|
||||
@@ -414,10 +414,13 @@ async fn cmd_view(repo: String, number: u64, node: String, dir: Option<PathBuf>)
|
||||
let author_short = author
|
||||
.split(':')
|
||||
.next_back()
|
||||
.map(|s| &s[..s.len().min(8)])
|
||||
.map(|s| crate::text::truncate(s, 8))
|
||||
.unwrap_or("?");
|
||||
let cbody = c["body"].as_str().unwrap_or("");
|
||||
let created = c["created_at"].as_str().map(|s| &s[..10]).unwrap_or("?");
|
||||
let created = c["created_at"]
|
||||
.as_str()
|
||||
.map(|s| crate::text::truncate(s, 10))
|
||||
.unwrap_or("?");
|
||||
println!(" · {author_short} ({created})");
|
||||
println!(" {cbody}");
|
||||
}
|
||||
@@ -464,7 +467,7 @@ async fn cmd_merge(repo: String, number: u64, node: String, dir: Option<PathBuf>
|
||||
|
||||
let sha = result["merge_sha"].as_str().unwrap_or("?");
|
||||
println!("✓ Merged PR #{number}");
|
||||
println!(" Merge commit: {}", &sha[..sha.len().min(12)]);
|
||||
println!(" Merge commit: {}", crate::text::truncate(sha, 12));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -555,10 +558,13 @@ async fn cmd_comments(repo: String, number: u64, node: String, dir: Option<PathB
|
||||
let author_short = author
|
||||
.split(':')
|
||||
.next_back()
|
||||
.map(|s| &s[..s.len().min(8)])
|
||||
.map(|s| crate::text::truncate(s, 8))
|
||||
.unwrap_or("?");
|
||||
let cbody = c["body"].as_str().unwrap_or("");
|
||||
let created = c["created_at"].as_str().map(|s| &s[..10]).unwrap_or("?");
|
||||
let created = c["created_at"]
|
||||
.as_str()
|
||||
.map(|s| crate::text::truncate(s, 10))
|
||||
.unwrap_or("?");
|
||||
println!(" · {author_short} ({created})");
|
||||
println!(" {cbody}");
|
||||
println!();
|
||||
|
||||
@@ -337,7 +337,10 @@ async fn cmd_list(node: String, dir: Option<PathBuf>) -> Result<()> {
|
||||
let name = r["name"].as_str().unwrap_or("?");
|
||||
let desc = r["description"].as_str().unwrap_or("");
|
||||
let public = r["is_public"].as_bool().unwrap_or(true);
|
||||
let updated = r["updated_at"].as_str().map(|s| &s[..10]).unwrap_or("?");
|
||||
let updated = r["updated_at"]
|
||||
.as_str()
|
||||
.map(|s| crate::text::truncate(s, 10))
|
||||
.unwrap_or("?");
|
||||
let vis = if public { "public" } else { "private" };
|
||||
println!(" {name:<24} {vis:<8} {updated} {desc}");
|
||||
}
|
||||
@@ -514,7 +517,7 @@ async fn cmd_replicas(repo: String, node: String, dir: Option<PathBuf>) -> Resul
|
||||
let url = r["replica_url"].as_str().unwrap_or("?");
|
||||
let registered = r["registered_at"]
|
||||
.as_str()
|
||||
.map(|s| &s[..10.min(s.len())])
|
||||
.map(|s| crate::text::truncate(s, 10))
|
||||
.unwrap_or("?");
|
||||
println!(" {registered} {did} → {url}");
|
||||
}
|
||||
@@ -556,7 +559,7 @@ pub(crate) async fn cmd_commits(
|
||||
.or_else(|| c["sha"].as_str())
|
||||
.or_else(|| c["oid"].as_str())
|
||||
.unwrap_or("?");
|
||||
let short_sha = &sha[..sha.len().min(10)];
|
||||
let short_sha = crate::text::truncate(sha, 10);
|
||||
let msg = c["message"].as_str().unwrap_or("(no message)");
|
||||
let first_line = msg.lines().next().unwrap_or(msg);
|
||||
let author = c["author_name"]
|
||||
@@ -566,7 +569,7 @@ pub(crate) async fn cmd_commits(
|
||||
let date = c["date"]
|
||||
.as_str()
|
||||
.or_else(|| c["committer_date"].as_str())
|
||||
.map(|s| &s[..10.min(s.len())])
|
||||
.map(|s| crate::text::truncate(s, 10))
|
||||
.unwrap_or("?");
|
||||
println!(" {short_sha} {date} {first_line} ({author})");
|
||||
}
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
//! Display helpers for strings that come off the wire unchecked.
|
||||
|
||||
/// Truncate `s` to at most `max` bytes, cutting at a char boundary.
|
||||
///
|
||||
/// `&s[..max]` panics when `s` is shorter than `max` bytes and when `max`
|
||||
/// lands inside a multi-byte char; `s.len().min(max)` guards only the first
|
||||
/// case. Node-supplied timestamps and ids hit both.
|
||||
pub(crate) fn truncate(s: &str, max: usize) -> &str {
|
||||
let mut end = max.min(s.len());
|
||||
while !s.is_char_boundary(end) {
|
||||
end -= 1;
|
||||
}
|
||||
&s[..end]
|
||||
}
|
||||
|
||||
/// Keep at most the last `max` bytes of `s`, cutting at a char boundary.
|
||||
///
|
||||
/// `&s[s.len().saturating_sub(max)..]` panics when the start index lands
|
||||
/// inside a multi-byte char; node-supplied ids hit that case.
|
||||
pub(crate) fn truncate_suffix(s: &str, max: usize) -> &str {
|
||||
let mut start = s.len().saturating_sub(max);
|
||||
while !s.is_char_boundary(start) {
|
||||
start += 1;
|
||||
}
|
||||
&s[start..]
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{truncate, truncate_suffix};
|
||||
|
||||
#[test]
|
||||
fn cuts_ascii_at_max() {
|
||||
assert_eq!(truncate("2026-08-15T12:34:56Z", 10), "2026-08-15");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn short_and_empty_inputs_return_whole() {
|
||||
assert_eq!(truncate("2026", 10), "2026");
|
||||
assert_eq!(truncate("", 10), "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn multibyte_cut_does_not_panic() {
|
||||
// Byte index 10 lands inside 'é'; the cut must land on the boundary
|
||||
// before it (9 bytes) rather than panic or exceed the byte limit.
|
||||
assert_eq!(truncate("2026-08-1\u{e9}5T12:34:56Z", 10), "2026-08-1");
|
||||
// A string that is entirely multi-byte and shorter than max.
|
||||
assert_eq!(truncate("\u{e9}\u{e9}", 10), "\u{e9}\u{e9}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn suffix_cut_does_not_panic() {
|
||||
// 'é' is two bytes; a start index of len-8 lands inside it.
|
||||
assert_eq!(truncate_suffix("did:key:\u{e9}1234567", 8), "1234567");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn suffix_short_input_returns_whole() {
|
||||
assert_eq!(truncate_suffix("short", 8), "short");
|
||||
assert_eq!(truncate_suffix("", 8), "");
|
||||
}
|
||||
}
|
||||
@@ -88,20 +88,81 @@ fn sanitize_excerpt(s: &str) -> String {
|
||||
out
|
||||
}
|
||||
|
||||
/// Whether `u` parses as a trusted URL.
|
||||
/// Fixed warning when `GITLAWB_ICAPTCHA_INSECURE` is present but not an
|
||||
/// explicit truthy value. Does not include the raw env value (arbitrary
|
||||
/// deployment input — secrets / terminal controls).
|
||||
fn warn_icaptcha_insecure_non_truthy() {
|
||||
// One literal line on purpose: a continuation escape here keeps emitting
|
||||
// correct prose (Rust strips the next line's leading whitespace), but the
|
||||
// single-line form makes that property visible without knowing the escape
|
||||
// semantics, which is what the review asked for.
|
||||
tracing::warn!(
|
||||
"GITLAWB_ICAPTCHA_INSECURE is set but not truthy (expected 1 or true); loopback HTTP trust relaxation remains disabled"
|
||||
);
|
||||
}
|
||||
|
||||
/// Parse a possibly-present `GITLAWB_ICAPTCHA_INSECURE` string.
|
||||
///
|
||||
/// Production trusts only `https`. When `GITLAWB_ICAPTCHA_INSECURE` is set (a
|
||||
/// runtime escape hatch for integration tests) the function also trusts
|
||||
/// `http://127.0.0.1` and `http://localhost` so the full iCaptcha retry path
|
||||
/// can be exercised against a local mockito server.
|
||||
fn is_https(u: &str) -> bool {
|
||||
/// Only an explicit truthy value (`1` / `true`, case-insensitive, after trim)
|
||||
/// enables the loopback-HTTP trust relaxation. Presence alone is not enough:
|
||||
/// `=0`, `=false`, or an empty value leave it off. A non-empty non-truthy
|
||||
/// value emits [`warn_icaptcha_insecure_non_truthy`].
|
||||
fn icaptcha_insecure_from_str(raw: Option<&str>) -> bool {
|
||||
let Some(v) = raw else {
|
||||
return false;
|
||||
};
|
||||
let t = v.trim();
|
||||
if t.eq_ignore_ascii_case("1") || t.eq_ignore_ascii_case("true") {
|
||||
true
|
||||
} else {
|
||||
if !t.is_empty() {
|
||||
warn_icaptcha_insecure_non_truthy();
|
||||
}
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
/// Interpret `std::env::var("GITLAWB_ICAPTCHA_INSECURE")` without reading the
|
||||
/// environment again — used by production and by unit tests.
|
||||
///
|
||||
/// `NotPresent` → disabled. `NotUnicode` → disabled + the same fixed warning
|
||||
/// as a present non-truthy value (fail closed; do not treat as unset).
|
||||
fn icaptcha_insecure_from_var(result: Result<String, std::env::VarError>) -> bool {
|
||||
match result {
|
||||
Ok(v) => icaptcha_insecure_from_str(Some(&v)),
|
||||
Err(std::env::VarError::NotUnicode(_)) => {
|
||||
warn_icaptcha_insecure_non_truthy();
|
||||
false
|
||||
}
|
||||
Err(std::env::VarError::NotPresent) => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether `GITLAWB_ICAPTCHA_INSECURE` explicitly enables the loopback-HTTP
|
||||
/// trust relaxation used by integration tests.
|
||||
///
|
||||
/// Only an explicit truthy value (`1` / `true`, case-insensitive, after trim)
|
||||
/// enables it. Presence alone is not enough: `=0`, `=false`, or an empty
|
||||
/// value leave the relaxation off. A non-empty non-truthy value (including a
|
||||
/// present non-Unicode value) emits a `tracing::warn!` (visible when the
|
||||
/// `icaptcha_client` target is at WARN+, e.g. `RUST_LOG=icaptcha_client=warn`).
|
||||
fn icaptcha_insecure_enabled() -> bool {
|
||||
icaptcha_insecure_from_var(std::env::var("GITLAWB_ICAPTCHA_INSECURE"))
|
||||
}
|
||||
|
||||
/// Whether `u` parses as a trusted URL, given a pre-parsed insecure flag.
|
||||
///
|
||||
/// Production trusts only `https`. When `insecure` is true the function also
|
||||
/// trusts `http://127.0.0.1` and `http://localhost` so the full iCaptcha retry
|
||||
/// path can be exercised against a local mockito server.
|
||||
fn is_https_with(u: &str, insecure: bool) -> bool {
|
||||
let Ok(parsed) = reqwest::Url::parse(u) else {
|
||||
return false;
|
||||
};
|
||||
if parsed.scheme() == "https" {
|
||||
return true;
|
||||
}
|
||||
if std::env::var_os("GITLAWB_ICAPTCHA_INSECURE").is_some()
|
||||
if insecure
|
||||
&& parsed.scheme() == "http"
|
||||
&& matches!(parsed.host_str(), Some("127.0.0.1" | "localhost"))
|
||||
{
|
||||
@@ -139,7 +200,15 @@ fn origin_key(u: &str) -> Option<String> {
|
||||
/// explicitly-configured host (never a node-discovered one), so a key is never
|
||||
/// exfiltrated to an origin the operator did not choose.
|
||||
fn resolve_solver_url(advertised: Option<&str>, operator: Option<&str>) -> (String, bool) {
|
||||
let operator = operator.filter(|u| is_https(u));
|
||||
resolve_solver_url_with(advertised, operator, icaptcha_insecure_enabled())
|
||||
}
|
||||
|
||||
fn resolve_solver_url_with(
|
||||
advertised: Option<&str>,
|
||||
operator: Option<&str>,
|
||||
insecure: bool,
|
||||
) -> (String, bool) {
|
||||
let operator = operator.filter(|u| is_https_with(u, insecure));
|
||||
let operator_origin = operator.as_ref().and_then(|u| origin_key(u));
|
||||
let default_origin = origin_key(DEFAULT_URL);
|
||||
|
||||
@@ -157,7 +226,7 @@ fn resolve_solver_url(advertised: Option<&str>, operator: Option<&str>) -> (Stri
|
||||
};
|
||||
|
||||
let chosen = match advertised {
|
||||
Some(a) if is_https(a) && allowed(&origin_key(a)) => a.to_string(),
|
||||
Some(a) if is_https_with(a, insecure) && allowed(&origin_key(a)) => a.to_string(),
|
||||
Some(a) => {
|
||||
tracing::warn!(
|
||||
advertised = %a,
|
||||
@@ -389,27 +458,36 @@ mod tests {
|
||||
use std::ffi::OsString;
|
||||
use std::sync::{Mutex, MutexGuard};
|
||||
|
||||
/// Serializes tests that touch the process-global
|
||||
/// `GITLAWB_ICAPTCHA_INSECURE` env var so they never race.
|
||||
/// Serializes the real-env reader test so it never races with other
|
||||
/// process-global env mutations (and recovers from a poisoned lock).
|
||||
static ICAPTCHA_ENV_LOCK: Mutex<()> = Mutex::new(());
|
||||
|
||||
/// Set `GITLAWB_ICAPTCHA_INSECURE` for the test lifetime, restoring any
|
||||
/// prior value on drop.
|
||||
struct InsecureEnv {
|
||||
struct EnvGuard {
|
||||
_lock: MutexGuard<'static, ()>,
|
||||
prev: Option<OsString>,
|
||||
}
|
||||
|
||||
impl InsecureEnv {
|
||||
fn new() -> Self {
|
||||
let lock = ICAPTCHA_ENV_LOCK.lock().unwrap();
|
||||
impl EnvGuard {
|
||||
fn lock() -> MutexGuard<'static, ()> {
|
||||
ICAPTCHA_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner())
|
||||
}
|
||||
|
||||
fn with_value(val: &str) -> Self {
|
||||
let lock = Self::lock();
|
||||
let prev = std::env::var_os("GITLAWB_ICAPTCHA_INSECURE");
|
||||
std::env::set_var("GITLAWB_ICAPTCHA_INSECURE", "1");
|
||||
InsecureEnv { _lock: lock, prev }
|
||||
std::env::set_var("GITLAWB_ICAPTCHA_INSECURE", val);
|
||||
EnvGuard { _lock: lock, prev }
|
||||
}
|
||||
|
||||
fn unset() -> Self {
|
||||
let lock = Self::lock();
|
||||
let prev = std::env::var_os("GITLAWB_ICAPTCHA_INSECURE");
|
||||
std::env::remove_var("GITLAWB_ICAPTCHA_INSECURE");
|
||||
EnvGuard { _lock: lock, prev }
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for InsecureEnv {
|
||||
impl Drop for EnvGuard {
|
||||
fn drop(&mut self) {
|
||||
match self.prev.take() {
|
||||
Some(v) => std::env::set_var("GITLAWB_ICAPTCHA_INSECURE", v),
|
||||
@@ -418,6 +496,118 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// #227 / #246: cover the production env read in `icaptcha_insecure_enabled`.
|
||||
/// Pure-helper tests alone leave mutations of this function green.
|
||||
#[test]
|
||||
fn icaptcha_insecure_enabled_reads_real_env() {
|
||||
{
|
||||
let _g = EnvGuard::unset();
|
||||
assert!(!icaptcha_insecure_enabled(), "unset must disable");
|
||||
}
|
||||
{
|
||||
let _g = EnvGuard::with_value("0");
|
||||
assert!(!icaptcha_insecure_enabled(), "=0 must disable");
|
||||
}
|
||||
{
|
||||
let _g = EnvGuard::with_value("false");
|
||||
assert!(!icaptcha_insecure_enabled(), "=false must disable");
|
||||
}
|
||||
{
|
||||
let _g = EnvGuard::with_value("");
|
||||
assert!(!icaptcha_insecure_enabled(), "empty must disable");
|
||||
}
|
||||
{
|
||||
let _g = EnvGuard::with_value("1");
|
||||
assert!(icaptcha_insecure_enabled(), "=1 must enable");
|
||||
}
|
||||
{
|
||||
let _g = EnvGuard::with_value("true");
|
||||
assert!(icaptcha_insecure_enabled(), "=true must enable");
|
||||
}
|
||||
{
|
||||
let _g = EnvGuard::with_value("TRUE");
|
||||
assert!(icaptcha_insecure_enabled(), "=TRUE must enable");
|
||||
}
|
||||
{
|
||||
let _g = EnvGuard::with_value(" 1 ");
|
||||
assert!(icaptcha_insecure_enabled(), "\" 1 \" (trimmed) must enable");
|
||||
}
|
||||
}
|
||||
|
||||
/// #227: `=0` / `=false` / empty / unset must NOT enable loopback HTTP;
|
||||
/// only explicit truthy `1` / `true` may. Uses pure helpers so tests do
|
||||
/// not mutate process-global environment (avoids races with parallel
|
||||
/// resolver tests that also call `resolve_solver_url`).
|
||||
#[test]
|
||||
fn insecure_env_only_truthy_enables_loopback_http() {
|
||||
assert!(
|
||||
!is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some("0"))),
|
||||
"=0 must leave loopback HTTP disabled"
|
||||
);
|
||||
assert!(!icaptcha_insecure_from_str(Some("0")));
|
||||
|
||||
assert!(
|
||||
!is_https_with(
|
||||
"http://127.0.0.1",
|
||||
icaptcha_insecure_from_str(Some("false"))
|
||||
),
|
||||
"=false must leave loopback HTTP disabled"
|
||||
);
|
||||
assert!(!icaptcha_insecure_from_str(Some("false")));
|
||||
|
||||
assert!(
|
||||
!is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some(""))),
|
||||
"empty value must leave loopback HTTP disabled"
|
||||
);
|
||||
assert!(!icaptcha_insecure_from_str(Some("")));
|
||||
|
||||
assert!(
|
||||
!is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(None)),
|
||||
"unset must leave loopback HTTP disabled"
|
||||
);
|
||||
assert!(!icaptcha_insecure_from_str(None));
|
||||
assert!(!icaptcha_insecure_from_var(Err(
|
||||
std::env::VarError::NotPresent
|
||||
)));
|
||||
|
||||
assert!(
|
||||
is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some("1"))),
|
||||
"=1 must enable loopback HTTP"
|
||||
);
|
||||
assert!(is_https_with(
|
||||
"http://localhost",
|
||||
icaptcha_insecure_from_str(Some("1"))
|
||||
));
|
||||
assert!(icaptcha_insecure_from_str(Some("1")));
|
||||
|
||||
assert!(
|
||||
is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some("true"))),
|
||||
"=true must enable loopback HTTP"
|
||||
);
|
||||
assert!(icaptcha_insecure_from_str(Some("true")));
|
||||
|
||||
assert!(
|
||||
is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some("TRUE"))),
|
||||
"=TRUE (case-insensitive) must enable loopback HTTP"
|
||||
);
|
||||
|
||||
// Trim is load-bearing: whitespace around a truthy token must still enable.
|
||||
assert!(
|
||||
is_https_with("http://127.0.0.1", icaptcha_insecure_from_str(Some(" 1 "))),
|
||||
"\" 1 \" (trimmed) must enable loopback HTTP"
|
||||
);
|
||||
assert!(icaptcha_insecure_from_str(Some(" 1 ")));
|
||||
|
||||
// Present non-Unicode: fail closed + same fixed warning path as non-truthy.
|
||||
assert!(!icaptcha_insecure_from_var(Err(
|
||||
std::env::VarError::NotUnicode(OsString::from("x"))
|
||||
)));
|
||||
|
||||
// https remains trusted regardless of the insecure flag.
|
||||
assert!(is_https_with("https://icaptcha.gitlawb.com", false));
|
||||
assert!(is_https_with("https://icaptcha.gitlawb.com", true));
|
||||
}
|
||||
|
||||
// ── P1: hostile error bodies must not reach the terminal raw ──────────
|
||||
|
||||
#[test]
|
||||
@@ -543,13 +733,12 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn rejects_insecure_advertised_url_on_different_port() {
|
||||
// With GITLAWB_ICAPTCHA_INSECURE=1, two loopback URLs on different
|
||||
// With insecure mode enabled, two loopback URLs on different
|
||||
// ports must be treated as distinct origins so a hostile node cannot
|
||||
// redirect the bearer key to a different listener on localhost.
|
||||
let _env = InsecureEnv::new();
|
||||
|
||||
let op = "http://localhost:3000";
|
||||
let (url, key_trusted) = resolve_solver_url(Some("http://localhost:9000"), Some(op));
|
||||
let (url, key_trusted) =
|
||||
resolve_solver_url_with(Some("http://localhost:9000"), Some(op), true);
|
||||
assert_eq!(
|
||||
url, op,
|
||||
"must fall back to operator origin, not use different port"
|
||||
|
||||
Reference in new issue
Block a user