feat(agent_api_keys): one-call webhook-trigger setup endpoint #329
+384
−0
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||
| import os | ||||||
| import secrets | ||||||
|
|
||||||
| from fastapi import APIRouter, HTTPException, Query | ||||||
|
|
@@ -7,6 +8,8 @@ | |||||
| AgentAPIKey, | ||||||
| CreateAPIKeyRequest, | ||||||
| CreateAPIKeyResponse, | ||||||
| CreateWebhookTriggerRequest, | ||||||
| CreateWebhookTriggerResponse, | ||||||
| ) | ||||||
| from src.api.schemas.authorization_types import ( | ||||||
| AgentexResourceType, | ||||||
|
|
@@ -93,6 +96,91 @@ async def create_api_key( | |||||
| ) | ||||||
|
|
||||||
|
|
||||||
| @router.post( | ||||||
| "/webhook-trigger", | ||||||
| response_model=CreateWebhookTriggerResponse, | ||||||
| ) | ||||||
| async def create_webhook_trigger( | ||||||
| request: CreateWebhookTriggerRequest, | ||||||
| agent_api_key_use_case: DAgentAPIKeysUseCase, | ||||||
| agent_use_case: DAgentsUseCase, | ||||||
| authorization_service: DAuthorizationService, | ||||||
| ) -> CreateWebhookTriggerResponse: | ||||||
| """Wire a webhook trigger in one call. | ||||||
|
|
||||||
| Registers the source's signature-verification key (github/slack) for the agent and | ||||||
| returns the ready-to-paste forward webhook URL plus the signing secret (shown once). | ||||||
| The webhook then flows through the existing /agents/forward ingress, which verifies | ||||||
| the signature against this key. Bundles the existing key-create + URL composition so | ||||||
| a UI (or a curl) can set up a trigger without two steps. | ||||||
| """ | ||||||
| if request.source not in (AgentAPIKeyType.GITHUB, AgentAPIKeyType.SLACK): | ||||||
| raise HTTPException( | ||||||
| status_code=400, | ||||||
| detail="source must be 'github' or 'slack' for a webhook trigger.", | ||||||
| ) | ||||||
| agent = await agent_use_case.get(name=request.agent_name) | ||||||
|
|
||||||
| # No api_key resource exists yet, so gate on the parent agent (update). | ||||||
| await _check_agent_or_collapse_to_404( | ||||||
| authorization_service, | ||||||
| agent.id, | ||||||
| AuthorizedOperationType.update, | ||||||
| ) | ||||||
|
|
||||||
| existing_api_key = await agent_api_key_use_case.get_by_agent_id_and_name( | ||||||
| agent_id=agent.id, | ||||||
| name=request.name, | ||||||
| api_key_type=request.source, | ||||||
| ) | ||||||
| if existing_api_key: | ||||||
| # A duplicate is an expected client condition (409), not a server error, and the | ||||||
| # message avoids leaking the internal agent UUID the caller never supplied. | ||||||
| raise HTTPException( | ||||||
| status_code=409, | ||||||
| detail=f"A {request.source} webhook key named '{request.name}' already exists for this agent.", | ||||||
| ) | ||||||
|
|
||||||
| # GitHub lets you supply (and we can generate) a per-webhook secret to paste into the | ||||||
| # repo's Secret field. Slack is different: it signs every request with the app's own | ||||||
| # Signing Secret, so the caller must supply that exact value — a generated one would | ||||||
| # never match, and validate_slack_delivery_webhook would reject every real delivery. | ||||||
| # (See PR #329 discussion.) | ||||||
| if request.source == AgentAPIKeyType.SLACK and not request.secret: | ||||||
| raise HTTPException( | ||||||
| status_code=400, | ||||||
| detail=( | ||||||
| "Slack triggers must supply 'secret' set to the Slack app's Signing Secret " | ||||||
| "(from your app credentials); it can't be generated." | ||||||
| ), | ||||||
| ) | ||||||
|
|
||||||
| secret = request.secret or secrets.token_hex(32) | ||||||
|
There was a problem hiding this comment.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: agentex/src/api/routes/agent_api_keys.py
Line: 157
Comment:
Empty string for `secret` is falsy in Python, so `request.secret or secrets.token_hex(32)` will generate a new random secret rather than treating the empty string as-is. A caller who sends `"secret": ""` intending to set up a no-secret GitHub webhook (GitHub allows omitting the secret, in which case it sends no `X-Hub-Signature-256` header) would receive a generated secret, register it on the key, but configure GitHub with no secret — causing every delivery to be rejected with an invalid-signature failure. Replacing `or` with an explicit `None` check avoids the silent coercion.
```suggestion
secret = request.secret if request.secret is not None else secrets.token_hex(32)
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| agent_api_key_entity = await agent_api_key_use_case.create( | ||||||
| agent_id=agent.id, | ||||||
| api_key=str(secret), | ||||||
| name=request.name, | ||||||
| api_key_type=request.source, | ||||||
|
greptile-apps[bot] marked this conversation as resolved.
|
||||||
| ) | ||||||
|
|
||||||
| forward_path = request.forward_path.lstrip("/") | ||||||
| webhook_path = f"/agents/forward/name/{request.agent_name}/{forward_path}" | ||||||
|
Comment on lines
+166
to
+167
There was a problem hiding this comment.
ArtifactsRepro: focused FastAPI pytest for raw forward_path delimiter routing
Prompt To Fix With AIThis is a comment left during a code review.
Path: agentex/src/api/routes/agent_api_keys.py
Line: 166-167
Comment:
**Raw path breaks routing**
`forward_path` is inserted into the returned URL as raw text, so URL delimiters can change what the forward route receives. For example, `forward_path="github-pr/cfg-9?mode=review"` returns `/agents/forward/name/<agent>/github-pr/cfg-9?mode=review`; when GitHub posts to it, FastAPI receives `github-pr/cfg-9` as the path and `mode=review` as the query string, so the agent never gets the configured subpath. `#`, spaces, and other reserved characters have similar effects. Encode the path segment or reject query, fragment, and control characters before returning the pasteable URL.
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| base_url = (request.base_url or os.environ.get("AGENTEX_PUBLIC_URL", "")).rstrip( | ||||||
| "/" | ||||||
| ) | ||||||
| webhook_url = f"{base_url}{webhook_path}" if base_url else None | ||||||
|
|
||||||
| return CreateWebhookTriggerResponse( | ||||||
| key_id=agent_api_key_entity.id, | ||||||
| agent_name=request.agent_name, | ||||||
| source=agent_api_key_entity.api_key_type, | ||||||
| name=request.name, | ||||||
| secret=str(secret), | ||||||
| webhook_path=webhook_path, | ||||||
| webhook_url=webhook_url, | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| @router.get( | ||||||
| "", | ||||||
| response_model=list[AgentAPIKey], | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,3 +53,53 @@ class CreateAPIKeyResponse(BaseModel): | |
| ..., | ||
| description="The value of the newly created API key.", | ||
| ) | ||
|
|
||
|
|
||
| class CreateWebhookTriggerRequest(BaseModel): | ||
| """One-call setup for a webhook trigger: register the source's signature key and | ||
| get back the ready-to-paste forward webhook URL.""" | ||
|
|
||
| agent_name: str = Field(..., description="The agent the webhook drives.") | ||
| source: AgentAPIKeyType = Field( | ||
| AgentAPIKeyType.GITHUB, | ||
| description="Webhook source whose signature is verified (github or slack).", | ||
| ) | ||
| name: str = Field( | ||
| ..., | ||
| description="Signature-lookup key: the repo full_name (github) or api_app_id " | ||
| "(slack) that the forward ingress matches the incoming webhook against.", | ||
| ) | ||
| forward_path: str = Field( | ||
| ..., | ||
| description="Subpath the agent's own route handles, e.g. 'github-pr/<config-id>'. " | ||
| "Appended to /agents/forward/name/{agent_name}/ to form the webhook URL.", | ||
| ) | ||
| secret: str | None = Field( | ||
| None, | ||
| description="Optional signing secret; if unset, one is generated and returned.", | ||
| ) | ||
|
Comment on lines
+77
to
+80
There was a problem hiding this comment.
The route now returns ArtifactsRepro: FastAPI TestClient schema and endpoint harness
Repro: OpenAPI schema excerpt and Slack endpoint 400 response output
Prompt To Fix With AIThis is a comment left during a code review.
Path: agentex/src/api/schemas/agent_api_keys.py
Line: 77-80
Comment:
**Slack contract is stale**
The route now returns `400` when `source` is `slack` and `secret` is omitted, but this request schema still documents `secret` as optional and generated when unset. Generated clients and UI built from this schema can omit the Slack signing secret, then every Slack setup call fails instead of completing the one-call trigger flow.
How can I resolve this? If you propose a fix, please make it concise. |
||
| base_url: str | None = Field( | ||
| None, | ||
| description="Optional public agentex base URL for the returned webhook_url; " | ||
| "defaults to the AGENTEX_PUBLIC_URL env var.", | ||
| ) | ||
|
|
||
|
|
||
| class CreateWebhookTriggerResponse(BaseModel): | ||
| key_id: str = Field(..., description="The created agent API key id.") | ||
| agent_name: str = Field(..., description="The agent the webhook drives.") | ||
| source: AgentAPIKeyType = Field( | ||
| ..., description="Webhook source (github or slack)." | ||
| ) | ||
| name: str = Field( | ||
| ..., description="Signature-lookup key (repo full_name / api_app_id)." | ||
| ) | ||
| secret: str = Field( | ||
| ..., | ||
| description="The signing secret — shown once; paste into the source's webhook config.", | ||
| ) | ||
| webhook_path: str = Field(..., description="The forward path to POST webhooks to.") | ||
| webhook_url: str | None = Field( | ||
| None, | ||
| description="Full webhook URL to paste into the source (None if no base URL configured).", | ||
| ) | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there other webhooks we want to scope out (linear, notion, pager duty, datadog?) Is there a way to make it easier to add a key than updating the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferring to follow-ups, keeping this PR scoped to github/slack. Each new source (Linear, Notion, PagerDuty, Datadog) needs its own signature-verification scheme wired into the forward path — Linear is HMAC-SHA256 like GitHub, but PagerDuty/Datadog/Notion differ — so it's real per-source work rather than just an enum value. When we add the next source we'll introduce a small
{source: signature_scheme}registry so adding one becomes a single entry instead of scattered changes. Tracking that as the extension point.