fix(gl): resolve webhook owner from caller identity, not node DID

crates/gl/src/mcp.rs

@@ -963,15 +963,15 @@ async fn call_tool(
                 .as_array()
                 .map(|a| a.iter().filter_map(|v| v.as_str()).collect::<Vec<_>>())
                 .unwrap_or_else(|| vec!["*"]);
-            let owner = resolve_owner(&args, &client).await?;
+            let (owner, name) = resolve_owner_repo(&args, repo, &keypair, &client).await?;
             let body = serde_json::to_vec(&json!({
                 "url": url,
                 "secret": args["secret"],
                 "events": events,
             }))?;
             let resp = crate::http::read_json(
                 client
-                    .post(&format!("/api/v1/repos/{owner}/{repo}/hooks"), &body)
+                    .post(&format!("/api/v1/repos/{owner}/{name}/hooks"), &body)
                     .await?,
                 "create webhook",
             )
@@ -981,19 +981,12 @@ async fn call_tool(
 
         "webhook_list" => {
             let repo = args["repo"].as_str().context("missing 'repo'")?;
-            // Owner-gated route: an explicit `owner` arg wins, otherwise default to
-            // the signing keypair's short DID (NOT the node DID that resolve_owner
-            // returns). The request is signed by this keypair, so the path owner
-            // must match it or the node returns 403/404.
-            let owner = if let Some(o) = args.get("owner").and_then(|v| v.as_str()) {
-                o.to_string()
-            } else {
-                let kp = keypair
-                    .as_ref()
-                    .context("no identity found — run `gl identity new` first")?;
-                let did = kp.did().to_string();
-                did.split(':').next_back().unwrap_or(&did).to_string()
-            };
+            // Owner-gated route: `owner/repo` splits first, an explicit `owner`
+            // arg wins next, otherwise the signing keypair's short DID (NOT the
+            // node DID that resolve_owner returns). The request is signed by
+            // this keypair, so the path owner must match it or the node
+            // returns 403/404.
+            let (owner, name) = resolve_owner_repo(&args, repo, &keypair, &client).await?;
             // Owner-gated AND auth-required route (list_webhooks 401s a headerless
             // caller, then 403s a non-owner). get_maybe_signed signs when an
             // identity is present (so the owner is authorized) and otherwise sends
@@ -1004,7 +997,7 @@ async fn call_tool(
             // failing client-side before the node is contacted.
             let resp = crate::http::read_json(
                 client
-                    .get_maybe_signed(&format!("/api/v1/repos/{owner}/{repo}/hooks"))
+                    .get_maybe_signed(&format!("/api/v1/repos/{owner}/{name}/hooks"))
                     .await?,
                 "webhook_list",
             )
@@ -1015,11 +1008,11 @@ async fn call_tool(
         "webhook_delete" => {
             let repo = args["repo"].as_str().context("missing 'repo'")?;
             let id = args["id"].as_str().context("missing 'id'")?;
-            let owner = resolve_owner(&args, &client).await?;
+            let (owner, name) = resolve_owner_repo(&args, repo, &keypair, &client).await?;
             let body = serde_json::to_vec(&json!({}))?;
             let resp = crate::http::read_json(
                 client
-                    .delete(&format!("/api/v1/repos/{owner}/{repo}/hooks/{id}"), &body)
+                    .delete(&format!("/api/v1/repos/{owner}/{name}/hooks/{id}"), &body)
                     .await?,
                 "delete webhook",
             )
@@ -1345,6 +1338,38 @@ async fn resolve_owner(args: &Value, client: &NodeClient) -> Result<String> {
     Ok(did.split(':').next_back().unwrap_or(did).to_string())
 }
 
+/// Resolve (owner, repo) for the webhook tools, matching the CLI's
+/// `resolve_owner_repo_pair` contract: an `owner/repo` string splits first, an
+/// explicit `owner` arg wins next, then the signing keypair's short DID. The
+/// last resort is the node root DID via `resolve_owner`, reachable only by
+/// callers that have no keypair — the unsigned `webhook_list` arm, and an
+/// unsigned `webhook_delete` the same way `resolve_owner` always served it —
+/// for a bare
+/// repo name under a signed caller the path owner must be the caller's DID or
+/// the node 403/404s the request under the wrong namespace.
+async fn resolve_owner_repo(
+    args: &Value,
+    repo: &str,
+    keypair: &Option<gitlawb_core::identity::Keypair>,
+    client: &NodeClient,
+) -> Result<(String, String)> {
+    if let Some((o, r)) = repo.split_once('/') {
+        return Ok((o.to_string(), r.to_string()));
+    }
+    if let Some(o) = args.get("owner").and_then(|v| v.as_str()) {
+        return Ok((o.to_string(), repo.to_string()));
+    }
+    if let Some(kp) = keypair {
+        let did = kp.did().to_string();
+        return Ok((
+            did.split(':').next_back().unwrap_or(&did).to_string(),
+            repo.to_string(),
+        ));
+    }
+    let owner = resolve_owner(args, client).await?;
+    Ok((owner, repo.to_string()))
+}
+
 /// Parse git pkt-line info/refs response into a list of {ref, sha} objects.
 fn parse_info_refs(bytes: &[u8]) -> Value {
     let mut refs = Vec::new();
@@ -2482,11 +2507,27 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_webhook_list_without_keypair_errors() {
-        // No identity loaded and no explicit owner → the tool must error rather
-        // than fall back to the node DID and issue an unsigned, mis-scoped GET.
-        let server = mockito::Server::new_async().await;
+    async fn test_webhook_list_without_keypair_surfaces_node_denial() {
+        // No identity loaded and no explicit owner → the tool falls back to the
+        // node DID for the path and still issues the GET unsigned, so the node
+        // itself answers the denial (list_webhooks 401s a headerless caller)
+        // rather than the client inventing a failure before contact.
+        let mut server = mockito::Server::new_async().await;
         let dir = tempfile::TempDir::new().unwrap(); // empty: no identity.pem
+        let _root = server
+            .mock("GET", "/")
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"did":"did:key:z6MkNodeOperator"}"#)
+            .create_async()
+            .await;
+        let _hooks = server
+            .mock("GET", "/api/v1/repos/z6MkNodeOperator/myrepo/hooks")
+            .with_status(401)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"message":"signature required"}"#)
+            .create_async()
+            .await;
 
         let err = call_tool(
             "webhook_list",
@@ -2495,8 +2536,13 @@ mod tests {
             Some(dir.path()),
         )
         .await
-        .expect_err("must error without an identity");
-        assert!(err.to_string().contains("no identity found"), "got: {err}");
+        .expect_err("unsigned list must surface the node denial");
+        let msg = err.to_string();
+        assert!(
+            msg.contains("401") || msg.contains("signature required"),
+            "got: {msg}"
+        );
+        _hooks.assert_async().await;
     }
 
     #[tokio::test]
@@ -3161,4 +3207,65 @@ mod tests {
         assert!(err.to_string().contains("404"), "got: {err}");
         _m.assert_async().await;
     }
+
+    #[tokio::test]
+    async fn resolve_owner_repo_splits_slash_form() {
+        let server = mockito::Server::new_async().await;
+        let (owner, name) = resolve_owner_repo(
+            &json!({}),
+            "alice/proj",
+            &None,
+            &NodeClient::new(server.url(), None),
+        )
+        .await
+        .unwrap();
+        assert_eq!((owner.as_str(), name.as_str()), ("alice", "proj"));
+    }
+
+    #[tokio::test]
+    async fn resolve_owner_repo_owner_arg_beats_keypair() {
+        let server = mockito::Server::new_async().await;
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let (owner, name) = resolve_owner_repo(
+            &json!({"owner": "bob"}),
+            "proj",
+            &Some(kp.clone()),
+            &NodeClient::new(server.url(), None),
+        )
+        .await
+        .unwrap();
+        assert_eq!((owner.as_str(), name.as_str()), ("bob", "proj"));
+    }
+
+    #[tokio::test]
+    async fn resolve_owner_repo_bare_repo_uses_keypair_did() {
+        let mut server = mockito::Server::new_async().await;
+        // A root mock answering with a different DID must never be consulted:
+        // under a signed caller the path owner is the keypair's short DID.
+        let _m = server
+            .mock("GET", "/")
+            .with_status(200)
+            .with_body(r#"{"did":"did:key:z6MkNodeOperator"}"#)
+            .expect(0)
+            .create_async()
+            .await;
+        let kp = gitlawb_core::identity::Keypair::generate();
+        let short = kp
+            .did()
+            .to_string()
+            .split(':')
+            .next_back()
+            .unwrap()
+            .to_string();
+        let (owner, name) = resolve_owner_repo(
+            &json!({}),
+            "proj",
+            &Some(kp),
+            &NodeClient::new(server.url(), None),
+        )
+        .await
+        .unwrap();
+        assert_eq!((owner.as_str(), name.as_str()), (short.as_str(), "proj"));
+        _m.assert_async().await;
+    }
 }