From d69d7c636f364f5c304810700801dd15a11e06c0 Mon Sep 17 00:00:00 2001 From: Hakan Kaya Date: Mon, 19 Oct 2026 13:40:38 +0200 Subject: [PATCH 1/2] feat(automation): filter api_trigger deliveries by GitHub event and action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An api_trigger fired one run per webhook delivery. With a GitHub hook subscribed to check_run — one delivery per CI job per state — a single push produced dozens of runs: 3,291 runs in 2.5 days on one automation, ~85% of them check_run. The graph's github.pr_state condition cannot prevent that; it runs after the trigger, and by then a trigger_ai_agent node may already have requested a sandbox. Handler now reads the X-GitHub-Event header and the body (bounded), and the service drops a delivery that matches none of the trigger's optional github_events entries, answering 202 as before so the sender has nothing to retry. Absent/empty filter keeps today's behaviour exactly. Fixes #605 --- .../api/internal/domain/automation/entity.go | 22 ++++ .../api/internal/domain/automation/service.go | 8 +- .../service/automation/automation_service.go | 49 ++++++++- .../automation/automation_service_test.go | 101 ++++++++++++++++++ .../http/handler/automation_handler.go | 11 +- .../api/test/e2e/automation_engine_test.go | 71 ++++++++++++ 6 files changed, 256 insertions(+), 6 deletions(-) diff --git a/services/api/internal/domain/automation/entity.go b/services/api/internal/domain/automation/entity.go index e97edbdb4..b4e935178 100644 --- a/services/api/internal/domain/automation/entity.go +++ b/services/api/internal/domain/automation/entity.go @@ -418,6 +418,28 @@ type TriggerConfig struct { // for this one sprint. Nil means "any sprint in the project" — the same // optional-narrowing shape as StatusID above. SprintID *uuid.UUID `json:"sprint_id,omitempty"` + // GithubEvents narrows TriggerAPITrigger to webhook deliveries GitHub + // actually sent for one of these events. Each entry is "event" or + // "event:action", matched against the X-GitHub-Event header and the + // JSON body's "action" field ("pull_request:opened"). Empty means + // every delivery fires — the behaviour of every api_trigger that + // predates this field. + // + // A delivery matching no entry is discarded: still 202, no run, no + // stream entry, so the sender has nothing to retry and cannot probe the + // filter through the response. GitHub emits one delivery per CI job per + // state, so without this a single push can fire dozens of runs. + GithubEvents []string `json:"github_events,omitempty"` +} + +// WebhookDelivery is the part of an external webhook POST that delivery +// filtering reads. Both fields are optional: a non-GitHub sender may set +// neither, in which case a non-empty GithubEvents filter never matches. +type WebhookDelivery struct { + // Event is the X-GitHub-Event header value. + Event string + // Body is the raw request body; only "action" is read from it. + Body []byte } // TaskTargetKind selects which task a condition leaf or action operates on, diff --git a/services/api/internal/domain/automation/service.go b/services/api/internal/domain/automation/service.go index a8c6adc50..c589d8be7 100644 --- a/services/api/internal/domain/automation/service.go +++ b/services/api/internal/domain/automation/service.go @@ -46,12 +46,14 @@ type Service interface { // only here — it's never persisted or retrievable again afterward. GenerateWebhookToken(ctx context.Context, projectID, automationID, nodeID uuid.UUID, actor WebhookTokenActor) (*WebhookToken, string, error) // HandleWebhookTrigger verifies rawToken against nodeID's active webhook - // token and, if valid, dispatches a graph-walk run against the node's + // token and, if valid and delivery passes the node's optional + // github_events filter, dispatches a graph-walk run against the node's // configured target task. Every failure path (unknown node, wrong // type, inactive automation, wrong/missing token) returns // ErrWebhookTokenInvalid or ErrNotFound so an unauthenticated caller - // can't distinguish "no such webhook" from "wrong token". - HandleWebhookTrigger(ctx context.Context, nodeID uuid.UUID, rawToken string) error + // can't distinguish "no such webhook" from "wrong token"; a delivery + // filtered out is not a failure and returns nil with nothing enqueued. + HandleWebhookTrigger(ctx context.Context, nodeID uuid.UUID, rawToken string, delivery WebhookDelivery) error } // WebhookTokenActor identifies who requested a webhook token generation, for diff --git a/services/api/internal/service/automation/automation_service.go b/services/api/internal/service/automation/automation_service.go index 8f3af430b..804562946 100644 --- a/services/api/internal/service/automation/automation_service.go +++ b/services/api/internal/service/automation/automation_service.go @@ -590,13 +590,39 @@ func (s *Service) GenerateWebhookToken(ctx context.Context, projectID, automatio return created, rawToken, nil } +// matchesGithubEvents reports whether delivery satisfies patterns. No +// patterns means "every delivery" — the behaviour of every api_trigger that +// predates the filter. An entry is "event" or "event:action", matched +// against the X-GitHub-Event header and the body's "action" field; a sender +// that sets neither matches only an absent filter. Malformed bodies are not +// an error here: an unparseable body simply has no action to match. +func matchesGithubEvents(patterns []string, delivery automationdom.WebhookDelivery) bool { + if len(patterns) == 0 { + return true + } + var body struct { + Action string `json:"action"` + } + _ = json.Unmarshal(delivery.Body, &body) + for _, pattern := range patterns { + event, action, hasAction := strings.Cut(pattern, ":") + if event != delivery.Event { + continue + } + if !hasAction || action == body.Action { + return true + } + } + return false +} + // HandleWebhookTrigger verifies rawToken against nodeID's active webhook // token and, if valid, dispatches an async graph-walk run against the // node's configured target task. Every failure path deliberately returns // the same class of error (ErrNotFound or ErrWebhookTokenInvalid) // regardless of which specific thing was wrong — an unauthenticated caller // can't use this to probe whether a webhook exists, or is just misconfigured. -func (s *Service) HandleWebhookTrigger(ctx context.Context, nodeID uuid.UUID, rawToken string) error { +func (s *Service) HandleWebhookTrigger(ctx context.Context, nodeID uuid.UUID, rawToken string, delivery automationdom.WebhookDelivery) error { node, err := s.repo.FindNodeByID(ctx, nodeID) if err != nil { return err @@ -628,6 +654,13 @@ func (s *Service) HandleWebhookTrigger(ctx context.Context, nodeID uuid.UUID, ra // Best-effort — a failure to record usage shouldn't block the trigger. _ = s.repo.RecordWebhookTokenUsed(ctx, tok.ID, time.Now()) + // Filtered deliveries are accepted and dropped: 202 with no run, so the + // sender sees a normal acceptance and does not retry a delivery this + // automation never wanted. + if !matchesGithubEvents(cfg.GithubEvents, delivery) { + return nil + } + // task_id is optional — TargetTaskID may be unset (validateTaskReachability // guarantees that's only possible when this node has no downstream nodes // other than call_api actions), in which case the consumer runs the walk @@ -794,7 +827,19 @@ func (s *Service) validateTriggerConfig(ctx context.Context, projectID uuid.UUID // TargetTaskID is optional — see the predecessor_done branch above. } // api_trigger has no fields of its own beyond the shared TargetTaskID - // (optional, see above). + // (optional, see above), plus the optional GithubEvents filter, which no + // other trigger type can use — nothing else consumes a webhook delivery. + if len(cfg.GithubEvents) > 0 { + if t != automationdom.TriggerAPITrigger { + return fmt.Errorf("%w: github_events is only valid on api_trigger", automationdom.ErrNodeConfigInvalid) + } + for _, entry := range cfg.GithubEvents { + event, action, hasAction := strings.Cut(entry, ":") + if event == "" || strings.Contains(action, ":") || (hasAction && action == "") { + return fmt.Errorf("%w: invalid github_events entry %q, want \"event\" or \"event:action\"", automationdom.ErrNodeConfigInvalid, entry) + } + } + } // Shared across every trigger type that carries a TargetTaskID // (predecessor_done, cron, api_trigger): whatever task is set must // belong to the same project. diff --git a/services/api/internal/service/automation/automation_service_test.go b/services/api/internal/service/automation/automation_service_test.go index de06feaaf..3ce75b361 100644 --- a/services/api/internal/service/automation/automation_service_test.go +++ b/services/api/internal/service/automation/automation_service_test.go @@ -839,3 +839,104 @@ func TestValidateNoSelfParent_DynamicTarget_Skipped(t *testing.T) { t.Fatalf("expected a children-targeted update_task to be skipped by the static check, got %v", err) } } + +// --- github_events delivery filter ------------------------------------------ + +// The filter is what keeps a GitHub webhook from firing the graph once per CI +// job: without it every delivery runs the automation, and a single push fans +// out into dozens of runs (one per check_run state per job). +func TestMatchesGithubEvents(t *testing.T) { + cases := []struct { + name string + patterns []string + delivery automationdom.WebhookDelivery + want bool + }{ + { + name: "no filter accepts every delivery", + patterns: nil, + delivery: automationdom.WebhookDelivery{Event: "check_run", Body: []byte(`{"action":"completed"}`)}, + want: true, + }, + { + name: "event alone matches any action", + patterns: []string{"pull_request"}, + delivery: automationdom.WebhookDelivery{Event: "pull_request", Body: []byte(`{"action":"labeled"}`)}, + want: true, + }, + { + name: "event:action matches only that action", + patterns: []string{"pull_request:opened"}, + delivery: automationdom.WebhookDelivery{Event: "pull_request", Body: []byte(`{"action":"opened"}`)}, + want: true, + }, + { + name: "event:action rejects a different action", + patterns: []string{"pull_request:opened"}, + delivery: automationdom.WebhookDelivery{Event: "pull_request", Body: []byte(`{"action":"labeled"}`)}, + want: false, + }, + { + name: "event:action rejects a different event", + patterns: []string{"pull_request:opened"}, + delivery: automationdom.WebhookDelivery{Event: "check_run", Body: []byte(`{"action":"opened"}`)}, + want: false, + }, + { + name: "any matching entry is enough", + patterns: []string{"push", "pull_request:opened", "pull_request:synchronize"}, + delivery: automationdom.WebhookDelivery{Event: "pull_request", Body: []byte(`{"action":"synchronize"}`)}, + want: true, + }, + { + name: "unparseable body matches an event-only entry", + patterns: []string{"push"}, + delivery: automationdom.WebhookDelivery{Event: "push", Body: []byte(`not json`)}, + want: true, + }, + { + name: "unparseable body cannot match event:action", + patterns: []string{"push:created"}, + delivery: automationdom.WebhookDelivery{Event: "push", Body: []byte(`not json`)}, + want: false, + }, + { + name: "missing X-GitHub-Event matches nothing", + patterns: []string{"pull_request"}, + delivery: automationdom.WebhookDelivery{Body: []byte(`{"action":"opened"}`)}, + want: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := matchesGithubEvents(tc.patterns, tc.delivery); got != tc.want { + t.Fatalf("matchesGithubEvents(%v, %+v) = %v, want %v", tc.patterns, tc.delivery, got, tc.want) + } + }) + } +} + +func TestValidateTriggerConfig_GithubEvents(t *testing.T) { + svc := &Service{} + withEvents := func(events ...string) json.RawMessage { + raw, err := json.Marshal(automationdom.TriggerConfig{GithubEvents: events}) + if err != nil { + t.Fatalf("marshal: %v", err) + } + return raw + } + if err := svc.validateTriggerConfig(context.TODO(), uuid.Nil, automationdom.TriggerAPITrigger, + withEvents("pull_request:opened", "pull_request:synchronize", "push"), true); err != nil { + t.Fatalf("expected valid github_events to pass validation, got %v", err) + } + for _, entry := range []string{"", ":opened", "pull_request:", "pull_request:opened:extra"} { + if err := svc.validateTriggerConfig(context.TODO(), uuid.Nil, automationdom.TriggerAPITrigger, + withEvents(entry), true); err == nil { + t.Fatalf("expected github_events entry %q to be rejected", entry) + } + } + if err := svc.validateTriggerConfig(context.TODO(), uuid.Nil, automationdom.TriggerTaskCreated, + withEvents("push"), true); err == nil { + t.Fatal("expected github_events to be rejected on a trigger type that consumes no webhook") + } +} diff --git a/services/api/internal/transport/http/handler/automation_handler.go b/services/api/internal/transport/http/handler/automation_handler.go index 5423cfed8..d6b48b797 100644 --- a/services/api/internal/transport/http/handler/automation_handler.go +++ b/services/api/internal/transport/http/handler/automation_handler.go @@ -1,6 +1,7 @@ package handler import ( + "io" "net/http" "strings" @@ -558,7 +559,15 @@ func (h *AutomationHandler) ReceiveWebhook(w http.ResponseWriter, r *http.Reques presenter.Error(w, r, automationdom.ErrWebhookTokenInvalid) return } - if err := h.svc.HandleWebhookTrigger(r.Context(), nodeID, token); err != nil { + // Bounded read: only the body's "action" is ever inspected (delivery + // filtering), and this endpoint is reachable by anyone holding the URL. + body, err := io.ReadAll(io.LimitReader(r.Body, 1<<20)) + if err != nil { + presenter.Error(w, r, apierr.New(apierr.CodeBadRequest, "unreadable webhook body")) + return + } + delivery := automationdom.WebhookDelivery{Event: r.Header.Get("X-GitHub-Event"), Body: body} + if err := h.svc.HandleWebhookTrigger(r.Context(), nodeID, token, delivery); err != nil { presenter.Error(w, r, err) return } diff --git a/services/api/test/e2e/automation_engine_test.go b/services/api/test/e2e/automation_engine_test.go index a4c8acb26..ff954bd52 100644 --- a/services/api/test/e2e/automation_engine_test.go +++ b/services/api/test/e2e/automation_engine_test.go @@ -1,6 +1,7 @@ package e2e_test import ( + "bytes" "fmt" "io" "log/slog" @@ -286,6 +287,25 @@ func postWebhookExpect(t *testing.T, env *e2eEnv, nodeID, webhookToken string, w assertStatus(t, resp, wantStatus) } +// postWebhookEventExpect POSTs to the public webhook receiver with a GitHub +// delivery's identity — the X-GitHub-Event header and the JSON body whose +// "action" the api_trigger's github_events filter matches on. +func postWebhookEventExpect(t *testing.T, env *e2eEnv, nodeID, webhookToken, githubEvent, body string, wantStatus int) { + t.Helper() + req := mustRequest(env.ctx, t, http.MethodPost, + fmt.Sprintf("%s/api/v1/webhooks/automations/%s", env.base, nodeID), bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + if webhookToken != "" { + req.Header.Set("X-Webhook-Token", webhookToken) + } + if githubEvent != "" { + req.Header.Set("X-GitHub-Event", githubEvent) + } + resp := mustDo(t, http.DefaultClient, req) + defer func() { _ = resp.Body.Close() }() + assertStatus(t, resp, wantStatus) +} + // waitForTaskField polls the task until getField(data) returns true, or fails // the test after timeout. Reassignment/mutation happens asynchronously via a // Valkey-stream event the AutomationConsumer processes in the background. @@ -876,6 +896,57 @@ func TestE2EAutomation_WebhookTriggerTokenLifecycle(t *testing.T) { postWebhookExpect(t, env, triggerID, newToken, http.StatusAccepted) } +// TestE2EAutomation_WebhookGithubEventsFilter is the regression guard for a +// webhook that fires the graph once per delivery: an api_trigger with +// github_events must accept and drop deliveries it did not ask for, without +// creating a run, and still fire on the ones it did. +func TestE2EAutomation_WebhookGithubEventsFilter(t *testing.T) { + t.Parallel() + env := newE2EEnv(t) + startAutomationConsumer(t, env) + + ownerUsername := "automation-filter-owner-" + uuid.NewString() + seedTaskMemberUser(t, env, ownerUsername, "automationfilterowner1") + ownerClient, ownerToken := taskMemberLogin(t, env, ownerUsername, "automationfilterowner1") + projID := createProjectForTasksViaAPI(t, env, ownerClient, ownerToken) + + memberID := addProjectMemberWithAutomationPerms(t, env, ownerClient, ownerToken, projID, + "automation-filter-assignee-"+uuid.NewString(), "automationfilterassignee1") + + task := createTaskViaAPIWithBody(t, env, ownerClient, ownerToken, projID, map[string]any{"title": "Filter Target Task"}) + taskID, _ := task["id"].(string) + + automationID := createAutomationViaAPI(t, env, ownerClient, ownerToken, projID, "Filtered Webhook Automation") + trigger := addAutomationNodeViaAPI(t, env, ownerClient, ownerToken, projID, automationID, + "trigger", "api_trigger", map[string]any{ + "target_task_id": taskID, + "github_events": []string{"pull_request:opened", "pull_request:synchronize"}, + }) + action := addAutomationNodeViaAPI(t, env, ownerClient, ownerToken, projID, automationID, + "action", "update_task", map[string]any{"update": map[string]any{"assignee_ids": []string{memberID}}}) + triggerID, _ := trigger["id"].(string) + actionID, _ := action["id"].(string) + addAutomationEdgeViaAPI(t, env, ownerClient, ownerToken, projID, automationID, triggerID, actionID) + activateAutomationViaAPI(t, env, ownerClient, ownerToken, projID, automationID) + rawToken := generateWebhookTokenViaAPI(t, env, ownerClient, ownerToken, projID, automationID, triggerID) + + // A CI job event is exactly what the filter exists to discard. + postWebhookEventExpect(t, env, triggerID, rawToken, "check_run", `{"action":"completed"}`, http.StatusAccepted) + // Same event as a wanted entry, different action. + postWebhookEventExpect(t, env, triggerID, rawToken, "pull_request", `{"action":"labeled"}`, http.StatusAccepted) + time.Sleep(3 * time.Second) + if runs := listAutomationRunsViaAPI(t, env, ownerClient, ownerToken, projID, automationID); len(runs) != 0 { + t.Fatalf("expected no runs for filtered deliveries, got %d", len(runs)) + } + if data := getTaskViaAPI(t, env, ownerClient, ownerToken, projID, taskID); len(data["assignee_ids"].([]any)) != 0 { + t.Fatal("expected a filtered delivery to leave the task untouched") + } + + // A wanted event still fires. + postWebhookEventExpect(t, env, triggerID, rawToken, "pull_request", `{"action":"opened"}`, http.StatusAccepted) + waitForAutomationAssignee(t, env, ownerClient, ownerToken, projID, taskID, memberID, 20*time.Second) +} + // --------------------------------------------------------------------------- // Engine: call_api action — outbound HTTP request // --------------------------------------------------------------------------- From ef63d8ffc089a2c71a3a7e7540ab0952cc80ef39 Mon Sep 17 00:00:00 2001 From: Hakan Kaya Date: Mon, 19 Oct 2026 13:49:38 +0200 Subject: [PATCH 2/2] fix(web): keep api_trigger config fields the builder cannot edit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The api_trigger form saved only {target_task_id} and the node PATCH replaces the config wholesale, so a github_events set through the API was silently dropped the next time anyone opened the trigger in the builder and pressed Save — restoring the runaway-run behaviour the filter exists to prevent. Spread the stored config into the saved object (target_task_id still comes from the form, so clearing it still clears) and document the field on the TriggerConfig interface. --- .../automation/automation-node-config-panel.tsx | 10 +++++++++- apps/web/src/lib/automation-api.ts | 6 ++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/apps/web/src/components/projects/automation/automation-node-config-panel.tsx b/apps/web/src/components/projects/automation/automation-node-config-panel.tsx index 49d847fa6..5b5289f1d 100644 --- a/apps/web/src/components/projects/automation/automation-node-config-panel.tsx +++ b/apps/web/src/components/projects/automation/automation-node-config-panel.tsx @@ -1189,7 +1189,15 @@ function TriggerConfigForm({ - onSave({ target_task_id: targetTaskId.trim() || undefined }) + onSave({ + // The PATCH replaces the node's whole config, so + // carry fields this form does not edit — github_events + // is API-only today and would otherwise be wiped by + // the next builder save, restoring the runaway-run + // behaviour that filter exists to stop. + ...config, + target_task_id: targetTaskId.trim() || undefined, + }) } /> )} diff --git a/apps/web/src/lib/automation-api.ts b/apps/web/src/lib/automation-api.ts index 90fe57af7..f507a79b6 100644 --- a/apps/web/src/lib/automation-api.ts +++ b/apps/web/src/lib/automation-api.ts @@ -210,6 +210,12 @@ export interface TriggerConfig { /** Narrows any of the four sprint_* trigger types to firing only for * this one sprint. Omitted means "any sprint in the project". */ sprint_id?: string; + /** api_trigger only: deliveries to accept, each "event" or + * "event:action" matched against X-GitHub-Event and the body's action + * ("pull_request:opened"). Omitted means every delivery fires. Not + * editable in the builder yet — the api_trigger form preserves whatever + * is stored rather than overwriting it. */ + github_events?: string[]; } export type ConditionOperator =