Skip to content

Commit 2cdba73

Browse files
fix(gl,git-remote): make a delegation issued the documented way actually work
Two silent failures, both of which end as a 403 telling the delegate to obtain the delegation they are already holding. The narrowing fix broke the documented issuing form ---------------------------------------------------- `build_invocation` built `gitlawb://repos/{owner}/{repo}` from the push URL and compared it to the delegation's `with` with `==`. The URL always carries the BARE owner, since `parse_gitlawb_url` takes the last colon-delimited segment, while `docs/RUN-A-NODE.md` tells the owner to issue `--cap gitlawb://repos/<owner-did>/<repo>` — the full DID. Those strings never match, `find` returns None, and because every failure in `delegation_header` is best-effort the push goes out with no `X-Ucan`. This was introduced by the previous round: copying `att` through unchanged worked because the node normalizes both forms in `did_matches`. Narrowing to a URL-derived string did not. The owner segment is now compared on the bare key, and the parent's `with` is kept VERBATIM whenever it already names this repo — `is_attenuated_by` compares `with` by exact equality, so re-emitting the bare form under a full-DID parent would fail attenuation at the node and trade one silent refusal for another. Only a `*` parent uses the URL-derived resource, which is the case narrowing exists for. Both combinations are now tested; neither side exercised them before. The two halves disagreed about where delegations live ------------------------------------------------------ `git-remote-gitlawb` resolves its store from `resolve_key_path().parent()`, which honors `GITLAWB_KEY`. `gl ucan import` wrote under `gitlawb_dir(None)`, which was always `~/.gitlawb`. With `GITLAWB_KEY=/data/keys/identity.pem` — the shape `.env.example` documents — the import stored the token in one directory and the helper read another, empty one. `gitlawb_dir` now falls back to the parent of `GITLAWB_KEY` before `~/.gitlawb`, so both halves derive the store from the same setting. Also: `.env.example` still said a push from a non-owner DID is rejected, which is the behaviour this branch removes.
1 parent d8eb3fc commit 2cdba73

3 files changed

Lines changed: 122 additions & 4 deletions

File tree

‎.env.example‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,9 @@ GITLAWB_REQUIRE_SIGNED_PEER_WRITES=false
9494

9595
# Require the authenticated pusher to be the repo owner on git-receive-pack.
9696
# A valid did:key signature is authentication, not authorization: anyone can
97-
# sign as their own DID. When true, pushes from a non-owner DID are rejected.
98-
# Keep false until the repo owner is ready for owner-only writes.
97+
# sign as their own DID. When true, a push is accepted only from the repo owner,
98+
# or from a holder of an owner-rooted git/push UCAN for that repo (see
99+
# docs/RUN-A-NODE.md). Keep false until your pushers are the owner or hold one.
99100
GITLAWB_ENFORCE_OWNER_PUSH=false
100101

101102
# Comma-separated libp2p multiaddrs.

‎crates/git-remote-gitlawb/src/main.rs‎

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -417,19 +417,44 @@ fn build_invocation(
417417
// Constraints are copied from the covering capability, not dropped: dropping
418418
// them is a widening and `verify_chain` refuses it.
419419
let resource = format!("gitlawb://repos/{owner}/{repo}");
420+
421+
// The owner segment must be compared on the bare key. `parse_gitlawb_url` yields
422+
// the bare form from the push URL, while an owner following the operator guide
423+
// issues `--cap gitlawb://repos/<owner-did>/<repo>` — the full DID. Comparing the
424+
// whole resource string finds nothing, and because every failure here is
425+
// best-effort the push goes out with no header and the delegate is told to obtain
426+
// the delegation they already hold.
427+
fn bare(d: &str) -> &str {
428+
d.strip_prefix("did:key:").unwrap_or(d)
429+
}
430+
let names_this_repo = |with: &str| {
431+
with.strip_prefix("gitlawb://repos/")
432+
.and_then(|rest| rest.rsplit_once('/'))
433+
.is_some_and(|(o, r)| bare(o) == bare(owner) && r == repo)
434+
};
435+
420436
let source = delegation
421437
.payload
422438
.att
423439
.iter()
424440
.find(|c| {
425-
(c.with == resource || c.with == "*")
441+
(c.with == "*" || names_this_repo(&c.with))
426442
&& (c.can == caps::GIT_PUSH || c.can == "*" || c.can == caps::REPO_ADMIN)
427443
})
428444
.ok_or_else(|| {
429445
anyhow::anyhow!("stored delegation carries no git/push capability for {resource}")
430446
})?;
431447

432-
let mut narrowed = Capability::new(resource, caps::GIT_PUSH);
448+
// Keep the parent's resource verbatim when it already names this repo. The node's
449+
// `is_attenuated_by` compares `with` by exact equality, so re-emitting a bare form
450+
// under a full-DID parent would fail attenuation and be refused. Only a `*` parent
451+
// needs the URL-derived string, and that is the case narrowing exists for.
452+
let narrowed_with = if source.with == "*" {
453+
resource
454+
} else {
455+
source.with.clone()
456+
};
457+
let mut narrowed = Capability::new(narrowed_with, caps::GIT_PUSH);
433458
narrowed.constraints = source.constraints.clone();
434459

435460
// Carry the delegation's own expiry onto the invocation. The node refuses a
@@ -2448,6 +2473,77 @@ mod delegated_push_tests {
24482473
);
24492474
}
24502475

2476+
/// The shipping combination: the owner issues with the FULL DID (what
2477+
/// `RUN-A-NODE.md` instructs), while the push URL yields the BARE key
2478+
/// (`parse_gitlawb_url` takes the last colon-delimited segment). Comparing the
2479+
/// whole resource string never matches, so the delegation is not found, no
2480+
/// `X-Ucan` is sent, and the delegate gets a 403 telling them to obtain the
2481+
/// delegation they are already holding.
2482+
#[test]
2483+
fn build_invocation_matches_a_full_did_delegation_against_a_bare_owner() {
2484+
use gitlawb_core::ucan::{caps, Capability, Ucan};
2485+
2486+
let owner = Keypair::generate();
2487+
let agent = Keypair::generate();
2488+
let node = Keypair::generate();
2489+
let full = owner.did().to_string();
2490+
let bare = full.strip_prefix("did:key:").unwrap().to_string();
2491+
let hour = chrono::Utc::now() + chrono::Duration::hours(1);
2492+
2493+
let delegation = Ucan::issue(
2494+
&owner,
2495+
agent.did(),
2496+
vec![Capability::new(
2497+
format!("gitlawb://repos/{full}/r"),
2498+
caps::GIT_PUSH,
2499+
)],
2500+
Some(hour),
2501+
)
2502+
.expect("issue");
2503+
2504+
let invocation = build_invocation(&agent, &node.did(), &delegation, &bare, "r")
2505+
.expect("a full-DID delegation must match a bare-owner push URL");
2506+
2507+
// The narrowed capability must keep the parent's exact `with`: the node's
2508+
// `is_attenuated_by` compares it by equality, so emitting the bare form under
2509+
// a full-DID parent would fail attenuation and be refused at the node.
2510+
assert_eq!(
2511+
invocation.payload.att[0].with,
2512+
format!("gitlawb://repos/{full}/r"),
2513+
"narrowing must not rewrite a resource that already names this repo"
2514+
);
2515+
assert!(
2516+
invocation.payload.att[0].is_attenuated_by(&delegation.payload.att[0]),
2517+
"the narrowed capability must still attenuate under its parent"
2518+
);
2519+
}
2520+
2521+
#[test]
2522+
fn build_invocation_narrows_a_wildcard_to_the_pushed_repo() {
2523+
use gitlawb_core::ucan::{caps, Capability, Ucan};
2524+
2525+
let owner = Keypair::generate();
2526+
let agent = Keypair::generate();
2527+
let node = Keypair::generate();
2528+
let hour = chrono::Utc::now() + chrono::Duration::hours(1);
2529+
2530+
let delegation = Ucan::issue(
2531+
&owner,
2532+
agent.did(),
2533+
vec![Capability::new("*", caps::GIT_PUSH)],
2534+
Some(hour),
2535+
)
2536+
.expect("issue");
2537+
2538+
let invocation =
2539+
build_invocation(&agent, &node.did(), &delegation, "z6MkOwner", "r").expect("wrap");
2540+
assert_eq!(
2541+
invocation.payload.att[0].with, "gitlawb://repos/z6MkOwner/r",
2542+
"a wildcard parent is the one case where the URL-derived resource is used"
2543+
);
2544+
assert!(invocation.payload.att[0].is_attenuated_by(&delegation.payload.att[0]));
2545+
}
2546+
24512547
#[test]
24522548
fn split_pack_post_url_separates_origin_owner_and_repo() {
24532549
assert_eq!(

‎crates/gl/src/identity.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,31 @@ pub async fn run(cmd: IdentityCmd) -> Result<()> {
6565

6666
/// Resolve the identity directory, honouring an explicit override.
6767
/// Public so sibling commands (`gl ucan import`) store alongside the identity.
68+
///
69+
/// Falls back to the parent of `GITLAWB_KEY` before `~/.gitlawb`. Without that,
70+
/// an operator who moved their key — `GITLAWB_KEY=/data/keys/identity.pem`, the
71+
/// shape `.env.example` documents — has `gl ucan import` write the delegation to
72+
/// `~/.gitlawb/delegations` while `git-remote-gitlawb`, which resolves its store
73+
/// from `GITLAWB_KEY`, reads an empty directory. The push then goes out with no
74+
/// `X-Ucan` and the delegate is refused, with nothing to indicate why.
6875
pub fn gitlawb_dir(override_dir: Option<PathBuf>) -> Result<PathBuf> {
6976
if let Some(d) = override_dir {
7077
return Ok(d);
7178
}
79+
if let Ok(key) = std::env::var("GITLAWB_KEY") {
80+
if !key.trim().is_empty() {
81+
let path = if let Some(rest) = key.strip_prefix("~/") {
82+
dirs::home_dir()
83+
.context("could not determine home directory")?
84+
.join(rest)
85+
} else {
86+
PathBuf::from(key)
87+
};
88+
if let Some(parent) = path.parent() {
89+
return Ok(parent.to_path_buf());
90+
}
91+
}
92+
}
7293
let home = dirs::home_dir().context("could not determine home directory")?;
7394
Ok(home.join(".gitlawb"))
7495
}

0 commit comments

Comments
 (0)