From c4c16317b2af38ef25278b1ea03deb1737dd03b7 Mon Sep 17 00:00:00 2001 From: Wojtek Siudzinski Date: Sun, 2 Aug 2026 18:33:03 +0200 Subject: [PATCH 1/2] feat(triggers): add external webhook trigger for agent dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New internal trigger internal:external-webhook: any third-party system can dispatch an agent by POSTing to the router at /external/webhook/:projectId/:agentType with Authorization: Bearer . - Password is the EXTERNAL_WEBHOOK_PASSWORD_ project credential (org-credential inheritance applies). Authentication fails closed: no stored password rejects every request with 403; comparison is timing-safe. - POST body (capped at 64 KiB) reaches the agent as trigger context via the manual-run path's triggerCommentBody. Dispatch reuses createQueuedRun + submitDashboardJob({type:'manual-run'}) — no new job variant, adapter, or trigger handler. Runs carry triggerType 'external-webhook'. - Enablement enforced end-to-end: agent enabled in project + trigger enabled (getResolvedTriggerConfig); unknown/undeclared/disabled all return 404 (anti-probing) with distinct decision reasons in webhook_logs (source 'external'; Authorization header never logged). - Declared on 7 agents (implementation, planning, splitting, backlog-manager, review, resolve-conflicts, alerting), defaultEnabled false. - UI: Agents → agent → Triggers tab shows the webhook URL (copy button + curl example) and password field when the trigger is enabled, via a new renderTriggerExtra slot on DefinitionTriggerToggles. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 2 + docs/architecture/03-trigger-system.md | 2 +- src/agents/definitions/alerting.yaml | 7 + src/agents/definitions/backlog-manager.yaml | 7 + src/agents/definitions/implementation.yaml | 7 + src/agents/definitions/planning.yaml | 7 + src/agents/definitions/resolve-conflicts.yaml | 7 + src/agents/definitions/review.yaml | 7 + src/agents/definitions/splitting.yaml | 7 + src/queue/client.ts | 7 + src/router/external-webhook.ts | 173 ++++++++++++ src/router/index.ts | 6 + src/triggers/README.md | 2 +- src/triggers/shared/events.ts | 1 + src/triggers/shared/external-webhook.ts | 38 +++ src/triggers/shared/manual-runner.ts | 7 +- src/types/index.ts | 3 +- src/worker-entry.ts | 6 + .../external-webhook-trigger.test.ts | 40 +++ tests/unit/router/external-webhook.test.ts | 252 ++++++++++++++++++ .../triggers/external-webhook-helper.test.ts | 74 +++++ tests/unit/triggers/shared/events.test.ts | 1 + .../projects/agent-config-detail.tsx | 11 + .../external-webhook-trigger-config.tsx | 97 +++++++ .../shared/definition-trigger-toggles.tsx | 6 + 25 files changed, 773 insertions(+), 4 deletions(-) create mode 100644 src/router/external-webhook.ts create mode 100644 src/triggers/shared/external-webhook.ts create mode 100644 tests/unit/agents/definitions/external-webhook-trigger.test.ts create mode 100644 tests/unit/router/external-webhook.test.ts create mode 100644 tests/unit/triggers/external-webhook-helper.test.ts create mode 100644 web/src/components/projects/external-webhook-trigger-config.tsx diff --git a/CLAUDE.md b/CLAUDE.md index a8a77ddad..5b2bc7f0f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,6 +134,8 @@ cascade projects trigger-set --agent --event --enabl Some triggers take params (e.g. `review` + `scm:check-suite-success` accepts `{"authorMode":"own"|"external"}`). Legacy configs on `project_integrations.triggers` are auto-migrated on merge to `dev`/`main`. +**External webhook trigger** — `internal:external-webhook` lets any third-party system dispatch an agent via `POST {router}/external/webhook/:projectId/:agentType` with `Authorization: Bearer `. The password is the `EXTERNAL_WEBHOOK_PASSWORD_` project credential (org inheritance applies); authentication **fails closed** — no stored password means every request is rejected with 403. The POST body (≤64 KiB) reaches the agent as trigger context (`triggerCommentBody`), and dispatch rides the existing manual-run dashboard-job path (`createQueuedRun` + `submitDashboardJob`), so runs appear in the dashboard as trigger `external-webhook`. Enable per agent in the project's Agents → Triggers tab, which displays the URL and password field (`src/router/external-webhook.ts`, helpers at `src/triggers/shared/external-webhook.ts`). + **Work-item concurrency lock** — the router prevents duplicate agent runs via a per-agent-type lock on `(projectId, workItemId, agentType)`. Only same-type duplicates are blocked; **different agent types can run concurrently** on the same work item (e.g. review starts while implementation's container is still cleaning up). The lock has a 30-minute TTL hard ceiling that auto-clears stale entries after router restart. **Implementation freshness gate** — MNG-1053. PM router adapters intentionally embed a pre-resolved `TriggerResult` for delayed/coalesced PM jobs, so the work-item lock alone cannot prevent a stale implementation snapshot from running. The shared execution pipeline at `src/triggers/shared/agent-execution.ts` now runs a worker-side freshness gate (`src/triggers/shared/implementation-freshness-gate.ts`) before `persistAgentWorkItemLinks()` / `prepareForAgent()`. The gate only fires for `agentType === 'implementation'` with a resolved `workItemId` — review/respond-to-* and follow-up agents bypass it. It reloads live PM work-item state and terminal checklists (`Implementation Steps`, `Acceptance Criteria`), counts active same-type runs, and verifies linked PRs by resolving the implementer GitHub persona token before calling `githubClient.getPR()` (so manual/retry pipeline callers do not depend on ambient GitHub scope). Open or merged PRs and fully-complete terminal checklists block dispatch with a durable `Implementation not started:` PM comment (updating the existing ack comment when present). Checklist read uncertainty always falls into `needs_human_reconciliation`; PR lookup uncertainty does the same when a DB/run-linked PR candidate exists. Closed-unmerged PRs do NOT permanently block reimplementation. diff --git a/docs/architecture/03-trigger-system.md b/docs/architecture/03-trigger-system.md index f5d6900f6..49a89ea7a 100644 --- a/docs/architecture/03-trigger-system.md +++ b/docs/architecture/03-trigger-system.md @@ -149,7 +149,7 @@ Triggers use category-prefixed events from `src/triggers/shared/events.ts`. `TRI - PM: `pm:status-changed`, `pm:label-added`, `pm:comment-mention` - SCM: `scm:check-suite-success`, `scm:check-suite-failure`, `scm:pr-review-submitted`, `scm:review-requested`, `scm:pr-opened`, `scm:pr-comment-mention`, `scm:pr-merged`, `scm:pr-ready-to-merge`, `scm:pr-conflict-detected` - Alerting: `alerting:issue-alert`, `alerting:metric-alert`, `alerting:issue-lifecycle` -- Internal: `internal:auto-chain` +- Internal: `internal:auto-chain`, `internal:external-webhook` New handlers should import `TRIGGER_EVENTS` instead of adding raw string literals. The static guard in `tests/unit/triggers/trigger-event-consistency.test.ts` fails when a handler gates on one event string and emits a different `agentInput.triggerEvent`. diff --git a/src/agents/definitions/alerting.yaml b/src/agents/definitions/alerting.yaml index 601be8271..15cc02e4a 100644 --- a/src/agents/definitions/alerting.yaml +++ b/src/agents/definitions/alerting.yaml @@ -49,6 +49,13 @@ triggers: defaultEnabled: true providers: [sentry] contextPipeline: [alertingIssue, directoryListing, contextFiles] + - event: internal:external-webhook + label: External Webhook + description: >- + Dispatch this agent from an external system via an authenticated POST to + the project's per-agent webhook URL. The request body reaches the agent + as trigger context. + defaultEnabled: false strategies: {} diff --git a/src/agents/definitions/backlog-manager.yaml b/src/agents/definitions/backlog-manager.yaml index 26f7692ca..450c214c9 100644 --- a/src/agents/definitions/backlog-manager.yaml +++ b/src/agents/definitions/backlog-manager.yaml @@ -54,6 +54,13 @@ triggers: description: When splitting completes on a card with the auto label, immediately chain to backlog manager defaultEnabled: false contextPipeline: [pipelineSnapshot] + - event: internal:external-webhook + label: External Webhook + description: >- + Dispatch this agent from an external system via an authenticated POST to + the project's per-agent webhook URL. The request body reaches the agent + as trigger context. + defaultEnabled: false # Required context — runs for EVERY invocation regardless of trigger source. # Manual `cascade runs trigger --agent-type backlog-manager` would otherwise diff --git a/src/agents/definitions/implementation.yaml b/src/agents/definitions/implementation.yaml index bcace71a7..dd5d2532d 100644 --- a/src/agents/definitions/implementation.yaml +++ b/src/agents/definitions/implementation.yaml @@ -58,6 +58,13 @@ triggers: options: [todo] defaultValue: todo contextPipeline: [directoryListing, contextFiles, workItem, prepopulateTodos] + - event: internal:external-webhook + label: External Webhook + description: >- + Dispatch this agent from an external system via an authenticated POST to + the project's per-agent webhook URL. The request body reaches the agent + as trigger context. + defaultEnabled: false strategies: {} diff --git a/src/agents/definitions/planning.yaml b/src/agents/definitions/planning.yaml index 8c37c505f..5e57ab851 100644 --- a/src/agents/definitions/planning.yaml +++ b/src/agents/definitions/planning.yaml @@ -55,6 +55,13 @@ triggers: options: [planning] defaultValue: planning contextPipeline: [directoryListing, contextFiles, workItem] + - event: internal:external-webhook + label: External Webhook + description: >- + Dispatch this agent from an external system via an authenticated POST to + the project's per-agent webhook URL. The request body reaches the agent + as trigger context. + defaultEnabled: false strategies: {} hooks: diff --git a/src/agents/definitions/resolve-conflicts.yaml b/src/agents/definitions/resolve-conflicts.yaml index ded107124..41f667e51 100644 --- a/src/agents/definitions/resolve-conflicts.yaml +++ b/src/agents/definitions/resolve-conflicts.yaml @@ -32,6 +32,13 @@ triggers: defaultEnabled: false providers: [github, gitlab] contextPipeline: [prContext, directoryListing, contextFiles, workItem] + - event: internal:external-webhook + label: External Webhook + description: >- + Dispatch this agent from an external system via an authenticated POST to + the project's per-agent webhook URL. The request body reaches the agent + as trigger context. + defaultEnabled: false strategies: {} diff --git a/src/agents/definitions/review.yaml b/src/agents/definitions/review.yaml index 849a88f04..34c82215d 100644 --- a/src/agents/definitions/review.yaml +++ b/src/agents/definitions/review.yaml @@ -57,6 +57,13 @@ triggers: options: [own, external, all] defaultValue: own contextPipeline: [prContext, contextFiles, workItem] + - event: internal:external-webhook + label: External Webhook + description: >- + Dispatch this agent from an external system via an authenticated POST to + the project's per-agent webhook URL. The request body reaches the agent + as trigger context. + defaultEnabled: false strategies: {} prompts: diff --git a/src/agents/definitions/splitting.yaml b/src/agents/definitions/splitting.yaml index 534363e2d..90403d928 100644 --- a/src/agents/definitions/splitting.yaml +++ b/src/agents/definitions/splitting.yaml @@ -56,6 +56,13 @@ triggers: options: [splitting] defaultValue: splitting contextPipeline: [directoryListing, contextFiles, workItem] + - event: internal:external-webhook + label: External Webhook + description: >- + Dispatch this agent from an external system via an authenticated POST to + the project's per-agent webhook URL. The request body reaches the agent + as trigger context. + defaultEnabled: false strategies: {} diff --git a/src/queue/client.ts b/src/queue/client.ts index 2bb8ea92e..6c6ab4d5d 100644 --- a/src/queue/client.ts +++ b/src/queue/client.ts @@ -27,6 +27,13 @@ export interface ManualRunJob { triggerCommentUrl?: string; triggerCommentPath?: string; triggerCommentAuthor?: string; + /** + * Trigger provenance for non-dashboard callers of the manual-run path + * (e.g. the router's external webhook endpoint). Defaults to 'manual'. + */ + triggerType?: 'manual' | 'external-webhook'; + /** Canonical trigger event (e.g. 'internal:external-webhook') for observability. */ + triggerEvent?: string; /** * MNG-1695: id of the `status='queued'` run row pre-created at tRPC trigger * time. The worker activates it (queued → running) instead of inserting a new diff --git a/src/router/external-webhook.ts b/src/router/external-webhook.ts new file mode 100644 index 000000000..556e7f69a --- /dev/null +++ b/src/router/external-webhook.ts @@ -0,0 +1,173 @@ +/** + * External webhook endpoint — dispatch an agent from any third-party system. + * + * POST /external/webhook/:projectId/:agentType + * Authorization: Bearer + * + * The password lives in project_credentials under + * EXTERNAL_WEBHOOK_PASSWORD_ (org-credential inheritance applies). + * Authentication FAILS CLOSED: no stored password → every request is rejected. + * The POST body (capped at 64 KiB) reaches the agent as trigger context via + * the manual-run path's triggerCommentBody. + * + * Deliberately hand-rolled rather than built on createWebhookHandler: that + * factory parses before verifying and treats a missing secret as "skip + * verification" (fail open) — both wrong for a token-authenticated endpoint. + * The logging discipline (webhook_logs row per decision) is mirrored instead. + * The Authorization header is never logged. + */ + +import { timingSafeEqual } from 'node:crypto'; +import type { Context, Handler } from 'hono'; +import { resolveEngineName } from '../backends/resolution.js'; +import { loadProjectConfigById } from '../config/provider.js'; +import { resolveProjectCredential } from '../db/repositories/credentialsRepository.js'; +import { createQueuedRun, failQueuedOrRunningRun } from '../db/repositories/runsRepository.js'; +import { submitDashboardJob } from '../queue/client.js'; +import { getResolvedTriggerConfig } from '../triggers/config-resolver.js'; +import { + EXTERNAL_WEBHOOK_EVENT, + externalWebhookCredentialKey, + isValidAgentTypeSlug, +} from '../triggers/shared/external-webhook.js'; +import { logger } from '../utils/logging.js'; +import { logWebhookCall } from '../utils/webhookLogger.js'; + +/** Kept below the 96 KiB JOB_DATA inline threshold (src/router/job-data-offload.ts). */ +export const EXTERNAL_WEBHOOK_BODY_MAX_BYTES = 64 * 1024; + +function timingSafeCompare(a: string, b: string): boolean { + const bufA = Buffer.from(a, 'utf8'); + const bufB = Buffer.from(b, 'utf8'); + if (bufA.length !== bufB.length) return false; + return timingSafeEqual(bufA, bufB); +} + +interface ReplyExtras { + runId?: string; + bodyRaw?: string; +} + +function reply( + c: Context, + status: 200 | 401 | 403 | 404 | 413 | 500, + decisionReason: string, + extras: ReplyExtras = {}, +) { + // Headers deliberately omitted from the log — the Authorization header + // carries the webhook password. Body is logged only on success. + logWebhookCall({ + source: 'external', + method: 'POST', + path: c.req.path, + bodyRaw: status === 200 ? extras.bodyRaw : undefined, + statusCode: status, + projectId: c.req.param('projectId'), + eventType: EXTERNAL_WEBHOOK_EVENT, + processed: status === 200, + decisionReason, + }); + if (status === 200) { + return c.json({ ok: true, runId: extras.runId }, 200); + } + return c.json({ error: decisionReason }, status); +} + +export function createExternalWebhookHandler(): Handler { + return async (c) => { + const projectId = c.req.param('projectId') ?? ''; + const agentType = c.req.param('agentType') ?? ''; + + // 1. Cheap slug guard before any DB hit + if (!projectId || !isValidAgentTypeSlug(agentType)) { + return reply(c, 404, 'Invalid agent type'); + } + + // 2. Project must exist (config also needed for engine resolution) + const pc = await loadProjectConfigById(projectId); + if (!pc) { + return reply(c, 404, 'Unknown project'); + } + + // 3. Resolve password — FAIL CLOSED when unset + const password = await resolveProjectCredential( + projectId, + externalWebhookCredentialKey(agentType), + ); + if (!password) { + return reply( + c, + 403, + 'No webhook password configured for this agent — rejecting (fail closed)', + ); + } + + // 4. Bearer parse + timing-safe compare + const auth = c.req.header('authorization') ?? ''; + const token = auth.startsWith('Bearer ') ? auth.slice('Bearer '.length) : ''; + if (!token || !timingSafeCompare(token, password)) { + return reply(c, 401, 'Invalid or missing bearer token'); + } + + // 5. Enablement — one resolver call covers: agent_configs row exists, + // definition declares the event, DB config / defaultEnabled. 404 for + // both undeclared and disabled (anti-probing); the decisionReason in + // webhook_logs distinguishes them for operators. + const triggerConfig = await getResolvedTriggerConfig( + projectId, + agentType, + EXTERNAL_WEBHOOK_EVENT, + ); + if (!triggerConfig) { + return reply(c, 404, 'Agent not enabled or external-webhook trigger not declared'); + } + if (!triggerConfig.enabled) { + return reply(c, 404, 'external-webhook trigger disabled for this agent'); + } + + // 6. Body read + cap (content-length pre-check, post-read check for chunked) + const declaredLength = Number(c.req.header('content-length') ?? 0); + if (declaredLength > EXTERNAL_WEBHOOK_BODY_MAX_BYTES) { + return reply(c, 413, 'Body too large'); + } + const body = await c.req.text(); + if (Buffer.byteLength(body, 'utf8') > EXTERNAL_WEBHOOK_BODY_MAX_BYTES) { + return reply(c, 413, 'Body too large'); + } + + // 7. Dispatch — exact manual-run pattern (src/api/routers/runs.ts:404-451). + // The router consumes cascade-dashboard-jobs itself and spawns the + // worker container; triggerManualRun re-checks enablement + integrations. + const engine = resolveEngineName(agentType, pc.project); + const runId = await createQueuedRun({ + projectId, + agentType, + engine, + triggerType: 'external-webhook', + }); + try { + await submitDashboardJob({ + type: 'manual-run', + projectId, + agentType, + triggerCommentBody: body.trim() ? body : undefined, + triggerType: 'external-webhook', + triggerEvent: EXTERNAL_WEBHOOK_EVENT, + runId, + }); + } catch (err) { + await failQueuedOrRunningRun(runId, 'Failed to enqueue external webhook run'); + logger.error('External webhook enqueue failed', { + projectId, + agentType, + error: String(err), + }); + return reply(c, 500, `Enqueue failed: ${String(err)}`); + } + + return reply(c, 200, `Job queued: ${agentType} agent (external webhook)`, { + runId, + bodyRaw: body || undefined, + }); + }; +} diff --git a/src/router/index.ts b/src/router/index.ts index 3465ac162..2fe3dce59 100644 --- a/src/router/index.ts +++ b/src/router/index.ts @@ -31,6 +31,7 @@ import { LinearRouterAdapter } from './adapters/linear.js'; import { SentryRouterAdapter } from './adapters/sentry.js'; import { TrelloRouterAdapter } from './adapters/trello.js'; import { startCancelListener, stopCancelListener } from './cancel-listener.js'; +import { createExternalWebhookHandler } from './external-webhook.js'; import { ROUTER_INSTANCE_ID } from './instance-id.js'; import { getQueueStats } from './queue.js'; import { processRouterWebhook } from './webhook-processor.js'; @@ -203,6 +204,11 @@ app.post( }), ); +// External webhook — dispatch an agent from any third-party system. +// Bearer-authenticated against the EXTERNAL_WEBHOOK_PASSWORD_ project +// credential; fails closed when no password is configured. See src/router/external-webhook.ts. +app.post('/external/webhook/:projectId/:agentType', createExternalWebhookHandler()); + // Linear webhook verification app.get('/linear/webhook', (c) => { return c.text('OK', 200); diff --git a/src/triggers/README.md b/src/triggers/README.md index 4c758e1e8..b0b26593f 100644 --- a/src/triggers/README.md +++ b/src/triggers/README.md @@ -109,7 +109,7 @@ Use `TRIGGER_EVENTS` from `src/triggers/shared/events.ts` for every new trigger | `PM` | `pm:status-changed`, `pm:label-added`, `pm:comment-mention` | | `SCM` | `scm:check-suite-success`, `scm:check-suite-failure`, `scm:pr-review-submitted`, `scm:review-requested`, `scm:pr-opened`, `scm:pr-comment-mention`, `scm:pr-merged`, `scm:pr-ready-to-merge`, `scm:pr-conflict-detected` | | `ALERTING` | `alerting:issue-alert`, `alerting:metric-alert` | -| `INTERNAL` | `internal:auto-chain` | +| `INTERNAL` | `internal:auto-chain`, `internal:external-webhook` | Do not introduce raw event-string literals in new handlers. If a handler checks `checkTriggerEnabled(..., event, ...)`, the same event must be emitted as `agentInput.triggerEvent`; `tests/unit/triggers/trigger-event-consistency.test.ts` enforces that invariant because mismatches make enabled triggers silently fall back to YAML defaults. diff --git a/src/triggers/shared/events.ts b/src/triggers/shared/events.ts index 043a4e688..911803756 100644 --- a/src/triggers/shared/events.ts +++ b/src/triggers/shared/events.ts @@ -22,6 +22,7 @@ export const TRIGGER_EVENTS = { }, INTERNAL: { AUTO_CHAIN: 'internal:auto-chain', + EXTERNAL_WEBHOOK: 'internal:external-webhook', }, } as const; diff --git a/src/triggers/shared/external-webhook.ts b/src/triggers/shared/external-webhook.ts new file mode 100644 index 000000000..f14dc2698 --- /dev/null +++ b/src/triggers/shared/external-webhook.ts @@ -0,0 +1,38 @@ +/** + * Shared helpers for the external webhook trigger (TRIGGER_EVENTS.INTERNAL.EXTERNAL_WEBHOOK). + * + * External systems dispatch an agent by POSTing to the router at + * `/external/webhook/:projectId/:agentType`, authenticated with a Bearer + * password stored as a project credential. This module is deliberately + * dependency-light: it is imported by the router endpoint AND by the web + * bundle (value-import-from-src precedent: web/src/lib/trigger-agent-mapping.ts). + */ + +import { TRIGGER_EVENTS } from './events.js'; + +export const EXTERNAL_WEBHOOK_EVENT = TRIGGER_EVENTS.INTERNAL.EXTERNAL_WEBHOOK; + +const AGENT_TYPE_SLUG_RE = /^[a-z][a-z0-9-]*$/; + +/** Agent type slugs are lowercase kebab identifiers (e.g. 'backlog-manager'). */ +export function isValidAgentTypeSlug(agentType: string): boolean { + return agentType.length <= 64 && AGENT_TYPE_SLUG_RE.test(agentType); +} + +/** + * The project_credentials env var key holding the webhook password for one + * agent type, e.g. 'implementation' → 'EXTERNAL_WEBHOOK_PASSWORD_IMPLEMENTATION', + * 'backlog-manager' → 'EXTERNAL_WEBHOOK_PASSWORD_BACKLOG_MANAGER'. + * Always matches the credential key pattern /^[A-Z_][A-Z0-9_]*$/. + */ +export function externalWebhookCredentialKey(agentType: string): string { + if (!isValidAgentTypeSlug(agentType)) { + throw new Error(`Invalid agent type slug: ${agentType}`); + } + return `EXTERNAL_WEBHOOK_PASSWORD_${agentType.toUpperCase().replace(/-/g, '_')}`; +} + +/** Router path for one project + agent's external webhook endpoint. */ +export function externalWebhookPath(projectId: string, agentType: string): string { + return `/external/webhook/${projectId}/${agentType}`; +} diff --git a/src/triggers/shared/manual-runner.ts b/src/triggers/shared/manual-runner.ts index 035562ac1..ea6746b23 100644 --- a/src/triggers/shared/manual-runner.ts +++ b/src/triggers/shared/manual-runner.ts @@ -77,6 +77,10 @@ export interface ManualTriggerInput { triggerCommentUrl?: string; triggerCommentPath?: string; triggerCommentAuthor?: string; + /** Trigger provenance override (router external webhook endpoint). Defaults to 'manual'. */ + triggerType?: 'manual' | 'external-webhook'; + /** Canonical trigger event (see TRIGGER_EVENTS in shared/events.ts). */ + triggerEvent?: string; /** * MNG-1695: id of a pre-created `status='queued'` run row. Rides the * agentInput to `executeWithEngine` → `tryCreateRun`, which activates it @@ -146,7 +150,8 @@ export async function triggerManualRun( repoFullName: input.repoFullName, headSha: input.headSha, modelOverride: input.modelOverride, - triggerType: 'manual', + triggerType: input.triggerType ?? 'manual', + triggerEvent: input.triggerEvent, triggerCommentBody: input.triggerCommentBody, triggerCommentId: input.triggerCommentId, triggerCommentUrl: input.triggerCommentUrl, diff --git a/src/types/index.ts b/src/types/index.ts index c61cae46e..ac97ae7cf 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -27,7 +27,8 @@ export interface AgentInput { | 'review-requested' | 'pr-opened' | 'conflict-resolution' - | 'manual'; + | 'manual' + | 'external-webhook'; /** YAML-format trigger event name for context pipeline resolution (e.g. 'scm:check-suite-success') */ triggerEvent?: CanonicalTriggerEvent | (string & {}); diff --git a/src/worker-entry.ts b/src/worker-entry.ts index 1199ebb6b..8c4d46e4f 100644 --- a/src/worker-entry.ts +++ b/src/worker-entry.ts @@ -170,6 +170,10 @@ export interface ManualRunJobData { triggerCommentUrl?: string; triggerCommentPath?: string; triggerCommentAuthor?: string; + /** Trigger provenance (router external webhook endpoint). Defaults to 'manual'. */ + triggerType?: 'manual' | 'external-webhook'; + /** Canonical trigger event (e.g. 'internal:external-webhook'). */ + triggerEvent?: string; /** MNG-1695: id of the pre-created `queued` run row to activate on boot. */ runId?: string; } @@ -228,6 +232,8 @@ export async function processDashboardJob(jobId: string, jobData: DashboardJobDa triggerCommentUrl: jobData.triggerCommentUrl, triggerCommentPath: jobData.triggerCommentPath, triggerCommentAuthor: jobData.triggerCommentAuthor, + triggerType: jobData.triggerType, + triggerEvent: jobData.triggerEvent, // MNG-1695: activate the pre-created queued row instead of inserting a new one. preCreatedRunId: jobData.runId, }, diff --git a/tests/unit/agents/definitions/external-webhook-trigger.test.ts b/tests/unit/agents/definitions/external-webhook-trigger.test.ts new file mode 100644 index 000000000..4aa33b5f8 --- /dev/null +++ b/tests/unit/agents/definitions/external-webhook-trigger.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest'; +import { + getBuiltinAgentTypes, + loadBuiltinDefinition, +} from '../../../../src/agents/definitions/loader.js'; +import { EXTERNAL_WEBHOOK_EVENT } from '../../../../src/triggers/shared/external-webhook.js'; + +const DECLARING_AGENTS = [ + 'implementation', + 'planning', + 'splitting', + 'backlog-manager', + 'review', + 'resolve-conflicts', + 'alerting', +] as const; + +describe('external webhook trigger declarations', () => { + for (const agentType of DECLARING_AGENTS) { + it(`${agentType} declares internal:external-webhook (opt-in, no params)`, () => { + const definition = loadBuiltinDefinition(agentType); + const trigger = definition.triggers?.find((t) => t.event === EXTERNAL_WEBHOOK_EVENT); + + expect(trigger).toBeDefined(); + expect(trigger?.defaultEnabled).toBe(false); + expect(trigger?.parameters ?? []).toEqual([]); + expect(trigger?.label).toBe('External Webhook'); + }); + } + + it('no other builtin agent declares it', () => { + const declaring = new Set(DECLARING_AGENTS); + for (const agentType of getBuiltinAgentTypes()) { + if (declaring.has(agentType)) continue; + const definition = loadBuiltinDefinition(agentType); + const trigger = definition.triggers?.find((t) => t.event === EXTERNAL_WEBHOOK_EVENT); + expect(trigger, `${agentType} should not declare external-webhook`).toBeUndefined(); + } + }); +}); diff --git a/tests/unit/router/external-webhook.test.ts b/tests/unit/router/external-webhook.test.ts new file mode 100644 index 000000000..270caf6ba --- /dev/null +++ b/tests/unit/router/external-webhook.test.ts @@ -0,0 +1,252 @@ +import { Hono } from 'hono'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Must mock heavy imports BEFORE importing the module under test +const { + mockLoadProjectConfigById, + mockResolveProjectCredential, + mockGetResolvedTriggerConfig, + mockCreateQueuedRun, + mockFailQueuedOrRunningRun, + mockSubmitDashboardJob, + mockResolveEngineName, + mockLogWebhookCall, +} = vi.hoisted(() => ({ + mockLoadProjectConfigById: vi.fn(), + mockResolveProjectCredential: vi.fn(), + mockGetResolvedTriggerConfig: vi.fn(), + mockCreateQueuedRun: vi.fn(), + mockFailQueuedOrRunningRun: vi.fn(), + mockSubmitDashboardJob: vi.fn(), + mockResolveEngineName: vi.fn(), + mockLogWebhookCall: vi.fn(), +})); + +vi.mock('../../../src/config/provider.js', () => ({ + loadProjectConfigById: mockLoadProjectConfigById, +})); +vi.mock('../../../src/db/repositories/credentialsRepository.js', () => ({ + resolveProjectCredential: mockResolveProjectCredential, +})); +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + getResolvedTriggerConfig: mockGetResolvedTriggerConfig, +})); +vi.mock('../../../src/db/repositories/runsRepository.js', () => ({ + createQueuedRun: mockCreateQueuedRun, + failQueuedOrRunningRun: mockFailQueuedOrRunningRun, +})); +vi.mock('../../../src/queue/client.js', () => ({ + submitDashboardJob: mockSubmitDashboardJob, +})); +vi.mock('../../../src/backends/resolution.js', () => ({ + resolveEngineName: mockResolveEngineName, +})); +vi.mock('../../../src/utils/webhookLogger.js', () => ({ + logWebhookCall: mockLogWebhookCall, +})); +vi.mock('../../../src/utils/logging.js', () => ({ + logger: { warn: vi.fn(), info: vi.fn(), debug: vi.fn(), error: vi.fn() }, +})); + +import { + createExternalWebhookHandler, + EXTERNAL_WEBHOOK_BODY_MAX_BYTES, +} from '../../../src/router/external-webhook.js'; + +function buildApp(): Hono { + const app = new Hono(); + app.post('/external/webhook/:projectId/:agentType', createExternalWebhookHandler()); + return app; +} + +function post( + app: Hono, + { + projectId = 'proj-1', + agentType = 'implementation', + body = '{"message":"do the thing"}', + headers = {} as Record, + } = {}, +): Promise { + return app.fetch( + new Request(`http://localhost/external/webhook/${projectId}/${agentType}`, { + method: 'POST', + headers, + body, + }), + ); +} + +const bearer = (password: string) => ({ Authorization: `Bearer ${password}` }); + +describe('external webhook endpoint', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockLoadProjectConfigById.mockResolvedValue({ project: { id: 'proj-1' }, config: {} }); + mockResolveProjectCredential.mockResolvedValue('correct-password'); + mockGetResolvedTriggerConfig.mockResolvedValue({ enabled: true, parameters: {} }); + mockResolveEngineName.mockReturnValue('claude-code'); + mockCreateQueuedRun.mockResolvedValue('run-123'); + mockSubmitDashboardJob.mockResolvedValue(undefined); + }); + + describe('fail closed', () => { + it('rejects with 403 when no password is configured — never dispatches', async () => { + mockResolveProjectCredential.mockResolvedValue(null); + + const res = await post(buildApp(), { headers: bearer('anything') }); + + expect(res.status).toBe(403); + expect(mockSubmitDashboardJob).not.toHaveBeenCalled(); + expect(mockCreateQueuedRun).not.toHaveBeenCalled(); + }); + }); + + describe('authentication', () => { + it('rejects missing Authorization header with 401', async () => { + const res = await post(buildApp()); + expect(res.status).toBe(401); + }); + + it('rejects non-Bearer scheme with 401', async () => { + const res = await post(buildApp(), { + headers: { Authorization: 'Basic correct-password' }, + }); + expect(res.status).toBe(401); + }); + + it('rejects wrong password with 401', async () => { + const res = await post(buildApp(), { headers: bearer('wrong-password!') }); + expect(res.status).toBe(401); + expect(mockSubmitDashboardJob).not.toHaveBeenCalled(); + }); + + it('rejects wrong-length password with 401', async () => { + const res = await post(buildApp(), { headers: bearer('short') }); + expect(res.status).toBe(401); + }); + + it('never passes the Authorization header to webhook logs', async () => { + await post(buildApp(), { headers: bearer('correct-password') }); + + for (const call of mockLogWebhookCall.mock.calls) { + const input = call[0] as Record; + expect(input.headers).toBeUndefined(); + expect(JSON.stringify(input)).not.toContain('correct-password'); + } + }); + }); + + describe('routing guards', () => { + it('returns 404 for an invalid agent type slug', async () => { + const res = await post(buildApp(), { agentType: 'Not-Valid!' }); + expect(res.status).toBe(404); + expect(mockLoadProjectConfigById).not.toHaveBeenCalled(); + }); + + it('returns 404 for an unknown project', async () => { + mockLoadProjectConfigById.mockResolvedValue(undefined); + const res = await post(buildApp(), { headers: bearer('correct-password') }); + expect(res.status).toBe(404); + }); + + it('returns 404 when the agent is not enabled or trigger undeclared', async () => { + mockGetResolvedTriggerConfig.mockResolvedValue(null); + const res = await post(buildApp(), { headers: bearer('correct-password') }); + expect(res.status).toBe(404); + expect(mockSubmitDashboardJob).not.toHaveBeenCalled(); + }); + + it('returns 404 when the trigger is declared but disabled — distinct decision reason', async () => { + mockGetResolvedTriggerConfig.mockResolvedValue({ enabled: false, parameters: {} }); + const res = await post(buildApp(), { headers: bearer('correct-password') }); + expect(res.status).toBe(404); + + const reasons = mockLogWebhookCall.mock.calls.map( + (call) => (call[0] as { decisionReason?: string }).decisionReason, + ); + expect(reasons.some((r) => r?.includes('disabled'))).toBe(true); + }); + }); + + describe('body handling', () => { + it('rejects oversized declared content-length with 413', async () => { + const res = await post(buildApp(), { + headers: { + ...bearer('correct-password'), + 'content-length': String(EXTERNAL_WEBHOOK_BODY_MAX_BYTES + 1), + }, + }); + expect(res.status).toBe(413); + }); + + it('rejects oversized actual body with 413', async () => { + const res = await post(buildApp(), { + headers: bearer('correct-password'), + body: 'x'.repeat(EXTERNAL_WEBHOOK_BODY_MAX_BYTES + 1), + }); + expect(res.status).toBe(413); + expect(mockCreateQueuedRun).not.toHaveBeenCalled(); + }); + + it('passes undefined triggerCommentBody for an empty body', async () => { + const res = await post(buildApp(), { headers: bearer('correct-password'), body: '' }); + expect(res.status).toBe(200); + expect(mockSubmitDashboardJob).toHaveBeenCalledWith( + expect.objectContaining({ triggerCommentBody: undefined }), + ); + }); + }); + + describe('dispatch', () => { + it('queues a run and returns 200 with the runId', async () => { + const res = await post(buildApp(), { headers: bearer('correct-password') }); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true, runId: 'run-123' }); + + expect(mockCreateQueuedRun).toHaveBeenCalledWith({ + projectId: 'proj-1', + agentType: 'implementation', + engine: 'claude-code', + triggerType: 'external-webhook', + }); + expect(mockSubmitDashboardJob).toHaveBeenCalledWith({ + type: 'manual-run', + projectId: 'proj-1', + agentType: 'implementation', + triggerCommentBody: '{"message":"do the thing"}', + triggerType: 'external-webhook', + triggerEvent: 'internal:external-webhook', + runId: 'run-123', + }); + }); + + it('fails the pre-created run and returns 500 when enqueue throws', async () => { + mockSubmitDashboardJob.mockRejectedValue(new Error('redis down')); + + const res = await post(buildApp(), { headers: bearer('correct-password') }); + + expect(res.status).toBe(500); + expect(mockFailQueuedOrRunningRun).toHaveBeenCalledWith( + 'run-123', + 'Failed to enqueue external webhook run', + ); + }); + + it('logs a processed webhook row with the body on success', async () => { + await post(buildApp(), { headers: bearer('correct-password') }); + + const successLog = mockLogWebhookCall.mock.calls + .map((call) => call[0] as Record) + .find((input) => input.statusCode === 200); + expect(successLog).toMatchObject({ + source: 'external', + processed: true, + projectId: 'proj-1', + eventType: 'internal:external-webhook', + bodyRaw: '{"message":"do the thing"}', + }); + }); + }); +}); diff --git a/tests/unit/triggers/external-webhook-helper.test.ts b/tests/unit/triggers/external-webhook-helper.test.ts new file mode 100644 index 000000000..5706e15b7 --- /dev/null +++ b/tests/unit/triggers/external-webhook-helper.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from 'vitest'; +import { TRIGGER_EVENTS } from '../../../src/triggers/shared/events.js'; +import { + EXTERNAL_WEBHOOK_EVENT, + externalWebhookCredentialKey, + externalWebhookPath, + isValidAgentTypeSlug, +} from '../../../src/triggers/shared/external-webhook.js'; + +const CREDENTIAL_KEY_RE = /^[A-Z_][A-Z0-9_]*$/; + +describe('external webhook helpers', () => { + it('event constant matches the canonical catalog', () => { + expect(EXTERNAL_WEBHOOK_EVENT).toBe(TRIGGER_EVENTS.INTERNAL.EXTERNAL_WEBHOOK); + }); + + describe('externalWebhookCredentialKey', () => { + it('maps simple agent types', () => { + expect(externalWebhookCredentialKey('implementation')).toBe( + 'EXTERNAL_WEBHOOK_PASSWORD_IMPLEMENTATION', + ); + }); + + it('maps kebab-case agent types with underscores', () => { + expect(externalWebhookCredentialKey('backlog-manager')).toBe( + 'EXTERNAL_WEBHOOK_PASSWORD_BACKLOG_MANAGER', + ); + expect(externalWebhookCredentialKey('resolve-conflicts')).toBe( + 'EXTERNAL_WEBHOOK_PASSWORD_RESOLVE_CONFLICTS', + ); + }); + + it('always produces a valid project credential key', () => { + for (const agent of [ + 'implementation', + 'planning', + 'splitting', + 'backlog-manager', + 'review', + 'resolve-conflicts', + 'alerting', + ]) { + expect(externalWebhookCredentialKey(agent)).toMatch(CREDENTIAL_KEY_RE); + } + }); + + it('throws on invalid slugs', () => { + expect(() => externalWebhookCredentialKey('Not-Valid')).toThrow(); + expect(() => externalWebhookCredentialKey('../etc')).toThrow(); + expect(() => externalWebhookCredentialKey('')).toThrow(); + }); + }); + + describe('isValidAgentTypeSlug', () => { + it('accepts lowercase kebab slugs', () => { + expect(isValidAgentTypeSlug('implementation')).toBe(true); + expect(isValidAgentTypeSlug('backlog-manager')).toBe(true); + }); + + it('rejects uppercase, traversal, empty, digit-leading, and overlong values', () => { + expect(isValidAgentTypeSlug('Implementation')).toBe(false); + expect(isValidAgentTypeSlug('../etc/passwd')).toBe(false); + expect(isValidAgentTypeSlug('')).toBe(false); + expect(isValidAgentTypeSlug('1agent')).toBe(false); + expect(isValidAgentTypeSlug('a'.repeat(65))).toBe(false); + }); + }); + + it('builds the router path', () => { + expect(externalWebhookPath('proj-1', 'implementation')).toBe( + '/external/webhook/proj-1/implementation', + ); + }); +}); diff --git a/tests/unit/triggers/shared/events.test.ts b/tests/unit/triggers/shared/events.test.ts index 70130e056..f7a2d7d6c 100644 --- a/tests/unit/triggers/shared/events.test.ts +++ b/tests/unit/triggers/shared/events.test.ts @@ -27,6 +27,7 @@ describe('TRIGGER_EVENTS', () => { 'alerting:metric-alert', 'alerting:issue-lifecycle', 'internal:auto-chain', + 'internal:external-webhook', ]); }); diff --git a/web/src/components/projects/agent-config-detail.tsx b/web/src/components/projects/agent-config-detail.tsx index 9e192bb9b..bb4b9a3d4 100644 --- a/web/src/components/projects/agent-config-detail.tsx +++ b/web/src/components/projects/agent-config-detail.tsx @@ -21,8 +21,10 @@ import { } from '@/components/ui/select.js'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs.js'; import { AGENT_LABELS, CATEGORY_LABELS } from '@/lib/trigger-agent-mapping.js'; +import { EXTERNAL_WEBHOOK_EVENT } from '../../../../src/triggers/shared/external-webhook.js'; import type { AgentDetailViewProps, DefinitionAgentSectionProps } from './agent-config-types.js'; import { AgentPromptOverrides } from './agent-prompt-overrides.js'; +import { ExternalWebhookTriggerConfig } from './external-webhook-trigger-config.js'; // biome-ignore lint/complexity/noExcessiveCognitiveComplexity: tabbed detail panel managing Engine/Prompts/Triggers tabs with per-tab state, mutations, and trigger category grouping function DefinitionAgentSection({ @@ -350,6 +352,15 @@ function DefinitionAgentSection({ onTriggerParamChange(agentType, event, params, currentTrigger?.enabled ?? true); }} idPrefix={`${agentType}-${category}`} + renderTriggerExtra={(trigger) => + trigger.event === EXTERNAL_WEBHOOK_EVENT ? ( + + ) : null + } /> ); diff --git a/web/src/components/projects/external-webhook-trigger-config.tsx b/web/src/components/projects/external-webhook-trigger-config.tsx new file mode 100644 index 000000000..244b0b0bb --- /dev/null +++ b/web/src/components/projects/external-webhook-trigger-config.tsx @@ -0,0 +1,97 @@ +/** + * Inline configuration block for the external webhook trigger, rendered under + * the trigger row in the agent's Triggers tab when the trigger is enabled. + * Shows the per-project-per-agent webhook URL (copy button + curl example) + * and the password field (a project credential — org inheritance applies). + */ + +import { useQuery } from '@tanstack/react-query'; +import { AlertTriangle } from 'lucide-react'; +import { ProjectSecretField } from '@/components/projects/project-secret-field.js'; +import { CopyButton } from '@/components/ui/copy-button.js'; +import { Label } from '@/components/ui/label.js'; +import { API_URL } from '@/lib/api.js'; +import { trpc } from '@/lib/trpc.js'; +import { + externalWebhookCredentialKey, + externalWebhookPath, +} from '../../../../src/triggers/shared/external-webhook.js'; + +export function ExternalWebhookTriggerConfig({ + projectId, + agentType, + enabled, +}: { + projectId: string; + agentType: string; + enabled: boolean; +}) { + const publicUrlQuery = useQuery({ + ...trpc.system.getPublicUrl.queryOptions(), + enabled, + }); + const credentialsQuery = useQuery({ + ...trpc.projects.credentials.list.queryOptions({ projectId }), + enabled, + }); + + if (!enabled) return null; + + const callbackBaseUrl = + publicUrlQuery.data?.routerPublicUrl ?? + API_URL ?? + (typeof window !== 'undefined' ? window.location.origin.replace(':5173', ':3000') : ''); + const webhookUrl = `${callbackBaseUrl || ''}${externalWebhookPath(projectId, agentType)}`; + + const credentialKey = externalWebhookCredentialKey(agentType); + const credential = credentialsQuery.data?.find((c) => c.envVarKey === credentialKey); + + const curlExample = [ + `curl -X POST '${webhookUrl}' \\`, + ` -H 'Authorization: Bearer ' \\`, + ` -H 'Content-Type: application/json' \\`, + ` -d '{"message": "Describe what the agent should do"}'`, + ].join('\n'); + + return ( +
+
+ +

+ POST to this URL to dispatch the {agentType} agent. The request body reaches the agent as + trigger context. +

+
+ {webhookUrl} + +
+
+ +
+ + Example request + +
+ 
{curlExample}
+ +
+
+ + {!credential?.isConfigured && ( +
+ + Requests are rejected with 403 until a password is set. +
+ )} + + +
+ ); +} diff --git a/web/src/components/shared/definition-trigger-toggles.tsx b/web/src/components/shared/definition-trigger-toggles.tsx index 0908259bf..7d38539bb 100644 --- a/web/src/components/shared/definition-trigger-toggles.tsx +++ b/web/src/components/shared/definition-trigger-toggles.tsx @@ -1,3 +1,4 @@ +import type { ReactNode } from 'react'; import { Badge } from '@/components/ui/badge.js'; import { TriggerParameterInput } from './trigger-parameter-input.js'; @@ -15,6 +16,8 @@ interface Props { onParamChange: (event: string, parameters: Record) => void; idPrefix?: string; disabled?: boolean; + /** Optional extra UI rendered below a trigger's params block (e.g. webhook URL + password). */ + renderTriggerExtra?: (trigger: ResolvedTrigger) => ReactNode; } /** @@ -27,6 +30,7 @@ export function DefinitionTriggerToggles({ onParamChange, idPrefix, disabled, + renderTriggerExtra, }: Props) { if (triggers.length === 0) return null; @@ -86,6 +90,8 @@ export function DefinitionTriggerToggles({ ))} )} + + {renderTriggerExtra?.(trigger)} ); })} From 212c169731137769c761a71fd9582127a40be590 Mon Sep 17 00:00:00 2001 From: Wojtek Siudzinski Date: Sun, 2 Aug 2026 21:30:39 +0200 Subject: [PATCH 2/2] fix(triggers): harden external webhook per adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all confirmed review findings plus one verified manually: - CRITICAL: the POST body never reached the 7 declaring agents' prompts (only respond-to-* task prompts render commentBody). New appendExternalTriggerRequest appends the payload to the rendered task prompt centrally, gated on triggerType === 'external-webhook'. - HIGH: EXTERNAL_WEBHOOK_PASSWORD_* credentials are inbound-auth verifiers and are now excluded from worker container env injection — a prompt-injected agent could otherwise read the password and gain a self-re-dispatch primitive (org-level passwords escalated cross-project). - Brute-force resistance: failed attempts are rate-limited per client IP via the shared sliding-window limiter (429 + Retry-After; successful auth resets the counter), and webhook passwords now require >=16 chars at both credential write paths (project + org). - Anti-enumeration: unknown project, unset password, and wrong token all return an identical generic 401; distinct decision reasons live only in webhook_logs. - Bearer scheme matched case-insensitively (RFC 7235). - Body read incrementally with the 64 KiB cap instead of buffering first. - Enqueue failures return a generic message; detail stays in logs. - internal:external-webhook added to TRIGGER_REGISTRY (definition editor). - Webhook URL display no longer falls back to the dashboard API_URL (wrong service) — router URL comes from WEBHOOK_CALLBACK_BASE_URL, with a dev-only origin swap, else an explicit placeholder. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 2 +- src/agents/definitions/profiles.ts | 6 +- src/agents/prompts/index.ts | 31 +++++ src/api/routers/_shared/triggerTypes.ts | 7 + .../routers/_shared/webhookPasswordPolicy.ts | 20 +++ src/api/routers/organization.ts | 2 + src/api/routers/projects.ts | 2 + src/router/external-webhook.ts | 120 +++++++++++++---- src/router/worker-env.ts | 11 +- src/triggers/shared/external-webhook.ts | 13 ++ .../unit/agents/definitions/profiles.test.ts | 2 + .../prompts/external-trigger-request.test.ts | 32 +++++ .../_shared/webhookPasswordPolicy.test.ts | 27 ++++ tests/unit/router/external-webhook.test.ts | 126 +++++++++++++----- tests/unit/router/worker-env.test.ts | 14 ++ .../external-webhook-trigger-config.tsx | 19 +-- 16 files changed, 368 insertions(+), 66 deletions(-) create mode 100644 src/api/routers/_shared/webhookPasswordPolicy.ts create mode 100644 tests/unit/agents/prompts/external-trigger-request.test.ts create mode 100644 tests/unit/api/routers/_shared/webhookPasswordPolicy.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 5b2bc7f0f..8e712bb8c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,7 +134,7 @@ cascade projects trigger-set --agent --event --enabl Some triggers take params (e.g. `review` + `scm:check-suite-success` accepts `{"authorMode":"own"|"external"}`). Legacy configs on `project_integrations.triggers` are auto-migrated on merge to `dev`/`main`. -**External webhook trigger** — `internal:external-webhook` lets any third-party system dispatch an agent via `POST {router}/external/webhook/:projectId/:agentType` with `Authorization: Bearer `. The password is the `EXTERNAL_WEBHOOK_PASSWORD_` project credential (org inheritance applies); authentication **fails closed** — no stored password means every request is rejected with 403. The POST body (≤64 KiB) reaches the agent as trigger context (`triggerCommentBody`), and dispatch rides the existing manual-run dashboard-job path (`createQueuedRun` + `submitDashboardJob`), so runs appear in the dashboard as trigger `external-webhook`. Enable per agent in the project's Agents → Triggers tab, which displays the URL and password field (`src/router/external-webhook.ts`, helpers at `src/triggers/shared/external-webhook.ts`). +**External webhook trigger** — `internal:external-webhook` lets any third-party system dispatch an agent via `POST {router}/external/webhook/:projectId/:agentType` with `Authorization: Bearer `. The password is the `EXTERNAL_WEBHOOK_PASSWORD_` project credential (org inheritance applies; server enforces ≥16 chars); authentication **fails closed** — unknown project, unset password, and wrong token all return an identical generic 401 (anti-enumeration; distinct decision reasons only in `webhook_logs`), and failed attempts are IP-rate-limited via the shared sliding-window limiter. The POST body (≤64 KiB, read with an incremental cap) reaches the agent as its work request — `appendExternalTriggerRequest` appends it to the rendered task prompt centrally, gated on `triggerType === 'external-webhook'`. Dispatch rides the existing manual-run dashboard-job path (`createQueuedRun` + `submitDashboardJob`), so runs appear as trigger `external-webhook`. **Security invariant:** `EXTERNAL_WEBHOOK_PASSWORD_*` keys are inbound-auth verifiers and are excluded from worker container env injection (`src/router/worker-env.ts`) — an agent must never see them. Enable per agent in the project's Agents → Triggers tab, which displays the URL and password field (`src/router/external-webhook.ts`, helpers at `src/triggers/shared/external-webhook.ts`). **Work-item concurrency lock** — the router prevents duplicate agent runs via a per-agent-type lock on `(projectId, workItemId, agentType)`. Only same-type duplicates are blocked; **different agent types can run concurrently** on the same work item (e.g. review starts while implementation's container is still cleaning up). The lock has a 30-minute TTL hard ceiling that auto-clears stale entries after router restart. diff --git a/src/agents/definitions/profiles.ts b/src/agents/definitions/profiles.ts index 009b73dfe..35d482e19 100644 --- a/src/agents/definitions/profiles.ts +++ b/src/agents/definitions/profiles.ts @@ -15,6 +15,7 @@ import { } from '../capabilities/resolver.js'; import type { ContextInjection, ToolManifest } from '../contracts/index.js'; import { + appendExternalTriggerRequest, buildTaskPromptContext, renderInlineTaskPrompt, validateTemplate, @@ -255,7 +256,10 @@ function buildProfileFromDefinition(def: AgentDefinition, agentType: string): Ag return injections; }, buildTaskPrompt: (input) => - renderInlineTaskPrompt(taskPromptTemplate, buildTaskPromptContext(input)), + appendExternalTriggerRequest( + renderInlineTaskPrompt(taskPromptTemplate, buildTaskPromptContext(input)), + input, + ), capabilities: def.capabilities, getLlmistGadgets: (integrationChecker?: IntegrationChecker) => { // Resolve effective capabilities based on integration availability diff --git a/src/agents/prompts/index.ts b/src/agents/prompts/index.ts index 3e0be9d1d..758ca19e5 100644 --- a/src/agents/prompts/index.ts +++ b/src/agents/prompts/index.ts @@ -245,6 +245,37 @@ export function buildTaskPromptContext(input: TaskPromptInput): TaskPromptContex }; } +/** + * Append the external webhook request payload to a rendered task prompt. + * + * The external webhook trigger's core promise is that the POST body reaches + * the agent as its work request — but only the respond-to-* task prompts + * render the commentBody template variable. Rather than editing every agent's + * task prompt, this appends a standard section centrally, gated on the + * external-webhook trigger type so no other dispatch path is affected. + */ +export function appendExternalTriggerRequest( + renderedTaskPrompt: string, + input: { triggerType?: string; triggerCommentBody?: string }, +): string { + if (input.triggerType !== 'external-webhook' || !input.triggerCommentBody) { + return renderedTaskPrompt; + } + return [ + renderedTaskPrompt, + '', + '## External trigger request', + '', + 'This run was dispatched by an external system via webhook. The request', + 'payload below describes what this run should accomplish — treat it as', + 'the work request:', + '', + '', + input.triggerCommentBody, + '', + ].join('\n'); +} + /** * Render an inline task prompt template with Eta variable interpolation. * Used for task prompts stored directly in agent definitions (prompts.taskPrompt). diff --git a/src/api/routers/_shared/triggerTypes.ts b/src/api/routers/_shared/triggerTypes.ts index dbf2ce1dc..1f6162c47 100644 --- a/src/api/routers/_shared/triggerTypes.ts +++ b/src/api/routers/_shared/triggerTypes.ts @@ -234,5 +234,12 @@ export const TRIGGER_REGISTRY: Record = { description: 'Orchestration trigger for chaining agents after completion', contextPipeline: [], }, + { + event: 'internal:external-webhook', + label: 'External Webhook', + description: + 'Dispatch from an external system via an authenticated POST to the per-agent webhook URL', + contextPipeline: [], + }, ], }; diff --git a/src/api/routers/_shared/webhookPasswordPolicy.ts b/src/api/routers/_shared/webhookPasswordPolicy.ts new file mode 100644 index 000000000..cfc722518 --- /dev/null +++ b/src/api/routers/_shared/webhookPasswordPolicy.ts @@ -0,0 +1,20 @@ +import { TRPCError } from '@trpc/server'; +import { isExternalWebhookPasswordKey } from '../../../triggers/shared/external-webhook.js'; + +export const EXTERNAL_WEBHOOK_PASSWORD_MIN_LENGTH = 16; + +/** + * External webhook passwords authenticate an unauthenticated internet-facing + * endpoint that dispatches agents — a guessable password is remote agent + * execution. Enforce a minimum length at every write path (project + org + * credential mutations). Other credential keys are unaffected. + */ +export function assertWebhookPasswordStrength(envVarKey: string, value: string): void { + if (!isExternalWebhookPasswordKey(envVarKey)) return; + if (value.length < EXTERNAL_WEBHOOK_PASSWORD_MIN_LENGTH) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: `Webhook passwords must be at least ${EXTERNAL_WEBHOOK_PASSWORD_MIN_LENGTH} characters`, + }); + } +} diff --git a/src/api/routers/organization.ts b/src/api/routers/organization.ts index 7edd15d4c..f25f438d1 100644 --- a/src/api/routers/organization.ts +++ b/src/api/routers/organization.ts @@ -15,6 +15,7 @@ import { captureException } from '../../sentry.js'; import { adminProcedure, protectedProcedure, router, superAdminProcedure } from '../trpc.js'; import { maskCredentialValue } from './_shared/maskCredential.js'; import { assertOrgAdmin, resolveActorRole } from './_shared/orgRole.js'; +import { assertWebhookPasswordStrength } from './_shared/webhookPasswordPolicy.js'; export const organizationRouter = router({ get: protectedProcedure.query(async ({ ctx }) => { @@ -99,6 +100,7 @@ export const organizationRouter = router({ ) .mutation(async ({ ctx, input }) => { assertOrgAdmin(await resolveActorRole(ctx)); + assertWebhookPasswordStrength(input.envVarKey, input.value); await writeOrgCredential( ctx.effectiveOrgId, input.envVarKey, diff --git a/src/api/routers/projects.ts b/src/api/routers/projects.ts index 0de784f67..55cacb45b 100644 --- a/src/api/routers/projects.ts +++ b/src/api/routers/projects.ts @@ -42,6 +42,7 @@ import { captureException } from '../../sentry.js'; import { logger } from '../../utils/logging.js'; import { protectedProcedure, publicProcedure, router, superAdminProcedure } from '../trpc.js'; import { maskCredentialValue } from './_shared/maskCredential.js'; +import { assertWebhookPasswordStrength } from './_shared/webhookPasswordPolicy.js'; /** * The current worker-image/dockerfile state read alongside the ownership check. @@ -921,6 +922,7 @@ export const projectsRouter = router({ ) .mutation(async ({ ctx, input }) => { await verifyProjectOwnership(input.projectId, ctx.effectiveOrgId); + assertWebhookPasswordStrength(input.envVarKey, input.value); await writeProjectCredential( input.projectId, input.envVarKey, diff --git a/src/router/external-webhook.ts b/src/router/external-webhook.ts index 556e7f69a..44b010418 100644 --- a/src/router/external-webhook.ts +++ b/src/router/external-webhook.ts @@ -10,15 +10,26 @@ * The POST body (capped at 64 KiB) reaches the agent as trigger context via * the manual-run path's triggerCommentBody. * + * Hardening notes (adversarial review, 2026-08-02): + * - All pre-auth failures (unknown project, no password configured, wrong + * token) return an identical generic 401 so unauthenticated callers cannot + * enumerate projects or distinguish password-armed agents. The distinct + * decisionReasons live only in webhook_logs (superadmin surface). + * - Failed attempts are rate-limited per client IP via the shared sliding + * window limiter; successful auth resets the counter. + * - The Bearer scheme is matched case-insensitively (RFC 7235). + * - The body is read incrementally and aborted at the cap, not buffered first. + * - Error responses never include internal error details; those go to logs. + * - The Authorization header is never logged. + * * Deliberately hand-rolled rather than built on createWebhookHandler: that * factory parses before verifying and treats a missing secret as "skip * verification" (fail open) — both wrong for a token-authenticated endpoint. - * The logging discipline (webhook_logs row per decision) is mirrored instead. - * The Authorization header is never logged. */ import { timingSafeEqual } from 'node:crypto'; import type { Context, Handler } from 'hono'; +import { checkRateLimit, recordSuccessfulLogin } from '../api/auth/rateLimiter.js'; import { resolveEngineName } from '../backends/resolution.js'; import { loadProjectConfigById } from '../config/provider.js'; import { resolveProjectCredential } from '../db/repositories/credentialsRepository.js'; @@ -36,6 +47,8 @@ import { logWebhookCall } from '../utils/webhookLogger.js'; /** Kept below the 96 KiB JOB_DATA inline threshold (src/router/job-data-offload.ts). */ export const EXTERNAL_WEBHOOK_BODY_MAX_BYTES = 64 * 1024; +const UNAUTHORIZED_MESSAGE = 'Unauthorized'; + function timingSafeCompare(a: string, b: string): boolean { const bufA = Buffer.from(a, 'utf8'); const bufB = Buffer.from(b, 'utf8'); @@ -43,14 +56,59 @@ function timingSafeCompare(a: string, b: string): boolean { return timingSafeEqual(bufA, bufB); } +/** RFC 7235: the auth scheme token is case-insensitive. */ +function parseBearerToken(authorizationHeader: string): string { + const match = /^bearer\s+(.+)$/i.exec(authorizationHeader); + return match?.[1] ?? ''; +} + +function getClientIp(c: Context): string { + const forwarded = c.req.header('x-forwarded-for'); + if (forwarded) { + return forwarded.split(',')[0].trim(); + } + return 'unknown'; +} + +/** + * Read the request body incrementally, aborting as soon as the byte count + * exceeds the cap — an oversized or endless body never gets fully buffered. + * Returns null when the cap was exceeded. + */ +async function readBodyWithCap(c: Context, maxBytes: number): Promise { + const stream = c.req.raw.body; + if (!stream) return ''; + + const reader = stream.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + total += value.byteLength; + if (total > maxBytes) { + await reader.cancel(); + return null; + } + chunks.push(value); + } + } finally { + reader.releaseLock(); + } + return Buffer.concat(chunks).toString('utf8'); +} + interface ReplyExtras { runId?: string; bodyRaw?: string; + /** Override the response body message (responses default to decisionReason). */ + publicMessage?: string; } function reply( c: Context, - status: 200 | 401 | 403 | 404 | 413 | 500, + status: 200 | 401 | 404 | 413 | 429 | 500, decisionReason: string, extras: ReplyExtras = {}, ) { @@ -70,7 +128,12 @@ function reply( if (status === 200) { return c.json({ ok: true, runId: extras.runId }, 200); } - return c.json({ error: decisionReason }, status); + return c.json({ error: extras.publicMessage ?? decisionReason }, status); +} + +/** Identical public 401 for every pre-auth failure — anti-enumeration. */ +function unauthorized(c: Context, decisionReason: string) { + return reply(c, 401, decisionReason, { publicMessage: UNAUTHORIZED_MESSAGE }); } export function createExternalWebhookHandler(): Handler { @@ -78,41 +141,50 @@ export function createExternalWebhookHandler(): Handler { const projectId = c.req.param('projectId') ?? ''; const agentType = c.req.param('agentType') ?? ''; - // 1. Cheap slug guard before any DB hit + // 0. Per-IP sliding-window rate limit — every attempt counts until a + // successful authentication resets the counter. + const rateKey = `external-webhook:${getClientIp(c)}`; + const rateCheck = checkRateLimit(rateKey); + if (rateCheck.limited) { + c.header('Retry-After', String(rateCheck.retryAfterSeconds)); + return reply(c, 429, 'Rate limited', { + publicMessage: 'Too many attempts. Please try again later.', + }); + } + + // 1. Cheap slug guard before any DB hit (format error — no state info) if (!projectId || !isValidAgentTypeSlug(agentType)) { return reply(c, 404, 'Invalid agent type'); } - // 2. Project must exist (config also needed for engine resolution) + // 2-4. Authentication. Unknown project, missing password (fail closed), + // and wrong token all collapse into the same generic 401; the + // decisionReason in webhook_logs distinguishes them for operators. const pc = await loadProjectConfigById(projectId); if (!pc) { - return reply(c, 404, 'Unknown project'); + return unauthorized(c, 'Unknown project'); } - // 3. Resolve password — FAIL CLOSED when unset const password = await resolveProjectCredential( projectId, externalWebhookCredentialKey(agentType), ); if (!password) { - return reply( + return unauthorized( c, - 403, 'No webhook password configured for this agent — rejecting (fail closed)', ); } - // 4. Bearer parse + timing-safe compare - const auth = c.req.header('authorization') ?? ''; - const token = auth.startsWith('Bearer ') ? auth.slice('Bearer '.length) : ''; + const token = parseBearerToken(c.req.header('authorization') ?? ''); if (!token || !timingSafeCompare(token, password)) { - return reply(c, 401, 'Invalid or missing bearer token'); + return unauthorized(c, 'Invalid or missing bearer token'); } + recordSuccessfulLogin(rateKey); // 5. Enablement — one resolver call covers: agent_configs row exists, - // definition declares the event, DB config / defaultEnabled. 404 for - // both undeclared and disabled (anti-probing); the decisionReason in - // webhook_logs distinguishes them for operators. + // definition declares the event, DB config / defaultEnabled. The + // caller is authenticated at this point, so a specific 404 is fine. const triggerConfig = await getResolvedTriggerConfig( projectId, agentType, @@ -125,13 +197,9 @@ export function createExternalWebhookHandler(): Handler { return reply(c, 404, 'external-webhook trigger disabled for this agent'); } - // 6. Body read + cap (content-length pre-check, post-read check for chunked) - const declaredLength = Number(c.req.header('content-length') ?? 0); - if (declaredLength > EXTERNAL_WEBHOOK_BODY_MAX_BYTES) { - return reply(c, 413, 'Body too large'); - } - const body = await c.req.text(); - if (Buffer.byteLength(body, 'utf8') > EXTERNAL_WEBHOOK_BODY_MAX_BYTES) { + // 6. Body read with incremental cap (never fully buffers an oversized body) + const body = await readBodyWithCap(c, EXTERNAL_WEBHOOK_BODY_MAX_BYTES); + if (body === null) { return reply(c, 413, 'Body too large'); } @@ -162,7 +230,9 @@ export function createExternalWebhookHandler(): Handler { agentType, error: String(err), }); - return reply(c, 500, `Enqueue failed: ${String(err)}`); + return reply(c, 500, `Enqueue failed: ${String(err)}`, { + publicMessage: 'Failed to queue the run', + }); } return reply(c, 200, `Job queued: ${agentType} agent (external webhook)`, { diff --git a/src/router/worker-env.ts b/src/router/worker-env.ts index 9cd3ebee1..14b076faf 100644 --- a/src/router/worker-env.ts +++ b/src/router/worker-env.ts @@ -10,6 +10,7 @@ import { findProjectByRepo, getAllProjectCredentials } from '../config/provider. import { getIntegrationProvider } from '../db/repositories/credentialsRepository.js'; import { extractProjectIdFromJobViaRegistry } from '../integrations/pm/_shared/project-id-extractor.js'; import { captureException } from '../sentry.js'; +import { isExternalWebhookPasswordKey } from '../triggers/shared/external-webhook.js'; import { logger } from '../utils/logging.js'; import { routerConfig } from './config.js'; import { @@ -169,13 +170,21 @@ export async function buildWorkerEnvWithProjectId( // Resolve project credentials in the router and set as individual env vars. // NOTE: CREDENTIAL_MASTER_KEY is intentionally NOT passed to workers. + // NOTE: EXTERNAL_WEBHOOK_PASSWORD_* keys are inbound-auth verifiers for the + // router's external webhook endpoint — agents never need them, and exposing + // them to worker containers would give a prompt-injected agent a direct + // "dispatch agent X with prompt Y" primitive (org-level passwords would + // escalate cross-project). They are excluded here. if (projectId) { try { const secrets = await getAllProjectCredentials(projectId); + const injectedKeys: string[] = []; for (const [key, value] of Object.entries(secrets)) { + if (isExternalWebhookPasswordKey(key)) continue; env.push(`${key}=${value}`); + injectedKeys.push(key); } - env.push(`CASCADE_CREDENTIAL_KEYS=${Object.keys(secrets).join(',')}`); + env.push(`CASCADE_CREDENTIAL_KEYS=${injectedKeys.join(',')}`); } catch (err) { logger.warn('[WorkerManager] Failed to resolve credentials for project:', { projectId, diff --git a/src/triggers/shared/external-webhook.ts b/src/triggers/shared/external-webhook.ts index f14dc2698..588ad6c2e 100644 --- a/src/triggers/shared/external-webhook.ts +++ b/src/triggers/shared/external-webhook.ts @@ -12,6 +12,19 @@ import { TRIGGER_EVENTS } from './events.js'; export const EXTERNAL_WEBHOOK_EVENT = TRIGGER_EVENTS.INTERNAL.EXTERNAL_WEBHOOK; +export const EXTERNAL_WEBHOOK_PASSWORD_PREFIX = 'EXTERNAL_WEBHOOK_PASSWORD_'; + +/** + * True for credential keys that hold inbound webhook passwords. These are + * VERIFIERS for requests arriving at the router — agents never need them, and + * injecting them into worker containers would hand any prompt-injected agent + * a "dispatch agent X with prompt Y" primitive (org-level passwords would even + * escalate cross-project). The worker env builder excludes them. + */ +export function isExternalWebhookPasswordKey(envVarKey: string): boolean { + return envVarKey.startsWith(EXTERNAL_WEBHOOK_PASSWORD_PREFIX); +} + const AGENT_TYPE_SLUG_RE = /^[a-z][a-z0-9-]*$/; /** Agent type slugs are lowercase kebab identifiers (e.g. 'backlog-manager'). */ diff --git a/tests/unit/agents/definitions/profiles.test.ts b/tests/unit/agents/definitions/profiles.test.ts index 3cced97f8..efad10cb5 100644 --- a/tests/unit/agents/definitions/profiles.test.ts +++ b/tests/unit/agents/definitions/profiles.test.ts @@ -22,6 +22,8 @@ vi.mock('../../../../src/agents/capabilities/resolver.js', () => ({ vi.mock('../../../../src/agents/prompts/index.js', () => ({ buildTaskPromptContext: vi.fn().mockReturnValue({ task: 'implement' }), renderInlineTaskPrompt: vi.fn().mockReturnValue('Rendered task prompt'), + // Pass-through like the real implementation for non-external-webhook input + appendExternalTriggerRequest: vi.fn((prompt: string) => prompt), validateTemplate: vi.fn().mockReturnValue({ valid: true }), })); diff --git a/tests/unit/agents/prompts/external-trigger-request.test.ts b/tests/unit/agents/prompts/external-trigger-request.test.ts new file mode 100644 index 000000000..1cb0dc900 --- /dev/null +++ b/tests/unit/agents/prompts/external-trigger-request.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest'; +import { appendExternalTriggerRequest } from '../../../../src/agents/prompts/index.js'; + +describe('appendExternalTriggerRequest', () => { + it('appends the request payload for external-webhook runs', () => { + const result = appendExternalTriggerRequest('Base task prompt.', { + triggerType: 'external-webhook', + triggerCommentBody: '{"message":"fix the login bug"}', + }); + + expect(result).toContain('Base task prompt.'); + expect(result).toContain('## External trigger request'); + expect(result).toContain(''); + expect(result).toContain('{"message":"fix the login bug"}'); + expect(result).toContain(''); + }); + + it('is a no-op for manual runs even with a comment body', () => { + const result = appendExternalTriggerRequest('Base task prompt.', { + triggerType: 'manual', + triggerCommentBody: 'some comment', + }); + expect(result).toBe('Base task prompt.'); + }); + + it('is a no-op for external-webhook runs without a body', () => { + const result = appendExternalTriggerRequest('Base task prompt.', { + triggerType: 'external-webhook', + }); + expect(result).toBe('Base task prompt.'); + }); +}); diff --git a/tests/unit/api/routers/_shared/webhookPasswordPolicy.test.ts b/tests/unit/api/routers/_shared/webhookPasswordPolicy.test.ts new file mode 100644 index 000000000..4210161e8 --- /dev/null +++ b/tests/unit/api/routers/_shared/webhookPasswordPolicy.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest'; +import { + assertWebhookPasswordStrength, + EXTERNAL_WEBHOOK_PASSWORD_MIN_LENGTH, +} from '../../../../../src/api/routers/_shared/webhookPasswordPolicy.js'; + +describe('assertWebhookPasswordStrength', () => { + it('rejects short webhook passwords', () => { + expect(() => + assertWebhookPasswordStrength('EXTERNAL_WEBHOOK_PASSWORD_IMPLEMENTATION', 'short'), + ).toThrow(/at least 16 characters/); + }); + + it('accepts webhook passwords at the minimum length', () => { + expect(() => + assertWebhookPasswordStrength( + 'EXTERNAL_WEBHOOK_PASSWORD_IMPLEMENTATION', + 'x'.repeat(EXTERNAL_WEBHOOK_PASSWORD_MIN_LENGTH), + ), + ).not.toThrow(); + }); + + it('ignores non-webhook credential keys entirely', () => { + expect(() => assertWebhookPasswordStrength('GITHUB_TOKEN_IMPLEMENTER', 'x')).not.toThrow(); + expect(() => assertWebhookPasswordStrength('OPENROUTER_API_KEY', 'a')).not.toThrow(); + }); +}); diff --git a/tests/unit/router/external-webhook.test.ts b/tests/unit/router/external-webhook.test.ts index 270caf6ba..cd7b5fce2 100644 --- a/tests/unit/router/external-webhook.test.ts +++ b/tests/unit/router/external-webhook.test.ts @@ -1,5 +1,6 @@ import { Hono } from 'hono'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { _resetForTesting } from '../../../src/api/auth/rateLimiter.js'; // Must mock heavy imports BEFORE importing the module under test const { @@ -59,6 +60,13 @@ function buildApp(): Hono { return app; } +let ipCounter = 0; +/** Unique per-test client IP so the shared rate limiter never couples tests. */ +function nextIp(): string { + ipCounter += 1; + return `10.0.0.${ipCounter}`; +} + function post( app: Hono, { @@ -66,12 +74,13 @@ function post( agentType = 'implementation', body = '{"message":"do the thing"}', headers = {} as Record, + ip = nextIp(), } = {}, ): Promise { return app.fetch( new Request(`http://localhost/external/webhook/${projectId}/${agentType}`, { method: 'POST', - headers, + headers: { 'x-forwarded-for': ip, ...headers }, body, }), ); @@ -82,6 +91,7 @@ const bearer = (password: string) => ({ Authorization: `Bearer ${password}` }); describe('external webhook endpoint', () => { beforeEach(() => { vi.clearAllMocks(); + _resetForTesting(); mockLoadProjectConfigById.mockResolvedValue({ project: { id: 'proj-1' }, config: {} }); mockResolveProjectCredential.mockResolvedValue('correct-password'); mockGetResolvedTriggerConfig.mockResolvedValue({ enabled: true, parameters: {} }); @@ -90,16 +100,51 @@ describe('external webhook endpoint', () => { mockSubmitDashboardJob.mockResolvedValue(undefined); }); - describe('fail closed', () => { - it('rejects with 403 when no password is configured — never dispatches', async () => { + describe('fail closed + anti-enumeration', () => { + it('rejects with generic 401 when no password is configured — never dispatches', async () => { mockResolveProjectCredential.mockResolvedValue(null); const res = await post(buildApp(), { headers: bearer('anything') }); - expect(res.status).toBe(403); + expect(res.status).toBe(401); + expect(await res.json()).toEqual({ error: 'Unauthorized' }); expect(mockSubmitDashboardJob).not.toHaveBeenCalled(); expect(mockCreateQueuedRun).not.toHaveBeenCalled(); }); + + it('unknown project, unset password, and wrong token are indistinguishable to callers', async () => { + const app = buildApp(); + + mockLoadProjectConfigById.mockResolvedValueOnce(undefined); + const unknownProject = await post(app, { headers: bearer('x') }); + + mockResolveProjectCredential.mockResolvedValueOnce(null); + const noPassword = await post(app, { headers: bearer('x') }); + + const wrongToken = await post(app, { headers: bearer('wrong-password!!') }); + + const bodies = await Promise.all( + [unknownProject, noPassword, wrongToken].map(async (r) => ({ + status: r.status, + body: await r.json(), + })), + ); + expect(bodies).toEqual([ + { status: 401, body: { error: 'Unauthorized' } }, + { status: 401, body: { error: 'Unauthorized' } }, + { status: 401, body: { error: 'Unauthorized' } }, + ]); + }); + + it('keeps distinct decision reasons in webhook logs for operators', async () => { + mockLoadProjectConfigById.mockResolvedValueOnce(undefined); + await post(buildApp(), { headers: bearer('x') }); + + const reasons = mockLogWebhookCall.mock.calls.map( + (call) => (call[0] as { decisionReason?: string }).decisionReason, + ); + expect(reasons).toContain('Unknown project'); + }); }); describe('authentication', () => { @@ -115,18 +160,20 @@ describe('external webhook endpoint', () => { expect(res.status).toBe(401); }); + it('accepts a case-insensitive bearer scheme (RFC 7235)', async () => { + const res = await post(buildApp(), { + headers: { Authorization: 'bearer correct-password' }, + }); + expect(res.status).toBe(200); + }); + it('rejects wrong password with 401', async () => { const res = await post(buildApp(), { headers: bearer('wrong-password!') }); expect(res.status).toBe(401); expect(mockSubmitDashboardJob).not.toHaveBeenCalled(); }); - it('rejects wrong-length password with 401', async () => { - const res = await post(buildApp(), { headers: bearer('short') }); - expect(res.status).toBe(401); - }); - - it('never passes the Authorization header to webhook logs', async () => { + it('never passes the Authorization header or password to webhook logs', async () => { await post(buildApp(), { headers: bearer('correct-password') }); for (const call of mockLogWebhookCall.mock.calls) { @@ -137,6 +184,39 @@ describe('external webhook endpoint', () => { }); }); + describe('rate limiting', () => { + it('returns 429 with Retry-After after repeated attempts from one IP', async () => { + const app = buildApp(); + const ip = nextIp(); + + let lastStatus = 0; + for (let i = 0; i < 12; i++) { + const res = await post(app, { headers: bearer('wrong-password!'), ip }); + lastStatus = res.status; + if (lastStatus === 429) { + expect(res.headers.get('Retry-After')).toBeTruthy(); + break; + } + } + expect(lastStatus).toBe(429); + }); + + it('successful auth resets the counter for the IP', async () => { + const app = buildApp(); + const ip = nextIp(); + + for (let i = 0; i < 5; i++) { + await post(app, { headers: bearer('wrong-password!'), ip }); + } + const success = await post(app, { headers: bearer('correct-password'), ip }); + expect(success.status).toBe(200); + + // Counter reset — several more attempts allowed before limiting again + const next = await post(app, { headers: bearer('wrong-password!'), ip }); + expect(next.status).toBe(401); + }); + }); + describe('routing guards', () => { it('returns 404 for an invalid agent type slug', async () => { const res = await post(buildApp(), { agentType: 'Not-Valid!' }); @@ -144,13 +224,7 @@ describe('external webhook endpoint', () => { expect(mockLoadProjectConfigById).not.toHaveBeenCalled(); }); - it('returns 404 for an unknown project', async () => { - mockLoadProjectConfigById.mockResolvedValue(undefined); - const res = await post(buildApp(), { headers: bearer('correct-password') }); - expect(res.status).toBe(404); - }); - - it('returns 404 when the agent is not enabled or trigger undeclared', async () => { + it('returns 404 when the agent is not enabled or trigger undeclared (post-auth)', async () => { mockGetResolvedTriggerConfig.mockResolvedValue(null); const res = await post(buildApp(), { headers: bearer('correct-password') }); expect(res.status).toBe(404); @@ -170,17 +244,7 @@ describe('external webhook endpoint', () => { }); describe('body handling', () => { - it('rejects oversized declared content-length with 413', async () => { - const res = await post(buildApp(), { - headers: { - ...bearer('correct-password'), - 'content-length': String(EXTERNAL_WEBHOOK_BODY_MAX_BYTES + 1), - }, - }); - expect(res.status).toBe(413); - }); - - it('rejects oversized actual body with 413', async () => { + it('rejects an oversized body with 413 without dispatching', async () => { const res = await post(buildApp(), { headers: bearer('correct-password'), body: 'x'.repeat(EXTERNAL_WEBHOOK_BODY_MAX_BYTES + 1), @@ -222,12 +286,14 @@ describe('external webhook endpoint', () => { }); }); - it('fails the pre-created run and returns 500 when enqueue throws', async () => { - mockSubmitDashboardJob.mockRejectedValue(new Error('redis down')); + it('fails the pre-created run and returns a generic 500 when enqueue throws', async () => { + mockSubmitDashboardJob.mockRejectedValue(new Error('redis down at 10.0.0.5:6379')); const res = await post(buildApp(), { headers: bearer('correct-password') }); expect(res.status).toBe(500); + // Internal detail stays out of the response body + expect(JSON.stringify(await res.json())).not.toContain('redis'); expect(mockFailQueuedOrRunningRun).toHaveBeenCalledWith( 'run-123', 'Failed to enqueue external webhook run', diff --git a/tests/unit/router/worker-env.test.ts b/tests/unit/router/worker-env.test.ts index 15f98dd60..eba5d015e 100644 --- a/tests/unit/router/worker-env.test.ts +++ b/tests/unit/router/worker-env.test.ts @@ -163,6 +163,20 @@ describe('buildWorkerEnv', () => { expect(env).toContain('CASCADE_CREDENTIAL_KEYS=GITHUB_TOKEN'); }); + it('excludes EXTERNAL_WEBHOOK_PASSWORD_* keys from worker env (inbound-auth verifiers)', async () => { + mockGetAllProjectCredentials.mockResolvedValue({ + GITHUB_TOKEN: 'ghp_test', + EXTERNAL_WEBHOOK_PASSWORD_IMPLEMENTATION: 'super-secret-webhook-pw', + }); + + const env = await buildWorkerEnv(makeJob() as never); + + expect(env.some((e) => e.includes('EXTERNAL_WEBHOOK_PASSWORD'))).toBe(false); + expect(env.some((e) => e.includes('super-secret-webhook-pw'))).toBe(false); + expect(env).toContain('GITHUB_TOKEN=ghp_test'); + expect(env).toContain('CASCADE_CREDENTIAL_KEYS=GITHUB_TOKEN'); + }); + it('skips credential env vars if credential resolution fails', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); mockGetAllProjectCredentials.mockRejectedValue(new Error('DB error')); diff --git a/web/src/components/projects/external-webhook-trigger-config.tsx b/web/src/components/projects/external-webhook-trigger-config.tsx index 244b0b0bb..eb0bd325d 100644 --- a/web/src/components/projects/external-webhook-trigger-config.tsx +++ b/web/src/components/projects/external-webhook-trigger-config.tsx @@ -10,7 +10,6 @@ import { AlertTriangle } from 'lucide-react'; import { ProjectSecretField } from '@/components/projects/project-secret-field.js'; import { CopyButton } from '@/components/ui/copy-button.js'; import { Label } from '@/components/ui/label.js'; -import { API_URL } from '@/lib/api.js'; import { trpc } from '@/lib/trpc.js'; import { externalWebhookCredentialKey, @@ -37,10 +36,14 @@ export function ExternalWebhookTriggerConfig({ if (!enabled) return null; - const callbackBaseUrl = - publicUrlQuery.data?.routerPublicUrl ?? - API_URL ?? - (typeof window !== 'undefined' ? window.location.origin.replace(':5173', ':3000') : ''); + // The URL must point at the ROUTER service, so API_URL (the dashboard API) + // is deliberately not a fallback here. WEBHOOK_CALLBACK_BASE_URL is the + // authoritative source; the dev-server origin swap covers local dev only. + const devOrigin = + typeof window !== 'undefined' && window.location.origin.includes(':5173') + ? window.location.origin.replace(':5173', ':3000') + : ''; + const callbackBaseUrl = publicUrlQuery.data?.routerPublicUrl ?? devOrigin; const webhookUrl = `${callbackBaseUrl || ''}${externalWebhookPath(projectId, agentType)}`; const credentialKey = externalWebhookCredentialKey(agentType); @@ -80,7 +83,7 @@ export function ExternalWebhookTriggerConfig({ {!credential?.isConfigured && (
- Requests are rejected with 403 until a password is set. + Requests are rejected until a password is set.
)} @@ -88,8 +91,8 @@ export function ExternalWebhookTriggerConfig({ projectId={projectId} envVarKey={credentialKey} label="Webhook Password" - description="Sent by callers as 'Authorization: Bearer '. Required — requests are rejected until set." - placeholder="Choose a strong password..." + description="Sent by callers as 'Authorization: Bearer '. Required (minimum 16 characters) — requests are rejected until set." + placeholder="Choose a strong password (16+ characters)..." credential={credential} />