fix(agent_api_keys): harden webhook trigger setup

Author: danielmillerp

agentex/src/api/routes/agent_api_keys.py

@@ -1,5 +1,6 @@
 import os
 import secrets
+from urllib.parse import quote
 
 from fastapi import APIRouter, HTTPException, Query
 
@@ -38,6 +39,10 @@
 )
 
 
+def _has_control_chars(value: str) -> bool:
+    return any(ord(char) < 32 or ord(char) == 127 for char in value)
+
+
 @router.post(
     "",
     response_model=CreateAPIKeyResponse,
@@ -155,7 +160,7 @@ async def create_webhook_trigger(
             ),
         )
 
-    secret = request.secret or secrets.token_hex(32)
+    secret = request.secret if request.secret is not None else secrets.token_hex(32)
     agent_api_key_entity = await agent_api_key_use_case.create(
         agent_id=agent.id,
         api_key=str(secret),
@@ -164,7 +169,14 @@ async def create_webhook_trigger(
     )
 
     forward_path = request.forward_path.lstrip("/")
-    webhook_path = f"/agents/forward/name/{request.agent_name}/{forward_path}"
+    if _has_control_chars(forward_path):
+        raise HTTPException(
+            status_code=400,
+            detail="forward_path must not contain control characters.",
+        )
+    encoded_agent_name = quote(request.agent_name, safe="")
+    encoded_forward_path = quote(forward_path, safe="/")
+    webhook_path = f"/agents/forward/name/{encoded_agent_name}/{encoded_forward_path}"
     base_url = (request.base_url or os.environ.get("AGENTEX_PUBLIC_URL", "")).rstrip(
         "/"
     )

agentex/src/api/schemas/agent_api_keys.py

@@ -76,7 +76,11 @@ class CreateWebhookTriggerRequest(BaseModel):
     )
     secret: str | None = Field(
         None,
-        description="Optional signing secret; if unset, one is generated and returned.",
+        description=(
+            "Signing secret. For GitHub, omit to generate one, or provide an existing "
+            "webhook secret. For Slack, this is required and must be the Slack app's "
+            "Signing Secret."
+        ),
     )
     base_url: str | None = Field(
         None,

agentex/tests/unit/api/test_webhook_trigger.py

@@ -76,6 +76,44 @@ async def test_uses_provided_secret_and_no_url_without_base(self, monkeypatch):
         assert resp.webhook_url is None  # no AGENTEX_PUBLIC_URL configured
         assert resp.webhook_path == "/agents/forward/name/a/gh"
 
+    async def test_github_empty_secret_is_preserved(self, monkeypatch):
+        req = CreateWebhookTriggerRequest(
+            agent_name="a",
+            source=AgentAPIKeyType.GITHUB,
+            name="o/r",
+            forward_path="gh",
+            secret="",
+        )
+        resp, akuc = await self._call(monkeypatch, req)
+        assert resp.secret == ""
+        assert akuc.create.await_args.kwargs["api_key"] == ""
+
+    async def test_forward_path_reserved_chars_are_url_encoded(self, monkeypatch):
+        req = CreateWebhookTriggerRequest(
+            agent_name="golden agent",
+            source=AgentAPIKeyType.GITHUB,
+            name="o/r",
+            forward_path="github-pr/cfg-9?mode=review#frag ment",
+        )
+        resp, _ = await self._call(monkeypatch, req, base_env="https://sgp.example.com/")
+
+        assert (
+            resp.webhook_path
+            == "/agents/forward/name/golden%20agent/github-pr/cfg-9%3Fmode%3Dreview%23frag%20ment"
+        )
+        assert resp.webhook_url == f"https://sgp.example.com{resp.webhook_path}"
+
+    async def test_forward_path_control_chars_rejected(self, monkeypatch):
+        req = CreateWebhookTriggerRequest(
+            agent_name="a",
+            source=AgentAPIKeyType.GITHUB,
+            name="o/r",
+            forward_path="github-pr/cfg-9\nnext",
+        )
+        with pytest.raises(HTTPException) as exc:
+            await self._call(monkeypatch, req)
+        assert exc.value.status_code == 400
+
     async def test_conflict_when_key_exists(self, monkeypatch):
         req = CreateWebhookTriggerRequest(
             agent_name="a", source=AgentAPIKeyType.GITHUB, name="o/r", forward_path="gh"
@@ -118,3 +156,11 @@ async def test_slack_with_provided_secret_ok(self, monkeypatch):
         resp, akuc = await self._call(monkeypatch, req)
         assert resp.secret == "slack-signing-secret"
         assert akuc.create.await_args.kwargs["api_key"] == "slack-signing-secret"
+
+    async def test_secret_schema_documents_slack_requirement(self):
+        description = CreateWebhookTriggerRequest.model_fields["secret"].description
+
+        assert description is not None
+        assert "For GitHub" in description
+        assert "For Slack" in description
+        assert "required" in description